Merge branch 'stable-3.3'
* stable-3.3:
Add the configuration for the merge changes grace time
Introduce change-ts cache for showing changes just merged
Extract the change cache key to a separate class
Align open changes cache class name with its name
Introduce cache for open changes
Reduce spamming by lowering to debug the invalid changes
Change-Id: I4c5ebf3a3d854cc25bdb91115b751433443655b6
diff --git a/README.md b/README.md
index c172399..e0d246a 100644
--- a/README.md
+++ b/README.md
@@ -27,6 +27,13 @@
git-refs-filter is slightly slower than using Git's hideRefs and it does require the configuration
of the change_notes cache in `gerrit.config` to avoid potentially high overhead.
+Additionally, this plugin uses an in-memory cache to store previously computed
+open/close change statuses to avoid processing them over and over again.
+
+Explicit invalidation of such cache is not necessary, since the change revision
+is part of the cache key, so that previous entries automatically become obsolete
+once a change status is updated.
+
### Gerrit ACLs
Use the Gerrit ACLs when you need to hide some of the refs on a per-project basis or when
@@ -63,9 +70,9 @@
has no side effects.
Filtering a closed change refs has the following meaning:
-- Merged changes and all their patch-sets
-- Abandoned changes and all their patch-sets
-- Corrupted changes and all their patch-sets
+- Merged changes and all their patch-sets older than the [grace time](#grace-time-for-closed-changes)
+- Abandoned changes and all their patch-sets older than the [grace time](#grace-time-for-closed-changes)
+- Corrupted changes and all their patch-sets older than the [grace time](#grace-time-for-closed-changes)
- All '/meta' refs of all changes
- All non-published edits of any changes
@@ -93,3 +100,18 @@
a READ rule to refs/*). To enable the closed changes filtering you need to disable any global read rule
for the group that needs refs filtering.
+### Grace time for closed changes
+
+The refsfilter allows to define `git-refs-filter: grace time [sec] for closed changes`
+project configuration parameter. This parameter controls the size of the grace
+time window in seconds. All closed changes newer than the grace time will not
+be filtered out. Value can be defined per project or can be inherited from its parents.
+
+Default value: 86400
+
+Example of setting the grace time parameter in `project.config`:
+
+```
+[plugin "gerrit"]
+ gitRefFilterClosedChangesGraceTimeSec = 3600
+```
\ No newline at end of file
diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangeCacheKey.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangeCacheKey.java
new file mode 100644
index 0000000..9a6b0d6
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangeCacheKey.java
@@ -0,0 +1,44 @@
+// Copyright (C) 2022 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.modules.gitrefsfilter;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.Project;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
+
+@AutoValue
+public abstract class ChangeCacheKey {
+ /* `repo` and `changeId` need to be part of the cache key because the
+ Loader requires them in order to fetch the relevant changeNote. */
+ public abstract Repository repo();
+
+ public abstract Change.Id changeId();
+
+ @Nullable
+ public abstract ObjectId changeRevision();
+
+ public abstract Project.NameKey project();
+
+ static ChangeCacheKey create(
+ Repository repo,
+ Change.Id changeId,
+ @Nullable ObjectId changeRevision,
+ Project.NameKey project) {
+ return new AutoValue_ChangeCacheKey(repo, changeId, changeRevision, project);
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangesTsCache.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangesTsCache.java
new file mode 100644
index 0000000..cfd7a0f
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangesTsCache.java
@@ -0,0 +1,64 @@
+// Copyright (C) 2022 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.modules.gitrefsfilter;
+
+import com.google.common.cache.CacheLoader;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Singleton;
+import com.google.inject.TypeLiteral;
+
+public class ChangesTsCache {
+ public static final String CHANGES_CACHE_TS = "changes_ts";
+
+ public static Module module() {
+ return new CacheModule() {
+ @Override
+ protected void configure() {
+ cache(CHANGES_CACHE_TS, ChangeCacheKey.class, new TypeLiteral<Long>() {})
+ .loader(Loader.class);
+ }
+ };
+ }
+
+ @Singleton
+ static class Loader extends CacheLoader<ChangeCacheKey, Long> {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final ChangeNotes.Factory changeNotesFactory;
+
+ @Inject
+ Loader(ChangeNotes.Factory changeNotesFactory) {
+ this.changeNotesFactory = changeNotesFactory;
+ }
+
+ @Override
+ public Long load(ChangeCacheKey key) throws Exception {
+ try {
+ return changeNotesFactory
+ .createChecked(key.repo(), key.project(), key.changeId(), key.changeRevision())
+ .getChange()
+ .getLastUpdatedOn()
+ .getTime();
+ } catch (NoSuchChangeException e) {
+ logger.atFine().withCause(e).log(
+ "Change %d does not exist: returning zero epoch", key.changeId());
+ return 0L;
+ }
+ }
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/FilterRefsConfig.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/FilterRefsConfig.java
index 2524f5e..1b29094 100644
--- a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/FilterRefsConfig.java
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/FilterRefsConfig.java
@@ -15,10 +15,14 @@
package com.googlesource.gerrit.modules.gitrefsfilter;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.PluginConfigFactory;
+import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.inject.Inject;
import java.util.Arrays;
import java.util.List;
+import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Ref;
@@ -26,13 +30,20 @@
public class FilterRefsConfig {
public static final String SECTION_GIT_REFS_FILTER = "git-refs-filter";
public static final String KEY_HIDE_REFS = "hideRefs";
+ public static final String PROJECT_CONFIG_CLOSED_CHANGES_GRACE_TIME_SEC =
+ "gitRefFilterClosedChangesGraceTimeSec";
+
+ static final long CLOSED_CHANGES_GRACE_TIME_SEC_DEFAULT =
+ TimeUnit.SECONDS.convert(24, TimeUnit.HOURS);
private final List<String> hideRefs;
private final List<String> showRefs;
+ private PluginConfigFactory cfgFactory;
@Inject
- public FilterRefsConfig(@GerritServerConfig Config gerritConfig) {
+ public FilterRefsConfig(@GerritServerConfig Config gerritConfig, PluginConfigFactory cfgFactory) {
+ this.cfgFactory = cfgFactory;
List<String> hideRefsConfig =
Arrays.asList(gerritConfig.getStringList(SECTION_GIT_REFS_FILTER, null, KEY_HIDE_REFS));
@@ -68,4 +79,13 @@
return true;
}
+
+ /** performance warning: this call can be expensive, please reuse the value */
+ public long getClosedChangeGraceTimeSec(Project.NameKey projectKey)
+ throws NoSuchProjectException {
+ return cfgFactory
+ .getFromProjectConfigWithInheritance(projectKey, "gerrit")
+ .getLong(
+ PROJECT_CONFIG_CLOSED_CHANGES_GRACE_TIME_SEC, CLOSED_CHANGES_GRACE_TIME_SEC_DEFAULT);
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java
index 3080694..5347f2d 100644
--- a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java
@@ -14,24 +14,33 @@
package com.googlesource.gerrit.modules.gitrefsfilter;
+import static com.googlesource.gerrit.modules.gitrefsfilter.ChangesTsCache.CHANGES_CACHE_TS;
+import static com.googlesource.gerrit.modules.gitrefsfilter.OpenChangesCache.OPEN_CHANGES_CACHE;
+
+import com.google.common.cache.LoadingCache;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.access.CoreOrPluginProjectPermission;
import com.google.gerrit.extensions.conditions.BooleanCondition;
import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend.ForProject;
import com.google.gerrit.server.permissions.PermissionBackend.ForRef;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import com.google.inject.name.Named;
+import java.sql.Timestamp;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
import java.util.Collection;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ExecutionException;
import java.util.stream.Collectors;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
@@ -40,10 +49,12 @@
public class ForProjectWrapper extends ForProject {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final LoadingCache<ChangeCacheKey, Boolean> openChangesCache;
+ private final LoadingCache<ChangeCacheKey, Long> changesTsCache;
private final ForProject defaultForProject;
private final Project.NameKey project;
- private final ChangeNotes.Factory changeNotesFactory;
private final FilterRefsConfig config;
+ private long closedChangesGraceTime;
public interface Factory {
ForProjectWrapper get(ForProject defaultForProject, Project.NameKey project);
@@ -51,14 +62,18 @@
@Inject
public ForProjectWrapper(
- ChangeNotes.Factory changeNotesFactory,
FilterRefsConfig config,
+ @Named(OPEN_CHANGES_CACHE) LoadingCache<ChangeCacheKey, Boolean> openChangesCache,
+ @Named(CHANGES_CACHE_TS) LoadingCache<ChangeCacheKey, Long> changesTsCache,
@Assisted ForProject defaultForProject,
- @Assisted Project.NameKey project) {
+ @Assisted Project.NameKey project)
+ throws NoSuchProjectException {
+ this.openChangesCache = openChangesCache;
+ this.changesTsCache = changesTsCache;
this.defaultForProject = defaultForProject;
this.project = project;
- this.changeNotesFactory = changeNotesFactory;
this.config = config;
+ this.closedChangesGraceTime = config.getClosedChangeGraceTimeSec(project);
}
@Override
@@ -98,7 +113,8 @@
return (!isChangeRef(refName)
|| (!isChangeMetaRef(refName)
&& changeId != null
- && isOpen(repo, changeId, changeRevisions.get(changeId))));
+ && (isOpen(repo, changeId, changeRevisions.get(changeId))
+ || isRecent(repo, changeId, changeRevisions.get(changeId)))));
})
.collect(Collectors.toList());
}
@@ -115,15 +131,32 @@
return isChangeRef(changeKey) && changeKey.endsWith("/meta");
}
- private boolean isOpen(Repository repo, Change.Id changeId, ObjectId changeRevision) {
+ private boolean isOpen(Repository repo, Change.Id changeId, @Nullable ObjectId changeRevision) {
try {
- ChangeNotes changeNotes =
- changeNotesFactory.createChecked(repo, project, changeId, changeRevision);
- return changeNotes.getChange().getStatus().isOpen();
- } catch (NoSuchChangeException e) {
+ return openChangesCache.get(ChangeCacheKey.create(repo, changeId, changeRevision, project));
+ } catch (ExecutionException e) {
logger.atWarning().withCause(e).log(
- "Change %d does not exist: hiding from the advertised refs", changeId);
- return false;
+ "Error getting change '%d' from the cache. Do not hide from the advertised refs",
+ changeId);
+ return true;
+ }
+ }
+
+ private boolean isRecent(Repository repo, Change.Id changeId, @Nullable ObjectId changeRevision) {
+ try {
+ Timestamp cutOffTs =
+ Timestamp.from(
+ Instant.now().truncatedTo(ChronoUnit.SECONDS).minusSeconds(closedChangesGraceTime));
+ return changesTsCache
+ .get(ChangeCacheKey.create(repo, changeId, changeRevision, project))
+ .longValue()
+ >= cutOffTs.getTime();
+
+ } catch (ExecutionException e) {
+ logger.atWarning().withCause(e).log(
+ "Error getting change '%d' from the cache. Do not hide from the advertised refs",
+ changeId);
+ return true;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java
new file mode 100644
index 0000000..8e1c371
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java
@@ -0,0 +1,63 @@
+// Copyright (C) 2022 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.modules.gitrefsfilter;
+
+import com.google.common.cache.CacheLoader;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Singleton;
+import com.google.inject.TypeLiteral;
+
+public class OpenChangesCache {
+ public static final String OPEN_CHANGES_CACHE = "open_changes";
+
+ public static Module module() {
+ return new CacheModule() {
+ @Override
+ protected void configure() {
+ cache(OPEN_CHANGES_CACHE, ChangeCacheKey.class, new TypeLiteral<Boolean>() {})
+ .loader(Loader.class);
+ }
+ };
+ }
+
+ @Singleton
+ static class Loader extends CacheLoader<ChangeCacheKey, Boolean> {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final ChangeNotes.Factory changeNotesFactory;
+
+ @Inject
+ Loader(ChangeNotes.Factory changeNotesFactory) {
+ this.changeNotesFactory = changeNotesFactory;
+ }
+
+ @Override
+ public Boolean load(ChangeCacheKey key) throws Exception {
+ try {
+ ChangeNotes changeNotes =
+ changeNotesFactory.createChecked(
+ key.repo(), key.project(), key.changeId(), key.changeRevision());
+ return changeNotes.getChange().getStatus().isOpen();
+ } catch (NoSuchChangeException e) {
+ logger.atFine().withCause(e).log(
+ "Change %d does not exist: hiding from the advertised refs", key.changeId());
+ return false;
+ }
+ }
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/RefsFilterModule.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/RefsFilterModule.java
index 98ce862..8b882c9 100644
--- a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/RefsFilterModule.java
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/RefsFilterModule.java
@@ -16,6 +16,7 @@
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.config.CapabilityDefinition;
+import com.google.gerrit.server.config.ProjectConfigEntry;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.inject.AbstractModule;
import com.google.inject.Scopes;
@@ -25,6 +26,8 @@
@Override
protected void configure() {
+ bind(FilterRefsConfig.class).in(Scopes.SINGLETON);
+
install(
new FactoryModuleBuilder()
.implement(WithUserWrapper.class, WithUserWrapper.class)
@@ -41,5 +44,17 @@
.annotatedWith(Exports.named(FilterRefsCapability.HIDE_CLOSED_CHANGES_REFS))
.to(FilterRefsCapability.class)
.in(Scopes.SINGLETON);
+
+ bind(ProjectConfigEntry.class)
+ .annotatedWith(Exports.named(FilterRefsConfig.PROJECT_CONFIG_CLOSED_CHANGES_GRACE_TIME_SEC))
+ .toInstance(
+ new ProjectConfigEntry(
+ "git-refs-filter: grace time [sec] for closed changes",
+ FilterRefsConfig.CLOSED_CHANGES_GRACE_TIME_SEC_DEFAULT,
+ true,
+ "Grace time for keeping closed changes from filtering by the git-refs-filter"));
+
+ install(OpenChangesCache.module());
+ install(ChangesTsCache.module());
}
}
diff --git a/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/AbstractGitDaemonTest.java b/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/AbstractGitDaemonTest.java
index ea1d61e..c58f0b8 100644
--- a/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/AbstractGitDaemonTest.java
+++ b/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/AbstractGitDaemonTest.java
@@ -43,11 +43,12 @@
@Inject private RequestScopeOperations requestScopeOperations;
@Inject private ProjectOperations projectOperations;
- protected void createChangeAndAbandon() throws Exception, RestApiException {
+ protected int createChangeAndAbandon() throws Exception, RestApiException {
requestScopeOperations.setApiUser(admin.id());
createChange();
int changeNum = changeNumOfRef(getChangesRefsAs(admin).get(0));
gApi.changes().id(changeNum).abandon();
+ return changeNum;
}
protected void createFilteredRefsGroup() throws Exception {
diff --git a/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterTest.java b/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterTest.java
index 957d4f7..1fd85c1 100644
--- a/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterTest.java
+++ b/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterTest.java
@@ -15,7 +15,11 @@
package com.googlesource.gerrit.libmodule.plugins.test;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.WaitUtil.waitUntil;
+import static com.googlesource.gerrit.modules.gitrefsfilter.ChangesTsCache.CHANGES_CACHE_TS;
+import static com.googlesource.gerrit.modules.gitrefsfilter.OpenChangesCache.OPEN_CHANGES_CACHE;
+import com.google.common.cache.LoadingCache;
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.Sandboxed;
@@ -23,12 +27,24 @@
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.project.ProjectConfig;
import com.google.inject.Inject;
import com.google.inject.Module;
+import com.google.inject.name.Named;
+import com.googlesource.gerrit.modules.gitrefsfilter.ChangeCacheKey;
+import com.googlesource.gerrit.modules.gitrefsfilter.FilterRefsConfig;
import com.googlesource.gerrit.modules.gitrefsfilter.RefsFilterModule;
import java.io.IOException;
+import java.sql.Timestamp;
+import java.time.Duration;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.ArrayList;
import java.util.List;
+import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
@@ -36,6 +52,7 @@
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.FetchResult;
import org.eclipse.jgit.util.FS;
import org.junit.Before;
@@ -46,6 +63,19 @@
public class GitRefsFilterTest extends AbstractGitDaemonTest {
@Inject private RequestScopeOperations requestScopeOperations;
+ @Inject
+ private @Named(OPEN_CHANGES_CACHE) LoadingCache<ChangeCacheKey, Boolean> changeOpenCache;
+
+ @Inject
+ private @Named(CHANGES_CACHE_TS) LoadingCache<ChangeCacheKey, Long> changesTsCache;
+
+ private static final int CLOSED_CHANGES_GRACE_TIME_SEC = 5;
+
+ private static final Duration TEST_PATIENCE_TIME =
+ Duration.ofSeconds(CLOSED_CHANGES_GRACE_TIME_SEC + 1);
+
+ private volatile Exception getRefsException = null;
+
@Override
public Module createModule() {
return new RefsFilterModule();
@@ -54,13 +84,41 @@
@Before
public void setup() throws Exception {
createFilteredRefsGroup();
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
+ ProjectConfig projectConfig = projectConfigFactory.create(project);
+ projectConfig.load(md);
+ projectConfig.updatePluginConfig(
+ "gerrit",
+ cfg ->
+ cfg.setLong(
+ FilterRefsConfig.PROJECT_CONFIG_CLOSED_CHANGES_GRACE_TIME_SEC,
+ CLOSED_CHANGES_GRACE_TIME_SEC));
+ projectConfig.commit(md);
+ projectCache.evict(project);
+ }
}
@Test
public void testUserWithFilterOutCapabilityShouldNotSeeAbandonedChangesRefs() throws Exception {
+ Timestamp changeTs = gApi.changes().id(createChangeAndAbandon()).get().updated;
+
+ waitUntil(() -> getRefsUnchecked(user).isEmpty(), TEST_PATIENCE_TIME);
+ checkGetRefsIsSuccessful();
+
+ Timestamp filterCutoffTs =
+ Timestamp.from(
+ Instant.now()
+ .truncatedTo(ChronoUnit.SECONDS)
+ .minusSeconds(CLOSED_CHANGES_GRACE_TIME_SEC));
+
+ assertThat(changeTs.before(filterCutoffTs)).isTrue();
+ }
+
+ @Test
+ public void testUserWithFilterOutCapabilityShouldSeeJustClosedChangesRefs() throws Exception {
createChangeAndAbandon();
- assertThat(getRefs(cloneProjectChangesRefs(user))).isEmpty();
+ assertThat(getRefs(cloneProjectChangesRefs(user))).isNotEmpty();
}
@Test
@@ -84,9 +142,11 @@
}
@Test
- public void testAdminUserShouldSeeAbandonedChangesRefs() throws Exception {
+ public void testAdminUserShouldSeeAbandonedChangesRefsAfterGracePeriod() throws Exception {
createChangeAndAbandon();
+ waitUntil(() -> getRefsUnchecked(user).isEmpty(), TEST_PATIENCE_TIME);
+
assertThat(getRefs(cloneProjectChangesRefs(admin))).isNotEmpty();
}
@@ -116,6 +176,78 @@
.isNotEmpty();
}
+ @Test
+ public void testShouldCacheChangeIsClosedWhenAbandoned() throws Exception {
+ Change.Id changeId = Change.id(createChangeAndAbandon());
+ Ref metaRef = getMetaId(changeId);
+
+ getRefs(cloneProjectChangesRefs(user));
+
+ assertThat(changeOpenCache.asMap().size()).isEqualTo(1);
+
+ Map.Entry<ChangeCacheKey, Boolean> cacheEntry =
+ new ArrayList<>(changeOpenCache.asMap().entrySet()).get(0);
+
+ assertThat(cacheEntry.getKey().project()).isEqualTo(project);
+ assertThat(cacheEntry.getKey().changeId()).isEqualTo(changeId);
+ assertThat(cacheEntry.getKey().changeRevision()).isEqualTo(metaRef.getObjectId());
+ assertThat(cacheEntry.getValue()).isFalse();
+ }
+
+ @Test
+ public void testShouldCacheWhenChangeIsOpen() throws Exception {
+ createChange();
+ List<Ref> refs = getRefs(cloneProjectChangesRefs(user));
+
+ assertThat(refs).isNotEmpty();
+
+ Change.Id changeId = Change.id(changeNumOfRef(refs.get(0)));
+
+ assertThat(changeOpenCache.asMap().size()).isEqualTo(1);
+
+ Map.Entry<ChangeCacheKey, Boolean> cacheEntry =
+ new ArrayList<>(changeOpenCache.asMap().entrySet()).get(0);
+
+ assertThat(cacheEntry.getKey().project()).isEqualTo(project);
+ assertThat(cacheEntry.getKey().changeId()).isEqualTo(changeId);
+ assertThat(cacheEntry.getKey().changeRevision()).isEqualTo(getMetaId(changeId).getObjectId());
+ assertThat(cacheEntry.getValue()).isTrue();
+ }
+
+ @Test
+ public void testShouldCacheChangeTsWhenAbandoned() throws Exception {
+ Change.Id changeId = Change.id(createChangeAndAbandon());
+ Ref metaRef = getMetaId(changeId);
+
+ getRefs(cloneProjectChangesRefs(user));
+
+ assertThat(changesTsCache.asMap().size()).isEqualTo(1);
+
+ Map.Entry<ChangeCacheKey, Long> cacheEntry =
+ new ArrayList<>(changesTsCache.asMap().entrySet()).get(0);
+
+ assertThat(cacheEntry.getKey().project()).isEqualTo(project);
+ assertThat(cacheEntry.getKey().changeId()).isEqualTo(changeId);
+ assertThat(cacheEntry.getKey().changeRevision()).isEqualTo(metaRef.getObjectId());
+ assertThat(cacheEntry.getValue())
+ .isEqualTo(gApi.changes().id(changeId.get()).get().updated.getTime());
+ }
+
+ private List<Ref> getRefsUnchecked(TestAccount user) {
+ try {
+ return super.getRefs(cloneProjectChangesRefs(user));
+ } catch (Exception e) {
+ getRefsException = e;
+ return new ArrayList<>();
+ }
+ }
+
+ private void checkGetRefsIsSuccessful() throws Exception {
+ if (getRefsException != null) {
+ throw getRefsException;
+ }
+ }
+
protected Stream<String> fetchAllRefs(TestAccount testAccount) throws Exception {
DfsRepositoryDescription desc = new DfsRepositoryDescription("clone of " + project.get());
@@ -137,4 +269,10 @@
throws IOException {
return repo.getRepository().getRefDatabase().getRefsByPrefix(prefix);
}
+
+ private Ref getMetaId(Change.Id changeId) throws Exception {
+ try (Repository r = repoManager.openRepository(project)) {
+ return r.exactRef(RefNames.changeMetaRef(changeId));
+ }
+ }
}
