Skip to content

Commit f158f2e

Browse files
authored
Unblock test createViewWithCustomMetadataLocation (#1320)
* Add test for invalid custom metadata location * Add missing properties during table/view creation
1 parent 7f97a95 commit f158f2e

File tree

2 files changed

+84
-8
lines changed

2 files changed

+84
-8
lines changed

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

Lines changed: 83 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,16 @@
2020

2121
import static org.apache.polaris.service.it.env.PolarisClient.polarisClient;
2222

23+
import com.google.common.collect.ImmutableMap;
2324
import java.lang.reflect.Method;
25+
import java.nio.file.Path;
26+
import java.nio.file.Paths;
2427
import java.util.Map;
28+
import org.apache.iceberg.catalog.TableIdentifier;
29+
import org.apache.iceberg.exceptions.ForbiddenException;
2530
import org.apache.iceberg.rest.RESTCatalog;
31+
import org.apache.iceberg.view.BaseView;
32+
import org.apache.iceberg.view.View;
2633
import org.apache.iceberg.view.ViewCatalogTests;
2734
import org.apache.polaris.core.admin.model.Catalog;
2835
import org.apache.polaris.core.admin.model.CatalogProperties;
@@ -31,22 +38,24 @@
3138
import org.apache.polaris.core.admin.model.StorageConfigInfo;
3239
import org.apache.polaris.core.config.FeatureConfiguration;
3340
import org.apache.polaris.core.entity.CatalogEntity;
41+
import org.apache.polaris.core.entity.table.IcebergTableLikeEntity;
3442
import org.apache.polaris.service.it.env.ClientCredentials;
3543
import org.apache.polaris.service.it.env.IcebergHelper;
3644
import org.apache.polaris.service.it.env.ManagementApi;
3745
import org.apache.polaris.service.it.env.PolarisApiEndpoints;
3846
import org.apache.polaris.service.it.env.PolarisClient;
3947
import org.apache.polaris.service.it.ext.PolarisIntegrationTestExtension;
48+
import org.assertj.core.api.Assertions;
4049
import org.assertj.core.api.Assumptions;
4150
import org.assertj.core.configuration.PreferredAssumptionException;
4251
import org.junit.jupiter.api.AfterAll;
4352
import org.junit.jupiter.api.AfterEach;
4453
import org.junit.jupiter.api.BeforeAll;
4554
import org.junit.jupiter.api.BeforeEach;
46-
import org.junit.jupiter.api.Disabled;
4755
import org.junit.jupiter.api.Test;
4856
import org.junit.jupiter.api.TestInfo;
4957
import org.junit.jupiter.api.extension.ExtendWith;
58+
import org.junit.jupiter.api.io.TempDir;
5059

5160
/**
5261
* Import the full core Iceberg catalog tests by hitting the REST service via the RESTCatalog
@@ -175,15 +184,81 @@ protected boolean overridesRequestedLocation() {
175184
return true;
176185
}
177186

178-
/** TODO: Unblock this test, see: https://github.com/apache/polaris/issues/1273 */
179187
@Override
180188
@Test
181-
@Disabled(
182-
"""
183-
Disabled because the behavior is not applicable to Polaris.
184-
To unblock, update this to expect an exception and add a Polaris-specific test.
185-
""")
186189
public void createViewWithCustomMetadataLocation() {
187-
super.createViewWithCustomMetadataLocation();
190+
Assertions.assertThatThrownBy(super::createViewWithCustomMetadataLocation)
191+
.isInstanceOf(ForbiddenException.class)
192+
.hasMessageContaining("Forbidden: Invalid locations");
193+
}
194+
195+
@Test
196+
public void createViewWithCustomMetadataLocationUsingPolaris(@TempDir Path tempDir) {
197+
TableIdentifier identifier = TableIdentifier.of("ns", "view");
198+
199+
String location = Paths.get(tempDir.toUri().toString()).toString();
200+
String customLocation = Paths.get(tempDir.toUri().toString(), "custom-location").toString();
201+
String customLocation2 = Paths.get(tempDir.toUri().toString(), "custom-location2").toString();
202+
String customLocationChild =
203+
Paths.get(tempDir.toUri().toString(), "custom-location/child").toString();
204+
205+
catalog()
206+
.createNamespace(
207+
identifier.namespace(),
208+
ImmutableMap.of(
209+
IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY, location));
210+
211+
Assertions.assertThat(catalog().viewExists(identifier)).as("View should not exist").isFalse();
212+
213+
// CAN create a view with a custom metadata location `baseLocation/customLocation`,
214+
// as long as the location is within the parent namespace's `write.metadata.path=baseLocation`
215+
View view =
216+
catalog()
217+
.buildView(identifier)
218+
.withSchema(SCHEMA)
219+
.withDefaultNamespace(identifier.namespace())
220+
.withDefaultCatalog(catalog().name())
221+
.withQuery("spark", "select * from ns.tbl")
222+
.withProperty(
223+
IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY, customLocation)
224+
.withLocation(location)
225+
.create();
226+
227+
Assertions.assertThat(view).isNotNull();
228+
Assertions.assertThat(catalog().viewExists(identifier)).as("View should exist").isTrue();
229+
Assertions.assertThat(view.properties()).containsEntry("write.metadata.path", customLocation);
230+
Assertions.assertThat(((BaseView) view).operations().current().metadataFileLocation())
231+
.isNotNull()
232+
.startsWith(customLocation);
233+
234+
// CANNOT update the view with a new metadata location `baseLocation/customLocation2`,
235+
// even though the new location is still under the parent namespace's
236+
// `write.metadata.path=baseLocation`.
237+
Assertions.assertThatThrownBy(
238+
() ->
239+
catalog()
240+
.loadView(identifier)
241+
.updateProperties()
242+
.set(
243+
IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY,
244+
customLocation2)
245+
.commit())
246+
.isInstanceOf(ForbiddenException.class)
247+
.hasMessageContaining("Forbidden: Invalid locations");
248+
249+
// CANNOT update the view with a child metadata location `baseLocation/customLocation/child`,
250+
// even though it is a subpath of the original view's
251+
// `write.metadata.path=baseLocation/customLocation`.
252+
Assertions.assertThatThrownBy(
253+
() ->
254+
catalog()
255+
.loadView(identifier)
256+
.updateProperties()
257+
.set(
258+
IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY,
259+
customLocationChild)
260+
.commit())
261+
.isInstanceOf(ForbiddenException.class)
262+
.hasMessageContaining("Forbidden: Invalid locations");
188263
}
189264
}

polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfo.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ public static Optional<PolarisStorageConfigurationInfo> forEntityPath(
181181
"Allowing unstructured table location for entity: {}",
182182
entityPathReversed.get(0).getName());
183183

184+
// TODO: figure out the purpose of adding `userSpecifiedWriteLocations`
184185
List<String> locs =
185186
userSpecifiedWriteLocations(entityPathReversed.get(0).getPropertiesAsMap());
186187
return new StorageConfigurationOverride(

0 commit comments

Comments
 (0)