From bc4581ccb4ba8002eb63da5072436ce4072814d4 Mon Sep 17 00:00:00 2001 From: Hyowoo Kim Date: Thu, 3 Nov 2022 22:36:07 +0900 Subject: [PATCH] Few improvements here and there (#43) * Few improvements here and there Some name changes. Fix TimeTicket comparison error. Use ByteString instead of ByteArray in CrdtPrimitive. * Add more tests --- .../kotlin/dev/yorkie/api/ElementConverter.kt | 43 +++--- .../dev/yorkie/document/change/CheckPoint.kt | 2 +- .../dev/yorkie/document/crdt/CrdtArray.kt | 9 +- .../dev/yorkie/document/crdt/CrdtCounter.kt | 4 +- .../dev/yorkie/document/crdt/CrdtObject.kt | 11 +- .../dev/yorkie/document/crdt/CrdtPrimitive.kt | 130 ++++++++++++------ .../dev/yorkie/document/crdt/CrdtRoot.kt | 4 +- .../yorkie/document/crdt/CrdtTextElement.kt | 2 +- .../dev/yorkie/document/crdt/RhtPQMap.kt | 42 +++--- .../dev/yorkie/document/json/JsonElement.kt | 33 ++++- .../dev/yorkie/document/json/JsonObject.kt | 36 ++++- .../dev/yorkie/document/json/JsonPrimitive.kt | 37 +++-- .../yorkie/document/json/JsonStringifier.kt | 8 +- .../document/operation/RemoveOperation.kt | 7 - .../dev/yorkie/document/time/TimeTicket.kt | 2 +- .../dev/yorkie/document/DocumentTest.kt | 9 +- .../yorkie/document/crdt/CrdtCounterTest.kt | 10 ++ .../yorkie/document/crdt/CrdtPrimitiveTest.kt | 71 ++++++++-- .../yorkie/document/json/JsonObjectTest.kt | 80 +++++++++++ .../yorkie/document/json/JsonPrimitiveTest.kt | 45 ++++++ 20 files changed, 441 insertions(+), 144 deletions(-) create mode 100644 yorkie/src/test/kotlin/dev/yorkie/document/json/JsonObjectTest.kt create mode 100644 yorkie/src/test/kotlin/dev/yorkie/document/json/JsonPrimitiveTest.kt diff --git a/yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt b/yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt index 3ab56475e..f398022d4 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt @@ -19,7 +19,6 @@ import dev.yorkie.document.crdt.CrdtCounter.CounterType import dev.yorkie.document.crdt.CrdtElement import dev.yorkie.document.crdt.CrdtObject import dev.yorkie.document.crdt.CrdtPrimitive -import dev.yorkie.document.crdt.PrimitiveType import dev.yorkie.document.crdt.RgaTreeList import dev.yorkie.document.crdt.RhtPQMap @@ -102,16 +101,16 @@ internal fun PBPrimitive.toCrdtPrimitive(): CrdtPrimitive { ) } -internal fun PBValueType.toPrimitiveType(): PrimitiveType { +internal fun PBValueType.toPrimitiveType(): CrdtPrimitive.Type { return when (this) { - PBValueType.VALUE_TYPE_NULL -> PrimitiveType.Null - PBValueType.VALUE_TYPE_BOOLEAN -> PrimitiveType.Boolean - PBValueType.VALUE_TYPE_INTEGER -> PrimitiveType.Integer - PBValueType.VALUE_TYPE_LONG -> PrimitiveType.Long - PBValueType.VALUE_TYPE_DOUBLE -> PrimitiveType.Double - PBValueType.VALUE_TYPE_STRING -> PrimitiveType.String - PBValueType.VALUE_TYPE_BYTES -> PrimitiveType.Bytes - PBValueType.VALUE_TYPE_DATE -> PrimitiveType.Date + PBValueType.VALUE_TYPE_NULL -> CrdtPrimitive.Type.Null + PBValueType.VALUE_TYPE_BOOLEAN -> CrdtPrimitive.Type.Boolean + PBValueType.VALUE_TYPE_INTEGER -> CrdtPrimitive.Type.Integer + PBValueType.VALUE_TYPE_LONG -> CrdtPrimitive.Type.Long + PBValueType.VALUE_TYPE_DOUBLE -> CrdtPrimitive.Type.Double + PBValueType.VALUE_TYPE_STRING -> CrdtPrimitive.Type.String + PBValueType.VALUE_TYPE_BYTES -> CrdtPrimitive.Type.Bytes + PBValueType.VALUE_TYPE_DATE -> CrdtPrimitive.Type.Date else -> error("unimplemented type $this") } } @@ -157,7 +156,7 @@ internal fun CrdtObject.toPBJsonObject(): PBJsonElement { } } -internal fun List>.toPBRhtNodes(): List { +internal fun List>.toPBRhtNodes(): List { return map { rHTNode { key = it.strKey @@ -189,7 +188,7 @@ internal fun CrdtPrimitive.toPBPrimitive(): PBJsonElement { return jSONElement { primitive = primitive { type = crdtPrimitive.type.toPBValueType() - value = crdtPrimitive.toBytes().toByteString() + value = crdtPrimitive.toBytes() createdAt = crdtPrimitive.createdAt.toPBTimeTicket() crdtPrimitive.movedAt?.let { movedAt = it.toPBTimeTicket() } crdtPrimitive.removedAt?.let { removedAt = it.toPBTimeTicket() } @@ -197,16 +196,16 @@ internal fun CrdtPrimitive.toPBPrimitive(): PBJsonElement { } } -internal fun PrimitiveType.toPBValueType(): PBValueType { +internal fun CrdtPrimitive.Type.toPBValueType(): PBValueType { return when (this) { - PrimitiveType.Null -> PBValueType.VALUE_TYPE_NULL - PrimitiveType.Boolean -> PBValueType.VALUE_TYPE_BOOLEAN - PrimitiveType.Integer -> PBValueType.VALUE_TYPE_INTEGER - PrimitiveType.Long -> PBValueType.VALUE_TYPE_LONG - PrimitiveType.Double -> PBValueType.VALUE_TYPE_DOUBLE - PrimitiveType.String -> PBValueType.VALUE_TYPE_STRING - PrimitiveType.Bytes -> PBValueType.VALUE_TYPE_BYTES - PrimitiveType.Date -> PBValueType.VALUE_TYPE_DATE + CrdtPrimitive.Type.Null -> PBValueType.VALUE_TYPE_NULL + CrdtPrimitive.Type.Boolean -> PBValueType.VALUE_TYPE_BOOLEAN + CrdtPrimitive.Type.Integer -> PBValueType.VALUE_TYPE_INTEGER + CrdtPrimitive.Type.Long -> PBValueType.VALUE_TYPE_LONG + CrdtPrimitive.Type.Double -> PBValueType.VALUE_TYPE_DOUBLE + CrdtPrimitive.Type.String -> PBValueType.VALUE_TYPE_STRING + CrdtPrimitive.Type.Bytes -> PBValueType.VALUE_TYPE_BYTES + CrdtPrimitive.Type.Date -> PBValueType.VALUE_TYPE_DATE } } @@ -267,7 +266,7 @@ internal fun CrdtElement.toPBJsonElementSimple(): PBJsonElementSimple { is CrdtArray -> type = PBValueType.VALUE_TYPE_JSON_ARRAY is CrdtPrimitive -> { type = element.type.toPBValueType() - value = element.toBytes().toByteString() + value = element.toBytes() } is CrdtCounter -> { type = element.type.toPBCounterType() diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/change/CheckPoint.kt b/yorkie/src/main/kotlin/dev/yorkie/document/change/CheckPoint.kt index 76d429309..fe52bd872 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/change/CheckPoint.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/change/CheckPoint.kt @@ -18,7 +18,7 @@ internal data class CheckPoint( return if (increase == 0) { this } else { - CheckPoint(serverSeq, clientSeq + increase) + copy(clientSeq = clientSeq + increase) } } 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 1bd3ec61f..6e5ebd59b 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtArray.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtArray.kt @@ -104,11 +104,12 @@ internal data class CrdtArray( } override fun deepCopy(): CrdtElement { - val clone = copy(elements = RgaTreeList()) - elements.forEach { node -> - clone.elements.insertAfter(clone.lastCreatedAt, node.value.deepCopy()) + val elementsClone = RgaTreeList().apply { + elements.forEach { node -> + insertAfter(lastCreatedAt, node.value.deepCopy()) + } } - return clone + return copy(elements = elementsClone) } override fun iterator(): Iterator { diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtCounter.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtCounter.kt index 597917e02..52d7ea913 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtCounter.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtCounter.kt @@ -54,7 +54,7 @@ internal data class CrdtCounter private constructor( } override fun deepCopy(): CrdtElement { - return CrdtCounter(value, createdAt, _movedAt, _removedAt) + return copy() } companion object { @@ -66,7 +66,7 @@ internal data class CrdtCounter private constructor( _removedAt: TimeTicket? = null, ) = CrdtCounter(value.sanitized(), createdAt, _movedAt, _removedAt) - fun CounterValue.sanitized(): Number = when (counterType()) { + private fun CounterValue.sanitized(): Number = when (counterType()) { CounterType.IntegerCnt -> toInt() CounterType.LongCnt -> toLong() CounterType.DoubleCnt -> toDouble() diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtObject.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtObject.kt index 878291773..1bc27aed0 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtObject.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtObject.kt @@ -74,11 +74,12 @@ internal data class CrdtObject( * Copies itself deeply. */ override fun deepCopy(): CrdtObject { - val clone = CrdtObject(createdAt, _movedAt, _removedAt, RhtPQMap()) - rht.forEach { - clone.rht[it.strKey] = it.value.deepCopy() + val rhtClone = RhtPQMap().apply { + rht.forEach { (strKey, value) -> + set(strKey, value) + } } - return clone + return copy(rht = rhtClone) } /** @@ -106,7 +107,7 @@ internal data class CrdtObject( val node = nodes[index] if (!keySet.contains(node.strKey)) { keySet.add(node.strKey) - if (!node.isRemoved()) return true + if (!node.isRemoved) return true } index++ } diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtPrimitive.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtPrimitive.kt index 2c491ca9a..866ec2823 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtPrimitive.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtPrimitive.kt @@ -1,25 +1,30 @@ package dev.yorkie.document.crdt import com.google.protobuf.ByteString +import com.google.protobuf.kotlin.toByteString +import com.google.protobuf.kotlin.toByteStringUtf8 import dev.yorkie.document.time.TimeTicket import java.nio.ByteBuffer import java.util.Date -internal data class CrdtPrimitive( - val value: Any?, +@Suppress("DataClassPrivateConstructor") +internal data class CrdtPrimitive private constructor( + private val _value: Any?, override val createdAt: TimeTicket, override var _movedAt: TimeTicket? = null, override var _removedAt: TimeTicket? = null, ) : CrdtElement() { + val value: Any? = _value.sanitized() + val type = when (value) { - is Boolean -> PrimitiveType.Boolean - is Int -> PrimitiveType.Integer - is Long -> PrimitiveType.Long - is Double -> PrimitiveType.Double - is String -> PrimitiveType.String - is ByteArray -> PrimitiveType.Bytes - is Date -> PrimitiveType.Date - else -> PrimitiveType.Null + is Boolean -> Type.Boolean + is Int -> Type.Integer + is Long -> Type.Long + is Double -> Type.Double + is String -> Type.String + is ByteString -> Type.Bytes + is Date -> Type.Date + else -> Type.Null } val isNumericType = type in NUMERIC_TYPES @@ -28,57 +33,98 @@ internal data class CrdtPrimitive( * Copies itself deeply. */ override fun deepCopy(): CrdtElement { - return CrdtPrimitive(value, createdAt, _movedAt, _removedAt) + return when (value) { + is ByteString -> { + copy(_value = value.toByteArray().toByteString()) + } + is Date -> { + copy(_value = value.clone()) + } + else -> copy() + } } - fun toBytes(): ByteArray { + fun toBytes(): ByteString { return when (type) { - PrimitiveType.Null -> byteArrayOf() - PrimitiveType.Boolean -> byteArrayOf(if (value == true) 1 else 0) - PrimitiveType.Integer -> { - ByteBuffer.allocate(Int.SIZE_BYTES).putInt(value as Int).array() + Type.Null -> ByteString.EMPTY + Type.Boolean -> byteArrayOf(if (value == true) 1 else 0).toByteString() + Type.Integer -> { + ByteBuffer.allocate(Int.SIZE_BYTES) + .putInt(value as Int) + .array() + .toByteString() } - PrimitiveType.Long -> { - ByteBuffer.allocate(Long.SIZE_BYTES).putLong(value as Long).array() + Type.Long -> { + ByteBuffer.allocate(Long.SIZE_BYTES) + .putLong(value as Long) + .array() + .toByteString() } - PrimitiveType.Double -> { - ByteBuffer.allocate(Double.SIZE_BYTES).putDouble(value as Double).array() + Type.Double -> { + ByteBuffer.allocate(Double.SIZE_BYTES) + .putDouble(value as Double) + .array() + .toByteString() } - PrimitiveType.String -> (value as String).toByteArray() - PrimitiveType.Bytes -> value as ByteArray - PrimitiveType.Date -> { - ByteBuffer.allocate(Long.SIZE_BYTES).putLong((value as Date).time).array() + Type.String -> (value as String).toByteStringUtf8() + Type.Bytes -> value as ByteString + Type.Date -> { + ByteBuffer.allocate(Long.SIZE_BYTES) + .putLong((value as Date).time) + .array() + .toByteString() } } } companion object { private val NUMERIC_TYPES = setOf( - PrimitiveType.Integer, - PrimitiveType.Long, - PrimitiveType.Double, + Type.Integer, + Type.Long, + Type.Double, ) - fun fromBytes(type: PrimitiveType, bytes: ByteString): Any? { + operator fun invoke( + value: Any?, + createdAt: TimeTicket, + _movedAt: TimeTicket? = null, + _removedAt: TimeTicket? = null, + ) = CrdtPrimitive(value.sanitized(), createdAt, _movedAt, _removedAt) + + private fun Any?.sanitized(): Any? = when (this) { + is Boolean -> this + is Byte -> toInt() + is Short -> toInt() + is Int -> this + is Long -> this + is Number -> toDouble() + is CharSequence -> toString() + is ByteArray -> toByteString() + is ByteString -> this + is Date -> this + else -> null + } + + fun fromBytes(type: Type, bytes: ByteString): Any? { fun ByteString.asByteBuffer() = ByteBuffer.wrap(toByteArray()) return when (type) { - PrimitiveType.Null -> null - PrimitiveType.Boolean -> bytes.first().toInt() == 1 - PrimitiveType.Integer -> bytes.asByteBuffer().int - PrimitiveType.Long -> bytes.asByteBuffer().long - PrimitiveType.Double -> bytes.asByteBuffer().double - PrimitiveType.String -> bytes.toStringUtf8() - PrimitiveType.Bytes -> bytes.toByteArray() - PrimitiveType.Date -> Date(bytes.asByteBuffer().long) + Type.Null -> null + Type.Boolean -> bytes.first().toInt() == 1 + Type.Integer -> bytes.asByteBuffer().int + Type.Long -> bytes.asByteBuffer().long + Type.Double -> bytes.asByteBuffer().double + Type.String -> bytes.toStringUtf8() + Type.Bytes -> bytes + Type.Date -> Date(bytes.asByteBuffer().long) } } } -} -/** - * Primitive is a CRDT element that represents a primitive value. - */ -internal enum class PrimitiveType { - Null, Boolean, Integer, Long, Double, String, Bytes, Date + /** + * Primitive is a CRDT element that represents a primitive value. + */ + internal enum class Type { + Null, Boolean, Integer, Long, Double, String, Bytes, Date + } } diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtRoot.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtRoot.kt index 076306080..5b18d3d10 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtRoot.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtRoot.kt @@ -94,7 +94,7 @@ internal class CrdtRoot(val rootObject: CrdtObject) { /** * Returns length of nodes which can be garbage collected. */ - fun getGarbageLen(): Int { + fun getGarbageLength(): Int { var count = 0 removedElementSetByCreatedAt.forEach { createdAt -> count++ @@ -110,7 +110,7 @@ internal class CrdtRoot(val rootObject: CrdtObject) { textWithGarbageSetByCreatedAt.forEach { createdAt -> val pair = elementPairMapByCreatedAt[createdAt] ?: return@forEach val text = pair.element as CrdtTextElement - count += text.getRemovedNodesLen() + count += text.getRemovedNodesLength() } return count diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTextElement.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTextElement.kt index 18aacc09c..a0b2c68c0 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTextElement.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTextElement.kt @@ -7,7 +7,7 @@ import dev.yorkie.document.time.TimeTicket */ internal abstract class CrdtTextElement : CrdtElement() { - abstract fun getRemovedNodesLen(): Int + abstract fun getRemovedNodesLength(): Int abstract fun deleteTextNodesWithGarbage(executedAt: TimeTicket): Int } diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/RhtPQMap.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/RhtPQMap.kt index cff7cb8e0..87fc433eb 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/RhtPQMap.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/RhtPQMap.kt @@ -7,13 +7,12 @@ import dev.yorkie.util.YorkieLogger /** * [RhtPQMap] is replicated hash table with a priority queue by creation time. */ -internal class RhtPQMap : Iterable> { +internal class RhtPQMap : Iterable> { private val logTag = "RhtPQMap" - private val elementQueueMapByKey: - MutableMap>> = mutableMapOf() + private val elementQueueMapByKey: MutableMap>> = mutableMapOf() - private val nodeMapByCreatedAt: MutableMap> = mutableMapOf() + private val nodeMapByCreatedAt: MutableMap> = mutableMapOf() /** * Sets the [value] using the given [key]. @@ -23,8 +22,8 @@ internal class RhtPQMap : Iterable> { var removed: T? = null val queue = elementQueueMapByKey[key] if (queue?.isNotEmpty() == true) { - val node = queue.peek() as RhtPQMapNode - if (!node.isRemoved() && node.remove(value.createdAt)) { + val node = queue.peek() as Node + if (!node.isRemoved && node.remove(value.createdAt)) { removed = node.value } } @@ -36,7 +35,7 @@ internal class RhtPQMap : Iterable> { * Sets the [value] using the given [key] internally. */ private fun setInternal(key: String, value: T) { - val node = RhtPQMapNode(key, value) + val node = Node(key, value) val queue = elementQueueMapByKey.getOrPut(key) { MaxPriorityQueue() } queue.add(node) nodeMapByCreatedAt[value.createdAt] = node @@ -47,7 +46,7 @@ internal class RhtPQMap : Iterable> { * and [TimeTicket] will be removed by [executedAt]. * If the object exists in [nodeMapByCreatedAt] by same key [createdAt], it will return. * - * @throws NoSuchElementException if [RhtPQMapNode] doesn't exist. + * @throws NoSuchElementException if [Node] doesn't exist. */ fun remove(createdAt: TimeTicket, executedAt: TimeTicket): T { val node = nodeMapByCreatedAt[createdAt] @@ -57,7 +56,7 @@ internal class RhtPQMap : Iterable> { } /** - * Returns [RhtPQMapNode.strKey] of node based on [createdAt]. + * Returns [Node.strKey] of node based on [createdAt]. * The node will be found in [nodeMapByCreatedAt] using [createdAt] * * @throws NoSuchElementException if RHTPQMapNode doesn't exist. @@ -106,12 +105,12 @@ internal class RhtPQMap : Iterable> { /** * Checks the element exists in [elementQueueMapByKey] using [key]. - * If the [RhtPQMapNode] is exist, then returns true, otherwise false. + * If the [Node] is exist, then returns true, otherwise false. */ fun has(key: String): Boolean { val queue = elementQueueMapByKey[key] ?: return false val node = queue.peek() ?: return false - return !node.isRemoved() + return !node.isRemoved } /** @@ -129,14 +128,14 @@ internal class RhtPQMap : Iterable> { * * @throws NoSuchElementException if there is an empty MaxPriorityQueue. */ - fun getKeyOfQueue(): Sequence> { + fun getKeyOfQueue(): Sequence> { return elementQueueMapByKey.values .asSequence() .map { it.element() } } - override fun iterator(): Iterator> { - return object : Iterator> { + override fun iterator(): Iterator> { + return object : Iterator> { val pqIterators = elementQueueMapByKey.values.map { it.iterator() } var nodes = buildList { pqIterators.forEach { @@ -149,7 +148,7 @@ internal class RhtPQMap : Iterable> { override fun hasNext() = index < nodes.size - override fun next(): RhtPQMapNode { + override fun next(): Node { return nodes[index++] } } @@ -167,19 +166,18 @@ internal class RhtPQMap : Iterable> { } /** - * [RhtPQMapNode] is a node of [RhtPQMap]. + * [Node] is a node of [RhtPQMap]. */ - data class RhtPQMapNode( + data class Node( val strKey: String, val value: T, - ) : Comparable> { + ) : Comparable> { /** * Checks whether this value was removed. */ - fun isRemoved(): Boolean { - return value.isRemoved - } + val isRemoved: Boolean + get() = value.isRemoved /** * Removes a value base on removing time. @@ -188,7 +186,7 @@ internal class RhtPQMap : Iterable> { return value.remove(removedAt) } - override fun compareTo(other: RhtPQMapNode): Int { + override fun compareTo(other: Node): Int { return value.createdAt.compareTo(other.value.createdAt) } } 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 713e4dcf4..f4179321b 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonElement.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonElement.kt @@ -14,15 +14,34 @@ public abstract class JsonElement { override fun toString() = toJson() companion object { + private val TypeMapper = mapOf( + CrdtObject::class.java to JsonObject::class.java, + CrdtArray::class.java to JsonArray::class.java, + CrdtPrimitive::class.java to JsonPrimitive::class.java, + ) @Suppress("UNCHECKED_CAST") - internal fun CrdtElement.toJsonElement(context: ChangeContext): T { - return when (this) { - is CrdtObject -> JsonObject(context, this) - is CrdtArray -> JsonArray(context, this) - is CrdtPrimitive -> JsonPrimitive(this) - else -> error("unknown CrdtElement type: $this") - } as T + internal inline fun CrdtElement.toJsonElement( + context: ChangeContext, + ): T { + val clazz = if (T::class.java == JsonElement::class.java) { + TypeMapper[this::class.java] + } else { + T::class.java + } + return try { + when (clazz) { + JsonObject::class.java -> JsonObject(context, this as CrdtObject) + JsonArray::class.java -> JsonArray(context, this as CrdtArray) + JsonPrimitive::class.java -> JsonPrimitive(this as CrdtPrimitive) + else -> throw TypeCastException("unknown CrdtElement type: $this") + } as T + } catch (e: ClassCastException) { + if (e is TypeCastException) { + throw e + } + throw TypeCastException(e.message) + } } } } 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 0df0e6442..fdc36963e 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonObject.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonObject.kt @@ -92,7 +92,41 @@ public class JsonObject internal constructor( ) } - public operator fun get(key: String) = target[key].toJsonElement(context) + public operator fun get(key: String): JsonElement { + return try { + target[key].toJsonElement(context) + } catch (e: IllegalStateException) { + throw NoSuchElementException("element with key: $key does not exist") + } + } + + public fun getOrNull(key: String): JsonElement? { + return try { + get(key) + } catch (e: NoSuchElementException) { + null + } + } + + @Suppress("NON_PUBLIC_CALL_FROM_PUBLIC_INLINE") + public inline fun getAs(key: String): T { + return try { + target[key].toJsonElement(context) + } catch (e: IllegalStateException) { + throw NoSuchElementException("element with key: $key does not exist") + } + } + + @Suppress("NON_PUBLIC_CALL_FROM_PUBLIC_INLINE") + public inline fun getAsOrNull(key: String): T? { + return try { + getAs(key) + } catch (e: TypeCastException) { + null + } catch (e: NoSuchElementException) { + null + } + } public fun remove(key: String) { val executedAt = context.issueTimeTicket() diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonPrimitive.kt b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonPrimitive.kt index 8421d5603..52988f591 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonPrimitive.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonPrimitive.kt @@ -1,9 +1,9 @@ package dev.yorkie.document.json +import com.google.protobuf.ByteString import dev.yorkie.document.crdt.CrdtPrimitive -import dev.yorkie.document.crdt.PrimitiveType +import dev.yorkie.document.crdt.CrdtPrimitive.Type import java.util.Date -import kotlin.reflect.KProperty public class JsonPrimitive internal constructor( override val target: CrdtPrimitive, @@ -13,25 +13,25 @@ public class JsonPrimitive internal constructor( public val type: Type get() = when (target.type) { - PrimitiveType.Null -> Type.Null - PrimitiveType.Boolean -> Type.Boolean - PrimitiveType.Integer -> Type.Integer - PrimitiveType.Long -> Type.Long - PrimitiveType.Double -> Type.Double - PrimitiveType.String -> Type.String - PrimitiveType.Bytes -> Type.Bytes - PrimitiveType.Date -> Type.Date + CrdtPrimitive.Type.Null -> Type.Null + CrdtPrimitive.Type.Boolean -> Type.Boolean + CrdtPrimitive.Type.Integer -> Type.Integer + CrdtPrimitive.Type.Long -> Type.Long + CrdtPrimitive.Type.Double -> Type.Double + CrdtPrimitive.Type.String -> Type.String + CrdtPrimitive.Type.Bytes -> Type.Bytes + CrdtPrimitive.Type.Date -> Type.Date } - public inline operator fun getValue(thisObj: Any?, property: KProperty<*>): T? { - val value = value ?: return null + public inline fun getValueAs(): T { + val value = checkNotNull(value) val isTypeValid = type == when (T::class) { Boolean::class -> Type.Boolean Int::class -> Type.Integer Long::class -> Type.Long Double::class -> Type.Double String::class -> Type.String - ByteArray::class -> Type.Bytes + ByteString::class -> Type.Bytes Date::class -> Type.Date else -> false } @@ -43,6 +43,17 @@ public class JsonPrimitive internal constructor( ) } + public inline fun getValueAsOrNull(): T? { + return runCatching { + getValueAs() + }.recover { + when (it) { + is IllegalStateException, is TypeCastException -> null + else -> throw it + } + }.getOrThrow() + } + public enum class Type { Null, Boolean, Integer, Long, Double, String, Bytes, Date } diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonStringifier.kt b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonStringifier.kt index 92e79c000..15f533715 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonStringifier.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/json/JsonStringifier.kt @@ -1,11 +1,11 @@ package dev.yorkie.document.json +import com.google.protobuf.ByteString import dev.yorkie.document.crdt.CrdtArray import dev.yorkie.document.crdt.CrdtCounter import dev.yorkie.document.crdt.CrdtElement import dev.yorkie.document.crdt.CrdtObject import dev.yorkie.document.crdt.CrdtPrimitive -import dev.yorkie.document.crdt.PrimitiveType import java.util.Date internal object JsonStringifier { @@ -21,9 +21,9 @@ internal object JsonStringifier { is CrdtPrimitive -> { buffer.append( when (type) { - PrimitiveType.String -> """"${escapeString(value as String)}"""" - PrimitiveType.Bytes -> """"${(value as ByteArray).decodeToString()}"""" - PrimitiveType.Date -> (value as Date).time.toString() + CrdtPrimitive.Type.String -> """"${escapeString(value as String)}"""" + CrdtPrimitive.Type.Bytes -> """"${(value as ByteString).toStringUtf8()}"""" + CrdtPrimitive.Type.Date -> (value as Date).time.toString() else -> "$value" }, ) diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/operation/RemoveOperation.kt b/yorkie/src/main/kotlin/dev/yorkie/document/operation/RemoveOperation.kt index 3467dc11a..7d93bbccd 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/operation/RemoveOperation.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/operation/RemoveOperation.kt @@ -36,13 +36,6 @@ internal data class RemoveOperation( } } - /** - * Returns a string containing the meta data. - */ - override fun toString(): String { - return "$parentCreatedAt.REMOVE" - } - companion object { private const val TAG = "RemoveOperation" } diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/time/TimeTicket.kt b/yorkie/src/main/kotlin/dev/yorkie/document/time/TimeTicket.kt index 368aaddd6..5a9c0c003 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/time/TimeTicket.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/time/TimeTicket.kt @@ -20,7 +20,7 @@ public data class TimeTicket( override fun compareTo(other: TimeTicket): Int { return lamport.compareTo(other.lamport).takeUnless { it == 0 } ?: actorID.compareTo(other.actorID).takeUnless { it == 0 } - ?: delimiter.compareTo(delimiter) + ?: delimiter.compareTo(other.delimiter) } companion object { diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/DocumentTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/DocumentTest.kt index 4dc347303..4d8a0bf35 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/DocumentTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/DocumentTest.kt @@ -44,7 +44,7 @@ class DocumentTest { result = target.updateAsync { it.remove("k1") - val array: JsonArray = it["k3"] + val array = it.getAs("k3") array.removeAt(0) it.remove("k2") array.removeAt(2) @@ -88,6 +88,11 @@ class DocumentTest { array.put("test") array.put("bytes".toByteArray()) array.put(Date(10_000)) + val arrayObj = array.putNewObject() + arrayObj["k1"] = 1 + val arrayArray = array.putNewArray() + arrayArray.put(1) + arrayArray.put(2) }.await() assertJsonContentEquals( @@ -101,7 +106,7 @@ class DocumentTest { "k6": "test string\n\n", "k7": "bytes", "k8": 1000, - "k9": [true, 1, 100, 111.111, "test", "bytes", 10000] + "k9": [true, 1, 100, 111.111, "test", "bytes", 10000, {"k1": 1}, [1, 2]] } }""", target.toJson(), diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtCounterTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtCounterTest.kt index ec3b9a055..ceb37ed98 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtCounterTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtCounterTest.kt @@ -8,6 +8,8 @@ import org.junit.Assert.assertThrows import org.junit.Test import java.nio.ByteBuffer import java.util.Date +import kotlin.test.assertNotSame +import kotlin.test.assertSame class CrdtCounterTest { @@ -114,4 +116,12 @@ class CrdtCounterTest { minInt.increase(CrdtPrimitive(-1, InitialTimeTicket)) assertEquals(Int.MIN_VALUE - 1L, minInt.value) } + + @Test + fun `should use same instance for value on deepCopy`() { + val counter = CrdtCounter(4, InitialTimeTicket) + val clone = counter.deepCopy() as CrdtCounter + assertNotSame(counter, clone) + assertSame(counter.value, clone.value) + } } diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtPrimitiveTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtPrimitiveTest.kt index 790fa8917..05c8ff3d1 100644 --- a/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtPrimitiveTest.kt +++ b/yorkie/src/test/kotlin/dev/yorkie/document/crdt/CrdtPrimitiveTest.kt @@ -3,38 +3,93 @@ package dev.yorkie.document.crdt import dev.yorkie.document.time.TimeTicket import org.junit.Assert.assertTrue import org.junit.Test +import java.util.Date +import kotlin.test.assertEquals +import kotlin.test.assertNotSame +import kotlin.test.assertNull +import kotlin.test.assertSame class CrdtPrimitiveTest { @Test fun `Verify the value`() { val primitive = CrdtPrimitive("hello", TimeTicket.InitialTimeTicket) - assertTrue("hello" == primitive.value) + assertEquals("hello", primitive.value) } @Test fun `Verify that the value of Primitive and PrimitiveType are mapped correctly`() { var primitive = CrdtPrimitive(true, TimeTicket.InitialTimeTicket) - assertTrue(PrimitiveType.Boolean == primitive.type) + assertTrue(CrdtPrimitive.Type.Boolean == primitive.type) primitive = CrdtPrimitive(1, TimeTicket.InitialTimeTicket) - assertTrue(PrimitiveType.Integer == primitive.type) + assertTrue(CrdtPrimitive.Type.Integer == primitive.type) primitive = CrdtPrimitive(1L, TimeTicket.InitialTimeTicket) - assertTrue(PrimitiveType.Long == primitive.type) + assertTrue(CrdtPrimitive.Type.Long == primitive.type) primitive = CrdtPrimitive(1.toDouble(), TimeTicket.InitialTimeTicket) - assertTrue(PrimitiveType.Double == primitive.type) + assertTrue(CrdtPrimitive.Type.Double == primitive.type) primitive = CrdtPrimitive("hello", TimeTicket.InitialTimeTicket) - assertTrue(PrimitiveType.String == primitive.type) + assertTrue(CrdtPrimitive.Type.String == primitive.type) primitive = CrdtPrimitive( byteArrayOf(-0x01, -0x01, 0x02, -0x02), TimeTicket.InitialTimeTicket, ) - assertTrue(PrimitiveType.Bytes == primitive.type) + assertTrue(CrdtPrimitive.Type.Bytes == primitive.type) primitive = CrdtPrimitive(null, TimeTicket.InitialTimeTicket) - assertTrue(PrimitiveType.Null == primitive.type) + assertTrue(CrdtPrimitive.Type.Null == primitive.type) + } + + @Test + fun `should create new instance when deep copying Date and ByteArray`() { + val datePrimitive = CrdtPrimitive(Date(1_000L), TimeTicket.InitialTimeTicket) + val dateClone = datePrimitive.deepCopy() as CrdtPrimitive + assertEquals(datePrimitive.value, dateClone.value) + assertNotSame(datePrimitive.value, dateClone.value) + + val byteArrayPrimitive = CrdtPrimitive( + ByteArray(4) { 1 }, + TimeTicket.InitialTimeTicket, + ) + val byteArrayClone = byteArrayPrimitive.deepCopy() as CrdtPrimitive + assertEquals(byteArrayPrimitive.value, byteArrayClone.value) + assertNotSame(byteArrayPrimitive.value, byteArrayClone.value) + } + + @Test + fun `should return same instance when deep copying immutable values`() { + val intPrimitive = CrdtPrimitive(1, TimeTicket.InitialTimeTicket) + val intClone = intPrimitive.deepCopy() as CrdtPrimitive + assertNotSame(intPrimitive, intClone) + assertSame(intPrimitive.value, intClone.value) + + val stringPrimitive = CrdtPrimitive("str", TimeTicket.InitialTimeTicket) + val stringClone = stringPrimitive.deepCopy() as CrdtPrimitive + assertNotSame(stringPrimitive, stringClone) + assertSame(stringPrimitive.value, stringClone.value) + + val boolPrimitive = CrdtPrimitive(false, TimeTicket.InitialTimeTicket) + val boolClone = boolPrimitive.deepCopy() as CrdtPrimitive + assertNotSame(boolPrimitive, boolClone) + assertSame(boolPrimitive.value, boolClone.value) + } + + @Test + fun `should sanitize value to supported types`() { + val bytePrimitive = CrdtPrimitive(14.toByte(), TimeTicket.InitialTimeTicket) + assertEquals(14, bytePrimitive.value) + + val shortPrimitive = CrdtPrimitive(1.toShort(), TimeTicket.InitialTimeTicket) + assertEquals(1, shortPrimitive.value) + + val numberPrimitive = CrdtPrimitive(111.111.toBigDecimal(), TimeTicket.InitialTimeTicket) + assertEquals(111.111, numberPrimitive.value) + + val unsupported = CrdtPrimitive(Unit, TimeTicket.InitialTimeTicket) + assertNull(unsupported.value) + assertEquals(CrdtPrimitive.Type.Null, unsupported.type) } } diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonObjectTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonObjectTest.kt new file mode 100644 index 000000000..b479b40ee --- /dev/null +++ b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonObjectTest.kt @@ -0,0 +1,80 @@ +package dev.yorkie.document.json + +import dev.yorkie.document.change.ChangeContext +import dev.yorkie.document.change.ChangeID +import dev.yorkie.document.crdt.CrdtObject +import dev.yorkie.document.crdt.CrdtRoot +import dev.yorkie.document.crdt.RhtPQMap +import dev.yorkie.document.time.TimeTicket +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertThrows +import org.junit.Before +import org.junit.Test +import kotlin.test.assertIs + +class JsonObjectTest { + private lateinit var target: JsonObject + + @Before + fun setUp() { + val obj = CrdtObject(TimeTicket.InitialTimeTicket, rht = RhtPQMap()) + target = JsonObject( + ChangeContext( + ChangeID.InitialChangeID, + CrdtRoot(obj), + null, + ), + obj, + ) + } + + @Test + fun `should throw when accessing a key not added with get function`() { + assertThrows(NoSuchElementException::class.java) { + target["k1"] + } + } + + @Test + fun `should return null when accessing a key not added with getOrNull function`() { + assertNull(target.getOrNull("k1")) + } + + @Test + fun `should throw when accessing a key not added with getAs function`() { + assertThrows(NoSuchElementException::class.java) { + target.getAs("k1") + } + } + + @Test + fun `should return null when accessing a key not added with getAsOrNull function`() { + assertNull(target.getAsOrNull("k1")) + } + + @Test + fun `should return requested type with getAs`() { + target["k1"] = 1 + val get = assertIs(target["k1"]) + assertEquals(1, get.value) + + val getAs = assertIs(target.getAs("k1")) + assertEquals(1, getAs.value) + + assertThrows(TypeCastException::class.java) { + target.getAs("k1") + } + + assertNull(target.getAsOrNull("k1")) + } + + @Test + fun `should return all keys added to object`() { + target["k1"] = "v1" + target["k2"] = true + target["k3"] = false + target.remove("k3") + assertEquals(listOf("k1", "k2"), target.keys) + } +} diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonPrimitiveTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonPrimitiveTest.kt new file mode 100644 index 000000000..3c46a3ec5 --- /dev/null +++ b/yorkie/src/test/kotlin/dev/yorkie/document/json/JsonPrimitiveTest.kt @@ -0,0 +1,45 @@ +package dev.yorkie.document.json + +import dev.yorkie.document.crdt.CrdtPrimitive +import dev.yorkie.document.time.TimeTicket +import org.junit.Assert.assertThrows +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +class JsonPrimitiveTest { + + @Test + fun `should get value as requested type if castable`() { + val json = JsonPrimitive(CrdtPrimitive(4, TimeTicket.InitialTimeTicket)) + assertEquals(4, json.getValueAs()) + } + + @Test + fun `should throw TypeCastException if requested value type is not valid`() { + assertThrows(TypeCastException::class.java) { + val json = JsonPrimitive(CrdtPrimitive(4, TimeTicket.InitialTimeTicket)) + json.getValueAs() + } + } + + @Test + fun `should throw IllegalStateException when requested non-null value when it is null`() { + assertThrows(IllegalStateException::class.java) { + val json = JsonPrimitive(CrdtPrimitive(null, TimeTicket.InitialTimeTicket)) + json.getValueAs() + } + } + + @Test + fun `should return null when requested type is not valid with getValueOrNull`() { + val json = JsonPrimitive(CrdtPrimitive(4, TimeTicket.InitialTimeTicket)) + assertNull(json.getValueAsOrNull()) + } + + @Test + fun `should return null when value is null with getValueOrNull`() { + val json = JsonPrimitive(CrdtPrimitive(null, TimeTicket.InitialTimeTicket)) + assertNull(json.getValueAsOrNull()) + } +}