This is an automated email from the ASF dual-hosted git repository.
mchades pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 90eca33160 [#14428] fix(policy): Accept known policyType
case-insensitively (#14520)
90eca33160 is described below
commit 90eca33160034d4581a838deb72a2ec5ee20914b
Author: MaSai <[email protected]>
AuthorDate: Wed Nov 4 16:56:35 2026 +0800
[#14428] fix(policy): Accept known policyType case-insensitively (#14520)
### What changes were proposed in this pull request?
- Resolve known `policyType` values (built-ins and `custom`)
case-insensitively via a scoped `PolicyContentTypeIdResolver`, then bind
and validate the matching content. Explicitly register `custom` and use
`defaultImpl = Void.class` so unknown types do not fall back to custom
content.
- Keep Jackson's default request-field binding (no whole-request
deserializers). Missing `policyType` binds content as null and
`validate()` returns a type-named 400; unknown types likewise get a
type-named 400 such as `Unknown built-in policy type:
system_iceberg_bogus`.
- Persist and return the canonical lowercase `BuiltInType.policyType()`
on create and `updateContent`.
- Align OpenAPI request schemas with case-insensitive acceptance
(patterns and request examples for lowercase, uppercase, and
mixed-case), while response schemas keep the canonical lowercase enum.
### Why are the changes needed?
Fixes #14428. Clients sending a typo such as `system_iceberg_bogus`, or
a known type in another case such as `SYSTEM_ICEBERG_COMPACTION` /
`System_Iceberg_Compaction`, previously got a misleading custom-content
error (`supportedObjectTypes cannot be empty` or `Malformed json
request`) instead of a clear type-named 400. `fromPolicyType()` already
matched ignore-case; Jackson subtype selection and the OpenAPI request
schemas did not.
### Does this PR introduce _any_ user-facing change?
Yes.
- Known `policyType` values are accepted case-insensitively on
create/update; the server stores and returns the canonical lowercase
form.
- Unknown `policyType` values return HTTP 400 naming the type.
- Missing `policyType` returns HTTP 400 naming the field (no longer HTTP
500).
### How was this patch tested?
```
./gradlew :common:test --tests
org.apache.gravitino.dto.requests.TestPolicyCreateRequest \
--tests org.apache.gravitino.dto.requests.TestPolicyUpdatesRequest \
:server:test --tests
org.apache.gravitino.server.web.rest.TestPolicyOperations \
:docs:build \
-PskipITs
```
Coverage includes lowercase / uppercase / mixed-case known types on
create and update (DTO + REST), unknown types with empty and non-empty
content, missing `policyType` with empty / custom / null content, and
OpenAPI request examples validated by `:docs:build`
(`no-invalid-media-type-examples`).
---------
Co-authored-by: Cursor <[email protected]>
---
.../java/org/apache/gravitino/policy/Policy.java | 3 +
.../dto/requests/PolicyContentTypeIdResolver.java | 103 ++++++++++
.../dto/requests/PolicyCreateRequest.java | 41 ++--
.../dto/requests/PolicyUpdateRequest.java | 39 ++--
.../dto/requests/TestPolicyCreateRequest.java | 121 +++++++++++-
.../dto/requests/TestPolicyUpdatesRequest.java | 75 ++++++-
docs/manage-policies-in-gravitino.md | 5 +
docs/open-api/policies.yaml | 217 +++++++++++++++------
.../server/web/rest/PolicyOperations.java | 12 --
.../server/web/rest/TestPolicyOperations.java | 144 ++++++++++++++
10 files changed, 626 insertions(+), 134 deletions(-)
diff --git a/api/src/main/java/org/apache/gravitino/policy/Policy.java
b/api/src/main/java/org/apache/gravitino/policy/Policy.java
index 337ca0f7ce..bb96715398 100644
--- a/api/src/main/java/org/apache/gravitino/policy/Policy.java
+++ b/api/src/main/java/org/apache/gravitino/policy/Policy.java
@@ -79,6 +79,9 @@ public interface Policy extends Auditable {
/**
* Get the built-in policy type from the policy type string.
*
+ * <p>Matching is case-insensitive. Callers should persist and return
{@link #policyType()} so
+ * responses use the canonical lowercase wire value.
+ *
* @param policyType the policy type string
* @return the matched built-in policy type
* @throws IllegalArgumentException if the policy type is unknown or blank
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/PolicyContentTypeIdResolver.java
b/common/src/main/java/org/apache/gravitino/dto/requests/PolicyContentTypeIdResolver.java
new file mode 100644
index 0000000000..72ea1cbe9a
--- /dev/null
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/PolicyContentTypeIdResolver.java
@@ -0,0 +1,103 @@
+/*
+ * 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.gravitino.dto.requests;
+
+import com.fasterxml.jackson.annotation.JsonTypeInfo;
+import com.fasterxml.jackson.databind.DatabindContext;
+import com.fasterxml.jackson.databind.JavaType;
+import com.fasterxml.jackson.databind.jsontype.impl.TypeIdResolverBase;
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import org.apache.gravitino.dto.policy.PolicyContentDTO;
+import org.apache.gravitino.policy.Policy;
+
+/**
+ * Selects the policy content DTO from the sibling {@code policyType},
matching known types
+ * case-insensitively.
+ *
+ * <p>An unknown type resolves to no content class, so the content is bound as
{@code null} and
+ * request validation reports the type by name.
+ */
+public class PolicyContentTypeIdResolver extends TypeIdResolverBase {
+
+ private static final Map<Policy.BuiltInType, Class<? extends
PolicyContentDTO>> CONTENT_CLASSES =
+ ImmutableMap.<Policy.BuiltInType, Class<? extends
PolicyContentDTO>>builder()
+ .put(
+ Policy.BuiltInType.ICEBERG_COMPACTION,
+ PolicyContentDTO.IcebergCompactionContentDTO.class)
+ .put(
+ Policy.BuiltInType.ICEBERG_ORPHAN_FILE_REMOVAL,
+ PolicyContentDTO.IcebergOrphanFileRemovalContentDTO.class)
+ .put(
+ Policy.BuiltInType.ICEBERG_REWRITE_MANIFESTS,
+ PolicyContentDTO.IcebergRewriteManifestsContentDTO.class)
+ .put(
+ Policy.BuiltInType.ICEBERG_SNAPSHOT_EXPIRATION,
+ PolicyContentDTO.IcebergSnapshotExpirationContentDTO.class)
+ .put(Policy.BuiltInType.ROW_FILTER,
PolicyContentDTO.RowFilterContentDTO.class)
+ .put(Policy.BuiltInType.COLUMN_MASK,
PolicyContentDTO.ColumnMaskContentDTO.class)
+ .put(Policy.BuiltInType.ACCESS_CONTROL,
PolicyContentDTO.AccessControlContentDTO.class)
+ .put(Policy.BuiltInType.CUSTOM,
PolicyContentDTO.CustomContentDTO.class)
+ .build();
+
+ private JavaType baseType;
+
+ @Override
+ public void init(JavaType baseType) {
+ this.baseType = baseType;
+ }
+
+ @Override
+ public String idFromValue(Object value) {
+ return idFromValueAndType(value, value.getClass());
+ }
+
+ @Override
+ public String idFromValueAndType(Object value, Class<?> suggestedType) {
+ if (suggestedType == Void.class) {
+ // Jackson asks for the id of the Void defaultImpl when policyType is
absent. A null id fails
+ // deserialization, so return one that resolves to no content class: the
content then binds
+ // as null and request validation reports the missing policyType.
+ return "";
+ }
+ for (Map.Entry<Policy.BuiltInType, Class<? extends PolicyContentDTO>>
entry :
+ CONTENT_CLASSES.entrySet()) {
+ if (entry.getValue().isAssignableFrom(suggestedType)) {
+ return entry.getKey().policyType();
+ }
+ }
+ throw new IllegalArgumentException("Unknown policy content class: " +
suggestedType);
+ }
+
+ @Override
+ public JavaType typeFromId(DatabindContext context, String id) {
+ for (Map.Entry<Policy.BuiltInType, Class<? extends PolicyContentDTO>>
entry :
+ CONTENT_CLASSES.entrySet()) {
+ if (entry.getKey().policyType().equalsIgnoreCase(id)) {
+ return context.constructSpecializedType(baseType, entry.getValue());
+ }
+ }
+ return null;
+ }
+
+ @Override
+ public JsonTypeInfo.Id getMechanism() {
+ return JsonTypeInfo.Id.NAME;
+ }
+}
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/PolicyCreateRequest.java
b/common/src/main/java/org/apache/gravitino/dto/requests/PolicyCreateRequest.java
index 5a9e830ccf..bf2e774d9c 100644
---
a/common/src/main/java/org/apache/gravitino/dto/requests/PolicyCreateRequest.java
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/PolicyCreateRequest.java
@@ -19,8 +19,8 @@
package org.apache.gravitino.dto.requests;
import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
+import com.fasterxml.jackson.databind.annotation.JsonTypeIdResolver;
import com.google.common.base.Preconditions;
import javax.annotation.Nullable;
import lombok.EqualsAndHashCode;
@@ -28,9 +28,15 @@ import lombok.Getter;
import lombok.ToString;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.dto.policy.PolicyContentDTO;
+import org.apache.gravitino.policy.Policy;
import org.apache.gravitino.rest.RESTRequest;
-/** Represents a request to create a policy. */
+/**
+ * Represents a request to create a policy.
+ *
+ * <p>{@code policyType} accepts known type ids case-insensitively; storage
and responses use the
+ * canonical lowercase value from {@link Policy.BuiltInType#policyType()}.
+ */
@Getter
@EqualsAndHashCode
@ToString
@@ -54,33 +60,8 @@ public class PolicyCreateRequest implements RESTRequest {
use = JsonTypeInfo.Id.NAME,
include = JsonTypeInfo.As.EXTERNAL_PROPERTY,
property = "policyType",
- defaultImpl = PolicyContentDTO.CustomContentDTO.class)
- @JsonSubTypes({
- // add mappings for built-in types here
- // For example: @JsonSubTypes.Type(value = DataCompactionContent.class,
name =
- // "system_data_compaction")
- @JsonSubTypes.Type(
- value = PolicyContentDTO.IcebergCompactionContentDTO.class,
- name = "system_iceberg_compaction"),
- @JsonSubTypes.Type(
- value = PolicyContentDTO.IcebergOrphanFileRemovalContentDTO.class,
- name = "system_iceberg_orphan_file_removal"),
- @JsonSubTypes.Type(
- value = PolicyContentDTO.IcebergRewriteManifestsContentDTO.class,
- name = "system_iceberg_rewrite_manifests"),
- @JsonSubTypes.Type(
- value = PolicyContentDTO.IcebergSnapshotExpirationContentDTO.class,
- name = "system_iceberg_snapshot_expiration"),
- @JsonSubTypes.Type(
- value = PolicyContentDTO.RowFilterContentDTO.class,
- name = "system_row_filter"),
- @JsonSubTypes.Type(
- value = PolicyContentDTO.ColumnMaskContentDTO.class,
- name = "system_column_mask"),
- @JsonSubTypes.Type(
- value = PolicyContentDTO.AccessControlContentDTO.class,
- name = "system_access_control")
- })
+ defaultImpl = Void.class)
+ @JsonTypeIdResolver(PolicyContentTypeIdResolver.class)
private final PolicyContentDTO policyContent;
/**
@@ -117,6 +98,8 @@ public class PolicyCreateRequest implements RESTRequest {
StringUtils.isNotBlank(name), "\"name\" is required and cannot be
empty");
Preconditions.checkArgument(
StringUtils.isNotBlank(policyType), "\"policyType\" is required and
cannot be empty");
+ // An unknown policyType binds content as null, so resolve the type before
the content checks.
+ Policy.BuiltInType.fromPolicyType(policyType);
Preconditions.checkArgument(
policyContent != null, "\"content\" is required and cannot be null");
policyContent.validate();
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/PolicyUpdateRequest.java
b/common/src/main/java/org/apache/gravitino/dto/requests/PolicyUpdateRequest.java
index 0473426043..59bf7265bf 100644
---
a/common/src/main/java/org/apache/gravitino/dto/requests/PolicyUpdateRequest.java
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/PolicyUpdateRequest.java
@@ -24,12 +24,14 @@ import
com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
+import com.fasterxml.jackson.databind.annotation.JsonTypeIdResolver;
import com.google.common.base.Preconditions;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.ToString;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.dto.policy.PolicyContentDTO;
+import org.apache.gravitino.policy.Policy;
import org.apache.gravitino.policy.PolicyChange;
import org.apache.gravitino.rest.RESTRequest;
@@ -126,6 +128,9 @@ public interface PolicyUpdateRequest extends RESTRequest {
*
* <p>{@code newContent} fully replaces the previous content. Omitted typed
fields use that
* content type's defaults; they are not merged with the previously stored
values.
+ *
+ * <p>{@code policyType} accepts known type ids case-insensitively; the
change stores the
+ * canonical lowercase value from {@link Policy.BuiltInType#policyType()}.
*/
@EqualsAndHashCode
@Getter
@@ -140,33 +145,8 @@ public interface PolicyUpdateRequest extends RESTRequest {
use = JsonTypeInfo.Id.NAME,
include = JsonTypeInfo.As.EXTERNAL_PROPERTY,
property = "policyType",
- defaultImpl = PolicyContentDTO.CustomContentDTO.class)
- @JsonSubTypes({
- // add mappings for built-in types here
- // For example: @JsonSubTypes.Type(value = DataCompactionContent.class,
name =
- // "system_data_compaction")
- @JsonSubTypes.Type(
- value = PolicyContentDTO.IcebergCompactionContentDTO.class,
- name = "system_iceberg_compaction"),
- @JsonSubTypes.Type(
- value = PolicyContentDTO.IcebergOrphanFileRemovalContentDTO.class,
- name = "system_iceberg_orphan_file_removal"),
- @JsonSubTypes.Type(
- value = PolicyContentDTO.IcebergRewriteManifestsContentDTO.class,
- name = "system_iceberg_rewrite_manifests"),
- @JsonSubTypes.Type(
- value = PolicyContentDTO.IcebergSnapshotExpirationContentDTO.class,
- name = "system_iceberg_snapshot_expiration"),
- @JsonSubTypes.Type(
- value = PolicyContentDTO.RowFilterContentDTO.class,
- name = "system_row_filter"),
- @JsonSubTypes.Type(
- value = PolicyContentDTO.ColumnMaskContentDTO.class,
- name = "system_column_mask"),
- @JsonSubTypes.Type(
- value = PolicyContentDTO.AccessControlContentDTO.class,
- name = "system_access_control")
- })
+ defaultImpl = Void.class)
+ @JsonTypeIdResolver(PolicyContentTypeIdResolver.class)
private final PolicyContentDTO newContent;
/**
@@ -188,12 +168,15 @@ public interface PolicyUpdateRequest extends RESTRequest {
@Override
public PolicyChange policyChange() {
- return PolicyChange.updateContent(policyType, fromDTO(newContent));
+ return PolicyChange.updateContent(
+ Policy.BuiltInType.fromPolicyType(policyType).policyType(),
fromDTO(newContent));
}
@Override
public void validate() throws IllegalArgumentException {
Preconditions.checkArgument(policyType != null, "\"policyType\" must not
be null");
+ // An unknown policyType binds newContent as null, so resolve the type
first.
+ Policy.BuiltInType.fromPolicyType(policyType);
Preconditions.checkArgument(newContent != null, "\"newContent\" must not
be null");
newContent.validate();
}
diff --git
a/common/src/test/java/org/apache/gravitino/dto/requests/TestPolicyCreateRequest.java
b/common/src/test/java/org/apache/gravitino/dto/requests/TestPolicyCreateRequest.java
index 81b33ca3de..6ececfaae1 100644
---
a/common/src/test/java/org/apache/gravitino/dto/requests/TestPolicyCreateRequest.java
+++
b/common/src/test/java/org/apache/gravitino/dto/requests/TestPolicyCreateRequest.java
@@ -24,8 +24,11 @@ import com.google.common.collect.ImmutableSet;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.dto.policy.PolicyContentDTO;
import org.apache.gravitino.json.JsonUtils;
+import org.apache.gravitino.policy.Policy;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
public class TestPolicyCreateRequest {
@@ -39,14 +42,14 @@ public class TestPolicyCreateRequest {
.withProperties(null)
.build();
PolicyCreateRequest request =
- new PolicyCreateRequest("policy_test", "test_type", "policy comment",
true, content);
+ new PolicyCreateRequest("policy_test", "custom", "policy comment",
true, content);
String serJson = JsonUtils.objectMapper().writeValueAsString(request);
PolicyCreateRequest deserRequest =
JsonUtils.objectMapper().readValue(serJson, PolicyCreateRequest.class);
Assertions.assertEquals(request, deserRequest);
Assertions.assertEquals("policy_test", deserRequest.getName());
Assertions.assertEquals("policy comment", deserRequest.getComment());
- Assertions.assertEquals("test_type", deserRequest.getPolicyType());
+ Assertions.assertEquals("custom", deserRequest.getPolicyType());
Assertions.assertTrue(deserRequest.getEnabled());
Assertions.assertEquals(content, deserRequest.getPolicyContent());
@@ -59,17 +62,127 @@ public class TestPolicyCreateRequest {
.build();
request =
new PolicyCreateRequest(
- "policy_test_null_types", "test_type", "policy comment", true,
contentWithNullTypes);
+ "policy_test_null_types", "custom", "policy comment", true,
contentWithNullTypes);
serJson = JsonUtils.objectMapper().writeValueAsString(request);
deserRequest = JsonUtils.objectMapper().readValue(serJson,
PolicyCreateRequest.class);
Assertions.assertEquals(request, deserRequest);
Assertions.assertEquals("policy_test_null_types", deserRequest.getName());
Assertions.assertEquals("policy comment", deserRequest.getComment());
- Assertions.assertEquals("test_type", deserRequest.getPolicyType());
+ Assertions.assertEquals("custom", deserRequest.getPolicyType());
Assertions.assertTrue(deserRequest.getEnabled());
Assertions.assertEquals(contentWithNullTypes,
deserRequest.getPolicyContent());
Exception e = Assertions.assertThrows(IllegalArgumentException.class,
request::validate);
Assertions.assertEquals("supportedObjectTypes cannot be empty",
e.getMessage());
}
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {
+ "system_iceberg_compaction",
+ "SYSTEM_ICEBERG_COMPACTION",
+ "System_Iceberg_Compaction"
+ })
+ public void testDeserializeAcceptsKnownPolicyTypeCaseInsensitively(String
policyType)
+ throws JsonProcessingException {
+ String json =
+ "{\"name\":\"p_upper\",\"policyType\":\""
+ + policyType
+ + "\",\"enabled\":true,\"content\":{\"maxPartitionNum\":5}}";
+ PolicyCreateRequest request =
+ JsonUtils.objectMapper().readValue(json, PolicyCreateRequest.class);
+ Assertions.assertEquals(policyType, request.getPolicyType());
+ Assertions.assertInstanceOf(
+ PolicyContentDTO.IcebergCompactionContentDTO.class,
request.getPolicyContent());
+ Assertions.assertEquals(
+ Policy.BuiltInType.ICEBERG_COMPACTION,
+ Policy.BuiltInType.fromPolicyType(request.getPolicyType()));
+ request.validate();
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"custom", "CUSTOM", "Custom"})
+ public void testDeserializeAcceptsCustomPolicyTypeCaseInsensitively(String
policyType)
+ throws JsonProcessingException {
+ String json =
+ "{\"name\":\"p\",\"policyType\":\""
+ + policyType
+ + "\",\"enabled\":true,"
+ +
"\"content\":{\"customRules\":{\"a\":1},\"supportedObjectTypes\":[\"TABLE\"]}}";
+ PolicyCreateRequest request =
+ JsonUtils.objectMapper().readValue(json, PolicyCreateRequest.class);
+ Assertions.assertEquals(
+ Policy.BuiltInType.CUSTOM,
Policy.BuiltInType.fromPolicyType(request.getPolicyType()));
+ request.validate();
+ }
+
+ @Test
+ public void testDeserializeRejectsUnknownPolicyType() throws
JsonProcessingException {
+ assertPolicyTypeNamedOnDeserialize(
+
"{\"name\":\"p_bogus\",\"policyType\":\"system_iceberg_bogus\",\"enabled\":true,\"content\":{}}",
+ "Unknown built-in policy type: system_iceberg_bogus");
+ assertPolicyTypeNamedOnDeserialize(
+
"{\"name\":\"p_bogus\",\"policyType\":\"system_iceberg_bogus\",\"enabled\":true,"
+ + "\"content\":{\"maxPartitionNum\":5}}",
+ "Unknown built-in policy type: system_iceberg_bogus");
+ assertPolicyTypeNamedOnDeserialize(
+
"{\"name\":\"p_other\",\"policyType\":\"not_a_policy\",\"enabled\":true,"
+ +
"\"content\":{\"customRules\":{},\"supportedObjectTypes\":[\"TABLE\"]}}",
+ "Unknown policy type: not_a_policy, it should start with 'system_' or
be 'custom'");
+ }
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {"{}",
"{\"customRules\":{},\"supportedObjectTypes\":[\"TABLE\"]}", "null"})
+ public void testDeserializeReportsMissingPolicyType(String content)
+ throws JsonProcessingException {
+ assertPolicyTypeNamedOnDeserialize(
+ "{\"name\":\"p\",\"content\":" + content + "}",
+ "\"policyType\" is required and cannot be empty");
+ }
+
+ @Test
+ public void testDeserializeRejectsMalformedEnabled() {
+ String content =
"\"content\":{\"customRules\":{},\"supportedObjectTypes\":[\"table\"]}}";
+ for (String enabled : new String[] {"\"typo\"", "{}", "[]"}) {
+ String json =
+ "{\"name\":\"p\",\"policyType\":\"custom\",\"enabled\":" + enabled +
"," + content;
+ Assertions.assertThrows(
+ JsonProcessingException.class,
+ () -> JsonUtils.objectMapper().readValue(json,
PolicyCreateRequest.class),
+ enabled);
+ }
+ }
+
+ @Test
+ public void testDeserializeRejectsUnknownProperty() {
+ String json =
+ "{\"name\":\"p\",\"policyType\":\"custom\",\"enabld\":false,"
+ +
"\"content\":{\"customRules\":{},\"supportedObjectTypes\":[\"table\"]}}";
+ JsonProcessingException thrown =
+ Assertions.assertThrows(
+ JsonProcessingException.class,
+ () -> JsonUtils.objectMapper().readValue(json,
PolicyCreateRequest.class));
+ Assertions.assertTrue(thrown.getMessage().contains("enabld"),
thrown.getMessage());
+ }
+
+ @Test
+ public void testDeserializeEnabledFalse() throws JsonProcessingException {
+ String json =
+ "{\"name\":\"p\",\"policyType\":\"custom\",\"enabled\":false,"
+ +
"\"content\":{\"customRules\":{},\"supportedObjectTypes\":[\"table\"]}}";
+ PolicyCreateRequest request =
+ JsonUtils.objectMapper().readValue(json, PolicyCreateRequest.class);
+ Assertions.assertFalse(request.getEnabled());
+ }
+
+ private static void assertPolicyTypeNamedOnDeserialize(String json, String
expectedMessage)
+ throws JsonProcessingException {
+ PolicyCreateRequest request =
+ JsonUtils.objectMapper().readValue(json, PolicyCreateRequest.class);
+ Assertions.assertNull(request.getPolicyContent());
+ IllegalArgumentException thrown =
+ Assertions.assertThrows(IllegalArgumentException.class,
request::validate);
+ Assertions.assertEquals(expectedMessage, thrown.getMessage());
+ }
}
diff --git
a/common/src/test/java/org/apache/gravitino/dto/requests/TestPolicyUpdatesRequest.java
b/common/src/test/java/org/apache/gravitino/dto/requests/TestPolicyUpdatesRequest.java
index 98da580afe..52c88b569b 100644
---
a/common/src/test/java/org/apache/gravitino/dto/requests/TestPolicyUpdatesRequest.java
+++
b/common/src/test/java/org/apache/gravitino/dto/requests/TestPolicyUpdatesRequest.java
@@ -21,11 +21,16 @@ package org.apache.gravitino.dto.requests;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import java.util.List;
+import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.dto.policy.PolicyContentDTO;
import org.apache.gravitino.json.JsonUtils;
+import org.apache.gravitino.policy.Policy;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
public class TestPolicyUpdatesRequest {
@@ -57,19 +62,82 @@ public class TestPolicyUpdatesRequest {
PolicyContentDTO contentDTO =
PolicyContentDTO.CustomContentDTO.builder()
.withCustomRules(ImmutableMap.of("rule1", "value1"))
+
.withSupportedObjectTypes(ImmutableSet.of(MetadataObject.Type.TABLE))
.withProperties(ImmutableMap.of("key", "value"))
.build();
PolicyUpdateRequest.UpdatePolicyContentRequest request =
- new PolicyUpdateRequest.UpdatePolicyContentRequest("test_type",
contentDTO);
+ new PolicyUpdateRequest.UpdatePolicyContentRequest("custom",
contentDTO);
String serJson = JsonUtils.objectMapper().writeValueAsString(request);
PolicyUpdateRequest.UpdatePolicyContentRequest deserRequest =
JsonUtils.objectMapper()
.readValue(serJson,
PolicyUpdateRequest.UpdatePolicyContentRequest.class);
Assertions.assertEquals(request, deserRequest);
- Assertions.assertEquals("test_type", deserRequest.getPolicyType());
+ Assertions.assertEquals("custom", deserRequest.getPolicyType());
Assertions.assertEquals(contentDTO, deserRequest.getNewContent());
}
+ @ParameterizedTest
+ @ValueSource(
+ strings = {
+ "system_iceberg_compaction",
+ "SYSTEM_ICEBERG_COMPACTION",
+ "System_Iceberg_Compaction"
+ })
+ public void
testUpdatePolicyContentAcceptsKnownPolicyTypeCaseInsensitively(String
policyType)
+ throws JsonProcessingException {
+ String json =
+ "{\"@type\":\"updateContent\",\"policyType\":\""
+ + policyType
+ + "\",\"newContent\":{\"maxPartitionNum\":5}}";
+ PolicyUpdateRequest.UpdatePolicyContentRequest request =
+ JsonUtils.objectMapper()
+ .readValue(json,
PolicyUpdateRequest.UpdatePolicyContentRequest.class);
+ Assertions.assertEquals(policyType, request.getPolicyType());
+ Assertions.assertInstanceOf(
+ PolicyContentDTO.IcebergCompactionContentDTO.class,
request.getNewContent());
+ request.validate();
+ Assertions.assertTrue(
+ request
+ .policyChange()
+ .toString()
+ .contains(Policy.BuiltInType.ICEBERG_COMPACTION.policyType()),
+ request.policyChange().toString());
+ }
+
+ @Test
+ public void testUpdatePolicyContentDeserializeRejectsUnknownPolicyType()
+ throws JsonProcessingException {
+ assertPolicyTypeNamedOnDeserialize(
+ "{\"@type\":\"updateContent\",\"policyType\":\"system_iceberg_bogus\","
+ + "\"newContent\":{}}",
+ "Unknown built-in policy type: system_iceberg_bogus");
+ assertPolicyTypeNamedOnDeserialize(
+ "{\"@type\":\"updateContent\",\"policyType\":\"system_iceberg_bogus\","
+ + "\"newContent\":{\"maxPartitionNum\":5}}",
+ "Unknown built-in policy type: system_iceberg_bogus");
+ }
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {"{}",
"{\"customRules\":{},\"supportedObjectTypes\":[\"TABLE\"]}", "null"})
+ public void
testUpdatePolicyContentDeserializeReportsMissingPolicyType(String content)
+ throws JsonProcessingException {
+ assertPolicyTypeNamedOnDeserialize(
+ "{\"@type\":\"updateContent\",\"newContent\":" + content + "}",
+ "\"policyType\" must not be null");
+ }
+
+ private static void assertPolicyTypeNamedOnDeserialize(String json, String
expectedMessage)
+ throws JsonProcessingException {
+ PolicyUpdateRequest.UpdatePolicyContentRequest request =
+ JsonUtils.objectMapper()
+ .readValue(json,
PolicyUpdateRequest.UpdatePolicyContentRequest.class);
+ Assertions.assertNull(request.getNewContent());
+ IllegalArgumentException thrown =
+ Assertions.assertThrows(IllegalArgumentException.class,
request::validate);
+ Assertions.assertEquals(expectedMessage, thrown.getMessage());
+ }
+
@Test
public void testPolicyUpdatesRequestSerDe() throws JsonProcessingException {
PolicyUpdateRequest request = new
PolicyUpdateRequest.RenamePolicyRequest("policy_test_new");
@@ -77,9 +145,10 @@ public class TestPolicyUpdatesRequest {
new PolicyUpdateRequest.UpdatePolicyCommentRequest("policy comment
new");
PolicyUpdateRequest request2 =
new PolicyUpdateRequest.UpdatePolicyContentRequest(
- "test_type",
+ "custom",
PolicyContentDTO.CustomContentDTO.builder()
.withCustomRules(ImmutableMap.of("rule1", "value1"))
+
.withSupportedObjectTypes(ImmutableSet.of(MetadataObject.Type.TABLE))
.withProperties(ImmutableMap.of("key", "value"))
.build());
diff --git a/docs/manage-policies-in-gravitino.md
b/docs/manage-policies-in-gravitino.md
index 9b1becb4ff..a7cf3bd233 100644
--- a/docs/manage-policies-in-gravitino.md
+++ b/docs/manage-policies-in-gravitino.md
@@ -27,6 +27,11 @@ creation. Updating custom policy content replaces the whole
content and can chan
Object policy lookup does not filter by this field; consumers decide which
policy types they can
use.
+`policyType` accepts the known wire values case-insensitively (for example
`CUSTOM` and
+`System_Iceberg_Compaction`). Gravitino stores and returns the canonical
lowercase value listed in
+the OpenAPI schema. Unknown types such as `system_iceberg_bogus` return HTTP
400 that names the
+type.
+
<Tabs groupId='language' queryString>
<TabItem value="shell" label="REST">
diff --git a/docs/open-api/policies.yaml b/docs/open-api/policies.yaml
index d8a8361017..995a53d5c9 100644
--- a/docs/open-api/policies.yaml
+++ b/docs/open-api/policies.yaml
@@ -70,6 +70,12 @@ paths:
examples:
PolicyCreate:
$ref: "#/components/examples/PolicyCreate"
+ PolicyCreateLowercaseType:
+ $ref: "#/components/examples/PolicyCreateLowercaseType"
+ PolicyCreateUppercaseType:
+ $ref: "#/components/examples/PolicyCreateUppercaseType"
+ PolicyCreateMixedCaseType:
+ $ref: "#/components/examples/PolicyCreateMixedCaseType"
responses:
"200":
@@ -139,6 +145,13 @@ paths:
application/json:
schema:
$ref: "#/components/schemas/PolicyUpdatesRequest"
+ examples:
+ PolicyUpdateContentLowercaseType:
+ $ref: "#/components/examples/PolicyUpdateContentLowercaseType"
+ PolicyUpdateContentUppercaseType:
+ $ref: "#/components/examples/PolicyUpdateContentUppercaseType"
+ PolicyUpdateContentMixedCaseType:
+ $ref: "#/components/examples/PolicyUpdateContentMixedCaseType"
responses:
"200":
description: Returns the updated policy object
@@ -408,8 +421,7 @@ components:
required: [content]
properties:
policyType:
- type: string
- enum: [system_iceberg_compaction]
+ $ref: '#/components/schemas/IcebergCompactionRequestPolicyType'
content:
$ref: '#/components/schemas/IcebergCompactionContent'
@@ -452,8 +464,7 @@ components:
required: [content]
properties:
policyType:
- type: string
- enum: [system_iceberg_orphan_file_removal]
+ $ref:
'#/components/schemas/IcebergOrphanFileRemovalRequestPolicyType'
content:
$ref: '#/components/schemas/IcebergOrphanFileRemovalContent'
@@ -510,8 +521,7 @@ components:
required: [content]
properties:
policyType:
- type: string
- enum: [system_iceberg_rewrite_manifests]
+ $ref:
'#/components/schemas/IcebergRewriteManifestsRequestPolicyType'
content:
$ref: '#/components/schemas/IcebergRewriteManifestsContent'
@@ -561,8 +571,7 @@ components:
required: [content]
properties:
policyType:
- type: string
- enum: [system_iceberg_snapshot_expiration]
+ $ref:
'#/components/schemas/IcebergSnapshotExpirationRequestPolicyType'
content:
$ref: '#/components/schemas/IcebergSnapshotExpirationContent'
@@ -655,8 +664,7 @@ components:
required: [content]
properties:
policyType:
- type: string
- enum: [system_row_filter]
+ $ref: '#/components/schemas/RowFilterRequestPolicyType'
content:
$ref: '#/components/schemas/RowFilterContent'
@@ -679,8 +687,7 @@ components:
required: [content]
properties:
policyType:
- type: string
- enum: [system_column_mask]
+ $ref: '#/components/schemas/ColumnMaskRequestPolicyType'
content:
$ref: '#/components/schemas/ColumnMaskContent'
@@ -703,8 +710,7 @@ components:
required: [content]
properties:
policyType:
- type: string
- enum: [system_access_control]
+ $ref: '#/components/schemas/AccessControlRequestPolicyType'
content:
$ref: '#/components/schemas/AccessControlContent'
@@ -878,6 +884,46 @@ components:
- "FUNCTION"
- "COLUMN"
+ CustomRequestPolicyType:
+ type: string
+ description: Request value `custom`, matched case-insensitively.
+ pattern: '^[cC][uU][sS][tT][oO][mM]$'
+
+ IcebergCompactionRequestPolicyType:
+ type: string
+ description: Request value `system_iceberg_compaction`, matched
case-insensitively.
+ pattern:
'^[sS][yY][sS][tT][eE][mM]_[iI][cC][eE][bB][eE][rR][gG]_[cC][oO][mM][pP][aA][cC][tT][iI][oO][nN]$'
+
+ IcebergOrphanFileRemovalRequestPolicyType:
+ type: string
+ description: Request value `system_iceberg_orphan_file_removal`, matched
case-insensitively.
+ pattern:
'^[sS][yY][sS][tT][eE][mM]_[iI][cC][eE][bB][eE][rR][gG]_[oO][rR][pP][hH][aA][nN]_[fF][iI][lL][eE]_[rR][eE][mM][oO][vV][aA][lL]$'
+
+ IcebergRewriteManifestsRequestPolicyType:
+ type: string
+ description: Request value `system_iceberg_rewrite_manifests`, matched
case-insensitively.
+ pattern:
'^[sS][yY][sS][tT][eE][mM]_[iI][cC][eE][bB][eE][rR][gG]_[rR][eE][wW][rR][iI][tT][eE]_[mM][aA][nN][iI][fF][eE][sS][tT][sS]$'
+
+ IcebergSnapshotExpirationRequestPolicyType:
+ type: string
+ description: Request value `system_iceberg_snapshot_expiration`, matched
case-insensitively.
+ pattern:
'^[sS][yY][sS][tT][eE][mM]_[iI][cC][eE][bB][eE][rR][gG]_[sS][nN][aA][pP][sS][hH][oO][tT]_[eE][xX][pP][iI][rR][aA][tT][iI][oO][nN]$'
+
+ RowFilterRequestPolicyType:
+ type: string
+ description: Request value `system_row_filter`, matched
case-insensitively.
+ pattern:
'^[sS][yY][sS][tT][eE][mM]_[rR][oO][wW]_[fF][iI][lL][tT][eE][rR]$'
+
+ ColumnMaskRequestPolicyType:
+ type: string
+ description: Request value `system_column_mask`, matched
case-insensitively.
+ pattern:
'^[sS][yY][sS][tT][eE][mM]_[cC][oO][lL][uU][mM][nN]_[mM][aA][sS][kK]$'
+
+ AccessControlRequestPolicyType:
+ type: string
+ description: Request value `system_access_control`, matched
case-insensitively.
+ pattern:
'^[sS][yY][sS][tT][eE][mM]_[aA][cC][cC][eE][sS][sS]_[cC][oO][nN][tT][rR][oO][lL]$'
+
PolicyCreateRequestBase:
type: object
description: A base policy create request without content
@@ -894,7 +940,11 @@ components:
nullable: true
policyType:
type: string
- description: The type of the policy
+ description: >-
+ The type of the policy, either `custom` or a built-in `system_*`
type. Known
+ values are matched case-insensitively, so request schemas select
the content by a
+ case-insensitive pattern; the server stores and returns the
canonical lowercase
+ form. An unknown value is rejected with HTTP 400 naming the type.
enabled:
type: boolean
description: Whether the policy is enabled
@@ -909,9 +959,7 @@ components:
- content
properties:
policyType:
- type: string
- not:
- enum: [system_iceberg_compaction,
system_iceberg_orphan_file_removal, system_iceberg_rewrite_manifests,
system_iceberg_snapshot_expiration, system_row_filter, system_column_mask,
system_access_control]
+ $ref: '#/components/schemas/CustomRequestPolicyType'
content:
$ref: '#/components/schemas/CustomPolicyContent'
@@ -927,17 +975,14 @@ components:
- $ref: '#/components/schemas/RowFilterPolicyCreateRequest'
- $ref: '#/components/schemas/ColumnMaskPolicyCreateRequest'
- $ref: '#/components/schemas/AccessControlPolicyCreateRequest'
- discriminator:
- propertyName: policyType
- mapping:
- custom: '#/components/schemas/CustomPolicyCreateRequest'
- system_iceberg_orphan_file_removal:
'#/components/schemas/IcebergOrphanFileRemovalPolicyCreateRequest'
- system_iceberg_rewrite_manifests:
'#/components/schemas/IcebergRewriteManifestsPolicyCreateRequest'
- system_iceberg_snapshot_expiration:
'#/components/schemas/IcebergSnapshotExpirationPolicyCreateRequest'
- system_iceberg_compaction:
'#/components/schemas/IcebergCompactionPolicyCreateRequest'
- system_row_filter:
'#/components/schemas/RowFilterPolicyCreateRequest'
- system_column_mask:
'#/components/schemas/ColumnMaskPolicyCreateRequest'
- system_access_control:
'#/components/schemas/AccessControlPolicyCreateRequest'
+ example: {
+ "name": "compaction_upper",
+ "policyType": "SYSTEM_ICEBERG_COMPACTION",
+ "enabled": true,
+ "content": {
+ "maxPartitionNum": 5
+ }
+ }
PolicyUpdatesRequest:
@@ -1017,7 +1062,11 @@ components:
enum: [updateContent]
policyType:
type: string
- description: The type of the policy.
+ description: >-
+ The type of the policy, either `custom` or a built-in `system_*`
type. Known
+ values are matched case-insensitively, so request schemas select
the content by a
+ case-insensitive pattern; the server stores and returns the
canonical lowercase
+ form. An unknown value is rejected with HTTP 400 naming the type.
UpdateCustomPolicyContentRequest:
allOf:
@@ -1025,9 +1074,7 @@ components:
- type: object
properties:
policyType:
- type: string
- not:
- enum: [system_iceberg_compaction,
system_iceberg_orphan_file_removal, system_iceberg_rewrite_manifests,
system_iceberg_snapshot_expiration, system_row_filter, system_column_mask,
system_access_control]
+ $ref: '#/components/schemas/CustomRequestPolicyType'
newContent:
$ref: '#/components/schemas/CustomPolicyContent'
@@ -1037,8 +1084,7 @@ components:
- type: object
properties:
policyType:
- type: string
- enum: [system_iceberg_compaction]
+ $ref: '#/components/schemas/IcebergCompactionRequestPolicyType'
newContent:
$ref: '#/components/schemas/IcebergCompactionContent'
@@ -1048,8 +1094,7 @@ components:
- type: object
properties:
policyType:
- type: string
- enum: [system_iceberg_orphan_file_removal]
+ $ref:
'#/components/schemas/IcebergOrphanFileRemovalRequestPolicyType'
newContent:
$ref: '#/components/schemas/IcebergOrphanFileRemovalContent'
@@ -1059,8 +1104,7 @@ components:
- type: object
properties:
policyType:
- type: string
- enum: [system_iceberg_rewrite_manifests]
+ $ref:
'#/components/schemas/IcebergRewriteManifestsRequestPolicyType'
newContent:
$ref: '#/components/schemas/IcebergRewriteManifestsContent'
@@ -1070,8 +1114,7 @@ components:
- type: object
properties:
policyType:
- type: string
- enum: [system_iceberg_snapshot_expiration]
+ $ref:
'#/components/schemas/IcebergSnapshotExpirationRequestPolicyType'
newContent:
$ref: '#/components/schemas/IcebergSnapshotExpirationContent'
@@ -1081,8 +1124,7 @@ components:
- type: object
properties:
policyType:
- type: string
- enum: [system_row_filter]
+ $ref: '#/components/schemas/RowFilterRequestPolicyType'
newContent:
$ref: '#/components/schemas/RowFilterContent'
@@ -1092,8 +1134,7 @@ components:
- type: object
properties:
policyType:
- type: string
- enum: [system_column_mask]
+ $ref: '#/components/schemas/ColumnMaskRequestPolicyType'
newContent:
$ref: '#/components/schemas/ColumnMaskContent'
@@ -1103,8 +1144,7 @@ components:
- type: object
properties:
policyType:
- type: string
- enum: [system_access_control]
+ $ref: '#/components/schemas/AccessControlRequestPolicyType'
newContent:
$ref: '#/components/schemas/AccessControlContent'
@@ -1119,17 +1159,6 @@ components:
- $ref: '#/components/schemas/UpdateRowFilterPolicyContentRequest'
- $ref: '#/components/schemas/UpdateColumnMaskPolicyContentRequest'
- $ref: '#/components/schemas/UpdateAccessControlPolicyContentRequest'
- discriminator:
- propertyName: policyType
- mapping:
- custom: '#/components/schemas/UpdateCustomPolicyContentRequest'
- system_iceberg_compaction:
'#/components/schemas/UpdateIcebergCompactionPolicyContentRequest'
- system_iceberg_orphan_file_removal:
'#/components/schemas/UpdateIcebergOrphanFileRemovalPolicyContentRequest'
- system_iceberg_rewrite_manifests:
'#/components/schemas/UpdateIcebergRewriteManifestsPolicyContentRequest'
- system_iceberg_snapshot_expiration:
'#/components/schemas/UpdateIcebergSnapshotExpirationPolicyContentRequest'
- system_row_filter:
'#/components/schemas/UpdateRowFilterPolicyContentRequest'
- system_column_mask:
'#/components/schemas/UpdateColumnMaskPolicyContentRequest'
- system_access_control:
'#/components/schemas/UpdateAccessControlPolicyContentRequest'
example: {
"@type": "updateContent",
"policyType": "custom",
@@ -1245,6 +1274,78 @@ components:
}
}
+ PolicyCreateLowercaseType:
+ summary: Create a compaction policy with the canonical lowercase
policyType
+ value: {
+ "name": "compaction_policy",
+ "policyType": "system_iceberg_compaction",
+ "content": {
+ "maxPartitionNum": 5
+ }
+ }
+
+ PolicyCreateUppercaseType:
+ summary: Create a compaction policy with an uppercase policyType
+ value: {
+ "name": "compaction_policy",
+ "policyType": "SYSTEM_ICEBERG_COMPACTION",
+ "content": {
+ "maxPartitionNum": 5
+ }
+ }
+
+ PolicyCreateMixedCaseType:
+ summary: Create a compaction policy with a mixed-case policyType
+ value: {
+ "name": "compaction_policy",
+ "policyType": "System_Iceberg_Compaction",
+ "content": {
+ "maxPartitionNum": 5
+ }
+ }
+
+ PolicyUpdateContentLowercaseType:
+ summary: Update compaction content with the canonical lowercase
policyType
+ value: {
+ "updates": [
+ {
+ "@type": "updateContent",
+ "policyType": "system_iceberg_compaction",
+ "newContent": {
+ "maxPartitionNum": 5
+ }
+ }
+ ]
+ }
+
+ PolicyUpdateContentUppercaseType:
+ summary: Update compaction content with an uppercase policyType
+ value: {
+ "updates": [
+ {
+ "@type": "updateContent",
+ "policyType": "SYSTEM_ICEBERG_COMPACTION",
+ "newContent": {
+ "maxPartitionNum": 5
+ }
+ }
+ ]
+ }
+
+ PolicyUpdateContentMixedCaseType:
+ summary: Update compaction content with a mixed-case policyType
+ value: {
+ "updates": [
+ {
+ "@type": "updateContent",
+ "policyType": "System_Iceberg_Compaction",
+ "newContent": {
+ "maxPartitionNum": 5
+ }
+ }
+ ]
+ }
+
PolicyResponse:
value: {
"code": 0,
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java
index 11c87f0b64..3d523c6207 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java
@@ -164,7 +164,6 @@ public class PolicyOperations {
request.validate();
NameIdentifierUtil.checkMetadataObjectName(
NameIdentifierUtil.ofPolicy(metalake, request.getName()),
Entity.EntityType.POLICY);
- validateCreatePolicyType(request.getPolicyType());
PolicyEntity policy =
policyDispatcher.createPolicy(
metalake,
@@ -405,15 +404,4 @@ public class PolicyOperations {
.map(PolicyAssociationSelectorSerde::deserialize)
.orElseGet(AllValuesSelector::get));
}
-
- private static void validateCreatePolicyType(String policyType) {
- Policy.BuiltInType builtInType =
Policy.BuiltInType.fromPolicyType(policyType);
- if (builtInType != Policy.BuiltInType.CUSTOM
- && !builtInType.policyType().equalsIgnoreCase(policyType)) {
- throw new IllegalArgumentException(
- String.format(
- "Built-in policy type must use prefixed value '%s', but got: %s",
- builtInType.policyType(), policyType));
- }
- }
}
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java
index 9b8745b9c7..9c29c71eba 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java
@@ -83,6 +83,7 @@ import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mockito;
public class TestPolicyOperations extends BaseOperationsTest {
@@ -544,6 +545,75 @@ public class TestPolicyOperations extends
BaseOperationsTest {
Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
resp.getStatus());
}
+ @Test
+ public void testCreatePolicyUnknownTypeNamedInError() {
+ assertCreatePolicyTypeNamedInError(
+
"{\"name\":\"p_bogus\",\"comment\":\"\",\"policyType\":\"system_iceberg_bogus\","
+ + "\"enabled\":true,\"content\":{}}",
+ "Unknown built-in policy type: system_iceberg_bogus");
+ assertCreatePolicyTypeNamedInError(
+
"{\"name\":\"p_bogus\",\"comment\":\"\",\"policyType\":\"system_iceberg_bogus\","
+ + "\"enabled\":true,\"content\":{\"maxPartitionNum\":5}}",
+ "Unknown built-in policy type: system_iceberg_bogus");
+ }
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {"{}",
"{\"customRules\":{},\"supportedObjectTypes\":[\"TABLE\"]}", "null"})
+ public void testCreatePolicyMissingTypeNamedInError(String content) {
+ assertCreatePolicyTypeNamedInError(
+ "{\"name\":\"p\",\"content\":" + content + "}",
+ "\\\"policyType\\\" is required and cannot be empty");
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"SYSTEM_ICEBERG_COMPACTION",
"System_Iceberg_Compaction"})
+ public void testCreatePolicyAcceptsMisCasedKnownType(String policyType) {
+ PolicyContent content = PolicyContents.icebergDataCompaction();
+ PolicyEntity policy =
+ PolicyEntity.builder()
+ .withId(1L)
+ .withName("p_upper")
+ .withPolicyType(Policy.BuiltInType.ICEBERG_COMPACTION)
+ .withEnabled(true)
+ .withContent(content)
+ .withAuditInfo(testAuditInfo1)
+ .build();
+ when(policyManager.createPolicy(any(), any(), any(), any(), anyBoolean(),
any()))
+ .thenAnswer(
+ invocation -> {
+ Assertions.assertEquals(
+ Policy.BuiltInType.ICEBERG_COMPACTION,
invocation.getArgument(2));
+ return policy;
+ });
+
+ Response resp =
+ target(policyPath(metalake))
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .post(
+ Entity.entity(
+ "{\"name\":\"p_upper\",\"comment\":\"\",\"policyType\":\""
+ + policyType
+ +
"\",\"enabled\":true,\"content\":{\"maxPartitionNum\":5}}",
+ MediaType.APPLICATION_JSON_TYPE));
+ Assertions.assertEquals(
+ Response.Status.OK.getStatusCode(), resp.getStatus(),
resp.readEntity(String.class));
+ }
+
+ private void assertCreatePolicyTypeNamedInError(String body, String
expectedMessage) {
+ Response resp =
+ target(policyPath(metalake))
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .post(Entity.entity(body, MediaType.APPLICATION_JSON_TYPE));
+ Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
resp.getStatus());
+ String msg = resp.readEntity(String.class);
+ Assertions.assertTrue(msg.contains(expectedMessage), msg);
+ Assertions.assertFalse(msg.contains("supportedObjectTypes"), msg);
+ Assertions.assertFalse(msg.contains("Malformed json request"), msg);
+ }
+
@Test
public void testCreatePolicyRejectsDottedName() {
PolicyContent content =
@@ -688,6 +758,80 @@ public class TestPolicyOperations extends
BaseOperationsTest {
}
}
+ @ParameterizedTest
+ @ValueSource(strings = {"SYSTEM_ICEBERG_COMPACTION",
"System_Iceberg_Compaction"})
+ public void testAlterPolicyContentAcceptsMisCasedKnownType(String
policyType) {
+ PolicyContent content = PolicyContents.icebergDataCompaction();
+ PolicyEntity policy =
+ PolicyEntity.builder()
+ .withId(1L)
+ .withName("p_upper")
+ .withPolicyType(Policy.BuiltInType.ICEBERG_COMPACTION)
+ .withEnabled(true)
+ .withContent(content)
+ .withAuditInfo(testAuditInfo1)
+ .build();
+ when(policyManager.alterPolicy(any(), any(), any()))
+ .thenAnswer(
+ invocation -> {
+ PolicyChange.UpdateContent change = invocation.getArgument(2);
+ Assertions.assertEquals(
+ Policy.BuiltInType.ICEBERG_COMPACTION.policyType(),
change.getPolicyType());
+ return policy;
+ });
+
+ Response resp =
+ target(policyPath(metalake))
+ .path("p_upper")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(
+ Entity.entity(
+ "{\"updates\":[{\"@type\":\"updateContent\","
+ + "\"policyType\":\""
+ + policyType
+ + "\",\"newContent\":{\"maxPartitionNum\":5}}]}",
+ MediaType.APPLICATION_JSON_TYPE));
+ Assertions.assertEquals(
+ Response.Status.OK.getStatusCode(), resp.getStatus(),
resp.readEntity(String.class));
+ }
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {"{}",
"{\"customRules\":{},\"supportedObjectTypes\":[\"TABLE\"]}", "null"})
+ public void testAlterPolicyContentMissingTypeNamedInError(String content) {
+ Response resp =
+ target(policyPath(metalake))
+ .path("p_missing")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(
+ Entity.entity(
+
"{\"updates\":[{\"@type\":\"updateContent\",\"newContent\":" + content + "}]}",
+ MediaType.APPLICATION_JSON_TYPE));
+ Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
resp.getStatus());
+ String msg = resp.readEntity(String.class);
+ Assertions.assertTrue(msg.contains("\\\"policyType\\\" must not be null"),
msg);
+ }
+
+ @Test
+ public void testAlterPolicyContentUnknownTypeNamedInError() {
+ Response resp =
+ target(policyPath(metalake))
+ .path("p_bogus")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(
+ Entity.entity(
+ "{\"updates\":[{\"@type\":\"updateContent\","
+ + "\"policyType\":\"system_iceberg_bogus\","
+ + "\"newContent\":{\"maxPartitionNum\":5}}]}",
+ MediaType.APPLICATION_JSON_TYPE));
+ Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
resp.getStatus());
+ String msg = resp.readEntity(String.class);
+ Assertions.assertTrue(msg.contains("Unknown built-in policy type:
system_iceberg_bogus"), msg);
+ }
+
@Test
public void testAlterPolicy() {
ImmutableMap<String, Object> contentFields =
ImmutableMap.of("target_file_size_bytes", 1000);
