Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,38 @@ The default processor is `pictures.tasks.process_picture`. It takes a single
argument, the `PictureFileFile` instance. You can use this to override the
processor, should you need to do some custom processing.

### Signals

The async image processing emits a signal when the task is complete.
You can use that to store when the pictures have been processed,
so that a placeholder could be rendered in the meantime.

```python
# models.py
from django.db import models
from pictures.models import PictureField


class Profile(models.Model):
title = models.CharField(max_length=255)
picture = PictureField(upload_to="avatars")
picture_processed = models.BooleanField(editable=False, null=True)


# signals.py
from django.dispatch import receiver
from pictures import signals

from .models import Profile


@receiver(signals.picture_processed, sender=Profile._meta.get_field("picture"))
def picture_processed_handler(*, sender, file_name, **__):
sender.model.objects.filter(**{sender.name: file_name}).update(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm… we should probably pass the model instance to the signal, just like the post- or pre-save signals.

Your filename doesn't need to be unique. It should, but it really doesn't have to.

However, this would mean fetching it from the DB in the processing task. I'd love to avoid DB IO in the processing task by default.

If you add a unique constraint and index, including a comment on why they matter, that might be the best solution. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An index makes sense for the lookup here, but I'm not sure if a unique constraint is strictly necessary. filter will update all rows that match. I think that makes sense as I think that the variations would have been removed for that file.

To send along the instance we would need acceess to it to serialize it. Accessing the instance in https://github.com/jmsmkn/django-pictures/blob/4077289ae6a75c8f8a810d04487af4a95b4f1a45/pictures/models.py#L179-L185 doesn't work as Django cleanup assigns a fake instance to self.instance. I don't think that there is another workaround for that through self.field.

picture_processed=True
)
```

### Validators

The library ships with validators to restrain image dimensions:
Expand Down
10 changes: 10 additions & 0 deletions pictures/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ def delete_all(self):
import_string(conf.get_settings().PROCESSOR)(
self.storage.deconstruct(),
self.name,
self.sender,
[],
[i.deconstruct() for i in self.get_picture_files_list()],
)
Expand All @@ -170,10 +171,19 @@ def update_all(self, other: PictureFieldFile | None = None):
import_string(conf.get_settings().PROCESSOR)(
self.storage.deconstruct(),
self.name,
self.sender,
[i.deconstruct() for i in new],
[i.deconstruct() for i in old],
)

@property
def sender(self):
return (
self.field.model._meta.app_label,
self.field.model._meta.model_name,
self.field.name,
)

@property
def width(self):
self._require_file()
Expand Down
3 changes: 3 additions & 0 deletions pictures/signals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import django.dispatch

picture_processed = django.dispatch.Signal()
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The picture_processed signal should include a docstring documenting its parameters (sender, file_name, new, old) and their types/meanings. This helps users understand what data is available when handling the signal.

Suggested change
picture_processed = django.dispatch.Signal()
picture_processed = django.dispatch.Signal()
picture_processed.__doc__ = """
Signal sent when a picture has been processed.
Parameters:
sender: The sender of the signal (usually the model class).
file_name (str): The name of the processed picture file.
new (bool): True if the picture is newly processed, False if updated.
old (bool): True if the picture existed before processing, False otherwise.
"""

Copilot uses AI. Check for mistakes.
31 changes: 27 additions & 4 deletions pictures/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

from typing import Protocol

from django.apps import apps
from django.db import transaction
from PIL import Image

from pictures import conf, utils
from pictures import conf, signals, utils


