mchades commented on code in PR #12625:
URL: https://github.com/apache/gravitino/pull/12625#discussion_r4120056849


##########
common/src/main/java/org/apache/gravitino/dto/semantic/SemanticModelDTO.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * 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.semantic;
+
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonPropertyOrder;
+import com.google.common.base.Preconditions;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import javax.annotation.Nullable;
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.dto.AuditDTO;
+import org.apache.gravitino.semantic.SemanticModel;
+import org.apache.gravitino.semantic.SemanticModelDefinition;
+
+/** DTO for a schema-scoped Semantic Model. */
+@EqualsAndHashCode
+@ToString
+@JsonInclude(JsonInclude.Include.NON_NULL)
+@JsonPropertyOrder({"name", "comment", "definition", "properties", "audit"})
+public class SemanticModelDTO implements SemanticModel {
+
+  @JsonProperty("name")
+  private String name;
+
+  @Nullable
+  @JsonProperty("comment")
+  private String comment;
+
+  @JsonProperty("definition")
+  private SemanticModelDefinitionDTO definition;
+
+  @JsonProperty("properties")
+  private Map<String, String> properties;
+
+  @JsonProperty("audit")
+  private AuditDTO audit;
+
+  private SemanticModelDTO() {}
+
+  private SemanticModelDTO(
+      String name,
+      @Nullable String comment,
+      SemanticModelDefinitionDTO definition,
+      @Nullable Map<String, String> properties,
+      AuditDTO audit) {
+    this.name = name;
+    this.comment = comment;
+    this.definition = definition;
+    this.properties = immutableProperties(properties);
+    this.audit = audit;
+  }
+
+  @Override
+  public String name() {
+    return name;
+  }
+
+  @Override
+  @Nullable
+  public String comment() {
+    return comment;
+  }
+
+  /**
+   * Returns whether this DTO contains a Semantic Model definition.
+   *
+   * @return {@code true} if the definition is present, otherwise {@code 
false}.
+   */
+  public boolean hasDefinition() {
+    return definition != null;
+  }
+
+  @Override
+  public SemanticModelDefinition definition() {
+    Preconditions.checkArgument(definition != null, "definition must not be 
null");
+    return definition.toDefinition();
+  }
+
+  @Override
+  public Map<String, String> properties() {
+    return immutableProperties(properties);
+  }
+
+  @Override
+  public AuditDTO auditInfo() {
+    return audit;
+  }
+
+  /**
+   * Creates a builder for a Semantic Model DTO.
+   *
+   * @return A new builder.
+   */
+  public static Builder builder() {
+    return new Builder();
+  }
+
+  /** Builder for {@link SemanticModelDTO}. */
+  public static final class Builder {
+
+    private String name;
+    @Nullable private String comment;
+    private SemanticModelDefinitionDTO definition;
+    @Nullable private Map<String, String> properties;
+    private AuditDTO audit;
+
+    private Builder() {}
+
+    /**
+     * Sets the Semantic Model name.
+     *
+     * @param name The Semantic Model name.
+     * @return This builder.
+     */
+    public Builder withName(String name) {
+      this.name = name;
+      return this;
+    }
+
+    /**
+     * Sets the Semantic Model comment.
+     *
+     * @param comment The comment, or {@code null} if it is not set.
+     * @return This builder.
+     */
+    public Builder withComment(@Nullable String comment) {
+      this.comment = comment;
+      return this;
+    }
+
+    /**
+     * Sets the Semantic Model definition.
+     *
+     * @param definition The complete definition DTO.
+     * @return This builder.
+     */
+    public Builder withDefinition(SemanticModelDefinitionDTO definition) {
+      this.definition = definition;
+      return this;
+    }
+
+    /**
+     * Sets the Gravitino-specific Semantic Model properties.
+     *
+     * @param properties The properties, or {@code null} if none are set.
+     * @return This builder.
+     */
+    public Builder withProperties(@Nullable Map<String, String> properties) {
+      this.properties = properties;
+      return this;
+    }
+
+    /**
+     * Sets the Semantic Model audit information.
+     *
+     * @param audit The audit information.
+     * @return This builder.
+     */
+    public Builder withAudit(AuditDTO audit) {
+      this.audit = audit;
+      return this;
+    }
+
+    /**
+     * Builds a Semantic Model DTO.
+     *
+     * @return The Semantic Model DTO.
+     * @throws IllegalArgumentException If a required field is missing.
+     */
+    public SemanticModelDTO build() {
+      Preconditions.checkArgument(StringUtils.isNotBlank(name), "name cannot 
be null or empty");
+      Preconditions.checkArgument(definition != null, "definition cannot be 
null");
+      Preconditions.checkArgument(audit != null, "audit cannot be null");
+
+      return new SemanticModelDTO(name, comment, definition, properties, 
audit);

Review Comment:
   The shallow build-time validation is intentional, but the two downstream 
effects are slightly different from described. The Java client’s 
`GenericSemanticModel` constructor immediately calls 
`semanticModel.definition()`, so an invalid nested response fails before the 
client operation returns rather than at an arbitrary later accessor. Also, 
`SemanticModelDefinitionDTO` copies its arrays in the constructor and returns 
defensive copies from its array getters; 
`testDefinitionAndPropertiesAreDefensive` covers mutation of the original 
datasets array. Storing the DTO directly therefore removes the conversion round 
trip without exposing mutable array state. No additional change is needed here.



-- 
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]

Reply via email to