diff --git a/core/src/main/java/dev/vml/es/acm/core/acl/Acl.java b/core/src/main/java/dev/vml/es/acm/core/acl/Acl.java index ca4019dc5..bc7668932 100644 --- a/core/src/main/java/dev/vml/es/acm/core/acl/Acl.java +++ b/core/src/main/java/dev/vml/es/acm/core/acl/Acl.java @@ -3,6 +3,7 @@ import dev.vml.es.acm.core.acl.authorizable.AclAuthorizable; import dev.vml.es.acm.core.acl.authorizable.AclGroup; import dev.vml.es.acm.core.acl.authorizable.AclUser; +import dev.vml.es.acm.core.repo.CommitPolicy; import dev.vml.es.acm.core.util.GroovyUtils; import groovy.lang.Closure; import org.apache.jackrabbit.api.security.user.Authorizable; @@ -17,7 +18,11 @@ public class Acl { private final AclChecker checker; public Acl(ResourceResolver resourceResolver) { - this.context = new AclContext(resourceResolver); + this(resourceResolver, CommitPolicy.of(resourceResolver, true)); + } + + public Acl(ResourceResolver resourceResolver, CommitPolicy commitPolicy) { + this.context = new AclContext(resourceResolver, commitPolicy); this.checker = new AclChecker(context); } diff --git a/core/src/main/java/dev/vml/es/acm/core/acl/AclContext.java b/core/src/main/java/dev/vml/es/acm/core/acl/AclContext.java index 3e60f0c3d..7f5259933 100644 --- a/core/src/main/java/dev/vml/es/acm/core/acl/AclContext.java +++ b/core/src/main/java/dev/vml/es/acm/core/acl/AclContext.java @@ -5,6 +5,7 @@ import dev.vml.es.acm.core.acl.authorizable.AclUser; import dev.vml.es.acm.core.acl.utils.AuthorizableManager; import dev.vml.es.acm.core.acl.utils.PermissionsManager; +import dev.vml.es.acm.core.repo.CommitPolicy; import dev.vml.es.acm.core.repo.Repo; import java.util.Optional; import javax.jcr.RepositoryException; @@ -31,6 +32,10 @@ public class AclContext { private final PermissionsManager permissionsManager; public AclContext(ResourceResolver resourceResolver) { + this(resourceResolver, CommitPolicy.of(resourceResolver, true)); + } + + public AclContext(ResourceResolver resourceResolver, CommitPolicy commitPolicy) { try { this.logger = LoggerFactory.getLogger(AclContext.class); JackrabbitSession session = (JackrabbitSession) resourceResolver.adaptTo(Session.class); @@ -38,8 +43,8 @@ public AclContext(ResourceResolver resourceResolver) { AccessControlManager accessControlManager = session.getAccessControlManager(); ValueFactory valueFactory = session.getValueFactory(); this.resourceResolver = resourceResolver; - this.authorizableManager = new AuthorizableManager(session, userManager, valueFactory); - this.permissionsManager = new PermissionsManager(session, accessControlManager, valueFactory); + this.authorizableManager = new AuthorizableManager(session, userManager, valueFactory, commitPolicy); + this.permissionsManager = new PermissionsManager(session, accessControlManager, valueFactory, commitPolicy); } catch (RepositoryException e) { throw new AclException("Cannot access repository while obtaining ACL context!", e); } diff --git a/core/src/main/java/dev/vml/es/acm/core/acl/utils/AuthorizableManager.java b/core/src/main/java/dev/vml/es/acm/core/acl/utils/AuthorizableManager.java index ce64364da..77dc804bb 100644 --- a/core/src/main/java/dev/vml/es/acm/core/acl/utils/AuthorizableManager.java +++ b/core/src/main/java/dev/vml/es/acm/core/acl/utils/AuthorizableManager.java @@ -1,6 +1,7 @@ package dev.vml.es.acm.core.acl.utils; import dev.vml.es.acm.core.acl.AclException; +import dev.vml.es.acm.core.repo.CommitPolicy; import java.security.Principal; import java.util.ArrayList; import java.util.List; @@ -27,10 +28,17 @@ public class AuthorizableManager { private final ValueFactory valueFactory; - public AuthorizableManager(JackrabbitSession session, UserManager userManager, ValueFactory valueFactory) { + private final CommitPolicy commitPolicy; + + public AuthorizableManager( + JackrabbitSession session, + UserManager userManager, + ValueFactory valueFactory, + CommitPolicy commitPolicy) { this.session = session; this.userManager = userManager; this.valueFactory = valueFactory; + this.commitPolicy = commitPolicy; } public User createUser(String id, String password, String path) { @@ -40,7 +48,7 @@ public User createUser(String id, String password, String path) { password = PasswordUtils.generateRandomPassword(); } User user = userManager.createUser(id, password, principal, path); - save(); + save(String.format("creating user '%s'", id)); return user; } catch (RepositoryException e) { throw new AclException(String.format("Failed to create user '%s'", id), e); @@ -54,7 +62,7 @@ public Group createGroup(String id, String path, String externalId) { if (StringUtils.isNotEmpty(externalId)) { group.setProperty("rep:externalId", valueFactory.createValue(externalId)); } - save(); + save(String.format("creating group '%s'", id)); return group; } catch (RepositoryException e) { throw new AclException(String.format("Failed to create group '%s'", id), e); @@ -64,7 +72,7 @@ public Group createGroup(String id, String path, String externalId) { public User createSystemUser(String id, String path) { try { User user = userManager.createSystemUser(id, path); - save(); + save(String.format("creating system user '%s'", id)); return user; } catch (RepositoryException e) { throw new AclException(String.format("Failed to create system user '%s'", id), e); @@ -87,7 +95,7 @@ public void deleteAuthorizable(Authorizable authorizable) { try { id = authorizable.getID(); authorizable.remove(); - save(); + save(String.format("deleting authorizable '%s'", id)); } catch (RepositoryException e) { throw new AclException(String.format("Failed to delete authorizable '%s'", id), e); } @@ -104,7 +112,7 @@ public boolean addMember(Group group, Authorizable member) { result = group.addMember(member); } if (result) { - save(); + save(String.format("adding member '%s' to group '%s'", memberId, groupId)); } return result; } catch (RepositoryException e) { @@ -123,11 +131,11 @@ public boolean removeMember(Group group, Authorizable member) { result = group.removeMember(member); } if (result) { - save(); + save(String.format("removing member '%s' from group '%s'", memberId, groupId)); } return result; } catch (RepositoryException e) { - throw new AclException(String.format("Failed to remove member '%s' to group '%s'", memberId, groupId), e); + throw new AclException(String.format("Failed to remove member '%s' from group '%s'", memberId, groupId), e); } } @@ -177,7 +185,7 @@ public void changePassword(User user, String password) { try { userId = user.getID(); user.changePassword(password); - save(); + save(String.format("changing password for user '%s'", userId)); } catch (RepositoryException e) { throw new AclException(String.format("Failed to set password for user '%s'", userId), e); } @@ -206,7 +214,7 @@ public void setProperty(Authorizable authorizable, String relPath, String value) try { id = authorizable.getID(); authorizable.setProperty(relPath, valueFactory.createValue(value)); - save(); + save(String.format("setting property '%s' for authorizable '%s'", relPath, id)); } catch (RepositoryException e) { throw new AclException(String.format("Failed to set property '%s' for authorizable '%s'", relPath, id), e); } @@ -218,7 +226,7 @@ public boolean removeProperty(Authorizable authorizable, String relPath) { id = authorizable.getID(); if (authorizable.hasProperty(relPath)) { authorizable.removeProperty(relPath); - save(); + save(String.format("removing property '%s' for authorizable '%s'", relPath, id)); return true; } return false; @@ -228,7 +236,7 @@ public boolean removeProperty(Authorizable authorizable, String relPath) { } } - private void save() throws RepositoryException { - session.save(); + private void save(String context) { + commitPolicy.commit(context); } } diff --git a/core/src/main/java/dev/vml/es/acm/core/acl/utils/PermissionsManager.java b/core/src/main/java/dev/vml/es/acm/core/acl/utils/PermissionsManager.java index d846a128a..3053f279b 100644 --- a/core/src/main/java/dev/vml/es/acm/core/acl/utils/PermissionsManager.java +++ b/core/src/main/java/dev/vml/es/acm/core/acl/utils/PermissionsManager.java @@ -1,6 +1,7 @@ package dev.vml.es.acm.core.acl.utils; import dev.vml.es.acm.core.acl.AclException; +import dev.vml.es.acm.core.repo.CommitPolicy; import java.security.Principal; import java.util.Arrays; import java.util.Collection; @@ -46,11 +47,21 @@ public class PermissionsManager { private final ValueFactory valueFactory; + private final CommitPolicy commitPolicy; + public PermissionsManager( - JackrabbitSession session, AccessControlManager accessControlManager, ValueFactory valueFactory) { + JackrabbitSession session, + AccessControlManager accessControlManager, + ValueFactory valueFactory, + CommitPolicy commitPolicy) { this.session = session; this.accessControlManager = accessControlManager; this.valueFactory = valueFactory; + this.commitPolicy = commitPolicy; + } + + private void save(String context) { + commitPolicy.commit(context); } public void apply( @@ -71,7 +82,9 @@ public void apply( updateAccessControlList( authorizable.getPrincipal(), path, modifyPermissions, modifyRestrictions, allow); } - session.save(); + save(String.format( + "applying permissions for authorizable '%s' at path '%s' with permissions '%s' and restrictions '%s'", + id, path, permissions, restrictions)); } catch (RepositoryException e) { throw new AclException( String.format("Failed to apply permissions for authorizable '%s' at path '%s'", id, path), e); @@ -215,7 +228,7 @@ private boolean removeAll(Authorizable authorizable, String path) { } if (result) { accessControlManager.setPolicy(path, jackrabbitAcl); - session.save(); + save(String.format("removing all permissions for authorizable '%s' at path '%s'", id, path)); } return result; } catch (RepositoryException e) { diff --git a/core/src/main/java/dev/vml/es/acm/core/code/CodeContext.java b/core/src/main/java/dev/vml/es/acm/core/code/CodeContext.java index b5b1b03c9..f0212f3b5 100644 --- a/core/src/main/java/dev/vml/es/acm/core/code/CodeContext.java +++ b/core/src/main/java/dev/vml/es/acm/core/code/CodeContext.java @@ -46,7 +46,7 @@ public CodeContext(OsgiContext osgiContext, ResourceResolver resourceResolver) { this.log = LoggerFactory.getLogger(getClass()); this.repo = new Repo(resourceResolver); - this.acl = new Acl(resourceResolver); + this.acl = new Acl(resourceResolver, repo); this.activator = new Activator(resourceResolver, osgiContext); this.formatter = new Formatter(); this.notifier = osgiContext.getService(NotificationManager.class); diff --git a/core/src/main/java/dev/vml/es/acm/core/repo/CommitPolicy.java b/core/src/main/java/dev/vml/es/acm/core/repo/CommitPolicy.java new file mode 100644 index 000000000..79c3eb847 --- /dev/null +++ b/core/src/main/java/dev/vml/es/acm/core/repo/CommitPolicy.java @@ -0,0 +1,33 @@ +package dev.vml.es.acm.core.repo; + +import org.apache.sling.api.resource.PersistenceException; +import org.apache.sling.api.resource.ResourceResolver; + +public interface CommitPolicy { + + boolean isAutoCommit(); + + void commit(String context); + + static CommitPolicy of(ResourceResolver resourceResolver, boolean autoCommit) { + return new CommitPolicy() { + @Override + public boolean isAutoCommit() { + return autoCommit; + } + + @Override + public void commit(String context) { + if (!autoCommit) { + return; + } + try { + resourceResolver.commit(); + } catch (PersistenceException e) { + throw new RepoException( + String.format("Cannot commit changes to repository while %s!", context), e); + } + } + }; + } +} diff --git a/core/src/main/java/dev/vml/es/acm/core/repo/Locker.java b/core/src/main/java/dev/vml/es/acm/core/repo/Locker.java index 67a9b15af..fc1948608 100644 --- a/core/src/main/java/dev/vml/es/acm/core/repo/Locker.java +++ b/core/src/main/java/dev/vml/es/acm/core/repo/Locker.java @@ -7,7 +7,6 @@ import java.util.Calendar; import java.util.HashMap; import java.util.Map; -import java.util.function.Supplier; import java.util.stream.Stream; import org.apache.commons.lang3.StringUtils; import org.apache.jackrabbit.JcrConstants; @@ -34,15 +33,15 @@ public class Locker { private final ResourceResolver resolver; - private final Supplier autoCommit; + private final CommitPolicy commitPolicy; public Locker(ResourceResolver resolver) { - this(resolver, () -> true); + this(resolver, CommitPolicy.of(resolver, true)); } - public Locker(ResourceResolver resolver, Supplier autoCommit) { + public Locker(ResourceResolver resolver, CommitPolicy commitPolicy) { this.resolver = resolver; - this.autoCommit = autoCommit; + this.commitPolicy = commitPolicy; } public boolean isLocked(String lockName) { @@ -106,13 +105,13 @@ public void lock(String lockName, Duration ttl) { props.put(LOCKED_UNTIL_PROP, lockedUntil); } resolver.create(dirResource, nodeName, props); - if (autoCommit.get()) { + if (commitPolicy.isAutoCommit()) { resolver.commit(); } LOG.debug("Created lock '{}'", name); return; } catch (PersistenceException e) { - if (autoCommit.get()) { + if (commitPolicy.isAutoCommit()) { resolver.revert(); resolver.refresh(); exceptionLast = e; @@ -142,13 +141,13 @@ public void unlock(String lockName) { return; } resolver.delete(lockCurrent); - if (autoCommit.get()) { + if (commitPolicy.isAutoCommit()) { resolver.commit(); } LOG.debug("Deleted lock '{}'", name); return; } catch (PersistenceException e) { - if (autoCommit.get()) { + if (commitPolicy.isAutoCommit()) { resolver.revert(); resolver.refresh(); exceptionLast = e; @@ -169,7 +168,7 @@ public void unlockAll() { } try { resolver.delete(root); - if (autoCommit.get()) { + if (commitPolicy.isAutoCommit()) { resolver.commit(); } LOG.debug("Deleted all locks"); diff --git a/core/src/main/java/dev/vml/es/acm/core/repo/Repo.java b/core/src/main/java/dev/vml/es/acm/core/repo/Repo.java index d5ee350d9..2f00c4dc1 100644 --- a/core/src/main/java/dev/vml/es/acm/core/repo/Repo.java +++ b/core/src/main/java/dev/vml/es/acm/core/repo/Repo.java @@ -15,7 +15,7 @@ import org.slf4j.LoggerFactory; import org.slf4j.helpers.NOPLogger; -public class Repo { +public class Repo implements CommitPolicy { private static final Logger LOG = LoggerFactory.getLogger(Repo.class); @@ -32,7 +32,7 @@ public class Repo { public Repo(ResourceResolver resourceResolver) { this.resourceResolver = resourceResolver; this.session = resourceResolver.adaptTo(Session.class); - this.locker = new Locker(resourceResolver, this::isAutoCommit); + this.locker = new Locker(resourceResolver, this); } public static Repo quiet(ResourceResolver resourceResolver) { @@ -130,6 +130,21 @@ public void dryRun(boolean enabled, Runnable operation) { } } + public void batch(Runnable operation) { + if (!autoCommit) { + throw new RepoException("Cannot start a batch: already inside a batch or dry run scope."); + } + this.autoCommit = false; + getLogger().info("Batch started. Changes will be committed once at the end."); + try { + operation.run(); + commit(); + getLogger().info("Batch completed. Changes committed."); + } finally { + this.autoCommit = true; + } + } + public void quiet(Runnable operation) { quiet(true, operation); } diff --git a/ui.content/src/main/content/jcr_root/conf/acm/settings/snippet/available/core/repo/batch.yml b/ui.content/src/main/content/jcr_root/conf/acm/settings/snippet/available/core/repo/batch.yml new file mode 100644 index 000000000..3f1b19d9d --- /dev/null +++ b/ui.content/src/main/content/jcr_root/conf/acm/settings/snippet/available/core/repo/batch.yml @@ -0,0 +1,30 @@ +group: Repo +name: repo_batch +content: | + repo.batch { + // operations + } +documentation: | + Group multiple repository operations into a single commit. + + By default every operation commits on its own (`autoCommit` is enabled). + Within a batch, auto-committing is turned off and all changes are committed once, at the end of the block. + This is useful for bulk changes, where a single save is more efficient than many individual commits. + + Batches cannot be nested and cannot be used inside a dry run - doing so throws an exception, + so that the "commit at the end" guarantee always holds. + + The `autoCommit` control applies only to operations performed via the `repo` and `acl` services. + Since everything shares the same session, calling vanilla Sling/AEM/JCR APIs directly (e.g. `session.save()`) + may trigger an accidental commit of the pending changes and break the "commit at the end" guarantee. + + For example: + ```groovy + void doRun() { + repo.batch { + acl.createUser("alice.doe") + acl.allow(userId: "alice.doe", path: "/content/acme", permissions: ["jcr:read"]) + repo.get("/content/acme").ensureRegularFolder() + } + } + ``` diff --git a/ui.content/src/main/content/jcr_root/conf/acm/settings/snippet/available/core/repo/dry_run.yml b/ui.content/src/main/content/jcr_root/conf/acm/settings/snippet/available/core/repo/dry_run.yml index ad1c10b7b..b22f5d7f6 100644 --- a/ui.content/src/main/content/jcr_root/conf/acm/settings/snippet/available/core/repo/dry_run.yml +++ b/ui.content/src/main/content/jcr_root/conf/acm/settings/snippet/available/core/repo/dry_run.yml @@ -11,6 +11,10 @@ documentation: | When exception is thrown during dry run, the revert operation is performed, so no changes are made to the repository. Useful especially when combined with `inputs.bool("dryRun")` to allow user to choose whether to perform dry run or not. + + The `autoCommit` control applies only to operations performed via the `repo` and `acl` services. + Since everything shares the same session, calling vanilla Sling/AEM/JCR APIs directly (e.g. `session.save()`) + may trigger an accidental commit of the pending changes and defeat the dry run. For example: ```groovy