Skip to content

fix: examples, model defaults and topic NUL bytes from SDK review - #2050

Merged
ChiragAgg5k merged 6 commits into
mainfrom
fix/examples-models-review-findings
Oct 15, 2026
Merged

ChiragAgg5k merged 6 commits into
mainfrom
fix/examples-models-review-findings

Conversation

@ChiragAgg5k

@ChiragAgg5k ChiragAgg5k commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Fixes review findings on the SDK major PRs at their source.

Finding Fix
PHP document examples use JSON objects (php -l fails) Array examples that hold objects render as PHP arrays; list items get no keys; quotes in strings are escaped. 21 of 800 examples failed php -l before, 0 after
Ruby examples use null; GraphQL example has unescaped quotes Empty optional numbers render nil; strings are single-quoted and escaped. 5 null literals and 1 parse error before, 0 after (ruby -c on all 800)
Dart getZone example typed models. result; DomainRegistrationType.new doesn't exist Text responses are typed String; enum examples use the escaped member name (xnew). Same fix in the Flutter template
Python BackupRestoration.options defaults to [] (pydantic rejects it) A map field's {} default renders as {}
Flutter Preferences.fromMap(null) throws when prefs is missing Required nested models fall back to an empty map
Ruby Topic.path accepts NUL bytes Every language's Topic helper rejects U+0000

e2e: new preset/settings and preset/profile mock endpoints return {}. Python and Dart/Flutter print preset settings: options=0 and preset profile: prefs=0, and every Topic program gains a nul case.

Watched red then green: Python313 (dict_type error), 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 TS applyDefaults count goes from 5 to 6 for the new endpoint.

Closes #2052

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.
@hansi-codes

hansi-codes Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

🔵 Tier A · Mergeable after minor fixes

The change appears sound, with one minor gap in automated coverage for generated Ruby example syntax.

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.

Verdict New comments Fixed Still open
✅ Approved 1 0 0
Finding Where
🟡 Add a syntax check for generated Ruby examples src/SDK/Language/Ruby.php:298
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
File Change
src/SDK/Language/{Dart,Python,Ruby}.php Adjusts enum example identifiers, Python map defaults, and Ruby example literals.
templates/dart/docs/example.md.twig; templates/flutter/docs/example.md.twig Uses String for text responses in generated Dart examples.
templates/dart/lib/src/models/model.dart.twig Defaults missing required nested model maps to an empty map.
templates/{android,dart,dotnet,go,kotlin,php,python,ruby,rust,swift,web}/**/*topic* Rejects NUL characters in Topic path levels across generated SDKs.
mock-server/app/http.php; tests/resources/spec-openapi3.json Adds preset settings and profile mock endpoints and schemas.
tests/e2e/Base.php; tests/e2e/*Test.php; tests/e2e/languages/** Adds default-model and Topic NUL e2e cases.
templates/rust/tests/tests.rs Adds a Rust unit-test case for Topic NUL validation.
tests/generation/GenerationTest.php Updates the generated TypeScript defaults expectation.

Reviewed bec92d3 · Details · Comment @hansi-codes review to re-run, or mention @hansi-codes with a question.

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Tier A · See the inline comments. Summary

Comment thread src/SDK/Language/PHP.php Outdated
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
@ChiragAgg5k

Copy link
Copy Markdown
Member Author

@hansi-codes review

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Tier A · Looks good to merge. Summary

Comment thread src/SDK/Language/Ruby.php
break;
case self::TYPE_STRING:
$output .= "'{$example}'";
$output .= "'" . \addcslashes((string) $example, "'\\") . "'";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

@ChiragAgg5k
ChiragAgg5k merged commit ee4acfc into main Oct 15, 2026
61 checks passed
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.

Fix examples, model defaults and topic NUL bytes from SDK review

1 participant