Use of getOriginal() on model in observer doesn't return original value, when using transactions and ShouldHandleEventsAfterCommit.

[emil-nasso]

Laravel Version

11.20

PHP Version

8.3

Database Driver & Version

sqlite

Description

When using and observer to subscribe to updated events on models, you can access the original value in a few different ways, including the getOriginal(...) method.

If the observer implements the ShouldHandleEventsAfterCommit and the changes to the model is wrapped in a databas transaction, the events should be processed after the transaction has commited. This works as expected.

However. When using transactions and ShouldHandleEventsAfterCommit the getOriginal(...) method no longer returns the original value.

I did not expect this, and can't find anything about it in the documentation. Is this a bug?

Steps To Reproduce

Below are test cases for this. It consists of:

• A simple model.
• An observer that observes the model and, on updated, asserts that the value of $model->test is updated and the value of $model->getOriginal('test') is original
• Two test-cases, one with transactions and one without. The cases both create a model with the field test set to original and then updates that field to updated.

I would expect both of these test cases to pass, but the test case that uses transactions fails.

<?php

namespace Tests;

use Illuminate\Database\Eloquent\Model;

class Test extends Model
{
    protected $connection = 'sqlite';

    protected $table = 'test';

    public $timestamps = false;

    protected $fillable = ['test'];
}

<?php

namespace Tests;

use Illuminate\Contracts\Events\ShouldHandleEventsAfterCommit;
use PHPUnit\Framework\Assert;

class Observer implements ShouldHandleEventsAfterCommit
{
    public function updated(Test $model)
    {
        Assert::assertEquals('updated', $model->test);
        Assert::assertEquals('original', $model->getOriginal('test'));
    }
}

<?php

namespace Tests;

use Illuminate\Database\Schema\Blueprint;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Schema;

class EventsInTransactionsTest extends TestCase
{
    use RefreshDatabase;

    protected function config(): void
    {
        config()->set('database.connections.sqlite', [
            'driver' => 'sqlite',
            'database' => ':memory:',
            'prefix' => '',
        ]);

        Schema::connection('sqlite')->create('test', function (Blueprint $table) {
            $table->id();
            $table->string('test');
        });

        Test::observe(Observer::class);
    }

    public function test_without_transaction(): void
    {
        $this->config();

        $model = new Test();
        $model->test = 'original';
        $model->save();

        $model->test = 'updated';
        $model->save();
    }

    public function test_with_transaction(): void
    {
        $this->config();

        DB::connection('sqlite')->transaction(function () {
            $model = new Test();
            $model->test = 'original';
            $model->save();

            $model->test = 'updated';
            $model->save();
        });
    }
}

[JakubSzczesniak]

It seems that you're dealing with a situation where the model object being tracked within a transaction is modified, which affects the behavior of the callbacks assigned to that transaction. This can lead to issues, especially when working with models in the context of a transaction, particularly in conjunction with events like updated.

In short, when a transaction is registered, Laravel creates callbacks that will be executed after the transaction is committed. If the model is modified during the transaction and Laravel uses the same object for the registered callbacks, this can lead to a scenario where the callbacks operate on outdated or inconsistent data.

In PHP, objects are passed by reference, meaning that if you modify an object in one place, those changes will be reflected everywhere that object is used unless you specifically clone the object before making changes.

[yurik94]

This is a big issue when building a logging mechanism for registering model changes, imagine this pretty common example:

Model to watch

class Order extends Model{
   protected $fillable = [
        'state_id', // ...
   ];
}

Model to log changes

#[ObservedBy(OrderObserver::class)]
class OrderStates extends Model{
   protected $fillable = [
        'order_id', 'old_state_id', 'new_state_id', // ...
   ];
   
  public function order(): BelongsTo
    {
        return $this->belongsTo(Order::class, 'order_id', 'id');
    }
}

Observer code

class OrderObserver implements ShouldHandleEventsAfterCommit
{
    public function updated(Order $model) // tried also updating with no luck
    {
        $new_state_id = $model->state_id;
        $previous_state_id = $model->getOriginal('state_id'));
        
        // here I would like to log the change in a OrderStates entry       
        // but real previous state_id is lost when wrapping the change in a transaction...
    }
}

Example code

DB::beginTransaction;

$order = Order::find(1);
$order->update(['state_id' => 2]);

// other relevant code...

DB::commit();

Is there any other clean way to do this?

[macropay-solutions]

Solution

1. Put this behavior in documentation (if not done already). This seems to be an issue converted into a discussion. Sad...
2. Stop using ShouldHandleEventsAfterCommit and use queue with delay. In that job check if the model has the new state and if so use the old state (including timestamp) sent in the job to notify or log things @yurik94 . Do not use model id as job payload. Use model to array.

Thank you for this. We had thoughts of using this ShouldHandleEventsAfterCommit but we never used it. Lucky us.