[exo-jcr-commits] exo-jcr SVN: r2205 - in jcr/trunk/exo.jcr.component.core/src: main/java/org/exoplatform/services/jcr/impl/dataflow/persistent and 2 other directories.

do-not-reply at jboss.org do-not-reply at jboss.org
Thu Apr 1 06:28:24 EDT 2010


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<http://www.gnu.org/licenses/>.
+ */
+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 at 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()
       {



More information about the exo-jcr-commits mailing list