fix(client): Adapt client graphSpace method to 1.7.0 server#712
fix(client): Adapt client graphSpace method to 1.7.0 server#712sadwitdastreetz wants to merge 2 commits intoapache:masterfrom
Conversation
…t/get logic Remove reliance on the removed '/profile' endpoint && Implement manual aggregation by listing all GraphSpaces and fetching details individually.
There was a problem hiding this comment.
Pull request overview
This pull request adapts the GraphSpaceAPI.listProfile() method to work with HugeGraph server 1.7.0, which removed the /profile endpoint. The implementation changes from a single server-side filtered API call to a "list-then-get" pattern with client-side filtering.
Changes:
- Replaced single
/profileendpoint call with list() followed by get() for each graph space - Implemented client-side prefix filtering using
startsWith() - Removed unused
LinkedHashMapimport
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (Object entry : results) { | ||
| profiles.add(JsonUtil.fromJson(JsonUtil.toJson(entry), Map.class)); | ||
| for (String name : names) { | ||
| if (name.startsWith(prefix)) { |
There was a problem hiding this comment.
The prefix parameter can be null (as seen in GraphSpaceManager.listProfile() which calls this method with null), but this code calls name.startsWith(prefix) without checking if prefix is null first. This will cause a NullPointerException. Consider using StringUtils.isEmpty(prefix) to handle null/empty prefix cases - when prefix is null or empty, all names should be included in the result.
| if (name.startsWith(prefix)) { | |
| if (StringUtils.isEmpty(prefix) || name.startsWith(prefix)) { |
| public List<Map<String, Object>> listProfile(String prefix) { | ||
| String profilePath = joinPath(this.path(), "profile"); | ||
| Map<String, Object> params = new LinkedHashMap<>(); | ||
| params.put("prefix", prefix); | ||
| RestResult result = this.client.get(profilePath, params); | ||
| List<Map> results = result.readList(Map.class); | ||
| List<String> names = this.list(); | ||
| List<Map<String, Object>> profiles = new ArrayList<>(); | ||
| for (Object entry : results) { | ||
| profiles.add(JsonUtil.fromJson(JsonUtil.toJson(entry), Map.class)); | ||
| for (String name : names) { | ||
| if (name.startsWith(prefix)) { | ||
| GraphSpace space = this.get(name); | ||
| Map<String, Object> profileMap = JsonUtil.fromJson(JsonUtil.toJson(space), Map.class); | ||
| profiles.add(profileMap); | ||
| } | ||
| } | ||
|
|
||
| return profiles; |
There was a problem hiding this comment.
The new implementation performs N+1 API calls (one list() call plus one get() call for each matching graph space), which could impact performance when there are many graph spaces. While this is necessary due to the removal of the /profile endpoint in server 1.7.0, consider documenting this performance characteristic in a code comment to inform future developers, especially if this method is called frequently or with many graph spaces.
Purpose of the PR
/profileendpoint (used to fetch detailed configurations of all GraphSpaces in one call) has been removed in the latest 1.7.0 server, causing thelistProfilemethod to fail.Main Changes
/profilewith a "List-then-Get" pattern (fetching the name list first, then retrieving details for each entry).startsWith(prefix)to maintain consistency with the legacy API behavior.Documentation Status
Doc - TODODoc - DoneDoc - No Need