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

Not deleting objects with pytest-django and S3 storage backend #107

Closed pavel-kalmykov closed 2 months ago

pavel-kalmykov commented 4 months ago

I have a Django app with this model:

from django.contrib.auth.models import User
from django.db import models
from django_resized import ResizedImageField

class Image(models.Model):
    id = models.AutoField(primary_key=True)
    footnote = models.CharField(max_length=255)
    image = ResizedImageField(upload_to="images/")
    creator = models.ForeignKey(User, on_delete=models.CASCADE)
    created_at = models.DateTimeField(auto_now_add=True)

And the following test:

from io import BytesIO

import pytest
from django.conf import settings
from django.contrib.auth.models import User
from django.core.exceptions import ObjectDoesNotExist
from django.core.files.base import ContentFile
from PIL import Image as PILImage

from .models import Image

@pytest.fixture
def test_user():
    return User.objects.create_user(username="testuser", password="testpass")

@pytest.fixture
def sample_image_file():
    file = BytesIO()
    image = PILImage.new("RGB", (100, 100), color="green")
    image.save(file, "JPEG")
    file.seek(0)
    return file

@pytest.fixture
def sample_image_instance(test_user, sample_image_file):
    return Image.objects.create(
        footnote="Test footnote",
        image=ContentFile(sample_image_file.getvalue(), name="test.jpg"),
        creator=test_user,
    )

@pytest.mark.django_db
class TestImage:
    def test_image_deletion(self, sample_image_instance):
        image_name = sample_image_instance.image.name
        image_id = sample_image_instance.id

        # Delete the image
        sample_image_instance.delete()

        # Check that the image is no longer in the database
        with pytest.raises(ObjectDoesNotExist):
            Image.objects.get(id=image_id)

        # Check that the file is no longer in S3
        assert not sample_image_instance.image.storage.exists(image_name)

And the last assertion, assert not sample_image_instance.image.storage.exists(image_name), is not working.

As for the configuration, I am using pytest-django with the following properties:

[tool.pytest.ini_options]
DJANGO_SETTINGS_MODULE = "myproject.settings"
# -- recommended but optional:
python_files = ["test_*.py", "*_test.py", "tests.py"]

[tool.pytest_env]
SQL_HOST = "localhost"
AWS_LOCATION = "test/"

And this is my storages setup (using django-storages) in settings.py:

with open(BASE_DIR / env("AWS_CLOUDFRONT_KEY_FILENAME"), "rb") as cloudfront_cert:
    aws_cloudfront_key = cloudfront_cert.read()

STORAGES = {
    "default": {
        "BACKEND": "storages.backends.s3.S3Storage",
        "OPTIONS": {
            "access_key": env("AWS_ACCESS_KEY"),
            "secret_key": env("AWS_SECRET_KEY"),
            "bucket_name": env("AWS_BUCKET_NAME"),
            "default_acl": env("AWS_DEFAULT_ACL"),
            # We use the location as a prefix indicating the environment (dev, test, prod...)
            # Some other (better) possibilities are using another bucket or an access point
            "location": env("AWS_LOCATION"),
            "region_name": env("AWS_S3_REGION_NAME"),
            "file_overwrite": env("AWS_S3_FILE_OVERWRITE"),
            "custom_domain": env("AWS_CLOUDFRONT_DOMAIN"),
            "cloudfront_key": aws_cloudfront_key,
            "cloudfront_key_id": env("AWS_CLOUDFRONT_KEY_ID"),
        },
    },
    "staticfiles": {
        "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage",
    },
}

The thing is that the cleanup works flawlessly when in development mode, as I have a view that looks like this:

from django.contrib import messages
from django.contrib.auth.decorators import login_required
from django.shortcuts import get_object_or_404, redirect
from django.views.decorators.http import require_POST
from .models import Image

@login_required
@require_POST
def delete_image(request, image_id):
    image = get_object_or_404(Image, id=image_id)
    if request.user == image.creator:
        image.delete()
        messages.success(request, "Image deleted successfully.")
    else:
        messages.error(request, "You don't have permission to delete this image.")
    return redirect("images")

And removes the stored images flawlessly.

I tried checking the loaded models as described in the troubleshooting section and I see my model loaded. Additionally, @pytest.mark.django_db is equivalent to using django.test.TestCase (which inherits from django.test.TransactionTestCase, as the testing section indicates). What am I missing for this to work?

vinnyrose commented 4 months ago

Use @pytest.mark.django_db(transaction=True)

image

pavel-kalmykov commented 4 months ago

That fixed it. Thanks!

Do you mind if I add an explanatory comment in the testing section about this?

vinnyrose commented 4 months ago

that's fine.

pavel-kalmykov commented 3 months ago

I just created #108. It took me some time because I was on holiday. Could you review it, please? Feel free to close both the issue and PR if everything looks OK.

vinnyrose commented 3 months ago

I will merge it into the upcoming feature branch, which I expect to create soon.

github-actions[bot] commented 1 month 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.