From 57e3d89cb8155c12bba707fba38c8d6eafd41966 Mon Sep 17 00:00:00 2001 From: Jeehyun Kim Date: Mon, 13 May 2024 16:22:41 +0900 Subject: [PATCH] Update `getRootThreadSafe()` to return a deep-copied `JsonObject` (#176) * update consumer-rules.pro * add getRootThreadSafe() --- .github/workflows/ci.yml | 36 +++++++++---------- yorkie/consumer-rules.pro | 3 ++ .../kotlin/dev/yorkie/document/Document.kt | 21 ++++++++--- .../dev/yorkie/document/crdt/CrdtTree.kt | 7 ++-- .../kotlin/dev/yorkie/document/crdt/Rht.kt | 7 ++-- 5 files changed, 47 insertions(+), 27 deletions(-) create mode 100644 yorkie/consumer-rules.pro diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c745364b1..0d457f1ce 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,14 +12,14 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 steps: - - uses: actions/checkout@v3 - - uses: actions/setup-java@v3 + - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 with: java-version: 17 distribution: 'zulu' cache: 'gradle' - run: chmod +x gradlew - - uses: gradle/gradle-build-action@v2 + - uses: gradle/actions/setup-gradle@v3 - run: ./gradlew lintKotlin - run: ./gradlew lint @@ -28,17 +28,17 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 steps: - - uses: actions/checkout@v3 - - uses: actions/setup-java@v3 + - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 with: java-version: 17 distribution: 'zulu' cache: 'gradle' - run: chmod +x gradlew - - uses: gradle/gradle-build-action@v2 + - uses: gradle/actions/setup-gradle@v3 - run: ./gradlew yorkie:testDebugUnitTest - run: ./gradlew yorkie:jacocoDebugTestReport - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 with: name: unit-test-artifact path: yorkie/build/reports/jacoco/jacocoDebugTestReport/jacocoDebugTestReport.xml @@ -53,18 +53,18 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 60 steps: - - uses: actions/checkout@v3 - - uses: actions/setup-java@v3 + - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 with: java-version: 17 distribution: 'zulu' cache: 'gradle' - run: chmod +x gradlew - - uses: gradle/gradle-build-action@v2 + - uses: gradle/actions/setup-gradle@v3 - run: | docker system prune --all --force docker-compose -f docker/docker-compose-ci.yml up --build -d - - uses: actions/cache@v3 + - uses: actions/cache@v4 id: avd-cache with: path: | @@ -101,7 +101,7 @@ jobs: - if: ${{ matrix.api-level == 24 }} run: ./gradlew yorkie:jacocoDebugTestReport - if: ${{ matrix.api-level == 24 }} - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: android-test-artifact path: yorkie/build/reports/jacoco/jacocoDebugTestReport/jacocoDebugTestReport.xml @@ -112,7 +112,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: actions/download-artifact@v2 - uses: codecov/codecov-action@v3 with: @@ -126,15 +126,15 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 60 steps: - - uses: actions/checkout@v3 - - uses: actions/setup-java@v3 + - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 with: java-version: 17 distribution: 'zulu' cache: 'gradle' - run: chmod +x gradlew - - uses: gradle/gradle-build-action@v2 - - uses: actions/cache@v3 + - uses: gradle/actions/setup-gradle@v3 + - uses: actions/cache@v4 id: avd-cache with: path: | @@ -169,7 +169,7 @@ jobs: adb shell find /sdcard/Download -name "*-benchmarkData.json" | tr -d '\r' | xargs -n1 adb pull - run: | echo "$(cat dev.yorkie.microbenchmark.test-benchmarkData.json)" - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 with: name: microbenchmark-artifact path: dev.yorkie.microbenchmark.test-benchmarkData.json diff --git a/yorkie/consumer-rules.pro b/yorkie/consumer-rules.pro new file mode 100644 index 000000000..9ec615fd1 --- /dev/null +++ b/yorkie/consumer-rules.pro @@ -0,0 +1,3 @@ +-keepclassmembernames class dev.yorkie.api.**.* { + *; +} diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt b/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt index 14d4cfb2a..c3287412d 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt @@ -420,12 +420,25 @@ public class Document( // TODO: also apply to root } + /** + * Returns a new proxy of cloned root. + */ public suspend fun getRoot(): JsonObject = withContext(dispatcher) { val clone = ensureClone() val context = ChangeContext(changeID.next(), clone.root) JsonObject(context, clone.root.rootObject) } + /** + * Returns a new proxy of deep-copied root. + * It ensures thread-safety by avoiding reuse of [clone]. + */ + public suspend fun getRootThreadSafe(): JsonObject = withContext(dispatcher) { + val clone = ensureClone().deepCopy() + val context = ChangeContext(changeID.next(), clone.root) + JsonObject(context, clone.root.rootObject) + } + /** * Deletes elements that were removed before the given time. */ @@ -554,8 +567,8 @@ public class Document( public val disableGC: Boolean = false, ) - internal data class RootClone( - val root: CrdtRoot, - val presences: Presences, - ) + internal data class RootClone(val root: CrdtRoot, val presences: Presences) { + + fun deepCopy() = copy(root = root.deepCopy(), presences = presences.asPresences()) + } } 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 c80efff61..9f865135f 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt @@ -20,7 +20,7 @@ public typealias TreePosRange = Pair internal typealias CrdtTreeToken = TreeToken -internal class CrdtTree( +internal data class CrdtTree( val root: CrdtTreeNode, override val createdAt: TimeTicket, override var _movedAt: TimeTicket? = null, @@ -556,7 +556,7 @@ internal class CrdtTree( * Copies itself deeply. */ override fun deepCopy(): CrdtElement { - return CrdtTree(root.deepCopy(), createdAt, movedAt, removedAt) + return copy(root = root.deepCopy()) } /** @@ -847,6 +847,9 @@ internal data class CrdtTreeNode private constructor( } it.insPrevID = insPrevID it.insNextID = insNextID + if (it.isText) { + it.value = value + } } } diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/Rht.kt b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/Rht.kt index 93359882a..ab02c9fde 100644 --- a/yorkie/src/main/kotlin/dev/yorkie/document/crdt/Rht.kt +++ b/yorkie/src/main/kotlin/dev/yorkie/document/crdt/Rht.kt @@ -14,7 +14,7 @@ internal class Rht : Collection { val nodeKeyValueMap: Map get() { - return nodeMapByKey.entries.associate { (key, node) -> + return nodeMapByKey.filterValues { !it.isRemoved }.entries.associate { (key, node) -> key to node.value } } @@ -23,13 +23,14 @@ internal class Rht : Collection { key: String, value: String, executedAt: TimeTicket, + isRemoved: Boolean = false, ) { val prev = nodeMapByKey[key] if (prev?.executedAt < executedAt) { if (prev?.isRemoved == false) { numberOfRemovedElements-- } - val node = Node(key, value, executedAt, false) + val node = Node(key, value, executedAt, isRemoved) nodeMapByKey[key] = node } } @@ -66,7 +67,7 @@ internal class Rht : Collection { val rht = Rht() nodeMapByKey.forEach { val node = it.value - rht.set(node.key, node.value, node.executedAt) + rht.set(node.key, node.value, node.executedAt, node.isRemoved) } return rht }