From 4a711eda84555d4fd0d178cdda34a78cbd09e24e Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Tue, 12 Nov 2019 13:53:29 -0800 Subject: [PATCH] Add 'CherryPickOf' field for a change After a change is created or updated using the 'cherry-pick' functionality, this field will contain the source change number and the patchset. Having this field helps us identify changes where actual dev time was spent on by filtering out propagated changes. This is especially useful for organizations wanting to generate cost metrics. Change-Id: I782a56aa52c52670ec74fabb713fe47ecba24de1 --- Documentation/intro-user.txt | 9 ++++ Documentation/user-search.txt | 10 ++++ java/com/google/gerrit/entities/Change.java | 12 +++++ java/com/google/gerrit/entities/PatchSet.java | 6 ++- .../converter/ChangeProtoConverter.java | 9 ++++ .../gerrit/extensions/common/ChangeInfo.java | 2 + .../gerrit/server/change/ActionJson.java | 2 + .../gerrit/server/change/ChangeInserter.java | 10 ++++ .../gerrit/server/change/ChangeJson.java | 6 +++ .../gerrit/server/change/SetCherryPickOp.java | 49 ++++++++++++++++++ .../gerrit/server/data/ChangeAttribute.java | 2 + .../gerrit/server/events/EventFactory.java | 4 ++ .../server/index/change/ChangeField.java | 18 +++++++ .../index/change/ChangeSchemaDefinitions.java | 9 ++++ .../gerrit/server/notedb/ChangeNoteUtil.java | 1 + .../server/notedb/ChangeNotesParser.java | 19 +++++++ .../server/notedb/ChangeNotesState.java | 15 ++++++ .../gerrit/server/notedb/ChangeUpdate.java | 13 ++++- .../server/query/change/ChangeData.java | 1 + .../query/change/ChangeQueryBuilder.java | 27 ++++++++++ .../change/CherryPickOfChangePredicate.java | 36 +++++++++++++ .../change/CherryPickOfPatchSetPredicate.java | 36 +++++++++++++ .../restapi/change/CherryPickChange.java | 25 ++++++++- .../gerrit/server/restapi/change/Module.java | 2 + .../acceptance/api/revision/RevisionIT.java | 51 ++++++++++++++++--- .../converter/ChangeProtoConverterTest.java | 1 + .../server/notedb/ChangeNotesStateTest.java | 1 + .../gr-change-metadata.html | 13 +++++ .../gr-change-metadata/gr-change-metadata.js | 11 ++++ .../gr-change-metadata_test.html | 16 ++++++ .../core/gr-search-bar/gr-search-bar.js | 1 + proto/cache.proto | 5 +- proto/entities.proto | 3 +- 33 files changed, 413 insertions(+), 12 deletions(-) create mode 100644 java/com/google/gerrit/server/change/SetCherryPickOp.java create mode 100644 java/com/google/gerrit/server/query/change/CherryPickOfChangePredicate.java create mode 100644 java/com/google/gerrit/server/query/change/CherryPickOfPatchSetPredicate.java diff --git a/Documentation/intro-user.txt b/Documentation/intro-user.txt index 6f55398040..9dd58b8818 100644 --- a/Documentation/intro-user.txt +++ b/Documentation/intro-user.txt @@ -453,6 +453,15 @@ in list of open changes anymore. Abandoned changes can be link:user-review-ui.html#restore[restored] if later they are needed again. +[[cherrypickof]] +== Cherry-Pick changes of a Change + +When a change is created/updated using the 'cherry-pick' functionalty, +the original change and patchset details are recorded in the Change's +cherrypick field. This field cannot be set or updated by the user in +any way. It is set automatically after the cherry-pick operation completes +successfully. + [[topics]] == Using Topics diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt index aa5edf0dbb..359c32a59d 100644 --- a/Documentation/user-search.txt +++ b/Documentation/user-search.txt @@ -229,6 +229,16 @@ hashtag:'HASHTAG':: Changes whose link:intro-user.html#hashtags[hashtag] matches 'HASHTAG'. The match is case-insensitive. +[[cherrypickof]] +cherrypickof:'CHANGE[,PATCHSET]':: ++ +Changes which were created using the 'cherry-pick' functionality and +whose source change number matches 'CHANGE' and source patchset number +matches 'PATCHSET'. Note that 'PATCHSET' is optional. For example, a +`cherrypickof:12345` matches all changes which were cherry-picked from +change 12345 and `cherrypickof:12345,2` matches all changes which were +cherry-picked from the 2nd patchset of change 12345. + [[ref]] ref:'REF':: + diff --git a/java/com/google/gerrit/entities/Change.java b/java/com/google/gerrit/entities/Change.java index 04e97dcf55..c768094151 100644 --- a/java/com/google/gerrit/entities/Change.java +++ b/java/com/google/gerrit/entities/Change.java @@ -531,6 +531,9 @@ public final class Change { /** References a change that this change reverts. */ @Nullable protected Id revertOf; + /** References the source change and patchset that this change was cherry-picked from. */ + @Nullable protected PatchSet.Id cherryPickOf; + protected Change() {} public Change( @@ -567,6 +570,7 @@ public final class Change { workInProgress = other.workInProgress; reviewStarted = other.reviewStarted; revertOf = other.revertOf; + cherryPickOf = other.cherryPickOf; } /** Legacy 32 bit integer identity for a change. */ @@ -760,6 +764,14 @@ public final class Change { return this.revertOf; } + public PatchSet.Id getCherryPickOf() { + return cherryPickOf; + } + + public void setCherryPickOf(@Nullable PatchSet.Id cherryPickOf) { + this.cherryPickOf = cherryPickOf; + } + @Override public String toString() { return new StringBuilder(getClass().getSimpleName()) diff --git a/java/com/google/gerrit/entities/PatchSet.java b/java/com/google/gerrit/entities/PatchSet.java index 8b93dbc92f..4a33bd79fe 100644 --- a/java/com/google/gerrit/entities/PatchSet.java +++ b/java/com/google/gerrit/entities/PatchSet.java @@ -124,13 +124,17 @@ public abstract class PatchSet { return id(); } + public String getCommaSeparatedChangeAndPatchSetId() { + return changeId().toString() + ',' + id(); + } + public String toRefName() { return changeId().refPrefixBuilder().append(id()).toString(); } @Override public final String toString() { - return changeId().toString() + ',' + id(); + return getCommaSeparatedChangeAndPatchSetId(); } } diff --git a/java/com/google/gerrit/entities/converter/ChangeProtoConverter.java b/java/com/google/gerrit/entities/converter/ChangeProtoConverter.java index 5b066ea87c..25e68f9813 100644 --- a/java/com/google/gerrit/entities/converter/ChangeProtoConverter.java +++ b/java/com/google/gerrit/entities/converter/ChangeProtoConverter.java @@ -29,6 +29,8 @@ public enum ChangeProtoConverter implements ProtoConverter changeIdConverter = ChangeIdProtoConverter.INSTANCE; + private final ProtoConverter patchSetIdConverter = + PatchSetIdProtoConverter.INSTANCE; private final ProtoConverter changeKeyConverter = ChangeKeyProtoConverter.INSTANCE; private final ProtoConverter accountIdConverter = @@ -78,6 +80,10 @@ public enum ChangeProtoConverter implements ProtoConverter newGroups = groups; if (newGroups.isEmpty()) { diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java index 21ee28a7d5..974cb47937 100644 --- a/java/com/google/gerrit/server/change/ChangeJson.java +++ b/java/com/google/gerrit/server/change/ChangeJson.java @@ -588,6 +588,12 @@ public class ChangeJson { } out.revertOf = cd.change().getRevertOf() != null ? cd.change().getRevertOf().get() : null; out.submissionId = cd.change().getSubmissionId(); + out.cherryPickOfChange = + cd.change().getCherryPickOf() != null + ? cd.change().getCherryPickOf().changeId().get() + : null; + out.cherryPickOfPatchSet = + cd.change().getCherryPickOf() != null ? cd.change().getCherryPickOf().get() : null; if (has(REVIEWER_UPDATES)) { out.reviewerUpdates = reviewerUpdates(cd); diff --git a/java/com/google/gerrit/server/change/SetCherryPickOp.java b/java/com/google/gerrit/server/change/SetCherryPickOp.java new file mode 100644 index 0000000000..d271923aaf --- /dev/null +++ b/java/com/google/gerrit/server/change/SetCherryPickOp.java @@ -0,0 +1,49 @@ +// Copyright (C) 2019 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.change; + +import com.google.gerrit.entities.Change; +import com.google.gerrit.entities.PatchSet; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.server.notedb.ChangeUpdate; +import com.google.gerrit.server.update.BatchUpdateOp; +import com.google.gerrit.server.update.ChangeContext; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; + +public class SetCherryPickOp implements BatchUpdateOp { + public interface Factory { + SetCherryPickOp create(PatchSet.Id cherryPickOf); + } + + private final PatchSet.Id newCherryPickOf; + + @Inject + SetCherryPickOp(@Assisted PatchSet.Id newCherryPickOf) { + this.newCherryPickOf = newCherryPickOf; + } + + @Override + public boolean updateChange(ChangeContext ctx) throws RestApiException { + Change change = ctx.getChange(); + if (newCherryPickOf.equals(change.getCherryPickOf())) { + return false; + } + + ChangeUpdate update = ctx.getUpdate(change.currentPatchSetId()); + update.setCherryPickOf(newCherryPickOf.getCommaSeparatedChangeAndPatchSetId()); + return true; + } +} diff --git a/java/com/google/gerrit/server/data/ChangeAttribute.java b/java/com/google/gerrit/server/data/ChangeAttribute.java index a6da2b98bc..bc51e2acc1 100644 --- a/java/com/google/gerrit/server/data/ChangeAttribute.java +++ b/java/com/google/gerrit/server/data/ChangeAttribute.java @@ -31,6 +31,8 @@ public class ChangeAttribute { public String url; public String commitMessage; public List hashtags; + public Integer cherryPickOfChange; + public Integer cherryPickOfPatchSet; public Long createdOn; public Long lastUpdated; diff --git a/java/com/google/gerrit/server/events/EventFactory.java b/java/com/google/gerrit/server/events/EventFactory.java index 3f22d7f8ab..9422c1874b 100644 --- a/java/com/google/gerrit/server/events/EventFactory.java +++ b/java/com/google/gerrit/server/events/EventFactory.java @@ -144,6 +144,10 @@ public class EventFactory { a.createdOn = change.getCreatedOn().getTime() / 1000L; a.wip = change.isWorkInProgress() ? true : null; a.isPrivate = change.isPrivate() ? true : null; + a.cherryPickOfChange = + change.getCherryPickOf() != null ? change.getCherryPickOf().changeId().get() : null; + a.cherryPickOfPatchSet = + change.getCherryPickOf() != null ? change.getCherryPickOf().get() : null; return a; } diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java index 07e9b9e4fc..d3a0065fce 100644 --- a/java/com/google/gerrit/server/index/change/ChangeField.java +++ b/java/com/google/gerrit/server/index/change/ChangeField.java @@ -269,6 +269,24 @@ public class ChangeField { public static final FieldDef OWNER = integer(ChangeQueryBuilder.FIELD_OWNER).build(changeGetter(c -> c.getOwner().get())); + /** References the source change number that this change was cherry-picked from. */ + public static final FieldDef CHERRY_PICK_OF_CHANGE = + integer(ChangeQueryBuilder.FIELD_CHERRY_PICK_OF_CHANGE) + .build( + cd -> + cd.change().getCherryPickOf() != null + ? cd.change().getCherryPickOf().changeId().get() + : null); + + /** References the source change patch-set that this change was cherry-picked from. */ + public static final FieldDef CHERRY_PICK_OF_PATCHSET = + integer(ChangeQueryBuilder.FIELD_CHERRY_PICK_OF_PATCHSET) + .build( + cd -> + cd.change().getCherryPickOf() != null + ? cd.change().getCherryPickOf().get() + : null); + /** The user assigned to the change. */ public static final FieldDef ASSIGNEE = integer(ChangeQueryBuilder.FIELD_ASSIGNEE) diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java index 55fe11d22e..6d6d4b8c37 100644 --- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java +++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java @@ -91,6 +91,7 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions { // New numeric types: use dimensional points using the k-d tree geo-spatial data structure // to offer fast single- and multi-dimensional numeric range. As the consequense, integer // document id type is replaced with string document id type. + @Deprecated static final Schema V57 = new Schema.Builder() .add(V56) @@ -99,6 +100,14 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions { .legacyNumericFields(false) .build(); + // Add new field CHERRY_PICK_OF + static final Schema V58 = + new Schema.Builder() + .add(V57) + .add(ChangeField.CHERRY_PICK_OF_CHANGE) + .add(ChangeField.CHERRY_PICK_OF_PATCHSET) + .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/ChangeNoteUtil.java b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java index b221ef51b3..cebb67d4c4 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java +++ b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java @@ -48,6 +48,7 @@ public class ChangeNoteUtil { 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"); + public static final FooterKey FOOTER_CHERRY_PICK_OF = new FooterKey("Cherry-pick-of"); static final String AUTHOR = "Author"; static final String BASE_PATCH_SET = "Base-for-patch-set"; diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java index 3322b683f8..369d1f26db 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java @@ -18,6 +18,7 @@ import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ASSIGNEE; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_BRANCH; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHANGE_ID; +import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHERRY_PICK_OF; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_COMMIT; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CURRENT; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_GROUPS; @@ -148,6 +149,7 @@ class ChangeNotesParser { private ReviewerByEmailSet pendingReviewersByEmail; private Change.Id revertOf; private int updateCount; + private PatchSet.Id cherryPickOf; ChangeNotesParser( Change.Id changeId, @@ -246,6 +248,7 @@ class ChangeNotesParser { firstNonNull(workInProgress, false), firstNonNull(hasReviewStarted, true), revertOf, + cherryPickOf, updateCount); } @@ -417,6 +420,10 @@ class ChangeNotesParser { revertOf = parseRevertOf(commit); } + if (cherryPickOf == null) { + cherryPickOf = parseCherryPickOf(commit); + } + previousWorkInProgressFooter = null; parseWorkInProgress(commit); } @@ -966,6 +973,18 @@ class ChangeNotesParser { return Change.id(revertOf); } + private PatchSet.Id parseCherryPickOf(ChangeNotesCommit commit) throws ConfigInvalidException { + String cherryPickOf = parseOneFooter(commit, FOOTER_CHERRY_PICK_OF); + if (cherryPickOf == null) { + return null; + } + try { + return PatchSet.Id.parse(cherryPickOf); + } catch (IllegalArgumentException e) { + throw new ConfigInvalidException("\"" + cherryPickOf + "\" is not a valid patchset", e); + } + } + private void pruneReviewers() { Iterator> rit = reviewers.cellSet().iterator(); diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java index 896cca3ba4..064e43b52b 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java @@ -123,6 +123,7 @@ public abstract class ChangeNotesState { boolean workInProgress, boolean reviewStarted, @Nullable Change.Id revertOf, + @Nullable PatchSet.Id cherryPickOf, int updateCount) { requireNonNull( metaId, @@ -152,6 +153,7 @@ public abstract class ChangeNotesState { .workInProgress(workInProgress) .reviewStarted(reviewStarted) .revertOf(revertOf) + .cherryPickOf(cherryPickOf) .build()) .hashtags(hashtags) .serverId(serverId) @@ -220,6 +222,9 @@ public abstract class ChangeNotesState { @Nullable abstract Change.Id revertOf(); + @Nullable + abstract PatchSet.Id cherryPickOf(); + abstract Builder toBuilder(); @AutoValue.Builder @@ -254,6 +259,8 @@ public abstract class ChangeNotesState { abstract Builder revertOf(@Nullable Change.Id revertOf); + abstract Builder cherryPickOf(@Nullable PatchSet.Id cherryPickOf); + abstract ChangeColumns build(); } } @@ -341,6 +348,7 @@ public abstract class ChangeNotesState { change.setWorkInProgress(c.workInProgress()); change.setReviewStarted(c.reviewStarted()); change.setRevertOf(c.revertOf()); + change.setCherryPickOf(c.cherryPickOf()); if (!patchSets().isEmpty()) { change.setCurrentPatchSet(c.currentPatchSetId(), c.subject(), c.originalSubject()); @@ -514,6 +522,10 @@ public abstract class ChangeNotesState { if (cols.revertOf() != null) { b.setRevertOf(cols.revertOf().get()).setHasRevertOf(true); } + if (cols.cherryPickOf() != null) { + b.setCherryPickOf(cols.cherryPickOf().getCommaSeparatedChangeAndPatchSetId()) + .setHasCherryPickOf(true); + } return b.build(); } @@ -637,6 +649,9 @@ public abstract class ChangeNotesState { if (proto.getHasRevertOf()) { b.revertOf(Change.id(proto.getRevertOf())); } + if (proto.getHasCherryPickOf()) { + b.cherryPickOf(PatchSet.Id.parse(proto.getCherryPickOf())); + } return b.build(); } diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java index 6a900c0c85..2822f61879 100644 --- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java +++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java @@ -21,6 +21,7 @@ import static com.google.gerrit.entities.RefNames.changeMetaRef; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ASSIGNEE; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_BRANCH; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHANGE_ID; +import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHERRY_PICK_OF; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_COMMIT; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CURRENT; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_GROUPS; @@ -137,6 +138,7 @@ public class ChangeUpdate extends AbstractChangeUpdate { private Boolean isPrivate; private Boolean workInProgress; private Integer revertOf; + private String cherryPickOf; private ChangeDraftUpdate draftUpdate; private RobotCommentUpdate robotCommentUpdate; @@ -417,6 +419,10 @@ public class ChangeUpdate extends AbstractChangeUpdate { rootOnly = true; } + public void setCherryPickOf(String cherryPickOf) { + this.cherryPickOf = cherryPickOf; + } + /** @return the tree id for the updated tree */ private ObjectId storeRevisionNotes(RevWalk rw, ObjectInserter inserter, ObjectId curr) throws ConfigInvalidException, IOException { @@ -665,6 +671,10 @@ public class ChangeUpdate extends AbstractChangeUpdate { addFooter(msg, FOOTER_REVERT_OF, revertOf); } + if (cherryPickOf != null) { + addFooter(msg, FOOTER_CHERRY_PICK_OF, cherryPickOf); + } + cb.setMessage(msg.toString()); try { ObjectId treeId = storeRevisionNotes(rw, ins, curr); @@ -714,7 +724,8 @@ public class ChangeUpdate extends AbstractChangeUpdate { && !currentPatchSet && isPrivate == null && workInProgress == null - && revertOf == null; + && revertOf == null + && cherryPickOf == null; } ChangeDraftUpdate getDraftUpdate() { diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java index 78ca0fcdfa..381dc6e9b6 100644 --- a/java/com/google/gerrit/server/query/change/ChangeData.java +++ b/java/com/google/gerrit/server/query/change/ChangeData.java @@ -301,6 +301,7 @@ public class ChangeData { private Integer unresolvedCommentCount; private Integer totalCommentCount; private LabelTypes labelTypes; + private Optional cherryPickOf; private ImmutableList refStates; private ImmutableList refStatePatterns; diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index 5f076b106c..7747998f1d 100644 --- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -34,6 +34,7 @@ import com.google.gerrit.entities.Account; import com.google.gerrit.entities.AccountGroup; import com.google.gerrit.entities.BranchNameKey; import com.google.gerrit.entities.Change; +import com.google.gerrit.entities.PatchSet; import com.google.gerrit.entities.RefNames; import com.google.gerrit.exceptions.NotSignedInException; import com.google.gerrit.exceptions.StorageException; @@ -189,6 +190,9 @@ public class ChangeQueryBuilder extends QueryBuilder cherryPickOf(String value) throws QueryParseException { + if (args.getSchema().hasField(ChangeField.CHERRY_PICK_OF_CHANGE) + && args.getSchema().hasField(ChangeField.CHERRY_PICK_OF_PATCHSET)) { + if (Ints.tryParse(value) != null) { + return new CherryPickOfChangePredicate(value); + } + try { + PatchSet.Id patchSetId = PatchSet.Id.parse(value); + return Predicate.and( + new CherryPickOfChangePredicate(patchSetId.changeId().toString()), + new CherryPickOfPatchSetPredicate(patchSetId.getId())); + } catch (IllegalArgumentException e) { + throw new QueryParseException( + "'" + + value + + "' is not a valid input. It must be in the 'ChangeNumber[,PatchsetNumber]' format."); + } + } + throw new QueryParseException( + "'cherrypickof' operator is not supported by change index version"); + } + @Override protected Predicate defaultField(String query) throws QueryParseException { if (query.startsWith("refs/")) { diff --git a/java/com/google/gerrit/server/query/change/CherryPickOfChangePredicate.java b/java/com/google/gerrit/server/query/change/CherryPickOfChangePredicate.java new file mode 100644 index 0000000000..d4520171d7 --- /dev/null +++ b/java/com/google/gerrit/server/query/change/CherryPickOfChangePredicate.java @@ -0,0 +1,36 @@ +// Copyright (C) 2019 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; + +public class CherryPickOfChangePredicate extends ChangeIndexPredicate { + public CherryPickOfChangePredicate(String cherryPickOfChange) { + super(ChangeField.CHERRY_PICK_OF_CHANGE, cherryPickOfChange); + } + + @Override + public boolean match(ChangeData cd) { + if (cd.change().getCherryPickOf() == null) { + return false; + } + return Integer.toString(cd.change().getCherryPickOf().changeId().get()).equals(value); + } + + @Override + public int getCost() { + return 1; + } +} diff --git a/java/com/google/gerrit/server/query/change/CherryPickOfPatchSetPredicate.java b/java/com/google/gerrit/server/query/change/CherryPickOfPatchSetPredicate.java new file mode 100644 index 0000000000..888f45d00c --- /dev/null +++ b/java/com/google/gerrit/server/query/change/CherryPickOfPatchSetPredicate.java @@ -0,0 +1,36 @@ +// Copyright (C) 2019 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; + +public class CherryPickOfPatchSetPredicate extends ChangeIndexPredicate { + public CherryPickOfPatchSetPredicate(String cherryPickOfPatchSet) { + super(ChangeField.CHERRY_PICK_OF_PATCHSET, cherryPickOfPatchSet); + } + + @Override + public boolean match(ChangeData cd) { + if (cd.change().getCherryPickOf() == null) { + return false; + } + return cd.change().getCherryPickOf().getId().equals(value); + } + + @Override + public int getCost() { + return 1; + } +} diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java index a7a7998056..255c15c8e9 100644 --- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java +++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java @@ -42,6 +42,7 @@ import com.google.gerrit.server.ReviewerSet; import com.google.gerrit.server.change.ChangeInserter; import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.change.PatchSetInserter; +import com.google.gerrit.server.change.SetCherryPickOp; import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk; import com.google.gerrit.server.git.GitRepositoryManager; @@ -102,6 +103,7 @@ public class CherryPickChange { private final Provider user; private final ChangeInserter.Factory changeInserterFactory; private final PatchSetInserter.Factory patchSetInserterFactory; + private final SetCherryPickOp.Factory setCherryPickOfFactory; private final MergeUtil.Factory mergeUtilFactory; private final ChangeNotes.Factory changeNotesFactory; private final ProjectCache projectCache; @@ -117,6 +119,7 @@ public class CherryPickChange { Provider user, ChangeInserter.Factory changeInserterFactory, PatchSetInserter.Factory patchSetInserterFactory, + SetCherryPickOp.Factory setCherryPickOfFactory, MergeUtil.Factory mergeUtilFactory, ChangeNotes.Factory changeNotesFactory, ProjectCache projectCache, @@ -129,6 +132,7 @@ public class CherryPickChange { this.user = user; this.changeInserterFactory = changeInserterFactory; this.patchSetInserterFactory = patchSetInserterFactory; + this.setCherryPickOfFactory = setCherryPickOfFactory; this.mergeUtilFactory = mergeUtilFactory; this.changeNotesFactory = changeNotesFactory; this.projectCache = projectCache; @@ -374,7 +378,13 @@ public class CherryPickChange { dest.project(), destChanges.get(0).getId().get())); } - changeId = insertPatchSet(bu, git, destChanges.get(0).notes(), cherryPickCommit); + changeId = + insertPatchSet( + bu, + git, + destChanges.get(0).notes(), + cherryPickCommit, + sourceChange.currentPatchSetId()); } else { // Change key not found on destination branch. We can create a new // change. @@ -457,13 +467,22 @@ public class CherryPickChange { } private Change.Id insertPatchSet( - BatchUpdate bu, Repository git, ChangeNotes destNotes, CodeReviewCommit cherryPickCommit) + BatchUpdate bu, + Repository git, + ChangeNotes destNotes, + CodeReviewCommit cherryPickCommit, + PatchSet.Id sourcePatchSetId) throws IOException { Change destChange = destNotes.getChange(); PatchSet.Id psId = ChangeUtil.nextPatchSetId(git, destChange.currentPatchSetId()); PatchSetInserter inserter = patchSetInserterFactory.create(destNotes, psId, cherryPickCommit); inserter.setMessage("Uploaded patch set " + inserter.getPatchSetId().get() + "."); bu.addOp(destChange.getId(), inserter); + if (destChange.getCherryPickOf() == null + || !destChange.getCherryPickOf().equals(sourcePatchSetId)) { + SetCherryPickOp cherryPickOfUpdater = setCherryPickOfFactory.create(sourcePatchSetId); + bu.addOp(destChange.getId(), cherryPickOfUpdater); + } return destChange.getId(); } @@ -483,6 +502,7 @@ public class CherryPickChange { ChangeInserter ins = changeInserterFactory.create(changeId, cherryPickCommit, refName); ins.setRevertOf(revertOf); BranchNameKey sourceBranch = sourceChange == null ? null : sourceChange.getDest(); + PatchSet.Id sourcePatchSetId = sourceChange == null ? null : sourceChange.currentPatchSetId(); ins.setMessage( revertOf == null ? messageForDestinationChange( @@ -490,6 +510,7 @@ public class CherryPickChange { : "Uploaded patch set 1.") // For revert commits, the message should not include // cherry-pick information. .setTopic(topic) + .setCherryPickOf(sourcePatchSetId) .setWorkInProgress( (sourceChange != null && sourceChange.isWorkInProgress()) || !cherryPickCommit.getFilesWithGitConflicts().isEmpty()); diff --git a/java/com/google/gerrit/server/restapi/change/Module.java b/java/com/google/gerrit/server/restapi/change/Module.java index 4409c6a007..e339c67592 100644 --- a/java/com/google/gerrit/server/restapi/change/Module.java +++ b/java/com/google/gerrit/server/restapi/change/Module.java @@ -40,6 +40,7 @@ import com.google.gerrit.server.change.PatchSetInserter; import com.google.gerrit.server.change.RebaseChangeOp; import com.google.gerrit.server.change.ReviewerResource; import com.google.gerrit.server.change.SetAssigneeOp; +import com.google.gerrit.server.change.SetCherryPickOp; import com.google.gerrit.server.change.SetHashtagsOp; import com.google.gerrit.server.change.SetPrivateOp; import com.google.gerrit.server.change.WorkInProgressOp; @@ -201,6 +202,7 @@ public class Module extends RestApiModule { factory(RebaseChangeOp.Factory.class); factory(ReviewerResource.Factory.class); factory(SetAssigneeOp.Factory.class); + factory(SetCherryPickOp.Factory.class); factory(SetHashtagsOp.Factory.class); factory(SetPrivateOp.Factory.class); factory(WorkInProgressOp.Factory.class); diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java index 61d0fd57cd..c4dec3190f 100644 --- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java +++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java @@ -337,6 +337,9 @@ public class RevisionIT extends AbstractDaemonTest { assertThat(cherry.get().subject).contains(in.message); assertThat(cherry.get().topic).isEqualTo("someTopic-foo"); + assertThat(cherry.get().cherryPickOfChange).isEqualTo(orig.get()._number); + assertThat(cherry.get().cherryPickOfPatchSet).isEqualTo(1); + cherry.current().review(ReviewInput.approve()); cherry.current().submit(); } @@ -416,23 +419,23 @@ public class RevisionIT extends AbstractDaemonTest { ChangeApi orig = gApi.changes().id(project.get() + "~master~" + r.getChangeId()); ChangeApi cherry = orig.revision(r.getCommit().name()).cherryPick(in); + assertThat(cherry.get().cherryPickOfChange).isEqualTo(orig.get()._number); + assertThat(cherry.get().cherryPickOfPatchSet).isEqualTo(1); assertThat(cherry.get().workInProgress).isTrue(); } @Test public void cherryPickToSameBranch() throws Exception { PushOneCommit.Result r = createChange(); + ChangeApi change = gApi.changes().id(project.get() + "~master~" + r.getChangeId()); CherryPickInput in = new CherryPickInput(); in.destination = "master"; in.message = "it generates a new patch set\n\nChange-Id: " + r.getChangeId(); - ChangeInfo cherryInfo = - gApi.changes() - .id(project.get() + "~master~" + r.getChangeId()) - .revision(r.getCommit().name()) - .cherryPick(in) - .get(); + ChangeInfo cherryInfo = change.revision(r.getCommit().name()).cherryPick(in).get(); assertThat(cherryInfo.messages).hasSize(2); Iterator cherryIt = cherryInfo.messages.iterator(); + assertThat(cherryInfo.cherryPickOfChange).isEqualTo(change.get()._number); + assertThat(cherryInfo.cherryPickOfPatchSet).isEqualTo(1); assertThat(cherryIt.next().message).isEqualTo("Uploaded patch set 1."); assertThat(cherryIt.next().message).isEqualTo("Uploaded patch set 2."); } @@ -624,6 +627,42 @@ public class RevisionIT extends AbstractDaemonTest { + PushOneCommit.FILE_NAME); } + @Test + public void cherryPickToExistingChangeUpdatesCherryPickOf() throws Exception { + PushOneCommit.Result r1 = + pushFactory + .create(admin.newIdent(), testRepo, SUBJECT, FILE_NAME, "a") + .to("refs/for/master"); + String t1 = project.get() + "~master~" + r1.getChangeId(); + ChangeApi orig = gApi.changes().id(project.get() + "~master~" + r1.getChangeId()); + + BranchInput bin = new BranchInput(); + bin.revision = r1.getCommit().getParent(0).name(); + gApi.projects().name(project.get()).branch("foo").create(bin); + + PushOneCommit.Result r2 = + pushFactory + .create(admin.newIdent(), testRepo, SUBJECT, FILE_NAME, "b", r1.getChangeId()) + .to("refs/for/foo"); + String t2 = project.get() + "~foo~" + r2.getChangeId(); + + CherryPickInput in = new CherryPickInput(); + in.destination = "foo"; + in.message = r1.getCommit().getFullMessage(); + ChangeApi cherry = gApi.changes().id(t1).current().cherryPick(in); + assertThat(get(t2, ALL_REVISIONS).revisions).hasSize(2); + assertThat(cherry.get().cherryPickOfChange).isEqualTo(orig.get()._number); + assertThat(cherry.get().cherryPickOfPatchSet).isEqualTo(1); + + PushOneCommit.Result r3 = amendChange(r1.getChangeId(), SUBJECT, "b.txt", "b"); + in = new CherryPickInput(); + in.destination = "foo"; + in.message = r3.getCommit().getFullMessage(); + cherry = gApi.changes().id(t1).current().cherryPick(in); + assertThat(cherry.get().cherryPickOfChange).isEqualTo(orig.get()._number); + assertThat(cherry.get().cherryPickOfPatchSet).isEqualTo(2); + } + @Test public void cherryPickToExistingChange() throws Exception { PushOneCommit.Result r1 = diff --git a/javatests/com/google/gerrit/entities/converter/ChangeProtoConverterTest.java b/javatests/com/google/gerrit/entities/converter/ChangeProtoConverterTest.java index 72e4a7a8ed..bc669cc393 100644 --- a/javatests/com/google/gerrit/entities/converter/ChangeProtoConverterTest.java +++ b/javatests/com/google/gerrit/entities/converter/ChangeProtoConverterTest.java @@ -335,6 +335,7 @@ public class ChangeProtoConverterTest { .put("workInProgress", boolean.class) .put("reviewStarted", boolean.class) .put("revertOf", Change.Id.class) + .put("cherryPickOf", PatchSet.Id.class) .build()); } diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java index 6ece894645..16981dc947 100644 --- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java +++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java @@ -776,6 +776,7 @@ public class ChangeNotesStateTest { .put("workInProgress", boolean.class) .put("reviewStarted", boolean.class) .put("revertOf", Change.Id.class) + .put("cherryPickOf", PatchSet.Id.class) .put("toBuilder", ChangeNotesState.ChangeColumns.Builder.class) .build()); } diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html index 539b1b66dd..9073342cd2 100644 --- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html +++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html @@ -270,6 +270,19 @@ limitations under the License. +