Final fix for MOB-1609: Incorrect number of results returned from audit log

- When the query size is limited, the DB query returns only the audit entries
 - The audited properties are pulled back in batches


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@16567 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

config/alfresco/ibatis/org.hibernate.dialect.Dialect/audit-common-SqlMap.xml

@@ -50,7 +50,7 @@
     <resultMap id="result_AuditQueryAllValues"
                extends="alfresco.audit.result_AuditQueryNoValues"
                class="AuditQueryResult">
-        <result property="auditValues" resultMap="alfresco.propval.result_PropertyIdSearchRow"/>
+        <result property="auditValueRows" resultMap="alfresco.propval.result_PropertyIdSearchRow"/>
     </resultMap>
   
     <!--                -->
@@ -203,4 +203,45 @@
         </dynamic>
     </select>
 
+    <!-- Get audit entries -->
+    <select id="select_AuditEntriesWithoutValues" parameterClass="AuditQueryParameters" resultMap="result_AuditQueryNoValues">
+        select
+            app.app_name_id             as audit_app_name_id,
+            entry.id                    as audit_entry_id,
+            entry.audit_user_id         as audit_user_id,
+            entry.audit_time            as audit_time,
+            entry.audit_values_id       as audit_values_id
+        from
+            alf_audit_app app
+            join alf_audit_entry entry on (entry.audit_app_id = app.id)
+            
+        <isNotNull property="searchKeyId">
+            join alf_prop_link sp_kpl on (sp_kpl.root_prop_id = entry.audit_values_id)
+        </isNotNull>
+        <isNotNull property="searchValueId">
+            join alf_prop_link sp_mpl on (sp_mpl.root_prop_id = entry.audit_values_id)
+        </isNotNull>
+        
+        <dynamic prepend="where">
+            <isNotNull prepend="and" property="auditAppNameId">
+                app.app_name_id = #auditAppNameId#
+            </isNotNull>
+            <isNotNull prepend="and" property="auditUserId">
+                entry.audit_user_id = #auditUserId#
+            </isNotNull>
+            <isNotNull prepend="and" property="auditFromTime">
+                <![CDATA[entry.audit_time >= #auditFromTime#]]>
+            </isNotNull>
+            <isNotNull prepend="and" property="auditToTime">
+                <![CDATA[entry.audit_time < #auditToTime#]]>
+            </isNotNull>
+            <isNotNull prepend="and" property="searchKeyId">
+                sp_kpl.key_prop_id = #searchKeyId#
+            </isNotNull>
+            <isNotNull prepend="and" property="searchValueId">
+                sp_mpl.value_prop_id = #searchValueId#
+            </isNotNull>
+        </dynamic>
+    </select>
+
 </sqlMap>

source/java/org/alfresco/repo/domain/audit/AbstractAuditDAOImpl.java

@@ -370,43 +370,46 @@ public abstract class AbstractAuditDAOImpl implements AuditDAO
                 // No more results required
                 return;
             }
-            // Get the value map
+            // See if the value is available or if it has to be built up
-            Map<String, Serializable> auditValues;
+            Map<String, Serializable> auditValues = row.getAuditValue();
-            List<PropertyIdSearchRow> propMapRows = row.getAuditValues();
+            if (auditValues == null)
-            if (propMapRows == null)
             {
-                // Use the audit values ID
+                List<PropertyIdSearchRow> propMapRows = row.getAuditValueRows();
-                Long auditValuesId = row.getAuditValuesId();
+                if (propMapRows == null)
-                Pair<Long, Serializable> auditValuesPair = propertyValueDAO.getPropertyValueById(auditValuesId);
-                if (auditValuesPair == null)
                 {
-                    // Ignore
+                    // Use the audit values ID
-                    logger.warn("Audit entry not joined to audit properties: " + row);
+                    Long auditValuesId = row.getAuditValuesId();
-                    return;
+                    Pair<Long, Serializable> auditValuesPair = propertyValueDAO.getPropertyValueById(auditValuesId);
+                    if (auditValuesPair == null)
+                    {
+                        // Ignore
+                        logger.warn("Audit entry not joined to audit properties: " + row);
+                        return;
+                    }
+                    auditValues = (Map<String, Serializable>) auditValuesPair.getSecond();
                 }
-                auditValues = (Map<String, Serializable>) auditValuesPair.getSecond();
+                else
-            }
-            else
-            {
-                // Resolve the map
-                try
                 {
-                    auditValues = (Map<String, Serializable>) propertyValueDAO.convertPropertyIdSearchRows(propMapRows);
+                    // Resolve the map
-                }
+                    try
-                catch (ClassCastException e)
+                    {
-                {
+                        auditValues = (Map<String, Serializable>) propertyValueDAO.convertPropertyIdSearchRows(propMapRows);
-                    logger.warn("Audit entry not linked to a Map<String, Serializable> value: " + row);
+                    }
-                    return;
+                    catch (ClassCastException e)
-                }
+                    {
-                catch (Throwable e)
+                        logger.warn("Audit entry not linked to a Map<String, Serializable> value: " + row);
-                {
+                        return;
-                    logger.warn("Audit entry unable to extract audited values: " + row, e);
+                    }
-                    return;
+                    catch (Throwable e)
-                }
+                    {
-                if (auditValues == null)
+                        logger.warn("Audit entry unable to extract audited values: " + row, e);
-                {
+                        return;
-                    logger.warn("Audit entry incompletely joined to audit properties: " + row);
+                    }
-                    return;
+                    if (auditValues == null)
+                    {
+                        logger.warn("Audit entry incompletely joined to audit properties: " + row);
+                        return;
+                    }
                 }
             }
             // Resolve the application and username

source/java/org/alfresco/repo/domain/audit/AuditQueryResult.java

@@ -24,8 +24,10 @@
  */
 package org.alfresco.repo.domain.audit;
 
+import java.io.Serializable;
 import java.util.Date;
 import java.util.List;
+import java.util.Map;
 
 import org.alfresco.repo.domain.propval.PropertyIdSearchRow;
 
@@ -42,7 +44,8 @@ public class AuditQueryResult
     private Long auditUserId;
     private long auditTime;
     private Long auditValuesId;
-    private List<PropertyIdSearchRow> auditValues;
+    private List<PropertyIdSearchRow> auditValueRows;
+    private Map<String, Serializable> auditValue;
     
     public AuditQueryResult()
     {
@@ -58,7 +61,8 @@ public class AuditQueryResult
           .append(", auditUserId=").append(auditUserId)
           .append(", auditTime").append(new Date(auditTime))
           .append(", auditValuesId=").append(auditValuesId)
-          .append(", auditValues=").append(auditValues.size())
+          .append(", auditValueRows=").append(auditValueRows == null ? null : auditValueRows.size())
+          .append(", auditValue=").append(auditValue)
           .append("]");
         return sb.toString();
     }
@@ -113,13 +117,23 @@ public class AuditQueryResult
         this.auditValuesId = auditValuesId;
     }
 
-    public List<PropertyIdSearchRow> getAuditValues()
+    public List<PropertyIdSearchRow> getAuditValueRows()
     {
-        return auditValues;
+        return auditValueRows;
     }
 
-    public void setAuditValues(List<PropertyIdSearchRow> auditValues)
+    public void setAuditValueRows(List<PropertyIdSearchRow> auditValueRows)
     {
-        this.auditValues = auditValues;
+        this.auditValueRows = auditValueRows;
+    }
+
+    public Map<String, Serializable> getAuditValue()
+    {
+        return auditValue;
+    }
+
+    public void setAuditValue(Map<String, Serializable> auditValue)
+    {
+        this.auditValue = auditValue;
     }
 }

source/java/org/alfresco/repo/domain/audit/ibatis/AuditDAOImpl.java

@@ -25,9 +25,11 @@
 package org.alfresco.repo.domain.audit.ibatis;
 
 import java.io.Serializable;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.TreeMap;
 
 import org.alfresco.ibatis.RollupRowHandler;
 import org.alfresco.repo.domain.audit.AbstractAuditDAOImpl;
@@ -37,6 +39,7 @@ import org.alfresco.repo.domain.audit.AuditEntryEntity;
 import org.alfresco.repo.domain.audit.AuditModelEntity;
 import org.alfresco.repo.domain.audit.AuditQueryParameters;
 import org.alfresco.repo.domain.audit.AuditQueryResult;
+import org.alfresco.repo.domain.propval.PropertyValueDAO.PropertyFinderCallback;
 import org.alfresco.util.Pair;
 import org.springframework.orm.ibatis.SqlMapClientTemplate;
 
@@ -64,6 +67,7 @@ public class AuditDAOImpl extends AbstractAuditDAOImpl
     @SuppressWarnings("unused")
     private static final String SELECT_ENTRIES_SIMPLE = "alfresco.audit.select_AuditEntriesSimple";
     private static final String SELECT_ENTRIES_WITH_VALUES = "alfresco.audit.select_AuditEntriesWithValues";
+    private static final String SELECT_ENTRIES_WITHOUT_VALUES = "alfresco.audit.select_AuditEntriesWithoutValues";
     
     private SqlMapClientTemplate template;
 
@@ -241,36 +245,66 @@ public class AuditDAOImpl extends AbstractAuditDAOImpl
             params.setSearchValueId(searchValuePair.getFirst());
         }
         
