diff --git a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java index b6289162cc..687380d3d0 100644 --- a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java @@ -1631,34 +1631,36 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO // Remove sys:referenceable ReferenceablePropertiesEntity.removeReferenceableProperties(node, newProps); - // Get the cached properties - Map oldPropsCached = getNodePropertiesCached(nodeId); + // Load the current properties. + // This means that we have to go to the DB during cold-write operations, + // but usually a write occurs after a node has been fetched of viewed in + // some way by the client code. Loading the existing properties has the + // advantage that the differencing code can eliminate unnecessary writes + // completely. + Map oldPropsCached = getNodePropertiesCached(nodeId); // Keep pristine for caching Map oldProps = new HashMap(oldPropsCached); - // If this is an add-only operation, remove any properties we are not interested in + // If we're adding, remove current properties that are not of interest if (isAddOnly) { oldProps.keySet().retainAll(newProps.keySet()); } - // Convert to a raw format for comparison - Map oldPropsRaw = nodePropertyHelper.convertToPersistentProperties(oldProps); - - // Get new property raw values + // We need to convert the new properties to our internally-used format, + // which is compatible with model i.e. people may have passed in data + // which needs to be converted to a model-compliant format. We do this + // before comparisons to avoid false negatives. Map newPropsRaw = nodePropertyHelper.convertToPersistentProperties(newProps); - - // Copy for modification - Map propsToDelete = new HashMap(oldPropsRaw); - Map propsToAdd = new HashMap(newPropsRaw); - - // Compare these fine-grained properties - Map persistableDiff = EqualsHelper.getMapComparison( - propsToDelete, - propsToAdd); - // Add or remove properties as we go - for (Map.Entry entry : persistableDiff.entrySet()) + newProps = nodePropertyHelper.convertToPublicProperties(newPropsRaw); + // Now find out what's changed + Map diff = EqualsHelper.getMapComparison( + oldProps, + newProps); + // Keep track of properties to delete and add + Set propsToDelete = new HashSet(oldProps.size()*2); + Map propsToAdd = new HashMap(newProps.size() * 2); + Set contentQNamesToDelete = new HashSet(5); + for (Map.Entry entry : diff.entrySet()) { - NodePropertyKey key = entry.getKey(); - - QName qname = qnameDAO.getQName(key.getQnameId()).getSecond(); + QName qname = entry.getKey(); PropertyDefinition removePropDef = dictionaryService.getProperty(qname); boolean isContent = (removePropDef != null && @@ -1666,135 +1668,102 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO switch (entry.getValue()) { - case NULL: - case EQUAL: - // The entries are the same - propsToDelete.remove(key); - propsToAdd.remove(key); - continue; - case RIGHT_ONLY: - // Only in new props: add - propsToDelete.remove(key); - // Handle new content - if (isContent) - { - // The new value needs conversion to the ID-based ContentData reference - NodePropertyValue newPropValue = propsToAdd.get(key); - ContentData newContentData = (ContentData) newPropValue.getValue(DataTypeDefinition.CONTENT); - if (newContentData != null) + case EQUAL: + // Ignore + break; + case LEFT_ONLY: + // Not in the new properties + propsToDelete.add(qname); + if (isContent) { + contentQNamesToDelete.add(qname); + } + break; + case NOT_EQUAL: + // Must remove from the LHS + propsToDelete.add(qname); + if (isContent) + { + contentQNamesToDelete.add(qname); + } + // Fall through to load up the RHS + case RIGHT_ONLY: + // We're adding this + Serializable value = newProps.get(qname); + if (isContent && value != null) + { + ContentData newContentData = (ContentData) value; 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))); + value = new ContentDataWithId(newContentData, newContentDataId); } - } - continue; - case LEFT_ONLY: - // Only present in old props: must not be added - propsToAdd.remove(key); - // Handle deleted content - if (isContent) - { - // The old values will be an ID-based ContentData reference - NodePropertyValue valueToDelete = propsToDelete.get(key); - ContentDataWithId contentDataWithId = (ContentDataWithId) valueToDelete.getValue(DataTypeDefinition.CONTENT); - if (contentDataWithId != null) - { - Long contentDataId = contentDataWithId.getId(); - contentDataDAO.deleteContentData(contentDataId); - } - } - continue; - case NOT_EQUAL: - // Value has changed: remove and add - if (isContent) - { - // The old values will be an ID-based ContentData reference - NodePropertyValue valueToDelete = propsToDelete.get(key); - ContentDataWithId contentDataWithId = (ContentDataWithId) valueToDelete.getValue(DataTypeDefinition.CONTENT); - if (contentDataWithId != null) - { - Long contentDataId = contentDataWithId.getId(); - contentDataDAO.deleteContentData(contentDataId); - } - // The new value needs conversion to the ID-based ContentData reference - NodePropertyValue newPropValue = propsToAdd.get(key); - ContentData newContentData = (ContentData) newPropValue.getValue(DataTypeDefinition.CONTENT); - if (newContentData != null) - { - 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; - default: - throw new IllegalStateException("Unknown MapValueComparison: " + entry.getValue()); + propsToAdd.put(qname, value); + break; + default: + throw new IllegalStateException("Unknown MapValueComparison: " + entry.getValue()); } } - try - { - // Remove by key - List propKeysToDeleteList = new ArrayList(propsToDelete.keySet()); - int deleted = deleteNodeProperties(nodeId, propKeysToDeleteList); - if (deleted != propKeysToDeleteList.size()) - { - // The translation of the left-hand map didn't match the database - // This happens when we use d:any and switch from a simple value to a collection: an edge case! foreach - throw new DataIntegrityViolationException("Only deleted " + deleted + "/" + propKeysToDeleteList.size()); - } - - // Add by key-value - insertNodeProperties(nodeId, propsToAdd); - } - catch (RuntimeException e) - { - // Don't trust the properties cache for the node - propertiesCache.removeByKey(nodeId); - // Focused error - throw new AlfrescoRuntimeException( - "Failed to write property deltas: \n" + - " Node: " + nodeId + "\n" + - " Old: " + oldProps + "\n" + - " New: " + newProps + "\n" + - " Old Raw: " + oldPropsRaw + "\n" + - " New Raw: " + newPropsRaw + "\n" + - " Diff: " + persistableDiff + "\n" + - " Delete Tried: " + propsToDelete.keySet() + "\n" + - " Add Tried: " + propsToAdd, - e); - } - boolean updated = propsToDelete.size() > 0 || propsToAdd.size() > 0; // Touch to bring into current txn if (updated) { - // Fix properties up w.r.t. types, etc - newProps = nodePropertyHelper.convertToPublicProperties(newPropsRaw); + // Clean up content properties + try + { + if (contentQNamesToDelete.size() > 0) + { + Set contentQNameIdsToDelete = qnameDAO.convertQNamesToIds(contentQNamesToDelete, false); + contentDataDAO.deleteContentDataForNode(nodeId, contentQNameIdsToDelete); + } + } + catch (Throwable e) + { + throw new AlfrescoRuntimeException( + "Failed to delete content properties: \n" + + " Node: " + nodeId + "\n" + + " Delete Tried: " + contentQNamesToDelete, + e); + } + + try + { + // Apply deletes + Set propQNameIdsToDelete = qnameDAO.convertQNamesToIds(propsToDelete, true); + deleteNodeProperties(nodeId, propQNameIdsToDelete); + // Now create the raw properties for adding + newPropsRaw = nodePropertyHelper.convertToPersistentProperties(propsToAdd); + insertNodeProperties(nodeId, newPropsRaw); + } + catch (Throwable e) + { + // Don't trust the properties cache for the node + propertiesCache.removeByKey(nodeId); + // Focused error + throw new AlfrescoRuntimeException( + "Failed to write property deltas: \n" + + " Node: " + nodeId + "\n" + + " Old: " + oldProps + "\n" + + " New: " + newProps + "\n" + + " Diff: " + diff + "\n" + + " Delete Tried: " + propsToDelete + "\n" + + " Add Tried: " + propsToAdd, + e); + } + // Build the properties to cache based on whether this is an append or replace Map propsToCache = null; if (isAddOnly) { // Combine the old and new properties propsToCache = oldPropsCached; - propsToCache.putAll(newProps); + propsToCache.putAll(propsToAdd); } else { // Replace old properties propsToCache = newProps; + propsToCache.putAll(propsToAdd); // Ensure correct types } // Update cache setNodePropertiesCached(nodeId, propsToCache); diff --git a/source/java/org/alfresco/repo/domain/node/NodePropertyHelper.java b/source/java/org/alfresco/repo/domain/node/NodePropertyHelper.java index b1605846f3..5df944b736 100644 --- a/source/java/org/alfresco/repo/domain/node/NodePropertyHelper.java +++ b/source/java/org/alfresco/repo/domain/node/NodePropertyHelper.java @@ -578,8 +578,9 @@ public class NodePropertyHelper // Get the local entry value Serializable entryValue = makeSerializableValue(propertyDef, propertyValue); - // A default locale indicates a simple value i.e. the entry represents the whole value. - if (isDefaultLocale) + // A default locale indicates a simple value i.e. the entry represents the whole value, + // unless the dictionary specifically declares it to be d:mltext + if (isDefaultLocale && !isMLText) { // Check and warn if there are other values if (propertyValuesSize > 1) diff --git a/source/java/org/alfresco/repo/domain/node/NodePropertyValue.java b/source/java/org/alfresco/repo/domain/node/NodePropertyValue.java index a76d978d80..a0a017c588 100644 --- a/source/java/org/alfresco/repo/domain/node/NodePropertyValue.java +++ b/source/java/org/alfresco/repo/domain/node/NodePropertyValue.java @@ -142,6 +142,10 @@ public class NodePropertyValue implements Cloneable, Serializable { return ((ContentDataId)value).getId(); } + else if (value instanceof ContentDataWithId) + { + return ((ContentDataWithId)value).getId(); + } else { return DefaultTypeConverter.INSTANCE.convert(Long.class, value); @@ -592,10 +596,6 @@ public class NodePropertyValue implements Cloneable, Serializable { return ValueType.DATE; } - else if (value instanceof ContentData) - { - return ValueType.CONTENT; - } else if (value instanceof NodeRef) { return ValueType.NODEREF; @@ -632,6 +632,14 @@ public class NodePropertyValue implements Cloneable, Serializable { return ValueType.CONTENT_DATA_ID; } + else if (value instanceof ContentDataWithId) + { + return ValueType.CONTENT_DATA_ID; + } + else if (value instanceof ContentData) + { + return ValueType.CONTENT; + } else { // type is not recognised as belonging to any particular slot diff --git a/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java b/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java index 233059ce26..eec1123ee4 100644 --- a/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java +++ b/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java @@ -1909,7 +1909,7 @@ public abstract class BaseNodeServiceTest extends BaseSpringTest QName.createQName("pathA"), TYPE_QNAME_TEST_MANY_PROPERTIES).getChildRef(); - for (int inc = 0; inc < 3; inc++) + for (int inc = 0; inc < 5; inc++) { System.out.println("----------------------------------------------"); int collectionSize = (int) Math.pow(10, inc);