Merge branch 'stable-3.2' into stable-3.3
* stable-3.2:
UI: Don't suggest unsupported system groups
Document that configured groups must be visible to uploader
Bazel: Remove unnecessary plugin_name parameter
Change-Id: I0781085e75a0145b49d4d86a4ea061518d8da410
diff --git a/BUILD b/BUILD
index 9fd9c55..6f5f9dc 100644
--- a/BUILD
+++ b/BUILD
@@ -1,5 +1,5 @@
load("@rules_java//java:defs.bzl", "java_library")
-load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
+load("@npm//@bazel/rollup:index.bzl", "rollup_bundle")
load("//tools/bzl:junit.bzl", "junit_tests")
load("//tools/js:eslint.bzl", "eslint")
load(
diff --git a/rv-reviewers/rv-edit-screen.js b/rv-reviewers/rv-edit-screen.js
index 0c692f5..b0419bf 100644
--- a/rv-reviewers/rv-edit-screen.js
+++ b/rv-reviewers/rv-edit-screen.js
@@ -65,7 +65,7 @@
}
_handleCreateSection() {
- const section = {filter: '', reviewers: [], editing: true};
+ const section = {filter: '', reviewers: [], ccs: [], editing: true};
this._editingFilter = true;
this.push('_filterSections', section);
}
diff --git a/rv-reviewers/rv-edit-screen_html.js b/rv-reviewers/rv-edit-screen_html.js
index 63ea346..954d46c 100644
--- a/rv-reviewers/rv-edit-screen_html.js
+++ b/rv-reviewers/rv-edit-screen_html.js
@@ -56,6 +56,7 @@
<rv-filter-section
filter="[[section.filter]]"
reviewers="[[section.reviewers]]"
+ ccs="[[section.ccs]]"
editing="[[section.editing]]"
reviewers-url="[[_getReviewersUrl(repoName)]]"
repo-name="[[repoName]]"
diff --git a/rv-reviewers/rv-filter-section.js b/rv-reviewers/rv-filter-section.js
index 3bf6d06..ae43a62 100644
--- a/rv-reviewers/rv-filter-section.js
+++ b/rv-reviewers/rv-filter-section.js
@@ -33,6 +33,7 @@
pluginRestApi: Object,
repoName: String,
reviewers: Array,
+ ccs: Array,
filter: String,
canModifyConfig: Boolean,
_originalFilter: String,
@@ -77,19 +78,24 @@
}
_handleReviewerDeleted(e) {
+ const type = e.detail.type;
if (e.detail.editing) {
- this.reviewers.pop();
+ if (type === 'CC') {
+ this.ccs.pop();
+ } else {
+ this.reviewers.pop();
+ }
this._editingReviewer = false;
} else {
const index = e.model.index;
- const deleted = this.reviewers[index];
- this._putReviewer(deleted, 'DELETE');
+ const deleted = type === 'CC' ? this.ccs[index] : this.reviewers[index];
+ this._putReviewer(deleted, 'REMOVE', type);
}
}
_handleReviewerAdded(e) {
this._editingReviewer = false;
- this._putReviewer(e.detail.reviewer, 'ADD').catch(err => {
+ this._putReviewer(e.detail.reviewer, 'ADD', e.detail.type).catch(err => {
this.dispatchEvent(new CustomEvent('show-alert', {
detail: {
message: err,
@@ -100,10 +106,11 @@
});
}
- _putReviewer(reviewer, action) {
+ _putReviewer(reviewer, action, type) {
return this.pluginRestApi.put(this.reviewersUrl, {
action,
reviewer,
+ type,
filter: this.filter,
}).then(result => {
const detail = {result};
@@ -116,6 +123,11 @@
this.push('reviewers', '');
this._editingReviewer = true;
}
+
+ _handleAddCc() {
+ this.push('ccs', '');
+ this._editingReviewer = true;
+ }
}
customElements.define(RvFilterSection.is, RvFilterSection);
diff --git a/rv-reviewers/rv-filter-section_html.js b/rv-reviewers/rv-filter-section_html.js
index b4a72e4..87cc7a3 100644
--- a/rv-reviewers/rv-filter-section_html.js
+++ b/rv-reviewers/rv-filter-section_html.js
@@ -84,6 +84,7 @@
items="{{reviewers}}">
<rv-reviewer
reviewer="{{item}}"
+ type="REVIEWER"
can-modify-config="[[canModifyConfig]]"
plugin-rest-api="[[pluginRestApi]]"
repo-name="[[repoName]]"
@@ -91,12 +92,30 @@
on-reviewer-added="_handleReviewerAdded">
</rv-reviewer>
</template>
+ <template
+ is="dom-repeat"
+ items="{{ccs}}">
+ <rv-reviewer
+ reviewer="{{item}}"
+ type="CC"
+ can-modify-config="[[canModifyConfig]]"
+ plugin-rest-api="[[pluginRestApi]]"
+ repo-name="[[repoName]]"
+ on-reviewer-deleted="_handleReviewerDeleted"
+ on-reviewer-added="_handleReviewerAdded">
+ </rv-reviewer>
+ </template>
<div id="addReviewer">
<gr-button
link
- id="addBtn"
+ id="addRevBtn"
on-tap="_handleAddReviewer"
hidden="[[_computeAddBtnHidden(canModifyConfig, _editingReviewer)]]">Add Reviewer</gr-button>
+ <gr-button
+ link
+ id="addCcBtn"
+ on-tap="_handleAddCc"
+ hidden="[[_computeAddBtnHidden(canModifyConfig, _editingReviewer)]]">Add Cc</gr-button>
</div><!-- addReviewer -->
</div><!-- reviewers -->
</div>
diff --git a/rv-reviewers/rv-reviewer.js b/rv-reviewers/rv-reviewer.js
index 77500b2..338b428 100644
--- a/rv-reviewers/rv-reviewer.js
+++ b/rv-reviewers/rv-reviewer.js
@@ -36,6 +36,11 @@
pluginRestAPi: Object,
repoName: String,
reviewer: String,
+ type: String,
+ _header: {
+ type: String,
+ computed: '_computeHeader(type)',
+ },
_reviewerSearchId: String,
_queryReviewers: {
type: Function,
@@ -57,6 +62,13 @@
this._originalReviewer = this.reviewer;
}
+ _computeHeader(type) {
+ if (type === 'CC') {
+ return 'Cc';
+ }
+ return 'Reviewer';
+ }
+
_computeEditing(reviewer, _originalReviewer) {
if (_originalReviewer === '') {
return true;
@@ -70,8 +82,8 @@
}
_computeHideAddButton(reviewer, _originalReviewer) {
- return !(this._computeEditing(reviewer, _originalReviewer)
- && this._reviewerSearchId);
+ return !(this._computeEditing(reviewer, _originalReviewer) &&
+ this._reviewerSearchId);
}
_computeHideDeleteButton(canModifyConfig) {
@@ -95,7 +107,10 @@
const groupSuggestions = [];
for (const key in groups) {
if (!groups.hasOwnProperty(key) ||
- groups[key].id.startsWith(SYSTEM_GROUP_PREFIX)) { continue; }
+ groups[key].id.startsWith(SYSTEM_GROUP_PREFIX) ||
+ key.startsWith('user/')) {
+ continue;
+ }
groupSuggestions.push({
name: key,
value: key,
@@ -137,7 +152,7 @@
}
_handleDeleteCancel() {
- const detail = {editing: this._editing};
+ const detail = {editing: this._editing, type: this.type};
if (this._editing) {
this.remove();
}
@@ -146,7 +161,7 @@
}
_handleAddReviewer() {
- const detail = {reviewer: this._reviewerSearchId};
+ const detail = {reviewer: this._reviewerSearchId, type: this.type};
this._originalReviewer = this.reviewer;
this.dispatchEvent(
new CustomEvent('reviewer-added', {detail, bubbles: true}));
diff --git a/rv-reviewers/rv-reviewer_html.js b/rv-reviewers/rv-reviewer_html.js
index 0e9becb..29b22de 100644
--- a/rv-reviewers/rv-reviewer_html.js
+++ b/rv-reviewers/rv-reviewer_html.js
@@ -43,7 +43,7 @@
</style>
<style include="gr-form-styles"></style>
<div class="reviewerRow">
- <h4 id="reviewerHeader">Reviewer:</h4>
+ <h4 id="reviewerHeader">[[_header]]:</h4>
<template is="dom-if" if="[[_computeEditing(reviewer, _originalReviewer)]]">
<span class="value">
<!--
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewers.java
index 30568dc..80fab28 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewers.java
@@ -19,6 +19,7 @@
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.util.ManualRequestContext;
@@ -27,6 +28,7 @@
import com.google.inject.assistedinject.Assisted;
import java.util.ArrayList;
import java.util.Set;
+import java.util.stream.Collectors;
/** Adds reviewers to a change. */
class AddReviewers implements Runnable {
@@ -36,9 +38,13 @@
private final OneOffRequestContext requestContext;
private final ChangeInfo changeInfo;
private final Set<Account.Id> reviewers;
+ private final Set<Account.Id> ccs;
interface Factory {
- AddReviewers create(ChangeInfo changeInfo, Set<Account.Id> reviewers);
+ AddReviewers create(
+ ChangeInfo changeInfo,
+ @Assisted("reviewers") Set<Account.Id> reviewers,
+ @Assisted("ccs") Set<Account.Id> ccs);
}
@Inject
@@ -46,11 +52,13 @@
GerritApi gApi,
OneOffRequestContext requestContext,
@Assisted ChangeInfo changeInfo,
- @Assisted Set<Account.Id> reviewers) {
+ @Assisted("reviewers") Set<Account.Id> reviewers,
+ @Assisted("ccs") Set<Account.Id> ccs) {
this.gApi = gApi;
this.requestContext = requestContext;
this.changeInfo = changeInfo;
this.reviewers = reviewers;
+ this.ccs = ccs;
}
@Override
@@ -65,13 +73,38 @@
try {
// TODO(davido): Switch back to using changes API again,
// when it supports batch mode for adding reviewers
+ Set<Account.Id> existingReviewers =
+ gApi.changes().id(changeInfo._number).reviewers().stream()
+ .map(r -> Account.id(r._accountId))
+ .collect(Collectors.toSet());
+ /* Don't add, or change state of, already existing reviewers. */
+ Set<Account.Id> reviewersToAdd =
+ reviewers.stream()
+ .filter(r -> !existingReviewers.contains(r))
+ .collect(Collectors.toSet());
+ /* If account is already configured to be added as reviewer, don't attempt to add as cc. */
+ Set<Account.Id> ccsToAdd =
+ ccs.stream()
+ .filter(c -> !existingReviewers.contains(c))
+ .filter(r -> !reviewersToAdd.contains(r))
+ .collect(Collectors.toSet());
+ if (reviewersToAdd.isEmpty() && ccsToAdd.isEmpty()) {
+ return;
+ }
+
ReviewInput in = new ReviewInput();
- in.reviewers = new ArrayList<>(reviewers.size());
- for (Account.Id account : reviewers) {
+ in.reviewers = new ArrayList<>(reviewers.size() + ccs.size());
+ for (Account.Id account : reviewersToAdd) {
AddReviewerInput addReviewerInput = new AddReviewerInput();
addReviewerInput.reviewer = account.toString();
in.reviewers.add(addReviewerInput);
}
+ for (Account.Id account : ccsToAdd) {
+ AddReviewerInput input = new AddReviewerInput();
+ input.state = ReviewerState.CC;
+ input.reviewer = account.toString();
+ in.reviewers.add(input);
+ }
gApi.changes().id(changeInfo._number).current().review(in);
} catch (RestApiException e) {
logger.atSevere().withCause(e).log("Couldn't add reviewers to the change");
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/GetReviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/GetReviewers.java
index 4aa8602..212b544 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/GetReviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/GetReviewers.java
@@ -20,11 +20,12 @@
import com.google.gerrit.server.project.ProjectResource;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.reviewers.config.ReviewersConfig;
import java.util.List;
/**
- * GET REST end-point for getting all configured {@link ReviewerFilterSection}s of a project, local
- * and inherited.
+ * GET REST end-point for getting all configured {@link ReviewerFilter}s of a project, local and
+ * inherited.
*/
@Singleton
class GetReviewers implements RestReadView<ProjectResource> {
@@ -36,8 +37,7 @@
}
@Override
- public Response<List<ReviewerFilterSection>> apply(ProjectResource resource)
- throws RestApiException {
- return Response.ok(config.forProject(resource.getNameKey()).getReviewerFilterSections());
+ public Response<List<ReviewerFilter>> apply(ProjectResource resource) throws RestApiException {
+ return Response.ok(config.filtersWithInheritance(resource.getNameKey()));
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
index 4bee073..8de2cc4 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.change.ReviewerSuggestion;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.reviewers.config.ReviewersConfig;
public class Module extends FactoryModule {
private final boolean enableREST;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/PutReviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/PutReviewers.java
index 5f55db2..d815633 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/PutReviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/PutReviewers.java
@@ -41,18 +41,20 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.reviewers.PutReviewers.Input;
+import com.googlesource.gerrit.plugins.reviewers.config.ReviewersConfig;
import java.io.IOException;
import java.util.List;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
-/** PUT REST end-point that removes or adds a reveiwer to a {@link ReviewerFilterSection}. */
+/** PUT REST end-point that removes or adds a reviewer to a {@link ReviewerFilter}. */
@Singleton
class PutReviewers implements RestModifyView<ProjectResource, Input> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
protected static class Input {
public Action action;
+ public ReviewerType type;
public String filter;
public String reviewer;
}
@@ -89,13 +91,10 @@
}
@Override
- public Response<List<ReviewerFilterSection>> apply(ProjectResource rsrc, Input input)
+ public Response<List<ReviewerFilter>> apply(ProjectResource rsrc, Input input)
throws RestApiException, PermissionBackendException {
Project.NameKey projectName = rsrc.getNameKey();
- ReviewersConfig.ForProject cfg = config.forProject(projectName);
- if (cfg == null) {
- throw new ResourceNotFoundException("Project" + projectName.get() + " not found");
- }
+ ReviewersConfig.ForProject forProject = new ReviewersConfig.ForProject();
PermissionBackend.WithUser userPermission = permissionBackend.user(rsrc.getUser());
if (!userPermission.project(rsrc.getNameKey()).testOrFalse(ProjectPermission.WRITE_CONFIG)
@@ -107,28 +106,34 @@
if (input.action == Action.ADD) {
validateReviewer(input.reviewer);
}
+ if (input.type == null) {
+ input.type = ReviewerType.REVIEWER;
+ }
try {
StringBuilder message = new StringBuilder(pluginName).append(" plugin: ");
- cfg.load(md);
+ forProject.load(md);
if (input.action == Action.ADD) {
message
- .append("Add reviewer ")
+ .append("Add ")
+ .append(input.type.name)
+ .append(" ")
.append(input.reviewer)
.append(" to filter ")
.append(input.filter);
- cfg.addReviewer(input.filter, input.reviewer);
+ forProject.addReviewer(input.filter, input.reviewer, input.type);
} else {
message
- .append("Remove reviewer ")
- .append(input.reviewer)
+ .append("Remove ")
+ .append(input.type.name)
+ .append(" ")
.append(" from filter ")
.append(input.filter);
- cfg.removeReviewer(input.filter, input.reviewer);
+ forProject.removeReviewer(input.filter, input.reviewer, input.type);
}
message.append("\n");
md.setMessage(message.toString());
try {
- cfg.commit(md);
+ forProject.commit(md);
projectCache.evict(projectName);
} catch (IOException e) {
if (e.getCause() instanceof ConfigInvalidException) {
@@ -149,7 +154,7 @@
} catch (IOException err) {
throw new ResourceNotFoundException(projectName.get(), err);
}
- return Response.ok(cfg.getReviewerFilterSections());
+ return Response.ok(config.filtersWithInheritance(projectName));
}
private void validateReviewer(String reviewer) throws RestApiException {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerFilterSection.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerFilter.java
similarity index 68%
rename from src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerFilterSection.java
rename to src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerFilter.java
index a083239..b991760 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerFilterSection.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerFilter.java
@@ -21,19 +21,15 @@
* Representation of a filter section in reviewers.config. Example:
*
* <pre>
- * [filter "'"]
+ * [filter "*"]
* reviewer = joe
* reviewer = jane
* </pre>
*/
-class ReviewerFilterSection {
- private final String filter;
- private final Set<String> reviewers;
-
- ReviewerFilterSection(String filter, Set<String> reviewers) {
- this.filter = filter;
- this.reviewers = reviewers;
- }
+public abstract class ReviewerFilter {
+ protected String filter;
+ protected Set<String> reviewers;
+ protected Set<String> ccs;
String getFilter() {
return filter;
@@ -43,17 +39,23 @@
return reviewers;
}
+ Set<String> getCcs() {
+ return ccs;
+ }
+
@Override
public boolean equals(Object o) {
- if (o instanceof ReviewerFilterSection) {
- ReviewerFilterSection other = ((ReviewerFilterSection) o);
- return Objects.equals(filter, other.filter) && Objects.equals(reviewers, other.reviewers);
+ if (o instanceof ReviewerFilter) {
+ ReviewerFilter other = ((ReviewerFilter) o);
+ return Objects.equals(filter, other.filter)
+ && Objects.equals(reviewers, other.reviewers)
+ && Objects.equals(ccs, other.ccs);
}
return false;
}
@Override
public int hashCode() {
- return Objects.hash(filter, reviewers);
+ return Objects.hash(filter, reviewers, ccs);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerType.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerType.java
new file mode 100644
index 0000000..2d8f943
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerType.java
@@ -0,0 +1,25 @@
+// Copyright (C) 2020 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.googlesource.gerrit.plugins.reviewers;
+
+public enum ReviewerType {
+ REVIEWER("reviewer"),
+ CC("cc");
+
+ String name;
+
+ ReviewerType(String name) {
+ this.name = name;
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
index 04e301b..348e856 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
@@ -20,6 +20,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
@@ -41,6 +42,7 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.reviewers.config.ReviewersConfig;
import java.util.List;
import java.util.Set;
@@ -100,14 +102,14 @@
@Nullable Change.Id changeId,
@Nullable String query,
Set<Account.Id> candidates) {
- List<ReviewerFilterSection> sections = getSections(projectName);
+ List<ReviewerFilter> filters = getFilters(projectName);
- if (sections.isEmpty() || changeId == null) {
+ if (filters.isEmpty() || changeId == null) {
return ImmutableSet.of();
}
try {
- Set<String> reviewers = findReviewers(changeId.get(), sections);
+ Set<String> reviewers = findReviewers(changeId.get(), filters);
if (!reviewers.isEmpty()) {
return resolver.resolve(reviewers, projectName, changeId.get(), null).stream()
.map(a -> suggestedReviewer(a))
@@ -126,9 +128,9 @@
return reviewer;
}
- private List<ReviewerFilterSection> getSections(Project.NameKey projectName) {
+ private List<ReviewerFilter> getFilters(Project.NameKey projectName) {
// TODO(davido): we have to cache per project configuration
- return config.forProject(projectName).getReviewerFilterSections();
+ return config.filtersWithInheritance(projectName);
}
private void onEvent(ChangeEvent event) {
@@ -142,22 +144,29 @@
}
Project.NameKey projectName = Project.nameKey(c.project);
- List<ReviewerFilterSection> sections = getSections(projectName);
+ List<ReviewerFilter> filters = getFilters(projectName);
- if (sections.isEmpty()) {
+ if (filters.isEmpty()) {
return;
}
AccountInfo uploader = event.getWho();
int changeNumber = c._number;
try {
- Set<String> reviewers = findReviewers(changeNumber, sections);
- if (reviewers.isEmpty()) {
+ List<ReviewerFilter> matching = findMatchingFilters(changeNumber, filters);
+ Set<String> reviewers = getReviewersFrom(matching);
+ Set<String> ccs = getCcsFrom(matching);
+ if (reviewers.isEmpty() && ccs.isEmpty()) {
return;
}
+ /* Remove all reviewer identifiers (account-ids, group-ids) from ccs that are present in reviewers.
+ * Further filtering of individual accounts is done in AddReviewers after the ids have been resolved into Accounts. */
+ ccs.removeAll(reviewers);
final AddReviewers addReviewers =
addReviewersFactory.create(
- c, resolver.resolve(reviewers, projectName, changeNumber, uploader));
+ c,
+ resolver.resolve(reviewers, projectName, changeNumber, uploader),
+ resolver.resolve(ccs, projectName, changeNumber, uploader));
workQueue.submit(addReviewers);
} catch (QueryParseException e) {
logger.atWarning().log(
@@ -168,21 +177,31 @@
}
}
- private Set<String> findReviewers(int change, List<ReviewerFilterSection> sections)
+ private Set<String> findReviewers(int change, List<ReviewerFilter> filters)
throws StorageException, QueryParseException {
+ return getReviewersFrom(findMatchingFilters(change, filters));
+ }
+
+ private Set<String> getReviewersFrom(List<ReviewerFilter> filters) throws StorageException {
ImmutableSet.Builder<String> reviewers = ImmutableSet.builder();
- List<ReviewerFilterSection> found = findReviewerSections(change, sections);
- for (ReviewerFilterSection s : found) {
- reviewers.addAll(s.getReviewers());
+ for (ReviewerFilter f : filters) {
+ reviewers.addAll(f.getReviewers());
}
return reviewers.build();
}
- private List<ReviewerFilterSection> findReviewerSections(
- int change, List<ReviewerFilterSection> sections)
+ private Set<String> getCcsFrom(List<ReviewerFilter> filters) throws StorageException {
+ Set<String> ccs = Sets.newHashSet();
+ for (ReviewerFilter f : filters) {
+ ccs.addAll(f.getCcs());
+ }
+ return ccs;
+ }
+
+ private List<ReviewerFilter> findMatchingFilters(int change, List<ReviewerFilter> filters)
throws StorageException, QueryParseException {
- ImmutableList.Builder<ReviewerFilterSection> found = ImmutableList.builder();
- for (ReviewerFilterSection s : sections) {
+ ImmutableList.Builder<ReviewerFilter> found = ImmutableList.builder();
+ for (ReviewerFilter s : filters) {
if (Strings.isNullOrEmpty(s.getFilter()) || s.getFilter().equals("*")) {
found.add(s);
} else if (filterMatch(change, s.getFilter())) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolver.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolver.java
index fc5d9e0..ea7d8e1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolver.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolver.java
@@ -98,8 +98,8 @@
Account.Id id = result.asList().get(0).account().id();
if (uploader == null || id.get() != uploader._accountId) {
reviewers.add(id);
- return true;
}
+ return true;
}
return false;
} catch (StorageException | IOException | ConfigInvalidException e) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/config/ReviewerFilterCollection.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/config/ReviewerFilterCollection.java
new file mode 100644
index 0000000..cd41373
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/config/ReviewerFilterCollection.java
@@ -0,0 +1,86 @@
+// Copyright (C) 2020 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.googlesource.gerrit.plugins.reviewers.config;
+
+import static com.googlesource.gerrit.plugins.reviewers.config.ReviewersConfig.KEY_CC;
+import static com.googlesource.gerrit.plugins.reviewers.config.ReviewersConfig.KEY_REVIEWER;
+import static com.googlesource.gerrit.plugins.reviewers.config.ReviewersConfig.SECTION_FILTER;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import com.googlesource.gerrit.plugins.reviewers.ReviewerFilter;
+import java.util.List;
+import org.eclipse.jgit.lib.Config;
+
+/** Representation of the collection of {@link ReviewerFilter}s in a {@link Config}. */
+class ReviewerFilterCollection {
+
+ private final Config cfg;
+
+ ReviewerFilterCollection(Config cfg) {
+ this.cfg = cfg;
+ }
+
+ List<ReviewerFilter> getAll() {
+ ImmutableList.Builder<ReviewerFilter> b = ImmutableList.builder();
+ for (String f : cfg.getSubsections(SECTION_FILTER)) {
+ b.add(new ReviewerFilterSection(f));
+ }
+ return b.build();
+ }
+
+ ReviewerFilterSection get(String filter) {
+ return new ReviewerFilterSection(filter);
+ }
+
+ class ReviewerFilterSection extends ReviewerFilter {
+
+ private ReviewerFilterSection(String filter) {
+ this.filter = filter;
+ this.reviewers = Sets.newHashSet(cfg.getStringList(SECTION_FILTER, filter, KEY_REVIEWER));
+ this.ccs = Sets.newHashSet(cfg.getStringList(SECTION_FILTER, filter, KEY_CC));
+ }
+
+ public void removeReviewer(String reviewer) {
+ reviewers.remove(reviewer);
+ save();
+ }
+
+ public void addReviewer(String reviewer) {
+ reviewers.add(reviewer);
+ save();
+ }
+
+ public void removeCc(String cc) {
+ ccs.remove(cc);
+ save();
+ }
+
+ public void addCc(String cc) {
+ ccs.add(cc);
+ save();
+ }
+
+ private void save() {
+ if (this.reviewers.isEmpty() && this.ccs.isEmpty()) {
+ cfg.unsetSection(SECTION_FILTER, filter);
+ } else {
+ cfg.setStringList(SECTION_FILTER, filter, KEY_REVIEWER, Lists.newArrayList(this.reviewers));
+ cfg.setStringList(SECTION_FILTER, filter, KEY_CC, Lists.newArrayList(this.ccs));
+ }
+ }
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfig.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/config/ReviewersConfig.java
similarity index 61%
rename from src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfig.java
rename to src/main/java/com/googlesource/gerrit/plugins/reviewers/config/ReviewersConfig.java
index 96a4f6b..c81cbd5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/config/ReviewersConfig.java
@@ -12,11 +12,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.googlesource.gerrit.plugins.reviewers;
+package com.googlesource.gerrit.plugins.reviewers.config;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
@@ -26,9 +25,9 @@
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.reviewers.ReviewerFilter;
+import com.googlesource.gerrit.plugins.reviewers.ReviewerType;
import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Arrays;
import java.util.List;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
@@ -39,9 +38,10 @@
public class ReviewersConfig {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- static final String FILENAME = "reviewers.config";
- static final String SECTION_FILTER = "filter";
- static final String KEY_REVIEWER = "reviewer";
+ @VisibleForTesting public static final String FILENAME = "reviewers.config";
+ @VisibleForTesting public static final String SECTION_FILTER = "filter";
+ @VisibleForTesting public static final String KEY_CC = "cc";
+ @VisibleForTesting public static final String KEY_REVIEWER = "reviewer";
private static final String KEY_ENABLE_REST = "enableREST";
private static final String KEY_SUGGEST_ONLY = "suggestOnly";
private static final String KEY_IGNORE_WIP = "ignoreWip";
@@ -63,7 +63,7 @@
this.ignoreWip = cfg.getBoolean(pluginName, null, KEY_IGNORE_WIP, true);
}
- public ForProject forProject(Project.NameKey projectName) {
+ public List<ReviewerFilter> filtersWithInheritance(Project.NameKey projectName) {
Config cfg;
try {
cfg = cfgFactory.getProjectPluginConfigWithMergedInheritance(projectName, pluginName);
@@ -71,7 +71,7 @@
logger.atSevere().log("Unable to get config for project %s", projectName.get());
cfg = new Config();
}
- return new ForProject(cfg);
+ return new ReviewerFilterCollection(cfg).getAll();
}
public boolean enableREST() {
@@ -86,51 +86,30 @@
return ignoreWip;
}
- static class ForProject extends VersionedMetaData {
+ public static class ForProject extends VersionedMetaData {
private Config cfg;
+ private ReviewerFilterCollection filters;
- ForProject(Config cfg) {
- this.cfg = cfg;
- }
-
- List<ReviewerFilterSection> getReviewerFilterSections() {
- ImmutableList.Builder<ReviewerFilterSection> b = ImmutableList.builder();
- for (String f : cfg.getSubsections(SECTION_FILTER)) {
- b.add(newReviewerFilterSection(f));
- }
- return b.build();
- }
-
- void addReviewer(String filter, String reviewer) {
- if (!newReviewerFilterSection(filter).getReviewers().contains(reviewer)) {
- List<String> values =
- new ArrayList<>(Arrays.asList(cfg.getStringList(SECTION_FILTER, filter, KEY_REVIEWER)));
- values.add(reviewer);
- cfg.setStringList(SECTION_FILTER, filter, KEY_REVIEWER, values);
+ public void addReviewer(String filter, String reviewer, ReviewerType type) {
+ switch (type) {
+ case REVIEWER:
+ filters.get(filter).addReviewer(reviewer);
+ break;
+ case CC:
+ filters.get(filter).addCc(reviewer);
}
}
- void removeReviewer(String filter, String reviewer) {
- if (newReviewerFilterSection(filter).getReviewers().contains(reviewer)) {
- List<String> values =
- new ArrayList<>(Arrays.asList(cfg.getStringList(SECTION_FILTER, filter, KEY_REVIEWER)));
- values.remove(reviewer);
- if (values.isEmpty()) {
- cfg.unsetSection(SECTION_FILTER, filter);
- } else {
- cfg.setStringList(SECTION_FILTER, filter, KEY_REVIEWER, values);
- }
+ public void removeReviewer(String filter, String reviewer, ReviewerType type) {
+ switch (type) {
+ case REVIEWER:
+ filters.get(filter).removeReviewer(reviewer);
+ break;
+ case CC:
+ filters.get(filter).removeCc(reviewer);
}
}
- private ReviewerFilterSection newReviewerFilterSection(String filter) {
- ImmutableSet.Builder<String> b = ImmutableSet.builder();
- for (String reviewer : cfg.getStringList(SECTION_FILTER, filter, KEY_REVIEWER)) {
- b.add(reviewer);
- }
- return new ReviewerFilterSection(filter, b.build());
- }
-
@Override
protected String getRefName() {
return RefNames.REFS_CONFIG;
@@ -138,7 +117,8 @@
@Override
protected void onLoad() throws IOException, ConfigInvalidException {
- cfg = readConfig(FILENAME);
+ this.cfg = readConfig(FILENAME);
+ this.filters = new ReviewerFilterCollection(cfg);
}
@Override
diff --git a/src/main/resources/Documentation/about.md b/src/main/resources/Documentation/about.md
index 9de1838..93c466f 100644
--- a/src/main/resources/Documentation/about.md
+++ b/src/main/resources/Documentation/about.md
@@ -8,6 +8,12 @@
to a change that is in `private` state. Reviewers will only be added to such a
change if it transitions out of `private` state.
+__NOTE__:
+This plugin automatically naively filters out groups that follow the style from
+the singleusergroup plugin.
+This means that groups that start with `user/` will not be suggested, regardless
+of source.
+
SEE ALSO
--------
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index d5f924e..173f51a 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -38,6 +38,7 @@
```
[filter "*"]
reviewer = john.doe@example.com
+ cc = DevGroup
[filter "branch:main file:^lib/.*"]
reviewer = jane.doe@example.com
@@ -55,9 +56,16 @@
that if a group is configured it needs to be visible to the uploader for
reviewers to be added.*
+filter.\<filter\>.cc
+: An account or a group name. Must be an exact match (case sensitive) with the
+ account's email address or username, or the group name. Multiple `cc`
+ occurrences are allowed.
+
##Multiple filter matches
The plugin supports multiple filter matches.
+If a reviewer, according to filter matches, should be added as both `reviewer` and `cc`,
+`reviewer` takes precedence.
###Example
diff --git a/src/main/resources/Documentation/rest-api.md b/src/main/resources/Documentation/rest-api.md
index ebd04d4..957318f 100644
--- a/src/main/resources/Documentation/rest-api.md
+++ b/src/main/resources/Documentation/rest-api.md
@@ -20,7 +20,7 @@
GET /projects/myproject/@PLUGIN@ HTTP/1.0
```
-As response a List of [ReviewerFilterSection](#reviewer-filter-section) is returned
+As response a List of [ReviewerFilter](#reviewer-filter) is returned
that describes the default reviewers for myproject.
#### Response
@@ -36,6 +36,9 @@
"reviewers": [
"UserA",
"UserB"
+ ],
+ "cc": [
+ "DevGroup"
]
},
{
@@ -66,13 +69,14 @@
Content-Type: application/json;charset=UTF-8
{
"action": "ADD",
- "filter": "branch:master"
- "reviewer": "UserA"
+ "type": "REVIEWER"
+ "filter": "branch:master",
+ "reviewer": "UserA",
}
```
As response the default reviewers are returned as a list of
-[ReviewerFilterSection](#reviewer-filter-section).
+[ReviewerFilter](#reviewer-filter).
#### Response
@@ -88,6 +92,9 @@
"UserA",
"UserB"
]
+ "ccs": [
+ "DevGroup"
+ ]
},
{
"filter": "file:^lib/*",
@@ -104,14 +111,15 @@
<a id="json-entities">JSON Entities
-----------------------------------
-### <a id="reviewer-filter-section"></a>ReviewerFilterSection
+### <a id="reviewer-filter"></a>ReviewerFilter
-The `ReviewerFilterSection` entity contains a filter section of the
-default reviewers.
+The `ReviewerFilter` entity contains a filter of the default reviewers.
* _filter_: A filter that is used to assign default reviewers.
* _reviewers_: List of usernames which are assigned as default reviewers
under the filter.
+* _ccs_: List of usernames which are assigned as default ccs
+ under the filter.
### <a id="config-reviewers-input"></a>ConfigReviewersInput
@@ -119,6 +127,7 @@
reviewers.
* _action_: Indicates whether to add or remove the input reviewer
+* _type_: Which type to add/remove the user as, (REVIEWER|CC), defaults to REVIEWER.
* _filter_: The filter associated with the input reviewer.
* _reviewer_: The user to add or remove from the default reviewers.
diff --git a/src/test/java/com/googlesource/gerrit/plugins/reviewers/AbstractReviewersPluginTest.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/AbstractReviewersPluginTest.java
index a8b308a..1407b48 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/reviewers/AbstractReviewersPluginTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/AbstractReviewersPluginTest.java
@@ -15,19 +15,20 @@
package com.googlesource.gerrit.plugins.reviewers;
import static com.google.gerrit.acceptance.GitUtil.fetch;
-import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.FILENAME;
-import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.KEY_REVIEWER;
-import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.SECTION_FILTER;
+import static com.googlesource.gerrit.plugins.reviewers.config.ReviewersConfig.FILENAME;
+import static com.googlesource.gerrit.plugins.reviewers.config.ReviewersConfig.KEY_CC;
+import static com.googlesource.gerrit.plugins.reviewers.config.ReviewersConfig.KEY_REVIEWER;
+import static com.googlesource.gerrit.plugins.reviewers.config.ReviewersConfig.SECTION_FILTER;
-import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.entities.RefNames;
import com.google.inject.Inject;
import java.util.Arrays;
-import java.util.List;
+import java.util.Set;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.junit.Ignore;
@@ -37,16 +38,21 @@
public class AbstractReviewersPluginTest extends LightweightPluginDaemonTest {
@Inject protected ProjectOperations projectOperations;
- protected void createFilters(FilterData... filters) throws Exception {
+ protected void createFilters(TestFilter... filters) throws Exception {
createFiltersFor(testRepo, filters);
}
- protected void createFiltersFor(TestRepository<?> repo, FilterData... filters) throws Exception {
+ protected void createFiltersFor(TestRepository<?> repo, TestFilter... filters) throws Exception {
String previousHead = repo.getRepository().getBranch();
checkoutRefsMetaConfig(repo);
Config cfg = new Config();
Arrays.stream(filters)
- .forEach(f -> cfg.setStringList(SECTION_FILTER, f.filter, KEY_REVIEWER, f.reviewers));
+ .forEach(
+ f -> {
+ cfg.setStringList(
+ SECTION_FILTER, f.filter, KEY_REVIEWER, Lists.newArrayList(f.reviewers));
+ cfg.setStringList(SECTION_FILTER, f.filter, KEY_CC, Lists.newArrayList(f.ccs));
+ });
pushFactory
.create(admin.newIdent(), repo, "Add reviewers", FILENAME, cfg.toText())
.to(RefNames.REFS_CONFIG)
@@ -60,31 +66,42 @@
return repo;
}
- protected FilterData filter(String filter) {
- return new FilterData(filter);
+ private TestFilter filter(String filter, Set<String> reviewers, Set<String> ccs) {
+ return new TestFilter(filter, reviewers, ccs);
}
- /** Assists tests to define a filter. */
- protected static class FilterData {
- List<String> reviewers;
- String filter;
+ protected TestFilter filter(String filter) {
+ return new TestFilter(filter);
+ }
- FilterData(String filter) {
+ protected static class TestFilter extends ReviewerFilter {
+
+ public TestFilter(String filter, Set<String> reviewers, Set<String> ccs) {
this.filter = filter;
- this.reviewers = Lists.newArrayList();
+ this.reviewers = reviewers;
+ this.ccs = ccs;
}
- FilterData reviewer(TestAccount reviewer) {
- return reviewer(reviewer.email());
+ public TestFilter(String filter) {
+ this(filter, Sets.newHashSet(), Sets.newHashSet());
}
- FilterData reviewer(String reviewer) {
- reviewers.add(reviewer);
+ public TestFilter reviewer(String reviewerId) {
+ reviewers.add(reviewerId);
return this;
}
- public ReviewerFilterSection asSection() {
- return new ReviewerFilterSection(filter, ImmutableSet.copyOf(reviewers));
+ public TestFilter reviewer(TestAccount reviewer) {
+ return reviewer(reviewer.email());
+ }
+
+ public TestFilter cc(String ccId) {
+ ccs.add(ccId);
+ return this;
+ }
+
+ public TestFilter cc(TestAccount cc) {
+ return cc(cc.email());
}
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
index a2a7035..02f4902 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
@@ -15,6 +15,7 @@
package com.googlesource.gerrit.plugins.reviewers;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.extensions.client.ReviewerState.CC;
import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
import static java.util.stream.Collectors.toSet;
@@ -25,8 +26,10 @@
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.client.ChangeStatus;
+import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.ChangeInfo;
import java.util.Set;
import org.junit.Test;
@@ -38,10 +41,19 @@
@Test
public void addReviewers() throws Exception {
TestAccount user2 = accountCreator.user2();
- createFilters(filter("*").reviewer(user).reviewer(user2));
+ createFilters(filter("*").reviewer(user).cc(user2));
String changeId = createChange().getChangeId();
- assertThat(reviewersFor(changeId))
- .containsExactlyElementsIn(ImmutableSet.of(user.id(), user2.id()));
+ assertThat(reviewersFor(changeId)).containsExactlyElementsIn(ImmutableSet.of(user.id()));
+ assertThat(ccsFor(changeId)).containsExactlyElementsIn(ImmutableSet.of(user2.id()));
+ }
+
+ @Test
+ public void addReviewerMatchingReviewerAndCc() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+ createFilters(filter("*").cc(user).cc(user2), filter("^a.txt").reviewer(user2));
+ String changeId = createChange().getChangeId();
+ assertThat(reviewersFor(changeId)).containsExactlyElementsIn(ImmutableSet.of(user2.id()));
+ assertThat(ccsFor(changeId)).containsExactlyElementsIn(ImmutableSet.of(user.id()));
}
@Test
@@ -54,8 +66,9 @@
}
@Test
- public void doNotAddReviewersFromNonMatchingFilters() throws Exception {
- createFilters(filter("branch:master").reviewer(user));
+ public void doNotAddReviewersOrCcFromNonMatchingFilters() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+ createFilters(filter("branch:master").reviewer(user).cc(user2));
createBranch(BranchNameKey.create(project, "other-branch"));
// Create a change that matches the filter section.
createChange("refs/for/master");
@@ -113,13 +126,60 @@
assertThat(reviewersFor(changeId)).containsExactlyElementsIn(ImmutableSet.of(user.id()));
}
+ @Test
+ public void dontChangeExistingStateOfReviewer() throws Exception {
+ Set<Account.Id> userSet = ImmutableSet.of(user.id());
+ TestAccount user2 = accountCreator.user2();
+ Set<Account.Id> user2Set = ImmutableSet.of(user2.id());
+
+ createFilters(filter("*").reviewer(user2).cc(user));
+ String changeId = createChange("refs/for/master").getChangeId();
+ assertThat(ccsFor(changeId)).containsExactlyElementsIn(userSet);
+ assertThat(reviewersFor(changeId)).containsExactlyElementsIn(user2Set);
+
+ addReviewer(changeId, user, ReviewerState.REVIEWER);
+ addReviewer(changeId, user2, ReviewerState.CC);
+ assertThat(reviewersFor(changeId)).containsExactlyElementsIn(userSet);
+ assertThat(ccsFor(changeId)).containsExactlyElementsIn(user2Set);
+
+ amendChange(changeId);
+ assertThat(reviewersFor(changeId)).containsExactlyElementsIn(userSet);
+ assertThat(ccsFor(changeId)).containsExactlyElementsIn(user2Set);
+ }
+
+ @Test
+ public void addAsReviewerIfConfiguredAsReviewerAndCc() throws Exception {
+ createFilters(filter("*").cc(user), filter("branch:master").reviewer(user));
+ String changeId = createChange("refs/for/master").getChangeId();
+ assertThat(reviewersFor(changeId)).containsExactlyElementsIn(ImmutableSet.of(user.id()));
+ assertThat(gApi.changes().id(changeId).get().reviewers.get(CC)).isNull();
+ }
+
+ private void addReviewer(String changeId, TestAccount user, ReviewerState state)
+ throws Exception {
+ AddReviewerInput input = new AddReviewerInput();
+ input.reviewer = user.id().toString();
+ input.state = state;
+ gApi.changes().id(changeId).addReviewer(input);
+ }
+
+ private Set<Account.Id> ccsFor(String changeId) throws Exception {
+ return reviewersFor(changeId, CC);
+ }
+
private Set<Account.Id> reviewersFor(String changeId) throws Exception {
- return gApi.changes().id(changeId).get().reviewers.get(REVIEWER).stream()
+ return reviewersFor(changeId, REVIEWER);
+ }
+
+ private Set<Account.Id> reviewersFor(String changeId, ReviewerState reviewerState)
+ throws Exception {
+ return gApi.changes().id(changeId).get().reviewers.get(reviewerState).stream()
.map(a -> Account.id(a._accountId))
.collect(toSet());
}
private void assertNoReviewersAddedFor(String changeId) throws Exception {
assertThat(gApi.changes().id(changeId).get().reviewers.get(REVIEWER)).isNull();
+ assertThat(gApi.changes().id(changeId).get().reviewers.get(CC)).isNull();
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfigIT.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/config/ReviewersConfigIT.java
similarity index 65%
rename from src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfigIT.java
rename to src/test/java/com/googlesource/gerrit/plugins/reviewers/config/ReviewersConfigIT.java
index 19bb268..cd593c8 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfigIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/config/ReviewersConfigIT.java
@@ -12,15 +12,15 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.googlesource.gerrit.plugins.reviewers;
+package com.googlesource.gerrit.plugins.reviewers.config;
-import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.entities.Project;
-import java.util.Arrays;
+import com.googlesource.gerrit.plugins.reviewers.AbstractReviewersPluginTest;
import org.eclipse.jgit.junit.TestRepository;
import org.junit.Before;
import org.junit.Test;
@@ -47,28 +47,39 @@
}
@Test
- public void reviewersConfigWithMergedInheritance() throws Exception {
+ public void reviewersConfigSingleWithCc() throws Exception {
+ createFilters(filter(NO_FILTER).reviewer(JOHN_DOE).cc(JANE_DOE));
+
+ assertProjectHasFilters(project, filter(NO_FILTER).reviewer(JOHN_DOE).cc(JANE_DOE));
+ }
+
+ @Test
+ public void reviewersConfigSingleWithCcSeparateFilters() throws Exception {
+ createFilters(filter(NO_FILTER).reviewer(JOHN_DOE), filter(BRANCH_MAIN).cc(JANE_DOE));
+
+ assertProjectHasFilters(
+ project, filter(NO_FILTER).reviewer(JOHN_DOE), filter(BRANCH_MAIN).cc(JANE_DOE));
+ }
+
+ @Test
+ public void reviewersConfigWithMergedInheritanceWithCc() throws Exception {
Project.NameKey childProject = projectOperations.newProject().parent(project).create();
TestRepository<?> childTestRepo = checkoutRefsMetaConfig(cloneProject(childProject));
createFiltersFor(
- childTestRepo,
- filter(NO_FILTER).reviewer(JANE_DOE),
- filter(BRANCH_MAIN).reviewer(JANE_DOE));
+ childTestRepo, filter(NO_FILTER).reviewer(JANE_DOE), filter(BRANCH_MAIN).cc(JANE_DOE));
- createFilters(filter(NO_FILTER).reviewer(JOHN_DOE), filter(BRANCH_MAIN).reviewer(JOHN_DOE));
+ createFilters(filter(NO_FILTER).cc(JOHN_DOE), filter(BRANCH_MAIN).reviewer(JOHN_DOE));
assertProjectHasFilters(
childProject,
- filter(NO_FILTER).reviewer(JOHN_DOE).reviewer(JANE_DOE),
- filter(BRANCH_MAIN).reviewer(JOHN_DOE).reviewer(JANE_DOE));
+ filter(NO_FILTER).cc(JOHN_DOE).reviewer(JANE_DOE),
+ filter(BRANCH_MAIN).reviewer(JOHN_DOE).cc(JANE_DOE));
}
- private void assertProjectHasFilters(Project.NameKey project, FilterData... filters) {
- assertThat(reviewersConfig().forProject(project).getReviewerFilterSections())
- .containsExactlyElementsIn(
- Arrays.stream(filters).map(f -> f.asSection()).collect(toImmutableList()))
- .inOrder();
+ private void assertProjectHasFilters(Project.NameKey project, TestFilter... filters) {
+ assertThat(reviewersConfig().filtersWithInheritance(project))
+ .containsExactlyElementsIn(ImmutableList.copyOf(filters));
}
private ReviewersConfig reviewersConfig() {
