refactor: hide SnapshotValue from Snapshot API

python/selfie-lib/pyproject.toml

@@ -34,3 +34,9 @@ ignore = [ "S", "FA",  "PYI", "EM", "PLR",  "FBT", "COM", "RET", "PTH", "PLW", "
 "PGH003", # specific rule codes when ignoring type issues
 "ISC001"
 ]
+
+[tool.ruff.lint.per-file-ignores]
+"selfie_lib/CacheSelfie.py" = ["SLF001"]
+"selfie_lib/Lens.py" = ["SLF001"]
+"selfie_lib/SelfieImplementations.py" = ["SLF001"]
+"selfie_lib/SnapshotFile.py" = ["SLF001"]

python/selfie-lib/selfie_lib/CacheSelfie.py

@@ -69,11 +69,12 @@ def _to_match_disk_impl(self, sub: str, is_todo: bool) -> T:
                 snapshot = self.disk.read_disk(sub, call)
                 if snapshot is None:
                     raise Exception(system.mode.msg_snapshot_not_found())
-                if snapshot.subject.is_binary or len(snapshot.facets) > 0:
+                subject = snapshot._subject
+                if subject.is_binary or len(snapshot.facets) > 0:
                     raise Exception(
                         f"Expected a string subject with no facets, got {snapshot}"
                     )
-                return self.roundtrip.parse(snapshot.subject.value_string())
+                return self.roundtrip.parse(subject.value_string())
 
     def to_be_TODO(self, _: Optional[Any] = None) -> T:
         return self._to_be_impl(None)
@@ -141,12 +142,13 @@ def _to_match_disk_impl(self, sub: str, is_todo: bool) -> T:
                 if snapshot is None:
                     raise Exception(system.mode.msg_snapshot_not_found())
 
-                if snapshot.subject.is_binary or len(snapshot.facets) > 0:
+                subject = snapshot._subject
+                if subject.is_binary or len(snapshot.facets) > 0:
                     raise Exception(
                         f"Expected a binary subject with no facets, got {snapshot}"
                     )
 
-                return self.roundtrip.parse(snapshot.subject.value_binary())
+                return self.roundtrip.parse(subject.value_binary())
 
     def to_be_file_TODO(self, subpath: str) -> T:
         return self._to_be_file_impl(subpath, True)

python/selfie-lib/selfie_lib/Lens.py

@@ -53,7 +53,7 @@ def set_facet_from(
         self, target: str, source: str, function: Callable[[str], Optional[str]]
     ) -> "CompoundLens":
         def _set_facet_from(snapshot: Snapshot) -> Snapshot:
-            source_value = snapshot.subject_or_facet_maybe(source)
+            source_value = snapshot._subject_or_facet_maybe_internal(source)
             if source_value is None:
                 return snapshot
             else:

python/selfie-lib/selfie_lib/SelfieImplementations.py

@@ -142,9 +142,10 @@ def to_match_disk_TODO(self, sub: str = "") -> "StringSelfie":
     def __actual(self) -> str:
         if not self.actual.facets or (self.only_facets and len(self.only_facets) == 1):
             # single value doesn't have to worry about escaping at all
-            only_value = self.actual.subject_or_facet(
-                self.only_facets[0] if self.only_facets else ""
-            )
+            facet_key = self.only_facets[0] if self.only_facets else ""
+            only_value = self.actual._subject_or_facet_maybe_internal(facet_key)
+            if only_value is None:
+                raise ValueError(f"Facet {facet_key} not found")
             if only_value.is_binary:
                 return (
                     base64.b64encode(only_value.value_binary())
@@ -175,16 +176,15 @@ def to_be(self, expected: str) -> str:
 
 class BinarySelfie(ReprSelfie[bytes], BinaryFacet):
     def __init__(self, actual: Snapshot, disk: DiskStorage, only_facet: str):
-        super().__init__(actual.subject.value_binary(), actual, disk)
-        self.only_facet = only_facet
-
-        facet_value = actual.subject_or_facet_maybe(only_facet)
+        facet_value = actual._subject_or_facet_maybe_internal(only_facet)
         if facet_value is None:
             raise ValueError(f"The facet {only_facet} was not found in the snapshot")
         elif not facet_value.is_binary:
             raise ValueError(
                 f"The facet {only_facet} is a string, not a binary snapshot"
             )
+        super().__init__(facet_value.value_binary(), actual, disk)
+        self.only_facet = only_facet
 
     def to_be_base64(self, expected: str) -> bytes:
         raise NotImplementedError
@@ -247,21 +247,17 @@ def _assertEqual(
         )
 
 
-def _serializeOnlyFacets(snapshot: Snapshot, keys: list[str]) -> str:
+def _serializeOnlyFacets(snapshot: Snapshot, facets: list[str]) -> str:
     writer = []
-    for key in keys:
-        if not key:
-            SnapshotFile.writeEntry(writer, "", None, snapshot.subject_or_facet(key))
+    for facet in facets:
+        value = snapshot._subject_or_facet_maybe_internal(facet)
+        if value is None:
+            continue
+        if not facet:
+            # Empty facets should have no header at all
+            writer.extend([value.value_string(), "\n"])
         else:
-            value = snapshot.subject_or_facet(key)
-            if value is not None:
-                SnapshotFile.writeEntry(writer, "", key, value)
+            SnapshotFile.writeEntry(writer, "", facet, value)
 
-    EMPTY_KEY_AND_FACET = "╔═  ═╗\n"
     writer_str = "".join(writer)
-
-    if writer_str.startswith(EMPTY_KEY_AND_FACET):
-        # this codepath is triggered by the `key.isEmpty()` line above
-        return writer_str[len(EMPTY_KEY_AND_FACET) : -1]
-    else:
-        return writer_str[:-1]
+    return writer_str[:-1] if writer_str else ""

python/selfie-lib/selfie_lib/Snapshot.py

@@ -1,3 +1,13 @@
+"""
+Snapshot class for managing test snapshots.
+
+Implementation Note:
+    This class uses private members (_subject, _facet_data) to maintain SnapshotValue
+    as an internal implementation detail while exposing Union[bytes, str] in the public API.
+    This design choice allows us to keep the implementation flexibility of SnapshotValue
+    while providing a simpler public interface.
+"""
+
 from collections.abc import Iterator
 from typing import Union
 
@@ -15,9 +25,12 @@ def __init__(
         self._subject = subject
         self._facet_data = facet_data
 
+    def _get_value(self, value: SnapshotValue) -> Union[bytes, str]:
+        return value.value_binary() if value.is_binary else value.value_string()
+
     @property
-    def subject(self) -> SnapshotValue:
-        return self._subject
+    def subject(self) -> Union[bytes, str]:
+        return self._get_value(self._subject)
 
     @property
     def facets(self) -> ArrayMap[str, SnapshotValue]:
@@ -54,10 +67,16 @@ def plus_or_replace(
                 ),
             )
 
-    def subject_or_facet_maybe(self, key: str) -> Union[SnapshotValue, None]:
+    def _subject_or_facet_maybe_internal(self, key: str) -> Union[SnapshotValue, None]:
         return self._subject if key == "" else self._facet_data.get(key)
 
-    def subject_or_facet(self, key: str) -> SnapshotValue:
+    def subject_or_facet_maybe(self, key: str) -> Union[bytes, str, None]:
+        value = self._subject_or_facet_maybe_internal(key)
+        if value is None:
+            return None
+        return self._get_value(value)
+
+    def subject_or_facet(self, key: str) -> Union[bytes, str]:
         value = self.subject_or_facet_maybe(key)
         if value is None:
             raise KeyError(f"'{key}' not found in snapshot.")
@@ -90,7 +109,9 @@ def items(self) -> Iterator[tuple[str, SnapshotValue]]:
         yield from self._facet_data.items()
 
     def __repr__(self) -> str:
-        pieces = [f"Snapshot.of({self.subject.value_string()!r})"]
-        for e in self.facets.items():
-            pieces.append(f"\n  .plus_facet({e[0]!r}, {e[1].value_string()!r})")  # noqa: PERF401
-        return "".join(pieces)
+        parts = [f"Snapshot.of({self._subject.value_string()!r})"]
+        parts.extend(
+            f"\n  .plus_facet({e[0]!r}, {e[1].value_string()!r})"
+            for e in self._facet_data.items()
+        )
+        return "".join(parts)

python/selfie-lib/selfie_lib/SnapshotFile.py

@@ -29,8 +29,8 @@ def serialize(self, valueWriter: list[str]):
             )
 
         for entry_key, entry_value in self.snapshots.items():
-            self.writeEntry(valueWriter, entry_key, None, entry_value.subject)
-            for facet_key, facet_value in entry_value.facets.items():
+            self.writeEntry(valueWriter, entry_key, None, entry_value._subject)
+            for facet_key, facet_value in entry_value._facet_data.items():
                 self.writeEntry(valueWriter, entry_key, facet_key, facet_value)
 
         self.writeEntry(valueWriter, "", "end of file", SnapshotValue.of(""))