Opened 11 years ago

Closed 11 years ago

#20090 closed Bug (invalid)

SubfieldBase/to_python handles both serialized and original string which breaks custom model fields like JSONField

Reported by: bjasper@… Owned by: nobody
Component: Uncategorized Version: 1.4
Severity: Normal Keywords: subfieldbase to_python custom field json jsonfield
Cc: Jacob Rief Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When using a custom model field liked JSONField (a field that lets you store validated JSON in your model), the way to_python() method handles both the serialized and original string makes it difficult (impossible?) to determine which case we're handling––so we never know if a string is still encoded JSON or has already been decoded.

This is a problem, for example, when storing raw numbers.

If I store the raw number 13, I'll get back the number 13.

But if I store the string "13", I'll still get back the number 13. This is because to_python() receives the "13", see's it's a string, thinks it's JSON, and decodes it.

It doesn't know if "13" should be returned as a string or returned as a number since to_python() doesn't know the context in which it's being called.

The only way we've found to get around this on our end is to prepend JSON encoded strings with the text "json:". This way we'd know for sure if a string was still encoded JSON. This is a big hack, so we'd like to avoid it as much as possible.

I have some tests but they are tied to the existing test suite––hope that's OK:

git clone -b broken_to_python https://github.com/bradjasper/django-jsonfield.git
cd broken_to_python
python setup.py test

Original discussion started here: https://github.com/bradjasper/django-jsonfield/issues/33

Change History (3)

comment:1 by anonymous, 11 years ago

See: django/db/models/fields/subclassing.py

The method Field.to_python is called in Field.clean and in Creator.set in a field with metaclass SubfieldBase.

You can create your own subfieldbase metaclass and Creator accessor class. In the new Creator class the method pre_init is called on the field object instead of to_python.

We now can determine the state of the model instance and know when to convert the string or not, see example below:

class SubfieldBase(type):
    """copied from django/db/models/fields/subclassing.py
"""

    def __new__(cls, name, bases, attrs):
        new_class = super(SubfieldBase, cls).__new__(cls, name, bases, attrs)
        new_class.contribute_to_class = make_contrib(
            new_class, attrs.get('contribute_to_class')
        )
        return new_class

class Creator(object):
    """
copied from django/db/models/fields/subclassing.py

"""
    def __init__(self, field):
        self.field = field

    def __get__(self, obj, type=None):
        if obj is None:
            raise AttributeError('Can only be accessed via an instance.')
        return obj.__dict__[self.field.name]

    def __set__(self, obj, value):
        #model instance is now accessible in the field object
        obj.__dict__[self.field.name] = self.field.pre_init(value, obj)

def make_contrib(superclass, func=None):
    """
copied from django/db/models/fields/subclassing.py
"""
    def contribute_to_class(self, cls, name):
        if func:
            func(self, cls, name)
        else:
            super(superclass, self).contribute_to_class(cls, name)
        setattr(cls, self.name, Creator(self))

    return contribute_to_class



class JSONField(models.Field):
    #use our own implementation which calls pre_init instead of to_python
    __metaclass__ = SubfieldBase 
    
    def pre_init(self, value, obj):
        #data from database
        if obj._state.adding and obj.pk is not None:
            return json.loads(value)

        return value

    #etc...
    

comment:2 by Jacob Rief, 11 years ago

Cc: Jacob Rief added

I can confirm this.

comment:3 by bjasper@…, 11 years ago

Resolution: invalid
Status: newclosed

Thanks for the help (and confirmation jrief). This should take care of what we need–-closing this ticket.

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