From e7dff0383aea13f77dddd47178ee9155e1d1c32f Mon Sep 17 00:00:00 2001 From: Derek Hulley Date: Tue, 13 Jul 2010 15:22:53 +0000 Subject: [PATCH] Fix SAIL-389 (SAIL-294): NodeDAO: single-valued, d:any properties don't handle increasing array values - Incorrect translation of raw values back to Serializable for cache purposes - Addition of Savepoint around alf_node insert git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@21136 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../repo/domain/node/AbstractNodeDAOImpl.java | 5 ++- .../repo/domain/node/NodePropertyHelper.java | 43 +++++++++++++------ .../repo/domain/node/ibatis/NodeDAOImpl.java | 8 ++-- .../repo/node/BaseNodeServiceTest.java | 34 +++++++++++++++ 4 files changed, 74 insertions(+), 16 deletions(-) diff --git a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java index b38b3bafd6..487b7ebf5c 100644 --- a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java @@ -938,13 +938,16 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO } Long id = null; + Savepoint savepoint = controlDAO.createSavepoint("newNodeImpl"); try { // First try a straight insert and risk the constraint violation if the node exists id = insertNode(node); + controlDAO.releaseSavepoint(savepoint); } catch (Throwable e) { + controlDAO.rollbackToSavepoint(savepoint); // This is probably because there is an existing node. We can handle existing deleted nodes. NodeRef targetNodeRef = node.getNodeRef(); NodeEntity deletedNode = selectNodeByNodeRef(targetNodeRef, true); // Only look for deleted nodes @@ -3071,7 +3074,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO protected abstract Map selectNodeProperties(Long nodeId); protected abstract Map selectNodeProperties(Long nodeId, Set qnameIds); protected abstract int deleteNodeProperties(Long nodeId, Set qnameIds); - protected abstract void deleteNodeProperties(Long nodeId, List propKeys); + protected abstract int deleteNodeProperties(Long nodeId, List propKeys); protected abstract void insertNodeProperties(Long nodeId, Map persistableProps); protected abstract Set selectNodeAspectIds(Long nodeId); protected abstract void insertNodeAspect(Long nodeId, Long qnameId); diff --git a/source/java/org/alfresco/repo/domain/node/NodePropertyHelper.java b/source/java/org/alfresco/repo/domain/node/NodePropertyHelper.java index d1a6356cff..98d59f1662 100644 --- a/source/java/org/alfresco/repo/domain/node/NodePropertyHelper.java +++ b/source/java/org/alfresco/repo/domain/node/NodePropertyHelper.java @@ -191,9 +191,10 @@ public class NodePropertyHelper // Check that multi-valued properties are supported if the property is a collection if (!isMultiValued) { - throw new DictionaryException("A single-valued property of this type may not be a collection: \n" - + " Property: " + propertyDef + "\n" + " Type: " + propertyTypeQName + "\n" + " Value: " - + value); + throw new DictionaryException("A single-valued property of this type may not be a collection: \n" + + " Property: " + propertyDef + "\n" + + " Type: " + propertyTypeQName + "\n" + + " Value: " + value); } // We have an allowable collection. @SuppressWarnings("unchecked") @@ -327,8 +328,7 @@ public class NodePropertyHelper } public Serializable getPublicProperty( - Map propertyValues, + Map propertyValues, QName propertyQName) { // Get the qname ID @@ -409,16 +409,24 @@ public class NodePropertyHelper // There is more than one value so the list indexes need to be collapsed collapsedValue = collapsePropertiesWithSameQName(currentPropertyDef, scratch); } + boolean forceCollection = false; // If the property is multi-valued then the output property must be a collection if (currentPropertyDef != null && currentPropertyDef.isMultiValued()) { - if (collapsedValue != null && !(collapsedValue instanceof Collection)) - { - // Can't use Collections.singletonList: ETHREEOH-1172 - ArrayList collection = new ArrayList(1); - collection.add(collapsedValue); - collapsedValue = collection; - } + forceCollection = true; + } + else if (scratch.size() == 1 && scratch.firstKey().getListIndex().intValue() > -1) + { + // This is to handle cases of collections where the property is d:any but not + // declared as multiple. + forceCollection = true; + } + if (forceCollection && collapsedValue != null && !(collapsedValue instanceof Collection)) + { + // Can't use Collections.singletonList: ETHREEOH-1172 + ArrayList collection = new ArrayList(1); + collection.add(collapsedValue); + collapsedValue = collection; } // Store the value propertyMap.put(currentQName, collapsedValue); @@ -534,12 +542,23 @@ public class NodePropertyHelper if (propertyValuesSize == 0) { // Nothing to do + return value; } + Integer listIndex = null; for (Map.Entry entry : propertyValues.entrySet()) { NodePropertyKey propertyKey = entry.getKey(); NodePropertyValue propertyValue = entry.getValue(); + if (listIndex == null) + { + listIndex = propertyKey.getListIndex(); + } + else if (!listIndex.equals(propertyKey.getListIndex())) + { + throw new IllegalStateException("Expecting to collapse properties with same list index: " + propertyValues); + } + if (propertyValuesSize == 1 && (propertyDef == null || !propertyDef.getDataType().getName().equals(DataTypeDefinition.MLTEXT))) { diff --git a/source/java/org/alfresco/repo/domain/node/ibatis/NodeDAOImpl.java b/source/java/org/alfresco/repo/domain/node/ibatis/NodeDAOImpl.java index 81feee96e1..a8375370c0 100644 --- a/source/java/org/alfresco/repo/domain/node/ibatis/NodeDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/node/ibatis/NodeDAOImpl.java @@ -468,14 +468,14 @@ public class NodeDAOImpl extends AbstractNodeDAOImpl } @Override - protected void deleteNodeProperties(Long nodeId, List propKeys) + protected int deleteNodeProperties(Long nodeId, List propKeys) { Assert.notNull(nodeId, "Must have 'nodeId'"); Assert.notNull(nodeId, "Must have 'propKeys'"); if (propKeys.size() == 0) { - return; + return 0; } NodePropertyEntity prop = new NodePropertyEntity(); @@ -483,18 +483,20 @@ public class NodeDAOImpl extends AbstractNodeDAOImpl prop.setNodeId(nodeId); startBatch(); + int count = 0; try { for (NodePropertyKey propKey : propKeys) { prop.setKey(propKey); - template.delete(DELETE_NODE_PROPERTIES, prop); + count += template.delete(DELETE_NODE_PROPERTIES, prop); } } finally { executeBatch(); } + return count; } @Override diff --git a/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java b/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java index 4b52a90fc3..dae6c274a2 100644 --- a/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java +++ b/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java @@ -1892,6 +1892,40 @@ public abstract class BaseNodeServiceTest extends BaseSpringTest assertEquals("MLText collection didn't come back correctly.", mlTextCollection, mlTextCollectionCheck); } + /** + * Ensures that d:any types are handled correctly when adding values + */ + public void testMultivaluedSerializable() throws Exception + { + ArrayList listProp = new ArrayList(); + + listProp.clear(); + nodeService.addProperties( + rootNodeRef, + Collections.singletonMap(PROP_QNAME_ANY_PROP_MULTIPLE, (Serializable) listProp)); + listProp.add("ONE"); + nodeService.addProperties( + rootNodeRef, + Collections.singletonMap(PROP_QNAME_ANY_PROP_MULTIPLE, (Serializable) listProp)); + listProp.add("TWO"); + nodeService.addProperties( + rootNodeRef, + Collections.singletonMap(PROP_QNAME_ANY_PROP_MULTIPLE, (Serializable) listProp)); + + listProp.clear(); + nodeService.addProperties( + rootNodeRef, + Collections.singletonMap(PROP_QNAME_ANY_PROP_SINGLE, (Serializable) listProp)); + listProp.add("ONE"); + nodeService.addProperties( + rootNodeRef, + Collections.singletonMap(PROP_QNAME_ANY_PROP_SINGLE, (Serializable) listProp)); + listProp.add("TWO"); + nodeService.addProperties( + rootNodeRef, + Collections.singletonMap(PROP_QNAME_ANY_PROP_SINGLE, (Serializable) listProp)); + } + /** * Checks that the {@link ContentModel#ASPECT_REFERENCEABLE referencable} properties * are present