mathieutu / laravel-json-syncer

A Json importer and exporter for Laravel.
MIT License
28 stars 7 forks source link

Avoid possible conflict with Spatie\SchemalessAttributes #2

Closed alariva closed 6 years ago

alariva commented 6 years ago

For some of my models (Control), I'm using spatie/laravel-schemaless-attributes

It looks like it's causing conflict when trying to retrieve this field.

alariva@trinsic:~/fimedi$ php artisan export-json:account

In SchemalessAttributes.php line 132:

  Return value of Spatie\SchemalessAttributes\SchemalessAttributes::getRawSchemalessAttributes() must be of the type array, string returned  

A possible workaround would be disabling the export of this attribute, but that way I'd have to manually export it separately. Maybe there is a way I can convert this field before export/import ops.

alariva commented 6 years ago

So, I could perfectly export Controls when disallowing a JSON (spatie/schemaless) attribute I call metadata.

@mathieutu , Since this fields casts to array, would there be a chance to serialize/unserialize array attributes upon export/import?

    public $casts = [
        'metadata' => 'array',
    ];
alariva commented 6 years ago

I managed to export and convert with a workaround: use an alternative attribute called metadata_json that serializes de array type value and excluding metadata + including metadata_json for export.

However, it feels a bit clumsy as I have to mostly copy-paste all the fillable attributes.

Any suggestion for seamlessly converting or pre-processing Export/Import attributes before performing the operation?

// Control model

    protected $fillable = ['patient_id', 'visit_date', 'total_weight', 'body_mass_index', 'body_fat', 'skeletal_muscle',
                           'visceral_fat', 'visceral_fat_level', 'visceral_fat_area', 'resting_metabolism', 'body_age',
                           'blood_pressure', 'waist_circumference', 'remarks', 'treatment', 'device_model_id',
                           'metadata',
                           ];

    public function getJsonExportableAttributes() : array
    {
        return ['patient_id', 'visit_date', 'total_weight', 'body_mass_index', 'body_fat', 'skeletal_muscle',
                'visceral_fat', 'visceral_fat_level', 'visceral_fat_area', 'resting_metabolism', 'body_age',
                'blood_pressure', 'waist_circumference', 'remarks', 'treatment', 'device_model_id', 'metadata_json'
            ];
    }

    public function getMetadataJsonAttribute()
    {
        return json_encode($this->attributes['metadata']);
    }
mathieutu commented 6 years ago

Hum. Actually I think the problem is coming from the Spatie package, as it's there method which return a string in place of an array. Could you please open an issue in their package, ping me, and put here the link?

Depending on their answer, I'll look more into that, but I think I don't handle casts at all, because the aim here is to export database fields, to be able to re-import them later.

Small tip for you workaround, you can use $fillable in getJsonExportableAttributes(), and array_mergeand array_except.

alariva commented 6 years ago

Hi @mathieutu. I definitely take the tips for the workaround, it feels more elegant.

Regarding issue for spatie's package. Did you mean "array instead of string"?

