opendev/gerrit
Code Issues Proposed changes

Implement Private Changes

Browse Source 
Private changes are only visible to owner, reviewers and users with the
configured permission. This lets users stage changes without
advertising their change, and conduct sensitive reviews (eg. security)
mong a small group.

- Add Private field to change in ReviewDb
- Check visibility for private changes
- Add permission that allows users to see all private changes
- Add Private Footer to NoteDb
- Add field for private changes to index and QueryBuilder
- Add REST endpoints to Mark/Unmark private change
- VisibleRefsFilter filters private changes
- GWT UI: Mark/Unmark change as private and show private label
- GWT UI: Show 'status (Private)' in ChangeTable.
- Support to control privacy of a change on push
- Add tests for reviewer visibility and new permission
- Add tests for query by private
- Add tests for advertised references
- Add user documentation in intro-user

  To push a private change or to turn a change private on push the
  'private' option can be specified:

    git push host HEAD:refs/for/master%private

  Removing the privacy flag should not happen accidentally, but should be
  a very explicit action. This is why omitting the 'private' option when
  pushing updates to a private change doesn't remove the privacy flag on
  the change. To remove the privacy flag from a change on push the
  'remove-private' option can be used:

    git push host HEAD:refs/for/master%remove-private

Change-Id: Ib2b26ea19c0286cff9c05754b0875f61e5e9fceb
Signed-off-by: Edwin Kempin <ekempin@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Patrick Hiesel <hiesel@google.com>
Signed-off-by: Changcheng Xiao <xchangcheng@google.com>
Signed-off-by: Alice Kober-Sotzek <aliceks@google.com>
This commit is contained in:
Edwin Kempin 2017-02-21 11:56:08 +01:00 committed by Han-Wen Nienhuys
parent 565a1510ae
commit 98ddc8a658
53 changed files with 895 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
Documentation/access-control.txt
View File

@ -850,6 +850,15 @@ when link:rest-api-changes.html#submit-change[submitting using the REST API].
Note that this permission is named `submitAs` in the `project.config`
file.
[[category_view_private_changes]]
=== View Private Changes
This category permits users to view all private changes.
The change owner and any explicitly added reviewers can always see
private changes (even without having the `View Private Changes` access
right assigned).
[[category_view_drafts]]
=== View Drafts

41
Documentation/intro-user.txt
View File

@ -482,9 +482,50 @@ followed by `topic=...`.
$ git push origin HEAD:refs/heads/master -o topic=multi-master
----
[[private changes]]
== Private changes
Private changes are changes that are only visible to their owners and
reviewers. Private changes are useful in a number of cases:
* You want to check what the change looks before formal review starts.
By marking the change private without reviewers, nobody can't
prematurely comment on your changes.
* You want to use Gerrit to sync data between different devices. By
creating a private throwaway change without reviewers, you can push
from one device, and fetch to another device.
* You want to do code review on a change that has sensitive
aspects. By reviewing a security fix in a private change,
outsiders can't discover the fix before it is pushed out. Even after
merging the change, the review can be kept private.
To create a private change, you push it with the `private` option.
.Push a private change
----
$ git commit
$ git push origin HEAD:refs/for/master%private
----
The change will remain private on subsequent pushes until you specify
the `remove-private` option. Alternatively, the web UI provides buttons
to mark a change private and non-private again.
For CI systems that must verify private changes, a special permission
can be granted
(link:access-control.html#category_view_private_changes[View Private Changes]).
In that case, care should be taken to prevent the CI system from
exposing secret details.
[[drafts]]
== Working with Drafts
Drafts is a deprecated feature and will be removed soon. Consider using
private changes instead.
Changes can be uploaded as drafts. By default draft changes are only
visible to the change owner. This gives you the possibility to have
some staging before making your changes visible to the reviewers. Draft

41
Documentation/rest-api-changes.txt
View File

@ -2107,6 +2107,47 @@ Only the change owner, a project owner, or an administrator may fix changes.
}
----
[[Mark-private]]
=== Mark Private
--
'PUT /changes/link:#change-id[\{change-id\}]/private'
--
Marks the change to be private. Note users can only mark own changes as private.
.Request
----
Set /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/private HTTP/1.0
----
.Response
----
HTTP/1.1 201 Created
----
If the change was already private the response is "`200 OK`".
[[unmark-private]]
=== Unmark Private
--
'DELETE /changes/link:#change-id[\{change-id\}]/private'
--
Marks the change to be non-private. Note users can only unmark own private
changes.
.Request
----
DELETE /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/private HTTP/1.0
----
.Response
----
HTTP/1.1 204 No Content
----
If the change was already not private, the response is "`409 Conflict`".
[[edit-endpoints]]
== Change Edit Endpoints

7
Documentation/user-search.txt
View File

@ -354,6 +354,13 @@ destination branch.
Mergeability of abandoned changes is not computed. This operator will
not find any abandoned but mergeable changes.
[[private]]
is:private::
+
True if the change is private, ie. only visible to owner and its
reviewers.
[[status]]
status:open, status:pending::
+

18
Documentation/user-upload.txt
View File

@ -212,6 +212,24 @@ updates:
git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/experimental -o topic=driver/i42
----
[[private]]
==== Private Changes
To push a private change or to turn a change private on push the `private`
option can be specified:
----
git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/master%private
----
Omitting the `private` option when pushing updates to a private change
doesn't make change non-private again. To remove the private
flag from a change on push, explicitly specify the `remove-private` option:
----
git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/master%remove-private
----
[[message]]
==== Message

66
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

