From c42032df0911f51c81a91a961eff2066b380607c Mon Sep 17 00:00:00 2001 From: James Moger Date: Wed, 2 Jul 2014 16:45:37 -0400 Subject: [PATCH] Extract ticket service into an injectable object with a custom provider --- .../java/com/gitblit/FederationClient.java | 2 +- src/main/java/com/gitblit/GitBlit.java | 140 +----------------- .../java/com/gitblit/guice/CoreModule.java | 2 + .../gitblit/guice/ITicketServiceProvider.java | 73 +++++++++ .../com/gitblit/manager/GitblitManager.java | 36 ++++- .../com/gitblit/servlet/GitblitContext.java | 2 + .../gitblit/tickets/BranchTicketService.java | 6 +- .../gitblit/tickets/FileTicketService.java | 6 +- .../com/gitblit/tickets/ITicketService.java | 5 +- .../gitblit/tickets/NullTicketService.java | 6 +- .../gitblit/tickets/RedisTicketService.java | 9 +- .../com/gitblit/wicket/GitBlitWebApp.java | 6 +- 12 files changed, 136 insertions(+), 157 deletions(-) create mode 100644 src/main/java/com/gitblit/guice/ITicketServiceProvider.java 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 publicKeyManagerProvider, + Provider 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 serviceClass = (Class) 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 { + + 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 serviceClass = (Class) 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 publicKeyManagerProvider; + protected final Provider ticketServiceProvider; + protected final IStoredSettings settings; protected final IRuntimeManager runtimeManager; @@ -130,6 +132,7 @@ public class GitblitManager implements IGitblit { @Inject public GitblitManager( Provider publicKeyManagerProvider, + Provider 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 publicKeyManagerProvider; + private final Provider ticketServiceProvider; + private final IStoredSettings settings; private final IRuntimeManager runtimeManager; @@ -130,6 +132,7 @@ public class GitBlitWebApp extends WebApplication implements GitblitWicketApp { @Inject public GitBlitWebApp( Provider publicKeyManagerProvider, + Provider 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) -- 2.39.5