Merge branch 'stable-3.5'
* stable-3.5:
Don't attempt to cache refs containing '[:|@]' in resolve call
Change-Id: Ia6128b70c22eef310f870427fc12f6fde5d129b1
diff --git a/src/main/java/com/googlesource/gerrit/plugins/cachedrefdb/CachedRefRepository.java b/src/main/java/com/googlesource/gerrit/plugins/cachedrefdb/CachedRefRepository.java
index dada373..6710188 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/cachedrefdb/CachedRefRepository.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/cachedrefdb/CachedRefRepository.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.cachedrefdb;
+import com.google.common.base.CharMatcher;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.server.git.DelegateRepository;
import com.google.inject.Inject;
@@ -58,6 +59,8 @@
private final RefUpdateWithCacheUpdate.Factory updateFactory;
private final RefRenameWithCacheUpdate.Factory renameFactory;
+ private static final CharMatcher REVISION_CHARS = CharMatcher.anyOf("^~:@");
+
@Inject
CachedRefRepository(
CachedRefDatabase.Factory refDbFactory,
@@ -365,6 +368,6 @@
}
private boolean isCacheableReference(String ref) {
- return ref.startsWith(RefNames.REFS) && !(ref.contains("^") || ref.contains("~"));
+ return ref.startsWith(RefNames.REFS) && REVISION_CHARS.matchesNoneOf(ref);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/cachedrefdb/CachedRefRepositoryIT.java b/src/test/java/com/googlesource/gerrit/plugins/cachedrefdb/CachedRefRepositoryIT.java
index 5f2c2e4..b04cac0 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/cachedrefdb/CachedRefRepositoryIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/cachedrefdb/CachedRefRepositoryIT.java
@@ -21,10 +21,12 @@
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.registration.DynamicItem;
import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.util.Optional;
import java.util.concurrent.Callable;
-import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
-import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
@@ -33,16 +35,28 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.After;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
public class CachedRefRepositoryIT {
- TestRepository<Repository> tr;
- CachedRefRepository objectUnderTest;
- TestRefByNameCacheImpl cache;
+ @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+ private TestRepository<Repository> tr;
+ private CachedRefRepository objectUnderTest;
+ private TestRefByNameCacheImpl cache;
@Before
public void setUp() throws IOException {
- Repository repo = new InMemoryRepository(new DfsRepositoryDescription("repo"));
+ Path repoPath = temporaryFolder.newFolder().toPath();
+ Repository repo = new FileRepository(repoPath.toFile());
+ repo.create(true);
+
+ // enable reflog for a repository so that references to it could be resolved
+ Files.write(
+ repoPath.resolve("config"),
+ "[core]\n logAllRefUpdates = true\n".getBytes(StandardCharsets.UTF_8));
+
objectUnderTest = createCachedRepository(repo);
tr = new TestRepository<>(repo);
}
@@ -72,7 +86,8 @@
@Test
public void shouldNotResolveRefsFromCache() throws Exception {
String master = RefNames.fullName("master");
- RevCommit first = tr.update(master, tr.commit().add("first", "foo").create());
+ String filename = "first";
+ RevCommit first = tr.update(master, tr.commit().add(filename, "foo").create());
String tag = "test_tag";
String fullTag = RefNames.REFS_TAGS + tag;
tr.update(fullTag, tr.tag(tag, first));
@@ -92,6 +107,14 @@
assertThat(resolvedByTilde).isEqualTo(first);
assertThat(resolvedByTilde).isEqualTo(repo().resolve(mastersParent));
+ String mastersFilename = master + ":" + filename;
+ ObjectId resolvedByFilename = objectUnderTest.resolve(mastersFilename);
+ assertThat(resolvedByFilename).isEqualTo(repo().resolve(mastersFilename));
+
+ String mastersPreviousRevision = master + "@{1}";
+ ObjectId resolvedByPreviousRevision = objectUnderTest.resolve(mastersPreviousRevision);
+ assertThat(resolvedByPreviousRevision).isEqualTo(repo().resolve(mastersPreviousRevision));
+
assertThat(cache.cacheCalled).isEqualTo(0);
}
