From b410737f407d647edfd1623f51a6c99e58a110cc Mon Sep 17 00:00:00 2001 From: jee-hyun-kim Date: Mon, 21 Aug 2023 18:19:13 +0900 Subject: [PATCH 01/13] add more tests on presences --- .../kotlin/dev/yorkie/core/PresenceTest.kt | 269 ++++++++++++++++++ .../src/main/kotlin/dev/yorkie/core/Client.kt | 47 +-- .../main/kotlin/dev/yorkie/core/Presence.kt | 2 +- .../main/kotlin/dev/yorkie/core/Presences.kt | 7 + .../kotlin/dev/yorkie/document/Document.kt | 37 ++- 5 files changed, 333 insertions(+), 29 deletions(-) diff --git a/yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt b/yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt index 02576e2c8..bd636b1d3 100644 --- a/yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt +++ b/yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt @@ -18,6 +18,7 @@ import kotlinx.coroutines.withTimeout import org.junit.Test import org.junit.runner.RunWith import java.util.UUID +import kotlin.test.assertIs @RunWith(AndroidJUnit4::class) class PresenceTest { @@ -306,5 +307,273 @@ class PresenceTest { } } + @Test + fun test_returning_online_clients_only() { + val cursor = gson.toJson(Cursor(0, 0)) + + runBlocking { + val c1 = createClient() + val c2 = createClient() + val c3 = createClient() + + val documentKey = UUID.randomUUID().toString().toDocKey() + val d1 = Document(documentKey) + val d2 = Document(documentKey) + val d3 = Document(documentKey) + val d1Events = mutableListOf() + + c1.activateAsync().await() + c2.activateAsync().await() + c3.activateAsync().await() + + val c1ID = c1.requireClientId() + val c2ID = c2.requireClientId() + val c3ID = c3.requireClientId() + + c1.attachAsync(d1, initialPresence = mapOf("name" to "a1", "cursor" to cursor)).await() + + val d1CollectJob = launch(start = CoroutineStart.UNDISPATCHED) { + d1.events.filterIsInstance() + .collect(d1Events::add) + } + + // 01. c2 attaches doc in realtime sync, and c3 attached doc in manual sync. + c2.attachAsync(d2, initialPresence = mapOf("name" to "b1", "cursor" to cursor)).await() + c3.attachAsync(d3, mapOf("name" to "c1", "cursor" to cursor), false).await() + + withTimeout(GENERAL_TIMEOUT) { + // c2 watched + while (d1Events.isEmpty()) { + delay(50) + } + } + + assertIs(d1Events.last()) + assertEquals( + mapOf(c1ID to d1.presences.value[c1ID], c2ID to d2.presences.value[c2ID]), + d1.presences.value.toMap(), + ) + assertNull(d1.presences.value[c3ID]) + + // 02. c2 pauses the document (in manual sync), c3 resumes the document (in realtime sync). + c2.pause(d2) + + withTimeout(GENERAL_TIMEOUT) { + // c2 unwatched + while (d1Events.size < 2) { + delay(50) + } + } + + assertIs(d1Events.last()) + c3.resume(d3) + + withTimeout(GENERAL_TIMEOUT) { + // c3 watched + while (d1Events.size < 3) { + delay(50) + } + } + + assertIs(d1Events.last()) + assertEquals( + mapOf(c1ID to d1.presences.value[c1ID], c3ID to d3.presences.value[c3ID]), + d1.presences.value.toMap(), + ) + assertNull(d1.presences.value[c2ID]) + + d1CollectJob.cancel() + c1.detachAsync(d1).await() + c3.detachAsync(d3).await() + c1.deactivateAsync().await() + c2.deactivateAsync().await() + c3.deactivateAsync().await() + } + } + + @Test + fun test_receiving_presence_events_only_for_realtime_sync() { + val cursor = gson.toJson(Cursor(0, 0)) + runBlocking { + val c1 = createClient() + val c2 = createClient() + val c3 = createClient() + + val documentKey = UUID.randomUUID().toString().toDocKey() + val d1 = Document(documentKey) + val d2 = Document(documentKey) + val d3 = Document(documentKey) + val d1Events = mutableListOf() + + c1.activateAsync().await() + c2.activateAsync().await() + c3.activateAsync().await() + + val c2ID = c2.requireClientId() + val c3ID = c3.requireClientId() + + c1.attachAsync(d1, initialPresence = mapOf("name" to "a1", "cursor" to cursor)).await() + + val d1CollectJob = launch(start = CoroutineStart.UNDISPATCHED) { + d1.events.filterIsInstance() + .collect(d1Events::add) + } + + // 01. c2 attaches doc in realtime sync, and c3 attached doc in manual sync. + // c1 receives the watched event from c2. + c2.attachAsync(d2, initialPresence = mapOf("name" to "b1", "cursor" to cursor)).await() + c3.attachAsync(d3, mapOf("name" to "c1", "cursor" to cursor), false).await() + + withTimeout(GENERAL_TIMEOUT) { + // c2 watched + while (d1Events.isEmpty()) { + delay(50) + } + } + + assertEquals(1, d1Events.size) + assertEquals( + Others.Watched(PresenceInfo(c2ID, mapOf("name" to "b1", "cursor" to cursor))), + d1Events.last(), + ) + + // 02. c2 and c3 update the presence. + // c1 receives the presence-changed event from c2. + d2.updateAsync { _, presence -> + presence.put(mapOf("name" to "b2")) + }.await() + + d3.updateAsync { _, presence -> + presence.put(mapOf("name" to "c2")) + }.await() + + withTimeout(GENERAL_TIMEOUT) { + // c2 presence changed + while (d1Events.size < 2) { + delay(50) + } + } + + assertEquals(2, d1Events.size) + assertEquals( + Others.PresenceChanged( + PresenceInfo( + c2ID, + mapOf("name" to "b2", "cursor" to cursor), + ), + ), + d1Events.last(), + ) + + // 03-1. c2 pauses the document, c1 receives an unwatched event from c2. + c2.pause(d2) + + withTimeout(GENERAL_TIMEOUT) { + // c2 unwatched + while (d1Events.size < 3) { + delay(50) + } + } + + assertEquals(3, d1Events.size) + assertEquals( + Others.Unwatched(PresenceInfo(c2ID, mapOf("name" to "b2", "cursor" to cursor))), + d1Events.last(), + ) + + // 03-2. c3 resumes the document, c1 receives a watched event from c3. + // NOTE(chacha912): The events are influenced by the timing of realtime sync + // and watch stream resolution. For deterministic testing, the resume is performed + // after the sync. Since the sync updates c1 with all previous presence changes + // from c3, only the watched event is triggered. + c3.syncAsync().await() + c1.syncAsync().await() + c3.resume(d3) + + withTimeout(GENERAL_TIMEOUT) { + // c3 watched + while (d1Events.size < 4) { + delay(50) + } + } + + assertEquals(4, d1Events.size) + assertEquals( + Others.Watched(PresenceInfo(c3ID, mapOf("name" to "c2", "cursor" to cursor))), + d1Events.last(), + ) + + // 04. c2 and c3 update the presence. + // c1 receives the presence-changed event from c3. + d2.updateAsync { _, presence -> + presence.put(mapOf("name" to "b3")) + }.await() + + d3.updateAsync { _, presence -> + presence.put(mapOf("name" to "c3")) + }.await() + + withTimeout(GENERAL_TIMEOUT) { + // c3 presence changed + while (d1Events.size < 5) { + delay(50) + } + } + + assertEquals(5, d1Events.size) + assertEquals( + Others.PresenceChanged( + PresenceInfo( + c3ID, + mapOf("name" to "c3", "cursor" to cursor), + ), + ), + d1Events.last(), + ) + + // 05-1. c3 pauses the document, c1 receives an unwatched event from c3. + c3.pause(d3) + + withTimeout(GENERAL_TIMEOUT) { + // c3 unwatched + while (d1Events.size < 6) { + delay(50) + } + } + + assertEquals(6, d1Events.size) + assertEquals( + Others.Unwatched(PresenceInfo(c3ID, mapOf("name" to "c3", "cursor" to cursor))), + d1Events.last(), + ) + + // 05-2. c2 resumes the document, c1 receives a watched event from c2. + c2.syncAsync().await() + c1.syncAsync().await() + c2.resume(d2) + + withTimeout(GENERAL_TIMEOUT) { + // c3 unwatched + while (d1Events.size < 7) { + delay(50) + } + } + + assertEquals(7, d1Events.size) + assertEquals( + Others.Watched(PresenceInfo(c2ID, mapOf("name" to "b3", "cursor" to cursor))), + d1Events.last(), + ) + + d1CollectJob.cancel() + c1.detachAsync(d1).await() + c2.detachAsync(d2).await() + c3.detachAsync(d3).await() + c1.deactivateAsync().await() + c2.deactivateAsync().await() + c3.deactivateAsync().await() + } + } + private data class Cursor(val x: Int, val y: Int) } diff --git a/yorkie/src/main/kotlin/dev/yorkie/core/Client.kt b/yorkie/src/main/kotlin/dev/yorkie/core/Client.kt index c45b07ede..7d4d80587 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/core/Client.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/core/Client.kt @@ -2,6 +2,7 @@ package dev.yorkie.core import android.content.Context import com.google.common.annotations.VisibleForTesting +import com.google.protobuf.ByteString import dev.yorkie.api.toActorID import dev.yorkie.api.toByteString import dev.yorkie.api.toChangePack @@ -19,6 +20,7 @@ import dev.yorkie.api.v1.watchDocumentRequest import dev.yorkie.core.Client.DocumentSyncResult.SyncFailed import dev.yorkie.core.Client.DocumentSyncResult.Synced import dev.yorkie.core.Client.Event.DocumentSynced +import dev.yorkie.core.Presences.Companion.UninitializedPresences import dev.yorkie.core.Presences.Companion.asPresences import dev.yorkie.document.Document import dev.yorkie.document.Document.DocumentStatus @@ -44,6 +46,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asFlow import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.fold import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.retry @@ -280,14 +283,12 @@ public class Client @VisibleForTesting internal constructor( response: WatchDocumentResponse, ) { if (response.hasInitialization()) { - response.initialization.clientIdsList.forEach { pbClientID -> - val clientID = pbClientID.toActorID() - val document = attachments.value[documentKey]?.document ?: return - document.onlineClients.add(clientID) - document.publish( - PresenceChange.MyPresence.Initialized(document.presences.value.asPresences()), - ) - } + val document = attachments.value[documentKey]?.document ?: return + val clientIDs = response.initialization.clientIdsList.map(ByteString::toActorID) + document.onlineClients.value = document.onlineClients.value + clientIDs + document.publish( + PresenceChange.MyPresence.Initialized(document.presences.value.asPresences()), + ) return } @@ -295,25 +296,29 @@ public class Client @VisibleForTesting internal constructor( val eventType = checkNotNull(watchEvent.type) // only single key will be received since 0.3.1 server. val attachment = attachments.value[documentKey] ?: return + val document = attachment.document val publisher = watchEvent.publisher.toActorID() when (eventType) { DocEventType.DOC_EVENT_TYPE_DOCUMENTS_WATCHED -> { - attachment.document.onlineClients.add(publisher) - if (publisher in attachment.document.presences.value) { - val presence = attachment.document.presences.value[publisher] ?: return - attachment.document.publish( + // NOTE(chacha912): We added to onlineClients, but we won't trigger watched event + // unless we also know their initial presence data at this point. + document.onlineClients.value = document.onlineClients.value + publisher + if (publisher in document.allPresences.value) { + val presence = document.presences.first { publisher in it }[publisher] ?: return + document.publish( PresenceChange.Others.Watched(PresenceInfo(publisher, presence)), ) } } DocEventType.DOC_EVENT_TYPE_DOCUMENTS_UNWATCHED -> { - attachment.document.onlineClients.remove(publisher) - val presence = attachment.document.presences.value[publisher] ?: return - attachment.document.publish( - PresenceChange.Others.Unwatched(PresenceInfo(publisher, presence)), - ) + // NOTE(chacha912): There is no presence, + // when PresenceChange(clear) is applied before unwatching. In that case, + // the 'unwatched' event is triggered while handling the PresenceChange. + val presence = document.presences.value[publisher] ?: return + document.onlineClients.value = document.onlineClients.value - publisher + document.publish(PresenceChange.Others.Unwatched(PresenceInfo(publisher, presence))) } DocEventType.DOC_EVENT_TYPE_DOCUMENTS_CHANGED -> { @@ -376,6 +381,7 @@ public class Client @VisibleForTesting internal constructor( response.documentId, isRealTimeSync, ) + waitForInitialization(document.key) true } } @@ -483,6 +489,13 @@ public class Client @VisibleForTesting internal constructor( } } + private suspend fun waitForInitialization(documentKey: Document.Key) { + val attachment = attachments.value[documentKey] ?: return + if (attachment.isRealTimeSync) { + attachment.document.presences.first { it != UninitializedPresences } + } + } + public fun requireClientId() = (status.value as Status.Activated).clientId /** diff --git a/yorkie/src/main/kotlin/dev/yorkie/core/Presence.kt b/yorkie/src/main/kotlin/dev/yorkie/core/Presence.kt index a00659b9a..4cdec0fae 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/core/Presence.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/core/Presence.kt @@ -13,7 +13,7 @@ public data class Presence internal constructor( public fun put(data: Map) { presence.putAll(data) - changeContext.presenceChange = PresenceChange.Put(presence + data) + changeContext.presenceChange = PresenceChange.Put(presence) } public fun clear() { diff --git a/yorkie/src/main/kotlin/dev/yorkie/core/Presences.kt b/yorkie/src/main/kotlin/dev/yorkie/core/Presences.kt index c4d082123..ec156a386 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/core/Presences.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/core/Presences.kt @@ -30,6 +30,13 @@ public class Presences private constructor( public fun Pair.asPresences(): Presences { return Presences(mutableMapOf(first to second.toMutableMap())) } + + internal val UninitializedPresences = Presences( + object : HashMap>() { + + override fun equals(other: Any?): Boolean = this === other + }, + ) } } diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt b/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt index b95393f70..284dce4d7 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt @@ -8,6 +8,7 @@ import dev.yorkie.core.Presence import dev.yorkie.core.PresenceChange import dev.yorkie.core.PresenceInfo import dev.yorkie.core.Presences +import dev.yorkie.core.Presences.Companion.UninitializedPresences import dev.yorkie.core.Presences.Companion.asPresences import dev.yorkie.document.Document.Event.PresenceChange.MyPresence import dev.yorkie.document.Document.Event.PresenceChange.Others @@ -39,9 +40,9 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.filterNot import kotlinx.coroutines.flow.first -import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapNotNull import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.withContext @@ -82,14 +83,14 @@ public class Document(public val key: Key) { internal val garbageLength: Int get() = root.getGarbageLength() - internal val onlineClients = mutableSetOf() + internal val onlineClients = MutableStateFlow(setOf()) - private val _presences = MutableStateFlow(Presences()) - public val presences: StateFlow = _presences.map { presences -> - presences.filterKeys { it in onlineClients }.asPresences() - }.stateIn(scope, SharingStarted.Eagerly, _presences.value.asPresences()) + private val _presences = MutableStateFlow(UninitializedPresences) + public val presences: StateFlow = + combine(_presences, onlineClients) { presences, onlineClients -> + presences.filterKeys { it in onlineClients }.asPresences() + }.stateIn(scope, SharingStarted.Eagerly, _presences.value.asPresences()) - @VisibleForTesting internal val allPresences: StateFlow = _presences.asStateFlow() /** @@ -130,7 +131,7 @@ public class Document(public val key: Key) { val change = context.getChange() val (operationInfos, newPresences) = change.execute(root, _presences.value) - newPresences?.let { _presences.emit(it) } + newPresences?.let { emitPresences(it) } localChanges += change changeID = change.id @@ -264,21 +265,30 @@ public class Document(public val key: Key) { } val actorID = change.id.actor var event: Event? = null - if (change.hasPresenceChange && actorID in onlineClients) { + if (change.hasPresenceChange && actorID in onlineClients.value) { val presenceChange = change.presenceChange ?: return@forEach event = when (presenceChange) { is PresenceChange.Put -> { if (actorID in _presences.value) { createPresenceChangedEvent(actorID, presenceChange.presence) } else { + // NOTE(chacha912): When the user exists in onlineClients, but + // their presence was initially absent, we can consider that we have + // received their initial presence, so trigger the 'watched' event. Others.Watched(PresenceInfo(actorID, presenceChange.presence)) } } is PresenceChange.Clear -> { - onlineClients.remove(actorID) + // NOTE(chacha912): When the user exists in onlineClients, but + // PresenceChange(clear) is received, we can consider it as detachment + // occurring before unwatching. + // Detached user is no longer participating in the document, we remove + // them from the online clients and trigger the 'unwatched' event. presences.value[actorID]?.let { presence -> Others.Unwatched(PresenceInfo(actorID, presence)) + }.also { + onlineClients.value = onlineClients.value - actorID } } } @@ -290,7 +300,7 @@ public class Document(public val key: Key) { } event?.let { eventStream.emit(it) } - newPresences?.let { _presences.emit(it) } + newPresences?.let { emitPresences(it) } changeID = changeID.syncLamport(change.id.lamport) } } @@ -299,6 +309,11 @@ public class Document(public val key: Key) { clone ?: RootClone(root.deepCopy(), _presences.value.asPresences()).also { clone = it } } + private suspend fun emitPresences(newPresences: Presences) { + _presences.emit(newPresences) + clone = ensureClone().copy(presences = newPresences) + } + /** * Create [ChangePack] of [localChanges] to send to the remote server. */ From 3d24c2955f076783bea334e383e213c67b891e9a Mon Sep 17 00:00:00 2001 From: jee-hyun-kim Date: Mon, 21 Aug 2023 18:31:38 +0900 Subject: [PATCH 02/13] add OperationInfo interfaces --- .../com/example/texteditor/EditorViewModel.kt | 8 +-- .../document/operation/OperationInfo.kt | 50 +++++++++++++++---- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/examples/texteditor/src/main/java/com/example/texteditor/EditorViewModel.kt b/examples/texteditor/src/main/java/com/example/texteditor/EditorViewModel.kt index e7f8cbdbf..4bdeaacd2 100644 --- a/examples/texteditor/src/main/java/com/example/texteditor/EditorViewModel.kt +++ b/examples/texteditor/src/main/java/com/example/texteditor/EditorViewModel.kt @@ -30,8 +30,8 @@ class EditorViewModel(private val client: Client) : ViewModel(), YorkieEditText. private val _content = MutableSharedFlow() val content = _content.asSharedFlow() - private val _textOpInfos = MutableSharedFlow>() - val textOpInfos = _textOpInfos.asSharedFlow() + private val _textOperationInfos = MutableSharedFlow>() + val textOpInfos = _textOperationInfos.asSharedFlow() val removedPeers = document.events.filterIsInstance() .map { it.unwatched.actorID } @@ -73,7 +73,7 @@ class EditorViewModel(private val client: Client) : ViewModel(), YorkieEditText. private suspend fun emitEditOpInfos(changeInfo: Document.Event.ChangeInfo) { changeInfo.operations.filterIsInstance() .forEach { opInfo -> - _textOpInfos.emit(changeInfo.actorID to opInfo) + _textOperationInfos.emit(changeInfo.actorID to opInfo) } } @@ -85,7 +85,7 @@ class EditorViewModel(private val client: Client) : ViewModel(), YorkieEditText. gson.fromJson(jsonArray.getString(1), TextPosStructure::class.java) ?: return val (from, to) = document.getRoot().getAs(TEXT_KEY) .posRangeToIndexRange(fromPos to toPos) - _textOpInfos.emit(actorID to OperationInfo.SelectOpInfo(from, to)) + _textOperationInfos.emit(actorID to OperationInfo.SelectOpInfo(from, to)) } fun syncText() { diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/operation/OperationInfo.kt b/yorkie/src/main/kotlin/dev/yorkie/document/operation/OperationInfo.kt index dfea2d74f..273e9a2a7 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/operation/OperationInfo.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/operation/OperationInfo.kt @@ -2,6 +2,11 @@ package dev.yorkie.document.operation import dev.yorkie.document.crdt.TextWithAttributes import dev.yorkie.document.crdt.TreeNode +import dev.yorkie.document.json.JsonArray +import dev.yorkie.document.json.JsonCounter +import dev.yorkie.document.json.JsonObject +import dev.yorkie.document.json.JsonText +import dev.yorkie.document.json.JsonTree import dev.yorkie.document.time.TimeTicket /** @@ -14,48 +19,71 @@ public sealed class OperationInfo { internal var executedAt: TimeTicket = TimeTicket.InitialTimeTicket - public interface TextOpInfo + /** + * [TextOperationInfo] represents the [OperationInfo] for the [JsonText]. + */ + public interface TextOperationInfo + + /** + * [CounterOperationInfo] represents the [OperationInfo] for the [JsonCounter]. + */ + public interface CounterOperationInfo + + /** + * [ArrayOperationInfo] represents the [OperationInfo] for the [JsonArray]. + */ + public interface ArrayOperationInfo + + /** + * [ObjectOperationInfo] represents the [OperationInfo] for the [JsonObject]. + */ + public interface ObjectOperationInfo + + /** + * [TreeOperationInfo] represents the [OperationInfo] for the [JsonTree]. + */ + public interface TreeOperationInfo public data class AddOpInfo(val index: Int, override var path: String = INITIAL_PATH) : - OperationInfo() + OperationInfo(), ArrayOperationInfo public data class MoveOpInfo( val previousIndex: Int, val index: Int, override var path: String = INITIAL_PATH, - ) : OperationInfo() + ) : OperationInfo(), ArrayOperationInfo public data class SetOpInfo(val key: String, override var path: String = INITIAL_PATH) : - OperationInfo() + OperationInfo(), ObjectOperationInfo public data class RemoveOpInfo( val key: String?, val index: Int?, override var path: String = INITIAL_PATH, - ) : OperationInfo() + ) : OperationInfo(), ArrayOperationInfo, ObjectOperationInfo public data class IncreaseOpInfo(val value: Number, override var path: String = INITIAL_PATH) : - OperationInfo() + OperationInfo(), CounterOperationInfo public data class EditOpInfo( val from: Int, val to: Int, val value: TextWithAttributes, override var path: String = INITIAL_PATH, - ) : OperationInfo(), TextOpInfo + ) : OperationInfo(), TextOperationInfo public data class StyleOpInfo( val from: Int, val to: Int, val attributes: Map, override var path: String = INITIAL_PATH, - ) : OperationInfo(), TextOpInfo + ) : OperationInfo(), TextOperationInfo public data class SelectOpInfo( val from: Int, val to: Int, override var path: String = INITIAL_PATH, - ) : OperationInfo(), TextOpInfo + ) : OperationInfo(), TextOperationInfo public data class TreeEditOpInfo( val from: Int, @@ -64,7 +92,7 @@ public sealed class OperationInfo { val toPath: List, val nodes: List?, override var path: String = INITIAL_PATH, - ) : OperationInfo() + ) : OperationInfo(), TreeOperationInfo public data class TreeStyleOpInfo( val from: Int, @@ -73,7 +101,7 @@ public sealed class OperationInfo { val toPath: List, val attributes: Map, override var path: String = INITIAL_PATH, - ) : OperationInfo() + ) : OperationInfo(), TreeOperationInfo companion object { private const val INITIAL_PATH = "initial path" From ae39f43d17789cb04a205a9afb761db73dcc9c29 Mon Sep 17 00:00:00 2001 From: jee-hyun-kim Date: Mon, 21 Aug 2023 19:31:09 +0900 Subject: [PATCH 03/13] documents -> document --- yorkie/proto/src/main/proto/yorkie/v1/resources.proto | 6 +++--- .../androidTest/kotlin/dev/yorkie/core/ClientTest.kt | 6 +++--- yorkie/src/main/kotlin/dev/yorkie/core/Client.kt | 10 +++++----- yorkie/src/main/kotlin/dev/yorkie/document/Document.kt | 6 +++--- yorkie/src/test/kotlin/dev/yorkie/core/ClientTest.kt | 6 +++--- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/yorkie/proto/src/main/proto/yorkie/v1/resources.proto b/yorkie/proto/src/main/proto/yorkie/v1/resources.proto index 46d879aad..15e3658c8 100644 --- a/yorkie/proto/src/main/proto/yorkie/v1/resources.proto +++ b/yorkie/proto/src/main/proto/yorkie/v1/resources.proto @@ -343,9 +343,9 @@ enum ValueType { } enum DocEventType { - DOC_EVENT_TYPE_DOCUMENTS_CHANGED = 0; - DOC_EVENT_TYPE_DOCUMENTS_WATCHED = 1; - DOC_EVENT_TYPE_DOCUMENTS_UNWATCHED = 2; + DOC_EVENT_TYPE_DOCUMENT_CHANGED = 0; + DOC_EVENT_TYPE_DOCUMENT_WATCHED = 1; + DOC_EVENT_TYPE_DOCUMENT_UNWATCHED = 2; } message DocEvent { diff --git a/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt b/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt index 4c296bb87..6bc93be34 100644 --- a/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt +++ b/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt @@ -4,7 +4,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import dev.yorkie.assertJsonContentEquals import dev.yorkie.core.Client.DocumentSyncResult import dev.yorkie.core.Client.Event.DocumentSynced -import dev.yorkie.core.Client.Event.DocumentsChanged +import dev.yorkie.core.Client.Event.DocumentChanged import dev.yorkie.core.Client.StreamConnectionStatus import dev.yorkie.document.Document import dev.yorkie.document.Document.Event.LocalChange @@ -89,8 +89,8 @@ class ClientTest { delay(50) } } - val changeEvent = assertIs( - client2Events.first { it is DocumentsChanged }, + val changeEvent = assertIs( + client2Events.first { it is DocumentChanged }, ) assertContentEquals(listOf(documentKey), changeEvent.documentKeys) var syncEvent = assertIs(client2Events.first { it is DocumentSynced }) diff --git a/yorkie/src/main/kotlin/dev/yorkie/core/Client.kt b/yorkie/src/main/kotlin/dev/yorkie/core/Client.kt index 7d4d80587..fbdc5a9d3 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/core/Client.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/core/Client.kt @@ -300,7 +300,7 @@ public class Client @VisibleForTesting internal constructor( val publisher = watchEvent.publisher.toActorID() when (eventType) { - DocEventType.DOC_EVENT_TYPE_DOCUMENTS_WATCHED -> { + DocEventType.DOC_EVENT_TYPE_DOCUMENT_WATCHED -> { // NOTE(chacha912): We added to onlineClients, but we won't trigger watched event // unless we also know their initial presence data at this point. document.onlineClients.value = document.onlineClients.value + publisher @@ -312,7 +312,7 @@ public class Client @VisibleForTesting internal constructor( } } - DocEventType.DOC_EVENT_TYPE_DOCUMENTS_UNWATCHED -> { + DocEventType.DOC_EVENT_TYPE_DOCUMENT_UNWATCHED -> { // NOTE(chacha912): There is no presence, // when PresenceChange(clear) is applied before unwatching. In that case, // the 'unwatched' event is triggered while handling the PresenceChange. @@ -321,11 +321,11 @@ public class Client @VisibleForTesting internal constructor( document.publish(PresenceChange.Others.Unwatched(PresenceInfo(publisher, presence))) } - DocEventType.DOC_EVENT_TYPE_DOCUMENTS_CHANGED -> { + DocEventType.DOC_EVENT_TYPE_DOCUMENT_CHANGED -> { attachments.value += documentKey to attachment.copy( remoteChangeEventReceived = true, ) - eventStream.emit(Event.DocumentsChanged(listOf(documentKey))) + eventStream.emit(Event.DocumentChanged(listOf(documentKey))) } DocEventType.UNRECOGNIZED -> { @@ -650,7 +650,7 @@ public class Client @VisibleForTesting internal constructor( /** * Means that the documents of the client has changed. */ - public class DocumentsChanged(public val documentKeys: List) : Event + public class DocumentChanged(public val documentKeys: List) : Event /** * Means that the document has synced with the server. diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt b/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt index 284dce4d7..8e8e0230e 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt @@ -264,10 +264,10 @@ public class Document(public val key: Key) { this.clone = clone.copy(presences = newPresences ?: return@also) } val actorID = change.id.actor - var event: Event? = null + var presenceEvent: Event.PresenceChange? = null if (change.hasPresenceChange && actorID in onlineClients.value) { val presenceChange = change.presenceChange ?: return@forEach - event = when (presenceChange) { + presenceEvent = when (presenceChange) { is PresenceChange.Put -> { if (actorID in _presences.value) { createPresenceChangedEvent(actorID, presenceChange.presence) @@ -299,7 +299,7 @@ public class Document(public val key: Key) { eventStream.emit(Event.RemoteChange(change.toChangeInfo(opInfos))) } - event?.let { eventStream.emit(it) } + presenceEvent?.let { eventStream.emit(it) } newPresences?.let { emitPresences(it) } changeID = changeID.syncLamport(change.id.lamport) } diff --git a/yorkie/src/test/kotlin/dev/yorkie/core/ClientTest.kt b/yorkie/src/test/kotlin/dev/yorkie/core/ClientTest.kt index 2016995a4..67720bee2 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/core/ClientTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/core/ClientTest.kt @@ -14,7 +14,7 @@ import dev.yorkie.api.v1.WatchDocumentRequest import dev.yorkie.api.v1.YorkieServiceGrpcKt import dev.yorkie.assertJsonContentEquals import dev.yorkie.core.Client.Event.DocumentSynced -import dev.yorkie.core.Client.Event.DocumentsChanged +import dev.yorkie.core.Client.Event.DocumentChanged import dev.yorkie.core.MockYorkieService.Companion.ATTACH_ERROR_DOCUMENT_KEY import dev.yorkie.core.MockYorkieService.Companion.DETACH_ERROR_DOCUMENT_KEY import dev.yorkie.core.MockYorkieService.Companion.NORMAL_DOCUMENT_KEY @@ -138,8 +138,8 @@ class ClientTest { val watchRequestCaptor = argumentCaptor() target.attachAsync(document).await() - val event = target.events.first { it is DocumentsChanged } - val changeEvent = assertIs(event) + val event = target.events.first { it is DocumentChanged } + val changeEvent = assertIs(event) verify(service, atLeastOnce()).watchDocument(watchRequestCaptor.capture()) assertIsTestActorID(watchRequestCaptor.firstValue.clientId) assertEquals(1, changeEvent.documentKeys.size) From e5665faf699ddd5e49b955dd99aca2e9e9577ea4 Mon Sep 17 00:00:00 2001 From: jee-hyun-kim Date: Tue, 22 Aug 2023 15:58:47 +0900 Subject: [PATCH 04/13] Concurrent case handling for Yorkie.tree --- docker/docker-compose-ci.yml | 2 +- docker/docker-compose.yml | 2 +- .../src/main/proto/yorkie/v1/resources.proto | 22 +- .../src/main/proto/yorkie/v1/yorkie.proto | 30 +- .../kotlin/dev/yorkie/api/ElementConverter.kt | 37 +- .../dev/yorkie/api/OperationConverter.kt | 9 + .../src/main/kotlin/dev/yorkie/core/Client.kt | 31 +- .../yorkie/document/JsonSerializableStruct.kt | 21 +- .../dev/yorkie/document/crdt/CrdtTree.kt | 566 +++++++++--------- .../dev/yorkie/document/json/JsonTree.kt | 67 +-- .../document/operation/TreeEditOperation.kt | 11 +- .../main/kotlin/dev/yorkie/util/IndexTree.kt | 39 +- .../kotlin/dev/yorkie/api/ConverterTest.kt | 24 +- .../test/kotlin/dev/yorkie/core/ClientTest.kt | 10 +- .../dev/yorkie/core/MockYorkieService.kt | 23 +- .../dev/yorkie/document/crdt/CrdtTreeTest.kt | 235 ++++---- .../dev/yorkie/document/json/JsonTreeTest.kt | 50 +- .../kotlin/dev/yorkie/util/IndexTreeTest.kt | 28 +- 18 files changed, 580 insertions(+), 627 deletions(-) diff --git a/docker/docker-compose-ci.yml b/docker/docker-compose-ci.yml index 4b2366f0b..edc186b7b 100644 --- a/docker/docker-compose-ci.yml +++ b/docker/docker-compose-ci.yml @@ -15,7 +15,7 @@ services: depends_on: - yorkie yorkie: - image: 'yorkieteam/yorkie:0.4.5' + image: 'yorkieteam/yorkie:0.4.5-alpha' container_name: 'yorkie' command: [ 'server', diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index 2f5c71af8..900e695c5 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -22,7 +22,7 @@ services: extra_hosts: - "host.docker.internal:host-gateway" yorkie: - image: 'yorkieteam/yorkie:0.4.5' + image: 'yorkieteam/yorkie:0.4.5-alpha' container_name: 'yorkie' command: [ 'server', diff --git a/yorkie/proto/src/main/proto/yorkie/v1/resources.proto b/yorkie/proto/src/main/proto/yorkie/v1/resources.proto index 15e3658c8..fd09dd601 100644 --- a/yorkie/proto/src/main/proto/yorkie/v1/resources.proto +++ b/yorkie/proto/src/main/proto/yorkie/v1/resources.proto @@ -95,6 +95,10 @@ message Operation { TimeTicket executed_at = 6; map attributes = 7; } + // NOTE(hackerwins): Select Operation is not used in the current version. + // In the previous version, it was used to represent selection of Text. + // However, it has been replaced by Presence now. It is retained for backward + // compatibility purposes. message Select { TimeTicket parent_created_at = 1; TextNodePos from = 2; @@ -117,8 +121,9 @@ message Operation { TimeTicket parent_created_at = 1; TreePos from = 2; TreePos to = 3; - repeated TreeNodes contents = 4; - TimeTicket executed_at = 5; + map created_at_map_by_actor = 4; + repeated TreeNodes contents = 5; + TimeTicket executed_at = 6; } message TreeStyle { TimeTicket parent_created_at = 1; @@ -233,11 +238,11 @@ message TextNodeID { } message TreeNode { - TreePos pos = 1; + TreeNodeID id = 1; string type = 2; string value = 3; TimeTicket removed_at = 4; - TreePos ins_prev_pos = 5; + TreeNodeID ins_prev_id = 5; int32 depth = 6; map attributes = 7; } @@ -246,11 +251,16 @@ message TreeNodes { repeated TreeNode content = 1; } -message TreePos { +message TreeNodeID { TimeTicket created_at = 1; int32 offset = 2; } +message TreePos { + TreeNodeID parent_id = 1; + TreeNodeID left_sibling_id = 2; +} + ///////////////////////////////////////// // Messages for Common // ///////////////////////////////////////// @@ -350,5 +360,5 @@ enum DocEventType { message DocEvent { DocEventType type = 1; - bytes publisher = 2; + string publisher = 2; } diff --git a/yorkie/proto/src/main/proto/yorkie/v1/yorkie.proto b/yorkie/proto/src/main/proto/yorkie/v1/yorkie.proto index b4f0eda6a..639ddd37d 100644 --- a/yorkie/proto/src/main/proto/yorkie/v1/yorkie.proto +++ b/yorkie/proto/src/main/proto/yorkie/v1/yorkie.proto @@ -42,49 +42,45 @@ message ActivateClientRequest { } message ActivateClientResponse { - string client_key = 1; - bytes client_id = 2; + string client_id = 1; } message DeactivateClientRequest { - bytes client_id = 1; + string client_id = 1; } message DeactivateClientResponse { - bytes client_id = 1; } message AttachDocumentRequest { - bytes client_id = 1; + string client_id = 1; ChangePack change_pack = 2; } message AttachDocumentResponse { - bytes client_id = 1; - string document_id = 2; - ChangePack change_pack = 3; + string document_id = 1; + ChangePack change_pack = 2; } message DetachDocumentRequest { - bytes client_id = 1; + string client_id = 1; string document_id = 2; ChangePack change_pack = 3; bool remove_if_not_attached = 4; } message DetachDocumentResponse { - string client_key = 1; ChangePack change_pack = 2; } message WatchDocumentRequest { - bytes client_id = 1; + string client_id = 1; string document_id = 2; } message WatchDocumentResponse { message Initialization { - repeated bytes client_ids = 1; + repeated string client_ids = 1; } oneof body { @@ -94,24 +90,22 @@ message WatchDocumentResponse { } message RemoveDocumentRequest { - bytes client_id = 1; + string client_id = 1; string document_id = 2; ChangePack change_pack = 3; } message RemoveDocumentResponse { - string client_key = 1; - ChangePack change_pack = 2; + ChangePack change_pack = 1; } message PushPullChangesRequest { - bytes client_id = 1; + string client_id = 1; string document_id = 2; ChangePack change_pack = 3; bool push_only = 4; } message PushPullChangesResponse { - bytes client_id = 1; - ChangePack change_pack = 2; + ChangePack change_pack = 1; } diff --git a/yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt b/yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt index 1cc546047..46563de8b 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt @@ -19,6 +19,7 @@ import dev.yorkie.api.v1.textNode import dev.yorkie.api.v1.textNodeID import dev.yorkie.api.v1.textNodePos import dev.yorkie.api.v1.treeNode +import dev.yorkie.api.v1.treeNodeID import dev.yorkie.api.v1.treeNodes import dev.yorkie.api.v1.treePos import dev.yorkie.core.P @@ -34,6 +35,7 @@ import dev.yorkie.document.crdt.CrdtTree import dev.yorkie.document.crdt.CrdtTreeNode import dev.yorkie.document.crdt.CrdtTreeNode.Companion.CrdtTreeElement import dev.yorkie.document.crdt.CrdtTreeNode.Companion.CrdtTreeText +import dev.yorkie.document.crdt.CrdtTreeNodeID import dev.yorkie.document.crdt.CrdtTreePos import dev.yorkie.document.crdt.ElementRht import dev.yorkie.document.crdt.RgaTreeList @@ -64,6 +66,7 @@ internal typealias PBTextNode = dev.yorkie.api.v1.TextNode internal typealias PBTree = dev.yorkie.api.v1.JSONElement.Tree internal typealias PBTreeNode = dev.yorkie.api.v1.TreeNode internal typealias PBTreePos = dev.yorkie.api.v1.TreePos +internal typealias PBTreeNodeID = dev.yorkie.api.v1.TreeNodeID internal typealias PBTreeNodes = dev.yorkie.api.v1.TreeNodes internal typealias PBSnapshot = dev.yorkie.api.v1.Snapshot @@ -225,12 +228,13 @@ internal fun List.toCrdtTreeRootNode(): CrdtTreeNode? { } internal fun PBTreeNode.toCrdtTreeNode(): CrdtTreeNode { - val pos = pos.toCrdtTreePos() + val id = id.toCrdtTreeNodeID() + val convertedRemovedAt = removedAt.toTimeTicket() return if (type == IndexTreeNode.DEFAULT_TEXT_TYPE) { - CrdtTreeText(pos, value) + CrdtTreeText(id, value) } else { CrdtTreeElement( - pos, + id, type, attributes = Rht().also { attributesMap.forEach { (key, value) -> @@ -238,10 +242,18 @@ internal fun PBTreeNode.toCrdtTreeNode(): CrdtTreeNode { } }, ) + }.apply { + remove(convertedRemovedAt) } } -internal fun PBTreePos.toCrdtTreePos(): CrdtTreePos = CrdtTreePos(createdAt.toTimeTicket(), offset) +internal fun PBTreePos.toCrdtTreePos(): CrdtTreePos { + return CrdtTreePos(parentId.toCrdtTreeNodeID(), leftSiblingId.toCrdtTreeNodeID()) +} + +internal fun PBTreeNodeID.toCrdtTreeNodeID(): CrdtTreeNodeID { + return CrdtTreeNodeID(createdAt.toTimeTicket(), offset) +} internal fun CrdtElement.toPBJsonElement(): PBJsonElement { return when (this) { @@ -295,10 +307,11 @@ internal fun RgaTreeList.toPBRgaNodes(): List { } internal fun CrdtTreeNode.toPBTreeNodes(): List { + val treeNode = this return buildList { - traverse(this@toPBTreeNodes) { node, nodeDepth -> + traverse(treeNode) { node, nodeDepth -> val pbTreeNode = treeNode { - pos = node.pos.toPBTreePos() + id = node.id.toPBTreeNodeID() type = node.type if (node.isText) { value = node.value @@ -336,8 +349,16 @@ internal fun List.toCrdtTreeNodesWhenEdit(): List? { internal fun CrdtTreePos.toPBTreePos(): PBTreePos { val crdtTreePos = this return treePos { - createdAt = crdtTreePos.createdAt.toPBTimeTicket() - offset = crdtTreePos.offset + parentId = crdtTreePos.parentID.toPBTreeNodeID() + leftSiblingId = crdtTreePos.leftSiblingID.toPBTreeNodeID() + } +} + +internal fun CrdtTreeNodeID.toPBTreeNodeID(): PBTreeNodeID { + val nodeID = this + return treeNodeID { + createdAt = nodeID.createdAt.toPBTimeTicket() + offset = nodeID.offset } } diff --git a/yorkie/src/main/kotlin/dev/yorkie/api/OperationConverter.kt b/yorkie/src/main/kotlin/dev/yorkie/api/OperationConverter.kt index aba0f2d40..63f343eb5 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/api/OperationConverter.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/api/OperationConverter.kt @@ -98,6 +98,10 @@ internal fun List.toOperations(): List { toPos = it.treeEdit.to.toCrdtTreePos(), contents = it.treeEdit.contentsList.toCrdtTreeNodesWhenEdit(), executedAt = it.treeEdit.executedAt.toTimeTicket(), + maxCreatedAtMapByActor = + it.treeEdit.createdAtMapByActorMap.entries.associate { (key, value) -> + ActorID(key) to value.toTimeTicket() + }, ) it.hasTreeStyle() -> TreeStyleOperation( @@ -215,6 +219,11 @@ internal fun Operation.toPBOperation(): PBOperation { to = operation.toPos.toPBTreePos() executedAt = operation.executedAt.toPBTimeTicket() contents.addAll(operation.contents?.toPBTreeNodesWhenEdit().orEmpty()) + createdAtMapByActor.putAll( + operation.maxCreatedAtMapByActor.entries.associate { + it.key.value to it.value.toPBTimeTicket() + }, + ) } } } diff --git a/yorkie/src/main/kotlin/dev/yorkie/core/Client.kt b/yorkie/src/main/kotlin/dev/yorkie/core/Client.kt index fbdc5a9d3..e3c66f07c 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/core/Client.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/core/Client.kt @@ -2,9 +2,6 @@ package dev.yorkie.core import android.content.Context import com.google.common.annotations.VisibleForTesting -import com.google.protobuf.ByteString -import dev.yorkie.api.toActorID -import dev.yorkie.api.toByteString import dev.yorkie.api.toChangePack import dev.yorkie.api.toPBChangePack import dev.yorkie.api.v1.DocEventType @@ -136,12 +133,7 @@ public class Client @VisibleForTesting internal constructor( YorkieLogger.e("Client.activate", e.stackTraceToString()) return@async false } - _status.emit( - Status.Activated( - activateResponse.clientId.toActorID(), - activateResponse.clientKey, - ), - ) + _status.emit(Status.Activated(ActorID(activateResponse.clientId))) runSyncLoop() runWatchLoop() true @@ -211,7 +203,7 @@ public class Client @VisibleForTesting internal constructor( document, runCatching { val request = pushPullChangesRequest { - clientId = requireClientId().toByteString() + clientId = requireClientId().value changePack = document.createChangePack().toPBChangePack() documentId = documentID pushOnly = syncMode == SyncMode.PushOnly @@ -263,7 +255,7 @@ public class Client @VisibleForTesting internal constructor( return scope.launch(activationJob) { service.watchDocument( watchDocumentRequest { - clientId = requireClientId().toByteString() + clientId = requireClientId().value documentId = attachment.documentID }, documentBasedRequestHeader(attachment.document.key), @@ -284,7 +276,7 @@ public class Client @VisibleForTesting internal constructor( ) { if (response.hasInitialization()) { val document = attachments.value[documentKey]?.document ?: return - val clientIDs = response.initialization.clientIdsList.map(ByteString::toActorID) + val clientIDs = response.initialization.clientIdsList.map { ActorID(it) } document.onlineClients.value = document.onlineClients.value + clientIDs document.publish( PresenceChange.MyPresence.Initialized(document.presences.value.asPresences()), @@ -297,7 +289,7 @@ public class Client @VisibleForTesting internal constructor( // only single key will be received since 0.3.1 server. val attachment = attachments.value[documentKey] ?: return val document = attachment.document - val publisher = watchEvent.publisher.toActorID() + val publisher = ActorID(watchEvent.publisher) when (eventType) { DocEventType.DOC_EVENT_TYPE_DOCUMENT_WATCHED -> { @@ -356,7 +348,7 @@ public class Client @VisibleForTesting internal constructor( }.await() val request = attachDocumentRequest { - clientId = requireClientId().toByteString() + clientId = requireClientId().value changePack = document.createChangePack().toPBChangePack() } val response = try { @@ -407,7 +399,7 @@ public class Client @VisibleForTesting internal constructor( }.await() val request = detachDocumentRequest { - clientId = requireClientId().toByteString() + clientId = requireClientId().value changePack = document.createChangePack().toPBChangePack() documentId = attachment.documentID } @@ -444,7 +436,7 @@ public class Client @VisibleForTesting internal constructor( try { service.deactivateClient( deactivateClientRequest { - clientId = requireClientId().toByteString() + clientId = requireClientId().value }, projectBasedRequestHeader, ) @@ -469,7 +461,7 @@ public class Client @VisibleForTesting internal constructor( ?: throw IllegalArgumentException("document is not attached") val request = removeDocumentRequest { - clientId = requireClientId().toByteString() + clientId = requireClientId().value changePack = document.createChangePack(forceRemove = true).toPBChangePack() documentId = attachment.documentID } @@ -565,10 +557,7 @@ public class Client @VisibleForTesting internal constructor( * Means that the client is activated. If the client is activated, * all [Document]s of the client are ready to be used. */ - public class Activated internal constructor( - public val clientId: ActorID, - public val clientKey: String, - ) : Status + public class Activated internal constructor(public val clientId: ActorID) : Status /** * Means that the client is not activated. It is the initial stastus of the client. diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/JsonSerializableStruct.kt b/yorkie/src/main/kotlin/dev/yorkie/document/JsonSerializableStruct.kt index 1afb5a88a..41170b644 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/JsonSerializableStruct.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/JsonSerializableStruct.kt @@ -1,5 +1,6 @@ package dev.yorkie.document +import dev.yorkie.document.crdt.CrdtTreeNodeID import dev.yorkie.document.crdt.CrdtTreePos import dev.yorkie.document.crdt.RgaTreeSplitNodeID import dev.yorkie.document.crdt.RgaTreeSplitPos @@ -16,17 +17,27 @@ internal interface JsonSerializable> { fun toStruct(): O } +public data class CrdtTreePosStruct( + val parentID: CrdtTreeNodeIDStruct, + val leftSiblingID: CrdtTreeNodeIDStruct, +) : JsonSerializable.Struct { + + override fun toOriginal(): CrdtTreePos { + return CrdtTreePos(parentID.toOriginal(), leftSiblingID.toOriginal()) + } +} + /** - * [CrdtTreePosStruct] represents the structure of [CrdtTreePos]. + * [CrdtTreeNodeIDStruct] represents the structure of [CrdtTreeNodeID]. * It is used to serialize and deserialize the CRDTTreePos. */ -public data class CrdtTreePosStruct( +public data class CrdtTreeNodeIDStruct( val createdAt: TimeTicketStruct, val offset: Int, -) : JsonSerializable.Struct { +) : JsonSerializable.Struct { - override fun toOriginal(): CrdtTreePos { - return CrdtTreePos(createdAt.toOriginal(), offset) + override fun toOriginal(): CrdtTreeNodeID { + return CrdtTreeNodeID(createdAt.toOriginal(), offset) } } diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt index a16c10789..5d94a7956 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt @@ -1,16 +1,17 @@ package dev.yorkie.document.crdt +import dev.yorkie.document.CrdtTreeNodeIDStruct import dev.yorkie.document.CrdtTreePosStruct import dev.yorkie.document.JsonSerializable -import dev.yorkie.document.crdt.CrdtTreeNode.Companion.CrdtTreeElement import dev.yorkie.document.json.TreePosStructRange +import dev.yorkie.document.time.ActorID import dev.yorkie.document.time.TimeTicket import dev.yorkie.document.time.TimeTicket.Companion.InitialTimeTicket +import dev.yorkie.document.time.TimeTicket.Companion.MaxTimeTicket import dev.yorkie.document.time.TimeTicket.Companion.compareTo import dev.yorkie.util.IndexTree import dev.yorkie.util.IndexTreeNode import dev.yorkie.util.TreePos -import dev.yorkie.util.traverse import java.util.TreeMap public typealias TreePosRange = Pair @@ -20,49 +21,27 @@ internal class CrdtTree( override val createdAt: TimeTicket, override var _movedAt: TimeTicket? = null, override var _removedAt: TimeTicket? = null, -) : CrdtGCElement(), Collection { - - private val head = CrdtTreeElement(CrdtTreePos.InitialCrdtTreePos, INITIAL_NODE_TYPE) +) : CrdtGCElement() { internal val indexTree = IndexTree(root) - private val nodeMapByPos = TreeMap() + private val nodeMapByID = TreeMap() private val removedNodeMap = mutableMapOf, CrdtTreeNode>() + init { - var previous = head indexTree.traverse { node, _ -> - insertAfter(previous, node) - previous = node + nodeMapByID[node.id] = node } } override val removedNodesLength: Int get() = removedNodeMap.size - override val size: Int + val size: Int get() = indexTree.size - /** - * Returns the nodes between the given range. - */ - fun nodesBetweenByTree( - from: Int, - to: Int, - action: ((CrdtTreeNode) -> Unit), - ) { - indexTree.nodesBetween(from, to, action) - } - - /** - * Finds the right node of the given [index] in postorder. - */ - fun findPostorderRight(index: Int): CrdtTreeNode? { - val pos = indexTree.findTreePos(index, true) - return indexTree.findPostorderRight(pos) - } - /** * Applies the given [attributes] of the given [range]. */ @@ -71,24 +50,37 @@ internal class CrdtTree( attributes: Map?, executedAt: TimeTicket, ): List { - val (_, toRight) = findTreePos(range.second, executedAt) - val (_, fromRight) = findTreePos(range.first, executedAt) + val (fromParent, fromLeft) = findNodesAndSplitText(range.first, executedAt) + val (toParent, toLeft) = findNodesAndSplitText(range.second, executedAt) + // TODO(7hong13): check whether toPath is set correctly val changes = listOf( TreeChange( type = TreeChangeType.Style.type, - from = toIndex(range.first), - to = toIndex(range.second), - fromPath = indexTree.indexToPath(posToStartIndex(range.first)), - toPath = indexTree.indexToPath(posToStartIndex(range.first)), + from = toIndex(fromParent, fromLeft), + to = toIndex(toParent, toLeft), + fromPath = toPath(fromParent, fromLeft), + toPath = toPath(fromParent, fromLeft), actorID = executedAt.actorID, attributes = attributes, ), ) - nodesBetween(fromRight, toRight) { node -> - if (!node.isRemoved && attributes != null) { - attributes.forEach { (key, value) -> - node.setAttribute(key, value, executedAt) + if (fromLeft != toLeft) { + val (parent, fromChildIndex) = if (fromLeft.parent == toLeft.parent) { + val leftParent = fromLeft.parent ?: return changes + leftParent to leftParent.allChildren.indexOf(fromLeft) + 1 + } else { + fromLeft to 0 + } + + val toChildIndex = parent.allChildren.indexOf(toLeft) + + for (i in fromChildIndex..toChildIndex) { + val node = parent.allChildren[i] + if (!node.isRemoved && attributes != null) { + attributes.forEach { (key, value) -> + node.setAttribute(key, value, executedAt) + } } } } @@ -96,43 +88,40 @@ internal class CrdtTree( return changes } - /** - * Finds [TreePos] of the given [CrdtTreePos]. - */ - private fun findTreePos( - pos: CrdtTreePos, - executedAt: TimeTicket, - ): Pair, CrdtTreeNode> { - val treePos = toTreePos(pos) ?: throw IllegalArgumentException("cannot find node at $pos") + private fun toPath(parentNode: CrdtTreeNode, leftSiblingNode: CrdtTreeNode): List { + return indexTree.treePosToPath(toTreePos(parentNode, leftSiblingNode)) + } - // Find the appropriate position. This logic is similar to - // handling the insertion of the same position in RGA. - var current = treePos - while (executedAt < current.node.next?.pos?.createdAt && - current.node.parent == current.node.next?.parent - ) { - val nextNode = current.node.next ?: break - current = TreePos(nextNode, nextNode.size) - } - val right = requireNotNull(indexTree.findPostorderRight(treePos)) - return current to right - } - - private fun toTreePos(pos: CrdtTreePos): TreePos? { - val (key, value) = nodeMapByPos.floorEntry(pos) ?: return null - return if (key?.createdAt == pos.createdAt) { - val node = - // Choose the left node if the position is on the boundary of the split nodes. - if (pos.offset > 0 && pos.offset == value.pos.offset && - value.insertionPrev != null - ) { - value.insertionPrev - } else { - value + private fun toTreePos( + parentNode: CrdtTreeNode, + leftSiblingNode: CrdtTreeNode, + ): TreePos { + return when { + parentNode.isRemoved -> { + var child = parentNode + var parent = parentNode + while (parent.isRemoved) { + child = parent + parent = child.parent ?: break } - TreePos(requireNotNull(node), pos.offset - node.pos.offset) - } else { - null + + val childOffset = parent.findOffset(child) + TreePos(parent, childOffset) + } + + parentNode == leftSiblingNode -> TreePos(parentNode, 0) + + else -> { + var offset = parentNode.findOffset(leftSiblingNode) + if (!leftSiblingNode.isRemoved) { + if (leftSiblingNode.isText) { + return TreePos(leftSiblingNode, leftSiblingNode.paddedSize) + } else { + offset++ + } + } + TreePos(parentNode, offset) + } } } @@ -144,88 +133,118 @@ internal class CrdtTree( range: TreePosRange, contents: List?, executedAt: TimeTicket, - ): List { + latestCreatedAtMapByActor: Map? = null, + ): Pair, Map> { // 01. split text nodes at the given range if needed. - val (toPos, toRight) = findTreePosWithSplitText(range.second, executedAt) - val (fromPos, fromRight) = findTreePosWithSplitText(range.first, executedAt) + val (fromParent, fromLeft) = findNodesAndSplitText(range.first, executedAt) + val (toParent, toLeft) = findNodesAndSplitText(range.second, executedAt) // NOTE(hackerwins): If concurrent deletion happens, we need to separate the // range(from, to) into multiple ranges. val changes = listOf( TreeChange( type = TreeChangeType.Content.type, - from = toIndex(range.first), - to = toIndex(range.second), - fromPath = indexTree.treePosToPath(fromPos), - toPath = indexTree.treePosToPath(toPos), + from = toIndex(fromParent, fromLeft), + to = toIndex(toParent, toLeft), + fromPath = toPath(fromParent, fromLeft), + toPath = toPath(toParent, toLeft), actorID = executedAt.actorID, value = contents?.map(CrdtTreeNode::toJson), ), ) val toBeRemoved = mutableListOf() + val latestCreatedAtMap = mutableMapOf() + // 02. remove the nodes and update linked list and index tree. - if (fromRight != toRight) { - nodesBetween(fromRight, toRight) { node -> - if (!node.isRemoved) { - toBeRemoved.add(node) - } + if (fromLeft != toLeft) { + val (parent, fromChildIndex) = if (fromLeft.parent == toLeft.parent) { + val leftParent = requireNotNull(fromLeft.parent) + leftParent to leftParent.allChildren.indexOf(fromLeft) + 1 + } else { + fromLeft to 0 } - val isRangeOnSameBranch = toPos.node.isAncestorOf(fromPos.node) - toBeRemoved.forEach { node -> - node.remove(executedAt) - if (node.isRemoved) { - removedNodeMap[node.createdAt to node.pos.offset] = node - } - } + val toChildIndex = parent.allChildren.indexOf(toLeft) - // move the alive children of the removed element node - if (isRangeOnSameBranch) { - val removedElementNode = when { - fromPos.node.parent?.isRemoved == true -> fromPos.node.parent - !fromPos.node.isText && fromPos.node.isRemoved -> fromPos.node - else -> null - } - removedElementNode?.let { removedNode -> - val elementNode = toPos.node - val offset = elementNode.findBranchOffset(removedNode) - removedNode.children.reversed().forEach { node -> - elementNode.insertAt(offset, node) + for (i in fromChildIndex..toChildIndex) { + val node = parent.allChildren[i] + val actorID = node.createdAt.actorID + val latestCreatedAt = latestCreatedAtMapByActor?.let { + latestCreatedAtMapByActor[actorID] ?: InitialTimeTicket + } ?: MaxTimeTicket + + if (node.canDelete(executedAt, latestCreatedAt)) { + val latest = latestCreatedAtMap[actorID] + val createdAt = node.createdAt + + if (latest == null || latest < createdAt) { + latestCreatedAtMap[actorID] = createdAt + } + + traverseAll(node, 0) { treeNode, _ -> + if (treeNode.canDelete(executedAt, MaxTimeTicket)) { + val latestTicket = latestCreatedAtMap[actorID] + val ticket = treeNode.createdAt + + if (latestTicket == null || latestTicket < ticket) { + latestCreatedAtMap[actorID] = ticket + } + + if (!treeNode.isRemoved) { + toBeRemoved.add(treeNode) + } + } } } - } else if (fromPos.node.parent?.isRemoved == true) { - val parent = requireNotNull(fromPos.node.parent) - toPos.node.parent?.prepend(*parent.children.toTypedArray()) + } + } + + toBeRemoved.forEach { node -> + node.remove(executedAt) + if (node.isRemoved) { + removedNodeMap[node.createdAt to node.id.offset] = node } } // 03. insert the given node at the given position. if (contents?.isNotEmpty() == true) { - // 03-1. insert the content nodes to the list. - var previous = requireNotNull(fromRight.prev) - var offset = fromPos.offset + var leftInChildren = fromLeft + contents.forEach { content -> - traverse(content) { node, _ -> - insertAfter(previous, node) - previous = node + // 03-1. insert the content nodes to the list. + if (leftInChildren == fromParent) { + // 03-1-1. when there's no leftSibling, then insert content into very front of parent's children List + fromParent.insertAt(0, content) + } else { + // 03-1-2. insert after leftSibling + fromParent.insertAfter(leftInChildren, content) } - // 03-2. insert the content nodes to the tree. - val node = fromPos.node - if (node.isText) { - if (fromPos.offset == 0) { - node.parent?.insertBefore(node, content) - } else { - node.parent?.insertAfter(node, content) + leftInChildren = content + traverseAll(content) { node, _ -> + // if insertion happens during concurrent editing and parent node has been removed, + // make new nodes as tombstone immediately + if (fromParent.isRemoved) { + node.remove(executedAt) + removedNodeMap[node.id.createdAt to node.id.offset] = node } - } else { - node.insertAt(offset, content) - offset++ + nodeMapByID[node.id] = node } } } - return changes + return changes to latestCreatedAtMap + } + + private fun traverseAll( + node: CrdtTreeNode, + depth: Int = 0, + action: ((CrdtTreeNode, Int) -> Unit), + ) { + node.allChildren.forEach { child -> + traverseAll(child, depth + 1, action) + } + action.invoke(node, depth) } /** @@ -234,81 +253,71 @@ internal class CrdtTree( * [CrdtTreePos] is a position in the CRDT perspective. This is * different from [TreePos] which is a position of the tree in the local perspective. */ - private fun findTreePosWithSplitText( + fun findNodesAndSplitText( pos: CrdtTreePos, executedAt: TimeTicket, - ): Pair, CrdtTreeNode> { - val treePos = toTreePos(pos) ?: throw IllegalArgumentException("cannot find node at $pos") + ): Pair { + val treeNodes = + toTreeNodes(pos) ?: throw IllegalArgumentException("cannot find node at $pos") + val (parentNode, leftSiblingNode) = treeNodes // Find the appropriate position. This logic is similar to // handling the insertion of the same position in RGA. - var current = treePos - while (executedAt < current.node.next?.pos?.createdAt && - current.node.parent == current.node.next?.parent - ) { - val nextNode = current.node.next ?: break - current = TreePos(nextNode, nextNode.size) - } + if (leftSiblingNode.isText) { + val absOffset = leftSiblingNode.id.offset + val split = leftSiblingNode.split(pos.leftSiblingID.offset - absOffset, absOffset) + split?.let { + split.insPrev = leftSiblingNode + nodeMapByID[split.id] = split + + leftSiblingNode.insNext?.apply { + insPrev = split + split.insNext = this + } - if (current.node.isText) { - current.node.split(current.offset)?.let { split -> - insertAfter(current.node, split) - split.insertionPrev = current.node + leftSiblingNode.insNext = split } } + val index = if (parentNode == leftSiblingNode) { + 0 + } else { + parentNode.allChildren.indexOf(leftSiblingNode) + 1 + } - val right = requireNotNull(indexTree.findPostorderRight(treePos)) - return current to right - } - - /** - * Inserts the [newNode] after the [prevNode] - */ - private fun insertAfter(prevNode: CrdtTreeNode, newNode: CrdtTreeNode) { - val next = prevNode.next - prevNode.next = newNode - newNode.prev = prevNode - next?.let { - newNode.next = next - next.prev = newNode + var updatedLeftSiblingNode = leftSiblingNode + for (i in index until parentNode.allChildren.size) { + val next = parentNode.allChildren[i] + if (executedAt < next.id.createdAt) { + updatedLeftSiblingNode = next + } else { + break + } } - nodeMapByPos[newNode.pos] = newNode + return parentNode to updatedLeftSiblingNode } - /** - * Returns the nodes between the given range. - * [left] is inclusive, while [right] is exclusive. - */ - private fun nodesBetween( - left: CrdtTreeNode, - right: CrdtTreeNode, - action: (CrdtTreeNode) -> Unit, - ) { - var current: CrdtTreeNode? = left - while (current != right) { - current?.let(action) - ?: throw IllegalArgumentException("left and right are not in the same list") - current = current.next + private fun toTreeNodes(pos: CrdtTreePos): Pair? { + val parentID = pos.parentID + val leftSiblingID = pos.leftSiblingID + val (parentKey, parentNode) = nodeMapByID.floorEntry(parentID) ?: return null + val (leftSiblingKey, leftSiblingNode) = nodeMapByID.floorEntry(leftSiblingID) ?: return null + + if (parentKey.createdAt != parentID.createdAt || + leftSiblingKey.createdAt != leftSiblingID.createdAt + ) { + return null } - } - /** - * Returns start index of pos - * 0 1 2 3 4 5 6 7 8 - *

t e x t

- * If tree is just like above, and the pos is pointing index of 7 - * this returns 0 (start index of tag) - */ - private fun posToStartIndex(pos: CrdtTreePos): Int { - val treePos = toTreePos(pos) - val index = toIndex(pos) - val size = if (treePos?.node?.isText == true) { - treePos.node.parent?.size - } else { - treePos?.node?.size - } ?: -1 + val updatedLeftSiblingNode = + if (leftSiblingID.offset > 0 && leftSiblingID.offset == leftSiblingNode.offset && + leftSiblingNode.insPrev != null + ) { + leftSiblingNode.insPrev + } else { + leftSiblingNode + } - return index - size - 1 + return requireNotNull(parentNode) to requireNotNull(updatedLeftSiblingNode) } /** @@ -353,9 +362,9 @@ internal class CrdtTree( nodesToBeRemoved.forEach { node -> node.parent?.removeChild(node) - nodeMapByPos.remove(node.pos) + nodeMapByID.remove(node.id) delete(node) - removedNodeMap.remove(node.createdAt to node.pos.offset) + removedNodeMap.remove(node.createdAt to node.id.offset) } return nodesToBeRemoved.size @@ -365,14 +374,14 @@ internal class CrdtTree( * Physically deletes the given [node] from [IndexTree]. */ private fun delete(node: CrdtTreeNode) { - val prev = node.prev - val next = node.next - prev?.next = next - next?.prev = prev + val insPrev = node.insPrev + val insNext = node.insNext + + insPrev?.insNext = insNext + insNext?.insPrev = insPrev - node.prev = null - node.next = null - node.insertionPrev = null + node.insPrev = null + node.insNext = null } /** @@ -380,7 +389,21 @@ internal class CrdtTree( */ fun findPos(index: Int, preferText: Boolean = true): CrdtTreePos { val (node, offset) = indexTree.findTreePos(index, preferText) - return CrdtTreePos(node.pos.createdAt, node.pos.offset + offset) + var updatedNode = node + val leftSibling = if (node.isText) { + updatedNode = requireNotNull(node.parent) + if (node.parent?.children?.firstOrNull() == node && offset == 0) { + node.parent + } else { + node + } + } else { + if (offset == 0) node else node.children[offset - 1] + } ?: throw IllegalArgumentException("left sibling should not be null") + return CrdtTreePos( + updatedNode.id, + CrdtTreeNodeID(leftSibling.createdAt, leftSibling.offset + offset), + ) } /** @@ -393,23 +416,15 @@ internal class CrdtTree( /** * Converts the given [pos] to the index of the tree. */ - fun toIndex(pos: CrdtTreePos): Int { - return toTreePos(pos)?.let(indexTree::indexOf) ?: -1 + fun toIndex(parentNode: CrdtTreeNode, leftSiblingNode: CrdtTreeNode): Int { + return indexTree.indexOf(toTreePos(parentNode, leftSiblingNode)) } /** * Converts the given path of the node to the range of the position. */ fun pathToPosRange(path: List): TreePosRange { - val index = pathToIndex(path) - val (parentNode, offset) = pathToTreePos(path) - - if (parentNode.hasTextChild) { - throw IllegalArgumentException("invalid path: $path") - } - - val node = parentNode.children[offset] - val fromIndex = index + node.size + 1 + val fromIndex = pathToIndex(path) return findPos(fromIndex) to findPos(fromIndex + 1) } @@ -424,8 +439,7 @@ internal class CrdtTree( * Finds the position of the given index in the tree by [path]. */ fun pathToPos(path: List): CrdtTreePos { - val (node, offset) = indexTree.pathToTreePos(path) - return CrdtTreePos(node.pos.createdAt, node.pos.offset + offset) + return findPos(indexTree.pathToIndex(path)) } /** @@ -466,60 +480,37 @@ internal class CrdtTree( * Converts the [range] into [TreePosStructRange]. */ fun indexRangeToPosStructRange(range: Pair): TreePosStructRange { - val (fromPos, toPos) = indexRangeToPosRange(range) - return fromPos.toStruct() to toPos.toStruct() + val (fromIndex, toIndex) = range + val fromPos = findPos(fromIndex) + return if (fromIndex == toIndex) { + fromPos.toStruct() to fromPos.toStruct() + } else { + fromPos.toStruct() to findPos(toIndex).toStruct() + } } /** - * Returns a pair of integer offsets of the tree. + * Converts the given position [range] to the path range. */ - fun rangeToIndex(range: TreePosRange): Pair { - return toIndex(range.first) to toIndex(range.second) + fun posRangeToPathRange( + range: TreePosRange, + executedAt: TimeTicket, + ): Pair, List> { + val (fromParent, fromLeft) = findNodesAndSplitText(range.first, executedAt) + val (toParent, toLeft) = findNodesAndSplitText(range.second, executedAt) + return toPath(fromParent, fromLeft) to toPath(toParent, toLeft) } /** - * Converts the given position [range] to the path range. + * Converts the given position range to the path range. */ - fun posRangeToPathRange(range: TreePosRange): Pair, List> { - val fromPath = indexTree.indexToPath(toIndex(range.first)) - val toPath = indexTree.indexToPath(toIndex(range.second)) - return fromPath to toPath - } - - override fun isEmpty() = indexTree.size == 0 - - override fun iterator(): Iterator { - return object : Iterator { - var node = head.next - - override fun hasNext(): Boolean { - while (node != null) { - if (node?.isRemoved == false) { - return true - } - node = node?.next - } - return false - } - - override fun next(): CrdtTreeNode { - return requireNotNull(node).also { - node = node?.next - } - } - } - } - - override fun containsAll(elements: Collection): Boolean { - return indexTree.root.children.containsAll(elements) - } - - override fun contains(element: CrdtTreeNode): Boolean { - return indexTree.root.children.contains(element) - } - - companion object { - private const val INITIAL_NODE_TYPE = "dummy" + fun posRangeToIndexRange( + range: TreePosRange, + executedAt: TimeTicket, + ): Pair { + val (fromParent, fromLeft) = findNodesAndSplitText(range.first, executedAt) + val (toParent, toLeft) = findNodesAndSplitText(range.second, executedAt) + return toIndex(fromParent, fromLeft) to toIndex(toParent, toLeft) } } @@ -529,7 +520,7 @@ internal class CrdtTree( */ @Suppress("DataClassPrivateConstructor") internal data class CrdtTreeNode private constructor( - val pos: CrdtTreePos, + val id: CrdtTreeNodeID, override val type: String, private val _value: String? = null, private val childNodes: MutableList = mutableListOf(), @@ -543,7 +534,7 @@ internal data class CrdtTreeNode private constructor( get() = _attributes.toXml() val createdAt: TimeTicket - get() = pos.createdAt + get() = id.createdAt var removedAt: TimeTicket? = null private set @@ -551,6 +542,9 @@ internal data class CrdtTreeNode private constructor( override val isRemoved: Boolean get() = removedAt != null + val offset: Int + get() = id.offset + override var value: String = _value.orEmpty() get() { check(isText) { @@ -566,11 +560,9 @@ internal data class CrdtTreeNode private constructor( size = value.length } - var next: CrdtTreeNode? = null + var insPrev: CrdtTreeNode? = null - var prev: CrdtTreeNode? = null - - var insertionPrev: CrdtTreeNode? = null + var insNext: CrdtTreeNode? = null val rhtNodes: List get() = _attributes.toList() @@ -583,7 +575,11 @@ internal data class CrdtTreeNode private constructor( * Clones this node with the given [offset]. */ override fun clone(offset: Int): CrdtTreeNode { - return copy(pos = CrdtTreePos(pos.createdAt, offset)) + return copy( + id = CrdtTreeNodeID(id.createdAt, offset), + _value = null, + childNodes = mutableListOf(), + ) } fun setAttribute(key: String, value: String, executedAt: TimeTicket) { @@ -623,15 +619,22 @@ internal data class CrdtTreeNode private constructor( } } + /** + * Checks if node is able to delete. + */ + fun canDelete(executedAt: TimeTicket, latestCreatedAt: TimeTicket): Boolean { + return createdAt <= latestCreatedAt && (removedAt == null || removedAt < executedAt) + } + @Suppress("FunctionName") companion object { - fun CrdtTreeText(pos: CrdtTreePos, value: String): CrdtTreeNode { + fun CrdtTreeText(pos: CrdtTreeNodeID, value: String): CrdtTreeNode { return CrdtTreeNode(pos, DEFAULT_TEXT_TYPE, value) } fun CrdtTreeElement( - pos: CrdtTreePos, + pos: CrdtTreeNodeID, type: String, children: List = emptyList(), attributes: Rht = Rht(), @@ -640,11 +643,30 @@ internal data class CrdtTreeNode private constructor( } /** - * [CrdtTreePos] represents a position in the tree. It indicates the virtual - * location in the tree, so whether the node is split or not, we can find - * the adjacent node to pos by calling `map.floorEntry()`. + * [CrdtTreePos] represent a position in the tree. It is used to identify a + * position in the tree. It is composed of the parent ID and the left sibling ID. + * If there's no left sibling in parent's children, then left sibling is parent. + */ +public data class CrdtTreePos internal constructor( + val parentID: CrdtTreeNodeID, + val leftSiblingID: CrdtTreeNodeID, +) : JsonSerializable { + + override fun toStruct(): CrdtTreePosStruct { + return CrdtTreePosStruct(parentID.toStruct(), leftSiblingID.toStruct()) + } +} + + +/** + * [CrdtTreeNodeID] represent an ID of a node in the tree. It is used to + * identify a node in the tree. It is composed of the creation time of the node + * and the offset from the beginning of the node if the node is split. + * + * Some of replicas may have nodes that are not split yet. In this case, we can + * use `map.floorEntry()` to find the adjacent node. */ -public data class CrdtTreePos( +public data class CrdtTreeNodeID internal constructor( /** * Creation time of the node. */ @@ -654,17 +676,17 @@ public data class CrdtTreePos( * The distance from the beginning of the node when the node is split. */ val offset: Int, -) : Comparable, JsonSerializable { +) : Comparable, JsonSerializable { - override fun compareTo(other: CrdtTreePos): Int { + override fun compareTo(other: CrdtTreeNodeID): Int { return compareValuesBy(this, other, { it.createdAt }, { it.offset }) } - override fun toStruct(): CrdtTreePosStruct { - return CrdtTreePosStruct(createdAt.toStruct(), offset) + override fun toStruct(): CrdtTreeNodeIDStruct { + return CrdtTreeNodeIDStruct(createdAt.toStruct(), offset) } companion object { - internal val InitialCrdtTreePos = CrdtTreePos(InitialTimeTicket, 0) + internal val InitialCrdtTreeNodeID = CrdtTreeNodeID(InitialTimeTicket, 0) } } diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonTree.kt b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonTree.kt index d3cec3252..5bb392ac2 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonTree.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonTree.kt @@ -5,10 +5,10 @@ import dev.yorkie.document.crdt.CrdtTree import dev.yorkie.document.crdt.CrdtTreeNode import dev.yorkie.document.crdt.CrdtTreeNode.Companion.CrdtTreeElement import dev.yorkie.document.crdt.CrdtTreeNode.Companion.CrdtTreeText +import dev.yorkie.document.crdt.CrdtTreeNodeID import dev.yorkie.document.crdt.CrdtTreePos import dev.yorkie.document.crdt.Rht import dev.yorkie.document.crdt.TreePosRange -import dev.yorkie.document.json.JsonTree.TreeNode import dev.yorkie.document.operation.TreeEditOperation import dev.yorkie.document.operation.TreeStyleOperation import dev.yorkie.util.IndexTreeNode.Companion.DEFAULT_ROOT_TYPE @@ -24,8 +24,8 @@ public typealias TreePosStructRange = Pair public class JsonTree internal constructor( internal val context: ChangeContext, override val target: CrdtTree, -) : JsonElement(), Collection { - public override val size: Int by target::size +) : JsonElement() { + public val size: Int by target::size internal val indexTree by target::indexTree @@ -105,7 +105,11 @@ public class JsonTree internal constructor( editByPos(fromPos, toPos, contents.toList()) } - private fun editByPos(fromPos: CrdtTreePos, toPos: CrdtTreePos, contents: List) { + private fun editByPos( + fromPos: CrdtTreePos, + toPos: CrdtTreePos, + contents: List, + ) { if (contents.isNotEmpty()) { validateTreeNodes(contents) if (contents.first().type != DEFAULT_TEXT_TYPE) { @@ -119,12 +123,12 @@ public class JsonTree internal constructor( val compVal = contents .filterIsInstance() .joinToString("") { it.value } - listOf(CrdtTreeText(CrdtTreePos(context.issueTimeTicket(), 0), compVal)) + listOf(CrdtTreeText(CrdtTreeNodeID(context.issueTimeTicket(), 0), compVal)) } else { contents.map { createCrdtTreeNode(context, it) } } val ticket = context.lastTimeTicket - target.edit( + val (_, maxCreatedAtMapByActor) = target.edit( fromPos to toPos, crdtNodes.map { it.deepCopy() }.ifEmpty { null }, ticket, @@ -135,12 +139,13 @@ public class JsonTree internal constructor( target.createdAt, fromPos, toPos, + maxCreatedAtMapByActor, crdtNodes.ifEmpty { null }, ticket, ), ) - if (fromPos.createdAt != toPos.createdAt || fromPos.offset != toPos.offset) { + if (fromPos != toPos) { context.registerElementHasRemovedNodes(target) } } @@ -218,7 +223,7 @@ public class JsonTree internal constructor( */ public fun posRangeToIndexRange(range: TreePosStructRange): Pair { val posRange = range.first.toOriginal() to range.second.toOriginal() - return target.toIndex(posRange.first) to target.toIndex(posRange.second) + return target.posRangeToIndexRange(posRange, context.lastTimeTicket) } /** @@ -226,45 +231,7 @@ public class JsonTree internal constructor( */ public fun posRangeToPathRange(range: TreePosStructRange): Pair, List> { val posRange = range.first.toOriginal() to range.second.toOriginal() - return target.posRangeToPathRange(posRange) - } - - override fun isEmpty(): Boolean = target.isEmpty() - - override fun contains(element: TreeNode): Boolean { - return find { it == element } != null - } - - override fun containsAll(elements: Collection): Boolean { - return toList().containsAll(elements) - } - - override fun iterator(): Iterator { - return object : Iterator { - val targetIterator = target.iterator() - - override fun hasNext(): Boolean { - return targetIterator.hasNext() - } - - override fun next(): TreeNode { - return targetIterator.next().toTreeNode() - } - - private fun CrdtTreeNode.toTreeNode(): TreeNode { - return if (isText) { - TextNode(value) - } else { - ElementNode( - type, - attributes, - children.map { - it.toTreeNode() - }, - ) - } - } - } + return target.posRangeToPathRange(posRange, context.lastTimeTicket) } companion object { @@ -273,7 +240,7 @@ public class JsonTree internal constructor( * Returns the root node of this tree. */ internal fun buildRoot(initialRoot: ElementNode?, context: ChangeContext): CrdtTreeNode { - val pos = CrdtTreePos(context.issueTimeTicket(), 0) + val pos = CrdtTreeNodeID(context.issueTimeTicket(), 0) if (initialRoot == null) { return CrdtTreeElement(pos, DEFAULT_ROOT_TYPE) } @@ -292,7 +259,7 @@ public class JsonTree internal constructor( ) { val type = treeNode.type val ticket = context.issueTimeTicket() - val pos = CrdtTreePos(ticket, 0) + val pos = CrdtTreeNodeID(ticket, 0) when (treeNode) { is TextNode -> { @@ -324,7 +291,7 @@ public class JsonTree internal constructor( */ private fun createCrdtTreeNode(context: ChangeContext, content: TreeNode): CrdtTreeNode { val ticket = context.issueTimeTicket() - val pos = CrdtTreePos(ticket, 0) + val pos = CrdtTreeNodeID(ticket, 0) return when (content) { is TextNode -> { diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/operation/TreeEditOperation.kt b/yorkie/src/main/kotlin/dev/yorkie/document/operation/TreeEditOperation.kt index 8835350d8..f0af2f7ce 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/operation/TreeEditOperation.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/operation/TreeEditOperation.kt @@ -4,6 +4,7 @@ import dev.yorkie.document.crdt.CrdtRoot import dev.yorkie.document.crdt.CrdtTree import dev.yorkie.document.crdt.CrdtTreeNode import dev.yorkie.document.crdt.CrdtTreePos +import dev.yorkie.document.time.ActorID import dev.yorkie.document.time.TimeTicket import dev.yorkie.util.YorkieLogger @@ -14,6 +15,7 @@ internal data class TreeEditOperation( override val parentCreatedAt: TimeTicket, val fromPos: CrdtTreePos, val toPos: CrdtTreePos, + val maxCreatedAtMapByActor: Map, val contents: List?, override var executedAt: TimeTicket, ) : Operation() { @@ -30,9 +32,14 @@ internal data class TreeEditOperation( return emptyList() } val changes = - tree.edit(fromPos to toPos, contents?.map(CrdtTreeNode::deepCopy), executedAt) + tree.edit( + fromPos to toPos, + contents?.map(CrdtTreeNode::deepCopy), + executedAt, + maxCreatedAtMapByActor, + ).first - if (fromPos.createdAt != toPos.createdAt || fromPos.offset != toPos.offset) { + if (fromPos != toPos) { root.registerElementHasRemovedNodes(tree) } diff --git a/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt b/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt index b28b1c50a..49a5b6b88 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt @@ -139,7 +139,7 @@ internal class IndexTree>(val root: T) { val currentNode = node if (currentNode == null || currentNode == root) return@repeat - currentNode.split(offset) + currentNode.split(offset, 0) val nextOffset = currentNode.parent?.findOffset(currentNode) ?: return@repeat offset = if (offset == 0) nextOffset else nextOffset + 1 @@ -239,7 +239,7 @@ internal class IndexTree>(val root: T) { private fun addSizeOfLeftSiblings(parent: T, offset: Int): Int { return parent.children.take(offset).fold(0) { acc, leftSibling -> - acc + leftSibling.paddedSize + acc + if (leftSibling.isRemoved) 0 else leftSibling.paddedSize } } @@ -294,18 +294,6 @@ internal class IndexTree>(val root: T) { return TreePos(updatedNode, updatedPathElement) } - /** - * Finds right node of the given [treePos] with postorder traversal. - */ - fun findPostorderRight(treePos: TreePos): T? { - val (node, offset) = treePos - return when { - node.isText -> if (node.size == offset) node.nextSibling ?: node.parent else node - node.children.size == offset -> node - else -> findLeftMost(node.children[offset]) - } - } - private fun findLeftMost(node: T): T { return if (node.isText || node.children.isEmpty()) { node @@ -467,6 +455,14 @@ internal abstract class IndexTreeNode>(children: MutableLis check(!isText) { "Text node cannot have children" } + if (node.isRemoved) { + val index = _children.indexOf(node) + + // If nodes are removed, the offset of the removed node is the number of + // nodes before the node excluding the removed nodes. + val refined = allChildren.take(index).filterNot { it.isRemoved }.size - 1 + return refined.coerceAtLeast(0) + } return children.indexOf(node) } @@ -502,7 +498,10 @@ internal abstract class IndexTreeNode>(children: MutableLis _children.addAll(newNode) newNode.forEach { node -> node.parent = this as T - node.updateAncestorSize() + + if (!node.isRemoved) { + node.updateAncestorSize() + } } } @@ -590,24 +589,24 @@ internal abstract class IndexTreeNode>(children: MutableLis /** * Splits the node at the given [offset]. */ - fun split(offset: Int): T? { + fun split(offset: Int, absOffset: Int): T? { return if (isText) { - splitText(offset) + splitText(offset, absOffset) } else { splitElement(offset) } } - private fun splitText(offset: Int): T? { + private fun splitText(offset: Int, absOffset: Int): T? { if (offset == 0 || offset == size) { return null } val leftValue = value.substring(0, offset) - val rightValue = value.substring(offset) + val rightValue = value.substring(offset).takeUnless { it.isEmpty() } ?: return null value = leftValue - val rightNode = clone(offset) + val rightNode = clone(offset + absOffset) rightNode.value = rightValue parent?.insertAfterInternal(this as T, rightNode) diff --git a/yorkie/src/test/kotlin/dev/yorkie/api/ConverterTest.kt b/yorkie/src/test/kotlin/dev/yorkie/api/ConverterTest.kt index 206a8f7d5..7e8e659e0 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/api/ConverterTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/api/ConverterTest.kt @@ -18,6 +18,7 @@ import dev.yorkie.document.crdt.CrdtText import dev.yorkie.document.crdt.CrdtTree import dev.yorkie.document.crdt.CrdtTreeNode.Companion.CrdtTreeElement import dev.yorkie.document.crdt.CrdtTreeNode.Companion.CrdtTreeText +import dev.yorkie.document.crdt.CrdtTreeNodeID import dev.yorkie.document.crdt.CrdtTreePos import dev.yorkie.document.crdt.ElementRht import dev.yorkie.document.crdt.RgaTreeList @@ -37,8 +38,10 @@ import dev.yorkie.document.operation.StyleOperation import dev.yorkie.document.operation.TreeEditOperation import dev.yorkie.document.operation.TreeStyleOperation import dev.yorkie.document.time.ActorID +import dev.yorkie.document.time.ActorID.Companion.INITIAL_ACTOR_ID import dev.yorkie.document.time.TimeTicket import dev.yorkie.document.time.TimeTicket.Companion.InitialTimeTicket +import dev.yorkie.document.time.TimeTicket.Companion.MaxTimeTicket import dev.yorkie.util.IndexTreeNode.Companion.DEFAULT_ROOT_TYPE import org.junit.Assert.assertThrows import org.junit.Test @@ -50,7 +53,7 @@ class ConverterTest { @Test fun `should convert ByteString`() { - val actorID = ActorID.INITIAL_ACTOR_ID + val actorID = INITIAL_ACTOR_ID val converted = actorID.toByteString().toActorID() val maxActorID = ActorID.MAX_ACTOR_ID val maxConverted = maxActorID.toByteString().toActorID() @@ -247,15 +250,22 @@ class ConverterTest { ) val treeEditOperation = TreeEditOperation( InitialTimeTicket, - CrdtTreePos(InitialTimeTicket, 5), - CrdtTreePos(InitialTimeTicket, 10), - listOf(CrdtTreeText(CrdtTreePos(InitialTimeTicket, 0), "hi")), + CrdtTreePos(CrdtTreeNodeID(InitialTimeTicket, 5), CrdtTreeNodeID(InitialTimeTicket, 5)), + CrdtTreePos( + CrdtTreeNodeID(InitialTimeTicket, 10), + CrdtTreeNodeID(InitialTimeTicket, 10), + ), + mapOf(INITIAL_ACTOR_ID to MaxTimeTicket), + listOf(CrdtTreeText(CrdtTreeNodeID(InitialTimeTicket, 0), "hi")), InitialTimeTicket, ) val treeStyleOperation = TreeStyleOperation( InitialTimeTicket, - CrdtTreePos(InitialTimeTicket, 5), - CrdtTreePos(InitialTimeTicket, 10), + CrdtTreePos(CrdtTreeNodeID(InitialTimeTicket, 5), CrdtTreeNodeID(InitialTimeTicket, 5)), + CrdtTreePos( + CrdtTreeNodeID(InitialTimeTicket, 10), + CrdtTreeNodeID(InitialTimeTicket, 10), + ), mapOf("a" to "b"), InitialTimeTicket, ) @@ -391,7 +401,7 @@ class ConverterTest { val crdtCounter = CrdtCounter(1, InitialTimeTicket) val crdtText = CrdtText(RgaTreeSplit(), InitialTimeTicket) val crdtTree = CrdtTree( - CrdtTreeElement(CrdtTreePos(InitialTimeTicket, 0), DEFAULT_ROOT_TYPE), + CrdtTreeElement(CrdtTreeNodeID(InitialTimeTicket, 0), DEFAULT_ROOT_TYPE), InitialTimeTicket, ) val crdtElements = listOf( diff --git a/yorkie/src/test/kotlin/dev/yorkie/core/ClientTest.kt b/yorkie/src/test/kotlin/dev/yorkie/core/ClientTest.kt index 67720bee2..8e7bdbef1 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/core/ClientTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/core/ClientTest.kt @@ -1,8 +1,6 @@ package dev.yorkie.core -import com.google.protobuf.ByteString import dev.yorkie.api.PBChangePack -import dev.yorkie.api.toActorID import dev.yorkie.api.toChangePack import dev.yorkie.api.v1.ActivateClientRequest import dev.yorkie.api.v1.AttachDocumentRequest @@ -13,8 +11,8 @@ import dev.yorkie.api.v1.RemoveDocumentRequest import dev.yorkie.api.v1.WatchDocumentRequest import dev.yorkie.api.v1.YorkieServiceGrpcKt import dev.yorkie.assertJsonContentEquals -import dev.yorkie.core.Client.Event.DocumentSynced import dev.yorkie.core.Client.Event.DocumentChanged +import dev.yorkie.core.Client.Event.DocumentSynced import dev.yorkie.core.MockYorkieService.Companion.ATTACH_ERROR_DOCUMENT_KEY import dev.yorkie.core.MockYorkieService.Companion.DETACH_ERROR_DOCUMENT_KEY import dev.yorkie.core.MockYorkieService.Companion.NORMAL_DOCUMENT_KEY @@ -28,6 +26,7 @@ import dev.yorkie.document.change.Change import dev.yorkie.document.change.ChangeID import dev.yorkie.document.change.ChangePack import dev.yorkie.document.change.CheckPoint +import dev.yorkie.document.time.ActorID import io.grpc.Channel import io.grpc.inprocess.InProcessChannelBuilder import io.grpc.inprocess.InProcessServerBuilder @@ -87,7 +86,6 @@ class ClientTest { assertTrue(target.isActive) val activatedStatus = assertIs(target.status.value) - assertEquals(TEST_KEY, activatedStatus.clientKey) assertEquals(TEST_ACTOR_ID, activatedStatus.clientId) val deactivateRequestCaptor = argumentCaptor() @@ -283,8 +281,8 @@ class ClientTest { } } - private fun assertIsTestActorID(clientId: ByteString) { - assertEquals(TEST_ACTOR_ID, clientId.toActorID()) + private fun assertIsTestActorID(clientId: String) { + assertEquals(TEST_ACTOR_ID, ActorID(clientId)) } private fun assertIsInitialChangePack(changePack: PBChangePack) { diff --git a/yorkie/src/test/kotlin/dev/yorkie/core/MockYorkieService.kt b/yorkie/src/test/kotlin/dev/yorkie/core/MockYorkieService.kt index e033f4b6c..63f6f3979 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/core/MockYorkieService.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/core/MockYorkieService.kt @@ -2,7 +2,6 @@ package dev.yorkie.core import com.google.protobuf.kotlin.toByteString import dev.yorkie.api.PBTimeTicket -import dev.yorkie.api.toByteString import dev.yorkie.api.toPBChange import dev.yorkie.api.toPBTimeTicket import dev.yorkie.api.v1.ActivateClientRequest @@ -54,17 +53,14 @@ class MockYorkieService : YorkieServiceGrpcKt.YorkieServiceCoroutineImplBase() { override suspend fun activateClient(request: ActivateClientRequest): ActivateClientResponse { return activateClientResponse { - clientId = TEST_ACTOR_ID.toByteString() - clientKey = request.clientKey + clientId = TEST_ACTOR_ID.value } } override suspend fun deactivateClient( request: DeactivateClientRequest, ): DeactivateClientResponse { - return deactivateClientResponse { - clientId = request.clientId - } + return deactivateClientResponse { } } override suspend fun attachDocument(request: AttachDocumentRequest): AttachDocumentResponse { @@ -72,7 +68,6 @@ class MockYorkieService : YorkieServiceGrpcKt.YorkieServiceCoroutineImplBase() { throw StatusException(Status.UNKNOWN) } return attachDocumentResponse { - clientId = request.clientId changePack = changePack { documentKey = request.changePack.documentKey changes.add( @@ -98,9 +93,7 @@ class MockYorkieService : YorkieServiceGrpcKt.YorkieServiceCoroutineImplBase() { if (request.changePack.documentKey == DETACH_ERROR_DOCUMENT_KEY) { throw StatusException(Status.UNKNOWN) } - return detachDocumentResponse { - clientKey = TEST_KEY - } + return detachDocumentResponse { } } override suspend fun pushPullChanges(request: PushPullChangesRequest): PushPullChangesResponse { @@ -108,7 +101,6 @@ class MockYorkieService : YorkieServiceGrpcKt.YorkieServiceCoroutineImplBase() { throw StatusException(Status.UNAVAILABLE) } return pushPullChangesResponse { - clientId = request.clientId changePack = changePack { minSyncedTicket = InitialTimeTicket.toPBTimeTicket() changes.add( @@ -149,7 +141,7 @@ class MockYorkieService : YorkieServiceGrpcKt.YorkieServiceCoroutineImplBase() { emit( watchDocumentResponse { initialization = initialization { - clientIds.add(TEST_ACTOR_ID.toByteString()) + clientIds.add(TEST_ACTOR_ID.value) } }, ) @@ -160,7 +152,7 @@ class MockYorkieService : YorkieServiceGrpcKt.YorkieServiceCoroutineImplBase() { emit( watchDocumentResponse { event = docEvent { - type = DocEventType.DOC_EVENT_TYPE_DOCUMENTS_CHANGED + type = DocEventType.DOC_EVENT_TYPE_DOCUMENT_CHANGED publisher = request.clientId } }, @@ -169,7 +161,7 @@ class MockYorkieService : YorkieServiceGrpcKt.YorkieServiceCoroutineImplBase() { emit( watchDocumentResponse { event = docEvent { - type = DocEventType.DOC_EVENT_TYPE_DOCUMENTS_WATCHED + type = DocEventType.DOC_EVENT_TYPE_DOCUMENT_WATCHED publisher = request.clientId } }, @@ -178,7 +170,7 @@ class MockYorkieService : YorkieServiceGrpcKt.YorkieServiceCoroutineImplBase() { emit( watchDocumentResponse { event = docEvent { - type = DocEventType.DOC_EVENT_TYPE_DOCUMENTS_UNWATCHED + type = DocEventType.DOC_EVENT_TYPE_DOCUMENT_UNWATCHED publisher = request.clientId } }, @@ -191,7 +183,6 @@ class MockYorkieService : YorkieServiceGrpcKt.YorkieServiceCoroutineImplBase() { throw StatusException(Status.UNAVAILABLE) } return removeDocumentResponse { - clientKey = TEST_KEY changePack = changePack { minSyncedTicket = InitialTimeTicket.toPBTimeTicket() changes.add( diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTreeTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTreeTest.kt index 53e2252f2..4d9c225a1 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTreeTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTreeTest.kt @@ -11,7 +11,6 @@ import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test -import dev.yorkie.document.crdt.CrdtTreePos.Companion.InitialCrdtTreePos as ITP class CrdtTreeTest { @@ -24,8 +23,8 @@ class CrdtTreeTest { @Test fun `should create CrdtTreeNode`() { - val node = CrdtTreeText(ITP, "hello") - assertEquals(ITP, node.pos) + val node = CrdtTreeText(DIP, "hello") + assertEquals(DIP, node.id) assertEquals(DEFAULT_TEXT_TYPE, node.type) assertEquals("hello", node.value) assertEquals(5, node.size) @@ -35,42 +34,39 @@ class CrdtTreeTest { @Test fun `should split CrdtTreeNode`() { - val para = CrdtTreeElement(ITP, "p") - para.append(CrdtTreeText(ITP, "helloyorkie")) + val para = CrdtTreeElement(DIP, "p") + para.append(CrdtTreeText(DIP, "helloyorkie")) assertEquals("

helloyorkie

", para.toXml()) assertEquals(11, para.size) assertFalse(para.isText) val left = para.children.first() - val right = left.split(5) + val right = left.split(5, 0) assertEquals(11, para.size) assertFalse(para.isText) assertEquals("hello", left.value) assertEquals("yorkie", right?.value) - assertEquals(CrdtTreePos(TimeTicket.InitialTimeTicket, 0), left.pos) - assertEquals(CrdtTreePos(TimeTicket.InitialTimeTicket, 5), right?.pos) + assertEquals(CrdtTreeNodeID(TimeTicket.InitialTimeTicket, 0), left.id) + assertEquals(CrdtTreeNodeID(TimeTicket.InitialTimeTicket, 5), right?.id) } @Test fun `should insert nodes with editByIndex`() { // 0 // - assertTrue(target.isEmpty()) + assertTrue(target.size == 0) assertEquals("", target.toXml()) - assertEquals(listOf("root"), target.toList()) // 1 //

target.editByIndex(0 to 0, CrdtTreeElement(issuePos(), "p").toList(), issueTime()) assertEquals("

", target.toXml()) - assertEquals(listOf("p", "root"), target.toList()) assertEquals(2, target.root.size) // 1 //

h e l l o

target.editByIndex(1 to 1, CrdtTreeText(issuePos(), "hello").toList(), issueTime()) assertEquals("

hello

", target.toXml()) - assertEquals(listOf("text.hello", "p", "root"), target.toList()) assertEquals(7, target.root.size) // 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 @@ -80,25 +76,16 @@ class CrdtTreeTest { } target.editByIndex(7 to 7, p.toList(), issueTime()) assertEquals("

hello

world

", target.toXml()) - assertEquals(listOf("text.hello", "p", "text.world", "p", "root"), target.toList()) assertEquals(14, target.root.size) // 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 //

h e l l o !

w o r l d

target.editByIndex(6 to 6, CrdtTreeText(issuePos(), "!").toList(), issueTime()) assertEquals("

hello!

world

", target.toXml()) - assertEquals( - listOf("text.hello", "text.!", "p", "text.world", "p", "root"), - target.toList(), - ) assertEquals(15, target.root.size) target.editByIndex(6 to 6, CrdtTreeText(issuePos(), "~").toList(), issueTime()) assertEquals("

hello~!

world

", target.toXml()) - assertEquals( - listOf("text.hello", "text.~", "text.!", "p", "text.world", "p", "root"), - target.toList(), - ) assertEquals(16, target.root.size) } @@ -112,16 +99,19 @@ class CrdtTreeTest { target.editByIndex(4 to 4, CrdtTreeElement(issuePos(), "p").toList(), issueTime()) target.editByIndex(5 to 5, CrdtTreeText(issuePos(), "cd").toList(), issueTime()) assertEquals("

ab

cd

", target.toXml()) - assertEquals(listOf("text.ab", "p", "text.cd", "p", "root"), target.toList()) assertEquals(8, target.root.size) // 02. delete b from first paragraph // 0 1 2 3 4 5 6 7 //

a

c d

- target.editByIndex(2 to 3, null, issueTime()) - assertEquals("

a

cd

", target.toXml()) - assertEquals(listOf("text.a", "p", "text.cd", "p", "root"), target.toList()) - assertEquals(7, target.root.size) + target.editByIndex(2 to 6, null, issueTime()) + // TODO(7hong13): should be resolved after the JS SDK fix + // assertEquals("root>

ad

", target.toXml()) + + // 03. insert a new text node at the start of the first paragraph. + target.editByIndex(1 to 1, CrdtTreeText(issuePos(), "@").toList(), issueTime()) + // TODO(7hong13): should be resolved after the JS SDK fix + // assertEquals("root>

@ad

", target.toXml()) } @Test @@ -134,130 +124,117 @@ class CrdtTreeTest { target.editByIndex(4 to 4, CrdtTreeElement(issuePos(), "p").toList(), issueTime()) target.editByIndex(5 to 5, CrdtTreeText(issuePos(), "cd").toList(), issueTime()) assertEquals("

ab

cd

", target.toXml()) - assertEquals(listOf("text.ab", "p", "text.cd", "p", "root"), target.toList()) - - // 02. delete b, c and first paragraph. - // 0 1 2 3 4 - //

a d

- target.editByIndex(2 to 6, null, issueTime()) - assertEquals("

ad

", target.toXml()) - // 03. insert a new text node at the start of the first paragraph. - target.editByIndex(1 to 1, CrdtTreeText(issuePos(), "@").toList(), issueTime()) - assertEquals("

@ad

", target.toXml()) + // 02. delete b from first paragraph + // 0 1 2 3 4 5 6 7 + //

a

c d

+ target.editByIndex(2 to 3, null, issueTime()) + assertEquals("

a

cd

", target.toXml()) } @Test - fun `should merge and edit different levels with editByIndex`() { - fun initializeTree() { - setUp() - target.editByIndex(0 to 0, CrdtTreeElement(issuePos(), "p").toList(), issueTime()) - target.editByIndex(1 to 1, CrdtTreeElement(issuePos(), "b").toList(), issueTime()) - target.editByIndex(2 to 2, CrdtTreeElement(issuePos(), "i").toList(), issueTime()) - target.editByIndex(3 to 3, CrdtTreeText(issuePos(), "ab").toList(), issueTime()) - assertEquals("

ab

", target.toXml()) - } - - // 01. edit between two element nodes in the same hierarchy. - // 0 1 2 3 4 5 6 7 8 - //

a b

- initializeTree() - target.editByIndex(5 to 6, null, issueTime()) - assertEquals("

ab

", target.toXml()) - - // 02. edit between two element nodes in same hierarchy. - initializeTree() - target.editByIndex(6 to 7, null, issueTime()) - assertEquals("

ab

", target.toXml()) - - // 03. edit between text and element node in different hierarchy. - initializeTree() - target.editByIndex(4 to 6, null, issueTime()) - assertEquals("

a

", target.toXml()) + fun `should find the closest TreePos when parentNode or leftSiblingNode does not exist`() { + val pNode = CrdtTreeElement(issuePos(), "p") + val textNode = CrdtTreeText(issuePos(), "ab") - // 04. edit between text and element node in different hierarchy. - initializeTree() - target.editByIndex(5 to 7, null, issueTime()) + // 0 1 2 3 4 + //

a b

+ target.editByIndex(0 to 0, pNode.toList(), issueTime()) + target.editByIndex(1 to 1, textNode.toList(), issueTime()) assertEquals("

ab

", target.toXml()) - // 05. edit between text and element node in different hierarchy. - initializeTree() - target.editByIndex(4 to 7, null, issueTime()) - assertEquals("

a

", target.toXml()) - - // 06. edit between text and element node in different hierarchy. - initializeTree() - target.editByIndex(3 to 7, null, issueTime()) + // Find the closest index.TreePos when leftSiblingNode in crdt.TreePos is removed. + // 0 1 2 + //

+ target.editByIndex(1 to 3, null, issueTime()) assertEquals("

", target.toXml()) - // 07. edit between text and element node in same hierarchy. - setUp() - target.editByIndex(0 to 0, CrdtTreeElement(issuePos(), "p").toList(), issueTime()) - target.editByIndex(1 to 1, CrdtTreeText(issuePos(), "ab").toList(), issueTime()) - target.editByIndex(4 to 4, CrdtTreeElement(issuePos(), "p").toList(), issueTime()) - target.editByIndex(5 to 5, CrdtTreeElement(issuePos(), "b").toList(), issueTime()) - target.editByIndex(6 to 6, CrdtTreeText(issuePos(), "cd").toList(), issueTime()) - target.editByIndex(10 to 10, CrdtTreeElement(issuePos(), "p").toList(), issueTime()) - target.editByIndex(11 to 11, CrdtTreeText(issuePos(), "ef").toList(), issueTime()) - assertEquals("

ab

cd

ef

", target.toXml()) - - target.editByIndex(9 to 10, null, issueTime()) - assertEquals("

ab

cd

ef

", target.toXml()) - } - - @Test - fun `should get correct index from CrdtTreePos`() { - // 0 1 2 3 4 5 6 7 8 - //

a b

- target.editByIndex(0 to 0, CrdtTreeElement(issuePos(), "p").toList(), issueTime()) - target.editByIndex(1 to 1, CrdtTreeElement(issuePos(), "b").toList(), issueTime()) - target.editByIndex(2 to 2, CrdtTreeElement(issuePos(), "i").toList(), issueTime()) - target.editByIndex(3 to 3, CrdtTreeText(issuePos(), "ab").toList(), issueTime()) - assertEquals("

ab

", target.toXml()) - - var (from, to) = target.pathToPosRange(listOf(0)) - var fromIndex = target.toIndex(from) - var toIndex = target.toIndex(to) - assertEquals(7 to 8, fromIndex to toIndex) + var (parent, left) = target.findNodesAndSplitText( + CrdtTreePos(pNode.id, textNode.id), + issueTime(), + ) + assertEquals(1, target.toIndex(parent, left)) - target.pathToPosRange(listOf(0, 0)).also { - from = it.first - to = it.second - } - fromIndex = target.toIndex(from) - toIndex = target.toIndex(to) - assertEquals(6 to 7, fromIndex to toIndex) + // Find the closest index.TreePos when parentNode in crdt.TreePos is removed. + // 0 + // + target.editByIndex(0 to 2, null, issueTime()) + assertEquals("", target.toXml()) - target.pathToPosRange(listOf(0, 0, 0)).also { - from = it.first - to = it.second + target.findNodesAndSplitText(CrdtTreePos(pNode.id, textNode.id), issueTime()).also { + parent = it.first + left = it.second } - fromIndex = target.toIndex(from) - toIndex = target.toIndex(to) - assertEquals(5 to 6, fromIndex to toIndex) - - var range = target.indexRangeToPosRange(0 to 5) - assertEquals(0 to 5, target.rangeToIndex(range)) + assertEquals(0, target.toIndex(parent, left)) + } - range = target.indexRangeToPosRange(5 to 7) - assertEquals(5 to 7, target.rangeToIndex(range)) + @Test + fun `should merge and edit different levels with editByIndex`() { + // TODO(7hong13): should be resolved after the JS SDK fix +// fun initializeTree() { +// setUp() +// target.editByIndex(0 to 0, CrdtTreeElement(issuePos(), "p").toList(), issueTime()) +// target.editByIndex(1 to 1, CrdtTreeElement(issuePos(), "b").toList(), issueTime()) +// target.editByIndex(2 to 2, CrdtTreeElement(issuePos(), "i").toList(), issueTime()) +// target.editByIndex(3 to 3, CrdtTreeText(issuePos(), "ab").toList(), issueTime()) +// assertEquals("

ab

", target.toXml()) +// } +// +// // 01. edit between two element nodes in the same hierarchy. +// // 0 1 2 3 4 5 6 7 8 +// //

a b

+// initializeTree() +// target.editByIndex(5 to 6, null, issueTime()) +// assertEquals("

ab

", target.toXml()) +// +// // 02. edit between two element nodes in same hierarchy. +// initializeTree() +// target.editByIndex(6 to 7, null, issueTime()) +// assertEquals("

ab

", target.toXml()) +// +// // 03. edit between text and element node in different hierarchy. +// initializeTree() +// target.editByIndex(4 to 6, null, issueTime()) +// assertEquals("

a

", target.toXml()) +// +// // 04. edit between text and element node in different hierarchy. +// initializeTree() +// target.editByIndex(5 to 7, null, issueTime()) +// assertEquals("

ab

", target.toXml()) +// +// // 05. edit between text and element node in different hierarchy. +// initializeTree() +// target.editByIndex(4 to 7, null, issueTime()) +// assertEquals("

a

", target.toXml()) +// +// // 06. edit between text and element node in different hierarchy. +// initializeTree() +// target.editByIndex(3 to 7, null, issueTime()) +// assertEquals("

", target.toXml()) +// +// // 07. edit between text and element node in same hierarchy. +// setUp() +// target.editByIndex(0 to 0, CrdtTreeElement(issuePos(), "p").toList(), issueTime()) +// target.editByIndex(1 to 1, CrdtTreeText(issuePos(), "ab").toList(), issueTime()) +// target.editByIndex(4 to 4, CrdtTreeElement(issuePos(), "p").toList(), issueTime()) +// target.editByIndex(5 to 5, CrdtTreeElement(issuePos(), "b").toList(), issueTime()) +// target.editByIndex(6 to 6, CrdtTreeText(issuePos(), "cd").toList(), issueTime()) +// target.editByIndex(10 to 10, CrdtTreeElement(issuePos(), "p").toList(), issueTime()) +// target.editByIndex(11 to 11, CrdtTreeText(issuePos(), "ef").toList(), issueTime()) +// assertEquals("

ab

cd

ef

", target.toXml()) +// +// target.editByIndex(9 to 10, null, issueTime()) +// assertEquals("

ab

cd

ef

", target.toXml()) } - private fun issuePos(offset: Int = 0) = CrdtTreePos(issueTime(), offset) + private fun issuePos(offset: Int = 0) = CrdtTreeNodeID(issueTime(), offset) private fun issueTime() = DummyContext.issueTimeTicket() - private fun CrdtTree.toList() = map { node -> - if (node.isText) { - "${node.type}.${node.value}" - } else { - node.type - } - } - private fun CrdtTreeNode.toList() = listOf(this) companion object { + private val DIP = CrdtTreeNodeID(TimeTicket.InitialTimeTicket, 0) private val DummyContext = ChangeContext( ChangeID.InitialChangeID, diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt index a9bd5b7c1..e3233cef6 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt @@ -9,7 +9,7 @@ import dev.yorkie.document.crdt.CrdtRoot import dev.yorkie.document.crdt.CrdtTree import dev.yorkie.document.crdt.CrdtTreeNode import dev.yorkie.document.crdt.CrdtTreeNode.Companion.CrdtTreeElement -import dev.yorkie.document.crdt.CrdtTreePos +import dev.yorkie.document.crdt.CrdtTreeNodeID import dev.yorkie.document.crdt.TreeNode import dev.yorkie.document.json.TreeBuilder.element import dev.yorkie.document.json.TreeBuilder.text @@ -95,36 +95,6 @@ class JsonTreeTest { target.toXml(), ) assertEquals(18, target.size) - assertContentEquals( - listOf( - text { "ab" }, - element("p") { - text { "ab" } - }, - text { "cd" }, - element("note") { - text { "cd" } - }, - text { "ef" }, - element("note") { - text { "ef" } - }, - element("ng") { - element("note") { - text { "cd" } - } - element("note") { - text { "ef" } - } - }, - text { "gh" }, - element("bp") { - text { "gh" } - }, - root, - ), - target.toList(), - ) } @Test @@ -255,19 +225,19 @@ class JsonTreeTest { target.toXml(), ) - target.style(4, 5, mapOf("c" to "d")) + target.style(1, 2, mapOf("c" to "d")) assertEquals( """

""", target.toXml(), ) - target.style(4, 5, mapOf("c" to "q")) + target.style(1, 2, mapOf("c" to "q")) assertEquals( """

""", target.toXml(), ) - target.style(3, 4, mapOf("z" to "m")) + target.style(2, 3, mapOf("z" to "m")) assertEquals( """

""", target.toXml(), @@ -337,7 +307,7 @@ class JsonTreeTest { root.tree().edit(1, 1, text { "X" }) assertEquals("

Xab

", root.tree().toXml()) - root.tree().style(4, 5, mapOf("a" to "b")) + root.tree().style(0, 1, mapOf("a" to "b")) }.await() assertContentEquals( listOf( @@ -350,8 +320,8 @@ class JsonTreeTest { "$.t", ), TreeStyleOpInfo( - 4, - 5, + 0, + 1, listOf(0), listOf(0), mapOf("a" to "b"), @@ -416,8 +386,8 @@ class JsonTreeTest { "$.t", ), TreeStyleOpInfo( - 6, - 7, + 2, + 3, listOf(0, 0, 0), listOf(0, 0, 0), mapOf("a" to "b"), @@ -470,7 +440,7 @@ class JsonTreeTest { get() = CrdtTree(rootCrdtTreeNode, InitialTimeTicket) private val rootCrdtTreeNode: CrdtTreeNode - get() = CrdtTreeElement(CrdtTreePos(InitialTimeTicket, 0), DEFAULT_ROOT_TYPE) + get() = CrdtTreeElement(CrdtTreeNodeID(InitialTimeTicket, 0), DEFAULT_ROOT_TYPE) private fun createTreeWithStyle(): JsonTree { val root = element("doc") { diff --git a/yorkie/src/test/kotlin/dev/yorkie/util/IndexTreeTest.kt b/yorkie/src/test/kotlin/dev/yorkie/util/IndexTreeTest.kt index 8c60702a3..8c60a729c 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/util/IndexTreeTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/util/IndexTreeTest.kt @@ -3,7 +3,7 @@ package dev.yorkie.util import dev.yorkie.document.crdt.CrdtTreeNode import dev.yorkie.document.crdt.CrdtTreeNode.Companion.CrdtTreeElement import dev.yorkie.document.crdt.CrdtTreeNode.Companion.CrdtTreeText -import dev.yorkie.document.crdt.CrdtTreePos.Companion.InitialCrdtTreePos +import dev.yorkie.document.crdt.CrdtTreeNodeID.Companion.InitialCrdtTreeNodeID import org.junit.Assert.assertEquals import org.junit.Assert.assertThrows import org.junit.Test @@ -48,28 +48,6 @@ class IndexTreeTest { } } - @Test - fun `should find right node from the given offset in postorder traversal`() { - // 0 1 2 3 4 5 6 7 8 - //

a b

c d

- val tree = createIndexTree( - createElementNode( - "root", - createElementNode("p", createTextNode("ab")), - createElementNode("p", createTextNode("cd")), - ), - ) - - // postorder traversal: "ab", , "cd",

, - assertEquals("text", tree.findPostorderRight(tree.findTreePos(0))?.type) - assertEquals("text", tree.findPostorderRight(tree.findTreePos(1))?.type) - assertEquals("p", tree.findPostorderRight(tree.findTreePos(3))?.type) - assertEquals("text", tree.findPostorderRight(tree.findTreePos(4))?.type) - assertEquals("text", tree.findPostorderRight(tree.findTreePos(5))?.type) - assertEquals("p", tree.findPostorderRight(tree.findTreePos(7))?.type) - assertEquals("root", tree.findPostorderRight(tree.findTreePos(8))?.type) - } - @Test fun `should find common ancestor of two given nodes`() { val tree = createIndexTree( @@ -405,11 +383,11 @@ class IndexTreeTest { companion object { private fun createElementNode(type: String, vararg childNode: CrdtTreeNode): CrdtTreeNode { - return CrdtTreeElement(InitialCrdtTreePos, type, childNode.toList()) + return CrdtTreeElement(InitialCrdtTreeNodeID, type, childNode.toList()) } private fun createTextNode(value: String) = - CrdtTreeText(InitialCrdtTreePos, value) + CrdtTreeText(InitialCrdtTreeNodeID, value) private val DefaultRootNode = createElementNode( "root", From f35c396eea6e37d0ac213a2f864c164b0d021a0c Mon Sep 17 00:00:00 2001 From: jee-hyun-kim Date: Wed, 23 Aug 2023 16:07:51 +0900 Subject: [PATCH 05/13] add more tree TCs --- docker/docker-compose-ci.yml | 2 +- docker/docker-compose.yml | 2 +- .../com/example/texteditor/EditorViewModel.kt | 3 +- .../src/main/proto/yorkie/v1/resources.proto | 5 +- .../kotlin/dev/yorkie/core/ClientTest.kt | 2 +- .../dev/yorkie/document/json/JsonTreeTest.kt | 1331 ++++++++++++++++- .../kotlin/dev/yorkie/api/ElementConverter.kt | 16 +- .../dev/yorkie/document/crdt/CrdtTree.kt | 81 +- .../dev/yorkie/document/json/JsonElement.kt | 1 + .../dev/yorkie/document/json/JsonTree.kt | 24 +- .../main/kotlin/dev/yorkie/util/IndexTree.kt | 5 +- .../dev/yorkie/document/crdt/CrdtTreeTest.kt | 6 +- .../dev/yorkie/document/json/JsonTreeTest.kt | 232 +++ 13 files changed, 1644 insertions(+), 66 deletions(-) diff --git a/docker/docker-compose-ci.yml b/docker/docker-compose-ci.yml index edc186b7b..0e733303d 100644 --- a/docker/docker-compose-ci.yml +++ b/docker/docker-compose-ci.yml @@ -15,7 +15,7 @@ services: depends_on: - yorkie yorkie: - image: 'yorkieteam/yorkie:0.4.5-alpha' + image: 'yorkieteam/yorkie:latest' container_name: 'yorkie' command: [ 'server', diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index 900e695c5..c99b7e27e 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -22,7 +22,7 @@ services: extra_hosts: - "host.docker.internal:host-gateway" yorkie: - image: 'yorkieteam/yorkie:0.4.5-alpha' + image: 'yorkieteam/yorkie:latest' container_name: 'yorkie' command: [ 'server', diff --git a/examples/texteditor/src/main/java/com/example/texteditor/EditorViewModel.kt b/examples/texteditor/src/main/java/com/example/texteditor/EditorViewModel.kt index 4bdeaacd2..665ed9a75 100644 --- a/examples/texteditor/src/main/java/com/example/texteditor/EditorViewModel.kt +++ b/examples/texteditor/src/main/java/com/example/texteditor/EditorViewModel.kt @@ -30,7 +30,8 @@ class EditorViewModel(private val client: Client) : ViewModel(), YorkieEditText. private val _content = MutableSharedFlow() val content = _content.asSharedFlow() - private val _textOperationInfos = MutableSharedFlow>() + private val _textOperationInfos = + MutableSharedFlow>() val textOpInfos = _textOperationInfos.asSharedFlow() val removedPeers = document.events.filterIsInstance() diff --git a/yorkie/proto/src/main/proto/yorkie/v1/resources.proto b/yorkie/proto/src/main/proto/yorkie/v1/resources.proto index fd09dd601..281b5906d 100644 --- a/yorkie/proto/src/main/proto/yorkie/v1/resources.proto +++ b/yorkie/proto/src/main/proto/yorkie/v1/resources.proto @@ -243,8 +243,9 @@ message TreeNode { string value = 3; TimeTicket removed_at = 4; TreeNodeID ins_prev_id = 5; - int32 depth = 6; - map attributes = 7; + TreeNodeID ins_next_id = 6; + int32 depth = 7; + map attributes = 8; } message TreeNodes { diff --git a/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt b/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt index 6bc93be34..7dde6b141 100644 --- a/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt +++ b/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt @@ -3,8 +3,8 @@ package dev.yorkie.core import androidx.test.ext.junit.runners.AndroidJUnit4 import dev.yorkie.assertJsonContentEquals import dev.yorkie.core.Client.DocumentSyncResult -import dev.yorkie.core.Client.Event.DocumentSynced import dev.yorkie.core.Client.Event.DocumentChanged +import dev.yorkie.core.Client.Event.DocumentSynced import dev.yorkie.core.Client.StreamConnectionStatus import dev.yorkie.document.Document import dev.yorkie.document.Document.Event.LocalChange diff --git a/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt b/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt index d97c7a272..d40b8bce7 100644 --- a/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt +++ b/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt @@ -67,8 +67,6 @@ class JsonTreeTest { ) assertTreesXmlEquals("

12

", d1, d2) - // TODO: add inserting on the leftmost after tree is fixed - updateAndSync( Updater(c1, d1) { root, _ -> root.rootTree().edit(2, 2, text { "A" }) @@ -118,7 +116,7 @@ class JsonTreeTest { updateAndSync( Updater(c1, d1) { root, _ -> - root.rootTree().style(6, 7, mapOf("bold" to "true")) + root.rootTree().style(0, 1, mapOf("bold" to "true")) }, Updater(c2, d2), ) @@ -130,6 +128,1333 @@ class JsonTreeTest { } } + @Test + fun test_deleting_overlapping_elements_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") + element("i") + element("b") + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(0, 4) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(2, 6) + }.await() + assertEquals("", d1.getRoot().rootTree().toXml()) + assertEquals("

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("", d1, d2) + } + } + + @Test + fun test_deleting_overlapping_text_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "abcd" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

abcd

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(1, 4) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(2, 5) + }.await() + assertEquals("

d

", d1.getRoot().rootTree().toXml()) + assertEquals("

a

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

", d1, d2) + } + } + + @Test + fun test_insert_and_delete_contained_elements_of_the_same_depth_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "1234" } + } + element("p") { + text { "abcd" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

1234

abcd

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(6, 6, element("p")) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(0, 12) + }.await() + assertEquals("

1234

abcd

", d1.getRoot().rootTree().toXml()) + assertEquals("", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

", d1, d2) + } + } + + @Test + fun test_multiple_insert_and_delete_contained_elements_of_the_same_depth_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "1234" } + } + element("p") { + text { "abcd" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

1234

abcd

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(6, 6, element("p")) + root.rootTree().edit(8, 8, element("p")) + root.rootTree().edit(10, 10, element("p")) + root.rootTree().edit(12, 12, element("p")) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(0, 12) + }.await() + assertEquals( + "

1234

abcd

", + d1.getRoot().rootTree().toXml(), + ) + assertEquals("", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

", d1, d2) + } + } + + @Test + fun test_detecting_error_when_inserting_and_deleting_contained_elements_at_different_depths() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + element("i") + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(2, 2, element("i")) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(1, 3) + }.await() + assertEquals("

", d1.getRoot().rootTree().toXml()) + assertEquals("

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

", d1, d2) + } + } + + @Test + fun test_deleting_contained_elements_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + element("i") { + text { "1234" } + } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

1234

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(0, 8) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(1, 7) + }.await() + assertEquals("", d1.getRoot().rootTree().toXml()) + assertEquals("

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("", d1, d2) + } + } + + @Test + fun test_insert_and_delete_contained_text_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "1234" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

1234

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(1, 5) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(3, 3, text { "a" }) + }.await() + assertEquals("

", d1.getRoot().rootTree().toXml()) + assertEquals("

12a34

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

a

", d1, d2) + } + } + + @Test + fun test_deleting_contained_text_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "1234" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

1234

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(1, 5) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(2, 4) + }.await() + assertEquals("

", d1.getRoot().rootTree().toXml()) + assertEquals("

14

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

", d1, d2) + } + } + + @Test + fun test_insert_and_delete_contained_text_and_elements_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "1234" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

1234

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(0, 6) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(3, 3, text { "a" }) + }.await() + assertEquals("", d1.getRoot().rootTree().toXml()) + assertEquals("

12a34

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("", d1, d2) + } + } + + @Test + fun test_delete_contained_text_and_elements_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "1234" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

1234

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(0, 6) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(1, 5) + }.await() + assertEquals("", d1.getRoot().rootTree().toXml()) + assertEquals("

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("", d1, d2) + } + } + + @Test + fun test_insert_side_by_side_elements_into_left_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(0, 0, element("b")) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(0, 0, element("i")) + }.await() + assertEquals("

", d1.getRoot().rootTree().toXml()) + assertEquals("

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

", d1, d2) + } + } + + @Test + fun test_insert_side_by_side_elements_into_middle_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(1, 1, element("b")) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(1, 1, element("i")) + }.await() + assertEquals("

", d1.getRoot().rootTree().toXml()) + assertEquals("

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

", d1, d2) + } + } + + @Test + fun test_insert_side_by_side_elements_into_right_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(2, 2, element("b")) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(2, 2, element("i")) + }.await() + assertEquals("

", d1.getRoot().rootTree().toXml()) + assertEquals("

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

", d1, d2) + } + } + + @Test + fun test_insert_and_delete_side_by_side_elements_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + element("b") + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(1, 3) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(1, 1, element("i")) + }.await() + assertEquals("

", d1.getRoot().rootTree().toXml()) + assertEquals("

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

", d1, d2) + } + } + + @Test + fun test_delete_and_insert_side_by_side_elements_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + element("b") + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(1, 3) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(3, 3, element("i")) + }.await() + assertEquals("

", d1.getRoot().rootTree().toXml()) + assertEquals("

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

", d1, d2) + } + } + + @Test + fun test_deleting_side_by_side_elements_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + element("b") + element("i") + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(1, 3) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(3, 5) + }.await() + assertEquals("

", d1.getRoot().rootTree().toXml()) + assertEquals("

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

", d1, d2) + } + } + + @Test + fun test_insert_text_to_the_same_left_position_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "12" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

12

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(1, 1, text { "A" }) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(1, 1, text { "B" }) + }.await() + assertEquals("

A12

", d1.getRoot().rootTree().toXml()) + assertEquals("

B12

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

BA12

", d1, d2) + } + } + + @Test + fun test_insert_text_to_the_same_middle_position_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "12" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

12

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(2, 2, text { "A" }) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(2, 2, text { "B" }) + }.await() + assertEquals("

1A2

", d1.getRoot().rootTree().toXml()) + assertEquals("

1B2

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

1BA2

", d1, d2) + } + } + + @Test + fun test_insert_text_to_the_same_right_position_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "12" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

12

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(3, 3, text { "A" }) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(3, 3, text { "B" }) + }.await() + assertEquals("

12A

", d1.getRoot().rootTree().toXml()) + assertEquals("

12B

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

12BA

", d1, d2) + } + } + + @Test + fun test_insert_and_delete_side_by_side_text_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "1234" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

1234

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(3, 3, text { "a" }) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(3, 5) + }.await() + assertEquals("

12a34

", d1.getRoot().rootTree().toXml()) + assertEquals("

12

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

12a

", d1, d2) + } + } + + @Test + fun test_delete_and_insert_side_by_side_text_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "1234" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

1234

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(3, 3, text { "a" }) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(1, 3) + }.await() + assertEquals("

12a34

", d1.getRoot().rootTree().toXml()) + assertEquals("

34

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

a34

", d1, d2) + } + } + + @Test + fun test_delete_side_by_side_text_blocks_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "1234" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

1234

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(3, 5) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(1, 3) + }.await() + assertEquals("

12

", d1.getRoot().rootTree().toXml()) + assertEquals("

34

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

", d1, d2) + } + } + + @Test + fun test_delete_text_content_at_the_same_left_position_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "123" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

123

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(1, 2) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(1, 2) + }.await() + assertEquals("

23

", d1.getRoot().rootTree().toXml()) + assertEquals("

23

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

23

", d1, d2) + } + } + + @Test + fun test_delete_text_content_at_the_same_middle_position_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "123" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

123

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(2, 3) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(2, 3) + }.await() + assertEquals("

13

", d1.getRoot().rootTree().toXml()) + assertEquals("

13

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

13

", d1, d2) + } + } + + @Test + fun test_delete_text_content_at_the_same_right_position_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "123" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

123

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(3, 4) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(3, 4) + }.await() + assertEquals("

12

", d1.getRoot().rootTree().toXml()) + assertEquals("

12

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

12

", d1, d2) + } + } + + @Test + fun test_delete_text_content_anchored_to_another_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "123" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

123

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(1, 2) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(2, 3) + }.await() + assertEquals("

23

", d1.getRoot().rootTree().toXml()) + assertEquals("

13

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

3

", d1, d2) + } + } + + @Test + fun test_producing_complete_deletion_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "123" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

123

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(1, 2) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(2, 4) + }.await() + assertEquals("

23

", d1.getRoot().rootTree().toXml()) + assertEquals("

1

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

", d1, d2) + } + } + + @Test + fun test_handling_block_delete_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "12345" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

12345

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(1, 3) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(4, 6) + }.await() + assertEquals("

345

", d1.getRoot().rootTree().toXml()) + assertEquals("

123

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

3

", d1, d2) + } + } + + @Test + fun test_handling_insertion_within_block_delete_concurrently_case1() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "12345" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

12345

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(2, 5) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(3, 3, text { "B" }) + }.await() + assertEquals("

15

", d1.getRoot().rootTree().toXml()) + assertEquals("

12B345

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

1B5

", d1, d2) + } + } + + @Test + fun test_handling_insertion_within_block_delete_concurrently_case2() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "12345" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

12345

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(2, 6) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(3, 3, text { "a" }, text { "bc" }) + }.await() + assertEquals("

1

", d1.getRoot().rootTree().toXml()) + assertEquals("

12abc345

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

1abc

", d1, d2) + } + } + + @Test + fun test_handling_block_element_insertion_within_deletion() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "1234" } + } + element("p") { + text { "5678" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

1234

5678

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(0, 12) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit( + 6, + 6, + element("p") { text { "cd" } }, + element("i") { text { "fg" } }, + ) + }.await() + assertEquals("", d1.getRoot().rootTree().toXml()) + assertEquals( + "

1234

cd

fg

5678

", + d2.getRoot().rootTree().toXml(), + ) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

cd

fg
", d1, d2) + } + } + + @Test + fun test_handling_concurrent_element_insertion_and_deletion_to_left() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "12345" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

12345

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(0, 7) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit( + 0, + 0, + element("p") { text { "cd" } }, + element("i") { text { "fg" } }, + ) + }.await() + assertEquals("", d1.getRoot().rootTree().toXml()) + assertEquals( + "

cd

fg

12345

", + d2.getRoot().rootTree().toXml(), + ) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

cd

fg
", d1, d2) + } + } + + @Test + fun test_handling_concurrent_element_insertion_and_deletion_to_right() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "12345" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

12345

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(0, 7) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit( + 7, + 7, + element("p") { text { "cd" } }, + element("i") { text { "fg" } }, + ) + }.await() + assertEquals("", d1.getRoot().rootTree().toXml()) + assertEquals( + "

12345

cd

fg
", + d2.getRoot().rootTree().toXml(), + ) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

cd

fg
", d1, d2) + } + } + + @Test + fun test_handling_deletion_of_insertion_anchor_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "12" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

12

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(2, 2, text { "A" }) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(1, 2) + }.await() + assertEquals("

1A2

", d1.getRoot().rootTree().toXml()) + assertEquals("

2

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

A2

", d1, d2) + } + } + + @Test + fun test_handling_deletion_after_insertion_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "12" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

12

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(1, 1, text { "A" }) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(1, 3) + }.await() + assertEquals("

A12

", d1.getRoot().rootTree().toXml()) + assertEquals("

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

A

", d1, d2) + } + } + + @Test + fun test_handling_deletion_before_insertion_concurrently() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("r") { + element("p") { + text { "12" } + } + }, + ) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

12

", d1, d2) + + d1.updateAsync { root, _ -> + root.rootTree().edit(3, 3, text { "A" }) + }.await() + d2.updateAsync { root, _ -> + root.rootTree().edit(1, 3) + }.await() + assertEquals("

12A

", d1.getRoot().rootTree().toXml()) + assertEquals("

", d2.getRoot().rootTree().toXml()) + + c1.syncAsync().await() + c2.syncAsync().await() + c1.syncAsync().await() + assertTreesXmlEquals("

A

", d1, d2) + } + } + + @Test + fun test_whether_split_link_can_be_transmitted_through_rpc() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + updateAndSync( + Updater(c1, d1) { root, _ -> + root.setNewTree( + "t", + element("doc") { + element("p") { + text { "ab" } + } + }, + ).edit(2, 2, text { "1" }) + }, + Updater(c2, d2), + ) + assertTreesXmlEquals("

a1b

", d1, d2) + + d2.updateAsync { root, _ -> + root.rootTree().edit(3, 3, text { "1" }) + }.await() + assertEquals("

a11b

", d2.getRoot().rootTree().toXml()) + + d2.updateAsync { root, _ -> + root.rootTree().apply { + edit(2, 3, text { "12" }) + edit(4, 5, text { "21" }) + } + }.await() + assertEquals("

a1221b

", d2.getRoot().rootTree().toXml()) + + // if split link is not transmitted, then left sibling in from index below, is "b" not "a" + d2.updateAsync { root, _ -> + root.rootTree().edit(2, 4, text { "123" }) + }.await() + assertEquals("

a12321b

", d2.getRoot().rootTree().toXml()) + } + } + + @Test + fun test_calculating_size_of_index_tree() { + withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ -> + d1.updateAsync { root, _ -> + root.setNewTree( + "t", + element("doc") { + element("p") { + text { "ab" } + } + }, + ).apply { + edit(2, 2, text { "123" }) + edit(2, 2, text { "456" }) + edit(2, 2, text { "789" }) + edit(2, 2, text { "0123" }) + } + }.await() + + assertEquals("

a0123789456123b

", d1.getRoot().rootTree().toXml()) + c1.syncAsync().await() + c2.syncAsync().await() + + val size = d1.getRoot().rootTree().indexTree.root.size + assertEquals(size, d2.getRoot().rootTree().indexTree.root.size) + } + } + companion object { fun JsonObject.rootTree() = getAs("t") diff --git a/yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt b/yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt index 46563de8b..4439e0d05 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt @@ -229,7 +229,7 @@ internal fun List.toCrdtTreeRootNode(): CrdtTreeNode? { internal fun PBTreeNode.toCrdtTreeNode(): CrdtTreeNode { val id = id.toCrdtTreeNodeID() - val convertedRemovedAt = removedAt.toTimeTicket() + val convertedRemovedAt = removedAtOrNull?.toTimeTicket() return if (type == IndexTreeNode.DEFAULT_TEXT_TYPE) { CrdtTreeText(id, value) } else { @@ -243,7 +243,13 @@ internal fun PBTreeNode.toCrdtTreeNode(): CrdtTreeNode { }, ) }.apply { - remove(convertedRemovedAt) + convertedRemovedAt?.let(::remove) + if (hasInsPrevId()) { + insPrevID = insPrevId.toCrdtTreeNodeID() + } + if (hasInsNextId()) { + insNextID = insNextId.toCrdtTreeNodeID() + } } } @@ -316,6 +322,12 @@ internal fun CrdtTreeNode.toPBTreeNodes(): List { if (node.isText) { value = node.value } + node.insPrevID?.let { + insPrevId = it.toPBTreeNodeID() + } + node.insNextID?.let { + insNextId = it.toPBTreeNodeID() + } node.removedAt?.toPBTimeTicket()?.let { removedAt = it } diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt index 5d94a7956..dccb4058d 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt @@ -29,7 +29,6 @@ internal class CrdtTree( private val removedNodeMap = mutableMapOf, CrdtTreeNode>() - init { indexTree.traverse { node, _ -> nodeMapByID[node.id] = node @@ -198,12 +197,12 @@ internal class CrdtTree( } } } - } - toBeRemoved.forEach { node -> - node.remove(executedAt) - if (node.isRemoved) { - removedNodeMap[node.createdAt to node.id.offset] = node + toBeRemoved.forEach { node -> + node.remove(executedAt) + if (node.isRemoved) { + removedNodeMap[node.createdAt to node.id.offset] = node + } } } @@ -267,15 +266,16 @@ internal class CrdtTree( val absOffset = leftSiblingNode.id.offset val split = leftSiblingNode.split(pos.leftSiblingID.offset - absOffset, absOffset) split?.let { - split.insPrev = leftSiblingNode + split.insPrevID = leftSiblingNode.id nodeMapByID[split.id] = split - leftSiblingNode.insNext?.apply { - insPrev = split - split.insNext = this + leftSiblingNode.insNextID?.let { insNextID -> + val insNext = findFloorNode(insNextID) + insNext?.insPrevID = split.id + split.insNextID = insNextID } - leftSiblingNode.insNext = split + leftSiblingNode.insNextID = split.id } } val index = if (parentNode == leftSiblingNode) { @@ -296,28 +296,26 @@ internal class CrdtTree( return parentNode to updatedLeftSiblingNode } + private fun findFloorNode(id: CrdtTreeNodeID): CrdtTreeNode? { + val (key, value) = nodeMapByID.floorEntry(id) ?: return null + return if (key == null || key.createdAt != id.createdAt) null else value + } + private fun toTreeNodes(pos: CrdtTreePos): Pair? { val parentID = pos.parentID val leftSiblingID = pos.leftSiblingID - val (parentKey, parentNode) = nodeMapByID.floorEntry(parentID) ?: return null - val (leftSiblingKey, leftSiblingNode) = nodeMapByID.floorEntry(leftSiblingID) ?: return null - - if (parentKey.createdAt != parentID.createdAt || - leftSiblingKey.createdAt != leftSiblingID.createdAt - ) { - return null - } + val parentNode = findFloorNode(parentID) ?: return null + val leftSiblingNode = findFloorNode(leftSiblingID) ?: return null val updatedLeftSiblingNode = - if (leftSiblingID.offset > 0 && leftSiblingID.offset == leftSiblingNode.offset && - leftSiblingNode.insPrev != null + if (leftSiblingID.offset > 0 && leftSiblingID.offset == leftSiblingNode.id.offset && + leftSiblingNode.insPrevID != null ) { - leftSiblingNode.insPrev + findFloorNode(requireNotNull(leftSiblingNode.insPrevID)) ?: leftSiblingNode } else { leftSiblingNode } - - return requireNotNull(parentNode) to requireNotNull(updatedLeftSiblingNode) + return parentNode to updatedLeftSiblingNode } /** @@ -374,14 +372,20 @@ internal class CrdtTree( * Physically deletes the given [node] from [IndexTree]. */ private fun delete(node: CrdtTreeNode) { - val insPrev = node.insPrev - val insNext = node.insNext + val insPrevID = node.insPrevID + val insNextID = node.insNextID - insPrev?.insNext = insNext - insNext?.insPrev = insPrev + insPrevID?.let { + val insPrev = findFloorNode(it) + insPrev?.insNextID = insNextID + } + insNextID?.let { + val insNext = findFloorNode(it) + insNext?.insPrevID = insPrevID + } - node.insPrev = null - node.insNext = null + node.insPrevID = null + node.insNextID = null } /** @@ -560,9 +564,9 @@ internal data class CrdtTreeNode private constructor( size = value.length } - var insPrev: CrdtTreeNode? = null + var insPrevID: CrdtTreeNodeID? = null - var insNext: CrdtTreeNode? = null + var insNextID: CrdtTreeNodeID? = null val rhtNodes: List get() = _attributes.toList() @@ -579,7 +583,9 @@ internal data class CrdtTreeNode private constructor( id = CrdtTreeNodeID(id.createdAt, offset), _value = null, childNodes = mutableListOf(), - ) + ).also { + it.removedAt = this.removedAt + } } fun setAttribute(key: String, value: String, executedAt: TimeTicket) { @@ -629,16 +635,16 @@ internal data class CrdtTreeNode private constructor( @Suppress("FunctionName") companion object { - fun CrdtTreeText(pos: CrdtTreeNodeID, value: String): CrdtTreeNode { - return CrdtTreeNode(pos, DEFAULT_TEXT_TYPE, value) + fun CrdtTreeText(id: CrdtTreeNodeID, value: String): CrdtTreeNode { + return CrdtTreeNode(id, DEFAULT_TEXT_TYPE, value) } fun CrdtTreeElement( - pos: CrdtTreeNodeID, + id: CrdtTreeNodeID, type: String, children: List = emptyList(), attributes: Rht = Rht(), - ) = CrdtTreeNode(pos, type, null, children.toMutableList(), attributes) + ) = CrdtTreeNode(id, type, null, children.toMutableList(), attributes) } } @@ -657,7 +663,6 @@ public data class CrdtTreePos internal constructor( } } - /** * [CrdtTreeNodeID] represent an ID of a node in the tree. It is used to * identify a node in the tree. It is composed of the creation time of the node diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonElement.kt b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonElement.kt index 121fe13d9..b85db0e34 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonElement.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonElement.kt @@ -31,6 +31,7 @@ public abstract class JsonElement { CrdtText::class.java to JsonText::class.java, CrdtPrimitive::class.java to JsonPrimitive::class.java, CrdtCounter::class.java to JsonCounter::class.java, + CrdtTree::class.java to JsonTree::class.java, ) internal inline fun CrdtElement.toJsonElement( diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonTree.kt b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonTree.kt index 5bb392ac2..2288576eb 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonTree.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonTree.kt @@ -113,12 +113,11 @@ public class JsonTree internal constructor( if (contents.isNotEmpty()) { validateTreeNodes(contents) if (contents.first().type != DEFAULT_TEXT_TYPE) { - val children = - contents.filterIsInstance().flatMap(ElementNode::children) - validateTreeNodes(children) + contents.forEach { validateTreeNodes(listOf(it))} } } + val ticket = context.lastTimeTicket val crdtNodes = if (contents.firstOrNull()?.type == DEFAULT_TEXT_TYPE) { val compVal = contents .filterIsInstance() @@ -127,7 +126,6 @@ public class JsonTree internal constructor( } else { contents.map { createCrdtTreeNode(context, it) } } - val ticket = context.lastTimeTicket val (_, maxCreatedAtMapByActor) = target.edit( fromPos to toPos, crdtNodes.map { it.deepCopy() }.ifEmpty { null }, @@ -240,12 +238,12 @@ public class JsonTree internal constructor( * Returns the root node of this tree. */ internal fun buildRoot(initialRoot: ElementNode?, context: ChangeContext): CrdtTreeNode { - val pos = CrdtTreeNodeID(context.issueTimeTicket(), 0) + val id = CrdtTreeNodeID(context.issueTimeTicket(), 0) if (initialRoot == null) { - return CrdtTreeElement(pos, DEFAULT_ROOT_TYPE) + return CrdtTreeElement(id, DEFAULT_ROOT_TYPE) } // TODO(hackerwins): Need to use the ticket of operation of creating tree. - return CrdtTreeElement(pos, initialRoot.type).also { root -> + return CrdtTreeElement(id, initialRoot.type).also { root -> initialRoot.children.forEach { child -> buildDescendants(child, root, context) } @@ -259,11 +257,11 @@ public class JsonTree internal constructor( ) { val type = treeNode.type val ticket = context.issueTimeTicket() - val pos = CrdtTreeNodeID(ticket, 0) + val id = CrdtTreeNodeID(ticket, 0) when (treeNode) { is TextNode -> { - val textNode = CrdtTreeText(pos, treeNode.value) + val textNode = CrdtTreeText(id, treeNode.value) parent.append(textNode) return } @@ -277,7 +275,7 @@ public class JsonTree internal constructor( attrs.set(key, value, ticket) } } - val elementNode = CrdtTreeElement(pos, type, attributes = attrs) + val elementNode = CrdtTreeElement(id, type, attributes = attrs) parent.append(elementNode) treeNode.children.forEach { child -> buildDescendants(child, elementNode, context) @@ -291,16 +289,16 @@ public class JsonTree internal constructor( */ private fun createCrdtTreeNode(context: ChangeContext, content: TreeNode): CrdtTreeNode { val ticket = context.issueTimeTicket() - val pos = CrdtTreeNodeID(ticket, 0) + val id = CrdtTreeNodeID(ticket, 0) return when (content) { is TextNode -> { - CrdtTreeText(pos, content.value) + CrdtTreeText(id, content.value) } is ElementNode -> { CrdtTreeElement( - pos, + id, content.type, attributes = Rht().apply { content.attributes.forEach { (key, value) -> diff --git a/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt b/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt index 49a5b6b88..3e55e9cb9 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt @@ -516,7 +516,10 @@ internal abstract class IndexTreeNode>(children: MutableLis _children.addAll(0, newNode.toList()) newNode.forEach { node -> node.parent = this as T - node.updateAncestorSize() + + if (!node.isRemoved) { + node.updateAncestorSize() + } } } diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTreeTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTreeTest.kt index 4d9c225a1..e266d0cd1 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTreeTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTreeTest.kt @@ -105,12 +105,12 @@ class CrdtTreeTest { // 0 1 2 3 4 5 6 7 //

a

c d

target.editByIndex(2 to 6, null, issueTime()) - // TODO(7hong13): should be resolved after the JS SDK fix + // TODO(7hong13): should be resolved after the JS SDK implementation // assertEquals("root>

ad

", target.toXml()) // 03. insert a new text node at the start of the first paragraph. target.editByIndex(1 to 1, CrdtTreeText(issuePos(), "@").toList(), issueTime()) - // TODO(7hong13): should be resolved after the JS SDK fix + // TODO(7hong13): should be resolved after the JS SDK implementation // assertEquals("root>

@ad

", target.toXml()) } @@ -170,7 +170,7 @@ class CrdtTreeTest { @Test fun `should merge and edit different levels with editByIndex`() { - // TODO(7hong13): should be resolved after the JS SDK fix + // TODO(7hong13): should be resolved after the JS SDK implementation // fun initializeTree() { // setUp() // target.editByIndex(0 to 0, CrdtTreeElement(issuePos(), "p").toList(), issueTime()) diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt index e3233cef6..538580e20 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt @@ -430,6 +430,238 @@ class JsonTreeTest { assertEquals(5 to 7, tree.posRangeToIndexRange(posRange)) } + @Test + fun `should find pos range from path and vice versa`() = runTest { + val document = Document(Document.Key("")) + fun JsonObject.tree() = getAs("t") + + document.updateAsync { root, _ -> + root.setNewTree( + "t", + element("root") { + element("p") { + element("b") { + element("i") { + text { "ab" } + } + } + } + }, + ) + }.await() + assertEquals( + """

ab

""", + document.getRoot().tree().toXml(), + ) + + val tree = document.getRoot().tree() + var range = tree.pathRangeToPosRange(listOf(0) to listOf(0, 0, 0, 2)) + assertEquals(listOf(0) to listOf(0, 0, 0, 2), tree.posRangeToPathRange(range)) + + range = tree.pathRangeToPosRange(listOf(0) to listOf(1)) + assertEquals(listOf(0) to listOf(1), tree.posRangeToPathRange(range)) + } + + @Test + fun `should insert multiple text nodes`() = runTest { + val document = Document(Document.Key("")) + fun JsonObject.tree() = getAs("t") + + document.updateAsync { root, _ -> + root.setNewTree( + "t", + element("doc") { + element("p") { + text { "ab" } + } + }, + ) + }.await() + assertEquals("

ab

", document.getRoot().tree().toXml()) + + document.updateAsync { root, _ -> + root.tree().edit(3, 3, text { "c" }, text { "d" }) + }.await() + assertEquals("

abcd

", document.getRoot().tree().toXml()) + } + + @Test + fun `should insert multiple element nodes`() = runTest { + val document = Document(Document.Key("")) + fun JsonObject.tree() = getAs("t") + + document.updateAsync { root, _ -> + root.setNewTree( + "t", + element("doc") { + element("p") { + text { "ab" } + } + }, + ) + }.await() + assertEquals("

ab

", document.getRoot().tree().toXml()) + + document.updateAsync { root, _ -> + root.tree().edit( + 4, + 4, + element("p") { text { "cd" } }, + element("i") { text { "fg" } }, + ) + }.await() + assertEquals("

ab

cd

fg
", document.getRoot().tree().toXml()) + } + + @Suppress("ktlint:standard:max-line-length") + @Test + fun `should edit content with path when multi tree nodes passed`() = runTest { + val document = Document(Document.Key("")) + fun JsonObject.tree() = getAs("t") + + document.updateAsync { root, _ -> + root.setNewTree( + "t", + element("doc") { + element("tc") { + element("p") { + element("tn") { + text { "ab" } + } + } + } + }, + ) + }.await() + assertEquals("

ab

", document.getRoot().tree().toXml()) + + document.getRoot().tree().editByPath( + listOf(0, 0, 0, 1), + listOf(0, 0, 0, 1), + text { "X" }, + text { "X" }, + ) + assertEquals("

aXXb

", document.getRoot().tree().toXml()) + + document.getRoot().tree().editByPath( + listOf(0, 1), + listOf(0, 1), + element("p") { + element("tn") { + text { "te" } + text { "st" } + } + }, + element("p") { + element("tn") { + text { "te" } + text { "xt" } + } + }, + ) + assertEquals( + "

aXXb

test

text

", + document.getRoot().tree().toXml(), + ) + + // TODO(7hong13): the test should be passed when text contents values are "test" and "text" + document.getRoot().tree().editByPath( + listOf(0, 3), + listOf(0, 3), + element("p") { + element("tn") { + text { "12" } + text { "34" } + } + }, + element("tn") { + text { "56" } + text { "78" } + }, + ) + assertEquals( + "

aXXb

test

text

1234

5678
", + document.getRoot().tree().toXml(), + ) + } + + @Test + fun `should delete the first text with tombstone in front of target text`() = runTest { + val document = Document(Document.Key("")) + fun JsonObject.tree() = getAs("t") + + document.updateAsync { root, _ -> + root.setNewTree("t").edit( + 0, 0, element("p") { text { "abcdefghi" } }, + ) + assertEquals("

abcdefghi

", document.getRoot().tree().toXml()) + + root.tree().edit(1, 1, text { "12345" }) + assertEquals("

12345abcdefghi

", root.tree().toXml()) + + root.tree().edit(2, 5) + assertEquals("

15abcdefghi

", root.tree().toXml()) + + root.tree().edit(3, 5) + assertEquals("

15cdefghi

", root.tree().toXml()) + + root.tree().edit(2, 4) + assertEquals("

1defghi

", root.tree().toXml()) + + root.tree().edit(1, 3) + assertEquals("

efghi

", root.tree().toXml()) + + root.tree().edit(1, 2) + assertEquals("

fghi

", root.tree().toXml()) + + root.tree().edit(2, 5) + assertEquals("

f

", root.tree().toXml()) + + root.tree().edit(1, 2) + assertEquals("

", root.tree().toXml()) + }.await() + } + + @Test + fun `should delete node with a text node in front whose size is bigger than 1`() = runTest { + val document = Document(Document.Key("")) + fun JsonObject.tree() = getAs("t") + + document.updateAsync { root, _ -> + root.setNewTree("t").edit( + 0, 0, element("p") { text { "abcde" } }, + ) + assertEquals("

abcde

", document.getRoot().tree().toXml()) + + root.tree().edit(6, 6, text { "f" }) + assertEquals("

abcdef

", root.tree().toXml()) + + root.tree().edit(7, 7, text { "g" }) + assertEquals("

abcdefg

", root.tree().toXml()) + + root.tree().edit(7, 8) + assertEquals("<

abcdef

", root.tree().toXml()) + + root.tree().edit(6, 7) + assertEquals("<

abcde

", root.tree().toXml()) + + root.tree().edit(5, 6) + assertEquals("<

abcd

", root.tree().toXml()) + + root.tree().edit(4, 5) + assertEquals("<

abc

", root.tree().toXml()) + + root.tree().edit(3, 4) + assertEquals("<

ab

", root.tree().toXml()) + + root.tree().edit(2, 3) + assertEquals("<

a

", root.tree().toXml()) + + root.tree().edit(1, 2) + assertEquals("<

", root.tree().toXml()) + }.await() + } + companion object { private val DummyContext = ChangeContext( ChangeID.InitialChangeID, From b6c64462f0a39af2ac1b9d78474e21550b1202cd Mon Sep 17 00:00:00 2001 From: jee-hyun-kim Date: Wed, 23 Aug 2023 16:08:33 +0900 Subject: [PATCH 06/13] format fixed --- .../src/main/kotlin/dev/yorkie/document/json/JsonTree.kt | 2 +- .../test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonTree.kt b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonTree.kt index 2288576eb..67ad12a3b 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonTree.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonTree.kt @@ -113,7 +113,7 @@ public class JsonTree internal constructor( if (contents.isNotEmpty()) { validateTreeNodes(contents) if (contents.first().type != DEFAULT_TEXT_TYPE) { - contents.forEach { validateTreeNodes(listOf(it))} + contents.forEach { validateTreeNodes(listOf(it)) } } } diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt index 538580e20..8c85dbb05 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt @@ -592,7 +592,9 @@ class JsonTreeTest { document.updateAsync { root, _ -> root.setNewTree("t").edit( - 0, 0, element("p") { text { "abcdefghi" } }, + 0, + 0, + element("p") { text { "abcdefghi" } }, ) assertEquals("

abcdefghi

", document.getRoot().tree().toXml()) @@ -629,7 +631,9 @@ class JsonTreeTest { document.updateAsync { root, _ -> root.setNewTree("t").edit( - 0, 0, element("p") { text { "abcde" } }, + 0, + 0, + element("p") { text { "abcde" } }, ) assertEquals("

abcde

", document.getRoot().tree().toXml()) From fd794f694fb3b976720c2d5d482a5ae57aa5bbd9 Mon Sep 17 00:00:00 2001 From: jee-hyun-kim Date: Wed, 23 Aug 2023 17:36:27 +0900 Subject: [PATCH 07/13] update github action --- .github/workflows/ci.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ac446a0f1..d082c8968 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,8 +62,7 @@ jobs: - run: chmod +x gradlew - uses: gradle/gradle-build-action@v2 - name: Setup Docker on macOS - uses: 7hong13/setup-docker-macos-action@fix_formula_paths - id: docker + uses: douglascamata/setup-docker-macos-action@v1-alpha - run: docker-compose -f docker/docker-compose-ci.yml up --build -d - uses: actions/cache@v3 id: avd-cache From 627fd00f3587ec5c0b566db4df2e896eccaaf4f5 Mon Sep 17 00:00:00 2001 From: jee-hyun-kim Date: Wed, 23 Aug 2023 23:53:03 +0900 Subject: [PATCH 08/13] fix test failures --- .github/workflows/ci.yml | 4 +- .../dev/yorkie/document/json/JsonTreeTest.kt | 2 + .../dev/yorkie/document/json/JsonObject.kt | 2 +- .../dev/yorkie/document/json/JsonTreeTest.kt | 97 ++++++++++--------- 4 files changed, 57 insertions(+), 48 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d082c8968..dd6eff7a1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -93,7 +93,9 @@ jobs: ram-size: 4096M emulator-boot-timeout: 12000 disable-animations: true - script: ./gradlew yorkie:connectedCheck --no-build-cache --no-daemon --stacktrace + script: | + ./gradlew yorkie:connectedCheck -Pandroid.testInstrumentationRunnerArguments.notAnnotation=androidx.test.filters.LargeTest --no-build-cache --no-daemon --stacktrace + ./gradlew yorkie:connectedCheck -Pandroid.testInstrumentationRunnerArguments.annotation=androidx.test.filters.LargeTest --no-build-cache --no-daemon --stacktrace - if: ${{ matrix.api-level == 30 }} run: ./gradlew yorkie:jacocoDebugTestReport - if: ${{ matrix.api-level == 30 }} diff --git a/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt b/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt index d40b8bce7..f6cb811aa 100644 --- a/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt +++ b/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeTest.kt @@ -1,6 +1,7 @@ package dev.yorkie.document.json import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.filters.LargeTest import dev.yorkie.core.Client import dev.yorkie.core.Presence import dev.yorkie.core.withTwoClientsAndDocuments @@ -12,6 +13,7 @@ import org.junit.Test import org.junit.runner.RunWith import kotlin.test.assertEquals +@LargeTest @RunWith(AndroidJUnit4::class) class JsonTreeTest { diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonObject.kt b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonObject.kt index c140f7d55..11433d83e 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonObject.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonObject.kt @@ -113,7 +113,7 @@ public class JsonObject internal constructor( val ticket = context.issueTimeTicket() val tree = CrdtTree(JsonTree.buildRoot(initialRoot, context), ticket) setAndRegister(key, tree) - return JsonTree(context, tree) + return tree.toJsonElement(context) } private fun setAndRegister(key: String, element: CrdtElement) { diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt index 8c85dbb05..1b808b296 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt @@ -532,57 +532,62 @@ class JsonTreeTest { } }, ) - }.await() - assertEquals("

ab

", document.getRoot().tree().toXml()) + assertEquals( + "

ab

", + document.getRoot().tree().toXml(), + ) - document.getRoot().tree().editByPath( - listOf(0, 0, 0, 1), - listOf(0, 0, 0, 1), - text { "X" }, - text { "X" }, - ) - assertEquals("

aXXb

", document.getRoot().tree().toXml()) + root.tree().editByPath( + listOf(0, 0, 0, 1), + listOf(0, 0, 0, 1), + text { "X" }, + text { "X" }, + ) + assertEquals( + "

aXXb

", + document.getRoot().tree().toXml(), + ) - document.getRoot().tree().editByPath( - listOf(0, 1), - listOf(0, 1), - element("p") { - element("tn") { - text { "te" } - text { "st" } - } - }, - element("p") { + root.tree().editByPath( + listOf(0, 1), + listOf(0, 1), + element("p") { + element("tn") { + text { "te" } + text { "st" } + } + }, + element("p") { + element("tn") { + text { "te" } + text { "xt" } + } + }, + ) + assertEquals( + "

aXXb

test

text

", + document.getRoot().tree().toXml(), + ) + + root.tree().editByPath( + listOf(0, 3), + listOf(0, 3), + element("p") { + element("tn") { + text { "te" } + text { "st" } + } + }, element("tn") { text { "te" } text { "xt" } - } - }, - ) - assertEquals( - "

aXXb

test

text

", - document.getRoot().tree().toXml(), - ) - - // TODO(7hong13): the test should be passed when text contents values are "test" and "text" - document.getRoot().tree().editByPath( - listOf(0, 3), - listOf(0, 3), - element("p") { - element("tn") { - text { "12" } - text { "34" } - } - }, - element("tn") { - text { "56" } - text { "78" } - }, - ) - assertEquals( - "

aXXb

test

text

1234

5678
", - document.getRoot().tree().toXml(), - ) + }, + ) + assertEquals( + "

aXXb

test

text

test

text
", + document.getRoot().tree().toXml(), + ) + }.await() } @Test From 820cacd6de2728ed44c005bf25546102a8535a35 Mon Sep 17 00:00:00 2001 From: jee-hyun-kim Date: Thu, 24 Aug 2023 02:16:24 +0900 Subject: [PATCH 09/13] improve TC on presences --- .../kotlin/dev/yorkie/core/PresenceTest.kt | 75 +++++++++++++------ .../kotlin/dev/yorkie/document/Document.kt | 2 +- 2 files changed, 55 insertions(+), 22 deletions(-) diff --git a/yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt b/yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt index bd636b1d3..161e22a06 100644 --- a/yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt +++ b/yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt @@ -84,22 +84,42 @@ class PresenceTest { @Test fun test_presence_sync() { - withTwoClientsAndDocuments( - presences = mapOf("name" to "a") to mapOf("name" to "b"), - ) { c1, c2, d1, d2, _ -> + runBlocking { + val c1 = createClient() + val c2 = createClient() + val documentKey = UUID.randomUUID().toString().toDocKey() + val d1 = Document(documentKey) + val d2 = Document(documentKey) val d1Events = mutableListOf() val d2Events = mutableListOf() - val collectJobs = listOf( - launch(start = CoroutineStart.UNDISPATCHED) { - d1.events.filterIsInstance() - .filterNot { it is MyPresence.Initialized } - .collect(d1Events::add) - }, - launch(start = CoroutineStart.UNDISPATCHED) { - d2.events.filterIsInstance() - .filterNot { it is MyPresence.Initialized } - .collect(d2Events::add) - }, + + c1.activateAsync().await() + c2.activateAsync().await() + + c1.attachAsync(d1, initialPresence = mapOf("name" to "a")).await() + val d1Job = launch(start = CoroutineStart.UNDISPATCHED) { + d1.events.filterIsInstance() + .filterNot { it is MyPresence.Initialized } + .collect(d1Events::add) + } + + c2.attachAsync(d2, initialPresence = mapOf("name" to "b")).await() + val d2Job = launch(start = CoroutineStart.UNDISPATCHED) { + d2.events.filterIsInstance() + .filterNot { it is MyPresence.Initialized } + .collect(d2Events::add) + } + + withTimeout(GENERAL_TIMEOUT) { + // watched from c2 + while (d1Events.isEmpty()) { + delay(50) + } + } + + assertEquals( + Others.Watched(PresenceInfo(c2.requireClientId(), mapOf("name" to "b"))), + d1Events.last(), ) d1.updateAsync { _, presence -> @@ -110,7 +130,7 @@ class PresenceTest { }.await() withTimeout(GENERAL_TIMEOUT) { - while (d1Events.size < 3 || d2Events.size < 2) { + while (d1Events.size < 3) { delay(50) } } @@ -120,13 +140,19 @@ class PresenceTest { MyPresence.PresenceChanged( PresenceInfo(c1.requireClientId(), mapOf("name" to "A")), ), - Others.Watched(PresenceInfo(c2.requireClientId(), mapOf("name" to "b"))), Others.PresenceChanged( PresenceInfo(c2.requireClientId(), mapOf("name" to "B")), ), ), - d1Events, + d1Events.takeLast(2), ) + + withTimeout(GENERAL_TIMEOUT) { + while (d2Events.size < 2) { + delay(50) + } + } + assertEquals( listOf( MyPresence.PresenceChanged( @@ -141,9 +167,14 @@ class PresenceTest { ), d2Events, ) - assertEquals(d1.presences.value.entries, d2.presences.value.entries) + assertEquals(d1.presences.value.toMap(), d2.presences.value.toMap()) - collectJobs.forEach(Job::cancel) + d1Job.cancel() + d2Job.cancel() + c1.detachAsync(d1).await() + c2.detachAsync(d2).await() + c1.deactivateAsync().await() + c2.deactivateAsync().await() } } @@ -412,7 +443,8 @@ class PresenceTest { val c2ID = c2.requireClientId() val c3ID = c3.requireClientId() - c1.attachAsync(d1, initialPresence = mapOf("name" to "a1", "cursor" to cursor)).await() + c1.attachAsync(d1, initialPresence = mapOf("name" to "a1", "cursor" to cursor)) + .await() val d1CollectJob = launch(start = CoroutineStart.UNDISPATCHED) { d1.events.filterIsInstance() @@ -421,7 +453,8 @@ class PresenceTest { // 01. c2 attaches doc in realtime sync, and c3 attached doc in manual sync. // c1 receives the watched event from c2. - c2.attachAsync(d2, initialPresence = mapOf("name" to "b1", "cursor" to cursor)).await() + c2.attachAsync(d2, initialPresence = mapOf("name" to "b1", "cursor" to cursor)) + .await() c3.attachAsync(d3, mapOf("name" to "c1", "cursor" to cursor), false).await() withTimeout(GENERAL_TIMEOUT) { diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt b/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt index 8e8e0230e..6b4399439 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt @@ -299,8 +299,8 @@ public class Document(public val key: Key) { eventStream.emit(Event.RemoteChange(change.toChangeInfo(opInfos))) } - presenceEvent?.let { eventStream.emit(it) } newPresences?.let { emitPresences(it) } + presenceEvent?.let { eventStream.emit(it) } changeID = changeID.syncLamport(change.id.lamport) } } From 575251b15aacd0d708444ba37e2aadd6bf6b37e1 Mon Sep 17 00:00:00 2001 From: jee-hyun-kim Date: Sun, 27 Aug 2023 23:00:55 +0900 Subject: [PATCH 10/13] Support multi-level and partial element selection --- .../dev/yorkie/document/crdt/CrdtTree.kt | 107 ++++++++---------- .../main/kotlin/dev/yorkie/util/IndexTree.kt | 24 +++- .../dev/yorkie/document/crdt/CrdtTreeTest.kt | 2 +- .../dev/yorkie/document/json/JsonTreeTest.kt | 45 +++++++- .../kotlin/dev/yorkie/util/IndexTreeTest.kt | 16 ++- 5 files changed, 121 insertions(+), 73 deletions(-) diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt index dccb4058d..c94869b2c 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt @@ -11,6 +11,7 @@ import dev.yorkie.document.time.TimeTicket.Companion.MaxTimeTicket import dev.yorkie.document.time.TimeTicket.Companion.compareTo import dev.yorkie.util.IndexTree import dev.yorkie.util.IndexTreeNode +import dev.yorkie.util.TagContained import dev.yorkie.util.TreePos import java.util.TreeMap @@ -58,28 +59,21 @@ internal class CrdtTree( from = toIndex(fromParent, fromLeft), to = toIndex(toParent, toLeft), fromPath = toPath(fromParent, fromLeft), - toPath = toPath(fromParent, fromLeft), + toPath = toPath(toParent, toLeft), actorID = executedAt.actorID, attributes = attributes, ), ) - if (fromLeft != toLeft) { - val (parent, fromChildIndex) = if (fromLeft.parent == toLeft.parent) { - val leftParent = fromLeft.parent ?: return changes - leftParent to leftParent.allChildren.indexOf(fromLeft) + 1 - } else { - fromLeft to 0 - } - - val toChildIndex = parent.allChildren.indexOf(toLeft) - - for (i in fromChildIndex..toChildIndex) { - val node = parent.allChildren[i] - if (!node.isRemoved && attributes != null) { - attributes.forEach { (key, value) -> - node.setAttribute(key, value, executedAt) - } + traverseInPosRange( + fromParent = fromParent, + fromLeft = fromLeft, + toParent = toParent, + toLeft = toLeft, + ) { node, _ -> + if (!node.isRemoved && attributes != null && !node.isText) { + attributes.forEach { (key, value) -> + node.setAttribute(key, value, executedAt) } } } @@ -155,54 +149,39 @@ internal class CrdtTree( val toBeRemoved = mutableListOf() val latestCreatedAtMap = mutableMapOf() - // 02. remove the nodes and update linked list and index tree. - if (fromLeft != toLeft) { - val (parent, fromChildIndex) = if (fromLeft.parent == toLeft.parent) { - val leftParent = requireNotNull(fromLeft.parent) - leftParent to leftParent.allChildren.indexOf(fromLeft) + 1 - } else { - fromLeft to 0 + traverseInPosRange( + fromParent = fromParent, + fromLeft = fromLeft, + toParent = toParent, + toLeft = toLeft, + ) { node, contained -> + // If node is a element node and half-contained in the range, + // it should not be removed. + if (!node.isText && contained != TagContained.All) { + return@traverseInPosRange } - val toChildIndex = parent.allChildren.indexOf(toLeft) - - for (i in fromChildIndex..toChildIndex) { - val node = parent.allChildren[i] - val actorID = node.createdAt.actorID - val latestCreatedAt = latestCreatedAtMapByActor?.let { - latestCreatedAtMapByActor[actorID] ?: InitialTimeTicket - } ?: MaxTimeTicket + val actorID = node.createdAt.actorID + val latestCreatedAt = latestCreatedAtMapByActor?.let { + latestCreatedAtMapByActor[actorID] ?: InitialTimeTicket + } ?: MaxTimeTicket - if (node.canDelete(executedAt, latestCreatedAt)) { - val latest = latestCreatedAtMap[actorID] - val createdAt = node.createdAt - - if (latest == null || latest < createdAt) { - latestCreatedAtMap[actorID] = createdAt - } + if (node.canDelete(executedAt, latestCreatedAt)) { + val latest = latestCreatedAtMap[actorID] + val createdAt = node.createdAt - traverseAll(node, 0) { treeNode, _ -> - if (treeNode.canDelete(executedAt, MaxTimeTicket)) { - val latestTicket = latestCreatedAtMap[actorID] - val ticket = treeNode.createdAt - - if (latestTicket == null || latestTicket < ticket) { - latestCreatedAtMap[actorID] = ticket - } - - if (!treeNode.isRemoved) { - toBeRemoved.add(treeNode) - } - } - } + if (latest == null || latest < createdAt) { + latestCreatedAtMap[actorID] = createdAt } + + toBeRemoved.add(node) } + } - toBeRemoved.forEach { node -> - node.remove(executedAt) - if (node.isRemoved) { - removedNodeMap[node.createdAt to node.id.offset] = node - } + toBeRemoved.forEach { node -> + node.remove(executedAt) + if (node.isRemoved) { + removedNodeMap[node.createdAt to node.id.offset] = node } } @@ -246,6 +225,18 @@ internal class CrdtTree( action.invoke(node, depth) } + private fun traverseInPosRange( + fromParent: CrdtTreeNode, + fromLeft: CrdtTreeNode, + toParent: CrdtTreeNode, + toLeft: CrdtTreeNode, + callback: (CrdtTreeNode, TagContained) -> Unit, + ) { + val fromIndex = toIndex(fromParent, fromLeft) + val toIndex = toIndex(toParent, toLeft) + indexTree.nodesBetween(fromIndex, toIndex, callback) + } + /** * Finds [TreePos] of the given [pos] and splits the text node if necessary. * diff --git a/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt b/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt index 3e55e9cb9..583fba1dc 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/util/IndexTree.kt @@ -67,7 +67,7 @@ internal class IndexTree>(val root: T) { fun nodesBetween( from: Int, to: Int, - action: (T) -> Unit, + action: (T, TagContained) -> Unit, ) { nodesBetweenInternal(root, from, to, action) } @@ -76,12 +76,13 @@ internal class IndexTree>(val root: T) { * Iterates the nodes between the given range. * If the given range is collapsed, the callback is not called. * It traverses the tree with postorder traversal. + * NOTE(sejongk): Nodes should not be removed in callback, because it leads to wrong behaviors. */ private fun nodesBetweenInternal( root: T, from: Int, to: Int, - action: ((T) -> Unit), + action: ((T, TagContained) -> Unit), ) { if (from > to) { throw IllegalArgumentException("from is greater than to: $from > $to") @@ -113,7 +114,12 @@ internal class IndexTree>(val root: T) { // If the range spans outside the child, // the callback is called with the child. if (fromChild < 0 || toChild > child.size || child.isText) { - action.invoke(child) + val contained = when { + (fromChild < 0 && toChild > child.size) || child.isText -> TagContained.All + fromChild < 0 -> TagContained.Opening + else -> TagContained.Closing + } + action.invoke(child, contained) } } pos += child.paddedSize @@ -344,12 +350,19 @@ internal class IndexTree>(val root: T) { /** * Returns the path of the given [index]. */ - public fun indexToPath(index: Int): List { + fun indexToPath(index: Int): List { val treePos = findTreePos(index) return treePosToPath(treePos) } } +/** + * [TagContained] represents whether the opening or closing tag of a element is selected. + */ +internal enum class TagContained { + All, Opening, Closing, +} + /** * [TreePos] is the position of a node in the tree. * @@ -460,8 +473,7 @@ internal abstract class IndexTreeNode>(children: MutableLis // If nodes are removed, the offset of the removed node is the number of // nodes before the node excluding the removed nodes. - val refined = allChildren.take(index).filterNot { it.isRemoved }.size - 1 - return refined.coerceAtLeast(0) + return allChildren.take(index).filterNot { it.isRemoved }.size } return children.indexOf(node) } diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTreeTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTreeTest.kt index e266d0cd1..9b1752734 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTreeTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTreeTest.kt @@ -106,7 +106,7 @@ class CrdtTreeTest { //

a

c d

target.editByIndex(2 to 6, null, issueTime()) // TODO(7hong13): should be resolved after the JS SDK implementation - // assertEquals("root>

ad

", target.toXml()) + // assertEquals("

ad

", target.toXml()) // 03. insert a new text node at the start of the first paragraph. target.editByIndex(1 to 1, CrdtTreeText(issuePos(), "@").toList(), issueTime()) diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt index 1b808b296..fef179425 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTreeTest.kt @@ -319,11 +319,12 @@ class JsonTreeTest { listOf(TreeNode("text", value = "X")), "$.t", ), + // TODO(7hong13): need to check whether toPath is correctly passed TreeStyleOpInfo( 0, 1, listOf(0), - listOf(0), + listOf(0, 0), mapOf("a" to "b"), "$.t", ), @@ -385,11 +386,12 @@ class JsonTreeTest { listOf(TreeNode("text", value = "X")), "$.t", ), + // TODO(7hong13): need to check whether toPath is correctly passed TreeStyleOpInfo( 2, 3, listOf(0, 0, 0), - listOf(0, 0, 0), + listOf(0, 0, 0, 0), mapOf("a" to "b"), "$.t", ), @@ -671,6 +673,45 @@ class JsonTreeTest { }.await() } + @Test + fun `should delete nodes correctly in a multi-level range`() = runTest { + val document = Document(Document.Key("")) + fun JsonObject.tree() = getAs("t") + + document.updateAsync { root, _ -> + root.setNewTree( + "t", + element("doc") { + element("p") { + text { "ab" } + element("p") { + text { "x" } + } + } + element("p") { + element("p") { + text { "cd" } + } + } + element("p") { + element("p") { + text { "y" } + } + text { "ef" } + } + }, + ) + assertEquals( + "

ab

x

cd

y

ef

", + document.getRoot().tree().toXml(), + ) + + root.tree().edit(2, 18) + // TODO(7hong13): should be resolved after implementing Tree.move() + // assertEquals("

a

af

", root.tree().toXml()) + }.await() + } + companion object { private val DummyContext = ChangeContext( ChangeID.InitialChangeID, diff --git a/yorkie/src/test/kotlin/dev/yorkie/util/IndexTreeTest.kt b/yorkie/src/test/kotlin/dev/yorkie/util/IndexTreeTest.kt index 8c60a729c..a3b8936c5 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/util/IndexTreeTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/util/IndexTreeTest.kt @@ -82,12 +82,16 @@ class IndexTreeTest { ), ) assertEquals( - listOf("text.b", "p", "text.cde", "p", "text.fg", "p"), + listOf("text.b:All", "p:Closing", "text.cde:All", "p:All", "text.fg:All", "p:Opening"), tree.nodesBetween(2, 11), ) - assertEquals(listOf("p"), tree.nodesBetween(0, 1)) - assertEquals(listOf("p"), tree.nodesBetween(3, 4)) - assertEquals(listOf("p", "p"), tree.nodesBetween(3, 5)) + assertEquals( + listOf("text.b:All", "p:Closing", "text.cde:All", "p:Opening"), + tree.nodesBetween(2, 6), + ) + assertEquals(listOf("p:Opening"), tree.nodesBetween(0, 1)) + assertEquals(listOf("p:Closing"), tree.nodesBetween(3, 4)) + assertEquals(listOf("p:Closing", "p:Opening"), tree.nodesBetween(3, 5)) } @Test @@ -376,8 +380,8 @@ class IndexTreeTest { } private fun IndexTree.nodesBetween(from: Int, to: Int) = buildList { - nodesBetween(from, to) { node -> - add(node.toDiagnostic()) + nodesBetween(from, to) { node, contain -> + add("${node.toDiagnostic()}:${contain.name}") } } From e0ada66df56b71e0586cfce28adba37dfdb02799 Mon Sep 17 00:00:00 2001 From: jee-hyun-kim Date: Sun, 27 Aug 2023 23:56:49 +0900 Subject: [PATCH 11/13] remove select operation from text --- .../dev/yorkie/api/OperationConverter.kt | 22 +--------- .../dev/yorkie/document/crdt/CrdtText.kt | 20 --------- .../dev/yorkie/document/crdt/TextInfo.kt | 11 +---- .../dev/yorkie/document/json/JsonText.kt | 36 ---------------- .../document/operation/SelectOperation.kt | 43 ------------------- .../kotlin/dev/yorkie/api/ConverterTest.kt | 15 ++----- .../dev/yorkie/document/crdt/CrdtTextTest.kt | 10 ----- .../dev/yorkie/document/json/JsonTextTest.kt | 7 --- 8 files changed, 6 insertions(+), 158 deletions(-) delete mode 100644 yorkie/src/main/kotlin/dev/yorkie/document/operation/SelectOperation.kt diff --git a/yorkie/src/main/kotlin/dev/yorkie/api/OperationConverter.kt b/yorkie/src/main/kotlin/dev/yorkie/api/OperationConverter.kt index 63f343eb5..4907d73b4 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/api/OperationConverter.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/api/OperationConverter.kt @@ -5,7 +5,6 @@ import dev.yorkie.api.v1.OperationKt.edit import dev.yorkie.api.v1.OperationKt.increase import dev.yorkie.api.v1.OperationKt.move import dev.yorkie.api.v1.OperationKt.remove -import dev.yorkie.api.v1.OperationKt.select import dev.yorkie.api.v1.OperationKt.set import dev.yorkie.api.v1.OperationKt.style import dev.yorkie.api.v1.OperationKt.treeEdit @@ -17,7 +16,6 @@ import dev.yorkie.document.operation.IncreaseOperation import dev.yorkie.document.operation.MoveOperation import dev.yorkie.document.operation.Operation import dev.yorkie.document.operation.RemoveOperation -import dev.yorkie.document.operation.SelectOperation import dev.yorkie.document.operation.SetOperation import dev.yorkie.document.operation.StyleOperation import dev.yorkie.document.operation.TreeEditOperation @@ -27,7 +25,7 @@ import dev.yorkie.document.time.ActorID internal typealias PBOperation = dev.yorkie.api.v1.Operation internal fun List.toOperations(): List { - return map { + return mapNotNull { when { it.hasSet() -> SetOperation( key = it.set.key, @@ -77,12 +75,7 @@ internal fun List.toOperations(): List { ?: mapOf(), ) - it.hasSelect() -> SelectOperation( - fromPos = it.select.from.toRgaTreeSplitNodePos(), - toPos = it.select.to.toRgaTreeSplitNodePos(), - parentCreatedAt = it.select.parentCreatedAt.toTimeTicket(), - executedAt = it.select.executedAt.toTimeTicket(), - ) + it.hasSelect() -> null it.hasStyle() -> StyleOperation( fromPos = it.style.from.toRgaTreeSplitNodePos(), @@ -188,17 +181,6 @@ internal fun Operation.toPBOperation(): PBOperation { } } - is SelectOperation -> { - operation { - select = select { - parentCreatedAt = operation.parentCreatedAt.toPBTimeTicket() - from = operation.fromPos.toPBTextNodePos() - to = operation.toPos.toPBTextNodePos() - executedAt = operation.executedAt.toPBTimeTicket() - } - } - } - is StyleOperation -> { operation { style = style { diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtText.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtText.kt index 94118926c..241a63f1f 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtText.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtText.kt @@ -12,8 +12,6 @@ internal data class CrdtText( override var _movedAt: TimeTicket? = null, override var _removedAt: TimeTicket? = null, ) : CrdtGCElement() { - private val selectionMap = mutableMapOf() - override val removedNodesLength: Int get() = rgaTreeSplit.removedNodesLength @@ -68,17 +66,6 @@ internal data class CrdtText( return Triple(latestCreatedAtMap, changes, caretPos to caretPos) } - private fun selectPrev(range: RgaTreeSplitPosRange, executedAt: TimeTicket): TextChange? { - val prevSelection = selectionMap[executedAt.actorID] - return if (prevSelection == null || prevSelection.executedAt < executedAt) { - selectionMap[executedAt.actorID] = Selection(range.first, range.second, executedAt) - val (from, to) = rgaTreeSplit.findIndexesFromRange(range) - TextChange(TextChangeType.Selection, executedAt.actorID, from, to) - } else { - null - } - } - /** * Applies the style of the given [range]. * 1. Split nodes with from and to. @@ -110,13 +97,6 @@ internal data class CrdtText( } } - /** - * Stores that the given [range] has been selected. - */ - fun select(range: RgaTreeSplitPosRange, executedAt: TimeTicket): TextChange? { - return selectPrev(range, executedAt) - } - /** * Returns a pair of [RgaTreeSplitPos] of the given integer offsets. */ diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/TextInfo.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/TextInfo.kt index 8f51d60a7..bdd447a30 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/TextInfo.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/TextInfo.kt @@ -17,18 +17,9 @@ internal data class TextChange( * The type of [TextChange]. */ internal enum class TextChangeType { - Content, Selection, Style + Content, Style } -/** - * Represents the selection of text range in the editor. - */ -internal data class Selection( - val from: RgaTreeSplitPos, - val to: RgaTreeSplitPos, - val executedAt: TimeTicket, -) - internal data class TextValue( val content: String, private val _attributes: Rht = Rht(), diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonText.kt b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonText.kt index 1fa6b46ec..4b4efcaea 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonText.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonText.kt @@ -5,7 +5,6 @@ import dev.yorkie.document.crdt.CrdtText import dev.yorkie.document.crdt.RgaTreeSplitPosRange import dev.yorkie.document.crdt.TextWithAttributes import dev.yorkie.document.operation.EditOperation -import dev.yorkie.document.operation.SelectOperation import dev.yorkie.document.operation.StyleOperation import dev.yorkie.util.YorkieLogger import dev.yorkie.document.RgaTreeSplitPosStruct as TextPosStruct @@ -125,41 +124,6 @@ public class JsonText internal constructor( return true } - /** - * Selects the given range. - */ - public fun select(fromIndex: Int, toIndex: Int): Boolean { - if (fromIndex > toIndex) { - YorkieLogger.e(TAG, "fromIndex should be less than or equal to toIndex") - return false - } - - val range = createRange(fromIndex, toIndex) ?: return false - - YorkieLogger.d( - TAG, - "SELT: f:$fromIndex->${range.first}, t:$toIndex->${range.second}", - ) - - val executedAt = context.issueTimeTicket() - try { - target.select(range, executedAt) - } catch (e: NoSuchElementException) { - YorkieLogger.e(TAG, "can't select text") - return false - } - - context.push( - SelectOperation( - parentCreatedAt = target.createdAt, - fromPos = range.first, - toPos = range.second, - executedAt = executedAt, - ), - ) - return true - } - private fun createRange(fromIndex: Int, toIndex: Int): RgaTreeSplitPosRange? { return runCatching { target.indexRangeToPosRange(fromIndex, toIndex) diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/operation/SelectOperation.kt b/yorkie/src/main/kotlin/dev/yorkie/document/operation/SelectOperation.kt deleted file mode 100644 index 96522dfed..000000000 --- a/yorkie/src/main/kotlin/dev/yorkie/document/operation/SelectOperation.kt +++ /dev/null @@ -1,43 +0,0 @@ -package dev.yorkie.document.operation - -import dev.yorkie.document.crdt.CrdtRoot -import dev.yorkie.document.crdt.CrdtText -import dev.yorkie.document.crdt.RgaTreeSplitPos -import dev.yorkie.document.crdt.RgaTreeSplitPosRange -import dev.yorkie.document.time.TimeTicket -import dev.yorkie.util.YorkieLogger - -internal data class SelectOperation( - val fromPos: RgaTreeSplitPos, - val toPos: RgaTreeSplitPos, - override val parentCreatedAt: TimeTicket, - override var executedAt: TimeTicket, -) : Operation() { - - override val effectedCreatedAt: TimeTicket - get() = parentCreatedAt - - /** - * Returns the created time of the effected element. - */ - override fun execute(root: CrdtRoot): List { - val parentObject = root.findByCreatedAt(parentCreatedAt) - return if (parentObject is CrdtText) { - val change = parentObject.select(RgaTreeSplitPosRange(fromPos, toPos), executedAt) - ?: return emptyList() - listOf( - OperationInfo.SelectOpInfo(from = change.from, to = change.to).apply { - executedAt = parentCreatedAt - }, - ) - } else { - parentObject ?: YorkieLogger.e(TAG, "fail to find $parentCreatedAt") - YorkieLogger.e(TAG, "fail to execute, only Text, RichText can execute select") - emptyList() - } - } - - companion object { - private const val TAG = "SelectOperation" - } -} diff --git a/yorkie/src/test/kotlin/dev/yorkie/api/ConverterTest.kt b/yorkie/src/test/kotlin/dev/yorkie/api/ConverterTest.kt index 7e8e659e0..b91b6b109 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/api/ConverterTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/api/ConverterTest.kt @@ -32,7 +32,6 @@ import dev.yorkie.document.operation.MoveOperation import dev.yorkie.document.operation.Operation import dev.yorkie.document.operation.OperationInfo import dev.yorkie.document.operation.RemoveOperation -import dev.yorkie.document.operation.SelectOperation import dev.yorkie.document.operation.SetOperation import dev.yorkie.document.operation.StyleOperation import dev.yorkie.document.operation.TreeEditOperation @@ -235,12 +234,6 @@ class ConverterTest { InitialTimeTicket, mapOf("style" to "bold"), ) - val selectOperation = SelectOperation( - nodePos, - nodePos, - InitialTimeTicket, - InitialTimeTicket, - ) val styleOperation = StyleOperation( nodePos, nodePos, @@ -277,7 +270,6 @@ class ConverterTest { increaseOperation.toPBOperation(), editOperationWithoutAttrs.toPBOperation(), editOperationWithAttrs.toPBOperation(), - selectOperation.toPBOperation(), styleOperation.toPBOperation(), treeEditOperation.toPBOperation(), treeStyleOperation.toPBOperation(), @@ -290,10 +282,9 @@ class ConverterTest { assertEquals(increaseOperation, converted[4]) assertEquals(editOperationWithoutAttrs, converted[5]) assertEquals(editOperationWithAttrs, converted[6]) - assertEquals(selectOperation, converted[7]) - assertEquals(styleOperation, converted[8]) - assertEquals(treeEditOperation, converted[9]) - assertEquals(treeStyleOperation, converted[10]) + assertEquals(styleOperation, converted[7]) + assertEquals(treeEditOperation, converted[8]) + assertEquals(treeStyleOperation, converted[9]) } @Test diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTextTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTextTest.kt index 3ccc5926b..302973b05 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTextTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtTextTest.kt @@ -1,6 +1,5 @@ package dev.yorkie.document.crdt -import dev.yorkie.document.time.ActorID import dev.yorkie.document.time.TimeTicket import org.junit.Before import org.junit.Test @@ -46,13 +45,4 @@ class CrdtTextTest { target.toJson(), ) } - - @Test - fun `should handle select operations`() { - target.edit(target.indexRangeToPosRange(0, 0), "ABCD", TimeTicket.InitialTimeTicket) - val executedAt = TimeTicket(1L, 1u, ActorID.INITIAL_ACTOR_ID) - val textChange = target.select(target.indexRangeToPosRange(2, 4), executedAt) - assertEquals(TextChangeType.Selection, textChange?.type) - assertEquals(2 to 4, textChange?.from to textChange?.to) - } } diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTextTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTextTest.kt index b7ab24662..ff6236d1e 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTextTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonTextTest.kt @@ -136,13 +136,6 @@ class JsonTextTest { assertFalse(target.style(5, 7, emptyMap())) } - @Test - fun `should return false when index range is invalid with select`() { - assertTrue(target.select(0, 0)) - assertFalse(target.select(5, 0)) - assertFalse(target.select(5, 7)) - } - @Test fun `should handle text clear operations`() { target.edit(0, 0, "ABCD") From 564a86b755a6e060480c5f0770fa8d8fb590ad24 Mon Sep 17 00:00:00 2001 From: jee-hyun-kim Date: Mon, 28 Aug 2023 12:27:12 +0900 Subject: [PATCH 12/13] improve TCs --- docker/docker-compose-ci.yml | 2 +- docker/docker-compose.yml | 2 +- .../kotlin/dev/yorkie/core/ClientTest.kt | 14 ++- .../kotlin/dev/yorkie/core/DocumentTest.kt | 4 +- .../kotlin/dev/yorkie/core/PresenceTest.kt | 92 ++++++++++--------- 5 files changed, 64 insertions(+), 50 deletions(-) diff --git a/docker/docker-compose-ci.yml b/docker/docker-compose-ci.yml index 0e733303d..5313b0289 100644 --- a/docker/docker-compose-ci.yml +++ b/docker/docker-compose-ci.yml @@ -15,7 +15,7 @@ services: depends_on: - yorkie yorkie: - image: 'yorkieteam/yorkie:latest' + image: 'yorkieteam/yorkie:0.4.6' container_name: 'yorkie' command: [ 'server', diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index c99b7e27e..7f4b860fd 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -22,7 +22,7 @@ services: extra_hosts: - "host.docker.internal:host-gateway" yorkie: - image: 'yorkieteam/yorkie:latest' + image: 'yorkieteam/yorkie:0.4.6' container_name: 'yorkie' command: [ 'server', diff --git a/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt b/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt index 7dde6b141..f245d4c03 100644 --- a/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt +++ b/yorkie/src/androidTest/kotlin/dev/yorkie/core/ClientTest.kt @@ -121,11 +121,15 @@ class ClientTest { root.remove("k1") }.await() - while (client1Events.none { it is DocumentSynced }) { - delay(50) + withTimeout(GENERAL_TIMEOUT) { + while (client1Events.none { it is DocumentSynced }) { + delay(50) + } } - while (client2Events.isEmpty()) { - delay(50) + withTimeout(GENERAL_TIMEOUT) { + while (client2Events.isEmpty()) { + delay(50) + } } syncEvent = assertIs(client2Events.first { it is DocumentSynced }) assertIs(syncEvent.result) @@ -191,7 +195,7 @@ class ClientTest { root["version"] = "v2" }.await() client1.syncAsync().await() - withTimeout(5_000) { + withTimeout(GENERAL_TIMEOUT) { while (client2Events.size < 2) { delay(50) } diff --git a/yorkie/src/androidTest/kotlin/dev/yorkie/core/DocumentTest.kt b/yorkie/src/androidTest/kotlin/dev/yorkie/core/DocumentTest.kt index a92813cb6..74b605130 100644 --- a/yorkie/src/androidTest/kotlin/dev/yorkie/core/DocumentTest.kt +++ b/yorkie/src/androidTest/kotlin/dev/yorkie/core/DocumentTest.kt @@ -147,7 +147,9 @@ class DocumentTest { assertEquals(document1.toJson(), document2.toJson()) client1.removeAsync(document1).await() - client2.detachAsync(document2).await() + if (document2.status != DocumentStatus.Removed) { + client2.detachAsync(document2).await() + } assertEquals(DocumentStatus.Removed, document1.status) assertEquals(DocumentStatus.Removed, document2.status) } diff --git a/yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt b/yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt index 161e22a06..4a097cf26 100644 --- a/yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt +++ b/yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt @@ -8,7 +8,6 @@ import dev.yorkie.gson import junit.framework.TestCase.assertEquals import junit.framework.TestCase.assertNull import kotlinx.coroutines.CoroutineStart -import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.flow.filterIsInstance import kotlinx.coroutines.flow.filterNot @@ -214,25 +213,45 @@ class PresenceTest { val previousCursor = gson.toJson(Cursor(0, 0)) val updatedCursor = gson.toJson(Cursor(1, 1)) - withTwoClientsAndDocuments( - presences = mapOf("name" to "a", "cursor" to previousCursor) to mapOf( - "name" to "b", - "cursor" to previousCursor, - ), - ) { c1, c2, d1, d2, _ -> + runBlocking { + val c1 = createClient() + val c2 = createClient() + val documentKey = UUID.randomUUID().toString().toDocKey() + val d1 = Document(documentKey) + val d2 = Document(documentKey) val d1Events = mutableListOf() val d2Events = mutableListOf() - val collectJobs = listOf( - launch(start = CoroutineStart.UNDISPATCHED) { - d1.events.filterIsInstance() - .filterNot { it is MyPresence.Initialized } - .collect(d1Events::add) - }, - launch(start = CoroutineStart.UNDISPATCHED) { - d2.events.filterIsInstance() - .filterNot { it is MyPresence.Initialized } - .collect(d2Events::add) - }, + + c1.activateAsync().await() + c2.activateAsync().await() + + val c1ID = c1.requireClientId() + val c2ID = c2.requireClientId() + + c1.attachAsync(d1, mapOf("name" to "a", "cursor" to previousCursor)).await() + val d1Job = launch(start = CoroutineStart.UNDISPATCHED) { + d1.events.filterIsInstance() + .filterNot { it is MyPresence.Initialized } + .collect(d1Events::add) + } + + println("c2 attach") + c2.attachAsync(d2, mapOf("name" to "b", "cursor" to previousCursor)).await() + val d2Job = launch(start = CoroutineStart.UNDISPATCHED) { + d2.events.filterIsInstance() + .filterNot { it is MyPresence.Initialized } + .collect(d2Events::add) + } + + withTimeout(GENERAL_TIMEOUT + 1) { + while (d1Events.isEmpty()) { + delay(50) + } + } + + assertEquals( + Others.Watched(PresenceInfo(c2ID, d2.presences.value[c2ID]!!)), + d1Events.last(), ) d1.updateAsync { _, presence -> @@ -241,42 +260,31 @@ class PresenceTest { presence.put(mapOf("name" to "X")) }.await() - withTimeout(GENERAL_TIMEOUT) { + withTimeout(GENERAL_TIMEOUT + 2) { while (d1Events.size < 2 || d2Events.isEmpty()) { delay(50) } } assertEquals( - listOf( - MyPresence.PresenceChanged( - PresenceInfo( - c1.requireClientId(), - mapOf("name" to "X", "cursor" to updatedCursor), - ), - ), - Others.Watched( - PresenceInfo( - c2.requireClientId(), - mapOf("name" to "b", "cursor" to previousCursor), - ), - ), + MyPresence.PresenceChanged( + PresenceInfo(c1ID, mapOf("name" to "X", "cursor" to updatedCursor)), ), - d1Events, + d1Events.last(), ) assertEquals( - listOf( - Others.PresenceChanged( - PresenceInfo( - c1.requireClientId(), - mapOf("name" to "X", "cursor" to updatedCursor), - ), - ), + Others.PresenceChanged( + PresenceInfo(c1ID, mapOf("name" to "X", "cursor" to updatedCursor)), ), - d2Events, + d2Events.last(), ) - collectJobs.forEach(Job::cancel) + d1Job.cancel() + d2Job.cancel() + c1.detachAsync(d1).await() + c2.detachAsync(d2).await() + c1.deactivateAsync().await() + c2.deactivateAsync().await() } } From 5e808f3b53eb680b59a40a0a755a2df4667029d4 Mon Sep 17 00:00:00 2001 From: jee-hyun-kim Date: Mon, 28 Aug 2023 13:49:22 +0900 Subject: [PATCH 13/13] change back the github action to forekd one --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dd6eff7a1..9123b65dc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,7 +62,8 @@ jobs: - run: chmod +x gradlew - uses: gradle/gradle-build-action@v2 - name: Setup Docker on macOS - uses: douglascamata/setup-docker-macos-action@v1-alpha + uses: 7hong13/setup-docker-macos-action@fix_formula_paths + id: docker - run: docker-compose -f docker/docker-compose-ci.yml up --build -d - uses: actions/cache@v3 id: avd-cache