Skip to content

Commit 37d3eb8

Browse files
committed
[ECO-5426] Fixed DefaultLiveObjects sync process
1. Removed unused class level currentSyncCursor field 2. Added missing check for empty syncChannelSerial for endSync 3. Added missing overrideWithObjectState as per spec RTO5c1b
1 parent 859bdfd commit 37d3eb8

4 files changed

Lines changed: 27 additions & 84 deletions

File tree

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

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ internal class DefaultLiveObjects(private val channelName: String, private val a
4343
*/
4444
private val syncObjectsDataPool = ConcurrentHashMap<String, ObjectState>()
4545
private var currentSyncId: String? = null
46-
private var currentSyncCursor: String? = null
4746
/**
4847
* @spec RTO5 - Buffered object operations during sync
4948
*/
@@ -107,7 +106,7 @@ internal class DefaultLiveObjects(private val channelName: String, private val a
107106
}
108107

109108
if (protocolMessage.state == null || protocolMessage.state.isEmpty()) {
110-
Log.w(tag, "Received ProtocolMessage with null or empty object state, ignoring")
109+
Log.w(tag, "Received ProtocolMessage with null or empty objects, ignoring")
111110
return
112111
}
113112

@@ -155,14 +154,14 @@ internal class DefaultLiveObjects(private val channelName: String, private val a
155154
val newSyncSequence = currentSyncId != syncId
156155
if (newSyncSequence) {
157156
// RTO5a2 - new sync sequence started
158-
startNewSync(syncId, syncCursor) // RTO5a2a
157+
startNewSync(syncId) // RTO5a2a
159158
}
160159

161160
// RTO5a3 - continue current sync sequence
162161
applyObjectSyncMessages(objectMessages) // RTO5b
163162

164163
// RTO5a4 - if this is the last (or only) message in a sequence of sync updates, end the sync
165-
if (syncCursor == null) {
164+
if (syncChannelSerial.isNullOrEmpty() || syncCursor.isNullOrEmpty()) {
166165
// defer the state change event until the next tick if this was a new sync sequence
167166
// to allow any event listeners to process the start of the new sequence event that was emitted earlier during this event loop.
168167
endSync(newSyncSequence)
@@ -195,14 +194,13 @@ internal class DefaultLiveObjects(private val channelName: String, private val a
195194
*
196195
* @spec RTO5 - Sync sequence initialization
197196
*/
198-
private fun startNewSync(syncId: String?, syncCursor: String?) {
199-
Log.v(tag, "Starting new sync sequence: syncId=$syncId, syncCursor=$syncCursor")
197+
private fun startNewSync(syncId: String?) {
198+
Log.v(tag, "Starting new sync sequence: syncId=$syncId")
200199

201200
// need to discard all buffered object operation messages on new sync start
202201
bufferedObjectOperations.clear()
203202
syncObjectsDataPool.clear()
204203
currentSyncId = syncId
205-
currentSyncCursor = syncCursor
206204
stateChange(ObjectsState.SYNCING, false)
207205
}
208206

@@ -213,16 +211,14 @@ internal class DefaultLiveObjects(private val channelName: String, private val a
213211
*/
214212
private fun endSync(deferStateEvent: Boolean) {
215213
Log.v(tag, "Ending sync sequence")
216-
217214
applySync()
218215
// should apply buffered object operations after we applied the sync.
219-
// can use regular object messages application logic
216+
// can use regular non-sync object.operation logic
220217
applyObjectMessages(bufferedObjectOperations)
221218

222219
bufferedObjectOperations.clear()
223220
syncObjectsDataPool.clear() // RTO5c4
224221
currentSyncId = null // RTO5c3
225-
currentSyncCursor = null // RTO5c3
226222
stateChange(ObjectsState.SYNCED, deferStateEvent)
227223
}
228224

@@ -249,16 +245,15 @@ internal class DefaultLiveObjects(private val channelName: String, private val a
249245
// Update existing object
250246
val update = existingObject.overrideWithObjectState(objectState) // RTO5c1a1
251247
existingObjectUpdates.add(Pair(existingObject, update))
252-
} else {
253-
// RTO5c1b
254-
// Create new object
255-
val newObject = createObjectFromState(objectState) // RTO5c1b1
248+
} else { // RTO5c1b
249+
// RTO5c1b1 - Create new object and add it to the pool
250+
val newObject = createObjectFromState(objectState) //
256251
objectsPool.set(objectId, newObject)
257252
}
258253
}
259254

260255
// RTO5c2 - need to remove LiveObject instances from the ObjectsPool for which objectIds were not received during the sync sequence
261-
objectsPool.deleteExtraObjectIds(receivedObjectIds.toList())
256+
objectsPool.deleteExtraObjectIds(receivedObjectIds)
262257

263258
// call subscription callbacks for all updated existing objects
264259
existingObjectUpdates.forEach { (obj, update) ->
@@ -298,21 +293,27 @@ internal class DefaultLiveObjects(private val channelName: String, private val a
298293
}
299294

300295
val objectState: ObjectState = objectMessage.objectState
301-
syncObjectsDataPool[objectState.objectId] = objectState
296+
if (objectState.counter != null || objectState.map != null) {
297+
syncObjectsDataPool[objectState.objectId] = objectState
298+
} else {
299+
// RTO5c1b1c - object state must contain either counter or map data
300+
Log.w(tag, "Object state received without counter or map data, skipping message: ${objectMessage.id}")
301+
}
302302
}
303303
}
304304

305305
/**
306306
* Creates an object from object state.
307307
*
308-
* @spec RTO5c1b - Creates objects from object state based on type
309-
* TODO - Need to update the implementation
308+
* @spec RTO5c1b - Creates objects from object state based on type
310309
*/
311310
private fun createObjectFromState(objectState: ObjectState): BaseLiveObject {
312311
return when {
313-
objectState.counter != null -> DefaultLiveCounter(objectState.objectId, objectsPool) // RTO5c1b1a
314-
objectState.map != null -> DefaultLiveMap(objectState.objectId, objectsPool) // RTO5c1b1b
315-
else -> throw serverError("Object state must contain either counter or map data") // RTO5c1b1c
312+
objectState.counter != null -> DefaultLiveCounter.zeroValue(objectState.objectId, objectsPool) // RTO5c1b1a
313+
objectState.map != null -> DefaultLiveMap.zeroValue(objectState.objectId, objectsPool) // RTO5c1b1b
314+
else -> throw clientError("Object state must contain either counter or map data") // RTO5c1b1c
315+
}.apply {
316+
overrideWithObjectState(objectState)
316317
}
317318
}
318319

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,10 @@ internal class ObjectsPool(
7070
/**
7171
* Deletes objects from the pool for which object ids are not found in the provided array of ids.
7272
*/
73-
fun deleteExtraObjectIds(objectIds: List<String>) {
74-
val poolObjectIds = pool.keys.toList()
75-
val extraObjectIds = poolObjectIds.filter { !objectIds.contains(it) }
76-
77-
extraObjectIds.forEach { remove(it) }
73+
fun deleteExtraObjectIds(objectIds: MutableSet<String>) {
74+
pool.keys.toList()
75+
.filter { it !in objectIds }
76+
.forEach { remove(it) }
7877
}
7978

8079
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ internal class DefaultLiveMap(
6767

6868
if (isTombstoned) {
6969
// this object is tombstoned. this is a terminal state which can't be overridden. skip the rest of object state message processing
70-
return mapOf<String, String>()
70+
return mapOf()
7171
}
7272

7373
val previousData = data.toMap()

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

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -48,63 +48,6 @@ class ObjectIdTest {
4848
assertAblyExceptionError(exception1)
4949
}
5050

51-
@Test
52-
fun testObjectIdWithMultipleColons() {
53-
val exception = assertThrows(AblyException::class.java) {
54-
ObjectId.fromString("map:abc:123@1640995200000")
55-
}
56-
57-
assertAblyExceptionError(exception)
58-
}
59-
60-
@Test
61-
fun testObjectIdWithoutAtSymbol() {
62-
val exception = assertThrows(AblyException::class.java) {
63-
ObjectId.fromString("map:abc1231640995200000")
64-
}
65-
assertAblyExceptionError(exception)
66-
}
67-
68-
@Test
69-
fun testObjectIdWithMultipleAtSymbols() {
70-
val exception = assertThrows(AblyException::class.java) {
71-
ObjectId.fromString("map:abc123@1640995200000@extra")
72-
}
73-
assertAblyExceptionError(exception)
74-
}
75-
76-
@Test
77-
fun testObjectIdWithEmptyHash() {
78-
val exception = assertThrows(AblyException::class.java) {
79-
ObjectId.fromString("map:@1640995200000")
80-
}
81-
assertAblyExceptionError(exception)
82-
}
83-
84-
@Test
85-
fun testObjectIdWithOnlyType() {
86-
val exception = assertThrows(AblyException::class.java) {
87-
ObjectId.fromString("map:")
88-
}
89-
assertAblyExceptionError(exception)
90-
}
91-
92-
@Test
93-
fun testObjectIdWithOnlyTypeAndHash() {
94-
val exception = assertThrows(AblyException::class.java) {
95-
ObjectId.fromString("map:abc123")
96-
}
97-
assertAblyExceptionError(exception)
98-
}
99-
100-
@Test
101-
fun testObjectIdWithOnlyTypeAndTimestamp() {
102-
val exception = assertThrows(AblyException::class.java) {
103-
ObjectId.fromString("map:1640995200000")
104-
}
105-
assertAblyExceptionError(exception)
106-
}
107-
10851
private fun assertAblyExceptionError(
10952
exception: AblyException
11053
) {

0 commit comments

Comments
 (0)