From a54d25cccee92c5331d5dd7e77fe024a24e975f3 Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Thu, 13 Jul 2017 13:13:27 +0200 Subject: [PATCH] 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) {