Implement versioned drafts in EditEngine comment forms

Summary:
Ref T9132. Fixes T5031. This approximately implements the plan described in T5031#67988:

When we recieve a preview request, don't write a draft if the form is from a version of the object before the last update the viewer made.

This should fix the race-related (?) zombie draft comments that sometimes show up.

I just added a new object for this stuff to make it easier to do stacked actions (or whatever we end up with) a little later, since I needed to do some schema adjustments anyway.

Test Plan:
  - Typed some text.
  - Reloaded page.
  - Draft stayed there.
  - Tried real hard to get it to ghost by submitting stuff in multiple windows and typing a lot and couldn't, although I didn't bother specifically narrowing down the race condition.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5031, T9132

Differential Revision: https://secure.phabricator.com/D14640

resources/sql/autopatches/20151202.versioneddraft.1.sql

@@ -0,0 +1,10 @@
+CREATE TABLE {$NAMESPACE}_draft.draft_versioneddraft (
+  id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
+  objectPHID VARBINARY(64) NOT NULL,
+  authorPHID VARBINARY(64) NOT NULL,
+  version INT UNSIGNED NOT NULL,
+  properties LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT},
+  dateCreated INT UNSIGNED NOT NULL,
+  dateModified INT UNSIGNED NOT NULL,
+  UNIQUE KEY `key_object` (objectPHID, authorPHID, version)
+) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT};

src/__phutil_library_map__.php

