Consolidate Macro loading

Summary: Fixes T2820 Grepped for `PhabricatorFileImageMacro`, a common approach to load image macros from storage. Cleaned up file loading too (in most cases where I could be sure that I won't break anything). Did not touch the `add_macro.php` util script, since many users will assume the user `ubuntu` or `ec2-user` to run the script. Add no sessions and no CSRF protection measures... Test Plan: Browsed around all kinds of places. Created and looked at memes, created and edited macros. Used them in Remarkup (with flushed cache). Used `macro.query`, verified it did not crash (that's always a good sign). Could not verify object handles, since I have no idea where they appear right now. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2820 Differential Revision: https://secure.phabricator.com/D5418
2024-11-30 10:42:41 +01:00 · 2013-03-22 13:07:20 -07:00 · 2013-03-22 13:07:20 -07:00 · f95710e799
commit f95710e799
parent 15c9287d43
11 changed files with 72 additions and 48 deletions
--- a/src/applications/macro/conduit/ConduitAPI_macro_query_Method.php
+++ b/src/applications/macro/conduit/ConduitAPI_macro_query_Method.php
@ -24,24 +24,14 @@ final class ConduitAPI_macro_query_Method extends ConduitAPI_macro_Method {
  }

  protected function execute(ConduitAPIRequest $request) {
-
-    $macros = id(new PhabricatorFileImageMacro())->loadAll();
-
-    $files = array();
-    if ($macros) {
-      $files = id(new PhabricatorFile())->loadAllWhere(
-        'phid IN (%Ls)',
-        mpull($macros, 'getFilePHID'));
-      $files = mpull($files, null, 'getPHID');
-    }
+    $macros = id(new PhabricatorMacroQuery())
+      ->setViewer($request->getUser())
+      ->execute();

    $results = array();
    foreach ($macros as $macro) {
-      if (empty($files[$macro->getFilePHID()])) {
-        continue;
-      }
      $results[$macro->getName()] = array(
-        'uri' => $files[$macro->getFilePHID()]->getBestURI(),
+        'uri' => $macro->getFile()->getBestURI(),
      );
    }

--- a/src/applications/macro/controller/PhabricatorMacroCommentController.php
+++ b/src/applications/macro/controller/PhabricatorMacroCommentController.php
@ -17,7 +17,10 @@ final class PhabricatorMacroCommentController
      return new Aphront400Response();
    }

-    $macro = id(new PhabricatorFileImageMacro())->load($this->id);
+    $macro = id(new PhabricatorMacroQuery())
+      ->setViewer($user)
+      ->withIDs(array($this->id))
+      ->executeOne();
    if (!$macro) {
      return new Aphront404Response();
    }
--- a/src/applications/macro/controller/PhabricatorMacroDisableController.php
+++ b/src/applications/macro/controller/PhabricatorMacroDisableController.php
@ -13,7 +13,15 @@ final class PhabricatorMacroDisableController
    $request = $this->getRequest();
    $user = $request->getUser();

-    $macro = id(new PhabricatorFileImageMacro())->load($this->id);
+    $macro = id(new PhabricatorMacroQuery())
+      ->setViewer($user)
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->withIDs(array($this->id))
+      ->executeOne();
    if (!$macro) {
      return new Aphront404Response();
    }
--- a/src/applications/macro/controller/PhabricatorMacroEditController.php
+++ b/src/applications/macro/controller/PhabricatorMacroEditController.php
@ -11,8 +11,19 @@ final class PhabricatorMacroEditController

  public function processRequest() {

+    $request = $this->getRequest();
+    $user = $request->getUser();
+
    if ($this->id) {
-      $macro = id(new PhabricatorFileImageMacro())->load($this->id);
+      $macro = id(new PhabricatorMacroQuery())
+        ->setViewer($user)
+        ->requireCapabilities(
+          array(
+            PhabricatorPolicyCapability::CAN_VIEW,
+            PhabricatorPolicyCapability::CAN_EDIT,
+          ))
+        ->withIDs(array($this->id))
+        ->executeOne();
      if (!$macro) {
        return new Aphront404Response();
      }
@ -26,8 +37,6 @@ final class PhabricatorMacroEditController
    $file = null;
    $can_fetch = PhabricatorEnv::getEnvConfig('security.allow-outbound-http');

-    $request = $this->getRequest();
-    $user = $request->getUser();
    if ($request->isFormPost()) {
      $original = clone $macro;

@ -81,6 +90,7 @@ final class PhabricatorMacroEditController
          $e_file = pht('Invalid');
        } else {
          $macro->setFilePHID($file->getPHID());
+          $macro->attachFile($file);
          $e_file = null;
        }
      }
@ -136,12 +146,9 @@ final class PhabricatorMacroEditController
      $error_view = null;
    }

-
    $current_file = null;
    if ($macro->getFilePHID()) {
-      $current_file = id(new PhabricatorFile())->loadOneWhere(
-        'phid = %s',
-        $macro->getFilePHID());
+      $current_file = $macro->getFile();
    }

    $form = new AphrontFormView();
--- a/src/applications/macro/controller/PhabricatorMacroListController.php
+++ b/src/applications/macro/controller/PhabricatorMacroListController.php
@ -14,10 +14,6 @@ final class PhabricatorMacroListController
    $request = $this->getRequest();
    $viewer = $request->getUser();

-    $macro_table = new PhabricatorFileImageMacro();
-    $file_table = new PhabricatorFile();
-    $conn = $macro_table->establishConnection('r');
-
    $pager = id(new AphrontCursorPagerView())
      ->readFromRequest($request);

