Skip to content

Commit b6609d4

Browse files
mhuster23mhuster23
authored
Merge pull request #818 from modelix/fix/delete-repository-data
MODELIX-856 Delete its data when a repository is deleted
2 parents f5af3b5 + 924485c commit b6609d4

File tree

10 files changed

+360
-13
lines changed

10 files changed

+360
-13
lines changed

‎gradle/libs.versions.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ junit = { group = "junit", name = "junit", version = "4.13.2" }
108108
xmlunit-core = { group = "org.xmlunit", name = "xmlunit-core", version.ref="xmlunit"}
109109
xmlunit-matchers = { group = "org.xmlunit", name = "xmlunit-matchers", version.ref="xmlunit"}
110110
jsoup = { group = "org.jsoup", name = "jsoup", version = "1.17.2" }
111+
mockk = { group = "io.mockk", name = "mockk", version = "1.13.11"}
111112

112113
apache-cxf-sse = { group = "org.apache.cxf", name = "cxf-rt-rs-sse", version.ref = "apacheCxf" }
113114
apache-cxf-client = { group = "org.apache.cxf", name = "cxf-rt-rs-client", version.ref = "apacheCxf" }

‎model-server-test/build.gradle.kts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ dependencies {
99
}
1010

1111
tasks.test {
12+
dependsOn(":model-server:compileTestKotlin")
13+
useJUnitPlatform()
1214
doFirst {
1315
val db = dockerCompose.servicesInfos.getValue("db")
1416
systemProperty("jdbc.url", "jdbc:postgresql://${db.host}:${db.port}/")

‎model-server-test/src/test/kotlin/IgnitePostgresRepositoryRemovalTest.kt‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/*
2+
* Copyright (c) 2024.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
import org.junit.jupiter.api.AfterAll
17+
import org.junit.jupiter.api.Test
18+
import org.junit.jupiter.api.assertDoesNotThrow
19+
import org.modelix.model.lazy.RepositoryId
20+
import org.modelix.model.server.store.IgniteStoreClient
21+
import org.modelix.model.server.store.ObjectInRepository
22+
import java.sql.Connection
23+
import java.sql.DriverManager
24+
import kotlin.test.AfterTest
25+
import kotlin.test.assertEquals
26+
import kotlin.test.assertFalse
27+
import kotlin.test.assertTrue
28+
29+
class IgnitePostgresRepositoryRemovalTest {
30+
private val store = IgniteStoreClient()
31+
32+
private val toDelete = RepositoryId("repository-removal-toDelete")
33+
private val existing = RepositoryId("repository-removal-existing")
34+
35+
@AfterTest
36+
fun cleanup() {
37+
dbConnection.prepareStatement("DELETE FROM modelix.model WHERE repository IN ( ? , ? )").use {
38+
it.setString(1, toDelete.id)
39+
it.setString(2, existing.id)
40+
}
41+
store.putAll(store.getAll().keys.associateWith { null })
42+
store.close()
43+
}
44+
45+
@Test
46+
fun `data is removed from the cache`() {
47+
val entries = mapOf(
48+
ObjectInRepository(toDelete.id, "key0") to "value0",
49+
ObjectInRepository(toDelete.id, "key1") to "value1",
50+
)
51+
store.putAll(entries)
52+
53+
store.removeRepositoryObjects(toDelete)
54+
55+
assertTrue { store.getAll(entries.keys).isEmpty() }
56+
}
57+
58+
@Test
59+
fun `data is removed from the database`() {
60+
dbConnection.prepareStatement("INSERT INTO modelix.model (repository, key, value) VALUES (?, 'myKey', 'myValue')")
61+
.use {
62+
it.setString(1, toDelete.id)
63+
it.execute()
64+
check(it.updateCount == 1)
65+
}
66+
67+
store.removeRepositoryObjects(toDelete)
68+
69+
dbConnection.prepareStatement("SELECT * FROM modelix.model WHERE repository = ?").use {
70+
it.setString(1, toDelete.id)
71+
val result = it.executeQuery()
72+
assertFalse("Database contained leftover repository data.") { result.isBeforeFirst }
73+
}
74+
}
75+
76+
@Test
77+
fun `removal does not affect other repository data in the database`() {
78+
dbConnection.prepareStatement("INSERT INTO modelix.model (repository, key, value) VALUES (?, 'myKey', 'myValue')")
79+
.use {
80+
it.setString(1, existing.id)
81+
it.execute()
82+
check(it.updateCount == 1)
83+
it.setString(1, toDelete.id)
84+
it.execute()
85+
check(it.updateCount == 1)
86+
}
87+
88+
store.removeRepositoryObjects(toDelete)
89+
90+
dbConnection.prepareStatement("SELECT * FROM modelix.model WHERE repository = ?").use {
91+
it.setString(1, existing.id)
92+
val result = it.executeQuery()
93+
assertTrue("Other repository data was removed from the database.") { result.isBeforeFirst }
94+
}
95+
}
96+
97+
@Test
98+
fun `removal does not affect other repository data in the cache`() {
99+
val existingEntries = mapOf(
100+
ObjectInRepository(existing.id, "key0") to "value0",
101+
ObjectInRepository(existing.id, "key1") to "value1",
102+
)
103+
store.putAll(existingEntries)
104+
105+
val toDeleteEntries = mapOf(
106+
ObjectInRepository(toDelete.id, "key0") to "value0",
107+
ObjectInRepository(toDelete.id, "key1") to "value1",
108+
)
109+
store.putAll(toDeleteEntries)
110+
store.removeRepositoryObjects(toDelete)
111+
112+
assertEquals(existingEntries, store.getAll())
113+
}
114+
115+
@Test
116+
fun `removing a non-existing repository does not throw`() {
117+
assertDoesNotThrow {
118+
store.removeRepositoryObjects(RepositoryId("invalid"))
119+
}
120+
}
121+
122+
companion object {
123+
val dbConnection: Connection = DriverManager.getConnection(System.getProperty("jdbc.url"), "modelix", "modelix")
124+
125+
@JvmStatic
126+
@AfterAll
127+
fun closeDbConnection() {
128+
dbConnection.close()
129+
}
130+
}
131+
}

‎model-server/build.gradle.kts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ dependencies {
7070
testImplementation(libs.ktor.server.test.host)
7171
testImplementation(libs.kotlin.coroutines.test)
7272
testImplementation(libs.jsoup)
73+
testImplementation(libs.mockk)
7374
testImplementation(kotlin("test"))
7475
testImplementation(project(":modelql-untyped"))
7576

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,8 @@ class RepositoriesManager(val client: LocalModelClient) : IRepositoriesManager {
226226
}
227227

228228
override suspend fun removeRepository(repository: RepositoryId): Boolean {
229+
val genericStore = store.getGenericStore()
230+
229231
return store.runTransactionSuspendable {
230232
if (!repositoryExists(repository)) {
231233
return@runTransactionSuspendable false
@@ -234,12 +236,12 @@ class RepositoriesManager(val client: LocalModelClient) : IRepositoriesManager {
234236
for (branchName in getBranchNames(repository)) {
235237
putVersionHash(repository.getBranchReference(branchName), null)
236238
}
237-
store.getGenericStore().put(branchListKey(repository), null)
239+
genericStore.put(branchListKey(repository), null)
238240
val isolated = checkNotNull(isIsolated(repository)) { "Repository not found: $repository" }
239241
val existingRepositories = getRepositories(isolated)
240242
val remainingRepositories = existingRepositories - repository
241243
store.put(repositoriesListKey(isolated), remainingRepositories.joinToString("\n") { it.id })
242-
244+
genericStore.removeRepositoryObjects(repository)
243245
true
244246
}
245247
}

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,27 @@
1717
package org.modelix.model.server.store
1818

1919
import org.modelix.model.IGenericKeyListener
20+
import org.modelix.model.lazy.RepositoryId
2021

21-
typealias IsolatingStore = IGenericStoreClient<ObjectInRepository>
22+
/**
23+
* A store that saves data on a per-repository basis.
24+
* The primary key is of type [ObjectInRepository].
25+
*/
26+
interface IsolatingStore : IGenericStoreClient<ObjectInRepository> {
27+
/**
28+
* Default implementation for removing repository objects.
29+
* May be overridden by more efficient, store-specific implementations.
30+
*
31+
* Callers need to ensure that the repository is not usable anymore before calling this method.
32+
*/
33+
fun removeRepositoryObjects(repositoryId: RepositoryId) {
34+
val keysToDelete = getAll().asSequence()
35+
.map { it.key }
36+
.filter { it.getRepositoryId() == repositoryId.id }
37+
.toSet()
38+
putAll(keysToDelete.associateWith { null })
39+
}
40+
}
2241

2342
interface IGenericStoreClient<KeyT> : AutoCloseable {
2443
operator fun get(key: KeyT): String? = getAll(listOf(key)).first()

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

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,29 @@ import mu.KotlinLogging
1818
import org.apache.ignite.Ignite
1919
import org.apache.ignite.IgniteCache
2020
import org.apache.ignite.Ignition
21+
import org.apache.ignite.cache.query.ScanQuery
22+
import org.apache.ignite.lang.IgniteBiPredicate
23+
import org.apache.ignite.lang.IgniteClosure
2124
import org.modelix.kotlin.utils.ContextValue
2225
import org.modelix.model.IGenericKeyListener
26+
import org.modelix.model.lazy.RepositoryId
2327
import org.modelix.model.persistent.HashUtil
2428
import org.modelix.model.server.SqlUtils
2529
import java.io.File
2630
import java.io.FileReader
2731
import java.io.IOException
32+
import java.sql.SQLException
2833
import java.util.*
34+
import javax.cache.Cache
2935
import javax.sql.DataSource
3036

3137
private val LOG = KotlinLogging.logger { }
3238

33-
class IgniteStoreClient(jdbcConfFile: File? = null, inmemory: Boolean = false) : IsolatingStore, AutoCloseable {
39+
/**
40+
* Store client implementation with an ignite cache.
41+
* If [inmemory] is true, the data is not persisted in a database.
42+
*/
43+
class IgniteStoreClient(jdbcConfFile: File? = null, private val inmemory: Boolean = false) : IsolatingStore, AutoCloseable {
3444

3545
companion object {
3646
private const val ENTRY_CHANGED_TOPIC = "entryChanged"
@@ -43,6 +53,14 @@ class IgniteStoreClient(jdbcConfFile: File? = null, inmemory: Boolean = false) :
4353
ignite.message().send(ENTRY_CHANGED_TOPIC, it)
4454
}
4555

56+
private val igniteConfigName: String = if (inmemory) "ignite-inmemory.xml" else "ignite.xml"
57+
private val dataSource: DataSource by lazy {
58+
Ignition.loadSpringBean(
59+
IgniteStoreClient::class.java.getResource(igniteConfigName),
60+
"dataSource",
61+
)
62+
}
63+
4664
/**
4765
* Instantiate an IgniteStoreClient
4866
*
@@ -74,8 +92,7 @@ class IgniteStoreClient(jdbcConfFile: File? = null, inmemory: Boolean = false) :
7492
)
7593
}
7694
}
77-
val igniteConfigName = if (inmemory) "ignite-inmemory.xml" else "ignite.xml"
78-
if (!inmemory) updateDatabaseSchema(igniteConfigName)
95+
if (!inmemory) updateDatabaseSchema()
7996
ignite = Ignition.start(javaClass.getResource(igniteConfigName))
8097
cache = ignite.getOrCreateCache("model")
8198

@@ -87,11 +104,7 @@ class IgniteStoreClient(jdbcConfFile: File? = null, inmemory: Boolean = false) :
87104
}
88105
}
89106

90-
private fun updateDatabaseSchema(igniteConfigName: String) {
91-
val dataSource: DataSource = Ignition.loadSpringBean<DataSource>(
92-
IgniteStoreClient::class.java.getResource(igniteConfigName),
93-
"dataSource",
94-
)
107+
private fun updateDatabaseSchema() {
95108
SqlUtils(dataSource.connection).ensureSchemaInitialization()
96109
}
97110

@@ -103,6 +116,44 @@ class IgniteStoreClient(jdbcConfFile: File? = null, inmemory: Boolean = false) :
103116
return cache.associate { it.key to it.value }
104117
}
105118

119+
override fun removeRepositoryObjects(repositoryId: RepositoryId) {
120+
if (!inmemory) {
121+
// Not all entries are in the cache. We delete them directly instead of loading them into the cache first.
122+
// This should be safe as the repository has already been removed from the list of available ones.
123+
removeRepositoryObjectsFromDatabase(repositoryId)
124+
}
125+
126+
val filter = IgniteBiPredicate<ObjectInRepository, String?> { key, _ ->
127+
key.getRepositoryId() == repositoryId.id
128+
}
129+
val transformer = IgniteClosure<Cache.Entry<ObjectInRepository, String?>, ObjectInRepository?> { entry ->
130+
entry.key
131+
}
132+
val query = ScanQuery(filter)
133+
134+
// sorting is necessary to avoid deadlocks, see documentation of IgniteCache::removeAllAsync
135+
val toDelete = cache.query(query, transformer).all.asSequence().filterNotNull().toSortedSet()
136+
LOG.info { "Deleting cache entries asynchronously. [numberOfEntries=${toDelete.size}]" }
137+
cache.removeAllAsync(toDelete).listen { LOG.info { "Cache entries deleted." } }
138+
}
139+
140+
private fun removeRepositoryObjectsFromDatabase(repositoryId: RepositoryId) {
141+
require(!inmemory) { "Cannot remove from database in in-memory mode." }
142+
LOG.info { "Removing repository objects from database." }
143+
144+
dataSource.connection.use { connection ->
145+
connection.prepareStatement("DELETE from model WHERE repository = ?").use { stmt ->
146+
stmt.setString(1, repositoryId.id)
147+
try {
148+
val deletedRows = stmt.executeUpdate()
149+
LOG.info { "Deleted rows from database. [deletedRows=$deletedRows]" }
150+
} catch (e: SQLException) {
151+
LOG.error { e }
152+
}
153+
}
154+
}
155+
}
156+
106157
override fun putAll(entries: Map<ObjectInRepository, String?>, silent: Boolean) {
107158
// Sorting is important to avoid deadlocks (lock ordering).
108159
// The documentation of IgniteCache.putAll also states that this a requirement.
@@ -174,7 +225,8 @@ class PendingChangeMessages(private val notifier: (ObjectInRepository) -> Unit)
174225
}
175226

176227
fun entryChanged(key: ObjectInRepository) {
177-
val messages = checkNotNull(pendingChangeMessages.getValueOrNull()) { "Only allowed inside PendingChangeMessages.runAndFlush" }
228+
val messages =
229+
checkNotNull(pendingChangeMessages.getValueOrNull()) { "Only allowed inside PendingChangeMessages.runAndFlush" }
178230
messages.add(key)
179231
}
180232
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,14 @@ class InMemoryStoreClient : IsolatingStore {
103103
try {
104104
transactionValues = HashMap()
105105
val result = body()
106-
values.putAll(transactionValues!!)
106+
val tValues = requireNotNull(transactionValues) { "Passed lambda set 'transactionValues' to null unexpectedly." }
107+
108+
val puts = tValues.filterValues { it != null }
109+
val deletes = tValues.filterValues { it == null }.keys
110+
values.putAll(puts)
111+
for (keyToDelete in deletes) {
112+
values.remove(keyToDelete)
113+
}
107114
result
108115
} finally {
109116
transactionValues = null

0 commit comments

Comments
 (0)