opendev/gerrit
Code Issues Proposed changes

InlineEdit: Add "has:edit" search operator

Browse Source 
Index a field "editby" with a value for each account ID that has an
edit on this change. Intentionally don't expose editby:'USER' because
only change edit owners can see them.

Given that "has:edit" predicate outcome depends on the existence of
the change edit ref and not on its content, only reindex the changes
on change edit ref creation and deletion, but not on its mutation.

Bug: Issue 3291
Change-Id: I2d541c781d057cdf9dcacfc02f81591ce6872595
This commit is contained in:
David Ostrovsky 2015-04-30 23:48:35 +02:00
parent 7dc16f2154
commit 9952870b5e
13 changed files with 197 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
Documentation/user-inline-edit.txt
View File

@ -144,6 +144,11 @@ command to your terminal. Note: only change edit owners and users that were
granted the link:access-control.html#capability_accessDatabase[accessDatabase]
global capability are able to access change edit refs.
[[search-for-change-edits]]
To search change edits from the UI the link:user-search.html#has[has:edit]
predicate can be used.
[[not-implemented-features]]
== Not Implemented Features

4
Documentation/user-search.txt
View File

@ -246,6 +246,10 @@ has:star::
Same as 'is:starred', true if the change has been starred by the
current user.
has:edit::
+
True if the change has inline edit created by the current user.
[[is]]
is:starred::
+

33
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
View File

