Skip to content

Commit

Permalink
Fix multiple listeners being attached to models (#192)
Browse files Browse the repository at this point in the history
Fixes wintercms/winter#1251 by replacing all instances of bindEvent inside a Laravel Model Event by bindEventOnce.

There remains an edge case if the event gets cancelled by another listener with a higher priority.

> Using bindEventOnce would fix this issue in most cases, but there's one I can think of where it would fail: if someone registers a handler for model.afterSave with more priority than the default, and this handler sometimes returns false.
> In this case, let's imagine we save the model a first time: we bind the afterSave() method once; we fire the event; it gets cancelled by the other handler; so we don't call the afterSave() method.
> Then we save a second time: we bind afterSave() again (we now have 2); we fire the event; for some reason the other handler does not return false; we call afterSave() twice.
  • Loading branch information
Fl0Cri authored Nov 20, 2024
1 parent 808fd93 commit 626409c
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/Database/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ protected function bootNicerEvents()
if ($model->methodExists($method)) {
// Register the method as a listener with default priority
// to allow for complete control over the execution order
$model->bindEvent('model.' . $method, [$model, $method]);
$model->bindEventOnce('model.' . $method, [$model, $method]);
}
// First listener that returns a non-null result will cancel the
// further propagation of the event; If that result is false, the
Expand Down
4 changes: 2 additions & 2 deletions src/Database/Traits/SoftDelete.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public static function bootSoftDelete()
if ($model->methodExists('beforeRestore')) {
// Register the method as a listener with default priority
// to allow for complete control over the execution order
$model->bindEvent('model.beforeRestore', [$model, 'beforeRestore']);
$model->bindEventOnce('model.beforeRestore', [$model, 'beforeRestore']);
}
/**
* @event model.beforeRestore
Expand All @@ -54,7 +54,7 @@ public static function bootSoftDelete()
if ($model->methodExists('afterRestore')) {
// Register the method as a listener with default priority
// to allow for complete control over the execution order
$model->bindEvent('model.afterRestore', [$model, 'afterRestore']);
$model->bindEventOnce('model.afterRestore', [$model, 'afterRestore']);
}
/**
* @event model.afterRestore
Expand Down
4 changes: 2 additions & 2 deletions src/Database/Traits/Validation.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public static function bootValidation()
if ($model->methodExists('beforeValidate')) {
// Register the method as a listener with default priority
// to allow for complete control over the execution order
$model->bindEvent('model.beforeValidate', [$model, 'beforeValidate']);
$model->bindEventOnce('model.beforeValidate', [$model, 'beforeValidate']);
}

/**
Expand All @@ -87,7 +87,7 @@ public static function bootValidation()
if ($model->methodExists('afterValidate')) {
// Register the method as a listener with default priority
// to allow for complete control over the execution order
$model->bindEvent('model.afterValidate', [$model, 'afterValidate']);
$model->bindEventOnce('model.afterValidate', [$model, 'afterValidate']);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Halcyon/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ protected function bootNicerEvents()
if ($model->methodExists($method)) {
// Register the method as a listener with default priority
// to allow for complete control over the execution order
$model->bindEvent('model.' . $method, [$model, $method]);
$model->bindEventOnce('model.' . $method, [$model, $method]);
}
// First listener that returns a non-null result will cancel the
// further propagation of the event; If that result is false, the
Expand Down

0 comments on commit 626409c

Please sign in to comment.