From c42032df0911f51c81a91a961eff2066b380607c Mon Sep 17 00:00:00 2001
From: James Moger <james.moger@gitblit.com>
Date: Thu, 03 Jul 2014 17:00:41 -0400
Subject: [PATCH] Extract ticket service into an injectable object with a custom provider
---
src/main/java/com/gitblit/tickets/FileTicketService.java | 6
src/main/java/com/gitblit/servlet/GitblitContext.java | 2
src/main/java/com/gitblit/guice/ITicketServiceProvider.java | 73 ++++++++++++++
src/main/java/com/gitblit/FederationClient.java | 2
src/main/java/com/gitblit/manager/GitblitManager.java | 36 +++++-
src/main/java/com/gitblit/tickets/ITicketService.java | 5
src/main/java/com/gitblit/wicket/GitBlitWebApp.java | 6 +
src/main/java/com/gitblit/tickets/BranchTicketService.java | 6
src/main/java/com/gitblit/GitBlit.java | 140 +---------------------------
src/main/java/com/gitblit/tickets/RedisTicketService.java | 9 +
src/main/java/com/gitblit/tickets/NullTicketService.java | 6
src/main/java/com/gitblit/guice/CoreModule.java | 2
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 af92b5f..822e8a7 100644
--- a/src/main/java/com/gitblit/FederationClient.java
+++ b/src/main/java/com/gitblit/FederationClient.java
@@ -97,7 +97,7 @@
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 9a01d3c..68a91bb 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 @@
super(
publicKeyManagerProvider,
+ ticketServiceProvider,
runtimeManager,
pluginManager,
notificationManager,
@@ -83,117 +65,5 @@
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 6dbe615..c0d39e9 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.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 @@
// 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 0000000..fd39955
--- /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 fbbafac..02b2d67 100644
--- a/src/main/java/com/gitblit/manager/GitblitManager.java
+++ b/src/main/java/com/gitblit/manager/GitblitManager.java
@@ -109,6 +109,8 @@
protected final Provider<IPublicKeyManager> publicKeyManagerProvider;
+ protected final Provider<ITicketService> ticketServiceProvider;
+
protected final IStoredSettings settings;
protected final IRuntimeManager runtimeManager;
@@ -130,6 +132,7 @@
@Inject
public GitblitManager(
Provider<IPublicKeyManager> publicKeyManagerProvider,
+ Provider<ITicketService> ticketServiceProvider,
IRuntimeManager runtimeManager,
IPluginManager pluginManager,
INotificationManager notificationManager,
@@ -140,6 +143,7 @@
IFederationManager federationManager) {
this.publicKeyManagerProvider = publicKeyManagerProvider;
+ this.ticketServiceProvider = ticketServiceProvider;
this.settings = runtimeManager.getSettings();
this.runtimeManager = runtimeManager;
@@ -478,13 +482,9 @@
}
}
- /**
- * 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 @@
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 @@
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 44a857c..115af65 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.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 @@
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 0633751..8a2bc95 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.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.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 @@
@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 74311f5..c8346d1 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.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.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 @@
@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 668d0bc..7e7ea9e 100644
--- a/src/main/java/com/gitblit/tickets/ITicketService.java
+++ b/src/main/java/com/gitblit/tickets/ITicketService.java
@@ -36,6 +36,7 @@
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 @@
* @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 @@
* 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 0980d29..3947b94 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.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.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 @@
@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 02c19e6..0f9ad17 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.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.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 @@
@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 e09799d..036a05a 100644
--- a/src/main/java/com/gitblit/wicket/GitBlitWebApp.java
+++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp.java
@@ -105,6 +105,8 @@
private final Provider<IPublicKeyManager> publicKeyManagerProvider;
+ private final Provider<ITicketService> ticketServiceProvider;
+
private final IStoredSettings settings;
private final IRuntimeManager runtimeManager;
@@ -130,6 +132,7 @@
@Inject
public GitBlitWebApp(
Provider<IPublicKeyManager> publicKeyManagerProvider,
+ Provider<ITicketService> ticketServiceProvider,
IRuntimeManager runtimeManager,
IPluginManager pluginManager,
INotificationManager notificationManager,
@@ -143,6 +146,7 @@
super();
this.publicKeyManagerProvider = publicKeyManagerProvider;
+ this.ticketServiceProvider = ticketServiceProvider;
this.settings = runtimeManager.getSettings();
this.runtimeManager = runtimeManager;
this.pluginManager = pluginManager;
@@ -438,7 +442,7 @@
*/
@Override
public ITicketService tickets() {
- return gitblit.getTicketService();
+ return ticketServiceProvider.get();
}
/* (non-Javadoc)
--
Gitblit v1.9.1