[exo-jcr-commits] exo-jcr SVN: r1089 - jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache.

do-not-reply at jboss.org do-not-reply at jboss.org
Wed Dec 16 05:29:59 EST 2009


Author: nfilotto
Date: 2009-12-16 05:29:59 -0500 (Wed, 16 Dec 2009)
New Revision: 1089

Added:
   jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JBossCacheStorageConnectionContextHolder.java
Removed:
   jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JDBCStorageConnectionFactory.java
Modified:
   jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JBossCacheStorageConnection.java
   jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JDBCCacheLoader.java
Log:
EXOJCR-321: Prevent useless database access for the methods add(NodeData) and add(PropertyData)
Just before the put I add the id into a context and in the CacheLoader, I check if the id that I need to retrieve from the database is new if it is new, I return null otherwise it accesses to the database

Modified: jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JBossCacheStorageConnection.java
===================================================================
--- jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JBossCacheStorageConnection.java	2009-12-16 09:33:10 UTC (rev 1088)
+++ jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JBossCacheStorageConnection.java	2009-12-16 10:29:59 UTC (rev 1089)
@@ -197,14 +197,11 @@
          }
       }
 
+      // Define the item id as a new id to prevent useless data access
+      JBossCacheStorageConnectionContextHolder.addNewItemsId(dataContainer, data.getIdentifier());
       // add in NODES
