un1t / django-cleanup

Automatically deletes old file for FileField and ImageField. It also deletes files on models instance deletion.
MIT License
1.11k stars 79 forks source link

Attribute error upon cache cleanup #109

Closed sevdog closed 1 month ago

sevdog commented 3 months ago

The currently implementation of fields_for_model_instance method performs a "safe" get-attribut but it is not currently handling the possibility that the field is not returned:

https://github.com/un1t/django-cleanup/blob/eee7785bd219c4bb8a79e88648e8843bd215be4d/src/django_cleanup/cache.py#L84-L86

If by any means the field is not found on the model instance the code tries to read attributes from a None, thus resulting in an AttributeError.

When using in conjunction with the InMemoryStorage with a configuration like the following an error is raised when a file is copied between two models

# models.py

class FirstModel(models.Model):
   file = FileField(...)

class MyOtherModel(models.Model)
    ...

class Attachment(models.Model):
    belong_to = models.ForeignKey(MyOtherModel, related_name='attachments', ...)
    attachment = models.FileField(...)

    def __str__(self):
        return self.attachment.name

# script

obj = FirstModel.objects.create(file=ContentFile('the content', 'file.txt')
other = MyOtherModel.objects.create(...)
other.attachments.create(attachment=obj.file)

Which causes the following error:

../.venv/lib/python3.10/site-packages/django/db/models/base.py:572: in __init__
    post_init.send(sender=cls, instance=self)
        _DEFERRED  = <Deferred field>
        __class__  = <class 'django.db.models.base.Model'>
        _setattr   = <built-in function setattr>
        args       = ()
        cls        = <class 'myapp.models.Attachment'>
        field      = <django.db.models.fields.BooleanField: is_private>
        fields_iter = <tuple_iterator object at 0x70e62f488a30>
        is_related_object = False
        kwargs     = {}
        opts       = <Options for Attachment>
        rel_obj    = <MyOtherModel: XXXXXXX>
        self       = <[AttributeError("'InMemoryFileNode' object has no attribute 'name'") raised in repr()] Attachment object at 0x70e62f1a0ee0>
        val        = False
../.venv/lib/python3.10/site-packages/django/dispatch/dispatcher.py:176: in send
    return [
        named      = {'instance': <[AttributeError("'InMemoryFileNode' object has no attribute 'name'") raised in repr()] Attachment object at 0x70e62f1a0ee0>}
        self       = <django.db.models.signals.ModelSignal object at 0x70e636161270>
        sender     = <class 'myapp.models.Attachment'>
../.venv/lib/python3.10/site-packages/django/dispatch/dispatcher.py:177: in <listcomp>
    (receiver, receiver(signal=self, sender=sender, **named))
        .0         = <list_iterator object at 0x70e62ef95ab0>
        named      = {'instance': <[AttributeError("'InMemoryFileNode' object has no attribute 'name'") raised in repr()] Attachment object at 0x70e62f1a0ee0>}
        receiver   = <function cache_original_post_init at 0x70e634dff0a0>
        self       = <django.db.models.signals.ModelSignal object at 0x70e636161270>
        sender     = <class 'myapp.models.Attachment'>
../.venv/lib/python3.10/site-packages/django_cleanup/handlers.py:22: in cache_original_post_init
    cache.make_cleanup_cache(instance)
        instance   = <[AttributeError("'InMemoryFileNode' object has no attribute 'name'") raised in repr()] Attachment object at 0x70e62f1a0ee0>
        kwargs     = {'signal': <django.db.models.signals.ModelSignal object at 0x70e636161270>}
        sender     = <class 'myapp.models.Attachment'>
../.venv/lib/python3.10/site-packages/django_cleanup/cache.py:156: in make_cleanup_cache
    setattr(instance, CACHE_NAME, dict(
        instance   = <[AttributeError("'InMemoryFileNode' object has no attribute 'name'") raised in repr()] Attachment object at 0x70e62f1a0ee0>
        source     = <[AttributeError("'InMemoryFileNode' object has no attribute 'name'") raised in repr()] Attachment object at 0x70e62f1a0ee0>
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

instance = <[AttributeError("'InMemoryFileNode' object has no attribute 'name'") raised in repr()] Attachment object at 0x70e62f1a0ee0>
using = <[AttributeError("'InMemoryFileNode' object has no attribute 'name'") raised in repr()] Attachment object at 0x70e62f1a0ee0>

    def fields_for_model_instance(instance, using=None):
        '''
            Yields (name, descriptor) for each file field given an instance

            Can use the `using` kwarg to change the instance that the `FieldFile`
            will receive.
        '''
        if using is None:
            using = instance
        model_name = get_model_name(instance)

        deferred_fields = instance.get_deferred_fields()

        for field_name in get_fields_for_model(model_name, exclude=deferred_fields):
            fieldfile = getattr(instance, field_name, None)
>           yield field_name, fieldfile.__class__(using, fieldfile.field, fieldfile.name)
E           AttributeError: 'NoneType' object has no attribute 'field'

deferred_fields = set()
field_name = 'attachment'
fieldfile  = None
instance   = <[AttributeError("'InMemoryFileNode' object has no attribute 'name'") raised in repr()] Attachment object at 0x70e62f1a0ee0>
model_name = 'myapp.attachment'
using      = <[AttributeError("'InMemoryFileNode' object has no attribute 'name'") raised in repr()] Attachment object at 0x70e62f1a0ee0>

../.venv/lib/python3.10/site-packages/django_cleanup/cache.py:86: AttributeError

We got into this issue while moving tests from FileSystemStorage /under which the code above works) to InMemoryStorage (under which it does not).

PS: there is also a problem with the file because it does not define a name property and that was reported to django in https://code.djangoproject.com/ticket/35658

vinnyrose commented 2 months ago

The storage exists on the FileField, the error you are seeing is on the FieldFile. Something doesn't add up, one shouldn't affect the other.

vinnyrose commented 1 month ago

Was unable to reproduce, let me know if this is still an issue.

github-actions[bot] commented 3 weeks ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.