Repository navigation
fix: examples, model defaults and topic NUL bytes from SDK review - #2050
Conversation
PHP printed JSON objects inside document lists and left apostrophes unescaped; Ruby used null for empty optional numbers and unescaped quotes in strings; Dart and Flutter typed text responses as models. and used enum members that are keywords without their escaped name.
Python rendered a map field's {} default as [], failing pydantic validation; Dart parsed a missing required nested model such as prefs from null.
MQTT topic filters cannot contain U+0000. Also adds e2e coverage for the model defaults above.
🔵 Tier A · Mergeable after minor fixes
This change updates generated Dart, Python, and Ruby examples and model handling, adds NUL validation to Topic helpers, and adds mock endpoints and e2e cases for map and nested-model defaults. It also updates generated Dart/Flutter docs and the generation/e2e fixtures for these cases.
Fix with agent prompt### Issue 1
src/SDK/Language/Ruby.php:298
**Add a syntax check for generated Ruby examples**
This changes how Ruby example strings are escaped, but the generation suite has no Ruby equivalent of `testPhpExamplesParse()` to parse the emitted docs. A later quoting regression could reintroduce invalid examples without failing CI; adding a Ruby parse check for generated examples would keep this fix covered.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.📂 Walkthrough · 8
Reviewed |
The longer message pushed the throw past prettier's print width in web, node and react-native topic.ts.
…eview-findings # Conflicts: # src/SDK/Language/PHP.php # templates/android/library/src/main/java/io/package/Topic.kt.twig # templates/kotlin/src/main/kotlin/io/appwrite/Topic.kt.twig # templates/python/package/topic.py.twig # templates/ruby/lib/container/topic.rb.twig
|
@hansi-codes review |
| break; | ||
| case self::TYPE_STRING: | ||
| $output .= "'{$example}'"; | ||
| $output .= "'" . \addcslashes((string) $example, "'\\") . "'"; |
There was a problem hiding this comment.
Add a syntax check for generated Ruby examples
This changes how Ruby example strings are escaped, but the generation suite has no Ruby equivalent of testPhpExamplesParse() to parse the emitted docs. A later quoting regression could reintroduce invalid examples without failing CI; adding a Ruby parse check for generated examples would keep this fix covered.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/SDK/Language/Ruby.php
Line: 298
Comment:
**Add a syntax check for generated Ruby examples**
This changes how Ruby example strings are escaped, but the generation suite has no Ruby equivalent of `testPhpExamplesParse()` to parse the emitted docs. A later quoting regression could reintroduce invalid examples without failing CI; adding a Ruby parse check for generated examples would keep this fix covered.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · testing · Reply if this doesn't apply.
Fixes review findings on the SDK major PRs at their source.
php -lfails)php -lbefore, 0 afternull; GraphQL example has unescaped quotesnil; strings are single-quoted and escaped. 5nullliterals and 1 parse error before, 0 after (ruby -con all 800)getZoneexample typedmodels. result;DomainRegistrationType.newdoesn't existString; enum examples use the escaped member name (xnew). Same fix in the Flutter templateBackupRestoration.optionsdefaults to[](pydantic rejects it){}default renders as{}Preferences.fromMap(null)throws whenprefsis missingTopic.pathaccepts NUL bytese2e: new
preset/settingsandpreset/profilemock endpoints return{}. Python and Dart/Flutter printpreset settings: options=0andpreset profile: prefs=0, and every Topic program gains anulcase.Watched red then green: Python313 (
dict_typeerror), DartStable (type 'Null' is not a subtype of type 'Map<String, dynamic>') and Ruby31 (Topic nul:failed). Node20, PHP85 and Go118 also pass. GenerationTest passes, and the TSapplyDefaultscount goes from 5 to 6 for the new endpoint.Closes #2052