Merge branch 'stable-3.9' into stable-3.10
* stable-3.9:
Revert "Ensure plugin modules are bound in the baseInjector"
Set version to 3.9.4-SNAPSHOT
Set version to 3.9.3
Fix endless loop when using "is:watched" in project watches
ReviewCommand: When available use project when identifying a change
Release-Notes: skip
Change-Id: I230b6113eb19a11b6f9abc68f9f8bdbeb7857762
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 7027eaf..96a6d32 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -1720,15 +1720,14 @@
protected AutoCloseable installPlugin(String pluginName, Class<? extends Module> sysModuleClass)
throws Exception {
- return installPlugin(pluginName, sysModuleClass, null, null, null);
+ return installPlugin(pluginName, sysModuleClass, null, null);
}
protected AutoCloseable installPlugin(
String pluginName,
@Nullable Class<? extends Module> sysModuleClass,
@Nullable Class<? extends Module> httpModuleClass,
- @Nullable Class<? extends Module> sshModuleClass,
- @Nullable Class<? extends Module> apiModuleClass)
+ @Nullable Class<? extends Module> sshModuleClass)
throws Exception {
checkStatic(sysModuleClass);
checkStatic(httpModuleClass);
@@ -1742,7 +1741,6 @@
sysModuleClass != null ? sysModuleClass.getName() : null,
httpModuleClass != null ? httpModuleClass.getName() : null,
sshModuleClass != null ? sshModuleClass.getName() : null,
- apiModuleClass != null ? apiModuleClass.getName() : null,
sitePaths.data_dir.resolve(pluginName));
plugin.start(pluginGuiceEnvironment);
pluginGuiceEnvironment.onStartPlugin(plugin);
diff --git a/java/com/google/gerrit/acceptance/LightweightPluginDaemonTest.java b/java/com/google/gerrit/acceptance/LightweightPluginDaemonTest.java
index e1bd818..7e50b83 100644
--- a/java/com/google/gerrit/acceptance/LightweightPluginDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/LightweightPluginDaemonTest.java
@@ -45,7 +45,6 @@
testPlugin.sysModule(),
testPlugin.httpModule(),
testPlugin.sshModule(),
- testPlugin.apiModule(),
tempDataDir.getRoot().toPath());
plugin.start(env);
diff --git a/java/com/google/gerrit/acceptance/TestPlugin.java b/java/com/google/gerrit/acceptance/TestPlugin.java
index ba71c11..cafc775 100644
--- a/java/com/google/gerrit/acceptance/TestPlugin.java
+++ b/java/com/google/gerrit/acceptance/TestPlugin.java
@@ -30,6 +30,4 @@
String httpModule() default "";
String sshModule() default "";
-
- String apiModule() default "";
}
diff --git a/java/com/google/gerrit/server/plugins/ServerPlugin.java b/java/com/google/gerrit/server/plugins/ServerPlugin.java
index f1804b3..036285e 100644
--- a/java/com/google/gerrit/server/plugins/ServerPlugin.java
+++ b/java/com/google/gerrit/server/plugins/ServerPlugin.java
@@ -51,7 +51,7 @@
protected Class<? extends Module> batchModule;
protected Class<? extends Module> sshModule;
protected Class<? extends Module> httpModule;
- protected Class<? extends Module> apiModuleClass;
+ private Class<? extends Module> apiModuleClass;
private Injector apiInjector;
private Injector sysInjector;
@@ -286,7 +286,7 @@
modules.add(new ServerPluginInfoModule(this, env.getServerMetrics()));
return apiInjector
.map(injector -> injector.createChildInjector(modules))
- .orElseGet(() -> Guice.createInjector(modules));
+ .orElse(Guice.createInjector(modules));
}
private Injector newRootInjectorWithApiModule(
diff --git a/java/com/google/gerrit/server/plugins/ServerPluginInfoModule.java b/java/com/google/gerrit/server/plugins/ServerPluginInfoModule.java
index e11be1b..8386b4c 100644
--- a/java/com/google/gerrit/server/plugins/ServerPluginInfoModule.java
+++ b/java/com/google/gerrit/server/plugins/ServerPluginInfoModule.java
@@ -22,7 +22,6 @@
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.PluginUser;
import com.google.inject.AbstractModule;
-import com.google.inject.Module;
import com.google.inject.Provides;
import com.google.inject.ProvisionException;
import java.io.File;
@@ -53,10 +52,6 @@
.annotatedWith(PluginCanonicalWebUrl.class)
.toInstance(plugin.getPluginCanonicalWebUrl());
bind(Plugin.class).toInstance(plugin);
- bindNonNull(plugin.batchModule);
- bindNonNull(plugin.sysModule);
- bindNonNull(plugin.sshModule);
- bindNonNull(plugin.httpModule);
install(
new LifecycleModule() {
@@ -101,10 +96,4 @@
File getPluginDataAsFile(@PluginData Path pluginData) {
return pluginData.toFile();
}
-
- private void bindNonNull(Class<? extends Module> module) {
- if (module != null) {
- bind(module);
- }
- }
}
diff --git a/java/com/google/gerrit/server/plugins/TestServerPlugin.java b/java/com/google/gerrit/server/plugins/TestServerPlugin.java
index a58ee3b..cd5d5e3 100644
--- a/java/com/google/gerrit/server/plugins/TestServerPlugin.java
+++ b/java/com/google/gerrit/server/plugins/TestServerPlugin.java
@@ -23,7 +23,6 @@
private String sysName;
private String httpName;
private String sshName;
- private String apiName;
public TestServerPlugin(
String name,
@@ -33,7 +32,6 @@
String sysName,
String httpName,
String sshName,
- String apiName,
Path dataDir)
throws InvalidPluginException {
super(
@@ -51,7 +49,6 @@
this.sysName = sysName;
this.httpName = httpName;
this.sshName = sshName;
- this.apiName = apiName;
loadGuiceModules();
}
@@ -60,7 +57,6 @@
this.sysModule = load(sysName, classLoader);
this.httpModule = load(httpName, classLoader);
this.sshModule = load(sshName, classLoader);
- this.apiModuleClass = load(apiName, classLoader);
} catch (ClassNotFoundException e) {
throw new InvalidPluginException("Unable to load plugin Guice Modules", e);
}
diff --git a/java/com/google/gerrit/sshd/commands/ReviewCommand.java b/java/com/google/gerrit/sshd/commands/ReviewCommand.java
index b8c8406..95f771c 100644
--- a/java/com/google/gerrit/sshd/commands/ReviewCommand.java
+++ b/java/com/google/gerrit/sshd/commands/ReviewCommand.java
@@ -321,11 +321,11 @@
AbandonInput input = new AbandonInput();
input.message = Strings.emptyToNull(changeComment);
applyReview(patchSet, review);
- changeApi(patchSet).abandon(input);
+ getChangeApi(patchSet).abandon(input);
} else if (restoreChange) {
RestoreInput input = new RestoreInput();
input.message = Strings.emptyToNull(changeComment);
- changeApi(patchSet).restore(input);
+ getChangeApi(patchSet).restore(input);
applyReview(patchSet, review);
} else {
applyReview(patchSet, review);
@@ -335,15 +335,15 @@
MoveInput moveInput = new MoveInput();
moveInput.destinationBranch = moveToBranch;
moveInput.message = Strings.emptyToNull(changeComment);
- changeApi(patchSet).move(moveInput);
+ getChangeApi(patchSet).move(moveInput);
}
if (rebaseChange) {
- revisionApi(patchSet).rebase();
+ getRevisionApi(patchSet).rebase();
}
if (submitChange) {
- revisionApi(patchSet).submit();
+ getRevisionApi(patchSet).submit();
}
} catch (IllegalStateException | RestApiException e) {
@@ -351,12 +351,19 @@
}
}
- private ChangeApi changeApi(PatchSet patchSet) throws RestApiException {
- return gApi.changes().id(patchSet.id().changeId().get());
+ private ChangeApi getChangeApi(PatchSet patchSet) throws RestApiException {
+ if (projectState != null) {
+ return gApi.changes().id(projectState.getName(), patchSet.id().changeId().get());
+ }
+ /* Since we didn't get a project from the CLI we have to use the ambiguous
+ * Changes#id(String) that may fail to identify one single change and throw
+ * an exception.
+ */
+ return gApi.changes().id(String.valueOf(patchSet.id().changeId().get()));
}
- private RevisionApi revisionApi(PatchSet patchSet) throws RestApiException {
- return changeApi(patchSet).revision(patchSet.commitId().name());
+ private RevisionApi getRevisionApi(PatchSet patchSet) throws RestApiException {
+ return getChangeApi(patchSet).revision(patchSet.commitId().name());
}
@Override
diff --git a/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedChildRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedChildRestApiBindingsIT.java
index f211366..d2c1430 100644
--- a/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedChildRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedChildRestApiBindingsIT.java
@@ -169,7 +169,7 @@
@Test
public void testRevisionEndpoints() throws Exception {
PatchSet.Id patchSetId = createChange().getPatchSetId();
- try (AutoCloseable ignored = installPlugin(PLUGIN_NAME, MyPluginSysModule.class)) {
+ try (AutoCloseable ignored = installPlugin(PLUGIN_NAME, MyPluginSysModule.class, null, null)) {
RestApiCallHelper.execute(
adminRestSession,
REVISION_TEST_CALLS.asList(),
@@ -182,7 +182,7 @@
public void testRobotCommentEndpoints() throws Exception {
PatchSet.Id patchSetId = createChange().getPatchSetId();
String robotCommentUuid = createRobotComment(patchSetId.changeId());
- try (AutoCloseable ignored = installPlugin(PLUGIN_NAME, MyPluginSysModule.class)) {
+ try (AutoCloseable ignored = installPlugin(PLUGIN_NAME, MyPluginSysModule.class, null, null)) {
RestApiCallHelper.execute(
adminRestSession,
ROBOTCOMMENT_TEST_CALLS.asList(),
diff --git a/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedRootRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedRootRestApiBindingsIT.java
index e9047e0..24ce605 100644
--- a/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedRootRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedRootRestApiBindingsIT.java
@@ -208,16 +208,14 @@
@Test
public void testEndpoints() throws Exception {
- try (AutoCloseable ignored =
- installPlugin(PLUGIN_NAME, null, MyPluginHttpModule.class, null, null)) {
+ try (AutoCloseable ignored = installPlugin(PLUGIN_NAME, null, MyPluginHttpModule.class, null)) {
RestApiCallHelper.execute(adminRestSession, TEST_CALLS.asList());
}
}
@Test
public void testOptionOnSingletonIsIgnored() throws Exception {
- try (AutoCloseable ignored =
- installPlugin(PLUGIN_NAME, null, MyPluginHttpModule.class, null, null)) {
+ try (AutoCloseable ignored = installPlugin(PLUGIN_NAME, null, MyPluginHttpModule.class, null)) {
RestApiCallHelper.execute(
adminRestSession,
RestCall.get("/plugins/" + PLUGIN_NAME + "/test-collection/1/detail?crash=xyz"));
diff --git a/javatests/com/google/gerrit/acceptance/server/plugins/BUILD b/javatests/com/google/gerrit/acceptance/server/plugins/BUILD
deleted file mode 100644
index 25cc33f..0000000
--- a/javatests/com/google/gerrit/acceptance/server/plugins/BUILD
+++ /dev/null
@@ -1,7 +0,0 @@
-load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
-
-acceptance_tests(
- srcs = glob(["*IT.java"]),
- group = "server_plugins",
- labels = ["server"],
-)
diff --git a/javatests/com/google/gerrit/acceptance/server/plugins/PluginBindingsIT.java b/javatests/com/google/gerrit/acceptance/server/plugins/PluginBindingsIT.java
deleted file mode 100644
index d8def1a..0000000
--- a/javatests/com/google/gerrit/acceptance/server/plugins/PluginBindingsIT.java
+++ /dev/null
@@ -1,101 +0,0 @@
-// Copyright (C) 2024 The Android Open Source Project
-//
-// 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.google.gerrit.acceptance.server.plugins;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.extensions.annotations.PluginName;
-import com.google.gerrit.server.config.GerritIsReplica;
-import com.google.inject.AbstractModule;
-import com.google.inject.Inject;
-import com.google.inject.Injector;
-import com.google.inject.Key;
-import org.junit.Test;
-
-public class PluginBindingsIT extends AbstractDaemonTest {
- public static class TestPluginApiModule extends AbstractModule {}
-
- public static class TestPluginSysModule extends AbstractModule {}
-
- public static class PluginInjectsInjectorModule extends AbstractModule {
- private final Injector injector;
-
- @Inject
- PluginInjectsInjectorModule(Injector injector) {
- this.injector = injector;
- }
-
- @Override
- protected void configure() {
- Key<String> pluginNameKey = Key.get(String.class, PluginName.class);
- injector.getInstance(pluginNameKey);
- }
- }
-
- public static class PluginInjectsGerritReplicaModule extends AbstractModule {
- private final boolean isReplica;
-
- @Inject
- PluginInjectsGerritReplicaModule(@GerritIsReplica boolean isReplica) {
- this.isReplica = isReplica;
- }
-
- @Override
- protected void configure() {
- assertThat(isReplica).isFalse();
- }
- }
-
- @Test
- public void testCanInstallPluginInjectingInjector() throws Exception {
- try (AutoCloseable ignored =
- installPlugin("my-plugin-injecting-injector", PluginInjectsInjectorModule.class)) {
- // test passes so long as no exception is thrown
- }
- }
-
- @Test
- public void testCanInstallPluginInjectingInjectorAfterInstallingApiModule() throws Exception {
- try (AutoCloseable ignored =
- installPlugin(
- "my-api-plugin", TestPluginSysModule.class, null, null, TestPluginApiModule.class)) {
- try (AutoCloseable ignored2 =
- installPlugin("my-plugin-injecting-injector", PluginInjectsInjectorModule.class)) {
- // test passes so long as no exception is thrown
- }
- }
- }
-
- @Test
- public void testCanInstallPluginInjectingReplica() throws Exception {
- try (AutoCloseable ignored =
- installPlugin("my-plugin-injecting-replica", PluginInjectsGerritReplicaModule.class)) {
- // test passes so long as no exception is thrown
- }
- }
-
- @Test
- public void testCanInstallPluginInjectingReplicaAfterInstallingApiModule() throws Exception {
- try (AutoCloseable ignored =
- installPlugin(
- "my-api-plugin", TestPluginSysModule.class, null, null, TestPluginApiModule.class)) {
- try (AutoCloseable ignored2 =
- installPlugin("my-plugin-injecting-replica", PluginInjectsGerritReplicaModule.class)) {
- // test passes so long as no exception is thrown
- }
- }
- }
-}
