From e559032bf9a5f3239ac81dde948cdf79238ac6e0 Mon Sep 17 00:00:00 2001 From: Steven Glover Date: Tue, 9 Aug 2016 16:02:19 +0000 Subject: [PATCH] MNT-16375 "CMISChangelog auditing enablement impacts CMIS connection if quantity records are excessive" replace use of template.select with template.selectList (to prevent MySQL streaming exceptions), fix up affected client classes, and add extra AuditDAOTests around audit query fromId, toId, fromTime, toTime and maxResults git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/BRANCHES/DEV/5.2.N/root@129387 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../audit-select-SqlMap.xml | 1 + .../alfresco/repo/domain/audit/AuditDAO.java | 3 +- .../domain/audit/ibatis/AuditDAOImpl.java | 60 +-- .../alfresco/repo/template/TemplateNode.java | 19 +- .../repo/audit/AuditComponentTest.java | 66 ++- .../audit/AuditMethodInterceptorTest.java | 2 +- .../repo/domain/audit/AuditDAOTest.java | 390 +++++++++++++----- 7 files changed, 370 insertions(+), 171 deletions(-) diff --git a/config/alfresco/ibatis/org.hibernate.dialect.MySQLInnoDBDialect/audit-select-SqlMap.xml b/config/alfresco/ibatis/org.hibernate.dialect.MySQLInnoDBDialect/audit-select-SqlMap.xml index 7f18c4a22d..a577d7f75c 100644 --- a/config/alfresco/ibatis/org.hibernate.dialect.MySQLInnoDBDialect/audit-select-SqlMap.xml +++ b/config/alfresco/ibatis/org.hibernate.dialect.MySQLInnoDBDialect/audit-select-SqlMap.xml @@ -9,6 +9,7 @@ + diff --git a/source/java/org/alfresco/repo/domain/audit/AuditDAO.java b/source/java/org/alfresco/repo/domain/audit/AuditDAO.java index 4c68053bb9..8984d24fee 100644 --- a/source/java/org/alfresco/repo/domain/audit/AuditDAO.java +++ b/source/java/org/alfresco/repo/domain/audit/AuditDAO.java @@ -203,7 +203,8 @@ public interface AuditDAO * * @param callback the data callback per entry * @param parameters the parameters for the query (may not be null) - * @param maxResults the maximum number of results to retrieve + * @param maxResults the maximum number of results to retrieve. Must be greater than 0. + * @throws IllegalArgumentException if maxResults < 1. */ void findAuditEntries( AuditQueryCallback callback, diff --git a/source/java/org/alfresco/repo/domain/audit/ibatis/AuditDAOImpl.java b/source/java/org/alfresco/repo/domain/audit/ibatis/AuditDAOImpl.java index a4e2a91af8..2342dd2522 100644 --- a/source/java/org/alfresco/repo/domain/audit/ibatis/AuditDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/audit/ibatis/AuditDAOImpl.java @@ -25,27 +25,23 @@ */ 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 org.alfresco.ibatis.RollupResultHandler; -import org.alfresco.repo.domain.audit.AbstractAuditDAOImpl; -import org.alfresco.repo.domain.audit.AuditApplicationEntity; -import org.alfresco.repo.domain.audit.AuditDeleteParameters; -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.apache.ibatis.session.Configuration; -import org.apache.ibatis.session.ResultContext; -import org.apache.ibatis.session.ResultHandler; -import org.apache.ibatis.session.RowBounds; -import org.mybatis.spring.SqlSessionTemplate; +import java.io.Serializable; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.alfresco.repo.domain.audit.AbstractAuditDAOImpl; +import org.alfresco.repo.domain.audit.AuditApplicationEntity; +import org.alfresco.repo.domain.audit.AuditDeleteParameters; +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.apache.ibatis.session.RowBounds; +import org.mybatis.spring.SqlSessionTemplate; import org.springframework.dao.ConcurrencyFailureException; /** @@ -316,26 +312,8 @@ public class AuditDAOImpl extends AbstractAuditDAOImpl } } else - { - // RowHandlers in RowHandlers: See 'groupBy' issue for iBatis 2.x https://issues.apache.org/jira/browse/IBATIS-503 - ResultHandler queryResultHandler = new ResultHandler() - { - public void handleResult(ResultContext context) - { - rowHandler.processResult((AuditQueryResult)context.getResultObject()); - } - }; - Configuration configuration = template.getConfiguration(); - RollupResultHandler rollupResultHandler = new RollupResultHandler( - configuration, - new String[] {"auditEntryId"}, - "auditValueRows", - queryResultHandler, - maxResults); - - template.select(rowHandler.valuesRequired() ? SELECT_ENTRIES_WITH_VALUES - : SELECT_ENTRIES_WITHOUT_VALUES, params, rollupResultHandler); - rollupResultHandler.processLastResults(); + { + throw new IllegalArgumentException("maxResults must be greater than 0"); } } } diff --git a/source/java/org/alfresco/repo/template/TemplateNode.java b/source/java/org/alfresco/repo/template/TemplateNode.java index db5acc8d8a..65d47ed0a5 100644 --- a/source/java/org/alfresco/repo/template/TemplateNode.java +++ b/source/java/org/alfresco/repo/template/TemplateNode.java @@ -599,18 +599,21 @@ public class TemplateNode extends BasePermissionsNode implements NamespacePrefix String applicationName = "alfresco-access"; AuditQueryParameters pathParams = new AuditQueryParameters(); pathParams.setApplicationName(applicationName); - pathParams.addSearchKey("/alfresco-access/transaction/path", nodePath); - services.getAuditService().auditQuery(callback, pathParams, -1); - + pathParams.addSearchKey("/alfresco-access/transaction/path", nodePath); + // unfortunately, the getAuditTrail API forces us to get them all + services.getAuditService().auditQuery(callback, pathParams, Integer.MAX_VALUE); + AuditQueryParameters copyFromPathParams = new AuditQueryParameters(); copyFromPathParams.setApplicationName(applicationName); - copyFromPathParams.addSearchKey("/alfresco-access/transaction/copy/from/path", nodePath); - services.getAuditService().auditQuery(callback, copyFromPathParams, -1); - + copyFromPathParams.addSearchKey("/alfresco-access/transaction/copy/from/path", nodePath); + // unfortunately, the getAuditTrail API forces us to get them all + services.getAuditService().auditQuery(callback, copyFromPathParams, Integer.MAX_VALUE); + AuditQueryParameters moveFromPathParams = new AuditQueryParameters(); moveFromPathParams.setApplicationName(applicationName); - moveFromPathParams.addSearchKey("/alfresco-access/transaction/move/from/path", nodePath); - services.getAuditService().auditQuery(callback, moveFromPathParams, -1); + moveFromPathParams.addSearchKey("/alfresco-access/transaction/move/from/path", nodePath); + // unfortunately, the getAuditTrail API forces us to get them all + services.getAuditService().auditQuery(callback, moveFromPathParams, Integer.MAX_VALUE); return null; } }, AuthenticationUtil.getAdminUserName()); diff --git a/source/test-java/org/alfresco/repo/audit/AuditComponentTest.java b/source/test-java/org/alfresco/repo/audit/AuditComponentTest.java index e151e8fc6d..50e12dddf3 100644 --- a/source/test-java/org/alfresco/repo/audit/AuditComponentTest.java +++ b/source/test-java/org/alfresco/repo/audit/AuditComponentTest.java @@ -419,7 +419,7 @@ public class AuditComponentTest extends TestCase sb.delete(0, sb.length()); rowCount.setValue(0); - auditComponent.auditQuery(callback, params, -1); + auditComponent.auditQuery(callback, params, Integer.MAX_VALUE); assertTrue("Expected some data", rowCount.intValue() > 0); logger.debug(sb.toString()); int allResults = rowCount.intValue(); @@ -435,7 +435,7 @@ public class AuditComponentTest extends TestCase sb.delete(0, sb.length()); rowCount.setValue(0); params.setToTime(beforeTime); - auditComponent.auditQuery(callback, params, -1); + auditComponent.auditQuery(callback, params, Integer.MAX_VALUE); params.setToTime(null); logger.debug(sb.toString()); int resultsBefore = rowCount.intValue(); @@ -444,7 +444,7 @@ public class AuditComponentTest extends TestCase sb.delete(0, sb.length()); rowCount.setValue(0); params.setFromTime(beforeTime); - auditComponent.auditQuery(callback, params, -1); + auditComponent.auditQuery(callback, params, Integer.MAX_VALUE); params.setFromTime(null); logger.debug(sb.toString()); int resultsAfter = rowCount.intValue(); @@ -456,7 +456,7 @@ public class AuditComponentTest extends TestCase sb.delete(0, sb.length()); rowCount.setValue(0); params.setUser(user); - auditComponent.auditQuery(callback, params, -1); + auditComponent.auditQuery(callback, params, Integer.MAX_VALUE); params.setUser(null); assertTrue("Expected some data for specific user", rowCount.intValue() > 0); logger.debug(sb.toString()); @@ -464,7 +464,7 @@ public class AuditComponentTest extends TestCase sb.delete(0, sb.length()); rowCount.setValue(0); params.setUser("Numpty"); - auditComponent.auditQuery(callback, params, -1); + auditComponent.auditQuery(callback, params, Integer.MAX_VALUE); params.setUser(null); assertTrue("Expected no data for bogus user", rowCount.intValue() == 0); logger.debug(sb.toString()); @@ -607,7 +607,7 @@ public class AuditComponentTest extends TestCase clearAuditLog(APPLICATION_API_TEST); results.clear(); sb.delete(0, sb.length()); - queryAuditLog(auditQueryCallback, params, -1); + queryAuditLog(auditQueryCallback, params, Integer.MAX_VALUE); logger.debug(sb.toString()); assertTrue("There should be no audit entries for the API test after a clear", results.isEmpty()); @@ -641,7 +641,7 @@ public class AuditComponentTest extends TestCase // Check that the call was audited results.clear(); sb.delete(0, sb.length()); - queryAuditLog(auditQueryCallback, params, -1); + queryAuditLog(auditQueryCallback, params, Integer.MAX_VALUE); logger.debug(sb.toString()); assertFalse("Did not get any audit results after successful login", results.isEmpty()); @@ -672,7 +672,7 @@ public class AuditComponentTest extends TestCase { results.clear(); sb.delete(0, sb.length()); - queryAuditLog(auditQueryCallback, params, -1); + queryAuditLog(auditQueryCallback, params, Integer.MAX_VALUE); if(results.size() == iterations) { break; @@ -690,11 +690,49 @@ public class AuditComponentTest extends TestCase "Clearing " + results.size() + " entries by ID took " + (System.currentTimeMillis() - before) + "ms."); results.clear(); sb.delete(0, sb.length()); - queryAuditLog(auditQueryCallback, params, -1); + queryAuditLog(auditQueryCallback, params, Integer.MAX_VALUE); logger.debug(sb.toString()); assertEquals("Explicit audit entries were not deleted", 0, results.size()); } - + + public void testAuditQuery_MinId() throws Exception + { + AuditQueryCallback auditQueryCallback = new AuditQueryCallback() + { + public boolean valuesRequired() + { + return true; + } + + public boolean handleAuditEntry( + Long entryId, + String applicationName, + String user, + long time, + Map values) + { + if (logger.isDebugEnabled()) + { + logger.debug( + "Audit Entry " + entryId + ": " + applicationName + ", " + user + ", " + new Date(time) + "\n" + + " Data: " + values); + } + return true; + } + + public boolean handleAuditEntryError(Long entryId, String errorMsg, Throwable error) + { + throw new AlfrescoRuntimeException(errorMsg, error); + } + }; + + AuditQueryParameters params = new AuditQueryParameters(); + params.setApplicationName(APPLICATION_API_TEST); + params.setForward(false); + params.setToId(Long.MAX_VALUE); + queryAuditLog(auditQueryCallback, params, 1); + } + public void testAuditQuery_MaxId() throws Exception { AuditQueryCallback auditQueryCallback = new AuditQueryCallback() @@ -785,7 +823,7 @@ public class AuditComponentTest extends TestCase clearAuditLog(APPLICATION_ALF12638_TEST); results.clear(); sb.delete(0, sb.length()); - queryAuditLog(auditQueryCallback, params, -1); + queryAuditLog(auditQueryCallback, params, Integer.MAX_VALUE); assertTrue("There should be no audit entries for the API test after a clear", results.isEmpty()); try @@ -801,7 +839,7 @@ public class AuditComponentTest extends TestCase boolean success = false; for (int i = 0; i < 30; i++) { - queryAuditLog(auditQueryCallback, params, -1); + queryAuditLog(auditQueryCallback, params, Integer.MAX_VALUE); if (results.size() > 1) { logger.debug(sb.toString()); @@ -936,7 +974,7 @@ public class AuditComponentTest extends TestCase clearAuditLog(APPLICATION_MNT10767_TEST); results.clear(); sb.delete(0, sb.length()); - queryAuditLog(auditQueryCallback, params, -1); + queryAuditLog(auditQueryCallback, params, Integer.MAX_VALUE); assertTrue("There should be no audit entries for the API test after a clear", results.isEmpty()); PolicyComponent policyComponent = (PolicyComponent) ctx.getBean("policyComponent"); @@ -955,7 +993,7 @@ public class AuditComponentTest extends TestCase boolean success = false; for (int i = 0; i < 30; i++) { - queryAuditLog(auditQueryCallback, params, -1); + queryAuditLog(auditQueryCallback, params, Integer.MAX_VALUE); if (results.size() > 1) { logger.debug(sb.toString()); diff --git a/source/test-java/org/alfresco/repo/audit/AuditMethodInterceptorTest.java b/source/test-java/org/alfresco/repo/audit/AuditMethodInterceptorTest.java index d751831e15..cd9bdfb94b 100644 --- a/source/test-java/org/alfresco/repo/audit/AuditMethodInterceptorTest.java +++ b/source/test-java/org/alfresco/repo/audit/AuditMethodInterceptorTest.java @@ -176,7 +176,7 @@ public class AuditMethodInterceptorTest extends TestCase params.setApplicationName(APPLICATION_NAME); rowCount.setValue(0); - auditComponent.auditQuery(callback, params, -1); + auditComponent.auditQuery(callback, params, Integer.MAX_VALUE); assertEquals("There should be one audit entry.", 1, rowCount.intValue()); assertTrue("The requested nodeRef should be in the audit entry.", diff --git a/source/test-java/org/alfresco/repo/domain/audit/AuditDAOTest.java b/source/test-java/org/alfresco/repo/domain/audit/AuditDAOTest.java index 4051f4a849..46a20b706b 100644 --- a/source/test-java/org/alfresco/repo/domain/audit/AuditDAOTest.java +++ b/source/test-java/org/alfresco/repo/domain/audit/AuditDAOTest.java @@ -25,44 +25,45 @@ */ package org.alfresco.repo.domain.audit; -import java.io.File; -import java.io.IOException; -import java.io.Serializable; -import java.net.URL; -import java.util.Collections; -import java.util.Date; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; - -import javax.transaction.UserTransaction; - +import java.io.File; +import java.io.IOException; +import java.io.Serializable; +import java.net.URL; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Date; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; + +import javax.transaction.UserTransaction; + +import org.alfresco.error.AlfrescoRuntimeException; +import org.alfresco.repo.content.transform.AbstractContentTransformerTest; +import org.alfresco.repo.domain.audit.AuditDAO.AuditApplicationInfo; +import org.alfresco.repo.domain.contentdata.ContentDataDAO; +import org.alfresco.repo.domain.hibernate.dialect.AlfrescoMySQLClusterNDBDialect; +import org.alfresco.repo.domain.propval.PropValGenerator; +import org.alfresco.repo.domain.propval.PropertyValueDAO; +import org.alfresco.repo.transaction.RetryingTransactionHelper; +import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; +import org.alfresco.service.ServiceRegistry; +import org.alfresco.service.cmr.audit.AuditQueryParameters; +import org.alfresco.service.cmr.audit.AuditService.AuditQueryCallback; +import org.alfresco.service.cmr.repository.ContentData; +import org.alfresco.service.transaction.TransactionService; +import org.alfresco.test_category.OwnJVMTestsCategory; +import org.alfresco.util.ApplicationContextHelper; +import org.alfresco.util.GUID; +import org.alfresco.util.Pair; +import org.apache.commons.lang.mutable.MutableInt; +import org.hibernate.dialect.Dialect; +import org.junit.experimental.categories.Category; +import org.springframework.context.ConfigurableApplicationContext; + import junit.framework.TestCase; -import org.alfresco.error.AlfrescoRuntimeException; -import org.alfresco.repo.content.transform.AbstractContentTransformerTest; -import org.alfresco.repo.domain.audit.AuditDAO.AuditApplicationInfo; -import org.alfresco.repo.domain.contentdata.ContentDataDAO; -import org.alfresco.repo.domain.hibernate.dialect.AlfrescoMySQLClusterNDBDialect; -import org.alfresco.repo.domain.propval.PropValGenerator; -import org.alfresco.repo.domain.propval.PropertyValueDAO; -import org.alfresco.repo.transaction.RetryingTransactionHelper; -import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; -import org.alfresco.service.ServiceRegistry; -import org.alfresco.service.cmr.audit.AuditQueryParameters; -import org.alfresco.service.cmr.audit.AuditService.AuditQueryCallback; -import org.alfresco.service.cmr.repository.ContentData; -import org.alfresco.service.transaction.TransactionService; -import org.alfresco.test_category.OwnJVMTestsCategory; -import org.alfresco.util.ApplicationContextHelper; -import org.alfresco.util.GUID; -import org.alfresco.util.Pair; -import org.apache.commons.lang.mutable.MutableInt; -import org.hibernate.dialect.Dialect; -import org.junit.experimental.categories.Category; -import org.springframework.context.ConfigurableApplicationContext; - /** * @see ContentDataDAO * @@ -89,7 +90,7 @@ public class AuditDAOTest extends TestCase auditDAO = (AuditDAO) ctx.getBean("auditDAO"); propertyValueDAO = ctx.getBean(PropertyValueDAO.class); } - + public void testAuditModel() throws Exception { final File file = AbstractContentTransformerTest.loadQuickTestFile("pdf"); @@ -110,7 +111,7 @@ public class AuditDAOTest extends TestCase assertNotNull(configPairCheck); assertEquals(configPair, configPairCheck); } - + public void testAuditApplication() throws Exception { final File file = AbstractContentTransformerTest.loadQuickTestFile("pdf"); @@ -149,11 +150,12 @@ public class AuditDAOTest extends TestCase "Time for " + count + " application creations was " + ((double)(after - before)/(10E6)) + "ms"); } - + public void testAuditEntry() throws Exception { doAuditEntryImpl(1000); - } + } + /** * @return Returns the name of the application */ @@ -202,7 +204,7 @@ public class AuditDAOTest extends TestCase // Done return appName; } - + public synchronized void testAuditQuery() throws Exception { // Some entries @@ -280,7 +282,172 @@ public class AuditDAOTest extends TestCase // secondLastTimeStamp = timestamps.removeLast(); // assertTrue("The timestamps should be in descending order", lastTimestamp.compareTo(secondLastTimeStamp) < 0); } - + + /* + * Test combinations of fromId, toId, fromTime, toTime and maxResults + */ + public synchronized void testAuditQueryCombos() throws Exception + { + // Some entries + doAuditEntryImpl(10); + + final MutableInt count = new MutableInt(0); + final LinkedList timestamps = new LinkedList(); + final List entryIds = new LinkedList<>(); + // Find everything + final AuditQueryCallback callback = new AuditQueryCallback() + { + public boolean valuesRequired() + { + return false; + } + + public boolean handleAuditEntry( + Long entryId, + String applicationName, + String user, + long time, + Map values) + { + count.setValue(count.intValue() + 1); + timestamps.add(time); + entryIds.add(entryId); + return true; + } + + public boolean handleAuditEntryError(Long entryId, String errorMsg, Throwable error) + { + throw new AlfrescoRuntimeException(errorMsg, error); + } + }; + + final AuditQueryParameters params = new AuditQueryParameters(); + params.addSearchKey("/a/b/c", null); + + //. get them all + RetryingTransactionCallback findCallback = new RetryingTransactionCallback() + { + public Void execute() throws Throwable + { + auditDAO.findAuditEntries(callback, params, 10); + return null; + } + }; + count.setValue(0); + timestamps.clear(); + txnHelper.doInTransaction(findCallback); + assertEquals(10, count.intValue()); + + // copy what we found so that we can compare subsequent audit queries + List allEntryIds = new ArrayList<>(entryIds); + List allTimestamps = new ArrayList<>(timestamps); + + // test fromId and maxResults + entryIds.clear(); + timestamps.clear(); + params.setFromId(allEntryIds.get(2)); + findCallback = new RetryingTransactionCallback() + { + public Void execute() throws Throwable + { + auditDAO.findAuditEntries(callback, params, 2); + return null; + } + }; + txnHelper.doInTransaction(findCallback); + assertTrue(allEntryIds.subList(2, 2 + 2).equals(entryIds)); + + // test toId and maxResults + entryIds.clear(); + timestamps.clear(); + params.setFromId(null); + params.setFromTime(null); + params.setToTime(null); + params.setToId(allEntryIds.get(2)); + findCallback = new RetryingTransactionCallback() + { + public Void execute() throws Throwable + { + auditDAO.findAuditEntries(callback, params, 2); + return null; + } + }; + txnHelper.doInTransaction(findCallback); + assertTrue(allEntryIds.subList(0, 2).equals(entryIds)); + + // test fromId and toId and maxResults + entryIds.clear(); + timestamps.clear(); + params.setFromId(allEntryIds.get(2)); + params.setToId(allEntryIds.get(5)); + params.setFromTime(null); + params.setToTime(null); + findCallback = new RetryingTransactionCallback() + { + public Void execute() throws Throwable + { + auditDAO.findAuditEntries(callback, params, 1); + return null; + } + }; + txnHelper.doInTransaction(findCallback); + assertTrue(allEntryIds.subList(2, 3).equals(entryIds)); + + // test fromTime and maxResults + entryIds.clear(); + timestamps.clear(); + params.setFromTime(allTimestamps.get(2)); + params.setFromId(null); + params.setToTime(null); + params.setToId(null); + findCallback = new RetryingTransactionCallback() + { + public Void execute() throws Throwable + { + auditDAO.findAuditEntries(callback, params, 2); + return null; + } + }; + txnHelper.doInTransaction(findCallback); + assertTrue(allTimestamps.subList(2, 4).equals(timestamps)); + + // test toTime and maxResults + entryIds.clear(); + timestamps.clear(); + params.setFromTime(null); + params.setFromId(null); + params.setToTime(allTimestamps.get(4)); + params.setToId(null); + findCallback = new RetryingTransactionCallback() + { + public Void execute() throws Throwable + { + auditDAO.findAuditEntries(callback, params, 2); + return null; + } + }; + txnHelper.doInTransaction(findCallback); + assertTrue(allTimestamps.subList(0, 2).equals(timestamps)); + + // test fromTime and toTime and maxResults + entryIds.clear(); + timestamps.clear(); + params.setFromTime(allTimestamps.get(2)); + params.setFromId(null); + params.setToTime(allTimestamps.get(5)); + params.setToId(null); + findCallback = new RetryingTransactionCallback() + { + public Void execute() throws Throwable + { + auditDAO.findAuditEntries(callback, params, 2); + return null; + } + }; + txnHelper.doInTransaction(findCallback); + assertTrue(allTimestamps.subList(2, 4).equals(timestamps)); + } + public void testAuditDeleteEntries() throws Exception { final AuditQueryCallback noResultsCallback = new AuditQueryCallback() @@ -306,7 +473,7 @@ public class AuditDAOTest extends TestCase throw new AlfrescoRuntimeException(errorMsg, error); } }; - + // Some entries final String appName = doAuditEntryImpl(1); @@ -320,13 +487,12 @@ public class AuditDAOTest extends TestCase Long appId = auditDAO.getAuditApplication(appName).getId(); auditDAO.deleteAuditEntries(appId, null, null); // There should be no entries - auditDAO.findAuditEntries(noResultsCallback, params, -1); + auditDAO.findAuditEntries(noResultsCallback, params, Integer.MAX_VALUE); return null; } }; txnHelper.doInTransaction(deletedCallback); } - /** * Ensure that only the correct application's audit entries are deleted. @@ -347,7 +513,7 @@ public class AuditDAOTest extends TestCase auditDAO.deleteAuditEntries(app1Id, null, null); // There should be no entries for app1 // but still entries for app2 - auditDAO.findAuditEntries(resultsCallback, new AuditQueryParameters(), -1); + auditDAO.findAuditEntries(resultsCallback, new AuditQueryParameters(), Integer.MAX_VALUE); assertEquals("All entries should have been deleted from app1", 0, resultsCallback.numEntries(app1)); assertEquals("No entries should have been deleted from app2", 18, resultsCallback.numEntries(app2)); return null; @@ -361,59 +527,58 @@ public class AuditDAOTest extends TestCase * Ensure that an application's audit entries can be deleted between 2 times. * @throws Exception */ - public void testAuditDeleteEntriesForApplicationBetweenTimes() throws Exception - { - RetryingTransactionCallback deletedCallback = new RetryingTransactionCallback() - { - AuditQueryCallbackImpl preDeleteCallback = new AuditQueryCallbackImpl(); - AuditQueryCallbackImpl resultsCallback = new AuditQueryCallbackImpl(); - - - public Void execute() throws Throwable - { - AuditApplicationInfo info1 = createAuditApp(); - String app1 = info1.getName(); - Long app1Id = info1.getId(); - AuditApplicationInfo info2 = createAuditApp(); - String app2 = info2.getName(); - - // Create items 10, 11, 12, 13, 14 for application 1 - // Create items 21, 22 for application 2 - createItem(info1, 10); - createItem(info1, 11); - Thread.sleep(10); // stop previous statements being executed during t1 - Thread.sleep(10); - final long t1 = System.currentTimeMillis(); - Thread.sleep(10); - Thread.sleep(10); - createItem(info2, 21); - createItem(info1, 12); - createItem(info1, 13); - Thread.sleep(10); - Thread.sleep(10); - final long t2 = System.currentTimeMillis(); - Thread.sleep(10); // stop next statements being executed during t2 - Thread.sleep(10); - createItem(info2, 22); - createItem(info1, 14); - - - auditDAO.findAuditEntries(preDeleteCallback, new AuditQueryParameters(), -1); - assertEquals(5, preDeleteCallback.numEntries(app1)); - assertEquals(2, preDeleteCallback.numEntries(app2)); - - auditDAO.deleteAuditEntries(app1Id, t1, t2); - - auditDAO.findAuditEntries(resultsCallback, new AuditQueryParameters(), -1); - assertEquals("Two entries should have been deleted from app1", 3, resultsCallback.numEntries(app1)); - assertEquals("No entries should have been deleted from app2", 2, resultsCallback.numEntries(app2)); - return null; - } - }; - txnHelper.doInTransaction(deletedCallback); + public void testAuditDeleteEntriesForApplicationBetweenTimes() throws Exception + { + RetryingTransactionCallback deletedCallback = new RetryingTransactionCallback() + { + AuditQueryCallbackImpl preDeleteCallback = new AuditQueryCallbackImpl(); + AuditQueryCallbackImpl resultsCallback = new AuditQueryCallbackImpl(); + + + public Void execute() throws Throwable + { + AuditApplicationInfo info1 = createAuditApp(); + String app1 = info1.getName(); + Long app1Id = info1.getId(); + AuditApplicationInfo info2 = createAuditApp(); + String app2 = info2.getName(); + + // Create items 10, 11, 12, 13, 14 for application 1 + // Create items 21, 22 for application 2 + createItem(info1, 10); + createItem(info1, 11); + Thread.sleep(10); // stop previous statements being executed during t1 + Thread.sleep(10); + final long t1 = System.currentTimeMillis(); + Thread.sleep(10); + Thread.sleep(10); + createItem(info2, 21); + createItem(info1, 12); + createItem(info1, 13); + Thread.sleep(10); + Thread.sleep(10); + final long t2 = System.currentTimeMillis(); + Thread.sleep(10); // stop next statements being executed during t2 + Thread.sleep(10); + createItem(info2, 22); + createItem(info1, 14); + + + auditDAO.findAuditEntries(preDeleteCallback, new AuditQueryParameters(), -1); + assertEquals(5, preDeleteCallback.numEntries(app1)); + assertEquals(2, preDeleteCallback.numEntries(app2)); + + auditDAO.deleteAuditEntries(app1Id, t1, t2); + + auditDAO.findAuditEntries(resultsCallback, new AuditQueryParameters(), -1); + assertEquals("Two entries should have been deleted from app1", 3, resultsCallback.numEntries(app1)); + assertEquals("No entries should have been deleted from app2", 2, resultsCallback.numEntries(app2)); + return null; + } + }; + txnHelper.doInTransaction(deletedCallback); } - - + /** * Ensure audit entries can be deleted between two times - for all applications. * @throws Exception @@ -452,16 +617,15 @@ public class AuditDAOTest extends TestCase Thread.sleep(10); createItem(info2, 22); createItem(info1, 14); - - - auditDAO.findAuditEntries(preDeleteCallback, new AuditQueryParameters(), -1); + + auditDAO.findAuditEntries(preDeleteCallback, new AuditQueryParameters(), Integer.MAX_VALUE); assertEquals(5, preDeleteCallback.numEntries(app1)); assertEquals(2, preDeleteCallback.numEntries(app2)); // Delete audit entries between times - for all applications. auditDAO.deleteAuditEntries(null, t1, t2); - auditDAO.findAuditEntries(resultsCallback, new AuditQueryParameters(), -1); + auditDAO.findAuditEntries(resultsCallback, new AuditQueryParameters(), Integer.MAX_VALUE); assertEquals("Two entries should have been deleted from app1", 3, resultsCallback.numEntries(app1)); assertEquals("One entry should have been deleted from app2", 1, resultsCallback.numEntries(app2)); return null; @@ -560,7 +724,21 @@ public class AuditDAOTest extends TestCase // single test scriptCanDeleteOrphanedPropsWork(false); } - + + public void testMaxResults() throws Exception + { + try + { + AuditQueryCallbackImpl callback = new AuditQueryCallbackImpl(); + auditDAO.findAuditEntries(callback, new AuditQueryParameters(), -1); + fail("maxResults == -1 should be disallowed"); + } + catch(IllegalArgumentException e) + { + // ok + } + } + private void scriptCanDeleteOrphanedPropsWork(final boolean performance) throws Exception { final int iterationStep, maxIterations; @@ -610,8 +788,8 @@ public class AuditDAOTest extends TestCase // TODO: how to deal with Serializable values which cannot be retrieved later in test by value alone? long now = System.currentTimeMillis(); auditDAO.createAuditEntry(info1.getId(), now, username, values); - - auditDAO.findAuditEntries(preDeleteCallback, new AuditQueryParameters(), -1); + + auditDAO.findAuditEntries(preDeleteCallback, new AuditQueryParameters(), Integer.MAX_VALUE); assertEquals(1, preDeleteCallback.numEntries(app1)); // Delete audit entries between times - for all applications. @@ -619,7 +797,7 @@ public class AuditDAOTest extends TestCase if (!performance) { - auditDAO.findAuditEntries(resultsCallback, new AuditQueryParameters(), -1); + auditDAO.findAuditEntries(resultsCallback, new AuditQueryParameters(), Integer.MAX_VALUE); assertEquals("All entries should have been deleted from app1", 0, resultsCallback.numEntries(app1)); } }