imbajin commented on code in PR #3159:
URL: https://github.com/apache/hugegraph/pull/3159#discussion_r3885699454
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java:
##########
@@ -519,6 +519,13 @@ public void clearBackend() {
LockUtil.lock(this.spaceGraphName(), LockUtil.GRAPH_LOCK);
try {
+ if (this.isHstore()) {
+ E.checkState(this.schemaTransaction() instanceof
+ CachedSchemaTransactionV2,
+ "The HStore schema transaction must be %s",
+ CachedSchemaTransactionV2.class.getSimpleName());
+ ((CachedSchemaTransactionV2) this.schemaTransaction()).clear();
+ }
Review Comment:
No code change. StandardHugeGraph.schemaTransaction() delegates to the
current thread-local Txs object: Transaction.schemaTransaction() returns
getOrNewTransaction().schemaTx, so both calls are the same instance throughout
this locked synchronous block. The instanceof guard already produces the
clearer HStore invariant failure; introducing a local here is a style-only
cleanup and is intentionally omitted from this minimal Server closeout.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/ManagerAPI.java:
##########
@@ -287,20 +287,18 @@ public String checkDefaultRole(@Context GraphManager
manager,
defaultRole = null; // unreachable, satisfies compiler
}
validGraphSpace(manager, graphSpace);
- boolean hasGraph = defaultRole.equals(HugeDefaultRole.OBSERVER);
- E.checkArgument(!hasGraph || StringUtils.isNotEmpty(graph),
- "Must set a graph for observer");
+ boolean hasGraph = defaultRole.equals(HugeDefaultRole.OBSERVER) &&
StringUtils.isNotEmpty(graph);
if (hasGraph) {
validGraph(manager, graphSpace, graph);
}
boolean result;
if (hasGraph) {
result = authManager.isDefaultRole(graphSpace, graph, user,
- defaultRole);
+ defaultRole) ||
+ authManager.isDefaultRole(graphSpace, user, defaultRole);
Review Comment:
No code change. ManagerAPI supports both StandardAuthManager and
StandardAuthManagerV2. Only the V2 graph overload folds in ALL_GRAPHS; the
standard manager checks the concrete graph marker alone. Keeping the explicit
GraphSpace-wide check preserves backend-neutral API 0.72 behavior and the
focused Manager/GraphSpace role tests pass.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java:
##########
@@ -203,20 +200,19 @@ public String checkDefaultRole(@Context GraphManager
manager,
defaultRole.equals(HugeDefaultRole.SPACE)) {
throw new ForbiddenException("Forbidden to check role " + role);
}
- boolean hasGraph = defaultRole.equals(HugeDefaultRole.OBSERVER);
- E.checkArgument(!hasGraph || StringUtils.isNotEmpty(graph),
- "Must set a graph for observer");
+ boolean hasGraph = defaultRole.equals(HugeDefaultRole.OBSERVER) &&
+ StringUtils.isNotEmpty(graph);
if (hasGraph) {
validGraph(manager, name, graph);
}
boolean result;
if (hasGraph) {
result = authManager.isDefaultRole(name, graph, user,
- defaultRole);
+ defaultRole) ||
+ authManager.isDefaultRole(name, user, defaultRole);
Review Comment:
No code change. The fallback is redundant only for StandardAuthManagerV2.
GraphSpaceAPI is backend-neutral and
StandardAuthManager.isDefaultRole(graphSpace, graph, ...) checks only the
concrete graph marker; it does not consult ALL_GRAPHS. The explicit second
check is therefore required to preserve API 0.72 ALL_GRAPHS behavior outside
the V2/HStore implementation. Focused GraphSpace API coverage passes on the
final head.
--
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]
