Merged HEAD-BUG-FIX (4.3/Cloud) to HEAD (4.3/Cloud)

64785: Merged V4.2-BUG-FIX (4.2.2) to HEAD-BUG-FIX (4.3/Cloud)
      64131: Merged DEV to V4.2-BUG-FIX (4.2.2)
         63233: MNT-10767: Guard in AuditMethodInterceptor is too restrictive preventing subordinate service calls from producing data.
          - Modify AuditMethodInterceptor to audit subordinate public services 
         64123: MNT-10767: Guard in AuditMethodInterceptor is too restrictive preventing subordinate service calls from producing data.
          - Add unit test 


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

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

@@ -187,7 +187,7 @@ public class AuditMethodInterceptor implements MethodInterceptor
         try
         {
             // If we are already in a nested audit call, there is nothing to do
-            if (wasInAudit != null)
+            if (Boolean.TRUE.equals(wasInAudit))
             {
                 return mi.proceed();
             }
@@ -211,8 +211,6 @@ public class AuditMethodInterceptor implements MethodInterceptor
             }
             String methodName = mi.getMethod().getName();
             
-            // Record that we have entered an audit method
-            inAudit.set(Boolean.TRUE);
             return proceedWithAudit(mi, auditableDef, serviceName, methodName, namedArguments);
         }
         finally
@@ -228,7 +226,10 @@ public class AuditMethodInterceptor implements MethodInterceptor
             String methodName,
             Map<String, Serializable> namedArguments) throws Throwable
     {
+        Boolean wasInAudit = inAudit.get();
         Map<String, Serializable> preAuditedData = null;
+        // Record that we have entered an audit method
+        inAudit.set(Boolean.TRUE);
         try
         {
             preAuditedData = auditInvocationBefore(serviceName, methodName, namedArguments);
@@ -241,7 +242,10 @@ public class AuditMethodInterceptor implements MethodInterceptor
                     "   Invocation: " + mi,
                     e);
         }
-        
+        finally
+        {
+            inAudit.set(wasInAudit);
+        }        
         // Execute the call
         Object ret = null;
         Throwable thrown = null;
@@ -256,6 +260,8 @@ public class AuditMethodInterceptor implements MethodInterceptor
         
         // We don't ALWAYS want to record the return value
         Object auditRet = auditableDef.recordReturnedObject() ? ret : null;
+        // Record that we have entered an audit method
+        inAudit.set(Boolean.TRUE);
         try
         {
             auditInvocationAfter(serviceName, methodName, namedArguments, auditRet, thrown, preAuditedData);
@@ -268,7 +274,10 @@ public class AuditMethodInterceptor implements MethodInterceptor
                     "   Invocation: " + mi,
                     e);
         }
-        
+        finally
+        {
+            inAudit.set(wasInAudit);
+        }        
         // Done
         if (thrown != null)
         {

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

@@ -31,6 +31,7 @@ import java.util.Map;
 import junit.framework.TestCase;
 
 import org.alfresco.error.AlfrescoRuntimeException;
+import org.alfresco.model.ContentModel;
 import org.alfresco.repo.audit.model.AuditApplication;
 import org.alfresco.repo.audit.model.AuditModelException;
 import org.alfresco.repo.audit.model.AuditModelRegistryImpl;
@@ -48,6 +49,8 @@ import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.repository.StoreRef;
 import org.alfresco.service.cmr.security.MutableAuthenticationService;
+import org.alfresco.service.namespace.NamespaceService;
+import org.alfresco.service.namespace.QName;
 import org.alfresco.service.transaction.TransactionService;
 import org.alfresco.test_category.OwnJVMTestsCategory;
 import org.alfresco.util.ApplicationContextHelper;
@@ -75,10 +78,11 @@ public class AuditComponentTest extends TestCase
     private static final String APPLICATION_ACTIONS_TEST = "Actions Test";
     private static final String APPLICATION_API_TEST = "Test AuthenticationService";
     private static final String APPLICATION_ALF12638_TEST = "Test ALF-12638";
+    private static final String APPLICATION_MNT10767_TEST = "Test MNT-10767";
     
     private static final Log logger = LogFactory.getLog(AuditComponentTest.class);
     
-    private static ApplicationContext ctx = ApplicationContextHelper.getApplicationContext();
+    private static ApplicationContext ctx = ApplicationContextHelper.getApplicationContext(new String[] {ApplicationContextHelper.CONFIG_LOCATIONS[0], "classpath:alfresco/testaudit/alfresco-audit-test-mnt-10767-context.xml"});
     
     private AuditModelRegistryImpl auditModelRegistry;
     private AuditComponent auditComponent;
@@ -787,6 +791,94 @@ public class AuditComponentTest extends TestCase
         assertTrue("There should be exactly one audit entry for the API test", success);
     }
 
