add clashing field name validation

Author: timgraham

django_mongodb_backend/fields/polymorphic_embedded_model.py

@@ -3,7 +3,7 @@
 
 from django.core import checks
 from django.core.exceptions import FieldDoesNotExist, ValidationError
-from django.db import models
+from django.db import connections, models
 from django.db.models.fields.related import lazy_related_operation
 from django.db.models.lookups import Transform
 
@@ -30,6 +30,7 @@ def check(self, **kwargs):
         from ..models import EmbeddedModel
 
         errors = super().check(**kwargs)
+        embedded_fields = {}
         for model in self.embedded_models:
             if not issubclass(model, EmbeddedModel):
                 return [
@@ -52,6 +53,23 @@ def check(self, **kwargs):
                             id="django_mongodb_backend.embedded_model.E001",
                         )
                     )
+                field_name = field.name
+                if existing_field := embedded_fields.get(field.name):
+                    connection = _get_mongodb_connection()
+                    if existing_field.db_type(connection) != field.db_type(connection):
+                        errors.append(
+                            checks.Warning(
+                                f"Embedded models {existing_field.model._meta.label} "
+                                f"and {field.model._meta.label} both have field "
+                                f"'{field_name}' of different type.",
+                                obj=self,
+                                id="django_mongodb_backend.embedded_model.E003",
+                                hint="It may be impossible to query both fields.",
+                            )
+                        )
+
+                else:
+                    embedded_fields[field_name] = field
         return errors
 
     def deconstruct(self):
@@ -219,3 +237,10 @@ def __init__(self, key_name, ref_field):
 
     def __call__(self, *args, **kwargs):
         return KeyTransform(self.key_name, self.ref_field, *args, **kwargs)
+
+
+def _get_mongodb_connection():
+    for alias in connections:
+        if connections[alias].vendor == "mongodb":
+            return connections[alias]
+    return None

docs/source/ref/models/fields.rst

@@ -321,7 +321,7 @@ These indexes use 0-based indexing.
 
     .. versionadded:: 5.2.0b2
 
-    Stores a model of type ``embedded_models``.
+    Stores a model of one of the types in ``embedded_models``.
 
     .. attribute:: embedded_models
 
@@ -337,7 +337,8 @@ These indexes use 0-based indexing.
 .. admonition:: Migrations support is limited
 
     :djadmin:`makemigrations` does not yet detect changes to embedded models,
-    nor does it create indexes or constraints for embedded models.
+    nor does it create indexes or constraints for embedded models referenced
+    by ``PolymorphicEmbeddedModelField``.
 
 .. admonition:: Forms are not supported
 

docs/source/topics/embedded-models.rst

@@ -246,8 +246,8 @@ Represented in BSON, the person structures looks like this:
       pet: { name: 'Pheobe', purrs: true, _label: 'myapp.Cat' }
     }
 
-The ``_label`` field contains the model's
-:attr:`~django.db.models.Options.label`.
+The ``_label`` field tracks the model's :attr:`~django.db.models.Options.label`
+so that the model can be initialized properly.
 
 Querying ``PolymorphicEmbeddedModelField``
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -262,3 +262,31 @@ You can also filter on fields that aren't shared among the embedded models. For
 example, if you filter on ``barks``, you'll only get back people with dogs::
 
     >>> Person.objects.filter(pet__barks=True)
+
+Clashing field names
+~~~~~~~~~~~~~~~~~~~~
+
+Be careful not to use embedded models with clashing field names of different
+types. For example::
+
+    from django.db import models
+
+    from django_mongodb_backend.fields import PolymorphicEmbeddedModelField
+    from django_mongodb_backend.models import EmbeddedModel
+
+    class Target1(EmbeddedModel):
+        number = models.IntegerField()
+
+    class Target2(EmbeddedModel):
+        number = models.DecimalField(max_digits=4, decimal_places=2)
+
+    class Example(models.Model):
+        target = PolymorphicEmbeddedModelField([Target1, Target2])
+
+In this case, it will be impossible to query the ``number`` field properly
+since Django won't know whether to prepare the lookup value as an integer or as
+a decimal.
+
+Additionally, querying into nested embedded model fields with the same name
+isn't well supported: the first model in ``embedded_models`` is the one that
+will be used for lookups.

tests/model_fields_/test_polymorphic_embedded_model.py

@@ -216,3 +216,41 @@ class MyModel(models.Model):
             msg,
             "Embedded models must be a subclass of django_mongodb_backend.models.EmbeddedModel.",
         )
+
+    def test_clashing_fields(self):
+        class Target1(EmbeddedModel):
+            clash = models.DecimalField(max_digits=4, decimal_places=2)
+
+        class Target2(EmbeddedModel):
+            clash = models.CharField(max_length=255)
+
+        class MyModel(models.Model):
+            field = PolymorphicEmbeddedModelField([Target1, Target2])
+
+        errors = MyModel().check()
+        self.assertEqual(len(errors), 1)
+        self.assertEqual(errors[0].id, "django_mongodb_backend.embedded_model.E003")
+        self.assertEqual(
+            errors[0].msg,
+            "Embedded models model_fields_.Target1 and model_fields_.Target2 "
+            "both have field 'clash' of different type.",
+        )
+        self.assertEqual(
+            errors[0].hint,
+            "It may be impossible to query both fields.",
+        )
+
+    def test_clashing_fields_of_same_type(self):
+        """Fields of different type don't clash if they use the same db_type."""
+
+        class Target1(EmbeddedModel):
+            clash = models.TextField()
+
+        class Target2(EmbeddedModel):
+            clash = models.CharField(max_length=255)
+
+        class MyModel(models.Model):
+            field = PolymorphicEmbeddedModelField([Target1, Target2])
+
+        errors = MyModel().check()
+        self.assertEqual(len(errors), 0)