Fixes for cm:auditable and properties fallout

- Deleted nodes were getting cm:auditable aspect
 - Added Savepoint around try-catch logic for store-move code (secondary PostgreSQL fallout from above)
 - Use cached Node properties (not query) as starting point when modifying node properties


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

source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java

@@ -956,7 +956,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         node.setId(id);
         
         Set<QName> nodeAspects = null;
-        if (addAuditableAspect)
+        if (addAuditableAspect && !deleted)
         {
             Long auditableAspectQNameId = qnameDAO.getOrCreateQName(ContentModel.ASPECT_AUDITABLE).getFirst();
             insertNodeAspect(id, auditableAspectQNameId);
@@ -1269,12 +1269,15 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         
         // Do the update
         int count = 0;
+        Savepoint savepoint = controlDAO.createSavepoint("updateNode");
         try
         {
             count = updateNode(nodeUpdate);
+            controlDAO.releaseSavepoint(savepoint);
         }
         catch (Throwable e)
         {
+            controlDAO.rollbackToSavepoint(savepoint);
             NodeRef targetNodeRef = nodeUpdate.getNodeRef();
             // Wipe the node ID from the caches just in case we have stale caches
             // The TransactionalCache will propagate removals to the shared cache on rollback
@@ -1569,24 +1572,16 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         // Remove sys:referenceable
         ReferenceablePropertiesEntity.removeReferenceableProperties(node, newProps);
 
-        // Get the old properties in the raw format, attempting a shortcut for new nodes
+        // Get the cached properties
-        Map<NodePropertyKey, NodePropertyValue> oldPropsRaw = null;
+        Map<QName, Serializable> oldPropsCached = getNodePropertiesCached(nodeId);
-        Map<QName, Serializable> oldProps = propertiesCache.getValue(nodeId);
+        Map<QName, Serializable> oldProps = new HashMap<QName, Serializable>(oldPropsCached);
-        if (oldProps != null && oldProps.isEmpty())
+        // If this is an add-only operation, remove any properties we are not interested in
+        if (isAddOnly)
         {
-            // Don't requery
+            oldProps.keySet().retainAll(newProps.keySet());
-            oldPropsRaw = Collections.emptyMap();
-            isAddOnly = false;
         }
-        else
+        // Convert to a raw format for comparison
-        {
+        Map<NodePropertyKey, NodePropertyValue> oldPropsRaw = nodePropertyHelper.convertToPersistentProperties(oldProps);
-            oldPropsRaw = selectNodeProperties(nodeId);
-        }
-        
-        // Determine which properties we are interested in.  For addition of properties, we only
-        // need the old properties for the QNames being added.  For complete setting of properties,
-        // we need the full set of old properties.
-        Set<Long> qnameIdsOfInterest = qnameDAO.convertQNamesToIds(newProps.keySet(), true);
         
         // Get new property raw values
         Map<NodePropertyKey, NodePropertyValue> newPropsRaw = nodePropertyHelper.convertToPersistentProperties(newProps);
@@ -1632,68 +1627,55 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
                         Long newContentDataId = contentDataDAO.createContentData(newContentData).getFirst();
                         newPropValue = new NodePropertyValue(DataTypeDefinition.CONTENT, new ContentDataId(newContentDataId));
                         propsToAdd.put(key, newPropValue);
+                        newPropsRaw.put(
+                                key,
+                                new NodePropertyValue(
+                                        DataTypeDefinition.CONTENT,
+                                        new ContentDataWithId(newContentData, newContentDataId)));
                     }
                 }
                 continue;
             case LEFT_ONLY:
                 // Only present in old props: must not be added
                 propsToAdd.remove(key);
-                // Handle the fact that this might be a QName we are not interested in
-                if (isAddOnly && !qnameIdsOfInterest.contains(key.getQnameId()))
-                {
-                    // We are adding properties and this is not a property type of interest
-                    propsToDelete.remove(key);
-                    continue;
-                }
                 // Handle deleted content
                 if (isContent)
                 {
                     // The old values will be an ID-based ContentData reference
                     NodePropertyValue valueToDelete = propsToDelete.get(key);
-                    Long contentDataId = (Long) valueToDelete.getValue(DataTypeDefinition.CONTENT);
+                    ContentDataWithId contentDataWithId = (ContentDataWithId) valueToDelete.getValue(DataTypeDefinition.CONTENT);
-                    if (contentDataId != null)
+                    if (contentDataWithId != null)
                     {
+                        Long contentDataId = contentDataWithId.getId();
                         contentDataDAO.deleteContentData(contentDataId);
                     }
                 }
                 continue;
