Fix createViewWithCustomMetadataLocation tests for cloudTest task

This patch fixes the createViewWithCustomMetadataLocation tests for
cloudTest tasks. The original test was generating temp directories
internally, causing cloudTests to fail with BadRequestException
instead of the expected ForbiddenException.

Changes:
- Switched to Hadoop's Path (Java Path removes slashes, e.g.
  s3://bucket/path -> s3:/bucket/path)
- Made base classes abstract to avoid running them
- Implemented createViewWithCustomMetadataLocation to allow
  passing a custom location

Testing:
- Verified locally

Author: tmater

integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewAwsIntegrationTest.java renamed to integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewAwsIntegrationTestBase.java

@@ -21,12 +21,12 @@
 import com.google.common.base.Strings;
 import java.util.List;
 import java.util.Optional;
-import java.util.stream.Stream;
+import org.apache.hadoop.fs.Path;
 import org.apache.polaris.core.admin.model.AwsStorageConfigInfo;
 import org.apache.polaris.core.admin.model.StorageConfigInfo;
 
 /** Runs PolarisRestCatalogViewIntegrationTest on AWS. */
-public class PolarisRestCatalogViewAwsIntegrationTest
+public abstract class PolarisRestCatalogViewAwsIntegrationTestBase
     extends PolarisRestCatalogViewIntegrationBase {
   public static final String ROLE_ARN =
       Optional.ofNullable(System.getenv("INTEGRATION_TEST_ROLE_ARN")) // Backward compatibility
@@ -38,12 +38,12 @@ protected StorageConfigInfo getStorageConfigInfo() {
     return AwsStorageConfigInfo.builder()
         .setRoleArn(ROLE_ARN)
         .setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
-        .setAllowedLocations(List.of(BASE_LOCATION))
+        .setAllowedLocations(List.of(new Path(BASE_LOCATION, POLARIS_IT_SUBDIR).toString()))
         .build();
   }
 
   @Override
-  protected boolean shouldSkip() {
-    return Stream.of(BASE_LOCATION, ROLE_ARN).anyMatch(Strings::isNullOrEmpty);
+  protected String getCustomMetadataLocationDir() {
+    return new Path(BASE_LOCATION, POLARIS_IT_CUSTOM_SUBDIR).toString();
   }
 }

integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewAzureIntegrationTest.java renamed to integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewAzureIntegrationTestBase.java

@@ -20,12 +20,12 @@
 
 import com.google.common.base.Strings;
 import java.util.List;
-import java.util.stream.Stream;
+import org.apache.hadoop.fs.Path;
 import org.apache.polaris.core.admin.model.AzureStorageConfigInfo;
 import org.apache.polaris.core.admin.model.StorageConfigInfo;
 
 /** Runs PolarisRestCatalogViewIntegrationTest on Azure. */
-public class PolarisRestCatalogViewAzureIntegrationTest
+public abstract class PolarisRestCatalogViewAzureIntegrationTestBase
     extends PolarisRestCatalogViewIntegrationBase {
   public static final String TENANT_ID = System.getenv("INTEGRATION_TEST_AZURE_TENANT_ID");
   public static final String BASE_LOCATION = System.getenv("INTEGRATION_TEST_AZURE_PATH");
@@ -35,12 +35,12 @@ protected StorageConfigInfo getStorageConfigInfo() {
     return AzureStorageConfigInfo.builder()
         .setTenantId(TENANT_ID)
         .setStorageType(StorageConfigInfo.StorageTypeEnum.AZURE)
-        .setAllowedLocations(List.of(BASE_LOCATION))
+        .setAllowedLocations(List.of(new Path(BASE_LOCATION, POLARIS_IT_SUBDIR).toString()))
         .build();
   }
 
   @Override
-  protected boolean shouldSkip() {
-    return Stream.of(BASE_LOCATION, TENANT_ID).anyMatch(Strings::isNullOrEmpty);
+  protected String getCustomMetadataLocationDir() {
+    return new Path(BASE_LOCATION, POLARIS_IT_CUSTOM_SUBDIR).toString();
   }
 }

integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewFileIntegrationTestBase.java

@@ -23,12 +23,14 @@
 import org.apache.polaris.core.admin.model.FileStorageConfigInfo;
 import org.apache.polaris.core.admin.model.StorageConfigInfo;
 import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.io.TempDir;
 
 /** Runs PolarisRestCatalogViewIntegrationTest on the local filesystem. */
 public abstract class PolarisRestCatalogViewFileIntegrationTestBase
     extends PolarisRestCatalogViewIntegrationBase {
   static String baseLocation;
+  private Path tempDirForCustomMetadataLocation;
 
   @BeforeAll
   public static void setUp(@TempDir Path tempDir) {
@@ -39,6 +41,11 @@ public static void setUp(@TempDir Path tempDir) {
     baseLocation = baseUri;
   }
 
+  @BeforeEach
+  public void setUpTempDirForCustomMetadata(@TempDir Path tempDir) {
+    this.tempDirForCustomMetadataLocation = tempDir;
+  }
+
   @Override
   protected StorageConfigInfo getStorageConfigInfo() {
     return FileStorageConfigInfo.builder()
@@ -48,7 +55,7 @@ protected StorageConfigInfo getStorageConfigInfo() {
   }
 
   @Override
-  protected boolean shouldSkip() {
-    return false;
+  protected String getCustomMetadataLocationDir() {
+    return Path.of(tempDirForCustomMetadataLocation.toUri().toString()).toString();
   }
 }

integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewGcpIntegrationTest.java renamed to integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewGcpIntegrationTestBase.java

@@ -20,12 +20,12 @@
 
 import com.google.common.base.Strings;
 import java.util.List;
-import java.util.stream.Stream;
+import org.apache.hadoop.fs.Path;
 import org.apache.polaris.core.admin.model.GcpStorageConfigInfo;
 import org.apache.polaris.core.admin.model.StorageConfigInfo;
 
 /** Runs PolarisRestCatalogViewIntegrationTest on GCP. */
-public class PolarisRestCatalogViewGcpIntegrationTest
+public abstract class PolarisRestCatalogViewGcpIntegrationTestBase
     extends PolarisRestCatalogViewIntegrationBase {
   public static final String SERVICE_ACCOUNT =
       System.getenv("INTEGRATION_TEST_GCS_SERVICE_ACCOUNT");
@@ -36,12 +36,12 @@ protected StorageConfigInfo getStorageConfigInfo() {
     return GcpStorageConfigInfo.builder()
         .setGcsServiceAccount(SERVICE_ACCOUNT)
         .setStorageType(StorageConfigInfo.StorageTypeEnum.GCS)
-        .setAllowedLocations(List.of(BASE_LOCATION))
+        .setAllowedLocations(List.of(new Path(BASE_LOCATION, POLARIS_IT_SUBDIR).toString()))
         .build();
   }
 
   @Override
-  protected boolean shouldSkip() {
-    return Stream.of(BASE_LOCATION, SERVICE_ACCOUNT).anyMatch(Strings::isNullOrEmpty);
+  protected String getCustomMetadataLocationDir() {
+    return new Path(BASE_LOCATION, POLARIS_IT_CUSTOM_SUBDIR).toString();
   }
 }

integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java

@@ -22,14 +22,14 @@
 
 import com.google.common.collect.ImmutableMap;
 import java.lang.reflect.Method;
-import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.Map;
+import org.apache.hadoop.fs.Path;
 import org.apache.iceberg.catalog.TableIdentifier;
 import org.apache.iceberg.exceptions.ForbiddenException;
 import org.apache.iceberg.rest.RESTCatalog;
 import org.apache.iceberg.view.BaseView;
 import org.apache.iceberg.view.View;
+import org.apache.iceberg.view.ViewBuilder;
 import org.apache.iceberg.view.ViewCatalogTests;
 import org.apache.polaris.core.admin.model.Catalog;
 import org.apache.polaris.core.admin.model.CatalogProperties;
@@ -45,6 +45,8 @@
 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.api.AbstractBooleanAssert;
+import org.assertj.core.api.AbstractStringAssert;
 import org.assertj.core.api.Assertions;
 import org.assertj.core.api.Assumptions;
 import org.assertj.core.configuration.PreferredAssumptionException;
@@ -55,7 +57,6 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInfo;
 import org.junit.jupiter.api.extension.ExtendWith;
-import org.junit.jupiter.api.io.TempDir;
 
 /**
  * Import the full core Iceberg catalog tests by hitting the REST service via the RESTCatalog
@@ -86,6 +87,8 @@ public abstract class PolarisRestCatalogViewIntegrationBase extends ViewCatalogT
   private static PolarisApiEndpoints endpoints;
   private static PolarisClient client;
   private static ManagementApi managementApi;
+  protected static final String POLARIS_IT_SUBDIR = "polaris_it";
+  protected static final String POLARIS_IT_CUSTOM_SUBDIR = "polaris_it_custom";
 
   private RESTCatalog restCatalog;
 
@@ -104,8 +107,6 @@ static void close() throws Exception {
 
   @BeforeEach
   public void before(TestInfo testInfo) {
-    Assumptions.assumeThat(shouldSkip()).isFalse();
-
     String principalName = client.newEntityName("snowman-rest");
     String principalRoleName = client.newEntityName("rest-admin");
     PrincipalWithCredentials principalCredentials =
@@ -157,12 +158,6 @@ public void cleanUp() {
    */
   protected abstract StorageConfigInfo getStorageConfigInfo();
 
-  /**
-   * @return Whether the tests should be skipped, for example due to environment variables not being
-   *     specified.
-   */
-  protected abstract boolean shouldSkip();
-
   @Override
   protected RESTCatalog catalog() {
     return restCatalog;
@@ -188,33 +183,84 @@ protected boolean overridesRequestedLocation() {
     return true;
   }
 
-  @Override
+  protected String getCustomMetadataLocationDir() {
+    return "";
+  }
+
   @Test
+  @Override
   public void createViewWithCustomMetadataLocation() {
-    Assertions.assertThatThrownBy(super::createViewWithCustomMetadataLocation)
+    TableIdentifier identifier = TableIdentifier.of("ns", "view");
+    String baseLocation = catalog().properties().get(CatalogEntity.DEFAULT_BASE_LOCATION_KEY);
+    if (this.requiresNamespaceCreate()) {
+      // Use the default baseLocation of the catalog. No "write.metadata.path" set.
+      catalog().createNamespace(identifier.namespace());
+    }
+
+    // Negative test, we cannot create views outside of base location
+    ((AbstractBooleanAssert)
+            Assertions.assertThat(this.catalog().viewExists(identifier))
+                .as("View should not exist", new Object[0]))
+        .isFalse();
+    ViewBuilder viewBuilder =
+        catalog()
+            .buildView(identifier)
+            .withSchema(SCHEMA)
+            .withDefaultNamespace(identifier.namespace())
+            .withDefaultCatalog(catalog().name())
+            .withQuery("spark", "select * from ns.tbl")
+            .withProperty(
+                IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY,
+                getCustomMetadataLocationDir())
+            .withLocation(baseLocation);
+    Assertions.assertThatThrownBy(viewBuilder::create)
         .isInstanceOf(ForbiddenException.class)
         .hasMessageContaining("Forbidden: Invalid locations");
+
+    // Positive, we can create views in the default base location's subdirectory
+    String baseViewLocation = catalog().properties().get(CatalogEntity.DEFAULT_BASE_LOCATION_KEY);
+    String baseCustomWriteMetadataLocation = baseViewLocation + "/custom_location";
+    View view =
+        this.catalog()
+            .buildView(identifier)
+            .withSchema(SCHEMA)
+            .withDefaultNamespace(identifier.namespace())
+            .withDefaultCatalog(this.catalog().name())
+            .withQuery("spark", "select * from ns.tbl")
+            .withProperty("write.metadata.path", baseCustomWriteMetadataLocation)
+            .withLocation(baseViewLocation)
+            .create();
+    Assertions.assertThat(view).isNotNull();
+    ((AbstractBooleanAssert)
+            Assertions.assertThat(this.catalog().viewExists(identifier))
+                .as("View should exist", new Object[0]))
+        .isTrue();
+    Assertions.assertThat(view.properties())
+        .containsEntry("write.metadata.path", baseCustomWriteMetadataLocation);
+    ((AbstractStringAssert)
+            Assertions.assertThat(((BaseView) view).operations().current().metadataFileLocation())
+                .isNotNull())
+        .startsWith(new Path(baseCustomWriteMetadataLocation).toString());
   }
 
   @Test
-  public void createViewWithCustomMetadataLocationUsingPolaris(@TempDir Path tempDir) {
+  public void createViewWithCustomMetadataLocationInheritedFromNamespace() {
     TableIdentifier identifier = TableIdentifier.of("ns", "view");
-
-    String location = Paths.get(tempDir.toUri().toString()).toString();
-    String customLocation = Paths.get(tempDir.toUri().toString(), "custom-location").toString();
-    String customLocation2 = Paths.get(tempDir.toUri().toString(), "custom-location2").toString();
-    String customLocationChild =
-        Paths.get(tempDir.toUri().toString(), "custom-location/child").toString();
-
-    catalog()
-        .createNamespace(
-            identifier.namespace(),
-            ImmutableMap.of(
-                IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY, location));
-
+    String viewBaseLocation = getCustomMetadataLocationDir();
+    String customWriteMetadataLocation = viewBaseLocation + "/custom-location";
+    String customWriteMetadataLocation2 = viewBaseLocation + "/custom-location2";
+    String customWriteMetadataLocationChild = viewBaseLocation + "/custom-location/child";
+    if (this.requiresNamespaceCreate()) {
+      // Views can inherit the namespace's "write.metadata.path" setting in Polaris.
+      catalog()
+          .createNamespace(
+              identifier.namespace(),
+              ImmutableMap.of(
+                  IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY,
+                  viewBaseLocation));
+    }
     Assertions.assertThat(catalog().viewExists(identifier)).as("View should not exist").isFalse();
-
-    // CAN create a view with a custom metadata location `baseLocation/customLocation`,
+    // CAN create a view with a custom metadata location `viewLocation/customLocation`,
     // as long as the location is within the parent namespace's `write.metadata.path=baseLocation`
     View view =
         catalog()
@@ -224,17 +270,17 @@ public void createViewWithCustomMetadataLocationUsingPolaris(@TempDir Path tempD
             .withDefaultCatalog(catalog().name())
             .withQuery("spark", "select * from ns.tbl")
             .withProperty(
-                IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY, customLocation)
-            .withLocation(location)
+                IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY,
+                customWriteMetadataLocation)
+            .withLocation(viewBaseLocation)
             .create();
-
     Assertions.assertThat(view).isNotNull();
     Assertions.assertThat(catalog().viewExists(identifier)).as("View should exist").isTrue();
-    Assertions.assertThat(view.properties()).containsEntry("write.metadata.path", customLocation);
+    Assertions.assertThat(view.properties())
+        .containsEntry("write.metadata.path", customWriteMetadataLocation);
     Assertions.assertThat(((BaseView) view).operations().current().metadataFileLocation())
         .isNotNull()
-        .startsWith(customLocation);
-
+        .startsWith(customWriteMetadataLocation);
     // CANNOT update the view with a new metadata location `baseLocation/customLocation2`,
     // even though the new location is still under the parent namespace's
     // `write.metadata.path=baseLocation`.
@@ -245,11 +291,10 @@ public void createViewWithCustomMetadataLocationUsingPolaris(@TempDir Path tempD
                     .updateProperties()
                     .set(
                         IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY,
-                        customLocation2)
+                        customWriteMetadataLocation2)
                     .commit())
         .isInstanceOf(ForbiddenException.class)
         .hasMessageContaining("Forbidden: Invalid locations");
-
     // CANNOT update the view with a child metadata location `baseLocation/customLocation/child`,
     // even though it is a subpath of the original view's
     // `write.metadata.path=baseLocation/customLocation`.
@@ -260,7 +305,7 @@ public void createViewWithCustomMetadataLocationUsingPolaris(@TempDir Path tempD
                     .updateProperties()
                     .set(
                         IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY,
-                        customLocationChild)
+                        customWriteMetadataLocationChild)
                     .commit())
         .isInstanceOf(ForbiddenException.class)
         .hasMessageContaining("Forbidden: Invalid locations");

runtime/service/src/cloudTest/java/org/apache/polaris/service/it/RestCatalogViewAwsIT.java

@@ -22,14 +22,14 @@
 import java.lang.reflect.Field;
 import java.nio.file.Path;
 import org.apache.iceberg.view.ViewCatalogTests;
-import org.apache.polaris.service.it.test.PolarisRestCatalogViewAwsIntegrationTest;
+import org.apache.polaris.service.it.test.PolarisRestCatalogViewAwsIntegrationTestBase;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable;
 import org.junit.jupiter.api.io.TempDir;
 
 @QuarkusIntegrationTest
 @EnabledIfEnvironmentVariable(named = "INTEGRATION_TEST_S3_PATH", matches = ".+")
-public class RestCatalogViewAwsIT extends PolarisRestCatalogViewAwsIntegrationTest {
+public class RestCatalogViewAwsIT extends PolarisRestCatalogViewAwsIntegrationTestBase {
 
   @BeforeEach
   public void setUpTempDir(@TempDir Path tempDir) throws Exception {

runtime/service/src/cloudTest/java/org/apache/polaris/service/it/RestCatalogViewAzureIT.java

@@ -22,14 +22,14 @@
 import java.lang.reflect.Field;
 import java.nio.file.Path;
 import org.apache.iceberg.view.ViewCatalogTests;
-import org.apache.polaris.service.it.test.PolarisRestCatalogViewAzureIntegrationTest;
+import org.apache.polaris.service.it.test.PolarisRestCatalogViewAzureIntegrationTestBase;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable;
 import org.junit.jupiter.api.io.TempDir;
 
 @QuarkusIntegrationTest
 @EnabledIfEnvironmentVariable(named = "INTEGRATION_TEST_AZURE_PATH", matches = ".+")
-public class RestCatalogViewAzureIT extends PolarisRestCatalogViewAzureIntegrationTest {
+public class RestCatalogViewAzureIT extends PolarisRestCatalogViewAzureIntegrationTestBase {
 
   @BeforeEach
   public void setUpTempDir(@TempDir Path tempDir) throws Exception {

runtime/service/src/cloudTest/java/org/apache/polaris/service/it/RestCatalogViewGcpIT.java

@@ -22,14 +22,14 @@
 import java.lang.reflect.Field;
 import java.nio.file.Path;
 import org.apache.iceberg.view.ViewCatalogTests;
-import org.apache.polaris.service.it.test.PolarisRestCatalogViewGcpIntegrationTest;
+import org.apache.polaris.service.it.test.PolarisRestCatalogViewGcpIntegrationTestBase;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable;
 import org.junit.jupiter.api.io.TempDir;
 
 @QuarkusIntegrationTest
 @EnabledIfEnvironmentVariable(named = "INTEGRATION_TEST_GCS_PATH", matches = ".+")
-public class RestCatalogViewGcpIT extends PolarisRestCatalogViewGcpIntegrationTest {
+public class RestCatalogViewGcpIT extends PolarisRestCatalogViewGcpIntegrationTestBase {
 
   @BeforeEach
   public void setUpTempDir(@TempDir Path tempDir) throws Exception {