Revised AvatarGenerator to be injectable

11 files changed, 142 insertions, 108 deletions

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<AvatarGenerator> {
+
+	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<? extends AvatarGenerator> generatorClass = (Class<? extends AvatarGenerator>) 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<AvatarGenerator> clazz;
-		try {
-			clazz = (Class<AvatarGenerator>) 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);
 	}
-	
+
 }