From cacba6b36a2b9173072b263d37041b3c5fe5d96d Mon Sep 17 00:00:00 2001 From: paduin Date: Fri, 17 Nov 2023 10:51:11 +0100 Subject: [PATCH 1/2] removed cache and using filter --- CHANGELOG.md | 1 + .../impl/StaticDatabaseMappingService.java | 82 +++---------------- .../StaticDatabaseMappingServiceTest.java | 31 ++++++- 3 files changed, 44 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cd6d578d..7c014371b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ ### Fixed * Added lombok * Fixed test cases +* Fixed issue where the primare metastore was not applying the allow filter to validate database clashes from other metastores. ## [3.9.5] - TBD ### Changed diff --git a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/service/impl/StaticDatabaseMappingService.java b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/service/impl/StaticDatabaseMappingService.java index 75b1eae3a..7fe9c0fbe 100644 --- a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/service/impl/StaticDatabaseMappingService.java +++ b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/service/impl/StaticDatabaseMappingService.java @@ -27,10 +27,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; import javax.validation.constraints.NotNull; @@ -42,13 +39,8 @@ import lombok.extern.log4j.Log4j2; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList.Builder; -import com.google.common.collect.Lists; -import com.google.common.collect.Sets; import com.hotels.bdp.waggledance.api.WaggleDanceException; import com.hotels.bdp.waggledance.api.model.AbstractMetaStore; @@ -69,9 +61,7 @@ @Log4j2 public class StaticDatabaseMappingService implements MappingEventListener { - private static final String PRIMARY_KEY = ""; private final MetaStoreMappingFactory metaStoreMappingFactory; - private final LoadingCache> primaryDatabasesCache; private final Map mappingsByMetaStoreName; private final Map mappingsByDatabaseName; private final Map> databaseMappingToDatabaseList; @@ -85,22 +75,6 @@ public StaticDatabaseMappingService( QueryMapping queryMapping) { this.metaStoreMappingFactory = metaStoreMappingFactory; this.queryMapping = queryMapping; - primaryDatabasesCache = CacheBuilder - .newBuilder() - .expireAfterAccess(1, TimeUnit.MINUTES) - .maximumSize(1) - .build(new CacheLoader>() { - - @Override - public List load(String key) throws Exception { - if (primaryDatabaseMapping != null) { - return primaryDatabaseMapping.getClient().get_all_databases(); - } else { - return Lists.newArrayList(); - } - } - }); - mappingsByMetaStoreName = Collections.synchronizedMap(new LinkedHashMap<>()); mappingsByDatabaseName = Collections.synchronizedMap(new LinkedHashMap<>()); databaseMappingToDatabaseList = new ConcurrentHashMap<>(); @@ -131,12 +105,9 @@ private void add(AbstractMetaStore metaStore) { validateMappableDatabases(mappableDatabases, metaStore); if (metaStore.getFederationType() == PRIMARY) { - validatePrimaryMetastoreDatabases(mappableDatabases); primaryDatabaseMapping = databaseMapping; - primaryDatabasesCache.invalidateAll(); - } else { - validateFederatedMetastoreDatabases(mappableDatabases, metaStoreMapping); } + validateMetastoreDatabases(mappableDatabases, metaStoreMapping); mappingsByMetaStoreName.put(metaStoreMapping.getMetastoreMappingName(), databaseMapping); addDatabaseMappings(mappableDatabases, databaseMapping); @@ -155,42 +126,16 @@ private void validateMappableDatabases(List mappableDatabases, AbstractM } } - private void validatePrimaryMetastoreDatabases(List databases) { + private void validateMetastoreDatabases(List databases, MetaStoreMapping metaStoreMapping) { for (String database : databases) { - if (mappingsByDatabaseName.containsKey(database)) { + if (mappingsByDatabaseName.containsKey(database.toLowerCase(Locale.ROOT))) { throw new WaggleDanceException("Database clash, found '" + database - + "' in primary that was already mapped to a federated metastore '" - + mappingsByDatabaseName.get(database).getMetastoreMappingName() - + "', please remove the database from the federated metastore list it can't be" - + " accessed via Waggle Dance"); - } - } - } - - private void validateFederatedMetastoreDatabases(List mappableDatabases, MetaStoreMapping metaStoreMapping) { - try { - Set allPrimaryDatabases = Sets.newHashSet(primaryDatabasesCache.get(PRIMARY_KEY)); - for (String database : mappableDatabases) { - if (allPrimaryDatabases.contains(database.toLowerCase(Locale.ROOT))) { - throw new WaggleDanceException("Database clash, found '" - + database - + "' to be mapped for the federated metastore '" - + metaStoreMapping.getMetastoreMappingName() - + "' already present in the primary database, please remove the database from the list it can't be" - + " accessed via Waggle Dance"); - } - if (mappingsByDatabaseName.containsKey(database.toLowerCase(Locale.ROOT))) { - throw new WaggleDanceException("Database clash, found '" - + database - + "' to be mapped for the federated metastore '" - + metaStoreMapping.getMetastoreMappingName() - + "' already present in another federated database, please remove the database from the list it can't" - + " be accessed via Waggle Dance"); - } + + "' to be mapped for the federated metastore '" + + metaStoreMapping.getMetastoreMappingName() + + "' already present in another federated metastore, please remove the database from the list it can't" + + " be accessed via Waggle Dance"); } - } catch (ExecutionException e) { - throw new WaggleDanceException("Can't validate database clashes", e.getCause()); } } @@ -228,7 +173,6 @@ private DatabaseMapping createDatabaseMapping(MetaStoreMapping metaStoreMapping) private void remove(AbstractMetaStore metaStore) { if (metaStore.getFederationType() == PRIMARY) { primaryDatabaseMapping = null; - primaryDatabasesCache.invalidateAll(); } DatabaseMapping removed = mappingsByMetaStoreName.remove(metaStore.getName()); @@ -306,8 +250,8 @@ public DatabaseMapping databaseMapping(@NotNull String databaseName) throws NoSu } @Override - public void checkTableAllowed(String databaseName, String tableName, - DatabaseMapping mapping) throws NoSuchObjectException { + public void checkTableAllowed(String databaseName, String tableName, DatabaseMapping mapping) + throws NoSuchObjectException { databaseName = GrammarUtils.removeCatName(databaseName); if (!isTableAllowed(databaseName, tableName)) { throw new NoSuchObjectException(String.format("%s.%s table not found in any mappings", databaseName, tableName)); @@ -319,7 +263,7 @@ public List filterTables(String databaseName, List tableNames, D List allowedTables = new ArrayList<>(); databaseName = GrammarUtils.removeCatName(databaseName); String db = databaseName.toLowerCase(Locale.ROOT); - for (String table: tableNames) + for (String table : tableNames) if (isTableAllowed(db, table)) { allowedTables.add(table); } @@ -368,8 +312,8 @@ public PanopticOperationHandler getPanopticOperationHandler() { @Override public List getTableMeta(String db_patterns, String tbl_patterns, List tbl_types) { - BiFunction filter = (tableMeta, mapping) -> - databaseAndTableAllowed(tableMeta.getDbName(), tableMeta.getTableName(), mapping); + BiFunction filter = (tableMeta, mapping) -> databaseAndTableAllowed( + tableMeta.getDbName(), tableMeta.getTableName(), mapping); Map mappingsForPattern = new LinkedHashMap<>(); for (DatabaseMapping mapping : getAvailableDatabaseMappings()) { @@ -384,7 +328,7 @@ public List getAllDatabases(String pattern) { .containsKey(database); BiFunction filter1 = (database, mapping) -> filter.apply(database, mapping) - && databaseMappingToDatabaseList.get(mapping.getMetastoreMappingName()).contains(database); + && databaseMappingToDatabaseList.get(mapping.getMetastoreMappingName()).contains(database); Map mappingsForPattern = new LinkedHashMap<>(); for (DatabaseMapping mapping : getAllDatabaseMappings()) { diff --git a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/service/impl/StaticDatabaseMappingServiceTest.java b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/service/impl/StaticDatabaseMappingServiceTest.java index f1f7b9603..9a9c9d773 100644 --- a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/service/impl/StaticDatabaseMappingServiceTest.java +++ b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/service/impl/StaticDatabaseMappingServiceTest.java @@ -145,7 +145,7 @@ private FederatedMetaStore newFederatedInstanceWithClient( when(federatedDatabaseClient.get_all_databases()).thenReturn(allLowerCased); return newMetastore; } - + @Test public void databaseMappingPrimary() throws NoSuchObjectException { DatabaseMapping databaseMapping = service.databaseMapping(PRIMARY_DB); @@ -206,6 +206,35 @@ public void validatePrimaryMetaStoreClashThrowsException() throws TException { service = new StaticDatabaseMappingService(metaStoreMappingFactory, Arrays.asList(federatedMetastore, primaryMetastore), queryMapping); } + + + @Test + public void validateNoPrimaryMetaStoreClashWhenMapped() throws TException { + federatedMetastore = newFederatedInstanceWithClient(FEDERATED_NAME, URI, Lists.newArrayList("db"), true); + + primaryMetastore.setMappedDatabases(Lists.newArrayList("mapped_db")); + metaStoreMappingPrimary = mockNewMapping(true, primaryMetastore); + when(metaStoreMappingPrimary.getClient()).thenReturn(primaryDatabaseClient); + when(primaryDatabaseClient.get_all_databases()).thenReturn(Lists.newArrayList("db", "mapped_db")); + when(metaStoreMappingFactory.newInstance(primaryMetastore)).thenReturn(metaStoreMappingPrimary); + + service = new StaticDatabaseMappingService(metaStoreMappingFactory, + Arrays.asList(primaryMetastore, federatedMetastore), queryMapping); + } + + @Test + public void validateNoPrimaryMetaStoreClashWhenMappedPrimarySpecifiedLast() throws TException { + federatedMetastore = newFederatedInstanceWithClient(FEDERATED_NAME, URI, Lists.newArrayList("db"), true); + + primaryMetastore.setMappedDatabases(Lists.newArrayList("mapped_db")); + metaStoreMappingPrimary = mockNewMapping(true, primaryMetastore); + when(metaStoreMappingPrimary.getClient()).thenReturn(primaryDatabaseClient); + when(primaryDatabaseClient.get_all_databases()).thenReturn(Lists.newArrayList("db", "mapped_db")); + when(metaStoreMappingFactory.newInstance(primaryMetastore)).thenReturn(metaStoreMappingPrimary); + + service = new StaticDatabaseMappingService(metaStoreMappingFactory, + Arrays.asList(federatedMetastore, primaryMetastore), queryMapping); + } @Test(expected = WaggleDanceException.class) public void onRegisterPrimaryThrowsExceptionDueToExistingPrimary() { From 411ebe1c4e03f8917b62124baffc02cd43ca3a1f Mon Sep 17 00:00:00 2001 From: Abhimanyu Gupta Date: Thu, 23 Nov 2023 14:39:07 +0000 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c014371b..a83c80c96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ ### Fixed * Added lombok * Fixed test cases -* Fixed issue where the primare metastore was not applying the allow filter to validate database clashes from other metastores. +* Fixed issue where the primary metastore was not applying the allow filter to validate database clashes from other metastores. ## [3.9.5] - TBD ### Changed