Repository navigation
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (6)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughClient configuration now uses one endpoint for REST and realtime traffic. Endpoint values determine primary and fallback hosts. Builders, tests, examples, and sandbox integrations use endpoint configuration. URL construction brackets IPv6 hosts. ChangesEndpoint Routing
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant ClientOptions
participant HttpCore
participant ConnectionManager
participant Hosts
participant Defaults
ClientOptions->>HttpCore: Provide endpoint options
HttpCore->>Hosts: Construct Hosts(options)
ClientOptions->>ConnectionManager: Provide endpoint options
ConnectionManager->>Hosts: Construct Hosts(options)
Hosts->>Defaults: Resolve primary domain and fallback hosts
Defaults-->>Hosts: Return endpoint-derived hosts
Hosts-->>HttpCore: Return resolved host configuration
Hosts-->>ConnectionManager: Return resolved host configuration
Merge Risk: ⚪ Minimal · up to No merge-blocking issue is identified; the PR is ready for normal checks. Pre-merge checks |
|
There was a problem hiding this comment.
🟡 Changes recommended
Accepted IPv6 endpoints produce malformed transport URLs and cannot connect.
1 open finding
What changed in this PR
This PR replaces separate host and environment settings with one endpoint for REST and realtime clients, aligning the SDK with the new host-routing model.
Changes:
- Resolves primary and fallback hosts from
endpoint. - Migrates builders, tests, examples, and integration helpers to the new option.
- Adds upgrade and test-setup guidance.
| File | Description |
|---|---|
uts/src/test/kotlin/io/ably/pubsub/uts/integration/standard/IntegrationInfraSmokeTest.kt |
Uses the sandbox endpoint. |
uts/src/main/kotlin/io/ably/pubsub/uts/infra/integration/SandboxApp.kt |
Defines the sandbox endpoint. |
uts/src/main/kotlin/io/ably/pubsub/uts/infra/integration/proxy/ProxySession.kt |
Routes proxy clients through endpoint. |
uts/README.md |
Updates integration setup guidance. |
UPGRADING.md |
Documents endpoint migration. |
server/src/test/java/io/ably/pubsub/server/PubSubServerTest.java |
Migrates server builder test. |
server/src/main/java/io/ably/pubsub/server/PubSubServer.java |
Replaces legacy builder methods. |
pubsub-adapter/src/test/kotlin/com/ably/Utils.kt |
Migrates local test client. |
liveobjects/src/test/kotlin/io/ably/pubsub/liveobjects/uts/proxy/ObjectsFaultsTest.kt |
Migrates direct sandbox client. |
liveobjects/src/test/kotlin/io/ably/pubsub/liveobjects/uts/integration/ObjectsSyncTest.kt |
Migrates sync test client. |
liveobjects/src/test/kotlin/io/ably/pubsub/liveobjects/uts/integration/ObjectsLifecycleTest.kt |
Migrates lifecycle test client. |
liveobjects/src/test/kotlin/io/ably/pubsub/liveobjects/uts/integration/Helpers.kt |
Migrates HTTP provisioning. |
liveobjects/src/test/kotlin/io/ably/pubsub/liveobjects/integration/setup/Sandbox.kt |
Migrates sandbox setup. |
lib/src/test/kotlin/io/ably/pubsub/uts/integration/standard/realtime/TokenRequestTest.kt |
Migrates token test clients. |
lib/src/test/kotlin/io/ably/pubsub/uts/integration/standard/realtime/ChannelHistoryTest.kt |
Migrates history test client. |
lib/src/test/java/io/ably/pubsub/transport/HostsTest.java |
Tests endpoint fallback behavior. |
lib/src/test/java/io/ably/pubsub/transport/DefaultsTest.java |
Tests endpoint resolution. |
lib/src/test/java/io/ably/pubsub/test/realtime/RealtimeJWTTest.java |
Migrates JWT test options. |
lib/src/test/java/io/ably/pubsub/test/realtime/RealtimeInitTest.java |
Migrates initialization test. |
lib/src/test/java/io/ably/pubsub/test/realtime/RealtimeHttpHeaderTest.java |
Migrates WebSocket test options. |
lib/src/test/java/io/ably/pubsub/test/realtime/RealtimeConnectFailTest.java |
Migrates failure test endpoints. |
lib/src/test/java/io/ably/pubsub/test/realtime/ConnectionManagerTest.java |
Updates connection host assertions. |
lib/src/test/java/io/ably/pubsub/test/http/HttpTimeTest.java |
Migrates invalid-host test. |
lib/src/test/java/io/ably/pubsub/test/http/HttpTest.java |
Updates HTTP host tests. |
lib/src/test/java/io/ably/pubsub/test/http/HttpRequestTest.java |
Migrates request test endpoints. |
lib/src/test/java/io/ably/pubsub/test/http/HttpProxyTest.java |
Disables fallback for a proxy test. |
lib/src/test/java/io/ably/pubsub/test/http/HttpJWTTest.java |
Derives JWT test environment. |
lib/src/test/java/io/ably/pubsub/test/http/HttpInitTest.java |
Tests new HTTP host resolution. |
lib/src/test/java/io/ably/pubsub/test/http/HttpHeaderTest.java |
Migrates local HTTP tests. |
lib/src/test/java/io/ably/pubsub/test/http/HttpErrorTest.java |
Migrates local error tests. |
lib/src/test/java/io/ably/pubsub/test/http/HttpClientTest.java |
Migrates fallback test endpoints. |
lib/src/test/java/io/ably/pubsub/test/http/HttpAuthTest.java |
Migrates authentication test options. |
lib/src/test/java/io/ably/pubsub/test/common/Setup.java |
Sets test endpoints centrally. |
lib/src/main/java/io/ably/pubsub/types/ClientOptions.java |
Adds endpoint; removes legacy options. |
lib/src/main/java/io/ably/pubsub/transport/Hosts.java |
Derives hosts from client options. |
lib/src/main/java/io/ably/pubsub/transport/Defaults.java |
Resolves endpoint domains and fallbacks. |
lib/src/main/java/io/ably/pubsub/transport/ConnectionManager.java |
Uses shared host resolution. |
lib/src/main/java/io/ably/pubsub/http/HttpCore.java |
Uses shared host resolution. |
lib/src/main/java/io/ably/pubsub/debug/DebugOptions.java |
Copies the endpoint option. |
examples/src/main/kotlin/com/ably/example/screen/MainScreen.kt |
Detects the sandbox endpoint. |
examples/src/main/kotlin/com/ably/example/MainActivity.kt |
Configures the sandbox endpoint. |
device/src/commonMain/kotlin/io/ably/pubsub/device/PubSubDevice.kt |
Replaces legacy builder methods. |
core-android/src/main/java/io/ably/pubsub/push/ActivationContext.java |
Updates an options TODO comment. |
core-android/src/androidTest/java/io/ably/pubsub/test/android/AndroidSuite.java |
Migrates Android HTTP test. |
CONTRIBUTING.md |
Updates test configuration guidance. |
.claude/skills/uts-to-kotlin/SKILL.md |
Updates test-translation guidance. |
.claude/skills/uts-to-kotlin/references/objects-mapping.md |
Updates sandbox mapping guidance. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (isHostname(endpoint)) { | ||
| return endpoint; |
2.0 adopts the REC1/REC2 host model (ADR-119): a single ClientOptions.endpoint selects where both REST requests and the realtime connection go, and the legacy environment, restHost and realtimeHost options are removed outright, so old code fails to compile rather than silently connecting to production. - endpoint resolves as: unset -> main.realtime.ably.net; a routing policy name -> [name].realtime.ably.net; nonprod:[name] -> [name].realtime.ably-nonprod.net; a hostname (contains '.' or '::', or is localhost) -> used as given. Default fallbacks follow the endpoint ([name].[a-e].fallback.ably-realtime[-nonprod].com) and a hostname has none. - Hosts now resolves from ClientOptions alone, so HttpCore and ConnectionManager share one primary domain. The restHost/environment conflict check, the rest.ably.io/realtime.ably.io defaults and the <env>-[a-e]-fallback hosts go with the legacy options. - An explicit fallbackHosts always replaces the defaults (REC2a2), and a custom port or tlsPort no longer suppresses the default fallbacks, as the spec has no such rule. - The server and device builders swap restHost(), realtimeHost() and environment() for endpoint(). The tests move to endpoint throughout: ABLY_ENV, ABLY_REST_HOST and ABLY_REALTIME_HOST become ABLY_ENDPOINT (default nonprod:sandbox), UTS and LiveObjects sandbox clients use the new SandboxApp.sandboxEndpoint, and mock/proxy clients use a localhost endpoint. Tests of the legacy host derivation and option conflicts are replaced by DefaultsTest/HostsTest cases for each endpoint form, custom fallbackHosts, and ports no longer disabling fallbacks. UPGRADING.md gains the migration table and the firewall allowlist note. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
3298eb3 to
f0f58a9
Compare

2.0 adopts the REC1/REC2 host model (ADR-119): a single
ClientOptions.endpoint selects where both REST requests and the realtime
connection go, and the legacy environment, restHost and realtimeHost
options are removed outright, so old code fails to compile rather than
silently connecting to production.
endpoint resolves as: unset -> main.realtime.ably.net; a routing policy
name -> [name].realtime.ably.net; nonprod:[name] ->
[name].realtime.ably-nonprod.net; a hostname (contains '.' or '::', or
is localhost) -> used as given. Default fallbacks follow the endpoint
([name].[a-e].fallback.ably-realtime[-nonprod].com) and a hostname has
none.
Hosts now resolves from ClientOptions alone, so HttpCore and
ConnectionManager share one primary domain. The restHost/environment
conflict check, the rest.ably.io/realtime.ably.io defaults and the
-[a-e]-fallback hosts go with the legacy options.
An explicit fallbackHosts always replaces the defaults (REC2a2), and a
custom port or tlsPort no longer suppresses the default fallbacks, as
the spec has no such rule.
The server and device builders swap restHost(), realtimeHost() and
environment() for endpoint().
Summary by CodeRabbit
New Features
Documentation
Compatibility