Author: nfilotto Date: 2010-04-01 06:28:23 -0400 (Thu, 01 Apr 2010) New Revision: 2205 Added: jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/datamodel/NullNodeData.java Modified: jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/CacheableWorkspaceDataManager.java jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/LinkedWorkspaceStorageCacheImpl.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/TestCacheableWorkspaceDataManager.java Log: EXOJCR-609: The missing values were not properly managed for the method getItem by name and by id Added: jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/datamodel/NullNodeData.java =================================================================== --- jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/datamodel/NullNodeData.java (rev 0) +++ jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/datamodel/NullNodeData.java 2010-04-01 10:28:23 UTC (rev 2205) @@ -0,0 +1,131 @@ +/* + * Copyright (C) 2003-2010 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&lt;http://www.gnu.org/licenses/&gt;. + */ +package org.exoplatform.services.jcr.datamodel; + +import org.exoplatform.services.jcr.access.AccessControlList; +import org.exoplatform.services.jcr.dataflow.ItemDataVisitor; + +import javax.jcr.RepositoryException; + +/** + * This class is used to represent <code>null</code> value, it is designed to be used + * into the cache to represent missing value. + * + * Created by The eXo Platform SAS + * Author : Nicolas Filotto + * nicolas.filotto(a)exoplatform.com + * 31 mars 2010 + */ +public class NullNodeData implements NodeData +{ + private final String id; + private final String parentId; + private final QPath path; + + public NullNodeData(String id) + { + this.id = id; + this.path = new QPath(new QPathEntry[]{new QPathEntry(null, null, 0)}); + this.parentId = null; + } + + public NullNodeData(NodeData parentData, QPathEntry name) + { + this.parentId = parentData.getIdentifier(); + this.path = QPath.makeChildPath(parentData.getQPath(), name); + this.id = parentId + "$" + name.asString(); + } + + /** + * {@inheritDoc} + */ + public void accept(ItemDataVisitor visitor) throws RepositoryException + { + } + + /** + * {@inheritDoc} + */ + public String getIdentifier() + { + return id; + } + + /** + * {@inheritDoc} + */ + public String getParentIdentifier() + { + return parentId; + } + + /** + * {@inheritDoc} + */ + public int getPersistedVersion() + { + return 0; + } + + /** + * {@inheritDoc} + */ + public QPath getQPath() + { + return path; + } + + /** + * {@inheritDoc} + */ + public boolean isNode() + { + return true; + } + + /** + * {@inheritDoc} + */ + public AccessControlList getACL() + { + return null; + } + + /** + * {@inheritDoc} + */ + public InternalQName[] getMixinTypeNames() + { + return null; + } + + /** + * {@inheritDoc} + */ + public int getOrderNumber() + { + return 0; + } + + /** + * {@inheritDoc} + */ + public InternalQName getPrimaryTypeName() + { + return null; + } +} \ No newline at end of file Modified: jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/CacheableWorkspaceDataManager.java =================================================================== --- jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/CacheableWorkspaceDataManager.java 2010-04-01 09:47:18 UTC (rev 2204) +++ jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/CacheableWorkspaceDataManager.java 2010-04-01 10:28:23 UTC (rev 2205) @@ -18,15 +18,12 @@ */ package org.exoplatform.services.jcr.impl.dataflow.persistent; -import org.exoplatform.services.jcr.access.AccessControlList; -import org.exoplatform.services.jcr.dataflow.ItemDataVisitor; import org.exoplatform.services.jcr.dataflow.ItemStateChangesLog; import org.exoplatform.services.jcr.dataflow.persistent.WorkspaceStorageCache; -import org.exoplatform.services.jcr.datamodel.InternalQName; import org.exoplatform.services.jcr.datamodel.ItemData; import org.exoplatform.services.jcr.datamodel.NodeData; +import org.exoplatform.services.jcr.datamodel.NullNodeData; import org.exoplatform.services.jcr.datamodel.PropertyData; -import org.exoplatform.services.jcr.datamodel.QPath; import org.exoplatform.services.jcr.datamodel.QPathEntry; import org.exoplatform.services.jcr.datamodel.ValueData; import org.exoplatform.services.jcr.impl.dataflow.persistent.jbosscache.JBossCacheWorkspaceStorageCache; @@ -54,71 +51,8 @@ */ public class CacheableWorkspaceDataManager extends WorkspacePersistentDataManager { - /** - * The identifier of the <code>null</code> value - */ - protected static final String ITEM_DATA_NULL_VALUE_ID = "$"; /** - * The <code>null</code> value for the itemData - */ - protected static final ItemData ITEM_DATA_NULL_VALUE = new NodeData() - { - - public void accept(ItemDataVisitor visitor) throws RepositoryException - { - } - - public String getIdentifier() - { - return ITEM_DATA_NULL_VALUE_ID; - } - - public String getParentIdentifier() - { - return null; - } - - public int getPersistedVersion() - { - return 0; - } - - QPath path = new QPath(new QPathEntry[]{new QPathEntry(null, null, 0)}); - - public QPath getQPath() - { - return path; - } - - public boolean isNode() - { - return true; - } - - public AccessControlList getACL() - { - return null; - } - - public InternalQName[] getMixinTypeNames() - { - return null; - } - - public int getOrderNumber() - { - return 0; - } - - public InternalQName getPrimaryTypeName() - { - return null; - } - - }; - - /** * Items cache. */ protected final WorkspaceStorageCache cache; @@ -483,7 +417,7 @@ fixPropertyValues((PropertyData)data); } - return data == ITEM_DATA_NULL_VALUE ? null : data; + return data instanceof NullNodeData ? null : data; } /** @@ -492,6 +426,10 @@ @Override public ItemData getItemData(String identifier) throws RepositoryException { + if (identifier == null) + { + return null; + } // 2. Try from cache ItemData data = getCachedItemData(identifier); @@ -525,7 +463,7 @@ fixPropertyValues((PropertyData)data); } - return data == ITEM_DATA_NULL_VALUE ? null : data; + return data instanceof NullNodeData ? null : data; } /** @@ -729,7 +667,7 @@ ItemData data = super.getItemData(parentData, name); if (cache.isEnabled()) { - cache.put(data == null ? ITEM_DATA_NULL_VALUE : data); + cache.put(data == null ? new NullNodeData(parentData, name) : data); } return data; } @@ -746,7 +684,7 @@ ItemData data = super.getItemData(identifier); if (cache.isEnabled()) { - cache.put(data == null ? ITEM_DATA_NULL_VALUE : data); + cache.put(data == null ? new NullNodeData(identifier) : data); } return data; } Modified: jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/LinkedWorkspaceStorageCacheImpl.java =================================================================== --- jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/LinkedWorkspaceStorageCacheImpl.java 2010-04-01 09:47:18 UTC (rev 2204) +++ jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/LinkedWorkspaceStorageCacheImpl.java 2010-04-01 10:28:23 UTC (rev 2205) @@ -26,6 +26,7 @@ import org.exoplatform.services.jcr.dataflow.persistent.WorkspaceStorageCache; import org.exoplatform.services.jcr.datamodel.ItemData; import org.exoplatform.services.jcr.datamodel.NodeData; +import org.exoplatform.services.jcr.datamodel.NullNodeData; import org.exoplatform.services.jcr.datamodel.PropertyData; import org.exoplatform.services.jcr.datamodel.QPath; import org.exoplatform.services.jcr.datamodel.QPathEntry; @@ -904,9 +905,12 @@ protected void putItem(final ItemData data) { cache.put(new CacheId(data.getIdentifier()), new CacheValue(data, System.currentTimeMillis() + liveTime)); - cache.put(new CacheQPath(data.getParentIdentifier(), data.getQPath()), new CacheValue(data, System - .currentTimeMillis() - + liveTime)); + if (data.getParentIdentifier() != null || !(data instanceof NullNodeData)) + { + cache.put(new CacheQPath(data.getParentIdentifier(), data.getQPath()), new CacheValue(data, System + .currentTimeMillis() + + liveTime)); + } } /** @@ -929,6 +933,11 @@ // add child item data to list of childs of the parent if (item.isNode()) { + if (item instanceof NullNodeData) + { + // Skip null values + return; + } // add child node List<NodeData> cachedParentChilds = nodesCache.get(item.getParentIdentifier()); if (cachedParentChilds != null) @@ -1300,8 +1309,11 @@ { cache.remove(k); - // remove by parentId + path - cache.remove(new CacheQPath(c.getParentIdentifier(), c.getQPath())); + if (c.getParentIdentifier() != null || !(c instanceof NullNodeData)) + { + // remove by parentId + path + cache.remove(new CacheQPath(c.getParentIdentifier(), c.getQPath())); + } // remove cached child lists if (c.isNode()) 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 2010-04-01 09:47:18 UTC (rev 2204) +++ jcr/trunk/exo.jcr.component.core/src/main/java/org/exoplatform/services/jcr/impl/dataflow/persistent/jbosscache/JBossCacheWorkspaceStorageCache.java 2010-04-01 10:28:23 UTC (rev 2205) @@ -28,6 +28,7 @@ import org.exoplatform.services.jcr.datamodel.InternalQName; import org.exoplatform.services.jcr.datamodel.ItemData; import org.exoplatform.services.jcr.datamodel.NodeData; +import org.exoplatform.services.jcr.datamodel.NullNodeData; import org.exoplatform.services.jcr.datamodel.PropertyData; import org.exoplatform.services.jcr.datamodel.QPath; import org.exoplatform.services.jcr.datamodel.QPathEntry; @@ -52,6 +53,7 @@ import java.util.Set; import javax.jcr.RepositoryException; +import javax.transaction.Transaction; import javax.transaction.TransactionManager; /** @@ -370,12 +372,53 @@ cache.setLocal(false); if (!inTransaction) { - cache.commitTransaction(); + dedicatedTxCommit(); } } } /** + * Allows to commit the cache changes in a dedicated XA Tx in order to avoid potential + * deadlocks + */ + private void dedicatedTxCommit() + { + // Ensure that the commit is done in a dedicated tx to avoid deadlock due + // to global XA Tx + TransactionManager tm = getTransactionManager(); + Transaction tx = null; + try + { + if (tm != null) + { + try + { + tx = tm.suspend(); + } + catch (Exception e) + { + LOG.warn("Cannot suspend the current transaction", e); + } + } + cache.commitTransaction(); + } + finally + { + if (tx != null) + { + try + { + tm.resume(tx); + } + catch (Exception e) + { + LOG.warn("Cannot resume the current transaction", e); + } + } + } + } + + /** * {@inheritDoc} */ public void remove(ItemData item) @@ -484,7 +527,7 @@ cache.setLocal(false); if (!inTransaction) { - cache.commitTransaction(); + dedicatedTxCommit(); } } } @@ -525,7 +568,7 @@ cache.setLocal(false); if (!inTransaction) { - cache.commitTransaction(); + dedicatedTxCommit(); } } } @@ -778,9 +821,9 @@ cache.put(makeChildFqn(childNodes, node.getParentIdentifier(), node.getQPath().getEntries()[node.getQPath() .getEntries().length - 1]), ITEM_ID, node.getIdentifier()); // if MODIFY and List present OR FORCE_MODIFY, then write - if ((modifyListsOfChild == ModifyChildOption.MODIFY && cache.getNode(makeChildListFqn(childNodesList, node - .getParentIdentifier())) != null) - || modifyListsOfChild == ModifyChildOption.FORCE_MODIFY) + if (!(node instanceof NullNodeData) + && ((modifyListsOfChild == ModifyChildOption.MODIFY && cache.getNode(makeChildListFqn(childNodesList, node + .getParentIdentifier())) != null) || modifyListsOfChild == ModifyChildOption.FORCE_MODIFY)) { cache.addToList(makeChildListFqn(childNodesList, node.getParentIdentifier()), ITEM_LIST, node .getIdentifier()); @@ -829,6 +872,14 @@ { cache.addToList(makeChildListFqn(childPropsList, prop.getParentIdentifier()), ITEM_LIST, prop.getIdentifier()); } + ItemData result = + get(prop.getParentIdentifier(), prop.getQPath().getEntries()[prop.getQPath().getEntries().length - 1]); + if (result instanceof NullNodeData) + { + // Remove null value if exists + cache.removeNode(makeChildFqn(childNodes, result.getParentIdentifier(), prop.getQPath().getEntries()[prop.getQPath().getEntries().length - 1])); + cache.removeNode(makeItemFqn(result.getIdentifier())); + } // add in ITEMS return (PropertyData)cache.put(makeItemFqn(prop.getIdentifier()), ITEM_DATA, prop); } @@ -883,7 +934,12 @@ */ protected void updateMixin(NodeData node) { - NodeData prevData = (NodeData)cache.put(makeItemFqn(node.getIdentifier()), ITEM_DATA, node); + Object oPrevValue = cache.put(makeItemFqn(node.getIdentifier()), ITEM_DATA, node); + if (oPrevValue instanceof NullNodeData) + { + oPrevValue = null; + } + NodeData prevData = (NodeData)oPrevValue; if (prevData != null) { // do update ACL if needed @@ -1022,6 +1078,10 @@ for (Iterator<NodeData> iter = new ChildNodesIterator<NodeData>(parentId); iter.hasNext();) { NodeData prevNode = iter.next(); + if (prevNode instanceof NullNodeData) + { + continue; + } // is ACL changes on this node (i.e. ACL inheritance brokes) for (InternalQName mixin : prevNode.getMixinTypeNames()) { Modified: jcr/trunk/exo.jcr.component.core/src/test/java/org/exoplatform/services/jcr/impl/dataflow/persistent/TestCacheableWorkspaceDataManager.java =================================================================== --- jcr/trunk/exo.jcr.component.core/src/test/java/org/exoplatform/services/jcr/impl/dataflow/persistent/TestCacheableWorkspaceDataManager.java 2010-04-01 09:47:18 UTC (rev 2204) +++ jcr/trunk/exo.jcr.component.core/src/test/java/org/exoplatform/services/jcr/impl/dataflow/persistent/TestCacheableWorkspaceDataManager.java 2010-04-01 10:28:23 UTC (rev 2205) @@ -25,6 +25,7 @@ import org.exoplatform.services.jcr.datamodel.ItemData; import org.exoplatform.services.jcr.datamodel.NodeData; import org.exoplatform.services.jcr.datamodel.PropertyData; +import org.exoplatform.services.jcr.datamodel.QPath; import org.exoplatform.services.jcr.datamodel.QPathEntry; import org.exoplatform.services.jcr.datamodel.ValueData; import org.exoplatform.services.jcr.impl.storage.SystemDataContainerHolder; @@ -163,7 +164,8 @@ public void testGetItemDataByNodeDataNQPathEntry() throws Exception { - final NodeData nodeData = new PersistedNodeData("getItemData", null, null, 0, 1, null, null, null); + final NodeData nodeData = + new PersistedNodeData("getItemData", new QPath(new QPathEntry[]{}), null, 0, 1, null, null, null); assertEquals(0, con.getItemDataByNodeDataNQPathEntryCalls.get()); MyTask task = new MyTask() {