Skip to content

Commit

Permalink
Remove event priorities.
Browse files Browse the repository at this point in the history 
The whole concept of event priorities doesn’t make sense as a concept
and can’t even be trusted as an end user because you are relying on the
exact order of listeners. Priority also makes no sense in modern apps
where many events may be queued. Can be implemented as a synchronous
pipeline. Also removed halting events as this suffers from same problem
of relying on the exact order and implementation of your events,
depending on no handlers being queued, etc. Should use other pattern
when this is needed.
  • Loading branch information
@taylorotwell
taylorotwell committed Jan 10, 2017
1 parent 0f76617 commit dbbfc62
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 179 deletions.
22 changes: 2 additions & 20 deletions src/Illuminate/Contracts/Events/Dispatcher.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@ interface Dispatcher
*
* @param string|array $events
* @param mixed $listener
* @param int $priority
* @return void
*/
public function listen($events, $listener, $priority = 0);
public function listen($events, $listener);

/**
* Determine if a given event has listeners.
Expand All @@ -39,15 +38,6 @@ public function push($event, $payload = []);
*/
public function subscribe($subscriber);

/**
* Fire an event until the first non-null response is returned.
*
* @param string $event
* @param array $payload
* @return mixed
*/
public function until($event, $payload = []);

/**
* Flush a set of pushed events.
*
Expand All @@ -61,17 +51,9 @@ public function flush($event);
*
* @param string|object $event
* @param mixed $payload
* @param bool $halt
* @return array|null
*/
public function dispatch($event, $payload = [], $halt = false);

/**
* Get the event that is currently firing.
*
* @return string
*/
public function dispatching();
public function dispatch($event, $payload = []);

/**
* Remove a set of listeners from the dispatcher.
Expand Down
161 changes: 41 additions & 120 deletions src/Illuminate/Events/Dispatcher.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,6 @@ class Dispatcher implements DispatcherContract
*/
protected $wildcards = [];

/**
* The sorted event listeners.
*
* @var array
*/
protected $sorted = [];

/**
* The event firing stack.
*
* @var array
*/
protected $firing = [];

/**
* The queue resolver instance.
*
Expand All @@ -71,18 +57,15 @@ public function __construct(ContainerContract $container = null)
*
* @param string|array $events
* @param mixed $listener
* @param int $priority
* @return void
*/
public function listen($events, $listener, $priority = 0)
public function listen($events, $listener)
{
foreach ((array) $events as $event) {
if (Str::contains($event, '*')) {
$this->setupWildcardListen($event, $listener);
} else {
$this->listeners[$event][$priority][] = $this->makeListener($listener);

unset($this->sorted[$event]);
$this->listeners[$event][] = $this->makeListener($event, $listener);
}
}
}
Expand All @@ -96,7 +79,7 @@ public function listen($events, $listener, $priority = 0)
*/
protected function setupWildcardListen($event, $listener)
{
$this->wildcards[$event][] = $this->makeListener($listener);
$this->wildcards[$event][] = $this->makeListener($event, $listener, true);
}

/**
Expand Down Expand Up @@ -152,18 +135,6 @@ protected function resolveSubscriber($subscriber)
return $subscriber;
}

/**
* Fire an event until the first non-null response is returned.
*
* @param string|object $event
* @param array $payload
* @return mixed
*/
public function until($event, $payload = [])
{
return $this->fire($event, $payload, true);
}

/**
* Flush a set of pushed events.
*
Expand All @@ -175,48 +146,26 @@ public function flush($event)
$this->fire($event.'_pushed');
}

/**
* Get the event that is currently firing.
*
* @return string
*/
public function dispatching()
{
return $this->firing();
}

/**
* Get the event that is currently firing.
*
* @return string
*/
public function firing()
{
return last($this->firing);
}

/**
* Fire an event and call the listeners.
*
* @param string|object $event
* @param mixed $payload
* @param bool $halt
* @return array|null
*/
public function dispatch($event, $payload = [], $halt = false)
public function dispatch($event, $payload = [])
{
return $this->fire($event, $payload, $halt);
return $this->fire($event, $payload);
}

/**
* Fire an event and call the listeners.
*
* @param string|object $event
* @param mixed $payload
* @param bool $halt
* @return array|null
*/
public function fire($event, $payload = [], $halt = false)
public function fire($event, $payload = [])
{
// When the given "event" is actually an object we will assume it is an event
// object and use the class as the event name and this event itself as the
Expand All @@ -225,25 +174,14 @@ public function fire($event, $payload = [], $halt = false)
$event, $payload
);

$responses = [];

$this->firing[] = $event;

if ($this->shouldBroadcast($payload)) {
$this->broadcastEvent($payload[0]);
}

foreach ($this->getListeners($event) as $listener) {
$response = call_user_func_array($listener, $payload);

// If a response is returned from the listener and event halting is enabled
// we will just return this response, and not call the rest of the event
// listeners. Otherwise we will add the response on the response list.
if (! is_null($response) && $halt) {
array_pop($this->firing);
$responses = [];

return $response;
}
foreach ($this->getListeners($event) as $listener) {
$response = $listener($event, $payload);

// If a boolean false is returned from a listener, we will stop propagating
// the event to any further listeners down in the chain, else we keep on
Expand All @@ -255,9 +193,7 @@ public function fire($event, $payload = [], $halt = false)
$responses[] = $response;
}

array_pop($this->firing);

return $halt ? null : $responses;
return $responses;
}

