diff --git a/.claude/rules/data-sync-storage.md b/.claude/rules/data-sync-storage.md index d9de4aa06b..9412f04609 100644 --- a/.claude/rules/data-sync-storage.md +++ b/.claude/rules/data-sync-storage.md @@ -13,7 +13,7 @@ paths: This path holds user data. A silent wrong answer here looks exactly like an empty database, so prove late-completion and cancellation behavior with a test. -- **A synced CKRecord type or field reaches CloudKit Production before anything writes it.** Both apps pin the container to Production, where nothing is auto-created, and CloudKit rejects a whole record that carries an undeclared field. Write fields only through `record.fields(SomeSyncField.self)`, never `record["..."]`. To ship one: add it in Development, deploy to Production in the Console, run `scripts/export-cloudkit-schema.sh`, commit `CloudKit/production-schema.ckdb`, then add it to the matching `verifiedInProduction`. `ProductionSchemaParityTests` checks code against the snapshot in both directions, not against the server, so never edit the snapshot by hand: `scripts/export-cloudkit-schema.sh --check` compares it with Production. +- **A synced CKRecord type or field reaches CloudKit Production before anything writes it.** Both apps pin the container to Production, where nothing is auto-created, and CloudKit rejects a whole record that carries an undeclared field. Write fields only through `record.fields(SomeSyncField.self)`, never `record["..."]`. To ship one: add it in Development, deploy to Production in the Console, run `scripts/export-cloudkit-schema.sh`, commit `CloudKit/production-schema.ckdb`, then add it to the matching `verifiedInProduction`. `ProductionSchemaParityTests` checks code against the snapshot in both directions, not against the server, so never edit the snapshot by hand: `scripts/export-cloudkit-schema.sh --check` compares it with Production. A tag's `app` job runs `--check-deployed` instead, which fails only on a record type, field, field type, index, role or grant the tag's snapshot has and Production lacks. - **A storage format stays readable by the previous stable.** Sparkle never downgrades, and a beta user can turn betas off (`RELEASING.md`, Channels). - **Persist, then notify.** `SyncChangeTracker.markDeleted()` runs after the storage write, because its notification can start a sync that re-uploads the record from the stale file. - **Concurrent schema loads await the one in-flight `loadTask`** in `SQLSchemaProvider`; never guard with a flag that returns without data. diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e0379268f6..c96e3c0644 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -103,7 +103,7 @@ jobs: - name: Check the Production CloudKit schema env: CLOUDKIT_MANAGEMENT_TOKEN: ${{ secrets.CLOUDKIT_MANAGEMENT_TOKEN }} - run: scripts/export-cloudkit-schema.sh --check + run: scripts/export-cloudkit-schema.sh --check-deployed - name: Write Secrets.xcconfig env: diff --git a/scripts/export-cloudkit-schema.sh b/scripts/export-cloudkit-schema.sh index d0942e6b52..b37eec6561 100755 --- a/scripts/export-cloudkit-schema.sh +++ b/scripts/export-cloudkit-schema.sh @@ -7,8 +7,9 @@ set -euo pipefail # Reads the management token from CLOUDKIT_MANAGEMENT_TOKEN, or from `xcrun cktool save-token --type management`. # # Usage: -# scripts/export-cloudkit-schema.sh # overwrite the snapshot -# scripts/export-cloudkit-schema.sh --check # compare without writing +# scripts/export-cloudkit-schema.sh # overwrite the snapshot +# scripts/export-cloudkit-schema.sh --check # compare without writing +# scripts/export-cloudkit-schema.sh --check-deployed # fail only on what the snapshot has and Production lacks TEAM_ID="D7HJ5TFYCU" CONTAINER_ID="iCloud.com.TablePro" @@ -21,8 +22,9 @@ MODE="export" case "${1:-}" in "") ;; --check) MODE="check" ;; + --check-deployed) MODE="check-deployed" ;; *) - echo "Usage: $0 [--check]" >&2 + echo "Usage: $0 [--check | --check-deployed]" >&2 exit 2 ;; esac @@ -50,6 +52,21 @@ if [[ "${MODE}" == "check" ]]; then exit 1 fi +# A release checks only its own snapshot's side: Production keeps every type and field a newer line +# deployed, and a build that never reads them is unaffected. +if [[ "${MODE}" == "check-deployed" ]]; then + LIVE_FILE="$(mktemp -t production-schema)" + trap 'rm -f "${LIVE_FILE}"' EXIT + echo "Checking that ${ENVIRONMENT} ${CONTAINER_ID} has everything CloudKit/production-schema.ckdb declares..." + export_schema "${LIVE_FILE}" + if python3 "${REPO_ROOT}/scripts/release/schema.py" covers "${SNAPSHOT_FILE}" "${LIVE_FILE}"; then + exit 0 + fi + echo >&2 + echo "${ENVIRONMENT} lacks what the snapshot declares: deploy it in CloudKit Console." >&2 + exit 1 +fi + mkdir -p "$(dirname "${SNAPSHOT_FILE}")" echo "Exporting ${ENVIRONMENT} schema for ${CONTAINER_ID}..." diff --git a/scripts/release/schema.py b/scripts/release/schema.py new file mode 100644 index 0000000000..02d9602fb9 --- /dev/null +++ b/scripts/release/schema.py @@ -0,0 +1,187 @@ +#!/usr/bin/env python3 + +import argparse +import re +import sys +from pathlib import Path +from typing import Dict, FrozenSet, List, NamedTuple, Optional, Set, Tuple + +TOKEN = re.compile(r'\s+|//[^\n]*|--[^\n]*|"(?:[^"\\]|\\.)*"|[A-Za-z_][A-Za-z0-9_]*|[(),;<>]|\S') +INDEXES = {"QUERYABLE", "SEARCHABLE", "SORTABLE"} +PERMISSIONS = {"READ", "CREATE", "WRITE"} + + +class SchemaError(Exception): + pass + + +class Field(NamedTuple): + type: str + indexes: FrozenSet[str] + + +class RecordType(NamedTuple): + fields: Dict[str, Field] + grants: Set[Tuple[str, str]] + + +class Schema(NamedTuple): + roles: Set[str] + types: Dict[str, RecordType] + + +class Tokens: + def __init__(self, text: str): + self.items = [token for token in TOKEN.findall(text) + if not token.isspace() and not token.startswith(("//", "--"))] + self.position = 0 + + def peek(self) -> str: + return self.items[self.position] if self.position < len(self.items) else "" + + def take(self) -> str: + token = self.peek() + if not token: + raise SchemaError("the schema ends early") + self.position += 1 + return token + + def keyword(self) -> str: + return self.take().upper() + + def expect(self, *words: str) -> None: + for word in words: + token = self.take() + if token.upper() != word: + raise SchemaError(f"expected {word}, got {token}") + + def name(self) -> str: + token = self.take() + if token.startswith('"'): + return token[1:-1] + if not re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", token): + raise SchemaError(f"expected a name, got {token}") + return token + + +def parse(text: str) -> Schema: + tokens = Tokens(text) + tokens.expect("DEFINE", "SCHEMA") + schema = Schema(set(), {}) + while tokens.peek(): + word = tokens.keyword() + if word == ";": + continue + if word == "CREATE": + tokens.expect("ROLE") + schema.roles.add(tokens.name()) + elif word == "RECORD": + tokens.expect("TYPE") + name = tokens.name() + if name in schema.types: + raise SchemaError(f"record type {name} is defined twice") + schema.types[name] = record_type(tokens, name) + else: + raise SchemaError(f"expected CREATE ROLE or RECORD TYPE, got {word}") + return schema + + +def record_type(tokens: Tokens, name: str) -> RecordType: + tokens.expect("(") + record = RecordType({}, set()) + while True: + if tokens.peek().upper() == "GRANT": + tokens.take() + permissions = [tokens.keyword()] + while tokens.peek() == ",": + tokens.take() + permissions.append(tokens.keyword()) + unknown = [permission for permission in permissions if permission not in PERMISSIONS] + if unknown: + raise SchemaError(f"{name}: unknown permission {unknown[0]}") + tokens.expect("TO") + role = tokens.name() + record.grants.update((permission, role) for permission in permissions) + else: + field = tokens.name() + if field in record.fields: + raise SchemaError(f"{name}.{field} is defined twice") + kind = data_type(tokens) + indexes = set() + while tokens.peek() not in (",", ")"): + index = tokens.keyword() + if index not in INDEXES: + raise SchemaError(f"{name}.{field}: expected an index, got {index}") + indexes.add(index) + record.fields[field] = Field(kind, frozenset(indexes)) + separator = tokens.take() + if separator == ")": + return record + if separator != ",": + raise SchemaError(f"{name}: expected , or ), got {separator}") + + +def data_type(tokens: Tokens) -> str: + word = tokens.keyword() + if word == "LIST": + tokens.expect("<") + element = data_type(tokens) + tokens.expect(">") + return f"LIST<{element}>" + if word == "ENCRYPTED": + return f"ENCRYPTED {tokens.keyword()}" + if word == "NUMBER" and tokens.peek().upper() == "PREFERRED": + tokens.take() + tokens.expect("AS") + return f"NUMBER PREFERRED AS {tokens.keyword()}" + return word + + +# Production only grows: a type, field, index or grant it has and the snapshot lacks is never read by +# the build that holds the snapshot, so only the snapshot's side is checked. +def gaps(snapshot: Schema, production: Schema) -> List[str]: + found = [f"role {role}: not in production" for role in sorted(snapshot.roles - production.roles)] + for name, record in snapshot.types.items(): + deployed = production.types.get(name) + if deployed is None: + found.append(f"record type {name}: not in production") + continue + for field, wanted in record.fields.items(): + live = deployed.fields.get(field) + if live is None: + found.append(f"{name}.{field}: not in production") + elif live.type != wanted.type: + found.append(f"{name}.{field}: {wanted.type} in the snapshot, {live.type} in production") + elif not wanted.indexes <= live.indexes: + missing = ", ".join(sorted(wanted.indexes - live.indexes)) + found.append(f"{name}.{field}: {missing} not in production") + for permission, role in sorted(record.grants - deployed.grants): + found.append(f"{name}: GRANT {permission} TO {role} not in production") + return found + + +def main(argv: Optional[List[str]] = None) -> int: + parser = argparse.ArgumentParser(description="Compare CloudKit schema files as cktool export-schema writes them.") + commands = parser.add_subparsers(dest="command", required=True) + command = commands.add_parser("covers", help="list what the snapshot declares and production lacks") + command.add_argument("snapshot", type=Path) + command.add_argument("production", type=Path) + args = parser.parse_args(argv) + + try: + snapshot = parse(args.snapshot.read_text(encoding="utf-8")) + production = parse(args.production.read_text(encoding="utf-8")) + except (SchemaError, OSError) as error: + print(f"error: {error}", file=sys.stderr) + return 1 + found = gaps(snapshot, production) + for gap in found: + print(gap, file=sys.stderr) + if found: + return 1 + print(f"production has every record type, field, index, role and grant in {args.snapshot.name}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/release/test_schema.py b/scripts/release/test_schema.py new file mode 100644 index 0000000000..2fa849a8eb --- /dev/null +++ b/scripts/release/test_schema.py @@ -0,0 +1,157 @@ +#!/usr/bin/env python3 + +import io +import tempfile +import unittest +from contextlib import redirect_stderr, redirect_stdout +from pathlib import Path + +import schema + +SNAPSHOT = Path(__file__).resolve().parents[2] / "CloudKit/production-schema.ckdb" +CONNECTION_FIELD = " sshConfigJson BYTES QUERYABLE SORTABLE,\n" +APP_SETTINGS_GRANT = ' GRANT CREATE TO "_icloud",\n' + + +def snapshot() -> str: + return SNAPSHOT.read_text(encoding="utf-8") + + +def without(text: str, line: str) -> str: + assert line in text, line + return text.replace(line, "", 1) + + +def with_type(text: str, definition: str) -> str: + return text.rstrip() + "\n\n" + definition + "\n" + + +def with_role(text: str) -> str: + assert text.startswith("DEFINE SCHEMA\n") + return text.replace("DEFINE SCHEMA\n", "DEFINE SCHEMA\n\n CREATE ROLE Extra;\n", 1) + + +def gaps(snapshot_text: str, production_text: str): + return schema.gaps(schema.parse(snapshot_text), schema.parse(production_text)) + + +class ParseTests(unittest.TestCase): + def test_reads_the_checked_in_snapshot(self): + parsed = schema.parse(snapshot()) + self.assertIn("Connection", parsed.types) + connection = parsed.types["Connection"] + self.assertEqual(connection.fields["sshConfigJson"], + schema.Field("BYTES", frozenset({"QUERYABLE", "SORTABLE"}))) + self.assertEqual(connection.fields["___recordID"], schema.Field("REFERENCE", frozenset({"QUERYABLE"}))) + self.assertEqual(connection.fields["tagIds"], schema.Field("LIST", frozenset())) + self.assertIn(("CREATE", "_icloud"), connection.grants) + + def test_reads_every_form_of_the_grammar(self): + parsed = schema.parse(""" + DEFINE SCHEMA + // a comment + CREATE ROLE Editor; + RECORD TYPE "Note" ( + title STRING QUERYABLE SEARCHABLE SORTABLE, -- another comment + secret ENCRYPTED STRING, + count NUMBER PREFERRED AS INT64, + tags LIST, + "GRANT" STRING, + GRANT READ, WRITE TO "Editor", + GRANT CREATE TO "_icloud" + ); + """) + self.assertEqual(parsed.roles, {"Editor"}) + note = parsed.types["Note"] + self.assertEqual(note.fields["title"].indexes, {"QUERYABLE", "SEARCHABLE", "SORTABLE"}) + self.assertEqual(note.fields["secret"].type, "ENCRYPTED STRING") + self.assertEqual(note.fields["count"].type, "NUMBER PREFERRED AS INT64") + self.assertEqual(note.fields["tags"].type, "LIST") + self.assertEqual(note.fields["GRANT"].type, "STRING") + self.assertEqual(note.grants, {("READ", "Editor"), ("WRITE", "Editor"), ("CREATE", "_icloud")}) + + def test_refuses_a_file_it_cannot_read(self): + for text in ("", "RECORD TYPE A (b STRING);", "DEFINE SCHEMA RECORD TYPE A (b STRING", + "DEFINE SCHEMA RECORD TYPE A (b STRING FAST);", + "DEFINE SCHEMA RECORD TYPE A (b STRING, b INT64);", + "DEFINE SCHEMA RECORD TYPE A (GRANT DELETE TO x);", + "DEFINE SCHEMA DROP TYPE A;"): + with self.subTest(text=text), self.assertRaises(schema.SchemaError): + schema.parse(text) + + +class ProductionAheadPasses(unittest.TestCase): + def test_identical(self): + self.assertEqual(gaps(snapshot(), snapshot()), []) + + def test_a_field_only_production_has(self): + self.assertEqual(gaps(without(snapshot(), CONNECTION_FIELD), snapshot()), []) + + def test_a_record_type_only_production_has(self): + self.assertEqual(gaps(snapshot(), with_type(snapshot(), "RECORD TYPE Extra (name STRING);")), []) + + def test_an_index_only_production_has(self): + lean = snapshot().replace(CONNECTION_FIELD, " sshConfigJson BYTES,\n") + self.assertEqual(gaps(lean, snapshot()), []) + + def test_a_grant_only_production_has(self): + self.assertEqual(gaps(without(snapshot(), APP_SETTINGS_GRANT), snapshot()), []) + + def test_a_role_only_production_has(self): + self.assertEqual(gaps(snapshot(), with_role(snapshot())), []) + + +class SnapshotAheadFails(unittest.TestCase): + def test_a_field_production_lacks(self): + self.assertEqual(gaps(snapshot(), without(snapshot(), CONNECTION_FIELD)), + ["Connection.sshConfigJson: not in production"]) + + def test_a_record_type_production_lacks(self): + self.assertEqual(gaps(with_type(snapshot(), "RECORD TYPE Extra (name STRING);"), snapshot()), + ["record type Extra: not in production"]) + + def test_a_field_type_that_differs(self): + production = snapshot().replace(CONNECTION_FIELD, " sshConfigJson STRING QUERYABLE SORTABLE,\n") + self.assertEqual(gaps(snapshot(), production), + ["Connection.sshConfigJson: BYTES in the snapshot, STRING in production"]) + + def test_an_index_production_lacks(self): + production = snapshot().replace(CONNECTION_FIELD, " sshConfigJson BYTES SORTABLE,\n") + self.assertEqual(gaps(snapshot(), production), ["Connection.sshConfigJson: QUERYABLE not in production"]) + + def test_a_grant_production_lacks(self): + self.assertEqual(gaps(snapshot(), without(snapshot(), APP_SETTINGS_GRANT)), + ["AppSettings: GRANT CREATE TO _icloud not in production"]) + + def test_a_role_production_lacks(self): + self.assertEqual(gaps(with_role(snapshot()), snapshot()), ["role Extra: not in production"]) + + +class CommandTests(unittest.TestCase): + def run_covers(self, snapshot_text: str, production_text: str): + with tempfile.TemporaryDirectory() as directory: + snapshot_file = Path(directory) / "snapshot.ckdb" + production_file = Path(directory) / "production.ckdb" + snapshot_file.write_text(snapshot_text, encoding="utf-8") + production_file.write_text(production_text, encoding="utf-8") + out, err = io.StringIO(), io.StringIO() + with redirect_stdout(out), redirect_stderr(err): + code = schema.main(["covers", str(snapshot_file), str(production_file)]) + return code, out.getvalue(), err.getvalue() + + def test_passes_when_production_is_ahead(self): + code, _, err = self.run_covers(without(snapshot(), CONNECTION_FIELD), snapshot()) + self.assertEqual((code, err), (0, "")) + + def test_fails_and_names_each_gap(self): + code, _, err = self.run_covers(snapshot(), without(snapshot(), CONNECTION_FIELD)) + self.assertEqual((code, err), (1, "Connection.sshConfigJson: not in production\n")) + + def test_fails_on_a_file_it_cannot_parse(self): + code, _, err = self.run_covers(snapshot(), "not a schema") + self.assertEqual(code, 1) + self.assertTrue(err.startswith("error: "), err) + + +if __name__ == "__main__": + unittest.main()