From 674388ce6aee6f2fdd23c5fb1726448346563996 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 20 Dec 2015 07:44:03 -0800 Subject: [PATCH] Prepare DestructionEngine to be modularized Summary: Ref T9979. The general shape of "engine" code feels pretty good, and I plan to move indexing to be more in line with other modern engines, with the ultimate goal of supporting subprojects (T10010) and several intermediate goals. Before moving indexing, clean up Destruction, since some of the new indexes will need destruction hooks and destruction currently has a lot of `instanceof` stuff that should be easy to fix by applying more modern approaches. Test Plan: - Used `bin/remove destroy` to destory an Almanac device. - Verified that properties for the device were destroyed. - Viewed module panel in UI. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9979 Differential Revision: https://secure.phabricator.com/D14831 --- src/__phutil_library_map__.php | 6 +++ ...acPropertiesDestructionEngineExtension.php | 32 ++++++++++++ ...PhabricatorSearchEngineExtensionModule.php | 2 +- .../engine/PhabricatorDestructionEngine.php | 50 +++++++++++-------- .../PhabricatorDestructionEngineExtension.php | 24 +++++++++ ...icatorDestructionEngineExtensionModule.php | 44 ++++++++++++++++ .../PhabricatorEditEngineExtensionModule.php | 2 +- 7 files changed, 138 insertions(+), 22 deletions(-) create mode 100644 src/applications/almanac/engineextension/AlmanacPropertiesDestructionEngineExtension.php create mode 100644 src/applications/system/engine/PhabricatorDestructionEngineExtension.php create mode 100644 src/applications/system/engine/PhabricatorDestructionEngineExtensionModule.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index cd7b559ad4..5c4425b6e1 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -71,6 +71,7 @@ phutil_register_library_map(array( 'AlmanacNetworkTransaction' => 'applications/almanac/storage/AlmanacNetworkTransaction.php', 'AlmanacNetworkTransactionQuery' => 'applications/almanac/query/AlmanacNetworkTransactionQuery.php', 'AlmanacNetworkViewController' => 'applications/almanac/controller/AlmanacNetworkViewController.php', + 'AlmanacPropertiesDestructionEngineExtension' => 'applications/almanac/engineextension/AlmanacPropertiesDestructionEngineExtension.php', 'AlmanacProperty' => 'applications/almanac/storage/AlmanacProperty.php', 'AlmanacPropertyController' => 'applications/almanac/controller/AlmanacPropertyController.php', 'AlmanacPropertyDeleteController' => 'applications/almanac/controller/AlmanacPropertyDeleteController.php', @@ -2126,6 +2127,8 @@ phutil_register_library_map(array( 'PhabricatorDesktopNotificationsSettingsPanel' => 'applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php', 'PhabricatorDestructibleInterface' => 'applications/system/interface/PhabricatorDestructibleInterface.php', 'PhabricatorDestructionEngine' => 'applications/system/engine/PhabricatorDestructionEngine.php', + 'PhabricatorDestructionEngineExtension' => 'applications/system/engine/PhabricatorDestructionEngineExtension.php', + 'PhabricatorDestructionEngineExtensionModule' => 'applications/system/engine/PhabricatorDestructionEngineExtensionModule.php', 'PhabricatorDeveloperConfigOptions' => 'applications/config/option/PhabricatorDeveloperConfigOptions.php', 'PhabricatorDeveloperPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorDeveloperPreferencesSettingsPanel.php', 'PhabricatorDiffInlineCommentQuery' => 'infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php', @@ -3924,6 +3927,7 @@ phutil_register_library_map(array( 'AlmanacNetworkTransaction' => 'PhabricatorApplicationTransaction', 'AlmanacNetworkTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'AlmanacNetworkViewController' => 'AlmanacNetworkController', + 'AlmanacPropertiesDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension', 'AlmanacProperty' => array( 'PhabricatorCustomFieldStorage', 'PhabricatorPolicyInterface', @@ -6295,6 +6299,8 @@ phutil_register_library_map(array( 'PhabricatorDefaultRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler', 'PhabricatorDesktopNotificationsSettingsPanel' => 'PhabricatorSettingsPanel', 'PhabricatorDestructionEngine' => 'Phobject', + 'PhabricatorDestructionEngineExtension' => 'Phobject', + 'PhabricatorDestructionEngineExtensionModule' => 'PhabricatorConfigModule', 'PhabricatorDeveloperConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorDeveloperPreferencesSettingsPanel' => 'PhabricatorSettingsPanel', 'PhabricatorDiffInlineCommentQuery' => 'PhabricatorApplicationTransactionCommentQuery', diff --git a/src/applications/almanac/engineextension/AlmanacPropertiesDestructionEngineExtension.php b/src/applications/almanac/engineextension/AlmanacPropertiesDestructionEngineExtension.php new file mode 100644 index 0000000000..48209cf4ca --- /dev/null +++ b/src/applications/almanac/engineextension/AlmanacPropertiesDestructionEngineExtension.php @@ -0,0 +1,32 @@ +establishConnection('w'); + + queryfx( + $conn_w, + 'DELETE FROM %T WHERE objectPHID = %s', + $table->getTableName(), + $object->getPHID()); + } + +} diff --git a/src/applications/search/engineextension/PhabricatorSearchEngineExtensionModule.php b/src/applications/search/engineextension/PhabricatorSearchEngineExtensionModule.php index 12787437db..2dd2697a9d 100644 --- a/src/applications/search/engineextension/PhabricatorSearchEngineExtensionModule.php +++ b/src/applications/search/engineextension/PhabricatorSearchEngineExtensionModule.php @@ -8,7 +8,7 @@ final class PhabricatorSearchEngineExtensionModule } public function getModuleName() { - return pht('SearchEngine Extensions'); + return pht('Engine: Search'); } public function renderModuleStatus(AphrontRequest $request) { diff --git a/src/applications/system/engine/PhabricatorDestructionEngine.php b/src/applications/system/engine/PhabricatorDestructionEngine.php index 607c526c14..2b958cd1ff 100644 --- a/src/applications/system/engine/PhabricatorDestructionEngine.php +++ b/src/applications/system/engine/PhabricatorDestructionEngine.php @@ -17,14 +17,9 @@ final class PhabricatorDestructionEngine extends Phobject { $log->setRootLogID($this->rootLogID); } - $object_phid = null; - if (method_exists($object, 'getPHID')) { - try { - $object_phid = $object->getPHID(); - $log->setObjectPHID($object_phid); - } catch (Exception $ex) { - // Ignore. - } + $object_phid = $this->getObjectPHID($object); + if ($object_phid) { + $log->setObjectPHID($object_phid); } if (method_exists($object, 'getMonogram')) { @@ -43,6 +38,18 @@ final class PhabricatorDestructionEngine extends Phobject { $object->destroyObjectPermanently($this); + $extensions = PhabricatorDestructionEngineExtension::getAllExtensions(); + foreach ($extensions as $key => $extension) { + if (!$extension->canDestroyObject($this, $object)) { + unset($extensions[$key]); + continue; + } + } + + foreach ($extensions as $key => $extension) { + $extension->destroyObject($this, $object); + } + if ($object_phid) { $this->destroyEdges($object_phid); @@ -92,10 +99,6 @@ final class PhabricatorDestructionEngine extends Phobject { $token->delete(); } } - - if ($object instanceof AlmanacPropertyInterface) { - $this->destroyAlmanacProperties($object_phid); - } } private function destroyEdges($src_phid) { @@ -152,15 +155,22 @@ final class PhabricatorDestructionEngine extends Phobject { $object_phid); } - private function destroyAlmanacProperties($object_phid) { - $table = new AlmanacProperty(); - $conn_w = $table->establishConnection('w'); + private function destroyAlmanacProperties($object_phid) {} - queryfx( - $conn_w, - 'DELETE FROM %T WHERE objectPHID = %s', - $table->getTableName(), - $object_phid); + public function getObjectPHID($object) { + if (!is_object($object)) { + return null; + } + + if (!method_exists($object, 'getPHID')) { + return null; + } + + try { + return $object->getPHID(); + } catch (Exception $ex) { + return null; + } } } diff --git a/src/applications/system/engine/PhabricatorDestructionEngineExtension.php b/src/applications/system/engine/PhabricatorDestructionEngineExtension.php new file mode 100644 index 0000000000..a27f89da8c --- /dev/null +++ b/src/applications/system/engine/PhabricatorDestructionEngineExtension.php @@ -0,0 +1,24 @@ +getPhobjectClassConstant('EXTENSIONKEY'); + } + + abstract public function getExtensionName(); + abstract public function canDestroyObject( + PhabricatorDestructionEngine $engine, + $object); + abstract public function destroyObject( + PhabricatorDestructionEngine $engine, + $object); + + final public static function getAllExtensions() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getExtensionKey') + ->execute(); + } + +} diff --git a/src/applications/system/engine/PhabricatorDestructionEngineExtensionModule.php b/src/applications/system/engine/PhabricatorDestructionEngineExtensionModule.php new file mode 100644 index 0000000000..e57375e336 --- /dev/null +++ b/src/applications/system/engine/PhabricatorDestructionEngineExtensionModule.php @@ -0,0 +1,44 @@ +getViewer(); + + $extensions = PhabricatorDestructionEngineExtension::getAllExtensions(); + + $rows = array(); + foreach ($extensions as $extension) { + $rows[] = array( + get_class($extension), + $extension->getExtensionName(), + ); + } + + $table = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('Class'), + pht('Name'), + )) + ->setColumnClasses( + array( + null, + 'wide pri', + )); + + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('DestructionEngine Extensions')) + ->setTable($table); + } + +} diff --git a/src/applications/transactions/editengineextension/PhabricatorEditEngineExtensionModule.php b/src/applications/transactions/editengineextension/PhabricatorEditEngineExtensionModule.php index 98a4e0cb3b..3cc572908a 100644 --- a/src/applications/transactions/editengineextension/PhabricatorEditEngineExtensionModule.php +++ b/src/applications/transactions/editengineextension/PhabricatorEditEngineExtensionModule.php @@ -8,7 +8,7 @@ final class PhabricatorEditEngineExtensionModule } public function getModuleName() { - return pht('EditEngine Extensions'); + return pht('Engine: Edit'); } public function renderModuleStatus(AphrontRequest $request) {