Merge "Store submit requirements in the change index"
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 1ee12fe..5caceef 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;
@@ -1078,6 +1079,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));