Merge "In AbstractChangeUpdate, restrict access levels and add @Nullable annotation"
diff --git a/Documentation/note-db.txt b/Documentation/note-db.txt
index 308e045..8725cee 100644
--- a/Documentation/note-db.txt
+++ b/Documentation/note-db.txt
@@ -192,3 +192,10 @@
of all changes in NoteDb is accurate, and so is only safe once all changes are
NoteDb primary. Otherwise, reading changes only from NoteDb might result in
inaccurate results, and writing to NoteDb would compound the problem. +
+
+== NoteDB to ReviewDB rollback
+
+In case of rollback from NoteDB to ReviewDB, all the meta refs and the
+sequence ref need to be removed.
+The [remove-notedb-refs.sh](https://gerrit.googlesource.com/gerrit/+/refs/heads/master/contrib/remove-notedb-refs.sh)
+script has been written to automate this process.
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index 6ef4e20..85cdace 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -733,8 +733,8 @@
[
{
"seq": 1,
- "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d john.doe@example.com",
- "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d",
+ "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw== john.doe@example.com",
+ "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw==",
"algorithm": "ssh-rsa",
"comment": "john.doe@example.com",
"valid": true
@@ -767,8 +767,8 @@
)]}'
{
"seq": 1,
- "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d john.doe@example.com",
- "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d",
+ "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw== john.doe@example.com",
+ "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw==",
"algorithm": "ssh-rsa",
"comment": "john.doe@example.com",
"valid": true
@@ -791,9 +791,9 @@
.Request
----
POST /accounts/self/sshkeys HTTP/1.0
- Content-Type: plain/text
+ Content-Type: text/plain
- AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d
+ ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw== john.doe@example.com
----
As response an link:#ssh-key-info[SshKeyInfo] entity is returned that
@@ -808,8 +808,8 @@
)]}'
{
"seq": 2,
- "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d john.doe@example.com",
- "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d",
+ "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw== john.doe@example.com",
+ "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw==",
"algorithm": "ssh-rsa",
"comment": "john.doe@example.com",
"valid": true
diff --git a/java/com/google/gerrit/server/BUILD b/java/com/google/gerrit/server/BUILD
index 979ab6f..45d037a 100644
--- a/java/com/google/gerrit/server/BUILD
+++ b/java/com/google/gerrit/server/BUILD
@@ -112,6 +112,7 @@
"//lib/commons:lang",
"//lib/commons:net",
"//lib/commons:validator",
+ "//lib/errorprone:annotations",
"//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-assistedinject",
diff --git a/java/com/google/gerrit/server/account/AccountResolver.java b/java/com/google/gerrit/server/account/AccountResolver.java
index 988d871..a41a36c 100644
--- a/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/java/com/google/gerrit/server/account/AccountResolver.java
@@ -516,12 +516,22 @@
* @throws IOException if an error occurs.
*/
public Result resolve(String input) throws ConfigInvalidException, IOException {
- return searchImpl(input, searchers, visibilitySupplier(), accountActivityPredicate());
+ return searchImpl(input, searchers, visibilitySupplierCanSee(), accountActivityPredicate());
}
public Result resolve(String input, Predicate<AccountState> accountActivityPredicate)
throws ConfigInvalidException, IOException {
- return searchImpl(input, searchers, visibilitySupplier(), accountActivityPredicate);
+ return searchImpl(input, searchers, visibilitySupplierCanSee(), accountActivityPredicate);
+ }
+
+ public Result resolveIgnoreVisibility(String input) throws ConfigInvalidException, IOException {
+ return searchImpl(input, searchers, visibilitySupplierAll(), accountActivityPredicate());
+ }
+
+ public Result resolveIgnoreVisibility(
+ String input, Predicate<AccountState> accountActivityPredicate)
+ throws ConfigInvalidException, IOException {
+ return searchImpl(input, searchers, visibilitySupplierAll(), accountActivityPredicate);
}
/**
@@ -550,13 +560,23 @@
@Deprecated
public Result resolveByNameOrEmail(String input) throws ConfigInvalidException, IOException {
return searchImpl(
- input, nameOrEmailSearchers, visibilitySupplier(), accountActivityPredicate());
+ input, nameOrEmailSearchers, visibilitySupplierCanSee(), accountActivityPredicate());
}
- private Supplier<Predicate<AccountState>> visibilitySupplier() {
+ private Supplier<Predicate<AccountState>> visibilitySupplierCanSee() {
return () -> accountControlFactory.get()::canSee;
}
+ private Supplier<Predicate<AccountState>> visibilitySupplierAll() {
+ return () -> all();
+ }
+
+ private Predicate<AccountState> all() {
+ return accountState -> {
+ return true;
+ };
+ }
+
private Predicate<AccountState> accountActivityPredicate() {
return (AccountState accountState) -> accountState.account().isActive();
}
diff --git a/java/com/google/gerrit/server/index/StalenessCheckResult.java b/java/com/google/gerrit/server/index/StalenessCheckResult.java
index cd3f592..fe35e6e 100644
--- a/java/com/google/gerrit/server/index/StalenessCheckResult.java
+++ b/java/com/google/gerrit/server/index/StalenessCheckResult.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.index;
import com.google.auto.value.AutoValue;
+import com.google.errorprone.annotations.FormatMethod;
import java.util.Optional;
/** Structured result of a staleness check. */
@@ -29,6 +30,7 @@
return new AutoValue_StalenessCheckResult(true, Optional.of(reason));
}
+ @FormatMethod
public static StalenessCheckResult stale(String reason, Object... args) {
return stale(String.format(reason, args));
}
diff --git a/java/com/google/gerrit/server/restapi/change/Reviewers.java b/java/com/google/gerrit/server/restapi/change/Reviewers.java
index b2714da..d702142 100644
--- a/java/com/google/gerrit/server/restapi/change/Reviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/Reviewers.java
@@ -21,12 +21,11 @@
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
-import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.mail.Address;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ReviewerResource;
-import com.google.gerrit.server.restapi.account.AccountsCollection;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@@ -37,22 +36,22 @@
public class Reviewers implements ChildCollection<ChangeResource, ReviewerResource> {
private final DynamicMap<RestView<ReviewerResource>> views;
private final ApprovalsUtil approvalsUtil;
- private final AccountsCollection accounts;
private final ReviewerResource.Factory resourceFactory;
private final ListReviewers list;
+ private final AccountResolver accountResolver;
@Inject
Reviewers(
ApprovalsUtil approvalsUtil,
- AccountsCollection accounts,
ReviewerResource.Factory resourceFactory,
DynamicMap<RestView<ReviewerResource>> views,
- ListReviewers list) {
+ ListReviewers list,
+ AccountResolver accountResolver) {
this.approvalsUtil = approvalsUtil;
- this.accounts = accounts;
this.resourceFactory = resourceFactory;
this.views = views;
this.list = list;
+ this.accountResolver = accountResolver;
}
@Override
@@ -68,22 +67,18 @@
@Override
public ReviewerResource parse(ChangeResource rsrc, IdString id)
throws ResourceNotFoundException, AuthException, IOException, ConfigInvalidException {
- Address address = Address.tryParse(id.get());
-
- Account.Id accountId = null;
try {
- accountId = accounts.parse(TopLevelResource.INSTANCE, id).getUser().getAccountId();
- } catch (ResourceNotFoundException e) {
- if (address == null) {
- throw e;
+
+ AccountResolver.Result result = accountResolver.resolveIgnoreVisibility(id.get());
+ if (fetchAccountIds(rsrc).contains(result.asUniqueUser().getAccountId())) {
+ return resourceFactory.create(rsrc, result.asUniqueUser().getAccountId());
+ }
+ } catch (AccountResolver.UnresolvableAccountException e) {
+ if (e.isSelf()) {
+ throw new AuthException(e.getMessage(), e);
}
}
- // See if the id exists as a reviewer for this change
- if (accountId != null && fetchAccountIds(rsrc).contains(accountId)) {
- return resourceFactory.create(rsrc, accountId);
- }
-
- // See if the address exists as a reviewer on the change
+ Address address = Address.tryParse(id.get());
if (address != null && rsrc.getNotes().getReviewersByEmail().all().contains(address)) {
return new ReviewerResource(rsrc, address);
}
diff --git a/java/com/google/gerrit/sshd/commands/SetAccountCommand.java b/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
index e0c4624..df1e3ed 100644
--- a/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
+++ b/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
@@ -275,7 +275,7 @@
throws RestApiException, IOException, ConfigInvalidException, PermissionBackendException {
for (String sshKey : sshKeys) {
SshKeyInput in = new SshKeyInput();
- in.raw = RawInputUtil.create(sshKey.getBytes(UTF_8), "plain/text");
+ in.raw = RawInputUtil.create(sshKey.getBytes(UTF_8), "text/plain");
addSshKey.apply(rsrc, in);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 477ec38..74f753d 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -2204,7 +2204,7 @@
gApi.changes().id(r.getChangeId()).reviewer(admin.id().toString()).votes();
assertThat(m).hasSize(1);
- assertThat(m).containsEntry("Code-Review", Short.valueOf((short) 2));
+ assertThat(m).containsExactly("Code-Review", Short.valueOf((short) 2));
requestScopeOperations.setApiUser(user.id());
gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.dislike());
@@ -2212,7 +2212,26 @@
m = gApi.changes().id(r.getChangeId()).reviewer(user.id().toString()).votes();
assertThat(m).hasSize(1);
- assertThat(m).containsEntry("Code-Review", Short.valueOf((short) -1));
+ assertThat(m).containsExactly("Code-Review", Short.valueOf((short) -1));
+ }
+
+ @Test
+ @GerritConfig(name = "accounts.visibility", value = "NONE")
+ public void listVotesEvenWhenAccountsAreNotVisible() throws Exception {
+ PushOneCommit.Result r = createChange();
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.approve());
+
+ requestScopeOperations.setApiUser(user.id());
+
+ // check finding by address works
+ Map<String, Short> m = gApi.changes().id(r.getChangeId()).reviewer(admin.email()).votes();
+ assertThat(m).hasSize(1);
+ assertThat(m).containsEntry("Code-Review", Short.valueOf((short) 2));
+
+ // check finding by id works
+ m = gApi.changes().id(r.getChangeId()).reviewer(admin.id().toString()).votes();
+ assertThat(m).hasSize(1);
+ assertThat(m).containsEntry("Code-Review", Short.valueOf((short) 2));
}
@Test
diff --git a/plugins/hooks b/plugins/hooks
index f4bf0ff..6316be2 160000
--- a/plugins/hooks
+++ b/plugins/hooks
@@ -1 +1 @@
-Subproject commit f4bf0ffbd13a748cc46a3368a8fadcc2cbab6e21
+Subproject commit 6316be2828808dafc546ecd11c055396d0b4951b
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 eae25ae..75593e8 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
@@ -2354,7 +2354,7 @@
method: 'POST',
url: '/accounts/self/sshkeys',
body: key,
- contentType: 'plain/text',
+ contentType: 'text/plain',
reportUrlAsIs: true,
};
return this._restApiHelper.send(req)
diff --git a/tools/BUILD b/tools/BUILD
index 5531c3e..f0a4ffa 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -53,6 +53,7 @@
"-Xep:ExpectedExceptionChecker:ERROR",
"-Xep:Finally:ERROR",
"-Xep:FloatingPointLiteralPrecision:ERROR",
+ "-Xep:FormatStringAnnotation:ERROR",
"-Xep:FragmentInjection:ERROR",
"-Xep:FragmentNotInstantiable:ERROR",
"-Xep:FunctionalInterfaceClash:ERROR",
