Support concurrent formatting of Text (#138)

* Support concurrent formatting of Text * add TextOperationResult * update version to 0.4.7
yorkie-team · Oct 17, 2023 · 28e9791 · 28e9791
1 parent 6782b5c
commit 28e9791
Show file tree

Hide file tree

Showing 11 changed files with 115 additions and 21 deletions.
diff --git a/gradle.properties b/gradle.properties
@@ -10,7 +10,7 @@ android.suppressUnsupportedOptionWarnings=android.suppressUnsupportedOptionWarni
 kotlin.code.style=official
 kotlin.mpp.stability.nowarn=true
 GROUP=dev.yorkie
-VERSION_NAME=0.4.6
+VERSION_NAME=0.4.7
 POM_DESCRIPTION=Document store for building collaborative editing applications.
 POM_INCEPTION_YEAR=2022
 POM_URL=https://github.com/yorkie-team/yorkie-android-sdk

diff --git a/yorkie/proto/src/main/proto/yorkie/v1/resources.proto b/yorkie/proto/src/main/proto/yorkie/v1/resources.proto
@@ -111,6 +111,7 @@ message Operation {
     TextNodePos to = 3;
     map<string, string> attributes = 4;
     TimeTicket executed_at = 5;
+    map<string, TimeTicket> created_at_map_by_actor = 6;
   }
   message Increase {
     TimeTicket parent_created_at = 1;

diff --git a/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTextTest.kt b/yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTextTest.kt
@@ -0,0 +1,45 @@
+package dev.yorkie.document.json
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import dev.yorkie.assertJsonContentEquals
+import dev.yorkie.core.withTwoClientsAndDocuments
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(AndroidJUnit4::class)
+class JsonTextTest {
+
+    @Test
+    fun test_concurrent_insertion_and_deletion() {
+        withTwoClientsAndDocuments(realTimeSync = false) { c1, c2, d1, d2, _ ->
+            d1.updateAsync { root, _ ->
+                root.setNewText("k1").apply {
+                    edit(0, 0, "AB")
+                }
+            }.await()
+
+            c1.syncAsync().await()
+            c2.syncAsync().await()
+
+            assertJsonContentEquals("""{"k1":[{"val":"AB"}]}""", d1.toJson())
+            assertJsonContentEquals(d1.toJson(), d2.toJson())
+
+            d1.updateAsync { root, _ ->
+                root.getAs<JsonText>("k1").edit(0, 2, "")
+            }.await()
+            assertJsonContentEquals("""{"k1":[]}""", d1.toJson())
+
+            d2.updateAsync { root, _ ->
+                root.getAs<JsonText>("k1").edit(1, 1, "C")
+            }.await()
+            assertJsonContentEquals("""{"k1":[{"val":"A"},{"val":"C"},{"val":"B"}]}""", d2.toJson())
+
+            c1.syncAsync().await()
+            c2.syncAsync().await()
+            c1.syncAsync().await()
+
+            assertJsonContentEquals("""{"k1":[{"val":"C"}]}""", d1.toJson())
+            assertJsonContentEquals(d1.toJson(), d2.toJson())
+        }
+    }
+}
diff --git a/yorkie/src/main/kotlin/dev/yorkie/api/OperationConverter.kt b/yorkie/src/main/kotlin/dev/yorkie/api/OperationConverter.kt
@@ -83,6 +83,10 @@ internal fun List<PBOperation>.toOperations(): List<Operation> {
                 attributes = it.style.attributesMap,
                 parentCreatedAt = it.style.parentCreatedAt.toTimeTicket(),
                 executedAt = it.style.executedAt.toTimeTicket(),
+                maxCreatedAtMapByActor = it.style.createdAtMapByActorMap.entries
+                    .associate { (actorID, createdAt) ->
+                        ActorID(actorID) to createdAt.toTimeTicket()
+                    },
             )
 
             it.hasTreeEdit() -> TreeEditOperation(
@@ -189,6 +193,9 @@ internal fun Operation.toPBOperation(): PBOperation {
                     to = operation.toPos.toPBTextNodePos()
                     executedAt = operation.executedAt.toPBTimeTicket()
                     operation.attributes.forEach { attributes[it.key] = it.value }
+                    operation.maxCreatedAtMapByActor.forEach {
+                        createdAtMapByActor[it.key.value] = it.value.toPBTimeTicket()
+                    }
                 }
             }
         }

diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtText.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtText.kt
@@ -34,7 +34,7 @@ internal data class CrdtText(
         executedAt: TimeTicket,
         attributes: Map<String, String>? = null,
         latestCreatedAtMapByActor: Map<ActorID, TimeTicket>? = null,
-    ): Triple<Map<ActorID, TimeTicket>, List<TextChange>, RgaTreeSplitPosRange> {
+    ): TextOperationResult {
         val textValue = if (value.isNotEmpty()) {
             TextValue(value).apply {
                 attributes?.forEach { setAttribute(it.key, it.value, executedAt) }
@@ -63,7 +63,7 @@ internal data class CrdtText(
         if (value.isNotEmpty() && attributes != null) {
             changes[changes.lastIndex] = changes.last().copy(attributes = attributes)
         }
-        return Triple(latestCreatedAtMap, changes, caretPos to caretPos)
+        return TextOperationResult(latestCreatedAtMap, changes, caretPos to caretPos)
     }
 
     /**
@@ -75,14 +75,32 @@ internal data class CrdtText(
         range: RgaTreeSplitPosRange,
         attributes: Map<String, String>,
         executedAt: TimeTicket,
-    ): List<TextChange> {
+        latestCreatedAtMapByActor: Map<ActorID, TimeTicket>? = null,
+    ): TextOperationResult {
         // 1. Split nodes with from and to.
         val toRight = rgaTreeSplit.findNodeWithSplit(range.second, executedAt).second
         val fromRight = rgaTreeSplit.findNodeWithSplit(range.first, executedAt).second
 
         // 2. Style nodes between from and to.
-        return rgaTreeSplit.findBetween(fromRight, toRight)
-            .filterNot { it.isRemoved }
+        val nodes = rgaTreeSplit.findBetween(fromRight, toRight)
+        val createdAtMapByActor = mutableMapOf<ActorID, TimeTicket>()
+        val toBeStyleds = nodes.mapNotNull { node ->
+            val actorID = node.createdAt.actorID
+            val latestCreatedAt = if (latestCreatedAtMapByActor?.isNotEmpty() == true) {
+                latestCreatedAtMapByActor[actorID] ?: TimeTicket.InitialTimeTicket
+            } else {
+                TimeTicket.MaxTimeTicket
+            }
+
+            node.takeIf { it.canStyle(executedAt, latestCreatedAt) }?.also {
+                val updatedLatestCreatedAt = createdAtMapByActor[actorID]
+                val updatedCreatedAt = node.createdAt
+                if (updatedLatestCreatedAt == null || updatedLatestCreatedAt < updatedCreatedAt) {
+                    createdAtMapByActor[actorID] = updatedCreatedAt
+                }
+            }
+        }
+        val changes = toBeStyleds.filterNot { it.isRemoved }
             .map { node ->
                 val (fromIndex, toIndex) = rgaTreeSplit.findIndexesFromRange(node.createPosRange())
                 attributes.forEach { node.value.setAttribute(it.key, it.value, executedAt) }
@@ -95,6 +113,8 @@ internal data class CrdtText(
                     attributes,
                 )
             }
+
+        return TextOperationResult(createdAtMapByActor, changes)
     }
 
     /**

diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/RgaTreeSplit.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/RgaTreeSplit.kt
@@ -533,6 +533,13 @@ internal data class RgaTreeSplitNode<T : RgaTreeSplitValue<T>>(
         return createdAt <= latestCreatedAt && (isRemoved || _removedAt < executedAt)
     }
 
+    /**
+     * Checks if node is able to set style.
+     */
+    fun canStyle(executedAt: TimeTicket, latestCreatedAt: TimeTicket): Boolean {
+        return createdAt <= latestCreatedAt && removedAt < executedAt
+    }
+
     /**
      * Removes this [RgaTreeSplitNode] at the given [executedAt].
      */

diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/TextInfo.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/TextInfo.kt
@@ -71,3 +71,9 @@ public value class TextWithAttributes(private val value: Pair<String, Map<String
     val attributes: Map<String, String>
         get() = value.second
 }
+
+internal data class TextOperationResult(
+    val createdAtMapByActor: Map<ActorID, TimeTicket>,
+    val textChanges: List<TextChange>,
+    val posRange: RgaTreeSplitPosRange? = null,
+)
diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonText.kt b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonText.kt
@@ -75,7 +75,7 @@ public class JsonText internal constructor(
         if (range.first != range.second) {
             context.registerElementHasRemovedNodes(target)
         }
-        return target.findIndexesFromRange(rangeAfterEdit)
+        return rangeAfterEdit?.let(target::findIndexesFromRange)
     }
 
     /**
@@ -100,7 +100,18 @@ public class JsonText internal constructor(
 
         val executedAt = context.issueTimeTicket()
         runCatching {
-            target.style(range, attributes, executedAt)
+            val maxCreatedAtMapByActor =
+                target.style(range, attributes, executedAt).createdAtMapByActor
+            context.push(
+                StyleOperation(
+                    parentCreatedAt = target.createdAt,
+                    fromPos = range.first,
+                    toPos = range.second,
+                    attributes = attributes,
+                    executedAt = executedAt,
+                    maxCreatedAtMapByActor = maxCreatedAtMapByActor,
+                ),
+            )
         }.getOrElse {
             when (it) {
                 is NoSuchElementException, is IllegalArgumentException -> {
@@ -111,16 +122,6 @@ public class JsonText internal constructor(
                 else -> throw it
             }
         }
-
-        context.push(
-            StyleOperation(
-                parentCreatedAt = target.createdAt,
-                fromPos = range.first,
-                toPos = range.second,
-                attributes = attributes,
-                executedAt = executedAt,
-            ),
-        )
         return true
     }
 

diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/operation/EditOperation.kt b/yorkie/src/main/kotlin/dev/yorkie/document/operation/EditOperation.kt
@@ -37,7 +37,7 @@ internal data class EditOperation(
                 executedAt,
                 attributes,
                 maxCreatedAtMapByActor,
-            ).second
+            ).textChanges
             if (fromPos != toPos) {
                 root.registerElementHasRemovedNodes(parentObject)
             }

diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/operation/StyleOperation.kt b/yorkie/src/main/kotlin/dev/yorkie/document/operation/StyleOperation.kt
@@ -4,12 +4,14 @@ 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.ActorID
 import dev.yorkie.document.time.TimeTicket
 import dev.yorkie.util.YorkieLogger
 
 internal data class StyleOperation(
     val fromPos: RgaTreeSplitPos,
     val toPos: RgaTreeSplitPos,
+    val maxCreatedAtMapByActor: Map<ActorID, TimeTicket>,
     val attributes: Map<String, String>,
     override val parentCreatedAt: TimeTicket,
     override var executedAt: TimeTicket,
@@ -21,8 +23,12 @@ internal data class StyleOperation(
     override fun execute(root: CrdtRoot): List<OperationInfo> {
         val parentObject = root.findByCreatedAt(parentCreatedAt)
         return if (parentObject is CrdtText) {
-            val changes =
-                parentObject.style(RgaTreeSplitPosRange(fromPos, toPos), attributes, executedAt)
+            val changes = parentObject.style(
+                RgaTreeSplitPosRange(fromPos, toPos),
+                attributes,
+                executedAt,
+                maxCreatedAtMapByActor,
+            ).textChanges
             changes.map {
                 OperationInfo.StyleOpInfo(
                     it.from,

diff --git a/yorkie/src/test/kotlin/dev/yorkie/api/ConverterTest.kt b/yorkie/src/test/kotlin/dev/yorkie/api/ConverterTest.kt
@@ -237,6 +237,7 @@ class ConverterTest {
         val styleOperation = StyleOperation(
             nodePos,
             nodePos,
+            emptyMap(),
             mapOf("style" to "bold"),
             InitialTimeTicket,
             InitialTimeTicket,