adnanhemani commented on code in PR #1665:
URL: https://github.com/apache/polaris/pull/1665#discussion_r2105487194
##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java:
##########
@@ -30,11 +30,7 @@
import java.lang.reflect.Method;
import java.net.URI;
import java.nio.file.Path;
-import java.util.Arrays;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.UUID;
+import java.util.*;
Review Comment:
Let's revert these wildcard imports...
##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java:
##########
@@ -333,6 +325,149 @@ public void testCreatePolicyWithInvalidName(String
policyName) {
}
}
+ @Test
+ public void testCreatePolicyWithNonExistingNamespace() {
+ CreatePolicyRequest request =
+ CreatePolicyRequest.builder()
+ .setType(PredefinedPolicyTypes.DATA_COMPACTION.getName())
+ .setName(currentCatalogName)
+ .setDescription("test policy")
+ .setContent(EXAMPLE_TABLE_MAINTENANCE_POLICY_CONTENT)
+ .build();
+ try (Response res =
+ policyApi
+ .request(
+ "polaris/v1/{cat}/namespaces/{ns}/policies",
+ Map.of("cat", currentCatalogName, "ns", "INVALID_NAMESPACE"))
+ .post(Entity.json(request))) {
+
Assertions.assertThat(res.getStatus()).isEqualTo(Response.Status.NOT_FOUND.getStatusCode());
+ Assertions.assertThat(res.readEntity(String.class))
+ .contains("Namespace does not exist: INVALID_NAMESPACE");
+ }
+ }
+
+ @Test
+ public void testAttachPolicyToNonExistingNamespace() {
+ restCatalog.createNamespace(NS1);
+ String ns = RESTUtil.encodeNamespace(NS1);
+ policyApi.createPolicy(
+ currentCatalogName,
+ NS1_P1,
+ PredefinedPolicyTypes.DATA_COMPACTION,
+ EXAMPLE_TABLE_MAINTENANCE_POLICY_CONTENT,
+ "test policy");
+
+ Namespace invalidNamespace = Namespace.of("INVALID_NAMESPACE");
+ var invalidTarget =
+ new PolicyAttachmentTarget(
+ PolicyAttachmentTarget.TypeEnum.NAMESPACE,
List.of(invalidNamespace.levels()));
+
+ AttachPolicyRequest request =
+
AttachPolicyRequest.builder().setTarget(invalidTarget).setParameters(Map.of()).build();
+ try (Response res =
+ policyApi
+ .request(
+
"polaris/v1/{cat}/namespaces/{ns}/policies/{policy-name}/mappings",
+ Map.of("cat", currentCatalogName, "ns", ns, "policy-name",
NS1_P1.getName()))
+ .put(Entity.json(request))) {
+
Assertions.assertThat(res.getStatus()).isEqualTo(Response.Status.NOT_FOUND.getStatusCode());
+ Assertions.assertThat(res.readEntity(String.class))
+ .contains("Namespace does not exist: INVALID_NAMESPACE");
+ }
+ policyApi.dropPolicy(currentCatalogName, NS1_P1);
+ }
+
+ @Test
+ public void testAttachPolicyToNonExistingTable() {
+ restCatalog.createNamespace(NS1);
+ String ns = RESTUtil.encodeNamespace(NS1);
+
+ policyApi.createPolicy(
+ currentCatalogName,
+ NS1_P1,
+ PredefinedPolicyTypes.DATA_COMPACTION,
+ EXAMPLE_TABLE_MAINTENANCE_POLICY_CONTENT,
+ "test policy");
+ TableIdentifier invalidTable = TableIdentifier.of(NS1, "INVALID_TABLE");
+ var invalidTarget =
+ new PolicyAttachmentTarget(
+ PolicyAttachmentTarget.TypeEnum.TABLE_LIKE,
+ List.of(invalidTable.toString().split("\\.")));
+
+ AttachPolicyRequest request =
+
AttachPolicyRequest.builder().setTarget(invalidTarget).setParameters(Map.of()).build();
+ try (Response res =
+ policyApi
+ .request(
+
"polaris/v1/{cat}/namespaces/{ns}/policies/{policy-name}/mappings",
+ Map.of("cat", currentCatalogName, "ns", ns, "policy-name",
NS1_P1.getName()))
+ .put(Entity.json(request))) {
+
Assertions.assertThat(res.getStatus()).isEqualTo(Response.Status.NOT_FOUND.getStatusCode());
+ Assertions.assertThat(res.readEntity(String.class))
+ .contains("Table or view does not exist: NS1.INVALID_TABLE");
+ }
+ policyApi.dropPolicy(currentCatalogName, NS1_P1);
+ }
+
+ @Test
+ public void testDetachPolicyFromNonExistingNamespace() {
Review Comment:
Maybe not directly regarding this test: But how does this case happen? A
namespace is created, policy is attached to it, and then namespace deleted?
Same for tables below it...
##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java:
##########
@@ -417,6 +589,79 @@ public void testListPolicies() {
policyApi.dropPolicy(currentCatalogName, NS1_P2);
}
+ @Test
+ public void testListPoliciesOnNonExistingNamespace() {
+ try (Response res =
+ policyApi
+ .request(
+ "polaris/v1/{cat}/namespaces/{ns}/policies",
+ Map.of("cat", currentCatalogName, "ns", "INVALID_NAMESPACE"))
+ .get()) {
+
Assertions.assertThat(res.getStatus()).isEqualTo(Response.Status.NOT_FOUND.getStatusCode());
+ Assertions.assertThat(res.readEntity(String.class))
+ .contains("Namespace does not exist: INVALID_NAMESPACE");
+ }
+ }
+
+ @Test
+ public void testGetApplicablePoliciesOnNonExistingNamespace() {
+ Map<String, String> queryParams = new HashMap<>();
+ queryParams.put("namespace", "INVALID_NAMESPACE");
+ try (Response res =
+ policyApi
+ .request(
+ "polaris/v1/{cat}/applicable-policies",
+ Map.of("cat", currentCatalogName),
+ queryParams)
+ .get()) {
+
Assertions.assertThat(res.getStatus()).isEqualTo(Response.Status.NOT_FOUND.getStatusCode());
+ Assertions.assertThat(res.readEntity(String.class))
+ .contains("Namespace does not exist: INVALID_NAMESPACE");
+ }
+ }
+
+ @Test
+ public void testGetApplicablePoliciesOnNonExistingTable() {
+ restCatalog.createNamespace(NS1);
+ policyApi.createPolicy(
+ currentCatalogName,
+ NS1_P1,
+ PredefinedPolicyTypes.DATA_COMPACTION,
+ EXAMPLE_TABLE_MAINTENANCE_POLICY_CONTENT,
+ "test policy");
+ Map<String, String> queryParams = new HashMap<>();
+ queryParams.put("namespace", RESTUtil.encodeNamespace(NS1));
+ queryParams.put("target-name", "INVALID_TABLE");
+ try (Response res =
+ policyApi
+ .request(
+ "polaris/v1/{cat}/applicable-policies",
+ Map.of("cat", currentCatalogName),
+ queryParams)
+ .get()) {
+
Assertions.assertThat(res.getStatus()).isEqualTo(Response.Status.NOT_FOUND.getStatusCode());
+ Assertions.assertThat(res.readEntity(String.class))
+ .contains("Table does not exist: NS1.INVALID_TABLE");
+ }
+ policyApi.dropPolicy(currentCatalogName, NS1_P1);
+ }
+
+ @Test
+ public void testLoadNonExistingPolicy() {
+ restCatalog.createNamespace(NS1);
+ try (Response res =
+ policyApi
+ .request(
+ "polaris/v1/{cat}/namespaces/{ns}/policies/{policy}",
+ Map.of("cat", currentCatalogName, "ns", NS2_T1.name(),
"policy", "INVALID_POLICY"))
+ .get()) {
+
Assertions.assertThat(res.getStatus()).isEqualTo(Response.Status.NOT_FOUND.getStatusCode());
+ Assertions.assertThat(res.readEntity(String.class))
+ .contains(
+ "Policy does not exist: class PolicyIdentifier {\\n
namespace: T1\\n name: INVALID_POLICY\\n}");
Review Comment:
nit: I've been through debugging hell with adding spaces and newlines in
tests for comparisons. I'd personally advocate for checking if all the
fragments (i.e. "Policy does not exist: class PolicyIdentifier", "namespace:
T1", and "name: INVALID_POLICY") are all in the resultant message rather than
comparing the whole string in case the the spacing/indentation changes for any
reasoning.
##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java:
##########
@@ -417,6 +589,79 @@ public void testListPolicies() {
policyApi.dropPolicy(currentCatalogName, NS1_P2);
}
+ @Test
+ public void testListPoliciesOnNonExistingNamespace() {
+ try (Response res =
+ policyApi
+ .request(
+ "polaris/v1/{cat}/namespaces/{ns}/policies",
+ Map.of("cat", currentCatalogName, "ns", "INVALID_NAMESPACE"))
+ .get()) {
+
Assertions.assertThat(res.getStatus()).isEqualTo(Response.Status.NOT_FOUND.getStatusCode());
+ Assertions.assertThat(res.readEntity(String.class))
+ .contains("Namespace does not exist: INVALID_NAMESPACE");
+ }
+ }
+
+ @Test
+ public void testGetApplicablePoliciesOnNonExistingNamespace() {
+ Map<String, String> queryParams = new HashMap<>();
+ queryParams.put("namespace", "INVALID_NAMESPACE");
+ try (Response res =
+ policyApi
+ .request(
+ "polaris/v1/{cat}/applicable-policies",
+ Map.of("cat", currentCatalogName),
+ queryParams)
+ .get()) {
+
Assertions.assertThat(res.getStatus()).isEqualTo(Response.Status.NOT_FOUND.getStatusCode());
+ Assertions.assertThat(res.readEntity(String.class))
+ .contains("Namespace does not exist: INVALID_NAMESPACE");
Review Comment:
Stylistic change: I'd recommend putting `INVALID_NAMESPACE` into a variable
and then substituting it everywhere it's required to keep everything modular
and DRY.
--
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]
