Change type signature of refine_integrity_error

We were using a mapping (ie: dict) from the matching rule to the refined exception, it turned out that this was a pain to correctly type. Seeing as we don't actually need a dict, we've changed this to a sequence of 2-tuples, where the first element is the matching rule, and the second is the exception. Co-authored-by: Sam Searles-Bryant <[email protected]>
kraken-tech · May 9, 2024 · e3447df · e3447df
1 parent 50482ae
commit e3447df
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 61 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file.
 
 ## Unreleased changes
 
+- Change type signature of `django_integrity.conversion.refine_integrity_error`.
+  Instead of accepting `Mapping[_Rule, Exception]`, it now accepts `Sequence[tuple[_Rule, Exception | type[Exception]]`.
+  This prevents issues with typing, and removes the need for `_Rule` to be hashable.
 - Fix some more incorrect type signatures:
     - `django_integrity.conversion.Unique.fields` was erroneously `tuple[str]` instead of `tuple[str, ...]`.
 - Protect against previously-unhandled potential `None` in errors from Psycopg.

diff --git a/mypy-ratchet.json b/mypy-ratchet.json
@@ -1,9 +1 @@
-{
-  "tests/django_integrity/test_conversion.py": {
-    "Argument 1 to \"refine_integrity_error\" has incompatible type \"dict[ForeignKey, type[SimpleError]]\"; expected \"Mapping[_Rule, Exception]\"  [arg-type]": 3,
-    "Argument 1 to \"refine_integrity_error\" has incompatible type \"dict[Named, type[SimpleError]]\"; expected \"Mapping[_Rule, Exception]\"  [arg-type]": 2,
-    "Argument 1 to \"refine_integrity_error\" has incompatible type \"dict[NotNull, type[SimpleError]]\"; expected \"Mapping[_Rule, Exception]\"  [arg-type]": 3,
-    "Argument 1 to \"refine_integrity_error\" has incompatible type \"dict[PrimaryKey, type[SimpleError]]\"; expected \"Mapping[_Rule, Exception]\"  [arg-type]": 2,
-    "Argument 1 to \"refine_integrity_error\" has incompatible type \"dict[Unique, type[SimpleError]]\"; expected \"Mapping[_Rule, Exception]\"  [arg-type]": 3
-  }
-}
+{}
diff --git a/src/django_integrity/conversion.py b/src/django_integrity/conversion.py
@@ -4,7 +4,7 @@
 import contextlib
 import dataclasses
 import re
-from collections.abc import Iterator, Mapping
+from collections.abc import Iterator, Sequence
 
 from django import db as django_db
 
@@ -16,23 +16,26 @@
 
 
 @contextlib.contextmanager
-def refine_integrity_error(rules: Mapping[_Rule, Exception]) -> Iterator[None]:
+def refine_integrity_error(
+    rules: Sequence[tuple[_Rule, Exception | type[Exception]]],
+) -> Iterator[None]:
     """
     Convert a generic IntegrityError into a more specific exception.
 
-    The conversion is based on a mapping of rules to exceptions.
+    The conversion is based on (rule, exception) pairs.
 
     Args:
-        rules: A mapping of rules to the exceptions we'll raise if they match.
+        rules: A sequence of rule, exception pairs.
+            If the rule matches the IntegrityError, the exception is raised.
 
     Raises:
-        An error from the rules mapping if an IntegrityError matches a rule.
+        The exception paired with the first matching rule.
         Otherwise, the original IntegrityError.
     """
     try:
         yield
     except django_db.IntegrityError as e:
-        for rule, refined_error in rules.items():
+        for rule, refined_error in rules:
             if rule.is_match(e):
                 raise refined_error from e
         raise

diff --git a/tests/django_integrity/test_conversion.py b/tests/django_integrity/test_conversion.py
@@ -12,7 +12,7 @@ class SimpleError(Exception):
 class TestRefineIntegrityError:
     def test_no_rules(self) -> None:
         # It is legal to call the context manager without any rules.
-        with conversion.refine_integrity_error(rules={}):
+        with conversion.refine_integrity_error(rules=()):
             pass
 
 
@@ -22,7 +22,7 @@ def test_error_refined(self) -> None:
         # Create a unique instance so that we can violate the constraint later.
         test_models.UniqueModel.objects.create(unique_field=42)
 
-        rules = {conversion.Named(name="unique_model_unique_field_key"): SimpleError}
+        rules = ((conversion.Named(name="unique_model_unique_field_key"), SimpleError),)
 
         # The original error should be transformed into our expected error.
         with pytest.raises(SimpleError):
@@ -34,7 +34,7 @@ def test_rules_mismatch(self) -> None:
         test_models.UniqueModel.objects.create(unique_field=42)
 
         # No constraints match the error:
-        rules = {conversion.Named(name="nonexistent_constraint"): SimpleError}
+        rules = ((conversion.Named(name="nonexistent_constraint"), SimpleError),)
 
         # The original error should be raised.
         with pytest.raises(django_db.IntegrityError):
@@ -48,11 +48,14 @@ def test_error_refined(self) -> None:
         # Create a unique instance so that we can violate the constraint later.
         test_models.UniqueModel.objects.create(unique_field=42)
 
-        rules = {
-            conversion.Unique(
-                model=test_models.UniqueModel, fields=("unique_field",)
-            ): SimpleError
-        }
+        rules = (
+            (
+                conversion.Unique(
+                    model=test_models.UniqueModel, fields=("unique_field",)
+                ),
+                SimpleError,
+            ),
+        )
 
         # The original error should be transformed into our expected error.
         with pytest.raises(SimpleError):
