From f4d3ae9197ced6f6fe16f95eddd926c3ad0686ed Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Mon, 8 Apr 2024 11:18:45 -0500 Subject: [PATCH 1/6] Add FileField as a supported field --- modeltrans/fields.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modeltrans/fields.py b/modeltrans/fields.py index 04da6f3..eee2c2d 100644 --- a/modeltrans/fields.py +++ b/modeltrans/fields.py @@ -13,7 +13,7 @@ get_translated_field_label, ) -SUPPORTED_FIELDS = (fields.CharField, fields.TextField, JSONField) +SUPPORTED_FIELDS = (fields.CharField, fields.TextField, JSONField, fields.files.FileField) DEFAULT_LANGUAGE = get_default_language() From 7898102489ad0d344879b2564caa559e0b33d164 Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Mon, 13 May 2024 09:48:39 -0500 Subject: [PATCH 2/6] Ensure consistent sorting in tests --- tests/test_querysets.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_querysets.py b/tests/test_querysets.py index 05f1fac..eb11197 100644 --- a/tests/test_querysets.py +++ b/tests/test_querysets.py @@ -4,6 +4,7 @@ import django from django.db import models from django.db.models import F, Q +from django.db.models.functions import Collate from django.test import TestCase, override_settings from django.utils.translation import override @@ -489,9 +490,9 @@ def test_order_by_lower(self): filtered = Blog.objects.filter(category=c) - # order by title should result in aA because it is case sensitive. - qs = filtered.order_by("title", "title_nl") - self.assertEqual(key(qs, "title"), "a A") + # order by title should result in Aa because it is case sensitive. + qs = filtered.order_by(Collate("title", "C"), Collate("title_nl", "C")) + self.assertEqual(key(qs, "title"), "A a") # order by Lower('title') should result in Aa because lower('A') == lower('A') # so the title_nl field should determine the sorting From 8ee41ee39dd97f6250f0b70d5e1d73bc47be4c50 Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Mon, 13 May 2024 11:59:46 -0500 Subject: [PATCH 3/6] Switch to InMemoryStorage for tests --- tests/app/settings.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/app/settings.py b/tests/app/settings.py index 8b65f34..11645da 100644 --- a/tests/app/settings.py +++ b/tests/app/settings.py @@ -103,3 +103,12 @@ USE_TZ = True STATIC_URL = "/static/" + +STORAGES = { + "default": { + "BACKEND": "django.core.files.storage.InMemoryStorage", + }, + "staticfiles": { + "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage", + }, +} From d782d5affbc68bd465015c3b97cd3a4497c9cf5e Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Mon, 13 May 2024 12:01:42 -0500 Subject: [PATCH 4/6] Add failing tests for FileField support --- tests/app/models.py | 15 +++++++++++++++ tests/test_models.py | 33 +++++++++++++++++++++++++++++++++ tests/test_translating.py | 1 + 3 files changed, 49 insertions(+) diff --git a/tests/app/models.py b/tests/app/models.py index c461c15..97cbdb4 100644 --- a/tests/app/models.py +++ b/tests/app/models.py @@ -234,3 +234,18 @@ class Comment(models.Model): def __str__(self): return self.text + + +class Attachment(models.Model): + post = models.ForeignKey( + Post, + on_delete=models.CASCADE, + limit_choices_to={"is_published": True}, + related_name="attachments", + ) + file = models.FileField() + + i18n = TranslationField(fields=("file",)) + + def __str__(self): + return self.file.name diff --git a/tests/test_models.py b/tests/test_models.py index 57f2eae..8637ee2 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -1,4 +1,5 @@ from django.core.exceptions import ValidationError +from django.core.files.base import ContentFile from django.db import DataError, models, transaction from django.test import TestCase, override_settings from django.utils.translation import override @@ -7,11 +8,13 @@ from .app.models import ( Article, + Attachment, Blog, Challenge, ChallengeContent, ChildArticle, NullableTextModel, + Post, TaggedBlog, TextModel, ) @@ -137,6 +140,36 @@ def test_fallback_getting_JSONField(self): # an empty list) self.assertEqual(m.tags_i18n, []) + def test_fallback_getting_FileField(self): + post = Post.objects.create(title="Test Post", is_published=True) + sample_file = ContentFile("sample content", name="sample-en.txt") + attachment = Attachment.objects.create(post=post, file=sample_file) + default_file_name = attachment.file.name + assert default_file_name + + with override("fr"): + self.assertIsInstance(attachment.file_i18n, models.fields.files.FieldFile) + self.assertEqual(attachment.file_i18n.name, default_file_name) + + def test_set_FileField(self): + post = Post.objects.create(title="Test Post", is_published=True) + + en_content = "sample content" + fr_content = "exemple de contenu" + sample_file_en = ContentFile(en_content, name="sample-en.txt") + sample_file_fr = ContentFile(fr_content, name="sample-fr.txt") + + attachment = Attachment.objects.create( + post=post, file=sample_file_en, file_fr=sample_file_fr + ) + + saved_fr_content = attachment.file_fr.read().decode("utf-8") + self.assertEqual(saved_fr_content, fr_content) + self.assertIsInstance(attachment.file_fr, models.fields.files.FieldFile) + + with override("fr"): + self.assertEqual(attachment.file_i18n, attachment.file_fr) + def test_creating_using_virtual_default_language_field(self): m = Blog.objects.create(title_en="Falcon") diff --git a/tests/test_translating.py b/tests/test_translating.py index f3281b7..cee52f7 100644 --- a/tests/test_translating.py +++ b/tests/test_translating.py @@ -45,6 +45,7 @@ def test_get_translated_models(self): app_models.Challenge, app_models.ChallengeContent, app_models.Post, + app_models.Attachment, app_models.Comment, } self.assertEqual(set(get_translated_models("app")), expected) From 593009be2c146c7c475b50baa9a0a9678a3757ec Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Mon, 13 May 2024 12:09:25 -0500 Subject: [PATCH 5/6] Implement FileField support Adding `get_localized_value` allows us mimic how a "native" FileField is instantiated: https://github.com/django/django/blob/629398e55fd260c2ac4c2a9fc8b0c7d8dbda9e56/django/db/models/fields/files.py#L166 `TranslationField.pre_save` is modified to mimic how `pre_save` calls work as well: https://github.com/django/django/blob/629398e55fd260c2ac4c2a9fc8b0c7d8dbda9e56/django/db/models/fields/files.py#L313 The virtual FileField instances are not considered concrete, so are not processed within the ORM: https://github.com/django/django/blob/629398e55fd260c2ac4c2a9fc8b0c7d8dbda9e56/django/db/models/sql/compiler.py#L1744 --- modeltrans/fields.py | 48 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/modeltrans/fields.py b/modeltrans/fields.py index eee2c2d..5e0a2fa 100644 --- a/modeltrans/fields.py +++ b/modeltrans/fields.py @@ -108,6 +108,15 @@ def get_instance_fallback_chain(self, instance, language): return default + def get_localized_value(self, instance, field_name): + value = instance.i18n.get(field_name) + + if isinstance(self.original_field, fields.files.FileField): + # TODO: Review this versus `descriptor_class`; need to write some additional tests to verify + return self.attr_class(instance, self, value) + + return value + def __get__(self, instance, instance_type=None): # This method is apparently called with instance=None from django. # django-hstor raises AttributeError here, but that doesn't solve our problem. @@ -132,7 +141,7 @@ def __get__(self, instance, instance_type=None): # Just return the value if this is an explicit field (_) if self.language is not None: - return instance.i18n.get(field_name) + return self.get_localized_value(instance, field_name) # This is the _i18n version of the field, and the current language is not available, # so we walk the fallback chain: @@ -145,7 +154,7 @@ def __get__(self, instance, instance_type=None): field_name = build_localized_fieldname(self.original_name, fallback_language) if field_name in instance.i18n and instance.i18n[field_name]: - return instance.i18n.get(field_name) + return self.get_localized_value(instance, field_name) # finally, return the original field if all else fails. return getattr(instance, self.original_name) @@ -307,8 +316,43 @@ def get_translated_fields(self): if isinstance(field, TranslatedVirtualField): yield field + def get_file_fields(self): + """Return a generator for all translated FileFields.""" + for field in self.get_translated_fields(): + if isinstance(field.original_field, fields.files.FileField): + yield field + def contribute_to_class(self, cls, name): if name != "i18n": raise ImproperlyConfigured('{} must have name "i18n"'.format(self.__class__.__name__)) super().contribute_to_class(cls, name) + + def pre_save(self, model_instance, add): + """Ensure that translated field values are serializable before save""" + data = super().pre_save(model_instance, add) + + if data is None: + return data + + for field in self.get_file_fields(): + localized_file_attname = field.attname + if localized_file_attname in data: + current_value = data[localized_file_attname] + + # wrap the value in the descriptor class, which will set + # the value within the model instance, _NOT_ the i18n key + descriptor = field.descriptor_class(field) + descriptor.__set__(model_instance, current_value) + + # retrieve the descriptor value, which will check to see if the + # file reference has been committed + file = descriptor.__get__(model_instance) + if file and not file._committed: + # Commit the file to storage prior to saving the model + file.save(file.name, file.file, save=False) + + # finally, mimic how `get_prep_value` works + # for "concrete" FileField instances + data[localized_file_attname] = str(file) + return data From 7cd0a2b7d570eb5c7da269d17e8e030125bd27cf Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Mon, 13 May 2024 13:37:13 -0500 Subject: [PATCH 6/6] Ensure virtual FileField instances mirror FileField Use descriptor_class to mimic how FileField returns a FieldFile instance https://github.com/django/django/blob/629398e55fd260c2ac4c2a9fc8b0c7d8dbda9e56/django/db/models/fields/files.py#L321 https://github.com/django/django/blob/629398e55fd260c2ac4c2a9fc8b0c7d8dbda9e56/django/db/models/fields/files.py#L166 --- modeltrans/fields.py | 5 +++-- tests/test_models.py | 51 ++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/modeltrans/fields.py b/modeltrans/fields.py index 5e0a2fa..1aa7331 100644 --- a/modeltrans/fields.py +++ b/modeltrans/fields.py @@ -112,8 +112,9 @@ def get_localized_value(self, instance, field_name): value = instance.i18n.get(field_name) if isinstance(self.original_field, fields.files.FileField): - # TODO: Review this versus `descriptor_class`; need to write some additional tests to verify - return self.attr_class(instance, self, value) + descriptor = self.descriptor_class(self) + descriptor.__set__(instance, value) + return descriptor.__get__(instance) return value diff --git a/tests/test_models.py b/tests/test_models.py index 8637ee2..bf1c411 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -1,5 +1,6 @@ from django.core.exceptions import ValidationError from django.core.files.base import ContentFile +from django.core.files.storage.memory import InMemoryFileNode from django.db import DataError, models, transaction from django.test import TestCase, override_settings from django.utils.translation import override @@ -144,12 +145,10 @@ def test_fallback_getting_FileField(self): post = Post.objects.create(title="Test Post", is_published=True) sample_file = ContentFile("sample content", name="sample-en.txt") attachment = Attachment.objects.create(post=post, file=sample_file) - default_file_name = attachment.file.name - assert default_file_name with override("fr"): self.assertIsInstance(attachment.file_i18n, models.fields.files.FieldFile) - self.assertEqual(attachment.file_i18n.name, default_file_name) + self.assertIsInstance(attachment.file_i18n.file, InMemoryFileNode) def test_set_FileField(self): post = Post.objects.create(title="Test Post", is_published=True) @@ -162,14 +161,58 @@ def test_set_FileField(self): attachment = Attachment.objects.create( post=post, file=sample_file_en, file_fr=sample_file_fr ) + self.assertIsInstance(attachment.file_fr, models.fields.files.FieldFile) + + self.assertIsInstance(attachment.file.file, InMemoryFileNode) + self.assertIsInstance(attachment.file_fr.file, InMemoryFileNode) saved_fr_content = attachment.file_fr.read().decode("utf-8") self.assertEqual(saved_fr_content, fr_content) - self.assertIsInstance(attachment.file_fr, models.fields.files.FieldFile) with override("fr"): self.assertEqual(attachment.file_i18n, attachment.file_fr) + def test_FileField_getter(self): + post = Post.objects.create(title="Test Post", is_published=True) + + fr_content = "exemple de contenu 2" + sample_file_fr = ContentFile(fr_content, name="sample-fr-2.txt") + + attachment = Attachment(post=post, file_fr=sample_file_fr) + # prior to invoking save, the file_fr should be an instance of FieldFile + self.assertIsInstance(attachment.file_fr, models.fields.files.FieldFile) + # but the file object should be an instance of ContentFile + self.assertIsInstance(attachment.file_fr.file, ContentFile) + attachment.save() + + # After saving, the file object should be the default storage class + self.assertIsInstance(attachment.file_fr.file, InMemoryFileNode) + + # Retreiving the instance from the database, file and file_fr + # should return the same kind of interface (a FieldFile instance) + attachment = Attachment.objects.get(pk=attachment.pk) + self.assertIsInstance(attachment.file, models.fields.files.FieldFile) + self.assertIsInstance(attachment.file_fr, models.fields.files.FieldFile) + + # Test that we can overwrite those with new content + new_content = "new content" + new_fr_content = "new French content" + attachment.file = ContentFile(new_content, name="content-new.txt") + attachment.file_fr = ContentFile(new_fr_content, name="content-new-fr.txt") + self.assertIsInstance(attachment.file, models.fields.files.FieldFile) + self.assertIsInstance(attachment.file_fr, models.fields.files.FieldFile) + + attachment.save() + + self.assertIsInstance(attachment.file_fr, models.fields.files.FieldFile) + self.assertIsInstance(attachment.file, models.fields.files.FieldFile) + + with attachment.file.open() as f: + self.assertEqual(f.read().decode("utf-8"), new_content) + + with attachment.file_fr.open() as f: + self.assertEqual(f.read().decode("utf-8"), new_fr_content) + def test_creating_using_virtual_default_language_field(self): m = Blog.objects.create(title_en="Falcon")