--- a/src/applications/macro/controller/PhabricatorMacroMemeController.php
+++ b/src/applications/macro/controller/PhabricatorMacroMemeController.php
@ -9,14 +9,14 @@ final class PhabricatorMacroMemeController
    $upper_text = $request->getStr('uppertext');
    $lower_text = $request->getStr('lowertext');
    $user = $request->getUser();
-    $macro = id(new PhabricatorFileImageMacro())
-      ->loadOneWhere('name=%s', $macro_name);
+    $macro = id(new PhabricatorMacroQuery())
+      ->setViewer($user)
+      ->withNames(array($macro_name))
+      ->executeOne();
    if (!$macro) {
      return new Aphront404Response();
    }
-    $file = id(new PhabricatorFile())->loadOneWhere(
-      'phid = %s',
-      $macro->getFilePHID());
+    $file = $macro->getFile();

    $upper_text = strtoupper($upper_text);
    $lower_text = strtoupper($lower_text);
--- a/src/applications/macro/controller/PhabricatorMacroMemeDialogController.php
+++ b/src/applications/macro/controller/PhabricatorMacroMemeDialogController.php
@ -18,9 +18,10 @@ final class PhabricatorMacroMemeDialogController
        $e_macro = pht('Required');
        $errors[] = pht('Macro name is required.');
      } else {
-        $macro = id(new PhabricatorFileImageMacro())->loadOneWhere(
-          'name = %s',
-          $name);
+        $macro = id(new PhabricatorMacroQuery())
+          ->setViewer($user)
+          ->withNames(array($name))
+          ->executeOne();
        if (!$macro) {
          $e_macro = pht('Invalid');
          $errors[] = pht('No such macro.');
--- a/src/applications/macro/controller/PhabricatorMacroViewController.php
+++ b/src/applications/macro/controller/PhabricatorMacroViewController.php
@ -13,14 +13,15 @@ final class PhabricatorMacroViewController
    $request = $this->getRequest();
    $user = $request->getUser();

-    $macro = id(new PhabricatorFileImageMacro())->load($this->id);
+    $macro = id(new PhabricatorMacroQuery())
+      ->setViewer($user)
+      ->withIDs(array($this->id))
+      ->executeOne();
    if (!$macro) {
      return new Aphront404Response();
    }

-    $file = id(new PhabricatorFile())->loadOneWhere(
-      'phid = %s',
-      $macro->getFilePHID());
+    $file = $macro->getFile();

    $title_short = pht('Macro "%s"', $macro->getName());
    $title_long  = pht('Image Macro "%s"', $macro->getName());
--- a/src/applications/macro/query/PhabricatorMacroQuery.php
+++ b/src/applications/macro/query/PhabricatorMacroQuery.php
@ -9,7 +9,8 @@ final class PhabricatorMacroQuery
  private $ids;
  private $phids;
  private $authors;
-  private $name;
+  private $names;
+  private $nameLike;

  private $status = 'status-any';
  const STATUS_ANY = 'status-any';
@ -31,7 +32,12 @@ final class PhabricatorMacroQuery
  }

  public function withNameLike($name) {
-    $this->name = $name;
+    $this->nameLike = $name;
+    return $this;
+  }
+
+  public function withNames(array $names) {
+    $this->names = $names;
    return $this;
  }

@ -94,11 +100,18 @@ final class PhabricatorMacroQuery
        $this->authors);
    }

-    if ($this->name) {
+    if ($this->nameLike) {
      $where[] = qsprintf(
        $conn,
        'm.name LIKE %~',
-        $this->name);
+        $this->nameLike);
+    }
+
+    if ($this->names) {
+      $where[] = qsprintf(
+        $conn,
+        'm.name IN (%Ls)',
+        $this->names);
    }

    if ($this->status == self::STATUS_ACTIVE) {
--- a/src/applications/macro/remarkup/PhabricatorRemarkupRuleImageMacro.php
+++ b/src/applications/macro/remarkup/PhabricatorRemarkupRuleImageMacro.php
@ -18,8 +18,12 @@ final class PhabricatorRemarkupRuleImageMacro
  public function markupImageMacro($matches) {
    if ($this->images === null) {
      $this->images = array();
-      $rows = id(new PhabricatorFileImageMacro())->loadAllWhere(
-        'isDisabled = 0');
+
+      $viewer = $this->getEngine()->getConfig('viewer');
+      $rows = id(new PhabricatorMacroQuery())
+        ->setViewer($viewer)
+        ->withStatus(PhabricatorMacroQuery::STATUS_ACTIVE)
+        ->execute();
      foreach ($rows as $row) {
        $this->images[$row->getName()] = $row->getFilePHID();
      }
--- a/src/applications/phid/handle/PhabricatorObjectHandleData.php
+++ b/src/applications/phid/handle/PhabricatorObjectHandleData.php
@ -158,9 +158,10 @@ final class PhabricatorObjectHandleData {
        return mpull($xactions, null, 'getPHID');

      case PhabricatorPHIDConstants::PHID_TYPE_MCRO:
-        $macros = id(new PhabricatorFileImageMacro())->loadAllWhere(
-          'phid IN (%Ls)',
-          $phids);
+        $macros = id(new PhabricatorMacroQuery())
+          ->setViewer($this->viewer)
+          ->withPHIDs($phids)
+          ->execute();
        return mpull($macros, null, 'getPHID');

      case PhabricatorPHIDConstants::PHID_TYPE_PSTE: