diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtArray.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtArray.kt index e246a23b0..ecdcf517e 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtArray.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtArray.kt @@ -10,7 +10,7 @@ internal class CrdtArray( createdAt: TimeTicket, ) : CrdtContainer(createdAt), Iterable { val head - get() = elements.dummyHead.value + get() = elements.head val last get() = elements.last.value @@ -18,6 +18,9 @@ internal class CrdtArray( val length get() = elements.length + val lastCreated + get() = elements.getLastCreatedAt() + /** * Returns the sub path of the given [createdAt] element. */ @@ -132,12 +135,16 @@ internal class CrdtArray( return object : Iterator { var node = elements.firstOrNull() - override fun hasNext() = node?.isRemoved == false + override fun hasNext(): Boolean { + while (node?.isRemoved == true) { + node = node?.next + } + return node != null + } override fun next(): CrdtElement { return requireNotNull(node).value.also { - elements.drop(1) - node = elements.firstOrNull() + node = node?.next } } } diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/RgaTreeList.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/RgaTreeList.kt index d3dedc344..7168f16c8 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/RgaTreeList.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/RgaTreeList.kt @@ -1,5 +1,6 @@ package dev.yorkie.document.crdt +import androidx.annotation.VisibleForTesting import dev.yorkie.document.time.TimeTicket import dev.yorkie.document.time.TimeTicket.Companion.InitialTimeTicket import dev.yorkie.document.time.TimeTicket.Companion.compareTo @@ -8,8 +9,8 @@ import dev.yorkie.util.SplayTreeSet /** * [RgaTreeList] is replicated growable array. */ -internal class RgaTreeList : Iterable { - val dummyHead = RgaTreeListNode(Primitive.of(1, InitialTimeTicket)).apply { +internal class RgaTreeList private constructor() : Iterable { + private val dummyHead = RgaTreeListNode(Primitive.of(1, InitialTimeTicket)).apply { value.removedAt = InitialTimeTicket } var last: RgaTreeListNode = dummyHead @@ -25,6 +26,9 @@ internal class RgaTreeList : Iterable { val length get() = nodeMapByIndex.length + val head + get() = dummyHead.value + /** * Adds a new node with [value] after the last node. */ @@ -82,9 +86,7 @@ internal class RgaTreeList : Iterable { val node = nodeMapByCreatedAt[createdAt] ?: error("can't find the given node createdAt: $createdAt") - if (prevNode != node && - (node.value.movedAt == null || checkNotNull(node.value.movedAt) < executedAt) - ) { + if (prevNode != node && node.value.movedAt < executedAt) { delete(node) insertAfter(prevNode.createdAt, node.value, executedAt) node.value.movedAt = executedAt @@ -202,6 +204,7 @@ internal class RgaTreeList : Iterable { } } + @VisibleForTesting data class RgaTreeListNode(val value: CrdtElement) { var prev: RgaTreeListNode? = null var next: RgaTreeListNode? = null diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtArrayTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtArrayTest.kt index d0a35e1f2..a346c470b 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtArrayTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtArrayTest.kt @@ -2,6 +2,7 @@ package dev.yorkie.document.crdt import dev.yorkie.document.time.ActorID import dev.yorkie.document.time.TimeTicket +import org.junit.Assert import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test @@ -31,13 +32,13 @@ class CrdtArrayTest { assertEquals(crdtElements.size + 1, target.length) target.remove(timeTickets[1], timeTickets[2]) - assertEquals("A1B0C1D1E1F1G1", target.getStructureAsString()) + assertEquals("ACDEFG", target.getStructureAsString()) target.remove(timeTickets[2], timeTickets[3]) - assertEquals("A1B0C0D1E1F1G1", target.getStructureAsString()) + assertEquals("ADEFG", target.getStructureAsString()) target.remove(timeTickets[3], timeTickets[4]) - assertEquals("A1B0C0D0E1F1G1", target.getStructureAsString()) + assertEquals("AEFG", target.getStructureAsString()) target.remove(timeTickets[6], timeTickets[5]) - assertEquals("A1B0C0D0E1F1G1", target.getStructureAsString()) + assertEquals("AEFG", target.getStructureAsString()) assertEquals(crdtElements.size - 3 + 1, target.length) } @@ -51,13 +52,34 @@ class CrdtArrayTest { target.delete(crdtElements[0]) assertEquals(crdtElements.size - 1 + 1, target.length) - assertEquals("B1C1D1E1F1G1", target.getStructureAsString()) + assertEquals("BCDEFG", target.getStructureAsString()) target.delete(crdtElements[1]) assertEquals(crdtElements.size - 2 + 1, target.length) - assertEquals("C1D1E1F1G1", target.getStructureAsString()) + assertEquals("CDEFG", target.getStructureAsString()) target.delete(crdtElements[2]) assertEquals(crdtElements.size - 3 + 1, target.length) - assertEquals("D1E1F1G1", target.getStructureAsString()) + assertEquals("DEFG", target.getStructureAsString()) + } + + @Test + fun `should handle removeByIndex operations`() { + assertEquals(1, target.length) + + crdtElements.forEach { target.insertAfter(target.last.createdAt, it) } + assertEquals(crdtElements.size + 1, target.length) + + target.removeByIndex(1, timeTickets[2]) + assertEquals("BCDEFG", target.getStructureAsString()) + target.removeByIndex(1, timeTickets[3]) + assertEquals("CDEFG", target.getStructureAsString()) + target.removeByIndex(1, timeTickets[4]) + assertEquals("DEFG", target.getStructureAsString()) + target.removeByIndex(1, timeTickets[5]) + assertEquals("EFG", target.getStructureAsString()) + target.removeByIndex(crdtElements.size, timeTickets[6]) + assertEquals("EFG", target.getStructureAsString()) + + assertEquals(crdtElements.size - 4 + 1, target.length) } @Test @@ -68,15 +90,18 @@ class CrdtArrayTest { assertEquals(crdtElements.size + 1, target.length) target.insertAfter(timeTickets[0], Primitive.of(1, createTimeTicket())) - assertEquals("A1H1B1C1D1E1F1G1", target.getStructureAsString()) + assertEquals("AHBCDEFG", target.getStructureAsString()) } @Test fun `should handle moving an element after the given element`() { + assertEquals(1, target.length) + crdtElements.forEach { target.insertAfter(target.last.createdAt, it) } + assertEquals(crdtElements.size + 1, target.length) target.moveAfter(timeTickets[5], timeTickets[4], createTimeTicket()) - assertEquals("A1B1C1D1F1E1G1", target.getStructureAsString()) + assertEquals("ABCDFEG", target.getStructureAsString()) assertEquals(crdtElements.size + 1, target.length) } @@ -116,17 +141,51 @@ class CrdtArrayTest { assertEquals(sampleTarget1.getStructureAsString(), sampleTarget2.getStructureAsString()) } + @Test + fun `should handle get operations with index`() { + assertEquals(1, target.length) + + crdtElements.forEach { target.insertAfter(target.last.createdAt, it) } + assertEquals(crdtElements.size + 1, target.length) + + assertEquals(crdtElements[0], target[1]) + assertEquals(null, target[10]) + } + + @Test + fun `should handle get operations with value`() { + assertEquals(1, target.length) + + crdtElements.forEach { target.insertAfter(target.last.createdAt, it) } + assertEquals(crdtElements.size + 1, target.length) + + assertEquals(crdtElements[0], target[timeTickets[0]]) + assertEquals(null, target[createTimeTicket()]) + } + + @Test + fun `should handle getPrevCreatedAt operations`() { + assertEquals(1, target.length) + + crdtElements.forEach { target.insertAfter(target.last.createdAt, it) } + assertEquals(crdtElements.size + 1, target.length) + + assertEquals(timeTickets[0], target.getPrevCreatedAt(timeTickets[1])) + assertEquals(timeTickets[1], target.getPrevCreatedAt(timeTickets[2])) + Assert.assertThrows(IllegalStateException::class.java) { + target.getPrevCreatedAt(createTimeTicket()) + } + } + private fun createTimeTicket(): TimeTicket { return TimeTicket(crdtElements.size.toLong(), TimeTicket.INITIAL_DELIMITER, ActorID("H")) } private fun createSampleCrdtArray(): CrdtArray { - return CrdtArray(RgaTreeList.create(), TimeTicket.InitialTimeTicket) + return CrdtArray.create(TimeTicket.InitialTimeTicket) } private fun CrdtArray.getStructureAsString() = buildString { - target.elements.forEach { - append(it.createdAt.actorID.id + if (it.isRemoved) 0 else 1) - } + target.forEach { append(it.createdAt.actorID.id) } } } diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/RgaTreeListTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/RgaTreeListTest.kt index 9605b8224..9fd8ba806 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/RgaTreeListTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/RgaTreeListTest.kt @@ -4,6 +4,7 @@ import dev.yorkie.document.time.ActorID import dev.yorkie.document.time.TimeTicket import org.junit.Assert.assertEquals import org.junit.Assert.assertNotSame +import org.junit.Assert.assertThrows import org.junit.Before import org.junit.Test @@ -48,6 +49,10 @@ class RgaTreeListTest { @Test fun `should handle remove operations`() { + assertThrows(IllegalStateException::class.java) { + target.remove(timeTickets[0], timeTickets[1]) + } + assertEquals(1, target.length) crdtElements.forEach(target::insert) @@ -64,8 +69,32 @@ class RgaTreeListTest { assertEquals(crdtElements.size - 3 + 1, target.length) } + @Test + fun `should handle removeByIndex operations`() { + assertEquals(1, target.length) + + crdtElements.forEach(target::insert) + + target.removeByIndex(1, timeTickets[2]) + assertEquals("A0B1C1D1E1F1G1", target.getStructureAsString()) + target.removeByIndex(1, timeTickets[3]) + assertEquals("A0B0C1D1E1F1G1", target.getStructureAsString()) + target.removeByIndex(1, timeTickets[4]) + assertEquals("A0B0C0D1E1F1G1", target.getStructureAsString()) + target.removeByIndex(1, timeTickets[5]) + assertEquals("A0B0C0D0E1F1G1", target.getStructureAsString()) + target.removeByIndex(crdtElements.size, timeTickets[6]) + assertEquals("A0B0C0D0E1F1G1", target.getStructureAsString()) + + assertEquals(crdtElements.size - 4 + 1, target.length) + } + @Test fun `should handle delete operations`() { + assertThrows(IllegalStateException::class.java) { + target.delete(crdtElements[0]) + } + assertEquals(1, target.length) crdtElements.forEach(target::insert) @@ -95,6 +124,10 @@ class RgaTreeListTest { @Test fun `should handle moving an element after the given element`() { + assertThrows(IllegalStateException::class.java) { + target.moveAfter(timeTickets[0], timeTickets[1], timeTickets[1]) + } + crdtElements.forEach(target::insert) target.moveAfter(timeTickets[5], timeTickets[4], createTimeTicket())