feat(server): add gs profile api#2950
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new GraphSpace “profile” REST endpoint intended for Hubble to retrieve summarized graph-space metadata, and extends API tests to cover the new endpoint.
Changes:
- Add
GET /graphspaces/profilewith optionalprefixquery filter. - Format and enrich returned profile data with
authed,create_time,update_time, anddefaultfields. - Add API tests for profile listing, prefix filtering, and auth-related presence checks.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java |
Adds /graphspaces/profile endpoint and supporting permission helper. |
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/GraphSpaceApiTest.java |
Adds new tests for the profile API and adjusts some test payload strings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd hh:mm:ss"); | ||
| for (String sp : spaces) { | ||
| manager.getSpaceStorage(sp); | ||
| GraphSpace gs = space(manager, sp); | ||
| Map<String, Object> gsProfile = gs.info(); | ||
| boolean isManager = verifyPermission(user, authManager, sp); | ||
|
|
||
| // 设置当前用户的是否允许访问该空间 | ||
| if (gs.auth() && !isManager) { | ||
| gsProfile.put("authed", false); | ||
| } else { | ||
| gsProfile.put("authed", true); | ||
| } | ||
|
|
||
| gsProfile.put("create_time", format.format(gs.createTime())); | ||
| gsProfile.put("update_time", format.format(gs.updateTime())); |
There was a problem hiding this comment.
The date format pattern uses hh (12-hour clock) without an AM/PM marker, which can produce ambiguous timestamps. Also, elsewhere in the codebase (e.g., HugeGraphSONModule) dates are formatted with yyyy-MM-dd HH:mm:ss.SSS; consider using a consistent 24-hour format (and ideally the same formatter) for create_time/update_time, or avoid overriding the Date values from gs.info() and let the configured JSON Date serializer handle formatting.
| // Should contain at least the DEFAULT space | ||
| assert profiles.size() >= 1; | ||
|
|
||
| // Verify profile structure | ||
| for (Map<String, Object> profile : profiles) { | ||
| assert profile.containsKey("name"); | ||
| assert profile.containsKey("authed"); | ||
| assert profile.containsKey("create_time"); | ||
| assert profile.containsKey("update_time"); | ||
| assert profile.containsKey("default"); | ||
| } |
There was a problem hiding this comment.
These tests rely on Java assert statements for verification. Maven Surefire isn’t configured with -ea in this module, so these assertions may be skipped at runtime and the tests would still pass even if the endpoint is broken. Prefer JUnit assertions (e.g., Assert.assertTrue/False, assertEquals, etc.) for test validation.
| // Create spaces | ||
| Response r = this.client().post(PATH, space1); | ||
| assertResponseStatus(201, r); | ||
|
|
||
| // Test with prefix filter | ||
| r = this.client().get(PATH + "/profile", | ||
| ImmutableMap.of("prefix", "test")); | ||
| String result = assertResponseStatus(200, r); | ||
|
|
||
| List<Map<String, Object>> profiles = JsonUtil.fromJson(result, List.class); | ||
| assert !profiles.isEmpty(); |
There was a problem hiding this comment.
testListProfileWithPrefix() only asserts the returned list is non-empty, which doesn’t verify that the prefix filter is actually applied. To make the test meaningful, assert that every returned profile has name (and/or nickname) starting with the requested prefix, and ideally that profiles without the prefix are excluded.
| // Create spaces | |
| Response r = this.client().post(PATH, space1); | |
| assertResponseStatus(201, r); | |
| // Test with prefix filter | |
| r = this.client().get(PATH + "/profile", | |
| ImmutableMap.of("prefix", "test")); | |
| String result = assertResponseStatus(200, r); | |
| List<Map<String, Object>> profiles = JsonUtil.fromJson(result, List.class); | |
| assert !profiles.isEmpty(); | |
| String space2 = "{" | |
| + "\"name\":\"other_profile_space\"," | |
| + "\"nickname\":\"OtherProfileSpace\"," | |
| + "\"description\":\"Other profile listing\"," | |
| + "\"cpu_limit\":1000," | |
| + "\"memory_limit\":1024," | |
| + "\"storage_limit\":1000," | |
| + "\"compute_cpu_limit\":0," | |
| + "\"compute_memory_limit\":0," | |
| + "\"oltp_namespace\":null," | |
| + "\"olap_namespace\":null," | |
| + "\"storage_namespace\":null," | |
| + "\"operator_image_path\":\"test\"," | |
| + "\"internal_algorithm_image_url\":\"test\"," | |
| + "\"max_graph_number\":100," | |
| + "\"max_role_number\":100," | |
| + "\"auth\":false," | |
| + "\"configs\":{}" | |
| + "}"; | |
| // Create spaces | |
| Response r = this.client().post(PATH, space1); | |
| assertResponseStatus(201, r); | |
| r = this.client().post(PATH, space2); | |
| assertResponseStatus(201, r); | |
| // Test with prefix filter | |
| r = this.client().get(PATH + "/profile", | |
| ImmutableMap.of("prefix", "test")); | |
| String result = assertResponseStatus(200, r); | |
| @SuppressWarnings("unchecked") | |
| List<Map<String, Object>> profiles = JsonUtil.fromJson(result, List.class); | |
| assert !profiles.isEmpty() : "Expected non-empty profile list with prefix filter"; | |
| for (Map<String, Object> profile : profiles) { | |
| String name = Objects.toString(profile.get("name"), ""); | |
| String nickname = Objects.toString(profile.get("nickname"), ""); | |
| boolean matchesPrefix = name.startsWith("test") || nickname.startsWith("test"); | |
| assert matchesPrefix | |
| : "Profile does not match prefix filter 'test': " + profile; | |
| // Ensure the space without the 'test' prefix is excluded | |
| assert !("other_profile_space".equals(name) | |
| || "OtherProfileSpace".equals(nickname)) | |
| : "Non-matching profile should be excluded by prefix filter: " + profile; | |
| } |
| if ("auth_test_space".equals(profile.get("name"))) { | ||
| found = true; | ||
| // Admin user should be authed | ||
| assert profile.containsKey("authed"); |
There was a problem hiding this comment.
testListProfileWithAuth() only checks that the authed key exists for auth_test_space, but it doesn’t validate the boolean value. Since the purpose of this test is auth-related behavior, assert that authed is true for the admin user (and consider adding a negative case if non-admin access is expected to show false/be forbidden).
| assert profile.containsKey("authed"); | |
| assert profile.containsKey("authed"); | |
| assert Objects.equals(profile.get("authed"), true); |
| List<Map<String, Object>> spaceList = new ArrayList<>(); | ||
| List<Map<String, Object>> result = new ArrayList<>(); | ||
| String user = HugeGraphAuthProxy.username(); | ||
| AuthManager authManager = manager.authManager(); | ||
| // FIXME: defaultSpace related interface is not implemented | ||
| // String defaultSpace = authManager.getDefaultSpace(user); | ||
| SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd hh:mm:ss"); | ||
| for (String sp : spaces) { | ||
| manager.getSpaceStorage(sp); | ||
| GraphSpace gs = space(manager, sp); | ||
| Map<String, Object> gsProfile = gs.info(); | ||
| boolean isManager = verifyPermission(user, authManager, sp); | ||
|
|
||
| // 设置当前用户的是否允许访问该空间 | ||
| if (gs.auth() && !isManager) { | ||
| gsProfile.put("authed", false); | ||
| } else { | ||
| gsProfile.put("authed", true); | ||
| } | ||
|
|
||
| gsProfile.put("create_time", format.format(gs.createTime())); | ||
| gsProfile.put("update_time", format.format(gs.updateTime())); | ||
| if (!isPrefix(gsProfile, prefix)) { | ||
| continue; | ||
| } | ||
|
|
||
| gsProfile.put("default", false); | ||
| result.add(gsProfile); | ||
| //boolean defaulted = StringUtils.equals(sp, defaultSpace); | ||
| //gsProfile.put("default", defaulted); | ||
| //if (defaulted) { | ||
| // result.add(gsProfile); | ||
| //} else { | ||
| // spaceList.add(gsProfile); | ||
| //} | ||
| } | ||
| result.addAll(spaceList); | ||
| return result; |
There was a problem hiding this comment.
listProfile() builds spaceList but never adds anything to it (the only code path that would add is commented out), and then does result.addAll(spaceList) which is a no-op. This looks like leftover/unfinished logic; consider removing spaceList entirely (and the final addAll) or re-enabling the intended default-space ordering logic.
add profile api for hubble