Models with GenericRelation are unnecessarily validated for clashes in reverse manager accessor

[Ricky Rosario]

GenericRelation fields don't add a reverse manager to the related model. However, the model validation verifies that there are no collisions forcing the use of an unused related_name parameter (which then can cause issues such as #16913).

To reproduce:

Create a model with a GenericForeignKey and a related model with a GenericRelation. testapp1/models.py:

from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes import generic
from django.db import models


class Topic(models.Model):
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    content_object = generic.GenericForeignKey('content_type', 'object_id')


class Post(models.Model):
    topic = generic.GenericRelation(Topic)

In a different app, create another related model with the same name. testapp2/models.py:

from django.contrib.contenttypes import generic
from django.db import models

from testapp1.models import Topic

class Post(models.Model):
    topic = generic.GenericRelation(Topic)

Now run syncdb:

$ ./manage.py syncdb

Error: One or more models did not validate:
testapp2.post: Accessor for m2m field 'topic' clashes with related m2m field 'Topic.post_set'. Add a related_name argument to the definition for 'topic'.
testapp1.post: Accessor for m2m field 'topic' clashes with related m2m field 'Topic.post_set'. Add a related_name argument to the definition for 'topic'.

[Carl Meyer]

This probably blocks on #16905.

[Carl Meyer]

Also, as part of this fix GenericRelation should probably be modified so it doesn't silently accept the useless (and problematic) related_name argument at all.

[Gabe Jackson]

#22207 is pre-cursor to fixing this.

[Gabe Jackson]

this was fixed by commit ​https://github.com/django/django/commit/97774429aeb54df4c09895c07cd1b09e70201f7d as part of #19385

works as of tag 1.6a1, where tag 1.5.1 fails.

[Gabe Jackson]

Wrote two tests to cover this.

On django 1.5.1:

gabejackson@jax: tests# PYTHONPATH=..:$PYTHONPATH python ./runtests.py --settings=test_sqlite generic_relations
Creating test database for alias 'default'...
Creating test database for alias 'other'...
..F...F..
======================================================================
FAIL: test_generic_relation_related_name_not_allowed (generic_relations.tests.GenericRelationsTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/gabejackson/Documents/pycharm/django-1.6.2/tests/generic_relations/tests.py", line 245, in test_generic_relation_related_name_not_allowed
    class InvalidGenericRelationModel(models.Model):
AssertionError: TypeError not raised

======================================================================
FAIL: test_multiple_gen_rel_with_same_class_name (generic_relations.tests.GenericRelationsTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/gabejackson/Documents/pycharm/django-1.6.2/tests/generic_relations/tests.py", line 259, in test_multiple_gen_rel_with_same_class_name
    self.fail("validate() failed with: %s" % e)
AssertionError: validate() failed with: One or more models did not validate:
appone.post: Accessor for m2m field 'topic' clashes with related m2m field 'Topic.post_set'. Add a related_name argument to the definition for 'topic'.
apptwo.post: Accessor for m2m field 'topic' clashes with related m2m field 'Topic.post_set'. Add a related_name argument to the definition for 'topic'.


----------------------------------------------------------------------
Ran 9 tests in 0.107s

FAILED (failures=2)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...

on master:

Testing against Django installed in '/Users/gabejackson/Documents/pycharm/django-generic-rel-reverse/django'
Creating test database for alias 'default'...
Creating test database for alias 'other'...
.........................
----------------------------------------------------------------------
Ran 25 tests in 0.210s

OK
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...

Pull Request is here: ​https://github.com/django/django/pull/2407

[Gabe Jackson]

after some more digging, this problem hasn't been solved – it only vanished. Apparently, GenericRelations currently don't get check()'ed. This may have gotten lost during implementation of the new checks framework. the PR now contains 3 tests:

• One to test that related_name will raise a TypeError when defined on GenericRelation
• One to test the OPs concern of defining the same model name in two different apps - this no longer leads to a clash since we do not check related_name clashes on GenericRelation anymore (changed this code)
• One to test that equal related_query_names still resolve in a clash when one or more GenericRelations define the same related_query_name to the same related object

PR is updated

[Tim Graham]

I left comments for improvement on the PR. Please uncheck "Patch needs improvement" when you update it, thanks.

[Gabe Jackson]

Clean up according to review. I'm not sure the refactor of _check_accessor_clashes and _check_reverse_query_clashes to _check_clashes(check_related_query_name=True/False) leads to more legible code, but at least code duplication is now gone.

Also rebased on current master.

[Tim Graham]

Tests are not passing.