@ -180,6 +180,70 @@ public class ChangeIT extends AbstractDaemonTest {
assertThat(c.owner.avatars).isNull();
}
@Test
public void setPrivateByOwner() throws Exception {
TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
PushOneCommit.Result result =
pushFactory.create(db, user.getIdent(), userRepo).to("refs/for/master");
setApiUser(user);
String changeId = result.getChangeId();
assertThat(gApi.changes().id(changeId).get().isPrivate).isFalse();
gApi.changes().id(changeId).setPrivate(true);
assertThat(gApi.changes().id(changeId).get().isPrivate).isTrue();
gApi.changes().id(changeId).setPrivate(false);
assertThat(gApi.changes().id(changeId).get().isPrivate).isFalse();
}
@Test
public void setPrivateByOtherUser() throws Exception {
TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
PushOneCommit.Result result =
pushFactory.create(db, user.getIdent(), userRepo).to("refs/for/master");
assertThat(gApi.changes().id(result.getChangeId()).get().isPrivate).isFalse();
exception.expect(AuthException.class);
exception.expectMessage("not allowed to mark private");
gApi.changes().id(result.getChangeId()).setPrivate(true);
}
@Test
public void accessPrivate() throws Exception {
TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
PushOneCommit.Result result =
pushFactory.create(db, user.getIdent(), userRepo).to("refs/for/master");
setApiUser(user);
gApi.changes().id(result.getChangeId()).setPrivate(true);
// Owner can always access its private changes.
assertThat(gApi.changes().id(result.getChangeId()).get().isPrivate).isTrue();
// Add admin as a reviewer.
gApi.changes().id(result.getChangeId()).addReviewer(admin.getId().toString());
// This change should be visible for admin as a reviewer.
setApiUser(admin);
assertThat(gApi.changes().id(result.getChangeId()).get().isPrivate).isTrue();
// Remove admin from reviewers.
gApi.changes().id(result.getChangeId()).reviewer(admin.getId().toString()).remove();
// This change should not be visible for admin anymore.
exception.expect(ResourceNotFoundException.class);
exception.expectMessage("Not found: " + result.getChangeId());
gApi.changes().id(result.getChangeId());
}
@Test
public void privateChangeOfOtherUserCanBeAccessedWithPermission() throws Exception {
PushOneCommit.Result result = createChange();
gApi.changes().id(result.getChangeId()).setPrivate(true);
allow(Permission.VIEW_PRIVATE_CHANGES, REGISTERED_USERS, "refs/*");
setApiUser(user);
assertThat(gApi.changes().id(result.getChangeId()).get().isPrivate).isTrue();
}
@Test
public void getAmbiguous() throws Exception {
PushOneCommit.Result r1 = createChange();
@ -1563,7 +1627,7 @@ public class ChangeIT extends AbstractDaemonTest {
Iterables.getOnlyElement(query("project:{" + project.get() + "} owner:self")).changeId)
.isEqualTo(r.getChangeId());
setApiUser(user);
assertThat(query("owner:self")).isEmpty();
assertThat(query("owner:self project:{" + project.get() + "}")).isEmpty();
}
@Test

32
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
View File

