Merge "Report low FPS to analytics"
diff --git a/.settings/org.eclipse.jdt.core.prefs b/.settings/org.eclipse.jdt.core.prefs
index d542a0b..40e022d 100644
--- a/.settings/org.eclipse.jdt.core.prefs
+++ b/.settings/org.eclipse.jdt.core.prefs
@@ -17,6 +17,7 @@
org.eclipse.jdt.core.compiler.debug.localVariable=generate
org.eclipse.jdt.core.compiler.debug.sourceFile=generate
org.eclipse.jdt.core.compiler.doc.comment.support=enabled
+org.eclipse.jdt.core.compiler.problem.APILeak=warning
org.eclipse.jdt.core.compiler.problem.annotationSuperInterface=ignore
org.eclipse.jdt.core.compiler.problem.assertIdentifier=error
org.eclipse.jdt.core.compiler.problem.autoboxing=ignore
@@ -91,6 +92,7 @@
org.eclipse.jdt.core.compiler.problem.suppressWarnings=enabled
org.eclipse.jdt.core.compiler.problem.syntacticNullAnalysisForFields=disabled
org.eclipse.jdt.core.compiler.problem.syntheticAccessEmulation=ignore
+org.eclipse.jdt.core.compiler.problem.terminalDeprecation=warning
org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
org.eclipse.jdt.core.compiler.problem.unavoidableGenericTypeProblems=enabled
org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
diff --git a/WORKSPACE b/WORKSPACE
index d482577..15d8651 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -680,8 +680,8 @@
maven_jar(
name = "junit",
- artifact = "junit:junit:4.11",
- sha1 = "4e031bb61df09069aeb2bffb4019e7a5034a4ee0",
+ artifact = "junit:junit:4.12",
+ sha1 = "2973d150c0dc1fefe998f834810d68f278ea58ec",
)
maven_jar(
@@ -697,18 +697,18 @@
sha1 = "4785a3c21320980282f9f33d0d1264a69040538f",
)
-TRUTH_VERS = "0.39"
+TRUTH_VERS = "0.40"
maven_jar(
name = "truth",
artifact = "com.google.truth:truth:" + TRUTH_VERS,
- sha1 = "bd1bf5706ff34eb7ff80fef8b0c4320f112ef899",
+ sha1 = "0d74e716afec045cc4a178dbbfde2a8314ae5574",
)
maven_jar(
name = "truth-java8-extension",
artifact = "com.google.truth.extensions:truth-java8-extension:" + TRUTH_VERS,
- sha1 = "1499bc88cda9d674afb30da9813b44bcd4512d0d",
+ sha1 = "636e49d675bc28e0b3ae0edd077d6acbbb159166",
)
# When bumping the easymock version number, make sure to also move powermock to a compatible version
diff --git a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
index 0a06c31..58a298e 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
@@ -15,9 +15,9 @@
package com.google.gerrit.elasticsearch;
import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.gerrit.server.index.change.ChangeField.APPROVAL_CODEC;
-import static com.google.gerrit.server.index.change.ChangeField.CHANGE_CODEC;
-import static com.google.gerrit.server.index.change.ChangeField.PATCH_SET_CODEC;
+import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.APPROVAL_CODEC;
+import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.CHANGE_CODEC;
+import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.PATCH_SET_CODEC;
import static com.google.gerrit.server.index.change.ChangeIndexRewriter.CLOSED_STATUSES;
import static com.google.gerrit.server.index.change.ChangeIndexRewriter.OPEN_STATUSES;
import static java.nio.charset.StandardCharsets.UTF_8;
diff --git a/java/com/google/gerrit/extensions/auth/oauth/OAuthToken.java b/java/com/google/gerrit/extensions/auth/oauth/OAuthToken.java
index b736262..84b6a04 100644
--- a/java/com/google/gerrit/extensions/auth/oauth/OAuthToken.java
+++ b/java/com/google/gerrit/extensions/auth/oauth/OAuthToken.java
@@ -14,9 +14,20 @@
package com.google.gerrit.extensions.auth.oauth;
-import java.io.Serializable;
+import static com.google.common.base.Preconditions.checkNotNull;
-/* OAuth token */
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Strings;
+import com.google.gerrit.common.Nullable;
+import java.io.Serializable;
+import java.util.Objects;
+
+/**
+ * OAuth token.
+ *
+ * <p>Only implements {@link Serializable} for backwards compatibility; new extensions should not
+ * depend on the serialized format.
+ */
public class OAuthToken implements Serializable {
private static final long serialVersionUID = 1L;
@@ -32,8 +43,9 @@
private final long expiresAt;
/**
- * The identifier of the OAuth provider that issued this token in the form
- * <tt>"plugin-name:provider-name"</tt>, or {@code null}.
+ * The identifier of the OAuth provider that issued this token in the form {@code
+ * "plugin-name:provider-name"}, or {@code null}. The empty string {@code ""} is treated the same
+ * as {@code null}.
*/
private final String providerId;
@@ -41,12 +53,13 @@
this(token, secret, raw, Long.MAX_VALUE, null);
}
- public OAuthToken(String token, String secret, String raw, long expiresAt, String providerId) {
- this.token = token;
- this.secret = secret;
- this.raw = raw;
+ public OAuthToken(
+ String token, String secret, String raw, long expiresAt, @Nullable String providerId) {
+ this.token = checkNotNull(token, "token");
+ this.secret = checkNotNull(secret, "secret");
+ this.raw = checkNotNull(raw, "raw");
this.expiresAt = expiresAt;
- this.providerId = providerId;
+ this.providerId = Strings.emptyToNull(providerId);
}
public String getToken() {
@@ -69,7 +82,37 @@
return System.currentTimeMillis() > expiresAt;
}
+ @Nullable
public String getProviderId() {
return providerId;
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (!(o instanceof OAuthToken)) {
+ return false;
+ }
+ OAuthToken t = (OAuthToken) o;
+ return token.equals(t.token)
+ && secret.equals(t.secret)
+ && raw.equals(t.raw)
+ && expiresAt == t.expiresAt
+ && Objects.equals(providerId, t.providerId);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(token, secret, raw, expiresAt, providerId);
+ }
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("token", token)
+ .add("secret", secret)
+ .add("raw", raw)
+ .add("expiresAt", expiresAt)
+ .add("providerId", providerId)
+ .toString();
+ }
}
diff --git a/java/com/google/gerrit/extensions/registration/DynamicMap.java b/java/com/google/gerrit/extensions/registration/DynamicMap.java
index e0db0c7..7178a16 100644
--- a/java/com/google/gerrit/extensions/registration/DynamicMap.java
+++ b/java/com/google/gerrit/extensions/registration/DynamicMap.java
@@ -83,6 +83,11 @@
binder.bind(key).toProvider(new DynamicMapProvider<>(member)).in(Scopes.SINGLETON);
}
+ /** Returns an empty DynamicMap instance * */
+ public static <T> DynamicMap<T> emptyMap() {
+ return new PrivateInternals_DynamicMapImpl<>();
+ }
+
final ConcurrentMap<NamePair, Provider<T>> items;
DynamicMap() {
@@ -188,8 +193,8 @@
private final String exportName;
NamePair(String pn, String en) {
- this.pluginName = pn;
- this.exportName = en;
+ pluginName = pn;
+ exportName = en;
}
@Override
@@ -206,8 +211,4 @@
return false;
}
}
-
- public static <T> DynamicMap<T> emptyMap() {
- return new DynamicMap<T>() {};
- }
}
diff --git a/java/com/google/gerrit/extensions/registration/DynamicSet.java b/java/com/google/gerrit/extensions/registration/DynamicSet.java
index 5cdf267..7ffb86d 100644
--- a/java/com/google/gerrit/extensions/registration/DynamicSet.java
+++ b/java/com/google/gerrit/extensions/registration/DynamicSet.java
@@ -139,7 +139,7 @@
}
public DynamicSet() {
- this(Collections.<AtomicReference<Provider<T>>>emptySet());
+ this(Collections.emptySet());
}
@Override
diff --git a/java/com/google/gerrit/extensions/registration/PrivateInternals_DynamicMapImpl.java b/java/com/google/gerrit/extensions/registration/PrivateInternals_DynamicMapImpl.java
index 50aed7d..1973f70 100644
--- a/java/com/google/gerrit/extensions/registration/PrivateInternals_DynamicMapImpl.java
+++ b/java/com/google/gerrit/extensions/registration/PrivateInternals_DynamicMapImpl.java
@@ -14,6 +14,8 @@
package com.google.gerrit.extensions.registration;
+import static com.google.common.base.Preconditions.checkNotNull;
+
import com.google.gerrit.extensions.annotations.Export;
import com.google.inject.Key;
import com.google.inject.Provider;
@@ -31,6 +33,7 @@
* @return handle to remove the item at a later point in time.
*/
public RegistrationHandle put(String pluginName, String exportName, Provider<T> item) {
+ checkNotNull(item);
final NamePair key = new NamePair(pluginName, exportName);
items.put(key, item);
return new RegistrationHandle() {
@@ -53,6 +56,7 @@
* the collection.
*/
public ReloadableRegistrationHandle<T> put(String pluginName, Key<T> key, Provider<T> item) {
+ checkNotNull(item);
String exportName = ((Export) key.getAnnotation()).value();
NamePair np = new NamePair(pluginName, exportName);
items.put(np, item);
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 468aa67..c8f8fff 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -16,11 +16,11 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.lucene.AbstractLuceneIndex.sortFieldName;
+import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.APPROVAL_CODEC;
+import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.CHANGE_CODEC;
+import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.PATCH_SET_CODEC;
import static com.google.gerrit.server.git.QueueProvider.QueueType.INTERACTIVE;
-import static com.google.gerrit.server.index.change.ChangeField.APPROVAL_CODEC;
-import static com.google.gerrit.server.index.change.ChangeField.CHANGE_CODEC;
import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID;
-import static com.google.gerrit.server.index.change.ChangeField.PATCH_SET_CODEC;
import static com.google.gerrit.server.index.change.ChangeField.PROJECT;
import static com.google.gerrit.server.index.change.ChangeIndexRewriter.CLOSED_STATUSES;
import static com.google.gerrit.server.index.change.ChangeIndexRewriter.OPEN_STATUSES;
diff --git a/java/com/google/gerrit/reviewdb/client/ChangeMessage.java b/java/com/google/gerrit/reviewdb/client/ChangeMessage.java
index edc022f..8e397f0 100644
--- a/java/com/google/gerrit/reviewdb/client/ChangeMessage.java
+++ b/java/com/google/gerrit/reviewdb/client/ChangeMessage.java
@@ -149,6 +149,26 @@
}
@Override
+ public boolean equals(Object o) {
+ if (!(o instanceof ChangeMessage)) {
+ return false;
+ }
+ ChangeMessage m = (ChangeMessage) o;
+ return Objects.equals(key, m.key)
+ && Objects.equals(author, m.author)
+ && Objects.equals(writtenOn, m.writtenOn)
+ && Objects.equals(message, m.message)
+ && Objects.equals(patchset, m.patchset)
+ && Objects.equals(tag, m.tag)
+ && Objects.equals(realAuthor, m.realAuthor);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(key, author, writtenOn, message, patchset, tag, realAuthor);
+ }
+
+ @Override
public String toString() {
return "ChangeMessage{"
+ "key="
diff --git a/java/com/google/gerrit/reviewdb/client/PatchSet.java b/java/com/google/gerrit/reviewdb/client/PatchSet.java
index 4536b67..849fd75 100644
--- a/java/com/google/gerrit/reviewdb/client/PatchSet.java
+++ b/java/com/google/gerrit/reviewdb/client/PatchSet.java
@@ -20,6 +20,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.Objects;
/** A single revision of a {@link Change}. */
public final class PatchSet {
@@ -280,6 +281,26 @@
}
@Override
+ public boolean equals(Object o) {
+ if (!(o instanceof PatchSet)) {
+ return false;
+ }
+ PatchSet p = (PatchSet) o;
+ return Objects.equals(id, p.id)
+ && Objects.equals(revision, p.revision)
+ && Objects.equals(uploader, p.uploader)
+ && Objects.equals(createdOn, p.createdOn)
+ && Objects.equals(groups, p.groups)
+ && Objects.equals(pushCertificate, p.pushCertificate)
+ && Objects.equals(description, p.description);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(id, revision, uploader, createdOn, groups, pushCertificate, description);
+ }
+
+ @Override
public String toString() {
return "[PatchSet " + getId().toString() + "]";
}
diff --git a/java/com/google/gerrit/reviewdb/server/ReviewDbCodecs.java b/java/com/google/gerrit/reviewdb/server/ReviewDbCodecs.java
new file mode 100644
index 0000000..631e7f5
--- /dev/null
+++ b/java/com/google/gerrit/reviewdb/server/ReviewDbCodecs.java
@@ -0,0 +1,34 @@
+// 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.reviewdb.server;
+
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gwtorm.protobuf.CodecFactory;
+import com.google.gwtorm.protobuf.ProtobufCodec;
+
+/** {@link ProtobufCodec} instances for ReviewDb types. */
+public class ReviewDbCodecs {
+ public static final ProtobufCodec<PatchSetApproval> APPROVAL_CODEC =
+ CodecFactory.encoder(PatchSetApproval.class);
+
+ public static final ProtobufCodec<Change> CHANGE_CODEC = CodecFactory.encoder(Change.class);
+
+ public static final ProtobufCodec<PatchSet> PATCH_SET_CODEC =
+ CodecFactory.encoder(PatchSet.class);
+
+ private ReviewDbCodecs() {}
+}
diff --git a/java/com/google/gerrit/server/ChangeMessagesUtil.java b/java/com/google/gerrit/server/ChangeMessagesUtil.java
index 75a9991..e635072 100644
--- a/java/com/google/gerrit/server/ChangeMessagesUtil.java
+++ b/java/com/google/gerrit/server/ChangeMessagesUtil.java
@@ -116,14 +116,6 @@
return notes.load().getChangeMessages();
}
- public Iterable<ChangeMessage> byPatchSet(ReviewDb db, ChangeNotes notes, PatchSet.Id psId)
- throws OrmException {
- if (!migration.readChanges()) {
- return db.changeMessages().byPatchSet(psId);
- }
- return notes.load().getChangeMessagesByPatchSet().get(psId);
- }
-
public void addChangeMessage(ReviewDb db, ChangeUpdate update, ChangeMessage changeMessage)
throws OrmException {
checkState(
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalId.java b/java/com/google/gerrit/server/account/externalids/ExternalId.java
index 442bc2a..db8ea41 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalId.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalId.java
@@ -118,6 +118,8 @@
* AuthType#HTTP_LDAP}, and {@link AuthType#LDAP_BIND} usernames.
*
* <p>The name {@code gerrit:} was a very poor choice.
+ *
+ * <p>Scheme names must not contain colons (':').
*/
public static final String SCHEME_GERRIT = "gerrit";
@@ -140,6 +142,13 @@
public abstract static class Key implements Serializable {
private static final long serialVersionUID = 1L;
+ /**
+ * Creates an external ID key.
+ *
+ * @param scheme the scheme name, must not contain colons (':'), can be {@code null}
+ * @param id the external ID, must not contain colons (':')
+ * @return the created external ID key
+ */
public static Key create(@Nullable String scheme, String id) {
return new AutoValue_ExternalId_Key(Strings.emptyToNull(scheme), id);
}
@@ -198,10 +207,28 @@
}
}
+ /**
+ * Creates an external ID.
+ *
+ * @param scheme the scheme name, must not contain colons (':')
+ * @param id the external ID, must not contain colons (':')
+ * @param accountId the ID of the account to which the external ID belongs
+ * @return the created external ID
+ */
public static ExternalId create(String scheme, String id, Account.Id accountId) {
return create(Key.create(scheme, id), accountId, null, null);
}
+ /**
+ * Creates an external ID.
+ *
+ * @param scheme the scheme name, must not contain colons (':')
+ * @param id the external ID, must not contain colons (':')
+ * @param accountId the ID of the account to which the external ID belongs
+ * @param email the email of the external ID, may be {@code null}
+ * @param hashedPassword the hashed password of the external ID, may be {@code null}
+ * @return the created external ID
+ */
public static ExternalId create(
String scheme,
String id,
@@ -222,17 +249,35 @@
}
public static ExternalId createWithPassword(
- Key key, Account.Id accountId, @Nullable String email, String plainPassword) {
+ Key key, Account.Id accountId, @Nullable String email, @Nullable String plainPassword) {
plainPassword = Strings.emptyToNull(plainPassword);
String hashedPassword =
plainPassword != null ? HashedPassword.fromPassword(plainPassword).encode() : null;
return create(key, accountId, email, hashedPassword);
}
- public static ExternalId createUsername(String id, Account.Id accountId, String plainPassword) {
+ /**
+ * Create a external ID for a username (scheme "username").
+ *
+ * @param id the external ID, must not contain colons (':')
+ * @param accountId the ID of the account to which the external ID belongs
+ * @param plainPassword the plain HTTP password, may be {@code null}
+ * @return the created external ID
+ */
+ public static ExternalId createUsername(
+ String id, Account.Id accountId, @Nullable String plainPassword) {
return createWithPassword(Key.create(SCHEME_USERNAME, id), accountId, null, plainPassword);
}
+ /**
+ * Creates an external ID with an email.
+ *
+ * @param scheme the scheme name, must not contain colons (':')
+ * @param id the external ID, must not contain colons (':')
+ * @param accountId the ID of the account to which the external ID belongs
+ * @param email the email of the external ID, may be {@code null}
+ * @return the created external ID
+ */
public static ExternalId createWithEmail(
String scheme, String id, Account.Id accountId, @Nullable String email) {
return createWithEmail(Key.create(scheme, id), accountId, email);
diff --git a/java/com/google/gerrit/server/auth/oauth/OAuthTokenCache.java b/java/com/google/gerrit/server/auth/oauth/OAuthTokenCache.java
index 1ac3bca..f380051 100644
--- a/java/com/google/gerrit/server/auth/oauth/OAuthTokenCache.java
+++ b/java/com/google/gerrit/server/auth/oauth/OAuthTokenCache.java
@@ -16,16 +16,23 @@
import static com.google.common.base.Preconditions.checkNotNull;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Strings;
import com.google.common.cache.Cache;
import com.google.gerrit.extensions.auth.oauth.OAuthToken;
import com.google.gerrit.extensions.auth.oauth.OAuthTokenEncrypter;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.cache.CacheSerializer;
+import com.google.gerrit.server.cache.IntKeyCacheSerializer;
+import com.google.gerrit.server.cache.ProtoCacheSerializers;
+import com.google.gerrit.server.cache.proto.Cache.OAuthTokenProto;
import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.Singleton;
import com.google.inject.name.Named;
+import java.io.IOException;
@Singleton
public class OAuthTokenCache {
@@ -37,11 +44,47 @@
return new CacheModule() {
@Override
protected void configure() {
- persist(OAUTH_TOKENS, Account.Id.class, OAuthToken.class);
+ persist(OAUTH_TOKENS, Account.Id.class, OAuthToken.class)
+ .version(1)
+ .keySerializer(new IntKeyCacheSerializer<>(Account.Id::new))
+ .valueSerializer(new Serializer());
}
};
}
+ // Defined outside of OAuthToken class, since that is in the extensions package which doesn't have
+ // access to the serializer code.
+ @VisibleForTesting
+ static class Serializer implements CacheSerializer<OAuthToken> {
+ @Override
+ public byte[] serialize(OAuthToken object) {
+ return ProtoCacheSerializers.toByteArray(
+ OAuthTokenProto.newBuilder()
+ .setToken(object.getToken())
+ .setSecret(object.getSecret())
+ .setRaw(object.getRaw())
+ .setExpiresAt(object.getExpiresAt())
+ .setProviderId(Strings.nullToEmpty(object.getProviderId()))
+ .build());
+ }
+
+ @Override
+ public OAuthToken deserialize(byte[] in) {
+ OAuthTokenProto proto;
+ try {
+ proto = OAuthTokenProto.parseFrom(in);
+ } catch (IOException e) {
+ throw new IllegalArgumentException("failed to deserialize OAuthToken");
+ }
+ return new OAuthToken(
+ proto.getToken(),
+ proto.getSecret(),
+ proto.getRaw(),
+ proto.getExpiresAt(),
+ Strings.emptyToNull(proto.getProviderId()));
+ }
+ }
+
private final Cache<Account.Id, OAuthToken> cache;
@Inject
diff --git a/java/com/google/gerrit/server/cache/BooleanCacheSerializer.java b/java/com/google/gerrit/server/cache/BooleanCacheSerializer.java
new file mode 100644
index 0000000..59fc946
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/BooleanCacheSerializer.java
@@ -0,0 +1,44 @@
+// 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.base.Preconditions.checkNotNull;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.protobuf.TextFormat;
+import java.util.Arrays;
+
+public enum BooleanCacheSerializer implements CacheSerializer<Boolean> {
+ INSTANCE;
+
+ private static final byte[] TRUE = Boolean.toString(true).getBytes(UTF_8);
+ private static final byte[] FALSE = Boolean.toString(false).getBytes(UTF_8);
+
+ @Override
+ public byte[] serialize(Boolean object) {
+ byte[] bytes = checkNotNull(object) ? TRUE : FALSE;
+ return Arrays.copyOf(bytes, bytes.length);
+ }
+
+ @Override
+ public Boolean deserialize(byte[] in) {
+ if (Arrays.equals(in, TRUE)) {
+ return Boolean.TRUE;
+ } else if (Arrays.equals(in, FALSE)) {
+ return Boolean.FALSE;
+ }
+ throw new IllegalArgumentException("Invalid Boolean value: " + TextFormat.escapeBytes(in));
+ }
+}
diff --git a/java/com/google/gerrit/server/cache/CacheSerializer.java b/java/com/google/gerrit/server/cache/CacheSerializer.java
index 6bd1322..08deecd 100644
--- a/java/com/google/gerrit/server/cache/CacheSerializer.java
+++ b/java/com/google/gerrit/server/cache/CacheSerializer.java
@@ -14,13 +14,29 @@
package com.google.gerrit.server.cache;
-import java.io.IOException;
-
-/** Interface for serializing/deserializing a type to/from a persistent cache. */
+/**
+ * Interface for serializing/deserializing a type to/from a persistent cache.
+ *
+ * <p>Implementations are null-hostile and will throw exceptions from {@link #serialize} when passed
+ * null values, unless otherwise specified.
+ */
public interface CacheSerializer<T> {
- /** Serializes the object to a new byte array. */
- byte[] serialize(T object) throws IOException;
+ /**
+ * Serializes the object to a new byte array.
+ *
+ * @param object object to serialize.
+ * @return serialized byte array representation.
+ * @throws RuntimeException for malformed input, for example null or an otherwise unsupported
+ * value.
+ */
+ byte[] serialize(T object);
- /** Deserializes a single object from the given byte array. */
- T deserialize(byte[] in) throws IOException;
+ /**
+ * Deserializes a single object from the given byte array.
+ *
+ * @param in serialized byte array representation.
+ * @throws RuntimeException for malformed input, for example null or an otherwise corrupt
+ * serialized representation.
+ */
+ T deserialize(byte[] in);
}
diff --git a/java/com/google/gerrit/server/cache/EnumCacheSerializer.java b/java/com/google/gerrit/server/cache/EnumCacheSerializer.java
index 6ea6121..c5be783 100644
--- a/java/com/google/gerrit/server/cache/EnumCacheSerializer.java
+++ b/java/com/google/gerrit/server/cache/EnumCacheSerializer.java
@@ -14,28 +14,26 @@
package com.google.gerrit.server.cache;
+import static com.google.common.base.Preconditions.checkNotNull;
import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.common.base.Converter;
import com.google.common.base.Enums;
-import java.io.IOException;
public class EnumCacheSerializer<E extends Enum<E>> implements CacheSerializer<E> {
- private final Class<E> clazz;
+ private final Converter<String, E> converter;
public EnumCacheSerializer(Class<E> clazz) {
- this.clazz = clazz;
+ this.converter = Enums.stringConverter(clazz);
}
@Override
- public byte[] serialize(E object) throws IOException {
- return object.name().getBytes(UTF_8);
+ public byte[] serialize(E object) {
+ return converter.reverse().convert(checkNotNull(object)).getBytes(UTF_8);
}
@Override
- public E deserialize(byte[] in) throws IOException {
- String name = new String(in, UTF_8);
- return Enums.getIfPresent(clazz, name)
- .toJavaUtil()
- .orElseThrow(() -> new IOException("Invalid " + clazz.getName() + " value: " + name));
+ public E deserialize(byte[] in) {
+ return converter.convert(new String(checkNotNull(in), UTF_8));
}
}
diff --git a/java/com/google/gerrit/server/cache/IntKeyCacheSerializer.java b/java/com/google/gerrit/server/cache/IntKeyCacheSerializer.java
new file mode 100644
index 0000000..a07c004
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/IntKeyCacheSerializer.java
@@ -0,0 +1,38 @@
+// 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.base.Preconditions.checkNotNull;
+
+import com.google.gwtorm.client.IntKey;
+import java.util.function.Function;
+
+public class IntKeyCacheSerializer<K extends IntKey<?>> implements CacheSerializer<K> {
+ private final Function<Integer, K> factory;
+
+ public IntKeyCacheSerializer(Function<Integer, K> factory) {
+ this.factory = checkNotNull(factory);
+ }
+
+ @Override
+ public byte[] serialize(K object) {
+ return IntegerCacheSerializer.INSTANCE.serialize(object.get());
+ }
+
+ @Override
+ public K deserialize(byte[] in) {
+ return factory.apply(IntegerCacheSerializer.INSTANCE.deserialize(in));
+ }
+}
diff --git a/java/com/google/gerrit/server/cache/IntegerCacheSerializer.java b/java/com/google/gerrit/server/cache/IntegerCacheSerializer.java
new file mode 100644
index 0000000..5eddb71
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/IntegerCacheSerializer.java
@@ -0,0 +1,63 @@
+// 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.base.Preconditions.checkNotNull;
+
+import com.google.protobuf.CodedInputStream;
+import com.google.protobuf.CodedOutputStream;
+import com.google.protobuf.TextFormat;
+import java.io.IOException;
+import java.util.Arrays;
+
+public enum IntegerCacheSerializer implements CacheSerializer<Integer> {
+ INSTANCE;
+
+ // Same as com.google.protobuf.WireFormat#MAX_VARINT_SIZE. Note that negative values take up more
+ // than MAX_VARINT32_SIZE space.
+ private static final int MAX_VARINT_SIZE = 10;
+
+ @Override
+ public byte[] serialize(Integer object) {
+ byte[] buf = new byte[MAX_VARINT_SIZE];
+ CodedOutputStream cout = CodedOutputStream.newInstance(buf);
+ try {
+ cout.writeInt32NoTag(checkNotNull(object));
+ cout.flush();
+ } catch (IOException e) {
+ throw new IllegalStateException("Failed to serialize int");
+ }
+ int n = cout.getTotalBytesWritten();
+ return n == buf.length ? buf : Arrays.copyOfRange(buf, 0, n);
+ }
+
+ @Override
+ public Integer deserialize(byte[] in) {
+ CodedInputStream cin = CodedInputStream.newInstance(checkNotNull(in));
+ int ret;
+ try {
+ ret = cin.readRawVarint32();
+ } catch (IOException e) {
+ throw new IllegalArgumentException("Failed to deserialize int");
+ }
+ int n = cin.getTotalBytesRead();
+ if (n != in.length) {
+ throw new IllegalArgumentException(
+ "Extra bytes in int representation: "
+ + TextFormat.escapeBytes(Arrays.copyOfRange(in, n, in.length)));
+ }
+ return ret;
+ }
+}
diff --git a/java/com/google/gerrit/server/cache/JavaCacheSerializer.java b/java/com/google/gerrit/server/cache/JavaCacheSerializer.java
index 750c5df..55358bc 100644
--- a/java/com/google/gerrit/server/cache/JavaCacheSerializer.java
+++ b/java/com/google/gerrit/server/cache/JavaCacheSerializer.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.cache;
+import com.google.gerrit.common.Nullable;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
@@ -23,29 +24,33 @@
/**
* Serializer that uses default Java serialization.
*
+ * <p>Unlike most {@link CacheSerializer} implementations, serializing null is supported.
+ *
* @param <T> type to serialize. Must implement {@code Serializable}, but due to implementation
* details this is only checked at runtime.
*/
public class JavaCacheSerializer<T> implements CacheSerializer<T> {
@Override
- public byte[] serialize(T object) throws IOException {
+ public byte[] serialize(@Nullable T object) {
try (ByteArrayOutputStream bout = new ByteArrayOutputStream();
ObjectOutputStream oout = new ObjectOutputStream(bout)) {
oout.writeObject(object);
oout.flush();
return bout.toByteArray();
+ } catch (IOException e) {
+ throw new IllegalArgumentException("Failed to serialize object", e);
}
}
@SuppressWarnings("unchecked")
@Override
- public T deserialize(byte[] in) throws IOException {
+ public T deserialize(byte[] in) {
Object object;
try (ByteArrayInputStream bin = new ByteArrayInputStream(in);
ObjectInputStream oin = new ObjectInputStream(bin)) {
object = oin.readObject();
- } catch (ClassNotFoundException e) {
- throw new IOException("Failed to deserialize object of type", e);
+ } catch (ClassNotFoundException | IOException e) {
+ throw new IllegalArgumentException("Failed to deserialize object", e);
}
return (T) object;
}
diff --git a/java/com/google/gerrit/server/cache/h2/ObjectKeyTypeImpl.java b/java/com/google/gerrit/server/cache/h2/ObjectKeyTypeImpl.java
index b1a65fe..44e2bb2 100644
--- a/java/com/google/gerrit/server/cache/h2/ObjectKeyTypeImpl.java
+++ b/java/com/google/gerrit/server/cache/h2/ObjectKeyTypeImpl.java
@@ -52,11 +52,7 @@
@Override
public void funnel(K from, PrimitiveSink into) {
- try {
- Funnels.byteArrayFunnel().funnel(serializer.serialize(from), into);
- } catch (IOException e) {
- throw new RuntimeException("Cannot hash", e);
- }
+ Funnels.byteArrayFunnel().funnel(serializer.serialize(from), into);
}
};
}
diff --git a/java/com/google/gerrit/server/cache/testing/BUILD b/java/com/google/gerrit/server/cache/testing/BUILD
new file mode 100644
index 0000000..c1293f9
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/testing/BUILD
@@ -0,0 +1,13 @@
+package(default_testonly = 1)
+
+java_library(
+ name = "testing",
+ srcs = glob(["*.java"]),
+ visibility = ["//visibility:public"],
+ deps = [
+ "//lib:guava",
+ "//lib:protobuf",
+ "//lib:truth",
+ "//lib/commons:lang3",
+ ],
+)
diff --git a/java/com/google/gerrit/server/cache/testing/CacheSerializerTestUtil.java b/java/com/google/gerrit/server/cache/testing/CacheSerializerTestUtil.java
new file mode 100644
index 0000000..5d41490
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/testing/CacheSerializerTestUtil.java
@@ -0,0 +1,30 @@
+// 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.testing;
+
+import com.google.protobuf.ByteString;
+
+/** Static utilities for testing cache serializers. */
+public class CacheSerializerTestUtil {
+ public static ByteString bytes(int... ints) {
+ byte[] bytes = new byte[ints.length];
+ for (int i = 0; i < ints.length; i++) {
+ bytes[i] = (byte) ints[i];
+ }
+ return ByteString.copyFrom(bytes);
+ }
+
+ private CacheSerializerTestUtil() {}
+}
diff --git a/java/com/google/gerrit/server/cache/testing/SerializedClassSubject.java b/java/com/google/gerrit/server/cache/testing/SerializedClassSubject.java
new file mode 100644
index 0000000..78900cb
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/testing/SerializedClassSubject.java
@@ -0,0 +1,81 @@
+// 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.testing;
+
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
+import static com.google.common.truth.Truth.assertAbout;
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
+
+import com.google.common.truth.FailureMetadata;
+import com.google.common.truth.Subject;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.lang.reflect.Type;
+import java.util.Map;
+import org.apache.commons.lang3.reflect.FieldUtils;
+
+/**
+ * Subject about classes that are serialized into persistent caches.
+ *
+ * <p>Hand-written {@link com.google.gerrit.server.cache.CacheSerializer CacheSerializer}
+ * implementations depend on the exact representation of the data stored in a class, so it is
+ * important to verify any assumptions about the structure of the serialized classes. This class
+ * contains assertions about serialized classes, and should be used for every class that has a
+ * custom serializer implementation.
+ *
+ * <p>Changing fields of a serialized class (or abstract methods, in the case of {@code @AutoValue}
+ * classes) will likely require changes to the serializer implementation, and may require bumping
+ * the {@link com.google.gerrit.server.cache.PersistentCacheBinding#version(int) version} in the
+ * cache binding, in case the representation has changed in such a way that old serialized data
+ * becomes unreadable.
+ *
+ * <p>Changes to a serialized class such as adding or removing fields generally requires a change to
+ * the hand-written serializer. Usually, serializer implementations should be written in such a way
+ * that new fields are considered optional, and won't require bumping the version.
+ */
+public class SerializedClassSubject extends Subject<SerializedClassSubject, Class<?>> {
+ public static SerializedClassSubject assertThatSerializedClass(Class<?> actual) {
+ // This formulation fails in Eclipse 4.7.3a with "The type
+ // SerializedClassSubject does not define SerializedClassSubject() that is
+ // applicable here", due to
+ // https://bugs.eclipse.org/bugs/show_bug.cgi?id=534694 or a similar bug:
+ // return assertAbout(SerializedClassSubject::new).that(actual);
+ Subject.Factory<SerializedClassSubject, Class<?>> factory =
+ (m, a) -> new SerializedClassSubject(m, a);
+ return assertAbout(factory).that(actual);
+ }
+
+ private SerializedClassSubject(FailureMetadata metadata, Class<?> actual) {
+ super(metadata, actual);
+ }
+
+ public void isConcrete() {
+ isNotNull();
+ assertWithMessage("expected class %s to be concrete", actual().getName())
+ .that(!Modifier.isAbstract(actual().getModifiers()))
+ .isTrue();
+ }
+
+ public void hasFields(Map<String, Type> expectedFields) {
+ isConcrete();
+ assertThat(
+ FieldUtils.getAllFieldsList(actual())
+ .stream()
+ .filter(f -> !Modifier.isStatic(f.getModifiers()))
+ .collect(toImmutableMap(Field::getName, Field::getGenericType)))
+ .containsExactlyEntriesIn(expectedFields);
+ }
+}
diff --git a/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java b/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
index 6449662..a4eb90f 100644
--- a/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
+++ b/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
@@ -42,6 +42,7 @@
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;
@@ -176,7 +177,7 @@
@VisibleForTesting
static class Serializer implements CacheSerializer<Key> {
@Override
- public byte[] serialize(Key object) throws IOException {
+ public byte[] serialize(Key object) {
byte[] buf = new byte[Constants.OBJECT_ID_LENGTH];
ChangeKindKeyProto.Builder b = ChangeKindKeyProto.newBuilder();
object.getPrior().copyRawTo(buf, 0);
@@ -188,12 +189,16 @@
}
@Override
- public Key deserialize(byte[] in) throws IOException {
- ChangeKindKeyProto proto = ChangeKindKeyProto.parseFrom(in);
- return new Key(
- ObjectId.fromRaw(proto.getPrior().toByteArray()),
- ObjectId.fromRaw(proto.getNext().toByteArray()),
- proto.getStrategyName());
+ 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);
+ }
}
}
}
diff --git a/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java b/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java
index ea91e0f..a192228 100644
--- a/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java
+++ b/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java
@@ -16,20 +16,20 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.base.Preconditions.checkState;
-import static com.google.gerrit.server.ioutil.BasicSerialization.readString;
-import static com.google.gerrit.server.ioutil.BasicSerialization.writeString;
-import static org.eclipse.jgit.lib.ObjectIdSerializer.readWithoutMarker;
-import static org.eclipse.jgit.lib.ObjectIdSerializer.writeWithoutMarker;
+import com.google.common.base.Converter;
+import com.google.common.base.Enums;
import com.google.common.base.MoreObjects;
import com.google.common.cache.Cache;
import com.google.common.cache.Weigher;
-import com.google.common.collect.ImmutableBiMap;
import com.google.common.util.concurrent.UncheckedExecutionException;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.server.cache.BooleanCacheSerializer;
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.proto.Cache.MergeabilityKeyProto;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.submit.SubmitDryRun;
@@ -37,14 +37,13 @@
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.io.ObjectInputStream;
-import java.io.ObjectOutputStream;
-import java.io.Serializable;
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;
@@ -58,30 +57,16 @@
private static final String CACHE_NAME = "mergeability";
- public static final ImmutableBiMap<SubmitType, Character> SUBMIT_TYPES =
- new ImmutableBiMap.Builder<SubmitType, Character>()
- .put(SubmitType.INHERIT, 'I')
- .put(SubmitType.FAST_FORWARD_ONLY, 'F')
- .put(SubmitType.MERGE_IF_NECESSARY, 'M')
- .put(SubmitType.REBASE_ALWAYS, 'P')
- .put(SubmitType.REBASE_IF_NECESSARY, 'R')
- .put(SubmitType.MERGE_ALWAYS, 'A')
- .put(SubmitType.CHERRY_PICK, 'C')
- .build();
-
- static {
- checkState(
- SUBMIT_TYPES.size() == SubmitType.values().length,
- "SubmitType <-> char BiMap needs updating");
- }
-
public static Module module() {
return new CacheModule() {
@Override
protected void configure() {
persist(CACHE_NAME, EntryKey.class, Boolean.class)
.maximumWeight(1 << 20)
- .weigher(MergeabilityWeigher.class);
+ .weigher(MergeabilityWeigher.class)
+ .version(1)
+ .keySerializer(EntryKey.Serializer.INSTANCE)
+ .valueSerializer(BooleanCacheSerializer.INSTANCE);
bind(MergeabilityCache.class).to(MergeabilityCacheImpl.class);
}
};
@@ -91,9 +76,7 @@
return ref != null && ref.getObjectId() != null ? ref.getObjectId() : ObjectId.zeroId();
}
- public static class EntryKey implements Serializable {
- private static final long serialVersionUID = 1L;
-
+ public static class EntryKey {
private ObjectId commit;
private ObjectId into;
private SubmitType submitType;
@@ -154,26 +137,44 @@
.toString();
}
- private void writeObject(ObjectOutputStream out) throws IOException {
- writeWithoutMarker(out, commit);
- writeWithoutMarker(out, into);
- Character c = SUBMIT_TYPES.get(submitType);
- if (c == null) {
- throw new IOException("Invalid submit type: " + submitType);
- }
- out.writeChar(c);
- writeString(out, mergeStrategy);
- }
+ static enum Serializer implements CacheSerializer<EntryKey> {
+ INSTANCE;
- private void readObject(ObjectInputStream in) throws IOException {
- commit = readWithoutMarker(in);
- into = readWithoutMarker(in);
- char t = in.readChar();
- submitType = SUBMIT_TYPES.inverse().get(t);
- if (submitType == null) {
- throw new IOException("Invalid submit type code: " + t);
+ private static final Converter<String, SubmitType> SUBMIT_TYPE_CONVERTER =
+ Enums.stringConverter(SubmitType.class);
+
+ @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());
}
- mergeStrategy = readString(in);
+
+ @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);
+ return new EntryKey(
+ commit,
+ into,
+ SUBMIT_TYPE_CONVERTER.convert(proto.getSubmitType()),
+ proto.getMergeStrategy());
+ }
}
}
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 68b1ff9..5db347e 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -22,6 +22,9 @@
import static com.google.gerrit.index.FieldDef.prefix;
import static com.google.gerrit.index.FieldDef.storedOnly;
import static com.google.gerrit.index.FieldDef.timestamp;
+import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.APPROVAL_CODEC;
+import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.CHANGE_CODEC;
+import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.PATCH_SET_CODEC;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
@@ -64,7 +67,6 @@
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ChangeStatusPredicate;
import com.google.gson.Gson;
-import com.google.gwtorm.protobuf.CodecFactory;
import com.google.gwtorm.protobuf.ProtobufCodec;
import com.google.gwtorm.server.OrmException;
import com.google.protobuf.CodedOutputStream;
@@ -468,15 +470,10 @@
exact(ChangeQueryBuilder.FIELD_EXACTCOMMITTER)
.buildRepeatable(ChangeField::getCommitterNameAndEmail);
- public static final ProtobufCodec<Change> CHANGE_CODEC = CodecFactory.encoder(Change.class);
-
/** Serialized change object, used for pre-populating results. */
public static final FieldDef<ChangeData, byte[]> CHANGE =
storedOnly("_change").build(changeGetter(CHANGE_CODEC::encodeToByteArray));
- public static final ProtobufCodec<PatchSetApproval> APPROVAL_CODEC =
- CodecFactory.encoder(PatchSetApproval.class);
-
/** Serialized approvals for the current patch set, used for pre-populating results. */
public static final FieldDef<ChangeData, Iterable<byte[]>> APPROVAL =
storedOnly("_approval")
@@ -596,9 +593,6 @@
cd ->
cd.patchSets().stream().flatMap(ps -> ps.getGroups().stream()).collect(toSet()));
- public static final ProtobufCodec<PatchSet> PATCH_SET_CODEC =
- CodecFactory.encoder(PatchSet.class);
-
/** Serialized patch set object, used for pre-populating results. */
public static final FieldDef<ChangeData, Iterable<byte[]>> PATCH_SET =
storedOnly("_patch_set").buildRepeatable(cd -> toProtos(PATCH_SET_CODEC, cd.patchSets()));
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index f5fefeb..1bbecc8 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -564,12 +564,7 @@
/** @return all change messages, in chronological order, oldest first. */
public ImmutableList<ChangeMessage> getChangeMessages() {
- return state.allChangeMessages();
- }
-
- /** @return change messages by patch set, in chronological order, oldest first. */
- public ImmutableListMultimap<PatchSet.Id, ChangeMessage> getChangeMessagesByPatchSet() {
- return state.changeMessagesByPatchSet();
+ return state.changeMessages();
}
/** @return inline comments on each revision. */
@@ -670,28 +665,6 @@
return state.readOnlyUntil();
}
- public boolean isPrivate() {
- if (state.isPrivate() == null) {
- return false;
- }
- return state.isPrivate();
- }
-
- public boolean isWorkInProgress() {
- if (state.isWorkInProgress() == null) {
- return false;
- }
- return state.isWorkInProgress();
- }
-
- public Change.Id getRevertOf() {
- return state.revertOf();
- }
-
- public boolean hasReviewStarted() {
- return state.hasReviewStarted();
- }
-
@Override
protected void onLoad(LoadHandle handle)
throws NoSuchChangeException, IOException, ConfigInvalidException {
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index 676dbb8..5658569 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -128,10 +128,7 @@
+ P
+ list(state.submitRecords(), P + list(2, str(4) + P + K) + P)
+ P
- + list(state.allChangeMessages(), changeMessage())
- // Just key overhead for map, already counted messages in previous.
- + P
- + map(state.changeMessagesByPatchSet().asMap(), patchSetId())
+ + list(state.changeMessages(), changeMessage())
+ P
+ map(state.publishedComments().asMap(), comment())
+ T // readOnlyUntil
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index d6472bc..2eb30ff 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.notedb;
+import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ASSIGNEE;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_BRANCH;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHANGE_ID;
@@ -44,7 +45,6 @@
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableTable;
-import com.google.common.collect.LinkedListMultimap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.MultimapBuilder;
@@ -145,7 +145,6 @@
private final Map<ApprovalKey, PatchSetApproval> approvals;
private final List<PatchSetApproval> bufferedApprovals;
private final List<ChangeMessage> allChangeMessages;
- private final ListMultimap<PatchSet.Id, ChangeMessage> changeMessagesByPatchSet;
// Non-final private members filled in during the parsing process.
private String branch;
@@ -193,7 +192,6 @@
reviewerUpdates = new ArrayList<>();
submitRecords = Lists.newArrayListWithExpectedSize(1);
allChangeMessages = new ArrayList<>();
- changeMessagesByPatchSet = LinkedListMultimap.create();
comments = MultimapBuilder.hashKeys().arrayListValues().build();
patchSets = new HashMap<>();
deletedPatchSets = new HashSet<>();
@@ -253,7 +251,7 @@
assignee != null ? assignee.orElse(null) : null,
status,
Sets.newLinkedHashSet(Lists.reverse(pastAssignees)),
- hashtags,
+ firstNonNull(hashtags, ImmutableSet.of()),
patchSets,
buildApprovals(),
ReviewerSet.fromTable(Tables.transpose(reviewers)),
@@ -264,12 +262,11 @@
buildReviewerUpdates(),
submitRecords,
buildAllMessages(),
- buildMessagesByPatchSet(),
comments,
readOnlyUntil,
- isPrivate,
- workInProgress,
- hasReviewStarted,
+ firstNonNull(isPrivate, false),
+ firstNonNull(workInProgress, false),
+ firstNonNull(hasReviewStarted, true),
revertOf);
}
@@ -318,13 +315,6 @@
return Lists.reverse(allChangeMessages);
}
- private ListMultimap<PatchSet.Id, ChangeMessage> buildMessagesByPatchSet() {
- for (Collection<ChangeMessage> v : changeMessagesByPatchSet.asMap().values()) {
- Collections.reverse((List<ChangeMessage>) v);
- }
- return changeMessagesByPatchSet;
- }
-
private void parse(ChangeNotesCommit commit) throws ConfigInvalidException {
Timestamp ts = new Timestamp(commit.getCommitterIdent().getWhen().getTime());
@@ -751,7 +741,6 @@
changeMessage.setMessage(changeMsgString);
changeMessage.setTag(tag);
changeMessage.setRealAuthor(realAccountId);
- changeMessagesByPatchSet.put(psId, changeMessage);
allChangeMessages.add(changeMessage);
}
@@ -1088,8 +1077,6 @@
// (or otherwise missing) patch sets. This is safer than trying to prevent
// insertion, as it will also filter out items racily added after the patch
// set was deleted.
- changeMessagesByPatchSet.keys().retainAll(patchSets.keySet());
-
int pruned =
pruneEntitiesForMissingPatchSets(allChangeMessages, ChangeMessage::getPatchSetId, missing);
pruned +=
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index 1dd944d..78734f9 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -57,33 +57,15 @@
@AutoValue
public abstract class ChangeNotesState {
static ChangeNotesState empty(Change change) {
- return new AutoValue_ChangeNotesState(
- null,
- change.getId(),
- null,
- ImmutableSet.of(),
- ImmutableSet.of(),
- ImmutableList.of(),
- ImmutableList.of(),
- ReviewerSet.empty(),
- ReviewerByEmailSet.empty(),
- ReviewerSet.empty(),
- ReviewerByEmailSet.empty(),
- ImmutableList.of(),
- ImmutableList.of(),
- ImmutableList.of(),
- ImmutableList.of(),
- ImmutableListMultimap.of(),
- ImmutableListMultimap.of(),
- null,
- null,
- null,
- true,
- null);
+ return Builder.empty(change.getId()).build();
+ }
+
+ static Builder builder() {
+ return new AutoValue_ChangeNotesState.Builder();
}
static ChangeNotesState create(
- @Nullable ObjectId metaId,
+ ObjectId metaId,
Change.Id changeId,
Change.Key changeKey,
Timestamp createdOn,
@@ -97,8 +79,8 @@
@Nullable String submissionId,
@Nullable Account.Id assignee,
@Nullable Change.Status status,
- @Nullable Set<Account.Id> pastAssignees,
- @Nullable Set<String> hashtags,
+ Set<Account.Id> pastAssignees,
+ Set<String> hashtags,
Map<PatchSet.Id, PatchSet> patchSets,
ListMultimap<PatchSet.Id, PatchSetApproval> approvals,
ReviewerSet reviewers,
@@ -108,56 +90,55 @@
List<Account.Id> allPastReviewers,
List<ReviewerStatusUpdate> reviewerUpdates,
List<SubmitRecord> submitRecords,
- List<ChangeMessage> allChangeMessages,
- ListMultimap<PatchSet.Id, ChangeMessage> changeMessagesByPatchSet,
+ List<ChangeMessage> changeMessages,
ListMultimap<RevId, Comment> publishedComments,
@Nullable Timestamp readOnlyUntil,
- @Nullable Boolean isPrivate,
- @Nullable Boolean workInProgress,
+ boolean isPrivate,
+ boolean workInProgress,
boolean hasReviewStarted,
@Nullable Change.Id revertOf) {
- if (hashtags == null) {
- hashtags = ImmutableSet.of();
- }
- return new AutoValue_ChangeNotesState(
+ checkNotNull(
metaId,
- changeId,
- new AutoValue_ChangeNotesState_ChangeColumns(
- changeKey,
- createdOn,
- lastUpdatedOn,
- owner,
- branch,
- currentPatchSetId,
- subject,
- topic,
- originalSubject,
- submissionId,
- assignee,
- status,
- isPrivate,
- workInProgress,
- hasReviewStarted,
- revertOf),
- ImmutableSet.copyOf(pastAssignees),
- ImmutableSet.copyOf(hashtags),
- ImmutableList.copyOf(patchSets.entrySet()),
- ImmutableList.copyOf(approvals.entries()),
- reviewers,
- reviewersByEmail,
- pendingReviewers,
- pendingReviewersByEmail,
- ImmutableList.copyOf(allPastReviewers),
- ImmutableList.copyOf(reviewerUpdates),
- ImmutableList.copyOf(submitRecords),
- ImmutableList.copyOf(allChangeMessages),
- ImmutableListMultimap.copyOf(changeMessagesByPatchSet),
- ImmutableListMultimap.copyOf(publishedComments),
- readOnlyUntil,
- isPrivate,
- workInProgress,
- hasReviewStarted,
- revertOf);
+ "metaId is required when passing arguments to create(...). To create an empty %s without"
+ + " NoteDb data, use empty(...) instead",
+ ChangeNotesState.class.getSimpleName());
+ return builder()
+ .metaId(metaId)
+ .changeId(changeId)
+ .columns(
+ new AutoValue_ChangeNotesState_ChangeColumns.Builder()
+ .changeKey(changeKey)
+ .createdOn(createdOn)
+ .lastUpdatedOn(lastUpdatedOn)
+ .owner(owner)
+ .branch(branch)
+ .currentPatchSetId(currentPatchSetId)
+ .subject(subject)
+ .topic(topic)
+ .originalSubject(originalSubject)
+ .submissionId(submissionId)
+ .assignee(assignee)
+ .status(status)
+ .isPrivate(isPrivate)
+ .isWorkInProgress(workInProgress)
+ .hasReviewStarted(hasReviewStarted)
+ .revertOf(revertOf)
+ .build())
+ .pastAssignees(pastAssignees)
+ .hashtags(hashtags)
+ .patchSets(patchSets.entrySet())
+ .approvals(approvals.entries())
+ .reviewers(reviewers)
+ .reviewersByEmail(reviewersByEmail)
+ .pendingReviewers(pendingReviewers)
+ .pendingReviewersByEmail(pendingReviewersByEmail)
+ .allPastReviewers(allPastReviewers)
+ .reviewerUpdates(reviewerUpdates)
+ .submitRecords(submitRecords)
+ .changeMessages(changeMessages)
+ .publishedComments(publishedComments)
+ .readOnlyUntil(readOnlyUntil)
+ .build();
}
/**
@@ -201,17 +182,51 @@
@Nullable
abstract Change.Status status();
- @Nullable
- abstract Boolean isPrivate();
+ abstract boolean isPrivate();
- @Nullable
- abstract Boolean isWorkInProgress();
+ abstract boolean isWorkInProgress();
- @Nullable
- abstract Boolean hasReviewStarted();
+ abstract boolean hasReviewStarted();
@Nullable
abstract Change.Id revertOf();
+
+ @AutoValue.Builder
+ abstract static class Builder {
+ abstract Builder changeKey(Change.Key changeKey);
+
+ abstract Builder createdOn(Timestamp createdOn);
+
+ abstract Builder lastUpdatedOn(Timestamp lastUpdatedOn);
+
+ abstract Builder owner(Account.Id owner);
+
+ abstract Builder branch(String branch);
+
+ abstract Builder currentPatchSetId(@Nullable PatchSet.Id currentPatchSetId);
+
+ abstract Builder subject(String subject);
+
+ abstract Builder topic(@Nullable String topic);
+
+ abstract Builder originalSubject(@Nullable String originalSubject);
+
+ abstract Builder submissionId(@Nullable String submissionId);
+
+ abstract Builder assignee(@Nullable Account.Id assignee);
+
+ abstract Builder status(@Nullable Change.Status status);
+
+ abstract Builder isPrivate(boolean isPrivate);
+
+ abstract Builder isWorkInProgress(boolean isWorkInProgress);
+
+ abstract Builder hasReviewStarted(boolean hasReviewStarted);
+
+ abstract Builder revertOf(@Nullable Change.Id revertOf);
+
+ abstract ChangeColumns build();
+ }
}
// Only null if NoteDb is disabled.
@@ -247,27 +262,13 @@
abstract ImmutableList<SubmitRecord> submitRecords();
- abstract ImmutableList<ChangeMessage> allChangeMessages();
-
- abstract ImmutableListMultimap<PatchSet.Id, ChangeMessage> changeMessagesByPatchSet();
+ abstract ImmutableList<ChangeMessage> changeMessages();
abstract ImmutableListMultimap<RevId, Comment> publishedComments();
@Nullable
abstract Timestamp readOnlyUntil();
- @Nullable
- abstract Boolean isPrivate();
-
- @Nullable
- abstract Boolean isWorkInProgress();
-
- @Nullable
- abstract Boolean hasReviewStarted();
-
- @Nullable
- abstract Change.Id revertOf();
-
Change newChange(Project.NameKey project) {
ChangeColumns c = checkNotNull(columns(), "columns are required");
Change change =
@@ -325,9 +326,9 @@
change.setLastUpdatedOn(c.lastUpdatedOn());
change.setSubmissionId(c.submissionId());
change.setAssignee(c.assignee());
- change.setPrivate(c.isPrivate() == null ? false : c.isPrivate());
- change.setWorkInProgress(c.isWorkInProgress() == null ? false : c.isWorkInProgress());
- change.setReviewStarted(c.hasReviewStarted() == null ? false : c.hasReviewStarted());
+ change.setPrivate(c.isPrivate());
+ change.setWorkInProgress(c.isWorkInProgress());
+ change.setReviewStarted(c.hasReviewStarted());
change.setRevertOf(c.revertOf());
if (!patchSets().isEmpty()) {
@@ -338,4 +339,61 @@
change.clearCurrentPatchSet();
}
}
+
+ @AutoValue.Builder
+ abstract static class Builder {
+ static Builder empty(Change.Id changeId) {
+ return new AutoValue_ChangeNotesState.Builder()
+ .changeId(changeId)
+ .pastAssignees(ImmutableSet.of())
+ .hashtags(ImmutableSet.of())
+ .patchSets(ImmutableList.of())
+ .approvals(ImmutableList.of())
+ .reviewers(ReviewerSet.empty())
+ .reviewersByEmail(ReviewerByEmailSet.empty())
+ .pendingReviewers(ReviewerSet.empty())
+ .pendingReviewersByEmail(ReviewerByEmailSet.empty())
+ .allPastReviewers(ImmutableList.of())
+ .reviewerUpdates(ImmutableList.of())
+ .submitRecords(ImmutableList.of())
+ .changeMessages(ImmutableList.of())
+ .publishedComments(ImmutableListMultimap.of());
+ }
+
+ abstract Builder metaId(ObjectId metaId);
+
+ abstract Builder changeId(Change.Id changeId);
+
+ abstract Builder columns(ChangeColumns columns);
+
+ abstract Builder pastAssignees(Set<Account.Id> pastAssignees);
+
+ abstract Builder hashtags(Set<String> hashtags);
+
+ abstract Builder patchSets(Iterable<Map.Entry<PatchSet.Id, PatchSet>> patchSets);
+
+ abstract Builder approvals(Iterable<Map.Entry<PatchSet.Id, PatchSetApproval>> approvals);
+
+ abstract Builder reviewers(ReviewerSet reviewers);
+
+ abstract Builder reviewersByEmail(ReviewerByEmailSet reviewersByEmail);
+
+ abstract Builder pendingReviewers(ReviewerSet pendingReviewers);
+
+ abstract Builder pendingReviewersByEmail(ReviewerByEmailSet pendingReviewersByEmail);
+
+ abstract Builder allPastReviewers(List<Account.Id> allPastReviewers);
+
+ abstract Builder reviewerUpdates(List<ReviewerStatusUpdate> reviewerUpdates);
+
+ abstract Builder submitRecords(List<SubmitRecord> submitRecords);
+
+ abstract Builder changeMessages(List<ChangeMessage> changeMessages);
+
+ abstract Builder publishedComments(ListMultimap<RevId, Comment> publishedComments);
+
+ abstract Builder readOnlyUntil(@Nullable Timestamp readOnlyUntil);
+
+ abstract ChangeNotesState build();
+ }
}
diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java
index b13d921..3a17965 100644
--- a/java/com/google/gerrit/server/permissions/ChangeControl.java
+++ b/java/com/google/gerrit/server/permissions/ChangeControl.java
@@ -29,6 +29,7 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
import com.google.gerrit.server.query.change.ChangeData;
@@ -47,11 +48,16 @@
static class Factory {
private final ChangeData.Factory changeDataFactory;
private final ChangeNotes.Factory notesFactory;
+ private final IdentifiedUser.GenericFactory identifiedUserFactory;
@Inject
- Factory(ChangeData.Factory changeDataFactory, ChangeNotes.Factory notesFactory) {
+ Factory(
+ ChangeData.Factory changeDataFactory,
+ ChangeNotes.Factory notesFactory,
+ IdentifiedUser.GenericFactory identifiedUserFactory) {
this.changeDataFactory = changeDataFactory;
this.notesFactory = notesFactory;
+ this.identifiedUserFactory = identifiedUserFactory;
}
ChangeControl create(
@@ -61,17 +67,22 @@
}
ChangeControl create(RefControl refControl, ChangeNotes notes) {
- return new ChangeControl(changeDataFactory, refControl, notes);
+ return new ChangeControl(changeDataFactory, identifiedUserFactory, refControl, notes);
}
}
private final ChangeData.Factory changeDataFactory;
+ private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final RefControl refControl;
private final ChangeNotes notes;
private ChangeControl(
- ChangeData.Factory changeDataFactory, RefControl refControl, ChangeNotes notes) {
+ ChangeData.Factory changeDataFactory,
+ IdentifiedUser.GenericFactory identifiedUserFactory,
+ RefControl refControl,
+ ChangeNotes notes) {
this.changeDataFactory = changeDataFactory;
+ this.identifiedUserFactory = identifiedUserFactory;
this.refControl = refControl;
this.notes = notes;
}
@@ -84,7 +95,8 @@
if (getUser().equals(who)) {
return this;
}
- return new ChangeControl(changeDataFactory, refControl.forUser(who), notes);
+ return new ChangeControl(
+ changeDataFactory, identifiedUserFactory, refControl.forUser(who), notes);
}
private CurrentUser getUser() {
@@ -261,6 +273,11 @@
}
@Override
+ public ForChange absentUser(Account.Id id) {
+ return user(identifiedUserFactory.create(id));
+ }
+
+ @Override
public String resourcePath() {
if (resourcePath == null) {
resourcePath =
diff --git a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
index 02eed30..490b45e 100644
--- a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
@@ -79,8 +79,8 @@
}
@Override
- public WithUser absentUser(Account.Id user) {
- IdentifiedUser identifiedUser = identifiedUserFactory.create(checkNotNull(user, "user"));
+ public WithUser absentUser(Account.Id id) {
+ IdentifiedUser identifiedUser = identifiedUserFactory.create(checkNotNull(id, "user"));
return new WithUserImpl(identifiedUser);
}
diff --git a/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java b/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
index 35b6e0d..431bfd9 100644
--- a/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.permissions;
import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
@@ -129,6 +130,11 @@
}
@Override
+ public ForProject absentUser(Account.Id id) {
+ return this;
+ }
+
+ @Override
public String resourcePath() {
throw new UnsupportedOperationException(
"FailedPermissionBackend is not scoped to a resource");
@@ -182,6 +188,11 @@
}
@Override
+ public ForRef absentUser(Account.Id id) {
+ return this;
+ }
+
+ @Override
public String resourcePath() {
throw new UnsupportedOperationException(
"FailedPermissionBackend is not scoped to a resource");
@@ -234,6 +245,11 @@
}
@Override
+ public ForChange absentUser(Account.Id id) {
+ return this;
+ }
+
+ @Override
public String resourcePath() {
throw new UnsupportedOperationException(
"FailedPermissionBackend is not scoped to a resource");
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java
index 4cbd77e..8cdb61d 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -112,7 +112,7 @@
*
* <p>Usage should be very limited as this can expose a group-oracle.
*/
- public abstract WithUser absentUser(Account.Id user);
+ public abstract WithUser absentUser(Account.Id id);
/**
* Check whether this {@code PermissionBackend} respects the same global capabilities as the
@@ -305,6 +305,9 @@
/** Returns a new instance rescoped to same project, but different {@code user}. */
public abstract ForProject user(CurrentUser user);
+ /** @see PermissionBackend#absentUser(Account.Id) */
+ public abstract ForProject absentUser(Account.Id id);
+
/** Returns an instance scoped for {@code ref} in this project. */
public abstract ForRef ref(String ref);
@@ -413,6 +416,9 @@
/** Returns a new instance rescoped to same reference, but different {@code user}. */
public abstract ForRef user(CurrentUser user);
+ /** @see PermissionBackend#absentUser(Account.Id) */
+ public abstract ForRef absentUser(Account.Id id);
+
/** Returns an instance scoped to change. */
public abstract ForChange change(ChangeData cd);
@@ -471,6 +477,9 @@
/** Returns a new instance rescoped to same change, but different {@code user}. */
public abstract ForChange user(CurrentUser user);
+ /** @see PermissionBackend#absentUser(Account.Id) */
+ public abstract ForChange absentUser(Account.Id id);
+
/** Verify scoped user can {@code perm}, throwing if denied. */
public abstract void check(ChangePermissionOrLabel perm)
throws AuthException, PermissionBackendException;
diff --git a/java/com/google/gerrit/server/permissions/ProjectControl.java b/java/com/google/gerrit/server/permissions/ProjectControl.java
index dbd60ea..2d2a64d 100644
--- a/java/com/google/gerrit/server/permissions/ProjectControl.java
+++ b/java/com/google/gerrit/server/permissions/ProjectControl.java
@@ -20,6 +20,7 @@
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
@@ -27,6 +28,7 @@
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.config.GitReceivePackGroups;
import com.google.gerrit.server.config.GitUploadPackGroups;
@@ -67,6 +69,7 @@
private final ChangeControl.Factory changeControlFactory;
private final PermissionCollection.Factory permissionFilter;
private final DefaultRefFilter.Factory refFilterFactory;
+ private final IdentifiedUser.GenericFactory identifiedUserFactory;
private List<SectionMatcher> allSections;
private Map<String, RefControl> refControls;
@@ -80,6 +83,7 @@
ChangeControl.Factory changeControlFactory,
PermissionBackend permissionBackend,
DefaultRefFilter.Factory refFilterFactory,
+ IdentifiedUser.GenericFactory identifiedUserFactory,
@Assisted CurrentUser who,
@Assisted ProjectState ps) {
this.changeControlFactory = changeControlFactory;
@@ -88,6 +92,7 @@
this.permissionFilter = permissionFilter;
this.permissionBackend = permissionBackend;
this.refFilterFactory = refFilterFactory;
+ this.identifiedUserFactory = identifiedUserFactory;
user = who;
state = ps;
}
@@ -101,6 +106,7 @@
changeControlFactory,
permissionBackend,
refFilterFactory,
+ identifiedUserFactory,
who,
state);
// Not per-user, and reusing saves lookup time.
@@ -132,7 +138,7 @@
RefControl ctl = refControls.get(refName);
if (ctl == null) {
PermissionCollection relevant = permissionFilter.filter(access(), refName, user);
- ctl = new RefControl(this, refName, relevant);
+ ctl = new RefControl(identifiedUserFactory, this, refName, relevant);
refControls.put(refName, ctl);
}
return ctl;
@@ -327,6 +333,11 @@
}
@Override
+ public ForProject absentUser(Account.Id id) {
+ return user(identifiedUserFactory.create(id));
+ }
+
+ @Override
public String resourcePath() {
if (resourcePath == null) {
resourcePath = "/projects/" + getProjectState().getName();
diff --git a/java/com/google/gerrit/server/permissions/RefControl.java b/java/com/google/gerrit/server/permissions/RefControl.java
index 28781ea..cd1f84a 100644
--- a/java/com/google/gerrit/server/permissions/RefControl.java
+++ b/java/com/google/gerrit/server/permissions/RefControl.java
@@ -21,10 +21,12 @@
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.data.PermissionRule.Action;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
import com.google.gerrit.server.permissions.PermissionBackend.ForRef;
@@ -39,6 +41,7 @@
/** Manages access control for Git references (aka branches, tags). */
class RefControl {
+ private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final ProjectControl projectControl;
private final String refName;
@@ -52,7 +55,12 @@
private Boolean canForgeCommitter;
private Boolean isVisible;
- RefControl(ProjectControl projectControl, String ref, PermissionCollection relevant) {
+ RefControl(
+ IdentifiedUser.GenericFactory identifiedUserFactory,
+ ProjectControl projectControl,
+ String ref,
+ PermissionCollection relevant) {
+ this.identifiedUserFactory = identifiedUserFactory;
this.projectControl = projectControl;
this.refName = ref;
this.relevant = relevant;
@@ -71,7 +79,7 @@
if (relevant.isUserSpecific()) {
return newCtl.controlForRef(refName);
}
- return new RefControl(newCtl, refName, relevant);
+ return new RefControl(identifiedUserFactory, newCtl, refName, relevant);
}
/** Is this user a ref owner? */
@@ -404,6 +412,11 @@
}
@Override
+ public ForRef absentUser(Account.Id id) {
+ return user(identifiedUserFactory.create(id));
+ }
+
+ @Override
public String resourcePath() {
if (resourcePath == null) {
resourcePath =
diff --git a/java/com/google/gerrit/server/restapi/change/PostReviewers.java b/java/com/google/gerrit/server/restapi/change/PostReviewers.java
index 46955e8..65c7db7 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReviewers.java
@@ -98,7 +98,6 @@
private final AccountLoader.Factory accountLoaderFactory;
private final Provider<ReviewDb> dbProvider;
private final ChangeData.Factory changeDataFactory;
- private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final Config cfg;
private final ReviewerJson json;
private final NotesMigration migration;
@@ -118,7 +117,6 @@
Provider<ReviewDb> db,
ChangeData.Factory changeDataFactory,
RetryHelper retryHelper,
- IdentifiedUser.GenericFactory identifiedUserFactory,
@GerritServerConfig Config cfg,
ReviewerJson json,
NotesMigration migration,
@@ -135,7 +133,6 @@
this.accountLoaderFactory = accountLoaderFactory;
this.dbProvider = db;
this.changeDataFactory = changeDataFactory;
- this.identifiedUserFactory = identifiedUserFactory;
this.cfg = cfg;
this.json = json;
this.migration = migration;
@@ -376,18 +373,18 @@
private boolean isValidReviewer(Account member, PermissionBackend.ForRef perm)
throws PermissionBackendException {
- if (member.isActive()) {
- IdentifiedUser user = identifiedUserFactory.create(member.getId());
- // Does not account for draft status as a user might want to let a
- // reviewer see a draft.
- try {
- perm.user(user).check(RefPermission.READ);
- return true;
- } catch (AuthException e) {
- return false;
- }
+ if (!member.isActive()) {
+ return false;
}
- return false;
+
+ // Does not account for draft status as a user might want to let a
+ // reviewer see a draft.
+ try {
+ perm.absentUser(member.getId()).check(RefPermission.READ);
+ return true;
+ } catch (AuthException e) {
+ return false;
+ }
}
private Addition fail(String reviewer, String error) {
@@ -464,8 +461,8 @@
if (migration.readChanges() && state == CC) {
result.ccs = Lists.newArrayListWithCapacity(opResult.addedCCs().size());
for (Account.Id accountId : opResult.addedCCs()) {
- IdentifiedUser u = identifiedUserFactory.create(accountId);
- result.ccs.add(json.format(new ReviewerInfo(accountId.get()), perm.user(u), cd));
+ result.ccs.add(
+ json.format(new ReviewerInfo(accountId.get()), perm.absentUser(accountId), cd));
}
accountLoaderFactory.create(true).fill(result.ccs);
for (Address a : reviewersByEmail) {
@@ -475,11 +472,10 @@
result.reviewers = Lists.newArrayListWithCapacity(opResult.addedReviewers().size());
for (PatchSetApproval psa : opResult.addedReviewers()) {
// New reviewers have value 0, don't bother normalizing.
- IdentifiedUser u = identifiedUserFactory.create(psa.getAccountId());
result.reviewers.add(
json.format(
new ReviewerInfo(psa.getAccountId().get()),
- perm.user(u),
+ perm.absentUser(psa.getAccountId()),
cd,
ImmutableList.of(psa)));
}
diff --git a/java/com/google/gerrit/server/restapi/change/Submit.java b/java/com/google/gerrit/server/restapi/change/Submit.java
index be63e5d..54ecd18 100644
--- a/java/com/google/gerrit/server/restapi/change/Submit.java
+++ b/java/com/google/gerrit/server/restapi/change/Submit.java
@@ -18,7 +18,6 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
-import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Sets;
@@ -33,12 +32,10 @@
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
@@ -119,7 +116,6 @@
private final GitRepositoryManager repoManager;
private final PermissionBackend permissionBackend;
private final ChangeData.Factory changeDataFactory;
- private final ChangeMessagesUtil cmUtil;
private final ChangeNotes.Factory changeNotesFactory;
private final Provider<MergeOp> mergeOpProvider;
private final Provider<MergeSuperSet> mergeSuperSet;
@@ -141,7 +137,6 @@
GitRepositoryManager repoManager,
PermissionBackend permissionBackend,
ChangeData.Factory changeDataFactory,
- ChangeMessagesUtil cmUtil,
ChangeNotes.Factory changeNotesFactory,
Provider<MergeOp> mergeOpProvider,
Provider<MergeSuperSet> mergeSuperSet,
@@ -154,7 +149,6 @@
this.repoManager = repoManager;
this.permissionBackend = permissionBackend;
this.changeDataFactory = changeDataFactory;
- this.cmUtil = cmUtil;
this.changeNotesFactory = changeNotesFactory;
this.mergeOpProvider = mergeOpProvider;
this.mergeSuperSet = mergeSuperSet;
@@ -237,11 +231,8 @@
case MERGED:
return change;
case NEW:
- ChangeMessage msg = getConflictMessage(rsrc);
- if (msg != null) {
- throw new ResourceConflictException(msg.getMessage());
- }
- // $FALL-THROUGH$
+ throw new RestApiException(
+ "change unexpectedly had status " + change.getStatus() + " after submit attempt");
case ABANDONED:
default:
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
@@ -394,18 +385,6 @@
.setEnabled(Boolean.TRUE.equals(enabled));
}
- /**
- * If the merge was attempted and it failed the system usually writes a comment as a ChangeMessage
- * and sets status to NEW. Find the relevant message and return it.
- */
- public ChangeMessage getConflictMessage(RevisionResource rsrc) throws OrmException {
- return FluentIterable.from(
- cmUtil.byPatchSet(dbProvider.get(), rsrc.getNotes(), rsrc.getPatchSet().getId()))
- .filter(cm -> cm.getAuthor() == null)
- .last()
- .orNull();
- }
-
public Collection<ChangeData> unmergeableChanges(ChangeSet cs) throws OrmException, IOException {
Set<ChangeData> mergeabilityMap = new HashSet<>();
for (ChangeData change : cs.changes()) {
diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index 3864676..1ab1124 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -42,6 +42,7 @@
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/server/cache/testing",
"//java/com/google/gerrit/server/group/testing",
"//java/com/google/gerrit/server/project/testing:project-test-util",
"//java/com/google/gerrit/server/restapi",
diff --git a/javatests/com/google/gerrit/server/auth/oauth/OAuthTokenCacheTest.java b/javatests/com/google/gerrit/server/auth/oauth/OAuthTokenCacheTest.java
new file mode 100644
index 0000000..586c065
--- /dev/null
+++ b/javatests/com/google/gerrit/server/auth/oauth/OAuthTokenCacheTest.java
@@ -0,0 +1,74 @@
+package com.google.gerrit.server.auth.oauth;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.cache.testing.SerializedClassSubject.assertThatSerializedClass;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.extensions.auth.oauth.OAuthToken;
+import com.google.gerrit.server.cache.CacheSerializer;
+import com.google.gerrit.server.cache.proto.Cache.OAuthTokenProto;
+import java.lang.reflect.Type;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public final class OAuthTokenCacheTest {
+ @Test
+ public void oAuthTokenSerializer() throws Exception {
+ OAuthToken token = new OAuthToken("token", "secret", "raw", 12345L, "provider");
+ CacheSerializer<OAuthToken> s = new OAuthTokenCache.Serializer();
+ byte[] serialized = s.serialize(token);
+ assertThat(OAuthTokenProto.parseFrom(serialized))
+ .isEqualTo(
+ OAuthTokenProto.newBuilder()
+ .setToken("token")
+ .setSecret("secret")
+ .setRaw("raw")
+ .setExpiresAt(12345L)
+ .setProviderId("provider")
+ .build());
+ assertThat(s.deserialize(serialized)).isEqualTo(token);
+ }
+
+ @Test
+ public void oAuthTokenSerializerWithNullProvider() throws Exception {
+ OAuthToken tokenWithNull = new OAuthToken("token", "secret", "raw", 12345L, null);
+ CacheSerializer<OAuthToken> s = new OAuthTokenCache.Serializer();
+ OAuthTokenProto expectedProto =
+ OAuthTokenProto.newBuilder()
+ .setToken("token")
+ .setSecret("secret")
+ .setRaw("raw")
+ .setExpiresAt(12345L)
+ .setProviderId("")
+ .build();
+
+ byte[] serializedWithNull = s.serialize(tokenWithNull);
+ assertThat(OAuthTokenProto.parseFrom(serializedWithNull)).isEqualTo(expectedProto);
+ assertThat(s.deserialize(serializedWithNull)).isEqualTo(tokenWithNull);
+
+ OAuthToken tokenWithEmptyString = new OAuthToken("token", "secret", "raw", 12345L, "");
+ assertThat(tokenWithEmptyString).isEqualTo(tokenWithNull);
+ byte[] serializedWithEmptyString = s.serialize(tokenWithEmptyString);
+ assertThat(OAuthTokenProto.parseFrom(serializedWithEmptyString)).isEqualTo(expectedProto);
+ assertThat(s.deserialize(serializedWithEmptyString)).isEqualTo(tokenWithNull);
+ }
+
+ /**
+ * See {@link com.google.gerrit.server.cache.testing.SerializedClassSubject} for background and
+ * what to do if this test fails.
+ */
+ @Test
+ public void oAuthTokenFields() throws Exception {
+ assertThatSerializedClass(OAuthToken.class)
+ .hasFields(
+ ImmutableMap.<String, Type>builder()
+ .put("token", String.class)
+ .put("secret", String.class)
+ .put("raw", String.class)
+ .put("expiresAt", long.class)
+ .put("providerId", String.class)
+ .build());
+ }
+}
diff --git a/javatests/com/google/gerrit/server/cache/BUILD b/javatests/com/google/gerrit/server/cache/BUILD
index eed4a87..b173957 100644
--- a/javatests/com/google/gerrit/server/cache/BUILD
+++ b/javatests/com/google/gerrit/server/cache/BUILD
@@ -6,7 +6,9 @@
deps = [
"//java/com/google/gerrit/server",
"//lib:guava",
+ "//lib:gwtorm",
"//lib:junit",
+ "//lib:protobuf",
"//lib:truth",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
diff --git a/javatests/com/google/gerrit/server/cache/BooleanCacheSerializerTest.java b/javatests/com/google/gerrit/server/cache/BooleanCacheSerializerTest.java
new file mode 100644
index 0000000..3186620
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/BooleanCacheSerializerTest.java
@@ -0,0 +1,62 @@
+// 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 java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.protobuf.TextFormat;
+import org.junit.Test;
+
+public class BooleanCacheSerializerTest {
+ @Test
+ public void serialize() throws Exception {
+ assertThat(BooleanCacheSerializer.INSTANCE.serialize(true))
+ .isEqualTo(new byte[] {'t', 'r', 'u', 'e'});
+ assertThat(BooleanCacheSerializer.INSTANCE.serialize(false))
+ .isEqualTo(new byte[] {'f', 'a', 'l', 's', 'e'});
+ }
+
+ @Test
+ public void deserialize() throws Exception {
+ assertThat(BooleanCacheSerializer.INSTANCE.deserialize(new byte[] {'t', 'r', 'u', 'e'}))
+ .isEqualTo(true);
+ assertThat(BooleanCacheSerializer.INSTANCE.deserialize(new byte[] {'f', 'a', 'l', 's', 'e'}))
+ .isEqualTo(false);
+ }
+
+ @Test
+ public void deserializeInvalid() throws Exception {
+ assertDeserializeFails(null);
+ assertDeserializeFails("t".getBytes(UTF_8));
+ assertDeserializeFails("tru".getBytes(UTF_8));
+ assertDeserializeFails("trueee".getBytes(UTF_8));
+ assertDeserializeFails("TRUE".getBytes(UTF_8));
+ assertDeserializeFails("f".getBytes(UTF_8));
+ assertDeserializeFails("fal".getBytes(UTF_8));
+ assertDeserializeFails("falseee".getBytes(UTF_8));
+ assertDeserializeFails("FALSE".getBytes(UTF_8));
+ }
+
+ private static void assertDeserializeFails(byte[] in) {
+ try {
+ BooleanCacheSerializer.INSTANCE.deserialize(in);
+ assert_().fail("expected deserialization to fail for \"%s\"", TextFormat.escapeBytes(in));
+ } catch (RuntimeException e) {
+ // Expected.
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/server/cache/EnumCacheSerializerTest.java b/javatests/com/google/gerrit/server/cache/EnumCacheSerializerTest.java
index 0e04d32..60bbb16 100644
--- a/javatests/com/google/gerrit/server/cache/EnumCacheSerializerTest.java
+++ b/javatests/com/google/gerrit/server/cache/EnumCacheSerializerTest.java
@@ -15,6 +15,8 @@
package com.google.gerrit.server.cache;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
+import static java.nio.charset.StandardCharsets.UTF_8;
import org.junit.Test;
@@ -26,6 +28,14 @@
assertRoundTrip(MyEnum.BAZ);
}
+ @Test
+ public void deserializeInvalidValues() throws Exception {
+ assertDeserializeFails(null);
+ assertDeserializeFails("".getBytes(UTF_8));
+ assertDeserializeFails("foo".getBytes(UTF_8));
+ assertDeserializeFails("QUUX".getBytes(UTF_8));
+ }
+
private enum MyEnum {
FOO,
BAR,
@@ -36,4 +46,14 @@
CacheSerializer<MyEnum> s = new EnumCacheSerializer<>(MyEnum.class);
assertThat(s.deserialize(s.serialize(e))).isEqualTo(e);
}
+
+ private static void assertDeserializeFails(byte[] in) {
+ CacheSerializer<MyEnum> s = new EnumCacheSerializer<>(MyEnum.class);
+ try {
+ s.deserialize(in);
+ assert_().fail("expected RuntimeException");
+ } catch (RuntimeException e) {
+ // Expected.
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/server/cache/IntKeyCacheSerializerTest.java b/javatests/com/google/gerrit/server/cache/IntKeyCacheSerializerTest.java
new file mode 100644
index 0000000..7a7c27c
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/IntKeyCacheSerializerTest.java
@@ -0,0 +1,66 @@
+// 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 com.google.gwtorm.client.IntKey;
+import com.google.gwtorm.client.Key;
+import org.junit.Test;
+
+public class IntKeyCacheSerializerTest {
+
+ private static class MyIntKey extends IntKey<Key<?>> {
+ private static final long serialVersionUID = 1L;
+
+ private int val;
+
+ MyIntKey(int val) {
+ this.val = val;
+ }
+
+ @Override
+ public int get() {
+ return val;
+ }
+
+ @Override
+ protected void set(int newValue) {
+ this.val = newValue;
+ }
+ }
+
+ private static final IntKeyCacheSerializer<MyIntKey> SERIALIZER =
+ new IntKeyCacheSerializer<>(MyIntKey::new);
+
+ @Test
+ public void serialize() throws Exception {
+ MyIntKey k = new MyIntKey(1234);
+ byte[] serialized = SERIALIZER.serialize(k);
+ assertThat(serialized).isEqualTo(new byte[] {-46, 9});
+ assertThat(SERIALIZER.deserialize(serialized).get()).isEqualTo(1234);
+ }
+
+ @Test
+ public void deserializeNullFails() throws Exception {
+ try {
+ SERIALIZER.deserialize(null);
+ assert_().fail("expected RuntimeException");
+ } catch (RuntimeException e) {
+ // Expected.
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/server/cache/IntegerCacheSerializerTest.java b/javatests/com/google/gerrit/server/cache/IntegerCacheSerializerTest.java
new file mode 100644
index 0000000..962b797
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/IntegerCacheSerializerTest.java
@@ -0,0 +1,64 @@
+// 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 com.google.common.collect.ImmutableList;
+import com.google.common.primitives.Bytes;
+import com.google.protobuf.TextFormat;
+import org.junit.Test;
+
+public class IntegerCacheSerializerTest {
+ @Test
+ public void serialize() throws Exception {
+ for (int i :
+ ImmutableList.of(
+ Integer.MIN_VALUE,
+ Integer.MIN_VALUE + 20,
+ -1,
+ 0,
+ 1,
+ Integer.MAX_VALUE - 20,
+ Integer.MAX_VALUE)) {
+ assertRoundTrip(i);
+ }
+ }
+
+ @Test
+ public void deserializeInvalidValues() throws Exception {
+ assertDeserializeFails(null);
+ assertDeserializeFails(
+ Bytes.concat(IntegerCacheSerializer.INSTANCE.serialize(1), new byte[] {0, 0, 0, 0}));
+ }
+
+ private static void assertRoundTrip(int i) throws Exception {
+ byte[] serialized = IntegerCacheSerializer.INSTANCE.serialize(i);
+ int result = IntegerCacheSerializer.INSTANCE.deserialize(serialized);
+ assertThat(result)
+ .named("round-trip of %s via \"%s\"", i, TextFormat.escapeBytes(serialized))
+ .isEqualTo(i);
+ }
+
+ private static void assertDeserializeFails(byte[] in) {
+ try {
+ IntegerCacheSerializer.INSTANCE.deserialize(in);
+ assert_().fail("expected RuntimeException");
+ } catch (RuntimeException e) {
+ // Expected.
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java b/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
index 4470f55..5b77094 100644
--- a/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
+++ b/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
@@ -15,10 +15,12 @@
package com.google.gerrit.server.change;
import static com.google.common.truth.Truth.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.protobuf.ByteString;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
@@ -45,11 +47,15 @@
assertThat(s.deserialize(serialized)).isEqualTo(key);
}
- private static ByteString bytes(int... ints) {
- byte[] bytes = new byte[ints.length];
- for (int i = 0; i < ints.length; i++) {
- bytes[i] = (byte) ints[i];
- }
- return ByteString.copyFrom(bytes);
+ /**
+ * See {@link com.google.gerrit.server.cache.testing.SerializedClassSubject} for background and
+ * what to do if this test fails.
+ */
+ @Test
+ public void keyFields() throws Exception {
+ assertThatSerializedClass(ChangeKindCacheImpl.Key.class)
+ .hasFields(
+ 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
new file mode 100644
index 0000000..69fc531
--- /dev/null
+++ b/javatests/com/google/gerrit/server/change/MergeabilityCacheImplTest.java
@@ -0,0 +1,69 @@
+// 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.change;
+
+import static com.google.common.truth.Truth.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.extensions.client.SubmitType;
+import com.google.gerrit.server.cache.proto.Cache.MergeabilityKeyProto;
+import org.eclipse.jgit.lib.ObjectId;
+import org.junit.Test;
+
+public class MergeabilityCacheImplTest {
+ @Test
+ public void keySerializer() throws Exception {
+ MergeabilityCacheImpl.EntryKey key =
+ new MergeabilityCacheImpl.EntryKey(
+ ObjectId.fromString("badc0feebadc0feebadc0feebadc0feebadc0fee"),
+ ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"),
+ SubmitType.MERGE_IF_NECESSARY,
+ "aStrategy");
+ byte[] serialized = MergeabilityCacheImpl.EntryKey.Serializer.INSTANCE.serialize(key);
+ assertThat(MergeabilityKeyProto.parseFrom(serialized))
+ .isEqualTo(
+ MergeabilityKeyProto.newBuilder()
+ .setCommit(
+ bytes(
+ 0xba, 0xdc, 0x0f, 0xee, 0xba, 0xdc, 0x0f, 0xee, 0xba, 0xdc, 0x0f, 0xee,
+ 0xba, 0xdc, 0x0f, 0xee, 0xba, 0xdc, 0x0f, 0xee))
+ .setInto(
+ 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")
+ .setMergeStrategy("aStrategy")
+ .build());
+ assertThat(MergeabilityCacheImpl.EntryKey.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 keyFields() throws Exception {
+ assertThatSerializedClass(MergeabilityCacheImpl.EntryKey.class)
+ .hasFields(
+ ImmutableMap.of(
+ "commit", ObjectId.class,
+ "into", ObjectId.class,
+ "submitType", SubmitType.class,
+ "mergeStrategy", String.class));
+ }
+}
diff --git a/javatests/com/google/gerrit/server/config/ListCapabilitiesTest.java b/javatests/com/google/gerrit/server/config/ListCapabilitiesTest.java
index 935dfc6..fd9c925 100644
--- a/javatests/com/google/gerrit/server/config/ListCapabilitiesTest.java
+++ b/javatests/com/google/gerrit/server/config/ListCapabilitiesTest.java
@@ -87,7 +87,7 @@
}
@Override
- public WithUser absentUser(Id user) {
+ public WithUser absentUser(Id id) {
throw new UnsupportedOperationException();
}
diff --git a/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java b/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
index d242962..834f658 100644
--- a/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
+++ b/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
@@ -78,6 +78,11 @@
}
@Override
+ public ForProject absentUser(Account.Id id) {
+ throw new UnsupportedOperationException("not implemented");
+ }
+
+ @Override
public ForRef ref(String ref) {
throw new UnsupportedOperationException("not implemented");
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index 9b7aad2..d974877 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -442,17 +442,17 @@
// Change created in WIP remains in WIP.
RevCommit commit = writeCommit("Update WIP change\n" + "\n" + "Patch-set: 1\n", true);
ChangeNotesState state = newParser(commit).parseAll();
- assertThat(state.hasReviewStarted()).isFalse();
+ assertThat(state.columns().hasReviewStarted()).isFalse();
// Moving change out of WIP starts review.
commit =
writeCommit("New ready change\n" + "\n" + "Patch-set: 1\n" + "Work-in-progress: false\n");
state = newParser(commit).parseAll();
- assertThat(state.hasReviewStarted()).isTrue();
+ assertThat(state.columns().hasReviewStarted()).isTrue();
// Change created not in WIP has always been in review started state.
state = assertParseSucceeds("New change that doesn't declare WIP\n" + "\n" + "Patch-set: 1\n");
- assertThat(state.hasReviewStarted()).isTrue();
+ assertThat(state.columns().hasReviewStarted()).isTrue();
}
@Test
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index e5a34aa..9d38704 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -1078,7 +1078,6 @@
ChangeNotes notes = newNotes(c);
assertThat(notes.getPatchSets().keySet()).containsExactly(psId1, psId2);
assertThat(notes.getApprovals()).isNotEmpty();
- assertThat(notes.getChangeMessagesByPatchSet()).isNotEmpty();
assertThat(notes.getChangeMessages()).isNotEmpty();
assertThat(notes.getComments()).isNotEmpty();
@@ -1095,7 +1094,6 @@
notes = newNotes(c);
assertThat(notes.getPatchSets().keySet()).containsExactly(psId1);
assertThat(notes.getApprovals()).isEmpty();
- assertThat(notes.getChangeMessagesByPatchSet()).isEmpty();
assertThat(notes.getChangeMessages()).isEmpty();
assertThat(notes.getComments()).isEmpty();
}
@@ -1349,16 +1347,12 @@
update.putReviewer(changeOwner.getAccount().getId(), REVIEWER);
update.setChangeMessage("Just a little code change.\n");
update.commit();
- PatchSet.Id ps1 = c.currentPatchSetId();
ChangeNotes notes = newNotes(c);
- ListMultimap<PatchSet.Id, ChangeMessage> changeMessages = notes.getChangeMessagesByPatchSet();
- assertThat(changeMessages.keySet()).containsExactly(ps1);
-
- ChangeMessage cm = Iterables.getOnlyElement(changeMessages.get(ps1));
+ ChangeMessage cm = Iterables.getOnlyElement(notes.getChangeMessages());
assertThat(cm.getMessage()).isEqualTo("Just a little code change.\n");
assertThat(cm.getAuthor()).isEqualTo(changeOwner.getAccount().getId());
- assertThat(cm.getPatchSetId()).isEqualTo(ps1);
+ assertThat(cm.getPatchSetId()).isEqualTo(c.currentPatchSetId());
}
@Test
@@ -1378,13 +1372,9 @@
ChangeUpdate update = newUpdate(c, changeOwner);
update.setChangeMessage("Testing trailing double newline\n\n");
update.commit();
- PatchSet.Id ps1 = c.currentPatchSetId();
ChangeNotes notes = newNotes(c);
- ListMultimap<PatchSet.Id, ChangeMessage> changeMessages = notes.getChangeMessagesByPatchSet();
- assertThat(changeMessages).hasSize(1);
-
- ChangeMessage cm1 = Iterables.getOnlyElement(changeMessages.get(ps1));
+ ChangeMessage cm1 = Iterables.getOnlyElement(notes.getChangeMessages());
assertThat(cm1.getMessage()).isEqualTo("Testing trailing double newline\n\n");
assertThat(cm1.getAuthor()).isEqualTo(changeOwner.getAccount().getId());
}
@@ -1395,13 +1385,9 @@
ChangeUpdate update = newUpdate(c, changeOwner);
update.setChangeMessage("Testing paragraph 1\n\nTesting paragraph 2\n\nTesting paragraph 3");
update.commit();
- PatchSet.Id ps1 = c.currentPatchSetId();
ChangeNotes notes = newNotes(c);
- ListMultimap<PatchSet.Id, ChangeMessage> changeMessages = notes.getChangeMessagesByPatchSet();
- assertThat(changeMessages).hasSize(1);
-
- ChangeMessage cm1 = Iterables.getOnlyElement(changeMessages.get(ps1));
+ ChangeMessage cm1 = Iterables.getOnlyElement(notes.getChangeMessages());
assertThat(cm1.getMessage())
.isEqualTo(
"Testing paragraph 1\n"
@@ -1429,15 +1415,15 @@
PatchSet.Id ps2 = c.currentPatchSetId();
ChangeNotes notes = newNotes(c);
- ListMultimap<PatchSet.Id, ChangeMessage> changeMessages = notes.getChangeMessagesByPatchSet();
- assertThat(changeMessages).hasSize(2);
+ assertThat(notes.getChangeMessages()).hasSize(2);
- ChangeMessage cm1 = Iterables.getOnlyElement(changeMessages.get(ps1));
+ ChangeMessage cm1 = notes.getChangeMessages().get(0);
+ assertThat(cm1.getPatchSetId()).isEqualTo(ps1);
assertThat(cm1.getMessage()).isEqualTo("This is the change message for the first PS.");
assertThat(cm1.getAuthor()).isEqualTo(changeOwner.getAccount().getId());
- ChangeMessage cm2 = Iterables.getOnlyElement(changeMessages.get(ps2));
- assertThat(cm1.getPatchSetId()).isEqualTo(ps1);
+ ChangeMessage cm2 = notes.getChangeMessages().get(1);
+ assertThat(cm2.getPatchSetId()).isEqualTo(ps2);
assertThat(cm2.getMessage()).isEqualTo("This is the change message for the second PS.");
assertThat(cm2.getAuthor()).isEqualTo(changeOwner.getAccount().getId());
assertThat(cm2.getPatchSetId()).isEqualTo(ps2);
@@ -1459,10 +1445,8 @@
update.commit();
ChangeNotes notes = newNotes(c);
- ListMultimap<PatchSet.Id, ChangeMessage> changeMessages = notes.getChangeMessagesByPatchSet();
- assertThat(changeMessages.keySet()).hasSize(1);
- List<ChangeMessage> cm = changeMessages.get(ps1);
+ List<ChangeMessage> cm = notes.getChangeMessages();
assertThat(cm).hasSize(2);
assertThat(cm.get(0).getMessage()).isEqualTo("First change message.\n");
assertThat(cm.get(0).getAuthor()).isEqualTo(changeOwner.getAccount().getId());
@@ -3266,7 +3250,7 @@
public void privateDefault() throws Exception {
Change c = newChange();
ChangeNotes notes = newNotes(c);
- assertThat(notes.isPrivate()).isFalse();
+ assertThat(notes.getChange().isPrivate()).isFalse();
}
@Test
@@ -3277,7 +3261,7 @@
update.commit();
ChangeNotes notes = newNotes(c);
- assertThat(notes.isPrivate()).isTrue();
+ assertThat(notes.getChange().isPrivate()).isTrue();
}
@Test
@@ -3292,7 +3276,7 @@
update.commit();
ChangeNotes notes = newNotes(c);
- assertThat(notes.isPrivate()).isFalse();
+ assertThat(notes.getChange().isPrivate()).isFalse();
}
@Test
@@ -3397,38 +3381,38 @@
@Test
public void hasReviewStarted() throws Exception {
ChangeNotes notes = newNotes(newChange());
- assertThat(notes.hasReviewStarted()).isTrue();
+ assertThat(notes.getChange().hasReviewStarted()).isTrue();
notes = newNotes(newWorkInProgressChange());
- assertThat(notes.hasReviewStarted()).isFalse();
+ assertThat(notes.getChange().hasReviewStarted()).isFalse();
Change c = newWorkInProgressChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.commit();
notes = newNotes(c);
- assertThat(notes.hasReviewStarted()).isFalse();
+ assertThat(notes.getChange().hasReviewStarted()).isFalse();
update = newUpdate(c, changeOwner);
update.setWorkInProgress(true);
update.commit();
notes = newNotes(c);
- assertThat(notes.hasReviewStarted()).isFalse();
+ assertThat(notes.getChange().hasReviewStarted()).isFalse();
update = newUpdate(c, changeOwner);
update.setWorkInProgress(false);
update.commit();
notes = newNotes(c);
- assertThat(notes.hasReviewStarted()).isTrue();
+ assertThat(notes.getChange().hasReviewStarted()).isTrue();
// Once review is started, setting WIP should have no impact.
c = newChange();
notes = newNotes(c);
- assertThat(notes.hasReviewStarted()).isTrue();
+ assertThat(notes.getChange().hasReviewStarted()).isTrue();
update = newUpdate(c, changeOwner);
update.setWorkInProgress(true);
update.commit();
notes = newNotes(c);
- assertThat(notes.hasReviewStarted()).isTrue();
+ assertThat(notes.getChange().hasReviewStarted()).isTrue();
}
@Test
@@ -3493,7 +3477,7 @@
public void revertOfIsNullByDefault() throws Exception {
Change c = newChange();
ChangeNotes notes = newNotes(c);
- assertThat(notes.getRevertOf()).isNull();
+ assertThat(notes.getChange().getRevertOf()).isNull();
}
@Test
@@ -3504,7 +3488,7 @@
update.setChangeId(c.getKey().get());
update.setRevertOf(changeToRevert.getId().get());
update.commit();
- assertThat(newNotes(c).getRevertOf()).isEqualTo(changeToRevert.getId());
+ assertThat(newNotes(c).getChange().getRevertOf()).isEqualTo(changeToRevert.getId());
}
@Test
diff --git a/javatests/com/google/gerrit/server/permissions/RefControlTest.java b/javatests/com/google/gerrit/server/permissions/RefControlTest.java
index c30803a..7890de8 100644
--- a/javatests/com/google/gerrit/server/permissions/RefControlTest.java
+++ b/javatests/com/google/gerrit/server/permissions/RefControlTest.java
@@ -47,6 +47,7 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.CapabilityCollection;
import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.account.ListGroupMembership;
@@ -202,6 +203,7 @@
@Inject private InMemoryDatabase schemaFactory;
@Inject private ThreadLocalRequestContext requestContext;
@Inject private DefaultRefFilter.Factory refFilterFactory;
+ @Inject private IdentifiedUser.GenericFactory identifiedUserFactory;
@Before
public void setUp() throws Exception {
@@ -986,6 +988,7 @@
changeControlFactory,
permissionBackend,
refFilterFactory,
+ identifiedUserFactory,
new MockUser(name, memberOf),
newProjectState(local));
}
diff --git a/lib/guava.bzl b/lib/guava.bzl
index db85900..e90c2b3 100644
--- a/lib/guava.bzl
+++ b/lib/guava.bzl
@@ -1,5 +1,5 @@
-GUAVA_VERSION = "24.1-jre"
+GUAVA_VERSION = "25.0-jre"
-GUAVA_BIN_SHA1 = "96c528475465aeb22cce60605d230a7e67cebd7b"
+GUAVA_BIN_SHA1 = "7319c34fa5866a85b6bad445adad69d402323129"
GUAVA_DOC_URL = "https://google.github.io/guava/releases/" + GUAVA_VERSION + "/api/docs/"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index b59742e..3f26628 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -283,6 +283,7 @@
as="file"
initial-count="[[fileListIncrement]]"
target-framerate="1">
+ [[_reportRenderedRow(index)]]
<div class="stickyArea">
<div class$="file-row row [[_computePathClass(file.__path, _expandedFilePaths.*)]]"
data-path$="[[file.__path]]" tabindex="-1">
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 0fa037c..c1eb39c 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
@@ -26,6 +26,8 @@
const SIZE_BAR_GAP_WIDTH = 1;
const SIZE_BAR_MIN_WIDTH = 1.5;
+ const RENDER_TIME = 'FileListRenderTime';
+
const FileStatus = {
A: 'Added',
C: 'Copied',
@@ -797,6 +799,12 @@
_computeFilesShown(numFilesShown, files) {
const filesShown = files.base.slice(0, numFilesShown);
this.fire('files-shown-changed', {length: filesShown.length});
+
+ // Start the timer for the rendering work hwere because this is where the
+ // _shownFiles property is being set, and _shownFiles is used in the
+ // dom-repeat binding.
+ this.$.reporting.time(RENDER_TIME);
+
return filesShown;
},
@@ -1175,5 +1183,21 @@
_noDiffsExpanded() {
return this.filesExpanded === GrFileListConstants.FilesExpandedState.NONE;
},
+
+ /**
+ * Method to call via binding when each file list row is rendered. This
+ * allows approximate detection of when the dom-repeat has completed
+ * rendering.
+ * @param {number} index The index of the row being rendered.
+ * @return {string} an empty string.
+ */
+ _reportRenderedRow(index) {
+ if (index === this._shownFiles.length - 1) {
+ this.async(() => {
+ this.$.reporting.timeEnd(RENDER_TIME);
+ }, 1);
+ }
+ return '';
+ },
});
})();
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 8541edf..0e0a39e 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
@@ -127,6 +127,19 @@
assert.isTrue(controlRow.classList.contains('invisible'));
});
+ test('rendering each row calls the _reportRenderedRow method', () => {
+ const renderedStub = sandbox.stub(element, '_reportRenderedRow');
+ element._filesByPath = _.range(10)
+ .reduce((_filesByPath, i) => {
+ _filesByPath['/file' + i] = {lines_inserted: 9};
+ return _filesByPath;
+ }, {});
+ flushAsynchronousOperations();
+ assert.equal(
+ Polymer.dom(element.root).querySelectorAll('.file-row').length, 10);
+ assert.equal(renderedStub.callCount, 10);
+ });
+
test('calculate totals for patch number', () => {
element._filesByPath = {
'/COMMIT_MSG': {
diff --git a/proto/cache.proto b/proto/cache.proto
index 4a84ab1..634b595 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -25,3 +25,23 @@
bytes next = 2;
string strategy_name = 3;
}
+
+// Serialized form of
+// com.google.gerrit.server.change.MergeabilityCacheImpl.EntryKey.
+// Next ID: 5
+message MergeabilityKeyProto {
+ bytes commit = 1;
+ bytes into = 2;
+ string submit_type = 3;
+ string merge_strategy = 4;
+}
+
+// Serialized form of com.google.gerrit.extensions.auth.oauth.OAuthToken.
+// Next ID: 6
+message OAuthTokenProto {
+ string token = 1;
+ string secret = 2;
+ string raw = 3;
+ int64 expires_at = 4;
+ string provider_id = 5;
+}
