opendev/gerrit
Code Issues Proposed changes

Support star labels

Browse Source 
At the moment Gerrit supports only one kind of star on changes, either
a change is starred by a user or it is not. For some use cases users
want to put different stars on changes. E.g. issue 1144 requests
colored stars (yellow, red, blue) and issue 2576 requests an ignore
star.

Star labels behave like private hashtags. A user can apply any label
to a change, but these labels are only visible to that user.

Having support for stars allows us to implement a first working
version of the ReviewIt Gerrit Android application.

To support star labels the API of StarredChangesUtil is extended so
that it can read/store star labels.

Star labels are stored in the change index. This is needed to be able
to return the star labels as part of ChangeInfo without needing to
scan for them. Including the star labels into ChangeInfo is done in a
follow-up change so that a online migration is possible, where search
results don't need to read from git:
1. The star labels are stored in the index, changes retured as query
results do not contain star labels (this change).
2. Run online schema migration, when finished the index contains the
new STAR field for all changes.
3. Include the star labels into ChangeInfo, access to git is not
needed since the star labels are read from the index (next change).

Also the STAR field in the index enables a new search operator for
star labels ("star:<label>") that finds changes that have been starred
by the current user with the given label

The STARREDBY field and the IsStarredByPredicate will be no longer
needed since the 'is:starred' query that they serve can be implemented
by using the new STAR field and the new StarPredicate. This is why the
STARREDBY field is and the IsStarredByPredicate predicate are
deprecated.

In addition a STARBY field is added to the index that tracks all users
that have starred the change with any label. This field is needed to
be able to list all starred changes (changes that have at least one
star by the user). Without this field we would need to scan the full
refs/starred-changes/ namespace for this. The STARBY field also backs
a new 'has:stars' query operator which finds all changes that have at
least one star by the current user.

The REST API is extended so that users can get/add/remove star labels.

New REST endpoints have been added to
- get star labels from a change
  GET /accounts/<account-id>/stars.changes/<change-id>
- update star labels on a change
  POST /accounts/<account-id>/stars.changes/<change-id>
- list changes that are starred by any label
  GET /accounts/<account-id>/stars.changes/

The REST endpoints that deal with the default stars are left
unmodified for backwards compatibility. They are exposed under
'/accounts/<id>/starred.changes/'.

Star labels are private to a user, hence a user can access only the
own star labels.

New methods have been added to the AccountApi to support get and
update of star labels, and listing of changes that have been starred
with any label.

Star labels also affect the ETag of a change. Instead of including the
actual labels into the ETag computation we simply use the ID of the
object in which the labels are stored in git. This way we don't need
to read the blob for the ETag computation.

The old method to retrieve starred changes from an IdentifiedUser and
the way to load the starred changes asynchroniously is deprecated now.
The asynchronous loading was needed when the starred changes were
(slowly) loaded from the database, but the new lookup is faster so
that we no longer need this asynchronous loading. The code for this is
still kept so that we can still support IsStarredByLegacyPredicate
which is needed to find starred changes when the change index doesn't
contain the new star fields yet.

Change-Id: I25d8af5a2a26930320c074225e26ff032889c891
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2016-04-15 10:39:13 +02:00
parent 398f7a5d0a
commit 9e972ccb44
31 changed files with 1116 additions and 114 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
Documentation/dev-stars.txt Normal file
View File

@ -0,0 +1,73 @@
= Gerrit Code Review - Stars
== Description
Changes can be starred with labels that behave like private hashtags.
Any label can be applied to a change, but these labels are only visible
to the user for which the labels have been set.
Stars allow users to categorize changes by self-defined criteria and
then build link:user-dashboards.html[dashboards] for them by making use
of the link:#query-stars[star query operators].
[[star-api]]
== Star API
The link:rest-api-accounts.html#star-endpoints[star REST API] supports:
* link:rest-api-accounts.html#get-stars[
get star labels from a change]
* link:rest-api-accounts.html#set-stars[
update star labels on a change]
* link:rest-api-accounts.html#get-starred-changes[
list changes that are starred by any label]
Star labels are also included in
link:rest-api-changes.html#change-info[ChangeInfo] entities that are
returned by the link:rest-api-changes.html[changes REST API].
There are link:rest-api-accounts.html#default-star-endpoints[
additional REST endpoints] for the link:#default-star[default star].
Only the link:#default-star[default star] is shown in the WebUi and
can be updated from there. Other stars do not show up in the WebUi.
[[default-star]]
== Default Star
If the default star is set by a user, this user is automatically
notified by email whenever updates are made to that change.
The default star is the star that is shown in the WebUI and which can
be updated from there.
The default star is represented by the special star label 'star'.
[[query-stars]]
== Query Stars
There are several query operators to find changes with stars:
* link:user-search.html#star[star:<LABEL>]:
Matches any change that was starred by the current user with the
label `<LABEL>`.
* link:user-search.html#has-stars[has:stars]:
Matches any change that was starred by the current user with any
label.
* link:user-search.html#is-starred[is:starred] /
link:user-search.html#has-star[has:star]:
Matches any change that was starred by the current user with the
link:#default-star[default star].
[[syntax]]
== Syntax
Star labels cannot contain whitespace characters. All other characters
are allowed.
GERRIT
------
Part of link:index.html[Gerrit Code Review]
SEARCHBOX
---------

1
Documentation/index.txt
View File

@ -68,6 +68,7 @@
.. link:dev-build-plugins.html[Building Gerrit plugins]
.. link:js-api.html[JavaScript Plugin API]
.. link:config-validation.html[Validation Interfaces]
.. link:dev-stars.html[Starring Changes]
. link:dev-design.html[System Design]
. link:i18n-readme.html[i18n Support]

168
Documentation/rest-api-accounts.txt
View File

