diff --git a/database/models.py b/database/models.py index ce439dd..3724465 100644 --- a/database/models.py +++ b/database/models.py @@ -18,7 +18,11 @@ from typing import Optional, Set, Tuple from .library_schema import library_schema_error, migrate_library_state from .image_favorites import image_favorites_schema_error, migrate_image_favorites from .annotation_schema import annotation_schema_error, migrate_annotation_state -from .review_schema import migrate_review_state, review_schema_error +from .review_schema import ( + migrate_review_state, + review_schema_error, + review_without_annotations_error, +) from .saved_views import migrate_saved_views, saved_views_schema_error from .execution_history_schema import ( history_schema_error, @@ -305,8 +309,9 @@ class PromptModel: migrate_image_favorites(conn) migrate_saved_views(conn) # A review group without its annotation group is refused before the - # annotation install could make it look valid. - review_error = review_schema_error(conn) + # annotation install could make it look valid (catalog only; the rows + # are validated once, in migrate_review_state). + review_error = review_without_annotations_error(conn) if review_error: raise sqlite3.DatabaseError(review_error) migrate_annotation_state(conn) diff --git a/database/review_schema.py b/database/review_schema.py index f610744..b7f5bdd 100644 --- a/database/review_schema.py +++ b/database/review_schema.py @@ -33,7 +33,10 @@ import re import sqlite3 from typing import Any, Optional -from .annotation_schema import BYTES_TABLE, annotation_schema_error +from .annotation_schema import BYTES_TABLE +from .annotation_schema import TABLES as ANNOTATION_TABLES +from .annotation_schema import _catalog as _annotation_catalog +from .annotation_schema import annotation_schema_error SCHEMA_VERSION = 1 METADATA_TABLE = "media_review_metadata" @@ -159,6 +162,13 @@ def _reject_constant(name: str) -> Any: raise _NonCanonical(f"JSON constant {name} is not allowed") +# Built once: validation re-encodes every stored document at startup. +_ENCODER = json.JSONEncoder( + sort_keys=True, separators=(",", ":"), ensure_ascii=True, allow_nan=False +) +_DECODER = json.JSONDecoder(parse_constant=_reject_constant) + + def canonical_evidence(value: Any) -> str: """Serialize one evidence object canonically; ``ValueError`` when invalid. @@ -168,13 +178,7 @@ def canonical_evidence(value: Any) -> str: if not isinstance(value, dict): raise ValueError("Review evidence must be a JSON object") try: - text = json.dumps( - value, - sort_keys=True, - separators=(",", ":"), - ensure_ascii=True, - allow_nan=False, - ) + text = _ENCODER.encode(value) except (TypeError, RecursionError) as error: raise ValueError(f"Review evidence is not serializable: {error}") from None if len(text) > MAX_EVIDENCE_BYTES: @@ -191,7 +195,7 @@ def _is_canonical(text: Any) -> bool: if not isinstance(text, str): return False try: - value = json.loads(text, parse_constant=_reject_constant) + value = _DECODER.decode(text) return canonical_evidence(value) == text except (ValueError, RecursionError): return False @@ -228,12 +232,12 @@ def _hex64(column: str) -> str: def _evidence(column: str) -> str: - # Printable ASCII only is a cheap necessary condition for canonical text; - # the streamed pass below proves the rest. + # Type and size only: the streamed pass below proves the text canonical + # (which implies printable ASCII), so a pattern scan here would read every + # document a second time. return ( f"typeof({column}) = 'text' " - f"AND length(CAST({column} AS BLOB)) BETWEEN 2 AND {MAX_EVIDENCE_BYTES} " - f"AND CAST({column} AS BLOB) NOT GLOB '*[^ -~]*'" + f"AND length(CAST({column} AS BLOB)) BETWEEN 2 AND {MAX_EVIDENCE_BYTES}" ) @@ -356,25 +360,55 @@ def _value_error(conn: sqlite3.Connection) -> Optional[str]: return _evidence_error(conn) +def review_without_annotations_error(conn: sqlite3.Connection) -> Optional[str]: + """Catalog only: a review group present without its annotation tables. + + Startup asks this before the annotation group installs, which would make + a review group copied without it look valid. It reads no rows; the full + validation runs once in :func:`migrate_review_state`. + """ + if not _catalog(conn): + return None + present = { + name for kind, name, _sql in _annotation_catalog(conn) if kind == "table" + } + if present != set(ANNOTATION_TABLES): + return ( + "Review queue schema requires a valid image annotation schema: " + "Missing image annotation schema" + ) + return None + + def review_schema_error( - conn: sqlite3.Connection, *, allow_missing: bool = True + conn: sqlite3.Connection, + *, + allow_missing: bool = True, + annotations_valid: bool = False, ) -> Optional[str]: """Reject partial or unsupported review schemas and invalid rows. Reads only: nothing is ever repaired. A database without any of the group's objects is acceptable for backups that predate it unless ``allow_missing`` is false. A present group requires a present, valid - image annotation group. + image annotation group; ``annotations_valid`` says the caller has just + validated it (startup's annotation install), so only its presence is + checked again. """ objects = _catalog(conn) if not objects: return None if allow_missing else "Missing review queue schema" - annotation_error = annotation_schema_error(conn, allow_missing=False) + if annotations_valid: + annotation_error = review_without_annotations_error(conn) + else: + annotation_error = annotation_schema_error(conn, allow_missing=False) + if annotation_error: + annotation_error = ( + "Review queue schema requires a valid image annotation schema: " + + annotation_error + ) if annotation_error: - return ( - "Review queue schema requires a valid image annotation schema: " - + annotation_error - ) + return annotation_error present = {name for kind, name, _sql in objects if kind == "table"} if present != set(TABLES): return "Incomplete review queue schema" @@ -387,13 +421,14 @@ def review_schema_error( def migrate_review_state(conn: sqlite3.Connection) -> None: """Install the group atomically; validate before and after creating it. - Runs after ``migrate_annotation_state``: the group is only valid beside - the annotation group it cites. + Runs after ``migrate_annotation_state`` (which has just validated the + annotation group): the group is only valid beside the annotation group it + cites. A present group is validated once, evidence included. """ conn.execute("SAVEPOINT media_review_schema") try: installed = bool(_catalog(conn)) - error = review_schema_error(conn) + error = review_schema_error(conn, annotations_valid=True) if error: raise sqlite3.DatabaseError(error) if installed: @@ -410,7 +445,7 @@ def migrate_review_state(conn: sqlite3.Connection) -> None: "(singleton_id, schema_version, review_index_revision) VALUES (1, ?, 0)", (SCHEMA_VERSION,), ) - error = review_schema_error(conn, allow_missing=False) + error = review_schema_error(conn, allow_missing=False, annotations_valid=True) if error: raise sqlite3.DatabaseError(error) conn.execute("RELEASE media_review_schema") diff --git a/tests/test_review_schema.py b/tests/test_review_schema.py index dfd6140..034e26d 100644 --- a/tests/test_review_schema.py +++ b/tests/test_review_schema.py @@ -330,6 +330,43 @@ class ReviewSchemaTests(unittest.TestCase): conn.execute(f"DELETE FROM {ITEMS}") self.assertEqual(table_rows(conn, TABLES), snapshot) + def test_startup_validates_stored_evidence_once(self): + """A reopen streams the evidence once and the annotation group twice + (its own install), never again for the review group (I3).""" + self.require_module() + import database.annotation_schema as annotation_module + import database.review_schema as review_module + + model = self.model() + with model.get_connection() as conn: + insert_group_rows(conn) + model.close_all() + for _ in range(2): + PromptModel.reset_schema_cache() + with ( + patch.object( + review_module, + "_evidence_error", + wraps=review_module._evidence_error, + ) as evidence, + patch.object( + review_module, + "annotation_schema_error", + wraps=review_module.annotation_schema_error, + ) as nested, + patch.object( + annotation_module, + "_value_error", + wraps=annotation_module._value_error, + ) as annotation_rows, + ): + model = self.model() + model.get_connection() + self.assertEqual(evidence.call_count, 1) + self.assertEqual(nested.call_count, 0) + self.assertEqual(annotation_rows.call_count, 2) + model.close_all() + def test_old_database_without_group_migrates_without_touching_legacy_rows(self): self.require_module() for label, dropped in (