mirror of
https://github.com/Alfresco/alfresco-community-repo.git
synced 2025-07-31 17:39:05 +00:00
MNT-22385 Cmis query GetTotalNumItems is returning wrong value (#504)
* Changes made to correct the value of totalItems when performing a TMDQ * Fixes after review - Slight change was made to NodePermissionAssessor to log when permission limits are exceeded * Now pre-computing maxPermissionChecks value as per review suggestion
This commit is contained in:
@@ -112,6 +112,8 @@ public class DBQueryEngine implements QueryEngine
|
||||
|
||||
private long maxPermissionCheckTimeMillis;
|
||||
|
||||
private boolean maxPermissionCheckEnabled;
|
||||
|
||||
protected EntityLookupCache<Long, Node, NodeRef> nodesCache;
|
||||
|
||||
private List<Pair<Long, StoreRef>> stores;
|
||||
@@ -133,6 +135,11 @@ public class DBQueryEngine implements QueryEngine
|
||||
this.maxPermissionCheckTimeMillis = maxPermissionCheckTimeMillis;
|
||||
}
|
||||
|
||||
public void setMaxPermissionCheckEnabled(boolean maxPermissionCheckEnabled)
|
||||
{
|
||||
this.maxPermissionCheckEnabled = maxPermissionCheckEnabled;
|
||||
}
|
||||
|
||||
public void setTemplate(SqlSessionTemplate template)
|
||||
{
|
||||
this.template = template;
|
||||
@@ -329,13 +336,7 @@ public class DBQueryEngine implements QueryEngine
|
||||
@Override
|
||||
public void handleResult(ResultContext<? extends Node> context)
|
||||
{
|
||||
doHandleResult(permissionAssessor, nodes, requiredNodes, context);
|
||||
}
|
||||
|
||||
private void doHandleResult(NodePermissionAssessor permissionAssessor, List<Node> nodes,
|
||||
int requiredNodes, ResultContext<? extends Node> context)
|
||||
{
|
||||
if (nodes.size() >= requiredNodes)
|
||||
if (!maxPermissionCheckEnabled && nodes.size() >= requiredNodes)
|
||||
{
|
||||
context.stop();
|
||||
return;
|
||||
@@ -344,7 +345,7 @@ public class DBQueryEngine implements QueryEngine
|
||||
Node node = context.getResultObject();
|
||||
addStoreInfo(node);
|
||||
|
||||
boolean shouldCache = nodes.size() >= options.getSkipCount();
|
||||
boolean shouldCache = shouldCache(options, nodes, requiredNodes);
|
||||
if(shouldCache)
|
||||
{
|
||||
logger.debug("- selected node "+nodes.size()+": "+node.getUuid()+" "+node.getId());
|
||||
@@ -356,9 +357,16 @@ public class DBQueryEngine implements QueryEngine
|
||||
}
|
||||
|
||||
if (permissionAssessor.isIncluded(node))
|
||||
{
|
||||
if (nodes.size() > requiredNodes)
|
||||
{
|
||||
nodes.add(node);
|
||||
}
|
||||
else
|
||||
{
|
||||
nodes.add(shouldCache ? node : null);
|
||||
}
|
||||
}
|
||||
|
||||
if (permissionAssessor.shouldQuitChecks())
|
||||
{
|
||||
@@ -366,6 +374,18 @@ public class DBQueryEngine implements QueryEngine
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
private boolean shouldCache(QueryOptions options, List<Node> nodes, int requiredNodes)
|
||||
{
|
||||
if (nodes.size() > requiredNodes)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
else
|
||||
{
|
||||
return nodes.size() >= options.getSkipCount();
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
int numberFound = nodes.size();
|
||||
|
@@ -41,9 +41,13 @@ import org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter;
|
||||
import org.alfresco.service.cmr.security.PermissionService;
|
||||
import org.alfresco.service.namespace.QName;
|
||||
import org.alfresco.util.EqualsHelper;
|
||||
import org.apache.commons.logging.Log;
|
||||
import org.apache.commons.logging.LogFactory;
|
||||
|
||||
public class NodePermissionAssessor
|
||||
{
|
||||
protected static final Log logger = LogFactory.getLog(NodePermissionAssessor.class);
|
||||
|
||||
private final boolean isSystemReading;
|
||||
private final boolean isAdminReading;
|
||||
private final boolean isNullReading;
|
||||
@@ -137,25 +141,32 @@ public class NodePermissionAssessor
|
||||
}
|
||||
|
||||
public void setMaxPermissionChecks(int maxPermissionChecks)
|
||||
{
|
||||
if (maxPermissionChecks == Integer.MAX_VALUE)
|
||||
{
|
||||
this.maxPermissionChecks = maxPermissionChecks;
|
||||
}
|
||||
else
|
||||
{
|
||||
this.maxPermissionChecks = maxPermissionChecks + 1;
|
||||
}
|
||||
}
|
||||
|
||||
public boolean shouldQuitChecks()
|
||||
{
|
||||
boolean result = false;
|
||||
|
||||
if (checksPerformed >= maxPermissionChecks)
|
||||
{
|
||||
result = true;
|
||||
logger.warn("Maximum permission checks exceeded (" + maxPermissionChecks + ")");
|
||||
return true;
|
||||
}
|
||||
|
||||
if ((System.currentTimeMillis() - startTime) >= maxPermissionCheckTimeMillis)
|
||||
{
|
||||
result = true;
|
||||
logger.warn("Maximum permission checks time exceeded (" + maxPermissionCheckTimeMillis + ")");
|
||||
return true;
|
||||
}
|
||||
|
||||
return result;
|
||||
return false;
|
||||
}
|
||||
|
||||
public void setMaxPermissionCheckTimeMillis(long maxPermissionCheckTimeMillis)
|
||||
|
@@ -153,6 +153,7 @@ system.cache.parentAssocs.limitFactor=8
|
||||
system.acl.maxPermissionCheckTimeMillis=10000
|
||||
# The maximum number of search results to perform permission checks against
|
||||
system.acl.maxPermissionChecks=1000
|
||||
system.acl.maxPermissionCheckEnabled=false
|
||||
|
||||
# The maximum number of filefolder list results
|
||||
system.filefolderservice.defaultListMaxResults=5000
|
||||
|
@@ -126,6 +126,9 @@
|
||||
<property name="maxPermissionCheckTimeMillis">
|
||||
<value>${system.acl.maxPermissionCheckTimeMillis}</value>
|
||||
</property>
|
||||
<property name="maxPermissionCheckEnabled">
|
||||
<value>${system.acl.maxPermissionCheckEnabled}</value>
|
||||
</property>
|
||||
</bean>
|
||||
|
||||
<bean id="search.dbQueryEngine" class="org.springframework.aop.framework.ProxyFactoryBean">
|
||||
|
@@ -168,6 +168,19 @@ public class DBQueryEngineTest
|
||||
assertNodePresent(7, result);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldResultSetLengthMatchTheAmountOfAllAccessibleNodesWhenMaxPermissionCheckEnabled()
|
||||
{
|
||||
withMaxItems(5);
|
||||
prepareTemplate(dbQuery, createNodes(10));
|
||||
when(assessor.isIncluded(any(Node.class))).thenReturn(true);
|
||||
|
||||
engine.setMaxPermissionCheckEnabled(true);
|
||||
FilteringResultSet result = engine.acceleratedNodeSelection(options, dbQuery, assessor);
|
||||
|
||||
assertEquals(10, result.length());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldNotConsiderInaccessibleNodesInResultSetWhenSkippingNodes()
|
||||
{
|
||||
|
Reference in New Issue
Block a user