Skip to content

Commit

Permalink
Improve hooks and allow to delete/check specific priority (#133)
Browse files Browse the repository at this point in the history 
* Improve hooks and allow to delete/check specific priority

* Fix tests

* Introduce indexes hooks
  • Loading branch information
@mvorisek
mvorisek authored Mar 21, 2020
1 parent 8707241 commit 3615739
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 61 deletions.
2 changes: 1 addition & 1 deletion examples/test2.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public function doWork()
}

$c = new MyClass();
$c->addHook('afterWork', function () {
$c->onHook('afterWork', function () {
echo "HOOKed on work\n";
});
$c->doWork();
2 changes: 1 addition & 1 deletion src/DynamicMethodTrait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ trait DynamicMethodTrait
public function __call(string $method, $args)
{
if ($ret = $this->tryCall($method, $args)) {
return $ret[0];
return reset($ret);
}

throw new Exception([
Expand Down
124 changes: 80 additions & 44 deletions src/HookTrait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,43 +18,50 @@ trait HookTrait
*/
protected $hooks = [];

/**
* Next hook index counter.
*
* @var int
*/
private $_hookIndexCounter = 0;

/**
* @deprecated use onHook instead
*/
public function addHook($spot, $fx, $args = null, $priority = null)
public function addHook($spot, $fx, array $args = null, int $priority = null)
{
return $this->onHook(...func_get_args());
return $this->onHook($spot, $fx, $args, $priority ?? 0);
}

/**
* Add another callback to be executed during hook($hook_spot);.
*
* If priority is negative, then hooks will be executed in reverse order.
*
* @param string $spot Hook identifier to bind on
* @param object|callable $fx Will be called on hook()
* @param array $args Arguments are passed to $fx
* @param int $priority Lower priority is called sooner
* @param string|string[] $spot Hook identifier to bind on
* @param object|callable|null $fx Will be called on hook()
* @param array $args Arguments are passed to $fx
* @param int $priority Lower priority is called sooner
*
* @return $this
* @return int|int[] Index under which the hook was added
*/
public function onHook($spot, $fx = null, $args = [], $priority = 5)
public function onHook($spot, $fx = null, array $args = [], int $priority = 5)
{
$fx = $fx ?: $this;

$args = (array) $args;

// multiple hooks can be linked
if (is_string($spot) && strpos($spot, ',') !== false) {
$spot = explode(',', $spot);
}
if (is_array($spot)) {
foreach ($spot as $h) {
$this->onHook($h, $fx, $args, $priority);
$indexes = [];
foreach ($spot as $k => $h) {
$indexes[$k] = $this->onHook($h, $fx, $args, $priority);
}

return $this;
return $indexes;
}
$spot = (string) $spot;

// short for onHook('test', $this); to call $this->test();
if (!is_callable($fx)) {
Expand All @@ -77,39 +84,71 @@ public function onHook($spot, $fx = null, $args = [], $priority = 5)
$this->hooks[$spot][$priority] = [];
}

if ($priority >= 0) {
$this->hooks[$spot][$priority][] = [$fx, $args];
$index = $this->_hookIndexCounter++;
$data = [$fx, $args];
if ($priority < 0) {
$this->hooks[$spot][$priority] = [$index => $data] + $this->hooks[$spot][$priority];
} else {
array_unshift($this->hooks[$spot][$priority], [$fx, $args]);
$this->hooks[$spot][$priority][$index] = $data;
}

return $this;
return $index;
}

/**
* Delete all hooks for specified spot.
* Delete all hooks for specified spot, priority and index.
*
* @param string $spot Hook identifier to bind on
* @param string $spot Hook identifier
* @param int|null $priority Filter specific priority, null for all
* @param int|null $priorityIsIndex Filter by index instead of priority
*
* @return $this
* @return static
*/
public function removeHook($spot)
public function removeHook(string $spot, int $priority = null, bool $priorityIsIndex = false)
{
unset($this->hooks[$spot]);
if ($priority !== null) {
if ($priorityIsIndex) {
$index = $priority;
foreach (array_keys($this->hooks[$spot]) as $priority) {
unset($this->hooks[$spot][$priority][$index]);
}
} else {
unset($this->hooks[$spot][$priority]);
}
} else {
unset($this->hooks[$spot]);
}

return $this;
}

/**
* Returns true if at least one callback is defined for this hook.
*
* @param string $spot Hook identifier
*
* @return bool
* @param string $spot Hook identifier
* @param int|null $priority Filter specific priority, null for all
* @param int|null $priorityIsIndex Filter by index instead of priority
*/
public function hookHasCallbacks($spot)
public function hookHasCallbacks(string $spot, int $priority = null, bool $priorityIsIndex = false): bool
{
return isset($this->hooks[$spot]);
if (!isset($this->hooks[$spot])) {
return false;
} elseif ($priority === null) {
return true;
}

if ($priorityIsIndex) {
$index = $priority;
foreach (array_keys($this->hooks[$spot]) as $priority) {
if (isset($this->hooks[$spot][$priority][$index])) {
return true;
}
}

return false;
}

return isset($this->hooks[$spot][$priority]);
}

/**
Expand All @@ -120,24 +159,20 @@ public function hookHasCallbacks($spot)
*
* @throws Exception
*
* @return mixed Array of responses or value specified to breakHook
* @return mixed Array of responses indexed by hook indexes or value specified to breakHook
*/
public function hook($spot, $args = null)
public function hook(string $spot, array $args = [])
{
$args = (array) $args;

$return = [];

try {
if (
isset($this->hooks[$spot])
&& is_array($this->hooks[$spot])
) {
krsort($this->hooks[$spot]); // lower priority is called sooner
$hookBackup = $this->hooks[$spot];
if (isset($this->hooks[$spot])) {
krsort($this->hooks[$spot]); // lower priority is called sooner
$hookBackup = $this->hooks[$spot];

try {
while ($_data = array_pop($this->hooks[$spot])) {
foreach ($_data as &$data) {
$return[] = call_user_func_array(
foreach ($_data as $index => &$data) {
$return[$index] = call_user_func_array(
$data[0],
array_merge(
[$this],
Expand All @@ -147,13 +182,14 @@ public function hook($spot, $args = null)
);
}
}
unset($data);

$this->hooks[$spot] = $hookBackup;
}
} catch (HookBreaker $e) {
$this->hooks[$spot] = $hookBackup;
} catch (HookBreaker $e) {
$this->hooks[$spot] = $hookBackup;

return $e->return_value;
return $e->return_value;
}
}

return $return;
Expand Down
41 changes: 26 additions & 15 deletions tests/HookTraitTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ public function testArguments()
$result = 0;
$m->onHook('test1', function () use (&$result) {
$result++;
}, 0);
}, [0]);

$this->assertEquals(0, $result);

$m->onHook('test1', function () use (&$result) {
$result++;
}, 5);
}, [5]);

$this->assertEquals(0, $result);
}
Expand Down Expand Up @@ -59,7 +59,7 @@ public function testAdvanced()

$m->onHook('test1', function () use (&$result) {
$result = 0;
}, null, 1);
}, [], 1);

$m->hook('test1'); // zero will be executed first, then increment
$this->assertEquals(1, $result);
Expand Down Expand Up @@ -152,51 +152,62 @@ public function testHookException1()
$result .= $arg;
});

$m->hook('tst', 'parameter');
$m->hook('tst', ['parameter']);

$this->assertEquals('parameter', $result);
}

public function testOrder()
{
$m = new HookMock();
$m->onHook('spot', function () {
$ind = $m->onHook('spot', function () {
return 3;
}, null, -1);
}, [], -1);
$m->onHook('spot', function () {
return 2;
}, null, -5);
}, [], -5);
$m->onHook('spot', function () {
return 1;
}, null, -5);
}, [], -5);

