Merge branch 'stable-3.5'
* stable-3.5:
Fetch change version from refdb on Git protocol v2
Do not cache negative results from open changes / timestamp lookups
Fix 'illegal format conversion' compilation error
follow-up to "Exclude repo from ChangeCacheKey..."
Exclude repo from ChangeCacheKey equals/hash code calculation
Change-Id: If2e339ebb82cfb850ce98431e4324449c706b185
diff --git a/BUILD b/BUILD
index b20f6fb..52ef2e5 100644
--- a/BUILD
+++ b/BUILD
@@ -15,7 +15,10 @@
junit_tests(
name = "git_refs_filter_tests",
srcs = glob(
- ["src/test/java/**/*Test.java"],
+ [
+ "src/test/java/**/*Test.java",
+ "src/test/java/**/*IT.java",
+ ],
exclude = ["src/test/java/**/Abstract*.java"],
),
visibility = ["//visibility:public"],
diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangeCacheKey.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangeCacheKey.java
index 9a6b0d6..291ec65 100644
--- a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangeCacheKey.java
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangeCacheKey.java
@@ -18,6 +18,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
+import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
@@ -25,7 +26,11 @@
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();
+ private final AtomicReference<Repository> repo = new AtomicReference<>();
+
+ public final Repository repo() {
+ return repo.get();
+ }
public abstract Change.Id changeId();
@@ -39,6 +44,8 @@
Change.Id changeId,
@Nullable ObjectId changeRevision,
Project.NameKey project) {
- return new AutoValue_ChangeCacheKey(repo, changeId, changeRevision, project);
+ ChangeCacheKey cacheKey = new AutoValue_ChangeCacheKey(changeId, changeRevision, project);
+ cacheKey.repo.set(repo);
+ return cacheKey;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangesTsCache.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangesTsCache.java
index 79a7fb4..722d40d 100644
--- a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangesTsCache.java
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangesTsCache.java
@@ -14,10 +14,8 @@
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;
@@ -38,7 +36,6 @@
@Singleton
static class Loader extends CacheLoader<ChangeCacheKey, Long> {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ChangeNotes.Factory changeNotesFactory;
@Inject
@@ -48,17 +45,11 @@
@Override
public Long load(ChangeCacheKey key) throws Exception {
- try {
- return changeNotesFactory
- .createChecked(key.repo(), key.project(), key.changeId(), key.changeRevision())
- .getChange()
- .getLastUpdatedOn()
- .toEpochMilli();
- } catch (NoSuchChangeException e) {
- logger.atFine().withCause(e).log(
- "Change %d does not exist: returning zero epoch", key.changeId().get());
- return 0L;
- }
+ return changeNotesFactory
+ .createChecked(key.repo(), key.project(), key.changeId(), key.changeRevision())
+ .getChange()
+ .getLastUpdatedOn()
+ .toEpochMilli();
}
}
}
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 58a80b1..2186b97 100644
--- a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java
@@ -19,8 +19,8 @@
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.Change.Id;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.access.CoreOrPluginProjectPermission;
@@ -34,16 +34,19 @@
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.name.Named;
+import java.io.IOException;
import java.sql.Timestamp;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Collection;
import java.util.Map;
+import java.util.Optional;
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;
+import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
public class ForProjectWrapper extends ForProject {
@@ -96,10 +99,11 @@
@Override
public Collection<Ref> filter(Collection<Ref> refs, Repository repo, RefFilterOptions opts)
throws PermissionBackendException {
- Map<Change.Id, ObjectId> changeRevisions =
+ Map<Optional<Id>, ObjectId> changeRevisions =
refs.stream()
.filter(ref -> ref.getName().endsWith("/meta"))
.collect(Collectors.toMap(ForProjectWrapper::changeIdFromRef, Ref::getObjectId));
+ RefDatabase refDb = repo.getRefDatabase();
return defaultForProject
.filter(refs, repo, opts)
.parallelStream()
@@ -108,30 +112,42 @@
.filter(config::isRefToShow)
.filter(
(ref) -> {
- Change.Id changeId = changeIdFromRef(ref);
+ Optional<Id> changeId = changeIdFromRef(ref);
+ Optional<ObjectId> changeRevision =
+ changeId.flatMap(cid -> Optional.ofNullable(changeRevisions.get(changeId)));
+ if (!changeRevision.isPresent()) {
+ changeRevision = changeRevisionFromRefDb(refDb, changeId);
+ }
String refName = ref.getName();
- return (!isChangeRef(refName)
- || (!isChangeMetaRef(refName)
- && changeId != null
- && (isOpen(repo, changeId, changeRevisions.get(changeId))
- || isRecent(repo, changeId, changeRevisions.get(changeId)))));
+ return (!changeId.isPresent()
+ || !changeRevision.isPresent()
+ || (!RefNames.isNoteDbMetaRef(refName)
+ && (isOpen(repo, changeId.get(), changeRevision.get())
+ || isRecent(repo, changeId.get(), changeRevision.get()))));
})
.collect(Collectors.toList());
}
- private static Change.Id changeIdFromRef(Ref ref) {
- return Change.Id.fromRef(ref.getName());
+ private static Optional<ObjectId> changeRevisionFromRefDb(
+ RefDatabase refDb, Optional<Change.Id> changeId) {
+ return changeId.flatMap(cid -> exactRefUnchecked(refDb, cid)).map(Ref::getObjectId);
}
- private boolean isChangeRef(String changeKey) {
- return changeKey.startsWith("refs/changes");
+ private static Optional<Ref> exactRefUnchecked(RefDatabase refDb, Change.Id changeId) {
+ try {
+ return Optional.ofNullable(refDb.exactRef(RefNames.changeMetaRef(changeId)));
+ } catch (IOException e) {
+ logger.atWarning().withCause(e).log(
+ "Error looking up change '%d' meta-ref from refs db.", changeId.get());
+ return Optional.empty();
+ }
}
- private boolean isChangeMetaRef(String changeKey) {
- return isChangeRef(changeKey) && changeKey.endsWith("/meta");
+ private static Optional<Change.Id> changeIdFromRef(Ref ref) {
+ return Optional.ofNullable(Change.Id.fromRef(ref.getName()));
}
- private boolean isOpen(Repository repo, Change.Id changeId, @Nullable ObjectId changeRevision) {
+ private boolean isOpen(Repository repo, Change.Id changeId, ObjectId changeRevision) {
try {
return openChangesCache.get(ChangeCacheKey.create(repo, changeId, changeRevision, project));
} catch (ExecutionException e) {
@@ -142,7 +158,7 @@
}
}
- private boolean isRecent(Repository repo, Change.Id changeId, @Nullable ObjectId changeRevision) {
+ private boolean isRecent(Repository repo, Change.Id changeId, ObjectId changeRevision) {
try {
Timestamp cutOffTs =
Timestamp.from(
diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java
index 73066d8..d8dd16b 100644
--- a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java
@@ -14,10 +14,8 @@
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;
@@ -38,7 +36,6 @@
@Singleton
static class Loader extends CacheLoader<ChangeCacheKey, Boolean> {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ChangeNotes.Factory changeNotesFactory;
@Inject
@@ -48,16 +45,10 @@
@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().get());
- return false;
- }
+ ChangeNotes changeNotes =
+ changeNotesFactory.createChecked(
+ key.repo(), key.project(), key.changeId(), key.changeRevision());
+ return changeNotes.getChange().getStatus().isOpen();
}
}
}
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 c58f0b8..243baa9 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
@@ -22,13 +22,22 @@
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.groups.GroupApi;
import com.google.gerrit.extensions.api.groups.GroupInput;
import com.google.gerrit.extensions.restapi.RestApiException;
+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.googlesource.gerrit.modules.gitrefsfilter.FilterRefsCapability;
+import com.googlesource.gerrit.modules.gitrefsfilter.FilterRefsConfig;
+import com.googlesource.gerrit.modules.gitrefsfilter.RefsFilterModule;
import java.io.IOException;
+import java.time.Duration;
import java.util.List;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
@@ -43,6 +52,11 @@
@Inject private RequestScopeOperations requestScopeOperations;
@Inject private ProjectOperations projectOperations;
+ @Override
+ public Module createModule() {
+ return new RefsFilterModule();
+ }
+
protected int createChangeAndAbandon() throws Exception, RestApiException {
requestScopeOperations.setApiUser(admin.id());
createChange();
@@ -63,6 +77,10 @@
groupApi.removeMembers(admin.username());
String groupId = groupApi.detail().id;
+ setHideClosedChangesRefs(groupId);
+ }
+
+ protected void setHideClosedChangesRefs(String groupId) {
projectOperations
.allProjectsForUpdate()
.add(
@@ -120,4 +138,20 @@
*/
return Integer.parseInt(ref.getName().split("/")[4]);
}
+
+ protected void setProjectClosedChangesGraceTime(Project.NameKey project, Duration graceTime)
+ throws IOException, ConfigInvalidException, RepositoryNotFoundException {
+ 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,
+ graceTime.toSeconds()));
+ projectConfig.commit(md);
+ projectCache.evict(project);
+ }
+ }
}
diff --git a/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterProtocolV2IT.java b/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterProtocolV2IT.java
new file mode 100644
index 0000000..0d56ec8
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterProtocolV2IT.java
@@ -0,0 +1,186 @@
+// 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.libmodule.plugins.test;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.WaitUtil.waitUntil;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.io.ByteStreams;
+import com.google.gerrit.acceptance.GitClientVersion;
+import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.acceptance.UseSsh;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Permission;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInput;
+import com.google.gerrit.httpd.CanonicalWebUrl;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.inject.Inject;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InterruptedIOException;
+import java.nio.file.Path;
+import java.time.Duration;
+import org.junit.Test;
+
+@UseSsh
+@UseLocalDisk
+public class GitRefsFilterProtocolV2IT extends AbstractGitDaemonTest {
+ private final String[] GIT_FETCH = new String[] {"git", "-c", "protocol.version=2", "fetch"};
+ private final String[] GIT_INIT = new String[] {"git", "init"};
+
+ private static final Duration TEST_PATIENCE_TIME = Duration.ofSeconds(1);
+
+ @Inject private ProjectOperations projectOperations;
+ @Inject private SitePaths sitePaths;
+ @Inject private CanonicalWebUrl url;
+
+ public static void assertGitClientVersion() throws Exception {
+ // Minimum required git-core version that supports wire protocol v2 is 2.18.0
+ GitClientVersion requiredGitVersion = new GitClientVersion(2, 18, 0);
+ GitClientVersion actualGitVersion =
+ new GitClientVersion(execute(ImmutableList.of("git", "version"), new File("/")));
+ // If git client version cannot be updated, consider to skip this tests. Due to
+ // an existing issue in bazel, JUnit assumption violation feature cannot be used.
+ assertThat(actualGitVersion).isAtLeast(requiredGitVersion);
+ }
+
+ @Test
+ public void testGitWireProtocolV2HidesAbandonedChange() throws Exception {
+ assertGitClientVersion();
+
+ Project.NameKey allRefsVisibleProject = Project.nameKey("all-refs-visible");
+ gApi.projects().create(allRefsVisibleProject.get());
+
+ setProjectPermissionReadAllRefs(allRefsVisibleProject);
+ setHideClosedChangesRefs(SystemGroupBackend.ANONYMOUS_USERS.get());
+ setProjectClosedChangesGraceTime(allRefsVisibleProject, Duration.ofSeconds(0));
+
+ // Create new change and retrieve refs for the created patch set
+ ChangeInput visibleChangeIn =
+ new ChangeInput(allRefsVisibleProject.get(), "master", "Test public change");
+ visibleChangeIn.newBranch = true;
+ ChangeInfo changeInfo = gApi.changes().create(visibleChangeIn).info();
+ int visibleChangeNumber = changeInfo._number;
+ Change.Id changeId = Change.id(visibleChangeNumber);
+ String visibleChangeNumberRef = RefNames.patchSetRef(PatchSet.id(changeId, 1));
+ String patchSetSha1 = gApi.changes().id(visibleChangeNumber).get().currentRevision;
+ String visibleChangeNumberWithSha1 = patchSetSha1 + " " + visibleChangeNumberRef;
+
+ execute(ImmutableList.<String>builder().add(GIT_INIT).build(), ImmutableMap.of());
+ String gitProtocolOutActiveChange = execFetch(allRefsVisibleProject, visibleChangeNumberRef);
+ assertThat(gitProtocolOutActiveChange).contains(visibleChangeNumberWithSha1);
+
+ gApi.changes().id(changeId.get()).abandon();
+
+ waitUntil(
+ () -> {
+ try {
+ return !execFetch(allRefsVisibleProject, visibleChangeNumberRef)
+ .contains(visibleChangeNumberWithSha1);
+ } catch (Exception e) {
+ throw new IllegalStateException(e);
+ }
+ },
+ TEST_PATIENCE_TIME);
+ }
+
+ private String execFetch(Project.NameKey project, String refs) throws Exception {
+ String outAnonymousLsRemote =
+ execute(
+ ImmutableList.<String>builder()
+ .add(GIT_FETCH)
+ .add(url.get(null) + "/" + project.get())
+ .add(refs)
+ .build(),
+ ImmutableMap.of("GIT_TRACE_PACKET", "1"));
+ assertGitProtocolV2(outAnonymousLsRemote);
+ return outAnonymousLsRemote;
+ }
+
+ private void assertGitProtocolV2(String outAnonymousLsRemote) {
+ assertThat(outAnonymousLsRemote).contains("git< version 2");
+ }
+
+ private void setProjectPermissionReadAllRefs(Project.NameKey project) {
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/*").group(SystemGroupBackend.ANONYMOUS_USERS))
+ .update();
+ }
+
+ private String execute(ImmutableList<String> cmd, ImmutableMap<String, String> env)
+ throws Exception {
+ return execute(cmd, sitePaths.data_dir.toFile(), env);
+ }
+
+ private static String execute(ImmutableList<String> cmd, File dir) throws Exception {
+ return execute(cmd, dir, ImmutableMap.of());
+ }
+
+ protected static String execute(
+ ImmutableList<String> cmd, File dir, ImmutableMap<String, String> env) throws IOException {
+ return execute(cmd, dir, env, null);
+ }
+
+ protected static String execute(
+ ImmutableList<String> cmd,
+ File dir,
+ @Nullable ImmutableMap<String, String> env,
+ @Nullable Path outputPath)
+ throws IOException {
+ ProcessBuilder pb = new ProcessBuilder(cmd);
+ pb.directory(dir);
+ if (outputPath != null) {
+ pb.redirectOutput(outputPath.toFile());
+ } else {
+ pb.redirectErrorStream(true);
+ }
+ pb.environment().putAll(env);
+ Process p = pb.start();
+ byte[] out;
+ try (InputStream in = p.getInputStream()) {
+ out = ByteStreams.toByteArray(in);
+ } finally {
+ p.getOutputStream().close();
+ }
+
+ try {
+ p.waitFor();
+ } catch (InterruptedException e) {
+ InterruptedIOException iioe =
+ new InterruptedIOException(
+ "interrupted waiting for: " + Joiner.on(' ').join(pb.command()));
+ iioe.initCause(e);
+ throw iioe;
+ }
+
+ String result = new String(out, UTF_8);
+ return result.trim();
+ }
+}
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 1fd85c1..7663d76 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
@@ -29,14 +29,9 @@
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;
@@ -76,26 +71,10 @@
private volatile Exception getRefsException = null;
- @Override
- public Module createModule() {
- return new RefsFilterModule();
- }
-
@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);
- }
+ setProjectClosedChangesGraceTime(project, Duration.ofSeconds(CLOSED_CHANGES_GRACE_TIME_SEC));
}
@Test
@@ -191,10 +170,24 @@
assertThat(cacheEntry.getKey().project()).isEqualTo(project);
assertThat(cacheEntry.getKey().changeId()).isEqualTo(changeId);
assertThat(cacheEntry.getKey().changeRevision()).isEqualTo(metaRef.getObjectId());
+ assertThat(cacheEntry.getKey().repo()).isNotNull();
assertThat(cacheEntry.getValue()).isFalse();
}
@Test
+ public void testShouldCacheChangeKeyContainRepoAfterDeserializing() throws Exception {
+ Change.Id changeId = Change.id(createChangeAndAbandon());
+ getRefs(cloneProjectChangesRefs(user));
+
+ assertThat(changeOpenCache.asMap().size()).isEqualTo(1);
+
+ Map.Entry<ChangeCacheKey, Boolean> cacheEntry =
+ new ArrayList<>(changeOpenCache.asMap().entrySet()).get(0);
+
+ assertThat(cacheEntry.getKey().repo()).isNotNull();
+ }
+
+ @Test
public void testShouldCacheWhenChangeIsOpen() throws Exception {
createChange();
List<Ref> refs = getRefs(cloneProjectChangesRefs(user));
diff --git a/src/test/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangeCacheKeyTest.java b/src/test/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangeCacheKeyTest.java
new file mode 100644
index 0000000..df68a5e
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangeCacheKeyTest.java
@@ -0,0 +1,50 @@
+// 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.modules.gitrefsfilter;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.mock;
+
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.Project.NameKey;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Test;
+
+public class ChangeCacheKeyTest {
+ private static final String REPO_NAME = "test_repo";
+ private static final Change.Id ID = Change.id(10000);
+ private static final ObjectId CHANGE_REVISION = ObjectId.zeroId();
+ private static final NameKey TEST_REPO = Project.nameKey(REPO_NAME);
+
+ @Test
+ public void shouldExcludeRepoFieldDuringEqualsCalculation() {
+ ChangeCacheKey cacheKey1 =
+ ChangeCacheKey.create(mock(Repository.class), ID, CHANGE_REVISION, TEST_REPO);
+ ChangeCacheKey cacheKey2 =
+ ChangeCacheKey.create(mock(Repository.class), ID, CHANGE_REVISION, TEST_REPO);
+ assertThat(cacheKey1).isEqualTo(cacheKey2);
+ }
+
+ @Test
+ public void shouldExcludeRepoFieldDuringHashCodeCalculation() {
+ ChangeCacheKey cacheKey1 =
+ ChangeCacheKey.create(mock(Repository.class), ID, CHANGE_REVISION, TEST_REPO);
+ ChangeCacheKey cacheKey2 =
+ ChangeCacheKey.create(mock(Repository.class), ID, CHANGE_REVISION, TEST_REPO);
+ assertThat(cacheKey1.hashCode()).isEqualTo(cacheKey2.hashCode());
+ }
+}
