diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt index 4238c8d2bc..fe44ce9979 100644 --- a/Documentation/access-control.txt +++ b/Documentation/access-control.txt @@ -405,23 +405,6 @@ Further documentation on how to push can be found on the link:user-upload.html#push_create[Upload changes] page. -==== refs/publish/* - -`+refs/publish/*+` is an alternative name to `+refs/for/*+` when pushing new changes -and patch sets. - - -==== refs/drafts/* - -Draft workflow is slated for removal. Consider using -link:intro-user.html#private-changes[private changes] or -link:user-upload.html#wip[work-in-progress changes] instead. - -Before the remove process is complete, pushing with `+refs/drafts/*+` to create a new -draft change will get a link:intro-user.html#private-changes[private change] instead. -Pushing with `+refs/drafts/*+` to an existing change to create a draft patch set will -get a link:user-upload.html#[change_edit][change edit] instead. - [[access_categories]] == Access Categories @@ -429,7 +412,6 @@ Gerrit has several permission categories that can be granted to groups within projects, enabling functionality for that group's members. - [[category_abandon]] === Abandon @@ -852,36 +834,6 @@ 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 - -This category permits users to view draft changes uploaded by other -users. - -The change owner and any explicitly added reviewers can always see -draft changes (even without having the `View Drafts` access right -assigned). - - -[[category_publish_drafts]] -=== Publish Drafts - -This category permits users to publish draft changes uploaded by other -users. - -The change owner can always publish draft changes (even without having -the `Publish Drafts` access right assigned). - - -[[category_delete_drafts]] -=== Delete Drafts - -This category permits users to delete draft changes uploaded by other -users. - -The change owner can always delete draft changes (even without having -the `Delete Drafts` access right assigned). - [[category_delete_own_changes]] === Delete Own Changes @@ -949,13 +901,7 @@ Suggested access rights to grant: If it's desired to have the possibility to upload temporarily hidden changes there's a specific permission for that. This enables someone to add specific reviewers for early feedback before making the change -publicly visible. If you want to allow others than the owners to -publish a draft you also need to grant them `Publish Drafts`. - -Optional access rights to grant: - -* xref:category_push[`Push`] to 'refs/drafts/*' -* xref:category_publish_drafts[`Publish Drafts`] to 'refs/heads/*' +publicly visible. [[examples_developer]] diff --git a/Documentation/cmd-review.txt b/Documentation/cmd-review.txt index 8f40d6cd76..3f6acf6cc9 100644 --- a/Documentation/cmd-review.txt +++ b/Documentation/cmd-review.txt @@ -109,16 +109,6 @@ branch. (option is mutually exclusive with --abandon, --publish --delete, --rebase and --json) ---publish:: - Publish the specified draft patch set(s). - (option is mutually exclusive with --submit, --restore, --abandon, --delete - and --json) - ---delete:: - Delete the specified draft patch set(s). - (option is mutually exclusive with --submit, --restore, --abandon, --publish, - --rebase and --json) - --code-review:: --verified:: Set the label to the value 'N'. The exact option names diff --git a/Documentation/cmd-stream-events.txt b/Documentation/cmd-stream-events.txt index 85fb5e62b0..87e2233cde 100644 --- a/Documentation/cmd-stream-events.txt +++ b/Documentation/cmd-stream-events.txt @@ -46,7 +46,7 @@ Only subscribe to specific event types: ---- $ ssh -p 29418 review.example.com gerrit stream-events \ - -s draft-published -s patchset-created -s ref-replicated + -s patchset-created -s ref-replicated ---- == SCHEMA @@ -143,21 +143,6 @@ comment:: Review comment cover message. eventCreatedOn:: Time in seconds since the UNIX epoch when this event was created. -=== Draft Published - -Sent when a draft change has been published. - -type:: "draft-published" - -change:: link:json.html#change[change attribute] - -patchSet:: link:json.html#patchSet[patchSet attribute] - -uploader:: link:json.html#account[account attribute] - -eventCreatedOn:: Time in seconds since the UNIX epoch when this event was -created. - === Dropped Output Sent to notify a client that events have been dropped. @@ -201,11 +186,6 @@ created. Sent when a new change has been uploaded, or a new patch set has been uploaded to an existing change. -Note that this event is also sent for changes or patch sets uploaded as draft, -but is only visible to the change owner, any existing reviewers, and users who -belong to a group that is granted the -link:access-control.html#category_view_drafts[View Drafts] capability. - type:: "patchset-created" change:: link:json.html#change[change attribute] diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index a3a813ed92..6e1f3941f6 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -3491,10 +3491,9 @@ operations when multiple changes are impacted at once. [[receive.checkMagicRefs]]receive.checkMagicRefs:: + If true, Gerrit will verify the destination repository has -no references under the magic 'refs/drafts', 'refs/for', or -'refs/publish' branch namespaces. Names under these locations -confuse clients when trying to upload code reviews so Gerrit -requires them to be empty. +no references under the magic 'refs/for' branch namespace. Names under +these locations confuse clients when trying to upload code reviews so +Gerrit requires them to be empty. + If false Gerrit skips the sanity check and assumes administrators have ensured the repository does not contain any magic references. diff --git a/Documentation/config-plugins.txt b/Documentation/config-plugins.txt index 3a55b48fb8..16a3a935b5 100644 --- a/Documentation/config-plugins.txt +++ b/Documentation/config-plugins.txt @@ -247,16 +247,6 @@ Documentation] | link:https://gerrit.googlesource.com/plugins/emoticons/+doc/master/src/main/resources/Documentation/config.md[ Configuration] -[[force-draft]] -=== force-draft - -Provides an ssh command to force a change or patch set to draft status. -This is useful for administrators to be able to easily completely -delete a change or patch set from the server. - -link:https://gerrit-review.googlesource.com/#/admin/projects/plugins/force-draft[ -Project] - [[gitblit]] === gitblit @@ -668,24 +658,6 @@ Documentation] | link:https://gerrit.googlesource.com/plugins/websession-flatfile/+doc/master/src/main/resources/Documentation/config.md[ Configuration] -[[wip]] -=== wip - -This plugin adds a new button that allows a change owner to set a -change to Work In Progress, and a button to change from WIP back to a -"Ready For Review" state. - -Any change in the WIP state will not show up in anyone's Review -Requests. Pushing a new patchset will reset the change to Review In -Progress. - -link:https://gerrit-review.googlesource.com/#/admin/projects/plugins/wip[ -Project] | -link:https://gerrit.googlesource.com/plugins/wip/+doc/master/src/main/resources/Documentation/about.md[ -Documentation] | -link:https://gerrit.googlesource.com/plugins/wip/+doc/master/src/main/resources/Documentation/config.md[ -Configuration] - [[x-docs]] === x-docs diff --git a/Documentation/intro-user.txt b/Documentation/intro-user.txt index 97c4beae4d..2b6ac22973 100644 --- a/Documentation/intro-user.txt +++ b/Documentation/intro-user.txt @@ -536,28 +536,6 @@ Alternatively, rather than completely ignoring the change, it can be muted. Muting a change means it will always be marked as "reviewed" in dashboards, until a new patch set is uploaded. -[[drafts]] -== Working with Drafts - -Drafts will be deprecated and removed soon. Consider using -link:#private-changes[private changes] or -link:user-upload.html#wip[work-in-progress changes] instead. - -Changes cannot be uploaded as drafts any more, but existing draft changes -and patch sets can still be published and deleted. - -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 changes can also be used to backup unfinished changes. - -Draft changes have the state link:user-review-ui.html#draft[Draft] and -can be link:user-review-ui.html#publish[published] or -link:user-review-ui.html#delete[deleted] from the change screen. - -By link:user-review-ui.html#reviewers[adding reviewers] to a draft -change the change is made visible to these users. This way you can -collaborate with other users in privacy. - [[inline-edit]] == Inline Edit diff --git a/Documentation/json.txt b/Documentation/json.txt index ef40aeec09..78b1bd7809 100644 --- a/Documentation/json.txt +++ b/Documentation/json.txt @@ -41,8 +41,6 @@ status:: Current state of this change. NEW;; Change is still being reviewed. - DRAFT;; Change is a draft change that only consists of draft patchsets. - MERGED;; Change has been merged to its branch. ABANDONED;; Change was abandoned by its owner or administrator. @@ -108,8 +106,6 @@ author:: Author of this patchset in <>. createdOn:: Time in seconds since the UNIX epoch when this patchset was created. -isDraft:: Whether or not the patch set is a draft patch set. - kind:: Kind of change uploaded. REWORK;; Nontrivial content changes. diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt index 5152055237..8dc3b9d754 100644 --- a/Documentation/rest-api-accounts.txt +++ b/Documentation/rest-api-accounts.txt @@ -1255,10 +1255,6 @@ any account. "url": "#/dashboard/self", "name": "Changes" }, - { - "url": "#/q/owner:self+is:draft", - "name": "Drafts" - }, { "url": "#/q/has:draft", "name": "Draft Comments" @@ -1313,10 +1309,6 @@ link:#preferences-input[PreferencesInput] entity. "url": "#/dashboard/self", "name": "Changes" }, - { - "url": "#/q/owner:self+is:draft", - "name": "Drafts" - }, { "url": "#/q/has:draft", "name": "Draft Comments" @@ -1369,10 +1361,6 @@ link:#preferences-info[PreferencesInfo] entity. "url": "#/dashboard/self", "name": "Changes" }, - { - "url": "#/q/owner:self+is:draft", - "name": "Drafts" - }, { "url": "#/q/has:draft", "name": "Draft Comments" diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 09fa16bdb6..fa91c2d5bb 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -26,7 +26,7 @@ request body. "subject" : "Let's support 100% Gerrit workflow direct in browser", "branch" : "master", "topic" : "create-change-in-browser", - "status" : "DRAFT" + "status" : "NEW" } ---- @@ -47,7 +47,7 @@ the resulting change. "topic": "create-change-in-browser", "change_id": "I8473b95934b5732ac55d26311a706c9c2bde9941", "subject": "Let's support 100% Gerrit workflow direct in browser", - "status": "DRAFT", + "status": "NEW", "created": "2014-05-05 07:15:44.639000000", "updated": "2014-05-05 07:15:44.639000000", "mergeable": true, @@ -1838,25 +1838,6 @@ message if the set of changes to be submitted with this change includes changes the caller cannot read. -[[publish-draft-change]] -=== Publish Draft Change --- -'POST /changes/link:#change-id[\{change-id\}]/publish' --- - -Publishes a draft change. - -.Request ----- - POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/publish HTTP/1.0 - Content-Type: application/json; charset=UTF-8 ----- - -.Response ----- - HTTP/1.1 204 No Content ----- - [[delete-change]] === Delete Change -- @@ -1869,10 +1850,6 @@ New or abandoned changes can be deleted by their owner if the user is granted the link:access-control.html#category_delete_own_changes[Delete Own Changes] permission, otherwise only by administrators. -Draft changes can only be deleted by their owner or other users who have the -permissions to view and delete drafts. If the draft workflow is disabled, only -administrators with those permissions may delete draft changes. - .Request ---- DELETE /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940 HTTP/1.0 @@ -4017,44 +3994,6 @@ message is contained in the response body. "revision 674ac754f91e64a0efb8087e59a176484bd534d1 is not current revision" ---- -[[publish-draft-revision]] -=== Publish Draft Revision --- -'POST /changes/link:#change-id[\{change-id\}]/revisions/link:#revision-id[\{revision-id\}]/publish' --- - -Publishes a draft revision. - -.Request ----- - POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/current/publish HTTP/1.0 - Content-Type: application/json; charset=UTF-8 ----- - -.Response ----- - HTTP/1.1 204 No Content ----- - -[[delete-draft-revision]] -=== Delete Draft Revision --- -'DELETE /changes/link:#change-id[\{change-id\}]/revisions/link:#revision-id[\{revision-id\}]' --- - -Deletes a draft revision. - -.Request ----- - DELETE /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1 HTTP/1.0 - Content-Type: application/json; charset=UTF-8 ----- - -.Response ----- - HTTP/1.1 204 No Content ----- - [[get-patch]] === Get Patch -- @@ -5682,7 +5621,7 @@ is enabled). |`subject` || The subject of the change (header line of the commit message). |`status` || -The status of the change (`NEW`, `MERGED`, `ABANDONED`, `DRAFT`). +The status of the change (`NEW`, `MERGED`, `ABANDONED`). |`created` || The link:rest-api.html#timestamp[timestamp] of when the change was created. @@ -6657,7 +6596,7 @@ link:#commit-info[CommitInfo] entity. |`_revision_number` |optional|The revision number. |`_current_revision_number`|optional|The current revision number. |`status` |optional|The status of the change. The status of -the change is one of (`NEW`, `MERGED`, `ABANDONED`, `DRAFT`). +the change is one of (`NEW`, `MERGED`, `ABANDONED`). |=========================== [[related-changes-info]] @@ -6881,7 +6820,6 @@ link:#list-changes[Query Changes]. [options="header",cols="1,^1,5"] |=========================== |Field Name ||Description -|`draft` |not set if `false`|Whether the patch set is a draft. |`kind` ||The change kind. Valid values are `REWORK`, `TRIVIAL_REBASE`, `MERGE_FIRST_PARENT_UPDATE`, `NO_CODE_CHANGE`, and `NO_CHANGE`. |`_number` ||The patch set number. diff --git a/Documentation/rest-api-config.txt b/Documentation/rest-api-config.txt index 6596eda5ba..07506ff0c6 100644 --- a/Documentation/rest-api-config.txt +++ b/Documentation/rest-api-config.txt @@ -1021,10 +1021,6 @@ PreferencesInfo] is returned. "url": "#/dashboard/self", "name": "Changes" }, - { - "url": "#/q/owner:self+is:draft", - "name": "Drafts" - }, { "url": "#/q/has:draft", "name": "Draft Comments" @@ -1104,10 +1100,6 @@ PreferencesInfo] is returned. "url": "#/dashboard/self", "name": "Changes" }, - { - "url": "#/q/owner:self+is:draft", - "name": "Drafts" - }, { "url": "#/q/has:draft", "name": "Draft Comments" @@ -1396,9 +1388,6 @@ section. |`allow_blame` |not set if `false`| link:config-gerrit.html#change.allowBlame[Whether blame on side by side diff is allowed]. -|`allow_drafts` |not set if `false`| -link:config-gerrit.html#change.allowDrafts[Whether draft workflow is -allowed]. |`large_change` || link:config-gerrit.html#change.largeChange[Number of changed lines from which on a change is considered as a large change]. diff --git a/Documentation/user-changeid.txt b/Documentation/user-changeid.txt index 44ca6e0770..f965db7d8c 100644 --- a/Documentation/user-changeid.txt +++ b/Documentation/user-changeid.txt @@ -70,10 +70,10 @@ For more details, see link:cmd-hook-commit-msg.html[commit-msg]. Change Upload -------------- -During upload by pushing to `+refs/for/*+`, `+refs/drafts/*+` or -`+refs/heads/*+`, Gerrit will try to find an existing review the -uploaded commit relates to. For an existing review to match, the -following properties have to match: +During upload by pushing to `+refs/for/*+` or `+refs/heads/*+`, +Gerrit will try to find an existing review the uploaded commit +relates to. For an existing review to match, the following properties +have to match: * Change-Id * Repository name @@ -104,7 +104,7 @@ message if additional revisions to a change are required. By default, Gerrit will prevent pushing for review if no Change-Id is provided, with the following message: - ! [remote rejected] HEAD -> refs/publish/master (missing Change-Id in commit + ! [remote rejected] HEAD -> refs/for/master (missing Change-Id in commit message footer) However, repositories can be configured to allow commits without Change-Ids diff --git a/Documentation/user-review-ui.txt b/Documentation/user-review-ui.txt index 434a7a4c99..3804734d53 100644 --- a/Documentation/user-review-ui.txt +++ b/Documentation/user-review-ui.txt @@ -61,13 +61,6 @@ The change was successfully merged into the destination branch. + The change was abandoned. -- [[draft]]`Draft`: -+ -The change is a draft that is only visible to the change owner, the -reviewers that were explicitly added to the change, and users who have -the link:access-control.html#category_view_drafts[View Drafts] global -capability assigned. - [[commit-info]] === Commit Info Block @@ -258,28 +251,14 @@ open changes. Users can only cherry-pick changes to branches for which they are allowed to upload changes for review. -** [[publish]]`Publish`: -+ -Publishes the currently viewed draft patch set. If this is the first -patch set of a change that is published, the change will be published -as well. -+ -The `Publish` button is only available if a draft patch set is viewed -and the user is the change owner or has the -link:access-control.html#category_publish_drafts[Publish Drafts] access -right assigned. - ** [[delete]]`Delete Change` / `Delete Revision`: + -Deletes the change / the currently viewed draft patch set. +Deletes the change. + For open or abandoned changes, the `Delete Change` button will be available and if the user is the change owner and is granted the link:access-control.html#category_delete_own_changes[Delete Own Changes] -permission or if they are an administrator. For draft changes, -the `Delete Change` / `Delete Revision` buttons will be available if the user is -the change owner or has the -link:access-control.html#category_delete_drafts[Delete Drafts] access right assigned. +permission or if they are an administrator. ** [[plugin-actions]]Further actions may be available if plugins are installed. diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt index d5b1e26e42..ba42304e35 100644 --- a/Documentation/user-search.txt +++ b/Documentation/user-search.txt @@ -14,7 +14,6 @@ matches the search, the change will be presented instead of a list. |All > Open | status:open '(or is:open)' |All > Merged | status:merged |All > Abandoned | status:abandoned -|My > Drafts | owner:self is:draft |My > Watched Changes | status:open is:watched |My > Starred Changes | is:starred |My > Draft Comments | has:draft @@ -337,10 +336,6 @@ is:open, is:pending:: + True if the change is open. -is:draft:: -+ -True if the change is a draft. - is:closed:: + True if the change is either merged or abandoned. diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index d5e2d9e248..38d491d9d2 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.TruthJUnit.assume; import static com.google.gerrit.acceptance.GitUtil.initSsh; import static com.google.gerrit.extensions.api.changes.SubmittedTogetherOption.NON_VISIBLE_CHANGES; -import static com.google.gerrit.extensions.client.ListChangesOption.ALL_REVISIONS; import static com.google.gerrit.reviewdb.client.Patch.COMMIT_MSG; import static com.google.gerrit.reviewdb.client.Patch.MERGE_LIST; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; @@ -35,7 +34,6 @@ import com.google.common.jimfs.Jimfs; import com.google.common.primitives.Chars; import com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context; import com.google.gerrit.common.Nullable; -import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.ContributorAgreement; import com.google.gerrit.common.data.GroupReference; @@ -50,7 +48,6 @@ import com.google.gerrit.extensions.api.groups.GroupInput; import com.google.gerrit.extensions.api.projects.BranchApi; import com.google.gerrit.extensions.api.projects.BranchInput; import com.google.gerrit.extensions.api.projects.ProjectInput; -import com.google.gerrit.extensions.client.ChangeStatus; import com.google.gerrit.extensions.client.InheritableBoolean; import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.extensions.client.ProjectWatchInfo; @@ -60,14 +57,11 @@ import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeType; import com.google.gerrit.extensions.common.DiffInfo; import com.google.gerrit.extensions.common.EditInfo; -import com.google.gerrit.extensions.common.RevisionInfo; import com.google.gerrit.extensions.restapi.BinaryResult; import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Branch; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.Change.Status; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; @@ -104,14 +98,11 @@ import com.google.gerrit.server.mail.send.EmailHeader; import com.google.gerrit.server.notedb.ChangeNoteUtil; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.MutableNotesMigration; -import com.google.gerrit.server.notedb.PatchSetState; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.Util; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gerrit.server.update.BatchUpdate; -import com.google.gerrit.server.update.BatchUpdateOp; -import com.google.gerrit.server.update.ChangeContext; import com.google.gerrit.testutil.ConfigSuite; import com.google.gerrit.testutil.FakeEmailSender; import com.google.gerrit.testutil.FakeEmailSender.Message; @@ -307,16 +298,6 @@ public abstract class AbstractDaemonTest { return cfg; } - protected static Config allowDraftsDisabledConfig() { - Config cfg = new Config(); - cfg.setBoolean("change", null, "allowDrafts", false); - return cfg; - } - - protected boolean isAllowDrafts() { - return cfg.getBoolean("change", "allowDrafts", true); - } - protected boolean isSubmitWholeTopicEnabled() { return cfg.getBoolean("change", null, "submitWholeTopic", false); } @@ -638,139 +619,6 @@ public abstract class AbstractDaemonTest { repo, "refs/for/master/" + name(topic), commitMsg, fileName, content); } - protected PushOneCommit.Result createDraftChange() throws Exception { - return createDraftChange(""); - } - - protected PushOneCommit.Result createDraftChange(String topic) throws Exception { - PushOneCommit.Result r = - Strings.isNullOrEmpty(topic) - ? createChange() - : createChangeWithTopic(testRepo, topic, "message", "a.txt", "content"); - markChangeAsDraft(r.getChange().change().getId()); - setCurrentPatchSetAsDraft(r.getChange().change().getId()); - ChangeData cd = Iterables.getOnlyElement(queryProvider.get().byKeyPrefix(r.getChangeId())); - assertThat(cd.change().getStatus()).isEqualTo(Status.DRAFT); - return r; - } - - protected String createDraftChangeWith2PS() throws Exception { - PushOneCommit.Result r = createDraftChange(); - amendChangeAndMarkChangeAndPatchSetsAsDraft(r.getChangeId()); - return r.getChangeId(); - } - - protected void markChangeAsDraft(Change.Id id) throws Exception { - markChangeAsDraft(project, id); - } - - protected void markChangeAsDraft(Project.NameKey project, Change.Id id) throws Exception { - try (BatchUpdate batchUpdate = - batchUpdateFactory.create(db, project, atrScope.get().getUser(), TimeUtil.nowTs())) { - batchUpdate.addOp(id, new MarkChangeAsDraftUpdateOp()); - batchUpdate.execute(); - } - - ChangeStatus changeStatus = gApi.changes().id(id.get()).get().status; - assertThat(changeStatus).isEqualTo(ChangeStatus.DRAFT); - } - - protected void setCurrentPatchSetAsDraft(Change.Id id) throws Exception { - try (BatchUpdate batchUpdate = - batchUpdateFactory.create(db, project, atrScope.get().getUser(), TimeUtil.nowTs())) { - batchUpdate.addOp(id, new SetDraftStatusOfCurrentPatchSetOp(true)); - batchUpdate.execute(); - } - - ChangeInfo changeInfo = - gApi.changes().id(id.get()).get(EnumSet.of(ListChangesOption.ALL_REVISIONS)); - RevisionInfo revisionInfo = changeInfo.revisions.get(changeInfo.currentRevision); - assertThat(revisionInfo.draft).isEqualTo(Boolean.TRUE); - } - - protected void setDraftStatusOfPatchSets(Change.Id id, boolean draftStatus) throws Exception { - try (BatchUpdate batchUpdate = - batchUpdateFactory.create(db, project, atrScope.get().getUser(), TimeUtil.nowTs())) { - batchUpdate.addOp(id, new DraftStatusOfPatchSetsUpdateOp(draftStatus)); - batchUpdate.execute(); - } - - Boolean expectedDraftStatus = draftStatus ? Boolean.TRUE : null; - List patchSetDraftStatuses = getPatchSetDraftStatuses(id); - patchSetDraftStatuses.forEach(status -> assertThat(status).isEqualTo(expectedDraftStatus)); - } - - private List getPatchSetDraftStatuses(Change.Id id) throws Exception { - Collection revisionInfos = - gApi.changes().id(id.get()).get(ALL_REVISIONS).revisions.values(); - return revisionInfos.stream().map(revisionInfo -> revisionInfo.draft).collect(toList()); - } - - private static class MarkChangeAsDraftUpdateOp implements BatchUpdateOp { - @Override - public boolean updateChange(ChangeContext ctx) throws Exception { - Change change = ctx.getChange(); - - // Change status in database. - change.setStatus(Change.Status.DRAFT); - - // Change status in NoteDb. - PatchSet.Id currentPatchSetId = change.currentPatchSetId(); - ctx.getUpdate(currentPatchSetId).setStatus(Change.Status.DRAFT); - - return true; - } - } - - private class SetDraftStatusOfCurrentPatchSetOp implements BatchUpdateOp { - private final boolean draftStatus; - - SetDraftStatusOfCurrentPatchSetOp(boolean draftStatus) { - this.draftStatus = draftStatus; - } - - @Override - public boolean updateChange(ChangeContext ctx) throws Exception { - PatchSet currentPatchSet = psUtil.current(db, ctx.getNotes()); - - // Change status in ReviewDb. - currentPatchSet.setDraft(draftStatus); - db.patchSets().update(Collections.singleton(currentPatchSet)); - - // Change status in NoteDb. - PatchSetState patchSetState = draftStatus ? PatchSetState.DRAFT : PatchSetState.PUBLISHED; - ctx.getUpdate(currentPatchSet.getId()).setPatchSetState(patchSetState); - return true; - } - } - - private class DraftStatusOfPatchSetsUpdateOp implements BatchUpdateOp { - private final boolean draftStatus; - - DraftStatusOfPatchSetsUpdateOp(boolean draftStatus) { - this.draftStatus = draftStatus; - } - - @Override - public boolean updateChange(ChangeContext ctx) throws Exception { - Collection patchSets = psUtil.byChange(db, ctx.getNotes()); - - // Change status in database. - patchSets.forEach(patchSet -> patchSet.setDraft(draftStatus)); - db.patchSets().update(patchSets); - - // Change status in NoteDb. - PatchSetState patchSetState = draftStatus ? PatchSetState.DRAFT : PatchSetState.PUBLISHED; - patchSets - .stream() - .map(PatchSet::getId) - .map(ctx::getUpdate) - .forEach(changeUpdate -> changeUpdate.setPatchSetState(patchSetState)); - - return true; - } - } - protected PushOneCommit.Result createWorkInProgressChange() throws Exception { return pushTo("refs/for/master%wip"); } @@ -864,23 +712,6 @@ public abstract class AbstractDaemonTest { revision(r).submit(); } - protected PushOneCommit.Result amendChangeAndMarkPatchSetAsDraft(String changeId) - throws Exception { - PushOneCommit.Result r = amendChange(changeId, "refs/for/master"); - r.assertOkStatus(); - setCurrentPatchSetAsDraft(r.getChange().getId()); - return r; - } - - protected PushOneCommit.Result amendChangeAndMarkChangeAndPatchSetsAsDraft(String changeId) - throws Exception { - PushOneCommit.Result r = amendChange(changeId, "refs/for/master"); - r.assertOkStatus(); - markChangeAsDraft(r.getChange().getId()); - setCurrentPatchSetAsDraft(r.getChange().change().getId()); - return r; - } - protected ChangeInfo info(String id) throws RestApiException { return gApi.changes().id(id).info(); } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java index b1ca5d0a2f..7f0d8cdb0b 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java @@ -73,7 +73,7 @@ public class GeneralPreferencesIT extends AbstractDaemonTest { public void getAndSetPreferences() throws Exception { GeneralPreferencesInfo o = gApi.accounts().id(user42.id.toString()).getPreferences(); assertPrefs(o, GeneralPreferencesInfo.defaults(), "my", "changeTable"); - assertThat(o.my).hasSize(7); + assertThat(o.my).hasSize(6); assertThat(o.changeTable).isEmpty(); GeneralPreferencesInfo i = GeneralPreferencesInfo.defaults(); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/AbandonIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/AbandonIT.java index 52e1ea4fd1..2c1a5b34b8 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/AbandonIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/AbandonIT.java @@ -98,17 +98,6 @@ public class AbandonIT extends AbstractDaemonTest { changeAbandoner.batchAbandon(batchUpdateFactory, new Project.NameKey(project1Name), user, list); } - @Test - public void abandonDraft() throws Exception { - PushOneCommit.Result r = createDraftChange(); - String changeId = r.getChangeId(); - assertThat(info(changeId).status).isEqualTo(ChangeStatus.DRAFT); - - exception.expect(ResourceConflictException.class); - exception.expectMessage("draft changes cannot be abandoned"); - gApi.changes().id(changeId).abandon(); - } - @Test @GerritConfig(name = "changeCleanup.abandonAfter", value = "1w") public void abandonInactiveOpenChanges() throws Exception { diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java index a8a712c63c..65e5909ce5 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java @@ -835,23 +835,6 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(changeId).rebase(); } - @Test - public void publish() throws Exception { - PushOneCommit.Result r = createDraftChange(); - assertThat(info(r.getChangeId()).status).isEqualTo(ChangeStatus.DRAFT); - gApi.changes().id(r.getChangeId()).publish(); - assertThat(info(r.getChangeId()).status).isEqualTo(ChangeStatus.NEW); - } - - @Test - public void deleteDraftChange() throws Exception { - PushOneCommit.Result r = createDraftChange(); - assertThat(query(r.getChangeId())).hasSize(1); - assertThat(info(r.getChangeId()).status).isEqualTo(ChangeStatus.DRAFT); - gApi.changes().id(r.getChangeId()).delete(); - assertThat(query(r.getChangeId())).isEmpty(); - } - @Test public void deleteNewChangeAsAdmin() throws Exception { PushOneCommit.Result changeResult = createChange(); @@ -2311,7 +2294,6 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(r1.getChangeId()).revision(r1.getCommit().name()).submit(); createChange(); - createDraftChange(); setApiUser(user); AcceptanceTestRequestScope.Context ctx = disableDb(); @@ -2462,64 +2444,6 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().create(in).get(); } - @Test - public void createNewPatchSetOnVisibleDraftPatchSet() throws Exception { - // Clone separate repositories of the same project as admin and as user - TestRepository adminTestRepo = cloneProject(project, admin); - TestRepository userTestRepo = cloneProject(project, user); - - // Create change as admin - PushOneCommit push = pushFactory.create(db, admin.getIdent(), adminTestRepo); - PushOneCommit.Result r1 = push.to("refs/for/master"); - r1.assertOkStatus(); - - // Amend draft as admin - PushOneCommit.Result r2 = - amendChange(r1.getChangeId(), "refs/for/master", admin, adminTestRepo); - r2.assertOkStatus(); - setCurrentPatchSetAsDraft(r2.getChange().getId()); - - // Add user as reviewer to make this patch set visible - AddReviewerInput in = new AddReviewerInput(); - in.reviewer = user.email; - gApi.changes().id(r1.getChangeId()).addReviewer(in); - - // Fetch change - GitUtil.fetch(userTestRepo, r2.getPatchSet().getRefName() + ":ps"); - userTestRepo.reset("ps"); - - // Amend change as user - PushOneCommit.Result r3 = amendChange(r2.getChangeId(), "refs/for/master", user, userTestRepo); - r3.assertOkStatus(); - setCurrentPatchSetAsDraft(r3.getChange().getId()); - } - - @Test - public void createNewPatchSetOnInvisibleDraftPatchSet() throws Exception { - // Clone separate repositories of the same project as admin and as user - TestRepository adminTestRepo = cloneProject(project, admin); - TestRepository userTestRepo = cloneProject(project, user); - - // Create change as admin - PushOneCommit push = pushFactory.create(db, admin.getIdent(), adminTestRepo); - PushOneCommit.Result r1 = push.to("refs/for/master"); - r1.assertOkStatus(); - - // Amend draft as admin - PushOneCommit.Result r2 = - amendChange(r1.getChangeId(), "refs/for/master", admin, adminTestRepo); - r2.assertOkStatus(); - setCurrentPatchSetAsDraft(r2.getChange().getId()); - - // Fetch change - GitUtil.fetch(userTestRepo, r1.getPatchSet().getRefName() + ":ps"); - userTestRepo.reset("ps"); - - // Amend change as user - PushOneCommit.Result r3 = amendChange(r1.getChangeId(), "refs/for/master", user, userTestRepo); - r3.assertErrorStatus("cannot add patch set to " + r3.getChange().change().getChangeId() + "."); - } - @Test public void createNewPatchSetWithoutPermission() throws Exception { // Create new project with clean permissions @@ -2591,63 +2515,6 @@ public class ChangeIT extends AbstractDaemonTest { r2.assertOkStatus(); } - @Test - public void createNewPatchSetAsReviewerOnDraftChange() throws Exception { - // Clone separate repositories of the same project as admin and as user - TestRepository adminTestRepo = cloneProject(project, admin); - TestRepository userTestRepo = cloneProject(project, user); - - // Create change as admin - PushOneCommit push = pushFactory.create(db, admin.getIdent(), adminTestRepo); - PushOneCommit.Result r1 = push.to("refs/for/master"); - r1.assertOkStatus(); - markChangeAsDraft(r1.getChange().getId()); - - // Add user as reviewer - AddReviewerInput in = new AddReviewerInput(); - in.reviewer = user.email; - gApi.changes().id(r1.getChangeId()).addReviewer(in); - - // Fetch change - GitUtil.fetch(userTestRepo, r1.getPatchSet().getRefName() + ":ps"); - userTestRepo.reset("ps"); - - // Amend change as user - PushOneCommit.Result r2 = amendChange(r1.getChangeId(), "refs/for/master", user, userTestRepo); - r2.assertOkStatus(); - } - - @Test - public void createNewDraftPatchSetOnDraftChange() throws Exception { - // Create new project with clean permissions - Project.NameKey p = createProject("addPatchSet4"); - // Clone separate repositories of the same project as admin and as user - TestRepository adminTestRepo = cloneProject(p, admin); - TestRepository userTestRepo = cloneProject(p, user); - - // Block default permission - block(p, "refs/for/*", Permission.ADD_PATCH_SET, REGISTERED_USERS); - - // Create change as admin - PushOneCommit push = pushFactory.create(db, admin.getIdent(), adminTestRepo); - PushOneCommit.Result r1 = push.to("refs/for/master"); - r1.assertOkStatus(); - markChangeAsDraft(p, r1.getChange().getId()); - - // Add user as reviewer - AddReviewerInput in = new AddReviewerInput(); - in.reviewer = user.email; - gApi.changes().id(r1.getChangeId()).addReviewer(in); - - // Fetch change - GitUtil.fetch(userTestRepo, r1.getPatchSet().getRefName() + ":ps"); - userTestRepo.reset("ps"); - - // Amend change as user - PushOneCommit.Result r2 = amendChange(r1.getChangeId(), "refs/for/master", user, userTestRepo); - r2.assertErrorStatus("cannot add patch set to " + r1.getChange().getId().id + "."); - } - @Test public void createMergePatchSet() throws Exception { PushOneCommit.Result start = pushTo("refs/heads/master"); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java index fac5d283b1..a2814c3218 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java @@ -301,12 +301,6 @@ public class RevisionIT extends AbstractDaemonTest { gApi.changes().id(r.getChange().getId().get()).current().review(ReviewInput.approve()); } - @Test - public void deleteDraft() throws Exception { - PushOneCommit.Result r = createDraftChange(); - gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).delete(); - } - @Test public void cherryPick() throws Exception { PushOneCommit.Result r = pushTo("refs/for/master%topic=someTopic"); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java index 4b87ec0e1b..95e4e16efd 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java @@ -42,6 +42,7 @@ import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GerritConfig; import com.google.gerrit.acceptance.GitUtil; import com.google.gerrit.acceptance.PushOneCommit; +import com.google.gerrit.acceptance.Sandboxed; import com.google.gerrit.acceptance.TestAccount; import com.google.gerrit.acceptance.TestProjectInput; import com.google.gerrit.common.data.LabelType; @@ -61,6 +62,7 @@ import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeMessageInfo; import com.google.gerrit.extensions.common.CommentInfo; import com.google.gerrit.extensions.common.EditInfo; +import com.google.gerrit.extensions.common.EditInfoSubject; import com.google.gerrit.extensions.common.LabelInfo; import com.google.gerrit.extensions.common.RevisionInfo; import com.google.gerrit.reviewdb.client.AccountGroup; @@ -636,34 +638,6 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { r.assertOkStatus(); } - @Test - public void pushForMasterAsDraft() throws Exception { - // create draft by pushing to 'refs/drafts/' will get a private change. - PushOneCommit.Result r = pushTo("refs/drafts/master"); - r.assertOkStatus(); - r.assertChange(Change.Status.NEW, null); - - assertThat(gApi.changes().id(r.getChangeId()).get().isPrivate).isTrue(); - - // create draft by using 'draft' option will get a private change, too. - r = pushTo("refs/for/master%draft"); - r.assertOkStatus(); - r.assertChange(Change.Status.NEW, null); - assertThat(gApi.changes().id(r.getChangeId()).get().isPrivate).isTrue(); - } - - @Test - public void publishDraftChangeByPushingNonDraftPatchSet() throws Exception { - PushOneCommit.Result r = createDraftChange(); - r.assertOkStatus(); - r.assertChange(Change.Status.DRAFT, null); - - // publish draft change by pushing non-draft patch set - r = amendChange(r.getChangeId(), "refs/for/master"); - r.assertOkStatus(); - r.assertChange(Change.Status.NEW, null); - } - @Test public void pushForMasterAsEdit() throws Exception { PushOneCommit.Result r = pushTo("refs/for/master"); @@ -1819,6 +1793,42 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { assertThat(getPublishedComments(r.getChangeId())).isEmpty(); } + @Test + public void pushDraftGetsPrivateChange() throws Exception { + String changeId1 = createChange("refs/drafts/master").getChangeId(); + String changeId2 = createChange("refs/for/master%draft").getChangeId(); + + ChangeInfo info1 = gApi.changes().id(changeId1).get(); + ChangeInfo info2 = gApi.changes().id(changeId2).get(); + + assertThat(info1.status).isEqualTo(ChangeStatus.NEW); + assertThat(info2.status).isEqualTo(ChangeStatus.NEW); + assertThat(info1.isPrivate).isEqualTo(true); + assertThat(info2.isPrivate).isEqualTo(true); + assertThat(info1.revisions).hasSize(1); + assertThat(info2.revisions).hasSize(1); + } + + @Sandboxed + @Test + public void pushWithDraftOptionToExistingNewChangeGetsChangeEdit() throws Exception { + String changeId = createChange().getChangeId(); + EditInfoSubject.assertThat(getEdit(changeId)).isAbsent(); + + ChangeInfo changeInfo = gApi.changes().id(changeId).get(); + ChangeStatus originalChangeStatus = changeInfo.status; + + PushOneCommit.Result result = amendChange(changeId, "refs/drafts/master"); + result.assertOkStatus(); + + changeInfo = gApi.changes().id(changeId).get(); + assertThat(changeInfo.status).isEqualTo(originalChangeStatus); + assertThat(changeInfo.isPrivate).isNull(); + assertThat(changeInfo.revisions).hasSize(1); + + EditInfoSubject.assertThat(getEdit(changeId)).isPresent(); + } + @GerritConfig(name = "receive.maxBatchCommits", value = "2") @Test public void maxBatchCommits() throws Exception { diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/DraftChangeBlockedIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/DraftChangeBlockedIT.java deleted file mode 100644 index 527f37d3d5..0000000000 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/DraftChangeBlockedIT.java +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright (C) 2014 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.acceptance.git; - -import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; - -import com.google.gerrit.acceptance.AbstractDaemonTest; -import com.google.gerrit.acceptance.NoHttpd; -import com.google.gerrit.acceptance.PushOneCommit; -import com.google.gerrit.common.data.Permission; -import org.junit.Before; -import org.junit.Test; - -@NoHttpd -public class DraftChangeBlockedIT extends AbstractDaemonTest { - - @Before - public void setUp() throws Exception { - block("refs/drafts/*", Permission.PUSH, ANONYMOUS_USERS); - } - - @Test - public void pushDraftChange_Blocked() throws Exception { - // create draft by pushing to 'refs/drafts/' - PushOneCommit.Result r = pushTo("refs/drafts/master"); - r.assertErrorStatus("cannot upload drafts"); - } - - @Test - public void pushDraftChangeMagic_Blocked() throws Exception { - // create draft by using 'draft' option - PushOneCommit.Result r = pushTo("refs/for/master%draft"); - r.assertErrorStatus("cannot upload drafts"); - } -} diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/RefAdvertisementIT.java index 4dac61fe04..a59103f86c 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/RefAdvertisementIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/RefAdvertisementIT.java @@ -319,53 +319,6 @@ public class RefAdvertisementIT extends AbstractDaemonTest { } } - @Test - public void uploadPackDraftRefs() throws Exception { - allow("refs/heads/*", Permission.READ, REGISTERED_USERS); - - PushOneCommit.Result br = createDraftChange(); - br.assertOkStatus(); - Change.Id c5 = br.getChange().getId(); - String r5 = changeRefPrefix(c5); - - // Only admin can see admin's draft change (5). - setApiUser(admin); - assertUploadPackRefs( - "HEAD", - r1 + "1", - r1 + "meta", - r2 + "1", - r2 + "meta", - r3 + "1", - r3 + "meta", - r4 + "1", - r4 + "meta", - r5 + "1", - r5 + "meta", - "refs/heads/branch", - "refs/heads/master", - RefNames.REFS_CONFIG, - "refs/tags/branch-tag", - "refs/tags/master-tag"); - - // user can't. - setApiUser(user); - assertUploadPackRefs( - "HEAD", - r1 + "1", - r1 + "meta", - r2 + "1", - r2 + "meta", - r3 + "1", - r3 + "meta", - r4 + "1", - r4 + "meta", - "refs/heads/branch", - "refs/heads/master", - "refs/tags/branch-tag", - "refs/tags/master-tag"); - } - @Test public void uploadPackNoSearchingChangeCacheImpl() throws Exception { allow("refs/heads/*", Permission.READ, REGISTERED_USERS); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java index c30e9037e9..e1e5b474c9 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java @@ -169,12 +169,6 @@ public class SubmitOnPushIT extends AbstractDaemonTest { r.assertErrorStatus("update by submit not permitted"); } - @Test - public void submitOnPushingDraft_Error() throws Exception { - PushOneCommit.Result r = pushTo("refs/for/master%draft,submit"); - r.assertErrorStatus(); - } - @Test public void submitOnPushToNonExistingBranch_Error() throws Exception { String branchName = "non-existing"; diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java index d8f236222b..682b5bc83a 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java @@ -520,20 +520,6 @@ public abstract class AbstractSubmit extends AbstractDaemonTest { .containsExactlyElementsIn(expected); } - @Test - public void submitDraftChange() throws Exception { - PushOneCommit.Result draft = createDraftChange(); - Change.Id num = draft.getChange().getId(); - submitWithConflict( - draft.getChangeId(), - "Failed to submit 1 change due to the following problems:\n" - + "Change " - + num - + ": Change " - + num - + " is draft"); - } - @Test public void submitWorkInProgressChange() throws Exception { PushOneCommit.Result change = createWorkInProgressChange(); @@ -548,21 +534,6 @@ public abstract class AbstractSubmit extends AbstractDaemonTest { + " is work in progress"); } - @Test - public void submitDraftPatchSet() throws Exception { - PushOneCommit.Result change = createChange(); - PushOneCommit.Result draft = amendChangeAndMarkPatchSetAsDraft(change.getChangeId()); - Change.Id num = draft.getChange().getId(); - - submitWithConflict( - draft.getChangeId(), - "Failed to submit 1 change due to the following problems:\n" - + "Change " - + num - + ": submit rule error: " - + "Cannot submit draft patch sets"); - } - @Test public void submitWithHiddenBranchInSameTopic() throws Exception { assume().that(isSubmitWholeTopicEnabled()).isTrue(); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java index e188afd5ea..72be3211c8 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java @@ -139,28 +139,6 @@ public class ActionsIT extends AbstractDaemonTest { } } - @Test - public void revisionActionsETagWithHiddenDraftInTopic() throws Exception { - String change = createChangeWithTopic().getChangeId(); - approve(change); - - setApiUser(user); - String etag1 = getETag(change); - - setApiUser(admin); - String draft = createDraftChange("topic").getChangeId(); - approve(draft); - - setApiUser(user); - String etag2 = getETag(change); - - if (isSubmitWholeTopicEnabled()) { - assertThat(etag2).isNotEqualTo(etag1); - } else { - assertThat(etag2).isEqualTo(etag1); - } - } - @Test public void revisionActionsAnonymousETag() throws Exception { String parent = createChange().getChangeId(); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java index 93bb3fabed..f4526e5800 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java @@ -276,19 +276,6 @@ public class ChangeReviewersByEmailIT extends AbstractDaemonTest { assertThat(result.reviewers).isNull(); } - @Test - public void rejectOnNonPublicChange() throws Exception { - assume().that(notesMigration.readChanges()).isTrue(); - PushOneCommit.Result r = createDraftChange(); - - AddReviewerResult result = - gApi.changes().id(r.getChangeId()).addReviewer("Foo Bar "); - assertThat(result.error) - .isEqualTo( - "Foo Bar does not have permission to see this change"); - assertThat(result.reviewers).isNull(); - } - @Test public void rejectWhenFeatureIsDisabled() throws Exception { assume().that(notesMigration.readChanges()).isTrue(); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java index 459fe4cfca..b8b56ce442 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java @@ -24,9 +24,7 @@ import static org.eclipse.jgit.lib.Constants.SIGNED_OFF_BY_TAG; import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Iterables; import com.google.gerrit.acceptance.AbstractDaemonTest; -import com.google.gerrit.acceptance.GerritConfig; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.PushOneCommit.Result; import com.google.gerrit.acceptance.RestResponse; @@ -138,13 +136,6 @@ public class CreateChangeIT extends AbstractDaemonTest { "%sAdministrator <%s>", SIGNED_OFF_BY_TAG, admin.getIdent().getEmailAddress())); } - @Test - @GerritConfig(name = "change.allowDrafts", value = "true") - public void createNewDraftChangeNotAllowed() throws Exception { - ChangeInput ci = newChangeInput(ChangeStatus.DRAFT); - assertCreateFails(ci, BadRequestException.class, "unsupported change status"); - } - @Test public void createNewPrivateChange() throws Exception { ChangeInput input = newChangeInput(ChangeStatus.NEW); @@ -388,9 +379,7 @@ public class CreateChangeIT extends AbstractDaemonTest { assertThat(out.workInProgress).isEqualTo(in.workInProgress); assertThat(out.revisions).hasSize(1); assertThat(out.submitted).isNull(); - assertThat(out.submitter).isNull(); - Boolean draft = Iterables.getOnlyElement(out.revisions.values()).draft; - assertThat(booleanToDraftStatus(draft)).isEqualTo(in.status); + assertThat(in.status).isEqualTo(ChangeStatus.NEW); return out; } @@ -402,13 +391,6 @@ public class CreateChangeIT extends AbstractDaemonTest { gApi.changes().create(in); } - private ChangeStatus booleanToDraftStatus(Boolean draft) { - if (draft == null) { - return ChangeStatus.NEW; - } - return draft ? ChangeStatus.DRAFT : ChangeStatus.NEW; - } - // TODO(davido): Expose setting of account preferences in the API private void setSignedOffByFooter() throws Exception { RestResponse r = adminRestSession.get("/accounts/" + admin.email + "/preferences"); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/DeleteDraftPatchSetIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/DeleteDraftPatchSetIT.java deleted file mode 100644 index 0a18b58a39..0000000000 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/DeleteDraftPatchSetIT.java +++ /dev/null @@ -1,261 +0,0 @@ -// Copyright (C) 2013 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.acceptance.rest.change; - -import static com.google.common.truth.Truth.assertThat; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterables; -import com.google.gerrit.acceptance.AbstractDaemonTest; -import com.google.gerrit.acceptance.NoHttpd; -import com.google.gerrit.acceptance.PushOneCommit; -import com.google.gerrit.acceptance.TestAccount; -import com.google.gerrit.extensions.api.changes.DraftInput; -import com.google.gerrit.extensions.api.changes.ReviewInput; -import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput; -import com.google.gerrit.extensions.client.ChangeStatus; -import com.google.gerrit.extensions.client.Side; -import com.google.gerrit.extensions.common.ChangeInfo; -import com.google.gerrit.extensions.restapi.ResourceConflictException; -import com.google.gerrit.extensions.restapi.ResourceNotFoundException; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.ChangeMessage; -import com.google.gerrit.reviewdb.client.Comment; -import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.reviewdb.client.RefNames; -import com.google.gerrit.server.config.AllUsersName; -import com.google.gerrit.server.query.change.ChangeData; -import com.google.inject.Inject; -import java.util.HashMap; -import org.eclipse.jgit.lib.Ref; -import org.eclipse.jgit.lib.Repository; -import org.junit.Test; - -@NoHttpd -public class DeleteDraftPatchSetIT extends AbstractDaemonTest { - - @Inject private AllUsersName allUsers; - - @Test - public void deletePatchSetNotDraft() throws Exception { - String changeId = createChange().getChangeId(); - PatchSet ps = getCurrentPatchSet(changeId); - String triplet = project.get() + "~master~" + changeId; - ChangeInfo c = get(triplet); - assertThat(c.id).isEqualTo(triplet); - assertThat(c.status).isEqualTo(ChangeStatus.NEW); - - exception.expect(ResourceConflictException.class); - exception.expectMessage("Patch set is not a draft"); - setApiUser(admin); - deletePatchSet(changeId, ps); - } - - @Test - public void deleteDraftPatchSetNoACL() throws Exception { - String changeId = createDraftChangeWith2PS(); - PatchSet ps = getCurrentPatchSet(changeId); - String triplet = project.get() + "~master~" + changeId; - ChangeInfo c = get(triplet); - assertThat(c.id).isEqualTo(triplet); - assertThat(c.status).isEqualTo(ChangeStatus.DRAFT); - - exception.expect(ResourceNotFoundException.class); - exception.expectMessage("Not found: " + changeId); - setApiUser(user); - deletePatchSet(changeId, ps); - } - - @Test - public void deleteDraftPatchSetAndChange() throws Exception { - String changeId = createDraftChangeWith2PS(); - PatchSet ps = getCurrentPatchSet(changeId); - Change.Id id = ps.getId().getParentKey(); - - DraftInput din = new DraftInput(); - din.path = "a.txt"; - din.message = "comment on a.txt"; - gApi.changes().id(changeId).current().createDraft(din); - - if (notesMigration.commitChangeWrites()) { - assertThat(getDraftRef(admin, id)).isNotNull(); - } - - ChangeData cd = getChange(changeId); - assertThat(cd.patchSets()).hasSize(2); - assertThat(cd.change().currentPatchSetId().get()).isEqualTo(2); - assertThat(cd.change().getStatus()).isEqualTo(Change.Status.DRAFT); - deletePatchSet(changeId, ps); - - cd = getChange(changeId); - assertThat(cd.patchSets()).hasSize(1); - assertThat(cd.change().currentPatchSetId().get()).isEqualTo(1); - - ps = getCurrentPatchSet(changeId); - deletePatchSet(changeId, ps); - assertThat(queryProvider.get().byKeyPrefix(changeId)).isEmpty(); - - if (notesMigration.commitChangeWrites()) { - assertThat(getDraftRef(admin, id)).isNull(); - assertThat(getMetaRef(id)).isNull(); - } - - exception.expect(ResourceNotFoundException.class); - gApi.changes().id(id.get()); - } - - @Test - public void deleteDraftPS1() throws Exception { - String changeId = createDraftChangeWith2PS(); - - ReviewInput rin = new ReviewInput(); - rin.message = "Change message"; - CommentInput cin = new CommentInput(); - cin.line = 1; - cin.patchSet = 1; - cin.path = PushOneCommit.FILE_NAME; - cin.side = Side.REVISION; - cin.message = "Inline comment"; - rin.comments = new HashMap<>(); - rin.comments.put(cin.path, ImmutableList.of(cin)); - gApi.changes().id(changeId).revision(1).review(rin); - - ChangeData cd = getChange(changeId); - PatchSet.Id delPsId = new PatchSet.Id(cd.getId(), 1); - PatchSet ps = cd.patchSet(delPsId); - deletePatchSet(changeId, ps); - - cd = getChange(changeId); - assertThat(cd.patchSets()).hasSize(1); - assertThat(Iterables.getOnlyElement(cd.patchSets()).getId().get()).isEqualTo(2); - - // Other entities based on deleted patch sets are also deleted. - for (ChangeMessage m : cd.messages()) { - assertThat(m.getPatchSetId()).named(m.toString()).isNotEqualTo(delPsId); - } - for (Comment c : cd.publishedComments()) { - assertThat(c.key.patchSetId).named(c.toString()).isNotEqualTo(delPsId.get()); - } - } - - @Test - public void deleteDraftPS2() throws Exception { - String changeId = createDraftChangeWith2PS(); - - ReviewInput rin = new ReviewInput(); - rin.message = "Change message"; - CommentInput cin = new CommentInput(); - cin.line = 1; - cin.patchSet = 1; - cin.path = PushOneCommit.FILE_NAME; - cin.side = Side.REVISION; - cin.message = "Inline comment"; - rin.comments = new HashMap<>(); - rin.comments.put(cin.path, ImmutableList.of(cin)); - gApi.changes().id(changeId).revision(1).review(rin); - - ChangeData cd = getChange(changeId); - PatchSet.Id delPsId = new PatchSet.Id(cd.getId(), 2); - PatchSet ps = cd.patchSet(delPsId); - deletePatchSet(changeId, ps); - - cd = getChange(changeId); - assertThat(cd.patchSets()).hasSize(1); - assertThat(Iterables.getOnlyElement(cd.patchSets()).getId().get()).isEqualTo(1); - - // Other entities based on deleted patch sets are also deleted. - for (ChangeMessage m : cd.messages()) { - assertThat(m.getPatchSetId()).named(m.toString()).isNotEqualTo(delPsId); - } - for (Comment c : cd.publishedComments()) { - assertThat(c.key.patchSetId).named(c.toString()).isNotEqualTo(delPsId.get()); - } - } - - @Test - public void deleteCurrentDraftPatchSetWhenPreviousPatchSetDoesNotExist() throws Exception { - String changeId = createChange().getChangeId(); - amendChangeAndMarkPatchSetAsDraft(changeId); - amendChangeAndMarkPatchSetAsDraft(changeId); - - deletePatchSet(changeId, 2); - deletePatchSet(changeId, 3); - - ChangeData cd = getChange(changeId); - assertThat(cd.patchSets()).hasSize(1); - assertThat(Iterables.getOnlyElement(cd.patchSets()).getId().get()).isEqualTo(1); - assertThat(cd.currentPatchSet().getId().get()).isEqualTo(1); - } - - @Test - public void deleteDraftPatchSetAndPushNewDraftPatchSet() throws Exception { - // Create change - PushOneCommit.Result r1 = createDraftChange(); - String changeId = r1.getChangeId(); - String revPs1 = r1.getChange().currentPatchSet().getRevision().get(); - - // Push draft patch set - PushOneCommit.Result r2 = amendChangeAndMarkPatchSetAsDraft(changeId); - String revPs2 = r2.getChange().currentPatchSet().getRevision().get(); - - assertThat(gApi.changes().id(r1.getChange().getId().get()).get().currentRevision) - .isEqualTo(revPs2); - - // Remove draft patch set - gApi.changes().id(r1.getChange().getId().get()).revision(revPs2).delete(); - - assertThat(gApi.changes().id(r1.getChange().getId().get()).get().currentRevision) - .isEqualTo(revPs1); - - // Push new draft patch set - amendChangeAndMarkPatchSetAsDraft(changeId); - String revPs3 = r2.getChange().currentPatchSet().getRevision().get(); - - assertThat(gApi.changes().id(r1.getChange().getId().get()).get().currentRevision) - .isEqualTo(revPs3); - - // Check that all patch sets have different SHA1s - assertThat(revPs1).doesNotMatch(revPs2); - assertThat(revPs2).doesNotMatch(revPs3); - } - - private Ref getDraftRef(TestAccount account, Change.Id changeId) throws Exception { - try (Repository repo = repoManager.openRepository(allUsers)) { - return repo.exactRef(RefNames.refsDraftComments(changeId, account.id)); - } - } - - private Ref getMetaRef(Change.Id changeId) throws Exception { - try (Repository repo = repoManager.openRepository(project)) { - return repo.exactRef(RefNames.changeMetaRef(changeId)); - } - } - - private PatchSet getCurrentPatchSet(String changeId) throws Exception { - return getChange(changeId).currentPatchSet(); - } - - private ChangeData getChange(String changeId) throws Exception { - return Iterables.getOnlyElement(queryProvider.get().byKeyPrefix(changeId)); - } - - private void deletePatchSet(String changeId, PatchSet ps) throws Exception { - deletePatchSet(changeId, ps.getId().get()); - } - - private void deletePatchSet(String changeId, int ps) throws Exception { - gApi.changes().id(changeId).revision(ps).delete(); - } -} diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/DraftChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/DraftChangeIT.java deleted file mode 100644 index dace621b94..0000000000 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/DraftChangeIT.java +++ /dev/null @@ -1,295 +0,0 @@ -// Copyright (C) 2013 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.acceptance.rest.change; - -import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.TruthJUnit.assume; - -import com.google.common.collect.Iterables; -import com.google.gerrit.acceptance.AbstractDaemonTest; -import com.google.gerrit.acceptance.PushOneCommit; -import com.google.gerrit.acceptance.RestResponse; -import com.google.gerrit.acceptance.RestSession; -import com.google.gerrit.acceptance.TestProjectInput; -import com.google.gerrit.common.data.Permission; -import com.google.gerrit.extensions.api.changes.ReviewInput; -import com.google.gerrit.extensions.client.ChangeStatus; -import com.google.gerrit.extensions.client.ReviewerState; -import com.google.gerrit.extensions.common.AccountInfo; -import com.google.gerrit.extensions.common.ChangeInfo; -import com.google.gerrit.extensions.common.EditInfo; -import com.google.gerrit.extensions.common.EditInfoSubject; -import com.google.gerrit.extensions.common.LabelInfo; -import com.google.gerrit.extensions.restapi.AuthException; -import com.google.gerrit.extensions.restapi.MethodNotAllowedException; -import com.google.gerrit.extensions.restapi.ResourceConflictException; -import com.google.gerrit.extensions.restapi.ResourceNotFoundException; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.testutil.ConfigSuite; -import java.util.Collection; -import java.util.Optional; -import org.eclipse.jgit.lib.Config; -import org.junit.Test; - -public class DraftChangeIT extends AbstractDaemonTest { - @ConfigSuite.Config - public static Config allowDraftsDisabled() { - return allowDraftsDisabledConfig(); - } - - @Test - public void forceCreateAndPublishDraftChangeWhenAllowDraftsDisabled() throws Exception { - PushOneCommit.Result result = createDraftChange(); - result.assertOkStatus(); - String changeId = result.getChangeId(); - String triplet = project.get() + "~master~" + changeId; - ChangeInfo c = get(triplet); - assertThat(c.id).isEqualTo(triplet); - assertThat(c.status).isEqualTo(ChangeStatus.DRAFT); - assertThat(c.revisions.get(c.currentRevision).draft).isTrue(); - publishPatchSet(changeId).assertNoContent(); - assertThat(get(triplet).status).isEqualTo(ChangeStatus.NEW); - } - - @Test - public void deleteDraftChange() throws Exception { - assume().that(isAllowDrafts()).isTrue(); - PushOneCommit.Result result = createDraftChange(); - result.assertOkStatus(); - String changeId = result.getChangeId(); - String triplet = project.get() + "~master~" + changeId; - ChangeInfo c = get(triplet); - assertThat(c.id).isEqualTo(triplet); - assertThat(c.status).isEqualTo(ChangeStatus.DRAFT); - deleteChange(changeId, adminRestSession).assertNoContent(); - - exception.expect(ResourceNotFoundException.class); - get(triplet); - } - - @Test - public void deleteDraftChangeOfAnotherUser() throws Exception { - assume().that(isAllowDrafts()).isTrue(); - PushOneCommit.Result changeResult = createDraftChange(); - changeResult.assertOkStatus(); - String changeId = changeResult.getChangeId(); - - // The user needs to be able to see the draft change (which reviewers can). - gApi.changes().id(changeId).addReviewer(user.fullName); - - setApiUser(user); - exception.expect(AuthException.class); - exception.expectMessage("delete not permitted"); - gApi.changes().id(changeId).delete(); - } - - @Test - @TestProjectInput(cloneAs = "user") - public void deleteDraftChangeWhenDraftsNotAllowedAsNormalUser() throws Exception { - assume().that(isAllowDrafts()).isFalse(); - - setApiUser(user); - // We can't create a draft change while the draft workflow is disabled. - // For this reason, we create a normal change and modify the database. - PushOneCommit.Result changeResult = - pushFactory.create(db, user.getIdent(), testRepo).to("refs/for/master"); - Change.Id id = changeResult.getChange().getId(); - markChangeAsDraft(id); - setDraftStatusOfPatchSets(id, true); - - String changeId = changeResult.getChangeId(); - exception.expect(MethodNotAllowedException.class); - exception.expectMessage("Draft workflow is disabled"); - gApi.changes().id(changeId).delete(); - } - - @Test - @TestProjectInput(cloneAs = "user") - public void deleteDraftChangeWhenDraftsNotAllowedAsAdmin() throws Exception { - assume().that(isAllowDrafts()).isFalse(); - - setApiUser(user); - // We can't create a draft change while the draft workflow is disabled. - // For this reason, we create a normal change and modify the database. - PushOneCommit.Result changeResult = - pushFactory.create(db, user.getIdent(), testRepo).to("refs/for/master"); - Change.Id id = changeResult.getChange().getId(); - markChangeAsDraft(id); - setDraftStatusOfPatchSets(id, true); - - String changeId = changeResult.getChangeId(); - - // Grant those permissions to admins. - grant(project, "refs/*", Permission.VIEW_DRAFTS); - grant(project, "refs/*", Permission.DELETE_DRAFTS); - - try { - setApiUser(admin); - gApi.changes().id(changeId).delete(); - } finally { - removePermission(project, "refs/*", Permission.DELETE_DRAFTS); - removePermission(project, "refs/*", Permission.VIEW_DRAFTS); - } - - setApiUser(user); - assertThat(query(changeId)).isEmpty(); - } - - @Test - public void deleteDraftChangeWithNonDraftPatchSet() throws Exception { - assume().that(isAllowDrafts()).isTrue(); - - PushOneCommit.Result changeResult = createDraftChange(); - Change.Id id = changeResult.getChange().getId(); - setDraftStatusOfPatchSets(id, false); - - String changeId = changeResult.getChangeId(); - exception.expect(ResourceConflictException.class); - exception.expectMessage( - String.format("Cannot delete draft change %s: patch set 1 is not a draft", id)); - gApi.changes().id(changeId).delete(); - } - - @Test - public void publishDraftChange() throws Exception { - assume().that(isAllowDrafts()).isTrue(); - PushOneCommit.Result result = createDraftChange(); - result.assertOkStatus(); - String changeId = result.getChangeId(); - String triplet = project.get() + "~master~" + changeId; - ChangeInfo c = get(triplet); - assertThat(c.id).isEqualTo(triplet); - assertThat(c.status).isEqualTo(ChangeStatus.DRAFT); - assertThat(c.revisions.get(c.currentRevision).draft).isTrue(); - publishChange(changeId).assertNoContent(); - c = get(triplet); - assertThat(c.status).isEqualTo(ChangeStatus.NEW); - assertThat(c.revisions.get(c.currentRevision).draft).isNull(); - } - - @Test - public void publishDraftPatchSet() throws Exception { - assume().that(isAllowDrafts()).isTrue(); - PushOneCommit.Result result = createDraftChange(); - result.assertOkStatus(); - String changeId = result.getChangeId(); - String triplet = project.get() + "~master~" + changeId; - ChangeInfo c = get(triplet); - assertThat(c.id).isEqualTo(triplet); - assertThat(c.status).isEqualTo(ChangeStatus.DRAFT); - publishPatchSet(changeId).assertNoContent(); - assertThat(get(triplet).status).isEqualTo(ChangeStatus.NEW); - } - - @Test - public void createDraftChangeWhenDraftsNotAllowed() throws Exception { - assume().that(isAllowDrafts()).isFalse(); - PushOneCommit.Result r = pushTo("refs/drafts/master"); - r.assertErrorStatus(); - } - - @Test - public void listApprovalsOnDraftChange() throws Exception { - assume().that(isAllowDrafts()).isTrue(); - PushOneCommit.Result result = createDraftChange(); - result.assertOkStatus(); - String changeId = result.getChangeId(); - String triplet = project.get() + "~master~" + changeId; - - gApi.changes().id(triplet).addReviewer(user.fullName); - - ChangeInfo info = get(triplet); - LabelInfo label = info.labels.get("Code-Review"); - assertThat(label.all).hasSize(1); - assertThat(label.all.get(0)._accountId).isEqualTo(user.id.get()); - assertThat(label.all.get(0).value).isEqualTo(0); - - Collection ccs = info.reviewers.get(ReviewerState.REVIEWER); - assertThat(ccs).hasSize(1); - assertThat(ccs.iterator().next()._accountId).isEqualTo(user.id.get()); - - setApiUser(user); - gApi.changes().id(triplet).current().review(ReviewInput.recommend()); - setApiUser(admin); - - label = get(triplet).labels.get("Code-Review"); - assertThat(label.all).hasSize(1); - assertThat(label.all.get(0)._accountId).isEqualTo(user.id.get()); - assertThat(label.all.get(0).value).isEqualTo(1); - } - - @Test - public void pushWithDraftOptionGetsPrivateChange() throws Exception { - assume().that(isAllowDrafts()).isTrue(); - PushOneCommit.Result result = createChange("refs/drafts/master"); - String changeId = result.getChangeId(); - ChangeInfo changeInfo = gApi.changes().id(changeId).get(); - - assertThat(changeInfo.status).isEqualTo(ChangeStatus.NEW); - assertThat(changeInfo.isPrivate).isEqualTo(true); - assertThat(changeInfo.revisions).hasSize(1); - assertThat(Iterables.getOnlyElement(changeInfo.revisions.values()).draft).isNull(); - } - - @Test - public void pushWithDraftOptionToExistingNewChangeGetsChangeEdit() throws Exception { - assume().that(isAllowDrafts()).isTrue(); - pushWithDraftOptionToExistingChangeGetsChangeEdit(createChange().getChangeId()); - } - - @Test - public void pushWithDraftOptionToExistingDraftChangeGetsChangeEdit() throws Exception { - assume().that(isAllowDrafts()).isTrue(); - pushWithDraftOptionToExistingChangeGetsChangeEdit(createDraftChange().getChangeId()); - } - - private void pushWithDraftOptionToExistingChangeGetsChangeEdit(String changeId) throws Exception { - Optional edit = getEdit(changeId); - EditInfoSubject.assertThat(edit).isAbsent(); - - ChangeInfo changeInfo = gApi.changes().id(changeId).get(); - ChangeStatus originalChangeStatus = changeInfo.status; - Boolean originalPatchSetStatus = Iterables.getOnlyElement(changeInfo.revisions.values()).draft; - - PushOneCommit.Result result = amendChange(changeId, "refs/drafts/master"); - result.assertOkStatus(); - - changeInfo = gApi.changes().id(changeId).get(); - assertThat(changeInfo.status).isEqualTo(originalChangeStatus); - assertThat(changeInfo.isPrivate).isNull(); - assertThat(changeInfo.revisions).hasSize(1); - assertThat(Iterables.getOnlyElement(changeInfo.revisions.values()).draft) - .isEqualTo(originalPatchSetStatus); - - edit = getEdit(changeId); - EditInfoSubject.assertThat(edit).isPresent(); - } - - private static RestResponse deleteChange(String changeId, RestSession s) throws Exception { - return s.delete("/changes/" + changeId); - } - - private RestResponse publishChange(String changeId) throws Exception { - return adminRestSession.post("/changes/" + changeId + "/publish"); - } - - private RestResponse publishPatchSet(String changeId) throws Exception { - PatchSet patchSet = - Iterables.getOnlyElement(queryProvider.get().byKeyPrefix(changeId)).currentPatchSet(); - return adminRestSession.post( - "/changes/" + changeId + "/revisions/" + patchSet.getRevision().get() + "/publish"); - } -} diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java index 0ac263f299..bb4abe1a61 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java @@ -19,7 +19,6 @@ import static org.junit.Assert.fail; import com.google.gerrit.acceptance.GitUtil; import com.google.gerrit.acceptance.PushOneCommit; -import com.google.gerrit.acceptance.TestProjectInput; import com.google.gerrit.extensions.api.changes.ChangeApi; import com.google.gerrit.extensions.api.changes.CherryPickInput; import com.google.gerrit.extensions.api.changes.ReviewInput; @@ -509,30 +508,6 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge { assertChangeMergedEvents(); } - @Test - @TestProjectInput(createEmptyCommit = false) - public void mergeWithMissingChange() throws Exception { - // create a draft change - PushOneCommit.Result draftResult = createDraftChange(); - - // create a new change based on the draft change - PushOneCommit.Result changeResult = createChange(); - - // delete the draft change - gApi.changes().id(draftResult.getChangeId()).delete(); - - // approve and submit the change - submitWithConflict( - changeResult.getChangeId(), - "Failed to submit 1 change due to the following problems:\n" - + "Change " - + changeResult.getChange().getId() - + ": depends on change that was not submitted"); - - assertRefUpdatedEvents(); - assertChangeMergedEvents(); - } - @Test public void testPreviewSubmitTgz() throws Exception { Project.NameKey p1 = createProject("project-name"); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java index 0f7e7dc54c..49588e7a03 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java @@ -25,15 +25,11 @@ import com.google.gerrit.extensions.api.changes.SubmittedTogetherInfo; import com.google.gerrit.extensions.client.ChangeStatus; import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.extensions.client.SubmitType; -import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.FileInfo; import com.google.gerrit.extensions.common.RevisionInfo; -import com.google.gerrit.extensions.restapi.AuthException; -import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.testutil.ConfigSuite; import java.util.EnumSet; -import java.util.List; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.revwalk.RevCommit; @@ -155,101 +151,6 @@ public class SubmittedTogetherIT extends AbstractDaemonTest { } } - @Test - public void hiddenDraftInTopic() throws Exception { - String id1 = createChange("subject", "a", "1", "topic").getChangeId(); - createDraftChange("topic"); - - setApiUser(user); - SubmittedTogetherInfo result = - gApi.changes().id(id1).submittedTogether(EnumSet.of(NON_VISIBLE_CHANGES)); - - if (isSubmitWholeTopicEnabled()) { - assertThat(result.changes).hasSize(1); - assertThat(result.changes.get(0).changeId).isEqualTo(id1); - assertThat(result.nonVisibleChanges).isEqualTo(1); - } else { - assertThat(result.changes).isEmpty(); - assertThat(result.nonVisibleChanges).isEqualTo(0); - } - } - - @Test - public void hiddenDraftInTopicOldApi() throws Exception { - String id1 = createChange("subject", "a", "1", "topic").getChangeId(); - createDraftChange("topic"); - - setApiUser(user); - if (isSubmitWholeTopicEnabled()) { - exception.expect(AuthException.class); - exception.expectMessage("change would be submitted with a change that you cannot see"); - gApi.changes().id(id1).submittedTogether(); - } else { - List result = gApi.changes().id(id1).submittedTogether(); - assertThat(result).isEmpty(); - } - } - - @Test - public void draftPatchSetInTopic() throws Exception { - RevCommit initialHead = getRemoteHead(); - RevCommit a1 = commitBuilder().add("a", "1").message("change 1").create(); - pushHead(testRepo, "refs/for/master/" + name("topic"), false); - String id1 = getChangeId(a1); - - testRepo.reset(initialHead); - String parentId = createChange().getChangeId(); - - // TODO(jrn): use insertChangeId(id1) once jgit TestRepository accepts the leading "I". - commitBuilder() - .insertChangeId(id1.substring(1)) - .add("a", "2") - .message("draft patch set on change 1") - .create(); - pushHead(testRepo, "refs/for/master/" + name("topic"), false); - setCurrentPatchSetAsDraft(new Change.Id(gApi.changes().id(id1).get()._number)); - - testRepo.reset(initialHead); - RevCommit b = commitBuilder().message("change with same topic").create(); - pushHead(testRepo, "refs/for/master/" + name("topic"), false); - String id2 = getChangeId(b); - - if (isSubmitWholeTopicEnabled()) { - setApiUser(user); - assertSubmittedTogether(id2, id2, id1); - setApiUser(admin); - assertSubmittedTogether(id2, id2, id1, parentId); - } else { - setApiUser(user); - assertSubmittedTogether(id2); - setApiUser(admin); - assertSubmittedTogether(id2); - } - } - - @Test - public void doNotRevealVisibleAncestorOfHiddenDraft() throws Exception { - RevCommit initialHead = getRemoteHead(); - createChange().getChangeId(); - - createDraftChange("topic"); - - testRepo.reset(initialHead); - String id = createChange("subject", "b", "1", "topic").getChangeId(); - - setApiUser(user); - SubmittedTogetherInfo result = - gApi.changes().id(id).submittedTogether(EnumSet.of(NON_VISIBLE_CHANGES)); - if (isSubmitWholeTopicEnabled()) { - assertThat(result.changes).hasSize(1); - assertThat(result.changes.get(0).changeId).isEqualTo(id); - assertThat(result.nonVisibleChanges).isEqualTo(2); - } else { - assertThat(result.changes).isEmpty(); - assertThat(result.nonVisibleChanges).isEqualTo(0); - } - } - @Test public void topicChaining() throws Exception { RevCommit initialHead = getRemoteHead(); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java index 59ccaddf03..0324ffa254 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java @@ -42,7 +42,6 @@ import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput; import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling; import com.google.gerrit.extensions.client.Side; -import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.CommentInfo; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestApiException; @@ -816,20 +815,6 @@ public class ChangeRebuilderIT extends AbstractDaemonTest { assertThat(nc.getOriginalSubject()).isEqualTo(orig); } - @Test - public void deleteDraftPS1WithNoOtherEntities() throws Exception { - String r = createDraftChangeWith2PS(); - gApi.changes().id(r).revision(1).delete(); - ChangeInfo changeInfo = get(r); - - Change.Id id = new Change.Id(changeInfo._number); - checker.rebuildAndCheckChanges(id); - - setNotesMigration(true, true); - ChangeNotes notes = notesFactory.create(db, project, id); - assertThat(notes.getPatchSets().keySet()).hasSize(1); - } - @Test public void ignorePatchLineCommentsOnPatchSet0() throws Exception { PushOneCommit.Result r = createChange(); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/QueryIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/QueryIT.java index ca45e7c31c..1d5af97e9d 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/QueryIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/QueryIT.java @@ -16,7 +16,6 @@ package com.google.gerrit.acceptance.ssh; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; -import static com.google.gerrit.acceptance.GitUtil.initSsh; import com.google.common.collect.Lists; import com.google.gerrit.acceptance.AbstractDaemonTest; @@ -290,27 +289,6 @@ public class QueryIT extends AbstractDaemonTest { assertThat(changes.get(0).submitRecords.size()).isEqualTo(1); } - @Test - public void queryWithNonVisibleCurrentPatchSet() throws Exception { - String changeId = createChange().getChangeId(); - amendChangeAndMarkPatchSetAsDraft(changeId); - String query = "--current-patch-set --patch-sets " + changeId; - List changes = executeSuccessfulQuery(query); - assertThat(changes.size()).isEqualTo(1); - assertThat(changes.get(0).patchSets).isNotNull(); - assertThat(changes.get(0).patchSets).hasSize(2); - assertThat(changes.get(0).currentPatchSet).isNotNull(); - - SshSession userSession = new SshSession(server, user); - initSsh(user); - userSession.open(); - changes = executeSuccessfulQuery(query, userSession); - assertThat(changes.size()).isEqualTo(1); - assertThat(changes.get(0).patchSets).hasSize(1); - assertThat(changes.get(0).currentPatchSet).isNull(); - userSession.close(); - } - @Test public void allChangeOptionsAreServedWithoutExceptions() throws Exception { PushOneCommit.Result r = createChange(); diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/PageLinks.java b/gerrit-common/src/main/java/com/google/gerrit/common/PageLinks.java index 3a14be7089..97e7ff3312 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/PageLinks.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/PageLinks.java @@ -142,7 +142,6 @@ public class PageLinks { switch (status) { case ABANDONED: return toChangeQuery(status(status) + " " + op("topic", topic)); - case DRAFT: case MERGED: case NEW: return toChangeQuery( @@ -169,7 +168,6 @@ public class PageLinks { return "status:abandoned"; case MERGED: return "status:merged"; - case DRAFT: case NEW: default: return "status:open"; diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java index 81646fe65e..7afc123e47 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java @@ -27,7 +27,6 @@ public class Permission implements Comparable { public static final String DELETE = "delete"; public static final String CREATE_TAG = "createTag"; public static final String CREATE_SIGNED_TAG = "createSignedTag"; - public static final String DELETE_DRAFTS = "deleteDrafts"; public static final String DELETE_OWN_CHANGES = "deleteOwnChanges"; public static final String EDIT_HASHTAGS = "editHashtags"; public static final String EDIT_ASSIGNEE = "editAssignee"; @@ -38,7 +37,6 @@ public class Permission implements Comparable { public static final String LABEL = "label-"; public static final String LABEL_AS = "labelAs-"; public static final String OWNER = "owner"; - public static final String PUBLISH_DRAFTS = "publishDrafts"; public static final String PUSH = "push"; public static final String PUSH_MERGE = "pushMerge"; public static final String READ = "read"; @@ -46,7 +44,6 @@ public class Permission implements Comparable { public static final String REMOVE_REVIEWER = "removeReviewer"; 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 NAMES_LC; @@ -74,14 +71,11 @@ public class Permission implements Comparable { NAMES_LC.add(REMOVE_REVIEWER.toLowerCase()); 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()); - NAMES_LC.add(DELETE_DRAFTS.toLowerCase()); NAMES_LC.add(DELETE_OWN_CHANGES.toLowerCase()); - NAMES_LC.add(PUBLISH_DRAFTS.toLowerCase()); LABEL_INDEX = NAMES_LC.indexOf(Permission.LABEL); LABEL_AS_INDEX = NAMES_LC.indexOf(Permission.LABEL_AS.toLowerCase()); diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java index 96dfac10db..1fc04b4576 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java @@ -159,6 +159,7 @@ public interface ChangeApi { throws RestApiException; /** Publishes a draft change. */ + @Deprecated void publish() throws RestApiException; /** Rebase the current revision of a change using default options. */ @@ -403,6 +404,7 @@ public interface ChangeApi { throw new NotImplementedException(); } + @Deprecated @Override public void rebase() throws RestApiException { throw new NotImplementedException(); diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java index b7fd2332d6..72e762c5dc 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java @@ -31,6 +31,7 @@ import java.util.Map; import java.util.Set; public interface RevisionApi { + @Deprecated void delete() throws RestApiException; String description() throws RestApiException; @@ -47,6 +48,7 @@ public interface RevisionApi { BinaryResult submitPreview(String format) throws RestApiException; + @Deprecated void publish() throws RestApiException; ChangeApi cherryPick(CherryPickInput in) throws RestApiException; @@ -155,6 +157,7 @@ public interface RevisionApi { * interface. */ class NotImplemented implements RevisionApi { + @Deprecated @Override public void delete() throws RestApiException { throw new NotImplementedException(); @@ -175,6 +178,7 @@ public interface RevisionApi { throw new NotImplementedException(); } + @Deprecated @Override public void publish() throws RestApiException { throw new NotImplementedException(); diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ChangeStatus.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ChangeStatus.java index 4ecde16cec..83d5bd2cc7 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ChangeStatus.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ChangeStatus.java @@ -33,22 +33,6 @@ public enum ChangeStatus { */ NEW, - /** - * Change is a draft change that only consists of draft patchsets. - * - *

