Merge pull request #795 from modelix/fix-merging-versions-in-v1

fix(model-server): fix merging versions through V1-API

Author: Oleksandr Dzhychko

model-server/src/main/kotlin/org/modelix/model/server/handlers/RepositoriesManager.kt

@@ -277,27 +277,28 @@ class RepositoriesManager(val client: LocalModelClient) : IRepositoriesManager {
      * Same as [mergeChanges] but blocking.
      * Caller is expected to execute it outside the request thread.
      */
-    override fun mergeChangesBlocking(branch: BranchReference, newVersionHash: String): String {
-        return store.runTransaction {
-            val headHash = getVersionHashBlocking(branch)
-            val mergedHash = if (headHash == null) {
-                newVersionHash
-            } else {
-                val headVersion = CLVersion(headHash, client.storeCache)
-                val newVersion = CLVersion(newVersionHash, client.storeCache)
-                require(headVersion.getTree().getId() == newVersion.getTree().getId()) {
-                    "Attempt to merge a model with ID '${newVersion.getTree().getId()}'" +
-                        " into one with ID '${headVersion.getTree().getId()}'"
+    override fun mergeChangesBlocking(branch: BranchReference, newVersionHash: String): String =
+        runWithRepositoryBlocking(branch.repositoryId) {
+            store.runTransaction {
+                val headHash = getVersionHashBlocking(branch)
+                val mergedHash = if (headHash == null) {
+                    newVersionHash
+                } else {
+                    val headVersion = CLVersion(headHash, client.storeCache)
+                    val newVersion = CLVersion(newVersionHash, client.storeCache)
+                    require(headVersion.getTree().getId() == newVersion.getTree().getId()) {
+                        "Attempt to merge a model with ID '${newVersion.getTree().getId()}'" +
+                            " into one with ID '${headVersion.getTree().getId()}'"
+                    }
+                    val mergedVersion = VersionMerger(client.storeCache, client.idGenerator)
+                        .mergeChange(headVersion, newVersion)
+                    mergedVersion.getContentHash()
                 }
-                val mergedVersion = VersionMerger(client.storeCache, client.idGenerator)
-                    .mergeChange(headVersion, newVersion)
-                mergedVersion.getContentHash()
+                ensureBranchInList(branch)
+                putVersionHash(branch, mergedHash)
+                mergedHash
             }
-            ensureBranchInList(branch)
-            putVersionHash(branch, mergedHash)
-            mergedHash
         }
-    }
 
     override suspend fun getVersion(branch: BranchReference): CLVersion? {
         return getVersionHash(branch)?.let { getVersion(branch.repositoryId, it) }
@@ -440,6 +441,10 @@ class RepositoriesManager(val client: LocalModelClient) : IRepositoriesManager {
         return store.withRepositoryInCoroutine(repository.takeIf { isIsolated(repository) == true }, body)
     }
 
+    private fun <R> runWithRepositoryBlocking(repository: RepositoryId, body: () -> R): R {
+        return store.withRepository(repository.takeIf { isIsolated(repository) == true }, body)
+    }
+
     companion object {
         private val LOG = LoggerFactory.getLogger(RepositoriesManager::class.java)
         const val KEY_PREFIX = ":v2"

model-server/src/test/kotlin/org/modelix/model/server/handlers/KeyValueLikeModelServerTest.kt

@@ -27,25 +27,33 @@ import kotlinx.serialization.json.Json
 import kotlinx.serialization.json.JsonElement
 import org.modelix.authorization.installAuthentication
 import org.modelix.model.InMemoryModels
+import org.modelix.model.client.RestWebModelClient
+import org.modelix.model.client2.runWrite
+import org.modelix.model.lazy.CLVersion
+import org.modelix.model.lazy.RepositoryId
+import org.modelix.model.server.createModelClient
 import org.modelix.model.server.installDefaultServerPlugins
 import org.modelix.model.server.store.InMemoryStoreClient
 import org.modelix.model.server.store.LocalModelClient
+import org.modelix.model.server.store.forContextRepository
 import org.modelix.model.server.store.forGlobalRepository
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertTrue
 
 class KeyValueLikeModelServerTest {
 
     private fun runTest(block: suspend ApplicationTestBuilder.() -> Unit) = testApplication {
-        val storeClient = InMemoryStoreClient()
-        val modelClient = LocalModelClient(storeClient.forGlobalRepository())
-        val repositoriesManager = RepositoriesManager(modelClient)
-        val handler = KeyValueLikeModelServer(repositoriesManager, modelClient.store, InMemoryModels())
+        val inMemoryModels = InMemoryModels()
+        val store = InMemoryStoreClient()
+        val localModelClient = LocalModelClient(store.forContextRepository())
+        val repositoriesManager = RepositoriesManager(localModelClient)
 
         application {
             installAuthentication(unitTestMode = true)
             installDefaultServerPlugins()
-            handler.init(this)
+            KeyValueLikeModelServer(repositoriesManager, store.forGlobalRepository(), InMemoryModels()).init(this)
+            ModelReplicationServer(repositoriesManager, localModelClient, inMemoryModels).init(this)
         }
 
         block()
@@ -85,4 +93,26 @@ class KeyValueLikeModelServerTest {
             )
         assertEquals(expected, actual)
     }
+
+    @Test
+    fun `model client V1 can merge versions`() = runTest {
+        val clientV1 = RestWebModelClient(baseUrl = "http://localhost/", providedClient = client)
+        val clientV2 = createModelClient()
+        val repositoryId = RepositoryId("repo1")
+        val branchA = repositoryId.getBranchReference("branchA")
+        val branchB = repositoryId.getBranchReference("branchB")
+        clientV2.initRepositoryWithLegacyStorage(repositoryId)
+        clientV2.runWrite(branchA) { rootNode ->
+            rootNode.setPropertyValue("propertyA", "valueA")
+        }
+        clientV2.runWrite(branchB) { rootNode ->
+            rootNode.setPropertyValue("propertyB", "valueB")
+        }
+        val branchBHash = clientV2.pullHash(branchB)
+
+        clientV1.putA(branchA.getKey(), branchBHash)
+
+        val branchAVersion = clientV2.pull(branchA, null) as CLVersion
+        assertTrue(branchAVersion.isMerge())
+    }
 }