diff --git a/config/alfresco/ibatis/org.hibernate.dialect.Dialect/node-common-SqlMap.xml b/config/alfresco/ibatis/org.hibernate.dialect.Dialect/node-common-SqlMap.xml index 6c2adb44ac..ce0d3412df 100644 --- a/config/alfresco/ibatis/org.hibernate.dialect.Dialect/node-common-SqlMap.xml +++ b/config/alfresco/ibatis/org.hibernate.dialect.Dialect/node-common-SqlMap.xml @@ -82,6 +82,7 @@ --> + diff --git a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java index b2abc670d5..e2553d0fd3 100644 --- a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java @@ -562,6 +562,9 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO } } + /** + * @return Returns a new transaction or an existing one if already active + */ private TransactionEntity getCurrentTransaction() { TransactionEntity txn = AlfrescoTransactionSupport.getResource(KEY_TRANSACTION); @@ -1216,6 +1219,9 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO } } + // Need the child node's name here in case it gets removed + final String childNodeName = (String) getNodeProperty(childNodeId, ContentModel.PROP_NAME); + // First attempt to move the node, which may rollback to a savepoint Node newChildNode = childNode; // Store @@ -1281,11 +1287,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO // Because we are retrying in-transaction i.e. absorbing exceptions, we need a Savepoint Savepoint savepoint = controlDAO.createSavepoint("DuplicateChildNodeNameException"); // We use the child node's UUID if there is no cm:name - String childNodeName = (String) getNodeProperty(childNodeId, ContentModel.PROP_NAME); - if (childNodeName == null) - { - childNodeName = childNode.getUuid(); - } + String childNodeNameToUse = childNodeName == null ? childNode.getUuid() : childNodeName; try { @@ -1294,7 +1296,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO newParentNodeId, assocTypeQName, assocQName, - childNodeName); + childNodeNameToUse); controlDAO.releaseSavepoint(savepoint); return updated; } @@ -1565,7 +1567,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO Long optionalOldSharedAlcIdInAdditionToNull, Long newSharedAclId) { - Long txnId = getCurrentTransactionId(); + Long txnId = getCurrentTransaction().getId(); updatePrimaryChildrenSharedAclId( txnId, primaryParentNodeId, @@ -2101,6 +2103,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO throw new DataIntegrityViolationException("Invalid node ID: " + nodeId); } Map cachedProperties = cacheEntry.getSecond(); + // Need to return a harmlessly mutable map Map properties = copyPropertiesAgainstModification(cachedProperties); // Done return properties; @@ -2153,7 +2156,27 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO public Pair> findByKey(Long nodeId) { - Map propsRaw = selectNodeProperties(nodeId); + NodeVersionKey nodeVersionKey = getNodeNotNull(nodeId).getNodeVersionKey(); + Map> propsRawByNodeVersionKey = selectNodeProperties(nodeId); + // Check the node Txn ID for mismatch + Map propsRaw = propsRawByNodeVersionKey.get(nodeVersionKey); + if (propsRaw == null) + { + // Didn't find a match. Is this because there are none? + if (propsRawByNodeVersionKey.size() == 0) + { + // This is OK. The node has no properties + propsRaw = Collections.emptyMap(); + } + else + { + // We found properties associated with a different node ID and txn + invalidateNodeCaches(nodeId); + throw new DataIntegrityViolationException( + "Detected stale node entry: " + nodeVersionKey + + " (now " + propsRawByNodeVersionKey.keySet() + ")"); + } + } // Convert to public properties Map props = nodePropertyHelper.convertToPublicProperties(propsRaw); // Done @@ -2351,9 +2374,27 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO public Pair> findByKey(Long nodeId) { - Set nodeAspectQNameIds = selectNodeAspectIds(nodeId); - // Convert to QNames - Set nodeAspectQNames = qnameDAO.convertIdsToQNames(nodeAspectQNameIds); + NodeVersionKey nodeVersionKey = getNodeNotNull(nodeId).getNodeVersionKey(); + Set nodeIds = Collections.singleton(nodeId); + Map> nodeAspectQNameIdsByVersionKey = selectNodeAspects(nodeIds); + Set nodeAspectQNames = nodeAspectQNameIdsByVersionKey.get(nodeVersionKey); + if (nodeAspectQNames == null) + { + // Didn't find a match. Is this because there are none? + if (nodeAspectQNameIdsByVersionKey.size() == 0) + { + // This is OK. The node has no properties + nodeAspectQNames = Collections.emptySet(); + } + else + { + // We found properties associated with a different node ID and txn + invalidateNodeCaches(nodeId); + throw new DataIntegrityViolationException( + "Detected stale node entry: " + nodeVersionKey + + " (now " + nodeAspectQNameIdsByVersionKey.keySet() + ")"); + } + } // Done return new Pair>(nodeId, Collections.unmodifiableSet(nodeAspectQNames)); } @@ -3619,9 +3660,11 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO // Get the nodes SortedSet aspectNodeIds = new TreeSet(); SortedSet propertiesNodeIds = new TreeSet(); + Map nodeVersionKeysFromCache = new HashMap(nodes.size()*2); // Keep for quick lookup for (Node node : nodes) { Long nodeId = node.getId(); + NodeVersionKey nodeVersionKey = node.getNodeVersionKey(); nodesCache.setValue(nodeId, node); if (propertiesCache.getValue(nodeId) == null) { @@ -3631,6 +3674,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO { aspectNodeIds.add(nodeId); } + nodeVersionKeysFromCache.put(nodeId, nodeVersionKey); } if(logger.isDebugEnabled()) @@ -3639,14 +3683,13 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO logger.debug("Pre-loaded " + propertiesNodeIds.size() + " aspects"); } - List nodeAspects = selectNodeAspects(aspectNodeIds); - for (NodeAspectsEntity nodeAspect : nodeAspects) + Map> nodeAspects = selectNodeAspects(aspectNodeIds); + for (Map.Entry> entry : nodeAspects.entrySet()) { - Long nodeId = nodeAspect.getNodeId(); - List qnameIds = nodeAspect.getAspectQNameIds(); - HashSet qnameIdsSet = new HashSet(qnameIds); - Set qnames = qnameDAO.convertIdsToQNames(qnameIdsSet); - aspectsCache.setValue(nodeId, qnames); + NodeVersionKey nodeVersionKeyFromDb = entry.getKey(); + Long nodeId = nodeVersionKeyFromDb.getNodeId(); + Set qnames = entry.getValue(); + setNodeAspectsCached(nodeId, qnames); aspectNodeIds.remove(nodeId); } // Cache the absence of aspects too! @@ -3655,13 +3698,13 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO aspectsCache.setValue(nodeId, Collections.emptySet()); } - Map> propsByNodeId = selectNodeProperties(propertiesNodeIds); - for (Map.Entry> entry : propsByNodeId.entrySet()) + Map> propsByNodeId = selectNodeProperties(propertiesNodeIds); + for (Map.Entry> entry : propsByNodeId.entrySet()) { - Long nodeId = entry.getKey(); + Long nodeId = entry.getKey().getNodeId(); Map propertyValues = entry.getValue(); Map props = nodePropertyHelper.convertToPublicProperties(propertyValues); - propertiesCache.setValue(nodeId, props); + setNodePropertiesCached(nodeId, props); } } @@ -3847,14 +3890,13 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO protected abstract NodeEntity selectNodeByNodeRef(NodeRef nodeRef, Boolean deleted); protected abstract List selectNodesByUuids(Long storeId, SortedSet uuids); protected abstract List selectNodesByIds(SortedSet ids); - protected abstract Map> selectNodeProperties(Set nodeIds); - protected abstract List selectNodeAspects(Set nodeIds); - protected abstract Map selectNodeProperties(Long nodeId); - protected abstract Map selectNodeProperties(Long nodeId, Set qnameIds); + protected abstract Map> selectNodeProperties(Set nodeIds); + protected abstract Map> selectNodeProperties(Long nodeId); + protected abstract Map> selectNodeProperties(Long nodeId, Set qnameIds); protected abstract int deleteNodeProperties(Long nodeId, Set qnameIds); protected abstract int deleteNodeProperties(Long nodeId, List propKeys); protected abstract void insertNodeProperties(Long nodeId, Map persistableProps); - protected abstract Set selectNodeAspectIds(Long nodeId); + protected abstract Map> selectNodeAspects(Set nodeIds); protected abstract void insertNodeAspect(Long nodeId, Long qnameId); protected abstract int deleteNodeAspects(Long nodeId, Set qnameIds); protected abstract void selectNodesWithAspects( diff --git a/source/java/org/alfresco/repo/domain/node/Node.java b/source/java/org/alfresco/repo/domain/node/Node.java index 9c262ac02a..804969b284 100644 --- a/source/java/org/alfresco/repo/domain/node/Node.java +++ b/source/java/org/alfresco/repo/domain/node/Node.java @@ -28,6 +28,11 @@ import org.alfresco.util.Pair; */ public interface Node extends NodeIdAndAclId { + /** + * Helper method to get a key that includes the node and its current version number + */ + public NodeVersionKey getNodeVersionKey(); + /** * Helper method to force the instance to be read-only */ diff --git a/source/java/org/alfresco/repo/domain/node/NodeAspectsEntity.java b/source/java/org/alfresco/repo/domain/node/NodeAspectsEntity.java index 680fec9b76..1cabf8a418 100644 --- a/source/java/org/alfresco/repo/domain/node/NodeAspectsEntity.java +++ b/source/java/org/alfresco/repo/domain/node/NodeAspectsEntity.java @@ -28,7 +28,8 @@ import java.util.List; */ public class NodeAspectsEntity { - private Long nodeId; + private Long nodeId; + private Long nodeVersion; private List aspectQNameIds; /** Carries data for queries */ @@ -47,6 +48,7 @@ public class NodeAspectsEntity StringBuilder sb = new StringBuilder(512); sb.append("NodeAspectsEntity") .append(", nodeId=").append(nodeId) + .append(", nodeVersion=").append(nodeVersion) .append(", aspects=").append(aspectQNameIds) .append("]"); return sb.toString(); @@ -62,6 +64,16 @@ public class NodeAspectsEntity this.nodeId = nodeId; } + public Long getNodeVersion() + { + return nodeVersion; + } + + public void setNodeVersion(Long nodeVersion) + { + this.nodeVersion = nodeVersion; + } + public List getAspectQNameIds() { return aspectQNameIds; diff --git a/source/java/org/alfresco/repo/domain/node/NodeEntity.java b/source/java/org/alfresco/repo/domain/node/NodeEntity.java index c5e57ca391..785fbc7509 100644 --- a/source/java/org/alfresco/repo/domain/node/NodeEntity.java +++ b/source/java/org/alfresco/repo/domain/node/NodeEntity.java @@ -105,6 +105,17 @@ public class NodeEntity implements Node, PermissionCheckValue return sb.toString(); } + @Override + // TODO: Must cache the key + public NodeVersionKey getNodeVersionKey() + { + if (id == null || version == null) + { + throw new IllegalStateException("The NodeEntity has not be filled: " + this); + } + return new NodeVersionKey(id, version); + } + /** * Lock the entity against further updates to prevent accidental modification */ @@ -125,8 +136,9 @@ public class NodeEntity implements Node, PermissionCheckValue } } - public void incrementVersion() + public synchronized void incrementVersion() { + checkLock(); if (version >= Short.MAX_VALUE) { this.version = 0L; @@ -162,7 +174,7 @@ public class NodeEntity implements Node, PermissionCheckValue return id; } - public void setId(Long id) + public synchronized void setId(Long id) { checkLock(); this.id = id; @@ -174,7 +186,7 @@ public class NodeEntity implements Node, PermissionCheckValue return version; } - public void setVersion(Long version) + public synchronized void setVersion(Long version) { checkLock(); this.version = version; @@ -186,7 +198,7 @@ public class NodeEntity implements Node, PermissionCheckValue return store; } - public void setStore(StoreEntity store) + public synchronized void setStore(StoreEntity store) { checkLock(); this.store = store; @@ -198,7 +210,7 @@ public class NodeEntity implements Node, PermissionCheckValue return uuid; } - public void setUuid(String uuid) + public synchronized void setUuid(String uuid) { checkLock(); this.uuid = uuid; @@ -210,7 +222,7 @@ public class NodeEntity implements Node, PermissionCheckValue return typeQNameId; } - public void setTypeQNameId(Long typeQNameId) + public synchronized void setTypeQNameId(Long typeQNameId) { checkLock(); this.typeQNameId = typeQNameId; @@ -222,7 +234,7 @@ public class NodeEntity implements Node, PermissionCheckValue return localeId; } - public void setLocaleId(Long localeId) + public synchronized void setLocaleId(Long localeId) { this.localeId = localeId; } @@ -233,7 +245,7 @@ public class NodeEntity implements Node, PermissionCheckValue return aclId; } - public void setAclId(Long aclId) + public synchronized void setAclId(Long aclId) { checkLock(); this.aclId = aclId; @@ -245,7 +257,7 @@ public class NodeEntity implements Node, PermissionCheckValue return deleted; } - public void setDeleted(Boolean deleted) + public synchronized void setDeleted(Boolean deleted) { checkLock(); this.deleted = deleted; @@ -257,7 +269,7 @@ public class NodeEntity implements Node, PermissionCheckValue return transaction; } - public void setTransaction(TransactionEntity transaction) + public synchronized void setTransaction(TransactionEntity transaction) { checkLock(); this.transaction = transaction; @@ -269,7 +281,7 @@ public class NodeEntity implements Node, PermissionCheckValue return auditableProperties; } - public void setAuditableProperties(AuditablePropertiesEntity auditableProperties) + public synchronized void setAuditableProperties(AuditablePropertiesEntity auditableProperties) { checkLock(); this.auditableProperties = auditableProperties; diff --git a/source/java/org/alfresco/repo/domain/node/NodePropertyEntity.java b/source/java/org/alfresco/repo/domain/node/NodePropertyEntity.java index 6f835fb23a..98725d4ff2 100644 --- a/source/java/org/alfresco/repo/domain/node/NodePropertyEntity.java +++ b/source/java/org/alfresco/repo/domain/node/NodePropertyEntity.java @@ -29,6 +29,7 @@ import java.util.List; public class NodePropertyEntity { private Long nodeId; + private Long nodeVersion; private NodePropertyKey key; private NodePropertyValue value; /** Carries data for queries and updates */ @@ -46,7 +47,7 @@ public class NodePropertyEntity @Override public String toString() { - return "NodePropertyEntity [node=" + nodeId + ", key=" + key + ", value=" + value + "]"; + return "NodePropertyEntity [node=" + nodeId + ", nodeVersion=" + nodeVersion + ", key=" + key + ", value=" + value + "]"; } public Long getNodeId() @@ -59,6 +60,16 @@ public class NodePropertyEntity this.nodeId = nodeId; } + public Long getNodeVersion() + { + return nodeVersion; + } + + public void setNodeVersion(Long nodeVersion) + { + this.nodeVersion = nodeVersion; + } + public NodePropertyKey getKey() { return key; diff --git a/source/java/org/alfresco/repo/domain/node/NodeVersionKey.java b/source/java/org/alfresco/repo/domain/node/NodeVersionKey.java new file mode 100644 index 0000000000..1668b17e34 --- /dev/null +++ b/source/java/org/alfresco/repo/domain/node/NodeVersionKey.java @@ -0,0 +1,89 @@ +/* + * Copyright (C) 2005-2010 Alfresco Software Limited. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + + * As a special exception to the terms and conditions of version 2.0 of + * the GPL, you may redistribute this Program in connection with Free/Libre + * and Open Source Software ("FLOSS") applications as described in Alfresco's + * FLOSS exception. You should have recieved a copy of the text describing + * the FLOSS exception, and it is also available here: + * http://www.alfresco.com/legal/licensing" + */ +package org.alfresco.repo.domain.node; + +import java.io.Serializable; + +/** + * Key for caches that need to be bound implicitly to the current version of a node. + * + * @author Derek Hulley + * @since 4.0 + */ +public class NodeVersionKey implements Serializable +{ + private static final long serialVersionUID = 2241045540959490539L; + + private final Long nodeId; + private final Long version; + + public NodeVersionKey(Long nodeId, Long version) + { + this.nodeId = nodeId; + this.version = version; + } + + @Override + public boolean equals(Object other) + { + if (this == other) + { + return true; + } + if (!(other instanceof NodeVersionKey)) + { + return false; + } + NodeVersionKey o = (NodeVersionKey)other; + return nodeId.equals(o.nodeId) && version.equals(o.version); + } + + @Override + public int hashCode() + { + return nodeId.hashCode() + version.hashCode(); + } + + @Override + public String toString() + { + StringBuilder builder = new StringBuilder(); + builder.append("NodeVersionKey ") + .append("[nodeId=").append(nodeId) + .append(", version=").append(version) + .append("]"); + return builder.toString(); + } + + public Long getNodeId() + { + return nodeId; + } + + public Long getVersion() + { + return version; + } +} 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 ca0ccb6b78..da202758a6 100644 --- a/source/java/org/alfresco/repo/domain/node/ibatis/NodeDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/node/ibatis/NodeDAOImpl.java @@ -42,6 +42,7 @@ import org.alfresco.repo.domain.node.NodePropertyEntity; import org.alfresco.repo.domain.node.NodePropertyKey; import org.alfresco.repo.domain.node.NodePropertyValue; import org.alfresco.repo.domain.node.NodeUpdateEntity; +import org.alfresco.repo.domain.node.NodeVersionKey; import org.alfresco.repo.domain.node.PrimaryChildrenAclUpdateEntity; import org.alfresco.repo.domain.node.ServerEntity; import org.alfresco.repo.domain.node.StoreEntity; @@ -418,21 +419,23 @@ public class NodeDAOImpl extends AbstractNodeDAOImpl /** * Pull out the key-value pairs from the rows */ - private Map> makePersistentPropertiesMap(List rows) + private Map> makePersistentPropertiesMap(List rows) { - Map> results = new HashMap>(3); + Map> results = new HashMap>(3); for (NodePropertyEntity row : rows) { Long nodeId = row.getNodeId(); - if (nodeId == null) + Long nodeVersion = row.getNodeVersion(); + if (nodeId == null || nodeVersion == null) { - throw new RuntimeException("Expect results with a Node ID: " + row); + throw new RuntimeException("Expect results with a Node and Version: " + row); } - Map props = results.get(nodeId); + NodeVersionKey nodeTxnKey = new NodeVersionKey(nodeId, nodeVersion); + Map props = results.get(nodeTxnKey); if (props == null) { props = new HashMap(17); - results.put(nodeId, props); + results.put(nodeTxnKey, props); } props.put(row.getKey(), row.getValue()); } @@ -460,22 +463,7 @@ public class NodeDAOImpl extends AbstractNodeDAOImpl @Override @SuppressWarnings("unchecked") - protected List selectNodeAspects(Set nodeIds) - { - if (nodeIds.size() == 0) - { - return Collections.emptyList(); - } - NodeAspectsEntity aspects = new NodeAspectsEntity(); - aspects.setNodeIds(new ArrayList(nodeIds)); - - List rows = (List) template.selectList(SELECT_NODE_ASPECTS, aspects); - return rows; - } - - @Override - @SuppressWarnings("unchecked") - protected Map> selectNodeProperties(Set nodeIds) + protected Map> selectNodeProperties(Set nodeIds) { if (nodeIds.size() == 0) { @@ -488,13 +476,13 @@ public class NodeDAOImpl extends AbstractNodeDAOImpl return makePersistentPropertiesMap(rows); } @Override - protected Map selectNodeProperties(Long nodeId) + protected Map> selectNodeProperties(Long nodeId) { return selectNodeProperties(nodeId, Collections.emptySet()); } @Override @SuppressWarnings("unchecked") - protected Map selectNodeProperties(Long nodeId, Set qnameIds) + protected Map> selectNodeProperties(Long nodeId, Set qnameIds) { NodePropertyEntity prop = new NodePropertyEntity(); // Node @@ -514,16 +502,7 @@ public class NodeDAOImpl extends AbstractNodeDAOImpl } List rows = (List) template.selectList(SELECT_NODE_PROPERTIES, prop); - Map> results = makePersistentPropertiesMap(rows); - Map props = results.get(nodeId); - if (props == null) - { - return Collections.emptyMap(); - } - else - { - return props; - } + return makePersistentPropertiesMap(rows); } @Override @@ -601,19 +580,34 @@ public class NodeDAOImpl extends AbstractNodeDAOImpl } } + @SuppressWarnings("unchecked") @Override - protected Set selectNodeAspectIds(Long nodeId) + protected Map> selectNodeAspects(Set nodeIds) { - Set aspectIds = new HashSet(); - Set nodeIds = new HashSet(); - nodeIds.add(nodeId); - List nodeAspectEntities = selectNodeAspects(nodeIds); - if(nodeAspectEntities.size() > 0) - { - NodeAspectsEntity nodeAspects = nodeAspectEntities.get(0); - aspectIds.addAll(nodeAspects.getAspectQNameIds()); - } - return aspectIds; + if (nodeIds.size() == 0) + { + return Collections.emptyMap(); + } + NodeAspectsEntity aspects = new NodeAspectsEntity(); + aspects.setNodeIds(new ArrayList(nodeIds)); + + List rows = (List) template.selectList(SELECT_NODE_ASPECTS, aspects); + + Map> results = new HashMap>(rows.size()*2); + for (NodeAspectsEntity nodeAspectsEntity : rows) + { + Long nodeId = nodeAspectsEntity.getNodeId(); + Long nodeVersion = nodeAspectsEntity.getNodeVersion(); + NodeVersionKey nodeVersionKey = new NodeVersionKey(nodeId, nodeVersion); + if (results.containsKey(nodeVersionKey)) + { + throw new IllegalStateException("Found existing key while querying for node aspects: " + nodeIds); + } + Set aspectIds = new HashSet(nodeAspectsEntity.getAspectQNameIds()); + Set aspectQNames = qnameDAO.convertIdsToQNames(aspectIds); + results.put(nodeVersionKey, aspectQNames); + } + return results; } @Override