jerryshao commented on code in PR #10982:
URL: https://github.com/apache/gravitino/pull/10982#discussion_r3206904117
##########
common/src/main/java/org/apache/gravitino/dto/rel/SQLRepresentationDTO.java:
##########
@@ -18,53 +18,170 @@
*/
package org.apache.gravitino.dto.rel;
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
-import lombok.EqualsAndHashCode;
-import lombok.Getter;
+import com.google.common.base.Preconditions;
+import java.util.Objects;
+import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.rel.Representation;
import org.apache.gravitino.rel.SQLRepresentation;
-/** DTO for SQL representation. */
-@Getter
-@EqualsAndHashCode(callSuper = true)
-public class SQLRepresentationDTO extends RepresentationDTO {
+/**
+ * A DTO mirroring {@link org.apache.gravitino.rel.SQLRepresentation}.
Represents a SQL-based view
+ * definition for a particular dialect.
+ */
+@JsonIgnoreProperties(ignoreUnknown = true)
+public final class SQLRepresentationDTO extends RepresentationDTO {
+
+ @JsonProperty("type")
+ private final String type = Representation.TYPE_SQL;
@JsonProperty("dialect")
private String dialect;
@JsonProperty("sql")
private String sql;
- private SQLRepresentationDTO() {}
+ private SQLRepresentationDTO() {
+ super();
+ }
+
+ private SQLRepresentationDTO(String dialect, String sql) {
+ this.dialect = dialect;
+ this.sql = sql;
+ }
/**
- * Creates a SQL representation DTO.
+ * Creates a new {@link Builder}.
*
- * @param dialect SQL dialect.
- * @param sql SQL body.
+ * @return A new builder instance.
*/
- public SQLRepresentationDTO(String dialect, String sql) {
- this.dialect = dialect;
- this.sql = sql;
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ /**
+ * Creates a new {@link SQLRepresentationDTO} from a {@link
SQLRepresentation} domain object.
+ *
+ * @param sqlRepresentation The SQL representation domain object.
+ * @return The SQL representation DTO.
+ */
+ public static SQLRepresentationDTO fromSQLRepresentation(SQLRepresentation
sqlRepresentation) {
+ return builder()
+ .withDialect(sqlRepresentation.dialect())
+ .withSql(sqlRepresentation.sql())
+ .build();
}
@Override
public String type() {
- return Representation.TYPE_SQL;
+ return type;
}
- @Override
- public SQLRepresentation toRepresentation() {
- return
SQLRepresentation.builder().withDialect(dialect).withSql(sql).build();
+ /**
+ * Returns the SQL dialect of this representation.
+ *
+ * @return The dialect identifier.
+ */
+ public String dialect() {
+ return dialect;
+ }
+
+ /**
+ * Returns the SQL dialect of this representation.
+ *
+ * @return The dialect identifier.
+ */
+ public String getDialect() {
Review Comment:
**Duplicate accessor / copy-paste Javadoc error.**
`getDialect()` and `dialect()` (similarly `getSql()` / `sql()`) are
identical in body and carry the same Javadoc. The class dropped `@Getter` in
this refactor; if JavaBean-style getters are genuinely required, re-add
`@Getter` and remove the hand-written `dialect()` / `sql()` overrides. If the
interface-style methods are sufficient, remove the `get*` copies. Having both
with identical Javadoc is misleading.
##########
common/src/main/java/org/apache/gravitino/dto/requests/ViewCreateRequest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import javax.annotation.Nullable;
+import lombok.Builder;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import lombok.ToString;
+import lombok.extern.jackson.Jacksonized;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.dto.rel.ColumnDTO;
+import org.apache.gravitino.dto.rel.RepresentationDTO;
+import org.apache.gravitino.dto.rel.SQLRepresentationDTO;
+import org.apache.gravitino.rest.RESTRequest;
+
+/** Represents a request to create a view. */
+@Getter
+@EqualsAndHashCode
+@ToString
+@Builder
+@Jacksonized
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class ViewCreateRequest implements RESTRequest {
+
+ @JsonProperty("name")
+ private final String name;
+
+ @JsonProperty("comment")
+ @Nullable
+ private final String comment;
+
+ @JsonProperty("columns")
+ private final ColumnDTO[] columns;
+
+ @JsonProperty("representations")
+ private final RepresentationDTO[] representations;
+
+ @JsonProperty("defaultCatalog")
+ @Nullable
+ private final String defaultCatalog;
+
+ @JsonProperty("defaultSchema")
+ @Nullable
+ private final String defaultSchema;
+
+ @JsonProperty("properties")
+ @Nullable
+ private final Map<String, String> properties;
+
+ /** Default constructor for Jackson deserialization. */
+ public ViewCreateRequest() {
Review Comment:
**Explicit constructors conflict with `@Builder` + `@Jacksonized`.**
When `@Builder` is present, Lombok generates its `build()` by calling an
all-args constructor. Declaring an explicit all-args constructor with the same
signature either shadows the Lombok-generated one or causes a compile error
(behaviour is Lombok-version dependent). `@Jacksonized` wires Jackson to the
Lombok builder for deserialization, making the no-arg constructor here
redundant.
Please either:
- Remove the explicit constructors and let `@Builder` + `@Jacksonized`
handle everything, or
- Remove `@Builder` / `@Jacksonized` and keep plain constructors (as
`ViewUpdatesRequest` does).
See `TableCreateRequest` for the established pattern.
##########
common/src/main/java/org/apache/gravitino/dto/requests/ViewUpdatesRequest.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * 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.JsonProperty;
+import com.google.common.base.Preconditions;
+import java.util.List;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import lombok.ToString;
+import org.apache.gravitino.rest.RESTRequest;
+
+/** Represents a request to update a view. */
+@Getter
+@EqualsAndHashCode
+@ToString
+public class ViewUpdatesRequest implements RESTRequest {
Review Comment:
**Missing `@JsonIgnoreProperties(ignoreUnknown = true)`.**
Every other DTO in this PR (`ViewCreateRequest`, `ViewUpdateRequest`,
`ViewDTO`, `SQLRepresentationDTO`, etc.) carries
`@JsonIgnoreProperties(ignoreUnknown = true)` for forward-compatibility.
Without it, Jackson will throw `UnrecognizedPropertyException` if a newer
server sends a field that an older client's `ViewUpdatesRequest` does not know
about.
##########
common/src/main/java/org/apache/gravitino/dto/requests/ViewUpdateRequest.java:
##########
@@ -0,0 +1,268 @@
+/*
+ * 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.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonSubTypes;
+import com.fasterxml.jackson.annotation.JsonTypeInfo;
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+import javax.annotation.Nullable;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import lombok.ToString;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.dto.rel.ColumnDTO;
+import org.apache.gravitino.dto.rel.RepresentationDTO;
+import org.apache.gravitino.dto.rel.SQLRepresentationDTO;
+import org.apache.gravitino.dto.util.DTOConverters;
+import org.apache.gravitino.rel.ViewChange;
+import org.apache.gravitino.rest.RESTRequest;
+
+/** Represents a request to update a view. */
+@JsonIgnoreProperties(ignoreUnknown = true)
+@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY)
+@JsonSubTypes({
+ @JsonSubTypes.Type(value = ViewUpdateRequest.RenameViewRequest.class, name =
"rename"),
+ @JsonSubTypes.Type(value = ViewUpdateRequest.SetViewPropertyRequest.class,
name = "setProperty"),
+ @JsonSubTypes.Type(
+ value = ViewUpdateRequest.RemoveViewPropertyRequest.class,
+ name = "removeProperty"),
+ @JsonSubTypes.Type(value = ViewUpdateRequest.ReplaceViewRequest.class, name
= "replaceView")
+})
+public interface ViewUpdateRequest extends RESTRequest {
+
+ /**
+ * The view change represented by this request.
+ *
+ * @return An instance of {@link ViewChange}.
+ */
+ ViewChange viewChange();
+
+ /** Represents a request to rename a view. */
+ @EqualsAndHashCode
+ @ToString
+ @Getter
+ class RenameViewRequest implements ViewUpdateRequest {
+
+ @JsonProperty("newName")
+ private final String newName;
+
+ /**
+ * Constructor for RenameViewRequest.
+ *
+ * @param newName The new name of the view.
+ */
+ public RenameViewRequest(String newName) {
+ this.newName = newName;
+ }
+
+ /** Default constructor for Jackson deserialization. */
+ public RenameViewRequest() {
+ this(null);
+ }
+
+ @Override
+ public void validate() throws IllegalArgumentException {
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(newName), "\"newName\" field is required and
cannot be empty");
+ }
+
+ @Override
+ public ViewChange viewChange() {
+ return ViewChange.rename(newName);
+ }
+ }
+
+ /** Represents a request to set a property of a view. */
+ @EqualsAndHashCode
+ @ToString
+ @Getter
+ class SetViewPropertyRequest implements ViewUpdateRequest {
+
+ @JsonProperty("property")
+ private final String property;
+
+ @JsonProperty("value")
+ private final String value;
+
+ /**
+ * Constructor for SetViewPropertyRequest.
+ *
+ * @param property The property to set.
+ * @param value The value of the property.
+ */
+ public SetViewPropertyRequest(String property, String value) {
+ this.property = property;
+ this.value = value;
+ }
+
+ /** Default constructor for Jackson deserialization. */
+ public SetViewPropertyRequest() {
+ this(null, null);
+ }
+
+ @Override
+ public void validate() throws IllegalArgumentException {
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(property), "\"property\" field is required
and cannot be empty");
+ Preconditions.checkArgument(value != null, "\"value\" field is required
and cannot be null");
+ }
+
+ @Override
+ public ViewChange viewChange() {
+ return ViewChange.setProperty(property, value);
+ }
+ }
+
+ /** Represents a request to remove a property of a view. */
+ @EqualsAndHashCode
+ @ToString
+ @Getter
+ class RemoveViewPropertyRequest implements ViewUpdateRequest {
+
+ @JsonProperty("property")
+ private final String property;
+
+ /**
+ * Constructor for RemoveViewPropertyRequest.
+ *
+ * @param property The property to remove.
+ */
+ public RemoveViewPropertyRequest(String property) {
+ this.property = property;
+ }
+
+ /** Default constructor for Jackson deserialization. */
+ public RemoveViewPropertyRequest() {
+ this(null);
+ }
+
+ @Override
+ public void validate() throws IllegalArgumentException {
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(property), "\"property\" field is required
and cannot be empty");
+ }
+
+ @Override
+ public ViewChange viewChange() {
+ return ViewChange.removeProperty(property);
+ }
+ }
+
+ /**
+ * Represents a request to atomically replace the body (columns,
representations, default catalog,
+ * default schema and comment) of a view. View name and properties are not
affected.
+ */
+ @EqualsAndHashCode
+ @ToString
+ @Getter
+ class ReplaceViewRequest implements ViewUpdateRequest {
+
+ @JsonProperty("columns")
+ private final ColumnDTO[] columns;
+
+ @JsonProperty("representations")
+ private final RepresentationDTO[] representations;
+
+ @JsonProperty("defaultCatalog")
+ @Nullable
+ private final String defaultCatalog;
+
+ @JsonProperty("defaultSchema")
+ @Nullable
+ private final String defaultSchema;
+
+ @JsonProperty("comment")
+ @Nullable
+ private final String comment;
+
+ /**
+ * Constructor for ReplaceViewRequest.
+ *
+ * @param columns The new output columns of the view.
+ * @param representations The new representations of the view.
+ * @param defaultCatalog The new default catalog, or {@code null} to unset
it.
+ * @param defaultSchema The new default schema, or {@code null} to unset
it.
+ * @param comment The new comment, or {@code null} to unset it.
+ */
+ public ReplaceViewRequest(
+ ColumnDTO[] columns,
+ RepresentationDTO[] representations,
+ @Nullable String defaultCatalog,
+ @Nullable String defaultSchema,
+ @Nullable String comment) {
+ this.columns = columns;
+ this.representations = representations;
+ this.defaultCatalog = defaultCatalog;
+ this.defaultSchema = defaultSchema;
+ this.comment = comment;
+ }
+
+ /** Default constructor for Jackson deserialization. */
+ public ReplaceViewRequest() {
+ this(null, null, null, null, null);
+ }
+
+ @Override
+ public void validate() throws IllegalArgumentException {
+ Preconditions.checkArgument(
+ representations != null && representations.length > 0,
+ "\"representations\" field is required and cannot be empty");
+ Arrays.stream(representations)
+ .forEach(
+ rep -> {
+ Preconditions.checkArgument(rep != null, "representation must
not be null");
+ rep.validate();
+ });
+ if (columns != null) {
+ Arrays.stream(columns)
+ .forEach(
+ column -> {
+ Preconditions.checkArgument(column != null, "column must not
be null");
+ column.validate();
+ });
+ }
+
+ Set<String> seenDialects = new HashSet<>();
Review Comment:
Same duplicate dialect-deduplication logic as in
`ViewCreateRequest.validate()`. Please extract to a shared helper (see comment
on `ViewCreateRequest` line 131).
##########
server/src/main/java/org/apache/gravitino/server/web/rest/ViewOperations.java:
##########
@@ -0,0 +1,223 @@
+/*
+ * 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.server.web.rest;
+
+import static org.apache.gravitino.dto.util.DTOConverters.fromDTOs;
+
+import com.codahale.metrics.annotation.ResponseMetered;
+import com.codahale.metrics.annotation.Timed;
+import javax.inject.Inject;
+import javax.servlet.http.HttpServletRequest;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.Context;
+import javax.ws.rs.core.Response;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.catalog.ViewDispatcher;
+import org.apache.gravitino.dto.requests.ViewCreateRequest;
+import org.apache.gravitino.dto.requests.ViewUpdateRequest;
+import org.apache.gravitino.dto.requests.ViewUpdatesRequest;
+import org.apache.gravitino.dto.responses.DropResponse;
+import org.apache.gravitino.dto.responses.EntityListResponse;
+import org.apache.gravitino.dto.responses.ViewResponse;
+import org.apache.gravitino.dto.util.DTOConverters;
+import org.apache.gravitino.metrics.MetricNames;
+import org.apache.gravitino.rel.View;
+import org.apache.gravitino.rel.ViewChange;
+import org.apache.gravitino.server.web.Utils;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+import org.apache.gravitino.utils.NamespaceUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Path("metalakes/{metalake}/catalogs/{catalog}/schemas/{schema}/views")
+public class ViewOperations {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(ViewOperations.class);
+
+ private final ViewDispatcher dispatcher;
+
+ @Context private HttpServletRequest httpRequest;
+
+ @Inject
+ public ViewOperations(ViewDispatcher dispatcher) {
+ this.dispatcher = dispatcher;
+ }
+
+ @GET
+ @Produces("application/vnd.gravitino.v1+json")
+ @Timed(name = "list-view." + MetricNames.HTTP_PROCESS_DURATION, absolute =
true)
+ @ResponseMetered(name = "list-view", absolute = true)
+ public Response listViews(
+ @PathParam("metalake") String metalake,
+ @PathParam("catalog") String catalog,
+ @PathParam("schema") String schema) {
+ LOG.info("Received list views request for schema: {}.{}.{}", metalake,
catalog, schema);
+ try {
+ return Utils.doAs(
+ httpRequest,
+ () -> {
+ Namespace viewNS = NamespaceUtil.ofView(metalake, catalog, schema);
+ NameIdentifier[] idents = dispatcher.listViews(viewNS);
+ Response response = Utils.ok(new EntityListResponse(idents));
+ LOG.info(
+ "List {} views under schema: {}.{}.{}", idents.length,
metalake, catalog, schema);
+ return response;
+ });
+
+ } catch (Exception e) {
+ return ExceptionHandlers.handleViewException(OperationType.LIST, "",
schema, e);
+ }
+ }
+
+ @POST
+ @Produces("application/vnd.gravitino.v1+json")
Review Comment:
**Missing `@Consumes` annotation on POST.**
`TableOperations` (and other `*Operations` classes in this package)
annotates non-GET methods with
`@Consumes("application/vnd.gravitino.v1+json")`. Without it this endpoint
accepts any content type, which is inconsistent with the rest of the API.
Please add `@Consumes("application/vnd.gravitino.v1+json")` here and on the PUT
handler below.
##########
server/src/main/java/org/apache/gravitino/server/web/rest/ViewOperations.java:
##########
@@ -0,0 +1,223 @@
+/*
+ * 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.server.web.rest;
+
+import static org.apache.gravitino.dto.util.DTOConverters.fromDTOs;
+
+import com.codahale.metrics.annotation.ResponseMetered;
+import com.codahale.metrics.annotation.Timed;
+import javax.inject.Inject;
+import javax.servlet.http.HttpServletRequest;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.Context;
+import javax.ws.rs.core.Response;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.catalog.ViewDispatcher;
+import org.apache.gravitino.dto.requests.ViewCreateRequest;
+import org.apache.gravitino.dto.requests.ViewUpdateRequest;
+import org.apache.gravitino.dto.requests.ViewUpdatesRequest;
+import org.apache.gravitino.dto.responses.DropResponse;
+import org.apache.gravitino.dto.responses.EntityListResponse;
+import org.apache.gravitino.dto.responses.ViewResponse;
+import org.apache.gravitino.dto.util.DTOConverters;
+import org.apache.gravitino.metrics.MetricNames;
+import org.apache.gravitino.rel.View;
+import org.apache.gravitino.rel.ViewChange;
+import org.apache.gravitino.server.web.Utils;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+import org.apache.gravitino.utils.NamespaceUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Path("metalakes/{metalake}/catalogs/{catalog}/schemas/{schema}/views")
+public class ViewOperations {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(ViewOperations.class);
+
+ private final ViewDispatcher dispatcher;
+
+ @Context private HttpServletRequest httpRequest;
+
+ @Inject
+ public ViewOperations(ViewDispatcher dispatcher) {
+ this.dispatcher = dispatcher;
+ }
+
+ @GET
+ @Produces("application/vnd.gravitino.v1+json")
+ @Timed(name = "list-view." + MetricNames.HTTP_PROCESS_DURATION, absolute =
true)
+ @ResponseMetered(name = "list-view", absolute = true)
+ public Response listViews(
+ @PathParam("metalake") String metalake,
+ @PathParam("catalog") String catalog,
+ @PathParam("schema") String schema) {
+ LOG.info("Received list views request for schema: {}.{}.{}", metalake,
catalog, schema);
+ try {
+ return Utils.doAs(
+ httpRequest,
+ () -> {
+ Namespace viewNS = NamespaceUtil.ofView(metalake, catalog, schema);
+ NameIdentifier[] idents = dispatcher.listViews(viewNS);
+ Response response = Utils.ok(new EntityListResponse(idents));
+ LOG.info(
+ "List {} views under schema: {}.{}.{}", idents.length,
metalake, catalog, schema);
+ return response;
+ });
+
+ } catch (Exception e) {
+ return ExceptionHandlers.handleViewException(OperationType.LIST, "",
schema, e);
+ }
+ }
+
+ @POST
+ @Produces("application/vnd.gravitino.v1+json")
+ @Timed(name = "create-view." + MetricNames.HTTP_PROCESS_DURATION, absolute =
true)
+ @ResponseMetered(name = "create-view", absolute = true)
+ public Response createView(
+ @PathParam("metalake") String metalake,
+ @PathParam("catalog") String catalog,
+ @PathParam("schema") String schema,
+ ViewCreateRequest request) {
+ LOG.info(
+ "Received create view request: {}.{}.{}.{}", metalake, catalog,
schema, request.getName());
+ try {
+ return Utils.doAs(
+ httpRequest,
+ () -> {
+ request.validate();
+ NameIdentifier ident =
+ NameIdentifierUtil.ofView(metalake, catalog, schema,
request.getName());
+
+ View view =
+ dispatcher.createView(
+ ident,
+ request.getComment(),
+ fromDTOs(request.getColumns()),
+ DTOConverters.fromDTOs(request.getRepresentations()),
+ request.getDefaultCatalog(),
+ request.getDefaultSchema(),
+ request.getProperties());
+ Response response = Utils.ok(new
ViewResponse(DTOConverters.toDTO(view)));
+ LOG.info("View created: {}.{}.{}.{}", metalake, catalog, schema,
request.getName());
+ return response;
+ });
+
+ } catch (Exception e) {
+ return ExceptionHandlers.handleViewException(
+ OperationType.CREATE, request.getName(), schema, e);
+ }
+ }
+
+ @GET
+ @Path("{view}")
+ @Produces("application/vnd.gravitino.v1+json")
+ @Timed(name = "load-view." + MetricNames.HTTP_PROCESS_DURATION, absolute =
true)
+ @ResponseMetered(name = "load-view", absolute = true)
+ public Response loadView(
+ @PathParam("metalake") String metalake,
+ @PathParam("catalog") String catalog,
+ @PathParam("schema") String schema,
+ @PathParam("view") String view) {
+ LOG.info("Received load view request for view: {}.{}.{}.{}", metalake,
catalog, schema, view);
+ try {
+ return Utils.doAs(
+ httpRequest,
+ () -> {
+ NameIdentifier ident = NameIdentifierUtil.ofView(metalake,
catalog, schema, view);
+ View v = dispatcher.loadView(ident);
+ Response response = Utils.ok(new
ViewResponse(DTOConverters.toDTO(v)));
+ LOG.info("View loaded: {}.{}.{}.{}", metalake, catalog, schema,
view);
+ return response;
+ });
+ } catch (Exception e) {
+ return ExceptionHandlers.handleViewException(OperationType.LOAD, view,
schema, e);
+ }
+ }
+
+ @PUT
Review Comment:
**Missing `@Consumes` annotation on PUT.**
Same issue as the POST handler above — please add
`@Consumes("application/vnd.gravitino.v1+json")`.
##########
common/src/main/java/org/apache/gravitino/dto/requests/ViewCreateRequest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import javax.annotation.Nullable;
+import lombok.Builder;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import lombok.ToString;
+import lombok.extern.jackson.Jacksonized;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.dto.rel.ColumnDTO;
+import org.apache.gravitino.dto.rel.RepresentationDTO;
+import org.apache.gravitino.dto.rel.SQLRepresentationDTO;
+import org.apache.gravitino.rest.RESTRequest;
+
+/** Represents a request to create a view. */
+@Getter
+@EqualsAndHashCode
+@ToString
+@Builder
+@Jacksonized
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class ViewCreateRequest implements RESTRequest {
+
+ @JsonProperty("name")
+ private final String name;
+
+ @JsonProperty("comment")
+ @Nullable
+ private final String comment;
+
+ @JsonProperty("columns")
+ private final ColumnDTO[] columns;
+
+ @JsonProperty("representations")
+ private final RepresentationDTO[] representations;
+
+ @JsonProperty("defaultCatalog")
+ @Nullable
+ private final String defaultCatalog;
+
+ @JsonProperty("defaultSchema")
+ @Nullable
+ private final String defaultSchema;
+
+ @JsonProperty("properties")
+ @Nullable
+ private final Map<String, String> properties;
+
+ /** Default constructor for Jackson deserialization. */
+ public ViewCreateRequest() {
+ this(null, null, null, null, null, null, null);
+ }
+
+ /**
+ * Creates a new {@link ViewCreateRequest}.
+ *
+ * @param name The name of the view.
+ * @param comment The comment of the view.
+ * @param columns The output columns of the view.
+ * @param representations The representations of the view.
+ * @param defaultCatalog The default catalog used to resolve unqualified
identifiers in view
+ * representations.
+ * @param defaultSchema The default schema used to resolve unqualified
identifiers in view
+ * representations.
+ * @param properties The properties of the view.
+ */
+ public ViewCreateRequest(
+ String name,
+ @Nullable String comment,
+ ColumnDTO[] columns,
+ RepresentationDTO[] representations,
+ @Nullable String defaultCatalog,
+ @Nullable String defaultSchema,
+ @Nullable Map<String, String> properties) {
+ this.name = name;
+ this.comment = comment;
+ this.columns = columns;
+ this.representations = representations;
+ this.defaultCatalog = defaultCatalog;
+ this.defaultSchema = defaultSchema;
+ this.properties = properties;
+ }
+
+ @Override
+ public void validate() throws IllegalArgumentException {
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(name), "\"name\" field is required and cannot
be empty");
+ Preconditions.checkArgument(
+ representations != null && representations.length > 0,
+ "\"representations\" field is required and cannot be empty");
+ Arrays.stream(representations)
+ .forEach(
+ rep -> {
+ Preconditions.checkArgument(rep != null, "representation must
not be null");
+ rep.validate();
+ });
+ if (columns != null) {
+ Arrays.stream(columns)
+ .forEach(
+ column -> {
+ Preconditions.checkArgument(column != null, "column must not
be null");
+ column.validate();
+ });
+ }
+
+ Set<String> seenDialects = new HashSet<>();
Review Comment:
**Duplicate dialect-deduplication logic.**
This `HashSet`-based duplicate-dialect check is copy-pasted verbatim into
`ViewUpdateRequest.ReplaceViewRequest.validate()`. Please extract it into a
`private static void validateNoDuplicateDialects(RepresentationDTO[])` helper
and call it from both places.
##########
server/src/main/java/org/apache/gravitino/server/web/rest/ViewOperations.java:
##########
@@ -0,0 +1,223 @@
+/*
+ * 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.server.web.rest;
+
+import static org.apache.gravitino.dto.util.DTOConverters.fromDTOs;
+
+import com.codahale.metrics.annotation.ResponseMetered;
+import com.codahale.metrics.annotation.Timed;
+import javax.inject.Inject;
+import javax.servlet.http.HttpServletRequest;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.Context;
+import javax.ws.rs.core.Response;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.catalog.ViewDispatcher;
+import org.apache.gravitino.dto.requests.ViewCreateRequest;
+import org.apache.gravitino.dto.requests.ViewUpdateRequest;
+import org.apache.gravitino.dto.requests.ViewUpdatesRequest;
+import org.apache.gravitino.dto.responses.DropResponse;
+import org.apache.gravitino.dto.responses.EntityListResponse;
+import org.apache.gravitino.dto.responses.ViewResponse;
+import org.apache.gravitino.dto.util.DTOConverters;
+import org.apache.gravitino.metrics.MetricNames;
+import org.apache.gravitino.rel.View;
+import org.apache.gravitino.rel.ViewChange;
+import org.apache.gravitino.server.web.Utils;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+import org.apache.gravitino.utils.NamespaceUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Path("metalakes/{metalake}/catalogs/{catalog}/schemas/{schema}/views")
+public class ViewOperations {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(ViewOperations.class);
+
+ private final ViewDispatcher dispatcher;
+
+ @Context private HttpServletRequest httpRequest;
+
+ @Inject
+ public ViewOperations(ViewDispatcher dispatcher) {
+ this.dispatcher = dispatcher;
+ }
+
+ @GET
+ @Produces("application/vnd.gravitino.v1+json")
+ @Timed(name = "list-view." + MetricNames.HTTP_PROCESS_DURATION, absolute =
true)
+ @ResponseMetered(name = "list-view", absolute = true)
+ public Response listViews(
+ @PathParam("metalake") String metalake,
+ @PathParam("catalog") String catalog,
+ @PathParam("schema") String schema) {
+ LOG.info("Received list views request for schema: {}.{}.{}", metalake,
catalog, schema);
+ try {
+ return Utils.doAs(
+ httpRequest,
+ () -> {
+ Namespace viewNS = NamespaceUtil.ofView(metalake, catalog, schema);
+ NameIdentifier[] idents = dispatcher.listViews(viewNS);
+ Response response = Utils.ok(new EntityListResponse(idents));
+ LOG.info(
+ "List {} views under schema: {}.{}.{}", idents.length,
metalake, catalog, schema);
+ return response;
+ });
+
+ } catch (Exception e) {
+ return ExceptionHandlers.handleViewException(OperationType.LIST, "",
schema, e);
+ }
+ }
+
+ @POST
+ @Produces("application/vnd.gravitino.v1+json")
+ @Timed(name = "create-view." + MetricNames.HTTP_PROCESS_DURATION, absolute =
true)
+ @ResponseMetered(name = "create-view", absolute = true)
+ public Response createView(
+ @PathParam("metalake") String metalake,
+ @PathParam("catalog") String catalog,
+ @PathParam("schema") String schema,
+ ViewCreateRequest request) {
+ LOG.info(
+ "Received create view request: {}.{}.{}.{}", metalake, catalog,
schema, request.getName());
+ try {
+ return Utils.doAs(
+ httpRequest,
+ () -> {
+ request.validate();
+ NameIdentifier ident =
+ NameIdentifierUtil.ofView(metalake, catalog, schema,
request.getName());
+
+ View view =
+ dispatcher.createView(
+ ident,
+ request.getComment(),
+ fromDTOs(request.getColumns()),
Review Comment:
**Potential wrong `fromDTOs` overload via static import.**
This file statically imports `DTOConverters.fromDTOs`. This PR adds
`fromDTOs(RepresentationDTO[])` but not `fromDTOs(ColumnDTO[])`. If a
pre-existing `fromDTOs(ColumnDTO[])` overload does not exist, this will not
compile.
Additionally, the call style is inconsistent: static import for columns,
qualified `DTOConverters.fromDTOs(...)` for representations on the next line.
Please use one style consistently and verify the column overload exists.
--
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]
