This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 8e5f701910 [Cherry-pick to branch-1.3] [#14428] fix(policy): Accept
known policyType case-insensitively (#14520) (#14619)
8e5f701910 is described below
commit 8e5f7019100899aff339719a5c6fb71a64dd3093
Author: MaSai <[email protected]>
AuthorDate: Thu Nov 5 10:11:22 2026 +0800
[Cherry-pick to branch-1.3] [#14428] fix(policy): Accept known policyType
case-insensitively (#14520) (#14619)
## Purpose
Cherry-pick of #14520 onto `branch-1.3`, replacing bot PR #14615 (closed
with conflicts).
## Changes
- Accept known `policyType` values case-insensitively via
`PolicyContentTypeIdResolver`.
- branch-1.3 adaptations (main has more built-in policy types):
- Resolver / OpenAPI / docs only cover `ICEBERG_COMPACTION` and `CUSTOM`
- Drop `validateCreatePolicyType` that depended on main-only built-in
types
- OpenAPI `policies.yaml` adapted to 1.3 schema (no conflict markers)
## Closes
Supersedes #14615
---------
Co-authored-by: Cursor <[email protected]>
---
.../java/org/apache/gravitino/policy/Policy.java | 3 +
.../dto/requests/PolicyContentTypeIdResolver.java | 91 +++++++++++
.../dto/requests/PolicyCreateRequest.java | 23 +--
.../dto/requests/PolicyUpdateRequest.java | 28 ++--
.../dto/requests/TestPolicyCreateRequest.java | 121 ++++++++++++++-
.../dto/requests/TestPolicyUpdatesRequest.java | 75 ++++++++-
docs/manage-policies-in-gravitino.md | 5 +
docs/open-api/policies.yaml | 167 +++++++++++++++++++--
.../server/web/rest/PolicyOperations.java | 12 --
.../server/web/rest/TestPolicyOperations.java | 145 ++++++++++++++++++
10 files changed, 619 insertions(+), 51 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 7b27b01fcd..c38ef4c9ac 100644
--- a/api/src/main/java/org/apache/gravitino/policy/Policy.java
+++ b/api/src/main/java/org/apache/gravitino/policy/Policy.java
@@ -60,6 +60,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..4e84bddd5f
--- /dev/null
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/PolicyContentTypeIdResolver.java
@@ -0,0 +1,91 @@
+/*
+ * 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.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 158bbb3e43..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,15 +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")
- })
+ defaultImpl = Void.class)
+ @JsonTypeIdResolver(PolicyContentTypeIdResolver.class)
private final PolicyContentDTO policyContent;
/**
@@ -99,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 2cc53c68d0..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;
@@ -121,7 +123,15 @@ public interface PolicyUpdateRequest extends RESTRequest {
public void validate() throws IllegalArgumentException {}
}
- /** The policy update request for updating a policy content. */
+ /**
+ * The policy update request for updating a policy content.
+ *
+ * <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
@ToString
@@ -135,15 +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")
- })
+ defaultImpl = Void.class)
+ @JsonTypeIdResolver(PolicyContentTypeIdResolver.class)
private final PolicyContentDTO newContent;
/**
@@ -165,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 f1edbba297..3c1ab3bfd9 100644
--- a/docs/manage-policies-in-gravitino.md
+++ b/docs/manage-policies-in-gravitino.md
@@ -23,6 +23,11 @@ The Python client does not cover policies, so the examples
below are REST and Ja
A policy needs a name and a type. Content carries the rules, the object types
the policy supports,
and optional properties. `supportedObjectTypes` cannot be changed after
creation.
+`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 7513b3bb72..56ffd9fb8e 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
@@ -538,6 +551,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]$'
+
+ IcebergCompactionContent:
+ type: object
+ description: Built-in Iceberg compaction thresholds, scoring weights,
and rewrite options.
+ properties:
+ minDataFileMse:
+ type: integer
+ format: int64
+ minimum: 0
+ minDeleteFileNumber:
+ type: integer
+ format: int64
+ minimum: 0
+ dataFileMseWeight:
+ type: integer
+ format: int64
+ minimum: 0
+ deleteFileNumberWeight:
+ type: integer
+ format: int64
+ minimum: 0
+ maxPartitionNum:
+ type: integer
+ format: int64
+ minimum: 1
+ rewriteOptions:
+ type: object
+ description: Rewrite options expanded to job.options.* during rule
generation.
+ additionalProperties:
+ type: string
+
PolicyCreateRequestBase:
type: object
description: A base policy create request without content
@@ -568,18 +621,40 @@ components:
required:
- content
properties:
+ policyType:
+ $ref: '#/components/schemas/CustomRequestPolicyType'
content:
$ref: '#/components/schemas/CustomPolicyContent'
+ IcebergCompactionPolicyCreateRequest:
+ description: A request to create an Iceberg compaction policy.
+ allOf:
+ - $ref: '#/components/schemas/PolicyCreateRequestBase'
+ - type: object
+ required:
+ - content
+ properties:
+ policyType:
+ $ref: '#/components/schemas/IcebergCompactionRequestPolicyType'
+ content:
+ $ref: '#/components/schemas/IcebergCompactionContent'
+
PolicyCreateRequest:
- description: A request to create a policy.
+ description: >-
+ A request to create a policy. Known `policyType` values are matched
case-insensitively;
+ the server stores and returns the canonical lowercase form.
type: object
oneOf:
- $ref: '#/components/schemas/CustomPolicyCreateRequest'
- discriminator:
- propertyName: policyType
- mapping:
- custom: '#/components/schemas/CustomPolicyCreateRequest'
+ - $ref: '#/components/schemas/IcebergCompactionPolicyCreateRequest'
+ example: {
+ "name": "compaction_upper",
+ "policyType": "SYSTEM_ICEBERG_COMPACTION",
+ "enabled": true,
+ "content": {
+ "maxPartitionNum": 5
+ }
+ }
PolicyUpdatesRequest:
@@ -660,16 +735,16 @@ components:
- updateContent
policyType:
type: string
- description: The type of the policy
+ description: >-
+ The type of the policy, either `custom` or
`system_iceberg_compaction`. Known
+ values are matched case-insensitively; the server stores and
returns the canonical
+ lowercase form. An unknown value is rejected with HTTP 400 naming
the type.
newContent:
type: object
description: The new content of the policy, its structure depends on
the 'policyType'.
oneOf:
- $ref: '#/components/schemas/CustomPolicyContent'
- discriminator:
- propertyName: policyType
- mapping:
- custom: '#/components/schemas/CustomPolicyContent'
+ - $ref: '#/components/schemas/IcebergCompactionContent'
example: {
"@type": "updateContent",
"policyType": "custom",
@@ -814,6 +889,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
+ }
+ }
+ ]
+ }
+
PolicyAssociate:
value: {
"policiesToAdd": ["my_policy1", "my_policy2"],
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 c1ad6c2766..a9b63111e1 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
@@ -160,7 +160,6 @@ public class PolicyOperations {
request.validate();
NameIdentifierUtil.checkMetadataObjectName(
NameIdentifierUtil.ofPolicy(metalake, request.getName()),
Entity.EntityType.POLICY);
- validateCreatePolicyType(request.getPolicyType());
PolicyEntity policy =
policyDispatcher.createPolicy(
metalake,
@@ -379,15 +378,4 @@ public class PolicyOperations {
return builder.build();
}
-
- 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 91232fc7a9..673ffb3eef 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
@@ -77,6 +77,8 @@ import org.glassfish.jersey.test.TestProperties;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mockito;
public class TestPolicyOperations extends BaseOperationsTest {
@@ -446,6 +448,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 =
@@ -590,6 +661,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);
