From b6229088bbd8051b93607a125aa1488e786ccdf8 Mon Sep 17 00:00:00 2001 From: Anshul Singh Date: Mon, 14 Apr 2025 15:02:06 +0530 Subject: [PATCH 1/2] [persistence] Disallow multiple namespace configuration rows with NULL namespace --- .../NamespaceConfigurationManagerImpl.java | 24 ++++++++++++------- ..._configuration_entity_uniqueness_check.sql | 12 ++++++++++ 2 files changed, 27 insertions(+), 9 deletions(-) create mode 100644 thirdeye-persistence/src/main/resources/db/migration/mysql/V1_345_0__add_namespace_configuration_entity_uniqueness_check.sql diff --git a/thirdeye-persistence/src/main/java/ai/startree/thirdeye/datalayer/bao/NamespaceConfigurationManagerImpl.java b/thirdeye-persistence/src/main/java/ai/startree/thirdeye/datalayer/bao/NamespaceConfigurationManagerImpl.java index 777d5ab9d2..876537358e 100644 --- a/thirdeye-persistence/src/main/java/ai/startree/thirdeye/datalayer/bao/NamespaceConfigurationManagerImpl.java +++ b/thirdeye-persistence/src/main/java/ai/startree/thirdeye/datalayer/bao/NamespaceConfigurationManagerImpl.java @@ -38,6 +38,8 @@ @Singleton public class NamespaceConfigurationManagerImpl implements NamespaceConfigurationManager { + private static final String NULL_NAMESPACE_KEY = "__NULL__"; + private final NamespaceConfigurationDao dao; private final @Nullable TimeConfiguration timeConfiguration; private final NamespaceConfigurationDTO defaultNamespaceConfiguration; @@ -58,7 +60,7 @@ public NamespaceConfigurationManagerImpl(final NamespaceConfigurationDao dao, public @NonNull NamespaceConfigurationDTO getNamespaceConfiguration(final String namespace) { final NamespaceConfigurationDTO existingNamespaceConfig = fetchExistingNamespaceConfiguration( - namespace); + nonNullNamespace(namespace)); // namespace config exists, update if components are empty if (existingNamespaceConfig != null) { @@ -73,7 +75,7 @@ public NamespaceConfigurationManagerImpl(final NamespaceConfigurationDao dao, return existingNamespaceConfig; } - return createNewNamespaceConfiguration(namespace); + return createNewNamespaceConfiguration(nonNullNamespace(namespace)); } /* @@ -109,7 +111,7 @@ private boolean updateDefaults(final NamespaceConfigurationDTO existingNamespace final NamespaceConfigurationDTO updatedNamespaceConfiguration) { final String namespace = updatedNamespaceConfiguration.namespace(); final NamespaceConfigurationDTO existingNamespaceConfig = fetchExistingNamespaceConfiguration( - namespace); + nonNullNamespace(namespace)); checkState(existingNamespaceConfig != null, "Trying to update non-existent namespace configuration for namespace %s", namespace); @@ -124,7 +126,7 @@ private boolean updateDefaults(final NamespaceConfigurationDTO existingNamespace public @NonNull NamespaceConfigurationDTO resetNamespaceConfiguration(final String namespace) { final NamespaceConfigurationDTO existingNamespaceConfig = fetchExistingNamespaceConfiguration( - namespace); + nonNullNamespace(namespace)); // namespace config exists, update values to default if (existingNamespaceConfig != null) { @@ -136,12 +138,12 @@ private boolean updateDefaults(final NamespaceConfigurationDTO existingNamespace return existingNamespaceConfig; } - return createNewNamespaceConfiguration(namespace); + return createNewNamespaceConfiguration(nonNullNamespace(namespace)); } - private NamespaceConfigurationDTO fetchExistingNamespaceConfiguration(String namespace) { + private NamespaceConfigurationDTO fetchExistingNamespaceConfiguration(final String namespace) { final DaoFilter daoFilter = new DaoFilter().setPredicate(Predicate.EQ( - "namespace", namespace)); + "namespace", nonNullNamespace(namespace))); final List results = filter(daoFilter); if (results != null && !results.isEmpty()) { if (results.size() != 1) { @@ -158,7 +160,7 @@ private NamespaceConfigurationDTO fetchExistingNamespaceConfiguration(String nam private @NonNull NamespaceConfigurationDTO createNewNamespaceConfiguration( final String namespace) { final NamespaceConfigurationDTO namespaceConfigurationDTO = defaultNamespaceConfiguration( - namespace); + nonNullNamespace(namespace)); final Long namespaceConfigurationId = save(namespaceConfigurationDTO); checkState(namespaceConfigurationId != null, @@ -173,7 +175,7 @@ private NamespaceConfigurationDTO defaultNamespaceConfiguration(final String nam .setTimeConfiguration(defaultTimeConfiguration()) .setTemplateConfiguration(defaultTemplateConfiguration()) .setNamespaceQuotasConfiguration(defaultNamespaceQuotasConfiguration()) - .setAuth(new AuthorizationConfigurationDTO().setNamespace(namespace)); + .setAuth(new AuthorizationConfigurationDTO().setNamespace(nonNullNamespace(namespace))); return namespaceConfigurationDTO; } @@ -242,4 +244,8 @@ private TaskQuotasConfigurationDTO defaultTaskQuotasConfiguration() { .map(NamespaceQuotasConfigurationDTO::getTaskQuotasConfiguration) .orElse(new TaskQuotasConfigurationDTO()); } + + private static @NonNull String nonNullNamespace(@Nullable String namespace) { + return namespace == null ? NULL_NAMESPACE_KEY : namespace; + } } diff --git a/thirdeye-persistence/src/main/resources/db/migration/mysql/V1_345_0__add_namespace_configuration_entity_uniqueness_check.sql b/thirdeye-persistence/src/main/resources/db/migration/mysql/V1_345_0__add_namespace_configuration_entity_uniqueness_check.sql new file mode 100644 index 0000000000..7b734172a6 --- /dev/null +++ b/thirdeye-persistence/src/main/resources/db/migration/mysql/V1_345_0__add_namespace_configuration_entity_uniqueness_check.sql @@ -0,0 +1,12 @@ +-- Before enforcing unique constraint +UPDATE namespace_configuration_entity +SET namespace = '__NULL__' +WHERE namespace IS NULL; + +-- Alter the column to disallow NULLs +ALTER TABLE namespace_configuration_entity +MODIFY namespace VARCHAR(200) NOT NULL; + +-- Now enforce real uniqueness +ALTER TABLE namespace_configuration_entity +ADD UNIQUE (namespace); \ No newline at end of file From 5ff0ab79b7ceff1573bae3fcefd3fae9bfcefc15 Mon Sep 17 00:00:00 2001 From: Anshul Singh Date: Mon, 14 Apr 2025 15:26:34 +0530 Subject: [PATCH 2/2] fix tests --- .../thirdeye/datalayer/dao/TestNamespaceConfigurationDao.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/thirdeye-persistence/src/test/java/ai/startree/thirdeye/datalayer/dao/TestNamespaceConfigurationDao.java b/thirdeye-persistence/src/test/java/ai/startree/thirdeye/datalayer/dao/TestNamespaceConfigurationDao.java index 753e468798..dd5c001099 100644 --- a/thirdeye-persistence/src/test/java/ai/startree/thirdeye/datalayer/dao/TestNamespaceConfigurationDao.java +++ b/thirdeye-persistence/src/test/java/ai/startree/thirdeye/datalayer/dao/TestNamespaceConfigurationDao.java @@ -150,7 +150,7 @@ public void countPredicateTest() { @Test public void deleteTest() { - final NamespaceConfigurationDTO dto = buildNamespaceConfiguration(null); + final NamespaceConfigurationDTO dto = buildNamespaceConfiguration("__NULL__"); Long id = dao.put(dto); assertThat(id).isGreaterThan(0L); dto.setId(id);