This is a change that is not meant to be submitted or reviewed yet. If the uploader - * publishes the change, it becomes a NEW change. Publishing is a one-way action, a change cannot - * return to DRAFT status. Draft changes are only visible to the uploader and those explicitly - * added as reviewers. Note that currently draft changes cannot be abandoned. - * - *

Changes in the DRAFT state can be moved to: - * - *

    - *
  • {@link #NEW} - when the change is published, it becomes a new change. - *
- */ - DRAFT, - /** * Change is closed, and submitted to its destination branch. * diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/RevisionInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/RevisionInfo.java index a3304156c5..fc443dd08a 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/RevisionInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/RevisionInfo.java @@ -20,7 +20,6 @@ import java.util.Map; public class RevisionInfo { public transient boolean isCurrent; - public Boolean draft; public ChangeKind kind; public int _number; public Timestamp created; diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/events/DraftPublishedListener.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/events/DraftPublishedListener.java deleted file mode 100644 index edbdcd8d56..0000000000 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/events/DraftPublishedListener.java +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright (C) 2015 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.extensions.events; - -import com.google.gerrit.extensions.annotations.ExtensionPoint; - -/** Notified whenever a Draft is published. */ -@ExtensionPoint -public interface DraftPublishedListener { - interface Event extends RevisionEvent {} - - void onDraftPublished(Event event); -} diff --git a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/ChangeInfo.java b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/ChangeInfo.java index 8db0ef717f..5ef3dce59f 100644 --- a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/ChangeInfo.java +++ b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/ChangeInfo.java @@ -419,8 +419,6 @@ public class ChangeInfo extends JavaScriptObject { public final native String name() /*-{ return this.name; }-*/; - public final native boolean draft() /*-{ return this.draft || false; }-*/; - public final native AccountInfo uploader() /*-{ return this.uploader; }-*/; public final native boolean isEdit() /*-{ return this._number == 0; }-*/; diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.java index b44cd1c59d..eae3431a6f 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.java @@ -91,8 +91,6 @@ public interface GerritConstants extends Constants { String menuMyChanges(); - String menuMyDrafts(); - String menuMyWatchedChanges(); String menuMyStarredChanges(); @@ -175,8 +173,6 @@ public interface GerritConstants extends Constants { String jumpMine(); - String jumpMineDrafts(); - String jumpMineWatched(); String jumpMineStarred(); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.properties index 9eb6f4cf74..2819d22e44 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.properties @@ -53,7 +53,6 @@ menuAllAbandoned = Abandoned menuMine = My menuMyChanges = Changes -menuMyDrafts = Drafts menuMyStarredChanges = Starred Changes menuMyWatchedChanges = Watched Changes menuMyDraftComments = Draft Comments @@ -105,7 +104,6 @@ jumpAllMerged = Go to all merged changes jumpAllAbandoned = Go to all abandoned changes jumpMine = Go to my dashboard jumpMineWatched = Go to watched changes -jumpMineDrafts = Go to drafts jumpMineStarred = Go to starred changes jumpMineDraftComments = Go to draft comments diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/JumpKeys.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/JumpKeys.java index cc05e128b8..a4879caae7 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/JumpKeys.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/JumpKeys.java @@ -70,13 +70,6 @@ public class JumpKeys { Gerrit.display(PageLinks.MINE); } }); - jumps.add( - new KeyCommand(0, 'd', Gerrit.C.jumpMineDrafts()) { - @Override - public void onKeyPress(KeyPressEvent event) { - Gerrit.display(PageLinks.toChangeQuery("owner:self is:draft")); - } - }); jumps.add( new KeyCommand(0, 'c', Gerrit.C.jumpMineDraftComments()) { @Override diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java index a5fc4f3d17..e46ba72954 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java @@ -129,7 +129,6 @@ public class SearchSuggestOracle extends HighlightSuggestOracle { suggestions.add("is:reviewer"); suggestions.add("is:open"); suggestions.add("is:pending"); - suggestions.add("is:draft"); suggestions.add("is:private"); suggestions.add("is:closed"); suggestions.add("is:merged"); @@ -145,7 +144,6 @@ public class SearchSuggestOracle extends HighlightSuggestOracle { suggestions.add("status:closed"); suggestions.add("status:merged"); suggestions.add("status:abandoned"); - suggestions.add("status:draft"); suggestions.add("added:"); suggestions.add("deleted:"); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties index 5521da09a6..62f37787d1 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties @@ -137,7 +137,6 @@ permissionNames = \ createTag, \ createSignedTag, \ delete, \ - deleteDrafts, \ deleteOwnChanges, \ editAssignee, \ editHashtags, \ @@ -146,7 +145,6 @@ permissionNames = \ forgeCommitter, \ forgeServerAsCommitter, \ owner, \ - publishDrafts, \ push, \ pushMerge, \ read, \ @@ -154,7 +152,6 @@ permissionNames = \ removeReviewer, \ submit, \ submitAs, \ - viewDrafts, \ viewPrivateChanges abandon = Abandon @@ -163,7 +160,6 @@ create = Create Reference createTag = Create Annotated Tag createSignedTag = Create Signed Tag delete = Delete Reference -deleteDrafts = Delete Drafts deleteOwnChanges = Delete Own Changes editAssignee = Edit Assignee editHashtags = Edit Hashtags @@ -172,7 +168,6 @@ forgeAuthor = Forge Author Identity forgeCommitter = Forge Committer Identity forgeServerAsCommitter = Forge Server Identity owner = Owner -publishDrafts = Publish Drafts push = Push pushMerge = Push Merge Commit read = Read @@ -180,7 +175,6 @@ rebase = Rebase removeReviewer = Remove Reviewer submit = Submit submitAs = Submit (On Behalf Of) -viewDrafts = View Drafts viewPrivateChanges = View Private Changes refErrorEmpty = Reference must be supplied diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeActions.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeActions.java index cf9d18599d..0bc74e4f96 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeActions.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeActions.java @@ -26,16 +26,6 @@ import com.google.gwt.user.client.ui.Button; public class ChangeActions { - static void publish( - Project.NameKey project, Change.Id id, String revision, Button... draftButtons) { - ChangeApi.publish(project.get(), id.get(), revision, cs(project, id, draftButtons)); - } - - static void delete( - Project.NameKey project, Change.Id id, String revision, Button... draftButtons) { - ChangeApi.deleteRevision(project.get(), id.get(), revision, cs(project, id, draftButtons)); - } - static void delete(Project.NameKey project, Change.Id id, Button... draftButtons) { ChangeApi.deleteChange(project.get(), id.get(), mine(draftButtons)); } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeConstants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeConstants.java index bd211b7f9b..ed67846d9a 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeConstants.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeConstants.java @@ -43,8 +43,6 @@ public interface ChangeConstants extends Constants { String author(); - String draft(); - String notAvailable(); String relatedChanges(); @@ -78,6 +76,4 @@ public interface ChangeConstants extends Constants { String deleteChangeEdit(); String deleteChange(); - - String deleteDraftRevision(); } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeConstants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeConstants.properties index dd4760d068..90001494ed 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeConstants.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeConstants.properties @@ -13,7 +13,6 @@ patchSet = Patch Set commit = Commit date = Date author = Author / Committer -draft = (DRAFT) notAvailable = N/A relatedChanges = Related Changes @@ -35,4 +34,3 @@ deleteChangeEdit = Delete Change Edit?\n\ \n\ All changes made in the edit revision will be lost. deleteChange = Delete Change? -deleteDraftRevision = Delete Draft Revision? diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java index fe8895a909..039948d678 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java @@ -234,8 +234,6 @@ public class ChangeScreen extends Screen { @UiField Button publishEdit; @UiField Button rebaseEdit; @UiField Button deleteEdit; - @UiField Button publish; - @UiField Button deleteRevision; @UiField Button openAll; @UiField Button editMode; @UiField Button reviewMode; @@ -562,8 +560,7 @@ public class ChangeScreen extends Screen { } } - private void initRevisionsAction( - ChangeInfo info, String revision, NativeMap actions) { + private void initRevisionsAction(ChangeInfo info, String revision) { int currentPatchSet; if (info.currentRevision() != null && info.revisions().containsKey(info.currentRevision())) { currentPatchSet = info.revision(info.currentRevision())._number(); @@ -591,18 +588,6 @@ public class ChangeScreen extends Screen { patchSetsAction = new PatchSetsAction( info.projectNameKey(), info.legacyId(), revision, edit, style, headerLine, patchSets); - - RevisionInfo revInfo = info.revision(revision); - if (revInfo.draft()) { - if (actions.containsKey("publish")) { - publish.setVisible(true); - publish.setTitle(actions.get("publish").title()); - } - if (actions.containsKey("/")) { - deleteRevision.setVisible(true); - deleteRevision.setTitle(actions.get("/").title()); - } - } } private void initDownloadAction(ChangeInfo info, String revision) { @@ -702,18 +687,6 @@ public class ChangeScreen extends Screen { } } - @UiHandler("publish") - void onPublish(@SuppressWarnings("unused") ClickEvent e) { - ChangeActions.publish(getProject(), changeId, revision, publish, deleteRevision); - } - - @UiHandler("deleteRevision") - void onDeleteRevision(@SuppressWarnings("unused") ClickEvent e) { - if (Window.confirm(Resources.C.deleteDraftRevision())) { - ChangeActions.delete(getProject(), changeId, revision, publish, deleteRevision); - } - } - @Override public void registerKeys() { super.registerKeys(); @@ -1323,10 +1296,7 @@ public class ChangeScreen extends Screen { } private boolean isSubmittable(ChangeInfo info) { - boolean canSubmit = - info.status().isOpen() - && revision.equals(info.currentRevision()) - && !info.revision(revision).draft(); + boolean canSubmit = info.status().isOpen() && revision.equals(info.currentRevision()); if (canSubmit && info.status() == Change.Status.NEW) { for (String name : info.labels()) { LabelInfo label = info.label(name); @@ -1404,7 +1374,7 @@ public class ChangeScreen extends Screen { // Properly render revision actions initially while waiting for // the callback to populate them correctly. NativeMap emptyMap = NativeMap.create(); - initRevisionsAction(info, revision, emptyMap); + initRevisionsAction(info, revision); quickApprove.setVisible(false); actions.reloadRevisionActions(emptyMap); @@ -1416,8 +1386,7 @@ public class ChangeScreen extends Screen { statusText.setInnerText(Util.C.notCurrent()); labels.setVisible(false); } else { - Status s = info.revision(revision).draft() ? Status.DRAFT : info.status(); - statusText.setInnerText(Util.toLongString(s)); + statusText.setInnerText(Util.toLongString(info.status())); } if (info.isPrivate()) { @@ -1444,7 +1413,7 @@ public class ChangeScreen extends Screen { } private void renderRevisionInfo(ChangeInfo info, NativeMap actionMap) { - initRevisionsAction(info, revision, actionMap); + initRevisionsAction(info, revision); commit.setParentNotCurrent( actionMap.containsKey("rebase") && actionMap.get("rebase").enabled()); actions.reloadRevisionActions(actionMap); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.ui.xml index 09bdc2422c..536af4f91a 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.ui.xml +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.ui.xml @@ -415,13 +415,6 @@ limitations under the License.
Delete Edit
 - -
Publish
- - -
Delete Revision
- diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/PatchSetsBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/PatchSetsBox.java index e75fe7a795..35cab4e2fb 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/PatchSetsBox.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/PatchSetsBox.java @@ -192,9 +192,6 @@ class PatchSetsBox extends Composite { } sb.openTd().setStyleName(style.legacy_id()); - if (r.draft()) { - sb.append(Resources.C.draft()).append(' '); - } sb.append(r.id()); sb.closeTd(); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/QuickApprove.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/QuickApprove.java index 0dcba2e3c8..56cc7a7852 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/QuickApprove.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/QuickApprove.java @@ -49,7 +49,7 @@ class QuickApprove extends Button implements ClickHandler { setVisible(false); return; } - if (info.revision(commit).isEdit() || info.revision(commit).draft()) { + if (info.revision(commit).isEdit()) { setVisible(false); return; } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java index 105815a97d..02be8c7639 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java @@ -245,25 +245,12 @@ public class ChangeApi { call(project, id, commit, "submit").post(in, cb); } - /** Publish a specific revision of a draft change. */ - public static void publish( - @Nullable String project, int id, String commit, AsyncCallback cb) { - JavaScriptObject in = JavaScriptObject.createObject(); - call(project, id, commit, "publish").post(in, cb); - } - /** Delete a specific draft change. */ public static void deleteChange( @Nullable String project, int id, AsyncCallback cb) { change(project, id).delete(cb); } - /** Delete a specific draft patch set. */ - public static void deleteRevision( - @Nullable String project, int id, String commit, AsyncCallback cb) { - revision(project, id, commit).delete(cb); - } - /** Delete change edit. */ public static void deleteEdit( @Nullable String project, int id, AsyncCallback cb) { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/Util.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/Util.java index b62b547ce1..8d949d136f 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/Util.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/Util.java @@ -30,8 +30,6 @@ public class Util { return ""; } switch (status) { - case DRAFT: - return C.statusLongDraft(); case NEW: return C.statusLongNew(); case MERGED: diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffScreen.java index 1e7d92b6b0..b4221cab84 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffScreen.java @@ -398,7 +398,6 @@ abstract class DiffScreen extends Screen { if (Gerrit.isSignedIn()) { keyMap .on("G I", () -> Gerrit.display(PageLinks.MINE)) - .on("G D", () -> Gerrit.display(PageLinks.toChangeQuery("owner:self is:draft"))) .on("G C", () -> Gerrit.display(PageLinks.toChangeQuery("has:draft"))) .on("G W", () -> Gerrit.display(PageLinks.toChangeQuery("is:watched status:open"))) .on("G S", () -> Gerrit.display(PageLinks.toChangeQuery("is:starred"))); diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java index 4252c7244e..201315ed72 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java @@ -302,8 +302,6 @@ public final class Change { private static final char MIN_OPEN = 'a'; /** Database constant for {@link Status#NEW}. */ public static final char STATUS_NEW = 'n'; - /** Database constant for {@link Status#DRAFT}. */ - public static final char STATUS_DRAFT = 'd'; /** Maximum database status constant for an open change. */ private static final char MAX_OPEN = 'z'; @@ -340,27 +338,10 @@ public final class Change { */ NEW(STATUS_NEW, ChangeStatus.NEW), - /** - * Change is a draft change that only consists of draft patchsets. - * - *

This is a change that is not meant to be submitted or reviewed yet. If the uploader - * publishes the change, it becomes a NEW change. Publishing is a one-way action, a change - * cannot return to DRAFT status. Draft changes are only visible to the uploader and those - * explicitly added as reviewers. - * - *

Changes in the DRAFT state can be moved to: - * - *

    - *
  • {@link #NEW} - when the change is published, it becomes a new change; - *
- */ - DRAFT(STATUS_DRAFT, ChangeStatus.DRAFT), - /** * Change is closed, and submitted to its destination branch. * *

Once a change has been merged, it cannot be further modified by adding a replacement patch - * set. Draft comments however may be published, supporting a post-submit review. */ MERGED(STATUS_MERGED, ChangeStatus.MERGED), @@ -422,6 +403,14 @@ public final class Change { return s; } } + + // TODO(davido): Remove in 3.0, after all sites upgraded to version, + // where DRAFT status was removed. This code path is still needed, + // when changes are deserialized from the secondary index, during + // the online migration to the new schema version wasn't completed. + if (c == 'd') { + return Status.NEW; + } return null; } diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSet.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSet.java index 0cc76ed68a..4536b67cc7 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSet.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSet.java @@ -168,8 +168,7 @@ public final class PatchSet { @Column(id = 4) protected Timestamp createdOn; - @Column(id = 5) - protected boolean draft; + // @Column(id = 5) /** * Opaque group identifier, usually assigned during creation. @@ -209,7 +208,6 @@ public final class PatchSet { this.revision = src.revision; this.uploader = src.uploader; this.createdOn = src.createdOn; - this.draft = src.draft; this.groups = src.groups; this.pushCertificate = src.pushCertificate; this.description = src.description; @@ -247,14 +245,6 @@ public final class PatchSet { createdOn = ts; } - public boolean isDraft() { - return draft; - } - - public void setDraft(boolean draftStatus) { - draft = draftStatus; - } - public List getGroups() { if (groups == null) { return Collections.emptyList(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/PatchSetUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/PatchSetUtil.java index c23d990f40..82ca8d2e95 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/PatchSetUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/PatchSetUtil.java @@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.gerrit.server.ChangeUtil.PS_ID_ORDER; -import static com.google.gerrit.server.notedb.PatchSetState.DRAFT; import static com.google.gerrit.server.notedb.PatchSetState.PUBLISHED; import static java.util.function.Function.identity; @@ -34,7 +33,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.notedb.NotesMigration; -import com.google.gerrit.server.notedb.PatchSetState; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -104,7 +102,6 @@ public class PatchSetUtil { ChangeUpdate update, PatchSet.Id psId, ObjectId commit, - boolean draft, List groups, String pushCertificate, String description) @@ -116,7 +113,6 @@ public class PatchSetUtil { ps.setRevision(new RevId(commit.name())); ps.setUploader(update.getAccountId()); ps.setCreatedOn(new Timestamp(update.getWhen().getTime())); - ps.setDraft(draft); ps.setGroups(groups); ps.setPushCertificate(pushCertificate); ps.setDescription(description); @@ -125,27 +121,16 @@ public class PatchSetUtil { update.setCommit(rw, commit, pushCertificate); update.setPsDescription(description); update.setGroups(groups); - if (draft) { - update.setPatchSetState(DRAFT); - } return ps; } public void publish(ReviewDb db, ChangeUpdate update, PatchSet ps) throws OrmException { ensurePatchSetMatches(ps.getId(), update); - ps.setDraft(false); update.setPatchSetState(PUBLISHED); db.patchSets().update(Collections.singleton(ps)); } - public void delete(ReviewDb db, ChangeUpdate update, PatchSet ps) throws OrmException { - ensurePatchSetMatches(ps.getId(), update); - checkArgument(ps.isDraft(), "cannot delete non-draft patch set %s", ps.getId()); - update.setPatchSetState(PatchSetState.DELETED); - db.patchSets().delete(Collections.singleton(ps)); - } - private void ensurePatchSetMatches(PatchSet.Id psId, ChangeUpdate update) { Change.Id changeId = update.getChange().getId(); checkArgument( diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GeneralPreferencesLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GeneralPreferencesLoader.java index df64c0b05e..804377336f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GeneralPreferencesLoader.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GeneralPreferencesLoader.java @@ -141,7 +141,6 @@ public class GeneralPreferencesLoader { } if (r.my.isEmpty()) { r.my.add(new MenuItem("Changes", "#/dashboard/self", null)); - r.my.add(new MenuItem("Drafts", "#/q/owner:self+is:draft", null)); r.my.add(new MenuItem("Draft Comments", "#/q/has:draft", null)); r.my.add(new MenuItem("Edits", "#/q/has:edit", null)); r.my.add(new MenuItem("Watched Changes", "#/q/is:watched+is:open", null)); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java index ab47f3b84c..d43327f538 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java @@ -74,7 +74,6 @@ import com.google.gerrit.server.change.Mute; import com.google.gerrit.server.change.PostHashtags; import com.google.gerrit.server.change.PostPrivate; 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.PutMessage; import com.google.gerrit.server.change.PutTopic; @@ -117,7 +116,6 @@ class ChangeApiImpl implements ChangeApi { private final Restore restore; private final CreateMergePatchSet updateByMerge; private final Provider submittedTogether; - private final PublishDraftPatchSet.CurrentRevision publishDraftChange; private final Rebase.CurrentRevision rebase; private final DeleteChange deleteChange; private final GetTopic getTopic; @@ -163,7 +161,6 @@ class ChangeApiImpl implements ChangeApi { Restore restore, CreateMergePatchSet updateByMerge, Provider submittedTogether, - PublishDraftPatchSet.CurrentRevision publishDraftChange, Rebase.CurrentRevision rebase, DeleteChange deleteChange, GetTopic getTopic, @@ -207,7 +204,6 @@ class ChangeApiImpl implements ChangeApi { this.restore = restore; this.updateByMerge = updateByMerge; this.submittedTogether = submittedTogether; - this.publishDraftChange = publishDraftChange; this.rebase = rebase; this.deleteChange = deleteChange; this.getTopic = getTopic; @@ -403,13 +399,10 @@ class ChangeApiImpl implements ChangeApi { } } + @Deprecated @Override public void publish() throws RestApiException { - try { - publishDraftChange.apply(change, null); - } catch (Exception e) { - throw asRestApiException("Cannot publish change", e); - } + throw new UnsupportedOperationException("draft workflow is discontinued"); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java index 4f51420855..65bbc47d22 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java @@ -49,7 +49,6 @@ import com.google.gerrit.server.change.ApplyFix; import com.google.gerrit.server.change.CherryPick; import com.google.gerrit.server.change.Comments; import com.google.gerrit.server.change.CreateDraftComment; -import com.google.gerrit.server.change.DeleteDraftPatchSet; import com.google.gerrit.server.change.DraftComments; import com.google.gerrit.server.change.FileResource; import com.google.gerrit.server.change.Files; @@ -65,7 +64,6 @@ import com.google.gerrit.server.change.ListRobotComments; import com.google.gerrit.server.change.Mergeable; import com.google.gerrit.server.change.PostReview; import com.google.gerrit.server.change.PreviewSubmit; -import com.google.gerrit.server.change.PublishDraftPatchSet; import com.google.gerrit.server.change.PutDescription; import com.google.gerrit.server.change.Rebase; import com.google.gerrit.server.change.RebaseUtil; @@ -95,12 +93,10 @@ class RevisionApiImpl implements RevisionApi { private final RevisionReviewers revisionReviewers; private final RevisionReviewerApiImpl.Factory revisionReviewerApi; private final CherryPick cherryPick; - private final DeleteDraftPatchSet deleteDraft; private final Rebase rebase; private final RebaseUtil rebaseUtil; private final Submit submit; private final PreviewSubmit submitPreview; - private final PublishDraftPatchSet publish; private final Reviewed.PutReviewed putReviewed; private final Reviewed.DeleteReviewed deleteReviewed; private final RevisionResource revision; @@ -137,12 +133,10 @@ class RevisionApiImpl implements RevisionApi { RevisionReviewers revisionReviewers, RevisionReviewerApiImpl.Factory revisionReviewerApi, CherryPick cherryPick, - DeleteDraftPatchSet deleteDraft, Rebase rebase, RebaseUtil rebaseUtil, Submit submit, PreviewSubmit submitPreview, - PublishDraftPatchSet publish, Reviewed.PutReviewed putReviewed, Reviewed.DeleteReviewed deleteReviewed, Files files, @@ -176,13 +170,11 @@ class RevisionApiImpl implements RevisionApi { this.revisionReviewers = revisionReviewers; this.revisionReviewerApi = revisionReviewerApi; this.cherryPick = cherryPick; - this.deleteDraft = deleteDraft; this.rebase = rebase; this.rebaseUtil = rebaseUtil; this.review = review; this.submit = submit; this.submitPreview = submitPreview; - this.publish = publish; this.files = files; this.putReviewed = putReviewed; this.deleteReviewed = deleteReviewed; @@ -253,20 +245,12 @@ class RevisionApiImpl implements RevisionApi { @Override public void publish() throws RestApiException { - try { - publish.apply(revision, new PublishDraftPatchSet.Input()); - } catch (Exception e) { - throw asRestApiException("Cannot publish draft patch set", e); - } + throw new UnsupportedOperationException("draft workflow is discontinued"); } @Override public void delete() throws RestApiException { - try { - deleteDraft.apply(revision, null); - } catch (Exception e) { - throw asRestApiException("Cannot delete draft ps", e); - } + throw new UnsupportedOperationException("draft workflow is discontinued"); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java index db13c14ad7..c9d016d953 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java @@ -204,7 +204,7 @@ public class Abandon extends RetryingRestModifyView messages( - ChangeControl ctl, ChangeData cd, Map map) throws OrmException { + private Collection messages(ChangeControl ctl, ChangeData cd) + throws OrmException { List messages = cmUtil.byChange(db.get(), cd.notes()); if (messages.isEmpty()) { return Collections.emptyList(); @@ -1122,8 +1122,7 @@ public class ChangeJson { List result = Lists.newArrayListWithCapacity(messages.size()); for (ChangeMessage message : messages) { PatchSet.Id patchNum = message.getPatchSetId(); - PatchSet ps = patchNum != null ? map.get(patchNum) : null; - if (patchNum == null || ctl.isPatchVisible(ps, db.get())) { + if (patchNum == null || ctl.isVisible(db.get())) { ChangeMessageInfo cmi = new ChangeMessageInfo(); cmi.id = message.getKey().get(); cmi.author = accountLoader.get(message.getAuthor()); @@ -1257,7 +1256,7 @@ public class ChangeJson { } else { want = id.equals(cd.change().currentPatchSetId()); } - if (want && ctl.isPatchVisible(in, db.get())) { + if (want && ctl.isVisible(db.get())) { res.put(in.getRevision().get(), toRevisionInfo(cd, in, repo, rw, false, changeInfo)); } } @@ -1318,7 +1317,6 @@ public class ChangeJson { out.ref = in.getRefName(); out.created = in.getCreatedOn(); out.uploader = accountLoader.get(in.getUploader()); - out.draft = in.isDraft() ? true : null; out.fetch = makeFetchMap(cd, in); out.kind = changeKindCache.getChangeKind(rw, repo != null ? repo.getConfig() : null, cd, in); out.description = in.getDescription(); @@ -1356,9 +1354,7 @@ public class ChangeJson { out.files.remove(Patch.MERGE_LIST); } - if ((out.isCurrent || (out.draft != null && out.draft)) - && has(CURRENT_ACTIONS) - && userProvider.get().isIdentifiedUser()) { + if (out.isCurrent && has(CURRENT_ACTIONS) && userProvider.get().isIdentifiedUser()) { actionJson.addRevisionActions( changeInfo, @@ -1423,7 +1419,7 @@ public class ChangeJson { continue; } - if (!scheme.isAuthSupported() && !ctl.isPatchVisible(in, db.get())) { + if (!scheme.isAuthSupported() && !ctl.isVisible(db.get())) { continue; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java index 12c0483dcd..4f03f37931 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java @@ -37,7 +37,6 @@ import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.ReviewerSet; import com.google.gerrit.server.Sequences; import com.google.gerrit.server.git.CodeReviewCommit; @@ -96,7 +95,6 @@ public class CherryPickChange { private final ProjectControl.GenericFactory projectControlFactory; private final ApprovalsUtil approvalsUtil; private final ChangeMessagesUtil changeMessagesUtil; - private final PatchSetUtil psUtil; private final NotifyUtil notifyUtil; @Inject @@ -114,7 +112,6 @@ public class CherryPickChange { ProjectControl.GenericFactory projectControlFactory, ApprovalsUtil approvalsUtil, ChangeMessagesUtil changeMessagesUtil, - PatchSetUtil psUtil, NotifyUtil notifyUtil) { this.dbProvider = dbProvider; this.seq = seq; @@ -129,7 +126,6 @@ public class CherryPickChange { this.projectControlFactory = projectControlFactory; this.approvalsUtil = approvalsUtil; this.changeMessagesUtil = changeMessagesUtil; - this.psUtil = psUtil; this.notifyUtil = notifyUtil; } @@ -325,12 +321,9 @@ public class CherryPickChange { throws IOException, OrmException, BadRequestException, ConfigInvalidException { Change destChange = destNotes.getChange(); PatchSet.Id psId = ChangeUtil.nextPatchSetId(git, destChange.currentPatchSetId()); - PatchSet current = psUtil.current(dbProvider.get(), destNotes); - PatchSetInserter inserter = patchSetInserterFactory.create(destNotes, psId, cherryPickCommit); inserter .setMessage("Uploaded patch set " + inserter.getPatchSetId().get() + ".") - .setDraft(current.isDraft()) .setNotify(input.notify) .setAccountsToNotify(notifyUtil.resolveAccounts(input.notifyDetails)); bu.addOp(destChange.getId(), inserter); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateMergePatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateMergePatchSet.java index 072052ba53..0425e53a18 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateMergePatchSet.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateMergePatchSet.java @@ -161,9 +161,9 @@ public class CreateMergePatchSet rsrc.getId(), psInserter .setMessage("Uploaded patch set " + nextPsId.get() + ".") - .setDraft(ps.isDraft()) .setNotify(NotifyHandling.NONE) - .setCheckAddPatchSetPermission(false)); + .setCheckAddPatchSetPermission(false) + .setNotify(NotifyHandling.NONE)); bu.execute(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChange.java index bc53a7a30d..af26e8ac6f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChange.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChange.java @@ -17,18 +17,14 @@ package com.google.gerrit.server.change; import static com.google.gerrit.extensions.conditions.BooleanCondition.and; import com.google.gerrit.common.TimeUtil; -import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.MethodNotAllowedException; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.webui.UiAction; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.change.DeleteChange.Input; -import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.permissions.ChangePermission; -import com.google.gerrit.server.permissions.GlobalPermission; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.update.BatchUpdate; @@ -39,7 +35,6 @@ import com.google.gerrit.server.update.UpdateException; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; -import org.eclipse.jgit.lib.Config; @Singleton public class DeleteChange extends RetryingRestModifyView> @@ -48,24 +43,13 @@ public class DeleteChange extends RetryingRestModifyView db; private final Provider opProvider; - private final Provider user; - private final PermissionBackend permissionBackend; - private final boolean allowDrafts; @Inject public DeleteChange( - Provider db, - RetryHelper retryHelper, - Provider opProvider, - Provider user, - PermissionBackend permissionBackend, - @GerritServerConfig Config cfg) { + Provider db, RetryHelper retryHelper, Provider opProvider) { super(retryHelper); this.db = db; this.opProvider = opProvider; - this.user = user; - this.permissionBackend = permissionBackend; - this.allowDrafts = DeleteChangeOp.allowDrafts(cfg); } @Override @@ -74,16 +58,8 @@ public class DeleteChange extends RetryingRestModifyView> - implements UiAction { - public static class Input {} - - private final Provider db; - private final PatchSetInfoFactory patchSetInfoFactory; - private final PatchSetUtil psUtil; - private final Provider deleteChangeOpProvider; - private final DynamicItem accountPatchReviewStore; - private final boolean allowDrafts; - private final ChangeControl.GenericFactory changeControlFactory; - - @Inject - public DeleteDraftPatchSet( - Provider db, - RetryHelper retryHelper, - PatchSetInfoFactory patchSetInfoFactory, - PatchSetUtil psUtil, - Provider deleteChangeOpProvider, - DynamicItem accountPatchReviewStore, - @GerritServerConfig Config cfg, - ChangeControl.GenericFactory changeControlFactory) { - super(retryHelper); - this.db = db; - this.patchSetInfoFactory = patchSetInfoFactory; - this.psUtil = psUtil; - this.deleteChangeOpProvider = deleteChangeOpProvider; - this.accountPatchReviewStore = accountPatchReviewStore; - this.allowDrafts = cfg.getBoolean("change", "allowDrafts", true); - this.changeControlFactory = changeControlFactory; - } - - @Override - protected Response applyImpl( - BatchUpdate.Factory updateFactory, RevisionResource rsrc, Input input) - throws RestApiException, UpdateException, OrmException, PermissionBackendException { - if (isDeletingOnlyPatchSet(rsrc)) { - // A change cannot have zero patch sets; the change is deleted instead. - rsrc.permissions().database(db).check(ChangePermission.DELETE); - } - - try (BatchUpdate bu = - updateFactory.create(db.get(), rsrc.getProject(), rsrc.getUser(), TimeUtil.nowTs())) { - bu.setOrder(Order.DB_BEFORE_REPO); - bu.addOp(rsrc.getChange().getId(), new Op(rsrc.getPatchSet().getId())); - bu.execute(); - } - return Response.none(); - } - - private boolean isDeletingOnlyPatchSet(RevisionResource rsrc) throws OrmException { - Collection patchSets = psUtil.byChange(db.get(), rsrc.getNotes()); - return patchSets.size() == 1 - && Iterables.getOnlyElement(patchSets).getId().equals(rsrc.getPatchSet().getId()); - } - - private class Op implements BatchUpdateOp { - private final PatchSet.Id psId; - - private Collection patchSetsBeforeDeletion; - private PatchSet patchSet; - private DeleteChangeOp deleteChangeOp; - - private Op(PatchSet.Id psId) { - this.psId = psId; - } - - @Override - public boolean updateChange(ChangeContext ctx) - throws RestApiException, OrmException, IOException, NoSuchChangeException { - Map patchSets = psUtil.byChangeAsMap(db.get(), ctx.getNotes()); - patchSet = patchSets.get(psId); - if (patchSet == null) { - return false; // Nothing to do. - } - if (!patchSet.isDraft()) { - throw new ResourceConflictException("Patch set is not a draft"); - } - if (!allowDrafts) { - throw new MethodNotAllowedException("Draft workflow is disabled"); - } - if (!changeControlFactory - .controlFor(ctx.getNotes(), ctx.getUser()) - .canDeleteDraft(ctx.getDb())) { - throw new AuthException("Not permitted to delete this draft patch set"); - } - - patchSetsBeforeDeletion = patchSets.values(); - deleteDraftPatchSet(patchSet, ctx); - deleteOrUpdateDraftChange(ctx, patchSets); - return true; - } - - @Override - public void updateRepo(RepoContext ctx) throws IOException { - if (deleteChangeOp != null) { - deleteChangeOp.updateRepo(ctx); - return; - } - ctx.addRefUpdate( - ObjectId.fromString(patchSet.getRevision().get()), - ObjectId.zeroId(), - patchSet.getRefName()); - } - - private void deleteDraftPatchSet(PatchSet patchSet, ChangeContext ctx) throws OrmException { - // For NoteDb itself, no need to delete these entities, as they are - // automatically filtered out when patch sets are deleted. - psUtil.delete(ctx.getDb(), ctx.getUpdate(patchSet.getId()), patchSet); - - accountPatchReviewStore.get().clearReviewed(psId); - // Use the unwrap from DeleteChangeOp to handle BatchUpdateReviewDb. - ReviewDb db = DeleteChangeOp.unwrap(ctx.getDb()); - db.changeMessages().delete(db.changeMessages().byPatchSet(psId)); - db.patchComments().delete(db.patchComments().byPatchSet(psId)); - db.patchSetApprovals().delete(db.patchSetApprovals().byPatchSet(psId)); - } - - private void deleteOrUpdateDraftChange(ChangeContext ctx, Map patchSets) - throws OrmException, RestApiException, IOException, NoSuchChangeException { - Change c = ctx.getChange(); - if (deletedOnlyPatchSet()) { - deleteChangeOp = deleteChangeOpProvider.get(); - deleteChangeOp.updateChange(ctx); - return; - } - if (c.currentPatchSetId().equals(psId)) { - c.setCurrentPatchSet(previousPatchSetInfo(ctx, patchSets)); - } - } - - private boolean deletedOnlyPatchSet() { - return patchSetsBeforeDeletion.size() == 1 - && patchSetsBeforeDeletion.iterator().next().getId().equals(psId); - } - - private PatchSetInfo previousPatchSetInfo( - ChangeContext ctx, Map patchSets) throws OrmException { - PatchSet.Id prevPsId = null; - for (PatchSet.Id id : patchSets.keySet()) { - if (id.get() < psId.get() && (prevPsId == null || id.get() > prevPsId.get())) { - prevPsId = id; - } - } - - try { - // TODO(dborowitz): Get this in a way that doesn't involve re-opening - // the repo after the updateRepo phase. - return patchSetInfoFactory.get( - ctx.getDb(), - ctx.getNotes(), - new PatchSet.Id(psId.getParentKey(), checkNotNull(prevPsId).get())); - } catch (PatchSetInfoNotAvailableException e) { - throw new OrmException(e); - } - } - } - - @Override - public UiAction.Description getDescription(RevisionResource rsrc) { - try { - return new UiAction.Description() - .setLabel("Delete") - .setTitle(String.format("Delete draft revision %d", rsrc.getPatchSet().getPatchSetId())) - .setVisible( - allowDrafts - && rsrc.getPatchSet().isDraft() - && psUtil.byChange(db.get(), rsrc.getNotes()).size() > 1 - && changeControlFactory - .controlFor(rsrc.getNotes(), rsrc.getUser()) - .canDeleteDraft(db.get())); - } catch (OrmException e) { - throw new IllegalStateException(e); - } - } -} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPureRevert.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPureRevert.java index 2d078eb2d1..6002f75886 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPureRevert.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPureRevert.java @@ -90,7 +90,7 @@ public class GetPureRevert implements RestReadView { throw new ResourceConflictException("current revision is missing"); } else if (!changeControlFactory .controlFor(rsrc.getNotes(), rsrc.getUser()) - .isPatchVisible(currentPatchSet, dbProvider.get())) { + .isVisible(dbProvider.get())) { throw new AuthException("current revision not accessible"); } return getPureRevert(rsrc.getNotes()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java index 5089574004..f3a6c66a20 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java @@ -77,7 +77,6 @@ public class Module extends RestApiModule { delete(CHANGE_KIND).to(DeleteChange.class); post(CHANGE_KIND, "abandon").to(Abandon.class); post(CHANGE_KIND, "hashtags").to(PostHashtags.class); - post(CHANGE_KIND, "publish").to(PublishDraftPatchSet.CurrentRevision.class); post(CHANGE_KIND, "restore").to(Restore.class); post(CHANGE_KIND, "revert").to(Revert.class); post(CHANGE_KIND, "submit").to(Submit.CurrentRevision.class); @@ -111,9 +110,7 @@ public class Module extends RestApiModule { get(REVISION_KIND, "actions").to(GetRevisionActions.class); post(REVISION_KIND, "cherrypick").to(CherryPick.class); get(REVISION_KIND, "commit").to(GetCommit.class); - delete(REVISION_KIND).to(DeleteDraftPatchSet.class); get(REVISION_KIND, "mergeable").to(Mergeable.class); - post(REVISION_KIND, "publish").to(PublishDraftPatchSet.class); get(REVISION_KIND, "related").to(GetRelated.class); get(REVISION_KIND, "review").to(GetReview.class); post(REVISION_KIND, "review").to(PostReview.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java index a4aa60aa66..2f3855c4fd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java @@ -140,7 +140,7 @@ public class Move extends RetryingRestModifyView groups = Collections.emptyList(); private boolean fireRevisionCreated = true; private NotifyHandling notify = NotifyHandling.ALL; @@ -168,11 +167,6 @@ public class PatchSetInserter implements BatchUpdateOp { return this; } - public PatchSetInserter setDraft(boolean draft) { - this.draft = draft; - return this; - } - public PatchSetInserter setGroups(List groups) { checkNotNull(groups, "groups may not be null"); this.groups = groups; @@ -253,7 +247,6 @@ public class PatchSetInserter implements BatchUpdateOp { ctx.getUpdate(psId), psId, commitId, - draft, newGroups, null, description); @@ -275,7 +268,7 @@ public class PatchSetInserter implements BatchUpdateOp { patchSetInfo = patchSetInfoFactory.get(ctx.getRevWalk(), ctx.getRevWalk().parseCommit(commitId), psId); - if (change.getStatus() != Change.Status.DRAFT && !allowClosed) { + if (!allowClosed) { change.setStatus(Change.Status.NEW); } change.setCurrentPatchSet(patchSetInfo); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java deleted file mode 100644 index fff64bad60..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java +++ /dev/null @@ -1,295 +0,0 @@ -// Copyright (C) 2013 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 static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters; - -import com.google.gerrit.common.Nullable; -import com.google.gerrit.common.TimeUtil; -import com.google.gerrit.common.data.LabelTypes; -import com.google.gerrit.common.errors.EmailException; -import com.google.gerrit.extensions.restapi.AuthException; -import com.google.gerrit.extensions.restapi.ResourceConflictException; -import com.google.gerrit.extensions.restapi.ResourceNotFoundException; -import com.google.gerrit.extensions.restapi.Response; -import com.google.gerrit.extensions.restapi.RestApiException; -import com.google.gerrit.extensions.webui.UiAction; -import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.ChangeMessage; -import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.reviewdb.client.PatchSetInfo; -import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.ApprovalsUtil; -import com.google.gerrit.server.ChangeMessagesUtil; -import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.PatchSetUtil; -import com.google.gerrit.server.account.AccountResolver; -import com.google.gerrit.server.change.PublishDraftPatchSet.Input; -import com.google.gerrit.server.extensions.events.DraftPublished; -import com.google.gerrit.server.mail.MailUtil.MailRecipients; -import com.google.gerrit.server.mail.send.CreateChangeSender; -import com.google.gerrit.server.mail.send.ReplacePatchSetSender; -import com.google.gerrit.server.notedb.ChangeUpdate; -import com.google.gerrit.server.patch.PatchSetInfoFactory; -import com.google.gerrit.server.project.ChangeControl; -import com.google.gerrit.server.project.ProjectCache; -import com.google.gerrit.server.project.ProjectState; -import com.google.gerrit.server.update.BatchUpdate; -import com.google.gerrit.server.update.BatchUpdateOp; -import com.google.gerrit.server.update.ChangeContext; -import com.google.gerrit.server.update.Context; -import com.google.gerrit.server.update.RetryHelper; -import com.google.gerrit.server.update.RetryingRestModifyView; -import com.google.gerrit.server.update.UpdateException; -import com.google.gwtorm.server.OrmException; -import com.google.inject.Inject; -import com.google.inject.Provider; -import com.google.inject.Singleton; -import java.io.IOException; -import java.util.Collection; -import java.util.List; -import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.revwalk.FooterLine; -import org.eclipse.jgit.revwalk.RevCommit; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -@Singleton -public class PublishDraftPatchSet - extends RetryingRestModifyView> - implements UiAction { - private static final Logger log = LoggerFactory.getLogger(PublishDraftPatchSet.class); - - public static class Input {} - - private final AccountResolver accountResolver; - private final ApprovalsUtil approvalsUtil; - private final CreateChangeSender.Factory createChangeSenderFactory; - private final PatchSetInfoFactory patchSetInfoFactory; - private final PatchSetUtil psUtil; - private final Provider dbProvider; - private final ReplacePatchSetSender.Factory replacePatchSetFactory; - private final DraftPublished draftPublished; - private final ProjectCache projectCache; - private final ChangeControl.GenericFactory changeControlFactory; - - @Inject - public PublishDraftPatchSet( - AccountResolver accountResolver, - ApprovalsUtil approvalsUtil, - RetryHelper retryHelper, - CreateChangeSender.Factory createChangeSenderFactory, - PatchSetInfoFactory patchSetInfoFactory, - PatchSetUtil psUtil, - Provider dbProvider, - ReplacePatchSetSender.Factory replacePatchSetFactory, - DraftPublished draftPublished, - ProjectCache projectCache, - ChangeControl.GenericFactory changeControlFactory) { - super(retryHelper); - this.accountResolver = accountResolver; - this.approvalsUtil = approvalsUtil; - this.createChangeSenderFactory = createChangeSenderFactory; - this.patchSetInfoFactory = patchSetInfoFactory; - this.psUtil = psUtil; - this.dbProvider = dbProvider; - this.replacePatchSetFactory = replacePatchSetFactory; - this.draftPublished = draftPublished; - this.projectCache = projectCache; - this.changeControlFactory = changeControlFactory; - } - - @Override - protected Response applyImpl( - BatchUpdate.Factory updateFactory, RevisionResource rsrc, Input input) - throws RestApiException, UpdateException, IOException { - return apply( - updateFactory, - rsrc.getUser(), - rsrc.getChange(), - rsrc.getPatchSet().getId(), - rsrc.getPatchSet()); - } - - private Response apply( - BatchUpdate.Factory updateFactory, CurrentUser u, Change c, PatchSet.Id psId, PatchSet ps) - throws RestApiException, UpdateException, IOException { - try (BatchUpdate bu = - updateFactory.create(dbProvider.get(), c.getProject(), u, TimeUtil.nowTs())) { - bu.addOp(c.getId(), new Op(psId, projectCache.checkedGet(c.getProject()), ps)); - bu.execute(); - } - return Response.none(); - } - - @Override - public UiAction.Description getDescription(RevisionResource rsrc) { - try { - return new UiAction.Description() - .setLabel("Publish") - .setTitle(String.format("Publish revision %d", rsrc.getPatchSet().getPatchSetId())) - .setVisible( - rsrc.getPatchSet().isDraft() - && changeControlFactory - .controlFor(rsrc.getNotes(), rsrc.getUser()) - .canPublish(dbProvider.get())); - } catch (OrmException e) { - throw new IllegalStateException(e); - } - } - - public static class CurrentRevision - extends RetryingRestModifyView> { - private final PublishDraftPatchSet publish; - - @Inject - CurrentRevision(RetryHelper retryHelper, PublishDraftPatchSet publish) { - super(retryHelper); - this.publish = publish; - } - - @Override - protected Response applyImpl( - BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input input) - throws RestApiException, UpdateException, IOException { - return publish.apply( - updateFactory, - rsrc.getUser(), - rsrc.getChange(), - rsrc.getChange().currentPatchSetId(), - null); - } - } - - private class Op implements BatchUpdateOp { - private final PatchSet.Id psId; - private final ProjectState projectState; - - private PatchSet patchSet; - private Change change; - private boolean wasDraftChange; - private PatchSetInfo patchSetInfo; - private MailRecipients recipients; - - private Op(PatchSet.Id psId, ProjectState projectState, @Nullable PatchSet patchSet) { - this.psId = psId; - this.projectState = projectState; - this.patchSet = patchSet; - } - - @Override - public boolean updateChange(ChangeContext ctx) - throws RestApiException, OrmException, IOException { - if (!changeControlFactory.controlFor(ctx.getNotes(), ctx.getUser()).canPublish(ctx.getDb())) { - throw new AuthException("Cannot publish this draft patch set"); - } - if (patchSet == null) { - patchSet = psUtil.get(ctx.getDb(), ctx.getNotes(), psId); - if (patchSet == null) { - throw new ResourceNotFoundException(psId.toString()); - } - } - saveChange(ctx); - savePatchSet(ctx); - addReviewers(ctx); - return true; - } - - private void saveChange(ChangeContext ctx) { - change = ctx.getChange(); - ChangeUpdate update = ctx.getUpdate(psId); - wasDraftChange = change.getStatus() == Change.Status.DRAFT; - if (wasDraftChange) { - change.setStatus(Change.Status.NEW); - update.setStatus(change.getStatus()); - } - } - - private void savePatchSet(ChangeContext ctx) throws RestApiException, OrmException { - if (!patchSet.isDraft()) { - throw new ResourceConflictException("Patch set is not a draft"); - } - psUtil.publish(ctx.getDb(), ctx.getUpdate(psId), patchSet); - } - - private void addReviewers(ChangeContext ctx) throws OrmException, IOException { - LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes(), ctx.getUser()); - Collection oldReviewers = - approvalsUtil.getReviewers(ctx.getDb(), ctx.getNotes()).all(); - RevCommit commit = - ctx.getRevWalk().parseCommit(ObjectId.fromString(patchSet.getRevision().get())); - patchSetInfo = patchSetInfoFactory.get(ctx.getRevWalk(), commit, psId); - - List footerLines = commit.getFooterLines(); - recipients = - getRecipientsFromFooters(ctx.getDb(), accountResolver, patchSet.isDraft(), footerLines); - recipients.remove(ctx.getAccountId()); - approvalsUtil.addReviewers( - ctx.getDb(), - ctx.getUpdate(psId), - labelTypes, - change, - patchSet, - patchSetInfo, - recipients.getReviewers(), - oldReviewers); - } - - @Override - public void postUpdate(Context ctx) throws OrmException { - draftPublished.fire(change, patchSet, ctx.getAccount(), ctx.getWhen()); - if (patchSet.isDraft() && change.getStatus() == Change.Status.DRAFT) { - // Skip emails if the patch set is still a draft. - return; - } - try { - if (wasDraftChange) { - sendCreateChange(ctx); - } else { - sendReplacePatchSet(ctx); - } - } catch (EmailException e) { - log.error("Cannot send email for publishing draft " + psId, e); - } - } - - private void sendCreateChange(Context ctx) throws EmailException { - CreateChangeSender cm = createChangeSenderFactory.create(ctx.getProject(), change.getId()); - cm.setFrom(ctx.getAccountId()); - cm.setPatchSet(patchSet, patchSetInfo); - cm.addReviewers(recipients.getReviewers()); - cm.addExtraCC(recipients.getCcOnly()); - cm.send(); - } - - private void sendReplacePatchSet(Context ctx) throws EmailException { - ChangeMessage msg = - ChangeMessagesUtil.newMessage( - psId, - ctx.getUser(), - ctx.getWhen(), - "Uploaded patch set " + psId.get() + ".", - ChangeMessagesUtil.TAG_UPLOADED_PATCH_SET); - ReplacePatchSetSender cm = replacePatchSetFactory.create(ctx.getProject(), change.getId()); - cm.setFrom(ctx.getAccountId()); - cm.setPatchSet(patchSet, patchSetInfo); - cm.setChangeMessage(msg.getMessage(), ctx.getWhen()); - cm.addReviewers(recipients.getReviewers()); - cm.addExtraCC(recipients.getCcOnly()); - cm.send(); - } - } -} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutMessage.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutMessage.java index f994fc96a6..5edef0a60f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutMessage.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutMessage.java @@ -111,7 +111,7 @@ public class PutMessage throw new ResourceConflictException("current revision is missing"); } else if (!changeControlFactory .controlFor(resource.getNotes(), resource.getUser()) - .isPatchVisible(ps, db.get())) { + .isVisible(db.get())) { throw new AuthException("current revision not accessible"); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java index 174dac9dab..7d929733dd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java @@ -158,7 +158,7 @@ public class Rebase extends RetryingRestModifyView match = Lists.newArrayListWithExpectedSize(2); for (RevisionResource rsrc : find(change, id.get())) { - if (visible(change, rsrc.getPatchSet())) { + if (visible(change)) { match.add(rsrc); } } @@ -100,10 +100,10 @@ public class Revisions implements ChildCollection find(ChangeResource change, String id) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java index f648e1b777..cab61b38ea 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java @@ -254,7 +254,6 @@ public class Submit } // $FALL-THROUGH$ case ABANDONED: - case DRAFT: default: throw new ResourceConflictException("change is " + ChangeUtil.status(change)); } @@ -316,7 +315,6 @@ public class Submit Change change = resource.getChange(); if (!change.getStatus().isOpen() || !resource.isCurrent() - || resource.getPatchSet().isDraft() || !resource.permissions().testOrFalse(ChangePermission.SUBMIT)) { return null; // submit not visible } @@ -534,7 +532,7 @@ public class Submit throw new ResourceConflictException("current revision is missing"); } else if (!changeControlFactory .controlFor(rsrc.getNotes(), rsrc.getUser()) - .isPatchVisible(ps, dbProvider.get())) { + .isVisible(dbProvider.get())) { throw new AuthException("current revision not accessible"); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index 8751787200..901084ed16 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -41,7 +41,6 @@ import com.google.gerrit.extensions.events.ChangeMergedListener; import com.google.gerrit.extensions.events.ChangeRestoredListener; import com.google.gerrit.extensions.events.ChangeRevertedListener; import com.google.gerrit.extensions.events.CommentAddedListener; -import com.google.gerrit.extensions.events.DraftPublishedListener; import com.google.gerrit.extensions.events.GarbageCollectorListener; import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; import com.google.gerrit.extensions.events.GroupIndexedListener; @@ -313,7 +312,6 @@ public class GerritGlobalModule extends FactoryModule { DynamicSet.setOf(binder(), AssigneeChangedListener.class); DynamicSet.setOf(binder(), ChangeAbandonedListener.class); DynamicSet.setOf(binder(), CommentAddedListener.class); - DynamicSet.setOf(binder(), DraftPublishedListener.class); DynamicSet.setOf(binder(), HashtagsEditedListener.class); DynamicSet.setOf(binder(), ChangeMergedListener.class); DynamicSet.setOf(binder(), ChangeRestoredListener.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/data/PatchSetAttribute.java b/gerrit-server/src/main/java/com/google/gerrit/server/data/PatchSetAttribute.java index d3b3786d5a..dc47057dc3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/data/PatchSetAttribute.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/data/PatchSetAttribute.java @@ -25,7 +25,6 @@ public class PatchSetAttribute { public AccountAttribute uploader; public Long createdOn; public AccountAttribute author; - public boolean isDraft; public ChangeKind kind; public List approvals; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java index 565ed9eef4..d1d72fa579 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java @@ -26,7 +26,6 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.Change.Status; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.server.ReviewDb; @@ -205,11 +204,7 @@ public class ChangeEditUtil { try (BatchUpdate bu = updateFactory.create(db.get(), change.getProject(), user, TimeUtil.nowTs())) { bu.setRepository(repo, rw, oi); - bu.addOp( - change.getId(), - inserter - .setDraft(change.getStatus() == Status.DRAFT || basePatchSet.isDraft()) - .setMessage(message.toString())); + bu.addOp(change.getId(), inserter.setMessage(message.toString())); bu.addOp( change.getId(), new BatchUpdateOp() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/DraftPublishedEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/DraftPublishedEvent.java deleted file mode 100644 index 0724253018..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/DraftPublishedEvent.java +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (C) 2012 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.events; - -import com.google.common.base.Supplier; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.server.data.AccountAttribute; - -public class DraftPublishedEvent extends PatchSetEvent { - static final String TYPE = "draft-published"; - public Supplier uploader; - - public DraftPublishedEvent(Change change) { - super(TYPE, change); - } -} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java index d31c26d76d..afd78dcb5f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java @@ -470,7 +470,6 @@ public class EventFactory { p.ref = patchSet.getRefName(); p.uploader = asAccountAttribute(patchSet.getUploader()); p.createdOn = patchSet.getCreatedOn().getTime() / 1000L; - p.isDraft = patchSet.isDraft(); PatchSet.Id pId = patchSet.getId(); try { p.parents = new ArrayList<>(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventTypes.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventTypes.java index 74453f3803..3d1d2fab81 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventTypes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventTypes.java @@ -28,7 +28,6 @@ public class EventTypes { register(ChangeRestoredEvent.TYPE, ChangeRestoredEvent.class); register(CommentAddedEvent.TYPE, CommentAddedEvent.class); register(CommitReceivedEvent.TYPE, CommitReceivedEvent.class); - register(DraftPublishedEvent.TYPE, DraftPublishedEvent.class); register(HashtagsChangedEvent.TYPE, HashtagsChangedEvent.class); register(RefUpdatedEvent.TYPE, RefUpdatedEvent.class); register(RefReceivedEvent.TYPE, RefReceivedEvent.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/StreamEventsApiListener.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/StreamEventsApiListener.java index f4a8715bb0..01fda6332d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/StreamEventsApiListener.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/StreamEventsApiListener.java @@ -29,7 +29,6 @@ import com.google.gerrit.extensions.events.ChangeAbandonedListener; import com.google.gerrit.extensions.events.ChangeMergedListener; import com.google.gerrit.extensions.events.ChangeRestoredListener; import com.google.gerrit.extensions.events.CommentAddedListener; -import com.google.gerrit.extensions.events.DraftPublishedListener; import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; import com.google.gerrit.extensions.events.HashtagsEditedListener; import com.google.gerrit.extensions.events.NewProjectCreatedListener; @@ -79,7 +78,6 @@ public class StreamEventsApiListener ChangeMergedListener, ChangeRestoredListener, CommentAddedListener, - DraftPublishedListener, GitReferenceUpdatedListener, HashtagsEditedListener, NewProjectCreatedListener, @@ -98,7 +96,6 @@ public class StreamEventsApiListener DynamicSet.bind(binder(), ChangeMergedListener.class).to(StreamEventsApiListener.class); DynamicSet.bind(binder(), ChangeRestoredListener.class).to(StreamEventsApiListener.class); DynamicSet.bind(binder(), CommentAddedListener.class).to(StreamEventsApiListener.class); - DynamicSet.bind(binder(), DraftPublishedListener.class).to(StreamEventsApiListener.class); DynamicSet.bind(binder(), GitReferenceUpdatedListener.class) .to(StreamEventsApiListener.class); DynamicSet.bind(binder(), HashtagsEditedListener.class).to(StreamEventsApiListener.class); @@ -391,24 +388,6 @@ public class StreamEventsApiListener } } - @Override - public void onDraftPublished(DraftPublishedListener.Event ev) { - try { - ChangeNotes notes = getNotes(ev.getChange()); - Change change = notes.getChange(); - PatchSet ps = getPatchSet(notes, ev.getRevision()); - DraftPublishedEvent event = new DraftPublishedEvent(change); - - event.change = changeAttributeSupplier(change); - event.patchSet = patchSetAttributeSupplier(change, ps); - event.uploader = accountAttributeSupplier(ev.getWho()); - - dispatcher.get().postEvent(change, event); - } catch (OrmException | PermissionBackendException e) { - log.error("Failed to dispatch event", e); - } - } - @Override public void onCommentAdded(CommentAddedListener.Event ev) { try { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/DraftPublished.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/DraftPublished.java deleted file mode 100644 index b5db0daf4e..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/DraftPublished.java +++ /dev/null @@ -1,73 +0,0 @@ -// Copyright (C) 2015 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.extensions.events; - -import com.google.gerrit.extensions.api.changes.NotifyHandling; -import com.google.gerrit.extensions.common.AccountInfo; -import com.google.gerrit.extensions.common.ChangeInfo; -import com.google.gerrit.extensions.common.RevisionInfo; -import com.google.gerrit.extensions.events.DraftPublishedListener; -import com.google.gerrit.extensions.registration.DynamicSet; -import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.server.GpgException; -import com.google.gerrit.server.patch.PatchListNotAvailableException; -import com.google.gwtorm.server.OrmException; -import com.google.inject.Inject; -import java.io.IOException; -import java.sql.Timestamp; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class DraftPublished { - private static final Logger log = LoggerFactory.getLogger(DraftPublished.class); - - private final DynamicSet listeners; - private final EventUtil util; - - @Inject - public DraftPublished(DynamicSet listeners, EventUtil util) { - this.listeners = listeners; - this.util = util; - } - - public void fire(Change change, PatchSet patchSet, Account accountId, Timestamp when) { - try { - Event event = - new Event( - util.changeInfo(change), - util.revisionInfo(change.getProject(), patchSet), - util.accountInfo(accountId), - when); - for (DraftPublishedListener l : listeners) { - try { - l.onDraftPublished(event); - } catch (Exception e) { - util.logEventListenerError(this, l, e); - } - } - } catch (PatchListNotAvailableException | GpgException | IOException | OrmException e) { - log.error("Couldn't fire event", e); - } - } - - private static class Event extends AbstractRevisionEvent implements DraftPublishedListener.Event { - - Event(ChangeInfo change, RevisionInfo revision, AccountInfo publisher, Timestamp when) { - super(change, revision, publisher, when, NotifyHandling.ALL); - } - } -} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/AbandonOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/AbandonOp.java index f4185c904b..8298db31b0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/AbandonOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/AbandonOp.java @@ -98,8 +98,6 @@ public class AbandonOp implements BatchUpdateOp { ChangeUpdate update = ctx.getUpdate(psId); if (!change.getStatus().isOpen()) { throw new ResourceConflictException("change is " + ChangeUtil.status(change)); - } else if (change.getStatus() == Change.Status.DRAFT) { - throw new ResourceConflictException("draft changes cannot be abandoned"); } patchSet = psUtil.get(ctx.getDb(), ctx.getNotes(), psId); change.setStatus(Change.Status.ABANDONED); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeReportFormatter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeReportFormatter.java index 3f3e2dd210..3ce6b2b085 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeReportFormatter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeReportFormatter.java @@ -26,9 +26,6 @@ public interface ChangeReportFormatter { @Nullable public abstract String subject(); - @Nullable - public abstract Boolean isDraft(); - @Nullable public abstract Boolean isEdit(); @@ -48,8 +45,6 @@ public interface ChangeReportFormatter { public abstract Builder setSubject(String val); - public abstract Builder setIsDraft(Boolean val); - public abstract Builder setIsEdit(Boolean val); public abstract Builder setIsPrivate(Boolean val); @@ -60,8 +55,6 @@ public interface ChangeReportFormatter { abstract String subject(); - abstract Boolean isDraft(); - abstract Boolean isEdit(); abstract Boolean isPrivate(); @@ -73,7 +66,6 @@ public interface ChangeReportFormatter { public Input build() { setChange(change()); setSubject(subject() == null ? change().getSubject() : subject()); - setIsDraft(isDraft() == null ? Change.Status.DRAFT == change().getStatus() : isDraft()); setIsEdit(isEdit() == null ? false : isEdit()); setIsPrivate(isPrivate() == null ? change().isPrivate() : isPrivate()); setIsWorkInProgress( diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java index 8b5ab06052..ac69ff10f3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java @@ -49,9 +49,6 @@ public class DefaultChangeReportFormatter implements ChangeReportFormatter { .append(ChangeUtil.formatChangeUrl(url, input.change())) .append(" ") .append(ChangeUtil.cropSubject(input.subject())); - if (input.isDraft()) { - m.append(" [DRAFT]"); - } if (input.isEdit()) { m.append(" [EDIT]"); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index c03ed6ce65..3c8bcea5bd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -58,7 +58,6 @@ import com.google.gerrit.common.Nullable; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelTypes; -import com.google.gerrit.common.data.Permission; import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.extensions.api.changes.HashtagsInput; import com.google.gerrit.extensions.api.changes.NotifyHandling; @@ -662,7 +661,6 @@ class ReceiveCommits { ChangeReportFormatter.Input.builder() .setChange(u.notes.getChange()) .setSubject(subject) - .setIsDraft(u.replaceOp != null && u.replaceOp.getPatchSet().isDraft()) .setIsEdit(edit) .setIsPrivate(isPrivate) .setIsWorkInProgress(wip) @@ -1239,11 +1237,6 @@ class ReceiveCommits { cc.add(id); } - @Option(name = "--publish", usage = "publish new/updated changes") - void publish(boolean publish) { - draft = !publish; - } - @Option( name = "--label", aliases = {"-l"}, @@ -1473,21 +1466,6 @@ class ReceiveCommits { return; } - if (magicBranch.draft) { - // TODO(xchangcheng): reject all after repo-tool supports private and wip changes. - if (!receiveConfig.allowDrafts) { - errors.put(ReceiveError.CODE_REVIEW, ref); - reject(cmd, "draft workflow is disabled"); - return; - } else if (projectControl - .controlForRef(MagicBranch.NEW_DRAFT_CHANGE + ref) - .isBlocked(Permission.PUSH)) { - errors.put(ReceiveError.CODE_REVIEW, ref); - reject(cmd, "cannot upload drafts"); - return; - } - } - try { magicBranch.perm.check(RefPermission.CREATE_CHANGE); } catch (AuthException denied) { @@ -1511,11 +1489,6 @@ class ReceiveCommits { return; } - if (magicBranch.draft && magicBranch.submit) { - reject(cmd, "cannot submit draft"); - return; - } - if (magicBranch.submit) { try { permissions.ref(ref).check(RefPermission.UPDATE_BY_SUBMIT); @@ -1539,10 +1512,6 @@ class ReceiveCommits { String destBranch = magicBranch.dest.get(); try { if (magicBranch.merged) { - if (magicBranch.draft) { - reject(cmd, "cannot be draft & merged"); - return; - } if (magicBranch.base != null) { reject(cmd, "cannot use merged with base"); return; @@ -2157,7 +2126,7 @@ class ReceiveCommits { checkNotNull(magicBranch); recipients.add(magicBranch.getMailRecipients()); approvals = magicBranch.labels; - recipients.add(getRecipientsFromFooters(db, accountResolver, false, footerLines)); + recipients.add(getRecipientsFromFooters(db, accountResolver, footerLines)); recipients.remove(me); StringBuilder msg = new StringBuilder( diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveConfig.java index 30d5071f43..89158d30fa 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveConfig.java @@ -27,7 +27,6 @@ import org.eclipse.jgit.lib.Config; class ReceiveConfig { final boolean checkMagicRefs; final boolean checkReferencedObjectsAreReachable; - final boolean allowDrafts; final int maxBatchCommits; private final int systemMaxBatchChanges; private final AccountLimits.Factory limitsFactory; @@ -37,7 +36,6 @@ class ReceiveConfig { checkMagicRefs = config.getBoolean("receive", null, "checkMagicRefs", true); checkReferencedObjectsAreReachable = config.getBoolean("receive", null, "checkReferencedObjectsAreReachable", true); - allowDrafts = config.getBoolean("change", null, "allowDrafts", true); maxBatchCommits = config.getInt("receive", null, "maxBatchCommits", 10000); systemMaxBatchChanges = config.getInt("receive", "maxBatchChanges", 0); this.limitsFactory = limitsFactory; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReplaceOp.java index c91e5cfa54..bcb85644e2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReplaceOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReplaceOp.java @@ -279,10 +279,6 @@ public class ReplaceOp implements BatchUpdateOp { } } - boolean draft = magicBranch != null && magicBranch.draft; - if (change.getStatus() == Change.Status.DRAFT && !draft) { - update.setStatus(Change.Status.NEW); - } newPatchSet = psUtil.insert( ctx.getDb(), @@ -290,14 +286,12 @@ public class ReplaceOp implements BatchUpdateOp { update, patchSetId, commitId, - draft, groups, pushCertificate != null ? pushCertificate.toTextWithSignature() : null, psDescription); update.setPsDescription(psDescription); - recipients.add( - getRecipientsFromFooters(ctx.getDb(), accountResolver, draft, commit.getFooterLines())); + recipients.add(getRecipientsFromFooters(ctx.getDb(), accountResolver, commit.getFooterLines())); recipients.remove(ctx.getAccountId()); ChangeData cd = changeDataFactory.create(ctx.getDb(), ctx.getNotes()); MailRecipients oldRecipients = getRecipientsFromReviewers(cd.reviewers()); @@ -430,11 +424,7 @@ public class ReplaceOp implements BatchUpdateOp { if (magicBranch != null && magicBranch.topic != null) { change.setTopic(magicBranch.topic); } - if (change.getStatus() == Change.Status.DRAFT && newPatchSet.isDraft()) { - // Leave in draft status. - } else { - change.setStatus(Change.Status.NEW); - } + change.setStatus(Change.Status.NEW); change.setCurrentPatchSet(info); List idList = commit.getFooterLines(CHANGE_ID); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java index 26010518d4..77aa9508b0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java @@ -157,7 +157,6 @@ public class CherryPick extends SubmitStrategy { ctx.getUpdate(psId), psId, newCommit, - false, prevPs != null ? prevPs.getGroups() : ImmutableList.of(), null, null); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java index 12526bb27e..5421254a29 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java @@ -222,7 +222,6 @@ public class RebaseSubmitStrategy extends SubmitStrategy { ctx.getUpdate(newPatchSetId), newPatchSetId, newCommit, - false, prevPs != null ? prevPs.getGroups() : ImmutableList.of(), null, null); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java index 0fd8782934..152d398ac0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java @@ -324,7 +324,6 @@ abstract class SubmitStrategyOp implements BatchUpdateOp { ctx.getUpdate(psId), psId, alreadyMergedCommit, - false, groups, null, null); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java index 24bde800a7..1f8b5404e0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java @@ -566,7 +566,7 @@ public class ChangeField { // Submit rule options in this class should never use fastEvalLabels. This // slows down indexing slightly but produces correct search results. public static final SubmitRuleOptions SUBMIT_RULE_OPTIONS_LENIENT = - SubmitRuleOptions.defaults().allowClosed(true).allowDraft(true).build(); + SubmitRuleOptions.defaults().allowClosed(true).build(); public static final SubmitRuleOptions SUBMIT_RULE_OPTIONS_STRICT = SubmitRuleOptions.defaults().build(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java index 41e8d1096b..144ec1d848 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java @@ -90,6 +90,9 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions { static final Schema V46 = schema(V45); + // Removal of draft change workflow requires reindexing + static final Schema V47 = schema(V46); + public static final String NAME = "changes"; public static final ChangeSchemaDefinitions INSTANCE = new ChangeSchemaDefinitions(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java index a7523245c4..be1c9f53a4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java @@ -39,24 +39,18 @@ public class MailUtil { DateTimeFormatter.ofPattern("EEE, dd MMM yyyy HH:mm:ss ZZZ"); public static MailRecipients getRecipientsFromFooters( - ReviewDb db, - AccountResolver accountResolver, - boolean draftPatchSet, - List footerLines) + ReviewDb db, AccountResolver accountResolver, List footerLines) throws OrmException, IOException { MailRecipients recipients = new MailRecipients(); - if (!draftPatchSet) { - for (FooterLine footerLine : footerLines) { - try { - if (isReviewer(footerLine)) { - recipients.reviewers.add( - toAccountId(db, accountResolver, footerLine.getValue().trim())); - } else if (footerLine.matches(FooterKey.CC)) { - recipients.cc.add(toAccountId(db, accountResolver, footerLine.getValue().trim())); - } - } catch (NoSuchAccountException e) { - continue; + for (FooterLine footerLine : footerLines) { + try { + if (isReviewer(footerLine)) { + recipients.reviewers.add(toAccountId(db, accountResolver, footerLine.getValue().trim())); + } else if (footerLine.matches(FooterKey.CC)) { + recipients.cc.add(toAccountId(db, accountResolver, footerLine.getValue().trim())); } + } catch (NoSuchAccountException e) { + continue; } } return recipients; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java index 2d1fc56817..5b7d3b79b2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java @@ -155,9 +155,7 @@ public class CommentSender extends ReplyToChangeSender { } if (notify.compareTo(NotifyHandling.ALL) >= 0) { bccStarredBy(); - includeWatchers( - NotifyType.ALL_COMMENTS, - !patchSet.isDraft() && !change.isWorkInProgress() && !change.isPrivate()); + includeWatchers(NotifyType.ALL_COMMENTS, !change.isWorkInProgress() && !change.isPrivate()); } removeUsersThatIgnoredTheChange(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CreateChangeSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CreateChangeSender.java index 8757a28346..6d15d6f0cd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CreateChangeSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CreateChangeSender.java @@ -47,14 +47,10 @@ public class CreateChangeSender extends NewChangeSender { protected void init() throws EmailException { super.init(); - 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 && !change.isWorkInProgress() && !change.isPrivate()); + getWatchers(NotifyType.NEW_CHANGES, !change.isWorkInProgress() && !change.isPrivate()); for (Account.Id user : Iterables.concat(matching.to.accounts, matching.cc.accounts, matching.bcc.accounts)) { if (isOwnerOfProjectOrBranch(user)) { @@ -73,8 +69,7 @@ public class CreateChangeSender extends NewChangeSender { log.warn("Cannot notify watchers for new change", err); } - includeWatchers( - NotifyType.NEW_PATCHSETS, !isDraft && !change.isWorkInProgress() && !change.isPrivate()); + includeWatchers(NotifyType.NEW_PATCHSETS, !change.isWorkInProgress() && !change.isPrivate()); } private boolean isOwnerOfProjectOrBranch(Account.Id user) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java index b5dbc84e74..c9e5791f5a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java @@ -69,9 +69,7 @@ public class ReplacePatchSetSender extends ReplyToChangeSender { } rcptToAuthors(RecipientType.CC); bccStarredBy(); - includeWatchers( - NotifyType.NEW_PATCHSETS, - !patchSet.isDraft() && !change.isWorkInProgress() && !change.isPrivate()); + includeWatchers(NotifyType.NEW_PATCHSETS, !change.isWorkInProgress() && !change.isPrivate()); removeUsersThatIgnoredTheChange(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java index 526f3c8cde..200e0d6bec 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java @@ -238,7 +238,7 @@ public class ChangeBundle { checkColumns(ChangeMessage.Key.class, 1, 2); checkColumns(ChangeMessage.class, 1, 2, 3, 4, 5, 6, 7); checkColumns(PatchSet.Id.class, 1, 2); - checkColumns(PatchSet.class, 1, 2, 3, 4, 5, 6, 8, 9); + checkColumns(PatchSet.class, 1, 2, 3, 4, 6, 8, 9); checkColumns(PatchSetApproval.Key.class, 1, 2, 3); checkColumns(PatchSetApproval.class, 1, 2, 3, 6, 7, 8); checkColumns(PatchLineComment.Key.class, 1, 2); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java index 4b239ea75e..d6472bc0c9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java @@ -716,9 +716,9 @@ class ChangeNotesParser { int changeMessageStart; if (raw[subjectEnd] == '\n') { - changeMessageStart = subjectEnd + 2; //\n\n ends paragraph + changeMessageStart = subjectEnd + 2; // \n\n ends paragraph } else if (raw[subjectEnd] == '\r') { - changeMessageStart = subjectEnd + 4; //\r\n\r\n ends paragraph + changeMessageStart = subjectEnd + 4; // \r\n\r\n ends paragraph } else { return; } @@ -1081,13 +1081,6 @@ class ChangeNotesParser { case DELETED: patchSets.remove(e.getKey()); break; - - case DRAFT: - PatchSet ps = patchSets.get(e.getKey()); - if (ps != null) { - ps.setDraft(true); - } - break; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/PatchSetState.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/PatchSetState.java index 32be9c5a94..d66880191d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/PatchSetState.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/PatchSetState.java @@ -18,9 +18,6 @@ public enum PatchSetState { /** Published and visible to anyone who can see the change; the default. */ PUBLISHED, - /** Draft patch set, only visible to certain users. */ - DRAFT, - /** * Deleted patch set. * diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java index be61642223..166d8a9c53 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java @@ -444,9 +444,7 @@ public class ChangeRebuilderImpl extends ChangeRebuilder { // patch set. // // Start with the first patch set that actually exists. If there are no patch sets at all, - // minPsNum will be null, so just bail and use 1 as the patch set ID. The corresponding patch - // set won't exist, but this change is probably corrupt anyway, as deleting the last draft patch - // set should have deleted the whole change. + // minPsNum will be null, so just bail and use 1 as the patch set ID. // // Second, ensure timestamps are nondecreasing, by copying the previous timestamp if this // happens. This assumes that the only way this can happen is due to dependency constraints, and diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/PatchSetEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/PatchSetEvent.java index e0ad6401cd..acb80c02e9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/PatchSetEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/PatchSetEvent.java @@ -17,7 +17,6 @@ package com.google.gerrit.server.notedb.rebuild; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.server.notedb.ChangeUpdate; -import com.google.gerrit.server.notedb.PatchSetState; import com.google.gwtorm.server.OrmException; import java.io.IOException; import java.util.List; @@ -65,9 +64,6 @@ class PatchSetEvent extends Event { if (!groups.isEmpty()) { update.setGroups(ps.getGroups()); } - if (ps.isDraft()) { - update.setPatchSetState(PatchSetState.DRAFT); - } } private void setRevision(ChangeUpdate update, PatchSet ps) throws IOException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java index 55fbab55fd..384d4fd6f2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java @@ -197,8 +197,7 @@ public class PatchScriptFactory implements Callable { PatchSet psEntityB = psb.get() == 0 ? new PatchSet(psb) : psUtil.get(db, notes, psb); ChangeControl ctl = changeControlFactory.controlFor(notes, userProvider.get()); - if ((psEntityA != null && !ctl.isPatchVisible(psEntityA, db)) - || (psEntityB != null && !ctl.isPatchVisible(psEntityB, db))) { + if ((psEntityA != null && !ctl.isVisible(db)) || (psEntityB != null && !ctl.isVisible(db))) { throw new NoSuchChangeException(changeId); } @@ -299,7 +298,7 @@ public class PatchScriptFactory implements Callable { // history = new ArrayList<>(); for (PatchSet ps : psUtil.byChange(db, notes)) { - if (!ctl.isPatchVisible(ps, db)) { + if (!ctl.isVisible(db)) { continue; } String name = fileName; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java index 6a55e50320..2203f772e3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java @@ -93,6 +93,14 @@ public class ChangeControl { throw new NoSuchChangeException(notes.getChangeId(), e); } } + + public ChangeControl validateFor(Change.Id changeId, CurrentUser user) throws OrmException { + return validateFor(notesFactory.createChecked(changeId), user); + } + + public ChangeControl validateFor(ChangeNotes notes, CurrentUser user) throws OrmException { + return controlFor(notes, user); + } } @Singleton @@ -181,7 +189,7 @@ public class ChangeControl { } /** Can this user see this change? */ - boolean isVisible(ReviewDb db) throws OrmException { + public boolean isVisible(ReviewDb db) throws OrmException { return isVisible(db, null); } @@ -190,19 +198,12 @@ public class ChangeControl { if (getChange().isPrivate() && !isPrivateVisible(db, cd)) { return false; } - if (getChange().getStatus() == Change.Status.DRAFT && !isDraftVisible(db, cd)) { - return false; - } - return getRefControl().isVisible(); + return isRefVisible(); } - /** Can this user see the given patchset? */ - public boolean isPatchVisible(PatchSet ps, ReviewDb db) throws OrmException { - // TODO(hiesel) These don't need to be migrated, just remove after support for drafts is removed - if (ps != null && ps.isDraft() && !isDraftVisible(db, null)) { - return false; - } - return isVisible(db); + /** Can the user see this change? Does not account for draft status */ + public boolean isRefVisible() { + return getRefControl().isVisible(); } /** Can this user see the given patchset? */ @@ -210,9 +211,6 @@ public class ChangeControl { // TODO(hiesel) These don't need to be migrated, just remove after support for drafts is removed checkArgument( cd.getId().equals(ps.getId().getParentKey()), "%s not for change %s", ps, cd.getId()); - if (ps.isDraft() && !isDraftVisible(cd.db(), cd)) { - return false; - } return isVisible(cd.db()); } @@ -226,7 +224,7 @@ public class ChangeControl { Predicate predicate = ps -> { try { - return isPatchVisible(ps, db); + return isVisible(db); } catch (OrmException e) { return false; } @@ -244,27 +242,9 @@ public class ChangeControl { && !isPatchSetLocked(db); } - /** Can this user publish this draft change or any draft patch set of this change? */ - public boolean canPublish(ReviewDb db) throws OrmException { - // TODO(hiesel) These don't need to be migrated, just remove after support for drafts is removed - return (isOwner() || getRefControl().canPublishDrafts()) && isVisible(db); - } - - /** Can this user delete this draft change or any patch set of this change? */ - public boolean canDeleteDraft(ReviewDb db) throws OrmException { - // TODO(hiesel) These don't need to be migrated, just remove after support for drafts is removed - return canDelete(db, Change.Status.DRAFT); - } - - /** Can this user delete this change or any patch set of this change? */ - private boolean canDelete(ReviewDb db, Change.Status status) throws OrmException { - if (!isVisible(db)) { - return false; - } - + /** Can this user delete this change? */ + public boolean canDelete(Change.Status status) { switch (status) { - case DRAFT: - return isOwner() || getRefControl().canDeleteDrafts() || getProjectControl().isAdmin(); case NEW: case ABANDONED: return (isOwner() && getRefControl().canDeleteOwnChanges()) @@ -295,9 +275,7 @@ public class ChangeControl { /** Can this user add a patch set to this change? */ private boolean canAddPatchSet(ReviewDb db) throws OrmException { - if (!refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE) - || isPatchSetLocked(db) - || !isPatchVisible(patchSetUtil.current(db, notes), db)) { + if (!refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE) || isPatchSetLocked(db)) { return false; } if (isOwner()) { @@ -400,14 +378,6 @@ public class ChangeControl { return cd != null ? cd : changeDataFactory.create(db, getNotes()); } - public boolean isDraftVisible(ReviewDb db, ChangeData cd) throws OrmException { - // TODO(hiesel) These don't need to be migrated, just remove after support for drafts is removed - return isOwner() - || isReviewer(db, cd) - || getRefControl().canViewDrafts() - || getUser().isInternalUser(); - } - private boolean isPrivateVisible(ReviewDb db, ChangeData cd) throws OrmException { return isOwner() || isReviewer(db, cd) @@ -496,7 +466,7 @@ public class ChangeControl { case ABANDON: return canAbandon(db()); case DELETE: - return canDelete(db(), getChange().getStatus()); + return canDelete(getChange().getStatus()); case ADD_PATCH_SET: return canAddPatchSet(db()); case EDIT_ASSIGNEE: diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java index 8aa475ba11..a92c0b7c31 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java @@ -280,26 +280,11 @@ public class RefControl { return canPerform(Permission.REMOVE_REVIEWER); } - /** @return true if this user can view draft changes. */ - boolean canViewDrafts() { - return canPerform(Permission.VIEW_DRAFTS); - } - /** @return true if this user can view private changes. */ boolean canViewPrivateChanges() { return canPerform(Permission.VIEW_PRIVATE_CHANGES); } - /** @return true if this user can publish draft changes. */ - boolean canPublishDrafts() { - return canPerform(Permission.PUBLISH_DRAFTS); - } - - /** @return true if this user can delete draft changes. */ - boolean canDeleteDrafts() { - return canPerform(Permission.DELETE_DRAFTS); - } - /** @return true if this user can delete their own changes. */ boolean canDeleteOwnChanges() { return canPerform(Permission.DELETE_OWN_CHANGES); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java index e4352e4ef7..f501dd3a7b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java @@ -24,7 +24,6 @@ import com.google.gerrit.extensions.client.SubmitType; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.rules.PrologEnvironment; import com.google.gerrit.rules.StoredValues; import com.google.gerrit.server.CurrentUser; @@ -34,7 +33,6 @@ import com.google.gerrit.server.account.Emails; import com.google.gerrit.server.query.change.ChangeData; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; import com.googlecode.prolog_cafe.exceptions.CompileException; import com.googlecode.prolog_cafe.exceptions.ReductionLimitException; @@ -97,8 +95,6 @@ public class SubmitRuleEvaluator { private final Accounts accounts; private final Emails emails; private final ProjectCache projectCache; - private final Provider dbProvider; - private final ChangeControl.GenericFactory changeControlFactory; private final ChangeData cd; private SubmitRuleOptions.Builder optsBuilder = SubmitRuleOptions.defaults(); @@ -118,16 +114,12 @@ public class SubmitRuleEvaluator { Accounts accounts, Emails emails, ProjectCache projectCache, - Provider dbProvider, - ChangeControl.GenericFactory changeControlFactory, @Assisted CurrentUser user, @Assisted ChangeData cd) { this.accountCache = accountCache; this.accounts = accounts; this.emails = emails; this.projectCache = projectCache; - this.dbProvider = dbProvider; - this.changeControlFactory = changeControlFactory; this.user = user; this.cd = cd; } @@ -191,16 +183,6 @@ public class SubmitRuleEvaluator { return this; } - /** - * @param allow whether to allow {@link #evaluate()} on draft changes. - * @return this - */ - public SubmitRuleEvaluator setAllowDraft(boolean allow) { - checkNotStarted(); - optsBuilder.allowDraft(allow); - return this; - } - /** * @param skip if true, submit filter will not be applied. * @return this @@ -254,11 +236,6 @@ public class SubmitRuleEvaluator { rec.status = SubmitRecord.Status.CLOSED; return Collections.singletonList(rec); } - if (!opts.allowDraft()) { - if (change.getStatus() == Change.Status.DRAFT || patchSet.isDraft()) { - return cannotSubmitDraft(); - } - } List results; try { @@ -287,25 +264,6 @@ public class SubmitRuleEvaluator { return resultsToSubmitRecord(getSubmitRule(), results); } - private List cannotSubmitDraft() { - try { - if (!changeControlFactory - .controlFor(dbProvider.get(), change, user) - .isDraftVisible(cd.db(), cd)) { - return createRuleError("Patch set " + patchSet.getId() + " not found"); - } - if (patchSet.isDraft()) { - return createRuleError("Cannot submit draft patch sets"); - } - return createRuleError("Cannot submit draft changes"); - } catch (OrmException err) { - PatchSet.Id psId = patchSet != null ? patchSet.getId() : patchSet.getId(); - String msg = "Cannot check visibility of patch set " + psId; - log.error(msg, err); - return createRuleError(msg); - } - } - /** * Convert the results from Prolog Cafe's format to Gerrit's common format. * @@ -438,25 +396,6 @@ public class SubmitRuleEvaluator { return typeError("Error looking up change " + cd.getId(), e); } - try { - if (change.getStatus() == Change.Status.DRAFT - && !changeControlFactory - .controlFor(dbProvider.get(), change, user) - .isDraftVisible(cd.db(), cd)) { - return SubmitTypeRecord.error("Patch set " + patchSet.getId() + " not found"); - } - if (patchSet.isDraft() - && !changeControlFactory - .controlFor(dbProvider.get(), change, user) - .isDraftVisible(cd.db(), cd)) { - return SubmitTypeRecord.error("Patch set " + patchSet.getId() + " not found"); - } - } catch (OrmException err) { - String msg = "Cannot read patch set " + patchSet.getId(); - log.error(msg, err); - return SubmitTypeRecord.error(msg); - } - List results; try { results = diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleOptions.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleOptions.java index 6d6aaad907..3e89f23550 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleOptions.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleOptions.java @@ -30,18 +30,11 @@ public abstract class SubmitRuleOptions { } public static Builder defaults() { - return builder() - .fastEvalLabels(false) - .allowDraft(false) - .allowClosed(false) - .skipFilters(false) - .rule(null); + return builder().fastEvalLabels(false).allowClosed(false).skipFilters(false).rule(null); } public abstract boolean fastEvalLabels(); - public abstract boolean allowDraft(); - public abstract boolean allowClosed(); public abstract boolean skipFilters(); @@ -53,8 +46,6 @@ public abstract class SubmitRuleOptions { public abstract static class Builder { public abstract SubmitRuleOptions.Builder fastEvalLabels(boolean fastEvalLabels); - public abstract SubmitRuleOptions.Builder allowDraft(boolean allowDraft); - public abstract SubmitRuleOptions.Builder allowClosed(boolean allowClosed); public abstract SubmitRuleOptions.Builder skipFilters(boolean skipFilters); @@ -67,7 +58,6 @@ public abstract class SubmitRuleOptions { public Builder toBuilder() { return builder() .fastEvalLabels(fastEvalLabels()) - .allowDraft(allowDraft()) .allowClosed(allowClosed()) .skipFilters(skipFilters()) .rule(rule()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java index 30bb7605c6..40b384db7d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java @@ -981,7 +981,7 @@ public class ChangeData { } PatchSet ps = currentPatchSet(); try { - if (ps == null || !changeControl().isPatchVisible(ps, db)) { + if (ps == null || !changeControl().isVisible(db)) { return null; } } catch (OrmException e) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java index db3b94c3c1..1eb27706e3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java @@ -17,9 +17,6 @@ package com.google.gerrit.server.query.change; import com.google.gerrit.index.FieldDef; import com.google.gerrit.index.query.IndexPredicate; import com.google.gerrit.index.query.Matchable; -import com.google.gerrit.index.query.Predicate; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.server.query.change.ChangeQueryBuilder.Arguments; public abstract class ChangeIndexPredicate extends IndexPredicate implements Matchable { @@ -30,11 +27,4 @@ public abstract class ChangeIndexPredicate extends IndexPredicate protected ChangeIndexPredicate(FieldDef def, String name, String value) { super(def, name, value); } - - protected static Predicate create(Arguments args, Predicate p) { - if (!args.allowsDrafts) { - return Predicate.and(p, Predicate.not(new ChangeStatusPredicate(Change.Status.DRAFT))); - } - return p; - } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index 5cd2a01487..309f15c5a0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -573,7 +573,7 @@ public class ChangeQueryBuilder extends QueryBuilder { } if ("cc".equalsIgnoreCase(value)) { - return ReviewerPredicate.cc(args, self()); + return ReviewerPredicate.cc(self()); } if ("mergeable".equalsIgnoreCase(value)) { @@ -1311,7 +1311,7 @@ public class ChangeQueryBuilder extends QueryBuilder { if (args.index.getSchema().hasField(ChangeField.REVIEWER_BY_EMAIL)) { Address address = Address.tryParse(who); if (address != null) { - reviewerByEmailPredicate = ReviewerByEmailPredicate.forState(args, address, state); + reviewerByEmailPredicate = ReviewerByEmailPredicate.forState(address, state); } } @@ -1323,7 +1323,7 @@ public class ChangeQueryBuilder extends QueryBuilder { Predicate.or( accounts .stream() - .map(id -> ReviewerPredicate.forState(args, id, state)) + .map(id -> ReviewerPredicate.forState(id, state)) .collect(toList())); } } catch (QueryParseException e) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java index 361b57ade3..ee9c570217 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java @@ -253,12 +253,7 @@ public class OutputStreamQuery { if (includeSubmitRecords) { eventFactory.addSubmitRecords( - c, - submitRuleEvaluatorFactory - .create(user, d) - .setAllowClosed(true) - .setAllowDraft(true) - .evaluate()); + c, submitRuleEvaluatorFactory.create(user, d).setAllowClosed(true).evaluate()); } if (includeCommitMessage) { @@ -293,7 +288,7 @@ public class OutputStreamQuery { if (includeCurrentPatchSet) { PatchSet current = d.currentPatchSet(); - if (current != null && ctl.isPatchVisible(current, d.db())) { + if (current != null && ctl.isVisible(d.db())) { c.currentPatchSet = eventFactory.asPatchSetAttribute(db, rw, d.change(), current); eventFactory.addApprovals(c.currentPatchSet, d.currentApprovals(), labelTypes); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerByEmailPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerByEmailPredicate.java index 16feed99f3..f4e979c82a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerByEmailPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerByEmailPredicate.java @@ -20,14 +20,13 @@ import com.google.gerrit.index.query.Predicate; import com.google.gerrit.server.index.change.ChangeField; import com.google.gerrit.server.mail.Address; import com.google.gerrit.server.notedb.ReviewerStateInternal; -import com.google.gerrit.server.query.change.ChangeQueryBuilder.Arguments; import com.google.gwtorm.server.OrmException; class ReviewerByEmailPredicate extends ChangeIndexPredicate { - static Predicate forState(Arguments args, Address adr, ReviewerStateInternal state) { + static Predicate forState(Address adr, ReviewerStateInternal state) { checkArgument(state != ReviewerStateInternal.REMOVED, "can't query by removed reviewer"); - return create(args, new ReviewerByEmailPredicate(state, adr)); + return new ReviewerByEmailPredicate(state, adr); } private final ReviewerStateInternal state; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java index 7bea4a4c09..5364a66316 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java @@ -26,30 +26,26 @@ import com.google.gwtorm.server.OrmException; import java.util.stream.Stream; public class ReviewerPredicate extends ChangeIndexPredicate { - protected static Predicate forState( - Arguments args, Account.Id id, ReviewerStateInternal state) { + protected static Predicate forState(Account.Id id, ReviewerStateInternal state) { checkArgument(state != ReviewerStateInternal.REMOVED, "can't query by removed reviewer"); - return create(args, new ReviewerPredicate(state, id)); + return new ReviewerPredicate(state, id); } protected static Predicate reviewer(Arguments args, Account.Id id) { - Predicate p; if (args.notesMigration.readChanges()) { // With NoteDb, Reviewer/CC are clearly distinct states, so only choose reviewer. - p = new ReviewerPredicate(ReviewerStateInternal.REVIEWER, id); - } else { - // Without NoteDb, Reviewer/CC are a bit unpredictable; maintain the old behavior of matching - // any reviewer state. - p = anyReviewerState(id); + return new ReviewerPredicate(ReviewerStateInternal.REVIEWER, id); } - return create(args, p); + // Without NoteDb, Reviewer/CC are a bit unpredictable; maintain the old behavior of matching + // any reviewer state. + return anyReviewerState(id); } - protected static Predicate cc(Arguments args, Account.Id id) { + protected static Predicate cc(Account.Id id) { // As noted above, CC is nebulous without NoteDb, but it certainly doesn't make sense to return // Reviewers for cc:foo. Most likely this will just not match anything, but let the index sort // it out. - return create(args, new ReviewerPredicate(ReviewerStateInternal.CC, id)); + return new ReviewerPredicate(ReviewerStateInternal.CC, id); } protected static Predicate anyReviewerState(Account.Id id) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java index 253e9d80e5..1919051085 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java @@ -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 C = Schema_158.class; + public static final Class C = Schema_159.class; public static int getBinaryVersion() { return guessVersion(C); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_159.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_159.java new file mode 100644 index 0000000000..956cb8e28a --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_159.java @@ -0,0 +1,61 @@ +// 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.gerrit.reviewdb.server.ReviewDb; +import com.google.gwtorm.server.OrmException; +import com.google.gwtorm.server.StatementExecutor; +import com.google.inject.Inject; +import com.google.inject.Provider; + +/** Migrate draft changes to private or wip changes. */ +public class Schema_159 extends SchemaVersion { + + private static enum DraftWorkflowMigrationStrategy { + PRIVATE, + WORK_IN_PROGRESS + } + + @Inject + Schema_159(Provider prior) { + super(prior); + } + + @Override + protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException { + DraftWorkflowMigrationStrategy strategy = DraftWorkflowMigrationStrategy.PRIVATE; + if (ui.yesno( + false, "Migrate draft changes to work-in-progress changes (default is private)?")) { + strategy = DraftWorkflowMigrationStrategy.WORK_IN_PROGRESS; + } + ui.message( + String.format("Replace draft changes with %s changes ...", strategy.name().toLowerCase())); + try (StatementExecutor e = newExecutor(db)) { + String column = + strategy == DraftWorkflowMigrationStrategy.PRIVATE ? "is_private" : "work_in_progress"; + // Mark changes private/wip if changes have status draft or + // if they have any draft patch sets. + e.execute( + String.format( + "UPDATE changes SET %s = 'Y' WHERE status = 'd' OR " + + "EXISTS (SELECT * FROM patch_sets WHERE " + + "patch_sets.change_id = changes.change_id AND patch_sets.draft = 'Y')", + column)); + // Change change status from draft to new. + e.execute("UPDATE changes SET status = 'n' WHERE status = 'd'"); + } + ui.message("done"); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/MagicBranch.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/MagicBranch.java index 75e14cb777..2088409ef5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/util/MagicBranch.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/MagicBranch.java @@ -27,25 +27,21 @@ public final class MagicBranch { private static final Logger log = LoggerFactory.getLogger(MagicBranch.class); public static final String NEW_CHANGE = "refs/for/"; + // TODO: remove after 'repo' supports private/wip changes. public static final String NEW_DRAFT_CHANGE = "refs/drafts/"; - public static final String NEW_PUBLISH_CHANGE = "refs/publish/"; /** Extracts the destination from a ref name */ public static String getDestBranchName(String refName) { String magicBranch = NEW_CHANGE; if (refName.startsWith(NEW_DRAFT_CHANGE)) { magicBranch = NEW_DRAFT_CHANGE; - } else if (refName.startsWith(NEW_PUBLISH_CHANGE)) { - magicBranch = NEW_PUBLISH_CHANGE; } return refName.substring(magicBranch.length()); } /** Checks if the supplied ref name is a magic branch */ public static boolean isMagicBranch(String refName) { - return refName.startsWith(NEW_DRAFT_CHANGE) - || refName.startsWith(NEW_PUBLISH_CHANGE) - || refName.startsWith(NEW_CHANGE); + return refName.startsWith(NEW_DRAFT_CHANGE) || refName.startsWith(NEW_CHANGE); } /** Returns the ref name prefix for a magic branch, {@code null} if the branch is not magic */ @@ -53,9 +49,6 @@ public final class MagicBranch { if (refName.startsWith(NEW_DRAFT_CHANGE)) { return NEW_DRAFT_CHANGE; } - if (refName.startsWith(NEW_PUBLISH_CHANGE)) { - return NEW_PUBLISH_CHANGE; - } if (refName.startsWith(NEW_CHANGE)) { return NEW_CHANGE; } @@ -79,11 +72,6 @@ public final class MagicBranch { if (result != Capable.OK) { return result; } - result = checkMagicBranchRef(NEW_PUBLISH_CHANGE, repo, project); - if (result != Capable.OK) { - return result; - } - return Capable.OK; } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java index b589289fce..97d8ae8f61 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java @@ -18,8 +18,6 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.common.data.GlobalCapability.DEFAULT_MAX_QUERY_LIMIT; import static com.google.gerrit.index.query.Predicate.and; import static com.google.gerrit.index.query.Predicate.or; -import static com.google.gerrit.reviewdb.client.Change.Status.ABANDONED; -import static com.google.gerrit.reviewdb.client.Change.Status.DRAFT; import static com.google.gerrit.reviewdb.client.Change.Status.MERGED; import static com.google.gerrit.reviewdb.client.Change.Status.NEW; import static com.google.gerrit.server.index.change.IndexedChangeQuery.convertOptions; @@ -136,7 +134,7 @@ public class ChangeIndexRewriterTest extends GerritBaseTests { @Test public void duplicateCompoundNonIndexOnlyPredicates() throws Exception { - Predicate in = parse("(status:new OR status:draft) bar:p file:a"); + Predicate in = parse("status:new bar:p file:a"); Predicate out = rewrite(in); assertThat(out.getClass()).isEqualTo(AndChangeSource.class); assertThat(out.getChildren()) @@ -179,14 +177,11 @@ public class ChangeIndexRewriterTest extends GerritBaseTests { public void getPossibleStatus() throws Exception { assertThat(status("file:a")).isEqualTo(EnumSet.allOf(Change.Status.class)); assertThat(status("is:new")).containsExactly(NEW); - assertThat(status("-is:new")).containsExactly(DRAFT, MERGED, ABANDONED); assertThat(status("is:new OR is:merged")).containsExactly(NEW, MERGED); assertThat(status("is:new is:merged")).isEmpty(); - assertThat(status("(is:new is:draft) (is:merged)")).isEmpty(); - assertThat(status("(is:new is:draft) (is:merged)")).isEmpty(); - - assertThat(status("(is:new is:draft) OR (is:merged)")).containsExactly(MERGED); + assertThat(status("(is:new) (is:merged)")).isEmpty(); + assertThat(status("(is:new) (is:merged)")).isEmpty(); } @Test diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java index a04b755f64..5fa7a30d9d 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java @@ -328,7 +328,7 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest { + "Branch: refs/heads/master\n" + "Change-id: I577fb248e474018276351785930358ec0450e9f7\n" + "Subject: Some subject of a change\n"); - assertParseSucceeds( + assertParseFails( "Update change\n" + "\n" + "Patch-set: 1 (DRAFT)\n" diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java index db8ec25880..baa51b724a 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java @@ -1056,7 +1056,6 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { RevCommit commit = tr.commit().message("PS2").create(); ChangeUpdate update = newUpdate(c, changeOwner); update.setCommit(rw, commit); - update.setPatchSetState(PatchSetState.DRAFT); update.putApproval("Code-Review", (short) 1); update.setChangeMessage("This is a message"); update.putComment( @@ -1077,7 +1076,6 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { update.commit(); ChangeNotes notes = newNotes(c); - assertThat(notes.getPatchSets().get(psId2).isDraft()).isTrue(); assertThat(notes.getPatchSets().keySet()).containsExactly(psId1, psId2); assertThat(notes.getApprovals()).isNotEmpty(); assertThat(notes.getChangeMessagesByPatchSet()).isNotEmpty(); @@ -1089,9 +1087,6 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { update.setPatchSetState(PatchSetState.PUBLISHED); update.commit(); - notes = newNotes(c); - assertThat(notes.getPatchSets().get(psId2).isDraft()).isFalse(); - // delete ps2 update = newUpdate(c, changeOwner); update.setPatchSetState(PatchSetState.DELETED); diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 9d38c2211a..b1e600acaa 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -331,11 +331,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { TestRepository repo = createProject("repo"); ChangeInserter ins1 = newChangeWithStatus(repo, Change.Status.NEW); Change change1 = insert(repo, ins1); - ChangeInserter ins2 = newChangeWithStatus(repo, Change.Status.DRAFT); - Change change2 = insert(repo, ins2); insert(repo, newChangeWithStatus(repo, Change.Status.MERGED)); - Change[] expected = new Change[] {change2, change1}; + Change[] expected = new Change[] {change1}; assertQuery("status:open", expected); assertQuery("status:OPEN", expected); assertQuery("status:o", expected); @@ -349,23 +347,6 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery("is:open", expected); } - @Test - public void byStatusDraft() throws Exception { - TestRepository repo = createProject("repo"); - insert(repo, newChangeWithStatus(repo, Change.Status.NEW)); - ChangeInserter ins2 = newChangeWithStatus(repo, Change.Status.DRAFT); - Change change2 = insert(repo, ins2); - - Change[] expected = new Change[] {change2}; - assertQuery("status:draft", expected); - assertQuery("status:DRAFT", expected); - assertQuery("status:d", expected); - assertQuery("status:dr", expected); - assertQuery("status:dra", expected); - assertQuery("status:draf", expected); - assertQuery("is:draft", expected); - } - @Test public void byStatusClosed() throws Exception { TestRepository repo = createProject("repo"); @@ -1474,37 +1455,6 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery(commit.getId().getName().substring(0, 6), change); } - @Test - public void implicitVisibleTo() throws Exception { - TestRepository repo = createProject("repo"); - Change change1 = insert(repo, newChange(repo), userId); - Change change2 = insert(repo, newChangeWithStatus(repo, Change.Status.DRAFT), userId); - - String q = "project:repo"; - assertQuery(q, change2, change1); - - // Second user cannot see first user's drafts. - requestContext.setContext( - newRequestContext( - accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId())); - assertQuery(q, change1); - } - - @Test - public void explicitVisibleTo() throws Exception { - TestRepository repo = createProject("repo"); - Change change1 = insert(repo, newChange(repo), userId); - Change change2 = insert(repo, newChangeWithStatus(repo, Change.Status.DRAFT), userId); - - String q = "project:repo"; - assertQuery(q, change2, change1); - - // Second user cannot see first user's drafts. - Account.Id user2 = - accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId(); - assertQuery(q + " visibleto:" + user2.get(), change1); - } - @Test public void byCommentBy() throws Exception { TestRepository repo = createProject("repo"); diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java index c46f2cadbc..2a82a26867 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java @@ -131,12 +131,6 @@ public class ReviewCommand extends SshCommand { @Option(name = "--submit", aliases = "-s", usage = "submit the specified patch set(s)") private boolean submitChange; - @Option(name = "--publish", usage = "publish the specified draft patch set(s)") - private boolean publishPatchSet; - - @Option(name = "--delete", usage = "delete the specified draft patch set(s)") - private boolean deleteDraftPatchSet; - @Option(name = "--json", aliases = "-j", usage = "read review input json from stdin") private boolean json; @@ -186,12 +180,6 @@ public class ReviewCommand extends SshCommand { if (submitChange) { throw die("abandon and submit actions are mutually exclusive"); } - if (publishPatchSet) { - throw die("abandon and publish actions are mutually exclusive"); - } - if (deleteDraftPatchSet) { - throw die("abandon and delete actions are mutually exclusive"); - } if (rebaseChange) { throw die("abandon and rebase actions are mutually exclusive"); } @@ -199,17 +187,6 @@ public class ReviewCommand extends SshCommand { throw die("abandon and move actions are mutually exclusive"); } } - if (publishPatchSet) { - if (restoreChange) { - throw die("publish and restore actions are mutually exclusive"); - } - if (submitChange) { - throw die("publish and submit actions are mutually exclusive"); - } - if (deleteDraftPatchSet) { - throw die("publish and delete actions are mutually exclusive"); - } - } if (json) { if (restoreChange) { throw die("json and restore actions are mutually exclusive"); @@ -217,12 +194,6 @@ public class ReviewCommand extends SshCommand { if (submitChange) { throw die("json and submit actions are mutually exclusive"); } - if (deleteDraftPatchSet) { - throw die("json and delete actions are mutually exclusive"); - } - if (publishPatchSet) { - throw die("json and publish actions are mutually exclusive"); - } if (abandonChange) { throw die("json and abandon actions are mutually exclusive"); } @@ -240,16 +211,10 @@ public class ReviewCommand extends SshCommand { } } if (rebaseChange) { - if (deleteDraftPatchSet) { - throw die("rebase and delete actions are mutually exclusive"); - } if (submitChange) { throw die("rebase and submit actions are mutually exclusive"); } } - if (deleteDraftPatchSet && submitChange) { - throw die("delete and submit actions are mutually exclusive"); - } boolean ok = true; ReviewInput input = null; @@ -353,11 +318,6 @@ public class ReviewCommand extends SshCommand { revisionApi(patchSet).submit(); } - if (publishPatchSet) { - revisionApi(patchSet).publish(); - } else if (deleteDraftPatchSet) { - revisionApi(patchSet).delete(); - } } catch (IllegalStateException | RestApiException e) { throw die(e); } diff --git a/plugins/hooks b/plugins/hooks index c52e0e6b5d..4bad0fe13f 160000 --- a/plugins/hooks +++ b/plugins/hooks @@ -1 +1 @@ -Subproject commit c52e0e6b5de088c820038654312711bba9eceaeb +Subproject commit 4bad0fe13f9a75306d552cd57b1866e6dceb54ef