-      value =
-         cache.put(Fqn.fromRelativeFqn(Fqn.fromElements(JBossCacheStorage.NODES), makeNodeFqn(data.getIdentifier())),
-            ITEM_DATA, data);
-      if (value != null)
-      {
-         throw new ItemExistsException("Node already exists " + data.getQPath().getAsString());
-      }
+      cache.put(Fqn.fromRelativeFqn(Fqn.fromElements(JBossCacheStorage.NODES), makeNodeFqn(data.getIdentifier())),
+         ITEM_DATA, data);
    }
 
    /**
@@ -232,16 +229,12 @@
          throw new ItemExistsException("Property already exists " + data.getQPath().getAsString());
       }
 
+      // Define the item id as a new id to prevent useless data access
+      JBossCacheStorageConnectionContextHolder.addNewItemsId(dataContainer, data.getIdentifier());      
       // add in PROPERTIES
-      value =
-         cache.put(Fqn.fromRelativeFqn(Fqn.fromElements(JBossCacheStorage.PROPS), makePropFqn(data.getIdentifier())),
-            ITEM_DATA, data);
+      cache.put(Fqn.fromRelativeFqn(Fqn.fromElements(JBossCacheStorage.PROPS), makePropFqn(data.getIdentifier())),
+         ITEM_DATA, data);
 
-      if (value != null)
-      {
-         throw new ItemExistsException("Property already exists " + data.getQPath().getAsString());
-      }
-
       // REFERENCEs hadnling
       if (data.getType() == PropertyType.REFERENCE)
       {
@@ -1044,7 +1037,7 @@
     */
    public void close() throws IllegalStateException, RepositoryException
    {
-      JDBCStorageConnectionFactory.close(dataContainer);
+      JBossCacheStorageConnectionContextHolder.close(dataContainer);
    }
 
    /**

Copied: jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JBossCacheStorageConnectionContextHolder.java (from rev 1083, jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JDBCStorageConnectionFactory.java)
===================================================================
--- jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JBossCacheStorageConnectionContextHolder.java	                        (rev 0)
+++ jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JBossCacheStorageConnectionContextHolder.java	2009-12-16 10:29:59 UTC (rev 1089)
@@ -0,0 +1,222 @@
+/*
+ * Copyright (C) 2009 eXo Platform SAS.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 3 of
+ * the License, or (at your option) any later version.
+ *
+ * This software 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this software; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+ */
+package org.exoplatform.services.jcr.impl.storage.jbosscache;
+
+import org.exoplatform.services.jcr.impl.storage.jdbc.JDBCStorageConnection;
+import org.exoplatform.services.jcr.storage.WorkspaceDataContainer;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import javax.jcr.RepositoryException;
+
+/**
+ * This class is used to holds the full context during the live time of
+ * the {@link JBossCacheStorageConnection}. We keep one
+ * {@link JDBCStorageConnection} per JDBCStorageConnection into a
+ * {@link ThreadLocal}
+ * 
+ * @author nicolasfilotto
+ * 
+ */
+public class JBossCacheStorageConnectionContextHolder
+{
+
+   /**
+    * The {@link ThreadLocal} in which we store all the current contexts
+    */
+   private static final ThreadLocal<Map<WorkspaceDataContainer, JBossCacheStorageConnectionContext>> CONTEXTS =
+      new ThreadLocal<Map<WorkspaceDataContainer, JBossCacheStorageConnectionContext>>();
+
+   /**
+    * Indicates if we are in a debug mode
+    */
+   public static AtomicBoolean DEBUG = new AtomicBoolean(false);
+
+   /**
+    * The total amount of connections. Used for debugging purpose only.
+    */
+   public static AtomicInteger TOTAL_CONNECTION = new AtomicInteger(0);
+
+   /**
+    * Indicates if we display the stack trace into the output stream. Used for
+    * debugging purpose only.
+    */
+   public static AtomicBoolean SHOW_TRACE = new AtomicBoolean(false);
+
+   /**
+    * Gives the current opened connection for the related
+    * {@link WorkspaceDataContainer} if it exists, returns a new one otherwise
+    * 
+    * @param dataContainer
+    *            the data container for which we want a connection
+    * @return an opened connection
+    * @throws RepositoryException
+    *             if an error occurs
+    */
+   static JDBCStorageConnection getConnection(WorkspaceDataContainer dataContainer) throws RepositoryException
+   {
+      if (DEBUG.get())
+      {
+         if (SHOW_TRACE.get())
+         {
+            (new Exception()).printStackTrace();
+         }
+         TOTAL_CONNECTION.incrementAndGet();
+      }
+      final JBossCacheStorageConnectionContext context = getContext(dataContainer);
+      return context.getConnection();
+   }
+
+   /**
+    * Close the context related to the given {@link WorkspaceDataContainer}
+    * 
+    * @param dataContainer
+    *            the data container for which we want to close the context
+    * @throws IllegalStateException
+    *             if connection is already closed
+    * @throws RepositoryException
+    *             if some exception occured
+    */
+   static void close(WorkspaceDataContainer dataContainer) throws IllegalStateException, RepositoryException
+   {
+      final Map<WorkspaceDataContainer, JBossCacheStorageConnectionContext> contexts = CONTEXTS.get();
+      if (contexts == null)
+      {
+         return;
+      }
+      final JBossCacheStorageConnectionContext context = contexts.remove(dataContainer);
+      if (context == null)
+      {
+         return;
+      }
+      context.close();
+   }
+   
+   /**
+    * Defines an item id has a new id. It is used to prevent the CacheLoader to access
+    * the database in order to know if the item already exists 
+    * @param dataContainer
+    *            the data container for which we want to define a new item id
+    * @param id the id of the new item
+    */
+   static void addNewItemsId(WorkspaceDataContainer dataContainer, String id)
+   {
+      final JBossCacheStorageConnectionContext context = getContext(dataContainer);
+      context.addNewItemsId(id);
+   }
+   
+   /**
+    * Checks if a given id is an id of a new item
+    * @param dataContainer
+    *            the data container for which we want to check the item id
+    * @param id the item id
+    * @return <code>true</code> id the id is known as a new item id, <code>false</code>
+    * otherwise
+    */
+   static boolean isNewItem(WorkspaceDataContainer dataContainer, String id)
+   {
+      final JBossCacheStorageConnectionContext context = getContext(dataContainer);
+      return context.isNewItem(id);      
+   }
+   
+   /**
+    * Gives the current context for the related data container
+    */
+   private static JBossCacheStorageConnectionContext getContext(WorkspaceDataContainer dataContainer)
+   {
+      Map<WorkspaceDataContainer, JBossCacheStorageConnectionContext> contexts = CONTEXTS.get();
+      if (contexts == null)
+      {
+         contexts = new HashMap<WorkspaceDataContainer, JBossCacheStorageConnectionContext>();
+         CONTEXTS.set(contexts);
+      }
+      JBossCacheStorageConnectionContext context = contexts.get(dataContainer);
+      if (context == null)
+      {
+         context = new JBossCacheStorageConnectionContext(dataContainer);
+         contexts.put(dataContainer, context);
+      }
+      return context;
+   }
+   
+   /**
+    * The full context of a {@link JBossCacheStorageConnection}
+    */
+   private static class JBossCacheStorageConnectionContext
+   {
+      /**
+       * The current connection
+       */
+      private JDBCStorageConnection connection;
+    
+      /**
+       * The list of all the ids of the new items
+       */
+      private Set<String> newItemsId;
+      
+      /**
+       * The related data container
+       */
+      private final WorkspaceDataContainer dataContainer;
+      
+      public JBossCacheStorageConnectionContext(WorkspaceDataContainer dataContainer)
+      {
+         this.dataContainer = dataContainer;
+      }
+
+      public JDBCStorageConnection getConnection() throws RepositoryException
+      {
+         if (connection == null || !connection.isOpened())
+         {
+            connection = (JDBCStorageConnection)dataContainer.openConnection();
+         }         
+         return connection;
+      }
+
+      public boolean isNewItem(String id)
+      {
+         return newItemsId != null && newItemsId.contains(id);
+      }
+
+      public void addNewItemsId(String id)
+      {
+         if (newItemsId == null)
+         {
+            this.newItemsId = new HashSet<String>();
+         }
+         newItemsId.add(id);
+      }
+      
+      public void close() throws IllegalStateException, RepositoryException
+      {
+         if (connection == null || !connection.isOpened())
+         {
+            return;
+         }
+         connection.close();
+         connection = null;         
+      }
+   }
+}

