Submit requirements: Add a custom serializer for ObjectId
Submit requirement results are serialized and stored as Json in NoteDb
when the change is merged. We use a custom Gson serializer (defined in
ChangeNoteJson) that registers adapters to different objects. We have
not previously registered an adapter for the ObjectId class, resulting
in serializing the ObjectId using the JGit format {w1, w2, w3, w4, w5}.
In this change, we implement a custom adapter for ObjectId and ensure
that the "deserialize" method can parse both the JGit and the new format
to preserve backward compatibility.
I also added tests to ensure proper serialization for all submit
requirement entities: SubmitRequirement, SubmitRequirementExpression,
SubmitRequirementResult and SubmitRequirementExpressionResult.
Bug: Google b/202927636
Change-Id: I91d0ceaf8dd0b1c4d5b0245ac97258e1a956fc8e
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteJson.java b/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
index 4c41a12..0d45abf 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
@@ -19,6 +19,9 @@
import com.google.gerrit.json.EnumTypeAdapterFactory;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonParser;
import com.google.gson.TypeAdapter;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonWriter;
@@ -26,6 +29,9 @@
import com.google.inject.TypeLiteral;
import java.io.IOException;
import java.sql.Timestamp;
+import java.util.Arrays;
+import java.util.List;
+import org.eclipse.jgit.lib.ObjectId;
@Singleton
public class ChangeNoteJson {
@@ -39,6 +45,7 @@
.registerTypeAdapter(
new TypeLiteral<ImmutableList<String>>() {}.getType(),
new ImmutableListAdapter().nullSafe())
+ .registerTypeAdapter(ObjectId.class, new ObjectIdAdapter())
.setPrettyPrinting()
.create();
}
@@ -47,6 +54,37 @@
return gson;
}
+ /** Json serializer for the {@link ObjectId} class. */
+ static class ObjectIdAdapter extends TypeAdapter<ObjectId> {
+ private static final List<String> legacyFields = Arrays.asList("w1", "w2", "w3", "w4", "w5");
+
+ @Override
+ public void write(JsonWriter out, ObjectId value) throws IOException {
+ out.value(value.name());
+ }
+
+ @Override
+ public ObjectId read(JsonReader in) throws IOException {
+ JsonElement parsed = new JsonParser().parse(in);
+ if (parsed.isJsonObject() && isJGitFormat(parsed)) {
+ // Some object IDs may have been serialized using the JGit format using the five integers
+ // w1, w2, w3, w4, w5. Detect this case so that we can deserialize properly.
+ int[] raw =
+ legacyFields.stream()
+ .mapToInt(field -> parsed.getAsJsonObject().get(field).getAsInt())
+ .toArray();
+ return ObjectId.fromRaw(raw);
+ }
+ return ObjectId.fromString(parsed.getAsString());
+ }
+
+ /** Return true if the json element contains the JGit serialized format of the Object ID. */
+ private boolean isJGitFormat(JsonElement elem) {
+ JsonObject asObj = elem.getAsJsonObject();
+ return legacyFields.stream().allMatch(field -> asObj.has(field));
+ }
+ }
+
static class ImmutableListAdapter extends TypeAdapter<ImmutableList<String>> {
@Override
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD b/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD
index 8df9292..d68a5c1 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD
@@ -16,5 +16,6 @@
"//lib/truth:truth-proto-extension",
"//proto:cache_java_proto",
"//proto/testing:test_java_proto",
+ "@gson//jar",
],
)
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementJsonSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementJsonSerializerTest.java
new file mode 100644
index 0000000..1df409a
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementJsonSerializerTest.java
@@ -0,0 +1,219 @@
+// Copyright (C) 2021 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.serialize.entities;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult.Status;
+import com.google.gerrit.entities.SubmitRequirementResult;
+import com.google.gerrit.server.notedb.ChangeNoteJson;
+import com.google.gson.Gson;
+import com.google.gson.TypeAdapter;
+import java.util.Optional;
+import org.eclipse.jgit.lib.ObjectId;
+import org.junit.Test;
+
+public class SubmitRequirementJsonSerializerTest {
+ private static final SubmitRequirementExpression srReqExp =
+ SubmitRequirementExpression.create("label:Code-Review=+2");
+
+ private static final String srReqExpSerial = "{\"expressionString\":\"label:Code-Review=+2\"}";
+
+ private static final SubmitRequirement sr =
+ SubmitRequirement.builder()
+ .setName("CR")
+ .setDescription(Optional.of("CR description"))
+ .setApplicabilityExpression(SubmitRequirementExpression.of("branch:refs/heads/master"))
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+2"))
+ .setAllowOverrideInChildProjects(true)
+ .build();
+
+ private static final String srSerial =
+ "{\"name\":\"CR\","
+ + "\"description\":{\"value\":\"CR description\"},"
+ + "\"applicabilityExpression\":{\"value\":"
+ + "{\"expressionString\":\"branch:refs/heads/master\"}},"
+ + "\"submittabilityExpression\":{"
+ + "\"expressionString\":\"label:Code-Review=+2\"},"
+ + "\"overrideExpression\":{\"value\":null},"
+ + "\"allowOverrideInChildProjects\":true}";
+
+ private static final SubmitRequirementExpressionResult srExpResult =
+ SubmitRequirementExpressionResult.create(
+ SubmitRequirementExpression.create("label:Code-Review=MAX AND -label:Code-Review=MIN"),
+ Status.FAIL,
+ /* passingAtoms= */ ImmutableList.of("label:Code-Review=MAX"),
+ /* failingAtoms= */ ImmutableList.of("label:Code-Review=MIN"));
+
+ private static final String srExpResultSerial =
+ "{\"expression\":{\"expressionString\":"
+ + "\"label:Code-Review=MAX AND -label:Code-Review=MIN\"},"
+ + "\"status\":\"FAIL\","
+ + "\"errorMessage\":{\"value\":null},"
+ + "\"passingAtoms\":[\"label:Code-Review=MAX\"],"
+ + "\"failingAtoms\":[\"label:Code-Review=MIN\"]}";
+
+ private static final SubmitRequirementResult srReqResult =
+ SubmitRequirementResult.builder()
+ .submitRequirement(
+ SubmitRequirement.builder()
+ .setName("CR")
+ .setDescription(Optional.of("CR Description"))
+ .setApplicabilityExpression(
+ SubmitRequirementExpression.of("branch:refs/heads/master"))
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("label:\"Code-Review=+2\""))
+ .setOverrideExpression(SubmitRequirementExpression.of("label:Override=+1"))
+ .setAllowOverrideInChildProjects(false)
+ .build())
+ .patchSetCommitId(ObjectId.fromString("4663ab9e9eb49a214e68e60f0fe5d0b6f44f763e"))
+ .applicabilityExpressionResult(
+ Optional.of(
+ SubmitRequirementExpressionResult.create(
+ SubmitRequirementExpression.create("branch:refs/heads/master"),
+ Status.PASS,
+ ImmutableList.of("refs/heads/master"),
+ ImmutableList.of())))
+ .submittabilityExpressionResult(
+ SubmitRequirementExpressionResult.create(
+ SubmitRequirementExpression.create("label:\"Code-Review=+2\""),
+ Status.PASS,
+ /* passingAtoms= */ ImmutableList.of("label:\"Code-Review=+2\""),
+ /* failingAtoms= */ ImmutableList.of()))
+ .overrideExpressionResult(
+ Optional.of(
+ SubmitRequirementExpressionResult.create(
+ SubmitRequirementExpression.create("label:Override=+1"),
+ Status.PASS,
+ /* passingAtoms= */ ImmutableList.of(),
+ /* failingAtoms= */ ImmutableList.of("label:Override=+1"))))
+ .legacy(Optional.of(true))
+ .build();
+
+ private static final String srReqResultSerial =
+ "{\"submitRequirement\":{\"name\":\"CR\",\"description\":{\"value\":\"CR Description\"},"
+ + "\"applicabilityExpression\":{\"value\":{"
+ + "\"expressionString\":\"branch:refs/heads/master\"}},"
+ + "\"submittabilityExpression\":{\"expressionString\":\"label:\\\"Code-Review=+2\\\"\"},"
+ + "\"overrideExpression\":{\"value\":{\"expressionString\":\"label:Override=+1\"}},"
+ + "\"allowOverrideInChildProjects\":false},"
+ + "\"applicabilityExpressionResult\":{\"value\":{"
+ + "\"expression\":{\"expressionString\":\"branch:refs/heads/master\"},"
+ + "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
+ + "\"passingAtoms\":[\"refs/heads/master\"],"
+ + "\"failingAtoms\":[]}},"
+ + "\"submittabilityExpressionResult\":{"
+ + "\"expression\":{\"expressionString\":\"label:\\\"Code-Review=+2\\\"\"},"
+ + "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
+ + "\"passingAtoms\":[\"label:\\\"Code-Review=+2\\\"\"],"
+ + "\"failingAtoms\":[]},"
+ + "\"overrideExpressionResult\":{\"value\":{"
+ + "\"expression\":{\"expressionString\":\"label:Override=+1\"},"
+ + "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
+ + "\"passingAtoms\":[],"
+ + "\"failingAtoms\":[\"label:Override=+1\"]}},"
+ + "\"patchSetCommitId\":\"4663ab9e9eb49a214e68e60f0fe5d0b6f44f763e\","
+ + "\"legacy\":{\"value\":true}}";
+
+ private static final Gson gson = new ChangeNoteJson().getGson();
+
+ @Test
+ public void submitRequirementExpression_serialize() {
+ assertThat(SubmitRequirementExpression.typeAdapter(gson).toJson(srReqExp))
+ .isEqualTo(srReqExpSerial);
+ }
+
+ @Test
+ public void submitRequirementExpression_deserialize() throws Exception {
+ assertThat(SubmitRequirementExpression.typeAdapter(gson).fromJson(srReqExpSerial))
+ .isEqualTo(srReqExp);
+ }
+
+ @Test
+ public void submitRequirementExpression_roundTrip() throws Exception {
+ SubmitRequirementExpression exp = SubmitRequirementExpression.create("label:Code-Review=+2");
+ TypeAdapter<SubmitRequirementExpression> adapter =
+ SubmitRequirementExpression.typeAdapter(gson);
+ assertThat(adapter.fromJson(adapter.toJson(exp))).isEqualTo(exp);
+ }
+
+ @Test
+ public void submitRequirement_serialize() throws Exception {
+ assertThat(SubmitRequirement.typeAdapter(gson).toJson(sr)).isEqualTo(srSerial);
+ }
+
+ @Test
+ public void submitRequirement_deserialize() throws Exception {
+ assertThat(SubmitRequirement.typeAdapter(gson).fromJson(srSerial)).isEqualTo(sr);
+ }
+
+ @Test
+ public void submitRequirement_roundTrip() throws Exception {
+ TypeAdapter<SubmitRequirement> adapter = SubmitRequirement.typeAdapter(gson);
+ assertThat(adapter.fromJson(adapter.toJson(sr))).isEqualTo(sr);
+ }
+
+ @Test
+ public void submitRequirementExpressionResult_serialize() throws Exception {
+ assertThat(SubmitRequirementExpressionResult.typeAdapter(gson).toJson(srExpResult))
+ .isEqualTo(srExpResultSerial);
+ }
+
+ @Test
+ public void submitRequirementExpressionResult_deserialize() throws Exception {
+ assertThat(SubmitRequirementExpressionResult.typeAdapter(gson).fromJson(srExpResultSerial))
+ .isEqualTo(srExpResult);
+ }
+
+ @Test
+ public void submitRequirementExpressionResult_roundtrip() throws Exception {
+ TypeAdapter<SubmitRequirementExpressionResult> adapter =
+ SubmitRequirementExpressionResult.typeAdapter(gson);
+ assertThat(adapter.fromJson(adapter.toJson(srExpResult))).isEqualTo(srExpResult);
+ }
+
+ @Test
+ public void submitRequirementResult_serialize() throws Exception {
+ assertThat(SubmitRequirementResult.typeAdapter(gson).toJson(srReqResult))
+ .isEqualTo(srReqResultSerial);
+ }
+
+ @Test
+ public void submitRequirementResult_deserialize() throws Exception {
+ assertThat(SubmitRequirementResult.typeAdapter(gson).fromJson(srReqResultSerial))
+ .isEqualTo(srReqResult);
+ }
+
+ @Test
+ public void submitRequirementResult_roundTrip() throws Exception {
+ TypeAdapter<SubmitRequirementResult> adapter = SubmitRequirementResult.typeAdapter(gson);
+ assertThat(adapter.fromJson(adapter.toJson(srReqResult))).isEqualTo(srReqResult);
+ }
+
+ @Test
+ public void deserializeSubmitRequirementResult_withJGitPatchsetIdFormat() throws Exception {
+ String srResultSerialJgitFormat =
+ srReqResultSerial.replace(
+ "\"4663ab9e9eb49a214e68e60f0fe5d0b6f44f763e\"",
+ "{\"w1\":1180937118,\"w2\":-1632331231,\"w3\":1315497487,"
+ + "\"w4\":266719414,\"w5\":-196118978}");
+ assertThat(SubmitRequirementResult.typeAdapter(gson).fromJson(srResultSerialJgitFormat))
+ .isEqualTo(srReqResult);
+ }
+}
