From 3ef0721adacce6e94eb855bb2a97b70733a6a3b2 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Fri, 15 May 2015 14:07:17 -0700 Subject: [PATCH] Reduce PhabricatorUser::getOmnipotentUser calls by adding a getViewer method to PhbaricatorDestructionEngine Summary: Fixes T6956. Before this change, we called PhabricatorUser::getOmnipotentUser in the various delete methods to query the data. Now, we use $engine->getViewer(), since its always a good thing to have less calls to PhabricatorUser::getOmnipotentUser thrown around the codebase. I used the "codemod" tool to audit the existing calls to PhabricatorDestructorEngine (all of them) so ostensibly this gets all the spots. If I missed something though, its still going to work, so this change is very low risk. Test Plan: ./bin/remove destroy P1; visit P1 and get a 404 Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T6956 Differential Revision: https://secure.phabricator.com/D12866 --- src/applications/almanac/storage/AlmanacDevice.php | 2 +- src/applications/almanac/storage/AlmanacInterface.php | 2 +- src/applications/almanac/storage/AlmanacNetwork.php | 2 +- src/applications/almanac/storage/AlmanacService.php | 2 +- .../differential/storage/DifferentialRevision.php | 2 +- src/applications/diviner/storage/DivinerLiveBook.php | 2 +- .../files/controller/PhabricatorFileTransformController.php | 4 ++-- src/applications/files/storage/PhabricatorFileChunk.php | 2 +- src/applications/paste/storage/PhabricatorPaste.php | 2 +- .../system/engine/PhabricatorDestructionEngine.php | 4 ++++ 10 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/applications/almanac/storage/AlmanacDevice.php b/src/applications/almanac/storage/AlmanacDevice.php index 19ee3f89ff..d0994174b3 100644 --- a/src/applications/almanac/storage/AlmanacDevice.php +++ b/src/applications/almanac/storage/AlmanacDevice.php @@ -240,7 +240,7 @@ final class AlmanacDevice PhabricatorDestructionEngine $engine) { $interfaces = id(new AlmanacInterfaceQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->setViewer($engine->getViewer()) ->withDevicePHIDs(array($this->getPHID())) ->execute(); foreach ($interfaces as $interface) { diff --git a/src/applications/almanac/storage/AlmanacInterface.php b/src/applications/almanac/storage/AlmanacInterface.php index 5ab892cef7..729e95aa9a 100644 --- a/src/applications/almanac/storage/AlmanacInterface.php +++ b/src/applications/almanac/storage/AlmanacInterface.php @@ -119,7 +119,7 @@ final class AlmanacInterface PhabricatorDestructionEngine $engine) { $bindings = id(new AlmanacBindingQuery()) - ->setViewer($this->getViewer()) + ->setViewer($engine->getViewer()) ->withInterfacePHIDs(array($this->getPHID())) ->execute(); foreach ($bindings as $binding) { diff --git a/src/applications/almanac/storage/AlmanacNetwork.php b/src/applications/almanac/storage/AlmanacNetwork.php index 92d4b8fa47..a623248fd9 100644 --- a/src/applications/almanac/storage/AlmanacNetwork.php +++ b/src/applications/almanac/storage/AlmanacNetwork.php @@ -103,7 +103,7 @@ final class AlmanacNetwork PhabricatorDestructionEngine $engine) { $interfaces = id(new AlmanacInterfaceQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->setViewer($engine->getViewer()) ->withNetworkPHIDs(array($this->getPHID())) ->execute(); diff --git a/src/applications/almanac/storage/AlmanacService.php b/src/applications/almanac/storage/AlmanacService.php index 36c7761e22..b351f0d3fc 100644 --- a/src/applications/almanac/storage/AlmanacService.php +++ b/src/applications/almanac/storage/AlmanacService.php @@ -221,7 +221,7 @@ final class AlmanacService PhabricatorDestructionEngine $engine) { $bindings = id(new AlmanacBindingQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->setViewer($engine->getViewer()) ->withServicePHIDs(array($this->getPHID())) ->execute(); foreach ($bindings as $binding) { diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index 74f3ff34d7..e6baa678e9 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -550,7 +550,7 @@ final class DifferentialRevision extends DifferentialDAO $this->openTransaction(); $diffs = id(new DifferentialDiffQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->setViewer($engine->getViewer()) ->withRevisionIDs(array($this->getID())) ->execute(); foreach ($diffs as $diff) { diff --git a/src/applications/diviner/storage/DivinerLiveBook.php b/src/applications/diviner/storage/DivinerLiveBook.php index 68a6f8a0ec..ef7a9bb277 100644 --- a/src/applications/diviner/storage/DivinerLiveBook.php +++ b/src/applications/diviner/storage/DivinerLiveBook.php @@ -91,7 +91,7 @@ final class DivinerLiveBook extends DivinerDAO $this->openTransaction(); $atoms = id(new DivinerAtomQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->setViewer($engine->getViewer()) ->withBookPHIDs(array($this->getPHID())) ->withIncludeGhosts(true) ->withIncludeUndocumentable(true) diff --git a/src/applications/files/controller/PhabricatorFileTransformController.php b/src/applications/files/controller/PhabricatorFileTransformController.php index 5062fe76fc..0e7e2b73d7 100644 --- a/src/applications/files/controller/PhabricatorFileTransformController.php +++ b/src/applications/files/controller/PhabricatorFileTransformController.php @@ -104,8 +104,9 @@ final class PhabricatorFileTransformController } private function destroyTransform(PhabricatorTransformedFile $xform) { + $engine = new PhabricatorDestructionEngine(); $file = id(new PhabricatorFileQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->setViewer($engine->getViewer()) ->withPHIDs(array($xform->getTransformedPHID())) ->executeOne(); @@ -114,7 +115,6 @@ final class PhabricatorFileTransformController if (!$file) { $xform->delete(); } else { - $engine = new PhabricatorDestructionEngine(); $engine->destroyObject($file); } diff --git a/src/applications/files/storage/PhabricatorFileChunk.php b/src/applications/files/storage/PhabricatorFileChunk.php index b69a421fed..eaa10acd63 100644 --- a/src/applications/files/storage/PhabricatorFileChunk.php +++ b/src/applications/files/storage/PhabricatorFileChunk.php @@ -91,7 +91,7 @@ final class PhabricatorFileChunk extends PhabricatorFileDAO $data_phid = $this->getDataFilePHID(); if ($data_phid) { $data_file = id(new PhabricatorFileQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->setViewer($engine->getViewer()) ->withPHIDs(array($data_phid)) ->executeOne(); if ($data_file) { diff --git a/src/applications/paste/storage/PhabricatorPaste.php b/src/applications/paste/storage/PhabricatorPaste.php index 9eb88a77b4..3a0ae754e7 100644 --- a/src/applications/paste/storage/PhabricatorPaste.php +++ b/src/applications/paste/storage/PhabricatorPaste.php @@ -172,7 +172,7 @@ final class PhabricatorPaste extends PhabricatorPasteDAO if ($this->filePHID) { $file = id(new PhabricatorFileQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->setViewer($engine->getViewer()) ->withPHIDs(array($this->filePHID)) ->executeOne(); if ($file) { diff --git a/src/applications/system/engine/PhabricatorDestructionEngine.php b/src/applications/system/engine/PhabricatorDestructionEngine.php index b55b336052..c5b813169c 100644 --- a/src/applications/system/engine/PhabricatorDestructionEngine.php +++ b/src/applications/system/engine/PhabricatorDestructionEngine.php @@ -4,6 +4,10 @@ final class PhabricatorDestructionEngine extends Phobject { private $rootLogID; + public function getViewer() { + return PhabricatorUser::getOmnipotentUser(); + } + public function destroyObject(PhabricatorDestructibleInterface $object) { $log = id(new PhabricatorSystemDestructionLog()) ->setEpoch(time())