Store submit requirements in the change index
We convert submit requirements results to proto and
store the byte[] representation in the change index documents. This
allows submit requirements to be backfilled for change data loaded from
the index. Two tests are added in ChangeIT to ensure that submit
requirements are returned when using the change query API.
The proto converter for submit requirements results is moved to a
separate class since it was previously used by ChangeNotesState. Now
it's also used from ChangeField.
Change-Id: I69df374c8bec1000292bb444586fd4107e68f9dd
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 810cd4d..95fe875 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -70,6 +70,7 @@
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.index.change.StalenessChecker.RefStatePattern;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
+import com.google.gerrit.server.notedb.SubmitRequirementProtoConverter;
import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
@@ -1076,6 +1077,27 @@
return result;
}
+ /** Serialized submit requirements, used for pre-populating results. */
+ public static final FieldDef<ChangeData, Iterable<byte[]>> STORED_SUBMIT_REQUIREMENTS =
+ storedOnly("full_submit_requirements")
+ .buildRepeatable(
+ cd ->
+ toProtos(
+ SubmitRequirementProtoConverter.INSTANCE, cd.submitRequirements().values()),
+ (cd, field) -> parseSubmitRequirements(field, cd));
+
+ private static void parseSubmitRequirements(Iterable<byte[]> values, ChangeData out) {
+ out.setSubmitRequirements(
+ StreamSupport.stream(values.spliterator(), false)
+ .map(
+ f ->
+ SubmitRequirementProtoConverter.INSTANCE.fromProto(
+ Protos.parseUnchecked(
+ SubmitRequirementProtoConverter.INSTANCE.getParser(), f)))
+ .collect(
+ ImmutableMap.toImmutableMap(sr -> sr.submitRequirement(), Function.identity())));
+ }
+
/**
* All values of all refs that were used in the course of indexing this document.
*
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 1fa2e1b..bee0c35 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -151,7 +151,11 @@
@Deprecated static final Schema<ChangeData> V63 = schema(V62, false);
/** Added support for MIN/MAX/ANY for {@link ChangeField#LABEL} */
- static final Schema<ChangeData> V64 = schema(V63, false);
+ @Deprecated static final Schema<ChangeData> V64 = schema(V63, false);
+
+ /** Added new field for submit requirements. */
+ static final Schema<ChangeData> V65 =
+ new Schema.Builder<ChangeData>().add(V64).add(ChangeField.STORED_SUBMIT_REQUIREMENTS).build();
/**
* Name of the change index to be used when contacting index backends or loading configurations.
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index e7da025..4d6b9cf 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -61,14 +61,10 @@
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerSetEntryProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerStatusUpdateProto;
-import com.google.gerrit.server.cache.proto.Cache.SubmitRequirementResultProto;
import com.google.gerrit.server.cache.serialize.CacheSerializer;
import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
-import com.google.gerrit.server.cache.serialize.entities.SubmitRequirementExpressionResultSerializer;
-import com.google.gerrit.server.cache.serialize.entities.SubmitRequirementSerializer;
import com.google.gerrit.server.index.change.ChangeField.StoredSubmitRecord;
import com.google.gson.Gson;
-import com.google.protobuf.Descriptors.FieldDescriptor;
import java.sql.Timestamp;
import java.time.Instant;
import java.util.List;
@@ -478,11 +474,6 @@
private static final Converter<String, ReviewerStateInternal> REVIEWER_STATE_CONVERTER =
Enums.stringConverter(ReviewerStateInternal.class);
- private static final FieldDescriptor SR_APPLICABILITY_EXPR_RESULT_FIELD =
- SubmitRequirementResultProto.getDescriptor().findFieldByNumber(2);
- private static final FieldDescriptor SR_OVERRIDE_EXPR_RESULT_FIELD =
- SubmitRequirementResultProto.getDescriptor().findFieldByNumber(4);
-
@Override
public byte[] serialize(ChangeNotesState object) {
checkArgument(object.metaId() != null, "meta ID is required in: %s", object);
@@ -539,7 +530,10 @@
object.publishedComments().values().forEach(c -> b.addPublishedComment(GSON.toJson(c)));
object
.submitRequirementsResult()
- .forEach(sr -> b.addSubmitRequirementResult(toSubmitRequirementResultProto(sr)));
+ .forEach(
+ sr ->
+ b.addSubmitRequirementResult(
+ SubmitRequirementProtoConverter.INSTANCE.toProto(sr)));
b.setUpdateCount(object.updateCount());
if (object.mergedOn() != null) {
b.setMergedOnMillis(object.mergedOn().getTime());
@@ -634,53 +628,6 @@
return builder.build();
}
- private static SubmitRequirementResultProto toSubmitRequirementResultProto(
- SubmitRequirementResult r) {
- SubmitRequirementResultProto.Builder builder = SubmitRequirementResultProto.newBuilder();
- builder
- .setSubmitRequirement(SubmitRequirementSerializer.serialize(r.submitRequirement()))
- .setCommit(ObjectIdConverter.create().toByteString(r.patchSetCommitId()));
- if (r.applicabilityExpressionResult().isPresent()) {
- builder.setApplicabilityExpressionResult(
- SubmitRequirementExpressionResultSerializer.serialize(
- r.applicabilityExpressionResult().get()));
- }
- builder.setSubmittabilityExpressionResult(
- SubmitRequirementExpressionResultSerializer.serialize(
- r.submittabilityExpressionResult()));
- if (r.overrideExpressionResult().isPresent()) {
- builder.setOverrideExpressionResult(
- SubmitRequirementExpressionResultSerializer.serialize(
- r.overrideExpressionResult().get()));
- }
- return builder.build();
- }
-
- private static SubmitRequirementResult toSubmitRequirementResult(
- SubmitRequirementResultProto proto) {
- SubmitRequirementResult.Builder builder =
- SubmitRequirementResult.builder()
- .patchSetCommitId(ObjectIdConverter.create().fromByteString(proto.getCommit()))
- .submitRequirement(
- SubmitRequirementSerializer.deserialize(proto.getSubmitRequirement()));
- if (proto.hasField(SR_APPLICABILITY_EXPR_RESULT_FIELD)) {
- builder.applicabilityExpressionResult(
- Optional.of(
- SubmitRequirementExpressionResultSerializer.deserialize(
- proto.getApplicabilityExpressionResult())));
- }
- builder.submittabilityExpressionResult(
- SubmitRequirementExpressionResultSerializer.deserialize(
- proto.getSubmittabilityExpressionResult()));
- if (proto.hasField(SR_OVERRIDE_EXPR_RESULT_FIELD)) {
- builder.overrideExpressionResult(
- Optional.of(
- SubmitRequirementExpressionResultSerializer.deserialize(
- proto.getOverrideExpressionResult())));
- }
- return builder.build();
- }
-
@Override
public ChangeNotesState deserialize(byte[] in) {
ChangeNotesStateProto proto = Protos.parseUnchecked(ChangeNotesStateProto.parser(), in);
@@ -728,7 +675,7 @@
.collect(toImmutableListMultimap(HumanComment::getCommitId, c -> c)))
.submitRequirementsResult(
proto.getSubmitRequirementResultList().stream()
- .map(sr -> toSubmitRequirementResult(sr))
+ .map(sr -> SubmitRequirementProtoConverter.INSTANCE.fromProto(sr))
.collect(toImmutableList()))
.updateCount(proto.getUpdateCount())
.mergedOn(proto.getHasMergedOn() ? new Timestamp(proto.getMergedOnMillis()) : null);
diff --git a/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java b/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java
new file mode 100644
index 0000000..416b850
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java
@@ -0,0 +1,88 @@
+// 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.notedb;
+
+import com.google.errorprone.annotations.Immutable;
+import com.google.gerrit.entities.SubmitRequirementResult;
+import com.google.gerrit.entities.converter.ProtoConverter;
+import com.google.gerrit.server.cache.proto.Cache.SubmitRequirementResultProto;
+import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
+import com.google.gerrit.server.cache.serialize.entities.SubmitRequirementExpressionResultSerializer;
+import com.google.gerrit.server.cache.serialize.entities.SubmitRequirementSerializer;
+import com.google.protobuf.Descriptors.FieldDescriptor;
+import com.google.protobuf.Parser;
+import java.util.Optional;
+
+@Immutable
+public enum SubmitRequirementProtoConverter
+ implements ProtoConverter<SubmitRequirementResultProto, SubmitRequirementResult> {
+ INSTANCE;
+
+ private static final FieldDescriptor SR_APPLICABILITY_EXPR_RESULT_FIELD =
+ SubmitRequirementResultProto.getDescriptor().findFieldByNumber(2);
+ private static final FieldDescriptor SR_OVERRIDE_EXPR_RESULT_FIELD =
+ SubmitRequirementResultProto.getDescriptor().findFieldByNumber(4);
+
+ @Override
+ public SubmitRequirementResultProto toProto(SubmitRequirementResult r) {
+ SubmitRequirementResultProto.Builder builder = SubmitRequirementResultProto.newBuilder();
+ builder
+ .setSubmitRequirement(SubmitRequirementSerializer.serialize(r.submitRequirement()))
+ .setCommit(ObjectIdConverter.create().toByteString(r.patchSetCommitId()));
+ if (r.applicabilityExpressionResult().isPresent()) {
+ builder.setApplicabilityExpressionResult(
+ SubmitRequirementExpressionResultSerializer.serialize(
+ r.applicabilityExpressionResult().get()));
+ }
+ builder.setSubmittabilityExpressionResult(
+ SubmitRequirementExpressionResultSerializer.serialize(r.submittabilityExpressionResult()));
+ if (r.overrideExpressionResult().isPresent()) {
+ builder.setOverrideExpressionResult(
+ SubmitRequirementExpressionResultSerializer.serialize(
+ r.overrideExpressionResult().get()));
+ }
+ return builder.build();
+ }
+
+ @Override
+ public SubmitRequirementResult fromProto(SubmitRequirementResultProto proto) {
+ SubmitRequirementResult.Builder builder =
+ SubmitRequirementResult.builder()
+ .patchSetCommitId(ObjectIdConverter.create().fromByteString(proto.getCommit()))
+ .submitRequirement(
+ SubmitRequirementSerializer.deserialize(proto.getSubmitRequirement()));
+ if (proto.hasField(SR_APPLICABILITY_EXPR_RESULT_FIELD)) {
+ builder.applicabilityExpressionResult(
+ Optional.of(
+ SubmitRequirementExpressionResultSerializer.deserialize(
+ proto.getApplicabilityExpressionResult())));
+ }
+ builder.submittabilityExpressionResult(
+ SubmitRequirementExpressionResultSerializer.deserialize(
+ proto.getSubmittabilityExpressionResult()));
+ if (proto.hasField(SR_OVERRIDE_EXPR_RESULT_FIELD)) {
+ builder.overrideExpressionResult(
+ Optional.of(
+ SubmitRequirementExpressionResultSerializer.deserialize(
+ proto.getOverrideExpressionResult())));
+ }
+ return builder.build();
+ }
+
+ @Override
+ public Parser<SubmitRequirementResultProto> getParser() {
+ return SubmitRequirementResultProto.parser();
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index f912250..9879f27 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -947,6 +947,11 @@
return submitRequirements;
}
+ public void setSubmitRequirements(
+ Map<SubmitRequirement, SubmitRequirementResult> submitRequirements) {
+ this.submitRequirements = submitRequirements;
+ }
+
public List<SubmitRecord> submitRecords(SubmitRuleOptions options) {
// If the change is not submitted yet, 'strict' and 'lenient' both have the same result. If the
// change is submitted, SubmitRecord requested with 'strict' will contain just a single entry
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index c08aa7f..d6ae16c 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -4325,6 +4325,61 @@
}
@Test
+ public void submitRequirement_backFilledFromIndexForActiveChanges() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("code-review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ voteLabel(changeId, "code-review", 2);
+
+ // Query the change. ChangeInfo is back-filled from the change index.
+ List<ChangeInfo> changeInfos =
+ gApi.changes()
+ .query()
+ .withQuery("project:{" + project.get() + "} (status:open OR status:closed)")
+ .withOptions(ImmutableSet.of(ListChangesOption.SUBMIT_REQUIREMENTS))
+ .get();
+ assertThat(changeInfos).hasSize(1);
+ assertSubmitRequirementStatus(
+ changeInfos.get(0).submitRequirements, "code-review", Status.SATISFIED);
+ }
+
+ @Test
+ public void submitRequirement_backFilledFromIndexForClosedChanges() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("code-review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ voteLabel(changeId, "code-review", 2);
+ gApi.changes().id(changeId).current().submit();
+
+ // Query the change. ChangeInfo is back-filled from the change index.
+ List<ChangeInfo> changeInfos =
+ gApi.changes()
+ .query()
+ .withQuery("project:{" + project.get() + "} (status:open OR status:closed)")
+ .withOptions(ImmutableSet.of(ListChangesOption.SUBMIT_REQUIREMENTS))
+ .get();
+ assertThat(changeInfos).hasSize(1);
+ assertSubmitRequirementStatus(
+ changeInfos.get(0).submitRequirements, "code-review", Status.SATISFIED);
+ }
+
+ @Test
public void fourByteEmoji() throws Exception {
// U+1F601 GRINNING FACE WITH SMILING EYES
String smile = new String(Character.toChars(0x1f601));