@ -680,6 +680,39 @@ public class ChangeEditIT extends AbstractDaemonTest {
assertThat(approvals.get(0).value).isEqualTo(1);
}
@Test
public void testHasEditPredicate() throws Exception {
assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);
assertThat(queryEdits()).hasSize(1);
PatchSet current = getCurrentPatchSet(changeId2);
assertThat(modifier.createEdit(change2, current)).isEqualTo(RefUpdate.Result.NEW);
assertThat(
modifier.modifyFile(editUtil.byChange(change2).get(), FILE_NAME,
RestSession.newRawInput(CONTENT_NEW))).isEqualTo(RefUpdate.Result.FORCED);
assertThat(queryEdits()).hasSize(2);
assertThat(
modifier.modifyFile(editUtil.byChange(change).get(), FILE_NAME,
RestSession.newRawInput(CONTENT_NEW))).isEqualTo(RefUpdate.Result.FORCED);
editUtil.delete(editUtil.byChange(change).get());
assertThat(queryEdits()).hasSize(1);
editUtil.publish(editUtil.byChange(change2).get());
assertThat(queryEdits()).hasSize(0);
setApiUser(user);
assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);
assertThat(queryEdits()).hasSize(1);
setApiUser(admin);
assertThat(queryEdits()).hasSize(0);
}
private List<ChangeInfo> queryEdits() throws Exception {
return query("project:{" + project.get() + "} has:edit");
}
private String newChange(PersonIdent ident) throws Exception {
PushOneCommit push =
pushFactory.create(db, ident, testRepo, PushOneCommit.SUBJECT, FILE_NAME,

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java
View File

@ -93,6 +93,7 @@ public class SearchSuggestOracle extends HighlightSuggestOracle {
suggestions.add("file:");
suggestions.add("has:");
suggestions.add("has:draft");
suggestions.add("has:edit");
suggestions.add("has:star");
suggestions.add("is:");

8
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
View File

@ -145,6 +145,14 @@ public final class Change {
return null;
}
public static Id fromEditRefPart(String ref) {
int startChangeId = ref.indexOf(RefNames.EDIT_PREFIX) +
RefNames.EDIT_PREFIX.length();
int endChangeId = nextNonDigit(ref, startChangeId);
String id = ref.substring(startChangeId, endChangeId);
return new Change.Id(Integer.parseInt(id));
}
static int startIndex(String ref) {
if (ref == null || !ref.startsWith(REFS_CHANGES)) {
return -1;

7
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java
View File

@ -55,6 +55,8 @@ public class RefNames {
/** Suffix of a meta ref in the notedb. */
public static final String META_SUFFIX = "/meta";
public static final String EDIT_PREFIX = "edit-";
public static String fullName(String ref) {
return (ref.startsWith(REFS) ? "" : REFS_HEADS) + ref;
}
@ -119,9 +121,10 @@ public class RefNames {
*/
public static String refsEditPrefix(Account.Id accountId, Change.Id changeId) {
return new StringBuilder(refsUsers(accountId))
.append("/edit-")
.append('/')
.append(EDIT_PREFIX)
.append(changeId.get())
.append("/")
.append('/')
.toString();
}

14
gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java
View File

@ -29,10 +29,12 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@ -54,6 +56,7 @@ import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.RefUpdate.Result;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.merge.MergeStrategy;
import org.eclipse.jgit.merge.ThreeWayMerger;
@ -86,13 +89,19 @@ public class ChangeEditModifier {
}
private final TimeZone tz;
private final GitRepositoryManager gitManager;
private final ChangeIndexer indexer;
private final Provider<ReviewDb> reviewDb;
private final Provider<CurrentUser> currentUser;
@Inject
ChangeEditModifier(@GerritPersonIdent PersonIdent gerritIdent,
GitRepositoryManager gitManager,
ChangeIndexer indexer,
Provider<ReviewDb> reviewDb,
Provider<CurrentUser> currentUser) {
this.gitManager = gitManager;
this.indexer = indexer;
this.reviewDb = reviewDb;
this.currentUser = currentUser;
this.tz = gerritIdent.getTimeZone();
}
@ -127,7 +136,10 @@ public class ChangeEditModifier {
ObjectId revision = ObjectId.fromString(ps.getRevision().get());
String editRefName = RefNames.refsEdit(me.getAccountId(), change.getId(),
ps.getId());
return update(repo, me, editRefName, rw, ObjectId.zeroId(), revision);
Result res =
update(repo, me, editRefName, rw, ObjectId.zeroId(), revision);
indexer.index(reviewDb.get(), change);
return res;
}
}
}

15
gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java
View File

@ -32,6 +32,7 @@ import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.PatchSetInserter;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
@ -63,6 +64,7 @@ public class ChangeEditUtil {
private final GitRepositoryManager gitManager;
private final PatchSetInserter.Factory patchSetInserterFactory;
private final ChangeControl.GenericFactory changeControlFactory;
private final ChangeIndexer indexer;
private final Provider<ReviewDb> db;
private final Provider<CurrentUser> user;
@ -70,11 +72,13 @@ public class ChangeEditUtil {
ChangeEditUtil(GitRepositoryManager gitManager,
PatchSetInserter.Factory patchSetInserterFactory,
ChangeControl.GenericFactory changeControlFactory,
ChangeIndexer indexer,
Provider<ReviewDb> db,
Provider<CurrentUser> user) {
this.gitManager = gitManager;
this.patchSetInserterFactory = patchSetInserterFactory;
this.changeControlFactory = changeControlFactory;
this.indexer = indexer;
this.db = db;
this.user = user;
}
@ -152,10 +156,12 @@ public class ChangeEditUtil {
"only edit for current patch set can be published");
}
insertPatchSet(edit, change, repo, rw, basePatchSet,
squashEdit(rw, inserter, edit.getEditCommit(), basePatchSet));
Change updatedChange =
insertPatchSet(edit, change, repo, rw, basePatchSet,
squashEdit(rw, inserter, edit.getEditCommit(), basePatchSet));
// TODO(davido): This should happen in the same BatchRefUpdate.
deleteRef(repo, edit);
indexer.index(db.get(), updatedChange);
}
}
@ -174,6 +180,7 @@ public class ChangeEditUtil {
} finally {
repo.close();
}
indexer.index(db.get(), change);
}
private PatchSet getBasePatchSet(Change change, Ref ref)
@ -201,7 +208,7 @@ public class ChangeEditUtil {
return writeSquashedCommit(rw, inserter, parent, edit);
}
private void insertPatchSet(ChangeEdit edit, Change change,
private Change insertPatchSet(ChangeEdit edit, Change change,
Repository repo, RevWalk rw, PatchSet basePatchSet, RevCommit squashed)
throws NoSuchChangeException, InvalidChangeOperationException,
OrmException, IOException {
@ -215,7 +222,7 @@ public class ChangeEditUtil {
patchSetInserterFactory.create(repo, rw,
changeControlFactory.controlFor(change, edit.getUser()),
squashed);
insr.setPatchSet(ps)
return insr.setPatchSet(ps)
.setDraft(change.getStatus() == Status.DRAFT ||
basePatchSet.isDraft())
.setMessage(

17
gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java
View File

@ -568,6 +568,23 @@ public class ChangeField {
/** Serialized patch set object, used for pre-populating results. */
public static final PatchSetProtoField PATCH_SET = new PatchSetProtoField();
/** Users who have edits on this change. */
public static final FieldDef<ChangeData, Iterable<Integer>> EDITBY =
new FieldDef.Repeatable<ChangeData, Integer>(
ChangeQueryBuilder.FIELD_EDITBY, FieldType.INTEGER, false) {
@Override
public Iterable<Integer> get(ChangeData input, FillArgs args)
throws OrmException {
return ImmutableSet.copyOf(Iterables.transform(input.editsByUser(),
new Function<Account.Id, Integer>() {
@Override
public Integer apply(Account.Id account) {
return account.get();
}
}));
}
};
private static String getTopic(ChangeData input) throws OrmException {
Change c = input.change();
if (c == null) {

31
gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java
View File

@ -232,6 +232,37 @@ public class ChangeSchemas {
ChangeField.PATCH_SET,
ChangeField.GROUP);
static final Schema<ChangeData> V19 = schema(
ChangeField.LEGACY_ID,
ChangeField.ID,
ChangeField.STATUS,
ChangeField.PROJECT,
ChangeField.PROJECTS,
ChangeField.REF,
ChangeField.TOPIC,
ChangeField.UPDATED,
ChangeField.FILE_PART,
ChangeField.PATH,
ChangeField.OWNER,
ChangeField.REVIEWER,
ChangeField.COMMIT,
ChangeField.TR,
ChangeField.LABEL,
ChangeField.REVIEWED,
ChangeField.COMMIT_MESSAGE,
ChangeField.COMMENT,
ChangeField.CHANGE,
ChangeField.APPROVAL,
ChangeField.MERGEABLE,
ChangeField.ADDED,
ChangeField.DELETED,
ChangeField.DELTA,
ChangeField.HASHTAG,
ChangeField.COMMENTBY,
ChangeField.PATCH_SET,
ChangeField.GROUP,
ChangeField.EDITBY);
private static Schema<ChangeData> schema(Collection<FieldDef<ChangeData, ?>> fields) {
return new Schema<>(ImmutableList.copyOf(fields));
}

26
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
View File

@ -31,6 +31,7 @@ import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
@ -71,8 +72,10 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
public class ChangeData {
public static List<Change> asChanges(List<ChangeData> changeDatas)
@ -216,6 +219,7 @@ public class ChangeData {
private List<SubmitRecord> submitRecords;
private ChangedLines changedLines;
private Boolean mergeable;
private Set<Account.Id> editsByUser;
@AssistedInject
private ChangeData(
@ -651,6 +655,28 @@ public class ChangeData {
return mergeable;
}
public Set<Account.Id> editsByUser() throws OrmException {
if (editsByUser == null) {
Change c = change();
if (c == null) {
return Collections.emptySet();
}
editsByUser = new HashSet<>();
Change.Id id = change.getId();
try (Repository repo = repoManager.openRepository(change.getProject())) {
for (String ref
: repo.getRefDatabase().getRefs(RefNames.REFS_USERS).keySet()) {
if (Change.Id.fromEditRefPart(ref).equals(id)) {
editsByUser.add(Account.Id.fromRefPart(ref));
}
}
} catch (IOException e) {
throw new OrmException(e);
}
}
return editsByUser;
}
@Override
public String toString() {
MoreObjects.ToStringHelper h = MoreObjects.toStringHelper(this);

4
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
View File

@ -100,6 +100,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
public static final String FIELD_DELETED = "deleted";
public static final String FIELD_DELTA = "delta";
public static final String FIELD_DRAFTBY = "draftby";
public static final String FIELD_EDITBY = "editby";
public static final String FIELD_FILE = "file";
public static final String FIELD_IS = "is";
public static final String FIELD_HAS = "has";
@ -378,6 +379,9 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
return new HasDraftByPredicate(args, self());
}
if ("edit".equalsIgnoreCase(value)) {
return new EditByPredicate(self());
}
throw new IllegalArgumentException();
}

39
gerrit-server/src/main/java/com/google/gerrit/server/query/change/EditByPredicate.java Normal file
View File

@ -0,0 +1,39 @@
// Copyright (C) 2015 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.reviewdb.client.Account;
import com.google.gerrit.server.index.ChangeField;
import com.google.gerrit.server.index.IndexPredicate;
import com.google.gwtorm.server.OrmException;
class EditByPredicate extends IndexPredicate<ChangeData> {
private final Account.Id id;
EditByPredicate(Account.Id id) {
super(ChangeField.EDITBY, id.toString());
this.id = id;
}
@Override
public boolean match(ChangeData cd) throws OrmException {
return cd.editsByUser().contains(id);
}
@Override
public int getCost() {
return 1;
}
}