Merge "OAuthRealm should not overwrite custom AuthRequest parameters"
diff --git a/.gitignore b/.gitignore
index d4ef1b8..e7fe393 100644
--- a/.gitignore
+++ b/.gitignore
@@ -8,6 +8,7 @@
/test_site
/.idea
*.iml
+*.eml
*.sublime-*
/gerrit-package-plugins
/.buckconfig.local
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 4881f2a..b88f1a3 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2400,10 +2400,10 @@
[[index.threads]]index.threads::
+
-Number of threads to use for indexing in normal interactive operations. If set
-to 0, indexing will be done in the same thread as the interactive operation.
+Number of threads to use for indexing in normal interactive operations.
+
-Defaults to 0 if not set, or set to a negative value.
+If not set or set to a negative value, defaults to 1 plus half of the number of
+logical CPUs as returned by the JVM.
[[index.batchThreads]]index.batchThreads::
+
diff --git a/Documentation/dev-stars.txt b/Documentation/dev-stars.txt
new file mode 100644
index 0000000..1a98438
--- /dev/null
+++ b/Documentation/dev-stars.txt
@@ -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
+---------
diff --git a/Documentation/index.txt b/Documentation/index.txt
index d4836e5..1693c18 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -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]
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index 423d87d..8e2dae9 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -1573,16 +1573,19 @@
]
----
-[[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 @@
"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 @@
----
[[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 @@
----
[[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 @@
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 @@
|`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
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index da17806..b2cd1fe 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -4113,7 +4113,10 @@
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.
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index b2b3614..b04898e 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -247,25 +247,43 @@
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 @@
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'::
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 1955c39..1b3b9d9 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -31,6 +31,7 @@
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.RevisionApi;
+import com.google.gerrit.extensions.api.groups.GroupInput;
import com.google.gerrit.extensions.api.projects.BranchApi;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.api.projects.ProjectInput;
@@ -197,6 +198,9 @@
@Inject
protected ChangeNoteUtil changeNoteUtil;
+ @Inject
+ protected ChangeResource.Factory changeResourceFactory;
+
protected TestRepository<InMemoryRepository> testRepo;
protected GerritServer server;
protected TestAccount admin;
@@ -741,6 +745,19 @@
List<ChangeControl> ctls = changeFinder.find(
changeId, atrScope.get().getUser());
assertThat(ctls).hasSize(1);
- return new ChangeResource(ctls.get(0));
+ return changeResourceFactory.create(ctls.get(0));
+ }
+
+ protected String createGroup(String name) throws Exception {
+ return createGroup(name, "Administrators");
+ }
+
+ protected String createGroup(String name, String owner) throws Exception {
+ name = name(name);
+ GroupInput in = new GroupInput();
+ in.name = name;
+ in.ownerId = owner;
+ gApi.groups().create(in);
+ return name;
}
}
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AcceptanceTestRequestScope.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AcceptanceTestRequestScope.java
index 839b393..71d738c 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AcceptanceTestRequestScope.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AcceptanceTestRequestScope.java
@@ -182,6 +182,14 @@
return old;
}
+ public Context reopenDb() {
+ // Setting a new context with the same fields is enough to get the ReviewDb
+ // provider to reopen the database.
+ Context old = current.get();
+ return set(
+ new Context(old.schemaFactory, old.session, old.user, old.created));
+ }
+
/** Returns exactly one instance per command executed. */
static final Scope REQUEST = new Scope() {
@Override
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 0c2e0c6..53a47a3 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -23,20 +23,25 @@
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 @@
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
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 631b480..7dc4cda 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1174,6 +1174,6 @@
List<ChangeControl> ctls = changeFinder.find(
r.getChangeId(), atrScope.get().getUser());
assertThat(ctls).hasSize(1);
- return new ChangeResource(ctls.get(0));
+ return changeResourceFactory.create(ctls.get(0));
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/group/GroupsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/group/GroupsIT.java
index 6e2e761..9c064c7 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -560,19 +560,6 @@
return groupCache.get(new AccountGroup.NameKey(name));
}
- private String createGroup(String name) throws Exception {
- return createGroup(name, "Administrators");
- }
-
- private String createGroup(String name, String owner) throws Exception {
- name = name(name);
- GroupInput in = new GroupInput();
- in.name = name;
- in.ownerId = owner;
- gApi.groups().create(in);
- return name;
- }
-
private String createAccount(String name, String group) throws Exception {
name = name(name);
accounts.create(name, group);
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
index f4ad11e..274b7d7 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
@@ -36,6 +36,7 @@
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.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchLineCommentsUtil;
@@ -90,7 +91,7 @@
public void setUp() {
assume().that(NoteDbMode.readWrite()).isFalse();
TestTimeUtil.resetWithClockStep(1, TimeUnit.SECONDS);
- notesMigration.setAllEnabled(false);
+ setNotesMigration(false, false);
}
@After
@@ -98,6 +99,12 @@
TestTimeUtil.useSystemTime();
}
+ private void setNotesMigration(boolean writeChanges, boolean readChanges) {
+ notesMigration.setWriteChanges(writeChanges);
+ notesMigration.setReadChanges(readChanges);
+ db = atrScope.reopenDb().getReviewDbProvider().get();
+ }
+
@Test
public void changeFields() throws Exception {
PushOneCommit.Result r = createChange();
@@ -199,8 +206,7 @@
checker.rebuildAndCheckChanges(id);
- notesMigration.setWriteChanges(true);
- notesMigration.setReadChanges(true);
+ setNotesMigration(true, true);
ChangeNotes notes = notesFactory.create(db, project, id);
Map<String, PatchSet.Id> psIds = new HashMap<>();
@@ -222,7 +228,7 @@
Change.Id id = r.getPatchSetId().getParentKey();
checker.assertNoChangeRef(project, id);
- notesMigration.setWriteChanges(true);
+ setNotesMigration(true, false);
gApi.changes().id(id.get()).topic(name("a-topic"));
// First write doesn't create the ref, but rebuilding works.
@@ -250,7 +256,7 @@
public void rebuildViaRestApi() throws Exception {
PushOneCommit.Result r = createChange();
Change.Id id = r.getPatchSetId().getParentKey();
- notesMigration.setWriteChanges(true);
+ setNotesMigration(true, false);
checker.assertNoChangeRef(project, id);
rebuildHandler.apply(
@@ -264,7 +270,7 @@
PushOneCommit.Result r1 = createChange();
Change.Id id1 = r1.getPatchSetId().getParentKey();
- notesMigration.setWriteChanges(true);
+ setNotesMigration(true, false);
gApi.changes().id(id1.get()).topic(name("a-topic"));
PushOneCommit.Result r2 = createChange();
Change.Id id2 = r2.getPatchSetId().getParentKey();
@@ -281,7 +287,7 @@
@Test
public void noteDbChangeState() throws Exception {
- notesMigration.setAllEnabled(true);
+ setNotesMigration(true, true);
PushOneCommit.Result r = createChange();
Change.Id id = r.getPatchSetId().getParentKey();
@@ -316,20 +322,20 @@
@Test
public void rebuildAutomaticallyWhenChangeOutOfDate() throws Exception {
- notesMigration.setAllEnabled(true);
+ setNotesMigration(true, true);
PushOneCommit.Result r = createChange();
Change.Id id = r.getPatchSetId().getParentKey();
assertChangeUpToDate(true, id);
// Make a ReviewDb change behind NoteDb's back and ensure it's detected.
- notesMigration.setAllEnabled(false);
+ setNotesMigration(false, false);
gApi.changes().id(id.get()).topic(name("a-topic"));
setInvalidNoteDbState(id);
assertChangeUpToDate(false, id);
// On next NoteDb read, the change is transparently rebuilt.
- notesMigration.setAllEnabled(true);
+ setNotesMigration(true, true);
assertThat(gApi.changes().id(id.get()).info().topic)
.isEqualTo(name("a-topic"));
assertChangeUpToDate(true, id);
@@ -343,7 +349,7 @@
@Test
public void rebuildAutomaticallyWhenDraftsOutOfDate() throws Exception {
- notesMigration.setAllEnabled(true);
+ setNotesMigration(true, true);
setApiUser(user);
PushOneCommit.Result r = createChange();
@@ -352,13 +358,13 @@
assertDraftsUpToDate(true, id, user);
// Make a ReviewDb change behind NoteDb's back and ensure it's detected.
- notesMigration.setAllEnabled(false);
+ setNotesMigration(false, false);
putDraft(user, id, 1, "comment");
setInvalidNoteDbState(id);
assertDraftsUpToDate(false, id, user);
// On next NoteDb read, the drafts are transparently rebuilt.
- notesMigration.setAllEnabled(true);
+ setNotesMigration(true, true);
assertThat(gApi.changes().id(id.get()).current().drafts())
.containsKey(PushOneCommit.FILE_NAME);
assertDraftsUpToDate(true, id, user);
@@ -412,8 +418,7 @@
checker.rebuildAndCheckChanges(id);
- notesMigration.setWriteChanges(true);
- notesMigration.setReadChanges(true);
+ setNotesMigration(true, true);
// Rebuild and check was successful, but NoteDb doesn't support storing an
// empty topic, so it comes out as null.
@@ -480,7 +485,7 @@
checker.rebuildAndCheckChanges(id);
- notesMigration.setAllEnabled(true);
+ setNotesMigration(true, true);
ChangeNotes notes = notesFactory.create(db, project, id);
Change nc = notes.getChange();
assertThat(nc.getSubject()).isEqualTo(c.getSubject());
@@ -503,7 +508,7 @@
checker.rebuildAndCheckChanges(id);
- notesMigration.setAllEnabled(true);
+ setNotesMigration(true, true);
ChangeNotes notes = notesFactory.create(db, project, id);
assertThat(notes.getPatchSets().keySet()).containsExactly(psId);
}
@@ -527,11 +532,52 @@
checker.rebuildAndCheckChanges(id);
- notesMigration.setAllEnabled(true);
+ setNotesMigration(true, true);
ChangeNotes notes = notesFactory.create(db, project, id);
assertThat(notes.getComments()).isEmpty();
}
+ @Test
+ public void skipPatchSetsGreaterThanCurrentPatchSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+ Change change = r.getChange().change();
+ Change.Id id = change.getId();
+
+ PatchSet badPs =
+ new PatchSet(new PatchSet.Id(id, change.currentPatchSetId().get() + 1));
+ badPs.setCreatedOn(TimeUtil.nowTs());
+ badPs.setUploader(new Account.Id(12345));
+ badPs.setRevision(new RevId("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"));
+ db.patchSets().insert(Collections.singleton(badPs));
+ indexer.index(db, change.getProject(), id);
+
+ checker.rebuildAndCheckChanges(id);
+
+ setNotesMigration(true, true);
+ ChangeNotes notes = notesFactory.create(db, project, id);
+ assertThat(notes.getPatchSets().keySet())
+ .containsExactly(change.currentPatchSetId());
+ }
+
+ @Test
+ public void leadingSpacesInSubject() throws Exception {
+ String subj = " " + PushOneCommit.SUBJECT;
+ PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo,
+ subj, PushOneCommit.FILE_NAME, PushOneCommit.FILE_CONTENT);
+ PushOneCommit.Result r = push.to("refs/for/master");
+ r.assertOkStatus();
+ Change change = r.getChange().change();
+ assertThat(change.getSubject()).isEqualTo(subj);
+ Change.Id id = r.getPatchSetId().getParentKey();
+
+ checker.rebuildAndCheckChanges(id);
+
+ setNotesMigration(true, true);
+ ChangeNotes notes = notesFactory.create(db, project, id);
+ assertThat(notes.getChange().getSubject()).isNotEqualTo(subj);
+ assertThat(notes.getChange().getSubject()).isEqualTo(PushOneCommit.SUBJECT);
+ }
+
private void setInvalidNoteDbState(Change.Id id) throws Exception {
ReviewDb db = unwrapDb();
Change c = db.changes().get(id);
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/PageLinks.java b/gerrit-common/src/main/java/com/google/gerrit/common/PageLinks.java
index 2fce1cd..3957a30 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/PageLinks.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/PageLinks.java
@@ -107,11 +107,6 @@
return QUERY + KeyUtil.encode(query);
}
- public static String toChangeQuery(String query, String start) {
- int s = Integer.parseInt(start);
- return QUERY + KeyUtil.encode(query) + (s > 0 ? "," + s : "");
- }
-
public static String toProjectDashboard(Project.NameKey name, String id) {
return PROJECTS + name.get() + DASHBOARDS + id;
}
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScript.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScript.java
index 5167fe6..5777396 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScript.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScript.java
@@ -35,30 +35,30 @@
FILE, SYMLINK, GITLINK
}
- protected Change.Key changeId;
- protected ChangeType changeType;
- protected String oldName;
- protected String newName;
- protected FileMode oldMode;
- protected FileMode newMode;
- protected List<String> header;
- protected DiffPreferencesInfo diffPrefs;
- protected SparseFileContent a;
- protected SparseFileContent b;
- protected List<Edit> edits;
- protected DisplayMethod displayMethodA;
- protected DisplayMethod displayMethodB;
- protected transient String mimeTypeA;
- protected transient String mimeTypeB;
- protected CommentDetail comments;
- protected List<Patch> history;
- protected boolean hugeFile;
- protected boolean intralineDifference;
- protected boolean intralineFailure;
- protected boolean intralineTimeout;
- protected boolean binary;
- protected transient String commitIdA;
- protected transient String commitIdB;
+ private Change.Key changeId;
+ private ChangeType changeType;
+ private String oldName;
+ private String newName;
+ private FileMode oldMode;
+ private FileMode newMode;
+ private List<String> header;
+ private DiffPreferencesInfo diffPrefs;
+ private SparseFileContent a;
+ private SparseFileContent b;
+ private List<Edit> edits;
+ private DisplayMethod displayMethodA;
+ private DisplayMethod displayMethodB;
+ private transient String mimeTypeA;
+ private transient String mimeTypeB;
+ private CommentDetail comments;
+ private List<Patch> history;
+ private boolean hugeFile;
+ private boolean intralineDifference;
+ private boolean intralineFailure;
+ private boolean intralineTimeout;
+ private boolean binary;
+ private transient String commitIdA;
+ private transient String commitIdB;
public PatchScript(final Change.Key ck, final ChangeType ct, final String on,
final String nn, final FileMode om, final FileMode nm,
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/errors/InactiveAccountException.java b/gerrit-common/src/main/java/com/google/gerrit/common/errors/InactiveAccountException.java
deleted file mode 100644
index 6ae5eb6..0000000
--- a/gerrit-common/src/main/java/com/google/gerrit/common/errors/InactiveAccountException.java
+++ /dev/null
@@ -1,26 +0,0 @@
-// Copyright (C) 2010 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.common.errors;
-
-/** Error indicating the account is currently inactive. */
-public class InactiveAccountException extends Exception {
- private static final long serialVersionUID = 1L;
-
- public static final String MESSAGE = "Account Inactive: ";
-
- public InactiveAccountException(String who) {
- super(MESSAGE + who);
- }
-}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/AccountApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/AccountApi.java
index 2105b8c..d17beca 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/AccountApi.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/AccountApi.java
@@ -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 java.util.List;
import java.util.Map;
+import java.util.SortedSet;
public interface AccountApi {
AccountInfo get() throws RestApiException;
@@ -50,8 +53,12 @@
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 @@
}
@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();
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/StarsInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/StarsInput.java
new file mode 100644
index 0000000..d3dff98
--- /dev/null
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/StarsInput.java
@@ -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;
+ }
+}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthToken.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthToken.java
index 901951e..99b2cfa 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthToken.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthToken.java
@@ -14,17 +14,33 @@
package com.google.gerrit.extensions.auth.oauth;
+import java.io.Serializable;
+
/* OAuth token */
-public class OAuthToken {
+public class OAuthToken implements Serializable {
+
+ private static final long serialVersionUID = 1L;
private final String token;
private final String secret;
private final String raw;
+ /**
+ * Time of expiration of this token, or {@code Long#MAX_VALUE} if this
+ * token never expires, or time of expiration is unknown.
+ */
+ private final long expiresAt;
+
public OAuthToken(String token, String secret, String raw) {
+ this(token, secret, raw, Long.MAX_VALUE);
+ }
+
+ public OAuthToken(String token, String secret, String raw,
+ long expiresAt) {
this.token = token;
this.secret = secret;
this.raw = raw;
+ this.expiresAt = expiresAt;
}
public String getToken() {
@@ -38,4 +54,12 @@
public String getRaw() {
return raw;
}
+
+ public long getExpiresAt() {
+ return expiresAt;
+ }
+
+ public boolean isExpired() {
+ return System.currentTimeMillis() > expiresAt;
+ }
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthTokenEncrypter.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthTokenEncrypter.java
new file mode 100644
index 0000000..b2f4262
--- /dev/null
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthTokenEncrypter.java
@@ -0,0 +1,35 @@
+// 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.auth.oauth;
+
+import com.google.gerrit.extensions.annotations.ExtensionPoint;
+
+@ExtensionPoint
+public interface OAuthTokenEncrypter {
+
+ /**
+ * Encrypts the secret parts of the given OAuth access token.
+ *
+ * @param unencrypted a raw OAuth access token.
+ */
+ OAuthToken encrypt(OAuthToken unencrypted);
+
+ /**
+ * Decrypts the secret parts of the given OAuth access token.
+ *
+ * @param encrypted an encryppted OAuth access token.
+ */
+ OAuthToken decrypt(OAuthToken encrypted);
+}
diff --git a/gerrit-gwtexpui/src/main/java/com/google/gwtexpui/globalkey/client/NpTextArea.java b/gerrit-gwtexpui/src/main/java/com/google/gwtexpui/globalkey/client/NpTextArea.java
index 38a488e..fd0da74 100644
--- a/gerrit-gwtexpui/src/main/java/com/google/gwtexpui/globalkey/client/NpTextArea.java
+++ b/gerrit-gwtexpui/src/main/java/com/google/gwtexpui/globalkey/client/NpTextArea.java
@@ -14,7 +14,6 @@
package com.google.gwtexpui.globalkey.client;
-import com.google.gwt.dom.client.Element;
import com.google.gwt.user.client.ui.TextArea;
public class NpTextArea extends TextArea {
@@ -22,11 +21,6 @@
addKeyPressHandler(GlobalKey.STOP_PROPAGATION);
}
- public NpTextArea(final Element element) {
- super(element);
- addKeyPressHandler(GlobalKey.STOP_PROPAGATION);
- }
-
public void setSpellCheck(boolean spell) {
getElement().setPropertyBoolean("spellcheck", spell);
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/AvatarImage.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/AvatarImage.java
index b2da7b5..4ca1721 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/AvatarImage.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/AvatarImage.java
@@ -46,18 +46,6 @@
* @param size A requested size. Note that the size can be ignored depending
* on the avatar provider. A size <= 0 indicates to let the provider
* decide a default size.
- */
- public AvatarImage(AccountInfo account, int size) {
- this(account, size, true);
- }
-
- /**
- * An avatar image for the given account using the requested size.
- *
- * @param account The account in which we are interested
- * @param size A requested size. Note that the size can be ignored depending
- * on the avatar provider. A size <= 0 indicates to let the provider
- * decide a default size.
* @param addPopup show avatar popup with user info on hovering over the
* avatar image
*/
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ErrorDialog.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ErrorDialog.java
index a5c5659..87cb4b3 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ErrorDialog.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ErrorDialog.java
@@ -27,7 +27,6 @@
import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwt.user.client.ui.Label;
import com.google.gwt.user.client.ui.PopupPanel;
-import com.google.gwt.user.client.ui.Widget;
import com.google.gwtexpui.safehtml.client.SafeHtml;
import com.google.gwtjsonrpc.client.RemoteJsonException;
@@ -96,11 +95,6 @@
body.add(message.toBlockWidget());
}
- public ErrorDialog(final Widget w) {
- this();
- body.add(w);
- }
-
/** Create a dialog box to nicely format an exception. */
public ErrorDialog(final Throwable what) {
this();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java
index 920c285..54c5b92 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java
@@ -104,6 +104,8 @@
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");
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Hashtags.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Hashtags.java
index efacbe3..44aae25 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Hashtags.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Hashtags.java
@@ -265,12 +265,4 @@
protected PostInput() {
}
}
-
- public static class Result extends JavaScriptObject {
- public final native JsArrayString hashtags() /*-{ return this.hashtags; }-*/;
- public final native String error() /*-{ return this.error; }-*/;
-
- protected Result() {
- }
- }
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffScreen.java
index 3d14ebd..98f4eb5 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffScreen.java
@@ -79,8 +79,8 @@
/** Base class for SideBySide and Unified */
abstract class DiffScreen extends Screen {
- static final KeyMap RENDER_ENTIRE_FILE_KEYMAP = KeyMap.create()
- .propagate("Ctrl-F");
+ private static final KeyMap RENDER_ENTIRE_FILE_KEYMAP = KeyMap.create()
+ .propagate("Ctrl-F").propagate("Ctrl-G").propagate("Shift-Ctrl-G");
enum FileSize {
SMALL(0),
@@ -269,10 +269,6 @@
return keysAction;
}
- KeyCommandSet getKeysComment() {
- return keysComment;
- }
-
@Override
protected void onUnload() {
super.onUnload();
@@ -317,7 +313,6 @@
.on("']'", header.navigate(Direction.NEXT))
.on("R", header.toggleReviewed())
.on("O", getCommentManager().toggleOpenBox(cm))
- .on("Enter", getCommentManager().toggleOpenBox(cm))
.on("N", maybeNextVimSearch(cm))
.on("Ctrl-Alt-E", openEditScreen(cm))
.on("P", getChunkManager().diffChunkNav(cm, Direction.PREV))
@@ -367,7 +362,34 @@
.on("Ctrl-F", new Runnable() {
@Override
public void run() {
- cm.vim().handleKey("/");
+ cm.execCommand("find");
+ }
+ })
+ .on("Ctrl-G", new Runnable() {
+ @Override
+ public void run() {
+ cm.execCommand("findNext");
+ }
+ })
+ .on("Enter", maybeNextCmSearch(cm))
+ .on("Shift-Ctrl-G", new Runnable() {
+ @Override
+ public void run() {
+ cm.execCommand("findPrev");
+ }
+ })
+ .on("Shift-Enter", new Runnable() {
+ @Override
+ public void run() {
+ cm.execCommand("findPrev");
+ }
+ })
+ .on("Esc", new Runnable() {
+ @Override
+ public void run() {
+ cm.setCursor(cm.getCursor());
+ cm.execCommand("clearSearch");
+ cm.vim().handleEx("nohlsearch");
}
})
.on("Ctrl-A", new Runnable() {
@@ -596,7 +618,7 @@
}
}
- void toggleShowIntraline() {
+ private void toggleShowIntraline() {
prefs.intralineDifference(!prefs.intralineDifference());
setShowIntraline(prefs.intralineDifference());
prefsAction.update();
@@ -738,7 +760,7 @@
abstract void operation(final Runnable apply);
- Runnable upToChange(final boolean openReplyBox) {
+ private Runnable upToChange(final boolean openReplyBox) {
return new Runnable() {
@Override
public void run() {
@@ -760,7 +782,7 @@
};
}
- Runnable maybePrevVimSearch(final CodeMirror cm) {
+ private Runnable maybePrevVimSearch(final CodeMirror cm) {
return new Runnable() {
@Override
public void run() {
@@ -773,7 +795,7 @@
};
}
- Runnable maybeNextVimSearch(final CodeMirror cm) {
+ private Runnable maybeNextVimSearch(final CodeMirror cm) {
return new Runnable() {
@Override
public void run() {
@@ -786,6 +808,20 @@
};
}
+ Runnable maybeNextCmSearch(final CodeMirror cm) {
+ return new Runnable() {
+ @Override
+ public void run() {
+ if (cm.hasSearchHighlight()) {
+ cm.execCommand("findNext");
+ } else {
+ cm.execCommand("clearSearch");
+ getCommentManager().toggleOpenBox(cm).run();
+ }
+ }
+ };
+ }
+
boolean renderEntireFile() {
return prefs.renderEntireFile() && canRenderEntireFile(prefs);
}
@@ -894,7 +930,7 @@
});
}
- static FileSize bucketFileSize(DiffInfo diff) {
+ private static FileSize bucketFileSize(DiffInfo diff) {
FileMeta a = diff.metaA();
FileMeta b = diff.metaB();
FileSize[] sizes = FileSize.values();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java
index 35d6cac..006788f 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java
@@ -369,10 +369,6 @@
return new CodeMirror[] {cm};
}
- CodeMirror getCm() {
- return cm;
- }
-
@Override
UnifiedTable getDiffTable() {
return diffTable;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/GerritCallback.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/GerritCallback.java
index daac7cf..cd44fac 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/GerritCallback.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/GerritCallback.java
@@ -17,7 +17,6 @@
import com.google.gerrit.client.ErrorDialog;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.NotSignedInDialog;
-import com.google.gerrit.common.errors.InactiveAccountException;
import com.google.gerrit.common.errors.NameAlreadyUsedException;
import com.google.gerrit.common.errors.NoSuchAccountException;
import com.google.gerrit.common.errors.NoSuchEntityException;
@@ -42,9 +41,6 @@
new NotSignedInDialog().center();
} else if (isNoSuchEntity(caught)) {
new ErrorDialog(Gerrit.C.notFoundBody()).center();
- } else if (isInactiveAccount(caught)) {
- new ErrorDialog(Gerrit.C.inactiveAccountBody()).center();
-
} else if (isNoSuchAccount(caught)) {
final String msg = caught.getMessage();
final String who = msg.substring(NoSuchAccountException.MESSAGE.length());
@@ -97,11 +93,6 @@
&& caught.getMessage().equals(NoSuchEntityException.MESSAGE));
}
- protected static boolean isInactiveAccount(final Throwable caught) {
- return caught instanceof RemoteJsonException
- && caught.getMessage().startsWith(InactiveAccountException.MESSAGE);
- }
-
protected static boolean isNoSuchAccount(final Throwable caught) {
return caught instanceof RemoteJsonException
&& caught.getMessage().startsWith(NoSuchAccountException.MESSAGE);
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ListenableValue.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ListenableValue.java
deleted file mode 100644
index c8bb12e..0000000
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ListenableValue.java
+++ /dev/null
@@ -1,59 +0,0 @@
-// Copyright (C) 2010 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.client.ui;
-
-import com.google.gwt.event.logical.shared.HasValueChangeHandlers;
-import com.google.gwt.event.logical.shared.ValueChangeEvent;
-import com.google.gwt.event.logical.shared.ValueChangeHandler;
-import com.google.gwt.event.shared.GwtEvent;
-import com.google.gwt.event.shared.HandlerManager;
-import com.google.gwt.event.shared.HandlerRegistration;
-
-
-public class ListenableValue<T> implements HasValueChangeHandlers<T> {
-
- private HandlerManager manager = new HandlerManager(this);
-
- private T value;
-
- public T get() {
- return value;
- }
-
- public void set(final T value) {
- this.value = value;
- fireEvent(new ValueChangeEvent<T>(value) {});
- }
-
- @Override
- public void fireEvent(GwtEvent<?> event) {
- manager.fireEvent(event);
- }
-
- @Override
- public HandlerRegistration addValueChangeHandler(
- ValueChangeHandler<T> handler) {
- return manager.addHandler(ValueChangeEvent.getType(), handler);
- }
-
- public int getHandlerCount() {
- return manager.getHandlerCount(ValueChangeEvent.getType());
- }
-
- public ValueChangeHandler<?> getHandler(int index) {
- return manager.getHandler(ValueChangeEvent.getType(), index);
- }
-
-}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/NpIntTextBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/NpIntTextBox.java
index 0933153..3994381 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/NpIntTextBox.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/NpIntTextBox.java
@@ -14,7 +14,6 @@
package com.google.gerrit.client.ui;
-import com.google.gwt.dom.client.Element;
import com.google.gwt.event.dom.client.KeyCodes;
import com.google.gwt.event.dom.client.KeyDownEvent;
import com.google.gwt.event.dom.client.KeyDownHandler;
@@ -31,11 +30,6 @@
init();
}
- public NpIntTextBox(Element element) {
- super(element);
- init();
- }
-
private void init() {
addKeyDownHandler(new KeyDownHandler() {
@Override
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/PatchLink.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/PatchLink.java
deleted file mode 100644
index 09244c9..0000000
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/PatchLink.java
+++ /dev/null
@@ -1,37 +0,0 @@
-// Copyright (C) 2008 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.client.ui;
-
-import com.google.gerrit.client.Dispatcher;
-import com.google.gerrit.reviewdb.client.Patch;
-import com.google.gerrit.reviewdb.client.PatchSet;
-
-public class PatchLink extends InlineHyperlink {
- private PatchLink(String text, String historyToken) {
- super(text, historyToken);
- }
-
- public static class SideBySide extends PatchLink {
- public SideBySide(String text, PatchSet.Id base, Patch.Key id) {
- super(text, Dispatcher.toSideBySide(base, id));
- }
- }
-
- public static class Unified extends PatchLink {
- public Unified(String text, PatchSet.Id base, Patch.Key id) {
- super(text, Dispatcher.toUnified(base, id));
- }
- }
-}
diff --git a/gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java b/gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java
index eccde6f..be914f3 100644
--- a/gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java
+++ b/gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java
@@ -395,6 +395,10 @@
return this.setGutterMarker(line, gutterId, value);
}-*/;
+ public final native boolean hasSearchHighlight() /*-{
+ return this.state.search && !!this.state.search.query;
+ }-*/;
+
protected CodeMirror() {
}
diff --git a/gerrit-gwtui/src/main/java/net/codemirror/lib/Vim.java b/gerrit-gwtui/src/main/java/net/codemirror/lib/Vim.java
index c3d58c9ba..84b4a6a 100644
--- a/gerrit-gwtui/src/main/java/net/codemirror/lib/Vim.java
+++ b/gerrit-gwtui/src/main/java/net/codemirror/lib/Vim.java
@@ -57,6 +57,10 @@
$wnd.CodeMirror.Vim.handleKey(this, key)
}-*/;
+ public final native void handleEx(String exCommand) /*-{
+ $wnd.CodeMirror.Vim.handleEx(this, exCommand);
+ }-*/;
+
public final native boolean hasSearchHighlight() /*-{
var v = this.state.vim;
return v && v.searchState_ && !!v.searchState_.getOverlay();
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/UrlModule.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/UrlModule.java
index 3083edd..45e5615 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/UrlModule.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/UrlModule.java
@@ -87,6 +87,9 @@
serve("/watched").with(query("is:watched status:open"));
serve("/starred").with(query("is:starred"));
+ // Forward PolyGerrit URLs to their respective GWT equivalents.
+ serveRegex("^/(c|q|x|admin|dashboard|settings)/(.*)").with(gerritUrl());
+
serveRegex("^/settings/?$").with(screen(PageLinks.SETTINGS));
serveRegex("^/register/?$").with(screen(PageLinks.REGISTER + "/"));
serveRegex("^/([1-9][0-9]*)/?$").with(directChangeById());
@@ -117,6 +120,18 @@
});
}
+ private Key<HttpServlet> gerritUrl() {
+ return key(new HttpServlet() {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doGet(final HttpServletRequest req,
+ final HttpServletResponse rsp) throws IOException {
+ toGerrit(req.getRequestURI(), req, rsp);
+ }
+ });
+ }
+
private Key<HttpServlet> screen(final String target) {
return key(new HttpServlet() {
private static final long serialVersionUID = 1L;
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 275fa48..bdc7189 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -24,8 +24,10 @@
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.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 @@
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 @@
if (fields.contains(STARREDBY_FIELD)) {
decodeStarredBy(doc, cd);
}
+ if (fields.contains(STAR_FIELD)) {
+ decodeStar(doc, cd);
+ }
return cd;
}
@@ -482,6 +490,7 @@
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 @@
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);
diff --git a/gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java b/gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java
index d24c8a0..3e65912 100644
--- a/gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java
+++ b/gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java
@@ -31,6 +31,7 @@
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.AuthResult;
+import com.google.gerrit.server.auth.oauth.OAuthTokenCache;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -58,8 +59,8 @@
private final Provider<IdentifiedUser> identifiedUser;
private final AccountManager accountManager;
private final CanonicalWebUrl urlProvider;
+ private final OAuthTokenCache tokenCache;
private OAuthServiceProvider serviceProvider;
- private OAuthToken token;
private OAuthUserInfo user;
private String redirectToken;
private boolean linkMode;
@@ -68,16 +69,18 @@
OAuthSession(DynamicItem<WebSession> webSession,
Provider<IdentifiedUser> identifiedUser,
AccountManager accountManager,
- CanonicalWebUrl urlProvider) {
+ CanonicalWebUrl urlProvider,
+ OAuthTokenCache tokenCache) {
this.state = generateRandomState();
this.identifiedUser = identifiedUser;
this.webSession = webSession;
this.accountManager = accountManager;
this.urlProvider = urlProvider;
+ this.tokenCache = tokenCache;
}
boolean isLoggedIn() {
- return token != null && user != null;
+ return tokenCache.has(user);
}
boolean isOAuthFinal(HttpServletRequest request) {
@@ -95,9 +98,12 @@
}
log.debug("Login-Retrieve-User " + this);
- token = oauth.getAccessToken(new OAuthVerifier(request.getParameter("code")));
-
+ OAuthToken token = oauth.getAccessToken(
+ new OAuthVerifier(request.getParameter("code")));
user = oauth.getUserInfo(token);
+ if (user != null && token != null) {
+ tokenCache.put(user, token);
+ }
if (isLoggedIn()) {
log.debug("Login-SUCCESS " + this);
@@ -211,7 +217,7 @@
}
void logout() {
- token = null;
+ tokenCache.remove(user);
user = null;
redirectToken = null;
serviceProvider = null;
@@ -243,7 +249,8 @@
@Override
public String toString() {
- return "OAuthSession [token=" + token + ", user=" + user + "]";
+ return "OAuthSession [token=" + tokenCache.get(user) + ", user=" + user
+ + "]";
}
public void setServiceProvider(OAuthServiceProvider provider) {
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNoteDb.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNoteDb.java
index aa8b21f..047789e 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNoteDb.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNoteDb.java
@@ -28,6 +28,7 @@
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
+import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.lifecycle.LifecycleManager;
@@ -38,6 +39,7 @@
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.change.ChangeResource;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -50,7 +52,6 @@
import com.google.gerrit.server.schema.DisabledChangesReviewDbWrapper;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
-import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Injector;
@@ -206,7 +207,7 @@
}
private Injector createSysInjector() {
- return dbInjector.createChildInjector(new AbstractModule() {
+ return dbInjector.createChildInjector(new FactoryModule() {
@Override
public void configure() {
install(dbInjector.getInstance(BatchProgramModule.class));
@@ -214,6 +215,7 @@
DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(
ReindexAfterUpdate.class);
install(new DummyIndexModule());
+ factory(ChangeResource.Factory.class);
}
});
}
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java
index 0e647b9..9cd7c6a 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java
@@ -18,11 +18,13 @@
import static com.google.gerrit.server.schema.DataSourceProvider.Context.MULTI_USER;
import com.google.gerrit.common.Die;
+import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.lifecycle.LifecycleManager;
import com.google.gerrit.lucene.LuceneIndexModule;
import com.google.gerrit.pgm.util.BatchProgramModule;
import com.google.gerrit.pgm.util.SiteProgram;
import com.google.gerrit.pgm.util.ThreadLimiter;
+import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.ScanningChangeCacheImpl;
import com.google.gerrit.server.index.Index;
@@ -129,6 +131,12 @@
// will have just deleted the old (possibly corrupt) index.
modules.add(ScanningChangeCacheImpl.module());
modules.add(dbInjector.getInstance(BatchProgramModule.class));
+ modules.add(new FactoryModule() {
+ @Override
+ protected void configure() {
+ factory(ChangeResource.Factory.class);
+ }
+ });
return dbInjector.createChildInjector(modules);
}
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SecureStoreException.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SecureStoreException.java
deleted file mode 100644
index 9693399..0000000
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SecureStoreException.java
+++ /dev/null
@@ -1,27 +0,0 @@
-// Copyright (C) 2013 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.pgm.util;
-
-public class SecureStoreException extends RuntimeException {
- private static final long serialVersionUID = 5581700510568485065L;
-
- SecureStoreException(String msg) {
- super(msg);
- }
-
- SecureStoreException(String msg, Exception e) {
- super(msg, e);
- }
-}
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbUtil.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbUtil.java
index 2460d57..5d782dd 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbUtil.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbUtil.java
@@ -21,7 +21,7 @@
/** 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) {
diff --git a/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/ChangeTest.java b/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/ChangeTest.java
index 47f409a..cf2d289 100644
--- a/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/ChangeTest.java
+++ b/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/ChangeTest.java
@@ -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 @@
.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 @@
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));
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java
index 6a8600f..f1242b9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java
@@ -79,6 +79,7 @@
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. */
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java
index a081197..f6fae8c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java
@@ -348,6 +348,7 @@
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 @@
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 {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
index 72ee0ef..77064fe 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
@@ -16,14 +16,18 @@
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.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 @@
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 @@
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 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 @@
}
}
- 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 @@
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 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 @@
}
}
- 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 @@
invalidLabels.add(label);
}
}
- return invalidLabels;
+ if (!invalidLabels.isEmpty()) {
+ throw IllegalLabelException.invalidLabels(invalidLabels);
+ }
}
private void updateLabels(Repository repo, String refName,
@@ -317,4 +425,29 @@
}
}
}
+
+ 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()));
+ }
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
index e801eb5..596c440 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
@@ -344,8 +344,6 @@
public AuthResult link(Account.Id to, AuthRequest who)
throws AccountException, OrmException {
try (ReviewDb db = schema.open()) {
- who = realm.link(db, to, who);
-
AccountExternalId.Key key = id(who);
AccountExternalId extId = getAccountExternalId(db, key);
if (extId != null) {
@@ -435,8 +433,6 @@
public AuthResult unlink(Account.Id from, AuthRequest who)
throws AccountException, OrmException {
try (ReviewDb db = schema.open()) {
- who = realm.unlink(db, from, who);
-
AccountExternalId.Key key = id(who);
AccountExternalId extId = getAccountExternalId(db, key);
if (extId != null) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResource.java
index 75e5ae5..8bebf52 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResource.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResource.java
@@ -22,6 +22,8 @@
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 @@
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;
+ }
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/DefaultRealm.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/DefaultRealm.java
index 96dc4b8..5b7a9a2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/DefaultRealm.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/DefaultRealm.java
@@ -17,7 +17,6 @@
import com.google.common.base.Strings;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AuthType;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.config.AuthConfig;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -75,16 +74,6 @@
}
@Override
- public AuthRequest link(ReviewDb db, Account.Id to, AuthRequest who) {
- return who;
- }
-
- @Override
- public AuthRequest unlink(ReviewDb db, Account.Id from, AuthRequest who) {
- return who;
- }
-
- @Override
public void onCreateAccount(final AuthRequest who, final Account account) {
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/FakeRealm.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/FakeRealm.java
index f8d6bd1..d3b938f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/FakeRealm.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/FakeRealm.java
@@ -16,7 +16,6 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Account.FieldName;
-import com.google.gerrit.reviewdb.server.ReviewDb;
/** Fake implementation of {@link Realm} that does not communicate. */
public class FakeRealm extends AbstractRealm {
@@ -31,16 +30,6 @@
}
@Override
- public AuthRequest link(ReviewDb db, Account.Id to, AuthRequest who) {
- return who;
- }
-
- @Override
- public AuthRequest unlink(ReviewDb db, Account.Id to, AuthRequest who) {
- return who;
- }
-
- @Override
public void onCreateAccount(AuthRequest who, Account account) {
// Do nothing.
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupDetailFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupDetailFactory.java
index 713cbda..94feb7d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupDetailFactory.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupDetailFactory.java
@@ -23,7 +23,7 @@
import com.google.gerrit.reviewdb.client.AccountGroupById;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.group.GroupInfoCacheFactory;
+import com.google.gerrit.server.group.GroupInfoCache;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -44,18 +44,19 @@
private final GroupCache groupCache;
private final GroupBackend groupBackend;
private final AccountInfoCacheFactory aic;
- private final GroupInfoCacheFactory gic;
+ private final GroupInfoCache gic;
private final AccountGroup.Id groupId;
private GroupControl control;
@Inject
- GroupDetailFactory(final ReviewDb db,
- final GroupControl.Factory groupControl, final GroupCache groupCache,
- final GroupBackend groupBackend,
- final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
- final GroupInfoCacheFactory.Factory groupInfoCacheFactory,
- @Assisted final AccountGroup.Id groupId) {
+ GroupDetailFactory(ReviewDb db,
+ GroupControl.Factory groupControl,
+ GroupCache groupCache,
+ GroupBackend groupBackend,
+ AccountInfoCacheFactory.Factory accountInfoCacheFactory,
+ GroupInfoCache.Factory groupInfoCacheFactory,
+ @Assisted AccountGroup.Id groupId) {
this.db = db;
this.groupControl = groupControl;
this.groupCache = groupCache;
@@ -69,8 +70,8 @@
@Override
public GroupDetail call() throws OrmException, NoSuchGroupException {
control = groupControl.validateFor(groupId);
- final AccountGroup group = groupCache.get(groupId);
- final GroupDetail detail = new GroupDetail();
+ AccountGroup group = groupCache.get(groupId);
+ GroupDetail detail = new GroupDetail();
detail.setGroup(group);
GroupDescription.Basic ownerGroup = groupBackend.get(group.getOwnerGroupUUID());
if (ownerGroup != null) {
@@ -85,7 +86,7 @@
private List<AccountGroupMember> loadMembers() throws OrmException {
List<AccountGroupMember> members = new ArrayList<>();
- for (final AccountGroupMember m : db.accountGroupMembers().byGroup(groupId)) {
+ for (AccountGroupMember m : db.accountGroupMembers().byGroup(groupId)) {
if (control.canSeeMember(m.getAccountId())) {
aic.want(m.getAccountId());
members.add(m);
@@ -94,10 +95,9 @@
Collections.sort(members, new Comparator<AccountGroupMember>() {
@Override
- public int compare(final AccountGroupMember o1,
- final AccountGroupMember o2) {
- final Account a = aic.get(o1.getAccountId());
- final Account b = aic.get(o2.getAccountId());
+ public int compare(AccountGroupMember o1, AccountGroupMember o2) {
+ Account a = aic.get(o1.getAccountId());
+ Account b = aic.get(o2.getAccountId());
return n(a).compareTo(n(b));
}
@@ -121,7 +121,7 @@
private List<AccountGroupById> loadIncludes() throws OrmException {
List<AccountGroupById> groups = new ArrayList<>();
- for (final AccountGroupById m : db.accountGroupById().byGroup(groupId)) {
+ for (AccountGroupById m : db.accountGroupById().byGroup(groupId)) {
if (control.canSeeGroup()) {
gic.want(m.getIncludeUUID());
groups.add(m);
@@ -130,8 +130,7 @@
Collections.sort(groups, new Comparator<AccountGroupById>() {
@Override
- public int compare(final AccountGroupById o1,
- final AccountGroupById o2) {
+ public int compare(AccountGroupById o1, AccountGroupById o2) {
GroupDescription.Basic a = gic.get(o1.getIncludeUUID());
GroupDescription.Basic b = gic.get(o2.getIncludeUUID());
return n(a).compareTo(n(b));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupMembership.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupMembership.java
index f9d5294..c45b7b7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupMembership.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupMembership.java
@@ -46,7 +46,7 @@
* Implementors may implement the method as:
*
* <pre>
- * Set<AccountGroup.UUID> r = new HashSet<>();
+ * Set<AccountGroup.UUID> r = new HashSet<>();
* for (AccountGroup.UUID id : groupIds)
* if (contains(id)) r.add(id);
* </pre>
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/Module.java
index b11be39..52e9d4c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/Module.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/Module.java
@@ -19,6 +19,7 @@
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 @@
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 @@
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);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/Realm.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/Realm.java
index 6fd76d4..85fde4e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/Realm.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/Realm.java
@@ -15,7 +15,6 @@
package com.google.gerrit.server.account;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import java.util.Set;
@@ -29,12 +28,6 @@
AuthRequest authenticate(AuthRequest who) throws AccountException;
- AuthRequest link(ReviewDb db, Account.Id to, AuthRequest who)
- throws AccountException;
-
- AuthRequest unlink(ReviewDb db, Account.Id to, AuthRequest who)
- throws AccountException;
-
void onCreateAccount(AuthRequest who, Account account);
/** @return true if the user has the given email address. */
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java
index 1ed29a7..b71fc68 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java
@@ -53,31 +53,29 @@
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 @@
}
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 @@
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();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/Stars.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/Stars.java
new file mode 100644
index 0000000..fddbb6a
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/Stars.java
@@ -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());
+ }
+ }
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
index 8e4476b..154780a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
@@ -19,11 +19,13 @@
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.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 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 @@
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 @@
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 @@
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 @@
}
@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 @@
}
@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,
@@ -261,6 +274,38 @@
}
@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 =
new AccountResource.Email(account.getUser(), input.email);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index 5d57ed7..f143bd6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -336,6 +336,7 @@
}
}
+ @SuppressWarnings("deprecation")
@Override
public ChangeInfo get(EnumSet<ListChangesOption> s)
throws RestApiException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java
index 5485052..f4c671f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java
@@ -117,6 +117,7 @@
return query().withQuery(query);
}
+ @SuppressWarnings("deprecation")
private List<ChangeInfo> get(final QueryRequest q) throws RestApiException {
QueryChanges qc = queryProvider.get();
if (q.getQuery() != null) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
index cd1c4d3..d55bbc3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
@@ -283,16 +283,6 @@
}
@Override
- public AuthRequest link(ReviewDb db, Account.Id to, AuthRequest who) {
- return who;
- }
-
- @Override
- public AuthRequest unlink(ReviewDb db, Account.Id from, AuthRequest who) {
- return who;
- }
-
- @Override
public void onCreateAccount(final AuthRequest who, final Account account) {
usernameCache.put(who.getLocalUser(), Optional.of(account.getId()));
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/oauth/OAuthRealm.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/oauth/OAuthRealm.java
index a7bb074..cf9000d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/oauth/OAuthRealm.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/oauth/OAuthRealm.java
@@ -20,7 +20,6 @@
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Account.FieldName;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AbstractRealm;
import com.google.gerrit.server.account.AccountException;
import com.google.gerrit.server.account.AccountManager;
@@ -118,17 +117,6 @@
}
@Override
- public AuthRequest link(ReviewDb db, Account.Id to, AuthRequest who) {
- return who;
- }
-
- @Override
- public AuthRequest unlink(ReviewDb db, Account.Id to, AuthRequest who)
- throws AccountException {
- return who;
- }
-
- @Override
public void onCreateAccount(AuthRequest who, Account account) {
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/oauth/OAuthTokenCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/oauth/OAuthTokenCache.java
new file mode 100644
index 0000000..bcc5eab
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/oauth/OAuthTokenCache.java
@@ -0,0 +1,105 @@
+// 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.auth.oauth;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import com.google.common.cache.Cache;
+import com.google.gerrit.extensions.auth.oauth.OAuthToken;
+import com.google.gerrit.extensions.auth.oauth.OAuthTokenEncrypter;
+import com.google.gerrit.extensions.auth.oauth.OAuthUserInfo;
+import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Singleton;
+import com.google.inject.name.Named;
+
+@Singleton
+public class OAuthTokenCache {
+ public static final String OAUTH_TOKENS = "oauth_tokens";
+
+ private final DynamicItem<OAuthTokenEncrypter> encrypter;
+
+ public static Module module() {
+ return new CacheModule() {
+ @Override
+ protected void configure() {
+ persist(OAUTH_TOKENS, String.class, OAuthToken.class);
+ }
+ };
+ }
+
+ private final Cache<String, OAuthToken> cache;
+
+ @Inject
+ OAuthTokenCache(@Named(OAUTH_TOKENS) Cache<String, OAuthToken> cache,
+ DynamicItem<OAuthTokenEncrypter> encrypter) {
+ this.cache = cache;
+ this.encrypter = encrypter;
+ }
+
+ public boolean has(OAuthUserInfo user) {
+ return user != null
+ ? cache.getIfPresent(user.getUserName()) != null
+ : false;
+ }
+
+ public OAuthToken get(OAuthUserInfo user) {
+ return user != null
+ ? get(user.getUserName())
+ : null;
+ }
+
+ public OAuthToken get(String userName) {
+ OAuthToken accessToken = cache.getIfPresent(userName);
+ if (accessToken == null) {
+ return null;
+ }
+ accessToken = decrypt(accessToken);
+ if (accessToken.isExpired()) {
+ cache.invalidate(userName);
+ return null;
+ }
+ return accessToken;
+ }
+
+ public void put(OAuthUserInfo user, OAuthToken accessToken) {
+ cache.put(checkNotNull(user.getUserName()),
+ encrypt(checkNotNull(accessToken)));
+ }
+
+ public void remove(OAuthUserInfo user) {
+ if (user != null) {
+ cache.invalidate(user.getUserName());
+ }
+ }
+
+ private OAuthToken encrypt(OAuthToken token) {
+ OAuthTokenEncrypter enc = encrypter.get();
+ if (enc == null) {
+ return token;
+ }
+ return enc.encrypt(token);
+ }
+
+ private OAuthToken decrypt(OAuthToken token) {
+ OAuthTokenEncrypter enc = encrypter.get();
+ if (enc == null) {
+ return token;
+ }
+ return enc.decrypt(token);
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ActionJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ActionJson.java
index db8c53b..4992c8e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ActionJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ActionJson.java
@@ -35,13 +35,16 @@
@Singleton
public class ActionJson {
private final Revisions revisions;
+ private final ChangeResource.Factory changeResourceFactory;
private final DynamicMap<RestView<ChangeResource>> changeViews;
@Inject
ActionJson(
Revisions revisions,
+ ChangeResource.Factory changeResourceFactory,
DynamicMap<RestView<ChangeResource>> changeViews) {
this.revisions = revisions;
+ this.changeResourceFactory = changeResourceFactory;
this.changeViews = changeViews;
}
@@ -69,7 +72,7 @@
Provider<CurrentUser> userProvider = Providers.of(ctl.getUser());
for (UiAction.Description d : UiActions.from(
changeViews,
- new ChangeResource(ctl),
+ changeResourceFactory.create(ctl),
userProvider)) {
out.put(d.getId(), new ActionInfo(d));
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index 9e2d5f0..eaf51e4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -161,6 +161,7 @@
private final ActionJson actionJson;
private final GpgApiAdapter gpgApi;
private final ChangeNotes.Factory notesFactory;
+ private final ChangeResource.Factory changeResourceFactory;
private AccountLoader accountLoader;
private Map<Change.Id, List<SubmitRecord>> submitRecords;
@@ -187,6 +188,7 @@
ActionJson actionJson,
GpgApiAdapter gpgApi,
ChangeNotes.Factory notesFactory,
+ ChangeResource.Factory changeResourceFactory,
@Assisted Set<ListChangesOption> options) {
this.db = db;
this.labelNormalizer = ln;
@@ -207,6 +209,7 @@
this.actionJson = actionJson;
this.gpgApi = gpgApi;
this.notesFactory = notesFactory;
+ this.changeResourceFactory = changeResourceFactory;
this.options = options.isEmpty()
? EnumSet.noneOf(ListChangesOption.class)
: EnumSet.copyOf(options);
@@ -374,6 +377,7 @@
return info;
}
+ @SuppressWarnings("deprecation")
private ChangeInfo toChangeInfo(ChangeData cd,
Optional<PatchSet.Id> limitToPsId) throws PatchListNotAvailableException,
GpgException, OrmException, IOException {
@@ -956,7 +960,7 @@
&& userProvider.get().isIdentifiedUser()) {
actionJson.addRevisionActions(out,
- new RevisionResource(new ChangeResource(ctl), in));
+ new RevisionResource(changeResourceFactory.create(ctl), in));
}
if (has(PUSH_CERTIFICATES)) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java
index 3171245..05d12b3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java
@@ -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,11 +27,14 @@
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;
import com.google.gwtorm.server.OrmException;
import com.google.inject.TypeLiteral;
+import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
import org.eclipse.jgit.lib.ObjectId;
@@ -45,9 +50,17 @@
public static final TypeLiteral<RestView<ChangeResource>> CHANGE_KIND =
new TypeLiteral<RestView<ChangeResource>>() {};
+ public interface Factory {
+ ChangeResource create(ChangeControl ctl);
+ }
+
+ private final StarredChangesUtil starredChangesUtil;
private final ChangeControl control;
- public ChangeResource(ChangeControl control) {
+ @AssistedInject
+ ChangeResource(StarredChangesUtil starredChangesUtil,
+ @Assisted ChangeControl control) {
+ this.starredChangesUtil = starredChangesUtil;
this.control = control;
}
@@ -108,8 +121,12 @@
@Override
public String getETag() {
CurrentUser user = control.getUser();
- Hasher h = Hashing.md5().newHasher()
- .putBoolean(user.getStarredChanges().contains(getId()));
+ Hasher h = Hashing.md5().newHasher();
+ if (user.isIdentifiedUser()) {
+ h.putString(
+ starredChangesUtil.getObjectId(user.getAccountId(), getId()).name(),
+ UTF_8);
+ }
prepareETag(h, user);
return h.hash().toString();
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java
index 058c7d5..b89691a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java
@@ -45,6 +45,7 @@
private final DynamicMap<RestView<ChangeResource>> views;
private final ChangeFinder changeFinder;
private final CreateChange createChange;
+ private final ChangeResource.Factory changeResourceFactory;
@Inject
ChangesCollection(
@@ -53,13 +54,15 @@
Provider<QueryChanges> queryFactory,
DynamicMap<RestView<ChangeResource>> views,
ChangeFinder changeFinder,
- CreateChange createChange) {
+ CreateChange createChange,
+ ChangeResource.Factory changeResourceFactory) {
this.db = db;
this.user = user;
this.queryFactory = queryFactory;
this.views = views;
this.changeFinder = changeFinder;
this.createChange = createChange;
+ this.changeResourceFactory = changeResourceFactory;
}
@Override
@@ -86,7 +89,7 @@
if (!ctl.isVisible(db.get())) {
throw new ResourceNotFoundException(id);
}
- return new ChangeResource(ctl);
+ return changeResourceFactory.create(ctl);
}
public ChangeResource parse(Change.Id id)
@@ -102,7 +105,7 @@
if (!ctl.isVisible(db.get())) {
throw new ResourceNotFoundException(toIdString(id));
}
- return new ChangeResource(ctl);
+ return changeResourceFactory.create(ctl);
}
private static IdString toIdString(Change.Id id) {
@@ -110,7 +113,7 @@
}
public ChangeResource parse(ChangeControl control) {
- return new ChangeResource(control);
+ return changeResourceFactory.create(control);
}
@SuppressWarnings("unchecked")
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java
index c5837b4..d758a77 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java
@@ -42,16 +42,19 @@
private final Config config;
private final Provider<ReviewDb> dbProvider;
private final MergeSuperSet mergeSuperSet;
+ private final ChangeResource.Factory changeResourceFactory;
@Inject
GetRevisionActions(
ActionJson delegate,
Provider<ReviewDb> dbProvider,
MergeSuperSet mergeSuperSet,
+ ChangeResource.Factory changeResourceFactory,
@GerritServerConfig Config config) {
this.delegate = delegate;
this.dbProvider = dbProvider;
this.mergeSuperSet = mergeSuperSet;
+ this.changeResourceFactory = changeResourceFactory;
this.config = config;
}
@@ -71,7 +74,7 @@
ChangeSet cs =
mergeSuperSet.completeChangeSet(db, rsrc.getChange(), user);
for (ChangeData cd : cs.changes()) {
- new ChangeResource(cd.changeControl()).prepareETag(h, user);
+ changeResourceFactory.create(cd.changeControl()).prepareETag(h, user);
}
} catch (IOException | OrmException e) {
throw new OrmRuntimeException(e);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
index 41b1019..c59bf5a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
@@ -136,5 +136,6 @@
factory(RebaseChangeOp.Factory.class);
factory(ReviewerResource.Factory.class);
factory(SetHashtagsOp.Factory.class);
+ factory(ChangeResource.Factory.class);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
index c7e11d2..50a8be0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -56,6 +56,7 @@
private final PatchSetInserter.Factory patchSetInserterFactory;
private final MergeUtil.Factory mergeUtilFactory;
private final RebaseUtil rebaseUtil;
+ private final ChangeResource.Factory changeResourceFactory;
private final ChangeControl ctl;
private final PatchSet originalPatchSet;
@@ -77,12 +78,14 @@
PatchSetInserter.Factory patchSetInserterFactory,
MergeUtil.Factory mergeUtilFactory,
RebaseUtil rebaseUtil,
+ ChangeResource.Factory changeResourceFactory,
@Assisted ChangeControl ctl,
@Assisted PatchSet originalPatchSet,
@Assisted @Nullable String baseCommitish) {
this.patchSetInserterFactory = patchSetInserterFactory;
this.mergeUtilFactory = mergeUtilFactory;
this.rebaseUtil = rebaseUtil;
+ this.changeResourceFactory = changeResourceFactory;
this.ctl = ctl;
this.originalPatchSet = originalPatchSet;
this.baseCommitish = baseCommitish;
@@ -138,7 +141,8 @@
RevId baseRevId = new RevId((baseCommitish != null) ? baseCommitish
: ObjectId.toString(baseCommit.getId()));
Base base = rebaseUtil.parseBase(
- new RevisionResource(new ChangeResource(ctl), originalPatchSet),
+ new RevisionResource(
+ changeResourceFactory.create(ctl), originalPatchSet),
baseRevId.get());
rebasedPatchSetId = ChangeUtil.nextPatchSetId(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
index b5b3b6a..81fca18 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -21,6 +21,7 @@
import com.google.gerrit.common.EventListener;
import com.google.gerrit.common.UserScopedEventListener;
import com.google.gerrit.extensions.auth.oauth.OAuthLoginProvider;
+import com.google.gerrit.extensions.auth.oauth.OAuthTokenEncrypter;
import com.google.gerrit.extensions.config.CapabilityDefinition;
import com.google.gerrit.extensions.config.CloneCommand;
import com.google.gerrit.extensions.config.DownloadCommand;
@@ -74,6 +75,7 @@
import com.google.gerrit.server.api.accounts.AccountExternalIdCreator;
import com.google.gerrit.server.auth.AuthBackend;
import com.google.gerrit.server.auth.UniversalAuthBackend;
+import com.google.gerrit.server.auth.oauth.OAuthTokenCache;
import com.google.gerrit.server.avatar.AvatarProvider;
import com.google.gerrit.server.cache.CacheRemovalListener;
import com.google.gerrit.server.change.ChangeJson;
@@ -103,7 +105,7 @@
import com.google.gerrit.server.git.validators.RefOperationValidators;
import com.google.gerrit.server.git.validators.UploadValidationListener;
import com.google.gerrit.server.git.validators.UploadValidators;
-import com.google.gerrit.server.group.GroupInfoCacheFactory;
+import com.google.gerrit.server.group.GroupInfoCache;
import com.google.gerrit.server.group.GroupModule;
import com.google.gerrit.server.index.change.ReindexAfterUpdate;
import com.google.gerrit.server.mail.AddKeySender;
@@ -191,6 +193,7 @@
install(SectionSortCache.module());
install(SubmitStrategy.module());
install(TagCache.module());
+ install(OAuthTokenCache.module());
install(new AccessControlModule());
install(new CmdLineParserModule());
@@ -214,7 +217,7 @@
factory(ChangeJson.Factory.class);
factory(CreateChangeSender.Factory.class);
factory(GroupDetailFactory.Factory.class);
- factory(GroupInfoCacheFactory.Factory.class);
+ factory(GroupInfoCache.Factory.class);
factory(GroupMembers.Factory.class);
factory(EmailMerge.Factory.class);
factory(MergedSender.Factory.class);
@@ -315,6 +318,7 @@
DynamicSet.setOf(binder(), ProjectWebLink.class);
DynamicSet.setOf(binder(), BranchWebLink.class);
DynamicMap.mapOf(binder(), OAuthLoginProvider.class);
+ DynamicItem.itemOf(binder(), OAuthTokenEncrypter.class);
DynamicSet.setOf(binder(), AccountExternalIdCreator.class);
DynamicSet.setOf(binder(), WebUiPlugin.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
index 56b7b90..d07dea5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
@@ -575,7 +575,7 @@
Config rc, Map<String, GroupReference> groupsByName) {
accessSections = new HashMap<>();
for (String refName : rc.getSubsections(ACCESS)) {
- if (RefConfigSection.isValid(refName) & isValidRegex(refName)) {
+ if (RefConfigSection.isValid(refName) && isValidRegex(refName)) {
AccessSection as = getAccessSection(refName, true);
for (String varName : rc.getStringList(ACCESS, refName, KEY_GROUP_PERMISSIONS)) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java
index e98653a..2deb44a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java
@@ -167,9 +167,6 @@
@Singleton
static class UpdateIncludedGroup implements RestModifyView<IncludedGroupResource, PutIncludedGroup.Input> {
- static class Input {
- }
-
private final Provider<GetIncludedGroup> get;
@Inject
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java
index 4cdbb5a..0039c3c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java
@@ -187,12 +187,13 @@
}
}
}
-
- auditService.dispatchAddAccountsToGroup(self.get().getAccountId(),
- newAccountGroupMembers.values());
- db.get().accountGroupMembers().insert(newAccountGroupMembers.values());
- for (AccountGroupMember m : newAccountGroupMembers.values()) {
- accountCache.evict(m.getAccountId());
+ if (!newAccountGroupMembers.isEmpty()) {
+ auditService.dispatchAddAccountsToGroup(self.get().getAccountId(),
+ newAccountGroupMembers.values());
+ db.get().accountGroupMembers().insert(newAccountGroupMembers.values());
+ for (AccountGroupMember m : newAccountGroupMembers.values()) {
+ accountCache.evict(m.getAccountId());
+ }
}
}
@@ -254,9 +255,6 @@
@Singleton
static class UpdateMember implements RestModifyView<MemberResource, PutMember.Input> {
- static class Input {
- }
-
private final GetMember get;
@Inject
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfoCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfoCache.java
index b7d499d..d660db0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfoCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfoCache.java
@@ -14,67 +14,42 @@
package com.google.gerrit.server.group;
-import com.google.gerrit.common.data.GroupInfo;
+import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.server.account.GroupBackend;
+import com.google.inject.Inject;
-import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
-/** In-memory table of {@link GroupInfo}, indexed by {@code AccountGroup.Id}. */
+/** Efficiently builds a {@link GroupInfoCache}. */
public class GroupInfoCache {
- private static final GroupInfoCache EMPTY;
- static {
- EMPTY = new GroupInfoCache();
- EMPTY.groups = Collections.emptyMap();
+ public interface Factory {
+ GroupInfoCache create();
}
- /** Obtain an empty cache singleton. */
- public static GroupInfoCache empty() {
- return EMPTY;
- }
+ private final GroupBackend groupBackend;
+ private final Map<AccountGroup.UUID, GroupDescription.Basic> out;
- protected Map<AccountGroup.UUID, GroupInfo> groups;
-
- protected GroupInfoCache() {
- }
-
- public GroupInfoCache(final Iterable<GroupInfo> list) {
- groups = new HashMap<>();
- for (final GroupInfo gi : list) {
- groups.put(gi.getId(), gi);
- }
+ @Inject
+ GroupInfoCache(GroupBackend groupBackend) {
+ this.groupBackend = groupBackend;
+ this.out = new HashMap<>();
}
/**
- * Lookup the group summary
- * <p>
- * The return value can take on one of three forms:
- * <ul>
- * <li>{@code null}, if {@code id == null}.</li>
- * <li>a valid info block, if {@code id} was loaded.</li>
- * <li>an anonymous info block, if {@code id} was not loaded.</li>
- * </ul>
+ * Indicate a group will be needed later on.
*
- * @param uuid the id desired.
- * @return info block for the group.
+ * @param uuid identity that will be needed in the future; may be null.
*/
- public GroupInfo get(final AccountGroup.UUID uuid) {
- if (uuid == null) {
- return null;
+ public void want(final AccountGroup.UUID uuid) {
+ if (uuid != null && !out.containsKey(uuid)) {
+ out.put(uuid, groupBackend.get(uuid));
}
-
- GroupInfo r = groups.get(uuid);
- if (r == null) {
- r = new GroupInfo(uuid);
- groups.put(uuid, r);
- }
- return r;
}
- /** Merge the information from another cache into this one. */
- public void merge(final GroupInfoCache other) {
- assert this != EMPTY;
- groups.putAll(other.groups);
+ public GroupDescription.Basic get(final AccountGroup.UUID uuid) {
+ want(uuid);
+ return out.get(uuid);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfoCacheFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfoCacheFactory.java
deleted file mode 100644
index 049680e..0000000
--- a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfoCacheFactory.java
+++ /dev/null
@@ -1,62 +0,0 @@
-// Copyright (C) 2011 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.group;
-
-import com.google.gerrit.common.data.GroupDescription;
-import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.server.account.GroupBackend;
-import com.google.inject.Inject;
-
-import java.util.HashMap;
-import java.util.Map;
-
-/** Efficiently builds a {@link GroupInfoCache}. */
-public class GroupInfoCacheFactory {
- public interface Factory {
- GroupInfoCacheFactory create();
- }
-
- private final GroupBackend groupBackend;
- private final Map<AccountGroup.UUID, GroupDescription.Basic> out;
-
- @Inject
- GroupInfoCacheFactory(GroupBackend groupBackend) {
- this.groupBackend = groupBackend;
- this.out = new HashMap<>();
- }
-
- /**
- * Indicate a group will be needed later on.
- *
- * @param uuid identity that will be needed in the future; may be null.
- */
- public void want(final AccountGroup.UUID uuid) {
- if (uuid != null && !out.containsKey(uuid)) {
- out.put(uuid, groupBackend.get(uuid));
- }
- }
-
- /** Indicate one or more groups will be needed later on. */
- public void want(final Iterable<AccountGroup.UUID> uuids) {
- for (final AccountGroup.UUID uuid : uuids) {
- want(uuid);
- }
- }
-
- public GroupDescription.Basic get(final AccountGroup.UUID uuid) {
- want(uuid);
- return out.get(uuid);
- }
-}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexModule.java
index befd5ed..a3d3ad5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexModule.java
@@ -140,7 +140,7 @@
threads = config.getInt("index", null, "threads", 0);
}
if (threads <= 0) {
- return MoreExecutors.newDirectExecutorService();
+ threads = Runtime.getRuntime().availableProcessors() / 2 + 1;
}
return MoreExecutors.listeningDecorator(
workQueue.createQueue(threads, "Index-Interactive"));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/Schema.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/Schema.java
index a8ec51f..10f5ecb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/Schema.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/Schema.java
@@ -21,6 +21,7 @@
import com.google.common.base.Optional;
import com.google.common.base.Predicates;
import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.server.index.FieldDef.FillArgs;
import com.google.gwtorm.server.OrmException;
@@ -28,10 +29,38 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
+import java.util.List;
/** Specific version of a secondary index schema. */
public class Schema<T> {
+ public static class Builder<T> {
+ private final List<FieldDef<T, ?>> fields = new ArrayList<>();
+
+ public Builder<T> add(Schema<T> schema) {
+ this.fields.addAll(schema.getFields().values());
+ return this;
+ }
+
+ @SafeVarargs
+ public final Builder<T> add(FieldDef<T, ?>... fields) {
+ this.fields.addAll(Arrays.asList(fields));
+ return this;
+ }
+
+ @SafeVarargs
+ public final Builder<T> remove(FieldDef<T, ?>... fields) {
+ this.fields.removeAll(Arrays.asList(fields));
+ return this;
+ }
+
+ public Schema<T> build() {
+ return new Schema<>(ImmutableList.copyOf(fields));
+ }
+ }
+
private static final Logger log = LoggerFactory.getLogger(Schema.class);
public static class Values<T> {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
index d61040c..e7d2ab2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -30,6 +30,8 @@
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.Collection;
import java.util.HashSet;
import java.util.List;
+import java.util.Map;
import java.util.Set;
/**
@@ -78,7 +81,7 @@
/** Newer style Change-Id key. */
public static final FieldDef<ChangeData, String> ID =
- new FieldDef.Single<ChangeData, String>("change_id",
+ new FieldDef.Single<ChangeData, String>(ChangeQueryBuilder.FIELD_CHANGE_ID,
FieldType.PREFIX, false) {
@Override
public String get(ChangeData input, FillArgs args)
@@ -176,7 +179,7 @@
/** Submission id assigned by MergeOp. */
public static final FieldDef<ChangeData, String> SUBMISSIONID =
new FieldDef.Single<ChangeData, String>(
- "submissionid", FieldType.EXACT, false) {
+ ChangeQueryBuilder.FIELD_SUBMISSIONID, FieldType.EXACT, false) {
@Override
public String get(ChangeData input, FillArgs args)
throws OrmException {
@@ -207,7 +210,7 @@
public static final FieldDef<ChangeData, Iterable<String>> PATH =
new FieldDef.Repeatable<ChangeData, String>(
// Named for backwards compatibility.
- "file", FieldType.EXACT, false) {
+ ChangeQueryBuilder.FIELD_FILE, FieldType.EXACT, false) {
@Override
public Iterable<String> get(ChangeData input, FillArgs args)
throws OrmException {
@@ -234,7 +237,7 @@
/** Hashtags tied to a change */
public static final FieldDef<ChangeData, Iterable<String>> HASHTAG =
new FieldDef.Repeatable<ChangeData, String>(
- "hashtag", FieldType.EXACT, false) {
+ ChangeQueryBuilder.FIELD_HASHTAG, FieldType.EXACT, false) {
@Override
public Iterable<String> get(ChangeData input, FillArgs args)
throws OrmException {
@@ -268,7 +271,7 @@
/** Components of each file path modified in the current patch set. */
public static final FieldDef<ChangeData, Iterable<String>> FILE_PART =
new FieldDef.Repeatable<ChangeData, String>(
- "filepart", FieldType.EXACT, false) {
+ ChangeQueryBuilder.FIELD_FILEPART, FieldType.EXACT, false) {
@Override
public Iterable<String> get(ChangeData input, FillArgs args)
throws OrmException {
@@ -327,7 +330,7 @@
/** Commit ID of any patch set on the change, using exact match. */
public static final FieldDef<ChangeData, Iterable<String>> EXACT_COMMIT =
new FieldDef.Repeatable<ChangeData, String>(
- "exactcommit", FieldType.EXACT, false) {
+ ChangeQueryBuilder.FIELD_EXACTCOMMIT, FieldType.EXACT, false) {
@Override
public Iterable<String> get(ChangeData input, FillArgs args)
throws OrmException {
@@ -519,7 +522,7 @@
/** Whether the change is mergeable. */
public static final FieldDef<ChangeData, String> MERGEABLE =
new FieldDef.Single<ChangeData, String>(
- "mergeable2", FieldType.EXACT, true) {
+ ChangeQueryBuilder.FIELD_MERGEABLE, FieldType.EXACT, true) {
@Override
public String get(ChangeData input, FillArgs args)
throws OrmException {
@@ -593,6 +596,7 @@
};
/** 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,10 +613,42 @@
}
};
+ /**
+ * 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>(
- "group", FieldType.EXACT, false) {
+ ChangeQueryBuilder.FIELD_GROUP, FieldType.EXACT, false) {
@Override
public Iterable<String> get(ChangeData input, FillArgs args)
throws OrmException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 1e59cdb..db889c2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -67,9 +67,13 @@
@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();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
index 4e1891a..367159d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
@@ -28,6 +28,7 @@
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 @@
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) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java
index 22255b5..4d2a719 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java
@@ -353,7 +353,7 @@
private Map<PatchSetApproval.Key, PatchSetApproval>
filterPatchSetApprovals() {
- return limitToExistingPatchSets(patchSetApprovals,
+ return limitToValidPatchSets(patchSetApprovals,
new Function<PatchSetApproval.Key, PatchSet.Id>() {
@Override
public PatchSet.Id apply(PatchSetApproval.Key in) {
@@ -364,7 +364,7 @@
private Map<PatchLineComment.Key, PatchLineComment>
filterPatchLineComments() {
- return limitToExistingPatchSets(patchLineComments,
+ return limitToValidPatchSets(patchLineComments,
new Function<PatchLineComment.Key, PatchSet.Id>() {
@Override
public PatchSet.Id apply(PatchLineComment.Key in) {
@@ -373,13 +373,15 @@
});
}
- private <K, V> Map<K, V> limitToExistingPatchSets(Map<K, V> in,
+ private <K, V> Map<K, V> limitToValidPatchSets(Map<K, V> in,
final Function<K, PatchSet.Id> func) {
+ final Predicate<PatchSet.Id> upToCurrent = upToCurrentPredicate();
return Maps.filterKeys(
in, new Predicate<K>() {
@Override
public boolean apply(K in) {
- return patchSets.containsKey(func.apply(in));
+ PatchSet.Id psId = func.apply(in);
+ return upToCurrent.apply(psId) && patchSets.containsKey(psId);
}
});
}
@@ -395,6 +397,20 @@
});
}
+ private Predicate<PatchSet.Id> upToCurrentPredicate() {
+ final int max = change.currentPatchSetId().get();
+ return new Predicate<PatchSet.Id>() {
+ @Override
+ public boolean apply(PatchSet.Id in) {
+ return in.get() <= max;
+ }
+ };
+ }
+
+ private Map<PatchSet.Id, PatchSet> filterPatchSets() {
+ return Maps.filterKeys(patchSets, upToCurrentPredicate());
+ }
+
private static void diffChanges(List<String> diffs, ChangeBundle bundleA,
ChangeBundle bundleB) {
Change a = bundleA.change;
@@ -402,12 +418,16 @@
String desc = a.getId().equals(b.getId()) ? describe(a.getId()) : "Changes";
boolean excludeCreatedOn = false;
- boolean excludeSubject = false;
- boolean excludeOrigSubj = false;
boolean excludeTopic = false;
Timestamp aUpdated = a.getLastUpdatedOn();
Timestamp bUpdated = b.getLastUpdatedOn();
+ CharMatcher s = CharMatcher.is(' ');
+ boolean excludeSubject = false;
+ boolean excludeOrigSubj = false;
+ String aSubj = a.getSubject();
+ String bSubj = b.getSubject();
+
// Allow created timestamp in NoteDb to be either the created timestamp of
// the change, or the timestamp of the first remaining patch set.
//
@@ -433,34 +453,39 @@
// Ignore original subject on the ReviewDb side if it equals the subject of
// the current patch set.
//
+ // For all of the above subject comparisons, first trim any leading spaces
+ // from the NoteDb strings. (We actually do represent the leading spaces
+ // faithfully during conversion, but JGit's FooterLine parser trims them
+ // when reading.)
+ //
// Ignore empty topic on the ReviewDb side if it is null on the NoteDb side.
//
// Use max timestamp of all ReviewDb entities when comparing with NoteDb.
if (bundleA.source == REVIEW_DB && bundleB.source == NOTE_DB) {
excludeCreatedOn = !timestampsDiffer(
bundleA, bundleA.getFirstPatchSetTime(), bundleB, b.getCreatedOn());
- excludeSubject = b.getSubject().startsWith(a.getSubject());
+ aSubj = s.trimLeadingFrom(aSubj);
+ excludeSubject = bSubj.startsWith(aSubj);
excludeOrigSubj = true;
excludeTopic = "".equals(a.getTopic()) && b.getTopic() == null;
aUpdated = bundleA.getLatestTimestamp();
} else if (bundleA.source == NOTE_DB && bundleB.source == REVIEW_DB) {
excludeCreatedOn = !timestampsDiffer(
bundleA, a.getCreatedOn(), bundleB, bundleB.getFirstPatchSetTime());
- excludeSubject = a.getSubject().startsWith(b.getSubject());
+ bSubj = s.trimLeadingFrom(bSubj);
+ excludeSubject = aSubj.startsWith(bSubj);
excludeOrigSubj = true;
excludeTopic = a.getTopic() == null && "".equals(b.getTopic());
bUpdated = bundleB.getLatestTimestamp();
}
+ String subjectField = "subject";
String updatedField = "lastUpdatedOn";
- List<String> exclude =
- Lists.newArrayList(updatedField, "noteDbState", "rowVersion");
+ List<String> exclude = Lists.newArrayList(
+ subjectField, updatedField, "noteDbState", "rowVersion");
if (excludeCreatedOn) {
exclude.add("createdOn");
}
- if (excludeSubject) {
- exclude.add("subject");
- }
if (excludeOrigSubj) {
exclude.add("originalSubject");
}
@@ -478,6 +503,9 @@
diffTimestamps(diffs, desc, bundleA, aUpdated, bundleB, bUpdated,
"effective last updated time");
}
+ if (!excludeSubject) {
+ diffValues(diffs, desc, aSubj, bSubj, subjectField);
+ }
}
/**
@@ -599,8 +627,8 @@
private static void diffPatchSets(List<String> diffs, ChangeBundle bundleA,
ChangeBundle bundleB) {
- Map<PatchSet.Id, PatchSet> as = bundleA.patchSets;
- Map<PatchSet.Id, PatchSet> bs = bundleB.patchSets;
+ Map<PatchSet.Id, PatchSet> as = bundleA.filterPatchSets();
+ Map<PatchSet.Id, PatchSet> bs = bundleB.filterPatchSets();
for (PatchSet.Id id : diffKeySets(diffs, as, bs)) {
PatchSet a = as.get(id);
PatchSet b = bs.get(id);
@@ -732,9 +760,17 @@
if (bundleA.source == bundleB.source || ta == null || tb == null) {
diffValues(diffs, desc, ta, tb, fieldDesc);
} else if (bundleA.source == NOTE_DB) {
- diffTimestamps(diffs, desc, ta, tb, fieldDesc);
+ diffTimestamps(
+ diffs, desc,
+ bundleA.getChange(), ta,
+ bundleB.getChange(), tb,
+ fieldDesc);
} else {
- diffTimestamps(diffs, desc, tb, ta, fieldDesc);
+ diffTimestamps(
+ diffs, desc,
+ bundleB.getChange(), tb,
+ bundleA.getChange(), ta,
+ fieldDesc);
}
}
@@ -746,13 +782,25 @@
}
private static void diffTimestamps(List<String> diffs, String desc,
- Timestamp tsFromNoteDb, Timestamp tsFromReviewDb, String field) {
+ Change changeFromNoteDb, Timestamp tsFromNoteDb,
+ Change changeFromReviewDb, Timestamp tsFromReviewDb,
+ String field) {
// Because ChangeRebuilder may batch events together that are several
// seconds apart, the timestamp in NoteDb may actually be several seconds
// *earlier* than the timestamp in ReviewDb that it was converted from.
checkArgument(tsFromNoteDb.equals(roundToSecond(tsFromNoteDb)),
"%s from NoteDb has non-rounded %s timestamp: %s",
desc, field, tsFromNoteDb);
+
+ if (tsFromReviewDb.before(changeFromReviewDb.getCreatedOn())
+ && tsFromNoteDb.equals(changeFromNoteDb.getCreatedOn())) {
+ // Timestamp predates change creation. These are truncated to change
+ // creation time during NoteDb conversion, so allow this if the timestamp
+ // in NoteDb matches the createdOn time in NoteDb.
+ return;
+ }
+
+
long delta = tsFromReviewDb.getTime() - tsFromNoteDb.getTime();
long max = ChangeRebuilderImpl.MAX_WINDOW_MS;
if (delta < 0 || delta > max) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java
index 87155d4..1a44f081 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java
@@ -228,6 +228,7 @@
private void buildUpdates(NoteDbUpdateManager manager, ChangeBundle bundle)
throws IOException, OrmException {
Change change = new Change(bundle.getChange());
+ PatchSet.Id currPsId = change.currentPatchSetId();
// We will rebuild all events, except for draft comments, in buckets based
// on author and timestamp.
List<Event> events = new ArrayList<>();
@@ -245,6 +246,12 @@
Sets.newHashSetWithExpectedSize(bundle.getPatchSets().size());
for (PatchSet ps : bundle.getPatchSets()) {
+ if (ps.getId().get() > currPsId.get()) {
+ log.info(
+ "Skipping patch set {}, which is higher than current patch set {}",
+ ps.getId(), currPsId);
+ continue;
+ }
psIds.add(ps.getId());
events.add(new PatchSetEvent(
change, ps, manager.getChangeRepo().rw));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java
index fb1dce5..075069f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java
@@ -73,7 +73,6 @@
@Singleton
public class PluginLoader implements LifecycleListener {
- static final String PLUGIN_TMP_PREFIX = "plugin_";
static final Logger log = LoggerFactory.getLogger(PluginLoader.class);
public String getPluginName(Path srcPath) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java
index 115ddb5..59ed261 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java
@@ -37,22 +37,6 @@
import java.util.jar.Manifest;
public class ServerPlugin extends Plugin {
-
- /** Unique key that changes whenever a plugin reloads. */
- public static final class CacheKey {
- private final String name;
-
- CacheKey(String name) {
- this.name = name;
- }
-
- @Override
- public String toString() {
- int id = System.identityHashCode(this);
- return String.format("Plugin[%s@%x]", name, id);
- }
- }
-
private final Manifest manifest;
private final PluginContentScanner scanner;
private final Path dataDir;
@@ -124,10 +108,6 @@
return (Class<? extends Module>) clazz;
}
- Path getSrcJar() {
- return getSrcFile();
- }
-
Path getDataDir() {
return dataDir;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeModifiedException.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeModifiedException.java
deleted file mode 100644
index f3ca8b6..0000000
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeModifiedException.java
+++ /dev/null
@@ -1,23 +0,0 @@
-// Copyright (C) 2013 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.project;
-
-public class ChangeModifiedException extends InvalidChangeOperationException {
- private static final long serialVersionUID = 1L;
-
- public ChangeModifiedException(String msg) {
- super(msg);
- }
-}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AbstractResultSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AbstractResultSet.java
deleted file mode 100644
index 9bbc02f..0000000
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/AbstractResultSet.java
+++ /dev/null
@@ -1,31 +0,0 @@
-// Copyright (C) 2010 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.gwtorm.server.ResultSet;
-
-import java.util.ArrayList;
-import java.util.List;
-
-abstract class AbstractResultSet<T> implements ResultSet<T> {
- @Override
- public List<T> toList() {
- ArrayList<T> r = new ArrayList<>();
- for (T t : this) {
- r.add(t);
- }
- return r;
- }
-}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index b32ccab..5eed391 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -24,10 +24,12 @@
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 @@
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 @@
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) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 230bf31..07c8c72 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -35,6 +35,7 @@
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;
@@ -102,12 +103,11 @@
// SearchSuggestOracle up to date.
public static final String FIELD_ADDED = "added";
- public static final String FIELD_AFTER = "after";
public static final String FIELD_AGE = "age";
public static final String FIELD_AUTHOR = "author";
public static final String FIELD_BEFORE = "before";
- public static final String FIELD_BRANCH = "branch";
public static final String FIELD_CHANGE = "change";
+ public static final String FIELD_CHANGE_ID = "change_id";
public static final String FIELD_COMMENT = "comment";
public static final String FIELD_COMMENTBY = "commentby";
public static final String FIELD_COMMIT = "commit";
@@ -118,14 +118,15 @@
public static final String FIELD_DESTINATION = "destination";
public static final String FIELD_DRAFTBY = "draftby";
public static final String FIELD_EDITBY = "editby";
+ public static final String FIELD_EXACTCOMMIT = "exactcommit";
public static final String FIELD_FILE = "file";
- public static final String FIELD_IS = "is";
- public static final String FIELD_HAS = "has";
+ public static final String FIELD_FILEPART = "filepart";
+ public static final String FIELD_GROUP = "group";
public static final String FIELD_HASHTAG = "hashtag";
public static final String FIELD_LABEL = "label";
public static final String FIELD_LIMIT = "limit";
public static final String FIELD_MERGE = "merge";
- public static final String FIELD_MERGEABLE = "mergeable";
+ public static final String FIELD_MERGEABLE = "mergeable2";
public static final String FIELD_MESSAGE = "message";
public static final String FIELD_OWNER = "owner";
public static final String FIELD_OWNERIN = "ownerin";
@@ -133,14 +134,15 @@
public static final String FIELD_PATH = "path";
public static final String FIELD_PROJECT = "project";
public static final String FIELD_PROJECTS = "projects";
- public static final String FIELD_QUERY = "query";
public static final String FIELD_REF = "ref";
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";
+ public static final String FIELD_SUBMISSIONID = "submissionid";
public static final String FIELD_TR = "tr";
public static final String FIELD_VISIBLETO = "visibleto";
public static final String FIELD_WATCHEDBY = "watchedby";
@@ -422,6 +424,10 @@
return starredby(self());
}
+ if ("stars".equalsIgnoreCase(value)) {
+ return new HasStarsPredicate(self());
+ }
+
if ("draft".equalsIgnoreCase(value)) {
return draftby(self());
}
@@ -646,6 +652,11 @@
}
@Operator
+ public Predicate<ChangeData> star(String label) throws QueryParseException {
+ return new StarPredicate(self(), label);
+ }
+
+ @Operator
public Predicate<ChangeData> starredby(String who)
throws QueryParseException, OrmException {
return starredby(parseAccount(who));
@@ -663,6 +674,10 @@
@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));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/HasStarsPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/HasStarsPredicate.java
new file mode 100644
index 0000000..83990bc
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/HasStarsPredicate.java
@@ -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;
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
index 37cdfaf..81d8754 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
@@ -298,6 +298,7 @@
return query(and(project(project), or(groupPredicates)));
}
+ @SuppressWarnings("deprecation")
public List<ChangeData> byIsStarred(Account.Id id) throws OrmException {
return query(new IsStarredByPredicate(id));
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java
index 50c54f6..634bc4a6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java
@@ -19,6 +19,7 @@
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;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java
index 88f1911..43fb0c5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java
@@ -111,6 +111,7 @@
return out.size() == 1 ? out.get(0) : out;
}
+ @SuppressWarnings("deprecation")
private List<List<ChangeInfo>> query()
throws OrmException, QueryParseException {
if (imp.isDisabled()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/StarPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/StarPredicate.java
new file mode 100644
index 0000000..2facdb7
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/StarPredicate.java
@@ -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;
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java
index f57fba4..ea1120f 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java
@@ -40,10 +40,13 @@
import com.google.gwtorm.protobuf.ProtobufCodec;
import com.google.gwtorm.server.StandardKeyEncoder;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
+import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -366,6 +369,64 @@
}
@Test
+ public void diffChangesTrimsLeadingSpacesFromReviewDbComparingToNoteDb()
+ throws Exception {
+ Change c1 = TestChanges.newChange(
+ new Project.NameKey("project"), new Account.Id(100));
+ Change c2 = clone(c1);
+ c2.setCurrentPatchSet(c1.currentPatchSetId(),
+ " " + c1.getSubject(), c1.getOriginalSubject());
+
+ // Both ReviewDb, exact match required.
+ ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
+ comments(), REVIEW_DB);
+ ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
+ comments(), REVIEW_DB);
+ assertDiffs(b1, b2,
+ "subject differs for Change.Id " + c1.getId() + ":"
+ + " {Change subject} != { Change subject}");
+
+ // ReviewDb is missing leading spaces, allowed.
+ b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
+ comments(), NOTE_DB);
+ b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
+ comments(), REVIEW_DB);
+ assertNoDiffs(b1, b2);
+ assertNoDiffs(b2, b1);
+ }
+
+ @Test
+ public void diffChangesDoesntTrimLeadingNonSpaceWhitespaceFromSubject()
+ throws Exception {
+ Change c1 = TestChanges.newChange(
+ new Project.NameKey("project"), new Account.Id(100));
+ Change c2 = clone(c1);
+ c2.setCurrentPatchSet(c1.currentPatchSetId(),
+ "\t" + c1.getSubject(), c1.getOriginalSubject());
+
+ // Both ReviewDb.
+ ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
+ comments(), REVIEW_DB);
+ ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
+ comments(), REVIEW_DB);
+ assertDiffs(b1, b2,
+ "subject differs for Change.Id " + c1.getId() + ":"
+ + " {Change subject} != {\tChange subject}");
+
+ // One NoteDb.
+ b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
+ comments(), NOTE_DB);
+ b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
+ comments(), REVIEW_DB);
+ assertDiffs(b1, b2,
+ "subject differs for Change.Id " + c1.getId() + ":"
+ + " {Change subject} != {\tChange subject}");
+ assertDiffs(b2, b1,
+ "subject differs for Change.Id " + c1.getId() + ":"
+ + " {\tChange subject} != {Change subject}");
+ }
+
+ @Test
public void diffChangeMessageKeySets() throws Exception {
Change c = TestChanges.newChange(project, accountId);
int id = c.getId().get();
@@ -706,6 +767,54 @@
}
@Test
+ public void diffIgnoresPatchSetsGreaterThanCurrent() throws Exception {
+ Change c = TestChanges.newChange(project, accountId);
+
+ PatchSet ps1 = new PatchSet(new PatchSet.Id(c.getId(), 1));
+ ps1.setRevision(new RevId("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"));
+ ps1.setUploader(accountId);
+ ps1.setCreatedOn(TimeUtil.nowTs());
+ PatchSet ps2 = new PatchSet(new PatchSet.Id(c.getId(), 2));
+ ps2.setRevision(new RevId("badc0feebadc0feebadc0feebadc0feebadc0fee"));
+ ps2.setUploader(accountId);
+ ps2.setCreatedOn(TimeUtil.nowTs());
+ assertThat(ps2.getId().get()).isGreaterThan(c.currentPatchSetId().get());
+
+ PatchSetApproval a1 = new PatchSetApproval(
+ new PatchSetApproval.Key(
+ ps1.getId(), accountId, new LabelId("Code-Review")),
+ (short) 1,
+ TimeUtil.nowTs());
+ PatchSetApproval a2 = new PatchSetApproval(
+ new PatchSetApproval.Key(
+ ps2.getId(), accountId, new LabelId("Code-Review")),
+ (short) 1,
+ TimeUtil.nowTs());
+
+ // Both ReviewDb.
+ ChangeBundle b1 = new ChangeBundle(c, messages(), patchSets(ps1),
+ approvals(a1), comments(), REVIEW_DB);
+ ChangeBundle b2 = new ChangeBundle(c, messages(), patchSets(ps1, ps2),
+ approvals(a1, a2), comments(), REVIEW_DB);
+ assertNoDiffs(b1, b2);
+
+ // One NoteDb.
+ b1 = new ChangeBundle(c, messages(), patchSets(ps1), approvals(a1),
+ comments(), NOTE_DB);
+ b2 = new ChangeBundle(c, messages(), patchSets(ps1, ps2), approvals(a1, a2),
+ comments(), REVIEW_DB);
+ assertNoDiffs(b1, b2);
+ assertNoDiffs(b2, b1);
+
+ // Both NoteDb.
+ b1 = new ChangeBundle(c, messages(), patchSets(ps1), approvals(a1),
+ comments(), NOTE_DB);
+ b2 = new ChangeBundle(c, messages(), patchSets(ps1, ps2), approvals(a1, a2),
+ comments(), NOTE_DB);
+ assertNoDiffs(b1, b2);
+ }
+
+ @Test
public void diffPatchSetApprovalKeySets() throws Exception {
Change c = TestChanges.newChange(project, accountId);
int id = c.getId().get();
@@ -799,6 +908,39 @@
}
@Test
+ public void diffPatchSetApprovalsAllowsTruncatedTimestampInNoteDb()
+ throws Exception {
+ Change c = TestChanges.newChange(project, accountId);
+ PatchSetApproval a1 = new PatchSetApproval(
+ new PatchSetApproval.Key(
+ c.currentPatchSetId(), accountId, new LabelId("Code-Review")),
+ (short) 1,
+ c.getCreatedOn());
+ PatchSetApproval a2 = clone(a1);
+ a2.setGranted(new Timestamp(new DateTime(
+ 1900, 1, 1, 0, 0, 0, DateTimeZone.forTimeZone(TimeZone.getDefault()))
+ .getMillis()));
+
+ // Both are ReviewDb, exact match is required.
+ ChangeBundle b1 = new ChangeBundle(c, messages(), latest(c), approvals(a1),
+ comments(), REVIEW_DB);
+ ChangeBundle b2 = new ChangeBundle(c, messages(), latest(c), approvals(a2),
+ comments(), REVIEW_DB);
+ assertDiffs(b1, b2,
+ "granted differs for PatchSetApproval.Key "
+ + c.getId() + "%2C1,100,Code-Review:"
+ + " {2009-09-30 17:00:00.0} != {1900-01-01 00:00:00.0}");
+
+ // Truncating NoteDb timestamp is allowed.
+ b1 = new ChangeBundle(c, messages(), latest(c), approvals(a1), comments(),
+ NOTE_DB);
+ b2 = new ChangeBundle(c, messages(), latest(c), approvals(a2), comments(),
+ REVIEW_DB);
+ assertNoDiffs(b1, b2);
+ assertNoDiffs(b2, b1);
+ }
+
+ @Test
public void diffPatchLineCommentKeySets() throws Exception {
Change c = TestChanges.newChange(project, accountId);
int id = c.getId().get();
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
index f1b5c9a..3f77498 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -47,6 +47,7 @@
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
+import com.google.gerrit.testutil.TestChanges;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -753,6 +754,34 @@
}
@Test
+ public void subjectLeadingWhitespaceChangeNotes() throws Exception {
+ Change c = TestChanges.newChange(project, changeOwner.getAccountId());
+ String trimmedSubj = c.getSubject();
+ c.setCurrentPatchSet(c.currentPatchSetId(), " " + trimmedSubj,
+ c.getOriginalSubject());
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.setChangeId(c.getKey().get());
+ update.setBranch(c.getDest().get());
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getChange().getSubject()).isEqualTo(trimmedSubj);
+
+ String tabSubj = "\t\t" + trimmedSubj;
+
+ c = TestChanges.newChange(project, changeOwner.getAccountId());
+ c.setCurrentPatchSet(c.currentPatchSetId(), tabSubj,
+ c.getOriginalSubject());
+ update = newUpdate(c, changeOwner);
+ update.setChangeId(c.getKey().get());
+ update.setBranch(c.getDest().get());
+ update.commit();
+
+ notes = newNotes(c);
+ assertThat(notes.getChange().getSubject()).isEqualTo(tabSubj);
+ }
+
+ @Test
public void commitChangeNotesUnique() throws Exception {
// PatchSetId -> RevId must be a one to one mapping
Change c = newChange();
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
index 47c8a03..46d8818 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
@@ -289,6 +289,43 @@
update.getResult());
}
+ @Test
+ public void leadingWhitespace() throws Exception {
+ Change c = TestChanges.newChange(project, changeOwner.getAccountId());
+ c.setCurrentPatchSet(c.currentPatchSetId(), " " + c.getSubject(),
+ c.getOriginalSubject());
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.setChangeId(c.getKey().get());
+ update.setBranch(c.getDest().get());
+ update.commit();
+
+ assertBodyEquals("Update patch set 1\n"
+ + "\n"
+ + "Patch-set: 1\n"
+ + "Change-id: " + c.getKey().get() + "\n"
+ + "Subject: Change subject\n"
+ + "Branch: refs/heads/master\n"
+ + "Commit: " + update.getCommit().name() + "\n",
+ update.getResult());
+
+ c = TestChanges.newChange(project, changeOwner.getAccountId());
+ c.setCurrentPatchSet(c.currentPatchSetId(), "\t\t" + c.getSubject(),
+ c.getOriginalSubject());
+ update = newUpdate(c, changeOwner);
+ update.setChangeId(c.getKey().get());
+ update.setBranch(c.getDest().get());
+ update.commit();
+
+ assertBodyEquals("Update patch set 1\n"
+ + "\n"
+ + "Patch-set: 1\n"
+ + "Change-id: " + c.getKey().get() + "\n"
+ + "Subject: \t\tChange subject\n"
+ + "Branch: refs/heads/master\n"
+ + "Commit: " + update.getCommit().name() + "\n",
+ update.getResult());
+ }
+
private RevCommit parseCommit(ObjectId id) throws Exception {
if (id instanceof RevCommit) {
return (RevCommit) id;
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 61d7590..bcc9b38 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -38,6 +38,7 @@
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.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 java.util.ArrayList;
import java.util.Arrays;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
@@ -1241,6 +1244,35 @@
}
@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");
Change change1 = insert(repo, newChange(repo));
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ShowQueue.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ShowQueue.java
index 5971b8d..34f281e 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ShowQueue.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ShowQueue.java
@@ -52,7 +52,7 @@
private IdentifiedUser currentUser;
private int columns = 80;
- private int taskNameWidth;
+ private int maxCommandWidth;
@Override
public void start(Environment env) throws IOException {
@@ -69,7 +69,7 @@
@Override
protected void run() throws UnloggedFailure {
- taskNameWidth = wide ? Integer.MAX_VALUE : columns - 8 - 12 - 12 - 4 - 4;
+ maxCommandWidth = wide ? Integer.MAX_VALUE : columns - 8 - 12 - 12 - 4 - 4;
stdout.print(String.format("%-8s %-12s %-12s %-4s %s\n", //
"Task", "State", "StartTime", "", "Command"));
stdout.print("----------------------------------------------"
@@ -97,9 +97,9 @@
// Shows information about tasks depending on the user rights
if (viewAll || task.projectName == null) {
- String command = task.command.length() < taskNameWidth
+ String command = task.command.length() < maxCommandWidth
? task.command
- : task.command.substring(0, taskNameWidth);
+ : task.command.substring(0, maxCommandWidth);
stdout.print(String.format("%8s %-12s %-12s %-4s %s\n",
task.id, start, startTime(task.startTime), "", command));
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 3139e89..4480319 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -103,6 +103,9 @@
.prefsButton {
text-align: right;
}
+ #modeSelect {
+ margin-left: .5em;
+ }
@media screen and (max-width: 50em) {
.dash {
display: none;
@@ -168,11 +171,17 @@
patch-range="[[_patchRange]]"
available-patches="[[_computeAvailablePatches(_change.revisions)]]">
</gr-patch-range-select>
- <gr-button link
- class="prefsButton"
- on-tap="_handlePrefsTap"
- hidden$="[[_computePrefsButtonHidden(_prefs, _loggedIn)]]"
- hidden>Diff View Preferences</gr-button>
+ <div>
+ <gr-button link
+ class="prefsButton"
+ on-tap="_handlePrefsTap"
+ hidden$="[[_computePrefsButtonHidden(_prefs, _loggedIn)]]"
+ hidden>Diff View Preferences</gr-button>
+ <select id="modeSelect" on-change="_handleModeChange">
+ <option value="SIDE_BY_SIDE">Side By Side</option>
+ <option value="UNIFIED_DIFF">Unified</option>
+ </select>
+ </div>
</div>
<gr-overlay id="prefsOverlay" with-backdrop>
<gr-diff-preferences
@@ -186,6 +195,7 @@
path="[[_path]]"
prefs="[[_prefs]]"
project-config="[[_projectConfig]]"
+ view-mode="[[_diffMode]]"
on-render="_handleDiffRender">
</gr-diff>
</div>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index b2adcf4..6932d10 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -16,6 +16,11 @@
var COMMIT_MESSAGE_PATH = '/COMMIT_MSG';
+ var DiffViewMode = {
+ SIDE_BY_SIDE: 'SIDE_BY_SIDE',
+ UNIFIED: 'UNIFIED_DIFF',
+ };
+
Polymer({
is: 'gr-diff-view',
@@ -64,6 +69,11 @@
value: true,
},
_prefs: Object,
+ _userPrefs: Object,
+ _diffMode: {
+ type: String,
+ computed: '_getDiffViewMode(changeViewState.diffMode, _userPrefs)'
+ },
},
behaviors: [
@@ -74,6 +84,7 @@
'_getChangeDetail(_changeNum)',
'_getProjectConfig(_change.project)',
'_getFiles(_changeNum, _patchRange.patchNum)',
+ '_updateModeSelect(_diffMode)',
],
attached: function() {
@@ -92,6 +103,9 @@
},
detached: function() {
+ // Reset the diff mode to null so that it reverts to the user preference.
+ this.changeViewState.diffMode = null;
+
window.removeEventListener('resize', this._boundWindowResizeHandler);
},
@@ -124,6 +138,10 @@
return this.$.restAPI.getDiffPreferences();
},
+ _getPreferences: function() {
+ return this.$.restAPI.getPreferences();
+ },
+
_handleReviewedChange: function(e) {
this._setReviewed(Polymer.dom(e).rootTarget.checked);
},
@@ -242,6 +260,10 @@
this._prefs = prefs;
}.bind(this)));
+ promises.push(this._getPreferences().then(function(prefs) {
+ this._userPrefs = prefs;
+ }.bind(this)));
+
promises.push(this.$.diff.reload());
Promise.all(promises).then(function() {
@@ -360,5 +382,43 @@
e.stopPropagation();
this.$.prefsOverlay.close();
},
+
+ _handleModeChange: function(e) {
+ this.set('changeViewState.diffMode', this.$.modeSelect.value);
+ },
+
+ /**
+ * _getDiffViewMode: Get the diff view (side-by-side or unified) based on
+ * the current state.
+ *
+ * The expected behavior is to use the mode specified in the user's
+ * preferences unless they have manually chosen the alternative view. If the
+ * user navigates up to the change view, it should clear this choice and
+ * revert to the preference the next time a diff is viewed.
+ *
+ * Use side-by-side if the user is not logged in.
+ *
+ * @return {String}
+ */
+ _getDiffViewMode: function() {
+ if (this.changeViewState.diffMode) {
+ return this.changeViewState.diffMode;
+ } else if (this._userPrefs && this._userPrefs.diff_view) {
+ return this.changeViewState.diffMode = this._userPrefs.diff_view;
+ }
+
+ return DiffViewMode.SIDE_BY_SIDE;
+ },
+
+ /**
+ * Synchronize the mode select if it deviates from the selected mode state.
+ * This is mainly to keep it accurate when the page loads.
+ */
+ _updateModeSelect: function() {
+ var mode = this._getDiffViewMode();
+ if (this.$.modeSelect.value !== mode) {
+ this.$.modeSelect.value = mode;
+ }
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
index e7e4055..4167424 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -356,5 +356,37 @@
done();
});
});
+
+ test('diff mode selector correctly toggles the diff', function() {
+ var select = element.$.modeSelect;
+ var diffDisplay = element.$.diff;
+
+ element._userPrefs = {diff_view: 'SIDE_BY_SIDE'};
+
+ // The mode selected in the view state reflects the selected option.
+ assert.equal(element._getDiffViewMode(), select.value);
+
+ // The mode selected in the view state reflects the view rednered in the
+ // diff.
+ assert.equal(select.value, diffDisplay.viewMode);
+
+ // We will simulate a user change of the selected mode.
+ var newMode = 'UNIFIED_DIFF';
+
+ // Listen to the change handler.
+ var eventStub = sinon.spy(element, '_handleModeChange');
+
+ // Set the actual value of the select, and simulate the change event.
+ select.value = newMode;
+ element.fire('change', {}, { node: select });
+
+ // Make sure the handler was called and the state is still coherent.
+ assert.isTrue(eventStub.called);
+ assert.equal(element._getDiffViewMode(), newMode);
+ assert.equal(element._getDiffViewMode(), select.value);
+ assert.equal(element._getDiffViewMode(), diffDisplay.viewMode);
+
+ eventStub.restore();
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 2c70ef1..7b1ce00 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -131,7 +131,7 @@
content: '\00BB';
}
</style>
- <div class$="[[_computeContainerClass(_loggedIn, _viewMode)]]"
+ <div class$="[[_computeContainerClass(_loggedIn, viewMode)]]"
on-tap="_handleTap"
on-mousedown="_handleMouseDown"
on-copy="_handleCopy">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 8ee3a0b..8fef29c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -47,7 +47,7 @@
type: Boolean,
value: false,
},
- _viewMode: {
+ viewMode: {
type: String,
value: DiffViewMode.SIDE_BY_SIDE,
},
@@ -69,7 +69,7 @@
},
observers: [
- '_prefsChanged(prefs.*)',
+ '_prefsChanged(prefs.*, viewMode)',
],
attached: function() {
@@ -439,14 +439,14 @@
},
_getDiffBuilder: function(diff, comments, prefs) {
- if (this._viewMode === DiffViewMode.SIDE_BY_SIDE) {
+ if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) {
return new GrDiffBuilderSideBySide(diff, comments, prefs,
this.$.diffTable);
- } else if (this._viewMode === DiffViewMode.UNIFIED) {
+ } else if (this.viewMode === DiffViewMode.UNIFIED) {
return new GrDiffBuilderUnified(diff, comments, prefs,
this.$.diffTable);
}
- throw Error('Unsupported diff view mode: ' + this._viewMode);
+ throw Error('Unsupported diff view mode: ' + this.viewMode);
},
_projectConfigChanged: function(projectConfig) {
diff --git a/polygerrit-ui/app/elements/gr-app.js b/polygerrit-ui/app/elements/gr-app.js
index b093a8c..8897967 100644
--- a/polygerrit-ui/app/elements/gr-app.js
+++ b/polygerrit-ui/app/elements/gr-app.js
@@ -71,6 +71,7 @@
patchNum: null,
selectedFileIndex: 0,
showReplyDialog: false,
+ diffMode: null,
},
changeListView: {
query: null,
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index 1ada7e9..a2fa039 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -143,9 +143,7 @@
encodeURIComponent(values[i]));
}
}
- // Sorting the params leaves the order deterministic which is easier
- // to test.
- return url + '?' + params.sort().join('&');
+ return url + '?' + params.join('&');
},
getResponseObject: function(response) {
@@ -630,16 +628,12 @@
_sendDiffDraftRequest: function(method, changeNum, patchNum, draft) {
var url = this.getChangeActionURL(changeNum, patchNum, '/drafts');
+ if (draft.id) {
+ url += '/' + draft.id;
+ }
var body;
- switch(method) {
- case 'PUT':
- body = draft;
- break;
- case 'DELETE':
- url += '/' + draft.id;
- break;
- default:
- throw Error('Unsupported HTTP method: ' + method);
+ if (method === 'PUT') {
+ body = draft;
}
return this.send(method, url, body);
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index 0004fa9..e7f7a21 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -88,13 +88,19 @@
gr: 'guten tag',
noval: null,
});
- assert.equal(url, '/path/?gr=guten%20tag&noval&sp=hola');
+ assert.equal(url, '/path/?sp=hola&gr=guten%20tag&noval');
url = element._urlWithParams('/path/', {
sp: 'hola',
en: ['hey', 'hi'],
});
- assert.equal(url, '/path/?en=hey&en=hi&sp=hola');
+ assert.equal(url, '/path/?sp=hola&en=hey&en=hi');
+
+ // Order must be maintained with array params.
+ url = element._urlWithParams('/path/', {
+ l: ['c', 'b', 'a'],
+ });
+ assert.equal(url, '/path/?l=c&l=b&l=a');
});
test('request callbacks can be canceled', function(done) {
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 9d16b84..efdbaef 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -49,6 +49,7 @@
'../elements/diff/gr-diff-preferences/gr-diff-preferences_test.html',
'../elements/diff/gr-diff-view/gr-diff-view_test.html',
'../elements/diff/gr-patch-range-select/gr-patch-range-select_test.html',
+ '../elements/shared/gr-alert/gr-alert_test.html',
'../elements/shared/gr-account-label/gr-account-label_test.html',
'../elements/shared/gr-account-link/gr-account-link_test.html',
'../elements/shared/gr-alert/gr-alert_test.html',
