Fix model serialization bug

[d0m4te]

immediately apply casting to geometry objects to prevent errors while trying to json_encode binary data

fixes #55

[d0m4te]

@MatanYadaev sorry, forgot to check against phpstan.

[MatanYadaev]

Thanks!

[patrickomeara]

This change breaks the Point casting when calling refresh() on a model.

TypeError: MatanYadaev\EloquentSpatial\Objects\Geometry::fromWkb(): Argument #1 ($wkb) must be of type string, MatanYadaev\EloquentSpatial\Objects\Point given, called in /Users/pat/Code/web/vendor/matanyadaev/laravel-eloquent-spatial/src/GeometryCast.php on line 47
/Users/pat/Code/web/vendor/matanyadaev/laravel-eloquent-spatial/src/Objects/Geometry.php:72
/Users/pat/Code/web/vendor/matanyadaev/laravel-eloquent-spatial/src/GeometryCast.php:47
/Users/pat/Code/web/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:816
/Users/pat/Code/web/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:792
/Users/pat/Code/web/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:2129
/Users/pat/Code/web/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:489
/Users/pat/Code/web/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:443
/Users/pat/Code/web/vendor/matanyadaev/laravel-eloquent-spatial/src/Traits/HasSpatial.php:27
/Users/pat/Code/web/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php:1697

It looks like the casting is happening twice and the second time via getAttribute() breaks the type.

[MatanYadaev]

Hi @patrickomeara , can you please submit a PR that fixes it?

[jbajou]

we're impacted by this, had to downgrade

[MatanYadaev]

Please provide examples on how to reproduce this.

[patrickomeara]

@jbajou Are you able to send me a code snippet that fails for you? I will add some failing tests and find a solution.

[jbajou]

Well, to be honest it's quite simple. We have a Location model

class Location extends Model
{
    use hasSpatial;
    
    protected $casts = [
        'point' => Point::class
    ];
}

Then, in a command (run on AWS SQS, with vapor) we do a updateOrCreate:

Location::query()
...
->updateOrCreate([
    ...
], [
    'point'           => $centralLocation->point,
]);

Here is the stack trace (partial):

TypeError MatanYadaev\EloquentSpatial\Objects\Geometry::fromWkb(): Argument #1 ($wkb) must be of type string, MatanYadaev\EloquentSpatial\Objects\Point given, called in /var/task/vendor/matanyadaev/laravel-eloquent-spatial/src/GeometryCast.php on line 47 
    vendor/matanyadaev/laravel-eloquent-spatial/src/Objects/Geometry.php:72 MatanYadaev\EloquentSpatial\Objects\Geometry::fromWkb
    vendor/matanyadaev/laravel-eloquent-spatial/src/GeometryCast.php:47 MatanYadaev\EloquentSpatial\GeometryCast::get
    vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:816 Illuminate\Database\Eloquent\Model::getClassCastableAttributeValue
    vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:792 Illuminate\Database\Eloquent\Model::castAttribute
    vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:2129 Illuminate\Database\Eloquent\Model::transformModelValue
    vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:489 Illuminate\Database\Eloquent\Model::getAttributeValue
    vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:443 Illuminate\Database\Eloquent\Model::getAttribute
    vendor/matanyadaev/laravel-eloquent-spatial/src/Traits/HasSpatial.php:27 App\Models\Location::setRawAttributes

[shengslogar]

Same stack trace over here.

Culprit line:

3.2.0...3.2.1#diff-8fa4ecc9982e0985829510274a951b022588e8be8c90e6fca5cbe23eb7edab8eR27

I don't fully understand the underlying implementation of or rationale behind this change, but obvious fix is updating GeometryCast to pass-through already-casted values.

Guessing the reason this was never encountered before is because Laravel caches casted values by default. Overriding setRawAttributes feels dangerous.

[MatanYadaev]

Hi @jbajou, your example doesn't appear to reproduce the bug.

I'm going to revert the latest changes to setRawAttributes, I know it means the serialization issue will return, but as other people said here, overriding setRawAttributes is not a good practice. I'll be happy to accept PRs and ideas on how to overcome this issue with a more elegant solution.