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)); + } + } }