From d153e494c6129831bc739f666442edcf313902d1 Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Mon, 17 Feb 2025 16:29:18 -0500 Subject: [PATCH 01/21] Upgrade Iceberg to 1.8.1 --- gradle/libs.versions.toml | 2 +- .../polaris/service/it/env/IcebergHelper.java | 5 +- .../PolarisApplicationIntegrationTest.java | 8 +-- .../PolarisRestCatalogIntegrationTest.java | 57 ++++++++++--------- ...PolarisRestCatalogViewIntegrationBase.java | 3 +- ...IcebergCatalogHandlerWrapperAuthzTest.java | 2 +- .../quarkus/catalog/IcebergCatalogTest.java | 50 +++++++++------- .../service/quarkus/task/TaskTestUtils.java | 3 +- .../iceberg/IcebergCatalogAdapter.java | 4 ++ 9 files changed, 73 insertions(+), 61 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 1f868f7029..02d6de1902 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -19,7 +19,7 @@ [versions] hadoop = "3.4.1" -iceberg = "1.7.1" +iceberg = "1.8.1" quarkus = "3.19.4" immutables = "2.10.1" picocli = "4.7.6" diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/IcebergHelper.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/IcebergHelper.java index 4d0c987b2b..7478b1ea2c 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/env/IcebergHelper.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/IcebergHelper.java @@ -34,6 +34,7 @@ public static RESTCatalog restCatalog( PolarisApiEndpoints endpoints, PrincipalWithCredentials credentials, String catalog, + String warehouse, Map extraProperties) { String authToken = client.obtainToken(credentials); SessionCatalog.SessionContext context = SessionCatalog.SessionContext.createEmpty(); @@ -53,11 +54,11 @@ public static RESTCatalog restCatalog( .put( org.apache.iceberg.CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO") - .put("warehouse", catalog) + .put("warehouse", warehouse) .put("header." + endpoints.realmHeaderName(), endpoints.realmId()) .putAll(extraProperties); - restCatalog.initialize("polaris", propertiesBuilder.buildKeepingLast()); + restCatalog.initialize(catalog, propertiesBuilder.buildKeepingLast()); return restCatalog; } } diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java index 2437601e06..99e736f798 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java @@ -454,8 +454,7 @@ public void testIcebergRegisterTableInExternalCatalog() throws IOException { .assignUUID() .addPartitionSpec(PartitionSpec.unpartitioned()) .addSortOrder(SortOrder.unsorted()) - .addSchema( - new Schema(Types.NestedField.of(1, false, "col1", Types.StringType.get())), 1) + .addSchema(new Schema(Types.NestedField.of(1, false, "col1", Types.StringType.get()))) .build(); TableMetadataParser.write(tableMetadata, fileIo.newOutputFile(metadataLocation)); @@ -498,7 +497,7 @@ public void testIcebergUpdateTableInExternalCatalog() throws IOException { .assignUUID() .addPartitionSpec(PartitionSpec.unpartitioned()) .addSortOrder(SortOrder.unsorted()) - .addSchema(new Schema(col1), 1) + .addSchema(new Schema(col1)) .build(); TableMetadataParser.write(tableMetadata, fileIo.newOutputFile(metadataLocation)); @@ -546,8 +545,7 @@ public void testIcebergDropTableInExternalCatalog() throws IOException { .assignUUID() .addPartitionSpec(PartitionSpec.unpartitioned()) .addSortOrder(SortOrder.unsorted()) - .addSchema( - new Schema(Types.NestedField.of(1, false, "col1", Types.StringType.get())), 1) + .addSchema(new Schema(Types.NestedField.of(1, false, "col1", Types.StringType.get()))) .build(); TableMetadataParser.write(tableMetadata, fileIo.newOutputFile(metadataLocation)); diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index a832553cf8..e7dddfda3c 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -118,16 +118,16 @@ public class PolarisRestCatalogIntegrationTest extends CatalogTests private static URI externalCatalogBase; protected static final String VIEW_QUERY = "select * from ns1.layer1_table"; - private static String principalRoleName; private static ClientCredentials adminCredentials; - private static PrincipalWithCredentials principalCredentials; private static PolarisApiEndpoints endpoints; private static PolarisClient client; private static ManagementApi managementApi; - private static CatalogApi catalogApi; + private PrincipalWithCredentials principalCredentials; + private CatalogApi catalogApi; private RESTCatalog restCatalog; private String currentCatalogName; + private TestInfo testInfo; private final String catalogBaseLocation = s3BucketBase + "/" + System.getenv("USER") + "/path/to/data"; @@ -159,10 +159,6 @@ static void setup( endpoints = apiEndpoints; client = polarisClient(endpoints); managementApi = client.managementApi(credentials); - String principalName = client.newEntityName("snowman-rest"); - principalRoleName = client.newEntityName("rest-admin"); - principalCredentials = managementApi.createPrincipalWithRole(principalName, principalRoleName); - catalogApi = client.catalogApi(principalCredentials); URI testRootUri = IntegrationTestsHelper.getTemporaryDirectory(tempDir); s3BucketBase = testRootUri.resolve("my-bucket"); externalCatalogBase = testRootUri.resolve("external-catalog"); @@ -175,10 +171,10 @@ static void close() throws Exception { @BeforeEach public void before(TestInfo testInfo) { + this.testInfo = testInfo; String principalName = "snowman-rest-" + UUID.randomUUID(); - principalRoleName = "rest-admin-" + UUID.randomUUID(); - PrincipalWithCredentials principalCredentials = - managementApi.createPrincipalWithRole(principalName, principalRoleName); + String principalRoleName = "rest-admin-" + UUID.randomUUID(); + principalCredentials = managementApi.createPrincipalWithRole(principalName, principalRoleName); catalogApi = client.catalogApi(principalCredentials); @@ -219,6 +215,21 @@ public void before(TestInfo testInfo) { managementApi.createCatalog(principalRoleName, catalog); + restCatalog = initCatalog(currentCatalogName, ImmutableMap.of()); + } + + @AfterEach + public void cleanUp() { + client.cleanUp(adminCredentials); + } + + @Override + protected RESTCatalog catalog() { + return restCatalog; + } + + @Override + protected RESTCatalog initCatalog(String catalogName, Map additionalProperties) { Optional restCatalogConfig = testInfo .getTestMethod() @@ -234,24 +245,14 @@ public void before(TestInfo testInfo) { extraPropertiesBuilder.put(config.value()[i], config.value()[i + 1]); } }); - - restCatalog = - IcebergHelper.restCatalog( - client, - endpoints, - principalCredentials, - currentCatalogName, - extraPropertiesBuilder.build()); - } - - @AfterEach - public void cleanUp() { - client.cleanUp(adminCredentials); - } - - @Override - protected RESTCatalog catalog() { - return restCatalog; + extraPropertiesBuilder.putAll(additionalProperties); + return IcebergHelper.restCatalog( + client, + endpoints, + principalCredentials, + catalogName, + currentCatalogName, + extraPropertiesBuilder.buildKeepingLast()); } @Override diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java index 28c38c99e0..c5eb99ef9a 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java @@ -113,7 +113,8 @@ public void before(TestInfo testInfo) { managementApi.createCatalog(principalRoleName, catalog); restCatalog = - IcebergHelper.restCatalog(client, endpoints, principalCredentials, catalogName, Map.of()); + IcebergHelper.restCatalog( + client, endpoints, principalCredentials, catalogName, catalogName, Map.of()); } @AfterEach diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java index 527f23d0be..e028e5e3b3 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java @@ -1721,7 +1721,7 @@ public Catalog createCallContextCatalog( FileIO fileIO = CatalogUtil.loadFileIO(fileIoImpl, Map.of(), new Configuration()); TableMetadata tableMetadata = TableMetadata.buildFromEmpty() - .addSchema(SCHEMA, SCHEMA.highestFieldId()) + .addSchema(SCHEMA) .setLocation( String.format("%s/bucket/table/metadata/v1.metadata.json", storageLocation)) .addPartitionSpec(PartitionSpec.unpartitioned()) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index 8d520b9a9f..92f133138d 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -266,10 +266,6 @@ public void before(TestInfo testInfo) { .setStorageConfigurationInfo(storageConfigModel, storageLocation) .build()); - PolarisPassthroughResolutionView passthroughView = - new PolarisPassthroughResolutionView( - callContext, entityManager, securityContext, CATALOG_NAME); - TaskExecutor taskExecutor = Mockito.mock(); RealmEntityManagerFactory realmEntityManagerFactory = new RealmEntityManagerFactory(createMockMetaStoreManagerFactory()); this.fileIOFactory = @@ -293,18 +289,10 @@ public void before(TestInfo testInfo) { .thenReturn((PolarisStorageIntegration) storageIntegration); this.catalog = - new IcebergCatalog( - entityManager, - metaStoreManager, - callContext, - passthroughView, - securityContext, - taskExecutor, - fileIOFactory); - this.catalog.initialize( - CATALOG_NAME, - ImmutableMap.of( - CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")); + initCatalog( + CATALOG_NAME, + ImmutableMap.of( + CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")); } @AfterEach @@ -318,6 +306,26 @@ protected IcebergCatalog catalog() { return catalog; } + @Override + protected IcebergCatalog initCatalog( + String catalogName, Map additionalProperties) { + PolarisPassthroughResolutionView passthroughView = + new PolarisPassthroughResolutionView( + callContext, entityManager, securityContext, CATALOG_NAME); + TaskExecutor taskExecutor = Mockito.mock(); + IcebergCatalog icebergCatalog = + new IcebergCatalog( + entityManager, + metaStoreManager, + callContext, + passthroughView, + securityContext, + taskExecutor, + fileIOFactory); + icebergCatalog.initialize(CATALOG_NAME, additionalProperties); + return icebergCatalog; + } + @Override protected boolean requiresNamespaceCreate() { return true; @@ -697,7 +705,7 @@ public void testCreateNotificationCreateTableInExternalLocation() { TableMetadata.buildFromEmpty() .assignUUID() .setLocation(anotherTableLocation) - .addSchema(SCHEMA, 4) + .addSchema(SCHEMA) .addPartitionSpec(PartitionSpec.unpartitioned()) .addSortOrder(SortOrder.unsorted()) .build(); @@ -754,7 +762,7 @@ public void testCreateNotificationCreateTableOutsideOfMetadataLocation() { TableMetadata.buildFromEmpty() .assignUUID() .setLocation(anotherTableLocation) - .addSchema(SCHEMA, 4) + .addSchema(SCHEMA) .addPartitionSpec(PartitionSpec.unpartitioned()) .addSortOrder(SortOrder.unsorted()) .build(); @@ -831,7 +839,7 @@ public void testUpdateNotificationCreateTableInExternalLocation() { TableMetadata.buildFromEmpty() .assignUUID() .setLocation(anotherTableLocation) - .addSchema(SCHEMA, 4) + .addSchema(SCHEMA) .addPartitionSpec(PartitionSpec.unpartitioned()) .addSortOrder(SortOrder.unsorted()) .build(); @@ -1401,7 +1409,7 @@ public void testDropNotificationWhenTableExists() { @Override public void testDropTableWithPurge() { if (this.requiresNamespaceCreate()) { - ((SupportsNamespaces) catalog).createNamespace(NS); + catalog.createNamespace(NS); } Assertions.assertThatPredicate(catalog::tableExists) @@ -1497,7 +1505,7 @@ public void testDropTableWithPurgeDisabled() { CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")); if (this.requiresNamespaceCreate()) { - ((SupportsNamespaces) noPurgeCatalog).createNamespace(NS); + noPurgeCatalog.createNamespace(NS); } Assertions.assertThatPredicate(noPurgeCatalog::tableExists) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TaskTestUtils.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TaskTestUtils.java index 9cd0ddd9e4..9d47fe9472 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TaskTestUtils.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TaskTestUtils.java @@ -100,8 +100,7 @@ static TableMetadata writeTableMetadata( tmBuilder .setLocation("path/to/table") .addSchema( - new Schema(List.of(Types.NestedField.of(1, false, "field1", Types.StringType.get()))), - 1) + new Schema(List.of(Types.NestedField.of(1, false, "field1", Types.StringType.get())))) .addSortOrder(SortOrder.unsorted()) .assignUUID(UUID.randomUUID().toString()) .addPartitionSpec(PartitionSpec.unpartitioned()); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index d88ea3a7e4..7cb4f2f957 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -91,23 +91,27 @@ public class IcebergCatalogAdapter ImmutableSet.builder() .add(Endpoint.V1_LIST_NAMESPACES) .add(Endpoint.V1_LOAD_NAMESPACE) + .add(Endpoint.V1_NAMESPACE_EXISTS) .add(Endpoint.V1_CREATE_NAMESPACE) .add(Endpoint.V1_UPDATE_NAMESPACE) .add(Endpoint.V1_DELETE_NAMESPACE) .add(Endpoint.V1_LIST_TABLES) .add(Endpoint.V1_LOAD_TABLE) + .add(Endpoint.V1_TABLE_EXISTS) .add(Endpoint.V1_CREATE_TABLE) .add(Endpoint.V1_UPDATE_TABLE) .add(Endpoint.V1_DELETE_TABLE) .add(Endpoint.V1_RENAME_TABLE) .add(Endpoint.V1_REGISTER_TABLE) .add(Endpoint.V1_REPORT_METRICS) + .add(Endpoint.V1_COMMIT_TRANSACTION) .build(); private static final Set VIEW_ENDPOINTS = ImmutableSet.builder() .add(Endpoint.V1_LIST_VIEWS) .add(Endpoint.V1_LOAD_VIEW) + .add(Endpoint.V1_VIEW_EXISTS) .add(Endpoint.V1_CREATE_VIEW) .add(Endpoint.V1_UPDATE_VIEW) .add(Endpoint.V1_DELETE_VIEW) From 9efde2aed0e042a7d39f9fdd45beb18854161888 Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Tue, 25 Feb 2025 21:00:52 -0500 Subject: [PATCH 02/21] Skip tests using Junit Assumptions --- .../PolarisRestCatalogIntegrationTest.java | 22 +++++++++++++++++++ .../quarkus/catalog/IcebergCatalogTest.java | 21 ++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index e7dddfda3c..0c8caaac3c 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -91,6 +91,7 @@ import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -275,6 +276,27 @@ protected boolean overridesRequestedLocation() { return true; } + @Test + @Override + public void createAndDropEmptyNamespace() { + Assumptions.assumeTrue(supportsEmptyNamespace()); + super.createAndDropEmptyNamespace(); + } + + @Test + @Override + public void namespacePropertiesOnEmptyNamespace() { + Assumptions.assumeTrue(supportsEmptyNamespace()); + super.namespacePropertiesOnEmptyNamespace(); + } + + @Test + @Override + public void listTablesInEmptyNamespace() { + Assumptions.assumeTrue(supportsEmptyNamespace()); + super.listTablesInEmptyNamespace(); + } + @Test public void testListGrantsOnCatalogObjectsToCatalogRoles() { restCatalog.createNamespace(Namespace.of("ns1")); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index 92f133138d..3553f7a9ee 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -381,6 +381,27 @@ public Map purgeRealms(Iterable realms) { }; } + @Test + @Override + public void createAndDropEmptyNamespace() { + Assumptions.assumeTrue(supportsEmptyNamespace()); + super.createAndDropEmptyNamespace(); + } + + @Test + @Override + public void namespacePropertiesOnEmptyNamespace() { + Assumptions.assumeTrue(supportsEmptyNamespace()); + super.namespacePropertiesOnEmptyNamespace(); + } + + @Test + @Override + public void listTablesInEmptyNamespace() { + Assumptions.assumeTrue(supportsEmptyNamespace()); + super.listTablesInEmptyNamespace(); + } + @Test public void testRenameTableMissingDestinationNamespace() { Assumptions.assumeTrue( From a472d6a3a9aa9aef08903928c08f34d4160f0b6a Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Wed, 26 Feb 2025 20:17:04 -0500 Subject: [PATCH 03/21] Fix `testCatalogWithCustomMetricsReporter` in `BasePolarisCatalogTest` --- .../quarkus/catalog/IcebergCatalogTest.java | 60 ++++++++++--------- .../catalog/iceberg/IcebergCatalog.java | 5 +- 2 files changed, 35 insertions(+), 30 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index 3553f7a9ee..56f2a4c12f 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -153,6 +153,8 @@ public Map getConfigOverrides() { "true", "polaris.features.defaults.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"", "true", + "polaris.features.defaults.\"ALLOW_OVERLAPPING_CATALOG_URLS\"", + "true", "polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", "[\"FILE\"]"); } @@ -244,28 +246,6 @@ public void before(TestInfo testInfo) { securityContext, new PolarisAuthorizerImpl(new PolarisConfigurationStore() {})); - String storageLocation = "s3://my-bucket/path/to/data"; - storageConfigModel = - AwsStorageConfigInfo.builder() - .setRoleArn("arn:aws:iam::012345678901:role/jdoe") - .setExternalId("externalId") - .setUserArn("aws::a:user:arn") - .setStorageType(StorageConfigInfo.StorageTypeEnum.S3) - .setAllowedLocations(List.of(storageLocation, "s3://externally-owned-bucket")) - .build(); - catalogEntity = - adminService.createCatalog( - new CatalogEntity.Builder() - .setName(CATALOG_NAME) - .setDefaultBaseLocation(storageLocation) - .setReplaceNewLocationPrefixWithCatalogDefault("file:") - .addProperty( - FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "true") - .addProperty( - FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true") - .setStorageConfigurationInfo(storageConfigModel, storageLocation) - .build()); - RealmEntityManagerFactory realmEntityManagerFactory = new RealmEntityManagerFactory(createMockMetaStoreManagerFactory()); this.fileIOFactory = @@ -288,11 +268,7 @@ public void before(TestInfo testInfo) { isA(AwsStorageConfigurationInfo.class))) .thenReturn((PolarisStorageIntegration) storageIntegration); - this.catalog = - initCatalog( - CATALOG_NAME, - ImmutableMap.of( - CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")); + this.catalog = initCatalog(CATALOG_NAME, ImmutableMap.of()); } @AfterEach @@ -309,9 +285,30 @@ protected IcebergCatalog catalog() { @Override protected IcebergCatalog initCatalog( String catalogName, Map additionalProperties) { + String storageLocation = "s3://my-bucket/path/to/data"; + storageConfigModel = + AwsStorageConfigInfo.builder() + .setRoleArn("arn:aws:iam::012345678901:role/jdoe") + .setExternalId("externalId") + .setUserArn("aws::a:user:arn") + .setStorageType(StorageConfigInfo.StorageTypeEnum.S3) + .setAllowedLocations(List.of(storageLocation, "s3://externally-owned-bucket")) + .build(); + catalogEntity = + adminService.createCatalog( + new CatalogEntity.Builder() + .setName(catalogName) + .setDefaultBaseLocation(storageLocation) + .setReplaceNewLocationPrefixWithCatalogDefault("file:") + .addProperty( + FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "true") + .addProperty( + FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true") + .setStorageConfigurationInfo(storageConfigModel, storageLocation) + .build()); PolarisPassthroughResolutionView passthroughView = new PolarisPassthroughResolutionView( - callContext, entityManager, securityContext, CATALOG_NAME); + callContext, entityManager, securityContext, catalogName); TaskExecutor taskExecutor = Mockito.mock(); IcebergCatalog icebergCatalog = new IcebergCatalog( @@ -322,7 +319,12 @@ protected IcebergCatalog initCatalog( securityContext, taskExecutor, fileIOFactory); - icebergCatalog.initialize(CATALOG_NAME, additionalProperties); + + ImmutableMap.Builder propertiesBuilder = + ImmutableMap.builder() + .put(CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO") + .putAll(additionalProperties); + icebergCatalog.initialize(catalogName, propertiesBuilder.buildKeepingLast()); return icebergCatalog; } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index a6c718151f..3f9722f793 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -217,6 +217,10 @@ public void initialize(String name, Map properties) { name, this.catalogName); + // Ensure catalogProperties is assigned before calling metricsReporter() for proper + // functionality. + catalogProperties = properties; + // Base location from catalogEntity is primary source of truth, otherwise fall through // to the same key from the properties map, and finally fall through to WAREHOUSE_LOCATION. String baseLocation = @@ -263,7 +267,6 @@ public void initialize(String name, Map properties) { closeableGroup.addCloseable(metricsReporter()); closeableGroup.setSuppressCloseFailure(true); - catalogProperties = properties; tableDefaultProperties = PropertyUtil.propertiesWithPrefix(properties, CatalogProperties.TABLE_DEFAULT_PREFIX); From 161c9b5b42066faec9a61d0aab681ede8c28c031 Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Thu, 27 Feb 2025 20:41:29 -0500 Subject: [PATCH 04/21] Skip `listNamespacesWithEmptyNamespace` and add comments --- .../test/PolarisRestCatalogIntegrationTest.java | 6 ++++++ .../quarkus/catalog/IcebergCatalogTest.java | 17 +++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index 0c8caaac3c..eaafd99981 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -279,6 +279,8 @@ protected boolean overridesRequestedLocation() { @Test @Override public void createAndDropEmptyNamespace() { + // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. + // This test can be removed once Quarkus supports AssertJ. Assumptions.assumeTrue(supportsEmptyNamespace()); super.createAndDropEmptyNamespace(); } @@ -286,6 +288,8 @@ public void createAndDropEmptyNamespace() { @Test @Override public void namespacePropertiesOnEmptyNamespace() { + // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. + // This test can be removed once Quarkus supports AssertJ. Assumptions.assumeTrue(supportsEmptyNamespace()); super.namespacePropertiesOnEmptyNamespace(); } @@ -293,6 +297,8 @@ public void namespacePropertiesOnEmptyNamespace() { @Test @Override public void listTablesInEmptyNamespace() { + // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. + // This test can be removed once Quarkus supports AssertJ. Assumptions.assumeTrue(supportsEmptyNamespace()); super.listTablesInEmptyNamespace(); } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index 56f2a4c12f..0567db7e5f 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -62,7 +62,6 @@ import org.apache.iceberg.UpdateSchema; import org.apache.iceberg.catalog.CatalogTests; import org.apache.iceberg.catalog.Namespace; -import org.apache.iceberg.catalog.SupportsNamespaces; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; import org.apache.iceberg.exceptions.BadRequestException; @@ -179,7 +178,6 @@ public Map getConfigOverrides() { private IcebergCatalog catalog; private CallContext callContext; private AwsStorageConfigInfo storageConfigModel; - private StsClient stsClient; private String realmName; private PolarisMetaStoreManager metaStoreManager; private PolarisCallContext polarisContext; @@ -383,9 +381,20 @@ public Map purgeRealms(Iterable realms) { }; } + @Test + @Override + public void listNamespacesWithEmptyNamespace() { + // TODO: remove this override test once Polaris handles empty namespaces the same as the + // superclass expectation. + Assumptions.assumeTrue(supportsEmptyNamespace()); + super.listNamespacesWithEmptyNamespace(); + } + @Test @Override public void createAndDropEmptyNamespace() { + // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. + // This test can be removed once Quarkus supports AssertJ or Polaris supports empty namespaces. Assumptions.assumeTrue(supportsEmptyNamespace()); super.createAndDropEmptyNamespace(); } @@ -393,6 +402,8 @@ public void createAndDropEmptyNamespace() { @Test @Override public void namespacePropertiesOnEmptyNamespace() { + // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. + // This test can be removed once Quarkus supports AssertJ or Polaris supports empty namespaces. Assumptions.assumeTrue(supportsEmptyNamespace()); super.namespacePropertiesOnEmptyNamespace(); } @@ -400,6 +411,8 @@ public void namespacePropertiesOnEmptyNamespace() { @Test @Override public void listTablesInEmptyNamespace() { + // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. + // This test can be removed once Quarkus supports AssertJ or Polaris supports empty namespaces. Assumptions.assumeTrue(supportsEmptyNamespace()); super.listTablesInEmptyNamespace(); } From 76b4e1e6c39ac101f94a75f16be97945ffcb31f8 Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Fri, 28 Feb 2025 16:56:28 -0500 Subject: [PATCH 05/21] Fix `defaultViewProperties` in `ViewCatalogTests` --- ...PolarisRestCatalogViewIntegrationBase.java | 21 ++++++++++++++++++- .../catalog/IcebergCatalogViewTest.java | 18 +++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java index c5eb99ef9a..27c85d0d80 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java @@ -42,6 +42,7 @@ import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.extension.ExtendWith; @@ -114,7 +115,16 @@ public void before(TestInfo testInfo) { restCatalog = IcebergHelper.restCatalog( - client, endpoints, principalCredentials, catalogName, catalogName, Map.of()); + client, + endpoints, + principalCredentials, + catalogName, + catalogName, + Map.of( + org.apache.iceberg.CatalogProperties.VIEW_DEFAULT_PREFIX + "key1", + "catalog-default-key1", + org.apache.iceberg.CatalogProperties.VIEW_DEFAULT_PREFIX + "key2", + "catalog-default-key2")); } @AfterEach @@ -157,4 +167,13 @@ protected boolean supportsServerSideRetry() { protected boolean overridesRequestedLocation() { return true; } + + @Test + @Override + public void listViewsInEmptyNamespace() { + // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. + // This test can be removed once Quarkus supports AssertJ or Polaris supports empty namespaces. + Assumptions.assumeTrue(supportsEmptyNamespace()); + super.listViewsInEmptyNamespace(); + } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java index bea9aa3100..a682a95495 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java @@ -66,8 +66,10 @@ import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.io.TempDir; import org.mockito.Mockito; @@ -199,7 +201,12 @@ public void before(TestInfo testInfo) { this.catalog.initialize( CATALOG_NAME, ImmutableMap.of( - CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")); + CatalogProperties.FILE_IO_IMPL, + "org.apache.iceberg.inmemory.InMemoryFileIO", + CatalogProperties.VIEW_DEFAULT_PREFIX + "key1", + "catalog-default-key1", + CatalogProperties.VIEW_DEFAULT_PREFIX + "key2", + "catalog-default-key2")); } @AfterEach @@ -222,4 +229,13 @@ protected Catalog tableCatalog() { protected boolean requiresNamespaceCreate() { return true; } + + @Test + @Override + public void listViewsInEmptyNamespace() { + // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. + // This test can be removed once Quarkus supports AssertJ or Polaris supports empty namespaces. + Assumptions.assumeTrue(supportsEmptyNamespace()); + super.listViewsInEmptyNamespace(); + } } From f09a0631ba2bd831912ac6c1d0c23965c7a899be Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Tue, 4 Mar 2025 20:47:05 -0500 Subject: [PATCH 06/21] Fix `createViewWithCustomMetadataLocation` in `ViewCatalogTests` --- .../it/QuarkusRestCatalogViewFileIT.java | 19 ++++++++++++++++++- ...kusRestCatalogViewFileIntegrationTest.java | 19 ++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/quarkus/service/src/intTest/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIT.java b/quarkus/service/src/intTest/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIT.java index 9b68ece256..b421586e2c 100644 --- a/quarkus/service/src/intTest/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIT.java +++ b/quarkus/service/src/intTest/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIT.java @@ -20,20 +20,37 @@ import io.quarkus.test.junit.QuarkusIntegrationTest; import java.lang.reflect.Field; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import org.apache.iceberg.view.ViewCatalogTests; import org.apache.polaris.service.it.test.PolarisRestCatalogViewFileIntegrationTest; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.extension.AnnotatedElementContext; +import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.api.io.TempDirFactory; @QuarkusIntegrationTest public class QuarkusRestCatalogViewFileIT extends PolarisRestCatalogViewFileIntegrationTest { @BeforeEach - public void setUpTempDir(@TempDir Path tempDir) throws Exception { + public void setUpTempDir(@TempDir(factory = CustomTempDirFactory.class) Path tempDir) + throws Exception { // see https://github.com/quarkusio/quarkus/issues/13261 Field field = ViewCatalogTests.class.getDeclaredField("tempDir"); field.setAccessible(true); field.set(this, tempDir); } + + private static class CustomTempDirFactory implements TempDirFactory { + @Override + public Path createTempDirectory( + AnnotatedElementContext elementContext, ExtensionContext extensionContext) + throws Exception { + Path basePath = Paths.get(BASE_LOCATION.replaceFirst("file://", "")); + Files.createDirectories(basePath); + return Files.createTempDirectory(basePath, null); + } + } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIntegrationTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIntegrationTest.java index 4444bcbee3..97bd91b68b 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIntegrationTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIntegrationTest.java @@ -21,12 +21,17 @@ import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.QuarkusTestProfile; import java.lang.reflect.Field; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Map; import org.apache.iceberg.view.ViewCatalogTests; import org.apache.polaris.service.it.test.PolarisRestCatalogViewFileIntegrationTest; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.extension.AnnotatedElementContext; +import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.api.io.TempDirFactory; @QuarkusTest public class QuarkusRestCatalogViewFileIntegrationTest @@ -41,10 +46,22 @@ public Map getConfigOverrides() { } @BeforeEach - public void setUpTempDir(@TempDir Path tempDir) throws Exception { + public void setUpTempDir(@TempDir(factory = CustomTempDirFactory.class) Path tempDir) + throws Exception { // see https://github.com/quarkusio/quarkus/issues/13261 Field field = ViewCatalogTests.class.getDeclaredField("tempDir"); field.setAccessible(true); field.set(this, tempDir); } + + private static class CustomTempDirFactory implements TempDirFactory { + @Override + public Path createTempDirectory( + AnnotatedElementContext elementContext, ExtensionContext extensionContext) + throws Exception { + Path basePath = Paths.get(BASE_LOCATION.replaceFirst("file://", "")); + Files.createDirectories(basePath); + return Files.createTempDirectory(basePath, null); + } + } } From 235846245b245fc6fcefe3dc84757266664fad0d Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Wed, 5 Mar 2025 10:05:39 -0500 Subject: [PATCH 07/21] Fix tests for Iceberg API token endpoint --- .../service/quarkus/it/QuarkusApplicationIntegrationTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusApplicationIntegrationTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusApplicationIntegrationTest.java index ba1395ac8e..9c72c9b065 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusApplicationIntegrationTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusApplicationIntegrationTest.java @@ -36,6 +36,7 @@ import org.apache.iceberg.rest.HTTPClient; import org.apache.iceberg.rest.RESTClient; import org.apache.iceberg.rest.auth.AuthConfig; +import org.apache.iceberg.rest.auth.AuthSession; import org.apache.iceberg.rest.auth.OAuth2Util; import org.apache.iceberg.rest.responses.OAuthTokenResponse; import org.apache.polaris.service.auth.TokenBrokerFactory; @@ -72,6 +73,7 @@ public void testIcebergRestApiRefreshExpiredToken( HTTPClient.builder(Map.of()) .withHeader(endpoints.realmHeaderName(), endpoints.realmId()) .uri(path) + .withAuthSession(AuthSession.EMPTY) .build()) { String credentialString = clientCredentials.clientId() + ":" + clientCredentials.clientSecret(); @@ -102,6 +104,7 @@ public void testIcebergRestApiRefreshValidToken( HTTPClient.builder(Map.of()) .withHeader(endpoints.realmHeaderName(), endpoints.realmId()) .uri(path) + .withAuthSession(AuthSession.EMPTY) .build()) { var response = client.postForm( @@ -142,6 +145,7 @@ public void testIcebergRestApiInvalidToken( HTTPClient.builder(Map.of()) .withHeader(endpoints.realmHeaderName(), endpoints.realmId()) .uri(path) + .withAuthSession(AuthSession.EMPTY) .build()) { var response = client.postForm( From 6da77dfa445515e92a5d2de9e402b7647215f621 Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Thu, 6 Mar 2025 10:18:51 -0500 Subject: [PATCH 08/21] Fix regtests --- regtests/t_spark_sql/ref/spark_sql_basic.sh.ref | 2 +- regtests/t_spark_sql/ref/spark_sql_views.sh.ref | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/regtests/t_spark_sql/ref/spark_sql_basic.sh.ref b/regtests/t_spark_sql/ref/spark_sql_basic.sh.ref index a23d1d941e..4cd8ce0f81 100755 --- a/regtests/t_spark_sql/ref/spark_sql_basic.sh.ref +++ b/regtests/t_spark_sql/ref/spark_sql_basic.sh.ref @@ -1,4 +1,4 @@ -{"defaults":{"default-base-location":"file:///tmp/spark_sql_s3_catalog"},"overrides":{"prefix":"spark_sql_basic_catalog"},"endpoints":["GET /v1/{prefix}/namespaces","GET /v1/{prefix}/namespaces/{namespace}","POST /v1/{prefix}/namespaces","POST /v1/{prefix}/namespaces/{namespace}/properties","DELETE /v1/{prefix}/namespaces/{namespace}","GET /v1/{prefix}/namespaces/{namespace}/tables","GET /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/namespaces/{namespace}/tables","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}","DELETE /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/tables/rename","POST /v1/{prefix}/namespaces/{namespace}/register","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics","GET /v1/{prefix}/namespaces/{namespace}/views","GET /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/namespaces/{namespace}/views","POST /v1/{prefix}/namespaces/{namespace}/views/{view}","DELETE /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/views/rename","POST /v1/{prefix}/transactions/commit"]} +{"defaults":{"default-base-location":"file:///tmp/spark_sql_s3_catalog"},"overrides":{"prefix":"spark_sql_basic_catalog"},"endpoints":["GET /v1/{prefix}/namespaces","GET /v1/{prefix}/namespaces/{namespace}","HEAD /v1/{prefix}/namespaces/{namespace}","POST /v1/{prefix}/namespaces","POST /v1/{prefix}/namespaces/{namespace}/properties","DELETE /v1/{prefix}/namespaces/{namespace}","GET /v1/{prefix}/namespaces/{namespace}/tables","GET /v1/{prefix}/namespaces/{namespace}/tables/{table}","HEAD /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/namespaces/{namespace}/tables","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}","DELETE /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/tables/rename","POST /v1/{prefix}/namespaces/{namespace}/register","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics","POST /v1/{prefix}/transactions/commit","GET /v1/{prefix}/namespaces/{namespace}/views","GET /v1/{prefix}/namespaces/{namespace}/views/{view}","HEAD /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/namespaces/{namespace}/views","POST /v1/{prefix}/namespaces/{namespace}/views/{view}","DELETE /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/views/rename","POST /v1/{prefix}/transactions/commit"]} Catalog created spark-sql (default)> use polaris; spark-sql ()> show namespaces; diff --git a/regtests/t_spark_sql/ref/spark_sql_views.sh.ref b/regtests/t_spark_sql/ref/spark_sql_views.sh.ref index 8e4bff2702..853c736dba 100755 --- a/regtests/t_spark_sql/ref/spark_sql_views.sh.ref +++ b/regtests/t_spark_sql/ref/spark_sql_views.sh.ref @@ -1,4 +1,4 @@ -{"defaults":{"default-base-location":"file:///tmp/spark_sql_s3_catalog"},"overrides":{"prefix":"spark_sql_views_catalog"},"endpoints":["GET /v1/{prefix}/namespaces","GET /v1/{prefix}/namespaces/{namespace}","POST /v1/{prefix}/namespaces","POST /v1/{prefix}/namespaces/{namespace}/properties","DELETE /v1/{prefix}/namespaces/{namespace}","GET /v1/{prefix}/namespaces/{namespace}/tables","GET /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/namespaces/{namespace}/tables","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}","DELETE /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/tables/rename","POST /v1/{prefix}/namespaces/{namespace}/register","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics","GET /v1/{prefix}/namespaces/{namespace}/views","GET /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/namespaces/{namespace}/views","POST /v1/{prefix}/namespaces/{namespace}/views/{view}","DELETE /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/views/rename","POST /v1/{prefix}/transactions/commit"]} +{"defaults":{"default-base-location":"file:///tmp/spark_sql_s3_catalog"},"overrides":{"prefix":"spark_sql_views_catalog"},"endpoints":["GET /v1/{prefix}/namespaces","GET /v1/{prefix}/namespaces/{namespace}","HEAD /v1/{prefix}/namespaces/{namespace}","POST /v1/{prefix}/namespaces","POST /v1/{prefix}/namespaces/{namespace}/properties","DELETE /v1/{prefix}/namespaces/{namespace}","GET /v1/{prefix}/namespaces/{namespace}/tables","GET /v1/{prefix}/namespaces/{namespace}/tables/{table}","HEAD /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/namespaces/{namespace}/tables","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}","DELETE /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/tables/rename","POST /v1/{prefix}/namespaces/{namespace}/register","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics","POST /v1/{prefix}/transactions/commit","GET /v1/{prefix}/namespaces/{namespace}/views","GET /v1/{prefix}/namespaces/{namespace}/views/{view}","HEAD /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/namespaces/{namespace}/views","POST /v1/{prefix}/namespaces/{namespace}/views/{view}","DELETE /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/views/rename","POST /v1/{prefix}/transactions/commit"]} Catalog created spark-sql (default)> use polaris; spark-sql ()> show namespaces; From 828af4bb15c0973e37cd6193f0f3e7b633ead345 Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Mon, 10 Mar 2025 21:51:49 -0400 Subject: [PATCH 09/21] Set preferredAssumptionException to JUnit5 --- .../PolarisRestCatalogIntegrationTest.java | 33 +++---------------- ...PolarisRestCatalogViewIntegrationBase.java | 15 +++------ .../quarkus/catalog/IcebergCatalogTest.java | 32 +++--------------- .../catalog/IcebergCatalogViewTest.java | 16 +++------ 4 files changed, 20 insertions(+), 76 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index eaafd99981..3c75244683 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -88,10 +88,11 @@ import org.apache.polaris.service.it.env.PolarisClient; import org.apache.polaris.service.it.ext.PolarisIntegrationTestExtension; import org.assertj.core.api.Assertions; +import org.assertj.core.api.Assumptions; import org.assertj.core.api.InstanceOfAssertFactories; +import org.assertj.core.configuration.PreferredAssumptionException; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -156,6 +157,9 @@ String[] properties() default { @BeforeAll static void setup( PolarisApiEndpoints apiEndpoints, ClientCredentials credentials, @TempDir Path tempDir) { + // Set preferredAssumptionException as Quarkus does not suppress JUnit4's + // AssumptionViolatedException + Assumptions.setPreferredAssumptionException(PreferredAssumptionException.JUNIT5); adminCredentials = credentials; endpoints = apiEndpoints; client = polarisClient(endpoints); @@ -276,33 +280,6 @@ protected boolean overridesRequestedLocation() { return true; } - @Test - @Override - public void createAndDropEmptyNamespace() { - // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. - // This test can be removed once Quarkus supports AssertJ. - Assumptions.assumeTrue(supportsEmptyNamespace()); - super.createAndDropEmptyNamespace(); - } - - @Test - @Override - public void namespacePropertiesOnEmptyNamespace() { - // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. - // This test can be removed once Quarkus supports AssertJ. - Assumptions.assumeTrue(supportsEmptyNamespace()); - super.namespacePropertiesOnEmptyNamespace(); - } - - @Test - @Override - public void listTablesInEmptyNamespace() { - // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. - // This test can be removed once Quarkus supports AssertJ. - Assumptions.assumeTrue(supportsEmptyNamespace()); - super.listTablesInEmptyNamespace(); - } - @Test public void testListGrantsOnCatalogObjectsToCatalogRoles() { restCatalog.createNamespace(Namespace.of("ns1")); diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java index 27c85d0d80..580ff896b1 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java @@ -37,12 +37,12 @@ import org.apache.polaris.service.it.env.PolarisApiEndpoints; import org.apache.polaris.service.it.env.PolarisClient; import org.apache.polaris.service.it.ext.PolarisIntegrationTestExtension; +import org.assertj.core.configuration.PreferredAssumptionException; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.extension.ExtendWith; @@ -66,6 +66,10 @@ public abstract class PolarisRestCatalogViewIntegrationBase extends ViewCatalogT @BeforeAll static void setup(PolarisApiEndpoints apiEndpoints, ClientCredentials credentials) { + // Set preferredAssumptionException as Quarkus does not suppress JUnit4's + // AssumptionViolatedException + org.assertj.core.api.Assumptions.setPreferredAssumptionException( + PreferredAssumptionException.JUNIT5); adminCredentials = credentials; endpoints = apiEndpoints; client = polarisClient(endpoints); @@ -167,13 +171,4 @@ protected boolean supportsServerSideRetry() { protected boolean overridesRequestedLocation() { return true; } - - @Test - @Override - public void listViewsInEmptyNamespace() { - // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. - // This test can be removed once Quarkus supports AssertJ or Polaris supports empty namespaces. - Assumptions.assumeTrue(supportsEmptyNamespace()); - super.listViewsInEmptyNamespace(); - } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index 0567db7e5f..2f9282155a 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -123,6 +123,7 @@ import org.apache.polaris.service.types.NotificationType; import org.apache.polaris.service.types.TableUpdateNotification; import org.assertj.core.api.Assertions; +import org.assertj.core.configuration.PreferredAssumptionException; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; @@ -193,6 +194,10 @@ public static void setUpMocks() { PolarisStorageIntegrationProviderImpl mock = Mockito.mock(PolarisStorageIntegrationProviderImpl.class); QuarkusMock.installMockForType(mock, PolarisStorageIntegrationProviderImpl.class); + // Set preferredAssumptionException as Quarkus does not suppress JUnit4's + // AssumptionViolatedException + org.assertj.core.api.Assumptions.setPreferredAssumptionException( + PreferredAssumptionException.JUNIT5); } @Nullable @@ -390,33 +395,6 @@ public void listNamespacesWithEmptyNamespace() { super.listNamespacesWithEmptyNamespace(); } - @Test - @Override - public void createAndDropEmptyNamespace() { - // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. - // This test can be removed once Quarkus supports AssertJ or Polaris supports empty namespaces. - Assumptions.assumeTrue(supportsEmptyNamespace()); - super.createAndDropEmptyNamespace(); - } - - @Test - @Override - public void namespacePropertiesOnEmptyNamespace() { - // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. - // This test can be removed once Quarkus supports AssertJ or Polaris supports empty namespaces. - Assumptions.assumeTrue(supportsEmptyNamespace()); - super.namespacePropertiesOnEmptyNamespace(); - } - - @Test - @Override - public void listTablesInEmptyNamespace() { - // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. - // This test can be removed once Quarkus supports AssertJ or Polaris supports empty namespaces. - Assumptions.assumeTrue(supportsEmptyNamespace()); - super.listTablesInEmptyNamespace(); - } - @Test public void testRenameTableMissingDestinationNamespace() { Assumptions.assumeTrue( diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java index a682a95495..e664537d38 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java @@ -65,11 +65,11 @@ import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl; +import org.assertj.core.api.Assumptions; +import org.assertj.core.configuration.PreferredAssumptionException; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.io.TempDir; import org.mockito.Mockito; @@ -113,6 +113,9 @@ public static void setUpMocks() { PolarisStorageIntegrationProviderImpl mock = Mockito.mock(PolarisStorageIntegrationProviderImpl.class); QuarkusMock.installMockForType(mock, PolarisStorageIntegrationProviderImpl.class); + // Set preferredAssumptionException as Quarkus does not suppress JUnit4's + // AssumptionViolatedException + Assumptions.setPreferredAssumptionException(PreferredAssumptionException.JUNIT5); } @BeforeEach @@ -229,13 +232,4 @@ protected Catalog tableCatalog() { protected boolean requiresNamespaceCreate() { return true; } - - @Test - @Override - public void listViewsInEmptyNamespace() { - // Skip this test because AssertJ's Assumptions.assumeThat() is not compatible with Quarkus. - // This test can be removed once Quarkus supports AssertJ or Polaris supports empty namespaces. - Assumptions.assumeTrue(supportsEmptyNamespace()); - super.listViewsInEmptyNamespace(); - } } From 0251608ffa7b85782fd7acf8052961dee436394d Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Wed, 12 Mar 2025 18:57:52 -0400 Subject: [PATCH 10/21] Fix warnings --- .../apache/polaris/service/quarkus/task/TaskTestUtils.java | 2 +- .../polaris/service/task/ManifestFileCleanupTaskHandler.java | 5 +---- .../polaris/service/exception/ExceptionMapperTest.java | 3 ++- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TaskTestUtils.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TaskTestUtils.java index 9d47fe9472..d7538b923f 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TaskTestUtils.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TaskTestUtils.java @@ -109,7 +109,7 @@ static TableMetadata writeTableMetadata( for (Snapshot snapshot : snapshots) { tmBuilder.addSnapshot(snapshot); if (statisticsFiles != null) { - tmBuilder.setStatistics(snapshot.snapshotId(), statisticsFiles.get(statisticsFileIndex++)); + tmBuilder.setStatistics(statisticsFiles.get(statisticsFileIndex++)); } } TableMetadata tableMetadata = tmBuilder.build(); diff --git a/service/common/src/main/java/org/apache/polaris/service/task/ManifestFileCleanupTaskHandler.java b/service/common/src/main/java/org/apache/polaris/service/task/ManifestFileCleanupTaskHandler.java index 84e9683a27..3173dc25e7 100644 --- a/service/common/src/main/java/org/apache/polaris/service/task/ManifestFileCleanupTaskHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/task/ManifestFileCleanupTaskHandler.java @@ -87,10 +87,7 @@ private boolean cleanUpManifestFile( StreamSupport.stream( Spliterators.spliteratorUnknownSize(dataFiles.iterator(), Spliterator.IMMUTABLE), false) - .map( - file -> - tryDelete( - tableId, fileIO, manifestFile.path(), file.path().toString(), null, 1)) + .map(file -> tryDelete(tableId, fileIO, manifestFile.path(), file.location(), null, 1)) .toList(); LOGGER.debug( "Scheduled {} data files to be deleted from manifest {}", diff --git a/service/common/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java b/service/common/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java index 2a27f2f381..bc5f84c1d3 100644 --- a/service/common/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java +++ b/service/common/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java @@ -42,7 +42,8 @@ public class ExceptionMapperTest { @ParameterizedTest @MethodSource("testFullExceptionIsLogged") - public void testFullExceptionIsLogged(ExceptionMapper mapper, Exception exception, Level level) { + public void testFullExceptionIsLogged( + ExceptionMapper mapper, Exception exception, Level level) { Logger logger = (Logger) LoggerFactory.getLogger(mapper.getClass()); ListAppender listAppender = new ListAppender<>(); listAppender.start(); From 71f8e13bc0dc2a6b1468f67388195181e5cfe969 Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Tue, 18 Mar 2025 21:46:20 -0400 Subject: [PATCH 11/21] Upgrade Iceberg version for regtests and docs --- getting-started/spark/notebooks/SparkPolaris.ipynb | 2 +- regtests/setup.sh | 2 +- regtests/t_pyspark/src/iceberg_spark.py | 2 +- site/content/in-dev/unreleased/quickstart.md | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/getting-started/spark/notebooks/SparkPolaris.ipynb b/getting-started/spark/notebooks/SparkPolaris.ipynb index adb2f1a2ce..48fff32ace 100644 --- a/getting-started/spark/notebooks/SparkPolaris.ipynb +++ b/getting-started/spark/notebooks/SparkPolaris.ipynb @@ -256,7 +256,7 @@ "\n", "spark = (SparkSession.builder\n", " .config(\"spark.sql.catalog.spark_catalog\", \"org.apache.iceberg.spark.SparkSessionCatalog\")\n", - " .config(\"spark.jars.packages\", \"org.apache.iceberg:iceberg-spark-runtime-3.5_2.12:1.7.1,org.apache.hadoop:hadoop-aws:3.4.0,software.amazon.awssdk:bundle:2.23.19,software.amazon.awssdk:url-connection-client:2.23.19\")\n", + " .config(\"spark.jars.packages\", \"org.apache.iceberg:iceberg-spark-runtime-3.5_2.12:1.8.1,org.apache.hadoop:hadoop-aws:3.4.0,software.amazon.awssdk:bundle:2.23.19,software.amazon.awssdk:url-connection-client:2.23.19\")\n", " .config('spark.sql.iceberg.vectorization.enabled', 'false')\n", " \n", " # Configure the 'polaris' catalog as an Iceberg rest catalog\n", diff --git a/regtests/setup.sh b/regtests/setup.sh index 91a8bf5f65..e65e5a337b 100755 --- a/regtests/setup.sh +++ b/regtests/setup.sh @@ -31,7 +31,7 @@ if [ -z "${SPARK_HOME}" ]; then fi SPARK_CONF="${SPARK_HOME}/conf/spark-defaults.conf" DERBY_HOME="/tmp/derby" -ICEBERG_VERSION="1.7.1" +ICEBERG_VERSION="1.8.1" export PYTHONPATH="${SPARK_HOME}/python/:${SPARK_HOME}/python/lib/py4j-0.10.9.7-src.zip:$PYTHONPATH" # Ensure binaries are downloaded locally diff --git a/regtests/t_pyspark/src/iceberg_spark.py b/regtests/t_pyspark/src/iceberg_spark.py index 9b6a393d09..cd0c4ade52 100644 --- a/regtests/t_pyspark/src/iceberg_spark.py +++ b/regtests/t_pyspark/src/iceberg_spark.py @@ -72,7 +72,7 @@ def __enter__(self): """Initial method for Iceberg Spark session. Creates a Spark session with specified configs. """ packages = [ - "org.apache.iceberg:iceberg-spark-runtime-3.5_2.12:1.7.1", + "org.apache.iceberg:iceberg-spark-runtime-3.5_2.12:1.8.1", "org.apache.hadoop:hadoop-aws:3.4.0", "software.amazon.awssdk:bundle:2.23.19", "software.amazon.awssdk:url-connection-client:2.23.19", diff --git a/site/content/in-dev/unreleased/quickstart.md b/site/content/in-dev/unreleased/quickstart.md index 9ab93c7fec..1b45ce91b0 100644 --- a/site/content/in-dev/unreleased/quickstart.md +++ b/site/content/in-dev/unreleased/quickstart.md @@ -280,7 +280,7 @@ _Note: the credentials provided here are those for our principal, not the root c ```shell bin/spark-shell \ ---packages org.apache.iceberg:iceberg-spark-runtime-3.5_2.12:1.7.1,org.apache.hadoop:hadoop-aws:3.4.0 \ +--packages org.apache.iceberg:iceberg-spark-runtime-3.5_2.12:1.8.1,org.apache.hadoop:hadoop-aws:3.4.0 \ --conf spark.sql.extensions=org.apache.iceberg.spark.extensions.IcebergSparkSessionExtensions \ --conf spark.sql.catalog.quickstart_catalog.warehouse=quickstart_catalog \ --conf spark.sql.catalog.quickstart_catalog.header.X-Iceberg-Access-Delegation=vended-credentials \ From 9e3f7f68a456084412af5b7db4e3455c87b0c879 Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Thu, 20 Mar 2025 16:34:15 -0400 Subject: [PATCH 12/21] Add comment libs.versions.toml --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 02d6de1902..ad834e45bb 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -19,7 +19,7 @@ [versions] hadoop = "3.4.1" -iceberg = "1.8.1" +iceberg = "1.8.1" # Ensure to update the iceberg version in regtests to keep regtests up-to-date quarkus = "3.19.4" immutables = "2.10.1" picocli = "4.7.6" From 1ceba749a470236fb8a22a1a6ff7d0381cee4706 Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Thu, 20 Mar 2025 16:48:21 -0400 Subject: [PATCH 13/21] Revert changes in IcebergHelper --- .../org/apache/polaris/service/it/env/IcebergHelper.java | 5 ++--- .../service/it/test/PolarisRestCatalogIntegrationTest.java | 1 - .../it/test/PolarisRestCatalogViewIntegrationBase.java | 1 - 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/IcebergHelper.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/IcebergHelper.java index 7478b1ea2c..4d0c987b2b 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/env/IcebergHelper.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/IcebergHelper.java @@ -34,7 +34,6 @@ public static RESTCatalog restCatalog( PolarisApiEndpoints endpoints, PrincipalWithCredentials credentials, String catalog, - String warehouse, Map extraProperties) { String authToken = client.obtainToken(credentials); SessionCatalog.SessionContext context = SessionCatalog.SessionContext.createEmpty(); @@ -54,11 +53,11 @@ public static RESTCatalog restCatalog( .put( org.apache.iceberg.CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO") - .put("warehouse", warehouse) + .put("warehouse", catalog) .put("header." + endpoints.realmHeaderName(), endpoints.realmId()) .putAll(extraProperties); - restCatalog.initialize(catalog, propertiesBuilder.buildKeepingLast()); + restCatalog.initialize("polaris", propertiesBuilder.buildKeepingLast()); return restCatalog; } } diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index 3c75244683..4f624cef8c 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -255,7 +255,6 @@ protected RESTCatalog initCatalog(String catalogName, Map additi client, endpoints, principalCredentials, - catalogName, currentCatalogName, extraPropertiesBuilder.buildKeepingLast()); } diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java index 580ff896b1..929ccc1fc5 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java @@ -123,7 +123,6 @@ public void before(TestInfo testInfo) { endpoints, principalCredentials, catalogName, - catalogName, Map.of( org.apache.iceberg.CatalogProperties.VIEW_DEFAULT_PREFIX + "key1", "catalog-default-key1", From 71d79cc082e27a646363b573d0cdf2a64536856e Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Thu, 20 Mar 2025 20:58:05 -0400 Subject: [PATCH 14/21] Refactor tests to remove allow_overlap config --- .../PolarisRestCatalogIntegrationTest.java | 14 ++-- .../quarkus/catalog/IcebergCatalogTest.java | 64 ++++++++++--------- 2 files changed, 44 insertions(+), 34 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index 4f624cef8c..d2fd6f0213 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -76,7 +76,6 @@ import org.apache.polaris.core.admin.model.ViewGrant; import org.apache.polaris.core.admin.model.ViewPrivilege; import org.apache.polaris.core.config.FeatureConfiguration; -import org.apache.polaris.core.config.PolarisConfiguration; import org.apache.polaris.core.entity.CatalogEntity; import org.apache.polaris.core.entity.PolarisEntityConstants; import org.apache.polaris.service.it.env.CatalogApi; @@ -107,7 +106,7 @@ * @implSpec @implSpec This test expects the server to be configured with the following features * configured: *
    - *
  • {@link PolarisConfiguration#ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING}: {@code false} + *
  • {@link FeatureConfiguration#ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING}: {@code false} *
*/ @ExtendWith(PolarisIntegrationTestExtension.class) @@ -177,8 +176,8 @@ static void close() throws Exception { @BeforeEach public void before(TestInfo testInfo) { this.testInfo = testInfo; - String principalName = "snowman-rest-" + UUID.randomUUID(); - String principalRoleName = "rest-admin-" + UUID.randomUUID(); + String principalName = client.newEntityName("snowman-rest"); + String principalRoleName = client.newEntityName("rest-admin"); principalCredentials = managementApi.createPrincipalWithRole(principalName, principalRoleName); catalogApi = client.catalogApi(principalCredentials); @@ -233,6 +232,13 @@ protected RESTCatalog catalog() { return restCatalog; } + /** + * Initialize a RESTCatalog for testing. + * + * @param catalogName this parameter is currently unused. + * @param additionalProperties additional properties to apply on top of the default test settings + * @return a configured instance of RESTCatalog + */ @Override protected RESTCatalog initCatalog(String catalogName, Map additionalProperties) { Optional restCatalogConfig = diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index 2f9282155a..ad89ac50c1 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -153,8 +153,6 @@ public Map getConfigOverrides() { "true", "polaris.features.defaults.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"", "true", - "polaris.features.defaults.\"ALLOW_OVERLAPPING_CATALOG_URLS\"", - "true", "polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", "[\"FILE\"]"); } @@ -178,14 +176,12 @@ public Map getConfigOverrides() { private IcebergCatalog catalog; private CallContext callContext; - private AwsStorageConfigInfo storageConfigModel; private String realmName; private PolarisMetaStoreManager metaStoreManager; private PolarisCallContext polarisContext; private PolarisAdminService adminService; private PolarisEntityManager entityManager; private FileIOFactory fileIOFactory; - private AuthenticatedPolarisPrincipal authenticatedRoot; private PolarisEntity catalogEntity; private SecurityContext securityContext; @@ -236,7 +232,8 @@ public void before(TestInfo testInfo) { "root") .getEntity())); - authenticatedRoot = new AuthenticatedPolarisPrincipal(rootEntity, Set.of()); + AuthenticatedPolarisPrincipal authenticatedRoot = + new AuthenticatedPolarisPrincipal(rootEntity, Set.of()); securityContext = Mockito.mock(SecurityContext.class); when(securityContext.getUserPrincipal()).thenReturn(authenticatedRoot); @@ -249,6 +246,28 @@ public void before(TestInfo testInfo) { securityContext, new PolarisAuthorizerImpl(new PolarisConfigurationStore() {})); + String storageLocation = "s3://my-bucket/path/to/data"; + AwsStorageConfigInfo storageConfigModel = + AwsStorageConfigInfo.builder() + .setRoleArn("arn:aws:iam::012345678901:role/jdoe") + .setExternalId("externalId") + .setUserArn("aws::a:user:arn") + .setStorageType(StorageConfigInfo.StorageTypeEnum.S3) + .setAllowedLocations(List.of(storageLocation, "s3://externally-owned-bucket")) + .build(); + catalogEntity = + adminService.createCatalog( + new CatalogEntity.Builder() + .setName(CATALOG_NAME) + .setDefaultBaseLocation(storageLocation) + .setReplaceNewLocationPrefixWithCatalogDefault("file:") + .addProperty( + FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "true") + .addProperty( + FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true") + .setStorageConfigurationInfo(storageConfigModel, storageLocation) + .build()); + RealmEntityManagerFactory realmEntityManagerFactory = new RealmEntityManagerFactory(createMockMetaStoreManagerFactory()); this.fileIOFactory = @@ -271,7 +290,7 @@ public void before(TestInfo testInfo) { isA(AwsStorageConfigurationInfo.class))) .thenReturn((PolarisStorageIntegration) storageIntegration); - this.catalog = initCatalog(CATALOG_NAME, ImmutableMap.of()); + this.catalog = initCatalog("my-catalog", ImmutableMap.of()); } @AfterEach @@ -285,33 +304,19 @@ protected IcebergCatalog catalog() { return catalog; } + /** + * Initialize a IcebergCatalog for testing. + * + * @param catalogName this parameter is currently unused. + * @param additionalProperties additional properties to apply on top of the default test settings + * @return a configured instance of IcebergCatalog + */ @Override protected IcebergCatalog initCatalog( String catalogName, Map additionalProperties) { - String storageLocation = "s3://my-bucket/path/to/data"; - storageConfigModel = - AwsStorageConfigInfo.builder() - .setRoleArn("arn:aws:iam::012345678901:role/jdoe") - .setExternalId("externalId") - .setUserArn("aws::a:user:arn") - .setStorageType(StorageConfigInfo.StorageTypeEnum.S3) - .setAllowedLocations(List.of(storageLocation, "s3://externally-owned-bucket")) - .build(); - catalogEntity = - adminService.createCatalog( - new CatalogEntity.Builder() - .setName(catalogName) - .setDefaultBaseLocation(storageLocation) - .setReplaceNewLocationPrefixWithCatalogDefault("file:") - .addProperty( - FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "true") - .addProperty( - FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true") - .setStorageConfigurationInfo(storageConfigModel, storageLocation) - .build()); PolarisPassthroughResolutionView passthroughView = new PolarisPassthroughResolutionView( - callContext, entityManager, securityContext, catalogName); + callContext, entityManager, securityContext, CATALOG_NAME); TaskExecutor taskExecutor = Mockito.mock(); IcebergCatalog icebergCatalog = new IcebergCatalog( @@ -322,12 +327,11 @@ protected IcebergCatalog initCatalog( securityContext, taskExecutor, fileIOFactory); - ImmutableMap.Builder propertiesBuilder = ImmutableMap.builder() .put(CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO") .putAll(additionalProperties); - icebergCatalog.initialize(catalogName, propertiesBuilder.buildKeepingLast()); + icebergCatalog.initialize(CATALOG_NAME, propertiesBuilder.buildKeepingLast()); return icebergCatalog; } From 72e6f32e14e3d7e164a97ca475b2b692acaaac45 Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Sun, 23 Mar 2025 13:17:58 -0400 Subject: [PATCH 15/21] Config AssertJ using `QuarkusTestBeforeClassCallback` --- .../PolarisRestCatalogIntegrationTest.java | 5 --- ...PolarisRestCatalogViewIntegrationBase.java | 5 --- quarkus/service/build.gradle.kts | 3 ++ .../quarkus/catalog/IcebergCatalogTest.java | 5 --- .../catalog/IcebergCatalogViewTest.java | 5 --- ...it.callback.QuarkusTestBeforeClassCallback | 19 +++++++++++ ...PolarisQuarkusTestBeforeClassCallback.java | 32 +++++++++++++++++++ 7 files changed, 54 insertions(+), 20 deletions(-) create mode 100644 quarkus/service/src/test/resources/META-INF/services/io.quarkus.test.junit.callback.QuarkusTestBeforeClassCallback create mode 100644 quarkus/service/src/testFixtures/java/org/apache/polaris/service/quarkus/it/PolarisQuarkusTestBeforeClassCallback.java diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index d2fd6f0213..733a06a7b6 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -87,9 +87,7 @@ import org.apache.polaris.service.it.env.PolarisClient; import org.apache.polaris.service.it.ext.PolarisIntegrationTestExtension; import org.assertj.core.api.Assertions; -import org.assertj.core.api.Assumptions; import org.assertj.core.api.InstanceOfAssertFactories; -import org.assertj.core.configuration.PreferredAssumptionException; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; @@ -156,9 +154,6 @@ String[] properties() default { @BeforeAll static void setup( PolarisApiEndpoints apiEndpoints, ClientCredentials credentials, @TempDir Path tempDir) { - // Set preferredAssumptionException as Quarkus does not suppress JUnit4's - // AssumptionViolatedException - Assumptions.setPreferredAssumptionException(PreferredAssumptionException.JUNIT5); adminCredentials = credentials; endpoints = apiEndpoints; client = polarisClient(endpoints); diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java index 929ccc1fc5..cde52df13b 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java @@ -37,7 +37,6 @@ import org.apache.polaris.service.it.env.PolarisApiEndpoints; import org.apache.polaris.service.it.env.PolarisClient; import org.apache.polaris.service.it.ext.PolarisIntegrationTestExtension; -import org.assertj.core.configuration.PreferredAssumptionException; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assumptions; @@ -66,10 +65,6 @@ public abstract class PolarisRestCatalogViewIntegrationBase extends ViewCatalogT @BeforeAll static void setup(PolarisApiEndpoints apiEndpoints, ClientCredentials credentials) { - // Set preferredAssumptionException as Quarkus does not suppress JUnit4's - // AssumptionViolatedException - org.assertj.core.api.Assumptions.setPreferredAssumptionException( - PreferredAssumptionException.JUNIT5); adminCredentials = credentials; endpoints = apiEndpoints; client = polarisClient(endpoints); diff --git a/quarkus/service/build.gradle.kts b/quarkus/service/build.gradle.kts index 2185c3ca11..345d040e28 100644 --- a/quarkus/service/build.gradle.kts +++ b/quarkus/service/build.gradle.kts @@ -94,6 +94,9 @@ dependencies { exclude(group = "org.apache.spark", module = "spark-sql_2.12") } + testFixturesImplementation(platform(libs.quarkus.bom)) + testFixturesImplementation("io.quarkus:quarkus-junit5") + testImplementation(project(":polaris-api-management-model")) testImplementation(testFixtures(project(":polaris-service-common"))) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index ad89ac50c1..7da024ab7f 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -123,7 +123,6 @@ import org.apache.polaris.service.types.NotificationType; import org.apache.polaris.service.types.TableUpdateNotification; import org.assertj.core.api.Assertions; -import org.assertj.core.configuration.PreferredAssumptionException; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; @@ -190,10 +189,6 @@ public static void setUpMocks() { PolarisStorageIntegrationProviderImpl mock = Mockito.mock(PolarisStorageIntegrationProviderImpl.class); QuarkusMock.installMockForType(mock, PolarisStorageIntegrationProviderImpl.class); - // Set preferredAssumptionException as Quarkus does not suppress JUnit4's - // AssumptionViolatedException - org.assertj.core.api.Assumptions.setPreferredAssumptionException( - PreferredAssumptionException.JUNIT5); } @Nullable diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java index e664537d38..7d817bec4f 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java @@ -65,8 +65,6 @@ import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl; -import org.assertj.core.api.Assumptions; -import org.assertj.core.configuration.PreferredAssumptionException; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; @@ -113,9 +111,6 @@ public static void setUpMocks() { PolarisStorageIntegrationProviderImpl mock = Mockito.mock(PolarisStorageIntegrationProviderImpl.class); QuarkusMock.installMockForType(mock, PolarisStorageIntegrationProviderImpl.class); - // Set preferredAssumptionException as Quarkus does not suppress JUnit4's - // AssumptionViolatedException - Assumptions.setPreferredAssumptionException(PreferredAssumptionException.JUNIT5); } @BeforeEach diff --git a/quarkus/service/src/test/resources/META-INF/services/io.quarkus.test.junit.callback.QuarkusTestBeforeClassCallback b/quarkus/service/src/test/resources/META-INF/services/io.quarkus.test.junit.callback.QuarkusTestBeforeClassCallback new file mode 100644 index 0000000000..ea2419f8ec --- /dev/null +++ b/quarkus/service/src/test/resources/META-INF/services/io.quarkus.test.junit.callback.QuarkusTestBeforeClassCallback @@ -0,0 +1,19 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +org.apache.polaris.service.quarkus.it.PolarisQuarkusTestBeforeClassCallback diff --git a/quarkus/service/src/testFixtures/java/org/apache/polaris/service/quarkus/it/PolarisQuarkusTestBeforeClassCallback.java b/quarkus/service/src/testFixtures/java/org/apache/polaris/service/quarkus/it/PolarisQuarkusTestBeforeClassCallback.java new file mode 100644 index 0000000000..24fdcc9e5d --- /dev/null +++ b/quarkus/service/src/testFixtures/java/org/apache/polaris/service/quarkus/it/PolarisQuarkusTestBeforeClassCallback.java @@ -0,0 +1,32 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.service.quarkus.it; + +import io.quarkus.test.junit.callback.QuarkusTestBeforeClassCallback; +import org.assertj.core.api.Assumptions; +import org.assertj.core.configuration.PreferredAssumptionException; + +public class PolarisQuarkusTestBeforeClassCallback implements QuarkusTestBeforeClassCallback { + @Override + public void beforeClass(Class testClass) { + // Set preferredAssumptionException as Quarkus does not suppress JUnit4's + // AssumptionViolatedException + Assumptions.setPreferredAssumptionException(PreferredAssumptionException.JUNIT5); + } +} From 8030aaaffcf6d1020d6e08eff297324fa7b7646f Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Mon, 24 Mar 2025 19:19:25 -0400 Subject: [PATCH 16/21] Remove `CustomTempDirFactory` and override test --- ...PolarisRestCatalogViewIntegrationBase.java | 47 ++++++++++++++++++- .../it/QuarkusRestCatalogViewFileIT.java | 19 +------- ...kusRestCatalogViewFileIntegrationTest.java | 19 +------- 3 files changed, 48 insertions(+), 37 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java index cde52df13b..a8bef821c9 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java @@ -19,11 +19,17 @@ package org.apache.polaris.service.it.test; import static org.apache.polaris.service.it.env.PolarisClient.polarisClient; +import static org.assertj.core.api.Assertions.assertThat; import java.lang.reflect.Method; +import java.nio.file.Paths; import java.util.Map; +import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.rest.RESTCatalog; +import org.apache.iceberg.view.BaseView; +import org.apache.iceberg.view.View; import org.apache.iceberg.view.ViewCatalogTests; +import org.apache.iceberg.view.ViewProperties; import org.apache.polaris.core.admin.model.Catalog; import org.apache.polaris.core.admin.model.CatalogProperties; import org.apache.polaris.core.admin.model.PolarisCatalog; @@ -42,6 +48,7 @@ import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.extension.ExtendWith; @@ -62,6 +69,7 @@ public abstract class PolarisRestCatalogViewIntegrationBase extends ViewCatalogT private static ManagementApi managementApi; private RESTCatalog restCatalog; + private String defaultBaseLocation; @BeforeAll static void setup(PolarisApiEndpoints apiEndpoints, ClientCredentials credentials) { @@ -89,7 +97,7 @@ public void before(TestInfo testInfo) { String catalogName = client.newEntityName(method.getName()); StorageConfigInfo storageConfig = getStorageConfigInfo(); - String defaultBaseLocation = + defaultBaseLocation = storageConfig.getAllowedLocations().getFirst() + "/" + System.getenv("USER") @@ -165,4 +173,41 @@ protected boolean supportsServerSideRetry() { protected boolean overridesRequestedLocation() { return true; } + + /** + * Overrides the test from the superclass because the tempDir used there is not accessible or + * included in the allowed locations. + */ + @Override + @Test + public void createViewWithCustomMetadataLocation() { + TableIdentifier identifier = TableIdentifier.of("ns", "view"); + + if (requiresNamespaceCreate()) { + catalog().createNamespace(identifier.namespace()); + } + + assertThat(catalog().viewExists(identifier)).as("View should not exist").isFalse(); + + String location = Paths.get(defaultBaseLocation).toString(); + String customLocation = Paths.get(defaultBaseLocation, "custom-location").toString(); + + View view = + catalog() + .buildView(identifier) + .withSchema(SCHEMA) + .withDefaultNamespace(identifier.namespace()) + .withDefaultCatalog(catalog().name()) + .withQuery("spark", "select * from ns.tbl") + .withProperty(ViewProperties.WRITE_METADATA_LOCATION, customLocation) + .withLocation(location) + .create(); + + assertThat(view).isNotNull(); + assertThat(catalog().viewExists(identifier)).as("View should exist").isTrue(); + assertThat(view.properties()).containsEntry("write.metadata.path", customLocation); + assertThat(((BaseView) view).operations().current().metadataFileLocation()) + .isNotNull() + .startsWith(customLocation); + } } diff --git a/quarkus/service/src/intTest/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIT.java b/quarkus/service/src/intTest/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIT.java index b421586e2c..9b68ece256 100644 --- a/quarkus/service/src/intTest/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIT.java +++ b/quarkus/service/src/intTest/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIT.java @@ -20,37 +20,20 @@ import io.quarkus.test.junit.QuarkusIntegrationTest; import java.lang.reflect.Field; -import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import org.apache.iceberg.view.ViewCatalogTests; import org.apache.polaris.service.it.test.PolarisRestCatalogViewFileIntegrationTest; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.extension.AnnotatedElementContext; -import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.api.io.TempDir; -import org.junit.jupiter.api.io.TempDirFactory; @QuarkusIntegrationTest public class QuarkusRestCatalogViewFileIT extends PolarisRestCatalogViewFileIntegrationTest { @BeforeEach - public void setUpTempDir(@TempDir(factory = CustomTempDirFactory.class) Path tempDir) - throws Exception { + public void setUpTempDir(@TempDir Path tempDir) throws Exception { // see https://github.com/quarkusio/quarkus/issues/13261 Field field = ViewCatalogTests.class.getDeclaredField("tempDir"); field.setAccessible(true); field.set(this, tempDir); } - - private static class CustomTempDirFactory implements TempDirFactory { - @Override - public Path createTempDirectory( - AnnotatedElementContext elementContext, ExtensionContext extensionContext) - throws Exception { - Path basePath = Paths.get(BASE_LOCATION.replaceFirst("file://", "")); - Files.createDirectories(basePath); - return Files.createTempDirectory(basePath, null); - } - } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIntegrationTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIntegrationTest.java index 97bd91b68b..4444bcbee3 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIntegrationTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIntegrationTest.java @@ -21,17 +21,12 @@ import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.QuarkusTestProfile; import java.lang.reflect.Field; -import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Map; import org.apache.iceberg.view.ViewCatalogTests; import org.apache.polaris.service.it.test.PolarisRestCatalogViewFileIntegrationTest; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.extension.AnnotatedElementContext; -import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.api.io.TempDir; -import org.junit.jupiter.api.io.TempDirFactory; @QuarkusTest public class QuarkusRestCatalogViewFileIntegrationTest @@ -46,22 +41,10 @@ public Map getConfigOverrides() { } @BeforeEach - public void setUpTempDir(@TempDir(factory = CustomTempDirFactory.class) Path tempDir) - throws Exception { + public void setUpTempDir(@TempDir Path tempDir) throws Exception { // see https://github.com/quarkusio/quarkus/issues/13261 Field field = ViewCatalogTests.class.getDeclaredField("tempDir"); field.setAccessible(true); field.set(this, tempDir); } - - private static class CustomTempDirFactory implements TempDirFactory { - @Override - public Path createTempDirectory( - AnnotatedElementContext elementContext, ExtensionContext extensionContext) - throws Exception { - Path basePath = Paths.get(BASE_LOCATION.replaceFirst("file://", "")); - Files.createDirectories(basePath); - return Files.createTempDirectory(basePath, null); - } - } } From 4c3ce1f32679dee5eda16a658fbed6afac157745 Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Tue, 25 Mar 2025 21:46:16 -0400 Subject: [PATCH 17/21] Disable tests that are not applicable to Polaris --- ...PolarisRestCatalogViewIntegrationBase.java | 48 ++++--------------- .../quarkus/catalog/IcebergCatalogTest.java | 13 +++-- 2 files changed, 17 insertions(+), 44 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java index a8bef821c9..33da334170 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java @@ -19,17 +19,11 @@ package org.apache.polaris.service.it.test; import static org.apache.polaris.service.it.env.PolarisClient.polarisClient; -import static org.assertj.core.api.Assertions.assertThat; import java.lang.reflect.Method; -import java.nio.file.Paths; import java.util.Map; -import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.rest.RESTCatalog; -import org.apache.iceberg.view.BaseView; -import org.apache.iceberg.view.View; import org.apache.iceberg.view.ViewCatalogTests; -import org.apache.iceberg.view.ViewProperties; import org.apache.polaris.core.admin.model.Catalog; import org.apache.polaris.core.admin.model.CatalogProperties; import org.apache.polaris.core.admin.model.PolarisCatalog; @@ -48,6 +42,7 @@ import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.extension.ExtendWith; @@ -69,7 +64,6 @@ public abstract class PolarisRestCatalogViewIntegrationBase extends ViewCatalogT private static ManagementApi managementApi; private RESTCatalog restCatalog; - private String defaultBaseLocation; @BeforeAll static void setup(PolarisApiEndpoints apiEndpoints, ClientCredentials credentials) { @@ -97,7 +91,7 @@ public void before(TestInfo testInfo) { String catalogName = client.newEntityName(method.getName()); StorageConfigInfo storageConfig = getStorageConfigInfo(); - defaultBaseLocation = + String defaultBaseLocation = storageConfig.getAllowedLocations().getFirst() + "/" + System.getenv("USER") @@ -174,40 +168,14 @@ protected boolean overridesRequestedLocation() { return true; } - /** - * Overrides the test from the superclass because the tempDir used there is not accessible or - * included in the allowed locations. - */ @Override @Test + @Disabled( + """ + Disabled because the behavior is not applicable to Polaris. + To unblock, update this to expect an exception and add a Polaris-specific test. + """) public void createViewWithCustomMetadataLocation() { - TableIdentifier identifier = TableIdentifier.of("ns", "view"); - - if (requiresNamespaceCreate()) { - catalog().createNamespace(identifier.namespace()); - } - - assertThat(catalog().viewExists(identifier)).as("View should not exist").isFalse(); - - String location = Paths.get(defaultBaseLocation).toString(); - String customLocation = Paths.get(defaultBaseLocation, "custom-location").toString(); - - View view = - catalog() - .buildView(identifier) - .withSchema(SCHEMA) - .withDefaultNamespace(identifier.namespace()) - .withDefaultCatalog(catalog().name()) - .withQuery("spark", "select * from ns.tbl") - .withProperty(ViewProperties.WRITE_METADATA_LOCATION, customLocation) - .withLocation(location) - .create(); - - assertThat(view).isNotNull(); - assertThat(catalog().viewExists(identifier)).as("View should exist").isTrue(); - assertThat(view.properties()).containsEntry("write.metadata.path", customLocation); - assertThat(((BaseView) view).operations().current().metadataFileLocation()) - .isNotNull() - .startsWith(customLocation); + super.createViewWithCustomMetadataLocation(); } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index 7da024ab7f..86e37caebb 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -127,6 +127,7 @@ import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.params.ParameterizedTest; @@ -385,12 +386,16 @@ public Map purgeRealms(Iterable realms) { }; } - @Test @Override + @Test + @Disabled( + """ + Disabled because the behavior is not applicable to Polaris. + To unblock: + 1) Align Polaris behavior with the superclass by handling empty namespaces the same way, or + 2) Modify this test to expect an exception and add a Polaris-specific version. + """) public void listNamespacesWithEmptyNamespace() { - // TODO: remove this override test once Polaris handles empty namespaces the same as the - // superclass expectation. - Assumptions.assumeTrue(supportsEmptyNamespace()); super.listNamespacesWithEmptyNamespace(); } From a79dadd2778345f1de3652fd434f5c138bb7d0f0 Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Thu, 27 Mar 2025 19:22:19 -0400 Subject: [PATCH 18/21] Add TODOs and link the issues --- .../service/it/test/PolarisRestCatalogViewIntegrationBase.java | 1 + .../polaris/service/quarkus/catalog/IcebergCatalogTest.java | 1 + 2 files changed, 2 insertions(+) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java index 33da334170..8dc48837bf 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java @@ -168,6 +168,7 @@ protected boolean overridesRequestedLocation() { return true; } + /** TODO: Unblock this test, see: https://github.com/apache/polaris/issues/1273 */ @Override @Test @Disabled( diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index 86e37caebb..4a5506fa22 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -386,6 +386,7 @@ public Map purgeRealms(Iterable realms) { }; } + /** TODO: Unblock this test, see: https://github.com/apache/polaris/issues/1272 */ @Override @Test @Disabled( From 6ad74115619172e74077f59962562e82c0b3783d Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Thu, 27 Mar 2025 20:49:26 -0400 Subject: [PATCH 19/21] Extract testInfo in `@BeforeEach` --- .../PolarisRestCatalogIntegrationTest.java | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index 733a06a7b6..b48ad866ae 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -126,7 +126,7 @@ public class PolarisRestCatalogIntegrationTest extends CatalogTests private CatalogApi catalogApi; private RESTCatalog restCatalog; private String currentCatalogName; - private TestInfo testInfo; + private String[] restCatalogConfig; private final String catalogBaseLocation = s3BucketBase + "/" + System.getenv("USER") + "/path/to/data"; @@ -170,7 +170,6 @@ static void close() throws Exception { @BeforeEach public void before(TestInfo testInfo) { - this.testInfo = testInfo; String principalName = client.newEntityName("snowman-rest"); String principalRoleName = client.newEntityName("rest-admin"); principalCredentials = managementApi.createPrincipalWithRole(principalName, principalRoleName); @@ -214,6 +213,13 @@ public void before(TestInfo testInfo) { managementApi.createCatalog(principalRoleName, catalog); + restCatalogConfig = + testInfo + .getTestMethod() + .map(m -> m.getAnnotation(RestCatalogConfig.class)) + .map(RestCatalogConfig::value) + .orElse(new String[0]); + restCatalog = initCatalog(currentCatalogName, ImmutableMap.of()); } @@ -236,21 +242,10 @@ protected RESTCatalog catalog() { */ @Override protected RESTCatalog initCatalog(String catalogName, Map additionalProperties) { - Optional restCatalogConfig = - testInfo - .getTestMethod() - .flatMap( - m -> - Optional.ofNullable( - m.getAnnotation( - PolarisRestCatalogIntegrationTest.RestCatalogConfig.class))); ImmutableMap.Builder extraPropertiesBuilder = ImmutableMap.builder(); - restCatalogConfig.ifPresent( - config -> { - for (int i = 0; i < config.value().length; i += 2) { - extraPropertiesBuilder.put(config.value()[i], config.value()[i + 1]); - } - }); + for (int i = 0; i < restCatalogConfig.length; i += 2) { + extraPropertiesBuilder.put(restCatalogConfig[i], restCatalogConfig[i + 1]); + } extraPropertiesBuilder.putAll(additionalProperties); return IcebergHelper.restCatalog( client, From b4556e8c8c6fdf049159db0fb1ef0d887c5177cb Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Sun, 30 Mar 2025 13:17:12 -0400 Subject: [PATCH 20/21] Exclude JUnit4 dependency for module `polaris-quarkus-service` --- quarkus/service/build.gradle.kts | 8 +++-- ...it.callback.QuarkusTestBeforeClassCallback | 19 ----------- ...PolarisQuarkusTestBeforeClassCallback.java | 32 ------------------- 3 files changed, 5 insertions(+), 54 deletions(-) delete mode 100644 quarkus/service/src/test/resources/META-INF/services/io.quarkus.test.junit.callback.QuarkusTestBeforeClassCallback delete mode 100644 quarkus/service/src/testFixtures/java/org/apache/polaris/service/quarkus/it/PolarisQuarkusTestBeforeClassCallback.java diff --git a/quarkus/service/build.gradle.kts b/quarkus/service/build.gradle.kts index 345d040e28..ebc2c2917e 100644 --- a/quarkus/service/build.gradle.kts +++ b/quarkus/service/build.gradle.kts @@ -23,6 +23,11 @@ plugins { id("polaris-quarkus") } +configurations.all { + // exclude junit4 dependency for this module + exclude(group = "junit", module = "junit") +} + dependencies { implementation(project(":polaris-core")) implementation(project(":polaris-api-management-service")) @@ -94,9 +99,6 @@ dependencies { exclude(group = "org.apache.spark", module = "spark-sql_2.12") } - testFixturesImplementation(platform(libs.quarkus.bom)) - testFixturesImplementation("io.quarkus:quarkus-junit5") - testImplementation(project(":polaris-api-management-model")) testImplementation(testFixtures(project(":polaris-service-common"))) diff --git a/quarkus/service/src/test/resources/META-INF/services/io.quarkus.test.junit.callback.QuarkusTestBeforeClassCallback b/quarkus/service/src/test/resources/META-INF/services/io.quarkus.test.junit.callback.QuarkusTestBeforeClassCallback deleted file mode 100644 index ea2419f8ec..0000000000 --- a/quarkus/service/src/test/resources/META-INF/services/io.quarkus.test.junit.callback.QuarkusTestBeforeClassCallback +++ /dev/null @@ -1,19 +0,0 @@ -# -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -# -org.apache.polaris.service.quarkus.it.PolarisQuarkusTestBeforeClassCallback diff --git a/quarkus/service/src/testFixtures/java/org/apache/polaris/service/quarkus/it/PolarisQuarkusTestBeforeClassCallback.java b/quarkus/service/src/testFixtures/java/org/apache/polaris/service/quarkus/it/PolarisQuarkusTestBeforeClassCallback.java deleted file mode 100644 index 24fdcc9e5d..0000000000 --- a/quarkus/service/src/testFixtures/java/org/apache/polaris/service/quarkus/it/PolarisQuarkusTestBeforeClassCallback.java +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.polaris.service.quarkus.it; - -import io.quarkus.test.junit.callback.QuarkusTestBeforeClassCallback; -import org.assertj.core.api.Assumptions; -import org.assertj.core.configuration.PreferredAssumptionException; - -public class PolarisQuarkusTestBeforeClassCallback implements QuarkusTestBeforeClassCallback { - @Override - public void beforeClass(Class testClass) { - // Set preferredAssumptionException as Quarkus does not suppress JUnit4's - // AssumptionViolatedException - Assumptions.setPreferredAssumptionException(PreferredAssumptionException.JUNIT5); - } -} From e8392fbd483eae67a62c11158c363ec7f8b225e8 Mon Sep 17 00:00:00 2001 From: Liam Bao Date: Mon, 31 Mar 2025 19:56:26 -0400 Subject: [PATCH 21/21] Change `restCatalogConfig` to Map --- .../PolarisRestCatalogIntegrationTest.java | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index b48ad866ae..83c9791b3c 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -126,7 +126,7 @@ public class PolarisRestCatalogIntegrationTest extends CatalogTests private CatalogApi catalogApi; private RESTCatalog restCatalog; private String currentCatalogName; - private String[] restCatalogConfig; + private Map restCatalogConfig; private final String catalogBaseLocation = s3BucketBase + "/" + System.getenv("USER") + "/path/to/data"; @@ -218,7 +218,19 @@ public void before(TestInfo testInfo) { .getTestMethod() .map(m -> m.getAnnotation(RestCatalogConfig.class)) .map(RestCatalogConfig::value) - .orElse(new String[0]); + .map( + values -> { + if (values.length % 2 != 0) { + throw new IllegalArgumentException( + String.format("Missing value for config '%s'", values[values.length - 1])); + } + Map config = new HashMap<>(); + for (int i = 0; i < values.length; i += 2) { + config.put(values[i], values[i + 1]); + } + return config; + }) + .orElse(ImmutableMap.of()); restCatalog = initCatalog(currentCatalogName, ImmutableMap.of()); } @@ -243,9 +255,7 @@ protected RESTCatalog catalog() { @Override protected RESTCatalog initCatalog(String catalogName, Map additionalProperties) { ImmutableMap.Builder extraPropertiesBuilder = ImmutableMap.builder(); - for (int i = 0; i < restCatalogConfig.length; i += 2) { - extraPropertiesBuilder.put(restCatalogConfig[i], restCatalogConfig[i + 1]); - } + extraPropertiesBuilder.putAll(restCatalogConfig); extraPropertiesBuilder.putAll(additionalProperties); return IcebergHelper.restCatalog( client,