fix(core): preserve data integrity and bound shared UI and response work

Release v0.1.46. Coordinated integrity review: GovOPlaN/govoplan-core#298.

tests/test_conditional_requests.py

@@ -1,15 +1,85 @@
 from __future__ import annotations
 
 import unittest
+from unittest.mock import patch
 
-from fastapi import APIRouter, Response
+from fastapi import APIRouter, Request, Response
 from fastapi.responses import PlainTextResponse
 from fastapi.testclient import TestClient
+from starlette.background import BackgroundTask
+from starlette.responses import StreamingResponse
 
 from govoplan_core.auth import get_api_principal
 from govoplan_core.core.registry import PlatformRegistry
 from govoplan_core.server.fastapi import create_govoplan_app
 from govoplan_core.server.platform import create_platform_router
+from govoplan_core.server.conditional_requests import conditional_json_get_middleware
+
+
+class ConditionalBufferTests(unittest.IsolatedAsyncioTestCase):
+    async def test_unknown_length_overflow_replays_exact_bytes_without_eager_drain(self) -> None:
+        chunks = [b'{"value":"', b'', b'a' * 32, b'b' * 32, b'c' * 32, b'"}']
+        consumed = []
+
+        async def body():
+            for chunk in chunks:
+                consumed.append(chunk)
+                yield chunk
+
+        background = BackgroundTask(lambda: None)
+        response = StreamingResponse(body(), media_type="application/json", background=background)
+
+        async def route(request):
+            return response
+
+        request = Request({"type": "http", "method": "GET", "headers": [(b'if-none-match', b'*')]})
+        with patch("govoplan_core.server.conditional_requests.MAX_CONDITIONAL_JSON_BYTES", 64, create=True):
+            result = await conditional_json_get_middleware(request, route)
+        self.assertIs(result, response)
+        self.assertEqual(4, len(consumed))
+        self.assertEqual(200, result.status_code)
+        self.assertNotIn("etag", result.headers)
+        self.assertIn("private", result.headers["cache-control"])
+        self.assertIn("Authorization", result.headers["vary"])
+        self.assertIs(background, result.background)
+        self.assertEqual(b"".join(chunks), b"".join([chunk async for chunk in result.body_iterator]))
+        self.assertEqual(chunks, consumed)
+
+    async def test_known_large_body_is_not_consumed(self) -> None:
+        consumed = []
+
+        async def body():
+            consumed.append(True)
+            yield b"x" * 65
+
+        response = StreamingResponse(body(), media_type="application/json", headers={"Content-Length": "65"})
+
+        async def route(request):
+            return response
+
+        request = Request({"type": "http", "method": "GET", "headers": []})
+        with patch("govoplan_core.server.conditional_requests.MAX_CONDITIONAL_JSON_BYTES", 64, create=True):
+            result = await conditional_json_get_middleware(request, route)
+        self.assertIs(result, response)
+        self.assertEqual([], consumed)
+        self.assertEqual("65", result.headers["content-length"])
+        self.assertEqual(b"x" * 65, b"".join([chunk async for chunk in result.body_iterator]))
+
+    async def test_matching_small_response_still_runs_current_route_authorization(self) -> None:
+        calls = []
+
+        async def route(request):
+            calls.append(True)
+            if len(calls) > 1:
+                return Response(status_code=403)
+            return StreamingResponse(iter([b'{"ok":true}']), media_type="application/json")
+
+        request = Request({"type": "http", "method": "GET", "headers": []})
+        first = await conditional_json_get_middleware(request, route)
+        conditional = Request({"type": "http", "method": "GET", "headers": [(b'if-none-match', first.headers['etag'].encode())]})
+        second = await conditional_json_get_middleware(conditional, route)
+        self.assertEqual(403, second.status_code)
+        self.assertEqual(2, len(calls))
 
 
 class ConditionalRequestTests(unittest.TestCase):

tests/test_json_predicates.py