@ -1573,16 +1573,19 @@ The result is sorted by project name in ascending order.
]
----
[[get-starred-changes]]
=== Get Starred Changes
[[default-star-endpoints]]
== Default Star Endpoints
[[get-changes-with-default-star]]
=== Get Changes With Default Star
--
'GET /accounts/link:#account-id[\{account-id\}]/starred.changes'
--
Gets the changes starred by the identified user account. This
URL endpoint is functionally identical to the changes query
`GET /changes/?q=is:starred`. The result is a list of
link:rest-api-changes.html#change-info[ChangeInfo] entities.
Gets the changes that were starred with the default star by the
identified user account. This URL endpoint is functionally identical
to the changes query `GET /changes/?q=is:starred`. The result is a list
of link:rest-api-changes.html#change-info[ChangeInfo] entities.
.Request
----
@ -1607,6 +1610,9 @@ link:rest-api-changes.html#change-info[ChangeInfo] entities.
"created": "2013-02-01 09:59:32.126000000",
"updated": "2013-02-21 11:16:36.775000000",
"starred": true,
"stars": [
"star"
],
"mergeable": true,
"submittable": false,
"insertions": 145,
@ -1620,14 +1626,15 @@ link:rest-api-changes.html#change-info[ChangeInfo] entities.
----
[[star-change]]
=== Star Change
=== Put Default Star On Change
--
'PUT /accounts/link:#account-id[\{account-id\}]/starred.changes/link:rest-api-changes.html#change-id[\{change-id\}]'
--
Star a change. Starred changes are returned for the search query
`is:starred` or `starredby:USER` and automatically notify the user
whenever updates are made to the change.
Star a change with the default label. Changes starred with the default
label are returned for the search query `is:starred` or `starredby:USER`
and automatically notify the user whenever updates are made to the
change.
.Request
----
@ -1640,12 +1647,12 @@ whenever updates are made to the change.
----
[[unstar-change]]
=== Unstar Change
=== Remove Default Star From Change
--
'DELETE /accounts/link:#account-id[\{account-id\}]/starred.changes/link:rest-api-changes.html#change-id[\{change-id\}]'
--
Unstar a change. Removes the starred flag, stopping notifications.
Remove the default star label from a change. This stops notifications.
.Request
----
@ -1657,6 +1664,131 @@ Unstar a change. Removes the starred flag, stopping notifications.
HTTP/1.1 204 No Content
----
[[star-endpoints]]
== Star Endpoints
[[get-starred-changes]]
=== Get Starred Changes
--
'GET /accounts/link:#account-id[\{account-id\}]/stars.changes'
--
Gets the changes that were starred with any label by the identified
user account. This URL endpoint is functionally identical to the
changes query `GET /changes/?q=has:stars`. The result is a list of
link:rest-api-changes.html#change-info[ChangeInfo] entities.
.Request
----
GET /a/accounts/self/stars.changes
----
.Response
----
HTTP/1.1 200 OK
Content-Disposition: attachment
Content-Type: application/json; charset=UTF-8
)]}'
[
{
"id": "myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940",
"project": "myProject",
"branch": "master",
"change_id": "I8473b95934b5732ac55d26311a706c9c2bde9940",
"subject": "Implementing Feature X",
"status": "NEW",
"created": "2013-02-01 09:59:32.126000000",
"updated": "2013-02-21 11:16:36.775000000",
"stars": [
"ignore",
"risky"
],
"mergeable": true,
"submittable": false,
"insertions": 145,
"deletions": 12,
"_number": 3965,
"owner": {
"name": "John Doe"
}
}
]
----
[[get-stars]]
=== Get Star Labels From Change
--
'GET /accounts/link:#account-id[\{account-id\}]/stars.changes/link:rest-api-changes.html#change-id[\{change-id\}]'
--
Get star labels from a change.
.Request
----
GET /a/accounts/self/stars.changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940 HTTP/1.0
----
As response the star labels that the user applied on the change are
returned. The labels are lexicographically sorted.
.Response
----
HTTP/1.1 200 OK
Content-Disposition: attachment
Content-Type: application/json; charset=UTF-8
)]
[
"blue",
"green",
"red"
]
----
[[set-stars]]
=== Update Star Labels On Change
--
'POST /accounts/link:#account-id[\{account-id\}]/stars.changes/link:rest-api-changes.html#change-id[\{change-id\}]'
--
Update star labels on a change. The star labels to be added/removed
must be specified in the request body as link:#star-input[StarInput]
entity. Starred changes are returned for the search query `has:stars`.
.Request
----
POST /a/accounts/self/stars.changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940 HTTP/1.0
Content-Type: application/json; charset=UTF-8
{
"add": [
"blue",
"red"
],
"remove": [
"yellow"
]
}
----
As response the star labels that the user applied on the change are
returned. The labels are lexicographically sorted.
.Response
----
HTTP/1.1 200 OK
Content-Disposition: attachment
Content-Type: application/json; charset=UTF-8
)]
[
"blue",
"green",
"red"
]
----
[[ids]]
== IDs
@ -2220,6 +2352,18 @@ user.
|`valid` ||Whether the SSH key is valid.
|=============================
[[stars-input]]
=== StarsInput
The `StarsInput` entity contains star labels that should be added to
or removed from a change.
[options="header",cols="1,^1,5"]
|========================
|Field Name ||Description
|`add` |optional|List of labels to add to the change.
|`remove` |optional|List of labels to remove from the change.
|========================
[[username-input]]
=== UsernameInput
The `UsernameInput` entity contains information for setting the

5
Documentation/rest-api-changes.txt
View File

@ -4113,7 +4113,10 @@ updated.
The link:rest-api.html#timestamp[timestamp] of when the change was
submitted.
|`starred` |not set if `false`|
Whether the calling user has starred this change.
Whether the calling user has starred this change with the default label.
|`stars` |optional|
A list of star labels that are applied by the calling user to this
change. The labels are lexicographically sorted.
|`reviewed` |not set if `false`|
Whether the change was reviewed by the calling user.
Only set if link:#reviewed[reviewed] is requested.

26
Documentation/user-search.txt
View File

@ -247,25 +247,43 @@ named exactly `Foo.java` and does not match `AbstractFoo.java`.
Regular expression matching can be enabled by starting the string
with `^`. In this mode `file:` is an alias of `path:` (see above).
[[star]]
star:'LABEL'::
+
Matches any change that was starred by the current user with the label
'LABEL'.
+
E.g. if changes that are not interesting are marked with an `ignore`
star, they could be filtered out by '-star:ignore'.
+
'star:star' is the same as 'has:star' and 'is:starred'.
[[has]]
has:draft::
+
True if there is a draft comment saved by the current user.
[[has-star]]
has:star::
+
Same as 'is:starred', true if the change has been starred by the
current user.
Same as 'is:starred' and 'star:star', true if the change has been
starred by the current user with the default label.
[[has-stars]]
has:stars::
+
True if the change has been starred by the current user with any label.
has:edit::
+
True if the change has inline edit created by the current user.
[[is]]
[[is-starred]]
is:starred::
+
Same as 'has:star', true if the change has been starred by the
current user.
current user with the default label.
is:watched::
+
@ -510,7 +528,7 @@ the change. This flag is always added to any query.
starredby:'USER'::
+
Matches changes that have been starred by 'USER'.
Matches changes that have been starred by 'USER' with the default label.
The special case `starredby:self` applies to the caller.
watchedby:'USER'::

63
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
View File

@ -23,20 +23,25 @@ import static com.google.gerrit.gpg.testutil.TestKeys.allValidKeys;
import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithExpiration;
import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithSecondUserId;
import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithoutExpiration;
import static com.google.gerrit.server.StarredChangesUtil.DEFAULT_LABEL;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.Function;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.io.BaseEncoding;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.AccountCreator;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.extensions.api.accounts.EmailInput;
import com.google.gerrit.extensions.api.changes.StarsInput;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.GpgKeyInfo;
import com.google.gerrit.extensions.common.SshKeyInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@ -173,11 +178,65 @@ public class AccountIT extends AbstractDaemonTest {
gApi.accounts()
.self()
.starChange(triplet);
assertThat(info(triplet).starred).isTrue();
ChangeInfo change = info(triplet);
assertThat(change.starred).isTrue();
gApi.accounts()
.self()
.unstarChange(triplet);
assertThat(info(triplet).starred).isNull();
change = info(triplet);
assertThat(change.starred).isNull();
}
@Test
public void starUnstarChangeWithLabels() throws Exception {
PushOneCommit.Result r = createChange();
String triplet = project.get() + "~master~" + r.getChangeId();
assertThat(gApi.accounts().self().getStars(triplet)).isEmpty();
assertThat(gApi.accounts().self().getStarredChanges()).isEmpty();
gApi.accounts().self().setStars(triplet,
new StarsInput(ImmutableSet.of(DEFAULT_LABEL, "red", "blue")));
ChangeInfo change = info(triplet);
assertThat(change.starred).isTrue();
assertThat(gApi.accounts().self().getStars(triplet))
.containsExactly("blue", "red", DEFAULT_LABEL).inOrder();
List<ChangeInfo> starredChanges =
gApi.accounts().self().getStarredChanges();
assertThat(starredChanges).hasSize(1);
ChangeInfo starredChange = starredChanges.get(0);
assertThat(starredChange._number).isEqualTo(r.getChange().getId().get());
assertThat(starredChange.starred).isTrue();
gApi.accounts().self().setStars(triplet,
new StarsInput(ImmutableSet.of("yellow"),
ImmutableSet.of(DEFAULT_LABEL, "blue")));
change = info(triplet);
assertThat(change.starred).isNull();
assertThat(gApi.accounts().self().getStars(triplet)).containsExactly(
"red", "yellow").inOrder();
starredChanges = gApi.accounts().self().getStarredChanges();
assertThat(starredChanges).hasSize(1);
starredChange = starredChanges.get(0);
assertThat(starredChange._number).isEqualTo(r.getChange().getId().get());
assertThat(starredChange.starred).isNull();
setApiUser(user);
exception.expect(AuthException.class);
exception.expectMessage("not allowed to get stars of another account");
gApi.accounts().id(Integer.toString((admin.id.get()))).getStars(triplet);
}
@Test
public void starWithInvalidLabels() throws Exception {
PushOneCommit.Result r = createChange();
String triplet = project.get() + "~master~" + r.getChangeId();
exception.expect(BadRequestException.class);
exception.expectMessage(
"invalid labels: another invalid label, invalid label");
gApi.accounts().self().setStars(triplet,
new StarsInput(ImmutableSet.of(DEFAULT_LABEL, "invalid label", "blue",
"another invalid label")));
}
@Test

