ref(backup): Convert InstanceID to NamedTuple

This is a bit unfortunate, as this requires the `model` to be passed as
an (unparsed) `str`, rather than as an already-parsed
`NormalizedModelName`, ultimately making the class easier to misuse.
However, `InstanceID`s will now need to be sent over RPC, and no other
solution I tried for this that retained the type-unwrapping constructor
was satisfactory:

- I considered using Pydantic's `BaseModel` or `dataclass` base classes,
  but opted against this, as I would like to avoid Pydantic types
  leaking out of the RPC-specific modules they mostly live in today.
- I tried to convince Pydantic to properly encode/decode the existing
  native dataclass, but ran across pydantic/pydantic#2531,
  which won't allow custom encoding/decoding of arbitrary types in
  Pydantic v1.
- I also messed with some other inheritance schemes, but in the end
  decided converting to a `NamedTuple` and dropping the type unwrapping
  was probably the best solution for now.

Issue: getsentry/team-ospo#196

Author: azaslavsky

src/sentry/backup/findings.py

@@ -1,16 +1,15 @@
 from __future__ import annotations
 
+from abc import ABC, abstractmethod
 from copy import deepcopy
 from dataclasses import dataclass
 from enum import IntEnum, auto, unique
-from typing import Optional
+from typing import List, NamedTuple, Optional
 
-from sentry.backup.dependencies import NormalizedModelName
 from sentry.utils import json
 
 
-@dataclass
-class InstanceID:
+class InstanceID(NamedTuple):
     """Every entry in the generated backup JSON file should have a unique model+ordinal combination,
     which serves as its identifier."""
 
@@ -19,21 +18,30 @@ class InstanceID:
     # The order that this model appeared in the JSON inputs. Because we validate that the same
     # number of models of each kind are present on both the left and right side when validating, we
     # can use the ordinal as a unique identifier.
-    ordinal: int | None = None
-
-    def __init__(self, model: NormalizedModelName, ordinal: Optional[int] = None):
-        self.model = str(model)
-        self.ordinal = ordinal
-
-    def __hash__(self):
-        return hash((self.model, self.ordinal))
+    ordinal: Optional[int] = None
 
     def pretty(self) -> str:
         out = f"InstanceID(model: {self.model!r}"
         if self.ordinal:
             out += f", ordinal: {self.ordinal}"
         return out + ")"
 
+    @classmethod
+    def encode(_, id: InstanceID) -> dict:
+        """
+        Encode to JSON as a dictionary.
+        """
+
+        return {"model": id.model, "ordinal": id.ordinal}
+
+    @classmethod
+    def decode(cls, data: dict) -> InstanceID:
+        """
+        Decode from a JSON dictionary.
+        """
+
+        return cls(model=data["model"], ordinal=data["ordinal"])
+
 
 class FindingKind(IntEnum):
     pass
@@ -132,21 +140,41 @@ class ComparatorFindingKind(FindingKind):
 
 
 @dataclass(frozen=True)
-class Finding:
+class Finding(ABC):
     """
-    A JSON serializable and user-reportable finding for an import/export operation.
+    A JSON serializable and user-reportable finding for an import/export operation. Don't use this
+    class directly - inherit from it, set a specific `kind` type, and define your own pretty
+    printer!
     """
 
     on: InstanceID
 
     # The original `pk` of the model in question, if one is specified in the `InstanceID`.
-    left_pk: int | None = None
+    left_pk: Optional[int] = None
 
     # The post-import `pk` of the model in question, if one is specified in the `InstanceID`.
-    right_pk: int | None = None
+    right_pk: Optional[int] = None
 
     reason: str = ""
 
+    def _pretty_inner(self) -> str:
+        """
+        Pretty print only the fields on the shared `Finding` portion.
+        """
+
+        out = f"\n\ton: {self.on.pretty()}"
+        if self.left_pk:
+            out += f",\n\tleft_pk: {self.left_pk}"
+        if self.right_pk:
+            out += f",\n\tright_pk: {self.right_pk}"
+        if self.reason:
+            out += f",\n\treason: {self.reason}"
+        return out
+
+    @abstractmethod
+    def pretty(self) -> str:
+        pass
+
 
 @dataclass(frozen=True)
 class ComparatorFinding(Finding):
@@ -157,20 +185,13 @@ class ComparatorFinding(Finding):
     kind: ComparatorFindingKind = ComparatorFindingKind.Unknown
 
     def pretty(self) -> str:
-        out = f"Finding(\n\tkind: {self.kind.name},\n\ton: {self.on.pretty()}"
-        if self.left_pk:
-            out += f",\n\tleft_pk: {self.left_pk}"
-        if self.right_pk:
-            out += f",\n\tright_pk: {self.right_pk}"
-        if self.reason:
-            out += f",\n\treason: {self.reason}"
-        return out + "\n)"
+        return f"ComparatorFinding(\n\tkind: {self.kind.name},{self._pretty_inner()}\n)"
 
 
 class ComparatorFindings:
     """A wrapper type for a list of 'ComparatorFinding' which enables pretty-printing in asserts."""
 
-    def __init__(self, findings: list[ComparatorFinding]):
+    def __init__(self, findings: List[ComparatorFinding]):
         self.findings = findings
 
     def append(self, finding: ComparatorFinding) -> None:
@@ -179,7 +200,7 @@ def append(self, finding: ComparatorFinding) -> None:
     def empty(self) -> bool:
         return not self.findings
 
-    def extend(self, findings: list[ComparatorFinding]) -> None:
+    def extend(self, findings: List[ComparatorFinding]) -> None:
         self.findings += findings
 
     def pretty(self) -> str:
@@ -196,6 +217,4 @@ def default(self, obj):
             if isinstance(kind, FindingKind):
                 d["kind"] = kind.name
             return d
-        if isinstance(obj, InstanceID):
-            return obj.__dict__
         return super().default(obj)

src/sentry/backup/validate.py

@@ -52,7 +52,7 @@ def assign(self, obj: JSONData, side: Side) -> Tuple[int, list[ComparatorFinding
                 findings.append(
                     ComparatorFinding(
                         kind=ComparatorFindingKind.UnorderedInput,
-                        on=InstanceID(model_name, self.next_ordinal),
+                        on=InstanceID(str(model_name), self.next_ordinal),
                         left_pk=pk if side == Side.left else None,
                         right_pk=pk if side == Side.right else None,
                         reason=f"""instances not listed in ascending `pk` order; `pk` {pk} is less than or equal to {self.max_seen_pk} which precedes it""",
@@ -78,7 +78,7 @@ def build_model_map(
             counter = ordinal_counters[model_name]
             ordinal, found = counter.assign(model, side)
             findings.extend(found)
-            id = InstanceID(model_name, ordinal)
+            id = InstanceID(str(model_name), ordinal)
             model_map[id] = model
         return (model_map, ordinal_counters)
 
@@ -109,7 +109,7 @@ def json_lines(obj: JSONData) -> list[str]:
             findings.append(
                 ComparatorFinding(
                     kind=ComparatorFindingKind.UnequalCounts,
-                    on=InstanceID(model_name),
+                    on=InstanceID(str(model_name)),
                     reason=f"""counted {left_count} left entries and {right_count} right entries""",
                 )
             )