Django-rest-framework: Writing null or empty string to FileField does not empty the reference

[wbbradley]

It does not seem possible to use PATCH on a FileField to null out the field. Is this by design?

[tomchristie]

Not really by design no, not sure if this needs addressing or not.
I expect the underlying issue is that when partial updates are used there's no real differentiation between fields with None values and fields that are not present. Maybe worth digging around to see if this is also an issue with other None-able fields.

[tomchristie]

Duplicated by #949. Let's follow up against that ticket.

[tomchristie]

Not sure if this is a dup or not afterall, will reopen for now.

[aburgel]

I've run into this issue as well.

When using FileField, in WritableField.field_from_native the field data is retrieved from files and not data, meaning it comes from the parsed multipart data, not from the request body data. If you're submitting null or an empty string to that field, it won't be in the multipart data, so it will effectively look like no data was submitted.

I think the fix is to fall back on data for that field if its not found in files. If something other than null or empty string is submitted, then the validation on FileField will catch it.

Does that make sense?

[tomchristie]

Fixed via #1003.

[fletchowns]

I was trying out the new version that has this change and noticed my ImageFields were being inadvertently emptied out.

I totally understand the reasoning behind this change but I think it may have had unintended consequences. For example, if you are viewing a specific record through the browsable API and click the PUT button on the HTML form at the bottom, you've now destroyed data without even thinking about it, because the FileField is rendered without a file selected, so it gets emptied out when the form is submitted.

I'm not sure if there's a better way of going about this or not, just wanted to share my experience. I definitely like the idea of having a built in way to clear out these fields so I can get rid of my custom code to handle that in pre_save.

[wbbradley]

Interesting. I wonder if we can pre-fill the form with the filename, to avoid it from 'changing'? Just a thought... note that I haven't thought about this problem in a few months now...

[fletchowns]

Hmmm possibly, I can't figure out a way to get it to work though. I think browser security restrictions may prevent us from going down that route.

I can't think of a way to do this without using magical fields that don't actually exist on the model. There's gotta be some clever solution out there!

[fletchowns]

Anybody have any other thoughts on this one?

[fletchowns]

@tomchristie Any thoughts on this? It seems odd that you can no longer use the browsable API to update a record that has a FileField on it without having to upload the same file every time.

[tomchristie]

@fletchowns I don't really have much spare time to look into it right now - it'd be great if someone else was able to take ownership of investigating the issue and pushing through a fix.

[martinmaillard]

Hi,

I hit this issue in my code and it seems like a real problem: it basically makes PUT requests unusable on views containing a FileField/ImageField.

Is it reasonable to expect that sending a PUT request with the content of a GET request on the same resource should have no effect ? If this is the case, the problem comes from the asymmetry between FileField.to_native and FileField.from_native. If to_native returns the file name, sending the file name in the PUT request should not clear out the file. In my use case, I override to_native to serve the URL of the image instead of the file name. Sending this URL back in a PUT request should not affect the stored file.

I'm not very familiar with the codebase yet. Do you think this is a good solution ? And is it possible to implement ?

[wbbradley]

The semantics of a null value are equally ambiguous, in my opinion. Perhaps
you want to be using PATCH? Files and Web Form REST semantics are not
clearly defined, AFAIK.

-W

On Tue, Apr 22, 2014 at 8:10 AM, Martin Maillard
[email protected]:

Hi,

I hit this issue in my code and it seems like a real problem: it basically
makes PUT requests unusable on views containing a FileField/ImageField.

Is it reasonable to expect that sending a PUT request with the content of
a GET request on the same resource should have no effect ? If this is the
case, the problem comes from the asymmetry between FileField.to_nativeand FileField.from_native.
Ifto_nativereturns the file name, sending the file name in the PUT
request should not clear out the file. In my use case, I overrideto_native`
to serve the URL of the image instead of the file name. Sending this URL
back in a PUT request should not affect the stored file.

I'm not very familiar with the codebase yet. Do you think this is a good
solution ? And is it possible to implement ?

—
Reply to this email directly or view it on GitHubhttps://github.com/tomchristie/django-rest-framework/issues/937#issuecomment-41051627
.

[litchfield]

We're using PATCH with FileFields/ImageFields etc, so need a way of clearing the individual field. On our frontend the files are objects, so more natural to express empty state as null rather than an empty string.

Here's our hack, applied to our base serializer --

def get_extra_kwargs(self): extra_kwargs = super().get_extra_kwargs() for f in self.Meta.model._meta.get_fields(): # Hack to always allow_null on FileFields if isinstance(f, FileField): kwargs = extra_kwargs.get(f.name, {}) kwargs['allow_null'] = True extra_kwargs[f.name] = kwargs return extra_kwargs

[johncpang]

We're using PATCH with FileFields/ImageFields etc, so need a way of clearing the individual field. On our frontend the files are objects, so more natural to express empty state as null rather than an empty string.

Here's our hack, applied to our base serializer --

    def get_extra_kwargs(self):
        extra_kwargs = super().get_extra_kwargs()
        for f in self.Meta.model._meta.get_fields():
            # Hack to always allow_null on FileFields
            if isinstance(f, FileField):
                kwargs = extra_kwargs.get(f.name, {})
                kwargs['allow_null'] = True
                extra_kwargs[f.name] = kwargs
        return extra_kwargs```

I verified, this works. However, I also found that another solution which might be a bit better.

Copy from the source, in serializers add "allow_null=True" to the ImageField / FileField:

class PartSerializer(serializers.ModelSerializer):
    image = serializers.ImageField(max_length=None, allow_empty_file=True, allow_null=True, required=False)

    class Meta:
        model = Part
        fields = ('id', 'image')