$m->onHook('spot', function () {
return 4;
}, null, 0);
}, [], 0);
$m->onHook('spot', function () {
return 5;
}, null, 0);
}, [], 0);

$m->onHook('spot', function () {
return 10;
}, null, 1000);
}, [], 1000);

$m->onHook('spot', function () {
return 6;
}, null, 2);
}, [], 2);
$m->onHook('spot', function () {
return 7;
}, null, 5);
}, [], 5);
$m->onHook('spot', function () {
return 8;
});
$m->onHook('spot', function () {
return 9;
}, null, 5);
}, [], 5);

$ret = $m->hook('spot');

$this->assertEquals([1, 2, 3, 4, 5, 6, 7, 8, 9, 10], $ret);
$this->assertEquals([
$ind + 2 => 1,
$ind + 1 => 2,
$ind => 3,
$ind + 3 => 4,
$ind + 4 => 5,
$ind + 6 => 6,
$ind + 7 => 7,
$ind + 8 => 8,
$ind + 9 => 9,
$ind + 5 => 10,
], $ret);
}

public function testMulti()
Expand Down

2 comments on commit 3615739

@georgehristov
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any background for removing the flexibility of $args and setting it firmly as array?

@mvorisek
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@georgehristov Yes, we want to enforce one canonical form of usage where appropriate for more type checking and to prevent misuses - I have analysed this deeply using AST of atk4/core, atk4/data and atk4/ui packages - and the only of non-array arguments I found was null when the additional arg - priority was set. This is very bad if [] means the same and it makes more sense - zero args.

Please sign in to comment.