31
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/AccountApi.java
View File

@ -14,11 +14,13 @@
package com.google.gerrit.extensions.api.accounts;
import com.google.gerrit.extensions.api.changes.StarsInput;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.client.EditPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.client.ProjectWatchInfo;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.GpgKeyInfo;
import com.google.gerrit.extensions.common.SshKeyInfo;
import com.google.gerrit.extensions.restapi.NotImplementedException;
@ -26,6 +28,7 @@ import com.google.gerrit.extensions.restapi.RestApiException;
import java.util.List;
import java.util.Map;
import java.util.SortedSet;
public interface AccountApi {
AccountInfo get() throws RestApiException;
@ -50,8 +53,12 @@ public interface AccountApi {
void deleteWatchedProjects(List<String> in)
throws RestApiException;
void starChange(String id) throws RestApiException;
void unstarChange(String id) throws RestApiException;
void starChange(String changeId) throws RestApiException;
void unstarChange(String changeId) throws RestApiException;
void setStars(String changeId, StarsInput input) throws RestApiException;
SortedSet<String> getStars(String changeId) throws RestApiException;
List<ChangeInfo> getStarredChanges() throws RestApiException;
void addEmail(EmailInput input) throws RestApiException;
List<SshKeyInfo> listSshKeys() throws RestApiException;
@ -130,12 +137,28 @@ public interface AccountApi {
}
@Override
public void starChange(String id) throws RestApiException {
public void starChange(String changeId) throws RestApiException {
throw new NotImplementedException();
}
@Override
public void unstarChange(String id) throws RestApiException {
public void unstarChange(String changeId) throws RestApiException {
throw new NotImplementedException();
}
@Override
public void setStars(String changeId, StarsInput input)
throws RestApiException {
throw new NotImplementedException();
}
@Override
public SortedSet<String> getStars(String changeId) throws RestApiException {
throw new NotImplementedException();
}
@Override
public List<ChangeInfo> getStarredChanges() throws RestApiException {
throw new NotImplementedException();
}

34
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/StarsInput.java Normal file
View File

@ -0,0 +1,34 @@
// Copyright (C) 2016 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.extensions.api.changes;
import java.util.Set;
public class StarsInput {
public Set<String> add;
public Set<String> remove;
public StarsInput() {
}
public StarsInput(Set<String> add) {
this.add = add;
}
public StarsInput(Set<String> add, Set<String> remove) {
this.add = add;
this.remove = remove;
}
}

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

@ -104,6 +104,8 @@ public class SearchSuggestOracle extends HighlightSuggestOracle {
suggestions.add("has:draft");
suggestions.add("has:edit");
suggestions.add("has:star");
suggestions.add("has:stars");
suggestions.add("star:");
suggestions.add("is:");
suggestions.add("is:starred");

22
gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
View File

@ -24,8 +24,10 @@ import static com.google.gerrit.server.index.change.ChangeField.PROJECT;
import static com.google.gerrit.server.index.change.IndexRewriter.CLOSED_STATUSES;
import static com.google.gerrit.server.index.change.IndexRewriter.OPEN_STATUSES;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
import com.google.common.collect.Sets;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
@ -35,6 +37,7 @@ 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.server.ReviewDb;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.index.FieldDef.FillArgs;
@ -116,6 +119,8 @@ public class LuceneChangeIndex implements ChangeIndex {
ChangeField.REVIEWEDBY.getName();
private static final String HASHTAG_FIELD =
ChangeField.HASHTAG_CASE_AWARE.getName();
private static final String STAR_FIELD = ChangeField.STAR.getName();
@Deprecated
private static final String STARREDBY_FIELD = ChangeField.STARREDBY.getName();
static Term idTerm(ChangeData cd) {
@ -420,6 +425,9 @@ public class LuceneChangeIndex implements ChangeIndex {
if (fields.contains(STARREDBY_FIELD)) {
decodeStarredBy(doc, cd);
}
if (fields.contains(STAR_FIELD)) {
decodeStar(doc, cd);
}
return cd;
}
@ -482,6 +490,7 @@ public class LuceneChangeIndex implements ChangeIndex {
cd.setHashtags(hashtags);
}
@Deprecated
private void decodeStarredBy(Document doc, ChangeData cd) {
IndexableField[] starredBy = doc.getFields(STARREDBY_FIELD);
Set<Account.Id> accounts =
@ -492,6 +501,19 @@ public class LuceneChangeIndex implements ChangeIndex {
cd.setStarredBy(accounts);
}
private void decodeStar(Document doc, ChangeData cd) {
IndexableField[] star = doc.getFields(STAR_FIELD);
Multimap<Account.Id, String> stars = ArrayListMultimap.create();
for (IndexableField r : star) {
StarredChangesUtil.StarField starField =
StarredChangesUtil.StarField.parse(r.stringValue());
if (starField != null) {
stars.put(starField.accountId(), starField.label());
}
}
cd.setStars(stars);
}
private static <T> List<T> decodeProtos(Document doc, String fieldName,
ProtobufCodec<T> codec) {
BytesRef[] bytesRefs = doc.getBinaryValues(fieldName);

2
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbUtil.java
View File

@ -21,7 +21,7 @@ import com.google.gwtorm.client.IntKey;
/** Static utilities for ReviewDb types. */
public class ReviewDbUtil {
private static final Function<IntKey<?>, Integer> INT_KEY_FUNCTION =
public static final Function<IntKey<?>, Integer> INT_KEY_FUNCTION =
new Function<IntKey<?>, Integer>() {
@Override
public Integer apply(IntKey<?> in) {

31
gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/ChangeTest.java
View File

@ -15,6 +15,8 @@
package com.google.gerrit.reviewdb.client;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import org.junit.Test;
@ -79,6 +81,27 @@ public class ChangeTest {
.isEqualTo("refs/changes/34/1234/");
}
@Test
public void parseRefNameParts() {
assertRefPart(1, "01/1");
assertNotRefPart(null);
assertNotRefPart("");
// This method assumes that the common prefix "refs/changes/" was removed.
assertNotRefPart("refs/changes/01/1");
// Invalid characters.
assertNotRefPart("01a/1");
assertNotRefPart("01/a1");
// Mismatched shard.
assertNotRefPart("01/23");
// Shard too short.
assertNotRefPart("1/1");
}
private static void assertRef(int changeId, String refName) {
assertThat(Change.Id.fromRef(refName)).isEqualTo(new Change.Id(changeId));
}
@ -86,4 +109,12 @@ public class ChangeTest {
private static void assertNotRef(String refName) {
assertThat(Change.Id.fromRef(refName)).isNull();
}
private static void assertRefPart(int changeId, String refName) {
assertEquals(new Change.Id(changeId), Change.Id.fromRefPart(refName));
}
private static void assertNotRefPart(String refName) {
assertNull(Change.Id.fromRefPart(refName));
}
}

1
gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java
View File

@ -79,6 +79,7 @@ public abstract class CurrentUser {
public abstract GroupMembership getEffectiveGroups();
/** Set of changes starred by this user. */
@Deprecated
public abstract Set<Change.Id> getStarredChanges();
/** Filters selecting changes the user wants to monitor. */

4
gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java
View File

@ -348,6 +348,7 @@ public class IdentifiedUser extends CurrentUser {
return starredChanges;
}
@Deprecated
public void clearStarredChanges() {
// Async query may have started before an update that the caller expects
// to see the results of, so we can't trust it.
@ -355,13 +356,14 @@ public class IdentifiedUser extends CurrentUser {
starredChanges = null;
}
@SuppressWarnings("deprecation")
@Deprecated
public void asyncStarredChanges() {
if (starredChanges == null && starredChangesUtil != null) {
starredQuery = starredChangesUtil.queryFromIndex(accountId);
}
}
@Deprecated
public void abortStarredChanges() {
if (starredQuery != null) {
try {

263
gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
View File

@ -16,14 +16,18 @@ package com.google.gerrit.server;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.auto.value.AutoValue;
import com.google.common.base.CharMatcher;
import com.google.common.base.Function;
import com.google.common.base.Joiner;
import com.google.common.base.Predicate;
import com.google.common.base.Splitter;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Iterables;
import com.google.common.primitives.Ints;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
@ -61,18 +65,66 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
@Singleton
public class StarredChangesUtil {
@AutoValue
public abstract static class StarField {
private static final String SEPARATOR = ":";
public static StarField parse(String s) {
int p = s.indexOf(SEPARATOR);
if (p >= 0) {
Integer id = Ints.tryParse(s.substring(0, p));
if (id == null) {
return null;
}
Account.Id accountId = new Account.Id(id);
String label = s.substring(p + 1);
return create(accountId, label);
}
return null;
}
public static StarField create(Account.Id accountId, String label) {
return new AutoValue_StarredChangesUtil_StarField(accountId, label);
}
public abstract Account.Id accountId();
public abstract String label();
@Override
public String toString() {
return accountId() + SEPARATOR + label();
}
}
public static class IllegalLabelException extends IllegalArgumentException {
private static final long serialVersionUID = 1L;
static IllegalLabelException invalidLabels(Set<String> invalidLabels) {
return new IllegalLabelException(
String.format("invalid labels: %s",
Joiner.on(", ").join(invalidLabels)));
}
IllegalLabelException(String message) {
super(message);
}
}
private static final Logger log =
LoggerFactory.getLogger(StarredChangesUtil.class);
private static final String DEFAULT_LABEL = "star";
public static final String DEFAULT_LABEL = "star";
public static final ImmutableSortedSet<String> DEFAULT_LABELS =
ImmutableSortedSet.of(DEFAULT_LABEL);
@ -98,53 +150,44 @@ public class StarredChangesUtil {
this.queryProvider = queryProvider;
}
public void star(Account.Id accountId, Project.NameKey project,
public ImmutableSortedSet<String> getLabels(Account.Id accountId,
Change.Id changeId) throws OrmException {
try (Repository repo = repoManager.openRepository(allUsers)) {
String refName = RefNames.refsStarredChanges(changeId, accountId);
ObjectId oldObjectId = getObjectId(repo, refName);
SortedSet<String> labels = readLabels(repo, oldObjectId);
labels.add(DEFAULT_LABEL);
updateLabels(repo, refName, oldObjectId, labels);
indexer.index(dbProvider.get(), project, changeId);
return ImmutableSortedSet.copyOf(
readLabels(repo, RefNames.refsStarredChanges(changeId, accountId)));
} catch (IOException e) {
throw new OrmException(
String.format("Star change %d for account %d failed",
String.format("Reading stars from change %d for account %d failed",
changeId.get(), accountId.get()), e);
}
}
public void unstar(Account.Id accountId, Project.NameKey project,
Change.Id changeId) throws OrmException {
try (Repository repo = repoManager.openRepository(allUsers);
RevWalk rw = new RevWalk(repo)) {
RefUpdate u = repo.updateRef(
RefNames.refsStarredChanges(changeId, accountId));
u.setForceUpdate(true);
u.setRefLogIdent(serverIdent);
u.setRefLogMessage("Unstar change " + changeId.get(), true);
RefUpdate.Result result = u.delete();
switch (result) {
case FORCED:
indexer.index(dbProvider.get(), project, changeId);
return;
case FAST_FORWARD:
case IO_FAILURE:
case LOCK_FAILURE:
case NEW:
case NOT_ATTEMPTED:
case NO_CHANGE:
case REJECTED:
case REJECTED_CURRENT_BRANCH:
case RENAMED:
default:
throw new OrmException(
String.format("Unstar change %d for account %d failed: %s",
changeId.get(), accountId.get(), result.name()));
public ImmutableSortedSet<String> star(Account.Id accountId,
Project.NameKey project, Change.Id changeId, Set<String> labelsToAdd,
Set<String> labelsToRemove) throws OrmException {
try (Repository repo = repoManager.openRepository(allUsers)) {
String refName = RefNames.refsStarredChanges(changeId, accountId);
ObjectId oldObjectId = getObjectId(repo, refName);
SortedSet<String> labels = readLabels(repo, oldObjectId);
if (labelsToAdd != null) {
labels.addAll(labelsToAdd);
}
if (labelsToRemove != null) {
labels.removeAll(labelsToRemove);
}
if (labels.isEmpty()) {
deleteRef(repo, refName, oldObjectId);
} else {
updateLabels(repo, refName, oldObjectId, labels);
}
indexer.index(dbProvider.get(), project, changeId);
return ImmutableSortedSet.copyOf(labels);
} catch (IOException e) {
throw new OrmException(
String.format("Unstar change %d for account %d failed",
String.format("Star change %d for account %d failed",
changeId.get(), accountId.get()), e);
}
}
@ -157,7 +200,7 @@ public class StarredChangesUtil {
batchUpdate.setAllowNonFastForwards(true);
batchUpdate.setRefLogIdent(serverIdent);
batchUpdate.setRefLogMessage("Unstar change " + changeId.get(), true);
for (Account.Id accountId : byChangeFromIndex(changeId)) {
for (Account.Id accountId : byChangeFromIndex(changeId).keySet()) {
String refName = RefNames.refsStarredChanges(changeId, accountId);
Ref ref = repo.getRefDatabase().getRef(refName);
batchUpdate.addCommand(new ReceiveCommand(ref.getObjectId(),
@ -178,29 +221,84 @@ public class StarredChangesUtil {
}
}
public Set<Account.Id> byChange(Change.Id changeId)
public ImmutableMultimap<Account.Id, String> byChange(Change.Id changeId)
throws OrmException {
return FluentIterable
.from(getRefNames(RefNames.refsStarredChangesPrefix(changeId)))
.transform(new Function<String, Account.Id>() {
@Override
public Account.Id apply(String refPart) {
return Account.Id.parse(refPart);
}
}).toSet();
try (Repository repo = repoManager.openRepository(allUsers)) {
ImmutableMultimap.Builder<Account.Id, String> builder =
new ImmutableMultimap.Builder<>();
for (String refPart : getRefNames(repo,
RefNames.refsStarredChangesPrefix(changeId))) {
Integer id = Ints.tryParse(refPart);
if (id == null) {
continue;
}
Account.Id accountId = new Account.Id(id);
builder.putAll(accountId,
readLabels(repo, RefNames.refsStarredChanges(changeId, accountId)));
}
return builder.build();
} catch (IOException e) {
throw new OrmException(String.format(
"Get accounts that starred change %d failed", changeId.get()), e);
}
}
public Set<Account.Id> byChangeFromIndex(Change.Id changeId)
throws OrmException, NoSuchChangeException {
public Set<Account.Id> byChange(final Change.Id changeId,
final String label) throws OrmException {
try (final Repository repo = repoManager.openRepository(allUsers)) {
return FluentIterable
.from(getRefNames(repo, RefNames.refsStarredChangesPrefix(changeId)))
.transform(new Function<String, Account.Id>() {
@Override
public Account.Id apply(String refPart) {
return Account.Id.parse(refPart);
}
})
.filter(new Predicate<Account.Id>() {
@Override
public boolean apply(Account.Id accountId) {
try {
return readLabels(repo,
RefNames.refsStarredChanges(changeId, accountId))
.contains(label);
} catch (IOException e) {
log.error(String.format(
"Cannot query stars by account %d on change %d",
accountId.get(), changeId.get()), e);
return false;
}
}
}).toSet();
} catch (IOException e) {
throw new OrmException(
String.format("Get accounts that starred change %d failed",
changeId.get()), e);
}
}
public ImmutableMultimap<Account.Id, String> byChangeFromIndex(
Change.Id changeId) throws OrmException, NoSuchChangeException {
Set<String> fields = ImmutableSet.of(
ChangeField.ID.getName(),
ChangeField.STARREDBY.getName());
ChangeField.STAR.getName());
List<ChangeData> changeData = queryProvider.get().setRequestedFields(fields)
.byLegacyChangeId(changeId);
if (changeData.size() != 1) {
throw new NoSuchChangeException(changeId);
}
return changeData.get(0).starredBy();
return changeData.get(0).stars();
}
public Set<Account.Id> byChangeFromIndex(Change.Id changeId, String label)
throws OrmException, NoSuchChangeException {
Set<Account.Id> accounts = new HashSet<>();
for (Map.Entry<Account.Id, Collection<String>> e : byChangeFromIndex(
changeId).asMap().entrySet()) {
if (e.getValue().contains(label)) {
accounts.add(e.getKey());
}
}
return accounts;
}
@Deprecated
@ -226,12 +324,21 @@ public class StarredChangesUtil {
}
}
private Set<String> getRefNames(String prefix) throws OrmException {
private static Set<String> getRefNames(Repository repo, String prefix)
throws IOException {
RefDatabase refDb = repo.getRefDatabase();
return refDb.getRefs(prefix).keySet();
}
public ObjectId getObjectId(Account.Id accountId, Change.Id changeId) {
try (Repository repo = repoManager.openRepository(allUsers)) {
RefDatabase refDb = repo.getRefDatabase();
return refDb.getRefs(prefix).keySet();
return getObjectId(repo,
RefNames.refsStarredChanges(changeId, accountId));
} catch (IOException e) {
throw new OrmException(e);
log.error(String.format(
"Getting star object ID for account %d on change %d failed",
accountId.get(), changeId.get()), e);
return ObjectId.zeroId();
}
}
@ -241,6 +348,11 @@ public class StarredChangesUtil {
return ref != null ? ref.getObjectId() : ObjectId.zeroId();
}
private static SortedSet<String> readLabels(Repository repo, String refName)
throws IOException {
return readLabels(repo, getObjectId(repo, refName));
}
private static TreeSet<String> readLabels(Repository repo, ObjectId id)
throws IOException {
if (ObjectId.zeroId().equals(id)) {
@ -259,13 +371,7 @@ public class StarredChangesUtil {
public static ObjectId writeLabels(Repository repo, SortedSet<String> labels)
throws IOException {
SortedSet<String> invalidLabels = validateLabels(labels);
if (!invalidLabels.isEmpty()) {
throw new IllegalArgumentException(
String.format("Invalid star labels: %s",
Joiner.on(", ").join(labels)));
}
validateLabels(labels);
try (ObjectInserter oi = repo.newObjectInserter()) {
ObjectId id = oi.insert(Constants.OBJ_BLOB,
Joiner.on("\n").join(labels).getBytes(UTF_8));
@ -274,9 +380,9 @@ public class StarredChangesUtil {
}
}
private static SortedSet<String> validateLabels(Set<String> labels) {
private static void validateLabels(Set<String> labels) {
if (labels == null) {
return ImmutableSortedSet.of();
return;
}
SortedSet<String> invalidLabels = new TreeSet<>();
@ -285,7 +391,9 @@ public class StarredChangesUtil {
invalidLabels.add(label);
}
}
return invalidLabels;
if (!invalidLabels.isEmpty()) {
throw IllegalLabelException.invalidLabels(invalidLabels);
}
}
private void updateLabels(Repository repo, String refName,
@ -317,4 +425,29 @@ public class StarredChangesUtil {
}
}
}
private void deleteRef(Repository repo, String refName, ObjectId oldObjectId)
throws IOException, OrmException {
RefUpdate u = repo.updateRef(refName);
u.setForceUpdate(true);
u.setExpectedOldObjectId(oldObjectId);
u.setRefLogIdent(serverIdent);
u.setRefLogMessage("Unstar change", true);
RefUpdate.Result result = u.delete();
switch (result) {
case FORCED:
return;
case NEW:
case NO_CHANGE:
case FAST_FORWARD:
case IO_FAILURE:
case LOCK_FAILURE:
case NOT_ATTEMPTED:
case REJECTED:
case REJECTED_CURRENT_BRANCH:
case RENAMED:
throw new OrmException(String.format("Delete star ref %s failed: %s",
refName, result.name()));
}
}
}

30
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResource.java
View File

@ -22,6 +22,8 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.ChangeResource;
import com.google.inject.TypeLiteral;
import java.util.Set;
public class AccountResource implements RestResource {
public static final TypeLiteral<RestView<AccountResource>> ACCOUNT_KIND =
new TypeLiteral<RestView<AccountResource>>() {};
@ -108,4 +110,32 @@ public class AccountResource implements RestResource {
return change.getChange();
}
}
public static class Star implements RestResource {
public static final TypeLiteral<RestView<Star>> STAR_KIND =
new TypeLiteral<RestView<Star>>() {};
private final IdentifiedUser user;
private final ChangeResource change;
private final Set<String> labels;
public Star(IdentifiedUser user, ChangeResource change,
Set<String> labels) {
this.user = user;
this.change = change;
this.labels = labels;
}
public IdentifiedUser getUser() {
return user;
}
public Change getChange() {
return change.getChange();
}
public Set<String> getLabels() {
return labels;
}
}
}

6
gerrit-server/src/main/java/com/google/gerrit/server/account/Module.java
View File

@ -19,6 +19,7 @@ import static com.google.gerrit.server.account.AccountResource.CAPABILITY_KIND;
import static com.google.gerrit.server.account.AccountResource.EMAIL_KIND;
import static com.google.gerrit.server.account.AccountResource.SSH_KEY_KIND;
import static com.google.gerrit.server.account.AccountResource.STARRED_CHANGE_KIND;
import static com.google.gerrit.server.account.AccountResource.Star.STAR_KIND;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.RestApiModule;
@ -34,6 +35,7 @@ public class Module extends RestApiModule {
DynamicMap.mapOf(binder(), EMAIL_KIND);
DynamicMap.mapOf(binder(), SSH_KEY_KIND);
DynamicMap.mapOf(binder(), STARRED_CHANGE_KIND);
DynamicMap.mapOf(binder(), STAR_KIND);
put(ACCOUNT_KIND).to(PutAccount.class);
get(ACCOUNT_KIND).to(GetAccount.class);
@ -83,6 +85,10 @@ public class Module extends RestApiModule {
delete(STARRED_CHANGE_KIND).to(StarredChanges.Delete.class);
bind(StarredChanges.Create.class);
child(ACCOUNT_KIND, "stars.changes").to(Stars.class);
get(STAR_KIND).to(Stars.Get.class);
post(STAR_KIND).to(Stars.Post.class);
factory(CreateAccount.Factory.class);
factory(CreateEmail.Factory.class);
}

27
gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java
View File

@ -53,31 +53,29 @@ public class StarredChanges implements
private final ChangesCollection changes;
private final DynamicMap<RestView<AccountResource.StarredChange>> views;
private final Provider<Create> createProvider;
private final StarredChangesUtil starredChangesUtil;
@Inject
StarredChanges(ChangesCollection changes,
DynamicMap<RestView<AccountResource.StarredChange>> views,
Provider<Create> createProvider) {
Provider<Create> createProvider,
StarredChangesUtil starredChangesUtil) {
this.changes = changes;
this.views = views;
this.createProvider = createProvider;
this.starredChangesUtil = starredChangesUtil;
}
@Override
public AccountResource.StarredChange parse(AccountResource parent, IdString id)
throws ResourceNotFoundException, OrmException {
IdentifiedUser user = parent.getUser();
try {
user.asyncStarredChanges();
ChangeResource change = changes.parse(TopLevelResource.INSTANCE, id);
if (user.getStarredChanges().contains(change.getId())) {
return new AccountResource.StarredChange(user, change);
}
throw new ResourceNotFoundException(id);
} finally {
user.abortStarredChanges();
ChangeResource change = changes.parse(TopLevelResource.INSTANCE, id);
if (starredChangesUtil.getLabels(user.getAccountId(), change.getId())
.contains(StarredChangesUtil.DEFAULT_LABEL)) {
return new AccountResource.StarredChange(user, change);
}
throw new ResourceNotFoundException(id);
}
@Override
@ -138,7 +136,7 @@ public class StarredChanges implements
}
try {
starredChangesUtil.star(self.get().getAccountId(), change.getProject(),
change.getId());
change.getId(), StarredChangesUtil.DEFAULT_LABELS, null);
} catch (OrmDuplicateKeyException e) {
return Response.none();
}
@ -184,8 +182,9 @@ public class StarredChanges implements
if (self.get() != rsrc.getUser()) {
throw new AuthException("not allowed remove starred change");
}
starredChangesUtil.unstar(self.get().getAccountId(),
rsrc.getChange().getProject(), rsrc.getChange().getId());
starredChangesUtil.star(self.get().getAccountId(),
rsrc.getChange().getProject(), rsrc.getChange().getId(), null,
StarredChangesUtil.DEFAULT_LABELS);
return Response.none();
}
}

167
gerrit-server/src/main/java/com/google/gerrit/server/account/Stars.java Normal file
View File

@ -0,0 +1,167 @@
// Copyright (C) 2016 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.account;
import com.google.gerrit.extensions.api.changes.StarsInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.StarredChangesUtil.IllegalLabelException;
import com.google.gerrit.server.account.AccountResource.Star;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.query.change.QueryChanges;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.SortedSet;
@Singleton
public class Stars implements
ChildCollection<AccountResource, AccountResource.Star> {
private final ChangesCollection changes;
private final ListStarredChanges listStarredChanges;
private final StarredChangesUtil starredChangesUtil;
private final DynamicMap<RestView<AccountResource.Star>> views;
@Inject
Stars(ChangesCollection changes,
ListStarredChanges listStarredChanges,
StarredChangesUtil starredChangesUtil,
DynamicMap<RestView<AccountResource.Star>> views) {
this.changes = changes;
this.listStarredChanges = listStarredChanges;
this.starredChangesUtil = starredChangesUtil;
this.views = views;
}
@Override
public Star parse(AccountResource parent, IdString id)
throws ResourceNotFoundException, OrmException {
IdentifiedUser user = parent.getUser();
ChangeResource change = changes.parse(TopLevelResource.INSTANCE, id);
Set<String> labels =
starredChangesUtil.getLabels(user.getAccountId(), change.getId());
return new AccountResource.Star(user, change, labels);
}
@Override
public DynamicMap<RestView<Star>> views() {
return views;
}
@Override
public ListStarredChanges list() {
return listStarredChanges;
}
@Singleton
public static class ListStarredChanges
implements RestReadView<AccountResource> {
private final Provider<CurrentUser> self;
private final ChangesCollection changes;
@Inject
ListStarredChanges(Provider<CurrentUser> self,
ChangesCollection changes) {
this.self = self;
this.changes = changes;
}
@Override
@SuppressWarnings("unchecked")
public List<ChangeInfo> apply(AccountResource rsrc)
throws BadRequestException, AuthException, OrmException {
if (self.get() != rsrc.getUser()) {
throw new AuthException(
"not allowed to list stars of another account");
}
QueryChanges query = changes.list();
query.addQuery("has:stars");
return (List<ChangeInfo>) query.apply(TopLevelResource.INSTANCE);
}
}
@Singleton
public static class Get implements
RestReadView<AccountResource.Star> {
private final Provider<CurrentUser> self;
private final StarredChangesUtil starredChangesUtil;
@Inject
Get(Provider<CurrentUser> self,
StarredChangesUtil starredChangesUtil) {
this.self = self;
this.starredChangesUtil = starredChangesUtil;
}
@Override
public SortedSet<String> apply(AccountResource.Star rsrc)
throws AuthException, OrmException {
if (self.get() != rsrc.getUser()) {
throw new AuthException("not allowed to get stars of another account");
}
return starredChangesUtil.getLabels(self.get().getAccountId(),
rsrc.getChange().getId());
}
}
@Singleton
public static class Post implements
RestModifyView<AccountResource.Star, StarsInput> {
private final Provider<CurrentUser> self;
private final StarredChangesUtil starredChangesUtil;
@Inject
Post(Provider<CurrentUser> self,
StarredChangesUtil starredChangesUtil) {
this.self = self;
this.starredChangesUtil = starredChangesUtil;
}
@Override
public Collection<String> apply(AccountResource.Star rsrc, StarsInput in)
throws AuthException, BadRequestException, OrmException {
if (self.get() != rsrc.getUser()) {
throw new AuthException(
"not allowed to update stars of another account");
}
try {
return starredChangesUtil.star(self.get().getAccountId(),
rsrc.getChange().getProject(), rsrc.getChange().getId(), in.add,
in.remove);
} catch (IllegalLabelException e) {
throw new BadRequestException(e.getMessage());
}
}
}
}

53
gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
View File

@ -19,11 +19,13 @@ import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.accounts.AccountApi;
import com.google.gerrit.extensions.api.accounts.EmailInput;
import com.google.gerrit.extensions.api.accounts.GpgKeyApi;
import com.google.gerrit.extensions.api.changes.StarsInput;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.client.EditPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.client.ProjectWatchInfo;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.GpgKeyInfo;
import com.google.gerrit.extensions.common.SshKeyInfo;
import com.google.gerrit.extensions.restapi.IdString;
@ -48,6 +50,7 @@ import com.google.gerrit.server.account.SetPreferences;
import com.google.gerrit.server.account.SshKeys;
import com.google.gerrit.server.account.PostWatchedProjects;
import com.google.gerrit.server.account.StarredChanges;
import com.google.gerrit.server.account.Stars;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gwtorm.server.OrmException;
@ -59,6 +62,7 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.SortedSet;
public class AccountApiImpl implements AccountApi {
interface Factory {
@ -80,6 +84,9 @@ public class AccountApiImpl implements AccountApi {
private final DeleteWatchedProjects deleteWatchedProjects;
private final StarredChanges.Create starredChangesCreate;
private final StarredChanges.Delete starredChangesDelete;
private final Stars stars;
private final Stars.Get starsGet;
private final Stars.Post starsPost;
private final CreateEmail.Factory createEmailFactory;
private final GpgApiAdapter gpgApiAdapter;
private final GetSshKeys getSshKeys;
@ -102,6 +109,9 @@ public class AccountApiImpl implements AccountApi {
DeleteWatchedProjects deleteWatchedProjects,
StarredChanges.Create starredChangesCreate,
StarredChanges.Delete starredChangesDelete,
Stars stars,
Stars.Get starsGet,
Stars.Post starsPost,
CreateEmail.Factory createEmailFactory,
GpgApiAdapter gpgApiAdapter,
GetSshKeys getSshKeys,
@ -124,6 +134,9 @@ public class AccountApiImpl implements AccountApi {
this.deleteWatchedProjects = deleteWatchedProjects;
this.starredChangesCreate = starredChangesCreate;
this.starredChangesDelete = starredChangesDelete;
this.stars = stars;
this.starsGet = starsGet;
this.starsPost = starsPost;
this.createEmailFactory = createEmailFactory;
this.getSshKeys = getSshKeys;
this.addSshKey = addSshKey;
@ -234,11 +247,11 @@ public class AccountApiImpl implements AccountApi {
}
@Override
public void starChange(String id) throws RestApiException {
public void starChange(String changeId) throws RestApiException {
try {
ChangeResource rsrc = changes.parse(
TopLevelResource.INSTANCE,
IdString.fromUrl(id));
IdString.fromUrl(changeId));
starredChangesCreate.setChange(rsrc);
starredChangesCreate.apply(account, new StarredChanges.EmptyInput());
} catch (OrmException | IOException e) {
@ -247,10 +260,10 @@ public class AccountApiImpl implements AccountApi {
}
@Override
public void unstarChange(String id) throws RestApiException {
public void unstarChange(String changeId) throws RestApiException {
try {
ChangeResource rsrc =
changes.parse(TopLevelResource.INSTANCE, IdString.fromUrl(id));
changes.parse(TopLevelResource.INSTANCE, IdString.fromUrl(changeId));
AccountResource.StarredChange starredChange =
new AccountResource.StarredChange(account.getUser(), rsrc);
starredChangesDelete.apply(starredChange,
@ -260,6 +273,38 @@ public class AccountApiImpl implements AccountApi {
}
}
@Override
public void setStars(String changeId, StarsInput input)
throws RestApiException {
try {
AccountResource.Star rsrc =
stars.parse(account, IdString.fromUrl(changeId));
starsPost.apply(rsrc, input);
} catch (OrmException e) {
throw new RestApiException("Cannot post stars", e);
}
}
@Override
public SortedSet<String> getStars(String changeId) throws RestApiException {
try {
AccountResource.Star rsrc =
stars.parse(account, IdString.fromUrl(changeId));
return starsGet.apply(rsrc);
} catch (OrmException e) {
throw new RestApiException("Cannot get stars", e);
}
}
@Override
public List<ChangeInfo> getStarredChanges() throws RestApiException {
try {
return stars.list().apply(account);
} catch (OrmException e) {
throw new RestApiException("Cannot get starred changes", e);
}
}
@Override
public void addEmail(EmailInput input) throws RestApiException {
AccountResource.Email rsrc =

1
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
View File

@ -89,6 +89,7 @@ import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.WebLinks;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.api.accounts.AccountInfoComparator;

14
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java
View File

@ -14,6 +14,8 @@
package com.google.gerrit.server.change;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.MoreObjects;
import com.google.common.hash.Hasher;
import com.google.common.hash.Hashing;
@ -25,6 +27,7 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectState;
@ -51,10 +54,13 @@ public class ChangeResource implements RestResource, HasETag {
ChangeResource create(ChangeControl ctl);
}
private final StarredChangesUtil starredChangesUtil;
private final ChangeControl control;
@AssistedInject
ChangeResource(@Assisted ChangeControl control) {
ChangeResource(StarredChangesUtil starredChangesUtil,
@Assisted ChangeControl control) {
this.starredChangesUtil = starredChangesUtil;
this.control = control;
}
@ -115,8 +121,10 @@ public class ChangeResource implements RestResource, HasETag {
@Override
public String getETag() {
CurrentUser user = control.getUser();
Hasher h = Hashing.md5().newHasher()
.putBoolean(user.getStarredChanges().contains(getId()));
Hasher h = Hashing.md5().newHasher();
h.putString(
starredChangesUtil.getObjectId(user.getAccountId(), getId()).name(),
UTF_8);
prepareETag(h, user);
return h.hash().toString();
}

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

@ -30,6 +30,8 @@ import com.google.gerrit.reviewdb.client.ChangeMessage;
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.server.ReviewDbUtil;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.index.FieldDef;
import com.google.gerrit.server.index.FieldType;
import com.google.gerrit.server.index.SchemaUtil;
@ -51,6 +53,7 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
/**
@ -593,6 +596,7 @@ public class ChangeField {
};
/** Users who have starred this change. */
@Deprecated
public static final FieldDef<ChangeData, Iterable<Integer>> STARREDBY =
new FieldDef.Repeatable<ChangeData, Integer>(
ChangeQueryBuilder.FIELD_STARREDBY, FieldType.INTEGER, true) {
@ -609,6 +613,38 @@ public class ChangeField {
}
};
/**
* Star labels on this change in the format: <account-id>:<label>
*/
public static final FieldDef<ChangeData, Iterable<String>> STAR =
new FieldDef.Repeatable<ChangeData, String>(
ChangeQueryBuilder.FIELD_STAR, FieldType.EXACT, true) {
@Override
public Iterable<String> get(ChangeData input, FillArgs args)
throws OrmException {
return Iterables.transform(input.stars().entries(),
new Function<Map.Entry<Account.Id, String>, String>() {
@Override
public String apply(Map.Entry<Account.Id, String> e) {
return StarredChangesUtil.StarField.create(
e.getKey(), e.getValue()).toString();
}
});
}
};
/** Users that have starred the change with any label. */
public static final FieldDef<ChangeData, Iterable<Integer>> STARBY =
new FieldDef.Repeatable<ChangeData, Integer>(
ChangeQueryBuilder.FIELD_STARBY, FieldType.INTEGER, false) {
@Override
public Iterable<Integer> get(ChangeData input, FillArgs args)
throws OrmException {
return Iterables.transform(input.stars().keySet(),
ReviewDbUtil.INT_KEY_FUNCTION);
}
};
/** Opaque group identifiers for this change's patch sets. */
public static final FieldDef<ChangeData, Iterable<String>> GROUP =
new FieldDef.Repeatable<ChangeData, String>(

4
gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
View File

@ -67,9 +67,13 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
@Deprecated
static final Schema<ChangeData> V28 = schema(V27, ChangeField.STARREDBY);
@Deprecated
static final Schema<ChangeData> V29 =
schema(V28, ChangeField.HASHTAG_CASE_AWARE);
static final Schema<ChangeData> V30 =
schema(V29, ChangeField.STAR, ChangeField.STARBY);
public static final String NAME = "changes";
public static final ChangeSchemaDefinitions INSTANCE =
new ChangeSchemaDefinitions();

5
gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
View File

@ -28,6 +28,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.mail.ProjectWatch.Watchers;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListEntry;
@ -305,8 +306,8 @@ public abstract class ChangeEmail extends NotificationEmail {
try {
// BCC anyone who has starred this change.
//
for (Account.Id accountId : args.starredChangesUtil
.byChangeFromIndex(change.getId())) {
for (Account.Id accountId : args.starredChangesUtil.byChangeFromIndex(
change.getId(), StarredChangesUtil.DEFAULT_LABEL)) {
super.add(RecipientType.BCC, accountId);
}
} catch (OrmException | NoSuchChangeException err) {

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

@ -24,10 +24,12 @@ import com.google.common.base.Optional;
import com.google.common.base.Predicate;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.common.collect.SetMultimap;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.SubmitRecord;
@ -345,7 +347,9 @@ public class ChangeData {
private Set<Account.Id> editsByUser;
private Set<Account.Id> reviewedBy;
private Set<Account.Id> draftsByUser;
@Deprecated
private Set<Account.Id> starredByUser;
private ImmutableMultimap<Account.Id, String> stars;
private PersonIdent author;
private PersonIdent committer;
@ -1050,17 +1054,31 @@ public void setPatchSets(Collection<PatchSet> patchSets) {
this.hashtags = hashtags;
}
@Deprecated
public Set<Account.Id> starredBy() throws OrmException {
if (starredByUser == null) {
starredByUser = checkNotNull(starredChangesUtil).byChange(legacyId);
starredByUser = checkNotNull(starredChangesUtil).byChange(
legacyId, StarredChangesUtil.DEFAULT_LABEL);
}
return starredByUser;
}
@Deprecated
public void setStarredBy(Set<Account.Id> starredByUser) {
this.starredByUser = starredByUser;
}
public ImmutableMultimap<Account.Id, String> stars() throws OrmException {
if (stars == null) {
stars = checkNotNull(starredChangesUtil).byChange(legacyId);
}
return stars;
}
public void setStars(Multimap<Account.Id, String> stars) {
this.stars = ImmutableMultimap.copyOf(stars);
}
@AutoValue
abstract static class ReviewedByEvent {
private static ReviewedByEvent create(ChangeMessage msg) {

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

@ -35,6 +35,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupBackend;
@ -138,6 +139,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
public static final String FIELD_REVIEWEDBY = "reviewedby";
public static final String FIELD_REVIEWER = "reviewer";
public static final String FIELD_REVIEWERIN = "reviewerin";
public static final String FIELD_STAR = "star";
public static final String FIELD_STARBY = "starby";
public static final String FIELD_STARREDBY = "starredby";
public static final String FIELD_STATUS = "status";
public static final String FIELD_TOPIC = "topic";
@ -422,6 +425,10 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
return starredby(self());
}
if ("stars".equalsIgnoreCase(value)) {
return new HasStarsPredicate(self());
}
if ("draft".equalsIgnoreCase(value)) {
return draftby(self());
}
@ -645,6 +652,11 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
return new MessagePredicate(args.index, text);
}
@Operator
public Predicate<ChangeData> star(String label) throws QueryParseException {
return new StarPredicate(self(), label);
}
@Operator
public Predicate<ChangeData> starredby(String who)
throws QueryParseException, OrmException {
@ -663,6 +675,10 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
@SuppressWarnings("deprecation")
private Predicate<ChangeData> starredby(Account.Id who)
throws QueryParseException {
if (args.getSchema().hasField(ChangeField.STAR)) {
return new StarPredicate(who, StarredChangesUtil.DEFAULT_LABEL);
}
return args.getSchema().hasField(ChangeField.STARREDBY)
? new IsStarredByPredicate(who)
: new IsStarredByLegacyPredicate(args.asUser(who));

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

@ -0,0 +1,44 @@
// Copyright (C) 2016 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.IndexPredicate;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gwtorm.server.OrmException;
public class HasStarsPredicate extends IndexPredicate<ChangeData> {
private final Account.Id accountId;
HasStarsPredicate(Account.Id accountId) {
super(ChangeField.STARBY, accountId.toString());
this.accountId = accountId;
}
@Override
public boolean match(ChangeData cd) throws OrmException {
return cd.stars().containsKey(accountId);
}
@Override
public int getCost() {
return 1;
}
@Override
public String toString() {
return ChangeQueryBuilder.FIELD_STARBY + ":" + accountId;
}
}

1
gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java
View File

@ -19,6 +19,7 @@ import com.google.gerrit.server.index.IndexPredicate;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gwtorm.server.OrmException;
@Deprecated
class IsStarredByPredicate extends IndexPredicate<ChangeData> {
private final Account.Id accountId;

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

@ -0,0 +1,48 @@
// Copyright (C) 2016 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.StarredChangesUtil;
import com.google.gerrit.server.index.IndexPredicate;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gwtorm.server.OrmException;
public class StarPredicate extends IndexPredicate<ChangeData> {
private final Account.Id accountId;
private final String label;
StarPredicate(Account.Id accountId, String label) {
super(ChangeField.STAR,
StarredChangesUtil.StarField.create(accountId, label).toString());
this.accountId = accountId;
this.label = label;
}
@Override
public boolean match(ChangeData cd) throws OrmException {
return cd.stars().get(accountId).contains(label);
}
@Override
public int getCost() {
return 1;
}
@Override
public String toString() {
return ChangeQueryBuilder.FIELD_STAR + ":" + label;
}
}

32
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@ -38,6 +38,7 @@ import com.google.gerrit.extensions.api.changes.Changes.QueryRequest;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.api.changes.HashtagsInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.StarsInput;
import com.google.gerrit.extensions.api.groups.GroupInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
@ -55,6 +56,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.Sequences;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.change.ChangeInserter;
@ -94,6 +96,7 @@ import org.junit.Test;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
@ -1240,6 +1243,35 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
assertQuery("starredby:" + user2);
}
@Test
public void byStar() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChangeWithStatus(repo, Change.Status.MERGED));
insert(repo, newChangeWithStatus(repo, Change.Status.MERGED));
gApi.accounts()
.self()
.setStars(change1.getId().toString(),
new StarsInput(new HashSet<>(Arrays.asList("red", "blue"))));
gApi.accounts()
.self()
.setStars(change2.getId().toString(),
new StarsInput(new HashSet<>(Arrays.asList(
StarredChangesUtil.DEFAULT_LABEL, "green", "blue"))));
// check labeled stars
assertQuery("star:red", change1);
assertQuery("star:blue", change2, change1);
assertQuery("has:stars", change2, change1);
// check default star
assertQuery("has:star", change2);
assertQuery("is:starred", change2);
assertQuery("starredby:self", change2);
assertQuery("star:" + StarredChangesUtil.DEFAULT_LABEL, change2);
}
@Test
public void byFrom() throws Exception {
TestRepository<Repo> repo = createProject("repo");