Merge "Replace PatchDetailService saveDraft and deleteDraft"

2013-12-12 20:09:46 +00:00
parent 43f029e7fe b74e0e34b3
commit ec51eba12b
7 changed files with 83 additions and 223 deletions
--- 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
@@ -15,29 +15,17 @@
 package com.google.gerrit.common.data;

 import com.google.gerrit.common.audit.Audit;
-import com.google.gerrit.common.auth.SignInRequired;
 import com.google.gerrit.reviewdb.client.AccountDiffPreference;
 import com.google.gerrit.reviewdb.client.Patch;
-import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gwtjsonrpc.common.AsyncCallback;
 import com.google.gwtjsonrpc.common.RemoteJsonService;
 import com.google.gwtjsonrpc.common.RpcImpl;
 import com.google.gwtjsonrpc.common.RpcImpl.Version;
-import com.google.gwtjsonrpc.common.VoidResult;

@RpcImpl(version = Version.V2_0)
 public interface PatchDetailService extends RemoteJsonService {
  @Audit
  void patchScript(Patch.Key key, PatchSet.Id a, PatchSet.Id b,
      AccountDiffPreference diffPrefs, AsyncCallback<PatchScript> callback);
-
-  @Audit
-  @SignInRequired
-  void saveDraft(PatchLineComment comment,
-      AsyncCallback<PatchLineComment> callback);
-
-  @Audit
-  @SignInRequired
-  void deleteDraft(PatchLineComment.Key key, AsyncCallback<VoidResult> callback);
 }
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
@@ -34,7 +34,6 @@ import com.google.gerrit.client.ui.PatchLink;
 import com.google.gerrit.client.ui.SmallHeading;
 import com.google.gerrit.common.PageLinks;
 import com.google.gerrit.common.changes.ListChangesOption;
-import com.google.gerrit.common.changes.Side;
 import com.google.gerrit.common.data.ChangeDetail;
 import com.google.gerrit.common.data.SubmitTypeRecord;
 import com.google.gerrit.reviewdb.client.Change;
@@ -485,26 +484,12 @@ public class PublishCommentScreen extends AccountScreen implements
    for (String path : paths) {
      JsArray<CommentInfo> comments = drafts.get(path);
      for (int i = 0; i < comments.length(); i++) {
-        d.add(toComment(path, comments.get(i)));
+        d.add(CommentEditorPanel.toComment(patchSetId, path, comments.get(i)));
      }
    }
    return d;
  }

