Tsukilc commented on code in PR #2937:
URL: https://github.com/apache/hugegraph/pull/2937#discussion_r3409185455
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -1937,26 +1938,29 @@ public Set<String> getServiceUrls(String graphSpace,
String service,
public HugeGraph graph(String graphSpace, String name) {
String key = String.join(DELIMITER, graphSpace, name);
Graph graph = this.graphs.get(key);
- if (graph == null && isPDEnabled()) {
- Map<String, Map<String, Object>> configs =
- this.metaManager.graphConfigs(graphSpace);
- // If current server registered graph space is not DEFAULT, only
load graph creation
- // under registered graph space
- if (!configs.containsKey(key) ||
- (!"DEFAULT".equals(this.serviceGraphSpace) &&
- !graphSpace.equals(this.serviceGraphSpace))) {
- return null;
+ if (graph == null) {
+ if (isPDEnabled()) {
+ Map<String, Map<String, Object>> configs =
+ this.metaManager.graphConfigs(graphSpace);
+ // If current server registered graph space is not DEFAULT,
only load graph creation
+ // under registered graph space
+ if (!configs.containsKey(key) ||
+ (!"DEFAULT".equals(this.serviceGraphSpace) &&
+ !graphSpace.equals(this.serviceGraphSpace))) {
+ return null;
+ }
+ Map<String, Object> config = configs.get(key);
+ String creator = String.valueOf(config.get("creator"));
+ Date createTime = parseDate(config.get("create_time"));
+ Date updateTime = parseDate(config.get("update_time"));
+ HugeGraph graph1 = this.createGraph(graphSpace, name,
+ creator, config, false);
+ graph1.createTime(createTime);
+ graph1.updateTime(updateTime);
+ this.graphs.put(key, graph1);
+ return graph1;
}
- Map<String, Object> config = configs.get(key);
- String creator = String.valueOf(config.get("creator"));
- Date createTime = parseDate(config.get("create_time"));
- Date updateTime = parseDate(config.get("update_time"));
- HugeGraph graph1 = this.createGraph(graphSpace, name,
- creator, config, false);
- graph1.createTime(createTime);
- graph1.updateTime(updateTime);
- this.graphs.put(key, graph1);
- return graph1;
+ throw new NotFoundException(String.format("Graph '%s' does not
exist", name));
Review Comment:
The throw was actually introduced to fix a latent NPE: previously, when PD
is disabled and the graph is missing, control fell through to throw new
NotSupportException("graph instance of %s", graph.getClass()) with graph ==
null — i.e. an NPE. The new NotFoundException is a strict improvement. I agree
the split between null (PD enabled, not visible) and throw (PD disabled,
missing) is an inconsistent contract worth unifying, but doing it safely
requires auditing every caller of graph(graphSpace, name) for null-handling,
which is broader than this PR's scope. Proposing to unify to a single nullable
contract in a dedicated follow-up.
--
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]
