opendev/gerrit
Code Issues Proposed changes

Merge changes from topic "private-by-default"

Browse Source 
* changes:
  PolyGerrit: Add "Set all new changes private by default" to gr-project
  Implement 'privateByDefault' option for project.config
This commit is contained in:
David Pursehouse 2017-08-23 04:44:14 +00:00 committed by Gerrit Code Review
commit 4a88a7a1e1
21 changed files with 176 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
Documentation/config-gerrit.txt
View File

@ -1193,16 +1193,6 @@ Zero or negative values allow robot comments of unlimited size.
+
The default limit is 1024kB.
[[change.privateByDefault]]change.privateByDefault::
+
If set to true, every change created will be private by default.
+
Note that the newly created change will be public if the `is_private` field in
link:rest-api-changes.html#change-input[ChangeInput] is set to `false` explicitly
or the `remove-private` link:user-upload.html#private[PushOption] is used in the push.
+
The default is false.
[[changeCleanup]]
=== Section changeCleanup

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

@ -199,6 +199,24 @@ the implicit merge check.
Default is `INHERIT`, which means that this property is inherited from
the parent project.
[[change-section]]
=== Change section
The change section includes configuration for project-specific change settings:
[[change.privateByDefault]]change.privateByDefault::
+
Controls whether all new changes in the project are set as private by default.
+
Note that a new change will be public if the `is_private` field in
link:rest-api-changes.html#change-input[ChangeInput] is set to `false` explicitly
when calling the link:rest-api-changes.html#create-change[CreateChange] REST API
or the `remove-private` link:user-upload.html#private[PushOption] is used during
the Git push.
+
Default is `INHERIT`, which means that this property is inherited from
the parent project.
[[submit-section]]
=== Submit section

3
Documentation/rest-api-projects.txt
View File

@ -2573,6 +2573,9 @@ signed push validation is required on the project.
|`reject_implicit_merges`|optional|
link:#inherited-boolean-info[InheritedBooleanInfo] that tells whether
implicit merges should be rejected on changes pushed to the project.
|`private_by_default` ||
link:#inherited-boolean-info[InheritedBooleanInfo] that tells whether
all new changes are set as private by default.
|`max_object_size_limit` ||
The link:config-gerrit.html#receive.maxObjectSizeLimit[max object size
limit] of this project as a link:#max-object-size-limit-info[

77
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/PrivateByDefaultIT.java
View File

