From f790d5cfe645f1e2075ab7c053517631be4458c7 Mon Sep 17 00:00:00 2001 From: James Moger Date: Mon, 15 Jun 2015 12:25:14 -0400 Subject: [PATCH] Revised AvatarGenerator to be injectable --- src/main/distrib/data/defaults.properties | 2 - .../java/com/gitblit/AvatarGenerator.java | 14 +-- .../java/com/gitblit/GravatarGenerator.java | 10 +- .../guice/AvatarGeneratorProvider.java | 71 +++++++++++++ .../java/com/gitblit/guice/WebModule.java | 5 + .../java/com/gitblit/utils/ActivityUtils.java | 4 +- .../com/gitblit/wicket/GitBlitWebApp.java | 36 +------ .../com/gitblit/wicket/GitblitWicketApp.java | 2 - .../gitblit/wicket/panels/AvatarImage.java | 4 +- .../java/com/gitblit/tests/GitBlitSuite.java | 3 +- .../java/com/gitblit/tests/GravatarTest.java | 99 +++++++++---------- 11 files changed, 142 insertions(+), 108 deletions(-) create mode 100644 src/main/java/com/gitblit/guice/AvatarGeneratorProvider.java diff --git a/src/main/distrib/data/defaults.properties b/src/main/distrib/data/defaults.properties index aa0d2cfa..0d556fdc 100644 --- a/src/main/distrib/data/defaults.properties +++ b/src/main/distrib/data/defaults.properties @@ -976,8 +976,6 @@ web.useResponsiveLayout = true web.allowGravatar = true # Define which class will generate the avatar URL. -# This class must inherit from com.gitblit.AvatarGenerator -# It must use a no argument constructor # # SINCE 1.7.0 web.avatarClass = com.gitblit.GravatarGenerator diff --git a/src/main/java/com/gitblit/AvatarGenerator.java b/src/main/java/com/gitblit/AvatarGenerator.java index 76f189de..0415f925 100644 --- a/src/main/java/com/gitblit/AvatarGenerator.java +++ b/src/main/java/com/gitblit/AvatarGenerator.java @@ -15,18 +15,8 @@ */ package com.gitblit; -public abstract class AvatarGenerator { +public interface AvatarGenerator { - public abstract String getURL(String username, String emailaddress, boolean identicon, int width); - - /** - * A method that can extract custom settings for the avatar generator - * The default does nothing, it can be overridden - * - * @param settings - */ - public void configure(IStoredSettings settings) { - - } + String getURL(String username, String emailaddress, boolean identicon, int width); } diff --git a/src/main/java/com/gitblit/GravatarGenerator.java b/src/main/java/com/gitblit/GravatarGenerator.java index 796a1a88..1ba02e55 100644 --- a/src/main/java/com/gitblit/GravatarGenerator.java +++ b/src/main/java/com/gitblit/GravatarGenerator.java @@ -16,12 +16,14 @@ package com.gitblit; import com.gitblit.utils.ActivityUtils; +import com.google.inject.Singleton; -public class GravatarGenerator extends AvatarGenerator { +@Singleton +public class GravatarGenerator implements AvatarGenerator { - public String getURL(String username, String emailaddress, - boolean identicon, int width) { - String email = emailaddress == null ? username.toLowerCase() : emailaddress.toLowerCase(); + @Override + public String getURL(String username, String emailaddress, boolean identicon, int width) { + String email = emailaddress == null ? username : emailaddress; if (identicon) { return ActivityUtils.getGravatarIdenticonUrl(email, width); } else { diff --git a/src/main/java/com/gitblit/guice/AvatarGeneratorProvider.java b/src/main/java/com/gitblit/guice/AvatarGeneratorProvider.java new file mode 100644 index 00000000..a18e2996 --- /dev/null +++ b/src/main/java/com/gitblit/guice/AvatarGeneratorProvider.java @@ -0,0 +1,71 @@ +/* + * Copyright 2015 gitblit.com. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.gitblit.guice; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.gitblit.AvatarGenerator; +import com.gitblit.GravatarGenerator; +import com.gitblit.IStoredSettings; +import com.gitblit.Keys; +import com.gitblit.manager.IRuntimeManager; +import com.gitblit.utils.StringUtils; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; + +/** + * Provides a lazily-instantiated AvatarGenerator configured from IStoredSettings. + * + * @author James Moger + * + */ +@Singleton +public class AvatarGeneratorProvider implements Provider { + + private final Logger logger = LoggerFactory.getLogger(getClass()); + + private final IRuntimeManager runtimeManager; + + private volatile AvatarGenerator avatarGenerator; + + @Inject + public AvatarGeneratorProvider(IRuntimeManager runtimeManager) { + this.runtimeManager = runtimeManager; + } + + @Override + public synchronized AvatarGenerator get() { + if (avatarGenerator != null) { + return avatarGenerator; + } + + IStoredSettings settings = runtimeManager.getSettings(); + String clazz = settings.getString(Keys.web.avatarClass, GravatarGenerator.class.getName()); + if (StringUtils.isEmpty(clazz)) { + clazz = GravatarGenerator.class.getName(); + } + try { + Class generatorClass = (Class) Class.forName(clazz); + avatarGenerator = runtimeManager.getInjector().getInstance(generatorClass); + } catch (Exception e) { + logger.error("failed to create avatar generator", e); + avatarGenerator = new GravatarGenerator(); + } + return avatarGenerator; + } +} \ No newline at end of file diff --git a/src/main/java/com/gitblit/guice/WebModule.java b/src/main/java/com/gitblit/guice/WebModule.java index c6172c3d..a4062703 100644 --- a/src/main/java/com/gitblit/guice/WebModule.java +++ b/src/main/java/com/gitblit/guice/WebModule.java @@ -18,6 +18,7 @@ package com.gitblit.guice; import java.util.HashMap; import java.util.Map; +import com.gitblit.AvatarGenerator; import com.gitblit.Constants; import com.gitblit.servlet.AccessDeniedServlet; import com.gitblit.servlet.BranchGraphServlet; @@ -56,6 +57,10 @@ public class WebModule extends ServletModule { @Override protected void configureServlets() { + + // bind web component providers + bind(AvatarGenerator.class).toProvider(AvatarGeneratorProvider.class); + // servlets serve(fuzzy(Constants.R_PATH), fuzzy(Constants.GIT_PATH)).with(GitServlet.class); serve(fuzzy(Constants.RAW_PATH)).with(RawServlet.class); diff --git a/src/main/java/com/gitblit/utils/ActivityUtils.java b/src/main/java/com/gitblit/utils/ActivityUtils.java index ba5599a1..ab1dad96 100644 --- a/src/main/java/com/gitblit/utils/ActivityUtils.java +++ b/src/main/java/com/gitblit/utils/ActivityUtils.java @@ -169,7 +169,7 @@ public class ActivityUtils { if (width <= 0) { width = 50; } - String emailHash = StringUtils.getMD5(email); + String emailHash = StringUtils.getMD5(email.toLowerCase()); String url = MessageFormat.format( "https://www.gravatar.com/avatar/{0}?s={1,number,0}&d=identicon", emailHash, width); return url; @@ -188,7 +188,7 @@ public class ActivityUtils { if (width <= 0) { width = 50; } - String emailHash = StringUtils.getMD5(email); + String emailHash = StringUtils.getMD5(email.toLowerCase()); String url = MessageFormat.format( "https://www.gravatar.com/avatar/{0}?s={1,number,0}&d=mm", emailHash, width); return url; diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp.java b/src/main/java/com/gitblit/wicket/GitBlitWebApp.java index 39cdbb41..359040b5 100644 --- a/src/main/java/com/gitblit/wicket/GitBlitWebApp.java +++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp.java @@ -32,7 +32,6 @@ import org.apache.wicket.protocol.http.WebApplication; import ro.fortsoft.pf4j.PluginState; import ro.fortsoft.pf4j.PluginWrapper; -import com.gitblit.AvatarGenerator; import com.gitblit.IStoredSettings; import com.gitblit.Keys; import com.gitblit.extensions.GitblitWicketPlugin; @@ -251,15 +250,12 @@ public class GitBlitWebApp extends WebApplication implements GitblitWicketApp { } } - // customize the Wicket class resolver to load from plugins + // customize the Wicket class resolver to load from plugins IClassResolver coreResolver = getApplicationSettings().getClassResolver(); - PluginClassResolver classResolver = new PluginClassResolver(coreResolver, pluginManager); - getApplicationSettings().setClassResolver(classResolver); + PluginClassResolver classResolver = new PluginClassResolver(coreResolver, pluginManager); + getApplicationSettings().setClassResolver(classResolver); getMarkupSettings().setDefaultMarkupEncoding("UTF-8"); - - buildAvatarGenerator(); - } /* (non-Javadoc) @@ -480,30 +476,4 @@ public class GitBlitWebApp extends WebApplication implements GitblitWicketApp { public static GitBlitWebApp get() { return (GitBlitWebApp) WebApplication.get(); } - - AvatarGenerator generator = null; - @SuppressWarnings("unchecked") - private void buildAvatarGenerator() { - Class clazz; - try { - clazz = (Class) getClass().getClassLoader().loadClass(settings.getString(Keys.web.avatarClass, "com.gitblit.GravatarGenerator")); - generator = clazz.newInstance(); - generator.configure(settings); - } catch (ClassNotFoundException | SecurityException | InstantiationException | IllegalAccessException e) { - throw new RuntimeException(e); - } - - } - - public String buildAvatarUrl(String username, String emailaddress, String cssClass, int width, boolean identicon) { - if (width <= 0) { - width = 50; - } - if(generator != null) { - return (String) generator.getURL(username, emailaddress, identicon, width); - } - return null; - } - - } diff --git a/src/main/java/com/gitblit/wicket/GitblitWicketApp.java b/src/main/java/com/gitblit/wicket/GitblitWicketApp.java index b3ca2073..3041c5da 100644 --- a/src/main/java/com/gitblit/wicket/GitblitWicketApp.java +++ b/src/main/java/com/gitblit/wicket/GitblitWicketApp.java @@ -75,6 +75,4 @@ public interface GitblitWicketApp { public abstract TimeZone getTimezone(); - public String buildAvatarUrl(String username, String emailaddress, String cssClass, int width, boolean identicon); - } \ No newline at end of file diff --git a/src/main/java/com/gitblit/wicket/panels/AvatarImage.java b/src/main/java/com/gitblit/wicket/panels/AvatarImage.java index 99015612..a562042d 100644 --- a/src/main/java/com/gitblit/wicket/panels/AvatarImage.java +++ b/src/main/java/com/gitblit/wicket/panels/AvatarImage.java @@ -17,6 +17,7 @@ package com.gitblit.wicket.panels; import org.eclipse.jgit.lib.PersonIdent; +import com.gitblit.AvatarGenerator; import com.gitblit.Keys; import com.gitblit.models.UserModel; import com.gitblit.wicket.ExternalImage; @@ -51,7 +52,8 @@ public class AvatarImage extends BasePanel { public AvatarImage(String id, String username, String emailaddress, String cssClass, int width, boolean identicon) { super(id); - String url = app().buildAvatarUrl(username, emailaddress, cssClass, width, identicon); + AvatarGenerator avatarGenerator = app().runtime().getInjector().getInstance(AvatarGenerator.class); + String url = avatarGenerator.getURL(username, emailaddress, identicon, width); ExternalImage image = new ExternalImage("image", url); if (cssClass != null) { WicketUtils.setCssClass(image, cssClass); diff --git a/src/test/java/com/gitblit/tests/GitBlitSuite.java b/src/test/java/com/gitblit/tests/GitBlitSuite.java index 79a0920d..af20a487 100644 --- a/src/test/java/com/gitblit/tests/GitBlitSuite.java +++ b/src/test/java/com/gitblit/tests/GitBlitSuite.java @@ -65,7 +65,8 @@ import com.gitblit.utils.JGitUtils; FanoutServiceTest.class, Issue0259Test.class, Issue0271Test.class, HtpasswdAuthenticationTest.class, ModelUtilsTest.class, JnaUtilsTest.class, LdapSyncServiceTest.class, FileTicketServiceTest.class, BranchTicketServiceTest.class, RedisTicketServiceTest.class, AuthenticationManagerTest.class, - SshKeysDispatcherTest.class, UITicketTest.class, PathUtilsTest.class, SshKerberosAuthenticationTest.class }) + SshKeysDispatcherTest.class, UITicketTest.class, PathUtilsTest.class, SshKerberosAuthenticationTest.class, + GravatarTest.class }) public class GitBlitSuite { public static final File BASEFOLDER = new File("data"); diff --git a/src/test/java/com/gitblit/tests/GravatarTest.java b/src/test/java/com/gitblit/tests/GravatarTest.java index 1fea8432..ba989d56 100644 --- a/src/test/java/com/gitblit/tests/GravatarTest.java +++ b/src/test/java/com/gitblit/tests/GravatarTest.java @@ -1,77 +1,74 @@ package com.gitblit.tests; -import org.junit.Assert; import org.junit.Test; +import com.gitblit.AvatarGenerator; +import com.gitblit.GravatarGenerator; import com.gitblit.IStoredSettings; import com.gitblit.Keys; -import com.gitblit.manager.AuthenticationManager; -import com.gitblit.manager.FederationManager; -import com.gitblit.manager.GitblitManager; -import com.gitblit.manager.IAuthenticationManager; -import com.gitblit.manager.IFederationManager; -import com.gitblit.manager.IGitblit; -import com.gitblit.manager.INotificationManager; -import com.gitblit.manager.IPluginManager; -import com.gitblit.manager.IProjectManager; -import com.gitblit.manager.IRepositoryManager; +import com.gitblit.guice.AvatarGeneratorProvider; import com.gitblit.manager.IRuntimeManager; -import com.gitblit.manager.IServicesManager; -import com.gitblit.manager.IUserManager; -import com.gitblit.manager.NotificationManager; -import com.gitblit.manager.PluginManager; -import com.gitblit.manager.ProjectManager; -import com.gitblit.manager.RepositoryManager; import com.gitblit.manager.RuntimeManager; -import com.gitblit.manager.ServicesManager; -import com.gitblit.manager.UserManager; import com.gitblit.tests.mock.MemorySettings; -import com.gitblit.tickets.FileTicketService; -import com.gitblit.tickets.ITicketService; -import com.gitblit.transport.ssh.IPublicKeyManager; -import com.gitblit.transport.ssh.MemoryKeyManager; -import com.gitblit.utils.IdGenerator; -import com.gitblit.utils.WorkQueue; +import com.gitblit.utils.ActivityUtils; import com.gitblit.utils.XssFilter; import com.gitblit.utils.XssFilter.AllowXssFilter; -import com.gitblit.wicket.GitBlitWebApp; import com.google.inject.AbstractModule; import com.google.inject.Guice; import com.google.inject.Injector; public class GravatarTest extends GitblitUnitTest { - public static class SomeModule extends AbstractModule { - private final IStoredSettings settings = new MemorySettings(); - SomeModule() { - settings.overrideSetting(Keys.web.avatarClass, "com.gitblit.GravatarGenerator"); + public static class AvatarModule extends AbstractModule { + private final IStoredSettings settings; + + AvatarModule(IStoredSettings settings) { + this.settings = settings; } - @Override + + @Override protected void configure() { - bind(IGitblit.class).to(GitblitManager.class); - bind(IAuthenticationManager.class).to(AuthenticationManager.class); - bind(IFederationManager.class).to(FederationManager.class); - bind(INotificationManager.class).to(NotificationManager.class); - bind(IPluginManager.class).to(PluginManager.class); - bind(IRepositoryManager.class).to(RepositoryManager.class); - bind(IProjectManager.class).to(ProjectManager.class); - bind(IRuntimeManager.class).to(RuntimeManager.class); - bind(IUserManager.class).to(UserManager.class); - bind(ITicketService.class).to(FileTicketService.class); - bind(XssFilter.class).to(AllowXssFilter.class); bind(IStoredSettings.class).toInstance(settings); - bind(IPublicKeyManager.class).to(MemoryKeyManager.class); - bind(IServicesManager.class).to(ServicesManager.class); - bind(WorkQueue.class).toInstance(new WorkQueue(new IdGenerator(), 0)); + bind(XssFilter.class).to(AllowXssFilter.class); + bind(IRuntimeManager.class).to(RuntimeManager.class); + bind(AvatarGenerator.class).toProvider(AvatarGeneratorProvider.class); } } @Test - public void oneTest() { - Injector injector = Guice.createInjector(new SomeModule()); - GitBlitWebApp webapp = injector.getInstance(GitBlitWebApp.class); - webapp.init(); - Assert.assertNotNull(webapp.buildAvatarUrl("username", "emailaddress", "cssClass", 10, true)); + public void gravatarIdenticonTest() { + IStoredSettings settings = new MemorySettings(); + settings.overrideSetting(Keys.web.avatarClass, GravatarGenerator.class.getName()); + + Injector injector = Guice.createInjector(new AvatarModule(settings)); + AvatarGenerator avatarGenerator = injector.getInstance(AvatarGenerator.class); + + String username = "username"; + String emailAddress = "emailAddress"; + int width = 10; + + String url = avatarGenerator.getURL(username, emailAddress, true, width); + assertNotNull(url); + + assertEquals(ActivityUtils.getGravatarIdenticonUrl(emailAddress, width), url); + } + + @Test + public void gravatarThumbnailTest() { + IStoredSettings settings = new MemorySettings(); + settings.overrideSetting(Keys.web.avatarClass, GravatarGenerator.class.getName()); + + Injector injector = Guice.createInjector(new AvatarModule(settings)); + AvatarGenerator avatarGenerator = injector.getInstance(AvatarGenerator.class); + + String username = "username"; + String emailAddress = "emailAddress"; + int width = 10; + + String url = avatarGenerator.getURL(username, emailAddress, false, width); + assertNotNull(url); + + assertEquals(ActivityUtils.getGravatarThumbnailUrl(emailAddress, width), url); } - + } -- 2.39.5