Store submit requirements for merged changes in NoteDb
Upon merging a change, we check for the changeset (list of changes) to
be merged, and add a batch update op for each change to store the
evaluated submit requirements in NoteDb.
Submit requirements are stored in the change meta ref
(refs/changes/$short-id/$id/meta) in the revision notes of the latest
patchset. The added test in ChangeIT makes sure submit requirements are
stored in the change notes when the change is submitted.
In a follow up change, we will update the 'Get change' endpoint to use
the logic added in this change:
* If the change is active, we will evaluate submit requirements (this is
how it's done now).
* If the change is merged, we will lookup the evaluated submit
requirements from NoteDb and return them to the user.
Change-Id: I2a63f725b9f96647bb438f9f546a48b0fbab4e12
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteJson.java b/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
index 483b2e9..4c41a12 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
@@ -14,9 +14,17 @@
package com.google.gerrit.server.notedb;
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.entities.EntitiesAdapterFactory;
+import com.google.gerrit.json.EnumTypeAdapterFactory;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
+import com.google.gson.TypeAdapter;
+import com.google.gson.stream.JsonReader;
+import com.google.gson.stream.JsonWriter;
import com.google.inject.Singleton;
+import com.google.inject.TypeLiteral;
+import java.io.IOException;
import java.sql.Timestamp;
@Singleton
@@ -26,6 +34,11 @@
static Gson newGson() {
return new GsonBuilder()
.registerTypeAdapter(Timestamp.class, new CommentTimestampAdapter().nullSafe())
+ .registerTypeAdapterFactory(new EnumTypeAdapterFactory())
+ .registerTypeAdapterFactory(EntitiesAdapterFactory.create())
+ .registerTypeAdapter(
+ new TypeLiteral<ImmutableList<String>>() {}.getType(),
+ new ImmutableListAdapter().nullSafe())
.setPrettyPrinting()
.create();
}
@@ -33,4 +46,27 @@
public Gson getGson() {
return gson;
}
+
+ static class ImmutableListAdapter extends TypeAdapter<ImmutableList<String>> {
+
+ @Override
+ public void write(JsonWriter out, ImmutableList<String> value) throws IOException {
+ out.beginArray();
+ for (String v : value) {
+ out.value(v);
+ }
+ out.endArray();
+ }
+
+ @Override
+ public ImmutableList<String> read(JsonReader in) throws IOException {
+ ImmutableList.Builder<String> builder = ImmutableList.builder();
+ in.beginArray();
+ while (in.hasNext()) {
+ builder.add(in.nextString());
+ }
+ in.endArray();
+ return builder.build();
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 5daf28c..6684493 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -50,6 +50,7 @@
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.entities.RobotComment;
import com.google.gerrit.entities.SubmitRecord;
+import com.google.gerrit.entities.SubmitRequirementResult;
import com.google.gerrit.server.AssigneeStatusUpdate;
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
@@ -413,6 +414,16 @@
}
/**
+ * Returns the evaluated submit requirements for the change. We only intend to store submit
+ * requirements in NoteDb for closed changes, hence the result will be an empty list for active
+ * changes, or a list of submit requirements results otherwise. For closed changes, the results
+ * represent the state of evaluating submit requirements for this change when it was merged.
+ */
+ public ImmutableList<SubmitRequirementResult> getSubmitRequirementsResult() {
+ return state.submitRequirementsResult();
+ }
+
+ /**
* @return an ImmutableSet of Account.Ids of all users that have been assigned to this change. The
* order of the set is the order in which they were assigned.
*/
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 8b9d2856..971e0a8 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -65,6 +65,7 @@
import com.google.gerrit.entities.RobotComment;
import com.google.gerrit.entities.SubmissionId;
import com.google.gerrit.entities.SubmitRecord;
+import com.google.gerrit.entities.SubmitRequirementResult;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.server.CurrentUser;
@@ -78,6 +79,7 @@
import com.google.inject.assistedinject.AssistedInject;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Comparator;
import java.util.Date;
import java.util.HashMap;
@@ -129,6 +131,7 @@
private final Map<Account.Id, ReviewerStateInternal> reviewers = new LinkedHashMap<>();
private final Map<Address, ReviewerStateInternal> reviewersByEmail = new LinkedHashMap<>();
private final List<HumanComment> comments = new ArrayList<>();
+ private final List<SubmitRequirementResult> submitRequirementResults = new ArrayList<>();
private String commitSubject;
private String subject;
@@ -302,6 +305,10 @@
this.psDescription = psDescription;
}
+ public void putSubmitRequirementResults(Collection<SubmitRequirementResult> rs) {
+ submitRequirementResults.addAll(rs);
+ }
+
public void putComment(HumanComment.Status status, HumanComment c) {
verifyComment(c);
createDraftUpdateIfNull();
@@ -488,7 +495,7 @@
/** @return the tree id for the updated tree */
private ObjectId storeRevisionNotes(RevWalk rw, ObjectInserter inserter, ObjectId curr)
throws ConfigInvalidException, IOException {
- if (comments.isEmpty() && pushCert == null) {
+ if (submitRequirementResults.isEmpty() && comments.isEmpty() && pushCert == null) {
return null;
}
RevisionNoteMap<ChangeRevisionNote> rnm = getRevisionNoteMap(rw, curr);
@@ -498,6 +505,9 @@
c.tag = tag;
cache.get(c.getCommitId()).putComment(c);
}
+ for (SubmitRequirementResult sr : submitRequirementResults) {
+ cache.get(sr.patchSetCommitId()).putSubmitRequirementResult(sr);
+ }
if (pushCert != null) {
checkState(commit != null);
cache.get(ObjectId.fromString(commit)).setPushCertificate(pushCert);
diff --git a/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java b/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java
index 3c1d359..7998476 100644
--- a/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java
+++ b/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java
@@ -22,16 +22,20 @@
import com.google.common.collect.Maps;
import com.google.common.collect.MultimapBuilder;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.SubmitRequirementResult;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
+import java.util.ArrayList;
import java.util.Collections;
+import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.stream.Collectors;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
@@ -60,11 +64,16 @@
}
}
+ /** Submit requirements are sorted w.r.t. their names before storing in NoteDb. */
+ private final Comparator<SubmitRequirementResult> SUBMIT_REQUIREMENT_RESULT_COMPARATOR =
+ Comparator.comparing(sr -> sr.submitRequirement().name());
+
final byte[] baseRaw;
private final List<? extends Comment> baseComments;
final Map<Comment.Key, Comment> put;
private final Set<Comment.Key> delete;
+ private List<SubmitRequirementResult> submitRequirementResults;
private String pushCert;
private RevisionNoteBuilder(RevisionNote<? extends Comment> base) {
@@ -81,6 +90,7 @@
put = new HashMap<>();
pushCert = null;
}
+ submitRequirementResults = new ArrayList<>();
delete = new HashSet<>();
}
@@ -99,6 +109,10 @@
put.put(comment.key, comment);
}
+ void putSubmitRequirementResult(SubmitRequirementResult result) {
+ submitRequirementResults.add(result);
+ }
+
void deleteComment(Comment.Key key) {
checkArgument(!put.containsKey(key), "cannot both delete and put %s", key);
delete.add(key);
@@ -126,13 +140,19 @@
private void buildNoteJson(ChangeNoteJson noteUtil, OutputStream out) throws IOException {
ListMultimap<Integer, Comment> comments = buildCommentMap();
- if (comments.isEmpty() && pushCert == null) {
+ if (submitRequirementResults.isEmpty() && comments.isEmpty() && pushCert == null) {
return;
}
RevisionNoteData data = new RevisionNoteData();
data.comments = COMMENT_ORDER.sortedCopy(comments.values());
data.pushCert = pushCert;
+ if (!submitRequirementResults.isEmpty()) {
+ data.submitRequirementResults =
+ submitRequirementResults.stream()
+ .sorted(SUBMIT_REQUIREMENT_RESULT_COMPARATOR)
+ .collect(Collectors.toList());
+ }
try (OutputStreamWriter osw = new OutputStreamWriter(out, UTF_8)) {
noteUtil.getGson().toJson(data, osw);
diff --git a/java/com/google/gerrit/server/notedb/RevisionNoteData.java b/java/com/google/gerrit/server/notedb/RevisionNoteData.java
index da15b34..c8770f1 100644
--- a/java/com/google/gerrit/server/notedb/RevisionNoteData.java
+++ b/java/com/google/gerrit/server/notedb/RevisionNoteData.java
@@ -15,15 +15,17 @@
package com.google.gerrit.server.notedb;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.SubmitRequirementResult;
import java.util.List;
/**
* Holds the raw data of a RevisionNote.
*
* <p>It is intended for serialization to JSON only. It is used for human comments and robot
- * comments.
+ * comments, as well as for storing submit requirements.
*/
class RevisionNoteData {
String pushCert;
List<Comment> comments;
+ List<SubmitRequirementResult> submitRequirementResults;
}
diff --git a/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java b/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
new file mode 100644
index 0000000..47948d7
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
@@ -0,0 +1,38 @@
+// 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.gerrit.entities.Change;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.update.BatchUpdateOp;
+import com.google.gerrit.server.update.ChangeContext;
+
+/** A {@link BatchUpdateOp} that stores the evaluated submit requirements of a change in NoteDb. */
+public class StoreSubmitRequirementsOp implements BatchUpdateOp {
+ private final ChangeData.Factory changeDataFactory;
+
+ public StoreSubmitRequirementsOp(ChangeData.Factory changeDataFactory) {
+ this.changeDataFactory = changeDataFactory;
+ }
+
+ @Override
+ public boolean updateChange(ChangeContext ctx) throws Exception {
+ Change change = ctx.getChange();
+ ChangeData changeData = changeDataFactory.create(change);
+ ChangeUpdate update = ctx.getUpdate(change.currentPatchSetId());
+ update.putSubmitRequirementResults(changeData.submitRequirements().values());
+ return !changeData.submitRequirements().isEmpty();
+ }
+}
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index 2b4fb3b..363cdca 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -67,6 +67,7 @@
import com.google.gerrit.server.logging.RequestId;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.StoreSubmitRequirementsOp;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.SubmitRuleOptions;
@@ -96,6 +97,8 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.lib.Constants;
@@ -654,6 +657,16 @@
toSubmit, updateOrderCalculator, submoduleCommits, subscriptionGraph, dryrun);
this.allProjects = updateOrderCalculator.getProjectsInOrder();
List<BatchUpdate> batchUpdates = orm.batchUpdates(allProjects);
+ // Group batch updates by project
+ Map<Project.NameKey, BatchUpdate> batchUpdatesByProject =
+ batchUpdates.stream().collect(Collectors.toMap(b -> b.getProject(), Function.identity()));
+ for (Map.Entry<Change.Id, ChangeData> entry : cs.changesById().entrySet()) {
+ Project.NameKey project = entry.getValue().project();
+ Change.Id changeId = entry.getKey();
+ batchUpdatesByProject
+ .get(project)
+ .addOp(changeId, new StoreSubmitRequirementsOp(changeDataFactory));
+ }
try {
submissionExecutor.setAdditionalBatchUpdateListeners(
ImmutableList.of(new SubmitStrategyListener(submitInput, strategies, commitStatus)));
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 42354ca..fcc0dec 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -69,6 +69,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
+import com.google.common.collect.MoreCollectors;
import com.google.common.truth.ThrowableSubject;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.ChangeIndexedCounter;
@@ -104,6 +105,8 @@
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.entities.SubmitRequirement;
import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult;
+import com.google.gerrit.entities.SubmitRequirementResult;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.api.accounts.DeleteDraftCommentsInput;
@@ -172,6 +175,7 @@
import com.google.gerrit.server.index.change.ChangeIndex;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.server.index.change.IndexedChangeQuery;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.DiffSummary;
import com.google.gerrit.server.patch.DiffSummaryKey;
import com.google.gerrit.server.patch.IntraLineDiff;
@@ -4207,6 +4211,40 @@
}
@Test
+ public void submitRequirement_storedForClosedChanges() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("code-review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange("Add a file", "foo", "content");
+ String changeId = r.getChangeId();
+
+ voteLabel(changeId, "code-review", 2);
+
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+
+ RevisionApi revision = gApi.changes().id(r.getChangeId()).current();
+ revision.review(ReviewInput.approve());
+ revision.submit();
+
+ ChangeNotes notes = notesFactory.create(project, r.getChange().getId());
+
+ SubmitRequirementResult result =
+ notes.getSubmitRequirementsResult().stream().collect(MoreCollectors.onlyElement());
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
+ assertThat(result.submittabilityExpressionResult().status())
+ .isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
+ assertThat(result.submittabilityExpressionResult().expression().expressionString())
+ .isEqualTo("label:code-review=+2");
+ }
+
+ @Test
public void fourByteEmoji() throws Exception {
// U+1F601 GRINNING FACE WITH SMILING EYES
String smile = new String(Character.toChars(0x1f601));
