fix: validate string-list parameters across SDKs - #1899
Conversation
|
|
@greptile-apps Please refresh the review summary for current head |
|
|
||
| # String-list validation follows the declared item type across query and body | ||
| # parameters, before reaching the request boundary. | ||
| with requests_mock.Mocker() as http: |
There was a problem hiding this comment.
instead of this pattern of testing, we should follow existing pattern of logging the same response across all sdks. makes it concrete all sdks assert the same behaviour
| ]; | ||
|
|
||
| protected const ARRAY_PARAMETER_RESPONSES = [ | ||
| 'String list validation:passed', |
There was a problem hiding this comment.
try never print "passed". it should print a response specific to the test
ChiragAgg5k
left a comment
There was a problem hiding this comment.
lets update pr description to add code snippets of what new validation will be generated in the sdks as its hard to tell my twig templates
Print the SDK validation messages and the mock's responses so ARRAY_PARAMETER_RESPONSES compares real output, replacing the requests_mock matrix and its passed markers.
Include type, code and response alongside the message so the e2e output covers the exception fields callers branch on.
Generate the schema-driven string-list check for Node, Web, Deno, React Native, PHP, Ruby, Kotlin and Android, matching Python, and send PHP enum objects in GET query lists as their values. Split the e2e contract into ARRAY_PARAMETER_RESPONSES for every SDK, STRING_LIST_VALIDATION_RESPONSES for SDKs with the check, and Python-only NESTED_LIST_RESPONSES. The mock echoes createDocuments input inside result so typed models can print it.
Their List<String> parameters only take other items through unchecked casts, the same boundary as Swift, Dart and .NET, so drop the generated check and assert only the pass-through lines.
Generated SDKs accepted any value for parameters the spec declares as an array of strings. Python crashed while serializing a non-string item (
can only concatenate str (not "dict") to str), and Node, Web, Deno and React Native sent one as[object Object]without complaint. Services now validate these parameters from the schema, whatever the parameter is called, and every SDK's e2e prints the same responses for them.Generated code
Each method calls the validator after its required-parameter checks, once per parameter whose schema is an array of strings (enum lists included), passing the nullable flag when the item schema is nullable.
TablesDB.listRowsfrom the Console spec, in Node:Python, on
Service:Node, Web, React Native and Deno, on
Client:PHP, on
Service:Ruby, on
Service:Callers get the SDK's exception with type
sdk_input_validation, code0and no response, and no request is sent. String contents aren't inspected, so JSON query syntax is still validated by the API.Swift/Apple, Kotlin/Android, Dart/Flutter, .NET/Unity, Go and Rust type these parameters as string lists, so a non-string item only gets in if the caller subverts the type system (an unchecked cast, or Dart's
.cast<String>()). They get no validator.Python's
Client.flattenalso picks the list index before building keys:PHP GET requests now prepare params the way JSON bodies are, so enum objects in query lists are sent as their values instead of being dropped by
http_build_query.E2E
Expected lines in
tests/e2e/Base.php:ARRAY_PARAMETER_RESPONSES, every SDK: a string list with a non-JSON string and an enum value reaches the mock unchanged, in a query (listRows) and in a body (createDocumentslabels). The mock's documents route now echoesdocumentsandlabelsinsideresult, so typed models can print them; this replaces Swift and Apple'sOBJECT_ARRAY_RESPONSEScall.STRING_LIST_VALIDATION_RESPONSES, SDKs with the validator: the exception JSON for a query parameter (queries) and a body parameter (z), then nullable items passing.NESTED_LIST_RESPONSES, Python only: rawclient.callwith nested lists in a query string and in multipart fields.Unity (licence disabled) and Deno are not in the CI matrix; their scripts follow the same contract.
Not in this PR
client.callwith maps inside lists is mis-serialized outside Python (Kotlin/Android, Swift/Apple, .NET/Unity, Go, Rust, the JS family, Dart and Ruby). Generated methods don't send such values..urlHostAllowed, so&and=inside a value split it into extra parameters.Addresses MCP-X; appwrite/mcp#117 consumes the new exception type.