@ -375,6 +375,38 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
r.assertErrorStatus("user \"" + nonExistingEmail + "\" not found");
}
@Test
public void pushPrivateChange() throws Exception {
// Push a private change.
PushOneCommit.Result r = pushTo("refs/for/master%private");
r.assertOkStatus();
assertThat(r.getChange().change().isPrivate()).isTrue();
// Pushing a new patch set without --private doesn't remove the privacy flag from the change.
r = amendChange(r.getChangeId(), "refs/for/master");
r.assertOkStatus();
assertThat(r.getChange().change().isPrivate()).isTrue();
// Remove the privacy flag from the change.
r = amendChange(r.getChangeId(), "refs/for/master%remove-private");
r.assertOkStatus();
assertThat(r.getChange().change().isPrivate()).isFalse();
// Normal push: privacy flag is not added back.
r = amendChange(r.getChangeId(), "refs/for/master");
r.assertOkStatus();
assertThat(r.getChange().change().isPrivate()).isFalse();
// Make the change private again.
r = pushTo("refs/for/master%private");
r.assertOkStatus();
assertThat(r.getChange().change().isPrivate()).isTrue();
// Can't use --private and --remove-private together.
r = pushTo("refs/for/master%private,remove-private");
r.assertErrorStatus();
}
@Test
public void pushForMasterAsDraft() throws Exception {
// create draft by pushing to 'refs/drafts/'

43
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
View File

@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static java.util.stream.Collectors.toList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
@ -54,6 +55,8 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.LsRemoteCommand;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
@ -470,6 +473,46 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
assertThat(getReceivePackRefs().additionalHaves()).containsExactly(obj(c4, 1));
}
@Test
public void advertisedReferencesOmitPrivateChangesOfOtherUsers() throws Exception {
allow(Permission.READ, REGISTERED_USERS, "refs/heads/master");
TestRepository<?> userTestRepository = cloneProject(project, user);
try (Git git = userTestRepository.git()) {
LsRemoteCommand lsRemoteCommand = git.lsRemote();
String change3RefName = c3.currentPatchSet().getRefName();
List<String> initialRefNames =
lsRemoteCommand.call().stream().map(Ref::getName).collect(toList());
assertWithMessage("Precondition violated").that(initialRefNames).contains(change3RefName);
gApi.changes().id(c3.getId().get()).setPrivate(true);
List<String> refNames = lsRemoteCommand.call().stream().map(Ref::getName).collect(toList());
assertThat(refNames).doesNotContain(change3RefName);
}
}
@Test
public void advertisedReferencesIncludePrivateChangesWhenAllRefsMayBeRead() throws Exception {
allow(Permission.READ, REGISTERED_USERS, "refs/*");
TestRepository<?> userTestRepository = cloneProject(project, user);
try (Git git = userTestRepository.git()) {
LsRemoteCommand lsRemoteCommand = git.lsRemote();
String change3RefName = c3.currentPatchSet().getRefName();
List<String> initialRefNames =
lsRemoteCommand.call().stream().map(Ref::getName).collect(toList());
assertWithMessage("Precondition violated").that(initialRefNames).contains(change3RefName);
gApi.changes().id(c3.getId().get()).setPrivate(true);
List<String> refNames = lsRemoteCommand.call().stream().map(Ref::getName).collect(toList());
assertThat(refNames).contains(change3RefName);
}
}
/**
* Assert that refs seen by a non-admin user match expected.
*

119
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
View File

@ -147,6 +147,60 @@ public class ProjectWatchIT extends AbstractDaemonTest {
assertThat(sender.getMessages()).isEmpty();
}
@Test
public void noNotificationForPrivateChangesForWatchersInNotifyConfig() throws Exception {
Address addr = new Address("Watcher", "watcher@example.com");
NotifyConfig nc = new NotifyConfig();
nc.addEmail(addr);
nc.setName("team");
nc.setHeader(NotifyConfig.Header.TO);
nc.setTypes(EnumSet.of(NotifyType.NEW_CHANGES));
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
cfg.putNotifyConfig("team", nc);
saveProjectConfig(project, cfg);
sender.clear();
PushOneCommit.Result r =
pushFactory
.create(db, admin.getIdent(), testRepo, "private change", "a", "a1")
.to("refs/for/master%private");
r.assertOkStatus();
assertThat(sender.getMessages()).isEmpty();
}
@Test
public void noNotificationForChangeThatIsTurnedPrivateForWatchersInNotifyConfig()
throws Exception {
Address addr = new Address("Watcher", "watcher@example.com");
NotifyConfig nc = new NotifyConfig();
nc.addEmail(addr);
nc.setName("team");
nc.setHeader(NotifyConfig.Header.TO);
nc.setTypes(EnumSet.of(NotifyType.NEW_PATCHSETS));
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
cfg.putNotifyConfig("team", nc);
saveProjectConfig(project, cfg);
PushOneCommit.Result r =
pushFactory
.create(db, admin.getIdent(), testRepo, "subject", "a", "a1")
.to("refs/for/master");
r.assertOkStatus();
sender.clear();
r =
pushFactory
.create(db, admin.getIdent(), testRepo, "subject", "a", "a2", r.getChangeId())
.to("refs/for/master%private");
r.assertOkStatus();
assertThat(sender.getMessages()).isEmpty();
}
@Test
public void watchProject() throws Exception {
// watch project
@ -521,4 +575,69 @@ public class ProjectWatchIT extends AbstractDaemonTest {
watchConfig.deleteAllProjectWatches(id);
assertThat(watchConfig.getProjectWatches(id)).isEmpty();
}
@Test
public void watchProjectNoNotificationForPrivateChange() throws Exception {
// watch project
String watchedProject = createProject("watchedProject").get();
setApiUser(user);
watch(watchedProject, null);
// push a private change to watched project -> should not trigger email notification
setApiUser(admin);
TestRepository<InMemoryRepository> watchedRepo =
cloneProject(new Project.NameKey(watchedProject), admin);
PushOneCommit.Result r =
pushFactory
.create(db, admin.getIdent(), watchedRepo, "private change", "a", "a1")
.to("refs/for/master%private");
r.assertOkStatus();
// assert email notification
assertThat(sender.getMessages()).isEmpty();
}
@Test
public void watchProjectNotifyOnPrivateChange() throws Exception {
String watchedProject = createProject("watchedProject").get();
// create group that can view all private changes
GroupInfo groupThatCanViewPrivateChanges =
gApi.groups().create("groupThatCanViewPrivateChanges").get();
grant(
Permission.VIEW_PRIVATE_CHANGES,
new Project.NameKey(watchedProject),
"refs/*",
false,
new AccountGroup.UUID(groupThatCanViewPrivateChanges.id));
// watch project as user that can't view private changes
setApiUser(user);
watch(watchedProject, null);
// watch project as user that can view all private change
TestAccount userThatCanViewPrivateChanges =
accounts.create("user2", "user2@test.com", "User2", groupThatCanViewPrivateChanges.name);
setApiUser(userThatCanViewPrivateChanges);
watch(watchedProject, null);
// push a private change to watched project -> should trigger email notification for
// userThatCanViewPrivateChanges, but not for user
setApiUser(admin);
TestRepository<InMemoryRepository> watchedRepo =
cloneProject(new Project.NameKey(watchedProject), admin);
PushOneCommit.Result r =
pushFactory
.create(db, admin.getIdent(), watchedRepo, "TRIGGER", "a", "a1")
.to("refs/for/master%private");
r.assertOkStatus();
// assert email notification
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(userThatCanViewPrivateChanges.emailAddress);
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
}

2
gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java
View File

@ -47,6 +47,7 @@ public class Permission implements Comparable<Permission> {
public static final String SUBMIT = "submit";
public static final String SUBMIT_AS = "submitAs";
public static final String VIEW_DRAFTS = "viewDrafts";
public static final String VIEW_PRIVATE_CHANGES = "viewPrivateChanges";
private static final List<String> NAMES_LC;
private static final int LABEL_INDEX;
@ -74,6 +75,7 @@ public class Permission implements Comparable<Permission> {
NAMES_LC.add(SUBMIT.toLowerCase());
NAMES_LC.add(SUBMIT_AS.toLowerCase());
NAMES_LC.add(VIEW_DRAFTS.toLowerCase());
NAMES_LC.add(VIEW_PRIVATE_CHANGES.toLowerCase());
NAMES_LC.add(EDIT_TOPIC_NAME.toLowerCase());
NAMES_LC.add(EDIT_HASHTAGS.toLowerCase());
NAMES_LC.add(EDIT_ASSIGNEE.toLowerCase());

7
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
View File

@ -85,6 +85,8 @@ public interface ChangeApi {
void move(MoveInput in) throws RestApiException;
void setPrivate(boolean value) throws RestApiException;
/**
* Create a new change that reverts this change.
*
@ -306,6 +308,11 @@ public interface ChangeApi {
throw new NotImplementedException();
}
@Override
public void setPrivate(boolean value) {
throw new NotImplementedException();
}
@Override
public ChangeApi revert() {
throw new NotImplementedException();

1
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java
View File

@ -44,6 +44,7 @@ public class ChangeInfo {
public Integer insertions;
public Integer deletions;
public Integer unresolvedCommentCount;
public Boolean isPrivate;
public int _number;

2
gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/ChangeInfo.java
View File

@ -138,6 +138,8 @@ public class ChangeInfo extends JavaScriptObject {
public final native boolean reviewed() /*-{ return this.reviewed ? true : false; }-*/;
public final native boolean isPrivate() /*-{ return this.is_private ? true : false; }-*/;
public final native NativeMap<LabelInfo> allLabels() /*-{ return this.labels; }-*/;
public final native LabelInfo label(String n) /*-{ return this.labels[n]; }-*/;

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

@ -131,6 +131,7 @@ public class SearchSuggestOracle extends HighlightSuggestOracle {
suggestions.add("is:open");
suggestions.add("is:pending");
suggestions.add("is:draft");
suggestions.add("is:private");
suggestions.add("is:closed");
suggestions.add("is:merged");
suggestions.add("is:abandoned");

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

@ -148,7 +148,8 @@ permissionNames = \
removeReviewer, \
submit, \
submitAs, \
viewDrafts
viewDrafts, \
viewPrivateChanges
abandon = Abandon
addPatchSet = Add Patch Set
@ -174,6 +175,7 @@ removeReviewer = Remove Reviewer
submit = Submit
submitAs = Submit (On Behalf Of)
viewDrafts = View Drafts
viewPrivateChanges = View Private Changes
refErrorEmpty = Reference must be supplied
refErrorBeginSlash = Reference must not start with '/'

19
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java
View File

@ -48,6 +48,7 @@ class Actions extends Composite {
"revert",
"submit",
"topic",
"private",
"/",
};
@ -65,6 +66,9 @@ class Actions extends Composite {
@UiField Button deleteChange;
@UiField Button markPrivate;
@UiField Button unmarkPrivate;
@UiField Button restore;
private RestoreAction restoreAction;
@ -122,6 +126,11 @@ class Actions extends Composite {
a2b(actions, "restore", restore);
a2b(actions, "revert", revert);
a2b(actions, "followup", followUp);
if (info.isPrivate()) {
a2b(actions, "private", unmarkPrivate);
} else {
a2b(actions, "private", markPrivate);
}
for (String id : filterNonCore(actions)) {
add(new ActionButton(info, actions.get(id)));
}
@ -192,6 +201,16 @@ class Actions extends Composite {
}
}
@UiHandler("markPrivate")
void onMarkPrivate(@SuppressWarnings("unused") ClickEvent e) {
ChangeActions.markPrivate(changeId, markPrivate);
}
@UiHandler("unmarkPrivate")
void onUnmarkPrivate(@SuppressWarnings("unused") ClickEvent e) {
ChangeActions.unmarkPrivate(changeId, unmarkPrivate);
}
@UiHandler("restore")
void onRestore(@SuppressWarnings("unused") ClickEvent e) {
if (restoreAction == null) {

6
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.ui.xml
View File

@ -81,6 +81,12 @@ limitations under the License.
<g:Button ui:field='followUp' styleName='' visible='false'>
<div><ui:msg>Follow-Up</ui:msg></div>
</g:Button>
<g:Button ui:field='markPrivate' styleName='' visible='false'>
<div><ui:msg>Mark Private</ui:msg></div>
</g:Button>
<g:Button ui:field='unmarkPrivate' styleName='' visible='false'>
<div><ui:msg>Unmark Private</ui:msg></div>
</g:Button>
<g:Button ui:field='submit' styleName='{style.submit}' visible='false'/>
</g:FlowPanel>

8
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeActions.java
View File

@ -37,6 +37,14 @@ public class ChangeActions {
ChangeApi.deleteChange(id.get(), mine(draftButtons));
}
static void markPrivate(Change.Id id, Button... draftButtons) {
ChangeApi.markPrivate(id.get(), cs(id, draftButtons));
}
static void unmarkPrivate(Change.Id id, Button... draftButtons) {
ChangeApi.unmarkPrivate(id.get(), cs(id, draftButtons));
}
public static GerritCallback<JavaScriptObject> cs(
final Change.Id id, final Button... draftButtons) {
setEnabled(false, draftButtons);

5
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java
View File

@ -198,6 +198,7 @@ public class ChangeScreen extends Screen {
@UiField InlineLabel uploaderName;
@UiField Element statusText;
@UiField Element privateText;
@UiField Image projectSettings;
@UiField AnchorElement projectSettingsLink;
@UiField InlineHyperlink projectDashboard;
@ -1370,6 +1371,10 @@ public class ChangeScreen extends Screen {
statusText.setInnerText(Util.toLongString(s));
}
if (info.isPrivate()) {
privateText.setInnerText(Util.C.isPrivate());
}
if (Gerrit.isSignedIn()) {
replyAction =
new ReplyAction(

6
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.ui.xml
View File

@ -99,6 +99,9 @@ limitations under the License.
.statusText {
font-weight: bold;
}
.privateText {
font-weight: bold;
}
div.popdown {
display: inline-block;
@ -376,7 +379,8 @@ limitations under the License.
<span class='{style.changeId}'>
<ui:msg>Change <g:Anchor ui:field='permalink' title='Reload the change (Shortcut: R)'>
<ui:attribute name='title'/>
</g:Anchor> - <span ui:field='statusText' class='{style.statusText}'/></ui:msg>
</g:Anchor> - <span ui:field='statusText' class='{style.statusText}'/>
<span ui:field='privateText' class='{style.privateText}'/></ui:msg>
</span>
<g:SimplePanel ui:field='headerExtension' styleName='{style.headerExtension}'/>
</div>

8
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java
View File

@ -121,6 +121,14 @@ public class ChangeApi {
change(id).view("assignee").put(input, cb);
}
public static void markPrivate(int id, AsyncCallback<JavaScriptObject> cb) {
change(id).view("private").put(cb);
}
public static void unmarkPrivate(int id, AsyncCallback<JavaScriptObject> cb) {
change(id).view("private").delete(cb);
}
public static RestApi comments(int id) {
return call(id, "comments");
}

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java
View File

@ -33,6 +33,8 @@ public interface ChangeConstants extends Constants {
String notCurrent();
String isPrivate();
String changeEdit();
String myDashboardTitle();

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties
View File

@ -7,6 +7,7 @@ readyToSubmit = Ready to Submit
mergeConflict = Merge Conflict
notCurrent = Not Current
changeEdit = Change Edit
isPrivate = (Private)
myDashboardTitle = My Reviews
unknownDashboardTitle = Code Review Dashboard

12
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java
View File

@ -237,9 +237,17 @@ public class ChangeTable extends NavigationTable<ChangeInfo> {
Change.Status status = c.status();
if (status != Change.Status.NEW) {
table.setText(row, C_STATUS, Util.toLongString(status));
table.setText(
row,
C_STATUS,
Util.toLongString(status) + (c.isPrivate() ? (" " + Util.C.isPrivate()) : ""));
} else if (!c.mergeable()) {
table.setText(row, C_STATUS, Util.C.changeTableNotMergeable());
table.setText(
row,
C_STATUS,
Util.C.changeTableNotMergeable() + (c.isPrivate() ? (" " + Util.C.isPrivate()) : ""));
} else if (c.isPrivate()) {
table.setText(row, C_STATUS, Util.C.isPrivate());
}
if (c.owner() != null) {

13
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
View File

@ -512,6 +512,10 @@ public final class Change {
@Column(id = 19, notNull = false)
protected Account.Id assignee;
/** Whether the change is private. */
@Column(id = 20)
protected boolean isPrivate;
/** @see com.google.gerrit.server.notedb.NoteDbChangeState */
@Column(id = 101, notNull = false, length = Integer.MAX_VALUE)
protected String noteDbState;
@ -548,6 +552,7 @@ public final class Change {
originalSubject = other.originalSubject;
submissionId = other.submissionId;
topic = other.topic;
isPrivate = other.isPrivate;
noteDbState = other.noteDbState;
}
@ -694,6 +699,14 @@ public final class Change {
this.topic = topic;
}
public boolean isPrivate() {
return isPrivate;
}
public void setPrivate(boolean isPrivate) {
this.isPrivate = isPrivate;
}
public String getNoteDbState() {
return noteDbState;
}

21
gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
View File

@ -51,6 +51,7 @@ import com.google.gerrit.server.change.Check;
import com.google.gerrit.server.change.CreateMergePatchSet;
import com.google.gerrit.server.change.DeleteAssignee;
import com.google.gerrit.server.change.DeleteChange;
import com.google.gerrit.server.change.DeletePrivate;
import com.google.gerrit.server.change.GetAssignee;
import com.google.gerrit.server.change.GetHashtags;
import com.google.gerrit.server.change.GetPastAssignees;
@ -64,6 +65,7 @@ import com.google.gerrit.server.change.PostHashtags;
import com.google.gerrit.server.change.PostReviewers;
import com.google.gerrit.server.change.PublishDraftPatchSet;
import com.google.gerrit.server.change.PutAssignee;
import com.google.gerrit.server.change.PutPrivate;
import com.google.gerrit.server.change.PutTopic;
import com.google.gerrit.server.change.Rebase;
import com.google.gerrit.server.change.Restore;
@ -122,6 +124,8 @@ class ChangeApiImpl implements ChangeApi {
private final Check check;
private final Index index;
private final Move move;
private final PutPrivate putPrivate;
private final DeletePrivate deletePrivate;
@Inject
ChangeApiImpl(
@ -157,6 +161,8 @@ class ChangeApiImpl implements ChangeApi {
Check check,
Index index,
Move move,
PutPrivate putPrivate,
DeletePrivate deletePrivate,
@Assisted ChangeResource change) {
this.changeApi = changeApi;
this.revert = revert;
@ -190,6 +196,8 @@ class ChangeApiImpl implements ChangeApi {
this.check = check;
this.index = index;
this.move = move;
this.putPrivate = putPrivate;
this.deletePrivate = deletePrivate;
this.change = change;
}
@ -270,6 +278,19 @@ class ChangeApiImpl implements ChangeApi {
}
}
@Override
public void setPrivate(boolean value) throws RestApiException {
try {
if (value) {
putPrivate.apply(change, null);
} else {
deletePrivate.apply(change, null);
}
} catch (UpdateException e) {
throw new RestApiException("Cannot change private status", e);
}
}
@Override
public ChangeApi revert() throws RestApiException {
return revert(new RevertInput());

1
gerrit-server/src/main/java/com/google/gerrit/server/change/ActionJson.java
View File

@ -122,6 +122,7 @@ public class ActionJson {
copy.mergeable = changeInfo.mergeable;
copy.insertions = changeInfo.insertions;
copy.deletions = changeInfo.deletions;
copy.isPrivate = changeInfo.isPrivate;
copy.subject = changeInfo.subject;
copy.status = changeInfo.status;
copy.owner = changeInfo.owner;

11
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
View File

@ -110,6 +110,7 @@ public class ChangeInserter implements InsertChangeOp {
private String topic;
private String message;
private String patchSetDescription;
private boolean isPrivate;
private List<String> groups = Collections.emptyList();
private CommitValidators.Policy validatePolicy = CommitValidators.Policy.GERRIT;
private NotifyHandling notify = NotifyHandling.ALL;
@ -184,6 +185,7 @@ public class ChangeInserter implements InsertChangeOp {
ctx.getWhen());
change.setStatus(MoreObjects.firstNonNull(status, Change.Status.NEW));
change.setTopic(topic);
change.setPrivate(isPrivate);
return change;
}
@ -259,6 +261,12 @@ public class ChangeInserter implements InsertChangeOp {
return this;
}
public ChangeInserter setPrivate(boolean isPrivate) {
checkState(change == null, "setPrivate(boolean) only valid before creating change");
this.isPrivate = isPrivate;
return this;
}
public ChangeInserter setDraft(boolean draft) {
checkState(change == null, "setDraft(boolean) only valid before creating change");
return setStatus(draft ? Change.Status.DRAFT : Change.Status.NEW);
@ -351,6 +359,9 @@ public class ChangeInserter implements InsertChangeOp {
update.setBranch(change.getDest().get());
update.setTopic(change.getTopic());
update.setPsDescription(patchSetDescription);
if (isPrivate) {
update.setPrivate(isPrivate);
}
boolean draft = status == Change.Status.DRAFT;
List<String> newGroups = groups;

2
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
View File

@ -439,6 +439,7 @@ public class ChangeJson {
info.updated = c.getLastUpdatedOn();
info._number = c.getId().get();
info.problems = result.problems();
info.isPrivate = c.isPrivate();
finish(info);
} else {
info = new ChangeInfo();
@ -491,6 +492,7 @@ public class ChangeJson {
out.insertions = changedLines.get().insertions;
out.deletions = changedLines.get().deletions;
}
out.isPrivate = in.isPrivate();
out.subject = in.getSubject();
out.status = in.getStatus().asChangeStatus();
out.owner = accountLoader.get(in.getOwner());

78
gerrit-server/src/main/java/com/google/gerrit/server/change/DeletePrivate.java Normal file
View File

@ -0,0 +1,78 @@
// Copyright (C) 2017 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.server.change;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@Singleton
public class DeletePrivate
implements RestModifyView<ChangeResource, DeletePrivate.Input>, UiAction<ChangeResource> {
public static class Input {}
private final Provider<ReviewDb> dbProvider;
private final BatchUpdate.Factory batchUpdateFactory;
@Inject
DeletePrivate(Provider<ReviewDb> dbProvider, BatchUpdate.Factory batchUpdateFactory) {
this.dbProvider = dbProvider;
this.batchUpdateFactory = batchUpdateFactory;
}
@Override
public Response<String> apply(ChangeResource rsrc, DeletePrivate.Input input)
throws RestApiException, UpdateException {
if (!rsrc.getControl().isOwner()) {
throw new AuthException("not allowed to unmark private");
}
if (!rsrc.getChange().isPrivate()) {
throw new ResourceConflictException("change is not private");
}
ChangeControl control = rsrc.getControl();
SetPrivateOp op = new SetPrivateOp(false);
try (BatchUpdate u =
batchUpdateFactory.create(
dbProvider.get(),
control.getProject().getNameKey(),
control.getUser(),
TimeUtil.nowTs())) {
u.addOp(control.getId(), op).execute();
}
return Response.none();
}
@Override
public Description getDescription(ChangeResource rsrc) {
return new UiAction.Description()
.setLabel("Unmark private")
.setTitle("Unmark change as private")
.setVisible(rsrc.getControl().isOwner());
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
View File

@ -82,6 +82,8 @@ public class Module extends RestApiModule {
post(CHANGE_KIND, "index").to(Index.class);
post(CHANGE_KIND, "rebuild.notedb").to(Rebuild.class);
post(CHANGE_KIND, "move").to(Move.class);
put(CHANGE_KIND, "private").to(PutPrivate.class);
delete(CHANGE_KIND, "private").to(DeletePrivate.class);
post(CHANGE_KIND, "reviewers").to(PostReviewers.class);
get(CHANGE_KIND, "suggest_reviewers").to(SuggestChangeReviewers.class);

77
gerrit-server/src/main/java/com/google/gerrit/server/change/PutPrivate.java Normal file
View File

@ -0,0 +1,77 @@
// Copyright (C) 2017 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.server.change;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@Singleton
public class PutPrivate
implements RestModifyView<ChangeResource, PutPrivate.Input>, UiAction<ChangeResource> {
public static class Input {}
private final Provider<ReviewDb> dbProvider;
private final BatchUpdate.Factory batchUpdateFactory;
@Inject
PutPrivate(Provider<ReviewDb> dbProvider, BatchUpdate.Factory batchUpdateFactory) {
this.dbProvider = dbProvider;
this.batchUpdateFactory = batchUpdateFactory;
}
@Override
public Response<String> apply(ChangeResource rsrc, Input input)
throws RestApiException, UpdateException {
if (!rsrc.getControl().isOwner()) {
throw new AuthException("not allowed to mark private");
}
if (rsrc.getChange().isPrivate()) {
return Response.ok("");
}
ChangeControl control = rsrc.getControl();
SetPrivateOp op = new SetPrivateOp(true);
try (BatchUpdate u =
batchUpdateFactory.create(
dbProvider.get(),
control.getProject().getNameKey(),
control.getUser(),
TimeUtil.nowTs())) {
u.addOp(control.getId(), op).execute();
}
return Response.created("");
}
@Override
public Description getDescription(ChangeResource rsrc) {
return new UiAction.Description()
.setLabel("Mark private")
.setTitle("Mark change as private")
.setVisible(rsrc.getControl().isOwner());
}
}

38
gerrit-server/src/main/java/com/google/gerrit/server/change/SetPrivateOp.java Normal file
View File

@ -0,0 +1,38 @@
// Copyright (C) 2017 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.server.change;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
class SetPrivateOp implements BatchUpdateOp {
private final boolean isPrivate;
SetPrivateOp(boolean isPrivate) {
this.isPrivate = isPrivate;
}
@Override
public boolean updateChange(ChangeContext ctx) {
Change change = ctx.getChange();
ChangeUpdate update = ctx.getUpdate(change.currentPatchSetId());
change.setPrivate(isPrivate);
change.setLastUpdatedOn(ctx.getWhen());
update.setPrivate(isPrivate);
return true;
}
}

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

@ -1272,6 +1272,12 @@ public class ReceiveCommits {
@Option(name = "--draft", usage = "mark new/updated changes as draft")
boolean draft;
@Option(name = "--private", usage = "mark new/updated change as private")
boolean isPrivate;
@Option(name = "--remove-private", usage = "remove privacy flag from updated change")
boolean removePrivate;
@Option(
name = "--edit",
aliases = {"-e"},
@ -1525,6 +1531,11 @@ public class ReceiveCommits {
return;
}
if (magicBranch.isPrivate && magicBranch.removePrivate) {
reject(cmd, "the options 'private' and 'remove-private' are mutually exclusive");
return;
}
if (magicBranch.draft && magicBranch.submit) {
reject(cmd, "cannot submit draft");
return;
@ -2133,6 +2144,7 @@ public class ReceiveCommits {
changeInserterFactory
.create(changeId, commit, refName)
.setTopic(magicBranch.topic)
.setPrivate(magicBranch.isPrivate)
// Changes already validated in validateNewCommits.
.setValidatePolicy(CommitValidators.Policy.NONE);

7
gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java
View File

@ -240,6 +240,13 @@ public class ReplaceOp implements BatchUpdateOp {
if (magicBranch.topic != null && !magicBranch.topic.equals(ctx.getChange().getTopic())) {
update.setTopic(magicBranch.topic);
}
if (magicBranch.removePrivate) {
change.setPrivate(false);
update.setPrivate(false);
} else if (magicBranch.isPrivate) {
change.setPrivate(true);
update.setPrivate(true);
}
}
boolean draft = magicBranch != null && magicBranch.draft;

4
gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
View File

@ -385,6 +385,10 @@ public class ChangeField {
intRange(ChangeQueryBuilder.FIELD_DELTA)
.build(cd -> cd.changedLines().map(c -> c.insertions + c.deletions).orElse(null));
/** Determines if this change is private. */
public static final FieldDef<ChangeData, String> PRIVATE =
exact(ChangeQueryBuilder.FIELD_PRIVATE).build(cd -> cd.change().isPrivate() ? "1" : "0");
/** Users who have commented on this change. */
public static final FieldDef<ChangeData, Iterable<Integer>> COMMENTBY =
integer(ChangeQueryBuilder.FIELD_COMMENTBY)

4
gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
View File

@ -90,7 +90,9 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
@Deprecated
static final Schema<ChangeData> V38 = schema(V37, ChangeField.UNRESOLVED_COMMENT_COUNT);
static final Schema<ChangeData> V39 = schema(V38);
@Deprecated static final Schema<ChangeData> V39 = schema(V38);
static final Schema<ChangeData> V40 = schema(V39, ChangeField.PRIVATE);
public static final String NAME = "changes";
public static final ChangeSchemaDefinitions INSTANCE = new ChangeSchemaDefinitions();

4
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CreateChangeSender.java
View File

@ -50,7 +50,7 @@ public class CreateChangeSender extends NewChangeSender {
boolean isDraft = change.getStatus() == Change.Status.DRAFT;
try {
// Try to mark interested owners with TO and CC or BCC line.
Watchers matching = getWatchers(NotifyType.NEW_CHANGES, !isDraft);
Watchers matching = getWatchers(NotifyType.NEW_CHANGES, !isDraft && !change.isPrivate());
for (Account.Id user :
Iterables.concat(matching.to.accounts, matching.cc.accounts, matching.bcc.accounts)) {
if (isOwnerOfProjectOrBranch(user)) {
@ -69,7 +69,7 @@ public class CreateChangeSender extends NewChangeSender {
log.warn("Cannot notify watchers for new change", err);
}
includeWatchers(NotifyType.NEW_PATCHSETS, !isDraft);
includeWatchers(NotifyType.NEW_PATCHSETS, !isDraft && !change.isPrivate());
}
private boolean isOwnerOfProjectOrBranch(Account.Id user) {

3
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java
View File

@ -66,7 +66,8 @@ public class ReplacePatchSetSender extends ReplyToChangeSender {
add(RecipientType.CC, extraCC);
rcptToAuthors(RecipientType.CC);
bccStarredBy();
includeWatchers(NotifyType.NEW_PATCHSETS, !patchSet.isDraft());
removeUsersThatIgnoredTheChange();
includeWatchers(NotifyType.NEW_PATCHSETS, !patchSet.isDraft() && !change.isPrivate());
removeUsersThatIgnoredTheChange();
}

2
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java
View File

@ -233,7 +233,7 @@ public class ChangeBundle {
// last time this file was updated.
checkColumns(Change.Id.class, 1);
checkColumns(Change.class, 1, 2, 3, 4, 5, 7, 8, 10, 12, 13, 14, 17, 18, 19, 101);
checkColumns(Change.class, 1, 2, 3, 4, 5, 7, 8, 10, 12, 13, 14, 17, 18, 19, 20, 101);
checkColumns(ChangeMessage.Key.class, 1, 2);
checkColumns(ChangeMessage.class, 1, 2, 3, 4, 5, 6, 7);
checkColumns(PatchSet.Id.class, 1, 2);

1
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
View File

@ -73,6 +73,7 @@ public class ChangeNoteUtil {
public static final FooterKey FOOTER_PATCH_SET = new FooterKey("Patch-set");
public static final FooterKey FOOTER_PATCH_SET_DESCRIPTION =
new FooterKey("Patch-set-description");
public static final FooterKey FOOTER_PRIVATE = new FooterKey("Private");
public static final FooterKey FOOTER_READ_ONLY_UNTIL = new FooterKey("Read-only-until");
public static final FooterKey FOOTER_REAL_USER = new FooterKey("Real-user");
public static final FooterKey FOOTER_STATUS = new FooterKey("Status");

7
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
View File

@ -563,6 +563,13 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
return state.readOnlyUntil();
}
public boolean isPrivate() {
if (state.isPrivate() == null) {
return false;
}
return state.isPrivate();
}
@Override
protected void onLoad(LoadHandle handle)
throws NoSuchChangeException, IOException, ConfigInvalidException {

23
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
View File

@ -24,6 +24,7 @@ import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_HASHTAGS;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_LABEL;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET_DESCRIPTION;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PRIVATE;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_READ_ONLY_UNTIL;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_REAL_USER;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_STATUS;
@ -157,6 +158,7 @@ class ChangeNotesParser {
private String tag;
private RevisionNoteMap<ChangeRevisionNote> revisionNoteMap;
private Timestamp readOnlyUntil;
private Boolean isPrivate;
ChangeNotesParser(
Change.Id changeId,
@ -238,7 +240,8 @@ class ChangeNotesParser {
buildAllMessages(),
buildMessagesByPatchSet(),
comments,
readOnlyUntil);
readOnlyUntil,
isPrivate);
}
private PatchSet.Id buildCurrentPatchSetId() {
@ -379,6 +382,10 @@ class ChangeNotesParser {
parseReadOnlyUntil(commit);
}
if (isPrivate == null) {
parseIsPrivate(commit);
}
if (lastUpdatedOn == null || ts.after(lastUpdatedOn)) {
lastUpdatedOn = ts;
}
@ -924,6 +931,20 @@ class ChangeNotesParser {
}
}
private void parseIsPrivate(ChangeNotesCommit commit) throws ConfigInvalidException {
String raw = parseOneFooter(commit, FOOTER_PRIVATE);
if (raw == null) {
return;
} else if (Boolean.TRUE.toString().equalsIgnoreCase(raw)) {
isPrivate = true;
return;
} else if (Boolean.FALSE.toString().equalsIgnoreCase(raw)) {
isPrivate = false;
return;
}
throw invalidFooter(FOOTER_PRIVATE, raw);
}
private void pruneReviewers() {
Iterator<Table.Cell<Account.Id, ReviewerStateInternal, Timestamp>> rit =
reviewers.cellSet().iterator();

17
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java
View File

@ -71,6 +71,7 @@ public abstract class ChangeNotesState {
ImmutableList.of(),
ImmutableListMultimap.of(),
ImmutableListMultimap.of(),
null,
null);
}
@ -100,7 +101,8 @@ public abstract class ChangeNotesState {
List<ChangeMessage> allChangeMessages,
ListMultimap<PatchSet.Id, ChangeMessage> changeMessagesByPatchSet,
ListMultimap<RevId, Comment> publishedComments,
@Nullable Timestamp readOnlyUntil) {
@Nullable Timestamp readOnlyUntil,
@Nullable Boolean isPrivate) {
if (hashtags == null) {
hashtags = ImmutableSet.of();
}
@ -119,7 +121,8 @@ public abstract class ChangeNotesState {
originalSubject,
submissionId,
assignee,
status),
status,
isPrivate),
ImmutableSet.copyOf(pastAssignees),
ImmutableSet.copyOf(hashtags),
ImmutableList.copyOf(patchSets.entrySet()),
@ -131,7 +134,8 @@ public abstract class ChangeNotesState {
ImmutableList.copyOf(allChangeMessages),
ImmutableListMultimap.copyOf(changeMessagesByPatchSet),
ImmutableListMultimap.copyOf(publishedComments),
readOnlyUntil);
readOnlyUntil,
isPrivate);
}
/**
@ -174,6 +178,9 @@ public abstract class ChangeNotesState {
// TODO(dborowitz): Use a sensible default other than null
@Nullable
abstract Change.Status status();
@Nullable
abstract Boolean isPrivate();
}
// Only null if NoteDb is disabled.
@ -212,6 +219,9 @@ public abstract class ChangeNotesState {
@Nullable
abstract Timestamp readOnlyUntil();
@Nullable
abstract Boolean isPrivate();
Change newChange(Project.NameKey project) {
ChangeColumns c = checkNotNull(columns(), "columns are required");
Change change =
@ -269,6 +279,7 @@ public abstract class ChangeNotesState {
change.setLastUpdatedOn(c.lastUpdatedOn());
change.setSubmissionId(c.submissionId());
change.setAssignee(c.assignee());
change.setPrivate(c.isPrivate() == null ? false : c.isPrivate());
if (!patchSets().isEmpty()) {
change.setCurrentPatchSet(c.currentPatchSetId(), c.subject(), c.originalSubject());

13
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
View File

@ -29,6 +29,7 @@ import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_HASHTAGS;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_LABEL;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET_DESCRIPTION;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PRIVATE;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_READ_ONLY_UNTIL;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_REAL_USER;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_STATUS;
@ -149,6 +150,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
private String psDescription;
private boolean currentPatchSet;
private Timestamp readOnlyUntil;
private Boolean isPrivate;
private ChangeDraftUpdate draftUpdate;
private RobotCommentUpdate robotCommentUpdate;
@ -711,6 +713,10 @@ public class ChangeUpdate extends AbstractChangeUpdate {
addFooter(msg, FOOTER_READ_ONLY_UNTIL, ChangeNoteUtil.formatTime(serverIdent, readOnlyUntil));
}
if (isPrivate != null) {
addFooter(msg, FOOTER_PRIVATE, isPrivate);
}
cb.setMessage(msg.toString());
try {
ObjectId treeId = storeRevisionNotes(rw, ins, curr);
@ -757,7 +763,8 @@ public class ChangeUpdate extends AbstractChangeUpdate {
&& tag == null
&& psDescription == null
&& !currentPatchSet
&& readOnlyUntil == null;
&& readOnlyUntil == null
&& isPrivate == null;
}
ChangeDraftUpdate getDraftUpdate() {
@ -777,6 +784,10 @@ public class ChangeUpdate extends AbstractChangeUpdate {
return isAllowWriteToNewtRef;
}
public void setPrivate(boolean isPrivate) {
this.isPrivate = isPrivate;
}
void setReadOnlyUntil(Timestamp readOnlyUntil) {
this.readOnlyUntil = readOnlyUntil;
}

10
gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
View File

@ -200,6 +200,9 @@ public class ChangeControl {
/** Can this user see this change? */
public boolean isVisible(ReviewDb db, @Nullable ChangeData cd) throws OrmException {
if (getChange().isPrivate() && !isPrivateVisible(db, cd)) {
return false;
}
if (getChange().getStatus() == Change.Status.DRAFT && !isDraftVisible(db, cd)) {
return false;
}
@ -478,4 +481,11 @@ public class ChangeControl {
|| getRefControl().canViewDrafts()
|| getUser().isInternalUser();
}
public boolean isPrivateVisible(ReviewDb db, ChangeData cd) throws OrmException {
return isOwner()
|| isReviewer(db, cd)
|| getRefControl().canViewPrivateChanges()
|| getUser().isInternalUser();
}
}

5
gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
View File

@ -407,6 +407,11 @@ public class RefControl {
return canPerform(Permission.VIEW_DRAFTS);
}
/** @return true if this user can view private changes. */
public boolean canViewPrivateChanges() {
return canPerform(Permission.VIEW_PRIVATE_CHANGES);
}
/** @return true if this user can publish draft changes. */
public boolean canPublishDrafts() {
return canPerform(Permission.PUBLISH_DRAFTS);

8
gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsMergeablePredicate.java → gerrit-server/src/main/java/com/google/gerrit/server/query/change/BooleanPredicate.java
View File

@ -14,15 +14,15 @@
package com.google.gerrit.server.query.change;
import com.google.gerrit.server.index.FieldDef;
import com.google.gerrit.server.index.FieldDef.FillArgs;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gwtorm.server.OrmException;
class IsMergeablePredicate extends ChangeIndexPredicate {
class BooleanPredicate extends ChangeIndexPredicate {
private final FillArgs args;
IsMergeablePredicate(FillArgs args) {
super(ChangeField.MERGEABLE, "1");
BooleanPredicate(FieldDef<ChangeData, String> field, FillArgs args) {
super(field, "1");
this.args = args;
}

7
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
View File

@ -149,6 +149,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
public static final String FIELD_OWNERIN = "ownerin";
public static final String FIELD_PARENTPROJECT = "parentproject";
public static final String FIELD_PATH = "path";
public static final String FIELD_PRIVATE = "private";
public static final String FIELD_PROJECT = "project";
public static final String FIELD_PROJECTS = "projects";
public static final String FIELD_REF = "ref";
@ -569,7 +570,11 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
}
if ("mergeable".equalsIgnoreCase(value)) {
return new IsMergeablePredicate(args.fillArgs);
return new BooleanPredicate(ChangeField.MERGEABLE, args.fillArgs);
}
if ("private".equalsIgnoreCase(value)) {
return new BooleanPredicate(ChangeField.PRIVATE, args.fillArgs);
}
if ("assigned".equalsIgnoreCase(value)) {

2
gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
View File

@ -35,7 +35,7 @@ import java.util.concurrent.TimeUnit;
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
public static final Class<Schema_142> C = Schema_142.class;
public static final Class<Schema_143> C = Schema_143.class;
public static int getBinaryVersion() {
return guessVersion(C);

26
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_143.java Normal file
View File

@ -0,0 +1,26 @@
// Copyright (C) 2017 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.server.schema;
import com.google.inject.Inject;
import com.google.inject.Provider;
/** Add isPrivate field to change. */
public class Schema_143 extends SchemaVersion {
@Inject
Schema_143(Provider<Schema_142> prior) {
super(prior);
}
}

35
gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
View File

@ -751,7 +751,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
try (RevWalk walk = new RevWalk(repo)) {
RevCommit commit = walk.parseCommit(update.getResult());
walk.parseBody(commit);
assertThat(commit.getFullMessage()).endsWith("Hashtags: tag1,tag2\n");
assertThat(commit.getFullMessage()).contains("Hashtags: tag1,tag2\n");
}
}
@ -3265,6 +3265,39 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(notes.getReadOnlyUntil()).isEqualTo(new Timestamp(0));
}
@Test
public void privateDefault() throws Exception {
Change c = newChange();
ChangeNotes notes = newNotes(c);
assertThat(notes.isPrivate()).isFalse();
}
@Test
public void privateSetPrivate() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.setPrivate(true);
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.isPrivate()).isTrue();
}
@Test
public void privateSetPrivateMultipleTimes() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.setPrivate(true);
update.commit();
update = newUpdate(c, changeOwner);
update.setPrivate(false);
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.isPrivate()).isFalse();
}
private boolean testJson() {
return noteUtil.getWriteJson();
}

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

@ -372,6 +372,30 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
.isEqualTo("invalid change status: newx");
}
@Test
public void byPrivate() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(repo, newChange(repo), userId);
Account.Id user2 =
accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId();
Change change2 = insert(repo, newChange(repo), user2);
// No private changes.
assertQuery("is:open", change2, change1);
assertQuery("is:private");
gApi.changes().id(change1.getChangeId()).setPrivate(true);
// Change1 is not private, but should be still visible to its owner.
assertQuery("is:open", change1, change2);
assertQuery("is:private", change1);
// Switch request context to user2.
requestContext.setContext(newRequestContext(user2));
assertQuery("is:open", change2);
assertQuery("is:private");
}
@Test
public void byCommit() throws Exception {
TestRepository<Repo> repo = createProject("repo");