imbajin commented on PR #3008:
URL: https://github.com/apache/hugegraph/pull/3008#issuecomment-4506421421
Follow-up TODOs that are not necessarily blockers for this PR, but are worth
tracking separately if they are not handled here:
1. Consider using one documented marker convention for default graph/default
role metadata. V1 uses a reserved `~default_*` prefix while V2 uses the
`_DEFAULT_SETTER_ROLE` suffix; the suffix form can collide with user-defined
role names that happen to end with that marker.
2. Consider splitting the default graph/default role storage workaround into
a follow-up issue. The marker-group approach is now documented, but it still
leaks implementation details into auth entities and may deserve a dedicated
storage model later.
3. Consider replacing `role.equals(HugeDefaultRole.OBSERVER)` checks with
`role.isGraphRole()` so future graph-scoped default roles do not need to touch
all API branches again.
4. `JsonDefaultRole` implements `Checkable`, but `checkCreate()` and
`checkUpdate()` are empty while validation is inline in the API method. Either
move validation into the DTO or drop the interface to avoid a misleading
contract.
5. Rename `@PathParam("graphspace") String name` to `graphSpace` in the
default-role methods for readability and consistency with the surrounding APIs.
6. `GraphsAPI.listProfile()` still calls `manager.graphSpace(graphSpace)`
twice and the diff has a few trailing-whitespace lines. These are low-risk
cleanups if another revision is already planned.
7. The response ordering in `GraphsAPI.listProfile()` appears to put default
profiles first, then other profiles. If the frontend depends on this, please
document or explicitly sort the response.
Some broader test gaps are already covered by existing review comments, so I
am not repeating them here. If these TODOs are out of scope for this PR, they
can be split into separate tracking issues.
--
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]
