From 074b82482f64c5173a687f79bac16e73f0ea9ddf Mon Sep 17 00:00:00 2001 From: MishaDemianenko Date: Thu, 16 Nov 2017 22:26:15 +0100 Subject: [PATCH] Adapt Load and Dump commands for possibility to have have transaction logs in a non default location. --- .../main/java/org/neo4j/commandline/Util.java | 14 ++++ .../neo4j/commandline/dbms/DumpCommand.java | 27 ++++-- .../neo4j/commandline/dbms/LoadCommand.java | 35 ++++++-- .../java/org/neo4j/dbms/archive/Dumper.java | 36 +++++--- .../java/org/neo4j/dbms/archive/Loader.java | 52 +++++++++--- .../commandline/dbms/DumpCommandTest.java | 62 +++++++++----- .../commandline/dbms/LoadCommandTest.java | 53 ++++++++---- .../org/neo4j/commandline/dbms/UtilTest.java | 34 ++++++++ .../org/neo4j/dbms/archive/ArchiveTest.java | 39 +++++++-- .../org/neo4j/dbms/archive/DumperTest.java | 19 ++--- .../org/neo4j/dbms/archive/LoaderTest.java | 82 +++++++++++++++---- .../log/files/TransactionLogFilesHelper.java | 2 +- .../neo4j/restore/RestoreDatabaseCommand.java | 4 +- .../restore/RestoreDatabaseCommandIT.java | 23 ++++++ 14 files changed, 378 insertions(+), 104 deletions(-) diff --git a/community/command-line/src/main/java/org/neo4j/commandline/Util.java b/community/command-line/src/main/java/org/neo4j/commandline/Util.java index cbbaccfa0c2dc..d724e529b0ed5 100644 --- a/community/command-line/src/main/java/org/neo4j/commandline/Util.java +++ b/community/command-line/src/main/java/org/neo4j/commandline/Util.java @@ -63,6 +63,20 @@ public static Path canonicalPath( File file ) throws IllegalArgumentException } } + public static boolean isSameOrChildFile( File parent, File candidate ) + { + Path canonicalCandidate = canonicalPath( candidate ); + Path canonicalParentPath = canonicalPath( parent ); + return canonicalCandidate.startsWith( canonicalParentPath ); + } + + public static boolean isSameOrChildPath( Path parent, Path candidate ) + { + Path normalizedCandidate = candidate.normalize(); + Path normalizedParent = parent.normalize(); + return normalizedCandidate.startsWith( normalizedParent ); + } + public static void checkLock( Path databaseDirectory ) throws CommandFailed { try ( FileSystemAbstraction fileSystem = new DefaultFileSystemAbstraction(); diff --git a/community/dbms/src/main/java/org/neo4j/commandline/dbms/DumpCommand.java b/community/dbms/src/main/java/org/neo4j/commandline/dbms/DumpCommand.java index 6691840a32b74..4549164726262 100644 --- a/community/dbms/src/main/java/org/neo4j/commandline/dbms/DumpCommand.java +++ b/community/dbms/src/main/java/org/neo4j/commandline/dbms/DumpCommand.java @@ -42,6 +42,7 @@ import static java.lang.String.format; import static org.neo4j.commandline.Util.canonicalPath; import static org.neo4j.graphdb.factory.GraphDatabaseSettings.database_path; +import static org.neo4j.graphdb.factory.GraphDatabaseSettings.logical_logs_location; public class DumpCommand implements AdminCommand { @@ -66,7 +67,10 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed { String database = arguments.parse( args ).get( "database" ); Path archive = calculateArchive( database, arguments.getMandatoryPath( "to" ) ); - Path databaseDirectory = canonicalPath( toDatabaseDirectory( database ) ); + + Config config = buildConfig( database ); + Path databaseDirectory = canonicalPath( getDatabaseDirectory( config ) ); + Path transactionLogsDirectory = canonicalPath( getTransactionalLogsDirectory( config ) ); try { @@ -79,7 +83,7 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed try ( Closeable ignored = StoreLockChecker.check( databaseDirectory ) ) { - dump( database, databaseDirectory, archive ); + dump( database, databaseDirectory, transactionLogsDirectory, archive ); } catch ( StoreLockException e ) { @@ -97,13 +101,23 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed } } - private Path toDatabaseDirectory( String databaseName ) + private Path getDatabaseDirectory( Config config ) + { + return config.get( database_path ).toPath(); + } + + private Path getTransactionalLogsDirectory( Config config ) + { + return config.get( logical_logs_location ).toPath(); + } + + private Config buildConfig( String databaseName ) { return Config.fromFile( configDir.resolve( Config.DEFAULT_CONFIG_FILE_NAME ) ) .withHome( homeDir ) .withConnectorsDisabled() .withSetting( GraphDatabaseSettings.active_database, databaseName ) - .build().get( database_path ).toPath(); + .build(); } private Path calculateArchive( String database, Path to ) @@ -111,11 +125,12 @@ private Path calculateArchive( String database, Path to ) return Files.isDirectory( to ) ? to.resolve( database + ".dump" ) : to; } - private void dump( String database, Path databaseDirectory, Path archive ) throws CommandFailed + private void dump( String database, Path databaseDirectory, Path transactionalLogsDirectory, Path archive ) + throws CommandFailed { try { - dumper.dump( databaseDirectory, archive, this::isStoreLock ); + dumper.dump( databaseDirectory, transactionalLogsDirectory, archive, this::isStoreLock ); } catch ( FileAlreadyExistsException e ) { diff --git a/community/dbms/src/main/java/org/neo4j/commandline/dbms/LoadCommand.java b/community/dbms/src/main/java/org/neo4j/commandline/dbms/LoadCommand.java index df8f057c8bd32..914e0aee7f6f4 100644 --- a/community/dbms/src/main/java/org/neo4j/commandline/dbms/LoadCommand.java +++ b/community/dbms/src/main/java/org/neo4j/commandline/dbms/LoadCommand.java @@ -41,8 +41,10 @@ import static java.util.Objects.requireNonNull; import static org.neo4j.commandline.Util.canonicalPath; import static org.neo4j.commandline.Util.checkLock; +import static org.neo4j.commandline.Util.isSameOrChildPath; import static org.neo4j.commandline.Util.wrapIOException; import static org.neo4j.graphdb.factory.GraphDatabaseSettings.database_path; +import static org.neo4j.graphdb.factory.GraphDatabaseSettings.logical_logs_location; public class LoadCommand implements AdminCommand { @@ -74,22 +76,35 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed String database = arguments.get( "database" ); boolean force = arguments.getBoolean( "force" ); - Path databaseDirectory = canonicalPath( toDatabaseDirectory( database ) ); + Config config = buildConfig( database ); - deleteIfNecessary( databaseDirectory, force ); - load( archive, database, databaseDirectory ); + Path databaseDirectory = canonicalPath( getDatabaseDirectory( config ) ); + Path transactionLogsDirectory = canonicalPath( getTransactionalLogsDirectory( config ) ); + + deleteIfNecessary( databaseDirectory, transactionLogsDirectory, force ); + load( archive, database, databaseDirectory, transactionLogsDirectory ); + } + + private Path getDatabaseDirectory( Config config ) + { + return config.get( database_path ).toPath(); + } + + private Path getTransactionalLogsDirectory( Config config ) + { + return config.get( logical_logs_location ).toPath(); } - private Path toDatabaseDirectory( String databaseName ) + private Config buildConfig( String databaseName ) { return Config.fromFile( configDir.resolve( Config.DEFAULT_CONFIG_FILE_NAME ) ) .withHome( homeDir ) .withConnectorsDisabled() .withSetting( GraphDatabaseSettings.active_database, databaseName ) - .build().get( database_path ).toPath(); + .build(); } - private void deleteIfNecessary( Path databaseDirectory, boolean force ) throws CommandFailed + private void deleteIfNecessary( Path databaseDirectory, Path transactionLogsDirectory, boolean force ) throws CommandFailed { try { @@ -97,6 +112,10 @@ private void deleteIfNecessary( Path databaseDirectory, boolean force ) throws C { checkLock( databaseDirectory ); FileUtils.deletePathRecursively( databaseDirectory ); + if ( !isSameOrChildPath( databaseDirectory, transactionLogsDirectory ) ) + { + FileUtils.deletePathRecursively( transactionLogsDirectory ); + } } } catch ( IOException e ) @@ -105,11 +124,11 @@ private void deleteIfNecessary( Path databaseDirectory, boolean force ) throws C } } - private void load( Path archive, String database, Path databaseDirectory ) throws CommandFailed + private void load( Path archive, String database, Path databaseDirectory, Path transactionLogsDirectory ) throws CommandFailed { try { - loader.load( archive, databaseDirectory ); + loader.load( archive, databaseDirectory, transactionLogsDirectory ); } catch ( NoSuchFileException e ) { diff --git a/community/dbms/src/main/java/org/neo4j/dbms/archive/Dumper.java b/community/dbms/src/main/java/org/neo4j/dbms/archive/Dumper.java index 69e471f3eb142..e7152d3b20adc 100644 --- a/community/dbms/src/main/java/org/neo4j/dbms/archive/Dumper.java +++ b/community/dbms/src/main/java/org/neo4j/dbms/archive/Dumper.java @@ -19,6 +19,11 @@ */ package org.neo4j.dbms.archive; +import org.apache.commons.compress.archivers.ArchiveEntry; +import org.apache.commons.compress.archivers.ArchiveOutputStream; +import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream; +import org.apache.commons.compress.compressors.gzip.GzipCompressorOutputStream; + import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; @@ -26,11 +31,7 @@ import java.nio.file.StandardOpenOption; import java.util.function.Predicate; -import org.apache.commons.compress.archivers.ArchiveEntry; -import org.apache.commons.compress.archivers.ArchiveOutputStream; -import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream; -import org.apache.commons.compress.compressors.gzip.GzipCompressorOutputStream; - +import org.neo4j.commandline.Util; import org.neo4j.function.ThrowingAction; import static org.neo4j.dbms.archive.Utils.checkWritableDirectory; @@ -45,20 +46,31 @@ public class Dumper { - public void dump( Path root, Path archive, Predicate exclude ) throws IOException + public void dump( Path dbPath, Path transactionalLogsPath, Path archive, Predicate exclude ) + throws IOException { checkWritableDirectory( archive.getParent() ); try ( ArchiveOutputStream stream = openArchiveOut( archive ) ) { - Files.walkFileTree( root, - onlyMatching( not( exclude ), - throwExceptions( - onDirectory( dir -> dumpDirectory( root, stream, dir ), - onFile( file -> dumpFile( root, stream, file ), - justContinue() ) ) ) ) ); + visitPath( dbPath, exclude, stream ); + if ( !Util.isSameOrChildPath( dbPath, transactionalLogsPath ) ) + { + visitPath( transactionalLogsPath, exclude, stream ); + } } } + private void visitPath( Path transactionalLogsPath, Predicate exclude, ArchiveOutputStream stream ) + throws IOException + { + Files.walkFileTree( transactionalLogsPath, + onlyMatching( not( exclude ), + throwExceptions( + onDirectory( dir -> dumpDirectory( transactionalLogsPath, stream, dir ), + onFile( file -> dumpFile( transactionalLogsPath, stream, file ), + justContinue() ) ) ) ) ); + } + private static ArchiveOutputStream openArchiveOut( Path archive ) throws IOException { // StandardOpenOption.CREATE_NEW is important here because it atomically asserts that the file doesn't diff --git a/community/dbms/src/main/java/org/neo4j/dbms/archive/Loader.java b/community/dbms/src/main/java/org/neo4j/dbms/archive/Loader.java index a9113eb09dc7b..e6fa932c1af8c 100644 --- a/community/dbms/src/main/java/org/neo4j/dbms/archive/Loader.java +++ b/community/dbms/src/main/java/org/neo4j/dbms/archive/Loader.java @@ -19,41 +19,71 @@ */ package org.neo4j.dbms.archive; +import org.apache.commons.compress.archivers.ArchiveEntry; +import org.apache.commons.compress.archivers.ArchiveInputStream; +import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; +import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream; + import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.nio.file.FileAlreadyExistsException; +import java.nio.file.FileSystemException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; -import org.apache.commons.compress.archivers.ArchiveEntry; -import org.apache.commons.compress.archivers.ArchiveInputStream; -import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; -import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream; +import org.neo4j.kernel.impl.transaction.log.files.TransactionLogFiles; import static java.nio.file.Files.exists; - import static org.neo4j.dbms.archive.Utils.checkWritableDirectory; public class Loader { - public void load( Path archive, Path destination ) throws IOException, IncorrectFormat + public void load( Path archive, Path databaseDestination, Path transactionLogsDirectory ) throws IOException, IncorrectFormat { - if ( exists( destination ) ) - { - throw new FileAlreadyExistsException( destination.toString() ); - } - checkWritableDirectory( destination.getParent() ); + validatePath( databaseDestination ); + validatePath( transactionLogsDirectory ); + + createDestination( databaseDestination ); + createDestination( transactionLogsDirectory ); + try ( ArchiveInputStream stream = openArchiveIn( archive ) ) { ArchiveEntry entry; while ( (entry = nextEntry( stream, archive )) != null ) { + Path destination = determineEntryDestination( entry, databaseDestination, transactionLogsDirectory ); loadEntry( destination, stream, entry ); } } } + private void createDestination( Path destination ) throws IOException + { + if ( !destination.toFile().exists() ) + { + Files.createDirectories( destination ); + } + } + + private void validatePath( Path path ) throws FileSystemException + { + if ( exists( path ) ) + { + throw new FileAlreadyExistsException( path.toString() ); + } + checkWritableDirectory( path.getParent() ); + } + + private static Path determineEntryDestination( ArchiveEntry entry, Path databaseDestination, + Path transactionLogsDirectory ) + { + String entryName = Paths.get( entry.getName() ).getFileName().toString(); + return TransactionLogFiles.DEFAULT_FILENAME_FILTER.accept( null, entryName ) ? transactionLogsDirectory + : databaseDestination; + } + private ArchiveEntry nextEntry( ArchiveInputStream stream, Path archive ) throws IncorrectFormat { try diff --git a/community/dbms/src/test/java/org/neo4j/commandline/dbms/DumpCommandTest.java b/community/dbms/src/test/java/org/neo4j/commandline/dbms/DumpCommandTest.java index 648597976090f..c58b7418025cb 100644 --- a/community/dbms/src/test/java/org/neo4j/commandline/dbms/DumpCommandTest.java +++ b/community/dbms/src/test/java/org/neo4j/commandline/dbms/DumpCommandTest.java @@ -43,6 +43,7 @@ import org.neo4j.commandline.admin.IncorrectUsage; import org.neo4j.commandline.admin.Usage; import org.neo4j.dbms.archive.Dumper; +import org.neo4j.graphdb.config.Setting; import org.neo4j.io.fs.DefaultFileSystemAbstraction; import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.kernel.configuration.Config; @@ -68,6 +69,7 @@ import static org.mockito.Mockito.verify; import static org.neo4j.dbms.archive.TestUtils.withPermissions; import static org.neo4j.graphdb.factory.GraphDatabaseSettings.data_directory; +import static org.neo4j.graphdb.factory.GraphDatabaseSettings.logical_logs_location; public class DumpCommandTest { @@ -97,7 +99,8 @@ public void setUp() throws Exception public void shouldDumpTheDatabaseToTheArchive() throws Exception { execute( "foo.db" ); - verify( dumper ).dump( eq( homeDir.resolve( "data/databases/foo.db" ) ), eq( archive ), any() ); + verify( dumper ).dump( eq( homeDir.resolve( "data/databases/foo.db" ) ), + eq( homeDir.resolve( "data/databases/foo.db" ) ), eq( archive ), any() ); } @Test @@ -107,10 +110,25 @@ public void shouldCalculateTheDatabaseDirectoryFromConfig() throws Exception Path databaseDir = dataDir.resolve( "databases/foo.db" ); putStoreInDirectory( databaseDir ); Files.write( configDir.resolve( Config.DEFAULT_CONFIG_FILE_NAME ), - asList( format( "%s=%s", data_directory.name(), dataDir.toString().replace( '\\', '/' ) ) ) ); + asList( formatProperty( data_directory, dataDir ) ) ); execute( "foo.db" ); - verify( dumper ).dump( eq( databaseDir ), any(), any() ); + verify( dumper ).dump( eq( databaseDir ), eq( databaseDir ), any(), any() ); + } + + @Test + public void shouldCalculateTheTxLogDirectoryFromConfig() throws Exception + { + Path dataDir = testDirectory.directory( "some-other-path" ).toPath(); + Path txLogsDir = testDirectory.directory( "txLogsPath" ).toPath(); + Path databaseDir = dataDir.resolve( "databases/foo.db" ); + putStoreInDirectory( databaseDir ); + Files.write( configDir.resolve( Config.DEFAULT_CONFIG_FILE_NAME ), + asList( formatProperty( data_directory, dataDir ), + formatProperty( logical_logs_location, txLogsDir ) ) ); + + execute( "foo.db" ); + verify( dumper ).dump( eq( databaseDir ), eq( txLogsDir ), any(), any() ); } @Test @@ -132,7 +150,7 @@ public void shouldHandleDatabaseSymlink() throws Exception asList( format( "%s=%s", data_directory.name(), dataDir.toString().replace( '\\', '/' ) ) ) ); execute( "foo.db" ); - verify( dumper ).dump( eq( realDatabaseDir ), any(), any() ); + verify( dumper ).dump( eq( realDatabaseDir ), eq( realDatabaseDir ), any(), any() ); } @Test @@ -141,7 +159,7 @@ public void shouldCalculateTheArchiveNameIfPassedAnExistingDirectory() { File to = testDirectory.directory( "some-dir" ); new DumpCommand( homeDir, configDir, dumper ).execute( new String[]{"--database=" + "foo.db", "--to=" + to} ); - verify( dumper ).dump( any( Path.class ), eq( to.toPath().resolve( "foo.db.dump" ) ), any() ); + verify( dumper ).dump( any( Path.class ), any( Path.class ), eq( to.toPath().resolve( "foo.db.dump" ) ), any() ); } @Test @@ -149,7 +167,8 @@ public void shouldConvertToCanonicalPath() throws Exception { new DumpCommand( homeDir, configDir, dumper ) .execute( new String[]{"--database=" + "foo.db", "--to=foo.dump"} ); - verify( dumper ).dump( any( Path.class ), eq( Paths.get( new File( "foo.dump" ).getCanonicalPath() ) ), any() ); + verify( dumper ).dump( any( Path.class ), any( Path.class ), + eq( Paths.get( new File( "foo.dump" ).getCanonicalPath() ) ), any() ); } @Test @@ -158,7 +177,7 @@ public void shouldNotCalculateTheArchiveNameIfPassedAnExistingFile() { Files.createFile( archive ); execute( "foo.db" ); - verify( dumper ).dump( any(), eq( archive ), any() ); + verify( dumper ).dump( any(), any(), eq( archive ), any() ); } @Test @@ -190,7 +209,7 @@ public void shouldReleaseTheStoreLockAfterDumping() throws Exception @Test public void shouldReleaseTheStoreLockEvenIfThereIsAnError() throws Exception { - doThrow( IOException.class ).when( dumper ).dump( any(), any(), any() ); + doThrow( IOException.class ).when( dumper ).dump( any(), any(), any(), any() ); try { @@ -213,7 +232,7 @@ public void shouldNotAccidentallyCreateTheDatabaseDirectoryAsASideEffectOfStoreL { assertThat( Files.exists( databaseDirectory ), equalTo( false ) ); return null; - } ).when( dumper ).dump( any(), any(), any() ); + } ).when( dumper ).dump( any(), any(), any(), any() ); execute( "foo.db" ); } @@ -249,11 +268,11 @@ public void shouldExcludeTheStoreLockFromTheArchiveToAvoidProblemsWithReadingLoc doAnswer( invocation -> { //noinspection unchecked - Predicate exclude = invocation.getArgument( 2 ); + Predicate exclude = invocation.getArgument( 3 ); assertThat( exclude.test( Paths.get( StoreLocker.STORE_LOCK_FILENAME ) ), is( true ) ); assertThat( exclude.test( Paths.get( "some-other-file" ) ), is( false ) ); return null; - } ).when( dumper ).dump( any(), any(), any() ); + } ).when( dumper ).dump(any(), any(), any(), any() ); execute( "foo.db" ); } @@ -265,10 +284,10 @@ public void shouldDefaultToGraphDB() throws Exception Path databaseDir = dataDir.resolve( "databases/graph.db" ); putStoreInDirectory( databaseDir ); Files.write( configDir.resolve( Config.DEFAULT_CONFIG_FILE_NAME ), - asList( format( "%s=%s", data_directory.name(), dataDir.toString().replace( '\\', '/' ) ) ) ); + asList( formatProperty( data_directory, dataDir ) ) ); new DumpCommand( homeDir, configDir, dumper ).execute( new String[]{"--to=" + archive} ); - verify( dumper ).dump( eq( databaseDir ), any(), any() ); + verify( dumper ).dump( eq( databaseDir ), eq( databaseDir ), any(), any() ); } @Test @@ -288,7 +307,7 @@ public void shouldObjectIfTheArchiveArgumentIsMissing() throws Exception @Test public void shouldGiveAClearErrorIfTheArchiveAlreadyExists() throws Exception { - doThrow( new FileAlreadyExistsException( "the-archive-path" ) ).when( dumper ).dump( any(), any(), any() ); + doThrow( new FileAlreadyExistsException( "the-archive-path" ) ).when( dumper ).dump( any(), any(), any(), any() ); try { execute( "foo.db" ); @@ -317,7 +336,7 @@ public void shouldGiveAClearMessageIfTheDatabaseDoesntExist() throws Exception @Test public void shouldGiveAClearMessageIfTheArchivesParentDoesntExist() throws Exception { - doThrow( new NoSuchFileException( archive.getParent().toString() ) ).when( dumper ).dump( any(), any(), any() ); + doThrow( new NoSuchFileException( archive.getParent().toString() ) ).when( dumper ).dump(any(), any(), any(), any() ); try { execute( "foo.db" ); @@ -331,10 +350,10 @@ public void shouldGiveAClearMessageIfTheArchivesParentDoesntExist() throws Excep } @Test - public void shouldWrapIOExceptionsCarefulllyBecauseCriticalInformationIsOftenEncodedInTheirNameButMissingFromTheirMessage() + public void shouldWrapIOExceptionsCarefullyBecauseCriticalInformationIsOftenEncodedInTheirNameButMissingFromTheirMessage() throws Exception { - doThrow( new IOException( "the-message" ) ).when( dumper ).dump( any(), any(), any() ); + doThrow( new IOException( "the-message" ) ).when( dumper ).dump(any(), any(), any(), any() ); try { execute( "foo.db" ); @@ -383,7 +402,7 @@ private void execute( final String database ) throws IncorrectUsage, CommandFail .execute( new String[]{"--database=" + database, "--to=" + archive} ); } - private void assertCanLockStore( Path databaseDirectory ) throws IOException + private static void assertCanLockStore( Path databaseDirectory ) throws IOException { try ( FileSystemAbstraction fileSystem = new DefaultFileSystemAbstraction(); StoreLocker storeLocker = new StoreLocker( fileSystem, databaseDirectory.toFile() ) ) @@ -392,10 +411,15 @@ private void assertCanLockStore( Path databaseDirectory ) throws IOException } } - private void putStoreInDirectory( Path storeDir ) throws IOException + private static void putStoreInDirectory( Path storeDir ) throws IOException { Files.createDirectories( storeDir ); Path storeFile = storeDir.resolve( StoreFileType.STORE.augment( MetaDataStore.DEFAULT_NAME ) ); Files.createFile( storeFile ); } + + private static String formatProperty( Setting setting, Path path ) + { + return format( "%s=%s", setting.name(), path.toString().replace( '\\', '/' ) ); + } } diff --git a/community/dbms/src/test/java/org/neo4j/commandline/dbms/LoadCommandTest.java b/community/dbms/src/test/java/org/neo4j/commandline/dbms/LoadCommandTest.java index e5dae64ea2c54..53dd0417d6674 100644 --- a/community/dbms/src/test/java/org/neo4j/commandline/dbms/LoadCommandTest.java +++ b/community/dbms/src/test/java/org/neo4j/commandline/dbms/LoadCommandTest.java @@ -42,6 +42,7 @@ import org.neo4j.commandline.admin.Usage; import org.neo4j.dbms.archive.IncorrectFormat; import org.neo4j.dbms.archive.Loader; +import org.neo4j.graphdb.config.Setting; import org.neo4j.helpers.ArrayUtil; import org.neo4j.io.fs.DefaultFileSystemAbstraction; import org.neo4j.io.fs.FileSystemAbstraction; @@ -64,6 +65,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.neo4j.graphdb.factory.GraphDatabaseSettings.data_directory; +import static org.neo4j.graphdb.factory.GraphDatabaseSettings.logical_logs_location; public class LoadCommandTest { @@ -87,7 +89,7 @@ public void setUp() throws Exception public void shouldLoadTheDatabaseFromTheArchive() throws CommandFailed, IncorrectUsage, IOException, IncorrectFormat { execute( "foo.db" ); - verify( loader ).load( archive, homeDir.resolve( "data/databases/foo.db" ) ); + verify( loader ).load( archive, homeDir.resolve( "data/databases/foo.db" ), homeDir.resolve( "data/databases/foo.db" ) ); } @Test @@ -98,10 +100,24 @@ public void shouldCalculateTheDatabaseDirectoryFromConfig() Path databaseDir = dataDir.resolve( "databases/foo.db" ); Files.createDirectories( databaseDir ); Files.write( configDir.resolve( Config.DEFAULT_CONFIG_FILE_NAME ), - asList( format( "%s=%s", data_directory.name(), dataDir.toString().replace( '\\', '/' ) ) ) ); + asList( formatProperty( data_directory, dataDir ) ) ); execute( "foo.db" ); - verify( loader ).load( any(), eq( databaseDir ) ); + verify( loader ).load( any(), eq( databaseDir ), eq( databaseDir ) ); + } + + @Test + public void shouldCalculateTheTxLogDirectoryFromConfig() throws Exception + { + Path dataDir = testDirectory.directory( "some-other-path" ).toPath(); + Path txLogsDir = testDirectory.directory( "txLogsPath" ).toPath(); + Path databaseDir = dataDir.resolve( "databases/foo.db" ); + Files.write( configDir.resolve( Config.DEFAULT_CONFIG_FILE_NAME ), + asList( formatProperty( data_directory, dataDir ), + formatProperty( logical_logs_location, txLogsDir ) ) ); + + execute( "foo.db" ); + verify( loader ).load( any(), eq( databaseDir ), eq( txLogsDir ) ); } @Test @@ -121,10 +137,10 @@ public void shouldHandleSymlinkToDatabaseDir() throws IOException, CommandFailed Files.createSymbolicLink( databaseDir, realDatabaseDir ); Files.write( configDir.resolve( Config.DEFAULT_CONFIG_FILE_NAME ), - asList( format( "%s=%s", data_directory.name(), dataDir.toString().replace( '\\', '/' ) ) ) ); + asList( formatProperty( data_directory, dataDir ) ) ); execute( "foo.db" ); - verify( loader ).load( any(), eq( realDatabaseDir ) ); + verify( loader ).load( any(), eq( realDatabaseDir ), eq( realDatabaseDir ) ); } @Test @@ -134,12 +150,12 @@ public void shouldMakeFromCanonical() throws IOException, CommandFailed, Incorre Path databaseDir = dataDir.resolve( "databases/foo.db" ); Files.createDirectories( databaseDir ); Files.write( configDir.resolve( Config.DEFAULT_CONFIG_FILE_NAME ), - asList( format( "%s=%s", data_directory.name(), dataDir.toString().replace( '\\', '/' ) ) ) ); + asList( formatProperty( data_directory, dataDir ) ) ); new LoadCommand( homeDir, configDir, loader ) .execute( ArrayUtil.concat( new String[]{"--database=foo.db", "--from=foo.dump"} ) ); - verify( loader ).load( eq( Paths.get( new File( "foo.dump" ).getCanonicalPath() ) ), any() ); + verify( loader ).load( eq( Paths.get( new File( "foo.dump" ).getCanonicalPath() ) ), any(), any() ); } @Test @@ -153,7 +169,7 @@ public void shouldDeleteTheOldDatabaseIfForceArgumentIsProvided() { assertThat( Files.exists( databaseDirectory ), equalTo( false ) ); return null; - } ).when( loader ).load( any(), any() ); + } ).when( loader ).load( any(), any(), any() ); execute( "foo.db", "--force" ); } @@ -169,7 +185,7 @@ public void shouldNotDeleteTheOldDatabaseIfForceArgumentIsNotProvided() { assertThat( Files.exists( databaseDirectory ), equalTo( true ) ); return null; - } ).when( loader ).load( any(), any() ); + } ).when( loader ).load( any(), any(), any() ); execute( "foo.db" ); } @@ -200,7 +216,7 @@ public void shouldDefaultToGraphDb() throws Exception Files.createDirectories( databaseDir ); new LoadCommand( homeDir, configDir, loader ).execute( new String[]{"--from=something"} ); - verify( loader ).load( any(), eq( databaseDir ) ); + verify( loader ).load( any(), eq( databaseDir ), eq( databaseDir ) ); } @Test @@ -220,7 +236,7 @@ public void shouldObjectIfTheArchiveArgumentIsMissing() throws Exception @Test public void shouldGiveAClearMessageIfTheArchiveDoesntExist() throws IOException, IncorrectFormat, IncorrectUsage { - doThrow( new NoSuchFileException( archive.toString() ) ).when( loader ).load( any(), any() ); + doThrow( new NoSuchFileException( archive.toString() ) ).when( loader ).load( any(), any(), any() ); try { execute( null ); @@ -235,7 +251,7 @@ public void shouldGiveAClearMessageIfTheArchiveDoesntExist() throws IOException, @Test public void shouldGiveAClearMessageIfTheDatabaseAlreadyExists() throws IOException, IncorrectFormat, IncorrectUsage { - doThrow( FileAlreadyExistsException.class ).when( loader ).load( any(), any() ); + doThrow( FileAlreadyExistsException.class ).when( loader ).load( any(), any(), any() ); try { execute( "foo.db" ); @@ -251,7 +267,7 @@ public void shouldGiveAClearMessageIfTheDatabaseAlreadyExists() throws IOExcepti public void shouldGiveAClearMessageIfTheDatabasesDirectoryIsNotWritable() throws IOException, IncorrectFormat, IncorrectUsage { - doThrow( AccessDeniedException.class ).when( loader ).load( any(), any() ); + doThrow( AccessDeniedException.class ).when( loader ).load( any(), any(), any() ); try { execute( null ); @@ -265,10 +281,10 @@ public void shouldGiveAClearMessageIfTheDatabasesDirectoryIsNotWritable() } @Test - public void shouldWrapIOExceptionsCarefulllyBecauseCriticalInformationIsOftenEncodedInTheirNameButMissingFromTheirMessage() + public void shouldWrapIOExceptionsCarefullyBecauseCriticalInformationIsOftenEncodedInTheirNameButMissingFromTheirMessage() throws IOException, IncorrectUsage, IncorrectFormat { - doThrow( new FileSystemException( "the-message" ) ).when( loader ).load( any(), any() ); + doThrow( new FileSystemException( "the-message" ) ).when( loader ).load( any(), any(), any() ); try { execute( null ); @@ -283,7 +299,7 @@ public void shouldWrapIOExceptionsCarefulllyBecauseCriticalInformationIsOftenEnc @Test public void shouldThrowIfTheArchiveFormatIsInvalid() throws IOException, IncorrectUsage, IncorrectFormat { - doThrow( IncorrectFormat.class ).when( loader ).load( any(), any() ); + doThrow( IncorrectFormat.class ).when( loader ).load( any(), any(), any() ); try { execute( null ); @@ -335,4 +351,9 @@ private void execute( String database, String... otherArgs ) throws IncorrectUsa new LoadCommand( homeDir, configDir, loader ) .execute( ArrayUtil.concat( new String[]{"--database=" + database, "--from=" + archive}, otherArgs ) ); } + + private static String formatProperty( Setting setting, Path path ) + { + return format( "%s=%s", setting.name(), path.toString().replace( '\\', '/' ) ); + } } diff --git a/community/dbms/src/test/java/org/neo4j/commandline/dbms/UtilTest.java b/community/dbms/src/test/java/org/neo4j/commandline/dbms/UtilTest.java index d38d9e9a5379b..06b6c5e730187 100644 --- a/community/dbms/src/test/java/org/neo4j/commandline/dbms/UtilTest.java +++ b/community/dbms/src/test/java/org/neo4j/commandline/dbms/UtilTest.java @@ -19,15 +19,24 @@ */ package org.neo4j.commandline.dbms; +import org.junit.Rule; import org.junit.Test; import org.neo4j.commandline.Util; +import org.neo4j.test.rule.TestDirectory; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.neo4j.commandline.Util.isSameOrChildFile; +import static org.neo4j.commandline.Util.isSameOrChildPath; import static org.neo4j.commandline.Util.neo4jVersion; public class UtilTest { + @Rule + public final TestDirectory directory = TestDirectory.testDirectory(); + @Test public void canonicalPath() throws Exception { @@ -39,4 +48,29 @@ public void returnsAVersion() throws Exception { assertNotNull( "A version should be returned", neo4jVersion() ); } + + @Test + public void correctlyIdentifySameOrChildFile() + { + assertTrue( isSameOrChildFile( directory.directory(), directory.directory( "a" ) ) ); + assertTrue( isSameOrChildFile( directory.directory(), directory.directory() ) ); + assertTrue( isSameOrChildFile( directory.directory( "/a/./b" ), directory.directory( "a/b" ) ) ); + assertTrue( isSameOrChildFile( directory.directory( "a/b" ), directory.directory( "/a/./b" ) ) ); + + assertFalse( isSameOrChildFile( directory.directory( "a" ), directory.directory( "b" ) ) ); + } + + @Test + public void correctlyIdentifySameOrChildPath() + { + assertTrue( isSameOrChildPath( directory.directory().toPath(), directory.directory( "a" ).toPath() ) ); + assertTrue( isSameOrChildPath( directory.directory().toPath(), directory.directory().toPath() ) ); + assertTrue( isSameOrChildPath( directory.directory( "/a/./b" ).toPath(), + directory.directory( "a/b" ).toPath() ) ); + assertTrue( isSameOrChildPath( directory.directory( "a/b" ).toPath(), + directory.directory( "/a/./b" ).toPath() ) ); + + assertFalse( isSameOrChildPath( directory.directory( "a" ).toPath(), + directory.directory( "b" ).toPath() ) ); + } } diff --git a/community/dbms/src/test/java/org/neo4j/dbms/archive/ArchiveTest.java b/community/dbms/src/test/java/org/neo4j/dbms/archive/ArchiveTest.java index bc4d113c08075..2221673f6dcaa 100644 --- a/community/dbms/src/test/java/org/neo4j/dbms/archive/ArchiveTest.java +++ b/community/dbms/src/test/java/org/neo4j/dbms/archive/ArchiveTest.java @@ -30,6 +30,7 @@ import java.util.Map; import org.neo4j.function.Predicates; +import org.neo4j.kernel.impl.transaction.log.files.TransactionLogFiles; import org.neo4j.test.rule.TestDirectory; import static java.nio.file.Files.isDirectory; @@ -121,9 +122,9 @@ public void shouldExcludeFilesMatchedByTheExclusionPredicate() throws IOExceptio Files.write( directory.resolve( "another-file" ), new byte[0] ); Path archive = testDirectory.file( "the-archive.dump" ).toPath(); - new Dumper().dump( directory, archive, path -> path.getFileName().toString().equals( "another-file" ) ); + new Dumper().dump( directory, directory, archive, path -> path.getFileName().toString().equals( "another-file" ) ); Path newDirectory = testDirectory.file( "the-new-directory" ).toPath(); - new Loader().load( archive, newDirectory ); + new Loader().load( archive, newDirectory, newDirectory ); Path expectedOutput = testDirectory.directory( "expected-output" ).toPath(); Files.createDirectories( expectedOutput ); @@ -141,9 +142,9 @@ public void shouldExcludeWholeDirectoriesMatchedByTheExclusionPredicate() throws Files.write( subdir.resolve( "a-file" ), new byte[0] ); Path archive = testDirectory.file( "the-archive.dump" ).toPath(); - new Dumper().dump( directory, archive, path -> path.getFileName().toString().equals( "subdir" ) ); + new Dumper().dump( directory, directory, archive, path -> path.getFileName().toString().equals( "subdir" ) ); Path newDirectory = testDirectory.file( "the-new-directory" ).toPath(); - new Loader().load( archive, newDirectory ); + new Loader().load( archive, newDirectory, newDirectory ); Path expectedOutput = testDirectory.directory( "expected-output" ).toPath(); Files.createDirectories( expectedOutput ); @@ -151,12 +152,38 @@ public void shouldExcludeWholeDirectoriesMatchedByTheExclusionPredicate() throws assertEquals( describeRecursively( expectedOutput ), describeRecursively( newDirectory ) ); } + @Test + public void dumpAndLoadTransactionLogsFromCustomLocations() throws IOException, IncorrectFormat + { + Path directory = testDirectory.directory( "dbDirectory" ).toPath(); + Path txLogsDirectory = testDirectory.directory( "txLogsDirectory" ).toPath(); + Files.write( directory.resolve( "dbfile" ), new byte[0] ); + Files.write( txLogsDirectory.resolve( TransactionLogFiles.DEFAULT_NAME + ".0" ), new byte[0] ); + + Path archive = testDirectory.file( "the-archive.dump" ).toPath(); + new Dumper().dump( directory, txLogsDirectory, archive, Predicates.alwaysFalse() ); + Path newDirectory = testDirectory.file( "the-new-directory" ).toPath(); + Path newTxLogsDirectory = testDirectory.file( "newTxLogsDirectory" ).toPath(); + new Loader().load( archive, newDirectory, newTxLogsDirectory ); + + Path expectedOutput = testDirectory.directory( "expected-output" ).toPath(); + Files.createDirectories( expectedOutput ); + Files.write( expectedOutput.resolve( "dbfile" ), new byte[0] ); + + Path expectedTxLogs = testDirectory.directory( "expectedTxLogs" ).toPath(); + Files.createDirectories( expectedTxLogs ); + Files.write( expectedTxLogs.resolve( TransactionLogFiles.DEFAULT_NAME + ".0" ), new byte[0] ); + + assertEquals( describeRecursively( expectedOutput ), describeRecursively( newDirectory ) ); + assertEquals( describeRecursively( expectedTxLogs ), describeRecursively( newTxLogsDirectory ) ); + } + private void assertRoundTrips( Path oldDirectory ) throws IOException, IncorrectFormat { Path archive = testDirectory.file( "the-archive.dump" ).toPath(); - new Dumper().dump( oldDirectory, archive, Predicates.alwaysFalse() ); + new Dumper().dump( oldDirectory, oldDirectory, archive, Predicates.alwaysFalse() ); Path newDirectory = testDirectory.file( "the-new-directory" ).toPath(); - new Loader().load( archive, newDirectory ); + new Loader().load( archive, newDirectory, newDirectory ); assertEquals( describeRecursively( oldDirectory ), describeRecursively( newDirectory ) ); } diff --git a/community/dbms/src/test/java/org/neo4j/dbms/archive/DumperTest.java b/community/dbms/src/test/java/org/neo4j/dbms/archive/DumperTest.java index 734e8a90a2406..834654638f945 100644 --- a/community/dbms/src/test/java/org/neo4j/dbms/archive/DumperTest.java +++ b/community/dbms/src/test/java/org/neo4j/dbms/archive/DumperTest.java @@ -19,6 +19,10 @@ */ package org.neo4j.dbms.archive; +import org.apache.commons.lang3.SystemUtils; +import org.junit.Rule; +import org.junit.Test; + import java.io.Closeable; import java.io.IOException; import java.nio.file.AccessDeniedException; @@ -28,15 +32,10 @@ import java.nio.file.NoSuchFileException; import java.nio.file.Path; -import org.apache.commons.lang3.SystemUtils; -import org.junit.Rule; -import org.junit.Test; - import org.neo4j.function.Predicates; import org.neo4j.test.rule.TestDirectory; import static java.util.Collections.emptySet; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; import static org.junit.Assume.assumeFalse; @@ -54,7 +53,7 @@ public void shouldGiveAClearErrorIfTheArchiveAlreadyExists() throws IOException Files.write( archive, new byte[0] ); try { - new Dumper().dump( directory, archive, Predicates.alwaysFalse() ); + new Dumper().dump( directory, directory, archive, Predicates.alwaysFalse() ); fail( "Expected an exception" ); } catch ( FileAlreadyExistsException e ) @@ -70,7 +69,7 @@ public void shouldGiveAClearErrorMessageIfTheDirectoryDoesntExist() throws IOExc Path archive = testDirectory.file( "the-archive.dump" ).toPath(); try { - new Dumper().dump( directory, archive, Predicates.alwaysFalse() ); + new Dumper().dump( directory, directory, archive, Predicates.alwaysFalse() ); fail( "Expected an exception" ); } catch ( NoSuchFileException e ) @@ -86,7 +85,7 @@ public void shouldGiveAClearErrorMessageIfTheArchivesParentDirectoryDoesntExist( Path archive = testDirectory.file( "subdir/the-archive.dump" ).toPath(); try { - new Dumper().dump( directory, archive, Predicates.alwaysFalse() ); + new Dumper().dump( directory, directory, archive, Predicates.alwaysFalse() ); fail( "Expected an exception" ); } catch ( NoSuchFileException e ) @@ -103,7 +102,7 @@ public void shouldGiveAClearErrorMessageIfTheArchivesParentDirectoryIsAFile() th Files.write( archive.getParent(), new byte[0] ); try { - new Dumper().dump( directory, archive, Predicates.alwaysFalse() ); + new Dumper().dump( directory, directory, archive, Predicates.alwaysFalse() ); fail( "Expected an exception" ); } catch ( FileSystemException e ) @@ -122,7 +121,7 @@ public void shouldGiveAClearErrorMessageIfTheArchivesParentDirectoryIsNotWritabl Files.createDirectories( archive.getParent() ); try ( Closeable ignored = TestUtils.withPermissions( archive.getParent(), emptySet() ) ) { - new Dumper().dump( directory, archive, Predicates.alwaysFalse() ); + new Dumper().dump( directory, directory, archive, Predicates.alwaysFalse() ); fail( "Expected an exception" ); } catch ( AccessDeniedException e ) diff --git a/community/dbms/src/test/java/org/neo4j/dbms/archive/LoaderTest.java b/community/dbms/src/test/java/org/neo4j/dbms/archive/LoaderTest.java index 7afd0298dcc62..0521d28b40313 100644 --- a/community/dbms/src/test/java/org/neo4j/dbms/archive/LoaderTest.java +++ b/community/dbms/src/test/java/org/neo4j/dbms/archive/LoaderTest.java @@ -19,6 +19,11 @@ */ package org.neo4j.dbms.archive; +import org.apache.commons.compress.compressors.gzip.GzipCompressorOutputStream; +import org.apache.commons.lang3.SystemUtils; +import org.junit.Rule; +import org.junit.Test; + import java.io.Closeable; import java.io.IOException; import java.nio.file.AccessDeniedException; @@ -29,20 +34,13 @@ import java.nio.file.Path; import java.util.Random; -import org.apache.commons.compress.compressors.gzip.GzipCompressorOutputStream; -import org.apache.commons.lang3.SystemUtils; -import org.junit.Rule; -import org.junit.Test; - import org.neo4j.test.rule.TestDirectory; import static java.util.Arrays.asList; import static java.util.Collections.emptySet; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; import static org.junit.Assume.assumeFalse; - import static org.neo4j.dbms.archive.TestUtils.withPermissions; public class LoaderTest @@ -57,7 +55,7 @@ public void shouldGiveAClearErrorMessageIfTheArchiveDoesntExist() throws IOExcep Path destination = testDirectory.file( "the-destination" ).toPath(); try { - new Loader().load( archive, destination ); + new Loader().load( archive, destination, destination ); fail( "Expected an exception" ); } catch ( NoSuchFileException e ) @@ -74,7 +72,7 @@ public void shouldGiveAClearErrorMessageIfTheArchiveIsNotInGzipFormat() throws I Path destination = testDirectory.file( "the-destination" ).toPath(); try { - new Loader().load( archive, destination ); + new Loader().load( archive, destination, destination ); fail( "Expected an exception" ); } catch ( IncorrectFormat e ) @@ -98,7 +96,7 @@ public void shouldGiveAClearErrorMessageIfTheArchiveIsNotInTarFormat() throws IO Path destination = testDirectory.file( "the-destination" ).toPath(); try { - new Loader().load( archive, destination ); + new Loader().load( archive, destination, destination ); fail( "Expected an exception" ); } catch ( IncorrectFormat e ) @@ -114,7 +112,7 @@ public void shouldGiveAClearErrorIfTheDestinationAlreadyExists() throws IOExcept Path destination = testDirectory.directory( "the-destination" ).toPath(); try { - new Loader().load( archive, destination ); + new Loader().load( archive, destination, destination ); fail( "Expected an exception" ); } catch ( FileAlreadyExistsException e ) @@ -123,6 +121,23 @@ public void shouldGiveAClearErrorIfTheDestinationAlreadyExists() throws IOExcept } } + @Test + public void shouldGiveAClearErrorIfTheDestinationTxLogAlreadyExists() throws IOException, IncorrectFormat + { + Path archive = testDirectory.file( "the-archive.dump" ).toPath(); + Path destination = testDirectory.file( "the-destination" ).toPath(); + Path txLogsDestination = testDirectory.directory( "txLogsDestination" ).toPath(); + try + { + new Loader().load( archive, destination, txLogsDestination ); + fail( "Expected an exception" ); + } + catch ( FileAlreadyExistsException e ) + { + assertEquals( txLogsDestination.toString(), e.getMessage() ); + } + } + @Test public void shouldGiveAClearErrorMessageIfTheDestinationsParentDirectoryDoesntExist() throws IOException, IncorrectFormat @@ -131,7 +146,7 @@ public void shouldGiveAClearErrorMessageIfTheDestinationsParentDirectoryDoesntEx Path destination = testDirectory.directory( "subdir/the-destination" ).toPath(); try { - new Loader().load( archive, destination ); + new Loader().load( archive, destination, destination ); fail( "Expected an exception" ); } catch ( NoSuchFileException e ) @@ -140,6 +155,24 @@ public void shouldGiveAClearErrorMessageIfTheDestinationsParentDirectoryDoesntEx } } + @Test + public void shouldGiveAClearErrorMessageIfTheTxLogsParentDirectoryDoesntExist() + throws IOException, IncorrectFormat + { + Path archive = testDirectory.file( "the-archive.dump" ).toPath(); + Path destination = testDirectory.file( "destination" ).toPath(); + Path txLogsDestination = testDirectory.directory( "subdir/txLogs" ).toPath(); + try + { + new Loader().load( archive, destination, txLogsDestination ); + fail( "Expected an exception" ); + } + catch ( NoSuchFileException e ) + { + assertEquals( txLogsDestination.getParent().toString(), e.getMessage() ); + } + } + @Test public void shouldGiveAClearErrorMessageIfTheDestinationsParentDirectoryIsAFile() throws IOException, IncorrectFormat @@ -149,7 +182,7 @@ public void shouldGiveAClearErrorMessageIfTheDestinationsParentDirectoryIsAFile( Files.write( destination.getParent(), new byte[0] ); try { - new Loader().load( archive, destination ); + new Loader().load( archive, destination, destination ); fail( "Expected an exception" ); } catch ( FileSystemException e ) @@ -169,7 +202,7 @@ public void shouldGiveAClearErrorMessageIfTheDestinationsParentDirectoryIsNotWri Files.createDirectories( destination.getParent() ); try ( Closeable ignored = withPermissions( destination.getParent(), emptySet() ) ) { - new Loader().load( archive, destination ); + new Loader().load( archive, destination, destination ); fail( "Expected an exception" ); } catch ( AccessDeniedException e ) @@ -177,4 +210,25 @@ public void shouldGiveAClearErrorMessageIfTheDestinationsParentDirectoryIsNotWri assertEquals( destination.getParent().toString(), e.getMessage() ); } } + + @Test + public void shouldGiveAClearErrorMessageIfTheTxLogsParentDirectoryIsNotWritable() + throws IOException, IncorrectFormat + { + assumeFalse( "We haven't found a way to reliably tests permissions on Windows", SystemUtils.IS_OS_WINDOWS ); + + Path archive = testDirectory.file( "the-archive.dump" ).toPath(); + Path destination = testDirectory.file( "destination" ).toPath(); + Path txLogsDrectory = testDirectory.directory( "subdir/txLogs" ).toPath(); + Files.createDirectories( txLogsDrectory.getParent() ); + try ( Closeable ignored = withPermissions( txLogsDrectory.getParent(), emptySet() ) ) + { + new Loader().load( archive, destination, txLogsDrectory ); + fail( "Expected an exception" ); + } + catch ( AccessDeniedException e ) + { + assertEquals( txLogsDrectory.getParent().toString(), e.getMessage() ); + } + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/log/files/TransactionLogFilesHelper.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/log/files/TransactionLogFilesHelper.java index bb37bb7c2c9c7..9627f23e87f02 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/log/files/TransactionLogFilesHelper.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/log/files/TransactionLogFilesHelper.java @@ -31,7 +31,7 @@ class TransactionLogFilesHelper private static final String VERSION_SUFFIX = "."; private static final String REGEX_VERSION_SUFFIX = "\\."; - public static final FilenameFilter DEFAULT_FILENAME_FILTER = new LogicalLogFilenameFilter( REGEX_DEFAULT_NAME ); + static final FilenameFilter DEFAULT_FILENAME_FILTER = new LogicalLogFilenameFilter( REGEX_DEFAULT_NAME ); private final File logBaseName; private final FilenameFilter logFileFilter; diff --git a/enterprise/backup/src/main/java/org/neo4j/restore/RestoreDatabaseCommand.java b/enterprise/backup/src/main/java/org/neo4j/restore/RestoreDatabaseCommand.java index db82ab87d0c92..50cb4808bb4e8 100644 --- a/enterprise/backup/src/main/java/org/neo4j/restore/RestoreDatabaseCommand.java +++ b/enterprise/backup/src/main/java/org/neo4j/restore/RestoreDatabaseCommand.java @@ -32,6 +32,7 @@ import static java.lang.String.format; import static org.neo4j.commandline.Util.checkLock; +import static org.neo4j.commandline.Util.isSameOrChildFile; import static org.neo4j.graphdb.factory.GraphDatabaseSettings.database_path; public class RestoreDatabaseCommand @@ -80,7 +81,8 @@ public void execute() throws IOException, CommandFailed checkLock( databaseDir.toPath() ); fs.deleteRecursively( databaseDir ); - if ( !databaseDir.equals( transactionLogsDirectory ) ) + + if ( !isSameOrChildFile( databaseDir, transactionLogsDirectory ) ) { fs.deleteRecursively( transactionLogsDirectory ); } diff --git a/enterprise/backup/src/test/java/org/neo4j/restore/RestoreDatabaseCommandIT.java b/enterprise/backup/src/test/java/org/neo4j/restore/RestoreDatabaseCommandIT.java index 31e33ea9e26a1..dda5120b588d5 100644 --- a/enterprise/backup/src/test/java/org/neo4j/restore/RestoreDatabaseCommandIT.java +++ b/enterprise/backup/src/test/java/org/neo4j/restore/RestoreDatabaseCommandIT.java @@ -21,6 +21,7 @@ import org.junit.Rule; import org.junit.Test; +import org.mockito.Mockito; import java.io.ByteArrayOutputStream; import java.io.File; @@ -52,7 +53,10 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.neo4j.helpers.collection.MapUtil.stringMap; public class RestoreDatabaseCommandIT @@ -235,6 +239,25 @@ public void restoreTransactionLogsInCustomDirectoryForTargetDatabaseWhenConfigur customLogLocationLogFiles.getLogFileForVersion( 0 ).length() ); } + @Test + public void doNotRemoveRelativeTransactionDirectoryAgain() throws IOException, CommandFailed + { + FileSystemAbstraction fileSystem = Mockito.spy( fileSystemRule.get() ); + File fromPath = directory.directory( "from" ); + File databaseFile = directory.directory(); + File relativeLogDirectory = directory.directory( "relativeDirectory" ); + + Config config = Config.defaults( GraphDatabaseSettings.database_path, databaseFile.getAbsolutePath() ); + config.augment( GraphDatabaseSettings.logical_logs_location, relativeLogDirectory.getAbsolutePath() ); + + createDbAt( fromPath, 10 ); + + new RestoreDatabaseCommand( fileSystem, fromPath, config, "testDatabase", true ).execute(); + + verify( fileSystem ).deleteRecursively( eq( databaseFile ) ); + verify( fileSystem, never() ).deleteRecursively( eq( relativeLogDirectory ) ); + } + @Test public void shouldPrintNiceHelp() throws Throwable {