@ -17,23 +17,39 @@ package com.google.gerrit.acceptance.rest.change;
import static com.google.common.truth.Truth.assertThat;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.extensions.api.projects.ConfigInput;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.reviewdb.client.Project;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.junit.Before;
import org.junit.Test;
public class PrivateByDefaultIT extends AbstractDaemonTest {
private Project.NameKey project1;
private Project.NameKey project2;
@Before
public void setUp() throws Exception {
project1 = createProject("project-1");
project2 = createProject("project-2", project1);
setPrivateByDefault(project1, InheritableBoolean.FALSE);
}
@Test
@GerritConfig(name = "change.privateByDefault", value = "true")
public void createChangeWithPrivateByDefaultEnabled() throws Exception {
ChangeInput input = new ChangeInput(project.get(), "master", "empty change");
setPrivateByDefault(project2, InheritableBoolean.TRUE);
ChangeInput input = new ChangeInput(project2.get(), "master", "empty change");
assertThat(gApi.changes().create(input).get().isPrivate).isEqualTo(true);
}
@Test
@GerritConfig(name = "change.privateByDefault", value = "true")
public void createChangeBypassPrivateByDefaultEnabled() throws Exception {
ChangeInput input = new ChangeInput(project.get(), "master", "empty change");
setPrivateByDefault(project2, InheritableBoolean.TRUE);
ChangeInput input = new ChangeInput(project2.get(), "master", "empty change");
input.isPrivate = false;
assertThat(gApi.changes().create(input).get().isPrivate).isNull();
}
@ -41,25 +57,62 @@ public class PrivateByDefaultIT extends AbstractDaemonTest {
@Test
public void createChangeWithPrivateByDefaultDisabled() throws Exception {
ChangeInfo info =
gApi.changes().create(new ChangeInput(project.get(), "master", "empty change")).get();
gApi.changes().create(new ChangeInput(project2.get(), "master", "empty change")).get();
assertThat(info.isPrivate).isNull();
}
@Test
@GerritConfig(name = "change.privateByDefault", value = "true")
public void pushWithPrivateByDefaultEnabled() throws Exception {
assertThat(createChange().getChange().change().isPrivate()).isEqualTo(true);
public void createChangeWithPrivateByDefaultInherited() throws Exception {
setPrivateByDefault(project1, InheritableBoolean.TRUE);
ChangeInfo info =
gApi.changes().create(new ChangeInput(project2.get(), "master", "empty change")).get();
assertThat(info.isPrivate).isTrue();
}
@Test
public void pushWithPrivateByDefaultEnabled() throws Exception {
setPrivateByDefault(project2, InheritableBoolean.TRUE);
assertThat(createChange(project2).getChange().change().isPrivate()).isEqualTo(true);
}
@Test
@GerritConfig(name = "change.privateByDefault", value = "true")
public void pushBypassPrivateByDefaultEnabled() throws Exception {
assertThat(createChange("refs/for/master%remove-private").getChange().change().isPrivate())
setPrivateByDefault(project2, InheritableBoolean.TRUE);
assertThat(
createChange(project2, "refs/for/master%remove-private")
.getChange()
.change()
.isPrivate())
.isEqualTo(false);
}
@Test
public void pushWithPrivateByDefaultDisabled() throws Exception {
assertThat(createChange().getChange().change().isPrivate()).isEqualTo(false);
assertThat(createChange(project2).getChange().change().isPrivate()).isEqualTo(false);
}
@Test
public void pushBypassPrivateByDefaultInherited() throws Exception {
setPrivateByDefault(project1, InheritableBoolean.TRUE);
assertThat(createChange(project2).getChange().change().isPrivate()).isEqualTo(true);
}
private void setPrivateByDefault(Project.NameKey proj, InheritableBoolean value)
throws Exception {
ConfigInput input = new ConfigInput();
input.privateByDefault = value;
gApi.projects().name(proj.get()).config(input);
}
private PushOneCommit.Result createChange(Project.NameKey proj) throws Exception {
return createChange(proj, "refs/for/master");
}
private PushOneCommit.Result createChange(Project.NameKey proj, String ref) throws Exception {
TestRepository<InMemoryRepository> testRepo = cloneProject(proj);
PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
PushOneCommit.Result result = push.to(ref);
result.assertOkStatus();
return result;
}
}

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

@ -31,6 +31,7 @@ public class ConfigInfo {
public InheritedBooleanInfo enableSignedPush;
public InheritedBooleanInfo requireSignedPush;
public InheritedBooleanInfo rejectImplicitMerges;
public InheritedBooleanInfo privateByDefault;
public InheritedBooleanInfo enableReviewerByEmail;
public InheritedBooleanInfo matchAuthorToCommitterDate;
public MaxObjectSizeLimitInfo maxObjectSizeLimit;

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 privateByDefault;
public InheritableBoolean enableReviewerByEmail;
public InheritableBoolean matchAuthorToCommitterDate;
public String maxObjectSizeLimit;

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
View File

@ -75,6 +75,8 @@ public interface AdminConstants extends Constants {
String rejectImplicitMerges();
String privateByDefault();
String enableReviewerByEmail();
String matchAuthorToCommitterDate();

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

@ -29,6 +29,7 @@ enableSignedPush = Enable signed push
requireSignedPush = Require signed push
requireChangeID = Require <code>Change-Id</code> in commit message
rejectImplicitMerges = Reject implicit merges when changes are pushed for review
privateByDefault = Set all new changes private by default
headingMaxObjectSizeLimit = Maximum Git object size limit
headingGroupOptions = Group Options
isVisibleToAll = Make group visible to all registered users.

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

@ -86,6 +86,7 @@ public class ProjectInfoScreen extends ProjectScreen {
private ListBox enableSignedPush;
private ListBox requireSignedPush;
private ListBox rejectImplicitMerges;
private ListBox privateByDefault;
private ListBox enableReviewerByEmail;
private ListBox matchAuthorToCommitterDate;
private NpTextBox maxObjectSizeLimit;
@ -192,6 +193,7 @@ public class ProjectInfoScreen extends ProjectScreen {
signedOffBy.setEnabled(isOwner);
requireChangeID.setEnabled(isOwner);
rejectImplicitMerges.setEnabled(isOwner);
privateByDefault.setEnabled(isOwner);
maxObjectSizeLimit.setEnabled(isOwner);
enableReviewerByEmail.setEnabled(isOwner);
matchAuthorToCommitterDate.setEnabled(isOwner);
@ -268,6 +270,10 @@ public class ProjectInfoScreen extends ProjectScreen {
saveEnabler.listenTo(rejectImplicitMerges);
grid.addHtml(AdminConstants.I.rejectImplicitMerges(), rejectImplicitMerges);
privateByDefault = newInheritedBooleanBox();
saveEnabler.listenTo(privateByDefault);
grid.addHtml(AdminConstants.I.privateByDefault(), privateByDefault);
enableReviewerByEmail = newInheritedBooleanBox();
saveEnabler.listenTo(enableReviewerByEmail);
grid.addHtml(AdminConstants.I.enableReviewerByEmail(), enableReviewerByEmail);
@ -407,6 +413,7 @@ public class ProjectInfoScreen extends ProjectScreen {
setBool(requireSignedPush, result.requireSignedPush());
}
setBool(rejectImplicitMerges, result.rejectImplicitMerges());
setBool(privateByDefault, result.privateByDefault());
setBool(enableReviewerByEmail, result.enableReviewerByEmail());
setBool(matchAuthorToCommitterDate, result.matchAuthorToCommitterDate());
setSubmitType(result.submitType());
@ -679,6 +686,7 @@ public class ProjectInfoScreen extends ProjectScreen {
esp,
rsp,
getBool(rejectImplicitMerges),
getBool(privateByDefault),
getBool(enableReviewerByEmail),
getBool(matchAuthorToCommitterDate),
maxObjectSizeLimit.getText().trim(),

3
gerrit-gwtui/src/main/java/com/google/gerrit/client/projects/ConfigInfo.java
View File

@ -57,6 +57,9 @@ public class ConfigInfo extends JavaScriptObject {
public final native InheritedBooleanInfo rejectImplicitMerges()
/*-{ return this.reject_implicit_merges; }-*/ ;
public final native InheritedBooleanInfo privateByDefault()
/*-{ return this.private_by_default; }-*/ ;
public final native InheritedBooleanInfo enableReviewerByEmail()
/*-{ return this.enable_reviewer_by_email; }-*/ ;

8
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 privateByDefault,
InheritableBoolean enableReviewerByEmail,
InheritableBoolean matchAuthorToCommitterDate,
String maxObjectSizeLimit,
@ -168,6 +169,7 @@ public class ProjectApi {
in.setRequireSignedPush(requireSignedPush);
}
in.setRejectImplicitMerges(rejectImplicitMerges);
in.setPrivateByDefault(privateByDefault);
in.setMaxObjectSizeLimit(maxObjectSizeLimit);
in.setSubmitType(submitType);
in.setState(state);
@ -298,6 +300,12 @@ public class ProjectApi {
setRequireSignedPushRaw(v.name());
}
final void setPrivateByDefault(InheritableBoolean v) {
setPrivateByDefault(v.name());
}
private native void setPrivateByDefault(String v) /*-{ if(v)this.private_by_default=v; }-*/;
final void setEnableReviewerByEmail(InheritableBoolean v) {
setEnableReviewerByEmailRaw(v.name());
}

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

@ -102,6 +102,7 @@ public final class Project {
protected InheritableBoolean requireSignedPush;
protected InheritableBoolean rejectImplicitMerges;
protected InheritableBoolean privateByDefault;
protected InheritableBoolean enableReviewerByEmail;
@ -120,6 +121,7 @@ public final class Project {
createNewChangeForAllNotInTarget = InheritableBoolean.INHERIT;
enableSignedPush = InheritableBoolean.INHERIT;
requireSignedPush = InheritableBoolean.INHERIT;
privateByDefault = InheritableBoolean.INHERIT;
enableReviewerByEmail = InheritableBoolean.INHERIT;
matchAuthorToCommitterDate = InheritableBoolean.INHERIT;
}
@ -164,6 +166,14 @@ public final class Project {
return rejectImplicitMerges;
}
public InheritableBoolean getPrivateByDefault() {
return privateByDefault;
}
public void setPrivateByDefault(InheritableBoolean privateByDefault) {
this.privateByDefault = privateByDefault;
}
public InheritableBoolean getEnableReviewerByEmail() {
return enableReviewerByEmail;
}

3
gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
View File

@ -109,7 +109,6 @@ public class CreateChange
private final ChangeFinder changeFinder;
private final PatchSetUtil psUtil;
private final boolean allowDrafts;
private final boolean privateByDefault;
private final MergeUtil.Factory mergeUtilFactory;
private final SubmitType submitType;
private final NotifyUtil notifyUtil;
@ -150,7 +149,6 @@ public class CreateChange
this.changeFinder = changeFinder;
this.psUtil = psUtil;
this.allowDrafts = config.getBoolean("change", "allowDrafts", true);
this.privateByDefault = config.getBoolean("change", "privateByDefault", false);
this.submitType = config.getEnum("project", null, "submitType", SubmitType.MERGE_IF_NECESSARY);
this.mergeUtilFactory = mergeUtilFactory;
this.notifyUtil = notifyUtil;
@ -261,6 +259,7 @@ public class CreateChange
c = newCommit(oi, rw, author, mergeTip, commitMessage);
}
boolean privateByDefault = rsrc.getProjectState().isPrivateByDefault();
Change.Id changeId = new Change.Id(seq.nextChangeId());
ChangeInserter ins = changeInserterFactory.create(changeId, c, refName);
ins.setMessage(String.format("Uploaded patch set %s.", ins.getPatchSetId().get()));

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

@ -125,6 +125,9 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
private static final String KEY_REQUIRE_SIGNED_PUSH = "requireSignedPush";
private static final String KEY_REJECT_IMPLICIT_MERGES = "rejectImplicitMerges";
private static final String CHANGE = "change";
private static final String KEY_PRIVATE_BY_DEFAULT = "privateByDefault";
private static final String SUBMIT = "submit";
private static final String KEY_ACTION = "action";
private static final String KEY_MERGE_CONTENT = "mergeContent";
@ -535,6 +538,10 @@ 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.setPrivateByDefault(
getEnum(rc, CHANGE, null, KEY_PRIVATE_BY_DEFAULT, InheritableBoolean.INHERIT));
p.setEnableReviewerByEmail(
getEnum(rc, REVIEWER, null, KEY_ENABLE_REVIEWER_BY_EMAIL, InheritableBoolean.INHERIT));
@ -1106,6 +1113,15 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
KEY_REJECT_IMPLICIT_MERGES,
p.getRejectImplicitMerges(),
InheritableBoolean.INHERIT);
set(
rc,
CHANGE,
null,
KEY_PRIVATE_BY_DEFAULT,
p.getPrivateByDefault(),
InheritableBoolean.INHERIT);
set(
rc,
REVIEWER,

6
gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
View File

@ -2095,14 +2095,14 @@ class ReceiveCommits {
}
private void setChangeId(int id) {
boolean privateByDefault = projectCache.get(project.getNameKey()).isPrivateByDefault();
changeId = new Change.Id(id);
ins =
changeInserterFactory
.create(changeId, commit, refName)
.setTopic(magicBranch.topic)
.setPrivate(
magicBranch.isPrivate
|| (receiveConfig.privateByDefault && !magicBranch.removePrivate))
.setPrivate(magicBranch.isPrivate || (privateByDefault && !magicBranch.removePrivate))
.setWorkInProgress(magicBranch.workInProgress)
// Changes already validated in validateNewCommits.
.setValidate(false);

2
gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveConfig.java
View File

@ -28,7 +28,6 @@ class ReceiveConfig {
final boolean checkMagicRefs;
final boolean checkReferencedObjectsAreReachable;
final boolean allowDrafts;
final boolean privateByDefault;
private final int systemMaxBatchChanges;
private final AccountLimits.Factory limitsFactory;
@ -38,7 +37,6 @@ class ReceiveConfig {
checkReferencedObjectsAreReachable =
config.getBoolean("receive", null, "checkReferencedObjectsAreReachable", true);
allowDrafts = config.getBoolean("change", null, "allowDrafts", true);
privateByDefault = config.getBoolean("change", null, "privateByDefault", false);
systemMaxBatchChanges = config.getInt("receive", "maxBatchChanges", 0);
this.limitsFactory = limitsFactory;
}

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

@ -58,6 +58,7 @@ public class ConfigInfoImpl extends ConfigInfo {
InheritedBooleanInfo enableSignedPush = new InheritedBooleanInfo();
InheritedBooleanInfo requireSignedPush = new InheritedBooleanInfo();
InheritedBooleanInfo rejectImplicitMerges = new InheritedBooleanInfo();
InheritedBooleanInfo privateByDefault = new InheritedBooleanInfo();
InheritedBooleanInfo enableReviewerByEmail = new InheritedBooleanInfo();
InheritedBooleanInfo matchAuthorToCommitterDate = new InheritedBooleanInfo();
@ -75,6 +76,7 @@ public class ConfigInfoImpl extends ConfigInfo {
enableSignedPush.configuredValue = p.getEnableSignedPush();
requireSignedPush.configuredValue = p.getRequireSignedPush();
rejectImplicitMerges.configuredValue = p.getRejectImplicitMerges();
privateByDefault.configuredValue = p.getPrivateByDefault();
enableReviewerByEmail.configuredValue = p.getEnableReviewerByEmail();
matchAuthorToCommitterDate.configuredValue = p.getMatchAuthorToCommitterDate();
@ -88,6 +90,7 @@ public class ConfigInfoImpl extends ConfigInfo {
parentState.isCreateNewChangeForAllNotInTarget();
enableSignedPush.inheritedValue = projectState.isEnableSignedPush();
requireSignedPush.inheritedValue = projectState.isRequireSignedPush();
privateByDefault.inheritedValue = projectState.isPrivateByDefault();
rejectImplicitMerges.inheritedValue = projectState.isRejectImplicitMerges();
enableReviewerByEmail.inheritedValue = projectState.isEnableReviewerByEmail();
matchAuthorToCommitterDate.inheritedValue = projectState.isMatchAuthorToCommitterDate();
@ -105,6 +108,7 @@ public class ConfigInfoImpl extends ConfigInfo {
this.enableSignedPush = enableSignedPush;
this.requireSignedPush = requireSignedPush;
}
this.privateByDefault = privateByDefault;
MaxObjectSizeLimitInfo maxObjectSizeLimit = new MaxObjectSizeLimitInfo();
maxObjectSizeLimit.value =

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

@ -394,6 +394,10 @@ public class ProjectState {
return getInheritableBoolean(Project::getRejectImplicitMerges);
}
public boolean isPrivateByDefault() {
return getInheritableBoolean(Project::getPrivateByDefault);
}
public boolean isEnableReviewerByEmail() {
return getInheritableBoolean(Project::getEnableReviewerByEmail);
}

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

@ -147,6 +147,10 @@ public class PutConfig implements RestModifyView<ProjectResource, ConfigInput> {
p.setRejectImplicitMerges(input.rejectImplicitMerges);
}
if (input.privateByDefault != null) {
p.setPrivateByDefault(input.privateByDefault);
}
if (input.maxObjectSizeLimit != null) {
p.setMaxObjectSizeLimit(input.maxObjectSizeLimit);
}
@ -258,7 +262,7 @@ public class PutConfig implements RestModifyView<ProjectResource, ConfigInput> {
value,
v.getKey()));
}
//$FALL-THROUGH$
// $FALL-THROUGH$
case STRING:
cfg.setString(v.getKey(), value);
break;

16
polygerrit-ui/app/elements/admin/gr-project/gr-project.html
View File

@ -167,6 +167,22 @@ limitations under the License.
</gr-select>
</span>
</section>
<section>
<span class="title">
Set all new changes private by default</span>
<span class="value">
<gr-select
id="setAllnewChangesPrivateByDefaultSelect"
bind-value="{{_projectConfig.private_by_default.configured_value}}">
<select disabled$="[[_readOnly]]">
<template is="dom-repeat"
items="[[_formatBooleanSelect(_projectConfig.private_by_default)]]">
<option value="[[item.value]]">[[item.label]]</option>
</template>
</select>
</gr-select>
</span>
</section>
<section>
<span class="title">Maximum Git object size limit</span>
<span class="value">

7
polygerrit-ui/app/elements/admin/gr-project/gr-project_test.html
View File

@ -77,6 +77,10 @@ limitations under the License.
value: false,
configured_value: 'FALSE',
},
private_by_default: {
value: false,
configured_value: 'FALSE',
},
match_author_to_committer_date: {
value: false,
configured_value: 'FALSE',
@ -229,6 +233,7 @@ limitations under the License.
create_new_change_for_all_not_in_target: 'TRUE',
require_change_id: 'TRUE',
reject_implicit_merges: 'TRUE',
private_by_default: 'TRUE',
match_author_to_committer_date: 'TRUE',
max_object_size_limit: 10,
submit_type: 'FAST_FORWARD_ONLY',
@ -256,6 +261,8 @@ limitations under the License.
configInputObj.require_change_id;
element.$.rejectImplicitMergesSelect.bindValue =
configInputObj.reject_implicit_merges;
element.$.setAllnewChangesPrivateByDefaultSelect.bindValue =
configInputObj.private_by_default;
element.$.matchAuthoredDateWithCommitterDateSelect.bindValue =
configInputObj.match_author_to_committer_date;
element.$.maxGitObjSizeInput.bindValue =