From 84b6c768301096849f5589d7612b129c5445abd3 Mon Sep 17 00:00:00 2001 From: Loic Bistuer Date: Tue, 27 Jan 2015 21:40:01 +0700 Subject: [PATCH] Fixed #24210 -- Cleaned up relational fields __init__(). Thanks Collin Anderson and Tim Graham for the reviews. --- django/contrib/contenttypes/fields.py | 47 ++++--- django/db/models/fields/__init__.py | 5 +- django/db/models/fields/related.py | 183 ++++++++++++++++---------- 3 files changed, 138 insertions(+), 97 deletions(-) diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py index fba8e0c222..2bb8ce9e95 100644 --- a/django/contrib/contenttypes/fields.py +++ b/django/contrib/contenttypes/fields.py @@ -33,7 +33,7 @@ class GenericForeignKey(object): one_to_one = False related_model = None - def __init__(self, ct_field="content_type", fk_field="object_id", for_concrete_model=True): + def __init__(self, ct_field='content_type', fk_field='object_id', for_concrete_model=True): self.ct_field = ct_field self.fk_field = fk_field self.for_concrete_model = for_concrete_model @@ -253,6 +253,17 @@ class GenericForeignKey(object): setattr(instance, self.cache_attr, value) +class GenericRel(ForeignObjectRel): + def __init__(self, field, to, related_name=None, related_query_name=None, limit_choices_to=None): + super(GenericRel, self).__init__( + field, to, + related_name=related_query_name or '+', + related_query_name=related_query_name, + limit_choices_to=limit_choices_to, + on_delete=DO_NOTHING, + ) + + class GenericRelation(ForeignObject): """Provides an accessor to generic related objects (e.g. comments)""" # Field flags @@ -263,30 +274,31 @@ class GenericRelation(ForeignObject): one_to_many = False one_to_one = False - def __init__(self, to, **kwargs): - kwargs['verbose_name'] = kwargs.get('verbose_name', None) - kwargs['rel'] = GenericRel( - self, to, - related_query_name=kwargs.pop('related_query_name', None), - limit_choices_to=kwargs.pop('limit_choices_to', None), - ) - # Override content-type/object-id field names on the related class - self.object_id_field_name = kwargs.pop("object_id_field", "object_id") - self.content_type_field_name = kwargs.pop("content_type_field", "content_type") + rel_class = GenericRel - self.for_concrete_model = kwargs.pop("for_concrete_model", True) + def __init__(self, to, object_id_field='object_id', content_type_field='content_type', + for_concrete_model=True, related_query_name=None, limit_choices_to=None, **kwargs): + kwargs['rel'] = self.rel_class( + self, to, + related_query_name=related_query_name, + limit_choices_to=limit_choices_to, + ) kwargs['blank'] = True kwargs['editable'] = False kwargs['serialize'] = False + # This construct is somewhat of an abuse of ForeignObject. This field # represents a relation from pk to object_id field. But, this relation # isn't direct, the join is generated reverse along foreign key. So, # the from_field is object_id field, to_field is pk because of the # reverse join. super(GenericRelation, self).__init__( - to, to_fields=[], - from_fields=[self.object_id_field_name], **kwargs) + to, from_fields=[object_id_field], to_fields=[], **kwargs) + + self.object_id_field_name = object_id_field + self.content_type_field_name = content_type_field + self.for_concrete_model = for_concrete_model def check(self, **kwargs): errors = super(GenericRelation, self).check(**kwargs) @@ -571,10 +583,3 @@ def create_generic_related_manager(superclass): update_or_create.alters_data = True return GenericRelatedObjectManager - - -class GenericRel(ForeignObjectRel): - def __init__(self, field, to, related_name=None, limit_choices_to=None, related_query_name=None): - super(GenericRel, self).__init__(field=field, to=to, related_name=related_query_name or '+', - limit_choices_to=limit_choices_to, on_delete=DO_NOTHING, - related_query_name=related_query_name) diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index d2493dbee0..d7e01e2960 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -157,14 +157,11 @@ class Field(RegisterLookupMixin): self.unique_for_year = unique_for_year self._choices = choices or [] self.help_text = help_text + self.db_index = db_index self.db_column = db_column self.db_tablespace = db_tablespace or settings.DEFAULT_INDEX_TABLESPACE self.auto_created = auto_created - # Set db_index to True if the field has a relationship and doesn't - # explicitly set db_index. - self.db_index = db_index - # Adjust the appropriate creation counter, and save our local copy. if auto_created: self.creation_counter = Field.auto_creation_counter diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index e6ecd98537..d917b01e4d 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -1269,17 +1269,18 @@ class ForeignObjectRel(object): editable = False is_relation = True - def __init__(self, field, to, related_name=None, limit_choices_to=None, - parent_link=False, on_delete=None, related_query_name=None): + def __init__(self, field, to, related_name=None, related_query_name=None, + limit_choices_to=None, parent_link=False, on_delete=None): self.field = field self.to = to self.related_name = related_name self.related_query_name = related_query_name self.limit_choices_to = {} if limit_choices_to is None else limit_choices_to - self.multiple = True self.parent_link = parent_link self.on_delete = on_delete + self.symmetrical = False + self.multiple = True # Some of the following cached_properties can't be initialized in # __init__ as the field doesn't have its model yet. Calling these methods @@ -1408,11 +1409,17 @@ class ForeignObjectRel(object): class ManyToOneRel(ForeignObjectRel): - def __init__(self, field, to, field_name, related_name=None, limit_choices_to=None, - parent_link=False, on_delete=None, related_query_name=None): + def __init__(self, field, to, field_name, related_name=None, related_query_name=None, + limit_choices_to=None, parent_link=False, on_delete=None): super(ManyToOneRel, self).__init__( - field, to, related_name=related_name, limit_choices_to=limit_choices_to, - parent_link=parent_link, on_delete=on_delete, related_query_name=related_query_name) + field, to, + related_name=related_name, + related_query_name=related_query_name, + limit_choices_to=limit_choices_to, + parent_link=parent_link, + on_delete=on_delete, + ) + self.field_name = field_name def get_related_field(self): @@ -1431,29 +1438,40 @@ class ManyToOneRel(ForeignObjectRel): class OneToOneRel(ManyToOneRel): - def __init__(self, field, to, field_name, related_name=None, limit_choices_to=None, - parent_link=False, on_delete=None, related_query_name=None): - super(OneToOneRel, self).__init__(field, to, field_name, - related_name=related_name, limit_choices_to=limit_choices_to, - parent_link=parent_link, on_delete=on_delete, related_query_name=related_query_name) + def __init__(self, field, to, field_name, related_name=None, related_query_name=None, + limit_choices_to=None, parent_link=False, on_delete=None): + super(OneToOneRel, self).__init__( + field, to, field_name, + related_name=related_name, + related_query_name=related_query_name, + limit_choices_to=limit_choices_to, + parent_link=parent_link, + on_delete=on_delete, + ) + self.multiple = False class ManyToManyRel(ForeignObjectRel): - def __init__(self, field, to, related_name=None, limit_choices_to=None, - symmetrical=True, through=None, through_fields=None, - db_constraint=True, related_query_name=None): + def __init__(self, field, to, related_name=None, related_query_name=None, + limit_choices_to=None, symmetrical=True, through=None, through_fields=None, + db_constraint=True): + super(ManyToManyRel, self).__init__( + field, to, + related_name=related_name, + related_query_name=related_query_name, + limit_choices_to=limit_choices_to, + ) + if through and not db_constraint: raise ValueError("Can't supply a through model and db_constraint=False") + self.through = through + if through_fields and not through: raise ValueError("Cannot specify through_fields without a through model") - super(ManyToManyRel, self).__init__( - field, to, related_name=related_name, - limit_choices_to=limit_choices_to, related_query_name=related_query_name) - self.symmetrical = symmetrical - self.multiple = True - self.through = through self.through_fields = through_fields + + self.symmetrical = symmetrical self.db_constraint = db_constraint def is_hidden(self): @@ -1485,25 +1503,27 @@ class ForeignObject(RelatedField): requires_unique_target = True related_accessor_class = ForeignRelatedObjectsDescriptor + rel_class = ForeignObjectRel + + def __init__(self, to, from_fields, to_fields, rel=None, related_name=None, + related_query_name=None, limit_choices_to=None, parent_link=False, + on_delete=CASCADE, swappable=True, **kwargs): + if rel is None: + rel = self.rel_class( + self, to, + related_name=related_name, + related_query_name=related_query_name, + limit_choices_to=limit_choices_to, + parent_link=parent_link, + on_delete=on_delete, + ) + + super(ForeignObject, self).__init__(rel=rel, **kwargs) - def __init__(self, to, from_fields, to_fields, swappable=True, **kwargs): self.from_fields = from_fields self.to_fields = to_fields self.swappable = swappable - if 'rel' not in kwargs: - kwargs['rel'] = ForeignObjectRel( - self, to, - related_name=kwargs.pop('related_name', None), - related_query_name=kwargs.pop('related_query_name', None), - limit_choices_to=kwargs.pop('limit_choices_to', None), - parent_link=kwargs.pop('parent_link', False), - on_delete=kwargs.pop('on_delete', CASCADE), - ) - kwargs['verbose_name'] = kwargs.get('verbose_name', None) - - super(ForeignObject, self).__init__(**kwargs) - def check(self, **kwargs): errors = super(ForeignObject, self).check(**kwargs) errors.extend(self._check_unique_target()) @@ -1793,17 +1813,20 @@ class ForeignKey(ForeignObject): one_to_many = True one_to_one = False + rel_class = ManyToOneRel + empty_strings_allowed = False default_error_messages = { 'invalid': _('%(model)s instance with %(field)s %(value)r does not exist.') } description = _("Foreign Key (type determined by related field)") - def __init__(self, to, to_field=None, rel_class=ManyToOneRel, - db_constraint=True, **kwargs): + def __init__(self, to, to_field=None, related_name=None, related_query_name=None, + limit_choices_to=None, parent_link=False, on_delete=CASCADE, + db_constraint=True, **kwargs): try: to._meta.model_name - except AttributeError: # to._meta doesn't exist, so it must be RECURSIVE_RELATIONSHIP_CONSTANT + except AttributeError: assert isinstance(to, six.string_types), ( "%s(%r) is invalid. First parameter to ForeignKey must be " "either a model, a model name, or the string %r" % ( @@ -1817,21 +1840,22 @@ class ForeignKey(ForeignObject): # be correct until contribute_to_class is called. Refs #12190. to_field = to_field or (to._meta.pk and to._meta.pk.name) - if 'db_index' not in kwargs: - kwargs['db_index'] = True + kwargs['rel'] = self.rel_class( + self, to, to_field, + related_name=related_name, + related_query_name=related_query_name, + limit_choices_to=limit_choices_to, + parent_link=parent_link, + on_delete=on_delete, + ) + + kwargs['db_index'] = kwargs.get('db_index', True) + + super(ForeignKey, self).__init__( + to, from_fields=['self'], to_fields=[to_field], **kwargs) self.db_constraint = db_constraint - kwargs['rel'] = rel_class( - self, to, to_field, - related_name=kwargs.pop('related_name', None), - related_query_name=kwargs.pop('related_query_name', None), - limit_choices_to=kwargs.pop('limit_choices_to', None), - parent_link=kwargs.pop('parent_link', False), - on_delete=kwargs.pop('on_delete', CASCADE), - ) - super(ForeignKey, self).__init__(to, ['self'], [to_field], **kwargs) - def check(self, **kwargs): errors = super(ForeignKey, self).check(**kwargs) errors.extend(self._check_on_delete()) @@ -2010,11 +2034,13 @@ class OneToOneField(ForeignKey): one_to_one = True related_accessor_class = SingleRelatedObjectDescriptor + rel_class = OneToOneRel + description = _("One-to-one relationship") def __init__(self, to, to_field=None, **kwargs): kwargs['unique'] = True - super(OneToOneField, self).__init__(to, to_field, OneToOneRel, **kwargs) + super(OneToOneField, self).__init__(to, to_field, **kwargs) def deconstruct(self): name, path, args, kwargs = super(OneToOneField, self).deconstruct() @@ -2100,12 +2126,17 @@ class ManyToManyField(RelatedField): one_to_many = False one_to_one = False + rel_class = ManyToManyRel + description = _("Many-to-many relationship") - def __init__(self, to, db_constraint=True, swappable=True, **kwargs): + def __init__(self, to, related_name=None, related_query_name=None, + limit_choices_to=None, symmetrical=None, through=None, + through_fields=None, db_constraint=True, db_table=None, + swappable=True, **kwargs): try: to._meta - except AttributeError: # to._meta doesn't exist, so it must be RECURSIVE_RELATIONSHIP_CONSTANT + except AttributeError: assert isinstance(to, six.string_types), ( "%s(%r) is invalid. First parameter to ManyToManyField must be " "either a model, a model name, or the string %r" % @@ -2114,25 +2145,31 @@ class ManyToManyField(RelatedField): # Class names must be ASCII in Python 2.x, so we forcibly coerce it # here to break early if there's a problem. to = str(to) - kwargs['verbose_name'] = kwargs.get('verbose_name', None) - kwargs['rel'] = ManyToManyRel( + + if symmetrical is None: + symmetrical = (to == RECURSIVE_RELATIONSHIP_CONSTANT) + + if through is not None: + assert db_table is None, ( + "Cannot specify a db_table if an intermediary model is used." + ) + + kwargs['rel'] = self.rel_class( self, to, - related_name=kwargs.pop('related_name', None), - related_query_name=kwargs.pop('related_query_name', None), - limit_choices_to=kwargs.pop('limit_choices_to', None), - symmetrical=kwargs.pop('symmetrical', to == RECURSIVE_RELATIONSHIP_CONSTANT), - through=kwargs.pop('through', None), - through_fields=kwargs.pop('through_fields', None), + related_name=related_name, + related_query_name=related_query_name, + limit_choices_to=limit_choices_to, + symmetrical=symmetrical, + through=through, + through_fields=through_fields, db_constraint=db_constraint, ) - self.swappable = swappable - self.db_table = kwargs.pop('db_table', None) - if kwargs['rel'].through is not None: - assert self.db_table is None, "Cannot specify a db_table if an intermediary model is used." - super(ManyToManyField, self).__init__(**kwargs) + self.db_table = db_table + self.swappable = swappable + def check(self, **kwargs): errors = super(ManyToManyField, self).check(**kwargs) errors.extend(self._check_unique(**kwargs)) @@ -2201,10 +2238,11 @@ class ManyToManyField(RelatedField): else: - assert from_model is not None, \ - "ManyToManyField with intermediate " \ - "tables cannot be checked if you don't pass the model " \ + assert from_model is not None, ( + "ManyToManyField with intermediate " + "tables cannot be checked if you don't pass the model " "where the field is attached to." + ) # Set some useful local variables to_model = self.rel.to @@ -2323,10 +2361,11 @@ class ManyToManyField(RelatedField): # fields on the through model, and also be foreign keys to the # expected models else: - assert from_model is not None, \ - "ManyToManyField with intermediate " \ - "tables cannot be checked if you don't pass the model " \ + assert from_model is not None, ( + "ManyToManyField with intermediate " + "tables cannot be checked if you don't pass the model " "where the field is attached to." + ) source, through, target = from_model, self.rel.through, self.rel.to source_field_name, target_field_name = self.rel.through_fields[:2]