From f1d53443daae56bc60a69d43a0d2dbeea233d661 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 5 Sep 2024 12:02:55 -0700 Subject: [PATCH 1/9] initial commit --- .../polaris/core/PolarisConfiguration.java | 18 ++++++++++++++++++ .../service/catalog/BasePolarisCatalog.java | 13 +++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java b/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java index 31af621d81..72defee43f 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java @@ -150,4 +150,22 @@ public static Builder builder() { "If set to true, allows tables to have external locations outside the default structure.") .defaultValue(false) .build(); + + public static final PolarisConfiguration CLEANUP_ON_NAMESPACE_DROP = + PolarisConfiguration.builder() + .key("CLEANUP_ON_NAMESPACE_DROP") + .catalogConfig("cleanup.on.namespace.drop") + .description( + "If set to true, clean up data when a namespace is dropped") + .defaultValue(false) + .build(); + + public static final PolarisConfiguration DROP_WITH_PURGE_ENABLED = + PolarisConfiguration.builder() + .key("ALLOW_DROP_WITH_PURGE") + .catalogConfig("dropWithPurge.enabled") + .description( + "If set to true, allows tables to be dropped with the purge parameter set to true.") + .defaultValue(true) + .build(); } diff --git a/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java b/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java index 2538caf509..b5955f7e11 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java @@ -157,7 +157,6 @@ public class BasePolarisCatalog extends BaseMetastoreViewCatalog && !(ex instanceof UnprocessableEntityException) && isStorageProviderRetryableException(ex); }; - public static final String CLEANUP_ON_NAMESPACE_DROP = "CLEANUP_ON_NAMESPACE_DROP"; private final PolarisEntityManager entityManager; private final CallContext callContext; @@ -628,7 +627,7 @@ public boolean dropNamespace(Namespace namespace) throws NamespaceNotEmptyExcept Map.of(), polarisCallContext .getConfigurationStore() - .getConfiguration(polarisCallContext, CLEANUP_ON_NAMESPACE_DROP, false)); + .getConfiguration(polarisCallContext, PolarisConfiguration.CLEANUP_ON_NAMESPACE_DROP)); if (!dropEntityResult.isSuccess() && dropEntityResult.failedBecauseNotEmpty()) { throw new NamespaceNotEmptyException("Namespace %s is not empty", namespace); @@ -1826,6 +1825,16 @@ private void updateTableLike(TableIdentifier identifier, PolarisEntity entity) { List catalogPath = resolvedEntities.getRawParentPath(); PolarisEntity leafEntity = resolvedEntities.getRawLeafEntity(); + + boolean dropWithPurgeEnabled = callContext + .getPolarisCallContext() + .getConfigurationStore() + .getConfiguration( + callContext.getPolarisCallContext(), + (CatalogEntity) catalogPath.getFirst(), + PolarisConfiguration.DROP_WITH_PURGE_ENABLED); + + return entityManager .getMetaStoreManager() .dropEntityIfExists( From ab6c3d368d7d8c48388e5c8fef7fba2c3f5c5d76 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 5 Sep 2024 12:07:06 -0700 Subject: [PATCH 2/9] rework --- .../PolarisMetaStoreManagerImpl.java | 19 +++++++++++++++++++ .../service/catalog/BasePolarisCatalog.java | 9 --------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java index b05129fa3d..1c7cfac3a7 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java @@ -33,8 +33,12 @@ import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; + +import org.apache.iceberg.exceptions.ForbiddenException; import org.apache.polaris.core.PolarisCallContext; +import org.apache.polaris.core.PolarisConfiguration; import org.apache.polaris.core.entity.AsyncTaskType; +import org.apache.polaris.core.entity.CatalogEntity; import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisChangeTrackingVersions; import org.apache.polaris.core.entity.PolarisEntitiesActiveKey; @@ -1502,6 +1506,21 @@ public Map deserializeProperties(PolarisCallContext callCtx, Str } } + // Check that cleanup is enabled, if it is set: + if (catalogPath != null && cleanup) { + boolean dropWithPurgeEnabled = callCtx + .getConfigurationStore() + .getConfiguration( + callCtx, + (CatalogEntity) catalogPath.getFirst(), + PolarisConfiguration.DROP_WITH_PURGE_ENABLED); + if (dropWithPurgeEnabled) { + throw new ForbiddenException("Unable to purge entity: " + entityToDrop + ". To enable this feature, " + + "set the Polaris configuration " + PolarisConfiguration.DROP_WITH_PURGE_ENABLED.key + " or the " + + "catalog configuration " + PolarisConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig()); + } + } + // simply delete that entity. Will be removed from entities_active, added to the // entities_dropped and its version will be changed. this.dropEntity(callCtx, ms, refreshEntityToDrop); diff --git a/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java b/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java index b5955f7e11..32df666b92 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java @@ -1826,15 +1826,6 @@ private void updateTableLike(TableIdentifier identifier, PolarisEntity entity) { List catalogPath = resolvedEntities.getRawParentPath(); PolarisEntity leafEntity = resolvedEntities.getRawLeafEntity(); - boolean dropWithPurgeEnabled = callContext - .getPolarisCallContext() - .getConfigurationStore() - .getConfiguration( - callContext.getPolarisCallContext(), - (CatalogEntity) catalogPath.getFirst(), - PolarisConfiguration.DROP_WITH_PURGE_ENABLED); - - return entityManager .getMetaStoreManager() .dropEntityIfExists( From 04c9b84e7e97ee32409954531f6c1d1e5426583f Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 5 Sep 2024 12:28:02 -0700 Subject: [PATCH 3/9] working on test --- .../admin/PolarisDropWithPurgeTest.java | 181 ++++++++++++++++++ 1 file changed, 181 insertions(+) create mode 100644 polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisDropWithPurgeTest.java diff --git a/polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisDropWithPurgeTest.java b/polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisDropWithPurgeTest.java new file mode 100644 index 0000000000..de53e702ec --- /dev/null +++ b/polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisDropWithPurgeTest.java @@ -0,0 +1,181 @@ +/* + * 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.admin; + +import static org.apache.polaris.service.admin.PolarisAuthzTestBase.SCHEMA; +import static org.apache.polaris.service.context.DefaultContextResolver.REALM_PROPERTY_KEY; + +import io.dropwizard.testing.ConfigOverride; +import io.dropwizard.testing.ResourceHelpers; +import io.dropwizard.testing.junit5.DropwizardAppExtension; +import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; +import jakarta.ws.rs.client.Entity; +import jakarta.ws.rs.client.Invocation; +import jakarta.ws.rs.core.Response; + +import java.util.Arrays; +import java.util.List; +import java.util.UUID; +import java.util.stream.Stream; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.rest.requests.CreateNamespaceRequest; +import org.apache.iceberg.rest.requests.CreateTableRequest; +import org.apache.polaris.core.PolarisConfiguration; +import org.apache.polaris.core.admin.model.Catalog; +import org.apache.polaris.core.admin.model.CatalogProperties; +import org.apache.polaris.core.admin.model.CreateCatalogRequest; +import org.apache.polaris.core.admin.model.FileStorageConfigInfo; +import org.apache.polaris.core.admin.model.StorageConfigInfo; +import org.apache.polaris.service.PolarisApplication; +import org.apache.polaris.service.config.PolarisApplicationConfig; +import org.apache.polaris.service.test.PolarisConnectionExtension; +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +@ExtendWith({DropwizardExtensionsSupport.class, PolarisConnectionExtension.class}) +public class PolarisDropWithPurgeTest { + private static final DropwizardAppExtension BASE_EXT = + new DropwizardAppExtension<>( + PolarisApplication.class, + ResourceHelpers.resourceFilePath("polaris-server-integrationtest.yml"), + // Bind to random port to support parallelism + ConfigOverride.config("server.applicationConnectors[0].port", "0"), + ConfigOverride.config("server.adminConnectors[0].port", "0"), + // Drop with purge disabled at the extension level + ConfigOverride.config("featureConfiguration.DROP_WITH_PURGE_ENABLED", "true")); + + private static PolarisConnectionExtension.PolarisToken adminToken; + private static String userToken; + private static String realm; + private static String catalog; + private static String namespace; + private static final String baseLocation = "file:///tmp/PolarisDropWithPurgeTest"; + + @BeforeEach + public void setup(PolarisConnectionExtension.PolarisToken adminToken) { + userToken = adminToken.token(); + realm = PolarisConnectionExtension.getTestRealm(PolarisServiceImplIntegrationTest.class); + catalog = String.format("catalog_%s", UUID.randomUUID().toString()); + CatalogProperties.Builder propertiesBuilder = + CatalogProperties.builder() + .setDefaultBaseLocation(String.format("%s/%s", baseLocation, catalog)); + StorageConfigInfo config = + FileStorageConfigInfo.builder() + .setStorageType(StorageConfigInfo.StorageTypeEnum.FILE) + .build(); + Catalog catalogObject = + new Catalog( + Catalog.TypeEnum.INTERNAL, + catalog, + propertiesBuilder.build(), + 1725487592064L, + 1725487592064L, + 1, + config); + try (Response response = + request(BASE_EXT, "management/v1/catalogs") + .post(Entity.json(new CreateCatalogRequest(catalogObject)))) { + if (response.getStatus() != Response.Status.CREATED.getStatusCode()) { + throw new IllegalStateException( + "Failed to create catalog: " + response.readEntity(String.class)); + } + } + + namespace = "ns"; + CreateNamespaceRequest createNamespaceRequest = + CreateNamespaceRequest.builder() + .withNamespace(Namespace.of(namespace)) + .build(); + try (Response response = + request(BASE_EXT, String.format("catalog/v1/%s/namespaces", catalog)) + .post(Entity.json(createNamespaceRequest))) { + if (response.getStatus() != Response.Status.OK.getStatusCode()) { + throw new IllegalStateException( + "Failed to create namespace: " + response.readEntity(String.class)); + } + } + } + + private String createTable() { + String name = "table_" + UUID.randomUUID().toString(); + CreateTableRequest createTableRequest = + CreateTableRequest.builder() + .withName(name) + .withSchema(SCHEMA) + .build(); + String prefix = String.format("catalog/v1/%s/namespaces/%s/tables", catalog, namespace); + try (Response response = request(BASE_EXT, prefix).post(Entity.json(createTableRequest))) { + if (response.getStatus() != Response.Status.OK.getStatusCode()) { + throw new IllegalStateException("Failed to create table: " + name); + } + return name; + } + } + + private Response dropTable(String name, boolean purge) { + String prefix = String.format("catalog/v1/%s/namespaces/%s/tables/%s", catalog, namespace, name); + + try (Response response = BASE_EXT.client() + .target(String.format("http://localhost:%d/api/%s", BASE_EXT.getLocalPort(), prefix)) + .queryParam("purgeRequested", purge) // Add purge flag + .request("application/json") + .header("Authorization", "Bearer " + userToken) + .header(REALM_PROPERTY_KEY, realm) + .delete()) { // Send DELETE request + + // Check for success status (204 No Content) + if (response.getStatus() != Response.Status.NO_CONTENT.getStatusCode()) { + throw new IllegalStateException("Failed to drop table: " + name); + } + return response; // Return response for further handling + } + } + + private static Invocation.Builder request( + DropwizardAppExtension extension, + String prefix) { + return extension + .client() + .target(String.format("http://localhost:%d/api/%s", extension.getLocalPort(), prefix)) + .request("application/json") + .header("Authorization", "Bearer " + userToken) + .header(REALM_PROPERTY_KEY, realm); + } + + @Test + void testDropTable() { + + // Drop a table without purge + Assertions + .assertThat(dropTable(createTable(), false)) + .returns(Response.Status.OK.getStatusCode(), Response::getStatus); + + +// assertThat( +// createCatalog( +// prefix, "kingdoms", initiallyExternal, Arrays.asList("plants", "animals"))) +// .returns(Response.Status.BAD_REQUEST.getStatusCode(), Response::getStatus); + + } +} \ No newline at end of file From 7943b2bdca9c14c40761bc5f82a57e0a055efbad Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 5 Sep 2024 12:57:25 -0700 Subject: [PATCH 4/9] about to yank test --- .../polaris/core/PolarisConfiguration.java | 3 +- .../PolarisMetaStoreManagerImpl.java | 25 +- .../service/catalog/BasePolarisCatalog.java | 3 +- .../admin/PolarisDropWithPurgeTest.java | 261 ++++++++++-------- 4 files changed, 159 insertions(+), 133 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java b/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java index 72defee43f..6ddb2bded1 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java @@ -155,8 +155,7 @@ public static Builder builder() { PolarisConfiguration.builder() .key("CLEANUP_ON_NAMESPACE_DROP") .catalogConfig("cleanup.on.namespace.drop") - .description( - "If set to true, clean up data when a namespace is dropped") + .description("If set to true, clean up data when a namespace is dropped") .defaultValue(false) .build(); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java index 1c7cfac3a7..7ee0d65603 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java @@ -33,7 +33,6 @@ import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; - import org.apache.iceberg.exceptions.ForbiddenException; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.PolarisConfiguration; @@ -1434,6 +1433,7 @@ public Map deserializeProperties(PolarisCallContext callCtx, Str * *

{@link #dropEntityIfExists(PolarisCallContext, List, PolarisEntityCore, Map, boolean)} */ + @SuppressWarnings("FormatStringAnnotation") private @NotNull DropEntityResult dropEntityIfExists( @NotNull PolarisCallContext callCtx, @NotNull PolarisMetaStoreSession ms, @@ -1508,16 +1508,21 @@ public Map deserializeProperties(PolarisCallContext callCtx, Str // Check that cleanup is enabled, if it is set: if (catalogPath != null && cleanup) { - boolean dropWithPurgeEnabled = callCtx - .getConfigurationStore() - .getConfiguration( - callCtx, - (CatalogEntity) catalogPath.getFirst(), - PolarisConfiguration.DROP_WITH_PURGE_ENABLED); + boolean dropWithPurgeEnabled = + callCtx + .getConfigurationStore() + .getConfiguration( + callCtx, + (CatalogEntity) catalogPath.get(0), + PolarisConfiguration.DROP_WITH_PURGE_ENABLED); if (dropWithPurgeEnabled) { - throw new ForbiddenException("Unable to purge entity: " + entityToDrop + ". To enable this feature, " + - "set the Polaris configuration " + PolarisConfiguration.DROP_WITH_PURGE_ENABLED.key + " or the " + - "catalog configuration " + PolarisConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig()); + throw new ForbiddenException( + String.format( + "Unable to purge entity: %s. To enable this feature, set the Polaris configuration %s " + + "or the catalog configuration %s", + entityToDrop.getName(), + PolarisConfiguration.DROP_WITH_PURGE_ENABLED.key, + PolarisConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig())); } } diff --git a/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java b/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java index 32df666b92..453d106a6a 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java @@ -627,7 +627,8 @@ public boolean dropNamespace(Namespace namespace) throws NamespaceNotEmptyExcept Map.of(), polarisCallContext .getConfigurationStore() - .getConfiguration(polarisCallContext, PolarisConfiguration.CLEANUP_ON_NAMESPACE_DROP)); + .getConfiguration( + polarisCallContext, PolarisConfiguration.CLEANUP_ON_NAMESPACE_DROP)); if (!dropEntityResult.isSuccess() && dropEntityResult.failedBecauseNotEmpty()) { throw new NamespaceNotEmptyException("Namespace %s is not empty", namespace); diff --git a/polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisDropWithPurgeTest.java b/polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisDropWithPurgeTest.java index de53e702ec..d4fe0793c6 100644 --- a/polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisDropWithPurgeTest.java +++ b/polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisDropWithPurgeTest.java @@ -28,9 +28,7 @@ import jakarta.ws.rs.client.Entity; import jakarta.ws.rs.client.Invocation; import jakarta.ws.rs.core.Response; - -import java.util.Arrays; -import java.util.List; +import java.util.Map; import java.util.UUID; import java.util.stream.Stream; import org.apache.iceberg.catalog.Namespace; @@ -46,136 +44,159 @@ import org.apache.polaris.service.config.PolarisApplicationConfig; import org.apache.polaris.service.test.PolarisConnectionExtension; import org.assertj.core.api.Assertions; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @ExtendWith({DropwizardExtensionsSupport.class, PolarisConnectionExtension.class}) public class PolarisDropWithPurgeTest { - private static final DropwizardAppExtension BASE_EXT = - new DropwizardAppExtension<>( - PolarisApplication.class, - ResourceHelpers.resourceFilePath("polaris-server-integrationtest.yml"), - // Bind to random port to support parallelism - ConfigOverride.config("server.applicationConnectors[0].port", "0"), - ConfigOverride.config("server.adminConnectors[0].port", "0"), - // Drop with purge disabled at the extension level - ConfigOverride.config("featureConfiguration.DROP_WITH_PURGE_ENABLED", "true")); - - private static PolarisConnectionExtension.PolarisToken adminToken; - private static String userToken; - private static String realm; - private static String catalog; - private static String namespace; - private static final String baseLocation = "file:///tmp/PolarisDropWithPurgeTest"; - - @BeforeEach - public void setup(PolarisConnectionExtension.PolarisToken adminToken) { - userToken = adminToken.token(); - realm = PolarisConnectionExtension.getTestRealm(PolarisServiceImplIntegrationTest.class); - catalog = String.format("catalog_%s", UUID.randomUUID().toString()); - CatalogProperties.Builder propertiesBuilder = - CatalogProperties.builder() - .setDefaultBaseLocation(String.format("%s/%s", baseLocation, catalog)); - StorageConfigInfo config = - FileStorageConfigInfo.builder() - .setStorageType(StorageConfigInfo.StorageTypeEnum.FILE) - .build(); - Catalog catalogObject = - new Catalog( - Catalog.TypeEnum.INTERNAL, - catalog, - propertiesBuilder.build(), - 1725487592064L, - 1725487592064L, - 1, - config); - try (Response response = - request(BASE_EXT, "management/v1/catalogs") - .post(Entity.json(new CreateCatalogRequest(catalogObject)))) { - if (response.getStatus() != Response.Status.CREATED.getStatusCode()) { - throw new IllegalStateException( - "Failed to create catalog: " + response.readEntity(String.class)); - } - } - - namespace = "ns"; - CreateNamespaceRequest createNamespaceRequest = - CreateNamespaceRequest.builder() - .withNamespace(Namespace.of(namespace)) - .build(); - try (Response response = - request(BASE_EXT, String.format("catalog/v1/%s/namespaces", catalog)) - .post(Entity.json(createNamespaceRequest))) { - if (response.getStatus() != Response.Status.OK.getStatusCode()) { - throw new IllegalStateException( - "Failed to create namespace: " + response.readEntity(String.class)); - } - } + private static final DropwizardAppExtension BASE_EXT = + new DropwizardAppExtension<>( + PolarisApplication.class, + ResourceHelpers.resourceFilePath("polaris-server-integrationtest.yml"), + // Bind to random port to support parallelism + ConfigOverride.config("server.applicationConnectors[0].port", "0"), + ConfigOverride.config("server.adminConnectors[0].port", "0"), + // Drop with purge disabled at the extension level + ConfigOverride.config("featureConfiguration.DROP_WITH_PURGE_ENABLED", "true")); + + private static PolarisConnectionExtension.PolarisToken adminToken; + private static String userToken; + private static String realm; + private static String defaultCatalog; + private static String strictCatalog; + private static String namespace; + private static final String baseLocation = "file:///tmp/PolarisDropWithPurgeTest"; + + private static Catalog createCatalog(String catalog, Map configs) { + StorageConfigInfo config = + FileStorageConfigInfo.builder() + .setStorageType(StorageConfigInfo.StorageTypeEnum.FILE) + .build(); + CatalogProperties.Builder propertiesBuilder = + CatalogProperties.builder() + .setDefaultBaseLocation(String.format("%s/%s", baseLocation, catalog)); + for (Map.Entry configEntry : configs.entrySet()) { + propertiesBuilder.addProperty(configEntry.getKey(), configEntry.getValue()); } - - private String createTable() { - String name = "table_" + UUID.randomUUID().toString(); - CreateTableRequest createTableRequest = - CreateTableRequest.builder() - .withName(name) - .withSchema(SCHEMA) - .build(); - String prefix = String.format("catalog/v1/%s/namespaces/%s/tables", catalog, namespace); - try (Response response = request(BASE_EXT, prefix).post(Entity.json(createTableRequest))) { - if (response.getStatus() != Response.Status.OK.getStatusCode()) { - throw new IllegalStateException("Failed to create table: " + name); - } - return name; - } + return new Catalog( + Catalog.TypeEnum.INTERNAL, + catalog, + propertiesBuilder.build(), + 1725487592064L, + 1725487592064L, + 1, + config); + } + + private static void setupCatalog(Catalog catalogObject) { + try (Response response = + request("management/v1/catalogs") + .post(Entity.json(new CreateCatalogRequest(catalogObject)))) { + if (response.getStatus() != Response.Status.CREATED.getStatusCode()) { + throw new IllegalStateException( + "Failed to create catalog: " + response.readEntity(String.class)); + } } - private Response dropTable(String name, boolean purge) { - String prefix = String.format("catalog/v1/%s/namespaces/%s/tables/%s", catalog, namespace, name); - - try (Response response = BASE_EXT.client() - .target(String.format("http://localhost:%d/api/%s", BASE_EXT.getLocalPort(), prefix)) - .queryParam("purgeRequested", purge) // Add purge flag - .request("application/json") - .header("Authorization", "Bearer " + userToken) - .header(REALM_PROPERTY_KEY, realm) - .delete()) { // Send DELETE request - - // Check for success status (204 No Content) - if (response.getStatus() != Response.Status.NO_CONTENT.getStatusCode()) { - throw new IllegalStateException("Failed to drop table: " + name); - } - return response; // Return response for further handling - } + namespace = "ns"; + CreateNamespaceRequest createNamespaceRequest = + CreateNamespaceRequest.builder().withNamespace(Namespace.of(namespace)).build(); + try (Response response = + request(String.format("catalog/v1/%s/namespaces", catalogObject.getName())) + .post(Entity.json(createNamespaceRequest))) { + if (response.getStatus() != Response.Status.OK.getStatusCode()) { + throw new IllegalStateException( + "Failed to create namespace: " + response.readEntity(String.class)); + } + } + } + + @BeforeAll + public static void setup(PolarisConnectionExtension.PolarisToken adminToken) { + userToken = adminToken.token(); + realm = PolarisConnectionExtension.getTestRealm(PolarisServiceImplIntegrationTest.class); + defaultCatalog = String.format("default_catalog_%s", UUID.randomUUID().toString()); + strictCatalog = String.format("strict_catalog_%s", UUID.randomUUID().toString()); + + setupCatalog(createCatalog(defaultCatalog, Map.of())); + setupCatalog( + createCatalog( + strictCatalog, + Map.of(PolarisConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig(), "false"))); + } + + private String createTable(String catalog) { + String name = "table_" + UUID.randomUUID().toString(); + CreateTableRequest createTableRequest = + CreateTableRequest.builder().withName(name).withSchema(SCHEMA).build(); + String prefix = String.format("catalog/v1/%s/namespaces/%s/tables", catalog, namespace); + try (Response response = request(prefix).post(Entity.json(createTableRequest))) { + if (response.getStatus() != Response.Status.OK.getStatusCode()) { + throw new IllegalStateException("Failed to create table: " + name); + } + return name; } + } - private static Invocation.Builder request( - DropwizardAppExtension extension, - String prefix) { - return extension + private Response dropTable(String catalog, String name, boolean purge) { + String prefix = + String.format("catalog/v1/%s/namespaces/%s/tables/%s", catalog, namespace, name); + try (Response response = + BASE_EXT .client() - .target(String.format("http://localhost:%d/api/%s", extension.getLocalPort(), prefix)) + .target(String.format("http://localhost:%d/api/%s", BASE_EXT.getLocalPort(), prefix)) + .queryParam("purgeRequested", purge) .request("application/json") .header("Authorization", "Bearer " + userToken) - .header(REALM_PROPERTY_KEY, realm); + .header(REALM_PROPERTY_KEY, realm) + .delete()) { + return response; } - - @Test - void testDropTable() { - - // Drop a table without purge - Assertions - .assertThat(dropTable(createTable(), false)) - .returns(Response.Status.OK.getStatusCode(), Response::getStatus); - - -// assertThat( -// createCatalog( -// prefix, "kingdoms", initiallyExternal, Arrays.asList("plants", "animals"))) -// .returns(Response.Status.BAD_REQUEST.getStatusCode(), Response::getStatus); - + } + + private static Invocation.Builder request(String prefix) { + return BASE_EXT + .client() + .target(String.format("http://localhost:%d/api/%s", BASE_EXT.getLocalPort(), prefix)) + .request("application/json") + .header("Authorization", "Bearer " + userToken) + .header(REALM_PROPERTY_KEY, realm); + } + + /** Used to define a parameterized test config */ + protected record TestConfig(String name, String catalog, Response.Status purgeResult) { + + @Override + public String toString() { + return name; } -} \ No newline at end of file + } + + private static Stream getTestConfigs() { + return Stream.of( + new TestConfig("default catalog", defaultCatalog, Response.Status.OK)); //, + // new TestConfig("strict catalog", strictCatalog, Response.Status.FORBIDDEN)); + } + + @ParameterizedTest + @MethodSource("getTestConfigs") + void testDropTable(TestConfig config) { + + // Drop a table without purge + Assertions.assertThat(dropTable(config.catalog, createTable(config.catalog), false)) + .returns(Response.Status.OK.getStatusCode(), Response::getStatus); + + // Drop a table twice: + String t1 = createTable(config.catalog); + Assertions.assertThat(dropTable(config.catalog, t1, false)) + .returns(Response.Status.OK.getStatusCode(), Response::getStatus); + Assertions.assertThat(dropTable(config.catalog, t1, false)) + .returns(Response.Status.NOT_FOUND.getStatusCode(), Response::getStatus); + + // Drop a table with purge + Assertions.assertThat(dropTable(config.catalog, createTable(config.catalog), true)) + .returns(config.purgeResult.getStatusCode(), Response::getStatus); + } +} From 4279481d8593fda3dfb6930e8c96965cec752f5f Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 5 Sep 2024 12:57:39 -0700 Subject: [PATCH 5/9] yanked test --- .../admin/PolarisDropWithPurgeTest.java | 202 ------------------ 1 file changed, 202 deletions(-) delete mode 100644 polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisDropWithPurgeTest.java diff --git a/polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisDropWithPurgeTest.java b/polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisDropWithPurgeTest.java deleted file mode 100644 index d4fe0793c6..0000000000 --- a/polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisDropWithPurgeTest.java +++ /dev/null @@ -1,202 +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.admin; - -import static org.apache.polaris.service.admin.PolarisAuthzTestBase.SCHEMA; -import static org.apache.polaris.service.context.DefaultContextResolver.REALM_PROPERTY_KEY; - -import io.dropwizard.testing.ConfigOverride; -import io.dropwizard.testing.ResourceHelpers; -import io.dropwizard.testing.junit5.DropwizardAppExtension; -import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; -import jakarta.ws.rs.client.Entity; -import jakarta.ws.rs.client.Invocation; -import jakarta.ws.rs.core.Response; -import java.util.Map; -import java.util.UUID; -import java.util.stream.Stream; -import org.apache.iceberg.catalog.Namespace; -import org.apache.iceberg.rest.requests.CreateNamespaceRequest; -import org.apache.iceberg.rest.requests.CreateTableRequest; -import org.apache.polaris.core.PolarisConfiguration; -import org.apache.polaris.core.admin.model.Catalog; -import org.apache.polaris.core.admin.model.CatalogProperties; -import org.apache.polaris.core.admin.model.CreateCatalogRequest; -import org.apache.polaris.core.admin.model.FileStorageConfigInfo; -import org.apache.polaris.core.admin.model.StorageConfigInfo; -import org.apache.polaris.service.PolarisApplication; -import org.apache.polaris.service.config.PolarisApplicationConfig; -import org.apache.polaris.service.test.PolarisConnectionExtension; -import org.assertj.core.api.Assertions; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.extension.ExtendWith; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.MethodSource; - -@ExtendWith({DropwizardExtensionsSupport.class, PolarisConnectionExtension.class}) -public class PolarisDropWithPurgeTest { - private static final DropwizardAppExtension BASE_EXT = - new DropwizardAppExtension<>( - PolarisApplication.class, - ResourceHelpers.resourceFilePath("polaris-server-integrationtest.yml"), - // Bind to random port to support parallelism - ConfigOverride.config("server.applicationConnectors[0].port", "0"), - ConfigOverride.config("server.adminConnectors[0].port", "0"), - // Drop with purge disabled at the extension level - ConfigOverride.config("featureConfiguration.DROP_WITH_PURGE_ENABLED", "true")); - - private static PolarisConnectionExtension.PolarisToken adminToken; - private static String userToken; - private static String realm; - private static String defaultCatalog; - private static String strictCatalog; - private static String namespace; - private static final String baseLocation = "file:///tmp/PolarisDropWithPurgeTest"; - - private static Catalog createCatalog(String catalog, Map configs) { - StorageConfigInfo config = - FileStorageConfigInfo.builder() - .setStorageType(StorageConfigInfo.StorageTypeEnum.FILE) - .build(); - CatalogProperties.Builder propertiesBuilder = - CatalogProperties.builder() - .setDefaultBaseLocation(String.format("%s/%s", baseLocation, catalog)); - for (Map.Entry configEntry : configs.entrySet()) { - propertiesBuilder.addProperty(configEntry.getKey(), configEntry.getValue()); - } - return new Catalog( - Catalog.TypeEnum.INTERNAL, - catalog, - propertiesBuilder.build(), - 1725487592064L, - 1725487592064L, - 1, - config); - } - - private static void setupCatalog(Catalog catalogObject) { - try (Response response = - request("management/v1/catalogs") - .post(Entity.json(new CreateCatalogRequest(catalogObject)))) { - if (response.getStatus() != Response.Status.CREATED.getStatusCode()) { - throw new IllegalStateException( - "Failed to create catalog: " + response.readEntity(String.class)); - } - } - - namespace = "ns"; - CreateNamespaceRequest createNamespaceRequest = - CreateNamespaceRequest.builder().withNamespace(Namespace.of(namespace)).build(); - try (Response response = - request(String.format("catalog/v1/%s/namespaces", catalogObject.getName())) - .post(Entity.json(createNamespaceRequest))) { - if (response.getStatus() != Response.Status.OK.getStatusCode()) { - throw new IllegalStateException( - "Failed to create namespace: " + response.readEntity(String.class)); - } - } - } - - @BeforeAll - public static void setup(PolarisConnectionExtension.PolarisToken adminToken) { - userToken = adminToken.token(); - realm = PolarisConnectionExtension.getTestRealm(PolarisServiceImplIntegrationTest.class); - defaultCatalog = String.format("default_catalog_%s", UUID.randomUUID().toString()); - strictCatalog = String.format("strict_catalog_%s", UUID.randomUUID().toString()); - - setupCatalog(createCatalog(defaultCatalog, Map.of())); - setupCatalog( - createCatalog( - strictCatalog, - Map.of(PolarisConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig(), "false"))); - } - - private String createTable(String catalog) { - String name = "table_" + UUID.randomUUID().toString(); - CreateTableRequest createTableRequest = - CreateTableRequest.builder().withName(name).withSchema(SCHEMA).build(); - String prefix = String.format("catalog/v1/%s/namespaces/%s/tables", catalog, namespace); - try (Response response = request(prefix).post(Entity.json(createTableRequest))) { - if (response.getStatus() != Response.Status.OK.getStatusCode()) { - throw new IllegalStateException("Failed to create table: " + name); - } - return name; - } - } - - private Response dropTable(String catalog, String name, boolean purge) { - String prefix = - String.format("catalog/v1/%s/namespaces/%s/tables/%s", catalog, namespace, name); - try (Response response = - BASE_EXT - .client() - .target(String.format("http://localhost:%d/api/%s", BASE_EXT.getLocalPort(), prefix)) - .queryParam("purgeRequested", purge) - .request("application/json") - .header("Authorization", "Bearer " + userToken) - .header(REALM_PROPERTY_KEY, realm) - .delete()) { - return response; - } - } - - private static Invocation.Builder request(String prefix) { - return BASE_EXT - .client() - .target(String.format("http://localhost:%d/api/%s", BASE_EXT.getLocalPort(), prefix)) - .request("application/json") - .header("Authorization", "Bearer " + userToken) - .header(REALM_PROPERTY_KEY, realm); - } - - /** Used to define a parameterized test config */ - protected record TestConfig(String name, String catalog, Response.Status purgeResult) { - - @Override - public String toString() { - return name; - } - } - - private static Stream getTestConfigs() { - return Stream.of( - new TestConfig("default catalog", defaultCatalog, Response.Status.OK)); //, - // new TestConfig("strict catalog", strictCatalog, Response.Status.FORBIDDEN)); - } - - @ParameterizedTest - @MethodSource("getTestConfigs") - void testDropTable(TestConfig config) { - - // Drop a table without purge - Assertions.assertThat(dropTable(config.catalog, createTable(config.catalog), false)) - .returns(Response.Status.OK.getStatusCode(), Response::getStatus); - - // Drop a table twice: - String t1 = createTable(config.catalog); - Assertions.assertThat(dropTable(config.catalog, t1, false)) - .returns(Response.Status.OK.getStatusCode(), Response::getStatus); - Assertions.assertThat(dropTable(config.catalog, t1, false)) - .returns(Response.Status.NOT_FOUND.getStatusCode(), Response::getStatus); - - // Drop a table with purge - Assertions.assertThat(dropTable(config.catalog, createTable(config.catalog), true)) - .returns(config.purgeResult.getStatusCode(), Response::getStatus); - } -} From 333725ab21aaaaced9193a7577c039f6a8fe9693 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 5 Sep 2024 14:05:01 -0700 Subject: [PATCH 6/9] changes per review --- .../org/apache/polaris/core/PolarisConfiguration.java | 10 +++++++++- .../core/persistence/PolarisMetaStoreManagerImpl.java | 2 +- .../polaris/service/admin/PolarisAdminService.java | 3 +-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java b/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java index 6ddb2bded1..8237c662e5 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java @@ -159,10 +159,18 @@ public static Builder builder() { .defaultValue(false) .build(); + public static final PolarisConfiguration CLEANUP_ON_CATALOG_DROP = + PolarisConfiguration.builder() + .key("CLEANUP_ON_CATALOG_DROP") + .catalogConfig("cleanup.on.catalog.drop") + .description("If set to true, clean up data when a catalog is dropped") + .defaultValue(false) + .build(); + public static final PolarisConfiguration DROP_WITH_PURGE_ENABLED = PolarisConfiguration.builder() .key("ALLOW_DROP_WITH_PURGE") - .catalogConfig("dropWithPurge.enabled") + .catalogConfig("drop-with-purge.enabled") .description( "If set to true, allows tables to be dropped with the purge parameter set to true.") .defaultValue(true) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java index 7ee0d65603..17343cad43 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java @@ -1515,7 +1515,7 @@ public Map deserializeProperties(PolarisCallContext callCtx, Str callCtx, (CatalogEntity) catalogPath.get(0), PolarisConfiguration.DROP_WITH_PURGE_ENABLED); - if (dropWithPurgeEnabled) { + if (!dropWithPurgeEnabled) { throw new ForbiddenException( String.format( "Unable to purge entity: %s. To enable this feature, set the Polaris configuration %s " diff --git a/polaris-service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/polaris-service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index ebe7d4df63..023c96f13c 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -97,7 +97,6 @@ */ public class PolarisAdminService { private static final Logger LOGGER = LoggerFactory.getLogger(PolarisAdminService.class); - public static final String CLEANUP_ON_CATALOG_DROP = "CLEANUP_ON_CATALOG_DROP"; private final CallContext callContext; private final PolarisEntityManager entityManager; @@ -578,7 +577,7 @@ public void deleteCatalog(String name) { boolean cleanup = polarisCallContext .getConfigurationStore() - .getConfiguration(polarisCallContext, CLEANUP_ON_CATALOG_DROP, false); + .getConfiguration(polarisCallContext, PolarisConfiguration.CLEANUP_ON_CATALOG_DROP); PolarisMetaStoreManager.DropEntityResult dropEntityResult = entityManager .getMetaStoreManager() From 2dde4f43a466a1ed4897fa02fbd033ba726fb4cf Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 5 Sep 2024 14:15:43 -0700 Subject: [PATCH 7/9] refactor --- .../PolarisMetaStoreManagerImpl.java | 23 ------------------- .../service/catalog/BasePolarisCatalog.java | 20 ++++++++++++++++ 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java index 17343cad43..7f28eca801 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java @@ -33,11 +33,8 @@ import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; -import org.apache.iceberg.exceptions.ForbiddenException; import org.apache.polaris.core.PolarisCallContext; -import org.apache.polaris.core.PolarisConfiguration; import org.apache.polaris.core.entity.AsyncTaskType; -import org.apache.polaris.core.entity.CatalogEntity; import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisChangeTrackingVersions; import org.apache.polaris.core.entity.PolarisEntitiesActiveKey; @@ -1506,26 +1503,6 @@ public Map deserializeProperties(PolarisCallContext callCtx, Str } } - // Check that cleanup is enabled, if it is set: - if (catalogPath != null && cleanup) { - boolean dropWithPurgeEnabled = - callCtx - .getConfigurationStore() - .getConfiguration( - callCtx, - (CatalogEntity) catalogPath.get(0), - PolarisConfiguration.DROP_WITH_PURGE_ENABLED); - if (!dropWithPurgeEnabled) { - throw new ForbiddenException( - String.format( - "Unable to purge entity: %s. To enable this feature, set the Polaris configuration %s " - + "or the catalog configuration %s", - entityToDrop.getName(), - PolarisConfiguration.DROP_WITH_PURGE_ENABLED.key, - PolarisConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig())); - } - } - // simply delete that entity. Will be removed from entities_active, added to the // entities_dropped and its version will be changed. this.dropEntity(callCtx, ms, refreshEntityToDrop); diff --git a/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java b/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java index 453d106a6a..feb02b50a1 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java @@ -1827,6 +1827,26 @@ private void updateTableLike(TableIdentifier identifier, PolarisEntity entity) { List catalogPath = resolvedEntities.getRawParentPath(); PolarisEntity leafEntity = resolvedEntities.getRawLeafEntity(); + // Check that purge is enabled, if it is set: + if (catalogPath != null && !catalogPath.isEmpty() && purge) { + boolean dropWithPurgeEnabled = + callContext.getPolarisCallContext() + .getConfigurationStore() + .getConfiguration( + callContext.getPolarisCallContext(), + (CatalogEntity) catalogPath.getFirst(), + PolarisConfiguration.DROP_WITH_PURGE_ENABLED); + if (!dropWithPurgeEnabled) { + throw new ForbiddenException( + String.format( + "Unable to purge entity: %s. To enable this feature, set the Polaris configuration %s " + + "or the catalog configuration %s", + identifier.name(), + PolarisConfiguration.DROP_WITH_PURGE_ENABLED.key, + PolarisConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig())); + } + } + return entityManager .getMetaStoreManager() .dropEntityIfExists( From e2baf566850065466e4296eb77e7b746a2971533 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 5 Sep 2024 14:16:46 -0700 Subject: [PATCH 8/9] revert --- .../polaris/core/persistence/PolarisMetaStoreManagerImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java index 7f28eca801..b05129fa3d 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java @@ -1430,7 +1430,6 @@ public Map deserializeProperties(PolarisCallContext callCtx, Str * *

{@link #dropEntityIfExists(PolarisCallContext, List, PolarisEntityCore, Map, boolean)} */ - @SuppressWarnings("FormatStringAnnotation") private @NotNull DropEntityResult dropEntityIfExists( @NotNull PolarisCallContext callCtx, @NotNull PolarisMetaStoreSession ms, From b4e3dfbc08a166db78e2f6c2a925b51d5fb006a1 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 5 Sep 2024 18:59:42 -0700 Subject: [PATCH 9/9] updateS --- .../polaris/core/PolarisConfiguration.java | 2 +- .../service/catalog/BasePolarisCatalog.java | 6 +- .../catalog/BasePolarisCatalogTest.java | 61 +++++++++++++++++++ 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java b/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java index 8237c662e5..ee101997d2 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java @@ -169,7 +169,7 @@ public static Builder builder() { public static final PolarisConfiguration DROP_WITH_PURGE_ENABLED = PolarisConfiguration.builder() - .key("ALLOW_DROP_WITH_PURGE") + .key("DROP_WITH_PURGE_ENABLED") .catalogConfig("drop-with-purge.enabled") .description( "If set to true, allows tables to be dropped with the purge parameter set to true.") diff --git a/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java b/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java index feb02b50a1..9a09fe5693 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java @@ -1811,6 +1811,7 @@ private void updateTableLike(TableIdentifier identifier, PolarisEntity entity) { } } + @SuppressWarnings("FormatStringAnnotation") private @NotNull PolarisMetaStoreManager.DropEntityResult dropTableLike( PolarisEntitySubType subType, TableIdentifier identifier, @@ -1830,11 +1831,12 @@ private void updateTableLike(TableIdentifier identifier, PolarisEntity entity) { // Check that purge is enabled, if it is set: if (catalogPath != null && !catalogPath.isEmpty() && purge) { boolean dropWithPurgeEnabled = - callContext.getPolarisCallContext() + callContext + .getPolarisCallContext() .getConfigurationStore() .getConfiguration( callContext.getPolarisCallContext(), - (CatalogEntity) catalogPath.getFirst(), + catalogEntity, PolarisConfiguration.DROP_WITH_PURGE_ENABLED); if (!dropWithPurgeEnabled) { throw new ForbiddenException( diff --git a/polaris-service/src/test/java/org/apache/polaris/service/catalog/BasePolarisCatalogTest.java b/polaris-service/src/test/java/org/apache/polaris/service/catalog/BasePolarisCatalogTest.java index c574e5f429..b9f1fa8f95 100644 --- a/polaris-service/src/test/java/org/apache/polaris/service/catalog/BasePolarisCatalogTest.java +++ b/polaris-service/src/test/java/org/apache/polaris/service/catalog/BasePolarisCatalogTest.java @@ -1146,6 +1146,67 @@ public void testDropTableWithPurge() { Assertions.assertThat(fileIO).isNotNull().isInstanceOf(InMemoryFileIO.class); } + @Test + public void testDropTableWithPurgeDisabled() { + // Create a catalog with purge disabled: + String noPurgeCatalogName = CATALOG_NAME + "_no_purge"; + String storageLocation = "s3://testDropTableWithPurgeDisabled/data"; + AwsStorageConfigInfo noPurgeStorageConfigModel = + AwsStorageConfigInfo.builder() + .setRoleArn("arn:aws:iam::012345678901:role/jdoe") + .setExternalId("externalId") + .setUserArn("aws::a:user:arn") + .setStorageType(StorageConfigInfo.StorageTypeEnum.S3) + .build(); + adminService.createCatalog( + new CatalogEntity.Builder() + .setName(noPurgeCatalogName) + .setDefaultBaseLocation(storageLocation) + .setReplaceNewLocationPrefixWithCatalogDefault("file:") + .addProperty(PolarisConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "true") + .addProperty( + PolarisConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true") + .addProperty(PolarisConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig(), "false") + .setStorageConfigurationInfo(noPurgeStorageConfigModel, storageLocation) + .build()); + RealmContext realmContext = () -> "realm"; + CallContext callContext = CallContext.of(realmContext, polarisContext); + PolarisPassthroughResolutionView passthroughView = + new PolarisPassthroughResolutionView( + callContext, entityManager, authenticatedRoot, noPurgeCatalogName); + BasePolarisCatalog noPurgeCatalog = + new BasePolarisCatalog( + entityManager, + callContext, + passthroughView, + authenticatedRoot, + Mockito.mock(), + new DefaultFileIOFactory()); + noPurgeCatalog.initialize( + noPurgeCatalogName, + ImmutableMap.of( + CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")); + + if (this.requiresNamespaceCreate()) { + ((SupportsNamespaces) noPurgeCatalog).createNamespace(NS); + } + + Assertions.assertThatPredicate(noPurgeCatalog::tableExists) + .as("Table should not exist before create") + .rejects(TABLE); + + Table table = noPurgeCatalog.buildTable(TABLE, SCHEMA).create(); + Assertions.assertThatPredicate(noPurgeCatalog::tableExists) + .as("Table should exist after create") + .accepts(TABLE); + Assertions.assertThat(table).isInstanceOf(BaseTable.class); + + // Attempt to drop the table: + Assertions.assertThatThrownBy(() -> noPurgeCatalog.dropTable(TABLE, true)) + .isInstanceOf(ForbiddenException.class) + .hasMessageContaining(PolarisConfiguration.DROP_WITH_PURGE_ENABLED.key); + } + private TableMetadata createSampleTableMetadata(String tableLocation) { Schema schema = new Schema(