anchela commented on code in PR #2868:
URL: https://github.com/apache/jackrabbit-oak/pull/2868#discussion_r3116313925


##########
oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java:
##########
@@ -251,6 +251,53 @@ public Group createGroup(@NotNull String groupID, @NotNull 
Principal principal,
         return group;
     }
 
+    @NotNull
+    @Override
+    public User createUserWithAbsolutePath(@NotNull String userID, @Nullable 
String password,
+                                           @NotNull Principal principal, 
@NotNull String absolutePath)
+            throws AuthorizableExistsException, 
UnsupportedRepositoryOperationException, RepositoryException {
+        checkValidId(userID);
+        checkValidPrincipal(principal, false);
+
+        String oakPath = namePathMapper.getOakPath(absolutePath);
+        if (oakPath == null) {
+            throw new RepositoryException("Invalid path: " + absolutePath);
+        }
+        Tree userTree = userProvider.createUser(userID, oakPath);

Review Comment:
   the userProvider.createUser call will add an additional element for the user 
node.
   this will not result in the absolute path being uses as it.... 



##########
oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java:
##########
@@ -251,6 +251,53 @@ public Group createGroup(@NotNull String groupID, @NotNull 
Principal principal,
         return group;
     }
 
+    @NotNull
+    @Override
+    public User createUserWithAbsolutePath(@NotNull String userID, @Nullable 
String password,
+                                           @NotNull Principal principal, 
@NotNull String absolutePath)
+            throws AuthorizableExistsException, 
UnsupportedRepositoryOperationException, RepositoryException {
+        checkValidId(userID);
+        checkValidPrincipal(principal, false);
+
+        String oakPath = namePathMapper.getOakPath(absolutePath);
+        if (oakPath == null) {
+            throw new RepositoryException("Invalid path: " + absolutePath);
+        }
+        Tree userTree = userProvider.createUser(userID, oakPath);
+        setPrincipal(userTree, principal);
+        if (password != null) {
+            setPassword(userTree, userID, password, false);
+        }
+
+        User user = new UserImpl(userID, userTree, this);
+        onCreate(user, password);
+
+        log.debug("User created: {}", userID);
+        return user;
+    }
+
+    @NotNull
+    @Override
+    public Group createGroupWithAbsolutePath(@NotNull String groupID, @NotNull 
Principal principal,
+                                             @NotNull String absolutePath)
+            throws AuthorizableExistsException, 
UnsupportedRepositoryOperationException, RepositoryException {
+        checkValidId(groupID);
+        checkValidPrincipal(principal, true);
+
+        String oakPath = namePathMapper.getOakPath(absolutePath);
+        if (oakPath == null) {

Review Comment:
   see above 



##########
oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java:
##########
@@ -251,6 +251,53 @@ public Group createGroup(@NotNull String groupID, @NotNull 
Principal principal,
         return group;
     }
 
+    @NotNull
+    @Override
+    public User createUserWithAbsolutePath(@NotNull String userID, @Nullable 
String password,
+                                           @NotNull Principal principal, 
@NotNull String absolutePath)
+            throws AuthorizableExistsException, 
UnsupportedRepositoryOperationException, RepositoryException {
+        checkValidId(userID);
+        checkValidPrincipal(principal, false);
+
+        String oakPath = namePathMapper.getOakPath(absolutePath);
+        if (oakPath == null) {
+            throw new RepositoryException("Invalid path: " + absolutePath);
+        }
+        Tree userTree = userProvider.createUser(userID, oakPath);
+        setPrincipal(userTree, principal);
+        if (password != null) {
+            setPassword(userTree, userID, password, false);
+        }
+
+        User user = new UserImpl(userID, userTree, this);
+        onCreate(user, password);
+
+        log.debug("User created: {}", userID);
+        return user;
+    }
+
+    @NotNull
+    @Override
+    public Group createGroupWithAbsolutePath(@NotNull String groupID, @NotNull 
Principal principal,
+                                             @NotNull String absolutePath)
+            throws AuthorizableExistsException, 
UnsupportedRepositoryOperationException, RepositoryException {
+        checkValidId(groupID);
+        checkValidPrincipal(principal, true);
+
+        String oakPath = namePathMapper.getOakPath(absolutePath);
+        if (oakPath == null) {
+            throw new RepositoryException("Invalid path: " + absolutePath);
+        }
+        Tree groupTree = userProvider.createGroup(groupID, oakPath);

Review Comment:
   same as above... this will not use the absolute path as it is but will add a 
node-name segment generated with the available authorizablenodename 
implentation.



##########
oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java:
##########
@@ -251,6 +251,53 @@ public Group createGroup(@NotNull String groupID, @NotNull 
Principal principal,
         return group;
     }
 
+    @NotNull
+    @Override
+    public User createUserWithAbsolutePath(@NotNull String userID, @Nullable 
String password,
+                                           @NotNull Principal principal, 
@NotNull String absolutePath)
+            throws AuthorizableExistsException, 
UnsupportedRepositoryOperationException, RepositoryException {
+        checkValidId(userID);
+        checkValidPrincipal(principal, false);
+
+        String oakPath = namePathMapper.getOakPath(absolutePath);
+        if (oakPath == null) {

Review Comment:
   i would move those duplicate lines from here and the group method into a 
private method getOakPath



##########
oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java:
##########
@@ -251,6 +251,53 @@ public Group createGroup(@NotNull String groupID, @NotNull 
Principal principal,
         return group;
     }
 
+    @NotNull
+    @Override
+    public User createUserWithAbsolutePath(@NotNull String userID, @Nullable 
String password,
+                                           @NotNull Principal principal, 
@NotNull String absolutePath)
+            throws AuthorizableExistsException, 
UnsupportedRepositoryOperationException, RepositoryException {
+        checkValidId(userID);
+        checkValidPrincipal(principal, false);
+
+        String oakPath = namePathMapper.getOakPath(absolutePath);
+        if (oakPath == null) {
+            throw new RepositoryException("Invalid path: " + absolutePath);
+        }
+        Tree userTree = userProvider.createUser(userID, oakPath);
+        setPrincipal(userTree, principal);

Review Comment:
   lines 267-276 will be duplicated from the regular create user call, no?
   please refactor to avoid duplciation



-- 
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]

Reply via email to