From 560c3b8f1f1cdca7f98380f18de7c7aec9dd8665 Mon Sep 17 00:00:00 2001 From: Conrad Date: Mon, 10 Aug 2026 11:15:13 -0400 Subject: [PATCH 01/10] build: Add a make target that regenerates schema.graphql MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit schema.graphql is a generated artifact, but nothing in the repo produced it — the strawberry CLI is not an installed extra, so the only way to refresh it was to reconstruct the print_schema incantation by hand. Wrap it in a script so the SDL stays reproducible from the Python types. --- Makefile | 4 ++++ scripts/export_schema.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 scripts/export_schema.py diff --git a/Makefile b/Makefile index c1493c0..79b0abb 100644 --- a/Makefile +++ b/Makefile @@ -47,6 +47,10 @@ api: docker run -d --name api --network cvh-backend-network --network-alias cvh-backend -p 8000:8000 -e SYNC_DATA_DIR=/tmp/sync-data api @echo "API container is up and running on port 8000 (http://0.0.0.0:8000/metadata)." +schema: + @echo "Regenerating schema.graphql from the Strawberry schema..." + uv run python scripts/export_schema.py + wool: @echo "Building the wool worker Docker image (cfdb-wool, linux/amd64)..." docker build --platform linux/amd64 -t cfdb-wool -f Dockerfile.wool . diff --git a/scripts/export_schema.py b/scripts/export_schema.py new file mode 100644 index 0000000..85fd086 --- /dev/null +++ b/scripts/export_schema.py @@ -0,0 +1,29 @@ +"""Regenerate the checked-in GraphQL SDL from the Strawberry schema. + +``schema.graphql`` is a generated artifact — edit the Python types and run +this script (``make schema``) rather than editing the SDL by hand. +``tests/test_schema.py`` fails when the two drift apart. +""" + +from pathlib import Path + +from strawberry.printer import print_schema + +from cfdb.api.gql.schema import schema + + +SCHEMA_PATH = Path(__file__).resolve().parent.parent / "schema.graphql" + + +def render() -> str: + """Return the SDL exactly as it is written to ``schema.graphql``.""" + return print_schema(schema) + "\n" + + +def main() -> None: + SCHEMA_PATH.write_text(render()) + print(f"Wrote {SCHEMA_PATH}") + + +if __name__ == "__main__": + main() From 842379bdbca9d25406a9635c9de93fb6b39a5e06 Mon Sep 17 00:00:00 2001 From: Conrad Date: Mon, 10 Aug 2026 11:15:23 -0400 Subject: [PATCH 02/10] fix!: Expose sizeInBytes as a 64-bit BigInt scalar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GraphQL fixes Int at 32 bits, so any file over ~2.1 GB failed to serialize: the field resolved to null and the response carried a per-field error, degrading a whole page of results to a partial one. Every ENCODE .hic file (6-51 GB) and the larger 4DN mcools hit this, so it is the common case for contact maps rather than an edge case. BigInt serializes as a JSON number rather than a string so sizeInBytes stays usable in client arithmetic with no parsing step. The usual objection to that — values above 2^53-1 lose precision in JavaScript — does not bind, because 2^53 bytes is roughly 9 PB. The input filter carries the same scalar. Widening only the output would leave exactly the files it newly exposes unfilterable by size. The override is keyed by (model, field name) rather than by annotation so that widening one model's int does not silently widen every other int in the schema. BREAKING CHANGE: sizeInBytes is typed BigInt instead of Int on both FileMetadataType and FileMetadataInput. A query declaring a variable as Int for that argument now fails validation and must declare BigInt, and generated clients must be regenerated. Clients that only read sizeInBytes out of the response are unaffected — the wire form is still a JSON number. --- src/cfdb/api/gql/inputs.py | 7 +++- src/cfdb/api/gql/types.py | 74 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/cfdb/api/gql/inputs.py b/src/cfdb/api/gql/inputs.py index 953c843..27974a9 100644 --- a/src/cfdb/api/gql/inputs.py +++ b/src/cfdb/api/gql/inputs.py @@ -2,6 +2,8 @@ import strawberry +from cfdb.api.gql.types import BigInt + @strawberry.input class AnatomyInput: @@ -243,7 +245,10 @@ class FileMetadataInput: project_local_id: list[str] | None = None persistent_id: list[str] | None = None creation_time: list[str] | None = None - size_in_bytes: list[int] | None = None + # ``BigInt``, not ``int``: a 32-bit ``Int`` filter cannot name the size of + # a file over ~2 GB, which would leave exactly the files the widened + # output field exposes unfilterable. + size_in_bytes: list[BigInt] | None = None sha256: list[str] | None = None md5: list[str] | None = None filename: list[str] | None = None diff --git a/src/cfdb/api/gql/types.py b/src/cfdb/api/gql/types.py index 84cb0cf..058adbd 100644 --- a/src/cfdb/api/gql/types.py +++ b/src/cfdb/api/gql/types.py @@ -18,6 +18,62 @@ class ObjectIdScalar: ... +# Bounds of a signed 64-bit integer — the range ``BigInt`` accepts, matching +# what MongoDB stores in a BSON int64 and what a C2M2 file size can be. +_INT64_MIN = -(2**63) +_INT64_MAX = 2**63 - 1 + +# Above this magnitude a JSON number is no longer exactly representable in an +# IEEE-754 double, which is the only numeric type a browser client has. No +# byte size can reach it (2**53 bytes is ~9 PB), so this is a guard against a +# non-size value being routed through ``BigInt``, not a live concern for +# ``size_in_bytes``. +_JS_SAFE_INTEGER_MAX = 2**53 - 1 + + +def _coerce_big_int(value): + """Coerce a ``BigInt`` value in either direction, rejecting non-integers. + + Serialization and parsing share one implementation because the wire form + is a JSON number: the value that goes out is the value that comes back. + ``bool`` is excluded explicitly because it is an ``int`` subclass in + Python and ``true`` is not a size. + """ + if isinstance(value, bool) or not isinstance(value, int): + raise ValueError(f"BigInt cannot represent non-integer value: {value!r}") + if not _INT64_MIN <= value <= _INT64_MAX: + raise ValueError( + f"BigInt cannot represent non 64-bit signed integer value: {value}" + ) + return value + + +@strawberry.scalar( + description=( + "A signed 64-bit integer, serialized as a JSON number. Widens the " + "GraphQL `Int` scalar, which the specification fixes at 32 bits and " + "which therefore cannot represent a file larger than ~2 GB. Values " + f"beyond {_JS_SAFE_INTEGER_MAX} (2^53-1) exceed what an IEEE-754 " + "double represents exactly and would lose precision in a JavaScript " + "client; byte sizes cannot reach that magnitude." + ), + serialize=_coerce_big_int, + parse_value=_coerce_big_int, +) +class BigInt: + """A signed 64-bit integer represented as a JSON number in GraphQL.""" + + ... + + +# Model fields whose Python ``int`` annotation must NOT become a GraphQL +# ``Int``. Keyed by ``(model, field name)`` so widening one model's field +# does not silently widen an unrelated field that happens to share its name. +_SCALAR_OVERRIDES: dict[tuple[type, str], object] = { + (FileMetadataModel, "size_in_bytes"): BigInt, +} + + @strawberry.type class DistinctFieldType: field: str @@ -104,6 +160,18 @@ def _resolve_json_type(field_type): return field_type +def _substitute_scalar(field_type, scalar): + """Replace a field's scalar type, preserving an ``Optional`` wrapper. + + Overridden fields are declared on the model as ``T`` or ``Optional[T]``, + so no deeper nesting needs handling. + """ + args = getattr(field_type, "__args__", None) + if args and type(None) in args: + return Optional[scalar] + return scalar + + def _rebuild_type(field_type, model_cls, strawberry_cls): """Replace a BaseModel class inside a (possibly nested) type annotation with its Strawberry equivalent, preserving Optional/List wrappers.""" @@ -146,6 +214,12 @@ def wrapper(cls): if name: cls.__name__ = f"{name}Type" for field_name, field_type in get_type_hints(model).items(): + override = _SCALAR_OVERRIDES.get((model, field_name)) + if override is not None: + cls.__annotations__[field_name] = _substitute_scalar( + field_type, override + ) + continue model_cls, _ = _find_basemodel(field_type) if model_cls is None: if field_type is ObjectId: From cc5eef07d60c89f226791ca412e7b61826b2e272 Mon Sep 17 00:00:00 2001 From: Conrad Date: Mon, 10 Aug 2026 11:15:29 -0400 Subject: [PATCH 03/10] build: Regenerate GraphQL schema --- schema.graphql | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/schema.graphql b/schema.graphql index 691a532..40055ba 100644 --- a/schema.graphql +++ b/schema.graphql @@ -22,6 +22,11 @@ type AssayTypeType { description: String } +""" +A signed 64-bit integer, serialized as a JSON number. Widens the GraphQL `Int` scalar, which the specification fixes at 32 bits and which therefore cannot represent a file larger than ~2 GB. Values beyond 9007199254740991 (2^53-1) exceed what an IEEE-754 double represents exactly and would lose precision in a JavaScript client; byte sizes cannot reach that magnitude. +""" +scalar BigInt + input BiosampleInput { idNamespace: [String!] = null localId: [String!] = null @@ -407,7 +412,7 @@ input FileMetadataInput { projectLocalId: [String!] = null persistentId: [String!] = null creationTime: [String!] = null - sizeInBytes: [Int!] = null + sizeInBytes: [BigInt!] = null sha256: [String!] = null md5: [String!] = null filename: [String!] = null @@ -443,7 +448,7 @@ type FileMetadataType { projectLocalId: String! persistentId: String creationTime: String - sizeInBytes: Int + sizeInBytes: BigInt sha256: String md5: String filename: String! @@ -555,4 +560,4 @@ type SubjectType { race: [String!]! taxonomy: NcbiTaxonomyType extra: EnrichedSubjectType -} \ No newline at end of file +} From e175325154253b1fb8c74a995f60650098376fae Mon Sep 17 00:00:00 2001 From: Conrad Date: Mon, 10 Aug 2026 11:15:29 -0400 Subject: [PATCH 04/10] docs: Document the BigInt scalar and what it breaks for clients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Record the wire-representation decision where a client author will look for it — number over string, and why the JavaScript precision ceiling does not bind on byte sizes — alongside the three concrete ways a client that hard-codes Int breaks. --- README.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 1313255..e81bbe8 100644 --- a/README.md +++ b/README.md @@ -84,6 +84,7 @@ Run `./certs/generate-certs.sh --help` for full usage information. | `make api` | Build and start the API container | | `make materialize-files` | Manually materialize all file metadata (usually done via sync) | | `make materialize-dcc DCC=hubmap` | Materialize a single DCC | +| `make schema` | Regenerate the checked-in `schema.graphql` from the Strawberry schema | | `make certs` | Generate TLS certificates for production | | `make mongodb-prod` | Start MongoDB with TLS/X.509 authentication | | `make api-prod` | Start API with X.509 client certificate | @@ -356,6 +357,19 @@ Single file lookup: `{ file(id: "507f1f77bcf86cd799439011") { filename accessUrl File count for a filter: `{ fileCount(input: [{ dcc: [{ dccAbbreviation: ["4DN"] }] }]) }` — returns the number of matching files without fetching any documents. It accepts the same `FileMetadataInput` filter shape as `files`; with no `input` it counts every file. +### Custom Scalars + +| Scalar | Wire form | Used by | +|--------|-----------|---------| +| `ObjectIdScalar` | JSON string | `file(id:)` | +| `BigInt` | JSON number | `sizeInBytes`, on both `FileMetadataType` and `FileMetadataInput` | + +`BigInt` is a signed 64-bit integer. The GraphQL specification fixes `Int` at 32 bits, so a file larger than 2,147,483,647 bytes (~2.1 GB) could not be represented at all: the field resolved to `null` and contributed a `Int cannot represent non 32-bit signed integer value` entry to the response's `errors` array, degrading a whole page of results to a partial one. That affects every ENCODE `.hic` file (6–51 GB) and the larger 4DN mcools, so it is the common case for contact maps rather than an edge case. The input filter carries the same scalar — a 32-bit filter would leave exactly the files the widened output field exposes unfilterable. + +`BigInt` stays a JSON **number** on the wire rather than a string, so `sizeInBytes` remains directly usable in client-side arithmetic and comparisons with no parsing step. The usual objection to that choice — values above `Number.MAX_SAFE_INTEGER` (2^53-1) lose precision in JavaScript — does not bind here: 2^53 bytes is ~9 PB, far above any file this API serves. The scalar rejects non-integers (including `true`/`false`) and anything outside the signed 64-bit range on both input and output. + +**This is a breaking schema change.** A client that hard-codes `Int` breaks in three ways: a query declaring `query Q($s: [Int!])` and passing it to `sizeInBytes` now fails variable-type validation and must declare `[BigInt!]`; generated clients must re-run codegen against the new SDL; and any client validating responses against a stored copy of the schema must refresh it. A client that merely *reads* `sizeInBytes` out of the JSON response needs no change — it was already receiving a JSON number, and now receives a correct one instead of `null`. + ### Query Mechanics The GraphQL API uses an implicit OR/AND clause system for building MongoDB queries: @@ -460,7 +474,7 @@ The central entity representing a stable digital asset. | `dcc` | Dcc | The Data Coordinating Center that produced this file | | `collections` | Collection[] | Collections containing this file | | `project` | Project? | The primary project within which this file was created | -| `size_in_bytes` | int? | File size | +| `size_in_bytes` | int? | File size, exposed as the 64-bit `BigInt` scalar (see [Custom Scalars](#custom-scalars)) | | `sha256` | string? | SHA-256 checksum (preferred) | | `md5` | string? | MD5 checksum (if SHA-256 unavailable) | | `filename` | string | Filename without path | From 8fd440aac5cd8ea6d8392befba1091d56dbb2333 Mon Sep 17 00:00:00 2001 From: Conrad Date: Mon, 10 Aug 2026 11:33:02 -0400 Subject: [PATCH 05/10] test: Cover the BigInt scalar on both sides of the size filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pins the reported symptom (a 6.2 GB file resolving to null plus a per-field error), the round trip across the declared 64-bit range, and the input filter matching a size the old Int could not name. Also pins two things that are easy to lose silently: the widening reached the input filter as well as the output type, and it did not spread to the neighbouring counts and pagination arguments, which must stay Int. The SDL drift test closes a gap that predates this change — schema.graphql is generated but nothing failed when it went stale, so a type change that skipped regeneration could ship a wrong public contract. --- tests/test_schema.py | 414 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 413 insertions(+), 1 deletion(-) diff --git a/tests/test_schema.py b/tests/test_schema.py index ad6ab3e..996e024 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -4,12 +4,14 @@ import asyncio import logging +from pathlib import Path import pytest from hypothesis import HealthCheck, given, settings from hypothesis import strategies as st from mongomock_motor import AsyncMongoMockClient from starlette.testclient import TestClient +from strawberry.printer import print_schema from cfdb import api from cfdb.api import main @@ -51,7 +53,9 @@ def test_from_pydantic_should_convert_nested_model_lists_and_leave_json_untouche assert result.collections[0].extra.hubmap.metadata == {"k": "v", "n": 1} -def _make_file_doc(local_id: str, submission: str = "hubmap") -> dict: +def _make_file_doc( + local_id: str, submission: str = "hubmap", size_in_bytes: int | None = None +) -> dict: """Return a minimal file document that satisfies FileMetadataModel.""" return { "id_namespace": "ns", @@ -61,6 +65,7 @@ def _make_file_doc(local_id: str, submission: str = "hubmap") -> dict: "filename": f"{local_id}.bam", "submission": submission, "data_access_level": "public", + "size_in_bytes": size_in_bytes, "dcc": { "dcc_name": submission.upper(), "dcc_abbreviation": submission, @@ -69,6 +74,13 @@ def _make_file_doc(local_id: str, submission: str = "hubmap") -> dict: } +def _named_type(type_ref: dict) -> str | None: + """Unwrap an introspection type reference down to its named type.""" + while type_ref is not None and type_ref.get("name") is None: + type_ref = type_ref.get("ofType") + return type_ref.get("name") if type_ref else None + + def _make_distinct_doc(local_id: str, dcc_name: str, submission: str = "hubmap") -> dict: """Return a file document with a configurable dcc_name for distinct-values tests.""" return { @@ -1924,3 +1936,403 @@ def test_files_should_answer_with_a_graphql_error_when_pagination_is_out_of_rang body = response.json() assert body["data"] is None assert expected in body["errors"][0]["message"] + + +# The exact size, in bytes, of the ENCODE .hic file named in issue #83 — +# above the 2**31-1 ceiling GraphQL's ``Int`` scalar imposes. +_ISSUE_83_SIZE = 6262125716 + + +class TestSizeInBytesScalar: + """Coverage of the 64-bit ``BigInt`` scalar carrying ``sizeInBytes``.""" + + @pytest.fixture(autouse=True) + def _patch_cutover(self, mocker): + """No-op ``locks.wait_for_cutover`` for every test in this class.""" + mocker.patch.object(locks, "wait_for_cutover", return_value=None) + + @pytest.mark.asyncio + async def test_size_in_bytes_should_resolve_a_file_above_the_int32_ceiling( + self, mock_db + ): + """Test a file larger than 2 GB reports its true size. + + Given: + The 6,262,125,716-byte ENCODE file from issue #83, whose size + exceeds what a 32-bit GraphQL Int can represent. + When: + The GraphQL files query selects sizeInBytes. + Then: + It should return the exact size with no errors, rather than the + null-plus-per-field-error the Int scalar produced. + """ + # Arrange + mock_db.files.docs = [_make_file_doc("f1", size_in_bytes=_ISSUE_83_SIZE)] + + # Act + result = await schema.execute( + "{ files { items { sizeInBytes } } }", + ) + + # Assert + assert result.errors is None + assert result.data["files"]["items"][0]["sizeInBytes"] == _ISSUE_83_SIZE + + @pytest.mark.parametrize( + "size", + [ + 0, + 4096, + 2**31 - 1, + 2**31, + 2**53 - 1, + 2**63 - 1, + ], + ids=["zero", "small", "int32-max", "int32-max-plus-one", "js-safe-max", "int64-max"], + ) + @pytest.mark.asyncio + async def test_size_in_bytes_should_round_trip_across_the_64_bit_range( + self, mock_db, size + ): + """Test sizes spanning the declared range survive serialization intact. + + Given: + A file whose size sits at a notable point of the 64-bit range — + zero, an ordinary size, either side of the old Int ceiling, the + JavaScript safe-integer maximum, and the 64-bit maximum. + When: + The GraphQL files query selects sizeInBytes. + Then: + It should return that exact value with no errors. + """ + # Arrange + mock_db.files.docs = [_make_file_doc("f1", size_in_bytes=size)] + + # Act + result = await schema.execute("{ files { items { sizeInBytes } } }") + + # Assert + assert result.errors is None + assert result.data["files"]["items"][0]["sizeInBytes"] == size + + @pytest.mark.asyncio + async def test_size_in_bytes_should_be_null_when_the_file_records_no_size( + self, mock_db + ): + """Test an absent size still resolves to null rather than an error. + + Given: + A file whose size_in_bytes is unset. + When: + The GraphQL files query selects sizeInBytes. + Then: + It should return null with no errors, as the field is optional. + """ + # Arrange + mock_db.files.docs = [_make_file_doc("f1")] + + # Act + result = await schema.execute("{ files { items { sizeInBytes } } }") + + # Assert + assert result.errors is None + assert result.data["files"]["items"][0]["sizeInBytes"] is None + + @pytest.mark.asyncio + async def test_size_in_bytes_should_null_only_its_own_field_when_unrepresentable( + self, mock_db + ): + """Test an out-of-range stored size does not take down the page. + + Given: + Two files, the first holding a size beyond the 64-bit range and + the second an ordinary size. + When: + The GraphQL files query selects sizeInBytes alongside other + fields. + Then: + It should null only the offending file's sizeInBytes, report the + failure at that field's path, and return every other field and + the sibling file untouched. + """ + # Arrange + mock_db.files.docs = [ + _make_file_doc("f1", size_in_bytes=2**70), + _make_file_doc("f2", size_in_bytes=1234), + ] + + # Act + result = await schema.execute( + "{ files { items { localId sizeInBytes } } }", + ) + + # Assert + assert [e.path for e in result.errors] == [ + ["files", "items", 0, "sizeInBytes"] + ] + items = result.data["files"]["items"] + assert items[0] == {"localId": "f1", "sizeInBytes": None} + assert items[1] == {"localId": "f2", "sizeInBytes": 1234} + + @pytest.mark.asyncio + async def test_files_should_filter_on_a_size_above_the_int32_ceiling(self, mock_db): + """Test a literal size filter selects a file larger than 2 GB. + + Given: + One file at the issue #83 size and one ordinary file. + When: + The GraphQL files query filters on that size as a query literal. + Then: + It should return only the large file, so sizes above the old Int + ceiling are filterable and not merely readable. + """ + # Arrange + mock_db.files.docs = [ + _make_file_doc("big", size_in_bytes=_ISSUE_83_SIZE), + _make_file_doc("small", size_in_bytes=1234), + ] + + # Act + result = await schema.execute( + "{ files(input: [{ sizeInBytes: [%d] }])" + " { totalCount items { localId } } }" % _ISSUE_83_SIZE, + ) + + # Assert + assert result.errors is None + assert result.data["files"]["totalCount"] == 1 + assert result.data["files"]["items"][0]["localId"] == "big" + + @pytest.mark.asyncio + async def test_files_should_filter_on_a_large_size_passed_as_a_variable( + self, mock_db + ): + """Test a BigInt variable filters as a query literal does. + + Given: + One file at the issue #83 size and one ordinary file. + When: + The GraphQL files query filters on that size through a + [BigInt!] variable, which GraphQL coerces by a different path + than a query literal. + Then: + It should return only the large file. + """ + # Arrange + mock_db.files.docs = [ + _make_file_doc("big", size_in_bytes=_ISSUE_83_SIZE), + _make_file_doc("small", size_in_bytes=1234), + ] + + # Act + result = await schema.execute( + "query Files($sizes: [BigInt!]) {" + " files(input: [{ sizeInBytes: $sizes }])" + " { totalCount items { localId } } }", + variable_values={"sizes": [_ISSUE_83_SIZE]}, + ) + + # Assert + assert result.errors is None + assert result.data["files"]["totalCount"] == 1 + assert result.data["files"]["items"][0]["localId"] == "big" + + @pytest.mark.asyncio + async def test_files_should_reject_an_int_typed_variable_for_the_size_filter( + self, mock_db + ): + """Test the documented break for clients still declaring Int. + + Given: + A files query declaring its size-filter variable as [Int!], as a + client written against the pre-BigInt schema would. + When: + The query is executed. + Then: + It should fail validation naming the expected [BigInt!] type, + rather than silently truncating at the 32-bit ceiling. + """ + # Arrange + mock_db.files.docs = [_make_file_doc("big", size_in_bytes=_ISSUE_83_SIZE)] + + # Act + result = await schema.execute( + "query Files($sizes: [Int!]) {" + " files(input: [{ sizeInBytes: $sizes }]) { totalCount } }", + variable_values={"sizes": [1234]}, + ) + + # Assert + assert result.data is None + assert "expecting type '[BigInt!]'" in result.errors[0].message + + @pytest.mark.parametrize( + "literal", + ["true", str(2**63), str(-(2**63) - 1)], + ids=["boolean", "above-int64-max", "below-int64-min"], + ) + @pytest.mark.asyncio + async def test_files_should_reject_a_size_filter_literal_outside_the_scalar( + self, mock_db, literal + ): + """Test the scalar refuses literals it cannot represent. + + Given: + A size filter literal that is a boolean, or an integer one step + beyond either end of the 64-bit range. + When: + The GraphQL files query is executed with that literal. + Then: + It should reject the query outright with a BigInt error rather + than coercing the value. + """ + # Arrange + mock_db.files.docs = [_make_file_doc("f1", size_in_bytes=1234)] + + # Act + result = await schema.execute( + f"{{ files(input: [{{ sizeInBytes: [{literal}] }}]) {{ totalCount }} }}", + ) + + # Assert + assert result.data is None + assert "BigInt cannot represent" in result.errors[0].message + + @pytest.mark.parametrize( + "value", ["6262125716", 1.5], ids=["string", "non-integral-float"] + ) + @pytest.mark.asyncio + async def test_files_should_reject_a_non_integer_size_filter_variable( + self, mock_db, value + ): + """Test the scalar refuses non-integer variable values. + + Given: + A [BigInt!] variable carrying a numeric string or a fractional + number — the shapes a client that hedged against the 32-bit + ceiling by stringifying would send. + When: + The GraphQL files query is executed with that variable. + Then: + It should reject the query with a BigInt error, so the wire form + stays unambiguously a JSON integer. + """ + # Arrange + mock_db.files.docs = [_make_file_doc("f1", size_in_bytes=1234)] + + # Act + result = await schema.execute( + "query Files($sizes: [BigInt!]) {" + " files(input: [{ sizeInBytes: $sizes }]) { totalCount } }", + variable_values={"sizes": [value]}, + ) + + # Assert + assert result.data is None + assert "BigInt cannot represent" in result.errors[0].message + + @pytest.mark.asyncio + async def test_schema_should_type_size_in_bytes_as_big_int_on_both_sides(self): + """Test the widening reached the input filter as well as the output. + + Given: + The published GraphQL schema. + When: + FileMetadataType and FileMetadataInput are introspected. + Then: + Both should name sizeInBytes as BigInt, since widening only the + output would leave the files it exposes unfilterable by size. + """ + # Act + result = await schema.execute( + """ + { + output: __type(name: "FileMetadataType") { + fields { name type { ...Ref } } + } + input: __type(name: "FileMetadataInput") { + inputFields { name type { ...Ref } } + } + } + fragment Ref on __Type { + name ofType { name ofType { name ofType { name } } } + } + """ + ) + + # Assert + assert result.errors is None + output = {f["name"]: f["type"] for f in result.data["output"]["fields"]} + inputs = {f["name"]: f["type"] for f in result.data["input"]["inputFields"]} + assert _named_type(output["sizeInBytes"]) == "BigInt" + assert _named_type(inputs["sizeInBytes"]) == "BigInt" + + @pytest.mark.asyncio + async def test_schema_should_leave_neighbouring_integer_fields_as_int(self): + """Test the widening did not spread to unrelated integer fields. + + Given: + The published GraphQL schema, in which ExtraFileType.fileSize is + another size-shaped int and totalCount, fileCount and the + pagination arguments are counts. + When: + Those fields and arguments are introspected. + Then: + Each should still be Int, since the override is scoped to one + model field rather than to every int in the schema. + """ + # Act + result = await schema.execute( + """ + { + extraFile: __type(name: "ExtraFileType") { + fields { name type { ...Ref } } + } + fileList: __type(name: "FileList") { + fields { name type { ...Ref } } + } + query: __type(name: "Query") { + fields { name type { ...Ref } args { name type { ...Ref } } } + } + } + fragment Ref on __Type { + name ofType { name ofType { name ofType { name } } } + } + """ + ) + + # Assert + assert result.errors is None + extra_file = {f["name"]: f["type"] for f in result.data["extraFile"]["fields"]} + file_list = {f["name"]: f["type"] for f in result.data["fileList"]["fields"]} + query = {f["name"]: f for f in result.data["query"]["fields"]} + files_args = {a["name"]: a["type"] for a in query["files"]["args"]} + assert _named_type(extra_file["fileSize"]) == "Int" + assert _named_type(file_list["totalCount"]) == "Int" + assert _named_type(query["fileCount"]["type"]) == "Int" + assert _named_type(files_args["page"]) == "Int" + assert _named_type(files_args["pageSize"]) == "Int" + + +def test_checked_in_sdl_should_match_the_generated_schema(): + """Test schema.graphql has not drifted from the Strawberry schema. + + Given: + The checked-in schema.graphql, which is a generated artifact and the + contract clients codegen against. + When: + The SDL is rendered from the live schema. + Then: + The two should be byte-identical, so a type change that skipped + regeneration cannot ship a stale public contract. + """ + # Arrange + sdl_path = Path(__file__).resolve().parent.parent / "schema.graphql" + + # Act + generated = print_schema(schema) + "\n" + + # Assert + assert sdl_path.read_text() == generated, ( + "schema.graphql is stale — run `make schema` to regenerate it." + ) From 0c66ddb0ace712721225646b08da020b64f1404e Mon Sep 17 00:00:00 2001 From: Conrad Date: Mon, 10 Aug 2026 11:33:02 -0400 Subject: [PATCH 06/10] test: Assert a multi-gigabyte size reaches the wire as a JSON number Whether a client receives a number or a string is the decision this scalar makes, and schema.execute never touches json.dumps or an HTTP response. Asserting on the raw body catches a stringified value that response.json() would silently accept, and the mongomock-backed insert puts a real BSON encode and decode between the filter value and the match, which the FakeCollection double cannot. --- tests/test_metadata_endpoint.py | 127 ++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 tests/test_metadata_endpoint.py diff --git a/tests/test_metadata_endpoint.py b/tests/test_metadata_endpoint.py new file mode 100644 index 0000000..1777b11 --- /dev/null +++ b/tests/test_metadata_endpoint.py @@ -0,0 +1,127 @@ +"""HTTP-boundary tests for the /metadata GraphQL endpoint.""" + +from __future__ import annotations + +import asyncio +import re +from unittest.mock import patch + +import pytest +from mongomock_motor import AsyncMongoMockClient +from starlette.testclient import TestClient + +from cfdb import api + + +# The ENCODE .hic file named in issue #83, whose size exceeds the 2**31-1 +# ceiling a 32-bit GraphQL Int imposes. +_ISSUE_83_SIZE = 6262125716 + + +@pytest.fixture() +def client(): + # Mirrors the fixture in test_cors.py: the app binds the real lifespan at + # import, and the lifespan ensures the operational indexes, so back it + # with an in-memory mongomock client and disable the workflow subsystem. + from cfdb.api import main + + with ( + patch.object(main, "create_mongodb_client", return_value=AsyncMongoMockClient()), + patch.object(main.WorkflowProfile, "from_env", return_value=None), + ): + with TestClient(main.app) as c: + yield c + + +@pytest.fixture() +def large_file(client): + # Insert through the same database handle the resolvers read, so the size + # round-trips a real BSON encode/decode rather than the in-memory + # FakeCollection double, which compares Python values directly. The + # mongomock-motor wrapper is async over a synchronous store, so a + # throwaway loop is enough to drive the insert from this sync fixture. + asyncio.run( + api.db.files.insert_one( + { + "id_namespace": "ns", + "local_id": "ENCFF502HMX", + "project_id_namespace": "ns", + "project_local_id": "proj", + "filename": "ENCFF502HMX.hic", + "submission": "encode", + "data_access_level": "public", + "size_in_bytes": _ISSUE_83_SIZE, + "dcc": {"dcc_name": "ENCODE", "dcc_abbreviation": "encode"}, + "collections": [], + } + ) + ) + return client + + +def test_metadata_should_serve_a_multi_gigabyte_size_as_a_json_number(large_file): + """Test the issue #83 reproduction returns a number over real HTTP. + + Given: + The 6,262,125,716-byte ENCODE file from issue #83 stored in the + database. + When: + The reported reproduction query is POSTed to /metadata, selecting + sizeInBytes for that file. + Then: + The response body should carry the exact size as an unquoted JSON + number with no errors — the value a browser client can use directly, + rather than the null-plus-error the Int scalar produced. + """ + # Act + response = large_file.post( + "/metadata", + json={ + "query": ( + "{ files(input: [{ localId: [\"ENCFF502HMX\"] }])" + " { items { filename sizeInBytes } } }" + ) + }, + ) + + # Assert + assert response.status_code == 200 + body = response.json() + assert "errors" not in body + assert body["data"]["files"]["items"][0]["sizeInBytes"] == _ISSUE_83_SIZE + # Assert on the raw bytes too: json.loads would silently accept a quoted + # value, and whether the client receives a number or a string is the + # decision this scalar makes. + assert re.search(rf'"sizeInBytes":\s*{_ISSUE_83_SIZE}\b', response.text) + + +def test_metadata_should_filter_on_a_multi_gigabyte_size_over_http(large_file): + """Test a size above the Int ceiling round-trips into the Mongo query. + + Given: + The same file stored in the database. + When: + A /metadata query filters on its size through a [BigInt!] variable. + Then: + It should match that file, confirming the widened value survives + BSON encoding rather than only the GraphQL layer. + """ + # Act + response = large_file.post( + "/metadata", + json={ + "query": ( + "query Files($sizes: [BigInt!]) {" + " files(input: [{ sizeInBytes: $sizes }])" + " { totalCount items { localId } } }" + ), + "variables": {"sizes": [_ISSUE_83_SIZE]}, + }, + ) + + # Assert + assert response.status_code == 200 + body = response.json() + assert "errors" not in body + assert body["data"]["files"]["totalCount"] == 1 + assert body["data"]["files"]["items"][0]["localId"] == "ENCFF502HMX" From f61e62662a9029ab9baffe3c02e101b70fe10758 Mon Sep 17 00:00:00 2001 From: Conrad Date: Mon, 10 Aug 2026 12:09:54 -0400 Subject: [PATCH 07/10] fix: Fail loudly on a scalar override the substitution cannot express MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _substitute_scalar handled T and Optional[T] and silently approximated everything else, so an override on a list-shaped field would publish a list as a bare scalar. The drift test does not catch that — the contributor's fix is to run make schema and commit the wrong shape — so an unsupported annotation has to raise at import instead. Raise GraphQLError rather than ValueError from the coercion. graphql-core logs a non-GraphQLError cause with its traceback, so on an unauthenticated endpoint every malformed filter value was writing a stack trace at ERROR. Correct the comment on the JavaScript safe-integer bound, which described a guard that does not exist. The constant is surfaced in the scalar description only; the 64-bit bound is the enforced one, because it is where BSON itself stops. Staying under 2^53 is an admission criterion for routing a field through BigInt, not something the scalar checks. --- src/cfdb/api/gql/types.py | 44 +++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/src/cfdb/api/gql/types.py b/src/cfdb/api/gql/types.py index 058adbd..c723909 100644 --- a/src/cfdb/api/gql/types.py +++ b/src/cfdb/api/gql/types.py @@ -3,6 +3,7 @@ import strawberry import strawberry.scalars from bson import ObjectId +from graphql import GraphQLError from pydantic import BaseModel from cfdb.models import FileMetadataModel @@ -24,10 +25,14 @@ class ObjectIdScalar: _INT64_MAX = 2**63 - 1 # Above this magnitude a JSON number is no longer exactly representable in an -# IEEE-754 double, which is the only numeric type a browser client has. No -# byte size can reach it (2**53 bytes is ~9 PB), so this is a guard against a -# non-size value being routed through ``BigInt``, not a live concern for -# ``size_in_bytes``. +# IEEE-754 double, which is the only numeric type a browser client has. This +# is surfaced in the scalar's description and is deliberately NOT enforced: +# the 64-bit bound above is the one every downstream layer shares (it is +# exactly where BSON raises), so clamping here would invent a second, softer +# limit inside a type whose name promises 64 bits. Staying JS-safe is +# therefore an admission criterion for routing a field through ``BigInt`` — +# ``size_in_bytes`` qualifies because 2**53 bytes is ~9 PB — rather than +# something the scalar checks. _JS_SAFE_INTEGER_MAX = 2**53 - 1 @@ -37,12 +42,19 @@ def _coerce_big_int(value): Serialization and parsing share one implementation because the wire form is a JSON number: the value that goes out is the value that comes back. ``bool`` is excluded explicitly because it is an ``int`` subclass in - Python and ``true`` is not a size. + Python and ``true`` is not a size. Integral floats are rejected too, so + the wire form stays one unambiguous representation — note this is + stricter than the ``Int`` it replaces, which coerced them. + + Raises ``GraphQLError`` rather than ``ValueError`` because graphql-core + logs a non-``GraphQLError`` cause with its traceback: on an + unauthenticated endpoint every malformed filter value would otherwise + write a stack trace at ERROR. """ if isinstance(value, bool) or not isinstance(value, int): - raise ValueError(f"BigInt cannot represent non-integer value: {value!r}") + raise GraphQLError(f"BigInt cannot represent non-integer value: {value!r}") if not _INT64_MIN <= value <= _INT64_MAX: - raise ValueError( + raise GraphQLError( f"BigInt cannot represent non 64-bit signed integer value: {value}" ) return value @@ -163,13 +175,23 @@ def _resolve_json_type(field_type): def _substitute_scalar(field_type, scalar): """Replace a field's scalar type, preserving an ``Optional`` wrapper. - Overridden fields are declared on the model as ``T`` or ``Optional[T]``, - so no deeper nesting needs handling. + Only ``T`` and ``Optional[T]`` are handled, and anything else raises + rather than being approximated. A silently mishandled wrapper would + publish a structurally wrong SDL — a list field emitted as a bare + scalar — that the drift test would then bless on the next + ``make schema``, so an unsupported shape has to fail here, at import. """ args = getattr(field_type, "__args__", None) - if args and type(None) in args: + if args is None: + return scalar + inner = [a for a in args if a is not type(None)] + # ``Optional[List[T]]`` also has two args, one of them ``None`` — hence + # the check that what remains is itself unparameterized. + if len(args) == 2 and len(inner) == 1 and not hasattr(inner[0], "__args__"): return Optional[scalar] - return scalar + raise TypeError( + f"scalar override supports only T and Optional[T], not {field_type!r}" + ) def _rebuild_type(field_type, model_cls, strawberry_cls): From 492b208f4a14b949983376b29c295ddc0c93b292 Mon Sep 17 00:00:00 2001 From: Conrad Date: Mon, 10 Aug 2026 12:09:54 -0400 Subject: [PATCH 08/10] build: Write the generated schema as UTF-8 explicitly The drift test compares the artifact byte for byte, so its encoding must not depend on the locale of whoever ran make schema. The SDL is ASCII today, but descriptions are authored as Python strings in prose that uses em-dashes freely. --- scripts/export_schema.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/export_schema.py b/scripts/export_schema.py index 85fd086..4857cab 100644 --- a/scripts/export_schema.py +++ b/scripts/export_schema.py @@ -21,7 +21,10 @@ def render() -> str: def main() -> None: - SCHEMA_PATH.write_text(render()) + # Explicit encoding, not the process locale: the artifact's bytes are + # compared for equality by the drift test, so they must not depend on + # the shell of whoever ran ``make schema``. + SCHEMA_PATH.write_text(render(), encoding="utf-8") print(f"Wrote {SCHEMA_PATH}") From ec4ff1e7bc2a14cd48fdb9d7c44268bc412e76ae Mon Sep 17 00:00:00 2001 From: Conrad Date: Mon, 10 Aug 2026 12:10:04 -0400 Subject: [PATCH 09/10] test: Pin the declared range by property rather than by example MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six hand-picked values were all non-negative, so the lower bound of the advertised range was never exercised in the accepting direction — had it been mistyped, the suite would still have passed. Add a Hypothesis round trip across the range and a rejection property beyond it, plus both inclusive bounds as named cases. Have the drift guard call the generator it names instead of re-deriving the SDL a second way, so its failure message stays true: the two expressions could otherwise disagree about what up to date means, and the message would send the reader back to the command that caused the failure. Split the ExtraFileType.fileSize assertion out of the counts test. Counts cannot overflow; a byte size can, so grouping them read as a design rule when it is a deliberate, revisitable deferral. Also pin the integral float that Int used to coerce, drop three Arrange blocks whose database state no resolver ever reads, loosen an assertion that pinned graphql-core's exact phrasing, and use the mocker fixture rather than unittest.mock. --- tests/conftest.py | 6 + tests/test_metadata_endpoint.py | 43 +++---- tests/test_schema.py | 222 +++++++++++++++++++++++--------- 3 files changed, 185 insertions(+), 86 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 9a50a30..9d3d790 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,6 +9,12 @@ from cfdb import api +# The exact size, in bytes, of the ENCODE .hic file named in issue #83 — +# above the 2**31-1 ceiling GraphQL's ``Int`` scalar imposes. Shared so the +# number that names the regression exists in exactly one place. +ISSUE_83_SIZE = 6262125716 + + def _resolve(doc: dict, key: str): """Resolve a possibly dot-notated key against a nested dict.""" value = doc diff --git a/tests/test_metadata_endpoint.py b/tests/test_metadata_endpoint.py index 1777b11..d456c6a 100644 --- a/tests/test_metadata_endpoint.py +++ b/tests/test_metadata_endpoint.py @@ -4,37 +4,32 @@ import asyncio import re -from unittest.mock import patch import pytest from mongomock_motor import AsyncMongoMockClient from starlette.testclient import TestClient from cfdb import api - - -# The ENCODE .hic file named in issue #83, whose size exceeds the 2**31-1 -# ceiling a 32-bit GraphQL Int imposes. -_ISSUE_83_SIZE = 6262125716 +from tests.conftest import ISSUE_83_SIZE @pytest.fixture() -def client(): +def client(mocker): # Mirrors the fixture in test_cors.py: the app binds the real lifespan at # import, and the lifespan ensures the operational indexes, so back it # with an in-memory mongomock client and disable the workflow subsystem. from cfdb.api import main - with ( - patch.object(main, "create_mongodb_client", return_value=AsyncMongoMockClient()), - patch.object(main.WorkflowProfile, "from_env", return_value=None), - ): - with TestClient(main.app) as c: - yield c + mocker.patch.object( + main, "create_mongodb_client", return_value=AsyncMongoMockClient() + ) + mocker.patch.object(main.WorkflowProfile, "from_env", return_value=None) + with TestClient(main.app) as c: + yield c @pytest.fixture() -def large_file(client): +def client_with_large_file(client): # Insert through the same database handle the resolvers read, so the size # round-trips a real BSON encode/decode rather than the in-memory # FakeCollection double, which compares Python values directly. The @@ -50,7 +45,7 @@ def large_file(client): "filename": "ENCFF502HMX.hic", "submission": "encode", "data_access_level": "public", - "size_in_bytes": _ISSUE_83_SIZE, + "size_in_bytes": ISSUE_83_SIZE, "dcc": {"dcc_name": "ENCODE", "dcc_abbreviation": "encode"}, "collections": [], } @@ -59,7 +54,9 @@ def large_file(client): return client -def test_metadata_should_serve_a_multi_gigabyte_size_as_a_json_number(large_file): +def test_metadata_should_serve_a_multi_gigabyte_size_as_a_json_number( + client_with_large_file, +): """Test the issue #83 reproduction returns a number over real HTTP. Given: @@ -74,7 +71,7 @@ def test_metadata_should_serve_a_multi_gigabyte_size_as_a_json_number(large_file rather than the null-plus-error the Int scalar produced. """ # Act - response = large_file.post( + response = client_with_large_file.post( "/metadata", json={ "query": ( @@ -88,14 +85,16 @@ def test_metadata_should_serve_a_multi_gigabyte_size_as_a_json_number(large_file assert response.status_code == 200 body = response.json() assert "errors" not in body - assert body["data"]["files"]["items"][0]["sizeInBytes"] == _ISSUE_83_SIZE + assert body["data"]["files"]["items"][0]["sizeInBytes"] == ISSUE_83_SIZE # Assert on the raw bytes too: json.loads would silently accept a quoted # value, and whether the client receives a number or a string is the # decision this scalar makes. - assert re.search(rf'"sizeInBytes":\s*{_ISSUE_83_SIZE}\b', response.text) + assert re.search(rf'"sizeInBytes":\s*{ISSUE_83_SIZE}\b', response.text) -def test_metadata_should_filter_on_a_multi_gigabyte_size_over_http(large_file): +def test_metadata_should_filter_on_a_multi_gigabyte_size_over_http( + client_with_large_file, +): """Test a size above the Int ceiling round-trips into the Mongo query. Given: @@ -107,7 +106,7 @@ def test_metadata_should_filter_on_a_multi_gigabyte_size_over_http(large_file): BSON encoding rather than only the GraphQL layer. """ # Act - response = large_file.post( + response = client_with_large_file.post( "/metadata", json={ "query": ( @@ -115,7 +114,7 @@ def test_metadata_should_filter_on_a_multi_gigabyte_size_over_http(large_file): " files(input: [{ sizeInBytes: $sizes }])" " { totalCount items { localId } } }" ), - "variables": {"sizes": [_ISSUE_83_SIZE]}, + "variables": {"sizes": [ISSUE_83_SIZE]}, }, ) diff --git a/tests/test_schema.py b/tests/test_schema.py index 996e024..22898a9 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -4,14 +4,12 @@ import asyncio import logging -from pathlib import Path import pytest from hypothesis import HealthCheck, given, settings from hypothesis import strategies as st from mongomock_motor import AsyncMongoMockClient from starlette.testclient import TestClient -from strawberry.printer import print_schema from cfdb import api from cfdb.api import main @@ -19,6 +17,8 @@ from cfdb.api.gql.types import FileMetadataType from cfdb.models import FileMetadataModel from cfdb.services import locks +from scripts.export_schema import SCHEMA_PATH, render +from tests.conftest import ISSUE_83_SIZE def test_from_pydantic_should_convert_nested_model_lists_and_leave_json_untouched(): @@ -1938,9 +1938,24 @@ def test_files_should_answer_with_a_graphql_error_when_pagination_is_out_of_rang assert expected in body["errors"][0]["message"] -# The exact size, in bytes, of the ENCODE .hic file named in issue #83 — -# above the 2**31-1 ceiling GraphQL's ``Int`` scalar imposes. -_ISSUE_83_SIZE = 6262125716 +# Introspects every integer field and argument that must NOT have been +# widened, shared by the two tests that assert different halves of it. +_INT_FIELD_INTROSPECTION = """ + { + extraFile: __type(name: "ExtraFileType") { + fields { name type { ...Ref } } + } + fileList: __type(name: "FileList") { + fields { name type { ...Ref } } + } + query: __type(name: "Query") { + fields { name type { ...Ref } args { name type { ...Ref } } } + } + } + fragment Ref on __Type { + name ofType { name ofType { name ofType { name } } } + } + """ class TestSizeInBytesScalar: @@ -1967,7 +1982,7 @@ async def test_size_in_bytes_should_resolve_a_file_above_the_int32_ceiling( null-plus-per-field-error the Int scalar produced. """ # Arrange - mock_db.files.docs = [_make_file_doc("f1", size_in_bytes=_ISSUE_83_SIZE)] + mock_db.files.docs = [_make_file_doc("f1", size_in_bytes=ISSUE_83_SIZE)] # Act result = await schema.execute( @@ -1976,11 +1991,13 @@ async def test_size_in_bytes_should_resolve_a_file_above_the_int32_ceiling( # Assert assert result.errors is None - assert result.data["files"]["items"][0]["sizeInBytes"] == _ISSUE_83_SIZE + assert result.data["files"]["items"][0]["sizeInBytes"] == ISSUE_83_SIZE @pytest.mark.parametrize( "size", [ + -(2**63), + -1, 0, 4096, 2**31 - 1, @@ -1988,7 +2005,16 @@ async def test_size_in_bytes_should_resolve_a_file_above_the_int32_ceiling( 2**53 - 1, 2**63 - 1, ], - ids=["zero", "small", "int32-max", "int32-max-plus-one", "js-safe-max", "int64-max"], + ids=[ + "int64-min", + "negative", + "zero", + "small", + "int32-max", + "int32-max-plus-one", + "js-safe-max", + "int64-max", + ], ) @pytest.mark.asyncio async def test_size_in_bytes_should_round_trip_across_the_64_bit_range( @@ -1998,8 +2024,8 @@ async def test_size_in_bytes_should_round_trip_across_the_64_bit_range( Given: A file whose size sits at a notable point of the 64-bit range — - zero, an ordinary size, either side of the old Int ceiling, the - JavaScript safe-integer maximum, and the 64-bit maximum. + both inclusive bounds, zero, an ordinary size, either side of the + old Int ceiling, and the JavaScript safe-integer maximum. When: The GraphQL files query selects sizeInBytes. Then: @@ -2015,6 +2041,75 @@ async def test_size_in_bytes_should_round_trip_across_the_64_bit_range( assert result.errors is None assert result.data["files"]["items"][0]["sizeInBytes"] == size + @given(size=st.integers(min_value=-(2**63), max_value=2**63 - 1)) + @settings( + max_examples=50, + suppress_health_check=[HealthCheck.function_scoped_fixture], + ) + def test_size_in_bytes_should_round_trip_any_value_in_the_declared_range( + self, mock_db, size + ): + """Test the round trip holds across the whole declared range. + + Given: + An arbitrary size drawn from the signed 64-bit range the scalar + advertises. + When: + The GraphQL files query selects sizeInBytes. + Then: + It should return that exact value with no errors, so the + contract is the declared range rather than the handful of + boundary values the parametrized case happens to name. + """ + # Arrange + # ``mock_db`` is function-scoped, so Hypothesis reuses one instance + # across examples (hence the suppressed health check); reseeding it + # each example keeps them independent. The resolver is async and + # Hypothesis does not compose with pytest-asyncio, so it is driven + # through asyncio.run. + mock_db.files.docs = [_make_file_doc("f1", size_in_bytes=size)] + + # Act + result = asyncio.run(schema.execute("{ files { items { sizeInBytes } } }")) + + # Assert + assert result.errors is None + assert result.data["files"]["items"][0]["sizeInBytes"] == size + + # Drawn as a union of two bounded strategies rather than by filtering + # ``st.integers()``, which discards the in-range majority and trips + # Hypothesis's filter_too_much health check. + @given( + size=st.integers(min_value=2**63) | st.integers(max_value=-(2**63) - 1), + ) + @settings( + max_examples=25, + suppress_health_check=[HealthCheck.function_scoped_fixture], + ) + def test_size_in_bytes_should_reject_any_value_outside_the_declared_range( + self, mock_db, size + ): + """Test the range bound holds for every value beyond it. + + Given: + An arbitrary integer outside the signed 64-bit range — the range + beyond which BSON itself cannot encode a value. + When: + The GraphQL files query selects sizeInBytes. + Then: + It should null that field and report a BigInt error, rather than + emitting a number no downstream layer can carry. + """ + # Arrange + mock_db.files.docs = [_make_file_doc("f1", size_in_bytes=size)] + + # Act + result = asyncio.run(schema.execute("{ files { items { sizeInBytes } } }")) + + # Assert + assert result.data["files"]["items"][0]["sizeInBytes"] is None + assert "BigInt cannot represent" in result.errors[0].message + @pytest.mark.asyncio async def test_size_in_bytes_should_be_null_when_the_file_records_no_size( self, mock_db @@ -2088,14 +2183,14 @@ async def test_files_should_filter_on_a_size_above_the_int32_ceiling(self, mock_ """ # Arrange mock_db.files.docs = [ - _make_file_doc("big", size_in_bytes=_ISSUE_83_SIZE), + _make_file_doc("big", size_in_bytes=ISSUE_83_SIZE), _make_file_doc("small", size_in_bytes=1234), ] # Act result = await schema.execute( "{ files(input: [{ sizeInBytes: [%d] }])" - " { totalCount items { localId } } }" % _ISSUE_83_SIZE, + " { totalCount items { localId } } }" % ISSUE_83_SIZE, ) # Assert @@ -2120,7 +2215,7 @@ async def test_files_should_filter_on_a_large_size_passed_as_a_variable( """ # Arrange mock_db.files.docs = [ - _make_file_doc("big", size_in_bytes=_ISSUE_83_SIZE), + _make_file_doc("big", size_in_bytes=ISSUE_83_SIZE), _make_file_doc("small", size_in_bytes=1234), ] @@ -2129,7 +2224,7 @@ async def test_files_should_filter_on_a_large_size_passed_as_a_variable( "query Files($sizes: [BigInt!]) {" " files(input: [{ sizeInBytes: $sizes }])" " { totalCount items { localId } } }", - variable_values={"sizes": [_ISSUE_83_SIZE]}, + variable_values={"sizes": [ISSUE_83_SIZE]}, ) # Assert @@ -2139,7 +2234,7 @@ async def test_files_should_filter_on_a_large_size_passed_as_a_variable( @pytest.mark.asyncio async def test_files_should_reject_an_int_typed_variable_for_the_size_filter( - self, mock_db + self, ): """Test the documented break for clients still declaring Int. @@ -2152,10 +2247,9 @@ async def test_files_should_reject_an_int_typed_variable_for_the_size_filter( It should fail validation naming the expected [BigInt!] type, rather than silently truncating at the 32-bit ceiling. """ - # Arrange - mock_db.files.docs = [_make_file_doc("big", size_in_bytes=_ISSUE_83_SIZE)] - # Act + # No arrangement: the query is rejected at validation, before any + # resolver runs, so no database state participates. result = await schema.execute( "query Files($sizes: [Int!]) {" " files(input: [{ sizeInBytes: $sizes }]) { totalCount } }", @@ -2164,7 +2258,7 @@ async def test_files_should_reject_an_int_typed_variable_for_the_size_filter( # Assert assert result.data is None - assert "expecting type '[BigInt!]'" in result.errors[0].message + assert "BigInt" in result.errors[0].message @pytest.mark.parametrize( "literal", @@ -2173,7 +2267,7 @@ async def test_files_should_reject_an_int_typed_variable_for_the_size_filter( ) @pytest.mark.asyncio async def test_files_should_reject_a_size_filter_literal_outside_the_scalar( - self, mock_db, literal + self, literal ): """Test the scalar refuses literals it cannot represent. @@ -2186,9 +2280,6 @@ async def test_files_should_reject_a_size_filter_literal_outside_the_scalar( It should reject the query outright with a BigInt error rather than coercing the value. """ - # Arrange - mock_db.files.docs = [_make_file_doc("f1", size_in_bytes=1234)] - # Act result = await schema.execute( f"{{ files(input: [{{ sizeInBytes: [{literal}] }}]) {{ totalCount }} }}", @@ -2199,27 +2290,26 @@ async def test_files_should_reject_a_size_filter_literal_outside_the_scalar( assert "BigInt cannot represent" in result.errors[0].message @pytest.mark.parametrize( - "value", ["6262125716", 1.5], ids=["string", "non-integral-float"] + "value", + ["6262125716", 1.5, 1234.0], + ids=["string", "non-integral-float", "integral-float"], ) @pytest.mark.asyncio async def test_files_should_reject_a_non_integer_size_filter_variable( - self, mock_db, value + self, value ): """Test the scalar refuses non-integer variable values. Given: - A [BigInt!] variable carrying a numeric string or a fractional - number — the shapes a client that hedged against the 32-bit - ceiling by stringifying would send. + A [BigInt!] variable carrying a numeric string, a fractional + number, or an integral float — the last being the one Int used + to coerce, so this is a fourth way a pre-BigInt client breaks. When: The GraphQL files query is executed with that variable. Then: It should reject the query with a BigInt error, so the wire form stays unambiguously a JSON integer. """ - # Arrange - mock_db.files.docs = [_make_file_doc("f1", size_in_bytes=1234)] - # Act result = await schema.execute( "query Files($sizes: [BigInt!]) {" @@ -2268,71 +2358,75 @@ async def test_schema_should_type_size_in_bytes_as_big_int_on_both_sides(self): assert _named_type(inputs["sizeInBytes"]) == "BigInt" @pytest.mark.asyncio - async def test_schema_should_leave_neighbouring_integer_fields_as_int(self): + async def test_schema_should_leave_the_count_fields_as_int(self): """Test the widening did not spread to unrelated integer fields. Given: - The published GraphQL schema, in which ExtraFileType.fileSize is - another size-shaped int and totalCount, fileCount and the - pagination arguments are counts. + The published GraphQL schema, in which totalCount, fileCount and + the pagination arguments count documents rather than bytes. When: Those fields and arguments are introspected. Then: - Each should still be Int, since the override is scoped to one - model field rather than to every int in the schema. + Each should still be Int, which no collection this API serves + can overflow, since the override is scoped to one model field + rather than to every int in the schema. """ # Act - result = await schema.execute( - """ - { - extraFile: __type(name: "ExtraFileType") { - fields { name type { ...Ref } } - } - fileList: __type(name: "FileList") { - fields { name type { ...Ref } } - } - query: __type(name: "Query") { - fields { name type { ...Ref } args { name type { ...Ref } } } - } - } - fragment Ref on __Type { - name ofType { name ofType { name ofType { name } } } - } - """ - ) + result = await schema.execute(_INT_FIELD_INTROSPECTION) # Assert assert result.errors is None - extra_file = {f["name"]: f["type"] for f in result.data["extraFile"]["fields"]} file_list = {f["name"]: f["type"] for f in result.data["fileList"]["fields"]} query = {f["name"]: f for f in result.data["query"]["fields"]} files_args = {a["name"]: a["type"] for a in query["files"]["args"]} - assert _named_type(extra_file["fileSize"]) == "Int" assert _named_type(file_list["totalCount"]) == "Int" assert _named_type(query["fileCount"]["type"]) == "Int" assert _named_type(files_args["page"]) == "Int" assert _named_type(files_args["pageSize"]) == "Int" + @pytest.mark.asyncio + async def test_schema_should_still_type_extra_file_size_as_int(self): + """Test the deliberate deferral of the other byte-size field. + + Given: + ExtraFileType.fileSize, which is a byte size like sizeInBytes and + so carries the same 32-bit ceiling. + When: + The type is introspected. + Then: + It should still be Int — a deliberate deferral, not a rule. + Issue #83 scopes this change to sizeInBytes, and 4DN extra_files + are index sidecars (bai, tbi, px2, beddb) that do not approach + 2 GB. Widen it and change this test when that stops holding. + """ + # Act + result = await schema.execute(_INT_FIELD_INTROSPECTION) + + # Assert + assert result.errors is None + extra_file = {f["name"]: f["type"] for f in result.data["extraFile"]["fields"]} + assert _named_type(extra_file["fileSize"]) == "Int" + -def test_checked_in_sdl_should_match_the_generated_schema(): +def test_render_should_match_the_checked_in_sdl(): """Test schema.graphql has not drifted from the Strawberry schema. Given: The checked-in schema.graphql, which is a generated artifact and the contract clients codegen against. When: - The SDL is rendered from the live schema. + The SDL is rendered by the same function `make schema` writes with. Then: The two should be byte-identical, so a type change that skipped regeneration cannot ship a stale public contract. """ - # Arrange - sdl_path = Path(__file__).resolve().parent.parent / "schema.graphql" - # Act - generated = print_schema(schema) + "\n" + # Call the generator rather than re-deriving it, so the failure message + # below stays true: re-rendering the SDL a second way would let the + # guard and `make schema` disagree about what "up to date" means. + generated = render() # Assert - assert sdl_path.read_text() == generated, ( + assert SCHEMA_PATH.read_text(encoding="utf-8") == generated, ( "schema.graphql is stale — run `make schema` to regenerate it." ) From 6e47966ed74b9de74d0b5e6ca968a066e2ba9c38 Mon Sep 17 00:00:00 2001 From: Conrad Date: Mon, 10 Aug 2026 12:10:14 -0400 Subject: [PATCH 10/10] docs: Name the codegen step and the migration order for BigInt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Telling a typed client to re-run codegen understates the work: an unrecognised custom scalar widens to any without failing the build, so the scalar mapping is the load-bearing step. Name it, and name the two other scalars a client must map for the same reason. Record the migration order, which is the non-obvious part. A rolling deploy serves both schemas at once, dev and prod publish different schemas by design, and a SHA rollback reverts the contract — but the leaf type only has to be named when a client declares a variable for it, so moving consumers to an inline literal or a whole-input variable first makes the deploy a non-event in either direction. Also qualify the filtering claim: size filtering is exact-match, and 4DN and HuBMAP store the size as a string through the C2M2 TSV path, so a numeric predicate does not match their documents. That gap is independent of the scalar's width and is tracked separately. --- README.md | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index e81bbe8..5eb151d 100644 --- a/README.md +++ b/README.md @@ -359,16 +359,28 @@ File count for a filter: `{ fileCount(input: [{ dcc: [{ dccAbbreviation: ["4DN"] ### Custom Scalars +The published contract is `schema.graphql` at the repo root — a generated artifact, regenerated by `make schema` and never edited by hand. Three of its scalars are outside the GraphQL specification, so a typed client must map all three explicitly: + | Scalar | Wire form | Used by | |--------|-----------|---------| | `ObjectIdScalar` | JSON string | `file(id:)` | | `BigInt` | JSON number | `sizeInBytes`, on both `FileMetadataType` and `FileMetadataInput` | +| `JSON` | Any JSON value | `distinctValues.values`, `collections[].extra.hubmap.metadata` | + +`BigInt` is a signed 64-bit integer. The GraphQL specification fixes `Int` at 32 bits, so a file larger than 2,147,483,647 bytes (~2.1 GB) could not be represented at all: the field resolved to `null` and contributed a `Int cannot represent non 32-bit signed integer value` entry to the response's `errors` array, degrading a whole page of results to a partial one. That affects every ENCODE `.hic` file (6–51 GB) and the larger 4DN mcools, so it is the common case for contact maps rather than an edge case. The input filter carries the same scalar — a 32-bit filter would leave exactly the files the widened output field exposes unfilterable. Note that size filtering is exact-match, not a range, and that `size_in_bytes` is stored as a BSON int64 only for ENCODE: 4DN and HuBMAP load it as a string through the C2M2 TSV path, so a numeric equality predicate does not match their documents. That is an ingest-layer gap independent of the scalar's width — the filter was equally inert at `Int` — and normalising it is tracked separately. + +`BigInt` stays a JSON **number** on the wire rather than a string, so `sizeInBytes` remains directly usable in client-side arithmetic and comparisons with no parsing step. The usual objection to that choice — values above `Number.MAX_SAFE_INTEGER` (2^53-1) lose precision in JavaScript — does not bind here: 2^53 bytes is ~9 PB, far above any file this API serves. That is a property of *which* fields are routed through the scalar rather than something the scalar enforces, so staying under 2^53 is an admission criterion for any future `BigInt` field. What the scalar does enforce is the signed 64-bit range — exactly where BSON itself stops — plus a rejection of non-integers, including `true`/`false`, on both input and output. + +**This is a breaking schema change.** A client that hard-codes `Int` breaks in four ways: -`BigInt` is a signed 64-bit integer. The GraphQL specification fixes `Int` at 32 bits, so a file larger than 2,147,483,647 bytes (~2.1 GB) could not be represented at all: the field resolved to `null` and contributed a `Int cannot represent non 32-bit signed integer value` entry to the response's `errors` array, degrading a whole page of results to a partial one. That affects every ENCODE `.hic` file (6–51 GB) and the larger 4DN mcools, so it is the common case for contact maps rather than an edge case. The input filter carries the same scalar — a 32-bit filter would leave exactly the files the widened output field exposes unfilterable. +- A query declaring `query Q($s: [Int!])` and passing it to `sizeInBytes` now fails variable-type validation and must declare `[BigInt!]`. +- Generated clients must re-run codegen against the new SDL **and add a scalar mapping** — `graphql-codegen` and most typed clients silently widen an unrecognised custom scalar to `any`, so `scalars: { BigInt: 'number' }` (or the equivalent) is required or `sizeInBytes` loses its type with no build failure. +- Any client validating responses against a stored copy of the schema must refresh it. +- A client sending an integral float — `1234.0`, which any language that round-trips numbers through a float will emit — is now rejected. `Int` coerced it; `BigInt` does not, so the wire form stays one unambiguous representation. -`BigInt` stays a JSON **number** on the wire rather than a string, so `sizeInBytes` remains directly usable in client-side arithmetic and comparisons with no parsing step. The usual objection to that choice — values above `Number.MAX_SAFE_INTEGER` (2^53-1) lose precision in JavaScript — does not bind here: 2^53 bytes is ~9 PB, far above any file this API serves. The scalar rejects non-integers (including `true`/`false`) and anything outside the signed 64-bit range on both input and output. +A client that merely *reads* `sizeInBytes` out of the JSON response needs no change — it was already receiving a JSON number, and now receives a correct one instead of `null`. -**This is a breaking schema change.** A client that hard-codes `Int` breaks in three ways: a query declaring `query Q($s: [Int!])` and passing it to `sizeInBytes` now fails variable-type validation and must declare `[BigInt!]`; generated clients must re-run codegen against the new SDL; and any client validating responses against a stored copy of the schema must refresh it. A client that merely *reads* `sizeInBytes` out of the JSON response needs no change — it was already receiving a JSON number, and now receives a correct one instead of `null`. +**Migrating without a window of failures.** The server side of this break is not atomic. A rolling ECS deploy keeps old and new tasks in the target group simultaneously, dev ships on every merge while prod is promoted by hand, and the documented rollback (re-tag an older `:`) reverts the schema under clients that have already migrated. In each case a client that *names* `BigInt` in a variable declaration fails against whichever tasks are still on the old schema. The way out is that the leaf type only has to be named when the client declares a variable for it: passing the filter as an inline literal (`sizeInBytes: [6262125716]`), or hoisting the variable up to the whole input object (`query Q($input: [FileMetadataInput!])`), validates against the old schema and the new one alike. Move consumers to one of those forms first, and the deploy — in either direction — is a non-event. ### Query Mechanics