Merge branch 'stable-2.16'
* stable-2.16:
Handle dashboard edge case
Fix title of help section for creating a change
Add Luca Milanesio as developer in pom.xml files
Bazel: Export j2objc in guava library to fix build warning
Change-Id: Ie0cbc23b0e0c31dcf16657a176935d0028099530
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/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 496ee5b..63321884 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;
@@ -1244,14 +1245,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 +1261,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);
@@ -1714,4 +1717,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/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/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..75cf3a6 100644
--- a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
@@ -133,6 +133,8 @@
MergeListRequest getMergeList() throws RestApiException;
+ RelatedChangesInfo related() throws RestApiException;
+
abstract class MergeListRequest {
private boolean addLinks;
private int uninterestingParent = 1;
@@ -371,6 +373,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/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/server/api/changes/RevisionApiImpl.java b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
index 33f211d..3f03b57 100644
--- a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
@@ -26,6 +26,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 +65,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 +131,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 +172,7 @@
TestSubmitType.Get getSubmitType,
Provider<TestSubmitRule> testSubmitRule,
Provider<GetMergeList> getMergeList,
+ GetRelated getRelated,
PutDescription putDescription,
GetDescription getDescription,
@Assisted RevisionResource r) {
@@ -207,6 +211,7 @@
this.getSubmitType = getSubmitType;
this.testSubmitRule = testSubmitRule;
this.getMergeList = getMergeList;
+ this.getRelated = getRelated;
this.putDescription = putDescription;
this.getDescription = getDescription;
this.revision = r;
@@ -590,6 +595,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/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/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 9f75c07..4c2d101 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -331,7 +331,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);
}
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/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index bccc415..a16c94e 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -127,6 +127,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";
@@ -593,6 +595,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 +758,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 +1184,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 +1237,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/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/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/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/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..7dce600 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;
@@ -943,13 +944,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
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/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/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/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/project/ProjectConfigTest.java b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
index 0e4ba10..4daa488 100644
--- a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
+++ b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
@@ -104,6 +104,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 +122,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 +271,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 +289,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 +307,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
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 ba768d9..cfe97bd 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/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/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index faf529b..43f90bf 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
@@ -311,7 +311,8 @@
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));
diff --git a/tools/maven/gerrit-acceptance-framework_pom.xml b/tools/maven/gerrit-acceptance-framework_pom.xml
index 4f3958e..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-rc0</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 d8af152..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-rc0</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 c63bd2d..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-rc0</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 3cb6702..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-rc0</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 c22aa75..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-rc0</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 8b644fa..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-rc0"
+GERRIT_VERSION = "3.0-SNAPSHOT"
