dennishuo commented on code in PR #321:
URL: https://github.com/apache/polaris/pull/321#discussion_r1777745434
##########
polaris-service/src/test/java/org/apache/polaris/service/catalog/BasePolarisCatalogTest.java:
##########
@@ -360,6 +360,141 @@ public void testCreateNestedNamespaceUnderMissingParent()
{
.hasMessageContaining("Parent");
}
+ @Test
+ public void testValidateNotificationWhenTableAndNamespacesDontExist() {
+ Assumptions.assumeTrue(
+ requiresNamespaceCreate(),
+ "Only applicable if namespaces must be created before adding
children");
+ Assumptions.assumeTrue(
+ supportsNestedNamespaces(), "Only applicable if nested namespaces are
supported");
+ Assumptions.assumeTrue(
+ supportsNotifications(), "Only applicable if notifications are
supported");
+
+ final String tableLocation =
"s3://externally-owned-bucket/validate_table/";
+ final String tableMetadataLocation = tableLocation +
"metadata/v1.metadata.json";
+ BasePolarisCatalog catalog = catalog();
+
+ Namespace namespace = Namespace.of("parent", "child1");
+ TableIdentifier table = TableIdentifier.of(namespace, "table");
+
+ // For a VALIDATE request we can pass in a full metadata JSON filename or
just the table's
+ // metadata directory; either way the path will be validated to be under
the allowed locations,
+ // but any actual metadata JSON file will not be accessed.
+ NotificationRequest request = new NotificationRequest();
+ request.setNotificationType(NotificationType.VALIDATE);
+ TableUpdateNotification update = new TableUpdateNotification();
+ update.setMetadataLocation(tableMetadataLocation);
+ update.setTableName(table.name());
+ update.setTableUuid(UUID.randomUUID().toString());
+ update.setTimestamp(230950845L);
+ request.setPayload(update);
+
+ // We should be able to send the notification without creating the
metadata file since it's
+ // only validating the ability to send the CREATE/UPDATE notification
possibly before actually
+ // creating the table at all on the remote catalog.
+ Assertions.assertThat(catalog.sendNotification(table, request))
+ .as("Notification should be sent successfully")
+ .isTrue();
+ Assertions.assertThat(catalog.namespaceExists(namespace))
+ .as("Intermediate namespaces should not be created")
+ .isFalse();
+ Assertions.assertThat(catalog.tableExists(table))
+ .as("Table should not be created for a VALIDATE notification")
+ .isFalse();
+
+ // Now also check that despite creating the metadata file, the validation
call still doesn't
+ // create any namespaces or tables.
+ InMemoryFileIO fileIO = (InMemoryFileIO) catalog.getIo();
+ fileIO.addFile(
+ tableMetadataLocation,
+
TableMetadataParser.toJson(createSampleTableMetadata(tableLocation)).getBytes(UTF_8));
+
+ Assertions.assertThat(catalog.sendNotification(table, request))
+ .as("Notification should be sent successfully")
+ .isTrue();
+ Assertions.assertThat(catalog.namespaceExists(namespace))
+ .as("Intermediate namespaces should not be created")
+ .isFalse();
+ Assertions.assertThat(catalog.tableExists(table))
+ .as("Table should not be created for a VALIDATE notification")
+ .isFalse();
+ }
+
+ @Test
+ public void testValidateNotificationInDisallowedLocation() {
+ Assumptions.assumeTrue(
+ requiresNamespaceCreate(),
+ "Only applicable if namespaces must be created before adding
children");
+ Assumptions.assumeTrue(
+ supportsNestedNamespaces(), "Only applicable if nested namespaces are
supported");
+ Assumptions.assumeTrue(
+ supportsNotifications(), "Only applicable if notifications are
supported");
+
+ // The location of the metadata JSON file specified in the create will be
forbidden.
+ // For a VALIDATE call we can pass in the metadata/ prefix itself instead
of a metadata JSON
+ // filename.
+ final String tableLocation = "s3://forbidden-table-location/table/";
+ final String tableMetadataLocation = tableLocation + "metadata/";
+ BasePolarisCatalog catalog = catalog();
+
+ Namespace namespace = Namespace.of("parent", "child1");
+ TableIdentifier table = TableIdentifier.of(namespace, "table");
+
+ NotificationRequest request = new NotificationRequest();
+ request.setNotificationType(NotificationType.VALIDATE);
+ TableUpdateNotification update = new TableUpdateNotification();
+ update.setMetadataLocation(tableMetadataLocation);
+ update.setTableName(table.name());
+ update.setTableUuid(UUID.randomUUID().toString());
+ update.setTimestamp(230950845L);
+ request.setPayload(update);
+
+ Assertions.assertThatThrownBy(() -> catalog.sendNotification(table,
request))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("Invalid location");
+ }
+
+ @Test
+ public void testValidateNotificationFailToCreateFileIO() {
+ Assumptions.assumeTrue(
+ requiresNamespaceCreate(),
+ "Only applicable if namespaces must be created before adding
children");
+ Assumptions.assumeTrue(
+ supportsNestedNamespaces(), "Only applicable if nested namespaces are
supported");
+ Assumptions.assumeTrue(
+ supportsNotifications(), "Only applicable if notifications are
supported");
+
+ // The location of the metadata JSON file specified in the create will be
forbidden.
+ // For a VALIDATE call we can pass in the metadata/ prefix itself instead
of a metadata JSON
+ // filename.
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