def noop(*args, **kwargs) -> None:
Expand All @@ -17,6 +18,7 @@ def __call__(
self,
storage: tuple[str, list, dict],
file_name: str,
sender: tuple[str, str, str],
new: list[tuple[str, list, dict]] | None = None,
old: list[tuple[str, list, dict]] | None = None,
) -> None: ...
Expand All @@ -25,6 +27,7 @@ def __call__(
def _process_picture(
storage: tuple[str, list, dict],
file_name: str,
sender: tuple[str, str, str],
new: list[tuple[str, list, dict]] | None = None,
old: list[tuple[str, list, dict]] | None = None,
) -> None:
Expand All @@ -41,6 +44,17 @@ def _process_picture(
picture = utils.reconstruct(*picture)
picture.delete()

app_label, model_name, field_name = sender
model = apps.get_model(app_label=app_label, model_name=model_name)
field = model._meta.get_field(field_name)

signals.picture_processed.send(
sender=field,
file_name=file_name,
new=new,
old=old,
)


process_picture: PictureProcessor = _process_picture

Expand All @@ -55,21 +69,24 @@ def _process_picture(
def process_picture_with_dramatiq(
storage: tuple[str, list, dict],
file_name: str,
sender: tuple[str, str, str],
new: list[tuple[str, list, dict]] | None = None,
old: list[tuple[str, list, dict]] | None = None,
) -> None:
_process_picture(storage, file_name, new, old)
_process_picture(storage, file_name, sender, new, old)

def process_picture( # noqa: F811
storage: tuple[str, list, dict],
file_name: str,
sender: tuple[str, str, str],
new: list[tuple[str, list, dict]] | None = None,
old: list[tuple[str, list, dict]] | None = None,
) -> None:
transaction.on_commit(
lambda: process_picture_with_dramatiq.send(
storage=storage,
file_name=file_name,
sender=sender,
new=new,
old=old,
)
Expand All @@ -89,14 +106,16 @@ def process_picture( # noqa: F811
def process_picture_with_celery(
storage: tuple[str, list, dict],
file_name: str,
sender: tuple[str, str, str],
new: list[tuple[str, list, dict]] | None = None,
old: list[tuple[str, list, dict]] | None = None,
) -> None:
_process_picture(storage, file_name, new, old)
_process_picture(storage, file_name, sender, new, old)

def process_picture( # noqa: F811
storage: tuple[str, list, dict],
file_name: str,
sender: tuple[str, str, str],
new: list[tuple[str, list, dict]] | None = None,
old: list[tuple[str, list, dict]] | None = None,
) -> None:
Expand All @@ -105,6 +124,7 @@ def process_picture( # noqa: F811
kwargs=dict(
storage=storage,
file_name=file_name,
sender=sender,
new=new,
old=old,
),
Expand All @@ -123,21 +143,24 @@ def process_picture( # noqa: F811
def process_picture_with_django_rq(
storage: tuple[str, list, dict],
file_name: str,
sender: tuple[str, str, str],
new: list[tuple[str, list, dict]] | None = None,
old: list[tuple[str, list, dict]] | None = None,
) -> None:
_process_picture(storage, file_name, new, old)
_process_picture(storage, file_name, sender, new, old)

def process_picture( # noqa: F811
storage: tuple[str, list, dict],
file_name: str,
sender: tuple[str, str, str],
new: list[tuple[str, list, dict]] | None = None,
old: list[tuple[str, list, dict]] | None = None,
) -> None:
transaction.on_commit(
lambda: process_picture_with_django_rq.delay(
storage=storage,
file_name=file_name,
sender=sender,
new=new,
old=old,
)
Expand Down
3 changes: 3 additions & 0 deletions tests/test_migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from django.core.management import call_command
from django.db import models
from django.db.models.fields.files import ImageFieldFile
from django.test.utils import isolate_apps

from pictures import migrations
from pictures.models import PictureField
Expand Down Expand Up @@ -117,6 +118,7 @@ class Meta:
assert not migration.to_picture_field.called

@pytest.mark.django_db
@isolate_apps
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The @isolate_apps decorator requires an argument specifying the app label(s) to isolate. It should be used as @isolate_apps('app_label') or @isolate_apps('app1', 'app2'). Without an argument, this will not work as intended.

Suggested change
@isolate_apps
@isolate_apps("testapp")

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried that initially, doesn't work.

def test_update_pictures(self, request, stub_worker, image_upload_file):
class ToModel(models.Model):
name = models.CharField(max_length=100)
Expand Down Expand Up @@ -172,6 +174,7 @@ class Meta:
assert not luke.picture

@pytest.mark.django_db
@isolate_apps
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The @isolate_apps decorator requires an argument specifying the app label(s) to isolate. It should be used as @isolate_apps('app_label') or @isolate_apps('app1', 'app2'). Without an argument, this will not work as intended.

Suggested change
@isolate_apps
@isolate_apps("testapp")

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried that initially, doesn't work.

def test_update_pictures__with_empty_pictures(
self, request, stub_worker, image_upload_file
):
Expand Down
68 changes: 68 additions & 0 deletions tests/test_signals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
from unittest.mock import Mock

import pytest
from django.dispatch import receiver

from pictures import signals, tasks
from tests.test_migrations import skip_dramatiq
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be moved somewhere else. Otherwise, module-level fixtures will be unintentionally loaded too and introduce side effects.

from tests.testapp.models import SimpleModel


@pytest.mark.django_db
@skip_dramatiq
def test_process_picture_sends_process_picture_done(image_upload_file):
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test function name references process_picture_done, but the actual signal being tested is picture_processed. The function name should match the signal name for clarity and consistency.

Copilot uses AI. Check for mistakes.
obj = SimpleModel.objects.create(picture=image_upload_file)

handler = Mock()
signals.picture_processed.connect(handler)

tasks._process_picture(
obj.picture.storage.deconstruct(),
obj.picture.name,
obj.picture.sender,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
)

handler.assert_called_once_with(
signal=signals.picture_processed,
sender=SimpleModel._meta.get_field("picture"),
file_name=obj.picture.name,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
old=[],
)


@pytest.mark.django_db
@skip_dramatiq
def test_process_picture_sends_process_picture_done_on_create(image_upload_file):
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test function name references process_picture_done, but the actual signal being tested is picture_processed. The function name should match the signal name for clarity and consistency.

Suggested change
def test_process_picture_sends_process_picture_done_on_create(image_upload_file):
def test_process_picture_sends_picture_processed_on_create(image_upload_file):

Copilot uses AI. Check for mistakes.
handler = Mock()
signals.picture_processed.connect(handler)

obj = SimpleModel.objects.create(picture=image_upload_file)

handler.assert_called_once_with(
signal=signals.picture_processed,
sender=SimpleModel._meta.get_field("picture"),
file_name=obj.picture.name,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
old=[],
)


Comment on lines +18 to +51
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The signal handler should be disconnected after the test to prevent test pollution. Consider adding signals.picture_processed.disconnect(handler) in a try/finally block or use a pytest fixture for cleanup.

Suggested change
tasks._process_picture(
obj.picture.storage.deconstruct(),
obj.picture.name,
obj.picture.sender,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
)
handler.assert_called_once_with(
signal=signals.picture_processed,
sender=SimpleModel._meta.get_field("picture"),
file_name=obj.picture.name,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
old=[],
)
@pytest.mark.django_db
@skip_dramatiq
def test_process_picture_sends_process_picture_done_on_create(image_upload_file):
handler = Mock()
signals.picture_processed.connect(handler)
obj = SimpleModel.objects.create(picture=image_upload_file)
handler.assert_called_once_with(
signal=signals.picture_processed,
sender=SimpleModel._meta.get_field("picture"),
file_name=obj.picture.name,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
old=[],
)
try:
tasks._process_picture(
obj.picture.storage.deconstruct(),
obj.picture.name,
obj.picture.sender,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
)
handler.assert_called_once_with(
signal=signals.picture_processed,
sender=SimpleModel._meta.get_field("picture"),
file_name=obj.picture.name,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
old=[],
)
finally:
signals.picture_processed.disconnect(handler)
@pytest.mark.django_db
@skip_dramatiq
def test_process_picture_sends_process_picture_done_on_create(image_upload_file):
handler = Mock()
signals.picture_processed.connect(handler)
try:
obj = SimpleModel.objects.create(picture=image_upload_file)
handler.assert_called_once_with(
signal=signals.picture_processed,
sender=SimpleModel._meta.get_field("picture"),
file_name=obj.picture.name,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
old=[],
)
finally:
signals.picture_processed.disconnect(handler)

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +51
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The signal handler should be disconnected after the test to prevent test pollution. Consider adding signals.picture_processed.disconnect(handler) in a try/finally block or use a pytest fixture for cleanup.

Suggested change
tasks._process_picture(
obj.picture.storage.deconstruct(),
obj.picture.name,
obj.picture.sender,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
)
handler.assert_called_once_with(
signal=signals.picture_processed,
sender=SimpleModel._meta.get_field("picture"),
file_name=obj.picture.name,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
old=[],
)
@pytest.mark.django_db
@skip_dramatiq
def test_process_picture_sends_process_picture_done_on_create(image_upload_file):
handler = Mock()
signals.picture_processed.connect(handler)
obj = SimpleModel.objects.create(picture=image_upload_file)
handler.assert_called_once_with(
signal=signals.picture_processed,
sender=SimpleModel._meta.get_field("picture"),
file_name=obj.picture.name,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
old=[],
)
try:
tasks._process_picture(
obj.picture.storage.deconstruct(),
obj.picture.name,
obj.picture.sender,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
)
handler.assert_called_once_with(
signal=signals.picture_processed,
sender=SimpleModel._meta.get_field("picture"),
file_name=obj.picture.name,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
old=[],
)
finally:
signals.picture_processed.disconnect(handler)
@pytest.mark.django_db
@skip_dramatiq
def test_process_picture_sends_process_picture_done_on_create(image_upload_file):
handler = Mock()
signals.picture_processed.connect(handler)
try:
obj = SimpleModel.objects.create(picture=image_upload_file)
handler.assert_called_once_with(
signal=signals.picture_processed,
sender=SimpleModel._meta.get_field("picture"),
file_name=obj.picture.name,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
old=[],
)
finally:
signals.picture_processed.disconnect(handler)

Copilot uses AI. Check for mistakes.
@pytest.mark.django_db
@skip_dramatiq
def test_processed_object_found(image_upload_file):
obj = SimpleModel.objects.create()

found_object = None

@receiver(signals.picture_processed, sender=SimpleModel._meta.get_field("picture"))
def handler(*, sender, file_name, **__):
nonlocal found_object

# Users can now modify the object that picture_processed corresponds to
found_object = sender.model.objects.get(**{sender.name: file_name})

obj.picture.save("image.png", image_upload_file)
Comment on lines +59 to +66
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The signal handler registered with @receiver decorator is not cleaned up after the test completes. This could cause test pollution and interfere with other tests. Consider using a try/finally block or pytest fixture to ensure the handler is disconnected after the test, or use .connect() with manual cleanup instead of the decorator.

Suggested change
@receiver(signals.picture_processed, sender=SimpleModel._meta.get_field("picture"))
def handler(*, sender, file_name, **__):
nonlocal found_object
# Users can now modify the object that picture_processed corresponds to
found_object = sender.model.objects.get(**{sender.name: file_name})
obj.picture.save("image.png", image_upload_file)
def handler(*, sender, file_name, **__):
nonlocal found_object
# Users can now modify the object that picture_processed corresponds to
found_object = sender.model.objects.get(**{sender.name: file_name})
signals.picture_processed.connect(handler, sender=SimpleModel._meta.get_field("picture"))
try:
obj.picture.save("image.png", image_upload_file)
finally:
signals.picture_processed.disconnect(handler, sender=SimpleModel._meta.get_field("picture"))

Copilot uses AI. Check for mistakes.

assert obj == found_object
1 change: 1 addition & 0 deletions tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def test_process_picture__file_cannot_be_reopened(image_upload_file):
tasks._process_picture(
obj.picture.storage.deconstruct(),
obj.picture.name,
obj.picture.sender,
new=[i.deconstruct() for i in obj.picture.get_picture_files_list()],
)

Expand Down