Merge branch 'stable-2.16'
* stable-2.16:
Add missing @Override annotation on setDynamicBean
Release 2.16-rc3
Fix broken sentence in description of unchanged file marker
Document when 'U' for unchanged files is used in file list on change screen
ElasticContainer: Test with versions 5.6.13 and 6.4.3
Upgrade elasticsearch-rest-client to 6.4.3
Bazel: Consume versions directly from lib:versions.bzl in skylib
AbstractChangeNotes: Never open repo when NoteDb is off
dev-contributing: Specify buildifier version 0.17.2
Remove unused dependency on httpcomponents:httpmime
Fix Elasticsearch dependency on httpcore-nio
Upgrade Jetty to 9.3.24.v20180605 to fix several CVEs
[CVE-2018-1000180, CVE-2018-1000613] Upgrade Bouncycastle to 1.60
Adapt PublicKeyStoreTest to work with BouncyCastle 1.60
[CVE-2018-10237]: Upgrade guava to 24.1.1-jre
Stop using CharMatcher.javaLetterOrDigit
project/Index: Assign and ignore unused future
[CVE-2017-12629] Upgrade Lucene to 5.5.5
[CVE-2018-10936] Upgrade postgresql to 42.2.5
[CVE-2015-1832] Upgrade Apache Derby to 10.12.1.1
Set version to 2.14.17-SNAPSHOT
Change-Id: I054b451abb7974812316559b4c55d99ddf41cc5d
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index d3f5d77..13e3a53 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -5,6 +5,12 @@
to those groups. Access rights cannot be granted to individual
users.
+To view/edit the access controls for a specific project, first
+navigate to the projects page: for example,
+https://gerrit-review.googlesource.com/admin/repos/ . Then click on
+the individual project, and then click Access. This will bring you
+to a url that looks like
+https://gerrit-review.googlesource.com/admin/repos/gerrit,access
[[system_groups]]
== System Groups
diff --git a/Documentation/config-cla.txt b/Documentation/config-cla.txt
index 2234808..2c7b194 100644
--- a/Documentation/config-cla.txt
+++ b/Documentation/config-cla.txt
@@ -25,13 +25,15 @@
----
Contributor agreements are defined as contributor-agreement sections in
-`project.config`:
+`project.config` of `All-Projects`:
----
[contributor-agreement "Individual"]
description = If you are going to be contributing code on your own, this is the one you want. You can sign this one online.
agreementUrl = static/cla_individual.html
autoVerify = group CLA Accepted - Individual
accepted = group CLA Accepted - Individual
+ matchProjects = ^/.*$
+ excludeProjects = ^/not/my/project/
----
Each `contributor-agreement` section within the `project.config` file must
@@ -75,6 +77,16 @@
contributor agreement has been accepted. The groups' UUID must also
appear in the `groups` file.
+[[contributor-agreement.name.matchProjects]]contributor-agreement.<name>.matchProjects::
++
+List of project regular expressions identifying projects where the
+agreement is required. Defaults to every project when omitted.
+
+[[contributor-agreement.name.excludeProjects]]contributor-agreement.<name>.excludeProjects::
++
+List of project regular expressions identifying projects where the
+agreement does not apply. Defaults to empty. i.e. no projects excluded.
+
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index 0ecc820..a68c3ac 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -258,8 +258,10 @@
bazel test //javatests/com/google/gerrit/acceptance/rest/account:rest_account
----
-To run the tests against NoteDb backend with write
-to NoteDb, but not read from it:
+The tests run with NoteDb fully enabled by default.
+
+To run the tests against NoteDb backend with write to NoteDb, but not read from
+it:
----
bazel test --test_env=GERRIT_NOTEDB=WRITE //...
@@ -277,10 +279,10 @@
bazel test --test_env=GERRIT_NOTEDB=PRIMARY //...
----
-Primary storage NoteDb and ReviewDb disabled:
+NoteDb entirely disabled:
----
- bazel test --test_env=GERRIT_NOTEDB=ON //...
+ bazel test --test_env=GERRIT_NOTEDB=OFF //...
----
To run only tests that do not use SSH:
diff --git a/Documentation/dev-intellij.txt b/Documentation/dev-intellij.txt
index 8bedd08..5ec6ae8 100644
--- a/Documentation/dev-intellij.txt
+++ b/Documentation/dev-intellij.txt
@@ -177,9 +177,9 @@
plugin manages a project, it intercepts the creation and creates a Bazel test
run configuration instead, which can be used just like the standard ones.
-TIP: If you would like to execute a test in NoteDb mode, add
-`--test_env=GERRIT_NOTEDB=READ_WRITE` to the *Bazel flags* of your run
-configuration.
+TIP: Tests run with NoteDb enabled by default. If you would like to execute a
+test with NoteDb turned off, add `--test_env=GERRIT_NOTEDB=OFF` to the *Bazel
+flags* of your run configuration.
[[remote-debug]]
=== Debugging a remote Gerrit server
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 37d6d01..db6f883 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -177,6 +177,10 @@
* `plugin/latency`: Latency for plugin invocation.
* `plugin/error_count`: Number of plugin errors.
+=== Group
+
+* `group/guess_relevant_groups_latency`: Latency for guessing relevant groups.
+
=== Replication Plugin
* `plugins/replication/replication_latency`: Time spent pushing to remote
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 1829c6b..b1c9a88 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -5769,8 +5769,12 @@
Number of inserted lines.
|`deletions` ||
Number of deleted lines.
+|`total_comment_count` |optional|
+Total number of inline comments across all patch sets. Not set if the current
+change index doesn't have the data.
|`unresolved_comment_count` |optional|
-Number of unresolved comments. Not set if the current change index doesn't have the data.
+Number of unresolved inline comment threads across all patch sets. Not set if
+the current change index doesn't have the data.
|`_number` ||The legacy numeric ID of the change.
|`owner` ||
The owner of the change as an link:rest-api-accounts.html#account-info[
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 3214761..49ab36f 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -1358,25 +1358,6 @@
Ref(ref)::
The branch for which to check access. This must be given if `perm` is specified.
-[[check-access-post]]
-=== Check Access (POST)
-
-This endpoint can also be accessed as a POST request (deprecated). In
-this case, the input for the access checks 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"
- }
-----
-
[[index]]
=== Index project
@@ -2936,21 +2917,6 @@
|`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
-|`permission` |optional|The ref permission for which to
-check. This defaults to `read`. If given, it `ref` must be given too.
-|=========================================
-
[[auto_closeable_changes_check_input]]
=== AutoCloseableChangesCheckInput
The `AutoCloseableChangesCheckInput` entity contains options for running
diff --git a/WORKSPACE b/WORKSPACE
index 3e871a7..84bbc10 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -15,9 +15,9 @@
http_archive(
name = "io_bazel_rules_closure",
- sha256 = "4dd84dd2bdd6c9f56cb5a475d504ea31d199c34309e202e9379501d01c3067e5",
- strip_prefix = "rules_closure-3103a773820b59b76345f94c231cb213e0d404e2",
- urls = ["https://github.com/bazelbuild/rules_closure/archive/3103a773820b59b76345f94c231cb213e0d404e2.tar.gz"],
+ sha256 = "5b4b610ea4892116b6126fa689218535629305590c43fbd68034d831953a9989",
+ strip_prefix = "rules_closure-409a86250c457ca15cafde35eb169e4c2601570e",
+ urls = ["https://github.com/bazelbuild/rules_closure/archive/409a86250c457ca15cafde35eb169e4c2601570e.zip"],
)
# File is specific to Polymer and copied from the Closure Github -- should be
@@ -48,8 +48,8 @@
# Golang support for PolyGerrit local dev server.
http_archive(
name = "io_bazel_rules_go",
- sha256 = "97cf62bdef33519412167fd1e4b0810a318a7c234f5f8dc4f53e2da86241c492",
- urls = ["https://github.com/bazelbuild/rules_go/releases/download/0.15.3/rules_go-0.15.3.tar.gz"],
+ sha256 = "ee5fe78fe417c685ecb77a0a725dc9f6040ae5beb44a0ba4ddb55453aad23a8a",
+ url = "https://github.com/bazelbuild/rules_go/releases/download/0.16.0/rules_go-0.16.0.tar.gz",
)
load("@io_bazel_rules_go//go:def.bzl", "go_register_toolchains", "go_rules_dependencies")
@@ -108,24 +108,24 @@
sha1 = "83cd2cd674a217ade95a4bb83a8a14f351f48bd0",
)
-GUICE_VERS = "4.2.1"
+GUICE_VERS = "4.2.2"
maven_jar(
name = "guice-library",
artifact = "com.google.inject:guice:" + GUICE_VERS,
- sha1 = "f77dfd89318fe3ff293bafceaa75fbf66e4e4b10",
+ sha1 = "6dacbe18e5eaa7f6c9c36db33b42e7985e94ce77",
)
maven_jar(
name = "guice-assistedinject",
artifact = "com.google.inject.extensions:guice-assistedinject:" + GUICE_VERS,
- sha1 = "d327e4aee7c96f08cd657c17da231a1f4a8999ac",
+ sha1 = "c33fb10080d58446f752b4fcfff8a5fabb80a449",
)
maven_jar(
name = "guice-servlet",
artifact = "com.google.inject.extensions:guice-servlet:" + GUICE_VERS,
- sha1 = "3927e462f923b0c672fdb045c5645bca4beab5c0",
+ sha1 = "0d0054bdd812224078357a9b11409e43d182a046",
)
maven_jar(
@@ -586,36 +586,36 @@
sha1 = "5e3bda828a80c7a21dfbe2308d1755759c2fd7b4",
)
-OW2_VERS = "6.2.1"
+OW2_VERS = "7.0"
maven_jar(
name = "ow2-asm",
artifact = "org.ow2.asm:asm:" + OW2_VERS,
- sha1 = "c01b6798f81b0fc2c5faa70cbe468c275d4b50c7",
+ sha1 = "d74d4ba0dee443f68fb2dcb7fcdb945a2cd89912",
)
maven_jar(
name = "ow2-asm-analysis",
artifact = "org.ow2.asm:asm-analysis:" + OW2_VERS,
- sha1 = "e8b876c5ccf226cae2f44ed2c436ad3407d0ec1d",
+ sha1 = "4b310d20d6f1c6b7197a75f1b5d69f169bc8ac1f",
)
maven_jar(
name = "ow2-asm-commons",
artifact = "org.ow2.asm:asm-commons:" + OW2_VERS,
- sha1 = "eaf31376d741a3e2017248a4c759209fe25c77d3",
+ sha1 = "478006d07b7c561ae3a92ddc1829bca81ae0cdd1",
)
maven_jar(
name = "ow2-asm-tree",
artifact = "org.ow2.asm:asm-tree:" + OW2_VERS,
- sha1 = "332b022092ecec53cdb6272dc436884b2d940615",
+ sha1 = "29bc62dcb85573af6e62e5b2d735ef65966c4180",
)
maven_jar(
name = "ow2-asm-util",
artifact = "org.ow2.asm:asm-util:" + OW2_VERS,
- sha1 = "400d664d7c92a659d988c00cb65150d1b30cf339",
+ sha1 = "18d4d07010c24405129a6dbb0e92057f8779fb9d",
)
AUTO_VALUE_VERSION = "1.6.2"
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 496ee5b..d168919 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -50,6 +50,7 @@
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.data.PermissionRule.Action;
import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.api.changes.RelatedChangeAndCommitInfo;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.api.changes.SubmittedTogetherInfo;
@@ -252,6 +253,7 @@
@Inject protected MetaDataUpdate.Server metaDataUpdateFactory;
@Inject protected PatchSetUtil psUtil;
@Inject protected ProjectCache projectCache;
+ @Inject protected ProjectConfig.Factory projectConfigFactory;
@Inject protected ProjectResetter.Builder.Factory projectResetter;
@Inject protected Provider<InternalChangeQuery> queryProvider;
@Inject protected PushOneCommit.Factory pushFactory;
@@ -268,6 +270,7 @@
protected Project.NameKey project;
protected RestSession adminRestSession;
protected RestSession userRestSession;
+ protected RestSession anonymousRestSession;
protected ReviewDb db;
protected SshSession adminSshSession;
protected SshSession userSshSession;
@@ -444,6 +447,7 @@
adminRestSession = new RestSession(server, admin);
userRestSession = new RestSession(server, user);
+ anonymousRestSession = new RestSession(server, null);
initSsh();
@@ -997,7 +1001,7 @@
protected void setUseContributorAgreements(InheritableBoolean value) throws Exception {
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
config.getProject().setBooleanConfig(BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS, value);
config.commit(md);
projectCache.evict(config.getProject());
@@ -1006,7 +1010,7 @@
protected void setUseSignedOffBy(InheritableBoolean value) throws Exception {
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
config.getProject().setBooleanConfig(BooleanProjectConfig.USE_SIGNED_OFF_BY, value);
config.commit(md);
projectCache.evict(config.getProject());
@@ -1015,7 +1019,7 @@
protected void setRequireChangeId(InheritableBoolean value) throws Exception {
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
config.getProject().setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, value);
config.commit(md);
projectCache.evict(config.getProject());
@@ -1077,7 +1081,7 @@
throws RepositoryNotFoundException, IOException, ConfigInvalidException {
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
md.setMessage(String.format("Grant %s on %s", permission, ref));
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
AccessSection s = config.getAccessSection(ref, true);
Permission p = s.getPermission(permission, true);
PermissionRule rule = Util.newRule(config, groupUUID);
@@ -1101,7 +1105,7 @@
String permission = Permission.LABEL + label;
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
md.setMessage(String.format("Grant %s on %s", permission, ref));
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
AccessSection s = config.getAccessSection(ref, true);
Permission p = s.getPermission(permission, true);
p.setExclusiveGroup(exclusive);
@@ -1119,7 +1123,7 @@
throws IOException, ConfigInvalidException {
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
md.setMessage(String.format("Remove %s on %s", permission, ref));
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
AccessSection s = config.getAccessSection(ref, true);
Permission p = s.getPermission(permission, true);
p.clearRules();
@@ -1244,14 +1248,14 @@
protected ContributorAgreement configureContributorAgreement(boolean autoVerify)
throws Exception {
ContributorAgreement ca;
+ String g = createGroup(autoVerify ? "cla-test-group" : "cla-test-no-auto-verify-group");
+ GroupApi groupApi = gApi.groups().id(g);
+ groupApi.description("CLA test group");
+ InternalGroup caGroup = group(new AccountGroup.UUID(groupApi.detail().id));
+ GroupReference groupRef = new GroupReference(caGroup.getGroupUUID(), caGroup.getName());
+ PermissionRule rule = new PermissionRule(groupRef);
+ rule.setAction(PermissionRule.Action.ALLOW);
if (autoVerify) {
- String g = createGroup("cla-test-group");
- GroupApi groupApi = gApi.groups().id(g);
- groupApi.description("CLA test group");
- InternalGroup caGroup = group(new AccountGroup.UUID(groupApi.detail().id));
- GroupReference groupRef = new GroupReference(caGroup.getGroupUUID(), caGroup.getName());
- PermissionRule rule = new PermissionRule(groupRef);
- rule.setAction(PermissionRule.Action.ALLOW);
ca = new ContributorAgreement("cla-test");
ca.setAutoVerify(groupRef);
ca.setAccepted(ImmutableList.of(rule));
@@ -1260,6 +1264,8 @@
}
ca.setDescription("description");
ca.setAgreementUrl("agreement-url");
+ ca.setAccepted(ImmutableList.of(rule));
+ ca.setExcludeProjectsRegexes(ImmutableList.of("ExcludedProject"));
try (ProjectConfigUpdate u = updateProject(allProjects)) {
u.getConfig().replace(ca);
@@ -1667,7 +1673,7 @@
private ProjectConfigUpdate(Project.NameKey projectName) throws Exception {
metaDataUpdate = metaDataUpdateFactory.create(projectName);
- projectConfig = ProjectConfig.read(metaDataUpdate);
+ projectConfig = projectConfigFactory.read(metaDataUpdate);
}
public ProjectConfig getConfig() {
@@ -1714,4 +1720,13 @@
comments.sort(Comparator.comparing(c -> c.id));
return comments;
}
+
+ protected List<RelatedChangeAndCommitInfo> getRelated(PatchSet.Id ps) throws Exception {
+ return getRelated(ps.getParentKey(), ps.get());
+ }
+
+ protected List<RelatedChangeAndCommitInfo> getRelated(Change.Id changeId, int ps)
+ throws Exception {
+ return gApi.changes().id(changeId.get()).revision(ps).related().changes;
+ }
}
diff --git a/java/com/google/gerrit/acceptance/HttpSession.java b/java/com/google/gerrit/acceptance/HttpSession.java
index fe446f4..fdd1fce 100644
--- a/java/com/google/gerrit/acceptance/HttpSession.java
+++ b/java/com/google/gerrit/acceptance/HttpSession.java
@@ -19,8 +19,10 @@
import java.io.IOException;
import java.net.URI;
import org.apache.http.HttpHost;
+import org.apache.http.client.HttpClient;
import org.apache.http.client.fluent.Executor;
import org.apache.http.client.fluent.Request;
+import org.apache.http.impl.client.HttpClientBuilder;
public class HttpSession {
protected TestAccount account;
@@ -30,7 +32,8 @@
public HttpSession(GerritServer server, @Nullable TestAccount account) {
this.url = CharMatcher.is('/').trimTrailingFrom(server.getUrl());
URI uri = URI.create(url);
- this.executor = Executor.newInstance();
+ HttpClient noRedirectClient = HttpClientBuilder.create().disableRedirectHandling().build();
+ this.executor = Executor.newInstance(noRedirectClient);
this.account = account;
if (account != null) {
executor.auth(
diff --git a/java/com/google/gerrit/acceptance/RestResponse.java b/java/com/google/gerrit/acceptance/RestResponse.java
index da08215..e8de5c6 100644
--- a/java/com/google/gerrit/acceptance/RestResponse.java
+++ b/java/com/google/gerrit/acceptance/RestResponse.java
@@ -14,6 +14,7 @@
package com.google.gerrit.acceptance;
+import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.gerrit.httpd.restapi.RestApiServlet.JSON_MAGIC;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -21,6 +22,7 @@
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.Reader;
+import java.net.URI;
import org.apache.http.HttpStatus;
public class RestResponse extends HttpResponse {
@@ -83,4 +85,9 @@
public void assertPreconditionFailed() throws Exception {
assertStatus(HttpStatus.SC_PRECONDITION_FAILED);
}
+
+ public void assertTemporaryRedirect(String path) throws Exception {
+ assertStatus(HttpStatus.SC_MOVED_TEMPORARILY);
+ assertThat(URI.create(getHeader("Location")).getPath()).isEqualTo(path);
+ }
}
diff --git a/java/com/google/gerrit/common/data/ContributorAgreement.java b/java/com/google/gerrit/common/data/ContributorAgreement.java
index b43dbfa..a6e8cdd 100644
--- a/java/com/google/gerrit/common/data/ContributorAgreement.java
+++ b/java/com/google/gerrit/common/data/ContributorAgreement.java
@@ -25,6 +25,8 @@
protected List<PermissionRule> accepted;
protected GroupReference autoVerify;
protected String agreementUrl;
+ protected List<String> excludeProjectsRegexes;
+ protected List<String> matchProjectsRegexes;
protected ContributorAgreement() {}
@@ -75,6 +77,28 @@
this.agreementUrl = agreementUrl;
}
+ public List<String> getExcludeProjectsRegexes() {
+ if (excludeProjectsRegexes == null) {
+ excludeProjectsRegexes = new ArrayList<>();
+ }
+ return excludeProjectsRegexes;
+ }
+
+ public void setExcludeProjectsRegexes(List<String> excludeProjectsRegexes) {
+ this.excludeProjectsRegexes = excludeProjectsRegexes;
+ }
+
+ public List<String> getMatchProjectsRegexes() {
+ if (matchProjectsRegexes == null) {
+ matchProjectsRegexes = new ArrayList<>();
+ }
+ return matchProjectsRegexes;
+ }
+
+ public void setMatchProjectsRegexes(List<String> matchProjectsRegexes) {
+ this.matchProjectsRegexes = matchProjectsRegexes;
+ }
+
@Override
public int compareTo(ContributorAgreement o) {
return getName().compareTo(o.getName());
diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
index 0150e1e..dfa2128 100644
--- a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
@@ -49,17 +49,21 @@
* @return API for accessing the revision.
* @throws RestApiException if an error occurred.
*/
- RevisionApi current() throws RestApiException;
+ default RevisionApi current() throws RestApiException {
+ return revision("current");
+ }
/**
* Look up a revision of a change by number.
*
* @see #current()
*/
- RevisionApi revision(int id) throws RestApiException;
+ default RevisionApi revision(int id) throws RestApiException {
+ return revision(Integer.toString(id));
+ }
/**
- * Look up a revision of a change by commit SHA-1.
+ * Look up a revision of a change by commit SHA-1 or other supported revision string.
*
* @see #current()
*/
@@ -79,15 +83,23 @@
*/
ReviewerApi reviewer(String id) throws RestApiException;
- void abandon() throws RestApiException;
+ default void abandon() throws RestApiException {
+ abandon(new AbandonInput());
+ }
void abandon(AbandonInput in) throws RestApiException;
- void restore() throws RestApiException;
+ default void restore() throws RestApiException {
+ restore(new RestoreInput());
+ }
void restore(RestoreInput in) throws RestApiException;
- void move(String destination) throws RestApiException;
+ default void move(String destination) throws RestApiException {
+ MoveInput in = new MoveInput();
+ in.destinationBranch = destination;
+ move(in);
+ }
void move(MoveInput in) throws RestApiException;
@@ -132,7 +144,9 @@
*
* @see Changes#id(int)
*/
- ChangeApi revert() throws RestApiException;
+ default ChangeApi revert() throws RestApiException {
+ return revert(new RevertInput());
+ }
/**
* Create a new change that reverts this change.
@@ -144,10 +158,17 @@
/** Create a merge patch set for the change. */
ChangeInfo createMergePatchSet(MergePatchSetInput in) throws RestApiException;
- List<ChangeInfo> submittedTogether() throws RestApiException;
+ default List<ChangeInfo> submittedTogether() throws RestApiException {
+ SubmittedTogetherInfo info =
+ submittedTogether(
+ EnumSet.noneOf(ListChangesOption.class), EnumSet.noneOf(SubmittedTogetherOption.class));
+ return info.changes;
+ }
- SubmittedTogetherInfo submittedTogether(EnumSet<SubmittedTogetherOption> options)
- throws RestApiException;
+ default SubmittedTogetherInfo submittedTogether(EnumSet<SubmittedTogetherOption> options)
+ throws RestApiException {
+ return submittedTogether(EnumSet.noneOf(ListChangesOption.class), options);
+ }
SubmittedTogetherInfo submittedTogether(
EnumSet<ListChangesOption> listOptions, EnumSet<SubmittedTogetherOption> submitOptions)
@@ -155,10 +176,14 @@
/** Publishes a draft change. */
@Deprecated
- void publish() throws RestApiException;
+ default void publish() throws RestApiException {
+ throw new UnsupportedOperationException("draft workflow is discontinued");
+ }
/** Rebase the current revision of a change using default options. */
- void rebase() throws RestApiException;
+ default void rebase() throws RestApiException {
+ rebase(new RebaseInput());
+ }
/** Rebase the current revision of a change. */
void rebase(RebaseInput in) throws RestApiException;
@@ -172,13 +197,19 @@
IncludedInInfo includedIn() throws RestApiException;
- AddReviewerResult addReviewer(AddReviewerInput in) throws RestApiException;
+ default AddReviewerResult addReviewer(String reviewer) throws RestApiException {
+ AddReviewerInput in = new AddReviewerInput();
+ in.reviewer = reviewer;
+ return addReviewer(in);
+ }
- AddReviewerResult addReviewer(String in) throws RestApiException;
+ AddReviewerResult addReviewer(AddReviewerInput in) throws RestApiException;
SuggestedReviewersRequest suggestReviewers() throws RestApiException;
- SuggestedReviewersRequest suggestReviewers(String query) throws RestApiException;
+ default SuggestedReviewersRequest suggestReviewers(String query) throws RestApiException {
+ return suggestReviewers().withQuery(query);
+ }
ChangeInfo get(EnumSet<ListChangesOption> options) throws RestApiException;
@@ -198,10 +229,16 @@
* <li>{@code SKIP_MERGEABLE} is omitted, so the {@code mergeable} bit <em>is</em> set.
* </ul>
*/
- ChangeInfo get() throws RestApiException;
+ default ChangeInfo get() throws RestApiException {
+ return get(
+ EnumSet.complementOf(
+ EnumSet.of(ListChangesOption.CHECK, ListChangesOption.SKIP_MERGEABLE)));
+ }
/** {@link #get(ListChangesOption...)} with no options included. */
- ChangeInfo info() throws RestApiException;
+ default ChangeInfo info() throws RestApiException {
+ return get(EnumSet.noneOf(ListChangesOption.class));
+ }
/**
* Retrieve change edit when exists.
@@ -210,7 +247,9 @@
* ChangeEditApi#get()}.
*/
@Deprecated
- EditInfo getEdit() throws RestApiException;
+ default EditInfo getEdit() throws RestApiException {
+ return edit().get().orElse(null);
+ }
/**
* Provides access to an API regarding the change edit of this change.
@@ -221,7 +260,11 @@
ChangeEditApi edit() throws RestApiException;
/** Create a new patch set with a new commit message. */
- void setMessage(String message) throws RestApiException;
+ default void setMessage(String message) throws RestApiException {
+ CommitMessageInput in = new CommitMessageInput();
+ in.message = message;
+ setMessage(in);
+ }
/** Create a new patch set with a new commit message. */
void setMessage(CommitMessageInput in) throws RestApiException;
@@ -347,16 +390,6 @@
}
@Override
- public RevisionApi current() throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
- public RevisionApi revision(int id) throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public ReviewerApi reviewer(String id) throws RestApiException {
throw new NotImplementedException();
}
@@ -367,31 +400,16 @@
}
@Override
- public void abandon() throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public void abandon(AbandonInput in) throws RestApiException {
throw new NotImplementedException();
}
@Override
- public void restore() throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public void restore(RestoreInput in) throws RestApiException {
throw new NotImplementedException();
}
@Override
- public void move(String destination) throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public void move(MoveInput in) throws RestApiException {
throw new NotImplementedException();
}
@@ -412,27 +430,11 @@
}
@Override
- public ChangeApi revert() throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public ChangeApi revert(RevertInput in) throws RestApiException {
throw new NotImplementedException();
}
@Override
- public void publish() throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Deprecated
- @Override
- public void rebase() throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public void rebase(RebaseInput in) throws RestApiException {
throw new NotImplementedException();
}
@@ -463,51 +465,21 @@
}
@Override
- public AddReviewerResult addReviewer(String in) throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public SuggestedReviewersRequest suggestReviewers() throws RestApiException {
throw new NotImplementedException();
}
@Override
- public SuggestedReviewersRequest suggestReviewers(String query) throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public ChangeInfo get(EnumSet<ListChangesOption> options) throws RestApiException {
throw new NotImplementedException();
}
@Override
- public ChangeInfo get() throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
- public ChangeInfo info() throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
- public void setMessage(String message) throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public void setMessage(CommitMessageInput in) throws RestApiException {
throw new NotImplementedException();
}
@Override
- public EditInfo getEdit() throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public ChangeEditApi edit() throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/api/changes/RelatedChangeAndCommitInfo.java b/java/com/google/gerrit/extensions/api/changes/RelatedChangeAndCommitInfo.java
new file mode 100644
index 0000000..5bf22aa
--- /dev/null
+++ b/java/com/google/gerrit/extensions/api/changes/RelatedChangeAndCommitInfo.java
@@ -0,0 +1,43 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.api.changes;
+
+import com.google.common.base.MoreObjects;
+import com.google.gerrit.extensions.common.CommitInfo;
+
+public class RelatedChangeAndCommitInfo {
+ public String project;
+ public String changeId;
+ public CommitInfo commit;
+ public Integer _changeNumber;
+ public Integer _revisionNumber;
+ public Integer _currentRevisionNumber;
+ public String status;
+
+ public RelatedChangeAndCommitInfo() {}
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("project", project)
+ .add("changeId", changeId)
+ .add("commit", commit)
+ .add("_changeNumber", _changeNumber)
+ .add("_revisionNumber", _revisionNumber)
+ .add("_currentRevisionNumber", _currentRevisionNumber)
+ .add("status", status)
+ .toString();
+ }
+}
diff --git a/java/com/google/gerrit/extensions/api/changes/RelatedChangesInfo.java b/java/com/google/gerrit/extensions/api/changes/RelatedChangesInfo.java
new file mode 100644
index 0000000..e1e70f3
--- /dev/null
+++ b/java/com/google/gerrit/extensions/api/changes/RelatedChangesInfo.java
@@ -0,0 +1,21 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.api.changes;
+
+import java.util.List;
+
+public class RelatedChangesInfo {
+ public List<RelatedChangeAndCommitInfo> changes;
+}
diff --git a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
index 4658eb3..14dc589 100644
--- a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
@@ -14,6 +14,7 @@
package com.google.gerrit.extensions.api.changes;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ActionInfo;
import com.google.gerrit.extensions.common.CherryPickChangeInfo;
@@ -34,7 +35,9 @@
public interface RevisionApi {
@Deprecated
- void delete() throws RestApiException;
+ default void delete() throws RestApiException {
+ throw new UnsupportedOperationException("draft workflow is discontinued");
+ }
String description() throws RestApiException;
@@ -42,22 +45,32 @@
ReviewResult review(ReviewInput in) throws RestApiException;
- void submit() throws RestApiException;
+ default void submit() throws RestApiException {
+ SubmitInput in = new SubmitInput();
+ submit(in);
+ }
void submit(SubmitInput in) throws RestApiException;
- BinaryResult submitPreview() throws RestApiException;
+ default BinaryResult submitPreview() throws RestApiException {
+ return submitPreview("zip");
+ }
BinaryResult submitPreview(String format) throws RestApiException;
@Deprecated
- void publish() throws RestApiException;
+ default void publish() throws RestApiException {
+ throw new UnsupportedOperationException("draft workflow is discontinued");
+ }
ChangeApi cherryPick(CherryPickInput in) throws RestApiException;
CherryPickChangeInfo cherryPickAsInfo(CherryPickInput in) throws RestApiException;
- ChangeApi rebase() throws RestApiException;
+ default ChangeApi rebase() throws RestApiException {
+ RebaseInput in = new RebaseInput();
+ return rebase(in);
+ }
ChangeApi rebase(RebaseInput in) throws RestApiException;
@@ -69,9 +82,11 @@
Set<String> reviewed() throws RestApiException;
- Map<String, FileInfo> files() throws RestApiException;
+ default Map<String, FileInfo> files() throws RestApiException {
+ return files(null);
+ }
- Map<String, FileInfo> files(String base) throws RestApiException;
+ Map<String, FileInfo> files(@Nullable String base) throws RestApiException;
Map<String, FileInfo> files(int parentNum) throws RestApiException;
@@ -133,6 +148,8 @@
MergeListRequest getMergeList() throws RestApiException;
+ RelatedChangesInfo related() throws RestApiException;
+
abstract class MergeListRequest {
private boolean addLinks;
private int uninterestingParent = 1;
@@ -163,33 +180,16 @@
* interface.
*/
class NotImplemented implements RevisionApi {
- @Deprecated
- @Override
- public void delete() throws RestApiException {
- throw new NotImplementedException();
- }
-
@Override
public ReviewResult review(ReviewInput in) throws RestApiException {
throw new NotImplementedException();
}
@Override
- public void submit() throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public void submit(SubmitInput in) throws RestApiException {
throw new NotImplementedException();
}
- @Deprecated
- @Override
- public void publish() throws RestApiException {
- throw new NotImplementedException();
- }
-
@Override
public ChangeApi cherryPick(CherryPickInput in) throws RestApiException {
throw new NotImplementedException();
@@ -201,11 +201,6 @@
}
@Override
- public ChangeApi rebase() throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public ChangeApi rebase(RebaseInput in) throws RestApiException {
throw new NotImplementedException();
}
@@ -251,11 +246,6 @@
}
@Override
- public Map<String, FileInfo> files() throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public List<String> queryFiles(String query) throws RestApiException {
throw new NotImplementedException();
}
@@ -346,11 +336,6 @@
}
@Override
- public BinaryResult submitPreview() throws RestApiException {
- throw new NotImplementedException();
- }
-
- @Override
public BinaryResult submitPreview(String format) throws RestApiException {
throw new NotImplementedException();
}
@@ -371,6 +356,11 @@
}
@Override
+ public RelatedChangesInfo related() throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public void description(String description) throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/common/ChangeInfo.java b/java/com/google/gerrit/extensions/common/ChangeInfo.java
index 945c239..9a739ef 100644
--- a/java/com/google/gerrit/extensions/common/ChangeInfo.java
+++ b/java/com/google/gerrit/extensions/common/ChangeInfo.java
@@ -46,6 +46,7 @@
public Boolean submittable;
public Integer insertions;
public Integer deletions;
+ public Integer totalCommentCount;
public Integer unresolvedCommentCount;
public Boolean isPrivate;
public Boolean workInProgress;
diff --git a/java/com/google/gerrit/extensions/common/CommitInfo.java b/java/com/google/gerrit/extensions/common/CommitInfo.java
index 213b366..1fd8755 100644
--- a/java/com/google/gerrit/extensions/common/CommitInfo.java
+++ b/java/com/google/gerrit/extensions/common/CommitInfo.java
@@ -16,6 +16,8 @@
import static java.util.stream.Collectors.joining;
+import com.google.common.base.MoreObjects;
+import com.google.common.base.MoreObjects.ToStringHelper;
import java.util.List;
import java.util.Objects;
@@ -50,17 +52,18 @@
@Override
public String toString() {
- // Using something like the raw commit format might be nice, but we can't depend on JGit here.
- StringBuilder sb = new StringBuilder().append(getClass().getSimpleName()).append('{');
- sb.append(commit);
- sb.append(", parents=").append(parents.stream().map(p -> p.commit).collect(joining(", ")));
- sb.append(", author=").append(author);
- sb.append(", committer=").append(committer);
- sb.append(", subject=").append(subject);
- sb.append(", message=").append(message);
- if (webLinks != null) {
- sb.append(", webLinks=").append(webLinks);
+ ToStringHelper helper = MoreObjects.toStringHelper(this).addValue(commit);
+ if (parents != null) {
+ helper.add("parents", parents.stream().map(p -> p.commit).collect(joining(", ")));
}
- return sb.append('}').toString();
+ helper
+ .add("author", author)
+ .add("committer", committer)
+ .add("subject", subject)
+ .add("message", message);
+ if (webLinks != null) {
+ helper.add("webLinks", webLinks);
+ }
+ return helper.toString();
}
}
diff --git a/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java b/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
new file mode 100644
index 0000000..164f957
--- /dev/null
+++ b/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
@@ -0,0 +1,71 @@
+// Copyright (C) 2018 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.httpd;
+
+import com.google.gerrit.common.PageLinks;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.restapi.change.ChangesCollection;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+/** Redirects {@code domain.tld/123} to {@code domain.tld/c/project/+/123}. */
+@Singleton
+public class NumericChangeIdRedirectServlet extends HttpServlet {
+ private static final long serialVersionUID = 1L;
+
+ private final ChangesCollection changesCollection;
+
+ @Inject
+ NumericChangeIdRedirectServlet(ChangesCollection changesCollection) {
+ this.changesCollection = changesCollection;
+ }
+
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse rsp) throws IOException {
+ String idString = req.getPathInfo();
+ if (idString.endsWith("/")) {
+ idString = idString.substring(0, idString.length() - 1);
+ }
+ Change.Id id;
+ try {
+ id = Change.Id.parse(idString);
+ } catch (IllegalArgumentException e) {
+ rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
+ return;
+ }
+
+ ChangeResource changeResource;
+ try {
+ changeResource = changesCollection.parse(id);
+ } catch (ResourceConflictException | ResourceNotFoundException e) {
+ rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
+ return;
+ } catch (OrmException | PermissionBackendException e) {
+ throw new IOException("Unable to lookup change " + id.id, e);
+ }
+ String path =
+ PageLinks.toChange(changeResource.getProject(), changeResource.getChange().getId());
+ UrlModule.toGerrit(path, req, rsp);
+ }
+}
diff --git a/java/com/google/gerrit/httpd/UrlModule.java b/java/com/google/gerrit/httpd/UrlModule.java
index 1dd176a..183b2bd 100644
--- a/java/com/google/gerrit/httpd/UrlModule.java
+++ b/java/com/google/gerrit/httpd/UrlModule.java
@@ -87,7 +87,7 @@
serveRegex("^/settings/?$").with(screen(PageLinks.SETTINGS));
serveRegex("^/register$").with(registerScreen(false));
serveRegex("^/register/(.+)$").with(registerScreen(true));
- serveRegex("^/([1-9][0-9]*)/?$").with(directChangeById());
+ serveRegex("^/([1-9][0-9]*)/?$").with(NumericChangeIdRedirectServlet.class);
serveRegex("^/p/(.*)$").with(queryProjectNew());
serveRegex("^/r/(.+)/?$").with(DirectChangeByCommit.class);
@@ -164,30 +164,6 @@
});
}
- private Key<HttpServlet> directChangeById() {
- return key(
- new HttpServlet() {
- private static final long serialVersionUID = 1L;
-
- @Override
- protected void doGet(HttpServletRequest req, HttpServletResponse rsp) throws IOException {
- try {
- String idString = req.getPathInfo();
- if (idString.endsWith("/")) {
- idString = idString.substring(0, idString.length() - 1);
- }
- Change.Id id = Change.Id.parse(idString);
- // User accessed Gerrit with /1234, so we have no project yet.
- // TODO(hiesel) Replace with a preflight request to obtain project before we deprecate
- // the numeric change id.
- toGerrit(PageLinks.toChange(null, id), req, rsp);
- } catch (IllegalArgumentException err) {
- rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
- }
- }
- });
- }
-
private Key<HttpServlet> queryProjectNew() {
return key(
new HttpServlet() {
diff --git a/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java b/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java
index 24efb86..dd6622a 100644
--- a/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java
+++ b/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java
@@ -62,6 +62,7 @@
ProjectAccessFactory.Factory projectAccessFactory,
ProjectCache projectCache,
GroupBackend groupBackend,
+ ProjectConfig.Factory projectConfigFactory,
MetaDataUpdate.User metaDataUpdateFactory,
AllProjectsName allProjects,
Provider<SetParent> setParent,
@@ -77,6 +78,7 @@
@Nullable @Assisted String message) {
super(
groupBackend,
+ projectConfigFactory,
metaDataUpdateFactory,
allProjects,
setParent,
diff --git a/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java b/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
index 6193e45..4c4959a 100644
--- a/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
+++ b/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
@@ -71,6 +71,7 @@
private final GroupControl.Factory groupControlFactory;
private final MetaDataUpdate.Server metaDataUpdateFactory;
private final AllProjectsName allProjectsName;
+ private final ProjectConfig.Factory projectConfigFactory;
private final Project.NameKey projectName;
private WebLinks webLinks;
@@ -83,6 +84,7 @@
GroupControl.Factory groupControlFactory,
MetaDataUpdate.Server metaDataUpdateFactory,
AllProjectsName allProjectsName,
+ ProjectConfig.Factory projectConfigFactory,
WebLinks webLinks,
@Assisted final Project.NameKey name) {
this.groupBackend = groupBackend;
@@ -91,6 +93,7 @@
this.groupControlFactory = groupControlFactory;
this.metaDataUpdateFactory = metaDataUpdateFactory;
this.allProjectsName = allProjectsName;
+ this.projectConfigFactory = projectConfigFactory;
this.webLinks = webLinks;
this.projectName = name;
@@ -108,7 +111,7 @@
//
ProjectConfig config;
try (MetaDataUpdate md = metaDataUpdateFactory.create(projectName)) {
- config = ProjectConfig.read(md);
+ config = projectConfigFactory.read(md);
if (config.updateGroupNames(groupBackend)) {
md.setMessage("Update group names\n");
config.commit(md);
diff --git a/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java b/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
index 44c8966..074f6a8 100644
--- a/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
+++ b/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
@@ -31,6 +31,7 @@
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.httpd.rpc.Handler;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.Project.NameKey;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupBackends;
@@ -58,6 +59,7 @@
public abstract class ProjectAccessHandler<T> extends Handler<T> {
protected final GroupBackend groupBackend;
+ protected final ProjectConfig.Factory projectConfigFactory;
protected final Project.NameKey projectName;
protected final ObjectId base;
protected final CurrentUser user;
@@ -77,14 +79,15 @@
protected ProjectAccessHandler(
GroupBackend groupBackend,
+ ProjectConfig.Factory projectConfigFactory,
MetaDataUpdate.User metaDataUpdateFactory,
AllProjectsName allProjects,
Provider<SetParent> setParent,
CurrentUser user,
- Project.NameKey projectName,
+ NameKey projectName,
ObjectId base,
List<AccessSection> sectionList,
- Project.NameKey parentProjectName,
+ NameKey parentProjectName,
String message,
ContributorAgreementsChecker contributorAgreements,
PermissionBackend permissionBackend,
@@ -102,6 +105,7 @@
this.message = message;
this.contributorAgreements = contributorAgreements;
this.permissionBackend = permissionBackend;
+ this.projectConfigFactory = projectConfigFactory;
this.checkIfOwner = checkIfOwner;
}
@@ -113,7 +117,7 @@
contributorAgreements.check(projectName, user);
try (MetaDataUpdate md = metaDataUpdateFactory.create(projectName)) {
- ProjectConfig config = ProjectConfig.read(md, base);
+ ProjectConfig config = projectConfigFactory.read(md, base);
Set<String> toDelete = scanSectionNames(config);
PermissionBackend.ForProject forProject = permissionBackend.user(user).project(projectName);
diff --git a/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java b/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
index 6a43a1d..9e8592a 100644
--- a/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
+++ b/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
@@ -88,6 +88,7 @@
ReviewProjectAccess(
PermissionBackend permissionBackend,
GroupBackend groupBackend,
+ ProjectConfig.Factory projectConfigFactory,
MetaDataUpdate.User metaDataUpdateFactory,
ReviewDb db,
Provider<PostReviewers> reviewersProvider,
@@ -108,6 +109,7 @@
@Nullable @Assisted String message) {
super(
groupBackend,
+ projectConfigFactory,
metaDataUpdateFactory,
allProjects,
setParent,
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 66db468..b208a31 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -76,6 +76,7 @@
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
+import java.util.function.Consumer;
import org.apache.lucene.document.Document;
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.index.IndexableField;
@@ -129,6 +130,7 @@
ChangeField.STORED_SUBMIT_RECORD_LENIENT.getName();
private static final String SUBMIT_RECORD_STRICT_FIELD =
ChangeField.STORED_SUBMIT_RECORD_STRICT.getName();
+ private static final String TOTAL_COMMENT_COUNT_FIELD = ChangeField.TOTAL_COMMENT_COUNT.getName();
private static final String UNRESOLVED_COMMENT_COUNT_FIELD =
ChangeField.UNRESOLVED_COMMENT_COUNT.getName();
@@ -504,6 +506,7 @@
}
decodeUnresolvedCommentCount(doc, cd);
+ decodeTotalCommentCount(doc, cd);
return cd;
}
@@ -633,9 +636,18 @@
private void decodeUnresolvedCommentCount(
ListMultimap<String, IndexableField> doc, ChangeData cd) {
- IndexableField f = Iterables.getFirst(doc.get(UNRESOLVED_COMMENT_COUNT_FIELD), null);
+ decodeIntField(doc, UNRESOLVED_COMMENT_COUNT_FIELD, cd::setUnresolvedCommentCount);
+ }
+
+ private void decodeTotalCommentCount(ListMultimap<String, IndexableField> doc, ChangeData cd) {
+ decodeIntField(doc, TOTAL_COMMENT_COUNT_FIELD, cd::setTotalCommentCount);
+ }
+
+ private static void decodeIntField(
+ ListMultimap<String, IndexableField> doc, String fieldName, Consumer<Integer> consumer) {
+ IndexableField f = Iterables.getFirst(doc.get(fieldName), null);
if (f != null && f.numericValue() != null) {
- cd.setUnresolvedCommentCount(f.numericValue().intValue());
+ consumer.accept(f.numericValue().intValue());
}
}
diff --git a/java/com/google/gerrit/pgm/http/jetty/HttpLog.java b/java/com/google/gerrit/pgm/http/jetty/HttpLog.java
index b7ec2be..dab9d7e 100644
--- a/java/com/google/gerrit/pgm/http/jetty/HttpLog.java
+++ b/java/com/google/gerrit/pgm/http/jetty/HttpLog.java
@@ -45,6 +45,7 @@
protected static final String P_PROTOCOL = "Version";
protected static final String P_STATUS = "Status";
protected static final String P_CONTENT_LENGTH = "Content-Length";
+ protected static final String P_LATENCY = "Latency";
protected static final String P_REFERER = "Referer";
protected static final String P_USER_AGENT = "User-Agent";
@@ -94,6 +95,7 @@
set(event, P_PROTOCOL, req.getProtocol());
set(event, P_STATUS, rsp.getStatus());
set(event, P_CONTENT_LENGTH, rsp.getContentCount());
+ set(event, P_LATENCY, System.currentTimeMillis() - req.getTimeStamp());
set(event, P_REFERER, req.getHeader("Referer"));
set(event, P_USER_AGENT, req.getHeader("User-Agent"));
diff --git a/java/com/google/gerrit/pgm/http/jetty/HttpLogLayout.java b/java/com/google/gerrit/pgm/http/jetty/HttpLogLayout.java
index 2eea88d..bd7d998 100644
--- a/java/com/google/gerrit/pgm/http/jetty/HttpLogLayout.java
+++ b/java/com/google/gerrit/pgm/http/jetty/HttpLogLayout.java
@@ -67,6 +67,9 @@
opt(buf, event, HttpLog.P_CONTENT_LENGTH);
buf.append(' ');
+ opt(buf, event, HttpLog.P_LATENCY);
+
+ buf.append(' ');
dq_opt(buf, event, HttpLog.P_REFERER);
buf.append(' ');
diff --git a/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java b/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java
index ee0138c..94ce924 100644
--- a/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java
+++ b/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java
@@ -57,17 +57,20 @@
private final AllUsersName allUsers;
private final ProjectCache projectCache;
private final Provider<MetaDataUpdate.Server> metaDataUpdateFactory;
+ private final ProjectConfig.Factory projectConfigFactory;
@Inject
CreateGroupPermissionSyncer(
AllProjectsName allProjects,
AllUsersName allUsers,
ProjectCache projectCache,
- Provider<MetaDataUpdate.Server> metaDataUpdateFactory) {
+ Provider<MetaDataUpdate.Server> metaDataUpdateFactory,
+ ProjectConfig.Factory projectConfigFactory) {
this.allProjects = allProjects;
this.allUsers = allUsers;
this.projectCache = projectCache;
this.metaDataUpdateFactory = metaDataUpdateFactory;
+ this.projectConfigFactory = projectConfigFactory;
}
/**
@@ -102,7 +105,7 @@
}
try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsers)) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
AccessSection createGroupAccessSection =
config.getAccessSection(RefNames.REFS_GROUPS + "*", true);
if (createGroupsGlobal.isEmpty()) {
diff --git a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index 358a3a8..0f731ed 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -43,7 +43,6 @@
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.CommitMessageInput;
-import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.common.MergePatchSetInput;
import com.google.gerrit.extensions.common.PureRevertInfo;
@@ -256,16 +255,6 @@
}
@Override
- public RevisionApi current() throws RestApiException {
- return revision("current");
- }
-
- @Override
- public RevisionApi revision(int id) throws RestApiException {
- return revision(String.valueOf(id));
- }
-
- @Override
public RevisionApi revision(String id) throws RestApiException {
try {
return revisionApi.create(revisions.parse(change, IdString.fromDecoded(id)));
@@ -284,11 +273,6 @@
}
@Override
- public void abandon() throws RestApiException {
- abandon(new AbandonInput());
- }
-
- @Override
public void abandon(AbandonInput in) throws RestApiException {
try {
abandon.apply(change, in);
@@ -298,11 +282,6 @@
}
@Override
- public void restore() throws RestApiException {
- restore(new RestoreInput());
- }
-
- @Override
public void restore(RestoreInput in) throws RestApiException {
try {
restore.apply(change, in);
@@ -312,13 +291,6 @@
}
@Override
- public void move(String destination) throws RestApiException {
- MoveInput in = new MoveInput();
- in.destinationBranch = destination;
- move(in);
- }
-
- @Override
public void move(MoveInput in) throws RestApiException {
try {
move.apply(change, in);
@@ -360,11 +332,6 @@
}
@Override
- public ChangeApi revert() throws RestApiException {
- return revert(new RevertInput());
- }
-
- @Override
public ChangeApi revert(RevertInput in) throws RestApiException {
try {
return changeApi.id(revert.apply(change, in)._number);
@@ -383,20 +350,6 @@
}
@Override
- public List<ChangeInfo> submittedTogether() throws RestApiException {
- SubmittedTogetherInfo info =
- submittedTogether(
- EnumSet.noneOf(ListChangesOption.class), EnumSet.noneOf(SubmittedTogetherOption.class));
- return info.changes;
- }
-
- @Override
- public SubmittedTogetherInfo submittedTogether(EnumSet<SubmittedTogetherOption> options)
- throws RestApiException {
- return submittedTogether(EnumSet.noneOf(ListChangesOption.class), options);
- }
-
- @Override
public SubmittedTogetherInfo submittedTogether(
EnumSet<ListChangesOption> listOptions, EnumSet<SubmittedTogetherOption> submitOptions)
throws RestApiException {
@@ -411,17 +364,6 @@
}
}
- @Deprecated
- @Override
- public void publish() throws RestApiException {
- throw new UnsupportedOperationException("draft workflow is discontinued");
- }
-
- @Override
- public void rebase() throws RestApiException {
- rebase(new RebaseInput());
- }
-
@Override
public void rebase(RebaseInput in) throws RestApiException {
try {
@@ -466,13 +408,6 @@
}
@Override
- public AddReviewerResult addReviewer(String reviewer) throws RestApiException {
- AddReviewerInput in = new AddReviewerInput();
- in.reviewer = reviewer;
- return addReviewer(in);
- }
-
- @Override
public AddReviewerResult addReviewer(AddReviewerInput in) throws RestApiException {
try {
return postReviewers.apply(change, in);
@@ -491,11 +426,6 @@
};
}
- @Override
- public SuggestedReviewersRequest suggestReviewers(String query) throws RestApiException {
- return suggestReviewers().withQuery(query);
- }
-
private List<SuggestedReviewerInfo> suggestReviewers(SuggestedReviewersRequest r)
throws RestApiException {
try {
@@ -517,30 +447,11 @@
}
@Override
- public ChangeInfo get() throws RestApiException {
- return get(
- EnumSet.complementOf(
- EnumSet.of(ListChangesOption.CHECK, ListChangesOption.SKIP_MERGEABLE)));
- }
-
- @Override
- public EditInfo getEdit() throws RestApiException {
- return edit().get().orElse(null);
- }
-
- @Override
public ChangeEditApi edit() throws RestApiException {
return changeEditApi.create(change);
}
@Override
- public void setMessage(String msg) throws RestApiException {
- CommitMessageInput in = new CommitMessageInput();
- in.message = msg;
- setMessage(in);
- }
-
- @Override
public void setMessage(CommitMessageInput in) throws RestApiException {
try {
putMessage.apply(change, in);
@@ -550,11 +461,6 @@
}
@Override
- public ChangeInfo info() throws RestApiException {
- return get(EnumSet.noneOf(ListChangesOption.class));
- }
-
- @Override
public void setHashtags(HashtagsInput input) throws RestApiException {
try {
postHashtags.apply(change, input);
diff --git a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
index 33f211d..f8a2ecb 100644
--- a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.Changes;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
@@ -26,6 +27,7 @@
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.api.changes.FileApi;
import com.google.gerrit.extensions.api.changes.RebaseInput;
+import com.google.gerrit.extensions.api.changes.RelatedChangesInfo;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewResult;
import com.google.gerrit.extensions.api.changes.RevisionApi;
@@ -64,6 +66,7 @@
import com.google.gerrit.server.restapi.change.GetDescription;
import com.google.gerrit.server.restapi.change.GetMergeList;
import com.google.gerrit.server.restapi.change.GetPatch;
+import com.google.gerrit.server.restapi.change.GetRelated;
import com.google.gerrit.server.restapi.change.GetRevisionActions;
import com.google.gerrit.server.restapi.change.ListRevisionComments;
import com.google.gerrit.server.restapi.change.ListRevisionDrafts;
@@ -129,6 +132,7 @@
private final TestSubmitType.Get getSubmitType;
private final Provider<TestSubmitRule> testSubmitRule;
private final Provider<GetMergeList> getMergeList;
+ private final GetRelated getRelated;
private final PutDescription putDescription;
private final GetDescription getDescription;
@@ -169,6 +173,7 @@
TestSubmitType.Get getSubmitType,
Provider<TestSubmitRule> testSubmitRule,
Provider<GetMergeList> getMergeList,
+ GetRelated getRelated,
PutDescription putDescription,
GetDescription getDescription,
@Assisted RevisionResource r) {
@@ -207,6 +212,7 @@
this.getSubmitType = getSubmitType;
this.testSubmitRule = testSubmitRule;
this.getMergeList = getMergeList;
+ this.getRelated = getRelated;
this.putDescription = putDescription;
this.getDescription = getDescription;
this.revision = r;
@@ -222,12 +228,6 @@
}
@Override
- public void submit() throws RestApiException {
- SubmitInput in = new SubmitInput();
- submit(in);
- }
-
- @Override
public void submit(SubmitInput in) throws RestApiException {
try {
submit.apply(revision, in);
@@ -237,11 +237,6 @@
}
@Override
- public BinaryResult submitPreview() throws RestApiException {
- return submitPreview("zip");
- }
-
- @Override
public BinaryResult submitPreview(String format) throws RestApiException {
try {
submitPreview.setFormat(format);
@@ -252,22 +247,6 @@
}
@Override
- public void publish() throws RestApiException {
- throw new UnsupportedOperationException("draft workflow is discontinued");
- }
-
- @Override
- public void delete() throws RestApiException {
- throw new UnsupportedOperationException("draft workflow is discontinued");
- }
-
- @Override
- public ChangeApi rebase() throws RestApiException {
- RebaseInput in = new RebaseInput();
- return rebase(in);
- }
-
- @Override
public ChangeApi rebase(RebaseInput in) throws RestApiException {
try {
return changes.id(rebase.apply(revision, in)._number);
@@ -361,17 +340,7 @@
@SuppressWarnings("unchecked")
@Override
- public Map<String, FileInfo> files() throws RestApiException {
- try {
- return (Map<String, FileInfo>) listFiles.apply(revision).value();
- } catch (Exception e) {
- throw asRestApiException("Cannot retrieve files", e);
- }
- }
-
- @SuppressWarnings("unchecked")
- @Override
- public Map<String, FileInfo> files(String base) throws RestApiException {
+ public Map<String, FileInfo> files(@Nullable String base) throws RestApiException {
try {
return (Map<String, FileInfo>) listFiles.setBase(base).apply(revision).value();
} catch (Exception e) {
@@ -590,6 +559,15 @@
}
@Override
+ public RelatedChangesInfo related() throws RestApiException {
+ try {
+ return getRelated.apply(revision);
+ } catch (Exception e) {
+ throw asRestApiException("Cannot get related changes", e);
+ }
+ }
+
+ @Override
public void description(String description) throws RestApiException {
DescriptionInput in = new DescriptionInput();
in.description = description;
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index c64cd12..b7049a7 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -535,6 +535,7 @@
out.created = in.getCreatedOn();
out.updated = in.getLastUpdatedOn();
out._number = in.getId().get();
+ out.totalCommentCount = cd.totalCommentCount();
out.unresolvedCommentCount = cd.unresolvedCommentCount();
if (user.isIdentifiedUser()) {
diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java
index 1f216f0..dac8fec 100644
--- a/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -20,12 +20,14 @@
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.RebaseUtil.Base;
+import com.google.gerrit.server.git.GroupCollector;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -199,8 +201,14 @@
+ " was rebased");
}
- if (base != null) {
- patchSetInserter.setGroups(base.patchSet().getGroups());
+ if (base != null && base.notes().getChange().getStatus() != Change.Status.MERGED) {
+ if (base.notes().getChange().getStatus() != Change.Status.MERGED) {
+ // Add to end of relation chain for open base change.
+ patchSetInserter.setGroups(base.patchSet().getGroups());
+ } else {
+ // If the base is merged, start a new relation chain.
+ patchSetInserter.setGroups(GroupCollector.getDefaultGroups(rebasedCommit));
+ }
}
patchSetInserter.updateRepo(ctx);
}
diff --git a/java/com/google/gerrit/server/config/ProjectConfigEntry.java b/java/com/google/gerrit/server/config/ProjectConfigEntry.java
index 5515f0e..92ae10a 100644
--- a/java/com/google/gerrit/server/config/ProjectConfigEntry.java
+++ b/java/com/google/gerrit/server/config/ProjectConfigEntry.java
@@ -302,12 +302,16 @@
private final GitRepositoryManager repoManager;
private final DynamicMap<ProjectConfigEntry> pluginConfigEntries;
+ private final ProjectConfig.Factory projectConfigFactory;
@Inject
UpdateChecker(
- GitRepositoryManager repoManager, DynamicMap<ProjectConfigEntry> pluginConfigEntries) {
+ GitRepositoryManager repoManager,
+ DynamicMap<ProjectConfigEntry> pluginConfigEntries,
+ ProjectConfig.Factory projectConfigFactory) {
this.repoManager = repoManager;
this.pluginConfigEntries = pluginConfigEntries;
+ this.projectConfigFactory = projectConfigFactory;
}
@Override
@@ -361,7 +365,7 @@
return null;
}
try (Repository repo = repoManager.openRepository(p)) {
- ProjectConfig pc = new ProjectConfig(p);
+ ProjectConfig pc = projectConfigFactory.create(p);
pc.load(repo, id);
return pc;
}
diff --git a/java/com/google/gerrit/server/git/DefaultAdvertiseRefsHook.java b/java/com/google/gerrit/server/git/DefaultAdvertiseRefsHook.java
index ef5e65b..be8fcdb 100644
--- a/java/com/google/gerrit/server/git/DefaultAdvertiseRefsHook.java
+++ b/java/com/google/gerrit/server/git/DefaultAdvertiseRefsHook.java
@@ -14,8 +14,11 @@
package com.google.gerrit.server.git;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import java.io.IOException;
+import java.util.List;
import java.util.Map;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@@ -28,7 +31,6 @@
* implements {@link org.eclipse.jgit.transport.AdvertiseRefsHook}.
*/
public class DefaultAdvertiseRefsHook extends AbstractAdvertiseRefsHook {
-
private final PermissionBackend.ForProject perm;
private final PermissionBackend.RefFilterOptions opts;
@@ -42,9 +44,24 @@
protected Map<String, Ref> getAdvertisedRefs(Repository repo, RevWalk revWalk)
throws ServiceMayNotContinueException {
try {
- return perm.filter(repo.getAllRefs(), repo, opts);
- } catch (PermissionBackendException e) {
- throw new ServiceMayNotContinueException(e);
+ Map<String, Ref> refs;
+ List<String> prefixes = opts.prefixes();
+ if (prefixes.isEmpty() || prefixes.get(0).isEmpty()) {
+ refs = repo.getAllRefs();
+ } else {
+ ImmutableMap.Builder<String, Ref> b = new ImmutableMap.Builder<>();
+ for (String prefix : prefixes) {
+ for (Ref ref : repo.getRefDatabase().getRefsByPrefix(prefix)) {
+ b.put(ref.getName(), ref);
+ }
+ }
+ refs = b.build();
+ }
+ return perm.filter(refs, repo, opts);
+ } catch (IOException | PermissionBackendException e) {
+ ServiceMayNotContinueException ex = new ServiceMayNotContinueException();
+ ex.initCause(e);
+ throw ex;
}
}
}
diff --git a/java/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManager.java b/java/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManager.java
index 6fafe4e..01e85cf 100644
--- a/java/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManager.java
+++ b/java/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManager.java
@@ -17,7 +17,7 @@
import static com.google.common.base.Preconditions.checkState;
import com.google.gerrit.lifecycle.LifecycleModule;
-import com.google.gerrit.reviewdb.client.Project.NameKey;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.RepositoryConfig;
import com.google.gerrit.server.config.SitePaths;
@@ -54,7 +54,7 @@
}
@Override
- public Path getBasePath(NameKey name) {
+ public Path getBasePath(Project.NameKey name) {
Path alternateBasePath = config.getBasePath(name);
return alternateBasePath != null ? alternateBasePath : super.getBasePath(name);
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 6267590..c747533 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -321,6 +321,7 @@
private final SetHashtagsOp.Factory hashtagsFactory;
private final SubmoduleOp.Factory subOpFactory;
private final TagCache tagCache;
+ private final ProjectConfig.Factory projectConfigFactory;
// Assisted injected fields.
private final AllRefsWatcher allRefsWatcher;
@@ -365,6 +366,7 @@
AccountResolver accountResolver,
AllProjectsName allProjectsName,
BatchUpdate.Factory batchUpdateFactory,
+ ProjectConfig.Factory projectConfigFactory,
@GerritServerConfig Config cfg,
ChangeEditUtil editUtil,
ChangeIndexer indexer,
@@ -437,6 +439,7 @@
this.seq = seq;
this.subOpFactory = subOpFactory;
this.tagCache = tagCache;
+ this.projectConfigFactory = projectConfigFactory;
// Assisted injected fields.
this.allRefsWatcher = allRefsWatcher;
@@ -1055,7 +1058,7 @@
case UPDATE:
case UPDATE_NONFASTFORWARD:
try {
- ProjectConfig cfg = new ProjectConfig(project.getNameKey());
+ ProjectConfig cfg = projectConfigFactory.create(project.getNameKey());
cfg.load(project.getNameKey(), receivePack.getRevWalk(), cmd.getNewId());
if (!cfg.getValidationErrors().isEmpty()) {
addError("Invalid project configuration:");
diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 9f75c07..e3dfa75 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -98,6 +98,7 @@
private final AccountValidator accountValidator;
private final String installCommitMsgHookCommand;
private final ProjectCache projectCache;
+ private final ProjectConfig.Factory projectConfigFactory;
@Inject
Factory(
@@ -110,7 +111,8 @@
AllProjectsName allProjects,
ExternalIdsConsistencyChecker externalIdsConsistencyChecker,
AccountValidator accountValidator,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ ProjectConfig.Factory projectConfigFactory) {
this.gerritIdent = gerritIdent;
this.urlFormatter = urlFormatter;
this.pluginValidators = pluginValidators;
@@ -122,6 +124,7 @@
this.installCommitMsgHookCommand =
cfg != null ? cfg.getString("gerrit", null, "installCommitMsgHookCommand") : null;
this.projectCache = projectCache;
+ this.projectConfigFactory = projectConfigFactory;
}
public CommitValidators forReceiveCommits(
@@ -145,7 +148,7 @@
new SignedOffByValidator(user, perm, projectState),
new ChangeIdValidator(
projectState, user, urlFormatter, installCommitMsgHookCommand, sshInfo, change),
- new ConfigValidator(branch, user, rw, allUsers, allProjects),
+ new ConfigValidator(projectConfigFactory, branch, user, rw, allUsers, allProjects),
new BannedCommitsValidator(rejectCommits),
new PluginCommitValidationListener(pluginValidators),
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
@@ -172,7 +175,7 @@
new SignedOffByValidator(user, perm, projectCache.checkedGet(branch.getParentKey())),
new ChangeIdValidator(
projectState, user, urlFormatter, installCommitMsgHookCommand, sshInfo, change),
- new ConfigValidator(branch, user, rw, allUsers, allProjects),
+ new ConfigValidator(projectConfigFactory, branch, user, rw, allUsers, allProjects),
new PluginCommitValidationListener(pluginValidators),
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
new AccountCommitValidator(repoManager, allUsers, accountValidator),
@@ -331,7 +334,8 @@
.append(getCommitMessageHookInstallationHint())
.append("\n")
.append("and then amend the commit:\n")
- .append(" git commit --amend\n");
+ .append(" git commit --amend\n")
+ .append("Finally, push your changes again\n");
}
return new CommitValidationMessage(sb.toString(), Type.ERROR);
}
@@ -378,6 +382,7 @@
/** If this is the special project configuration branch, validate the config. */
public static class ConfigValidator implements CommitValidationListener {
+ private final ProjectConfig.Factory projectConfigFactory;
private final Branch.NameKey branch;
private final IdentifiedUser user;
private final RevWalk rw;
@@ -385,11 +390,13 @@
private final AllProjectsName allProjects;
public ConfigValidator(
+ ProjectConfig.Factory projectConfigFactory,
Branch.NameKey branch,
IdentifiedUser user,
RevWalk rw,
AllUsersName allUsers,
AllProjectsName allProjects) {
+ this.projectConfigFactory = projectConfigFactory;
this.branch = branch;
this.user = user;
this.rw = rw;
@@ -404,7 +411,7 @@
List<CommitValidationMessage> messages = new ArrayList<>();
try {
- ProjectConfig cfg = new ProjectConfig(receiveEvent.project.getNameKey());
+ ProjectConfig cfg = projectConfigFactory.create(receiveEvent.project.getNameKey());
cfg.load(rw, receiveEvent.command.getNewId());
if (!cfg.getValidationErrors().isEmpty()) {
addError("Invalid project configuration:", messages);
diff --git a/java/com/google/gerrit/server/git/validators/MergeValidators.java b/java/com/google/gerrit/server/git/validators/MergeValidators.java
index 0422c51..56a641f 100644
--- a/java/com/google/gerrit/server/git/validators/MergeValidators.java
+++ b/java/com/google/gerrit/server/git/validators/MergeValidators.java
@@ -127,6 +127,7 @@
private final ProjectCache projectCache;
private final PermissionBackend permissionBackend;
private final DynamicMap<ProjectConfigEntry> pluginConfigEntries;
+ private final ProjectConfig.Factory projectConfigFactory;
private final boolean allowProjectOwnersToChangeParent;
public interface Factory {
@@ -140,12 +141,14 @@
ProjectCache projectCache,
PermissionBackend permissionBackend,
DynamicMap<ProjectConfigEntry> pluginConfigEntries,
+ ProjectConfig.Factory projectConfigFactory,
@GerritServerConfig Config config) {
this.allProjectsName = allProjectsName;
this.allUsersName = allUsersName;
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
this.pluginConfigEntries = pluginConfigEntries;
+ this.projectConfigFactory = projectConfigFactory;
this.allowProjectOwnersToChangeParent =
config.getBoolean("receive", "allowProjectOwnersToChangeParent", false);
}
@@ -162,7 +165,7 @@
if (RefNames.REFS_CONFIG.equals(destBranch.get())) {
final Project.NameKey newParent;
try {
- ProjectConfig cfg = new ProjectConfig(destProject.getNameKey());
+ ProjectConfig cfg = projectConfigFactory.create(destProject.getNameKey());
cfg.load(destProject.getNameKey(), repo, commit);
newParent = cfg.getProject().getParent(allProjectsName);
final Project.NameKey oldParent = destProject.getProject().getParent(allProjectsName);
diff --git a/java/com/google/gerrit/server/group/db/RenameGroupOp.java b/java/com/google/gerrit/server/group/db/RenameGroupOp.java
index eada57d..e002192 100644
--- a/java/com/google/gerrit/server/group/db/RenameGroupOp.java
+++ b/java/com/google/gerrit/server/group/db/RenameGroupOp.java
@@ -49,6 +49,7 @@
private final ProjectCache projectCache;
private final MetaDataUpdate.Server metaDataUpdateFactory;
+ private final ProjectConfig.Factory projectConfigFactory;
private final PersonIdent author;
private final AccountGroup.UUID uuid;
@@ -63,6 +64,7 @@
WorkQueue workQueue,
ProjectCache projectCache,
MetaDataUpdate.Server metaDataUpdateFactory,
+ ProjectConfig.Factory projectConfigFactory,
@Assisted("author") PersonIdent author,
@Assisted AccountGroup.UUID uuid,
@Assisted("oldName") String oldName,
@@ -70,6 +72,7 @@
super(workQueue);
this.projectCache = projectCache;
this.metaDataUpdateFactory = metaDataUpdateFactory;
+ this.projectConfigFactory = projectConfigFactory;
this.author = author;
this.uuid = uuid;
@@ -109,7 +112,7 @@
private void rename(MetaDataUpdate md) throws IOException, ConfigInvalidException {
boolean success = false;
for (int attempts = 0; !success && attempts < MAX_TRIES; attempts++) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
// The group isn't referenced, or its name has been fixed already.
//
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 5d12e79..b79a1c2 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -508,11 +508,15 @@
cd.messages().stream().map(ChangeMessage::getMessage))
.collect(toSet()));
- /** Number of unresolved comments of the change. */
+ /** Number of unresolved comment threads of the change, including robot comments. */
public static final FieldDef<ChangeData, Integer> UNRESOLVED_COMMENT_COUNT =
intRange(ChangeQueryBuilder.FIELD_UNRESOLVED_COMMENT_COUNT)
.build(ChangeData::unresolvedCommentCount);
+ /** Total number of published inline comments of the change, including robot comments. */
+ public static final FieldDef<ChangeData, Integer> TOTAL_COMMENT_COUNT =
+ intRange("total_comments").build(ChangeData::totalCommentCount);
+
/** Whether the change is mergeable. */
public static final FieldDef<ChangeData, String> MERGEABLE =
exact(ChangeQueryBuilder.FIELD_MERGEABLE)
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 2000cd1..9016fd1 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -99,7 +99,9 @@
@Deprecated static final Schema<ChangeData> V49 = schema(V48);
// Bump Lucene version requires reindexing
- static final Schema<ChangeData> V50 = schema(V49);
+ @Deprecated static final Schema<ChangeData> V50 = schema(V49);
+
+ static final Schema<ChangeData> V51 = schema(V50, ChangeField.TOTAL_COMMENT_COUNT);
public static final String NAME = "changes";
public static final ChangeSchemaDefinitions INSTANCE = new ChangeSchemaDefinitions();
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java
index db3c961..bea760c 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -18,6 +18,7 @@
import static java.util.stream.Collectors.toSet;
import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.LabelType;
@@ -40,6 +41,7 @@
import java.util.Collections;
import java.util.EnumSet;
import java.util.Iterator;
+import java.util.List;
import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
@@ -367,12 +369,19 @@
/** Separately add reachable tags. */
public abstract boolean filterTagsSeparately();
+ /**
+ * Select only refs with names matching prefixes per {@link
+ * org.eclipse.jgit.lib.RefDatabase#getRefsByPrefix}.
+ */
+ public abstract ImmutableList<String> prefixes();
+
public abstract Builder toBuilder();
public static Builder builder() {
return new AutoValue_PermissionBackend_RefFilterOptions.Builder()
.setFilterMeta(false)
- .setFilterTagsSeparately(false);
+ .setFilterTagsSeparately(false)
+ .setPrefixes(Collections.singletonList(""));
}
@AutoValue.Builder
@@ -381,6 +390,8 @@
public abstract Builder setFilterTagsSeparately(boolean val);
+ public abstract Builder setPrefixes(List<String> prefixes);
+
public abstract RefFilterOptions build();
}
diff --git a/java/com/google/gerrit/server/plugins/JarScanner.java b/java/com/google/gerrit/server/plugins/JarScanner.java
index 1a9b859..486e264 100644
--- a/java/com/google/gerrit/server/plugins/JarScanner.java
+++ b/java/com/google/gerrit/server/plugins/JarScanner.java
@@ -195,7 +195,7 @@
Collection<String> exports;
private ClassData(Collection<String> exports) {
- super(Opcodes.ASM6);
+ super(Opcodes.ASM7);
this.exports = exports;
}
@@ -263,7 +263,7 @@
private abstract static class AbstractAnnotationVisitor extends AnnotationVisitor {
AbstractAnnotationVisitor() {
- super(Opcodes.ASM6);
+ super(Opcodes.ASM7);
}
@Override
diff --git a/java/com/google/gerrit/server/project/ContributorAgreementsChecker.java b/java/com/google/gerrit/server/project/ContributorAgreementsChecker.java
index fc342db..4ed1c0c 100644
--- a/java/com/google/gerrit/server/project/ContributorAgreementsChecker.java
+++ b/java/com/google/gerrit/server/project/ContributorAgreementsChecker.java
@@ -14,6 +14,9 @@
package com.google.gerrit.server.project;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
import com.google.gerrit.common.data.ContributorAgreement;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.data.PermissionRule.Action;
@@ -34,6 +37,8 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
@Singleton
public class ContributorAgreementsChecker {
@@ -93,6 +98,20 @@
List<AccountGroup.UUID> groupIds;
groupIds = okGroupIds;
+ // matchProjects defaults to match all projects when missing.
+ List<String> matchProjectsRegexes = ca.getMatchProjectsRegexes();
+ if (!matchProjectsRegexes.isEmpty()
+ && !projectMatchesAnyPattern(project.get(), matchProjectsRegexes)) {
+ // Doesn't match, isn't checked.
+ continue;
+ }
+ // excludeProjects defaults to exclude no projects when missing.
+ List<String> excludeProjectsRegexes = ca.getExcludeProjectsRegexes();
+ if (!excludeProjectsRegexes.isEmpty()
+ && projectMatchesAnyPattern(project.get(), excludeProjectsRegexes)) {
+ // Matches, isn't checked.
+ continue;
+ }
for (PermissionRule rule : ca.getAccepted()) {
if ((rule.getAction() == Action.ALLOW)
&& (rule.getGroup() != null)
@@ -102,7 +121,7 @@
}
}
- if (!iUser.getEffectiveGroups().containsAnyOf(okGroupIds)) {
+ if (!okGroupIds.isEmpty() && !iUser.getEffectiveGroups().containsAnyOf(okGroupIds)) {
final StringBuilder msg = new StringBuilder();
msg.append("No Contributor Agreement on file for user ")
.append(iUser.getNameEmail())
@@ -114,4 +133,23 @@
throw new AuthException(msg.toString());
}
}
+
+ private boolean projectMatchesAnyPattern(String projectName, List<String> regexes) {
+ checkNotNull(regexes);
+ checkArgument(!regexes.isEmpty());
+ for (String patternString : regexes) {
+ Pattern pattern;
+ try {
+ pattern = Pattern.compile(patternString);
+ } catch (PatternSyntaxException e) {
+ // Should never happen: Regular expressions validated when reading project.config.
+ throw new IllegalStateException(
+ "Invalid matchProjects or excludeProjects clause in project.config", e);
+ }
+ if (pattern.matcher(projectName).find()) {
+ return true;
+ }
+ }
+ return false;
+ }
}
diff --git a/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
index aa455e6..ddeeb8a 100644
--- a/java/com/google/gerrit/server/project/ProjectCacheImpl.java
+++ b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
@@ -25,6 +25,10 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.index.project.ProjectIndexer;
import com.google.gerrit.lifecycle.LifecycleModule;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Description.Units;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.cache.CacheModule;
@@ -89,6 +93,7 @@
private final Lock listLock;
private final ProjectCacheClock clock;
private final Provider<ProjectIndexer> indexer;
+ private final Timer0 guessRelevantGroupsLatency;
@Inject
ProjectCacheImpl(
@@ -97,7 +102,8 @@
@Named(CACHE_NAME) LoadingCache<String, ProjectState> byName,
@Named(CACHE_LIST) LoadingCache<ListKey, ImmutableSortedSet<Project.NameKey>> list,
ProjectCacheClock clock,
- Provider<ProjectIndexer> indexer) {
+ Provider<ProjectIndexer> indexer,
+ MetricMaker metricMaker) {
this.allProjectsName = allProjectsName;
this.allUsersName = allUsersName;
this.byName = byName;
@@ -105,6 +111,13 @@
this.listLock = new ReentrantLock(true /* fair */);
this.clock = clock;
this.indexer = indexer;
+
+ this.guessRelevantGroupsLatency =
+ metricMaker.newTimer(
+ "group/guess_relevant_groups_latency",
+ new Description("Latency for guessing relevant groups")
+ .setCumulative()
+ .setUnit(Units.NANOSECONDS));
}
@Override
@@ -234,15 +247,17 @@
@Override
public Set<AccountGroup.UUID> guessRelevantGroupUUIDs() {
- return all()
- .stream()
- .map(n -> byName.getIfPresent(n.get()))
- .filter(Objects::nonNull)
- .flatMap(p -> p.getConfig().getAllGroupUUIDs().stream())
- // getAllGroupUUIDs shouldn't really return null UUIDs, but harden
- // against them just in case there is a bug or corner case.
- .filter(id -> id != null && id.get() != null)
- .collect(toSet());
+ try (Timer0.Context ignored = guessRelevantGroupsLatency.start()) {
+ return all()
+ .stream()
+ .map(n -> byName.getIfPresent(n.get()))
+ .filter(Objects::nonNull)
+ .flatMap(p -> p.getConfig().getAllGroupUUIDs().stream())
+ // getAllGroupUUIDs shouldn't really return null UUIDs, but harden
+ // against them just in case there is a bug or corner case.
+ .filter(id -> id != null && id.get() != null)
+ .collect(toSet());
+ }
}
@Override
@@ -262,12 +277,18 @@
private final ProjectState.Factory projectStateFactory;
private final GitRepositoryManager mgr;
private final ProjectCacheClock clock;
+ private final ProjectConfig.Factory projectConfigFactory;
@Inject
- Loader(ProjectState.Factory psf, GitRepositoryManager g, ProjectCacheClock clock) {
+ Loader(
+ ProjectState.Factory psf,
+ GitRepositoryManager g,
+ ProjectCacheClock clock,
+ ProjectConfig.Factory projectConfigFactory) {
projectStateFactory = psf;
mgr = g;
this.clock = clock;
+ this.projectConfigFactory = projectConfigFactory;
}
@Override
@@ -276,7 +297,7 @@
long now = clock.read();
Project.NameKey key = new Project.NameKey(projectName);
try (Repository git = mgr.openRepository(key)) {
- ProjectConfig cfg = new ProjectConfig(key);
+ ProjectConfig cfg = projectConfigFactory.create(key);
cfg.load(key, git);
ProjectState state = projectStateFactory.create(cfg);
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index bccc415..74c0f3b 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -59,6 +59,7 @@
import com.google.gerrit.server.git.ValidationError;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.git.meta.VersionedMetaData;
+import com.google.inject.Singleton;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
@@ -127,6 +128,8 @@
private static final String KEY_ACCEPTED = "accepted";
private static final String KEY_AUTO_VERIFY = "autoVerify";
private static final String KEY_AGREEMENT_URL = "agreementUrl";
+ private static final String KEY_MATCH_PROJECTS = "matchProjects";
+ private static final String KEY_EXCLUDE_PROJECTS = "excludeProjects";
private static final String NOTIFY = "notify";
private static final String KEY_EMAIL = "email";
@@ -165,6 +168,29 @@
private static final Pattern EXCLUSIVE_PERMISSIONS_SPLIT_PATTERN = Pattern.compile("[, \t]{1,}");
+ // Don't use an assisted factory, since instances created by an assisted factory retain references
+ // to their enclosing injector. Instances of ProjectConfig are cached for a long time in the
+ // ProjectCache, so this would retain lots more memory.
+ @Singleton
+ public static class Factory {
+ public ProjectConfig create(Project.NameKey projectName) {
+ return new ProjectConfig(projectName);
+ }
+
+ public ProjectConfig read(MetaDataUpdate update) throws IOException, ConfigInvalidException {
+ ProjectConfig r = create(update.getProjectName());
+ r.load(update);
+ return r;
+ }
+
+ public ProjectConfig read(MetaDataUpdate update, ObjectId id)
+ throws IOException, ConfigInvalidException {
+ ProjectConfig r = create(update.getProjectName());
+ r.load(update, id);
+ return r;
+ }
+ }
+
private Project project;
private AccountsSection accountsSection;
private GroupList groupList;
@@ -186,20 +212,6 @@
private Map<String, List<String>> extensionPanelSections;
private Map<String, GroupReference> groupsByName;
- public static ProjectConfig read(MetaDataUpdate update)
- throws IOException, ConfigInvalidException {
- ProjectConfig r = new ProjectConfig(update.getProjectName());
- r.load(update);
- return r;
- }
-
- public static ProjectConfig read(MetaDataUpdate update, ObjectId id)
- throws IOException, ConfigInvalidException {
- ProjectConfig r = new ProjectConfig(update.getProjectName());
- r.load(update, id);
- return r;
- }
-
public static CommentLinkInfoImpl buildCommentLink(Config cfg, String name, boolean allowRaw)
throws IllegalArgumentException {
String match = cfg.getString(COMMENTLINK, name, KEY_MATCH);
@@ -241,7 +253,7 @@
commentLinkSections.add(commentLink);
}
- public ProjectConfig(Project.NameKey projectName) {
+ private ProjectConfig(Project.NameKey projectName) {
this.projectName = projectName;
}
@@ -593,6 +605,9 @@
ca.setAgreementUrl(rc.getString(CONTRIBUTOR_AGREEMENT, name, KEY_AGREEMENT_URL));
ca.setAccepted(
loadPermissionRules(rc, CONTRIBUTOR_AGREEMENT, name, KEY_ACCEPTED, groupsByName, false));
+ ca.setExcludeProjectsRegexes(
+ loadPatterns(rc, CONTRIBUTOR_AGREEMENT, name, KEY_EXCLUDE_PROJECTS));
+ ca.setMatchProjectsRegexes(loadPatterns(rc, CONTRIBUTOR_AGREEMENT, name, KEY_MATCH_PROJECTS));
List<PermissionRule> rules =
loadPermissionRules(
@@ -753,6 +768,22 @@
}
}
+ private ImmutableList<String> loadPatterns(
+ Config rc, String section, String subsection, String varName) {
+ ImmutableList.Builder<String> patterns = ImmutableList.builder();
+ for (String patternString : rc.getStringList(section, subsection, varName)) {
+ try {
+ // While one could just use getStringList directly, compiling first will cause the server
+ // to fail fast if any of the patterns are invalid.
+ patterns.add(Pattern.compile(patternString).pattern());
+ } catch (PatternSyntaxException e) {
+ error(new ValidationError(PROJECT_CONFIG, "Invalid regular expression: " + e.getMessage()));
+ continue;
+ }
+ }
+ return patterns.build();
+ }
+
private ImmutableList<PermissionRule> loadPermissionRules(
Config rc,
String section,
@@ -1163,6 +1194,16 @@
ca.getName(),
KEY_ACCEPTED,
ruleToStringList(ca.getAccepted(), keepGroups));
+ rc.setStringList(
+ CONTRIBUTOR_AGREEMENT,
+ ca.getName(),
+ KEY_EXCLUDE_PROJECTS,
+ patternToStringList(ca.getExcludeProjectsRegexes()));
+ rc.setStringList(
+ CONTRIBUTOR_AGREEMENT,
+ ca.getName(),
+ KEY_MATCH_PROJECTS,
+ patternToStringList(ca.getMatchProjectsRegexes()));
}
}
@@ -1206,6 +1247,10 @@
}
}
+ private List<String> patternToStringList(List<String> list) {
+ return list;
+ }
+
private List<String> ruleToStringList(
List<PermissionRule> list, Set<AccountGroup.UUID> keepGroups) {
List<String> rules = new ArrayList<>();
diff --git a/java/com/google/gerrit/server/project/SectionMatcher.java b/java/com/google/gerrit/server/project/SectionMatcher.java
index 11b1f37..3095f2f 100644
--- a/java/com/google/gerrit/server/project/SectionMatcher.java
+++ b/java/com/google/gerrit/server/project/SectionMatcher.java
@@ -16,7 +16,6 @@
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.Project.NameKey;
import com.google.gerrit.server.CurrentUser;
/**
@@ -57,7 +56,7 @@
return matcher;
}
- public NameKey getProject() {
+ public Project.NameKey getProject() {
return project;
}
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 83d68db..0f5d938 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -30,6 +30,7 @@
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
+import com.google.common.primitives.Ints;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.SubmitRecord;
@@ -391,6 +392,7 @@
private PersonIdent committer;
private int parentCount;
private Integer unresolvedCommentCount;
+ private Integer totalCommentCount;
private LabelTypes labelTypes;
private ImmutableList<byte[]> refStates;
@@ -925,6 +927,23 @@
this.unresolvedCommentCount = count;
}
+ public Integer totalCommentCount() throws OrmException {
+ if (totalCommentCount == null) {
+ if (!lazyLoad) {
+ return null;
+ }
+
+ // Fail on overflow.
+ totalCommentCount =
+ Ints.checkedCast((long) publishedComments().size() + robotComments().size());
+ }
+ return totalCommentCount;
+ }
+
+ public void setTotalCommentCount(Integer count) {
+ this.totalCommentCount = count;
+ }
+
public List<ChangeMessage> messages() throws OrmException {
if (messages == null) {
if (!lazyLoad) {
diff --git a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
index f81ea15..d682b93 100644
--- a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
@@ -29,6 +29,7 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
@@ -43,6 +44,7 @@
protected final ProjectCache projectCache;
private final Provider<AnonymousUser> anonymousUserProvider;
+ @Inject
public ChangeIsVisibleToPredicate(
Provider<ReviewDb> db,
ChangeNotes.Factory notesFactory,
diff --git a/java/com/google/gerrit/server/restapi/change/ChangesCollection.java b/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
index 561c27c..30d93be 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
@@ -17,6 +17,7 @@
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestCollection;
@@ -101,7 +102,8 @@
}
public ChangeResource parse(Change.Id id)
- throws RestApiException, OrmException, PermissionBackendException, IOException {
+ throws ResourceConflictException, ResourceNotFoundException, OrmException,
+ PermissionBackendException, IOException {
List<ChangeNotes> notes = changeFinder.find(id);
if (notes.isEmpty()) {
throw new ResourceNotFoundException(toIdString(id));
@@ -139,7 +141,7 @@
}
private void checkProjectStatePermitsRead(Project.NameKey project)
- throws IOException, RestApiException {
+ throws IOException, ResourceNotFoundException, ResourceConflictException {
ProjectState projectState = projectCache.checkedGet(project);
if (projectState == null) {
throw new ResourceNotFoundException("project not found: " + project.get());
diff --git a/java/com/google/gerrit/server/restapi/change/Files.java b/java/com/google/gerrit/server/restapi/change/Files.java
index 1bb6bf2..b374fdc 100644
--- a/java/com/google/gerrit/server/restapi/change/Files.java
+++ b/java/com/google/gerrit/server/restapi/change/Files.java
@@ -18,6 +18,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.common.hash.Hasher;
import com.google.common.hash.Hashing;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.common.FileInfo;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -332,7 +333,7 @@
return this;
}
- public ListFiles setBase(String base) {
+ public ListFiles setBase(@Nullable String base) {
this.base = base;
return this;
}
diff --git a/java/com/google/gerrit/server/restapi/change/GetRelated.java b/java/com/google/gerrit/server/restapi/change/GetRelated.java
index 3313136..30fbf39 100644
--- a/java/com/google/gerrit/server/restapi/change/GetRelated.java
+++ b/java/com/google/gerrit/server/restapi/change/GetRelated.java
@@ -17,9 +17,10 @@
import static java.util.stream.Collectors.toSet;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.MoreObjects;
import com.google.common.collect.Lists;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.api.changes.RelatedChangeAndCommitInfo;
+import com.google.gerrit.extensions.api.changes.RelatedChangesInfo;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.index.IndexConfig;
@@ -70,15 +71,15 @@
}
@Override
- public RelatedInfo apply(RevisionResource rsrc)
+ public RelatedChangesInfo apply(RevisionResource rsrc)
throws RepositoryNotFoundException, IOException, OrmException, NoSuchProjectException,
PermissionBackendException {
- RelatedInfo relatedInfo = new RelatedInfo();
- relatedInfo.changes = getRelated(rsrc);
- return relatedInfo;
+ RelatedChangesInfo relatedChangesInfo = new RelatedChangesInfo();
+ relatedChangesInfo.changes = getRelated(rsrc);
+ return relatedChangesInfo;
}
- private List<ChangeAndCommit> getRelated(RevisionResource rsrc)
+ private List<RelatedChangeAndCommitInfo> getRelated(RevisionResource rsrc)
throws OrmException, IOException, PermissionBackendException {
Set<String> groups = getAllGroups(rsrc.getNotes(), db.get(), psUtil);
if (groups.isEmpty()) {
@@ -94,7 +95,7 @@
if (cds.size() == 1 && cds.get(0).getId().equals(rsrc.getChange().getId())) {
return Collections.emptyList();
}
- List<ChangeAndCommit> result = new ArrayList<>(cds.size());
+ List<RelatedChangeAndCommitInfo> result = new ArrayList<>(cds.size());
boolean isEdit = rsrc.getEdit().isPresent();
PatchSet basePs = isEdit ? rsrc.getEdit().get().getBasePatchSet() : rsrc.getPatchSet();
@@ -111,11 +112,11 @@
} else {
commit = d.commit();
}
- result.add(new ChangeAndCommit(rsrc.getProject(), d.data().change(), ps, commit));
+ result.add(newChangeAndCommit(rsrc.getProject(), d.data().change(), ps, commit));
}
if (result.size() == 1) {
- ChangeAndCommit r = result.get(0);
+ RelatedChangeAndCommitInfo r = result.get(0);
if (r.commit != null && r.commit.commit.equals(rsrc.getPatchSet().getRevision().get())) {
return Collections.emptyList();
}
@@ -143,69 +144,30 @@
}
}
- public static class RelatedInfo {
- public List<ChangeAndCommit> changes;
- }
+ static RelatedChangeAndCommitInfo newChangeAndCommit(
+ Project.NameKey project, @Nullable Change change, @Nullable PatchSet ps, RevCommit c) {
+ RelatedChangeAndCommitInfo info = new RelatedChangeAndCommitInfo();
+ info.project = project.get();
- public static class ChangeAndCommit {
- public String project;
- public String changeId;
- public CommitInfo commit;
- public Integer _changeNumber;
- public Integer _revisionNumber;
- public Integer _currentRevisionNumber;
- public String status;
-
- public ChangeAndCommit() {}
-
- ChangeAndCommit(
- Project.NameKey project, @Nullable Change change, @Nullable PatchSet ps, RevCommit c) {
- this.project = project.get();
-
- if (change != null) {
- changeId = change.getKey().get();
- _changeNumber = change.getChangeId();
- _revisionNumber = ps != null ? ps.getPatchSetId() : null;
- PatchSet.Id curr = change.currentPatchSetId();
- _currentRevisionNumber = curr != null ? curr.get() : null;
- status = change.getStatus().asChangeStatus().toString();
- }
-
- commit = new CommitInfo();
- commit.commit = c.name();
- commit.parents = Lists.newArrayListWithCapacity(c.getParentCount());
- for (int i = 0; i < c.getParentCount(); i++) {
- CommitInfo p = new CommitInfo();
- p.commit = c.getParent(i).name();
- commit.parents.add(p);
- }
- commit.author = CommonConverters.toGitPerson(c.getAuthorIdent());
- commit.subject = c.getShortMessage();
+ if (change != null) {
+ info.changeId = change.getKey().get();
+ info._changeNumber = change.getChangeId();
+ info._revisionNumber = ps != null ? ps.getPatchSetId() : null;
+ PatchSet.Id curr = change.currentPatchSetId();
+ info._currentRevisionNumber = curr != null ? curr.get() : null;
+ info.status = change.getStatus().asChangeStatus().toString();
}
- @Override
- public String toString() {
- return MoreObjects.toStringHelper(this)
- .add("project", project)
- .add("changeId", changeId)
- .add("commit", toString(commit))
- .add("_changeNumber", _changeNumber)
- .add("_revisionNumber", _revisionNumber)
- .add("_currentRevisionNumber", _currentRevisionNumber)
- .add("status", status)
- .toString();
+ info.commit = new CommitInfo();
+ info.commit.commit = c.name();
+ info.commit.parents = Lists.newArrayListWithCapacity(c.getParentCount());
+ for (int i = 0; i < c.getParentCount(); i++) {
+ CommitInfo p = new CommitInfo();
+ p.commit = c.getParent(i).name();
+ info.commit.parents.add(p);
}
-
- private static String toString(CommitInfo commit) {
- return MoreObjects.toStringHelper(commit)
- .add("commit", commit.commit)
- .add("parent", commit.parents)
- .add("author", commit.author)
- .add("committer", commit.committer)
- .add("subject", commit.subject)
- .add("message", commit.message)
- .add("webLinks", commit.webLinks)
- .toString();
- }
+ info.commit.author = CommonConverters.toGitPerson(c.getAuthorIdent());
+ info.commit.subject = c.getShortMessage();
+ return info;
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/Move.java b/java/com/google/gerrit/server/restapi/change/Move.java
index 013d3e9..df1d9b9 100644
--- a/java/com/google/gerrit/server/restapi/change/Move.java
+++ b/java/com/google/gerrit/server/restapi/change/Move.java
@@ -27,6 +27,7 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.webui.UiAction;
@@ -47,6 +48,7 @@
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -68,6 +70,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -87,6 +90,7 @@
private final PatchSetUtil psUtil;
private final ApprovalsUtil approvalsUtil;
private final ProjectCache projectCache;
+ private final boolean moveEnabled;
@Inject
Move(
@@ -99,7 +103,8 @@
RetryHelper retryHelper,
PatchSetUtil psUtil,
ApprovalsUtil approvalsUtil,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ @GerritServerConfig Config gerritConfig) {
super(retryHelper);
this.permissionBackend = permissionBackend;
this.dbProvider = dbProvider;
@@ -110,6 +115,7 @@
this.psUtil = psUtil;
this.approvalsUtil = approvalsUtil;
this.projectCache = projectCache;
+ this.moveEnabled = gerritConfig.getBoolean("change", null, "move", true);
}
@Override
@@ -117,6 +123,12 @@
BatchUpdate.Factory updateFactory, ChangeResource rsrc, MoveInput input)
throws RestApiException, OrmException, UpdateException, PermissionBackendException,
IOException {
+ if (!moveEnabled) {
+ // This will be removed with the above config once we reach consensus for the move change
+ // behavior. See: https://bugs.chromium.org/p/gerrit/issues/detail?id=9877
+ throw new MethodNotAllowedException("move changes endpoint is disabled");
+ }
+
Change change = rsrc.getChange();
Project.NameKey project = rsrc.getProject();
IdentifiedUser caller = rsrc.getUser().asIdentifiedUser();
diff --git a/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java b/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
index feb37c0..f679610 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
@@ -68,6 +68,7 @@
private final SetAccessUtil setAccess;
private final ChangeJson.Factory jsonFactory;
private final ProjectCache projectCache;
+ private final ProjectConfig.Factory projectConfigFactory;
@Inject
CreateAccessChange(
@@ -79,7 +80,8 @@
Provider<ReviewDb> db,
SetAccessUtil accessUtil,
ChangeJson.Factory jsonFactory,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ ProjectConfig.Factory projectConfigFactory) {
this.permissionBackend = permissionBackend;
this.seq = seq;
this.changeInserterFactory = changeInserterFactory;
@@ -89,6 +91,7 @@
this.setAccess = accessUtil;
this.jsonFactory = jsonFactory;
this.projectCache = projectCache;
+ this.projectConfigFactory = projectConfigFactory;
}
@Override
@@ -117,7 +120,7 @@
input.parent == null ? null : new Project.NameKey(input.parent);
try (MetaDataUpdate md = metaDataUpdateUser.create(rsrc.getNameKey())) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
ObjectId oldCommit = config.getRevision();
String oldCommitSha1 = oldCommit == null ? null : oldCommit.getName();
diff --git a/java/com/google/gerrit/server/restapi/project/CreateProject.java b/java/com/google/gerrit/server/restapi/project/CreateProject.java
index 5620370..7773914 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateProject.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateProject.java
@@ -115,6 +115,7 @@
private final AllProjectsName allProjects;
private final AllUsersName allUsers;
private final PluginItemContext<ProjectNameLockManager> lockManager;
+ private final ProjectConfig.Factory projectConfigFactory;
@Inject
CreateProject(
@@ -135,7 +136,8 @@
Provider<PutConfig> putConfig,
AllProjectsName allProjects,
AllUsersName allUsers,
- PluginItemContext<ProjectNameLockManager> lockManager) {
+ PluginItemContext<ProjectNameLockManager> lockManager,
+ ProjectConfig.Factory projectConfigFactory) {
this.projectsCollection = projectsCollection;
this.groupResolver = groupResolver;
this.projectCreationValidationListeners = projectCreationValidationListeners;
@@ -154,6 +156,7 @@
this.allProjects = allProjects;
this.allUsers = allUsers;
this.lockManager = lockManager;
+ this.projectConfigFactory = projectConfigFactory;
}
@Override
@@ -286,7 +289,7 @@
private void createProjectConfig(CreateProjectArgs args)
throws IOException, ConfigInvalidException {
try (MetaDataUpdate md = metaDataUpdateFactory.create(args.getProject())) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
Project newProject = config.getProject();
newProject.setDescription(args.projectDescription);
diff --git a/java/com/google/gerrit/server/restapi/project/GetAccess.java b/java/com/google/gerrit/server/restapi/project/GetAccess.java
index a6b9404..8875d40 100644
--- a/java/com/google/gerrit/server/restapi/project/GetAccess.java
+++ b/java/com/google/gerrit/server/restapi/project/GetAccess.java
@@ -95,6 +95,7 @@
private final MetaDataUpdate.Server metaDataUpdateFactory;
private final GroupBackend groupBackend;
private final WebLinks webLinks;
+ private final ProjectConfig.Factory projectConfigFactory;
@Inject
public GetAccess(
@@ -105,7 +106,8 @@
MetaDataUpdate.Server metaDataUpdateFactory,
ProjectJson projectJson,
GroupBackend groupBackend,
- WebLinks webLinks) {
+ WebLinks webLinks,
+ ProjectConfig.Factory projectConfigFactory) {
this.user = self;
this.permissionBackend = permissionBackend;
this.allProjectsName = allProjectsName;
@@ -114,6 +116,7 @@
this.metaDataUpdateFactory = metaDataUpdateFactory;
this.groupBackend = groupBackend;
this.webLinks = webLinks;
+ this.projectConfigFactory = projectConfigFactory;
}
public ProjectAccessInfo apply(Project.NameKey nameKey)
@@ -141,7 +144,7 @@
ProjectConfig config;
try (MetaDataUpdate md = metaDataUpdateFactory.create(projectName)) {
- config = ProjectConfig.read(md);
+ config = projectConfigFactory.read(md);
info.configWebLinks = new ArrayList<>();
// config may have a null revision if the repo doesn't have its own refs/meta/config.
diff --git a/java/com/google/gerrit/server/restapi/project/Module.java b/java/com/google/gerrit/server/restapi/project/Module.java
index 8c8ab49..de5661d 100644
--- a/java/com/google/gerrit/server/restapi/project/Module.java
+++ b/java/com/google/gerrit/server/restapi/project/Module.java
@@ -55,7 +55,6 @@
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, "check.access").to(CheckAccessReadView.class);
post(PROJECT_KIND, "check").to(Check.class);
diff --git a/java/com/google/gerrit/server/restapi/project/PutConfig.java b/java/com/google/gerrit/server/restapi/project/PutConfig.java
index 76ea0c9..921a591 100644
--- a/java/com/google/gerrit/server/restapi/project/PutConfig.java
+++ b/java/com/google/gerrit/server/restapi/project/PutConfig.java
@@ -47,6 +47,7 @@
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.project.ProjectState.Factory;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -77,20 +78,22 @@
private final DynamicMap<RestView<ProjectResource>> views;
private final Provider<CurrentUser> user;
private final PermissionBackend permissionBackend;
+ private final ProjectConfig.Factory projectConfigFactory;
@Inject
PutConfig(
@EnableSignedPush boolean serverEnableSignedPush,
Provider<MetaDataUpdate.User> metaDataUpdateFactory,
ProjectCache projectCache,
- ProjectState.Factory projectStateFactory,
+ Factory projectStateFactory,
DynamicMap<ProjectConfigEntry> pluginConfigEntries,
PluginConfigFactory cfgFactory,
AllProjectsName allProjects,
UiActions uiActions,
DynamicMap<RestView<ProjectResource>> views,
Provider<CurrentUser> user,
- PermissionBackend permissionBackend) {
+ PermissionBackend permissionBackend,
+ ProjectConfig.Factory projectConfigFactory) {
this.serverEnableSignedPush = serverEnableSignedPush;
this.metaDataUpdateFactory = metaDataUpdateFactory;
this.projectCache = projectCache;
@@ -102,6 +105,7 @@
this.views = views;
this.user = user;
this.permissionBackend = permissionBackend;
+ this.projectConfigFactory = projectConfigFactory;
}
@Override
@@ -122,7 +126,7 @@
}
try (MetaDataUpdate md = metaDataUpdateFactory.get().create(projectName)) {
- ProjectConfig projectConfig = ProjectConfig.read(md);
+ ProjectConfig projectConfig = projectConfigFactory.read(md);
Project p = projectConfig.getProject();
p.setDescription(Strings.emptyToNull(input.description));
@@ -164,7 +168,7 @@
throw new ResourceConflictException("Cannot update " + projectName);
}
- ProjectState state = projectStateFactory.create(ProjectConfig.read(md));
+ ProjectState state = projectStateFactory.create(projectConfigFactory.read(md));
return new ConfigInfoImpl(
serverEnableSignedPush,
state,
diff --git a/java/com/google/gerrit/server/restapi/project/PutDescription.java b/java/com/google/gerrit/server/restapi/project/PutDescription.java
index 1c74021..081669a 100644
--- a/java/com/google/gerrit/server/restapi/project/PutDescription.java
+++ b/java/com/google/gerrit/server/restapi/project/PutDescription.java
@@ -42,15 +42,18 @@
private final ProjectCache cache;
private final MetaDataUpdate.Server updateFactory;
private final PermissionBackend permissionBackend;
+ private final ProjectConfig.Factory projectConfigFactory;
@Inject
PutDescription(
ProjectCache cache,
MetaDataUpdate.Server updateFactory,
- PermissionBackend permissionBackend) {
+ PermissionBackend permissionBackend,
+ ProjectConfig.Factory projectConfigFactory) {
this.cache = cache;
this.updateFactory = updateFactory;
this.permissionBackend = permissionBackend;
+ this.projectConfigFactory = projectConfigFactory;
}
@Override
@@ -68,7 +71,7 @@
.check(ProjectPermission.WRITE_CONFIG);
try (MetaDataUpdate md = updateFactory.create(resource.getNameKey())) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
Project project = config.getProject();
project.setDescription(Strings.emptyToNull(input.description));
diff --git a/java/com/google/gerrit/server/restapi/project/SetAccess.java b/java/com/google/gerrit/server/restapi/project/SetAccess.java
index c9d69a5..19e89a9 100644
--- a/java/com/google/gerrit/server/restapi/project/SetAccess.java
+++ b/java/com/google/gerrit/server/restapi/project/SetAccess.java
@@ -56,6 +56,7 @@
private final Provider<IdentifiedUser> identifiedUser;
private final SetAccessUtil accessUtil;
private final CreateGroupPermissionSyncer createGroupPermissionSyncer;
+ private final ProjectConfig.Factory projectConfigFactory;
@Inject
private SetAccess(
@@ -66,7 +67,8 @@
GetAccess getAccess,
Provider<IdentifiedUser> identifiedUser,
SetAccessUtil accessUtil,
- CreateGroupPermissionSyncer createGroupPermissionSyncer) {
+ CreateGroupPermissionSyncer createGroupPermissionSyncer,
+ ProjectConfig.Factory projectConfigFactory) {
this.groupBackend = groupBackend;
this.permissionBackend = permissionBackend;
this.metaDataUpdateFactory = metaDataUpdateFactory;
@@ -75,6 +77,7 @@
this.identifiedUser = identifiedUser;
this.accessUtil = accessUtil;
this.createGroupPermissionSyncer = createGroupPermissionSyncer;
+ this.projectConfigFactory = projectConfigFactory;
}
@Override
@@ -89,7 +92,7 @@
List<AccessSection> removals = accessUtil.getAccessSections(input.remove);
List<AccessSection> additions = accessUtil.getAccessSections(input.add);
try (MetaDataUpdate md = metaDataUpdateUser.create(rsrc.getNameKey())) {
- config = ProjectConfig.read(md);
+ config = projectConfigFactory.read(md);
// Check that the user has the right permissions.
boolean checkedAdmin = false;
diff --git a/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java b/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java
index ef292e5..0f346df 100644
--- a/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java
+++ b/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java
@@ -47,6 +47,7 @@
private final DashboardsCollection dashboards;
private final Provider<GetDashboard> get;
private final PermissionBackend permissionBackend;
+ private final ProjectConfig.Factory projectConfigFactory;
@Option(name = "--inherited", usage = "set dashboard inherited by children")
boolean inherited;
@@ -57,12 +58,14 @@
MetaDataUpdate.Server updateFactory,
DashboardsCollection dashboards,
Provider<GetDashboard> get,
- PermissionBackend permissionBackend) {
+ PermissionBackend permissionBackend,
+ ProjectConfig.Factory projectConfigFactory) {
this.cache = cache;
this.updateFactory = updateFactory;
this.dashboards = dashboards;
this.get = get;
this.permissionBackend = permissionBackend;
+ this.projectConfigFactory = projectConfigFactory;
}
@Override
@@ -93,7 +96,7 @@
}
try (MetaDataUpdate md = updateFactory.create(rsrc.getProjectState().getNameKey())) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
Project project = config.getProject();
if (inherited) {
project.setDefaultDashboard(input.id);
diff --git a/java/com/google/gerrit/server/restapi/project/SetParent.java b/java/com/google/gerrit/server/restapi/project/SetParent.java
index ca7e7aa..15cb7f8 100644
--- a/java/com/google/gerrit/server/restapi/project/SetParent.java
+++ b/java/com/google/gerrit/server/restapi/project/SetParent.java
@@ -60,6 +60,7 @@
private final MetaDataUpdate.Server updateFactory;
private final AllProjectsName allProjects;
private final AllUsersName allUsers;
+ private final ProjectConfig.Factory projectConfigFactory;
private volatile boolean allowProjectOwnersToChangeParent;
@Inject
@@ -69,12 +70,14 @@
MetaDataUpdate.Server updateFactory,
AllProjectsName allProjects,
AllUsersName allUsers,
+ ProjectConfig.Factory projectConfigFactory,
@GerritServerConfig Config config) {
this.cache = cache;
this.permissionBackend = permissionBackend;
this.updateFactory = updateFactory;
this.allProjects = allProjects;
this.allUsers = allUsers;
+ this.projectConfigFactory = projectConfigFactory;
this.allowProjectOwnersToChangeParent =
config.getBoolean("receive", "allowProjectOwnersToChangeParent", false);
}
@@ -96,7 +99,7 @@
MoreObjects.firstNonNull(Strings.emptyToNull(input.parent), allProjects.get());
validateParentUpdate(rsrc.getProjectState().getNameKey(), user, parentName, checkIfAdmin);
try (MetaDataUpdate md = updateFactory.create(rsrc.getNameKey())) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
Project project = config.getProject();
project.setParentName(parentName);
diff --git a/java/com/google/gerrit/server/rules/StoredValues.java b/java/com/google/gerrit/server/rules/StoredValues.java
index 8b9cfe3..a712635 100644
--- a/java/com/google/gerrit/server/rules/StoredValues.java
+++ b/java/com/google/gerrit/server/rules/StoredValues.java
@@ -110,6 +110,18 @@
}
};
+ // Accessing GitRepositoryManager could be slow.
+ // It should be minimized or cached to reduce pause time
+ // when evaluating Prolog submit rules.
+ public static final StoredValue<GitRepositoryManager> REPO_MANAGER =
+ new StoredValue<GitRepositoryManager>() {
+ @Override
+ public GitRepositoryManager createValue(Prolog engine) {
+ PrologEnvironment env = (PrologEnvironment) engine.control;
+ return env.getArgs().getGitRepositoryManager();
+ }
+ };
+
public static final StoredValue<Repository> REPOSITORY =
new StoredValue<Repository>() {
@Override
diff --git a/java/com/google/gerrit/server/schema/AllProjectsCreator.java b/java/com/google/gerrit/server/schema/AllProjectsCreator.java
index 348f88c..85965ef 100644
--- a/java/com/google/gerrit/server/schema/AllProjectsCreator.java
+++ b/java/com/google/gerrit/server/schema/AllProjectsCreator.java
@@ -73,6 +73,7 @@
private final AllProjectsName allProjectsName;
private final PersonIdent serverUser;
private final NotesMigration notesMigration;
+ private final ProjectConfig.Factory projectConfigFactory;
private final GroupReference anonymous;
private final GroupReference registered;
private final GroupReference owners;
@@ -90,11 +91,13 @@
AllProjectsName allProjectsName,
@GerritPersonIdent PersonIdent serverUser,
NotesMigration notesMigration,
- SystemGroupBackend systemGroupBackend) {
+ SystemGroupBackend systemGroupBackend,
+ ProjectConfig.Factory projectConfigFactory) {
this.repositoryManager = repositoryManager;
this.allProjectsName = allProjectsName;
this.serverUser = serverUser;
this.notesMigration = notesMigration;
+ this.projectConfigFactory = projectConfigFactory;
this.anonymous = systemGroupBackend.getGroup(ANONYMOUS_USERS);
this.registered = systemGroupBackend.getGroup(REGISTERED_USERS);
@@ -170,7 +173,7 @@
Strings.emptyToNull(message),
"Initialized Gerrit Code Review " + Version.getVersion()));
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
Project p = config.getProject();
p.setDescription("Access inherited by all other projects.");
p.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, InheritableBoolean.TRUE);
diff --git a/java/com/google/gerrit/server/schema/AllUsersCreator.java b/java/com/google/gerrit/server/schema/AllUsersCreator.java
index 3779d0d..3b1124d 100644
--- a/java/com/google/gerrit/server/schema/AllUsersCreator.java
+++ b/java/com/google/gerrit/server/schema/AllUsersCreator.java
@@ -35,6 +35,7 @@
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gerrit.server.project.ProjectConfig.Factory;
import com.google.gerrit.server.project.RefPattern;
import com.google.inject.Inject;
import java.io.IOException;
@@ -50,6 +51,7 @@
private final GitRepositoryManager mgr;
private final AllUsersName allUsersName;
private final PersonIdent serverUser;
+ private final ProjectConfig.Factory projectConfigFactory;
private final GroupReference registered;
@Nullable private GroupReference admin;
@@ -60,11 +62,13 @@
GitRepositoryManager mgr,
AllUsersName allUsersName,
SystemGroupBackend systemGroupBackend,
- @GerritPersonIdent PersonIdent serverUser) {
+ @GerritPersonIdent PersonIdent serverUser,
+ Factory projectConfigFactory) {
this.mgr = mgr;
this.allUsersName = allUsersName;
this.serverUser = serverUser;
this.registered = systemGroupBackend.getGroup(REGISTERED_USERS);
+ this.projectConfigFactory = projectConfigFactory;
this.codeReviewLabel = getDefaultCodeReviewLabel();
}
@@ -107,7 +111,7 @@
md.getCommitBuilder().setCommitter(serverUser);
md.setMessage("Initialized Gerrit Code Review " + Version.getVersion());
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
Project project = config.getProject();
project.setDescription("Individual user settings and preferences.");
diff --git a/java/com/google/gerrit/server/schema/Schema_108.java b/java/com/google/gerrit/server/schema/Schema_108.java
index 4e62460..b37fab3 100644
--- a/java/com/google/gerrit/server/schema/Schema_108.java
+++ b/java/com/google/gerrit/server/schema/Schema_108.java
@@ -24,7 +24,6 @@
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.Project.NameKey;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -145,8 +144,8 @@
private SetMultimap<Project.NameKey, Change.Id> getOpenChangesByProject(ReviewDb db, UpdateUI ui)
throws OrmException {
- SortedSet<NameKey> projects = repoManager.list();
- SortedSet<NameKey> nonExistentProjects = Sets.newTreeSet();
+ SortedSet<Project.NameKey> projects = repoManager.list();
+ SortedSet<Project.NameKey> nonExistentProjects = Sets.newTreeSet();
SetMultimap<Project.NameKey, Change.Id> openByProject =
MultimapBuilder.hashKeys().hashSetValues().build();
for (Change c : db.changes().all()) {
@@ -155,7 +154,7 @@
continue;
}
- NameKey projectKey = c.getProject();
+ Project.NameKey projectKey = c.getProject();
if (!projects.contains(projectKey)) {
nonExistentProjects.add(projectKey);
} else {
diff --git a/java/com/google/gerrit/server/schema/Schema_120.java b/java/com/google/gerrit/server/schema/Schema_120.java
index f2f3b99..15e34ab 100644
--- a/java/com/google/gerrit/server/schema/Schema_120.java
+++ b/java/com/google/gerrit/server/schema/Schema_120.java
@@ -42,15 +42,18 @@
public class Schema_120 extends SchemaVersion {
private final GitRepositoryManager mgr;
+ private final ProjectConfig.Factory projectConfigFactory;
private final PersonIdent serverUser;
@Inject
Schema_120(
Provider<Schema_119> prior,
GitRepositoryManager mgr,
+ ProjectConfig.Factory projectConfigFactory,
@GerritPersonIdent PersonIdent serverUser) {
super(prior);
this.mgr = mgr;
+ this.projectConfigFactory = projectConfigFactory;
this.serverUser = serverUser;
}
@@ -64,7 +67,7 @@
md.getCommitBuilder().setAuthor(serverUser);
md.getCommitBuilder().setCommitter(serverUser);
md.setMessage("Added superproject subscription during upgrade");
- ProjectConfig pc = ProjectConfig.read(md);
+ ProjectConfig pc = projectConfigFactory.read(md);
SubscribeSection s = null;
for (SubscribeSection s1 : pc.getSubscribeSections(subbranch)) {
diff --git a/java/com/google/gerrit/server/schema/Schema_125.java b/java/com/google/gerrit/server/schema/Schema_125.java
index 7aab7c7..474d7ac 100644
--- a/java/com/google/gerrit/server/schema/Schema_125.java
+++ b/java/com/google/gerrit/server/schema/Schema_125.java
@@ -57,6 +57,7 @@
private final AllUsersName allUsersName;
private final AllProjectsName allProjectsName;
private final SystemGroupBackend systemGroupBackend;
+ private final ProjectConfig.Factory projectConfigFactory;
private final PersonIdent serverUser;
@Inject
@@ -66,12 +67,14 @@
AllUsersName allUsersName,
AllProjectsName allProjectsName,
SystemGroupBackend systemGroupBackend,
+ ProjectConfig.Factory projectConfigFactory,
@GerritPersonIdent PersonIdent serverUser) {
super(prior);
this.repoManager = repoManager;
this.allUsersName = allUsersName;
this.allProjectsName = allProjectsName;
this.systemGroupBackend = systemGroupBackend;
+ this.projectConfigFactory = projectConfigFactory;
this.serverUser = serverUser;
}
@@ -79,7 +82,7 @@
protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException {
try (Repository git = repoManager.openRepository(allUsersName);
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, git)) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
config
.getAccessSection(RefNames.REFS_USERS + "*", true)
@@ -114,7 +117,7 @@
while (parent != null) {
try (Repository git = repoManager.openRepository(parent);
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, parent, git)) {
- ProjectConfig parentConfig = ProjectConfig.read(md);
+ ProjectConfig parentConfig = projectConfigFactory.read(md);
for (LabelType lt : parentConfig.getLabelSections().values()) {
if (!labelTypes.containsKey(lt.getName())) {
labelTypes.put(lt.getName(), lt);
diff --git a/java/com/google/gerrit/server/schema/Schema_126.java b/java/com/google/gerrit/server/schema/Schema_126.java
index 5dbda72..23de169 100644
--- a/java/com/google/gerrit/server/schema/Schema_126.java
+++ b/java/com/google/gerrit/server/schema/Schema_126.java
@@ -44,6 +44,7 @@
private final GitRepositoryManager repoManager;
private final AllUsersName allUsersName;
private final SystemGroupBackend systemGroupBackend;
+ private final ProjectConfig.Factory projectConfigFactory;
private final PersonIdent serverUser;
@Inject
@@ -52,11 +53,13 @@
GitRepositoryManager repoManager,
AllUsersName allUsersName,
SystemGroupBackend systemGroupBackend,
+ ProjectConfig.Factory projectConfigFactory,
@GerritPersonIdent PersonIdent serverUser) {
super(prior);
this.repoManager = repoManager;
this.allUsersName = allUsersName;
this.systemGroupBackend = systemGroupBackend;
+ this.projectConfigFactory = projectConfigFactory;
this.serverUser = serverUser;
}
@@ -64,7 +67,7 @@
protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException {
try (Repository git = repoManager.openRepository(allUsersName);
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, git)) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
String refsUsersShardedId = RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}";
config.remove(config.getAccessSection(refsUsersShardedId));
diff --git a/java/com/google/gerrit/server/schema/Schema_128.java b/java/com/google/gerrit/server/schema/Schema_128.java
index bd6b76a..4f5f962 100644
--- a/java/com/google/gerrit/server/schema/Schema_128.java
+++ b/java/com/google/gerrit/server/schema/Schema_128.java
@@ -42,6 +42,7 @@
private final GitRepositoryManager repoManager;
private final AllProjectsName allProjectsName;
private final SystemGroupBackend systemGroupBackend;
+ private final ProjectConfig.Factory projectConfigFactory;
private final PersonIdent serverUser;
@Inject
@@ -50,11 +51,13 @@
GitRepositoryManager repoManager,
AllProjectsName allProjectsName,
SystemGroupBackend systemGroupBackend,
+ ProjectConfig.Factory projectConfigFactory,
@GerritPersonIdent PersonIdent serverUser) {
super(prior);
this.repoManager = repoManager;
this.allProjectsName = allProjectsName;
this.systemGroupBackend = systemGroupBackend;
+ this.projectConfigFactory = projectConfigFactory;
this.serverUser = serverUser;
}
@@ -63,7 +66,7 @@
try (Repository git = repoManager.openRepository(allProjectsName);
MetaDataUpdate md =
new MetaDataUpdate(GitReferenceUpdated.DISABLED, allProjectsName, git)) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
GroupReference registered = systemGroupBackend.getGroup(REGISTERED_USERS);
AccessSection refsFor = config.getAccessSection("refs/for/*", true);
diff --git a/java/com/google/gerrit/server/schema/Schema_131.java b/java/com/google/gerrit/server/schema/Schema_131.java
index 3755211..e496096 100644
--- a/java/com/google/gerrit/server/schema/Schema_131.java
+++ b/java/com/google/gerrit/server/schema/Schema_131.java
@@ -38,15 +38,18 @@
"Rename 'Push Annotated/Signed Tag' permission to 'Create Annotated/Signed Tag'";
private final GitRepositoryManager repoManager;
+ private final ProjectConfig.Factory projectConfigFactory;
private final PersonIdent serverUser;
@Inject
Schema_131(
Provider<Schema_130> prior,
GitRepositoryManager repoManager,
+ ProjectConfig.Factory projectConfigFactory,
@GerritPersonIdent PersonIdent serverUser) {
super(prior);
this.repoManager = repoManager;
+ this.projectConfigFactory = projectConfigFactory;
this.serverUser = serverUser;
}
@@ -58,7 +61,7 @@
for (Project.NameKey projectName : repoList) {
try (Repository git = repoManager.openRepository(projectName);
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, projectName, git)) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
if (config.hasLegacyPermissions()) {
md.getCommitBuilder().setAuthor(serverUser);
md.getCommitBuilder().setCommitter(serverUser);
diff --git a/java/com/google/gerrit/server/schema/Schema_135.java b/java/com/google/gerrit/server/schema/Schema_135.java
index 66224c2..6281251 100644
--- a/java/com/google/gerrit/server/schema/Schema_135.java
+++ b/java/com/google/gerrit/server/schema/Schema_135.java
@@ -48,6 +48,7 @@
private final GitRepositoryManager repoManager;
private final AllProjectsName allProjectsName;
private final SystemGroupBackend systemGroupBackend;
+ private final ProjectConfig.Factory projectConfigFactory;
private final PersonIdent serverUser;
@Inject
@@ -56,11 +57,13 @@
GitRepositoryManager repoManager,
AllProjectsName allProjectsName,
SystemGroupBackend systemGroupBackend,
+ ProjectConfig.Factory projectConfigFactory,
@GerritPersonIdent PersonIdent serverUser) {
super(prior);
this.repoManager = repoManager;
this.allProjectsName = allProjectsName;
this.systemGroupBackend = systemGroupBackend;
+ this.projectConfigFactory = projectConfigFactory;
this.serverUser = serverUser;
}
@@ -69,7 +72,7 @@
try (Repository git = repoManager.openRepository(allProjectsName);
MetaDataUpdate md =
new MetaDataUpdate(GitReferenceUpdated.DISABLED, allProjectsName, git)) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
AccessSection meta = config.getAccessSection(RefNames.REFS_CONFIG, true);
Permission createRefsMetaConfigPermission = meta.getPermission(Permission.CREATE, true);
diff --git a/java/com/google/gerrit/server/schema/Schema_162.java b/java/com/google/gerrit/server/schema/Schema_162.java
index 7406bc6..85220c7 100644
--- a/java/com/google/gerrit/server/schema/Schema_162.java
+++ b/java/com/google/gerrit/server/schema/Schema_162.java
@@ -34,6 +34,7 @@
private final GitRepositoryManager repoManager;
private final AllProjectsName allProjectsName;
private final AllUsersName allUsersName;
+ private final ProjectConfig.Factory projectConfigFactory;
private final PersonIdent serverUser;
@Inject
@@ -42,11 +43,13 @@
GitRepositoryManager repoManager,
AllProjectsName allProjectsName,
AllUsersName allUsersName,
+ ProjectConfig.Factory projectConfigFactory,
@GerritPersonIdent PersonIdent serverUser) {
super(prior);
this.repoManager = repoManager;
this.allProjectsName = allProjectsName;
this.allUsersName = allUsersName;
+ this.projectConfigFactory = projectConfigFactory;
this.serverUser = serverUser;
}
@@ -54,7 +57,7 @@
protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException {
try (Repository git = repoManager.openRepository(allUsersName);
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, git)) {
- ProjectConfig cfg = ProjectConfig.read(md);
+ ProjectConfig cfg = projectConfigFactory.read(md);
if (allProjectsName.equals(cfg.getProject().getParent(allProjectsName))) {
return;
}
diff --git a/java/com/google/gerrit/server/schema/Schema_164.java b/java/com/google/gerrit/server/schema/Schema_164.java
index 8525478..818f84c 100644
--- a/java/com/google/gerrit/server/schema/Schema_164.java
+++ b/java/com/google/gerrit/server/schema/Schema_164.java
@@ -44,6 +44,7 @@
private final GitRepositoryManager repoManager;
private final AllUsersName allUsersName;
private final SystemGroupBackend systemGroupBackend;
+ private final ProjectConfig.Factory projectConfigFactory;
private final PersonIdent serverUser;
@Inject
@@ -52,11 +53,13 @@
GitRepositoryManager repoManager,
AllUsersName allUsersName,
SystemGroupBackend systemGroupBackend,
+ ProjectConfig.Factory projectConfigFactory,
@GerritPersonIdent PersonIdent serverUser) {
super(prior);
this.repoManager = repoManager;
this.allUsersName = allUsersName;
this.systemGroupBackend = systemGroupBackend;
+ this.projectConfigFactory = projectConfigFactory;
this.serverUser = serverUser;
}
@@ -68,7 +71,7 @@
md.getCommitBuilder().setCommitter(serverUser);
md.setMessage(COMMIT_MSG);
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
AccessSection groups = config.getAccessSection(RefNames.REFS_GROUPS + "*", true);
grant(
config,
diff --git a/java/com/google/gerrit/server/schema/Schema_165.java b/java/com/google/gerrit/server/schema/Schema_165.java
index cd6da55..b4f9523 100644
--- a/java/com/google/gerrit/server/schema/Schema_165.java
+++ b/java/com/google/gerrit/server/schema/Schema_165.java
@@ -48,6 +48,7 @@
private final GitRepositoryManager repoManager;
private final AllUsersName allUsersName;
private final SystemGroupBackend systemGroupBackend;
+ private final ProjectConfig.Factory projectConfigFactory;
private final PersonIdent serverUser;
@Inject
@@ -56,11 +57,13 @@
GitRepositoryManager repoManager,
AllUsersName allUsersName,
SystemGroupBackend systemGroupBackend,
+ ProjectConfig.Factory projectConfigFactory,
@GerritPersonIdent PersonIdent serverUser) {
super(prior);
this.repoManager = repoManager;
this.allUsersName = allUsersName;
this.systemGroupBackend = systemGroupBackend;
+ this.projectConfigFactory = projectConfigFactory;
this.serverUser = serverUser;
}
@@ -68,7 +71,7 @@
protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException {
try (Repository git = repoManager.openRepository(allUsersName);
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, git)) {
- ProjectConfig config = ProjectConfig.read(md);
+ ProjectConfig config = projectConfigFactory.read(md);
Optional<Permission> permission = findDefaultPermission(config);
if (!permission.isPresent()) {
// the default permission was not found, hence it cannot be fixed
diff --git a/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java b/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java
index 9efb976..66463be 100644
--- a/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java
+++ b/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java
@@ -37,6 +37,7 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.ChangeIsVisibleToPredicate;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.submit.MergeOpRepoManager.OpenRepo;
import com.google.gwtorm.server.OrmException;
@@ -87,20 +88,23 @@
private final PermissionBackend permissionBackend;
private final Provider<InternalChangeQuery> queryProvider;
- private final Map<QueryKey, List<ChangeData>> queryCache;
+ private final Map<QueryKey, ImmutableList<ChangeData>> queryCache;
private final Map<Branch.NameKey, Optional<RevCommit>> heads;
private final ProjectCache projectCache;
+ private final ChangeIsVisibleToPredicate changeIsVisibleToPredicate;
@Inject
LocalMergeSuperSetComputation(
PermissionBackend permissionBackend,
Provider<InternalChangeQuery> queryProvider,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ ChangeIsVisibleToPredicate changeIsVisibleToPredicate) {
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
this.queryProvider = queryProvider;
this.queryCache = new HashMap<>();
this.heads = new HashMap<>();
+ this.changeIsVisibleToPredicate = changeIsVisibleToPredicate;
}
@Override
@@ -147,10 +151,11 @@
Set<String> visibleHashes =
walkChangesByHashes(visibleCommits, Collections.emptySet(), or, b);
- Iterables.addAll(visibleChanges, byCommitsOnBranchNotMerged(or, db, b, visibleHashes));
-
Set<String> nonVisibleHashes = walkChangesByHashes(nonVisibleCommits, visibleHashes, or, b);
- Iterables.addAll(nonVisibleChanges, byCommitsOnBranchNotMerged(or, db, b, nonVisibleHashes));
+
+ ChangeSet partialSet = byCommitsOnBranchNotMerged(or, db, b, visibleHashes, nonVisibleHashes);
+ Iterables.addAll(visibleChanges, partialSet.changes());
+ Iterables.addAll(nonVisibleChanges, partialSet.nonVisibleChanges());
}
return new ChangeSet(visibleChanges, nonVisibleChanges);
@@ -206,24 +211,41 @@
return str.type;
}
- private List<ChangeData> byCommitsOnBranchNotMerged(
+ private ChangeSet byCommitsOnBranchNotMerged(
+ OpenRepo or,
+ ReviewDb db,
+ Branch.NameKey branch,
+ Set<String> visibleHashes,
+ Set<String> nonVisibleHashes)
+ throws OrmException, IOException {
+ List<ChangeData> potentiallyVisibleChanges =
+ byCommitsOnBranchNotMerged(or, db, branch, visibleHashes);
+ List<ChangeData> invisibleChanges =
+ new ArrayList<>(byCommitsOnBranchNotMerged(or, db, branch, nonVisibleHashes));
+ List<ChangeData> visibleChanges = new ArrayList<>(potentiallyVisibleChanges.size());
+ for (ChangeData cd : potentiallyVisibleChanges) {
+ if (changeIsVisibleToPredicate.match(cd)) {
+ visibleChanges.add(cd);
+ } else {
+ invisibleChanges.add(cd);
+ }
+ }
+ return new ChangeSet(visibleChanges, invisibleChanges);
+ }
+
+ private ImmutableList<ChangeData> byCommitsOnBranchNotMerged(
OpenRepo or, ReviewDb db, Branch.NameKey branch, Set<String> hashes)
throws OrmException, IOException {
if (hashes.isEmpty()) {
return ImmutableList.of();
}
QueryKey k = QueryKey.create(branch, hashes);
- List<ChangeData> cached = queryCache.get(k);
- if (cached != null) {
- return cached;
+ if (queryCache.containsKey(k)) {
+ return queryCache.get(k);
}
-
- List<ChangeData> result = new ArrayList<>();
- Iterable<ChangeData> destChanges =
- queryProvider.get().byCommitsOnBranchNotMerged(or.repo, db, branch, hashes);
- for (ChangeData chd : destChanges) {
- result.add(chd);
- }
+ ImmutableList<ChangeData> result =
+ ImmutableList.copyOf(
+ queryProvider.get().byCommitsOnBranchNotMerged(or.repo, db, branch, hashes));
queryCache.put(k, result);
return result;
}
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategy.java b/java/com/google/gerrit/server/submit/SubmitStrategy.java
index 4cefd7d..6511d25 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategy.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategy.java
@@ -46,6 +46,7 @@
import com.google.gerrit.server.logging.RequestId;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.submit.MergeOp.CommitStatus;
@@ -115,6 +116,7 @@
final OnSubmitValidators.Factory onSubmitValidatorsFactory;
final TagCache tagCache;
final Provider<InternalChangeQuery> queryProvider;
+ final ProjectConfig.Factory projectConfigFactory;
final Branch.NameKey destBranch;
final CodeReviewRevWalk rw;
@@ -154,6 +156,7 @@
OnSubmitValidators.Factory onSubmitValidatorsFactory,
TagCache tagCache,
Provider<InternalChangeQuery> queryProvider,
+ ProjectConfig.Factory projectConfigFactory,
@Assisted Branch.NameKey destBranch,
@Assisted CommitStatus commitStatus,
@Assisted CodeReviewRevWalk rw,
@@ -176,6 +179,7 @@
this.repoManager = repoManager;
this.cmUtil = cmUtil;
this.labelNormalizer = labelNormalizer;
+ this.projectConfigFactory = projectConfigFactory;
this.patchSetInfoFactory = patchSetInfoFactory;
this.psUtil = psUtil;
this.projectCache = projectCache;
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
index 8a4fbfb..3be4c31 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
@@ -145,7 +145,7 @@
if (RefNames.REFS_CONFIG.equals(refName)) {
logger.atFine().log("Loading new configuration from %s", RefNames.REFS_CONFIG);
try {
- ProjectConfig cfg = new ProjectConfig(getProject());
+ ProjectConfig cfg = args.projectConfigFactory.create(getProject());
cfg.load(ctx.getRevWalk(), commit);
} catch (Exception e) {
throw new IntegrationException(
diff --git a/java/com/google/gerrit/testing/NoteDbMode.java b/java/com/google/gerrit/testing/NoteDbMode.java
index d4a7c7e..e46acc3 100644
--- a/java/com/google/gerrit/testing/NoteDbMode.java
+++ b/java/com/google/gerrit/testing/NoteDbMode.java
@@ -52,7 +52,7 @@
value = System.getProperty(SYS_PROP);
}
if (Strings.isNullOrEmpty(value)) {
- return OFF;
+ return ON;
}
value = value.toUpperCase().replace("-", "_");
NoteDbMode mode = Enums.getIfPresent(NoteDbMode.class, value).orNull();
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
index 7a4a901..c939ac1 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
@@ -182,6 +182,28 @@
}
@Test
+ public void revertExcludedProjectChangeWithoutCLA() throws Exception {
+ // Contributor agreements configured with excludeProjects = ExcludedProject
+ // in AbstractDaemonTest.configureContributorAgreement(...)
+ assume().that(isContributorAgreementsEnabled()).isTrue();
+
+ // Create a change succeeds when agreement is not required
+ setUseContributorAgreements(InheritableBoolean.FALSE);
+ // Project name includes test method name which contains ExcludedProject
+ ChangeInfo change = gApi.changes().create(newChangeInput()).get();
+
+ // Approve and submit it
+ setApiUser(admin);
+ gApi.changes().id(change.changeId).current().review(ReviewInput.approve());
+ gApi.changes().id(change.changeId).current().submit(new SubmitInput());
+
+ // Revert in excluded project is allowed even when CLA is required but not signed
+ setApiUser(user);
+ setUseContributorAgreements(InheritableBoolean.TRUE);
+ gApi.changes().id(change.changeId).revert();
+ }
+
+ @Test
public void cherrypickChangeWithoutCLA() throws Exception {
assume().that(isContributorAgreementsEnabled()).isTrue();
@@ -240,6 +262,17 @@
gApi.changes().create(newChangeInput());
}
+ @Test
+ public void createExcludedProjectChangeIgnoresCLA() throws Exception {
+ // Contributor agreements configured with excludeProjects = ExcludedProject
+ // in AbstractDaemonTest.configureContributorAgreement(...)
+ assume().that(isContributorAgreementsEnabled()).isTrue();
+
+ // Create a change in excluded project is allowed even when CLA is required but not signed.
+ setUseContributorAgreements(InheritableBoolean.TRUE);
+ gApi.changes().create(newChangeInput());
+ }
+
private void assertAgreement(AgreementInfo info, ContributorAgreement ca) {
assertThat(info.name).isEqualTo(ca.getName());
assertThat(info.description).isEqualTo(ca.getDescription());
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 7063b27..ed4137d 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -80,6 +80,7 @@
import com.google.gerrit.extensions.api.changes.NotifyInfo;
import com.google.gerrit.extensions.api.changes.RebaseInput;
import com.google.gerrit.extensions.api.changes.RecipientType;
+import com.google.gerrit.extensions.api.changes.RelatedChangeAndCommitInfo;
import com.google.gerrit.extensions.api.changes.RevertInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
@@ -124,6 +125,7 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.mail.Address;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -139,8 +141,12 @@
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.index.change.ChangeIndex;
+import com.google.gerrit.server.index.change.ChangeIndexCollection;
+import com.google.gerrit.server.index.change.IndexedChangeQuery;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.project.testing.Util;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.restapi.change.PostReview;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
@@ -185,6 +191,9 @@
@Inject private AccountOperations accountOperations;
+ @Inject private ChangeIndexCollection changeIndexCollection;
+ @Inject private IndexConfig indexConfig;
+
private ChangeIndexedCounter changeIndexedCounter;
private RegistrationHandle changeIndexedCounterHandle;
@@ -943,13 +952,77 @@
RevisionInfo ri2 = ci2.revisions.get(ci2.currentRevision);
assertThat(ri2.commit.parents.get(0).commit).isEqualTo(branchTip);
+ Change.Id id1 = r1.getChange().getId();
RebaseInput in = new RebaseInput();
- in.base = Integer.toString(r1.getChange().getId().get());
+ in.base = id1.toString();
gApi.changes().id(r2.getChangeId()).rebase(in);
+ Change.Id id2 = r2.getChange().getId();
ci2 = get(r2.getChangeId(), CURRENT_REVISION, CURRENT_COMMIT);
ri2 = ci2.revisions.get(ci2.currentRevision);
assertThat(ri2.commit.parents.get(0).commit).isEqualTo(r1.getCommit().name());
+
+ List<RelatedChangeAndCommitInfo> related = getRelated(id2, ri2._number);
+ assertThat(related).hasSize(2);
+ assertThat(related.get(0)._changeNumber).isEqualTo(id2.get());
+ assertThat(related.get(0)._revisionNumber).isEqualTo(2);
+ assertThat(related.get(1)._changeNumber).isEqualTo(id1.get());
+ assertThat(related.get(1)._revisionNumber).isEqualTo(1);
+ }
+
+ @Test
+ public void rebaseOnClosedChange() throws Exception {
+ String branchTip = testRepo.getRepository().exactRef("HEAD").getObjectId().name();
+ PushOneCommit.Result r1 = createChange();
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result r2 = createChange();
+
+ ChangeInfo ci2 = get(r2.getChangeId(), CURRENT_REVISION, CURRENT_COMMIT);
+ RevisionInfo ri2 = ci2.revisions.get(ci2.currentRevision);
+ assertThat(ri2.commit.parents.get(0).commit).isEqualTo(branchTip);
+
+ // Submit first change.
+ Change.Id id1 = r1.getChange().getId();
+ gApi.changes().id(id1.get()).current().review(ReviewInput.approve());
+ gApi.changes().id(id1.get()).current().submit();
+
+ // Rebase second change on first change.
+ RebaseInput in = new RebaseInput();
+ in.base = id1.toString();
+ gApi.changes().id(r2.getChangeId()).rebase(in);
+
+ Change.Id id2 = r2.getChange().getId();
+ ci2 = get(r2.getChangeId(), CURRENT_REVISION, CURRENT_COMMIT);
+ ri2 = ci2.revisions.get(ci2.currentRevision);
+ assertThat(ri2.commit.parents.get(0).commit).isEqualTo(r1.getCommit().name());
+
+ assertThat(getRelated(id2, ri2._number)).isEmpty();
+ }
+
+ @Test
+ public void rebaseFromRelationChainToClosedChange() throws Exception {
+ PushOneCommit.Result r1 = createChange();
+ testRepo.reset("HEAD~1");
+
+ createChange();
+ PushOneCommit.Result r3 = createChange();
+
+ // Submit first change.
+ Change.Id id1 = r1.getChange().getId();
+ gApi.changes().id(id1.get()).current().review(ReviewInput.approve());
+ gApi.changes().id(id1.get()).current().submit();
+
+ // Rebase third change on first change.
+ RebaseInput in = new RebaseInput();
+ in.base = id1.toString();
+ gApi.changes().id(r3.getChangeId()).rebase(in);
+
+ Change.Id id3 = r3.getChange().getId();
+ ChangeInfo ci3 = get(r3.getChangeId(), CURRENT_REVISION, CURRENT_COMMIT);
+ RevisionInfo ri3 = ci3.revisions.get(ci3.currentRevision);
+ assertThat(ri3.commit.parents.get(0).commit).isEqualTo(r1.getCommit().name());
+
+ assertThat(getRelated(id3, ri3._number)).isEmpty();
}
@Test
@@ -1244,6 +1317,23 @@
}
@Test
+ public void deleteChangeUpdatesIndex() throws Exception {
+ PushOneCommit.Result changeResult = createChange();
+ String changeId = changeResult.getChangeId();
+ Change.Id id = changeResult.getChange().getId();
+
+ ChangeIndex idx = changeIndexCollection.getSearchIndex();
+
+ Optional<ChangeData> result =
+ idx.get(id, IndexedChangeQuery.createOptions(indexConfig, 0, 1, ImmutableSet.of()));
+
+ assertThat(result.isPresent()).isTrue();
+ gApi.changes().id(changeId).delete();
+ result = idx.get(id, IndexedChangeQuery.createOptions(indexConfig, 0, 1, ImmutableSet.of()));
+ assertThat(result.isPresent()).isFalse();
+ }
+
+ @Test
public void rebaseUpToDateChange() throws Exception {
PushOneCommit.Result r = createChange();
exception.expect(ResourceConflictException.class);
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
index 9fcbaa7..995f89b 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
@@ -33,6 +33,7 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.projects.BranchInput;
+import com.google.gerrit.extensions.api.projects.CommentLinkInfo;
import com.google.gerrit.extensions.api.projects.ConfigInfo;
import com.google.gerrit.extensions.api.projects.ConfigInput;
import com.google.gerrit.extensions.api.projects.DescriptionInput;
@@ -51,7 +52,10 @@
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.index.IndexExecutor;
+import com.google.gerrit.server.project.CommentLinkInfoImpl;
import com.google.inject.Inject;
+import java.util.HashMap;
+import java.util.Map;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
@@ -61,6 +65,13 @@
@NoHttpd
public class ProjectIT extends AbstractDaemonTest {
+ private static final String BUGZILLA = "bugzilla";
+ private static final String BUGZILLA_LINK = "http://bugzilla.example.com/?id=$2";
+ private static final String BUGZILLA_MATCH = "(bug\\\\s+#?)(\\\\d+)";
+ private static final String JIRA = "jira";
+ private static final String JIRA_LINK = "http://jira.example.com/?id=$2";
+ private static final String JIRA_MATCH = "(jira\\\\s+#?)(\\\\d+)";
+
@Inject private DynamicSet<ProjectIndexedListener> projectIndexedListeners;
@Inject
@@ -603,6 +614,31 @@
setMaxObjectSize("100 foo");
}
+ @Test
+ public void noCommentlinksByDefault() throws Exception {
+ assertThat(getConfig().commentlinks).isEmpty();
+ }
+
+ @Test
+ @GerritConfig(name = "commentlink.bugzilla.match", value = BUGZILLA_MATCH)
+ @GerritConfig(name = "commentlink.bugzilla.link", value = BUGZILLA_LINK)
+ @GerritConfig(name = "commentlink.jira.match", value = JIRA_MATCH)
+ @GerritConfig(name = "commentlink.jira.link", value = JIRA_LINK)
+ public void projectConfigUsesCommentlinksFromGlobalConfig() throws Exception {
+ Map<String, CommentLinkInfo> expected = new HashMap<>();
+ expected.put(BUGZILLA, commentLinkInfo(BUGZILLA, BUGZILLA_MATCH, BUGZILLA_LINK));
+ expected.put(JIRA, commentLinkInfo(JIRA, JIRA_MATCH, JIRA_LINK));
+ assertCommentLinks(getConfig(), expected);
+ }
+
+ private CommentLinkInfo commentLinkInfo(String name, String match, String link) {
+ return new CommentLinkInfoImpl(name, match, link, null /*html*/, null /*enabled*/);
+ }
+
+ private void assertCommentLinks(ConfigInfo actual, Map<String, CommentLinkInfo> expected) {
+ assertThat(actual.commentlinks).containsExactlyEntriesIn(expected);
+ }
+
private ConfigInfo setConfig(Project.NameKey name, ConfigInput input) throws Exception {
return gApi.projects().name(name.get()).config(input);
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
index cd20765..d713db6 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
@@ -1024,7 +1024,7 @@
}
@Test
- public void queryChangesWithUnresolvedCommentCount() throws Exception {
+ public void queryChangesWithCommentCounts() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
PushOneCommit.Result r1 = createChange();
@@ -1043,6 +1043,7 @@
// if we allow users to resolve a robot comment, then this test should
// be modified.
assertThat(result.unresolvedCommentCount).isEqualTo(0);
+ assertThat(result.totalCommentCount).isEqualTo(1);
} finally {
enableDb(ctx);
}
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index 91a1278..9ad14ee 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -239,6 +239,7 @@
@Test
public void publishEditRestWithoutCLA() throws Exception {
+ configureContributorAgreement(true);
createArbitraryEditFor(changeId);
setUseContributorAgreements(InheritableBoolean.TRUE);
adminRestSession.post(urlPublish(changeId)).assertForbidden();
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
index 943b052..66a09f8 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
@@ -189,7 +189,7 @@
try (MetaDataUpdate md = metaDataUpdateFactory.create(sub)) {
md.setMessage("Added superproject subscription");
SubscribeSection s;
- ProjectConfig pc = ProjectConfig.read(md);
+ ProjectConfig pc = projectConfigFactory.read(md);
if (pc.getSubscribeSections().containsKey(superName)) {
s = pc.getSubscribeSections().get(superName);
} else {
diff --git a/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java
index 6563de3..ca8d3ce 100644
--- a/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java
@@ -60,7 +60,6 @@
RestCall.post("/projects/%s/access"),
RestCall.put("/projects/%s/access:review"),
RestCall.get("/projects/%s/check.access"),
- RestCall.post("/projects/%s/check.access"),
RestCall.put("/projects/%s/ban"),
RestCall.get("/projects/%s/statistics.git"),
RestCall.post("/projects/%s/index"),
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeIdIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeIdIT.java
index a2ad7fc..3f1608c 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeIdIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeIdIT.java
@@ -18,6 +18,7 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.extensions.api.changes.ChangeApi;
+import com.google.gerrit.reviewdb.client.Change;
import org.junit.Test;
public class ChangeIdIT extends AbstractDaemonTest {
@@ -92,6 +93,43 @@
res.assertNotFound();
}
+ @Test
+ public void changeNumberRedirects() throws Exception {
+ // This test tests a redirect that is primarily intended for the UI (though the backend doesn't
+ // really care who the caller is). The redirect rewrites a shorthand change number URL (/123) to
+ // it's canonical long form (/c/project/+/123).
+ int changeId = createChange().getChange().getId().id;
+ RestResponse res = anonymousRestSession.get("/" + changeId);
+ res.assertTemporaryRedirect("/c/" + project.get() + "/+/" + changeId + "/");
+ }
+
+ @Test
+ public void changeNumberRedirectsWithTrailingSlash() throws Exception {
+ int changeId = createChange().getChange().getId().id;
+ RestResponse res = anonymousRestSession.get("/" + changeId + "/");
+ res.assertTemporaryRedirect("/c/" + project.get() + "/+/" + changeId + "/");
+ }
+
+ @Test
+ public void changeNumberOverflowNotFound() throws Exception {
+ RestResponse res = anonymousRestSession.get("/9" + Long.MAX_VALUE);
+ res.assertNotFound();
+ }
+
+ @Test
+ public void unknownChangeNumberNotFound() throws Exception {
+ RestResponse res = anonymousRestSession.get("/10");
+ res.assertNotFound();
+ }
+
+ @Test
+ public void hiddenChangeNotFound() throws Exception {
+ Change.Id changeId = createChange().getChange().getId();
+ gApi.changes().id(changeId.id).setPrivate(true, null);
+ RestResponse res = anonymousRestSession.get("/" + changeId.id);
+ res.assertNotFound();
+ }
+
private static String changeDetail(String changeId) {
return "/changes/" + changeId + "/detail";
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CorsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CorsIT.java
index 0af9708..6fd3bab 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CorsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CorsIT.java
@@ -34,6 +34,7 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit.Result;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.RestSession;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -44,6 +45,7 @@
import java.util.stream.Stream;
import org.apache.http.Header;
import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
import org.apache.http.client.fluent.Executor;
import org.apache.http.client.fluent.Request;
import org.apache.http.cookie.Cookie;
@@ -99,6 +101,30 @@
}
@Test
+ public void originsOnNotFoundException() throws Exception {
+ String url = "/changes/999/detail";
+ check(url, true, "http://example.com", adminRestSession, 404);
+ check(url, false, "http://friendsly", adminRestSession, 404);
+ }
+
+ @Test
+ public void originsOnBadRequestException() throws Exception {
+ String url = "/config/server/caches/?format=NONSENSE";
+ check(url, true, "http://example.com", adminRestSession, 400);
+ check(url, false, "http://friendsly", adminRestSession, 400);
+ }
+
+ @Test
+ public void originsOnForbidden() throws Exception {
+ Result change = createChange();
+ // Make change private to hide it
+ gApi.changes().id(change.getChangeId()).setPrivate(true, "now private");
+ String url = "/changes/" + change.getChangeId() + "/detail";
+ check(url, true, "http://example.com", anonymousRestSession, 404);
+ check(url, false, "http://friendsly", anonymousRestSession, 404);
+ }
+
+ @Test
public void putWithServerOriginAcceptedWithNoCorsResponseHeaders() throws Exception {
Result change = createChange();
String origin = adminRestSession.url();
@@ -247,9 +273,15 @@
}
private void check(String url, boolean accept, String origin) throws Exception {
+ check(url, accept, origin, adminRestSession, HttpStatus.SC_OK);
+ }
+
+ private void check(
+ String url, boolean accept, String origin, RestSession restSession, int httpStatusCode)
+ throws Exception {
Header hdr = new BasicHeader(ORIGIN, origin);
- RestResponse r = adminRestSession.getWithHeader(url, hdr);
- r.assertOK();
+ RestResponse r = restSession.getWithHeader(url, hdr);
+ r.assertStatus(httpStatusCode);
checkCors(r, accept, origin);
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
index 206cc68..222973e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
@@ -19,6 +19,7 @@
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
@@ -30,6 +31,7 @@
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -332,6 +334,16 @@
move(r.getChangeId(), null);
}
+ @Test
+ @GerritConfig(name = "change.move", value = "false")
+ public void moveCanBeDisabledByConfig() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ exception.expect(MethodNotAllowedException.class);
+ exception.expectMessage("move changes endpoint is disabled");
+ move(r.getChangeId(), null);
+ }
+
private void move(int changeNum, String destination) throws RestApiException {
gApi.changes().id(changeNum).move(destination);
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java
index c1dda00..229122b 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.rest.change;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
@@ -24,10 +25,9 @@
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.api.projects.BranchInput;
-import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.SubmitType;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -657,7 +657,7 @@
}
@Test
- public void dependencyOnHiddenChangeShouldPreventMergeButDoesnt() throws Exception {
+ public void dependencyOnHiddenChangePreventsMerge() throws Exception {
grantLabel("Code-Review", -2, 2, project, "refs/heads/*", false, REGISTERED_USERS, false);
grant(project, "refs/*", Permission.SUBMIT, false, REGISTERED_USERS);
@@ -686,18 +686,80 @@
assertThat(e.getMessage()).isEqualTo("Not found: " + changeResult.getChangeId());
}
- // Submit the second change which has a dependency on the first change which is not visible to
- // the user. We would expect the submit to fail, but instead the submit succeeds and the hidden
- // change gets submitted too.
- // TODO(ekempin): Make this submit fail.
- gApi.changes().id(change2Result.getChangeId()).current().submit(new SubmitInput());
+ // Submit is expected to fail.
+ try {
+ gApi.changes().id(change2Result.getChangeId()).current().submit();
+ fail("expected failure");
+ } catch (AuthException e) {
+ assertThat(e.getMessage())
+ .isEqualTo(
+ "A change to be submitted with "
+ + change2Result.getChange().getId().id
+ + " is not visible");
+ }
+ assertRefUpdatedEvents();
+ assertChangeMergedEvents();
+ }
- // Verify that both changes have been submitted.
- setApiUser(admin);
- assertThat(gApi.changes().id(changeResult.getChangeId()).get().status)
- .isEqualTo(ChangeStatus.MERGED);
- assertThat(gApi.changes().id(change2Result.getChangeId()).get().status)
- .isEqualTo(ChangeStatus.MERGED);
+ @Test
+ public void dependencyOnHiddenChangeUsingTopicPreventsMerge() throws Exception {
+ // Construct a topic where a change included by topic depends on a private change that is not
+ // visible to the submitting user
+ // (c1) --- topic --- (c2b)
+ // |
+ // (c2a) <= private
+ assume().that(isSubmitWholeTopicEnabled()).isTrue();
+
+ Project.NameKey p1 = createProject("project-where-we-submit");
+ Project.NameKey p2 = createProject("project-impacted-via-topic");
+
+ grantLabel("Code-Review", -2, 2, p1, "refs/heads/*", false, REGISTERED_USERS, false);
+ grant(p1, "refs/*", Permission.SUBMIT, false, REGISTERED_USERS);
+ grantLabel("Code-Review", -2, 2, p2, "refs/heads/*", false, REGISTERED_USERS, false);
+ grant(p2, "refs/*", Permission.SUBMIT, false, REGISTERED_USERS);
+
+ TestRepository<?> repo1 = cloneProject(p1);
+ TestRepository<?> repo2 = cloneProject(p2);
+
+ PushOneCommit.Result change1 =
+ createChange(repo1, "master", "A fresh change in repo1", "a.txt", "1", "topic-to-submit");
+ approve(change1.getChangeId());
+ PushOneCommit push =
+ pushFactory.create(
+ db, admin.getIdent(), repo2, "An ancestor change in repo2", "a.txt", "2");
+ PushOneCommit.Result change2a = push.to("refs/for/master");
+ approve(change2a.getChangeId());
+ PushOneCommit.Result change2b =
+ createChange(
+ repo2, "master", "A topic-linked change in repo2", "a.txt", "2", "topic-to-submit");
+ approve(change2b.getChangeId());
+
+ // Mark change2a private so that it's not visible to user.
+ gApi.changes().id(change2a.getChangeId()).setPrivate(true, "nobody should see this");
+
+ setApiUser(user);
+
+ // Verify that user cannot see change2a
+ try {
+ gApi.changes().id(change2a.getChangeId()).get();
+ fail("expected failure");
+ } catch (ResourceNotFoundException e) {
+ assertThat(e.getMessage()).isEqualTo("Not found: " + change2a.getChangeId());
+ }
+
+ // Submit is expected to fail.
+ try {
+ gApi.changes().id(change1.getChangeId()).current().submit();
+ fail("expected failure");
+ } catch (AuthException e) {
+ assertThat(e.getMessage())
+ .isEqualTo(
+ "A change to be submitted with "
+ + change1.getChange().getId().id
+ + " is not visible");
+ }
+ assertRefUpdatedEvents();
+ assertChangeMergedEvents();
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 8b1f4bc..0d40a1c 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -716,7 +716,7 @@
}
@Test
- public void queryChangesWithUnresolvedCommentCount() throws Exception {
+ public void queryChangesWithCommentCount() throws Exception {
// PS1 has three comments in three different threads, PS2 has one comment in one thread.
PushOneCommit.Result result = createChange("change 1", FILE_NAME, "content 1");
String changeId1 = result.getChangeId();
@@ -751,8 +751,11 @@
ChangeInfo changeInfo2 = Iterables.getOnlyElement(query(changeId2));
ChangeInfo changeInfo3 = Iterables.getOnlyElement(query(changeId3));
assertThat(changeInfo1.unresolvedCommentCount).isEqualTo(2);
+ assertThat(changeInfo1.totalCommentCount).isEqualTo(4);
assertThat(changeInfo2.unresolvedCommentCount).isEqualTo(0);
+ assertThat(changeInfo2.totalCommentCount).isEqualTo(2);
assertThat(changeInfo3.unresolvedCommentCount).isEqualTo(1);
+ assertThat(changeInfo3.totalCommentCount).isEqualTo(2);
} finally {
enableDb(ctx);
}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java b/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
index 5d3b223..8e8aeac 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
@@ -24,9 +24,10 @@
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GerritConfig;
+import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.common.RawInputUtil;
+import com.google.gerrit.extensions.api.changes.RelatedChangeAndCommitInfo;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.index.IndexConfig;
@@ -35,8 +36,6 @@
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.restapi.change.ChangesCollection;
import com.google.gerrit.server.restapi.change.GetRelated;
-import com.google.gerrit.server.restapi.change.GetRelated.ChangeAndCommit;
-import com.google.gerrit.server.restapi.change.GetRelated.RelatedInfo;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
@@ -56,6 +55,7 @@
import org.junit.Before;
import org.junit.Test;
+@NoHttpd
public class GetRelatedIT extends AbstractDaemonTest {
private static final int MAX_TERMS = 10;
@@ -577,17 +577,6 @@
assertRelated(cd.change().currentPatchSetId());
}
- private List<ChangeAndCommit> getRelated(PatchSet.Id ps) throws Exception {
- return getRelated(ps.getParentKey(), ps.get());
- }
-
- private List<ChangeAndCommit> getRelated(Change.Id changeId, int ps) throws Exception {
- String url = String.format("/changes/%d/revisions/%d/related", changeId.get(), ps);
- RestResponse r = adminRestSession.get(url);
- r.assertOK();
- return newGson().fromJson(r.getReader(), RelatedInfo.class).changes;
- }
-
private RevCommit parseBody(RevCommit c) throws Exception {
testRepo.getRevWalk().parseBody(c);
return c;
@@ -601,9 +590,9 @@
return Iterables.getOnlyElement(queryProvider.get().byCommit(c));
}
- private ChangeAndCommit changeAndCommit(
+ private RelatedChangeAndCommitInfo changeAndCommit(
PatchSet.Id psId, ObjectId commitId, int currentRevisionNum) {
- ChangeAndCommit result = new ChangeAndCommit();
+ RelatedChangeAndCommitInfo result = new RelatedChangeAndCommitInfo();
result.project = project.get();
result._changeNumber = psId.getParentKey().get();
result.commit = new CommitInfo();
@@ -631,13 +620,14 @@
}
}
- private void assertRelated(PatchSet.Id psId, ChangeAndCommit... expected) throws Exception {
- List<ChangeAndCommit> actual = getRelated(psId);
+ private void assertRelated(PatchSet.Id psId, RelatedChangeAndCommitInfo... expected)
+ throws Exception {
+ List<RelatedChangeAndCommitInfo> actual = getRelated(psId);
assertThat(actual).named("related to " + psId).hasSize(expected.length);
for (int i = 0; i < actual.size(); i++) {
String name = "index " + i + " related to " + psId;
- ChangeAndCommit a = actual.get(i);
- ChangeAndCommit e = expected[i];
+ RelatedChangeAndCommitInfo a = actual.get(i);
+ RelatedChangeAndCommitInfo e = expected[i];
assertThat(a.project).named("project of " + name).isEqualTo(e.project);
assertThat(a._changeNumber).named("change ID of " + name).isEqualTo(e._changeNumber);
// Don't bother checking changeId; assume _changeNumber is sufficient.
diff --git a/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java b/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java
index 5a067f1..eb6f189 100644
--- a/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java
+++ b/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java
@@ -74,6 +74,7 @@
@Inject private SchemaCreator schemaCreator;
@Inject protected ThreadLocalRequestContext requestContext;
@Inject private ChangeNotes.Factory changeNotesFactory;
+ @Inject private ProjectConfig.Factory projectConfigFactory;
private LifecycleManager lifecycle;
private ReviewDb db;
@@ -198,7 +199,7 @@
private ProjectConfig loadAllProjects() throws Exception {
try (Repository repo = repoManager.openRepository(allProjects)) {
- ProjectConfig pc = new ProjectConfig(allProjects);
+ ProjectConfig pc = projectConfigFactory.create(allProjects);
pc.load(repo);
return pc;
}
diff --git a/javatests/com/google/gerrit/server/config/RepositoryConfigTest.java b/javatests/com/google/gerrit/server/config/RepositoryConfigTest.java
index 2edcf7c..3da35b2 100644
--- a/javatests/com/google/gerrit/server/config/RepositoryConfigTest.java
+++ b/javatests/com/google/gerrit/server/config/RepositoryConfigTest.java
@@ -19,7 +19,7 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.client.SubmitType;
-import com.google.gerrit.reviewdb.client.Project.NameKey;
+import com.google.gerrit.reviewdb.client.Project;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
@@ -41,35 +41,35 @@
@Test
public void defaultSubmitTypeWhenNotConfigured() {
// Check expected value explicitly rather than depending on constant.
- assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
+ assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someProject")))
.isEqualTo(SubmitType.INHERIT);
}
@Test
public void defaultSubmitTypeForStarFilter() {
configureDefaultSubmitType("*", SubmitType.CHERRY_PICK);
- assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
+ assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someProject")))
.isEqualTo(SubmitType.CHERRY_PICK);
configureDefaultSubmitType("*", SubmitType.FAST_FORWARD_ONLY);
- assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
+ assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someProject")))
.isEqualTo(SubmitType.FAST_FORWARD_ONLY);
configureDefaultSubmitType("*", SubmitType.REBASE_IF_NECESSARY);
- assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
+ assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someProject")))
.isEqualTo(SubmitType.REBASE_IF_NECESSARY);
configureDefaultSubmitType("*", SubmitType.REBASE_ALWAYS);
- assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
+ assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someProject")))
.isEqualTo(SubmitType.REBASE_ALWAYS);
}
@Test
public void defaultSubmitTypeForSpecificFilter() {
configureDefaultSubmitType("someProject", SubmitType.CHERRY_PICK);
- assertThat(repoCfg.getDefaultSubmitType(new NameKey("someOtherProject")))
+ assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someOtherProject")))
.isEqualTo(RepositoryConfig.DEFAULT_SUBMIT_TYPE);
- assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
+ assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someProject")))
.isEqualTo(SubmitType.CHERRY_PICK);
}
@@ -79,13 +79,13 @@
configureDefaultSubmitType("somePath/*", SubmitType.CHERRY_PICK);
configureDefaultSubmitType("*", SubmitType.MERGE_ALWAYS);
- assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
+ assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someProject")))
.isEqualTo(SubmitType.MERGE_ALWAYS);
- assertThat(repoCfg.getDefaultSubmitType(new NameKey("somePath/someProject")))
+ assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("somePath/someProject")))
.isEqualTo(SubmitType.CHERRY_PICK);
- assertThat(repoCfg.getDefaultSubmitType(new NameKey("somePath/somePath/someProject")))
+ assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("somePath/somePath/someProject")))
.isEqualTo(SubmitType.REBASE_IF_NECESSARY);
}
@@ -99,14 +99,14 @@
@Test
public void ownerGroupsWhenNotConfigured() {
- assertThat(repoCfg.getOwnerGroups(new NameKey("someProject"))).isEmpty();
+ assertThat(repoCfg.getOwnerGroups(new Project.NameKey("someProject"))).isEmpty();
}
@Test
public void ownerGroupsForStarFilter() {
ImmutableList<String> ownerGroups = ImmutableList.of("group1", "group2");
configureOwnerGroups("*", ownerGroups);
- assertThat(repoCfg.getOwnerGroups(new NameKey("someProject")))
+ assertThat(repoCfg.getOwnerGroups(new Project.NameKey("someProject")))
.containsExactlyElementsIn(ownerGroups);
}
@@ -114,8 +114,8 @@
public void ownerGroupsForSpecificFilter() {
ImmutableList<String> ownerGroups = ImmutableList.of("group1", "group2");
configureOwnerGroups("someProject", ownerGroups);
- assertThat(repoCfg.getOwnerGroups(new NameKey("someOtherProject"))).isEmpty();
- assertThat(repoCfg.getOwnerGroups(new NameKey("someProject")))
+ assertThat(repoCfg.getOwnerGroups(new Project.NameKey("someOtherProject"))).isEmpty();
+ assertThat(repoCfg.getOwnerGroups(new Project.NameKey("someProject")))
.containsExactlyElementsIn(ownerGroups);
}
@@ -129,13 +129,13 @@
configureOwnerGroups("somePath/*", ownerGroups2);
configureOwnerGroups("somePath/somePath/*", ownerGroups3);
- assertThat(repoCfg.getOwnerGroups(new NameKey("someProject")))
+ assertThat(repoCfg.getOwnerGroups(new Project.NameKey("someProject")))
.containsExactlyElementsIn(ownerGroups1);
- assertThat(repoCfg.getOwnerGroups(new NameKey("somePath/someProject")))
+ assertThat(repoCfg.getOwnerGroups(new Project.NameKey("somePath/someProject")))
.containsExactlyElementsIn(ownerGroups2);
- assertThat(repoCfg.getOwnerGroups(new NameKey("somePath/somePath/someProject")))
+ assertThat(repoCfg.getOwnerGroups(new Project.NameKey("somePath/somePath/someProject")))
.containsExactlyElementsIn(ownerGroups3);
}
@@ -149,22 +149,24 @@
@Test
public void basePathWhenNotConfigured() {
- assertThat(repoCfg.getBasePath(new NameKey("someProject"))).isNull();
+ assertThat(repoCfg.getBasePath(new Project.NameKey("someProject"))).isNull();
}
@Test
public void basePathForStarFilter() {
String basePath = "/someAbsolutePath/someDirectory";
configureBasePath("*", basePath);
- assertThat(repoCfg.getBasePath(new NameKey("someProject")).toString()).isEqualTo(basePath);
+ assertThat(repoCfg.getBasePath(new Project.NameKey("someProject")).toString())
+ .isEqualTo(basePath);
}
@Test
public void basePathForSpecificFilter() {
String basePath = "/someAbsolutePath/someDirectory";
configureBasePath("someProject", basePath);
- assertThat(repoCfg.getBasePath(new NameKey("someOtherProject"))).isNull();
- assertThat(repoCfg.getBasePath(new NameKey("someProject")).toString()).isEqualTo(basePath);
+ assertThat(repoCfg.getBasePath(new Project.NameKey("someOtherProject"))).isNull();
+ assertThat(repoCfg.getBasePath(new Project.NameKey("someProject")).toString())
+ .isEqualTo(basePath);
}
@Test
@@ -179,12 +181,14 @@
configureBasePath("project/*", basePath3);
configureBasePath("*", basePath4);
- assertThat(repoCfg.getBasePath(new NameKey("project1")).toString()).isEqualTo(basePath1);
- assertThat(repoCfg.getBasePath(new NameKey("project/project/someProject")).toString())
+ assertThat(repoCfg.getBasePath(new Project.NameKey("project1")).toString())
+ .isEqualTo(basePath1);
+ assertThat(repoCfg.getBasePath(new Project.NameKey("project/project/someProject")).toString())
.isEqualTo(basePath2);
- assertThat(repoCfg.getBasePath(new NameKey("project/someProject")).toString())
+ assertThat(repoCfg.getBasePath(new Project.NameKey("project/someProject")).toString())
.isEqualTo(basePath3);
- assertThat(repoCfg.getBasePath(new NameKey("someProject")).toString()).isEqualTo(basePath4);
+ assertThat(repoCfg.getBasePath(new Project.NameKey("someProject")).toString())
+ .isEqualTo(basePath4);
}
@Test
diff --git a/javatests/com/google/gerrit/server/permissions/RefControlTest.java b/javatests/com/google/gerrit/server/permissions/RefControlTest.java
index a3f9f93..1d2d04c 100644
--- a/javatests/com/google/gerrit/server/permissions/RefControlTest.java
+++ b/javatests/com/google/gerrit/server/permissions/RefControlTest.java
@@ -206,6 +206,7 @@
@Inject private DefaultRefFilter.Factory refFilterFactory;
@Inject private TransferConfig transferConfig;
@Inject private MetricMaker metricMaker;
+ @Inject private ProjectConfig.Factory projectConfigFactory;
@Before
public void setUp() throws Exception {
@@ -274,7 +275,8 @@
try {
Repository repo = repoManager.createRepository(allProjectsName);
- ProjectConfig allProjects = new ProjectConfig(new Project.NameKey(allProjectsName.get()));
+ ProjectConfig allProjects =
+ projectConfigFactory.create(new Project.NameKey(allProjectsName.get()));
allProjects.load(repo);
LabelType cr = Util.codeReview();
allProjects.getLabelSections().put(cr.getName(), cr);
@@ -295,11 +297,11 @@
CacheBuilder.newBuilder().build();
sectionSorter = new PermissionCollection.Factory(new SectionSortCache(c), metricMaker);
- parent = new ProjectConfig(parentKey);
+ parent = projectConfigFactory.create(parentKey);
parent.load(newRepository(parentKey));
add(parent);
- local = new ProjectConfig(localKey);
+ local = projectConfigFactory.create(localKey);
local.load(newRepository(localKey));
add(local);
local.getProject().setParentName(parentKey);
@@ -455,7 +457,7 @@
allow(local, READ, DEVS, "refs/heads/*");
assertCanAccess(user(local, "a", ADMIN));
- local = new ProjectConfig(localKey);
+ local = projectConfigFactory.create(localKey);
local.load(newRepository(localKey));
local.getProject().setParentName(parentKey);
allow(local, READ, DEVS, "refs/*");
diff --git a/javatests/com/google/gerrit/server/project/CommitsCollectionTest.java b/javatests/com/google/gerrit/server/project/CommitsCollectionTest.java
index b39208a..2ae9e90 100644
--- a/javatests/com/google/gerrit/server/project/CommitsCollectionTest.java
+++ b/javatests/com/google/gerrit/server/project/CommitsCollectionTest.java
@@ -57,6 +57,7 @@
@Inject protected MetaDataUpdate.Server metaDataUpdateFactory;
@Inject protected AllProjectsName allProjects;
@Inject private CommitsCollection commits;
+ @Inject private ProjectConfig.Factory projectConfigFactory;
private TestRepository<InMemoryRepository> repo;
private ProjectConfig project;
@@ -70,7 +71,7 @@
Project.NameKey name = new Project.NameKey("project");
InMemoryRepository inMemoryRepo = repoManager.createRepository(name);
- project = new ProjectConfig(name);
+ project = projectConfigFactory.create(name);
project.load(inMemoryRepo);
repo = new TestRepository<>(inMemoryRepo);
}
diff --git a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
index 0e4ba10..764d49a 100644
--- a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
+++ b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
@@ -78,11 +78,13 @@
new GroupReference(new AccountGroup.UUID("X"), "Developers");
private final GroupReference staff = new GroupReference(new AccountGroup.UUID("Y"), "Staff");
+ private ProjectConfig.Factory factory;
private Repository db;
private TestRepository<?> tr;
@Before
public void setUp() throws Exception {
+ factory = new ProjectConfig.Factory();
db = new InMemoryRepository(new DfsRepositoryDescription("repo"));
tr = new TestRepository<>(db);
}
@@ -104,6 +106,13 @@
+ " sameGroupVisibility = block group Staff\n"
+ "[contributor-agreement \"Individual\"]\n"
+ " description = A simple description\n"
+ + " matchProjects = ^/ourproject\n"
+ + " matchProjects = ^/ourotherproject\n"
+ + " matchProjects = ^/someotherroot/ourproject\n"
+ + " excludeProjects = ^/theirproject\n"
+ + " excludeProjects = ^/theirotherproject\n"
+ + " excludeProjects = ^/someotherroot/theirproject\n"
+ + " excludeProjects = ^/someotherroot/theirotherproject\n"
+ " accepted = group Developers\n"
+ " accepted = group Staff\n"
+ " autoVerify = group Developers\n"
@@ -115,6 +124,14 @@
ContributorAgreement ca = cfg.getContributorAgreement("Individual");
assertThat(ca.getName()).isEqualTo("Individual");
assertThat(ca.getDescription()).isEqualTo("A simple description");
+ assertThat(ca.getMatchProjectsRegexes())
+ .containsExactly("^/ourproject", "^/ourotherproject", "^/someotherroot/ourproject");
+ assertThat(ca.getExcludeProjectsRegexes())
+ .containsExactly(
+ "^/theirproject",
+ "^/theirotherproject",
+ "^/someotherroot/theirproject",
+ "^/someotherroot/theirotherproject");
assertThat(ca.getAgreementUrl()).isEqualTo("http://www.example.com/agree");
assertThat(ca.getAccepted()).hasSize(2);
assertThat(ca.getAccepted().get(0).getGroup()).isEqualTo(developers);
@@ -256,6 +273,7 @@
+ " sameGroupVisibility = block group Staff\n"
+ "[contributor-agreement \"Individual\"]\n"
+ " description = A simple description\n"
+ + " matchProjects = ^/ourproject\n"
+ " accepted = group Developers\n"
+ " autoVerify = group Developers\n"
+ " agreementUrl = http://www.example.com/agree\n"
@@ -273,6 +291,8 @@
ContributorAgreement ca = cfg.getContributorAgreement("Individual");
ca.setAccepted(Collections.singletonList(new PermissionRule(cfg.resolve(staff))));
ca.setAutoVerify(null);
+ ca.setMatchProjectsRegexes(null);
+ ca.setExcludeProjectsRegexes(Collections.singletonList("^/theirproject"));
ca.setDescription("A new description");
rev = commit(cfg);
assertThat(text(rev, "project.config"))
@@ -289,6 +309,7 @@
+ " description = A new description\n"
+ " accepted = group Staff\n"
+ " agreementUrl = http://www.example.com/agree\n"
+ + "\texcludeProjects = ^/theirproject\n"
+ "[label \"CustomLabel\"]\n"
+ LABEL_SCORES_CONFIG
+ "\tfunction = MaxWithBlock\n" // label gets this function when it is created
@@ -385,7 +406,7 @@
@Test
public void readUnexistingPluginConfig() throws Exception {
- ProjectConfig cfg = new ProjectConfig(new Project.NameKey("test"));
+ ProjectConfig cfg = factory.create(new Project.NameKey("test"));
cfg.load(db);
PluginConfig pluginCfg = cfg.getPluginConfig("somePlugin");
assertThat(pluginCfg.getNames()).isEmpty();
@@ -573,7 +594,7 @@
}
private ProjectConfig read(RevCommit rev) throws IOException, ConfigInvalidException {
- ProjectConfig cfg = new ProjectConfig(new Project.NameKey("test"));
+ ProjectConfig cfg = factory.create(new Project.NameKey("test"));
cfg.load(db, rev);
return cfg;
}
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index b9973e9..f362e5a 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -2378,6 +2378,12 @@
cd.reviewers();
cd.unresolvedCommentCount();
+ if (getSchemaVersion() < 51) {
+ assertMissingField(ChangeField.TOTAL_COMMENT_COUNT);
+ } else {
+ cd.totalCommentCount();
+ }
+
// TODO(dborowitz): Swap out GitRepositoryManager somehow? Will probably be
// necessary for NoteDb anyway.
cd.isMergeable();
diff --git a/javatests/com/google/gerrit/server/schema/SchemaCreatorTest.java b/javatests/com/google/gerrit/server/schema/SchemaCreatorTest.java
index d3f69982..9cb9333 100644
--- a/javatests/com/google/gerrit/server/schema/SchemaCreatorTest.java
+++ b/javatests/com/google/gerrit/server/schema/SchemaCreatorTest.java
@@ -50,6 +50,8 @@
@Inject private InMemoryDatabase db;
+ @Inject private ProjectConfig.Factory projectConfigFactory;
+
@Before
public void setUp() throws Exception {
new InMemoryModule().inject(this);
@@ -85,7 +87,7 @@
private LabelTypes getLabelTypes() throws Exception {
db.create();
- ProjectConfig c = new ProjectConfig(allProjects);
+ ProjectConfig c = projectConfigFactory.create(allProjects);
try (Repository repo = repoManager.openRepository(allProjects)) {
c.load(repo);
return new LabelTypes(ImmutableList.copyOf(c.getLabelSections().values()));
diff --git a/javatests/com/google/gerrit/server/schema/Schema_161_to_162_Test.java b/javatests/com/google/gerrit/server/schema/Schema_161_to_162_Test.java
index 67d071d..10aabe8 100644
--- a/javatests/com/google/gerrit/server/schema/Schema_161_to_162_Test.java
+++ b/javatests/com/google/gerrit/server/schema/Schema_161_to_162_Test.java
@@ -47,6 +47,7 @@
@Inject private GitRepositoryManager repoManager;
@Inject private Schema_162 schema162;
@Inject private ReviewDb db;
+ @Inject private ProjectConfig.Factory projectConfigFactory;
@Inject @GerritPersonIdent private PersonIdent serverUser;
@Test
@@ -73,7 +74,7 @@
try (Repository git = repoManager.openRepository(allUsersName);
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, git)) {
- ProjectConfig cfg = ProjectConfig.read(md);
+ ProjectConfig cfg = projectConfigFactory.read(md);
cfg.getProject().setParentName(testProject);
md.getCommitBuilder().setCommitter(serverUser);
md.getCommitBuilder().setAuthor(serverUser);
diff --git a/plugins/replication b/plugins/replication
index 092792e..a7b900b 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 092792edacf9c29732a560a30967b92664cd65f9
+Subproject commit a7b900bd524c333c8ca2825e37fa781ac055ac36
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
index 372e6be..2235ae1 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
@@ -55,14 +55,14 @@
</tr>
<template is="dom-repeat" items="[[sections]]" as="changeSection"
index-as="sectionIndex">
- <template is="dom-if" if="[[changeSection.sectionName]]">
+ <template is="dom-if" if="[[changeSection.name]]">
<tr class="groupHeader">
<td class="leftPadding"></td>
<td class="star" hidden$="[[!showStar]]" hidden></td>
<td class="cell"
colspan$="[[_computeColspan(changeTableColumns, labelNames)]]">
<a href$="[[_sectionHref(changeSection.query)]]">
- [[changeSection.sectionName]]
+ [[changeSection.name]]
</a>
</td>
</tr>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
index de97e62..708a730 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
@@ -63,7 +63,7 @@
* properties should not be used together.
*
* @type {!Array<{
- * sectionName: string,
+ * name: string,
* query: string,
* results: !Array<!Object>
* }>}
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js
index 775c046..3fc4089 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js
@@ -207,7 +207,7 @@
this._showNewUserHelp = lastResultSet.length == 0;
}
this._results = changes.map((results, i) => ({
- sectionName: res.sections[i].name,
+ name: res.sections[i].name,
query: res.sections[i].query,
results,
isOutgoing: res.sections[i].isOutgoing,
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.html b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.html
index de74218..78fdee6 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.html
@@ -279,7 +279,7 @@
return element._fetchDashboardChanges({sections}, false).then(() => {
assert.equal(element._results.length, 1);
- assert.equal(element._results[0].sectionName, 'test2');
+ assert.equal(element._results[0].name, 'test2');
});
});
diff --git a/polygerrit-ui/app/elements/change-list/gr-embed-dashboard/gr-embed-dashboard.html b/polygerrit-ui/app/elements/change-list/gr-embed-dashboard/gr-embed-dashboard.html
new file mode 100644
index 0000000..2394e24
--- /dev/null
+++ b/polygerrit-ui/app/elements/change-list/gr-embed-dashboard/gr-embed-dashboard.html
@@ -0,0 +1,41 @@
+<!--
+@license
+Copyright (C) 2018 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.
+-->
+
+<link rel="import" href="../../../bower_components/polymer/polymer.html">
+
+<link rel="import" href="../../change-list/gr-change-list/gr-change-list.html">
+<link rel="import" href="../gr-create-change-help/gr-create-change-help.html">
+
+<dom-module id="gr-embed-dashboard">
+ <template>
+ <gr-change-list
+ show-star
+ account="[[account]]"
+ preferences="[[preferences]]"
+ sections="[[sections]]">
+ <div id="emptyOutgoing" slot="empty-outgoing">
+ <template is="dom-if" if="[[showNewUserHelp]]">
+ <gr-create-change-help></gr-create-change-help>
+ </template>
+ <template is="dom-if" if="[[!showNewUserHelp]]">
+ No changes
+ </template>
+ </div>
+ </gr-change-list>
+ </template>
+ <script src="gr-embed-dashboard.js"></script>
+</dom-module>
diff --git a/polygerrit-ui/app/elements/change-list/gr-embed-dashboard/gr-embed-dashboard.js b/polygerrit-ui/app/elements/change-list/gr-embed-dashboard/gr-embed-dashboard.js
new file mode 100644
index 0000000..e31f9ad
--- /dev/null
+++ b/polygerrit-ui/app/elements/change-list/gr-embed-dashboard/gr-embed-dashboard.js
@@ -0,0 +1,29 @@
+/**
+ * @license
+ * Copyright (C) 2018 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.
+ */
+(function() {
+ 'use strict';
+
+ Polymer({
+ is: 'gr-embed-dashboard',
+ properties: {
+ account: Object,
+ sections: Array,
+ preferences: Object,
+ showNewUserHelp: Boolean,
+ },
+ });
+})();
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 3f5bd13..148cd34 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -587,6 +587,7 @@
change-comments="[[_changeComments]]"
project-name="[[_change.project]]"
show-reply-buttons="[[_loggedIn]]"
+ on-message-anchor-tap="_handleMessageAnchorTap"
on-reply="_handleMessageReply"></gr-messages-list>
</template>
<template is="dom-if" if="[[!_showMessagesView]]">
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 42aae9c..a905c9f 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -44,6 +44,8 @@
const TRAILING_WHITESPACE_REGEX = /[ \t]+$/gm;
+ const MSG_PREFIX = '#message-';
+
const ReloadToastMessage = {
NEWER_REVISION: 'A newer patch set has been uploaded',
RESTORED: 'This change has been restored',
@@ -720,10 +722,17 @@
this.viewState.numFilesShown = numFilesShown;
},
+ _handleMessageAnchorTap(e) {
+ const hash = MSG_PREFIX + e.detail.id;
+ const url = Gerrit.Nav.getUrlForChange(this._change,
+ this._patchRange.patchNum, this._patchRange.basePatchNum,
+ this._editMode, hash);
+ history.replaceState(null, '', url);
+ },
+
_maybeScrollToMessage(hash) {
- const msgPrefix = '#message-';
- if (hash.startsWith(msgPrefix)) {
- this.messagesList.scrollToMessage(hash.substr(msgPrefix.length));
+ if (hash.startsWith(MSG_PREFIX)) {
+ this.messagesList.scrollToMessage(hash.substr(MSG_PREFIX.length));
}
},
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index ef0a376..4b6cc6c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -94,6 +94,20 @@
return element.getComputedStyleValue(cssParam);
};
+ test('_handleMessageAnchorTap', () => {
+ element._changeNum = '1';
+ element._patchRange = {
+ basePatchNum: 'PARENT',
+ patchNum: 1,
+ };
+ const getUrlStub = sandbox.stub(Gerrit.Nav, 'getUrlForChange');
+ const replaceStateStub = sandbox.stub(history, 'replaceState');
+ element._handleMessageAnchorTap({detail: {id: 'a12345'}});
+
+ assert.equal(getUrlStub.lastCall.args[4], '#message-a12345');
+ assert.isTrue(replaceStateStub.called);
+ });
+
suite('keyboard shortcuts', () => {
test('S should toggle the CL star', () => {
const starStub = sandbox.stub(element.$.changeStar, 'toggleStar');
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 358e994..fa9ec7f 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -400,7 +400,6 @@
path="[[file.__path]]"
prefs="[[diffPrefs]]"
project-name="[[change.project]]"
- project-config="[[projectConfig]]"
on-line-selected="_onLineSelected"
no-render-on-prefs-change
view-mode="[[diffViewMode]]"></gr-diff-host>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index df92b1e..51f0e5f 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -1358,7 +1358,7 @@
id: '503008e2_0ab203ee',
line: 10,
updated: '2018-02-14 22:07:43.000000000',
- message: 'response',
+ message: 'a comment',
unresolved: true,
},
{
@@ -1367,7 +1367,7 @@
line: 20,
in_reply_to: 'ecf0b9fa_fe1a5f62',
updated: '2018-02-13 22:07:43.000000000',
- message: 'a comments',
+ message: 'response',
unresolved: true,
},
];
@@ -1704,11 +1704,43 @@
});
test('reloadCommentsForThreadWithRootId', () => {
+ // Expand the commit message diff
+ MockInteractions.keyUpOn(element, 73, 'shift', 'i');
+ const diffs = renderAndGetNewDiffs(0);
+ flushAsynchronousOperations();
+
+ // Two comment threads should be generated by renderAndGetNewDiffs
+ const threadEls = diffs[0].getThreadEls();
+ assert.equal(threadEls.length, 2);
+ const threadElsByRootId = new Map(
+ threadEls.map(threadEl => [threadEl.rootId, threadEl]));
+
+ const thread1 = threadElsByRootId.get('503008e2_0ab203ee');
+ assert.equal(thread1.comments.length, 1);
+ assert.equal(thread1.comments[0].message, 'a comment');
+ assert.equal(thread1.comments[0].line, 10);
+
+ const thread2 = threadElsByRootId.get('ecf0b9fa_fe1a5f62');
+ assert.equal(thread2.comments.length, 2);
+ assert.isTrue(thread2.comments[0].unresolved);
+ assert.equal(thread2.comments[0].message, 'another comment');
+ assert.equal(thread2.comments[0].line, 20);
+
const commentStub =
sandbox.stub(element.changeComments, 'getCommentsForThread');
const commentStubRes1 = [
{
patch_set: 2,
+ id: '503008e2_0ab203ee',
+ line: 20,
+ updated: '2018-02-08 18:49:18.000000000',
+ message: 'edited text',
+ unresolved: false,
+ },
+ ];
+ const commentStubRes2 = [
+ {
+ patch_set: 2,
id: 'ecf0b9fa_fe1a5f62',
line: 20,
updated: '2018-02-08 18:49:18.000000000',
@@ -1719,6 +1751,7 @@
patch_set: 2,
id: '503008e2_0ab203ee',
line: 10,
+ in_reply_to: 'ecf0b9fa_fe1a5f62',
updated: '2018-02-14 22:07:43.000000000',
message: 'response',
unresolved: true,
@@ -1727,57 +1760,35 @@
patch_set: 2,
id: '503008e2_0ab203ef',
line: 20,
- in_reply_to: 'ecf0b9fa_fe1a5f62',
+ in_reply_to: '503008e2_0ab203ee',
updated: '2018-02-15 22:07:43.000000000',
message: 'a third comment in the thread',
unresolved: true,
},
];
- const commentStubRes2 = [
- {
- patch_set: 2,
- id: 'ecf0b9fa_fe1a5f62',
- line: 20,
- updated: '2018-02-08 18:49:18.000000000',
- message: 'edited text',
- unresolved: false,
- },
- ];
- commentStub.withArgs('cc788d2c_cb1d728c').returns(
+ commentStub.withArgs('503008e2_0ab203ee').returns(
commentStubRes1);
commentStub.withArgs('ecf0b9fa_fe1a5f62').returns(
commentStubRes2);
- // Expand the commit message diff
- MockInteractions.keyUpOn(element, 73, 'shift', 'i');
- const diffs = renderAndGetNewDiffs(0);
- flushAsynchronousOperations();
-
- // Two comment threads sould be generated
- const commentThreadEls = diffs[0].getThreadEls();
- assert(commentThreadEls[0].comments.length, 2);
- assert(commentThreadEls[1].comments.length, 1);
- assert.isTrue(commentThreadEls[1].comments[0].unresolved);
- assert.equal(commentThreadEls[1].comments[0].message, 'another comment');
-
- // Reload comments from the first comment thread, which should have a new
- // reply.
- element.reloadCommentsForThreadWithRootId('cc788d2c_cb1d728c',
- '/COMMIT_MSG');
- assert(commentThreadEls[0].comments.length, 3);
-
// Reload comments from the first comment thread, which should have a
// an updated message and a toggled resolve state.
+ element.reloadCommentsForThreadWithRootId('503008e2_0ab203ee',
+ '/COMMIT_MSG');
+ assert.equal(thread1.comments.length, 1);
+ assert.isFalse(thread1.comments[0].unresolved);
+ assert.equal(thread1.comments[0].message, 'edited text');
+
+ // Reload comments from the second comment thread, which should have a new
+ // reply.
element.reloadCommentsForThreadWithRootId('ecf0b9fa_fe1a5f62',
'/COMMIT_MSG');
- assert(commentThreadEls[1].comments.length, 1);
- assert.isFalse(commentThreadEls[1].comments[0].unresolved);
- assert.equal(commentThreadEls[1].comments[0].message, 'edited text');
+ assert.equal(thread2.comments.length, 3);
const commentStubCount = commentStub.callCount;
const getThreadsSpy = sandbox.spy(diffs[0], 'getThreadEls');
- // Should not be getting threadss when the file is not expanded.
+ // Should not be getting threads when the file is not expanded.
element.reloadCommentsForThreadWithRootId('ecf0b9fa_fe1a5f62',
'other/file');
assert.isFalse(getThreadsSpy.called);
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 0682ab2..f472331 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
@@ -69,7 +69,8 @@
},
_computeBlankItems(permittedLabels, label, side) {
- if (!permittedLabels || !permittedLabels[label] || !this.labelValues ||
+ if (!permittedLabels || !permittedLabels[label] ||
+ !permittedLabels[label].length || !this.labelValues ||
!Object.keys(this.labelValues).length) {
return [];
}
@@ -135,7 +136,8 @@
},
_computeAnyPermittedLabelValues(permittedLabels, label) {
- return permittedLabels.hasOwnProperty(label);
+ return permittedLabels.hasOwnProperty(label) &&
+ permittedLabels[label].length;
},
_computeHiddenClass(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 e5431f6..1e4d471 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
@@ -258,6 +258,11 @@
flushAsynchronousOperations();
assert.isOk(element.$$('iron-selector'));
assert.isTrue(element.$$('iron-selector').hidden);
+
+ element.permittedLabels = {Verified: []};
+ flushAsynchronousOperations();
+ assert.isOk(element.$$('iron-selector'));
+ assert.isTrue(element.$$('iron-selector').hidden);
});
test('asymetrical labels', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.html b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
index 19b0716b..32c4c1f 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
@@ -132,9 +132,12 @@
right: var(--default-horizontal-margin);
top: 10px;
}
- .date {
+ span.date {
color: var(--deemphasized-text-color);
}
+ span.date:hover {
+ text-decoration: underline;
+ }
.dateContainer iron-icon {
cursor: pointer;
}
@@ -227,12 +230,12 @@
</span>
</template>
<template is="dom-if" if="[[message.id]]">
- <a class="date" href$="[[_computeMessageHash(message)]]" on-tap="_handleLinkTap">
+ <span class="date" on-tap="_handleAnchorTap">
<gr-date-formatter
has-tooltip
show-date-and-time
date-str="[[message.date]]"></gr-date-formatter>
- </a>
+ </span>
</template>
<iron-icon
id="expandToggle"
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.js b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
index 0590c73..8ecd6b0 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
@@ -24,17 +24,17 @@
is: 'gr-message',
/**
- * Fired when this message's permalink is tapped.
- *
- * @event scroll-to
- */
-
- /**
* Fired when this message's reply link is tapped.
*
* @event reply
*/
+ /**
+ * Fired when the message's timestamp is tapped.
+ *
+ * @event message-anchor-tap
+ */
+
listeners: {
tap: '_handleTap',
},
@@ -223,22 +223,12 @@
return classes.join(' ');
},
- _computeMessageHash(message) {
- return '#message-' + message.id;
- },
-
- _handleLinkTap(e) {
+ _handleAnchorTap(e) {
e.preventDefault();
-
- this.fire('scroll-to', {message: this.message}, {bubbles: false});
-
- const hash = this._computeMessageHash(this.message);
- // Don't add the hash to the window history if it's already there.
- // Otherwise you mess up expected back button behavior.
- if (window.location.hash == hash) { return; }
- // Change the URL but don’t trigger a nav event. Otherwise it will
- // reload the page.
- page.show(window.location.pathname + hash, null, false);
+ this.dispatchEvent(new CustomEvent('message-anchor-tap', {
+ bubbles: true,
+ detail: {id: this.message.id},
+ }));
},
_handleReplyTap(e) {
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
index 870f366..64a5b26 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
@@ -164,6 +164,24 @@
});
});
+ test('clicking on date link fires event', () => {
+ element.message = {
+ type: 'REVIEWER_UPDATE',
+ updated: '2016-01-12 20:24:49.448000000',
+ reviewer: {},
+ id: '47c43261_55aa2c41',
+ };
+ flushAsynchronousOperations();
+ const stub = sinon.stub();
+ element.addEventListener('message-anchor-tap', stub);
+ const dateEl = element.$$('.date');
+ assert.ok(dateEl);
+ MockInteractions.tap(dateEl);
+
+ assert.isTrue(stub.called);
+ assert.deepEqual(stub.lastCall.args[0].detail, {id: element.message.id});
+ });
+
test('votes', () => {
element.message = {
author: {},
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
index 0a7dacc..80708a1 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
@@ -109,7 +109,7 @@
hide-automated="[[_hideAutomated]]"
project-name="[[projectName]]"
show-reply-button="[[showReplyButtons]]"
- on-scroll-to="_handleScrollTo"
+ on-message-anchor-tap="_handleAnchorTap"
label-extremes="[[_labelExtremes]]"
data-message-id$="[[message.id]]"></gr-message>
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
index 89a3523..023431c 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
@@ -184,8 +184,8 @@
this.handleExpandCollapse(!this._expanded);
},
- _handleScrollTo(e) {
- this.scrollToMessage(e.detail.message.id);
+ _handleAnchorTap(e) {
+ this.scrollToMessage(e.detail.id);
},
_hasAutomatedMessages(messages) {
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js
index ff5b290..f2cfe87 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js
@@ -290,14 +290,14 @@
_resultsChanged(related, submittedTogether, conflicts,
cherryPicks, sameTopic) {
const results = [
- related,
- submittedTogether,
+ related && related.changes,
+ submittedTogether && submittedTogether.changes,
conflicts,
cherryPicks,
sameTopic,
];
for (let i = 0; i < results.length; i++) {
- if (results[i].length > 0) {
+ if (results[i] && results[i].length > 0) {
this.hidden = false;
this.fire('update', null, {bubbles: false});
return;
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.html b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.html
index bfeb694..48cc565 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.html
@@ -382,7 +382,7 @@
true);
});
- test('clear and empties', () => {
+ suite('hidden attribute and update event', () => {
const changes = [{
project: 'foo/bar',
change_id: 'Ideadbeef',
@@ -397,33 +397,68 @@
_current_revision_number: 1,
status: 'NEW',
}];
- element._relatedResponse = {changes};
- element._submittedTogether = {changes};
- element._conflicts = changes;
- element._cherryPicks = changes;
- element._sameTopic = changes;
- element.hidden = false;
- element.clear();
- assert.isTrue(element.hidden);
- assert.equal(element._relatedResponse.changes.length, 0);
- assert.equal(element._submittedTogether.changes.length, 0);
- assert.equal(element._conflicts.length, 0);
- assert.equal(element._cherryPicks.length, 0);
- assert.equal(element._sameTopic.length, 0);
- });
+ test('clear and empties', () => {
+ element._relatedResponse = {changes};
+ element._submittedTogether = {changes};
+ element._conflicts = changes;
+ element._cherryPicks = changes;
+ element._sameTopic = changes;
- test('update fires', () => {
- const updateHandler = sandbox.stub();
- element.addEventListener('update', updateHandler);
+ element.hidden = false;
+ element.clear();
+ assert.isTrue(element.hidden);
+ assert.equal(element._relatedResponse.changes.length, 0);
+ assert.equal(element._submittedTogether.changes.length, 0);
+ assert.equal(element._conflicts.length, 0);
+ assert.equal(element._cherryPicks.length, 0);
+ assert.equal(element._sameTopic.length, 0);
+ });
- element._resultsChanged([], [], [], [], []);
- assert.isTrue(element.hidden);
- assert.isFalse(updateHandler.called);
+ test('update fires', () => {
+ const updateHandler = sandbox.stub();
+ element.addEventListener('update', updateHandler);
- element._resultsChanged([], [], [], [], ['test']);
- assert.isFalse(element.hidden);
- assert.isTrue(updateHandler.called);
+ element._resultsChanged({}, {}, [], [], []);
+ assert.isTrue(element.hidden);
+ assert.isFalse(updateHandler.called);
+
+ element._resultsChanged({}, {}, [], [], ['test']);
+ assert.isFalse(element.hidden);
+ assert.isTrue(updateHandler.called);
+ });
+
+ suite('hiding and unhiding', () => {
+ test('related response', () => {
+ assert.isTrue(element.hidden);
+ element._resultsChanged({changes}, {}, [], [], []);
+ assert.isFalse(element.hidden);
+ });
+
+ test('submitted together', () => {
+ assert.isTrue(element.hidden);
+ element._resultsChanged({}, {changes}, [], [], []);
+ assert.isFalse(element.hidden);
+ });
+
+ test('conflicts', () => {
+ assert.isTrue(element.hidden);
+ element._resultsChanged({}, {}, changes, [], []);
+ assert.isFalse(element.hidden);
+ });
+
+ test('cherrypicks', () => {
+ assert.isTrue(element.hidden);
+ element._resultsChanged({}, {}, [], changes, []);
+ assert.isFalse(element.hidden);
+ });
+
+ test('same topic', () => {
+ assert.isTrue(element.hidden);
+ element._resultsChanged({}, {}, [], [], changes);
+ assert.isFalse(element.hidden);
+ });
+ });
});
test('_computeChangeURL uses Gerrit.Nav', () => {
diff --git a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html
index bc31b60..5be259c 100644
--- a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html
+++ b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html
@@ -33,6 +33,8 @@
// also be provided.
// - `edit`, optional, Boolean: whether or not to load the file list with
// edit controls.
+ // - `messageHash`, optional, String: the hash of the change message to
+ // scroll to.
//
// - Gerrit.Nav.View.SEARCH:
// - `query`, optional, String: the literal search query. If provided,
@@ -352,9 +354,11 @@
* @param {number|string=} opt_basePatchNum The string 'PARENT' can be
* used for none.
* @param {boolean=} opt_isEdit
+ * @param {string=} opt_messageHash
* @return {string}
*/
- getUrlForChange(change, opt_patchNum, opt_basePatchNum, opt_isEdit) {
+ getUrlForChange(change, opt_patchNum, opt_basePatchNum, opt_isEdit,
+ opt_messageHash) {
if (opt_basePatchNum === PARENT_PATCHNUM) {
opt_basePatchNum = undefined;
}
@@ -368,6 +372,7 @@
basePatchNum: opt_basePatchNum,
edit: opt_isEdit,
host: change.internalHost || undefined,
+ messageHash: opt_messageHash,
});
},
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 bdd0942..2ea7e37 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -395,6 +395,9 @@
} else if (params.edit) {
suffix += ',edit';
}
+ if (params.messageHash) {
+ suffix += params.messageHash;
+ }
if (params.project) {
const encodedProject = this.encodeURL(params.project, true);
return `/c/${encodedProject}/+/${params.changeNum}${suffix}`;
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 781e25b..7d4f9ba 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
@@ -281,6 +281,9 @@
paramsWithQuery.basePatchNum = 5;
assert.equal(element._generateUrl(paramsWithQuery),
'/c/test/+/1234/5..10?revert&foo=bar');
+
+ params.messageHash = '#123';
+ assert.equal(element._generateUrl(params), '/c/test/+/1234/5..10#123');
});
test('change with repo name encoding', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index cc66e3b..0997fee 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -114,7 +114,6 @@
baseImage: Object,
revisionImage: Object,
projectName: String,
- parentIndex: Number,
/**
* @type {Defs.LineOfInterest|null}
*/
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index 4b85c22..be53fda 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -67,6 +67,16 @@
layer.addListener(this._handleLayerUpdate.bind(this));
}
}
+
+ const allComments = [];
+ for (const side of [GrDiffBuilder.Side.LEFT, GrDiffBuilder.Side.RIGHT]) {
+ // This is needed by the threading.
+ for (const comment of this._comments[side]) {
+ comment.__commentSide = side;
+ }
+ allComments.push(...this._comments[side]);
+ }
+ this._threads = this._createThreads(allComments);
}
GrDiffBuilder.GroupType = {
@@ -319,44 +329,51 @@
return button;
};
- GrDiffBuilder.prototype._getCommentsForLine = function(comments, line,
- opt_side) {
- function byLineNum(lineNum) {
- return function(c) {
- return (c.line === lineNum) ||
- (c.line === undefined && lineNum === GrDiffLine.FILE);
- };
+ /**
+ * @param {!Array<Object>} threads
+ * @param {!GrDiffLine} line
+ * @param {!GrDiffBuilder.Side=} side The side (LEFT, RIGHT, BOTH) for which
+ * to return the threads (default: BOTH).
+ */
+ GrDiffBuilder.prototype._filterThreadsForLine = function(
+ threads, line, side = GrDiffBuilder.Side.BOTH) {
+ function matchesLeftLine(thread) {
+ return thread.commentSide == GrDiffBuilder.Side.LEFT &&
+ thread.comments[0].line == line.beforeNumber;
}
- const leftComments =
- comments[GrDiffBuilder.Side.LEFT].filter(byLineNum(line.beforeNumber));
- const rightComments =
- comments[GrDiffBuilder.Side.RIGHT].filter(byLineNum(line.afterNumber));
-
- leftComments.forEach(c => { c.__commentSide = 'left'; });
- rightComments.forEach(c => { c.__commentSide = 'right'; });
-
- let result;
-
- switch (opt_side) {
- case GrDiffBuilder.Side.LEFT:
- result = leftComments;
- break;
- case GrDiffBuilder.Side.RIGHT:
- result = rightComments;
- break;
- default:
- result = leftComments.concat(rightComments);
- break;
+ function matchesRightLine(thread) {
+ return thread.commentSide == GrDiffBuilder.Side.RIGHT &&
+ thread.comments[0].line == line.afterNumber;
+ }
+ function matchesFileComment(thread) {
+ return (side === GrDiffBuilder.Side.BOTH ||
+ thread.commentSide == side) &&
+ // line/range comments have 1-based line set, if line is falsy it's
+ // a file comment
+ !thread.comments[0].line;
}
- return result;
+ // Select the appropriate matchers for the desired side and line
+ // If side is BOTH, we want both the left and right matcher.
+ const matchers = [];
+ if (side !== GrDiffBuilder.Side.RIGHT) {
+ matchers.push(matchesLeftLine);
+ }
+ if (side !== GrDiffBuilder.Side.LEFT) {
+ matchers.push(matchesRightLine);
+ }
+ if (line.afterNumber === GrDiffLine.FILE ||
+ line.beforeNumber === GrDiffLine.FILE) {
+ matchers.push(matchesFileComment);
+ }
+
+ return threads.filter(thread => matchers.find(matcher => matcher(thread)));
};
/**
* @param {Array<Object>} comments
- * @param {string} patchForNewThreads
*/
- GrDiffBuilder.prototype._getThreads = function(comments, patchForNewThreads) {
+ GrDiffBuilder.prototype._createThreads = function(comments) {
const sortedComments = comments.slice(0).sort((a, b) => {
if (b.__draft && !a.__draft ) { return 0; }
if (a.__draft && !b.__draft ) { return 1; }
@@ -381,13 +398,7 @@
start_datetime: comment.updated,
comments: [comment],
commentSide: comment.__commentSide,
- /**
- * Determines what the patchNum of a thread should be. Use patchNum from
- * comment if it exists, otherwise the property of the thread group.
- * This is needed for switching between side-by-side and unified views
- * when there are unsaved drafts.
- */
- patchNum: comment.patch_set || patchForNewThreads,
+ patchNum: comment.patch_set,
rootId: comment.id || comment.__draftID,
};
if (comment.range) {
@@ -439,28 +450,30 @@
};
/**
- * @param {GrDiffLine} line
- * @param {string=} opt_side
+ * @param {!GrDiffLine} line
+ * @param {!GrDiffBuilder.Side=} side The side (LEFT, RIGHT, BOTH) for which to return
+ * the thread group (default: BOTH).
* @return {!Object}
*/
GrDiffBuilder.prototype._commentThreadGroupForLine = function(
- line, opt_side) {
- const comments =
- this._getCommentsForLine(this._comments, line, opt_side);
- if (!comments || comments.length === 0) {
+ line, side = GrDiffBuilder.Side.BOTH) {
+ const threads =
+ this._filterThreadsForLine(this._threads, line, side);
+ if (!threads || threads.length === 0) {
return null;
}
- const patchNum = this._determinePatchNumForNewThreads(
- this._comments.meta.patchRange, line, opt_side);
+ const patchRange = this._comments.meta.patchRange;
+ const patchNumForNewThread = this._determinePatchNumForNewThreads(
+ patchRange, line, side);
const isOnParent = this._determineIsOnParent(
- comments[0].side, this._comments.meta.patchRange, line, opt_side);
+ threads[0].side, patchRange, line, side);
- const threadGroupEl = this._createThreadGroupFn(patchNum, isOnParent,
- opt_side);
- threadGroupEl.threads = this._getThreads(comments, patchNum);
- if (opt_side) {
- threadGroupEl.setAttribute('data-side', opt_side);
+ const threadGroupEl = this._createThreadGroupFn(
+ patchNumForNewThread, isOnParent, side);
+ threadGroupEl.threads = threads;
+ if (side) {
+ threadGroupEl.setAttribute('data-side', side);
}
return threadGroupEl;
};
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index 3238cbc..80b45a4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -57,6 +57,7 @@
<script>
suite('gr-diff-builder tests', () => {
+ let prefs;
let element;
let builder;
let createThreadGroupFn;
@@ -70,7 +71,7 @@
getLoggedIn() { return Promise.resolve(false); },
getProjectConfig() { return Promise.resolve({}); },
});
- const prefs = {
+ prefs = {
line_length: 10,
show_tabs: true,
tab_size: 4,
@@ -84,8 +85,7 @@
teardown(() => { sandbox.restore(); });
- test('_getThreads', () => {
- const patchForNewThreads = 3;
+ test('_createThreads', () => {
const comments = [
{
id: 'sallys_confession',
@@ -108,7 +108,7 @@
},
];
- let expectedThreadGroups = [
+ const expectedThreadGroups = [
{
start_datetime: '2015-12-23 15:00:20.396000000',
commentSide: 'left',
@@ -124,8 +124,8 @@
__commentSide: 'left',
in_reply_to: 'sallys_confession',
}],
+ patchNum: undefined,
rootId: 'sallys_confession',
- patchNum: 3,
},
{
start_datetime: '2015-12-20 15:01:20.396000000',
@@ -139,17 +139,18 @@
updated: '2015-12-20 15:01:20.396000000',
},
],
+ patchNum: undefined,
rootId: 'new_draft',
- patchNum: 3,
},
];
assert.deepEqual(
- builder._getThreads(comments, patchForNewThreads),
+ builder._createThreads(comments),
expectedThreadGroups);
+ });
- // Patch num should get inherited from comment rather
- comments.push({
+ test('_createThreads inherits patchNum amd range', () => {
+ const comments = [{
id: 'betsys_confession',
message: 'i like you, jack',
updated: '2015-12-24 15:00:10.396000000',
@@ -159,29 +160,12 @@
end_line: 1,
end_character: 2,
},
+ patch_set: 5,
__commentSide: 'left',
- });
+ }];
expectedThreadGroups = [
{
- start_datetime: '2015-12-23 15:00:20.396000000',
- commentSide: 'left',
- comments: [{
- id: 'sallys_confession',
- message: 'i like you, jack',
- updated: '2015-12-23 15:00:20.396000000',
- __commentSide: 'left',
- }, {
- id: 'jacks_reply',
- in_reply_to: 'sallys_confession',
- message: 'i like you, too',
- updated: '2015-12-24 15:01:20.396000000',
- __commentSide: 'left',
- }],
- patchNum: 3,
- rootId: 'sallys_confession',
- },
- {
start_datetime: '2015-12-24 15:00:10.396000000',
commentSide: 'left',
comments: [{
@@ -194,9 +178,10 @@
end_line: 1,
end_character: 2,
},
+ patch_set: 5,
__commentSide: 'left',
}],
- patchNum: 3,
+ patchNum: 5,
rootId: 'betsys_confession',
range: {
start_line: 1,
@@ -205,25 +190,10 @@
end_character: 2,
},
},
- {
- start_datetime: '2015-12-20 15:01:20.396000000',
- commentSide: 'left',
- comments: [
- {
- id: 'new_draft',
- message: 'i do not like either of you',
- __commentSide: 'left',
- __draft: true,
- updated: '2015-12-20 15:01:20.396000000',
- },
- ],
- rootId: 'new_draft',
- patchNum: 3,
- },
];
assert.deepEqual(
- builder._getThreads(comments, patchForNewThreads),
+ builder._createThreads(comments),
expectedThreadGroups);
});
@@ -241,7 +211,7 @@
__commentSide: 'left',
},
];
- assert.equal(builder._getThreads(comments, '3').length, 2);
+ assert.equal(builder._createThreads(comments).length, 2);
});
test('_createElement classStr applies all classes', () => {
@@ -417,37 +387,78 @@
}
});
- test('comments', () => {
+ test('_filterThreadsForLine with no threads', () => {
const line = new GrDiffLine(GrDiffLine.Type.BOTH);
line.beforeNumber = 3;
line.afterNumber = 5;
- let comments = {left: [], right: []};
- assert.deepEqual(builder._getCommentsForLine(comments, line), []);
- assert.deepEqual(builder._getCommentsForLine(comments, line,
+ const threads = [];
+ assert.deepEqual(
+ builder._filterThreadsForLine(threads, line), []);
+ assert.deepEqual(builder._filterThreadsForLine(threads, line,
GrDiffBuilder.Side.LEFT), []);
- assert.deepEqual(builder._getCommentsForLine(comments, line,
+ assert.deepEqual(builder._filterThreadsForLine(threads, line,
GrDiffBuilder.Side.RIGHT), []);
+ });
- comments = {
- left: [
- {id: 'l3', line: 3},
- {id: 'l5', line: 5},
- ],
- right: [
- {id: 'r3', line: 3},
- {id: 'r5', line: 5},
- ],
+ test('_filterThreadsForLine for line comments', () => {
+ const line = new GrDiffLine(GrDiffLine.Type.BOTH);
+ line.beforeNumber = 3;
+ line.afterNumber = 5;
+
+ const l3 = {
+ comments: [{id: 'l3', line: 3}],
+ range: {end_line: 3},
+ commentSide: 'left',
};
- assert.deepEqual(builder._getCommentsForLine(comments, line),
- [{id: 'l3', line: 3, __commentSide: 'left'},
- {id: 'r5', line: 5, __commentSide: 'right'}]);
- assert.deepEqual(builder._getCommentsForLine(comments, line,
- GrDiffBuilder.Side.LEFT), [{id: 'l3', line: 3,
- __commentSide: 'left'}]);
- assert.deepEqual(builder._getCommentsForLine(comments, line,
- GrDiffBuilder.Side.RIGHT), [{id: 'r5', line: 5,
- __commentSide: 'right'}]);
+ const l5 = {
+ comments: [{id: 'l5', line: 5}],
+ range: {end_line: 5},
+ commentSide: 'left',
+ };
+ const r3 = {
+ comments: [{id: 'r3', line: 3}],
+ range: {end_line: 3},
+ commentSide: 'right',
+ };
+ const r5 = {
+ comments: [{id: 'r5', line: 5}],
+ range: {end_line: 5},
+ commentSide: 'right',
+ };
+
+ const threads = [l3, l5, r3, r5];
+ assert.deepEqual(builder._filterThreadsForLine(threads, line),
+ [l3, r5]);
+ assert.deepEqual(builder._filterThreadsForLine(threads, line,
+ GrDiffBuilder.Side.LEFT), [l3]);
+ assert.deepEqual(builder._filterThreadsForLine(threads, line,
+ GrDiffBuilder.Side.RIGHT), [r5]);
+ });
+
+ test('_filterThreadsForLine for file comments', () => {
+ const line = new GrDiffLine(GrDiffLine.Type.BOTH);
+ line.beforeNumber = GrDiffLine.FILE;
+ line.afterNumber = GrDiffLine.FILE;
+
+ const l = {
+ comments: [{id: 'l', line: undefined}],
+ commentSide: 'left',
+ };
+ const r = {
+ comments: [{id: 'r', line: undefined}],
+ commentSide: 'right',
+ };
+
+ const threads = [l, r];
+ assert.deepEqual(builder._filterThreadsForLine(threads, line),
+ [l, r]);
+ assert.deepEqual(builder._filterThreadsForLine(threads, line,
+ GrDiffBuilder.Side.BOTH), [l, r]);
+ assert.deepEqual(builder._filterThreadsForLine(threads, line,
+ GrDiffBuilder.Side.LEFT), [l]);
+ assert.deepEqual(builder._filterThreadsForLine(threads, line,
+ GrDiffBuilder.Side.RIGHT), [r]);
});
test('comment thread group creation', () => {
@@ -458,19 +469,20 @@
const r5 = {id: 'r5', line: 5, updated: '2016-08-09 00:42:32.000000000',
__commentSide: 'right'};
- builder._comments = {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: '3',
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [l3, l5],
- right: [r5],
- };
+ builder = new GrDiffBuilder(
+ {content: []}, {
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: '3',
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [l3, l5],
+ right: [r5],
+ }, createThreadGroupFn, prefs);
function threadForComment(c, patchNum) {
return {
@@ -488,7 +500,7 @@
assert.equal(createThreadGroupFn.lastCall.args[1], isOnParent);
assert.deepEqual(
threadGroupEl.threads,
- comments.map(c => threadForComment(c, patchNum)));
+ comments.map(c => threadForComment(c, undefined)));
}
let line = new GrDiffLine(GrDiffLine.Type.BOTH);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js
index ae45c93..23d0a58 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js
@@ -24,7 +24,6 @@
changeNum: String,
projectName: String,
patchForNewThreads: String,
- range: Object,
isOnParent: {
type: Boolean,
value: false,
@@ -105,29 +104,5 @@
a.endLine === b.endLine &&
a.endChar === b.endChar;
},
-
- _sortByDate(threadGroups) {
- if (!threadGroups.length) { return; }
- return threadGroups.sort((a, b) => {
- // If a comment is a draft, it doesn't have a start_datetime yet.
- // Assume it is newer than the comment it is being compared to.
- if (!a.start_datetime) {
- return 1;
- }
- if (!b.start_datetime) {
- return -1;
- }
- return util.parseDate(a.start_datetime) -
- util.parseDate(b.start_datetime);
- });
- },
-
- _calculateLocationRange(range, comment) {
- return 'range-' + range.start_line + '-' +
- range.start_character + '-' +
- range.end_line + '-' +
- range.end_character + '-' +
- comment.__commentSide;
- },
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html
index 1fb8136..81181b1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html
@@ -122,93 +122,29 @@
assert.deepEqual(element.getThread('left', range).comments.length, 1);
});
- test('_sortByDate', () => {
- let threadGroups = [
- {
- start_datetime: '2015-12-23 15:00:20.396000000',
- comments: [],
- locationRange: 'line',
- },
- {
- start_datetime: '2015-12-22 15:00:10.396000000',
- comments: [],
- locationRange: 'range-1-1-1-2',
- },
- ];
-
- let expectedResult = [
- {
- start_datetime: '2015-12-22 15:00:10.396000000',
- comments: [],
- locationRange: 'range-1-1-1-2',
- }, {
- start_datetime: '2015-12-23 15:00:20.396000000',
- comments: [],
- locationRange: 'line',
- },
- ];
-
- assert.deepEqual(element._sortByDate(threadGroups), expectedResult);
-
- // When a comment doesn't have a date, the one without the date should be
- // last.
- threadGroups = [
- {
- start_datetime: '2015-12-23 15:00:20.396000000',
- comments: [],
- locationRange: 'line',
- },
- {
- comments: [],
- locationRange: 'range-1-1-1-2',
- },
- ];
-
- expectedResult = [
- {
- start_datetime: '2015-12-23 15:00:20.396000000',
- comments: [],
- locationRange: 'line',
- },
- {
- comments: [],
- locationRange: 'range-1-1-1-2',
- },
- ];
-
- assert.deepEqual(element._sortByDate(threadGroups), expectedResult);
- });
-
- test('_calculateLocationRange', () => {
- const comment = {__commentSide: 'left'};
- const range = {
- start_line: 1,
- start_character: 2,
- end_line: 3,
- end_character: 4,
- };
- assert.equal(
- element._calculateLocationRange(range, comment),
- 'range-1-2-3-4-left');
- });
-
test('addNewThread', () => {
- const locationRange = 'range-1-2-3-4';
- element._threads = [{locationRange: 'line'}];
- element.addNewThread(locationRange);
- assert(element._threads.length, 2);
+ const commentSide = 'left';
+ const range = {startLine: 1, endLine: 2, startChar: 3, endChar: 4};
+ element.patchForNewThreads = 5;
+ element.addNewThread(commentSide, range);
+ assert.equal(element.threads.length, 1);
+ assert.equal(element.threads[0].comments.length, 0);
+ assert.equal(element.threads[0].commentSide, commentSide);
+ assert.equal(element.threads[0].patchNum, 5);
+ assert.equal(element.threads[0].range.startLine, range.startLine);
+ assert.equal(element.threads[0].range.endLine, range.endLine);
+ assert.equal(element.threads[0].range.startChar, range.startChar);
+ assert.equal(element.threads[0].range.endChar, range.endChar);
});
test('removeThread', () => {
- const locationRange = 'range-1-2-3-4';
- element._threads = [
- {locationRange: 'range-1-2-3-4', comments: []},
- {locationRange: 'line', comments: []},
+ element.threads = [
+ {rootId: 4711},
+ {rootId: 42},
];
- flushAsynchronousOperations();
- element.removeThread(locationRange);
- flushAsynchronousOperations();
- assert(element._threads.length, 1);
+ element.removeThread(4711);
+ assert.equal(element.threads.length, 1);
+ assert.equal(element.threads[0].rootId, 42);
});
test('_rangesEqual', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
index d5e6855..f3e3249 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
@@ -35,18 +35,47 @@
* @event thread-changed
*/
+ /**
+ * gr-diff-comment-thread exposes the following attributes that allow a
+ * diff widget like gr-diff to show the thread in the right location:
+ *
+ * line-num:
+ * 1-based line number or undefined if it refers to the entire file.
+ *
+ * comment-side:
+ * "left" or "right". These indicate which of the two diffed versions
+ * the comment relates to. In the case of unified diff, the left
+ * version is the one whose line number column is further to the left.
+ *
+ * range:
+ * The range of text that the comment refers to (startLine, startChar,
+ * endLine, endChar), serialized as JSON. If set, range's startLine
+ * will have the same value as line-num. Line numbers are 1-based,
+ * char numbers are 0-based. The start position (startLine, startChar)
+ * is inclusive, and the end position (endLine, endChar) is exclusive.
+ */
properties: {
changeNum: String,
comments: {
type: Array,
value() { return []; },
},
- range: Object,
+ /**
+ * @type {?{startLine: number, startChar: number, endLine: number,
+ * endChar: number}}
+ */
+ range: {
+ type: Object,
+ reflectToAttribute: true,
+ },
keyEventTarget: {
type: Object,
value() { return document.body; },
},
- commentSide: String,
+ commentSide: {
+ type: String,
+ reflectToAttribute: true,
+ },
patchNum: String,
path: String,
projectName: {
@@ -79,8 +108,11 @@
type: Boolean,
value: false,
},
- /** Necessary only if showFilePath is true */
- lineNum: Number,
+ /** Necessary only if showFilePath is true or when used with gr-diff */
+ lineNum: {
+ type: Number,
+ reflectToAttribute: true,
+ },
unresolved: {
type: Boolean,
notify: true,
@@ -231,7 +263,7 @@
// Ensure drafts are at the end. There should only be one but in edge
// cases could be more. In the unlikely event two drafts are being
// compared, use the typical date compare.
- if (c2.__draft && !c1.__draft ) { return 0; }
+ if (c2.__draft && !c1.__draft ) { return -1; }
if (c1.__draft && !c2.__draft ) { return 1; }
if (dateCompare === 0 && (!c1.id || !c1.id.localeCompare)) { return 0; }
// If same date, fall back to sorting by id.
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
index b525a60..58648bf 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
@@ -714,5 +714,28 @@
assert.equal(element._orderedComments[1].id, '2');
assert.equal(element._orderedComments[2].id, '3');
});
+
+ test('reflects lineNum and commentSide to attributes', () => {
+ element.lineNum = 7;
+ element.commentSide = 'left';
+
+ assert.equal(element.getAttribute('line-num'), '7');
+ assert.equal(element.getAttribute('comment-side'), 'left');
+ });
+
+ test('reflects range to JSON serialized attribute if set', () => {
+ element.range = {startLine: 4, endLine: 5, startChar: 6, endChar: 7};
+
+ assert.deepEqual(
+ JSON.parse(element.getAttribute('range')),
+ {startLine: 4, endLine: 5, startChar: 6, endChar: 7});
+ });
+
+ test('removes range attribute if range is unset', () => {
+ element.range = {startLine: 4, endLine: 5, startChar: 6, endChar: 7};
+ element.range = undefined;
+
+ assert.notOk(element.hasAttribute('range'));
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
index 6ea0330..72c7285 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
@@ -91,10 +91,12 @@
text-align: right;
white-space: nowrap;
}
- a.date:link,
- a.date:visited {
+ span.date {
color: var(--deemphasized-text-color);
}
+ span.date:hover {
+ text-decoration: underline;
+ }
.actions {
display: flex;
justify-content: flex-end;
@@ -255,11 +257,11 @@
on-tap="_handleCommentDelete">
(Delete)
</gr-button>
- <a class="date" href$="[[_computeLinkToComment(comment)]]" on-tap="_handleLinkTap">
+ <span class="date" on-tap="_handleAnchorTap">
<gr-date-formatter
has-tooltip
date-str="[[comment.updated]]"></gr-date-formatter>
- </a>
+ </span>
<div class="show-hide">
<label class="show-hide">
<input type="checkbox" class="show-hide"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
index 90d465f..5165db0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -29,6 +29,8 @@
const REPORT_UPDATE_DRAFT = 'UpdateDraftComment';
const REPORT_DISCARD_DRAFT = 'DiscardDraftComment';
+ const FILE = 'FILE';
+
Polymer({
is: 'gr-diff-comment',
@@ -64,6 +66,12 @@
* @event comment-mouse-out
*/
+ /**
+ * Fired when the comment's timestamp is tapped.
+ *
+ * @event comment-anchor-tap
+ */
+
properties: {
changeNum: String,
/** @type {?} */
@@ -333,10 +341,6 @@
}
},
- _computeLinkToComment(comment) {
- return '#' + comment.line;
- },
-
_computeDeleteButtonClass(isAdmin, draft) {
return isAdmin && !draft ? 'showDeleteButtons' : '';
},
@@ -401,15 +405,16 @@
}, STORAGE_DEBOUNCE_INTERVAL);
},
- _handleLinkTap(e) {
+ _handleAnchorTap(e) {
e.preventDefault();
- const hash = this._computeLinkToComment(this.comment);
- // Don't add the hash to the window history if it's already there.
- // Otherwise you mess up expected back button behavior.
- if (window.location.hash == hash) { return; }
- // Change the URL but don’t trigger a nav event. Otherwise it will
- // reload the page.
- page.show(window.location.pathname + hash, null, false);
+ if (!this.comment.line) { return; }
+ this.dispatchEvent(new CustomEvent('comment-anchor-tap', {
+ bubbles: true,
+ detail: {
+ number: this.comment.line || FILE,
+ side: this.side,
+ },
+ }));
},
_handleEdit(e) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
index ca85892..912e615 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
@@ -99,15 +99,17 @@
'header middle content is not visible');
});
- test('clicking on date link does not trigger nav', () => {
- const showStub = sinon.stub(page, 'show');
+ test('clicking on date link fires event', () => {
+ element.side = 'PARENT';
+ const stub = sinon.stub();
+ element.addEventListener('comment-anchor-tap', stub);
const dateEl = element.$$('.date');
assert.ok(dateEl);
MockInteractions.tap(dateEl);
- const dest = window.location.pathname + '#5';
- assert(showStub.lastCall.calledWithExactly(dest, null, false),
- 'Should navigate to ' + dest + ' without triggering nav');
- showStub.restore();
+
+ assert.isTrue(stub.called);
+ assert.deepEqual(stub.lastCall.args[0].detail,
+ {side: element.side, number: element.comment.line});
});
test('message is not retrieved from storage when other edits', done => {
@@ -733,17 +735,6 @@
assert.isTrue(saveStub.calledOnce);
});
- test('clicking on date link does not trigger nav', () => {
- const showStub = sinon.stub(page, 'show');
- const dateEl = element.$$('.date');
- assert.ok(dateEl);
- MockInteractions.tap(dateEl);
- const dest = window.location.pathname + '#5';
- assert(showStub.lastCall.calledWithExactly(dest, null, false),
- 'Should navigate to ' + dest + ' without triggering nav');
- showStub.restore();
- });
-
test('proper event fires on resolve, comment is not saved', done => {
const save = sandbox.stub(element, 'save');
element.addEventListener('comment-update', e => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
index af8725e..577eec6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
@@ -35,7 +35,7 @@
listeners: {
'comment-mouse-out': '_handleCommentMouseOut',
'comment-mouse-over': '_handleCommentMouseOver',
- 'create-comment': '_createComment',
+ 'create-range-comment': '_createRangeComment',
},
observers: [
@@ -317,7 +317,7 @@
}
},
- _createComment(e) {
+ _createRangeComment(e) {
this._removeActionBox();
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
index b10e3cc..98d55c0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
@@ -215,9 +215,9 @@
assert.isFalse(builder.getContentsByLineRange.called);
});
- test('on create-comment action box is removed', () => {
+ test('on create-range-comment action box is removed', () => {
sandbox.stub(element, '_removeActionBox');
- element.fire('create-comment', {
+ element.fire('create-range-comment', {
comment: {
range: {},
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
index e3bf866..a5f5fd9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
@@ -16,9 +16,9 @@
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
-
<link rel="import" href="../gr-diff/gr-diff.html">
<dom-module id="gr-diff-host">
@@ -30,7 +30,6 @@
patch-range="[[patchRange]]"
path="[[path]]"
prefs="[[prefs]]"
- project-config="[[projectConfig]]"
project-name="[[projectName]]"
display-line="[[displayLine]]"
is-image-diff="[[isImageDiff]]"
@@ -47,7 +46,8 @@
base-image="[[_baseImage]]"
revision-image=[[_revisionImage]]
blame="[[_blame]]"
- diff="[[_diff]]"></gr-diff>
+ diff="[[_diff]]"
+ parent-index="[[_parentIndex]]"></gr-diff>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
<gr-reporting id="reporting" category="diff"></gr-reporting>
</template>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index 6f61fb9..056ab60 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -84,9 +84,6 @@
prefs: {
type: Object,
},
- projectConfig: {
- type: Object,
- },
projectName: String,
displayLine: {
type: Boolean,
@@ -174,10 +171,19 @@
},
_loadedWhitespaceLevel: String,
+
+ _parentIndex: {
+ type: Number,
+ computed: '_computeParentIndex(patchRange.*)',
+ },
},
+ behaviors: [
+ Gerrit.PatchSetBehavior,
+ ],
+
listeners: {
- 'draft-interaction': '_handleDraftInteraction',
+ 'create-comment': '_handleCreateComment',
},
observers: [
@@ -299,7 +305,10 @@
this._blame = null;
},
- /** @return {!Array<!HTMLElement>} */
+ /**
+ * The thread elements in this diff, in no particular order.
+ * @return {!Array<!HTMLElement>}
+ */
getThreadEls() {
return this.$.diff.getThreadEls();
},
@@ -445,11 +454,35 @@
this.patchRange);
},
- _handleDraftInteraction() {
+ /** @param {CustomEvent} e */
+ _handleCreateComment(e) {
+ const {threadGroupEl, lineNum, side, range} = e.detail;
+ const threadEl = this._getOrCreateThread(threadGroupEl, side, range);
+ threadEl.addOrEditDraft(lineNum, range);
this.$.reporting.recordDraftInteraction();
},
/**
+ * Gets or creates a comment thread from a specific thread group.
+ * May include a range, if the comment is a range comment.
+ *
+ * @param {!Object} threadGroupEl
+ * @param {string} commentSide
+ * @param {!Object=} range
+ * @return {!Object}
+ */
+ _getOrCreateThread(threadGroupEl, commentSide, range=undefined) {
+ let threadEl = threadGroupEl.getThread(commentSide, range);
+
+ if (!threadEl) {
+ threadGroupEl.addNewThread(commentSide, range);
+ Polymer.dom.flush();
+ threadEl = threadGroupEl.getThread(commentSide, range);
+ }
+ return threadEl;
+ },
+
+ /**
* Take a diff that was loaded with a ignore-whitespace other than
* IGNORE_NONE, and convert delta chunks labeled as common into shared
* chunks.
@@ -506,5 +539,15 @@
this.reload();
}
},
+
+ /**
+ * @param {Object} patchRangeRecord
+ * @return {number|null}
+ */
+ _computeParentIndex(patchRangeRecord) {
+ return this.isMergeParent(patchRangeRecord.base.basePatchNum) ?
+ this.getParentIndex(patchRangeRecord.base.basePatchNum) : null;
+ },
+
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index f83253e..24afe87 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -37,9 +37,14 @@
suite('gr-diff-host tests', () => {
let element;
let sandbox;
+ let getLoggedIn;
setup(() => {
sandbox = sinon.sandbox.create();
+ getLoggedIn = false;
+ stub('gr-rest-api-interface', {
+ async getLoggedIn() { return getLoggedIn; },
+ });
element = fixture('basic');
});
@@ -59,12 +64,8 @@
suite('not logged in', () => {
setup(() => {
- const getLoggedInPromise = Promise.resolve(false);
- stub('gr-rest-api-interface', {
- getLoggedIn() { return getLoggedInPromise; },
- });
+ getLoggedIn = false;
element = fixture('basic');
- return getLoggedInPromise;
});
test('reload() loads files weblinks', () => {
@@ -617,12 +618,6 @@
assert.equal(element.$.diff.prefs, value);
});
- test('passes in projectConfig', () => {
- const value = {};
- element.projectConfig = value;
- assert.equal(element.$.diff.projectConfig, value);
- });
-
test('passes in changeNum', () => {
const value = '12345';
element.changeNum = value;
@@ -776,6 +771,29 @@
});
});
+ test('_getOrCreateThread', () => {
+ const threadGroupEl =
+ document.createElement('gr-diff-comment-thread-group');
+ const commentSide = 'left';
+
+ assert.isOk(element._getOrCreateThread(threadGroupEl,
+ commentSide));
+
+ // Try to fetch a thread with a different range.
+ range = {
+ startLine: 1,
+ startChar: 1,
+ endLine: 1,
+ endChar: 3,
+ };
+
+ assert.isOk(element._getOrCreateThread(
+ threadGroupEl, commentSide, range));
+ const threadCount = Polymer.dom(threadGroupEl.root).
+ querySelectorAll('gr-diff-comment-thread').length;
+ assert.equal(threadCount, 2);
+ });
+
suite('_translateChunksToIgnore', () => {
let content;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 0866849..bb3eff9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -336,6 +336,7 @@
project-name="[[_change.project]]"
view-mode="[[_diffMode]]"
is-blame-loaded="{{_isBlameLoaded}}"
+ on-comment-anchor-tap="_onLineSelected"
on-line-selected="_onLineSelected">
</gr-diff-host>
<gr-diff-preferences
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index bddfc6d..adb4dd6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -290,7 +290,6 @@
is-image-diff="[[isImageDiff]]"
base-image="[[baseImage]]"
revision-image="[[revisionImage]]"
- parent-index="[[_parentIndex]]"
create-comment-fn="[[_createThreadGroupFn]]"
line-of-interest="[[lineOfInterest]]">
<table
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 3ae8806..6016a5a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -60,9 +60,9 @@
*/
/**
- * Fired when a draft is added or edited.
+ * Fired when a comment is created
*
- * @event draft-interaction
+ * @event create-comment
*/
properties: {
@@ -78,10 +78,6 @@
type: Object,
observer: '_prefsObserver',
},
- projectConfig: {
- type: Object,
- observer: '_projectConfigChanged',
- },
projectName: String,
displayLine: {
type: Boolean,
@@ -176,10 +172,7 @@
observer: '_blameChanged',
},
- _parentIndex: {
- type: Number,
- computed: '_computeParentIndex(patchRange.*)',
- },
+ parentIndex: Number,
_newlineWarning: {
type: String,
@@ -207,7 +200,7 @@
'comment-discard': '_handleCommentDiscard',
'comment-update': '_handleCommentUpdate',
'comment-save': '_handleCommentSave',
- 'create-comment': '_handleCreateComment',
+ 'create-range-comment': '_handleCreateRangeComment',
},
/** Cancel any remaining diff builder rendering work. */
@@ -322,7 +315,7 @@
this._createComment(el, lineNum);
},
- _handleCreateComment(e) {
+ _handleCreateRangeComment(e) {
const range = e.detail.range;
const side = e.detail.side;
const lineNum = range.endLine;
@@ -359,35 +352,29 @@
/**
* @param {!Object} lineEl
- * @param {number=} opt_lineNum
- * @param {string=} opt_side
- * @param {!Object=} opt_range
+ * @param {number=} lineNum
+ * @param {string=} side
+ * @param {!Object=} range
*/
- _createComment(lineEl, opt_lineNum, opt_side, opt_range) {
- this.dispatchEvent(new CustomEvent('draft-interaction', {bubbles: true}));
+ _createComment(lineEl, lineNum=undefined, side=undefined, range=undefined) {
const contentText = this.$.diffBuilder.getContentByLineEl(lineEl);
const contentEl = contentText.parentElement;
- const side = opt_side ||
+ side = side ||
this._getCommentSideByLineAndContent(lineEl, contentEl);
const patchNum = this._getPatchNumByLineAndContent(lineEl, contentEl);
const isOnParent =
this._getIsParentCommentByLineAndContent(lineEl, contentEl);
- const threadEl = this._getOrCreateThread(contentEl, patchNum,
- side, isOnParent, opt_range);
- threadEl.addOrEditDraft(opt_lineNum, opt_range);
- },
-
- /**
- * Fetch the thread group at the given range, or the range-less thread
- * on the line if no range is provided.
- *
- * @param {!Object} threadGroupEl
- * @param {string} commentSide
- * @param {!Object=} opt_range
- * @return {!Object}
- */
- _getThread(threadGroupEl, commentSide, opt_range) {
- return threadGroupEl.getThread(commentSide, opt_range);
+ const threadGroupEl = this._getOrCreateThreadGroup(contentEl, patchNum,
+ side, isOnParent);
+ this.dispatchEvent(new CustomEvent('create-comment', {
+ bubbles: true,
+ detail: {
+ threadGroupEl,
+ lineNum,
+ side,
+ range,
+ },
+ }));
},
_getThreadGroupForLine(contentEl) {
@@ -395,18 +382,15 @@
},
/**
- * Gets or creates a comment thread for a specific spot on a diff.
- * May include a range, if the comment is a range comment.
- *
+ * Gets or creates a comment thread group for a specific line and side on a
+ * diff.
* @param {!Object} contentEl
* @param {number} patchNum
* @param {string} commentSide
* @param {boolean} isOnParent
- * @param {!Object=} opt_range
* @return {!Object}
*/
- _getOrCreateThread(contentEl, patchNum, commentSide,
- isOnParent, opt_range) {
+ _getOrCreateThreadGroup(contentEl, patchNum, commentSide, isOnParent) {
// Check if thread group exists.
let threadGroupEl = this._getThreadGroupForLine(contentEl);
if (!threadGroupEl) {
@@ -414,15 +398,7 @@
commentSide);
contentEl.appendChild(threadGroupEl);
}
-
- let threadEl = this._getThread(threadGroupEl, commentSide, opt_range);
-
- if (!threadEl) {
- threadGroupEl.addNewThread(commentSide, opt_range);
- Polymer.dom.flush();
- threadEl = this._getThread(threadGroupEl, commentSide, opt_range);
- }
- return threadEl;
+ return threadGroupEl;
},
/**
@@ -662,13 +638,6 @@
this.$.diffTable.innerHTML = null;
},
- _projectConfigChanged(projectConfig) {
- const threadEls = this.getThreadEls();
- for (let i = 0; i < threadEls.length; i++) {
- threadEls[i].projectConfig = projectConfig;
- }
- },
-
/** @return {!Array} */
_computeDiffHeaderItems(diffInfoRecord) {
const diffInfo = diffInfoRecord.base;
@@ -710,16 +679,6 @@
return errorMessage ? 'showError' : '';
},
- /**
- * @return {number|null}
- */
- _computeParentIndex(patchRangeRecord) {
- if (!this.isMergeParent(patchRangeRecord.base.basePatchNum)) {
- return null;
- }
- return this.getParentIndex(patchRangeRecord.base.basePatchNum);
- },
-
expandAllContext() {
this._handleFullBypass();
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index faf529b..07584c7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -298,12 +298,6 @@
const commentSide = 'left';
const patchNum = 1;
const side = 'PARENT';
- let range = {
- startLine: 1,
- startChar: 1,
- endLine: 1,
- endChar: 2,
- };
element.changeNum = 123;
element.patchRange = {basePatchNum: 1, patchNum: 2};
@@ -311,36 +305,22 @@
const mock = document.createElement('mock-diff-response');
element.$.diffBuilder._builder = element.$.diffBuilder._getDiffBuilder(
- mock.diffResponse, {}, {tab_size: 2, line_length: 80});
+ mock.diffResponse, {left: [], right: []},
+ {tab_size: 2, line_length: 80});
// No thread groups.
assert.isNotOk(element._getThreadGroupForLine(contentEl));
// A thread group gets created.
- assert.isOk(element._getOrCreateThread(contentEl,
- patchNum, commentSide, side));
+ const threadGroupEl = element._getOrCreateThreadGroup(contentEl,
+ patchNum, commentSide, side);
+ assert.isOk(threadGroupEl);
- // Try to fetch a thread with a different range.
- range = {
- startLine: 1,
- startChar: 1,
- endLine: 1,
- endChar: 3,
- };
-
- assert.isOk(element._getOrCreateThread(
- contentEl, patchNum, commentSide, side, range));
// The new thread group can be fetched.
assert.isOk(element._getThreadGroupForLine(contentEl));
assert.equal(contentEl.querySelectorAll(
'gr-diff-comment-thread-group').length, 1);
-
- const threadGroup = contentEl.querySelector(
- 'gr-diff-comment-thread-group');
- const threadLength = Polymer.dom(threadGroup.root).
- querySelectorAll('gr-diff-comment-thread').length;
- assert.equal(threadLength, 2);
});
suite('image diffs', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
index 6349ab6..0f84877 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
@@ -23,7 +23,7 @@
/**
* Fired when the comment creation action was taken (hotkey, click).
*
- * @event create-comment
+ * @event create-range-comment
*/
properties: {
@@ -110,7 +110,7 @@
},
_fireCreateComment() {
- this.fire('create-comment', {side: this.side, range: this.range});
+ this.fire('create-range-comment', {side: this.side, range: this.range});
},
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
index 19155e4..4f1065a 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
@@ -98,7 +98,7 @@
element.range = range;
MockInteractions.pressAndReleaseKeyOn(document.body, 67, null, 'c');
assert(element.fire.calledWithExactly(
- 'create-comment',
+ 'create-range-comment',
{
side,
range,
diff --git a/polygerrit-ui/app/embed/embed.html b/polygerrit-ui/app/embed/embed.html
index 948916f..1b2f20f 100644
--- a/polygerrit-ui/app/embed/embed.html
+++ b/polygerrit-ui/app/embed/embed.html
@@ -25,7 +25,6 @@
<link rel="import" href="../elements/core/gr-search-bar/gr-search-bar.html">
<link rel="import" href="../elements/diff/gr-diff-view/gr-diff-view.html">
<link rel="import" href="../elements/change-list/gr-change-list-view/gr-change-list-view.html">
-<link rel="import" href="../elements/change-list/gr-change-list/gr-change-list.html">
-<link rel="import" href="../elements/change-list/gr-create-change-help/gr-create-change-help.html">
<link rel="import" href="../elements/change-list/gr-dashboard-view/gr-dashboard-view.html">
+<link rel="import" href="../elements/change-list/gr-embed-dashboard/gr-embed-dashboard.html">
<link rel="import" href="../styles/themes/app-theme.html">
diff --git a/tools/maven/gerrit-acceptance-framework_pom.xml b/tools/maven/gerrit-acceptance-framework_pom.xml
index e66a938..c5faa49 100644
--- a/tools/maven/gerrit-acceptance-framework_pom.xml
+++ b/tools/maven/gerrit-acceptance-framework_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-acceptance-framework</artifactId>
- <version>2.16-rc3</version>
+ <version>3.0-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Acceptance Test Framework</name>
<description>Framework for Gerrit's acceptance tests</description>
diff --git a/tools/maven/gerrit-extension-api_pom.xml b/tools/maven/gerrit-extension-api_pom.xml
index 623964c..52b11c1 100644
--- a/tools/maven/gerrit-extension-api_pom.xml
+++ b/tools/maven/gerrit-extension-api_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-extension-api</artifactId>
- <version>2.16-rc3</version>
+ <version>3.0-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Extension API</name>
<description>API for Gerrit Extensions</description>
diff --git a/tools/maven/gerrit-plugin-api_pom.xml b/tools/maven/gerrit-plugin-api_pom.xml
index 212e739..d22c3ee 100644
--- a/tools/maven/gerrit-plugin-api_pom.xml
+++ b/tools/maven/gerrit-plugin-api_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-plugin-api</artifactId>
- <version>2.16-rc3</version>
+ <version>3.0-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Plugin API</name>
<description>API for Gerrit Plugins</description>
diff --git a/tools/maven/gerrit-plugin-gwtui_pom.xml b/tools/maven/gerrit-plugin-gwtui_pom.xml
index 1fe482c..e756352 100644
--- a/tools/maven/gerrit-plugin-gwtui_pom.xml
+++ b/tools/maven/gerrit-plugin-gwtui_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-plugin-gwtui</artifactId>
- <version>2.16-rc3</version>
+ <version>3.0-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Plugin GWT UI</name>
<description>Common Classes for Gerrit GWT UI Plugins</description>
diff --git a/tools/maven/gerrit-war_pom.xml b/tools/maven/gerrit-war_pom.xml
index 4a84174..e6c04e2 100644
--- a/tools/maven/gerrit-war_pom.xml
+++ b/tools/maven/gerrit-war_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-war</artifactId>
- <version>2.16-rc3</version>
+ <version>3.0-SNAPSHOT</version>
<packaging>war</packaging>
<name>Gerrit Code Review - WAR</name>
<description>Gerrit WAR</description>
diff --git a/version.bzl b/version.bzl
index 04b03a7..20fd8a7 100644
--- a/version.bzl
+++ b/version.bzl
@@ -2,4 +2,4 @@
# Used by :api_install and :api_deploy targets
# when talking to the destination repository.
#
-GERRIT_VERSION = "2.16-rc3"
+GERRIT_VERSION = "3.0-SNAPSHOT"
