ALF-4106: AuditService and audit DAO refactor

- Added 'dataSource' attribute to 'RecordValue': <RecordValue ... dataSource='...'/>
 - This doesn't affect any existing configurations as the 'dataSource' remains the current path
 - Process data extraction by DataExtractor rather than by path (simpler)
 - Added unit tests specific to 'dataSource' attribute


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

config/alfresco/audit/alfresco-audit-3.2.xsd

@@ -86,6 +86,7 @@
         <xs:complexContent>
             <xs:extension base="a:KeyedAuditDefinition">
                 <xs:attribute name="dataExtractor" type="a:NameAttribute" use="required" />
+                <xs:attribute name="dataSource" type="a:PathAttribute" use="optional" />
             </xs:extension>
         </xs:complexContent>
     </xs:complexType>

config/alfresco/audit/alfresco-audit-repository.xml

@@ -23,18 +23,18 @@
 
     <Application name="AlfrescoRepository" key="repository">
         <AuditPath key="login">
+            <!--
             <AuditPath key="args">
                 <AuditPath key="userName">
                     <RecordValue key="value" dataExtractor="simpleValue"/>
                 </AuditPath>
             </AuditPath>
-            <!--
             <AuditPath key="no-error">
                 <GenerateValue key="fullName" dataGenerator="personFullName"/>
             </AuditPath>
             -->
             <AuditPath key="error">
-                <RecordValue key="value" dataExtractor="nullValue"/>
+                <RecordValue key="user" dataExtractor="simpleValue" dataSource="/repository/login/args/userName"/>
             </AuditPath>
         </AuditPath>
     </Application>

source/java/org/alfresco/repo/audit/AuditBootstrapTest.java

@@ -19,15 +19,16 @@
 package org.alfresco.repo.audit;
 
 import java.net.URL;
+import java.util.List;
 import java.util.Map;
 
 import junit.framework.TestCase;
 
-import org.alfresco.repo.audit.extractor.DataExtractor;
 import org.alfresco.repo.audit.generator.DataGenerator;
 import org.alfresco.repo.audit.model.AuditApplication;
 import org.alfresco.repo.audit.model.AuditModelException;
 import org.alfresco.repo.audit.model.AuditModelRegistryImpl;
+import org.alfresco.repo.audit.model.AuditApplication.DataExtractorDefinition;
 import org.alfresco.util.ApplicationContextHelper;
 import org.alfresco.util.PathMapper;
 import org.apache.commons.logging.Log;
@@ -188,17 +189,9 @@ public class AuditBootstrapTest extends TestCase
         AuditApplication app = auditModelRegistry.getAuditApplicationByName(APPLICATION_TEST);
         assertNotNull(app);
         
-        Map<String, DataExtractor> extractors = app.getDataExtractors("/blah");
+        List<DataExtractorDefinition> extractors = app.getDataExtractors();
-        assertNotNull("Should never get a null map", extractors);
+        assertNotNull("Should never get a null list", extractors);
-        assertTrue("Expected no extractors", extractors.isEmpty());
+        assertEquals("Expected 13 extractors", 13, extractors.size());
-
-        extractors = app.getDataExtractors("/test/1.1/2.1/3.1/4.1");
-        assertEquals(1, extractors.size());
-        assertTrue(extractors.containsKey("/test/1.1/2.1/3.1/4.1/value.1"));
-
-        extractors = app.getDataExtractors("/test/1.1/2.1/3.1");
-        assertEquals(1, extractors.size());
-        assertTrue(extractors.containsKey("/test/1.1/2.1/3.1/value.1"));
     }
     
     public void testAuditApplication_GetDataGenerators()

source/java/org/alfresco/repo/audit/AuditComponentImpl.java

@@ -23,6 +23,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -32,6 +33,7 @@ import org.alfresco.repo.audit.generator.DataGenerator;
 import org.alfresco.repo.audit.model.AuditApplication;
 import org.alfresco.repo.audit.model.AuditModelRegistry;
 import org.alfresco.repo.audit.model.AuditModelRegistryImpl;
+import org.alfresco.repo.audit.model.AuditApplication.DataExtractorDefinition;
 import org.alfresco.repo.domain.audit.AuditDAO;
 import org.alfresco.repo.domain.propval.PropertyValueDAO;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
@@ -561,21 +563,7 @@ public class AuditComponentImpl implements AuditComponent
             throw new AuditException("No persisted instance exists for audit application: " + application);
         }
 
