[ECO-5426] Added missing unit tests for BaseLiveObject, refactored code

Author: sacOO7

live-objects/src/main/kotlin/io/ably/lib/objects/ObjectsPool.kt

@@ -45,17 +45,13 @@ internal class ObjectsPool(
    * Coroutine scope for garbage collection
    */
   private val gcScope = CoroutineScope(Dispatchers.Default + SupervisorJob())
-
-  /**
-   * Job for the garbage collection coroutine
-   */
-  private var gcJob: Job? = null
+  private var gcJob: Job // Job for the garbage collection coroutine
 
   init {
     // Initialize pool with root object
     createInitialPool()
     // Start garbage collection coroutine
-    startGCJob()
+    gcJob = startGCJob()
   }
 
   /**
@@ -146,8 +142,8 @@ internal class ObjectsPool(
   /**
    * Starts the garbage collection coroutine.
    */
-  private fun startGCJob() {
-    gcJob = gcScope.launch {
+  private fun startGCJob() : Job {
+    return gcScope.launch {
       while (isActive) {
         try {
           onGCInterval()
@@ -164,7 +160,7 @@ internal class ObjectsPool(
    * Should be called when the pool is no longer needed.
    */
   fun dispose() {
-    gcJob?.cancel()
+    gcJob.cancel()
     gcScope.cancel()
     pool.clear()
   }

live-objects/src/main/kotlin/io/ably/lib/objects/ObjectsSyncTracker.kt

@@ -4,9 +4,9 @@ package io.ably.lib.objects
  * @spec RTO5 - SyncTracker class for tracking objects sync status
  */
 internal class ObjectsSyncTracker(syncChannelSerial: String?) {
+  private val syncSerial: String? = syncChannelSerial
   internal val syncId: String?
   internal val syncCursor: String?
-  private val syncSerial: String? = syncChannelSerial
 
   init {
     val parsed = parseSyncChannelSerial(syncChannelSerial)

live-objects/src/main/kotlin/io/ably/lib/objects/type/BaseLiveObject.kt

@@ -29,7 +29,7 @@ internal abstract class BaseLiveObject(
 
   protected open val tag = "BaseLiveObject"
 
-  private val siteTimeserials = mutableMapOf<String, String>() // RTLO3b
+  internal val siteTimeserials = mutableMapOf<String, String>() // RTLO3b
 
   internal var createOperationIsMerged = false // RTLO3c
 
@@ -111,7 +111,7 @@ internal abstract class BaseLiveObject(
    *
    * @spec RTLO4a - Serial comparison logic for LiveMap/LiveCounter operations
    */
-  private fun canApplyOperation(siteCode: String?, timeSerial: String?): Boolean {
+  internal fun canApplyOperation(siteCode: String?, timeSerial: String?): Boolean {
     if (timeSerial.isNullOrEmpty()) {
       throw objectError("Invalid serial: $timeSerial") // RTLO4a3
     }

live-objects/src/main/kotlin/io/ably/lib/objects/type/livecounter/DefaultLiveCounter.kt

@@ -12,7 +12,7 @@ import io.ably.lib.types.Callback
  *
  * @spec RTLC1/RTLC2 - LiveCounter implementation extends LiveObject
  */
-internal class DefaultLiveCounter(
+internal class DefaultLiveCounter private constructor(
   objectId: String,
   adapter: LiveObjectsAdapter,
 ) : LiveCounter, BaseLiveObject(objectId, ObjectType.Counter, adapter) {

live-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.kt

@@ -35,7 +35,7 @@ private fun LiveMapEntry.isEligibleForGc(): Boolean {
  *
  * @spec RTLM1/RTLM2 - LiveMap implementation extends LiveObject
  */
-internal class DefaultLiveMap(
+internal class DefaultLiveMap private constructor(
   objectId: String,
   adapter: LiveObjectsAdapter,
   internal val objectsPool: ObjectsPool,

live-objects/src/test/kotlin/io/ably/lib/objects/TestUtils.kt

@@ -6,6 +6,7 @@ import kotlinx.coroutines.delay
 import kotlinx.coroutines.suspendCancellableCoroutine
 import kotlinx.coroutines.withContext
 import kotlinx.coroutines.withTimeout
+import java.lang.reflect.Method
 
 suspend fun assertWaiter(timeoutInMs: Long = 10_000, block: suspend () -> Boolean) {
   withContext(Dispatchers.Default) {
@@ -58,3 +59,7 @@ fun <T> Any.invokePrivateMethod(methodName: String, vararg args: Any?): T {
   @Suppress("UNCHECKED_CAST")
   return method.invoke(this, *args) as T
 }
+
+fun Class<*>.findMethod(methodName: String): Method {
+  return methods.find { it.name.contains(methodName) } ?: error("Method '$methodName' not found")
+}

live-objects/src/test/kotlin/io/ably/lib/objects/unit/objects/DefaultLiveObjectsTest.kt

@@ -51,7 +51,7 @@ class DefaultLiveObjectsTest {
     // Set up some objects in objectPool that should be cleared
     val rootObject = defaultLiveObjects.objectsPool.get(ROOT_OBJECT_ID) as DefaultLiveMap
     rootObject.data["key1"] = LiveMapEntry(data = ObjectData("testValue1"))
-    defaultLiveObjects.objectsPool.set("counter:testObject@1", DefaultLiveCounter("counter:testObject@1", mockk(relaxed = true)))
+    defaultLiveObjects.objectsPool.set("counter:testObject@1", DefaultLiveCounter.zeroValue("counter:testObject@1", mockk()))
     assertEquals(2, defaultLiveObjects.objectsPool.size(), "RTO4b - Should have 2 objects before state change")
 
     // RTO4b - If the HAS_OBJECTS flag is 0, the sync sequence must be considered complete immediately

live-objects/src/test/kotlin/io/ably/lib/objects/unit/objects/ObjectsManagerTest.kt

@@ -163,8 +163,10 @@ class ObjectsManagerTest {
   }
 
   @Test
-  fun `(RTO7) ObjectsManager should buffer operations during sync, apply them after synced`() {
+  fun `(RTO7) ObjectsManager should buffer operations when not in sync, apply them after synced`() {
     val defaultLiveObjects = getDefaultLiveObjectsWithMockedDeps()
+    assertEquals(ObjectsState.INITIALIZED, defaultLiveObjects.state, "Initial state should be INITIALIZED")
+
     val objectsManager = defaultLiveObjects.ObjectsManager
     assertEquals(0, objectsManager.BufferedObjectOperations.size, "RTO7a1 - Initial buffer should be empty")
 

live-objects/src/test/kotlin/io/ably/lib/objects/unit/objects/ObjectsPoolTest.kt

@@ -32,9 +32,9 @@ class ObjectsPoolTest {
     assertEquals(1, objectsPool.size(), "RTO3 - Should only contain the root object initially")
 
     // RTO3a - ObjectsPool is a Dict, a map of LiveObjects keyed by objectId string
-    val testLiveMap = DefaultLiveMap("map:testObject@1", mockk(relaxed = true), objectsPool)
+    val testLiveMap = DefaultLiveMap.zeroValue("map:testObject@1", mockk(relaxed = true), objectsPool)
     objectsPool.set("map:testObject@1", testLiveMap)
-    val testLiveCounter = DefaultLiveCounter("counter:testObject@1", mockk(relaxed = true))
+    val testLiveCounter = DefaultLiveCounter.zeroValue("counter:testObject@1", mockk(relaxed = true))
     objectsPool.set("counter:testObject@1", testLiveCounter)
     // Assert that the objects are stored in the pool
     assertEquals(testLiveMap, objectsPool.get("map:testObject@1"))
@@ -88,11 +88,11 @@ class ObjectsPoolTest {
     assertEquals(2, rootMap.data.size, "RTO3 - Root map should have initial data")
 
     // Add some objects
-    objectsPool.set("counter:testObject@1", DefaultLiveCounter("counter:testObject@1", mockk(relaxed = true)))
+    objectsPool.set("counter:testObject@1", DefaultLiveCounter.zeroValue("counter:testObject@1", mockk(relaxed = true)))
     assertEquals(2, objectsPool.size()) // root + testObject
-    objectsPool.set("counter:testObject@2", DefaultLiveCounter("counter:testObject@2", mockk(relaxed = true)))
+    objectsPool.set("counter:testObject@2", DefaultLiveCounter.zeroValue("counter:testObject@2", mockk(relaxed = true)))
     assertEquals(3, objectsPool.size()) // root + testObject + anotherObject
-    objectsPool.set("map:testObject@1", DefaultLiveMap("map:testObject@1", mockk(relaxed = true), objectsPool))
+    objectsPool.set("map:testObject@1", DefaultLiveMap.zeroValue("map:testObject@1", mockk(relaxed = true), objectsPool))
     assertEquals(4, objectsPool.size()) // root + testObject + anotherObject + testMap
 
     // Reset to initial pool
@@ -111,9 +111,9 @@ class ObjectsPoolTest {
     val objectsPool = defaultLiveObjects.objectsPool
 
     // Add some objects
-    objectsPool.set("counter:testObject@1", DefaultLiveCounter("counter:testObject@1", mockk(relaxed = true)))
-    objectsPool.set("counter:testObject@2", DefaultLiveCounter("counter:testObject@2", mockk(relaxed = true)))
-    objectsPool.set("counter:testObject@3", DefaultLiveCounter("counter:testObject@3", mockk(relaxed = true)))
+    objectsPool.set("counter:testObject@1", DefaultLiveCounter.zeroValue("counter:testObject@1", mockk(relaxed = true)))
+    objectsPool.set("counter:testObject@2", DefaultLiveCounter.zeroValue("counter:testObject@2", mockk(relaxed = true)))
+    objectsPool.set("counter:testObject@3", DefaultLiveCounter.zeroValue("counter:testObject@3", mockk(relaxed = true)))
     assertEquals(4, objectsPool.size()) // root + 3 objects
 
     // Delete extra object IDs (keep only object1 and object2)

live-objects/src/test/kotlin/io/ably/lib/objects/unit/type/BaseLiveObjectTest.kt

@@ -0,0 +1,170 @@
+package io.ably.lib.objects.unit.type
+
+import io.ably.lib.objects.*
+import io.ably.lib.objects.type.BaseLiveObject
+import io.ably.lib.objects.type.livecounter.DefaultLiveCounter
+import io.ably.lib.objects.type.livemap.DefaultLiveMap
+import io.mockk.mockk
+import org.junit.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertTrue
+import kotlin.test.assertFailsWith
+
+class BaseLiveObjectTest {
+
+  @Test
+  fun `(RTLO1, RTLO2) BaseLiveObject should be abstract base class for LiveMap and LiveCounter`() {
+    // RTLO2 - Check that BaseLiveObject is abstract
+    val isAbstract = java.lang.reflect.Modifier.isAbstract(BaseLiveObject::class.java.modifiers)
+    assertTrue(isAbstract, "BaseLiveObject should be an abstract class")
+
+    // RTLO1 - Check that BaseLiveObject is the parent class of DefaultLiveMap and DefaultLiveCounter
+    assertTrue(BaseLiveObject::class.java.isAssignableFrom(DefaultLiveMap::class.java),
+      "DefaultLiveMap should extend BaseLiveObject")
+    assertTrue(BaseLiveObject::class.java.isAssignableFrom(DefaultLiveCounter::class.java),
+      "DefaultLiveCounter should extend BaseLiveObject")
+  }
+
+  @Test
+  fun `(RTLO3) BaseLiveObject should have required properties`() {
+    val liveMap: BaseLiveObject = DefaultLiveMap.zeroValue("map:testObject@1", mockk(), mockk())
+    val liveCounter: BaseLiveObject = DefaultLiveCounter.zeroValue("counter:testObject@1", mockk())
+    // RTLO3a - check that objectId is set correctly
+    assertEquals("map:testObject@1", liveMap.objectId)
+    assertEquals("counter:testObject@1", liveCounter.objectId)
+
+    // RTLO3b, RTLO3b1 - check that siteTimeserials is initialized as an empty map
+    assertEquals(emptyMap(), liveMap.siteTimeserials)
+    assertEquals(emptyMap(), liveCounter.siteTimeserials)
+
+    // RTLO3c - Create operation merged flag
+    assertFalse(liveMap.createOperationIsMerged, "Create operation should not be merged by default")
+    assertFalse(liveCounter.createOperationIsMerged, "Create operation should not be merged by default")
+  }
+
+    @Test
+  fun `(RTLO4a1, RTLO4a2) canApplyOperation should accept ObjectMessage params and return boolean`() {
+    // RTLO4a1a - Assert parameter types and return type based on method signature using reflection
+    val method = BaseLiveObject::class.java.findMethod("canApplyOperation")
+
+    // RTLO4a1a - Verify parameter types
+    val parameters = method.parameters
+    assertEquals(2, parameters.size, "canApplyOperation should have exactly 2 parameters")
+
+    // First parameter should be String? (siteCode)
+    assertEquals(String::class.java, parameters[0].type, "First parameter should be of type String?")
+    assertTrue(parameters[0].isVarArgs.not(), "First parameter should not be varargs")
+
+    // Second parameter should be String? (timeSerial)
+    assertEquals(String::class.java, parameters[1].type, "Second parameter should be of type String?")
+    assertTrue(parameters[1].isVarArgs.not(), "Second parameter should not be varargs")
+
+    // RTLO4a2 - Verify return type
+    assertEquals(Boolean::class.java, method.returnType, "canApplyOperation should return Boolean")
+  }
+
+  @Test
+  fun `(RTLO4a3) canApplyOperation should throw error for null or empty incoming siteSerial`() {
+    val liveMap: BaseLiveObject = DefaultLiveMap.zeroValue("map:testObject@1", mockk(), mockk())
+
+    // Test null serial
+    assertFailsWith<Exception>("Should throw error for null serial") {
+      liveMap.canApplyOperation("site1", null)
+    }
+
+    // Test empty serial
+    assertFailsWith<Exception>("Should throw error for empty serial") {
+      liveMap.canApplyOperation("site1", "")
+    }
+
+    // Test null siteCode
+    assertFailsWith<Exception>("Should throw error for null site code") {
+      liveMap.canApplyOperation(null, "serial1")
+    }
+
+    // Test empty siteCode
+    assertFailsWith<Exception>("Should throw error for empty site code") {
+      liveMap.canApplyOperation("", "serial1")
+    }
+  }
+
+  @Test
+  fun `(RTLO4a4, RTLO4a5) canApplyOperation should return true when existing siteSerial is null or empty`() {
+    val liveMap: BaseLiveObject = DefaultLiveMap.zeroValue("map:testObject@1", mockk(), mockk())
+    assertTrue(liveMap.siteTimeserials.isEmpty(), "Initial siteTimeserials should be empty")
+
+    // RTLO4a4 - Get siteSerial from siteTimeserials map
+    // RTLO4a5 - Return true when siteSerial is null (no entry in map)
+    assertTrue(liveMap.canApplyOperation("site1", "serial1"),
+      "Should return true when no siteSerial exists for the site")
+
+    // RTLO4a5 - Return true when siteSerial is empty string
+    liveMap.siteTimeserials["site1"] = ""
+    assertTrue(liveMap.canApplyOperation("site1", "serial1"),
+      "Should return true when siteSerial is empty string")
+  }
+
+  @Test
+  fun `(RTLO4a6) canApplyOperation should return true when message siteSerial is greater than existing siteSerial`() {
+    val liveMap: BaseLiveObject = DefaultLiveMap.zeroValue("map:testObject@1", mockk(), mockk())
+
+    // Set existing siteSerial
+    liveMap.siteTimeserials["site1"] = "serial1"
+
+    // RTLO4a6 - Return true when message serial is greater (lexicographically)
+    assertTrue(liveMap.canApplyOperation("site1", "serial2"),
+      "Should return true when message serial 'serial2' > siteSerial 'serial1'")
+
+    assertTrue(liveMap.canApplyOperation("site1", "serial10"),
+      "Should return true when message serial 'serial10' > siteSerial 'serial1'")
+
+    assertTrue(liveMap.canApplyOperation("site1", "serialA"),
+      "Should return true when message serial 'serialA' > siteSerial 'serial1'")
+  }
+
+  @Test
+  fun `(RTLO4a6) canApplyOperation should return false when message siteSerial is less than or equal to siteSerial`() {
+    val liveMap: BaseLiveObject = DefaultLiveMap.zeroValue("map:testObject@1", mockk(), mockk())
+
+    // Set existing siteSerial
+    liveMap.siteTimeserials["site1"] = "serial2"
+
+    // RTLO4a6 - Return false when message serial is less than siteSerial
+    assertFalse(liveMap.canApplyOperation("site1", "serial1"),
+      "Should return false when message serial 'serial1' < siteSerial 'serial2'")
+
+    // RTLO4a6 - Return false when message serial equals siteSerial
+    assertFalse(liveMap.canApplyOperation("site1", "serial2"),
+      "Should return false when message serial equals siteSerial")
+
+    // RTLO4a6 - Return false when message serial is less (lexicographically)
+    assertTrue(liveMap.canApplyOperation("site1", "serialA"),
+      "Should return false when message serial 'serialA' < siteSerial 'serial2'")
+  }
+
+  @Test
+  fun `(RTLO4a) canApplyOperation should work with different site codes`() {
+    val liveMap: BaseLiveObject = DefaultLiveCounter.zeroValue("map:testObject@1", mockk())
+
+    // Set serials for different sites
+    liveMap.siteTimeserials["site1"] = "serial1"
+    liveMap.siteTimeserials["site2"] = "serial5"
+
+    // Test site1
+    assertTrue(liveMap.canApplyOperation("site1", "serial2"),
+      "Should return true for site1 when serial2 > serial1")
+    assertFalse(liveMap.canApplyOperation("site1", "serial1"),
+      "Should return false for site1 when serial1 = serial1")
+
+    // Test site2
+    assertTrue(liveMap.canApplyOperation("site2", "serial6"),
+      "Should return true for site2 when serial6 > serial5")
+    assertFalse(liveMap.canApplyOperation("site2", "serial4"),
+      "Should return false for site2 when serial4 < serial5")
+
+    // Test new site (should return true)
+    assertTrue(liveMap.canApplyOperation("site3", "serial1"),
+      "Should return true for new site with any serial")
+  }
+}