Opened 9 years ago

Closed 8 years ago

#2536 closed defect (fixed)

Mutually referential many-to-one relationship AddManipulator fails.

Reported by: anonymous Owned by: adrian
Component: Database layer (models, ORM) Version:
Severity: normal Keywords:
Cc: bmurdock@…, pythonmailing@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: UI/UX:

Description

There is an example of this given here:

http://www.djangoproject.com/documentation/models/mutually_referential/

It hasn't been updated to reflect magic removal changes, but seems pretty straightforward:

class Parent( models.Model ):
    name = models.CharField(maxlength=100, core=True)
    bestchild = models.ForeignKey("Child", null=True, related_name="favoured_by")

    class Admin:
        pass

class Child( models.Model ):
    name = models.CharField(maxlength=100)
    parent = models.ForeignKey(Parent)

    class Admin:
        pass

When playing with these models in the shell, everything seems to work fine. When I use the admin interface and click to add a child I get:

'bool' object has no attribute 'get'

with the full stack trace:

Traceback (most recent call last):
File "/usr/lib/python2.4/site-packages/django/core/handlers/base.py" in get_response
  74. response = callback(request, *callback_args, **callback_kwargs)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/views/decorators.py" in _checklogin
  55. return view_func(request, *args, **kwargs)
File "/home/bryan/web/django_src/django/views/decorators/cache.py" in _wrapped_view_func
  40. response = view_func(request, *args, **kwargs)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/views/main.py" in add_stage
  246. manipulator = model.AddManipulator()
File "/home/bryan/web/django_src/django/db/models/manipulators.py" in __init__
  75. self.fields.extend(f.get_manipulator_fields(self.opts, self, self.change, fol))
File "/usr/lib/python2.4/site-packages/django/db/models/related.py" in get_manipulator_fields
  116. if follow.get(f.name, False):

  AttributeError at /admin/organizer/child/add/
  'bool' object has no attribute 'get'

Adding a Parent works until you try and add the bestchild attribute.

Attachments (1)

models.py (4.5 KB) - added by ramiro <rm0 _at_ gmx.net> 9 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 9 years ago by bmurdock@…

I didn't mean to leave the submitter as anonymous, sorry. It was me, bmurdock@…

comment:2 Changed 9 years ago by johann.queuniet@…

I'm having a similar issue with a forum script and the following model.

class Topic(models.Model):
        last = models.ForeignKey('Post', related_name="last_mess_set")
        title = models.CharField(maxlength=100, unique=True)

class Post(models.Model):
        topic = models.ForeignKey(Topic)
        date = models.DateTimeField(editable=False, auto_now_add=True)
        content = models.TextField()

This issue isn't specific to the admin, and seems to come from one of the AddManipulator itself, the Post model one in my case. Looks really similar to #1839, but with a circular relationship instead.

This is what I get from my shell.

In [1]: from testcirc.test.models import Topic, Post

In [2]: Topic.AddManipulator()
Out[2]: <django.db.models.manipulators.AddManipulator object at 0xb78f084c>

In [3]: Post.AddManipulator()
---------------------------------------------------------------------------
exceptions.AttributeError                            Traceback (most recent call last)

/home/johann/web/testcirc/<console> 

/usr/lib/python2.4/site-packages/django/db/models/manipulators.py in __init__(self, follow)
     73             if self.follow.get(f.name, False):
     74                 fol = self.follow[f.name]
---> 75                 self.fields.extend(f.get_manipulator_fields(self.opts, self, self.change, fol))
     76 
     77         # Add field for ordering.

/usr/lib/python2.4/site-packages/django/db/models/related.py in get_manipulator_fields(self, opts, manipulator, change, follow)
    114         for i in range(count):
    115             for f in self.opts.fields + self.opts.many_to_many:
