Bugs from pr#57

[JimChenWYU]

use Spatie\DataTransferObject\DataTransferObject;

class DemoSettings extends Settings
{
    /** @var DummyDto */
    public $json_config;

    public static function group(): string
    {
        return 'demo';
    }
}

class DummyDto extends DataTransferObject
{
     /** @var int */
     public $a;
}

when I enable the cache, get the demoSettings from cache, it will be unserialized like that

public function unserialize($serialized): void
{
    $properties = unserialize($serialized);

    $this->fill($properties);
    $this->originalValues = collect($properties);
    $this->loaded = true;
}

then DemoSettings::$json_config is same object as DemoSettings::$originalValues['json_config']

when I change the json_config and the originalValues will change as well, event I don't save it.

[rubenvanassche]

Hi @JimChenWYU,

I'm not exactly following what's going wrong. When you unserialize the settings class from the cache, something is going wrong but what exactly? Changing json_config shouldn't automatically change originalValues. Could you please provide some more information about the issue?

[JimChenWYU]

Hi @rubenvanassche

For example

use Spatie\DataTransferObject\DataTransferObject;

class DemoSettings extends Settings
{
    public DummyDto $json_config;

    public static function group(): string
    {
        return 'demo';
    }
}

class DummyDto extends DataTransferObject
{
     public int $a;
}

My init migrations:

use Spatie\LaravelSettings\Migrations\SettingsMigration;

class CreateDemoSettings extends SettingsMigration
{
    public function up(): void
    {
        $this->migrator->add('demo.json_config', [
            'a' => 1
        ]);
    }
}

In My routes, I write like this to test

Route::get('/', function (\App\Settings\Demo $demo) {
    dump($demo);
    $demo->json_config->a = 2;
    dump($demo);
});

so it is expected? when json_config is marked as an object, the json_config in originalValues will be an same object as json_config in DemoSettings, when I change json_config in DemoSettings, the the json_config in originalValues will change, event I no save

or may I only change like this?

$newJsonConfig = new DummyDto();
$newJsonConfig->a = 2;
$demo->json_config = $newJsonConfig;

[ragingdave]

I'm not sure of the actual solution here, but that's just a by product of any object, not just DTOs. The only idea I have would be to recursively clone any objects, but that gets potentially tricky when an object may or may not be traversable (ie a DTO in a DTO). Then there's potentially dealing with circular references and all sorts of other issues.

Maybe if clone was added without deep cloning, that'd work potentially, but I'm not sure if there's perhaps a better way.

[rubenvanassche]

In my opinion you cannot change a DTO, since that could create inconsistencies like this one. I always create a new DTO when a property is changed within it. So I think the solution @JimChenWYU proposed in the end is the best one:

$newJsonConfig = new DummyDto();
$newJsonConfig->a = 2;
$demo->json_config = $newJsonConfig;

As @ragingdave says, we could add support for cloning objects but that would bring us a bit too far in my opinion. I'm gonna close this issue for now.

[ragingdave]

@JimChenWYU Maybe a readme PR for best practices when dealing with objects? to treat them as immutable in code and replace them instead of altering them?