Skip to content

Commit

Permalink
add signal tests and update readme
Browse files Browse the repository at this point in the history
  • Loading branch information
vinnyrose committed Jan 28, 2023
1 parent eab3ae7 commit a13ae5e
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 14 deletions.
40 changes: 28 additions & 12 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,7 @@ whether or not a :code:`FileField`'s value has changed a local cache of original
the model instance. If a condition is detected that should result in a file deletion, a function to
delete the file is setup and inserted into the commit phase of the current transaction.

**Warning! If you are using a database that does not support transactions you may lose files if a
transaction will rollback at the right instance. This outcome is mitigated by our use of
post_save and post_delete signals, and by following the recommended configuration below. This
outcome will still occur if there are signals registered after app initialization and there are
exceptions when those signals are handled. In this case, the old file will be lost and the new file
will not be referenced in a model, though the new file will likely still exist on disk. If you are
concerned about this behavior you will need another solution for old file deletion in your project.**
**Warning! Please be aware of the Known Limitations documented below!**

Installation
============
Expand Down Expand Up @@ -77,6 +71,27 @@ You can check if your ``Model`` is loaded by using
from django.apps import apps
apps.get_models()
Known Limitations
=================

Database Should Support Transactions
------------------------------------
If you are using a database that does not support transactions you may lose files if a
transaction will rollback at the right instance. This outcome is mitigated by our use of
post_save and post_delete signals, and by following the recommended configuration in this README.
This outcome will still occur if there are signals registered after app initialization and there are
exceptions when those signals are handled. In this case, the old file will be lost and the new file
will not be referenced in a model, though the new file will likely still exist on disk. If you are
concerned about this behavior you will need another solution for old file deletion in your project.

File referenced by multiple model instances
-------------------------------------------
This app is designed with the assumption that each file is referenced only once. If you are sharing
a file over two or more model instances you will not have the desired functionality. If you want to
reference a file from multiple models add a level of indirection. That is, use a separate file model
that is referenced from other models through a foreign key. There are many file management apps
already available in the django ecosystem that fulfill this behavior.

Advanced
========
This section contains additional functionality that can be used to interact with django-cleanup for
Expand Down Expand Up @@ -115,7 +130,8 @@ There have been rare cases where the cache would need to be refreshed. To do so
Ignore cleanup for a specific model
-----------------------------------
Ignore a model and do not perform cleanup when the model is deleted or its files change.
To ignore a model and not have cleanup performed when the model is deleted or its files change, use
the :code:`ignore` decorator to mark that model:

.. code-block:: py
Expand All @@ -128,10 +144,10 @@ Ignore a model and do not perform cleanup when the model is deleted or its files
Only cleanup selected models
----------------------------
If you have many models to ignore, or if you prefer to be explicit about what models are selected,
you can change the mode of django-cleanup to select mode by using the select mode app config. In
your ``INSTALLED_APPS`` setting you will replace ``'django_cleanup.apps.CleanupConfig'``
with ``'django_cleanup.apps.CleanupSelectedConfig'``. Then use the ``select`` decorator to mark a
model for cleanup:
you can change the mode of django-cleanup to "select mode" by using the select mode app config. In
your ``INSTALLED_APPS`` setting you will replace ':code:`django_cleanup.apps.CleanupConfig`'
with ':code:`django_cleanup.apps.CleanupSelectedConfig`'. Then use the :code:`select` decorator to
mark a model for cleanup:

.. code-block:: py
Expand Down
3 changes: 3 additions & 0 deletions src/django_cleanup/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ def delete_file(sender, instance, field_name, file_, using, reason):

