Skip to content

Commit 36b1134

Browse files
slissonslisson
committed
fix(model-server): make changes via the v1 API visible to the v2 API
When writing to a branch via the API in KeyValueLikeModelServer (v1) the changes weren't visible to the API in ModelReplicationServer (v2). It only worked the other way around, writing to v2 was also visible to v1.
1 parent 4cd2cfd commit 36b1134

File tree

8 files changed

+188
-40
lines changed

8 files changed

+188
-40
lines changed

‎model-server/src/main/kotlin/org/modelix/model/server/Main.kt‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,17 +141,16 @@ object Main {
141141
storeClient.put(cmdLineArgs.setValues[i], cmdLineArgs.setValues[i + 1])
142142
i += 2
143143
}
144-
val modelServer = KeyValueLikeModelServer(storeClient)
145144
val localModelClient = LocalModelClient(storeClient)
145+
val repositoriesManager = RepositoriesManager(localModelClient)
146+
val modelServer = KeyValueLikeModelServer(repositoriesManager)
146147
val sharedSecretFile = cmdLineArgs.secretFile
147148
if (sharedSecretFile.exists()) {
148149
modelServer.setSharedSecret(
149150
FileUtils.readFileToString(sharedSecretFile, StandardCharsets.UTF_8),
150151
)
151152
}
152-
153153
val jsonModelServer = DeprecatedLightModelServer(localModelClient)
154-
val repositoriesManager = RepositoriesManager(localModelClient)
155154
val repositoryOverview = RepositoryOverview(repositoriesManager)
156155
val historyHandler = HistoryHandler(localModelClient, repositoriesManager)
157156
val contentExplorer = ContentExplorer(localModelClient, repositoriesManager)

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

Lines changed: 46 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import org.modelix.authorization.checkPermission
4242
import org.modelix.authorization.getUserName
4343
import org.modelix.authorization.requiresPermission
4444
import org.modelix.authorization.toKeycloakScope
45+
import org.modelix.model.lazy.BranchReference
4546
import org.modelix.model.persistent.HashUtil
4647
import org.modelix.model.server.store.IStoreClient
4748
import org.modelix.model.server.store.pollEntry
@@ -50,6 +51,7 @@ import org.slf4j.LoggerFactory
5051
import java.io.IOException
5152
import java.util.*
5253
import java.util.regex.Pattern
54+
import kotlin.collections.LinkedHashMap
5355