I will do open the issue for them, but since the package is specially built for returning an array for that attribute, their response would be all the way right to say "It's the expected behavior / We don't see reasons for this package to become export-aware". Could you point me out what exactly should I ask for? (Maybe it's my task to serialize the attibute before performing the export action)

alariva commented 6 years ago

Hi @mathieutu

We finally got an answer from spatie/laravel-schemaless-attributes. As we could imagine, the package will not yield any modifications in order to serialize the output.

I'll be happy to work this out with you to find a solution from either the package, the app, or both sides and write about this as a howto like in this article. What you think?

mathieutu commented 6 years ago

For the article it will be amazing.

For the issue, I've directly answered in the Spatie one, as it's really not a problem from this package, there are no serialization at the step of the process, this is just a type error on their side or yours.

alariva commented 6 years ago

Hi @mathieutu

I have good news.

As you see, I've created the sandbox project to reproduce the issue I was running into.

https://github.com/alariva/laravel-json-syncer-schemaless-attributes

Surprisingly to me, I could not reproduce the problem, at least with the very basic scenario (no relationships involved).

So I now have to check if there's anything with my application that might be interfering.

So far, the only evident difference with my app and sandbox project is Laravel 5.6 and 5.7 respectively, but that should not be related.

I'll be investigating deeper on my app to see what's going on and will keep you posted!

alariva commented 6 years ago

So, this is what I'm getting when trying to import a nested model from JSON (Account-Patients-Controls)

I'll try to reproduce this into the sandbox project and let you know.

$ php artisan import-json:account 91

In HasAttributes.php line 760:

  preg_match() expects parameter 2 to be string, array given  

Exception trace:
 Illuminate\Foundation\Bootstrap\HandleExceptions->handleError() at n/a:n/a
 preg_match() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:760
 Illuminate\Database\Eloquent\Model->isStandardDateFormat() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:740
 Illuminate\Database\Eloquent\Model->asDateTime() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:771
 Illuminate\Database\Eloquent\Model->fromDateTime() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:553
 Illuminate\Database\Eloquent\Model->setAttribute() at ~/fimedi/vendor/sofa/hookable/src/Hookable.php:111
 App\Patient->Sofa\Hookable\{closure}() at n/a:n/a
 call_user_func_array() at ~/fimedi/vendor/sofa/hookable/src/Pipeline.php:88
 Sofa\Hookable\Pipeline->to() at ~/fimedi/vendor/sofa/hookable/src/Hookable.php:247
 App\Patient->pipe() at ~/fimedi/vendor/sofa/hookable/src/Hookable.php:114
 App\Patient->setAttribute() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php:292
 Illuminate\Database\Eloquent\Model->fill() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php:160
 Illuminate\Database\Eloquent\Model->__construct() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php:355
 Illuminate\Database\Eloquent\Model->newInstance() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Builder.php:1066
 Illuminate\Database\Eloquent\Builder->newModelInstance() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Builder.php:754
 Illuminate\Database\Eloquent\Builder->create() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php:1570
 Illuminate\Database\Eloquent\Model->__call() at ~/fimedi/vendor/mathieutu/laravel-json-syncer/src/Helpers/JsonImporter.php:61
 MathieuTu\JsonSyncer\Helpers\JsonImporter->importAttributes() at ~/fimedi/vendor/mathieutu/laravel-json-syncer/src/Helpers/JsonImporter.php:29
 MathieuTu\JsonSyncer\Helpers\JsonImporter->importFromJson() at ~/fimedi/vendor/mathieutu/laravel-json-syncer/src/Traits/JsonImporter.php:16
 App\Patient::importFromJson() at ~/fimedi/vendor/mathieutu/laravel-json-syncer/src/Helpers/JsonImporter.php:79
 MathieuTu\JsonSyncer\Helpers\JsonImporter->importChildrenIfImportable() at ~/fimedi/vendor/mathieutu/laravel-json-syncer/src/Helpers/JsonImporter.php:69
 MathieuTu\JsonSyncer\Helpers\JsonImporter->importRelations() at ~/fimedi/vendor/mathieutu/laravel-json-syncer/src/Helpers/JsonImporter.php:31
 MathieuTu\JsonSyncer\Helpers\JsonImporter->importFromJson() at ~/fimedi/vendor/mathieutu/laravel-json-syncer/src/Traits/JsonImporter.php:16
 App\Account::importFromJson() at ~/fimedi/app/Console/Commands/Maintenance/ImportAccountJSON.php:47
 App\Console\Commands\Maintenance\ImportAccountJSON->handle() at n/a:n/a
 call_user_func_array() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:29
 Illuminate\Container\BoundMethod::Illuminate\Container\{closure}() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:87
 Illuminate\Container\BoundMethod::callBoundMethod() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:31
 Illuminate\Container\BoundMethod::call() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Container/Container.php:564
 Illuminate\Container\Container->call() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Console/Command.php:184
 Illuminate\Console\Command->execute() at ~/fimedi/vendor/symfony/console/Command/Command.php:251
 Symfony\Component\Console\Command\Command->run() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Console/Command.php:171
 Illuminate\Console\Command->run() at ~/fimedi/vendor/symfony/console/Application.php:886
 Symfony\Component\Console\Application->doRunCommand() at ~/fimedi/vendor/symfony/console/Application.php:262
 Symfony\Component\Console\Application->doRun() at ~/fimedi/vendor/symfony/console/Application.php:145
 Symfony\Component\Console\Application->run() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Console/Application.php:89
 Illuminate\Console\Application->run() at ~/fimedi/vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php:122
 Illuminate\Foundation\Console\Kernel->handle() at ~/fimedi/artisan:35
    public function handle()
    {
        $accountId = $this->argument('account');

        $filepath = storage_path("account-export-{$accountId}.json");

        $data = file_get_contents($filepath);

        Account::importFromJson($data);

        $this->info("Imported from {$filepath}");
    }
alariva commented 6 years ago

What a surprise, at this point, it looks like the attributes causing the problem are created_at and updated_at. So far, it seems not related to schemaless-attributes.

I've done a dump($attributes) on MathieuTu\JsonSyncer\Helpers\JsonImporter:60

    protected function importAttributes($attributes): JsonImportable
    {
        $attributes = array_only($attributes, $this->importable->getJsonImportableAttributes());

        dump($attributes);

        return $this->importable instanceof Model ? $object = $this->importable->create($attributes) : $this->importable;
    }

and discovered this:

onpaste 20180919-193347

Maybe you faced this before and there is something I'm doing wrong on this.

How are you handling the these timestamps?

Did I export these fields as expected?

            "created_at": {
                "date": "2017-12-28 23:39:33.000000",
                "timezone_type": 3,
                "timezone": "UTC"
            },
            "updated_at": {
                "date": "2017-12-29 17:01:38.000000",
                "timezone_type": 3,
                "timezone": "UTC"
            },
alariva commented 6 years ago

I have just added a test case for using dates, and I still cannot reproduce the error.

However, in my app, I can continue import when doing:

    public function getJsonImportableAttributes() : array
    {
        // removed created_at and updated_at
        return ['firstname', 'lastname', 'birthdate', 'gender', 'location', 'nin', 'height',
               'physical_structure', 'wrist_circumference', 'job', 'notes', 'email', 'mobile',
               'mobile_carrier', 'consultation_reason', 'first_visit',
               'subscription', 'diet', 'physical_activity_level',
            ];
    }

and the problem araises again with

    public function getJsonImportableAttributes() : array
    {
        // added back created_at and updated_at
        return ['firstname', 'lastname', 'birthdate', 'gender', 'location', 'nin', 'height',
               'physical_structure', 'wrist_circumference', 'job', 'notes', 'email', 'mobile',
               'mobile_carrier', 'consultation_reason', 'first_visit',
               'subscription', 'diet', 'physical_activity_level', 'created_at', 'updated_at'
            ];
    }

Which leaves me again with absolutely no clue. Maybe I need to recreate the sandbox in Laravel 5.6 to match my project version just in case.

alariva commented 6 years ago

Hi @mathieutu , looks like I finally narrowed down the problem, and it was so silly mistake!

I was adding created_at and updated_at to $fillable in one of my models. That was causing the preg_match() expects parameter 2 to be string, array given. So I could solve that.

I have a new and derived problem, then, I'll refer to a new issue.