anchela commented on code in PR #1152:
URL: https://github.com/apache/jackrabbit-oak/pull/1152#discussion_r1362384562
##########
oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipIgnoreTest.java:
##########
@@ -65,7 +65,7 @@ public void before() throws Exception {
assertTrue(importer.start(memberRefList));
}
- @Test(expected = IllegalArgumentException.class)
Review Comment:
if the IdentifierManager no longer verifies the validity of an given uuid
(as they are used for storing group membership, we might consider doing so in
the UserImporter.
reason: there exist 3 different import modes
- ABORT: must fail with ConstraintViolationException if the member uuid
cannot be resolved to a valid user
- IGNORE: ignore the uuids that cannot be resolved
- BESTEFFORT: assume the member does just not yet exist and add the value.
that latter is the one that bothers me here: if the validity of the uuid
format is no longer verified, we will end up with garbage in the rep:members
properties that might later on fail the resolution (i haven't checked). the
goal of BESTEFFORT is to not mandate that members exist already.... but
obviously storing completely illegal values should be avoided.
i am not sure yet what the best option is.
throwing RepositoryException in the IdentifierManager instead of
illegalargumentexception would have the same problem btw as the UserImporter is
catching them (and then proceeding as described above).
--
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]
