Copilot commented on code in PR #3008:
URL: https://github.com/apache/hugegraph/pull/3008#discussion_r3186315575
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManagerV2.java:
##########
@@ -1510,6 +1516,189 @@ public HugeGroup findGroup(String name) {
return result;
}
+ @Override
+ public void setDefaultGraph(String graphSpace, String graph, String user) {
+ try {
+ HugeBelong belong = new HugeBelong(graphSpace,
+ IdGenerator.of(user),
+ IdGenerator.of(graph +
+
DEFAULT_SETTER_ROLE_KEY));
+ this.tryInitDefaultGraph(graphSpace, graph);
+ this.updateCreator(belong);
+ belong.create(belong.update());
+ this.metaManager.createBelong(graphSpace, belong);
+ this.invalidateUserCache();
+ } catch (Exception e) {
+ throw new HugeException("Exception occurs when " +
+ "set default graph", e);
+ }
+ }
+
+ @Override
+ public void unsetDefaultGraph(String graphSpace, String graph, String
user) {
+ String role = graph + DEFAULT_SETTER_ROLE_KEY;
+ String belongId = this.metaManager.belongId(user, role);
+ try {
+ this.metaManager.deleteBelong(graphSpace,
IdGenerator.of(belongId));
+ this.invalidateUserCache();
+ } catch (Exception e) {
+ throw new HugeException("Exception occurs when unset default " +
+ "graph", e);
+ }
Review Comment:
`unsetDefaultGraph()` will throw (and get wrapped into a `HugeException`) if
the belong record doesn't exist because `MetaManager.deleteBelong()` validates
existence. This makes the API non-idempotent and can surface as a 500 when
clients "unset" an already-unset default graph. Consider checking
`existBelong()` first (or catching `IllegalArgumentException` from
`deleteBelong` and treating it as a no-op) so `unset` is safe to call
repeatedly.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java:
##########
@@ -155,6 +313,61 @@ 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.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;
Review Comment:
This code calls `value.getClass()` in the validation message, but
`actionMap` may contain the `action` key with a null value. In that case `value
instanceof String` is false and `value.getClass()` will throw NPE while
building the error message. Add an explicit null check (or pass `value` safely)
so invalid requests reliably return a 4xx instead of NPE.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java:
##########
@@ -103,6 +108,141 @@ 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) {
+ E.checkArgumentNotNull(jsonRole, "Request body cannot be null");
+ E.checkArgument(StringUtils.isNotEmpty(jsonRole.user),
+ "The 'user' field cannot be null or empty");
+ E.checkArgument(StringUtils.isNotEmpty(jsonRole.role),
+ "The 'role' field cannot be null or empty");
+ String user = jsonRole.user;
+ String graph = jsonRole.graph;
+ HugeDefaultRole role;
+ try {
+ role = HugeDefaultRole.valueOf(jsonRole.role.toUpperCase());
+ } catch (IllegalArgumentException e) {
+ E.checkArgument(false, "Invalid role value '%s'", jsonRole.role);
+ role = null; // unreachable, satisfies compiler
+ }
+ 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 ForbiddenException("Forbidden to set role " +
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) {
+ E.checkArgument(StringUtils.isNotEmpty(user),
+ "The 'user' query param cannot be null or empty");
+ E.checkArgument(StringUtils.isNotEmpty(role),
+ "The 'role' query param cannot be null or empty");
+ LOG.debug("Check space role: {} {} {}", user, role,
+ name);
+ AuthManager authManager = manager.authManager();
+
+ HugeDefaultRole defaultRole;
+ try {
+ defaultRole = HugeDefaultRole.valueOf(role.toUpperCase());
+ } catch (IllegalArgumentException e) {
+ E.checkArgument(false, "Invalid role value '%s'", role);
+ defaultRole = null; // unreachable, satisfies compiler
+ }
+ 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) {
+ E.checkArgument(StringUtils.isNotEmpty(user),
+ "The 'user' query param cannot be null or empty");
+ E.checkArgument(StringUtils.isNotEmpty(role),
+ "The 'role' query param cannot be null or empty");
+ 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 ForbiddenException("Forbidden to delete role " + role);
+ }
+
+ HugeDefaultRole defaultRole =
+ HugeDefaultRole.valueOf(role.toUpperCase());
Review Comment:
`deleteDefaultRole()` parses the `role` query param via
`HugeDefaultRole.valueOf()` without handling `IllegalArgumentException`. For
invalid values this will bubble up as a 500 instead of a clean 4xx like the
POST/GET handlers above. Please wrap the parse in a try/catch and return an
argument error consistent with the other endpoints.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java:
##########
@@ -120,6 +128,86 @@ 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);
+ // graphSpace.nickname() may be null in non-PD mode (GraphManager
returns
+ // a placeholder GraphSpace without a nickname set)
+ String gsNickname = gs.nickname() != null ? gs.nickname() : graphSpace;
+
+ AuthManager authManager = manager.authManager();
+ String user = HugeGraphAuthProxy.username();
+ // Default graph concept relies on PD meta storage; in non-PD
standalone
+ // mode there is no persistent store for this, so we gracefully
degrade.
+ Map<String, Date> defaultGraphs;
+ if (manager.isPDEnabled()) {
+ defaultGraphs = authManager.getDefaultGraph(graphSpace, user);
+ } else {
+ defaultGraphs = new HashMap<>();
+ }
+
+ 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);
+ profile.put("name", graph);
+ profile.put("nickname", hg.nickname());
+ if (!isPrefix(profile, prefix)) {
+ continue;
+ }
+ profile.put("graphspace_nickname", gsNickname);
+
+ boolean isDefault = defaultGraphs.containsKey(graph);
+ profile.put("default", isDefault);
+ if (isDefault) {
+ profile.put("default_update_time",
defaultGraphs.get(graph));
Review Comment:
`default_update_time` is being set to a raw `java.util.Date` while
`create_time` is formatted as a string. This makes the response inconsistent
and may serialize differently depending on Jackson configuration. Consider
formatting `default_update_time` using the same `DATE_FORMATTER` (and timezone
conversion) as `create_time` for predictable frontend consumption.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java:
##########
@@ -136,6 +224,76 @@ public Object get(@Context GraphManager manager,
return ImmutableMap.of("name", g.name(), "backend", g.backend());
}
+ @POST
+ @Timed
+ @Path("{name}/default")
+ @Produces(APPLICATION_JSON_WITH_CHARSET)
+ @RolesAllowed({"space_member", "$owner=$name"})
+ public Map<String, Object> setDefault(@Context GraphManager manager,
+ @Parameter(description = "The graph
space name")
+ @PathParam("graphspace") String
graphSpace,
+ @Parameter(description = "The graph
name")
+ @PathParam("name") String name) {
+ LOG.debug("Set default graph '{}' in graph space '{}'", name,
graphSpace);
+ E.checkArgument(manager.graph(graphSpace, name) != null,
+ "Graph '%s/%s' does not exist", graphSpace, name);
+ // Default graph persistence requires PD meta storage.
+ // In non-PD (standalone RocksDB) mode, gracefully return empty.
+ if (!manager.isPDEnabled()) {
+ return ImmutableMap.of("default_graph", Collections.emptySet());
+ }
+ String user = HugeGraphAuthProxy.username();
+ AuthManager authManager = manager.authManager();
+ authManager.setDefaultGraph(graphSpace, name, user);
+ Map<String, Date> defaults = authManager.getDefaultGraph(graphSpace,
user);
+ return ImmutableMap.of("default_graph", defaults.keySet());
+ }
+
+ @DELETE
+ @Timed
+ @Path("{name}/default")
+ @Produces(APPLICATION_JSON_WITH_CHARSET)
+ @RolesAllowed({"space_member", "$owner=$name"})
Review Comment:
PR description says setting/unsetting default graph uses `GET
/graphs/{name}/default` and `GET /graphs/{name}/undefault`, but the
implementation uses `POST {name}/default` and `DELETE {name}/default`. Please
either update the PR description (preferred) or adjust the endpoint methods to
match the documented contract so frontend integrators don't get mismatched
expectations.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/SchemaTemplateAPI.java:
##########
@@ -0,0 +1,212 @@
+/*
+ * 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.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;
+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.annotation.security.RolesAllowed;
+import jakarta.inject.Singleton;
+import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.DELETE;
+import jakarta.ws.rs.ForbiddenException;
+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)
+ @RolesAllowed({"space_member", "$dynamic"})
+ 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)
+ @RolesAllowed({"space_member", "$dynamic"})
+ 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)
+ @RolesAllowed({"space_member", "$dynamic"})
+ 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)
+ @RolesAllowed({"space_member", "$dynamic"})
+ 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 ForbiddenException("No permission to delete schema
template");
+ }
+ }
+
+ @PUT
+ @Timed
+ @Path("{name}")
+ @Consumes(APPLICATION_JSON)
+ @Produces(APPLICATION_JSON_WITH_CHARSET)
+ @RolesAllowed({"space_member", "$dynamic"})
+ public String update(@Context GraphManager manager,
+ @PathParam("graphspace") String graphSpace,
+ @PathParam("name") String name,
+ @Context SecurityContext sc,
+ JsonSchemaTemplate jsonSchemaTemplate) {
Review Comment:
`jsonSchemaTemplate` can be null for an empty request body, but
`checkUpdate()` is called without validation, leading to an NPE/500. Please add
a null check and return a 4xx validation error when the body is missing.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/SchemaTemplateAPI.java:
##########
@@ -0,0 +1,212 @@
+/*
+ * 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.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;
+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.annotation.security.RolesAllowed;
+import jakarta.inject.Singleton;
+import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.DELETE;
+import jakarta.ws.rs.ForbiddenException;
+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)
+ @RolesAllowed({"space_member", "$dynamic"})
+ 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);
Review Comment:
This API uses meta-backed schema template operations, but it doesn't enforce
PD mode. In standalone mode this is likely to NPE (MetaManager not ready) or
behave unpredictably. Consider calling `ensurePdModeEnabled(manager)` at the
start of each handler (or otherwise returning a clear "not supported in
standalone" error).
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -2174,6 +2199,38 @@ public Map<String, Object> graphConfig(String graphSpace,
return this.metaManager.getGraphConfig(graphSpace, graphName);
}
+ /**
+ * Update the nickname of a graph.
+ * In non-PD mode (standalone RocksDB), only the in-memory instance is
updated
+ * since local config files cannot be hot-reloaded.
+ * In PD mode, the change is also persisted to the meta storage so it
+ * survives restarts.
+ */
+ public void updateGraphNickname(String graphSpace, String graphName,
+ String nickname) {
+ // Always update the in-memory graph instance first
+ HugeGraph g = this.graph(graphSpace, graphName);
+ if (g != null) {
+ g.nickname(nickname);
+ }
+ if (!isPDEnabled()) {
+ // Non-PD mode: in-memory only, acceptable for standalone RocksDB
+ return;
+ }
+ try {
+ Map<String, Object> configs =
+ this.metaManager.getGraphConfig(graphSpace, graphName);
+ if (configs != null) {
+ configs.put("nickname", nickname);
+ this.metaManager.updateGraphConfig(graphSpace, graphName,
configs);
+ this.metaManager.notifyGraphUpdate(graphSpace, graphName);
+ }
+ } catch (Exception e) {
+ LOG.warn("Failed to persist nickname for graph '{}/{}': {}",
+ graphSpace, graphName, e.getMessage());
+ }
Review Comment:
In PD mode, failures to persist the nickname are swallowed and the API will
still report success (the in-memory nickname is updated but meta storage may
not be). This can mislead callers and cause the nickname to revert on restart.
Consider propagating the exception (or at least returning an error to the API
layer) when `updateGraphConfig/notifyGraphUpdate` fails in PD mode.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/SchemaTemplateAPI.java:
##########
@@ -0,0 +1,212 @@
+/*
+ * 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.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;
+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.annotation.security.RolesAllowed;
+import jakarta.inject.Singleton;
+import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.DELETE;
+import jakarta.ws.rs.ForbiddenException;
+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)
+ @RolesAllowed({"space_member", "$dynamic"})
+ 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)
+ @RolesAllowed({"space_member", "$dynamic"})
+ 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)
+ @RolesAllowed({"space_member", "$dynamic"})
+ public String create(@Context GraphManager manager,
+ @PathParam("graphspace") String graphSpace,
+ JsonSchemaTemplate jsonSchemaTemplate) {
+ LOG.debug("Create schema template {} for graph space: '{}'",
+ jsonSchemaTemplate, graphSpace);
Review Comment:
`jsonSchemaTemplate` is dereferenced immediately (`checkCreate(false)`)
without a null check. If the request body is missing/empty, this will throw NPE
and return a 500. Add `E.checkArgumentNotNull(jsonSchemaTemplate, ...)` (or
equivalent) so the API returns a proper 4xx validation error.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -2182,6 +2239,27 @@ public String cluster() {
return this.cluster;
}
+ public Set<String> schemaTemplates(String graphSpace) {
+ return this.metaManager.schemaTemplates(graphSpace);
+ }
+
+ public void createSchemaTemplate(String graphSpace, SchemaTemplate
template) {
+ checkSchemaTemplateName(template.name());
+ this.metaManager.addSchemaTemplate(graphSpace, template);
+ }
+
+ public void dropSchemaTemplate(String graphSpace, String name) {
+ this.metaManager.removeSchemaTemplate(graphSpace, name);
+ }
+
+ public void updateSchemaTemplate(String graphSpace, SchemaTemplate
template) {
+ this.metaManager.updateSchemaTemplate(graphSpace, template);
+ }
+
Review Comment:
These schema-template methods call into `MetaManager` unconditionally. When
PD mode is disabled, `MetaManager` is typically not initialized and this can
lead to NPEs at runtime. Please either guard these methods with `isPDEnabled()`
(throw the same standalone error used elsewhere) or make them safely return
empty/unsupported in non-PD mode.
--
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]
