Fix bugs in EphemeralFileSystemAbstraction

neo4j · Jan 20, 2017 · 60b36ee · 60b36ee
1 parent b2c6c60
commit 60b36ee
Show file tree

Hide file tree

Showing 4 changed files with 267 additions and 80 deletions.
diff --git a/community/io/src/test/java/org/neo4j/graphdb/mockfs/EphemeralFileSystemAbstraction.java b/community/io/src/test/java/org/neo4j/graphdb/mockfs/EphemeralFileSystemAbstraction.java
@@ -47,6 +47,7 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -377,18 +378,22 @@ public boolean deleteFile( File fileName )
     }
 
     @Override
-    public void deleteRecursively( File directory ) throws IOException
+    public void deleteRecursively( File path ) throws IOException
     {
-        List<String> directoryPathItems = splitPath( canonicalFile( directory ) );
-        for ( Map.Entry<File,EphemeralFileData> file : files.entrySet() )
+        if ( isDirectory( path ) )
         {
-            File fileName = file.getKey();
-            List<String> fileNamePathItems = splitPath( fileName );
-            if ( directoryMatches( directoryPathItems, fileNamePathItems ) )
+            List<String> directoryPathItems = splitPath( canonicalFile( path ) );
+            for ( Map.Entry<File,EphemeralFileData> file : files.entrySet() )
             {
-                deleteFile( fileName );
+                File fileName = file.getKey();
+                List<String> fileNamePathItems = splitPath( fileName );
+                if ( directoryMatches( directoryPathItems, fileNamePathItems ) )
+                {
+                    deleteFile( fileName );
+                }
             }
         }
+        deleteFile( path );
     }
 
     @Override
@@ -437,7 +442,7 @@ public File[] listFiles( File directory )
             List<String> fileNamePathItems = splitPath( file );
             if ( directoryMatches( directoryPathItems, fileNamePathItems ) )
             {
-                found.add( constructPath( fileNamePathItems, directoryPathItems.size() + 1 ) );
+                found.add( constructPath( fileNamePathItems, directoryPathItems ) );
             }
         }
 