@@ -63,11 +66,14 @@ def test_multiple_fields(self) -> None:
         # Create a unique instance so that we can violate the constraint later.
         test_models.UniqueTogetherModel.objects.create(field_1=1, field_2=2)
 
-        rules = {
-            conversion.Unique(
-                model=test_models.UniqueTogetherModel, fields=("field_1", "field_2")
-            ): SimpleError
-        }
+        rules = (
+            (
+                conversion.Unique(
+                    model=test_models.UniqueTogetherModel, fields=("field_1", "field_2")
+                ),
+                SimpleError,
+            ),
+        )
 
         # The original error should be transformed into our expected error.
         with pytest.raises(SimpleError):
@@ -99,7 +105,7 @@ def test_rules_mismatch(
         # Create a unique instance so that we can violate the constraint later.
         test_models.UniqueModel.objects.create(unique_field=42)
 
-        rules = {conversion.Unique(model=Model, fields=(field,)): SimpleError}
+        rules = ((conversion.Unique(model=Model, fields=(field,)), SimpleError),)
 
         # We shouldn't transform the error, because it didn't match the rule.
         with pytest.raises(django_db.IntegrityError):
@@ -132,7 +138,7 @@ def test_error_refined(
         # Create a unique instance so that we can violate the constraint later.
         existing_primary_key = ModelClass.objects.create().pk
 
-        rules = {conversion.PrimaryKey(model=ModelClass): SimpleError}
+        rules = ((conversion.PrimaryKey(model=ModelClass), SimpleError),)
 
         # The original error should be transformed into our expected error.
         with pytest.raises(SimpleError):
@@ -144,7 +150,7 @@ def test_rules_mismatch(self) -> None:
         existing_primary_key = test_models.PrimaryKeyModel.objects.create().pk
 
         # A similar rule, but for a different model with the same field name..
-        rules = {conversion.PrimaryKey(model=test_models.UniqueModel): SimpleError}
+        rules = ((conversion.PrimaryKey(model=test_models.UniqueModel), SimpleError),)
 
         # The original error should be raised.
         with pytest.raises(django_db.IntegrityError):
@@ -165,11 +171,12 @@ def test_model_without_primary_key(self) -> None:
 @pytest.mark.django_db
 class TestNotNull:
     def test_error_refined(self) -> None:
-        rules = {
-            conversion.NotNull(
-                model=test_models.UniqueModel, field="unique_field"
-            ): SimpleError
-        }
+        rules = (
+            (
+                conversion.NotNull(model=test_models.UniqueModel, field="unique_field"),
+                SimpleError,
+            ),
+        )
 
         # The original error should be transformed into our expected error.
         with pytest.raises(SimpleError):
@@ -179,11 +186,14 @@ def test_error_refined(self) -> None:
 
     def test_model_mismatch(self) -> None:
         # Same field, but different model.
-        rules = {
-            conversion.NotNull(
-                model=test_models.AlternativeUniqueModel, field="unique_field"
-            ): SimpleError
-        }
+        rules = (
+            (
+                conversion.NotNull(
+                    model=test_models.AlternativeUniqueModel, field="unique_field"
+                ),
+                SimpleError,
+            ),
+        )
 
         with pytest.raises(django_db.IntegrityError):
             with conversion.refine_integrity_error(rules):
@@ -192,11 +202,14 @@ def test_model_mismatch(self) -> None:
 
     def test_field_mismatch(self) -> None:
         # Same model, but different field.
-        rules = {
-            conversion.NotNull(
-                model=test_models.AlternativeUniqueModel, field="unique_field_2"
-            ): SimpleError
-        }
+        rules = (
+            (
+                conversion.NotNull(
+                    model=test_models.AlternativeUniqueModel, field="unique_field_2"
+                ),
+                SimpleError,
+            ),
+        )
 
         # The original error should be raised.
         with pytest.raises(django_db.IntegrityError):
@@ -211,11 +224,14 @@ def test_field_mismatch(self) -> None:
 @pytest.mark.django_db
 class TestForeignKey:
     def test_error_refined(self) -> None:
-        rules = {
-            conversion.ForeignKey(
-                model=test_models.ForeignKeyModel, field="related_id"
-            ): SimpleError
-        }
+        rules = (
+            (
+                conversion.ForeignKey(
+                    model=test_models.ForeignKeyModel, field="related_id"
+                ),
+                SimpleError,
+            ),
+        )
         constraints.set_all_immediate(using="default")
 
         # The original error should be transformed into our expected error.
@@ -226,11 +242,14 @@ def test_error_refined(self) -> None:
 
     def test_source_mismatch(self) -> None:
         # The field name matches, but the source model is different.
-        rules = {
-            conversion.ForeignKey(
-                model=test_models.ForeignKeyModel2, field="related_id"
-            ): SimpleError
-        }
+        rules = (
+            (
+                conversion.ForeignKey(
+                    model=test_models.ForeignKeyModel2, field="related_id"
+                ),
+                SimpleError,
+            ),
+        )
         constraints.set_all_immediate(using="default")
 
         with pytest.raises(django_db.IntegrityError):
@@ -239,11 +258,14 @@ def test_source_mismatch(self) -> None:
 
     def test_field_mismatch(self) -> None:
         # The source model matches, but the field name is different.
-        rules = {
-            conversion.ForeignKey(
-                model=test_models.ForeignKeyModel3, field="related_2_id"
-            ): SimpleError
-        }
+        rules = (
+            (
+                conversion.ForeignKey(
+                    model=test_models.ForeignKeyModel3, field="related_2_id"
+                ),
+                SimpleError,
+            ),
+        )
         constraints.set_all_immediate(using="default")
 
         with pytest.raises(django_db.IntegrityError):