Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can not store ordinary Python strings in JSONfield #33

Closed
jrief opened this issue Feb 26, 2013 · 20 comments
Closed

Can not store ordinary Python strings in JSONfield #33

jrief opened this issue Feb 26, 2013 · 20 comments

Comments

@jrief
Copy link
Contributor

jrief commented Feb 26, 2013

Since version 0.9.4 I am not able to store ordinary Python strings in a JSONfield. Up to version 0.9.2 this worked without problems.
Did I miss something? From my point of view, it should not matter, if I store ordinary strings, lists or dicts in a JSONfield.

@jrief
Copy link
Contributor Author

jrief commented Feb 26, 2013

From my point of view, this additional test shall pass:

    def test_string_in_json_field(self):
        """Test saving an ordinary Python string in our JSONField"""
        json_obj = 'blah blah blah'
        obj = self.json_model.objects.create(json=json_obj)
        new_obj = self.json_model.objects.get(id=obj.id)

        self.failUnlessEqual(new_obj.json, json_obj)

@bradjasper
Copy link
Collaborator

Hey @jrief you are correct that a regular string should be valid. I'll look into this––thanks for the test.

@jrief
Copy link
Contributor Author

jrief commented Feb 26, 2013

Just an idea on how to solve it.
Put the object of unknown type as a single element into a list. When converting back from string to object, just extract this single object and return it. This saves all kinds of case distinctions. A problem could be backward compatibility. Would you agree with such a design?

@bradjasper
Copy link
Collaborator

Hey @jrief sorry for the delay on this. I'm not crazy about storing elements in a list for backward compatibility reasons––I'd love to find a cleaner way to do this. I'm going to spend some time working/thinking on this and try to figure out a solution.

jrief added a commit to jrief/django-jsonfield that referenced this issue Mar 1, 2013
@bradjasper
Copy link
Collaborator

Closing as this was fixed in e7978a0

@lmeunier
Copy link

Hi,

It seems this issue is not totally fixed, I can't store strings with specific values like "123" or "false", strings are converted to integer or boolean.

I have added two new tests to illustrate this: lmeunier/django-jsonfield@8e6e915a37a1a38c00fa86f34a0d3462932cacaa

@jrief
Copy link
Contributor Author

jrief commented Mar 11, 2013

The problem is that in JSONFieldBase.to_python(self, value) value always is a string, regardless if the input was float(1.23) or str("1.23").
The only difference I can see, is that a float is converted to unicode, whereas "1.23" is left as ascii, but using that information would create a whole bunch of other problems.
And I have no idea how to solve this.

@bradjasper
Copy link
Collaborator

Thanks for the tests @lmeunier and comments @jrief

Agreed on not sure the best way to handle this. I've got some time tomorrow to spend on this and will post an update with any info I find.

@bradjasper bradjasper reopened this Mar 11, 2013
@bradjasper
Copy link
Collaborator

I spent some time looking into this and didn't come away with great results.

As you said @jrief it seems to_python always receives a string, which makes storing JSON in a string a problem.

I found someone experiencing a similar problem (http://stackoverflow.com/questions/4510162/django-custom-field-only-run-to-python-on-values-from-db/7668745#7668745) and their solution was to prepend the JSON encoded string with "json:"

I don't really like this solution––but as of right now this is the only thing I've found that might work.

I'm going to spend some more time looking into this, but wanted to see what everyone else thought about this approach.

@lmeunier
Copy link

Thanks for looking at this issue @bradjasper. We come to the same conclusion: we can never be sure that the string passed to to_python is already JSON encoded. This is because the to_python method must handle in the same parameter the serialized string and the original string. Maybe it could be worth opening a ticket on the Django bugtracker to see if this behaviour can be changed.

The solution to prepend the JSON encoded string with "json:" should work for every cases except one: I can't store a string beginning with "json:". But, if it's documented somewhere, I think we could live with it.

@fletom
Copy link
Contributor

fletom commented Mar 13, 2013

Strings can be serialized just fine in JSON, so the problem, as I understand it, is that Django is badly designed such that it applies the database deserialization method even to values that are set directly by Python?

So when you write obj.my_JSON_field = "spam" then to_python is called with "spam"?

In that case, this is really a problem with Django and we should be discussing this on their issue tracker. In theory it should be easy to add a feature that lets you choose different methods for processing values from the database and values from Python itself.

@jrief
Copy link
Contributor Author

jrief commented Mar 13, 2013

@fletom Yes, from my point of view this is a Django issue, not an issue of this module.
@lmeunier I had a similar proposal: Embed our object into a list of length 1. Then the JSON decode will work fine. Both approaches however have backward compatibility issues. I'am not sure, if they can be solved by a migration script. If we can agree to break backward compatibilities I would go for the list of length 1, because it adds less "magicness" to the module.

@lmeunier
Copy link

@jrief I have tried to put the field content into a list of length 1, but then you can't store a string formatted like this obj.my_json_field='["foo"]'. In the to_python method, you can never be sure if the string is already JSON encoded or not.

@fletom I would love to open a ticket in Django, but my english is too bad to explain the problem clearly :)

@bradjasper
Copy link
Collaborator

I posted a quick message to the django-users mailing list to see if anyone there had feedback before escalating to the Django issue tracker: https://groups.google.com/forum/?fromgroups=#!topic/django-users/UORVadDO61w

@bradjasper
Copy link
Collaborator

I've posted this to the Django bug tracker: https://code.djangoproject.com/ticket/20090#ticket

@jrief
Copy link
Contributor Author

jrief commented Mar 20, 2013

Thanks, I'll watch this ticket.

@bradjasper
Copy link
Collaborator

I just pushed a commit that should fix this issue once and for all. I'd appreciate some extra eyes on it if anyone has some time: bf0a412

Thanks for all the help @jrief @fletom @lmeunier –– please let me know if you see anything else here that needs to be addressed.

@lmeunier
Copy link

@bradjasper Many thanks for the fix. From my point of view, all is ok now.

@dogada
Copy link

dogada commented Feb 5, 2015

Strings again can't be saved with JSONField. With Django 1.7.4 and django-jsonfield 0.9.13 I get:

>>> e1.value = unicode("xyz")
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/fields/subclassing.py", line 37, in __set__
    obj.__dict__[self.field.name] = self.field.to_python(value)
  File "/usr/local/lib/python2.7/dist-packages/jsonfield/fields.py", line 101, in to_python
    raise ValidationError(msg)
ValidationError: [u"'xyz' is not a valid JSON string."]

@pedroteixeira
Copy link

👍 didn't work for me either

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants