diff --git a/docs/changelog.rst b/docs/changelog.rst index e82cc124..6a56325e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -6,6 +6,8 @@ Changelog Development =========== - (Fill this out as you fix issues and develop your features). +- BREAKING CHANGE: Drop support for positional arguments when instantiating a document. #? + - From now on keyword arguments (e.g. `Doc(field_name=value)`) are required. Changes in 0.18.1 ================= diff --git a/mongoengine/base/document.py b/mongoengine/base/document.py index 057258f5..047d50a4 100644 --- a/mongoengine/base/document.py +++ b/mongoengine/base/document.py @@ -60,18 +60,10 @@ class BaseDocument(object): self._created = True if args: - # Combine positional arguments with named arguments. - # We only want named arguments. - field = iter(self._fields_ordered) - # If its an automatic id field then skip to the first defined field - if getattr(self, '_auto_id_field', False): - next(field) - for value in args: - name = next(field) - if name in values: - raise TypeError( - 'Multiple values for keyword argument "%s"' % name) - values[name] = value + raise TypeError( + 'Instantiating a document with positional arguments is not ' + 'supported. Please use `field_name=value` keyword arguments.' + ) __auto_convert = values.pop('__auto_convert', True) diff --git a/mongoengine/base/metaclasses.py b/mongoengine/base/metaclasses.py index 6f507eaa..c5363b09 100644 --- a/mongoengine/base/metaclasses.py +++ b/mongoengine/base/metaclasses.py @@ -385,30 +385,35 @@ class TopLevelDocumentMetaclass(DocumentMetaclass): new_class._meta['id_field'] = field_name new_class.id = field - # Set primary key if not defined by the document - new_class._auto_id_field = getattr(parent_doc_cls, - '_auto_id_field', False) + # If the document doesn't explicitly define a primary key field, create + # one. Make it an ObjectIdField and give it a non-clashing name ("id" + # by default, but can be different if that one's taken). if not new_class._meta.get('id_field'): - # After 0.10, find not existing names, instead of overwriting id_name, id_db_name = mcs.get_auto_id_names(new_class) - new_class._auto_id_field = True 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 + + # Prepend the ID field to _fields_ordered (so that it's *always* + # the first field). new_class._fields_ordered = (id_name, ) + new_class._fields_ordered - # Merge in exceptions with parent hierarchy + # Merge in exceptions with parent hierarchy. exceptions_to_merge = (DoesNotExist, MultipleObjectsReturned) module = attrs.get('__module__') for exc in exceptions_to_merge: name = exc.__name__ - parents = tuple(getattr(base, name) for base in flattened_bases - if hasattr(base, name)) or (exc,) - # Create new exception and set to new_class + parents = tuple( + getattr(base, name) + for base in flattened_bases + if hasattr(base, name) + ) or (exc,) + + # Create a new exception and set it as an attribute on the new + # class. exception = type(name, parents, {'__module__': module}) setattr(new_class, name, exception) @@ -416,17 +421,35 @@ class TopLevelDocumentMetaclass(DocumentMetaclass): @classmethod def get_auto_id_names(mcs, new_class): + """Find a name for the automatic ID field for the given new class. + + Return a two-element tuple where the first item is the field name (i.e. + the attribute name on the object) and the second element is the DB + field name (i.e. the name of the key stored in MongoDB). + + Defaults to ('id', '_id'), or generates a non-clashing name in the form + of ('auto_id_X', '_auto_id_X') if the default name is already taken. + """ id_name, id_db_name = ('id', '_id') - if id_name not in new_class._fields and \ - id_db_name not in (v.db_field for v in new_class._fields.values()): + existing_fields = new_class._fields + existing_db_fields = (v.db_field for v in new_class._fields.values()) + if ( + id_name not in existing_fields and + id_db_name not in existing_db_fields + ): return id_name, id_db_name - id_basename, id_db_basename, i = 'auto_id', '_auto_id', 0 - while id_name in new_class._fields or \ - id_db_name in (v.db_field for v in new_class._fields.values()): + + id_basename, id_db_basename, i = ('auto_id', '_auto_id', 0) + while True: 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 + if ( + id_name not in existing_fields and + id_db_name not in existing_db_fields + ): + return id_name, id_db_name + else: + i += 1 class MetaDict(dict): diff --git a/tests/document/instance.py b/tests/document/instance.py index 02617b67..4be8aa45 100644 --- a/tests/document/instance.py +++ b/tests/document/instance.py @@ -3130,48 +3130,44 @@ class InstanceTest(MongoDBTestCase): self.assertEqual(classic_doc._data, dict_doc._data) def test_positional_creation(self): - """Ensure that document may be created using positional arguments.""" - person = self.Person("Test User", 42) - self.assertEqual(person.name, "Test User") - self.assertEqual(person.age, 42) + """Document cannot be instantiated using positional arguments.""" + with self.assertRaises(TypeError) as e: + person = self.Person("Test User", 42) + expected_msg = ( + 'Instantiating a document with positional arguments is not ' + 'supported. Please use `field_name=value` keyword arguments.' + ) + self.assertEqual(e.exception.message, expected_msg) def test_mixed_creation(self): - """Ensure that document may be created using mixed arguments.""" - person = self.Person("Test User", age=42) - self.assertEqual(person.name, "Test User") - self.assertEqual(person.age, 42) + """Document cannot be instantiated using mixed arguments.""" + with self.assertRaises(TypeError) as e: + person = self.Person("Test User", age=42) + expected_msg = ( + 'Instantiating a document with positional arguments is not ' + 'supported. Please use `field_name=value` keyword arguments.' + ) + self.assertEqual(e.exception.message, expected_msg) def test_positional_creation_embedded(self): - """Ensure that embedded document may be created using positional - arguments. - """ - job = self.Job("Test Job", 4) - self.assertEqual(job.name, "Test Job") - self.assertEqual(job.years, 4) + """Embedded document cannot be created using positional arguments.""" + with self.assertRaises(TypeError) as e: + job = self.Job("Test Job", 4) + expected_msg = ( + 'Instantiating a document with positional arguments is not ' + 'supported. Please use `field_name=value` keyword arguments.' + ) + self.assertEqual(e.exception.message, expected_msg) def test_mixed_creation_embedded(self): - """Ensure that embedded document may be created using mixed - arguments. - """ - job = self.Job("Test Job", years=4) - self.assertEqual(job.name, "Test Job") - self.assertEqual(job.years, 4) - - def test_mixed_creation_dynamic(self): - """Ensure that document may be created using mixed arguments.""" - class Person(DynamicDocument): - name = StringField() - - person = Person("Test User", age=42) - self.assertEqual(person.name, "Test User") - self.assertEqual(person.age, 42) - - def test_bad_mixed_creation(self): - """Ensure that document gives correct error when duplicating - arguments. - """ - with self.assertRaises(TypeError): - return self.Person("Test User", 42, name="Bad User") + """Embedded document cannot be created using mixed arguments.""" + with self.assertRaises(TypeError) as e: + job = self.Job("Test Job", years=4) + expected_msg = ( + 'Instantiating a document with positional arguments is not ' + 'supported. Please use `field_name=value` keyword arguments.' + ) + self.assertEqual(e.exception.message, expected_msg) def test_data_contains_id_field(self): """Ensure that asking for _data returns 'id'.""" diff --git a/tests/fields/test_lazy_reference_field.py b/tests/fields/test_lazy_reference_field.py index b10506e7..1d6e6e79 100644 --- a/tests/fields/test_lazy_reference_field.py +++ b/tests/fields/test_lazy_reference_field.py @@ -303,8 +303,8 @@ class TestLazyReferenceField(MongoDBTestCase): Animal.drop_collection() Ocurrence.drop_collection() - animal1 = Animal('doggo').save() - animal2 = Animal('cheeta').save() + animal1 = Animal(name='doggo').save() + animal2 = Animal(name='cheeta').save() def check_fields_type(occ): self.assertIsInstance(occ.direct, LazyReference) @@ -542,8 +542,8 @@ class TestGenericLazyReferenceField(MongoDBTestCase): Animal.drop_collection() Ocurrence.drop_collection() - animal1 = Animal('doggo').save() - animal2 = Animal('cheeta').save() + animal1 = Animal(name='doggo').save() + animal2 = Animal(name='cheeta').save() def check_fields_type(occ): self.assertIsInstance(occ.direct, LazyReference)