Page MenuHomePhabricator
Files F13181885

D10624.id25505.diff
No OneTemporary
Actions

D10624.id25505.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -815,6 +815,7 @@
'HeraldRulePHIDType' => 'applications/herald/phid/HeraldRulePHIDType.php',
'HeraldRuleQuery' => 'applications/herald/query/HeraldRuleQuery.php',
'HeraldRuleSearchEngine' => 'applications/herald/query/HeraldRuleSearchEngine.php',
+ 'HeraldRuleTestCase' => 'applications/herald/storage/__tests__/HeraldRuleTestCase.php',
'HeraldRuleTransaction' => 'applications/herald/storage/HeraldRuleTransaction.php',
'HeraldRuleTransactionComment' => 'applications/herald/storage/HeraldRuleTransactionComment.php',
'HeraldRuleTranscript' => 'applications/herald/storage/transcript/HeraldRuleTranscript.php',
@@ -3712,6 +3713,7 @@
'HeraldRulePHIDType' => 'PhabricatorPHIDType',
'HeraldRuleQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'HeraldRuleSearchEngine' => 'PhabricatorApplicationSearchEngine',
+ 'HeraldRuleTestCase' => 'PhabricatorTestCase',
'HeraldRuleTransaction' => 'PhabricatorApplicationTransaction',
'HeraldRuleTransactionComment' => 'PhabricatorApplicationTransactionComment',
'HeraldRuleViewController' => 'HeraldController',
diff --git a/src/applications/herald/engine/HeraldEngine.php b/src/applications/herald/engine/HeraldEngine.php
--- a/src/applications/herald/engine/HeraldEngine.php
+++ b/src/applications/herald/engine/HeraldEngine.php
@@ -49,6 +49,8 @@
assert_instances_of($rules, 'HeraldRule');
$t_start = microtime(true);
+ // Rules execute in a well-defined order: sort them into execution order.
+ $rules = msort($rules, 'getRuleExecutionOrderSortKey');
$rules = mpull($rules, null, 'getPHID');
$this->transcript = new HeraldTranscript();
diff --git a/src/applications/herald/storage/HeraldRule.php b/src/applications/herald/storage/HeraldRule.php
--- a/src/applications/herald/storage/HeraldRule.php
+++ b/src/applications/herald/storage/HeraldRule.php
@@ -228,6 +228,42 @@
return $this->assertAttached($this->triggerObject);
}
+ /**
+ * Get a sortable key for rule execution order.
+ *
+ * Rules execute in a well-defined order: personal rules first, then object
+ * rules, then global rules. Within each rule type, rules execute from lowest
+ * ID to highest ID.
+ *
+ * This ordering allows more powerful rules (like global rules) to override
+ * weaker rules (like personal rules) when multiple rules exist which try to
+ * affect the same field. Executing from low IDs to high IDs makes
+ * interactions easier to understand when adding new rules, because the newest
+ * rules always happen last.
+ *
+ * @return string A sortable key for this rule.
+ */
+ public function getRuleExecutionOrderSortKey() {
+
+ $rule_type = $this->getRuleType();
+
+ switch ($rule_type) {
+ case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
+ $type_order = 1;
+ break;
+ case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
+ $type_order = 2;
+ break;
+ case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
+ $type_order = 3;
+ break;
+ default:
+ throw new Exception(pht('Unknown rule type "%s"!', $rule_type));
+ }
+
+ return sprintf('~%d%010d', $type_order, $this->getID());
+ }
+
/* -( PhabricatorPolicyInterface )----------------------------------------- */
diff --git a/src/applications/herald/storage/__tests__/HeraldRuleTestCase.php b/src/applications/herald/storage/__tests__/HeraldRuleTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/applications/herald/storage/__tests__/HeraldRuleTestCase.php
@@ -0,0 +1,41 @@
+<?php
+
+final class HeraldRuleTestCase extends PhabricatorTestCase {
+
+ public function testHeraldRuleExecutionOrder() {
+ $rules = array(
+ 1 => HeraldRuleTypeConfig::RULE_TYPE_GLOBAL,
+ 2 => HeraldRuleTypeConfig::RULE_TYPE_GLOBAL,
+ 3 => HeraldRuleTypeConfig::RULE_TYPE_OBJECT,
+ 4 => HeraldRuleTypeConfig::RULE_TYPE_PERSONAL,
+ 5 => HeraldRuleTypeConfig::RULE_TYPE_GLOBAL,
+ 6 => HeraldRuleTypeConfig::RULE_TYPE_PERSONAL,
+ );
+
+ foreach ($rules as $id => $type) {
+ $rules[$id] = id(new HeraldRule())
+ ->setID($id)
+ ->setRuleType($type);
+ }
+
+ shuffle($rules);
+ $rules = msort($rules, 'getRuleExecutionOrderSortKey');
+ $this->assertEqual(
+ array(
+ // Personal
+ 4,
+ 6,
+
+ // Object
+ 3,
+
+ // Global
+ 1,
+ 2,
+ 5
+ ),
+ array_values(mpull($rules, 'getID')));
+ }
+
+
+}

File Metadata

Mime Type
text/plain
Expires
Fri, May 10, 4:33 PM (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6283784
Default Alt Text
D10624.id25505.diff (4 KB)

Event Timeline

Phabricator · Built by Phacility