From c339a5a6f72690cd90d5a653dc108fbb60274a20 Mon Sep 17 00:00:00 2001 From: Michal Petrucha Date: Sun, 20 Mar 2016 18:10:55 +0100 Subject: [PATCH] Refs #16508 -- Renamed the current "virtual" fields to "private". The only reason why GenericForeignKey and GenericRelation are stored separately inside _meta is that they need to be cloned for every model subclass, but that's not true for any other virtual field. Actually, it's only true for GenericRelation. --- django/contrib/contenttypes/admin.py | 2 +- django/contrib/contenttypes/fields.py | 6 ++-- django/db/models/base.py | 6 ++-- django/db/models/deletion.py | 4 +-- django/db/models/fields/__init__.py | 20 +++++++++++-- django/db/models/fields/related.py | 8 ++--- django/db/models/options.py | 36 ++++++++++++++++++----- django/forms/models.py | 11 ++++--- docs/internals/deprecation.txt | 6 ++++ docs/releases/1.10.txt | 8 +++++ tests/foreign_object/models/empty_join.py | 4 +-- tests/model_fields/test_field_flags.py | 4 +-- tests/model_meta/results.py | 2 +- tests/model_meta/tests.py | 8 ++--- 14 files changed, 86 insertions(+), 39 deletions(-) diff --git a/django/contrib/contenttypes/admin.py b/django/contrib/contenttypes/admin.py index be0ee2dea5..f15f66b2e1 100644 --- a/django/contrib/contenttypes/admin.py +++ b/django/contrib/contenttypes/admin.py @@ -24,7 +24,7 @@ class GenericInlineModelAdminChecks(InlineModelAdminChecks): # and that they are part of a GenericForeignKey. gfks = [ - f for f in obj.model._meta.virtual_fields + f for f in obj.model._meta.private_fields if isinstance(f, GenericForeignKey) ] if len(gfks) == 0: diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py index 3ff5ad7f0e..592aee6c9f 100644 --- a/django/contrib/contenttypes/fields.py +++ b/django/contrib/contenttypes/fields.py @@ -53,7 +53,7 @@ class GenericForeignKey(object): self.name = name self.model = cls self.cache_attr = "_%s_cache" % name - cls._meta.add_field(self, virtual=True) + cls._meta.add_field(self, private=True) # Only run pre-initialization field assignment on non-abstract models if not cls._meta.abstract: @@ -331,7 +331,7 @@ class GenericRelation(ForeignObject): def _check_generic_foreign_key_existence(self): target = self.remote_field.model if isinstance(target, ModelBase): - fields = target._meta.virtual_fields + fields = target._meta.private_fields if any(isinstance(field, GenericForeignKey) and field.ct_field == self.content_type_field_name and field.fk_field == self.object_id_field_name @@ -409,7 +409,7 @@ class GenericRelation(ForeignObject): return smart_text([instance._get_pk_val() for instance in qs]) def contribute_to_class(self, cls, name, **kwargs): - kwargs['virtual_only'] = True + kwargs['private_only'] = True super(GenericRelation, self).contribute_to_class(cls, name, **kwargs) self.model = cls setattr(cls, self.name, ReverseGenericManyToOneDescriptor(self.remote_field)) diff --git a/django/db/models/base.py b/django/db/models/base.py index c029fd812a..1c454f9b0a 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -161,7 +161,7 @@ class ModelBase(type): new_fields = chain( new_class._meta.local_fields, new_class._meta.local_many_to_many, - new_class._meta.virtual_fields + new_class._meta.private_fields ) field_names = {f.name for f in new_fields} @@ -268,9 +268,9 @@ class ModelBase(type): if is_proxy: new_class.copy_managers(original_base._meta.concrete_managers) - # Inherit virtual fields (like GenericForeignKey) from the parent + # Inherit private fields (like GenericForeignKey) from the parent # class - for field in base._meta.virtual_fields: + for field in base._meta.private_fields: if base._meta.abstract and field.name in field_names: raise FieldError( 'Local field %r in class %r clashes ' diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py index 07ee0459de..91c1bfa245 100644 --- a/django/db/models/deletion.py +++ b/django/db/models/deletion.py @@ -147,7 +147,7 @@ class Collector(object): for related in get_candidate_relations_to_delete(opts): if related.field.remote_field.on_delete is not DO_NOTHING: return False - for field in model._meta.virtual_fields: + for field in model._meta.private_fields: if hasattr(field, 'bulk_related_objects'): # It's something like generic foreign key. return False @@ -221,7 +221,7 @@ class Collector(object): self.fast_deletes.append(sub_objs) elif sub_objs: field.remote_field.on_delete(self, field, sub_objs, self.using) - for field in model._meta.virtual_fields: + for field in model._meta.private_fields: if hasattr(field, 'bulk_related_objects'): # It's something like generic foreign key. sub_objs = field.bulk_related_objects(new_objs, self.using) diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index 15f4789bbc..a4018dd6e9 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -672,11 +672,25 @@ class Field(RegisterLookupMixin): if self.verbose_name is None and self.name: self.verbose_name = self.name.replace('_', ' ') - def contribute_to_class(self, cls, name, virtual_only=False): + def contribute_to_class(self, cls, name, private_only=False, virtual_only=NOT_PROVIDED): + """ + Register the field with the model class it belongs to. + + If private_only is True, a separate instance of this field will be + created for every subclass of cls, even if cls is not an abstract + model. + """ + if virtual_only is not NOT_PROVIDED: + warnings.warn( + "The `virtual_only` argument of Field.contribute_to_class() " + "has been renamed to `private_only`.", + RemovedInDjango20Warning, stacklevel=2 + ) + private_only = virtual_only self.set_attributes_from_name(name) self.model = cls - if virtual_only: - cls._meta.add_field(self, virtual=True) + if private_only: + cls._meta.add_field(self, private=True) else: cls._meta.add_field(self) if self.choices: diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 0539647bcc..1bd1248c71 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -282,9 +282,9 @@ class RelatedField(Field): # columns from another table. return None - def contribute_to_class(self, cls, name, virtual_only=False): + def contribute_to_class(self, cls, name, private_only=False, **kwargs): - super(RelatedField, self).contribute_to_class(cls, name, virtual_only=virtual_only) + super(RelatedField, self).contribute_to_class(cls, name, private_only=private_only, **kwargs) self.opts = cls._meta @@ -703,8 +703,8 @@ class ForeignObject(RelatedField): def get_defaults(self): return tuple(field.get_default() for field in self.local_related_fields) - def contribute_to_class(self, cls, name, virtual_only=False): - super(ForeignObject, self).contribute_to_class(cls, name, virtual_only=virtual_only) + def contribute_to_class(self, cls, name, private_only=False, **kwargs): + super(ForeignObject, self).contribute_to_class(cls, name, private_only=private_only, **kwargs) setattr(cls, self.name, ForwardManyToOneDescriptor(self)) def contribute_to_related_class(self, cls, related): diff --git a/django/db/models/options.py b/django/db/models/options.py index 0432d6958a..c6ddd2a6f9 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +import warnings from bisect import bisect from collections import OrderedDict, defaultdict from itertools import chain @@ -12,6 +13,9 @@ from django.db.models.fields import AutoField from django.db.models.fields.proxy import OrderWrt from django.utils import six from django.utils.datastructures import ImmutableList, OrderedSet +from django.utils.deprecation import ( + RemovedInDjango20Warning, warn_about_renamed_method, +) from django.utils.encoding import ( force_text, python_2_unicode_compatible, smart_text, ) @@ -19,6 +23,8 @@ from django.utils.functional import cached_property from django.utils.text import camel_case_to_spaces from django.utils.translation import override, string_concat +NOT_PROVIDED = object() + PROXY_PARENTS = object() EMPTY_RELATION_TREE = tuple() @@ -76,7 +82,7 @@ class Options(object): self._get_fields_cache = {} self.local_fields = [] self.local_many_to_many = [] - self.virtual_fields = [] + self.private_fields = [] self.model_name = None self.verbose_name = None self.verbose_name_plural = None @@ -253,13 +259,19 @@ class Options(object): auto = AutoField(verbose_name='ID', primary_key=True, auto_created=True) model.add_to_class('id', auto) - def add_field(self, field, virtual=False): + def add_field(self, field, private=False, virtual=NOT_PROVIDED): + if virtual is not NOT_PROVIDED: + warnings.warn( + "The `virtual` argument of Options.add_field() has been renamed to `private`.", + RemovedInDjango20Warning, stacklevel=2 + ) + private = virtual # Insert the given field in the order in which it was created, using # the "creation_counter" attribute of the field. # Move many-to-many related fields from self.fields into # self.many_to_many. - if virtual: - self.virtual_fields.append(field) + if private: + self.private_fields.append(field) elif field.is_relation and field.many_to_many: self.local_many_to_many.insert(bisect(self.local_many_to_many, field), field) else: @@ -365,7 +377,7 @@ class Options(object): obtaining this field list. """ # For legacy reasons, the fields property should only contain forward - # fields that are not virtual or with a m2m cardinality. Therefore we + # fields that are not private or with a m2m cardinality. Therefore we # pass these three filters as filters to the generator. # The third lambda is a longwinded way of checking f.related_model - we don't # use that property directly because related_model is a cached property, @@ -401,6 +413,14 @@ class Options(object): "concrete_fields", (f for f in self.fields if f.concrete) ) + @property + @warn_about_renamed_method( + 'Options', 'virtual_fields', 'private_fields', + RemovedInDjango20Warning + ) + def virtual_fields(self): + return self.private_fields + @cached_property def local_concrete_fields(self): """ @@ -686,14 +706,14 @@ class Options(object): fields.extend( field for field in chain(self.local_fields, self.local_many_to_many) ) - # Virtual fields are recopied to each child model, and they get a + # Private fields are recopied to each child model, and they get a # different model as field.model in each child. Hence we have to - # add the virtual fields separately from the topmost call. If we + # add the private fields separately from the topmost call. If we # did this recursively similar to local_fields, we would get field # instances with field.model != self.model. if topmost_call: fields.extend( - f for f in self.virtual_fields + f for f in self.private_fields ) # In order to avoid list manipulation. Always diff --git a/django/forms/models.py b/django/forms/models.py index 849f7a111a..aac75b54ff 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -81,7 +81,7 @@ def model_to_dict(instance, fields=None, exclude=None): """ opts = instance._meta data = {} - for f in chain(opts.concrete_fields, opts.virtual_fields, opts.many_to_many): + for f in chain(opts.concrete_fields, opts.private_fields, opts.many_to_many): if not getattr(f, 'editable', False): continue if fields and f.name not in fields: @@ -142,9 +142,8 @@ def fields_for_model(model, fields=None, exclude=None, widgets=None, opts = model._meta # Avoid circular import from django.db.models.fields import Field as ModelField - sortable_virtual_fields = [f for f in opts.virtual_fields - if isinstance(f, ModelField)] - for f in sorted(chain(opts.concrete_fields, sortable_virtual_fields, opts.many_to_many)): + sortable_private_fields = [f for f in opts.private_fields if isinstance(f, ModelField)] + for f in sorted(chain(opts.concrete_fields, sortable_private_fields, opts.many_to_many)): if not getattr(f, 'editable', False): if (fields is not None and f.name in fields and (exclude is None or f.name not in exclude)): @@ -431,9 +430,9 @@ class BaseModelForm(BaseForm): fields = self._meta.fields opts = self.instance._meta # Note that for historical reasons we want to include also - # virtual_fields here. (GenericRelation was previously a fake + # private_fields here. (GenericRelation was previously a fake # m2m field). - for f in chain(opts.many_to_many, opts.virtual_fields): + for f in chain(opts.many_to_many, opts.private_fields): if not hasattr(f, 'save_form_data'): continue if fields and f.name not in fields: diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index d428f414f7..fee9c8cef7 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -150,6 +150,12 @@ details on these changes. * Using ``User.is_authenticated()`` and ``User.is_anonymous()`` as methods will no longer be supported. +* The private attribute ``virtual_fields`` of ``Model._meta`` will be removed. + +* The private keyword arguments ``virtual_only`` in + ``Field.contribute_to_class()`` and ``virtual`` in + ``Model._meta.add_field()`` will be removed. + .. _deprecation-removed-in-1.10: 1.10 diff --git a/docs/releases/1.10.txt b/docs/releases/1.10.txt index 463cd4706e..c06602dc95 100644 --- a/docs/releases/1.10.txt +++ b/docs/releases/1.10.txt @@ -921,6 +921,14 @@ Miscellaneous * The template ``Context.has_key()`` method is deprecated in favor of ``in``. +* The private attribute ``virtual_fields`` of ``Model._meta`` is + deprecated in favor of ``private_fields``. + +* The private keyword arguments ``virtual_only`` in + ``Field.contribute_to_class()`` and ``virtual`` in + ``Model._meta.add_field()`` are deprecated in favor of ``private_only`` + and ``private``, respectively. + .. _removed-features-1.10: Features removed in 1.10 diff --git a/tests/foreign_object/models/empty_join.py b/tests/foreign_object/models/empty_join.py index b8b05ea1c2..61ac5e35a4 100644 --- a/tests/foreign_object/models/empty_join.py +++ b/tests/foreign_object/models/empty_join.py @@ -64,8 +64,8 @@ class StartsWithRelation(models.ForeignObject): from_opts = self.remote_field.model._meta return [PathInfo(from_opts, to_opts, (to_opts.pk,), self.remote_field, False, False)] - def contribute_to_class(self, cls, name, virtual_only=False): - super(StartsWithRelation, self).contribute_to_class(cls, name, virtual_only) + def contribute_to_class(self, cls, name, private_only=False): + super(StartsWithRelation, self).contribute_to_class(cls, name, private_only) setattr(cls, self.name, ReverseManyToOneDescriptor(self)) diff --git a/tests/model_fields/test_field_flags.py b/tests/model_fields/test_field_flags.py index 33f49a139d..ff6f595e27 100644 --- a/tests/model_fields/test_field_flags.py +++ b/tests/model_fields/test_field_flags.py @@ -78,13 +78,13 @@ class FieldFlagsTests(test.SimpleTestCase): super(FieldFlagsTests, cls).setUpClass() cls.fields = ( list(AllFieldsModel._meta.fields) + - list(AllFieldsModel._meta.virtual_fields) + list(AllFieldsModel._meta.private_fields) ) cls.all_fields = ( cls.fields + list(AllFieldsModel._meta.many_to_many) + - list(AllFieldsModel._meta.virtual_fields) + list(AllFieldsModel._meta.private_fields) ) cls.fields_and_reverse_objects = ( diff --git a/tests/model_meta/results.py b/tests/model_meta/results.py index 50cb59a169..db8ccf650e 100644 --- a/tests/model_meta/results.py +++ b/tests/model_meta/results.py @@ -863,7 +863,7 @@ TEST_RESULTS = { 'm2m_concrete_rel', ], }, - 'virtual_fields': { + 'private_fields': { AbstractPerson: [ 'generic_relation_abstract', 'content_object_abstract', diff --git a/tests/model_meta/tests.py b/tests/model_meta/tests.py index d662c8257c..9a692ffdd2 100644 --- a/tests/model_meta/tests.py +++ b/tests/model_meta/tests.py @@ -157,11 +157,11 @@ class RelatedObjectsTests(OptionsBaseTests): ) -class VirtualFieldsTests(OptionsBaseTests): +class PrivateFieldsTests(OptionsBaseTests): - def test_virtual_fields(self): - for model, expected_names in TEST_RESULTS['virtual_fields'].items(): - objects = model._meta.virtual_fields + def test_private_fields(self): + for model, expected_names in TEST_RESULTS['private_fields'].items(): + objects = model._meta.private_fields self.assertEqual(sorted([f.name for f in objects]), sorted(expected_names))