Merge "Fix lint errors for promise-reject-errors"
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index 8078922..c326b66 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -2153,8 +2153,8 @@
link:config-gerrit.txt#accounts.visibility[visible] to the calling user are not
considered.
-In all cases _except_ a bare account ID, inactive accounts are not considered.
-Inactive accounts may only be referenced by bare ID.
+In all cases _except_ a bare account ID and `self`/`me`, inactive accounts are
+not considered. Inactive accounts should only be referenced by bare ID.
If the input is a bare account ID, this will always resolve to exactly
one account if there is a visible account with that ID, and zero accounts
diff --git a/WORKSPACE b/WORKSPACE
index e916a02..b06b971 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -1110,22 +1110,22 @@
maven_jar(
name = "mockito",
- artifact = "org.mockito:mockito-core:2.23.4",
- sha1 = "a35b6f8ffcfa786771eac7d7d903429e790fdf3f",
+ artifact = "org.mockito:mockito-core:2.24.0",
+ sha1 = "969a7bcb6f16e076904336ebc7ca171d412cc1f9",
)
-BYTE_BUDDY_VERSION = "1.9.3"
+BYTE_BUDDY_VERSION = "1.9.7"
maven_jar(
name = "byte-buddy",
artifact = "net.bytebuddy:byte-buddy:" + BYTE_BUDDY_VERSION,
- sha1 = "f32e510b239620852fc9a2387fac41fd053d6a4d",
+ sha1 = "8fea78fea6449e1738b675cb155ce8422661e237",
)
maven_jar(
name = "byte-buddy-agent",
artifact = "net.bytebuddy:byte-buddy-agent:" + BYTE_BUDDY_VERSION,
- sha1 = "f5b78c16cf4060664d80b6ca32d80dca4bd3d264",
+ sha1 = "8e7d1b599f4943851ffea125fd9780e572727fc0",
)
maven_jar(
diff --git a/java/com/google/gerrit/server/account/AccountResolver.java b/java/com/google/gerrit/server/account/AccountResolver.java
index 24a2596..9b4952b 100644
--- a/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/java/com/google/gerrit/server/account/AccountResolver.java
@@ -68,7 +68,7 @@
* </ol>
*
* <p>The result never includes accounts that are not visible to the calling user. It also never
- * includes inactive accounts, with one specific exception noted in method Javadoc.
+ * includes inactive accounts, with a small number of specific exceptions noted in method Javadoc.
*/
@Singleton
public class AccountResolver {
@@ -123,7 +123,7 @@
+ state.getAccount().getNameEmail(result.accountResolver().anonymousCowardName);
}
- private static boolean isSelf(String input) {
+ public static boolean isSelf(String input) {
return "self".equals(input) || "me".equals(input);
}
@@ -252,6 +252,11 @@
private class BySelf extends StringSearcher {
@Override
+ public boolean callerShouldFilterOutInactiveCandidates() {
+ return false;
+ }
+
+ @Override
public boolean callerMayAssumeCandidatesAreVisible() {
return true;
}
@@ -279,8 +284,6 @@
private class ByExactAccountId extends AccountIdSearcher {
@Override
public boolean callerShouldFilterOutInactiveCandidates() {
- // The only case where we *don't* enforce that the account is active is when passing an exact
- // numeric account ID.
return false;
}
@@ -497,9 +500,9 @@
*
* <ul>
* <li>The strings {@code "self"} and {@code "me"}, if the current user is an {@link
- * IdentifiedUser}.
- * <li>A bare account ID ({@code "18419"}). In this case, and <strong>only</strong> this case,
- * may return exactly one inactive account. This case short-circuits if the input matches.
+ * IdentifiedUser}. In this case, may return exactly one inactive account.
+ * <li>A bare account ID ({@code "18419"}). In this case, may return exactly one inactive
+ * account. This case short-circuits if the input matches.
* <li>An account ID in parentheses following a full name ({@code "Full Name (18419)"}). This
* case short-circuits if the input matches.
* <li>A username ({@code "username"}).
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index b62a1d7..93ece2b 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -16,6 +16,7 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.gerrit.reviewdb.client.Change.CHANGE_ID_PATTERN;
+import static com.google.gerrit.server.account.AccountResolver.isSelf;
import static com.google.gerrit.server.query.change.ChangeData.asChanges;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
@@ -951,6 +952,9 @@
@Operator
public Predicate<ChangeData> visibleto(String who)
throws QueryParseException, OrmException, IOException, ConfigInvalidException {
+ if (isSelf(who)) {
+ return is_visible();
+ }
try {
return Predicate.or(
parseAccount(who)
diff --git a/java/com/google/gerrit/server/restapi/project/ListProjects.java b/java/com/google/gerrit/server/restapi/project/ListProjects.java
index 1600fed..4bf1230 100644
--- a/java/com/google/gerrit/server/restapi/project/ListProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/ListProjects.java
@@ -359,7 +359,7 @@
natural(), p -> p.name, p -> showDescription ? p : nullifyDescription(p)));
} catch (OrmException | MethodNotAllowedException e) {
logger.atWarning().withCause(e).log(
- "Internal error while processing the query '{}' request", query);
+ "Internal error while processing the query '%s' request", query);
throw new BadRequestException("Internal error while processing the query request");
}
}
@@ -379,7 +379,7 @@
out.flush();
} catch (OrmException | MethodNotAllowedException e) {
logger.atWarning().withCause(e).log(
- "Internal error while processing the query '{}' request", query);
+ "Internal error while processing the query '%s' request", query);
throw new BadRequestException("Internal error while processing the query request");
}
}
diff --git a/javatests/com/google/gerrit/acceptance/server/account/AccountResolverIT.java b/javatests/com/google/gerrit/acceptance/server/account/AccountResolverIT.java
index 63fda12..d99fa72 100644
--- a/javatests/com/google/gerrit/acceptance/server/account/AccountResolverIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/account/AccountResolverIT.java
@@ -105,6 +105,19 @@
}
@Test
+ public void bySelfInactive() throws Exception {
+ gApi.accounts().id(user.id.get()).setActive(false);
+
+ requestScopeOperations.setApiUser(user.id);
+ assertThat(gApi.accounts().id("self").getActive()).isFalse();
+
+ Result result = resolveAsResult("self");
+ assertThat(result.asIdSet()).containsExactly(user.id);
+ assertThat(result.isSelf()).isTrue();
+ assertThat(result.asUniqueUser()).isSameAs(self.get());
+ }
+
+ @Test
public void byExactAccountId() throws Exception {
Account.Id existingId = accountOperations.newAccount().create();
Account.Id idWithExistingIdAsFullname =
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index af9a006..da62b7c 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -64,6 +64,7 @@
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.index.FieldDef;
@@ -77,6 +78,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.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
@@ -155,6 +157,7 @@
@Inject protected ChangeIndexer indexer;
@Inject protected IndexConfig indexConfig;
@Inject protected InMemoryRepositoryManager repoManager;
+ @Inject protected Provider<AnonymousUser> anonymousUserProvider;
@Inject protected Provider<InternalChangeQuery> queryProvider;
@Inject protected ChangeNotes.Factory notesFactory;
@Inject protected OneOffRequestContext oneOffRequestContext;
@@ -1857,6 +1860,24 @@
}
@Test
+ public void visibleToSelf() throws Exception {
+ TestRepository<Repo> repo = createProject("repo");
+ Change change1 = insert(repo, newChange(repo));
+ Change change2 = insert(repo, newChange(repo));
+
+ gApi.changes().id(change2.getChangeId()).setPrivate(true, "private");
+
+ String q = "project:repo";
+ assertQuery(q + " visibleto:self", change2, change1);
+ assertQuery(q + " visibleto:me", change2, change1);
+
+ // Anonymous user cannot see first user's private change.
+ requestContext.setContext(anonymousUserProvider::get);
+ assertQuery(q + " visibleto:self", change1);
+ assertQuery(q + " visibleto:me", change1);
+ }
+
+ @Test
public void byCommentBy() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(repo, newChange(repo));
@@ -3075,6 +3096,44 @@
assertQuery("project:repo+foo", change);
}
+ @Test
+ public void selfFailsForAnonymousUser() throws Exception {
+ for (String query : ImmutableList.of("assignee:self", "starredby:self", "is:starred")) {
+ assertQuery(query);
+ RequestContext oldContext = requestContext.setContext(anonymousUserProvider::get);
+
+ try {
+ requestContext.setContext(anonymousUserProvider::get);
+ assertThatAuthException(query)
+ .hasMessageThat()
+ .isEqualTo("Must be signed-in to use this operator");
+ } finally {
+ requestContext.setContext(oldContext);
+ }
+ }
+ }
+
+ @Test
+ public void selfSucceedsForInactiveAccount() throws Exception {
+ Account.Id user2 =
+ accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId();
+
+ TestRepository<Repo> repo = createProject("repo");
+ Change change = insert(repo, newChange(repo));
+ AssigneeInput ain = new AssigneeInput();
+ ain.assignee = user2.toString();
+ gApi.changes().id(change.getId().get()).setAssignee(ain);
+
+ RequestContext adminContext = requestContext.setContext(newRequestContext(user2));
+ assertQuery("assignee:self", change);
+
+ requestContext.setContext(adminContext);
+ gApi.accounts().id(user2.get()).setActive(false);
+
+ requestContext.setContext(newRequestContext(user2));
+ assertQuery("assignee:self", change);
+ }
+
protected ChangeInserter newChange(TestRepository<Repo> repo) throws Exception {
return newChange(repo, null, null, null, null, false);
}
@@ -3204,6 +3263,15 @@
}
}
+ protected ThrowableSubject assertThatAuthException(Object query) throws Exception {
+ try {
+ newQuery(query).get();
+ throw new AssertionError("expected AuthException for query: " + query);
+ } catch (AuthException e) {
+ return assertThat(e);
+ }
+ }
+
protected TestRepository<Repo> createProject(String name) throws Exception {
gApi.projects().create(name).get();
return new TestRepository<>(repoManager.openRepository(new Project.NameKey(name)));
diff --git a/plugins/delete-project b/plugins/delete-project
index 93e1145..59b9220 160000
--- a/plugins/delete-project
+++ b/plugins/delete-project
@@ -1 +1 @@
-Subproject commit 93e114582f6c9c4dfceaabc8353565635791336e
+Subproject commit 59b922061d791aaa0e65922023177d3347bea365
diff --git a/plugins/gitiles b/plugins/gitiles
index 09a5ff0..b90db6d 160000
--- a/plugins/gitiles
+++ b/plugins/gitiles
@@ -1 +1 @@
-Subproject commit 09a5ff01af4607b54b5a584fa87ae3d7901c7ee9
+Subproject commit b90db6dd4c43294e94523222a9ee57a65d11d058
diff --git a/plugins/webhooks b/plugins/webhooks
index 217c8ef..4d67d66 160000
--- a/plugins/webhooks
+++ b/plugins/webhooks
@@ -1 +1 @@
-Subproject commit 217c8ef00564e29309b76fdf71bc91c127ec67ec
+Subproject commit 4d67d6654a121462032c96bc45e193a4bcfed025
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
index 708a730..0ce622546 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
@@ -226,6 +226,7 @@
_computeItemNeedsReview(account, change, showReviewedState) {
return showReviewedState && !change.reviewed &&
+ !change.work_in_progress &&
this.changeIsOpen(change.status) &&
(!account || account._account_id != change.owner._account_id);
},
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
index d20d40a..d5b9aa9 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
@@ -231,11 +231,17 @@
status: 'ABANDONED',
owner: {_account_id: 0},
},
+ {
+ _number: 4,
+ status: 'NEW',
+ work_in_progress: true,
+ owner: {_account_id: 0},
+ },
];
flushAsynchronousOperations();
let elementItems = Polymer.dom(element.root).querySelectorAll(
'gr-change-list-item');
- assert.equal(elementItems.length, 4);
+ assert.equal(elementItems.length, 5);
for (let i = 0; i < elementItems.length; i++) {
assert.isFalse(elementItems[i].hasAttribute('needs-review'));
}
@@ -243,20 +249,22 @@
element.showReviewedState = true;
elementItems = Polymer.dom(element.root).querySelectorAll(
'gr-change-list-item');
- assert.equal(elementItems.length, 4);
+ assert.equal(elementItems.length, 5);
assert.isFalse(elementItems[0].hasAttribute('needs-review'));
assert.isTrue(elementItems[1].hasAttribute('needs-review'));
assert.isFalse(elementItems[2].hasAttribute('needs-review'));
assert.isFalse(elementItems[3].hasAttribute('needs-review'));
+ assert.isFalse(elementItems[4].hasAttribute('needs-review'));
element.account = {_account_id: 42};
elementItems = Polymer.dom(element.root).querySelectorAll(
'gr-change-list-item');
- assert.equal(elementItems.length, 4);
+ assert.equal(elementItems.length, 5);
assert.isFalse(elementItems[0].hasAttribute('needs-review'));
assert.isTrue(elementItems[1].hasAttribute('needs-review'));
assert.isFalse(elementItems[2].hasAttribute('needs-review'));
assert.isFalse(elementItems[3].hasAttribute('needs-review'));
+ assert.isFalse(elementItems[4].hasAttribute('needs-review'));
});
test('no changes', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
index f4328f9..bc072b1 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
@@ -22,6 +22,7 @@
'application/json': 'json',
'application/x-powershell': 'powershell',
'application/typescript': 'typescript',
+ 'application/xml': 'xml',
'application/xquery': 'xquery',
'application/x-erb': 'erb',
'text/css': 'css',
diff --git a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js
index 5006461..70eed78 100644
--- a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js
+++ b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js
@@ -29,6 +29,16 @@
type: Map,
value() { return new Map(); },
},
+ /**
+ * This map prevents importing the same endpoint twice.
+ * Without caching, if a plugin is loaded after the loaded plugins
+ * callback fires, it will be imported twice and appear twice on the page.
+ * @type {!Map}
+ */
+ _initializedPlugins: {
+ type: Map,
+ value() { return new Map(); },
+ },
},
detached() {
@@ -102,6 +112,9 @@
},
_initModule({moduleName, plugin, type, domHook}) {
+ if (this._initializedPlugins.get(plugin.name)) {
+ return;
+ }
let initPromise;
switch (type) {
case 'decorate':
@@ -115,6 +128,7 @@
console.warn('Unable to initialize module' +
`${moduleName} from ${plugin.getPluginName()}`);
}
+ this._initializedPlugins.set(plugin.name, true);
initPromise.then(el => {
domHook.handleInstanceAttached(el);
this._domHooks.set(el, domHook);
diff --git a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.js b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.js
index 8efd309..3afbe54 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.js
+++ b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.js
@@ -26,8 +26,8 @@
};
const WIP_TOOLTIP = 'This change isn\'t ready to be reviewed or submitted. ' +
- 'It will not appear in dashboards, and email notifications will be ' +
- 'silenced until the review is started.';
+ 'It will not appear on dashboards unless you are CC\'ed or assigned, ' +
+ 'and email notifications will be silenced until the review is started.';
const PRIVATE_TOOLTIP = 'This change is only visible to its owner and ' +
'current reviewers (or anyone with "View Private Changes" permission).';
