Problem with file uploads about doctrine-laminas-hydrator HOT 10 CLOSED

I am not sure if there is a best practice for this. However, I usually handle this on the level of my service classes as file uploads usually require a bit more intervention anyways. For example, I hydrate the FileUpload into my entity and then, when persisting the entity, my service moves the file into the right place and pushes all required meta information into the entity. This gives me greater flexibility, also in terms of renaming the file to include the ID of my entity. In other words, I don't use the RenameUpload filter to move the uploads into the right place.

from doctrine-laminas-hydrator.

Hi, @driehle
I appreciate your comment as a suggestion.
But I still think that if we have a specific hydrator for laminas, it's ideal that it behaves properly.
An attempt I've made, trying to have less impact, is to try to pass an array with image data to the setImage method and then store the correct information in the image property. looks like this:

public function setImage(?array $data): self
{
     $this->image = $data['tmp_name'] ?? null;
}

But here an error occurred, because $this->image refers to string and Doctrine Laminas Hydrator tries to Hydrate (call setImage) by $this->image property type (string) and not by type of parameter $data (array).

Perhaps, not for this specifically, wouldn't it be correct to say the hydrator should work with the expected parameter's type, not the property's type?

from doctrine-laminas-hydrator.

I think that if you want to hydrate to properties, then you should use a hydrator from the laminas-hydrator package directly, not the hydrator from doctrine-laminas-hydrator. The idea of this package is to hydrate to Doctrine fields, especially regarding the toOne and toMany relations.

However, I think there is one thing you would probably try:

class MyImage
{
    #[ORM\Id]
    #[ORM\Column(type: 'integer')]
    #[ORM\GeneratedValue(strategy: 'AUTO')]
    private ?int $id = null;

    #[ORM\Column(type: 'string')]
    private ?string $imagePath = null;

    private ?array $imageRawData = null;

    public function setImageRawData(array $imageRawData)
    {
        // do some processing here...
        $this->imagePath = 'abcdef.jpg';
    }
}

Now in your form use imageRawData as the field name for the upload field. I think there is an somehow undocumented feature (others might call it bug) in this hydrator, that allows hydration to any properties, not just Doctrine fields. This is because line 349 simply builds the setter name and checks for its existence (line 374). It never checks if that name refers to a Doctrine field or not.

It might make sense to add this to the documentation though. We could also change something in this behaviour, but that will could probably result in a BC break and would need to go in a new major then.

from doctrine-laminas-hydrator.

@driehle my bad. In my comment, properties refer to Doctrine fields. I don't want to hydrate not Doctrine fields.
Also, if you look at the line

IMHO - Perhaps, not for this case, wouldn't it be correct to say the hydrator should work with the expected parameter's type of setter Method, not the property (Doctrine field) of the Entity type?
it may even be unnecessary to use accessor method without it

from doctrine-laminas-hydrator.

@armenio Well, checking the parameter type of the setter method would be possible, for sure. However, this would lead to inconsistencies between the by value and the by reference hydration. This library can not only hydrate using the setter methods, but also directly via the property using reflection (see docs).

Obviously, hydration by value and by reference would then need to handled differently and I don't think that this is desirable. Furthermore, some people might not type their parameters (though this obviously is a bad design), which would cause further issues when relying on parameter types.

To me, using two different fields, like array $imageUploadRawData and string $imagePath looks like a more clean approach. In such a scenario you can actually have the image storing process (moving it somewhere, uploading it to a CDN, or whatever) handled by your service when calling $service->persist($image).

However, I am open to other suggestions. Feel free to convince me, why a different solutions is better. You are welcome to provide a pull request showcasing a better solution.

from doctrine-laminas-hydrator.

@driehle I certainly mean about hydrate using setter (accessor method).

I still understand that since you use an accessor method (typed, of course) it cannot be ignored and must have priority. this is an architectural flaw.

Also working to hydrate a non-"Doctrine field" object property with Doctrine Hydrator seems strange to me, besides messing up entities.

from doctrine-laminas-hydrator.

Would you have suggestions on how to overcome this architectural flaw?

from doctrine-laminas-hydrator.

Btw, have you thought of implementing your or filter to put in the input filter chain that reduces the upload array to a string? That could probably solve the hydration issue.

from doctrine-laminas-hydrator.

@driehle
1 - A good suggestion is to check the typing following the flow of attribution/recording of the value to the field (both strategies are byValue or byReference).
I can work on something here.

2 - I try to do this. But this does throw up other issue.
When a FORM FILE field is empty in the Browser request, an empty array is provided by the http request.
In this case, the filters are not executed (Filters works in chain) and the Doctrine Hydrator tries to assign an empty array to the string Doctrine Field and this causes a Notice: Array to string conversion.

from doctrine-laminas-hydrator.

Closing for lack of response. Please provide a PR if this is still relevant to you.

from doctrine-laminas-hydrator.