From 4169d7bfd5b1c636e8c9672d6f10fa51084a6272 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 14 Oct 2015 02:56:39 -0700 Subject: [PATCH] Fix an issue where Harbormaster might cycle while saving The way custom field interact with storage is a little odd, and can send us down a bad path when applying external effect while saving changes. --- .../customfield/field/PhabricatorCustomField.php | 4 +--- .../standard/PhabricatorStandardCustomField.php | 7 ++++++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/infrastructure/customfield/field/PhabricatorCustomField.php b/src/infrastructure/customfield/field/PhabricatorCustomField.php index f9f5979029..64639ed34f 100644 --- a/src/infrastructure/customfield/field/PhabricatorCustomField.php +++ b/src/infrastructure/customfield/field/PhabricatorCustomField.php @@ -540,9 +540,7 @@ abstract class PhabricatorCustomField extends Phobject { * @task storage */ public function newStorageObject() { - if ($this->proxy) { - return $this->proxy->newStorageObject(); - } + // NOTE: This intentionally isn't proxied, to avoid call cycles. throw new PhabricatorCustomFieldImplementationIncompleteException($this); } diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php index 76a1de9989..ac3e163f6a 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php @@ -186,7 +186,12 @@ abstract class PhabricatorStandardCustomField } public function shouldUseStorage() { - return true; + try { + $object = $this->newStorageObject(); + return true; + } catch (PhabricatorCustomFieldImplementationIncompleteException $ex) { + return false; + } } public function getValueForStorage() {