[PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["contrueCT (via GitHub)" <gi...@apache.org>]

contrueCT opened a new pull request, #2966:
URL: https://github.com/apache/hugegraph/pull/2966

   <!-- 
     Thank you very much for contributing to Apache HugeGraph, we are happy that you want to help us improve it!
   
     Here are some tips for you:
       1. If this is your first time, please read the [contributing guidelines](https://github.com/apache/hugegraph/blob/master/CONTRIBUTING.md)
   
       2. If a PR fix/close an issue, type the message "close xxx" (xxx is the link of related 
   issue) in the content, GitHub will auto link it (Required)
   
       3. Name the PR title in "Google Commit Format", start with "feat | fix | perf | refactor | doc | chore", 
         such like: "feat(core): support the PageRank algorithm" or "fix: wrong break in the compute loop" (module is optional)
         skip it if you are unsure about which is the best component.
   
       4. One PR address one issue, better not to mix up multiple issues.
   
       5. Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]` (or click it directly after 
   published)
   -->
   
   ## Purpose of the PR
   
   - close #2964 <!-- or use "fix #xxx", "xxx" is the ID-link of related issue, e.g: close #1024 -->
   
   <!--
   Please explain more context in this section, clarify why the changes are needed. 
   e.g:
   - If you propose a new API, clarify the use case for a new API.
   - If you fix a bug, you can clarify why it is a bug, and should be associated with an issue.
   -->
   
   ## Main Changes
   
   The issue requires GraphSpaceAPI to return a friendly response in standalone mode. During code investigation, I found that ManagerAPI has a similar problem: in standalone mode, manager.graphSpace(name) returns a fake new GraphSpace("DEFAULT"), which causes ManagerAPI.validGraphSpace() to pass incorrectly. As a result, ManagerAPI can still enter logic that is not actually supported in standalone mode, leading to unfriendly errors or behavior that does not match the API semantics. For consistency and correctness, I applied the same standalone-mode guard to ManagerAPI as well.
   
   
   - Add public isUsePD() accessor to GraphManager to expose PD status
   - Add checkPdModeEnabled() helper in API base class
   - Call checkPdModeEnabled() in all public methods of GraphSpaceAPI (list/get/create/manage/delete)
   - Call checkPdModeEnabled() in all public methods of ManagerAPI (createManager/delete/list/checkRole/getRolesInGs)
   - Returns HTTP 400 with message 'GraphSpace management is not supported in standalone mode'
   - Add standalone-mode rejection tests in GraphSpaceApiTest and ManagerApiTest
   
   <!-- Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. These change logs are helpful for better and faster reviews.)
   
   For example:
   
   - If you introduce a new feature, please show detailed design here or add the link of design documentation.
   - If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
   - If there is a discussion in the mailing list, please add the link. -->
   
   ## Verifying these changes
   
   <!-- Please pick the proper options below -->
   
   - [ ] Trivial rework / code cleanup without any test coverage. (No Need)
   - [ ] Already covered by existing tests, such as *(please modify tests here)*.
   - [ ] Need tests and can be verified as follows:
       - xxx
   
   ## Does this PR potentially affect the following parts?
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ]  Dependencies ([add/update license](https://hugegraph.apache.org/docs/contribution-guidelines/contribute/#321-check-licenses) info & [regenerate_known_dependencies.sh](../install-dist/scripts/dependency/regenerate_known_dependencies.sh)) <!-- Don't forget to add/update the info in "LICENSE" & "NOTICE" files (both in root & dist module) -->
   - [ ]  Modify configurations
   - [ ]  The public API
   - [ ]  Other affects (typed here)
   - [x]  Nope
   
   
   ## Documentation Status
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ]  `Doc - TODO` <!-- Your PR changes impact docs and you will update later -->
   - [ ]  `Doc - Done` <!-- Related docs have been already added or updated -->
   - [x]  `Doc - No Need` <!-- Your PR changes don't impact/need docs -->
   


-- 
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: commits-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2917494522


##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/GraphSpaceApiStandaloneTest.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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;
+
+import java.util.Map;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import jakarta.ws.rs.core.Response;
+
+/**
+ * Tests that GraphSpaceAPI returns a friendly HTTP 400 error in standalone
+ * mode (i.e. when the server is started without PD / hstore backend).
+ * <p>
+ * This class intentionally does NOT have a class-level Assume guard so that
+ * the tests are actually executed in non-hstore CI runs.
+ */
+public class GraphSpaceApiStandaloneTest extends BaseApiTest {
+
+    private static final String PATH = "graphspaces";
+    private static final String STANDALONE_ERROR =
+            "GraphSpace management is not supported in standalone mode";
+
+    @Before
+    public void skipForPdMode() {
+        assumeStandaloneMode();
+    }
+
+    @Test

Review Comment:
   ‼️ The standalone suite still doesn't cover `/graphspaces/profile`, and `GraphSpaceAPI#listProfile()` remains unguarded on the current head. That means this PR can still pass while one GraphSpace endpoint returns a different standalone behavior than the others. Please add a profile assertion here and guard `listProfile()` the same way as the rest of the GraphSpace endpoints.
   ```suggestion
       @Test
       public void testProfileReturnsFriendlyError() {
           Response r = this.client().get(PATH + "/profile");
           String content = assertResponseStatus(400, r);
           Assert.assertTrue(content.contains(STANDALONE_ERROR));
       }
   
       @Test
   ```



-- 
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: commits-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2917501717


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java:
##########
@@ -76,6 +76,7 @@ public class GraphSpaceAPI extends API {
     @Produces(APPLICATION_JSON_WITH_CHARSET)
     public Object list(@Context GraphManager manager,
                        @Context SecurityContext sc) {
+        checkPdModeEnabled(manager);
         Set<String> spaces = manager.graphSpaces();
         return ImmutableMap.of("graphSpaces", spaces);

Review Comment:
   GraphSpaceAPI is intended to be disabled in standalone mode, but only some endpoints call checkPdModeEnabled(). The /graphspaces/profile endpoint (listProfile) is still reachable without the guard, so standalone mode can still access GraphSpace-related management data. Consider adding the same checkPdModeEnabled(manager) at the start of listProfile (and any other GraphSpaceAPI endpoints) to fully enforce the standalone restriction.



##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ManagerApiStandaloneTest.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * 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;
+
+import java.util.Map;
+
+import org.apache.hugegraph.auth.HugePermission;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import jakarta.ws.rs.core.Response;
+
+/**
+ * Tests that ManagerAPI returns a friendly HTTP 400 error in standalone mode
+ * (i.e. when the server is started without PD / hstore backend).
+ * <p>
+ * This class intentionally does NOT have a class-level Assume guard so that
+ * the tests are actually executed in non-hstore CI runs.
+ */
+public class ManagerApiStandaloneTest extends BaseApiTest {
+
+    private static final String STANDALONE_ERROR =
+            "GraphSpace management is not supported in standalone mode";
+
+    private static String managerPath(String graphSpace) {
+        return String.format("graphspaces/%s/auth/managers", graphSpace);
+    }
+
+    @Before
+    public void skipForPdMode() {
+        assumeStandaloneMode();
+    }
+
+    @Test
+    public void testCreateManagerReturnsFriendlyError() {
+        String body = "{\"user\":\"admin\",\"type\":\"ADMIN\"}";
+        Response r = this.client().post(managerPath("DEFAULT"), body);
+        String content = assertResponseStatus(400, r);
+        Assert.assertTrue(content.contains(STANDALONE_ERROR));
+    }
+
+    @Test
+    public void testDeleteManagerReturnsFriendlyError() {
+        Response r = this.client().delete(managerPath("DEFAULT"),
+                                          Map.of("user", "admin",
+                                                 "type", HugePermission.ADMIN));
+        String content = assertResponseStatus(400, r);
+        Assert.assertTrue(content.contains(STANDALONE_ERROR));
+    }
+
+    @Test
+    public void testListManagerReturnsFriendlyError() {
+        Response r = this.client().get(managerPath("DEFAULT"),
+                                       Map.of("type", (Object)HugePermission.ADMIN));
+        String content = assertResponseStatus(400, r);
+        Assert.assertTrue(content.contains(STANDALONE_ERROR));

Review Comment:
   This test uses java.util.Map.of(...) for query params, but other API tests in this module consistently use Guava ImmutableMap.of(...). Using ImmutableMap here would avoid the extra (Object) casts needed for type inference and keep parameter-map construction consistent across the test suite.



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -280,6 +280,10 @@ private boolean usePD() {
         return this.PDExist;
     }
 
+    public boolean isUsePD() {
+        return this.PDExist;

Review Comment:
   GraphManager already has a private usePD() helper used internally; adding a separate public isUsePD() with the same implementation duplicates the source of truth and risks divergence over time. Consider either making usePD() public (or package-private) and using it consistently, or having isUsePD() delegate to usePD().
   ```suggestion
           return this.usePD();
   ```



##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/GraphSpaceApiStandaloneTest.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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;
+
+import java.util.Map;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import jakarta.ws.rs.core.Response;
+
+/**
+ * Tests that GraphSpaceAPI returns a friendly HTTP 400 error in standalone
+ * mode (i.e. when the server is started without PD / hstore backend).
+ * <p>
+ * This class intentionally does NOT have a class-level Assume guard so that
+ * the tests are actually executed in non-hstore CI runs.
+ */
+public class GraphSpaceApiStandaloneTest extends BaseApiTest {
+
+    private static final String PATH = "graphspaces";
+    private static final String STANDALONE_ERROR =
+            "GraphSpace management is not supported in standalone mode";
+
+    @Before
+    public void skipForPdMode() {
+        assumeStandaloneMode();
+    }
+
+    @Test
+    public void testListReturnsFriendlyError() {
+        Response r = this.client().get(PATH);
+        String content = assertResponseStatus(400, r);
+        Assert.assertTrue(content.contains(STANDALONE_ERROR));
+    }
+
+    @Test
+    public void testGetReturnsFriendlyError() {
+        Response r = this.client().get(PATH, "DEFAULT");
+        String content = assertResponseStatus(400, r);
+        Assert.assertTrue(content.contains(STANDALONE_ERROR));
+    }
+
+    @Test
+    public void testCreateReturnsFriendlyError() {
+        String body = "{\"name\":\"test_standalone\",\"nickname\":\"test\","
+                      + "\"description\":\"test\",\"cpu_limit\":10,"
+                      + "\"memory_limit\":10,\"storage_limit\":10,"
+                      + "\"max_graph_number\":10,\"max_role_number\":10,"
+                      + "\"auth\":false,\"configs\":{}}";
+        Response r = this.client().post(PATH, body);
+        String content = assertResponseStatus(400, r);
+        Assert.assertTrue(content.contains(STANDALONE_ERROR));
+    }
+
+    @Test
+    public void testManageReturnsFriendlyError() {
+        String body = "{\"action\":\"update\",\"update\":{\"name\":\"DEFAULT\"}}";
+        Response r = this.client().put(PATH, "DEFAULT", body, Map.of());
+        String content = assertResponseStatus(400, r);

Review Comment:
   This test uses Map.of() for the (empty) query-params map, while other API tests in this module use ImmutableMap.of(). Consider switching to ImmutableMap.of() for consistency with the rest of the test suite.



-- 
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: commits-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["contrueCT (via GitHub)" <gi...@apache.org>]

contrueCT commented on PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#issuecomment-4087535854

   @imbajin  The latest CI build failed, but it appears unrelated to my changes. Could you please re-run the CI?


-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2936757127


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }
     }
 

Review Comment:
   🧹 Minor: Extract the literal error message into a constant to avoid duplication and typos. For example:
   
   private static final String STANDALONE_GRAPHSPACE_ERROR = "GraphSpace management is not supported in standalone mode";
   
   Then throw new HugeException(STANDALONE_GRAPHSPACE_ERROR); and update tests to reference the constant where practical.



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2936769177


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }
     }
 
+    public static void checkPdModeEnabled(GraphManager manager) {

Review Comment:
   Consider renaming `checkPdModeEnabled(GraphManager manager)`. The current name is non-idiomatic for a method that throws on failure; prefer a verb that indicates it asserts/ensures the [3
 9mcondition, e.g. `ensurePdModeEnabled(...)` or `requirePdModeEnabled(...)`.
   
   Also consider the following improvements:
   - Add a defensive null-check for the `manager` parameter, e.g. `E.checkArgumentNotNull(manager, "manager can't be null")` to avoid potential NPEs.
   - Add Javadoc describing the method's behaviour and the exception it throws (`HugeException` when PD mode is not enabled).



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }
     }
 
+    public static void checkPdModeEnabled(GraphManager manager) {

Review Comment:
   This helper is declared `public static` on the widely-used `API` base class, which unnecessarily expands the public API surface. If this method is only used [
 39mby API subclasses, consider reducing visibility (for example `protected` or package-private) so it is not exposed as part of the public API.



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }
     }
 
+    public static void checkPdModeEnabled(GraphManager manager) {
+        if (!manager.usePD()) {
+            throw new HugeException(
+                    "GraphSpace management is not supported in standalone mode");

Review Comment:
   The error message string `"GraphSpace management is not supported in standalone mode"` is duplicated between this code and the tests (e.g. GraphSpaceApiStandaloneTest / ManagerApiStandaloneTest). Extract this literal
  into a single constant to avoid duplication and ensure consistency, for example:
   
   ```
   private static final String STANDALONE_ERROR =
           "GraphSpace management is not supported in standalone mode";
   ```
   
   If tests need to reference it, consider placing it in a shared test-friendly location or making it package-visible/public as appropriate.



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -276,7 +276,7 @@ private static String serviceId(String graphSpace, Service.ServiceType type,
                      .replace("_", "-").toLowerCase();
     }
 
-    private boolean usePD() {
+    public boolean usePD() {

Review Comment:
   The method name `usePD()` is ambiguous for a boolean getter and does not follow standard Java boolean naming conventions. Consider renaming to `isPdEnabled()` (or `[38;5
 ;15misUsePD()`) for clarity, and review whether it needs to be `public` or can be made less visible if used only internally.



##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java:
##########
@@ -748,6 +739,28 @@ public static RestClient analystClient(String graphSpace, String username) {
         return analystClient;
     }
 
+    /**
+     * Skips the current test if the server is running in hstore / PD mode.
+     * Treats both {@code "hstore"} and {@code null} (i.e. the property is not
+     * set, which is the default in hstore CI runs) as PD mode.
+     * Call this from a {@code @Before} method in standalone-only test classes.
+     */
+    public static void assumeStandaloneMode() {
+        String backend = System.getProperty("backend");
+        Assume.assumeTrue(
+                "skip standalone tests: backend is '" + backend + "' (hstore/PD mode)",
+                backend != null && !backend.equals("hstore"));

Review Comment:
   The current `assumeStandaloneMode()` uses a double-negative in the condition (`backend != null && !backend.equals("hstore")`), which reduces readability. For clarity and null-safety prefer an explicit bo
 olean variable and `Assume.assumeFalse`, for example:
   
   ```
   boolean isPdMode = backend == null || backend.equals("hstore");
   Assume.assumeFalse(
           "skip standalone tests: backend is '" + backend + "' (hstore/PD mode)",
           isPdMode);
   ```
   
   This makes the intent (`isPdMode`) explicit and avoids the double-negative.



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2936757831


##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java:
##########
@@ -748,6 +739,28 @@ public static RestClient analystClient(String graphSpace, String username) {
         return analystClient;
     }
 
+    /**
+     * Skips the current test if the server is running in hstore / PD mode.
+     * Treats both {@code "hstore"} and {@code null} (i.e. the property is not
+     * set, which is the default in hstore CI runs) as PD mode.
+     * Call this from a {@code @Before} method in standalone-only test classes.
+     */
+    public static void assumeStandaloneMode() {
+        String backend = System.getProperty("backend");

Review Comment:
   ⚠️ Suggestion: clarify and simplify assumeStandaloneMode()
   
   The current condition `Assume.assumeTrue(backend != null && !backend.equals("hstore"))` is a double-negative and can be hard to read. Consider expressing the skip condition explicitly with `assumeFalse` and avoiding[39
 m NPEs. For example:
   
   public static void assumeStandaloneMode() {
       String backend = System.getProperty("backend");
       boolean isPdMode = backend == null || "hstore".equals(backend);
       Assume.assumeFalse("skip standalone tests: backend='" + backend + "' (hstore/PD mode)", isPdMode);
   }
   
   This makes the intent clearer and avoids null checks in the boolean expression.



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["contrueCT (via GitHub)" <gi...@apache.org>]

contrueCT commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2919588048


##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java:
##########
@@ -193,7 +183,8 @@ protected static void waitTaskStatus(int task, Set<String> expectedStatus) {
                 Assert.fail(String.format("Failed to wait for task %s " +
                                           "due to timeout", task));
             }
-        } while (!expectedStatus.contains(status));
+        }
+        while (!expectedStatus.contains(status));

Review Comment:
   This is not a part of this pr.I am not sure its logic.



-- 
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: commits-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2936782282


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }
     }
 
+    public static void checkPdModeEnabled(GraphManager manager) {

Review Comment:
   ⚠️ Method name `checkPdModeEnabled()` is non-idiomatic for a method that throws — Java convention uses imperative names like `ensure*` or `require*` for assertion/guard methods. Consider renaming:
   
   ```java
   public static void ensurePdModeEnabled(GraphManager manager) {
   ```
   
   Also missing: a null-check for `manager` and a Javadoc comment explaining what exception is thrown and when.
   



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }
     }
 
+    public static void checkPdModeEnabled(GraphManager manager) {

Review Comment:
   ⚠️ Declaring this as `public static` on the widely-used `API` base class unnecessarily expands the public API surface. Since this guard is only called from API subclasses, consider reducing visibility to `protected`:
   
   ```java
   protected static void checkPdModeEnabled(GraphManager manager) {
   ```
   



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }
     }
 
+    public static void checkPdModeEnabled(GraphManager manager) {
+        if (!manager.usePD()) {
+            throw new HugeException(
+                    "GraphSpace management is not supported in standalone mode");

Review Comment:
   🧹 The error message `"GraphSpace management is not supported in standalone mode"` is duplicated across production code and tests. Extract it to a constant to avoid typos and make future changes easier:
   
   ```java
   private static final String PD_MODE_REQUIRED_MSG =
           "GraphSpace management is not supported in standalone 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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2958443350


##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java:
##########
@@ -74,6 +74,8 @@ public class BaseApiTest {
     private static final String GRAPH_VERTEX = "/graph/vertices";
     private static final String GRAPH_EDGE = "/graph/edges";
     private static final String BATCH = "/batch";
+    static final String STANDALONE_ERROR =
+            "GraphSpace management is not supported in standalone mode";

Review Comment:
   Should not add an error log in BaseTest



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2911691470


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java:
##########
@@ -76,6 +76,7 @@ public class GraphSpaceAPI extends API {
     @Produces(APPLICATION_JSON_WITH_CHARSET)
     public Object list(@Context GraphManager manager,
                        @Context SecurityContext sc) {
+        checkPdModeEnabled(manager);

Review Comment:
   ‼️ `checkPdModeEnabled()` is added to the other GraphSpace endpoints, but `/graphspaces/profile` is still unguarded. In standalone mode this method will continue to return a different result instead of the new 400, so the API contract remains inconsistent. Please add the same guard to `listProfile()` and extend the standalone coverage to hit `/graphspaces/profile` as well.



-- 
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: commits-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2936778089


##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java:
##########
@@ -748,6 +739,28 @@ public static RestClient analystClient(String graphSpace, String username) {
         return analystClient;
     }
 
+    /**
+     * Skips the current test if the server is running in hstore / PD mode.
+     * Treats both {@code "hstore"} and {@code null} (i.e. the property is not
+     * set, which is the default in hstore CI runs) as PD mode.
+     * Call this from a {@code @Before} method in standalone-only test classes.
+     */
+    public static void assumeStandaloneMode() {
+        String backend = System.getProperty("backend");
+        Assume.assumeTrue(
+                "skip standalone tests: backend is '" + backend + "' (hstore/PD mode)",
+                backend != null && !backend.equals("hstore"));

Review Comment:
   ⚠️ `assumeStandaloneMode()` uses a double-negative condition (`backend != null && !backend.equals("hstore")`), which reduces readability. Prefer an explicit boolean variable and `Assume.assumeFalse(...)` for clarity and null-safety:
   
   ```java
   boolean isPdMode = "hstore".equals(backend);
   Assume.assumeFalse("Skip in PD/distributed mode", isPdMode);
   ```
   



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2936742351


##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ManagerApiTest.java:
##########
@@ -326,15 +322,15 @@ protected List<Map<String, Object>> listUsers(String graphSpace, String graph) {
         Response r = this.client().get(userPath, ImmutableMap.of("limit", NO_LIMIT));
         String result = assertResponseStatus(200, r);
 
-        Map<String, List<Map<String, Object>>> resultMap =
-                JsonUtil.fromJson(result, new TypeReference<Map<String,
-                        List<Map<String, Object>>>>() {
-                });
+        Map<String, List<Map<String, Object>>> resultMap = JsonUtil.fromJson(result,
+                                                                             new TypeReference<Map<String, List<Map<String, Object>>>>() {
+                                                                             });

Review Comment:
   This reformatted block exceeds the 100-character line length enforced by checkstyle (see `style/checkstyle.xml:29-30`). Line 326 is approximately 130+ characters due to deep alignment indentation. Consider breaking the `TypeReference` instantiation onto a separate variable or reducing the alignment depth.
   



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2958159664


##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/GraphSpaceApiStandaloneTest.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * 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;
+
+import com.google.common.collect.ImmutableMap;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import jakarta.ws.rs.core.Response;
+
+/**
+ * Tests that GraphSpaceAPI returns a friendly HTTP 400 error in standalone
+ * mode (i.e. when the server is started without PD / hstore backend).
+ * <p>
+ * This class intentionally does NOT have a class-level Assume guard so that
+ * the tests are actually executed in non-hstore CI runs.
+ */
+public class GraphSpaceApiStandaloneTest extends BaseApiTest {
+
+    private static final String PATH = "graphspaces";
+    private static final String STANDALONE_ERROR =
+            "GraphSpace management is not supported in standalone mode";

Review Comment:
   `STANDALONE_ERROR` is duplicated here and in `ManagerApiStandaloneTest`. To avoid future drift if the server message changes, consider centralizing this string in a shared test constant (e.g., `BaseApiTest`) and referencing it from both standalone-mode test classes.
   



##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ManagerApiStandaloneTest.java:
##########
@@ -0,0 +1,131 @@
+/*
+ * 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;
+
+import java.util.Map;
+
+import org.apache.hugegraph.auth.HugePermission;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import jakarta.ws.rs.core.Response;
+
+/**
+ * Tests that ManagerAPI returns a friendly HTTP 400 error in standalone mode
+ * (i.e. when the server is started without PD / hstore backend).
+ * <p>
+ * This class intentionally does NOT have a class-level Assume guard so that
+ * the tests are actually executed in non-hstore CI runs.
+ */
+public class ManagerApiStandaloneTest extends BaseApiTest {
+
+    private static final String STANDALONE_ERROR =
+            "GraphSpace management is not supported in standalone mode";
+

Review Comment:
   `STANDALONE_ERROR` is duplicated here and in `GraphSpaceApiStandaloneTest`. To avoid future drift if the server message changes, consider centralizing this string in a shared test constant (e.g., `BaseApiTest`) and referencing it from both standalone-mode test classes.



##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java:
##########
@@ -748,6 +739,27 @@ public static RestClient analystClient(String graphSpace, String username) {
         return analystClient;
     }
 
+    /**
+     * Skips the current test if the server is running in hstore / PD mode.
+     * Treats both {@code "hstore"} and {@code null} (i.e. the property is not
+     * set, which is the default in hstore CI runs) as PD mode.
+     * Call this from a {@code @Before} method in standalone-only test classes.
+     */
+    public static void assumeStandaloneMode() {
+        String backend = System.getProperty("backend");
+        boolean isPdMode = backend == null || "hstore".equals(backend);
+        Assume.assumeFalse("Skip in PD/distributed mode", isPdMode);

Review Comment:
   The Javadoc and assumption message imply `backend == null` is effectively PD/hstore mode. In this test module, `backend` is generally expected to be explicitly set (and other tests treat null as its own “unset/invalid” case). Consider updating the Javadoc and the Assume message to reflect that null means “backend not provided/unknown” (and is skipped for safety), rather than specifically “default in hstore CI runs”.
   



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2936755627


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }

Review Comment:
   ⚠️ Suggestion: rename checkPdModeEnabled to a clearer name and add null-check
   
   This method throws when PD mode is not enabled. Rename to make the contract explicit (for example ensurePdModeEnabled or requirePdModeEnabled) and add a null-check + [38;5
 ;204mJavadoc. Suggested change:
   
   public static void ensurePdModeEnabled(GraphManager manager) {
       E.checkArgumentNotNull(manager, "G[3
 8;5;198;48;5;233mraphManager must not be null");
       if (!manager.usePD()) {
           throw new HugeException("GraphSpac[38;5;198;4
 8;5;233me management is not supported in stan[38;5;198;48;
 5;233mdalone mode");
       }
   }
   
   Then update call sites (GraphSpaceAPI, ManagerAPI) to call ensurePdModeEnabled(manager).



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2936769177


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }
     }
 
+    public static void checkPdModeEnabled(GraphManager manager) {

Review Comment:
   Consider renaming `checkPdModeEnabled(GraphManager manager)`. The current name is non-idiomatic for a method that throws on failure; prefer a verb that indicates it asserts/ensures the [3
 9mcondition, e.g. `ensurePdModeEnabled(...)` or `requirePdModeEnabled(...)`.
   
   Also consider the following improvements:
   - Add a defensive null-check for the `manager` parameter, e.g. `E.checkArgumentNotNull(manager, "manager can't be null")` to avoid potential NPEs.
   - Add Javadoc describing the method's behaviour and the exception it throws (`HugeException` when PD mode is not enabled).



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }
     }
 
+    public static void checkPdModeEnabled(GraphManager manager) {

Review Comment:
   This helper is declared `public static` on the widely-used `API` base class, which unnecessarily expands the public API surface. If this method is only used [
 39mby API subclasses, consider reducing visibility (for example `protected` or package-private) so it is not exposed as part of the public API.



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }
     }
 
+    public static void checkPdModeEnabled(GraphManager manager) {
+        if (!manager.usePD()) {
+            throw new HugeException(
+                    "GraphSpace management is not supported in standalone mode");

Review Comment:
   The error message string `"GraphSpace management is not supported in standalone mode"` is duplicated between this code and the tests (e.g. GraphSpaceApiStandaloneTest / ManagerApiStandaloneTest). Extract this literal
  into a single constant to avoid duplication and ensure consistency, for example:
   
   ```
   private static final String STANDALONE_ERROR =
           "GraphSpace management is not supported in standalone mode";
   ```
   
   If tests need to reference it, consider placing it in a shared test-friendly location or making it package-visible/public as appropriate.



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -276,7 +276,7 @@ private static String serviceId(String graphSpace, Service.ServiceType type,
                      .replace("_", "-").toLowerCase();
     }
 
-    private boolean usePD() {
+    public boolean usePD() {

Review Comment:
   The method name `usePD()` is ambiguous for a boolean getter and does not follow standard Java boolean naming conventions. Consider renaming to `isPdEnabled()` (or `[38;5
 ;15misUsePD()`) for clarity, and review whether it needs to be `public` or can be made less visible if used only internally.



##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java:
##########
@@ -748,6 +739,28 @@ public static RestClient analystClient(String graphSpace, String username) {
         return analystClient;
     }
 
+    /**
+     * Skips the current test if the server is running in hstore / PD mode.
+     * Treats both {@code "hstore"} and {@code null} (i.e. the property is not
+     * set, which is the default in hstore CI runs) as PD mode.
+     * Call this from a {@code @Before} method in standalone-only test classes.
+     */
+    public static void assumeStandaloneMode() {
+        String backend = System.getProperty("backend");
+        Assume.assumeTrue(
+                "skip standalone tests: backend is '" + backend + "' (hstore/PD mode)",
+                backend != null && !backend.equals("hstore"));

Review Comment:
   The current `assumeStandaloneMode()` uses a double-negative in the condition (`backend != null && !backend.equals("hstore")`), which reduces readability. For clarity and null-safety prefer an explicit bo
 olean variable and `Assume.assumeFalse`, for example:
   
   ```
   boolean isPdMode = backend == null || backend.equals("hstore");
   Assume.assumeFalse(
           "skip standalone tests: backend is '" + backend + "' (hstore/PD mode)",
           isPdMode);
   ```
   
   This makes the intent (`isPdMode`) explicit and avoids the double-negative.



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2919474145


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }
     }
 
+    public static void checkPdModeEnabled(GraphManager manager) {
+        if (!manager.usePD()) {
+            throw new HugeException(
+                    "GraphSpace management is not supported in standalone mode");
+        }
+    }

Review Comment:
   `checkPdModeEnabled()` is introduced as `public static` on a widely-used API base class, which unnecessarily expands the public surface area. Since it’s an internal guard used by subclasses, consider making it `protected` (or package-private) to avoid exposing it as a public utility.



##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java:
##########
@@ -193,7 +183,8 @@ protected static void waitTaskStatus(int task, Set<String> expectedStatus) {
                 Assert.fail(String.format("Failed to wait for task %s " +
                                           "due to timeout", task));
             }
-        } while (!expectedStatus.contains(status));
+        }
+        while (!expectedStatus.contains(status));

Review Comment:
   This changes the original `do { ... } while (...)` polling into an empty-body `while (...);` loop, and `status` is no longer updated inside the loop. This can spin forever (or at least behave incorrectly) if `status` isn’t already in `expectedStatus`. Restore the polling loop structure so `status` is refreshed each iteration and the loop has a real body.



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -276,7 +276,7 @@ private static String serviceId(String graphSpace, Service.ServiceType type,
                      .replace("_", "-").toLowerCase();
     }
 
-    private boolean usePD() {
+    public boolean usePD() {

Review Comment:
   For a public boolean accessor, `usePD()` is a bit ambiguous as a getter name. Consider renaming to a clearer boolean-style accessor (e.g., `isPdEnabled()` / `isUsePD()`), which also matches the PR description and improves readability at call sites.
   ```suggestion
       public boolean isPdEnabled() {
   ```



-- 
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: commits-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2930146640


##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java:
##########
@@ -519,19 +510,20 @@ protected static void initOrClear() {
                 // isn't hstore
                 BaseApiTest.clearData();
             }
-        } else {
+        }
+        else {

Review Comment:
   The brace style `}\nelse {` (else on a new line) deviates from the established codebase convention of `} else {` (else on the same line as the closing brace). This is consistently used throughout the codebase (e.g., `API.java:239`, `GraphSpaceAPI.java:129`, and dozens of other files). The same applies to the `}\ncatch` on line 537-538. These appear to be auto-formatter artifacts and should be reverted to match the codebase convention.



##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java:
##########
@@ -542,14 +534,15 @@ protected static <T> List<T> readList(String content,
             JavaType type = MAPPER.getTypeFactory()
                                   .constructParametricType(List.class, clazz);
             return MAPPER.readValue(element.toString(), type);
-        } catch (IOException e) {
+        }
+        catch (IOException e) {

Review Comment:
   Same issue: `}\ncatch` deviates from the codebase convention of `} catch {` on the same line. See the same convention established throughout the codebase (e.g., `API.java:239`, `GraphsAPI.java:113`).
   



-- 
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: commits-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2911693010


##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ManagerApiTest.java:
##########
@@ -981,4 +981,45 @@ public void testUserWithDifferentRolesInMultipleSpaces() {
         Assert.assertTrue("Only graphb1 should remain in spaceb",
                           graphsList.contains("graphb1") && !graphsList.contains("graphb2"));
     }
+
+    @Test
+    public void testStandaloneModeForbidsAllEndpoints() {
+        Assume.assumeFalse("skip this test for hstore (PD mode)",

Review Comment:
   ⚠️ This standalone test never actually runs. `ManagerApiTest#setUpClass()` has a class-level `Assume.assumeTrue(... hstore ...)`, so JUnit skips the whole class before the method-level `assumeFalse()` is reached here. That means this PR adds no regression coverage for the new standalone behavior. Please move these assertions into a test class that is allowed to run without PD, or relax the class-level guard so this method can execute.



-- 
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: commits-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2911593735


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }
     }
 
+    public static void checkPdModeEnabled(GraphManager manager) {
+        if (!manager.isUsePD()) {
+            throw new HugeException(

Review Comment:
   `HugeException` is a general internal exception and will typically be mapped to an HTTP 500 response, not 400 as intended and as tested. The tests assert HTTP 400 (`assertResponseStatus(400, r)`), but using `HugeException` directly will likely result in a 500. You should throw the appropriate exception type that maps to HTTP 400 (e.g., `IllegalArgumentException` or a `BadRequestException`), or use an existing API-level exception that carries the desired status code.
   ```suggestion
               throw new IllegalArgumentException(
   ```



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -280,6 +280,10 @@ private boolean usePD() {
         return this.PDExist;
     }
 
+    public boolean isUsePD() {
+        return this.PDExist;
+    }

Review Comment:
   The private `usePD()` method and the newly added public `isUsePD()` method have identical implementations. The private method should be removed or replaced by delegating to the public one to avoid duplicated logic.



-- 
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: commits-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["contrueCT (via GitHub)" <gi...@apache.org>]

contrueCT commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2912035511


##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ManagerApiTest.java:
##########
@@ -981,4 +981,45 @@ public void testUserWithDifferentRolesInMultipleSpaces() {
         Assert.assertTrue("Only graphb1 should remain in spaceb",
                           graphsList.contains("graphb1") && !graphsList.contains("graphb2"));
     }
+
+    @Test
+    public void testStandaloneModeForbidsAllEndpoints() {
+        Assume.assumeFalse("skip this test for hstore (PD mode)",

Review Comment:
   OK, I will create a new test class for standalone test.



-- 
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: commits-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2936756643


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -276,7 +276,7 @@ private static String serviceId(String graphSpace, Service.ServiceType type,
                      .replace("_", "-").toLowerCase();

Review Comment:
   ⚠️ Suggestion: rename public boolean usePD() to follow Java boolean getter conventions and add Javadoc
   
   Publicly exposing usePD() increases the class surface; prefer a clearer name such as isPdEnabled() and add javadoc describing PD semantics. Example:
   
   /** Returns true if placement driver (PD / hstore) is configured/available. */
   public boolean isPdEnabled() {
       return this.PDExist;
   }
   
   If callers outside core need the current name, keep usePD() as a deprecated delegate that calls the new method.



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2958443350


##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java:
##########
@@ -74,6 +74,8 @@ public class BaseApiTest {
     private static final String GRAPH_VERTEX = "/graph/vertices";
     private static final String GRAPH_EDGE = "/graph/edges";
     private static final String BATCH = "/batch";
+    static final String STANDALONE_ERROR =
+            "GraphSpace management is not supported in standalone mode";

Review Comment:
   Should not add an error log in BaseTest



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] refactor(server): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin merged PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966


-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2930102211


##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ManagerApiStandaloneTest.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * 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;
+
+import java.util.Map;
+
+import org.apache.hugegraph.auth.HugePermission;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import jakarta.ws.rs.core.Response;
+
+/**
+ * Tests that ManagerAPI returns a friendly HTTP 400 error in standalone mode
+ * (i.e. when the server is started without PD / hstore backend).
+ * <p>
+ * This class intentionally does NOT have a class-level Assume guard so that
+ * the tests are actually executed in non-hstore CI runs.
+ */
+public class ManagerApiStandaloneTest extends BaseApiTest {
+
+    private static final String STANDALONE_ERROR =
+            "GraphSpace management is not supported in standalone mode";
+
+    private static String managerPath(String graphSpace) {
+        return String.format("graphspaces/%s/auth/managers", graphSpace);
+    }
+
+    @Before
+    public void skipForPdMode() {
+        assumeStandaloneMode();
+    }
+
+    @Test
+    public void testCreateManagerReturnsFriendlyError() {
+        String body = "{\"user\":\"admin\",\"type\":\"ADMIN\"}";
+        Response r = this.client().post(managerPath("DEFAULT"), body);
+        String content = assertResponseStatus(400, r);
+        Assert.assertTrue(content.contains(STANDALONE_ERROR));
+    }
+
+    @Test
+    public void testDeleteManagerReturnsFriendlyError() {
+        Response r = this.client().delete(managerPath("DEFAULT"),
+                                          Map.of("user", "admin",
+                                                 "type", HugePermission.ADMIN));
+        String content = assertResponseStatus(400, r);
+        Assert.assertTrue(content.contains(STANDALONE_ERROR));
+    }
+
+    @Test
+    public void testListManagerReturnsFriendlyError() {

Review Comment:
   ⚠️ The new standalone tests only exercise the `ADMIN` branch, so they don't actually lock down the regression described in the PR body. The problematic path was that `validGraphSpace()` could still succeed in standalone mode because `manager.graphSpace(name)` returns a synthetic `DEFAULT` space, and that only happens in the `SPACE` / `SPACE_MEMBER` branches.
   
   ASCII view:
   ```text
   checkPdModeEnabled()
           |
           +-- ADMIN         <- covered here
           +-- SPACE         <- regression path
           \-- SPACE_MEMBER <- regression path
   ```
   
   Please add at least one standalone test with `type=SPACE` or `type=SPACE_MEMBER` against a non-`DEFAULT` graphspace, so this PR verifies the branch that used to fall through.
   
   ```suggestion
       @Test
       public void testListSpaceManagerReturnsFriendlyError() {
           Response r = this.client().get(managerPath("nonexistent"),
                                          Map.of("type", (Object) HugePermission.SPACE));
           String content = assertResponseStatus(400, r);
           Assert.assertTrue(content.contains(STANDALONE_ERROR));
       }
   ```



-- 
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: commits-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2936782589


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -276,7 +276,7 @@ private static String serviceId(String graphSpace, Service.ServiceType type,
                      .replace("_", "-").toLowerCase();
     }
 
-    private boolean usePD() {
+    public boolean usePD() {

Review Comment:
   ⚠️ `usePD()` is an ambiguous name for a public boolean accessor — Java convention for boolean getters is `is*` or `has*`. Consider renaming to `isPdEnabled()` or `isUsePD()` to make the intent clear:
   
   ```java
   public boolean isPdEnabled() {
       return this.PDExist;
   }
   ```
   



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2938108173


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -276,7 +276,7 @@ private static String serviceId(String graphSpace, Service.ServiceType type,
                      .replace("_", "-").toLowerCase();
     }
 
-    private boolean usePD() {
+    public boolean isPDEnable() {
         return this.PDExist;
     }
 

Review Comment:
   The method name `isPDEnable` is grammatically incorrect — the conventional Java naming for a boolean getter would be `isPDEnabled` (past participle). This is a public API method on `GraphManager`, so it's worth getting the name right before it gets more callers.
   



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org

Re: [PR] optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode [hugegraph]

["imbajin (via GitHub)" <gi...@apache.org>]

imbajin commented on code in PR #2966:
URL: https://github.com/apache/hugegraph/pull/2966#discussion_r2936755627


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }

Review Comment:
   ⚠️ Suggestion: rename checkPdModeEnabled to a clearer name and add null-check
   
   This method throws when PD mode is not enabled. Rename to make the contract explicit (for example ensurePdModeEnabled or requirePdModeEnabled) and add a null-check + [38;5
 ;204mJavadoc. Suggested change:
   
   public static void ensurePdModeEnabled(GraphManager manager) {
       E.checkArgumentNotNull(manager, "G[3
 8;5;198;48;5;233mraphManager must not be null");
       if (!manager.usePD()) {
           throw new HugeException("GraphSpac[38;5;198;4
 8;5;233me management is not supported in stan[38;5;198;48;
 5;233mdalone mode");
       }
   }
   
   Then update call sites (GraphSpaceAPI, ManagerAPI) to call ensurePdModeEnabled(manager).



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -276,7 +276,7 @@ private static String serviceId(String graphSpace, Service.ServiceType type,
                      .replace("_", "-").toLowerCase();

Review Comment:
   ⚠️ Suggestion: rename public boolean usePD() to follow Java boolean getter conventions and add Javadoc
   
   Publicly exposing usePD() increases the class surface; prefer a clearer name such as isPdEnabled() and add javadoc describing PD semantics. Example:
   
   /** Returns true if placement driver (PD / hstore) is configured/available. */
   public boolean isPdEnabled() {
       return this.PDExist;
   }
   
   If callers outside core need the current name, keep usePD() as a deprecated delegate that calls the new method.



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -241,6 +241,13 @@ public static boolean checkAndParseAction(String action) {
         }
     }
 

Review Comment:
   🧹 Minor: Extract the literal error message into a constant to avoid duplication and typos. For example:
   
   private static final String STANDALONE_GRAPHSPACE_ERROR = "GraphSpace management is not supported in standalone mode";
   
   Then throw new HugeException(STANDALONE_GRAPHSPACE_ERROR); and update tests to reference the constant where practical.



##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java:
##########
@@ -748,6 +739,28 @@ public static RestClient analystClient(String graphSpace, String username) {
         return analystClient;
     }
 
+    /**
+     * Skips the current test if the server is running in hstore / PD mode.
+     * Treats both {@code "hstore"} and {@code null} (i.e. the property is not
+     * set, which is the default in hstore CI runs) as PD mode.
+     * Call this from a {@code @Before} method in standalone-only test classes.
+     */
+    public static void assumeStandaloneMode() {
+        String backend = System.getProperty("backend");

Review Comment:
   ⚠️ Suggestion: clarify and simplify assumeStandaloneMode()
   
   The current condition `Assume.assumeTrue(backend != null && !backend.equals("hstore"))` is a double-negative and can be hard to read. Consider expressing the skip condition explicitly with `assumeFalse` and avoiding[39
 m NPEs. For example:
   
   public static void assumeStandaloneMode() {
       String backend = System.getProperty("backend");
       boolean isPdMode = backend == null || "hstore".equals(backend);
       Assume.assumeFalse("skip standalone tests: backend='" + backend + "' (hstore/PD mode)", isPdMode);
   }
   
   This makes the intent clearer and avoids null checks in the boolean expression.



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org