Skip to content

Commit

Permalink
Represent FileFields as their file path+name in the captured state
Browse files Browse the repository at this point in the history
This means a FileField is only 'dirty' if the file path/name changes.
This fixes FileField performance regression caused by Django 3.1
  • Loading branch information
2019342a authored and LincolnPuzey committed Jul 16, 2022
1 parent 1a64d33 commit 6c430e7
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 12 deletions.
4 changes: 4 additions & 0 deletions ChangeLog.rst
Expand Up @@ -6,6 +6,10 @@ ChangeLog
unreleased
------

*Changed:*
- Only look at the file name when determining if FileFields are dirty.
- Return only the file name in :code:`get_dirty_fields()` for FileFields.


.. _v1.8.2:

Expand Down
6 changes: 6 additions & 0 deletions src/dirtyfields/dirtyfields.py
@@ -1,6 +1,7 @@
from copy import deepcopy

from django.core.exceptions import ValidationError
from django.core.files import File
from django.db.models.expressions import BaseExpression
from django.db.models.expressions import Combinable
from django.db.models.signals import post_save, m2m_changed
Expand Down Expand Up @@ -79,6 +80,11 @@ def _as_dict(self, check_relationship, include_primary_key=True):

field_value = getattr(self, field.attname)

if isinstance(field_value, File):
# Uses the name for files due to a perfomance regression caused by Django 3.1.
# For more info see: https://github.com/romgar/django-dirtyfields/issues/165
field_value = field_value.name

# If current field value is an expression, we are not evaluating it
if isinstance(field_value, (BaseExpression, Combinable)):
continue
Expand Down
13 changes: 6 additions & 7 deletions tests/test_core.py
Expand Up @@ -10,7 +10,6 @@
ModelWithOneToOneFieldTest,
SubclassModelTest, ModelWithDecimalFieldTest,
FileFieldModel, OrdinaryModelTest, OrdinaryWithDirtyFieldsProxy)
from .utils import FakeFieldFile


def test_version_numbers():
Expand Down Expand Up @@ -219,19 +218,19 @@ def test_refresh_from_db_no_fields():
def test_file_fields_content_file():
tm = FileFieldModel()
# field is dirty because model is unsaved
assert tm.get_dirty_fields() == {"file1": FakeFieldFile("")}
assert tm.get_dirty_fields() == {"file1": ""}
tm.save()
assert tm.get_dirty_fields() == {}

# set file makes field dirty
tm.file1.save("test-file-1.txt", ContentFile(b"Test file content 1"), save=False)
assert tm.get_dirty_fields() == {"file1": FakeFieldFile("")}
assert tm.get_dirty_fields() == {"file1": ""}
tm.save()
assert tm.get_dirty_fields() == {}

# change file makes field dirty
tm.file1.save("test-file-2.txt", ContentFile(b"Test file content 2"), save=False)
assert tm.get_dirty_fields() == {"file1": FakeFieldFile("file1/test-file-1.txt")}
assert tm.get_dirty_fields() == {"file1": "file1/test-file-1.txt"}
tm.save()
assert tm.get_dirty_fields() == {}

Expand All @@ -240,21 +239,21 @@ def test_file_fields_content_file():
def test_file_fields_real_file():
tm = FileFieldModel()
# field is dirty because model is unsaved
assert tm.get_dirty_fields() == {"file1": FakeFieldFile("")}
assert tm.get_dirty_fields() == {"file1": ""}
tm.save()
assert tm.get_dirty_fields() == {}

# set file makes field dirty
with open(join(dirname(__file__), "files", "foo.txt"), "rb") as f:
tm.file1.save("test-file-3.txt", File(f), save=False)
assert tm.get_dirty_fields() == {"file1": FakeFieldFile("")}
assert tm.get_dirty_fields() == {"file1": ""}
tm.save()
assert tm.get_dirty_fields() == {}

# change file makes field dirty
with open(join(dirname(__file__), "files", "bar.txt"), "rb") as f:
tm.file1.save("test-file-4.txt", File(f), save=False)
assert tm.get_dirty_fields() == {"file1": FakeFieldFile("file1/test-file-3.txt")}
assert tm.get_dirty_fields() == {"file1": "file1/test-file-3.txt"}
tm.save()
assert tm.get_dirty_fields() == {}

Expand Down
5 changes: 0 additions & 5 deletions tests/utils.py
@@ -1,5 +1,4 @@
import re
from collections import namedtuple

from django.conf import settings
from django.db import connection
Expand Down Expand Up @@ -63,7 +62,3 @@ def is_postgresql_env_with_jsonb_field():
PG_VERSION = 0

return PG_VERSION >= 90400


# Will compare equal with a django `FieldFile` instance in tests.
FakeFieldFile = namedtuple("FakeFieldFile", ["name"])

0 comments on commit 6c430e7

Please sign in to comment.