Merge "Changes to font sizes"
diff --git a/WORKSPACE b/WORKSPACE
index 94138e4..757d86e 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -181,8 +181,8 @@
maven_jar(
name = "gson",
- artifact = "com.google.code.gson:gson:2.8.2",
- sha1 = "3edcfe49d2c6053a70a2a47e4e1c2f94998a49cf",
+ artifact = "com.google.code.gson:gson:2.8.4",
+ sha1 = "d0de1ca9b69e69d1d497ee3c6009d015f64dad57",
)
maven_jar(
@@ -194,8 +194,8 @@
maven_jar(
name = "protobuf",
- artifact = "com.google.protobuf:protobuf-java:3.4.0",
- sha1 = "b32aba0cbe737a4ca953f71688725972e3ee927c",
+ artifact = "com.google.protobuf:protobuf-java:3.5.1",
+ sha1 = "8c3492f7662fa1cbf8ca76a0f5eb1146f7725acd",
)
load("//lib:guava.bzl", "GUAVA_VERSION", "GUAVA_BIN_SHA1")
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..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;
@@ -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;
@@ -51,8 +50,8 @@
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.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.Repository;
@@ -106,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(
@@ -127,78 +126,44 @@
}
}
- 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> {
@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.prior()))
+ .setNext(idConverter.toByteString(object.next()))
+ .setStrategyName(object.strategyName())
+ .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 create(
+ idConverter.fromByteString(proto.getPrior()),
+ idConverter.fromByteString(proto.getNext()),
+ proto.getStrategyName());
}
}
}
@@ -231,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;
}
@@ -244,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())) {
@@ -277,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())) {
@@ -321,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;
}
@@ -334,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;
}
}
@@ -347,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
}
}
@@ -377,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/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/git/GarbageCollection.java b/java/com/google/gerrit/server/git/GarbageCollection.java
index 3bf89c7..997907e 100644
--- a/java/com/google/gerrit/server/git/GarbageCollection.java
+++ b/java/com/google/gerrit/server/git/GarbageCollection.java
@@ -41,9 +41,6 @@
public class GarbageCollection {
private static final Logger log = LoggerFactory.getLogger(GarbageCollection.class);
- public static final String LOG_NAME = "gc_log";
- private static final Logger gcLog = LoggerFactory.getLogger(LOG_NAME);
-
private final GitRepositoryManager repoManager;
private final GarbageCollectionQueue gcQueue;
private final GcConfig gcConfig;
@@ -142,7 +139,7 @@
}
b.append(s);
}
- gcLog.info(b.toString());
+ log.info(b.toString());
}
private static void logGcConfiguration(
@@ -182,7 +179,6 @@
print(writer, "failed.\n\n");
StringBuilder b = new StringBuilder();
b.append("[").append(projectName.get()).append("]");
- gcLog.error(b.toString(), e);
log.error(b.toString(), e);
}
diff --git a/java/com/google/gerrit/server/git/GarbageCollectionLogFile.java b/java/com/google/gerrit/server/git/GarbageCollectionLogFile.java
index e03ef67..8796fdf 100644
--- a/java/com/google/gerrit/server/git/GarbageCollectionLogFile.java
+++ b/java/com/google/gerrit/server/git/GarbageCollectionLogFile.java
@@ -26,6 +26,8 @@
import org.eclipse.jgit.lib.Config;
public class GarbageCollectionLogFile implements LifecycleListener {
+ private static final String LOG_NAME = "gc_log";
+
@Inject
public GarbageCollectionLogFile(SitePaths sitePaths, @GerritServerConfig Config config) {
if (SystemLog.shouldConfigure()) {
@@ -38,15 +40,20 @@
@Override
public void stop() {
- LogManager.getLogger(GarbageCollection.LOG_NAME).removeAllAppenders();
+ LogManager.getLogger(GarbageCollection.class).removeAllAppenders();
+ LogManager.getLogger(GarbageCollectionRunner.class).removeAllAppenders();
}
private static void initLogSystem(Path logdir, boolean rotate) {
- Logger gcLogger = LogManager.getLogger(GarbageCollection.LOG_NAME);
+ initGcLogger(logdir, rotate, LogManager.getLogger(GarbageCollection.class));
+ initGcLogger(logdir, rotate, LogManager.getLogger(GarbageCollectionRunner.class));
+ }
+
+ private static void initGcLogger(Path logdir, boolean rotate, Logger gcLogger) {
gcLogger.removeAllAppenders();
gcLogger.addAppender(
SystemLog.createAppender(
- logdir, GarbageCollection.LOG_NAME, new PatternLayout("[%d] %-5p %x: %m%n"), rotate));
+ logdir, LOG_NAME, new PatternLayout("[%d] %-5p %x: %m%n"), rotate));
gcLogger.setAdditivity(false);
}
}
diff --git a/java/com/google/gerrit/server/git/GarbageCollectionRunner.java b/java/com/google/gerrit/server/git/GarbageCollectionRunner.java
index e4316c5..054e56a 100644
--- a/java/com/google/gerrit/server/git/GarbageCollectionRunner.java
+++ b/java/com/google/gerrit/server/git/GarbageCollectionRunner.java
@@ -24,7 +24,7 @@
/** Runnable to enable scheduling gc to run periodically */
public class GarbageCollectionRunner implements Runnable {
- private static final Logger gcLog = LoggerFactory.getLogger(GarbageCollection.LOG_NAME);
+ private static final Logger log = LoggerFactory.getLogger(GarbageCollectionRunner.class);
static class Lifecycle implements LifecycleListener {
private final WorkQueue queue;
@@ -61,7 +61,7 @@
@Override
public void run() {
- gcLog.info("Triggering gc on all repositories");
+ log.info("Triggering gc on all repositories");
garbageCollectionFactory.create().run(Lists.newArrayList(projectCache.all()));
}
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/java/com/google/gerrit/server/restapi/change/Submit.java b/java/com/google/gerrit/server/restapi/change/Submit.java
index 54ecd18..51a4090 100644
--- a/java/com/google/gerrit/server/restapi/change/Submit.java
+++ b/java/com/google/gerrit/server/restapi/change/Submit.java
@@ -95,14 +95,10 @@
"Submit all ${topicSize} changes of the same topic "
+ "(${submitSize} changes including ancestors and other "
+ "changes related by topic)";
- private static final String BLOCKED_SUBMIT_TOOLTIP =
- "This change depends on other changes which are not ready";
private static final String BLOCKED_HIDDEN_SUBMIT_TOOLTIP =
"This change depends on other hidden changes which are not ready";
- private static final String BLOCKED_WORK_IN_PROGRESS = "This change is marked work in progress";
private static final String CLICK_FAILURE_TOOLTIP = "Clicking the button would fail";
private static final String CHANGE_UNMERGEABLE = "Problems with integrating this change";
- private static final String CHANGES_NOT_MERGEABLE = "Problems with change(s): ";
public static class Output {
transient Change change;
@@ -240,6 +236,11 @@
}
/**
+ * Returns a message describing what prevents the current change from being submitted - or null.
+ * This method only considers parent changes, and changes in the same topic. The caller is
+ * responsible for making sure the current change to be submitted can indeed be submitted
+ * (permissions, submit rules, is not a WIP...)
+ *
* @param cd the change the user is currently looking at
* @param cs set of changes to be submitted at once
* @param user the user who is checking to submit
@@ -251,6 +252,11 @@
return BLOCKED_HIDDEN_SUBMIT_TOOLTIP;
}
for (ChangeData c : cs.changes()) {
+ if (cd.getId().equals(c.getId())) {
+ // We ignore the change about to be submitted, as these checks are already done in the
+ // #apply and #getDescription methods.
+ continue;
+ }
Set<ChangePermission> can =
permissionBackend
.user(user)
@@ -261,12 +267,16 @@
return BLOCKED_HIDDEN_SUBMIT_TOOLTIP;
}
if (!can.contains(ChangePermission.SUBMIT)) {
- return BLOCKED_SUBMIT_TOOLTIP;
+ return "You don't have permission to submit change " + c.getId();
}
if (c.change().isWorkInProgress()) {
- return BLOCKED_WORK_IN_PROGRESS;
+ return "Change " + c.getId() + " is marked work in progress";
}
- MergeOp.checkSubmitRule(c, false);
+ try {
+ MergeOp.checkSubmitRule(c, false);
+ } catch (ResourceConflictException e) {
+ return "Change " + c.getId() + " is not ready: " + e.getMessage();
+ }
}
Collection<ChangeData> unmergeable = unmergeableChanges(cs);
@@ -278,11 +288,10 @@
return CHANGE_UNMERGEABLE;
}
}
- return CHANGES_NOT_MERGEABLE
+
+ return "Problems with change(s): "
+ unmergeable.stream().map(c -> c.getId().toString()).collect(joining(", "));
}
- } catch (ResourceConflictException e) {
- return BLOCKED_SUBMIT_TOOLTIP;
} catch (PermissionBackendException | OrmException | IOException e) {
log.error("Error checking if change is submittable", e);
throw new OrmRuntimeException("Could not determine problems for the change", e);
@@ -294,6 +303,7 @@
public UiAction.Description getDescription(RevisionResource resource) {
Change change = resource.getChange();
if (!change.getStatus().isOpen()
+ || change.isWorkInProgress()
|| !resource.isCurrent()
|| !resource.permissions().testOrFalse(ChangePermission.SUBMIT)) {
return null; // submit not visible
diff --git a/javatests/com/google/gerrit/acceptance/pgm/ReindexIT.java b/javatests/com/google/gerrit/acceptance/pgm/ReindexIT.java
index 4b6f8b2..41640a9 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/ReindexIT.java
+++ b/javatests/com/google/gerrit/acceptance/pgm/ReindexIT.java
@@ -80,6 +80,9 @@
.flatMap(g -> g.members.stream())
.map(a -> a._accountId))
.containsExactly(adminId.get());
+ // Query project index
+ assertThat(gApi.projects().query(project.get()).get().stream().map(p -> p.name))
+ .containsExactly(project.get());
}
}
@@ -220,7 +223,7 @@
}
private void setUpChange() throws Exception {
- project = new Project.NameKey("project");
+ project = new Project.NameKey("reindex-project-test");
try (ServerContext ctx = startServer()) {
GerritApi gApi = ctx.getInjector().getInstance(GerritApi.class);
gApi.projects().create(project.get());
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index f89f2a1..171babd 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -105,7 +105,9 @@
public void revisionActionsTwoChangesInTopic() throws Exception {
String changeId = createChangeWithTopic().getChangeId();
approve(changeId);
- String changeId2 = createChangeWithTopic().getChangeId();
+ PushOneCommit.Result change2 = createChangeWithTopic();
+ int legacyId2 = change2.getChange().getId().get();
+ String changeId2 = change2.getChangeId();
Map<String, ActionInfo> actions = getActions(changeId);
commonActionsAssertions(actions);
if (isSubmitWholeTopicEnabled()) {
@@ -113,7 +115,7 @@
assertThat(info.enabled).isNull();
assertThat(info.label).isEqualTo("Submit whole topic");
assertThat(info.method).isEqualTo("POST");
- assertThat(info.title).isEqualTo("This change depends on other changes which are not ready");
+ assertThat(info.title).matches("Change " + legacyId2 + " is not ready: needs Code-Review");
} else {
noSubmitWholeTopicAssertions(actions, 1);
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..03e0d4e 100644
--- a/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
+++ b/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
@@ -15,12 +15,14 @@
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;
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;
@@ -28,7 +30,7 @@
@Test
public void keySerializer() throws Exception {
ChangeKindCacheImpl.Key key =
- new ChangeKindCacheImpl.Key(
+ Key.create(
ObjectId.zeroId(),
ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"),
"aStrategy");
@@ -54,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));
}
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/plugins/codemirror-editor b/plugins/codemirror-editor
index ee50e45..53dccff 160000
--- a/plugins/codemirror-editor
+++ b/plugins/codemirror-editor
@@ -1 +1 @@
-Subproject commit ee50e45b449e282ed78917175daf8b359da8d943
+Subproject commit 53dccff17c029459999ff70ac886b80626af634b
diff --git a/plugins/download-commands b/plugins/download-commands
index 37219fe..39b9d56 160000
--- a/plugins/download-commands
+++ b/plugins/download-commands
@@ -1 +1 @@
-Subproject commit 37219fe3fd59727af1ac7a3b0ee00a6924ff8e00
+Subproject commit 39b9d56312d505308f54b810e59ff481b9a380aa
diff --git a/plugins/hooks b/plugins/hooks
index da73b23..d497bed 160000
--- a/plugins/hooks
+++ b/plugins/hooks
@@ -1 +1 @@
-Subproject commit da73b23cfb065fc28c9e7653860ccd34bd68f0f0
+Subproject commit d497bed6963134388e7f500364a4ae59b94bafe7
diff --git a/plugins/reviewnotes b/plugins/reviewnotes
index 4672856..8ddb7e2 160000
--- a/plugins/reviewnotes
+++ b/plugins/reviewnotes
@@ -1 +1 @@
-Subproject commit 467285664ebf8eb6f1e03ff13ebc706eee6d8662
+Subproject commit 8ddb7e2ebda8cb7813ae0f3f0b8602e14915e300
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 83f1565..1d3a020 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -103,8 +103,6 @@
_filesByPath: Object,
_files: {
type: Array,
- computed: '_computeFiles(_filesByPath, changeComments, patchRange, ' +
- '_reviewed)',
observer: '_filesChanged',
value() { return []; },
},
@@ -183,6 +181,8 @@
observers: [
'_expandedPathsChanged(_expandedFilePaths.splices)',
+ '_computeFiles(_filesByPath, changeComments, patchRange, _reviewed, ' +
+ '_loading)',
],
keyBindings: {
@@ -782,13 +782,14 @@
'gr-icons:expand-less' : 'gr-icons:expand-more';
},
- _computeFiles(filesByPath, changeComments, patchRange, reviewed) {
+ _computeFiles(filesByPath, changeComments, patchRange, reviewed, loading) {
+ // Await all promises resolving from reload. @See Issue 9057
+ if (loading) { return; }
+
const commentedPaths = changeComments.getPaths(patchRange);
const files = Object.assign({}, filesByPath);
Object.keys(commentedPaths).forEach(commentedPath => {
- if (files.hasOwnProperty(commentedPath)) {
- return;
- }
+ if (files.hasOwnProperty(commentedPath)) { return; }
files[commentedPath] = {status: 'U'};
});
const reviewedSet = new Set(reviewed || []);
@@ -797,7 +798,7 @@
files[filePath].isReviewed = reviewedSet.has(filePath);
}
- return this._normalizeChangeFilesResponse(files);
+ this._files = this._normalizeChangeFilesResponse(files);
},
_computeFilesShown(numFilesShown, files) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index 3c90a1f..0289449 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -60,9 +60,11 @@
stub('gr-rest-api-interface', {
getLoggedIn() { return Promise.resolve(true); },
getPreferences() { return Promise.resolve({}); },
+ getDiffPreferences() { return Promise.resolve({}); },
getDiffComments() { return Promise.resolve({}); },
getDiffRobotComments() { return Promise.resolve({}); },
getDiffDrafts() { return Promise.resolve({}); },
+ getAccountCapabilities() { return Promise.resolve({}); },
});
stub('gr-date-formatter', {
_loadTimeFormat() { return Promise.resolve(''); },
@@ -85,6 +87,7 @@
.returns({meta: {}, left: [], right: []});
done();
});
+ element._loading = false;
element.diffPrefs = {};
element.numFilesShown = 200;
element.patchRange = {
@@ -1321,6 +1324,7 @@
.returns({meta: {}, left: [], right: []});
done();
});
+ element._loading = false;
element.numFilesShown = 75;
element.selectedIndex = 0;
element._filesByPath = {
diff --git a/polygerrit-ui/app/elements/shared/gr-lib-loader/gr-lib-loader.js b/polygerrit-ui/app/elements/shared/gr-lib-loader/gr-lib-loader.js
index 28ff45d..de92d8a 100644
--- a/polygerrit-ui/app/elements/shared/gr-lib-loader/gr-lib-loader.js
+++ b/polygerrit-ui/app/elements/shared/gr-lib-loader/gr-lib-loader.js
@@ -112,7 +112,9 @@
_getLibRoot() {
// TODO(wyatta): Remove the remainder of this method logic once the
// STATIC_RESOURCE_PATH variable is being provided generally.
- if (window.STATIC_RESOURCE_PATH) { return window.STATIC_RESOURCE_PATH; }
+ if (window.STATIC_RESOURCE_PATH) {
+ return window.STATIC_RESOURCE_PATH + '/';
+ }
if (this._cachedLibRoot) { return this._cachedLibRoot; }
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;
+}
