Cannot add metada after upgrading to v3

[rodrigopedra]

Cannot add metada after upgrading to v3

[freekmurze]

@riasvdv could you take a look at this one?

[rodrigopedra]

As a workaround, replacing the whole meta data array works:

<?php

use Illuminate\Support\Facades\Event;
use Illuminate\Support\Facades\Request;
use Spatie\EventProjector\ShouldBeStored;
use Spatie\EventProjector\Models\EloquentStoredEvent;

EloquentStoredEvent::creating(function ($event) {
    // **** CHANGED HERE ****
    $event->meta_data = ['ip' => Request::ip()];
});

Route::get('/', function () {
    Event::dispatch(new TestEvent());

    return 'ok';
});

class TestEvent implements ShouldBeStored
{
}

I got the idea to test this workaround from this issue (in Laravel repo) description:

laravel/framework#7672

[rodrigopedra]

It seems to be an issue with Laravel. I tried the following to double check:

1. added a meta_data column to the users table and casted it to array
2. Changed the test code as follow

<?php

use App\User;
use Illuminate\Support\Facades\Event;
use Illuminate\Support\Facades\Request;
use Spatie\EventProjector\ShouldBeStored;
use Spatie\EventProjector\Models\EloquentStoredEvent;

EloquentStoredEvent::creating(function ($event) {
    $event->meta_data['ip'] = Request::ip();
});

User::creating(function ($user) {
    $user->meta_data['ip'] = Request::ip();
});

Route::get('/', function () {
    User::create(['name' => 'dummy', 'email' => '[email protected]', 'password' => '123']);
    Event::dispatch(new TestEvent());

    return 'ok';
});

class TestEvent implements ShouldBeStored
{
}

Now it fails before getting to the EloquentStoredEvent event listener.

I will open an issue in the Laravel framework

---

EDIT

Tested the User Model with previous Laravel versions and the exception is also thown. I don't know if it is related to a PHP versioning (updated my php 7.3 yesterday), but I can assure it was working before.

I guess I will stick with the workaround

[rodrigopedra]

Also the docs for storing metadata in v3 are not updated to use the EloquentStoredEvent:

https://docs.spatie.be/laravel-event-projector/v3/advanced-usage/storing-metadata/

I can send a PR fixing it, but I want to know how we can handle this issue.

I am ok with the workaround, just want to confirm if there is not a solution on using the old way.

Thanks.

[rodrigopedra]

Ok, promise it will be the last comment for now...

Testes with a clean Laravel 5.8 install and it works as in documentation.

Steps:

1. composer create-project --prefer-dist laravel/laravel laravel58 "5.8.*"
2. composer require spatie/laravel-event-projector:^2.0.0
3. publish migration, run migration
4. Add code below to routes/web.php
5. Navigate to home page

Seems that something changed with how the meta data is handled. Unfortunately I don't have the time now to dig in, also there are a lot of commits between the versions.

I am ok with the workaround of reassigning the meta data array. Just want to raise the issue as it was the only thing preventing me to migrate a project to Laravel 6.

If the solution is to reassign the metadata array, please let me know and I will send a PR to the docs.

If you read until here, thanks for the patience. :)

<?php

use Illuminate\Support\Facades\Event;
use Illuminate\Support\Facades\Request;
use Spatie\EventProjector\ShouldBeStored;
use Spatie\EventProjector\Models\StoredEvent;

StoredEvent::creating(function ($event) {
    $event->meta_data['ip'] = Request::ip();
});

Route::get('/', function () {
    Event::dispatch(new TestEvent());

    return 'ok';
});

class TestEvent implements ShouldBeStored
{
}

[riasvdv]

@rodrigopedra I forgot to update the docs for this, they should now be updated to reflect the StoredEvent changes. https://docs.spatie.be/laravel-event-projector/v3/advanced-usage/storing-metadata/

Since we're now using a repository to store the events, you'll need that in your projector to update the storedEvent, if you want to use model events, you'll need to create your own repository that extends the EloquentStoredEventRepository and changes the model there.

Let me know if this helps or you run into any other issues!

[riasvdv]

@rodrigopedra I've made a PR #191 that would make the upgrade a lot easier if you just want to use a custom Eloquent model

[rodrigopedra]

Hi @riasvdv thanks for the response.

Maybe due to the fact English is not my native language I might have expressed myself in a confusing way. Or might be due to the numerous messages.

Note I never said that I am using a custom model. It is just out of the box with minimal configuration. I don't even publish the config, just the migration file. In my production app I also do not use a Custom Model.

I'll try to be more objective.

• This works on Laravel Event Projector 2.x and Laravel 5.8:
• This does not work on Laravel Event Projector 3.x and Laravel 6.0:

$event->meta_data['foo'] = 'bar';

Executing the code above with version 3 of this package raises the ErrorException described in this issue's opening message. I updated my sample app to version 3.1 and it fails as well.

As a workaround the code below works with version 3.

$event->meta_data = ['foo' => 'bar'];

Note that the difference is that in this code I am reassigning the whole array to the meta_data property.

It is different from what worked before and is different from the updated docs.

If you want to try it out, just follow the steps in this opening post. Besides running the migrations you need only to modify the routes/web.php file. No custom config or custom models are needed .

I am ok if there is no solution to get the old syntax back, from my research this is a limitation of PHP as the meta_data array is returned from a magic property (__get on Eloquent Model). This blog post has some related links that describes why it should not work: https://phpolyk.wordpress.com/2012/07/25/indirect-modification-of-overloaded-property/

I couldn't figure out why it worked in the previous version, if you read in one of my previous comments I tested using a plain Eloquent Model with an attribute casted to array, and the same exception is thrown in both Laravel 5.8 and Laravel 6. But somehow it worked for the meta_data from the model on this package.

In my real app, where I use this package I also do not extend the StoredEvent model. I updated to reassigning the whole array and not to setting the property as the example in the docs.

My question is if we can revert to the old syntax or if not, if we should update the docs accordingly. It would also nice to have tests. I don't know how to write them as I don't know what caused the behavior change.

---

EDIT My guess, (very naive guess, I didn't have the time to dig in the changes) is that the behavior changed due to the removal of the https://github.com/spatie/laravel-schemaless-attributes dependency. But again this is just a guess.

[rodrigopedra]

Hi @riasvdv ,

Indeed adding the laravel-schemaless-attributes dependency solved the issue.

I created PR #194 to revert on using that package.

I outlined the reasons in the PR description.

[riasvdv]

You're right, I've merged the PR, hope everything is fixed then!

[rodrigopedra]

Thanks @riasvdv ! And thanks for the patience.