Skip to content

Commit 4c8a3bc

Browse files
sacOO7sacOO7
committed
[ECO-5426] Fixed DefaultLiveMap and DefaultLiveCounter constructors
1. Removed `objectPool` from LiveCounter constructor, added adapter to both LiveCounter and LiveMap 2. Updated calculateUpdateFromDataDiff method to calculate update diff. in livemap 3. Added unit test covering all cases for calculateUpdateFromDataDiff
1 parent c71df5a commit 4c8a3bc

7 files changed

Lines changed: 257 additions & 32 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,8 @@ internal class DefaultLiveObjects(private val channelName: String, private val a
309309
*/
310310
private fun createObjectFromState(objectState: ObjectState): BaseLiveObject {
311311
return when {
312-
objectState.counter != null -> DefaultLiveCounter.zeroValue(objectState.objectId, objectsPool) // RTO5c1b1a
313-
objectState.map != null -> DefaultLiveMap.zeroValue(objectState.objectId, objectsPool) // RTO5c1b1b
312+
objectState.counter != null -> DefaultLiveCounter.zeroValue(objectState.objectId, adapter) // RTO5c1b1a
313+
objectState.map != null -> DefaultLiveMap.zeroValue(objectState.objectId, adapter, objectsPool) // RTO5c1b1b
314314
else -> throw clientError("Object state must contain either counter or map data") // RTO5c1b1c
315315
}.apply {
316316
overrideWithObjectState(objectState)

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,8 @@ internal class ObjectsPool(
126126

127127
val parsedObjectId = ObjectId.fromString(objectId) // RTO6b
128128
val zeroValueObject = when (parsedObjectId.type) {
129-
ObjectType.Map -> DefaultLiveMap.zeroValue(objectId, this) // RTO6b2
130-
ObjectType.Counter -> DefaultLiveCounter.zeroValue(objectId, this) // RTO6b3
129+
ObjectType.Map -> DefaultLiveMap.zeroValue(objectId, adapter, this) // RTO6b2
130+
ObjectType.Counter -> DefaultLiveCounter.zeroValue(objectId, adapter) // RTO6b3
131131
}
132132

133133
set(objectId, zeroValueObject)
@@ -140,7 +140,7 @@ internal class ObjectsPool(
140140
* @spec RTO3b - Creates root LiveMap object
141141
*/
142142
private fun createInitialPool() {
143-
val root = DefaultLiveMap.zeroValue(ROOT_OBJECT_ID, this)
143+
val root = DefaultLiveMap.zeroValue(ROOT_OBJECT_ID, adapter, this)
144144
pool[ROOT_OBJECT_ID] = root
145145
}
146146

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.ably.lib.objects.type
22

3+
import io.ably.lib.objects.LiveObjectsAdapter
34
import io.ably.lib.objects.ObjectMessage
45
import io.ably.lib.objects.ObjectOperation
56
import io.ably.lib.objects.ObjectState
@@ -15,7 +16,7 @@ import io.ably.lib.util.Log
1516
*/
1617
internal abstract class BaseLiveObject(
1718
protected val objectId: String,
18-
protected val objectsPool: ObjectsPool
19+
protected val adapter: LiveObjectsAdapter
1920
) {
2021

2122
protected open val tag = "BaseLiveObject"

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ import io.ably.lib.util.Log
2222
*/
2323
internal class DefaultLiveCounter(
2424
objectId: String,
25-
objectsPool: ObjectsPool
26-
) : BaseLiveObject(objectId, objectsPool), LiveCounter {
25+
adapter: LiveObjectsAdapter,
26+
) : BaseLiveObject(objectId, adapter), LiveCounter {
2727

2828
override val tag = "LiveCounter"
2929
/**
@@ -46,7 +46,7 @@ internal class DefaultLiveCounter(
4646

4747
if (isTombstoned) {
4848
// this object is tombstoned. this is a terminal state which can't be overridden. skip the rest of object state message processing
49-
return mapOf("amount" to 0L)
49+
return mapOf()
5050
}
5151

5252
val previousData = data
@@ -195,8 +195,8 @@ internal class DefaultLiveCounter(
195195
* Creates a zero-value counter object.
196196
* @spec RTLC4 - Returns LiveCounter with 0 value
197197
*/
198-
internal fun zeroValue(objectId: String, objectsPool: ObjectsPool): DefaultLiveCounter {
199-
return DefaultLiveCounter(objectId, objectsPool)
198+
internal fun zeroValue(objectId: String, adapter: LiveObjectsAdapter): DefaultLiveCounter {
199+
return DefaultLiveCounter(objectId, adapter)
200200
}
201201
}
202202
}

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

Lines changed: 39 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,17 @@ import io.ably.lib.util.Log
2525
*/
2626
internal class DefaultLiveMap(
2727
objectId: String,
28-
objectsPool: ObjectsPool,
28+
adapter: LiveObjectsAdapter,
29+
private val objectsPool: ObjectsPool,
2930
private val semantics: MapSemantics = MapSemantics.LWW
30-
) : BaseLiveObject(objectId, objectsPool), LiveMap {
31+
) : BaseLiveObject(objectId, adapter), LiveMap {
3132

3233
override val tag = "LiveMap"
3334

3435
/**
3536
* @spec RTLM3 - Map data structure storing entries
3637
*/
37-
private data class LiveMapEntry(
38+
internal data class LiveMapEntry(
3839
var tombstone: Boolean = false,
3940
var tombstonedAt: Long? = null,
4041
var timeserial: String? = null,
@@ -322,30 +323,47 @@ internal class DefaultLiveMap(
322323
val update = mutableMapOf<String, String>()
323324

324325
// Check for removed entries
325-
for ((key, entry) in prevData) {
326-
if (!entry.tombstone && !newData.containsKey(key)) {
326+
for ((key, prevEntry) in prevData) {
327+
if (!prevEntry.tombstone && !newData.containsKey(key)) {
327328
update[key] = "removed"
328329
}
329330
}
330331

331332
// Check for added/updated entries
332-
for ((key, entry) in newData) {
333+
for ((key, newEntry) in newData) {
333334
if (!prevData.containsKey(key)) {
334-
if (!entry.tombstone) {
335+
// if property does not exist in current map, but new data has it as non-tombstoned property - got updated
336+
if (!newEntry.tombstone) {
335337
update[key] = "updated"
336338
}
337-
} else {
338-
val prevEntry = prevData[key]!!
339-
if (prevEntry.tombstone && !entry.tombstone) {
340-
update[key] = "updated"
341-
} else if (!prevEntry.tombstone && entry.tombstone) {
342-
update[key] = "removed"
343-
} else if (!prevEntry.tombstone && !entry.tombstone) {
344-
// Compare values
345-
if (prevEntry.data != entry.data) {
346-
update[key] = "updated"
347-
}
348-
}
339+
// otherwise, if new data has this prop tombstoned - do nothing, as property didn't exist anyway
340+
continue
341+
}
342+
343+
// properties that exist both in current and new map data need to have their values compared to decide on update type
344+
val prevEntry = prevData[key]!!
345+
346+
// compare tombstones first
347+
if (prevEntry.tombstone && !newEntry.tombstone) {
348+
// prev prop is tombstoned, but new is not. it means prop was updated to a meaningful value
349+
update[key] = "updated"
350+
continue
351+
}
352+
if (!prevEntry.tombstone && newEntry.tombstone) {
353+
// prev prop is not tombstoned, but new is. it means prop was removed
354+
update[key] = "removed"
355+
continue
356+
}
357+
if (prevEntry.tombstone && newEntry.tombstone) {
358+
// props are tombstoned - treat as noop, as there is no data to compare
359+
continue
360+
}
361+
362+
// both props exist and are not tombstoned, need to compare values to see if it was changed
363+
val valueChanged = prevEntry.data != newEntry.data
364+
if (valueChanged) {
365+
update[key] = "updated"
366+
continue
349367
}
350368
}
351369

@@ -412,8 +430,8 @@ internal class DefaultLiveMap(
412430
* Creates a zero-value map object.
413431
* @spec RTLM4 - Returns LiveMap with empty map data
414432
*/
415-
internal fun zeroValue(objectId: String, objectsPool: ObjectsPool): DefaultLiveMap {
416-
return DefaultLiveMap(objectId, objectsPool)
433+
internal fun zeroValue(objectId: String, adapter: LiveObjectsAdapter, objectsPool: ObjectsPool): DefaultLiveMap {
434+
return DefaultLiveMap(objectId, adapter, objectsPool)
417435
}
418436
}
419437
}

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

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
package io.ably.lib.objects.unit
2+
3+
import io.ably.lib.objects.*
4+
import io.ably.lib.objects.type.DefaultLiveMap
5+
import io.ably.lib.objects.type.DefaultLiveMap.LiveMapEntry
6+
import io.mockk.mockk
7+
import org.junit.Test
8+
import org.junit.Assert.*
9+
10+
class LiveMapTest {
11+
12+
private val livemap = DefaultLiveMap("test-channel", mockk(), mockk())
13+
14+
@Test
15+
fun shouldCalculateMapDifferenceCorrectly() {
16+
// Test case 1: No changes
17+
val prevData1 = mapOf<String, LiveMapEntry>()
18+
val newData1 = mapOf<String, LiveMapEntry>()
19+
val result1 = livemap.calculateDiff(prevData1, newData1)
20+
assertEquals("Should return empty map for no changes", emptyMap<String, String>(), result1)
21+
22+
// Test case 2: Entry added
23+
val prevData2 = mapOf<String, LiveMapEntry>()
24+
val newData2 = mapOf(
25+
"key1" to LiveMapEntry(
26+
tombstone = false,
27+
timeserial = "1",
28+
data = ObjectData(value = ObjectValue("value1"))
29+
)
30+
)
31+
val result2 = livemap.calculateDiff(prevData2, newData2)
32+
assertEquals("Should detect added entry", mapOf("key1" to "updated"), result2)
33+
34+
// Test case 3: Entry removed
35+
val prevData3 = mapOf(
36+
"key1" to LiveMapEntry(
37+
tombstone = false,
38+
timeserial = "1",
39+
data = ObjectData(value = ObjectValue("value1"))
40+
)
41+
)
42+
val newData3 = mapOf<String, LiveMapEntry>()
43+
val result3 = livemap.calculateDiff(prevData3, newData3)
44+
assertEquals("Should detect removed entry", mapOf("key1" to "removed"), result3)
45+
46+
// Test case 4: Entry updated
47+
val prevData4 = mapOf(
48+
"key1" to LiveMapEntry(
49+
tombstone = false,
50+
timeserial = "1",
51+
data = ObjectData(value = ObjectValue("value1"))
52+
)
53+
)
54+
val newData4 = mapOf(
55+
"key1" to LiveMapEntry(
56+
tombstone = false,
57+
timeserial = "2",
58+
data = ObjectData(value = ObjectValue("value2"))
59+
)
60+
)
61+
val result4 = livemap.calculateDiff(prevData4, newData4)
62+
assertEquals("Should detect updated entry", mapOf("key1" to "updated"), result4)
63+
64+
// Test case 5: Entry tombstoned
65+
val prevData5 = mapOf(
66+
"key1" to LiveMapEntry(
67+
tombstone = false,
68+
timeserial = "1",
69+
data = ObjectData(value = ObjectValue("value1"))
70+
)
71+
)
72+
val newData5 = mapOf(
73+
"key1" to LiveMapEntry(
74+
tombstone = true,
75+
timeserial = "2",
76+
data = null
77+
)
78+
)
79+
val result5 = livemap.calculateDiff(prevData5, newData5)
80+
assertEquals("Should detect tombstoned entry", mapOf("key1" to "removed"), result5)
81+
82+
// Test case 6: Entry untombstoned
83+
val prevData6 = mapOf(
84+
"key1" to LiveMapEntry(
85+
tombstone = true,
86+
timeserial = "1",
87+
data = null
88+
)
89+
)
90+
val newData6 = mapOf(
91+
"key1" to LiveMapEntry(
92+
tombstone = false,
93+
timeserial = "2",
94+
data = ObjectData(value = ObjectValue("value1"))
95+
)
96+
)
97+
val result6 = livemap.calculateDiff(prevData6, newData6)
98+
assertEquals("Should detect untombstoned entry", mapOf("key1" to "updated"), result6)
99+
100+
// Test case 7: Both entries tombstoned (noop)
101+
val prevData7 = mapOf(
102+
"key1" to LiveMapEntry(
103+
tombstone = true,
104+
timeserial = "1",
105+
data = null
106+
)
107+
)
108+
val newData7 = mapOf(
109+
"key1" to LiveMapEntry(
110+
tombstone = true,
111+
timeserial = "2",
112+
data = ObjectData(value = ObjectValue("value1"))
113+
)
114+
)
115+
val result7 = livemap.calculateDiff(prevData7, newData7)
116+
assertEquals("Should not detect change for both tombstoned entries", emptyMap<String, String>(), result7)
117+
118+
// Test case 8: New tombstoned entry (noop)
119+
val prevData8 = mapOf<String, LiveMapEntry>()
120+
val newData8 = mapOf(
121+
"key1" to LiveMapEntry(
122+
tombstone = true,
123+
timeserial = "1",
124+
data = null
125+
)
126+
)
127+
val result8 = livemap.calculateDiff(prevData8, newData8)
128+
assertEquals("Should not detect change for new tombstoned entry", emptyMap<String, String>(), result8)
129+
130+
// Test case 9: Multiple changes
131+
val prevData9 = mapOf(
132+
"key1" to LiveMapEntry(
133+
tombstone = false,
134+
timeserial = "1",
135+
data = ObjectData(value = ObjectValue("value1"))
136+
),
137+
"key2" to LiveMapEntry(
138+
tombstone = false,
139+
timeserial = "1",
140+
data = ObjectData(value = ObjectValue("value2"))
141+
)
142+
)
143+
val newData9 = mapOf(
144+
"key1" to LiveMapEntry(
145+
tombstone = false,
146+
timeserial = "2",
147+
data = ObjectData(value = ObjectValue("value1_updated"))
148+
),
149+
"key3" to LiveMapEntry(
150+
tombstone = false,
151+
timeserial = "1",
152+
data = ObjectData(value = ObjectValue("value3"))
153+
)
154+
)
155+
val result9 = livemap.calculateDiff(prevData9, newData9)
156+
val expected9 = mapOf(
157+
"key1" to "updated",
158+
"key2" to "removed",
159+
"key3" to "updated"
160+
)
161+
assertEquals("Should detect multiple changes correctly", expected9, result9)
162+
163+
// Test case 10: ObjectId references
164+
val prevData10 = mapOf(
165+
"key1" to LiveMapEntry(
166+
tombstone = false,
167+
timeserial = "1",
168+
data = ObjectData(objectId = "obj1")
169+
)
170+
)
171+
val newData10 = mapOf(
172+
"key1" to LiveMapEntry(
173+
tombstone = false,
174+
timeserial = "1",
175+
data = ObjectData(objectId = "obj2")
176+
)
177+
)
178+
val result10 = livemap.calculateDiff(prevData10, newData10)
179+
assertEquals("Should detect objectId change", mapOf("key1" to "updated"), result10)
180+
181+
// Test case 11: Same data, no change
182+
val prevData11 = mapOf(
183+
"key1" to LiveMapEntry(
184+
tombstone = false,
185+
timeserial = "1",
186+
data = ObjectData(value = ObjectValue("value1"))
187+
)
188+
)
189+
val newData11 = mapOf(
190+
"key1" to LiveMapEntry(
191+
tombstone = false,
192+
timeserial = "2",
193+
data = ObjectData(value = ObjectValue("value1"))
194+
)
195+
)
196+
val result11 = livemap.calculateDiff(prevData11, newData11)
197+
assertEquals("Should not detect change for same data", emptyMap<String, String>(), result11)
198+
}
199+
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package io.ably.lib.objects.unit
22

3+
import io.ably.lib.objects.invokePrivateMethod
4+
import io.ably.lib.objects.type.DefaultLiveMap
5+
import io.ably.lib.objects.type.DefaultLiveMap.LiveMapEntry
36
import io.ably.lib.realtime.AblyRealtime
47
import io.ably.lib.realtime.Channel
58
import io.ably.lib.realtime.ChannelState
@@ -35,3 +38,7 @@ internal fun getMockRealtimeChannel(
3538
state = ChannelState.attached
3639
}
3740
}
41+
42+
internal fun DefaultLiveMap.calculateDiff(prevData: Map<String, LiveMapEntry>, newData: Map<String, LiveMapEntry>): Map<String, String> {
43+
return this.invokePrivateMethod("calculateUpdateFromDataDiff",prevData, newData)
44+
}

0 commit comments

Comments
 (0)