Request model support #1264
Request model support #1264
Conversation
WalkthroughThis pull request introduces comprehensive support for request models across the SDK generator ecosystem. Changes include: updating PHP platform requirements to 8.3-8.4; adding a new Player model, validator, and HTTP endpoints in the mock server; enhancing the Spec interface with type hints and a getRequestModels() method; extending all SDK language generators (16+ languages) to resolve model types and generate RequestModel files from templates; introducing request model templates for each language; and updating ~30+ test files to validate the new functionality with new Player creation test cases and expected responses. The feature enables SDKs to handle request-specific model definitions distinctly from response models. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/SDK/Language/Ruby.php (1)
269-269: Pre-existing typo:'nill'should be'nil'.This is a pre-existing issue not introduced by this PR, but worth noting: Ruby's null value is
nil, notnill.- $output .= 'nill'; + $output .= 'nil';src/Spec/Spec.php (1)
166-188: Stale docblock parameter.Line 168 references
@param array $parameterbut the method signature has$typeas the third parameter, not$parameter./** * Set Attribute * * Method for setting a specific field attribute * * @param string $key * @param mixed $value - * @param array $parameter + * @param string $type * @return mixed */ public function setAttribute(string $key, mixed $value, $type = self::SET_TYPE_ASSIGN): mixed
🧹 Nitpick comments (17)
templates/php/src/Services/Service.php.twig (1)
9-23: Model imports for service methods look good; consider separating enum/model dedupeThe new
parameter.modelhandling correctly addsModels\...imports only once per model and mirrors the existing enum pattern, so generated services will have the right model classes in scope.Since
addedis shared between enums and models, if you ever have an enum and a model with the same base name, one of theusestatements could be skipped. If that’s a realistic case, consider tracking them separately (e.g.,addedEnumsvsaddedModels) to avoid accidental collisions.src/SDK/Language/CLI.php (1)
347-366: Improved CLI type resolution for model-backed paramsThe new branches in
getTypeName()forarray.modelandmodelnicely ensure model-backed parameters resolve to a concrete PascalCase type (with[]for arrays) instead of falling through to generic mappings—this should make generated CLI types much more accurate.Minor nit: the docblock still refers to the second parameter as
$nestedTypes, but the signature uses$spec. If you touch this again, aligning those names would avoid confusion for readers and tooling.src/SDK/Language/ReactNative.php (1)
131-150: React Native model types now resolved explicitlyThe added checks for
array.modelandmodelingetTypeName()correctly map model-backed parameters toModels.<PascalCase>(and[]for arrays), which should give much stronger typings for request models and other modeled inputs without disturbing the existingresponseModel-based object handling.Similar to CLI, the docblock still names the second argument
$nestedTypeswhile the method uses$method; if you revisit this, updating the annotation would keep docs and implementation in sync.templates/php/src/Models/RequestModel.php.twig (2)
74-74: Simplify the complex ternary logic for better readability.Line 74 contains deeply nested ternary operators handling sub_schema, arrays, and enums in a single expression. This makes the template difficult to read and maintain.
Consider breaking this into a Twig macro or helper function that handles the conversion logic more explicitly:
'{{ property.name }}' => {{ property | convertToArrayValue }},Or split the logic across multiple lines using Twig's set/if blocks before the array definition.
84-86: Add error handling for JSON encoding.The
json_encode()call can returnfalseon failure (e.g., for non-UTF-8 strings or circular references), but the method signature promises astringreturn type.Consider adding error handling:
public function toJson(): string { - return json_encode($this->toArray()); + $json = json_encode($this->toArray()); + if ($json === false) { + throw new \RuntimeException('Failed to encode model to JSON: ' . json_last_error_msg()); + } + return $json; }tests/languages/ruby/tests.rb (1)
93-96: Optional: Add trailing comma for consistency.RuboCop suggests adding a trailing comma after the last array item for consistency with Ruby style conventions.
Apply this diff:
response = general.create_players(players: [ {id: 'player1', name: 'John Doe', score: 100}, - {id: 'player2', name: 'Jane Doe', score: 200} + {id: 'player2', name: 'Jane Doe', score: 200}, ])src/SDK/Language/Swift.php (2)
310-314: Request-model file mapping for Swift looks consistent; double-check path conventionThe new
requestModelscope entry mirrors the existingdefinitionscope (same...{{ spec.title | caseUcfirst}}Models/...andswift/Sources/Models/*.twigusage), so it should integrate cleanly into generation.One thing to verify: other Swift files (e.g.,
Error.swift,UploadProgress.swift) use/Sources/{{ spec.title | caseUcfirst}}/Models/...with a slash beforeModels, while both thedefinitionand newrequestModelentries use{{ spec.title | caseUcfirst}}Modelswithout the slash. If the intention isSources/AppwriteModels/..., this is fine; if the desired layout isSources/Appwrite/Models/..., you may want to adjust both entries together for consistency.
335-341: Model and array-of-model type resolution ingetTypeNamelooks correct; consider minor robustness tweakThe new branches correctly map:
array.model→[<SpecTitle>Models.<ModelName>]model→<SpecTitle>Models.<ModelName>or[<SpecTitle>Models.<ModelName>]whentype === TYPE_ARRAY.To make this a bit more defensive in case
$parameter['array']or$parameter['type']is missing for some inputs, you could mirror the laterTYPE_ARRAYbranch and use null-coalescing/local temporaries, e.g.:$array = $parameter['array'] ?? null; if (!empty($array['model'])) { return '[' . ($spec['title'] ?? '') . 'Models.' . $this->toPascalCase($array['model']) . ']'; } $model = $parameter['model'] ?? null; if (!empty($model)) { $modelType = ($spec['title'] ?? '') . 'Models.' . $this->toPascalCase($model); return (($parameter['type'] ?? null) === self::TYPE_ARRAY) ? '[' . $modelType . ']' : $modelType; }This keeps behavior the same for well-formed parameters while reducing the chance of notices if a caller passes an unexpected shape.
src/SDK/SDK.php (1)
707-717: LGTM! Request model generation implemented correctly.The new
requestModelcase follows the established pattern used fordefinitiongeneration. The implementation properly iterates through request models, respects exclusion rules, and renders templates.Consider whether the
setExclude()method (line 765) andexclude()method (line 788) should be extended to support request model exclusions in the future. The current implementation only handles services, methods, and definitions, which may be sufficient for now but could be expanded if selective request model exclusion becomes necessary.mock-server/src/Utopia/Model/Player.php (1)
16-23: Consider defensive handling for missing array keys.The
fromArraymethod directly accesses array keys without checking for their existence. If a malformed payload is passed, this will throw anUndefined array keyerror rather than a more descriptive exception.public static function fromArray(array $value): static { + if (!isset($value['id'], $value['name'], $value['score'])) { + throw new \InvalidArgumentException('Missing required keys: id, name, score'); + } return new static( id: $value['id'], name: $value['name'], score: $value['score'], ); }templates/swift/Sources/Models/RequestModel.swift.twig (1)
36-43: Consider extracting complex mapping logic for readability.The inline logic on line 39 handles multiple cases (sub_schema arrays, sub_schema objects, enums, plain values) in a single dense line. While functional, this could be challenging to maintain.
Consider breaking out the value mapping into a Twig macro or splitting across multiple lines for clarity:
{%~ for property in requestModel.properties %} {% set propName = property.name | escapeSwiftKeyword | removeDollarSign %} {% set optionalChain = property.required ? '' : '?' %} {% if property.sub_schema %} {% if property.type == 'array' %} "{{ property.name }}": {{ propName }}{{ optionalChain }}.map { $0.toMap() }, {% else %} "{{ property.name }}": {{ propName }}{{ optionalChain }}.toMap(), {% endif %} {% elseif property.enum %} "{{ property.name }}": {{ propName }}{{ optionalChain }}.rawValue, {% else %} "{{ property.name }}": {{ propName }}, {% endif %} {%~ endfor %}templates/android/library/src/main/java/io/package/models/RequestModel.kt.twig (1)
1-30: Template duplication withtemplates/kotlin/.../RequestModel.kt.twig.This template is identical to
templates/kotlin/src/main/kotlin/io/appwrite/models/RequestModel.kt.twig. While this may be intentional to maintain separate Android and Kotlin SDK templates, consider whether a shared template could reduce maintenance burden if the logic remains identical.The template itself is correctly structured for generating Kotlin request model classes.
templates/python/package/models/request_model.py.twig (1)
33-36: Consider movingjsonimport to file level.The
jsonmodule import insideto_json()works but is unconventional. Moving it to the file-level imports would be more Pythonic and avoid repeated import overhead if called multiple times.-from typing import Optional, List, Dict, Any +from typing import Optional, List, Dict, Any +import json ... def to_json(self) -> str: """Convert the model to a JSON string.""" - import json return json.dumps(self.to_map())templates/ruby/lib/container/models/request_model.rb.twig (1)
29-40: Inconsistent use ofremoveDollarSignfilter.On line 32,
removeDollarSignis applied to properties withsub_schemabut not to regular properties in the else branch. If the property name can contain a$character, this inconsistency could cause issues.Consider applying the filter consistently:
- "{{ property.name }}": {% if property.sub_schema %}{% if property.type == 'array' %}@{{ property.name | caseSnake | escapeKeyword | removeDollarSign }}.map { |it| it.to_map }{% else %}@{{property.name | caseSnake | escapeKeyword | removeDollarSign }}.to_map{% endif %}{% else %}@{{property.name | caseSnake | escapeKeyword }}{% endif %}{% if not loop.last %},{% endif %} + "{{ property.name }}": {% if property.sub_schema %}{% if property.type == 'array' %}@{{ property.name | caseSnake | escapeKeyword | removeDollarSign }}.map { |it| it.to_map }{% else %}@{{property.name | caseSnake | escapeKeyword | removeDollarSign }}.to_map{% endif %}{% else %}@{{property.name | caseSnake | escapeKeyword | removeDollarSign }}{% endif %}{% if not loop.last %},{% endif %}mock-server/src/Utopia/Response.php (2)
32-36: Unused constructor parameter.The
$responseparameter is declared but never used in the constructor body. If this is intentional (to satisfy the parent signature without using it), consider adding a comment or suppression annotation to clarify intent.public function __construct(SwooleResponse $response) { + // Intentionally not calling parent constructor or using $response }
257-260: Consider using strict null comparison.Using
!=for null comparison works but!==is preferred for clarity and to avoid potential type coercion issues.public static function hasFilter(): bool { - return self::$filter != null; + return self::$filter !== null; }src/Spec/Swagger2.php (1)
516-559: Significant code duplication withgetDefinitions().
getRequestModels()shares ~90% of its logic withgetDefinitions(). Consider extracting the common property enrichment logic into a private helper method to reduce duplication and ensure consistent behavior.Also, the same
descriptiontype issue exists on line 527:$model = [ 'name' => $key, 'properties' => $schema['properties'] ?? [], - 'description' => $schema['description'] ?? [], + 'description' => $schema['description'] ?? '', 'required' => $schema['required'] ?? [], 'additionalProperties' => $schema['additionalProperties'] ?? [] ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
mock-server/composer.lockis excluded by!**/*.lock
📒 Files selected for processing (89)
composer.json(1 hunks)example.php(1 hunks)mock-server/app/http.php(3 hunks)mock-server/composer.json(1 hunks)mock-server/src/Utopia/Model.php(2 hunks)mock-server/src/Utopia/Model/Player.php(1 hunks)mock-server/src/Utopia/Response.php(6 hunks)mock-server/src/Utopia/Validator/Player.php(1 hunks)src/SDK/Language/Android.php(1 hunks)src/SDK/Language/Apple.php(1 hunks)src/SDK/Language/CLI.php(1 hunks)src/SDK/Language/Dart.php(2 hunks)src/SDK/Language/Deno.php(1 hunks)src/SDK/Language/DotNet.php(2 hunks)src/SDK/Language/Flutter.php(1 hunks)src/SDK/Language/Go.php(2 hunks)src/SDK/Language/JS.php(1 hunks)src/SDK/Language/Kotlin.php(2 hunks)src/SDK/Language/Node.php(2 hunks)src/SDK/Language/PHP.php(2 hunks)src/SDK/Language/Python.php(2 hunks)src/SDK/Language/ReactNative.php(1 hunks)src/SDK/Language/Ruby.php(2 hunks)src/SDK/Language/Swift.php(2 hunks)src/SDK/Language/Web.php(1 hunks)src/SDK/SDK.php(2 hunks)src/Spec/Spec.php(5 hunks)src/Spec/Swagger2.php(8 hunks)templates/android/library/src/main/java/io/package/models/RequestModel.kt.twig(1 hunks)templates/dart/lib/models.dart.twig(1 hunks)templates/dart/lib/src/models/request_model.dart.twig(1 hunks)templates/dotnet/Package/Models/RequestModel.cs.twig(1 hunks)templates/go/models/request_model.go.twig(1 hunks)templates/kotlin/src/main/kotlin/io/appwrite/models/RequestModel.kt.twig(1 hunks)templates/node/src/models/request_model.ts.twig(1 hunks)templates/php/src/Models/RequestModel.php.twig(1 hunks)templates/php/src/Services/Service.php.twig(1 hunks)templates/python/package/models/request_model.py.twig(1 hunks)templates/ruby/lib/container/models/request_model.rb.twig(1 hunks)templates/swift/Sources/Models/RequestModel.swift.twig(1 hunks)tests/Android14Java11Test.php(1 hunks)tests/Android14Java17Test.php(1 hunks)tests/Android14Java8Test.php(1 hunks)tests/Android5Java17Test.php(1 hunks)tests/AppleSwift56Test.php(1 hunks)tests/Base.php(2 hunks)tests/DartBetaTest.php(1 hunks)tests/DartStableTest.php(1 hunks)tests/Deno1193Test.php(1 hunks)tests/Deno1303Test.php(1 hunks)tests/DotNet60Test.php(1 hunks)tests/DotNet80Test.php(1 hunks)tests/DotNet90Test.php(1 hunks)tests/FlutterBetaTest.php(1 hunks)tests/FlutterStableTest.php(1 hunks)tests/KotlinJava11Test.php(1 hunks)tests/KotlinJava17Test.php(1 hunks)tests/KotlinJava8Test.php(1 hunks)tests/Node16Test.php(1 hunks)tests/Node18Test.php(1 hunks)tests/Node20Test.php(1 hunks)tests/PHP80Test.php(1 hunks)tests/PHP83Test.php(1 hunks)tests/Python310Test.php(1 hunks)tests/Python311Test.php(1 hunks)tests/Python312Test.php(1 hunks)tests/Python313Test.php(1 hunks)tests/Python39Test.php(1 hunks)tests/Ruby27Test.php(1 hunks)tests/Ruby30Test.php(1 hunks)tests/Ruby31Test.php(1 hunks)tests/Swift56Test.php(1 hunks)tests/WebChromiumTest.php(1 hunks)tests/WebNodeTest.php(1 hunks)tests/languages/android/Tests.kt(2 hunks)tests/languages/apple/Tests.swift(1 hunks)tests/languages/dart/tests.dart(1 hunks)tests/languages/deno/tests.ts(1 hunks)tests/languages/dotnet/Tests.cs(1 hunks)tests/languages/flutter/tests.dart(1 hunks)tests/languages/kotlin/Tests.kt(2 hunks)tests/languages/node/test.js(1 hunks)tests/languages/php/test.php(2 hunks)tests/languages/python/tests.py(1 hunks)tests/languages/ruby/tests.rb(1 hunks)tests/languages/swift/Tests.swift(1 hunks)tests/languages/web/index.html(1 hunks)tests/languages/web/node.js(1 hunks)tests/resources/spec.json(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (56)
tests/languages/web/node.js (1)
tests/languages/node/test.js (1)
general(28-28)
src/SDK/Language/Dart.php (1)
src/SDK/Language.php (1)
toPascalCase(116-119)
tests/Android5Java17Test.php (1)
tests/Base.php (1)
Base(17-335)
tests/FlutterStableTest.php (1)
tests/Base.php (1)
Base(17-335)
tests/Python311Test.php (1)
tests/Base.php (1)
Base(17-335)
mock-server/app/http.php (2)
mock-server/src/Utopia/Model/Player.php (1)
Player(7-24)mock-server/src/Utopia/Validator/Player.php (1)
Player(8-29)
tests/Deno1303Test.php (1)
tests/Base.php (1)
Base(17-335)
tests/languages/swift/Tests.swift (4)
tests/languages/node/test.js (1)
general(28-28)tests/languages/web/node.js (1)
general(11-11)mock-server/src/Utopia/Model/Player.php (1)
Player(7-24)mock-server/src/Utopia/Validator/Player.php (1)
Player(8-29)
tests/DartBetaTest.php (1)
tests/Base.php (1)
Base(17-335)
tests/KotlinJava11Test.php (1)
tests/Base.php (1)
Base(17-335)
mock-server/src/Utopia/Validator/Player.php (4)
mock-server/src/Utopia/Model.php (2)
Model(7-166)getType(67-67)mock-server/src/Utopia/Model/Player.php (1)
Player(7-24)src/Spec/Spec.php (1)
getDescription(53-53)src/Spec/Swagger2.php (1)
getDescription(20-23)
tests/WebChromiumTest.php (1)
tests/Base.php (1)
Base(17-335)
tests/Ruby27Test.php (1)
tests/Base.php (1)
Base(17-335)
tests/Python313Test.php (1)
tests/Base.php (1)
Base(17-335)
mock-server/src/Utopia/Model/Player.php (2)
mock-server/src/Utopia/Model.php (1)
Model(7-166)mock-server/src/Utopia/Validator/Player.php (1)
Player(8-29)
tests/Node16Test.php (1)
tests/Base.php (1)
Base(17-335)
tests/languages/apple/Tests.swift (4)
tests/languages/node/test.js (1)
general(28-28)tests/languages/web/node.js (1)
general(11-11)mock-server/src/Utopia/Model/Player.php (1)
Player(7-24)mock-server/src/Utopia/Validator/Player.php (1)
Player(8-29)
src/SDK/Language/Web.php (1)
src/SDK/Language.php (1)
toPascalCase(116-119)
tests/KotlinJava8Test.php (1)
tests/Base.php (1)
Base(17-335)
tests/languages/node/test.js (1)
tests/languages/web/node.js (2)
response(4-4)general(11-11)
src/SDK/Language/DotNet.php (1)
src/SDK/Language.php (1)
toPascalCase(116-119)
tests/PHP80Test.php (1)
tests/Base.php (1)
Base(17-335)
src/SDK/Language/ReactNative.php (1)
src/SDK/Language.php (1)
toPascalCase(116-119)
tests/languages/deno/tests.ts (2)
tests/languages/node/test.js (2)
response(18-18)general(28-28)tests/languages/web/node.js (2)
response(4-4)general(11-11)
tests/Python310Test.php (1)
tests/Base.php (1)
Base(17-335)
tests/Android14Java11Test.php (1)
tests/Base.php (1)
Base(17-335)
tests/Python312Test.php (1)
tests/Base.php (1)
Base(17-335)
src/SDK/Language/Python.php (1)
src/SDK/Language.php (1)
toPascalCase(116-119)
tests/WebNodeTest.php (1)
tests/Base.php (1)
Base(17-335)
tests/Swift56Test.php (1)
tests/Base.php (1)
Base(17-335)
tests/Android14Java8Test.php (1)
tests/Base.php (1)
Base(17-335)
src/SDK/Language/CLI.php (1)
src/SDK/Language.php (1)
toPascalCase(116-119)
tests/Android14Java17Test.php (1)
tests/Base.php (1)
Base(17-335)
tests/Ruby30Test.php (1)
tests/Base.php (1)
Base(17-335)
tests/PHP83Test.php (1)
tests/Base.php (1)
Base(17-335)
src/SDK/Language/Deno.php (1)
src/SDK/Language.php (1)
toPascalCase(116-119)
tests/AppleSwift56Test.php (1)
tests/Base.php (1)
Base(17-335)
tests/DotNet60Test.php (1)
tests/Base.php (1)
Base(17-335)
src/SDK/Language/Kotlin.php (1)
src/SDK/Language.php (1)
toPascalCase(116-119)
tests/DartStableTest.php (1)
tests/Base.php (1)
Base(17-335)
src/SDK/Language/Node.php (1)
src/SDK/Language.php (1)
toPascalCase(116-119)
src/SDK/Language/JS.php (1)
src/SDK/Language.php (1)
toPascalCase(116-119)
tests/languages/dotnet/Tests.cs (4)
tests/languages/node/test.js (1)
general(28-28)tests/languages/web/node.js (1)
general(11-11)mock-server/src/Utopia/Model/Player.php (1)
Player(7-24)mock-server/src/Utopia/Validator/Player.php (1)
Player(8-29)
src/SDK/Language/PHP.php (1)
src/SDK/Language.php (1)
toPascalCase(116-119)
src/SDK/Language/Go.php (1)
src/SDK/Language.php (1)
toPascalCase(116-119)
tests/languages/kotlin/Tests.kt (3)
mock-server/src/Utopia/Model/Player.php (1)
Player(7-24)mock-server/src/Utopia/Validator/Player.php (1)
Player(8-29)tests/languages/android/Tests.kt (1)
writeToFile(315-318)
src/SDK/SDK.php (2)
src/Spec/Spec.php (1)
getRequestModels(133-133)src/Spec/Swagger2.php (1)
getRequestModels(516-559)
tests/Ruby31Test.php (1)
tests/Base.php (1)
Base(17-335)
tests/FlutterBetaTest.php (1)
tests/Base.php (1)
Base(17-335)
tests/languages/php/test.php (2)
mock-server/src/Utopia/Model/Player.php (1)
Player(7-24)mock-server/src/Utopia/Validator/Player.php (1)
Player(8-29)
tests/DotNet80Test.php (1)
tests/Base.php (1)
Base(17-335)
tests/KotlinJava17Test.php (1)
tests/Base.php (1)
Base(17-335)
src/SDK/Language/Swift.php (1)
src/SDK/Language.php (1)
toPascalCase(116-119)
tests/DotNet90Test.php (1)
tests/Base.php (1)
Base(17-335)
mock-server/src/Utopia/Response.php (1)
mock-server/src/Utopia/Model.php (2)
filter(50-53)Model(7-166)
src/Spec/Spec.php (1)
src/Spec/Swagger2.php (19)
getTitle(12-15)getDescription(20-23)getNamespace(28-31)getVersion(36-39)getEndpoint(44-49)getEndpointDocs(54-59)getLicenseName(64-67)getLicenseURL(72-75)getContactName(80-83)getContactURL(88-91)getContactEmail(96-99)getServices(104-136)getMethods(300-339)getTargetNamespace(432-435)getGlobalHeaders(440-457)getDefinitions(459-509)getRequestModels(516-559)getRequestEnums(564-587)getResponseEnums(592-629)
🪛 GitHub Actions: Twig Linting
templates/python/package/models/request_model.py.twig
[error] 6-6: H014 6:3 Found extra blank lines. endfor %}. djlint lint reported a formatting issue.
🪛 RuboCop (1.81.7)
tests/languages/ruby/tests.rb
[convention] 95-95: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: build (8.3, Python313)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, Python39)
- GitHub Check: build (8.3, Ruby30)
- GitHub Check: build (8.3, KotlinJava17)
- GitHub Check: build (8.3, WebNode)
- GitHub Check: build (8.3, PHP83)
- GitHub Check: build (8.3, DartBeta)
- GitHub Check: build (8.3, KotlinJava8)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: build (8.3, KotlinJava11)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: dotnet (server)
- GitHub Check: kotlin (server)
- GitHub Check: swift (server)
- GitHub Check: flutter (client)
- GitHub Check: android (client)
- GitHub Check: apple (client)
| use Utopia\MockServer\Utopia\Response; | ||
| use Utopia\Swoole\Request; | ||
| use Utopia\Swoole\Response as UtopiaSwooleResponse; | ||
| use Utopia\Validator\JSON; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n '\bJSON\b' mock-server/app/http.phpRepository: appwrite/sdk-generator
Length of output: 95
Remove unused import: Utopia\Validator\JSON.
The JSON validator is imported on line 21 but is not used anywhere in this file.
🤖 Prompt for AI Agents
In mock-server/app/http.php around line 21, the file imports
Utopia\Validator\JSON which is unused; remove the unused import line (delete the
"use Utopia\Validator\JSON;" statement) to clean up imports and avoid dead code
warnings, then run a quick linter or PHP syntax check to confirm no references
remain.
| "platform": { | ||
| "php": "8.2" | ||
| "php": "8.4" | ||
| }, | ||
| "config": { | ||
| "allow-plugins": { | ||
| "php-http/discovery": true, | ||
| "tbachert/spi": true | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the root composer.json
find . -maxdepth 2 -name "composer.json" -type fRepository: appwrite/sdk-generator
Length of output: 109
🏁 Script executed:
# Check root composer.json for PHP version constraint
cat -n composer.json | grep -A 5 -B 5 "php"Repository: appwrite/sdk-generator
Length of output: 1127
🏁 Script executed:
# Check mock-server composer.json dependencies
cat -n mock-server/composer.jsonRepository: appwrite/sdk-generator
Length of output: 873
🏁 Script executed:
# Look for CI configuration files
find . -type f \( -name ".github" -o -name ".gitlab-ci.yml" -o -name ".travis.yml" -o -name "phpunit.xml" \) 2>/dev/null | head -20Repository: appwrite/sdk-generator
Length of output: 79
🏁 Script executed:
# Search for workflow files in .github
find .github -type f -name "*.yml" -o -name "*.yaml" 2>/dev/null | head -20Repository: appwrite/sdk-generator
Length of output: 275
🏁 Script executed:
cat -n .github/workflows/tests.ymlRepository: appwrite/sdk-generator
Length of output: 3426
🏁 Script executed:
# Search for references to the plugins in the codebase
rg "php-http/discovery|tbachert/spi" --type jsonRepository: appwrite/sdk-generator
Length of output: 1052
Mock-server PHP 8.4 platform constraint: ensure CI/runtime alignment
The mock-server platform is set to PHP 8.4, which is stricter than the root constraint ("php": ">=8.3"). The allowed plugins (php-http/discovery and tbachert/spi) are available as transitive dependencies and won't cause Composer warnings.
However, the CI workflow currently runs tests with PHP 8.3 only (line 15 of .github/workflows/tests.yml), not 8.4. Confirm that your mock-server runtime environment actually uses PHP 8.4, or adjust the platform constraint if testing remains on 8.3.
| /** | ||
| * @return string | ||
| */ | ||
| abstract public function getGlobalHeaders(); | ||
| abstract public function getGlobalHeaders(): array; |
There was a problem hiding this comment.
Incorrect docblock: @return string should be @return array.
The docblock on line 119 says @return string but the method signature declares array return type.
/**
- * @return string
+ * @return array
*/
abstract public function getGlobalHeaders(): array;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @return string | |
| */ | |
| abstract public function getGlobalHeaders(); | |
| abstract public function getGlobalHeaders(): array; | |
| /** | |
| * @return array | |
| */ | |
| abstract public function getGlobalHeaders(): array; |
🤖 Prompt for AI Agents
In src/Spec/Spec.php around lines 118 to 121, the docblock incorrectly states
"@return string" while the abstract method signature declares it returns an
array; update the docblock to "@return array" (or remove the docblock return tag
if you prefer relying on the scalar type) so the PHPDoc matches the method
signature.
| if ($schema['x-request-model'] ?? false) { | ||
| continue; | ||
| } | ||
| $model = [ | ||
| 'name' => $key, | ||
| 'properties' => $schema['properties'] ?? [], | ||
| 'description' => $schema['description'] ?? [], | ||
| 'required' => $schema['required'] ?? [], | ||
| 'additionalProperties' => $schema['additionalProperties'] ?? [] | ||
| ]; | ||
| if (isset($sch['properties'])) { | ||
| foreach ($sch['properties'] as $name => $def) { | ||
| $sch['properties'][$name]['name'] = $name; | ||
| $sch['properties'][$name]['description'] = $def['description']; | ||
| $sch['properties'][$name]['example'] = $def['x-example']; | ||
| $sch['properties'][$name]['required'] = in_array($name, $sch['required']); | ||
| if (isset($model['properties'])) { | ||
| foreach ($model['properties'] as $name => $def) { | ||
| $model['properties'][$name]['name'] = $name; | ||
| $model['properties'][$name]['description'] = $def['description']; | ||
| $model['properties'][$name]['example'] = $def['x-example']; | ||
| $model['properties'][$name]['required'] = in_array($name, $model['required']); | ||
| if (isset($def['items']['$ref'])) { | ||
| //nested model | ||
| $sch['properties'][$name]['sub_schema'] = str_replace('#/definitions/', '', $def['items']['$ref']); | ||
| $model['properties'][$name]['sub_schema'] = str_replace('#/definitions/', '', $def['items']['$ref']); | ||
| } | ||
|
|
||
| if (isset($def['items']['x-anyOf'])) { | ||
| //nested model | ||
| $sch['properties'][$name]['sub_schemas'] = \array_map(fn($schema) => str_replace('#/definitions/', '', $schema['$ref']), $def['items']['x-anyOf']); | ||
| $model['properties'][$name]['sub_schemas'] = \array_map(fn($schema) => str_replace('#/definitions/', '', $schema['$ref']), $def['items']['x-anyOf']); | ||
| } | ||
|
|
||
| if (isset($def['items']['x-oneOf'])) { | ||
| //nested model | ||
| $sch['properties'][$name]['sub_schemas'] = \array_map(fn($schema) => str_replace('#/definitions/', '', $schema['$ref']), $def['items']['x-oneOf']); | ||
| $model['properties'][$name]['sub_schemas'] = \array_map(fn($schema) => str_replace('#/definitions/', '', $schema['$ref']), $def['items']['x-oneOf']); | ||
| } | ||
|
|
||
| if (isset($def['enum'])) { | ||
| // enum property | ||
| $sch['properties'][$name]['enum'] = $def['enum']; | ||
| $sch['properties'][$name]['enumName'] = $def['x-enum-name'] ?? ucfirst($key) . ucfirst($name); | ||
| $sch['properties'][$name]['enumKeys'] = $def['x-enum-keys'] ?? []; | ||
| $model['properties'][$name]['enum'] = $def['enum']; | ||
| $model['properties'][$name]['enumName'] = $def['x-enum-name'] ?? ucfirst($key) . ucfirst($name); | ||
| $model['properties'][$name]['enumKeys'] = $def['x-enum-keys'] ?? []; | ||
| } | ||
| } | ||
| } | ||
| $list[$key] = $model; | ||
| } | ||
| return $list; | ||
| } |
There was a problem hiding this comment.
Type mismatch: description default should be a string, not an array.
On line 473, 'description' => $schema['description'] ?? [] uses an empty array as the default, but description is expected to be a string.
$model = [
'name' => $key,
'properties' => $schema['properties'] ?? [],
- 'description' => $schema['description'] ?? [],
+ 'description' => $schema['description'] ?? '',
'required' => $schema['required'] ?? [],
'additionalProperties' => $schema['additionalProperties'] ?? []
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ($schema['x-request-model'] ?? false) { | |
| continue; | |
| } | |
| $model = [ | |
| 'name' => $key, | |
| 'properties' => $schema['properties'] ?? [], | |
| 'description' => $schema['description'] ?? [], | |
| 'required' => $schema['required'] ?? [], | |
| 'additionalProperties' => $schema['additionalProperties'] ?? [] | |
| ]; | |
| if (isset($sch['properties'])) { | |
| foreach ($sch['properties'] as $name => $def) { | |
| $sch['properties'][$name]['name'] = $name; | |
| $sch['properties'][$name]['description'] = $def['description']; | |
| $sch['properties'][$name]['example'] = $def['x-example']; | |
| $sch['properties'][$name]['required'] = in_array($name, $sch['required']); | |
| if (isset($model['properties'])) { | |
| foreach ($model['properties'] as $name => $def) { | |
| $model['properties'][$name]['name'] = $name; | |
| $model['properties'][$name]['description'] = $def['description']; | |
| $model['properties'][$name]['example'] = $def['x-example']; | |
| $model['properties'][$name]['required'] = in_array($name, $model['required']); | |
| if (isset($def['items']['$ref'])) { | |
| //nested model | |
| $sch['properties'][$name]['sub_schema'] = str_replace('#/definitions/', '', $def['items']['$ref']); | |
| $model['properties'][$name]['sub_schema'] = str_replace('#/definitions/', '', $def['items']['$ref']); | |
| } | |
| if (isset($def['items']['x-anyOf'])) { | |
| //nested model | |
| $sch['properties'][$name]['sub_schemas'] = \array_map(fn($schema) => str_replace('#/definitions/', '', $schema['$ref']), $def['items']['x-anyOf']); | |
| $model['properties'][$name]['sub_schemas'] = \array_map(fn($schema) => str_replace('#/definitions/', '', $schema['$ref']), $def['items']['x-anyOf']); | |
| } | |
| if (isset($def['items']['x-oneOf'])) { | |
| //nested model | |
| $sch['properties'][$name]['sub_schemas'] = \array_map(fn($schema) => str_replace('#/definitions/', '', $schema['$ref']), $def['items']['x-oneOf']); | |
| $model['properties'][$name]['sub_schemas'] = \array_map(fn($schema) => str_replace('#/definitions/', '', $schema['$ref']), $def['items']['x-oneOf']); | |
| } | |
| if (isset($def['enum'])) { | |
| // enum property | |
| $sch['properties'][$name]['enum'] = $def['enum']; | |
| $sch['properties'][$name]['enumName'] = $def['x-enum-name'] ?? ucfirst($key) . ucfirst($name); | |
| $sch['properties'][$name]['enumKeys'] = $def['x-enum-keys'] ?? []; | |
| $model['properties'][$name]['enum'] = $def['enum']; | |
| $model['properties'][$name]['enumName'] = $def['x-enum-name'] ?? ucfirst($key) . ucfirst($name); | |
| $model['properties'][$name]['enumKeys'] = $def['x-enum-keys'] ?? []; | |
| } | |
| } | |
| } | |
| $list[$key] = $model; | |
| } | |
| return $list; | |
| } | |
| if ($schema['x-request-model'] ?? false) { | |
| continue; | |
| } | |
| $model = [ | |
| 'name' => $key, | |
| 'properties' => $schema['properties'] ?? [], | |
| 'description' => $schema['description'] ?? '', | |
| 'required' => $schema['required'] ?? [], | |
| 'additionalProperties' => $schema['additionalProperties'] ?? [] | |
| ]; | |
| if (isset($model['properties'])) { | |
| foreach ($model['properties'] as $name => $def) { | |
| $model['properties'][$name]['name'] = $name; | |
| $model['properties'][$name]['description'] = $def['description']; | |
| $model['properties'][$name]['example'] = $def['x-example']; | |
| $model['properties'][$name]['required'] = in_array($name, $model['required']); | |
| if (isset($def['items']['$ref'])) { | |
| //nested model | |
| $model['properties'][$name]['sub_schema'] = str_replace('#/definitions/', '', $def['items']['$ref']); | |
| } | |
| if (isset($def['items']['x-anyOf'])) { | |
| //nested model | |
| $model['properties'][$name]['sub_schemas'] = \array_map(fn($schema) => str_replace('#/definitions/', '', $schema['$ref']), $def['items']['x-anyOf']); | |
| } | |
| if (isset($def['items']['x-oneOf'])) { | |
| //nested model | |
| $model['properties'][$name]['sub_schemas'] = \array_map(fn($schema) => str_replace('#/definitions/', '', $schema['$ref']), $def['items']['x-oneOf']); | |
| } | |
| if (isset($def['enum'])) { | |
| // enum property | |
| $model['properties'][$name]['enum'] = $def['enum']; | |
| $model['properties'][$name]['enumName'] = $def['x-enum-name'] ?? ucfirst($key) . ucfirst($name); | |
| $model['properties'][$name]['enumKeys'] = $def['x-enum-keys'] ?? []; | |
| } | |
| } | |
| } | |
| $list[$key] = $model; | |
| } | |
| return $list; | |
| } |
🤖 Prompt for AI Agents
In src/Spec/Swagger2.php around lines 467 to 509, the model's 'description'
default is set to an empty array which mismatches the expected string type;
change the default to an empty string (''), and likewise ensure individual
property descriptions use a safe string default when assigning
$model['properties'][$name]['description'] (e.g. use $def['description'] ?? '')
so description fields are always strings.
| if (isset($model['properties'])) { | ||
| foreach ($model['properties'] as $name => $def) { | ||
| $model['properties'][$name]['name'] = $name; | ||
| $model['properties'][$name]['description'] = $def['description']; | ||
| $model['properties'][$name]['example'] = $def['x-example']; | ||
| $model['properties'][$name]['required'] = in_array($name, $model['required']); |
There was a problem hiding this comment.
Potential undefined index access on $def['description'] and $def['x-example'].
In getDefinitions(), lines 480-481 access $def['description'] and $def['x-example'] directly without null coalescing, which could cause notices if these keys are missing. The getRequestModels() method (lines 534-535) correctly uses ?? '' and ?? null.
- $model['properties'][$name]['description'] = $def['description'];
- $model['properties'][$name]['example'] = $def['x-example'];
+ $model['properties'][$name]['description'] = $def['description'] ?? '';
+ $model['properties'][$name]['example'] = $def['x-example'] ?? null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (isset($model['properties'])) { | |
| foreach ($model['properties'] as $name => $def) { | |
| $model['properties'][$name]['name'] = $name; | |
| $model['properties'][$name]['description'] = $def['description']; | |
| $model['properties'][$name]['example'] = $def['x-example']; | |
| $model['properties'][$name]['required'] = in_array($name, $model['required']); | |
| if (isset($model['properties'])) { | |
| foreach ($model['properties'] as $name => $def) { | |
| $model['properties'][$name]['name'] = $name; | |
| $model['properties'][$name]['description'] = $def['description'] ?? ''; | |
| $model['properties'][$name]['example'] = $def['x-example'] ?? null; | |
| $model['properties'][$name]['required'] = in_array($name, $model['required']); |
🤖 Prompt for AI Agents
In src/Spec/Swagger2.php around lines 477-482, the code accesses
$def['description'] and $def['x-example'] directly which can raise undefined
index notices; change those accesses to use null coalescing (e.g.
$def['description'] ?? '' and $def['x-example'] ?? null) and also guard the
required check by ensuring $model['required'] exists before in_array (e.g. use
isset($model['required']) && in_array(...)) so missing keys get safe defaults
like in getRequestModels().
| public Dictionary<string, object?> ToMap() | ||
| { | ||
| return new Dictionary<string, object?> | ||
| { | ||
| {%~ for property in requestModel.properties %} | ||
| { "{{ property.name }}", {% if property.sub_schema %}{% if property.type == 'array' %}{{ property.name | caseUcfirst | overrideIdentifier }}?.ConvertAll(p => p.ToMap()){% else %}{{ property.name | caseUcfirst | overrideIdentifier }}?.ToMap(){% endif %}{% elseif property.enum %}{{ property.name | caseUcfirst | overrideIdentifier }}{% if not property.required %}?{% endif %}.Value{% else %}{{ property.name | caseUcfirst | overrideIdentifier }}{% endif %} }{% if not loop.last %},{% endif %} | ||
|
|
||
| {%~ endfor %} | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the repository structure
git ls-files | grep -E "(filter|twig|dotnet)" | head -20Repository: appwrite/sdk-generator
Length of output: 1309
🏁 Script executed:
# Search for propertyType filter definition
rg -n "propertyType" --type-add 'php:*.php' --type php | head -30Repository: appwrite/sdk-generator
Length of output: 501
🏁 Script executed:
# Look for ConvertAll usages in templates
rg -rn "ConvertAll" --type-add 'twig:*.twig' --type twigRepository: appwrite/sdk-generator
Length of output: 617
🏁 Script executed:
# Search for how arrays are defined in the dotnet templates
rg -rn "List<" templates/dotnet/ --type-add 'twig:*.twig' --type twig | head -20Repository: appwrite/sdk-generator
Length of output: 2601
🏁 Script executed:
# Check for any propertyType usage in dotnet templates
rg -rn "propertyType" templates/dotnet/ --type-add 'twig:*.twig' --type twig | head -30Repository: appwrite/sdk-generator
Length of output: 548
🏁 Script executed:
# Find the DotNet language class definition
find . -name "*.php" | xargs grep -l "class.*Dotnet\|class.*DotNet\|class.*NetSDK" | head -10Repository: appwrite/sdk-generator
Length of output: 193
🏁 Script executed:
# Search for propertyType filter in DotNet language
rg -n "propertyType" src/SDK/Language/*.php | grep -i dotnetRepository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Get the full DotNet language class file
find . -path "*/Language/Dotnet.php" -o -path "*/Language/DotNet.php" -o -path "*/Language/Csharp.php"Repository: appwrite/sdk-generator
Length of output: 95
🏁 Script executed:
# Look at RequestModel.cs.twig content more carefully
cat -n templates/dotnet/Package/Models/RequestModel.cs.twig | head -50Repository: appwrite/sdk-generator
Length of output: 2613
🏁 Script executed:
# Search for how the "n" filter is defined in DotNet context
rg -B5 -A10 "new TwigFilter.*'n'" src/SDK/Language/ | grep -A10 -B5 "Dotnet\|DotNet\|Csharp"Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Read the DotNet.php language class to find propertyType filter
cat -n src/SDK/Language/DotNet.php | head -100Repository: appwrite/sdk-generator
Length of output: 2745
🏁 Script executed:
# Search for propertyType filter definition in DotNet.php
rg -n "propertyType" src/SDK/Language/DotNet.php -A 20Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Get full length of DotNet.php
wc -l src/SDK/Language/DotNet.phpRepository: appwrite/sdk-generator
Length of output: 97
🏁 Script executed:
# Search for propertyType in the entire file
rg -n "propertyType" src/SDK/Language/DotNet.phpRepository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Look for filter definitions in DotNet.php
rg -n "new TwigFilter" src/SDK/Language/DotNet.php | head -20Repository: appwrite/sdk-generator
Length of output: 302
🏁 Script executed:
# Check the parent Language class for propertyType filter
rg -n "propertyType" src/SDK/Language.php -A 20Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Also check if DotNet extends and overrides from Language
rg -n "class DotNet extends" src/SDK/Language/DotNet.phpRepository: appwrite/sdk-generator
Length of output: 97
🏁 Script executed:
# Broader search for propertyType in all PHP files
rg -n "propertyType" src/ --type phpRepository: appwrite/sdk-generator
Length of output: 501
🏁 Script executed:
# Search for how filters are registered/added in general
rg -n "TwigFilter.*property" src/SDK/Language/ --type php -A 5Repository: appwrite/sdk-generator
Length of output: 4295
🏁 Script executed:
# Check if propertyType exists anywhere in Language.php
wc -l src/SDK/Language.phpRepository: appwrite/sdk-generator
Length of output: 90
🏁 Script executed:
# Read Language.php to look for propertyType
cat src/SDK/Language.php | grep -n "propertyType"Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Check test files to see if they work
rg -n "propertyType" tests/ --type phpRepository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Look at DotNet-specific tests
cat -n tests/DotNet60Test.php | head -50Repository: appwrite/sdk-generator
Length of output: 1615
🏁 Script executed:
# Check how filters are registered - look for getFilters or addFilters
rg -n "getFilters|addFilters|registerFilters" src/SDK/Language.php -A 5Repository: appwrite/sdk-generator
Length of output: 169
🏁 Script executed:
# Look at full filter registration in DotNet.php
rg -n "return \[" src/SDK/Language/DotNet.php -B 5 -A 30 | head -100Repository: appwrite/sdk-generator
Length of output: 2873
🏁 Script executed:
# Check if there's a getPropertyType method in DotNet
rg -n "getPropertyType" src/SDK/Language/DotNet.phpRepository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Get the full getTypeName implementation in DotNet.php
rg -n "getTypeName" src/SDK/Language/DotNet.php -A 30Repository: appwrite/sdk-generator
Length of output: 5289
🏁 Script executed:
# Check if DotNet has a getFilters override
sed -n '480,520p' src/SDK/Language/DotNet.phpRepository: appwrite/sdk-generator
Length of output: 1628
🏁 Script executed:
# Look for all TwigFilter additions in DotNet.php
rg -n "new TwigFilter" src/SDK/Language/DotNet.phpRepository: appwrite/sdk-generator
Length of output: 302
🏁 Script executed:
# Get the full property_type function from DotNet.php
sed -n '520,560p' src/SDK/Language/DotNet.phpRepository: appwrite/sdk-generator
Length of output: 1700
🏁 Script executed:
# Check how RequestModel.cs.twig should use property_type
cat -n templates/dotnet/Package/Models/RequestModel.cs.twig | sed -n '15,30p'Repository: appwrite/sdk-generator
Length of output: 1075
🏁 Script executed:
# Check other DotNet templates to see if they use propertyType or property_type
rg -n "propertyType\|property_type" templates/dotnet/ --type-add 'twig:*.twig' --type twig | head -20Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Check the sub_schema function more carefully - does it handle sub_schema properly?
sed -n '520,545p' src/SDK/Language/DotNet.phpRepository: appwrite/sdk-generator
Length of output: 1234
🏁 Script executed:
# Check if propertyType is supposed to be a generic filter in Language.php base class
grep -n "propertyType\|'property'" src/SDK/Language.phpRepository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Look at other language implementations to see if they have propertyType filter
rg -n "getFilters.*function" src/SDK/Language/ -A 50 | head -100Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Verify that propertyType filter really doesn't exist anywhere
rg -rn "propertyType" src/SDK/Language/ --type phpRepository: appwrite/sdk-generator
Length of output: 456
🏁 Script executed:
# Check when this template was added or modified - look at git log
git log --oneline -20 -- templates/dotnet/Package/Models/RequestModel.cs.twigRepository: appwrite/sdk-generator
Length of output: 96
🏁 Script executed:
# Check if propertyType and 'n' are the same or different in other languages
rg -n "TwigFilter\('propertyType'|'n'" src/SDK/Language/Swift.php -A 3Repository: appwrite/sdk-generator
Length of output: 370
🏁 Script executed:
# Check the full git commit to understand what was added
git show --stat 55106f4c | head -40Repository: appwrite/sdk-generator
Length of output: 2343
🏁 Script executed:
# Check if DotNet has 'n' filter
rg -n "TwigFilter\('n'" src/SDK/Language/DotNet.phpRepository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Check what getPropertyType does in Swift - this is what DotNet needs to implement
rg -n "getPropertyType" src/SDK/Language/Swift.php -A 30 | head -50Repository: appwrite/sdk-generator
Length of output: 2096
🏁 Script executed:
# Verify DotNet doesn't have getPropertyType method
rg -n "getPropertyType" src/SDK/Language/DotNet.phpRepository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Double-check: is propertyType filter used in any DotNet template?
rg -rn "propertyType" templates/dotnet/ --type-add 'twig:*.twig' --type twigRepository: appwrite/sdk-generator
Length of output: 548
🏁 Script executed:
# Look at the actual git diff for the commit to see what changed
git show 55106f4c -- templates/dotnet/Package/Models/RequestModel.cs.twig | head -100Repository: appwrite/sdk-generator
Length of output: 2726
🏁 Script executed:
# Check if 'n' filter exists in DotNet now
rg -n "TwigFilter\('n'" src/SDK/Language/DotNet.php -A 5Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Check what languages have the 'n' filter
rg -rn "TwigFilter\('n'" src/SDK/Language/ -A 3Repository: appwrite/sdk-generator
Length of output: 48
Add the missing propertyType filter to the DotNet language class.
The DotNet language implementation does not define a propertyType filter, but the RequestModel.cs.twig template uses it on lines 18 and 24. This will cause a Twig compilation error. Other languages (Swift, Kotlin, Go) implement propertyType by delegating to a getPropertyType() method. For DotNet, implement this filter following the same pattern and ensure it returns List<T> (not T[]) for array properties, so that ConvertAll() on line 39 works correctly.
| export function {{ requestModel.name | caseCamel }}ToMap(model: {{ requestModel.name | caseUcfirst }}): Record<string, unknown> { | ||
| return { | ||
| {%~ for property in requestModel.properties %} | ||
| '{{ property.name }}': {% if property.sub_schema %}{% if property.type == 'array' %}model.{{ property.name | escapeKeyword }}{% if not property.required %}?{% endif %}.map(item => {{ property.sub_schema | caseCamel }}ToMap(item)){% else %}model.{{ property.name | escapeKeyword }} ? {{ property.sub_schema | caseCamel }}ToMap(model.{{ property.name | escapeKeyword }}) : undefined{% endif %}{% elseif property.enum %}model.{{ property.name | escapeKeyword }}{% else %}model.{{ property.name | escapeKeyword }}{% endif %}, | ||
| {%~ endfor %} | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the full request_model.ts.twig template file
cat -n templates/node/src/models/request_model.ts.twigRepository: appwrite/sdk-generator
Length of output: 1659
🏁 Script executed:
# Search for sub_schema usage in Node templates
rg -n 'sub_schema' templates/node/ -A 2 -B 2Repository: appwrite/sdk-generator
Length of output: 947
🏁 Script executed:
# Check if sub_schema is used in request model properties across the codebase
rg -n 'sub_schema' templates/ -l | grep -E '(request|model)' | head -20Repository: appwrite/sdk-generator
Length of output: 753
🏁 Script executed:
# Search for how other languages/templates handle sub_schema imports
rg -n 'sub_schema' templates/ -A 1 -B 1 | head -100Repository: appwrite/sdk-generator
Length of output: 16290
🏁 Script executed:
# Check how other templates handle sub_schema imports for request models
# Let's look at Ruby request model
head -50 templates/ruby/lib/container/models/request_model.rb.twigRepository: appwrite/sdk-generator
Length of output: 2316
🏁 Script executed:
# Check Swift request model for import handling
head -50 templates/swift/Sources/Models/RequestModel.swift.twigRepository: appwrite/sdk-generator
Length of output: 2259
🏁 Script executed:
# Search for examples or specs that show sub_schema in request models
find . -name "*.json" -o -name "*.yaml" -o -name "*.yml" | xargs grep -l "requestModel" 2>/dev/null | head -5Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Look at the general model template to see how it handles sub_schema imports
head -80 templates/node/src/models/model.ts.twigRepository: appwrite/sdk-generator
Length of output: 164
🏁 Script executed:
# Search for test files or examples showing request models with sub_schema
find . -path ./node_modules -prune -o -type f \( -name "*.test.ts" -o -name "*.spec.ts" -o -name "*_test.ts" \) -print | head -10Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Look for any configuration or spec files that define request models
find . -name "*.json" -path "*/spec*" -o -name "*.yaml" -path "*/spec*" | head -10Repository: appwrite/sdk-generator
Length of output: 93
🏁 Script executed:
# Search for how requestModel properties are defined in the codebase
rg -l "requestModel\.properties" --type ts --type js | head -5Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Check if sub_schema is ever set in requestModel context
rg "requestModel" templates/ -A 5 -B 5 | grep -E "(sub_schema|requestModel)" | head -30Repository: appwrite/sdk-generator
Length of output: 4780
🏁 Script executed:
# Check if any request model properties in spec.json have sub_schema
python3 << 'EOF'
import json
with open('./tests/resources/spec.json', 'r') as f:
spec = json.load(f)
# Look for requestModels and their properties with sub_schema
if 'requestModels' in spec:
for model_name, model in spec['requestModels'].items():
if 'properties' in model:
for prop_name, prop in model['properties'].items():
if 'x-schema' in prop or 'sub_schema' in prop:
print(f"Found sub_schema in {model_name}.{prop_name}: {prop}")
elif prop.get('type') == 'object' and 'x-schema' not in prop:
print(f"Found object type in {model_name}.{prop_name}: {prop}")
else:
print("No requestModels found in spec")
print("Available keys:", list(spec.keys())[:20])
EOFRepository: appwrite/sdk-generator
Length of output: 152
🏁 Script executed:
# Also check the structure of the spec file
jq 'keys' tests/resources/spec.jsonRepository: appwrite/sdk-generator
Length of output: 237
🏁 Script executed:
# Search the spec more efficiently for sub_schema
jq '.definitions | to_entries[] | select(.value.properties != null) | .key as $model | .value.properties | to_entries[] | select(.value | has("x-schema") or .value.type == "object") | {model: $model, property: .key, value: .value}' tests/resources/spec.json 2>/dev/null | head -100Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# More direct approach: search for x-schema in the entire spec
grep -o '"x-schema":"[^"]*"' tests/resources/spec.json | head -20Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Check a specific definition structure to understand the pattern
jq '.definitions | keys | .[0:5]' tests/resources/spec.jsonRepository: appwrite/sdk-generator
Length of output: 110
🏁 Script executed:
# Let's examine the actual definitions structure
jq '.definitions.player | .properties | keys' tests/resources/spec.json 2>/dev/nullRepository: appwrite/sdk-generator
Length of output: 97
🏁 Script executed:
# Check the generator code to understand when sub_schema is set
rg -n "sub_schema" --type ts --type js --type py -B 3 -A 3 | head -80Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Look for where properties get their sub_schema field
rg -n "\.sub_schema\s*=" --type ts --type js --type py | head -20Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Broad search for sub_schema across entire codebase
rg -n "sub_schema" . --type-list | head -20Repository: appwrite/sdk-generator
Length of output: 455
🏁 Script executed:
# Check if sub_schema is defined in any filter or function
rg -n "sub_schema" . -A 2 -B 2 | grep -E "(filter|function|def|const|let|var)" | head -30Repository: appwrite/sdk-generator
Length of output: 6540
🏁 Script executed:
# Look for where property objects are constructed/enriched
rg -n "property\[" . --type ts --type js | head -20Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Search for where properties get enriched with sub_schema
rg -n "properties\[" . --type ts --type js -A 5 | head -40Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Check if there's any code that sets or processes sub_schema
find . -name "*.ts" -o -name "*.js" | xargs grep -l "sub_schema" | grep -v node_modules | grep -v templatesRepository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Look for getPropertyType or similar filters that might set sub_schema
rg -n "getPropertyType|propertyType" . --type ts --type js -A 5 | head -50Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Search for any documentation or comments about sub_schema
rg -n "sub_schema" . -B 2 -A 2 | grep -E "(#|//|comment|doc|TODO|FIXME)" | head -20Repository: appwrite/sdk-generator
Length of output: 2568
🏁 Script executed:
# Check if there are any request models in the spec with nested/object properties
jq '.definitions | to_entries[] | select(.value.properties) | {model: .key, properties: (.value.properties | keys)}' tests/resources/spec.jsonRepository: appwrite/sdk-generator
Length of output: 317
🏁 Script executed:
# Look at the entire player definition to understand structure
jq '.definitions.player' tests/resources/spec.jsonRepository: appwrite/sdk-generator
Length of output: 548
🏁 Script executed:
# Look at the full context where sub_schema is set
sed -n '480,550p' src/Spec/Swagger2.phpRepository: appwrite/sdk-generator
Length of output: 3662
🏁 Script executed:
# Check if request models can have array properties with $ref
rg -n "requestModel|x-request-model" tests/resources/spec.json -B 5 -A 10 | head -50Repository: appwrite/sdk-generator
Length of output: 534
🏁 Script executed:
# Search for any models with nested array references to understand the pattern
jq '.definitions | to_entries[] | select(.value.properties[] | .items | select(. != null) | select(has("$ref") or has("x-anyOf") or has("x-oneOf"))) | .key' tests/resources/spec.jsonRepository: appwrite/sdk-generator
Length of output: 144
🏁 Script executed:
# Check how Ruby/PHP/Swift handle the import for sub_schema in request models
head -20 templates/ruby/lib/container/models/request_model.rb.twigRepository: appwrite/sdk-generator
Length of output: 862
🏁 Script executed:
head -20 templates/php/src/Models/RequestModel.php.twigRepository: appwrite/sdk-generator
Length of output: 555
🏁 Script executed:
# Check if there's any template that DOES import sub_schema for request models
rg -n "import.*sub_schema" templates/Repository: appwrite/sdk-generator
Length of output: 48
Missing imports for nested request model ToMap functions.
When a property has sub_schema, the generated code calls {{ property.sub_schema | caseCamel }}ToMap(item), but no corresponding import statement is generated for nested request models' ToMap functions.
If sub_schema refers to another request model (e.g., Player), the generated file would require an import like:
import { playerToMap } from './player';This occurs when a request model has an array property with items referencing another model definition. The import section currently only generates enum imports (lines 1-5), leaving calls to nested model ToMap functions without their required imports.
| from typing import Optional, List, Dict, Any | ||
| {%~ for property in requestModel.properties %} | ||
| {%~ if property.enum %} | ||
| from ..enums.{{ property.enumName | caseSnake }} import {{ property.enumName | caseUcfirst }} | ||
| {%~ endif %} | ||
| {%~ endfor %} | ||
|
|
There was a problem hiding this comment.
Fix pipeline failure: extra blank lines after import loop.
The pipeline reports a formatting issue (H014) at line 6. The template has extra blank lines between the enum import block and the class definition.
Consider adjusting the template to reduce blank lines:
from typing import Optional, List, Dict, Any
{%~ for property in requestModel.properties %}
{%~ if property.enum %}
from ..enums.{{ property.enumName | caseSnake }} import {{ property.enumName | caseUcfirst }}
{%~ endif %}
{%~ endfor %}
-
-
+
class {{ requestModel.name | caseUcfirst }}:Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Twig Linting
[error] 6-6: H014 6:3 Found extra blank lines. endfor %}. djlint lint reported a formatting issue.
🤖 Prompt for AI Agents
In templates/python/package/models/request_model.py.twig lines 1-7 there are
extra blank lines emitted after the enum import loop which cause H014; update
the template so the loop does not produce an empty newline when no enums are
present and ensure only a single newline separates the imports from the class
definition (remove the extra blank line(s) emitted by the loop and adjust the
template's whitespace/newline control so it always outputs exactly one blank
line before the class).
| @@ -0,0 +1,66 @@ | |||
| #frozen_string_literal: true | |||
There was a problem hiding this comment.
Fix the frozen_string_literal magic comment syntax.
The magic comment is missing a space after #. Ruby requires # frozen_string_literal: true (with space) for it to be recognized.
-#frozen_string_literal: true
+# frozen_string_literal: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #frozen_string_literal: true | |
| # frozen_string_literal: true |
🤖 Prompt for AI Agents
In templates/ruby/lib/container/models/request_model.rb.twig around lines 1 to
1, the frozen_string_literal magic comment is written as
"#frozen_string_literal: true" which isn't recognized by Ruby; update it to
include a space after the hash so it reads "# frozen_string_literal: true"
(i.e., insert a single space between '#' and 'frozen_string_literal') to fix the
syntax.
| unless valid_{{ property.name | caseSnake }}.include?({{ property.name | caseSnake | escapeKeyword }}) | ||
| raise ArgumentError, "Invalid " + {{ property.name | caseSnake | escapeKeyword }} + ". Must be one of: " + valid_{{ property.name | caseSnake }}.join(', ') | ||
| end |
There was a problem hiding this comment.
TypeError risk when concatenating non-string enum value.
If {{ property.name | caseSnake | escapeKeyword }} is a symbol or non-string type, string concatenation with + will raise a TypeError. Use string interpolation instead.
- unless valid_{{ property.name | caseSnake }}.include?({{ property.name | caseSnake | escapeKeyword }})
- raise ArgumentError, "Invalid " + {{ property.name | caseSnake | escapeKeyword }} + ". Must be one of: " + valid_{{ property.name | caseSnake }}.join(', ')
- end
+ unless valid_{{ property.name | caseSnake }}.include?({{ property.name | caseSnake | escapeKeyword }})
+ raise ArgumentError, "Invalid #{{{ property.name | caseSnake | escapeKeyword }}}. Must be one of: #{valid_{{ property.name | caseSnake }}.join(', ')}"
+ end🤖 Prompt for AI Agents
In templates/ruby/lib/container/models/request_model.rb.twig around lines 55 to
57, the code concatenates a possibly non-string enum value using +, which can
raise a TypeError for symbols or other non-String values; change the raise to
use string interpolation or call .to_s on the variable (e.g. raise
ArgumentError, "Invalid #{...}. Must be one of: #{valid_....join(', ')}") so the
enum value is safely converted to a string and the message remains clear.
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.