Extract ticket service into an injectable object with a custom provider

12 files changed, 136 insertions, 157 deletions

diff --git a/src/main/java/com/gitblit/FederationClient.java b/src/main/java/com/gitblit/FederationClient.java
index af92b5f9..822e8a7f 100644
--- a/src/main/java/com/gitblit/FederationClient.java
+++ b/src/main/java/com/gitblit/FederationClient.java
@@ -97,7 +97,7 @@ public class FederationClient {
 		UserManager users = new UserManager(runtime, null).start();
 		RepositoryManager repositories = new RepositoryManager(runtime, null, users).start();
 		FederationManager federation = new FederationManager(runtime, notifications, repositories).start();
-		IGitblit gitblit = new GitblitManager(null, runtime, null, notifications, users, null, repositories, null, federation);
+		IGitblit gitblit = new GitblitManager(null, null, runtime, null, notifications, users, null, repositories, null, federation);
 
 		FederationPullService puller = new FederationPullService(gitblit, federation.getFederationRegistrations()) {
 			@Override
diff --git a/src/main/java/com/gitblit/GitBlit.java b/src/main/java/com/gitblit/GitBlit.java
index 9a01d3ce..68a91bb5 100644
--- a/src/main/java/com/gitblit/GitBlit.java
+++ b/src/main/java/com/gitblit/GitBlit.java
@@ -15,55 +15,36 @@
  */
 package com.gitblit;
 
-import java.text.MessageFormat;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Set;
-
 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.manager.IRuntimeManager;
 import com.gitblit.manager.IUserManager;
-import com.gitblit.models.RepositoryModel;
-import com.gitblit.models.UserModel;
 import com.gitblit.tickets.ITicketService;
-import com.gitblit.tickets.NullTicketService;
 import com.gitblit.transport.ssh.IPublicKeyManager;
-import com.gitblit.utils.StringUtils;
-import com.google.inject.AbstractModule;
-import com.google.inject.Guice;
 import com.google.inject.Inject;
-import com.google.inject.Injector;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
 /**
- * GitBlit is the aggregate manager for the Gitblit webapp.  It provides all
- * management functions and also manages some long-running services.
+ * GitBlit is the aggregate manager for the Gitblit webapp.  The parent class provides all
+ * functionality.  This class exists to not break existing Groovy push hooks.
  *
  * @author James Moger
  *
  */
 @Singleton
+@Deprecated
 public class GitBlit extends GitblitManager {
 
-	private final Injector injector;
-
-	private ITicketService ticketService;
-
 	@Inject
 	public GitBlit(
 			Provider<IPublicKeyManager> publicKeyManagerProvider,
+			Provider<ITicketService> ticketServiceProvider,
 			IRuntimeManager runtimeManager,
 			IPluginManager pluginManager,
 			INotificationManager notificationManager,
@@ -75,6 +56,7 @@ public class GitBlit extends GitblitManager {
 
 		super(
 				publicKeyManagerProvider,
+				ticketServiceProvider,
 				runtimeManager,
 				pluginManager,
 				notificationManager,
@@ -83,117 +65,5 @@ public class GitBlit extends GitblitManager {
 				repositoryManager,
 				projectManager,
 				federationManager);
-
-		this.injector = Guice.createInjector(getModules());
-	}
-
-	@Override
-	public GitBlit start() {
-		super.start();
-		configureTicketService();
-		return this;
-	}
-
-	@Override
-	public GitBlit stop() {
-		super.stop();
-		ticketService.stop();
-		return this;
-	}
-
-	protected AbstractModule [] getModules() {
-		return new AbstractModule [] { new GitBlitModule()};
-	}
-
-	/**
-	 * Detect renames and reindex as appropriate.
-	 */
-	@Override
-	public void updateRepositoryModel(String repositoryName, RepositoryModel repository,
-			boolean isCreate) throws GitBlitException {
-		RepositoryModel oldModel = null;
-		boolean isRename = !isCreate && !repositoryName.equalsIgnoreCase(repository.name);
-		if (isRename) {
-			oldModel = repositoryManager.getRepositoryModel(repositoryName);
-		}
-
-		super.updateRepositoryModel(repositoryName, repository, isCreate);
-
-		if (isRename && ticketService != null) {
-			ticketService.rename(oldModel, repository);
-		}
-	}
-
-	/**
-	 * Delete the repository and all associated tickets.
-	 */
-	@Override
-	public boolean deleteRepository(String repositoryName) {
-		RepositoryModel repository = repositoryManager.getRepositoryModel(repositoryName);
-		return deleteRepositoryModel(repository);
-	}
-
-	@Override
-	public boolean deleteRepositoryModel(RepositoryModel model) {
-		boolean success = repositoryManager.deleteRepositoryModel(model);
-		if (success && ticketService != null) {
-			ticketService.deleteAll(model);
-		}
-		return success;
-	}
-
-	/**
-	 * Returns the configured ticket service.
-	 *
-	 * @return a ticket service
-	 */
-	@Override
-	public ITicketService getTicketService() {
-		return ticketService;
-	}
-
-	protected void configureTicketService() {
-		String clazz = settings.getString(Keys.tickets.service, NullTicketService.class.getName());
-		if (StringUtils.isEmpty(clazz)) {
-			clazz = NullTicketService.class.getName();
-		}
-		try {
-			Class<? extends ITicketService> serviceClass = (Class<? extends ITicketService>) Class.forName(clazz);
-			ticketService = injector.getInstance(serviceClass).start();
-			if (ticketService instanceof NullTicketService) {
-				logger.warn("No ticket service configured.");
-			} else if (ticketService.isReady()) {
-				logger.info("{} is ready.", ticketService);
-			} else {
-				logger.warn("{} is disabled.", ticketService);
-			}
-		} catch (Exception e) {
-			logger.error("failed to create ticket service " + clazz, e);
-			ticketService = injector.getInstance(NullTicketService.class).start();
-		}
-	}
-
-	/**
-	 * A nested Guice Module is used for constructor dependency injection of
-	 * complex classes.
-	 *
-	 * @author James Moger
-	 *
-	 */
-	class GitBlitModule extends AbstractModule {
-
-		@Override
-		protected void configure() {
-			bind(IStoredSettings.class).toInstance(settings);
-			bind(IRuntimeManager.class).toInstance(runtimeManager);
-			bind(IPluginManager.class).toInstance(pluginManager);
-			bind(INotificationManager.class).toInstance(notificationManager);
-			bind(IUserManager.class).toInstance(userManager);
-			bind(IAuthenticationManager.class).toInstance(authenticationManager);
-			bind(IRepositoryManager.class).toInstance(repositoryManager);
-			bind(IProjectManager.class).toInstance(projectManager);
-			bind(IFederationManager.class).toInstance(federationManager);
-			bind(IGitblit.class).toInstance(GitBlit.this);
-		}
 	}
 }
diff --git a/src/main/java/com/gitblit/guice/CoreModule.java b/src/main/java/com/gitblit/guice/CoreModule.java
index 6dbe6150..c0d39e99 100644
--- a/src/main/java/com/gitblit/guice/CoreModule.java
+++ b/src/main/java/com/gitblit/guice/CoreModule.java
@@ -37,6 +37,7 @@ import com.gitblit.manager.RepositoryManager;
 import com.gitblit.manager.RuntimeManager;
 import com.gitblit.manager.ServicesManager;
 import com.gitblit.manager.UserManager;
+import com.gitblit.tickets.ITicketService;
 import com.gitblit.transport.ssh.IPublicKeyManager;
 import com.gitblit.utils.WorkQueue;
 import com.google.inject.AbstractModule;
@@ -56,6 +57,7 @@ public class CoreModule extends AbstractModule {
 
 		// bind complex providers
 		bind(IPublicKeyManager.class).toProvider(IPublicKeyManagerProvider.class);
+		bind(ITicketService.class).toProvider(ITicketServiceProvider.class);
 		bind(WorkQueue.class).toProvider(WorkQueueProvider.class);
 
 		// core managers
diff --git a/src/main/java/com/gitblit/guice/ITicketServiceProvider.java b/src/main/java/com/gitblit/guice/ITicketServiceProvider.java
new file mode 100644
index 00000000..fd39955d
--- /dev/null
+++ b/src/main/java/com/gitblit/guice/ITicketServiceProvider.java
@@ -0,0 +1,73 @@
+/*
+ * Copyright 2014 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.IStoredSettings;
+import com.gitblit.Keys;
+import com.gitblit.manager.IRuntimeManager;
+import com.gitblit.tickets.ITicketService;
+import com.gitblit.tickets.NullTicketService;
+import com.gitblit.utils.StringUtils;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
+/**
+ * Provides a lazily-instantiated ITicketService configured from IStoredSettings.
+ *
+ * @author James Moger
+ *
+ */
+@Singleton
+public class ITicketServiceProvider implements Provider<ITicketService> {
+
+	private final Logger logger = LoggerFactory.getLogger(getClass());
+
+	private final IRuntimeManager runtimeManager;
+
+	private volatile ITicketService service;
+
+	@Inject
+	public ITicketServiceProvider(IRuntimeManager runtimeManager) {
+		this.runtimeManager = runtimeManager;
+	}
+
+	@Override
+	public synchronized ITicketService get() {
+		if (service != null) {
+			return service;
+		}
+
+		IStoredSettings settings = runtimeManager.getSettings();
+		String clazz = settings.getString(Keys.tickets.service, NullTicketService.class.getName());
+		if (StringUtils.isEmpty(clazz)) {
+			clazz = NullTicketService.class.getName();
+		}
+
+		try {
+			Class<? extends ITicketService> serviceClass = (Class<? extends ITicketService>) Class.forName(clazz);
+			service = runtimeManager.getInjector().getInstance(serviceClass);
+		} catch (Exception e) {
+			logger.error("failed to create ticket service", e);
+			service = runtimeManager.getInjector().getInstance(NullTicketService.class);
+		}
+
+		return service;
+	}
+}
\ No newline at end of file
diff --git a/src/main/java/com/gitblit/manager/GitblitManager.java b/src/main/java/com/gitblit/manager/GitblitManager.java
index fbbafacb..02b2d675 100644
--- a/src/main/java/com/gitblit/manager/GitblitManager.java
+++ b/src/main/java/com/gitblit/manager/GitblitManager.java
@@ -109,6 +109,8 @@ public class GitblitManager implements IGitblit {
 
 	protected final Provider<IPublicKeyManager> publicKeyManagerProvider;
 
+	protected final Provider<ITicketService> ticketServiceProvider;
+
 	protected final IStoredSettings settings;
 
 	protected final IRuntimeManager runtimeManager;
@@ -130,6 +132,7 @@ public class GitblitManager implements IGitblit {
 	@Inject
 	public GitblitManager(
 			Provider<IPublicKeyManager> publicKeyManagerProvider,
+			Provider<ITicketService> ticketServiceProvider,
 			IRuntimeManager runtimeManager,
 			IPluginManager pluginManager,
 			INotificationManager notificationManager,
@@ -140,6 +143,7 @@ public class GitblitManager implements IGitblit {
 			IFederationManager federationManager) {
 
 		this.publicKeyManagerProvider = publicKeyManagerProvider;
+		this.ticketServiceProvider = ticketServiceProvider;
 
 		this.settings = runtimeManager.getSettings();
 		this.runtimeManager = runtimeManager;
@@ -478,13 +482,9 @@ public class GitblitManager implements IGitblit {
 		}
 	}
 
-	/**
-	 * Throws an exception if trying to get a ticket service.
-	 *
-	 */
 	@Override
 	public ITicketService getTicketService() {
-		throw new RuntimeException("This class does not have a ticket service!");
+		return ticketServiceProvider.get();
 	}
 
 	@Override
@@ -960,10 +960,23 @@ public class GitblitManager implements IGitblit {
 		return repositoryManager.getRepositoryDefaultMetrics(model, repository);
 	}
 
+	/**
+	 * Detect renames and reindex as appropriate.
+	 */
 	@Override
 	public void updateRepositoryModel(String repositoryName, RepositoryModel repository,
 			boolean isCreate) throws GitBlitException {
+		RepositoryModel oldModel = null;
+		boolean isRename = !isCreate && !repositoryName.equalsIgnoreCase(repository.name);
+		if (isRename) {
+			oldModel = repositoryManager.getRepositoryModel(repositoryName);
+		}
+
 		repositoryManager.updateRepositoryModel(repositoryName, repository, isCreate);
+
+		if (isRename && ticketServiceProvider.get() != null) {
+			ticketServiceProvider.get().rename(oldModel, repository);
+		}
 	}
 
 	@Override
@@ -976,14 +989,23 @@ public class GitblitManager implements IGitblit {
 		return repositoryManager.canDelete(model);
 	}
 
+	/**
+	 * Delete the repository and all associated tickets.
+	 */
 	@Override
 	public boolean deleteRepositoryModel(RepositoryModel model) {
-		return repositoryManager.deleteRepositoryModel(model);
+		boolean success = repositoryManager.deleteRepositoryModel(model);
+		if (success && ticketServiceProvider.get() != null) {
+			ticketServiceProvider.get().deleteAll(model);
+		}
+		return success;
 	}
 
 	@Override
 	public boolean deleteRepository(String repositoryName) {
-		return repositoryManager.deleteRepository(repositoryName);
+		// delegate to deleteRepositoryModel() to destroy indexed tickets
+		RepositoryModel repository = repositoryManager.getRepositoryModel(repositoryName);
+		return deleteRepositoryModel(repository);
 	}
 
 	@Override
diff --git a/src/main/java/com/gitblit/servlet/GitblitContext.java b/src/main/java/com/gitblit/servlet/GitblitContext.java
index 44a857ca..115af656 100644
--- a/src/main/java/com/gitblit/servlet/GitblitContext.java
+++ b/src/main/java/com/gitblit/servlet/GitblitContext.java
@@ -53,6 +53,7 @@ import com.gitblit.manager.IRepositoryManager;
 import com.gitblit.manager.IRuntimeManager;
 import com.gitblit.manager.IServicesManager;
 import com.gitblit.manager.IUserManager;
+import com.gitblit.tickets.ITicketService;
 import com.gitblit.transport.ssh.IPublicKeyManager;
 import com.gitblit.utils.ContainerUtils;
 import com.gitblit.utils.StringUtils;
@@ -200,6 +201,7 @@ public class GitblitContext extends GuiceServletContextListener {
 		startManager(injector, IRepositoryManager.class);
 		startManager(injector, IProjectManager.class);
 		startManager(injector, IFederationManager.class);
+		startManager(injector, ITicketService.class);
 		startManager(injector, IGitblit.class);
 		startManager(injector, IServicesManager.class);
 
diff --git a/src/main/java/com/gitblit/tickets/BranchTicketService.java b/src/main/java/com/gitblit/tickets/BranchTicketService.java
index 0633751a..8a2bc951 100644
--- a/src/main/java/com/gitblit/tickets/BranchTicketService.java
+++ b/src/main/java/com/gitblit/tickets/BranchTicketService.java
@@ -30,9 +30,6 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
 
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-
 import org.eclipse.jgit.api.errors.ConcurrentRefUpdateException;
 import org.eclipse.jgit.api.errors.JGitInternalException;
 import org.eclipse.jgit.dircache.DirCache;
@@ -75,6 +72,8 @@ import com.gitblit.models.TicketModel.Change;
 import com.gitblit.utils.ArrayUtils;
 import com.gitblit.utils.JGitUtils;
 import com.gitblit.utils.StringUtils;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
 
 /**
  * Implementation of a ticket service based on an orphan branch.  All tickets
@@ -117,6 +116,7 @@ public class BranchTicketService extends ITicketService implements RefsChangedLi
 
 	@Override
 	public BranchTicketService start() {
+		log.info("{} started", getClass().getSimpleName());
 		return this;
 	}
 
diff --git a/src/main/java/com/gitblit/tickets/FileTicketService.java b/src/main/java/com/gitblit/tickets/FileTicketService.java
index 74311f53..c8346d14 100644
--- a/src/main/java/com/gitblit/tickets/FileTicketService.java
+++ b/src/main/java/com/gitblit/tickets/FileTicketService.java
@@ -27,9 +27,6 @@ import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicLong;
 
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-
 import org.eclipse.jgit.lib.Repository;
 
 import com.gitblit.Constants;
@@ -45,6 +42,8 @@ import com.gitblit.models.TicketModel.Change;
 import com.gitblit.utils.ArrayUtils;
 import com.gitblit.utils.FileUtils;
 import com.gitblit.utils.StringUtils;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
 
 /**
  * Implementation of a ticket service based on a directory within the repository.
@@ -82,6 +81,7 @@ public class FileTicketService extends ITicketService {
 
 	@Override
 	public FileTicketService start() {
+		log.info("{} started", getClass().getSimpleName());
 		return this;
 	}
 
diff --git a/src/main/java/com/gitblit/tickets/ITicketService.java b/src/main/java/com/gitblit/tickets/ITicketService.java
index 668d0bcc..7e7ea9e8 100644
--- a/src/main/java/com/gitblit/tickets/ITicketService.java
+++ b/src/main/java/com/gitblit/tickets/ITicketService.java
@@ -36,6 +36,7 @@ import org.slf4j.LoggerFactory;
 import com.gitblit.IStoredSettings;
 import com.gitblit.Keys;
 import com.gitblit.extensions.TicketHook;
+import com.gitblit.manager.IManager;
 import com.gitblit.manager.INotificationManager;
 import com.gitblit.manager.IPluginManager;
 import com.gitblit.manager.IRepositoryManager;
@@ -63,7 +64,7 @@ import com.google.common.cache.CacheBuilder;
  * @author James Moger
  *
  */
-public abstract class ITicketService {
+public abstract class ITicketService implements IManager {
 
 	public static final String SETTING_UPDATE_DIFFSTATS = "migration.updateDiffstats";
 
@@ -176,12 +177,14 @@ public abstract class ITicketService {
 	 * Start the service.
 	 * @since 1.4.0
 	 */
+	@Override
 	public abstract ITicketService start();
 
 	/**
 	 * Stop the service.
 	 * @since 1.4.0
 	 */
+	@Override
 	public final ITicketService stop() {
 		indexer.close();
 		ticketsCache.invalidateAll();
diff --git a/src/main/java/com/gitblit/tickets/NullTicketService.java b/src/main/java/com/gitblit/tickets/NullTicketService.java
index 0980d29d..3947b945 100644
--- a/src/main/java/com/gitblit/tickets/NullTicketService.java
+++ b/src/main/java/com/gitblit/tickets/NullTicketService.java
@@ -19,9 +19,6 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-
 import com.gitblit.manager.INotificationManager;
 import com.gitblit.manager.IPluginManager;
 import com.gitblit.manager.IRepositoryManager;
@@ -31,6 +28,8 @@ import com.gitblit.models.RepositoryModel;
 import com.gitblit.models.TicketModel;
 import com.gitblit.models.TicketModel.Attachment;
 import com.gitblit.models.TicketModel.Change;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
 
 /**
  * Implementation of a ticket service that rejects everything.
@@ -63,6 +62,7 @@ public class NullTicketService extends ITicketService {
 
 	@Override
 	public NullTicketService start() {
+		log.info("{} started", getClass().getSimpleName());
 		return this;
 	}
 
diff --git a/src/main/java/com/gitblit/tickets/RedisTicketService.java b/src/main/java/com/gitblit/tickets/RedisTicketService.java
index 02c19e63..0f9ad174 100644
--- a/src/main/java/com/gitblit/tickets/RedisTicketService.java
+++ b/src/main/java/com/gitblit/tickets/RedisTicketService.java
@@ -22,9 +22,6 @@ import java.util.List;
 import java.util.Set;
 import java.util.TreeSet;
 
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-
 import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
 
 import redis.clients.jedis.Client;
@@ -46,6 +43,8 @@ import com.gitblit.models.TicketModel.Attachment;
 import com.gitblit.models.TicketModel.Change;
 import com.gitblit.utils.ArrayUtils;
 import com.gitblit.utils.StringUtils;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
 
 /**
  * Implementation of a ticket service based on a Redis key-value store.  All
@@ -85,6 +84,10 @@ public class RedisTicketService extends ITicketService {
 
 	@Override
 	public RedisTicketService start() {
+		log.info("{} started", getClass().getSimpleName());
+		if (!isReady()) {
+			log.warn("{} is not ready!", getClass().getSimpleName());
+		}
 		return this;
 	}
 
diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp.java b/src/main/java/com/gitblit/wicket/GitBlitWebApp.java
index e09799d1..036a05a5 100644
--- a/src/main/java/com/gitblit/wicket/GitBlitWebApp.java
+++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp.java
@@ -105,6 +105,8 @@ public class GitBlitWebApp extends WebApplication implements GitblitWicketApp {
 
 	private final Provider<IPublicKeyManager> publicKeyManagerProvider;
 
+	private final Provider<ITicketService> ticketServiceProvider;
+
 	private final IStoredSettings settings;
 
 	private final IRuntimeManager runtimeManager;
@@ -130,6 +132,7 @@ public class GitBlitWebApp extends WebApplication implements GitblitWicketApp {
 	@Inject
 	public GitBlitWebApp(
 			Provider<IPublicKeyManager> publicKeyManagerProvider,
+			Provider<ITicketService> ticketServiceProvider,
 			IRuntimeManager runtimeManager,
 			IPluginManager pluginManager,
 			INotificationManager notificationManager,
@@ -143,6 +146,7 @@ public class GitBlitWebApp extends WebApplication implements GitblitWicketApp {
 
 		super();
 		this.publicKeyManagerProvider = publicKeyManagerProvider;
+		this.ticketServiceProvider = ticketServiceProvider;
 		this.settings = runtimeManager.getSettings();
 		this.runtimeManager = runtimeManager;
 		this.pluginManager = pluginManager;
@@ -438,7 +442,7 @@ public class GitBlitWebApp extends WebApplication implements GitblitWicketApp {
 	 */
 	@Override
 	public ITicketService tickets() {
-		return gitblit.getTicketService();
+		return ticketServiceProvider.get();
 	}
 
 	/* (non-Javadoc)