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

Object mismatch with pymongo FixedOffset #117

Closed
jaraco opened this Issue Mar 1, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@jaraco
Contributor

jaraco commented Mar 1, 2015

I've discovered that the round-trip serialization/deserialization of a simple FixedOffset object produces a corrupted object. The output of this test script produces:

============================= test session starts =============================
platform win32 -- Python 3.4.2 -- py-1.4.26 -- pytest-2.6.4
collected 1 items

test_fo.py F

================================== FAILURES ===================================
________________________ test_FixedOffsetSerializable _________________________

    def test_FixedOffsetSerializable():
        import jsonpickle
        import bson.tz_util
        fo = bson.tz_util.FixedOffset(-60*5, 'EST')
        serialized = jsonpickle.dumps(fo)
        pprint.pprint(json.loads(serialized))
        restored = jsonpickle.loads(serialized)
        print(restored._FixedOffset__offset)
>       assert restored == fo
E    assert <bson.tz_util.FixedOffset object at 0x0000000005F494A8> == <bson.tz_util.FixedOffset object at 0x0000000005EEFA90>

test_fo.py:16: AssertionError
---------------------------- Captured stdout call -----------------------------
{'_FixedOffset__name': 'EST',
 '_FixedOffset__offset': {'py/id': 1},
 'py/initargs': {'py/tuple': [{'__reduce__': [{'py/type': 'datetime.timedelta'},
                                              {'py/tuple': [-1, 68400, 0]}],
                               'py/object': 'datetime.timedelta'},
                              'EST']},
 'py/object': 'bson.tz_util.FixedOffset'}
<bson.tz_util.FixedOffset object at 0x0000000005F494A8>
========================== 1 failed in 0.52 seconds ===========================

As you can see, the object produced isn't equal to the one serialized. Furthermore, it appears as if the .__offset property of the object has been set to itself instead of the timedelta object.

@marcintustin

This comment has been minimized.

Show comment
Hide comment
@marcintustin

marcintustin Mar 1, 2015

Contributor

Does it roundtrip correctly with pickle? I wonder if there are two problems here - one with the pickling machinery they provide, and one with ours.

Contributor

marcintustin commented Mar 1, 2015

Does it roundtrip correctly with pickle? I wonder if there are two problems here - one with the pickling machinery they provide, and one with ours.

@jaraco

This comment has been minimized.

Show comment
Hide comment
@jaraco

jaraco Mar 1, 2015

Contributor

The roundtrip does work correctly with stdlib pickle. Here's rev 3 of the test script where the first test still fails, but the other two succeed.

Contributor

jaraco commented Mar 1, 2015

The roundtrip does work correctly with stdlib pickle. Here's rev 3 of the test script where the first test still fails, but the other two succeed.

@jaraco

This comment has been minimized.

Show comment
Hide comment
@jaraco

jaraco Mar 1, 2015

Contributor

I've stepped through the flattening code, and I believe the serialized version is correct (the {'py/id': 1} is the correct reference as the FixedOffset object is the first object serialized (index 0) and the timedelta is the second object serialized (index 1).

Contributor

jaraco commented Mar 1, 2015

I've stepped through the flattening code, and I believe the serialized version is correct (the {'py/id': 1} is the correct reference as the FixedOffset object is the first object serialized (index 0) and the timedelta is the second object serialized (index 1).

@jaraco

This comment has been minimized.

Show comment
Hide comment
@jaraco

jaraco Mar 1, 2015

Contributor

The issue appears to be in Unpickler._swapref. Here's my pdb session during the unpickling during the test:

> c:\users\jaraco\p\jsonpickle\jsonpickle\unpickler.py(288)_restore_object_instance()
-> self._swapref(proxy, instance)
(Pdb) self._objs
[<jsonpickle.unpickler._Proxy object at 0x000000000686AC88>, datetime.timedelta(-1, 68400)]
(Pdb) n
> c:\users\jaraco\p\jsonpickle\jsonpickle\unpickler.py(290)_restore_object_instance()
-> if isinstance(instance, tuple):
(Pdb) self._objs
[<jsonpickle.unpickler._Proxy object at 0x000000000686AC88>, <bson.tz_util.FixedOffset object at 0x0000000006841320>]

Instead of replacing the Proxy reference, _swapref appears to be replacing the timedelta instance with the new FixedOffset instance.

Contributor

jaraco commented Mar 1, 2015

The issue appears to be in Unpickler._swapref. Here's my pdb session during the unpickling during the test:

> c:\users\jaraco\p\jsonpickle\jsonpickle\unpickler.py(288)_restore_object_instance()
-> self._swapref(proxy, instance)
(Pdb) self._objs
[<jsonpickle.unpickler._Proxy object at 0x000000000686AC88>, datetime.timedelta(-1, 68400)]
(Pdb) n
> c:\users\jaraco\p\jsonpickle\jsonpickle\unpickler.py(290)_restore_object_instance()
-> if isinstance(instance, tuple):
(Pdb) self._objs
[<jsonpickle.unpickler._Proxy object at 0x000000000686AC88>, <bson.tz_util.FixedOffset object at 0x0000000006841320>]

Instead of replacing the Proxy reference, _swapref appears to be replacing the timedelta instance with the new FixedOffset instance.

@jaraco

This comment has been minimized.

Show comment
Hide comment
@jaraco

jaraco Mar 1, 2015

Contributor

I've figured out the issue and I'm writing a test now to capture the failure in jsonpickle.

Contributor

jaraco commented Mar 1, 2015

I've figured out the issue and I'm writing a test now to capture the failure in jsonpickle.

jaraco added a commit to jaraco/jsonpickle that referenced this issue Mar 1, 2015

This was referenced Mar 1, 2015

@davvid davvid closed this in d7f84a2 Mar 2, 2015

davvid added a commit that referenced this issue Mar 2, 2015

Merge pull request #118 from jaraco/issue-117
unpickler: properly handle referencing timedelta objects with fixed offsets

Closes #117
Signed-off-by: David Aguilar <davvid@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment