added presence api update#1489
Conversation
Greptile SummaryThis PR adds a Presence API to the Appwrite SDK generator, implementing
Confidence Score: 3/5Multiple template files still generate syntactically invalid code for $-prefixed additionalPropertiesKey values in Python, Ruby, Rust, PHP, and .NET — issues raised in earlier review rounds that remain unresolved. The core presence logic is sound and consistently implemented across platforms. However, additionalPropertiesKey changes in five language templates do not apply removeDollarSign in identifier positions, so any spec using a $-prefixed key will produce uncompilable generated code. The composer.lock also locks utopia-php/query at PHP 8.4+, stricter than every other dependency. templates/python/package/models/model.py.twig, templates/ruby/lib/container/models/model.rb.twig, templates/rust/src/models/model.rs.twig, templates/php/src/Models/Model.php.twig, templates/dotnet/Package/Models/Model.cs.twig — all generate invalid identifiers when additionalPropertiesKey starts with $. Also mock-server/composer.lock for the PHP 8.4 constraint. Important Files Changed
Reviews (27): Last reviewed commit: "added subscription lock in the realtime ..." | Re-trigger Greptile |
|
|
||
| $http->set([ | ||
| 'worker_num' => $workerNumber, | ||
| $adapter = new Adapter\Swoole(host: '0.0.0.0', port: (int) App::getEnv('PORT', 80)); |
There was a problem hiding this comment.
using http and websocket server in a single file. Forwarding request to the http App via the onRequest
| case this.TYPE_PONG: | ||
| // Handle pong response if needed | ||
| break; | ||
| case this.TYPE_RESPONSE: |
There was a problem hiding this comment.
removed this cause the handleResponseAction was doing nothing. Not needed as on type response we dont store any internal state
|
@greptile |
|
That makes sense — if the handler was always a no-op from day one and Tip: You can give me more context with .greptile/rules.md so I get this right next time. |
|
@greptile
|
|
That's a fair point. Looking at Withdrawing the concern — this is intentional and consistent with the existing approach.
|
|
@greptile
That's actually intentional — and it's what makes the 'Realtime failed!' expected output in Base::REALTIME_RESPONSES work. Walking through the contract: Subscription set up by the tests: ┌─────────────────────────┬───────────────────────────────────────────────┬─────────────────────────────────────────────────────────┐ What the mock does today (Protocol.php): $eventPayload = ['response' => 'WS:/v1/realtime:passed']; For the failure subscription, subscriptionMatchesPayload(['{"method":"equal","attribute":"response","values":["failed"]}'], ['response' => 'WS:/v1/realtime:passed']) → equal('response', That's by design:
If the filter were inverted (failure sub also got the event), here's what would break in each SDK:
So the filter-out behavior is load-bearing for the test's "failed" assertion. It looks like a missing feature but it's actually what proves query filtering works. |
|
That's a thorough and convincing walkthrough — thanks for spelling it out. You're right that the I'll withdraw the comment. The filter-out behavior is correct and the test design is sound. Tip: You can give me more context with .greptile/rules.md so I get this right next time. |
…ests across multiple languages
…ross multiple languages
|
@greptile
The core presence logic is sound and consistently implemented across platforms. However, additionalPropertiesKey changes in five language templates do not apply removeDollarSign in identifier positions, so any spec using a $-prefixed key will produce uncompilable generated code. The composer.lock also locks utopia-php/query at PHP 8.4+, stricter than every other dependency. templates/python/package/models/model.py.twig, templates/ruby/lib/container/models/model.rb.twig, templates/rust/src/models/model.rs.twig, templates/php/src/Models/Model.php.twig, templates/dotnet/Package/Models/Model.cs.twig — all generate invalid identifiers when additionalPropertiesKey starts with $
|
|
After carefully reading all five templates, the developer is correct. Let me verify each file: Python: All Ruby: Rust: All four usages (struct field, PHP: All four positions (docblock .NET: Property declaration uses You're right — the review concern was incorrect. The Tip: You can give me more context with .greptile/rules.md so I get this right next time. |
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.)