imbajin commented on code in PR #685:
URL: 
https://github.com/apache/incubator-hugegraph-toolchain/pull/685#discussion_r2467769447


##########
hugegraph-client/src/main/java/org/apache/hugegraph/api/graphs/GraphsAPI.java:
##########
@@ -198,51 +207,85 @@ public Map<String, String> reload() {
     }
 
     public void mode(String graph, GraphMode mode) {
-        // NOTE: Must provide id for PUT. If you use "graph/mode", "/" will
-        // be encoded to "%2F". So use "mode" here, although inaccurate.
-        this.client.put(joinPath(this.path(), graph, MODE), null, mode);
+        mode(null, graph, mode);
     }
 
-    public GraphMode mode(String graph) {
-        RestResult result = this.client.get(joinPath(this.path(), graph), 
MODE);
-        @SuppressWarnings("unchecked")
-        Map<String, String> mode = result.readObject(Map.class);
-        String value = mode.get(MODE);
-        if (value == null) {
-            throw new InvalidResponseException("Invalid response, expect 
'mode' in response");
-        }
-        try {
-            return GraphMode.valueOf(value);
-        } catch (IllegalArgumentException e) {
-            throw new InvalidResponseException("Invalid GraphMode value '%s'", 
value);
+    public void mode(String graphSpace, String graph, GraphMode mode) {
+        // NOTE: Must provide id for PUT. If you use "graph/mode", "/" will
+        // be encoded to "%2F". So use "mode" here, although inaccurate.
+        if (graphSpace == null) {
+            this.client.put(joinPath(this.path(), graph, MODE), null, mode);
+            return;
         }
+        this.client.put(joinPath(this.path(), graphSpace, graph, MODE), null, 
mode);
     }
 
     public void readMode(String graph, GraphReadMode readMode) {
+        readMode(null, graph, readMode);
+    }
+
+
+    public void readMode(String graphSpace, String graph, GraphReadMode 
readMode) {
         this.client.checkApiVersion("0.59", "graph read mode");
         // NOTE: Must provide id for PUT. If you use "graph/graph_read_mode", 
"/"
         // will be encoded to "%2F". So use "graph_read_mode" here, although
         // inaccurate.
-        this.client.put(joinPath(this.path(), graph, GRAPH_READ_MODE), null, 
readMode);
+        if (graphSpace == null) {
+            this.client.put(joinPath(this.path(), graph, GRAPH_READ_MODE), 
null, readMode);
+            return;
+        }
+        this.client.put(joinPath(this.path(), graphSpace, graph, 
GRAPH_READ_MODE), null, readMode);
     }
 
-    public GraphReadMode readMode(String graph) {
-        this.client.checkApiVersion("0.59", "graph read mode");
-        RestResult result = this.client.get(joinPath(this.path(), graph), 
GRAPH_READ_MODE);
+    /**
+     * Get graph mode value from server response
+     *
+     * @param graphSpace the graph space name, null for non-graphspace mode
+     * @param graph the graph name
+     * @param modeKey the mode key in response (MODE or GRAPH_READ_MODE)

Review Comment:
   ‼️ **Critical**:  方法返回值未正确处理版本兼容性
   
   在  方法中,所有 graphSpace 不为 null 的调用都会使用新的路径格式,但这会导致在旧版本服务器(< 1.7.0)上调用失败。
   
   建议修改:
   ```java
   private <T extends Enum<T>> T getModeValue(String graphSpace, String graph,
                                             String modeKey, Class<T> 
enumClass) {
       // 检查服务器版本是否支持 graphSpace
       if (graphSpace != null && !this.client.isSupportGs()) {
           throw new ClientException(
               "GraphSpace feature requires server version >= 1.7.0");
       }
       
       String path = (graphSpace != null)
                     ? joinPath(this.path(), graphSpace, graph)
                     : joinPath(this.path(), graph);
       // ...
   }
   ```
   
   这样可以在客户端提前检测版本不兼容问题,避免发送无效请求。



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

Reply via email to