Author: sergiykarpenko Date: 2011-04-13 09:47:01 -0400 (Wed, 13 Apr 2011) New Revision: 4234 Added: jcr/trunk/exo.jcr.component.core/src/test/java/org/exoplatform/services/jcr/impl/dataflow/persistent/TestISPNCacheWorkspaceStorageCache.java Modified: jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/infinispan/BufferedISPNCache.java jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/infinispan/ISPNCacheWorkspaceStorageCache.java jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/jbosscache/BufferedJBossCache.java jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/jbosscache/JBossCacheWorkspaceStorageCache.java jcr/trunk/exo.jcr.component.core/src/test/java/org/exoplatform/services/jcr/impl/dataflow/persistent/TestJBossCacheWorkspaceStorageCache.java Log: EXOJCR-1302: Avoid overwrite data in cache in multi-thread environment. Modified: jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/infinispan/BufferedISPNCache.java =================================================================== --- jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/infinispan/BufferedISPNCache.java 2011-04-13 12:55:17 UTC (rev 4233) +++ jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/infinispan/BufferedISPNCache.java 2011-04-13 13:47:01 UTC (rev 4234) @@ -29,7 +29,6 @@ import org.infinispan.manager.CacheContainer; import org.infinispan.util.concurrent.NotifyingFuture; -import java.io.Serializable; import java.security.AccessController; import java.security.PrivilegedAction; import java.util.Collection; @@ -49,12 +48,12 @@ * */ @SuppressWarnings("unchecked") -public class BufferedISPNCache implements Cache<Serializable, Object> +public class BufferedISPNCache implements Cache<CacheKey, Object> { /** * Parent cache. */ - private final AdvancedCache<Serializable, Object> parentCache; + private final AdvancedCache<CacheKey, Object> parentCache; private final ThreadLocal<CompressedISPNChangesBuffer> changesList = new ThreadLocal<CompressedISPNChangesBuffer>(); @@ -80,7 +79,7 @@ protected final ChangesType changesType; - protected final AdvancedCache<Serializable, Object> cache; + protected final AdvancedCache<CacheKey, Object> cache; protected final int historicalIndex; @@ -88,7 +87,7 @@ private final Boolean allowLocalChanges; - public ChangesContainer(CacheKey key, ChangesType changesType, AdvancedCache<Serializable, Object> cache, + public ChangesContainer(CacheKey key, ChangesType changesType, AdvancedCache<CacheKey, Object> cache, int historicalIndex, boolean localMode, Boolean allowLocalChanges) { this.key = key; @@ -170,7 +169,7 @@ { private final Object value; - public PutObjectContainer(CacheKey key, Object value, AdvancedCache<Serializable, Object> cache, + public PutObjectContainer(CacheKey key, Object value, AdvancedCache<CacheKey, Object> cache, int historicalIndex, boolean local, Boolean allowLocalChanges) { super(key, ChangesType.PUT, cache, historicalIndex, local, allowLocalChanges); @@ -185,8 +184,32 @@ cache.put(key, value); } } + /** + * Put object if absent container + */ + public static class PutObjectIfAbsentContainer extends ChangesContainer + { + private final Object value; + + public PutObjectIfAbsentContainer(CacheKey key, Object value, AdvancedCache<CacheKey, Object> cache, + int historicalIndex, boolean local, Boolean allowLocalChanges) + { + super(key, ChangesType.PUT, cache, historicalIndex, local, allowLocalChanges); + + this.value = value; + } + + @Override + public void apply() + { + setCacheLocalMode(); + cache.putIfAbsent(key, value); + } + } + + /** * It tries to get Set by given key. If it is Set then adds new value and puts new set back. If * null found, then new Set created (ordinary cache does). */ @@ -196,7 +219,7 @@ private final boolean forceModify; - public AddToListContainer(CacheKey key, Object value, AdvancedCache<Serializable, Object> cache, + public AddToListContainer(CacheKey key, Object value, AdvancedCache<CacheKey, Object> cache, boolean forceModify, int historicalIndex, boolean local, Boolean allowLocalChanges) { super(key, ChangesType.PUT, cache, historicalIndex, local, allowLocalChanges); @@ -242,7 +265,7 @@ { private final Object value; - public RemoveFromListContainer(CacheKey key, Object value, AdvancedCache<Serializable, Object> cache, + public RemoveFromListContainer(CacheKey key, Object value, AdvancedCache<CacheKey, Object> cache, int historicalIndex, boolean local, Boolean allowLocalChanges) { super(key, ChangesType.REMOVE, cache, historicalIndex, local, allowLocalChanges); @@ -275,7 +298,7 @@ */ public static class RemoveObjectContainer extends ChangesContainer { - public RemoveObjectContainer(CacheKey key, AdvancedCache<Serializable, Object> cache, int historicalIndex, + public RemoveObjectContainer(CacheKey key, AdvancedCache<CacheKey, Object> cache, int historicalIndex, boolean local, Boolean allowLocalChanges) { super(key, ChangesType.REMOVE, cache, historicalIndex, local, allowLocalChanges); @@ -289,7 +312,7 @@ } } - public BufferedISPNCache(Cache<Serializable, Object> parentCache, Boolean allowLocalChanges) + public BufferedISPNCache(Cache<CacheKey, Object> parentCache, Boolean allowLocalChanges) { this.parentCache = parentCache.getAdvancedCache(); this.allowLocalChanges = allowLocalChanges; @@ -322,7 +345,7 @@ /** * {@inheritDoc} */ - public Set<java.util.Map.Entry<Serializable, Object>> entrySet() + public Set<java.util.Map.Entry<CacheKey, Object>> entrySet() { return parentCache.entrySet(); } @@ -330,7 +353,7 @@ /** * {@inheritDoc} */ - public void evict(Serializable key) + public void evict(CacheKey key) { parentCache.evict(key); } @@ -338,7 +361,7 @@ /** * {@inheritDoc} */ - public AdvancedCache<Serializable, Object> getAdvancedCache() + public AdvancedCache<CacheKey, Object> getAdvancedCache() { return parentCache.getAdvancedCache(); } @@ -386,7 +409,7 @@ /** * {@inheritDoc} */ - public Set<Serializable> keySet() + public Set<CacheKey> keySet() { return parentCache.keySet(); } @@ -394,7 +417,7 @@ /** * {@inheritDoc} */ - public Object put(Serializable key, Object value, long lifespan, TimeUnit unit) + public Object put(CacheKey key, Object value, long lifespan, TimeUnit unit) { return parentCache.put(key, value, lifespan, unit); } @@ -402,7 +425,7 @@ /** * {@inheritDoc} */ - public Object put(Serializable key, Object value, long lifespan, TimeUnit lifespanUnit, long maxIdleTime, + public Object put(CacheKey key, Object value, long lifespan, TimeUnit lifespanUnit, long maxIdleTime, TimeUnit maxIdleTimeUnit) { return parentCache.put(key, value, lifespan, lifespanUnit, maxIdleTime, maxIdleTimeUnit); @@ -411,7 +434,7 @@ /** * {@inheritDoc} */ - public void putAll(Map<? extends Serializable, ? extends Object> map, long lifespan, TimeUnit unit) + public void putAll(Map<? extends CacheKey, ? extends Object> map, long lifespan, TimeUnit unit) { parentCache.putAll(map, lifespan, unit); } @@ -419,7 +442,7 @@ /** * {@inheritDoc} */ - public void putAll(Map<? extends Serializable, ? extends Object> map, long lifespan, TimeUnit lifespanUnit, + public void putAll(Map<? extends CacheKey, ? extends Object> map, long lifespan, TimeUnit lifespanUnit, long maxIdleTime, TimeUnit maxIdleTimeUnit) { parentCache.putAll(map, lifespan, lifespanUnit, maxIdleTime, maxIdleTimeUnit); @@ -428,7 +451,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Void> putAllAsync(Map<? extends Serializable, ? extends Object> data) + public NotifyingFuture<Void> putAllAsync(Map<? extends CacheKey, ? extends Object> data) { return parentCache.putAllAsync(data); } @@ -436,7 +459,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Void> putAllAsync(Map<? extends Serializable, ? extends Object> data, long lifespan, + public NotifyingFuture<Void> putAllAsync(Map<? extends CacheKey, ? extends Object> data, long lifespan, TimeUnit unit) { return parentCache.putAllAsync(data, lifespan, unit); @@ -445,7 +468,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Void> putAllAsync(Map<? extends Serializable, ? extends Object> data, long lifespan, + public NotifyingFuture<Void> putAllAsync(Map<? extends CacheKey, ? extends Object> data, long lifespan, TimeUnit lifespanUnit, long maxIdle, TimeUnit maxIdleUnit) { return parentCache.putAllAsync(data, lifespan, lifespanUnit, maxIdle, maxIdleUnit); @@ -454,7 +477,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Object> putAsync(Serializable key, Object value) + public NotifyingFuture<Object> putAsync(CacheKey key, Object value) { return parentCache.putAsync(key, value); } @@ -462,7 +485,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Object> putAsync(Serializable key, Object value, long lifespan, TimeUnit unit) + public NotifyingFuture<Object> putAsync(CacheKey key, Object value, long lifespan, TimeUnit unit) { return parentCache.putAsync(key, value, lifespan, unit); } @@ -470,7 +493,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Object> putAsync(Serializable key, Object value, long lifespan, TimeUnit lifespanUnit, + public NotifyingFuture<Object> putAsync(CacheKey key, Object value, long lifespan, TimeUnit lifespanUnit, long maxIdle, TimeUnit maxIdleUnit) { return parentCache.putAsync(key, value, lifespan, lifespanUnit, maxIdle, maxIdleUnit); @@ -479,7 +502,7 @@ /** * {@inheritDoc} */ - public void putForExternalRead(Serializable key, Object value) + public void putForExternalRead(CacheKey key, Object value) { parentCache.putForExternalRead(key, value); } @@ -487,7 +510,7 @@ /** * {@inheritDoc} */ - public Object putIfAbsent(Serializable key, Object value, long lifespan, TimeUnit unit) + public Object putIfAbsent(CacheKey key, Object value, long lifespan, TimeUnit unit) { return parentCache.putIfAbsent(key, value, lifespan, unit); } @@ -495,7 +518,7 @@ /** * {@inheritDoc} */ - public Object putIfAbsent(Serializable key, Object value, long lifespan, TimeUnit lifespanUnit, long maxIdleTime, + public Object putIfAbsent(CacheKey key, Object value, long lifespan, TimeUnit lifespanUnit, long maxIdleTime, TimeUnit maxIdleTimeUnit) { return parentCache.putIfAbsent(key, value, lifespan, lifespanUnit, maxIdleTime, maxIdleTimeUnit); @@ -504,7 +527,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Object> putIfAbsentAsync(Serializable key, Object value) + public NotifyingFuture<Object> putIfAbsentAsync(CacheKey key, Object value) { return parentCache.putIfAbsentAsync(key, value); } @@ -512,7 +535,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Object> putIfAbsentAsync(Serializable key, Object value, long lifespan, TimeUnit unit) + public NotifyingFuture<Object> putIfAbsentAsync(CacheKey key, Object value, long lifespan, TimeUnit unit) { return parentCache.putIfAbsentAsync(key, value, lifespan, unit); } @@ -520,7 +543,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Object> putIfAbsentAsync(Serializable key, Object value, long lifespan, + public NotifyingFuture<Object> putIfAbsentAsync(CacheKey key, Object value, long lifespan, TimeUnit lifespanUnit, long maxIdle, TimeUnit maxIdleUnit) { return parentCache.putIfAbsentAsync(key, value, lifespan, lifespanUnit, maxIdle, maxIdleUnit); @@ -545,7 +568,7 @@ /** * {@inheritDoc} */ - public Object replace(Serializable key, Object value, long lifespan, TimeUnit unit) + public Object replace(CacheKey key, Object value, long lifespan, TimeUnit unit) { return parentCache.replace(key, value, lifespan, unit); } @@ -553,7 +576,7 @@ /** * {@inheritDoc} */ - public boolean replace(Serializable key, Object oldValue, Object value, long lifespan, TimeUnit unit) + public boolean replace(CacheKey key, Object oldValue, Object value, long lifespan, TimeUnit unit) { return parentCache.replace(key, oldValue, value, lifespan, unit); } @@ -561,7 +584,7 @@ /** * {@inheritDoc} */ - public Object replace(Serializable key, Object value, long lifespan, TimeUnit lifespanUnit, long maxIdleTime, + public Object replace(CacheKey key, Object value, long lifespan, TimeUnit lifespanUnit, long maxIdleTime, TimeUnit maxIdleTimeUnit) { return parentCache.replace(key, value, lifespan, lifespanUnit, maxIdleTime, maxIdleTimeUnit); @@ -570,7 +593,7 @@ /** * {@inheritDoc} */ - public boolean replace(Serializable key, Object oldValue, Object value, long lifespan, TimeUnit lifespanUnit, + public boolean replace(CacheKey key, Object oldValue, Object value, long lifespan, TimeUnit lifespanUnit, long maxIdleTime, TimeUnit maxIdleTimeUnit) { return parentCache.replace(key, oldValue, value, lifespan, lifespanUnit, maxIdleTime, maxIdleTimeUnit); @@ -579,7 +602,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Object> replaceAsync(Serializable key, Object value) + public NotifyingFuture<Object> replaceAsync(CacheKey key, Object value) { return parentCache.replaceAsync(key, value); } @@ -587,7 +610,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Boolean> replaceAsync(Serializable key, Object oldValue, Object newValue) + public NotifyingFuture<Boolean> replaceAsync(CacheKey key, Object oldValue, Object newValue) { return parentCache.replaceAsync(key, oldValue, newValue); } @@ -595,7 +618,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Object> replaceAsync(Serializable key, Object value, long lifespan, TimeUnit unit) + public NotifyingFuture<Object> replaceAsync(CacheKey key, Object value, long lifespan, TimeUnit unit) { return parentCache.replaceAsync(key, value, lifespan, unit); } @@ -603,7 +626,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Boolean> replaceAsync(Serializable key, Object oldValue, Object newValue, long lifespan, + public NotifyingFuture<Boolean> replaceAsync(CacheKey key, Object oldValue, Object newValue, long lifespan, TimeUnit unit) { return parentCache.replaceAsync(key, oldValue, newValue, lifespan, unit); @@ -612,7 +635,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Object> replaceAsync(Serializable key, Object value, long lifespan, TimeUnit lifespanUnit, + public NotifyingFuture<Object> replaceAsync(CacheKey key, Object value, long lifespan, TimeUnit lifespanUnit, long maxIdle, TimeUnit maxIdleUnit) { return parentCache.replaceAsync(key, value, lifespan, lifespanUnit, maxIdle, maxIdleUnit); @@ -621,7 +644,7 @@ /** * {@inheritDoc} */ - public NotifyingFuture<Boolean> replaceAsync(Serializable key, Object oldValue, Object newValue, long lifespan, + public NotifyingFuture<Boolean> replaceAsync(CacheKey key, Object oldValue, Object newValue, long lifespan, TimeUnit lifespanUnit, long maxIdle, TimeUnit maxIdleUnit) { return parentCache.replaceAsync(key, oldValue, newValue); @@ -646,9 +669,12 @@ /** * {@inheritDoc} */ - public Object putIfAbsent(Serializable key, Object value) + public Object putIfAbsent(CacheKey key, Object value) { - return parentCache.putIfAbsent(key, value); + CompressedISPNChangesBuffer changesContainer = getChangesBufferSafe(); + changesContainer.add(new PutObjectIfAbsentContainer(key, value, parentCache, changesContainer.getHistoryIndex(), local + .get(), allowLocalChanges)); + return null; } /** @@ -662,7 +688,7 @@ /** * {@inheritDoc} */ - public Object replace(Serializable key, Object value) + public Object replace(CacheKey key, Object value) { return parentCache.replace(key, value); } @@ -670,7 +696,7 @@ /** * {@inheritDoc} */ - public boolean replace(Serializable key, Object oldValue, Object newValue) + public boolean replace(CacheKey key, Object oldValue, Object newValue) { return parentCache.replace(key, oldValue, newValue); } @@ -760,16 +786,8 @@ /** * {@inheritDoc} */ - public Object put(Serializable key, Object value) + public void putAll(Map<? extends CacheKey, ? extends Object> m) { - throw new UnsupportedOperationException("Unexpected method call use put(CacheKey key, Object value)"); - } - - /** - * {@inheritDoc} - */ - public void putAll(Map<? extends Serializable, ? extends Object> m) - { parentCache.putAll(m); } Modified: jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/infinispan/ISPNCacheWorkspaceStorageCache.java =================================================================== --- jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/infinispan/ISPNCacheWorkspaceStorageCache.java 2011-04-13 12:55:17 UTC (rev 4233) +++ jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/infinispan/ISPNCacheWorkspaceStorageCache.java 2011-04-13 13:47:01 UTC (rev 4234) @@ -53,7 +53,6 @@ import java.io.File; import java.io.IOException; -import java.io.Serializable; import java.security.PrivilegedExceptionAction; import java.util.ArrayList; import java.util.Collections; @@ -223,11 +222,11 @@ this.enabled = wsConfig.getCache().isEnabled(); // create cache using custom factory - ISPNCacheFactory<Serializable, Object> factory = new ISPNCacheFactory<Serializable, Object>(cfm); + ISPNCacheFactory<CacheKey, Object> factory = new ISPNCacheFactory<CacheKey, Object>(cfm); // create parent Infinispan instance CacheEntry cacheEntry = wsConfig.getCache(); - Cache<Serializable, Object> parentCache = factory.createCache("Data_" + wsConfig.getUniqueName(), cacheEntry); + Cache<CacheKey, Object> parentCache = factory.createCache("Data_" + wsConfig.getUniqueName(), cacheEntry); Boolean allowLocalChanges = null; try @@ -369,10 +368,6 @@ } cache.setLocal(true); - - // remove previous all (to be sure about consistency) - cache.remove(new CacheNodesId(parent.getIdentifier())); - if (childs.size() > 0) { Set<Object> set = new HashSet<Object>(); @@ -381,12 +376,12 @@ putNode(child, ModifyChildOption.NOT_MODIFY); set.add(child.getIdentifier()); } - cache.put(new CacheNodesId(parent.getIdentifier()), set); + cache.putIfAbsent(new CacheNodesId(parent.getIdentifier()), set); } else { // cache fact of empty childs list - cache.put(new CacheNodesId(parent.getIdentifier()), new HashSet<Object>()); + cache.putIfAbsent(new CacheNodesId(parent.getIdentifier()), new HashSet<Object>()); } } finally @@ -412,8 +407,6 @@ cache.beginTransaction(); } cache.setLocal(true); - // remove previous all (to be sure about consistency) - cache.remove(new CachePropsId(parent.getIdentifier())); if (childs.size() > 0) { // add all new @@ -423,7 +416,7 @@ putProperty(child, ModifyChildOption.NOT_MODIFY); set.add(child.getIdentifier()); } - cache.put(new CachePropsId(parent.getIdentifier()), set); + cache.putIfAbsent(new CachePropsId(parent.getIdentifier()), set); } else @@ -670,7 +663,14 @@ { if (node.getParentIdentifier() != null) { - cache.put(new CacheQPath(node.getParentIdentifier(), node.getQPath(), ItemType.NODE), node.getIdentifier()); + if (modifyListsOfChild == ModifyChildOption.NOT_MODIFY) + { + cache.putIfAbsent(new CacheQPath(node.getParentIdentifier(), node.getQPath(), ItemType.NODE), node.getIdentifier()); + } + else + { + cache.put(new CacheQPath(node.getParentIdentifier(), node.getQPath(), ItemType.NODE), node.getIdentifier()); + } // if MODIFY and List present OR FORCE_MODIFY, then write if (modifyListsOfChild != ModifyChildOption.NOT_MODIFY) @@ -680,7 +680,14 @@ } } - return (ItemData)cache.put(new CacheId(node.getIdentifier()), node, true); + if (modifyListsOfChild == ModifyChildOption.NOT_MODIFY) + { + return (ItemData)cache.putIfAbsent(new CacheId(node.getIdentifier()), node); + } + else + { + return (ItemData)cache.put(new CacheId(node.getIdentifier()), node, true); + } } protected ItemData putNodeInBufferedCache(NodeData node, ModifyChildOption modifyListsOfChild) @@ -720,11 +727,11 @@ if (!item.getIdentifier().equals(NullItemData.NULL_ID)) { - cache.put(new CacheId(item.getIdentifier()), item); + cache.putIfAbsent(new CacheId(item.getIdentifier()), item); } else if (item.getName() != null && item.getParentIdentifier() != null) { - cache.put(new CacheQPath(item.getParentIdentifier(), item.getName(), ItemType.getItemType(item)), + cache.putIfAbsent(new CacheQPath(item.getParentIdentifier(), item.getName(), ItemType.getItemType(item)), NullItemData.NULL_ID); } } @@ -752,8 +759,15 @@ cache.addToList(new CachePropsId(prop.getParentIdentifier()), prop.getIdentifier(), modifyListsOfChild == ModifyChildOption.FORCE_MODIFY); } + if (modifyListsOfChild == ModifyChildOption.NOT_MODIFY) + { + cache.putIfAbsent(new CacheQPath(prop.getParentIdentifier(), prop.getQPath(), ItemType.PROPERTY), prop.getIdentifier()); + } + else + { + cache.put(new CacheQPath(prop.getParentIdentifier(), prop.getQPath(), ItemType.PROPERTY), prop.getIdentifier()); + } - cache.put(new CacheQPath(prop.getParentIdentifier(), prop.getQPath(), ItemType.PROPERTY), prop.getIdentifier()); // add referenced property if (modifyListsOfChild != ModifyChildOption.NOT_MODIFY && prop.getType() == PropertyType.REFERENCE) @@ -780,7 +794,16 @@ } } // NullItemData must never be returned inside internal cache operations. - PropertyData propData = (PropertyData)cache.put(new CacheId(prop.getIdentifier()), prop, true); + PropertyData propData; + if (modifyListsOfChild == ModifyChildOption.NOT_MODIFY) + { + propData = (PropertyData)cache.putIfAbsent(new CacheId(prop.getIdentifier()), prop); + } + else + { + propData = (PropertyData)cache.put(new CacheId(prop.getIdentifier()), prop, true); + } + return (propData instanceof NullPropertyData) ? null : propData; } @@ -871,7 +894,7 @@ boolean inheritACL = acl != null; // check all ITEMS in cache - Iterator<Serializable> keys = cache.keySet().iterator(); + Iterator<CacheKey> keys = cache.keySet().iterator(); while (keys.hasNext()) { @@ -947,7 +970,7 @@ */ protected void updateChildsACL(final String parentId, final AccessControlList acl) { - for (Iterator<NodeData> iter = new ChildNodesIterator<NodeData>(parentId); iter.hasNext();) + loop: for (Iterator<NodeData> iter = new ChildNodesIterator<NodeData>(parentId); iter.hasNext();) { NodeData prevNode = iter.next(); @@ -956,7 +979,7 @@ { if (mixin.equals(Constants.EXO_PRIVILEGEABLE) || mixin.equals(Constants.EXO_OWNEABLE)) { - continue; + continue loop; } } @@ -1074,8 +1097,6 @@ cache.beginTransaction(); } cache.setLocal(true); - // remove previous all (to be sure about consistency) - cache.remove(new CacheRefsId(identifier)); Set<Object> set = new HashSet<Object>(); for (PropertyData prop : refProperties) @@ -1083,7 +1104,7 @@ putProperty(prop, ModifyChildOption.NOT_MODIFY); set.add(prop.getIdentifier()); } - cache.put(new CacheRefsId(identifier), set); + cache.putIfAbsent(new CacheRefsId(identifier), set); } finally { Modified: jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/jbosscache/BufferedJBossCache.java =================================================================== --- jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/jbosscache/BufferedJBossCache.java 2011-04-13 12:55:17 UTC (rev 4233) +++ jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/jbosscache/BufferedJBossCache.java 2011-04-13 13:47:01 UTC (rev 4234) @@ -489,7 +489,32 @@ return parentCache.get(fqn, key); } + + /** + * in case putIfAbsent is set to <code>true</code> this method will call cache.putIfAbsent(Fqn fqn, Serializable key, Object value) + * otherwise it will call cache.put(Fqn fqn, Serializable key, Object value) + */ + protected Object put(Fqn fqn, Serializable key, Object value, boolean putIfAbsent) + { + if (putIfAbsent) + { + putIfAbsent(fqn, key, value); + return null; + } + return put(fqn, key, value); + } + /** + * This method will create and add a ChangesContainer that will put the value only if no value has been added + */ + protected Object putIfAbsent(Fqn fqn, Serializable key, Object value) + { + CompressedChangesBuffer changesContainer = getChangesBufferSafe(); + changesContainer.add(new PutIfAbsentKeyValueContainer(fqn, key, value, parentCache, changesContainer.getHistoryIndex(), + local.get(), useExpiration, expirationTimeOut)); + return null; + } + public Object putInBuffer(Fqn fqn, Serializable key, Object value) { CompressedChangesBuffer changesContainer = getChangesBufferSafe(); @@ -824,6 +849,42 @@ } /** + * PutIfAbsent container. + */ + public static class PutIfAbsentKeyValueContainer extends ChangesContainer + { + private final Serializable key; + + private final Object value; + + public PutIfAbsentKeyValueContainer(Fqn fqn, Serializable key, Object value, Cache<Serializable, Object> cache, + int historicalIndex, boolean local, boolean useExpiration, long timeOut) + { + super(fqn, ChangesType.PUT_KEY, cache, historicalIndex, local, useExpiration, timeOut); + this.key = key; + this.value = value; + } + + @Override + public void apply() + { + cache.getInvocationContext().getOptionOverrides().setForceWriteLock(true); + if (cache.get(fqn, key) != null) + { + // skip + return; + } + if (useExpiration) + { + putExpiration(fqn); + } + + setCacheLocalMode(); + cache.put(fqn, key, value); + } + } + + /** * Put container. */ public static class PutKeyValueContainer extends ChangesContainer Modified: jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/jbosscache/JBossCacheWorkspaceStorageCache.java =================================================================== --- jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/jbosscache/JBossCacheWorkspaceStorageCache.java 2011-04-13 12:55:17 UTC (rev 4233) +++ jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/jbosscache/JBossCacheWorkspaceStorageCache.java 2011-04-13 13:47:01 UTC (rev 4234) @@ -594,10 +594,6 @@ } cache.setLocal(true); - // remove previous all (to be sure about consistency) - //TODO do we need remove node, while we can replace node? - cache.removeNode(makeChildListFqn(childNodesList, parent.getIdentifier())); - if (childs.size() > 0) { Set<Object> set = new HashSet<Object>(); @@ -606,12 +602,12 @@ putNode(child, ModifyChildOption.NOT_MODIFY); set.add(child.getIdentifier()); } - cache.put(makeChildListFqn(childNodesList, parent.getIdentifier()), ITEM_LIST, set); + cache.putIfAbsent(makeChildListFqn(childNodesList, parent.getIdentifier()), ITEM_LIST, set); } else { // cache fact of empty childs list - cache.put(makeChildListFqn(childNodesList, parent.getIdentifier()), ITEM_LIST, new HashSet<Object>()); + cache.putIfAbsent(makeChildListFqn(childNodesList, parent.getIdentifier()), ITEM_LIST, new HashSet<Object>()); } } finally @@ -637,9 +633,6 @@ cache.beginTransaction(); } cache.setLocal(true); - // remove previous all (to be sure about consistency) - //TODO do we need remove node, while we can replace node? - cache.removeNode(makeChildListFqn(childPropsList, parent.getIdentifier())); if (childs.size() > 0) { // add all new @@ -649,7 +642,7 @@ putProperty(child, ModifyChildOption.NOT_MODIFY); set.add(child.getIdentifier()); } - cache.put(makeChildListFqn(childPropsList, parent.getIdentifier()), ITEM_LIST, set); + cache.putIfAbsent(makeChildListFqn(childPropsList, parent.getIdentifier()), ITEM_LIST, set); } else @@ -872,16 +865,13 @@ cache.setLocal(true); - // remove previous all - cache.removeNode(makeRefFqn(identifier)); - Set<Object> set = new HashSet<Object>(); for (PropertyData prop : refProperties) { putProperty(prop, ModifyChildOption.NOT_MODIFY); set.add(prop.getIdentifier()); } - cache.put(makeRefFqn(identifier), ITEM_LIST, set); + cache.putIfAbsent(makeRefFqn(identifier), ITEM_LIST, set); } finally { @@ -1077,8 +1067,10 @@ if (node.getParentIdentifier() != null) { // add in CHILD_NODES - cache.put(makeChildFqn(childNodes, node.getParentIdentifier(), node.getQPath().getEntries()[node.getQPath() - .getEntries().length - 1]), ITEM_ID, node.getIdentifier()); + cache.put( + makeChildFqn(childNodes, node.getParentIdentifier(), node.getQPath().getEntries()[node.getQPath() + .getEntries().length - 1]), ITEM_ID, node.getIdentifier(), + modifyListsOfChild == ModifyChildOption.NOT_MODIFY); if (modifyListsOfChild != ModifyChildOption.NOT_MODIFY) { @@ -1088,7 +1080,7 @@ } // add in ITEMS - return (ItemData)cache.put(makeItemFqn(node.getIdentifier()), ITEM_DATA, node); + return (ItemData)cache.put(makeItemFqn(node.getIdentifier()), ITEM_DATA, node, modifyListsOfChild == ModifyChildOption.NOT_MODIFY); } /** @@ -1110,20 +1102,20 @@ if (!item.getIdentifier().equals(NullItemData.NULL_ID)) { //put in $ITEMS - cache.put(makeItemFqn(item.getIdentifier()), ITEM_DATA, item); + cache.putIfAbsent(makeItemFqn(item.getIdentifier()), ITEM_DATA, item); } else if (item.getName() != null && item.getParentIdentifier() != null) { if (item.isNode()) { // put in $CHILD_NODES - cache.put(makeChildFqn(childNodes, item.getParentIdentifier(), item.getName()), ITEM_ID, + cache.putIfAbsent(makeChildFqn(childNodes, item.getParentIdentifier(), item.getName()), ITEM_ID, NullItemData.NULL_ID); } else { // put in $CHILD_PROPERTIES - cache.put(makeChildFqn(childProps, item.getParentIdentifier(), item.getName()), ITEM_ID, + cache.putIfAbsent(makeChildFqn(childProps, item.getParentIdentifier(), item.getName()), ITEM_ID, NullItemData.NULL_ID); } } @@ -1170,7 +1162,7 @@ { // add in CHILD_PROPS cache.put(makeChildFqn(childProps, prop.getParentIdentifier(), prop.getQPath().getEntries()[prop.getQPath() - .getEntries().length - 1]), ITEM_ID, prop.getIdentifier()); + .getEntries().length - 1]), ITEM_ID, prop.getIdentifier(), modifyListsOfChild == ModifyChildOption.NOT_MODIFY); if (modifyListsOfChild != ModifyChildOption.NOT_MODIFY) { @@ -1206,7 +1198,7 @@ // add in ITEMS // NullItemData must never be returned inside internal cache operations. - ItemData returnedData = (ItemData)cache.put(makeItemFqn(prop.getIdentifier()), ITEM_DATA, prop); + ItemData returnedData = (ItemData)cache.put(makeItemFqn(prop.getIdentifier()), ITEM_DATA, prop, modifyListsOfChild == ModifyChildOption.NOT_MODIFY); return (returnedData instanceof NullItemData) ? null : (PropertyData)returnedData; } @@ -1279,9 +1271,9 @@ } /** - * Update Node hierachy in case of same-name siblings reorder. - * Assumes the new (updated) nodes already putted in the cache. Previous name of updated nodes will be calculated - * and that node will be deleted (if has same id as the new node). Childs paths will be updated to a new node path. + * Update Node hierarchy in case of same-name siblings reorder. + * Assumes the new (updated) nodes already put in the cache. Previous name of updated nodes will be calculated + * and that node will be deleted (if has same id as the new node). Children paths will be updated to a new node path. * * @param node NodeData * @param prevNode NodeData @@ -1477,15 +1469,15 @@ */ protected void updateChildsACL(final String parentId, final AccessControlList acl) { - for (Iterator<NodeData> iter = new ChildNodesIterator<NodeData>(parentId); iter.hasNext();) + loop: for (Iterator<NodeData> iter = new ChildNodesIterator<NodeData>(parentId); iter.hasNext();) { NodeData prevNode = iter.next(); - // is ACL changes on this node (i.e. ACL inheritance brokes) + // is ACL changes on this node (i.e. ACL inheritance broken) for (InternalQName mixin : prevNode.getMixinTypeNames()) { if (mixin.equals(Constants.EXO_PRIVILEGEABLE) || mixin.equals(Constants.EXO_OWNEABLE)) { - continue; + continue loop; } } // recreate with new path for child Nodes only Added: jcr/trunk/exo.jcr.component.core/src/test/java/org/exoplatform/services/jcr/impl/dataflow/persistent/TestISPNCacheWorkspaceStorageCache.java =================================================================== --- jcr/trunk/exo.jcr.component.core/src/test/java/org/exoplatform/services/jcr/impl/dataflow/persistent/TestISPNCacheWorkspaceStorageCache.java (rev 0) +++ jcr/trunk/exo.jcr.component.core/src/test/java/org/exoplatform/services/jcr/impl/dataflow/persistent/TestISPNCacheWorkspaceStorageCache.java 2011-04-13 13:47:01 UTC (rev 4234) @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2003-2011 eXo Platform SAS. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Affero General Public License + * as published by the Free Software Foundation; either version 3 + * 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, see<http://www.gnu.org/licenses/>;. + */ +package org.exoplatform.services.jcr.impl.dataflow.persistent; + +import org.exoplatform.container.configuration.ConfigurationManagerImpl; +import org.exoplatform.services.jcr.config.CacheEntry; +import org.exoplatform.services.jcr.config.SimpleParameterEntry; +import org.exoplatform.services.jcr.config.WorkspaceEntry; +import org.exoplatform.services.jcr.dataflow.persistent.WorkspaceStorageCache; +import org.exoplatform.services.jcr.impl.dataflow.persistent.infinispan.ISPNCacheWorkspaceStorageCache; + +import java.util.ArrayList; + +/** + * Created by The eXo Platform SAS. + * + * <br/>Date: + * + * @author <a href=&quot;karpenko.sergiy(a)gmail.com&quot;&gt;Karpenko Sergiy</a> + * @version $Id: TestISPNCacheWorkspaceStorageCache.java 111 2008-11-11 11:11:11Z serg $ + */ +public class TestISPNCacheWorkspaceStorageCache extends TestJBossCacheWorkspaceStorageCache +{ + @Override + public WorkspaceStorageCache getCacheImpl() throws Exception + { + ArrayList<SimpleParameterEntry> list = new ArrayList<SimpleParameterEntry>(); + list.add(new SimpleParameterEntry("infinispan-configuration", "jar:/conf/standalone/test-infinispan-config.xml")); + + CacheEntry entry = new CacheEntry(list); + entry.setEnabled(true); + WorkspaceEntry workspaceEntry = new WorkspaceEntry(); + workspaceEntry.setUniqueName("WS_UUID"); + workspaceEntry.setCache(entry); + return new ISPNCacheWorkspaceStorageCache(workspaceEntry, new ConfigurationManagerImpl()); + } +} \ No newline at end of file Modified: jcr/trunk/exo.jcr.component.core/src/test/java/org/exoplatform/services/jcr/impl/dataflow/persistent/TestJBossCacheWorkspaceStorageCache.java =================================================================== --- jcr/trunk/exo.jcr.component.core/src/test/java/org/exoplatform/services/jcr/impl/dataflow/persistent/TestJBossCacheWorkspaceStorageCache.java 2011-04-13 12:55:17 UTC (rev 4233) +++ jcr/trunk/exo.jcr.component.core/src/test/java/org/exoplatform/services/jcr/impl/dataflow/persistent/TestJBossCacheWorkspaceStorageCache.java 2011-04-13 13:47:01 UTC (rev 4234) @@ -22,12 +22,32 @@ import org.exoplatform.services.jcr.config.CacheEntry; import org.exoplatform.services.jcr.config.SimpleParameterEntry; import org.exoplatform.services.jcr.config.WorkspaceEntry; +import org.exoplatform.services.jcr.dataflow.ItemState; +import org.exoplatform.services.jcr.dataflow.PlainChangesLog; +import org.exoplatform.services.jcr.dataflow.PlainChangesLogImpl; +import org.exoplatform.services.jcr.dataflow.persistent.PersistedNodeData; import org.exoplatform.services.jcr.dataflow.persistent.WorkspaceStorageCache; +import org.exoplatform.services.jcr.datamodel.ItemData; +import org.exoplatform.services.jcr.datamodel.ItemType; +import org.exoplatform.services.jcr.datamodel.NodeData; +import org.exoplatform.services.jcr.datamodel.PropertyData; +import org.exoplatform.services.jcr.datamodel.QPathEntry; +import org.exoplatform.services.jcr.impl.Constants; import org.exoplatform.services.jcr.impl.dataflow.persistent.jbosscache.JBossCacheWorkspaceStorageCache; +import org.exoplatform.services.jcr.impl.storage.SystemDataContainerHolder; +import org.exoplatform.services.jcr.impl.storage.WorkspaceDataContainerBase; +import org.exoplatform.services.jcr.storage.WorkspaceDataContainer; +import org.exoplatform.services.jcr.storage.WorkspaceStorageConnection; import org.exoplatform.services.transaction.TransactionService; import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicReference; +import javax.jcr.InvalidItemStateException; +import javax.jcr.RepositoryException; + /** * Created by The eXo Platform SAS. * @@ -48,10 +68,294 @@ "jar:/conf/standalone/test-jbosscache-config.xml")); CacheEntry entry = new CacheEntry(list); + entry.setEnabled(true); WorkspaceEntry workspaceEntry = new WorkspaceEntry(); workspaceEntry.setUniqueName("WS_UUID"); workspaceEntry.setCache(entry); return new JBossCacheWorkspaceStorageCache(workspaceEntry, transactionService == null ? null : transactionService, new ConfigurationManagerImpl()); } + + public void testRaceConditions() throws Exception + { + MyWorkspaceStorageConnection con = new MyWorkspaceStorageConnection(); + WorkspaceDataContainer wdc = new MyWorkspaceDataContainer(con); + final CacheableWorkspaceDataManager cwdm = + new CacheableWorkspaceDataManager(wdc, getCacheImpl(), new SystemDataContainerHolder(wdc)); + String idNode = "foo1"; + executeConcurrentReadNWrite(con, cwdm, Mode.READ_FIRST, idNode); + assertNotNull(cwdm.getItemData(idNode)); + idNode = "foo2"; + executeConcurrentReadNWrite(con, cwdm, Mode.WRITE_FIRST, idNode); + assertNotNull(cwdm.getItemData(idNode)); + } + + /** + * @param con + * @param cwdm + * @param mode + * @param idNode + * @throws InterruptedException + */ + private void executeConcurrentReadNWrite(MyWorkspaceStorageConnection con, final CacheableWorkspaceDataManager cwdm, + final Mode mode, final String idNode) throws InterruptedException + { + final CountDownLatch goSignal = con.setMode(mode); + final AtomicReference<Exception> ex = new AtomicReference<Exception>(); + final CountDownLatch startSignal = new CountDownLatch(1); + final CountDownLatch doneSignal = new CountDownLatch(2); + Thread writer = new Thread() + { + public void run() + { + try + { + startSignal.await(); + PlainChangesLog chlog = new PlainChangesLogImpl(); + chlog.add(ItemState.createAddedState(new PersistedNodeData(idNode, Constants.ROOT_PATH, "parent-id", 1, 0, + Constants.NT_UNSTRUCTURED, null, null))); + cwdm.save(chlog); + if (mode == Mode.WRITE_FIRST) goSignal.countDown(); + } + catch (Exception e) + { + ex.set(e); + } + finally + { + doneSignal.countDown(); + } + } + }; + writer.start(); + Thread reader = new Thread() + { + public void run() + { + try + { + startSignal.await(); + cwdm.getItemData(idNode); + if (mode == Mode.READ_FIRST) goSignal.countDown(); + } + catch (Exception e) + { + ex.set(e); + } + finally + { + doneSignal.countDown(); + } + } + }; + reader.start(); + startSignal.countDown(); + doneSignal.await(); + assertNull(ex.get()); + } + + private static enum Mode + { + READ_FIRST, WRITE_FIRST; + } + private static class MyWorkspaceStorageConnection implements WorkspaceStorageConnection + { + + + private Mode mode; + private CountDownLatch goSignal; + + public CountDownLatch setMode(Mode mode) + { + this.mode = mode; + this.goSignal = new CountDownLatch(1); + return goSignal; + } + + + public void add(NodeData data) throws RepositoryException, UnsupportedOperationException, + InvalidItemStateException, IllegalStateException + { + } + + public void add(PropertyData data) throws RepositoryException, UnsupportedOperationException, + InvalidItemStateException, IllegalStateException + { + } + + public void close() throws IllegalStateException, RepositoryException + { + } + + public void commit() throws IllegalStateException, RepositoryException + { + if (mode == Mode.READ_FIRST) + { + try + { + goSignal.await(); + } + catch (InterruptedException e) + { + Thread.currentThread().interrupt(); + } + } + } + + public void delete(NodeData data) throws RepositoryException, UnsupportedOperationException, + InvalidItemStateException, IllegalStateException + { + } + + public void delete(PropertyData data) throws RepositoryException, UnsupportedOperationException, + InvalidItemStateException, IllegalStateException + { + } + + public int getChildNodesCount(NodeData parent) throws RepositoryException + { + return -1; + } + + public List<NodeData> getChildNodesData(NodeData parent) throws RepositoryException, IllegalStateException + { + return null; + } + + public List<PropertyData> getChildPropertiesData(NodeData parent) throws RepositoryException, + IllegalStateException + { + return null; + } + + /** + * {@inheritDoc} + */ + public ItemData getItemData(NodeData parentData, QPathEntry name) throws RepositoryException, + IllegalStateException + { + return getItemData(parentData, name, ItemType.UNKNOWN); + } + + public ItemData getItemData(NodeData parentData, QPathEntry name, ItemType itemType) throws RepositoryException, + IllegalStateException + { + return null; + } + + public ItemData getItemData(String identifier) throws RepositoryException, IllegalStateException + { + if (mode == Mode.WRITE_FIRST) + { + try + { + goSignal.await(); + } + catch (InterruptedException e) + { + Thread.currentThread().interrupt(); + } + } + return null; + } + + public List<PropertyData> getReferencesData(String nodeIdentifier) throws RepositoryException, + IllegalStateException, UnsupportedOperationException + { + return null; + } + + public boolean isOpened() + { + return true; + } + + public List<PropertyData> listChildPropertiesData(NodeData parent) throws RepositoryException, + IllegalStateException + { + return null; + } + + public void rename(NodeData data) throws RepositoryException, UnsupportedOperationException, + InvalidItemStateException, IllegalStateException + { + } + + public void rollback() throws IllegalStateException, RepositoryException + { + } + + public void update(NodeData data) throws RepositoryException, UnsupportedOperationException, + InvalidItemStateException, IllegalStateException + { + } + + public void update(PropertyData data) throws RepositoryException, UnsupportedOperationException, + InvalidItemStateException, IllegalStateException + { + } + + public int getLastOrderNumber(NodeData parent) throws RepositoryException + { + return -1; + } + + }; + + private static class MyWorkspaceDataContainer extends WorkspaceDataContainerBase + { + + private WorkspaceStorageConnection con; + + public MyWorkspaceDataContainer(WorkspaceStorageConnection con) + { + this.con = con; + } + + public boolean isCheckSNSNewConnection() + { + return false; + } + + public boolean isSame(WorkspaceDataContainer another) + { + return false; + } + + public WorkspaceStorageConnection openConnection() throws RepositoryException + { + return con; + } + + public WorkspaceStorageConnection openConnection(boolean readOnly) throws RepositoryException + { + return con; + } + + public WorkspaceStorageConnection reuseConnection(WorkspaceStorageConnection original) throws RepositoryException + { + return con; + } + + public String getInfo() + { + return "MyWorkspaceDataContainer"; + } + + public String getName() + { + return "MyWorkspaceDataContainer"; + } + + public String getStorageVersion() + { + return "0"; + } + + public String getUniqueName() + { + return "MyWorkspaceDataContainer"; + } + }; + }