jerryshao commented on code in PR #12499: URL: https://github.com/apache/gravitino/pull/12499#discussion_r3829712632
########## api/src/main/java/org/apache/gravitino/semantic/AIContextObject.java: ########## @@ -0,0 +1,385 @@ +/* + * 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.semantic; + +import com.google.common.base.Preconditions; +import java.lang.reflect.Array; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.IdentityHashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import javax.annotation.Nullable; +import org.apache.gravitino.annotation.Evolving; + +/** Structured AI context with optional standard fields and retained custom JSON properties. */ +@Evolving +public final class AIContextObject { + + @Nullable private final String instructions; + @Nullable private final String[] synonyms; + @Nullable private final String[] examples; + private final Map<String, Object> additionalProperties; + + private AIContextObject(Builder builder) { + this.instructions = builder.instructions; + this.synonyms = copyOrNull(builder.synonyms); + this.examples = copyOrNull(builder.examples); + this.additionalProperties = immutableAdditionalProperties(builder.additionalProperties); + } + + /** + * Creates a builder for structured AI context. + * + * @return A new builder. + */ + public static Builder builder() { + return new Builder(); + } + + /** + * Returns instructions for AI tools. + * + * @return The instructions, or null when not provided. + */ + @Nullable + public String instructions() { + return instructions; + } + + /** + * Returns alternative names and terms. + * + * @return A defensive copy of the synonyms, or {@code null} when not provided. + */ + @Nullable + public String[] synonyms() { + return copyOrNull(synonyms); + } + + /** + * Returns sample questions or use cases. + * + * @return A defensive copy of the examples, or {@code null} when not provided. + */ + @Nullable + public String[] examples() { + return copyOrNull(examples); + } + + /** + * Returns custom JSON-compatible properties not represented by the standard fields. + * + * <p>The returned map and every nested map or JSON array are unmodifiable. Java arrays supplied + * to the builder are represented as unmodifiable lists. Integral numbers are represented as + * {@link BigInteger}, and decimal numbers are represented as {@link BigDecimal}. + * + * @return The deeply immutable additional properties, preserving iteration order. + */ + public Map<String, Object> additionalProperties() { + return additionalProperties; + } + + /** + * Compares this structured AI context with another object. + * + * @param other The object to compare. + * @return {@code true} if the object has the same standard and additional properties. + */ + @Override + public boolean equals(@Nullable Object other) { + if (this == other) { + return true; + } + if (!(other instanceof AIContextObject)) { + return false; + } + AIContextObject that = (AIContextObject) other; + return Objects.equals(instructions, that.instructions) + && Arrays.equals(synonyms, that.synonyms) + && Arrays.equals(examples, that.examples) + && additionalProperties.equals(that.additionalProperties); + } + + /** + * Returns the hash code of this structured AI context. + * + * @return The hash code. + */ + @Override + public int hashCode() { + int result = Objects.hash(instructions, additionalProperties); + result = 31 * result + Arrays.hashCode(synonyms); + result = 31 * result + Arrays.hashCode(examples); + return result; + } + + /** + * Returns a string representation of this structured AI context. + * + * @return The string representation. + */ + @Override + public String toString() { + return "AIContextObject{" + + "instructions='" + + instructions + + '\'' + + ", synonyms=" + + Arrays.toString(synonyms) + + ", examples=" + + Arrays.toString(examples) + + ", additionalProperties=" + + additionalProperties + + '}'; + } + + /** A builder for {@link AIContextObject}. */ + public static final class Builder { + + @Nullable private String instructions; + @Nullable private String[] synonyms; + @Nullable private String[] examples; + private Map<String, Object> additionalProperties = Collections.emptyMap(); + + private Builder() {} + + /** + * Sets or clears instructions for AI tools. + * + * @param instructions The instructions, or null to leave them unset. + * @return This builder. + */ + public Builder withInstructions(@Nullable String instructions) { + this.instructions = instructions; + return this; + } + + /** + * Sets or clears alternative names and terms. + * + * @param synonyms The synonyms, or null to leave them unset. + * @return This builder. + */ + public Builder withSynonyms(@Nullable String[] synonyms) { + this.synonyms = synonyms; + return this; + } + + /** + * Sets or clears sample questions or use cases. + * + * @param examples The examples, or null to leave them unset. + * @return This builder. + */ + public Builder withExamples(@Nullable String[] examples) { + this.examples = examples; + return this; + } + + /** + * Sets additional JSON-compatible properties. + * + * <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. Property names must not duplicate {@code instructions}, {@code synonyms}, + * or {@code examples}. + * + * @param additionalProperties The additional properties. + * @return This builder. + */ + public Builder withAdditionalProperties(Map<String, Object> additionalProperties) { + this.additionalProperties = additionalProperties; + return this; + } + + /** + * Builds structured AI context. + * + * @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. + */ + public AIContextObject build() { + validateStringArray("synonyms", synonyms); + validateStringArray("examples", examples); + Preconditions.checkArgument( + additionalProperties != null, "additionalProperties must not be null"); + return new AIContextObject(this); + } + } + + @Nullable + private static String[] copyOrNull(@Nullable String[] values) { + return values == null ? null : Arrays.copyOf(values, values.length); + } + + private static void validateStringArray(String name, @Nullable String[] values) { + if (values == null) { + return; + } + for (String value : values) { + Preconditions.checkArgument(value != null, "%s must not contain null", name); + } + } + + private static Map<String, Object> immutableAdditionalProperties(Map<String, Object> properties) { Review Comment: **Simplification: recursive JSON deep-freeze is disproportionately complex for an "additionalProperties" bag** `immutableAdditionalProperties`/`immutableJsonValue`/`immutableJsonMap`/`immutableJsonList`/`immutableJsonArray`/`canonicalizeJsonNumber`/`enterContainer` (~130 lines total) implement a full recursive JSON deep-freeze with `IdentityHashMap`-based cycle detection and number canonicalization. Every other DTO in this PR (`Dataset`, `Field`, `Metric`, ...) is satisfied with simple `Arrays.copyOf`-style shallow defensive copies. Unless cyclic JSON and mixed-number-type canonicalization are genuinely expected inputs here, a shallow `Collections.unmodifiableMap` wrap (deferring structural validation to the JSON deserialization layer) would achieve the same practical immutability guarantee with far less code to maintain and test. ########## api/src/main/java/org/apache/gravitino/semantic/Relationship.java: ########## @@ -0,0 +1,331 @@ +/* + * 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.semantic; + +import com.google.common.base.Preconditions; +import java.util.Arrays; +import java.util.Objects; +import javax.annotation.Nullable; +import org.apache.gravitino.annotation.Evolving; + +/** + * An immutable relationship between two datasets in the same semantic model. The endpoint column + * arrays describe the corresponding join columns on the source and target datasets. + */ +@Evolving +public final class Relationship { + + private final String name; + private final String from; + private final String to; + private final String[] fromColumns; + private final String[] toColumns; + + @Nullable private final AIContext aiContext; + @Nullable private final CustomExtension[] customExtensions; + + private Relationship(Builder builder) { + this.name = builder.name; + this.from = builder.from; + this.to = builder.to; + this.fromColumns = Arrays.copyOf(builder.fromColumns, builder.fromColumns.length); + this.toColumns = Arrays.copyOf(builder.toColumns, builder.toColumns.length); + this.aiContext = builder.aiContext; + this.customExtensions = + builder.customExtensions == null + ? null + : Arrays.copyOf(builder.customExtensions, builder.customExtensions.length); + } + + /** + * Creates a builder for an immutable {@link Relationship}. + * + * @return A new builder. + */ + public static Builder builder() { + return new Builder(); + } + + /** + * Returns the relationship name. + * + * @return The relationship name. + */ + public String name() { + return name; + } + + /** + * Returns the name of the source dataset. + * + * @return The source dataset name. + */ + public String from() { + return from; + } + + /** + * Returns the name of the target dataset. + * + * @return The target dataset name. + */ + public String to() { + return to; + } + + /** + * Returns a copy of the source dataset columns. + * + * @return The source columns. + */ + public String[] fromColumns() { + return Arrays.copyOf(fromColumns, fromColumns.length); + } + + /** + * Returns a copy of the target dataset columns. + * + * @return The target columns. + */ + public String[] toColumns() { + return Arrays.copyOf(toColumns, toColumns.length); + } + + /** + * Returns the AI context associated with the relationship. + * + * @return The AI context, or {@code null} if it is not set. + */ + @Nullable + public AIContext aiContext() { + return aiContext; + } + + /** + * Returns a copy of the custom extensions associated with the relationship. + * + * @return The custom extensions, or {@code null} if they are not set. + */ + @Nullable + public CustomExtension[] customExtensions() { + return customExtensions == null + ? null + : Arrays.copyOf(customExtensions, customExtensions.length); + } + + /** + * Compares this relationship with another object for value equality. + * + * @param other The object to compare with. + * @return {@code true} if the objects are equal, otherwise {@code false}. + */ + @Override + public boolean equals(@Nullable Object other) { + if (this == other) { + return true; + } + if (!(other instanceof Relationship)) { + return false; + } + Relationship that = (Relationship) other; + return name.equals(that.name) + && from.equals(that.from) + && to.equals(that.to) + && Arrays.equals(fromColumns, that.fromColumns) + && Arrays.equals(toColumns, that.toColumns) + && Objects.equals(aiContext, that.aiContext) + && Arrays.equals(customExtensions, that.customExtensions); + } + + /** + * Returns the value-based hash code for this relationship. + * + * @return The hash code. + */ + @Override + public int hashCode() { + return Objects.hash( + name, + from, + to, + Arrays.hashCode(fromColumns), + Arrays.hashCode(toColumns), + aiContext, + Arrays.hashCode(customExtensions)); + } + + /** + * Returns a string representation of this relationship. + * + * @return The string representation. + */ + @Override + public String toString() { + return "Relationship{" + + "name='" + + name + + '\'' + + ", from='" + + from + + '\'' + + ", to='" + + to + + '\'' + + ", fromColumns=" + + Arrays.toString(fromColumns) + + ", toColumns=" + + Arrays.toString(toColumns) + + ", aiContext=" + + aiContext + + ", customExtensions=" + + Arrays.toString(customExtensions) + + '}'; + } + + /** A builder for immutable {@link Relationship} values. */ + public static final class Builder { + + private String name; + private String from; + private String to; + private String[] fromColumns; + private String[] toColumns; + + @Nullable private AIContext aiContext; + @Nullable private CustomExtension[] customExtensions; + + private Builder() {} + + /** + * Sets the relationship name. + * + * @param name The non-empty relationship name. + * @return This builder. + */ + public Builder withName(String name) { + this.name = name; + return this; + } + + /** + * Sets the source dataset name. + * + * @param from The non-empty source dataset name. + * @return This builder. + */ + public Builder withFrom(String from) { + this.from = from; + return this; + } + + /** + * Sets the target dataset name. + * + * @param to The non-empty target dataset name. + * @return This builder. + */ + public Builder withTo(String to) { + this.to = to; + return this; + } + + /** + * Sets the source dataset columns. + * + * @param fromColumns The non-empty source column array. + * @return This builder. + */ + public Builder withFromColumns(String[] fromColumns) { + this.fromColumns = fromColumns; + return this; + } + + /** + * Sets the target dataset columns. + * + * @param toColumns The non-empty target column array. + * @return This builder. + */ + public Builder withToColumns(String[] toColumns) { + this.toColumns = toColumns; + return this; + } + + /** + * Sets the optional AI context. + * + * @param aiContext The AI context, or {@code null} to leave it unset. + * @return This builder. + */ + public Builder withAIContext(@Nullable AIContext aiContext) { + this.aiContext = aiContext; + return this; + } + + /** + * Sets the optional custom extensions. + * + * @param customExtensions The custom extensions, or {@code null} to leave them unset. + * @return This builder. + */ + public Builder withCustomExtensions(@Nullable CustomExtension[] customExtensions) { + this.customExtensions = customExtensions; + return this; + } + + /** + * Builds an immutable {@link Relationship}. + * + * @return The new relationship. + * @throws IllegalArgumentException If a required name is null or empty, a required column array + * is null or empty, a column name or custom extension is null, or the endpoint column + * arrays have different lengths. + */ + public Relationship build() { + Preconditions.checkArgument( + name != null && !name.isEmpty(), "name must not be null or empty"); + Preconditions.checkArgument( + from != null && !from.isEmpty(), "from must not be null or empty"); + Preconditions.checkArgument(to != null && !to.isEmpty(), "to must not be null or empty"); + Preconditions.checkArgument( + fromColumns != null && fromColumns.length > 0, "fromColumns must not be null or empty"); + Preconditions.checkArgument( + toColumns != null && toColumns.length > 0, "toColumns must not be null or empty"); + validateColumnNames("fromColumns", fromColumns); + validateColumnNames("toColumns", toColumns); + Preconditions.checkArgument( + fromColumns.length == toColumns.length, + "fromColumns and toColumns must have the same length"); + Preconditions.checkArgument( + customExtensions == null || Arrays.stream(customExtensions).allMatch(Objects::nonNull), + "customExtensions must not contain null"); + return new Relationship(this); + } + } + + private static void validateColumnNames(String name, String[] columns) { Review Comment: **Cleanup: duplicated "array must not contain null" validation with two inconsistent styles** This same check is reimplemented several times across the new builders with two different styles: `Relationship.validateColumnNames` and `Dataset.validateNoNullElements` use hand-rolled indexed for-loops with per-index `Preconditions` messages, while `Metric`, `Field`, `Relationship`'s `customExtensions` check, and `SemanticModelDefinition` use `Arrays.stream(x).allMatch(Objects::nonNull)` with one generic message. No shared helper exists, so a future fix to one style's error-message format won't propagate to the other. Consider factoring this into one shared private/internal validation helper used by all the builders in this package. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