@@ -0,0 +1,74 @@
+from __future__ import annotations
+
+import unittest
+
+from sqlalchemy import JSON, Column, Integer, MetaData, Table, create_engine, select
+from sqlalchemy.dialects import mysql, postgresql
+from sqlalchemy.exc import CompileError
+
+from govoplan_core.db.json_predicates import (
+    json_array_contains_object_strings,
+    json_array_contains_string,
+    json_object_matches_strings,
+)
+
+
+class JsonPredicateTests(unittest.TestCase):
+    def setUp(self):
+        self.engine = create_engine("sqlite:///:memory:")
+        self.metadata = MetaData()
+        self.records = Table("records", self.metadata, Column("id", Integer, primary_key=True), Column("value", JSON))
+        self.metadata.create_all(self.engine)
+
+    def tearDown(self):
+        self.engine.dispose()
+
+    def matched(self, values, predicate):
+        with self.engine.begin() as connection:
+            connection.execute(self.records.insert(), [{"id": index, "value": value} for index, value in enumerate(values)])
+            return list(connection.scalars(select(self.records.c.id).where(predicate).order_by(self.records.c.id)))
+
+    def test_string_membership_is_array_and_type_exact(self):
+        self.assertEqual([0], self.matched(
+            [["1"], [1], [True], [None], {"key": "1"}, "1", None, ["11"]],
+            json_array_contains_string(self.records.c.value, "1"),
+        ))
+
+    def test_object_fields_are_exact_strings_not_coerced_or_substrings(self):
+        self.assertEqual([0], self.matched(
+            [{"kind": "account", "id": "1", "label": "Extra field allowed"},
+             {"kind": "account", "id": 1}, {"kind": "account", "id": "11"},
+             "not-json", None, ["account", "1"]],
+            json_object_matches_strings(self.records.c.value, {"kind": "account", "id": "1"}),
+        ))
+
+    def test_array_objects_do_not_match_encoded_objects_or_scalar_elements(self):
+        self.assertEqual([0], self.matched(
+            [[{"kind": "account", "id": "1"}], ['{"kind":"account","id":"1"}'],
+             [{"kind": "account", "id": 1}], ["not-json", None, True, 1],
+             {"kind": "account", "id": "1"}, None],
+            json_array_contains_object_strings(self.records.c.value, {"kind": "account", "id": "1"}),
+        ))
+
+    def test_values_remain_bound_for_both_dialects(self):
+        value = "x' OR 1=1 --"
+        predicates = [
+            json_array_contains_string(self.records.c.value, value),
+            json_object_matches_strings(self.records.c.value, {"id": value}),
+            json_array_contains_object_strings(self.records.c.value, {"id": value}),
+        ]
+        for predicate in predicates:
+            for dialect in (self.engine.dialect, postgresql.dialect()):
+                with self.subTest(predicate=type(predicate).__name__, dialect=dialect.name):
+                    compiled = select(self.records.c.id).where(predicate).compile(dialect=dialect)
+                    self.assertNotIn(value, str(compiled))
+                    self.assertIn(value, compiled.params.values())
+            with self.assertRaises(CompileError):
+                select(self.records.c.id).where(predicate).compile(dialect=mysql.dialect())
+
+    def test_invalid_field_names_and_non_string_matches_are_rejected(self):
+        for fields in ({"id": 1}, {"not.a.field": "1"}, {}):
+            with self.subTest(fields=fields), self.assertRaises((TypeError, ValueError)):
+                json_array_contains_object_strings(self.records.c.value, fields)
+        with self.assertRaises(TypeError):
+            json_array_contains_string(self.records.c.value, True)

tests/test_migration_url_config.py

