Implemented method to auto-generate non-collisioning auto_id names

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
 ================

docs/guide/defining-documents.rst

@@ -310,7 +310,7 @@ Dealing with deletion of referred documents
 By default, MongoDB doesn't check the integrity of your data, so deleting
 documents that other documents still hold references to will lead to consistency
 issues.  Mongoengine's :class:`ReferenceField` adds some functionality to
-safeguard against these kinds of database integrity problems, providing each
+safeguard against these kinds of database integrit2y problems, providing each
 reference with a delete rule specification.  A delete rule is specified by
 supplying the :attr:`reverse_delete_rule` attributes on the
 :class:`ReferenceField` definition, like this::

mongoengine/base/metaclasses.py

@@ -385,21 +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._meta['id_field'] = id_name
-            new_class._fields['id'] = ObjectIdField(db_field='_id')
+            new_class._fields[id_name] = ObjectIdField(db_field=id_db_name)
-            new_class._fields['id'].name = 'id'
+            new_class._fields[id_name].name = id_name
-            new_class.id = new_class._fields['id']
+            new_class.id = new_class._fields[id_name]
-            new_class._db_field_map['id'] = '_id'
+            new_class._db_field_map[id_name] = id_db_name
-            new_class._reverse_db_field_map['_id'] = 'id'
+            new_class._reverse_db_field_map[id_db_name] = id_name
-            if 'id' in new_class._fields_ordered:
+            # Prepend id field to _fields_ordered
-                # An existing id field will be overwritten anyway, so remove it
+            new_class._fields_ordered = (id_name, ) + new_class._fields_ordered
-                loc = new_class._fields_ordered.index('id')
-                new_class._fields_ordered = new_class._fields_ordered[:loc] + \
-                                            new_class._fields_ordered[loc+1:]
-            else:
-                # Prepend id field to _fields_ordered
-                new_class._fields_ordered = ('id', ) + new_class._fields_ordered
 
         # Merge in exceptions with parent hierarchy
         exceptions_to_merge = (DoesNotExist, MultipleObjectsReturned)
@@ -414,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 = '{}_{}'.format(id_basename, i)
+            id_db_name = '{}_{}'.format(id_db_basename, i)
+            i += 1
+        return id_name, id_db_name
+
+
 
 class MetaDict(dict):
 

tests/document/inheritance.py

@@ -316,14 +316,30 @@ class InheritanceTest(unittest.TestCase):
 
         class EuropeanCity(City):
             name = StringField()
-            country = 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(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):
@@ -334,13 +350,14 @@ class InheritanceTest(unittest.TestCase):
 
         class EuropeanCity(City):
             name = StringField()
-            country = 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), 5)
+        self.assertEqual(len(berlin._fields_ordered), 4)
-        self.assertEqual(berlin._fields_ordered[0], 'id')
+        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):
 
@@ -350,7 +367,7 @@ class InheritanceTest(unittest.TestCase):
                     'allow_inheritance': False}
         bkk = City(continent='asia')
         self.assertEqual(None, bkk.pk)
-        # TODO: expected error? Shouldn'twe created a new error type
+        # 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):

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()