[5.7] Add Model::withoutEventDispatcher() method to temporarily bypass model events

[jessarcher]

It is sometimes desirable to temporarily bypass model events. For example, when seeding - imagine your model has a created listener and a saved listener. You are trying to test the saved listener, but the created listener is also firing when seeding the data for the test.

While it is already possible to do this, the code is verbose and requires either muddying up the test with implementation details, or by creating a base model or trait to add the functionality directly to the model (my current workaround). This aims to simplify that by creating an intuitively named helper method to execute a callback with the event dispatcher temporarily disabled, returning the result of the callback.

As this is a new method, it should not break anything. It just enhances the existing API around getting, setting, and unsetting the event dispatcher.

Before:

$dispatcher = User::getEventDispatcher();
User::unsetEventDispatcher();
$user = factory(User::class)->create();
User::setEventDispatcher($dispatcher);

$user->doSomethingWhereEventsShouldFire();

After:

$user = User::withoutEventDispatcher(function () {
    return factory(User::class)->create();
});

$user->doSomethingWhereEventsShouldFire();

I realise that a similar PR was rejected in the past, but I feel this PR is different for the following reasons:

1. The method name matches the existing Model::unsetEventDispatcher() API so I believe it achieves the same clarity about what it is doing.
2. The use case seems quite common.
3. This method allows you to return the result of the callback
4. This method handles the situation where an event dispatcher was never defined
5. This PR has tests

[mfn]

I like this, I'm using a not-so-elegant version for the test suite:

    /**
     * Saves the given models without any events
     *
     * The event disable/enable logic is from
     * https://laravel.io/forum/09-14-2014-turn-off-eloquent-events?page=1#reply-29610
     *
     * @param Model $model
     * @return void
     */
    protected function modelSaveNoEvents(Model $model): void
    {
        // This is trick for the IDE to resolve the static functions below
        // and technically not necessary…
        $modelName = $model;
        // …and this will always succeed
        if ($model instanceof Model) {
            $modelName = \get_class($model);
        }

        $dispatcher = $modelName::getEventDispatcher();
        $modelName::unsetEventDispatcher();

        $model->save();

        $modelName::setEventDispatcher($dispatcher);
    }

I've never had a use-case outside ->save() so that's why it's hardcoded but I can see that the with* approach makes sense though it's a bit verbose.

Wish it would be possible do to something similar to the higher order proxy: $model->withoutEvents->update()

One thing I'm not sure if we need this functionality on the core model or rather just a helper somewhere else?

[jessarcher]

Thanks for the feedback. I can think of other reasons to temporarily disable this outside of tests so it would be good if the solution wasn't test-specific.

The higher-order proxy method would be nice, but it's probably overkill for this. I see this as just a small natural extension of the existing getEventDispatcher(), setEventDispatcher(), and unsetEventDispatcher() methods on the HasEvents trait. It felt natural enough that I actually expected it to already be there and was surprised when it wasn't. I don't expect it has enough use cases to be in the documentation or anything though.

[mfn]

Sounds reasonable to me 👍

[derekmd]

Code formatting could be shortened to:

    public static function withoutEventDispatcher(callable $callback)
    {
        $dispatcher = static::getEventDispatcher();

        static::unsetEventDispatcher();

        try {            
            return $callback();
        } finally {
            static::setEventDispatcher($dispatcher);
        }
    }

[jessarcher]

Good call on removing the temp variable - I didn't realise finally would still be executed after a return. Thanks for teaching me something new!

Regarding the if condition - the reason I added this is because setEventDispatcher() requires a valid Dispatcher and it's possible that the user didn't actually have an event dispatcher set.

You might say that someone shouldn't use this method if they don't have an event dispatcher set, but I guess it would be nice to be able to call the method without having to worry about that.

[taylorotwell]

Is there going to be confusion on how this applies to models? People may not realize it's a static variable and to do $model->withoutModelEvents() is to actually disable events for all models. They may only expect it to work for the specific model instance. Thoughts?

[jessarcher]

Fair question, and the answer is probably yes. However, from my perspective I don't think it's any different to the existing *EventDispatcher() methods. They're quite technically-named and not so easily stumbled upon, so I'd hope those reaching for them would do so deliberately and see what they are doing.

It could be helpful to add a comment to each of these methods to clarify it though.

[derekmd]

Since it's mainly for test data setup, maybe locating the method as TestCase@withoutModelEvents() would be more appropriate?

[jessarcher]

That would certainly be less ambiguous and the location still feels intuitive and discoverable. It would also fulfil my own current use case, but it's not too hard to imagine other scenarios where it wouldn't. I don't think it feels quite as tidy and encapsulated as co-locating it with the other [get|[un]set]EventDispatcher() methods though.

If the use case is common enough, I could see Model::withoutEventDispatcher() being more of an under-the-hood public-but-undocumented method (like the others), and a TestCase@withoutModelEvents() method that is encouraged and perhaps documented, which just calls Model::withoutEventDispatcher() behind the scenes?

[robclancy]

I've wanted this forever :D laravel/ideas#344

[ludo237]

@robclancy yea I can finally get rid of those awful methods in my TestCase file for each model 😆

[emielmolenaar]

This is very useful! Thank you very much guys. 🍻

[pmochine]

Btw I don't know why, but for me the method is named: withoutEvents (if someone is wondering why you get Call to undefined method App\Order::withoutEventDispatcher())

[mfn]

That's correct, it was changed afterwards.