Extract the keys for the (Git)ModifiedFilesCaches to separate classes
Change-Id: If8ea827f82ae369eb744fe8eb60e88f528762179
diff --git a/java/com/google/gerrit/server/patch/diff/ModifiedFilesCache.java b/java/com/google/gerrit/server/patch/diff/ModifiedFilesCache.java
index 9fc8150..bcae238 100644
--- a/java/com/google/gerrit/server/patch/diff/ModifiedFilesCache.java
+++ b/java/com/google/gerrit/server/patch/diff/ModifiedFilesCache.java
@@ -18,6 +18,7 @@
import com.google.gerrit.server.patch.DiffNotAvailableException;
import com.google.gerrit.server.patch.gitdiff.GitModifiedFilesCache;
import com.google.gerrit.server.patch.gitdiff.GitModifiedFilesCacheImpl;
+import com.google.gerrit.server.patch.gitdiff.GitModifiedFilesCacheKey;
import com.google.gerrit.server.patch.gitdiff.ModifiedFile;
/**
@@ -26,10 +27,10 @@
* <p>The loader uses the underlying {@link GitModifiedFilesCacheImpl} to retrieve the git modified
* files.
*
- * <p>If the {@link ModifiedFilesCacheImpl.Key#aCommit()} is equal to {@link
+ * <p>If the {@link ModifiedFilesCacheKey#aCommit()} is equal to {@link
* org.eclipse.jgit.lib.Constants#EMPTY_TREE_ID}, the diff will be evaluated against the empty tree,
* and the result will be exactly the same as the caller can get from {@link
- * GitModifiedFilesCache#get(GitModifiedFilesCacheImpl.Key)}
+ * GitModifiedFilesCache#get(GitModifiedFilesCacheKey)}
*/
public interface ModifiedFilesCache {
@@ -40,5 +41,5 @@
* @throws DiffNotAvailableException the supplied commits IDs of the key do no exist, are not IDs
* of a commit, or an exception occurred while reading a pack file.
*/
- ImmutableList<ModifiedFile> get(ModifiedFilesCacheImpl.Key key) throws DiffNotAvailableException;
+ ImmutableList<ModifiedFile> get(ModifiedFilesCacheKey key) throws DiffNotAvailableException;
}
diff --git a/java/com/google/gerrit/server/patch/diff/ModifiedFilesCacheImpl.java b/java/com/google/gerrit/server/patch/diff/ModifiedFilesCacheImpl.java
index a87277c..6023c0e 100644
--- a/java/com/google/gerrit/server/patch/diff/ModifiedFilesCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/diff/ModifiedFilesCacheImpl.java
@@ -17,7 +17,6 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static org.eclipse.jgit.lib.Constants.EMPTY_TREE_ID;
-import com.google.auto.value.AutoValue;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableList;
@@ -25,23 +24,19 @@
import com.google.common.collect.Sets;
import com.google.common.collect.Streams;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.entities.Project;
-import com.google.gerrit.entities.Project.NameKey;
import com.google.gerrit.server.cache.CacheModule;
-import com.google.gerrit.server.cache.serialize.CacheSerializer;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.DiffNotAvailableException;
import com.google.gerrit.server.patch.DiffUtil;
-import com.google.gerrit.server.patch.diff.ModifiedFilesCacheImpl.Key.Serializer;
import com.google.gerrit.server.patch.gitdiff.GitModifiedFilesCache;
import com.google.gerrit.server.patch.gitdiff.GitModifiedFilesCacheImpl;
+import com.google.gerrit.server.patch.gitdiff.GitModifiedFilesCacheKey;
import com.google.gerrit.server.patch.gitdiff.ModifiedFile;
import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
import java.io.IOException;
-import java.io.Serializable;
import java.util.List;
import java.util.Set;
import java.util.stream.Stream;
@@ -56,16 +51,17 @@
* <p>The loader of this cache wraps a {@link GitModifiedFilesCache} to retrieve the git modified
* files.
*
- * <p>If the {@link Key#aCommit()} is equal to {@link org.eclipse.jgit.lib.Constants#EMPTY_TREE_ID},
- * the diff will be evaluated against the empty tree, and the result will be exactly the same as the
- * caller can get from {@link GitModifiedFilesCache#get(GitModifiedFilesCacheImpl.Key)}
+ * <p>If the {@link ModifiedFilesCacheKey#aCommit()} is equal to {@link
+ * org.eclipse.jgit.lib.Constants#EMPTY_TREE_ID}, the diff will be evaluated against the empty tree,
+ * and the result will be exactly the same as the caller can get from {@link
+ * GitModifiedFilesCache#get(GitModifiedFilesCacheKey)}
*/
public class ModifiedFilesCacheImpl implements ModifiedFilesCache {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private static final String MODIFIED_FILES = "modified_files";
- private final LoadingCache<Key, ImmutableList<ModifiedFile>> cache;
+ private final LoadingCache<ModifiedFilesCacheKey, ImmutableList<ModifiedFile>> cache;
public static Module module() {
return new CacheModule() {
@@ -80,9 +76,9 @@
// in the cache documentation link.
persist(
ModifiedFilesCacheImpl.MODIFIED_FILES,
- Key.class,
+ ModifiedFilesCacheKey.class,
new TypeLiteral<ImmutableList<ModifiedFile>>() {})
- .keySerializer(Serializer.INSTANCE)
+ .keySerializer(ModifiedFilesCacheKey.Serializer.INSTANCE)
.valueSerializer(GitModifiedFilesCacheImpl.ValueSerializer.INSTANCE)
.maximumWeight(10 << 20)
.weigher(ModifiedFilesWeigher.class)
@@ -95,12 +91,13 @@
@Inject
public ModifiedFilesCacheImpl(
@Named(ModifiedFilesCacheImpl.MODIFIED_FILES)
- LoadingCache<Key, ImmutableList<ModifiedFile>> cache) {
+ LoadingCache<ModifiedFilesCacheKey, ImmutableList<ModifiedFile>> cache) {
this.cache = cache;
}
@Override
- public ImmutableList<ModifiedFile> get(Key key) throws DiffNotAvailableException {
+ public ImmutableList<ModifiedFile> get(ModifiedFilesCacheKey key)
+ throws DiffNotAvailableException {
try {
return cache.get(key);
} catch (Exception e) {
@@ -108,7 +105,8 @@
}
}
- static class ModifiedFilesLoader extends CacheLoader<Key, ImmutableList<ModifiedFile>> {
+ static class ModifiedFilesLoader
+ extends CacheLoader<ModifiedFilesCacheKey, ImmutableList<ModifiedFile>> {
private final GitModifiedFilesCache gitCache;
private final GitRepositoryManager repoManager;
@@ -119,22 +117,23 @@
}
@Override
- public ImmutableList<ModifiedFile> load(Key key) throws IOException, DiffNotAvailableException {
+ public ImmutableList<ModifiedFile> load(ModifiedFilesCacheKey key)
+ throws IOException, DiffNotAvailableException {
try (Repository repo = repoManager.openRepository(key.project());
RevWalk rw = new RevWalk(repo.newObjectReader())) {
return loadModifiedFiles(key, rw);
}
}
- private ImmutableList<ModifiedFile> loadModifiedFiles(Key key, RevWalk rw)
+ private ImmutableList<ModifiedFile> loadModifiedFiles(ModifiedFilesCacheKey key, RevWalk rw)
throws IOException, DiffNotAvailableException {
ObjectId aTree =
key.aCommit().equals(EMPTY_TREE_ID)
? key.aCommit()
: DiffUtil.getTreeId(rw, key.aCommit());
ObjectId bTree = DiffUtil.getTreeId(rw, key.bCommit());
- GitModifiedFilesCacheImpl.Key gitKey =
- GitModifiedFilesCacheImpl.Key.builder()
+ GitModifiedFilesCacheKey gitKey =
+ GitModifiedFilesCacheKey.builder()
.project(key.project())
.aTree(aTree)
.bTree(bTree)
@@ -161,21 +160,22 @@
* Returns the paths of files that were modified between the old and new commits versus their
* parents (i.e. old commit vs. its parent, and new commit vs. its parent).
*
- * @param key the {@link Key} representing the commits we are diffing
+ * @param key the {@link ModifiedFilesCacheKey} representing the commits we are diffing
* @param rw a {@link RevWalk} for the repository
* @return The list of modified files between the old/new commits and their parents
*/
private Set<String> getTouchedFilesWithParents(
- Key key, ObjectId parentOfA, ObjectId parentOfB, RevWalk rw) throws IOException {
+ ModifiedFilesCacheKey key, ObjectId parentOfA, ObjectId parentOfB, RevWalk rw)
+ throws IOException {
try {
// TODO(ghareeb): as an enhancement: the 3 calls of the underlying git cache can be combined
- GitModifiedFilesCacheImpl.Key oldVsBaseKey =
- GitModifiedFilesCacheImpl.Key.create(
+ GitModifiedFilesCacheKey oldVsBaseKey =
+ GitModifiedFilesCacheKey.create(
key.project(), parentOfA, key.aCommit(), key.renameScore(), rw);
List<ModifiedFile> oldVsBase = gitCache.get(oldVsBaseKey);
- GitModifiedFilesCacheImpl.Key newVsBaseKey =
- GitModifiedFilesCacheImpl.Key.create(
+ GitModifiedFilesCacheKey newVsBaseKey =
+ GitModifiedFilesCacheKey.create(
key.project(), parentOfB, key.bCommit(), key.renameScore(), rw);
List<ModifiedFile> newVsBase = gitCache.get(newVsBaseKey);
@@ -203,85 +203,4 @@
return touchedFilePaths.contains(oldFilePath) || touchedFilePaths.contains(newFilePath);
}
}
-
- @AutoValue
- public abstract static class Key implements Serializable {
- public abstract Project.NameKey project();
-
- /** @return the old commit ID used in the git tree diff */
- public abstract ObjectId aCommit();
-
- /** @return the new commit ID used in the git tree diff */
- public abstract ObjectId bCommit();
-
- /**
- * Percentage score used to identify a file as a "rename". A special value of -1 means that the
- * computation will ignore renames and rename detection will be disabled.
- */
- public abstract int renameScore();
-
- public boolean renameDetectionEnabled() {
- return renameScore() != -1;
- }
-
- public int weight() {
- return project().get().length() + 20 * 2 + 4;
- }
-
- public static Builder builder() {
- return new AutoValue_ModifiedFilesCacheImpl_Key.Builder();
- }
-
- @AutoValue.Builder
- public abstract static class Builder {
-
- public abstract Builder project(NameKey value);
-
- public abstract Builder aCommit(ObjectId value);
-
- public abstract Builder bCommit(ObjectId value);
-
- public Builder disableRenameDetection() {
- renameScore(-1);
- return this;
- }
-
- public abstract Builder renameScore(int value);
-
- public abstract Key build();
- }
-
- public enum Serializer implements CacheSerializer<Key> {
- INSTANCE;
-
- @Override
- public byte[] serialize(Key key) {
- // We are reusing the serializer of the GitModifiedFilesCacheImpl#Key since both classes
- // contain exactly the same fields, with the difference that the Object Ids here refer
- // to the commit SHA-1s instead of the tree SHA-1s, but they are still can be serialized
- // and deserialized in the same way.
- GitModifiedFilesCacheImpl.Key gitKey =
- GitModifiedFilesCacheImpl.Key.builder()
- .project(key.project())
- .aTree(key.aCommit())
- .bTree(key.bCommit())
- .renameScore(key.renameScore())
- .build();
-
- return GitModifiedFilesCacheImpl.Key.KeySerializer.INSTANCE.serialize(gitKey);
- }
-
- @Override
- public Key deserialize(byte[] in) {
- GitModifiedFilesCacheImpl.Key gitKey =
- GitModifiedFilesCacheImpl.Key.KeySerializer.INSTANCE.deserialize(in);
- return Key.builder()
- .project(gitKey.project())
- .aCommit(gitKey.aTree())
- .bCommit(gitKey.bTree())
- .renameScore(gitKey.renameScore())
- .build();
- }
- }
- }
}
diff --git a/java/com/google/gerrit/server/patch/diff/ModifiedFilesCacheKey.java b/java/com/google/gerrit/server/patch/diff/ModifiedFilesCacheKey.java
new file mode 100644
index 0000000..5aa31ec
--- /dev/null
+++ b/java/com/google/gerrit/server/patch/diff/ModifiedFilesCacheKey.java
@@ -0,0 +1,116 @@
+// Copyright (C) 2020 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.google.gerrit.server.patch.diff;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.Project.NameKey;
+import com.google.gerrit.proto.Protos;
+import com.google.gerrit.server.cache.proto.Cache.ModifiedFilesKeyProto;
+import com.google.gerrit.server.cache.serialize.CacheSerializer;
+import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
+import org.eclipse.jgit.lib.ObjectId;
+
+/** Cache key for the {@link com.google.gerrit.server.patch.diff.ModifiedFilesCache} */
+@AutoValue
+public abstract class ModifiedFilesCacheKey {
+
+ /** A specific git project / repository. */
+ public abstract Project.NameKey project();
+
+ /** @return the old commit ID used in the git tree diff */
+ public abstract ObjectId aCommit();
+
+ /** @return the new commit ID used in the git tree diff */
+ public abstract ObjectId bCommit();
+
+ /**
+ * Percentage score used to identify a file as a "rename". A special value of -1 means that the
+ * computation will ignore renames and rename detection will be disabled.
+ */
+ public abstract int renameScore();
+
+ public boolean renameDetectionEnabled() {
+ return renameScore() != -1;
+ }
+
+ /** Returns the size of the object in bytes */
+ public int weight() {
+ return stringSize(project().get()) // project
+ + 20 * 2 // aCommit and bCommit
+ + 4; // renameScore
+ }
+
+ public static ModifiedFilesCacheKey.Builder builder() {
+ return new AutoValue_ModifiedFilesCacheKey.Builder();
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+
+ public abstract ModifiedFilesCacheKey.Builder project(NameKey value);
+
+ public abstract ModifiedFilesCacheKey.Builder aCommit(ObjectId value);
+
+ public abstract ModifiedFilesCacheKey.Builder bCommit(ObjectId value);
+
+ public ModifiedFilesCacheKey.Builder disableRenameDetection() {
+ renameScore(-1);
+ return this;
+ }
+
+ public abstract ModifiedFilesCacheKey.Builder renameScore(int value);
+
+ public abstract ModifiedFilesCacheKey build();
+ }
+
+ public enum Serializer implements CacheSerializer<ModifiedFilesCacheKey> {
+ INSTANCE;
+
+ @Override
+ public byte[] serialize(ModifiedFilesCacheKey key) {
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ return Protos.toByteArray(
+ ModifiedFilesKeyProto.newBuilder()
+ .setProject(key.project().get())
+ .setACommit(idConverter.toByteString(key.aCommit()))
+ .setBCommit(idConverter.toByteString(key.bCommit()))
+ .setRenameScore(key.renameScore())
+ .build());
+ }
+
+ @Override
+ public ModifiedFilesCacheKey deserialize(byte[] in) {
+ ModifiedFilesKeyProto proto = Protos.parseUnchecked(ModifiedFilesKeyProto.parser(), in);
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ return ModifiedFilesCacheKey.builder()
+ .project(NameKey.parse(proto.getProject()))
+ .aCommit(idConverter.fromByteString(proto.getACommit()))
+ .bCommit(idConverter.fromByteString(proto.getBCommit()))
+ .renameScore(proto.getRenameScore())
+ .build();
+ }
+ }
+
+ private static int stringSize(String str) {
+ if (str != null) {
+ // each character in the string occupies 2 bytes. Ignoring the fixed overhead for the string
+ // (length, offset and hash code) since they are negligible and do not
+ // affect the comparison of 2 strings
+ return str.length() * 2;
+ }
+ return 0;
+ }
+}
diff --git a/java/com/google/gerrit/server/patch/diff/ModifiedFilesWeigher.java b/java/com/google/gerrit/server/patch/diff/ModifiedFilesWeigher.java
index 0b2c69e..512da6f 100644
--- a/java/com/google/gerrit/server/patch/diff/ModifiedFilesWeigher.java
+++ b/java/com/google/gerrit/server/patch/diff/ModifiedFilesWeigher.java
@@ -19,9 +19,9 @@
import com.google.gerrit.server.patch.gitdiff.ModifiedFile;
public class ModifiedFilesWeigher
- implements Weigher<ModifiedFilesCacheImpl.Key, ImmutableList<ModifiedFile>> {
+ implements Weigher<ModifiedFilesCacheKey, ImmutableList<ModifiedFile>> {
@Override
- public int weigh(ModifiedFilesCacheImpl.Key key, ImmutableList<ModifiedFile> modifiedFiles) {
+ public int weigh(ModifiedFilesCacheKey key, ImmutableList<ModifiedFile> modifiedFiles) {
int weight = key.weight();
for (ModifiedFile modifiedFile : modifiedFiles) {
weight += modifiedFile.weight();
diff --git a/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCache.java b/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCache.java
index 36af9fe..d178f22 100644
--- a/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCache.java
+++ b/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCache.java
@@ -36,6 +36,5 @@
* @return the list of {@link ModifiedFile}s between the 2 git trees identified by the key.
* @throws DiffNotAvailableException trees cannot be read or file contents cannot be read.
*/
- ImmutableList<ModifiedFile> get(GitModifiedFilesCacheImpl.Key key)
- throws DiffNotAvailableException;
+ ImmutableList<ModifiedFile> get(GitModifiedFilesCacheKey key) throws DiffNotAvailableException;
}
diff --git a/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCacheImpl.java b/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCacheImpl.java
index 230e12d..b3b82bb 100644
--- a/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCacheImpl.java
@@ -16,23 +16,17 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
-import com.google.auto.value.AutoValue;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.entities.Patch;
-import com.google.gerrit.entities.Project;
-import com.google.gerrit.entities.Project.NameKey;
import com.google.gerrit.proto.Protos;
import com.google.gerrit.server.cache.CacheModule;
-import com.google.gerrit.server.cache.proto.Cache.GitModifiedFilesKeyProto;
import com.google.gerrit.server.cache.proto.Cache.ModifiedFilesProto;
import com.google.gerrit.server.cache.serialize.CacheSerializer;
-import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.DiffNotAvailableException;
-import com.google.gerrit.server.patch.DiffUtil;
import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.TypeLiteral;
@@ -44,10 +38,8 @@
import org.eclipse.jgit.diff.DiffEntry;
import org.eclipse.jgit.diff.DiffEntry.ChangeType;
import org.eclipse.jgit.diff.DiffFormatter;
-import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.util.io.DisabledOutputStream;
/** Implementation of the {@link GitModifiedFilesCache} */
@@ -66,7 +58,7 @@
DiffEntry.ChangeType.COPY,
Patch.ChangeType.COPIED);
- private LoadingCache<Key, ImmutableList<ModifiedFile>> cache;
+ private LoadingCache<GitModifiedFilesCacheKey, ImmutableList<ModifiedFile>> cache;
public static Module module() {
return new CacheModule() {
@@ -74,8 +66,11 @@
protected void configure() {
bind(GitModifiedFilesCache.class).to(GitModifiedFilesCacheImpl.class);
- persist(GIT_MODIFIED_FILES, Key.class, new TypeLiteral<ImmutableList<ModifiedFile>>() {})
- .keySerializer(Key.KeySerializer.INSTANCE)
+ persist(
+ GIT_MODIFIED_FILES,
+ GitModifiedFilesCacheKey.class,
+ new TypeLiteral<ImmutableList<ModifiedFile>>() {})
+ .keySerializer(GitModifiedFilesCacheKey.Serializer.INSTANCE)
.valueSerializer(ValueSerializer.INSTANCE)
// The documentation has some defaults and recommendations for setting the cache
// attributes:
@@ -92,12 +87,14 @@
@Inject
public GitModifiedFilesCacheImpl(
- @Named(GIT_MODIFIED_FILES) LoadingCache<Key, ImmutableList<ModifiedFile>> cache) {
+ @Named(GIT_MODIFIED_FILES)
+ LoadingCache<GitModifiedFilesCacheKey, ImmutableList<ModifiedFile>> cache) {
this.cache = cache;
}
@Override
- public ImmutableList<ModifiedFile> get(Key key) throws DiffNotAvailableException {
+ public ImmutableList<ModifiedFile> get(GitModifiedFilesCacheKey key)
+ throws DiffNotAvailableException {
try {
return cache.get(key);
} catch (ExecutionException e) {
@@ -105,7 +102,7 @@
}
}
- static class Loader extends CacheLoader<Key, ImmutableList<ModifiedFile>> {
+ static class Loader extends CacheLoader<GitModifiedFilesCacheKey, ImmutableList<ModifiedFile>> {
private final GitRepositoryManager repoManager;
@Inject
@@ -114,7 +111,7 @@
}
@Override
- public ImmutableList<ModifiedFile> load(Key key) throws IOException {
+ public ImmutableList<ModifiedFile> load(GitModifiedFilesCacheKey key) throws IOException {
try (Repository repo = repoManager.openRepository(key.project());
ObjectReader reader = repo.newObjectReader()) {
List<DiffEntry> entries = getGitTreeDiff(repo, reader, key);
@@ -124,8 +121,7 @@
}
private List<DiffEntry> getGitTreeDiff(
- Repository repo, ObjectReader reader, GitModifiedFilesCacheImpl.Key key)
- throws IOException {
+ Repository repo, ObjectReader reader, GitModifiedFilesCacheKey key) throws IOException {
try (DiffFormatter df = new DiffFormatter(DisabledOutputStream.INSTANCE)) {
df.setReader(reader, repo.getConfig());
if (key.renameDetection()) {
@@ -156,109 +152,6 @@
}
}
- /**
- * In this cache, we evaluate the diffs between two git trees (instead of git commits), hence the
- * key contains the tree IDs.
- */
- @AutoValue
- public abstract static class Key {
-
- public abstract Project.NameKey project();
-
- /**
- * The git SHA-1 {@link ObjectId} of the first git tree object for which the diff should be
- * computed.
- */
- public abstract ObjectId aTree();
-
- /**
- * The git SHA-1 {@link ObjectId} of the second git tree object for which the diff should be
- * computed.
- */
- public abstract ObjectId bTree();
-
- /**
- * Percentage score used to identify a file as a rename. This value is only available if {@link
- * #renameDetection()} is true. Otherwise, this method will return -1.
- *
- * <p>This value will be used to set the rename score of {@link
- * DiffFormatter#getRenameDetector()}.
- */
- public abstract int renameScore();
-
- /** Returns true if rename detection was set for this key. */
- public boolean renameDetection() {
- return renameScore() != -1;
- }
-
- public static Key create(
- Project.NameKey project, ObjectId aCommit, ObjectId bCommit, int renameScore, RevWalk rw)
- throws IOException {
- ObjectId aTree = DiffUtil.getTreeId(rw, aCommit);
- ObjectId bTree = DiffUtil.getTreeId(rw, bCommit);
- return builder().project(project).aTree(aTree).bTree(bTree).renameScore(renameScore).build();
- }
-
- public static Builder builder() {
- return new AutoValue_GitModifiedFilesCacheImpl_Key.Builder();
- }
-
- /** Returns the size of the object in bytes */
- public int weight() {
- return project().get().length()
- + 20 * 2 // old and new tree IDs
- + 4; // rename score
- }
-
- @AutoValue.Builder
- public abstract static class Builder {
-
- public abstract Builder project(NameKey value);
-
- public abstract Builder aTree(ObjectId value);
-
- public abstract Builder bTree(ObjectId value);
-
- public abstract Builder renameScore(int value);
-
- public Builder disableRenameDetection() {
- renameScore(-1);
- return this;
- }
-
- public abstract Key build();
- }
-
- public enum KeySerializer implements CacheSerializer<Key> {
- INSTANCE;
-
- @Override
- public byte[] serialize(Key key) {
- ObjectIdConverter idConverter = ObjectIdConverter.create();
- return Protos.toByteArray(
- GitModifiedFilesKeyProto.newBuilder()
- .setProject(key.project().get())
- .setATree(idConverter.toByteString(key.aTree()))
- .setBTree(idConverter.toByteString(key.bTree()))
- .setRenameScore(key.renameScore())
- .build());
- }
-
- @Override
- public Key deserialize(byte[] in) {
- GitModifiedFilesKeyProto proto =
- Protos.parseUnchecked(GitModifiedFilesKeyProto.parser(), in);
- ObjectIdConverter idConverter = ObjectIdConverter.create();
- return Key.builder()
- .project(Project.NameKey.parse(proto.getProject()))
- .aTree(idConverter.fromByteString(proto.getATree()))
- .bTree(idConverter.fromByteString(proto.getBTree()))
- .renameScore(proto.getRenameScore())
- .build();
- }
- }
- }
-
public enum ValueSerializer implements CacheSerializer<ImmutableList<ModifiedFile>> {
INSTANCE;
diff --git a/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCacheKey.java b/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCacheKey.java
new file mode 100644
index 0000000..f94f2c9
--- /dev/null
+++ b/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCacheKey.java
@@ -0,0 +1,137 @@
+// Copyright (C) 2020 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.google.gerrit.server.patch.gitdiff;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.Project.NameKey;
+import com.google.gerrit.proto.Protos;
+import com.google.gerrit.server.cache.proto.Cache.GitModifiedFilesKeyProto;
+import com.google.gerrit.server.cache.serialize.CacheSerializer;
+import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
+import com.google.gerrit.server.patch.DiffUtil;
+import java.io.IOException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+/** Cache key for the {@link GitModifiedFilesCache}. */
+@AutoValue
+public abstract class GitModifiedFilesCacheKey {
+
+ /** A specific git project / repository. */
+ public abstract Project.NameKey project();
+
+ /**
+ * The git SHA-1 {@link ObjectId} of the first git tree object for which the diff should be
+ * computed.
+ */
+ public abstract ObjectId aTree();
+
+ /**
+ * The git SHA-1 {@link ObjectId} of the second git tree object for which the diff should be
+ * computed.
+ */
+ public abstract ObjectId bTree();
+
+ /**
+ * Percentage score used to identify a file as a rename. This value is only available if {@link
+ * #renameDetection()} is true. Otherwise, this method will return -1.
+ *
+ * <p>This value will be used to set the rename score of {@link
+ * org.eclipse.jgit.diff.DiffFormatter#getRenameDetector()}.
+ */
+ public abstract int renameScore();
+
+ /** Returns true if rename detection was set for this key. */
+ public boolean renameDetection() {
+ return renameScore() != -1;
+ }
+
+ public static GitModifiedFilesCacheKey create(
+ Project.NameKey project, ObjectId aCommit, ObjectId bCommit, int renameScore, RevWalk rw)
+ throws IOException {
+ ObjectId aTree = DiffUtil.getTreeId(rw, aCommit);
+ ObjectId bTree = DiffUtil.getTreeId(rw, bCommit);
+ return builder().project(project).aTree(aTree).bTree(bTree).renameScore(renameScore).build();
+ }
+
+ public static Builder builder() {
+ return new AutoValue_GitModifiedFilesCacheKey.Builder();
+ }
+
+ /** Returns the size of the object in bytes */
+ public int weight() {
+ return stringSize(project().get())
+ + 20 * 2 // old and new tree IDs
+ + 4; // rename score
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+
+ public abstract Builder project(NameKey value);
+
+ public abstract Builder aTree(ObjectId value);
+
+ public abstract Builder bTree(ObjectId value);
+
+ public abstract Builder renameScore(int value);
+
+ public Builder disableRenameDetection() {
+ renameScore(-1);
+ return this;
+ }
+
+ public abstract GitModifiedFilesCacheKey build();
+ }
+
+ public enum Serializer implements CacheSerializer<GitModifiedFilesCacheKey> {
+ INSTANCE;
+
+ @Override
+ public byte[] serialize(GitModifiedFilesCacheKey key) {
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ return Protos.toByteArray(
+ GitModifiedFilesKeyProto.newBuilder()
+ .setProject(key.project().get())
+ .setATree(idConverter.toByteString(key.aTree()))
+ .setBTree(idConverter.toByteString(key.bTree()))
+ .setRenameScore(key.renameScore())
+ .build());
+ }
+
+ @Override
+ public GitModifiedFilesCacheKey deserialize(byte[] in) {
+ GitModifiedFilesKeyProto proto = Protos.parseUnchecked(GitModifiedFilesKeyProto.parser(), in);
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ return GitModifiedFilesCacheKey.builder()
+ .project(NameKey.parse(proto.getProject()))
+ .aTree(idConverter.fromByteString(proto.getATree()))
+ .bTree(idConverter.fromByteString(proto.getBTree()))
+ .renameScore(proto.getRenameScore())
+ .build();
+ }
+ }
+
+ private static int stringSize(String str) {
+ if (str != null) {
+ // each character in the string occupies 2 bytes. Ignoring the fixed overhead for the string
+ // (length, offset and hash code) since they are negligible and do not
+ // affect the comparison of 2 strings
+ return str.length() * 2;
+ }
+ return 0;
+ }
+}
diff --git a/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesWeigher.java b/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesWeigher.java
index f096cef..a678379 100644
--- a/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesWeigher.java
+++ b/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesWeigher.java
@@ -18,9 +18,9 @@
import com.google.common.collect.ImmutableList;
public class GitModifiedFilesWeigher
- implements Weigher<GitModifiedFilesCacheImpl.Key, ImmutableList<ModifiedFile>> {
+ implements Weigher<GitModifiedFilesCacheKey, ImmutableList<ModifiedFile>> {
@Override
- public int weigh(GitModifiedFilesCacheImpl.Key key, ImmutableList<ModifiedFile> modifiedFiles) {
+ public int weigh(GitModifiedFilesCacheKey key, ImmutableList<ModifiedFile> modifiedFiles) {
return key.weight() + modifiedFiles.stream().mapToInt(ModifiedFile::weight).sum();
}
}
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/GitModifiedFilesCacheKeySerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/GitModifiedFilesCacheKeySerializerTest.java
index 67edd55..caf1fbb 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/entities/GitModifiedFilesCacheKeySerializerTest.java
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/GitModifiedFilesCacheKeySerializerTest.java
@@ -17,8 +17,8 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.gerrit.entities.Project;
-import com.google.gerrit.server.patch.gitdiff.GitModifiedFilesCacheImpl;
-import com.google.gerrit.server.patch.gitdiff.GitModifiedFilesCacheImpl.Key.KeySerializer;
+import com.google.gerrit.server.patch.gitdiff.GitModifiedFilesCacheKey;
+import com.google.gerrit.server.patch.gitdiff.GitModifiedFilesCacheKey.Serializer;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
@@ -30,14 +30,14 @@
@Test
public void roundTrip() {
- GitModifiedFilesCacheImpl.Key key =
- GitModifiedFilesCacheImpl.Key.builder()
+ GitModifiedFilesCacheKey key =
+ GitModifiedFilesCacheKey.builder()
.project(Project.NameKey.parse("Project/X"))
.aTree(TREE_ID_1)
.bTree(TREE_ID_2)
.renameScore(65)
.build();
- byte[] serialized = KeySerializer.INSTANCE.serialize(key);
- assertThat(KeySerializer.INSTANCE.deserialize(serialized)).isEqualTo(key);
+ byte[] serialized = Serializer.INSTANCE.serialize(key);
+ assertThat(Serializer.INSTANCE.deserialize(serialized)).isEqualTo(key);
}
}
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/ModifiedFilesCacheKeySerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/ModifiedFilesCacheKeySerializerTest.java
index dbf11d4..b39ba57 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/entities/ModifiedFilesCacheKeySerializerTest.java
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/ModifiedFilesCacheKeySerializerTest.java
@@ -17,7 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.gerrit.entities.Project;
-import com.google.gerrit.server.patch.diff.ModifiedFilesCacheImpl;
+import com.google.gerrit.server.patch.diff.ModifiedFilesCacheKey;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
@@ -29,15 +29,14 @@
@Test
public void roundTrip() {
- ModifiedFilesCacheImpl.Key key =
- ModifiedFilesCacheImpl.Key.builder()
+ ModifiedFilesCacheKey key =
+ ModifiedFilesCacheKey.builder()
.project(Project.NameKey.parse("Project/X"))
.aCommit(COMMIT_ID_1)
.bCommit(COMMIT_ID_2)
.renameScore(65)
.build();
- byte[] serialized = ModifiedFilesCacheImpl.Key.Serializer.INSTANCE.serialize(key);
- assertThat(ModifiedFilesCacheImpl.Key.Serializer.INSTANCE.deserialize(serialized))
- .isEqualTo(key);
+ byte[] serialized = ModifiedFilesCacheKey.Serializer.INSTANCE.serialize(key);
+ assertThat(ModifiedFilesCacheKey.Serializer.INSTANCE.deserialize(serialized)).isEqualTo(key);
}
}
diff --git a/proto/cache.proto b/proto/cache.proto
index e7fbe6d..aa71b87 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -528,7 +528,7 @@
}
// Serialized key for
-// com.google.gerrit.server.patch.gitdiff.GitModifiedCacheImpl.Key
+// com.google.gerrit.server.patch.gitdiff.GitModifiedFilesCacheKey
// Next ID: 5
message GitModifiedFilesKeyProto {
string project = 1;
@@ -537,6 +537,16 @@
int32 rename_score = 4;
}
+// Serialized key for
+// com.google.gerrit.server.patch.diff.ModifiedFilesCacheKey
+// Next ID: 5
+message ModifiedFilesKeyProto {
+ string project = 1;
+ bytes a_commit = 2; // SHA-1 hash of the left commit ID in the diff
+ bytes b_commit = 3; // SHA-1 hash of the right commit ID in the diff
+ int32 rename_score = 4;
+}
+
// Serialized form of com.google.gerrit.server.patch.gitdiff.ModifiedFile
// Next ID: 4
message ModifiedFileProto {