5456
val PERMISSION_MODEL_SERVER = "model-server".asResource()
5557
val MODEL_SERVER_ENTRY = KeycloakResourceType("model-server-entry", KeycloakScope.READ_WRITE_DELETE)
@@ -62,37 +64,17 @@ private class NotFoundException(description: String?) : RuntimeException(descrip
6264

6365
typealias CallContext = PipelineContext<Unit, ApplicationCall>
6466

65-
class KeyValueLikeModelServer(val storeClient: IStoreClient) {
67+
class KeyValueLikeModelServer(val repositoriesManager: RepositoriesManager) {
6668
companion object {
6769
private val LOG = LoggerFactory.getLogger(KeyValueLikeModelServer::class.java)
6870
val HASH_PATTERN = Pattern.compile("[a-zA-Z0-9\\-_]{5}\\*[a-zA-Z0-9\\-_]{38}")
6971
const val PROTECTED_PREFIX = "$$$"
7072
val HEALTH_KEY = PROTECTED_PREFIX + "health2"
71-
private const val LEGACY_SERVER_ID_KEY = "repositoryId"
72-
private const val SERVER_ID_KEY = "server-id"
73-
74-
private fun randomUUID(): String {
75-
return UUID.randomUUID().toString().replace("[^a-zA-Z0-9]".toRegex(), "")
76-
}
77-
78-
fun initServerId(storeClient: IStoreClient) {
79-
storeClient.runTransaction {
80-
var serverId = storeClient[SERVER_ID_KEY]
81-
if (serverId == null) {
82-
serverId = storeClient[LEGACY_SERVER_ID_KEY] ?: randomUUID()
83-
storeClient.put(SERVER_ID_KEY, serverId)
84-
storeClient.put(LEGACY_SERVER_ID_KEY, serverId)
85-
}
86-
}
87-
}
88-
89-
fun getServerId(storeClient: IStoreClient): String {
90-
return storeClient[SERVER_ID_KEY]!!
91-
}
9273
}
9374

75+
val storeClient: IStoreClient get() = repositoriesManager.client.store
76+
9477
fun init(application: Application) {
95-
initServerId(storeClient)
9678
application.apply {
9779
modelServerModule()
9880
}
@@ -298,7 +280,46 @@ class KeyValueLikeModelServer(val storeClient: IStoreClient) {
298280
throw NotFoundException("Referenced key $key not found")
299281
}
300282
}
301-
storeClient.putAll(newEntries)
283+
284+
// Entries were previously written directly to the store.
285+
// Now we use the RepositoriesManager to merge changes instead of just overwriting a branch.
286+
287+
val hashedObjects = LinkedHashMap<String, String>()
288+
val branchChanges = LinkedHashMap<BranchReference, String>()
289+
val userDefinedEntries = LinkedHashMap<String, String?>()
290+
291+
for ((key, value) in newEntries) {
292+
when {
293+
HashUtil.isSha256(key) -> {
294+
hashedObjects[key] = value ?: throw IllegalArgumentException("No value provided for $key")
295+
}
296+
BranchReference.tryParseBranch(key) != null -> {
297+
branchChanges[BranchReference.tryParseBranch(key)!!] = value ?: throw IllegalArgumentException("Deleting branch $key not allowed")
298+
}
299+
key.startsWith(PROTECTED_PREFIX) -> {
300+
throw NoPermissionException("Access to keys starting with '$PROTECTED_PREFIX' is only permitted to the model server itself.")
301+
}
302+
key.startsWith(RepositoriesManager.KEY_PREFIX) -> {
303+
throw NoPermissionException("Access to keys starting with '${RepositoriesManager.KEY_PREFIX}' is only permitted to the model server itself.")
304+
}
305+
key == RepositoriesManager.LEGACY_SERVER_ID_KEY || key == RepositoriesManager.LEGACY_SERVER_ID_KEY2 -> {
306+
throw NoPermissionException("'$key' is read-only.")
307+
}
308+
else -> {
309+
userDefinedEntries[key] = value
310+
}
311+
}
312+
}
313+
314+
HashUtil.checkObjectHashes(hashedObjects)
315+
316+
repositoriesManager.client.store.runTransaction {
317+
storeClient.putAll(hashedObjects)
318+
storeClient.putAll(userDefinedEntries)
319+
for ((branch, value) in branchChanges) {
320+
repositoriesManager.mergeChanges(branch, value)
321+
}
322+
}
302323
}
303324

304325
private suspend fun CallContext.respondValue(key: String, value: String?) {
@@ -317,7 +338,7 @@ class KeyValueLikeModelServer(val storeClient: IStoreClient) {
317338
if (key.startsWith(RepositoriesManager.KEY_PREFIX)) {
318339
throw NoPermissionException("Access to keys starting with '${RepositoriesManager.KEY_PREFIX}' is only permitted to the model server itself.")
319340
}
320-
if ((key == SERVER_ID_KEY || key == LEGACY_SERVER_ID_KEY) && type.includes(EPermissionType.WRITE)) {
341+
if ((key == RepositoriesManager.LEGACY_SERVER_ID_KEY || key == RepositoriesManager.LEGACY_SERVER_ID_KEY2) && type.includes(EPermissionType.WRITE)) {
321342
throw NoPermissionException("'$key' is read-only.")
322343
}
323344
if (HashUtil.isSha256(key)) {

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ class ModelReplicationServer(val repositoriesManager: RepositoriesManager) {
7575
private val storeClient: IStoreClient get() = modelClient.store
7676

7777
fun init(application: Application) {
78-
KeyValueLikeModelServer.initServerId(storeClient)
7978
application.apply {
8079
routing {
8180
route("v2") {
@@ -90,7 +89,7 @@ class ModelReplicationServer(val repositoriesManager: RepositoriesManager) {
9089
call.respondText(storeClient.generateId("clientId").toString())
9190
}
9291
get("server-id") {
93-
call.respondText(KeyValueLikeModelServer.getServerId(storeClient))
92+
call.respondText(repositoriesManager.getServerId())
9493
}
9594
get("user-id") {
9695
call.respondText(call.getUserName() ?: call.request.origin.remoteHost)

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import org.modelix.model.metameta.MetaModelBranch
3131
import org.modelix.model.server.store.IStoreClient
3232
import org.modelix.model.server.store.LocalModelClient
3333
import org.modelix.model.server.store.pollEntry
34+
import java.util.UUID
3435

3536
class RepositoriesManager(val client: LocalModelClient) {
3637
init {
@@ -43,6 +44,21 @@ class RepositoriesManager(val client: LocalModelClient) {
4344
return client.store.generateId("$KEY_PREFIX:${repositoryId.id}:clientId")
4445
}
4546

47+
fun getServerId(): String {
48+
return store.runTransaction {
49+
var serverId = store[SERVER_ID_KEY]
50+
if (serverId == null) {
51+
serverId = store[LEGACY_SERVER_ID_KEY2]
52+
?: store[LEGACY_SERVER_ID_KEY]
53+
?: UUID.randomUUID().toString().replace("[^a-zA-Z0-9]".toRegex(), "")
54+
store.put(SERVER_ID_KEY, serverId)
55+
store.put(LEGACY_SERVER_ID_KEY, serverId)
56+
store.put(LEGACY_SERVER_ID_KEY2, serverId)
57+
}
58+
serverId
59+
}
60+
}
61+
4662
fun getRepositories(): Set<RepositoryId> {
4763
return store[REPOSITORIES_LIST_KEY]?.lines()?.map { RepositoryId(it) }?.toSet() ?: emptySet()
4864
}
@@ -208,6 +224,9 @@ class RepositoriesManager(val client: LocalModelClient) {
208224
companion object {
209225
const val KEY_PREFIX = ":v2"
210226
private const val REPOSITORIES_LIST_KEY = "$KEY_PREFIX:repositories"
227+
const val LEGACY_SERVER_ID_KEY = "repositoryId"
228+
const val LEGACY_SERVER_ID_KEY2 = "server-id"
229+
const val SERVER_ID_KEY = "$KEY_PREFIX:server-id"
211230
}
212231
}
213232

‎model-server/src/test/java/org/modelix/model/server/RestModelServerTest.java‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,17 @@
2222
import org.json.JSONArray;
2323
import org.junit.Test;
2424
import org.modelix.model.server.handlers.KeyValueLikeModelServer;
25+
import org.modelix.model.server.handlers.RepositoriesManager;
2526
import org.modelix.model.server.store.InMemoryStoreClient;
27+
import org.modelix.model.server.store.LocalModelClient;
2628

2729
public class RestModelServerTest {
2830

2931
@Test
3032
public void testCollectUnexistingKey() {
31-
InMemoryStoreClient storeClient = new InMemoryStoreClient();
32-
KeyValueLikeModelServer rms = new KeyValueLikeModelServer(storeClient);
33+
KeyValueLikeModelServer rms =
34+
new KeyValueLikeModelServer(
35+
new RepositoriesManager(new LocalModelClient(new InMemoryStoreClient())));
3336
JSONArray result = rms.collect("unexistingKey");
3437
assertEquals(1, result.length());
3538
assertEquals(new HashSet<>(Arrays.asList("key")), result.getJSONObject(0).keySet());
@@ -40,7 +43,9 @@ public void testCollectUnexistingKey() {
4043
public void testCollectExistingKeyNotHash() {
4144
InMemoryStoreClient storeClient = new InMemoryStoreClient();
4245
storeClient.put("existingKey", "foo", false);
43-
KeyValueLikeModelServer rms = new KeyValueLikeModelServer(storeClient);
46+
KeyValueLikeModelServer rms =
47+
new KeyValueLikeModelServer(
48+
new RepositoriesManager(new LocalModelClient(storeClient)));
4449
JSONArray result = rms.collect("existingKey");
4550
assertEquals(1, result.length());
4651
assertEquals(
@@ -54,7 +59,9 @@ public void testCollectExistingKeyHash() {
5459
InMemoryStoreClient storeClient = new InMemoryStoreClient();
5560
storeClient.put("existingKey", "hash-*0123456789-0123456789-0123456789-00001", false);
5661
storeClient.put("hash-*0123456789-0123456789-0123456789-00001", "bar", false);
57-
KeyValueLikeModelServer rms = new KeyValueLikeModelServer(storeClient);
62+
KeyValueLikeModelServer rms =
63+
new KeyValueLikeModelServer(
64+
new RepositoriesManager(new LocalModelClient(storeClient)));
5865
JSONArray result = rms.collect("existingKey");
5966
assertEquals(2, result.length());
6067

@@ -86,7 +93,9 @@ public void testCollectExistingKeyHashChained() {
8693
"hash-*0123456789-0123456789-0123456789-00004",
8794
false);
8895
storeClient.put("hash-*0123456789-0123456789-0123456789-00004", "end", false);
89-
KeyValueLikeModelServer rms = new KeyValueLikeModelServer(storeClient);
96+
KeyValueLikeModelServer rms =
97+
new KeyValueLikeModelServer(
98+
new RepositoriesManager(new LocalModelClient(storeClient)));
9099
JSONArray result = rms.collect("root");
91100
assertEquals(5, result.length());
92101

@@ -132,7 +141,9 @@ public void testCollectExistingKeyHashChainedWithRepetitions() {
132141
"hash-*0123456789-0123456789-0123456789-00003",
133142
"hash-*0123456789-0123456789-0123456789-00001",
134143
false);
135-
KeyValueLikeModelServer rms = new KeyValueLikeModelServer(storeClient);
144+
KeyValueLikeModelServer rms =
145+
new KeyValueLikeModelServer(
146+
new RepositoriesManager(new LocalModelClient(storeClient)));
136147
JSONArray result = rms.collect("root");
137148
assertEquals(4, result.length());
138149

‎model-server/src/test/kotlin/org/modelix/model/server/ModelClientTest.kt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ import org.modelix.authorization.installAuthentication
2323
import org.modelix.model.IKeyListener
2424
import org.modelix.model.client.RestWebModelClient
2525
import org.modelix.model.server.handlers.KeyValueLikeModelServer
26+
import org.modelix.model.server.handlers.RepositoriesManager
2627
import org.modelix.model.server.store.InMemoryStoreClient
28+
import org.modelix.model.server.store.LocalModelClient
2729
import java.util.Random
2830
import kotlin.test.Test
2931
import kotlin.test.assertEquals
@@ -35,7 +37,7 @@ class ModelClientTest {
3537
private fun runTest(block: suspend ApplicationTestBuilder.() -> Unit) = testApplication {
3638
application {
3739
installAuthentication(unitTestMode = true)
38-
KeyValueLikeModelServer(InMemoryStoreClient()).init(this)
40+
KeyValueLikeModelServer(RepositoriesManager(LocalModelClient(InMemoryStoreClient()))).init(this)
3941
}
4042
block()
4143
}

‎model-server/src/test/kotlin/org/modelix/model/server/ReplicatedRepositoryTest.kt‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ import org.modelix.model.api.PBranch
3939
import org.modelix.model.api.PNodeAdapter
4040
import org.modelix.model.api.getRootNode
4141
import org.modelix.model.client.IdGenerator
42+
import org.modelix.model.client.ReplicatedRepository
43+
import org.modelix.model.client.RestWebModelClient
4244
import org.modelix.model.client2.ModelClientV2
4345
import org.modelix.model.client2.ReplicatedModel
4446
import org.modelix.model.client2.getReplicatedModel
@@ -327,6 +329,101 @@ class ReplicatedRepositoryTest {
327329
println("all successful")
328330
}
329331

332+
@RepeatedTest(value = 10)
333+
fun clientCompatibility(repetitionInfo: RepetitionInfo) = runTest { scope ->
334+
val url = "http://localhost/v2"
335+
val clients = (1..2).map {
336+
ModelClientV2.builder().url(url).client(client).build().also { it.init() }
337+
}
338+
val v1clients = (1..2).map {
339+
RestWebModelClient("http://localhost/", providedClient = client)
340+
}
341+
342+
val repositoryId = RepositoryId("repo1")
343+
val initialVersion = clients[0].initRepository(repositoryId)
344+
val branchId = repositoryId.getBranchReference("my-branch")
345+
clients[0].push(branchId, initialVersion, initialVersion)
346+
val models = clients.map { client -> client.getReplicatedModel(branchId, scope).also { it.start() } }
347+
val v1models = v1clients.map { ReplicatedRepository(it, repositoryId, branchId.branchName, { "user" }) }
348+
349+
try {
350+
val createdNodes: MutableSet<String> = Collections.synchronizedSet(TreeSet<String>())
351+
352+
coroutineScope {
353+
suspend fun launchWriter(model: ReplicatedModel, seed: Int) {
354+
launch {
355+
val rand = Random(seed)
356+
for (i in 1..10) {
357+
delay(rand.nextLong(50, 100))
358+
model.getBranch().runWriteT { t ->
359+
createdNodes += t.addNewChild(ITree.ROOT_ID, "role", -1, null as IConceptReference?).toString(16)
360+
}
361+
}
362+
}
363+
}
364+
models.forEachIndexed { index, model ->
365+
launchWriter(model, 56456 + index + repetitionInfo.currentRepetition * 100000)
366+
delay(200.milliseconds)
367+
}
368+
suspend fun launchWriterv1(model: ReplicatedRepository, seed: Int) {
369+
launch {
370+
val rand = Random(seed)
371+
for (i in 1..10) {
372+
delay(rand.nextLong(50, 100))
373+
model.branch.runWriteT { t ->
374+
createdNodes += t.addNewChild(ITree.ROOT_ID, "role", -1, null as IConceptReference?).toString(16)
375+
}
376+
}
377+
}
378+
}
379+
v1models.forEachIndexed { index, model ->
380+
launchWriterv1(model, 56456 + index + repetitionInfo.currentRepetition * 100000)
381+
delay(200.milliseconds)
382+
}
383+
}
384+
385+
fun getChildren(model: ReplicatedModel): SortedSet<String> {
386+
return getChildren(model.getBranch())
387+
}
388+
fun getChildren(model: ReplicatedRepository): SortedSet<String> {
389+
return getChildren(model.branch)
390+
}
391+
392+
assertEquals((clients.size + v1clients.size) * 10, createdNodes.size)
393+
394+
println("writing done. waiting for convergence.")
395+
runCatching {
396+
withTimeout(30.seconds) {
397+
models.forEach { model ->
398+
while (getChildren(model) != createdNodes) delay(100.milliseconds)
399+
}
400+
v1models.forEach { model ->
401+
while (getChildren(model) != createdNodes) delay(100.milliseconds)
402+
}
403+
}
404+
}
405+
406+
// models.forEach { it.resetToServerVersion() }
407+
408+
val serverVersion = clients[0].pull(branchId, null)
409+
val childrenOnServer = getChildren(PBranch(serverVersion.getTree(), IdGeneratorDummy()))
410+
411+
assertEquals(createdNodes, childrenOnServer)
412+
413+
for (model in models) {
414+
assertEquals(createdNodes, getChildren(model))
415+
}
416+
for (model in v1models) {
417+
assertEquals(createdNodes, getChildren(model))
418+
}
419+
} finally {
420+
models.forEach { it.dispose() }
421+
v1models.forEach { it.dispose() }
422+
v1clients.forEach { it.dispose() }
423+
}
424+
println("all successful")
425+
}
426+
330427
@Ignore
331428
@Test
332429
fun mergePerformanceTest() {

‎model-server/src/test/resources/functionaltests/storing.feature‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ Feature: Storing routes
6363

6464
Scenario: Get recursively
6565
Given the server has been started with in-memory storage
66-
And I PUT on "/put/hash-*0123456789-0123456789-0123456789-00001" the value "bar"
67-
And I PUT on "/put/existingKey" the value "hash-*0123456789-0123456789-0123456789-00001"
66+
And I PUT on "/put/_N4rL*tula_QIYB-3If6bXDONEO5CnqBPrlURto-_j7k" the value "bar"
67+
And I PUT on "/put/existingKey" the value "_N4rL*tula_QIYB-3If6bXDONEO5CnqBPrlURto-_j7k"
6868
When I visit "/getRecursively/existingKey"
6969
Then I should get an OK response
70-
And the text of the page should be this JSON "[{'value': 'hash-*0123456789-0123456789-0123456789-00001', 'key': 'existingKey'}, {'key': 'hash-*0123456789-0123456789-0123456789-00001', 'value': 'bar'}]"
70+
And the text of the page should be this JSON "[{'value': '_N4rL*tula_QIYB-3If6bXDONEO5CnqBPrlURto-_j7k', 'key': 'existingKey'}, {'key': '_N4rL*tula_QIYB-3If6bXDONEO5CnqBPrlURto-_j7k', 'value': 'bar'}]"

0 commit comments

Comments
 (0)