Copilot commented on code in PR #3008: URL: https://github.com/apache/hugegraph/pull/3008#discussion_r3142097462
########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/SchemaTemplateAPI.java: ########## @@ -0,0 +1,197 @@ +/* + * 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.hugegraph.api.space; + +import java.util.Date; +import java.util.Set; + +import org.apache.commons.lang3.StringUtils; +import org.apache.hugegraph.api.API; +import org.apache.hugegraph.api.filter.StatusFilter; +import org.apache.hugegraph.auth.HugeGraphAuthProxy; +import org.apache.hugegraph.core.GraphManager; +import org.apache.hugegraph.define.Checkable; +import org.apache.hugegraph.exception.HugeException; +import org.apache.hugegraph.server.RestServer; +import org.apache.hugegraph.space.SchemaTemplate; +import org.apache.hugegraph.util.E; +import org.apache.hugegraph.util.Log; +import org.slf4j.Logger; + +import com.codahale.metrics.annotation.Timed; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.collect.ImmutableMap; + +import io.swagger.v3.oas.annotations.tags.Tag; +import jakarta.inject.Singleton; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.DELETE; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.PUT; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.Context; +import jakarta.ws.rs.core.SecurityContext; + +@Path("graphspaces/{graphspace}/schematemplates") +@Singleton +@Tag(name = "SchemaTemplateAPI") +public class SchemaTemplateAPI extends API { + + private static final Logger LOG = Log.logger(RestServer.class); + + @GET + @Timed + @Produces(APPLICATION_JSON_WITH_CHARSET) + public Object list(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace) { + LOG.debug("List all schema templates for graph space {}", graphSpace); + + Set<String> templates = manager.schemaTemplates(graphSpace); + return ImmutableMap.of("schema_templates", templates); + } + + @GET + @Timed + @Path("{name}") + @Produces(APPLICATION_JSON_WITH_CHARSET) + public Object get(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace, + @PathParam("name") String name) { + LOG.debug("Get schema template by name '{}' for graph space {}", + name, graphSpace); + + return manager.serializer().writeSchemaTemplate( + schemaTemplate(manager, graphSpace, name)); + } + + @POST + @Timed + @StatusFilter.Status(StatusFilter.Status.CREATED) + @Consumes(APPLICATION_JSON) + @Produces(APPLICATION_JSON_WITH_CHARSET) + public String create(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace, + JsonSchemaTemplate jsonSchemaTemplate) { + LOG.debug("Create schema template {} for graph space: '{}'", + jsonSchemaTemplate, graphSpace); + jsonSchemaTemplate.checkCreate(false); + + E.checkArgument(manager.graphSpace(graphSpace) != null, + "The graph space '%s' is not exist", graphSpace); + + SchemaTemplate template = jsonSchemaTemplate.toSchemaTemplate(); + template.create(new Date()); + template.creator(HugeGraphAuthProxy.username()); + manager.createSchemaTemplate(graphSpace, template); + return manager.serializer().writeSchemaTemplate(template); + } + + @DELETE + @Timed + @Path("{name}") + @Produces(APPLICATION_JSON_WITH_CHARSET) + public void delete(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace, + @PathParam("name") String name, + @Context SecurityContext sc) { + LOG.debug("Remove schema template by name '{}' for graph space", + name, graphSpace); + E.checkArgument(manager.graphSpace(graphSpace) != null, + "The graph space '%s' is not exist", graphSpace); + + SchemaTemplate st = schemaTemplate(manager, graphSpace, name); + E.checkArgument(st != null, + "Schema template '%s' does not exist", name); + + String username = HugeGraphAuthProxy.username(); + boolean isSpace = manager.authManager() + .isSpaceManager(graphSpace, username); + if (st.creator().equals(username) || isSpace) { + manager.dropSchemaTemplate(graphSpace, name); + } else { + throw new HugeException("No permission to delete schema template"); + } + } + + @PUT + @Timed + @Path("{name}") + @Produces(APPLICATION_JSON_WITH_CHARSET) + public String update(@Context GraphManager manager, Review Comment: `update()` lacks `@Consumes(APPLICATION_JSON)` and doesn’t call `jsonSchemaTemplate.checkUpdate()`. If `schema` is missing/empty, the `SchemaTemplate` constructor will throw and likely produce a non-actionable error for clients; please add explicit validation and a consistent 400 response. ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java: ########## @@ -120,6 +128,85 @@ public Object list(@Context GraphManager manager, return ImmutableMap.of("graphs", filterGraphs); } + @GET + @Timed + @Path("profile") + @Produces(APPLICATION_JSON_WITH_CHARSET) + @RolesAllowed({"space_member", "$dynamic"}) + public Object listProfile(@Context GraphManager manager, + @Parameter(description = "The graph space name") + @PathParam("graphspace") String graphSpace, + @Parameter(description = "Filter graphs by name or nickname prefix") + @QueryParam("prefix") String prefix, + @Context SecurityContext sc) { + LOG.debug("List graph profiles in graph space {}", graphSpace); + if (null == manager.graphSpace(graphSpace)) { + throw new HugeException("Graphspace not exist!"); + } + GraphSpace gs = manager.graphSpace(graphSpace); + String gsNickname = gs.nickname(); + + AuthManager authManager = manager.authManager(); + String user = HugeGraphAuthProxy.username(); + Map<String, Date> defaultGraphs = authManager.getDefaultGraph(graphSpace, user); + + SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); + Set<String> graphs = manager.graphs(graphSpace); + List<Map<String, Object>> profiles = new ArrayList<>(); + List<Map<String, Object>> defaultProfiles = new ArrayList<>(); + for (String graph : graphs) { + String role = RequiredPerm.roleFor(graphSpace, graph, + HugePermission.READ); + if (!sc.isUserInRole(role)) { + continue; + } + try { + HugeGraph hg = graph(manager, graphSpace, graph); + HugeConfig config = (HugeConfig) hg.configuration(); + String configResp = ConfigUtil.writeConfigToString(config); + Map<String, Object> profile = + JsonUtil.fromJson(configResp, Map.class); Review Comment: `listProfile()` parses `ConfigUtil.writeConfigToString(config)` with `JsonUtil.fromJson(...)`. If the graph is using a local config file, that helper currently returns the raw `.properties` file content (not JSON), which will cause JSON parsing failures and break this endpoint. Ensure the config is returned in a JSON-compatible format (or avoid JSON parsing and build the profile map directly from `HugeConfig`). ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/SchemaTemplateAPI.java: ########## @@ -0,0 +1,197 @@ +/* + * 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.hugegraph.api.space; + +import java.util.Date; +import java.util.Set; + +import org.apache.commons.lang3.StringUtils; +import org.apache.hugegraph.api.API; +import org.apache.hugegraph.api.filter.StatusFilter; +import org.apache.hugegraph.auth.HugeGraphAuthProxy; +import org.apache.hugegraph.core.GraphManager; +import org.apache.hugegraph.define.Checkable; +import org.apache.hugegraph.exception.HugeException; Review Comment: `org.apache.hugegraph.exception.HugeException` doesn’t exist (exceptions in this package extend `org.apache.hugegraph.HugeException`). This import will break compilation; switch to `org.apache.hugegraph.HugeException` or a more specific exception type. ```suggestion import org.apache.hugegraph.HugeException; import org.apache.hugegraph.api.API; import org.apache.hugegraph.api.filter.StatusFilter; import org.apache.hugegraph.auth.HugeGraphAuthProxy; import org.apache.hugegraph.core.GraphManager; import org.apache.hugegraph.define.Checkable; ``` ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java: ########## @@ -155,6 +297,76 @@ public void drop(@Context GraphManager manager, manager.dropGraph(graphSpace, name, true); } + @PUT + @Timed + @Path("{name}") + @Consumes(APPLICATION_JSON) + @Produces(APPLICATION_JSON_WITH_CHARSET) + @RolesAllowed({"space"}) + public Map<String, String> manage(@Context GraphManager manager, + @Parameter(description = "The graph space name") + @PathParam("graphspace") String graphSpace, + @Parameter(description = "The graph name") + @PathParam("name") String name, + @Parameter(description = "Action map: {'action':'update','update':{...}}") + Map<String, Object> actionMap) { + LOG.debug("Manage graph '{}' with action '{}'", name, actionMap); + E.checkArgument(actionMap != null && actionMap.size() == 2 && + actionMap.containsKey(GRAPH_ACTION), + "Invalid request body '%s'", actionMap); + Object value = actionMap.get(GRAPH_ACTION); + E.checkArgument(value instanceof String, + "Invalid action type '%s', must be string", + value.getClass()); + String action = (String) value; + switch (action) { + case UPDATE: + E.checkArgument(actionMap.containsKey(UPDATE), + "Please pass '%s' for graph update", + UPDATE); + value = actionMap.get(UPDATE); + E.checkArgument(value instanceof Map, + "The '%s' must be map, but got %s", + UPDATE, value.getClass()); + @SuppressWarnings("unchecked") + Map<String, Object> graphMap = (Map<String, Object>) value; + String graphName = (String) graphMap.get("name"); + E.checkArgument(graphName != null && graphName.equals(name), + "Different name in update body '%s' with path '%s'", + graphName, name); + HugeGraph exist = graph(manager, graphSpace, name); + String nickname = (String) graphMap.get("nickname"); + if (!Strings.isEmpty(nickname)) { + GraphManager.checkNickname(nickname); + E.checkArgument(!manager.isExistedGraphNickname(graphSpace, nickname) || + nickname.equals(exist.nickname()), + "Nickname '%s' has already existed in graphspace '%s'", + nickname, graphSpace); Review Comment: In the `update` action, `exist.nickname(nickname)` only updates the in-memory `HugeGraph` instance; it doesn’t persist the nickname change to the graph metadata/config (so it will be lost after reload, and `isExistedGraphNickname()` checks may still see the old value). This endpoint should update the stored graph config via `metaManager.updateGraphConfig(...)`/`notifyGraphUpdate(...)` (or a dedicated `GraphManager` helper) in addition to updating the runtime object. ```suggestion nickname, graphSpace); Map<String, Object> updatedGraphConfig = new HashMap<>(); updatedGraphConfig.put("nickname", nickname); manager.meta().updateGraphConfig(graphSpace, name, updatedGraphConfig); manager.meta().notifyGraphUpdate(graphSpace, name); ``` ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/SchemaTemplateAPI.java: ########## @@ -0,0 +1,197 @@ +/* + * 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.hugegraph.api.space; + +import java.util.Date; +import java.util.Set; + +import org.apache.commons.lang3.StringUtils; +import org.apache.hugegraph.api.API; +import org.apache.hugegraph.api.filter.StatusFilter; +import org.apache.hugegraph.auth.HugeGraphAuthProxy; +import org.apache.hugegraph.core.GraphManager; +import org.apache.hugegraph.define.Checkable; +import org.apache.hugegraph.exception.HugeException; +import org.apache.hugegraph.server.RestServer; +import org.apache.hugegraph.space.SchemaTemplate; +import org.apache.hugegraph.util.E; +import org.apache.hugegraph.util.Log; +import org.slf4j.Logger; + +import com.codahale.metrics.annotation.Timed; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.collect.ImmutableMap; + +import io.swagger.v3.oas.annotations.tags.Tag; +import jakarta.inject.Singleton; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.DELETE; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.PUT; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.Context; +import jakarta.ws.rs.core.SecurityContext; + +@Path("graphspaces/{graphspace}/schematemplates") +@Singleton +@Tag(name = "SchemaTemplateAPI") +public class SchemaTemplateAPI extends API { + + private static final Logger LOG = Log.logger(RestServer.class); + + @GET + @Timed + @Produces(APPLICATION_JSON_WITH_CHARSET) + public Object list(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace) { + LOG.debug("List all schema templates for graph space {}", graphSpace); + + Set<String> templates = manager.schemaTemplates(graphSpace); + return ImmutableMap.of("schema_templates", templates); + } + + @GET + @Timed + @Path("{name}") + @Produces(APPLICATION_JSON_WITH_CHARSET) + public Object get(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace, + @PathParam("name") String name) { + LOG.debug("Get schema template by name '{}' for graph space {}", + name, graphSpace); + + return manager.serializer().writeSchemaTemplate( + schemaTemplate(manager, graphSpace, name)); + } + + @POST + @Timed + @StatusFilter.Status(StatusFilter.Status.CREATED) + @Consumes(APPLICATION_JSON) + @Produces(APPLICATION_JSON_WITH_CHARSET) + public String create(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace, + JsonSchemaTemplate jsonSchemaTemplate) { + LOG.debug("Create schema template {} for graph space: '{}'", + jsonSchemaTemplate, graphSpace); + jsonSchemaTemplate.checkCreate(false); + + E.checkArgument(manager.graphSpace(graphSpace) != null, + "The graph space '%s' is not exist", graphSpace); + + SchemaTemplate template = jsonSchemaTemplate.toSchemaTemplate(); + template.create(new Date()); + template.creator(HugeGraphAuthProxy.username()); + manager.createSchemaTemplate(graphSpace, template); + return manager.serializer().writeSchemaTemplate(template); + } + + @DELETE + @Timed + @Path("{name}") + @Produces(APPLICATION_JSON_WITH_CHARSET) + public void delete(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace, + @PathParam("name") String name, + @Context SecurityContext sc) { + LOG.debug("Remove schema template by name '{}' for graph space", + name, graphSpace); + E.checkArgument(manager.graphSpace(graphSpace) != null, + "The graph space '%s' is not exist", graphSpace); + + SchemaTemplate st = schemaTemplate(manager, graphSpace, name); + E.checkArgument(st != null, + "Schema template '%s' does not exist", name); + + String username = HugeGraphAuthProxy.username(); + boolean isSpace = manager.authManager() + .isSpaceManager(graphSpace, username); + if (st.creator().equals(username) || isSpace) { + manager.dropSchemaTemplate(graphSpace, name); + } else { + throw new HugeException("No permission to delete schema template"); + } Review Comment: Permission failures are thrown as `HugeException`, which the server maps to HTTP 400 (see `ExceptionFilter.HugeExceptionMapper`). For authorization failures, please throw `jakarta.ws.rs.ForbiddenException` (or another `WebApplicationException`) so clients receive a proper 403 response. ########## hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/util/ConfigUtil.java: ########## @@ -188,4 +192,30 @@ private static void validateGraphName(String graphName) { "Graph name can only contain letters, numbers, hyphens and underscores: %s", graphName); } + + public static String writeConfigToString(HugeConfig config) { + String content; + try { + if (config.file() == null) { + Map<String, Object> configMap = new HashMap<>(); + Iterator<String> iterator = config.getKeys(); + while (iterator.hasNext()) { + String key = iterator.next(); + configMap.put(key, config.getProperty(key)); + } + content = JsonUtil.toJson(configMap); + } else { + File file = config.file(); + if (file == null) { + throw new NotSupportedException( + "Can't access the api in a node which started " + + "with non local file config."); + } + content = FileUtils.readFileToString(file); + } + } catch (IOException e) { + throw new HugeException("Failed to read config of graph", e); + } + return content; Review Comment: `writeConfigToString()` returns raw file content when `config.file()` is non-null (likely a `.properties` file), but callers (e.g. `GraphsAPI.listProfile`) treat the result as JSON and parse it via `JsonUtil.fromJson(...)`, which will fail at runtime. Consider always serializing `config` to a JSON object (iterate `config.getKeys()` in both branches, or parse properties into a map) so the output format is consistent. ```suggestion Map<String, Object> configMap = new HashMap<>(); Iterator<String> iterator = config.getKeys(); while (iterator.hasNext()) { String key = iterator.next(); configMap.put(key, config.getProperty(key)); } return JsonUtil.toJson(configMap); ``` ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/ManagerAPI.java: ########## @@ -22,9 +22,12 @@ import java.util.ArrayList; import java.util.List; +import com.alipay.remoting.util.StringUtils; Review Comment: `ManagerAPI` imports `com.alipay.remoting.util.StringUtils`, but `hugegraph-api` explicitly excludes the Sofa Bolt dependency (`com.alipay.sofa:bolt`) in its POM, so this will fail compilation. Please switch to an available utility like `org.apache.commons.lang3.StringUtils` (already used elsewhere). ```suggestion import org.apache.commons.lang3.StringUtils; ``` ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java: ########## @@ -103,6 +106,118 @@ public Object get(@Context GraphManager manager, return gsInfo; } + @POST + @Timed + @Status(Status.CREATED) + @Consumes(APPLICATION_JSON) + @Produces(APPLICATION_JSON_WITH_CHARSET) + @Path("{graphspace}/role") + @RolesAllowed({"analyst"}) + public String setDefaultRole(@Context GraphManager manager, + @PathParam("graphspace") String name, + JsonDefaultRole jsonRole) { + String user = jsonRole.user; + String graph = jsonRole.graph; + HugeDefaultRole role = + HugeDefaultRole.valueOf(jsonRole.role.toUpperCase()); + LOG.debug("Create default role: {} {} {}", user, role, + name); + AuthManager authManager = manager.authManager(); + E.checkArgument(authManager.findUser(user) != null || + authManager.findGroup(user) != null, + "The user or group is not exist"); + // only admin can set space admin + if (!authManager.isAdminManager(HugeGraphAuthProxy.username()) && + role.equals(HugeDefaultRole.SPACE)) { + throw new HugeException("Forbidden to set role %s", role.toString()); + } + + boolean hasGraph = role.equals(HugeDefaultRole.OBSERVER); + + E.checkArgument(!hasGraph || StringUtils.isNotEmpty(graph), + "Must set a graph for observer"); + + Map <String, String> result = new HashMap<>(); + result.put("user", user); + result.put("role", jsonRole.role); + result.put("graphSpace", name); + + if (hasGraph) { + authManager.createDefaultRole(name, user, role, graph); + result.put("graph", graph); + } else { + authManager.createSpaceDefaultRole(name, user, role); + } + + return manager.serializer().writeMap(result); + } + + @GET + @Timed + @Path("{graphspace}/role") + @Consumes(APPLICATION_JSON) + @RolesAllowed("analyst") + public String checkDefaultRole(@Context GraphManager manager, + @PathParam("graphspace") String name, + @QueryParam("user") String user, + @QueryParam("role") String role, + @QueryParam("graph") String graph) { + LOG.debug("Check space role: {} {} {}", user, role, + name); + AuthManager authManager = manager.authManager(); + + HugeDefaultRole defaultRole = + HugeDefaultRole.valueOf(role.toUpperCase()); + boolean hasGraph = defaultRole.equals(HugeDefaultRole.OBSERVER); + E.checkArgument(!hasGraph || StringUtils.isNotEmpty(graph), + "Must set a graph for observer"); + + boolean result; + if (hasGraph) { + result = authManager.isDefaultRole(name, graph, user, + defaultRole); + } else { + result = authManager.isDefaultRole(name, user, + defaultRole); + } + return manager.serializer().writeMap(ImmutableMap.of("check", result)); + } + + @DELETE + @Timed + @Path("{graphspace}/role") + @Consumes(APPLICATION_JSON) + @RolesAllowed("analyst") + public void deleteDefaultRole(@Context GraphManager manager, + @PathParam("graphspace") String name, + @QueryParam("user") String user, + @QueryParam("role") String role, + @QueryParam("graph") String graph) { + LOG.debug("Delete space role: {} {} {}", user, role, + name); + + AuthManager authManager = manager.authManager(); + E.checkArgument(authManager.findUser(user) != null || + authManager.findGroup(user) != null, + "The user or group is not exist"); + + if (!authManager.isAdminManager(HugeGraphAuthProxy.username()) && + role.equalsIgnoreCase(HugeDefaultRole.SPACE.toString())) { + throw new HugeException("Forbidden to delete role %s", role); + } Review Comment: `deleteDefaultRole()` throws `HugeException` for permission denials (non-admin deleting SPACE role), which will be returned as HTTP 400. Please use `jakarta.ws.rs.ForbiddenException` (403) for authorization failures to match client expectations. ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java: ########## @@ -103,6 +106,118 @@ public Object get(@Context GraphManager manager, return gsInfo; } + @POST + @Timed + @Status(Status.CREATED) + @Consumes(APPLICATION_JSON) + @Produces(APPLICATION_JSON_WITH_CHARSET) + @Path("{graphspace}/role") + @RolesAllowed({"analyst"}) + public String setDefaultRole(@Context GraphManager manager, + @PathParam("graphspace") String name, + JsonDefaultRole jsonRole) { + String user = jsonRole.user; + String graph = jsonRole.graph; + HugeDefaultRole role = + HugeDefaultRole.valueOf(jsonRole.role.toUpperCase()); + LOG.debug("Create default role: {} {} {}", user, role, + name); + AuthManager authManager = manager.authManager(); + E.checkArgument(authManager.findUser(user) != null || + authManager.findGroup(user) != null, + "The user or group is not exist"); + // only admin can set space admin + if (!authManager.isAdminManager(HugeGraphAuthProxy.username()) && + role.equals(HugeDefaultRole.SPACE)) { + throw new HugeException("Forbidden to set role %s", role.toString()); + } + + boolean hasGraph = role.equals(HugeDefaultRole.OBSERVER); + + E.checkArgument(!hasGraph || StringUtils.isNotEmpty(graph), + "Must set a graph for observer"); + + Map <String, String> result = new HashMap<>(); + result.put("user", user); + result.put("role", jsonRole.role); + result.put("graphSpace", name); + + if (hasGraph) { + authManager.createDefaultRole(name, user, role, graph); + result.put("graph", graph); + } else { + authManager.createSpaceDefaultRole(name, user, role); + } + + return manager.serializer().writeMap(result); + } + + @GET + @Timed + @Path("{graphspace}/role") + @Consumes(APPLICATION_JSON) + @RolesAllowed("analyst") + public String checkDefaultRole(@Context GraphManager manager, + @PathParam("graphspace") String name, + @QueryParam("user") String user, + @QueryParam("role") String role, + @QueryParam("graph") String graph) { + LOG.debug("Check space role: {} {} {}", user, role, + name); + AuthManager authManager = manager.authManager(); + + HugeDefaultRole defaultRole = + HugeDefaultRole.valueOf(role.toUpperCase()); + boolean hasGraph = defaultRole.equals(HugeDefaultRole.OBSERVER); + E.checkArgument(!hasGraph || StringUtils.isNotEmpty(graph), + "Must set a graph for observer"); Review Comment: `checkDefaultRole()` calls `role.toUpperCase()` without checking `role` is non-null/non-empty. Please validate required query params (`user`, `role`, and `graph` when needed) and return a 400 with a clear message instead of risking an NPE. ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java: ########## @@ -103,6 +106,118 @@ public Object get(@Context GraphManager manager, return gsInfo; } + @POST + @Timed + @Status(Status.CREATED) + @Consumes(APPLICATION_JSON) + @Produces(APPLICATION_JSON_WITH_CHARSET) + @Path("{graphspace}/role") + @RolesAllowed({"analyst"}) + public String setDefaultRole(@Context GraphManager manager, + @PathParam("graphspace") String name, + JsonDefaultRole jsonRole) { + String user = jsonRole.user; + String graph = jsonRole.graph; + HugeDefaultRole role = + HugeDefaultRole.valueOf(jsonRole.role.toUpperCase()); + LOG.debug("Create default role: {} {} {}", user, role, Review Comment: `setDefaultRole()` calls `HugeDefaultRole.valueOf(jsonRole.role.toUpperCase())` without validating `jsonRole.role` (and `jsonRole.user/graph`) are present. If the frontend sends a missing/empty field, this will throw `NullPointerException`/`IllegalArgumentException` and likely become a 500. Please add request validation (e.g. implement & call `JsonDefaultRole.checkCreate()` with required fields). ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/SchemaTemplateAPI.java: ########## @@ -0,0 +1,197 @@ +/* + * 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.hugegraph.api.space; + +import java.util.Date; +import java.util.Set; + +import org.apache.commons.lang3.StringUtils; +import org.apache.hugegraph.api.API; +import org.apache.hugegraph.api.filter.StatusFilter; +import org.apache.hugegraph.auth.HugeGraphAuthProxy; +import org.apache.hugegraph.core.GraphManager; +import org.apache.hugegraph.define.Checkable; +import org.apache.hugegraph.exception.HugeException; +import org.apache.hugegraph.server.RestServer; +import org.apache.hugegraph.space.SchemaTemplate; +import org.apache.hugegraph.util.E; +import org.apache.hugegraph.util.Log; +import org.slf4j.Logger; + +import com.codahale.metrics.annotation.Timed; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.collect.ImmutableMap; + +import io.swagger.v3.oas.annotations.tags.Tag; +import jakarta.inject.Singleton; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.DELETE; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.PUT; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.Context; +import jakarta.ws.rs.core.SecurityContext; + +@Path("graphspaces/{graphspace}/schematemplates") +@Singleton +@Tag(name = "SchemaTemplateAPI") +public class SchemaTemplateAPI extends API { + + private static final Logger LOG = Log.logger(RestServer.class); + + @GET + @Timed + @Produces(APPLICATION_JSON_WITH_CHARSET) + public Object list(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace) { + LOG.debug("List all schema templates for graph space {}", graphSpace); + + Set<String> templates = manager.schemaTemplates(graphSpace); + return ImmutableMap.of("schema_templates", templates); + } + + @GET + @Timed + @Path("{name}") + @Produces(APPLICATION_JSON_WITH_CHARSET) + public Object get(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace, + @PathParam("name") String name) { + LOG.debug("Get schema template by name '{}' for graph space {}", + name, graphSpace); + + return manager.serializer().writeSchemaTemplate( + schemaTemplate(manager, graphSpace, name)); Review Comment: `schemaTemplate(manager, graphSpace, name)` is called but no such method exists in this class (and it’s not inherited from `API`). This will not compile; likely the intent was to call `manager.schemaTemplate(graphSpace, name)` (which exists) or add a private helper that delegates to `GraphManager`. ```suggestion manager.schemaTemplate(graphSpace, name)); ``` ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/SchemaTemplateAPI.java: ########## @@ -0,0 +1,197 @@ +/* + * 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.hugegraph.api.space; + +import java.util.Date; +import java.util.Set; + +import org.apache.commons.lang3.StringUtils; +import org.apache.hugegraph.api.API; +import org.apache.hugegraph.api.filter.StatusFilter; +import org.apache.hugegraph.auth.HugeGraphAuthProxy; +import org.apache.hugegraph.core.GraphManager; +import org.apache.hugegraph.define.Checkable; +import org.apache.hugegraph.exception.HugeException; +import org.apache.hugegraph.server.RestServer; +import org.apache.hugegraph.space.SchemaTemplate; +import org.apache.hugegraph.util.E; +import org.apache.hugegraph.util.Log; +import org.slf4j.Logger; + +import com.codahale.metrics.annotation.Timed; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.collect.ImmutableMap; + +import io.swagger.v3.oas.annotations.tags.Tag; +import jakarta.inject.Singleton; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.DELETE; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.PUT; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.Context; +import jakarta.ws.rs.core.SecurityContext; + +@Path("graphspaces/{graphspace}/schematemplates") +@Singleton +@Tag(name = "SchemaTemplateAPI") +public class SchemaTemplateAPI extends API { + + private static final Logger LOG = Log.logger(RestServer.class); + + @GET + @Timed + @Produces(APPLICATION_JSON_WITH_CHARSET) + public Object list(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace) { + LOG.debug("List all schema templates for graph space {}", graphSpace); + + Set<String> templates = manager.schemaTemplates(graphSpace); + return ImmutableMap.of("schema_templates", templates); + } + + @GET + @Timed + @Path("{name}") + @Produces(APPLICATION_JSON_WITH_CHARSET) + public Object get(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace, + @PathParam("name") String name) { + LOG.debug("Get schema template by name '{}' for graph space {}", + name, graphSpace); + + return manager.serializer().writeSchemaTemplate( + schemaTemplate(manager, graphSpace, name)); + } + + @POST + @Timed + @StatusFilter.Status(StatusFilter.Status.CREATED) + @Consumes(APPLICATION_JSON) + @Produces(APPLICATION_JSON_WITH_CHARSET) + public String create(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace, + JsonSchemaTemplate jsonSchemaTemplate) { + LOG.debug("Create schema template {} for graph space: '{}'", + jsonSchemaTemplate, graphSpace); + jsonSchemaTemplate.checkCreate(false); + + E.checkArgument(manager.graphSpace(graphSpace) != null, + "The graph space '%s' is not exist", graphSpace); + + SchemaTemplate template = jsonSchemaTemplate.toSchemaTemplate(); + template.create(new Date()); + template.creator(HugeGraphAuthProxy.username()); + manager.createSchemaTemplate(graphSpace, template); + return manager.serializer().writeSchemaTemplate(template); + } + + @DELETE + @Timed + @Path("{name}") + @Produces(APPLICATION_JSON_WITH_CHARSET) + public void delete(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace, + @PathParam("name") String name, + @Context SecurityContext sc) { + LOG.debug("Remove schema template by name '{}' for graph space", + name, graphSpace); + E.checkArgument(manager.graphSpace(graphSpace) != null, + "The graph space '%s' is not exist", graphSpace); + + SchemaTemplate st = schemaTemplate(manager, graphSpace, name); + E.checkArgument(st != null, + "Schema template '%s' does not exist", name); + + String username = HugeGraphAuthProxy.username(); + boolean isSpace = manager.authManager() + .isSpaceManager(graphSpace, username); + if (st.creator().equals(username) || isSpace) { + manager.dropSchemaTemplate(graphSpace, name); + } else { + throw new HugeException("No permission to delete schema template"); + } + } + + @PUT + @Timed + @Path("{name}") + @Produces(APPLICATION_JSON_WITH_CHARSET) + public String update(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace, + @PathParam("name") String name, + @Context SecurityContext sc, + JsonSchemaTemplate jsonSchemaTemplate) { + + SchemaTemplate old = schemaTemplate(manager, graphSpace, name); + if (null == old) { + throw new HugeException("Schema template {} does not exist", name); Review Comment: `throw new HugeException("Schema template {} does not exist", name)` uses `{}` placeholders, but HugeException messages in this codebase use `%s` formatting. As written, the client will likely see the literal `{}` instead of the template name. ```suggestion throw new HugeException("Schema template %s does not exist", name); ``` ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java: ########## @@ -103,6 +106,118 @@ public Object get(@Context GraphManager manager, return gsInfo; } + @POST + @Timed + @Status(Status.CREATED) + @Consumes(APPLICATION_JSON) + @Produces(APPLICATION_JSON_WITH_CHARSET) + @Path("{graphspace}/role") + @RolesAllowed({"analyst"}) + public String setDefaultRole(@Context GraphManager manager, + @PathParam("graphspace") String name, + JsonDefaultRole jsonRole) { + String user = jsonRole.user; + String graph = jsonRole.graph; + HugeDefaultRole role = + HugeDefaultRole.valueOf(jsonRole.role.toUpperCase()); + LOG.debug("Create default role: {} {} {}", user, role, + name); + AuthManager authManager = manager.authManager(); + E.checkArgument(authManager.findUser(user) != null || + authManager.findGroup(user) != null, + "The user or group is not exist"); + // only admin can set space admin + if (!authManager.isAdminManager(HugeGraphAuthProxy.username()) && + role.equals(HugeDefaultRole.SPACE)) { + throw new HugeException("Forbidden to set role %s", role.toString()); + } Review Comment: Authorization failures are thrown as `HugeException` (e.g. when a non-admin tries to set SPACE role), which the server maps to HTTP 400. For permission denials, throw `jakarta.ws.rs.ForbiddenException` (or another `WebApplicationException`) so clients get a proper 403 response. ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/ManagerAPI.java: ########## @@ -259,6 +262,40 @@ public String getRolesInGs(@Context GraphManager manager, result)); } + @GET + @Timed + @Path("default") + @Consumes(APPLICATION_JSON) + public String checkDefaultRole(@Context GraphManager manager, + @QueryParam("graphspace") String graphSpace, + @QueryParam("role") String role, + @QueryParam("graph") String graph) { + LOG.debug("check if current user is default role: {} {} {}", + role, graphSpace, graph); Review Comment: `checkDefaultRole()` doesn’t call `ensurePdModeEnabled(manager)` while the other endpoints in `ManagerAPI` do. If this API requires PD mode, add the same guard here to avoid inconsistent behavior when PD is disabled (or explicitly handle the non-PD case). ```suggestion role, graphSpace, graph); ensurePdModeEnabled(manager); ``` ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java: ########## @@ -1,38 +1,40 @@ /* - * Copyright 2017 HugeGraph Authors - * * 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 + * 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. + * 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.hugegraph.api.space; import java.text.SimpleDateFormat; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.lang.StringUtils; + import org.apache.hugegraph.api.API; import org.apache.hugegraph.api.filter.StatusFilter.Status; import org.apache.hugegraph.auth.AuthManager; +import org.apache.hugegraph.auth.HugeDefaultRole; import org.apache.hugegraph.auth.HugeGraphAuthProxy; import org.apache.hugegraph.core.GraphManager; import org.apache.hugegraph.define.Checkable; +import org.apache.hugegraph.exception.HugeException; import org.apache.hugegraph.exception.NotFoundException; Review Comment: `org.apache.hugegraph.exception.HugeException` doesn’t exist in the codebase (exceptions in this package extend `org.apache.hugegraph.HugeException`). This import will fail compilation; please switch to `org.apache.hugegraph.HugeException` (or a more specific exception type like `jakarta.ws.rs.ForbiddenException` where appropriate). ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java: ########## @@ -103,6 +106,118 @@ public Object get(@Context GraphManager manager, return gsInfo; } + @POST + @Timed + @Status(Status.CREATED) + @Consumes(APPLICATION_JSON) + @Produces(APPLICATION_JSON_WITH_CHARSET) + @Path("{graphspace}/role") + @RolesAllowed({"analyst"}) + public String setDefaultRole(@Context GraphManager manager, + @PathParam("graphspace") String name, + JsonDefaultRole jsonRole) { + String user = jsonRole.user; + String graph = jsonRole.graph; + HugeDefaultRole role = + HugeDefaultRole.valueOf(jsonRole.role.toUpperCase()); + LOG.debug("Create default role: {} {} {}", user, role, + name); + AuthManager authManager = manager.authManager(); + E.checkArgument(authManager.findUser(user) != null || + authManager.findGroup(user) != null, + "The user or group is not exist"); + // only admin can set space admin + if (!authManager.isAdminManager(HugeGraphAuthProxy.username()) && + role.equals(HugeDefaultRole.SPACE)) { + throw new HugeException("Forbidden to set role %s", role.toString()); + } + + boolean hasGraph = role.equals(HugeDefaultRole.OBSERVER); + + E.checkArgument(!hasGraph || StringUtils.isNotEmpty(graph), + "Must set a graph for observer"); + + Map <String, String> result = new HashMap<>(); + result.put("user", user); + result.put("role", jsonRole.role); + result.put("graphSpace", name); + + if (hasGraph) { + authManager.createDefaultRole(name, user, role, graph); + result.put("graph", graph); + } else { + authManager.createSpaceDefaultRole(name, user, role); + } + + return manager.serializer().writeMap(result); + } + + @GET + @Timed + @Path("{graphspace}/role") + @Consumes(APPLICATION_JSON) + @RolesAllowed("analyst") + public String checkDefaultRole(@Context GraphManager manager, + @PathParam("graphspace") String name, + @QueryParam("user") String user, + @QueryParam("role") String role, + @QueryParam("graph") String graph) { + LOG.debug("Check space role: {} {} {}", user, role, + name); + AuthManager authManager = manager.authManager(); + + HugeDefaultRole defaultRole = + HugeDefaultRole.valueOf(role.toUpperCase()); + boolean hasGraph = defaultRole.equals(HugeDefaultRole.OBSERVER); + E.checkArgument(!hasGraph || StringUtils.isNotEmpty(graph), + "Must set a graph for observer"); + + boolean result; + if (hasGraph) { + result = authManager.isDefaultRole(name, graph, user, + defaultRole); + } else { + result = authManager.isDefaultRole(name, user, + defaultRole); + } + return manager.serializer().writeMap(ImmutableMap.of("check", result)); + } + + @DELETE + @Timed + @Path("{graphspace}/role") + @Consumes(APPLICATION_JSON) + @RolesAllowed("analyst") + public void deleteDefaultRole(@Context GraphManager manager, + @PathParam("graphspace") String name, + @QueryParam("user") String user, + @QueryParam("role") String role, + @QueryParam("graph") String graph) { + LOG.debug("Delete space role: {} {} {}", user, role, + name); + + AuthManager authManager = manager.authManager(); + E.checkArgument(authManager.findUser(user) != null || + authManager.findGroup(user) != null, + "The user or group is not exist"); + + if (!authManager.isAdminManager(HugeGraphAuthProxy.username()) && + role.equalsIgnoreCase(HugeDefaultRole.SPACE.toString())) { + throw new HugeException("Forbidden to delete role %s", role); + } + + HugeDefaultRole defaultRole = + HugeDefaultRole.valueOf(role.toUpperCase()); + boolean hasGraph = defaultRole.equals(HugeDefaultRole.OBSERVER); Review Comment: In `deleteDefaultRole()`, `role.equalsIgnoreCase(...)` is evaluated before validating `role` is provided, so a missing `role` query param will cause a `NullPointerException`. Add `E.checkArgument(StringUtils.isNotEmpty(role), ...)` (and validate `user`/`graph` similarly) before any dereference. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