+   /**
+     * See <a href="https://issues.alfresco.com/jira/browse/MNT-10767">MNT-10767</a>
+     */
+    public void testAuditSubordinateCall() throws Exception
+    {
+        AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getSystemUserName());
+
+        AuditQueryParameters params = new AuditQueryParameters();
+        params.setForward(true);
+        params.setApplicationName(APPLICATION_MNT10767_TEST);
+
+        // Load in the config for this specific test: alfresco-audit-test-mnt-10767
+        URL testModelUrl = ResourceUtils.getURL("classpath:alfresco/testaudit/alfresco-audit-test-mnt-10767.xml");
+        auditModelRegistry.registerModel(testModelUrl);
+        auditModelRegistry.loadAuditModels();
+
+        // There should be a log entry for the application
+        final List<Long> results = new ArrayList<Long>(5);
+        final StringBuilder sb = new StringBuilder();
+        AuditQueryCallback auditQueryCallback = new AuditQueryCallback()
+        {
+            public boolean valuesRequired()
+            {
+                return true;
+            }
+
+            public boolean handleAuditEntry(Long entryId, String applicationName, String user, long time, Map<String, Serializable> values)
+            {
+                results.add(entryId);
+                sb.append("Row: ").append(entryId).append(" | ").append(applicationName).append(" | ").append(user).append(" | ").append(new Date(time)).append(" | ").append(
+                        values).append(" | ").append("\n");
+                ;
+                return true;
+            }
+
+            public boolean handleAuditEntryError(Long entryId, String errorMsg, Throwable error)
+            {
+                throw new AlfrescoRuntimeException(errorMsg, error);
+            }
+        };
+
+        clearAuditLog(APPLICATION_MNT10767_TEST);
+        results.clear();
+        sb.delete(0, sb.length());
+        queryAuditLog(auditQueryCallback, params, -1);
+        assertTrue("There should be no audit entries for the API test after a clear", results.isEmpty());
+
+        TestCreateFileFolderService testCreateFileFolderService = (TestCreateFileFolderService) ctx.getBean("TestCreateFileFolderService");
+        NodeRef workingRootNodeRef = null;
+        try
+        {
+            workingRootNodeRef = nodeService.createNode(nodeRef, ContentModel.ASSOC_CHILDREN,
+                    QName.createQName(NamespaceService.ALFRESCO_URI, "working_root" + System.currentTimeMillis()), ContentModel.TYPE_FOLDER).getChildRef();
+            testCreateFileFolderService.create(workingRootNodeRef, "TestFolder-" + System.currentTimeMillis(), ContentModel.TYPE_FOLDER);
+
+            // Try this for a while until we get a result
+            boolean success = false;
+            for (int i = 0; i < 30; i++)
+            {
+                queryAuditLog(auditQueryCallback, params, -1);
+                if (results.size() > 1)
+                {
+                    logger.debug(sb.toString());
+                    success = true;
+                    break;
+                }
+                synchronized (this)
+                {
+                    try
+                    {
+                        this.wait(1000L);
+                    }
+                    catch (InterruptedException e)
+                    {
+                    }
+                }
+            }
+            assertTrue("There should be audit entry for the API test", success);
+        }
+        finally
+        {
+            if (workingRootNodeRef != null)
+            {
+                nodeService.deleteNode(workingRootNodeRef);
+            }
+        }
+    }	
+	
     public void testAuditOverlimitProperties() throws Exception
     {
         final int OVERLIMIT_SIZE = 1500;

source/test-java/org/alfresco/repo/audit/TestCreateFileFolderService.java

@@ -0,0 +1 @@
+package org.alfresco.repo.audit;

source/test-java/org/alfresco/repo/audit/TestCreateFileFolderServiceImpl.java

@@ -0,0 +1 @@
+package org.alfresco.repo.audit;

source/test-resources/alfresco/testaudit/alfresco-audit-test-mnt-10767-context.xml

@@ -0,0 +1,24 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<!DOCTYPE beans PUBLIC '-//SPRING//DTD BEAN//EN' 'http://www.springframework.org/dtd/spring-beans.dtd'>
+
+<beans>
+
+   <bean id="testCreateFileFolderService" class="org.alfresco.repo.audit.TestCreateFileFolderServiceImpl">
+      <property name="fileFolderService"><ref bean="FileFolderService" /></property>
+   </bean>
+
+    <bean id="TestCreateFileFolderService" class="org.springframework.aop.framework.ProxyFactoryBean">
+        <property name="proxyInterfaces">
+            <value>org.alfresco.repo.audit.TestCreateFileFolderService</value>
+        </property>
+        <property name="target">
+            <ref bean="testCreateFileFolderService"/>
+        </property>
+        <property name="interceptorNames">
+            <list>
+                <value>AuditMethodInterceptor</value>
+            </list>
+        </property>
+    </bean>
+
+</beans>

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

@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<Audit xmlns="http://www.alfresco.org/repo/audit/model/3.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://www.alfresco.org/repo/audit/model/3.2 alfresco-audit-3.2.xsd">
+
+    <DataExtractors>
+        <DataExtractor name="simpleValue" registeredName="auditModel.extractor.simpleValue" />
+    </DataExtractors>
+
+    <PathMappings>
+        <PathMap source="/alfresco-api/post/FileFolderService/create" target="/test-mnt-10767/FileFolderService/create" />
+    </PathMappings>
+
+    <Application name="Test MNT-10767" key="test-mnt-10767">
+
+        <AuditPath key="FileFolderService">
+            <AuditPath key="create">
+                <RecordValue key="parentNodeRef" dataSource="/test-mnt-10767/FileFolderService/create/args/parentNodeRef"
+                    dataTrigger="/test-mnt-10767/FileFolderService/create/no-error" dataExtractor="simpleValue" />
+                <!-- Dummy path to trick around oddities in the AuditComponent "do we need to audit?" logic -->
+                <AuditPath key="no-error" />
+            </AuditPath>
+        </AuditPath>
+    </Application>
+</Audit>
+