fix custom columns interfering between apps. Closes #499

acrarium/src/main/kotlin/com/faendir/acra/persistence/app/AppRepository.kt

@@ -1,5 +1,5 @@
 /*
- * (C) Copyright 2022-2023 Lukas Morawietz (https://github.com/F43nd1r)
+ * (C) Copyright 2022-2024 Lukas Morawietz (https://github.com/F43nd1r)
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -74,6 +74,8 @@ class AppRepository(private val jooq: DSLContext, private val userRepository: Us
         val reporterUsername = jooq.select(APP.REPORTER_USERNAME).from(APP).where(APP.ID.eq(id)).fetchValue() ?: return
         // app is cascade deleted
         userRepository.delete(reporterUsername)
+        // might need to remove custom columns
+        ensureCustomColumns()
     }
 
     @PreAuthorize("isUser()")
@@ -89,19 +91,7 @@ class AppRepository(private val jooq: DSLContext, private val userRepository: Us
     @Transactional
     @PreAuthorize("hasAdminPermission(#id)")
     fun setCustomColumns(id: AppId, customColumns: List<CustomColumn>) {
-        val tableMeta = jooq.meta().getTables(REPORT.name).first()
-        val fields = tableMeta.fields()
-        val indexes = tableMeta.indexes
-
         jooq.deleteFrom(APP_REPORT_COLUMNS).where(APP_REPORT_COLUMNS.APP_ID.eq(id), APP_REPORT_COLUMNS.PATH.notIn(customColumns.map { it.path })).execute()
-        indexes.filter { index -> index.isCustomColumnIndex && customColumns.none { it.indexName == index.name } }.forEach {
-            jooq.dropIndex(it).on(REPORT).execute()
-        }
-        val columnsToDelete = fields.filter { field -> field.isCustomColumn && customColumns.none { it.fieldName == field.name } }
-        if (columnsToDelete.isNotEmpty()) {
-            jooq.alterTable(REPORT).dropColumns(columnsToDelete).execute()
-        }
-
         for (customColumn in customColumns) {
             jooq.insertInto(APP_REPORT_COLUMNS)
                 .set(APP_REPORT_COLUMNS.APP_ID, id)
@@ -110,14 +100,37 @@ class AppRepository(private val jooq: DSLContext, private val userRepository: Us
                 .onDuplicateKeyUpdate()
                 .set(APP_REPORT_COLUMNS.NAME, customColumn.name)
                 .execute()
-            if (fields.none { it.name == customColumn.fieldName }) {
-                jooq.execute(
-                    "ALTER TABLE `${REPORT.name}` ADD COLUMN `${customColumn.fieldName}` VARCHAR(255) GENERATED ALWAYS AS (JSON_UNQUOTE(JSON_VALUE(`${REPORT.name}`.`content`, '$.${customColumn.path}'))) STORED",
-                )
-            }
-            if (indexes.none { it.name == customColumn.indexName }) {
-                jooq.createIndex(customColumn.indexName).on(REPORT.name, customColumn.fieldName).execute()
-            }
+        }
+
+        ensureCustomColumns()
+    }
+
+    private fun ensureCustomColumns() {
+        val tableMeta = jooq.meta().getTables(REPORT.name).first()
+        val fields = tableMeta.fields()
+        val indexes = tableMeta.indexes
+
+        val wantedCustomColumns = jooq.select(APP_REPORT_COLUMNS.NAME, APP_REPORT_COLUMNS.PATH).from(APP_REPORT_COLUMNS).fetchListInto<CustomColumn>().distinctBy { it.path }
+        // drop unwanted indices
+        val wantedCustomColumnIndexNames = wantedCustomColumns.mapTo(mutableSetOf()) { it.indexName }
+        for (index in indexes.filter { it.isCustomColumnIndex && it.name !in wantedCustomColumnIndexNames }) {
+            jooq.dropIndex(index).on(REPORT).execute()
+        }
+        // drop unwanted columns
+        val wantedCustomColumnFieldNames = wantedCustomColumns.mapTo(mutableSetOf()) { it.fieldName }
+        val columnsToDelete = fields.filter { it.isCustomColumn && it.name !in wantedCustomColumnFieldNames }
+        if (columnsToDelete.isNotEmpty()) {
+            jooq.alterTable(REPORT).dropColumns(columnsToDelete).execute()
+        }
+        // create missing columns
+        for (customColumn in wantedCustomColumns.filter { customColumn -> fields.none { it.name == customColumn.fieldName } }) {
+            jooq.execute(
+                "ALTER TABLE `${REPORT.name}` ADD COLUMN `${customColumn.fieldName}` VARCHAR(255) GENERATED ALWAYS AS (JSON_UNQUOTE(JSON_VALUE(`${REPORT.name}`.`content`, '$.${customColumn.path}'))) STORED",
+            )
+        }
+        // create missing indices
+        for (customColumn in wantedCustomColumns.filter { customColumn -> indexes.none { it.name == customColumn.indexName } }) {
+            jooq.createIndex(customColumn.indexName).on(REPORT.name, customColumn.fieldName).execute()
         }
     }
 

acrarium/src/main/kotlin/com/faendir/acra/security/BasicSecurityConfiguration.kt

@@ -1,5 +1,5 @@
 /*
- * (C) Copyright 2022 Lukas Morawietz (https://github.com/F43nd1r)
+ * (C) Copyright 2022-2024 Lukas Morawietz (https://github.com/F43nd1r)
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -40,5 +40,5 @@ class BasicSecurityConfiguration {
 
     @Bean
     fun grantedAuthoritiesMapper() =
-        GrantedAuthoritiesMapper { authorities -> Role.values().filter { role -> authorities.any { it?.authority == role.authority } } }
+        GrantedAuthoritiesMapper { authorities -> Role.entries.filter { role -> authorities.any { it?.authority == role.authority } } }
 }

acrarium/src/test/kotlin/com/faendir/acra/common/DatabaseTestConfiguration.kt

@@ -1,5 +1,5 @@
 /*
- * (C) Copyright 2023 Lukas Morawietz (https://github.com/F43nd1r)
+ * (C) Copyright 2023-2024 Lukas Morawietz (https://github.com/F43nd1r)
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +16,9 @@
 package com.faendir.acra.common
 
 import com.faendir.acra.jooq.generated.Acrarium
+import com.faendir.acra.jooq.generated.tables.references.REPORT
+import com.faendir.acra.persistence.app.isCustomColumn
+import com.faendir.acra.persistence.app.isCustomColumnIndex
 import com.faendir.acra.persistence.jooq.JooqConfigurationCustomizer
 import org.springframework.beans.factory.NoSuchBeanDefinitionException
 import org.springframework.boot.jdbc.DataSourceBuilder
@@ -40,9 +43,10 @@ class DatabaseCleanupTestExecutionListener : TestExecutionListener {
     override fun afterTestMethod(testContext: org.springframework.test.context.TestContext) {
         try {
             val jooq = testContext.applicationContext.getBean(org.jooq.DSLContext::class.java)
-            Acrarium.ACRARIUM.tables.forEach {
-                jooq.deleteFrom(it).execute()
-            }
+            Acrarium.ACRARIUM.tables.forEach { jooq.deleteFrom(it).execute() }
+            val reportMeta = jooq.meta().getTables(REPORT.name).first()
+            jooq.alterTable(REPORT).dropColumns(reportMeta.fields().filter { it.isCustomColumn }).execute()
+            reportMeta.indexes.filter { it.isCustomColumnIndex }.forEach { jooq.dropIndex(it).execute() }
         } catch (e: NoSuchBeanDefinitionException) {
             // not a database test
         }

acrarium/src/test/kotlin/com/faendir/acra/persistence/app/AppRepositoryTest.kt

@@ -1,5 +1,5 @@
 /*
- * (C) Copyright 2022-2023 Lukas Morawietz (https://github.com/F43nd1r)
+ * (C) Copyright 2022-2024 Lukas Morawietz (https://github.com/F43nd1r)
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -136,7 +136,9 @@ class AppRepositoryTest(
     }
 
     @Nested
-    inner class Delete {
+    inner class Delete(
+        @Autowired private val jooq: DSLContext,
+    ) {
         @Test
         fun `should delete app and reporter`() {
             val reporter = testDataBuilder.createUser()
@@ -147,6 +149,18 @@ class AppRepositoryTest(
             expectThat(appRepository.find(appId)).isNull()
             expectThat(userRepository.find(reporter)).isNull()
         }
+
+        @Test
+        fun `should remove custom columns`() {
+            val app = testDataBuilder.createApp()
+            appRepository.setCustomColumns(app, listOf(CustomColumn("c1", "p1")))
+
+            appRepository.delete(app)
+
+            val meta = jooq.meta().getTables(REPORT.name).first()
+            expectThat(meta.fields().toList().map { it.name }).doesNotContain("custom_p1")
+            expectThat(meta.indexes.toList().map { it.name }).doesNotContain("idx_custom_p1")
+        }
     }
 
     @Nested
@@ -301,7 +315,6 @@ class AppRepositoryTest(
 
         @Test
         fun `should not touch any other columns or indexes`() {
-
             val app1 = testDataBuilder.createApp()
 
             appRepository.setCustomColumns(app1, listOf())
@@ -310,6 +323,20 @@ class AppRepositoryTest(
             expectThat(meta.fields().map { it.name }).isEqualTo(REPORT.fields().map { it.name })
             expectThat(meta.indexes.map { it.name }).isEqualTo(REPORT.indexes.map { it.name })
         }
+
+        @Test
+        fun `should not touch any custom columns of other apps`() {
+            val app1 = testDataBuilder.createApp()
+            val app2 = testDataBuilder.createApp()
+
+            appRepository.setCustomColumns(app2, listOf(CustomColumn("c2", "p2")))
+
+            appRepository.setCustomColumns(app1, listOf())
+
+            val meta = jooq.meta().getTables(REPORT.name).first()
+            expectThat(meta.fields().map { it.name }).isEqualTo(REPORT.fields().map { it.name } + "custom_p2")
+            expectThat(meta.indexes.map { it.name }).isEqualTo(REPORT.indexes.map { it.name } + "idx_custom_p2")
+        }
     }
 
     @Nested

acrarium/src/test/kotlin/com/faendir/acra/persistence/report/ReportRepositoryTest.kt

@@ -1,5 +1,5 @@
 /*
- * (C) Copyright 2022-2023 Lukas Morawietz (https://github.com/F43nd1r)
+ * (C) Copyright 2022-2024 Lukas Morawietz (https://github.com/F43nd1r)
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -573,8 +573,7 @@ internal class ReportRepositoryTest(
                 CustomColumn(name = "nested custom field column name", "NESTED_CUSTOM_FIELD"), CustomColumn(name = "custom field column name", "CUSTOM_FIELD")
             )
             appRepository.setCustomColumns(appId, customColumns)
-            val version = testDataBuilder.createVersion(appId)
-            testDataBuilder.createReport(appId, version = version, content = "{\"CUSTOM_FIELD\": \"customField\"}")
+            testDataBuilder.createReport(appId, content = "{\"CUSTOM_FIELD\": \"customField\"}")
             val provider = reportRepository.getProvider(appId, customColumns)
 
             expectThat(provider.fetch(emptySet(), emptyList(), 0, 1).toList().first().customColumns).isEqualTo(listOf(null, "customField"))