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


##########
hugegraph-client/src/main/java/org/apache/hugegraph/api/graphs/GraphsAPI.java:
##########
@@ -203,19 +203,26 @@ public void mode(String graph, GraphMode mode) {
         this.client.put(joinPath(this.path(), graph, MODE), null, 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");
+    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) {
+            mode(graph, mode);
+            return;
         }
-        try {
-            return GraphMode.valueOf(value);
-        } catch (IllegalArgumentException e) {
-            throw new InvalidResponseException("Invalid GraphMode value '%s'", 
value);
+        this.client.put(joinPath(this.path(), graphSpace, graph, MODE), null, 
mode);

Review Comment:
   ‼️ **代码重复问题 (Critical)**
   
   这段代码与旧的 `mode(String graph, GraphMode mode)` 方法完全相同。应该将旧方法重构为调用新方法,而不是保留重复代码。
   
   **建议修改:**
   ```java
   public void mode(String graph, GraphMode mode) {
       mode(null, graph, mode);
   }
   ```
   
   这样可以:
   1. 消除代码重复
   2. 保证逻辑一致性
   3. 便于后续维护



##########
hugegraph-client/src/main/java/org/apache/hugegraph/api/graphs/GraphsAPI.java:
##########
@@ -203,19 +203,26 @@ public void mode(String graph, GraphMode mode) {
         this.client.put(joinPath(this.path(), graph, MODE), null, 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");
+    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) {
+            mode(graph, mode);
+            return;
         }
-        try {
-            return GraphMode.valueOf(value);
-        } catch (IllegalArgumentException e) {
-            throw new InvalidResponseException("Invalid GraphMode value '%s'", 
value);
+        this.client.put(joinPath(this.path(), graphSpace, graph, MODE), null, 
mode);
+    }
+
+    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.
+        if (graphSpace == null) {
+            readMode(graph, readMode);
+            return;
         }
+        this.client.put(joinPath(this.path(), graphSpace, graph, 
GRAPH_READ_MODE), null, readMode);
     }
 
     public void readMode(String graph, GraphReadMode readMode) {

Review Comment:
   ‼️ **代码重复问题 (Critical)**
   
   与新增的 `readMode(String graphSpace, String graph, GraphReadMode readMode)` 
方法存在重复逻辑。
   
   **建议修改:**
   ```java
   public void readMode(String graph, GraphReadMode readMode) {
       readMode(null, graph, readMode);
   }
   ```
   
   保持 API 版本检查在被调用的方法中进行即可。



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