Merge branch 'stable-2.15' into stable-2.16 * stable-2.15: Introduce a configuration to define time lapse for rate limiter Change-Id: Ie63b70cfdf46b2ba997eab1357f3be52ee650814
diff --git a/.bazelversion b/.bazelversion index 9084fa2..fd2a018 100644 --- a/.bazelversion +++ b/.bazelversion
@@ -1 +1 @@ -1.1.0 +3.1.0
diff --git a/WORKSPACE b/WORKSPACE index 3359311..6c8cbef 100644 --- a/WORKSPACE +++ b/WORKSPACE
@@ -3,7 +3,7 @@ load("//:bazlets.bzl", "load_bazlets") load_bazlets( - commit = "9af263722b7eafe99af079d6ef7cf1de23e6f8d7", + commit = "6c39deb06f58bb62162ccb6865964f531739f512", #local_path = "/home/<user>/projects/bazlets", )
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Configuration.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Configuration.java index db21aa9..c05a01b 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Configuration.java +++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Configuration.java
@@ -17,11 +17,11 @@ import com.google.common.collect.ArrayTable; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Table; -import com.google.gerrit.common.data.GroupDescription; +import com.google.gerrit.common.data.GroupDescription.Basic; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.server.config.PluginConfigFactory; -import com.google.gerrit.server.group.GroupsCollection; +import com.google.gerrit.server.group.GroupResolver; import com.google.inject.Inject; import com.google.inject.ProvisionException; import com.google.inject.Singleton; @@ -30,14 +30,10 @@ import java.util.Map; import java.util.Map.Entry; import org.eclipse.jgit.lib.Config; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @Singleton class Configuration { - private static final Logger log = LoggerFactory.getLogger(Configuration.class); - static final String RATE_LIMIT_TOKEN = "${rateLimit}"; private static final String GROUP_SECTION = "group"; private static final String DEFAULT_UPLOADPACK_LIMIT_EXCEEDED_MSG = @@ -50,13 +46,13 @@ Configuration( PluginConfigFactory pluginConfigFactory, @PluginName String pluginName, - GroupsCollection groupsCollection) { + GroupResolver groupsCollection) { Config config = pluginConfigFactory.getGlobalPluginConfig(pluginName); parseAllGroupsRateLimits(config, groupsCollection); rateLimitExceededMsg = parseLimitExceededMsg(config); } - private void parseAllGroupsRateLimits(Config config, GroupsCollection groupsCollection) { + private void parseAllGroupsRateLimits(Config config, GroupResolver groupsCollection) { Map<String, AccountGroup.UUID> groups = getResolvedGroups(config, groupsCollection); if (groups.size() == 0) { return; @@ -68,16 +64,12 @@ } private Map<String, AccountGroup.UUID> getResolvedGroups( - Config config, GroupsCollection groupsCollection) { + Config config, GroupResolver groupsCollection) { LinkedHashMap<String, AccountGroup.UUID> groups = new LinkedHashMap<>(); for (String groupName : config.getSubsections(GROUP_SECTION)) { - GroupDescription.Basic groupDesc = groupsCollection.parseId(groupName); - - // Group either is mis-configured, never existed, or was deleted/removed since. - if (groupDesc == null) { - log.warn("Invalid configuration, group not found: {}", groupName); - } else { - groups.put(groupName, groupDesc.getGroupUUID()); + Basic basic = groupsCollection.parseId(groupName); + if (basic != null) { + groups.put(groupName, basic.getGroupUUID()); } } return groups;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UserResolver.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UserResolver.java index 852367e..11dffed 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UserResolver.java +++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UserResolver.java
@@ -39,7 +39,9 @@ Optional<String> getUserName(String key) { Optional<IdentifiedUser> user = getIdentifiedUser(key); - return user.isPresent() ? Optional.ofNullable(user.get().getUserName()) : Optional.empty(); + return user.isPresent() + ? Optional.ofNullable(user.get().getUserName().get()) + : Optional.empty(); } private static boolean isNumeric(String key) {
diff --git a/src/main/resources/Documentation/build.md b/src/main/resources/Documentation/build.md index aa04426..68e848c 100644 --- a/src/main/resources/Documentation/build.md +++ b/src/main/resources/Documentation/build.md
@@ -61,6 +61,13 @@ bazel-bin/plugins/@PLUGIN@/@PLUGIN@.jar ``` +To execute the tests run either one of: + +``` + bazel test --test_tag_filters=@PLUGIN@ //... + bazel test plugins/@PLUGIN@:@PLUGIN@_tests +``` + This project can be imported into the Eclipse IDE: Add the plugin name to the `CUSTOM_PLUGINS` in `tools/bzl/plugins.bzl`, and execute: @@ -69,13 +76,6 @@ ./tools/eclipse/project.py ``` -To execute the tests run: - -``` - bazel test plugins/@PLUGIN@:@PLUGIN@_tests -``` - - [Back to @PLUGIN@ documentation index][index] [index]: index.html
diff --git a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/ConfigurationTest.java b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/ConfigurationTest.java index 78ef8dc..4f8d1a1 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/ConfigurationTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/ConfigurationTest.java
@@ -15,20 +15,19 @@ package com.googlesource.gerrit.plugins.ratelimiter; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.mockito.Mockito.when; import com.google.gerrit.common.data.GroupDescription; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.server.config.PluginConfigFactory; -import com.google.gerrit.server.group.GroupsCollection; +import com.google.gerrit.server.group.GroupResolver; import com.google.inject.ProvisionException; import java.util.Map; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; import org.junit.Before; -import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @@ -37,10 +36,8 @@ public class ConfigurationTest { private static final String PLUGIN_NAME = "rate-limiter"; - @Rule public ExpectedException exception = ExpectedException.none(); - @Mock private PluginConfigFactory pluginConfigFactoryMock; - @Mock private GroupsCollection groupsCollectionMock; + @Mock private GroupResolver groupsCollectionMock; @Mock private GroupDescription.Basic administratorsGroupDescMock; @Mock private GroupDescription.Basic someGroupDescMock; @@ -51,17 +48,16 @@ @Before public void setUp() { globalPluginConfig = new Config(); + when(pluginConfigFactoryMock.getGlobalPluginConfig(PLUGIN_NAME)).thenReturn(globalPluginConfig); - when(administratorsGroupDescMock.getName()).thenReturn("Administrators"); when(administratorsGroupDescMock.getGroupUUID()) .thenReturn(new AccountGroup.UUID("admin_uuid")); - when(groupsCollectionMock.parseId(administratorsGroupDescMock.getName())) - .thenReturn(administratorsGroupDescMock); + when(groupsCollectionMock.parseId("Administrators")).thenReturn(administratorsGroupDescMock); when(someGroupDescMock.getName()).thenReturn("someGroup"); when(someGroupDescMock.getGroupUUID()).thenReturn(new AccountGroup.UUID("some_uuid")); - when(groupsCollectionMock.parseId(someGroupDescMock.getName())).thenReturn(someGroupDescMock); + when(groupsCollectionMock.parseId("someGroup")).thenReturn(someGroupDescMock); } private Configuration getConfiguration() { @@ -93,10 +89,10 @@ globalPluginConfig.setInt( groupTagName, someGroupDescMock.getName(), "invalidTypePerHour", validRate); - exception.expect(ProvisionException.class); - exception.expectMessage( - "Invalid configuration, unsupported rate limit type: invalidTypePerHour"); - getConfiguration(); + ProvisionException thrown = assertThrows(ProvisionException.class, () -> getConfiguration()); + assertThat(thrown) + .hasMessageThat() + .contains("Invalid configuration, unsupported rate limit type: invalidTypePerHour"); } @Test @@ -109,12 +105,12 @@ RateLimitType.UPLOAD_PACK_PER_HOUR.toString(), invalidType); - exception.expect(ProvisionException.class); - exception.expectMessage( - "Invalid configuration, 'rate limit value '" - + invalidType - + "' for 'group.someGroup.uploadpackperhour' is not a valid number"); - getConfiguration(); + String expectedMessage = + String.format( + "Invalid configuration, 'rate limit value '%s' for 'group.someGroup.uploadpackperhour' is not a valid number", + invalidType); + ProvisionException thrown = assertThrows(ProvisionException.class, () -> getConfiguration()); + assertThat(thrown).hasMessageThat().contains(expectedMessage); } @Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitUploadPackIT.java b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitUploadPackIT.java index 7b32006..5b175b4 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitUploadPackIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitUploadPackIT.java
@@ -32,11 +32,11 @@ public class RateLimitUploadPackIT extends LightweightPluginDaemonTest { @Override - public void setUp() throws Exception { + public void setUpTestPlugin() throws Exception { // Create the group before the plugin is loaded since limits per group are // resolved at plugin load time. addUserToNewGroup(); - super.setUp(); + super.setUpTestPlugin(); } @Test
diff --git a/tools/BUILD b/tools/BUILD new file mode 100644 index 0000000..1fa2160 --- /dev/null +++ b/tools/BUILD
@@ -0,0 +1 @@ +# Empty file - bazel treat directories with BUILD file as a package \ No newline at end of file