/**
Expand Down Expand Up @@ -306,13 +242,15 @@ protected function broadcastEvent($event)
*/
public function getListeners($eventName)
{
$wildcards = $this->getWildcardListeners($eventName);
$listeners = isset($this->listeners[$eventName]) ? $this->listeners[$eventName] : [];

if (! isset($this->sorted[$eventName])) {
$this->sortListeners($eventName);
}
$listeners = array_merge(
$listeners, $this->getWildcardListeners($eventName)
);

return array_merge($this->sorted[$eventName], $wildcards);
return class_exists($eventName, false)
? $this->addInterfaceListeners($eventName, $listeners)
: $listeners;
}

/**
Expand All @@ -334,33 +272,6 @@ protected function getWildcardListeners($eventName)
return $wildcards;
}

/**
* Sort the listeners for a given event by priority.
*
* @param string $eventName
* @return void
*/
protected function sortListeners($eventName)
{
// If listeners exist for the given event, we will sort them by the priority
// so that we can call them in the correct order. We will cache off these
// sorted event listeners so we do not have to re-sort on every events.
$listeners = isset($this->listeners[$eventName])
? $this->listeners[$eventName] : [];

if (class_exists($eventName, false)) {
$listeners = $this->addInterfaceListeners($eventName, $listeners);
}

if ($listeners) {
krsort($listeners);

$this->sorted[$eventName] = call_user_func_array('array_merge', $listeners);
} else {
$this->sorted[$eventName] = [];
}
}

/**
* Add the listeners for the event's interfaces to the given array.
*
Expand All @@ -372,12 +283,8 @@ protected function addInterfaceListeners($eventName, array $listeners = [])
{
foreach (class_implements($eventName) as $interface) {
if (isset($this->listeners[$interface])) {
foreach ($this->listeners[$interface] as $priority => $names) {
if (isset($listeners[$priority])) {
$listeners[$priority] = array_merge($listeners[$priority], $names);
} else {
$listeners[$priority] = $names;
}
foreach ($this->listeners[$interface] as $names) {
$listeners = array_merge($listeners, (array) $names);
}
}
}
Expand All @@ -391,9 +298,19 @@ protected function addInterfaceListeners($eventName, array $listeners = [])
* @param string|\Closure $listener
* @return mixed
*/
public function makeListener($listener)
public function makeListener($event, $listener, $wildcard = false)
{
return is_string($listener) ? $this->createClassListener($listener) : $listener;
if (is_string($listener)) {
return $this->createClassListener($listener, $wildcard);
}

return function ($event, $payload) use ($listener, $wildcard) {
if ($wildcard) {
return $listener($event, $payload);
} else {
return $listener(...array_values($payload));
}
};
}

/**
Expand All @@ -402,12 +319,16 @@ public function makeListener($listener)
* @param string $listener
* @return \Closure
*/
public function createClassListener($listener)
public function createClassListener($listener, $wildcard = false)
{
return function () use ($listener) {
return call_user_func_array(
$this->createClassCallable($listener), func_get_args()
);
return function ($event, $payload) use ($listener, $wildcard) {
if ($wildcard) {
return call_user_func($this->createClassCallable($listener), $event, $payload);
} else {
return call_user_func_array(
$this->createClassCallable($listener), $payload
);
}
};
}

Expand Down Expand Up @@ -533,7 +454,7 @@ public function forget($event)
if (Str::contains($event, '*')) {
unset($this->wildcards[$event]);
} else {
unset($this->listeners[$event], $this->sorted[$event]);
unset($this->listeners[$event]);
}
}

Expand Down
28 changes: 2 additions & 26 deletions src/Illuminate/Support/Testing/Fakes/EventFake.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,9 @@ protected function mapEventArguments($arguments)
*
* @param string|array $events
* @param mixed $listener
* @param int $priority
* @return void
*/
public function listen($events, $listener, $priority = 0)
public function listen($events, $listener)
{
//
}
Expand Down Expand Up @@ -144,18 +143,6 @@ public function subscribe($subscriber)
//
}

/**
* Fire an event until the first non-null response is returned.
*
* @param string $event
* @param array $payload
* @return mixed
*/
public function until($event, $payload = [])
{
return $this->dispatch($event, $payload, true);
}

/**
* Flush a set of pushed events.
*
Expand All @@ -172,26 +159,15 @@ public function flush($event)
*
* @param string|object $event
* @param mixed $payload
* @param bool $halt
* @return array|null
*/
public function dispatch($event, $payload = [], $halt = false)
public function dispatch($event, $payload = [])
{
$name = is_object($event) ? get_class($event) : (string) $event;

$this->events[$name][] = func_get_args();
}

/**
* Get the event that is currently dispatching.
*
* @return string
*/
public function dispatching()
{
//
}

/**
* Remove a set of listeners from the dispatcher.
*
Expand Down
Loading

0 comments on commit dbbfc62

Please sign in to comment.