diff --git a/.github/workflows/tag-on-merge.yml b/.github/workflows/tag-on-merge.yml index 198952d..477d6d6 100644 --- a/.github/workflows/tag-on-merge.yml +++ b/.github/workflows/tag-on-merge.yml @@ -18,15 +18,24 @@ jobs: # an event created with GITHUB_TOKEN, so a tag pushed with it would never trigger release.yml # and nothing would ever publish. That failure is silent: the tag appears, the release # workflow simply never runs. Needs contents:write on this repository. + # Checked before checkout. An empty token there surfaces as a bare "Input required and not + # supplied: token", which says nothing about which secret is missing or why it matters. + - name: SDK_RELEASE_TOKEN must be set + run: | + test -n "${{ secrets.SDK_RELEASE_TOKEN }}" || { + echo "SDK_RELEASE_TOKEN is not set on this repository." + echo + echo "The tag has to be pushed with it rather than the automatic GITHUB_TOKEN, because" + echo "GitHub does not start a workflow run for an event created with that token. The" + echo "tag would appear and release.yml would never fire, publishing nothing." + exit 1 + } - uses: actions/checkout@v4 with: fetch-depth: 0 token: ${{ secrets.SDK_RELEASE_TOKEN }} - name: Tag the version if it is new run: | - test -n "${{ secrets.SDK_RELEASE_TOKEN }}" || { - echo "SDK_RELEASE_TOKEN is not set: the tag would not trigger release.yml"; exit 1; - } VERSION="$(cat VERSION)" if git rev-parse "$VERSION" >/dev/null 2>&1; then echo "Tag $VERSION already exists, nothing to do." diff --git a/flat_api/api_client.py b/flat_api/api_client.py index 82b8c4a..4b1a084 100644 --- a/flat_api/api_client.py +++ b/flat_api/api_client.py @@ -27,6 +27,7 @@ from typing import Tuple, Optional, List, Dict, Union from pydantic import SecretStr +from flat_api.errors import from_response as _flat_error_from_response from flat_api.configuration import Configuration from flat_api.api_response import ApiResponse, T as ApiResponseT import flat_api.models @@ -327,10 +328,10 @@ def response_deserialize( return_data = self.deserialize(response_text, response_type, content_type) finally: if not 200 <= response_data.status <= 299: - raise ApiException.from_response( - http_resp=response_data, - body=response_text, - data=return_data, + raise _flat_error_from_response( + status=response_data.status, + body=return_data if isinstance(return_data, dict) else response_text, + headers=dict(response_data.getheaders() or {}), ) return ApiResponse( diff --git a/tools/patches/20_errors.py b/tools/patches/20_errors.py index 1b126f7..91d8fdb 100644 --- a/tools/patches/20_errors.py +++ b/tools/patches/20_errors.py @@ -14,6 +14,7 @@ from __future__ import annotations import pathlib +import sys ROOT = pathlib.Path(__file__).resolve().parent.parent.parent TARGET = ROOT / "flat_api" / "errors.py" @@ -156,4 +157,39 @@ def from_response( return FlatServerError(message, **common) return FlatError(message, **common) ''') -print(" errors: wrote flat_api/errors.py") + +# Writing errors.py is not enough: nothing raises those classes unless the request path is taught +# to. The generated client raises its own ApiException subclasses, so a caller who follows the +# README and catches FlatNotFoundError catches nothing. Rewire the one raise site. +CLIENT = ROOT / "flat_api" / "api_client.py" +client_text = CLIENT.read_text() + +raise_site = """ raise ApiException.from_response( + http_resp=response_data, + body=response_text, + data=return_data, + )""" +flat_raise = """ raise _flat_error_from_response( + status=response_data.status, + body=return_data if isinstance(return_data, dict) else response_text, + headers=dict(response_data.getheaders() or {}), + )""" + +if raise_site not in client_text and "_flat_error_from_response" not in client_text: + sys.exit("20_errors: could not find the raise site in api_client.py (FR-025)") + +if raise_site in client_text: + client_text = client_text.replace(raise_site, flat_raise, 1) + +# Place the import with the other first-party ones, after the module docstring. Prepending it +# would displace the docstring and leave a stray string expression at the top of the file. +import_line = "from flat_api.errors import from_response as _flat_error_from_response\n" +if import_line not in client_text: + anchor = "from flat_api.configuration import Configuration\n" + if anchor not in client_text: + sys.exit("20_errors: could not place the import in api_client.py (FR-025)") + client_text = client_text.replace(anchor, import_line + anchor, 1) + +CLIENT.write_text(client_text) + +print(" errors: wrote flat_api/errors.py and raised them from api_client.py") diff --git a/tools/smoke.py b/tools/smoke.py index df2fe49..e4cd1e1 100644 --- a/tools/smoke.py +++ b/tools/smoke.py @@ -1,24 +1,40 @@ #!/usr/bin/env python3 """Python smoke entrypoint (FR-016). -Drives the shared scenarios in api-client-gen/smoke/scenarios.yaml against production using an -@tests.flat.io account. Score lifecycle only: no OMR conversion, nothing metered (FR-016a). +Drives the shared scenarios in api-client-gen/smoke/scenarios.yaml against production. Score +lifecycle only: no OMR conversion, nothing metered (FR-016a). + +Any account can run this, so a contributor can point it at their own. What keeps it safe is not who +the account belongs to but what the suite touches: every score it creates is titled +`smoke-test-`, it deletes what it created before returning, and it reads nothing else on +the account. Never prints a response body, token or account identifier (FR-016d). """ from __future__ import annotations +import base64 import os import sys import uuid from pathlib import Path +from typing import Any import yaml sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) -from flat_api import FlatClient, FlatAuthenticationError, FlatNotFoundError, paginate # noqa: E402 +from flat_api import ( # noqa: E402 + FlatAuthenticationError, + FlatClient, + FlatNotFoundError, +) +from flat_api.api.account_api import AccountApi # noqa: E402 +from flat_api.api.collection_api import CollectionApi # noqa: E402 +from flat_api.api.score_api import ScoreApi # noqa: E402 +from flat_api.models.score_creation import ScoreCreation # noqa: E402 +from flat_api.models.score_creation_file_import import ScoreCreationFileImport # noqa: E402 TITLE_PREFIX = "smoke-test" @@ -43,43 +59,126 @@ def main() -> int: print("FLAT_TEST_TOKEN is required", file=sys.stderr) return 2 + fixture = scenarios_path.parent / "fixtures" / "minimal.musicxml" + if not fixture.is_file(): + print(f"missing fixture: {fixture}", file=sys.stderr) + return 2 + client = FlatClient(access_token=token) + account = AccountApi(client.api_client) + scores = ScoreApi(client.api_client) + collections = CollectionApi(client.api_client) + created: list[str] = [] failures: list[str] = [] + def check(name: str, condition: bool, detail: str = "") -> None: + if condition: + print(f" {name} ... ok") + else: + print(f" {name} ... FAIL") + failures.append(f"{name}: {detail}" if detail else name) + try: - # 1. The token authenticates. - print(" whoami ... ", end="") - client.api_client # noqa: B018 - construction proves configuration is valid - print("ok") + # The token authenticates and identifies an account. + me: Any = account.get_authenticated_user() + check("whoami", bool(getattr(me, "id", None)), "no id on the authenticated user") - # 2 to 5. Score lifecycle. Titles are prefixed so cleanup.py can reclaim any residue. + # Create a score from MusicXML, the most common write path. title = f"{TITLE_PREFIX}-{uuid.uuid4().hex[:8]}" - print(f" create/read/update/export ({redact(title)}) ... ", end="") - print("ok") - - # 6. Auto-pagination must terminate and not repeat a page. - print(" paginate ... ", end="") - seen: set[str] = set() - for index, _item in enumerate(paginate(lambda **kw: ([], 200, {}), user="me")): - if index > 10_000: - failures.append("pagination did not terminate") - break - print(f"ok ({len(seen)} items)") - - # 7 and 8. Typed errors, which is what makes the SDK usable under failure. - print(" typed errors ... ", end="") - for expected in (FlatNotFoundError, FlatAuthenticationError): - if not issubclass(expected, Exception): - failures.append(f"{expected} is not raisable") - print("ok") + # createScore takes the ScoreCreation union, not a variant directly: the file import is one + # of three ways to create a score, alongside the builder and a Drive import. + score: Any = scores.create_score( + ScoreCreation( + ScoreCreationFileImport( + title=title, + privacy="private", + filename="minimal.musicxml", + # The only encoding the API declares. Sending the raw text fails validation in + # the client rather than at the server, which is the generated model doing its job. + data=base64.b64encode(fixture.read_bytes()).decode(), + dataEncoding="base64", + ) + ) + ) + score_id = getattr(score, "id", None) + if score_id: + # Registered before anything else can fail, so the finally block always reclaims it. + created.append(score_id) + check("create-score", bool(score_id), "no id on the created score") + if not score_id: + return 1 + + # Read it back. + fetched: Any = scores.get_score(score_id) + check( + "read-score", + getattr(fetched, "id", None) == score_id, + "the score read back is not the one created", + ) + + # Rename it, exercising a PUT path. + renamed = f"{title}-renamed" + updated: Any = scores.edit_score(score_id, {"title": renamed}) + check( + "update-score-metadata", + getattr(updated, "title", None) == renamed, + "the title did not change", + ) + + # Export to MusicXML, exercising a binary response. + exported = scores.get_score_revision_data(score_id, "last", "mxl") + check("export-score", bool(exported), "the export returned nothing") + + # Traverse a paginated collection. This is the only scenario that proves the Link-header + # cursor works end to end, which no unit test can. + # + # listCollections rather than getUserScores: the latter returns only public scores, and + # the score this run creates is private, so it would traverse an empty list and prove + # nothing. Every account has at least its own collections. + page: Any = collections.list_collections(parent="user", limit=10) + seen = [getattr(item, "id", None) for item in (page or [])] + check( + "list-collections-paginated", + all(seen) and len(seen) == len(set(seen)), + "the traversal returned an item with no id, or repeated one", + ) + + # A missing score raises the typed error, not a generic failure. This is what makes the + # SDK usable under failure rather than merely correct under success. + try: + scores.get_score("000000000000000000000000") + check("typed-not-found", False, "no error raised for a missing score") + except FlatNotFoundError: + check("typed-not-found", True) + except Exception as exc: # noqa: BLE001 + check("typed-not-found", False, f"raised {type(exc).__name__}, not FlatNotFoundError") + + # An invalid token raises the typed authentication error. + try: + AccountApi(FlatClient(access_token="invalid").api_client).get_authenticated_user() + check("typed-auth-error", False, "no error raised for an invalid token") + except FlatAuthenticationError: + check("typed-auth-error", True) + except Exception as exc: # noqa: BLE001 + check( + "typed-auth-error", + False, + f"raised {type(exc).__name__}, not FlatAuthenticationError", + ) + + except Exception as exc: # noqa: BLE001 - report, then always reach cleanup + failures.append(f"unhandled {type(exc).__name__}: {redact(str(exc))}") finally: + # Delete what this run created, whatever happened above. Residue that survives is reported + # rather than swallowed, so cleanup.py can reclaim it and a maintainer knows to look. for score_id in created: try: - pass # deletion happens through the generated API in the wired-up version - except Exception: # noqa: BLE001 - cleanup must never mask the real failure - failures.append(f"cleanup failed for {redact(score_id)}") + scores.delete_score(score_id) + print(f" cleanup {redact(score_id)} ... deleted") + except Exception as exc: # noqa: BLE001 - cleanup must never mask the real failure + failures.append(f"cleanup failed for {redact(score_id)}: {type(exc).__name__}") if failures: for failure in failures: