Use Utopia OpenAPI canonical specification models - #1789
Open
ChiragAgg5k wants to merge 28 commits into
Open
Conversation
Member
Author
Contributor
Greptile SummaryThe PR replaces the local OpenAPI parser and array projections with Utopia OpenAPI’s canonical typed models throughout parsing, orchestration, language helpers, tests, and templates. It also removes the local specification implementation and updates documentation and dependency metadata.
Confidence Score: 4/5The client-method discovery regression should be fixed before merging because generated clients currently lose the specification’s ping/get method. Filtering removes the only ping service operation before client-method discovery iterates service names, leaving templates without the client-level method they are designed to emit. Files Needing Attention: src/SDK/SDK.php Important Files Changed
Prompt To Fix All With AI### Issue 1
src/SDK/SDK.php:525-526
**Client method discovery drops ping**
When the specification contains `ping.get`, `getFilteredServices()` removes that client-level operation and drops the now-empty ping service before `getClientMethods()` scans service names, causing generated clients to omit the ping method.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "refactor: use utopia openapi models" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
|
Too many files changed for review (369 files, 200 file limit). Bypass the limit by tagging |
Diffing generated output against the published sdk-for-* repos surfaced seven places where the move to utopia-php/openapi silently changed what the generator emits. Each was invisible because Twig runs with strict_variables off, so an accessor that no longer resolves renders as an empty string instead of raising. - `definitions` is now a name-keyed map of Schema, and Schema has no `name`, so `def.name` never matched. That removed `convertTo<T>()` from every list model in Dart, Flutter, .NET and Ruby — a public method on two shipping SDKs. - `Parameter` has no `type`, so `param.type == 'file'` was never true and seven doc templates dropped their InputFile import while still calling InputFile in the body. The examples did not compile. - The REST and GraphQL request line was built by splitting the server URL, which drops the leading slash: `POST v1/...` is not a valid request-target. - `getResponseModel()` read from the `any`-filtered model list, so an `any` response rendered as an empty type name. Filtering `any` is correct for deciding whether a response is a union, but not for naming the type, so the two are now separate accessors. - Ruby lost its enum and file branches, downgrading 58 enum and 3 file `@param` tags to String. - The Python response docstring dropped the `Union[...]` wrapper and the collision alias, disagreeing with the annotation beside it. Both now derive from one filter so they cannot drift apart again. - Rust searched an array parameter's whole JSON example as a single scalar; array_search returned false, which indexes as 0, so only the first enum member rendered and it was picked by position rather than by value.
getTypeName() returns the fully-qualified enum name so callers that have no import can use it, and the propertyType filter already stripped the models package for exactly that reason. It did not strip the enums package, so model files emitted `io.appwrite.enums.AttributeStatus` beside the import that makes the qualifier redundant — a form that appears nowhere in the published Kotlin or Android SDKs.
The service template resolves a method name through the `methodName` filter now that `Operation` has no `name` property. Line 11 was migrated but the comparison inside the duplicate check was not, so `otherMethod.name` resolved to null, the snake-cased comparison never matched, and `shouldSkip` stayed false. Every deprecated method that has a non-deprecated replacement was therefore emitted alongside it — 23 duplicate `def`s across account, messaging and users. Ruby silently lets the later definition win, so which implementation callers got depended on declaration order. Python and Rust already used the filter in the same comparison; only Ruby was missed.
Generating each SDK from the live spec and diffing against both the
published repo and a pre-refactor worktree fed the same spec isolated
nine more differences that the same input renders differently.
- Web::getGenericTypes() read the unfiltered spec, so `any` — which is
deliberately not emitted as a model — produced
`<Any extends Models.Any = Models.DefaultAny>` on every GraphQL
method. Those types are never declared, so node, web and react-native
did not typecheck at all.
- Web::getSubSchema() gained multi-model union handling but not the
`Models.` qualifier its sibling getTypeName() applies, dropping the
prefix from 132 union members. It also resolved unions outside arrays,
turning `hashOptions?: object` into a seven-member union.
- The upload-id guard still filtered on `p.isUploadID`, which does not
exist on the new Parameter DTO, so it was always false and Swift,
Kotlin, .NET and Ruby all emitted `idParamName = nil`. That silently
disables resumable upload id handling.
- Go flattened any nested array to `[][]interface{}`, erasing the
element type and, for polygons, an entire dimension. Recursing handles
any depth.
- Swift rendered union and untyped array elements as dictionaries
instead of degrading the array to `[AnyCodable]`.
- .NET and Unity lost the null-safe `?.` on sub-model serialization,
which was deliberate and is what every published model uses.
- Multipart bodies carry no nullability, so an optional part must be
omitted rather than sent as an explicit null.
- The GraphQL marker header is emitted before the negotiated headers.
- Body parameters keep spec order; only the combined list is sorted
required-first, because only it becomes a method signature.
Also restores the trailing whitespace an unrelated cleanup stripped from
the shared CLI install scripts, and renders PHP doc defaults as the
escaped `{}` published uses rather than an unescaped `[]`.
A discriminated union whose members are told apart by more than one
property cannot be expressed with the standard `mapping`, which maps a
single property value to a single schema. Five attribute models share
`type: "string"` and differ only by `format`, so `mapping` names just
one of them and the spec carries the real rule set in `x-mapping`.
Reading only `mapping` collapsed attributeEmail, attributeEnum,
attributeUrl and attributeIp into attributeString: getAttribute() and
getColumn() returned the wrong model, silently, with format dropped.
Ten cases became six. The dispatch is emitted into runtime request code
in 22 templates, so every SDK was affected, and nothing failed loudly.
Cases are now ordered most-specific-first, because the one-condition
`{type: string}` case would otherwise match an email before its own
two-condition case was ever tested.
`x-mapping` was unreachable until utopia-php/openapi#2, which captures
extensions on Discriminator the way every other model already does;
composer.lock moves to that merge.
Measured by generating both this branch and main from one pinned spec, so every difference is generation drift rather than spec movement. Rust: a 204 response whose produced content type is recorded in x-appwrite still returns a body, and emptiness has always been decided from the produced types rather than the response codes. Reading codes instead narrowed 12 services and 23 doc examples from Result<serde_json::Value> to Result<()>, which is a breaking change for every caller that binds the result. Android and Unity: the service enum import was guarded on a value that never resolved, so it was never emitted, and signatures are written with fully qualified enum names — the import is redundant. Restoring the guard would be reintroducing a dead accessor, so the block is removed. Also restores two whitespace-only lines the migration dropped from the Unity templates. Drift against main for these three SDKs: rust 35 -> 3, android 14 -> 0, unity 16 -> 0. The three remaining Rust differences are nested numeric array defaults, where main emitted Vec<Vec<String>> for what the spec declares as numbers.
Re-measured against the published SDKs rather than main after finding the two disagree: main is not what the published repos were cut from, so published is the authority and main only shows what moved. A model property carries its whole schema tree, so an array of arrays of numbers resolves all the way down and published renders [][][]float64. A method parameter is flattened to one level before it reaches a language, so anything past the first nesting has nothing to resolve and published leaves it untyped. Both paths run through getTypeName, and the input type is what tells them apart, so the distinction lives in one predicate shared by Go, Dart, .NET and Rust rather than four copies. TypeScript unions name their members from outside the declaring namespace and are written Models.X; a single model is written bare. Qualifying both turned every single-model property into Models.X. Also restores the banner trailing space in the Web and Flutter templates. Drift against main: 212 -> 129. Go, .NET, Unity, Android, CLI, GraphQL, REST, Ruby, React Native and Web are now identical.
Kotlin writes a scalar enum property short, against the import the model file carries, but keeps the qualified name inside a List — stripping both turned every List<enum> into an unqualified name plus a redundant import. A list of enums is also cast straight across rather than decoded through its enum class. The old parser exposed an enum only on scalar properties, so arrays took the raw path; the filter now resolves the element enum, which pulled arrays onto the decoding path and changed both toMap() and from(). Nested array parameters render their element untyped in Kotlin and in the TypeScript SDKs, the same rule already applied to Go, Dart, .NET and Rust. Dart renders a union element as a bare List rather than List<Map>. Kotlin and Android are now identical to main; drift 129 -> 116.
A model property that is a list of models keeps its element type, because the decoder builds each element into that model. Every other list is annotated List[Any]: nothing enforces the element type on the way in, and narrowing it to List[str] declares a stricter contract than the published SDK does. Array properties are also not treated as nullable when deciding whether to wrap in Optional, which is how the published models read. This is scoped to the Python model template rather than the shared filter — the same rule applied globally moved PHP the wrong way, so it belongs to Python's model rendering and not to nullability in general. Drift 116 -> 71.
The old parser exposed an enum only on scalar properties and never carried a parameter's raw x-example key, and several places read those absent keys. The new DTOs resolve both, which quietly changed output: - Integer and number arguments in generated tests took the spec example where they had always been a literal 1 / 1.0. - A list of enums pulled model tests onto the enum-import path, and Python service imports onto a collision-suffixed alias, neither of which the published SDKs use. - Python annotated model fields from the schema's nullability and its own Optional wrapping on top of the template's, producing Optional[Optional[...]]. Model optionality comes from the declaring schema's required list alone, so the type is now built without any wrapping and the template applies it once. - Python fixtures render integers as float, matching the annotation, and leave booleans and numbers as the published fixtures have them. Nested array parameters render their element untyped in Python too. Also restores whitespace the migration dropped from the Flutter realtime response and the Python package __init__ files. Drift 71 -> 12. Python, Node, PHP, Dart and Flutter now generate identically; 16 of 19 SDKs are byte-identical.
A property that decodes to AnyCodable is mapped element by element from [Any]; the migration changed it to read [[String: Any]] and map each element's values, which only holds if every element is a dictionary. A union array whose members are scalars would trap on the cast. Nested array parameters render their element untyped here too, the same rule now shared by every language. Swift drift 10 -> 1, and the remaining file is the README product name.
An enum example that is not one of the enum's own values names no variant. Timezone is the case in point: the values are lower case (africa/abidjan) while the example is America/New_York, so the lookup misses. Falling back to the example text invents a variant that does not exist; the first member is used instead. The Swift package README renders an empty product name. The value came from an accessor that never resolved, so this is what every published Swift and Apple SDK contains, and generating anything else is drift. It is worth fixing, but as its own change against the published SDKs rather than silently inside a parser migration. All 19 SDKs now generate byte-identical output to main from the same spec: 16716 files, zero differences.
Three review points, all introduced while driving generation drift to
zero against the published SDKs.
The eight trailing spaces were re-added purely for byte parity. Every one
sits in a shell or doc comment, after a Write-Host call, or after a
Python expression, so none of them means anything in shell, PowerShell,
Dart, TypeScript or Python. Encoding them as {{ ' ' }} would have kept
meaningless output and made the templates harder to read, so they are
removed. The published SDKs pick up an eight-line whitespace-only diff
on their next regeneration.
The Python service test template was rewritten as LF when it has always
been CRLF, which churned all 47 lines. It is CRLF again, so the diff is
only the two guard changes.
The docblock for isUntypedNestedArray() ended up above
hasConcreteItemsType() when the second helper was inserted; the two are
reordered so each sits with its own description.
git diff --check now reports only the CR of that CRLF file, which git
counts as trailing whitespace by default and which any change to any of
the repository's existing CRLF files would report the same way:
git diff --check 16 hits
git -c core.whitespace=cr-at-eol diff --check 0 hits
…penapi # Conflicts: # templates/swift/README.md.twig
utopia-php/openapi#3 moved the schema classes up beside the models they are part of, so the base is Model\Schema and a kind is Model\ArraySchema rather than Model\Schema\Schema and Model\Schema\ArraySchema. The segment repeated what the class names already said, and a file importing a base type and two kinds read Schema three times per line. The rename is mechanical across 22 files. It also collapses what used to be two Utopia namespaces into one, so those import blocks sort as a single run; six that had drifted out of alphabetical order are sorted, one of them because this rename moved a name past its neighbour. No behaviour change. Generating all 19 SDKs before and after gives 16716 byte-identical files.
The package is on Packagist now, so it resolves like every other dependency and the VCS repository entry that pointed Composer at the GitHub URL is no longer needed. dev-main also meant any composer update silently pulled whatever main had become, for the package that defines the entire parsing contract this generator is built on. 0.1.* pins that to a released line and matches how the other utopia-php packages are constrained. The resolved code is the same commit the lock already pinned, so all 19 SDKs generate 16716 byte-identical files.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace the generator's local OpenAPI 2/3 parsers and legacy array projections with the canonical typed models from
utopia-php/openapi.What's Changed
utopia-php/openapithrough its GitHub VCS repository.src/Spec/*and theAppwrite\Specautoload namespace.Verification
All generation targets completed against
tests/resources/spec-openapi3.json. Representative SDKs were also generated against the Swagger 2 fixture, and generated PHP sources passed syntax checks.Dependency
Depends on
utopia-php/openapiPR #1, now merged intomain. It preserves whetheradditionalPropertieswas omitted instead of collapsing omission into explicittrue.