chore(liveobjects): rename plugin and artifactId name #1155
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughRenamed the Live Objects module/artifact from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant A as AblyRealtime
participant C as Connection
participant CM as ConnectionManager
participant P as LiveObjectsPlugin
A->>C: construct(..., liveObjectsPlugin)
C->>CM: construct(..., liveObjectsPlugin)
Note over CM,P: on incoming object / object_sync messages
CM->>P: handle(message)
P-->>CM: handled / ignored
CM-->>C: continue processing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
temporarily deployed
to
staging/pull/1155/features
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java (1)
15-25: DefaultObjectsPlugin must implement LiveObjectsPluginThe Kotlin implementation of your plugin still implements the old
ObjectsPlugininterface, so the cast in ObjectsHelper will fail at runtime. Please:
Update
liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultObjectsPlugin.kt
Change the class declaration from
public class DefaultObjectsPlugin(private val adapter: ObjectsAdapter) : ObjectsPlugin
to
public class DefaultObjectsPlugin(private val adapter: ObjectsAdapter) : LiveObjectsPlugin
and update the import toio.ably.lib.objects.LiveObjectsPlugin.Verify the fully-qualified class name
io.ably.lib.objects.DefaultObjectsPluginremains unchanged so that
Class.forName("io.ably.lib.objects.DefaultObjectsPlugin")still locates your plugin.There are still numerous references to
ObjectsPlugin(e.g. inChannel.java,ConnectionManager.java, Android modules). Ensure all of those usages are renamed toLiveObjectsPluginto complete the refactoring.
🧹 Nitpick comments (11)
README.md (1)
112-118: Dependency coordinate updated; consider adding a Maven snippet for Live Objects.Gradle example now points to io.ably:liveobjects, which matches the artifact rename. For parity, add a Maven example so Maven users aren’t blocked by the rename.
Apply this addition near the Gradle block:
+### Install for Maven (Live Objects): +```xml +<dependency> + <groupId>io.ably</groupId> + <artifactId>liveobjects</artifactId> + <version>1.2.54</version> +</dependency> +```liveobjects/gradle.properties (1)
1-1: ArtifactId switched to 'liveobjects'; consider publishing a relocation POM for 'live-objects'.The rename is correct, but consumers pinned to io.ably:live-objects will 404 on newer versions. If feasible, publish a stub POM under artifactId live-objects with a Maven relocation to liveobjects to smooth upgrades.
Example (to add in a tiny helper subproject publishing only a POM):
publishing { publications { maven(MavenPublication) { groupId = "io.ably" artifactId = "live-objects" version = "$version" pom.withXml { asNode().appendNode('distributionManagement') .appendNode('relocation').with { appendNode('groupId', 'io.ably') appendNode('artifactId', 'liveobjects') appendNode('version', "$version") } } } } }lib/src/main/java/io/ably/lib/objects/LiveObjectsPlugin.java (1)
7-13: Fix Javadoc to match the new type nameThe Javadoc still refers to “ObjectsPlugin” while the interface is now LiveObjectsPlugin. Suggest updating for clarity and API docs correctness.
Apply this minimal edit:
-/** - * The ObjectsPlugin interface provides a mechanism for managing and interacting with +/** + * The LiveObjectsPlugin interface provides a mechanism for managing and interacting with * live data objects in a real-time environment. It allows for the retrieval, disposal, and - * management of Objects instances associated with specific channel names. + * management of RealtimeObjects instances associated with specific channel names. */android/src/main/java/io/ably/lib/realtime/Channel.java (1)
17-18: Annotate plugin parameter as @nullable to mirror ChannelBaseChannelBase takes a @nullable LiveObjectsPlugin. Annotating here preserves intent and aids static analysis on Android build variants.
Apply:
import io.ably.lib.objects.LiveObjectsPlugin; +import org.jetbrains.annotations.Nullable; -Channel(AblyRealtime ably, String name, ChannelOptions options, LiveObjectsPlugin liveObjectsPlugin) throws AblyException { +Channel(AblyRealtime ably, String name, ChannelOptions options, @Nullable LiveObjectsPlugin liveObjectsPlugin) throws AblyException { super(ably, name, options, liveObjectsPlugin);lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java (1)
17-25: Tighten reflection with type-safe asSubclass and catch ClassCastExceptionUsing asSubclass ensures the loaded class actually implements LiveObjectsPlugin, failing fast with a precise exception. Extend the catch to include ClassCastException so we log-and-null consistently.
Apply:
- Class<?> objectsImplementation = Class.forName("io.ably.lib.objects.DefaultObjectsPlugin"); - ObjectsAdapter adapter = new Adapter(ablyRealtime); - return (LiveObjectsPlugin) objectsImplementation - .getDeclaredConstructor(ObjectsAdapter.class) - .newInstance(adapter); + Class<? extends LiveObjectsPlugin> pluginClass = + Class.forName("io.ably.lib.objects.DefaultObjectsPlugin").asSubclass(LiveObjectsPlugin.class); + ObjectsAdapter adapter = new Adapter(ablyRealtime); + return pluginClass + .getDeclaredConstructor(ObjectsAdapter.class) + .newInstance(adapter); - } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | NoSuchMethodException | - InvocationTargetException e) { + } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | NoSuchMethodException | + InvocationTargetException | ClassCastException e) { Log.i(TAG, "LiveObjects plugin not found in classpath. LiveObjects functionality will not be available.", e); return null; }lib/src/main/java/io/ably/lib/transport/ConnectionManager.java (3)
97-103: Mark the LiveObjectsPlugin field as @nullable to reflect its documented nullability.The Javadoc states this can be null; annotate it to avoid confusion and help static analysis.
Apply:
+import org.jetbrains.annotations.Nullable @@ - private final LiveObjectsPlugin liveObjectsPlugin; + @Nullable private final LiveObjectsPlugin liveObjectsPlugin;
776-782: Annotate the constructor parameter as @nullable and keep types aligned.Matches the nullable field and makes the contract explicit.
- public ConnectionManager(final AblyRealtime ably, final Connection connection, final Channels channels, final PlatformAgentProvider platformAgentProvider, LiveObjectsPlugin liveObjectsPlugin) throws AblyException { + public ConnectionManager(final AblyRealtime ably, final Connection connection, final Channels channels, final PlatformAgentProvider platformAgentProvider, @Nullable LiveObjectsPlugin liveObjectsPlugin) throws AblyException { @@ - this.liveObjectsPlugin = liveObjectsPlugin; + this.liveObjectsPlugin = liveObjectsPlugin;Note: ensure the import for org.jetbrains.annotations.Nullable is added as in the previous comment.
1242-1249: Tighten logging and handle the “plugin missing” path explicitly.
- Rename the log to “LiveObjects plugin …” (current “objectsPlugin” is a stale label).
- Optionally warn when object/object_sync is received without the plugin to aid integration debugging.
case object: case object_sync: - if (liveObjectsPlugin != null) { + if (liveObjectsPlugin != null) { try { - liveObjectsPlugin.handle(message); + liveObjectsPlugin.handle(message); } catch (Throwable t) { - Log.e(TAG, "objectsPlugin threw while handling message", t); + Log.e(TAG, "LiveObjects plugin threw while handling message", t); } - } + } else { + Log.w(TAG, "Received Objects protocol message but LiveObjects plugin is not installed; ignoring. Add dependency io.ably:liveobjects:<ably-version>."); + } break;If log volume is a concern, consider a “warn once” guard.
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java (3)
150-156: Update log message to reflect the new symbol and method.Current text “objectsPlugin.handle” is stale; it’s calling liveObjectsPlugin.handleStateChange.
- Log.e(TAG, "Unexpected exception in objectsPlugin.handle", t); + Log.e(TAG, "Unexpected exception in liveObjectsPlugin.handleStateChange", t);
452-458: Same logging nit: reflect LiveObjectsPlugin and the method invoked.- Log.e(TAG, "Unexpected exception in objectsPlugin.handle", t); + Log.e(TAG, "Unexpected exception in liveObjectsPlugin.handleStateChange", t);
1329-1342: Pre-initialize objects instance: wrap in try/catch to avoid constructor failure on plugin issues.A defensive guard prevents a plugin-side exception from failing channel construction unexpectedly.
this.liveObjectsPlugin = liveObjectsPlugin; - if (liveObjectsPlugin != null) { - liveObjectsPlugin.getInstance(name); // Make objects instance ready to process sync messages - } + if (liveObjectsPlugin != null) { + try { + liveObjectsPlugin.getInstance(name); // Make objects instance ready to process sync messages + } catch (Throwable t) { + Log.e(TAG, "Failed to initialize LiveObjects instance for channel " + name, t); + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
CONTRIBUTING.md(1 hunks)README.md(1 hunks)android/src/main/java/io/ably/lib/realtime/Channel.java(2 hunks)examples/build.gradle.kts(1 hunks)lib/src/main/java/io/ably/lib/objects/LiveObjectsPlugin.java(1 hunks)lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java(1 hunks)lib/src/main/java/io/ably/lib/realtime/AblyRealtime.java(6 hunks)lib/src/main/java/io/ably/lib/realtime/ChannelBase.java(6 hunks)lib/src/main/java/io/ably/lib/realtime/Connection.java(2 hunks)lib/src/main/java/io/ably/lib/transport/ConnectionManager.java(4 hunks)liveobjects/gradle.properties(1 hunks)settings.gradle.kts(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:0-0
Timestamp: 2025-08-14T10:43:48.159Z
Learning: In the ably-java codebase, some "LiveObjectsPlugin" references in log messages, Javadoc, build files, and workflows are intentionally retained due to ongoing internal technical debate about isolating API naming from product naming, as discussed in LODR-033. These are not considered incomplete refactoring work but deliberate exceptions to the broader effort to eliminate "liveobject" keyword usage.
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:24-25
Timestamp: 2025-08-14T10:43:57.974Z
Learning: In the ably-java LiveObjects renaming effort, log messages may intentionally retain "LiveObjects" terminology even when other parts of the codebase are being renamed to "Objects", due to internal technical debates about where the renaming should be applied. The team makes deliberate decisions about which parts of the codebase to rename versus which to keep as-is.
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:24-25
Timestamp: 2025-08-15T10:25:24.011Z
Learning: In the ably-java codebase, the ObjectsHelper.java log messages that reference "LiveObjects plugin not found" and "LiveObjects functionality" are also part of the deliberate exceptions to the renaming effort, similar to other "LiveObjectsPlugin" references, due to ongoing internal technical debate about isolating API naming from product naming as discussed in LODR-033.
📚 Learning: 2025-08-14T10:43:48.159Z
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:0-0
Timestamp: 2025-08-14T10:43:48.159Z
Learning: In the ably-java codebase, some "LiveObjectsPlugin" references in log messages, Javadoc, build files, and workflows are intentionally retained due to ongoing internal technical debate about isolating API naming from product naming, as discussed in LODR-033. These are not considered incomplete refactoring work but deliberate exceptions to the broader effort to eliminate "liveobject" keyword usage.
Applied to files:
examples/build.gradle.ktsandroid/src/main/java/io/ably/lib/realtime/Channel.javaliveobjects/gradle.propertiesREADME.mdlib/src/main/java/io/ably/lib/objects/LiveObjectsPlugin.javalib/src/main/java/io/ably/lib/transport/ConnectionManager.javalib/src/main/java/io/ably/lib/realtime/Connection.javalib/src/main/java/io/ably/lib/objects/ObjectsHelper.javalib/src/main/java/io/ably/lib/realtime/AblyRealtime.javalib/src/main/java/io/ably/lib/realtime/ChannelBase.java
📚 Learning: 2025-08-15T10:25:24.011Z
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:24-25
Timestamp: 2025-08-15T10:25:24.011Z
Learning: In the ably-java codebase, the ObjectsHelper.java log messages that reference "LiveObjects plugin not found" and "LiveObjects functionality" are also part of the deliberate exceptions to the renaming effort, similar to other "LiveObjectsPlugin" references, due to ongoing internal technical debate about isolating API naming from product naming as discussed in LODR-033.
Applied to files:
android/src/main/java/io/ably/lib/realtime/Channel.javaliveobjects/gradle.propertieslib/src/main/java/io/ably/lib/objects/LiveObjectsPlugin.javalib/src/main/java/io/ably/lib/transport/ConnectionManager.javalib/src/main/java/io/ably/lib/realtime/Connection.javalib/src/main/java/io/ably/lib/objects/ObjectsHelper.javalib/src/main/java/io/ably/lib/realtime/AblyRealtime.javalib/src/main/java/io/ably/lib/realtime/ChannelBase.java
📚 Learning: 2025-08-14T10:43:57.974Z
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:24-25
Timestamp: 2025-08-14T10:43:57.974Z
Learning: In the ably-java LiveObjects renaming effort, log messages may intentionally retain "LiveObjects" terminology even when other parts of the codebase are being renamed to "Objects", due to internal technical debates about where the renaming should be applied. The team makes deliberate decisions about which parts of the codebase to rename versus which to keep as-is.
Applied to files:
liveobjects/gradle.propertieslib/src/main/java/io/ably/lib/objects/LiveObjectsPlugin.javalib/src/main/java/io/ably/lib/transport/ConnectionManager.javalib/src/main/java/io/ably/lib/realtime/Connection.javalib/src/main/java/io/ably/lib/objects/ObjectsHelper.javalib/src/main/java/io/ably/lib/realtime/AblyRealtime.javalib/src/main/java/io/ably/lib/realtime/ChannelBase.java
📚 Learning: 2025-06-05T10:27:53.946Z
Learnt from: sacOO7
PR: ably/ably-java#1095
File: gradle/libs.versions.toml:24-26
Timestamp: 2025-06-05T10:27:53.946Z
Learning: The ably-java project prefers to use the latest available versions of testing dependencies (including pre-release versions) when they contain relevant bug fixes, rather than sticking strictly to stable releases.
Applied to files:
README.md
📚 Learning: 2025-05-27T12:11:25.084Z
Learnt from: sacOO7
PR: ably/ably-java#1087
File: lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.java:32-34
Timestamp: 2025-05-27T12:11:25.084Z
Learning: In LiveObjects implementation (lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.java), the send method intentionally hardcodes queueEvents to true rather than respecting ably.options.queueMessages. This is because LiveObjects requires reliable message delivery to ensure proper state synchronization and acknowledgment, unlike other realtime components that may allow configurable queuing behavior.
Applied to files:
lib/src/main/java/io/ably/lib/realtime/Connection.javalib/src/main/java/io/ably/lib/realtime/ChannelBase.java
📚 Learning: 2025-06-03T09:15:15.338Z
Learnt from: sacOO7
PR: ably/ably-java#1092
File: live-objects/src/test/kotlin/io/ably/lib/objects/TestUtils.kt:21-32
Timestamp: 2025-06-03T09:15:15.338Z
Learning: In test utility code for the Ably Java Live Objects module, the team prefers to keep reflection-based field access utilities simple without additional error handling, allowing tests to fail fast if incorrect field names are used.
Applied to files:
lib/src/main/java/io/ably/lib/realtime/AblyRealtime.java
📚 Learning: 2025-06-06T09:28:12.298Z
Learnt from: sacOO7
PR: ably/ably-java#1095
File: live-objects/src/test/kotlin/io/ably/lib/objects/integration/setup/IntegrationTest.kt:87-89
Timestamp: 2025-06-06T09:28:12.298Z
Learning: The Sandbox.kt file in ably-java live-objects module already has comprehensive HTTP retry mechanism using HttpRequestRetry with 5 retries, exponential backoff, and automatic retry on non-success responses and timeout exceptions.
Applied to files:
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java
🧬 Code graph analysis (4)
android/src/main/java/io/ably/lib/realtime/Channel.java (2)
java/src/main/java/io/ably/lib/realtime/Channel.java (1)
Channel(8-14)android/src/main/java/io/ably/lib/rest/Channel.java (1)
Channel(7-19)
lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java (1)
liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultObjectsPlugin.kt (1)
adapter(7-35)
lib/src/main/java/io/ably/lib/realtime/AblyRealtime.java (1)
lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java (1)
ObjectsHelper(9-48)
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java (2)
liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultObjectsPlugin.kt (2)
getInstance(11-13)handleStateChange(20-22)liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultRealtimeObjects.kt (1)
handleStateChange(248-286)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: check (29)
- GitHub Check: check-rest
- GitHub Check: check-realtime
- GitHub Check: build
- GitHub Check: check-liveobjects
- GitHub Check: check-rest-okhttp
- GitHub Check: check-realtime-okhttp
- GitHub Check: check (24)
- GitHub Check: check
- GitHub Check: check (29)
- GitHub Check: check (19)
- GitHub Check: check (21)
🔇 Additional comments (13)
CONTRIBUTING.md (1)
56-59: All legacy ‘:live-objects’ references removed and module rename verified
- Ran a repo-wide search for
:live-objects,include("live-objects"), andio.ably:live-objects; no hits found.- Confirmed the new
liveobjects/directory is present.No further stale references detected—ready to approve.
settings.gradle.kts (1)
18-18: No remaininglive-objectsreferences; rename complete
- Ran comprehensive ripgrep searches across all
.gradleand.gradle.ktsfiles (excluding build directories) for the old module namelive-objects; no occurrences found.- Confirmed that
settings.gradle.ktsonly includesliveobjects, and there are noproject(":live-objects")or task-path references.All build configurations have been successfully migrated.
examples/build.gradle.kts (1)
70-71: ✅ Live-objects reference removal verified
Repo-wide searches confirmed no occurrences of the old:live-objectspath in examples, docs, or source. All references have been successfully updated toproject(":liveobjects"). Changes can be approved.lib/src/main/java/io/ably/lib/objects/LiveObjectsPlugin.java (1)
12-12: Interface rename LGTMThe type rename to LiveObjectsPlugin is consistent with the PR goals and does not introduce behavioral changes.
android/src/main/java/io/ably/lib/realtime/Channel.java (1)
6-6: Import and constructor wiring LGTMImporting LiveObjectsPlugin and passing it through to the super constructor align with the wider rename. The cast to AblyRest remains valid as AblyRealtime extends AblyRest.
Also applies to: 17-19
lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java (1)
15-20: Return type/cast switched to LiveObjectsPlugin — looks goodThe factory now returns LiveObjectsPlugin and casts accordingly; behavior remains the same when the plugin is present.
lib/src/main/java/io/ably/lib/realtime/Connection.java (2)
3-3: Constructor and wiring updated to LiveObjectsPlugin — LGTMThe package-private constructor takes LiveObjectsPlugin and forwards it to ConnectionManager; scope limits external API impact.
Also applies to: 126-130
126-130: Verified: ConnectionManager Signature and Call Sites UpdatedThe ConnectionManager constructor at
lib/src/main/java/io/ably/lib/transport/ConnectionManager.java:776now correctly accepts aLiveObjectsPluginparameter, and theConnectioninstantiation inAblyRealtime.java:79passesliveObjectsPluginas required. No remaining plainObjectsPluginreferences were found.lib/src/main/java/io/ably/lib/realtime/AblyRealtime.java (2)
52-53: End-to-end LiveObjectsPlugin integration LGTMField, initialization, propagation into Connection/Channel, and disposal paths are consistently updated. Null-guard on dispose preserves optional classpath inclusion behavior.
Also applies to: 77-80, 124-127, 207-208, 234-236
77-80: No stale ‘live-objects’ references detected—rename is completeAll grep/ripgrep checks for module paths, include directives, Maven coordinates, and bare mentions of “live-objects” returned no matches.
No further action needed.lib/src/main/java/io/ably/lib/transport/ConnectionManager.java (1)
17-17: Rename import looks correct and consistent with PR objective.lib/src/main/java/io/ably/lib/realtime/ChannelBase.java (2)
17-18: Import rename aligns with module and artifact rename.
98-108: getObjects(): good guard + updated guidance to io.ably:liveobjects.Exception contract and message match the rename; returning LiveObjects instance via plugin is correct.
Optional: add/adjust tests to assert
- getObjects() throws 400/40019 when plugin absent,
- returns a non-null instance when plugin is present.
Do you want me to draft these?
ttypic
force-pushed
the
chore/live-objects-renaming
branch
from
39fdfdd to
1c5aad7
Compare
github-actions
bot
temporarily deployed
to
staging/pull/1155/features
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjectsPlugin.kt (1)
15-18: Guard against null channel in ProtocolMessage to prevent CHM NPEs.ProtocolMessage.channel originates from Java and may be null for non-channel messages; ConcurrentHashMap disallows null keys.
Apply:
override fun handle(msg: ProtocolMessage) { - val channelName = msg.channel - objects[channelName]?.handle(msg) + val channelName = msg.channel ?: return + objects[channelName]?.handle(msg) }
🧹 Nitpick comments (3)
liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjectsPlugin.kt (3)
7-7: Drop redundant ‘public’ in Kotlin class declaration (style).public is the default visibility for top-level Kotlin classes; omitting it matches common Kotlin style in this repo.
Apply:
-public class DefaultLiveObjectsPlugin(private val adapter: ObjectsAdapter) : LiveObjectsPlugin { +class DefaultLiveObjectsPlugin(private val adapter: ObjectsAdapter) : LiveObjectsPlugin {
11-13: Avoid potential double-instantiation under concurrency by using computeIfAbsent.getOrPut on ConcurrentHashMap is not atomic and can construct multiple instances for the same key under races. computeIfAbsent guarantees single construction.
Apply:
override fun getInstance(channelName: String): RealtimeObjects { - return objects.getOrPut(channelName) { DefaultRealtimeObjects(channelName, adapter) } + return objects.computeIfAbsent(channelName) { DefaultRealtimeObjects(it, adapter) } }
29-34: Dispose using a snapshot to avoid racing with concurrent mutations.While CHM iterators are weakly consistent, taking a snapshot avoids surprises and lets you reuse a single ErrorInfo instance.
Apply:
override fun dispose() { - objects.values.forEach { - it.dispose(clientError("AblyClient has been closed using client.close()")) - } - objects.clear() + val error = clientError("AblyClient has been closed using client.close()") + val snapshot = objects.values.toList() + snapshot.forEach { it.dispose(error) } + objects.clear() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
CONTRIBUTING.md(1 hunks)README.md(1 hunks)android/src/main/java/io/ably/lib/realtime/Channel.java(2 hunks)examples/build.gradle.kts(1 hunks)java/src/main/java/io/ably/lib/realtime/Channel.java(1 hunks)lib/src/main/java/io/ably/lib/objects/LiveObjectsPlugin.java(1 hunks)lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java(1 hunks)lib/src/main/java/io/ably/lib/realtime/AblyRealtime.java(6 hunks)lib/src/main/java/io/ably/lib/realtime/ChannelBase.java(6 hunks)lib/src/main/java/io/ably/lib/realtime/Connection.java(2 hunks)lib/src/main/java/io/ably/lib/transport/ConnectionManager.java(4 hunks)liveobjects/gradle.properties(1 hunks)liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjectsPlugin.kt(1 hunks)settings.gradle.kts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- README.md
- lib/src/main/java/io/ably/lib/realtime/AblyRealtime.java
- liveobjects/gradle.properties
- examples/build.gradle.kts
- settings.gradle.kts
- lib/src/main/java/io/ably/lib/realtime/Connection.java
- android/src/main/java/io/ably/lib/realtime/Channel.java
- lib/src/main/java/io/ably/lib/objects/LiveObjectsPlugin.java
- lib/src/main/java/io/ably/lib/realtime/ChannelBase.java
- CONTRIBUTING.md
- lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java
- lib/src/main/java/io/ably/lib/transport/ConnectionManager.java
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:0-0
Timestamp: 2025-08-14T10:43:48.159Z
Learning: In the ably-java codebase, some "LiveObjectsPlugin" references in log messages, Javadoc, build files, and workflows are intentionally retained due to ongoing internal technical debate about isolating API naming from product naming, as discussed in LODR-033. These are not considered incomplete refactoring work but deliberate exceptions to the broader effort to eliminate "liveobject" keyword usage.
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:24-25
Timestamp: 2025-08-14T10:43:57.974Z
Learning: In the ably-java LiveObjects renaming effort, log messages may intentionally retain "LiveObjects" terminology even when other parts of the codebase are being renamed to "Objects", due to internal technical debates about where the renaming should be applied. The team makes deliberate decisions about which parts of the codebase to rename versus which to keep as-is.
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:24-25
Timestamp: 2025-08-15T10:25:24.011Z
Learning: In the ably-java codebase, the ObjectsHelper.java log messages that reference "LiveObjects plugin not found" and "LiveObjects functionality" are also part of the deliberate exceptions to the renaming effort, similar to other "LiveObjectsPlugin" references, due to ongoing internal technical debate about isolating API naming from product naming as discussed in LODR-033.
📚 Learning: 2025-08-14T10:43:48.159Z
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:0-0
Timestamp: 2025-08-14T10:43:48.159Z
Learning: In the ably-java codebase, some "LiveObjectsPlugin" references in log messages, Javadoc, build files, and workflows are intentionally retained due to ongoing internal technical debate about isolating API naming from product naming, as discussed in LODR-033. These are not considered incomplete refactoring work but deliberate exceptions to the broader effort to eliminate "liveobject" keyword usage.
Applied to files:
liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjectsPlugin.ktjava/src/main/java/io/ably/lib/realtime/Channel.java
📚 Learning: 2025-08-15T10:25:24.011Z
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:24-25
Timestamp: 2025-08-15T10:25:24.011Z
Learning: In the ably-java codebase, the ObjectsHelper.java log messages that reference "LiveObjects plugin not found" and "LiveObjects functionality" are also part of the deliberate exceptions to the renaming effort, similar to other "LiveObjectsPlugin" references, due to ongoing internal technical debate about isolating API naming from product naming as discussed in LODR-033.
Applied to files:
liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjectsPlugin.ktjava/src/main/java/io/ably/lib/realtime/Channel.java
📚 Learning: 2025-08-14T10:43:57.974Z
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:24-25
Timestamp: 2025-08-14T10:43:57.974Z
Learning: In the ably-java LiveObjects renaming effort, log messages may intentionally retain "LiveObjects" terminology even when other parts of the codebase are being renamed to "Objects", due to internal technical debates about where the renaming should be applied. The team makes deliberate decisions about which parts of the codebase to rename versus which to keep as-is.
Applied to files:
liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjectsPlugin.ktjava/src/main/java/io/ably/lib/realtime/Channel.java
📚 Learning: 2025-08-01T09:53:16.730Z
Learnt from: sacOO7
PR: ably/ably-java#1120
File: live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.kt:0-0
Timestamp: 2025-08-01T09:53:16.730Z
Learning: In the ably-java LiveObjects test code, extension properties with capital letter names (like `State`, `ObjectId`) are defined in live-objects/src/test/kotlin/io/ably/lib/objects/integration/helpers/Utils.kt to provide access to internal fields of concrete implementations through their public interfaces. For example, `LiveObjects.State` casts to `DefaultLiveObjects` to access the internal `state` field for testing purposes.
Applied to files:
liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjectsPlugin.kt
🧬 Code graph analysis (1)
java/src/main/java/io/ably/lib/realtime/Channel.java (2)
android/src/main/java/io/ably/lib/realtime/Channel.java (1)
Channel(9-26)java/src/main/java/io/ably/lib/rest/Channel.java (1)
Channel(6-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: check-liveobjects
- GitHub Check: check-realtime
- GitHub Check: check-rest-okhttp
- GitHub Check: check-realtime-okhttp
- GitHub Check: build
- GitHub Check: check-rest
- GitHub Check: check (19)
- GitHub Check: check (24)
- GitHub Check: check (29)
- GitHub Check: check (21)
- GitHub Check: check
- GitHub Check: check (29)
🔇 Additional comments (3)
liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjectsPlugin.kt (1)
7-7: Rename and interface switch look correct.DefaultLiveObjectsPlugin implements LiveObjectsPlugin as intended; no behavioral change introduced here.
java/src/main/java/io/ably/lib/realtime/Channel.java (2)
3-3: Import updated to LiveObjectsPlugin — consistent with rename.No issues spotted; aligns with Android Channel counterpart.
9-10: All Channel constructor changes and plugin wiring have been verified; no stale references remain
- Confirmed that
ChannelBasedeclares a matching constructor acceptingLiveObjectsPlugin, ensuring thesuper(...)call inChannelis valid.- Verified the presence of the new
DefaultLiveObjectsPluginimplementation inliveobjects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjectsPlugin.kt.- Ran a repository-wide search for
ObjectsPlugin(excluding build outputs): only generic keep rules inandroid/proguard.txt(-keep class io.ably.lib.objects.*ObjectsPlugin) were found, which correctly apply to the new plugin class and are not stale.- Ensured there are no leftover
live-objectsidentifiers or module names outside of CHANGELOG files.
ttypic
force-pushed
the
chore/live-objects-renaming
branch
from
1c5aad7 to
5147cbb
Compare
github-actions
bot
temporarily deployed
to
staging/pull/1155/features
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java (4)
150-156: Name the method in the log for easier diagnosticsThe catch log says “liveObjectsPlugin.handle” but the invoked method is handleStateChange(...). Name it precisely to aid log-based triage. Passing hasObjects=false for non-attached states is sensible and matches DefaultLiveObjectsPlugin’s contract.
- Log.e(TAG, "Unexpected exception in liveObjectsPlugin.handle", t); + Log.e(TAG, "Unexpected exception in liveObjectsPlugin.handleStateChange", t);
452-458: Align log message with the invoked APISame nit as above: reference handleStateChange explicitly.
- Log.e(TAG, "Unexpected exception in liveObjectsPlugin.handle", t); + Log.e(TAG, "Unexpected exception in liveObjectsPlugin.handleStateChange", t);
1329-1347: Guard eager plugin initialization to avoid constructor-time failuresCalling getInstance(name) eagerly is fine to prepare for sync, but if a plugin implementation throws, it will fail channel construction. Since subsequent plugin invocations are already try/catch-guarded elsewhere, consider catching here as well to preserve non-Objects functionality.
- ChannelBase(AblyRealtime ably, String name, ChannelOptions options, @Nullable LiveObjectsPlugin liveObjectsPlugin) throws AblyException { + /** + * Internal constructor. + * @param liveObjectsPlugin Optional LiveObjects plugin; if non-null, an instance for this channel is eagerly initialized. + */ + ChannelBase(AblyRealtime ably, String name, ChannelOptions options, @Nullable LiveObjectsPlugin liveObjectsPlugin) throws AblyException { @@ - if (liveObjectsPlugin != null) { - liveObjectsPlugin.getInstance(name); // Make objects instance ready to process sync messages - } + if (liveObjectsPlugin != null) { + try { + liveObjectsPlugin.getInstance(name); // Eagerly init to process sync messages + } catch (Throwable t) { + // Avoid failing channel construction due to plugin init issues; later calls are already guarded. + Log.e(TAG, "Unexpected exception during liveObjectsPlugin.getInstance() initialization", t); + } + }
98-108: Update “plugin not installed” error to be build-tool-agnosticVerified with
rg -n -S '40019|io\.ably:live-objects'that:
- Error code 40019 only occurs here in ChannelBase.java (lib/src/main/java/io/ably/lib/realtime/ChannelBase.java:104).
- No other hard-coded Maven/Gradle coordinates remain.
The proposed wording aligns with existing “LiveObjects plugin not found” phrasing in ObjectsHelper.java and avoids tooling-specific syntax.
Apply the following change:
@@ lib/src/main/java/io/ably/lib/realtime/ChannelBase.java:102-108 - if (liveObjectsPlugin == null) { - throw AblyException.fromErrorInfo( - new ErrorInfo("LiveObjects plugin hasn't been installed, " + - "add runtimeOnly('io.ably:liveobjects:<ably-version>') to your dependency tree", 400, 40019) - ); - } + if (liveObjectsPlugin == null) { + throw AblyException.fromErrorInfo( + new ErrorInfo( + "LiveObjects plugin not found on classpath. " + + "Add dependency 'io.ably:liveobjects:<ably-version>' to your build.", + 400, 40019) + ); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
CONTRIBUTING.md(1 hunks)README.md(1 hunks)android/src/main/java/io/ably/lib/realtime/Channel.java(2 hunks)examples/build.gradle.kts(1 hunks)java/src/main/java/io/ably/lib/realtime/Channel.java(1 hunks)lib/src/main/java/io/ably/lib/objects/LiveObjectsPlugin.java(1 hunks)lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java(1 hunks)lib/src/main/java/io/ably/lib/realtime/AblyRealtime.java(6 hunks)lib/src/main/java/io/ably/lib/realtime/ChannelBase.java(6 hunks)lib/src/main/java/io/ably/lib/realtime/Connection.java(2 hunks)lib/src/main/java/io/ably/lib/transport/ConnectionManager.java(4 hunks)liveobjects/gradle.properties(1 hunks)liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjectsPlugin.kt(1 hunks)settings.gradle.kts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- lib/src/main/java/io/ably/lib/objects/LiveObjectsPlugin.java
- README.md
- liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjectsPlugin.kt
- lib/src/main/java/io/ably/lib/transport/ConnectionManager.java
- CONTRIBUTING.md
- settings.gradle.kts
- java/src/main/java/io/ably/lib/realtime/Channel.java
- liveobjects/gradle.properties
- lib/src/main/java/io/ably/lib/realtime/AblyRealtime.java
- lib/src/main/java/io/ably/lib/realtime/Connection.java
- lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java
- examples/build.gradle.kts
- android/src/main/java/io/ably/lib/realtime/Channel.java
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:0-0
Timestamp: 2025-08-14T10:43:48.159Z
Learning: In the ably-java codebase, some "LiveObjectsPlugin" references in log messages, Javadoc, build files, and workflows are intentionally retained due to ongoing internal technical debate about isolating API naming from product naming, as discussed in LODR-033. These are not considered incomplete refactoring work but deliberate exceptions to the broader effort to eliminate "liveobject" keyword usage.
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:24-25
Timestamp: 2025-08-14T10:43:57.974Z
Learning: In the ably-java LiveObjects renaming effort, log messages may intentionally retain "LiveObjects" terminology even when other parts of the codebase are being renamed to "Objects", due to internal technical debates about where the renaming should be applied. The team makes deliberate decisions about which parts of the codebase to rename versus which to keep as-is.
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:24-25
Timestamp: 2025-08-15T10:25:24.011Z
Learning: In the ably-java codebase, the ObjectsHelper.java log messages that reference "LiveObjects plugin not found" and "LiveObjects functionality" are also part of the deliberate exceptions to the renaming effort, similar to other "LiveObjectsPlugin" references, due to ongoing internal technical debate about isolating API naming from product naming as discussed in LODR-033.
📚 Learning: 2025-08-14T10:43:48.159Z
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:0-0
Timestamp: 2025-08-14T10:43:48.159Z
Learning: In the ably-java codebase, some "LiveObjectsPlugin" references in log messages, Javadoc, build files, and workflows are intentionally retained due to ongoing internal technical debate about isolating API naming from product naming, as discussed in LODR-033. These are not considered incomplete refactoring work but deliberate exceptions to the broader effort to eliminate "liveobject" keyword usage.
Applied to files:
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java
📚 Learning: 2025-08-15T10:25:24.011Z
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:24-25
Timestamp: 2025-08-15T10:25:24.011Z
Learning: In the ably-java codebase, the ObjectsHelper.java log messages that reference "LiveObjects plugin not found" and "LiveObjects functionality" are also part of the deliberate exceptions to the renaming effort, similar to other "LiveObjectsPlugin" references, due to ongoing internal technical debate about isolating API naming from product naming as discussed in LODR-033.
Applied to files:
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java
📚 Learning: 2025-08-14T10:43:57.974Z
Learnt from: sacOO7
PR: ably/ably-java#1148
File: lib/src/main/java/io/ably/lib/objects/ObjectsHelper.java:24-25
Timestamp: 2025-08-14T10:43:57.974Z
Learning: In the ably-java LiveObjects renaming effort, log messages may intentionally retain "LiveObjects" terminology even when other parts of the codebase are being renamed to "Objects", due to internal technical debates about where the renaming should be applied. The team makes deliberate decisions about which parts of the codebase to rename versus which to keep as-is.
Applied to files:
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java
📚 Learning: 2025-05-27T12:11:25.084Z
Learnt from: sacOO7
PR: ably/ably-java#1087
File: lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.java:32-34
Timestamp: 2025-05-27T12:11:25.084Z
Learning: In LiveObjects implementation (lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.java), the send method intentionally hardcodes queueEvents to true rather than respecting ably.options.queueMessages. This is because LiveObjects requires reliable message delivery to ensure proper state synchronization and acknowledgment, unlike other realtime components that may allow configurable queuing behavior.
Applied to files:
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java
📚 Learning: 2025-06-06T09:28:12.298Z
Learnt from: sacOO7
PR: ably/ably-java#1095
File: live-objects/src/test/kotlin/io/ably/lib/objects/integration/setup/IntegrationTest.kt:87-89
Timestamp: 2025-06-06T09:28:12.298Z
Learning: The Sandbox.kt file in ably-java live-objects module already has comprehensive HTTP retry mechanism using HttpRequestRetry with 5 retries, exponential backoff, and automatic retry on non-success responses and timeout exceptions.
Applied to files:
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java
🧬 Code graph analysis (1)
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java (3)
lib/src/main/java/io/ably/lib/types/AblyException.java (1)
AblyException(12-67)liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjectsPlugin.kt (2)
getInstance(11-13)handleStateChange(20-22)liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultRealtimeObjects.kt (1)
handleStateChange(248-286)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: build
- GitHub Check: check (19)
- GitHub Check: check (21)
- GitHub Check: check (24)
- GitHub Check: check (29)
- GitHub Check: check
- GitHub Check: check-realtime
- GitHub Check: check (29)
- GitHub Check: check-liveobjects
- GitHub Check: check-rest-okhttp
- GitHub Check: check-rest
- GitHub Check: check-realtime-okhttp
sacOO7
left a comment
sacOO7
left a comment
There was a problem hiding this comment.
LGMT.
We also need to update proguard.txt as per comment.
ObjectsPlugintoLiveObjectsPluginio.ably:live-objectstoio.ably:liveobjects(one word)Summary by CodeRabbit
Refactor
Documentation
Chores