From 6d1df18939c202100d0c52dcf48c08c3cb405471 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Thu, 24 May 2018 08:03:39 -0400 Subject: [PATCH] Convert ChangeKindCacheImpl.Key to AutoValue Change-Id: Ia9009debab17f60af5ff289fcd8ef1c6c4fa484f --- .../server/change/ChangeKindCacheImpl.java | 74 ++++++------------- .../change/ChangeKindCacheImplTest.java | 5 +- 2 files changed, 25 insertions(+), 54 deletions(-) diff --git a/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java b/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java index f84c585f25..f3ab847075 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.Set; 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 @@ public class ChangeKindCacheImpl implements ChangeKindCache { 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 class ChangeKindCacheImpl implements ChangeKindCache { } } - 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 { @@ -179,9 +149,9 @@ public class ChangeKindCacheImpl implements ChangeKindCache { 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 @@ public class ChangeKindCacheImpl implements ChangeKindCache { 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 @@ public class ChangeKindCacheImpl implements ChangeKindCache { @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 @@ public class ChangeKindCacheImpl implements ChangeKindCache { 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 @@ public class ChangeKindCacheImpl implements ChangeKindCache { // 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())) { @@ -342,7 +312,7 @@ public class ChangeKindCacheImpl implements ChangeKindCache { 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 @@ public class ChangeKindCacheImpl implements ChangeKindCache { 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 b0d7ae4522..03e0d4e615 100644 --- a/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java +++ b/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java @@ -22,6 +22,7 @@ import static com.google.gerrit.server.cache.testing.SerializedClassSubject.asse 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 @@ public class ChangeKindCacheImplTest { @Test public void keySerializer() throws Exception { ChangeKindCacheImpl.Key key = - new ChangeKindCacheImpl.Key( + Key.create( ObjectId.zeroId(), ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"), "aStrategy"); @@ -55,7 +56,7 @@ public class ChangeKindCacheImplTest { @Test public void keyFields() throws Exception { assertThatSerializedClass(ChangeKindCacheImpl.Key.class) - .hasFields( + .hasAutoValueMethods( ImmutableMap.of( "prior", ObjectId.class, "next", ObjectId.class, "strategyName", String.class)); }