--> 116                 if follow.get(f.name, False):
    117                     prefix = '%s.%d.' % (self.var_name, i)
    118                     fields.extend(f.get_manipulator_fields(self.opts, manipulator, change,

AttributeError: 'bool' object has no attribute 'get'
———

comment:3 Changed 9 years ago by bmurdock@…

  • Cc bmurdock@… added
  • Component changed from Admin interface to Database wrapper
  • Summary changed from Mutually referential many-to-one relationships admin broken to 'manipulators' module: API test failed

Interesting. I just ran the unit tests and it looks like they catch this defect too:

Running tests with database 'postgresql'

'manipulators' module: API test failed
======================================
Code: 'man = Musician.AddManipulator()'
Line: 4
Expected: ''
Got: "f is: <RelatedObject: album related to musician>\nself.follow is: {'first_name': True, 'last_name': True, 'id': True}\n"
1 error:

I just did a search on "manipulators" and found a few other tickets that might be the same, or at least related, to this one:

http://code.djangoproject.com/ticket/2415

http://code.djangoproject.com/ticket/1839 (mentioned above)

http://code.djangoproject.com/ticket/2213 (maybe not, not too many details)

http://code.djangoproject.com/ticket/1808 (again, not totally sure, but looks kinda similar)

Changing the summary and component to reflect this seemingly closer-to-root-cause information.

comment:4 Changed 9 years ago by mtredinnick

The existing tests do not *universally* fail, unfortunately. They run (against all three databases) without error on my system (using [3588]).

bmurdock: can you provide some more specifics about your setup please. Are you using a current svn checkout? Which version of psycopg, which OS, which postgreSQL version? Do the tests fail for you when run against other databases (MySQL or SQLite)? It would be nice to track down the specific differences that are affecting this failure, since tests that only fail sometimes are not so much fun.

comment:5 Changed 9 years ago by bmurdock@…

*sigh*

Great, one of /those/ kinds of problems. But thanks for the help! Here's what I'm using:

  • Ubuntu 6.06.1 LTS
  • python 2.4.2-0ubuntu3
  • django from svn, revision [3571]
  • python2.4-psycopg, version 1.1.21-3ubuntu3
  • postgresql-8.1, version 8.1.4-0ubuntu1

Don't know if these are as relevant to this issue, but:

  • apache2 2.0.55-4ubuntu2.1
  • libapache2-mod-python2.4 3.1.4-0ubuntu1

Using sqlite:

  • sqlite3, version 3.2.8-1ubuntu1
  • python2.4-pysqlite2, version 2.0.5-1ubuntu1

I get:

$ ./runtests.py --settings=settings
Running tests with database 'sqlite3'

'manipulators' module: API test failed
======================================
Code: 'man = Musician.AddManipulator()'
Line: 4
Expected: ''
Got: "f is: <RelatedObject: album related to musician>\nself.follow is: {'first_name': True, 'last_name': True, 'id': True}\n"
1 error:

At this point I'm guessing MySQL would do the same. What versions of stuff are you using?

comment:6 Changed 9 years ago by bmurdock@…

  • Summary changed from 'manipulators' module: API test failed to Mutually referential many-to-one relationship AddManipulator fails.

First of all, I'm an idiot. The unit tests do pass. I was doing a little "printf" debugging (I guess it's just "print" debugging with Python), and left my print statements in the code when I ran the unit tests, and they didn't like the extra output and failed. I took the print statments back out, and they didn't fail. My apologies.

So, in summary, the unit tests all pass, and it *isn't* one of /those/ kinds of problems.

My AddManipulator problem is still there. The print statements are starting to enlighten me, but this being my first big forray into the django source, and being pretty new to python (and being an idiot, as we've seen) it's slow going. I don't think I can articulate what I've learned yet, the behavior is slightly different with my models versus the example Parent and Child models, but it has to do with the dict named 'follow' in the AutomaticManipulator constructor. I haven't quite traced where that comes from yet. Probably the easiest way to reproduce this is to change django_src/tests/modeltests/mutually_referential/models.py to have these additional test lines before the q.delete():

# test AddManipulator for both()
>>> Parent.AddManipulator()
>>> Child.AddManipulator()

In case anyone else is interested in reproducing it.

While I'm disclosing my idiocy, I should also point out that the documentation for the Mutually referential many-to-one relationship is in fact using the latest magic-removal style, just a little differently than the example in the tutorial. Phew, there, that feels better. Hmmm, how else can I make this ticket long and obnoxious?

comment:7 Changed 9 years ago by bmurdock@…

I played with this some more this evening (I'd really like the admin interface to work for my mutually referential models!) If I make the child look like this:

#! python
class Child(Model):
    name = CharField(maxlength=100)
    myparent = ForeignKey(Parent)

Then Child.AddManipulator() now works. The admin app seems to work wonderfully now too. Though I wonder, there's gotta be some downside to this workaround, right?

I decided to do a little more playing around. If I make the classes look like this:

#! python
from django.db.models import *

class Parent(Model):
    name = CharField(maxlength=100, core=True)
    child = ForeignKey("Child", null=True, related_name="favoured_by")

class Child(Model):
    name = CharField(maxlength=100)
    parent = ForeignKey(Parent, related_name="kiddo")

Then AddManipulator() fails for both models with the same complaint: 'bool' object has no attribute 'get'. So, even though the general recommendation is to name ForeignKey attributes after the classes they references, if you do that in this case, you get errors.

comment:8 Changed 9 years ago by Marek Kubica <pythonmailing@…>

  • Cc pythonmailing@… added

I'm getting a similar problem here. I tried to add a new ForeignKey to my database without resetting all data. The relevant database-part is a Series-model which was supposed to have a representative ForeignKey which should point to a Watch-model. The Watch-model itself has another ForeignKey referencing the Series-model to define which series it is in. But after adding the new representative FK, I cannot display my existing Watch-models in the admin (inspecting them with the shell works, though). Adding does not work either.

The traceback when I want to display an existing Watch is:

Traceback (most recent call last):
File "/usr/local/lib/python2.5/site-packages/django/core/handlers/base.py" in get_response
  77. response = callback(request, *callback_args, **callback_kwargs)
File "/usr/local/lib/python2.5/site-packages/django/contrib/admin/views/decorators.py" in _checklogin
  55. return view_func(request, *args, **kwargs)
File "/usr/local/lib/python2.5/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  39. response = view_func(request, *args, **kwargs)
File "/usr/local/lib/python2.5/site-packages/django/contrib/admin/views/main.py" in add_stage
  243. manipulator = model.AddManipulator()
File "/usr/local/lib/python2.5/site-packages/django/db/models/manipulators.py" in __init__
  75. self.fields.extend(f.get_manipulator_fields(self.opts, self, self.change, fol))
File "/usr/local/lib/python2.5/site-packages/django/db/models/related.py" in get_manipulator_fields
  116. if follow.get(f.name, False):

  AttributeError at /admin/homepage/watch/add/
  'bool' object has no attribute 'get'

When I try to add a new object I get this in turn:

>>> Watch.AddManipulator()
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/usr/local/lib/python2.5/site-packages/django/db/models/manipulators.py", line 75, in __init__
    self.fields.extend(f.get_manipulator_fields(self.opts, self, self.change, fol))
  File "/usr/local/lib/python2.5/site-packages/django/db/models/related.py", line 116, in get_manipulator_fields
    if follow.get(f.name, False):
AttributeError: 'bool' object has no attribute 'get'

My system is

  • Django from SVN, currently revision [4458] (updated today)
  • Debian Sarge 3.1
  • LigHTTPd 1.4.11-2bpo1
  • PostgreSQL 7.4.7-6sarge3
  • Python 2.5-2~bpo.1 (actually, I built this myself and added a bpo suffix)
  • python2.5-psycopg 1.1.21-3~bpo.1 (this was taken from Ubuntu and modified for Debian)

But I can say that this is most certainly some issue with Django and not something related with my setup which works rather well.

comment:8 Changed 9 years ago by Marek Kubica <pythonmailing@…>

  • Cc pythonmailing@… added

I'm getting a similar problem here. I tried to add a new ForeignKey to my database without resetting all data. The relevant database-part is a Series-model which was supposed to have a representative ForeignKey which should point to a Watch-model. The Watch-model itself has another ForeignKey referencing the Series-model to define which series it is in. But after adding the new representative FK, I cannot display my existing Watch-models in the admin (inspecting them with the shell works, though). Adding does not work either.

The traceback when I want to display an existing Watch is:

Traceback (most recent call last):
File "/usr/local/lib/python2.5/site-packages/django/core/handlers/base.py" in get_response
  77. response = callback(request, *callback_args, **callback_kwargs)
File "/usr/local/lib/python2.5/site-packages/django/contrib/admin/views/decorators.py" in _checklogin
  55. return view_func(request, *args, **kwargs)
File "/usr/local/lib/python2.5/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  39. response = view_func(request, *args, **kwargs)
File "/usr/local/lib/python2.5/site-packages/django/contrib/admin/views/main.py" in add_stage
  243. manipulator = model.AddManipulator()
File "/usr/local/lib/python2.5/site-packages/django/db/models/manipulators.py" in __init__
  75. self.fields.extend(f.get_manipulator_fields(self.opts, self, self.change, fol))
File "/usr/local/lib/python2.5/site-packages/django/db/models/related.py" in get_manipulator_fields
  116. if follow.get(f.name, False):

  AttributeError at /admin/homepage/watch/add/
  'bool' object has no attribute 'get'

When I try to add a new object I get this in turn:

>>> Watch.AddManipulator()
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/usr/local/lib/python2.5/site-packages/django/db/models/manipulators.py", line 75, in __init__
    self.fields.extend(f.get_manipulator_fields(self.opts, self, self.change, fol))
  File "/usr/local/lib/python2.5/site-packages/django/db/models/related.py", line 116, in get_manipulator_fields
    if follow.get(f.name, False):
AttributeError: 'bool' object has no attribute 'get'

My system is

  • Django from SVN, currently revision [4458] (updated today)
  • Debian Sarge 3.1
  • LigHTTPd 1.4.11-2bpo1
  • PostgreSQL 7.4.7-6sarge3
  • Python 2.5-2~bpo.1 (actually, I built this myself and added a bpo suffix)
  • python2.5-psycopg 1.1.21-3~bpo.1 (this was taken from Ubuntu and modified for Debian)

But I can say that this is most certainly some issue with Django and not something related with my setup which works rather good.

comment:9 Changed 9 years ago by Marek Kubica <pythonmailing@…>

My progress this far is that both AddManipulator and ChangeManipulator are broken, which causes the Django-admin to fail when it should display or create one of these models. They are both instances of AutomaticManipulator defined in source:django/trunk/django/db/models/manipulators.py, they both use the same init, so this problem has to be fixed most probably somewhere in there. It is definitely not a problem of the ORM itself, as that is able so create and modify instances of these models perfectly well (tested manually in the interactive interpreter).

Changed 9 years ago by ramiro <rm0 _at_ gmx.net>

comment:10 Changed 9 years ago by ramiro <rm0 _at_ gmx.net>

Find attached a models.py file that contains six model pairs with mutually referential many-to-one relationships. Every pair of models modifies something trying to isolate the real cause. Conclusion: it seems problem is related to the fact the FK name is equal to the related model name but in lowercase (something already hinted by bmurdock in a comment above).

Note that the doctest in the Child model creates a Child.AddManipulator instance w/o getting any error but the
same instantiation leads to the reported traceback when the user is going to create a Child instance in the Admin app.

comment:11 Changed 9 years ago by Marek Kubica <pythonmailing@…>

ramiro and bmurdock are right - this is caused by some name-clash (but in my tests with the shell Child.AddManipulator() crashes always, not just in the admin). Taking the first two models or ramiro's models.py that gives us two models. To check it, I added two print statements to source:django/trunk/django/db/models/manipulators.py#4458 between line 72 and 73:

print f.name
print self.follow

Now, trying to add a Child object with the admin shows (on stdout):

parent
{'id': True, 'parent': True, 'name': True}

As self.follow.get(f.name, False) yields True, the code inside the if statement is executed, causing a crash. Trying to add a Parent objects gives different results:

child
{'bestchild': True, 'id': True, 'name': True}

Now self.follow.get(f.name, False) really is False, so the code is not executed. This seems to me to be some kind of misbehaviour, because it looks up for the wrong attribute - if it looked up correctly, this would have to be True as well, I think.

I suspect that this works only because it is looking up incorrectly (name-clashes or whatever) - if it would look up correctly both Child.AddManipulator() and Parent.AddManipulator() would crash. Sounds paradoxical, indeed. Finally, it does not solve the whole problem, but the wrong lookup causes some AddManipulator to fail and others not to fail. To solve this problem the lookup mechanics need to be fixed first. Unfortunately, I've got no idea whether f.name is wrong or the key in self.follow is incorrect. Hopefully someone who knows more about the internals of Django can tell me which one is to fix (or explain to me that the lookup is correct).

comment:12 Changed 9 years ago by Simon G. <dev@…>

  • Triage Stage changed from Unreviewed to Accepted

comment:13 Changed 9 years ago by ubernostrum

  • Resolution set to wontfix
  • Status changed from new to closed

(closing this because the manipulator system is being deprecated and removed)

comment:14 Changed 9 years ago by Gary Wilson <gary.wilson@…>

  • Needs tests set
  • Resolution wontfix deleted
  • Status changed from closed to reopened

Even though this probably won't get fixed for manipulators (unless someone really wants it and submits a patch, in which case we will gladly accept), we do need to make sure that the bug doesn't also exist in newforms. At the very least, we should make a test for this using newforms to be assured it has been fixed.

So as a general rule, tickets that are requesting new features for Manipulators (like #1679) can be closed as wontfix, but tickets that are bugs should be left open to make certain that the same problem doesn't exist in newforms.

comment:15 follow-up: Changed 9 years ago by ubernostrum

  • Resolution set to wontfix
  • Status changed from reopened to closed

Gary, as far as I know follow (the root of many bugs, including this one) isn't being replicated by anything in newforms, so this should be safe to keep closed.

comment:16 in reply to: ↑ 15 Changed 9 years ago by Gary Wilson <gary.wilson@…>

I was just thinking that we should at least have a test for this so that it never becomes a problem again in the future. You don't think so?

comment:17 Changed 8 years ago by ubernostrum

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Actually...

I'm reopening this, because now I think I know what causes the bug and it's not in the manipulator system at all; the manipulator errors are a symptom of the deeper problem, and just happen to be an easy way to trigger it.

comment:19 Changed 8 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [4676]) Fixed #1839, #2415, #2536 -- Fixed a generated name clash that was common in
self-referential and circular relations. A lot of community debugging went into
this fix, so thanks to bmurdock@…, Marek Kubica, ramiro, Michael
Radziej (the last two giving test cases showing the problem) and James Bennett
(who did the hard work to actually diagnose the true problem and fix it).

Note: See TracTickets for help on using tickets.
Back to Top