commit 4b9ecddf917d0a027cd99dba4cd4c53e17075fca Author: Florent Guillaume Date: Thu Nov 30 19:44:47 2017 +0100 NXP-23788: optimize Read ACL update on big volume diff --git a/nuxeo-core/nuxeo-core-api/src/main/java/org/nuxeo/ecm/core/api/CoreSession.java b/nuxeo-core/nuxeo-core-api/src/main/java/org/nuxeo/ecm/core/api/CoreSession.java index 91b7843f7a1..8294e2ace91 100644 --- a/nuxeo-core/nuxeo-core-api/src/main/java/org/nuxeo/ecm/core/api/CoreSession.java +++ b/nuxeo-core/nuxeo-core-api/src/main/java/org/nuxeo/ecm/core/api/CoreSession.java @@ -715,6 +715,16 @@ public interface CoreSession extends AutoCloseable { */ void replaceACE(DocumentRef docRef, String aclName, ACE oldACE, ACE newACE); + /** + * Updates the Read ACLs for some documents. + *

+ * For INTERNAL use by the core. + * + * @param docIds the document ids + * @since 9.10 + */ + void updateReadACLs(Collection docIds); + /** * Returns {@code true} if negative ACLs are allowed. *

diff --git a/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSCachingRepository.java b/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSCachingRepository.java index 2b26ed035fa..abeda39afae 100644 --- a/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSCachingRepository.java +++ b/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSCachingRepository.java @@ -184,6 +184,12 @@ public class DBSCachingRepository implements DBSRepository { return state; } + @Override + public State readPartialState(String id, Collection keys) { + // bypass caches, as the goal of this method is to not trash caches for one-shot reads + return repository.readPartialState(id, keys); + } + @Override public List readStates(List ids) { ImmutableMap statesMap = cache.getAllPresent(ids); @@ -346,6 +352,12 @@ public class DBSCachingRepository implements DBSRepository { repository.queryKeyValueArray(key, value, ids, proxyTargets, targetProxies); } + @Override + public void queryKeyValueArray(String key, Object value, Set ids, Map proxyTargets, + Map targetProxies, int limit) { + repository.queryKeyValueArray(key, value, ids, proxyTargets, targetProxies, limit); + } + @Override public boolean queryKeyValuePresence(String key, String value, Set ignored) { return repository.queryKeyValuePresence(key, value, ignored); diff --git a/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSRepository.java b/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSRepository.java index c32e7c84f4d..ada38582731 100644 --- a/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSRepository.java +++ b/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSRepository.java @@ -19,6 +19,7 @@ package org.nuxeo.ecm.core.storage.dbs; import java.io.Serializable; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Set; @@ -85,6 +86,18 @@ public interface DBSRepository extends Repository, LockManager { */ State readState(String id); + /** + * Reads the partial state of a document. + * + * @param id the document id + * @param keys the keys to read + * @return the document partial state, or {@code null} if not found + */ + default State readPartialState(String id, Collection keys) { + // overrides should optimize to only return the required keys and nothing more + return readState(id); + } + /** * Reads the states of several documents. *

@@ -180,6 +193,22 @@ public interface DBSRepository extends Repository, LockManager { void queryKeyValueArray(String key, Object value, Set ids, Map proxyTargets, Map targetProxies); + /** + * Queries the repository for document ids having value in key (an array). + * + * @param key the key + * @param value the value + * @param ids the set which receives the documents ids + * @param proxyTargets returns a map of proxy to target among the documents found + * @param targetProxies returns a map of target to proxies among the document found + * @param limit the maximum number of ids to return + */ + default void queryKeyValueArray(String key, Object value, Set ids, Map proxyTargets, + Map targetProxies, int limit) { + // limit unused by default, override for a more efficient implementation + queryKeyValueArray(key, value, ids, proxyTargets, targetProxies); + } + /** * Queries the repository to check if there are documents having key = value. * diff --git a/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSSession.java b/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSSession.java index ca66cacd0c1..80636d2edc5 100644 --- a/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSSession.java +++ b/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSSession.java @@ -69,6 +69,7 @@ import java.text.Normalizer; import java.text.ParseException; import java.util.ArrayList; import java.util.Calendar; +import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.GregorianCalendar; @@ -691,7 +692,6 @@ public class DBSSession implements Session { // update read acls // the save also makes copy results visible in searches, like in VCS - transaction.save(); // read acls update needs full tree transaction.updateTreeReadAcls(copyId); return getDocument(copyState); @@ -816,7 +816,6 @@ public class DBSSession implements Session { transaction.updateAncestors(sourceId, ndel, ancestorIds); // update read acls - transaction.save(); // read acls update needs full tree transaction.updateTreeReadAcls(sourceId); return source; @@ -845,7 +844,7 @@ public class DBSSession implements Session { // find all sub-docs and whether they're proxies Map proxyTargets = new HashMap<>(); Map targetProxies = new HashMap<>(); - Set removedIds = transaction.getSubTree(id, proxyTargets, targetProxies); + Set removedIds = transaction.getSubTree(id, proxyTargets, targetProxies, 0); // add this node removedIds.add(id); @@ -1196,6 +1195,11 @@ public class DBSSession implements Session { } }; + @Override + public void updateReadACLs(Collection docIds) { + transaction.updateReadACLs(docIds); + } + @Override public boolean isNegativeAclAllowed() { return false; @@ -1271,7 +1275,6 @@ public class DBSSession implements Session { docState.put(KEY_ACP, acpToMem(acp)); // update read acls - transaction.save(); // read acls update needs full tree transaction.updateTreeReadAcls(id); } diff --git a/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSTransactionState.java b/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSTransactionState.java index 0143938e182..6554a890b52 100644 --- a/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSTransactionState.java +++ b/nuxeo-core/nuxeo-core-storage-dbs/src/main/java/org/nuxeo/ecm/core/storage/dbs/DBSTransactionState.java @@ -47,8 +47,10 @@ import static org.nuxeo.ecm.core.storage.dbs.DBSDocument.KEY_READ_ACL; import static org.nuxeo.ecm.core.storage.dbs.DBSDocument.KEY_VERSION_SERIES_ID; import java.io.Serializable; +import java.security.Principal; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -62,8 +64,14 @@ import java.util.Set; import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.nuxeo.ecm.core.BatchFinderWork; +import org.nuxeo.ecm.core.BatchProcessorWork; import org.nuxeo.ecm.core.api.ConcurrentUpdateException; +import org.nuxeo.ecm.core.api.PartialList; +import org.nuxeo.ecm.core.api.SystemPrincipal; import org.nuxeo.ecm.core.api.repository.RepositoryManager; +import org.nuxeo.ecm.core.query.QueryFilter; +import org.nuxeo.ecm.core.query.sql.NXQL; import org.nuxeo.ecm.core.schema.SchemaManager; import org.nuxeo.ecm.core.schema.types.Schema; import org.nuxeo.ecm.core.security.SecurityService; @@ -99,6 +107,18 @@ public class DBSTransactionState { private static final String KEY_UNDOLOG_CREATE = "__UNDOLOG_CREATE__\0\0"; + /** Keys used when computing Read ACLs. */ + protected static final Set READ_ACL_RECURSION_KEYS = new HashSet<>( + Arrays.asList(KEY_READ_ACL, KEY_ACP, KEY_IS_VERSION, KEY_VERSION_SERIES_ID, KEY_PARENT_ID)); + + public static final String READ_ACL_ASYNC_ENABLED_PROPERTY = "nuxeo.core.readacl.async.enabled"; + + public static final String READ_ACL_ASYNC_ENABLED_DEFAULT = "false"; + + public static final String READ_ACL_ASYNC_THRESHOLD_PROPERTY = "nuxeo.core.readacl.async.threshold"; + + public static final String READ_ACL_ASYNC_THRESHOLD_DEFAULT = "500"; + protected final DBSRepository repository; protected final DBSSession session; @@ -384,7 +404,7 @@ public class DBSTransactionState { */ public void updateAncestors(String id, int ndel, Object[] ancestorIds) { int nadd = ancestorIds.length; - Set ids = getSubTree(id, null, null); + Set ids = getSubTree(id, null, null, 0); ids.add(id); for (String cid : ids) { // XXX TODO oneShot update, don't pollute transient space @@ -402,31 +422,156 @@ public class DBSTransactionState { } } + protected int getReadAclsAsyncThreshold() { + boolean enabled = Boolean.parseBoolean( + Framework.getProperty(READ_ACL_ASYNC_ENABLED_PROPERTY, READ_ACL_ASYNC_ENABLED_DEFAULT)); + if (enabled) { + return Integer.parseInt( + Framework.getProperty(READ_ACL_ASYNC_THRESHOLD_PROPERTY, READ_ACL_ASYNC_THRESHOLD_DEFAULT)); + } else { + return 0; + } + } + /** * Updates the Read ACLs recursively on a document. */ public void updateTreeReadAcls(String id) { // versions too XXX TODO - Set ids = getSubTree(id, null, null); - ids.add(id); - ids.forEach(this::updateDocumentReadAcls); + + save(); // flush everything to the database + + // update the doc itself + updateDocumentReadAcls(id); + + // check if we have a small enough number of descendants that we can process them synchronously + int limit = getReadAclsAsyncThreshold(); + Set ids = getSubTree(id, null, null, limit); + if (limit == 0 || ids.size() < limit) { + // update all descendants synchronously + ids.forEach(this::updateDocumentReadAcls); + } else { + // update the direct children synchronously, the rest asynchronously + + // update the direct children (with a limit in case it's too big) + String nxql = String.format("SELECT ecm:uuid FROM Document WHERE ecm:parentId = '%s'", id); + Principal principal = new SystemPrincipal(null); + QueryFilter queryFilter = new QueryFilter(principal, null, null, null, Collections.emptyList(), limit, 0); + PartialList> pl = session.queryProjection(nxql, NXQL.NXQL, queryFilter, false, 0, + new Object[0]); + for (Map map : pl) { + String childId = (String) map.get(NXQL.ECM_UUID); + updateDocumentReadAcls(childId); + } + + // asynchronous work to do the whole tree + nxql = String.format("SELECT ecm:uuid FROM Document WHERE ecm:ancestorId = '%s'", id); + Work work = new FindReadAclsWork(repository.getName(), nxql, null); + Framework.getService(WorkManager.class).schedule(work); + } + } + + /** + * Work to find the ids of documents for which Read ACLs must be recomputed, and launch the needed update works. + * + * @since 9.10 + */ + public static class FindReadAclsWork extends BatchFinderWork { + + private static final long serialVersionUID = 1L; + + public FindReadAclsWork(String repositoryName, String nxql, String originatingUsername) { + super(repositoryName, nxql, originatingUsername); + } + + @Override + public String getTitle() { + return "Find descendants for Read ACLs"; + } + + @Override + public String getCategory() { + return "security"; + } + + @Override + public Work getBatchProcessorWork(List docIds) { + return new UpdateReadAclsWork(repositoryName, docIds, getOriginatingUsername()); + } + } + + /** + * Work to update the Read ACLs on a list of documents, without recursion. + * + * @since 9.10 + */ + public static class UpdateReadAclsWork extends BatchProcessorWork { + + private static final long serialVersionUID = 1L; + + public UpdateReadAclsWork(String repositoryName, List docIds, String originatingUsername) { + super(repositoryName, docIds, originatingUsername); + } + + @Override + public String getTitle() { + return "Update Read ACLs"; + } + + @Override + public String getCategory() { + return "security"; + } + + @Override + public void processBatch(List docIds) { + session.updateReadACLs(docIds); + } + } + + /** + * Updates the Read ACLs on a document (not recursively), bypassing transient space and caches for the document + * itself (not the ancestors, needed for ACL inheritance and for which caching is useful). + */ + public void updateReadACLs(Collection docIds) { + docIds.forEach(id -> updateDocumentReadAclsNoCache(id)); } /** * Updates the Read ACLs on a document (not recursively) */ protected void updateDocumentReadAcls(String id) { - // XXX TODO oneShot update, don't pollute transient space DBSDocumentState docState = getStateForUpdate(id); - docState.put(KEY_READ_ACL, getReadACL(docState)); + docState.put(KEY_READ_ACL, getReadACL(docState.getState())); + } + + /** + * Updates the Read ACLs on a document, without polluting caches. + *

+ * When fetching parents recursively to compute inheritance, the regular transient space and repository caching are + * used. + */ + protected void updateDocumentReadAclsNoCache(String id) { + // no transient for state read, and we don't want to trash caches + // fetch from repository only the properties needed for Read ACL computation and recursion + State state = repository.readPartialState(id, READ_ACL_RECURSION_KEYS); + State oldState = new State(1); + oldState.put(KEY_READ_ACL, state.get(KEY_READ_ACL)); + // compute new value + State newState = new State(1); + newState.put(KEY_READ_ACL, getReadACL(state)); + StateDiff diff = StateHelper.diff(oldState, newState); + if (!diff.isEmpty()) { + // no transient for state write, we write directly and just invalidate caches + repository.updateState(id, diff); + } } /** * Gets the Read ACL (flat list of users having browse permission, including inheritance) on a document. */ - protected String[] getReadACL(DBSDocumentState docState) { + protected String[] getReadACL(State state) { Set racls = new HashSet<>(); - State state = docState.getState(); LOOP: do { @SuppressWarnings("unchecked") List aclList = (List) state.get(KEY_ACP); @@ -455,14 +600,10 @@ public class DBSTransactionState { } } } - // get parent - if (TRUE.equals(state.get(KEY_IS_VERSION))) { - String versionSeriesId = (String) state.get(KEY_VERSION_SERIES_ID); - state = versionSeriesId == null ? null : getStateForRead(versionSeriesId); - } else { - String parentId = (String) state.get(KEY_PARENT_ID); - state = parentId == null ? null : getStateForRead(parentId); - } + // get the parent; for a version the parent is the live document + String parentKey = TRUE.equals(state.get(KEY_IS_VERSION)) ? KEY_VERSION_SERIES_ID : KEY_PARENT_ID; + String parentId = (String) state.get(parentKey); + state = parentId == null ? null : getStateForRead(parentId); } while (state != null); // sort to have canonical order @@ -479,11 +620,13 @@ public class DBSTransactionState { * @param id the root of the tree (not included in results) * @param proxyTargets returns a map of proxy to target among the documents found * @param targetProxies returns a map of target to proxies among the document found + * @param limit the maximum number of ids to return */ - protected Set getSubTree(String id, Map proxyTargets, Map targetProxies) { + protected Set getSubTree(String id, Map proxyTargets, Map targetProxies, + int limit) { Set ids = new HashSet<>(); // check repository - repository.queryKeyValueArray(KEY_ANCESTOR_IDS, id, ids, proxyTargets, targetProxies); + repository.queryKeyValueArray(KEY_ANCESTOR_IDS, id, ids, proxyTargets, targetProxies, limit); return ids; } diff --git a/nuxeo-core/nuxeo-core-storage-mem/src/main/java/org/nuxeo/ecm/core/storage/mem/MemRepository.java b/nuxeo-core/nuxeo-core-storage-mem/src/main/java/org/nuxeo/ecm/core/storage/mem/MemRepository.java index da297b619a9..63879e039f5 100644 --- a/nuxeo-core/nuxeo-core-storage-mem/src/main/java/org/nuxeo/ecm/core/storage/mem/MemRepository.java +++ b/nuxeo-core/nuxeo-core-storage-mem/src/main/java/org/nuxeo/ecm/core/storage/mem/MemRepository.java @@ -36,6 +36,7 @@ import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; @@ -128,8 +129,23 @@ public class MemRepository extends DBSRepositoryBase { @Override public State readState(String id) { + return readPartialState(id, null); + } + + @Override + public State readPartialState(String id, Collection keys) { State state = states.get(id); if (state != null) { + if (keys != null && !keys.isEmpty()) { + State partialState = new State(); + for (String key : keys) { + Serializable value = state.get(key); + if (value != null) { + partialState.put(key, value); + } + } + state = partialState; + } if (log.isTraceEnabled()) { log.trace("Mem: READ " + id + ": " + state); } @@ -254,6 +270,12 @@ public class MemRepository extends DBSRepositoryBase { @Override public void queryKeyValueArray(String key, Object value, Set ids, Map proxyTargets, Map targetProxies) { + queryKeyValueArray(key, value, ids, proxyTargets, targetProxies, 0); + } + + @Override + public void queryKeyValueArray(String key, Object value, Set ids, Map proxyTargets, + Map targetProxies, int limit) { if (log.isTraceEnabled()) { log.trace("Mem: QUERY " + key + " = " + value); } @@ -274,6 +296,9 @@ public class MemRepository extends DBSRepositoryBase { targetProxies.put(id, proxyIds); } } + if (limit != 0 && ids.size() >= limit) { + break STATE; + } continue STATE; } } diff --git a/nuxeo-core/nuxeo-core-storage-mongodb/src/main/java/org/nuxeo/ecm/core/storage/mongodb/MongoDBRepository.java b/nuxeo-core/nuxeo-core-storage-mongodb/src/main/java/org/nuxeo/ecm/core/storage/mongodb/MongoDBRepository.java index 9cca7d5430a..a381def29e5 100644 --- a/nuxeo-core/nuxeo-core-storage-mongodb/src/main/java/org/nuxeo/ecm/core/storage/mongodb/MongoDBRepository.java +++ b/nuxeo-core/nuxeo-core-storage-mongodb/src/main/java/org/nuxeo/ecm/core/storage/mongodb/MongoDBRepository.java @@ -50,6 +50,7 @@ import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; +import java.util.Collection; import java.util.Date; import java.util.HashSet; import java.util.List; @@ -662,6 +663,12 @@ public class MongoDBRepository extends DBSRepositoryBase { return findOne(query); } + @Override + public State readPartialState(String id, Collection keys) { + DBObject query = new BasicDBObject(idKey, id); + return findOne(query, keys); + } + @Override public List readStates(List ids) { DBObject query = new BasicDBObject(idKey, new BasicDBObject(QueryOperators.IN, ids)); @@ -758,6 +765,12 @@ public class MongoDBRepository extends DBSRepositoryBase { @Override public void queryKeyValueArray(String key, Object value, Set ids, Map proxyTargets, Map targetProxies) { + queryKeyValueArray(key, value, ids, proxyTargets, targetProxies, 0); + } + + @Override + public void queryKeyValueArray(String key, Object value, Set ids, Map proxyTargets, + Map targetProxies, int limit) { DBObject query = new BasicDBObject(key, value); DBObject fields = new BasicDBObject(); if (useCustomId) { @@ -770,7 +783,7 @@ public class MongoDBRepository extends DBSRepositoryBase { if (log.isTraceEnabled()) { logQuery(query, fields); } - DBCursor cursor = coll.find(query, fields); + DBCursor cursor = coll.find(query, fields).limit(limit); try { for (DBObject ob : cursor) { String id = (String) ob.get(idKey); @@ -808,6 +821,15 @@ public class MongoDBRepository extends DBSRepositoryBase { return bsonToState(coll.findOne(query)); } + protected State findOne(DBObject query, Collection keys) { + DBObject fields = new BasicDBObject(keys.size()); + keys.forEach(key -> fields.put(key, ONE)); + if (log.isTraceEnabled()) { + logQuery(query, fields); + } + return bsonToState(coll.findOne(query, fields)); + } + protected List findAll(DBObject query, int sizeHint) { if (log.isTraceEnabled()) { logQuery(query, null); diff --git a/nuxeo-core/nuxeo-core-storage-sql/nuxeo-core-storage-sql/src/main/java/org/nuxeo/ecm/core/storage/sql/coremodel/SQLSession.java b/nuxeo-core/nuxeo-core-storage-sql/nuxeo-core-storage-sql/src/main/java/org/nuxeo/ecm/core/storage/sql/coremodel/SQLSession.java index 3951c32a8b2..c25b67f2b41 100644 --- a/nuxeo-core/nuxeo-core-storage-sql/nuxeo-core-storage-sql/src/main/java/org/nuxeo/ecm/core/storage/sql/coremodel/SQLSession.java +++ b/nuxeo-core/nuxeo-core-storage-sql/nuxeo-core-storage-sql/src/main/java/org/nuxeo/ecm/core/storage/sql/coremodel/SQLSession.java @@ -24,6 +24,7 @@ import java.text.ParseException; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; +import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.GregorianCalendar; @@ -678,6 +679,11 @@ public class SQLSession implements Session { return negativeAclAllowed; } + @Override + public void updateReadACLs(Collection docIds) { + throw new UnsupportedOperationException(); + } + @Override public void setACP(Document doc, ACP acp, boolean overwrite) { if (!overwrite && acp == null) { diff --git a/nuxeo-core/nuxeo-core-test/src/test/java/org/nuxeo/ecm/core/TestSQLRepositorySecurity.java b/nuxeo-core/nuxeo-core-test/src/test/java/org/nuxeo/ecm/core/TestSQLRepositorySecurity.java index 519fccf8b86..2739da517ba 100644 --- a/nuxeo-core/nuxeo-core-test/src/test/java/org/nuxeo/ecm/core/TestSQLRepositorySecurity.java +++ b/nuxeo-core/nuxeo-core-test/src/test/java/org/nuxeo/ecm/core/TestSQLRepositorySecurity.java @@ -40,9 +40,11 @@ import static org.nuxeo.ecm.core.api.security.SecurityConstants.WRITE; import static org.nuxeo.ecm.core.api.security.SecurityConstants.WRITE_PROPERTIES; import static org.nuxeo.ecm.core.api.security.SecurityConstants.WRITE_SECURITY; +import java.io.Serializable; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Map; import javax.inject.Inject; @@ -63,6 +65,7 @@ import org.nuxeo.ecm.core.api.DocumentModelList; import org.nuxeo.ecm.core.api.DocumentRef; import org.nuxeo.ecm.core.api.DocumentSecurityException; import org.nuxeo.ecm.core.api.NuxeoPrincipal; +import org.nuxeo.ecm.core.api.PartialList; import org.nuxeo.ecm.core.api.PathRef; import org.nuxeo.ecm.core.api.impl.DocumentModelImpl; import org.nuxeo.ecm.core.api.impl.UserPrincipal; @@ -74,6 +77,7 @@ import org.nuxeo.ecm.core.api.security.UserEntry; import org.nuxeo.ecm.core.api.security.impl.ACLImpl; import org.nuxeo.ecm.core.api.security.impl.ACPImpl; import org.nuxeo.ecm.core.api.security.impl.UserEntryImpl; +import org.nuxeo.ecm.core.event.EventService; import org.nuxeo.ecm.core.security.SecurityService; import org.nuxeo.ecm.core.storage.sql.coremodel.SQLSession; import org.nuxeo.ecm.core.test.CoreFeature; @@ -99,6 +103,9 @@ public class TestSQLRepositorySecurity { @Inject protected CoreSession session; + @Inject + protected EventService eventService; + @Before public void setUp() { if (allowNegativeAcl()) { @@ -866,4 +873,72 @@ public class TestSQLRepositorySecurity { logCaptureResults.assertHasEvent(); } + @Test + public void testReadAclOnLargeTree() { + String enabledProp = "nuxeo.core.readacl.async.enabled"; + String thresholdProp = "nuxeo.core.readacl.async.threshold"; + Framework.getProperties().put(enabledProp, "true"); + Framework.getProperties().put(thresholdProp, "10"); + try { + doTestReadAclOnLargeTree(); + } finally { + Framework.getProperties().remove(enabledProp); + Framework.getProperties().remove(thresholdProp); + } + } + + protected void doTestReadAclOnLargeTree() { + DocumentModel rootFolder = session.createDocumentModel("/", "folder", "Folder"); + rootFolder = session.createDocument(rootFolder); + String firstUser = "mickey"; + String secondUser = "minnie"; + + // set ACL for first user on root folder + ACP acp = new ACPImpl(); + acp.addACE(ACL.LOCAL_ACL, new ACE(firstUser, READ, true)); + rootFolder.setACP(acp, true); + + int nbLevels = 10; + int nbPerLevel = 10; + DocumentModel parent = rootFolder; + for (int level = 0; level < nbLevels; level++) { + DocumentModel folder = session.createDocumentModel(parent.getPathAsString(), "folder-" + level, "Folder"); + folder = session.createDocument(folder); + for (int i = 0; i < nbPerLevel; i++) { + DocumentModel doc = session.createDocumentModel(folder.getPathAsString(), "doc-" + level + "-" + i, + "File"); + doc = session.createDocument(doc); + } + parent = folder; + } + session.save(); + int nbDocs = 1 + nbLevels * (nbPerLevel + 1); + + // check that only first user has access to everything, but not second user + assertEquals(nbDocs, numberOfReadableDocuments(firstUser)); + assertEquals(0, numberOfReadableDocuments(secondUser)); + + // set ACL for user on root folder + acp.addACE(ACL.LOCAL_ACL, new ACE(secondUser, READ, true)); + rootFolder.setACP(acp, true); + session.save(); + + // wait for asynchronous stuff to finish + TransactionHelper.commitOrRollbackTransaction(); + TransactionHelper.startTransaction(); + eventService.waitForAsyncCompletion(); + + // check that both users now have access to everything + assertEquals(nbDocs, numberOfReadableDocuments(firstUser)); + assertEquals(nbDocs, numberOfReadableDocuments(secondUser)); + } + + protected int numberOfReadableDocuments(String username) { + try (CoreSession userSession = openSessionAs(username)) { + String nxql = "SELECT ecm:uuid FROM Document"; + PartialList> pl = userSession.queryProjection(nxql, 0, 0); + return pl.list.size(); + } + } + } diff --git a/nuxeo-core/nuxeo-core/src/main/java/org/nuxeo/ecm/core/BatchFinderWork.java b/nuxeo-core/nuxeo-core/src/main/java/org/nuxeo/ecm/core/BatchFinderWork.java new file mode 100644 index 00000000000..8ddfb12cc7c --- /dev/null +++ b/nuxeo-core/nuxeo-core/src/main/java/org/nuxeo/ecm/core/BatchFinderWork.java @@ -0,0 +1,102 @@ +/* + * (C) Copyright 2017 Nuxeo SA (http://nuxeo.com/) and others. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Contributors: + * Florent Guillaume + */ +package org.nuxeo.ecm.core; + +import java.util.List; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.nuxeo.ecm.core.api.ScrollResult; +import org.nuxeo.ecm.core.work.AbstractWork; +import org.nuxeo.ecm.core.work.api.Work; +import org.nuxeo.ecm.core.work.api.WorkManager; +import org.nuxeo.runtime.api.Framework; +import org.nuxeo.runtime.transaction.TransactionHelper; + +/** + * Abstract Work to find the ids of documents for which some process must be executed in batch, based on a NXQL query. + * + * @since 9.10 + */ +public abstract class BatchFinderWork extends AbstractWork { + + private static final long serialVersionUID = 1L; + + private static final Log log = LogFactory.getLog(BatchFinderWork.class); + + protected static final int SCROLL_KEEPALIVE_SECONDS = 60; + + protected String nxql; + + public BatchFinderWork(String repositoryName, String nxql, String originatingUsername) { + this.repositoryName = repositoryName; + this.nxql = nxql; + setOriginatingUsername(originatingUsername); + } + + @Override + public int getRetryCount() { + // even read-only threads may encounter concurrent update exceptions when trying to read + // a previously deleted complex property due to read committed semantics (see NXP-17384) + return 1; + } + + public int getBatchSize() { + return 500; + } + + @Override + public void work() { + int batchSize = getBatchSize(); + if (log.isDebugEnabled()) { + log.debug(getTitle() + ": Starting batch find for query: " + nxql + " with batch size: " + batchSize); + } + openSystemSession(); + setProgress(Progress.PROGRESS_INDETERMINATE); + setStatus("Searching"); + + long batchCount = 0; + long documentCount = 0; + ScrollResult scroll = session.scroll(nxql, batchSize, SCROLL_KEEPALIVE_SECONDS); + while (scroll.hasResults()) { + List docIds = scroll.getResultIds(); + // schedule the batch + if (!docIds.isEmpty()) { + Framework.getService(WorkManager.class).schedule(getBatchProcessorWork(docIds)); + } + batchCount += 1; + documentCount += docIds.size(); + setProgress(new Progress(documentCount, -1)); + // next batch + scroll = session.scroll(scroll.getScrollId()); + TransactionHelper.commitOrRollbackTransaction(); + TransactionHelper.startTransaction(); + } + + if (log.isDebugEnabled()) { + log.debug(getTitle() + ": Submitted " + documentCount + " documents in " + batchCount + + " batch processor workers"); + } + setProgress(new Progress(documentCount, documentCount)); + setStatus("Done"); + } + + public abstract Work getBatchProcessorWork(List docIds); + +} diff --git a/nuxeo-core/nuxeo-core/src/main/java/org/nuxeo/ecm/core/BatchProcessorWork.java b/nuxeo-core/nuxeo-core/src/main/java/org/nuxeo/ecm/core/BatchProcessorWork.java new file mode 100644 index 00000000000..ab2be476440 --- /dev/null +++ b/nuxeo-core/nuxeo-core/src/main/java/org/nuxeo/ecm/core/BatchProcessorWork.java @@ -0,0 +1,88 @@ +/* + * (C) Copyright 2017 Nuxeo SA (http://nuxeo.com/) and others. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Contributors: + * Florent Guillaume + */ +package org.nuxeo.ecm.core; + +import java.util.List; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.nuxeo.ecm.core.work.AbstractWork; +import org.nuxeo.runtime.transaction.TransactionHelper; + +/** + * Abstract Work to process a list of documents. + * + * @since 9.10 + */ +public abstract class BatchProcessorWork extends AbstractWork { + + private static final long serialVersionUID = 1L; + + private static final Log log = LogFactory.getLog(BatchProcessorWork.class); + + public BatchProcessorWork(String repositoryName, List docIds, String originatingUsername) { + setDocuments(repositoryName, docIds); + setOriginatingUsername(originatingUsername); + } + + @Override + public int getRetryCount() { + // even read-only threads may encounter concurrent update exceptions when trying to read + // a previously deleted complex property due to read committed semantics (see NXP-17384) + return 1; + } + + public int getBatchSize() { + return 50; + } + + @Override + public void work() { + int size = docIds.size(); + int batchSize = getBatchSize(); + if (log.isDebugEnabled()) { + log.debug(getTitle() + ": Starting processing: " + size + " documents with batch size: " + batchSize); + } + openSystemSession(); + setProgress(new Progress(0, size)); + setStatus("Processing"); + + for (int start = 0; start < size; start += batchSize) { + int end = start + batchSize; + if (end > size) { + end = size; + } + List batch = docIds.subList(start, end); + // process the batch + processBatch(batch); + setProgress(new Progress(end, size)); + // next batch + TransactionHelper.commitOrRollbackTransaction(); + TransactionHelper.startTransaction(); + } + + if (log.isDebugEnabled()) { + log.debug(getTitle() + ": Finished processing for batch of size:" + size); + } + setStatus("Done"); + } + + public abstract void processBatch(List docIds); + +} diff --git a/nuxeo-core/nuxeo-core/src/main/java/org/nuxeo/ecm/core/api/AbstractSession.java b/nuxeo-core/nuxeo-core/src/main/java/org/nuxeo/ecm/core/api/AbstractSession.java index 8a2bc109e64..183d1e3c961 100644 --- a/nuxeo-core/nuxeo-core/src/main/java/org/nuxeo/ecm/core/api/AbstractSession.java +++ b/nuxeo-core/nuxeo-core/src/main/java/org/nuxeo/ecm/core/api/AbstractSession.java @@ -608,6 +608,11 @@ public abstract class AbstractSession implements CoreSession, Serializable { } } + @Override + public void updateReadACLs(Collection docIds) { + getSession().updateReadACLs(docIds); + } + @Override public boolean isNegativeAclAllowed() { return getSession().isNegativeAclAllowed(); diff --git a/nuxeo-core/nuxeo-core/src/main/java/org/nuxeo/ecm/core/model/Session.java b/nuxeo-core/nuxeo-core/src/main/java/org/nuxeo/ecm/core/model/Session.java index 89f2123ec67..efaaf54379f 100644 --- a/nuxeo-core/nuxeo-core/src/main/java/org/nuxeo/ecm/core/model/Session.java +++ b/nuxeo-core/nuxeo-core/src/main/java/org/nuxeo/ecm/core/model/Session.java @@ -20,6 +20,7 @@ package org.nuxeo.ecm.core.model; import java.io.Serializable; +import java.util.Collection; import java.util.List; import java.util.Map; @@ -71,7 +72,7 @@ public interface Session { /** * Executes the given query and returns the first batch of results of batchSize, next batch must be requested within * the keepAliveSeconds delay. - * + * * @since 8.4 */ ScrollResult scroll(String query, int batchSize, int keepAliveSeconds); @@ -236,6 +237,14 @@ public interface Session { void setACP(Document doc, ACP acp, boolean overwrite); + /** + * Updates the Read ACLs for some documents. + * + * @param docIds the document ids + * @since 9.10 + */ + void updateReadACLs(Collection docIds); + /** * Gets the fulltext extracted from the binary fields. *