@@ -0,0 +1,67 @@
+from __future__ import annotations
+
+from pathlib import Path
+import runpy
+from types import SimpleNamespace
+import unittest
+from unittest.mock import MagicMock, patch
+
+from alembic import context
+from alembic.config import Config
+
+
+class MigrationUrlConfigurationTests(unittest.TestCase):
+    def test_database_urls_round_trip_exactly_in_online_and_offline_modes(self) -> None:
+        urls = (
+            "postgresql+psycopg://localhost/example?host=%2Ftmp%2Fexample",
+            "postgresql+psycopg://synthetic%40user:synthetic%25%40pass@localhost/example",
+            "sqlite:////tmp/synthetic%25-database.db",
+            "sqlite:////tmp/synthetic%%-database.db",
+            "sqlite:////tmp/%(here)s-literal.db",
+            "sqlite:////tmp/synthetic-database.db",
+        )
+        environment = Path(__file__).resolve().parents[1] / "alembic" / "env.py"
+        for url in urls:
+            for offline in (False, True):
+                for from_settings in (False, True):
+                    with self.subTest(url=url, offline=offline, from_settings=from_settings):
+                        config = Config()
+                        config.attributes.update(enabled_modules=(), manifest_factories=())
+                        if not from_settings:
+                            config.attributes["database_url"] = url
+                        engine = MagicMock()
+                        connection = engine.connect.return_value.__enter__.return_value
+                        with (
+                            patch.object(context, "config", config, create=True),
+                            patch.object(context, "is_offline_mode", return_value=offline),
+                            patch.object(context, "configure") as configure,
+                            patch.object(context, "begin_transaction"),
+                            patch.object(context, "run_migrations") as run_migrations,
+                            patch("sqlalchemy.engine_from_config", return_value=engine) as engine_from_config,
+                            patch(
+                                "govoplan_core.server.default_config.get_server_config",
+                                return_value=SimpleNamespace(enabled_modules=(), manifest_factories=()),
+                            ),
+                            patch("govoplan_core.server.registry.build_platform_registry"),
+                            patch(
+                                "govoplan_core.core.migrations.migration_metadata_plan",
+                                return_value=SimpleNamespace(metadata=()),
+                            ),
+                            patch("govoplan_core.settings.settings.database_url", url),
+                        ):
+                            runpy.run_path(str(environment))
+
+                        self.assertEqual(config.get_main_option("sqlalchemy.url"), url)
+                        self.assertEqual(config.get_section(config.config_ini_section)["sqlalchemy.url"], url)
+                        run_migrations.assert_called_once_with()
+                        if offline:
+                            engine_from_config.assert_not_called()
+                            self.assertEqual(configure.call_args.kwargs["url"], url)
+                        else:
+                            engine_from_config.assert_called_once()
+                            self.assertEqual(engine_from_config.call_args.args[0]["sqlalchemy.url"], url)
+                            self.assertIs(configure.call_args.kwargs["connection"], connection)
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/test_optimistic_concurrency.py

@@ -27,6 +27,25 @@ class _RevisionFixture(Base):
 
 
 class OptimisticConcurrencyTests(unittest.TestCase):
+    def test_keyed_insertions_keep_their_anchors_during_disjoint_edits(self) -> None:
+        base = [{"id": "a", "value": 1}, {"id": "b", "value": 2}]
+        local = [base[0], {"id": "x", "value": 3}, base[1]]
+        current = [base[0], {"id": "b", "value": 20}]
+        result = three_way_merge(base, local, current)
+        self.assertTrue(result.merged)
+        self.assertEqual(["a", "x", "b"], [item["id"] for item in result.value])
+        self.assertEqual(20, result.value[-1]["value"])
+        self.assertEqual(["a", "b"], [item["id"] for item in base])
+
+    def test_concurrent_insertions_are_stable_and_incompatible_anchors_conflict(self) -> None:
+        base = [{"id": "a"}, {"id": "b"}]
+        result = three_way_merge(base, [base[0], {"id": "x"}, base[1]], [base[0], {"id": "y"}, base[1]])
+        self.assertTrue(result.merged)
+        self.assertEqual(["a", "y", "x", "b"], [item["id"] for item in result.value])
+        conflict = three_way_merge(base, [base[0], {"id": "x"}, base[1]], [base[1], base[0]])
+        self.assertFalse(conflict.merged)
+        self.assertEqual("collection_reorder", conflict.conflicts[0].kind)
+
     def test_strong_etags_and_if_match_use_strong_comparison(self) -> None:
         etag = strong_resource_etag("campaign_version", "version-1", 3)
 

tests/test_principal_helpers.py

