From 9d6b2e758034197feed357b4e75a905bb87d918d Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Tue, 23 Jun 2020 06:13:10 +0200 Subject: [PATCH 01/11] LifecycleListener: Mark stop method as default to make it optional Many caller sites would only want to implement start() and left out custom stop() method implementation. Better support that use case so that caller sites wouldn't have to provide empty methods. Similar approach is implemented with InitStep#postRun() method so this change is consistent with the rest of the gerrit code. Change-Id: Ib991939a4dca531d61fd8bc6307bdc1d4eb7ed9f --- java/com/google/gerrit/extensions/events/LifecycleListener.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/com/google/gerrit/extensions/events/LifecycleListener.java b/java/com/google/gerrit/extensions/events/LifecycleListener.java index dae4b543a3..893871b0be 100644 --- a/java/com/google/gerrit/extensions/events/LifecycleListener.java +++ b/java/com/google/gerrit/extensions/events/LifecycleListener.java @@ -24,5 +24,5 @@ public interface LifecycleListener extends EventListener { void start(); /** Invoked when the server is stopping. */ - void stop(); + default void stop() {} } From 234c9675b291ad3201e51169cf0b40c27bad93ee Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 23 Jun 2020 15:39:52 +0900 Subject: [PATCH 02/11] Update git submodules * Update plugins/replication from branch 'stable-3.0' to 727d62d9d0dddc53fad52ac61d936155cad782ad - ReplicationIT: Test that branch deletion is replicated When a branch is deleted, the deletion should be replicated to the remote when "remote.name.mirror" is enabled. Change-Id: If59a9bb15958f4559d62452a309afcf1ca6c3789 --- plugins/replication | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/replication b/plugins/replication index 116cbc1157..727d62d9d0 160000 --- a/plugins/replication +++ b/plugins/replication @@ -1 +1 @@ -Subproject commit 116cbc11573e7afb0ce0782518a172cf798ec15c +Subproject commit 727d62d9d0dddc53fad52ac61d936155cad782ad From 1fe4fb16adc109f3f50278238325a93e5183250f Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Wed, 24 Jun 2020 04:56:01 +0000 Subject: [PATCH 03/11] Revert "LifecycleListener: Mark stop method as default to make it optional" This reverts commit 9d6b2e758034197feed357b4e75a905bb87d918d. Reason for revert: FunctionalInterfaceClash is flagged by Error Prone. Change-Id: I6cec468e10dabedfa820259cb57a60376e4f3675 --- java/com/google/gerrit/extensions/events/LifecycleListener.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/com/google/gerrit/extensions/events/LifecycleListener.java b/java/com/google/gerrit/extensions/events/LifecycleListener.java index 893871b0be..dae4b543a3 100644 --- a/java/com/google/gerrit/extensions/events/LifecycleListener.java +++ b/java/com/google/gerrit/extensions/events/LifecycleListener.java @@ -24,5 +24,5 @@ public interface LifecycleListener extends EventListener { void start(); /** Invoked when the server is stopping. */ - default void stop() {} + void stop(); } From 5611bebc9c4ac98f6fcd571d9398745bc3fdfc74 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 23 Jun 2020 15:53:35 +0900 Subject: [PATCH 04/11] ErrorProne: Increase severity of FunctionalInterfaceClash to ERROR Change-Id: I6008da8506eaa0764e8265a10384987ef0b1c3b1 --- tools/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/BUILD b/tools/BUILD index 9a53c8b6c6..fb3ed434c6 100644 --- a/tools/BUILD +++ b/tools/BUILD @@ -44,7 +44,7 @@ java_package_configuration( "-Xep:FloatingPointLiteralPrecision:WARN", "-Xep:FragmentInjection:WARN", "-Xep:FragmentNotInstantiable:WARN", - "-Xep:FunctionalInterfaceClash:WARN", + "-Xep:FunctionalInterfaceClash:ERROR", "-Xep:FutureReturnValueIgnored:WARN", "-Xep:GetClassOnEnum:WARN", "-Xep:ImmutableAnnotationChecker:WARN", From 89e87b5fe1ba2a7e8b177c620d9ba8520958f5f0 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 24 Jun 2020 17:55:51 +0900 Subject: [PATCH 05/11] Update git submodules * Update plugins/replication from branch 'stable-3.1' to 35ef13dd9ebe3dd1995273ea886f23544d33bc5d - Merge branch 'stable-3.0' into stable-3.1 * stable-3.0: ReplicationIT: Test that branch deletion is replicated Make SecureCredentialsFactory public Change-Id: If3417a41e390a296b7d4eec44e2d49c966fc8416 - ReplicationIT: Test that branch deletion is replicated When a branch is deleted, the deletion should be replicated to the remote when "remote.name.mirror" is enabled. Change-Id: If59a9bb15958f4559d62452a309afcf1ca6c3789 - Merge branch 'stable-2.16' into stable-3.0 * stable-2.16: Make SecureCredentialsFactory public Change-Id: I757ba1004ce2a851c7857762b178de9294deae21 - Make SecureCredentialsFactory public Access to secure.config is useful to more than just replication plugin. Allow instantiating this class from packages other than replication plugin. Specifically this is useful, as this class can be used from pull-replication too. Change-Id: Id268c869e993c6cabacfa0043ec269172e0efba1 (cherry picked from commit c09a7c08fb44094c7475313ac52154adac39a54c) --- plugins/replication | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/replication b/plugins/replication index 5019d29de0..35ef13dd9e 160000 --- a/plugins/replication +++ b/plugins/replication @@ -1 +1 @@ -Subproject commit 5019d29de075b90e66762b2b27909da14ed01293 +Subproject commit 35ef13dd9ebe3dd1995273ea886f23544d33bc5d From fa1a7b006053b7fbabf0ab9bb23f9cf47a18c5ae Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 24 Jun 2020 17:55:57 +0900 Subject: [PATCH 06/11] Update git submodules * Update plugins/replication from branch 'stable-3.1' to 2eced6589e02c1d733ffa2666fae851f2a8000cb - ReplicationIT: Enable task deletion when testing delete branch When deletion of tasks is disabled, the test fails because the previous task for replication of the branch creation is still persisted. This causes the actual number of tasks to differ from the expected. Re-enable deletion for this test only. Change-Id: I52f58e9f61fbc5f751139c4c11e04ad850b772a3 --- plugins/replication | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/replication b/plugins/replication index 35ef13dd9e..2eced6589e 160000 --- a/plugins/replication +++ b/plugins/replication @@ -1 +1 @@ -Subproject commit 35ef13dd9ebe3dd1995273ea886f23544d33bc5d +Subproject commit 2eced6589e02c1d733ffa2666fae851f2a8000cb From 3c1c4978b803a1b03d04babbef080471cacbd271 Mon Sep 17 00:00:00 2001 From: Tao Zhou Date: Wed, 24 Jun 2020 10:30:09 +0200 Subject: [PATCH 07/11] Convert plugin's name to lowercase when generate hook name Change-Id: I6bf2ad70f3d771b5caf71dc2bb665b8b5d860b4b --- .../app/elements/plugins/gr-dom-hooks/gr-dom-hooks.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/polygerrit-ui/app/elements/plugins/gr-dom-hooks/gr-dom-hooks.js b/polygerrit-ui/app/elements/plugins/gr-dom-hooks/gr-dom-hooks.js index 028285bc14..c6d3e0f764 100644 --- a/polygerrit-ui/app/elements/plugins/gr-dom-hooks/gr-dom-hooks.js +++ b/polygerrit-ui/app/elements/plugins/gr-dom-hooks/gr-dom-hooks.js @@ -27,7 +27,12 @@ if (opt_moduleName) { return endpointName + ' ' + opt_moduleName; } else { - return this._plugin.getPluginName() + '-autogenerated-' + endpointName; + // lowercase in case plugin's name contains uppercase letters + // TODO: this still can not prevent if plugin has invalid char + // other than uppercase, but is the first step + // https://html.spec.whatwg.org/multipage/custom-elements.html#valid-custom-element-name + const pluginName = this._plugin.getPluginName() || 'unknown_plugin'; + return pluginName.toLowerCase() + '-autogenerated-' + endpointName; } }; From feb36258f36755586a68c5f27285bc1e5c5e591c Mon Sep 17 00:00:00 2001 From: Marcin Czech Date: Tue, 23 Jun 2020 14:36:04 +0200 Subject: [PATCH 08/11] Fix issue with auto registering ssh commands AutoRegisterModules and PluginGuiceEnvironment checks if ssh command implements org.apache.sshd.server.Command interface. Correct interface name is org.apache.sshd.server.command.Command Bug: Issue 12988 Change-Id: Ia17954396f6c7b16c932fcc30c9de522e991cffa --- .../server/plugins/AutoRegisterModules.java | 2 +- .../plugins/PluginGuiceEnvironment.java | 2 +- javatests/com/google/gerrit/server/BUILD | 1 + .../plugins/AutoRegisterModulesTest.java | 108 ++++++++++++++++++ 4 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 javatests/com/google/gerrit/server/plugins/AutoRegisterModulesTest.java diff --git a/java/com/google/gerrit/server/plugins/AutoRegisterModules.java b/java/com/google/gerrit/server/plugins/AutoRegisterModules.java index fde61ff3c8..9d93ed2fda 100644 --- a/java/com/google/gerrit/server/plugins/AutoRegisterModules.java +++ b/java/com/google/gerrit/server/plugins/AutoRegisterModules.java @@ -158,7 +158,7 @@ class AutoRegisterModules { return; } - if (is("org.apache.sshd.server.Command", clazz)) { + if (is("org.apache.sshd.server.command.Command", clazz)) { sshGen.export(export, clazz); } else if (is("javax.servlet.http.HttpServlet", clazz)) { httpGen.export(export, clazz); diff --git a/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java b/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java index ed9d2c78bb..ee4381be61 100644 --- a/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java +++ b/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java @@ -587,7 +587,7 @@ public class PluginGuiceEnvironment { return false; } - if (is("org.apache.sshd.server.Command", type)) { + if (is("org.apache.sshd.server.command.Command", type)) { return false; } diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD index 63ae008352..e850fdc387 100644 --- a/javatests/com/google/gerrit/server/BUILD +++ b/javatests/com/google/gerrit/server/BUILD @@ -55,6 +55,7 @@ junit_tests( "//java/com/google/gerrit/server/restapi", "//java/com/google/gerrit/server/schema", "//java/com/google/gerrit/server/util/time", + "//java/com/google/gerrit/sshd", "//java/com/google/gerrit/testing:gerrit-test-util", "//java/com/google/gerrit/truth", "//java/org/eclipse/jgit:server", diff --git a/javatests/com/google/gerrit/server/plugins/AutoRegisterModulesTest.java b/javatests/com/google/gerrit/server/plugins/AutoRegisterModulesTest.java new file mode 100644 index 0000000000..412798bef4 --- /dev/null +++ b/javatests/com/google/gerrit/server/plugins/AutoRegisterModulesTest.java @@ -0,0 +1,108 @@ +// Copyright (C) 2020 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 static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.eq; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.expectLastCall; + +import com.google.common.collect.Lists; +import com.google.gerrit.extensions.annotations.Export; +import com.google.gerrit.extensions.annotations.Listen; +import com.google.gerrit.sshd.SshCommand; +import java.io.IOException; +import java.io.InputStream; +import java.lang.annotation.Annotation; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import java.util.jar.Manifest; +import org.easymock.EasyMockSupport; +import org.junit.Test; + +public class AutoRegisterModulesTest { + + @Test + public void shouldRegisterSshCommand() throws InvalidPluginException { + EasyMockSupport ems = new EasyMockSupport(); + ModuleGenerator sshModule = ems.createNiceMock(ModuleGenerator.class); + + PluginGuiceEnvironment env = ems.createNiceMock(PluginGuiceEnvironment.class); + expect(env.hasSshModule()).andReturn(true); + expect(env.newSshModuleGenerator()).andReturn(sshModule); + + sshModule.setPluginName("test_plugin_name"); + expectLastCall(); + sshModule.export(anyObject(Export.class), eq(TestSshCommand.class)); + expectLastCall(); + + ems.replayAll(); + + PluginContentScanner scanner = new TestPluginContextScanner(); + ClassLoader classLoader = this.getClass().getClassLoader(); + + AutoRegisterModules objectUnderTest = + new AutoRegisterModules("test_plugin_name", env, scanner, classLoader); + objectUnderTest.discover(); + + ems.verifyAll(); + } + + @Export(value = "test") + public class TestSshCommand extends SshCommand { + @Override + protected void run() throws UnloggedFailure, Failure, Exception {} + } + + private class TestPluginContextScanner implements PluginContentScanner { + + @Override + public Manifest getManifest() throws IOException { + return null; + } + + @Override + public Map, Iterable> scan( + String pluginName, Iterable> annotations) + throws InvalidPluginException { + Map, Iterable> extensions = new HashMap<>(); + extensions.put( + Export.class, + Lists.newArrayList( + new ExtensionMetaData( + "com.google.gerrit.server.plugins.AutoRegisterModulesTest$TestSshCommand", + "com.google.gerrit.extensions.annotations.Export"))); + extensions.put(Listen.class, Lists.newArrayList()); + return extensions; + } + + @Override + public Optional getEntry(String resourcePath) throws IOException { + return null; + } + + @Override + public InputStream getInputStream(PluginEntry entry) throws IOException { + return null; + } + + @Override + public Enumeration entries() { + return null; + } + } +} From 771aceedbfc39ba0cbaa17462732c8ac18f9a3a8 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 25 Jun 2020 07:08:17 +0900 Subject: [PATCH 09/11] Update git submodules * Update plugins/replication from branch 'stable-3.2' to 291560e57712fc8ea61db80ae05f0453eae0c4df - Merge branch 'stable-3.1' into stable-3.2 * stable-3.1: ReplicationIT: Enable task deletion when testing delete branch ReplicationIT: Test that branch deletion is replicated Make SecureCredentialsFactory public Change-Id: I07b9127c3e6930b96a38e115265e7cf7cff92b76 - ReplicationIT: Enable task deletion when testing delete branch When deletion of tasks is disabled, the test fails because the previous task for replication of the branch creation is still persisted. This causes the actual number of tasks to differ from the expected. Re-enable deletion for this test only. Change-Id: I52f58e9f61fbc5f751139c4c11e04ad850b772a3 - Merge branch 'stable-3.0' into stable-3.1 * stable-3.0: ReplicationIT: Test that branch deletion is replicated Make SecureCredentialsFactory public Change-Id: If3417a41e390a296b7d4eec44e2d49c966fc8416 - ReplicationIT: Test that branch deletion is replicated When a branch is deleted, the deletion should be replicated to the remote when "remote.name.mirror" is enabled. Change-Id: If59a9bb15958f4559d62452a309afcf1ca6c3789 - Merge branch 'stable-2.16' into stable-3.0 * stable-2.16: Make SecureCredentialsFactory public Change-Id: I757ba1004ce2a851c7857762b178de9294deae21 - Make SecureCredentialsFactory public Access to secure.config is useful to more than just replication plugin. Allow instantiating this class from packages other than replication plugin. Specifically this is useful, as this class can be used from pull-replication too. Change-Id: Id268c869e993c6cabacfa0043ec269172e0efba1 (cherry picked from commit c09a7c08fb44094c7475313ac52154adac39a54c) --- plugins/replication | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/replication b/plugins/replication index 97291b26b3..291560e577 160000 --- a/plugins/replication +++ b/plugins/replication @@ -1 +1 @@ -Subproject commit 97291b26b32cb16ae33ac318ea02ae22a980adfd +Subproject commit 291560e57712fc8ea61db80ae05f0453eae0c4df From 51e7e5ac56607b41789dcddbc3c5731686790d20 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 25 Jun 2020 08:31:54 +0900 Subject: [PATCH 10/11] AutoRegisterModulesTest: Declare inner classes as static Change-Id: I6e6df0455c51da61da662bbcafc7a2f35db43c40 --- .../google/gerrit/server/plugins/AutoRegisterModulesTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/javatests/com/google/gerrit/server/plugins/AutoRegisterModulesTest.java b/javatests/com/google/gerrit/server/plugins/AutoRegisterModulesTest.java index 412798bef4..0ffd4acbc1 100644 --- a/javatests/com/google/gerrit/server/plugins/AutoRegisterModulesTest.java +++ b/javatests/com/google/gerrit/server/plugins/AutoRegisterModulesTest.java @@ -63,12 +63,12 @@ public class AutoRegisterModulesTest { } @Export(value = "test") - public class TestSshCommand extends SshCommand { + public static class TestSshCommand extends SshCommand { @Override protected void run() throws UnloggedFailure, Failure, Exception {} } - private class TestPluginContextScanner implements PluginContentScanner { + private static class TestPluginContextScanner implements PluginContentScanner { @Override public Manifest getManifest() throws IOException { From 14d8be43627354cc866087f2e08b8a24bbc29509 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Fri, 24 May 2019 10:15:40 -0700 Subject: [PATCH 11/11] Error Prone: Enable and fix ClassCanBeStatic Change-Id: Ice8202695c2a525d751682185e1aa94663e0fbe0 --- tools/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/BUILD b/tools/BUILD index 22860fdc27..9a53c8b6c6 100644 --- a/tools/BUILD +++ b/tools/BUILD @@ -31,7 +31,7 @@ java_package_configuration( "-Xep:BadComparable:WARN", "-Xep:BoxedPrimitiveConstructor:ERROR", "-Xep:CannotMockFinalClass:WARN", - "-Xep:ClassCanBeStatic:WARN", + "-Xep:ClassCanBeStatic:ERROR", "-Xep:ClassNewInstance:WARN", "-Xep:DateFormatConstant:ERROR", "-Xep:DefaultCharset:ERROR",