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

(cherry picked from commit cb636d1140)

repository/src/main/java/org/alfresco/repo/search/impl/querymodel/impl/db/DBQueryEngine.java

@@ -35,6 +35,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 import javax.annotation.concurrent.NotThreadSafe;
@@ -46,6 +47,7 @@ import org.alfresco.repo.cache.lookup.EntityLookupCache;
 import org.alfresco.repo.cache.lookup.EntityLookupCache.EntityLookupCallbackDAOAdaptor;
 import org.alfresco.repo.domain.node.Node;
 import org.alfresco.repo.domain.node.NodeDAO;
+import org.alfresco.repo.domain.node.StoreEntity;
 import org.alfresco.repo.domain.permissions.AclCrudDAO;
 import org.alfresco.repo.domain.permissions.Authority;
 import org.alfresco.repo.domain.qname.QNameDAO;
@@ -110,8 +112,12 @@ public class DBQueryEngine implements QueryEngine
     
     private long maxPermissionCheckTimeMillis;
 
+    private boolean maxPermissionCheckEnabled;
+
     protected EntityLookupCache<Long, Node, NodeRef> nodesCache;
 
+    private List<Pair<Long, StoreRef>> stores;
+    
     AclCrudDAO aclCrudDAO;
 
     public void setAclCrudDAO(AclCrudDAO aclCrudDAO)
@@ -129,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;
@@ -313,6 +324,9 @@ public class DBQueryEngine implements QueryEngine
 
     FilteringResultSet acceleratedNodeSelection(QueryOptions options, DBQuery dbQuery, NodePermissionAssessor permissionAssessor)
     {
+        // get list of stores from database
+        stores = nodeDAO.getStores();
+
         List<Node> nodes = new ArrayList<>();
         int requiredNodes = computeRequiredNodesCount(options);
         
@@ -322,21 +336,16 @@ 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;
                 }
                 
                 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());
@@ -348,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())
                 {
@@ -358,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();
@@ -457,4 +485,21 @@ public class DBQueryEngine implements QueryEngine
             return value.getNodeRef();
         }
     }
+
+    private void addStoreInfo(Node node)
+    {
+        StoreEntity storeEntity = node.getStore();
+        logger.debug("Adding store info for store id " + storeEntity.getId());
+        for (Pair<Long, StoreRef> storeRefPair : stores)
+        {
+            if (Objects.equals(storeEntity.getId(), storeRefPair.getFirst()))
+            {
+                StoreRef storeRef = storeRefPair.getSecond();
+                storeEntity.setIdentifier(storeRef.getIdentifier());
+                storeEntity.setProtocol(storeRef.getProtocol());
+                logger.debug("Added store info" + storeEntity.toString());
+                break;
+            }
+        }
+    }
 }

repository/src/main/java/org/alfresco/repo/search/impl/querymodel/impl/db/NodePermissionAssessor.java

@@ -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)

repository/src/main/resources/alfresco/repository.properties

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

repository/src/main/resources/alfresco/subsystems/Search/common-search-context.xml

@@ -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">

repository/src/test/java/org/alfresco/repo/search/impl/querymodel/impl/db/DBQueryEngineTest.java

@@ -159,6 +159,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()
     {