natural_key: fix get_by_natural_key_json for objects with partial unique indexes (fixes #23857)

Unicity on NULL column must be explicitely stated in the list of natural
keys.

src/authentic2/a2_rbac/models.py

@@ -115,7 +115,10 @@ class Permission(PermissionAbstractBase):
                                    object_id_field='admin_scope_id')
 
 
-Permission._meta.natural_key = ['operation', 'ou', 'target']
+Permission._meta.natural_key = [
+    ['operation', 'ou', 'target'],
+    ['operation', 'ou__isnull', 'target'],
+]
 
 
 class Role(RoleAbstractBase):
@@ -247,7 +250,13 @@ class Role(RoleAbstractBase):
 
 
 Role._meta.natural_key = [
-    ['uuid'], ['slug', 'ou'], ['name', 'ou'], ['slug', 'service'], ['name', 'service']
+    ['uuid'],
+    ['slug', 'ou__isnull', 'service__isnull'],
+    ['name', 'ou__isnull', 'service__isnull'],
+    ['slug', 'ou', 'service'],
+    ['name', 'ou', 'service'],
+    ['slug', 'ou', 'service__isnull'],
+    ['name', 'ou', 'service__isnull'],
 ]
 
 

src/authentic2/natural_key.py

@@ -24,6 +24,8 @@ def natural_key_json(self):
             names.add(key)
 
     for name in names:
+        if name.endswith('__isnull'):
+            name = name.split('__isnull')[0]
         field = self._meta.get_field(name)
         if not (field.concrete or isinstance(field, GenericForeignKey)):
             raise ValueError('field %s is not concrete' % name)
@@ -49,6 +51,9 @@ def get_by_natural_key_json(self, d):
     for natural_key in natural_keys:
         get_kwargs = {}
         for name in natural_key:
+            isnull = name.endswith('__isnull')
+            if isnull:
+                name = name.split('__isnull')[0]
             field = model._meta.get_field(name)
             if not (field.concrete or isinstance(field, GenericForeignKey)):
                 raise ValueError('field %s is not concrete' % name)
@@ -57,34 +62,44 @@ def get_by_natural_key_json(self, d):
             try:
                 value = d[name]
             except KeyError:
-                break
+                if not isnull:
+                    break
+                value = None
             if isinstance(field, GenericForeignKey):
-                try:
-                    ct_nk = d[field.ct_field]
-                except KeyError:
+                if isnull and value:
                     break
-                try:
-                    ct = ContentType.objects.get_by_natural_key_json(ct_nk)
-                except ContentType.DoesNotExist:
-                    break
-                related_model = ct.model_class()
-                try:
-                    value = related_model._default_manager.get_by_natural_key_json(value)
-                except related_model.DoesNotExist:
-                    break
-                get_kwargs[field.ct_field] = ct
-                name = field.fk_field
-                value = value.pk
+                elif not isnull:
+                    if not value:
+                        break
+                    try:
+                        ct_nk = d[field.ct_field]
+                    except KeyError:
+                        break
+                    try:
+                        ct = ContentType.objects.get_by_natural_key_json(ct_nk)
+                    except ContentType.DoesNotExist:
+                        break
+                    related_model = ct.model_class()
+                    try:
+                        value = related_model._default_manager.get_by_natural_key_json(value)
+                    except related_model.DoesNotExist:
+                        break
+                    name = field.fk_field
+                    value = value.pk
             elif field.is_relation:
-                if value is None:
-                    name = '%s__isnull' % name
-                    value = True
-                else:
+                if isnull and value:
+                    break
+                elif not isnull:
+                    if not value:
+                        break
                     try:
                         value = field.related_model._default_manager.get_by_natural_key_json(value)
                     except field.related_model.DoesNotExist:
                         break
-            get_kwargs[name] = value
+            if isnull:
+                get_kwargs[name + '__isnull'] = True
+            else:
+                get_kwargs[name] = value
         else:
             try:
                 return self.get(**get_kwargs)

tests/test_natural_key.py

@@ -1,3 +1,5 @@
+import pytest
+
 from django.contrib.contenttypes.models import ContentType
 from authentic2.a2_rbac.models import Role, OrganizationalUnit as OU, Permission
 
@@ -29,12 +31,20 @@ def test_natural_key_json(db, ou1):
         }
         assert role == Role.objects.get_by_natural_key_json(nk)
         assert role == Role.objects.get_by_natural_key_json({'uuid': role.uuid})
-        assert role == Role.objects.get_by_natural_key_json({'slug': role.slug, 'ou': ou_nk})
-        assert role == Role.objects.get_by_natural_key_json({'name': role.name, 'ou': ou_nk})
+        if service_nk:
+            with pytest.raises(Role.DoesNotExist):
+                Role.objects.get_by_natural_key_json({'slug': role.slug, 'ou': ou_nk})
+        else:
+            assert Role.objects.get_by_natural_key_json({'slug': role.slug, 'ou': ou_nk}) == role
+        if service_nk:
+            with pytest.raises(Role.DoesNotExist):
+                assert Role.objects.get_by_natural_key_json({'name': role.name, 'ou': ou_nk})
+        else:
+            assert Role.objects.get_by_natural_key_json({'name': role.name, 'ou': ou_nk}) == role
         assert role == Role.objects.get_by_natural_key_json(
-            {'slug': role.slug, 'service': service_nk})
+            {'slug': role.slug, 'ou': ou_nk, 'service': service_nk})
         assert role == Role.objects.get_by_natural_key_json(
-            {'name': role.name, 'service': service_nk})
+            {'name': role.name, 'ou': ou_nk, 'service': service_nk})
 
     for permission in Permission.objects.all():
         ou_nk = permission.ou and permission.ou.natural_key_json()