From 426cc3cb43b05c49422259e8436777864bd1dd99 Mon Sep 17 00:00:00 2001 From: f43nd1r Date: Sat, 16 Mar 2024 13:36:36 +0100 Subject: [PATCH] fix custom columns interfering between apps. Closes #499 --- .../acra/persistence/app/AppRepository.kt | 55 ++++++++++++------- .../security/BasicSecurityConfiguration.kt | 4 +- .../acra/common/DatabaseTestConfiguration.kt | 12 ++-- .../acra/persistence/app/AppRepositoryTest.kt | 33 ++++++++++- .../report/ReportRepositoryTest.kt | 5 +- 5 files changed, 76 insertions(+), 33 deletions(-) diff --git a/acrarium/src/main/kotlin/com/faendir/acra/persistence/app/AppRepository.kt b/acrarium/src/main/kotlin/com/faendir/acra/persistence/app/AppRepository.kt index 2c58ab27..7913fcff 100644 --- a/acrarium/src/main/kotlin/com/faendir/acra/persistence/app/AppRepository.kt +++ b/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) { - 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().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() } } diff --git a/acrarium/src/main/kotlin/com/faendir/acra/security/BasicSecurityConfiguration.kt b/acrarium/src/main/kotlin/com/faendir/acra/security/BasicSecurityConfiguration.kt index 39deb2a6..9aebb4b1 100644 --- a/acrarium/src/main/kotlin/com/faendir/acra/security/BasicSecurityConfiguration.kt +++ b/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 } } } } \ No newline at end of file diff --git a/acrarium/src/test/kotlin/com/faendir/acra/common/DatabaseTestConfiguration.kt b/acrarium/src/test/kotlin/com/faendir/acra/common/DatabaseTestConfiguration.kt index 06c01fac..cb99b1d8 100644 --- a/acrarium/src/test/kotlin/com/faendir/acra/common/DatabaseTestConfiguration.kt +++ b/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 } diff --git a/acrarium/src/test/kotlin/com/faendir/acra/persistence/app/AppRepositoryTest.kt b/acrarium/src/test/kotlin/com/faendir/acra/persistence/app/AppRepositoryTest.kt index 52f4b815..6f6fa4f7 100644 --- a/acrarium/src/test/kotlin/com/faendir/acra/persistence/app/AppRepositoryTest.kt +++ b/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 diff --git a/acrarium/src/test/kotlin/com/faendir/acra/persistence/report/ReportRepositoryTest.kt b/acrarium/src/test/kotlin/com/faendir/acra/persistence/report/ReportRepositoryTest.kt index fa2c8eaa..94ab69c9 100644 --- a/acrarium/src/test/kotlin/com/faendir/acra/persistence/report/ReportRepositoryTest.kt +++ b/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"))