@@ -3226,6 +3226,7 @@ phutil_register_library_map(array(
     'PhabricatorUsersPolicyRule' => 'applications/people/policyrule/PhabricatorUsersPolicyRule.php',
     'PhabricatorUsersSearchField' => 'applications/people/searchfield/PhabricatorUsersSearchField.php',
     'PhabricatorVCSResponse' => 'applications/repository/response/PhabricatorVCSResponse.php',
+    'PhabricatorVersionedDraft' => 'applications/draft/storage/PhabricatorVersionedDraft.php',
     'PhabricatorVeryWowEnglishTranslation' => 'infrastructure/internationalization/translation/PhabricatorVeryWowEnglishTranslation.php',
     'PhabricatorViewerDatasource' => 'applications/people/typeahead/PhabricatorViewerDatasource.php',
     'PhabricatorWatcherHasObjectEdgeType' => 'applications/transactions/edges/PhabricatorWatcherHasObjectEdgeType.php',
@@ -7536,6 +7537,7 @@ phutil_register_library_map(array(
     'PhabricatorUsersPolicyRule' => 'PhabricatorPolicyRule',
     'PhabricatorUsersSearchField' => 'PhabricatorSearchTokenizerField',
     'PhabricatorVCSResponse' => 'AphrontResponse',
+    'PhabricatorVersionedDraft' => 'PhabricatorDraftDAO',
     'PhabricatorVeryWowEnglishTranslation' => 'PhutilTranslation',
     'PhabricatorViewerDatasource' => 'PhabricatorTypeaheadDatasource',
     'PhabricatorWatcherHasObjectEdgeType' => 'PhabricatorEdgeType',

src/applications/draft/storage/PhabricatorVersionedDraft.php

@@ -0,0 +1,83 @@
+<?php
+
+final class PhabricatorVersionedDraft extends PhabricatorDraftDAO {
+
+  const KEY_VERSION = 'draft.version';
+
+  protected $objectPHID;
+  protected $authorPHID;
+  protected $version;
+  protected $properties = array();
+
+  protected function getConfiguration() {
+    return array(
+      self::CONFIG_SERIALIZATION => array(
+        'properties' => self::SERIALIZATION_JSON,
+      ),
+      self::CONFIG_COLUMN_SCHEMA => array(
+        'version' => 'uint32',
+      ),
+      self::CONFIG_KEY_SCHEMA => array(
+        'key_object' => array(
+          'columns' => array('objectPHID', 'authorPHID', 'version'),
+          'unique' => true,
+        ),
+      ),
+    ) + parent::getConfiguration();
+  }
+
+  public function setProperty($key, $value) {
+    $this->properties[$key] = $value;
+    return $this;
+  }
+
+  public function getProperty($key, $default = null) {
+    return idx($this->properties, $key, $default);
+  }
+
+  public static function loadDraft(
+    $object_phid,
+    $viewer_phid) {
+
+    return id(new PhabricatorVersionedDraft())->loadOneWhere(
+      'objectPHID = %s AND authorPHID = %s ORDER BY version DESC LIMIT 1',
+      $object_phid,
+      $viewer_phid);
+  }
+
+  public static function loadOrCreateDraft(
+    $object_phid,
+    $viewer_phid,
+    $version) {
+
+    $draft = self::loadDraft($object_phid, $viewer_phid);
+    if ($draft) {
+      return $draft;
+    }
+
+    return id(new PhabricatorVersionedDraft())
+      ->setObjectPHID($object_phid)
+      ->setAuthorPHID($viewer_phid)
+      ->setVersion($version)
+      ->save();
+  }
+
+  public static function purgeDrafts(
+    $object_phid,
+    $viewer_phid,
+    $version) {
+
+    $draft = new PhabricatorVersionedDraft();
+    $conn_w = $draft->establishConnection('w');
+
+    queryfx(
+      $conn_w,
+      'DELETE FROM %T WHERE objectPHID = %s AND authorPHID = %s
+        AND version <= %d',
+      $draft->getTableName(),
+      $object_phid,
+      $viewer_phid,
+      $version);
+  }
+
+}

src/applications/transactions/editengine/PhabricatorEditEngine.php

@@ -879,21 +879,56 @@ abstract class PhabricatorEditEngine
     $header_text = $this->getCommentViewHeaderText($object);
     $button_text = $this->getCommentViewButtonText($object);
 
-    // TODO: Drafts.
-    // $draft = PhabricatorDraft::newFromUserAndKey(
-    //   $viewer,
-    //   $object_phid);
-
     $comment_uri = $this->getEditURI($object, 'comment/');
 
-    return id(new PhabricatorApplicationTransactionCommentView())
+    $view = id(new PhabricatorApplicationTransactionCommentView())
       ->setUser($viewer)
       ->setObjectPHID($object_phid)
       ->setHeaderText($header_text)
       ->setAction($comment_uri)
       ->setSubmitButtonName($button_text);
+
+    $draft = PhabricatorVersionedDraft::loadDraft(
+      $object_phid,
+      $viewer->getPHID());
+    if ($draft) {
+      $view->setVersionedDraft($draft);
+    }
+
+    $view->setCurrentVersion($this->loadDraftVersion($object));
+
+    return $view;
   }
 
+  protected function loadDraftVersion($object) {
+    $viewer = $this->getViewer();
+
+    if (!$viewer->isLoggedIn()) {
+      return null;
+    }
+
+    $template = $object->getApplicationTransactionTemplate();
+    $conn_r = $template->establishConnection('r');
+
+    // Find the most recent transaction the user has written. We'll use this
+    // as a version number to make sure that out-of-date drafts get discarded.
+    $result = queryfx_one(
+      $conn_r,
+      'SELECT id AS version FROM %T
+        WHERE objectPHID = %s AND authorPHID = %s
+        ORDER BY id DESC LIMIT 1',
+      $template->getTableName(),
+      $object->getPHID(),
+      $viewer->getPHID());
+
+    if ($result) {
+      return (int)$result['version'];
+    } else {
+      return null;
+    }
+  }
+
+
 /* -(  Responding to HTTP Parameter Requests  )------------------------------ */
 
 
@@ -970,13 +1005,31 @@ abstract class PhabricatorEditEngine
     $template = $object->getApplicationTransactionTemplate();
     $comment_template = $template->getApplicationTransactionCommentObject();
 
+    $comment_text = $request->getStr('comment');
+
+    if ($is_preview) {
+      $version_key = PhabricatorVersionedDraft::KEY_VERSION;
+      $request_version = $request->getInt($version_key);
+      $current_version = $this->loadDraftVersion($object);
+      if ($request_version >= $current_version) {
+        $draft = PhabricatorVersionedDraft::loadOrCreateDraft(
+          $object->getPHID(),
+          $viewer->getPHID(),
+          $current_version);
+
+        // TODO: This is just a proof of concept.
+        $draft->setProperty('temporary.comment', $comment_text);
+        $draft->save();
+      }
+    }
+
     $xactions = array();
 
     $xactions[] = id(clone $template)
       ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
       ->attachComment(
         id(clone $comment_template)
-          ->setContent($request->getStr('comment')));
+          ->setContent($comment_text));
 
     $editor = $object->getApplicationTransactionEditor()
       ->setActor($viewer)
@@ -992,6 +1045,13 @@ abstract class PhabricatorEditEngine
         ->setException($ex);
     }
 
+    if (!$is_preview) {
+      PhabricatorVersionedDraft::purgeDrafts(
+        $object->getPHID(),
+        $viewer->getPHID(),
+        $this->loadDraftVersion($object));
+    }
+
     if ($request->isAjax() && $is_preview) {
       return id(new PhabricatorApplicationTransactionResponse())
         ->setViewer($viewer)

src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php

@@ -20,6 +20,9 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {
   private $objectPHID;
   private $headerText;
 
+  private $currentVersion;
+  private $versionedDraft;
+
   public function setObjectPHID($object_phid) {
     $this->objectPHID = $object_phid;
     return $this;
@@ -46,6 +49,25 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {
     return $this->requestURI;
   }
 
+  public function setCurrentVersion($current_version) {
+    $this->currentVersion = $current_version;
+    return $this;
+  }
+
+  public function getCurrentVersion() {
+    return $this->currentVersion;
+  }
+
+  public function setVersionedDraft(
+    PhabricatorVersionedDraft $versioned_draft) {
+    $this->versionedDraft = $versioned_draft;
+    return $this;
+  }
+
+  public function getVersionedDraft() {
+    return $this->versionedDraft;
+  }
+
   public function setDraft(PhabricatorDraft $draft) {
     $this->draft = $draft;
     return $this;
@@ -148,10 +170,18 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {
       $draft_key = $this->getDraft()->getDraftKey();
     }
 
+    $versioned_draft = $this->getVersionedDraft();
+    if ($versioned_draft) {
+      $draft_comment = $versioned_draft->getProperty('temporary.comment', '');
+    }
+
     if (!$this->getObjectPHID()) {
       throw new PhutilInvalidStateException('setObjectPHID', 'render');
     }
 
+    $version_key = PhabricatorVersionedDraft::KEY_VERSION;
+    $version_value = $this->getCurrentVersion();
+
     return id(new AphrontFormView())
       ->setUser($this->getUser())
       ->addSigil('transaction-append')
@@ -163,6 +193,7 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {
       ->setAction($this->getAction())
       ->setID($this->getFormID())
       ->addHiddenInput('__draft__', $draft_key)
+      ->addHiddenInput($version_key, $version_value)
       ->appendChild(
         id(new PhabricatorRemarkupControl())
           ->setID($this->getCommentID())