-        // RowHandlers in RowHandlers: See 'groupBy' issue https://issues.apache.org/jira/browse/IBATIS-503
-        RowHandler shreddedRowHandler = new RowHandler()
-        {
-            public void handleRow(Object valueObject)
-            {
-                rowHandler.processResult((AuditQueryResult)valueObject);
-            }
-        };
-        RollupRowHandler rollupRowHandler = new RollupRowHandler(
-                new String[] {"auditEntryId"},
-                "auditValues",
-                shreddedRowHandler,
-                maxResults);
-        
         if (maxResults > 0)
         {
-            // Calculate the maximum results required
+            // Query without getting the values.  We gather all the results and batch-fetch the audited
-            int sqlMaxResults = (maxResults > 0 ? ((maxResults+1) * 100) : Integer.MAX_VALUE);
+            // values afterwards.
-            
+            final TreeMap<Long, AuditQueryResult> resultsByValueId = new TreeMap<Long, AuditQueryResult>();
-            List<AuditQueryResult> rows = template.queryForList(SELECT_ENTRIES_WITH_VALUES, params, 0, sqlMaxResults);
+            PropertyFinderCallback propertyFinderCallback = new PropertyFinderCallback()
+            {
+                public void handleProperty(Long id, Serializable value)
+                {
+                    // get the row
+                    AuditQueryResult row = resultsByValueId.get(id);
+                    try
+                    {
+                        row.setAuditValue((Map<String, Serializable>) value);
+                    }
+                    catch (ClassCastException e)
+                    {
+                        // The handler will deal with the entry
+                    }
+                    rowHandler.processResult(row);
+                }
+            };
+
+            List<AuditQueryResult> rows = template.queryForList(SELECT_ENTRIES_WITHOUT_VALUES, params, 0, maxResults);
             for (AuditQueryResult row : rows)
             {
-                rollupRowHandler.handleRow(row);
+                resultsByValueId.put(row.getAuditValuesId(), row);
+                if (resultsByValueId.size() >= 100)
+                {
+                    // Fetch values for the results.  The treemap is ordered.
+                    List<Long> valueIds = new ArrayList<Long>(resultsByValueId.keySet());
+                    propertyValueDAO.getPropertiesByIds(valueIds, propertyFinderCallback);
+                    // Clear and continue
+                    resultsByValueId.clear();
+                }
+            }
+            // Process any remaining results
+            if (resultsByValueId.size() > 0)
+            {
+                // Fetch values for the results.  The treemap is ordered.
+                List<Long> valueIds = new ArrayList<Long>(resultsByValueId.keySet());
+                propertyValueDAO.getPropertiesByIds(valueIds, propertyFinderCallback);
             }
-            // Don't process last result:
-            //    rollupRowHandler.processLastResults();
-            //    The last result may be incomplete
         }
         else
         {
+            // RowHandlers in RowHandlers: See 'groupBy' issue https://issues.apache.org/jira/browse/IBATIS-503
+            RowHandler queryRowHandler = new RowHandler()
+            {
+                public void handleRow(Object valueObject)
+                {
+                    rowHandler.processResult((AuditQueryResult)valueObject);
+                }
+            };
+            RollupRowHandler rollupRowHandler = new RollupRowHandler(
+                    new String[] {"auditEntryId"},
+                    "auditValueRows",
+                    queryRowHandler,
+                    maxResults);
+            
             template.queryWithRowHandler(SELECT_ENTRIES_WITH_VALUES, params, rollupRowHandler);
             rollupRowHandler.processLastResults();
         }