Code

Opened 3 years ago

Last modified 13 months ago

#15184 new Bug

Error when subclassing models.ForeignKey field

Reported by: rupa108 Owned by: furious_luke
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: ForeignKey, models, subclassing
Cc: furious_luke Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hello,

subclassing from

djago.db.models.ForeignKey

does not work as documented.

As soon as I do:

__metaclass__ = models.SubfieldBase

accessing a model instance gives the error:

Traceback (most recent call last):
  File "/home/roman/devel/kundebunt_new/src/kundebunt/contrib/views.py", line 474, in __call__
    call_method_return = call_method(request,obj)
  File "/home/roman/devel/kundebunt_new/src/kundebunt/contrib/views.py", line 561, in view
    fields = self._get_record_properties_data(obj)
  File "/home/roman/devel/kundebunt_new/src/kundebunt/contrib/views.py", line 533, in _get_record_properties_data
    value = self._get_field_data(record,spalte)
  File "/home/roman/devel/kundebunt_new/src/kundebunt/contrib/views.py", line 268, in _get_field_data
    if getattr(record,spalte['name']) == None:
  File "/home/roman/devel/django-trunk/django/db/models/fields/subclassing.py", line 96, in __get__
    return obj.__dict__[self.field.name]
KeyError: 'kunde'

Please see http://groups.google.com/group/django-users/browse_thread/thread/129bbebfb9df206f/2fbe2264a59ded79?lnk=gst&q=roman#2fbe2264a59ded79

Regards

Roman

Attachments (1)

foreignkey_subclass_patch.diff (4.3 KB) - added by furious_luke 3 years ago.
Patch to correct a bug in subclass ForeignKeyField.

Download all attachments as: .zip

Change History (13)

comment:1 follow-up: Changed 3 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

As it says in the docs:
"
If you're handling custom Python types, such as our Hand class, we need to make sure that when Django initializes an instance of our model and assigns a database value to our custom field attribute, we convert that value into the appropriate Python object. The details of how this happens internally are a little complex, but the code you need to write in your Field class is simple: make sure your field subclass uses a special metaclass:
"

That is, you only use SubfieldBase if your Field type will be handling custom Python types -- such as the Hand object in the custom fields example. If you're just looking to add behavior to an existing Django field type, you don't have to use the metaclass.

comment:2 in reply to: ↑ 1 Changed 3 years ago by rupa108

  • Resolution invalid deleted
  • Status changed from closed to reopened

Hello,

Replying to russellm:

As it says in the docs:

thanks for your reply and sorry if I'm wrong here, however:

I read the docs, and I think I understand them. And I do use SubfieldBase successfully in various places and everything works as expected.
It does not work when subclassing from ForeignKey.

Of course I implement to_python() and occaionally formfield() in my derived classes and also get_prep_lookup(), that's why I need to SubfieldBase ...

But it does not work when subclassing from ForeignKey.

Here is another example:

class FkStandortAX(models.ForeignKey):
    __metaclass__ = models.SubfieldBase
    description = "ForeignKey displaying address field prepared for Ajax Autocompletion"
           
    def to_python(self,value):
        standort = dict(
            name =  value.name,
            strasse = value.adresse and  value.adresse.strasse,
            kfz = value.adresse and value.adresse.land and value.adresse.land.kfz,
            plz = value.adresse and value.adresse.plz,
            ort = value.adresse and value.adresse.ort)
        
        return u'{name} -- {strasse}, {kfz}{plz} {ort}'.format(**standort)

The column definition in the model:

standort = FkStandortAX(verbose_name=_(u"Standort"),to="popkern.Person", db_column='standort')

When using it:

In [1]: from kundebunt.housing import models    

In [2]: r=models.RZ.objects.all()[3]

In [3]: r._meta.fields
Out[3]: 
[<django.db.models.fields.AutoField object at 0x1434410>,
 <django.db.models.fields.DateTimeField object at 0x1434250>,
 <django.db.models.fields.CharField object at 0x1434290>,
 <kundebunt.contrib.models.fields.FkStandortAX object at 0x1434310>]

In [4]: [f.name for f in r._meta.fields]
Out[4]: ['id', 'timestamp', 'name', 'standort']

In [5]: r.standort  

---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)

/home/roman/devel/kundebunt_new/src/kundebunt/<ipython console> in <module>()

/home/roman/devel/django-trunk/django/db/models/fields/subclassing.pyc in __get__(self, obj, type)
     94         if obj is None:
     95             raise AttributeError('Can only be accessed via an instance.')
---> 96         return obj.__dict__[self.field.name]
     97 
     98     def __set__(self, obj, value):

KeyError: 'standort'

Accessing r.standort should not raise an error here. It's there in the field list but it's not accessible. How is the value that to_python returns supposed to be accessed if not like this?

Regards

Roman

comment:3 Changed 3 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

Ok - I'm going to mark this accepted -- line 96 of subclassing.py looks like it should be field.attname, not field.name. There may be other problems involved with subclassing ForeignKey that follow on from that fix; that will require some additional investigation.

However, I would make two comments:

  1. When reporting a bug, you need to give us *all* the information you have, not just the bits you want to give us. Nowhere in your mailing list posts or in this ticket did you mention that you were using to_python. That's a pretty important detail for anybody that wants to reproduce the problem. Remember, if we can't reproduce your problem, the problem effectively doesn't exist.
  1. The reason you've found this bug is that you are almost certainly Doing It Wrong (tm). Your model layer shouldn't care whether it's being used for AJAX or not. A foreign key is a pointer to another model. The "value" for that model is something that uniquely references another object. A primary key value (or, by proxy, a Django object that encapsulates that primary key value) is the most sensible representation of that value. The need to be "AJAXified" is a display level concern. AJAX rendering considerations should be in your forms/templating layer, not your data model.

comment:4 Changed 3 years ago by rupa108

Thank's for taking the time to investigate this and sorry for confusing things. You're right, I certainly got mixed up and the things I'm trying to do should be done differently. Nevertheless I'm glad that you consider to make changes here.

comment:5 Changed 3 years ago by lrekucki

  • Severity set to Normal
  • Type set to Bug

comment:6 Changed 3 years ago by furious_luke

  • Cc furious_luke added
  • Owner changed from nobody to furious_luke
  • Status changed from reopened to new

Changed 3 years ago by furious_luke

Patch to correct a bug in subclass ForeignKeyField.

comment:7 Changed 3 years ago by furious_luke

  • Has patch set
  • Resolution set to fixed
  • Status changed from new to closed

The issue appeared to be that SubfieldBase's descriptor was stomping on ForeignKey's. I've added a check for an existing value under the same name in SubfieldBases's descriptor; this way if the field name is missing from a model's __dict__ attribute we can hand off responsibility to the previously existing descriptor.

A new unit test has been added to check for this problem.

comment:8 Changed 3 years ago by julien

  • Resolution fixed deleted
  • Status changed from closed to reopened

The ticket will be closed as fixed the day that a patch actually gets *checked in* to Django.

comment:9 Changed 3 years ago by furious_luke

Ah, right you are then. Sorry about that.

comment:10 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:11 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:12 Changed 13 months ago by aaugustin

  • Status changed from reopened to new

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from furious_luke to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.