cloudant / python-cloudant

A Python library for Cloudant and CouchDB
Apache License 2.0
163 stars 55 forks source link

dict methods (update, clear, pop*) breaks `_id` <-> '_document_id' internal logic #430

Closed aogier closed 5 years ago

aogier commented 5 years ago

Bug Description

Not overriden dict methods are breaking Document class logic in regards to keep _id key in sync with _document_id class property. As this breaks my work I'm currently implementing a fix and to me the most sensible way is to completely remove this feature, using _id property when needed.
If I correctly understand the code, this is only used in cloudant.document module and that logic seems completely removable. What do you think? TIA

1. Steps to reproduce and the simplest code sample possible to demonstrate the issue

In this example:

import cloudant
from uuid import uuid4

client = cloudant.CouchDB('user', 'pass',
                          url='http://localhost:5984', connect=True)

db = client.create_database('db-' + str(uuid4()))
db.delete()
db.create()

doc = db.create_document({'_id': 'julia006'})
doc['_id'] = 'julia007'
doc.save()
print('_id:')
print('doc:', doc)
print(doc.document_url)

doc = db.create_document({'_id': 'julia008'})
doc.update({'_id': 'julia009'})
doc.save()
print('update:')
print('doc:', doc)
print(doc.document_url)

# db.delete()

produce this output:

_id:
doc: {'_id': 'julia007', '_rev': '1-967a00dff5e02add41819138abb3284d'}
http://localhost:5984/db-28328934-87a6-4434-ab96-f8cb54351e2d/julia007
update:
doc: {'_id': 'julia009', '_rev': '2-7051cbe5c8faecd085a3fa619e6e6337'}
http://localhost:5984/db-28328934-87a6-4434-ab96-f8cb54351e2d/julia008

As you can see, using __setitem__ correctly produce a new document with an url coherent with doc._id. Using update instead make a new revision on an doc._id mismatching url.

2. What you expected to happen

Every dict updating methods (so also pop, popitem, clear, setdefault) should work the same way as setitem/delitem dunders.

3. What actually happened

Only setitem/delitem keeps ids in sync

aogier commented 5 years ago

Removing _document_id logic and staying DRY on _id works, tests passes and now every dict manipulation is inherently supported. I'd like to know what do you think! :)