Merge "Suggest parent for 'create-project' in the UI."
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index eded40b..ebe71b2 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -592,9 +592,8 @@
+
Default is 5 minutes.
-
[[changeMerge]]Section changeMerge
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Controls whether or not the mergeability test of changes is
enabled. If enabled, when the change page is loaded, the test is
@@ -606,11 +605,10 @@
test = true
----
-+
By default this is false (test is not enabled).
[[commentlink]]Section commentlink
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Comment links are find/replace strings applied to change descriptions,
patch comments, and in-line code comments to turn set strings into
hyperlinks. One common use is for linking to bug-tracking systems.
@@ -1136,7 +1134,7 @@
Valid values are the characters '*', '(' and ')'.
[[hooks]]Section hooks
-~~~~~~~~~~~~~~~~~~~~~~~~
+~~~~~~~~~~~~~~~~~~~~~~
See also link:config-hooks.html[Hooks].
@@ -2157,7 +2155,7 @@
[[upload]]Section upload
-~~~~~~~~~~~~~~~~~~~~~~~~~~
+~~~~~~~~~~~~~~~~~~~~~~~~
Sets the group of users allowed to execute 'upload-pack' on the
server, 'upload-pack' is what runs on the server during a user's
fetch, clone or repo sync command.
@@ -2206,7 +2204,7 @@
File `etc/secure.config`
--------------------------
+------------------------
The optional file `'$site_path'/etc/secure.config` overrides (or
supplements) the settings supplied by `'$site_path'/etc/gerrit.config`.
The file should be readable only by the daemon process and can be
diff --git a/Documentation/dev-contributing.txt b/Documentation/dev-contributing.txt
new file mode 100644
index 0000000..fd98c1d
--- /dev/null
+++ b/Documentation/dev-contributing.txt
@@ -0,0 +1,241 @@
+Gerrit Code Review - Contributing
+=================================
+
+Gerrit is developed as a self-hosting open source project and
+very much welcomes contributions from anyone with a contributor's
+agreement on file with the project.
+
+* https://gerrit-review.googlesource.com/
+
+The Contributor License Agreements:
+
+* https://gerrit-review.googlesource.com/static/cla_individual.html
+* https://gerrit-review.googlesource.com/static/cla_corporate.html
+
+As Gerrit is a code review tool, naturally contributions will
+be reviewed before they will get submitted to the code base. To
+start your contribution, please make a git commit and upload it
+for review to the main Gerrit review server. To help speed up the
+review of your change, review these guidelines before submitting
+your change. You can view the pending Gerrit contributions and
+their statuses here:
+
+* https://gerrit-review.googlesource.com/#/q/status:open+project:gerrit,n,z
+
+Depending on the size of that list it might take a while for
+your change to get reviewed. Naturally there are fewer
+approvers than contributors; so anything that you can do to
+ensure that your contribution will undergo fewer revisions
+will speed up the contribution process. This includes helping
+out reviewing other people's changes to relieve the load from
+the approvers. Even if you are not familiar with Gerrit's
+internals, it would be of great help if you can download, try
+out, and comment on new features. If it works as advertised,
+say so, and if you have the priviliges to do so, go ahead
+and give it a +1 Verified. If you would find the feature
+useful, say so and give it a +1 code review.
+
+And finally, the quicker you respond to the comments of your
+reviewers, the quicker your change might get merged! Try to
+reply to every comment after submitting your new patch,
+particularly if you decided against making the suggested change.
+Reviewers don't want to seem like nags and pester you if you
+haven't replied or made a fix, so it helps them know if you
+missed it or decided against it.
+
+
+Review Criteria
+---------------
+
+Here are some hints as to what approvers may be looking for
+before approving or submitting changes to the Gerrit project.
+Let's start with the simple nit picky stuff. You are likely
+excited that your code works; help us share your excitement
+by not distracting us with the simple stuff. Thanks to Gerrit,
+problems are often highlighted and we find it hard to look
+beyond simple spacing issues. Blame it on our short attention
+spans, we really do want your code.
+
+
+Commit Message
+--------------
+
+It is essential to have a good commit message if you want your
+change to be reviewed.
+
+ * Keep lines no longer than 72 chars
+ * Start with a short one line summary
+ * Followed by a blank line
+ * Followed by one or more explanatory paragraphs
+ * Use the present tense (fix instead of fixed)
+ * Include a Bug: Issue <#> line if fixing a Gerrit issue
+ * Include a Change-Id line
+
+
+A sample good Gerrit commit message:
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+====
+ Add sample commit message to guidelines doc
+
+ The original patch set for the contributing guidelines doc did not
+ include a sample commit message, this new patchset does. Hopefully this
+ makes things a bit clearer since examples can sometimes help when
+ explanations don't.
+
+ Note that the body of this commit message can be several paragraphs, and
+ that I word wrap it at 72 characters. Also note that I keep the summary
+ line under 50 characters since it is often truncated by tools which
+ display just the git summary.
+
+ Bug: Issue 98765605
+ Change-Id: Ic4a7c07eeb98cdeaf44e9d231a65a51f3fceae52
+====
+
+
+Style
+-----
+
+The basic coding style is covered by the tools/GoogleFormat.xml
+doc, see the link:dev-eclipse.html#Formatting[Eclipse Setup]
+for that.
+
+Highlighted/additional styling notes:
+
+ * It is generally more important to match the style of the nearby
+ code which you are modifying than it is to match the style
+ in the formatting guidelines. This is especially true within the
+ same file.
+ * Review your change in Gerrit to see if it highlights
+ mistakingly deleted/added spaces on lines, trailing spaces.
+ * Line length should be 80 or less, unless the code reads
+ better with something slightly longer. Shorter lines not only
+ help reviewers who may use a tablet to review the code, but future
+ contributors may also like to open several editors side by
+ side while editing new changes.
+ * Use 2 spaces for indent (no tabs)
+ * Use brackets in all ifs, spaces before/after if parens.
+ * Use /** */ style Javadocs for variables.
+
+Additionally, you will notice that most of the newline spacing
+is fairly consistent throughout the code in Gerrit, it helps to
+stick to the blank line conventions. Here are some specific
+examples:
+
+ * Keep a blank line between all class and method declarations.
+ * Do not add blank lines at the beginning or end of class/methods.
+ * Put a blank line between external import sources, but not
+ between internal ones.
+
+
+Code Organization
+-----------------
+
+Do your best to organize classes and methods in a logical way.
+Here are some guidelines that Gerrit uses:
+
+ * Ensure a standard copyright header is included at the top
+ of any new files (copy it from another file, update the year).
+ * Always place loggers first in your class!
+ * Define any static interfaces next in your class.
+ * Define non static interfaces after static interfaces in your
+ class.
+ * Next you should define static types and members.
+ * Finally instance members, then constuctors, and then instance
+ methods.
+ * Some common exceptions are private helper static methods which
+ might appear near the instance methods which they help.
+ * Getters and setters for the same instance field should usually
+ be near each other baring a good reason not to.
+ * If you are using assisted injection, the factory for your class
+ should be before the instance members.
+ * Imports should be mostly aphabetical (uppercase sorts before
+ all lowercase, which means classes come before packages at the
+ same level).
+
+Wow that's a lot! But don't worry, you'll get the habit and most
+of the code is organized this way already; so if you pay attention
+to the class you are editing you will likely pick up on it.
+Naturally new classes are a little harder; you may want to come
+back and consult this section when creating them.
+
+
+Design
+------
+
+Here are some design level ojectives that you should keep in mind
+when coding:
+
+ * ORM entity objects should match exactly one row in the database.
+ * Most client pages should perform only one RPC to load so as to
+ keep latencies down. Exceptions would apply to RPCs which need
+ to load large data sets if splitting them out will help the
+ page load faster. Generally page loads are expected to complete
+ in under 100ms. This will be the case for most operations,
+ unless the data being fetched is not using Gerrit's caching
+ infrastructure. In these slower cases, it is worth considering
+ mitigating this longer load by using a second RPC to fill in
+ this data after the page is displayed (or alternatively it might
+ be worth proposing caching this data).
+ * @Inject should be used on constructors, not on fields. The
+ current exceptions are the ssh commands, these were implemented
+ earlier in Gerrit's development. To stay consistent, new ssh
+ commands should follow this older pattern; but eventually these
+ should get converted to eliminate this exception.
+ * Don't leave repository objects (git or schema) open. A .close()
+ after every open should be placed in a finally{} block.
+ * Don't leave UI components, which can cause new actions to occur,
+ enabled during RPCs which update the DB. This is to prevent
+ people from submitting actions more than once when operating
+ on slow links. If the action buttons are disabled, they cannot
+ be resubmitted and the user can see that Gerrit is still busy.
+ * GWT EventBus is the new way forward.
+
+
+Tests
+-----
+
+ * Tests for new code will greatly help your change get approved.
+
+
+Change Size/Number of Files Touched
+-----------------------------------
+
+And finally, I probably cannot say enough about change sizes.
+Generally, smaller is better, hopefully within reason. Do try to
+keep things which will be confusing on their own together,
+especially if changing one without the other will break something!
+
+ * If a new feature is implemented and it is a larger one, try to
+ identify if it can be split into smaller logical features; when
+ in doubt, err on the smaller side.
+ * Separate bug fixes from feature improvements. The bug fix may
+ be an easy candidate for approval and should not need to wait
+ for new features to be approved. Also, combining the two makes
+ reviewing harder since then there is no clear line between the
+ fix and the feature.
+ * Separate supporting refactoring from feature changes. If your
+ new feature requires some refactoring, it helps to make the
+ refactoring a separate change which your feature change
+ depends on. This way, reviewers can easily review the refactor
+ change as a something that should not alter the current
+ functionality, and feel more confident they can more easily
+ spot errors this way. Of course, it also makes it easier to
+ test and locate later on if an unfortunate error does slip in.
+ Lastly, by not having to see refactoring changes at the same
+ time, it helps reviewers understand how your feature changes
+ the current functionality.
+ * Separate logical features into separate changes. This
+ is often the hardest part. Here is an example: when adding a
+ new ability, make separate changes for the UI and the ssh
+ commands if possible.
+ * Do only what the commit message describes. In other words, things which
+ are not strictly related to the commit message shouldn't be part of
+ a change, even trivial things like externalizing a string somewhere
+ or fixing a typo. This help keep "git blame" more useful in the future
+ and it also makes "git revert" more useful.
+ * Use topic branches to link your separate changes together.
+
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
diff --git a/Documentation/dev-eclipse.txt b/Documentation/dev-eclipse.txt
index 82d0a67..b37223c 100644
--- a/Documentation/dev-eclipse.txt
+++ b/Documentation/dev-eclipse.txt
@@ -16,6 +16,7 @@
http://m2eclipse.codehaus.org/[m2eclipse]
+[[Formatting]]
Code Formatter Settings
-----------------------
diff --git a/Documentation/dev-readme.txt b/Documentation/dev-readme.txt
index 2490695..552b5a8 100644
--- a/Documentation/dev-readme.txt
+++ b/Documentation/dev-readme.txt
@@ -11,7 +11,7 @@
Create a new client workspace:
----
- git clone https://code.google.com/p/gerrit
+ git clone https://gerrit.googlesource.com/gerrit
cd gerrit
----
diff --git a/Documentation/index.txt b/Documentation/index.txt
index b42a35a..5143bf7 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -46,6 +46,7 @@
* link:dev-readme.html[Developer Setup]
* link:dev-eclipse.html[Eclipse Setup]
+* link:dev-contributing.html[Contributing to Gerrit]
* link:dev-design.html[System Design]
* link:i18n-readme.html[i18n Support]
diff --git a/ReleaseNotes/ReleaseNotes-2.2.2.1.txt b/ReleaseNotes/ReleaseNotes-2.2.2.1.txt
new file mode 100644
index 0000000..4613787
--- /dev/null
+++ b/ReleaseNotes/ReleaseNotes-2.2.2.1.txt
@@ -0,0 +1,53 @@
+Release notes for Gerrit 2.2.2.1
+================================
+
+Gerrit 2.2.2.1 is now available:
+
+link:http://code.google.com/p/gerrit/downloads/detail?name=gerrit-2.2.2.1.war[http://code.google.com/p/gerrit/downloads/detail?name=gerrit-2.2.2.1.war]
+
+
+There are no schema changes from 2.2.2. However, if upgrading from
+anything but 2.2.2, follow the upgrade procedure in the 2.2.2
+link:ReleaseNotes-2.2.2.html[ReleaseNotes].
+
+
+Bug Fixes
+---------
+* issue 1139 Fix change state in patch set approval if reviewer is added to
+closed change
++
+For the dummy patch set approval that is created when a reviewer is
+added the cached change state is always open, which is incorrect if a
+reviewer is added to a closed change. As a result the closed change will
+appear in the reviewers dashboard in the 'Review Requests' section and will
+stay there forever. Ensure the correct change state is cached in the dummy
+patch set approval when it is created.
+
+* issue 1171 Fix ownerin and reviewerin searches
++
+Update the ownerin and reviewerin searches to use AccountGroup.UUID as
+required by commit e662fb3d4d7d0ad05791b8d2143ac5ce58117335.
+
+* issue 871 Display hash of the cherry-pick merge in comment
++
+After merging a change via cherry-pick, we add the commit's
+hash to the comment. This was accidentally removed in
+commit 14246de3c0f81c06bba8d4530e6bf00e918c11b0
+
+
+Documentation
+-------------
+* Update top level SUBMITTING_PATCHES
++
+This document is out of date, the URLs are from last August.
+Direct readers to the new server.
+
+* Add contributing guideline document
+
+* Documentation: update version references for 2.2.2
++
+Correct wording and instructions to be sure they match what would
+be observed with the indicated version of gerrit.
+Expand instructions when needed to ensure all commands could be
+executed and were successful.
+Indent commands and output based on a run of the instructions
diff --git a/ReleaseNotes/index.txt b/ReleaseNotes/index.txt
index b6dd4b2..8d223ad 100644
--- a/ReleaseNotes/index.txt
+++ b/ReleaseNotes/index.txt
@@ -5,6 +5,7 @@
Version 2.2.x
-------------
* link:ReleaseNotes-2.2.2.html[2.2.2],
+* link:ReleaseNotes-2.2.2.1.html[2.2.2.1],
* link:ReleaseNotes-2.2.1.html[2.2.1],
* link:ReleaseNotes-2.2.0.html[2.2.0]
diff --git a/SUBMITTING_PATCHES b/SUBMITTING_PATCHES
index b4448620..e766ef1 100644
--- a/SUBMITTING_PATCHES
+++ b/SUBMITTING_PATCHES
@@ -5,7 +5,7 @@
- Make sure all code is under the Apache License, 2.0.
- Publish your changes for review:
- git push ssh://review.source.android.com:29418/tools/gerrit.git HEAD:refs/for/master
+ git push https://gerrit-review.googlesource.com/gerrit HEAD:refs/for/master
Long Version:
@@ -55,23 +55,22 @@
Instead, login to the Gerrit Code Review tool at:
- https://review.source.android.com/
+ https://gerrit-review.googlesource.com/
Ensure you have completed one of the necessary contributor
agreements, providing documentation to the project maintainers that
they have right to redistribute your work under the Apache License:
- https://review.source.android.com/#settings,agreements
+ https://gerrit-review.googlesource.com/#/settings/agreements
-Ensure you have registered one or more SSH public keys, so you can
-push your commits directly over SSH:
+Ensure you have obtained a unique HTTP password to identify yourself:
- https://review.source.android.com/#settings,ssh-keys
+ https://gerrit-review.googlesource.com/#/settings/http-password
-Push your patches over SSH to the review server, possibly through
+Push your patches over HTTPS to the review server, possibly through
a remembered remote to make this easier in the future:
- git config remote.review.url ssh://review.source.android.com:29418/tools/gerrit.git
+ git config remote.review.url https://google-review.googlesource.com/gerrit
git config remote.review.push HEAD:refs/for/master
git push review
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java
index 3744ef0..c9be1d4 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java
@@ -44,8 +44,21 @@
@SignInRequired
void deleteDraft(PatchLineComment.Key key, AsyncCallback<VoidResult> callback);
+ /**
+ * Deletes the specified draft patch set. If the draft patch set is the only
+ * patch set of the change, then also the change gets deleted.
+ *
+ * @param psid ID of the draft patch set that should be deleted
+ * @param callback callback to report the result of the draft patch set
+ * deletion operation; if the draft patch set was successfully deleted
+ * {@link AsyncCallback#onSuccess(Object)} is invoked and the change
+ * details are passed as parameter; if the change gets deleted because
+ * the draft patch set that was deleted was the only patch set in the
+ * change, then <code>null</code> is passed as result to
+ * {@link AsyncCallback#onSuccess(Object)}
+ */
@SignInRequired
- void deleteDraftPatchSet(PatchSet.Id psid, AsyncCallback<VoidResult> callback);
+ void deleteDraftPatchSet(PatchSet.Id psid, AsyncCallback<ChangeDetail> callback);
@SignInRequired
void publishComments(PatchSet.Id psid, String message,
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java
index dc08e07..9378526 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java
@@ -66,11 +66,17 @@
/** Not permitted to publish this draft patch set */
PUBLISH_NOT_PERMITTED,
+ /** Not permitted to delete this draft patch set */
+ DELETE_NOT_PERMITTED,
+
/** Review operation not permitted by rule. */
RULE_ERROR,
/** Review operation invalid because patch set is not a draft. */
NOT_A_DRAFT,
+
+ /** Error writing change to git repository */
+ GIT_ERROR
}
protected Type type;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java
index 45a3315..d3c3f76 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java
@@ -501,7 +501,8 @@
actionsPanel.add(b);
}
- if (changeDetail.canDeleteDraft()) {
+ if (changeDetail.getChange().getStatus() == Change.Status.DRAFT
+ && changeDetail.canDeleteDraft()) {
final Button b = new Button(Util.C.buttonDeleteDraftChange());
b.addClickHandler(new ClickHandler() {
@Override
@@ -613,9 +614,13 @@
public void onClick(final ClickEvent event) {
b.setEnabled(false);
PatchUtil.DETAIL_SVC.deleteDraftPatchSet(patchSet.getId(),
- new GerritCallback<VoidResult>() {
- public void onSuccess(VoidResult result) {
- Gerrit.display(PageLinks.MINE);
+ new GerritCallback<ChangeDetail>() {
+ public void onSuccess(final ChangeDetail result) {
+ if (result != null) {
+ changeScreen.update(result);
+ } else {
+ Gerrit.display(PageLinks.MINE);
+ }
}
@Override
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
index a1dbc2f..d226fd0 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
@@ -124,7 +124,7 @@
detail.setCanAbandon(change.getStatus() != Change.Status.DRAFT && change.getStatus().isOpen() && control.canAbandon());
detail.setCanPublish(control.canPublish(db));
detail.setCanRestore(change.getStatus() == Change.Status.ABANDONED && control.canRestore());
- detail.setCanDeleteDraft(control.canDelete(db));
+ detail.setCanDeleteDraft(control.canDeleteDraft(db));
detail.setStarred(control.getCurrentUser().getStarredChanges().contains(
changeId));
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/DeleteDraftChange.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/DeleteDraftChange.java
index 70aca83..ab809ce 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/DeleteDraftChange.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/DeleteDraftChange.java
@@ -61,7 +61,7 @@
final Change.Id changeId = patchSetId.getParentKey();
final ChangeControl control = changeControlFactory.validateFor(changeId);
- if (!control.canDelete(db)) {
+ if (!control.canDeleteDraft(db)) {
throw new NoSuchChangeException(changeId);
}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java
index 6e0e1af..50d6cea 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java
@@ -14,7 +14,9 @@
package com.google.gerrit.httpd.rpc.patch;
+import com.google.gerrit.common.data.ChangeDetail;
import com.google.gerrit.common.data.ReviewerResult;
+import com.google.gerrit.common.data.ReviewResult;
import com.google.gerrit.common.data.ApprovalSummary;
import com.google.gerrit.common.data.ApprovalSummarySet;
import com.google.gerrit.common.data.ApprovalTypes;
@@ -23,6 +25,7 @@
import com.google.gerrit.common.errors.NoSuchEntityException;
import com.google.gerrit.httpd.rpc.BaseServiceImplementation;
import com.google.gerrit.httpd.rpc.Handler;
+import com.google.gerrit.httpd.rpc.changedetail.ChangeDetailFactory;
import com.google.gerrit.reviewdb.Account;
import com.google.gerrit.reviewdb.AccountDiffPreference;
import com.google.gerrit.reviewdb.AccountPatchReview;
@@ -38,6 +41,7 @@
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AccountInfoCacheFactory;
+import com.google.gerrit.server.changedetail.DeleteDraftPatchSet;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.ReplicationQueue;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
@@ -52,7 +56,6 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
-import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@@ -66,6 +69,7 @@
private final AccountInfoCacheFactory.Factory accountInfoCacheFactory;
private final AddReviewerHandler.Factory addReviewerHandlerFactory;
private final ChangeControl.Factory changeControlFactory;
+ private final DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory;
private final RemoveReviewerHandler.Factory removeReviewerHandlerFactory;
private final FunctionState.Factory functionStateFactory;
private final PublishComments.Factory publishCommentsFactory;
@@ -74,6 +78,7 @@
private final PatchSetInfoFactory patchSetInfoFactory;
private final GitRepositoryManager gitManager;
private final ReplicationQueue replication;
+ private final ChangeDetailFactory.Factory changeDetailFactory;
@Inject
PatchDetailServiceImpl(final Provider<ReviewDb> schema,
@@ -83,13 +88,15 @@
final AddReviewerHandler.Factory addReviewerHandlerFactory,
final RemoveReviewerHandler.Factory removeReviewerHandlerFactory,
final ChangeControl.Factory changeControlFactory,
+ final DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory,
final FunctionState.Factory functionStateFactory,
final PatchScriptFactory.Factory patchScriptFactoryFactory,
final PublishComments.Factory publishCommentsFactory,
final SaveDraft.Factory saveDraftFactory,
final PatchSetInfoFactory patchSetInfoFactory,
final GitRepositoryManager gitManager,
- final ReplicationQueue replication) {
+ final ReplicationQueue replication,
+ final ChangeDetailFactory.Factory changeDetailFactory) {
super(schema, currentUser);
this.approvalTypes = approvalTypes;
@@ -97,6 +104,7 @@
this.addReviewerHandlerFactory = addReviewerHandlerFactory;
this.removeReviewerHandlerFactory = removeReviewerHandlerFactory;
this.changeControlFactory = changeControlFactory;
+ this.deleteDraftPatchSetFactory = deleteDraftPatchSetFactory;
this.functionStateFactory = functionStateFactory;
this.patchScriptFactoryFactory = patchScriptFactoryFactory;
this.publishCommentsFactory = publishCommentsFactory;
@@ -104,6 +112,7 @@
this.patchSetInfoFactory = patchSetInfoFactory;
this.gitManager = gitManager;
this.replication = replication;
+ this.changeDetailFactory = changeDetailFactory;
}
public void patchScript(final Patch.Key patchKey, final PatchSet.Id psa,
@@ -149,23 +158,28 @@
}
public void deleteDraftPatchSet(final PatchSet.Id psid,
- final AsyncCallback<VoidResult> callback) {
- run(callback, new Action<VoidResult>() {
- public VoidResult run(ReviewDb db) throws OrmException, Failure {
+ final AsyncCallback<ChangeDetail> callback) {
+ run(callback, new Action<ChangeDetail>() {
+ public ChangeDetail run(ReviewDb db) throws OrmException, Failure {
+ ReviewResult result = null;
try {
- final ChangeControl cc = changeControlFactory.validateFor(psid.getParentKey());
- if (!cc.isOwner()) {
+ result = deleteDraftPatchSetFactory.create(psid).call();
+ if (result.getErrors().size() > 0) {
throw new Failure(new NoSuchEntityException());
}
- ChangeUtil.deleteDraftPatchSet(psid, gitManager, replication, patchSetInfoFactory, db);
+ if (result.getChangeId() == null) {
+ // the change was deleted because the draft patch set that was
+ // deleted was the only patch set in the change
+ return null;
+ }
+ return changeDetailFactory.create(result.getChangeId()).call();
} catch (NoSuchChangeException e) {
- throw new Failure(new NoSuchChangeException(psid.getParentKey()));
+ throw new Failure(new NoSuchChangeException(result.getChangeId()));
+ } catch (NoSuchEntityException e) {
+ throw new Failure(e);
} catch (PatchSetInfoNotAvailableException e) {
throw new Failure(e);
- } catch (IOException e) {
- throw new Failure(e);
}
- return VoidResult.INSTANCE;
}
});
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
index 0c17b25..55de507 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
@@ -286,37 +286,7 @@
db.changes().delete(Collections.singleton(change));
}
- public static void deleteDraftPatchSet(final PatchSet.Id patchSetId,
- GitRepositoryManager gitManager,
- final ReplicationQueue replication,
- final PatchSetInfoFactory patchSetInfoFactory,
- final ReviewDb db) throws NoSuchChangeException, OrmException,
- PatchSetInfoNotAvailableException, IOException {
- final Change.Id changeId = patchSetId.getParentKey();
- final Change change = db.changes().get(changeId);
- final PatchSet patch = db.patchSets().get(patchSetId);
-
- deleteOnlyDraftPatchSet(patch, change, gitManager, replication, db);
-
- List<PatchSet> restOfPatches = db.patchSets().byChange(changeId).toList();
- if (restOfPatches.size() == 0) {
- deleteDraftChange(patchSetId, gitManager, replication, db);
- } else {
- PatchSet.Id highestId = null;
- for (PatchSet ps : restOfPatches) {
- if (highestId == null || ps.getPatchSetId() > highestId.get()) {
- highestId = ps.getId();
- }
- }
- if (change.currentPatchSetId().equals(patchSetId)) {
- change.removeLastPatchSetId();
- change.setCurrentPatchSet(patchSetInfoFactory.get(db, change.currPatchSetId()));
- db.changes().update(Collections.singleton(change));
- }
- }
- }
-
- private static void deleteOnlyDraftPatchSet(final PatchSet patch,
+ public static void deleteOnlyDraftPatchSet(final PatchSet patch,
final Change change, GitRepositoryManager gitManager,
final ReplicationQueue replication, final ReviewDb db)
throws NoSuchChangeException, OrmException, IOException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/DeleteDraftPatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/DeleteDraftPatchSet.java
new file mode 100644
index 0000000..761b765
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/DeleteDraftPatchSet.java
@@ -0,0 +1,125 @@
+// Copyright (C) 2011 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.changedetail;
+
+import com.google.gerrit.common.data.ReviewResult;
+import com.google.gerrit.reviewdb.Change;
+import com.google.gerrit.reviewdb.PatchSet;
+import com.google.gerrit.reviewdb.ReviewDb;
+import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.ReplicationQueue;
+import com.google.gerrit.server.patch.PatchSetInfoFactory;
+import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.project.ChangeControl;
+import com.google.gwtorm.client.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.Callable;
+
+public class DeleteDraftPatchSet implements Callable<ReviewResult> {
+
+ public interface Factory {
+ DeleteDraftPatchSet create(PatchSet.Id patchSetId);
+ }
+
+ private final ChangeControl.Factory changeControlFactory;
+ private final ReviewDb db;
+ private final GitRepositoryManager gitManager;
+ private final ReplicationQueue replication;
+ private final PatchSetInfoFactory patchSetInfoFactory;
+
+ private final PatchSet.Id patchSetId;
+
+ @Inject
+ DeleteDraftPatchSet(ChangeControl.Factory changeControlFactory,
+ ReviewDb db, GitRepositoryManager gitManager,
+ ReplicationQueue replication, PatchSetInfoFactory patchSetInfoFactory,
+ @Assisted final PatchSet.Id patchSetId) {
+ this.changeControlFactory = changeControlFactory;
+ this.db = db;
+ this.gitManager = gitManager;
+ this.replication = replication;
+ this.patchSetInfoFactory = patchSetInfoFactory;
+
+ this.patchSetId = patchSetId;
+ }
+
+ @Override
+ public ReviewResult call() throws NoSuchChangeException, OrmException {
+ final ReviewResult result = new ReviewResult();
+
+ final Change.Id changeId = patchSetId.getParentKey();
+ result.setChangeId(changeId);
+ final ChangeControl control = changeControlFactory.validateFor(changeId);
+ final PatchSet patch = db.patchSets().get(patchSetId);
+ if (patch == null) {
+ throw new NoSuchChangeException(changeId);
+ }
+ if (!patch.isDraft()) {
+ result.addError(new ReviewResult.Error(
+ ReviewResult.Error.Type.NOT_A_DRAFT));
+ return result;
+ }
+
+ if (!control.canDeleteDraft(db)) {
+ result.addError(new ReviewResult.Error(
+ ReviewResult.Error.Type.DELETE_NOT_PERMITTED));
+ return result;
+ }
+ final Change change = control.getChange();
+
+ try {
+ ChangeUtil.deleteOnlyDraftPatchSet(patch, change, gitManager, replication, db);
+ } catch (IOException e) {
+ result.addError(new ReviewResult.Error(
+ ReviewResult.Error.Type.GIT_ERROR, e.getMessage()));
+ }
+
+ List<PatchSet> restOfPatches = db.patchSets().byChange(changeId).toList();
+ if (restOfPatches.size() == 0) {
+ try {
+ ChangeUtil.deleteDraftChange(patchSetId, gitManager, replication, db);
+ result.setChangeId(null);
+ } catch (IOException e) {
+ result.addError(new ReviewResult.Error(
+ ReviewResult.Error.Type.GIT_ERROR, e.getMessage()));
+ }
+ } else {
+ PatchSet.Id highestId = null;
+ for (PatchSet ps : restOfPatches) {
+ if (highestId == null || ps.getPatchSetId() > highestId.get()) {
+ highestId = ps.getId();
+ }
+ }
+ if (change.currentPatchSetId().equals(patchSetId)) {
+ change.removeLastPatchSetId();
+ try {
+ change.setCurrentPatchSet(patchSetInfoFactory.get(db, change.currPatchSetId()));
+ } catch (PatchSetInfoNotAvailableException e) {
+ throw new NoSuchChangeException(changeId);
+ }
+ db.changes().update(Collections.singleton(change));
+ }
+ }
+ return result;
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java
index 2bc6d5f..54445c5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java
@@ -28,6 +28,7 @@
import com.google.gerrit.server.account.PerformRenameGroup;
import com.google.gerrit.server.account.VisibleGroups;
import com.google.gerrit.server.changedetail.AbandonChange;
+import com.google.gerrit.server.changedetail.DeleteDraftPatchSet;
import com.google.gerrit.server.changedetail.PublishDraft;
import com.google.gerrit.server.changedetail.RestoreChange;
import com.google.gerrit.server.changedetail.Submit;
@@ -88,6 +89,7 @@
factory(AddReviewer.Factory.class);
factory(AddReviewerSender.Factory.class);
factory(CreateChangeSender.Factory.class);
+ factory(DeleteDraftPatchSet.Factory.class);
factory(PublishComments.Factory.class);
factory(PublishDraft.Factory.class);
factory(ReplacePatchSetSender.Factory.class);
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 26e9a98..073372c 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
@@ -189,16 +189,14 @@
;
}
- /** Can this user publish this change? */
+ /** Can this user publish this draft change or any draft patch set of this change? */
public boolean canPublish(final ReviewDb db) throws OrmException {
- return change.getStatus() == Change.Status.DRAFT && isOwner()
- && isVisible(db);
+ return isOwner() && isVisible(db);
}
- /** Can this user delete this change? */
- public boolean canDelete(final ReviewDb db) throws OrmException {
- return change.getStatus() == Change.Status.DRAFT && isOwner()
- && isVisible(db);
+ /** Can this user delete this draft change or any draft patch set of this change? */
+ public boolean canDeleteDraft(final ReviewDb db) throws OrmException {
+ return isOwner() && isVisible(db);
}
/** Can this user restore this change? */
diff --git a/gerrit-server/src/main/prolog/gerrit_common.pl b/gerrit-server/src/main/prolog/gerrit_common.pl
index 634f8ea..3313162 100644
--- a/gerrit-server/src/main/prolog/gerrit_common.pl
+++ b/gerrit-server/src/main/prolog/gerrit_common.pl
@@ -198,8 +198,8 @@
%%
legacy_submit_rule('MaxWithBlock', Label, Id, Min, Max, T) :- !, max_with_block(Label, Min, Max, T).
legacy_submit_rule('MaxNoBlock', Label, Id, Min, Max, T) :- !, max_no_block(Label, Max, T).
-legacy_submit_rule('NoBlock', Label, Id, Min, Max, T) :- T = ok(_), true.
-legacy_submit_rule('NoOp', Label, Id, Min, Max, T) :- T = ok(_), true.
+legacy_submit_rule('NoBlock', Label, Id, Min, Max, T) :- !, T = ok(_).
+legacy_submit_rule('NoOp', Label, Id, Min, Max, T) :- !, T = ok(_).
legacy_submit_rule(Fun, Label, Id, Min, Max, T) :- T = impossible(unsupported(Fun)).
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ProjectNode.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ProjectNode.java
index 47d9512..8e12571 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ProjectNode.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ProjectNode.java
@@ -16,6 +16,7 @@
import com.google.gerrit.reviewdb.Project;
import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.sshd.commands.TreeFormatter.TreeNode;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
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 81f760f..8eb6058 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
@@ -24,17 +24,13 @@
import com.google.gerrit.reviewdb.PatchSetApproval;
import com.google.gerrit.reviewdb.RevId;
import com.google.gerrit.reviewdb.ReviewDb;
-import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.changedetail.AbandonChange;
+import com.google.gerrit.server.changedetail.DeleteDraftPatchSet;
import com.google.gerrit.server.changedetail.PublishDraft;
import com.google.gerrit.server.changedetail.RestoreChange;
import com.google.gerrit.server.changedetail.Submit;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.ReplicationQueue;
import com.google.gerrit.server.mail.EmailException;
-import com.google.gerrit.server.patch.PatchSetInfoFactory;
-import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.patch.PublishComments;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
@@ -124,6 +120,9 @@
private ChangeControl.Factory changeControlFactory;
@Inject
+ private DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory;
+
+ @Inject
private AbandonChange.Factory abandonChangeFactory;
@Inject
@@ -139,17 +138,8 @@
private RestoreChange.Factory restoreChangeFactory;
@Inject
- private GitRepositoryManager gitManager;
-
- @Inject
- private ReplicationQueue replication;
-
- @Inject
private Submit.Factory submitFactory;
- @Inject
- private PatchSetInfoFactory patchSetInfoFactory;
-
private List<ApproveOption> optionList;
@Override
@@ -192,6 +182,9 @@
} catch (UnloggedFailure e) {
ok = false;
writeError("error: " + e.getMessage() + "\n");
+ } catch (NoSuchChangeException e) {
+ ok = false;
+ writeError("no such change " + patchSetId.getParentKey().get());
} catch (Exception e) {
ok = false;
writeError("fatal: internal server error while approving "
@@ -233,16 +226,16 @@
publishCommentsFactory.create(patchSetId, changeComment, aps, forceMessage).call();
if (abandonChange) {
- ReviewResult result = abandonChangeFactory.create(
+ final ReviewResult result = abandonChangeFactory.create(
patchSetId, changeComment).call();
handleReviewResultErrors(result);
} else if (restoreChange) {
- ReviewResult result = restoreChangeFactory.create(
+ final ReviewResult result = restoreChangeFactory.create(
patchSetId, changeComment).call();
handleReviewResultErrors(result);
}
if (submitChange) {
- ReviewResult result = submitFactory.create(patchSetId).call();
+ final ReviewResult result = submitFactory.create(patchSetId).call();
handleReviewResultErrors(result);
}
} catch (InvalidChangeOperationException e) {
@@ -252,20 +245,12 @@
}
if (publishPatchSet) {
- ReviewResult result = publishDraftFactory.create(patchSetId).call();
+ final ReviewResult result = publishDraftFactory.create(patchSetId).call();
handleReviewResultErrors(result);
} else if (deleteDraftPatchSet) {
- if (changeControl.canDelete(db)) {
- try {
- ChangeUtil.deleteDraftPatchSet(patchSetId, gitManager, replication, patchSetInfoFactory, db);
- } catch (PatchSetInfoNotAvailableException e) {
- throw error("Error retrieving draft patchset: " + patchSetId);
- } catch (IOException e) {
- throw error("Error deleting draft patchset: " + patchSetId);
- }
- } else {
- throw error("Not permitted to delete draft patchset");
- }
+ final ReviewResult result =
+ deleteDraftPatchSetFactory.create(patchSetId).call();
+ handleReviewResultErrors(result);
}
}
@@ -291,12 +276,18 @@
case PUBLISH_NOT_PERMITTED:
errMsg += "not permitted to publish change";
break;
+ case DELETE_NOT_PERMITTED:
+ errMsg += "not permitted to delete change/patch set";
+ break;
case RULE_ERROR:
errMsg += "rule error";
break;
case NOT_A_DRAFT:
errMsg += "change is not a draft";
break;
+ case GIT_ERROR:
+ errMsg += "error writing change to git repository";
+ break;
default:
errMsg += "failure in review";
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TreeFormatter.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TreeFormatter.java
index f8b518e..3aa83c3 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TreeFormatter.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TreeFormatter.java
@@ -19,6 +19,12 @@
public class TreeFormatter {
+ public static interface TreeNode {
+ public String getDisplayName();
+ public boolean isVisible();
+ public SortedSet<? extends TreeNode> getChildren();
+ }
+
public static final String NOT_VISIBLE_NODE = "(x)";
private static final String NODE_PREFIX = "|-- ";
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TreeNode.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TreeNode.java
deleted file mode 100644
index 8b7d3a9..0000000
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TreeNode.java
+++ /dev/null
@@ -1,23 +0,0 @@
-// Copyright (C) 2011 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.sshd.commands;
-
-import java.util.SortedSet;
-
-public interface TreeNode {
- public String getDisplayName();
- public boolean isVisible();
- public SortedSet<? extends TreeNode> getChildren();
-}
diff --git a/pom.xml b/pom.xml
index cbe2dc1..98da773 100644
--- a/pom.xml
+++ b/pom.xml
@@ -47,7 +47,7 @@
<properties>
<jgitVersion>1.1.0.201109151100-r.141-gcd958ba</jgitVersion>
- <gwtormVersion>1.2-SNAPSHOT</gwtormVersion>
+ <gwtormVersion>1.2</gwtormVersion>
<gwtjsonrpcVersion>1.2.5</gwtjsonrpcVersion>
<gwtexpuiVersion>1.2.5</gwtexpuiVersion>
<gwtVersion>2.3.0</gwtVersion>
