Revert "Ensure plugin modules are bound in the baseInjector"

This reverts commit 22145cb21b9346c2a60bf4e8dd97f4a44606de4e.

Reason for revert: Gitiles does load anymore after this change

Release-Notes: Fix Gitiles not loading because of injection issues
Change-Id: Iba4d33faacec7d24ad461f83521ee59a632f2085

10 files changed, 8 insertions, 138 deletions

diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 7491068090..b4b99350d1 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -1905,15 +1905,14 @@ public abstract class AbstractDaemonTest {
 
   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);
@@ -1927,7 +1926,6 @@ public abstract class AbstractDaemonTest {
             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 e1bd81839e..7e50b83b4f 100644
--- a/java/com/google/gerrit/acceptance/LightweightPluginDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/LightweightPluginDaemonTest.java
@@ -45,7 +45,6 @@ public class LightweightPluginDaemonTest extends AbstractDaemonTest {
             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 ba71c1185a..cafc775460 100644
--- a/java/com/google/gerrit/acceptance/TestPlugin.java
+++ b/java/com/google/gerrit/acceptance/TestPlugin.java
@@ -30,6 +30,4 @@ public @interface TestPlugin {
   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 f1804b3799..036285e330 100644
--- a/java/com/google/gerrit/server/plugins/ServerPlugin.java
+++ b/java/com/google/gerrit/server/plugins/ServerPlugin.java
@@ -51,7 +51,7 @@ public class ServerPlugin extends Plugin {
   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 @@ public class ServerPlugin extends Plugin {
     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 739dcdca16..0f7e87e3bd 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.lifecycle.LifecycleModule;
 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;
@@ -52,10 +51,6 @@ class ServerPluginInfoModule extends AbstractModule {
         .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() {
@@ -100,10 +95,4 @@ class ServerPluginInfoModule extends AbstractModule {
   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 a58ee3b366..cd5d5e38cf 100644
--- a/java/com/google/gerrit/server/plugins/TestServerPlugin.java
+++ b/java/com/google/gerrit/server/plugins/TestServerPlugin.java
@@ -23,7 +23,6 @@ public class TestServerPlugin extends ServerPlugin {
   private String sysName;
   private String httpName;
   private String sshName;
-  private String apiName;
 
   public TestServerPlugin(
       String name,
@@ -33,7 +32,6 @@ public class TestServerPlugin extends ServerPlugin {
       String sysName,
       String httpName,
       String sshName,
-      String apiName,
       Path dataDir)
       throws InvalidPluginException {
     super(
@@ -51,7 +49,6 @@ public class TestServerPlugin extends ServerPlugin {
     this.sysName = sysName;
     this.httpName = httpName;
     this.sshName = sshName;
-    this.apiName = apiName;
     loadGuiceModules();
   }
 
@@ -60,7 +57,6 @@ public class TestServerPlugin extends ServerPlugin {
       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/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedChildRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/binding/PluginProvidedChildRestApiBindingsIT.java
index f211366e44..d2c1430409 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 @@ public class PluginProvidedChildRestApiBindingsIT extends AbstractDaemonTest {
   @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 class PluginProvidedChildRestApiBindingsIT extends AbstractDaemonTest {
   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 e9047e07d9..24ce605635 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 @@ public class PluginProvidedRootRestApiBindingsIT extends AbstractDaemonTest {
 
   @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 25cc33fc1d..0000000000
--- 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 d8def1ab01..0000000000
--- 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
-      }
-    }
-  }
-}