Merge "Give change metadata chips a disabled state"
diff --git a/Documentation/rest-api-config.txt b/Documentation/rest-api-config.txt
index 871a4d2..6596eda 100644
--- a/Documentation/rest-api-config.txt
+++ b/Documentation/rest-api-config.txt
@@ -52,6 +52,9 @@
)]}'
{
+ "accounts": {
+ "visibility": "ALL"
+ },
"auth": {
"auth_type": "LDAP",
"use_contributor_agreements": true,
@@ -190,45 +193,6 @@
}
----
-[[check-access]]
-=== Check Access
---
-'POST /config/server/check.access'
---
-
-Runs access checks for other users.
-
-Input for the access checks that should be run must be provided in
-the request body inside a
-link:#access-check-input[AccessCheckInput] entity.
-
-.Request
-----
- POST /config/server/check HTTP/1.0
- Content-Type: application/json; charset=UTF-8
-
- {
- "project": "medium",
- "account": "Kristen.Burns@gerritcodereview.com",
- "ref": "refs/heads/secret/bla"
- }
-----
-
-The result is a link:#access-check-info[AccessCheckInfo] entity
-detailing the read access of the given user for the given project (or
-project-ref combination).
-
-.Response
-----
- HTTP/1.1 200 OK
- Content-Type: application/json; charset=UTF-8
-
- )]}'
- {
- "message": "user Kristen Burns \u003cKristen.Burns@gerritcodereview.com\u003e (1000098) cannot see ref refs/heads/secret/master in project medium",
- "status": 403
- }
-----
[[confirm-email]]
=== Confirm Email
@@ -1294,33 +1258,19 @@
[[json-entities]]
== JSON Entities
-[[access-check-info]]
-=== AccessCheckInfo
-The `AccessCheckInfo` entity is the result of a
-an access check.
+[[accounts-config-info]]
+=== AccountsConfigInfo
+The `AccountsConfigInfo` entity contains information about Gerrit
+configuration from the link:config-gerrit.html#accounts[accounts]
+section.
-[options="header",cols="1,^1,5"]
-|=========================================
-|Field Name ||Description
-|`status`||The HTTP status code for the access.
-200 means success, 403 means denied and 404 means the project does not
-exist.
-|`message`|optional|A clarifying message if `status` is not 200.
-|=========================================
-
-[[access-check-input]]
-=== AccessCheckInput
-The `AccessCheckInput` entity is a tuple of (account, project) or
-(account, project, ref) for which we want to check access.
-
-[options="header",cols="1,^1,5"]
-|=========================================
-|Field Name ||Description
-|`account`||The account for which to check access
-|`project`||The project for which to check access
-|`ref`|optional|The refname for which to check access
-|=========================================
-
+[options="header",cols="1,6"]
+|=============================
+|Field Name |Description
+|`visibility` |
+link:config-gerrit.html#accounts.visibility[Visibility setting for
+accounts].
+|=============================
[[auth-info]]
=== AuthInfo
@@ -1743,6 +1693,10 @@
[options="header",cols="1,^1,5"]
|=======================================
|Field Name ||Description
+|`accounts` ||
+Information about the configuration from the
+link:config-gerrit.html#accounts[accounts] section as
+link:#accounts-config-info[AccountsConfigInfo] entity.
|`auth` ||
Information about the authentication configuration as
link:#auth-info[AuthInfo] entity.
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index f083d073..9d76d34 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -1112,7 +1112,6 @@
}
----
-
[[create-access-change]]
=== Create Access Rights Change for review.
--
@@ -1178,6 +1177,46 @@
}
----
+[[check-access]]
+=== Check Access
+--
+'POST /projects/MyProject/check.access'
+--
+
+Runs access checks for other users. This requires the
+link:access-control.html#capability_administrateServer[Administrate Server]
+global capability.
+
+Input for the access checks that should be run must be provided in
+the request body inside a
+link:#access-check-input[AccessCheckInput] entity.
+
+.Request
+----
+ POST /projects/MyProject/check.access HTTP/1.0
+ Content-Type: application/json; charset=UTF-8
+
+ {
+ "account": "Kristen.Burns@gerritcodereview.com",
+ "ref": "refs/heads/secret/bla"
+ }
+----
+
+The result is a link:#access-check-info[AccessCheckInfo] entity
+detailing the read access of the given user for the given project (or
+project-ref combination).
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "message": "user Kristen Burns \u003cKristen.Burns@gerritcodereview.com\u003e (1000098) cannot see ref refs/heads/secret/bla in project MyProject",
+ "status": 403
+ }
+----
[[index]]
=== Index all changes in a project
@@ -2564,6 +2603,31 @@
[[json-entities]]
== JSON Entities
+[[access-check-info]]
+=== AccessCheckInfo
+
+The `AccessCheckInfo` entity is the result of an access check.
+
+[options="header",cols="1,^1,5"]
+|=========================================
+|Field Name ||Description
+|`status` ||The HTTP status code for the access.
+200 means success, 403 means denied and 404 means the project does not exist.
+|`message` |optional|A clarifying message if `status` is not 200.
+|=========================================
+
+[[access-check-input]]
+=== AccessCheckInput
+The `AccessCheckInput` entity is either an account or
+(account, ref) tuple for which we want to check access.
+
+[options="header",cols="1,^1,5"]
+|=========================================
+|Field Name ||Description
+|`account` ||The account for which to check access
+|`ref` |optional|The refname for which to check access
+|=========================================
+
[[ban-input]]
=== BanInput
The `BanInput` entity contains information for banning commits in a
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 7f7fe00..d399e2b 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
@@ -3507,6 +3507,42 @@
}
@Test
+ public void mute() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ PushOneCommit.Result r = createChange();
+
+ AddReviewerInput in = new AddReviewerInput();
+ in.reviewer = user.email;
+ gApi.changes().id(r.getChangeId()).addReviewer(in);
+
+ setApiUser(user);
+ assertThat(gApi.changes().id(r.getChangeId()).muted()).isFalse();
+ gApi.changes().id(r.getChangeId()).mute(true);
+ assertThat(gApi.changes().id(r.getChangeId()).muted()).isTrue();
+
+ setApiUser(user2);
+ sender.clear();
+ amendChange(r.getChangeId());
+
+ setApiUser(user);
+ assertThat(gApi.changes().id(r.getChangeId()).muted()).isFalse();
+
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ assertThat(messages.get(0).rcpt()).containsExactly(user.emailAddress);
+ }
+
+ @Test
+ public void cannotMuteOwnChange() throws Exception {
+ String changeId = createChange().getChangeId();
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("cannot mute own change");
+ gApi.changes().id(changeId).mute(true);
+ }
+
+ @Test
public void cannotSetInvalidLabel() throws Exception {
String changeId = createChange().getChangeId();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/CheckAccessIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/CheckAccessIT.java
similarity index 63%
rename from gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/CheckAccessIT.java
rename to gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/CheckAccessIT.java
index 53c89c5..b471efc 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/CheckAccessIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/CheckAccessIT.java
@@ -12,24 +12,24 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.acceptance.rest.account;
+package com.google.gerrit.acceptance.api.project;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.config.AccessCheckInfo;
import com.google.gerrit.extensions.api.config.AccessCheckInput;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.group.SystemGroupBackend;
import java.util.List;
-import java.util.Map;
import org.junit.Before;
import org.junit.Test;
@@ -79,46 +79,59 @@
}
@Test
- public void invalidInputs() {
- List<AccessCheckInput> inputs =
- ImmutableList.of(
- new AccessCheckInput(),
- new AccessCheckInput(user.email, null, null),
- new AccessCheckInput(null, normalProject.toString(), null),
- new AccessCheckInput("doesnotexist@invalid.com", normalProject.toString(), null));
- for (AccessCheckInput input : inputs) {
- try {
- gApi.config().server().checkAccess(input);
- fail(String.format("want RestApiException for %s", newGson().toJson(input)));
- } catch (RestApiException e) {
+ public void emptyInput() throws Exception {
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("input requires 'account'");
+ gApi.projects().name(normalProject.get()).checkAccess(new AccessCheckInput());
+ }
- }
+ @Test
+ public void nonexistentEmail() throws Exception {
+ exception.expect(UnprocessableEntityException.class);
+ exception.expectMessage("cannot find account doesnotexist@invalid.com");
+ gApi.projects()
+ .name(normalProject.get())
+ .checkAccess(new AccessCheckInput("doesnotexist@invalid.com", null));
+ }
+
+ private static class TestCase {
+ AccessCheckInput input;
+ String project;
+ int want;
+
+ TestCase(String mail, String project, String ref, int want) {
+ this.input = new AccessCheckInput(mail, ref);
+ this.project = project;
+ this.want = want;
}
}
@Test
- public void accessible() {
- Map<AccessCheckInput, Integer> inputs =
- ImmutableMap.of(
- new AccessCheckInput(user.email, normalProject.get(), null), 200,
- new AccessCheckInput(user.email, secretProject.get(), null), 403,
- new AccessCheckInput(user.email, "nonexistent", null), 404,
- new AccessCheckInput(privilegedUser.email, normalProject.get(), null), 200,
- new AccessCheckInput(privilegedUser.email, secretProject.get(), null), 200);
+ public void accessible() throws Exception {
+ List<TestCase> inputs =
+ ImmutableList.of(
+ new TestCase(user.email, normalProject.get(), null, 200),
+ new TestCase(user.email, secretProject.get(), null, 403),
+ new TestCase(user.email, secretRefProject.get(), "refs/heads/secret/master", 403),
+ new TestCase(
+ privilegedUser.email, secretRefProject.get(), "refs/heads/secret/master", 200),
+ new TestCase(privilegedUser.email, normalProject.get(), null, 200),
+ new TestCase(privilegedUser.email, secretProject.get(), null, 200));
- for (Map.Entry<AccessCheckInput, Integer> entry : inputs.entrySet()) {
- String in = newGson().toJson(entry.getKey());
+ for (TestCase tc : inputs) {
+ String in = newGson().toJson(tc.input);
AccessCheckInfo info = null;
try {
- info = gApi.config().server().checkAccess(entry.getKey());
+ info = gApi.projects().name(tc.project).checkAccess(tc.input);
} catch (RestApiException e) {
- fail(String.format("check.check(%s): exception %s", in, e));
+ fail(String.format("check.access(%s, %s): exception %s", tc.project, in, e));
}
- int want = entry.getValue();
+ int want = tc.want;
if (want != info.status) {
- fail(String.format("check.access(%s) = %d, want %d", in, info.status, want));
+ fail(
+ String.format("check.access(%s, %s) = %d, want %d", tc.project, in, info.status, want));
}
switch (want) {
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/DashboardIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/DashboardIT.java
index 64f4106..bfe6b8b 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/DashboardIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/DashboardIT.java
@@ -30,6 +30,6 @@
@Test
public void dashboardDoesNotExist() throws Exception {
exception.expect(ResourceNotFoundException.class);
- gApi.projects().name(project.get()).dashboard("dashboard").get();
+ gApi.projects().name(project.get()).dashboard("my:dashboard").get();
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index 66dcaf0..e188afd 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -408,7 +408,7 @@
revisionInfo =
changeJsonFactory
.create(opts)
- .getRevisionInfo(cd.changeControl(), cd.patchSet(new PatchSet.Id(changeId, 1)));
+ .getRevisionInfo(cd, cd.patchSet(new PatchSet.Id(changeId, 1)));
}
private void visitedCurrentRevisionActionsAssertions(
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/HashtagsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/HashtagsIT.java
index b8c2cf8..08f0699 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/HashtagsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/HashtagsIT.java
@@ -30,6 +30,7 @@
import com.google.gerrit.extensions.api.changes.HashtagsInput;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.testutil.TestTimeUtil;
import org.junit.AfterClass;
import org.junit.Before;
@@ -77,6 +78,15 @@
}
@Test
+ public void addInvalidHashtag() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("hashtags may not contain commas");
+ addHashtags(r, "invalid,hashtag");
+ }
+
+ @Test
public void addMultipleHashtags() throws Exception {
PushOneCommit.Result r = createChange();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java
index 9a23a25..492245b 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java
@@ -23,6 +23,7 @@
import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.extensions.client.AccountFieldName;
import com.google.gerrit.extensions.client.AuthType;
+import com.google.gerrit.extensions.common.AccountVisibility;
import com.google.gerrit.extensions.common.InstallPluginInput;
import com.google.gerrit.extensions.common.ServerInfo;
import com.google.gerrit.server.config.AllProjectsNameProvider;
@@ -36,6 +37,9 @@
"Gerrit.install(function(self){});\n".getBytes(UTF_8);
@Test
+ // accounts
+ @GerritConfig(name = "accounts.visibility", value = "VISIBLE_GROUP")
+
// auth
@GerritConfig(name = "auth.type", value = "HTTP")
@GerritConfig(name = "auth.contributorAgreements", value = "true")
@@ -78,6 +82,9 @@
public void serverConfig() throws Exception {
ServerInfo i = gApi.config().server().getInfo();
+ // accounts
+ assertThat(i.accounts.visibility).isEqualTo(AccountVisibility.VISIBLE_GROUP);
+
// auth
assertThat(i.auth.authType).isEqualTo(AuthType.HTTP);
assertThat(i.auth.editableAccountFields)
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
index 7081245..96dfac1 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
@@ -126,6 +126,13 @@
void mute(boolean mute) throws RestApiException;
/**
+ * Check if this change is muted.
+ *
+ * @return true if the change is muted.
+ */
+ boolean muted() throws RestApiException;
+
+ /**
* Create a new change that reverts this change.
*
* @see Changes#id(int)
@@ -579,6 +586,11 @@
}
@Override
+ public boolean muted() throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public PureRevertInfo pureRevert() throws RestApiException {
throw new NotImplementedException();
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/config/AccessCheckInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/config/AccessCheckInput.java
index 80a537c..7b7c19d 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/config/AccessCheckInput.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/config/AccessCheckInput.java
@@ -18,13 +18,11 @@
public class AccessCheckInput {
public String account;
- public String project;
@Nullable public String ref;
- public AccessCheckInput(String account, String project, @Nullable String ref) {
+ public AccessCheckInput(String account, @Nullable String ref) {
this.account = account;
- this.project = project;
this.ref = ref;
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/config/Server.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/config/Server.java
index de59cee..ba81698 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/config/Server.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/config/Server.java
@@ -36,8 +36,6 @@
ConsistencyCheckInfo checkConsistency(ConsistencyCheckInput in) throws RestApiException;
- AccessCheckInfo checkAccess(AccessCheckInput in) throws RestApiException;
-
/**
* A default implementation which allows source compatibility when adding new methods to the
* interface.
@@ -79,10 +77,5 @@
public ConsistencyCheckInfo checkConsistency(ConsistencyCheckInput in) throws RestApiException {
throw new NotImplementedException();
}
-
- @Override
- public AccessCheckInfo checkAccess(AccessCheckInput in) throws RestApiException {
- throw new NotImplementedException();
- }
}
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/DashboardInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/DashboardInfo.java
index 775224c..f629294 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/DashboardInfo.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/DashboardInfo.java
@@ -14,8 +14,6 @@
package com.google.gerrit.extensions.api.projects;
-import com.google.common.base.Joiner;
-import com.google.gerrit.extensions.restapi.Url;
import java.util.ArrayList;
import java.util.List;
@@ -34,9 +32,5 @@
public String title;
public List<DashboardSectionInfo> sections = new ArrayList<>();
- public DashboardInfo(String ref, String name) {
- this.ref = ref;
- this.path = name;
- this.id = Joiner.on(':').join(Url.encode(ref), Url.encode(path));
- }
+ public DashboardInfo() {}
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/ProjectApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
index 8687ba3e..a61b68a 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
@@ -16,6 +16,8 @@
import com.google.gerrit.extensions.api.access.ProjectAccessInfo;
import com.google.gerrit.extensions.api.access.ProjectAccessInput;
+import com.google.gerrit.extensions.api.config.AccessCheckInfo;
+import com.google.gerrit.extensions.api.config.AccessCheckInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.extensions.restapi.NotImplementedException;
@@ -39,6 +41,8 @@
ChangeInfo accessChange(ProjectAccessInput p) throws RestApiException;
+ AccessCheckInfo checkAccess(AccessCheckInput in) throws RestApiException;
+
ConfigInfo config() throws RestApiException;
ConfigInfo config(ConfigInput in) throws RestApiException;
@@ -181,11 +185,21 @@
}
@Override
+ public ProjectAccessInfo access(ProjectAccessInput p) throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public ChangeInfo accessChange(ProjectAccessInput input) throws RestApiException {
throw new NotImplementedException();
}
@Override
+ public AccessCheckInfo checkAccess(AccessCheckInput in) throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public ConfigInfo config() throws RestApiException {
throw new NotImplementedException();
}
@@ -196,11 +210,6 @@
}
@Override
- public ProjectAccessInfo access(ProjectAccessInput p) throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public void description(DescriptionInput in) throws RestApiException {
throw new NotImplementedException();
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountVisibility.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountVisibility.java
similarity index 95%
rename from gerrit-server/src/main/java/com/google/gerrit/server/account/AccountVisibility.java
rename to gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountVisibility.java
index 9957134..32ec318 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountVisibility.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountVisibility.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.account;
+package com.google.gerrit.extensions.common;
/** Visibility level of other accounts to a given user. */
public enum AccountVisibility {
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountsInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountsInfo.java
new file mode 100644
index 0000000..e1c2825
--- /dev/null
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountsInfo.java
@@ -0,0 +1,19 @@
+// Copyright (C) 2017 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.common;
+
+public class AccountsInfo {
+ public AccountVisibility visibility;
+}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ServerInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ServerInfo.java
index 2eb63f4..8904f0a 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ServerInfo.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ServerInfo.java
@@ -17,6 +17,7 @@
import java.util.Map;
public class ServerInfo {
+ public AccountsInfo accounts;
public AuthInfo auth;
public ChangeConfigInfo change;
public DownloadInfo download;
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/SetDashboardInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/SetDashboardInput.java
new file mode 100644
index 0000000..13d2b9d
--- /dev/null
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/SetDashboardInput.java
@@ -0,0 +1,22 @@
+// Copyright (C) 2017 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.common;
+
+import com.google.gerrit.extensions.restapi.DefaultInput;
+
+public class SetDashboardInput {
+ @DefaultInput public String id;
+ public String commitMessage;
+}
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index a28aa64..32d8dd8 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.extensions.api.projects.CommentLinkInfo;
+import com.google.gerrit.extensions.common.AccountVisibility;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.DynamicSet;
@@ -29,7 +30,6 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCacheImpl;
-import com.google.gerrit.server.account.AccountVisibility;
import com.google.gerrit.server.account.AccountVisibilityProvider;
import com.google.gerrit.server.account.CapabilityCollection;
import com.google.gerrit.server.account.FakeRealm;
@@ -68,6 +68,7 @@
import com.google.gerrit.server.project.ProjectCacheImpl;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SectionSortCache;
+import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryProcessor;
import com.google.gerrit.server.update.BatchUpdate;
@@ -167,6 +168,7 @@
factory(CapabilityCollection.Factory.class);
factory(ChangeData.AssistedFactory.class);
factory(ProjectState.Factory.class);
+ factory(SubmitRuleEvaluator.Factory.class);
bind(ChangeJson.Factory.class).toProvider(Providers.<ChangeJson.Factory>of(null));
bind(AccountVisibility.class).toProvider(AccountVisibilityProvider.class).in(SINGLETON);
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 61ab3a1..13c24e0 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
@@ -334,30 +334,32 @@
return MUTE_LABEL + "/" + change.currentPatchSetId().get();
}
- public void mute(Account.Id accountId, Project.NameKey project, Change change)
- throws OrmException, IllegalLabelException {
+ public void mute(ChangeResource rsrc) throws OrmException, IllegalLabelException {
star(
- accountId,
- project,
- change.getId(),
- ImmutableSet.of(getMuteLabel(change)),
+ rsrc.getUser().asIdentifiedUser().getAccountId(),
+ rsrc.getProject(),
+ rsrc.getChange().getId(),
+ ImmutableSet.of(getMuteLabel(rsrc.getChange())),
ImmutableSet.of());
}
- public void unmute(Account.Id accountId, Project.NameKey project, Change change)
- throws OrmException, IllegalLabelException {
+ public void unmute(ChangeResource rsrc) throws OrmException, IllegalLabelException {
star(
- accountId,
- project,
- change.getId(),
+ rsrc.getUser().asIdentifiedUser().getAccountId(),
+ rsrc.getProject(),
+ rsrc.getChange().getId(),
ImmutableSet.of(),
- ImmutableSet.of(getMuteLabel(change)));
+ ImmutableSet.of(getMuteLabel(rsrc.getChange())));
}
public boolean isMutedBy(Change change, Account.Id accountId) throws OrmException {
return getLabels(accountId, change.getId()).contains(getMuteLabel(change));
}
+ public boolean isMuted(ChangeResource rsrc) throws OrmException {
+ return isMutedBy(rsrc.getChange(), rsrc.getUser().asIdentifiedUser().getAccountId());
+ }
+
private static StarRef readLabels(Repository repo, String refName) throws IOException {
Ref ref = repo.exactRef(refName);
if (ref == null) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountControl.java
index bb118a3..6f25703 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountControl.java
@@ -18,6 +18,7 @@
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.errors.NoSuchGroupException;
+import com.google.gerrit.extensions.common.AccountVisibility;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountVisibilityProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountVisibilityProvider.java
index 4521cd5..ef0a917 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountVisibilityProvider.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountVisibilityProvider.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.account;
+import com.google.gerrit.extensions.common.AccountVisibility;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
import com.google.inject.Provider;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/QueryAccounts.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/QueryAccounts.java
index cbfb7f5..e6ac0f6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/QueryAccounts.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/QueryAccounts.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.client.ListAccountsOption;
import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.common.AccountVisibility;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.RestReadView;
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 d006c7e..ab47f3b 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
@@ -677,7 +677,7 @@
@Override
public boolean ignored() throws RestApiException {
try {
- return stars.isIgnoredBy(change.getId(), change.getUser().getAccountId());
+ return stars.isIgnored(change);
} catch (OrmException e) {
throw asRestApiException("Cannot check if ignored", e);
}
@@ -699,6 +699,15 @@
}
@Override
+ public boolean muted() throws RestApiException {
+ try {
+ return stars.isMuted(change);
+ } catch (OrmException e) {
+ throw asRestApiException("Cannot check if muted", e);
+ }
+ }
+
+ @Override
public PureRevertInfo pureRevert() throws RestApiException {
return pureRevert(null);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/config/ServerImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/config/ServerImpl.java
index 87118c7..2148d97 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/config/ServerImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/config/ServerImpl.java
@@ -17,8 +17,6 @@
import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
import com.google.gerrit.common.Version;
-import com.google.gerrit.extensions.api.config.AccessCheckInfo;
-import com.google.gerrit.extensions.api.config.AccessCheckInput;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInput;
import com.google.gerrit.extensions.api.config.Server;
@@ -26,7 +24,6 @@
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.common.ServerInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.server.config.CheckAccess;
import com.google.gerrit.server.config.CheckConsistency;
import com.google.gerrit.server.config.ConfigResource;
import com.google.gerrit.server.config.GetDiffPreferences;
@@ -46,7 +43,6 @@
private final SetDiffPreferences setDiffPreferences;
private final GetServerInfo getServerInfo;
private final Provider<CheckConsistency> checkConsistency;
- private final Provider<CheckAccess> checkAccess;
@Inject
ServerImpl(
@@ -55,15 +51,13 @@
GetDiffPreferences getDiffPreferences,
SetDiffPreferences setDiffPreferences,
GetServerInfo getServerInfo,
- Provider<CheckConsistency> checkConsistency,
- Provider<CheckAccess> checkAccess) {
+ Provider<CheckConsistency> checkConsistency) {
this.getPreferences = getPreferences;
this.setPreferences = setPreferences;
this.getDiffPreferences = getDiffPreferences;
this.setDiffPreferences = setDiffPreferences;
this.getServerInfo = getServerInfo;
this.checkConsistency = checkConsistency;
- this.checkAccess = checkAccess;
}
@Override
@@ -126,13 +120,4 @@
throw asRestApiException("Cannot check consistency", e);
}
}
-
- @Override
- public AccessCheckInfo checkAccess(AccessCheckInput in) throws RestApiException {
- try {
- return checkAccess.get().apply(new ConfigResource(), in);
- } catch (Exception e) {
- throw asRestApiException("Cannot check access", e);
- }
- }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/DashboardApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/DashboardApiImpl.java
index 28ed494..58cb59e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/DashboardApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/DashboardApiImpl.java
@@ -34,24 +34,24 @@
public class DashboardApiImpl implements DashboardApi {
interface Factory {
- DashboardApiImpl create(ProjectResource project, String name);
+ DashboardApiImpl create(ProjectResource project, String id);
}
private final DashboardsCollection dashboards;
- private final Provider<GetDashboard> getDashboard;
+ private final Provider<GetDashboard> get;
private final ProjectResource project;
- private final String name;
+ private final String id;
@Inject
DashboardApiImpl(
DashboardsCollection dashboards,
- Provider<GetDashboard> getDashboard,
+ Provider<GetDashboard> get,
@Assisted ProjectResource project,
- @Assisted String name) {
+ @Assisted String id) {
this.dashboards = dashboards;
- this.getDashboard = getDashboard;
+ this.get = get;
this.project = project;
- this.name = name;
+ this.id = id;
}
@Override
@@ -62,7 +62,7 @@
@Override
public DashboardInfo get(boolean inherited) throws RestApiException {
try {
- return getDashboard.get().setInherited(inherited).apply(resource());
+ return get.get().setInherited(inherited).apply(resource());
} catch (IOException | PermissionBackendException | ConfigInvalidException e) {
throw asRestApiException("Cannot read dashboard", e);
}
@@ -71,6 +71,6 @@
private DashboardResource resource()
throws ResourceNotFoundException, IOException, ConfigInvalidException,
PermissionBackendException {
- return dashboards.parse(project, IdString.fromDecoded(name));
+ return dashboards.parse(project, IdString.fromDecoded(id));
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
index 60dc474..5012280 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
@@ -19,6 +19,8 @@
import com.google.gerrit.extensions.api.access.ProjectAccessInfo;
import com.google.gerrit.extensions.api.access.ProjectAccessInput;
+import com.google.gerrit.extensions.api.config.AccessCheckInfo;
+import com.google.gerrit.extensions.api.config.AccessCheckInput;
import com.google.gerrit.extensions.api.projects.BranchApi;
import com.google.gerrit.extensions.api.projects.BranchInfo;
import com.google.gerrit.extensions.api.projects.ChildProjectApi;
@@ -44,6 +46,7 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.project.CheckAccess;
import com.google.gerrit.server.project.ChildProjectsCollection;
import com.google.gerrit.server.project.CommitsCollection;
import com.google.gerrit.server.project.CreateAccessChange;
@@ -100,6 +103,7 @@
private final CommitsCollection commitsCollection;
private final CommitApiImpl.Factory commitApi;
private final DashboardApiImpl.Factory dashboardApi;
+ private final CheckAccess checkAccess;
@AssistedInject
ProjectApiImpl(
@@ -127,6 +131,7 @@
CommitsCollection commitsCollection,
CommitApiImpl.Factory commitApi,
DashboardApiImpl.Factory dashboardApi,
+ CheckAccess checkAccess,
@Assisted ProjectResource project) {
this(
user,
@@ -154,6 +159,7 @@
commitsCollection,
commitApi,
dashboardApi,
+ checkAccess,
null);
}
@@ -183,6 +189,7 @@
CommitsCollection commitsCollection,
CommitApiImpl.Factory commitApi,
DashboardApiImpl.Factory dashboardApi,
+ CheckAccess checkAccess,
@Assisted String name) {
this(
user,
@@ -210,6 +217,7 @@
commitsCollection,
commitApi,
dashboardApi,
+ checkAccess,
name);
}
@@ -239,6 +247,7 @@
CommitsCollection commitsCollection,
CommitApiImpl.Factory commitApi,
DashboardApiImpl.Factory dashboardApi,
+ CheckAccess checkAccess,
String name) {
this.user = user;
this.permissionBackend = permissionBackend;
@@ -251,7 +260,6 @@
this.children = children;
this.projectJson = projectJson;
this.project = project;
- this.name = name;
this.branchApi = branchApiFactory;
this.tagApi = tagApiFactory;
this.getAccess = getAccess;
@@ -266,6 +274,8 @@
this.commitApi = commitApi;
this.createAccessChange = createAccessChange;
this.dashboardApi = dashboardApi;
+ this.checkAccess = checkAccess;
+ this.name = name;
}
@Override
@@ -314,6 +324,15 @@
}
@Override
+ public AccessCheckInfo checkAccess(AccessCheckInput in) throws RestApiException {
+ try {
+ return checkAccess.apply(checkExists(), in);
+ } catch (Exception e) {
+ throw asRestApiException("Cannot check access rights", e);
+ }
+ }
+
+ @Override
public ProjectAccessInfo access(ProjectAccessInput p) throws RestApiException {
try {
return setAccess.apply(checkExists(), p);
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 ffea604..18e373f 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
@@ -119,7 +119,6 @@
import com.google.gerrit.server.permissions.LabelPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.RemoveReviewerControl;
@@ -166,7 +165,7 @@
ChangeField.SUBMIT_RULE_OPTIONS_STRICT.toBuilder().fastEvalLabels(true).build();
public static final ImmutableSet<ListChangesOption> REQUIRE_LAZY_LOAD =
- ImmutableSet.of(ALL_REVISIONS, MESSAGES);
+ ImmutableSet.of(ALL_REVISIONS, CHANGE_ACTIONS, CHECK, CURRENT_ACTIONS, MESSAGES);
@Singleton
public static class Factory {
@@ -430,9 +429,9 @@
}
private ChangeInfo checkOnly(ChangeData cd) {
- ChangeControl ctl;
+ ChangeNotes notes;
try {
- ctl = cd.changeControl().forUser(userProvider.get());
+ notes = cd.notes();
} catch (OrmException e) {
String msg = "Error loading change";
log.warn(msg + " " + cd.getId(), e);
@@ -444,7 +443,7 @@
return info;
}
- ConsistencyChecker.Result result = checkerProvider.get().check(ctl.getNotes(), fix);
+ ConsistencyChecker.Result result = checkerProvider.get().check(notes, fix);
ChangeInfo info;
Change c = result.change();
if (c != null) {
@@ -477,10 +476,9 @@
PermissionBackendException {
ChangeInfo out = new ChangeInfo();
CurrentUser user = userProvider.get();
- ChangeControl ctl = cd.changeControl().forUser(user);
if (has(CHECK)) {
- out.problems = checkerProvider.get().check(ctl.getNotes(), fix).problems();
+ out.problems = checkerProvider.get().check(cd.notes(), fix).problems();
// If any problems were fixed, the ChangeData needs to be reloaded.
for (ProblemInfo p : out.problems) {
if (p.status == ProblemInfo.Status.FIXED) {
@@ -549,7 +547,7 @@
}
}
- out.labels = labelsFor(perm, ctl, cd, has(LABELS), has(DETAILED_LABELS));
+ out.labels = labelsFor(perm, cd, has(LABELS), has(DETAILED_LABELS));
if (out.labels != null && has(DETAILED_LABELS)) {
// If limited to specific patch sets but not the current patch set, don't
@@ -564,7 +562,7 @@
out.reviewers = reviewerMap(cd.reviewers(), cd.reviewersByEmail(), false);
out.pendingReviewers = reviewerMap(cd.pendingReviewers(), cd.pendingReviewersByEmail(), true);
- out.removableReviewers = removableReviewers(ctl, out);
+ out.removableReviewers = removableReviewers(cd, out);
}
setSubmitter(cd, out);
@@ -585,14 +583,14 @@
src = null;
}
if (needMessages) {
- out.messages = messages(ctl, cd, src);
+ out.messages = messages(cd, src);
}
finish(out);
// This block must come after the ChangeInfo is mostly populated, since
// it will be passed to ActionVisitors as-is.
if (needRevisions) {
- out.revisions = revisions(ctl, cd, src, limitToPsId, out);
+ out.revisions = revisions(cd, src, limitToPsId, out);
if (out.revisions != null) {
for (Map.Entry<String, RevisionInfo> entry : out.revisions.entrySet()) {
if (entry.getValue().isCurrent) {
@@ -604,7 +602,7 @@
}
if (has(CURRENT_ACTIONS) || has(CHANGE_ACTIONS)) {
- actionJson.addChangeActions(out, ctl.getNotes());
+ actionJson.addChangeActions(out, cd.notes());
}
if (has(TRACKING_IDS)) {
@@ -658,20 +656,12 @@
}
private Map<String, LabelInfo> labelsFor(
- PermissionBackend.ForChange perm,
- ChangeControl ctl,
- ChangeData cd,
- boolean standard,
- boolean detailed)
+ PermissionBackend.ForChange perm, ChangeData cd, boolean standard, boolean detailed)
throws OrmException, PermissionBackendException {
if (!standard && !detailed) {
return null;
}
- if (ctl == null) {
- return null;
- }
-
LabelTypes labelTypes = cd.getLabelTypes();
Map<String, LabelWithStatus> withStatus =
cd.change().getStatus() == Change.Status.MERGED
@@ -1095,8 +1085,8 @@
return result;
}
- private Collection<ChangeMessageInfo> messages(
- ChangeControl ctl, ChangeData cd, Map<PatchSet.Id, PatchSet> map) throws OrmException {
+ private Collection<ChangeMessageInfo> messages(ChangeData cd, Map<PatchSet.Id, PatchSet> map)
+ throws OrmException {
List<ChangeMessage> messages = cmUtil.byChange(db.get(), cd.notes());
if (messages.isEmpty()) {
return Collections.emptyList();
@@ -1106,7 +1096,7 @@
for (ChangeMessage message : messages) {
PatchSet.Id patchNum = message.getPatchSetId();
PatchSet ps = patchNum != null ? map.get(patchNum) : null;
- if (patchNum == null || ctl.isPatchVisible(ps, db.get())) {
+ if (patchNum == null || cd.changeControl().isPatchVisible(ps, db.get())) {
ChangeMessageInfo cmi = new ChangeMessageInfo();
cmi.id = message.getKey().get();
cmi.author = accountLoader.get(message.getAuthor());
@@ -1124,8 +1114,8 @@
return result;
}
- private Collection<AccountInfo> removableReviewers(ChangeControl ctl, ChangeInfo out)
- throws PermissionBackendException, NoSuchChangeException {
+ private Collection<AccountInfo> removableReviewers(ChangeData cd, ChangeInfo out)
+ throws PermissionBackendException, NoSuchChangeException, OrmException {
// Although this is called removableReviewers, this method also determines
// which CCs are removable.
//
@@ -1147,7 +1137,7 @@
Account.Id id = new Account.Id(ai._accountId);
if (removeReviewerControl.testRemoveReviewer(
- ctl.getNotes(), ctl.getUser(), id, MoreObjects.firstNonNull(ai.value, 0))) {
+ cd, userProvider.get(), id, MoreObjects.firstNonNull(ai.value, 0))) {
removable.add(id);
} else {
fixed.add(id);
@@ -1164,7 +1154,7 @@
for (AccountInfo ai : ccs) {
if (ai._accountId != null) {
Account.Id id = new Account.Id(ai._accountId);
- if (removeReviewerControl.testRemoveReviewer(ctl.getNotes(), ctl.getUser(), id, 0)) {
+ if (removeReviewerControl.testRemoveReviewer(cd, userProvider.get(), id, 0)) {
removable.add(id);
}
}
@@ -1208,9 +1198,9 @@
}
@Nullable
- private Repository openRepoIfNecessary(ChangeControl ctl) throws IOException {
+ private Repository openRepoIfNecessary(Project.NameKey project) throws IOException {
if (has(ALL_COMMITS) || has(CURRENT_COMMIT) || has(COMMIT_FOOTERS)) {
- return repoManager.openRepository(ctl.getProject().getNameKey());
+ return repoManager.openRepository(project);
}
return null;
}
@@ -1221,14 +1211,13 @@
}
private Map<String, RevisionInfo> revisions(
- ChangeControl ctl,
ChangeData cd,
Map<PatchSet.Id, PatchSet> map,
Optional<PatchSet.Id> limitToPsId,
ChangeInfo changeInfo)
throws PatchListNotAvailableException, GpgException, OrmException, IOException {
Map<String, RevisionInfo> res = new LinkedHashMap<>();
- try (Repository repo = openRepoIfNecessary(ctl);
+ try (Repository repo = openRepoIfNecessary(cd.project());
RevWalk rw = newRevWalk(repo)) {
for (PatchSet in : map.values()) {
PatchSet.Id id = in.getId();
@@ -1238,10 +1227,10 @@
} else if (limitToPsId.isPresent()) {
want = id.equals(limitToPsId.get());
} else {
- want = id.equals(ctl.getChange().currentPatchSetId());
+ want = id.equals(cd.change().currentPatchSetId());
}
- if (want && ctl.isPatchVisible(in, db.get())) {
- res.put(in.getRevision().get(), toRevisionInfo(ctl, cd, in, repo, rw, false, changeInfo));
+ if (want && cd.changeControl().isPatchVisible(in, db.get())) {
+ res.put(in.getRevision().get(), toRevisionInfo(cd, in, repo, rw, false, changeInfo));
}
}
return res;
@@ -1275,20 +1264,18 @@
return map;
}
- public RevisionInfo getRevisionInfo(ChangeControl ctl, PatchSet in)
+ public RevisionInfo getRevisionInfo(ChangeData cd, PatchSet in)
throws PatchListNotAvailableException, GpgException, OrmException, IOException {
accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
- try (Repository repo = openRepoIfNecessary(ctl);
+ try (Repository repo = openRepoIfNecessary(cd.project());
RevWalk rw = newRevWalk(repo)) {
- RevisionInfo rev =
- toRevisionInfo(ctl, changeDataFactory.create(db.get(), ctl), in, repo, rw, true, null);
+ RevisionInfo rev = toRevisionInfo(cd, in, repo, rw, true, null);
accountLoader.fill();
return rev;
}
}
private RevisionInfo toRevisionInfo(
- ChangeControl ctl,
ChangeData cd,
PatchSet in,
@Nullable Repository repo,
@@ -1296,7 +1283,7 @@
boolean fillCommit,
@Nullable ChangeInfo changeInfo)
throws PatchListNotAvailableException, GpgException, OrmException, IOException {
- Change c = ctl.getChange();
+ Change c = cd.change();
RevisionInfo out = new RevisionInfo();
out.isCurrent = in.getId().equals(c.currentPatchSetId());
out._number = in.getId().get();
@@ -1304,7 +1291,7 @@
out.created = in.getCreatedOn();
out.uploader = accountLoader.get(in.getUploader());
out.draft = in.isDraft() ? true : null;
- out.fetch = makeFetchMap(ctl, in);
+ out.fetch = makeFetchMap(cd, in);
out.kind = changeKindCache.getChangeKind(rw, repo != null ? repo.getConfig() : null, cd, in);
out.description = in.getDescription();
@@ -1321,7 +1308,7 @@
out.commit = toCommit(project, rw, commit, has(WEB_LINKS), fillCommit);
}
if (addFooters) {
- Ref ref = repo.exactRef(ctl.getChange().getDest().get());
+ Ref ref = repo.exactRef(cd.change().getDest().get());
RevCommit mergeTip = null;
if (ref != null) {
mergeTip = rw.parseCommit(ref.getObjectId());
@@ -1331,7 +1318,7 @@
mergeUtilFactory
.create(projectCache.get(project))
.createCommitMessageOnSubmit(
- commit, mergeTip, ctl.getNotes(), ctl.getUser(), in.getId());
+ commit, mergeTip, cd.notes(), userProvider.get(), in.getId());
}
}
@@ -1348,7 +1335,7 @@
actionJson.addRevisionActions(
changeInfo,
out,
- new RevisionResource(changeResourceFactory.create(ctl.getNotes(), ctl.getUser()), in));
+ new RevisionResource(changeResourceFactory.create(cd.notes(), userProvider.get()), in));
}
if (gpgApi.isEnabled() && has(PUSH_CERTIFICATES)) {
@@ -1396,7 +1383,7 @@
return info;
}
- private Map<String, FetchInfo> makeFetchMap(ChangeControl ctl, PatchSet in) throws OrmException {
+ private Map<String, FetchInfo> makeFetchMap(ChangeData cd, PatchSet in) throws OrmException {
Map<String, FetchInfo> r = new LinkedHashMap<>();
for (DynamicMap.Entry<DownloadScheme> e : downloadSchemes) {
@@ -1407,11 +1394,12 @@
continue;
}
- if (!scheme.isAuthSupported() && !ctl.forUser(anonymous).isPatchVisible(in, db.get())) {
+ if (!scheme.isAuthSupported()
+ && !cd.changeControl().forUser(anonymous).isPatchVisible(in, db.get())) {
continue;
}
- String projectName = ctl.getProject().getNameKey().get();
+ String projectName = cd.project().get();
String url = scheme.getUrl(projectName);
String refName = in.getRefName();
FetchInfo fetchInfo = new FetchInfo(url, refName);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/HashtagsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/HashtagsUtil.java
index e9b0af2..42b56b6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/HashtagsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/HashtagsUtil.java
@@ -23,6 +23,18 @@
import java.util.regex.Pattern;
public class HashtagsUtil {
+ public static class InvalidHashtagException extends Exception {
+ private static final long serialVersionUID = 1L;
+
+ static InvalidHashtagException hashtagsMayNotContainCommas() {
+ return new InvalidHashtagException("hashtags may not contain commas");
+ }
+
+ InvalidHashtagException(String message) {
+ super(message);
+ }
+ }
+
private static final CharMatcher LEADER = CharMatcher.whitespace().or(CharMatcher.is('#'));
private static final String PATTERN = "(?:\\s|\\A)#[\\p{L}[0-9]-_]+";
@@ -43,14 +55,14 @@
return result;
}
- static Set<String> extractTags(Set<String> input) throws IllegalArgumentException {
+ static Set<String> extractTags(Set<String> input) throws InvalidHashtagException {
if (input == null) {
return Collections.emptySet();
}
HashSet<String> result = new HashSet<>();
for (String hashtag : input) {
if (hashtag.contains(",")) {
- throw new IllegalArgumentException("Hashtags may not contain commas");
+ throw InvalidHashtagException.hashtagsMayNotContainCommas();
}
hashtag = cleanupHashtag(hashtag);
if (!hashtag.isEmpty()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Mergeable.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Mergeable.java
index 929529d..8e0d9d9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Mergeable.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Mergeable.java
@@ -26,9 +26,6 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.ChangeUtil;
-import com.google.gerrit.server.account.AccountCache;
-import com.google.gerrit.server.account.Accounts;
-import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.git.BranchOrderSection;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
@@ -64,38 +61,32 @@
private boolean otherBranches;
private final GitRepositoryManager gitManager;
- private final AccountCache accountCache;
- private final Accounts accounts;
- private final Emails emails;
private final ProjectCache projectCache;
private final MergeUtil.Factory mergeUtilFactory;
private final ChangeData.Factory changeDataFactory;
private final Provider<ReviewDb> db;
private final ChangeIndexer indexer;
private final MergeabilityCache cache;
+ private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
@Inject
Mergeable(
GitRepositoryManager gitManager,
- AccountCache accountCache,
- Accounts accounts,
- Emails emails,
ProjectCache projectCache,
MergeUtil.Factory mergeUtilFactory,
ChangeData.Factory changeDataFactory,
Provider<ReviewDb> db,
ChangeIndexer indexer,
- MergeabilityCache cache) {
+ MergeabilityCache cache,
+ SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
this.gitManager = gitManager;
- this.accountCache = accountCache;
- this.accounts = accounts;
- this.emails = emails;
this.projectCache = projectCache;
this.mergeUtilFactory = mergeUtilFactory;
this.changeDataFactory = changeDataFactory;
this.db = db;
this.indexer = indexer;
this.cache = cache;
+ this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
}
public void setOtherBranches(boolean otherBranches) {
@@ -152,9 +143,7 @@
private SubmitType getSubmitType(ChangeData cd, PatchSet patchSet) throws OrmException {
SubmitTypeRecord rec =
- new SubmitRuleEvaluator(accountCache, accounts, emails, cd)
- .setPatchSet(patchSet)
- .getSubmitType();
+ submitRuleEvaluatorFactory.create(cd).setPatchSet(patchSet).getSubmitType();
if (rec.status != SubmitTypeRecord.Status.OK) {
throw new OrmException("Submit type rule failed: " + rec);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Mute.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Mute.java
index 5fb3ef6..9da993b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Mute.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Mute.java
@@ -14,17 +14,15 @@
package com.google.gerrit.server.change;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.StarredChangesUtil.IllegalLabelException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -35,12 +33,10 @@
public static class Input {}
- private final Provider<IdentifiedUser> self;
private final StarredChangesUtil stars;
@Inject
- Mute(Provider<IdentifiedUser> self, StarredChangesUtil stars) {
- this.self = self;
+ Mute(StarredChangesUtil stars) {
this.stars = stars;
}
@@ -49,32 +45,33 @@
return new UiAction.Description()
.setLabel("Mute")
.setTitle("Mute the change to unhighlight it in the dashboard")
- .setVisible(!rsrc.isUserOwner() && isMuteable(rsrc.getChange()));
+ .setVisible(isMuteable(rsrc));
}
@Override
public Response<String> apply(ChangeResource rsrc, Input input)
throws RestApiException, OrmException, IllegalLabelException {
- if (rsrc.isUserOwner() || isMuted(rsrc.getChange())) {
- // early exit for own changes and already muted changes
- return Response.ok("");
+ if (rsrc.isUserOwner()) {
+ throw new BadRequestException("cannot mute own change");
}
- stars.mute(self.get().getAccountId(), rsrc.getProject(), rsrc.getChange());
+ if (!isMuted(rsrc)) {
+ stars.mute(rsrc);
+ }
return Response.ok("");
}
- private boolean isMuted(Change change) {
+ private boolean isMuted(ChangeResource rsrc) {
try {
- return stars.isMutedBy(change, self.get().getAccountId());
+ return stars.isMuted(rsrc);
} catch (OrmException e) {
log.error("failed to check muted star", e);
}
return false;
}
- private boolean isMuteable(Change change) {
+ private boolean isMuteable(ChangeResource rsrc) {
try {
- return !isMuted(change) && !stars.isIgnoredBy(change.getId(), self.get().getAccountId());
+ return !rsrc.isUserOwner() && !isMuted(rsrc) && !stars.isIgnored(rsrc);
} catch (OrmException e) {
log.error("failed to check ignored star", e);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
index 9b1a5dc..4b2f5b7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
@@ -27,10 +27,7 @@
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
-import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountLoader;
-import com.google.gerrit.server.account.Accounts;
-import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.permissions.LabelPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -49,30 +46,24 @@
private final Provider<ReviewDb> db;
private final PermissionBackend permissionBackend;
private final ChangeData.Factory changeDataFactory;
- private final AccountCache accountCache;
- private final Accounts accounts;
- private final Emails emails;
private final ApprovalsUtil approvalsUtil;
private final AccountLoader.Factory accountLoaderFactory;
+ private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
@Inject
ReviewerJson(
Provider<ReviewDb> db,
PermissionBackend permissionBackend,
ChangeData.Factory changeDataFactory,
- AccountCache accountCache,
- Accounts accounts,
- Emails emails,
ApprovalsUtil approvalsUtil,
- AccountLoader.Factory accountLoaderFactory) {
+ AccountLoader.Factory accountLoaderFactory,
+ SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
this.db = db;
this.permissionBackend = permissionBackend;
this.changeDataFactory = changeDataFactory;
- this.accountCache = accountCache;
- this.accounts = accounts;
- this.emails = emails;
this.approvalsUtil = approvalsUtil;
this.accountLoaderFactory = accountLoaderFactory;
+ this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
}
public List<ReviewerInfo> format(Collection<ReviewerResource> rsrcs)
@@ -133,7 +124,8 @@
PatchSet ps = cd.currentPatchSet();
if (ps != null) {
for (SubmitRecord rec :
- new SubmitRuleEvaluator(accountCache, accounts, emails, cd)
+ submitRuleEvaluatorFactory
+ .create(cd)
.setFastEvalLabels(true)
.setAllowDraft(true)
.evaluate()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java
index 724598a..1f17dd3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java
@@ -29,6 +29,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.change.HashtagsUtil.InvalidHashtagException;
import com.google.gerrit.server.extensions.events.HashtagsEdited;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
@@ -99,31 +100,31 @@
ChangeUpdate update = ctx.getUpdate(change.currentPatchSetId());
ChangeNotes notes = update.getNotes().load();
- Set<String> existingHashtags = notes.getHashtags();
- Set<String> updated = new HashSet<>();
- toAdd = new HashSet<>(extractTags(input.add));
- toRemove = new HashSet<>(extractTags(input.remove));
-
try {
+ Set<String> existingHashtags = notes.getHashtags();
+ Set<String> updated = new HashSet<>();
+ toAdd = new HashSet<>(extractTags(input.add));
+ toRemove = new HashSet<>(extractTags(input.remove));
+
for (HashtagValidationListener validator : validationListeners) {
validator.validateHashtags(update.getChange(), toAdd, toRemove);
}
- } catch (ValidationException e) {
+
+ updated.addAll(existingHashtags);
+ toAdd.removeAll(existingHashtags);
+ toRemove.retainAll(existingHashtags);
+ if (updated()) {
+ updated.addAll(toAdd);
+ updated.removeAll(toRemove);
+ update.setHashtags(updated);
+ addMessage(ctx, update);
+ }
+
+ updatedHashtags = ImmutableSortedSet.copyOf(updated);
+ return true;
+ } catch (ValidationException | InvalidHashtagException e) {
throw new BadRequestException(e.getMessage());
}
-
- updated.addAll(existingHashtags);
- toAdd.removeAll(existingHashtags);
- toRemove.retainAll(existingHashtags);
- if (updated()) {
- updated.addAll(toAdd);
- updated.removeAll(toRemove);
- update.setHashtags(updated);
- addMessage(ctx, update);
- }
-
- updatedHashtags = ImmutableSortedSet.copyOf(updated);
- return true;
}
private void addMessage(ChangeContext ctx, ChangeUpdate update) throws OrmException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestChangeReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestChangeReviewers.java
index bb4a357..fe21858 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestChangeReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestChangeReviewers.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.change;
+import com.google.gerrit.extensions.common.AccountVisibility;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -25,7 +26,6 @@
import com.google.gerrit.server.IdentifiedUser.GenericFactory;
import com.google.gerrit.server.ReviewersUtil;
import com.google.gerrit.server.ReviewersUtil.VisibilityControl;
-import com.google.gerrit.server.account.AccountVisibility;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.RefPermission;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestReviewers.java
index 47fb513..2ed80718 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestReviewers.java
@@ -14,10 +14,10 @@
package com.google.gerrit.server.change;
+import com.google.gerrit.extensions.common.AccountVisibility;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ReviewersUtil;
-import com.google.gerrit.server.account.AccountVisibility;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
import com.google.inject.Provider;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitRule.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitRule.java
index ad716e6..371c796 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitRule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitRule.java
@@ -24,10 +24,7 @@
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.rules.RulesCache;
-import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountLoader;
-import com.google.gerrit.server.account.Accounts;
-import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
@@ -42,10 +39,8 @@
private final Provider<ReviewDb> db;
private final ChangeData.Factory changeDataFactory;
private final RulesCache rules;
- private final AccountCache accountCache;
private final AccountLoader.Factory accountInfoFactory;
- private final Accounts accounts;
- private final Emails emails;
+ private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
@Option(name = "--filters", usage = "impact of filters in parent projects")
private Filters filters = Filters.RUN;
@@ -55,17 +50,13 @@
Provider<ReviewDb> db,
ChangeData.Factory changeDataFactory,
RulesCache rules,
- AccountCache accountCache,
AccountLoader.Factory infoFactory,
- Accounts accounts,
- Emails emails) {
+ SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
this.db = db;
this.changeDataFactory = changeDataFactory;
this.rules = rules;
- this.accountCache = accountCache;
this.accountInfoFactory = infoFactory;
- this.accounts = accounts;
- this.emails = emails;
+ this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
}
@Override
@@ -79,10 +70,7 @@
}
input.filters = MoreObjects.firstNonNull(input.filters, filters);
SubmitRuleEvaluator evaluator =
- new SubmitRuleEvaluator(
- accountCache,
- accounts,
- emails,
+ submitRuleEvaluatorFactory.create(
changeDataFactory.create(db.get(), rsrc.getChangeResource()));
List<SubmitRecord> records =
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitType.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitType.java
index 48b657e..0dd1019 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitType.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitType.java
@@ -25,9 +25,6 @@
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.rules.RulesCache;
-import com.google.gerrit.server.account.AccountCache;
-import com.google.gerrit.server.account.Accounts;
-import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
@@ -37,11 +34,9 @@
public class TestSubmitType implements RestModifyView<RevisionResource, TestSubmitRuleInput> {
private final Provider<ReviewDb> db;
- private final AccountCache accountCache;
- private final Accounts accounts;
- private final Emails emails;
private final ChangeData.Factory changeDataFactory;
private final RulesCache rules;
+ private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
@Option(name = "--filters", usage = "impact of filters in parent projects")
private Filters filters = Filters.RUN;
@@ -49,17 +44,13 @@
@Inject
TestSubmitType(
Provider<ReviewDb> db,
- AccountCache accountCache,
- Accounts accounts,
- Emails emails,
ChangeData.Factory changeDataFactory,
- RulesCache rules) {
+ RulesCache rules,
+ SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
this.db = db;
- this.accountCache = accountCache;
- this.accounts = accounts;
- this.emails = emails;
this.changeDataFactory = changeDataFactory;
this.rules = rules;
+ this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
}
@Override
@@ -73,10 +64,7 @@
}
input.filters = MoreObjects.firstNonNull(input.filters, filters);
SubmitRuleEvaluator evaluator =
- new SubmitRuleEvaluator(
- accountCache,
- accounts,
- emails,
+ submitRuleEvaluatorFactory.create(
changeDataFactory.create(db.get(), rsrc.getChangeResource()));
SubmitTypeRecord rec =
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Unignore.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Unignore.java
index a10e754..2bad16c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Unignore.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Unignore.java
@@ -15,7 +15,6 @@
package com.google.gerrit.server.change;
import com.google.gerrit.extensions.restapi.Response;
-import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.server.StarredChangesUtil;
@@ -50,7 +49,7 @@
@Override
public Response<String> apply(ChangeResource rsrc, Input input)
- throws RestApiException, OrmException, IllegalLabelException {
+ throws OrmException, IllegalLabelException {
if (isIgnored(rsrc)) {
stars.unignore(rsrc);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Unmute.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Unmute.java
index 586f3e6..16d6d88 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Unmute.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Unmute.java
@@ -15,16 +15,12 @@
package com.google.gerrit.server.change;
import com.google.gerrit.extensions.restapi.Response;
-import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.StarredChangesUtil.IllegalLabelException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -36,12 +32,10 @@
public static class Input {}
- private final Provider<IdentifiedUser> self;
private final StarredChangesUtil stars;
@Inject
- Unmute(Provider<IdentifiedUser> self, StarredChangesUtil stars) {
- this.self = self;
+ Unmute(StarredChangesUtil stars) {
this.stars = stars;
}
@@ -50,35 +44,24 @@
return new UiAction.Description()
.setLabel("Unmute")
.setTitle("Unmute the change")
- .setVisible(!rsrc.isUserOwner() && isUnMuteable(rsrc.getChange()));
+ .setVisible(isMuted(rsrc));
}
@Override
public Response<String> apply(ChangeResource rsrc, Input input)
- throws RestApiException, OrmException, IllegalLabelException {
- if (rsrc.isUserOwner() || !isMuted(rsrc.getChange())) {
- // early exit for own changes and not muted changes
- return Response.ok("");
+ throws OrmException, IllegalLabelException {
+ if (isMuted(rsrc)) {
+ stars.unmute(rsrc);
}
- stars.unmute(self.get().getAccountId(), rsrc.getProject(), rsrc.getChange());
return Response.ok("");
}
- private boolean isMuted(Change change) {
+ private boolean isMuted(ChangeResource rsrc) {
try {
- return stars.isMutedBy(change, self.get().getAccountId());
+ return stars.isMuted(rsrc);
} catch (OrmException e) {
log.error("failed to check muted star", e);
}
return false;
}
-
- private boolean isUnMuteable(Change change) {
- try {
- return isMuted(change) && !stars.isIgnoredBy(change.getId(), self.get().getAccountId());
- } catch (OrmException e) {
- log.error("failed to check ignored star", e);
- }
- return false;
- }
}
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 08abfbb..8751787 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
@@ -25,6 +25,7 @@
import com.google.gerrit.extensions.api.projects.CommentLinkInfo;
import com.google.gerrit.extensions.auth.oauth.OAuthLoginProvider;
import com.google.gerrit.extensions.auth.oauth.OAuthTokenEncrypter;
+import com.google.gerrit.extensions.common.AccountVisibility;
import com.google.gerrit.extensions.config.CapabilityDefinition;
import com.google.gerrit.extensions.config.CloneCommand;
import com.google.gerrit.extensions.config.DownloadCommand;
@@ -83,7 +84,6 @@
import com.google.gerrit.server.account.AccountControl;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AccountResolver;
-import com.google.gerrit.server.account.AccountVisibility;
import com.google.gerrit.server.account.AccountVisibilityProvider;
import com.google.gerrit.server.account.CapabilityCollection;
import com.google.gerrit.server.account.ChangeUserName;
@@ -166,6 +166,7 @@
import com.google.gerrit.server.project.ProjectNode;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SectionSortCache;
+import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ChangeQueryProcessor;
@@ -264,6 +265,7 @@
bind(PermissionCollection.Factory.class);
bind(AccountVisibility.class).toProvider(AccountVisibilityProvider.class).in(SINGLETON);
factory(ProjectOwnerGroupsProvider.Factory.class);
+ factory(SubmitRuleEvaluator.Factory.class);
bind(AuthBackend.class).to(UniversalAuthBackend.class).in(SINGLETON);
DynamicSet.setOf(binder(), AuthBackend.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GetServerInfo.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GetServerInfo.java
index 1afcc33..90e2838 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GetServerInfo.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GetServerInfo.java
@@ -21,6 +21,7 @@
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.ContributorAgreement;
import com.google.gerrit.extensions.client.UiType;
+import com.google.gerrit.extensions.common.AccountsInfo;
import com.google.gerrit.extensions.common.AuthInfo;
import com.google.gerrit.extensions.common.ChangeConfigInfo;
import com.google.gerrit.extensions.common.DownloadInfo;
@@ -41,6 +42,7 @@
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.webui.WebUiPlugin;
import com.google.gerrit.server.EnableSignedPush;
+import com.google.gerrit.server.account.AccountVisibilityProvider;
import com.google.gerrit.server.account.Realm;
import com.google.gerrit.server.avatar.AvatarProvider;
import com.google.gerrit.server.change.AllowedFormats;
@@ -69,6 +71,7 @@
private static final String KEY_TOKEN = "token";
private final Config config;
+ private final AccountVisibilityProvider accountVisibilityProvider;
private final AuthConfig authConfig;
private final Realm realm;
private final DynamicMap<DownloadScheme> downloadSchemes;
@@ -92,6 +95,7 @@
@Inject
public GetServerInfo(
@GerritServerConfig Config config,
+ AccountVisibilityProvider accountVisibilityProvider,
AuthConfig authConfig,
Realm realm,
DynamicMap<DownloadScheme> downloadSchemes,
@@ -112,6 +116,7 @@
ChangeIndexCollection indexes,
SitePaths sitePaths) {
this.config = config;
+ this.accountVisibilityProvider = accountVisibilityProvider;
this.authConfig = authConfig;
this.realm = realm;
this.downloadSchemes = downloadSchemes;
@@ -136,6 +141,7 @@
@Override
public ServerInfo apply(ConfigResource rsrc) throws MalformedURLException {
ServerInfo info = new ServerInfo();
+ info.accounts = getAccountsInfo(accountVisibilityProvider);
info.auth = getAuthInfo(authConfig, realm);
info.change = getChangeInfo(config);
info.download =
@@ -157,6 +163,12 @@
return info;
}
+ private AccountsInfo getAccountsInfo(AccountVisibilityProvider accountVisibilityProvider) {
+ AccountsInfo info = new AccountsInfo();
+ info.visibility = accountVisibilityProvider.get();
+ return info;
+ }
+
private AuthInfo getAuthInfo(AuthConfig cfg, Realm realm) {
AuthInfo info = new AuthInfo();
info.authType = cfg.getAuthType();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/Module.java
index 4f93a1a..7bf5ad5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/Module.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/Module.java
@@ -37,7 +37,6 @@
get(CONFIG_KIND, "version").to(GetVersion.class);
get(CONFIG_KIND, "info").to(GetServerInfo.class);
post(CONFIG_KIND, "check.consistency").to(CheckConsistency.class);
- post(CONFIG_KIND, "check.access").to(CheckAccess.class);
get(CONFIG_KIND, "preferences").to(GetPreferences.class);
put(CONFIG_KIND, "preferences").to(SetPreferences.class);
get(CONFIG_KIND, "preferences.diff").to(GetDiffPreferences.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java
index f367e23..be308a9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java
@@ -29,7 +29,6 @@
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
-import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -88,8 +87,7 @@
public RevisionInfo revisionInfo(Project.NameKey project, PatchSet ps)
throws OrmException, PatchListNotAvailableException, GpgException, IOException {
ChangeData cd = changeDataFactory.create(db.get(), project, ps.getId().getParentKey());
- ChangeControl ctl = cd.changeControl();
- return changeJson.getRevisionInfo(ctl, ps);
+ return changeJson.getRevisionInfo(cd, ps);
}
public AccountInfo accountInfo(Account a) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
index 978ef3e..85e5d06 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
@@ -31,9 +31,6 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.account.AccountCache;
-import com.google.gerrit.server.account.Accounts;
-import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.change.Submit;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo;
@@ -100,9 +97,6 @@
abstract ImmutableSet<String> hashes();
}
- private final AccountCache accountCache;
- private final Accounts accounts;
- private final Emails emails;
private final ChangeData.Factory changeDataFactory;
private final Provider<InternalChangeQuery> queryProvider;
private final Provider<MergeOpRepoManager> repoManagerProvider;
@@ -110,6 +104,7 @@
private final Config cfg;
private final Map<QueryKey, List<ChangeData>> queryCache;
private final Map<Branch.NameKey, Optional<RevCommit>> heads;
+ private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
private MergeOpRepoManager orm;
private boolean closeOrm;
@@ -117,21 +112,17 @@
@Inject
MergeSuperSet(
@GerritServerConfig Config cfg,
- AccountCache accountCache,
- Accounts accounts,
- Emails emails,
ChangeData.Factory changeDataFactory,
Provider<InternalChangeQuery> queryProvider,
Provider<MergeOpRepoManager> repoManagerProvider,
- PermissionBackend permissionBackend) {
+ PermissionBackend permissionBackend,
+ SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
this.cfg = cfg;
- this.accountCache = accountCache;
- this.accounts = accounts;
- this.emails = emails;
this.changeDataFactory = changeDataFactory;
this.queryProvider = queryProvider;
this.repoManagerProvider = repoManagerProvider;
this.permissionBackend = permissionBackend;
+ this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
queryCache = new HashMap<>();
heads = new HashMap<>();
}
@@ -181,9 +172,7 @@
SubmitTypeRecord str =
ps == cd.currentPatchSet()
? cd.submitTypeRecord()
- : new SubmitRuleEvaluator(accountCache, accounts, emails, cd)
- .setPatchSet(ps)
- .getSubmitType();
+ : submitRuleEvaluatorFactory.create(cd).setPatchSet(ps).getSubmitType();
if (!str.isOk()) {
logErrorAndThrow("Failed to get submit type for " + cd.getId() + ": " + str.errorMessage);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/CheckAccess.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CheckAccess.java
similarity index 73%
rename from gerrit-server/src/main/java/com/google/gerrit/server/config/CheckAccess.java
rename to gerrit-server/src/main/java/com/google/gerrit/server/project/CheckAccess.java
index c89684c..281e37e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/CheckAccess.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CheckAccess.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.config;
+package com.google.gerrit.server.project;
import com.google.common.base.Strings;
import com.google.gerrit.extensions.api.config.AccessCheckInfo;
@@ -21,9 +21,9 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
-import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResolver;
@@ -32,7 +32,6 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.permissions.RefPermission;
-import com.google.gerrit.server.project.ProjectCache;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -42,35 +41,29 @@
import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
-public class CheckAccess implements RestModifyView<ConfigResource, AccessCheckInput> {
- private final Provider<IdentifiedUser> currentUser;
+public class CheckAccess implements RestModifyView<ProjectResource, AccessCheckInput> {
private final AccountResolver accountResolver;
private final Provider<ReviewDb> db;
private final IdentifiedUser.GenericFactory userFactory;
- private final ProjectCache projectCache;
private final PermissionBackend permissionBackend;
@Inject
CheckAccess(
- Provider<IdentifiedUser> currentUser,
AccountResolver resolver,
Provider<ReviewDb> db,
IdentifiedUser.GenericFactory userFactory,
- ProjectCache projectCache,
PermissionBackend permissionBackend) {
- this.currentUser = currentUser;
this.accountResolver = resolver;
this.db = db;
this.userFactory = userFactory;
- this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
}
@Override
- public AccessCheckInfo apply(ConfigResource unused, AccessCheckInput input)
+ public AccessCheckInfo apply(ProjectResource rsrc, AccessCheckInput input)
throws OrmException, PermissionBackendException, RestApiException, IOException,
ConfigInvalidException {
- permissionBackend.user(currentUser.get()).check(GlobalPermission.ADMINISTRATE_SERVER);
+ permissionBackend.user(rsrc.getUser()).check(GlobalPermission.ADMINISTRATE_SERVER);
if (input == null) {
throw new BadRequestException("input is required");
@@ -78,32 +71,23 @@
if (Strings.isNullOrEmpty(input.account)) {
throw new BadRequestException("input requires 'account'");
}
- if (Strings.isNullOrEmpty(input.project)) {
- throw new BadRequestException("input requires 'project'");
- }
Account match = accountResolver.find(db.get(), input.account);
if (match == null) {
- throw new BadRequestException(String.format("cannot find account %s", input.account));
+ throw new UnprocessableEntityException(
+ String.format("cannot find account %s", input.account));
}
AccessCheckInfo info = new AccessCheckInfo();
- Project.NameKey key = new Project.NameKey(input.project);
- if (projectCache.get(key) == null) {
- info.message = String.format("project %s does not exist", key);
- info.status = HttpServletResponse.SC_NOT_FOUND;
- return info;
- }
-
IdentifiedUser user = userFactory.create(match.getId());
try {
- permissionBackend.user(user).project(key).check(ProjectPermission.ACCESS);
+ permissionBackend.user(user).project(rsrc.getNameKey()).check(ProjectPermission.ACCESS);
} catch (AuthException | PermissionBackendException e) {
info.message =
String.format(
"user %s (%s) cannot see project %s",
- user.getNameEmail(), user.getAccount().getId(), key);
+ user.getNameEmail(), user.getAccount().getId(), rsrc.getName());
info.status = HttpServletResponse.SC_FORBIDDEN;
return info;
}
@@ -112,14 +96,14 @@
try {
permissionBackend
.user(user)
- .ref(new Branch.NameKey(key, input.ref))
+ .ref(new Branch.NameKey(rsrc.getNameKey(), input.ref))
.check(RefPermission.READ);
} catch (AuthException | PermissionBackendException e) {
info.status = HttpServletResponse.SC_FORBIDDEN;
info.message =
String.format(
"user %s (%s) cannot see ref %s in project %s",
- user.getNameEmail(), user.getAccount().getId(), input.ref, key);
+ user.getNameEmail(), user.getAccount().getId(), input.ref, rsrc.getName());
return info;
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DashboardsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DashboardsCollection.java
index 3ca62e1..70271b7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DashboardsCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DashboardsCollection.java
@@ -16,6 +16,7 @@
import static com.google.gerrit.reviewdb.client.RefNames.REFS_DASHBOARDS;
+import com.google.common.base.Joiner;
import com.google.common.base.MoreObjects;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
@@ -32,6 +33,7 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.UrlEncoded;
@@ -167,6 +169,14 @@
return views;
}
+ static DashboardInfo newDashboardInfo(String ref, String path) {
+ DashboardInfo info = new DashboardInfo();
+ info.ref = ref;
+ info.path = path;
+ info.id = Joiner.on(':').join(Url.encode(ref), Url.encode(path));
+ return info;
+ }
+
static DashboardInfo parse(
Project definingProject,
String refName,
@@ -174,7 +184,7 @@
Config config,
String project,
boolean setDefault) {
- DashboardInfo info = new DashboardInfo(refName, path);
+ DashboardInfo info = newDashboardInfo(refName, path);
info.project = project;
info.definingProject = definingProject.getName();
String query = config.getString("dashboard", null, "title");
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteDashboard.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteDashboard.java
index 926abd5..7296311 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteDashboard.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteDashboard.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.project;
import com.google.gerrit.extensions.api.projects.DashboardInfo;
+import com.google.gerrit.extensions.common.SetDashboardInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
@@ -29,7 +30,7 @@
import java.io.IOException;
@Singleton
-class DeleteDashboard implements RestModifyView<DashboardResource, SetDashboard.Input> {
+class DeleteDashboard implements RestModifyView<DashboardResource, SetDashboardInput> {
private final Provider<SetDefaultDashboard> defaultSetter;
@Inject
@@ -38,12 +39,12 @@
}
@Override
- public Response<DashboardInfo> apply(DashboardResource resource, SetDashboard.Input input)
+ public Response<DashboardInfo> apply(DashboardResource resource, SetDashboardInput input)
throws AuthException, BadRequestException, ResourceConflictException,
ResourceNotFoundException, MethodNotAllowedException, IOException,
PermissionBackendException {
if (resource.isProjectDefault()) {
- SetDashboard.Input in = new SetDashboard.Input();
+ SetDashboardInput in = new SetDashboardInput();
in.commitMessage = input != null ? input.commitMessage : null;
return defaultSetter.get().apply(resource, in);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/Module.java
index 1f33bc3..d0753eb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/Module.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/Module.java
@@ -49,6 +49,7 @@
get(PROJECT_KIND, "access").to(GetAccess.class);
post(PROJECT_KIND, "access").to(SetAccess.class);
put(PROJECT_KIND, "access:review").to(CreateAccessChange.class);
+ post(PROJECT_KIND, "check.access").to(CheckAccess.class);
get(PROJECT_KIND, "parent").to(GetParent.class);
put(PROJECT_KIND, "parent").to(SetParent.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RemoveReviewerControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RemoveReviewerControl.java
index f205686..ded2bf8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RemoveReviewerControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RemoveReviewerControl.java
@@ -16,6 +16,7 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
@@ -23,6 +24,8 @@
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -46,9 +49,9 @@
/** @throws AuthException if this user is not allowed to remove this approval. */
public void checkRemoveReviewer(
ChangeNotes notes, CurrentUser currentUser, PatchSetApproval approval)
- throws PermissionBackendException, AuthException, NoSuchChangeException {
+ throws PermissionBackendException, AuthException, NoSuchChangeException, OrmException {
if (canRemoveReviewerWithoutPermissionCheck(
- notes, currentUser, approval.getAccountId(), approval.getValue())) {
+ notes.getChange(), currentUser, approval.getAccountId(), approval.getValue())) {
return;
}
@@ -61,22 +64,22 @@
/** @return true if the user is allowed to remove this reviewer. */
public boolean testRemoveReviewer(
- ChangeNotes notes, CurrentUser currentUser, Account.Id reviewer, int value)
- throws PermissionBackendException, NoSuchChangeException {
- if (canRemoveReviewerWithoutPermissionCheck(notes, currentUser, reviewer, value)) {
+ ChangeData cd, CurrentUser currentUser, Account.Id reviewer, int value)
+ throws PermissionBackendException, NoSuchChangeException, OrmException {
+ if (canRemoveReviewerWithoutPermissionCheck(cd.change(), currentUser, reviewer, value)) {
return true;
}
return permissionBackend
.user(currentUser)
- .change(notes)
+ .change(cd)
.database(dbProvider)
.test(ChangePermission.REMOVE_REVIEWER);
}
private boolean canRemoveReviewerWithoutPermissionCheck(
- ChangeNotes notes, CurrentUser currentUser, Account.Id reviewer, int value)
- throws NoSuchChangeException {
- if (!notes.getChange().getStatus().isOpen()) {
+ Change change, CurrentUser currentUser, Account.Id reviewer, int value)
+ throws NoSuchChangeException, OrmException {
+ if (!change.getStatus().isOpen()) {
return false;
}
@@ -84,14 +87,15 @@
Account.Id aId = currentUser.getAccountId();
if (aId.equals(reviewer)) {
return true; // A user can always remove themselves.
- } else if (aId.equals(notes.getChange().getOwner()) && 0 <= value) {
+ } else if (aId.equals(change.getOwner()) && 0 <= value) {
return true; // The change owner may remove any zero or positive score.
}
}
// Users with the remove reviewer permission, the branch owner, project
// owner and site admin can remove anyone
- ChangeControl changeControl = changeControlFactory.controlFor(notes, currentUser);
+ ChangeControl changeControl =
+ changeControlFactory.controlFor(dbProvider.get(), change, currentUser);
if (changeControl.getRefControl().isOwner() // branch owner
|| changeControl.getProjectControl().isOwner() // project owner
|| changeControl.getProjectControl().isAdmin()) { // project admin
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDashboard.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDashboard.java
index f216f40..9222322 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDashboard.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDashboard.java
@@ -15,28 +15,22 @@
package com.google.gerrit.server.project;
import com.google.gerrit.extensions.api.projects.DashboardInfo;
+import com.google.gerrit.extensions.common.SetDashboardInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.extensions.restapi.DefaultInput;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.project.SetDashboard.Input;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
@Singleton
-class SetDashboard implements RestModifyView<DashboardResource, Input> {
- static class Input {
- @DefaultInput String id;
- String commitMessage;
- }
-
+class SetDashboard implements RestModifyView<DashboardResource, SetDashboardInput> {
private final Provider<SetDefaultDashboard> defaultSetter;
@Inject
@@ -45,7 +39,7 @@
}
@Override
- public Response<DashboardInfo> apply(DashboardResource resource, Input input)
+ public Response<DashboardInfo> apply(DashboardResource resource, SetDashboardInput input)
throws AuthException, BadRequestException, ResourceConflictException,
MethodNotAllowedException, ResourceNotFoundException, IOException,
PermissionBackendException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java
index fa03e45..256b6f2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java
@@ -17,6 +17,7 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.gerrit.extensions.api.projects.DashboardInfo;
+import com.google.gerrit.extensions.common.SetDashboardInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.IdString;
@@ -28,7 +29,6 @@
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.project.SetDashboard.Input;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
@@ -36,7 +36,7 @@
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.kohsuke.args4j.Option;
-class SetDefaultDashboard implements RestModifyView<DashboardResource, Input> {
+class SetDefaultDashboard implements RestModifyView<DashboardResource, SetDashboardInput> {
private final ProjectCache cache;
private final MetaDataUpdate.Server updateFactory;
private final DashboardsCollection dashboards;
@@ -58,11 +58,11 @@
}
@Override
- public Response<DashboardInfo> apply(DashboardResource resource, Input input)
+ public Response<DashboardInfo> apply(DashboardResource resource, SetDashboardInput input)
throws AuthException, BadRequestException, ResourceConflictException,
ResourceNotFoundException, IOException, PermissionBackendException {
if (input == null) {
- input = new Input(); // Delete would set input to null.
+ input = new SetDashboardInput(); // Delete would set input to null.
}
input.id = Strings.emptyToNull(input.id);
@@ -119,7 +119,7 @@
}
}
- static class CreateDefault implements RestModifyView<ProjectResource, SetDashboard.Input> {
+ static class CreateDefault implements RestModifyView<ProjectResource, SetDashboardInput> {
private final Provider<SetDefaultDashboard> setDefault;
@Option(name = "--inherited", usage = "set dashboard inherited by children")
@@ -131,7 +131,7 @@
}
@Override
- public Response<DashboardInfo> apply(ProjectResource resource, Input input)
+ public Response<DashboardInfo> apply(ProjectResource resource, SetDashboardInput input)
throws AuthException, BadRequestException, ResourceConflictException,
ResourceNotFoundException, IOException, PermissionBackendException {
SetDefaultDashboard set = setDefault.get();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
index 5ab629a..87d10c9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
@@ -32,6 +32,8 @@
import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
import com.googlecode.prolog_cafe.exceptions.CompileException;
import com.googlecode.prolog_cafe.exceptions.ReductionLimitException;
import com.googlecode.prolog_cafe.lang.IntegerTerm;
@@ -84,28 +86,31 @@
}
}
+ public interface Factory {
+ SubmitRuleEvaluator create(ChangeData cd);
+ }
+
private final AccountCache accountCache;
private final Accounts accounts;
private final Emails emails;
private final ChangeData cd;
- private final ChangeControl control;
private SubmitRuleOptions.Builder optsBuilder = SubmitRuleOptions.defaults();
private SubmitRuleOptions opts;
private PatchSet patchSet;
private boolean logErrors = true;
private long reductionsConsumed;
+ private ChangeControl control;
private Term submitRule;
- public SubmitRuleEvaluator(
- AccountCache accountCache, Accounts accounts, Emails emails, ChangeData cd)
- throws OrmException {
+ @Inject
+ SubmitRuleEvaluator(
+ AccountCache accountCache, Accounts accounts, Emails emails, @Assisted ChangeData cd) {
this.accountCache = accountCache;
this.accounts = accounts;
this.emails = emails;
this.cd = cd;
- this.control = cd.changeControl();
}
/**
@@ -219,6 +224,12 @@
*/
public List<SubmitRecord> evaluate() {
initOptions();
+ try {
+ initChange();
+ } catch (OrmException e) {
+ return ruleError("Error looking up change " + cd.getId(), e);
+ }
+
Change c = control.getChange();
if (!opts.allowClosed() && c.getStatus().isClosed()) {
SubmitRecord rec = new SubmitRecord();
@@ -226,12 +237,6 @@
return Collections.singletonList(rec);
}
if (!opts.allowDraft()) {
- try {
- initPatchSet();
- } catch (OrmException e) {
- return ruleError(
- "Error looking up patch set " + control.getChange().currentPatchSetId(), e);
- }
if (c.getStatus() == Change.Status.DRAFT || patchSet.isDraft()) {
return cannotSubmitDraft();
}
@@ -409,9 +414,9 @@
public SubmitTypeRecord getSubmitType() {
initOptions();
try {
- initPatchSet();
+ initChange();
} catch (OrmException e) {
- return typeError("Error looking up patch set " + control.getChange().currentPatchSetId(), e);
+ return typeError("Error looking up change " + cd.getId(), e);
}
try {
@@ -679,7 +684,11 @@
}
}
- private void initPatchSet() throws OrmException {
+ private void initChange() throws OrmException {
+ if (control == null) {
+ control = cd.changeControl();
+ }
+
if (patchSet == null) {
patchSet = cd.currentPatchSet();
if (patchSet == null) {
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 b750019..2a3ff45 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
@@ -59,9 +59,6 @@
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.StarredChangesUtil.StarRef;
-import com.google.gerrit.server.account.AccountCache;
-import com.google.gerrit.server.account.Accounts;
-import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.GetPureRevert;
import com.google.gerrit.server.change.MergeabilityCache;
@@ -352,22 +349,19 @@
ChangeData cd =
new ChangeData(
null, null, null, null, null, null, null, null, null, null, null, null, null, null,
- null, null, null, null, null, null, null, project, id, null, null, null);
+ null, null, null, null, null, project, id, null, null, null);
cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId));
return cd;
}
// Injected fields.
private @Nullable final StarredChangesUtil starredChangesUtil;
- private final AccountCache accountCache;
- private final Accounts accounts;
private final AllUsersName allUsersName;
private final ApprovalsUtil approvalsUtil;
private final ChangeControl.GenericFactory changeControlFactory;
private final ChangeMessagesUtil cmUtil;
private final ChangeNotes.Factory notesFactory;
private final CommentsUtil commentsUtil;
- private final Emails emails;
private final GitRepositoryManager repoManager;
private final IdentifiedUser.GenericFactory userFactory;
private final MergeUtil.Factory mergeUtilFactory;
@@ -378,6 +372,7 @@
private final ProjectCache projectCache;
private final TrackingFooters trackingFooters;
private final GetPureRevert pureRevert;
+ private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
// Required assisted injected fields.
private final ReviewDb db;
@@ -431,15 +426,12 @@
@Inject
private ChangeData(
@Nullable StarredChangesUtil starredChangesUtil,
- AccountCache accountCache,
- Accounts accounts,
ApprovalsUtil approvalsUtil,
AllUsersName allUsersName,
ChangeControl.GenericFactory changeControlFactory,
ChangeMessagesUtil cmUtil,
ChangeNotes.Factory notesFactory,
CommentsUtil commentsUtil,
- Emails emails,
GitRepositoryManager repoManager,
IdentifiedUser.GenericFactory userFactory,
MergeUtil.Factory mergeUtilFactory,
@@ -450,21 +442,19 @@
ProjectCache projectCache,
TrackingFooters trackingFooters,
GetPureRevert pureRevert,
+ SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory,
@Assisted ReviewDb db,
@Assisted Project.NameKey project,
@Assisted Change.Id id,
@Assisted @Nullable Change change,
@Assisted @Nullable ChangeNotes notes,
@Assisted @Nullable ChangeControl control) {
- this.accountCache = accountCache;
- this.accounts = accounts;
this.approvalsUtil = approvalsUtil;
this.allUsersName = allUsersName;
this.changeControlFactory = changeControlFactory;
this.cmUtil = cmUtil;
this.notesFactory = notesFactory;
this.commentsUtil = commentsUtil;
- this.emails = emails;
this.repoManager = repoManager;
this.userFactory = userFactory;
this.mergeUtilFactory = mergeUtilFactory;
@@ -476,6 +466,7 @@
this.starredChangesUtil = starredChangesUtil;
this.trackingFooters = trackingFooters;
this.pureRevert = pureRevert;
+ this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
// May be null in tests when created via createForTest above, in which case lazy-loading will
// intentionally fail with NPE. Still not marked @Nullable in the constructor, to force callers
@@ -1032,10 +1023,7 @@
if (!lazyLoad) {
return Collections.emptyList();
}
- records =
- new SubmitRuleEvaluator(accountCache, accounts, emails, this)
- .setOptions(options)
- .evaluate();
+ records = submitRuleEvaluatorFactory.create(this).setOptions(options).evaluate();
submitRecords.put(options, records);
}
return records;
@@ -1052,8 +1040,7 @@
public SubmitTypeRecord submitTypeRecord() throws OrmException {
if (submitTypeRecord == null) {
- submitTypeRecord =
- new SubmitRuleEvaluator(accountCache, accounts, emails, this).getSubmitType();
+ submitTypeRecord = submitRuleEvaluatorFactory.create(this).getSubmitType();
}
return submitTypeRecord;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
index 7b53a30..1fe982f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
@@ -25,9 +25,6 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.account.AccountCache;
-import com.google.gerrit.server.account.Accounts;
-import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.data.ChangeAttribute;
import com.google.gerrit.server.data.PatchSetAttribute;
@@ -75,15 +72,13 @@
}
private final ReviewDb db;
- private final AccountCache accountCache;
- private final Accounts accounts;
- private final Emails emails;
private final GitRepositoryManager repoManager;
private final ChangeQueryBuilder queryBuilder;
private final ChangeQueryProcessor queryProcessor;
private final EventFactory eventFactory;
private final TrackingFooters trackingFooters;
private final CurrentUser user;
+ private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
private OutputFormat outputFormat = OutputFormat.TEXT;
private boolean includePatchSets;
@@ -102,25 +97,21 @@
@Inject
OutputStreamQuery(
ReviewDb db,
- AccountCache accountCache,
- Accounts accounts,
- Emails emails,
GitRepositoryManager repoManager,
ChangeQueryBuilder queryBuilder,
ChangeQueryProcessor queryProcessor,
EventFactory eventFactory,
TrackingFooters trackingFooters,
- CurrentUser user) {
+ CurrentUser user,
+ SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
this.db = db;
- this.accountCache = accountCache;
- this.accounts = accounts;
- this.emails = emails;
this.repoManager = repoManager;
this.queryBuilder = queryBuilder;
this.queryProcessor = queryProcessor;
this.eventFactory = eventFactory;
this.trackingFooters = trackingFooters;
this.user = user;
+ this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
}
void setLimit(int n) {
@@ -259,10 +250,7 @@
if (includeSubmitRecords) {
eventFactory.addSubmitRecords(
c,
- new SubmitRuleEvaluator(accountCache, accounts, emails, d)
- .setAllowClosed(true)
- .setAllowDraft(true)
- .evaluate());
+ submitRuleEvaluatorFactory.create(d).setAllowClosed(true).setAllowDraft(true).evaluate());
}
if (includeCommitMessage) {
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 c1fc0ec..b288bd5 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
@@ -14,9 +14,6 @@
package com.google.gerrit.server.query.change;
-import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
-import static com.google.gerrit.extensions.client.ListChangesOption.LABELS;
-
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.gerrit.extensions.client.ListChangesOption;
@@ -28,7 +25,6 @@
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.index.query.QueryResult;
import com.google.gerrit.server.change.ChangeJson;
-import com.google.gerrit.server.index.change.ChangeField;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.util.ArrayList;
@@ -138,15 +134,11 @@
int cnt = queries.size();
List<QueryResult<ChangeData>> results = imp.query(qb.parse(queries));
- boolean requireLazyLoad =
- containsAnyOf(options, ImmutableSet.of(DETAILED_LABELS, LABELS))
- && !qb.getArgs().getSchema().hasField(ChangeField.STORED_SUBMIT_RECORD_LENIENT);
-
ChangeJson cjson = json.create(options);
cjson.setPluginDefinedAttributesFactory(this.imp);
List<List<ChangeInfo>> res =
cjson
- .lazyLoad(requireLazyLoad || containsAnyOf(options, ChangeJson.REQUIRE_LAZY_LOAD))
+ .lazyLoad(containsAnyOf(options, ChangeJson.REQUIRE_LAZY_LOAD))
.formatQueryResults(results);
for (int n = 0; n < cnt; n++) {
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
index 2d8da15..ae28a05 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
@@ -60,6 +60,10 @@
type: Boolean,
value: false,
},
+ _isAdmin: {
+ type: Boolean,
+ value: false,
+ },
_showGroup: Boolean,
_showGroupAuditLog: Boolean,
_showGroupList: Boolean,
@@ -155,7 +159,7 @@
},
],
};
- if (this._groupOwner) {
+ if (this._isAdmin || this._groupOwner) {
linkCopy.subsection.children.push(
{
name: 'Audit Log',
@@ -242,16 +246,20 @@
_computeGroupName(groupId) {
if (!groupId) { return ''; }
+ const promises = [];
this.$.restAPI.getGroupConfig(groupId).then(group => {
this._groupName = group.name;
this.reload();
- this.$.restAPI.getIsGroupOwner(group.name).then(
+ promises.push(this.$.restAPI.getIsAdmin().then(isAdmin => {
+ this._isAdmin = isAdmin;
+ }));
+ promises.push(this.$.restAPI.getIsGroupOwner(group.name).then(
isOwner => {
- if (isOwner) {
- this._groupOwner = true;
- this.reload();
- }
- });
+ this._groupOwner = isOwner;
+ }));
+ return Promise.all(promises).then(() => {
+ this.reload();
+ });
});
},
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
index 9371b17..9e08381 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
@@ -67,9 +67,18 @@
font-family: var(--font-family-bold);
text-align: left;
}
+ .canModify #groupMemberSearchInput,
+ .canModify #saveGroupMember,
+ .canModify .deleteHeader,
+ .canModify .deleteColumn,
+ .canModify #includedGroupSearchInput,
+ .canModify #saveIncludedGroups,
+ .canModify .deleteIncludedHeader,
+ .canModify #saveIncludedGroups {
+ display: none;
+ }
</style>
- <style include="gr-form-styles"></style>
- <main class="gr-form-styles">
+ <main class$="gr-form-styles [[_computeHideItemClass(_groupOwner, _isAdmin)]]">
<div id="loading" class$="[[_computeLoadingClass(_loading)]]">
Loading...
</div>
@@ -84,15 +93,13 @@
id="groupMemberSearchInput"
text="{{_groupMemberSearch}}"
query="[[_queryMembers]]"
- placeholder="Name Or Email"
- hidden$="[[!_groupOwner]]">
+ placeholder="Name Or Email">
</gr-autocomplete>
</span>
<gr-button
id="saveGroupMember"
on-tap="_handleSavingGroupMember"
- disabled="[[!_groupMemberSearch]]"
- hidden$="[[!_groupOwner]]">
+ disabled="[[!_groupMemberSearch]]">
Add
</gr-button>
<div class="gr-form-styles">
@@ -100,9 +107,7 @@
<tr class="headerRow">
<th class="nameHeader">Name</th>
<th class="emailAddressHeader">Email Address</th>
- <th class="deleteHeader" hidden$="[[!_groupOwner]]">
- Delete Member
- </th>
+ <th class="deleteHeader">Delete Member</th>
</tr>
<tbody>
<template is="dom-repeat" items="[[_groupMembers]]">
@@ -111,7 +116,7 @@
<gr-account-link account="[[item]]"></gr-account-link>
</td>
<td>[[item.email]]</td>
- <td hidden$="[[!_groupOwner]]">
+ <td class="deleteColumn">
<gr-button
class="deleteButton"
on-tap="_handleDeleteMember">
@@ -133,15 +138,13 @@
id="includedGroupSearchInput"
text="{{_includedGroupSearch}}"
query="[[_queryIncludedGroup]]"
- placeholder="Group Name"
- hidden$="[[!_groupOwner]]">
+ placeholder="Group Name">
</gr-autocomplete>
</span>
<gr-button
id="saveIncludedGroups"
on-tap="_handleSavingIncludedGroups"
- disabled="[[!_includedGroupSearch]]"
- hidden$="[[!_groupOwner]]">
+ disabled="[[!_includedGroupSearch]]">
Add
</gr-button>
<div class="gr-form-styles">
@@ -149,7 +152,7 @@
<tr class="headerRow">
<th class="groupNameHeader">Group Name</th>
<th class="descriptionHeader">Description</th>
- <th class="deleteIncludedHeader" hidden$="[[!_groupOwner]]">
+ <th class="deleteIncludedHeader">
Delete Included Group
</th>
</tr>
@@ -162,7 +165,7 @@
</a>
</td>
<td>[[item.description]]</td>
- <td hidden$="[[!_groupOwner]]">
+ <td class="deleteColumn">
<gr-button
class="deleteIncludedGroupButton"
on-tap="_handleDeleteIncludedGroup">
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js
index cde6c66..81bb902 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js
@@ -48,6 +48,10 @@
type: Boolean,
value: false,
},
+ _isAdmin: {
+ type: Boolean,
+ value: false,
+ },
},
behaviors: [
@@ -68,17 +72,29 @@
return this.$.restAPI.getGroupConfig(this.groupId).then(
config => {
+ if (!config.name) { return; }
+
this._groupName = config.name;
+
+ promises.push(this.$.restAPI.getIsAdmin().then(isAdmin => {
+ this._isAdmin = isAdmin ? true : false;
+ }));
+
promises.push(this.$.restAPI.getIsGroupOwner(config.name)
- .then(isOwner => { this._groupOwner = isOwner; }));
+ .then(isOwner => {
+ this._groupOwner = isOwner ? true : false;
+ }));
+
promises.push(this.$.restAPI.getGroupMembers(config.name).then(
members => {
this._groupMembers = members;
}));
+
promises.push(this.$.restAPI.getIncludedGroup(config.name)
.then(includedGroup => {
this._includedGroups = includedGroup;
}));
+
return Promise.all(promises).then(() => {
this._loading = false;
});
@@ -223,5 +239,9 @@
return groups;
});
},
+
+ _computeHideItemClass(owner, admin) {
+ return admin || owner ? '' : 'canModify';
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.html b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.html
index 76a182b..ef89e37 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.html
@@ -152,5 +152,23 @@
done();
});
});
+
+ test('_computeHideItemClass returns string for admin', () => {
+ const admin = true;
+ const owner = false;
+ assert.equal(element._computeHideItemClass(owner, admin), '');
+ });
+
+ test('_computeHideItemClass returns hideItem for admin and owner', () => {
+ const admin = false;
+ const owner = false;
+ assert.equal(element._computeHideItemClass(owner, admin), 'canModify');
+ });
+
+ test('_computeHideItemClass returns string for owner', () => {
+ const admin = false;
+ const owner = true;
+ assert.equal(element._computeHideItemClass(owner, admin), '');
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group.html b/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
index 2f3d59a..0449ffa 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
@@ -71,13 +71,15 @@
<gr-autocomplete
id="groupNameInput"
text="{{_groupConfig.name}}"
- disabled="[[!_groupOwner]]"></gr-autocomplete>
+ disabled="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]"></gr-autocomplete>
</span>
- <gr-button
- id="inputUpdateNameBtn"
- on-tap="_handleSaveName"
- disabled="[[_computeButtonDisabled(_groupOwner, _rename)]]">
- Rename Group</gr-button>
+ <span class="value" disabled$="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]">
+ <gr-button
+ id="inputUpdateNameBtn"
+ on-tap="_handleSaveName"
+ disabled="[[!_rename]]">
+ Rename Group</gr-button>
+ </span>
</fieldset>
<h3 class$="[[_computeHeaderClass(_owner)]]">
Owners
@@ -87,13 +89,15 @@
<gr-autocomplete
text="{{_groupConfig.owner}}"
query="[[_query]]"
- disabled$="[[!_groupOwner]]">
+ disabled="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]">
</gr-autocomplete>
</span>
- <gr-button
- on-tap="_handleSaveOwner"
- disabled="[[_computeButtonDisabled(_groupOwner, _owner)]]">
- Change Owners</gr-button>
+ <span class="value" disabled$="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]">
+ <gr-button
+ on-tap="_handleSaveOwner"
+ disabled="[[!_owner]]">
+ Change Owners</gr-button>
+ </span>
</fieldset>
<h3 class$="[[_computeHeaderClass(_description)]]">
Description
@@ -104,14 +108,15 @@
class="description"
autocomplete="on"
bind-value="{{_groupConfig.description}}"
- disabled="[[!_groupOwner]]"></iron-autogrow-textarea>
+ disabled="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]"></iron-autogrow-textarea>
</div>
- <gr-button
- on-tap="_handleSaveDescription"
- disabled=
- "[[_computeButtonDisabled(_groupOwner, _description)]]">
- Save Description
- </gr-button>
+ <span class="value" disabled$="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]">
+ <gr-button
+ on-tap="_handleSaveDescription"
+ disabled="[[!_description]]">
+ Save Description
+ </gr-button>
+ </span>
</fieldset>
<h3 id="options" class$="[[_computeHeaderClass(_options)]]">
Group Options
@@ -124,7 +129,7 @@
<span class="value">
<gr-select
bind-value="{{_groupConfig.options.visible_to_all}}">
- <select disabled$="[[!_groupOwner]]">
+ <select disabled$="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]">
<template is="dom-repeat" items="[[_submitTypes]]">
<option value="[[item.value]]">[[item.label]]</option>
</template>
@@ -132,12 +137,13 @@
</gr-select>
</span>
</section>
- <gr-button
- on-tap="_handleSaveOptions"
- disabled=
- "[[_computeButtonDisabled(_groupOwner, _options)]]">
- Save Group Options
- </gr-button>
+ <span class="value" disabled$="[[_computeGroupDisabled(_groupOwner, _isAdmin)]]">
+ <gr-button
+ on-tap="_handleSaveOptions"
+ disabled="[[!_options]]">
+ Save Group Options
+ </gr-button>
+ </span>
</fieldset>
</fieldset>
</div>
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group.js b/polygerrit-ui/app/elements/admin/gr-group/gr-group.js
index 55bf3b0..9e0e8c6 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group.js
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group.js
@@ -56,11 +56,6 @@
type: Boolean,
value: true,
},
- _loggedIn: {
- type: Boolean,
- value: false,
- observer: '_loggedInChanged',
- },
/** @type {?} */
_groupConfig: Object,
_groupName: Object,
@@ -80,6 +75,10 @@
return this._getGroupSuggestions.bind(this);
},
},
+ _isAdmin: {
+ type: Boolean,
+ value: false,
+ },
},
observers: [
@@ -98,12 +97,22 @@
return this.$.restAPI.getGroupConfig(this.groupId).then(
config => {
- this._groupConfig = config;
this._groupName = config.name;
- this.fire('title-change', {title: config.name});
- this._loading = false;
+
+ this.$.restAPI.getIsAdmin().then(isAdmin => {
+ this._isAdmin = isAdmin ? true : false;
+ });
+
this.$.restAPI.getIsGroupOwner(config.name)
- .then(isOwner => { this._groupOwner = isOwner; });
+ .then(isOwner => {
+ this._groupOwner = isOwner ? true : false;
+ });
+
+ this._groupConfig = config;
+
+ this.fire('title-change', {title: config.name});
+
+ this._loading = false;
});
},
@@ -111,18 +120,10 @@
return loading ? 'loading' : '';
},
- _loggedInChanged(_loggedIn) {
- if (!_loggedIn) { return; }
- },
-
_isLoading() {
return this._loading || this._loading === undefined;
},
- _getLoggedIn() {
- return this.$.restAPI.getLoggedIn();
- },
-
_handleSaveName() {
return this.$.restAPI.saveGroupName(this.groupId, this._groupConfig.name)
.then(config => {
@@ -182,10 +183,6 @@
this._options = true;
},
- _computeButtonDisabled(options, option) {
- return !options || !option;
- },
-
_computeHeaderClass(configChanged) {
return configChanged ? 'edited' : '';
},
@@ -204,5 +201,9 @@
return groups;
});
},
+
+ _computeGroupDisabled(owner, admin) {
+ return admin || owner ? false : true;
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group_test.html b/polygerrit-ui/app/elements/admin/gr-group/gr-group_test.html
index 75bd905..3c3b938 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group_test.html
@@ -121,5 +121,29 @@
done();
});
});
+
+ test('_computeGroupDisabled return false for admin', () => {
+ const admin = true;
+ const owner = false;
+ assert.equal(element._computeGroupDisabled(owner, admin), false);
+ });
+
+ test('_computeGroupDisabled return true for admin', () => {
+ const admin = false;
+ const owner = false;
+ assert.equal(element._computeGroupDisabled(owner, admin), true);
+ });
+
+ test('_computeGroupDisabled return false for owner', () => {
+ const admin = false;
+ const owner = true;
+ assert.equal(element._computeGroupDisabled(owner, admin), false);
+ });
+
+ test('_computeGroupDisabled return true for owner', () => {
+ const admin = false;
+ const owner = false;
+ assert.equal(element._computeGroupDisabled(owner, admin), true);
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js
index d5506ad..20fd147 100644
--- a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js
+++ b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js
@@ -16,6 +16,13 @@
Polymer({
is: 'gr-label-score-row',
+
+ /**
+ * Fired when any label is changed.
+ *
+ * @event labels-changed
+ */
+
properties: {
/**
* @type {{ name: string }}
@@ -96,6 +103,7 @@
// nothing and then to the new item.
if (!e.target.selectedItem) { return; }
this._selectedValueText = e.target.selectedItem.getAttribute('title');
+ this.fire('labels-changed');
},
_computeAnyPermittedLabelValues(permittedLabels, label) {
diff --git a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_test.html b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_test.html
index 537bd25..da450e1 100644
--- a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_test.html
+++ b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_test.html
@@ -103,6 +103,8 @@
});
test('label picker', () => {
+ const labelsChangedHandler = sandbox.stub();
+ element.addEventListener('labels-changed', labelsChangedHandler);
assert.ok(element.$$('iron-selector'));
MockInteractions.tap(element.$$(
'gr-button[value="-1"]'));
@@ -112,6 +114,7 @@
.textContent.trim(), '-1');
assert.strictEqual(
element.$.selectedValueLabel.textContent.trim(), 'bad');
+ assert.isTrue(labelsChangedHandler.called);
});
test('correct item is selected', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
index 2f9f78c..24673ff 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
@@ -241,6 +241,7 @@
id="labelScores"
account="[[_account]]"
change="[[change]]"
+ on-labels-changed="_handleLabelsChanged"
permitted-labels=[[permittedLabels]]></gr-label-scores>
</section>
<section class="draftsContainer" hidden$="[[_computeHideDraftList(diffDrafts)]]">
@@ -265,7 +266,7 @@
<section>
<gr-button
primary
- disabled="[[_isState(knownLatestState, 'not-latest')]]"
+ disabled="[[_computeSendButtonDisabled(knownLatestState, _sendButtonLabel, diffDrafts, draft, _reviewersMutated, _labelsChanged)]]"
class="action send"
on-tap="_sendTapHandler">[[_sendButtonLabel]]</gr-button>
<template is="dom-if" if="[[canBeStarted]]">
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index 752f15a..9f4a79a 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -34,6 +34,11 @@
NOT_LATEST: 'not-latest',
};
+ const ButtonLabels = {
+ START_REVIEW: 'Start review',
+ SEND: 'Send',
+ };
+
// TODO(logan): Remove once the fix for issue 6841 is stable on
// googlesource.com.
const START_REVIEW_MESSAGE = 'This change is ready for review.';
@@ -175,6 +180,14 @@
computed: '_computeCCsEnabled(serverConfig)',
},
_savingComments: Boolean,
+ _reviewersMutated: {
+ type: Boolean,
+ value: false,
+ },
+ _labelsChanged: {
+ type: Boolean,
+ value: false,
+ },
},
FocusTarget,
@@ -268,12 +281,14 @@
_ccsChanged(splices) {
if (splices && splices.indexSplices) {
+ this._reviewersMutated = true;
this._processReviewerChange(splices.indexSplices, ReviewerTypes.CC);
}
},
_reviewersChanged(splices) {
if (splices && splices.indexSplices) {
+ this._reviewersMutated = true;
this._processReviewerChange(splices.indexSplices,
ReviewerTypes.REVIEWER);
let key;
@@ -768,6 +783,11 @@
});
},
+ _handleLabelsChanged() {
+ this._labelsChanged = Object.keys(
+ this.$.labelScores.getLabelValues()).length !== 0;
+ },
+
_isState(knownLatestState, value) {
return knownLatestState === value;
},
@@ -778,7 +798,7 @@
},
_computeSendButtonLabel(canBeStarted) {
- return canBeStarted ? 'Start review' : 'Send';
+ return canBeStarted ? ButtonLabels.START_REVIEW : ButtonLabels.SEND;
},
_computeCCsEnabled(serverConfig) {
@@ -788,5 +808,17 @@
_computeSavingLabelClass(savingComments) {
return savingComments ? 'saving' : '';
},
+
+ _computeSendButtonDisabled(knownLatestState, buttonLabel, drafts, text,
+ reviewersMutated, labelsChanged) {
+ if (this._isState(knownLatestState, LatestPatchState.NOT_LATEST)) {
+ return true;
+ }
+ if (buttonLabel === ButtonLabels.START_REVIEW) {
+ return false;
+ }
+ return !(drafts.length || text.length || reviewersMutated ||
+ labelsChanged);
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
index 0f90019..9a83259 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
@@ -55,6 +55,8 @@
stub('gr-rest-api-interface', {
getConfig() { return Promise.resolve({}); },
getAccount() { return Promise.resolve({}); },
+ getChange() { return Promise.resolve({}); },
+ getChangeSuggestedReviewers() { return Promise.resolve([]); },
});
element = fixture('basic');
@@ -1007,5 +1009,20 @@
});
});
});
+
+ test('_computeSendButtonDisabled', () => {
+ const fn = element._computeSendButtonDisabled.bind(element);
+ assert.isTrue(fn('not-latest'));
+ assert.isFalse(fn('latest', 'Start review'));
+ assert.isTrue(fn('latest', 'Send', [], '', false, false));
+ // Mock nonempty comment draft array.
+ assert.isFalse(fn('latest', 'Send', ['test'], '', false, false));
+ // Mock nonempty change message.
+ assert.isFalse(fn('latest', 'Send', [], 'test', false, false));
+ // Mock reviewers mutated.
+ assert.isFalse(fn('latest', 'Send', [], '', true, false));
+ // Mock labels changed.
+ assert.isFalse(fn('latest', 'Send', [], '', false, true));
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.html b/polygerrit-ui/app/elements/core/gr-router/gr-router.html
index f07daa5..2ef65f4 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.html
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.html
@@ -23,6 +23,9 @@
<link rel="import" href="../gr-reporting/gr-reporting.html">
<dom-module id="gr-router">
+ <template>
+ <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
+ </template>
<script src="../../../bower_components/page/page.js"></script>
<script src="gr-router.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
index acc9d4f..3a724bb 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -154,10 +154,6 @@
is: 'gr-router',
properties: {
- _restAPI: {
- type: Object,
- value: () => document.createElement('gr-rest-api-interface'),
- },
_app: {
type: Object,
value: app,
@@ -277,8 +273,12 @@
_normalizeLegacyRouteParams(params) {
if (!params.changeNum) { return Promise.resolve(); }
- return this._restAPI.getFromProjectLookup(params.changeNum)
+ return this.$.restAPI.getFromProjectLookup(params.changeNum)
.then(project => {
+ // Do nothing if the lookup request failed. This avoids an infinite
+ // loop of project lookups.
+ if (!project) { return; }
+
params.project = project;
this._normalizePatchRangeParams(params);
this._redirect(this._generateUrl(params));
@@ -370,7 +370,7 @@
* (if it resolves).
*/
_redirectIfNotLoggedIn(data) {
- return this._restAPI.getLoggedIn().then(loggedIn => {
+ return this.$.restAPI.getLoggedIn().then(loggedIn => {
if (loggedIn) {
return Promise.resolve();
} else {
@@ -382,7 +382,7 @@
/** Page.js middleware that warms the REST API's logged-in cache line. */
_loadUserMiddleware(ctx, next) {
- this._restAPI.getLoggedIn().then(() => { next(); });
+ this.$.restAPI.getLoggedIn().then(() => { next(); });
},
/**
@@ -447,7 +447,8 @@
this._mapRoute(RoutePattern.GROUP_AUDIT_LOG, '_handleGroupAuditLogRoute',
true);
- this._mapRoute(RoutePattern.GROUP_MEMBERS, '_handleGroupMembersRoute');
+ this._mapRoute(RoutePattern.GROUP_MEMBERS, '_handleGroupMembersRoute',
+ true);
this._mapRoute(RoutePattern.GROUP_LIST_OFFSET,
'_handleGroupListOffsetRoute', true);
@@ -584,7 +585,7 @@
this._redirect(newUrl);
return null;
}
- return this._restAPI.getLoggedIn().then(loggedIn => {
+ return this.$.restAPI.getLoggedIn().then(loggedIn => {
if (loggedIn) {
this._redirect('/dashboard/self');
} else {
@@ -599,7 +600,7 @@
return;
}
- return this._restAPI.getLoggedIn().then(loggedIn => {
+ return this.$.restAPI.getLoggedIn().then(loggedIn => {
if (!loggedIn) {
if (data.params[0].toLowerCase() === 'self') {
this._redirectToLogin(data.canonicalPath);
@@ -937,7 +938,7 @@
this._redirect(this._generateUrl(params));
} else {
this._setParams(params);
- this._restAPI.setInProjectLookup(params.changeNum,
+ this.$.restAPI.setInProjectLookup(params.changeNum,
params.project);
}
},
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
index a9231b6..c294b22 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
@@ -133,6 +133,7 @@
'_handleGroupListFilterOffsetRoute',
'_handleGroupListFilterRoute',
'_handleGroupListOffsetRoute',
+ '_handleGroupMembersRoute',
'_handleGroupRoute',
'_handlePluginListFilterOffsetRoute',
'_handlePluginListFilterRoute',
@@ -153,7 +154,6 @@
'_handleDefaultRoute',
'_handleChangeLegacyRoute',
'_handleDiffLegacyRoute',
- '_handleGroupMembersRoute',
'_handleImproperlyEncodedPlusRoute',
'_handlePassThroughRoute',
'_handleProjectAccessRoute',
@@ -183,7 +183,7 @@
});
test('_redirectIfNotLoggedIn while logged in', () => {
- sandbox.stub(element._restAPI, 'getLoggedIn')
+ sandbox.stub(element.$.restAPI, 'getLoggedIn')
.returns(Promise.resolve(true));
const data = {canonicalPath: ''};
const redirectStub = sandbox.stub(element, '_redirectToLogin');
@@ -193,7 +193,7 @@
});
test('_redirectIfNotLoggedIn while logged out', () => {
- sandbox.stub(element._restAPI, 'getLoggedIn')
+ sandbox.stub(element.$.restAPI, 'getLoggedIn')
.returns(Promise.resolve(false));
const redirectStub = sandbox.stub(element, '_redirectToLogin');
const data = {canonicalPath: ''};
@@ -304,34 +304,51 @@
setup(() => {
projectLookupStub = sandbox
- .stub(element._restAPI, 'getFromProjectLookup')
- .returns(Promise.resolve('foo/bar'));
+ .stub(element.$.restAPI, 'getFromProjectLookup');
sandbox.stub(element, '_generateUrl');
});
suite('_normalizeLegacyRouteParams', () => {
let rangeStub;
+ let redirectStub;
setup(() => {
rangeStub = sandbox.stub(element, '_normalizePatchRangeParams')
.returns(Promise.resolve());
+ redirectStub = sandbox.stub(element, '_redirect');
});
test('w/o changeNum', () => {
+ projectLookupStub.returns(Promise.resolve('foo/bar'));
const params = {};
return element._normalizeLegacyRouteParams(params).then(() => {
assert.isFalse(projectLookupStub.called);
assert.isFalse(rangeStub.called);
assert.isNotOk(params.project);
+ assert.isFalse(redirectStub.called);
});
});
test('w/ changeNum', () => {
+ projectLookupStub.returns(Promise.resolve('foo/bar'));
const params = {changeNum: 1234};
return element._normalizeLegacyRouteParams(params).then(() => {
assert.isTrue(projectLookupStub.called);
assert.isTrue(rangeStub.called);
assert.equal(params.project, 'foo/bar');
+ assert.isTrue(redirectStub.calledOnce);
+ });
+ });
+
+ test('halts on project lookup failure', () => {
+ projectLookupStub.returns(Promise.resolve(undefined));
+
+ const params = {changeNum: 1234};
+ return element._normalizeLegacyRouteParams(params).then(() => {
+ assert.isTrue(projectLookupStub.called);
+ assert.isFalse(rangeStub.called);
+ assert.isUndefined(params.project);
+ assert.isFalse(redirectStub.called);
});
});
});
@@ -472,7 +489,7 @@
});
test('redirects to dahsboard if logged in', () => {
- sandbox.stub(element._restAPI, 'getLoggedIn')
+ sandbox.stub(element.$.restAPI, 'getLoggedIn')
.returns(Promise.resolve(true));
const data = {
canonicalPath: '/', path: '/', querystring: '', hash: '',
@@ -485,7 +502,7 @@
});
test('redirects to open changes if not logged in', () => {
- sandbox.stub(element._restAPI, 'getLoggedIn')
+ sandbox.stub(element.$.restAPI, 'getLoggedIn')
.returns(Promise.resolve(false));
const data = {
canonicalPath: '/', path: '/', querystring: '', hash: '',
@@ -592,7 +609,7 @@
});
test('own dahsboard but signed out redirects to login', () => {
- sandbox.stub(element._restAPI, 'getLoggedIn')
+ sandbox.stub(element.$.restAPI, 'getLoggedIn')
.returns(Promise.resolve(false));
const data = {canonicalPath: '/dashboard', params: {0: 'seLF'}};
const result = element._handleDashboardRoute(data);
@@ -605,7 +622,7 @@
});
test('non-self dahsboard but signed out does not redirect', () => {
- sandbox.stub(element._restAPI, 'getLoggedIn')
+ sandbox.stub(element.$.restAPI, 'getLoggedIn')
.returns(Promise.resolve(false));
const data = {canonicalPath: '/dashboard', params: {0: 'foo'}};
const result = element._handleDashboardRoute(data);
@@ -619,7 +636,7 @@
});
test('dahsboard while signed in sets params', () => {
- sandbox.stub(element._restAPI, 'getLoggedIn')
+ sandbox.stub(element.$.restAPI, 'getLoggedIn')
.returns(Promise.resolve(true));
const data = {canonicalPath: '/dashboard', params: {0: 'foo'}};
const result = element._handleDashboardRoute(data);
@@ -1051,7 +1068,7 @@
setup(() => {
normalizeRangeStub = sandbox.stub(element,
'_normalizePatchRangeParams');
- sandbox.stub(element._restAPI, 'setInProjectLookup');
+ sandbox.stub(element.$.restAPI, 'setInProjectLookup');
});
test('needs redirect', () => {
@@ -1103,7 +1120,7 @@
test('_handleDiffEditRoute', () => {
const normalizeRangeStub =
sandbox.stub(element, '_normalizePatchRangeParams');
- sandbox.stub(element._restAPI, 'setInProjectLookup');
+ sandbox.stub(element.$.restAPI, 'setInProjectLookup');
const ctx = {
params: [
'foo/bar', // 0 Project
diff --git a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html
index c93ed0c..43343de 100644
--- a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html
+++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html
@@ -49,6 +49,17 @@
</tr>
</thead>
<tbody>
+ <tr>
+ <td>Number</td>
+ <td
+ class="checkboxContainer"
+ on-tap="_handleTargetTap">
+ <input
+ type="checkbox"
+ name="number"
+ checked$="[[showNumber]]">
+ </td>
+ </tr>
<template is="dom-repeat" items="[[columnNames]]">
<tr>
<td>[[item]]</td>
diff --git a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.js b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.js
index 50a1146..4d87f9e 100644
--- a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.js
+++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.js
@@ -22,6 +22,10 @@
type: Array,
notify: true,
},
+ showNumber: {
+ type: Boolean,
+ notify: true,
+ },
},
behaviors: [
@@ -53,6 +57,12 @@
// The target is the checkbox itself.
checkbox = Polymer.dom(e).rootTarget;
}
+
+ if (checkbox.name === 'number') {
+ this.showNumber = checkbox.checked;
+ return;
+ }
+
this.set('displayedColumns',
this._updateDisplayedColumns(
this.displayedColumns, checkbox.name, checkbox.checked));
diff --git a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html
index 61093b9..00ef16f 100644
--- a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html
+++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html
@@ -62,15 +62,17 @@
const rows = element.$$('tbody').querySelectorAll('tr');
let tds;
- assert.equal(rows.length, element.columnNames.length);
+ // The `+ 1` is for the number column, which isn't included in the change
+ // table behavior's list.
+ assert.equal(rows.length, element.columnNames.length + 1);
for (let i = 0; i < columns.length; i++) {
- tds = rows[i].querySelectorAll('td');
+ tds = rows[i + 1].querySelectorAll('td');
assert.equal(tds[0].textContent, columns[i]);
}
});
test('hide item', () => {
- const checkbox = element.$$('table input');
+ const checkbox = element.$$('table tr:nth-child(2) input');
const isChecked = checkbox.checked;
const displayedLength = element.displayedColumns.length;
assert.isTrue(isChecked);
@@ -78,8 +80,7 @@
MockInteractions.tap(checkbox);
flushAsynchronousOperations();
- assert.equal(element.displayedColumns.length,
- displayedLength - 1);
+ assert.equal(element.displayedColumns.length, displayedLength - 1);
});
test('show item', () => {
@@ -91,7 +92,7 @@
'Updated',
]);
flushAsynchronousOperations();
- const checkbox = element.$$('table input');
+ const checkbox = element.$$('table tr:nth-child(2) input');
const isChecked = checkbox.checked;
const displayedLength = element.displayedColumns.length;
assert.isFalse(isChecked);
@@ -105,9 +106,9 @@
});
test('_handleTargetTap', () => {
- const checkbox = element.$$('table input');
+ const checkbox = element.$$('table tr:nth-child(2) input');
let originalDisplayedColumns = element.displayedColumns;
- const td = element.$$('table .checkboxContainer');
+ const td = element.$$('table tr:nth-child(2) .checkboxContainer');
const displayedColumnStub =
sandbox.stub(element, '_updateDisplayedColumns');
@@ -125,6 +126,20 @@
checkbox.checked));
});
+ test('_handleTargetTap on number', () => {
+ element.showNumber = false;
+ const checkbox = element.$$('table tr:nth-child(1) input');
+ const displayedColumnStub =
+ sandbox.stub(element, '_updateDisplayedColumns');
+
+ MockInteractions.tap(checkbox);
+ assert.isFalse(displayedColumnStub.called);
+ assert.isTrue(element.showNumber);
+
+ MockInteractions.tap(checkbox);
+ assert.isFalse(element.showNumber);
+ });
+
test('_updateDisplayedColumns', () => {
let name = 'Subject';
let checked = false;
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
index d039a3b..cacccda 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
@@ -321,6 +321,7 @@
</h2>
<fieldset id="changeTableColumns">
<gr-change-table-editor
+ show-number="{{_showNumber}}"
displayed-columns="{{_localChangeTableColumns}}">
</gr-change-table-editor>
<gr-button
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js
index c3332b3..25dd259 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js
@@ -121,6 +121,8 @@
* For testing purposes.
*/
_loadingPromise: Object,
+
+ _showNumber: Boolean,
},
behaviors: [
@@ -132,7 +134,7 @@
'_handlePrefsChanged(_localPrefs.*)',
'_handleDiffPrefsChanged(_diffPrefs.*)',
'_handleMenuChanged(_localMenu.splices)',
- '_handleChangeTableChanged(_localChangeTableColumns)',
+ '_handleChangeTableChanged(_localChangeTableColumns, _showNumber)',
],
attached() {
@@ -147,6 +149,7 @@
promises.push(this.$.restAPI.getPreferences().then(prefs => {
this.prefs = prefs;
+ this._showNumber = !!prefs.legacycid_in_change_table;
this._copyPrefs('_localPrefs', 'prefs');
this._cloneMenu();
this._cloneChangeTableColumns();
@@ -303,6 +306,7 @@
_handleSaveChangeTable() {
this.set('prefs.change_table', this._localChangeTableColumns);
+ this.set('prefs.legacycid_in_change_table', this._showNumber);
this._cloneChangeTableColumns();
return this.$.restAPI.savePreferences(this.prefs).then(() => {
this._changeTableChanged = false;
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html
index e1182c1..868a9e3 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html
@@ -386,6 +386,25 @@
assert.isTrue(element.$.emailEditor.loadData.calledOnce);
});
+ test('_handleSaveChangeTable', () => {
+ let newColumns = ['Owner', 'Project', 'Branch'];
+ element._localChangeTableColumns = newColumns.slice(0);
+ element._showNumber = false;
+ const cloneStub = sandbox.stub(element, '_cloneChangeTableColumns');
+ element._handleSaveChangeTable();
+ assert.isTrue(cloneStub.calledOnce);
+ assert.deepEqual(element.prefs.change_table, newColumns);
+ assert.isNotOk(element.prefs.legacycid_in_change_table);
+
+ newColumns = ['Size'];
+ element._localChangeTableColumns = newColumns;
+ element._showNumber = true;
+ element._handleSaveChangeTable();
+ assert.isTrue(cloneStub.calledTwice);
+ assert.deepEqual(element.prefs.change_table, newColumns);
+ assert.isTrue(element.prefs.legacycid_in_change_table);
+ });
+
suite('_getFilterDocsLink', () => {
test('with http: docs base URL', () => {
const base = 'http://example.com/';
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 210c42f..06a3fcf 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
@@ -274,7 +274,7 @@
getGroupConfig(group) {
const encodeName = encodeURIComponent(group);
- return this._fetchSharedCacheURL('/groups/' + encodeName + '/detail');
+ return this.fetchJSON(`/groups/${encodeName}/detail`);
},
/**
@@ -348,8 +348,8 @@
*/
getIsGroupOwner(groupName) {
const encodeName = encodeURIComponent(groupName);
- return this._fetchSharedCacheURL('/groups/?owned&q=' + encodeName)
- .then(configs => configs.hasOwnProperty(encodeName));
+ return this._fetchSharedCacheURL(`/groups/?owned&q=${encodeName}`)
+ .then(configs => configs.hasOwnProperty(groupName));
},
getGroupMembers(groupName) {
@@ -1645,7 +1645,7 @@
setChangeTopic(changeNum, topic) {
const p = {topic};
return this.getChangeURLAndSend(changeNum, 'PUT', null, '/topic', p)
- .then(this.getResponseObject);
+ .then(this.getResponseObject.bind(this));
},
/**
@@ -1655,7 +1655,7 @@
*/
setChangeHashtag(changeNum, hashtag) {
return this.getChangeURLAndSend(changeNum, 'POST', null, '/hashtags',
- hashtag).then(this.getResponseObject);
+ hashtag).then(this.getResponseObject.bind(this));
},
deleteAccountHttpPassword() {
@@ -1669,7 +1669,7 @@
*/
generateAccountHttpPassword() {
return this.send('PUT', '/accounts/self/password.http', {generate: true})
- .then(this.getResponseObject);
+ .then(this.getResponseObject.bind(this));
},
getAccountSSHKeys() {
@@ -1772,7 +1772,7 @@
const endpoint = `/comments/${commentID}/delete`;
const payload = {reason};
return this.getChangeURLAndSend(changeNum, 'POST', patchNum, endpoint,
- payload).then(this.getResponseObject);
+ payload).then(this.getResponseObject.bind(this));
},
/**
@@ -1811,7 +1811,13 @@
getFromProjectLookup(changeNum) {
const project = this._projectLookup[changeNum];
if (project) { return Promise.resolve(project); }
- return this.getChange(changeNum).then(change => {
+
+ const onError = response => {
+ // Fire a page error so that the visual 404 is displayed.
+ this.fire('page-error', {response});
+ };
+
+ return this.getChange(changeNum, onError).then(change => {
if (!change || !change.project) { return; }
this.setInProjectLookup(changeNum, change.project);
return change.project;
@@ -1842,7 +1848,7 @@
/**
* Alias for _changeBaseURL.then(fetchJSON).
- * @todo(beckysiegel) clean up comments
+ * @todo(beckysiegel) clean up comments
* @param {string|number} changeNum
* @param {string} endpoint
* @param {?string|number=} opt_patchNum gets passed as null.
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 2058f59..eb4f418 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
@@ -865,8 +865,7 @@
});
test('getChange succeeds, no project', () => {
- sandbox.stub(element, 'getChange')
- .returns(Promise.resolve({}));
+ sandbox.stub(element, 'getChange').returns(Promise.resolve());
return element.getFromProjectLookup().then(val => {
assert.strictEqual(val, undefined);
assert.deepEqual(element._projectLookup, {});
@@ -959,5 +958,29 @@
assert.deepEqual(result, obj);
});
});
+
+ test('setChangeTopic', () => {
+ const sendSpy = sandbox.spy(element, 'getChangeURLAndSend');
+ return element.setChangeTopic(123, 'foo-bar').then(() => {
+ assert.isTrue(sendSpy.calledOnce);
+ assert.deepEqual(sendSpy.lastCall.args[4], {topic: 'foo-bar'});
+ });
+ });
+
+ test('setChangeHashtag', () => {
+ const sendSpy = sandbox.spy(element, 'getChangeURLAndSend');
+ return element.setChangeHashtag(123, 'foo-bar').then(() => {
+ assert.isTrue(sendSpy.calledOnce);
+ assert.equal(sendSpy.lastCall.args[4], 'foo-bar');
+ });
+ });
+
+ test('generateAccountHttpPassword', () => {
+ const sendSpy = sandbox.spy(element, 'send');
+ return element.generateAccountHttpPassword().then(() => {
+ assert.isTrue(sendSpy.calledOnce);
+ assert.deepEqual(sendSpy.lastCall.args[2], {generate: true});
+ });
+ });
});
</script>
