Implement remaining LiveMap access API properties

lawrence-forooghian · lawrence-forooghian · commit 982147adc0f4 · 2025-07-04T15:48:29.000-03:00
Based on [1] at 7d4c215. A few outstanding questions on the PR; have implemented based on my current understanding of what's there. Development approach similar to that described in 4494033. Also, have not implemented the specification points related to RTO2's channel mode checking for same reasons as mentioned there. [1] ably/specification#341
diff --git a/Sources/AblyLiveObjects/Internal/DefaultLiveMap.swift b/Sources/AblyLiveObjects/Internal/DefaultLiveMap.swift
@@ -120,78 +120,66 @@ internal final class DefaultLiveMap: LiveMap {
             return nil
         }
 
-        // RTLM5d2: If a ObjectsMapEntry exists at the key
-
-        // RTLM5d2a: If ObjectsMapEntry.tombstone is true, return undefined/null
-        if entry.tombstone == true {
-            return nil
-        }
-
-        // Handle primitive values in the order specified by RTLM5d2b through RTLM5d2e
-
-        // RTLM5d2b: If ObjectsMapEntry.data.boolean exists, return it
-        if let boolean = entry.data.boolean {
-            return .primitive(.bool(boolean))
-        }
-
-        // RTLM5d2c: If ObjectsMapEntry.data.bytes exists, return it
-        if let bytes = entry.data.bytes {
-            return .primitive(.data(bytes))
-        }
+        // RTLM5d2: If a ObjectsMapEntry exists at the key, convert it using the shared logic
+        return convertEntryToLiveMapValue(entry)
+    }
 