event = {
'deleted': reason == 'deleted',
'model_name': model_name,
'field_name': field_name,
'file_name': file_.name,
'default_file_name': default,
'file': file_,
'instance': instance,
'updated': reason == 'updated'
Expand Down
2 changes: 2 additions & 0 deletions test/models/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,11 @@ class Meta:
managed = False
db_table = 'test_product'


class RootProduct(models.Model):
pass


class BranchProduct(models.Model):
root = models.ForeignKey(RootProduct, on_delete=models.CASCADE)
image = models.FileField(upload_to='test', blank=True, null=True)
70 changes: 69 additions & 1 deletion test/test_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
from django.conf import settings
from django.core.files import File
from django.db import transaction
from django.db.models.fields import files
from django.db.models.fields import NOT_PROVIDED, files

import pytest

from django_cleanup import cache, handlers
from django_cleanup.signals import cleanup_post_delete, cleanup_pre_delete

from . import storage
from .models.app import (
Expand Down Expand Up @@ -352,6 +353,73 @@ def test_file_exists_on_create_and_update():
assert not os.path.isfile(product.image.path)


@pytest.mark.django_db(transaction=True)
def test_signals(picture):
prekwargs = None
postkwargs = None
def assn_prekwargs(**kwargs):
nonlocal prekwargs
prekwargs = kwargs

def assn_postkwargs(**kwargs):
nonlocal postkwargs
postkwargs = kwargs

cleanup_pre_delete.connect(assn_prekwargs, dispatch_uid='pre_test_replace_file_with_file_signals')
cleanup_post_delete.connect(assn_postkwargs, dispatch_uid='post_test_replace_file_with_file_signals')
product = Product.objects.create(image=picture['filename'])
random_pic_name = get_random_pic_name()
product.image = random_pic_name
with transaction.atomic(get_using(product)):
product.save()

assert prekwargs['deleted'] == False
assert prekwargs['updated'] == True
assert prekwargs['instance'] == product
assert prekwargs['file'] is not None
assert prekwargs['file_name'] == picture['filename']
assert isinstance(prekwargs['default_file_name'], NOT_PROVIDED)
assert prekwargs['model_name'] == 'test.product'
assert prekwargs['field_name'] == 'image'

assert postkwargs['deleted'] == False
assert postkwargs['updated'] == True
assert postkwargs['instance'] == product
assert postkwargs['file'] is not None
assert postkwargs['file_name'] == picture['filename']
assert isinstance(postkwargs['default_file_name'], NOT_PROVIDED)
assert postkwargs['model_name'] == 'test.product'
assert postkwargs['field_name'] == 'image'
assert postkwargs['success'] == True
assert postkwargs['error'] is None

with transaction.atomic(get_using(product)):
product.delete()

assert prekwargs['deleted'] == True
assert prekwargs['updated'] == False
assert prekwargs['instance'] == product
assert prekwargs['file'] is not None
assert prekwargs['file_name'] == random_pic_name
assert isinstance(prekwargs['default_file_name'], NOT_PROVIDED)
assert prekwargs['model_name'] == 'test.product'
assert prekwargs['field_name'] == 'image'

assert postkwargs['deleted'] == True
assert postkwargs['updated'] == False
assert postkwargs['instance'] == product
assert postkwargs['file'] is not None
assert postkwargs['file_name'] == random_pic_name
assert isinstance(postkwargs['default_file_name'], NOT_PROVIDED)
assert postkwargs['model_name'] == 'test.product'
assert postkwargs['field_name'] == 'image'
assert postkwargs['success'] == True
assert postkwargs['error'] is None

cleanup_pre_delete.disconnect(None, dispatch_uid='pre_test_replace_file_with_file_signals')
cleanup_post_delete.disconnect(None, dispatch_uid='post_test_replace_file_with_file_signals')


#region select config
@pytest.mark.CleanupSelectedConfig
@pytest.mark.django_db(transaction=True)
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
envlist =
py{36,37,38,39,310,py3}-django32
py{38,39,310,py3}-django40
py{38,39,310,311,py3}-django{41, main}
py{38,39,310,311,py3}-django{41,main}
[testenv]
deps =
djangomain: https://github.com/django/django/tarball/main
Expand Down

0 comments on commit a13ae5e

Please sign in to comment.