@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Allow TransactionEditor to move publishing work to the daemons

Summary:
Ref T6367. This is similar to D11329, but not quite as ambitious.

Allow Editors to implement `supportsWorkers()` and move their publishing work into a daemon. So far, only Paste supports this.

Most of the complexity here is saving and restoring state across the barrier between the web process and the worker process, but I think this is ~90% of it and then we'll pick up a couple of random things in applications.

I'm primarily trying to keep this as gradual as possible.

Test Plan:
- Published transactions with and without daemon support.
- Looked at mail, feed.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: fabe, epriestley

Maniphest Tasks: T6367

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

+320 -44
+2
src/__phutil_library_map__.php
··· 1334 1334 'PhabricatorApplicationTransactionInterface' => 'applications/transactions/interface/PhabricatorApplicationTransactionInterface.php', 1335 1335 'PhabricatorApplicationTransactionNoEffectException' => 'applications/transactions/exception/PhabricatorApplicationTransactionNoEffectException.php', 1336 1336 'PhabricatorApplicationTransactionNoEffectResponse' => 'applications/transactions/response/PhabricatorApplicationTransactionNoEffectResponse.php', 1337 + 'PhabricatorApplicationTransactionPublishWorker' => 'applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php', 1337 1338 'PhabricatorApplicationTransactionQuery' => 'applications/transactions/query/PhabricatorApplicationTransactionQuery.php', 1338 1339 'PhabricatorApplicationTransactionReplyHandler' => 'applications/transactions/replyhandler/PhabricatorApplicationTransactionReplyHandler.php', 1339 1340 'PhabricatorApplicationTransactionResponse' => 'applications/transactions/response/PhabricatorApplicationTransactionResponse.php', ··· 4678 4679 'PhabricatorApplicationTransactionFeedStory' => 'PhabricatorFeedStory', 4679 4680 'PhabricatorApplicationTransactionNoEffectException' => 'Exception', 4680 4681 'PhabricatorApplicationTransactionNoEffectResponse' => 'AphrontProxyResponse', 4682 + 'PhabricatorApplicationTransactionPublishWorker' => 'PhabricatorWorker', 4681 4683 'PhabricatorApplicationTransactionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 4682 4684 'PhabricatorApplicationTransactionReplyHandler' => 'PhabricatorMailReplyHandler', 4683 4685 'PhabricatorApplicationTransactionResponse' => 'AphrontProxyResponse',
+5 -3
src/applications/paste/editor/PhabricatorPasteEditor.php
··· 3 3 final class PhabricatorPasteEditor 4 4 extends PhabricatorApplicationTransactionEditor { 5 5 6 - private $pasteFile; 7 - 8 6 public function getEditorApplicationClass() { 9 7 return 'PhabricatorPasteApplication'; 10 8 } ··· 134 132 protected function getMailTo(PhabricatorLiskDAO $object) { 135 133 return array( 136 134 $object->getAuthorPHID(), 137 - $this->requireActor()->getPHID(), 135 + $this->getActingAsPHID(), 138 136 ); 139 137 } 140 138 ··· 184 182 185 183 protected function supportsSearch() { 186 184 return false; 185 + } 186 + 187 + protected function supportsWorkers() { 188 + return true; 187 189 } 188 190 189 191 }
+179 -41
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
··· 1 1 <?php 2 2 3 3 /** 4 - * @task mail Sending Mail 5 - * @task feed Publishing Feed Stories 4 + * @task mail Sending Mail 5 + * @task feed Publishing Feed Stories 6 6 * @task search Search Index 7 - * @task files Integration with Files 7 + * @task files Integration with Files 8 + * @task workers Managing Workers 8 9 */ 9 10 abstract class PhabricatorApplicationTransactionEditor 10 11 extends PhabricatorEditor { ··· 30 31 private $actingAsPHID; 31 32 private $disableEmail; 32 33 34 + private $heraldEmailPHIDs = array(); 35 + private $heraldForcedEmailPHIDs = array(); 36 + private $heraldHeader; 33 37 34 38 /** 35 39 * Get the class name for the application this editor is a part of. ··· 861 865 $object, 862 866 $herald_xactions); 863 867 868 + $adapter = $this->getHeraldAdapter(); 869 + $this->heraldEmailPHIDs = $adapter->getEmailPHIDs(); 870 + $this->heraldForcedEmailPHIDs = $adapter->getForcedEmailPHIDs(); 871 + 864 872 // Merge the new transactions into the transaction list: we want to 865 873 // send email and publish feed stories about them, too. 866 874 $xactions = array_merge($xactions, $herald_xactions); 867 875 } 868 876 } 869 877 878 + $this->didApplyTransactions($xactions); 879 + 880 + if ($object instanceof PhabricatorCustomFieldInterface) { 881 + // Maybe this makes more sense to move into the search index itself? For 882 + // now I'm putting it here since I think we might end up with things that 883 + // need it to be up to date once the next page loads, but if we don't go 884 + // there we we could move it into search once search moves to the daemons. 885 + 886 + // It now happens in the search indexer as well, but the search indexer is 887 + // always daemonized, so the logic above still potentially holds. We could 888 + // possibly get rid of this. The major motivation for putting it in the 889 + // indexer was to enable reindexing to work. 890 + 891 + $fields = PhabricatorCustomField::getObjectFields( 892 + $object, 893 + PhabricatorCustomField::ROLE_APPLICATIONSEARCH); 894 + $fields->readFieldsFromStorage($object); 895 + $fields->rebuildIndexes($object); 896 + } 897 + 898 + $herald_xscript = $this->getHeraldTranscript(); 899 + if ($herald_xscript) { 900 + $herald_header = $herald_xscript->getXHeraldRulesHeader(); 901 + $herald_header = HeraldTranscript::saveXHeraldRulesHeader( 902 + $object->getPHID(), 903 + $herald_header); 904 + } else { 905 + $herald_header = HeraldTranscript::loadXHeraldRulesHeader( 906 + $object->getPHID()); 907 + } 908 + $this->heraldHeader = $herald_header; 909 + 910 + if ($this->supportsWorkers()) { 911 + PhabricatorWorker::scheduleTask( 912 + 'PhabricatorApplicationTransactionPublishWorker', 913 + array( 914 + 'objectPHID' => $object->getPHID(), 915 + 'actorPHID' => $this->getActingAsPHID(), 916 + 'xactionPHIDs' => mpull($xactions, 'getPHID'), 917 + 'state' => $this->getWorkerState(), 918 + ), 919 + array( 920 + 'objectPHID' => $object->getPHID(), 921 + 'priority' => PhabricatorWorker::PRIORITY_ALERTS, 922 + )); 923 + } else { 924 + $this->publishTransactions($object, $xactions); 925 + } 926 + 927 + return $xactions; 928 + } 929 + 930 + public function publishTransactions( 931 + PhabricatorLiskDAO $object, 932 + array $xactions) { 933 + 870 934 // Before sending mail or publishing feed stories, reload the object 871 935 // subscribers to pick up changes caused by Herald (or by other side effects 872 936 // in various transaction phases). ··· 899 963 $object, 900 964 $xactions, 901 965 $mailed); 902 - } 903 - 904 - $this->didApplyTransactions($xactions); 905 - 906 - if ($object instanceof PhabricatorCustomFieldInterface) { 907 - // Maybe this makes more sense to move into the search index itself? For 908 - // now I'm putting it here since I think we might end up with things that 909 - // need it to be up to date once the next page loads, but if we don't go 910 - // there we we could move it into search once search moves to the daemons. 911 - 912 - // It now happens in the search indexer as well, but the search indexer is 913 - // always daemonized, so the logic above still potentially holds. We could 914 - // possibly get rid of this. The major motivation for putting it in the 915 - // indexer was to enable reindexing to work. 916 - 917 - $fields = PhabricatorCustomField::getObjectFields( 918 - $object, 919 - PhabricatorCustomField::ROLE_APPLICATIONSEARCH); 920 - $fields->readFieldsFromStorage($object); 921 - $fields->rebuildIndexes($object); 922 966 } 923 967 924 968 return $xactions; ··· 2043 2087 $email_to = $this->getMailTo($object); 2044 2088 $email_cc = $this->getMailCC($object); 2045 2089 2046 - $adapter = $this->getHeraldAdapter(); 2047 - if ($adapter) { 2048 - $email_cc = array_merge($email_cc, $adapter->getEmailPHIDs()); 2049 - $email_force = $adapter->getForcedEmailPHIDs(); 2050 - } 2090 + $email_cc = array_merge($email_cc, $this->heraldEmailPHIDs); 2091 + $email_force = $this->heraldForcedEmailPHIDs; 2051 2092 2052 2093 $phids = array_merge($email_to, $email_cc); 2053 2094 $handles = id(new PhabricatorHandleQuery()) ··· 2087 2128 $template->addAttachment($attachment); 2088 2129 } 2089 2130 2090 - $herald_xscript = $this->getHeraldTranscript(); 2091 - if ($herald_xscript) { 2092 - $herald_header = $herald_xscript->getXHeraldRulesHeader(); 2093 - $herald_header = HeraldTranscript::saveXHeraldRulesHeader( 2094 - $object->getPHID(), 2095 - $herald_header); 2096 - } else { 2097 - $herald_header = HeraldTranscript::loadXHeraldRulesHeader( 2098 - $object->getPHID()); 2099 - } 2100 - 2101 - if ($herald_header) { 2102 - $template->addHeader('X-Herald-Rules', $herald_header); 2131 + if ($this->heraldHeader) { 2132 + $template->addHeader('X-Herald-Rules', $this->heraldHeader); 2103 2133 } 2104 2134 2105 2135 if ($object instanceof PhabricatorProjectInterface) { ··· 2763 2793 2764 2794 $editor->applyTransactions($target, array($template)); 2765 2795 } 2796 + } 2797 + 2798 + 2799 + /* -( Workers )------------------------------------------------------------ */ 2800 + 2801 + 2802 + /** 2803 + * @task workers 2804 + */ 2805 + protected function supportsWorkers() { 2806 + // TODO: Remove this method once everything supports workers. 2807 + return false; 2808 + } 2809 + 2810 + 2811 + /** 2812 + * Convert the editor state to a serializable dictionary which can be passed 2813 + * to a worker. 2814 + * 2815 + * This data will be loaded with @{method:loadWorkerState} in the worker. 2816 + * 2817 + * @return dict<string, wild> Serializable editor state. 2818 + * @task workers 2819 + */ 2820 + final private function getWorkerState() { 2821 + $state = array(); 2822 + foreach ($this->getAutomaticStateProperties() as $property) { 2823 + $state[$property] = $this->$property; 2824 + } 2825 + 2826 + $state += array( 2827 + 'excludeMailRecipientPHIDs' => $this->getExcludeMailRecipientPHIDs(), 2828 + ); 2829 + 2830 + return $state + array( 2831 + 'custom' => $this->getCustomWorkerState(), 2832 + ); 2833 + } 2834 + 2835 + 2836 + /** 2837 + * Hook; return custom properties which need to be passed to workers. 2838 + * 2839 + * @return dict<string, wild> Custom properties. 2840 + * @task workers 2841 + */ 2842 + protected function getCustomWorkerState() { 2843 + return array(); 2844 + } 2845 + 2846 + 2847 + /** 2848 + * Load editor state using a dictionary emitted by @{method:getWorkerState}. 2849 + * 2850 + * This method is used to load state when running worker operations. 2851 + * 2852 + * @param dict<string, wild> Editor state, from @{method:getWorkerState}. 2853 + * @return this 2854 + * @task workers 2855 + */ 2856 + final public function loadWorkerState(array $state) { 2857 + foreach ($this->getAutomaticStateProperties() as $property) { 2858 + $this->$property = idx($state, $property); 2859 + } 2860 + 2861 + $exclude = idx($state, 'excludeMailRecipientPHIDs', array()); 2862 + $this->setExcludeMailRecipientPHIDs($exclude); 2863 + 2864 + $custom = idx($state, 'custom', array()); 2865 + $this->loadCustomWorkerState($custom); 2866 + 2867 + return $this; 2868 + } 2869 + 2870 + 2871 + /** 2872 + * Hook; set custom properties on the editor from data emitted by 2873 + * @{method:getCustomWorkerState}. 2874 + * 2875 + * @param dict<string, wild> Custom state, 2876 + * from @{method:getCustomWorkerState}. 2877 + * @return this 2878 + * @task workers 2879 + */ 2880 + protected function loadCustomWorkerState(array $state) { 2881 + return $this; 2882 + } 2883 + 2884 + 2885 + /** 2886 + * Get a list of object properties which should be automatically sent to 2887 + * workers in the state data. 2888 + * 2889 + * These properties will be automatically stored and loaded by the editor in 2890 + * the worker. 2891 + * 2892 + * @return list<string> List of properties. 2893 + * @task workers 2894 + */ 2895 + private function getAutomaticStateProperties() { 2896 + return array( 2897 + 'parentMessageID', 2898 + 'disableEmail', 2899 + 'isNewObject', 2900 + 'heraldEmailPHIDs', 2901 + 'heraldForcedEmailPHIDs', 2902 + 'heraldHeader', 2903 + ); 2766 2904 } 2767 2905 2768 2906 }
+134
src/applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php
··· 1 + <?php 2 + 3 + /** 4 + * Performs backgroundable work after applying transactions. 5 + * 6 + * This class handles email, notifications, feed stories, search indexes, and 7 + * other similar backgroundable work. 8 + */ 9 + final class PhabricatorApplicationTransactionPublishWorker 10 + extends PhabricatorWorker { 11 + 12 + 13 + /** 14 + * Publish information about a set of transactions. 15 + */ 16 + protected function doWork() { 17 + $object = $this->loadObject(); 18 + $editor = $this->buildEditor($object); 19 + $xactions = $this->loadTransactions($object); 20 + 21 + $editor->publishTransactions($object, $xactions); 22 + } 23 + 24 + 25 + /** 26 + * Load the object the transactions affect. 27 + */ 28 + private function loadObject() { 29 + $data = $this->getTaskData(); 30 + $viewer = PhabricatorUser::getOmnipotentUser(); 31 + 32 + $phid = idx($data, 'objectPHID'); 33 + if (!$phid) { 34 + throw new PhabricatorWorkerPermanentFailureException( 35 + pht('Task has no object PHID!')); 36 + } 37 + 38 + $object = id(new PhabricatorObjectQuery()) 39 + ->setViewer($viewer) 40 + ->withPHIDs(array($phid)) 41 + ->executeOne(); 42 + if (!$object) { 43 + throw new PhabricatorWorkerPermanentFailureException( 44 + pht( 45 + 'Unable to load object with PHID "%s"!', 46 + $phid)); 47 + } 48 + 49 + return $object; 50 + } 51 + 52 + 53 + /** 54 + * Build and configure an Editor to publish these transactions. 55 + */ 56 + private function buildEditor( 57 + PhabricatorApplicationTransactionInterface $object) { 58 + $data = $this->getTaskData(); 59 + 60 + $daemon_source = PhabricatorContentSource::newForSource( 61 + PhabricatorContentSource::SOURCE_DAEMON, 62 + array()); 63 + 64 + $viewer = PhabricatorUser::getOmnipotentUser(); 65 + $editor = $object->getApplicationTransactionEditor() 66 + ->setActor($viewer) 67 + ->setContentSource($daemon_source) 68 + ->setActingAsPHID(idx($data, 'actorPHID')) 69 + ->loadWorkerState(idx($data, 'state', array())); 70 + 71 + return $editor; 72 + } 73 + 74 + 75 + /** 76 + * Load the transactions to be published. 77 + */ 78 + private function loadTransactions( 79 + PhabricatorApplicationTransactionInterface $object) { 80 + $data = $this->getTaskData(); 81 + 82 + $xaction_phids = idx($data, 'xactionPHIDs'); 83 + if (!$xaction_phids) { 84 + throw new PhabricatorWorkerPermanentFailureException( 85 + pht('Task has no transaction PHIDs!')); 86 + } 87 + 88 + $viewer = PhabricatorUser::getOmnipotentUser(); 89 + 90 + $type = phid_get_subtype(head($xaction_phids)); 91 + $xactions = $this->buildTransactionQuery($type) 92 + ->setViewer($viewer) 93 + ->withPHIDs($xaction_phids) 94 + ->needComments(true) 95 + ->execute(); 96 + $xactions = mpull($xactions, null, 'getPHID'); 97 + 98 + $missing = array_diff($xaction_phids, array_keys($xactions)); 99 + if ($missing) { 100 + throw new PhabricatorWorkerPermanentFailureException( 101 + pht( 102 + 'Unable to load transactions: %s.', 103 + implode(', ', $missing))); 104 + } 105 + 106 + return array_select_keys($xactions, $xaction_phids); 107 + } 108 + 109 + 110 + /** 111 + * Build a new transaction query of the appropriate class so we can load 112 + * the transactions. 113 + */ 114 + private function buildTransactionQuery($type) { 115 + $queries = id(new PhutilSymbolLoader()) 116 + ->setAncestorClass('PhabricatorApplicationTransactionQuery') 117 + ->loadObjects(); 118 + 119 + foreach ($queries as $query) { 120 + $query_type = $query 121 + ->getTemplateApplicationTransaction() 122 + ->getApplicationTransactionType(); 123 + if ($query_type == $type) { 124 + return $query; 125 + } 126 + } 127 + 128 + throw new PhabricatorWorkerPermanentFailureException( 129 + pht( 130 + 'Unable to load query for transaction type "%s"!', 131 + $type)); 132 + } 133 + 134 + }