Move Files to ModularTransactions

Summary: Ref T11357. A lot of file creation doesn't go through transactions, so we only actually have one real transaction type: editing a file name.

Test Plan:
Created and edited files.

{F4559287}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11357

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

src/__phutil_library_map__.php

@@ -2765,6 +2765,7 @@ phutil_register_library_map(array(
     'PhabricatorFileLightboxController' => 'applications/files/controller/PhabricatorFileLightboxController.php',
     'PhabricatorFileLinkView' => 'view/layout/PhabricatorFileLinkView.php',
     'PhabricatorFileListController' => 'applications/files/controller/PhabricatorFileListController.php',
+    'PhabricatorFileNameTransaction' => 'applications/files/xaction/PhabricatorFileNameTransaction.php',
     'PhabricatorFileQuery' => 'applications/files/query/PhabricatorFileQuery.php',
     'PhabricatorFileROT13StorageFormat' => 'applications/files/format/PhabricatorFileROT13StorageFormat.php',
     'PhabricatorFileRawStorageFormat' => 'applications/files/format/PhabricatorFileRawStorageFormat.php',
@@ -2783,6 +2784,7 @@ phutil_register_library_map(array(
     'PhabricatorFileTransaction' => 'applications/files/storage/PhabricatorFileTransaction.php',
     'PhabricatorFileTransactionComment' => 'applications/files/storage/PhabricatorFileTransactionComment.php',
     'PhabricatorFileTransactionQuery' => 'applications/files/query/PhabricatorFileTransactionQuery.php',
+    'PhabricatorFileTransactionType' => 'applications/files/xaction/PhabricatorFileTransactionType.php',
     'PhabricatorFileTransform' => 'applications/files/transform/PhabricatorFileTransform.php',
     'PhabricatorFileTransformController' => 'applications/files/controller/PhabricatorFileTransformController.php',
     'PhabricatorFileTransformListController' => 'applications/files/controller/PhabricatorFileTransformListController.php',
@@ -7890,6 +7892,7 @@ phutil_register_library_map(array(
     'PhabricatorFileLightboxController' => 'PhabricatorFileController',
     'PhabricatorFileLinkView' => 'AphrontTagView',
     'PhabricatorFileListController' => 'PhabricatorFileController',
+    'PhabricatorFileNameTransaction' => 'PhabricatorFileTransactionType',
     'PhabricatorFileQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorFileROT13StorageFormat' => 'PhabricatorFileStorageFormat',
     'PhabricatorFileRawStorageFormat' => 'PhabricatorFileStorageFormat',
@@ -7905,9 +7908,10 @@ phutil_register_library_map(array(
     'PhabricatorFileTestCase' => 'PhabricatorTestCase',
     'PhabricatorFileTestDataGenerator' => 'PhabricatorTestDataGenerator',
     'PhabricatorFileThumbnailTransform' => 'PhabricatorFileImageTransform',
-    'PhabricatorFileTransaction' => 'PhabricatorApplicationTransaction',
+    'PhabricatorFileTransaction' => 'PhabricatorModularTransaction',
     'PhabricatorFileTransactionComment' => 'PhabricatorApplicationTransactionComment',
     'PhabricatorFileTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
+    'PhabricatorFileTransactionType' => 'PhabricatorModularTransactionType',
     'PhabricatorFileTransform' => 'Phobject',
     'PhabricatorFileTransformController' => 'PhabricatorFileController',
     'PhabricatorFileTransformListController' => 'PhabricatorFileController',

src/applications/badges/xaction/PhabricatorBadgesBadgeNameTransaction.php

@@ -43,7 +43,7 @@ final class PhabricatorBadgesBadgeNameTransaction
       $new_value = $xaction->getNewValue();
       $new_length = strlen($new_value);
       if ($new_length > $max_length) {
-        $errors[] = $this->newRequiredError(
+        $errors[] = $this->newInvalidError(
           pht('The name can be no longer than %s characters.',
           new PhutilNumber($max_length)));
       }

src/applications/files/controller/PhabricatorFileEditController.php

@@ -31,7 +31,7 @@ final class PhabricatorFileEditController extends PhabricatorFileController {
       $file_name = $request->getStr('name');
       $errors = array();
 
-      $type_name = PhabricatorFileTransaction::TYPE_NAME;
+      $type_name = PhabricatorFileNameTransaction::TRANSACTIONTYPE;
 
       $xactions = array();
 
@@ -40,7 +40,7 @@ final class PhabricatorFileEditController extends PhabricatorFileController {
         ->setNewValue($can_view);
 
       $xactions[] = id(new PhabricatorFileTransaction())
-        ->setTransactionType(PhabricatorFileTransaction::TYPE_NAME)
+        ->setTransactionType($type_name)
         ->setNewValue($file_name);
 
       $editor = id(new PhabricatorFileEditor())

src/applications/files/editor/PhabricatorFileEditor.php

@@ -17,46 +17,9 @@ final class PhabricatorFileEditor
     $types[] = PhabricatorTransactions::TYPE_COMMENT;
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
 
-    $types[] = PhabricatorFileTransaction::TYPE_NAME;
-
     return $types;
   }
 
-  protected function getCustomTransactionOldValue(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorFileTransaction::TYPE_NAME:
-        return $object->getName();
-    }
-  }
-
-  protected function getCustomTransactionNewValue(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorFileTransaction::TYPE_NAME:
-        return $xaction->getNewValue();
-    }
-  }
-
-  protected function applyCustomInternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorFileTransaction::TYPE_NAME:
-        $object->setName($xaction->getNewValue());
-        break;
-    }
-  }
-
-  protected function applyCustomExternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {}
-
   protected function shouldSendMail(
     PhabricatorLiskDAO $object,
     array $xactions) {
@@ -111,34 +74,4 @@ final class PhabricatorFileEditor
     return false;
   }
 
-  protected function validateTransaction(
-    PhabricatorLiskDAO $object,
-    $type,
-    array $xactions) {
-
-    $errors = parent::validateTransaction($object, $type, $xactions);
-
-    switch ($type) {
-      case PhabricatorFileTransaction::TYPE_NAME:
-        $missing = $this->validateIsEmptyTextField(
-          $object->getName(),
-          $xactions);
-
-        if ($missing) {
-          $error = new PhabricatorApplicationTransactionValidationError(
-            $type,
-            pht('Required'),
-            pht('File name is required.'),
-            nonempty(last($xactions), null));
-
-          $error->setIsMissingFieldError(true);
-          $errors[] = $error;
-        }
-        break;
-    }
-
-    return $errors;
-  }
-
-
 }

src/applications/files/storage/PhabricatorFileTransaction.php

@@ -1,9 +1,7 @@
 <?php
 
 final class PhabricatorFileTransaction
-  extends PhabricatorApplicationTransaction {
+  extends PhabricatorModularTransaction {
-
-  const TYPE_NAME  = 'file:name';
 
   public function getApplicationName() {
     return 'file';
@@ -17,69 +15,8 @@ final class PhabricatorFileTransaction
     return new PhabricatorFileTransactionComment();
   }
 
-  public function getTitle() {
+  public function getBaseTransactionClass() {
-    $author_phid = $this->getAuthorPHID();
+    return 'PhabricatorFileTransactionType';
-
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_NAME:
-        return pht(
-          '%s updated the name for this file from "%s" to "%s".',
-          $this->renderHandleLink($author_phid),
-          $old,
-          $new);
-        break;
-    }
-
-    return parent::getTitle();
   }
 
-  public function getTitleForFeed() {
-    $author_phid = $this->getAuthorPHID();
-    $object_phid = $this->getObjectPHID();
-
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    $type = $this->getTransactionType();
-    switch ($type) {
-      case self::TYPE_NAME:
-        return pht(
-          '%s updated the name of %s from "%s" to "%s".',
-          $this->renderHandleLink($author_phid),
-          $this->renderHandleLink($object_phid),
-          $old,
-          $new);
-        break;
-      }
-
-    return parent::getTitleForFeed();
-  }
-
-  public function getIcon() {
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_NAME:
-        return 'fa-pencil';
-    }
-
-    return parent::getIcon();
-  }
-
-
-  public function getColor() {
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_NAME:
-        return PhabricatorTransactions::COLOR_BLUE;
-    }
-
-    return parent::getColor();
-  }
 }

src/applications/files/storage/__tests__/PhabricatorFileTestCase.php

@@ -32,7 +32,7 @@ final class PhabricatorFileTestCase extends PhabricatorTestCase {
     // First, change the name: this should not scramble the secret.
     $xactions = array();
     $xactions[] = id(new PhabricatorFileTransaction())
-      ->setTransactionType(PhabricatorFileTransaction::TYPE_NAME)
+      ->setTransactionType(PhabricatorFileNameTransaction::TRANSACTIONTYPE)
       ->setNewValue('test.dat2');
 
     $engine = id(new PhabricatorFileEditor())

src/applications/files/xaction/PhabricatorFileNameTransaction.php

@@ -0,0 +1,55 @@
+<?php
+
+final class PhabricatorFileNameTransaction
+  extends PhabricatorFileTransactionType {
+
+  const TRANSACTIONTYPE = 'file:name';
+
+  public function generateOldValue($object) {
+    return $object->getName();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setName($value);
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s updated the name for this file from "%s" to "%s".',
+      $this->renderAuthor(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s updated the name of %s from "%s" to "%s".',
+      $this->renderAuthor(),
+      $this->renderObject(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    if ($this->isEmptyTextTransaction($object->getName(), $xactions)) {
+      $errors[] = $this->newRequiredError(pht('Files must have a name.'));
+    }
+
+    $max_length = $object->getColumnMaximumByteLength('name');
+    foreach ($xactions as $xaction) {
+      $new_value = $xaction->getNewValue();
+      $new_length = strlen($new_value);
+      if ($new_length > $max_length) {
+        $errors[] = $this->newInvalidError(
+          pht(
+            'File names must not be longer than %s character(s).',
+            new PhutilNumber($max_length)));
+      }
+    }
+
+    return $errors;
+  }
+
+}

src/applications/files/xaction/PhabricatorFileTransactionType.php

@@ -0,0 +1,4 @@
+<?php
+
+abstract class PhabricatorFileTransactionType
+  extends PhabricatorModularTransactionType {}