This is an automated email from the ASF dual-hosted git repository.
mchades pushed a commit to branch codex/issue-13540-ossie-import-export
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to
refs/heads/codex/issue-13540-ossie-import-export by this push:
new baf49af650 fix(semantic-model): address Ossie import/export review
comments
baf49af650 is described below
commit baf49af6508721038b030db04c76d5e90dbd4989
Author: mchades <[email protected]>
AuthorDate: Fri Oct 2 00:12:27 2026 +0800
fix(semantic-model): address Ossie import/export review comments
---
.../apache/gravitino/semantic/AIContextObject.java | 52 ++----
.../apache/gravitino/semantic/CustomExtension.java | 6 +
.../gravitino/semantic/SemanticModelCatalog.java | 13 ++
.../semantic/TestSemanticModelSupportingTypes.java | 31 +---
.../gravitino/api/semantic/ai_context_object.py | 39 ++---
.../test_semantic_model_supporting_types.py | 13 +-
.../gravitino/catalog/SemanticModelValidator.java | 22 ++-
.../OssieSemanticModelDocumentConverter.java | 122 ++++---------
.../TestSemanticModelOperationDispatcher.java | 64 ++++++-
.../catalog/TestSemanticModelValidator.java | 45 +++++
.../TestOssieSemanticModelDocumentConverter.java | 193 +++++++++++++++------
.../server/web/rest/SemanticModelOperations.java | 4 +
.../web/rest/TestSemanticModelOperations.java | 4 +-
13 files changed, 364 insertions(+), 244 deletions(-)
diff --git
a/api/src/main/java/org/apache/gravitino/semantic/AIContextObject.java
b/api/src/main/java/org/apache/gravitino/semantic/AIContextObject.java
index 6e9c156786..c804d509e5 100644
--- a/api/src/main/java/org/apache/gravitino/semantic/AIContextObject.java
+++ b/api/src/main/java/org/apache/gravitino/semantic/AIContextObject.java
@@ -37,8 +37,6 @@ import org.apache.gravitino.annotation.Evolving;
@Evolving
public final class AIContextObject {
- static final int MAX_ADDITIONAL_PROPERTY_NESTING_DEPTH = 100;
-
@Nullable private final String instructions;
@Nullable private final String[] synonyms;
@Nullable private final String[] examples;
@@ -206,9 +204,8 @@ public final class AIContextObject {
* <p>Values may be null, strings, booleans, JSON-compatible numbers, maps
with string keys,
* lists, or Java arrays. Integral numbers are normalized to {@link
BigInteger}, and decimal
* numbers are normalized to {@link BigDecimal} so their value semantics
remain stable across
- * JSON round trips. Nested maps, lists, and arrays may be at most {@value
- * #MAX_ADDITIONAL_PROPERTY_NESTING_DEPTH} levels deep. Property names
must not duplicate {@code
- * instructions}, {@code synonyms}, or {@code examples}.
+ * JSON round trips. Property names must not duplicate {@code
instructions}, {@code synonyms},
+ * or {@code examples}.
*
* @param additionalProperties The additional properties.
* @return This builder.
@@ -224,7 +221,7 @@ public final class AIContextObject {
* @return The immutable structured AI context.
* @throws IllegalArgumentException If a string array contains null, the
additional properties
* are null, a property duplicates a standard field, or a property
value is not
- * JSON-compatible or exceeds the supported nesting depth.
+ * JSON-compatible or contains a cyclic reference.
*/
public AIContextObject build() {
SemanticModelDefinition.validateNoNullElements("synonyms", synonyms);
@@ -245,16 +242,13 @@ public final class AIContextObject {
!isStandardProperty(name),
"additional property must not duplicate standard property: %s",
name);
- result.put(name, immutableJsonValue(entry.getValue(), name, visiting,
0));
+ result.put(name, immutableJsonValue(entry.getValue(), name, visiting));
}
return Collections.unmodifiableMap(result);
}
private static Object immutableJsonValue(
- @Nullable Object value,
- String path,
- IdentityHashMap<Object, Boolean> visiting,
- int containerDepth) {
+ @Nullable Object value, String path, IdentityHashMap<Object, Boolean>
visiting) {
if (value == null || value instanceof String || value instanceof Boolean) {
return value;
}
@@ -262,13 +256,13 @@ public final class AIContextObject {
return canonicalizeJsonNumber((Number) value, path);
}
if (value instanceof Map) {
- return immutableJsonMap((Map<?, ?>) value, path, visiting,
containerDepth + 1);
+ return immutableJsonMap((Map<?, ?>) value, path, visiting);
}
if (value instanceof List) {
- return immutableJsonList((List<?>) value, path, visiting, containerDepth
+ 1);
+ return immutableJsonList((List<?>) value, path, visiting);
}
if (value.getClass().isArray()) {
- return immutableJsonArray(value, path, visiting, containerDepth + 1);
+ return immutableJsonArray(value, path, visiting);
}
throw new IllegalArgumentException(
String.format(
@@ -277,8 +271,8 @@ public final class AIContextObject {
}
private static Map<String, Object> immutableJsonMap(
- Map<?, ?> value, String path, IdentityHashMap<Object, Boolean> visiting,
int containerDepth) {
- enterContainer(value, path, visiting, containerDepth);
+ Map<?, ?> value, String path, IdentityHashMap<Object, Boolean> visiting)
{
+ enterContainer(value, path, visiting);
try {
Map<String, Object> result = new LinkedHashMap<>();
for (Map.Entry<?, ?> entry : value.entrySet()) {
@@ -287,8 +281,7 @@ public final class AIContextObject {
"Additional property %s contains a map key that is not a string",
path);
String key = (String) entry.getKey();
- result.put(
- key, immutableJsonValue(entry.getValue(), path + "." + key,
visiting, containerDepth));
+ result.put(key, immutableJsonValue(entry.getValue(), path + "." + key,
visiting));
}
return Collections.unmodifiableMap(result);
} finally {
@@ -297,14 +290,12 @@ public final class AIContextObject {
}
private static List<Object> immutableJsonList(
- List<?> value, String path, IdentityHashMap<Object, Boolean> visiting,
int containerDepth) {
- enterContainer(value, path, visiting, containerDepth);
+ List<?> value, String path, IdentityHashMap<Object, Boolean> visiting) {
+ enterContainer(value, path, visiting);
try {
List<Object> result = new ArrayList<>(value.size());
for (int index = 0; index < value.size(); index++) {
- result.add(
- immutableJsonValue(
- value.get(index), path + "[" + index + "]", visiting,
containerDepth));
+ result.add(immutableJsonValue(value.get(index), path + "[" + index +
"]", visiting));
}
return Collections.unmodifiableList(result);
} finally {
@@ -313,15 +304,13 @@ public final class AIContextObject {
}
private static List<Object> immutableJsonArray(
- Object value, String path, IdentityHashMap<Object, Boolean> visiting,
int containerDepth) {
- enterContainer(value, path, visiting, containerDepth);
+ Object value, String path, IdentityHashMap<Object, Boolean> visiting) {
+ enterContainer(value, path, visiting);
try {
int length = Array.getLength(value);
List<Object> result = new ArrayList<>(length);
for (int index = 0; index < length; index++) {
- result.add(
- immutableJsonValue(
- Array.get(value, index), path + "[" + index + "]", visiting,
containerDepth));
+ result.add(immutableJsonValue(Array.get(value, index), path + "[" +
index + "]", visiting));
}
return Collections.unmodifiableList(result);
} finally {
@@ -370,12 +359,7 @@ public final class AIContextObject {
}
private static void enterContainer(
- Object value, String path, IdentityHashMap<Object, Boolean> visiting,
int containerDepth) {
- Preconditions.checkArgument(
- containerDepth <= MAX_ADDITIONAL_PROPERTY_NESTING_DEPTH,
- "Additional property %s exceeds maximum nesting depth of %s",
- path,
- MAX_ADDITIONAL_PROPERTY_NESTING_DEPTH);
+ Object value, String path, IdentityHashMap<Object, Boolean> visiting) {
Preconditions.checkArgument(
!visiting.containsKey(value), "Additional property %s contains a
cyclic value", path);
visiting.put(value, Boolean.TRUE);
diff --git
a/api/src/main/java/org/apache/gravitino/semantic/CustomExtension.java
b/api/src/main/java/org/apache/gravitino/semantic/CustomExtension.java
index 1b0b8a8e6c..828f62923b 100644
--- a/api/src/main/java/org/apache/gravitino/semantic/CustomExtension.java
+++ b/api/src/main/java/org/apache/gravitino/semantic/CustomExtension.java
@@ -27,6 +27,12 @@ import org.apache.gravitino.annotation.Evolving;
@Evolving
public final class CustomExtension {
+ /**
+ * The root-level extension vendor reserved for carrying Gravitino
properties in Ossie documents.
+ * Native Semantic Model definitions must not use this vendor in their root
custom extensions.
+ */
+ public static final String GRAVITINO_PROPERTIES_VENDOR =
"GRAVITINO_PROPERTIES";
+
private final String vendorName;
private final String data;
diff --git
a/api/src/main/java/org/apache/gravitino/semantic/SemanticModelCatalog.java
b/api/src/main/java/org/apache/gravitino/semantic/SemanticModelCatalog.java
index 577f5c4099..c16b5f309f 100644
--- a/api/src/main/java/org/apache/gravitino/semantic/SemanticModelCatalog.java
+++ b/api/src/main/java/org/apache/gravitino/semantic/SemanticModelCatalog.java
@@ -92,6 +92,15 @@ public interface SemanticModelCatalog {
* #createSemanticModel}; it does not replace an existing model. The
document is parsed according
* to its declared format, without falling back to another format.
*
+ * <p>REST clients send {@link OssieDocument#content()} as the raw request
body, without
+ * JSON-encoding the document string again. The {@code Content-Type} header
selects the parser:
+ * {@code application/json} for JSON, or {@code application/yaml}, {@code
application/x-yaml}, or
+ * {@code text/yaml} for YAML. If the header is absent, the REST endpoint
defaults to YAML.
+ *
+ * <p>A root {@link CustomExtension#GRAVITINO_PROPERTIES_VENDOR} extension
is consumed as model
+ * properties before creation. Its data must encode a JSON object with
string values, and at most
+ * one such extension is allowed. Other custom extensions are preserved.
+ *
* @param namespace The destination schema namespace.
* @param document The standalone Ossie document and its serialization
format.
* @return The created Semantic Model.
@@ -109,6 +118,10 @@ public interface SemanticModelCatalog {
/**
* Exports a Semantic Model as a standalone Apache Ossie document.
*
+ * <p>The {@link SemanticModel#PROPERTY_OSSIE_VERSION} property supplies the
document version.
+ * Other properties, if present, are serialized as a JSON object in the data
of a root {@link
+ * CustomExtension#GRAVITINO_PROPERTIES_VENDOR} extension.
+ *
* @param ident The Semantic Model identifier.
* @param format The requested serialization format.
* @return The serialized Ossie document and its format.
diff --git
a/api/src/test/java/org/apache/gravitino/semantic/TestSemanticModelSupportingTypes.java
b/api/src/test/java/org/apache/gravitino/semantic/TestSemanticModelSupportingTypes.java
index 5389f214e7..04307ebc2c 100644
---
a/api/src/test/java/org/apache/gravitino/semantic/TestSemanticModelSupportingTypes.java
+++
b/api/src/test/java/org/apache/gravitino/semantic/TestSemanticModelSupportingTypes.java
@@ -180,31 +180,6 @@ public class TestSemanticModelSupportingTypes {
assertEquals(equivalentContext.hashCode(),
equivalentLongContext.hashCode());
}
- @Test
- public void testAIContextAdditionalPropertyBounds() {
- Object maximumDepthValue = "value";
- for (int depth = 0; depth <
AIContextObject.MAX_ADDITIONAL_PROPERTY_NESTING_DEPTH; depth++) {
- maximumDepthValue = Map.of("nested", maximumDepthValue);
- }
- AIContextObject.builder()
- .withAdditionalProperties(Map.of("maximumDepth", maximumDepthValue))
- .build();
-
- Object excessiveDepthValue = Map.of("nested", maximumDepthValue);
- assertThrows(
- IllegalArgumentException.class,
- () ->
- AIContextObject.builder()
- .withAdditionalProperties(Map.of("excessiveDepth",
excessiveDepthValue))
- .build());
- assertThrows(
- IllegalArgumentException.class,
- () ->
- AIContextObject.builder()
- .withAdditionalProperties(Map.of("unsupportedNumber", new
AtomicInteger(1)))
- .build());
- }
-
@Test
public void testSupportingTypeValidation() {
assertThrows(IllegalArgumentException.class, () ->
DialectExpression.builder().build());
@@ -244,6 +219,12 @@ public class TestSemanticModelSupportingTypes {
AIContextObject.builder()
.withAdditionalProperties(Map.of("unsupported", new Object()))
.build());
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ AIContextObject.builder()
+ .withAdditionalProperties(Map.of("unsupportedNumber", new
AtomicInteger(1)))
+ .build());
assertThrows(
IllegalArgumentException.class,
() ->
diff --git a/clients/client-python/gravitino/api/semantic/ai_context_object.py
b/clients/client-python/gravitino/api/semantic/ai_context_object.py
index bee224a78a..b269b0842d 100644
--- a/clients/client-python/gravitino/api/semantic/ai_context_object.py
+++ b/clients/client-python/gravitino/api/semantic/ai_context_object.py
@@ -23,8 +23,6 @@ from typing import Any, Final, Optional
from gravitino.exceptions.base import IllegalArgumentException
from gravitino.utils.precondition import Precondition
-MAX_ADDITIONAL_PROPERTY_NESTING_DEPTH: Final[int] = 100
-
_STANDARD_PROPERTIES: Final[frozenset] = frozenset(
{"instructions", "synonyms", "examples"}
)
@@ -121,13 +119,11 @@ def _normalize_additional_properties(
name not in _STANDARD_PROPERTIES,
f"additional property must not duplicate standard property:
{name}",
)
- normalized[name] = _normalize_json_value(value, name, set(), 0)
+ normalized[name] = _normalize_json_value(value, name, set())
return normalized
-def _normalize_json_value(
- value: Any, path: str, visiting: set[int], container_depth: int
-) -> Any:
+def _normalize_json_value(value: Any, path: str, visiting: set[int]) -> Any:
if value is None or isinstance(value, (str, bool)):
return value
@@ -146,10 +142,10 @@ def _normalize_json_value(
return value
if isinstance(value, dict):
- return _normalize_json_dict(value, path, visiting, container_depth + 1)
+ return _normalize_json_dict(value, path, visiting)
if isinstance(value, (list, tuple)):
- return _normalize_json_list(value, path, visiting, container_depth + 1)
+ return _normalize_json_list(value, path, visiting)
raise IllegalArgumentException(
f"Additional property {path} has non-JSON-compatible value type: "
@@ -157,10 +153,8 @@ def _normalize_json_value(
)
-def _normalize_json_dict(
- value: dict, path: str, visiting: set[int], container_depth: int
-) -> dict[str, Any]:
- _enter_container(value, path, visiting, container_depth)
+def _normalize_json_dict(value: dict, path: str, visiting: set[int]) ->
dict[str, Any]:
+ _enter_container(value, path, visiting)
try:
normalized = {}
for key, item in value.items():
@@ -168,35 +162,24 @@ def _normalize_json_dict(
isinstance(key, str),
f"Additional property {path} contains a map key that is not a
string",
)
- normalized[key] = _normalize_json_value(
- item, f"{path}.{key}", visiting, container_depth
- )
+ normalized[key] = _normalize_json_value(item, f"{path}.{key}",
visiting)
return normalized
finally:
visiting.discard(id(value))
-def _normalize_json_list(
- value, path: str, visiting: set[int], container_depth: int
-) -> list[Any]:
- _enter_container(value, path, visiting, container_depth)
+def _normalize_json_list(value, path: str, visiting: set[int]) -> list[Any]:
+ _enter_container(value, path, visiting)
try:
return [
- _normalize_json_value(item, f"{path}[{index}]", visiting,
container_depth)
+ _normalize_json_value(item, f"{path}[{index}]", visiting)
for index, item in enumerate(value)
]
finally:
visiting.discard(id(value))
-def _enter_container(
- value: Any, path: str, visiting: set[int], container_depth: int
-) -> None:
- Precondition.check_argument(
- container_depth <= MAX_ADDITIONAL_PROPERTY_NESTING_DEPTH,
- f"Additional property {path} exceeds maximum nesting depth of "
- f"{MAX_ADDITIONAL_PROPERTY_NESTING_DEPTH}",
- )
+def _enter_container(value: Any, path: str, visiting: set[int]) -> None:
Precondition.check_argument(
id(value) not in visiting,
f"Additional property {path} contains a cyclic value",
diff --git
a/clients/client-python/tests/unittests/api/semantic/test_semantic_model_supporting_types.py
b/clients/client-python/tests/unittests/api/semantic/test_semantic_model_supporting_types.py
index 22fe689033..b26e0648a1 100644
---
a/clients/client-python/tests/unittests/api/semantic/test_semantic_model_supporting_types.py
+++
b/clients/client-python/tests/unittests/api/semantic/test_semantic_model_supporting_types.py
@@ -18,10 +18,7 @@
import unittest
from gravitino.api.semantic.ai_context import AIContext
-from gravitino.api.semantic.ai_context_object import (
- MAX_ADDITIONAL_PROPERTY_NESTING_DEPTH,
- AIContextObject,
-)
+from gravitino.api.semantic.ai_context_object import AIContextObject
from gravitino.api.semantic.custom_extension import CustomExtension
from gravitino.api.semantic.data_type import DataType
from gravitino.api.semantic.dialect_expression import DialectExpression
@@ -250,14 +247,6 @@ class TestSemanticModelSupportingTypes(unittest.TestCase):
):
AIContextObject(additional_properties={"nested": cyclic})
- def
test_ai_context_object_rejects_deeply_nested_additional_properties(self):
- value = "leaf"
- for _ in range(MAX_ADDITIONAL_PROPERTY_NESTING_DEPTH + 1):
- value = [value]
-
- with self.assertRaisesRegex(ValueError, "exceeds maximum nesting depth
of 100"):
- AIContextObject(additional_properties={"nested": value})
-
def test_ai_context_object_rejects_none_elements(self):
with self.assertRaisesRegex(ValueError, r"synonyms\[1\] must be a
string"):
AIContextObject(synonyms=["sales", None])
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/SemanticModelValidator.java
b/core/src/main/java/org/apache/gravitino/catalog/SemanticModelValidator.java
index 211585b6ae..bebda80bb1 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/SemanticModelValidator.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/SemanticModelValidator.java
@@ -18,11 +18,14 @@
*/
package org.apache.gravitino.catalog;
+import static
org.apache.gravitino.semantic.CustomExtension.GRAVITINO_PROPERTIES_VENDOR;
+
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.exceptions.IllegalSemanticModelException;
+import org.apache.gravitino.semantic.CustomExtension;
import org.apache.gravitino.semantic.Dataset;
import org.apache.gravitino.semantic.Field;
import org.apache.gravitino.semantic.Metric;
@@ -42,7 +45,7 @@ import org.apache.gravitino.semantic.SemanticModelDefinition;
* <p>The API value-object builders enforce value-level invariants, including
required values, array
* element validity, relationship column shapes, and expression dialect
uniqueness. This validator
* handles constraints that span value objects: name uniqueness, relationship
endpoint resolution,
- * and Semantic Model source identifier shape.
+ * Semantic Model source identifier shape, and the reserved root properties
extension vendor.
*
* <p>Validation is deterministic and performs no catalog I/O. Catalog-backed
source validation,
* including existence, columns, and authorization, must be completed by the
caller before this
@@ -54,7 +57,8 @@ final class SemanticModelValidator {
private SemanticModelValidator() {}
// TODO(#12594): Validate source existence, columns, and authorization in
the caller before
- // invoking this definition-only validator.
+ // invoking this definition-only validator. Cover whitespace-only source
segments in regression
+ // tests for create and replaceDefinition, and verify consistency with Ossie
import/export.
static void validateDefinition(@Nullable SemanticModelDefinition definition)
{
if (definition == null) {
throw invalid("$", "definition must not be null");
@@ -68,6 +72,20 @@ final class SemanticModelValidator {
validateRelationships(definition.relationships(), datasetNames);
validateMetrics(definition.metrics());
+ validateRootCustomExtensions(definition.customExtensions());
+ }
+
+ private static void validateRootCustomExtensions(@Nullable CustomExtension[]
extensions) {
+ if (extensions == null) {
+ return;
+ }
+ for (int index = 0; index < extensions.length; index++) {
+ if (GRAVITINO_PROPERTIES_VENDOR.equals(extensions[index].vendorName())) {
+ throw invalid(
+ "customExtensions[" + index + "].vendorName",
+ "'" + GRAVITINO_PROPERTIES_VENDOR + "' is reserved for Gravitino
properties");
+ }
+ }
}
private static void validateDataset(
diff --git
a/core/src/main/java/org/apache/gravitino/semantic/OssieSemanticModelDocumentConverter.java
b/core/src/main/java/org/apache/gravitino/semantic/OssieSemanticModelDocumentConverter.java
index ffeb0529e6..e13330844a 100644
---
a/core/src/main/java/org/apache/gravitino/semantic/OssieSemanticModelDocumentConverter.java
+++
b/core/src/main/java/org/apache/gravitino/semantic/OssieSemanticModelDocumentConverter.java
@@ -18,17 +18,14 @@
*/
package org.apache.gravitino.semantic;
+import static
org.apache.gravitino.semantic.CustomExtension.GRAVITINO_PROPERTIES_VENDOR;
import static
org.apache.gravitino.semantic.SemanticModel.DEFAULT_OSSIE_VERSION;
import static
org.apache.gravitino.semantic.SemanticModel.PROPERTY_OSSIE_VERSION;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.core.JsonFactory;
-import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.core.JsonToken;
-import com.fasterxml.jackson.core.StreamReadConstraints;
import com.fasterxml.jackson.core.StreamReadFeature;
-import com.fasterxml.jackson.core.util.JsonParserDelegate;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -54,19 +51,6 @@ import
org.apache.gravitino.exceptions.IllegalSemanticModelException;
/** Converts between standalone Apache Ossie documents and Gravitino Semantic
Models. */
public final class OssieSemanticModelDocumentConverter {
- /** Maximum accepted document length in characters. */
- public static final int MAX_DOCUMENT_LENGTH = 4 * 1024 * 1024;
-
- private static final String GRAVITINO_VENDOR = "GRAVITINO";
- private static final String INTERCHANGE_MARKER =
"_apache_gravitino_interchange";
- private static final int INTERCHANGE_MARKER_VERSION = 1;
- private static final int MAX_NESTING_DEPTH = 100;
- private static final StreamReadConstraints STREAM_READ_CONSTRAINTS =
- StreamReadConstraints.builder()
- .maxNestingDepth(MAX_NESTING_DEPTH)
- .maxStringLength(MAX_DOCUMENT_LENGTH)
- .build();
-
private static final Set<String> ROOT_PROPERTIES =
Set.of(
"version",
@@ -170,7 +154,6 @@ public final class OssieSemanticModelDocumentConverter {
private static ObjectMapper createJsonMapper() {
JsonFactory factory =
JsonFactory.builder().enable(StreamReadFeature.STRICT_DUPLICATE_DETECTION).build();
- factory.setStreamReadConstraints(STREAM_READ_CONSTRAINTS);
return JsonMapper.builder(factory)
.enable(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS)
.enable(DeserializationFeature.USE_BIG_INTEGER_FOR_INTS)
@@ -184,7 +167,6 @@ public final class OssieSemanticModelDocumentConverter {
.enable(StreamReadFeature.STRICT_DUPLICATE_DETECTION)
.disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER)
.build();
- factory.setStreamReadConstraints(STREAM_READ_CONSTRAINTS);
return new ObjectMapper(factory)
.enable(DeserializationFeature.FAIL_ON_TRAILING_TOKENS)
.enable(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS)
@@ -197,14 +179,11 @@ public final class OssieSemanticModelDocumentConverter {
if (StringUtils.isBlank(content)) {
throw new IllegalSemanticModelException("Apache Ossie document must not
be empty");
}
- if (content.length() > MAX_DOCUMENT_LENGTH) {
- throw new IllegalSemanticModelException(
- "Apache Ossie document exceeds the maximum length of %s characters",
MAX_DOCUMENT_LENGTH);
- }
-
try {
ObjectMapper mapper = document.format() == OssieFormat.JSON ?
JSON_MAPPER : YAML_MAPPER;
- JsonNode parsed = readDocumentTree(mapper, content);
+ // Check the whole document, without enabling this on nested DTO
deserializers.
+ JsonNode parsed =
+
mapper.reader().with(DeserializationFeature.FAIL_ON_TRAILING_TOKENS).readTree(content);
if (!(parsed instanceof ObjectNode)) {
throw invalid("$", "document root must be an object");
}
@@ -215,22 +194,6 @@ public final class OssieSemanticModelDocumentConverter {
}
}
- private static JsonNode readDocumentTree(ObjectMapper mapper, String
content) throws IOException {
- // Older YAML parsers do not enforce StreamReadConstraints on nesting
depth.
- try (JsonParser parser =
- new JsonParserDelegate(mapper.createParser(content)) {
- @Override
- public JsonToken nextToken() throws IOException {
- JsonToken token = super.nextToken();
-
STREAM_READ_CONSTRAINTS.validateNestingDepth(getParsingContext().getNestingDepth());
- return token;
- }
- }) {
- // Check the whole document, without enabling this on nested DTO
deserializers.
- return
mapper.reader().with(DeserializationFeature.FAIL_ON_TRAILING_TOKENS).readTree(parser);
- }
- }
-
private static SemanticModelCreateRequest toCreateRequest(ObjectNode root) {
validateObject(root, "$", ROOT_PROPERTIES);
validateOptionalText(root, "name", "$");
@@ -265,7 +228,6 @@ public final class OssieSemanticModelDocumentConverter {
SemanticModelCreateRequest request =
JSON_MAPPER.treeToValue(requestNode,
SemanticModelCreateRequest.class);
request.validate();
- request.toDefinition();
return request;
} catch (JsonProcessingException | IllegalArgumentException e) {
throw new IllegalSemanticModelException(
@@ -615,20 +577,15 @@ public final class OssieSemanticModelDocumentConverter {
} else {
throw invalid("$.custom_extensions", "must be an array");
}
- ensureNoReservedMarker(extensions);
+ ensureNoPropertiesExtension(extensions);
if (interchangeProperties.isEmpty()) {
return;
}
- ObjectNode payload = JSON_MAPPER.createObjectNode();
- ObjectNode marker = payload.putObject(INTERCHANGE_MARKER);
- marker.put("version", INTERCHANGE_MARKER_VERSION);
- marker.set("properties", JSON_MAPPER.valueToTree(interchangeProperties));
-
ObjectNode extension = extensions.addObject();
- extension.put("vendor_name", GRAVITINO_VENDOR);
- extension.put("data", writeExtensionData(payload));
+ extension.put("vendor_name", GRAVITINO_PROPERTIES_VENDOR);
+ extension.put("data",
writeExtensionData(JSON_MAPPER.valueToTree(interchangeProperties)));
}
private static Map<String, String> extractProperties(ObjectNode root) {
@@ -640,52 +597,41 @@ public final class OssieSemanticModelDocumentConverter {
ArrayNode extensions = (ArrayNode) extensionsNode;
Map<String, String> properties = new LinkedHashMap<>();
boolean found = false;
+ int index = 0;
Iterator<JsonNode> iterator = extensions.iterator();
while (iterator.hasNext()) {
JsonNode candidate = iterator.next();
+ String path = "$.custom_extensions[" + index++ + "]";
if (!(candidate instanceof ObjectNode)) {
continue;
}
ObjectNode extension = (ObjectNode) candidate;
- if
(!GRAVITINO_VENDOR.equals(extension.path("vendor_name").asText(null))) {
+ if
(!GRAVITINO_PROPERTIES_VENDOR.equals(extension.path("vendor_name").asText(null)))
{
continue;
}
- ObjectNode payload =
parseExtensionData(extension.path("data").asText(null));
- JsonNode markerNode = payload == null ? null :
payload.get(INTERCHANGE_MARKER);
- if (markerNode == null) {
- continue;
- }
if (found) {
- throw invalid("$.custom_extensions", "contains multiple Gravitino
interchange markers");
- }
- if (!(markerNode instanceof ObjectNode)
- || markerNode.path("version").asInt(-1) != INTERCHANGE_MARKER_VERSION
- || !(markerNode.get("properties") instanceof ObjectNode)) {
throw invalid(
- "$.custom_extensions", "contains an unsupported Gravitino
interchange marker");
+ "$.custom_extensions",
+ "contains multiple " + GRAVITINO_PROPERTIES_VENDOR + "
extensions");
}
- ObjectNode propertyNode = (ObjectNode) markerNode.get("properties");
+ validateObject(extension, path, CUSTOM_EXTENSION_PROPERTIES);
+ ObjectNode propertyNode = parsePropertiesData(extension.get("data"),
path + ".data");
propertyNode
.fields()
.forEachRemaining(
entry -> {
if (!entry.getValue().isTextual()) {
throw invalid(
- "$.custom_extensions",
+ path + ".data",
"Gravitino property '" + entry.getKey() + "' must be a
string");
}
properties.put(entry.getKey(), entry.getValue().textValue());
});
found = true;
- payload.remove(INTERCHANGE_MARKER);
- if (payload.isEmpty()) {
- iterator.remove();
- } else {
- extension.put("data", writeExtensionData(payload));
- }
+ iterator.remove();
}
if (extensions.isEmpty()) {
root.remove("custom_extensions");
@@ -693,31 +639,33 @@ public final class OssieSemanticModelDocumentConverter {
return properties;
}
- private static void ensureNoReservedMarker(ArrayNode extensions) {
- for (JsonNode candidate : extensions) {
- if (!(candidate instanceof ObjectNode)
- ||
!GRAVITINO_VENDOR.equals(candidate.path("vendor_name").asText(null))) {
- continue;
- }
- ObjectNode payload =
parseExtensionData(candidate.path("data").asText(null));
- if (payload != null && payload.has(INTERCHANGE_MARKER)) {
+ private static void ensureNoPropertiesExtension(ArrayNode extensions) {
+ for (int index = 0; index < extensions.size(); index++) {
+ if (GRAVITINO_PROPERTIES_VENDOR.equals(
+ extensions.get(index).path("vendor_name").asText(null))) {
throw invalid(
- "$.custom_extensions", "already contains a reserved Gravitino
interchange marker");
+ "$.custom_extensions[" + index + "].vendor_name",
+ "'" + GRAVITINO_PROPERTIES_VENDOR + "' is reserved for Gravitino
properties");
}
}
}
- @Nullable
- private static ObjectNode parseExtensionData(@Nullable String data) {
- if (data == null) {
- return null;
+ private static ObjectNode parsePropertiesData(@Nullable JsonNode data,
String path) {
+ if (data == null || !data.isTextual()) {
+ throw invalid(path, "must be a JSON string containing an object with
string values");
}
try {
JsonNode parsed =
-
JSON_MAPPER.reader().with(DeserializationFeature.FAIL_ON_TRAILING_TOKENS).readTree(data);
- return parsed instanceof ObjectNode ? (ObjectNode) parsed : null;
+ JSON_MAPPER
+ .reader()
+ .with(DeserializationFeature.FAIL_ON_TRAILING_TOKENS)
+ .readTree(data.textValue());
+ if (!(parsed instanceof ObjectNode)) {
+ throw invalid(path, "must contain a JSON object with string values");
+ }
+ return (ObjectNode) parsed;
} catch (JsonProcessingException e) {
- return null;
+ throw invalid(path, "cannot parse Gravitino properties: " +
e.getOriginalMessage());
}
}
@@ -726,7 +674,7 @@ public final class OssieSemanticModelDocumentConverter {
return JSON_MAPPER.writeValueAsString(payload);
} catch (JsonProcessingException e) {
throw new IllegalSemanticModelException(
- e, "Cannot serialize Gravitino interchange extension: %s",
e.getOriginalMessage());
+ e, "Cannot serialize Gravitino properties extension: %s",
e.getOriginalMessage());
}
}
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestSemanticModelOperationDispatcher.java
b/core/src/test/java/org/apache/gravitino/catalog/TestSemanticModelOperationDispatcher.java
index 06d5d051d8..8421bc6472 100644
---
a/core/src/test/java/org/apache/gravitino/catalog/TestSemanticModelOperationDispatcher.java
+++
b/core/src/test/java/org/apache/gravitino/catalog/TestSemanticModelOperationDispatcher.java
@@ -21,6 +21,7 @@ package org.apache.gravitino.catalog;
import static org.apache.gravitino.Configs.TREE_LOCK_CLEAN_INTERVAL;
import static org.apache.gravitino.Configs.TREE_LOCK_MAX_NODE_IN_MEMORY;
import static org.apache.gravitino.Configs.TREE_LOCK_MIN_NODE_IN_MEMORY;
+import static
org.apache.gravitino.semantic.CustomExtension.GRAVITINO_PROPERTIES_VENDOR;
import static
org.apache.gravitino.semantic.SemanticModel.DEFAULT_OSSIE_VERSION;
import static
org.apache.gravitino.semantic.SemanticModel.PROPERTY_OSSIE_VERSION;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
@@ -49,6 +50,7 @@ import
org.apache.gravitino.exceptions.NoSuchSemanticModelException;
import org.apache.gravitino.exceptions.SemanticModelAlreadyExistsException;
import org.apache.gravitino.lock.LockManager;
import org.apache.gravitino.secret.SecretManager;
+import org.apache.gravitino.semantic.CustomExtension;
import org.apache.gravitino.semantic.Dataset;
import org.apache.gravitino.semantic.OssieDocument;
import org.apache.gravitino.semantic.OssieFormat;
@@ -142,6 +144,42 @@ public class TestSemanticModelOperationDispatcher {
verify(catalogManager, never()).loadCatalog(METADATA_CATALOG_IDENT);
}
+ @Test
+ public void testCreateRejectsReservedPropertiesExtensionWithoutPersisting() {
+ IllegalSemanticModelException exception =
+ assertThrows(
+ IllegalSemanticModelException.class,
+ () ->
+ dispatcher.createSemanticModel(
+ MODEL_IDENT, "Sales", definitionWithReservedExtension(),
Map.of()));
+
+ assertTrue(exception.getMessage().contains("reserved for Gravitino
properties"));
+ assertFalse(dispatcher.semanticModelExists(MODEL_IDENT));
+ }
+
+ @Test
+ public void
testReplaceRejectsReservedPropertiesExtensionWithoutPersistingOtherChanges() {
+ SemanticModel original =
+ dispatcher.createSemanticModel(
+ MODEL_IDENT, "Original", validDefinition(), Map.of("owner",
"sales"));
+
+ IllegalSemanticModelException exception =
+ assertThrows(
+ IllegalSemanticModelException.class,
+ () ->
+ dispatcher.alterSemanticModel(
+ MODEL_IDENT,
+ SemanticModelChange.updateComment("Must not persist"),
+ SemanticModelChange.setProperty("owner", "changed"),
+
SemanticModelChange.replaceDefinition(definitionWithReservedExtension())));
+
+ assertTrue(exception.getMessage().contains("reserved for Gravitino
properties"));
+ SemanticModel loaded = dispatcher.loadSemanticModel(MODEL_IDENT);
+ assertEquals(original.comment(), loaded.comment());
+ assertEquals(original.properties(), loaded.properties());
+ assertEquals(original.definition(), loaded.definition());
+ }
+
@Test
public void testAlterRejectsBlankOssieVersionBeforeCatalogLookup() {
assertThrows(
@@ -286,11 +324,23 @@ public class TestSemanticModelOperationDispatcher {
@ParameterizedTest
@EnumSource(OssieFormat.class)
public void testOssieExportImportRoundTrip(OssieFormat format) {
+ CustomExtension ordinary =
+ CustomExtension.builder()
+ .withVendorName("GRAVITINO")
+ .withData("{\"_apache_gravitino_interchange\":1}")
+ .build();
+ SemanticModelDefinition baseDefinition = validDefinition();
+ SemanticModelDefinition definition =
+ SemanticModelDefinition.builder()
+ .withDatasets(baseDefinition.datasets())
+ .withRelationships(baseDefinition.relationships())
+ .withCustomExtensions(new CustomExtension[] {ordinary})
+ .build();
SemanticModel original =
dispatcher.createSemanticModel(
MODEL_IDENT,
"Sales",
- validDefinition(),
+ definition,
Map.of("domain", "sales", PROPERTY_OSSIE_VERSION,
"future-version"));
OssieDocument document = dispatcher.exportOssieSemanticModel(MODEL_IDENT,
format);
@@ -385,6 +435,18 @@ public class TestSemanticModelOperationDispatcher {
.build();
}
+ private static SemanticModelDefinition definitionWithReservedExtension() {
+ CustomExtension extension =
+ CustomExtension.builder()
+ .withVendorName(GRAVITINO_PROPERTIES_VENDOR)
+ .withData("not JSON")
+ .build();
+ return SemanticModelDefinition.builder()
+ .withDatasets(validDefinition().datasets())
+ .withCustomExtensions(new CustomExtension[] {extension})
+ .build();
+ }
+
private static Dataset dataset(
String name, String source, String[] primaryKey, String[][] uniqueKeys) {
return Dataset.builder()
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestSemanticModelValidator.java
b/core/src/test/java/org/apache/gravitino/catalog/TestSemanticModelValidator.java
index cff181337a..757354f7d7 100644
---
a/core/src/test/java/org/apache/gravitino/catalog/TestSemanticModelValidator.java
+++
b/core/src/test/java/org/apache/gravitino/catalog/TestSemanticModelValidator.java
@@ -18,6 +18,7 @@
*/
package org.apache.gravitino.catalog;
+import static
org.apache.gravitino.semantic.CustomExtension.GRAVITINO_PROPERTIES_VENDOR;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -105,6 +106,50 @@ public class TestSemanticModelValidator {
"datasets[0].source: must contain exactly catalog.schema.name, but was
'sales.orders'");
}
+ @Test
+ public void testRejectsReservedRootExtensionRegardlessOfData() {
+ for (String data : new String[] {"{}", "{\"owner\":\"analytics\"}", "not
JSON"}) {
+ CustomExtension reserved =
+ CustomExtension.builder()
+ .withVendorName(GRAVITINO_PROPERTIES_VENDOR)
+ .withData(data)
+ .build();
+ SemanticModelDefinition definition =
+ SemanticModelDefinition.builder()
+ .withDatasets(new Dataset[] {dataset("orders")})
+ .withCustomExtensions(new CustomExtension[] {reserved})
+ .build();
+
+ assertInvalid(
+ definition,
+ "customExtensions[0].vendorName: 'GRAVITINO_PROPERTIES' is reserved
for Gravitino properties");
+ }
+ }
+
+ @Test
+ public void testPreservesOrdinaryRootAndNestedPropertiesVendorExtensions() {
+ CustomExtension nested =
+ CustomExtension.builder()
+ .withVendorName(GRAVITINO_PROPERTIES_VENDOR)
+ .withData("not JSON")
+ .build();
+ Dataset orders =
+ Dataset.builder()
+ .withName("orders")
+ .withSource(NameIdentifier.of("sales", "mart", "orders"))
+ .withCustomExtensions(new CustomExtension[] {nested})
+ .build();
+ CustomExtension ordinary =
+ CustomExtension.builder().withVendorName("GRAVITINO").withData("not
JSON").build();
+ SemanticModelDefinition definition =
+ SemanticModelDefinition.builder()
+ .withDatasets(new Dataset[] {orders})
+ .withCustomExtensions(new CustomExtension[] {ordinary})
+ .build();
+
+ assertDoesNotThrow(() ->
SemanticModelValidator.validateDefinition(definition));
+ }
+
@Test
public void testFieldNamesAreUniquePerDataset() {
Dataset duplicateFields =
diff --git
a/core/src/test/java/org/apache/gravitino/semantic/TestOssieSemanticModelDocumentConverter.java
b/core/src/test/java/org/apache/gravitino/semantic/TestOssieSemanticModelDocumentConverter.java
index 43eaa6634d..3bb74e6c1c 100644
---
a/core/src/test/java/org/apache/gravitino/semantic/TestOssieSemanticModelDocumentConverter.java
+++
b/core/src/test/java/org/apache/gravitino/semantic/TestOssieSemanticModelDocumentConverter.java
@@ -18,10 +18,12 @@
*/
package org.apache.gravitino.semantic;
+import static
org.apache.gravitino.semantic.CustomExtension.GRAVITINO_PROPERTIES_VENDOR;
import static
org.apache.gravitino.semantic.SemanticModel.DEFAULT_OSSIE_VERSION;
import static
org.apache.gravitino.semantic.SemanticModel.PROPERTY_OSSIE_VERSION;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -37,6 +39,8 @@ import
org.apache.gravitino.exceptions.IllegalSemanticModelException;
import org.apache.gravitino.meta.AuditInfo;
import org.apache.gravitino.meta.SemanticModelEntity;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
/** Tests standalone Apache Ossie document import, export, and conversion
diagnostics. */
public class TestOssieSemanticModelDocumentConverter {
@@ -76,8 +80,8 @@ public class TestOssieSemanticModelDocumentConverter {
custom_extensions:
- vendor_name: EXAMPLE
data: '{"certified":true}'
- - vendor_name: GRAVITINO
- data:
'{"_apache_gravitino_interchange":{"version":1,"properties":{"domain":"sales"}}}'
+ - vendor_name: GRAVITINO_PROPERTIES
+ data: '{"domain":"sales"}'
""";
SemanticModelCreateRequest yamlRequest =
@@ -143,11 +147,11 @@ public class TestOssieSemanticModelDocumentConverter {
assertFalse(json.has("definition"));
assertFalse(json.has("properties"));
assertFalse(json.has("audit"));
- assertEquals("GRAVITINO",
json.at("/custom_extensions/1/vendor_name").textValue());
- JsonNode marker =
JSON_MAPPER.readTree(json.at("/custom_extensions/1/data").textValue());
assertEquals(
- "sales",
marker.at("/_apache_gravitino_interchange/properties/domain").textValue());
-
assertFalse(marker.at("/_apache_gravitino_interchange/properties").has(PROPERTY_OSSIE_VERSION));
+ GRAVITINO_PROPERTIES_VENDOR,
json.at("/custom_extensions/1/vendor_name").textValue());
+ JsonNode properties =
JSON_MAPPER.readTree(json.at("/custom_extensions/1/data").textValue());
+ assertEquals(JSON_MAPPER.readTree("{\"domain\":\"sales\"}"), properties);
+ assertFalse(properties.has(PROPERTY_OSSIE_VERSION));
SemanticModelCreateRequest jsonRoundTrip =
OssieSemanticModelDocumentConverter.importDocument(jsonDocument);
@@ -251,9 +255,6 @@ public class TestOssieSemanticModelDocumentConverter {
"version: 0.2.0.dev0\nname: first\ndatasets: []\n"
+ "---\nversion: 0.2.0.dev0\nname: second\ndatasets: []\n",
"Trailing token");
- assertInvalid(
- "x".repeat(OssieSemanticModelDocumentConverter.MAX_DOCUMENT_LENGTH +
1),
- "exceeds the maximum length");
}
@Test
@@ -309,19 +310,6 @@ public class TestOssieSemanticModelDocumentConverter {
OssieDocument.json("{\"name\":\"first\",\"name\":\"second\"}"),
"Duplicate field 'name'");
assertInvalid(OssieDocument.json("{} {}"), "Trailing token");
assertInvalid(OssieDocument.json("{\"name\":}"), "Cannot parse Apache
Ossie JSON");
- assertInvalid(
-
OssieDocument.json("x".repeat(OssieSemanticModelDocumentConverter.MAX_DOCUMENT_LENGTH
+ 1)),
- "exceeds the maximum length");
- }
-
- @Test
- public void testLimitsDocumentNesting() {
- String arrays = "[".repeat(110) + "0" + "]".repeat(110);
- assertInvalid(OssieDocument.json("{\"nested\":" + arrays + "}"), "nesting
depth");
- assertInvalid(OssieDocument.yaml("nested: " + arrays), "nesting depth");
- String objects = "{\"nested\":".repeat(110) + "0" + "}".repeat(110);
- assertInvalid(OssieDocument.json(objects), "nesting depth");
- assertInvalid(OssieDocument.yaml(objects), "nesting depth");
}
@Test
@@ -334,45 +322,141 @@ public class TestOssieSemanticModelDocumentConverter {
- name: orders
source: sales.mart.orders
custom_extensions:
- - vendor_name: GRAVITINO
- data:
'{"_apache_gravitino_interchange":{"version":1,"properties":{"ossie-version":"extension-version"}}}'
+ - vendor_name: GRAVITINO_PROPERTIES
+ data: '{"ossie-version":"extension-version"}'
""";
assertInvalid(document, "property 'ossie-version' conflicts with
$.version");
}
- @Test
- public void testLimitsNestedCustomExtensionParsing() throws Exception {
- String nested = "{}";
- for (int depth = 0; depth < 110; depth++) {
- nested = "{\"nested\":" + nested + "}";
+ @ParameterizedTest
+ @EnumSource(OssieFormat.class)
+ public void testRejectsInvalidPropertiesExtensionData(OssieFormat format)
throws Exception {
+ String[] payloads = {
+ "",
+ "{",
+ "{} {}",
+ "{\"domain\":\"sales\",\"domain\":\"other\"}",
+ "null",
+ "[]",
+ "123",
+ "\"sales\""
+ };
+ for (String payload : payloads) {
+ assertInvalid(
+ ossieDocument(documentWithExtension(GRAVITINO_PROPERTIES_VENDOR,
payload), format),
+ "$.custom_extensions[0].data");
+ }
+ for (String value : new String[] {"null", "true", "1", "[]", "{}"}) {
+ assertInvalid(
+ ossieDocument(
+ documentWithExtension(GRAVITINO_PROPERTIES_VENDOR,
"{\"domain\":" + value + "}"),
+ format),
+ "Gravitino property 'domain' must be a string");
}
- String payload =
-
"{\"_apache_gravitino_interchange\":{\"version\":1,\"properties\":{\"domain\":\"sales\"}},\"nested\":"
- + nested
- + "}";
-
- SemanticModelCreateRequest request = importWithGravitinoExtension(payload);
- assertFalse(request.getProperties().containsKey("domain"));
- assertEquals(payload, request.toDefinition().customExtensions()[0].data());
}
@Test
- public void
testPreservesMalformedCustomExtensionJsonWithoutExtractingProperties()
- throws Exception {
- String marker =
-
"{\"_apache_gravitino_interchange\":{\"version\":1,\"properties\":{\"domain\":\"sales\"}}}";
+ public void testValidatesPropertiesExtensionBeforeConsumingIt() throws
Exception {
+ ObjectNode document = documentWithExtension(GRAVITINO_PROPERTIES_VENDOR,
"{}");
+ ObjectNode extension = (ObjectNode)
document.path("custom_extensions").get(0);
+ extension.put("extra", "not allowed");
+ assertInvalid(ossieDocument(document, OssieFormat.JSON),
"$.custom_extensions[0].extra");
+ extension.remove("extra");
+ extension.put("data", 123);
+ assertInvalid(ossieDocument(document, OssieFormat.JSON),
"$.custom_extensions[0].data");
+ extension.remove("data");
+ assertInvalid(ossieDocument(document, OssieFormat.JSON),
"$.custom_extensions[0].data");
+ }
+
+ @ParameterizedTest
+ @EnumSource(OssieFormat.class)
+ public void testRejectsDuplicatePropertiesExtensions(OssieFormat format)
throws Exception {
+ ObjectNode document = documentWithExtension(GRAVITINO_PROPERTIES_VENDOR,
"{}");
+ document
+ .withArray("custom_extensions")
+ .add(document.path("custom_extensions").get(0).deepCopy());
+ assertInvalid(
+ ossieDocument(document, format), "contains multiple
GRAVITINO_PROPERTIES extensions");
+ }
+
+ @ParameterizedTest
+ @EnumSource(OssieFormat.class)
+ public void testConsumesEmptyPropertiesExtension(OssieFormat format) throws
Exception {
+ SemanticModelCreateRequest request =
+ OssieSemanticModelDocumentConverter.importDocument(
+ ossieDocument(documentWithExtension(GRAVITINO_PROPERTIES_VENDOR,
"{}"), format));
+ assertEquals(Map.of(PROPERTY_OSSIE_VERSION, DEFAULT_OSSIE_VERSION),
request.getProperties());
+ assertNull(request.toDefinition().customExtensions());
+ }
+
+ @ParameterizedTest
+ @EnumSource(OssieFormat.class)
+ public void testPreservesOrdinaryGravitinoExtensions(OssieFormat format)
throws Exception {
String[] payloads = {
- marker + " {}",
- marker.substring(0, marker.length() - 1) +
",\"_apache_gravitino_interchange\":{}}"
+ "{\"_apache_gravitino_interchange\":1}",
+
"{\"_apache_gravitino_interchange\":{\"version\":1,\"properties\":{\"domain\":\"sales\"}}}",
+ "{\"domain\":\"sales\"}",
+ "{\"domain\":\"sales\",\"domain\":\"other\"}",
+ "{} {}",
+ "not JSON"
};
for (String payload : payloads) {
- SemanticModelCreateRequest request =
importWithGravitinoExtension(payload);
- assertFalse(request.getProperties().containsKey("domain"));
+ SemanticModelCreateRequest request =
+ OssieSemanticModelDocumentConverter.importDocument(
+ ossieDocument(documentWithExtension("GRAVITINO", payload),
format));
+ assertEquals(Map.of(PROPERTY_OSSIE_VERSION, DEFAULT_OSSIE_VERSION),
request.getProperties());
assertEquals(payload,
request.toDefinition().customExtensions()[0].data());
+
+ SemanticModel model =
+ semanticModel(
+ request.toDefinition(),
+ Map.of("owner", "analytics", PROPERTY_OSSIE_VERSION,
DEFAULT_OSSIE_VERSION));
+ SemanticModelCreateRequest roundTrip =
+ OssieSemanticModelDocumentConverter.importDocument(
+ OssieSemanticModelDocumentConverter.exportDocument(model,
format));
+ assertEquals(model.definition(), roundTrip.toDefinition());
+ assertEquals(model.properties(), roundTrip.getProperties());
}
}
+ @ParameterizedTest
+ @EnumSource(OssieFormat.class)
+ public void testPreservesNestedPropertiesVendorExtension(OssieFormat format)
throws Exception {
+ CustomExtension nested =
+ CustomExtension.builder()
+ .withVendorName(GRAVITINO_PROPERTIES_VENDOR)
+ .withData("{\"nested\":true}")
+ .build();
+ Dataset dataset =
+ Dataset.builder()
+ .withName("orders")
+ .withSource(NameIdentifier.of("sales", "mart", "orders"))
+ .withCustomExtensions(new CustomExtension[] {nested})
+ .build();
+ SemanticModel model =
+ semanticModel(
+ SemanticModelDefinition.builder().withDatasets(new Dataset[]
{dataset}).build(),
+ Map.of("owner", "analytics", PROPERTY_OSSIE_VERSION,
DEFAULT_OSSIE_VERSION));
+
+ SemanticModelCreateRequest roundTrip =
+ OssieSemanticModelDocumentConverter.importDocument(
+ OssieSemanticModelDocumentConverter.exportDocument(model, format));
+ assertEquals(model.definition(), roundTrip.toDefinition());
+ assertEquals(model.properties(), roundTrip.getProperties());
+ }
+
+ @Test
+ public void testDoesNotExportPropertiesExtensionForVersionOnly() throws
Exception {
+ SemanticModel model =
+ semanticModel(definition(), Map.of(PROPERTY_OSSIE_VERSION,
DEFAULT_OSSIE_VERSION));
+ JsonNode document =
+ JSON_MAPPER.readTree(
+ OssieSemanticModelDocumentConverter.exportDocument(model,
OssieFormat.JSON).content());
+ assertEquals(1, document.path("custom_extensions").size());
+ assertEquals("EXAMPLE",
document.at("/custom_extensions/0/vendor_name").textValue());
+ }
+
@Test
public void testRejectsBlankOssieVersionPropertyOnExport() {
IllegalSemanticModelException exception =
@@ -390,8 +474,8 @@ public class TestOssieSemanticModelDocumentConverter {
public void testRejectsReservedExtensionCollisionOnExport() {
CustomExtension reserved =
CustomExtension.builder()
- .withVendorName("GRAVITINO")
-
.withData("{\"_apache_gravitino_interchange\":{\"version\":1,\"properties\":{}}}")
+ .withVendorName(GRAVITINO_PROPERTIES_VENDOR)
+ .withData("{}")
.build();
SemanticModelDefinition definition =
SemanticModelDefinition.builder()
@@ -405,7 +489,7 @@ public class TestOssieSemanticModelDocumentConverter {
() ->
OssieSemanticModelDocumentConverter.exportDocument(
semanticModel(definition, Map.of()), OssieFormat.JSON));
- assertTrue(exception.getMessage().contains("reserved Gravitino interchange
marker"));
+ assertTrue(exception.getMessage().contains("reserved for Gravitino
properties"));
}
@Test
@@ -441,8 +525,7 @@ public class TestOssieSemanticModelDocumentConverter {
"TRINO",
roundTrip.toDefinition().metrics()[0].expression().dialects()[0].dialect());
}
- private static SemanticModelCreateRequest
importWithGravitinoExtension(String payload)
- throws Exception {
+ private static ObjectNode documentWithExtension(String vendorName, String
payload) {
ObjectNode root = JSON_MAPPER.createObjectNode();
root.put("version", DEFAULT_OSSIE_VERSION);
root.put("name", "sales");
@@ -450,10 +533,14 @@ public class TestOssieSemanticModelDocumentConverter {
dataset.put("name", "orders");
dataset.put("source", "sales.mart.orders");
ObjectNode extension = root.putArray("custom_extensions").addObject();
- extension.put("vendor_name", "GRAVITINO");
+ extension.put("vendor_name", vendorName);
extension.put("data", payload);
- return OssieSemanticModelDocumentConverter.importDocument(
- OssieDocument.json(JSON_MAPPER.writeValueAsString(root)));
+ return root;
+ }
+
+ private static OssieDocument ossieDocument(ObjectNode root, OssieFormat
format) throws Exception {
+ String content = JSON_MAPPER.writeValueAsString(root);
+ return format == OssieFormat.JSON ? OssieDocument.json(content) :
OssieDocument.yaml(content);
}
private static SemanticModel semanticModel(
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/SemanticModelOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/SemanticModelOperations.java
index 3dd3d5050d..88d250556b 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/SemanticModelOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/SemanticModelOperations.java
@@ -167,6 +167,10 @@ public class SemanticModelOperations {
/**
* Imports a standalone Apache Ossie YAML or JSON document as a Semantic
Model.
*
+ * <p>The request body is raw document text, not a JSON-encoded string. The
{@code Content-Type}
+ * header selects the document format; an absent header defaults to YAML.
Parsing does not fall
+ * back to another format if the document is invalid for the selected parser.
+ *
* @param metalake The metalake name.
* @param catalog The catalog name.
* @param schema The schema name.
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestSemanticModelOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestSemanticModelOperations.java
index 85cacbd7db..4d6e6dd177 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestSemanticModelOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestSemanticModelOperations.java
@@ -578,8 +578,8 @@ public class TestSemanticModelOperations extends
BaseOperationsTest {
expression: orders.order_id
datatype: String
custom_extensions:
- - vendor_name: GRAVITINO
- data:
'{"_apache_gravitino_interchange":{"version":1,"properties":{"domain":"sales"}}}'
+ - vendor_name: GRAVITINO_PROPERTIES
+ data: '{"domain":"sales"}'
""";
Response yamlResponse = postDocument(semanticModelPath() + "/ossie", yaml,
"application/yaml");
