From 48320913ac79ae25b8dd32eb534bfb8c8b1e1ac9 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Fri, 17 Feb 2006 12:46:03 +0000 Subject: [PATCH] magic-removal: Removed last references to OLD_get_accessor_name, and added unit tests to ensure accessor names are used correctly during validation. git-svn-id: http://code.djangoproject.com/svn/django/branches/magic-removal@2313 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/core/management.py | 77 ++++++++------- django/db/models/query.py | 2 +- django/db/models/related.py | 6 -- tests/modeltests/invalid_models/models.py | 109 ++++++++++++++++++++-- tests/runtests.py | 16 +++- 5 files changed, 154 insertions(+), 56 deletions(-) diff --git a/django/core/management.py b/django/core/management.py index f303742bfd..d7349f265a 100644 --- a/django/core/management.py +++ b/django/core/management.py @@ -837,67 +837,74 @@ def get_validation_errors(outfile, app=None): # Check for deprecated args dep_args = getattr(f, 'deprecated_args', None) if dep_args: - e.add(opts, "'%s' field: Initialised with deprecated args:%s" % (f.name, ",".join(dep_args))) + e.add(opts, "'%s' Initialised with deprecated args:%s" % (f.name, ",".join(dep_args))) if isinstance(f, models.CharField) and f.maxlength in (None, 0): - e.add(opts, '"%s" field: CharFields require a "maxlength" attribute.' % f.name) + e.add(opts, '"%s": CharFields require a "maxlength" attribute.' % f.name) if isinstance(f, models.FloatField): if f.decimal_places is None: - e.add(opts, '"%s" field: FloatFields require a "decimal_places" attribute.' % f.name) + e.add(opts, '"%s": FloatFields require a "decimal_places" attribute.' % f.name) if f.max_digits is None: - e.add(opts, '"%s" field: FloatFields require a "max_digits" attribute.' % f.name) + e.add(opts, '"%s": FloatFields require a "max_digits" attribute.' % f.name) if isinstance(f, models.FileField) and not f.upload_to: - e.add(opts, '"%s" field: FileFields require an "upload_to" attribute.' % f.name) + e.add(opts, '"%s": FileFields require an "upload_to" attribute.' % f.name) if isinstance(f, models.ImageField): try: from PIL import Image except ImportError: - e.add(opts, '"%s" field: To use ImageFields, you need to install the Python Imaging Library. Get it at http://www.pythonware.com/products/pil/ .' % f.name) + e.add(opts, '"%s": To use ImageFields, you need to install the Python Imaging Library. Get it at http://www.pythonware.com/products/pil/ .' % f.name) if f.prepopulate_from is not None and type(f.prepopulate_from) not in (list, tuple): - e.add(opts, '"%s" field: prepopulate_from should be a list or tuple.' % f.name) + e.add(opts, '"%s": prepopulate_from should be a list or tuple.' % f.name) if f.choices: if not type(f.choices) in (tuple, list): - e.add(opts, '"%s" field: "choices" should be either a tuple or list.' % f.name) + e.add(opts, '"%s": "choices" should be either a tuple or list.' % f.name) else: for c in f.choices: if not type(c) in (tuple, list) or len(c) != 2: - e.add(opts, '"%s" field: "choices" should be a sequence of two-tuples.' % f.name) + e.add(opts, '"%s": "choices" should be a sequence of two-tuples.' % f.name) if f.db_index not in (None, True, False): - e.add(opts, '"%s" field: "db_index" should be either None, True or False.' % f.name) + e.add(opts, '"%s": "db_index" should be either None, True or False.' % f.name) # Check to see if the related field will clash with any # existing fields, m2m fields, m2m related objects or related objects if f.rel: rel_opts = f.rel.to._meta if f.rel.to not in models.get_models(): - e.add(opts, "'%s' field: relates to uninstalled model %s" % (f.name, rel_opts.object_name)) - - rel_name = RelatedObject(f.rel.to, cls, f).OLD_get_accessor_name() - if rel_name in [r.name for r in rel_opts.fields]: - e.add(opts, "'%s.%s' related field: Clashes with field on '%s.%s'" % (opts.object_name, f.name, rel_opts.object_name, rel_name)) - elif rel_name in [r.name for r in rel_opts.many_to_many]: - e.add(opts, "'%s.%s' related field: Clashes with m2m field on '%s.%s'" % (opts.object_name, f.name, rel_opts.object_name, rel_name)) - elif rel_name in [r.OLD_get_accessor_name() for r in rel_opts.get_all_related_many_to_many_objects()]: - e.add(opts, "'%s.%s' related field: Clashes with related m2m field '%s.%s'" % (opts.object_name, f.name, rel_opts.object_name, rel_name)) - elif rel_name in [r.OLD_get_accessor_name() for r in rel_opts.get_all_related_objects() if r.field is not f]: - e.add(opts, "'%s.%s' related field: Clashes with related field on '%s.%s'" % (opts.object_name, f.name, rel_opts.object_name, rel_name)) - + e.add(opts, "'%s' relates to uninstalled model %s" % (f.name, rel_opts.object_name)) + + rel_name = RelatedObject(f.rel.to, cls, f).get_accessor_name() + for r in rel_opts.fields: + if r.name == rel_name: + e.add(opts, "'%s' accessor name '%s.%s' clashes with another field" % (f.name, rel_opts.object_name, r.name)) + for r in rel_opts.many_to_many: + if r.name == rel_name: + e.add(opts, "'%s' accessor name '%s.%s' clashes with a m2m field" % (f.name, rel_opts.object_name, r.name)) + for r in rel_opts.get_all_related_many_to_many_objects(): + if r.get_accessor_name() == rel_name: + e.add(opts, "'%s' accessor name '%s.%s' clashes with a related m2m field" % (f.name, rel_opts.object_name, r.get_accessor_name())) + for r in rel_opts.get_all_related_objects(): + if r.get_accessor_name() == rel_name and r.field is not f: + e.add(opts, "'%s' accessor name '%s.%s' clashes with a related field" % (f.name, rel_opts.object_name, r.get_accessor_name())) + for i, f in enumerate(opts.many_to_many): # Check to see if the related m2m field will clash with any # existing fields, m2m fields, m2m related objects or related objects - if f.rel: - rel_opts = f.rel.to._meta - if f.rel.to not in models.get_models(): - e.add(opts, "'%s' field: has m2m relation with uninstalled model %s" % (f.name, rel_opts.object_name)) + rel_opts = f.rel.to._meta + if f.rel.to not in models.get_models(): + e.add(opts, "'%s' has m2m relation with uninstalled model %s" % (f.name, rel_opts.object_name)) - rel_name = RelatedObject(f.rel.to, cls, f).OLD_get_accessor_name() - if rel_name in [r.name for r in rel_opts.fields]: - e.add(opts, "'%s.%s' related m2m field: Clashes with field on '%s.%s'" % (opts.object_name, f.name, rel_opts.object_name, rel_name)) - elif rel_name in [r.name for r in rel_opts.many_to_many]: - e.add(opts, "'%s.%s' related m2m field: Clashes with m2m field on '%s.%s'" % (opts.object_name, f.name, rel_opts.object_name, rel_name)) - elif rel_name in [r.OLD_get_accessor_name() for r in rel_opts.get_all_related_many_to_many_objects() if r.field is not f]: - e.add(opts, "'%s.%s' related m2m field: Clashes with related m2m field '%s.%s'" % (opts.object_name, f.name, rel_opts.object_name, rel_name)) - elif rel_name in [r.OLD_get_accessor_name() for r in rel_opts.get_all_related_objects()]: - e.add(opts, "'%s.%s' related m2m field: Clashes with related field on '%s.%s'" % (opts.object_name, f.name, rel_opts.object_name, rel_name)) + rel_name = RelatedObject(f.rel.to, cls, f).get_accessor_name() + for r in rel_opts.fields: + if r.name == rel_name: + e.add(opts, "'%s' m2m accessor name '%s.%s' clashes with another field" % (f.name, rel_opts.object_name, r.name)) + for r in rel_opts.many_to_many: + if r.name == rel_name: + e.add(opts, "'%s' m2m accessor name '%s.%s' clashes with a m2m field" % (f.name, rel_opts.object_name, r.name)) + for r in rel_opts.get_all_related_many_to_many_objects(): + if r.get_accessor_name() == rel_name and r.field is not f: + e.add(opts, "'%s' m2m accessor name '%s.%s' clashes with a related m2m field" % (f.name, rel_opts.object_name, r.get_accessor_name())) + for r in rel_opts.get_all_related_objects(): + if r.get_accessor_name() == rel_name: + e.add(opts, "'%s' m2m accessor name '%s.%s' clashes with a related field" % (f.name, rel_opts.object_name, r.get_accessor_name())) # Check admin attribute. if opts.admin is not None: diff --git a/django/db/models/query.py b/django/db/models/query.py index 13d20a1f10..c8d75bba72 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -614,7 +614,7 @@ def find_field(name, field_list, use_accessor=False): Returns None if there are no matches, or several matches. """ if use_accessor: - matches = [f for f in field_list if f.OLD_get_accessor_name() == name] + matches = [f for f in field_list if (f.field.rel.related_name or f.opts.object_name.lower()) == name] else: matches = [f for f in field_list if f.name == name] if len(matches) != 1: diff --git a/django/db/models/related.py b/django/db/models/related.py index f92f5d1160..895a2a8c6d 100644 --- a/django/db/models/related.py +++ b/django/db/models/related.py @@ -75,9 +75,3 @@ class RelatedObject(object): # but this can be overridden with the "related_name" option. return self.field.rel.related_name or (self.opts.object_name.lower() + '_set') - # TODO: Remove this. - def OLD_get_accessor_name(self): - rel_obj_name = self.field.rel.related_name or self.opts.object_name.lower() - if self.parent_model._meta.app_label != self.opts.app_label: - rel_obj_name = '%s_%s' % (self.opts.app_label, rel_obj_name) - return rel_obj_name diff --git a/tests/modeltests/invalid_models/models.py b/tests/modeltests/invalid_models/models.py index 02f7288dbf..ed50388a0b 100644 --- a/tests/modeltests/invalid_models/models.py +++ b/tests/modeltests/invalid_models/models.py @@ -13,14 +13,105 @@ class FieldErrors(models.Model): choices2 = models.CharField(maxlength=10, choices=[(1,2,3),(1,2,3)]) index = models.CharField(maxlength=10, db_index='bad') +class Target(models.Model): + tgt_safe = models.CharField(maxlength=10) + + clash1_set = models.CharField(maxlength=10) + +class Clash1(models.Model): + src_safe = models.CharField(maxlength=10) + + foreign = models.ForeignKey(Target) + m2m = models.ManyToManyField(Target) -error_log = """invalid_models.fielderrors: "charfield" field: CharFields require a "maxlength" attribute. -invalid_models.fielderrors: "floatfield" field: FloatFields require a "decimal_places" attribute. -invalid_models.fielderrors: "floatfield" field: FloatFields require a "max_digits" attribute. -invalid_models.fielderrors: "filefield" field: FileFields require an "upload_to" attribute. -invalid_models.fielderrors: "prepopulate" field: prepopulate_from should be a list or tuple. -invalid_models.fielderrors: "choices" field: "choices" should be either a tuple or list. -invalid_models.fielderrors: "choices2" field: "choices" should be a sequence of two-tuples. -invalid_models.fielderrors: "choices2" field: "choices" should be a sequence of two-tuples. -invalid_models.fielderrors: "index" field: "db_index" should be either None, True or False. +class Clash2(models.Model): + src_safe = models.CharField(maxlength=10) + + foreign_1 = models.ForeignKey(Target, related_name='id') + foreign_2 = models.ForeignKey(Target, related_name='src_safe') + + m2m_1 = models.ManyToManyField(Target, related_name='id') + m2m_2 = models.ManyToManyField(Target, related_name='src_safe') + +class Target2(models.Model): + foreign_tgt = models.ForeignKey(Target) + clashforeign_set = models.ForeignKey(Target) + + m2m_tgt = models.ManyToManyField(Target) + clashm2m_set = models.ManyToManyField(Target) + +class Clash3(models.Model): + foreign_1 = models.ForeignKey(Target2, related_name='foreign_tgt') + foreign_2 = models.ForeignKey(Target2, related_name='m2m_tgt') + + m2m_1 = models.ManyToManyField(Target2, related_name='foreign_tgt') + m2m_2 = models.ManyToManyField(Target2, related_name='m2m_tgt') + +class ClashForeign(models.Model): + foreign = models.ForeignKey(Target2) + +class ClashM2M(models.Model): + m2m = models.ManyToManyField(Target2) + +class SelfClashForeign(models.Model): + src_safe = models.CharField(maxlength=10) + + selfclashforeign_set = models.ForeignKey("SelfClashForeign") + foreign_1 = models.ForeignKey("SelfClashForeign", related_name='id') + foreign_2 = models.ForeignKey("SelfClashForeign", related_name='src_safe') + +class SelfClashM2M(models.Model): + src_safe = models.CharField(maxlength=10) + + selfclashm2m_set = models.ManyToManyField("SelfClashM2M") + m2m_1 = models.ManyToManyField("SelfClashM2M", related_name='id') + m2m_2 = models.ManyToManyField("SelfClashM2M", related_name='src_safe') + +error_log = """invalid_models.fielderrors: "charfield": CharFields require a "maxlength" attribute. +invalid_models.fielderrors: "floatfield": FloatFields require a "decimal_places" attribute. +invalid_models.fielderrors: "floatfield": FloatFields require a "max_digits" attribute. +invalid_models.fielderrors: "filefield": FileFields require an "upload_to" attribute. +invalid_models.fielderrors: "prepopulate": prepopulate_from should be a list or tuple. +invalid_models.fielderrors: "choices": "choices" should be either a tuple or list. +invalid_models.fielderrors: "choices2": "choices" should be a sequence of two-tuples. +invalid_models.fielderrors: "choices2": "choices" should be a sequence of two-tuples. +invalid_models.fielderrors: "index": "db_index" should be either None, True or False. +invalid_models.clash1: 'foreign' accessor name 'Target.clash1_set' clashes with another field +invalid_models.clash1: 'foreign' accessor name 'Target.clash1_set' clashes with a related m2m field +invalid_models.clash1: 'm2m' m2m accessor name 'Target.clash1_set' clashes with another field +invalid_models.clash1: 'm2m' m2m accessor name 'Target.clash1_set' clashes with a related field +invalid_models.clash2: 'foreign_1' accessor name 'Target.id' clashes with another field +invalid_models.clash2: 'foreign_1' accessor name 'Target.id' clashes with a related m2m field +invalid_models.clash2: 'foreign_2' accessor name 'Target.src_safe' clashes with a related m2m field +invalid_models.clash2: 'm2m_1' m2m accessor name 'Target.id' clashes with another field +invalid_models.clash2: 'm2m_1' m2m accessor name 'Target.id' clashes with a related field +invalid_models.clash2: 'm2m_2' m2m accessor name 'Target.src_safe' clashes with a related field +invalid_models.clash3: 'foreign_1' accessor name 'Target2.foreign_tgt' clashes with another field +invalid_models.clash3: 'foreign_1' accessor name 'Target2.foreign_tgt' clashes with a related m2m field +invalid_models.clash3: 'foreign_2' accessor name 'Target2.m2m_tgt' clashes with a m2m field +invalid_models.clash3: 'foreign_2' accessor name 'Target2.m2m_tgt' clashes with a related m2m field +invalid_models.clash3: 'm2m_1' m2m accessor name 'Target2.foreign_tgt' clashes with another field +invalid_models.clash3: 'm2m_1' m2m accessor name 'Target2.foreign_tgt' clashes with a related field +invalid_models.clash3: 'm2m_2' m2m accessor name 'Target2.m2m_tgt' clashes with a m2m field +invalid_models.clash3: 'm2m_2' m2m accessor name 'Target2.m2m_tgt' clashes with a related field +invalid_models.clashforeign: 'foreign' accessor name 'Target2.clashforeign_set' clashes with another field +invalid_models.clashm2m: 'm2m' m2m accessor name 'Target2.clashm2m_set' clashes with a m2m field +invalid_models.target2: 'foreign_tgt' accessor name 'Target.target2_set' clashes with a related m2m field +invalid_models.target2: 'foreign_tgt' accessor name 'Target.target2_set' clashes with a related m2m field +invalid_models.target2: 'foreign_tgt' accessor name 'Target.target2_set' clashes with a related field +invalid_models.target2: 'clashforeign_set' accessor name 'Target.target2_set' clashes with a related m2m field +invalid_models.target2: 'clashforeign_set' accessor name 'Target.target2_set' clashes with a related m2m field +invalid_models.target2: 'clashforeign_set' accessor name 'Target.target2_set' clashes with a related field +invalid_models.target2: 'm2m_tgt' m2m accessor name 'Target.target2_set' clashes with a related m2m field +invalid_models.target2: 'm2m_tgt' m2m accessor name 'Target.target2_set' clashes with a related field +invalid_models.target2: 'm2m_tgt' m2m accessor name 'Target.target2_set' clashes with a related field +invalid_models.target2: 'clashm2m_set' m2m accessor name 'Target.target2_set' clashes with a related m2m field +invalid_models.target2: 'clashm2m_set' m2m accessor name 'Target.target2_set' clashes with a related field +invalid_models.target2: 'clashm2m_set' m2m accessor name 'Target.target2_set' clashes with a related field +invalid_models.selfclashforeign: 'selfclashforeign_set' accessor name 'SelfClashForeign.selfclashforeign_set' clashes with another field +invalid_models.selfclashforeign: 'foreign_1' accessor name 'SelfClashForeign.id' clashes with another field +invalid_models.selfclashforeign: 'foreign_2' accessor name 'SelfClashForeign.src_safe' clashes with another field +invalid_models.selfclashm2m: 'selfclashm2m_set' m2m accessor name 'SelfClashM2M.selfclashm2m_set' clashes with a m2m field +invalid_models.selfclashm2m: 'm2m_1' m2m accessor name 'SelfClashM2M.id' clashes with another field +invalid_models.selfclashm2m: 'm2m_2' m2m accessor name 'SelfClashM2M.src_safe' clashes with another field """ diff --git a/tests/runtests.py b/tests/runtests.py index ba387502d0..4a5e5d8a3c 100755 --- a/tests/runtests.py +++ b/tests/runtests.py @@ -171,12 +171,18 @@ class TestRunner: count = management.get_validation_errors(s, mod) s.seek(0) error_log = s.read() - expected = len(mod.error_log.split('\n')) - 1 - if error_log != mod.error_log: + actual = error_log.split('\n') + expected = mod.error_log.split('\n') + + unexpected = [err for err in actual if err not in expected] + missing = [err for err in expected if err not in actual] + + if unexpected or missing: + unexpected_log = '\n'.join(unexpected) + missing_log = '\n'.join(missing) log_error(model_name, - "Validator found %d validation errors, %d expected" % (count, expected), - "Expected errors:\n%s\n\nActual errors:\n%s" % (mod.error_log, error_log)) - + "Validator found %d validation errors, %d expected" % (count, len(expected) - 1), + "Missing errors:\n%s\n\nUnexpected errors:\n%s" % (missing_log, unexpected_log)) if run_othertests: # Run the non-model tests in the other tests dir