opendev/gerrit
Code Issues Proposed changes

Make reviewer.enableByEmail inheritable

Browse Source 
This change makes the config inheritable, adds it to PutConfig
and adapts the test code and docs where needed.

The GWT UI is adapted such that the config can be set using the UI
control.

Change-Id: I7b4a52eebbb69c44c1095a6a4ea3e402d1eb4206
This commit is contained in:
Patrick Hiesel 2017-03-29 14:19:23 +02:00
parent 8d2b2bb95c
commit 502b26ca18
11 changed files with 52 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Documentation/config-project-config.txt
View File

@ -302,7 +302,9 @@ reviewers and CCs by email.
A boolean indicating if reviewers and CCs that do not currently have a Gerrit
account can be added to a change by providing their email address.
Defaults to `false`.
Default is `INHERIT`, which means that this property is inherited from
the parent project. If the property is not set in any parent project, the
default value is `FALSE`.
[[file-groups]]
== The file +groups+

15
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
View File

@ -25,13 +25,14 @@ import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.projects.ConfigInput;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.testutil.FakeEmailSender.Message;
import java.util.EnumSet;
@ -44,9 +45,9 @@ public class ChangeReviewersByEmailIT extends AbstractDaemonTest {
@Before
public void setUp() throws Exception {
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
cfg.setEnableReviewerByEmail(true);
saveProjectConfig(project, cfg);
ConfigInput conf = new ConfigInput();
conf.enableReviewerByEmail = InheritableBoolean.TRUE;
gApi.projects().name(project.get()).config(conf);
}
@Test
@ -228,9 +229,9 @@ public class ChangeReviewersByEmailIT extends AbstractDaemonTest {
public void rejectWhenFeatureIsDisabled() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
cfg.setEnableReviewerByEmail(false);
saveProjectConfig(project, cfg);
ConfigInput conf = new ConfigInput();
conf.enableReviewerByEmail = InheritableBoolean.FALSE;
gApi.projects().name(project.get()).config(conf);
PushOneCommit.Result r = createChange();

1
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/ConfigInput.java
View File

@ -29,6 +29,7 @@ public class ConfigInput {
public InheritableBoolean enableSignedPush;
public InheritableBoolean requireSignedPush;
public InheritableBoolean rejectImplicitMerges;
public InheritableBoolean enableReviewerByEmail;
public String maxObjectSizeLimit;
public SubmitType submitType;
public ProjectState state;

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectInfoScreen.java
View File

@ -672,6 +672,7 @@ public class ProjectInfoScreen extends ProjectScreen {
esp,
rsp,
getBool(rejectImplicitMerges),
getBool(enableReviewerByEmail),
maxObjectSizeLimit.getText().trim(),
SubmitType.valueOf(submitType.getValue(submitType.getSelectedIndex())),
ProjectState.valueOf(state.getValue(state.getSelectedIndex())),

9
gerrit-gwtui/src/main/java/com/google/gerrit/client/projects/ProjectApi.java
View File

@ -147,6 +147,7 @@ public class ProjectApi {
InheritableBoolean enableSignedPush,
InheritableBoolean requireSignedPush,
InheritableBoolean rejectImplicitMerges,
InheritableBoolean enableReviewerByEmail,
String maxObjectSizeLimit,
SubmitType submitType,
ProjectState state,
@ -170,6 +171,7 @@ public class ProjectApi {
in.setSubmitType(submitType);
in.setState(state);
in.setPluginConfigValues(pluginConfigValues);
in.setEnableReviewerByEmail(enableReviewerByEmail);
project(name).view("config").put(in, cb);
}
@ -294,6 +296,13 @@ public class ProjectApi {
setRequireSignedPushRaw(v.name());
}
final void setEnableReviewerByEmail(InheritableBoolean v) {
setEnableReviewerByEmailRaw(v.name());
}
private native void setEnableReviewerByEmailRaw(String v)
/*-{ if(v)this.enable_reviewer_by_email=v; }-*/ ;
private native void setRequireSignedPushRaw(String v)
/*-{ if(v)this.require_signed_push=v; }-*/ ;

4
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Project.java
View File

@ -161,6 +161,10 @@ public final class Project {
return enableReviewerByEmail;
}
public void setEnableReviewerByEmail(final InheritableBoolean enable) {
enableReviewerByEmail = enable;
}
public void setUseContributorAgreements(final InheritableBoolean u) {
useContributorAgreements = u;
}

8
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
View File

@ -55,7 +55,6 @@ import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.account.GroupMembers;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.extensions.events.ReviewerAdded;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.group.GroupsCollection;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.mail.Address;
@ -189,19 +188,20 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
try {
accountId = accounts.parse(input.reviewer).getAccountId();
} catch (UnprocessableEntityException e) {
ProjectConfig projectConfig = projectCache.checkedGet(rsrc.getProject()).getConfig();
boolean enableReviewerByEmail =
projectCache.checkedGet(rsrc.getProject()).isEnableReviewerByEmail();
if (allowGroup) {
try {
return putGroup(rsrc, input);
} catch (UnprocessableEntityException e2) {
if (!projectConfig.getEnableReviewerByEmail()) {
if (!enableReviewerByEmail) {
throw new UnprocessableEntityException(
MessageFormat.format(
ChangeMessages.get().reviewerNotFoundUserOrGroup, input.reviewer));
}
}
}
if (!projectConfig.getEnableReviewerByEmail()) {
if (!enableReviewerByEmail) {
throw new UnprocessableEntityException(
MessageFormat.format(ChangeMessages.get().reviewerNotFoundUser, input.reviewer));
}

31
gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
View File

@ -188,7 +188,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
private boolean checkReceivedObjects;
private Set<String> sectionsWithUnknownPermissions;
private boolean hasLegacyPermissions;
private boolean enableReviewerByEmail;
private Map<String, List<String>> extensionPanelSections;
public static ProjectConfig read(MetaDataUpdate update)
@ -447,16 +446,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
return checkReceivedObjects;
}
/** @return the enableReviewerByEmail for this project, default is false. */
public boolean getEnableReviewerByEmail() {
return enableReviewerByEmail;
}
/** Set enableReviewerByEmail for this project, default is false. */
public void setEnableReviewerByEmail(boolean val) {
enableReviewerByEmail = val;
}
/**
* Check all GroupReferences use current group name, repairing stale ones.
*
@ -529,6 +518,8 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
p.setMaxObjectSizeLimit(rc.getString(RECEIVE, null, KEY_MAX_OBJECT_SIZE_LIMIT));
p.setRejectImplicitMerges(
getEnum(rc, RECEIVE, null, KEY_REJECT_IMPLICIT_MERGES, InheritableBoolean.INHERIT));
p.setEnableReviewerByEmail(
getEnum(rc, REVIEWER, null, KEY_ENABLE_REVIEWER_BY_EMAIL, InheritableBoolean.INHERIT));
p.setSubmitType(getEnum(rc, SUBMIT, null, KEY_ACTION, DEFAULT_SUBMIT_ACTION));
p.setUseContentMerge(getEnum(rc, SUBMIT, null, KEY_MERGE_CONTENT, InheritableBoolean.INHERIT));
@ -548,7 +539,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
mimeTypes = new ConfiguredMimeTypes(projectName.get(), rc);
loadPluginSections(rc);
loadReceiveSection(rc);
loadReviewerSection(rc);
loadExtensionPanelSections(rc);
}
@ -976,10 +966,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
maxObjectSizeLimit = rc.getLong(RECEIVE, null, KEY_MAX_OBJECT_SIZE_LIMIT, 0);
}
private void loadReviewerSection(Config rc) {
enableReviewerByEmail = rc.getBoolean(REVIEWER, null, KEY_ENABLE_REVIEWER_BY_EMAIL, false);
}
private void loadPluginSections(Config rc) {
pluginConfigs = new HashMap<>();
for (String plugin : rc.getSubsections(PLUGIN)) {
@ -1099,6 +1085,13 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
KEY_REJECT_IMPLICIT_MERGES,
p.getRejectImplicitMerges(),
InheritableBoolean.INHERIT);
set(
rc,
REVIEWER,
null,
KEY_ENABLE_REVIEWER_BY_EMAIL,
p.getEnableReviewerByEmail(),
InheritableBoolean.INHERIT);
set(rc, SUBMIT, null, KEY_ACTION, p.getSubmitType(), DEFAULT_SUBMIT_ACTION);
set(rc, SUBMIT, null, KEY_MERGE_CONTENT, p.getUseContentMerge(), InheritableBoolean.INHERIT);
@ -1114,7 +1107,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
saveAccessSections(rc, keepGroups);
saveNotifySections(rc, keepGroups);
savePluginSections(rc, keepGroups);
saveReviewerSection(rc);
groupList.retainUUIDs(keepGroups);
saveLabelSections(rc);
saveSubscribeSections(rc);
@ -1430,11 +1422,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
}
}
private void saveReviewerSection(Config rc) {
setBooleanConfigKey(
rc, REVIEWER, null, KEY_ENABLE_REVIEWER_BY_EMAIL, enableReviewerByEmail, false);
}
private void saveGroupList() throws IOException {
saveUTF8(GroupList.FILE_NAME, groupList.asText());
}

4
gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java
View File

@ -154,6 +154,10 @@ public class PutConfig implements RestModifyView<ProjectResource, ConfigInput> {
p.setState(input.state);
}
if (input.enableReviewerByEmail != null) {
p.setEnableReviewerByEmail(input.enableReviewerByEmail);
}
if (input.pluginConfigValues != null) {
setPluginConfigValues(ctrl.getProjectState(), projectConfig, input.pluginConfigValues);
}

6
gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java
View File

@ -105,9 +105,7 @@ public class ProjectConfigTest extends LocalDiskRepositoryTestCase {
+ " accepted = group Developers\n" //
+ " accepted = group Staff\n" //
+ " autoVerify = group Developers\n" //
+ " agreementUrl = http://www.example.com/agree\n" //
+ "[reviewer]\n" //
+ " enableByEmail = true\n")) //
+ " agreementUrl = http://www.example.com/agree\n")) //
));
ProjectConfig cfg = read(rev);
@ -134,8 +132,6 @@ public class ProjectConfigTest extends LocalDiskRepositoryTestCase {
assertThat(submit.getExclusiveGroup()).isTrue();
assertThat(read.getExclusiveGroup()).isTrue();
assertThat(push.getExclusiveGroup()).isFalse();
assertThat(cfg.getEnableReviewerByEmail()).isTrue();
}
@Test

23
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@ -44,6 +44,8 @@ import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput;
import com.google.gerrit.extensions.api.changes.StarsInput;
import com.google.gerrit.extensions.api.groups.GroupInput;
import com.google.gerrit.extensions.api.projects.ConfigInput;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
@ -72,7 +74,6 @@ import com.google.gerrit.server.change.ChangeTriplet;
import com.google.gerrit.server.change.PatchSetInserter;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.index.IndexConfig;
import com.google.gerrit.server.index.QueryOptions;
@ -1566,9 +1567,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
Project.NameKey project = new Project.NameKey("repo");
TestRepository<Repo> repo = createProject(project.get());
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
cfg.setEnableReviewerByEmail(true);
saveProjectConfig(project, cfg);
ConfigInput conf = new ConfigInput();
conf.enableReviewerByEmail = InheritableBoolean.TRUE;
gApi.projects().name(project.get()).config(conf);
String userByEmail = "un.registered@reviewer.com";
String userByEmailWithName = "John Doe <" + userByEmail + ">";
@ -1601,9 +1602,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
Project.NameKey project = new Project.NameKey("repo");
TestRepository<Repo> repo = createProject(project.get());
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
cfg.setEnableReviewerByEmail(true);
saveProjectConfig(project, cfg);
ConfigInput conf = new ConfigInput();
conf.enableReviewerByEmail = InheritableBoolean.TRUE;
gApi.projects().name(project.get()).config(conf);
String userByEmail = "John Doe <un.registered@reviewer.com>";
@ -2127,12 +2128,4 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
Patch.COMMIT_MSG, ImmutableList.<ReviewInput.CommentInput>of(comment));
gApi.changes().id(changeId).current().review(input);
}
private void saveProjectConfig(Project.NameKey p, ProjectConfig cfg) throws Exception {
try (MetaDataUpdate md = metaDataUpdateFactory.create(p)) {
md.setAuthor(userFactory.create(userId));
cfg.commit(md);
}
projectCache.evict(cfg.getProject());
}
}