Fix: Migrate legacy name based roles

model-datastructure/src/commonMain/kotlin/org/modelix/datastructures/model/DefaultModelTree.kt

@@ -26,7 +26,7 @@ class Int64ModelTree(nodesMap: IPersistentMap<Long, NodeObjectData<Long>>, treeI
             id = getId(),
             int64Hamt = nodesMap.asObject().ref.upcast(),
             trieWithNodeRefIds = null,
-            usesRoleIds = true,
+            usesRoleIds = useRoleIds,
         ).asObject(graph)
     }
     override fun getNodeIdType(): IDataTypeConfiguration<Long> = LongDataTypeConfiguration()
@@ -49,7 +49,7 @@ class DefaultModelTree(
             id = getId(),
             int64Hamt = null,
             trieWithNodeRefIds = nodesMap.asObject().ref.upcast(),
-            usesRoleIds = true,
+            usesRoleIds = useRoleIds,
         ).asObject(graph)
     }
     override fun getNodeIdType(): IDataTypeConfiguration<INodeReference> = NodeReferenceDataTypeConfig()

model-server-api/src/commonMain/kotlin/org/modelix/model/server/api/RepositoryConfig.kt

@@ -27,7 +27,10 @@ data class RepositoryConfig(
      * structure can be chosen for the model.
      */
     @Deprecated("Not implemented yet. Tree type is chosen based on the nodeIdType.")
-    val primaryTreeType: TreeType = TreeType.PATRICIA_TRIE,
+    val primaryTreeType: TreeType = when (nodeIdType) {
+        NodeIdType.INT64 -> TreeType.HASH_ARRAY_MAPPED_TRIE
+        NodeIdType.STRING -> TreeType.PATRICIA_TRIE
+    },
 
     /**
      * Is assigned when the repository is created and cannot be changed later.

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

@@ -75,7 +75,7 @@ interface IRepositoriesManager {
     fun getTransactionManager(): ITransactionManager
 
     @RequiresTransaction
-    fun migrateRepository(newConfig: RepositoryConfig, author: String?)
+    fun migrateRepository(newConfig: RepositoryConfig, branch: BranchReference, author: String?)
 
     @RequiresTransaction
     fun getConfig(repositoryId: RepositoryId, branchReference: BranchReference): RepositoryConfig

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

@@ -386,7 +386,7 @@ class ModelReplicationServer(
                 val repositoryId = RepositoryId(repository)
                 val branch = repositoriesManager.getBranches(repositoryId).firstOrNull()
                     ?: throw BranchNotFoundException(RepositoryId.DEFAULT_BRANCH, repository)
-                repositoriesManager.migrateRepository(newConfig, call.getUserName())
+                repositoriesManager.migrateRepository(newConfig, branch, call.getUserName())
                 repositoriesManager.getConfig(repositoryId, branch)
             }
         call.respond(updatedConfig)

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

@@ -177,7 +177,7 @@ class RepositoriesManager(val stores: StoreManager) : IRepositoriesManager {
         )
         stores.genericStore.put(branchListKey(repositoryId, isolated), masterBranch.branchName, false)
 
-        val tree = createEmptyTree(config)
+        val tree = config.createEmptyTree()
 
         val initialVersion = CLVersion.builder()
             .time(Clock.System.now())
@@ -190,13 +190,13 @@ class RepositoriesManager(val stores: StoreManager) : IRepositoriesManager {
         return initialVersion
     }
 
-    private fun createEmptyTree(config: RepositoryConfig): IGenericModelTree<INodeReference> {
+    fun RepositoryConfig.createEmptyTree(): IGenericModelTree<INodeReference> {
         return IGenericModelTree.builder()
-            .treeId(config.modelId)
-            .storeRoleNames(config.legacyNameBasedRoles)
-            .graph(LazyLoadingObjectGraph(getAsyncStore(RepositoryId(config.repositoryId))))
+            .treeId(modelId)
+            .storeRoleNames(legacyNameBasedRoles)
+            .graph(LazyLoadingObjectGraph(getAsyncStore(RepositoryId(repositoryId))))
             .let {
-                when (config.nodeIdType) {
+                when (nodeIdType) {
                     RepositoryConfig.NodeIdType.INT64 -> it.withInt64Ids().build().withIdTranslation()
                     RepositoryConfig.NodeIdType.STRING -> it.withNodeReferenceIds().build()
                 }
@@ -483,10 +483,11 @@ class RepositoriesManager(val stores: StoreManager) : IRepositoriesManager {
     @RequiresTransaction
     override fun migrateRepository(
         newConfig: RepositoryConfig,
+        branch: BranchReference,
         author: String?,
     ) {
         val repositoryId = RepositoryId(newConfig.repositoryId)
-        val currentConfig = getConfig(repositoryId, repositoryId.getBranchReference())
+        val currentConfig = getConfig(repositoryId, branch)
 
         // Validate that the migration is supported
         validateMigration(currentConfig, newConfig)
@@ -512,7 +513,7 @@ class RepositoriesManager(val stores: StoreManager) : IRepositoriesManager {
         for (branch in branches) {
             val oldVersion = getVersion(branch) ?: continue
             val sourceModel = oldVersion.getModelTree().asModelSingleThreaded()
-            val targetTree = createEmptyTree(newConfig).asMutableSingleThreaded()
+            val targetTree = newConfig.createEmptyTree().asMutableSingleThreaded()
             val targetModel = targetTree.asModel()
             ModelSynchronizer(
                 sourceRoot = sourceModel.getRootNode(),

model-server/src/test/kotlin/org/modelix/model/server/RepositoryMigrationTest.kt

@@ -2,6 +2,7 @@ package org.modelix.model.server
 
 import io.ktor.server.testing.testApplication
 import kotlinx.coroutines.test.runTest
+import org.modelix.model.IVersion
 import org.modelix.model.ObjectDeltaFilter
 import org.modelix.model.api.ITree
 import org.modelix.model.api.PNodeReference
@@ -16,6 +17,7 @@ import org.modelix.model.lazy.BranchReference
 import org.modelix.model.lazy.CLVersion
 import org.modelix.model.lazy.RepositoryId
 import org.modelix.model.mutable.asModelSingleThreaded
+import org.modelix.model.mutable.asMutableSingleThreaded
 import org.modelix.model.server.api.RepositoryConfig
 import org.modelix.model.server.api.RepositoryConfig.NodeIdType
 import org.modelix.model.server.handlers.IdsApiImpl
@@ -114,8 +116,9 @@ class RepositoryMigrationTest {
         newConfig: RepositoryConfig,
     ): CLVersion {
         return repositoryManager.getTransactionManager().runWrite {
-            repositoryManager.migrateRepository(newConfig, null)
-            repositoryManager.getVersion(RepositoryId(newConfig.repositoryId).getBranchReference())!!
+            val branch = RepositoryId(newConfig.repositoryId).getBranchReference()
+            repositoryManager.migrateRepository(newConfig, branch, null)
+            repositoryManager.getVersion(branch)!!
         }
     }
 
@@ -207,6 +210,68 @@ class RepositoryMigrationTest {
         assertEquals(expectedImportData, extractNodeData(version2), "Data should be preserved after migration")
     }
 
+    @Test
+    fun `migrate roundtrip with client on repo with multiple branches`() = testApplication {
+        application {
+            try {
+                installDefaultServerPlugins()
+                val repoManager = RepositoriesManager(InMemoryStoreClient())
+                ModelReplicationServer(repoManager).init(this)
+                IdsApiImpl(repoManager).init(this)
+            } catch (ex: Throwable) {
+                ex.printStackTrace()
+            }
+        }
+
+        // Given I have a repository with global storage (legacyGlobalStorage = true)
+        val modelClient = ModelClientV2.builder().url("http://localhost/v2").client(client).build().also { it.init() }
+        val repositoryManager = RepositoriesManager(InMemoryStoreClient())
+        val repositoryId = RepositoryId(config.repositoryId)
+
+        val mainConfig = config.copy(nodeIdType = NodeIdType.STRING, legacyGlobalStorage = true, legacyNameBasedRoles = false)
+
+        val emptyVersion = modelClient.initRepository(mainConfig)
+
+        val versionWithData = emptyVersion.runWrite(IdGenerator.newInstance(456), author = null) {
+            [email protected](it)
+        }!! as CLVersion
+
+        // and I have a branch main with (legacyNameBasedRoles = false)
+        modelClient.push(
+            RepositoryId(mainConfig.repositoryId).getBranchReference("main"),
+            versionWithData,
+            null,
+            force = true,
+        )
+
+        // and I have a branch master with (legacyNameBasedRoles = true)
+        val masterConfig = mainConfig.copy(legacyNameBasedRoles = true)
+        modelClient.push(
+            branch = RepositoryId(mainConfig.repositoryId).getBranchReference("master"),
+            version = repositoryManager.run {
+                val createEmptyTree = masterConfig.createEmptyTree()
+                createEmptyTree.asObject().data.usesRoleIds
+                val value = createEmptyTree.asMutableSingleThreaded().getTransaction().tree
+                value.asObject().data.usesRoleIds
+                IVersion.builder()
+                    .tree(value)
+                    .baseVersion(versionWithData)
+                    .currentTime()
+                    .build()
+            },
+            baseVersion = null,
+            force = true,
+        )
+
+        // When I fetch the config and send it back to the server
+        val configBeforeMigration = modelClient.getRepositoryConfig(repositoryId)
+        assertEquals(configBeforeMigration.legacyNameBasedRoles, false)
+        val configAfterMigration = modelClient.changeRepositoryConfig(configBeforeMigration)
+
+        // Then the configs have stayed the same
+        assertEquals(configBeforeMigration, configAfterMigration)
+    }
+
     @Test
     fun `migrate int64 to string IDs and global to isolated storage simultaneously`() = runTest {
         val repositoryManager = RepositoriesManager(InMemoryStoreClient())

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

@@ -4,6 +4,8 @@ import io.mockk.spyk
 import io.mockk.verify
 import kotlinx.coroutines.test.runTest
 import org.modelix.model.lazy.RepositoryId
+import org.modelix.model.server.api.RepositoryConfig
+import org.modelix.model.server.api.RepositoryConfig.NodeIdType
 import org.modelix.model.server.store.IRepositoryAwareStore
 import org.modelix.model.server.store.InMemoryStoreClient
 import org.modelix.model.server.store.RequiresTransaction
@@ -62,6 +64,70 @@ class RepositoriesManagerTest {
         assertTrue { config.legacyGlobalStorage }
     }
 
+    fun testConfigGetsCreatedAsSpecified(config: RepositoryConfig) = runTest {
+        val repoId = RepositoryId(config.repositoryId)
+        val branch = repoId.getBranchReference()
+        @OptIn(RequiresTransaction::class)
+        repoManager.getTransactionManager().runWrite {
+            [email protected](
+                config,
+                "testUser",
+            )
+        }
+
+        @OptIn(RequiresTransaction::class)
+        val newConfig = repoManager.getTransactionManager().runRead {
+            repoManager.getConfig(repoId, branch)
+        }
+        assertEquals(config, newConfig)
+    }
+
+    @Test
+    fun `createRepository as specified with legacyNameBasedRoles=true`() =
+        testConfigGetsCreatedAsSpecified(
+            RepositoryConfig(
+                repositoryId = "createRepository1",
+                modelId = "",
+                repositoryName = "createRepository1",
+                legacyNameBasedRoles = true,
+            ),
+        )
+
+    @Test
+    fun `createRepository as specified with legacyNameBasedRoles=false`() =
+        testConfigGetsCreatedAsSpecified(
+            RepositoryConfig(
+                repositoryId = "createRepository2",
+                modelId = "",
+                repositoryName = "createRepository2",
+                legacyNameBasedRoles = false,
+            ),
+        )
+
+    @Test
+    fun `createRepository as specified with legacyNameBasedRoles=false for INT64`() =
+        testConfigGetsCreatedAsSpecified(
+            RepositoryConfig(
+                repositoryId = "createRepository3",
+                modelId = "",
+                repositoryName = "createRepository3",
+                nodeIdType = NodeIdType.INT64,
+                legacyNameBasedRoles = false,
+            ),
+        )
+
+    @Test
+    fun `createRepository as specified with legacyNameBasedRoles=true for INT64`() =
+        testConfigGetsCreatedAsSpecified(
+            RepositoryConfig(
+                repositoryId = "createRepository4",
+                modelId = "",
+                repositoryName = "createRepository4",
+                nodeIdType = NodeIdType.INT64,
+                legacyNameBasedRoles = true,
+            ),
+        )
+
     @Test
     fun `repository data is removed when removing repository`() = runTest {
         val repoId = RepositoryId("abc")