-  private PatchLineComment toComment(String path, CommentInfo i) {
-    PatchLineComment p = new PatchLineComment(
-        new PatchLineComment.Key(
-            new Patch.Key(patchSetId, path),
-            i.id()),
-        i.line(),
-        Gerrit.getUserAccount().getId(),
-        i.in_reply_to(),
-        i.updated());
-    p.setMessage(i.message());
-    p.setSide((short) (i.side() == Side.PARENT ? 0 : 1));
-    return p;
-  }
-
  private static class ValueRadioButton extends RadioButton {
    final LabelInfo label;
    final String value;
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java
@@ -18,6 +18,9 @@ import com.google.gerrit.client.Dispatcher;
 import com.google.gerrit.client.FormatUtil;
 import com.google.gerrit.client.Gerrit;
 import com.google.gerrit.client.account.AccountInfo;
+import com.google.gerrit.client.changes.CommentApi;
+import com.google.gerrit.client.changes.CommentInfo;
+import com.google.gerrit.client.changes.CommentInput;
 import com.google.gerrit.client.changes.PatchTable;
 import com.google.gerrit.client.changes.Util;
 import com.google.gerrit.client.rpc.GerritCallback;
@@ -937,13 +940,17 @@ public abstract class AbstractPatchContentTable extends NavigationTable<Object>
      CommentEditorPanel p = findOrCreateEditor(newComment, false);
      if (p == null) {
        enableButtons(false);
-        PatchUtil.DETAIL_SVC.saveDraft(newComment,
-            new GerritCallback<PatchLineComment>() {
+        final PatchSet.Id psId = newComment.getKey().getParentKey().getParentKey();
+        CommentInput in = CommentEditorPanel.toInput(newComment);
+        CommentApi.createDraft(psId, in,
+            new GerritCallback<CommentInfo>() {
              @Override
-              public void onSuccess(final PatchLineComment result) {
+              public void onSuccess(CommentInfo result) {
                enableButtons(true);
                notifyDraftDelta(1);
-                findOrCreateEditor(result, true).setOpen(false);
+                findOrCreateEditor(CommentEditorPanel.toComment(
+                    psId, newComment.getKey().getParentKey().get(), result),
+                  true).setOpen(false);
              }

              @Override
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorPanel.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorPanel.java
@@ -15,10 +15,17 @@
 package com.google.gerrit.client.patches;

 import com.google.gerrit.client.Gerrit;
+import com.google.gerrit.client.changes.CommentApi;
+import com.google.gerrit.client.changes.CommentInfo;
+import com.google.gerrit.client.changes.CommentInput;
 import com.google.gerrit.client.rpc.GerritCallback;
 import com.google.gerrit.client.ui.CommentLinkProcessor;
 import com.google.gerrit.client.ui.CommentPanel;
+import com.google.gerrit.common.changes.Side;
+import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.event.dom.client.ClickEvent;
 import com.google.gwt.event.dom.client.ClickHandler;
 import com.google.gwt.event.dom.client.DoubleClickEvent;
@@ -236,30 +243,37 @@ public class CommentEditorPanel extends CommentPanel implements ClickHandler,
    cancel.setEnabled(false);
    discard.setEnabled(false);

-    PatchUtil.DETAIL_SVC.saveDraft(comment,
-        new GerritCallback<PatchLineComment>() {
-          public void onSuccess(final PatchLineComment result) {
-            notifyDraftDelta(isNew() ? 1 : 0);
-            comment = result;
-            text.setReadOnly(false);
-            save.setEnabled(true);
-            cancel.setEnabled(true);
-            discard.setEnabled(true);
-            render();
-            onSave.onSuccess(VoidResult.INSTANCE);
-          }
+    final PatchSet.Id psId = comment.getKey().getParentKey().getParentKey();
+    final boolean wasNew = isNew();
+    GerritCallback<CommentInfo> cb = new GerritCallback<CommentInfo>() {
+      public void onSuccess(CommentInfo result) {
+        notifyDraftDelta(wasNew ? 1 : 0);
+        comment = toComment(psId, comment.getKey().get(), result);
+        text.setReadOnly(false);
+        save.setEnabled(true);
+        cancel.setEnabled(true);
+        discard.setEnabled(true);
+        render();
+        onSave.onSuccess(VoidResult.INSTANCE);
+      }

-          @Override
-          public void onFailure(final Throwable caught) {
-            text.setReadOnly(false);
-            text.setFocus(true);
-            save.setEnabled(true);
-            cancel.setEnabled(true);
-            discard.setEnabled(true);
-            super.onFailure(caught);
-            onSave.onFailure(caught);
-          }
-        });
+      @Override
+      public void onFailure(final Throwable caught) {
+        text.setReadOnly(false);
+        text.setFocus(true);
+        save.setEnabled(true);
+        cancel.setEnabled(true);
+        discard.setEnabled(true);
+        super.onFailure(caught);
+        onSave.onFailure(caught);
+      }
+    };
+    CommentInput input = toInput(comment);
+    if (wasNew) {
+      CommentApi.createDraft(psId, input, cb);
+    } else {
+      CommentApi.updateDraft(psId, input.id(), input, cb);
+    }
  }

  private void notifyDraftDelta(final int delta) {
@@ -283,9 +297,11 @@ public class CommentEditorPanel extends CommentPanel implements ClickHandler,
    cancel.setEnabled(false);
    discard.setEnabled(false);

-    PatchUtil.DETAIL_SVC.deleteDraft(comment.getKey(),
-        new GerritCallback<VoidResult>() {
-          public void onSuccess(final VoidResult result) {
+    CommentApi.deleteDraft(
+        comment.getKey().getParentKey().getParentKey(),
+        comment.getKey().get(),
+        new GerritCallback<JavaScriptObject>() {
+          public void onSuccess(JavaScriptObject result) {
            notifyDraftDelta(-1);
            removeUI();
          }
@@ -319,4 +335,33 @@ public class CommentEditorPanel extends CommentPanel implements ClickHandler,
    }
    return null;
  }
+
+  public static CommentInput toInput(PatchLineComment c) {
+    CommentInput i = CommentInput.createObject().cast();
+    i.setId(c.getKey().get());
+    i.setPath(c.getKey().getParentKey().get());
+    i.setSide(c.getSide() == 0 ? Side.PARENT : Side.REVISION);
+    if (c.getLine() > 0) {
+      i.setLine(c.getLine());
+    }
+    i.setInReplyTo(c.getParentUuid());
+    i.setMessage(c.getMessage());
+    return i;
+  }
+
+  public static PatchLineComment toComment(PatchSet.Id ps,
+      String path,
+      CommentInfo i) {
+    PatchLineComment p = new PatchLineComment(
+        new PatchLineComment.Key(
+            new Patch.Key(ps, path),
+            i.id()),
+        i.line(),
+        Gerrit.getUserAccount().getId(),
+        i.in_reply_to(),
+        i.updated());
+    p.setMessage(i.message());
+    p.setSide((short) (i.side() == Side.PARENT ? 0 : 1));
+    return p;
+  }
 }
--- 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
@@ -22,36 +22,28 @@ import com.google.gerrit.httpd.rpc.Handler;
 import com.google.gerrit.reviewdb.client.AccountDiffPreference;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Patch;
-import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.patch.PatchScriptFactory;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gwtjsonrpc.common.AsyncCallback;
-import com.google.gwtjsonrpc.common.VoidResult;
-import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;

-import java.util.Collections;
-
 class PatchDetailServiceImpl extends BaseServiceImplementation implements
    PatchDetailService {
  private final PatchScriptFactory.Factory patchScriptFactoryFactory;
-  private final SaveDraft.Factory saveDraftFactory;
  private final ChangeControl.Factory changeControlFactory;

  @Inject
  PatchDetailServiceImpl(final Provider<ReviewDb> schema,
      final Provider<CurrentUser> currentUser,
      final PatchScriptFactory.Factory patchScriptFactoryFactory,
-      final SaveDraft.Factory saveDraftFactory,
      final ChangeControl.Factory changeControlFactory) {
    super(schema, currentUser);

    this.patchScriptFactoryFactory = patchScriptFactoryFactory;
-    this.saveDraftFactory = saveDraftFactory;
    this.changeControlFactory = changeControlFactory;
  }

@@ -73,36 +65,4 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
      }
    }.to(callback);
  }
-
-  public void saveDraft(final PatchLineComment comment,
-      final AsyncCallback<PatchLineComment> callback) {
-    saveDraftFactory.create(comment).to(callback);
-  }
-
-  public void deleteDraft(final PatchLineComment.Key commentKey,
-      final AsyncCallback<VoidResult> callback) {
-    run(callback, new Action<VoidResult>() {
-      public VoidResult run(ReviewDb db) throws OrmException, Failure {
-        Change.Id id = commentKey.getParentKey().getParentKey().getParentKey();
-        db.changes().beginTransaction(id);
-        try {
-          final PatchLineComment comment = db.patchComments().get(commentKey);
-          if (comment == null) {
-            throw new Failure(new NoSuchEntityException());
-          }
-          if (!getAccountId().equals(comment.getAuthor())) {
-            throw new Failure(new NoSuchEntityException());
-          }
-          if (comment.getStatus() != PatchLineComment.Status.DRAFT) {
-            throw new Failure(new IllegalStateException("Comment published"));
-          }
-          db.patchComments().delete(Collections.singleton(comment));
-          db.commit();
-          return VoidResult.INSTANCE;
-        } finally {
-          db.rollback();
-        }
-      }
-    });
-  }
 }
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchModule.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchModule.java
@@ -16,7 +16,6 @@ package com.google.gerrit.httpd.rpc.patch;

 import com.google.gerrit.httpd.rpc.RpcServletModule;
 import com.google.gerrit.httpd.rpc.UiRpcModule;
-import com.google.gerrit.server.config.FactoryModule;

 public class PatchModule extends RpcServletModule {
  public PatchModule() {
@@ -25,12 +24,6 @@ public class PatchModule extends RpcServletModule {

  @Override
  protected void configureServlets() {
-    install(new FactoryModule() {
-      @Override
-      protected void configure() {
-        factory(SaveDraft.Factory.class);
-      }
-    });
    rpc(PatchDetailServiceImpl.class);
  }
 }
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/SaveDraft.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/SaveDraft.java
@@ -1,118 +0,0 @@
-// Copyright (C) 2009 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.httpd.rpc.patch;
-
-import com.google.gerrit.httpd.rpc.Handler;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Patch;
-import com.google.gerrit.reviewdb.client.PatchLineComment;
-import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.ChangeUtil;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.project.ChangeControl;
-import com.google.gerrit.server.project.NoSuchChangeException;
-import com.google.gerrit.server.util.TimeUtil;
-import com.google.gwtorm.server.OrmException;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
-
-import java.util.Collections;
-
-class SaveDraft extends Handler<PatchLineComment> {
-  interface Factory {
-    SaveDraft create(PatchLineComment comment);
-  }
-
-  private final ChangeControl.Factory changeControlFactory;
-  private final ReviewDb db;
-  private final IdentifiedUser currentUser;
-
-  private final PatchLineComment comment;
-
-  @Inject
-  SaveDraft(final ChangeControl.Factory changeControlFactory,
-      final ReviewDb db, final IdentifiedUser currentUser,
-      @Assisted final PatchLineComment comment) {
-    this.changeControlFactory = changeControlFactory;
-    this.db = db;
-    this.currentUser = currentUser;
-    this.comment = comment;
-  }
-
-  @Override
-  public PatchLineComment call() throws NoSuchChangeException, OrmException {
-    if (comment.getStatus() != PatchLineComment.Status.DRAFT) {
-      throw new IllegalStateException("Comment published");
-    }
-
-    final Patch.Key patchKey = comment.getKey().getParentKey();
-    final PatchSet.Id patchSetId = patchKey.getParentKey();
-    final Change.Id changeId = patchKey.getParentKey().getParentKey();
-
-    db.changes().beginTransaction(changeId);
-    try {
-      changeControlFactory.validateFor(changeId);
-      if (db.patchSets().get(patchSetId) == null) {
-        throw new NoSuchChangeException(changeId);
-      }
-
-      final Account.Id me = currentUser.getAccountId();
-      if (comment.getKey().get() == null) {
-        if (comment.getLine() < 0) {
-          throw new IllegalStateException("Comment line must be >= 0, not "
-              + comment.getLine());
-        }
-
-        if (comment.getParentUuid() != null) {
-          final PatchLineComment parent =
-              db.patchComments().get(
-                  new PatchLineComment.Key(patchKey, comment.getParentUuid()));
-          if (parent == null || parent.getSide() != comment.getSide()) {
-            throw new IllegalStateException("Parent comment must be on same side");
-          }
-        }
-        if (comment.getRange() != null
-            && comment.getLine() != comment.getRange().getEndLine()) {
-            throw new IllegalStateException(
-              "Range endLine must be on the same line as the comment");
-        }
-
-        final PatchLineComment nc =
-            new PatchLineComment(new PatchLineComment.Key(patchKey,
-                ChangeUtil.messageUUID(db)), comment.getLine(), me,
-                comment.getParentUuid(), TimeUtil.nowTs());
-        nc.setSide(comment.getSide());
-        nc.setMessage(comment.getMessage());
-        nc.setRange(comment.getRange());
-        db.patchComments().insert(Collections.singleton(nc));
-        db.commit();
-        return nc;
-
-      } else {
-        if (!me.equals(comment.getAuthor())) {
-          throw new NoSuchChangeException(changeId);
-        }
-        comment.setWrittenOn(TimeUtil.nowTs());
-        db.patchComments().update(Collections.singleton(comment));
-        db.commit();
-        return comment;
-      }
-    } finally {
-      db.rollback();
-    }
-  }
-}