-        // Eliminate any paths that have been disabled
+        // Check if there is anything to audit
-        Iterator<String> pathedValuesKeyIterator = values.keySet().iterator();
-        while(pathedValuesKeyIterator.hasNext())
-        {
-            String pathedValueKey = pathedValuesKeyIterator.next();
-            for (String disabledPath : disabledPaths)
-            {
-                if (pathedValueKey.startsWith(disabledPath))
-                {
-                    // The pathed value is excluded
-                    pathedValuesKeyIterator.remove();
-                }
-            }
-        }
-        // Check if there is anything left
         if (values.size() == 0)
         {
             if (logger.isDebugEnabled())
@@ -588,8 +576,24 @@ public class AuditComponentImpl implements AuditComponent
             return Collections.emptyMap();
         }
 
+        Set<String> generatorKeys = values.keySet();
+        // Eliminate any paths that have been disabled
+        Iterator<String> generatorKeysIterator = generatorKeys.iterator();
+        while(generatorKeysIterator.hasNext())
+        {
+            String generatorKey = generatorKeysIterator.next();
+            for (String disabledPath : disabledPaths)
+            {
+                if (generatorKey.startsWith(disabledPath))
+                {
+                    // The pathed value is excluded
+                    generatorKeysIterator.remove();
+                }
+            }
+        }
+        
         // Generate data
-        Map<String, DataGenerator> generators = application.getDataGenerators(values.keySet());
+        Map<String, DataGenerator> generators = application.getDataGenerators(generatorKeys);
         Map<String, Serializable> auditData = generateData(generators);
         
         // Now extract values
@@ -641,40 +645,45 @@ public class AuditComponentImpl implements AuditComponent
             AuditApplication application,
             Map<String, Serializable> values)
     {
-        Map<String, Serializable> newData = new HashMap<String, Serializable>(values.size() + 5);
+        Map<String, Serializable> newData = new HashMap<String, Serializable>(values.size());
-        for (Map.Entry<String, Serializable> entry : values.entrySet())
+        
+        List<DataExtractorDefinition> extractors = application.getDataExtractors();
+        for (DataExtractorDefinition extractorDef : extractors)
         {
-            String path = entry.getKey();
+            DataExtractor extractor = extractorDef.getDataExtractor();
-            Serializable value = entry.getValue();
+            String sourcePath = extractorDef.getDataSource();
-            // Get the applicable extractor
+            String targetPath = extractorDef.getDataTarget();
-            Map<String, DataExtractor> extractors = application.getDataExtractors(path);
+            
-            for (Map.Entry<String, DataExtractor> extractorElement : extractors.entrySet())
+            // We observe the key, not the actual value
+            if (!values.containsKey(sourcePath))
             {
-                String extractorPath = extractorElement.getKey();
+                continue;               // There is no data to extract
-                DataExtractor extractor = extractorElement.getValue();
-                // Check if the extraction is supported
-                if (!extractor.isSupported(value))
-                {
-                    continue;
-                }
-                // Use the extractor to pull the value out
-                final Serializable data;
-                try
-                {
-                    data = extractor.extractData(value);
-                }
-                catch (Throwable e)
-                {
-                    throw new AlfrescoRuntimeException(
-                            "Failed to extract audit data: \n" +
-                            "   Path:      " + path + "\n" +
-                            "   Raw value: " + value + "\n" +
-                            "   Extractor: " + extractor,
-                            e);
-                }
-                // Add it to the map
-                newData.put(extractorPath, data);
             }
+            
+            Serializable value = values.get(sourcePath);
+            
+            // Check if the extraction is supported
+            if (!extractor.isSupported(value))
+            {
+                continue;
+            }
+            // Use the extractor to pull the value out
+            final Serializable data;
+            try
+            {
+                data = extractor.extractData(value);
+            }
+            catch (Throwable e)
+            {
+                throw new AlfrescoRuntimeException(
+                        "Failed to extract audit data: \n" +
+                        "   Path:      " + sourcePath + "\n" +
+                        "   Raw value: " + value + "\n" +
+                        "   Extractor: " + extractor,
+                        e);
+            }
+            // Add it to the map
+            newData.put(targetPath, data);
         }
         // Done
         if (logger.isDebugEnabled())
@@ -682,7 +691,7 @@ public class AuditComponentImpl implements AuditComponent
             logger.debug("Extracted audit data: \n" +
                     "   Application: " + application + "\n" +
                     "   Raw values:  " + values + "\n" +
-                    "   Extracted: " + newData);
+                    "   Extracted:   " + newData);
         }
         return newData;
     }

source/java/org/alfresco/repo/audit/AuditComponentTest.java

