1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-24 06:20:56 +01:00

Make execution order of Herald rules explicit

Summary: Fixes T6211. This gives Herald rules an explicit execution order, which seems generally good. See some discussion on T6211 and inline.

Test Plan:
  - Added unit test.
  - Dry ran rules and saw rules appear in the expected order in the transcript.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6211

Differential Revision: https://secure.phabricator.com/D10624
This commit is contained in:
epriestley 2014-10-02 09:49:32 -07:00
parent 3c6781b177
commit 5f82805068
4 changed files with 81 additions and 0 deletions

View file

@ -815,6 +815,7 @@ phutil_register_library_map(array(
'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 @@ phutil_register_library_map(array(
'HeraldRulePHIDType' => 'PhabricatorPHIDType',
'HeraldRuleQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'HeraldRuleSearchEngine' => 'PhabricatorApplicationSearchEngine',
'HeraldRuleTestCase' => 'PhabricatorTestCase',
'HeraldRuleTransaction' => 'PhabricatorApplicationTransaction',
'HeraldRuleTransactionComment' => 'PhabricatorApplicationTransactionComment',
'HeraldRuleViewController' => 'HeraldController',

View file

@ -49,6 +49,8 @@ final class HeraldEngine {
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();

View file

@ -228,6 +228,42 @@ final class HeraldRule extends HeraldDAO
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 )----------------------------------------- */

View file

@ -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')));
}
}