From 5d7ac0e2c0421025773bf548bb646254ff808bc1 Mon Sep 17 00:00:00 2001 From: andrey-skvortsov Date: Wed, 28 Feb 2018 13:04:03 +0000 Subject: [PATCH] Add native support translated fields in queryset.only() --- .travis.yml | 2 +- example/article/tests.py | 1 - parler/fields.py | 40 -------------- parler/managers.py | 48 ++++++++++++----- parler/models.py | 36 +++++++++---- parler/tests/test_model_construction.py | 9 +--- parler/tests/test_query_count.py | 8 +-- parler/tests/test_querysets.py | 69 +++++++++++++++++++++++++ parler/utils/fields.py | 10 ++-- setup.py | 5 +- tox.ini | 4 +- 11 files changed, 149 insertions(+), 83 deletions(-) create mode 100644 parler/tests/test_querysets.py diff --git a/.travis.yml b/.travis.yml index 6f40ca8e..82506b0b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -35,7 +35,7 @@ before_install: - pip install codecov install: - pip install -U pip wheel setuptools -- pip install django-composite-foreignkey +- pip install -e git+https://github.com/onysos/django-composite-foreignkey.git@7ac6b5fa7a54ddc6f527f648d87fec87540ea00e#egg=django-composite-foreignkey-1.0.1 - travis_retry pip install $DJANGO -e . script: - coverage run --rcfile=.coveragerc runtests.py diff --git a/example/article/tests.py b/example/article/tests.py index f89e963e..0775caa2 100644 --- a/example/article/tests.py +++ b/example/article/tests.py @@ -255,7 +255,6 @@ def test_admin_delete_translation(self): self.assertEqual(200, resp.status_code) self.assertTemplateUsed(resp, 'admin/parler/deletion_not_allowed.html') - @expectedFailure def test_admin_delete_translation_unavailable(self): """ To be fixed : when trying to delete the last language when a translation diff --git a/parler/fields.py b/parler/fields.py index 8e981e1d..bc38844e 100644 --- a/parler/fields.py +++ b/parler/fields.py @@ -13,10 +13,6 @@ import django from django.forms.forms import pretty_name -from parler.utils.i18n import get_language - -if (1, 8) <= django.VERSION < (2, 0): - from compositefk.fields import RawFieldValue, CompositeOneToOneField # TODO: inherit RelatedField? @@ -166,39 +162,3 @@ def __set__(self, instance, value): def __delete__(self, instance): raise AttributeError("The 'language_code' attribute cannot be deleted!") - - -class DONOTHING(object): - pass - - -if (1, 8) <= django.VERSION < (2, 0): - class CompositeOneToOneVirtualField(CompositeOneToOneField): - """ - Class to fix problem with creation repetitive migrations - """ - def deconstruct(self): - name, path, args, kwargs = super(CompositeOneToOneVirtualField, self).deconstruct() - if 'to_fields' in kwargs: - kwargs['to_fields'] = {'master_id': None, 'language_code': None} # hack: Need always the same dict - if "on_delete" in kwargs: - kwargs['on_delete'] = DONOTHING # hack: Need always the same global object with __module__ attr - if "null_if_equal" in kwargs: - del kwargs['null_if_equal'] - return name, path, args, kwargs - - - class RawActiveLangFieldValue(RawFieldValue): - """ - Raw value with active language - """ - def __init__(self): - super(RawActiveLangFieldValue, self).__init__(None) - - @property - def value(self): - return get_language() - - @value.setter - def value(self, value): - pass diff --git a/parler/managers.py b/parler/managers.py index 1a25d66b..11357adb 100644 --- a/parler/managers.py +++ b/parler/managers.py @@ -9,7 +9,7 @@ from django.utils import six from parler import appsettings from parler.utils import get_active_language_choices -from parler.utils.fields import get_extra_related_translalation_paths +from parler.utils.fields import get_extra_related_translation_paths class SelectRelatedTranslationsQuerySetMixin(object): @@ -24,7 +24,7 @@ class SelectRelatedTranslationsQuerySetMixin(object): def select_related(self, *fields): extra_paths = [] for field in fields: - extra_paths += get_extra_related_translalation_paths(self.model, field) + extra_paths += get_extra_related_translation_paths(self.model, field) if extra_paths: fields = tuple(set(extra_paths)) + fields return super(SelectRelatedTranslationsQuerySetMixin, self).select_related(*fields) @@ -52,20 +52,38 @@ def __init__(self, *args, **kwargs): self._language = None def select_related(self, *fields): + """ + Updates select_related to have active and default always together + Replaces main field refer to translations ('translations') with 'translations_active' and 'translations_default' + """ fields_to_add = set() - fields_to_exclude = set([None]) # if rel_name_active, rel_name_default is None + fields_to_exclude = set() for extension in self.model._parler_meta: - if extension.rel_name in fields: + select_related_translations_fields = extension.get_select_related_translations_fields() + fields_to_search = set(select_related_translations_fields + [extension.rel_name]) + if fields_to_search.intersection(fields): fields_to_exclude.add(extension.rel_name) # Can not select related OneToMany field - fields_to_add.add(extension.rel_name_active) - fields_to_add.add(extension.rel_name_default) - if extension.rel_name_active in fields: - fields_to_add.add(extension.rel_name_default) - if extension.rel_name_default in fields: - fields_to_add.add(extension.rel_name_active) + fields_to_add.update(select_related_translations_fields) fields = set(fields).union(fields_to_add).difference(fields_to_exclude) return super(TranslatableQuerySet, self).select_related(*tuple(fields)) + def only(self, *fields): + """ + Replaces translated fields with 'translations_active' and 'translations_default' + pretending they are in original model so we can use .only + for translated fields as usual: .objects.only('some_translated_field') + """ + fields_to_add = set() + fields_to_exclude = set() + for extension in self.model._parler_meta: + select_related_translations_fields = extension.get_select_related_translations_fields() + translated_fields = set(extension.get_translated_fields()).intersection(fields) + if translated_fields: + fields_to_exclude.update(translated_fields) # Can not select related field form translated model (o2m) + fields_to_add.update(select_related_translations_fields) + fields = set(fields).union(fields_to_add).difference(fields_to_exclude) + return super(TranslatableQuerySet, self).only(*tuple(fields)) + if (1, 9) <= django.VERSION: def _values(self, *fields): result = super(TranslatableQuerySet, self)._values(*fields) @@ -89,15 +107,17 @@ def create(self, **kwargs): return super(TranslatableQuerySet, self).create(**kwargs) def _add_active_default_select_related(self): + """ + Auto-adds select_related for active and default languages. + Takes in account deferred fields. + """ existing, defer = self.query.deferred_loading related_to_add = set() for extension in self.model._parler_meta: if not extension.rel_name: continue - if extension.rel_name_active: - related_to_add.add(extension.rel_name_active) - if extension.rel_name_default: - related_to_add.add(extension.rel_name_default) + select_related_translations_fields = extension.get_select_related_translations_fields() + related_to_add.update(select_related_translations_fields) if defer: related_to_add = related_to_add.difference(existing) elif existing: diff --git a/parler/models.py b/parler/models.py index e81543f7..194a68ba 100644 --- a/parler/models.py +++ b/parler/models.py @@ -94,9 +94,8 @@ class Meta: else: from django.db.models.fields.related import ReverseSingleRelatedObjectDescriptor as ForwardManyToOneDescriptor -if (1, 8) <= django.VERSION < (2, 0): - from compositefk.fields import RawFieldValue - from parler.fields import CompositeOneToOneVirtualField, RawActiveLangFieldValue +if django.VERSION >= (1, 8): + from compositefk.fields import RawFieldValue, FunctionBasedFieldValue, CompositeOneToOneField __all__ = ( @@ -143,32 +142,39 @@ def create_translations_composite_fk(shared_model, related_name, translated_mode Note: django-composite-foreignkey does not work in django 1.7 and 2+ """ - if not (1, 8) <= django.VERSION < (2, 0): + if django.VERSION < (1, 8): return meta = shared_model._parler_meta._get_extension_by_related_name(related_name) meta.rel_name_active = related_name + '_active' meta.rel_name_default = related_name + '_default' - translations_active = CompositeOneToOneVirtualField( + translations_active = CompositeOneToOneField( translated_model, null=True, on_delete=models.DO_NOTHING, - related_name='master_active', + related_name='+', to_fields={ 'master_id': shared_model._meta.pk.name, - 'language_code': RawActiveLangFieldValue() + 'language_code': FunctionBasedFieldValue(get_language) }) + # Needs hack here. + # Set one_to_one = False as Django treat this field as a reversed + # see: django.db.models.sql.query.is_reverse_o2o + # Django does not include this field to 'must query fields', so it became deferred field if it used with only. + # To be able use the field in select_related field must be not deferred. + translations_active.one_to_one = False translations_active.contribute_to_class(shared_model, meta.rel_name_active) - translations_default = CompositeOneToOneVirtualField( + translations_default = CompositeOneToOneField( translated_model, null=True, on_delete=models.DO_NOTHING, - related_name='master_default', + related_name='+', to_fields={ 'master_id': shared_model._meta.pk.name, 'language_code': RawFieldValue(appsettings.PARLER_LANGUAGES.get_default_language()) }) + translations_default.one_to_one = False translations_default.contribute_to_class(shared_model, meta.rel_name_default) @@ -1175,6 +1181,14 @@ def __repr__(self): self.model.__name__ ) + def get_select_related_translations_fields(self): + result = [] + if self.rel_name_active: + result.append(self.rel_name_active) + if self.rel_name_default: + result.append(self.rel_name_default) + return result + class ParlerOptions(object): """ @@ -1298,6 +1312,10 @@ def get_translated_fields(self, related_name=None): meta = self._get_extension_by_related_name(related_name) return meta.get_translated_fields() + def get_select_related_translations_fields(self, related_name=None): + meta = self._get_extension_by_related_name(related_name) + return meta.get_select_related_translations_fields() + def get_model_by_field(self, name): """ Find the :class:`TranslatedFieldsModel` that contains the given field. diff --git a/parler/tests/test_model_construction.py b/parler/tests/test_model_construction.py index 7233eff4..02050107 100644 --- a/parler/tests/test_model_construction.py +++ b/parler/tests/test_model_construction.py @@ -1,18 +1,11 @@ from functools import wraps - +from unittest import expectedFailure, skipIf import django from django.db import models from django.db.models import Manager from django.utils import six from parler.models import TranslatableModel from parler.models import TranslatedFields - -try: - from unittest import expectedFailure, skipIf -except ImportError: - # python<2.7 - from django.utils.unittest import expectedFailure, skipIf - from .utils import AppTestCase from .testapp.models import ManualModel, ManualModelTranslations, SimpleModel, Level1, Level2, ProxyBase, ProxyModel, DoubleModel, RegularModel, CharModel diff --git a/parler/tests/test_query_count.py b/parler/tests/test_query_count.py index f0bab544..c2cea752 100644 --- a/parler/tests/test_query_count.py +++ b/parler/tests/test_query_count.py @@ -65,7 +65,7 @@ def test_qs(): with translation.override(language_code): self.assertNumQueries(num, test_qs) - @skipIf((1, 8) <= django.VERSION < (2, 0), 'Test for django ver 1.7, 2') + @skipIf((1, 8) <= django.VERSION , 'Test for django ver 1.7') def test_uncached_queries(self): """ Test that uncached queries work, albeit slowly. @@ -73,7 +73,7 @@ def test_uncached_queries(self): with override_parler_settings(PARLER_ENABLE_CACHING=False): self.assertNumTranslatedQueries(1 + len(self.country_list), SimpleModel.objects.all()) - @skipIf(not (1, 8) <= django.VERSION < (2, 0), 'Test for django ver 1.8, 1.9, 1.10, 1.11') + @skipIf(django.VERSION < (1, 8), 'Test for django ver > 1.7') def test_uncached_queries_with_force_select_related(self): """ Test that uncached queries work, albeit slowly. @@ -82,7 +82,7 @@ def test_uncached_queries_with_force_select_related(self): self.assertNumTranslatedQueries(1, SimpleModel.objects.all().select_related('translations')) self.assertNumTranslatedQueries(1, SimpleModel.objects.all()) - @skipIf(not (1, 8) <= django.VERSION < (2, 0), 'Test for django ver 1.8, 1.9, 1.10, 1.11') + @skipIf(django.VERSION < (1, 8), 'Test for django ver > 1.7') def test_uncached_queries_with_using_select_related(self): """ Test that uncached queries work, albeit slowly. @@ -121,7 +121,7 @@ def test_model_cache_queries(self): with override_parler_settings(PARLER_ENABLE_CACHING=False): qs = SimpleModel.objects.all() - if (1, 8) <= django.VERSION < (2, 0): + if (1, 8) <= django.VERSION: self.assertNumTranslatedQueries(1, qs) else: self.assertNumTranslatedQueries(1 + len(self.country_list), qs) diff --git a/parler/tests/test_querysets.py b/parler/tests/test_querysets.py new file mode 100644 index 00000000..583db04e --- /dev/null +++ b/parler/tests/test_querysets.py @@ -0,0 +1,69 @@ +from __future__ import absolute_import, unicode_literals +from unittest import skipIf +import django +from django.utils import translation +from .utils import AppTestCase +from .testapp.models import SimpleModel, SimpleLightModel + + +class QuerySetsTests(AppTestCase): + def setUp(self): + super(QuerySetsTests, self).setUp() + self.title = 'TITLE_XX' + self.id = SimpleModel.objects.create(tr_title=self.title).pk + self.light_model_id = SimpleLightModel.objects.create(tr_title=self.title).pk + + def assertNumTranslatedQueries(self, num, qs): + def test_qs(): + for obj in qs: + str(obj.tr_title) + self.assertNumQueries(num, test_qs) + + @skipIf(django.VERSION < (1, 8), 'Test for django ver > 1.7') + def test_select_related_light_model(self): + with translation.override('ca-fr'): + qs = SimpleLightModel.objects.select_related('translations').filter(pk=self.light_model_id) + + self.assertNumTranslatedQueries(1, qs.select_related('translations')) + self.assertNumTranslatedQueries(1, qs.select_related('translations_active')) + + x = SimpleLightModel.objects.select_related('translations').get(pk=self.light_model_id) + self.assertEqual(x.tr_title, self.title) + + x = SimpleLightModel.objects.select_related('translations_active').get(pk=self.light_model_id) + self.assertEqual(x.tr_title, self.title) + + @skipIf(django.VERSION < (1, 8), 'Test for django ver > 1.7') + def test_select_related_force_model(self): + with translation.override('ca-fr'): + qs = SimpleModel.objects.select_related('translations').filter(pk=self.id) + + self.assertNumTranslatedQueries(1, qs.select_related('translations')) + self.assertNumTranslatedQueries(1, qs.select_related('translations_active')) + + x = SimpleModel.objects.select_related('translations').get(pk=self.id) + self.assertEqual(x.tr_title, self.title) + + x = SimpleModel.objects.select_related('translations_active').get(pk=self.id) + self.assertEqual(x.tr_title, self.title) + + @skipIf(django.VERSION < (1, 8), 'Test for django ver > 1.7') + def test_only(self): + with translation.override('ca-fr'): + qs = SimpleModel.objects.all().only('id') + self.assertNumTranslatedQueries(2, qs) # needs query for ca-fr + + qs = SimpleModel.objects.all().only('id', 'translations_default') # needs query for ca-fr + self.assertNumTranslatedQueries(2, qs) + + qs = SimpleModel.objects.all().only('id', 'translations_active') # active 'ca-fr' should been select_related + self.assertNumTranslatedQueries(1, qs) + + qs = SimpleModel.objects.all().only('id', 'tr_title') # should be replaced with active and default + self.assertNumTranslatedQueries(1, qs) + + x = SimpleModel.objects.all().only('id').get(pk=self.id) + self.assertEqual(x.tr_title, self.title) + + x = SimpleModel.objects.all().only('id', 'tr_title').get(pk=self.id) + self.assertEqual(x.tr_title, self.title) diff --git a/parler/utils/fields.py b/parler/utils/fields.py index c027d1d3..8256b849 100644 --- a/parler/utils/fields.py +++ b/parler/utils/fields.py @@ -9,13 +9,15 @@ class NotRelationField(Exception): def get_model_from_relation(field): # type: (django.db.models.fields.Field) -> models.Model - if hasattr(field, 'get_path_info'): - return field.get_path_info()[-1].to_opts.model - else: + try: + path_info = field.get_path_info() + except AttributeError: raise NotRelationField + else: + return path_info[-1].to_opts.model -def get_extra_related_translalation_paths(model, path): +def get_extra_related_translation_paths(model, path): # type: (models.Model, str) -> List[str] """ Returns paths with active and default transalation models for all Translatable models in path diff --git a/setup.py b/setup.py index 76649aaf..1b343f9e 100755 --- a/setup.py +++ b/setup.py @@ -40,7 +40,7 @@ def find_version(*parts): install_requires=[ 'Django (>=1.7)', - 'django-composite-foreignkey (>=1.0.0.a10)', + 'django-composite-foreignkey (==1.0.1)', ], description='Simple Django model translations without nasty hacks, featuring nice admin integration.', @@ -80,5 +80,8 @@ def find_version(*parts): 'Topic :: Internet :: WWW/HTTP :: Dynamic Content', 'Topic :: Software Development :: Libraries :: Application Frameworks', 'Topic :: Software Development :: Libraries :: Python Modules', + ], + dependency_links=[ + 'git+https://github.com/onysos/django-composite-foreignkey.git@7ac6b5fa7a54ddc6f527f648d87fec87540ea00e#egg=django-composite-foreignkey-1.0.1' ] ) diff --git a/tox.ini b/tox.ini index a0183848..3b8bac83 100644 --- a/tox.ini +++ b/tox.ini @@ -10,7 +10,9 @@ envlist= docs, [testenv] -deps = django-polymorphic +deps = + django-polymorphic + django-composite-foreignkey: https://github.com/onysos/django-composite-foreignkey.git == 1.0.1 django17: Django >= 1.7,<1.8 django18: Django >= 1.8,<1.9 django19: Django >= 1.9,<1.10