@@ -253,7 +253,7 @@ public class AuditComponentTest extends TestCase
         Serializable valueA = new Date();
         Serializable valueB = "BBB-value-here";
         Serializable valueC = new Float(16.0F);
-        // Get a noderef
+        
         final Map<String, Serializable> parameters = new HashMap<String, Serializable>(13);
         parameters.put("A", valueA);
         parameters.put("B", valueB);
@@ -292,6 +292,45 @@ public class AuditComponentTest extends TestCase
         auditAction01("action-01-mapped");
     }
     
+    /**
+     * Test auditing of something resembling real-world data
+     */
+    private void auditAction02(String actionName) throws Exception
+    {
+        Serializable valueA = new Date();
+        Serializable valueB = "BBB-value-here";
+        Serializable valueC = new Float(16.0F);
+        
+        final Map<String, Serializable> parameters = new HashMap<String, Serializable>(13);
+        parameters.put("A", valueA);
+        parameters.put("B", valueB);
+        parameters.put("C", valueC);
+        // lowercase versions are not in the config
+        parameters.put("a", valueA);
+        parameters.put("b", valueB);
+        parameters.put("c", valueC);
+        
+        Map<String, Serializable> result = auditTestAction(actionName, nodeRef, parameters);
+        
+        Map<String, Serializable> expected = new HashMap<String, Serializable>();
+        expected.put("/actions-test/actions/user", AuthenticationUtil.getFullyAuthenticatedUser());
+        expected.put("/actions-test/actions/context-node/noderef", nodeRef);
+        expected.put("/actions-test/actions/action-02/valueA", valueA);
+        expected.put("/actions-test/actions/action-02/valueB", valueB);
+        expected.put("/actions-test/actions/action-02/valueC", valueC);
+        
+        // Check
+        checkAuditMaps(result, expected);
+    }
+    
+    /**
+     * Test auditing using alternative data sources
+     */
+    public void testAudit_Action02Sourced() throws Exception
+    {
+        auditAction02("action-02-sourced");
+    }
+    
     public void testQuery_Action01() throws Exception
     {
         final Long beforeTime = new Long(System.currentTimeMillis());

source/java/org/alfresco/repo/audit/model/AuditApplication.java

@@ -18,9 +18,11 @@
  */
 package org.alfresco.repo.audit.model;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -60,7 +62,7 @@ public class AuditApplication
     private final Long disabledPathsId;
 
     /** Derived expaned map for fast lookup */
-    private Map<String, Map<String, DataExtractor>> dataExtractors = new HashMap<String, Map<String, DataExtractor>>(11);
+    private List<DataExtractorDefinition> dataExtractors = new ArrayList<DataExtractorDefinition>();
     /** Derived expaned map for fast lookup */
     private Map<String, Map<String, DataGenerator>> dataGenerators = new HashMap<String, Map<String, DataGenerator>>(11);
     
@@ -281,31 +283,59 @@ public class AuditApplication
     }
     
     /**
-     * Get all data extractors applicable to a given path and scope.
+     * Utility class carrying information around a {@link DataExtractor}.
      * 
-     * @param path              the audit path
+     * @author Derek Hulley
-     * @return                  Returns all data extractors mapped to their key-path
+     * @since 3.4
      */
