FriendsOfCake / cakephp-upload

CakePHP: Handle file uploading sans ridiculous automagic
https://cakephp-upload.readthedocs.io/
MIT License
551 stars 255 forks source link

Duplicate configs on certain situations #578

Open xavier83ar opened 2 years ago

xavier83ar commented 2 years ago

I was dealing with an issue that duplicates items when I passed nameCallback as an array like this [$object, 'methodName'], for example, with this config:

    // inside some Table::initialize()
    $this->addBehavior('Josegonzalez/Upload.Upload', [
        'imagen' => [
            'path' => 'webroot{DS}files{DS}mayoristas{DS}',
            'nameCallback' => [$this, 'buildName'],
        ],
    ]);

nameCallback is never called, to understand why, I read Behavior config after adding it, and nameCallback had this value

[
    0 => \MyObject\Instance,
    1 => "buildName",
    2 => \MyObject\Instance,
    3 => "buildName",
]

Digging into the code I find out that \Josegonzalez\Upload\Model\Behavior\UploadBehavior doesn't have a constructor, so it inherits from \Cake\ORM\Behavior, so the setConfig is called twice:

and in both cases is called with merge = true which is the default, I've tried replace that last line with $this->setConfig($configs, null, 'shallow'); and it works ok, but I don't know if that's the best solution as it is calling setConfig twice anyway, maybe a better solution is to add a constructor and call parent constructor with an empty array as configs.

ADmad commented 2 years ago

I had made a PR to overwrite the config instead of merging again but closed it to avoid any potential issue I might have not considered.

Your issue can be fixed by using 'nameCallback' => \Closure::fromCallable([$this, 'buildName']).

ADmad commented 2 years ago

In future the fields config should be nested inside a fields key so that it can be overwritten without worrying about affecting other configs. There's currently another issue too where className key needs to be explicitly unset due to having fields at top level of config array.