Modified: jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JDBCCacheLoader.java
===================================================================
--- jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JDBCCacheLoader.java	2009-12-16 09:33:10 UTC (rev 1088)
+++ jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JDBCCacheLoader.java	2009-12-16 10:29:59 UTC (rev 1089)
@@ -163,7 +163,7 @@
          LOG.debug("Modifications list  size = " + modifications.size());
       }
 
-      JDBCStorageConnection conn = JDBCStorageConnectionFactory.get(dataContainer);
+      JDBCStorageConnection conn = JBossCacheStorageConnectionContextHolder.getConnection(dataContainer);
 
       try
       {
@@ -319,8 +319,12 @@
             // /$NODES/<NODE_ID>
             if (name.size() == 2)
             {
-               JDBCStorageConnection conn = JDBCStorageConnectionFactory.get(dataContainer);
                String nodeId = name.getLastElementAsString();
+               if (JBossCacheStorageConnectionContextHolder.isNewItem(dataContainer, nodeId))
+               {
+                  return null;
+               }               
+               JDBCStorageConnection conn = JBossCacheStorageConnectionContextHolder.getConnection(dataContainer);
                NodeData nodeData = (NodeData) conn.getItemData(nodeId);
                if (nodeData != null)
                {
@@ -346,7 +350,7 @@
             // /$NODES/<NODE_ID>/<SUB_NODE_NAME>
             else if (name.size() == 3)
             {
-               JDBCStorageConnection conn = JDBCStorageConnectionFactory.get(dataContainer);
+               JDBCStorageConnection conn = JBossCacheStorageConnectionContextHolder.getConnection(dataContainer);
                QPathEntry childNodeName = QPathEntry.parse(name.getLastElementAsString());
                String parentId = (String) name.get(1);
 
@@ -373,8 +377,12 @@
          {
             if (name.size() == 2)
             {
-               JDBCStorageConnection conn = JDBCStorageConnectionFactory.get(dataContainer);
                String propertyId = name.getLastElementAsString();
+               if (JBossCacheStorageConnectionContextHolder.isNewItem(dataContainer, propertyId))
+               {
+                  return null;
+               }
+               JDBCStorageConnection conn = JBossCacheStorageConnectionContextHolder.getConnection(dataContainer);
                PropertyData propertyData = (PropertyData) conn.getItemData(propertyId);
                if (propertyData != null)
                {
@@ -397,7 +405,7 @@
             // get reference
             if (name.size() == 3)
             {
-               JDBCStorageConnection conn = JDBCStorageConnectionFactory.get(dataContainer);
+               JDBCStorageConnection conn = JBossCacheStorageConnectionContextHolder.getConnection(dataContainer);
                // do we have such property reference
                String nodeID = String.valueOf(name.get(1));
                String propID = String.valueOf(name.get(2));
@@ -413,7 +421,7 @@
             }
             else if (name.size() == 2)
             {
-               JDBCStorageConnection conn = JDBCStorageConnectionFactory.get(dataContainer);
+               JDBCStorageConnection conn = JBossCacheStorageConnectionContextHolder.getConnection(dataContainer);
                // do we have node with any reference
                String nodeID = name.getLastElementAsString();
                if (conn.getReferenceIdentifiers(nodeID).size() > 0)
@@ -476,7 +484,7 @@
             // /$NODES/<NODE_ID>
             if (name.size() == 2)
             {
-               JDBCStorageConnection conn = JDBCStorageConnectionFactory.get(dataContainer);
+               JDBCStorageConnection conn = JBossCacheStorageConnectionContextHolder.getConnection(dataContainer);
                String nodeId = name.getLastElementAsString();
                String nodeName = conn.getNodeName(nodeId);
 
@@ -492,7 +500,7 @@
             // /$NODES/<NODE_ID>/<SUB_NODE_NAME>
             else if (name.size() == 3)
             {
-               JDBCStorageConnection conn = JDBCStorageConnectionFactory.get(dataContainer);
+               JDBCStorageConnection conn = JBossCacheStorageConnectionContextHolder.getConnection(dataContainer);
                QPathEntry childNodeName = QPathEntry.parse(name.getLastElementAsString());
                String parentId = (String) name.get(1);
 
@@ -518,7 +526,7 @@
          {
             if (name.size() == 2)
             {
-               JDBCStorageConnection conn = JDBCStorageConnectionFactory.get(dataContainer);
+               JDBCStorageConnection conn = JBossCacheStorageConnectionContextHolder.getConnection(dataContainer);
                String propertyId = name.getLastElementAsString();
                String propertyName = conn.getPropertyName(propertyId);
 
@@ -542,14 +550,14 @@
             // /$REFS/<NODE_ID>
             if (name.size() == 2)
             {
-               JDBCStorageConnection conn = JDBCStorageConnectionFactory.get(dataContainer);
+               JDBCStorageConnection conn = JBossCacheStorageConnectionContextHolder.getConnection(dataContainer);
                String nodeId = name.getLastElementAsString();
                exists = conn.getReferenceIdentifiers(nodeId).size() > 0;
             }
             // /$REFS/<NODE_ID>/<REF_PROP_ID>
             else if (name.size() == 3)
             {
-               JDBCStorageConnection conn = JDBCStorageConnectionFactory.get(dataContainer);
+               JDBCStorageConnection conn = JBossCacheStorageConnectionContextHolder.getConnection(dataContainer);
                String refPropertyId = name.getLastElementAsString();
                String nodeId = (String) name.get(1);
 
@@ -605,12 +613,12 @@
          String nodeId = (String) name.get(1);
          if (name.get(0).equals(JBossCacheStorage.NODES))
          {
-            JDBCStorageConnection conn = JDBCStorageConnectionFactory.get(dataContainer);
+            JDBCStorageConnection conn = JBossCacheStorageConnectionContextHolder.getConnection(dataContainer);
             return conn.getChildNodeNames(nodeId);
          }
          else if (name.get(0).equals(JBossCacheStorage.REFS))
          {
-            JDBCStorageConnection conn = JDBCStorageConnectionFactory.get(dataContainer);
+            JDBCStorageConnection conn = JBossCacheStorageConnectionContextHolder.getConnection(dataContainer);
             List<String> references = conn.getReferenceIdentifiers(nodeId);
             Set<String> result = new LinkedHashSet<String>();
             result.addAll(references);
@@ -689,7 +697,7 @@
     */
    public void prepare(Object tx, List<Modification> modifications, boolean onePhase) throws Exception
    {
-      final JDBCStorageConnection conn = JDBCStorageConnectionFactory.get(dataContainer);
+      final JDBCStorageConnection conn = JBossCacheStorageConnectionContextHolder.getConnection(dataContainer);
       apply(modifications, conn);
       conn.commit();
       

Deleted: jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JDBCStorageConnectionFactory.java
===================================================================
--- jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JDBCStorageConnectionFactory.java	2009-12-16 09:33:10 UTC (rev 1088)
+++ jcr/branches/1.12.0-JBC/component/core/src/main/java/org/exoplatform/services/jcr/impl/storage/jbosscache/JDBCStorageConnectionFactory.java	2009-12-16 10:29:59 UTC (rev 1089)
@@ -1,127 +0,0 @@
-/*
- * Copyright (C) 2009 eXo Platform SAS.
- *
- * This is free software; you can redistribute it and/or modify it
- * under the terms of the GNU Lesser General Public License as
- * published by the Free Software Foundation; either version 3 of
- * the License, or (at your option) any later version.
- *
- * This software 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
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this software; if not, write to the Free
- * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
- * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
- */
-package org.exoplatform.services.jcr.impl.storage.jbosscache;
-
-import org.exoplatform.services.jcr.impl.storage.jdbc.JDBCStorageConnection;
-import org.exoplatform.services.jcr.storage.WorkspaceDataContainer;
-
-import java.util.HashMap;
-import java.util.Map;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicInteger;
-
-import javax.jcr.RepositoryException;
-
-/**
- * This class is used to holds the JDBC connection open during the live time of
- * the {@link JBossCacheStorageConnection}. We keep one
- * {@link JDBCStorageConnection} per JDBCStorageConnection into a
- * {@link ThreadLocal}
- * 
- * @author nicolasfilotto
- * 
- */
-public class JDBCStorageConnectionFactory
-{
-
-   /**
-    * The {@link ThreadLocal} in which we store all the current opened
-    * connections
-    */
-   private static final ThreadLocal<Map<WorkspaceDataContainer, JDBCStorageConnection>> CONNECTIONS =
-      new ThreadLocal<Map<WorkspaceDataContainer, JDBCStorageConnection>>();
-
-   /**
-    * Indicates if we are in a debug mode
-    */
-   public static AtomicBoolean DEBUG = new AtomicBoolean(false);
-
-   /**
-    * The total amount of connections. Used for debugging purpose only.
-    */
-   public static AtomicInteger TOTAL_CONNECTION = new AtomicInteger(0);
-
-   /**
-    * Indicates if we display the stack trace into the output stream. Used for
-    * debugging purpose only.
-    */
-   public static AtomicBoolean SHOW_TRACE = new AtomicBoolean(false);
-
-   /**
-    * Gives the current opened connection for the related
-    * {@link WorkspaceDataContainer} if it exists, returns a new one otherwise
-    * 
-    * @param dataContainer
-    *            the data container for which we want a connection
-    * @return an opened connection
-    * @throws RepositoryException
-    *             if an error occurs
-    */
-   static JDBCStorageConnection get(WorkspaceDataContainer dataContainer) throws RepositoryException
-   {
-      if (DEBUG.get())
-      {
-         if (SHOW_TRACE.get())
-         {
-            (new Exception()).printStackTrace();
-         }
-         TOTAL_CONNECTION.incrementAndGet();
-      }
-      Map<WorkspaceDataContainer, JDBCStorageConnection> connections = CONNECTIONS.get();
-      if (connections == null)
-      {
-         connections = new HashMap<WorkspaceDataContainer, JDBCStorageConnection>();
-         CONNECTIONS.set(connections);
-      }
-      JDBCStorageConnection connection = connections.get(dataContainer);
-      if (connection == null || !connection.isOpened())
-      {
-         connection = (JDBCStorageConnection)dataContainer.openConnection();
-         connections.put(dataContainer, connection);
-      }
-      return connection;
-   }
-
-   /**
-    * Close the current opened connection related to the given
-    * {@link WorkspaceDataContainer}
-    * 
-    * @param dataContainer
-    *            the data container for which we want to close the connection
-    * @throws IllegalStateException
-    *             if connection is already closed
-    * @throws RepositoryException
-    *             if some exception occured
-    */
-   static void close(WorkspaceDataContainer dataContainer) throws IllegalStateException, RepositoryException
-   {
-      Map<WorkspaceDataContainer, JDBCStorageConnection> connections = CONNECTIONS.get();
-      if (connections == null)
-      {
-         return;
-      }
-      JDBCStorageConnection connection = connections.remove(dataContainer);
-      if (connection == null || !connection.isOpened())
-      {
-         return;
-      }
-      connection.close();
-      connection = null;
-   }
-}



More information about the exo-jcr-commits mailing list