-    public Map<String, DataExtractor> getDataExtractors(String path)
+    public static class DataExtractorDefinition
     {
-        Map<String, DataExtractor> extractors = dataExtractors.get(path);
+        private final String dataSource;
-        if (extractors == null)
+        private final String dataTarget;
+        private final DataExtractor dataExtractor;
+
+        /**
+         * @param dataSource            the path to get data from
+         * @param dataTarget            the path to write data to
+         * @param dataExtractor         the implementation to use
+         */
+        public DataExtractorDefinition(String dataSource, String dataTarget, DataExtractor dataExtractor)
         {
-            // Don't give back a null
+            this.dataSource = dataSource;
-            extractors = Collections.emptyMap();
+            this.dataTarget = dataTarget;
+            this.dataExtractor = dataExtractor;
         }
-        else
+
+        public String getDataSource()
         {
-            // we don't want to give back a modifiable map
+            return dataSource;
-            extractors = Collections.unmodifiableMap(extractors);
         }
 
+        public String getDataTarget()
+        {
+            return dataTarget;
+        }
+
+        public DataExtractor getDataExtractor()
+        {
+            return dataExtractor;
+        }
+    }
+    
+    /**
+     * Get all data extractors applicable to this application.
+     * 
+     * @return                  Returns all data extractors contained in the application
+     */
+    public List<DataExtractorDefinition> getDataExtractors()
+    {
+        List<DataExtractorDefinition> extractors = Collections.unmodifiableList(dataExtractors);
+        
         // Done
         if (logger.isDebugEnabled())
         {
             logger.debug(
                     "Looked up data extractors: \n" +
-                    "   Path:  " + path + "\n" +
                     "   Found: " + extractors);
         }
         return extractors;
@@ -361,7 +391,6 @@ public class AuditApplication
                 auditPath,
                 null,
                 new HashSet<String>(37),
-                new HashMap<String, DataExtractor>(13),
                 new HashMap<String, DataGenerator>(13));
     }
     /**
@@ -371,11 +400,9 @@ public class AuditApplication
             AuditPath auditPath,
             String currentPath,
             Set<String> existingPaths,
-            Map<String, DataExtractor> upperExtractorsByPath,
             Map<String, DataGenerator> upperGeneratorsByPath)
     {
         // Clone the upper maps to prevent pollution
-        upperExtractorsByPath = new HashMap<String, DataExtractor>(upperExtractorsByPath);
         upperGeneratorsByPath = new HashMap<String, DataGenerator>(upperGeneratorsByPath);
         
         // Append the current audit path to the current path
@@ -409,16 +436,16 @@ public class AuditApplication
             {
                 generateException(extractorPath, "No data extractor exists for name: " + extractorName);
             }
-            // All generators that occur earlier in the path will also be applicable here
+            // The extractor may pull data from somewhere else
-            upperExtractorsByPath.put(extractorPath, extractor);
+            String sourcePath = element.getDataSource();
+            if (sourcePath == null)
+            {
+                sourcePath = currentPath;
+            }
+            // Store the extractor definition
+            DataExtractorDefinition extractorDef = new DataExtractorDefinition(sourcePath, extractorPath, extractor);
+            dataExtractors.add(extractorDef);
         }
-        // All the extractors apply to the current path
-        dataExtractors.put(currentPath, upperExtractorsByPath);
-        // Data extractors only apply directly to data in which they appear.
-        //    TODO: Examine this assumption.  If it is not true, i.e. data extractors apply to
-        //          data anywhere down the hierarchy, then the followin line of code should be
-        //          removed and the use-cases tested appropriately.
-        upperExtractorsByPath = new HashMap<String, DataExtractor>();
 
         // Get the data generators declared for this key
         for (GenerateValue element : auditPath.getGenerateValue())
@@ -445,7 +472,7 @@ public class AuditApplication
         // Find all sub audit paths and recurse
         for (AuditPath element : auditPath.getAuditPath())
         {
-            buildAuditPaths(element, currentPath, existingPaths, upperExtractorsByPath, upperGeneratorsByPath);
+            buildAuditPaths(element, currentPath, existingPaths, upperGeneratorsByPath);
         }
     }
     

source/java/org/alfresco/repo/audit/model/_3/KeyedAuditDefinition.java

@@ -45,9 +45,9 @@ import javax.xml.bind.annotation.XmlType;
 @XmlAccessorType(XmlAccessType.FIELD)
 @XmlType(name = "KeyedAuditDefinition")
 @XmlSeeAlso({
-    GenerateValue.class,
+    RecordValue.class,
     AuditPath.class,
-    RecordValue.class
+    GenerateValue.class
 })
 public class KeyedAuditDefinition {
 

source/java/org/alfresco/repo/audit/model/_3/ObjectFactory.java

@@ -59,11 +59,19 @@ public class ObjectFactory {
     }
 
     /**
-     * Create an instance of {@link DataGenerator }
+     * Create an instance of {@link RecordValue }
      * 
      */
