OAK-12190 Add createUserWithAbsolutePath and createGroupWithAbsolutePath to UserManager API by nscendoni · Pull Request #2868 · apache/jackrabbit-oak

nscendoni · 2026-04-20T13:25:57Z

No description provided.

…olutePath Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

anchela

first bunch of comments..... will take another look later today

anchela · 2026-04-21T07:23:11Z

+    default User createUserWithAbsolutePath(@NotNull String userID, @Nullable String password,
+                                            @NotNull Principal principal, @NotNull String absoluteOakPath)
+            throws AuthorizableExistsException, UnsupportedRepositoryOperationException, RepositoryException {
+        throw new UnsupportedRepositoryOperationException("createUserWithAbsolutePath is not supported by this implementation");


ideally we would have a default implementation that avoids having to touch the old jackrabbit code.

anchela · 2026-04-21T07:23:48Z

+     */
+    @NotNull
+    default Group createGroupWithAbsolutePath(@NotNull String groupID, @NotNull Principal principal,
+                                              @NotNull String oakPath)


see above it should be 'absolutePath' and the description should make it clear that this is a JCR path not an oak path.
naming should be consistent with the user method.

anchela · 2026-04-21T07:24:48Z

+    default Group createGroupWithAbsolutePath(@NotNull String groupID, @NotNull Principal principal,
+                                              @NotNull String oakPath)
+            throws AuthorizableExistsException, UnsupportedRepositoryOperationException, RepositoryException {
+        throw new UnsupportedRepositoryOperationException("createGroupWithAbsolutePath is not supported by this implementation");


see above. ideally we have a default implementation

anchela · 2026-04-21T07:25:14Z

+    @NotNull
+    @Override
+    public User createUserWithAbsolutePath(@NotNull final String userID, @Nullable final String password,
+                                           @NotNull final Principal principal, @NotNull final String absoluteOakPath)


rename param here as well

anchela · 2026-04-21T07:25:29Z

+    @NotNull
+    @Override
+    public Group createGroupWithAbsolutePath(@NotNull final String groupID, @NotNull final Principal principal,
+                                             @NotNull final String oakPath)


rename param here as well

anchela · 2026-04-21T07:26:54Z

    Group createGroup(@NotNull String groupID, @NotNull Principal principal, @Nullable String intermediatePath) throws AuthorizableExistsException, RepositoryException;

+    /**
+     * Creates a new {@code Group} at the specified absolute Oak repository path.


the javadoc should describe what happens in the following cases:

the path collides with an existing node

the last segment (name) violates the contract of the configured authorizableNodeName

…ide default implementations, and convert to Oak path in UserManagerImpl

anchela

found new stuff.... UserProvider.createX would add an additional segment for the node name

also: do you want to offer this method also for service-users? they come with a separate create method.

anchela · 2026-04-21T09:09:48Z

+        checkValidPrincipal(principal, false);
+
+        String oakPath = namePathMapper.getOakPath(absolutePath);
+        if (oakPath == null) {


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

anchela · 2026-04-21T09:10:35Z

+        if (oakPath == null) {
+            throw new RepositoryException("Invalid path: " + absolutePath);
+        }
+        Tree userTree = userProvider.createUser(userID, oakPath);


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

anchela · 2026-04-21T09:11:15Z

+            throw new RepositoryException("Invalid path: " + absolutePath);
+        }
+        Tree userTree = userProvider.createUser(userID, oakPath);
+        setPrincipal(userTree, principal);


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

anchela · 2026-04-21T09:11:24Z

+        checkValidPrincipal(principal, true);
+
+        String oakPath = namePathMapper.getOakPath(absolutePath);
+        if (oakPath == null) {


anchela · 2026-04-21T09:12:25Z

+        if (oakPath == null) {
+            throw new RepositoryException("Invalid path: " + absolutePath);
+        }
+        Tree groupTree = userProvider.createGroup(groupID, oakPath);


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.

…User/GroupWithAbsolutePath

…ser/buildGroup helpers; add unit tests

… usersync

anchela

just minor nitpicking left. the rest all looks good now.
@nscendoni let me know what you think about my comments.

anchela · 2026-04-30T08:17:57Z

+            tree = tree.getParent();
+        }
+        if (!tree.exists()) {
+            throw new AccessDeniedException("Missing permission to create intermediate authorizable folders.");


the exception message may be a bit misleading. it's actually that read permission is lacking to read an existing parent path, no? ((note: i am seeing this now but i may be consistent with the existing code)). maybe still worth doing better here: "Unable to retrieve authorizable parent folder" (or something like that)

anchela · 2026-04-30T08:25:49Z

+     * If a node already exists at the resolved location, the implementation may
+     * append a numeric suffix to the node name to avoid the collision.
+     * <p>
+     * If the last segment of the resolved node name violates the contract of the


the current implementation does not do that. so, i would recommend to state "an ConstraintViolationException may be thrown".
or if we want to keep that as a mandate then the impl should verify... but there is currently no API in AuthorizableNodeName to verify a given name... this would need to be added.
either way fine with me.

anchela · 2026-04-30T08:26:47Z

+    default Group createGroupWithAbsolutePath(@NotNull String groupID, @NotNull Principal principal,
+                                              @NotNull String absolutePath)
+            throws AuthorizableExistsException, UnsupportedRepositoryOperationException, RepositoryException {
+        return createGroup(groupID, principal, absolutePath);


calling the method that takes an intermediate path will not yield the desired result, no? maybe throwing "unsupportedrepositoryoperationexception" would be more aligned with the api contract. wdyt?

anchela · 2026-04-30T08:28:19Z

+     * the group node must be created. The path must start with {@code /} and must
+     * be within the configured group root.
+     * <p>
+     * If a node already exists at the resolved location, the implementation may


i am not 100% convinced about this option to add a suffix. this reads like a contradiction to the mandate that the resulting group node has indeed the specified path. wdyt?

anchela · 2026-04-30T08:29:05Z

+    default User createUserWithAbsolutePath(@NotNull String userID, @Nullable String password,
+                                            @NotNull Principal principal, @NotNull String absolutePath)
+            throws AuthorizableExistsException, UnsupportedRepositoryOperationException, RepositoryException {
+        return createUser(userID, password, principal, absolutePath);


see comment with createGroupWithAbsolute path below. same here

Verify that absolute JCR paths supplied to the two new methods are properly converted from the session-local namespace mapping to the internal Oak path representation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Ai-Assisted-By: claude

anchela

@nscendoni the changes look good to me now. in addition i asked claude to create test cases for renamed namespaces -> additions to RemappingTest.java.

@Amoratinos do you want to take a look as well before we merge?

OAK-12190: Add remapping tests for createUser/GroupWithAbsolutePath

anchela · 2026-05-12T09:52:02Z

@reschke , we would appreciate if you could also take a look. it was reviewed by anchela, the ai-review tool from @Joscorbe . but we may still have missed something.

nscendoni and others added 2 commits April 17, 2026 08:31

Added UserManager API to create users and groups with oak path

bc375ac

Added UserManagerDelegator implementation for createUser/GroupWithAbs…

53b2678

…olutePath Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

reschke requested review from anchela and kwin April 21, 2026 04:43

anchela requested changes Apr 21, 2026

View reviewed changes

OAK-12190: Use JCR path in createUser/GroupWithAbsolutePath API, prov…

1edf3a1

…ide default implementations, and convert to Oak path in UserManagerImpl

nscendoni requested a review from anchela April 21, 2026 08:43

anchela requested changes Apr 21, 2026

View reviewed changes

nscendoni added 2 commits April 21, 2026 11:39

OAK-12190: Extract getOakPath helper and remove duplication in create…

e66ef0a

…User/GroupWithAbsolutePath

Fix on node name

029158d

nscendoni requested a review from anchela April 22, 2026 08:56

nscendoni added 2 commits April 22, 2026 11:42

OAK-12190: Create authorizable at exact absolute path; extract buildU…

0843470

…ser/buildGroup helpers; add unit tests

Merge branch 'trunk' of https://github.com/apache/jackrabbit-oak into…

d0bf4d7

… usersync

anchela requested changes Apr 30, 2026

View reviewed changes

nscendoni and others added 4 commits April 30, 2026 16:42

Fixes from Angela

a3bb99b

updated Exception message

ec797ef

Fixes on analyse-pr report

f4c2b63

anchela self-requested a review May 12, 2026 09:34

anchela approved these changes May 12, 2026

View reviewed changes

anchela requested a review from Amoratinos May 12, 2026 09:34

Merge pull request #1 from anchela/usersync

b6fcc44

OAK-12190: Add remapping tests for createUser/GroupWithAbsolutePath

anchela requested a review from reschke May 12, 2026 09:50

Conversation

nscendoni commented Apr 20, 2026

Uh oh!

anchela left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

anchela left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anchela left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anchela left a comment

Choose a reason for hiding this comment

Uh oh!

anchela commented May 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants