opendev/gerrit
Code Issues Proposed changes

Support to query for user specific named destinations

Browse Source 
Before this change, named destination searches for changes based on the
destinations defined by the current user. This change makes it possible
to search for changes based on destinations defined by other users.
This feature can be used to allow users to define personal sets of
destinations that interest them, perhaps destinations that they would
like to watch, or review, and it allows them to share those sets with
other users.

Feature: Issue 13336
Change-Id: I605df94d7d213ec6d733e6616c610774d941c8ea
This commit is contained in:
Prudhvi Akhil Alahari
2020-10-15 23:25:07 +05:30
parent 7e32debdc6
commit 02933c84ed
4 changed files with 82 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
Documentation/user-named-destinations.txt
View File

@@ -13,6 +13,7 @@ user's destination files are a 2 column tab delimited file. Each
row in a destination file represents a single destination in the
named set. The left column represents the ref of the destination,
and the right column represents the project of the destination.
The named destinations can be publicly accessible by other users.
Example destination file named `destinations/myreviews`:

6
Documentation/user-search.txt
View File

@@ -106,9 +106,11 @@ as a legacy numerical 'ID' such as 15183, or a newer style Change-Id
that was scraped out of the commit message.
[[destination]]
destination:'NAME'::
destination:'[name=]NAME[,user=USER]'::
+
Changes which match the current user's destination named 'NAME'.
Changes which match the specified USER's destination named 'NAME'. If 'USER'
is unspecified, the current user is used. The named destinations can be
publicly accessible by other users.
(see link:user-named-destinations.html[Named Destinations]).
[[owner]]

36
java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
View File

@@ -199,6 +199,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
public static final String FIELD_CHERRY_PICK_OF_CHANGE = "cherrypickofchange";
public static final String FIELD_CHERRY_PICK_OF_PATCHSET = "cherrypickofpatchset";
public static final String ARG_ID_NAME = "name";
public static final String ARG_ID_USER = "user";
public static final String ARG_ID_GROUP = "group";
public static final String ARG_ID_OWNER = "owner";
@@ -1248,19 +1249,46 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
}
@Operator
public Predicate<ChangeData> destination(String name) throws QueryParseException {
public Predicate<ChangeData> destination(String value) throws QueryParseException {
// [name=]<name>[,user=<user>] || [user=<user>,][name=]<name>
PredicateArgs inputArgs = new PredicateArgs(value);
String name = null;
Account.Id account = null;
try (Repository git = args.repoManager.openRepository(args.allUsersName)) {
VersionedAccountDestinations d = VersionedAccountDestinations.forUser(self());
// [name=]<name>
if (inputArgs.keyValue.containsKey(ARG_ID_NAME)) {
name = inputArgs.keyValue.get(ARG_ID_NAME);
} else if (inputArgs.positional.size() == 1) {
name = Iterables.getOnlyElement(inputArgs.positional);
} else if (inputArgs.positional.size() > 1) {
throw new QueryParseException("Error parsing named destination: " + value);
}
// [,user=<user>]
if (inputArgs.keyValue.containsKey(ARG_ID_USER)) {
Set<Account.Id> accounts = parseAccount(inputArgs.keyValue.get(ARG_ID_USER));
if (accounts != null && accounts.size() > 1) {
throw error(
String.format(
"\"%s\" resolves to multiple accounts", inputArgs.keyValue.get(ARG_ID_USER)));
}
account = (accounts == null ? self() : Iterables.getOnlyElement(accounts));
} else {
account = self();
}
VersionedAccountDestinations d = VersionedAccountDestinations.forUser(account);
d.load(args.allUsersName, git);
Set<BranchNameKey> destinations = d.getDestinationList().getDestinations(name);
if (destinations != null && !destinations.isEmpty()) {
return new DestinationPredicate(destinations, name);
return new DestinationPredicate(destinations, value);
}
} catch (RepositoryNotFoundException e) {
throw new QueryParseException(
"Unknown named destination (no " + args.allUsersName + " repo): " + name, e);
} catch (IOException | ConfigInvalidException e) {
throw new QueryParseException("Error parsing named destination: " + name, e);
throw new QueryParseException("Error parsing named destination: " + value, e);
}
throw new QueryParseException("Unknown named destination: " + name);
}

45
javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -3085,6 +3085,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
assertQuery("-assignee:" + user.getUserName().get(), change2);
}
@GerritConfig(name = "accounts.visibility", value = "NONE")
@Test
public void userDestination() throws Exception {
TestRepository<Repo> repo1 = createProject("repo1");
@@ -3096,6 +3097,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
.hasMessageThat()
.isEqualTo("Unknown named destination: foo");
Account.Id anotherUserId =
accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId();
String destination1 = "refs/heads/master\trepo1";
String destination2 = "refs/heads/master\trepo2";
String destination3 = "refs/heads/master\trepo1\nrefs/heads/master\trepo2";
@@ -3111,8 +3114,32 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
allUsers.branch(refsUsers).commit().add("destinations/destination4", destination4).create();
allUsers.branch(refsUsers).commit().add("destinations/destination5", destination5).create();
String anotherRefsUsers = RefNames.refsUsers(anotherUserId);
allUsers
.branch(anotherRefsUsers)
.commit()
.add("destinations/destination6", destination1)
.create();
allUsers
.branch(anotherRefsUsers)
.commit()
.add("destinations/destination7", destination2)
.create();
allUsers
.branch(anotherRefsUsers)
.commit()
.add("destinations/destination8", destination3)
.create();
allUsers
.branch(anotherRefsUsers)
.commit()
.add("destinations/destination9", destination4)
.create();
Ref userRef = allUsers.getRepository().exactRef(refsUsers);
Ref anotherUserRef = allUsers.getRepository().exactRef(anotherRefsUsers);
assertThat(userRef).isNotNull();
assertThat(anotherUserRef).isNotNull();
}
assertQuery("destination:destination1", change1);
@@ -3120,6 +3147,24 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
assertQuery("destination:destination3", change2, change1);
assertQuery("destination:destination4");
assertQuery("destination:destination5");
assertQuery("destination:destination6,user=" + anotherUserId, change1);
assertQuery("destination:name=destination6,user=" + anotherUserId, change1);
assertQuery("destination:user=" + anotherUserId + ",destination7", change2);
assertQuery("destination:user=" + anotherUserId + ",name=destination8", change2, change1);
assertQuery("destination:destination9,user=" + anotherUserId);
assertThatQueryException("destination:destination3,user=" + anotherUserId)
.hasMessageThat()
.isEqualTo("Unknown named destination: destination3");
assertThatQueryException("destination:destination3,user=test")
.hasMessageThat()
.isEqualTo("Account 'test' not found");
requestContext.setContext(newRequestContext(anotherUserId));
// account 1000000 is not visible to 'anotheruser' as they are not an admin
assertThatQueryException("destination:destination3,user=" + userId)
.hasMessageThat()
.isEqualTo("Account '1000000' not found");
}
@Test