diff --git a/src/main/java/com/dnastack/ga4gh/dataconnect/DataModelSupplier.java b/src/main/java/com/dnastack/ga4gh/dataconnect/DataModelSupplier.java index d136807c..e5a2f7b4 100644 --- a/src/main/java/com/dnastack/ga4gh/dataconnect/DataModelSupplier.java +++ b/src/main/java/com/dnastack/ga4gh/dataconnect/DataModelSupplier.java @@ -1,13 +1,15 @@ package com.dnastack.ga4gh.dataconnect; import com.dnastack.ga4gh.dataconnect.model.DataModel; +import com.dnastack.tenancy.context.TenantId; public interface DataModelSupplier { /** * Provides the data model of a given table. * + * @param tenantId the tenant the table belongs to * @param tableName the name of the target table * @return The supplier returns null if the table is not found. */ - DataModel supply(String tableName); + DataModel supply(TenantId tenantId, String tableName); } diff --git a/src/main/java/com/dnastack/ga4gh/dataconnect/adapter/trino/TrinoDataConnectAdapter.java b/src/main/java/com/dnastack/ga4gh/dataconnect/adapter/trino/TrinoDataConnectAdapter.java index f957bb24..8d31e985 100644 --- a/src/main/java/com/dnastack/ga4gh/dataconnect/adapter/trino/TrinoDataConnectAdapter.java +++ b/src/main/java/com/dnastack/ga4gh/dataconnect/adapter/trino/TrinoDataConnectAdapter.java @@ -1110,7 +1110,7 @@ private URI getDataModelId(String tableName, HttpServletRequest request) { private DataModel getDataModelFromSupplier(String tableName) { for (DataModelSupplier dataModelSupplier : dataModelSuppliers) { log.debug("Trying to get data model for {} from {}", tableName, dataModelSupplier.getClass()); - final var dataModel = dataModelSupplier.supply(tableName); + final var dataModel = dataModelSupplier.supply(tenantContextAccessor.getTenantId(), tableName); if (dataModel != null) { log.debug("Using data model from {}", dataModelSupplier.getClass()); return dataModel; diff --git a/src/main/java/com/dnastack/ga4gh/dataconnect/client/collectionservice/CollectionServiceClientConfiguration.java b/src/main/java/com/dnastack/ga4gh/dataconnect/client/collectionservice/CollectionServiceClientConfiguration.java index 58601b10..17ad1078 100644 --- a/src/main/java/com/dnastack/ga4gh/dataconnect/client/collectionservice/CollectionServiceClientConfiguration.java +++ b/src/main/java/com/dnastack/ga4gh/dataconnect/client/collectionservice/CollectionServiceClientConfiguration.java @@ -36,11 +36,34 @@ public CollectionServiceClient collectionServiceClient( .target(CollectionServiceClient.class, configuration.getBaseUri()); } + @Bean("tenantCollectionServiceClient") + @ConditionalOnProperty(name = {"app.collection-service.enabled"}, havingValue = "true") + public TenantCollectionServiceClient tenantCollectionServiceClient( + OAuthClientFactoryConfiguration oAuthClientFactoryConfiguration, + CollectionServiceConfiguration configuration, + ObservationRegistry observationRegistry + ) { + log.info("Initializing the tenant-scoped collection-service API client..."); + OkHttpClient httpClient = OkHttpClients.buildOkHttpClient( + "collection-service", observationRegistry, TenantCollectionServiceClient.class); + feign.okhttp.OkHttpClient feignClient = new feign.okhttp.OkHttpClient(httpClient); + return FeignClients.newBuilder( + oAuthClientFactoryConfiguration.getDefaultConfig().withOverrides(configuration.getOauthClient()), + TokenTenantPolicy.tenantFromRequestPath(), + feignClient, + feignClient) + .target(TenantCollectionServiceClient.class, configuration.getBaseUri()); + } + @Bean - @ConditionalOnBean(CollectionServiceClient.class) - public DataModelSupplier collectionServiceDataModelSupplier(CollectionServiceClient client, CollectionServiceConfiguration configuration) { + @ConditionalOnBean({CollectionServiceClient.class, TenantCollectionServiceClient.class}) + public DataModelSupplier collectionServiceDataModelSupplier( + CollectionServiceClient client, + TenantCollectionServiceClient tenantClient, + CollectionServiceConfiguration configuration + ) { log.info("Initializing a collection-service data model supplier for catalog {}", configuration.getCollectionsCatalogName()); - return new CollectionServiceDataModelSupplier(client, configuration.getCollectionsCatalogName()); + return new CollectionServiceDataModelSupplier(client, tenantClient, configuration.getCollectionsCatalogName()); } } diff --git a/src/main/java/com/dnastack/ga4gh/dataconnect/client/collectionservice/CollectionServiceDataModelSupplier.java b/src/main/java/com/dnastack/ga4gh/dataconnect/client/collectionservice/CollectionServiceDataModelSupplier.java index c083ffb3..96ca305e 100644 --- a/src/main/java/com/dnastack/ga4gh/dataconnect/client/collectionservice/CollectionServiceDataModelSupplier.java +++ b/src/main/java/com/dnastack/ga4gh/dataconnect/client/collectionservice/CollectionServiceDataModelSupplier.java @@ -2,6 +2,8 @@ import com.dnastack.ga4gh.dataconnect.DataModelSupplier; import com.dnastack.ga4gh.dataconnect.model.DataModel; +import com.dnastack.oauth.client.TokenExchangeException; +import com.dnastack.tenancy.context.TenantId; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; @@ -14,6 +16,7 @@ public class CollectionServiceDataModelSupplier implements DataModelSupplier { private final CollectionServiceClient client; + private final TenantCollectionServiceClient tenantClient; private final ObjectMapper objectMapper = new ObjectMapper() .registerModule(new JavaTimeModule()) .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES); @@ -23,13 +26,15 @@ public class CollectionServiceDataModelSupplier implements DataModelSupplier { public CollectionServiceDataModelSupplier( @NonNull CollectionServiceClient client, + @NonNull TenantCollectionServiceClient tenantClient, @NonNull String collectionsCatalogName) { this.client = client; + this.tenantClient = tenantClient; this.collectionsCatalogName = collectionsCatalogName; } @Override - public DataModel supply(String fullyQualifiedTableName) { + public DataModel supply(TenantId tenantId, String fullyQualifiedTableName) { String[] tableNameParts = fullyQualifiedTableName.split("\\.", 3); String catalogName = tableNameParts[0]; // must match this.collectionsCatalogName String schemaName = tableNameParts[1]; // the collection's dbSchemaName @@ -42,10 +47,13 @@ public DataModel supply(String fullyQualifiedTableName) { final CollectionItem collectionItem; try { - collectionItem = client.getItem(schemaName, tableName); + // The management tenant keeps the path without a tenant prefix, as before. + collectionItem = tenantId.isManagement() + ? client.getItem(schemaName, tableName) + : tenantClient.getItem(tenantId.asString(), schemaName, tableName); log.debug("{} CollectionItem is {}", fullyQualifiedTableName, collectionItem); return collectionItem.getJsonSchema(); - } catch (FeignException e) { + } catch (FeignException | TokenExchangeException e) { log.warn("Failed to fetch collection item for {} -- returning null data model", fullyQualifiedTableName, e); return null; } diff --git a/src/main/java/com/dnastack/ga4gh/dataconnect/client/collectionservice/TenantCollectionServiceClient.java b/src/main/java/com/dnastack/ga4gh/dataconnect/client/collectionservice/TenantCollectionServiceClient.java new file mode 100644 index 00000000..09d75ec5 --- /dev/null +++ b/src/main/java/com/dnastack/ga4gh/dataconnect/client/collectionservice/TenantCollectionServiceClient.java @@ -0,0 +1,15 @@ +package com.dnastack.ga4gh.dataconnect.client.collectionservice; + +import feign.Param; +import feign.RequestLine; + +/** + * The collection-service calls that act as one tenant, named by the {@code /tenants/{tenantId}} prefix of the path. + */ +public interface TenantCollectionServiceClient { + @RequestLine("GET /tenants/{tenantId}/collection/{collectionIdOrSlugNameOrDbSchemaName}/item/{itemIdOrDisplayName}") + CollectionItem getItem( + @Param("tenantId") String tenantId, + @Param("collectionIdOrSlugNameOrDbSchemaName") String collectionIdOrSlugNameOrDbSchemaName, + @Param("itemIdOrDisplayName") String itemIdOrDisplayName); +} diff --git a/src/main/java/com/dnastack/ga4gh/dataconnect/client/indexingservice/IndexingServiceDataModelSupplier.java b/src/main/java/com/dnastack/ga4gh/dataconnect/client/indexingservice/IndexingServiceDataModelSupplier.java index 101a9378..284d0f79 100644 --- a/src/main/java/com/dnastack/ga4gh/dataconnect/client/indexingservice/IndexingServiceDataModelSupplier.java +++ b/src/main/java/com/dnastack/ga4gh/dataconnect/client/indexingservice/IndexingServiceDataModelSupplier.java @@ -2,6 +2,7 @@ import com.dnastack.ga4gh.dataconnect.DataModelSupplier; import com.dnastack.ga4gh.dataconnect.model.DataModel; +import com.dnastack.tenancy.context.TenantId; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; @@ -25,7 +26,7 @@ public IndexingServiceDataModelSupplier(IndexingServiceClient client, String pub } @Override - public DataModel supply(String tableName) { + public DataModel supply(TenantId tenantId, String tableName) { String jsonSchemaAsString; if (publisherDataCatalogName != null && tableName.startsWith(publisherDataCatalogName + ".") ) { tableName = tableName.substring(publisherDataCatalogName.length() + 1); diff --git a/src/main/java/com/dnastack/ga4gh/dataconnect/client/tablesregistry/TablesRegistryDataModelSupplier.java b/src/main/java/com/dnastack/ga4gh/dataconnect/client/tablesregistry/TablesRegistryDataModelSupplier.java index 56532b2d..615272d8 100644 --- a/src/main/java/com/dnastack/ga4gh/dataconnect/client/tablesregistry/TablesRegistryDataModelSupplier.java +++ b/src/main/java/com/dnastack/ga4gh/dataconnect/client/tablesregistry/TablesRegistryDataModelSupplier.java @@ -3,6 +3,7 @@ import com.dnastack.ga4gh.dataconnect.DataModelSupplier; import com.dnastack.ga4gh.dataconnect.client.tablesregistry.model.ListTableRegistryEntry; import com.dnastack.ga4gh.dataconnect.model.DataModel; +import com.dnastack.tenancy.context.TenantId; @Deprecated(since = "2021-06-01 per #177369206") public class TablesRegistryDataModelSupplier implements DataModelSupplier { @@ -15,7 +16,7 @@ public TablesRegistryDataModelSupplier(TablesRegistryClient tablesRegistryClient } @Override - public DataModel supply(String tableName) { + public DataModel supply(TenantId tenantId, String tableName) { ListTableRegistryEntry registryEntry = tablesRegistryClient.getTableRegistryEntry(oAuthClientConfig.getClientId(), tableName); if (registryEntry == null || registryEntry.getTableCollections().isEmpty()) { diff --git a/src/test/java/com/dnastack/ga4gh/dataconnect/adapter/trino/TrinoDataConnectAdapterTest.java b/src/test/java/com/dnastack/ga4gh/dataconnect/adapter/trino/TrinoDataConnectAdapterTest.java index c970af0f..b81b019f 100644 --- a/src/test/java/com/dnastack/ga4gh/dataconnect/adapter/trino/TrinoDataConnectAdapterTest.java +++ b/src/test/java/com/dnastack/ga4gh/dataconnect/adapter/trino/TrinoDataConnectAdapterTest.java @@ -162,7 +162,7 @@ public void setUp() { null, Map.of(), null); - DataModelSupplier dataModelSupplier = tableName -> fakeDataModel; + DataModelSupplier dataModelSupplier = (tenantId, tableName) -> fakeDataModel; mockApplicationConfig = mock(ApplicationConfig.class); when(mockApplicationConfig.getHiddenCatalogs()).thenReturn(Collections.emptySet()); // Default: no hidden catalogs diff --git a/src/test/java/com/dnastack/ga4gh/dataconnect/client/collectionservice/CollectionServiceDataModelSupplierTest.java b/src/test/java/com/dnastack/ga4gh/dataconnect/client/collectionservice/CollectionServiceDataModelSupplierTest.java new file mode 100644 index 00000000..7228d96f --- /dev/null +++ b/src/test/java/com/dnastack/ga4gh/dataconnect/client/collectionservice/CollectionServiceDataModelSupplierTest.java @@ -0,0 +1,69 @@ +package com.dnastack.ga4gh.dataconnect.client.collectionservice; + +import com.dnastack.ga4gh.dataconnect.model.DataModel; +import com.dnastack.oauth.client.TokenExchangeException; +import com.dnastack.tenancy.context.TenantId; +import com.dnastack.tenancy.context.TestTenantIds; +import org.junit.Test; + +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class CollectionServiceDataModelSupplierTest { + + private static final String CATALOG = "collections"; + + private final CollectionServiceClient managementClient = mock(CollectionServiceClient.class); + private final TenantCollectionServiceClient tenantClient = mock(TenantCollectionServiceClient.class); + private final CollectionServiceDataModelSupplier supplier = + new CollectionServiceDataModelSupplier(managementClient, tenantClient, CATALOG); + + @Test + public void supply_should_lookUpTheDataModelInTheRequestTenant_when_theTenantIsNotManagement() { + TenantId tenantId = TestTenantIds.of(UUID.randomUUID()); + DataModel tenantDataModel = new DataModel(); + when(tenantClient.getItem(tenantId.asString(), "my_collection", "my_table")) + .thenReturn(CollectionItem.builder().jsonSchema(tenantDataModel).build()); + + DataModel supplied = supplier.supply(tenantId, CATALOG + ".my_collection.my_table"); + + assertThat(supplied).isSameAs(tenantDataModel); + verify(managementClient, never()).getItem(any(), any()); + } + + @Test + public void supply_should_lookUpTheDataModelOnTheManagementPath_when_theTenantIsManagement() { + DataModel managementDataModel = new DataModel(); + when(managementClient.getItem("my_collection", "my_table")) + .thenReturn(CollectionItem.builder().jsonSchema(managementDataModel).build()); + + DataModel supplied = supplier.supply(TenantId.MANAGEMENT, CATALOG + ".my_collection.my_table"); + + assertThat(supplied).isSameAs(managementDataModel); + verify(tenantClient, never()).getItem(any(), any(), any()); + } + + @Test + public void supply_should_returnNull_when_theTableIsNotInTheCollectionsCatalog() { + TenantId tenantId = TestTenantIds.of(UUID.randomUUID()); + + assertThat(supplier.supply(tenantId, "other_catalog.my_collection.my_table")).isNull(); + verify(tenantClient, never()).getItem(any(), any(), any()); + verify(managementClient, never()).getItem(any(), any()); + } + + @Test + public void supply_should_returnNull_when_noTokenCanBeObtainedForTheTenant() { + TenantId tenantId = TestTenantIds.of(UUID.randomUUID()); + when(tenantClient.getItem(tenantId.asString(), "my_collection", "my_table")) + .thenThrow(new TokenExchangeException("wallet refused the token request")); + + assertThat(supplier.supply(tenantId, CATALOG + ".my_collection.my_table")).isNull(); + } +}