Skip to content

added presence api update#1489

Open
ArnabChatterjee20k wants to merge 38 commits into
masterfrom
presence-api
Open

added presence api update#1489
ArnabChatterjee20k wants to merge 38 commits into
masterfrom
presence-api

Conversation

@ArnabChatterjee20k
Copy link
Copy Markdown
Member

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.)

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 29, 2026

Greptile Summary

This PR adds a Presence API to the Appwrite SDK generator, implementing upsertPresence() across Flutter, Android, Apple, Web, and ReactNative templates using a fire-and-forget design that stores the latest presence payload for automatic replay on reconnect. It also generalises model templates to support a configurable additionalPropertiesKey (driven by x-additional-properties-key in the OpenAPI spec), and migrates the mock server's realtime handling from raw Swoole to a Utopia\\WebSocket\\Server wrapper with a new Protocol class.

  • The fire-and-forget presence design stores the latest payload in pendingPresence (single-value, not a queue), clears it on disconnect(), and re-sends it automatically on each reconnect's connected event.
  • The mock server correctly fans out presence events to all matching subscribers, enabling end-to-end round-trip tests across all four platforms.
  • additionalPropertiesKey template changes correctly apply removeDollarSign in identifier positions for Kotlin, Swift, and Android, though several other language templates (Python, Ruby, Rust, PHP, .NET) have outstanding $-prefix identifier-safety issues noted in prior review rounds.

Confidence Score: 3/5

Multiple 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

Filename Overview
templates/web/src/services/realtime.ts.twig Adds fire-and-forget upsertPresence() that stores the latest presence payload for automatic replay on reconnect; pendingPresence is cleared on disconnect() and on every explicit call, consistent with documented behavior.
templates/flutter/lib/src/realtime_mixin.dart.twig Adds upsertPresenceTo() with single-flight socket guard; _pendingPresence stored for replay on reconnect and cleared in disconnect(). No continuation queue — prior thread concerns about pendingPresenceRequests do not apply to this fire-and-forget design.
templates/android/library/src/main/java/io/package/services/Realtime.kt.twig Adds upsertPresence() with presenceLock/subscriptionLock synchronisation; companion-object pendingPresence cleared in disconnect(). onFailure still lacks reconnect scheduling (pre-existing gap, not introduced here).
templates/apple/Sources/Services/Realtime.swift.twig Adds upsertPresence() (synchronous throws) with presenceSync queue for thread-safe pendingPresence access; disconnect() cancels the in-flight socketCreationTask before tearing down.
mock-server/src/Utopia/Realtime/Protocol.php New Realtime WebSocket protocol handler using Utopia\WebSocket\Server correctly; handles subscribe/unsubscribe/presence/ping and fans out events. Previously flagged type-mismatch is fixed by using the Utopia wrapper throughout.
mock-server/app/http.php Migrates from raw Swoole server to Utopia\WebSocket\Server adapter; WebSocket handlers now delegate to RealtimeProtocol, resolving the previously flagged type-mismatch issue.
src/Spec/Swagger2.php Reads x-additional-properties-key from the OpenAPI spec and falls back to 'data'; feeds additionalPropertiesKey to all model templates.
templates/dart/lib/src/models/model.dart.twig Replaces hardcoded data with additionalPropertiesKey; uses escapeKeyword (not removeDollarSign) in identifier positions — correct since Dart allows $-prefixed identifiers natively.
mock-server/composer.lock Locks utopia-php/query at 0.3.1 which declares php: >=8.4 — stricter than all other utopia-php/* packages in the file; may break composer install on PHP 8.0-8.3 CI environments (previously flagged, not yet resolved).
templates/android/library/src/main/java/io/package/Channel.kt.twig Adds _Presence interface and Channel.presence()/presences() statics with extension functions for upsert/update/delete/create — note create() is present here but absent in Web/ReactNative templates and untested.
templates/flutter/lib/src/realtime.dart.twig Adds upsertPresence() abstract method signature; docstring inaccurately states it throws synchronously on no connection — the mixin silently queues for replay (previously flagged).
tests/languages/flutter/tests.dart Attaches broadcast listeners before the 5-second delay to avoid missing buffered events; calls fire-and-forget upsertPresence() with a 1-second fan-out wait. Design correctly avoids the previous _websok == null race.
tests/languages/android/Tests.kt Adds presence subscription and round-trip test via upsertPresence + 1-second delay; switches realtime endpoint to ws://mockapi/v1.
tests/languages/apple/Tests.swift Adds presence subscription and upsertPresence round-trip test; migrates from deprecated wait(for:timeout:) to await fulfillment(of:timeout:).
mock-server/src/Utopia/Realtime/Connection.php New per-connection state container; provides matchingSubscriptions() for event fan-out routing.
tests/Base.php Adds 'Realtime presence:passed' to expected output and presence channel strings to channel helper test expectations.

Reviews (27): Last reviewed commit: "added subscription lock in the realtime ..." | Re-trigger Greptile

Comment thread templates/flutter/lib/src/realtime_mixin.dart.twig
Comment thread templates/swift/Sources/Models/Model.swift.twig Outdated
Comment thread mock-server/src/Utopia/Realtime/Protocol.php
Comment thread mock-server/app/http.php Outdated

$http->set([
'worker_num' => $workerNumber,
$adapter = new Adapter\Swoole(host: '0.0.0.0', port: (int) App::getEnv('PORT', 80));
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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:
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

removed this cause the handleResponseAction was doing nothing. Not needed as on type response we dont store any internal state

@ArnabChatterjee20k
Copy link
Copy Markdown
Member Author

@greptile The mock server's handlePresence sends a type: 'response' acknowledgment frame that every SDK client now silently discards — the TYPE_RESPONSE case was removed from all switch statements in this same PR
They are not out of sync. rather the mock server is trying to replicate the original appwrite realtime server as much as possible. for the response type message, the sdks doesn't perform anything -> no change in internal state. so nothing to perform here. From the beginning, the handler was a function with empty body

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 11, 2026

That makes sense — if the handler was always a no-op from day one and response frames never triggered any state changes in the SDKs, then omitting the TYPE_RESPONSE case is a clean removal rather than a regression. The mock server faithfully replicates the server behavior, and the SDKs correctly ignore frames they have no action for. Thanks for the clarification!

Tip: You can give me more context with .greptile/rules.md so I get this right next time.

@ArnabChatterjee20k
Copy link
Copy Markdown
Member Author

@greptile

tests/languages/web/node.js (presence test always prints passed without executing)
nodejs a backend one, we dont support presence realtime test there

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 13, 2026

That's a fair point. Looking at node.js, the pattern of printing passed without executing is already established for all realtime/WebSocket tests — file uploads, realtime connections, etc. The presence skip follows the same convention.

Withdrawing the concern — this is intentional and consistent with the existing approach.

Tip: You can give me more context with [.greptile/rules.md](https://www.greptile.com/docs/code-review/custom-standards) so I get this right next time.

@ArnabChatterjee20k
Copy link
Copy Markdown
Member Author

@greptile

server/src/Utopia/Realtime/Protocol.php (query-filtered subscriptions never receive the test event)

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:

┌─────────────────────────┬───────────────────────────────────────────────┬─────────────────────────────────────────────────────────┐
│ Handle │ Queries │ What it's asserting │
├─────────────────────────┼───────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ rtsub │ none │ "no filter → receives the event" │
├─────────────────────────┼───────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ rtsubWithQueries │ equal('response', ['WS:/v1/realtime:passed']) │ "query that matches the payload → receives the event" │
├─────────────────────────┼───────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ rtsubWithQueriesFailure │ equal('response', ['failed']) │ "query that doesn't match → does NOT receive the event" │
└─────────────────────────┴───────────────────────────────────────────────┴─────────────────────────────────────────────────────────┘

What the mock does today (Protocol.php):

$eventPayload = ['response' => 'WS:/v1/realtime:passed'];
if (!$this->subscriptionMatchesPayload($queries, $eventPayload)) {
continue; // ← this is the "never receives" you flagged
}
$this->send($server, $connection->fd, 'event', [...]);

For the failure subscription, subscriptionMatchesPayload(['{"method":"equal","attribute":"response","values":["failed"]}'], ['response' => 'WS:/v1/realtime:passed']) → equal('response',
['failed']) → in_array('WS:/v1/realtime:passed', ['failed'], true) → false. So the continue fires and no event is sent.

That's by design:

  • The test fixtures explicitly want the failure subscription to time out — both the Flutter Completer-based read and Web/Android/Apple's pre-initialized "Realtime failed!" variable
    depend on the callback not firing.
  • Base::REALTIME_RESPONSES order is ['passed', 'passed', 'failed', ...], asserting the failure subscription prints the un-changed initial string.
  • It matches real Appwrite realtime semantics — queries filter events, so a subscription whose query doesn't match an event simply doesn't see it.

If the filter were inverted (failure sub also got the event), here's what would break in each SDK:

  • Web/Android/Apple: their callback would fire and overwrite responseRealtimeWithQueriesFailure to 'WS:/v1/realtime:passed', then the test would print that instead of 'Realtime failed!'
    → mismatch at the failure-output index.
  • Flutter: the rtsubWithQueriesFailureFirst Completer would complete with the message → the try branch's print("Realtime failed!") would still fire (both branches print the same string,
    so this one wouldn't observably regress — but the TimeoutException path would never run, which defeats the intent of the test).

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.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 13, 2026

That's a thorough and convincing walkthrough — thanks for spelling it out.

You're right that the continue in Protocol.php is doing exactly what it should: the rtsubWithQueriesFailure subscription is intentionally starved of events to prove that query filtering excludes non-matching payloads. The 'Realtime failed!' in Base::REALTIME_RESPONSES is the assertion, not a fallback, and the whole test would be vacuous if that subscription also received the event.

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.

Comment thread templates/python/package/models/model.py.twig Outdated
Comment thread templates/ruby/lib/container/models/model.rb.twig
Comment thread templates/rust/src/models/model.rs.twig
Comment thread templates/dotnet/Package/Models/Model.cs.twig Outdated
Comment thread templates/php/src/Models/Model.php.twig
Comment thread templates/flutter/lib/src/realtime.dart.twig
@ArnabChatterjee20k
Copy link
Copy Markdown
Member Author

@greptile

Multiple 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 $
removeDollarSign is already applied in every identifier position in all five templates:

  • templates/python/package/models/model.py.twig — every reference to additionalPropertiesKey used as an identifier (lines 73, 97, 100,
    101, 103, 104, 105, 109, 110, 112, 115) is piped through | caseSnake | removeDollarSign. The three usages at lines 111, 113, 115 that
    omit the filter are string keys in result['…'] for the outgoing JSON dict — they must keep the $ because that's the wire format.
  • templates/ruby/lib/container/models/model.rb.twig — every identifier usage (lines 11, 20, 35, 46, 58, 65) chains | caseSnake |
    escapeKeyword | removeDollarSign. Lines 46 and 58 use the raw form only in "…": JSON string-key positions.
  • templates/rust/src/models/model.rs.twig — all four usages (lines 29, 60, 64, 65) chain | caseSnake | removeDollarSign. There is no
    non-identifier usage.
  • templates/php/src/Models/Model.php.twig — every usage including the @param ${{…}} docblock, the constructor property, the named
    argument, and the property access (lines 55, 66, 143, 164) chains | caseCamel | removeDollarSign.
  • templates/dotnet/Package/Models/Model.cs.twig — every identifier usage (lines 20, 29, 37, 84, 97, 103) chains | caseUcfirst |
    escapeKeyword | removeDollarSign or | caseCamel | escapeKeyword | removeDollarSign. Line 97's bare form is inside a "…" string key.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 15, 2026

After carefully reading all five templates, the developer is correct. Let me verify each file:

Python: All additionalPropertiesKey identifier usages (private attr, property getter, setter, hasattr check, isinstance check) correctly chain | caseSnake | removeDollarSign. The bare {{ definition.additionalPropertiesKey | default('data') }} forms appear only as the dict string key in result['...'] assignments — exactly right for wire format.

Ruby: attr_reader, constructor param, @-variable assignment, named argument in from(), instance var in to_map, and convert_to argument all correctly chain | caseSnake | escapeKeyword | removeDollarSign. The raw form appears only in "{{ ... }}" string key positions.

Rust: All four usages (struct field, self. accessor in get(), method name, and &self. return) correctly chain | caseSnake | removeDollarSign.

PHP: All four positions (docblock $, constructor property, named argument, $this-> access) correctly chain | caseCamel | removeDollarSign.

.NET: Property declaration uses | caseUcfirst | escapeKeyword | removeDollarSign, constructor param uses | caseCamel | escapeKeyword | removeDollarSign, the assignment correctly applies both, and ConvertTo<T> uses the ucfirst form. The bare form in ToMap() and From() is only the "..." string key.

You're right — the review concern was incorrect. The removeDollarSign filter is applied in every identifier position across all five templates. The raw form is used exclusively in JSON wire-format string keys, which is the correct behavior since the actual API field names (like $id) must be preserved in serialized output.

Tip: You can give me more context with .greptile/rules.md so I get this right next time.

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.

1 participant