From 0a6eb6a52b3f8557109aef9b4cae7e9634c49be0 Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Thu, 13 Jul 2017 10:58:22 +0200 Subject: [PATCH 1/3] Add revertOf to ReviewDb and NoteDb This new field will keep track of changes that revert another change. It will only be populated when a change gets reverted by the API. Future changes in this series will add the new field to the index, populate it on reverts and add it to ChangeJson. Change-Id: I17f71c6db554f4e31b2451416379107abf055b5b --- .../google/gerrit/reviewdb/client/Change.java | 13 +++++++ .../server/notedb/AbstractChangeUpdate.java | 6 +++ .../gerrit/server/notedb/ChangeBundle.java | 3 +- .../gerrit/server/notedb/ChangeNoteUtil.java | 1 + .../gerrit/server/notedb/ChangeNotes.java | 4 ++ .../server/notedb/ChangeNotesParser.java | 21 ++++++++++- .../server/notedb/ChangeNotesState.java | 19 ++++++++-- .../gerrit/server/notedb/ChangeUpdate.java | 16 +++++++- .../server/notedb/NoteDbUpdateManager.java | 3 ++ .../gerrit/server/schema/SchemaVersion.java | 2 +- .../gerrit/server/schema/Schema_156.java | 26 +++++++++++++ .../gerrit/server/notedb/ChangeNotesTest.java | 37 +++++++++++++++++++ 12 files changed, 143 insertions(+), 8 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_156.java diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java index 1a08d17e22..4252c7244e 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java @@ -524,6 +524,10 @@ public final class Change { @Column(id = 22) protected boolean reviewStarted; + /** References a change that this change reverts. */ + @Column(id = 23, notNull = false) + protected Id revertOf; + /** @see com.google.gerrit.server.notedb.NoteDbChangeState */ @Column(id = 101, notNull = false, length = Integer.MAX_VALUE) protected String noteDbState; @@ -564,6 +568,7 @@ public final class Change { workInProgress = other.workInProgress; reviewStarted = other.reviewStarted; noteDbState = other.noteDbState; + revertOf = other.revertOf; } /** Legacy 32 bit integer identity for a change. */ @@ -733,6 +738,14 @@ public final class Change { this.reviewStarted = reviewStarted; } + public void setRevertOf(Id revertOf) { + this.revertOf = revertOf; + } + + public Id getRevertOf() { + return this.revertOf; + } + public String getNoteDbState() { return noteDbState; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java index 472eda1219..6dfe7a375d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java @@ -57,6 +57,7 @@ public abstract class AbstractChangeUpdate { protected PatchSet.Id psId; private ObjectId result; + protected boolean rootOnly; protected AbstractChangeUpdate( Config cfg, @@ -190,6 +191,11 @@ public abstract class AbstractChangeUpdate { /** Whether no updates have been done. */ public abstract boolean isEmpty(); + /** Wether this update can only be a root commit. */ + public boolean isRootOnly() { + return rootOnly; + } + /** * @return the NameKey for the project where the update will be stored, which is not necessarily * the same as the change's project. diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java index d56baedca8..526f3c8cde 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java @@ -233,7 +233,8 @@ public class ChangeBundle { // last time this file was updated. checkColumns(Change.Id.class, 1); - checkColumns(Change.class, 1, 2, 3, 4, 5, 7, 8, 10, 12, 13, 14, 17, 18, 19, 20, 21, 22, 101); + checkColumns( + Change.class, 1, 2, 3, 4, 5, 7, 8, 10, 12, 13, 14, 17, 18, 19, 20, 21, 22, 23, 101); checkColumns(ChangeMessage.Key.class, 1, 2); checkColumns(ChangeMessage.class, 1, 2, 3, 4, 5, 6, 7); checkColumns(PatchSet.Id.class, 1, 2); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java index ce3b664eec..b23c1defb5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java @@ -83,6 +83,7 @@ public class ChangeNoteUtil { public static final FooterKey FOOTER_TOPIC = new FooterKey("Topic"); public static final FooterKey FOOTER_TAG = new FooterKey("Tag"); public static final FooterKey FOOTER_WORK_IN_PROGRESS = new FooterKey("Work-in-progress"); + public static final FooterKey FOOTER_REVERT_OF = new FooterKey("Revert-of"); private static final String AUTHOR = "Author"; private static final String BASE_PATCH_SET = "Base-for-patch-set"; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java index 85e49ce580..b01a8b307c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java @@ -623,6 +623,10 @@ public class ChangeNotes extends AbstractChangeNotes { return state.isWorkInProgress(); } + public Change.Id getRevertOf() { + return state.revertOf(); + } + public boolean hasReviewStarted() { return state.hasReviewStarted(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java index eb366bb92c..4b239ea75e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java @@ -27,6 +27,7 @@ import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET_DE import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PRIVATE; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_READ_ONLY_UNTIL; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_REAL_USER; +import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_REVERT_OF; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_STATUS; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBJECT; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMISSION_ID; @@ -169,6 +170,7 @@ class ChangeNotesParser { private Boolean hasReviewStarted; private ReviewerSet pendingReviewers; private ReviewerByEmailSet pendingReviewersByEmail; + private Change.Id revertOf; ChangeNotesParser( Change.Id changeId, @@ -267,7 +269,8 @@ class ChangeNotesParser { readOnlyUntil, isPrivate, workInProgress, - hasReviewStarted); + hasReviewStarted, + revertOf); } private PatchSet.Id buildCurrentPatchSetId() { @@ -415,6 +418,10 @@ class ChangeNotesParser { parseIsPrivate(commit); } + if (revertOf == null) { + revertOf = parseRevertOf(commit); + } + previousWorkInProgressFooter = null; parseWorkInProgress(commit); @@ -1022,6 +1029,18 @@ class ChangeNotesParser { throw invalidFooter(FOOTER_WORK_IN_PROGRESS, raw); } + private Change.Id parseRevertOf(ChangeNotesCommit commit) throws ConfigInvalidException { + String footer = parseOneFooter(commit, FOOTER_REVERT_OF); + if (footer == null) { + return null; + } + Integer revertOf = Ints.tryParse(footer); + if (revertOf == null) { + throw invalidFooter(FOOTER_REVERT_OF, footer); + } + return new Change.Id(revertOf); + } + private void pruneReviewers() { Iterator> rit = reviewers.cellSet().iterator(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java index f9899e5c7f..1dd944d6d1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java @@ -78,7 +78,8 @@ public abstract class ChangeNotesState { null, null, null, - true); + true, + null); } static ChangeNotesState create( @@ -113,7 +114,8 @@ public abstract class ChangeNotesState { @Nullable Timestamp readOnlyUntil, @Nullable Boolean isPrivate, @Nullable Boolean workInProgress, - boolean hasReviewStarted) { + boolean hasReviewStarted, + @Nullable Change.Id revertOf) { if (hashtags == null) { hashtags = ImmutableSet.of(); } @@ -135,7 +137,8 @@ public abstract class ChangeNotesState { status, isPrivate, workInProgress, - hasReviewStarted), + hasReviewStarted, + revertOf), ImmutableSet.copyOf(pastAssignees), ImmutableSet.copyOf(hashtags), ImmutableList.copyOf(patchSets.entrySet()), @@ -153,7 +156,8 @@ public abstract class ChangeNotesState { readOnlyUntil, isPrivate, workInProgress, - hasReviewStarted); + hasReviewStarted, + revertOf); } /** @@ -205,6 +209,9 @@ public abstract class ChangeNotesState { @Nullable abstract Boolean hasReviewStarted(); + + @Nullable + abstract Change.Id revertOf(); } // Only null if NoteDb is disabled. @@ -258,6 +265,9 @@ public abstract class ChangeNotesState { @Nullable abstract Boolean hasReviewStarted(); + @Nullable + abstract Change.Id revertOf(); + Change newChange(Project.NameKey project) { ChangeColumns c = checkNotNull(columns(), "columns are required"); Change change = @@ -318,6 +328,7 @@ public abstract class ChangeNotesState { 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.setRevertOf(c.revertOf()); if (!patchSets().isEmpty()) { change.setCurrentPatchSet(c.currentPatchSetId(), c.subject(), c.originalSubject()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java index fcde617f93..d692dff2c6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java @@ -32,6 +32,7 @@ import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET_DE import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PRIVATE; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_READ_ONLY_UNTIL; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_REAL_USER; +import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_REVERT_OF; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_STATUS; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBJECT; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMISSION_ID; @@ -156,6 +157,7 @@ public class ChangeUpdate extends AbstractChangeUpdate { private Timestamp readOnlyUntil; private Boolean isPrivate; private Boolean workInProgress; + private Integer revertOf; private ChangeDraftUpdate draftUpdate; private RobotCommentUpdate robotCommentUpdate; @@ -512,6 +514,13 @@ public class ChangeUpdate extends AbstractChangeUpdate { this.groups = groups; } + public void setRevertOf(int revertOf) { + int ownId = getChange().getId().get(); + checkArgument(ownId != revertOf, "A change cannot revert itself"); + this.revertOf = revertOf; + rootOnly = true; + } + /** @return the tree id for the updated tree */ private ObjectId storeRevisionNotes(RevWalk rw, ObjectInserter inserter, ObjectId curr) throws ConfigInvalidException, OrmException, IOException { @@ -755,6 +764,10 @@ public class ChangeUpdate extends AbstractChangeUpdate { addFooter(msg, FOOTER_WORK_IN_PROGRESS, workInProgress); } + if (revertOf != null) { + addFooter(msg, FOOTER_REVERT_OF, revertOf); + } + cb.setMessage(msg.toString()); try { ObjectId treeId = storeRevisionNotes(rw, ins, curr); @@ -804,7 +817,8 @@ public class ChangeUpdate extends AbstractChangeUpdate { && !currentPatchSet && readOnlyUntil == null && isPrivate == null - && workInProgress == null; + && workInProgress == null + && revertOf == null; } ChangeDraftUpdate getDraftUpdate() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java index 45cf2443f1..eef16fb9d6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java @@ -688,6 +688,9 @@ public class NoteDbUpdateManager implements AutoCloseable { ObjectId curr = old; for (U u : updates) { + if (u.isRootOnly() && !old.equals(ObjectId.zeroId())) { + throw new OrmException("Given ChangeUpdate is only allowed on initial commit"); + } ObjectId next = u.apply(or.rw, or.tempIns, curr); if (next == null) { continue; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java index 647a20503d..2a74aeecfc 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java @@ -35,7 +35,7 @@ import java.util.concurrent.TimeUnit; /** A version of the database schema. */ public abstract class SchemaVersion { /** The current schema version. */ - public static final Class C = Schema_155.class; + public static final Class C = Schema_156.class; public static int getBinaryVersion() { return guessVersion(C); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_156.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_156.java new file mode 100644 index 0000000000..fd8fc003ce --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_156.java @@ -0,0 +1,26 @@ +// Copyright (C) 2017 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.schema; + +import com.google.inject.Inject; +import com.google.inject.Provider; + +/** Add revertOf field to change. */ +public class Schema_156 extends SchemaVersion { + @Inject + Schema_156(Provider prior) { + super(prior); + } +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java index 3d12680d99..db8ec25880 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java @@ -3494,6 +3494,43 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { assertThat(notes.getPendingReviewersByEmail().asTable()).isEmpty(); } + @Test + public void revertOfIsNullByDefault() throws Exception { + Change c = newChange(); + ChangeNotes notes = newNotes(c); + assertThat(notes.getRevertOf()).isNull(); + } + + @Test + public void setRevertOfPersistsValue() throws Exception { + Change changeToRevert = newChange(); + Change c = TestChanges.newChange(project, changeOwner.getAccountId()); + ChangeUpdate update = newUpdate(c, changeOwner); + update.setChangeId(c.getKey().get()); + update.setRevertOf(changeToRevert.getId().get()); + update.commit(); + assertThat(newNotes(c).getRevertOf()).isEqualTo(changeToRevert.getId()); + } + + @Test + public void setRevertOfToCurrentChangeFails() throws Exception { + Change c = newChange(); + ChangeUpdate update = newUpdate(c, changeOwner); + exception.expect(IllegalArgumentException.class); + exception.expectMessage("A change cannot revert itself"); + update.setRevertOf(c.getId().get()); + } + + @Test + public void setRevertOfOnChildCommitFails() throws Exception { + Change c = newChange(); + ChangeUpdate update = newUpdate(c, changeOwner); + exception.expect(OrmException.class); + exception.expectMessage("Given ChangeUpdate is only allowed on initial commit"); + update.setRevertOf(newChange().getId().get()); + update.commit(); + } + private boolean testJson() { return noteUtil.getWriteJson(); } From 828f322d55f3babfc544fb99837ebaec42698560 Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Thu, 13 Jul 2017 14:18:38 +0200 Subject: [PATCH 2/3] Add revertOf to ChangeInfo and populate it when /revert is called Change-Id: I10c16c667e4bbb61d35a6c7b6be917fc595ab3ec --- Documentation/rest-api-changes.txt | 2 ++ .../google/gerrit/acceptance/api/change/ChangeIT.java | 1 + .../google/gerrit/extensions/common/ChangeInfo.java | 1 + .../google/gerrit/server/change/ChangeInserter.java | 10 ++++++++++ .../com/google/gerrit/server/change/ChangeJson.java | 1 + .../java/com/google/gerrit/server/change/Revert.java | 1 + .../server/notedb/rebuild/ChangeRebuilderImpl.java | 3 +++ 7 files changed, 19 insertions(+) diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 8b4e5293c0..b307efaaae 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -5729,6 +5729,8 @@ When present, change is marked as private. When present, change is marked as Work In Progress. |`has_review_started` |optional, not set if `false`| When present, change has been marked Ready at some point in time. +|`revert_of` |optional| +The numeric Change-Id of the change that this change reverts. |================================== [[change-input]] diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java index 4dc8e51975..6d0ba190a9 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java @@ -617,6 +617,7 @@ public class ChangeIT extends AbstractDaemonTest { assertThat(revertChange.messages).hasSize(1); assertThat(revertChange.messages.iterator().next().message).isEqualTo("Uploaded patch set 1."); + assertThat(revertChange.revertOf).isEqualTo(gApi.changes().id(r.getChangeId()).get()._number); } @Test diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java index f6d9f4cd21..a4f85e57ad 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java @@ -48,6 +48,7 @@ public class ChangeInfo { public Boolean isPrivate; public Boolean workInProgress; public Boolean hasReviewStarted; + public Integer revertOf; public int _number; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index bf417d0561..3e8a14693f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -129,6 +129,7 @@ public class ChangeInserter implements InsertChangeOp { private boolean fireRevisionCreated; private boolean sendMail; private boolean updateRef; + private Change.Id revertOf; // Fields set during the insertion process. private ReceiveCommand cmd; @@ -198,6 +199,7 @@ public class ChangeInserter implements InsertChangeOp { change.setPrivate(isPrivate); change.setWorkInProgress(workInProgress); change.setReviewStarted(!workInProgress); + change.setRevertOf(revertOf); return change; } @@ -319,6 +321,11 @@ public class ChangeInserter implements InsertChangeOp { return this; } + public ChangeInserter setRevertOf(Change.Id revertOf) { + this.revertOf = revertOf; + return this; + } + public void setPushCertificate(String cert) { pushCert = cert; } @@ -390,6 +397,9 @@ public class ChangeInserter implements InsertChangeOp { update.setPsDescription(patchSetDescription); update.setPrivate(isPrivate); update.setWorkInProgress(workInProgress); + if (revertOf != null) { + update.setRevertOf(revertOf.get()); + } boolean draft = status == Change.Status.DRAFT; List newGroups = groups; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java index 33a1565121..80bf1c3dd4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java @@ -542,6 +542,7 @@ public class ChangeJson { out.submitted = getSubmittedOn(cd); out.plugins = pluginDefinedAttributesFactory != null ? pluginDefinedAttributesFactory.create(cd) : null; + out.revertOf = cd.change().getRevertOf() != null ? cd.change().getRevertOf().get() : null; if (out.labels != null && has(DETAILED_LABELS)) { // If limited to specific patch sets but not the current patch set, don't diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java index af06054e03..53e09d4dfe 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java @@ -218,6 +218,7 @@ public class Revert extends RetryingRestModifyView ccs = new HashSet<>(reviewerSet.byState(ReviewerStateInternal.CC)); ccs.remove(user.getAccountId()); ins.setExtraCC(ccs); + ins.setRevertOf(changeIdToRevert); try (BatchUpdate bu = updateFactory.create(db.get(), project, user, now)) { bu.setRepository(git, revWalk, oi); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java index 9c7f3bdbca..7aff21370c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java @@ -618,6 +618,9 @@ public class ChangeRebuilderImpl extends ChangeRebuilder { update.setChangeId(change.getKey().get()); update.setBranch(change.getDest().get()); update.setSubject(change.getOriginalSubject()); + if (change.getRevertOf() != null) { + update.setRevertOf(change.getRevertOf().get()); + } } @Override From a54d25cccee92c5331d5dd7e77fe024a24e975f3 Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Thu, 13 Jul 2017 13:13:27 +0200 Subject: [PATCH 3/3] Add revertOf to ChangeIndex This change adds revertOf to the ChangeIndex and adds a predicate to query the new propery. Change-Id: I40684400ab8fa1d854dc70af5e76214222581d07 --- Documentation/user-search.txt | 5 ++ .../gerrit/client/SearchSuggestOracle.java | 2 + .../server/index/change/ChangeField.java | 5 ++ .../index/change/ChangeSchemaDefinitions.java | 3 + .../server/query/change/ChangeData.java | 1 + .../query/change/ChangeQueryBuilder.java | 9 +++ .../query/change/RevertOfPredicate.java | 37 +++++++++++ .../change/AbstractQueryChangesTest.java | 62 +++++++++++++++---- 8 files changed, 112 insertions(+), 12 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/query/change/RevertOfPredicate.java diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt index 74ae568c9c..bbca36483c 100644 --- a/Documentation/user-search.txt +++ b/Documentation/user-search.txt @@ -129,6 +129,11 @@ cc:'USER':: Changes that have the given user CC'ed on them. The special case of `cc:self` will find changes where the caller has been CC'ed. +[[revertof]] +revertof:'ID':: ++ +Changes that revert the change specified by the numeric 'ID'. + [[reviewerin]] reviewerin:'GROUP':: + diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java index 753d4217bd..a5fc4f3d17 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java @@ -154,6 +154,8 @@ public class SearchSuggestOracle extends HighlightSuggestOracle { suggestions.add("unresolved:"); + suggestions.add("revertof:"); + if (Gerrit.isNoteDbEnabled()) { suggestions.add("cc:"); suggestions.add("hashtag:"); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java index 7a8cc7253c..3fe2d13864 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java @@ -207,6 +207,11 @@ public class ChangeField { .stored() .buildRepeatable(cd -> getReviewerByEmailFieldValues(cd.pendingReviewersByEmail())); + /** References a change that this change reverts. */ + public static final FieldDef REVERT_OF = + integer(ChangeQueryBuilder.FIELD_REVERTOF) + .build(cd -> cd.change().getRevertOf() != null ? cd.change().getRevertOf().get() : null); + @VisibleForTesting static List getReviewerFieldValues(ReviewerSet reviewers) { List r = new ArrayList<>(reviewers.asTable().size() * 2); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java index bb0118ba5f..2f037798a4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java @@ -78,6 +78,7 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions { static final Schema V43 = schema(V42, ChangeField.EXACT_AUTHOR, ChangeField.EXACT_COMMITTER); + @Deprecated static final Schema V44 = schema( V43, @@ -85,6 +86,8 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions { ChangeField.PENDING_REVIEWER, ChangeField.PENDING_REVIEWER_BY_EMAIL); + static final Schema V45 = schema(V44, ChangeField.REVERT_OF); + public static final String NAME = "changes"; public static final ChangeSchemaDefinitions INSTANCE = new ChangeSchemaDefinitions(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java index 96f6b5d10e..9a939bbe4c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java @@ -365,6 +365,7 @@ public class ChangeData { private PersonIdent author; private PersonIdent committer; private Integer unresolvedCommentCount; + private Change.Id revertOf; private ImmutableList refStates; private ImmutableList refStatePatterns; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index 161233edb4..aecfc42edf 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -179,6 +179,7 @@ public class ChangeQueryBuilder extends QueryBuilder { public static final String FIELD_VISIBLETO = "visibleto"; public static final String FIELD_WATCHEDBY = "watchedby"; public static final String FIELD_WIP = "wip"; + public static final String FIELD_REVERTOF = "revertof"; public static final String ARG_ID_USER = "user"; public static final String ARG_ID_GROUP = "group"; @@ -1179,6 +1180,14 @@ public class ChangeQueryBuilder extends QueryBuilder { return new IsUnresolvedPredicate(value); } + @Operator + public Predicate revertof(String value) throws QueryParseException { + if (args.getSchema().hasField(ChangeField.REVERT_OF)) { + return new RevertOfPredicate(value); + } + throw new QueryParseException("'revertof' operator is not supported by change index version"); + } + @Override protected Predicate defaultField(String query) throws QueryParseException { if (query.startsWith("refs/")) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RevertOfPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RevertOfPredicate.java new file mode 100644 index 0000000000..7f4ade0a71 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RevertOfPredicate.java @@ -0,0 +1,37 @@ +// Copyright (C) 2017 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.query.change; + +import com.google.gerrit.server.index.change.ChangeField; +import com.google.gwtorm.server.OrmException; + +public class RevertOfPredicate extends ChangeIndexPredicate { + public RevertOfPredicate(String revertOf) { + super(ChangeField.REVERT_OF, revertOf); + } + + @Override + public boolean match(ChangeData cd) throws OrmException { + if (cd.change().getRevertOf() == null) { + return false; + } + return cd.change().getRevertOf().toString().equals(value); + } + + @Override + public int getCost() { + return 1; + } +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 95859415ee..4a066a64e0 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -31,6 +31,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; +import com.google.common.collect.Streams; import com.google.common.truth.ThrowableSubject; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.TimeUtil; @@ -49,6 +50,7 @@ import com.google.gerrit.extensions.api.projects.ConfigInput; import com.google.gerrit.extensions.client.InheritableBoolean; import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.ChangeInput; import com.google.gerrit.extensions.common.ChangeMessageInfo; import com.google.gerrit.extensions.common.CommentInfo; import com.google.gerrit.extensions.restapi.BadRequestException; @@ -2121,6 +2123,31 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery("us"); } + @Test + public void revertOf() throws Exception { + if (getSchemaVersion() < 45) { + assertMissingField(ChangeField.REVERT_OF); + assertFailingQuery( + "revertof:1", "'revertof' operator is not supported by change index version"); + return; + } + + TestRepository repo = createProject("repo"); + // Create two commits and revert second commit (initial commit can't be reverted) + Change initial = insert(repo, newChange(repo)); + gApi.changes().id(initial.getChangeId()).current().review(ReviewInput.approve()); + gApi.changes().id(initial.getChangeId()).current().submit(); + + ChangeInfo changeToRevert = + gApi.changes().create(new ChangeInput("repo", "master", "commit to revert")).get(); + gApi.changes().id(changeToRevert.id).current().review(ReviewInput.approve()); + gApi.changes().id(changeToRevert.id).current().submit(); + + ChangeInfo changeThatReverts = gApi.changes().id(changeToRevert.id).revert().get(); + assertQueryByIds( + "revertof:" + changeToRevert._number, new Change.Id(changeThatReverts._number)); + } + protected ChangeInserter newChange(TestRepository repo) throws Exception { return newChange(repo, null, null, null, null, false); } @@ -2255,36 +2282,47 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { return assertQuery(newQuery(query), changes); } + protected List assertQueryByIds(Object query, Change.Id... changes) throws Exception { + return assertQueryByIds(newQuery(query), changes); + } + protected List assertQuery(QueryRequest query, Change... changes) throws Exception { + return assertQueryByIds( + query, Arrays.stream(changes).map(Change::getId).toArray(Change.Id[]::new)); + } + + protected List assertQueryByIds(QueryRequest query, Change.Id... changes) + throws Exception { List result = query.get(); - Iterable ids = ids(result); + Iterable ids = ids(result); assertThat(ids) .named(format(query, ids, changes)) - .containsExactlyElementsIn(ids(changes)) + .containsExactlyElementsIn(Arrays.asList(changes)) .inOrder(); return result; } - private String format(QueryRequest query, Iterable actualIds, Change... expectedChanges) + private String format( + QueryRequest query, Iterable actualIds, Change.Id... expectedChanges) throws RestApiException { StringBuilder b = new StringBuilder(); b.append("query '").append(query.getQuery()).append("' with expected changes "); - b.append(format(Arrays.stream(expectedChanges).map(Change::getChangeId).iterator())); + b.append(format(Arrays.asList(expectedChanges))); b.append(" and result "); b.append(format(actualIds)); return b.toString(); } - private String format(Iterable changeIds) throws RestApiException { + private String format(Iterable changeIds) throws RestApiException { return format(changeIds.iterator()); } - private String format(Iterator changeIds) throws RestApiException { + private String format(Iterator changeIds) throws RestApiException { StringBuilder b = new StringBuilder(); b.append("["); while (changeIds.hasNext()) { - int id = changeIds.next(); - ChangeInfo c = gApi.changes().id(id).get(); + Change.Id id = changeIds.next(); + ChangeInfo c = gApi.changes().id(id.get()).get(); b.append("{") .append(id) .append(" (") @@ -2307,12 +2345,12 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { return b.toString(); } - protected static Iterable ids(Change... changes) { - return FluentIterable.from(Arrays.asList(changes)).transform(in -> in.getId().get()); + protected static Iterable ids(Change... changes) { + return Arrays.stream(changes).map(c -> c.getId()).collect(toList()); } - protected static Iterable ids(Iterable changes) { - return FluentIterable.from(changes).transform(in -> in._number); + protected static Iterable ids(Iterable changes) { + return Streams.stream(changes).map(c -> new Change.Id(c._number)).collect(toList()); } protected static long lastUpdatedMs(Change c) {