sort keys when serializing objects to JSON

[nmarley]

Currently the add_json method doesn't sort keys before serializing a Python object to JSON.

Example script:

import ipfsapi
import json
from pprint import pprint


api = ipfsapi.connect('127.0.0.1', 5001)
data = """{
  "Action": 0,
  "Type": 0,
  "Name": "hiya",
  "PubKey": "tprv8ZgxMBicQKsPd1Cv3T..."
}"""
print(data)

data = json.loads(data)

ipfs_object_hash = api.add_json(data)
print("ipfs_object_hash: %s" % ipfs_object_hash)

Script on node1:

$ ./venv/bin/python t1.py
{
  "Action": 0,
  "Type": 0,
  "Name": "hiya",
  "PubKey": "tprv8ZgxMBicQKsPd1Cv3T..."
}
ipfs_object_hash: QmarCcvguM1tsyxboHKtRD8J3RA9446HKg2TnLPhevMiQd

Same script on node2:

$ ./venv/bin/python t1.py
{
  "Action": 0,
  "Type": 0,
  "Name": "hiya",
  "PubKey": "tprv8ZgxMBicQKsPd1Cv3T..."
}
ipfs_object_hash: QmPEpZ1C9jghELZh9pFAyamyd6pBXum7JBd79QasLoGp5E

Even though the same content was added via add_json, the IPFS hashes are different b/c the dict keys weren't sorted when serialized to JSON (note the keys are moved around):

/usr/local/bin/ipfs cat QmarCcvguM1tsyxboHKtRD8J3RA9446HKg2TnLPhevMiQd
{"Action": 0, "Type": 0, "Name": "hiya", "PubKey": "tprv8ZgxMBicQKsPd1Cv3T..."}

/usr/local/bin/ipfs cat QmPEpZ1C9jghELZh9pFAyamyd6pBXum7JBd79QasLoGp5E
{"Type": 0, "Name": "hiya", "PubKey": "tprv8ZgxMBicQKsPd1Cv3T...", "Action": 0}

[whereswaldon]

@nmarley Thanks so much for adding this functionality! We'd love to merge it, but could you write a quick test for this first?

[nmarley]

Yep, sure! Build failed but it looks like an error w/IPFS that might be resolved if the build is restarted. Otherwise not sure how to proceed.

[nmarley]

I don't have permission in Travis-CI to restart the build.

[whereswaldon]

I also don't have permission to restart the build, but I think it's failing due to a broken test. Could you try to replicate this error by running the tests with python 3.5 on your machine?

=================================== FAILURES ===================================
___________________ IpfsApiTest.test_add_multiple_from_list ____________________
self = <tests.IpfsApiTest testMethod=test_add_multiple_from_list>
    def test_add_multiple_from_list(self):
        res = self.api.add([self.fake_file, self.fake_file2])
>       self.assertEqual(self.fake_files_res, res)
E       AssertionError: [{'Hash': 'QmQcCtMgLVwvMQGu6mvsRYLjwqrZJc[113 chars]iu'}] != {'Hash': 'QmQcCtMgLVwvMQGu6mvsRYLjwqrZJcY[30 chars]fgh'}
test/functional/tests.py:189: AssertionError
===================== 1 failed, 99 passed in 9.38 seconds ======================
ERROR: InvocationError: '/home/travis/build/ipfs/py-ipfs-api/.tox/py35/bin/py.test --verbose --cov-config=.coveragerc-py35 --cov=ipfsapi --cov-report html'
___________________________________ summary ____________________________________
ERROR:   py35: commands failed

[whereswaldon]

@nmarley I was mistaken about not having permission; I was looking on the wrong UI for it. I've restarted the build. Hopefully this will confirm whether the problem is a test or with Travis.

[whereswaldon]

Okay, everything's good here. Thanks @nmarley for your work!

[whereswaldon]

@Alexander255 I have some concerns about the seemingly nondeterministic behavior of IPFS here. Any idea what might have made this thing hash differently sometimes (which seems the be the cause of the test failure)?

[nmarley]

Awesome, thanks!

[ntninja]

@nmarley After looking at the unit test I think we should probably use the most compact + reproducible JSON representation available. Doing so should maximize our chances of the resulting hash being identical across different implementations (and also saves some minor space).
TL;DR: Can you change the json.dumps line to json.dumps(…, sort_keys=True, indent=None, separators=(',', ':')) which is the most compact JSON representation according to the Python docs. The unit test will have to be adapted to match '{"Action":"Open","Name":"IPFS","Pubkey":7,"Type":"PR"}' instead.

Thanks for spotting this in the first place, btw! 🙂

@whereswaldon: The CI failure is actually correct and due to a known issue: #85 („Sometimes not all files passed to add() function are added”). We could solve this by getting that issue fixed. 😝