@@ -0,0 +1,43 @@
+from types import SimpleNamespace
+import unittest
+
+from govoplan_core.core.principal_helpers import principal_actor_ids, principal_user_first_actor
+
+
+class PrincipalHelperTests(unittest.TestCase):
+    def test_contracts_keep_their_distinct_precedence_and_whitespace(self) -> None:
+        principal = SimpleNamespace(account_id=" account ", identity_id=" identity ", membership_id=" membership ", user=SimpleNamespace(id=" user "))
+        self.assertEqual((" account ", " identity ", " membership ", " user "), principal_actor_ids(principal))
+        self.assertEqual("user", principal_user_first_actor(principal))
+        self.assertEqual(" account ", principal.account_id)
+        self.assertEqual(" user ", principal.user.id)
+
+    def test_duplicate_ids_are_removed_in_stable_order_without_normalizing(self) -> None:
+        principal = SimpleNamespace(account_id="same", identity_id=" same ", membership_id="same", user=SimpleNamespace(id="same"))
+        self.assertEqual(("same", " same "), principal_actor_ids(principal))
+        self.assertEqual("same", principal_user_first_actor(principal))
+
+    def test_missing_blank_and_service_only_principals_remain_unattributed(self) -> None:
+        for principal in (
+            object(), None, SimpleNamespace(), SimpleNamespace(service_account_id="service"),
+            SimpleNamespace(account_id=0, identity_id=False, membership_id="\t", user=SimpleNamespace(id="\u00a0")),
+        ):
+            with self.subTest(principal=principal):
+                self.assertEqual((), principal_actor_ids(principal))
+                self.assertIsNone(principal_user_first_actor(principal))
+
+    def test_fallback_and_existing_string_coercion_are_unchanged(self) -> None:
+        for values, expected_ids, expected_actor in (
+            ({"account_id": 17, "identity_id": "identity"}, ("17", "identity"), "17"),
+            ({"identity_id": " identity ", "membership_id": "member"}, (" identity ", "member"), "identity"),
+            ({"membership_id": " member "}, (" member ",), "member"),
+            ({"user": SimpleNamespace(id=" user ")}, (" user ",), "user"),
+        ):
+            with self.subTest(values=values):
+                principal = SimpleNamespace(**values)
+                self.assertEqual(expected_ids, principal_actor_ids(principal))
+                self.assertEqual(expected_actor, principal_user_first_actor(principal))
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/test_review_mechanics.py

@@ -0,0 +1,144 @@
+from __future__ import annotations
+
+import random
+import unittest
+from decimal import Decimal
+
+from govoplan_core.core.modules import DocumentationTopic, localize_documentation_topics
+from govoplan_core.core.tabular_sources import (
+    TabularColumn,
+    TabularCsvSource,
+    TabularSourceUnavailableError,
+    TabularSourceValidationError,
+    csv_source_payload,
+    csv_source_summary,
+    verified_csv_source_text,
+    csv_projection_matches,
+    infer_tabular_schema,
+    parse_tabular_csv,
+    tabular_type_name,
+)
+
+
+class SharedReviewMechanicsTests(unittest.TestCase):
+    def test_invalid_csv_unicode_fails_explicitly_without_echoing_content(self) -> None:
+        source = TabularCsvSource(text="value\nprivate-\ud800\n")
+        for action in (
+            lambda: csv_source_payload(source),
+            lambda: parse_tabular_csv(source.text),
+        ):
+            with (
+                self.subTest(action=action),
+                self.assertRaises(TabularSourceValidationError) as raised,
+            ):
+                action()
+            self.assertNotIn("private", str(raised.exception))
+        with self.assertRaises(TabularSourceUnavailableError):
+            verified_csv_source_text({"text": source.text})
+
+    def test_csv_huge_integer_has_explicit_validation_and_lossless_text_option(
+        self,
+    ) -> None:
+        import sys
+
+        maximum = sys.get_int_max_str_digits()
+        if not maximum:
+            self.skipTest("Interpreter integer conversion limit is disabled")
+        text = "9" * (maximum + 1)
+        with self.assertRaisesRegex(TabularSourceValidationError, "use text mode"):
+            parse_tabular_csv("value\n" + text + "\n")
+        self.assertEqual(
+            ({"value": text},),
+            parse_tabular_csv("value\n" + text + "\n", value_mode="text"),
+        )
+
+    def test_csv_projection_binding_distinguishes_boolean_integer_and_float(
+        self,
+    ) -> None:
+        for expected in (True, 1, 1.0):
+            for actual in (True, 1, 1.0):
+                with self.subTest(
+                    expected_type=type(expected), actual_type=type(actual)
+                ):
+                    self.assertEqual(
+                        type(expected) is type(actual),
+                        csv_projection_matches(
+                            ({"value": expected},), ({"value": actual},)
+                        ),
+                    )
+        self.assertFalse(csv_projection_matches(({"value": 1},), ({"other": 1},)))
+
+    def test_translation_merge_preserves_owner_data_and_untranslated_identity(
+        self,
+    ) -> None:
+        translations = {"de": {"summary": "vorhanden"}, "fr": {"title": "Français"}}
+        first = DocumentationTopic(
+            id="one", title="One", summary="First", translations=translations
+        )
+        unchanged = DocumentationTopic(id="two", title="Two", summary="Second")
+        localized = localize_documentation_topics(
+            iter((first, unchanged)),
+            locale="de",
+            translations={"one": {"title": "Eins"}},
+        )
+        self.assertEqual(
+            {"title": "Eins", "summary": "vorhanden"}, localized[0].translations["de"]
+        )
+        self.assertEqual({"title": "Français"}, localized[0].translations["fr"])
+        self.assertIs(unchanged, localized[1])
+        self.assertEqual({"summary": "vorhanden"}, first.translations["de"])
+        self.assertIsNot(translations["fr"], localized[0].translations["fr"])
+
+    def test_schema_inference_matches_legacy_projection_for_sparse_mixed_rows(
+        self,
+    ) -> None:
+        generator = random.Random(298)
+        values = [None, True, False, 1, 1.5, Decimal("1.00"), "001", [], {}]
+        rows = [
+            {
+                f"field_{column}": generator.choice(values)
+                for column in generator.sample(range(80), 20)
+            }
+            for _ in range(100)
+        ]
+        names = list(dict.fromkeys(name for row in rows for name in row))
+        expected = []
+        for name in names:
+            concrete = [row[name] for row in rows if row.get(name) is not None]
+            types = {tabular_type_name(value) for value in concrete}
+            kind = (
+                "unknown"
+                if not types
+                else next(iter(types))
+                if len(types) == 1
+                else "mixed"
+            )
+            expected.append(
+                TabularColumn(
+                    name=name, data_type=kind, nullable=len(concrete) != len(rows)
+                )
+            )
+        self.assertEqual(tuple(expected), infer_tabular_schema(rows))
+        self.assertEqual(
+            (TabularColumn("only_null", "unknown", True),),
+            infer_tabular_schema([{"only_null": None}]),
+        )
+        self.assertEqual((), infer_tabular_schema([]))
+        unknown = type("ẞ", (), {})()
+        self.assertEqual("ß", tabular_type_name(unknown))
+        self.assertEqual("ss", tabular_type_name(unknown, casefold_unknown=True))
+
+    def test_csv_evidence_keeps_exact_text_but_summary_never_contains_content(
+        self,
+    ) -> None:
+        source = TabularCsvSource(
+            text='\ufeffvalue\r\n"  text  "\r\n', value_mode="text"
+        )
+        payload = csv_source_payload(source)
+        self.assertEqual(source.text, verified_csv_source_text(payload))
+        self.assertNotIn("text", csv_source_summary(payload))
+        self.assertEqual(len(source.text.encode("utf-8")), payload["byte_count"])
+        with self.assertRaises(TabularSourceUnavailableError):
+            verified_csv_source_text(
+                {**payload, "text": source.text.replace("text", "edited")}
+            )

