Skip to content

Commit b67df7b

Browse files
committed
fix(api): fix compilation error and address imbajin review issues
- GraphsAPI: remove duplicate private isPrefix() that shadowed the protected version in API base class, causing compilation failure - GraphsAPI: fix NPE in manage() when 'update' value is null by guarding value.getClass() in both action and update validation - GraphsAPI/GraphSpaceAPI: replace org.apache.commons.lang.StringUtils with commons-lang3 and remove org.apache.logging.log4j.util.Strings import (replaced with StringUtils equivalents) to fix potential classpath issues - GraphManager.updateGraphNickname: restore old nickname (not null) on PD persistence failure to avoid a third inconsistent state - StandardAuthManagerV2.setDefaultGraph: check existBelong() before createBelong() to make the operation idempotent; repeated POST calls for the same user/graph no longer throw HugeException - StandardAuthManagerV2.createDefaultRole: same idempotent guard via existBelong() check before createBelong() - ManagerAPI.checkDefaultRole: use @PathParam graphspace instead of @QueryParam to match the mounted path contract
1 parent 6ddfb17 commit b67df7b

5 files changed

Lines changed: 34 additions & 28 deletions

File tree

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/ManagerAPI.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ public String getRolesInGs(@Context GraphManager manager,
267267
@Path("default")
268268
@Consumes(APPLICATION_JSON)
269269
public String checkDefaultRole(@Context GraphManager manager,
270-
@QueryParam("graphspace") String graphSpace,
270+
@PathParam("graphspace") String graphSpace,
271271
@QueryParam("role") String role,
272272
@QueryParam("graph") String graph) {
273273
LOG.debug("check if current user is default role: {} {} {}",

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
import org.apache.hugegraph.util.E;
4747
import org.apache.hugegraph.util.JsonUtil;
4848
import org.apache.hugegraph.util.Log;
49-
import org.apache.logging.log4j.util.Strings;
5049
import org.slf4j.Logger;
5150

5251
import com.codahale.metrics.annotation.Timed;
@@ -213,17 +212,6 @@ public Object listProfile(@Context GraphManager manager,
213212
return defaultProfiles;
214213
}
215214

216-
private static boolean isPrefix(Map<String, Object> profile, String prefix) {
217-
if (StringUtils.isEmpty(prefix)) {
218-
return true;
219-
}
220-
// graph name or nickname is not empty
221-
String name = profile.get("name").toString();
222-
Object nicknameObj = profile.get("nickname");
223-
String nickname = nicknameObj != null ? nicknameObj.toString() : "";
224-
return name.startsWith(prefix) || nickname.startsWith(prefix);
225-
}
226-
227215
@GET
228216
@Timed
229217
@Path("{name}")
@@ -358,7 +346,8 @@ public Map<String, String> manage(@Context GraphManager manager,
358346
value = actionMap.get(UPDATE);
359347
E.checkArgument(value instanceof Map,
360348
"The '%s' must be map, but got %s",
361-
UPDATE, value.getClass());
349+
UPDATE,
350+
value == null ? "null" : value.getClass().getSimpleName());
362351
@SuppressWarnings("unchecked")
363352
Map<String, Object> graphMap = (Map<String, Object>) value;
364353
String graphName = (String) graphMap.get("name");
@@ -367,7 +356,7 @@ public Map<String, String> manage(@Context GraphManager manager,
367356
graphName, name);
368357
HugeGraph exist = graph(manager, graphSpace, name);
369358
String nickname = (String) graphMap.get("nickname");
370-
if (!Strings.isEmpty(nickname)) {
359+
if (!StringUtils.isEmpty(nickname)) {
371360
GraphManager.checkNickname(nickname);
372361
E.checkArgument(!manager.isExistedGraphNickname(graphSpace, nickname) ||
373362
nickname.equals(exist.nickname()),
@@ -471,7 +460,7 @@ public Object create(@Context GraphManager manager,
471460
String description = (configs != null) ?
472461
(String) configs.get(GRAPH_DESCRIPTION) : null;
473462
if (description == null) {
474-
description = Strings.EMPTY;
463+
description = StringUtils.EMPTY;
475464
}
476465
Object result = ImmutableMap.of("name", graph.name(),
477466
"nickname", graph.nickname(),

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
import java.util.Set;
2727

2828
import org.apache.commons.codec.digest.DigestUtils;
29-
import org.apache.commons.lang.StringUtils;
29+
import org.apache.commons.lang3.StringUtils;
3030

3131
import org.apache.hugegraph.HugeException;
3232
import org.apache.hugegraph.api.API;
@@ -41,7 +41,6 @@
4141
import org.apache.hugegraph.util.E;
4242
import org.apache.hugegraph.util.JsonUtil;
4343
import org.apache.hugegraph.util.Log;
44-
import org.apache.logging.log4j.util.Strings;
4544
import org.slf4j.Logger;
4645

4746
import com.codahale.metrics.annotation.Timed;
@@ -363,13 +362,13 @@ public Map<String, Object> manage(@Context GraphManager manager,
363362
}
364363

365364
String nickname = (String) graphSpaceMap.get("nickname");
366-
if (!Strings.isEmpty(nickname)) {
365+
if (!StringUtils.isEmpty(nickname)) {
367366
GraphManager.checkNickname(nickname);
368367
exist.nickname(nickname);
369368
}
370369

371370
String description = (String) graphSpaceMap.get("description");
372-
if (!Strings.isEmpty(description)) {
371+
if (!StringUtils.isEmpty(description)) {
373372
exist.description(description);
374373
}
375374

@@ -410,31 +409,31 @@ public Map<String, Object> manage(@Context GraphManager manager,
410409
String oltpNamespace =
411410
(String) graphSpaceMap.get("oltp_namespace");
412411
if (oltpNamespace != null &&
413-
!Strings.isEmpty(oltpNamespace)) {
412+
!StringUtils.isEmpty(oltpNamespace)) {
414413
exist.oltpNamespace(oltpNamespace);
415414
}
416415
String olapNamespace =
417416
(String) graphSpaceMap.get("olap_namespace");
418417
if (olapNamespace != null &&
419-
!Strings.isEmpty(olapNamespace)) {
418+
!StringUtils.isEmpty(olapNamespace)) {
420419
exist.olapNamespace(olapNamespace);
421420
}
422421
String storageNamespace =
423422
(String) graphSpaceMap.get("storage_namespace");
424423
if (storageNamespace != null &&
425-
!Strings.isEmpty(storageNamespace)) {
424+
!StringUtils.isEmpty(storageNamespace)) {
426425
exist.storageNamespace(storageNamespace);
427426
}
428427

429428
String operatorImagePath = (String) graphSpaceMap
430429
.getOrDefault("operator_image_path", "");
431-
if (!Strings.isEmpty(operatorImagePath)) {
430+
if (!StringUtils.isEmpty(operatorImagePath)) {
432431
exist.operatorImagePath(operatorImagePath);
433432
}
434433

435434
String internalAlgorithmImageUrl = (String) graphSpaceMap
436435
.getOrDefault("internal_algorithm_image_url", "");
437-
if (!Strings.isEmpty(internalAlgorithmImageUrl)) {
436+
if (!StringUtils.isEmpty(internalAlgorithmImageUrl)) {
438437
exist.internalAlgorithmImageUrl(internalAlgorithmImageUrl);
439438
}
440439

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2210,6 +2210,8 @@ public void updateGraphNickname(String graphSpace, String graphName,
22102210
String nickname) {
22112211
// Always update the in-memory graph instance first
22122212
HugeGraph g = this.graph(graphSpace, graphName);
2213+
// Capture the old nickname so we can restore it on failure
2214+
String oldNickname = g != null ? g.nickname() : null;
22132215
if (g != null) {
22142216
g.nickname(nickname);
22152217
}
@@ -2226,10 +2228,10 @@ public void updateGraphNickname(String graphSpace, String graphName,
22262228
this.metaManager.notifyGraphUpdate(graphSpace, graphName);
22272229
}
22282230
} catch (Exception e) {
2229-
// Roll back the in-memory change so that the runtime state stays
2230-
// consistent with what was actually persisted.
2231+
// Roll back the in-memory change to the previous value so that
2232+
// runtime state stays consistent with what was actually persisted.
22312233
if (g != null) {
2232-
g.nickname(null);
2234+
g.nickname(oldNickname);
22332235
}
22342236
throw new HugeException("Failed to persist nickname for graph " +
22352237
"'%s/%s'", e, graphSpace, graphName);

hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManagerV2.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,6 +1519,13 @@ public HugeGroup findGroup(String name) {
15191519
@Override
15201520
public void setDefaultGraph(String graphSpace, String graph, String user) {
15211521
try {
1522+
String role = graph + DEFAULT_SETTER_ROLE_KEY;
1523+
String belongId = this.metaManager.belongId(user, role);
1524+
Id id = IdGenerator.of(belongId);
1525+
// Idempotent: if binding already exists, treat as success
1526+
if (this.metaManager.existBelong(graphSpace, id)) {
1527+
return;
1528+
}
15221529
HugeBelong belong = new HugeBelong(graphSpace,
15231530
IdGenerator.of(user),
15241531
IdGenerator.of(graph +
@@ -1577,15 +1584,24 @@ public Id createDefaultRole(String graphSpace, String owner,
15771584
getGraphDefaultRole(graph, role.toString()) : role.toString();
15781585
try {
15791586
HugeBelong belong;
1587+
String link;
15801588
if (HugeGroup.isGroup(owner)) {
15811589
belong = new HugeBelong(graphSpace, null,
15821590
IdGenerator.of(owner),
15831591
IdGenerator.of(roleName),
15841592
HugeBelong.GR);
1593+
link = HugeBelong.GR;
15851594
} else {
15861595
belong = new HugeBelong(graphSpace, IdGenerator.of(owner),
15871596
null, IdGenerator.of(roleName),
15881597
HugeBelong.UR);
1598+
link = HugeBelong.UR;
1599+
}
1600+
1601+
// Idempotent: if binding already exists, treat as success
1602+
String belongId = this.metaManager.belongId(owner, roleName, link);
1603+
if (this.metaManager.existBelong(graphSpace, IdGenerator.of(belongId))) {
1604+
return IdGenerator.of(belongId);
15891605
}
15901606

15911607
this.tryInitDefaultRole(graphSpace, roleName, graph);

0 commit comments

Comments
 (0)