diff --git a/config/alfresco/audit/alfresco-audit-3.2.xsd b/config/alfresco/audit/alfresco-audit-3.2.xsd index d79038a1e0..60f16aa225 100644 --- a/config/alfresco/audit/alfresco-audit-3.2.xsd +++ b/config/alfresco/audit/alfresco-audit-3.2.xsd @@ -104,7 +104,7 @@ - + diff --git a/config/alfresco/dbscripts/create/3.2/org.hibernate.dialect.MySQLInnoDBDialect/AlfrescoPostCreate-3.2-AuditTables.sql b/config/alfresco/dbscripts/create/3.2/org.hibernate.dialect.MySQLInnoDBDialect/AlfrescoPostCreate-3.2-AuditTables.sql index 4795de2da9..ecdb9c3af1 100644 --- a/config/alfresco/dbscripts/create/3.2/org.hibernate.dialect.MySQLInnoDBDialect/AlfrescoPostCreate-3.2-AuditTables.sql +++ b/config/alfresco/dbscripts/create/3.2/org.hibernate.dialect.MySQLInnoDBDialect/AlfrescoPostCreate-3.2-AuditTables.sql @@ -33,7 +33,7 @@ CREATE TABLE alf_audit_entry audit_session_id BIGINT NOT NULL, audit_time BIGINT NOT NULL, audit_user_id BIGINT NULL, - audit_values_id BIGINT NOT NULL, + audit_values_id BIGINT NULL, CONSTRAINT fk_alf_audit_ent_sess FOREIGN KEY (audit_session_id) REFERENCES alf_audit_session (id), INDEX idx_alf_audit_ent_time (audit_time), CONSTRAINT fk_alf_audit_ent_user FOREIGN KEY (audit_user_id) REFERENCES alf_prop_value (id), diff --git a/source/java/org/alfresco/repo/audit/AuditBootstrapTest.java b/source/java/org/alfresco/repo/audit/AuditBootstrapTest.java index b566dc9745..6248f53778 100644 --- a/source/java/org/alfresco/repo/audit/AuditBootstrapTest.java +++ b/source/java/org/alfresco/repo/audit/AuditBootstrapTest.java @@ -36,6 +36,8 @@ import org.alfresco.repo.audit.model.AuditApplication; import org.alfresco.repo.audit.model.AuditModelException; import org.alfresco.repo.audit.model.AuditModelRegistry; import org.alfresco.util.ApplicationContextHelper; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.context.ApplicationContext; import org.springframework.util.ResourceUtils; @@ -52,6 +54,7 @@ public class AuditBootstrapTest extends TestCase private static final String APPLICATION_TEST = "Alfresco Test"; private static ApplicationContext ctx = ApplicationContextHelper.getApplicationContext(); + private static final Log logger = LogFactory.getLog(AuditBootstrapTest.class); private AuditModelRegistry auditModelRegistry; @@ -83,6 +86,7 @@ public class AuditBootstrapTest extends TestCase catch (AuditModelException e) { // Expected + logger.error("Expected AuditModelException: " + e.getMessage()); } } @@ -111,6 +115,11 @@ public class AuditBootstrapTest extends TestCase loadBadModel("classpath:alfresco/audit/alfresco-audit-test-bad-05.xml"); } + public void testModelLoading_BadGeneratorRegisteredName() throws Exception + { + loadBadModel("classpath:alfresco/audit/alfresco-audit-test-bad-06.xml"); + } + public void testGetModelId() { Long repoId = auditModelRegistry.getAuditModelId(APPLICATION_TEST); diff --git a/source/java/org/alfresco/repo/audit/AuditComponent.java b/source/java/org/alfresco/repo/audit/AuditComponent.java index a016caf948..3df7d94480 100644 --- a/source/java/org/alfresco/repo/audit/AuditComponent.java +++ b/source/java/org/alfresco/repo/audit/AuditComponent.java @@ -109,18 +109,25 @@ public interface AuditComponent AuditSession startAuditSession(String applicationName, String rootPath, Map values); /** - * Record a set of values against the given session. + * Record a set of values against the given session. The map is a path (starting with '/') relative + * to the root path given when {@link #startAuditSession(String, String) starting the session}. All + * resulting path values (session root path + map entry paths) must have data recorder entries and + * be enabled for data to be recorded. + *

+ * The return values reflect what was actually persisted and is controlled by the data extractors + * defined in the audit configuration. *

* This is a read-write method. Client code must wrap calls in the appropriate transactional wrappers. * * @param session a pre-existing audit session to continue with * @param values the values to audit mapped by {@link AuditPath} key relative to the session * root path + * @return Returns the values that were actually persisted, keyed by their full path. * @throws IllegalStateException if there is not a writable transaction present * * @see #startAuditSession() * * @since 3.2 */ - void audit(AuditSession session, Map values); + Map audit(AuditSession session, Map values); } diff --git a/source/java/org/alfresco/repo/audit/AuditComponentImpl.java b/source/java/org/alfresco/repo/audit/AuditComponentImpl.java index d5c819c4e4..b6a01dfddf 100644 --- a/source/java/org/alfresco/repo/audit/AuditComponentImpl.java +++ b/source/java/org/alfresco/repo/audit/AuditComponentImpl.java @@ -837,7 +837,7 @@ public class AuditComponentImpl implements AuditComponent * {@inheritDoc} * @since 3.2 */ - public void audit(AuditSession session, Map values) + public Map audit(AuditSession session, Map values) { ParameterCheck.mandatory("session", session); ParameterCheck.mandatory("values", values); @@ -847,27 +847,40 @@ public class AuditComponentImpl implements AuditComponent throw new IllegalStateException("Auditing requires a read-write transaction."); } - // Audit nothing if there are no values (otherwise we're just creating maps for nothing) - if (values.size() == 0) + AuditApplication app = session.getApplication(); + String rootPath = session.getRootPath(); + Long sessionId = session.getSessionId(); + + // Build the key paths using the session root path + Map pathedValues = new HashMap(values.size() * 2); + for (Map.Entry entry : values.entrySet()) { - if (logger.isDebugEnabled()) - { - logger.debug("Nothing audited because there are no audit values."); - } - return; + String pathElement = entry.getKey(); + String path = AuditApplication.buildPath(rootPath, pathElement); + pathedValues.put(path, entry.getValue()); } - Long sessionId = session.getSessionId(); + // Now extract values + Map extractedValues = extractData(app, pathedValues); + + // Time and username are intrinsic long time = System.currentTimeMillis(); String username = AuthenticationUtil.getFullyAuthenticatedUser(); - Long entryId = auditDAO.createAuditEntry(sessionId, time, username, values); + // Persist the values + Long entryId = auditDAO.createAuditEntry(sessionId, time, username, pathedValues); // Done if (logger.isDebugEnabled()) { - logger.debug("New audit entry: " + entryId); + logger.debug( + "New audit entry: \n" + + " Session ID: " + sessionId + "\n" + + " Entry ID: " + entryId + "\n" + + " Path Values: " + pathedValues + "\n" + + " Extracted Values: " + extractedValues); } + return extractedValues; } /** diff --git a/source/java/org/alfresco/repo/audit/AuditComponentTest.java b/source/java/org/alfresco/repo/audit/AuditComponentTest.java index 99b8d6163e..61cb7845cd 100644 --- a/source/java/org/alfresco/repo/audit/AuditComponentTest.java +++ b/source/java/org/alfresco/repo/audit/AuditComponentTest.java @@ -32,13 +32,19 @@ import java.util.Map; import junit.framework.TestCase; +import org.alfresco.repo.audit.model.AuditApplication; import org.alfresco.repo.audit.model.AuditModelException; import org.alfresco.repo.audit.model.AuditModelRegistry; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; +import org.alfresco.service.ServiceRegistry; +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.transaction.TransactionService; import org.alfresco.util.ApplicationContextHelper; +import org.alfresco.util.EqualsHelper; import org.springframework.context.ApplicationContext; import org.springframework.util.ResourceUtils; @@ -59,22 +65,37 @@ public class AuditComponentTest extends TestCase private AuditModelRegistry auditModelRegistry; private AuditComponent auditComponent; + private ServiceRegistry serviceRegistry; private TransactionService transactionService; + private NodeService nodeService; + + private NodeRef nodeRef; @Override public void setUp() throws Exception { auditModelRegistry = (AuditModelRegistry) ctx.getBean("auditModel.modelRegistry"); auditComponent = (AuditComponent) ctx.getBean("auditComponent"); - transactionService = (TransactionService) ctx.getBean("transactionService"); + serviceRegistry = (ServiceRegistry) ctx.getBean(ServiceRegistry.SERVICE_REGISTRY); + transactionService = serviceRegistry.getTransactionService(); + nodeService = serviceRegistry.getNodeService(); // Register the test model URL testModelUrl = ResourceUtils.getURL("classpath:alfresco/audit/alfresco-audit-test.xml"); auditModelRegistry.registerModel(testModelUrl); auditModelRegistry.loadAuditModels(); + RunAsWork testRunAs = new RunAsWork() + { + public NodeRef doWork() throws Exception + { + return nodeService.getRootNode(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE); + } + }; + nodeRef = AuthenticationUtil.runAs(testRunAs, AuthenticationUtil.getSystemUserName()); + // Authenticate - AuthenticationUtil.setFullyAuthenticatedUser("User-" + getName() + System.currentTimeMillis()); + AuthenticationUtil.setFullyAuthenticatedUser("User-" + getName()); } @Override @@ -164,34 +185,102 @@ public class AuditComponentTest extends TestCase AuthenticationUtil.runAs(testRunAs, "SomeOtherUser"); } + private Map auditTestAction( + final String action, + NodeRef nodeRef, + Map parameters) + { + final Map adjustedValues = new HashMap(parameters.size() * 2); + // Add the noderef + adjustedValues.put(AuditApplication.buildPath("context-node"), nodeRef); + // Compile path-name snippets for the parameters + for (Map.Entry entry : parameters.entrySet()) + { + String paramName = entry.getKey(); + String path = AuditApplication.buildPath("params", paramName); + adjustedValues.put(path, entry.getValue()); + } + + RetryingTransactionCallback> auditCallback = + new RetryingTransactionCallback>() + { + public Map execute() throws Throwable + { + String actionPath = AuditApplication.buildPath("test/actions", action); + AuditSession session = auditComponent.startAuditSession(APPLICATION_TEST, actionPath); + + return auditComponent.audit(session, adjustedValues); + } + }; + return transactionService.getRetryingTransactionHelper().doInTransaction(auditCallback); + } + + private void checkAuditMaps(Map result, Map expected) + { + Map copyResult = new HashMap(result); + + boolean failure = false; + + StringBuilder sb = new StringBuilder(1024); + sb.append("\nValues that don't match the expected values: "); + for (Map.Entry entry : expected.entrySet()) + { + String key = entry.getKey(); + Serializable expectedValue = entry.getValue(); + Serializable resultValue = result.get(key); + if (!EqualsHelper.nullSafeEquals(resultValue, expectedValue)) + { + sb.append("\n") + .append(" Key: ").append(key).append("\n") + .append(" Result: ").append(resultValue).append("\n") + .append(" Expected: ").append(expectedValue); + failure = true; + } + copyResult.remove(key); + } + sb.append("\nValues that are present but should not be: "); + for (Map.Entry entry : copyResult.entrySet()) + { + String key = entry.getKey(); + Serializable resultValue = entry.getValue(); + sb.append("\n") + .append(" Key: ").append(key).append("\n") + .append(" Result: ").append(resultValue); + failure = true; + } + if (failure) + { + fail(sb.toString()); + } + } + /** * Start a session and use it within a single txn */ - public void testSession_Extended01() throws Exception + public void testSession_Action01() throws Exception { - final RetryingTransactionCallback testCallback = new RetryingTransactionCallback() - { - public Void execute() throws Throwable - { - AuditSession session = auditComponent.startAuditSession(APPLICATION_TEST, "/test/1.1"); - - Map values = new HashMap(13); - values.put("/test/1.1/2.1/3.1/4.1", new Long(41)); - values.put("/test/1.1/2.1/3.1/4.2", "42"); - values.put("/test/1.1/2.1/3.1/4.2", new Date()); - - auditComponent.audit(session, values); - - return null; - } - }; - RunAsWork testRunAs = new RunAsWork() - { - public Void doWork() throws Exception - { - return transactionService.getRetryingTransactionHelper().doInTransaction(testCallback); - } - }; - AuthenticationUtil.runAs(testRunAs, "SomeOtherUser"); + Serializable valueA = new Date(); + Serializable valueB = "BBB-value-here"; + Serializable valueC = new Float(16.0F); + // Get a noderef + final Map parameters = new HashMap(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 result = auditTestAction("action-01", nodeRef, parameters); + + Map expected = new HashMap(); + expected.put("/test/actions/action-01/context-node/noderef", nodeRef); + expected.put("/test/actions/action-01/params/A/value", valueA); + expected.put("/test/actions/action-01/params/B/value", valueB); + expected.put("/test/actions/action-01/params/C/value", valueC); + + // Check + checkAuditMaps(result, expected); } } diff --git a/source/java/org/alfresco/repo/audit/model/AuditApplication.java b/source/java/org/alfresco/repo/audit/model/AuditApplication.java index a2040215df..8c41b76b93 100644 --- a/source/java/org/alfresco/repo/audit/model/AuditApplication.java +++ b/source/java/org/alfresco/repo/audit/model/AuditApplication.java @@ -53,6 +53,8 @@ import org.apache.commons.logging.LogFactory; public class AuditApplication { public static final String AUDIT_PATH_SEPARATOR = "/"; + public static final String AUDIT_KEY_REGEX = "[a-zA-Z0-9\\-\\.]+"; + public static final String AUDIT_PATH_REGEX = "(/[a-zA-Z0-9\\-\\.]+)+"; private static final Log logger = LogFactory.getLog(AuditApplication.class); @@ -145,57 +147,73 @@ public class AuditApplication * Helper method to check that a path is correct for this application instance * * @param path the path in format /app-key/x/y/z - * @throws AuditModelException if the path is invalid + * @throws AuditModelException if the path is invalid + * + * @see #AUDIT_PATH_REGEX */ public void checkPath(String path) { if (path == null || path.length() == 0) { - generateException(path, "Invalid audit path."); + generateException(path, "Empty or null audit path"); } - if (!path.startsWith(AUDIT_PATH_SEPARATOR)) + else if (!path.matches(AUDIT_PATH_REGEX)) { generateException( path, - "An audit path must always start with the separator '" + AUDIT_PATH_SEPARATOR + "'."); + "An audit must match regular expression: " + AUDIT_PATH_REGEX); } - if (path.indexOf(applicationKey, 0) != 1) + else if (path.indexOf(applicationKey, 0) != 1) { generateException( path, "An audit path's first element must be the application's key i.e. '" + applicationKey + "'."); } - if (path.endsWith(AUDIT_PATH_SEPARATOR)) - { - generateException( - path, - "An audit path may not end with the separator '" + AUDIT_PATH_SEPARATOR + "'."); - } - if (!path.toLowerCase().equals(path)) - { - generateException( - path, - "An audit path may only contain lowercase letters, '-' and '.' i.e. regex ([a-z]|[0-9]|\\-|\\.)*"); - } } /** * Compile a path or part of a path into a single string which always starts with the * {@link #AUDIT_PATH_SEPARATOR}. This can be a relative path so need not always start with * the application root key. + *

+ * If the path separator is present at the beginning of a path component, then it is not added, + * so "/a", "b", "/c" becomes "/a/b/c" allowing path to be appended + * to other paths. + *

+ * The final result is checked against a {@link #AUDIT_PATH_REGEX regular expression} to ensure + * it is valid. * * @param pathElements the elements of the path e.g. "a", "b", "c". * @return Returns the compiled path e.g "/a/b/c". */ - public String buildPath(String ... pathComponents) + public static String buildPath(String ... pathComponents) { StringBuilder sb = new StringBuilder(pathComponents.length * 10); for (String pathComponent : pathComponents) { - sb.append(AUDIT_PATH_SEPARATOR).append(pathComponent); + if (!pathComponent.startsWith(AUDIT_PATH_SEPARATOR)) + { + sb.append(AUDIT_PATH_SEPARATOR); + } + sb.append(pathComponent); + } + String path = sb.toString(); + // Check the path format + if (!path.matches(AUDIT_PATH_REGEX)) + { + StringBuffer msg = new StringBuffer(); + msg.append("The audit path is invalid and must be matched by regular expression: ").append(AUDIT_PATH_REGEX).append("\n") + .append(" Path elements: "); + for (String pathComponent : pathComponents) + { + msg.append(pathComponent).append(", "); + } + msg.append("\n") + .append(" Result: ").append(path); + throw new AuditModelException(msg.toString()); } // Done - return sb.toString(); + return path; } /** @@ -292,7 +310,7 @@ public class AuditApplication { buildAuditPaths( auditPath, - "", + null, new HashSet(37), new HashMap(13), new HashMap(13)); @@ -312,26 +330,29 @@ public class AuditApplication upperGeneratorsByPath = new HashMap(upperGeneratorsByPath); // Append the current audit path to the current path - currentPath = (currentPath + AUDIT_PATH_SEPARATOR + auditPath.getKey()); + if (currentPath == null) + { + currentPath = AuditApplication.buildPath(auditPath.getKey()); + } + else + { + currentPath = AuditApplication.buildPath(currentPath, auditPath.getKey()); + } // Make sure we have not processed it before if (!existingPaths.add(currentPath)) { generateException(currentPath, "The audit path already exists."); } - // Make sure that the path is all lowercase - checkPathCase(currentPath); // Get the data extractors declared for this key for (RecordValue element : auditPath.getRecordValue()) { String key = element.getKey(); - String extractorPath = (currentPath + AUDIT_PATH_SEPARATOR + key); + String extractorPath = AuditApplication.buildPath(currentPath, key); if (!existingPaths.add(extractorPath)) { generateException(extractorPath, "The audit path already exists."); } - // Make sure that the path is all lowercase - checkPathCase(extractorPath); String extractorName = element.getDataExtractor(); DataExtractor extractor = dataExtractorsByName.get(extractorName); @@ -354,13 +375,11 @@ public class AuditApplication for (GenerateValue element : auditPath.getGenerateValue()) { String key = element.getKey(); - String generatorPath = (currentPath + AUDIT_PATH_SEPARATOR + key); + String generatorPath = AuditApplication.buildPath(currentPath, key); if (!existingPaths.add(generatorPath)) { generateException(generatorPath, "The audit path already exists."); } - // Make sure that the path is all lowercase - checkPathCase(generatorPath); String generatorName = element.getDataGenerator(); DataGenerator generator = dataGeneratorsByName.get(generatorName); @@ -401,14 +420,6 @@ public class AuditApplication } } - private void checkPathCase(String path) - { - if (!path.equals(path.toLowerCase())) - { - generateException(path, "Audit key entries may only be in lowercase to ensure case-insensitivity."); - } - } - private void generateException(String path, String msg) throws AuditModelException { throw new AuditModelException("" + diff --git a/source/java/org/alfresco/repo/audit/model/AuditModelRegistry.java b/source/java/org/alfresco/repo/audit/model/AuditModelRegistry.java index 1c453ecb07..0ddae8201a 100644 --- a/source/java/org/alfresco/repo/audit/model/AuditModelRegistry.java +++ b/source/java/org/alfresco/repo/audit/model/AuditModelRegistry.java @@ -435,11 +435,12 @@ public class AuditModelRegistry } else if (extractorElement.getRegisteredName() != null) { - dataExtractor = dataExtractors.getNamedObject(extractorElement.getRegisteredName()); + String registeredName = extractorElement.getRegisteredName(); + dataExtractor = dataExtractors.getNamedObject(registeredName); if (dataExtractor == null) { throw new AuditModelException( - "No registered audit data extractor exists for '" + name + "'."); + "No registered audit data extractor exists for '" + registeredName + "'."); } } else @@ -488,11 +489,12 @@ public class AuditModelRegistry } else if (generatorElement.getRegisteredName() != null) { - dataGenerator = dataGenerators.getNamedObject(generatorElement.getRegisteredName()); + String registeredName = generatorElement.getRegisteredName(); + dataGenerator = dataGenerators.getNamedObject(registeredName); if (dataGenerator == null) { throw new AuditModelException( - "No registered audit data generator exists for '" + name + "'."); + "No registered audit data generator exists for '" + registeredName + "'."); } } else diff --git a/source/java/org/alfresco/repo/audit/model/_3/ObjectFactory.java b/source/java/org/alfresco/repo/audit/model/_3/ObjectFactory.java index 971701d3d9..2828f80fd4 100644 --- a/source/java/org/alfresco/repo/audit/model/_3/ObjectFactory.java +++ b/source/java/org/alfresco/repo/audit/model/_3/ObjectFactory.java @@ -34,19 +34,11 @@ public class ObjectFactory { } /** - * Create an instance of {@link Application } + * Create an instance of {@link KeyedAuditDefinition } * */ - public Application createApplication() { - return new Application(); - } - - /** - * Create an instance of {@link DataGenerators } - * - */ - public DataGenerators createDataGenerators() { - return new DataGenerators(); + public KeyedAuditDefinition createKeyedAuditDefinition() { + return new KeyedAuditDefinition(); } /** @@ -57,30 +49,6 @@ public class ObjectFactory { return new DataExtractor(); } - /** - * Create an instance of {@link AuditPath } - * - */ - public AuditPath createAuditPath() { - return new AuditPath(); - } - - /** - * Create an instance of {@link RecordValue } - * - */ - public RecordValue createRecordValue() { - return new RecordValue(); - } - - /** - * Create an instance of {@link GenerateValue } - * - */ - public GenerateValue createGenerateValue() { - return new GenerateValue(); - } - /** * Create an instance of {@link Audit } * @@ -89,14 +57,6 @@ public class ObjectFactory { return new Audit(); } - /** - * Create an instance of {@link KeyedAuditDefinition } - * - */ - public KeyedAuditDefinition createKeyedAuditDefinition() { - return new KeyedAuditDefinition(); - } - /** * Create an instance of {@link DataExtractors } * @@ -105,6 +65,14 @@ public class ObjectFactory { return new DataExtractors(); } + /** + * Create an instance of {@link AuditPath } + * + */ + public AuditPath createAuditPath() { + return new AuditPath(); + } + /** * Create an instance of {@link DataGenerator } * @@ -113,6 +81,38 @@ public class ObjectFactory { return new DataGenerator(); } + /** + * Create an instance of {@link DataGenerators } + * + */ + public DataGenerators createDataGenerators() { + return new DataGenerators(); + } + + /** + * Create an instance of {@link RecordValue } + * + */ + public RecordValue createRecordValue() { + return new RecordValue(); + } + + /** + * Create an instance of {@link Application } + * + */ + public Application createApplication() { + return new Application(); + } + + /** + * Create an instance of {@link GenerateValue } + * + */ + public GenerateValue createGenerateValue() { + return new GenerateValue(); + } + /** * Create an instance of {@link JAXBElement }{@code <}{@link Audit }{@code >}} * diff --git a/source/java/org/alfresco/repo/domain/audit/AbstractAuditDAOImpl.java b/source/java/org/alfresco/repo/domain/audit/AbstractAuditDAOImpl.java index 98ffaedbec..211c7fde54 100644 --- a/source/java/org/alfresco/repo/domain/audit/AbstractAuditDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/audit/AbstractAuditDAOImpl.java @@ -235,7 +235,11 @@ public abstract class AbstractAuditDAOImpl implements AuditDAO usernameId = null; } // Now persist the data values - final Long valuesId = propertyValueDAO.getOrCreatePropertyValue((Serializable)values).getFirst(); + Long valuesId = null; + if (values != null && values.size() > 0) + { + valuesId = propertyValueDAO.getOrCreatePropertyValue((Serializable)values).getFirst(); + } // Create the audit entry AuditEntryEntity entity = createAuditEntry(sessionId, time, usernameId, valuesId); diff --git a/source/java/org/alfresco/repo/domain/propval/AbstractPropertyValueDAOImpl.java b/source/java/org/alfresco/repo/domain/propval/AbstractPropertyValueDAOImpl.java index d405c7584e..e1a9918697 100644 --- a/source/java/org/alfresco/repo/domain/propval/AbstractPropertyValueDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/propval/AbstractPropertyValueDAOImpl.java @@ -583,7 +583,7 @@ public abstract class AbstractPropertyValueDAOImpl implements PropertyValueDAO /** * {@inheritDoc} - * @see #getOrCreatePropertyValueImpl(Serializable, int, int) + * @see #getOrCreatePropertyValueImpl(Serializable, Long, int, int) */ public Pair getOrCreatePropertyValue(Serializable value, int maxDepth) { @@ -599,6 +599,7 @@ public abstract class AbstractPropertyValueDAOImpl implements PropertyValueDAO { if (value != null && maxDepth > currentDepth && value instanceof Map) { + // TODO: Go through cache? // The default is to do a deep expansion Long mapId = createPropertyMapImpl( (Map)value, @@ -606,11 +607,13 @@ public abstract class AbstractPropertyValueDAOImpl implements PropertyValueDAO maxDepth, currentDepth); Pair entityPair = new Pair(mapId, value); - // TODO: Go through cache? + // Cache instance by ID only + propertyValueCache.updateValue(mapId, value); return entityPair; } else if (value != null && maxDepth > currentDepth && value instanceof Collection) { + // TODO: Go through cache? // The default is to do a deep expansion Long collectionId = createPropertyCollectionImpl( (Collection)value, @@ -618,7 +621,8 @@ public abstract class AbstractPropertyValueDAOImpl implements PropertyValueDAO maxDepth, currentDepth); Pair entityPair = new Pair(collectionId, value); - // TODO: Go through cache? + // Cache instance by ID only + propertyValueCache.updateValue(collectionId, value); return entityPair; } else @@ -722,6 +726,16 @@ public abstract class AbstractPropertyValueDAOImpl implements PropertyValueDAO PropertyValueEntity entity = findPropertyValueByValue(value); return convertEntityToPair(entity); } + + /** + * No-op. This is implemented as we just want to update the cache. + * @return Returns 0 always + */ + @Override + public int updateValue(Long key, Serializable value) + { + return 0; + } } protected abstract List findPropertyValueById(Long id); diff --git a/source/java/org/alfresco/repo/domain/propval/DefaultPropertyTypeConverter.java b/source/java/org/alfresco/repo/domain/propval/DefaultPropertyTypeConverter.java index e4ad789241..49cc11a1c6 100644 --- a/source/java/org/alfresco/repo/domain/propval/DefaultPropertyTypeConverter.java +++ b/source/java/org/alfresco/repo/domain/propval/DefaultPropertyTypeConverter.java @@ -24,26 +24,89 @@ */ package org.alfresco.repo.domain.propval; +import java.io.Serializable; +import java.util.Collections; +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; + +import org.alfresco.repo.domain.propval.PropertyValueEntity.PersistedType; +import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.Period; import org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter; +import org.alfresco.util.ParameterCheck; /** * Default converter for handling data going to and from the persistence layer. *

- * Apart from converting between Boolean and Long values, - * the {@link DefaultTypeConverter} is used. + * Properties are stored as a set of well-defined types defined by the enumeration + * {@link PersistedType}. Ultimately, data can be persisted as BLOB data, but must + * be the last resort. * * @author Derek Hulley * @since 3.2 */ public class DefaultPropertyTypeConverter implements PropertyTypeConverter { + /** + * An unmodifiable map of types and how they should be persisted + */ + protected static final Map, PersistedType> defaultPersistedTypesByClass; + + static + { + // Create the map of class-type + Map, PersistedType> mapClass = new HashMap, PersistedType>(29); + mapClass.put(NodeRef.class, PersistedType.STRING); + mapClass.put(Period.class, PersistedType.STRING); + mapClass.put(Locale.class, PersistedType.STRING); + defaultPersistedTypesByClass = Collections.unmodifiableMap(mapClass); + } + + private Map, PersistedType> persistenceMapping; + /** * Default constructor */ public DefaultPropertyTypeConverter() { + persistenceMapping = new HashMap, PersistedType>( + DefaultPropertyTypeConverter.defaultPersistedTypesByClass); } + /** + * Allow subclasses to add further type mappings specific to the implementation + * + * @param clazz the class to be converted + * @param targetType the target persisted type + */ + protected void addTypeMapping(Class clazz, PersistedType targetType) + { + this.persistenceMapping.put(clazz, targetType); + } + + /** + * {@inheritDoc} + */ + public PersistedType getPersistentType(Serializable value) + { + ParameterCheck.mandatory("value", value); + + Class clazz = value.getClass(); + PersistedType type = persistenceMapping.get(clazz); + if (type == null) + { + return PersistedType.SERIALIZABLE; + } + else + { + return type; + } + } + + /** + * Performs the conversion + */ public T convert(Class targetClass, Object value) { return DefaultTypeConverter.INSTANCE.convert(targetClass, value); diff --git a/source/java/org/alfresco/repo/domain/propval/PropertyTypeConverter.java b/source/java/org/alfresco/repo/domain/propval/PropertyTypeConverter.java index 71f34159fd..f6a151cc8a 100644 --- a/source/java/org/alfresco/repo/domain/propval/PropertyTypeConverter.java +++ b/source/java/org/alfresco/repo/domain/propval/PropertyTypeConverter.java @@ -24,6 +24,10 @@ */ package org.alfresco.repo.domain.propval; +import java.io.Serializable; + +import org.alfresco.repo.domain.propval.PropertyValueEntity.PersistedType; + /** * Interface for converters that to translate between persisted values and external values. *

@@ -35,6 +39,25 @@ package org.alfresco.repo.domain.propval; */ public interface PropertyTypeConverter { + /** + * When external to persisted type mappings are not obvious, the persistence framework, + * before persisting as {@link PersistedType#SERIALIZABLE}, will give the converter + * a chance to choose how the value must be persisted: + *

+ * The converter should return {@link PersistedType#SERIALIZABLE} if no further conversions + * are possible. Implicit in the return value is the converter's ability to do the + * conversion when required. + * + * @param value the value that does not have an obvious persistence slot + * @return Returns the type of persistence to use + */ + PersistedType getPersistentType(Serializable value); + /** * Convert a value to a given type. * diff --git a/source/java/org/alfresco/repo/domain/propval/PropertyValueEntity.java b/source/java/org/alfresco/repo/domain/propval/PropertyValueEntity.java index bc04f9a14a..bf269a3d59 100644 --- a/source/java/org/alfresco/repo/domain/propval/PropertyValueEntity.java +++ b/source/java/org/alfresco/repo/domain/propval/PropertyValueEntity.java @@ -326,7 +326,8 @@ public class PropertyValueEntity persistedTypeEnum = persistedTypesByClass.get(valueClazz); if (persistedTypeEnum == null) { - persistedTypeEnum = PersistedType.SERIALIZABLE; + // Give the converter a chance to change the type it must be persisted as + persistedTypeEnum = converter.getPersistentType(value); } persistedType = persistedTypeEnum.getOrdinalNumber(); // Get the class to persist as @@ -345,7 +346,11 @@ public class PropertyValueEntity serializableValue = value; break; default: - throw new IllegalStateException("Should not be able to get through switch"); + throw new IllegalStateException( + "PropertyTypeConverter.convertToPersistentType returned illegal type: " + + " Converter: " + converter + "\n" + + " Type Returned: " + persistedTypeEnum + "\n" + + " From Value: " + value); } } } diff --git a/source/test-resources/alfresco/audit/alfresco-audit-test-bad-03.xml b/source/test-resources/alfresco/audit/alfresco-audit-test-bad-03.xml index fce198634e..7f1633af61 100644 --- a/source/test-resources/alfresco/audit/alfresco-audit-test-bad-03.xml +++ b/source/test-resources/alfresco/audit/alfresco-audit-test-bad-03.xml @@ -8,6 +8,10 @@ xsi:schemaLocation="http://www.alfresco.org/repo/audit/model/3.2 alfresco-audit-3.2.xsd" > + + + + diff --git a/source/test-resources/alfresco/audit/alfresco-audit-test-bad-04.xml b/source/test-resources/alfresco/audit/alfresco-audit-test-bad-04.xml index a0e47761a7..9ad6396588 100644 --- a/source/test-resources/alfresco/audit/alfresco-audit-test-bad-04.xml +++ b/source/test-resources/alfresco/audit/alfresco-audit-test-bad-04.xml @@ -8,10 +8,14 @@ xsi:schemaLocation="http://www.alfresco.org/repo/audit/model/3.2 alfresco-audit-3.2.xsd" > + + + + - + diff --git a/source/test-resources/alfresco/audit/alfresco-audit-test-bad-05.xml b/source/test-resources/alfresco/audit/alfresco-audit-test-bad-05.xml index 752fea97b4..e1e39b7070 100644 --- a/source/test-resources/alfresco/audit/alfresco-audit-test-bad-05.xml +++ b/source/test-resources/alfresco/audit/alfresco-audit-test-bad-05.xml @@ -8,6 +8,10 @@ xsi:schemaLocation="http://www.alfresco.org/repo/audit/model/3.2 alfresco-audit-3.2.xsd" > + + + + diff --git a/source/test-resources/alfresco/audit/alfresco-audit-test-bad-06.xml b/source/test-resources/alfresco/audit/alfresco-audit-test-bad-06.xml new file mode 100644 index 0000000000..bd1f47ab0b --- /dev/null +++ b/source/test-resources/alfresco/audit/alfresco-audit-test-bad-06.xml @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/source/test-resources/alfresco/audit/alfresco-audit-test.xml b/source/test-resources/alfresco/audit/alfresco-audit-test.xml index 2dbb536cde..adcfd95cc7 100644 --- a/source/test-resources/alfresco/audit/alfresco-audit-test.xml +++ b/source/test-resources/alfresco/audit/alfresco-audit-test.xml @@ -64,9 +64,22 @@ - - - + + + + + + + + + + + + + + + +