-                // Fall through for content dereferencing
             case NOT_EQUAL:
                 // Value has changed: remove and add
-                // Handle changed content.  We may have equal ContentData values here but the ID-ContentData
-                //    mix will always turn up NOT_EQUAL, hence the double-check
                 if (isContent)
                 {
                     // The old values will be an ID-based ContentData reference
                     NodePropertyValue valueToDelete = propsToDelete.get(key);
-                    Long contentDataIdToDelete = (Long) valueToDelete.getValue(DataTypeDefinition.CONTENT);
+                    ContentDataWithId contentDataWithId = (ContentDataWithId) valueToDelete.getValue(DataTypeDefinition.CONTENT);
-                    ContentData contentDataToDelete =
+                    if (contentDataWithId != null)
-                            (contentDataIdToDelete == null)
+                    {
-                            ? null
+                        Long contentDataId = contentDataWithId.getId();
-                            : contentDataDAO.getContentData(contentDataIdToDelete).getSecond();
+                        contentDataDAO.deleteContentData(contentDataId);
-                    // The new value will NOT be an ID-based reference
+                    }
+                    // The new value needs conversion to the ID-based ContentData reference
                     NodePropertyValue newPropValue = propsToAdd.get(key);
                     ContentData newContentData = (ContentData) newPropValue.getValue(DataTypeDefinition.CONTENT);
-                    // Are they actually different?
+                    if (newContentData != null)
-                    if (EqualsHelper.nullSafeEquals(contentDataToDelete, newContentData))
                     {
-                        // The are the same, so don't do anything
+                        Long newContentDataId = contentDataDAO.createContentData(newContentData).getFirst();
-                        propsToDelete.remove(key);
+                        newPropValue = new NodePropertyValue(DataTypeDefinition.CONTENT, new ContentDataId(newContentDataId));
-                        propsToAdd.remove(key);
+                        propsToAdd.put(key, newPropValue);
-                    }
+                        newPropsRaw.put(
-                    else
+                                key,
-                    {
+                                new NodePropertyValue(
-                        // The ContentData values are different
+                                        DataTypeDefinition.CONTENT,
-                        if (contentDataIdToDelete != null)
+                                        new ContentDataWithId(newContentData, newContentDataId)));
-                        {
-                            contentDataDAO.deleteContentData(contentDataIdToDelete);
-                        }
-                        if (newContentData != null)
-                        {
-                            Long newContentDataId = contentDataDAO.createContentData(newContentData).getFirst();
-                            newPropValue = new NodePropertyValue(DataTypeDefinition.CONTENT, new ContentDataId(newContentDataId));
-                            propsToAdd.put(key, newPropValue);
-                        }
                     }
                 }
                 continue;
@@ -1736,7 +1718,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
             if (isAddOnly)
             {
                 // Combine the old and new properties
-                propsToCache = nodePropertyHelper.convertToPublicProperties(oldPropsRaw);
+                propsToCache = oldPropsCached;
                 propsToCache.putAll(newProps);
             }
             else

source/java/org/alfresco/repo/domain/node/NodePropertyValue.java

@@ -278,13 +278,13 @@ public class NodePropertyValue implements Cloneable, Serializable
             @Override
             protected ValueType getPersistedType(Serializable value)
             {
-                if (value instanceof Long)
+                if (value instanceof ContentData)
                 {
-                    return ValueType.LONG;
+                    return ValueType.SERIALIZABLE;
                 }
                 else
                 {
-                    return ValueType.STRING;
+                    throw new RuntimeException("ContentData persistence must be by ContentDataId.");
                 }
             }
 
@@ -295,6 +295,11 @@ public class NodePropertyValue implements Cloneable, Serializable
                 {
                     return value;
                 }
+                else if (value instanceof String)
+                {
+                    logger.warn("Content URL converter has not run to completion: " + value);
+                    return DefaultTypeConverter.INSTANCE.convert(ContentData.class, value);
+                }
                 else
                 {
                     return DefaultTypeConverter.INSTANCE.convert(ContentData.class, value);