tests/test_tabular_source_contract.py

@@ -178,6 +178,29 @@ class TabularSourceContractTests(unittest.TestCase):
         with self.assertRaises(TabularSourceValidationError):
             parse_tabular_csv("id,name\n1,Ada,extra\n")
 
+    def test_text_csv_mode_preserves_lexical_values_and_explicit_empty_records(self) -> None:
+        source = 'id,value\r\n9007199254740993,"  keep me  "\r\ntrue,0.123456789012345678901234567890\r\n"   ",""\r\n'
+        self.assertEqual(
+            (
+                {"id": "9007199254740993", "value": "  keep me  "},
+                {"id": "true", "value": "0.123456789012345678901234567890"},
+                {"id": "   ", "value": ""},
+            ),
+            parse_tabular_csv(source, value_mode="text"),
+        )
+        self.assertEqual(({"value": "   "},), parse_tabular_csv('value\n"   "\n', value_mode="text"))
+
+    def test_text_csv_mode_rejects_shape_loss_and_applies_input_and_row_bounds(self) -> None:
+        for source in ('id,name\n1\n', 'id,name\n1,Ada,\n'):
+            with self.subTest(source=source), self.assertRaises(TabularSourceValidationError):
+                parse_tabular_csv(source, value_mode="text")
+        with self.assertRaises(TabularSourceValidationError):
+            parse_tabular_csv('value\n""\n""\n', value_mode="text", max_rows=1)
+        with self.assertRaises(TabularSourceValidationError):
+            parse_tabular_csv('value\nä\n', value_mode="text", max_bytes=8)
+        with self.assertRaises(TabularSourceValidationError):
+            parse_tabular_csv('value\nx\n', value_mode="unknown")
+
 
 if __name__ == "__main__":
     unittest.main()