From 8df8af2572fe2fd46bafb630f8f4a538dba2f308 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Wed, 7 Dec 2016 18:54:02 +0100 Subject: [PATCH] Manually bind plugin guice modules in gerrit Guice universe Short circuit the plugin loading phase from the unit tests by passing the module names from the manifest file. This allows us to avoid building and deploying the plugin JAR from within build tool chain. To use the simplified plugin bootstrap tests, plugin test class must be inherited from the LightweightPluginDaemonTest class and must be annotated with @TestPlugin annotation: @TestPlugin( name = "cookbook", sysModule = "com.googlesource.gerrit.plugins.cookbook.Module", httpModule = "com.googlesource.gerrit.plugins.cookbook.HttpModule", sshModule = "com.googlesource.gerrit.plugins.cookbook.SshModule" ) public class CookbookIT extends LightweightPluginDaemonTest { @Test public void revisionTest() throws Exception { createChange(); RestResponse response = adminRestSession.post("/changes/1/revisions/1/cookbook~hello-revision"); assertThat(response.getEntityContent()) .contains("Hello admin from change 1, patch set 1!"); } } Inspired-By: Dave Borowitz Change-Id: I689bb71413ecfbbf99f72730b0d2617bf526d9dd --- .../LightweightPluginDaemonTest.java | 64 ++++++++++++++++ .../gerrit/acceptance/PluginDaemonTest.java | 5 +- .../google/gerrit/acceptance/TestPlugin.java | 31 ++++++++ .../google/gerrit/server/plugins/Plugin.java | 3 +- .../plugins/PluginGuiceEnvironment.java | 2 +- .../gerrit/server/plugins/ServerPlugin.java | 18 +++-- .../server/plugins/TestServerPlugin.java | 73 +++++++++++++++++++ plugins/cookbook-plugin | 2 +- 8 files changed, 185 insertions(+), 13 deletions(-) create mode 100644 gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/LightweightPluginDaemonTest.java create mode 100644 gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/TestPlugin.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/plugins/TestServerPlugin.java diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/LightweightPluginDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/LightweightPluginDaemonTest.java new file mode 100644 index 0000000000..3a870cb50c --- /dev/null +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/LightweightPluginDaemonTest.java @@ -0,0 +1,64 @@ +// Copyright (C) 2016 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; + +import com.google.gerrit.server.PluginUser; +import com.google.gerrit.server.plugins.PluginGuiceEnvironment; +import com.google.gerrit.server.plugins.TestServerPlugin; +import com.google.inject.Inject; + +import org.junit.After; +import org.junit.Before; + +public class LightweightPluginDaemonTest extends AbstractDaemonTest { + @Inject + private PluginGuiceEnvironment env; + + @Inject + private PluginUser.Factory pluginUserFactory; + + private TestServerPlugin plugin; + + @Before + public void setUp() throws Exception { + TestPlugin testPlugin = getTestPlugin(getClass()); + String name = testPlugin.name(); + plugin = new TestServerPlugin(name, + canonicalWebUrl.get() + "plugins/" + name, + pluginUserFactory.create(name), + getClass().getClassLoader(), + testPlugin.sysModule(), + testPlugin.httpModule(), + testPlugin.sshModule()); + + plugin.start(env); + env.onStartPlugin(plugin); + } + + @After + public void tearDown() { + plugin.stop(env); + env.onStopPlugin(plugin); + } + + private static TestPlugin getTestPlugin(Class clazz) { + for (; clazz != null; clazz = clazz.getSuperclass()) { + if (clazz.getAnnotation(TestPlugin.class) != null) { + return clazz.getAnnotation(TestPlugin.class); + } + } + throw new IllegalStateException("TestPlugin annotation missing"); + } +} diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PluginDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PluginDaemonTest.java index dd73a838cb..6c387af5da 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PluginDaemonTest.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PluginDaemonTest.java @@ -104,12 +104,11 @@ public abstract class PluginDaemonTest extends AbstractDaemonTest { if (subPath.endsWith("plugins")) { pluginsIdx = idx; } - if (subPath.endsWith(BAZELOUT)) { + if (subPath.endsWith(BAZELOUT) || subPath.endsWith(ECLIPSE)) { bazel = true; buckOutIdx = idx; } - // TODO(davido): Fix Bazel plugin test from Eclipse - if (subPath.endsWith(BUCKOUT) || subPath.endsWith(ECLIPSE)) { + if (subPath.endsWith(BUCKOUT)) { buckOutIdx = idx; } idx++; diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/TestPlugin.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/TestPlugin.java new file mode 100644 index 0000000000..4a838ad434 --- /dev/null +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/TestPlugin.java @@ -0,0 +1,31 @@ +// Copyright (C) 2016 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; + +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +@Target({TYPE}) +@Retention(RUNTIME) +public @interface TestPlugin { + String name(); + + String sysModule() default ""; + String httpModule() default ""; + String sshModule() default ""; +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java index 4fe0c2a397..63a254a483 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java @@ -90,7 +90,8 @@ public abstract class Plugin { this.snapshot = snapshot; this.pluginUser = pluginUser; this.cacheKey = new Plugin.CacheKey(name); - this.disabled = srcPath.getFileName().toString().endsWith(".disabled"); + this.disabled = srcPath != null + && srcPath.getFileName().toString().endsWith(".disabled"); } public CleanupHandle getCleanupHandle() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java index 2c5354ecad..6dc6deaf5e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java @@ -265,7 +265,7 @@ public class PluginGuiceEnvironment { } } - void onStopPlugin(Plugin plugin) { + public void onStopPlugin(Plugin plugin) { for (StopPluginListener l : onStop) { l.onStopPlugin(plugin); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java index 59ed261e1c..7f89f71197 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java @@ -42,9 +42,9 @@ public class ServerPlugin extends Plugin { private final Path dataDir; private final String pluginCanonicalWebUrl; private final ClassLoader classLoader; - private Class sysModule; - private Class sshModule; - private Class httpModule; + protected Class sysModule; + protected Class sshModule; + protected Class httpModule; private Injector sysInjector; private Injector sshInjector; @@ -61,13 +61,17 @@ public class ServerPlugin extends Plugin { Path dataDir, ClassLoader classLoader) throws InvalidPluginException { super(name, srcJar, pluginUser, snapshot, - Plugin.getApiType(getPluginManifest(scanner))); + scanner == null + ? ApiType.PLUGIN + : Plugin.getApiType(getPluginManifest(scanner))); this.pluginCanonicalWebUrl = pluginCanonicalWebUrl; this.scanner = scanner; this.dataDir = dataDir; this.classLoader = classLoader; - this.manifest = getPluginManifest(scanner); - loadGuiceModules(manifest, classLoader); + this.manifest = scanner == null ? null : getPluginManifest(scanner); + if (manifest != null) { + loadGuiceModules(manifest, classLoader); + } } private void loadGuiceModules(Manifest manifest, ClassLoader classLoader) throws InvalidPluginException { @@ -92,7 +96,7 @@ public class ServerPlugin extends Plugin { } @SuppressWarnings("unchecked") - private static Class load(String name, ClassLoader pluginLoader) + protected static Class load(String name, ClassLoader pluginLoader) throws ClassNotFoundException { if (Strings.isNullOrEmpty(name)) { return null; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/TestServerPlugin.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/TestServerPlugin.java new file mode 100644 index 0000000000..98cd3d2a5e --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/TestServerPlugin.java @@ -0,0 +1,73 @@ +// Copyright (C) 2016 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.server.plugins; + +import com.google.gerrit.server.PluginUser; + +public class TestServerPlugin extends ServerPlugin { + private final ClassLoader classLoader; + private String sysName; + private String httpName; + private String sshName; + + public TestServerPlugin(String name, String pluginCanonicalWebUrl, + PluginUser user, ClassLoader classloader, String sysName, + String httpName, String sshName) + throws InvalidPluginException { + super(name, pluginCanonicalWebUrl, user, null, null, null, null, classloader); + this.classLoader = classloader; + this.sysName = sysName; + this.httpName = httpName; + this.sshName = sshName; + loadGuiceModules(); + } + + private void loadGuiceModules() throws InvalidPluginException { + try { + this.sysModule = load(sysName, classLoader); + this.httpModule = load(httpName, classLoader); + this.sshModule = load(sshName, classLoader); + } catch (ClassNotFoundException e) { + throw new InvalidPluginException("Unable to load plugin Guice Modules", e); + } + } + + @Override + public String getVersion() { + return "1.0"; + } + + @Override + protected boolean canReload() { + return false; + } + + @Override + // Widen access modifier in derived class + public void start(PluginGuiceEnvironment env) throws Exception { + super.start(env); + } + + @Override + // Widen access modifier in derived class + public void stop(PluginGuiceEnvironment env) { + super.stop(env); + } + + @Override + public PluginContentScanner getContentScanner() { + return null; + } +} diff --git a/plugins/cookbook-plugin b/plugins/cookbook-plugin index aa4601c16e..d3c74f3692 160000 --- a/plugins/cookbook-plugin +++ b/plugins/cookbook-plugin @@ -1 +1 @@ -Subproject commit aa4601c16e42c8ea0de251299b2a493a4068add0 +Subproject commit d3c74f3692b0b10926cab300baaef905fcdf4136