Add option to specify config in All-Projects
Now it is possible to change configuration of rate limiter in
All-Projects configuration.
Rate-limiter would only apply configuration from the All-Projects, if
gerrit is not replica.
To change configuration of rate limiter one needs to add
rate-limiter.config file to All-Projects refs/meta/config. If there is
no rate-limiter.config in All-Projects then the configuration would be
taken from etc/rate-limiter.config.
If the rate-limiter configuration changed, then it would only affect users
whose permits were changed. If the permit of the user did not change
then the rate limit would stay. In case permit was changed then rate-limit
of the user would be deleted.
Feature: Issue 10306
Change-Id: Ie412837fb23d9c5c7e27e9edf5c0f92cf864ae57
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 db9c2e5..f4e8982 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Configuration.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Configuration.java
@@ -20,9 +20,15 @@
import com.google.common.collect.Table;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.GroupDescription.Basic;
+import com.google.gerrit.entities.ImmutableConfig;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.GerritIsReplica;
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.group.GroupResolver;
+import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.project.ProjectConfig;
import com.google.inject.Inject;
import com.google.inject.ProvisionException;
import com.google.inject.Singleton;
@@ -33,29 +39,67 @@
import java.util.Map.Entry;
import java.util.Optional;
import java.util.concurrent.CopyOnWriteArrayList;
+import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
@Singleton
class Configuration {
static final String RATE_LIMIT_TOKEN = "${rateLimit}";
+ private static final Logger log = LoggerFactory.getLogger(RateLimitUploadPack.class);
private static final String GROUP_SECTION = "group";
private static final String DEFAULT_UPLOADPACK_LIMIT_EXCEEDED_MSG =
"Exceeded rate limit of " + RATE_LIMIT_TOKEN + " fetch requests/hour";
-
+ private static final String RATE_LIMITER_CONFIG = "rate-limiter.config";
private Table<RateLimitType, AccountGroup.UUID, RateLimit> rateLimits;
private List<AccountGroup.UUID> recipients;
- private final String rateLimitExceededMsg;
+ private String rateLimitExceededMsg;
+ private final Boolean isReplica;
+ private final PluginConfigFactory pluginConfigFactory;
+ private final GroupResolver groupsCollection;
+ private final String pluginName;
+ private final Config defaultRateLimiterConfig;
@Inject
Configuration(
+ AllProjectsName allProjectsName,
PluginConfigFactory pluginConfigFactory,
@PluginName String pluginName,
+ @GerritIsReplica Boolean isReplica,
GroupResolver groupsCollection) {
- Config config = pluginConfigFactory.getGlobalPluginConfig(pluginName);
- parseAllGroupsRateLimits(config, groupsCollection);
- rateLimitExceededMsg = parseLimitExceededMsg(config);
+ this.pluginConfigFactory = pluginConfigFactory;
+ this.groupsCollection = groupsCollection;
+ this.pluginName = pluginName;
+ this.isReplica = isReplica;
+ this.defaultRateLimiterConfig = pluginConfigFactory.getGlobalPluginConfig(pluginName);
+ initConfig(loadConfig(allProjectsName.get()));
+ }
+
+ private void initConfig(Config config) {
recipients = parseUserGroupsForEmailNotification(config, groupsCollection);
+ rateLimitExceededMsg = parseLimitExceededMsg(config);
+ Map<String, AccountGroup.UUID> groups = getResolvedGroups(config, groupsCollection);
+ parseAllGroupsRateLimits(config, groups);
+ }
+
+ private Config loadConfig(String projectName) {
+ if (isReplica) {
+ return defaultRateLimiterConfig;
+ }
+ try {
+ Config config =
+ pluginConfigFactory.getProjectPluginConfigWithInheritance(
+ Project.NameKey.parse(projectName), pluginName);
+ if (config == null || config.getSubsections(GROUP_SECTION).isEmpty()) {
+ config = defaultRateLimiterConfig;
+ }
+ return config;
+ } catch (NoSuchProjectException e) {
+ log.warn("No project {} found", projectName);
+ return defaultRateLimiterConfig;
+ }
}
private List<AccountGroup.UUID> parseUserGroupsForEmailNotification(
@@ -82,9 +126,32 @@
return groups;
}
- private void parseAllGroupsRateLimits(Config config, GroupResolver groupsCollection) {
- Map<String, AccountGroup.UUID> groups = getResolvedGroups(config, groupsCollection);
+ void refreshTable(ProjectConfig newCfg, ProjectConfig oldCfg) {
+ if (oldCfg != null) {
+ try {
+ ImmutableMap<String, String> oldCacheable = oldCfg.getCacheable().getProjectLevelConfigs();
+ String oldStringConfig = oldCacheable.getOrDefault(RATE_LIMITER_CONFIG, "");
+ ImmutableMap<String, String> newCacheable = newCfg.getCacheable().getProjectLevelConfigs();
+ String newStringConfig = newCacheable.getOrDefault(RATE_LIMITER_CONFIG, "");
+ if (oldStringConfig.equals(newStringConfig)) {
+ return;
+ }
+ if (newStringConfig != "") {
+ Config newConfig = ImmutableConfig.parse(newStringConfig).mutableCopy();
+ initConfig(newConfig);
+ } else {
+ initConfig(defaultRateLimiterConfig);
+ }
+ } catch (ConfigInvalidException e) {
+ log.warn("Invalid Configuration");
+ }
+ }
+ }
+
+ private void parseAllGroupsRateLimits(Config config, Map<String, AccountGroup.UUID> groups) {
if (groups.size() == 0) {
+ log.warn("No configuration found");
+ rateLimits = null;
return;
}
rateLimits = ArrayTable.create(Arrays.asList(RateLimitType.values()), groups.values());
@@ -150,4 +217,36 @@
List<AccountGroup.UUID> getRecipients() {
return !recipients.isEmpty() ? recipients : ImmutableList.of();
}
+
+ static boolean isSameRateLimitType(
+ RateLimiter limiter, Optional<RateLimit> limit, Optional<RateLimit> warn) {
+ if (limit.isPresent() && warn.isPresent()) {
+ return limiter instanceof WarningRateLimiter;
+ }
+ if (limit.isEmpty() && warn.isPresent()) {
+ return limiter instanceof WarningUnlimitedRateLimiter;
+ }
+ if (limit.isPresent()) {
+ return limiter instanceof PeriodicRateLimiter;
+ } else {
+ return limiter instanceof UnlimitedRateLimiter;
+ }
+ }
+
+ public static boolean validTimeLapse(Optional<RateLimit> timeLapse, int defaultTimeLapce) {
+ if (timeLapse.isPresent()) {
+ long providedTimeLapse = timeLapse.get().getRatePerHour();
+ if (providedTimeLapse > 0 && providedTimeLapse <= defaultTimeLapce) {
+ return true;
+ }
+ log.warn(
+ "The time lapse is set to the default {} minutes, as the configured value is invalid.",
+ defaultTimeLapce);
+ } else {
+ log.warn(
+ "The time lapse is set to the default {} minutes, as the configured value is not present.",
+ defaultTimeLapce);
+ }
+ return false;
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Module.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Module.java
index 536a3d7..e5d6309 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Module.java
@@ -18,6 +18,7 @@
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.cache.RemovalListener;
+import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.git.validators.UploadValidationListener;
@@ -32,14 +33,15 @@
import java.util.Optional;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
-import org.slf4j.Logger;
class Module extends AbstractModule {
static final String UPLOAD_PACK_PER_HOUR = "upload_pack_per_hour";
static final String DEFAULT_RATE_LIMIT_TYPE = "upload pack";
+ static final Integer DEFAULT_LIMIT = Integer.MAX_VALUE;
@Override
protected void configure() {
+ DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(RateLimiterListener.class);
DynamicSet.bind(binder(), UploadValidationListener.class).to(RateLimitUploadPack.class);
bind(Configuration.class).asEagerSingleton();
bind(ScheduledExecutorService.class)
@@ -69,12 +71,11 @@
.build(loader.get());
}
- private static class RateLimiterLoader extends CacheLoader<String, RateLimiter> {
+ static class RateLimiterLoader extends CacheLoader<String, RateLimiter> {
private final RateLimitFinder finder;
private final PeriodicRateLimiter.Factory periodicRateLimiterFactory;
private final WarningRateLimiter.Factory warningRateLimiterFactory;
private final WarningUnlimitedRateLimiter.Factory warningUnlimitedRateLimiterFactory;
- private final Logger logger;
@Inject
RateLimiterLoader(
@@ -86,7 +87,6 @@
this.periodicRateLimiterFactory = periodicRateLimiterFactory;
this.warningRateLimiterFactory = warningRateLimiterFactory;
this.warningUnlimitedRateLimiterFactory = warningUnlimitedRateLimiterFactory;
- this.logger = RateLimiterStatsLog.getLogger();
}
@Override
@@ -101,24 +101,18 @@
String rateLimitType = DEFAULT_RATE_LIMIT_TYPE;
// In the case that there is a warning but no limit
- Integer myLimit = Integer.MAX_VALUE;
+ int myLimit = DEFAULT_LIMIT;
if (limit.isPresent()) {
myLimit = limit.get().getRatePerHour();
rateLimitType = limit.get().getType().getLimitType();
}
int effectiveTimeLapse = PeriodicRateLimiter.DEFAULT_TIME_LAPSE_IN_MINUTES;
- if (timeLapse.isPresent()) {
- int providedTimeLapse = timeLapse.get().getRatePerHour();
- if (providedTimeLapse > 0 && providedTimeLapse <= effectiveTimeLapse) {
- effectiveTimeLapse = providedTimeLapse;
- rateLimitType = timeLapse.get().getType().getLimitType();
- } else {
- logger.warn(
- "The time lapse is set to the default {} minutes, as the configured value is invalid.",
- effectiveTimeLapse);
- }
+ if (Configuration.validTimeLapse(timeLapse, effectiveTimeLapse)) {
+ effectiveTimeLapse = timeLapse.get().getRatePerHour();
+ rateLimitType = timeLapse.get().getType().getLimitType();
}
+
RateLimiter rateLimiter =
periodicRateLimiterFactory.create(myLimit, effectiveTimeLapse, rateLimitType);
@@ -131,5 +125,36 @@
}
return rateLimiter;
}
+
+ boolean isValidKey(String key, RateLimiter limiter) {
+ Optional<RateLimit> limit = finder.find(RateLimitType.UPLOAD_PACK_PER_HOUR, key);
+ Optional<RateLimit> warn = finder.find(RateLimitType.UPLOAD_PACK_PER_HOUR_WARN, key);
+ Optional<RateLimit> timeLapse = finder.find(RateLimitType.TIME_LAPSE_IN_MINUTES, key);
+
+ int tableLimit = limit.map(RateLimit::getRatePerHour).orElse(DEFAULT_LIMIT);
+ int tableTimeLapse =
+ timeLapse
+ .map(RateLimit::getRatePerHour)
+ .orElse(PeriodicRateLimiter.DEFAULT_TIME_LAPSE_IN_MINUTES);
+ // Check if two limiters are same type
+ if (!Configuration.isSameRateLimitType(limiter, limit, warn)) {
+ return false;
+ }
+ // Check if two limiters have same permits
+ if (limiter.permitsPerHour() != tableLimit) {
+ return false;
+ }
+ // Check if two limiters have same timeLapse
+ if (limiter.getTimeLapse().orElse(PeriodicRateLimiter.DEFAULT_TIME_LAPSE_IN_MINUTES)
+ != tableTimeLapse) {
+ return false;
+ }
+ // Check if two limiters have same warnLimit
+ if (limiter instanceof WarningRateLimiter || limiter instanceof WarningUnlimitedRateLimiter) {
+ Optional<Integer> warningLimit = limiter.getWarnLimit();
+ return warningLimit.isEmpty() || warningLimit.get() == warn.get().getRatePerHour();
+ }
+ return true;
+ }
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiter.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiter.java
index 0e99d83..9285d22 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiter.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.ratelimiter;
+import com.google.common.annotations.VisibleForTesting;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.util.Optional;
@@ -29,9 +30,9 @@
private final Semaphore semaphore;
private final int maxPermits;
private final AtomicInteger usedPermits;
- private final ScheduledFuture<?> replenishTask;
private final String rateLimitType;
private final int timeLapse;
+ private ScheduledFuture<?> replenishTask;
interface Factory {
PeriodicRateLimiter create(
@@ -56,6 +57,11 @@
this::replenishPermits, timeLapse, timeLapse, TimeUnit.MINUTES);
}
+ @VisibleForTesting
+ void setReplenishTask(ScheduledFuture<?> replenishTask) {
+ this.replenishTask = replenishTask;
+ }
+
@Override
public int permitsPerHour() {
return maxPermits;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitUploadPack.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitUploadPack.java
index fddb2ec..0f93b0f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitUploadPack.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitUploadPack.java
@@ -21,6 +21,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.validators.UploadValidationListener;
+import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.validators.ValidationException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -42,16 +43,21 @@
private final Provider<CurrentUser> user;
private final LoadingCache<String, RateLimiter> uploadPackPerHour;
private final String limitExceededMsgFormat;
+ private final Module.RateLimiterLoader rateLimiterLoader;
+ private final Configuration configuration;
@Inject
RateLimitUploadPack(
Provider<CurrentUser> user,
@Named(UPLOAD_PACK_PER_HOUR) LoadingCache<String, RateLimiter> uploadPackPerHour,
- Configuration configuration) {
+ Configuration configuration,
+ Module.RateLimiterLoader rateLimiterLoader) {
this.user = user;
this.uploadPackPerHour = uploadPackPerHour;
limitExceededMsgFormat =
configuration.getRateLimitExceededMsg().replace(RATE_LIMIT_TOKEN, "{0,number,##.##}");
+ this.rateLimiterLoader = rateLimiterLoader;
+ this.configuration = configuration;
}
@Override
@@ -73,7 +79,7 @@
try {
RateLimiter limiter = uploadPackPerHour.get(key);
- if (limiter != null && !limiter.acquirePermit()) {
+ if (!limiter.acquirePermit()) {
throw new RateLimitException(
MessageFormat.format(limitExceededMsgFormat, limiter.permitsPerHour()));
}
@@ -82,6 +88,36 @@
}
}
+ void refresh(ProjectConfig newCfg, ProjectConfig oldCfg) {
+ configuration.refreshTable(newCfg, oldCfg);
+ refreshCache();
+ }
+
+ private void refreshCache() {
+ uploadPackPerHour
+ .asMap()
+ .keySet()
+ .parallelStream()
+ .filter(
+ key -> {
+ try {
+ return !rateLimiterLoader.isValidKey(key, uploadPackPerHour.get(key));
+ } catch (ExecutionException e) {
+ log.warn("Cannot get rate limits for {}: {}", key, e);
+ }
+ return false;
+ })
+ .forEach(
+ key -> {
+ try {
+ uploadPackPerHour.get(key).close();
+ uploadPackPerHour.invalidate(key);
+ } catch (ExecutionException e) {
+ throw new RuntimeException(e);
+ }
+ });
+ }
+
@Override
public void onPreUpload(
Repository repository,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimiterListener.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimiterListener.java
new file mode 100644
index 0000000..84dcd98
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimiterListener.java
@@ -0,0 +1,78 @@
+// Copyright (C) 2023 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.googlesource.gerrit.plugins.ratelimiter;
+
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.GerritIsReplica;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.project.ProjectConfig;
+import com.google.inject.Inject;
+import java.io.IOException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RateLimiterListener implements GitReferenceUpdatedListener {
+ private static final Logger log = LoggerFactory.getLogger(RateLimitUploadPack.class);
+ private final AllProjectsName allProjectsName;
+ private final boolean isReplica;
+ private final MetaDataUpdate.Server metaDataUpdateFactory;
+ private final ProjectConfig.Factory projectConfigFactory;
+ private final RateLimitUploadPack rateLimitUploadPack;
+
+ @Inject
+ public RateLimiterListener(
+ AllProjectsName allProjectsName,
+ @GerritIsReplica Boolean isReplica,
+ MetaDataUpdate.Server metaDataUpdateFactory,
+ ProjectConfig.Factory projectConfigFactory,
+ RateLimitUploadPack rateLimitUploadPack) {
+ this.allProjectsName = allProjectsName;
+ this.isReplica = isReplica;
+ this.metaDataUpdateFactory = metaDataUpdateFactory;
+ this.projectConfigFactory = projectConfigFactory;
+ this.rateLimitUploadPack = rateLimitUploadPack;
+ }
+
+ @Override
+ public void onGitReferenceUpdated(Event event) {
+ String projectName = event.getProjectName();
+ if (projectName.equals(allProjectsName.get())
+ && !isReplica
+ && event.getRefName().equals(RefNames.REFS_CONFIG)) {
+ Project.NameKey p = Project.nameKey(projectName);
+ try {
+ ProjectConfig newCfg = parseConfig(p, event.getNewObjectId());
+ ProjectConfig oldCfg = parseConfig(p, event.getOldObjectId());
+ rateLimitUploadPack.refresh(newCfg, oldCfg);
+ } catch (IOException | ConfigInvalidException eIo) {
+ log.warn("Failed to parse configuration");
+ }
+ }
+ }
+
+ private ProjectConfig parseConfig(Project.NameKey p, String idStr)
+ throws IOException, ConfigInvalidException {
+ ObjectId id = ObjectId.fromString(idStr);
+ if (ObjectId.zeroId().equals(id)) {
+ return null;
+ }
+ return projectConfigFactory.read(metaDataUpdateFactory.create(p), id);
+ }
+}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 12faa84..ee0144c 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -4,9 +4,20 @@
Rate Limits
-----------
-The defined rate limits and user groups for email notification are stored in a
-`rate-limiter.config` file in the `{review_site}/etc` directory. Rate limits are
-defined per user group and rate limit type.
+In case gerrit is not replica, the defined rate limits are stored in a
+`rate-limiter.config` file in the `{review_site}/etc` directory or in a
+`refs/meta/config` of `All-Projects`. In case `rate-limiter.config` is present
+in both `{review_site}/etc` and `refs/meta/config` of `All-Projects`, the
+configuration will be applied from `refs/meta/config` of `All-Projects`. If
+`rate-limiter.config` is invalid in `All-Projects` then `rate-limiter.config`
+will be applied from `{review_site}/etc`. Rate limits are defined per user group
+and rate limit type.
+If `rate-limiter.config` is missing from or corrupted in both
+`{review_site}/etc` and `refs/meta/config` then all users will have unlimited
+permits.
+
+In case gerrit is replica, the configuration will only load from be applied from
+`{review_site}/etc`.
Notification user groups are defined by setting 'recipients' in 'sendemail'
section. The 'recipients' property specifies user groups, which members will
@@ -140,3 +151,5 @@
`timelapseinminutes` defines a period of time in which the limit of
uploadpack takes place. If it is not configured, a default value of 1 hour
is established.
+
+If `rate-limiter.config` is changed in `All-Projects` then rate limit will reset for users.
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 9c08d11..7eed860 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/ConfigurationTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/ConfigurationTest.java
@@ -18,10 +18,16 @@
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static org.mockito.Mockito.when;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.CachedProjectConfig;
import com.google.gerrit.entities.GroupDescription;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.group.GroupResolver;
+import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.project.ProjectConfig;
import com.google.inject.ProvisionException;
import java.util.Map;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -35,19 +41,53 @@
@RunWith(MockitoJUnitRunner.class)
public class ConfigurationTest {
private static final String PLUGIN_NAME = "rate-limiter";
+ private static final String PROJECT_NAME = "All-Projects";
@Mock private PluginConfigFactory pluginConfigFactoryMock;
@Mock private GroupResolver groupsCollectionMock;
@Mock private GroupDescription.Basic administratorsGroupDescMock;
@Mock private GroupDescription.Basic someGroupDescMock;
-
+ @Mock private ProjectConfig newProjectConfig;
+ @Mock private ProjectConfig oldProjectConfig;
+ @Mock private CachedProjectConfig cachedProjectConfig;
+ @Mock private CachedProjectConfig oldCachedProjectConfig;
+ private AllProjectsName allProjectsName;
private Config globalPluginConfig;
+ private ImmutableMap<String, String> cacheableConfig;
+ private ImmutableMap<String, String> oldCacheableConfig;
private final int validRate = 123;
+ private final int validWarningRate = 50;
+ private final int newValidRate = 100;
+ private final int newValidWarningRate = 60;
+ private final int validTimeLapse = 10;
+ private final int newValidTimeLapse = 20;
private final String groupTagName = "group";
+ private final String newRateLimiterConfigInAllProject =
+ String.format(
+ "[%s \"someGroup\"]\n "
+ + "uploadpackperhour = %s\n "
+ + "uploadpackperhourwarn = %s\n "
+ + "timelapseinminutes = %s",
+ groupTagName, newValidRate, newValidWarningRate, newValidTimeLapse);
+ private final String oldRateLimiterConfigInAllProject =
+ String.format(
+ "[%s \"someGroup\"]\n "
+ + "uploadpackperhour = %s\n "
+ + "uploadpackperhourwarn = %s\n "
+ + "timelapseinminutes = %s",
+ groupTagName, validRate, validWarningRate, validTimeLapse);
+ private final String badConfiguration =
+ String.format(
+ "[%s \"someGroup\"\n "
+ + "uploadpackperhour = %s\n "
+ + "uploadpackperhourwarn = %s\n "
+ + "timelapseinminutes = %s",
+ groupTagName, newValidRate, newValidWarningRate, newValidTimeLapse);
@Before
public void setUp() {
globalPluginConfig = new Config();
+ allProjectsName = new AllProjectsName(PROJECT_NAME);
when(pluginConfigFactoryMock.getGlobalPluginConfig(PLUGIN_NAME)).thenReturn(globalPluginConfig);
@@ -57,15 +97,144 @@
when(someGroupDescMock.getName()).thenReturn("someGroup");
when(someGroupDescMock.getGroupUUID()).thenReturn(AccountGroup.uuid("some_uuid"));
when(groupsCollectionMock.parseId("someGroup")).thenReturn(someGroupDescMock);
+
+ when(newProjectConfig.getCacheable()).thenReturn(cachedProjectConfig);
+ when(oldProjectConfig.getCacheable()).thenReturn(oldCachedProjectConfig);
}
- private Configuration getConfiguration() {
- return new Configuration(pluginConfigFactoryMock, PLUGIN_NAME, groupsCollectionMock);
+ @Test
+ public void testConfigInNonReplca() throws NoSuchProjectException {
+ assertThat(
+ getUploadPackLimits(false)
+ .getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR)
+ .get(AccountGroup.uuid("some_uuid"))
+ .getRatePerHour())
+ .isEqualTo(newValidRate);
+ }
+
+ @Test
+ public void testConfigInReplca() throws NoSuchProjectException {
+ assertThat(
+ getUploadPackLimits(true)
+ .getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR)
+ .get(AccountGroup.uuid("some_uuid"))
+ .getRatePerHour())
+ .isEqualTo(validRate);
}
@Test
public void testEmptyConfig() {
- assertThat(getConfiguration().getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR)).isEmpty();
+ assertThat(getConfiguration(false).getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR)).isEmpty();
+ }
+
+ @Test
+ public void testConfigtFromAllProjectConfig() {
+ // Config in table
+ globalPluginConfig.setInt(
+ groupTagName,
+ someGroupDescMock.getName(),
+ RateLimitType.UPLOAD_PACK_PER_HOUR.toString(),
+ validRate);
+ globalPluginConfig.setInt(
+ groupTagName,
+ someGroupDescMock.getName(),
+ RateLimitType.UPLOAD_PACK_PER_HOUR_WARN.toString(),
+ validWarningRate);
+ globalPluginConfig.setInt(
+ groupTagName,
+ someGroupDescMock.getName(),
+ RateLimitType.TIME_LAPSE_IN_MINUTES.toString(),
+ validTimeLapse);
+
+ oldCacheableConfig =
+ ImmutableMap.<String, String>builder()
+ .put("rate-limiter.config", oldRateLimiterConfigInAllProject)
+ .build();
+ ;
+ cacheableConfig =
+ ImmutableMap.<String, String>builder()
+ .put("rate-limiter.config", newRateLimiterConfigInAllProject)
+ .build();
+
+ when(oldCachedProjectConfig.getProjectLevelConfigs()).thenReturn(oldCacheableConfig);
+ when(cachedProjectConfig.getProjectLevelConfigs()).thenReturn(cacheableConfig);
+
+ Configuration configuration = getConfiguration(false);
+
+ verifyConfiginTable(configuration, validRate, validWarningRate, validTimeLapse);
+ configuration.refreshTable(
+ newProjectConfig,
+ oldProjectConfig); // Change config in table base in config in all-projects
+ verifyConfiginTable(configuration, newValidRate, newValidWarningRate, newValidTimeLapse);
+ }
+
+ @Test
+ public void testInvalidConfigInAllProjects() {
+ globalPluginConfig.setInt(
+ groupTagName,
+ someGroupDescMock.getName(),
+ RateLimitType.UPLOAD_PACK_PER_HOUR.toString(),
+ validRate);
+ globalPluginConfig.setInt(
+ groupTagName,
+ someGroupDescMock.getName(),
+ RateLimitType.UPLOAD_PACK_PER_HOUR_WARN.toString(),
+ validWarningRate);
+ globalPluginConfig.setInt(
+ groupTagName,
+ someGroupDescMock.getName(),
+ RateLimitType.TIME_LAPSE_IN_MINUTES.toString(),
+ validTimeLapse);
+
+ oldCacheableConfig =
+ ImmutableMap.<String, String>builder()
+ .put("rate-limiter.config", oldRateLimiterConfigInAllProject)
+ .build();
+ cacheableConfig =
+ ImmutableMap.<String, String>builder().put("rate-limiter.config", badConfiguration).build();
+
+ when(oldCachedProjectConfig.getProjectLevelConfigs()).thenReturn(oldCacheableConfig);
+ when(cachedProjectConfig.getProjectLevelConfigs()).thenReturn(cacheableConfig);
+
+ Configuration configuration = getConfiguration(false);
+
+ verifyConfiginTable(configuration, validRate, validWarningRate, validTimeLapse);
+ configuration.refreshTable(newProjectConfig, oldProjectConfig);
+ verifyConfiginTable(configuration, validRate, validWarningRate, validTimeLapse);
+ }
+
+ @Test
+ public void testNoConfiginAllProjects() {
+ globalPluginConfig.setInt(
+ groupTagName,
+ someGroupDescMock.getName(),
+ RateLimitType.UPLOAD_PACK_PER_HOUR.toString(),
+ validRate);
+ globalPluginConfig.setInt(
+ groupTagName,
+ someGroupDescMock.getName(),
+ RateLimitType.UPLOAD_PACK_PER_HOUR_WARN.toString(),
+ validWarningRate);
+ globalPluginConfig.setInt(
+ groupTagName,
+ someGroupDescMock.getName(),
+ RateLimitType.TIME_LAPSE_IN_MINUTES.toString(),
+ validTimeLapse);
+
+ oldCacheableConfig =
+ ImmutableMap.<String, String>builder()
+ .put("rate-limiter.config", oldRateLimiterConfigInAllProject)
+ .build();
+ cacheableConfig = ImmutableMap.<String, String>builder().build();
+
+ when(cachedProjectConfig.getProjectLevelConfigs()).thenReturn(cacheableConfig);
+ when(oldCachedProjectConfig.getProjectLevelConfigs()).thenReturn(oldCacheableConfig);
+
+ Configuration configuration = getConfiguration(false);
+
+ verifyConfiginTable(configuration, validRate, validWarningRate, validTimeLapse);
+ configuration.refreshTable(newProjectConfig, oldProjectConfig);
+ verifyConfiginTable(configuration, validRate, validWarningRate, validTimeLapse);
}
@Test
@@ -77,7 +246,7 @@
validRate);
Map<AccountGroup.UUID, RateLimit> rateLimit =
- getConfiguration().getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR);
+ getConfiguration(false).getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR);
assertThat(rateLimit).hasSize(1);
assertThat(rateLimit.get(someGroupDescMock.getGroupUUID()).getRatePerHour())
.isEqualTo(validRate);
@@ -88,7 +257,8 @@
globalPluginConfig.setInt(
groupTagName, someGroupDescMock.getName(), "invalidTypePerHour", validRate);
- ProvisionException thrown = assertThrows(ProvisionException.class, () -> getConfiguration());
+ ProvisionException thrown =
+ assertThrows(ProvisionException.class, () -> getConfiguration(false));
assertThat(thrown)
.hasMessageThat()
.contains("Invalid configuration, unsupported rate limit type: invalidTypePerHour");
@@ -108,7 +278,8 @@
String.format(
"Invalid configuration, 'rate limit value '%s' for 'group.someGroup.uploadpackperhour' is not a valid number",
invalidType);
- ProvisionException thrown = assertThrows(ProvisionException.class, () -> getConfiguration());
+ ProvisionException thrown =
+ assertThrows(ProvisionException.class, () -> getConfiguration(false));
assertThat(thrown).hasMessageThat().contains(expectedMessage);
}
@@ -129,7 +300,7 @@
"badGroup");
Map<AccountGroup.UUID, RateLimit> rateLimit =
- getConfiguration().getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR);
+ getConfiguration(false).getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR);
assertThat(rateLimit).hasSize(1);
assertThat(rateLimit.get(someGroupDescMock.getGroupUUID()).getRatePerHour())
.isEqualTo(validRate);
@@ -140,14 +311,14 @@
globalPluginConfig.fromText("[group \"Administrators\"]");
Map<AccountGroup.UUID, RateLimit> rateLimit =
- getConfiguration().getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR);
+ getConfiguration(false).getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR);
assertThat(rateLimit).hasSize(1);
assertThat(rateLimit.get(administratorsGroupDescMock.getGroupUUID())).isNull();
}
@Test
public void testDefaultRateLimitExceededMsg() {
- assertThat(getConfiguration().getRateLimitExceededMsg())
+ assertThat(getConfiguration(false).getRateLimitExceededMsg())
.isEqualTo("Exceeded rate limit of ${rateLimit} fetch requests/hour");
}
@@ -155,6 +326,52 @@
public void testRateLimitExceededMsg() {
String msg = "Some error message.";
globalPluginConfig.setString("configuration", null, "uploadpackLimitExceededMsg", msg);
- assertThat(getConfiguration().getRateLimitExceededMsg()).isEqualTo(msg);
+ assertThat(getConfiguration(false).getRateLimitExceededMsg()).isEqualTo(msg);
+ }
+
+ private Configuration getConfiguration(Boolean isReplica) {
+ return new Configuration(
+ allProjectsName, pluginConfigFactoryMock, PLUGIN_NAME, isReplica, groupsCollectionMock);
+ }
+
+ private void verifyConfiginTable(
+ Configuration configuration, int rateToCheck, int warningToCheck, int timeLapseToCheck) {
+ Map<AccountGroup.UUID, RateLimit> rateLimit =
+ configuration.getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR);
+ assertThat(rateLimit).hasSize(1);
+ assertThat(rateLimit.get(someGroupDescMock.getGroupUUID()).getRatePerHour())
+ .isEqualTo(rateToCheck);
+ Map<AccountGroup.UUID, RateLimit> warningRate =
+ configuration.getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR_WARN);
+ assertThat(warningRate).hasSize(1);
+ assertThat(warningRate.get(someGroupDescMock.getGroupUUID()).getRatePerHour())
+ .isEqualTo(warningToCheck);
+ Map<AccountGroup.UUID, RateLimit> timeLapse =
+ configuration.getRateLimits(RateLimitType.TIME_LAPSE_IN_MINUTES);
+ assertThat(timeLapse).hasSize(1);
+ assertThat(timeLapse.get(someGroupDescMock.getGroupUUID()).getRatePerHour())
+ .isEqualTo(timeLapseToCheck);
+ }
+
+ public Configuration getUploadPackLimits(Boolean isReplica) throws NoSuchProjectException {
+ Config configInAllProjects = new Config();
+ // Config in All-Project
+ configInAllProjects.setInt(
+ groupTagName,
+ someGroupDescMock.getName(),
+ RateLimitType.UPLOAD_PACK_PER_HOUR.toString(),
+ newValidRate);
+ // Config in etc/rate-limiter
+ globalPluginConfig.setInt(
+ groupTagName,
+ someGroupDescMock.getName(),
+ RateLimitType.UPLOAD_PACK_PER_HOUR.toString(),
+ validRate);
+
+ when(pluginConfigFactoryMock.getProjectPluginConfigWithInheritance(
+ Project.NameKey.parse(PROJECT_NAME), PLUGIN_NAME))
+ .thenReturn(configInAllProjects);
+
+ return getConfiguration(isReplica);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimiterListenerTest.java b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimiterListenerTest.java
new file mode 100644
index 0000000..edfdcfb
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimiterListenerTest.java
@@ -0,0 +1,120 @@
+// Copyright (C) 2023 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.googlesource.gerrit.plugins.ratelimiter;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.api.changes.NotifyHandling;
+import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.events.GitReferenceUpdatedListener.Event;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.project.ProjectConfig;
+import java.io.IOException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class RateLimiterListenerTest {
+ @Mock private ProjectConfig.Factory projectConfigFactoryMock;
+ @Mock private MetaDataUpdate.Server metaDataUpdateFactorymock;
+ @Mock private RateLimitUploadPack rateLimitUploadPack;
+ private AllProjectsName allProjectsName;
+ private RateLimiterListener rateLimiterListener;
+ private final Boolean isReplica = false;
+ private final String someId = "0070000700007000070000700007000070000700";
+ private final String ALL_PROJECTS = "All-Projects";
+ private final String SOME_PROJECT = "Something";
+
+ @Before
+ public void setUp() throws ConfigInvalidException, IOException {
+ allProjectsName = new AllProjectsName("All-Projects");
+ rateLimiterListener =
+ new RateLimiterListener(
+ allProjectsName,
+ isReplica,
+ metaDataUpdateFactorymock,
+ projectConfigFactoryMock,
+ rateLimitUploadPack);
+ }
+
+ private Event configChangeEvent(String projectName) {
+ return new Event() {
+ @Override
+ public String getRefName() {
+ return RefNames.REFS_CONFIG;
+ }
+
+ @Override
+ public String getOldObjectId() {
+ return someId;
+ }
+
+ @Override
+ public String getNewObjectId() {
+ return someId;
+ }
+
+ @Override
+ public boolean isCreate() {
+ return false;
+ }
+
+ @Override
+ public boolean isDelete() {
+ return false;
+ }
+
+ @Override
+ public boolean isNonFastForward() {
+ return false;
+ }
+
+ @Override
+ public AccountInfo getUpdater() {
+ return null;
+ }
+
+ @Override
+ public String getProjectName() {
+ return projectName;
+ }
+
+ @Override
+ public NotifyHandling getNotify() {
+ return null;
+ }
+ };
+ }
+
+ @Test
+ public void shouldTriggerRefresh() {
+ rateLimiterListener.onGitReferenceUpdated(configChangeEvent(ALL_PROJECTS));
+ verify(rateLimitUploadPack).refresh(any(), any());
+ }
+
+ @Test
+ public void shouldNotTriggerRefresh() {
+ rateLimiterListener.onGitReferenceUpdated(configChangeEvent(SOME_PROJECT));
+ verify(rateLimitUploadPack, never()).refresh(any(), any());
+ }
+}