-        // RTLM5d2d: If ObjectsMapEntry.data.number exists, return it
-        if let number = entry.data.number {
-            return .primitive(.number(number.doubleValue))
-        }
+    internal var size: Int {
+        get throws(ARTErrorInfo) {
+            // RTLM10c: If the channel is in the DETACHED or FAILED state, the library should throw an ErrorInfo error with statusCode 400 and code 90001
+            let currentChannelState = coreSDK.channelState
+            if currentChannelState == .detached || currentChannelState == .failed {
+                throw ARTErrorInfo.create(withCode: Int(ARTErrorCode.channelOperationFailedInvalidState.rawValue), message: "LiveMap.size operation failed (invalid channel state: \(currentChannelState))")
+            }
 
-        // RTLM5d2e: If ObjectsMapEntry.data.string exists, return it
-        if let string = entry.data.string {
-            switch string {
-            case let .string(string):
-                return .primitive(.string(string))
-            case .json:
-                // TODO: Understand how to handle JSON values (https://github.com/ably/specification/pull/333/files#r2164561055)
-                notYetImplemented()
+            return mutex.withLock {
+                // RTLM10d: Returns the number of non-tombstoned entries (per RTLM14) in the internal data map
+                mutableState.data.values.count { entry in
+                    // RTLM14a: The method returns true if ObjectsMapEntry.tombstone is true
+                    // RTLM14b: Otherwise, it returns false
+                    entry.tombstone != true
+                }
             }
         }
+    }
 
-        // RTLM5d2f: If ObjectsMapEntry.data.objectId exists, get the object stored at that objectId from the internal ObjectsPool
-        if let objectId = entry.data.objectId {
-            // RTLM5d2f1: If an object with id objectId does not exist, return undefined/null
-            guard let poolEntry = delegate.referenced?.getObjectFromPool(id: objectId) else {
-                return nil
+    internal var entries: [(key: String, value: LiveMapValue)] {
+        get throws(ARTErrorInfo) {
+            // RTLM11c: If the channel is in the DETACHED or FAILED state, the library should throw an ErrorInfo error with statusCode 400 and code 90001
+            let currentChannelState = coreSDK.channelState
+            if currentChannelState == .detached || currentChannelState == .failed {
+                throw ARTErrorInfo.create(withCode: Int(ARTErrorCode.channelOperationFailedInvalidState.rawValue), message: "LiveMap.entries operation failed (invalid channel state: \(currentChannelState))")
             }
 
-            // RTLM5d2f2: If an object with id objectId exists, return it
-            switch poolEntry {
-            case let .map(map):
-                return .liveMap(map)
-            case let .counter(counter):
-                return .liveCounter(counter)
-            }
-        }
+            return mutex.withLock {
+                // RTLM11d: Returns key-value pairs from the internal data map
+                // RTLM11d1: Pairs with tombstoned entries (per RTLM14) are not returned
+                var result: [(key: String, value: LiveMapValue)] = []
 
-        // RTLM5d2g: Otherwise, return undefined/null
-        return nil
-    }
+                for (key, entry) in mutableState.data {
+                    // Convert entry to LiveMapValue using the same logic as get(key:)
+                    if let value = convertEntryToLiveMapValue(entry) {
+                        result.append((key: key, value: value))
+                    }
+                }
 
-    internal var size: Int {
-        mutex.withLock {
-            // TODO: this is not yet specified, but it seems like the obvious right thing and it unlocks some integration tests; add spec point once specified
-            mutableState.data.count
+                return result
+            }
         }
     }
 
-    internal var entries: [(key: String, value: LiveMapValue)] {
-        notYetImplemented()
-    }
-
     internal var keys: [String] {
-        notYetImplemented()
+        get throws(ARTErrorInfo) {
+            // RTLM12b: Identical to LiveMap#entries, except that it returns only the keys from the internal data map
+            try entries.map(\.key)
+        }
     }
 
     internal var values: [LiveMapValue] {
-        notYetImplemented()
+        get throws(ARTErrorInfo) {
+            // RTLM13b: Identical to LiveMap#entries, except that it returns only the values from the internal data map
+            try entries.map(\.value)
+        }
     }
 
     internal func set(key _: String, value _: LiveMapValue) async throws(ARTErrorInfo) {
@@ -441,4 +429,63 @@ internal final class DefaultLiveMap: LiveMap {
             }
         }
     }
+
+    // MARK: - Helper Methods
+
+    /// Converts an ObjectsMapEntry to LiveMapValue using the same logic as get(key:)
+    /// This is used by entries to ensure consistent value conversion
+    private func convertEntryToLiveMapValue(_ entry: ObjectsMapEntry) -> LiveMapValue? {
+        // RTLM5d2a: If ObjectsMapEntry.tombstone is true, return undefined/null
+        // This is also equivalent to the RTLM14 check
+        if entry.tombstone == true {
+            return nil
+        }
+
+        // Handle primitive values in the order specified by RTLM5d2b through RTLM5d2e
+
+        // RTLM5d2b: If ObjectsMapEntry.data.boolean exists, return it
+        if let boolean = entry.data.boolean {
+            return .primitive(.bool(boolean))
+        }
+
+        // RTLM5d2c: If ObjectsMapEntry.data.bytes exists, return it
+        if let bytes = entry.data.bytes {
+            return .primitive(.data(bytes))
+        }
+
+        // RTLM5d2d: If ObjectsMapEntry.data.number exists, return it
+        if let number = entry.data.number {
+            return .primitive(.number(number.doubleValue))
+        }
+
+        // RTLM5d2e: If ObjectsMapEntry.data.string exists, return it
+        if let string = entry.data.string {
+            switch string {
+            case let .string(string):
+                return .primitive(.string(string))
+            case .json:
+                // TODO: Understand how to handle JSON values (https://github.com/ably/specification/pull/333/files#r2164561055)
+                notYetImplemented()
+            }
+        }
+
+        // RTLM5d2f: If ObjectsMapEntry.data.objectId exists, get the object stored at that objectId from the internal ObjectsPool
+        if let objectId = entry.data.objectId {
+            // RTLM5d2f1: If an object with id objectId does not exist, return undefined/null
+            guard let poolEntry = delegate.referenced?.getObjectFromPool(id: objectId) else {
+                return nil
+            }
+
+            // RTLM5d2f2: If an object with id objectId exists, return it
+            switch poolEntry {
+            case let .map(map):
+                return .liveMap(map)
+            case let .counter(counter):
+                return .liveCounter(counter)
+            }
+        }
+
+        // RTLM5d2g: Otherwise, return undefined/null
+        return nil
+    }
 }
diff --git a/Tests/AblyLiveObjectsTests/DefaultLiveMapTests.swift b/Tests/AblyLiveObjectsTests/DefaultLiveMapTests.swift
@@ -263,6 +263,174 @@ struct DefaultLiveMapTests {
         }
     }
 
+    /// Tests for the `size`, `entries`, `keys`, and `values` properties, covering RTLM10, RTLM11, RTLM12, and RTLM13 specification points
+    struct AccessPropertiesTests {
+        // MARK: - Error Throwing Tests (RTLM10c, RTLM11c, RTLM12b, RTLM13b)
+
+        // @spec RTLM10c
+        // @spec RTLM11c
+        // @spec RTLM12b
+        // @spec RTLM13b
+        @Test(arguments: [.detached, .failed] as [ARTRealtimeChannelState])
+        func allPropertiesThrowIfChannelIsDetachedOrFailed(channelState: ARTRealtimeChannelState) async throws {
+            let map = DefaultLiveMap.createZeroValued(delegate: MockLiveMapObjectPoolDelegate(), coreSDK: MockCoreSDK(channelState: channelState))
+
+            // Define actions to test
+            let actions: [(String, () throws -> Any)] = [
+                ("size", { try map.size }),
+                ("entries", { try map.entries }),
+                ("keys", { try map.keys }),
+                ("values", { try map.values }),
+            ]
+
+            // Test each property throws the expected error
+            for (propertyName, action) in actions {
+                #expect("\(propertyName) should throw") {
+                    _ = try action()
+                } throws: { error in
+                    guard let errorInfo = error as? ARTErrorInfo else {
+                        return false
+                    }
+                    return errorInfo.code == 90001
+                }
+            }
+        }
+
+        // MARK: - Tombstone Filtering Tests (RTLM10d, RTLM11d1, RTLM12b, RTLM13b)
+
+        // @specOneOf(1/2) RTLM10d - Tests the "non-tombstoned" part of spec point
+        // @spec RTLM11d1
+        // @specOneOf(1/2) RTLM12b - Tests the "non-tombstoned" part of RTLM10d
+        // @specOneOf(1/2) RTLM13b - Tests the "non-tombstoned" part of RTLM10d
+        // @spec RTLM14
+        @Test
+        func allPropertiesFilterOutTombstonedEntries() throws {
+            let coreSDK = MockCoreSDK(channelState: .attaching)
+            let map = DefaultLiveMap(
+                testsOnly_data: [
+                    // tombstone is nil, so not considered tombstoned
+                    "active1": TestFactories.mapEntry(data: ObjectData(string: .string("value1"))),
+                    // tombstone is false, so not considered tombstoned[
+                    "active2": TestFactories.mapEntry(tombstone: false, data: ObjectData(string: .string("value2"))),
+                    "tombstoned": TestFactories.mapEntry(tombstone: true, data: ObjectData(string: .string("tombstoned"))),
+                    "tombstoned2": TestFactories.mapEntry(tombstone: true, data: ObjectData(string: .string("tombstoned2"))),
+                ],
+                delegate: nil,
+                coreSDK: coreSDK,
+            )
+
+            // Test size - should only count non-tombstoned entries
+            let size = try map.size
+            #expect(size == 2)
+
+            // Test entries - should only return non-tombstoned entries
+            let entries = try map.entries
+            #expect(entries.count == 2)
+            #expect(Set(entries.map(\.key)) == ["active1", "active2"])
+            #expect(entries.first { $0.key == "active1" }?.value.stringValue == "value1")
+            #expect(entries.first { $0.key == "active2" }?.value.stringValue == "value2")
+
+            // Test keys - should only return keys from non-tombstoned entries
+            let keys = try map.keys
+            #expect(keys.count == 2)
+            #expect(Set(keys) == ["active1", "active2"])
+
+            // Test values - should only return values from non-tombstoned entries
+            let values = try map.values
+            #expect(values.count == 2)
+            #expect(Set(values.compactMap(\.stringValue)) == Set(["value1", "value2"]))
+        }
+
+        // MARK: - Consistency Tests
+
+        // @specOneOf(2/2) RTLM10d
+        // @specOneOf(2/2) RTLM12b
+        // @specOneOf(2/2) RTLM13b
+        @Test
+        func allAccessPropertiesReturnExpectedValuesAndAreConsistentWithEachOther() throws {
+            let coreSDK = MockCoreSDK(channelState: .attaching)
+            let map = DefaultLiveMap(
+                testsOnly_data: [
+                    "key1": TestFactories.mapEntry(data: ObjectData(string: .string("value1"))),
+                    "key2": TestFactories.mapEntry(data: ObjectData(string: .string("value2"))),
+                    "key3": TestFactories.mapEntry(data: ObjectData(string: .string("value3"))),
+                ],
+                delegate: nil,
+                coreSDK: coreSDK,
+            )
+
+            let size = try map.size
+            let entries = try map.entries
+            let keys = try map.keys
+            let values = try map.values
+
+            // All properties should return the same count
+            #expect(size == 3)
+            #expect(entries.count == 3)
+            #expect(keys.count == 3)
+            #expect(values.count == 3)
+
+            // Keys should match the keys from entries
+            #expect(Set(keys) == Set(entries.map(\.key)))
+
+            // Values should match the values from entries
+            #expect(Set(values.compactMap(\.stringValue)) == Set(entries.compactMap(\.value.stringValue)))
+        }
+
+        // MARK: - `entries` handling of different value types, per RTLM5d2
+
+        // @spec RTLM11d
+        @Test
+        func entriesHandlesAllValueTypes() throws {
+            let delegate = MockLiveMapObjectPoolDelegate()
+            let coreSDK = MockCoreSDK(channelState: .attaching)
+
+            // Create referenced objects for testing
+            let referencedMap = DefaultLiveMap.createZeroValued(delegate: delegate, coreSDK: coreSDK)
+            let referencedCounter = DefaultLiveCounter.createZeroValued(coreSDK: coreSDK)
+            delegate.objects["map:ref@123"] = .map(referencedMap)
+            delegate.objects["counter:ref@456"] = .counter(referencedCounter)
+
+            let map = DefaultLiveMap(
+                testsOnly_data: [
+                    "boolean": TestFactories.mapEntry(data: ObjectData(boolean: true)), // RTLM5d2b
+                    "bytes": TestFactories.mapEntry(data: ObjectData(bytes: Data([0x01, 0x02, 0x03]))), // RTLM5d2c
+                    "number": TestFactories.mapEntry(data: ObjectData(number: NSNumber(value: 42))), // RTLM5d2d
+                    "string": TestFactories.mapEntry(data: ObjectData(string: .string("hello"))), // RTLM5d2e
+                    "mapRef": TestFactories.mapEntry(data: ObjectData(objectId: "map:ref@123")), // RTLM5d2f2
+                    "counterRef": TestFactories.mapEntry(data: ObjectData(objectId: "counter:ref@456")), // RTLM5d2f2
+                ],
+                delegate: delegate,
+                coreSDK: coreSDK,
+            )
+
+            let size = try map.size
+            let entries = try map.entries
+            let keys = try map.keys
+            let values = try map.values
+
+            #expect(size == 6)
+            #expect(entries.count == 6)
+            #expect(keys.count == 6)
+            #expect(values.count == 6)
+
+            // Verify the correct values are returned by `entries`
+            let booleanEntry = entries.first { $0.key == "boolean" } // RTLM5d2b
+            let bytesEntry = entries.first { $0.key == "bytes" } // RTLM5d2c
+            let numberEntry = entries.first { $0.key == "number" } // RTLM5d2d
+            let stringEntry = entries.first { $0.key == "string" } // RTLM5d2e
+            let mapRefEntry = entries.first { $0.key == "mapRef" } // RTLM5d2f2
+            let counterRefEntry = entries.first { $0.key == "counterRef" } // RTLM5d2f2
+
+            #expect(booleanEntry?.value.boolValue == true) // RTLM5d2b
+            #expect(bytesEntry?.value.dataValue == Data([0x01, 0x02, 0x03])) // RTLM5d2c
+            #expect(numberEntry?.value.numberValue == 42) // RTLM5d2d
+            #expect(stringEntry?.value.stringValue == "hello") // RTLM5d2e
+            #expect(mapRefEntry?.value.liveMapValue as AnyObject === referencedMap as AnyObject) // RTLM5d2f2
+            #expect(counterRefEntry?.value.liveCounterValue as AnyObject === referencedCounter as AnyObject) // RTLM5d2f2
+        }
+    }
+
     /// Tests for `MAP_SET` operations, covering RTLM7 specification points
     struct MapSetOperationTests {
         // MARK: - RTLM7a Tests (Existing Entry)
