opendev/gerrit
Code Issues Proposed changes

Only bind index {start/activate} if needed

Browse Source 
These commands are only interacting with the AbstractVersionManager, if
it is not available then they have no reason to be available.

Because those commands were always binded, binding an implementation of
AbstractVersionManager was mandatory otherwise Guice would have
complained. For that reason, the implementation of
AbstractVersionManager was done even in the case when index is a single
version and an implementation of AbstractVersionManager is not required.
Only bind implementations of AbstractVersionManager when needed and bind
the commands only if AbstractVersionManager implementation is binded.

Change-Id: I3db48f6b5d030ecc6efa840c59eb4e177ae1b6b1
This commit is contained in:
Hugo Arès
2018-04-10 15:24:26 -04:00
parent 52c2b4afe0
commit 37d47e9c6e
5 changed files with 17 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticIndexModule.java
View File

@@ -64,11 +64,11 @@ public class ElasticIndexModule extends LifecycleModule {
install(new IndexModule(threads));
if (singleVersions == null) {
bind(AbstractVersionManager.class).to(ElasticVersionManager.class);
listener().to(ElasticVersionManager.class);
} else {
install(new SingleVersionModule(singleVersions));
}
bind(AbstractVersionManager.class).to(ElasticVersionManager.class);
}
@Provides

2
gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java
View File

@@ -78,7 +78,6 @@ public class LuceneIndexModule extends LifecycleModule {
} else {
install(new SingleVersionModule(singleVersions));
}
bind(AbstractVersionManager.class).to(LuceneVersionManager.class);
}
@Provides
@@ -92,6 +91,7 @@ public class LuceneIndexModule extends LifecycleModule {
private static class MultiVersionModule extends LifecycleModule {
@Override
public void configure() {
bind(AbstractVersionManager.class).to(LuceneVersionManager.class);
listener().to(LuceneVersionManager.class);
}
}

2
gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
View File

@@ -487,7 +487,7 @@ public class Daemon extends SiteProgram {
sysInjector.getInstance(DownloadConfig.class),
sysInjector.getInstance(LfsPluginAuthCommand.Module.class)));
if (!slave) {
modules.add(new IndexCommandsModule());
modules.add(new IndexCommandsModule(sysInjector));
}
return sysInjector.createChildInjector(modules);
}

15
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexCommandsModule.java
View File

@@ -14,20 +14,31 @@
package com.google.gerrit.sshd.commands;
import com.google.gerrit.server.index.AbstractVersionManager;
import com.google.gerrit.sshd.CommandModule;
import com.google.gerrit.sshd.CommandName;
import com.google.gerrit.sshd.Commands;
import com.google.gerrit.sshd.DispatchCommandProvider;
import com.google.inject.Injector;
import com.google.inject.Key;
public class IndexCommandsModule extends CommandModule {
private final Injector injector;
public IndexCommandsModule(Injector injector) {
this.injector = injector;
}
@Override
protected void configure() {
CommandName gerrit = Commands.named("gerrit");
CommandName index = Commands.named(gerrit, "index");
command(index).toProvider(new DispatchCommandProvider(index));
command(index, IndexActivateCommand.class);
command(index, IndexStartCommand.class);
if (injector.getExistingBinding(Key.get(AbstractVersionManager.class)) != null) {
command(index, IndexActivateCommand.class);
command(index, IndexStartCommand.class);
}
command(index, IndexChangesCommand.class);
command(index, IndexProjectCommand.class);
}

2
gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
View File

@@ -391,7 +391,7 @@ public class WebAppInitializer extends GuiceServletContextListener implements Fi
false,
sysInjector.getInstance(DownloadConfig.class),
sysInjector.getInstance(LfsPluginAuthCommand.Module.class)));
modules.add(new IndexCommandsModule());
modules.add(new IndexCommandsModule(sysInjector));
return sysInjector.createChildInjector(modules);
}