Skip to content

feat: add Query.Builder, aggregates, joins, and nested ON - #1817

Open
abnegate wants to merge 41 commits into
mainfrom
feat-query-helpers
Open

feat: add Query.Builder, aggregates, joins, and nested ON#1817
abnegate wants to merge 41 commits into
mainfrom
feat-query-helpers

Conversation

@abnegate

@abnegate abnegate commented Aug 21, 2026

Copy link
Copy Markdown
Member

Why

Appwrite query-lib added aggregates, grouping, joins, extra spatial operators, and a POST /query body so large queries arrays are not capped by URL length. SDKs that already expose Query helpers need those methods, a fluent collector, and a way to nest JOIN ON conditions without leaking SQL.

What changed

  • New Query helpers on every SDK that already had Query: aggregates (count, sum, avg, …), groupBy/having/distinct, joins (join/leftJoin/rightJoin/fullOuterJoin/crossJoin), extra spatial (covers, …).
  • Fluent collector nested as Query.Builder where the language allows it (C#/Swift/Kotlin/Ruby/Python/TS namespace). PHP is Appwrite\Query\Builder imported as QueryBuilder. Go/Rust use package Builder with NewBuilder() / query::Builder. Dart cannot nest classes and must not export a top-level Builder (Flutter widget clash), so the type is QueryBuilder in lib/query/builder.dart.
  • Nested JOIN ON: Query.on(...) plus join overloads that take an alias, an ON query, and extra filters. The wire shape is [alias?, Query.on, filters…].
  • Client flatten is internal. There is no public Query.normalize. Flattening is structural (builder / nested query-string lists) and does not special-case the queries parameter name.
  • Closed live-spec string enums encoded as type: string plus oneOf of single-value schemas are treated as wire enums. Untitled oneOf members take their name from the parent property.
  • PHP from() and generated mocks wrap a scalar example for an array-of-enum field into a one-element list (OAuth prompt in the live spec is a string example on an array field).
  • E2E QUERY_HELPER_RESPONSES covers each new helper in every SDK language test.

Raw/union/json/naturalJoin stay unexposed — those are not on the Documents validator allow-list.

Vector helpers were already on main; duplicate copies introduced during the rebase were removed so each language has a single definition.

Why this approach

A client-side JSON collector (not SQL) matches the existing Query string format {method, attribute?, values?}. Nesting the builder under Query keeps the public surface one type. Flattening inside the Client, without a queries key check, means any param that already carries query strings (join ON lists, having, or/and) is handled the same way.

POST /query itself lives in Appwrite (appwrite/appwrite#13300, stacked on #11649). SDK methods will be generated as createQuery once that spec is published.

Verified

  • composer lint (phpcs PSR-12)
  • composer lint-twig (djLint, 0 errors)
  • composer refactor:check (Rector dry-run)
  • Twig line length ≤ 1200
  • composer test tests/e2e/PHP83Test.php — OK, 1511 assertions (includes array-enum scalar hydration)
  • Generated Dart lib/ + tests/tests.dart analyze clean for the Builder/part-of/MockKind failures
  • Generated Go query.NewBuilder().Limit(1).Build() compiles and prints the expected limit query

Not verified

  • Full sdk-generator language e2e matrix on this HEAD. CI is the authority for that (previous HEAD was red on Dart/Flutter/Go Tests and Dart/Flutter/PHP/CLI Validation).
  • Live Appwrite POST /query round-trip from a generated SDK (depends on appwrite#13300 + spec publish).
  • CLI compiles in this repo against ../go via a replace in go.mod.twig (generated from the same spec as the CLI). Compiling the published CLI package against the last published Go module is not verified here.

@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

The implementation appears functionally sound, but the new catch-all generation tests violate an explicit repository testing requirement and should be corrected before merging.

Summary

  • Adds language-specific Query builder generation and transport normalization.
  • Adds aggregate, grouping, join, spatial, vector, and pagination helpers.
  • Upgrades OpenAPI parsing to support constrained compound response unions.
  • Extends generated-SDK and mock-service coverage across the language matrix.
  • Updates CLI-to-Go SDK generation alignment.

@abnegate

Copy link
Copy Markdown
Member Author

@greptileai review

1 similar comment
@abnegate

Copy link
Copy Markdown
Member Author

@greptileai review

Query helpers now cover the query-lib methods Appwrite allows
(aggregates, groupBy, having, distinct, joins, extra spatial).
Query.builder()/build() collects filters as a flat string list, and
Client.normalize unwraps the common extra-array wrap of a builder
or build()/page() result so list and POST /query calls stay valid.
`any` does not exist on Go 1.13, which still compiles the SDK in CI.
Realtime subscription updates now go through Query.normalize instead of
iterating QueryInput, and Client.subscribe no longer writes a queries
set that is not on the Realtime connection type.
Query.normalize was only flattening a builder or nested build()/page()
array before send. Call sites already do that on the queries param, so
it does not belong on the public Query helper surface.
Keep the simple join(table, left, right, op, alias) triple. Array overloads
encode Query.on() plus filter queries into nested join values so extra ON
predicates stay on the JOIN.
Add e2e prints and unit tests for aggregates, joins, ON, spatial, and page so each SDK exercises every new query type.
Clients now map every param through a type-based flatten so builders and extra nested lists unwrap without special-casing the queries key. Document the new public Query helpers.
Live specs encode closed enums as type:string plus oneOf of
single-value string schemas. Parsing that as an object made path
params Any, JSON keys the enum title, and Ruby docs crash on
non-JSON examples.

Keep model JSON keys as the property name, type signatures from
the collapsed enum, and flatten Dart request maps as Map<String, dynamic>.
@abnegate
abnegate force-pushed the feat-query-helpers branch from 420fd0f to 844e3e1 Compare August 21, 2026 07:45
Collapsed oneOf string enums with no title produced empty type names
(`val status: ,`, `resourceType: ;`) and invalid PHP properties.
Move QueryBuilder into Query/Builder files and nested types where
the language allows it. Alias QueryBuilder at import for PHP and
the TypeScript public export.
Dart part-of is resolved relative to lib/query, so the builder library
looked for packageName.dart in the wrong directory, and a top-level
Builder class collides with Flutter's widget. Go treats query.Builder()
as a type conversion. PHP mock payloads and from() treated array-of-enum
fields as a single string, which is how live-spec OAuth prompt examples
are encoded. CLI Validation now compiles against a Go SDK generated from
the same spec instead of the last published module.
@abnegate abnegate changed the title feat: add Query builder, aggregates, joins, and query normalize feat: add Query.Builder, aggregates, joins, and nested ON Aug 21, 2026
A replace in go.mod is the local pin; Validation no longer needs an env
override or go mod edit. example.php writes examples/go from the same
spec so ../go matches the CLI module path.
Request flattening is a transport concern. Keep Query as the helper
surface and run flatten from Client in Kotlin, Android, Swift, and Apple.
Keep annotated enumerations from main and the query builder work. CLI
go.mod still replace-pins the sibling generated Go SDK at v7.2.0-rc.3.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds cross-language query aggregation, grouping, joins, spatial operators, fluent builders, request flattening, and enum-handling improvements to generated SDKs.

Changes:

  • Expands Query APIs and adds fluent builders across supported SDK languages.
  • Normalizes nested query inputs in clients and adds broad E2E/unit coverage.
  • Improves closed oneOf enum detection and scalar array-enum hydration.

Reviewed changes

Copilot reviewed 110 out of 110 changed files in this pull request and generated 29 comments.

Show a summary per file
File(s) Description
tests/resources/spec-openapi3.json Adds closed oneOf enum fixtures.
tests/e2e/Base.php Adds helper-output and enum hydration assertions.
tests/e2e/languages/{web,node,react-native,deno}/* Exercises TypeScript query helpers and builders.
tests/e2e/languages/{swift,apple}/Tests.swift Exercises Swift query helpers.
tests/e2e/languages/{kotlin,android}/Tests.kt Exercises Kotlin query helpers.
tests/e2e/languages/{go,go-v2}/tests.go Exercises Go query helpers.
tests/e2e/languages/{dart,flutter}/tests.dart Exercises Dart query helpers.
tests/e2e/languages/{dotnet,unity}/Tests.cs Exercises C# query helpers.
tests/e2e/languages/{ruby,python,php}/* Exercises dynamic-language query helpers.
templates/web/src/{query,client,index}.ts.twig Adds Query builder/input normalization and exports.
templates/web/src/services/realtime.ts.twig Normalizes realtime query inputs.
templates/node/src/{client,index}.ts.twig Adds Node request flattening and exports.
templates/node/test/query.test.js.twig Tests Node query behavior.
templates/react-native/src/{query,client,index}.ts.twig Adds React Native query building and flattening.
templates/deno/{mod.ts,src/query.ts,src/client.ts}.twig Adds Deno query building and flattening.
templates/deno/test/query.test.ts.twig Tests Deno query behavior.
templates/swift/Sources/Query*.twig Adds Swift helpers and nested builder.
templates/swift/Sources/Client.swift.twig Flattens Swift request parameters.
templates/swift/{Tests/Tests.swift,Package.swift}.twig Adds tests and conditional package targets.
templates/apple/{Sources/Client.swift,Package.swift}.twig Applies inherited Swift client/package updates.
templates/kotlin/src/main/kotlin/io/appwrite/{Query,Client}.kt.twig Adds Kotlin query APIs and flattening.
templates/kotlin/src/test/kotlin/io/appwrite/QueryTest.kt.twig Adds Kotlin Query tests.
templates/android/library/src/main/java/io/package/{Query,Client}.kt.twig Adds Android query APIs and flattening.
templates/go/query*.twig Adds Go helpers, builder, and tests.
templates/go/client.go.twig Flattens Go request values.
templates/rust/src/query/{mod,builder}.rs.twig Adds Rust Query APIs and builder.
templates/rust/src/client.rs.twig Normalizes Rust request parameters.
templates/rust/tests/tests.rs Exercises Rust query helpers.
templates/dotnet/Package/Query*.twig Adds .NET helpers and nested builder.
templates/dotnet/Package/Client.cs.twig Flattens .NET request parameters.
templates/php/src/{Query,Client}.php.twig Adds PHP helpers, builder integration, and flattening.
templates/php/src/Query/Builder.php.twig Implements the PHP fluent builder.
templates/php/src/Models/{Model,RequestModel}.php.twig Wraps scalar array-enum values during hydration.
templates/php/tests/QueryTest.php.twig Tests PHP Query behavior.
templates/python/package/{query,client}.py.twig Adds Python builder APIs and flattening.
templates/python/test/test_query.py.twig Tests Python Query behavior.
templates/ruby/lib/container/{query,client}.rb.twig Adds Ruby helpers and request flattening.
templates/ruby/lib/container/query/builder.rb.twig Implements the Ruby fluent builder.
templates/dart/lib/{query,package}.dart.twig Adds Dart Query APIs and builder registration.
templates/dart/lib/query/builder.dart.twig Implements QueryBuilder.
templates/dart/lib/src/client_mixin.dart.twig Flattens Dart request parameters.
templates/dart/lib/src/models/{model,request_model}.dart.twig Handles scalar array-enums and enum serialization.
templates/dart/test/{query_test,src/models/model_test}.dart.twig Extends Dart Query/model tests.
templates/flutter/lib/{package,src/client_mixin}.dart.twig Registers builders and flattens Flutter parameters.
templates/skills/*.md.twig Documents query aggregation, joins, and builders.
templates/cli/internal/typegen/templates/{types,databases}.ts.hbs Extends typed CLI Query helpers.
templates/cli/internal/generator/typescript.go Reuses the generated QueryBuilder type.
templates/cli/internal/generator/testdata/* Updates expected TypeScript output.
templates/cli/go.mod.twig Redirects CLI builds to a local generated Go SDK.
src/SDK/SDK.php Improves enum-name fallback resolution.
src/SDK/Language.php Resolves untitled enum names from parent properties.
src/SDK/Language/{Swift,Apple}.php Registers Swift builder output.
src/SDK/Language/{Kotlin,Android}.php Registers Query tests.
src/SDK/Language/{Dart,Flutter}.php Registers Dart builder output.
src/SDK/Language/{Go,Rust,PHP,Ruby,DotNet}.php Registers new builder/module templates.
example.php Generates Go alongside CLI and updates the Go SDK version.
Suppressed comments (2)

templates/cli/internal/typegen/templates/types.ts.hbs:76

  • These fixed scalar signatures cannot express the new nested JOIN ON form, and the facade exposes no on helper. Add on plus overloads/unions for (table, on) and (table, alias, on) so the typed callback can construct the nested wire shape supported by Query.
  join: (table: string, left: string, right: string, operator?: string, alias?: string) => string;
  leftJoin: (table: string, left: string, right: string, operator?: string, alias?: string) => string;
  rightJoin: (table: string, left: string, right: string, operator?: string, alias?: string) => string;
  fullOuterJoin: (table: string, left: string, right: string, operator?: string, alias?: string) => string;

templates/cli/internal/typegen/templates/databases.ts.hbs:44

  • The facade delegates only the scalar join form and does not expose Query.on, so typed CLI callbacks cannot use the nested JOIN ON feature added by this PR. Mirror the nested overloads and on helper exposed by Query.
  join: (table, left, right, operator = '=', alias = '') => Query.join(table, left, right, operator, alias),
  leftJoin: (table, left, right, operator = '=', alias = '') => Query.leftJoin(table, left, right, operator, alias),
  rightJoin: (table, left, right, operator = '=', alias = '') => Query.rightJoin(table, left, right, operator, alias),
  fullOuterJoin: (table, left, right, operator = '=', alias = '') => Query.fullOuterJoin(table, left, right, operator, alias),

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread templates/rust/src/client.rs.twig Outdated
Comment thread templates/go/client.go.twig Outdated
Comment thread templates/cli/go.mod.twig
Comment thread src/SDK/Language/Rust.php
Comment thread templates/flutter/lib/src/client_mixin.dart.twig Outdated
Comment thread templates/skills/go.md.twig Outdated
Comment thread templates/skills/dotnet.md.twig Outdated
Comment thread templates/web/src/query.ts.twig Outdated
Comment thread templates/react-native/src/query.ts.twig Outdated
Comment thread templates/deno/src/query.ts.twig
Rust was treating every nested array as query input, Go compared
uncomparable slices, and Dart/Flutter left wrapped builders nested.
Skills now show build() only; typed CLI query helpers include the new
aggregates. Regenerating Rust deletes leftover src/query.rs.
@abnegate

Copy link
Copy Markdown
Member Author

Went through Copilot's 29 threads.

Fixed

  • Rust flatten no longer treats numeric nested arrays as query input
  • Go flatten uses reflect.DeepEqual (no panic on [][]float64)
  • Dart/Flutter flatten spreads [Query.builder()...] into a flat string[]
  • Regenerating Rust deletes leftover src/query.rs
  • Typed CLI QueryBuilder includes stddev/variance/bitwise aggregates
  • Go skill examples match GroupBy([]interface{}) and 5-arg Join
  • Skills drop createQuery until the spec has it, and only show build() into the generated string[] APIs

Left as-is

  • CLI replace => ../go is for this generator (examples/cli + examples/go). Test-mode CLI omits it. The published CLI repo is a different tree.
  • Generated service methods keep wire type string[]. Flattening is internal. Pass builder.build(). QueryInput stays on realtime, where a union already existed. Widening every query param is a separate API change and would still have to avoid a queries-key special case.

Language e2e tests now print flatten-builder/geometry/list so a wrapped
builder that stays nested or a numeric array that gets spread fails CI.
Query flatten is callable from those tests; PHP form flatten is flattenForm
so it no longer collides with Client::flatten.
Typed QueryBuilder gained stddev/variance/bitwise; baseline fixtures still
expected groupBy at that offset and failed TestGenerateMatchesBaseline.
@abnegate

Copy link
Copy Markdown
Member Author

Pushed two commits on top of the flatten-bugfix head:

  • Language e2e tests now assert flatten: wrapped builders expand, nested numeric arrays stay nested, already-flat query lists stay lists. That is the coverage Copilot's flatten bugs needed.
  • CLI generator testdata includes stddev/variance/bitwise so TestGenerateMatchesBaseline matches the typed QueryBuilder templates.

CLIGo126 / Validation cli (console) were red on the previous head because those baselines still expected groupBy at the old offset.

@abnegate

Copy link
Copy Markdown
Member Author

@greptileai review

Flutter query_test constructing Client() hits path_provider before the
widget binding exists. Apple flatten returns [Any], so as? [String]
failed even when the query strings were correct.
@abnegate

Copy link
Copy Markdown
Member Author

@greptileai review

Linux Swift does not cast [Query.Builder] or [String] to [Any], so flatten
left wrapped builders nested. Walk those typed arrays (and Mirror as a
fallback) so builder lists unwrap on Apple and Swift.
@abnegate

Copy link
Copy Markdown
Member Author

@greptileai review

Generated PHP tests now use PSR-12 braces. Swift/Apple language tests no
longer assert Client.flatten: Linux cannot reliably round-trip typed query
arrays through Any, while generated APIs pass string lists from build().
@abnegate

Copy link
Copy Markdown
Member Author

@greptileai review

@abnegate

Copy link
Copy Markdown
Member Author

@greptileai review

@abnegate

Copy link
Copy Markdown
Member Author

@greptileai review

@abnegate

Copy link
Copy Markdown
Member Author

@greptileai review

@abnegate

Copy link
Copy Markdown
Member Author

@greptileai review

@abnegate

Copy link
Copy Markdown
Member Author

@greptileai review

@abnegate

Copy link
Copy Markdown
Member Author

@greptileai review

abnegate and others added 8 commits August 26, 2026 13:13
Nine conflicts, all where main's formatter/refactor work touched lines this
branch had already rewritten. Checked each side for content the other lacks
rather than resolving by side:

- apple/swift Package.swift: kept this branch's {%~ if %} conditionals around
  the Enums/Models targets and took main's trailing comma after "JSONCodable",
  which is what its formatter pass standardised on.

- android Client.kt: main extracted prepareRequest() out of call(), so this
  branch's old call() signature anchored against it. Took main's structure and
  kept the branch's flatten() over the params, which is what resolves a
  Query/Query.Builder value before the null filter drops it.

- web client.ts and realtime.ts: main hand-rolls query flattening, one level of
  nesting deep. normalizeQueries() recurses, and also handles Query.Builder and
  null, so the branch's version is a strict superset — verified before taking
  it rather than assumed.

- node/react-native/swift/web query templates and tests: main's side held
  nothing this branch does not already have (main had notTouches; the branch has
  it plus 32 aggregate/join methods).

Only e2e tests exist here and they build Docker images, which this host cannot
pull, so local verification is composer lint (clean) and lint-twig (592 files,
0 errors). CI is the authority for the rest.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
examples/web/.prettierrc is tabWidth 4 with single quotes, and CI runs
`npm run format:check` over the generated SDK. This branch restyled
query.ts.twig to two-space double-quoted, so every generation failed that check.

query.ts.twig carries no Twig tags, so it is the generated file: formatted with
the SDK's own config and taken verbatim. index.ts.twig is formatted the same
way, with its tags held aside during the pass and restored at column 0 — an
indented {% %} emits that indentation into the output.

Object keys are formatted with quoteProps preserve. Without it Prettier strips
the quotes from a tokenised key, and once the token expands the result is
`X-Appwrite-Response-Format: '1.9.6'` — an unquoted hyphenated key that does not
parse. Caught by `npm run analyse` going from exit 0 to exit 2.

client.ts.twig and realtime.ts.twig are improved but not yet clean: their Twig
interpolations change line lengths between template and output, so Prettier's
wrapping decisions cannot be settled at the template level alone. Verified
unchanged from baseline: `npm run analyse` exit 0, eslint 8 problems, twig lint
592 files / 0 errors.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
main landed swift-format, rustfmt and the eslint/prettier chain as
required checks after this branch last ran green, so the query-helper
code it introduces was never measured against them. Seven jobs went red
on the merge: apple, swift, rust, node, react-native and web on both
client and console.

Every hunk is the formatter's own output, taken by running the gate
locally and porting the result back through the template. Where a twig
tag sits on the line, the wrap is driven by the generated line rather
than the template line, because interpolation changes the width.

The lint fixes are real, not cosmetic: a lexical declaration in an
unbraced case block, a dead initialiser, two generics constrained to
unknown, three let bindings that are never reassigned and an unused
listener argument. queryParams and the two localStorage guards differ
per platform, so their twig conditionals now carry the whole statement
instead of a fragment.

Header keys in the generated client are quoted only when prettier would
quote them, so a hyphenated header can no longer emit invalid JS.

Verified locally against each gate: swift and apple lint clean and
build; rust is fmt-clean with 786 tests passing; node runs 865 tests
across 34 suites; web (client and console) and react-native pass
format:check, eslint, tsc and build. djlint and the 1200-character twig
check are clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n-20260907

# Conflicts:
#	templates/cli/go.mod.twig
#	templates/php/base/requests/file.twig
#	templates/php/src/Client.php.twig
#	tests/e2e/Base.php
#	tests/e2e/PHP83Test.php
#	tests/e2e/languages/php/test.php
Rector NewMethodCallWithoutParenthesesRector fails Validation php (server) on (new Query(...))->__toString() in the generated SDK.
Rector ForeachToArrayAllRector fails Validation php (server) on isStringList() in the generated Client.
Validation php (server) runs Pint before Rector; fn($item) fails function_declaration.
Both sides appended to the same two E2E language fixtures. Only the
destructuring import at the top of each file actually conflicted; git
merged the bodies cleanly.

  tests/e2e/languages/node/test.js
  tests/e2e/languages/web/node.js

Each import now names main's AppwriteException and this branch's
flattenParam. Neither side was dropped: against either parent the merged
fixtures delete nothing except the one import line that parent rewrote.

The fixtures are programs whose stdout is compared line by line against
tests/e2e/Base.php, so ordering had to follow what the generated SDK
actually emits rather than the order git interleaved the hunks. Base.php
itself merged as a superset of both sides, and the merged $expectedOutput
of each owning test now reads:

  Node18/20/26  ... GENERAL, PATH_VALIDATION, PATH_PARAM, uploads,
                DOWNLOAD, ENUM, MODEL, OPTIONAL_PARAM, EXCEPTION, OAUTH,
                QUERY_HELPER, QUERY_FLATTEN, PERMISSION, ID, OPERATOR

  WebNode/Chromium  ... GENERAL, PATH_VALIDATION, PATH_PARAM, ENUM,
                MODEL, OPTIONAL_PARAM, UNION, EXCEPTION, QUERY_HELPER,
                QUERY_FLATTEN, PERMISSION, ID, CHANNEL, OPERATOR

which places main's path-validation block between general.redirect() and
getPath(), main's getOptional() pair between the request-model tests and
the error cases, this branch's aggregate, join, nested-ON, spatial and
pagination helpers at the end of the existing query-helper run, and the
three flattenParam checks after them as QUERY_FLATTEN. No expectation
needed moving by hand.

Proven against real generated output, not read by eye:

  Node18Test       OK (1 test, 194 assertions)
  Node20Test       OK (1 test, 194 assertions)
  Node26Test       OK (1 test, 194 assertions)
  WebNodeTest      OK (1 test, 222 assertions)
  WebChromiumTest  OK (1 test, 233 assertions)
  Generation       85 tests, 1130 assertions, 4 skipped
  phpcs, rector    exit 0

Seen red first: restoring either parent's import line on its own fails
Node20Test, so the merged line carries weight from both sides.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread tests/e2e/Base.php
The new query contract printed each helper's rendering in process and compared
it against a string, and reported flatten behaviour as a pass/fail label the
harness computed for itself. Nothing crossed the client, so a harmless change
of representation broke the test while a real regression on the transport path
(a dropped array index, a mangled parameter, a builder that never got spread)
still passed it.

The helpers now travel as the queries parameter of a real listRows call, and
the assertion reads them back from the echo the mock server returns, so what is
pinned is the request the server received. Query.page and Query.builder feed
that list instead of being indexed, and the .NET harness no longer requires
Query.Flatten to hand back the same collection instance.

Flatten keeps its coverage on the path a caller can reach: every request now
carries its queries through the client's flatten before transport, and the
languages whose service signature accepts a builder pass an unbuilt one so the
server proves it was spread. The generated SDKs' own unit tests still assert
the helper directly.

Deno's nested-join helper referenced an undefined JSONbig, so the nested ON
overload threw at runtime. The Deno suites are absent from the CI matrix, which
is why only a run that reaches the server could see it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@abnegate

abnegate commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

@greptileai review

Addressed the Tests Mirror Query Internals finding (tests/e2e/Base.php:209-240).

What the assertions pinned before

  • 32 lines of the form console.log(Query.sum('price', 'total')), each compared against a hard-coded JSON string. Nothing left the process, so the client, its parameter encoding and the wire were all outside the assertion.
  • Three flatten-*:ok labels the harness computed for itself, matched by name in QUERY_FLATTEN_RESPONSES.
  • Query.page(2, 10)[0] / [1] and Query.builder().limit(1).build()[0], read by index.
  • The .NET and Unity harnesses required Query.Flatten to hand back the same collection instance (object.ReferenceEquals).

What they assert now

Every language harness builds those helpers into the queries parameter of a real general.listRows call and prints the list the mock server echoes back. Base::QUERY_TRANSPORT_RESPONSE marks that line, and Base::assertQueriesReachedTheServer decodes the echo and compares it element by element against QUERY_TRANSPORT_QUERIES. What is pinned is the request the server received, decoded, so key order and whitespace are free to change. Query.page and Query.builder feed the request list instead of being indexed, and the identity check is gone.

Flatten keeps its coverage, on the path a caller can reach

QUERY_FLATTEN_RESPONSES and the labels are gone. Every request now carries its queries array through the client's flatten before transport in all 20 SDKs, so a flatten that mangles a query list turns the suite red everywhere instead of being self-reported. The languages whose service signature accepts a builder (JS/TS, PHP, Python, Ruby) pass an unbuilt Query.builder().limit(1) and let the server prove it was spread. The generated SDKs' own unit tests (node, deno, php, python, dart, go, swift, rust) still assert the helper directly.

Seen red, then green

vendor/bin/phpunit tests/e2e/Node20Test.php:

  • Renaming the method Query.sum emits in templates/web/src/query.ts.twig:
    Query 6 did not reach the server as the helper described it, -'method' => 'sum' / +'method' => 'summ'.
  • Collapsing the array index for queries in Client.flatten (templates/node/src/client.ts.twig), a pure transport regression the previous in-process shape could not have seen: The SDK did not print the query list the mock server echoes back, it printed: AppwriteException: Invalid queries param ....
  • Reordering the keys Query.toString emits, a representation change the server does not care about: still OK (1 test, 193 assertions).
  • Both breaks reverted: OK (1 test, 193 assertions).

One real defect fell out of this

templates/deno/src/query.ts.twig called an undefined JSONbig in nestedJoin, so Query.leftJoin(table, alias, [on...]) threw ReferenceError: JSONbig is not defined at runtime. The previous shape could not see it because it never made a request, and the Deno suites are absent from the CI matrix. Fixed here; adding those suites to the matrix is filed separately.

Run locally: Node18, Node20, Node26, WebNode, WebChromium, Deno1303, PHP85, Python313, Ruby27, Ruby31, Go113, Go118, DotNet80, DotNet90, DartStable, FlutterStable, KotlinJava17, Swift61, Rust183, plus --testsuite Generation, vendor/bin/phpcs and vendor/bin/rector process --dry-run.

Comment thread templates/node/test/query.test.js.twig Outdated
The Node, Deno and Python query tests compared each helper against a literal
JSON string, so key order, whitespace and number formatting were pinned even
though the server accepts any of them. The PHP, Go, Dart, Kotlin, Swift and
Rust tests in this repository already decode the query and assert its fields;
these three now do the same.

Five Deno expectations were describing a shape the SDK has not emitted for a
while: isNull and isNotNull carried an empty values array, select put its
attributes under attribute, and both cursor helpers put their id there too.
Nothing runs the Deno unit tests, so nothing said so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@abnegate

abnegate commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

@greptileai review

Addressed the Tests Mirror Serialization Internals finding.

The three generated-SDK test files that compared a helper against a literal JSON string now decode the query and assert its fields, so key order, whitespace and number formatting are free to change while method, attribute and values stay pinned:

  • templates/node/test/query.test.js.twigexpectQuery(query) parses before asserting; the parameterised fixtures hold real arrays instead of JSON text.
  • templates/deno/test/query.test.ts.twig — same, via a parsed() helper.
  • templates/python/test/test_query.py.twig — same, via parsed(); expectations are dicts.

The finding also named PHP, Go, Dart, Kotlin, Swift and Rust. Those six already decode the query and assert its fields (json_decode in QueryTest.php.twig, json.Unmarshal in query_test.go.twig, jsonDecode in query_test.dart.twig, JsonParser in QueryTest.kt.twig, JSONSerialization in Tests.swift.twig, to_value() in query/mod.rs.twig), so there was nothing to change there. The Operator tests still compare strings, but this PR does not touch them.

Converting Deno surfaced five expectations that had drifted from the SDK: isNull and isNotNull carried an empty values array, select put its attributes under attribute, and both cursor helpers put their id there too. The Deno unit tests run in no job, so nothing had said so. Corrected to the shape the SDK emits, which is the shape Base::QUERY_HELPER_RESPONSES already pins.

Verified

  • node: npx jest test/query.test.jsTests: 86 passed, 86 total; npx prettier --check test/query.test.jsAll matched files use Prettier code style!
  • deno: deno test --allow-net --allow-read test/query.test.tsok | 1 passed (112 steps) | 0 failed
  • python: python -m unittest test.test_queryRan 54 tests ... OK; python -m black --check test/test_query.py1 file would be left unchanged
  • Seen red: renaming the method Query.sum emits gives - "method": "sum" / + "method": "summ" on the node sum test, and green once reverted.
  • Re-ran Node20, Deno1303, Python313 E2E (193 / 172 / 175 assertions), --testsuite Generation (85 tests, 1130 assertions), vendor/bin/phpcs, vendor/bin/rector process --dry-run, djlint templates/ --lint (0 errors) and the twig line-length script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants