Merge changes Ia9009deb,I7ae66ac8
* changes:
Convert ChangeKindCacheImpl.Key to AutoValue
ChangeKindCacheImpl: Don't use reference equality for trees
diff --git a/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java b/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
index b2eb62d..f3ab847 100644
--- a/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
+++ b/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
@@ -15,8 +15,8 @@
package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkNotNull;
+import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.cache.Cache;
import com.google.common.cache.Weigher;
@@ -50,6 +50,7 @@
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import org.eclipse.jgit.errors.LargeObjectException;
+import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
@@ -104,7 +105,7 @@
ObjectId prior,
ObjectId next) {
try {
- Key key = new Key(prior, next, useRecursiveMerge);
+ Key key = Key.create(prior, next, useRecursiveMerge);
return new Loader(key, repoManager, project, rw, repoConfig).call();
} catch (IOException e) {
log.warn(
@@ -125,52 +126,21 @@
}
}
- public static class Key {
- private transient ObjectId prior;
- private transient ObjectId next;
- private transient String strategyName;
-
- private Key(ObjectId prior, ObjectId next, boolean useRecursiveMerge) {
- checkNotNull(next, "next");
- String strategyName = MergeUtil.mergeStrategyName(true, useRecursiveMerge);
- this.prior = prior.copy();
- this.next = next.copy();
- this.strategyName = strategyName;
+ @AutoValue
+ public abstract static class Key {
+ public static Key create(AnyObjectId prior, AnyObjectId next, String strategyName) {
+ return new AutoValue_ChangeKindCacheImpl_Key(prior.copy(), next.copy(), strategyName);
}
- public Key(ObjectId prior, ObjectId next, String strategyName) {
- this.prior = prior;
- this.next = next;
- this.strategyName = strategyName;
+ private static Key create(AnyObjectId prior, AnyObjectId next, boolean useRecursiveMerge) {
+ return create(prior, next, MergeUtil.mergeStrategyName(true, useRecursiveMerge));
}
- public ObjectId getPrior() {
- return prior;
- }
+ public abstract ObjectId prior();
- public ObjectId getNext() {
- return next;
- }
+ public abstract ObjectId next();
- public String getStrategyName() {
- return strategyName;
- }
-
- @Override
- public boolean equals(Object o) {
- if (o instanceof Key) {
- Key k = (Key) o;
- return Objects.equals(prior, k.prior)
- && Objects.equals(next, k.next)
- && Objects.equals(strategyName, k.strategyName);
- }
- return false;
- }
-
- @Override
- public int hashCode() {
- return Objects.hash(prior, next, strategyName);
- }
+ public abstract String strategyName();
@VisibleForTesting
static class Serializer implements CacheSerializer<Key> {
@@ -179,9 +149,9 @@
ObjectIdConverter idConverter = ObjectIdConverter.create();
return ProtoCacheSerializers.toByteArray(
ChangeKindKeyProto.newBuilder()
- .setPrior(idConverter.toByteString(object.getPrior()))
- .setNext(idConverter.toByteString(object.getNext()))
- .setStrategyName(object.getStrategyName())
+ .setPrior(idConverter.toByteString(object.prior()))
+ .setNext(idConverter.toByteString(object.next()))
+ .setStrategyName(object.strategyName())
.build());
}
@@ -190,7 +160,7 @@
ChangeKindKeyProto proto =
ProtoCacheSerializers.parseUnchecked(ChangeKindKeyProto.parser(), in);
ObjectIdConverter idConverter = ObjectIdConverter.create();
- return new Key(
+ return create(
idConverter.fromByteString(proto.getPrior()),
idConverter.fromByteString(proto.getNext()),
proto.getStrategyName());
@@ -226,7 +196,7 @@
@SuppressWarnings("resource") // Resources are manually managed.
@Override
public ChangeKind call() throws IOException {
- if (Objects.equals(key.prior, key.next)) {
+ if (Objects.equals(key.prior(), key.next())) {
return ChangeKind.NO_CODE_CHANGE;
}
@@ -239,9 +209,9 @@
config = repo.getConfig();
}
try {
- RevCommit prior = rw.parseCommit(key.prior);
+ RevCommit prior = rw.parseCommit(key.prior());
rw.parseBody(prior);
- RevCommit next = rw.parseCommit(key.next);
+ RevCommit next = rw.parseCommit(key.next());
rw.parseBody(next);
if (!next.getFullMessage().equals(prior.getFullMessage())) {
@@ -272,7 +242,7 @@
// having the same tree as would exist when the prior commit is
// cherry-picked onto the next commit's new first parent.
try (ObjectInserter ins = new InMemoryInserter(rw.getObjectReader())) {
- ThreeWayMerger merger = MergeUtil.newThreeWayMerger(ins, config, key.strategyName);
+ ThreeWayMerger merger = MergeUtil.newThreeWayMerger(ins, config, key.strategyName());
merger.setBase(prior.getParent(0));
if (merger.merge(next.getParent(0), prior)
&& merger.getResultTreeId().equals(next.getTree())) {
@@ -316,7 +286,7 @@
}
private static boolean isSameDeltaAndTree(RevCommit prior, RevCommit next) {
- if (next.getTree() != prior.getTree()) {
+ if (!Objects.equals(next.getTree(), prior.getTree())) {
return false;
}
@@ -329,7 +299,7 @@
// Make sure that the prior/next delta is the same - not just the tree.
// This is done by making sure that the parent trees are equal.
for (int i = 0; i < prior.getParentCount(); i++) {
- if (next.getParent(i).getTree() != prior.getParent(i).getTree()) {
+ if (!Objects.equals(next.getParent(i).getTree(), prior.getParent(i).getTree())) {
return false;
}
}
@@ -342,7 +312,7 @@
public int weigh(Key key, ChangeKind changeKind) {
return 16
+ 2 * 36
- + 2 * key.strategyName.length() // Size of Key, 64 bit JVM
+ + 2 * key.strategyName().length() // Size of Key, 64 bit JVM
+ 2 * changeKind.name().length(); // Size of ChangeKind, 64 bit JVM
}
}
@@ -372,7 +342,7 @@
ObjectId prior,
ObjectId next) {
try {
- Key key = new Key(prior, next, useRecursiveMerge);
+ Key key = Key.create(prior, next, useRecursiveMerge);
return cache.get(key, new Loader(key, repoManager, project, rw, repoConfig));
} catch (ExecutionException e) {
log.warn("Cannot check trivial rebase of new patch set " + next.name() + " in " + project, e);
diff --git a/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java b/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
index b0d7ae4..03e0d4e 100644
--- a/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
+++ b/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
@@ -22,6 +22,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.server.cache.CacheSerializer;
import com.google.gerrit.server.cache.proto.Cache.ChangeKindKeyProto;
+import com.google.gerrit.server.change.ChangeKindCacheImpl.Key;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
@@ -29,7 +30,7 @@
@Test
public void keySerializer() throws Exception {
ChangeKindCacheImpl.Key key =
- new ChangeKindCacheImpl.Key(
+ Key.create(
ObjectId.zeroId(),
ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"),
"aStrategy");
@@ -55,7 +56,7 @@
@Test
public void keyFields() throws Exception {
assertThatSerializedClass(ChangeKindCacheImpl.Key.class)
- .hasFields(
+ .hasAutoValueMethods(
ImmutableMap.of(
"prior", ObjectId.class, "next", ObjectId.class, "strategyName", String.class));
}