-    public DataGenerator createDataGenerator() {
+    public RecordValue createRecordValue() {
-        return new DataGenerator();
+        return new RecordValue();
+    }
+
+    /**
+     * Create an instance of {@link PathMap }
+     * 
+     */
+    public PathMap createPathMap() {
+        return new PathMap();
     }
 
     /**
@@ -74,14 +82,6 @@ public class ObjectFactory {
         return new AuditPath();
     }
 
-    /**
-     * Create an instance of {@link KeyedAuditDefinition }
-     * 
-     */
-    public KeyedAuditDefinition createKeyedAuditDefinition() {
-        return new KeyedAuditDefinition();
-    }
-
     /**
      * Create an instance of {@link GenerateValue }
      * 
@@ -90,30 +90,6 @@ public class ObjectFactory {
         return new GenerateValue();
     }
 
-    /**
-     * Create an instance of {@link DataGenerators }
-     * 
-     */
-    public DataGenerators createDataGenerators() {
-        return new DataGenerators();
-    }
-
-    /**
-     * Create an instance of {@link DataExtractors }
-     * 
-     */
-    public DataExtractors createDataExtractors() {
-        return new DataExtractors();
-    }
-
-    /**
-     * Create an instance of {@link RecordValue }
-     * 
-     */
-    public RecordValue createRecordValue() {
-        return new RecordValue();
-    }
-
     /**
      * Create an instance of {@link Application }
      * 
@@ -123,11 +99,27 @@ public class ObjectFactory {
     }
 
     /**
-     * Create an instance of {@link DataExtractor }
+     * Create an instance of {@link KeyedAuditDefinition }
      * 
      */
-    public DataExtractor createDataExtractor() {
+    public KeyedAuditDefinition createKeyedAuditDefinition() {
-        return new DataExtractor();
+        return new KeyedAuditDefinition();
+    }
+
+    /**
+     * Create an instance of {@link DataExtractors }
+     * 
+     */
+    public DataExtractors createDataExtractors() {
+        return new DataExtractors();
+    }
+
+    /**
+     * Create an instance of {@link DataGenerator }
+     * 
+     */
+    public DataGenerator createDataGenerator() {
+        return new DataGenerator();
     }
 
     /**
@@ -139,11 +131,19 @@ public class ObjectFactory {
     }
 
     /**
-     * Create an instance of {@link PathMap }
+     * Create an instance of {@link DataExtractor }
      * 
      */
-    public PathMap createPathMap() {
+    public DataExtractor createDataExtractor() {
-        return new PathMap();
+        return new DataExtractor();
+    }
+
+    /**
+     * Create an instance of {@link DataGenerators }
+     * 
+     */
+    public DataGenerators createDataGenerators() {
+        return new DataGenerators();
     }
 
     /**

source/java/org/alfresco/repo/audit/model/_3/RecordValue.java

@@ -34,6 +34,7 @@ import javax.xml.bind.annotation.XmlType;
  *   <complexContent>
  *     <extension base="{http://www.alfresco.org/repo/audit/model/3.2}KeyedAuditDefinition">
  *       <attribute name="dataExtractor" use="required" type="{http://www.alfresco.org/repo/audit/model/3.2}NameAttribute" />
+ *       <attribute name="dataSource" type="{http://www.alfresco.org/repo/audit/model/3.2}PathAttribute" />
  *     </extension>
  *   </complexContent>
  * </complexType>
@@ -49,6 +50,8 @@ public class RecordValue
 
     @XmlAttribute(required = true)
     protected String dataExtractor;
+    @XmlAttribute
+    protected String dataSource;
 
     /**
      * Gets the value of the dataExtractor property.
@@ -74,4 +77,28 @@ public class RecordValue
         this.dataExtractor = value;
     }
 
+    /**
+     * Gets the value of the dataSource property.
+     * 
+     * @return
+     *     possible object is
+     *     {@link String }
+     *     
+     */
+    public String getDataSource() {
+        return dataSource;
+    }
+
+    /**
+     * Sets the value of the dataSource property.
+     * 
+     * @param value
+     *     allowed object is
+     *     {@link String }
+     *     
+     */
+    public void setDataSource(String value) {
+        this.dataSource = value;
+    }
+
 }

source/test-resources/alfresco/testaudit/alfresco-audit-test.xml

@@ -24,6 +24,7 @@
         <PathMap source="/test/one.one/two.two" target="/test/1.1/2.2"/>
         <PathMap source="/actions-test" target="/actions-test"/>
         <PathMap source="/actions-test/actions/action-01-mapped" target="/actions-test/actions/action-01"/>
+        <PathMap source="/actions-test/actions/action-02-sourced" target="/actions-test/actions/action-02"/>
     </PathMappings>
 
     <Application name="Alfresco Test" key="test">
@@ -101,6 +102,11 @@
                     </AuditPath>
                 </AuditPath>
             </AuditPath>
+            <AuditPath key="action-02">
+                <RecordValue key="valueA" dataExtractor="simpleValue" dataSource="/actions-test/actions/action-02/params/A"/>
+                <RecordValue key="valueB" dataExtractor="simpleValue" dataSource="/actions-test/actions/action-02/params/B"/>
+                <RecordValue key="valueC" dataExtractor="simpleValue" dataSource="/actions-test/actions/action-02/params/C"/>
+            </AuditPath>
         </AuditPath>
     </Application>