feat: auth project support #1504

zyxxoo · 2021-06-17T08:55:14Z

No description provided.

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/HugeProject.java

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

hugegraph-test/src/main/java/com/baidu/hugegraph/core/AuthTest.java

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/StandardAuthManager.java

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/StandardAuthManager.java

javeme

address other comments.

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

javeme · 2021-06-18T12:30:19Z

hugegraph-test/src/main/java/com/baidu/hugegraph/api/ProjectApiTest.java

+        createProject("test_project", "this is a good project");
+    }
+
+    public String createProject(String name, String desc) {


set private static and move the method to the bottom of the class

this method need use client that is a field of object, so may i replace public to private

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/StandardAuthManager.java

hugegraph-test/src/main/java/com/baidu/hugegraph/api/ProjectApiTest.java

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/HugeProject.java

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/StandardAuthManager.java

hugegraph-test/src/main/java/com/baidu/hugegraph/api/ProjectApiTest.java

hugegraph-test/src/main/java/com/baidu/hugegraph/unit/core/RolePermissionTest.java

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/HugeProject.java

hugegraph-test/src/main/java/com/baidu/hugegraph/core/AuthTest.java

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/StandardAuthManager.java

javeme · 2021-06-23T11:42:35Z

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

@@ -76,7 +76,8 @@ public String create(@Context GraphManager manager,
        HugeGraph g = graph(manager, graph);
        HugeProject project = jsonTarget.build();
        Id projectId = manager.authManager().createProject(project);
-        // Some fields of project be known must be after create
+        // Some fields of project(like admin_group ) can only be known after
+        // created


expect "/*" for comment with multi lines

javeme · 2021-06-23T11:46:28Z

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/StandardAuthManager.java

@@ -557,7 +557,7 @@ public Id updateProject(HugeProject project) {
        E.checkArgumentNotNull(project.id(),
                               "The project's id can't be null");
        E.checkArgument(project.description() != null,
-                        "the project's '%s' description can't be null",
+                        "The project's '%s' description can't be null",


keep the same style

do you mean use "The project's '%s' " this style to other places?

common style is "The description of project '%s' can't be null"

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/StandardAuthManager.java

javeme · 2021-06-23T11:48:34Z

hugegraph-test/src/main/java/com/baidu/hugegraph/api/ProjectApiTest.java

@@ -46,8 +46,9 @@ public void testCreate() {
    }

    private String createProject(String name, String desc) {
-        String reqBody = String.format("{\"name\": \"%s\",\"desc\": \"%s\"}",
-                                       name, desc);
+        String reqBody = String.format("{\"project_name\": \"%s\"," +


rename to body or request

this params is a request body, so i named it reqBody

refer to VertexApiTest/EdgeApiTest

javeme · 2021-06-23T11:52:11Z

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

+        return ACTION_DELETE_GRAPH.equals(action);
+    }
+
+    private static class JsonTarget implements Checkable {


JsonTarget => JsonProject

javeme · 2021-06-23T11:53:20Z

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

+        @Override
+        public void checkCreate(boolean isBatch) {
+            E.checkArgumentNotNull(this.name,
+                                   "The project's name can't be null");


this code already aligned here, Do you mean something else？

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/HugeProject.java

javeme · 2021-06-23T12:09:39Z

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/RelationshipManager.java

@@ -54,6 +54,7 @@
    private final HugeGraphParams graph;
    private final String label;
    private final Function<Edge, T> deser;
+    private final ThreadLocal<Boolean> shouldCommitTrans = new ThreadLocal<>();


rename to autoCommit

javeme · 2021-06-23T12:10:05Z

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/StandardAuthManager.java

+                E.checkArgumentNotNull(oldProject,
+                                       "The project with id '%s' " +
+                                       "can't be found", id);
+                // Check not graph bind this project


improve "Check not graph bind this project"

Check whether there are any graph binding this project, throw ForbiddenException, if it is

codecov · 2021-06-24T13:14:39Z

Codecov Report

Merging #1504 (d9de8c6) into master (4cb4348) will increase coverage by 0.16%.
The diff coverage is 85.02%.

@@             Coverage Diff              @@
##             master    #1504      +/-   ##
============================================
+ Coverage     59.65%   59.82%   +0.16%     
- Complexity     6153     6209      +56     
============================================
  Files           410      412       +2     
  Lines         33272    33603     +331     
  Branches       4590     4618      +28     
============================================
+ Hits          19850    20104     +254     
- Misses        11349    11400      +51     
- Partials       2073     2099      +26

Impacted Files	Coverage Δ
...in/java/com/baidu/hugegraph/auth/HugeResource.java	`79.22% <ø> (ø)`
...a/com/baidu/hugegraph/auth/HugeGraphAuthProxy.java	`58.03% <70.00%> (+0.69%)`	⬆️
.../com/baidu/hugegraph/auth/StandardAuthManager.java	`93.06% <84.61%> (-4.44%)`	⬇️
...ain/java/com/baidu/hugegraph/auth/HugeProject.java	`85.24% <85.24%> (ø)`
.../com/baidu/hugegraph/auth/RelationshipManager.java	`80.00% <85.71%> (-1.64%)`	⬇️
.../java/com/baidu/hugegraph/api/auth/ProjectAPI.java	`86.95% <86.95%> (ø)`
...n/java/com/baidu/hugegraph/version/ApiVersion.java	`75.00% <100.00%> (ø)`
...n/java/com/baidu/hugegraph/auth/EntityManager.java	`92.20% <100.00%> (+2.06%)`	⬆️
...in/java/com/baidu/hugegraph/auth/ResourceType.java	`93.61% <100.00%> (+0.13%)`	⬆️
...c/main/java/com/baidu/hugegraph/util/LockUtil.java	`84.93% <100.00%> (+0.20%)`	⬆️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4cb4348...d9de8c6. Read the comment docs.

javeme · 2021-07-02T15:17:41Z

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

+        }
+        project = jsonProject.build(action, project);
+        if (this.isAddGraph(action)) {
+            authManager.updateProjectAddGraph(projectId, jsonProject.graph);


add method jsonProject.graph()

javeme · 2021-07-02T15:18:45Z

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

+    private static class JsonProject implements Checkable {
+
+        @JsonProperty("project_description")
+        private String description;


move to line 187

javeme · 2021-07-02T15:18:58Z

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

+        @JsonProperty("project_name")
+        private String name;
+        @JsonProperty("project_graph")
+        private String graph;


Currently only one graph needs to be updated or deleted, so here is singular

keep consistency between server response and client request

javeme · 2021-07-02T15:20:28Z

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

+        } else if (this.isRemoveGraph(action)) {
+            authManager.updateProjectRemoveGraph(projectId, jsonProject.graph);
+        } else {
+            authManager.updateProject(project);


checkUpdate

The check logic is in the method "JsonProject.build", so there is no need to call the method "checkUpdate" here

expect to check params for each action

check the action param is empty

javeme · 2021-07-02T15:22:52Z

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/HugeProject.java

+    private String name;
+    private String desc;
+    private Id adminGroupId;
+    private Id opGroupId;
+    private Set<String> graphs;
+    private Id targetId;


improve order

javeme · 2021-07-02T15:27:33Z

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/StandardAuthManager.java

+                                               this.graph.name(),
+                                               "localhost:8080",
+                                               ImmutableList.of(resource));
+            target.creator(project.creator());


"creator" is a parameter. I see that when other places are created, the method "AuthManagerProxy.updateCreator" is called, so to be consistent, I also update "creator" when I create it.

javeme · 2021-07-02T15:28:33Z

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/StandardAuthManager.java

+
+                HugeProject oldProject = this.project.get(id);
+                E.checkArgumentNotNull(oldProject,
+                                       "The project '%s' can't be found", id);


maybe change to "The project '%s' not found"?

javeme · 2021-07-02T15:31:08Z

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/StandardAuthManager.java

+                                       id);
+                E.checkArgument(project.adminGroupId() != null,
+                                "Failed to delete the project '%s'," +
+                                "the admin group id of project can't be null",


the admin group of project

javeme · 2021-07-02T15:31:21Z

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/StandardAuthManager.java

+                                id);
+                E.checkArgument(project.opGroupId() != null,
+                                "Failed to delete the project '%s'," +
+                                "the op group id of project can't be null", id);


the op group of project

javeme · 2021-07-02T15:31:50Z

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/StandardAuthManager.java

+                                "the op group id of project can't be null", id);
+                E.checkArgument(project.targetId() != null,
+                                "Failed to delete the project '%s', " +
+                                "the target id of project can't be null", id);


the target resource of project

javeme · 2021-07-05T05:44:14Z

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

+        } else if (this.isRemoveGraph(action)) {
+            authManager.updateProjectRemoveGraph(projectId, jsonProject.graph);
+        } else {
+            authManager.updateProject(project);


expect to check params for each action

javeme · 2021-07-05T05:47:15Z

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

+        } else if (this.isRemoveGraph(action)) {
+            authManager.updateProjectRemoveGraph(projectId, jsonProject.graph);
+        } else {
+            authManager.updateProject(project);


check the action param is empty

javeme · 2021-07-05T06:15:22Z

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

+        @JsonProperty("project_name")
+        private String name;
+        @JsonProperty("project_graph")
+        private String graph;


keep consistency between server response and client request

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

javeme · 2021-07-05T06:18:26Z

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/HugeProject.java

+    public String opGroupName() {
+        return "op_" + this.name;
+    }
+
+    public String adminGroupName() {
+        return "admin_" + this.name;
+    }


can move into createProject() method

hugegraph-test/src/main/java/com/baidu/hugegraph/core/AuthTest.java

hugegraph-test/src/main/java/com/baidu/hugegraph/unit/core/RolePermissionTest.java

hugegraph-test/src/main/java/com/baidu/hugegraph/api/ProjectApiTest.java

hugegraph-core/src/main/java/com/baidu/hugegraph/util/ObjectUtils.java

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

… in ProjectAPI

javeme · 2021-07-07T06:08:21Z

hugegraph-api/src/main/java/com/baidu/hugegraph/api/auth/ProjectAPI.java

-                                this.action);
+            E.checkArgument(!CollectionUtils.isEmpty(this.graphs) ||
+                            this.description != null,
+                            "Must specify the fields 'graphs/description' " +


specify 'graphs' or 'description' field

zyxxoo force-pushed the zy_auth_project branch 4 times, most recently from cf65262 to 59b456f Compare June 17, 2021 11:16

corgiboygsj requested changes Jun 17, 2021

View reviewed changes

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/HugeProject.java Outdated Show resolved Hide resolved

hugegraph-core/src/main/java/com/baidu/hugegraph/auth/HugeProject.java Outdated Show resolved Hide resolved

zyxxoo force-pushed the zy_auth_project branch from 31d06a5 to d09f422 Compare June 17, 2021 13:19

javeme reviewed Jun 17, 2021

View reviewed changes

javeme reviewed Jun 18, 2021

View reviewed changes

zyxxoo force-pushed the zy_auth_project branch 2 times, most recently from 1a4ce69 to 4728cd4 Compare June 18, 2021 12:03

javeme requested changes Jun 18, 2021

View reviewed changes

zyxxoo changed the title ~~feat: add hugeProject java api suppport~~ feat: auth project support Jun 21, 2021

zyxxoo force-pushed the zy_auth_project branch 2 times, most recently from 8442b8b to 07e9516 Compare June 21, 2021 11:02

javeme reviewed Jun 21, 2021

View reviewed changes

zyxxoo force-pushed the zy_auth_project branch 2 times, most recently from 35458ce to a61006f Compare June 22, 2021 06:58

javeme reviewed Jun 22, 2021

View reviewed changes

zyxxoo force-pushed the zy_auth_project branch 5 times, most recently from 28b7d3c to dafa78e Compare June 22, 2021 11:47

javeme reviewed Jun 23, 2021

View reviewed changes

zyxxoo force-pushed the zy_auth_project branch 3 times, most recently from d7aa42b to 534692c Compare June 24, 2021 13:06

corgiboygsj previously approved these changes Jul 2, 2021

View reviewed changes

chore: upgrade api version

48f0b47

zyxxoo dismissed corgiboygsj’s stale review via 48f0b47 July 2, 2021 10:21

javeme reviewed Jul 2, 2021

View reviewed changes

zyxxoo force-pushed the zy_auth_project branch from e7274d4 to 8a1d5f9 Compare July 2, 2021 17:01

chore: code style

6605e02

zyxxoo force-pushed the zy_auth_project branch from 8a1d5f9 to 6605e02 Compare July 5, 2021 03:32

javeme reviewed Jul 5, 2021

View reviewed changes

chore: improve code style

1f000fc

zyxxoo force-pushed the zy_auth_project branch 6 times, most recently from 4e51d5f to 27e05b9 Compare July 5, 2021 12:43

chore: add or remove graph collections

1340c4c

zyxxoo force-pushed the zy_auth_project branch from 27e05b9 to 1340c4c Compare July 5, 2021 14:48

javeme reviewed Jul 6, 2021

View reviewed changes

zyxxoo force-pushed the zy_auth_project branch 3 times, most recently from 74835fa to bea69b4 Compare July 7, 2021 05:40

refactor: remove ObjectUtils, remove action filed of JsonProject that…

8bc4023

… in ProjectAPI

zyxxoo force-pushed the zy_auth_project branch from bea69b4 to 8bc4023 Compare July 7, 2021 05:46

javeme reviewed Jul 7, 2021

View reviewed changes

chore: improve error hint

d9de8c6

zyxxoo force-pushed the zy_auth_project branch from 7d09a87 to d9de8c6 Compare July 7, 2021 06:15

javeme approved these changes Jul 7, 2021

View reviewed changes

corgiboygsj approved these changes Jul 7, 2021

View reviewed changes

javeme merged commit 6bff777 into master Jul 7, 2021

javeme deleted the zy_auth_project branch July 7, 2021 06:35

feat: auth project support #1504

feat: auth project support #1504

Conversation

zyxxoo commented Jun 17, 2021

javeme left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zyxxoo Jun 24, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 24, 2021 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zyxxoo Jun 24, 2021 •

edited

codecov bot commented Jun 24, 2021 •

edited