Merge changes I293cffba,Iec78e15c,I89cf813e,Ibc713068,I7b5d6a5a
* changes:
Serialize ConflictsCache as proto
ConflictKey: Add tests for order normalizing behavior
Add helper method for parsing protos with unchecked exceptions
Add a helper for serializing ObjectIds to protobuf
Use ProtoTruth#assertThat in more places
diff --git a/java/com/google/gerrit/server/auth/oauth/OAuthTokenCache.java b/java/com/google/gerrit/server/auth/oauth/OAuthTokenCache.java
index f380051..13a09a1 100644
--- a/java/com/google/gerrit/server/auth/oauth/OAuthTokenCache.java
+++ b/java/com/google/gerrit/server/auth/oauth/OAuthTokenCache.java
@@ -32,7 +32,6 @@
import com.google.inject.Module;
import com.google.inject.Singleton;
import com.google.inject.name.Named;
-import java.io.IOException;
@Singleton
public class OAuthTokenCache {
@@ -70,12 +69,7 @@
@Override
public OAuthToken deserialize(byte[] in) {
- OAuthTokenProto proto;
- try {
- proto = OAuthTokenProto.parseFrom(in);
- } catch (IOException e) {
- throw new IllegalArgumentException("failed to deserialize OAuthToken");
- }
+ OAuthTokenProto proto = ProtoCacheSerializers.parseUnchecked(OAuthTokenProto.parser(), in);
return new OAuthToken(
proto.getToken(),
proto.getSecret(),
diff --git a/java/com/google/gerrit/server/cache/ProtoCacheSerializers.java b/java/com/google/gerrit/server/cache/ProtoCacheSerializers.java
index 9fe6b83..c6fc0b9 100644
--- a/java/com/google/gerrit/server/cache/ProtoCacheSerializers.java
+++ b/java/com/google/gerrit/server/cache/ProtoCacheSerializers.java
@@ -14,11 +14,16 @@
package com.google.gerrit.server.cache;
+import static com.google.common.base.Preconditions.checkArgument;
+import static org.eclipse.jgit.lib.Constants.OBJECT_ID_LENGTH;
+
import com.google.gwtorm.protobuf.ProtobufCodec;
import com.google.protobuf.ByteString;
import com.google.protobuf.CodedOutputStream;
import com.google.protobuf.MessageLite;
+import com.google.protobuf.Parser;
import java.io.IOException;
+import org.eclipse.jgit.lib.ObjectId;
/** Static utilities for writing protobuf-based {@link CacheSerializer} implementations. */
public class ProtoCacheSerializers {
@@ -68,5 +73,55 @@
}
}
+ /**
+ * Parses a byte array to a protobuf message.
+ *
+ * @param parser parser for the proto type.
+ * @param in byte array with the message contents.
+ * @return parsed proto.
+ */
+ public static <M extends MessageLite> M parseUnchecked(Parser<M> parser, byte[] in) {
+ try {
+ return parser.parseFrom(in);
+ } catch (IOException e) {
+ throw new IllegalArgumentException("exception parsing byte array to proto", e);
+ }
+ }
+
+ /**
+ * Helper for serializing {@link ObjectId} instances to/from protobuf fields.
+ *
+ * <p>Reuse a single instance's {@link #toByteString(ObjectId)} and {@link
+ * #fromByteString(ByteString)} within a single {@link CacheSerializer#serialize} or {@link
+ * CacheSerializer#deserialize} method body to minimize allocation of temporary buffers.
+ *
+ * <p><strong>Note:</strong> This class is not threadsafe. Instances must not be stored in {@link
+ * CacheSerializer} fields if the serializer instances will be used from multiple threads.
+ */
+ public static class ObjectIdConverter {
+ public static ObjectIdConverter create() {
+ return new ObjectIdConverter();
+ }
+
+ private final byte[] buf = new byte[OBJECT_ID_LENGTH];
+
+ private ObjectIdConverter() {}
+
+ public ByteString toByteString(ObjectId id) {
+ id.copyRawTo(buf, 0);
+ return ByteString.copyFrom(buf);
+ }
+
+ public ObjectId fromByteString(ByteString in) {
+ checkArgument(
+ in.size() == OBJECT_ID_LENGTH,
+ "expected ByteString of length %s: %s",
+ OBJECT_ID_LENGTH,
+ in);
+ in.copyTo(buf, 0);
+ return ObjectId.fromRaw(buf);
+ }
+ }
+
private ProtoCacheSerializers() {}
}
diff --git a/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java b/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
index a4eb90f..b2eb62d 100644
--- a/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
+++ b/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
@@ -31,6 +31,7 @@
import com.google.gerrit.server.cache.CacheSerializer;
import com.google.gerrit.server.cache.EnumCacheSerializer;
import com.google.gerrit.server.cache.ProtoCacheSerializers;
+import com.google.gerrit.server.cache.ProtoCacheSerializers.ObjectIdConverter;
import com.google.gerrit.server.cache.proto.Cache.ChangeKindKeyProto;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -41,8 +42,6 @@
import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.name.Named;
-import com.google.protobuf.ByteString;
-import com.google.protobuf.InvalidProtocolBufferException;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
@@ -52,7 +51,6 @@
import java.util.concurrent.ExecutionException;
import org.eclipse.jgit.errors.LargeObjectException;
import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.Repository;
@@ -178,27 +176,24 @@
static class Serializer implements CacheSerializer<Key> {
@Override
public byte[] serialize(Key object) {
- byte[] buf = new byte[Constants.OBJECT_ID_LENGTH];
- ChangeKindKeyProto.Builder b = ChangeKindKeyProto.newBuilder();
- object.getPrior().copyRawTo(buf, 0);
- b.setPrior(ByteString.copyFrom(buf));
- object.getNext().copyRawTo(buf, 0);
- b.setNext(ByteString.copyFrom(buf));
- b.setStrategyName(object.getStrategyName());
- return ProtoCacheSerializers.toByteArray(b.build());
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ return ProtoCacheSerializers.toByteArray(
+ ChangeKindKeyProto.newBuilder()
+ .setPrior(idConverter.toByteString(object.getPrior()))
+ .setNext(idConverter.toByteString(object.getNext()))
+ .setStrategyName(object.getStrategyName())
+ .build());
}
@Override
public Key deserialize(byte[] in) {
- try {
- ChangeKindKeyProto proto = ChangeKindKeyProto.parseFrom(in);
- return new Key(
- ObjectId.fromRaw(proto.getPrior().toByteArray()),
- ObjectId.fromRaw(proto.getNext().toByteArray()),
- proto.getStrategyName());
- } catch (InvalidProtocolBufferException e) {
- throw new IllegalArgumentException("Failed to deserialize object", e);
- }
+ ChangeKindKeyProto proto =
+ ProtoCacheSerializers.parseUnchecked(ChangeKindKeyProto.parser(), in);
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ return new Key(
+ idConverter.fromByteString(proto.getPrior()),
+ idConverter.fromByteString(proto.getNext()),
+ proto.getStrategyName());
}
}
}
diff --git a/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java b/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java
index a192228..b57be15 100644
--- a/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java
+++ b/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java
@@ -29,6 +29,7 @@
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.cache.CacheSerializer;
import com.google.gerrit.server.cache.ProtoCacheSerializers;
+import com.google.gerrit.server.cache.ProtoCacheSerializers.ObjectIdConverter;
import com.google.gerrit.server.cache.proto.Cache.MergeabilityKeyProto;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
@@ -37,13 +38,10 @@
import com.google.inject.Module;
import com.google.inject.Singleton;
import com.google.inject.name.Named;
-import com.google.protobuf.ByteString;
-import java.io.IOException;
import java.util.Arrays;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ExecutionException;
-import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@@ -145,33 +143,24 @@
@Override
public byte[] serialize(EntryKey object) {
- byte[] buf = new byte[Constants.OBJECT_ID_LENGTH];
- MergeabilityKeyProto.Builder b = MergeabilityKeyProto.newBuilder();
- object.getCommit().copyRawTo(buf, 0);
- b.setCommit(ByteString.copyFrom(buf));
- object.getInto().copyRawTo(buf, 0);
- b.setInto(ByteString.copyFrom(buf));
- b.setSubmitType(SUBMIT_TYPE_CONVERTER.reverse().convert(object.getSubmitType()));
- b.setMergeStrategy(object.getMergeStrategy());
- return ProtoCacheSerializers.toByteArray(b.build());
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ return ProtoCacheSerializers.toByteArray(
+ MergeabilityKeyProto.newBuilder()
+ .setCommit(idConverter.toByteString(object.getCommit()))
+ .setInto(idConverter.toByteString(object.getInto()))
+ .setSubmitType(SUBMIT_TYPE_CONVERTER.reverse().convert(object.getSubmitType()))
+ .setMergeStrategy(object.getMergeStrategy())
+ .build());
}
@Override
public EntryKey deserialize(byte[] in) {
- MergeabilityKeyProto proto;
- try {
- proto = MergeabilityKeyProto.parseFrom(in);
- } catch (IOException e) {
- throw new IllegalArgumentException("Failed to deserialize mergeability cache key");
- }
- byte[] buf = new byte[Constants.OBJECT_ID_LENGTH];
- proto.getCommit().copyTo(buf, 0);
- ObjectId commit = ObjectId.fromRaw(buf);
- proto.getInto().copyTo(buf, 0);
- ObjectId into = ObjectId.fromRaw(buf);
+ MergeabilityKeyProto proto =
+ ProtoCacheSerializers.parseUnchecked(MergeabilityKeyProto.parser(), in);
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
return new EntryKey(
- commit,
- into,
+ idConverter.fromByteString(proto.getCommit()),
+ idConverter.fromByteString(proto.getInto()),
SUBMIT_TYPE_CONVERTER.convert(proto.getSubmitType()),
proto.getMergeStrategy());
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index d1c28c4..06d940e 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -27,6 +27,7 @@
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.cache.CacheSerializer;
import com.google.gerrit.server.cache.ProtoCacheSerializers;
+import com.google.gerrit.server.cache.ProtoCacheSerializers.ObjectIdConverter;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesKeyProto;
import com.google.gerrit.server.notedb.AbstractChangeNotes.Args;
import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
@@ -34,7 +35,6 @@
import com.google.inject.Module;
import com.google.inject.Singleton;
import com.google.inject.name.Named;
-import com.google.protobuf.ByteString;
import java.io.IOException;
import java.util.List;
import java.util.Map;
@@ -42,7 +42,6 @@
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
@Singleton
@@ -83,28 +82,22 @@
@Override
public byte[] serialize(Key object) {
- byte[] buf = new byte[Constants.OBJECT_ID_LENGTH];
- object.id().copyRawTo(buf, 0);
return ProtoCacheSerializers.toByteArray(
ChangeNotesKeyProto.newBuilder()
.setProject(object.project().get())
.setChangeId(object.changeId().get())
- .setId(ByteString.copyFrom(buf))
+ .setId(ObjectIdConverter.create().toByteString(object.id()))
.build());
}
@Override
public Key deserialize(byte[] in) {
- ChangeNotesKeyProto proto;
- try {
- proto = ChangeNotesKeyProto.parseFrom(in);
- } catch (IOException e) {
- throw new IllegalArgumentException("Failed to deserialize " + Key.class.getName());
- }
+ ChangeNotesKeyProto proto =
+ ProtoCacheSerializers.parseUnchecked(ChangeNotesKeyProto.parser(), in);
return Key.create(
new Project.NameKey(proto.getProject()),
new Change.Id(proto.getChangeId()),
- ObjectId.fromRaw(proto.getId().toByteArray()));
+ ObjectIdConverter.create().fromByteString(proto.getId()));
}
}
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index 1b09494..3eb06b2 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -54,6 +54,7 @@
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.cache.CacheSerializer;
import com.google.gerrit.server.cache.ProtoCacheSerializers;
+import com.google.gerrit.server.cache.ProtoCacheSerializers.ObjectIdConverter;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ChangeColumnsProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto;
@@ -63,13 +64,11 @@
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gson.Gson;
-import com.google.protobuf.ByteString;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
/**
@@ -447,9 +446,7 @@
checkArgument(object.columns() != null, "ChangeColumns is required in: %s", object);
ChangeNotesStateProto.Builder b = ChangeNotesStateProto.newBuilder();
- byte[] idBuf = new byte[Constants.OBJECT_ID_LENGTH];
- object.metaId().copyRawTo(idBuf, 0);
- b.setMetaId(ByteString.copyFrom(idBuf))
+ b.setMetaId(ObjectIdConverter.create().toByteString(object.metaId()))
.setChangeId(object.changeId().get())
.setColumns(toChangeColumnsProto(object.columns()));
@@ -555,18 +552,13 @@
@Override
public ChangeNotesState deserialize(byte[] in) {
- ChangeNotesStateProto proto;
- try {
- proto = ChangeNotesStateProto.parseFrom(in);
- } catch (IOException e) {
- throw new IllegalArgumentException(
- "Failed to deserialize " + ChangeNotesState.class.getName());
- }
+ ChangeNotesStateProto proto =
+ ProtoCacheSerializers.parseUnchecked(ChangeNotesStateProto.parser(), in);
Change.Id changeId = new Change.Id(proto.getChangeId());
ChangeNotesState.Builder b =
builder()
- .metaId(ObjectId.fromRaw(proto.getMetaId().toByteArray()))
+ .metaId(ObjectIdConverter.create().fromByteString(proto.getMetaId()))
.changeId(changeId)
.columns(toChangeColumns(changeId, proto.getColumns()))
.pastAssignees(
diff --git a/java/com/google/gerrit/server/query/change/ConflictKey.java b/java/com/google/gerrit/server/query/change/ConflictKey.java
index 0101ffe..9daf886 100644
--- a/java/com/google/gerrit/server/query/change/ConflictKey.java
+++ b/java/com/google/gerrit/server/query/change/ConflictKey.java
@@ -14,62 +14,80 @@
package com.google.gerrit.server.query.change;
+import com.google.auto.value.AutoValue;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Converter;
+import com.google.common.base.Enums;
+import com.google.common.collect.Ordering;
import com.google.gerrit.extensions.client.SubmitType;
-import java.io.Serializable;
-import java.util.Objects;
+import com.google.gerrit.server.cache.CacheSerializer;
+import com.google.gerrit.server.cache.ProtoCacheSerializers;
+import com.google.gerrit.server.cache.ProtoCacheSerializers.ObjectIdConverter;
+import com.google.gerrit.server.cache.proto.Cache.ConflictKeyProto;
+import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
-public class ConflictKey implements Serializable {
- private static final long serialVersionUID = 2L;
-
- private final ObjectId commit;
- private final ObjectId otherCommit;
- private final SubmitType submitType;
- private final boolean contentMerge;
-
- public ConflictKey(
- ObjectId commit, ObjectId otherCommit, SubmitType submitType, boolean contentMerge) {
- if (SubmitType.FAST_FORWARD_ONLY.equals(submitType) || commit.compareTo(otherCommit) < 0) {
- this.commit = commit;
- this.otherCommit = otherCommit;
- } else {
- this.commit = otherCommit;
- this.otherCommit = commit;
+@AutoValue
+public abstract class ConflictKey {
+ public static ConflictKey create(
+ AnyObjectId commit, AnyObjectId otherCommit, SubmitType submitType, boolean contentMerge) {
+ ObjectId commitCopy = commit.copy();
+ ObjectId otherCommitCopy = otherCommit.copy();
+ if (submitType == SubmitType.FAST_FORWARD_ONLY) {
+ // The conflict check for FF-only is non-symmetrical, and we need to treat (X, Y) differently
+ // from (Y, X). Store the commits in the input order.
+ return new AutoValue_ConflictKey(commitCopy, otherCommitCopy, submitType, contentMerge);
}
- this.submitType = submitType;
- this.contentMerge = contentMerge;
+ // Otherwise, the check is symmetrical; sort commit/otherCommit before storing, so the actual
+ // key is independent of the order in which they are passed to this method.
+ return new AutoValue_ConflictKey(
+ Ordering.natural().min(commitCopy, otherCommitCopy),
+ Ordering.natural().max(commitCopy, otherCommitCopy),
+ submitType,
+ contentMerge);
}
- public ObjectId getCommit() {
- return commit;
+ @VisibleForTesting
+ static ConflictKey createWithoutNormalization(
+ AnyObjectId commit, AnyObjectId otherCommit, SubmitType submitType, boolean contentMerge) {
+ return new AutoValue_ConflictKey(commit.copy(), otherCommit.copy(), submitType, contentMerge);
}
- public ObjectId getOtherCommit() {
- return otherCommit;
- }
+ public abstract ObjectId commit();
- public SubmitType getSubmitType() {
- return submitType;
- }
+ public abstract ObjectId otherCommit();
- public boolean isContentMerge() {
- return contentMerge;
- }
+ public abstract SubmitType submitType();
- @Override
- public boolean equals(Object o) {
- if (!(o instanceof ConflictKey)) {
- return false;
+ public abstract boolean contentMerge();
+
+ public static enum Serializer implements CacheSerializer<ConflictKey> {
+ INSTANCE;
+
+ private static final Converter<String, SubmitType> SUBMIT_TYPE_CONVERTER =
+ Enums.stringConverter(SubmitType.class);
+
+ @Override
+ public byte[] serialize(ConflictKey object) {
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ return ProtoCacheSerializers.toByteArray(
+ ConflictKeyProto.newBuilder()
+ .setCommit(idConverter.toByteString(object.commit()))
+ .setOtherCommit(idConverter.toByteString(object.otherCommit()))
+ .setSubmitType(SUBMIT_TYPE_CONVERTER.reverse().convert(object.submitType()))
+ .setContentMerge(object.contentMerge())
+ .build());
}
- ConflictKey other = (ConflictKey) o;
- return commit.equals(other.commit)
- && otherCommit.equals(other.otherCommit)
- && submitType.equals(other.submitType)
- && contentMerge == other.contentMerge;
- }
- @Override
- public int hashCode() {
- return Objects.hash(commit, otherCommit, submitType, contentMerge);
+ @Override
+ public ConflictKey deserialize(byte[] in) {
+ ConflictKeyProto proto = ProtoCacheSerializers.parseUnchecked(ConflictKeyProto.parser(), in);
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ return create(
+ idConverter.fromByteString(proto.getCommit()),
+ idConverter.fromByteString(proto.getOtherCommit()),
+ SUBMIT_TYPE_CONVERTER.convert(proto.getSubmitType()),
+ proto.getContentMerge());
+ }
}
}
diff --git a/java/com/google/gerrit/server/query/change/ConflictsCache.java b/java/com/google/gerrit/server/query/change/ConflictsCache.java
index e8b2fef..c7ee79b 100644
--- a/java/com/google/gerrit/server/query/change/ConflictsCache.java
+++ b/java/com/google/gerrit/server/query/change/ConflictsCache.java
@@ -18,7 +18,7 @@
public interface ConflictsCache {
- void put(ConflictKey key, Boolean value);
+ void put(ConflictKey key, boolean value);
@Nullable
Boolean getIfPresent(ConflictKey key);
diff --git a/java/com/google/gerrit/server/query/change/ConflictsCacheImpl.java b/java/com/google/gerrit/server/query/change/ConflictsCacheImpl.java
index 1185677..0b8c5ee 100644
--- a/java/com/google/gerrit/server/query/change/ConflictsCacheImpl.java
+++ b/java/com/google/gerrit/server/query/change/ConflictsCacheImpl.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.query.change;
import com.google.common.cache.Cache;
+import com.google.gerrit.server.cache.BooleanCacheSerializer;
import com.google.gerrit.server.cache.CacheModule;
import com.google.inject.Inject;
import com.google.inject.Module;
@@ -29,7 +30,11 @@
return new CacheModule() {
@Override
protected void configure() {
- persist(NAME, ConflictKey.class, Boolean.class).maximumWeight(37400);
+ persist(NAME, ConflictKey.class, Boolean.class)
+ .version(1)
+ .keySerializer(ConflictKey.Serializer.INSTANCE)
+ .valueSerializer(BooleanCacheSerializer.INSTANCE)
+ .maximumWeight(37400);
bind(ConflictsCache.class).to(ConflictsCacheImpl.class);
}
};
@@ -43,7 +48,7 @@
}
@Override
- public void put(ConflictKey key, Boolean value) {
+ public void put(ConflictKey key, boolean value) {
conflictsCache.put(key, value);
}
diff --git a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
index f870951..7dc7a0b 100644
--- a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
@@ -115,19 +115,19 @@
ObjectId other = ObjectId.fromString(object.currentPatchSet().getRevision().get());
ConflictKey conflictsKey =
- new ConflictKey(
+ ConflictKey.create(
changeDataCache.getTestAgainst(),
other,
str.type,
projectState.is(BooleanProjectConfig.USE_CONTENT_MERGE));
- Boolean conflicts = args.conflictsCache.getIfPresent(conflictsKey);
- if (conflicts != null) {
- return conflicts;
+ Boolean maybeConflicts = args.conflictsCache.getIfPresent(conflictsKey);
+ if (maybeConflicts != null) {
+ return maybeConflicts;
}
try (Repository repo = args.repoManager.openRepository(otherChange.getProject());
CodeReviewRevWalk rw = CodeReviewCommit.newRevWalk(repo)) {
- conflicts =
+ boolean conflicts =
!args.submitDryRun.run(
str.type,
repo,
diff --git a/javatests/com/google/gerrit/server/auth/oauth/OAuthTokenCacheTest.java b/javatests/com/google/gerrit/server/auth/oauth/OAuthTokenCacheTest.java
index 586c065..5e93a09 100644
--- a/javatests/com/google/gerrit/server/auth/oauth/OAuthTokenCacheTest.java
+++ b/javatests/com/google/gerrit/server/auth/oauth/OAuthTokenCacheTest.java
@@ -1,6 +1,7 @@
package com.google.gerrit.server.auth.oauth;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
import static com.google.gerrit.server.cache.testing.SerializedClassSubject.assertThatSerializedClass;
import com.google.common.collect.ImmutableMap;
diff --git a/javatests/com/google/gerrit/server/cache/BUILD b/javatests/com/google/gerrit/server/cache/BUILD
index 278330b..ab88169 100644
--- a/javatests/com/google/gerrit/server/cache/BUILD
+++ b/javatests/com/google/gerrit/server/cache/BUILD
@@ -5,12 +5,16 @@
srcs = glob(["*.java"]),
deps = [
"//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/server/cache/testing",
"//lib:guava",
"//lib:gwtorm",
"//lib:junit",
"//lib:protobuf",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
+ "//lib/jgit/org.eclipse.jgit:jgit",
"//lib/truth",
+ "//lib/truth:truth-proto-extension",
+ "//proto:cache_java_proto",
],
)
diff --git a/javatests/com/google/gerrit/server/cache/ProtoCacheSerializersTest.java b/javatests/com/google/gerrit/server/cache/ProtoCacheSerializersTest.java
new file mode 100644
index 0000000..8bf9762
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/ProtoCacheSerializersTest.java
@@ -0,0 +1,116 @@
+// Copyright (C) 2018 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.cache;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
+import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
+import static com.google.gerrit.server.cache.testing.CacheSerializerTestUtil.bytes;
+
+import com.google.gerrit.server.cache.ProtoCacheSerializers.ObjectIdConverter;
+import com.google.gerrit.server.cache.proto.Cache.ChangeNotesKeyProto;
+import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto;
+import com.google.protobuf.ByteString;
+import org.eclipse.jgit.lib.ObjectId;
+import org.junit.Test;
+
+public class ProtoCacheSerializersTest {
+ @Test
+ public void objectIdFromByteString() {
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ assertThat(
+ idConverter.fromByteString(
+ bytes(
+ 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
+ 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa)))
+ .isEqualTo(ObjectId.fromString("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"));
+ assertThat(
+ idConverter.fromByteString(
+ bytes(
+ 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb,
+ 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb)))
+ .isEqualTo(ObjectId.fromString("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"));
+ }
+
+ @Test
+ public void objectIdFromByteStringWrongSize() {
+ try {
+ ObjectIdConverter.create().fromByteString(ByteString.copyFromUtf8("foo"));
+ assert_().fail("expected IllegalArgumentException");
+ } catch (IllegalArgumentException e) {
+ // Expected.
+ }
+ }
+
+ @Test
+ public void objectIdToByteString() {
+ ObjectIdConverter idConverter = ObjectIdConverter.create();
+ assertThat(
+ idConverter.toByteString(
+ ObjectId.fromString("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")))
+ .isEqualTo(
+ bytes(
+ 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
+ 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa));
+ assertThat(
+ idConverter.toByteString(
+ ObjectId.fromString("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")))
+ .isEqualTo(
+ bytes(
+ 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb,
+ 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb));
+ }
+
+ @Test
+ public void parseUncheckedWrongProtoType() {
+ ChangeNotesKeyProto proto =
+ ChangeNotesKeyProto.newBuilder()
+ .setProject("project")
+ .setChangeId(1234)
+ .setId(ByteString.copyFromUtf8("foo"))
+ .build();
+ byte[] bytes = ProtoCacheSerializers.toByteArray(proto);
+ try {
+ ProtoCacheSerializers.parseUnchecked(ChangeNotesStateProto.parser(), bytes);
+ assert_().fail("expected IllegalArgumentException");
+ } catch (IllegalArgumentException e) {
+ // Expected.
+ }
+ }
+
+ @Test
+ public void parseUncheckedInvalidData() {
+ byte[] bytes = new byte[] {0x00};
+ try {
+ ProtoCacheSerializers.parseUnchecked(ChangeNotesStateProto.parser(), bytes);
+ assert_().fail("expected IllegalArgumentException");
+ } catch (IllegalArgumentException e) {
+ // Expected.
+ }
+ }
+
+ @Test
+ public void parseUnchecked() {
+ ChangeNotesKeyProto proto =
+ ChangeNotesKeyProto.newBuilder()
+ .setProject("project")
+ .setChangeId(1234)
+ .setId(ByteString.copyFromUtf8("foo"))
+ .build();
+ byte[] bytes = ProtoCacheSerializers.toByteArray(proto);
+ assertThat(ProtoCacheSerializers.parseUnchecked(ChangeNotesKeyProto.parser(), bytes))
+ .isEqualTo(proto);
+ }
+}
diff --git a/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java b/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
index 5b77094..b0d7ae4 100644
--- a/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
+++ b/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.change;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
import static com.google.gerrit.server.cache.testing.CacheSerializerTestUtil.bytes;
import static com.google.gerrit.server.cache.testing.SerializedClassSubject.assertThatSerializedClass;
diff --git a/javatests/com/google/gerrit/server/change/MergeabilityCacheImplTest.java b/javatests/com/google/gerrit/server/change/MergeabilityCacheImplTest.java
index 69fc531..c8e6f2b 100644
--- a/javatests/com/google/gerrit/server/change/MergeabilityCacheImplTest.java
+++ b/javatests/com/google/gerrit/server/change/MergeabilityCacheImplTest.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.change;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
import static com.google.gerrit.server.cache.testing.CacheSerializerTestUtil.bytes;
import static com.google.gerrit.server.cache.testing.SerializedClassSubject.assertThatSerializedClass;
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
index c0f2c43..3d65eae 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
@@ -19,6 +19,7 @@
import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.APPROVAL_CODEC;
import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.MESSAGE_CODEC;
import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.PATCH_SET_CODEC;
+import static com.google.gerrit.server.cache.ProtoCacheSerializers.toByteString;
import static com.google.gerrit.server.cache.testing.SerializedClassSubject.assertThatSerializedClass;
import com.google.common.collect.ImmutableList;
@@ -40,7 +41,7 @@
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
-import com.google.gerrit.server.cache.ProtoCacheSerializers;
+import com.google.gerrit.server.cache.ProtoCacheSerializers.ObjectIdConverter;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ChangeColumnsProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto;
@@ -50,7 +51,6 @@
import com.google.gerrit.server.notedb.ChangeNotesState.ChangeColumns;
import com.google.gerrit.server.notedb.ChangeNotesState.Serializer;
import com.google.gwtorm.client.KeyUtil;
-import com.google.gwtorm.protobuf.ProtobufCodec;
import com.google.gwtorm.server.StandardKeyEncoder;
import com.google.inject.TypeLiteral;
import com.google.protobuf.ByteString;
@@ -58,7 +58,6 @@
import java.sql.Timestamp;
import java.util.List;
import java.util.Map;
-import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Before;
import org.junit.Test;
@@ -71,7 +70,7 @@
private static final Change.Id ID = new Change.Id(123);
private static final ObjectId SHA =
ObjectId.fromString("1234567812345678123456781234567812345678");
- private static final ByteString SHA_BYTES = toByteString(SHA);
+ private static final ByteString SHA_BYTES = ObjectIdConverter.create().toByteString(SHA);
private static final String CHANGE_KEY = "Iabcdabcdabcdabcdabcdabcdabcdabcdabcdabcd";
private ChangeColumns cols;
@@ -944,14 +943,4 @@
// additional assertions if necessary.
return actual;
}
-
- private static ByteString toByteString(ObjectId id) {
- byte[] buf = new byte[Constants.OBJECT_ID_LENGTH];
- id.copyRawTo(buf, 0);
- return ByteString.copyFrom(buf);
- }
-
- private <T> ByteString toByteString(T object, ProtobufCodec<T> codec) {
- return ProtoCacheSerializers.toByteString(object, codec);
- }
}
diff --git a/javatests/com/google/gerrit/server/query/change/BUILD b/javatests/com/google/gerrit/server/query/change/BUILD
index 78ec176..09e3243 100644
--- a/javatests/com/google/gerrit/server/query/change/BUILD
+++ b/javatests/com/google/gerrit/server/query/change/BUILD
@@ -28,13 +28,12 @@
],
)
+LUCENE_QUERY_TEST = ["LuceneQueryChangesTest.java"]
+
junit_tests(
name = "lucene_query_test",
size = "large",
- srcs = glob(
- ["*.java"],
- exclude = ABSTRACT_QUERY_TEST,
- ),
+ srcs = LUCENE_QUERY_TEST,
visibility = ["//visibility:public"],
deps = [
":abstract_query_tests",
@@ -50,3 +49,26 @@
"//lib/truth",
],
)
+
+junit_tests(
+ name = "small_tests",
+ size = "small",
+ srcs = glob(
+ ["*.java"],
+ exclude = ABSTRACT_QUERY_TEST + LUCENE_QUERY_TEST,
+ ),
+ visibility = ["//visibility:public"],
+ deps = [
+ "//java/com/google/gerrit/extensions:api",
+ "//java/com/google/gerrit/reviewdb:server",
+ "//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/server/cache/testing",
+ "//java/com/google/gerrit/testing:gerrit-test-util",
+ "//lib:guava",
+ "//lib:gwtorm",
+ "//lib/jgit/org.eclipse.jgit:jgit",
+ "//lib/truth",
+ "//lib/truth:truth-proto-extension",
+ "//proto:cache_java_proto",
+ ],
+)
diff --git a/javatests/com/google/gerrit/server/query/change/ConflictKeyTest.java b/javatests/com/google/gerrit/server/query/change/ConflictKeyTest.java
new file mode 100644
index 0000000..b87bbf7
--- /dev/null
+++ b/javatests/com/google/gerrit/server/query/change/ConflictKeyTest.java
@@ -0,0 +1,98 @@
+// Copyright (C) 2018 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.query.change;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
+import static com.google.gerrit.extensions.client.SubmitType.FAST_FORWARD_ONLY;
+import static com.google.gerrit.extensions.client.SubmitType.MERGE_IF_NECESSARY;
+import static com.google.gerrit.server.cache.testing.CacheSerializerTestUtil.bytes;
+import static com.google.gerrit.server.cache.testing.SerializedClassSubject.assertThatSerializedClass;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.extensions.client.SubmitType;
+import com.google.gerrit.server.cache.proto.Cache.ConflictKeyProto;
+import org.eclipse.jgit.lib.ObjectId;
+import org.junit.Test;
+
+public class ConflictKeyTest {
+ @Test
+ public void ffOnlyPreservesInputOrder() {
+ ObjectId id1 = ObjectId.fromString("badc0feebadc0feebadc0feebadc0feebadc0fee");
+ ObjectId id2 = ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef");
+ ConflictKey id1First = ConflictKey.create(id1, id2, FAST_FORWARD_ONLY, true);
+ ConflictKey id2First = ConflictKey.create(id2, id1, FAST_FORWARD_ONLY, true);
+
+ assertThat(id1First)
+ .isEqualTo(ConflictKey.createWithoutNormalization(id1, id2, FAST_FORWARD_ONLY, true));
+ assertThat(id2First)
+ .isEqualTo(ConflictKey.createWithoutNormalization(id2, id1, FAST_FORWARD_ONLY, true));
+ assertThat(id1First).isNotEqualTo(id2First);
+ }
+
+ @Test
+ public void nonFfOnlyNormalizesInputOrder() {
+ ObjectId id1 = ObjectId.fromString("badc0feebadc0feebadc0feebadc0feebadc0fee");
+ ObjectId id2 = ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef");
+ ConflictKey id1First = ConflictKey.create(id1, id2, MERGE_IF_NECESSARY, true);
+ ConflictKey id2First = ConflictKey.create(id2, id1, MERGE_IF_NECESSARY, true);
+ ConflictKey expected =
+ ConflictKey.createWithoutNormalization(id1, id2, MERGE_IF_NECESSARY, true);
+
+ assertThat(id1First).isEqualTo(expected);
+ assertThat(id2First).isEqualTo(expected);
+ }
+
+ @Test
+ public void serializer() throws Exception {
+ ConflictKey key =
+ ConflictKey.create(
+ ObjectId.fromString("badc0feebadc0feebadc0feebadc0feebadc0fee"),
+ ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"),
+ SubmitType.MERGE_IF_NECESSARY,
+ false);
+ byte[] serialized = ConflictKey.Serializer.INSTANCE.serialize(key);
+ assertThat(ConflictKeyProto.parseFrom(serialized))
+ .isEqualTo(
+ ConflictKeyProto.newBuilder()
+ .setCommit(
+ bytes(
+ 0xba, 0xdc, 0x0f, 0xee, 0xba, 0xdc, 0x0f, 0xee, 0xba, 0xdc, 0x0f, 0xee,
+ 0xba, 0xdc, 0x0f, 0xee, 0xba, 0xdc, 0x0f, 0xee))
+ .setOtherCommit(
+ bytes(
+ 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+ 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef))
+ .setSubmitType("MERGE_IF_NECESSARY")
+ .setContentMerge(false)
+ .build());
+ assertThat(ConflictKey.Serializer.INSTANCE.deserialize(serialized)).isEqualTo(key);
+ }
+
+ /**
+ * See {@link com.google.gerrit.server.cache.testing.SerializedClassSubject} for background and
+ * what to do if this test fails.
+ */
+ @Test
+ public void methods() throws Exception {
+ assertThatSerializedClass(ConflictKey.class)
+ .hasAutoValueMethods(
+ ImmutableMap.of(
+ "commit", ObjectId.class,
+ "otherCommit", ObjectId.class,
+ "submitType", SubmitType.class,
+ "contentMerge", boolean.class));
+ }
+}
diff --git a/proto/cache.proto b/proto/cache.proto
index 7e2e75a..a826f8c 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -184,3 +184,12 @@
int64 read_only_until = 17;
bool has_read_only_until = 18;
}
+
+
+// Serialized form of com.google.gerrit.server.query.change.ConflictKey
+message ConflictKeyProto {
+ bytes commit = 1;
+ bytes other_commit = 2;
+ string submit_type = 3;
+ bool content_merge = 4;
+}
