diff --git a/docs/changelog.rst b/docs/changelog.rst index 65e236ba..dfa9c7a6 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,6 +21,7 @@ Changes in 0.9.X - DEV - Fix for issue where FileField deletion did not free space in GridFS. - No_dereference() not respected on embedded docs containing reference. #517 - Document save raise an exception if save_condition fails #1005 +- Fixes some internal _id handling issue. #961 Changes in 0.9.0 ================ diff --git a/mongoengine/base/document.py b/mongoengine/base/document.py index 5707992a..7142a3de 100644 --- a/mongoengine/base/document.py +++ b/mongoengine/base/document.py @@ -184,7 +184,7 @@ class BaseDocument(object): self__initialised = False # Check if the user has created a new instance of a class if (self._is_document and self__initialised - and self__created and name == self._meta['id_field']): + and self__created and name == self._meta.get('id_field')): super(BaseDocument, self).__setattr__('_created', False) super(BaseDocument, self).__setattr__(name, value) diff --git a/mongoengine/base/metaclasses.py b/mongoengine/base/metaclasses.py index 8a25ff3d..16e6bd29 100644 --- a/mongoengine/base/metaclasses.py +++ b/mongoengine/base/metaclasses.py @@ -385,15 +385,17 @@ class TopLevelDocumentMetaclass(DocumentMetaclass): new_class._auto_id_field = getattr(parent_doc_cls, '_auto_id_field', False) if not new_class._meta.get('id_field'): + # After 0.10, find not existing names, instead of overwriting + id_name, id_db_name = cls.get_auto_id_names(new_class) new_class._auto_id_field = True - new_class._meta['id_field'] = 'id' - new_class._fields['id'] = ObjectIdField(db_field='_id') - new_class._fields['id'].name = 'id' - new_class.id = new_class._fields['id'] - - # Prepend id field to _fields_ordered - if 'id' in new_class._fields and 'id' not in new_class._fields_ordered: - new_class._fields_ordered = ('id', ) + new_class._fields_ordered + new_class._meta['id_field'] = id_name + new_class._fields[id_name] = ObjectIdField(db_field=id_db_name) + new_class._fields[id_name].name = id_name + new_class.id = new_class._fields[id_name] + new_class._db_field_map[id_name] = id_db_name + new_class._reverse_db_field_map[id_db_name] = id_name + # Prepend id field to _fields_ordered + new_class._fields_ordered = (id_name, ) + new_class._fields_ordered # Merge in exceptions with parent hierarchy exceptions_to_merge = (DoesNotExist, MultipleObjectsReturned) @@ -408,6 +410,20 @@ class TopLevelDocumentMetaclass(DocumentMetaclass): return new_class + def get_auto_id_names(self): + id_name, id_db_name = ('id', '_id') + if id_name not in self._fields and \ + id_db_name not in (v.db_field for v in self._fields.values()): + return id_name, id_db_name + id_basename, id_db_basename, i = 'auto_id', '_auto_id', 0 + while id_name in self._fields or \ + id_db_name in (v.db_field for v in self._fields.values()): + id_name = '{0}_{1}'.format(id_basename, i) + id_db_name = '{0}_{1}'.format(id_db_basename, i) + i += 1 + return id_name, id_db_name + + class MetaDict(dict): diff --git a/mongoengine/document.py b/mongoengine/document.py index eedd01d2..7b05ef8c 100644 --- a/mongoengine/document.py +++ b/mongoengine/document.py @@ -152,6 +152,8 @@ class Document(BaseDocument): """ def fget(self): + if 'id_field' not in self._meta: + return None return getattr(self, self._meta['id_field']) def fset(self, value): diff --git a/tests/document/inheritance.py b/tests/document/inheritance.py index e8347054..7673a103 100644 --- a/tests/document/inheritance.py +++ b/tests/document/inheritance.py @@ -307,6 +307,69 @@ class InheritanceTest(unittest.TestCase): doc = Animal(name='dog') self.assertFalse('_cls' in doc.to_mongo()) + def test_abstract_handle_ids_in_metaclass_properly(self): + + class City(Document): + continent = StringField() + meta = {'abstract': True, + 'allow_inheritance': False} + + class EuropeanCity(City): + name = StringField() + + berlin = EuropeanCity(name='Berlin', continent='Europe') + self.assertEqual(len(berlin._db_field_map), len(berlin._fields_ordered)) + self.assertEqual(len(berlin._reverse_db_field_map), len(berlin._fields_ordered)) + self.assertEqual(len(berlin._fields_ordered), 3) + self.assertEqual(berlin._fields_ordered[0], 'id') + + def test_auto_id_not_set_if_specific_in_parent_class(self): + + class City(Document): + continent = StringField() + city_id = IntField(primary_key=True) + meta = {'abstract': True, + 'allow_inheritance': False} + + class EuropeanCity(City): + name = StringField() + + berlin = EuropeanCity(name='Berlin', continent='Europe') + self.assertEqual(len(berlin._db_field_map), len(berlin._fields_ordered)) + self.assertEqual(len(berlin._reverse_db_field_map), len(berlin._fields_ordered)) + self.assertEqual(len(berlin._fields_ordered), 3) + self.assertEqual(berlin._fields_ordered[0], 'city_id') + + def test_auto_id_vs_non_pk_id_field(self): + + class City(Document): + continent = StringField() + id = IntField() + meta = {'abstract': True, + 'allow_inheritance': False} + + class EuropeanCity(City): + name = StringField() + + berlin = EuropeanCity(name='Berlin', continent='Europe') + self.assertEqual(len(berlin._db_field_map), len(berlin._fields_ordered)) + self.assertEqual(len(berlin._reverse_db_field_map), len(berlin._fields_ordered)) + self.assertEqual(len(berlin._fields_ordered), 4) + self.assertEqual(berlin._fields_ordered[0], 'auto_id_0') + berlin.save() + self.assertEqual(berlin.pk, berlin.auto_id_0) + + def test_abstract_document_creation_does_not_fail(self): + + class City(Document): + continent = StringField() + meta = {'abstract': True, + 'allow_inheritance': False} + bkk = City(continent='asia') + self.assertEqual(None, bkk.pk) + # TODO: expected error? Shouldn't we create a new error type? + self.assertRaises(KeyError, lambda: setattr(bkk, 'pk', 1)) + def test_allow_inheritance_embedded_document(self): """Ensure embedded documents respect inheritance """ diff --git a/tests/queryset/queryset.py b/tests/queryset/queryset.py index 0965e40f..83fb52df 100644 --- a/tests/queryset/queryset.py +++ b/tests/queryset/queryset.py @@ -3689,11 +3689,9 @@ class QuerySetTest(unittest.TestCase): def test_scalar(self): class Organization(Document): - id = ObjectIdField('_id') name = StringField() class User(Document): - id = ObjectIdField('_id') name = StringField() organization = ObjectIdField()