@@ -463,7 +468,7 @@ public File[] listFiles( File directory, FilenameFilter filter )
             List<String> fileNamePathItems = splitPath( file );
             if ( directoryMatches( directoryPathItems, fileNamePathItems ) )
             {
-                File path = constructPath( fileNamePathItems, directoryPathItems.size() + 1 );
+                File path = constructPath( fileNamePathItems, directoryPathItems );
                 if ( filter.accept( path.getParentFile(), path.getName() ) )
                 {
                     found.add( path );
@@ -473,10 +478,15 @@ public File[] listFiles( File directory, FilenameFilter filter )
         return found.toArray( new File[found.size()] );
     }
 
-    private File constructPath( List<String> pathItems, int count )
+    private File constructPath( List<String> pathItems, List<String> base )
     {
         File file = null;
-        for ( String pathItem : pathItems.subList( 0, count ) )
+        if ( base.size() > 0 )
+        {
+            // We're not directly basing off the root directory
+            pathItems = pathItems.subList( 0, base.size() + 1 );
+        }
+        for ( String pathItem : pathItems )
         {
             file = file == null ? new File( pathItem ) : new File( file, pathItem );
         }
@@ -497,12 +507,25 @@ private List<String> splitPath( File path )
     @Override
     public void moveToDirectory( File file, File toDirectory ) throws IOException
     {
-        EphemeralFileData fileToMove = files.remove( canonicalFile( file ) );
-        if ( fileToMove == null )
+        if ( isDirectory( file ) )
+        {
+            File inner = new File( toDirectory, file.getName() );
+            mkdir( inner );
+            for ( File f : listFiles( file ) )
+            {
+                moveToDirectory( f, inner );
+            }
+            deleteFile( file );
+        }
+        else
         {
-            throw new FileNotFoundException( file.getPath() );
+            EphemeralFileData fileToMove = files.remove( canonicalFile( file ) );
+            if ( fileToMove == null )
+            {
+                throw new FileNotFoundException( file.getPath() );
+            }
+            files.put( canonicalFile( new File( toDirectory, file.getName() ) ), fileToMove );
         }
-        files.put( canonicalFile( new File( toDirectory, file.getName() ) ), fileToMove );
     }
 
     @Override
@@ -546,7 +569,7 @@ public long checksum()
         List<File> names = new ArrayList<>( files.size() );
         names.addAll( files.keySet() );
 
-        Collections.sort( names, ( o1, o2 ) -> o1.getAbsolutePath().compareTo( o2.getAbsolutePath() ) );
+        names.sort( Comparator.comparing( File::getAbsolutePath ) );
 
         for ( File name : names )
         {
@@ -888,14 +911,7 @@ public void pos( long position )
 
     private static class EphemeralFileData
     {
-        private static final ThreadLocal<byte[]> SCRATCH_PAD = new ThreadLocal<byte[]>()
-        {
-            @Override
-            protected byte[] initialValue()
-            {
-                return new byte[1024];
-            }
-        };
+        private static final ThreadLocal<byte[]> SCRATCH_PAD = ThreadLocal.withInitial( () -> new byte[1024] );
         private DynamicByteBuffer fileAsBuffer;
         private DynamicByteBuffer forcedBuffer;
         private final Collection<WeakReference<EphemeralFileChannel>> channels = new LinkedList<>();

diff --git a/community/io/src/test/java/org/neo4j/io/fs/DefaultFileSystemAbstractionTest.java b/community/io/src/test/java/org/neo4j/io/fs/DefaultFileSystemAbstractionTest.java
@@ -19,66 +19,25 @@
  */
 package org.neo4j.io.fs;
 
+import org.junit.Test;
+
 import java.io.File;
 import java.io.IOException;
 import java.util.UUID;
 
-import org.junit.Before;
-import org.junit.Test;
-
+import static java.lang.String.format;
 import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
+import static org.neo4j.io.fs.DefaultFileSystemAbstraction.UNABLE_TO_CREATE_DIRECTORY_FORMAT;
 
-public class DefaultFileSystemAbstractionTest
+public class DefaultFileSystemAbstractionTest extends FileSystemAbstractionTest
 {
-    private final DefaultFileSystemAbstraction defaultFileSystemAbstraction = new DefaultFileSystemAbstraction();
-
-    private File path;
-
-    @Before
-    public void before() throws Exception
-    {
-        path = new File( "target/" + UUID.randomUUID() );
-    }
-
-    @Test
-    public void shouldCreatePath() throws Exception
-    {
-        defaultFileSystemAbstraction.mkdirs( path );
-
-        assertThat( path.exists(), is( true ) );
-    }
-
-    @Test
-    public void shouldCreateDeepPath() throws Exception
+    @Override
+    protected FileSystemAbstraction buildFileSystemAbstraction()
     {
-        path = new File( path, UUID.randomUUID() + "/" + UUID.randomUUID() );
-
-        defaultFileSystemAbstraction.mkdirs( path );
-
-        assertThat( path.exists(), is( true ) );
-    }
-
-    @Test
-    public void shouldCreatePathThatAlreadyExists() throws Exception
-    {
-        assertTrue( path.mkdir() );
-
-        defaultFileSystemAbstraction.mkdirs( path );
-
-        assertThat( path.exists(), is( true ) );
-    }
-
-    @Test
-    public void shouldCreatePathThatPointsToFile() throws Exception
-    {
-        assertTrue( path.mkdir() );
-        path = new File( path, "some_file" );
-        assertTrue( path.createNewFile() );
-
-        defaultFileSystemAbstraction.mkdirs( path );
-
-        assertThat( path.exists(), is( true ) );
+        return new DefaultFileSystemAbstraction();
     }
 
     @Test
@@ -95,15 +54,15 @@ public boolean mkdirs()
 
         try
         {
-            defaultFileSystemAbstraction.mkdirs( path );
+            fsa.mkdirs( path );
 
             fail();
         }
         catch ( IOException e )
         {
-            assertThat( path.exists(), is( false ) );
-            assertThat( e.getMessage(), is( String.format( DefaultFileSystemAbstraction
-                    .UNABLE_TO_CREATE_DIRECTORY_FORMAT, path ) ) );
+            assertFalse( fsa.fileExists( path ) );
+            String expectedMessage = format( UNABLE_TO_CREATE_DIRECTORY_FORMAT, path );
+            assertThat( e.getMessage(), is( expectedMessage ) );
         }
     }
 }
diff --git a/community/io/src/test/java/org/neo4j/io/fs/EphemeralFileSystemAbstractionTest.java b/community/io/src/test/java/org/neo4j/io/fs/EphemeralFileSystemAbstractionTest.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2002-2017 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU 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.neo4j.io.fs;
+
+import org.junit.After;
+
+import org.neo4j.graphdb.mockfs.EphemeralFileSystemAbstraction;
+
+public class EphemeralFileSystemAbstractionTest extends FileSystemAbstractionTest
+{
+    @Override
+    protected FileSystemAbstraction buildFileSystemAbstraction()
+    {
+        return new EphemeralFileSystemAbstraction();
+    }
+
+    @After
+    public void tearDown()
+    {
+        ((EphemeralFileSystemAbstraction) fsa).shutdown();
+    }
+}