Changeset 6509

Timestamp:

Aug 8, 2014, 11:32:09 AM (8 years ago)

Author:

Nicklas Nordborg

Message:

Fixes #1828: Calling Directory.getNew() multiple times with overlapping non-existing paths

A thread-local cache of directories that has already been created in the current transaction are used to keep track so that the same directory is not created multiple times.

Location:

branches/3.3-stable/src

Files:

• core/net/sf/basedb/core/Directory.java (modified) (5 diffs)
• test/TestDirectory.java (modified) (3 diffs)

branches/3.3-stable/src/core/net/sf/basedb/core/Directory.java

@@ -29 +29 @@
 import net.sf.basedb.core.query.Restrictions;
 
+import java.util.HashMap;
 import java.util.LinkedList;
+import java.util.Map;
 import java.util.Set;
 
@@ -106 +108 @@
   }
 
+  private static ThreadLocal<Map<String, Directory>> DIRECTORY_CACHE = new ThreadLocal<Map<String,Directory>>()
+  {
+    @Override
+    protected Map<String, Directory> initialValue() 
+    {
+      return new HashMap<String, Directory>();
+    }
+  };
+  
   /**
     Create a new or existing <code>Directory</code> from a path. Every 
@@ -137 +148 @@
     Directory currentDir = null;
     boolean creating = false;
+    String currentPath = "";
+    Map<String, Directory> directoryCache = null;
+    
     org.hibernate.Query query = HibernateUtil.getPredefinedQuery(
         dc.getHibernateSession(), "GET_SUBDIRECTORY_WITH_NAME");
@@ -148 +162 @@
     {
       String subDirName = path.getDirectory(i);
+      currentPath +="/" + subDirName;
       if (!creating)
       {
@@ -168 +183 @@
       if (creating)
       {
-        // This must be permission checked
-        Directory newSubDir = currentDir.newSubDirectory();
-        newSubDir.setName(subDirName);
-        dc.saveItem(newSubDir);
-        currentDir = newSubDir;
+        directoryCache = DIRECTORY_CACHE.get();
+        if (directoryCache.size() == 0)
+        {
+          // Clean up when transaction is about to complete
+          dc.addTransactionalAction(
+            new TransactionalAction() 
+            {
+              @Override
+              public void onRollback()
+              {}
+              
+              @Override
+              public void onBeforeCommit() 
+              {
+                DIRECTORY_CACHE.remove();
+              }
+              
+              @Override
+              public void onAfterCommit() 
+              {}
+            }
+          );
+        }
+        // Check cache if we have already created the subdir
+        if (directoryCache.containsKey(currentPath))
+        {
+          currentDir = directoryCache.get(currentPath);
+        }
+        else
+        {
+          // This must be permission checked
+          Directory newSubDir = currentDir.newSubDirectory();
+          newSubDir.setName(subDirName);
+          dc.saveItem(newSubDir);
+          currentDir = newSubDir;
+          directoryCache.put(currentPath, currentDir);
+        }
       }
     }

branches/3.3-stable/src/test/TestDirectory.java

@@ -65 +65 @@
     test_list(id1);
     
-    int[] pid = test_create_multiple("/foo", "/foo/bar1", "/foo/bar2", "/foo/bar1/sub1");
+    int[] pid = test_create_multiple("/foo", "/foo/bar1", "/foo/bar2", "/foo/bar1/sub1", "/foo/bar2/sub1", "/foo/bar2/sub2");
     
     if (TestUtil.waitBeforeDelete()) TestUtil.waitForEnter();
@@ -153 +153 @@
   {
     if (!TestUtil.hasPermission(Permission.CREATE, Item.DIRECTORY)) return null;
+    Directory d[] = new Directory[path.length];
     int id[] = new int[path.length];
     DbControl dc = null;
@@ -161 +162 @@
       {
         Path p = new Path(path[i], Path.Type.DIRECTORY);
-        Directory d = Directory.getNew(dc, p);
-        write_item(i, d);
-        id[i] = d.getId();
+        d[i] = Directory.getNew(dc, p);
       }
       dc.commit();
+      
+      // Copy id values
+      dc = TestUtil.getDbControl();
+      for (int i = 0; i < path.length; i++)
+      {
+        id[i] = d[i].getId();
+        dc.reattachItem(d[i], false);
+        write_item(i, d[i]);
+      }
+      
       write("--Create multiple by path OK");
     }