diff --git a/community/kernel/src/test/java/org/neo4j/test/rule/EmbeddedDatabaseRule.java b/community/kernel/src/test/java/org/neo4j/test/rule/EmbeddedDatabaseRule.java index a53df8ccbb1e2..3116c9ed07e03 100644 --- a/community/kernel/src/test/java/org/neo4j/test/rule/EmbeddedDatabaseRule.java +++ b/community/kernel/src/test/java/org/neo4j/test/rule/EmbeddedDatabaseRule.java @@ -71,7 +71,7 @@ protected GraphDatabaseFactory newFactory() @Override protected GraphDatabaseBuilder newBuilder( GraphDatabaseFactory factory ) { - return factory.newEmbeddedDatabaseBuilder( getStoreDir() ); + return factory.newEmbeddedDatabaseBuilder( testDirectory.graphDbDir() ); } @Override diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/impl/BackupCopyService.java b/enterprise/backup/src/main/java/org/neo4j/backup/impl/BackupCopyService.java index a2e073a3e6cb1..33f2cffc48612 100644 --- a/enterprise/backup/src/main/java/org/neo4j/backup/impl/BackupCopyService.java +++ b/enterprise/backup/src/main/java/org/neo4j/backup/impl/BackupCopyService.java @@ -70,7 +70,6 @@ public void moveBackupLocation( Path oldLocation, Path newLocation ) throws IOEx { moves.next().move( target ); } - oldLocation.toFile().delete(); } catch ( IOException e ) { diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/impl/BackupStrategyWrapper.java b/enterprise/backup/src/main/java/org/neo4j/backup/impl/BackupStrategyWrapper.java index 271937c412228..9850692457958 100644 --- a/enterprise/backup/src/main/java/org/neo4j/backup/impl/BackupStrategyWrapper.java +++ b/enterprise/backup/src/main/java/org/neo4j/backup/impl/BackupStrategyWrapper.java @@ -74,7 +74,8 @@ Fallible doBackup( OnlineBackupContext onlineBackupContex return state; } - private Fallible performBackupWithoutLifecycle( OnlineBackupContext onlineBackupContext ) + private Fallible performBackupWithoutLifecycle( + OnlineBackupContext onlineBackupContext ) { Path backupLocation = onlineBackupContext.getResolvedLocationFromName(); Path userSpecifiedBackupLocation = onlineBackupContext.getResolvedLocationFromName(); @@ -135,7 +136,8 @@ private void clearIdFiles( Path backupLocation ) * @param onlineBackupContext command line arguments, config etc. * @return outcome of full backup */ - private Fallible fullBackupWithTemporaryFolderResolutions( OnlineBackupContext onlineBackupContext ) + private Fallible fullBackupWithTemporaryFolderResolutions( + OnlineBackupContext onlineBackupContext ) { Path userSpecifiedBackupLocation = onlineBackupContext.getResolvedLocationFromName(); Path temporaryFullBackupLocation = backupCopyService.findAnAvailableLocationForNewFullBackup( userSpecifiedBackupLocation ); @@ -144,12 +146,12 @@ private Fallible fullBackupWithTemporaryFolderResolutions( O Fallible state = backupStrategy.performFullBackup( temporaryFullBackupLocation, config, address ); // NOTE temporaryFullBackupLocation can be equal to desired - boolean backupWasMadeToATemporaryLocation = !userSpecifiedBackupLocation.equals( temporaryFullBackupLocation ); + boolean aBackupAlreadyExisted = userSpecifiedBackupLocation.equals( temporaryFullBackupLocation ); if ( BackupStageOutcome.SUCCESS.equals( state.getState() ) ) { backupRecoveryService.recoverWithDatabase( temporaryFullBackupLocation, pageCache, config ); - if ( backupWasMadeToATemporaryLocation ) + if ( !aBackupAlreadyExisted ) { try { diff --git a/enterprise/backup/src/test/java/org/neo4j/backup/impl/OnlineBackupCommandCcIT.java b/enterprise/backup/src/test/java/org/neo4j/backup/impl/OnlineBackupCommandCcIT.java index 577af9fe1aebc..339504ae7ba14 100644 --- a/enterprise/backup/src/test/java/org/neo4j/backup/impl/OnlineBackupCommandCcIT.java +++ b/enterprise/backup/src/test/java/org/neo4j/backup/impl/OnlineBackupCommandCcIT.java @@ -40,10 +40,7 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; -import java.util.HashMap; import java.util.List; -import java.util.Map; -import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicBoolean; import org.neo4j.causalclustering.ClusterHelper; @@ -52,8 +49,6 @@ import org.neo4j.causalclustering.core.consensus.roles.Role; import org.neo4j.causalclustering.discovery.Cluster; import org.neo4j.causalclustering.discovery.CoreClusterMember; -import org.neo4j.causalclustering.discovery.IpFamily; -import org.neo4j.causalclustering.discovery.SharedDiscoveryServiceFactory; import org.neo4j.causalclustering.helpers.CausalClusteringTestHelpers; import org.neo4j.graphdb.factory.GraphDatabaseSettings; import org.neo4j.kernel.configuration.Config; @@ -69,10 +64,8 @@ import org.neo4j.util.TestHelpers; import static java.lang.String.format; -import static java.util.Collections.emptyMap; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeFalse; @@ -276,45 +269,6 @@ public void reportsProgress() throws Exception assertTrue( output.contains( "Finished receiving index snapshots" ) ); } - @Test - public void backupRenamesWork() throws Exception - { - // given a prexisting backup from a different store - String backupName = "preexistingBackup_" + recordFormat; - Cluster cluster = startCluster( recordFormat ); - String firstBackupAddress = CausalClusteringTestHelpers.transactionAddress( clusterLeader( cluster ).database() ); - - assertEquals( 0, runBackupToolFromOtherJvmToGetExitCode( - "--from", firstBackupAddress, - "--cc-report-dir=" + backupDir, - "--backup-dir=" + backupDir, - "--name=" + backupName ) ); - DbRepresentation firstDatabaseRepresentation = DbRepresentation.of( clusterLeader( cluster ).database() ); - - // and a different database - Cluster cluster2 = startCluster2( recordFormat ); - DbRepresentation secondDatabaseRepresentation = DbRepresentation.of( clusterLeader( cluster2 ).database() ); - assertNotEquals( firstDatabaseRepresentation, secondDatabaseRepresentation ); - String secondBackupAddress = CausalClusteringTestHelpers.transactionAddress( clusterLeader( cluster2 ).database() ); - - // when backup is performed - assertEquals( 0, runBackupToolFromOtherJvmToGetExitCode( - "--from", secondBackupAddress, - "--cc-report-dir=" + backupDir, - "--backup-dir=" + backupDir, - "--name=" + backupName ) ); - cluster2.shutdown(); - - // then the new backup has the correct name - assertEquals( secondDatabaseRepresentation, getBackupDbRepresentation( backupName, backupDir ) ); - - // and the old backup is in a renamed location - assertEquals( firstDatabaseRepresentation, getBackupDbRepresentation( backupName + ".err.0", backupDir ) ); - - // and the data isn't equal (sanity check) - assertNotEquals( firstDatabaseRepresentation, secondDatabaseRepresentation ); - } - static PrintStream wrapWithNormalOutput( PrintStream normalOutput, PrintStream nullAbleOutput ) { if ( nullAbleOutput == null ) @@ -388,19 +342,6 @@ private Cluster startCluster( String recordFormat ) throws Exception return cluster; } - private Cluster startCluster2( String recordFormat ) throws ExecutionException, InterruptedException - { - Map sharedParams = new HashMap<>( ); - sharedParams.put( GraphDatabaseSettings.record_format.name(), recordFormat ); - Cluster cluster = - new Cluster( testDirectory.directory( "cluster-b_" + recordFormat ), 3, 0, new SharedDiscoveryServiceFactory(), sharedParams, emptyMap(), - sharedParams, emptyMap(), - recordFormat, IpFamily.IPV4, false ); - cluster.start(); - createSomeData( cluster ); - return cluster; - } - public static DbRepresentation createSomeData( Cluster cluster ) { try diff --git a/enterprise/backup/src/test/java/org/neo4j/backup/impl/OnlineBackupCommandHaIT.java b/enterprise/backup/src/test/java/org/neo4j/backup/impl/OnlineBackupCommandHaIT.java index 909f0cbf40972..65827aea6d5c2 100644 --- a/enterprise/backup/src/test/java/org/neo4j/backup/impl/OnlineBackupCommandHaIT.java +++ b/enterprise/backup/src/test/java/org/neo4j/backup/impl/OnlineBackupCommandHaIT.java @@ -48,8 +48,6 @@ import org.neo4j.graphdb.Node; import org.neo4j.graphdb.RelationshipType; import org.neo4j.graphdb.Transaction; -import org.neo4j.graphdb.factory.GraphDatabaseBuilder; -import org.neo4j.graphdb.factory.GraphDatabaseFactory; import org.neo4j.graphdb.factory.GraphDatabaseSettings; import org.neo4j.io.fs.DefaultFileSystemAbstraction; import org.neo4j.kernel.configuration.Config; @@ -68,7 +66,6 @@ import static org.hamcrest.Matchers.greaterThan; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeFalse; @@ -113,19 +110,6 @@ private static void createSomeData( GraphDatabaseService db ) } } - private static void createSpecificNodePair( GraphDatabaseService db, String name ) - { - try ( Transaction tx = db.beginTx() ) - { - Node left = db.createNode(); - left.setProperty( "name", name + "Left" ); - Node right = db.createNode(); - right.setProperty( "name", name + "Right" ); - right.createRelationshipTo( left, RelationshipType.withName( "KNOWS" ) ); - tx.success(); - } - } - @Before public void resetTasks() { @@ -159,7 +143,7 @@ public void makeSureBackupCanBePerformedWithCustomPort() throws Exception "--cc-report-dir=" + backupDir, "--backup-dir=" + backupDir, "--name=" + backupName ) ); - assertEquals( DbRepresentation.of( db ), getBackupDbRepresentation( backupName ) ); + assertEquals( getDbRepresentation(), getBackupDbRepresentation( backupName ) ); createSomeData( db ); assertEquals( 0, @@ -167,7 +151,7 @@ public void makeSureBackupCanBePerformedWithCustomPort() throws Exception "--cc-report-dir=" + backupDir, "--backup-dir=" + backupDir, "--name=" + backupName ) ); - assertEquals( DbRepresentation.of( db ), getBackupDbRepresentation( backupName ) ); + assertEquals( getDbRepresentation(), getBackupDbRepresentation( backupName ) ); } @Test @@ -305,48 +289,6 @@ public void reportsProgress() throws Exception assertFalse( output.contains( "Finished receiving index snapshots" ) ); } - @Test - public void backupRenamesWork() throws Exception - { - // given a prexisting backup from a different store - String backupName = "preexistingBackup_" + recordFormat; - int firstBackupPort = PortAuthority.allocatePort(); - startDb( firstBackupPort ); - createSpecificNodePair( db, "first" ); - - assertEquals( 0, runSameJvm( backupDir, backupName, - "--from", "127.0.0.1:" + firstBackupPort, - "--cc-report-dir=" + backupDir, - "--protocol=common", - "--backup-dir=" + backupDir, - "--name=" + backupName ) ); - DbRepresentation firstDatabaseRepresentation = DbRepresentation.of( db ); - - // and a different database - int secondBackupPort = PortAuthority.allocatePort(); - GraphDatabaseService db2 = createDb2( secondBackupPort ); - createSpecificNodePair( db2, "second" ); - DbRepresentation secondDatabaseRepresentation = DbRepresentation.of( db2 ); - - // when backup is performed - assertEquals( 0, runSameJvm(backupDir, backupName, - "--from", "127.0.0.1:" + secondBackupPort, - "--cc-report-dir=" + backupDir, - "--backup-dir=" + backupDir, - "--protocol=common", - "--name=" + backupName ) ); - - // then the new backup has the correct name - assertEquals( secondDatabaseRepresentation, getBackupDbRepresentation( backupName ) ); - - // and the old backup is in a renamed location - assertEquals( firstDatabaseRepresentation, getBackupDbRepresentation( backupName + ".err.0" ) ); - - // and the data isn't equal (sanity check) - assertNotEquals( firstDatabaseRepresentation, secondDatabaseRepresentation ); - db2.shutdown(); - } - private void repeatedlyPopulateDatabase( GraphDatabaseService db, AtomicBoolean continueFlagReference ) { while ( continueFlagReference.get() ) @@ -371,21 +313,7 @@ private void createIndexes( GraphDatabaseService db ) } } - private GraphDatabaseService createDb2( Integer backupPort ) - { - File storeDir = testDirectory.directory("graph-db-2"); - GraphDatabaseFactory factory = new GraphDatabaseFactory(); - GraphDatabaseBuilder builder = factory.newEmbeddedDatabaseBuilder( storeDir ); - builder.setConfig( OnlineBackupSettings.online_backup_server, "0.0.0.0:" + backupPort ); - return builder.newGraphDatabase(); - } - private void startDb( Integer backupPort ) - { - startDb( db, backupPort ); - } - - private void startDb( EmbeddedDatabaseRule db, Integer backupPort ) { db.setConfig( GraphDatabaseSettings.record_format, recordFormat ); db.setConfig( OnlineBackupSettings.online_backup_enabled, Settings.TRUE ); @@ -405,18 +333,9 @@ private static int runBackupTool( File neo4jHome, String... args ) return runBackupToolFromOtherJvmToGetExitCode( neo4jHome, args ); } - private static int runSameJvm( File home, String name, String... args ) + private DbRepresentation getDbRepresentation() { - try - { - new OnlineBackupCommandBuilder().withRawArgs( args ).backup( home, name ); - return 0; - } - catch ( Exception e ) - { - e.printStackTrace(); - return 1; - } + return DbRepresentation.of( db ); } private DbRepresentation getBackupDbRepresentation( String name ) diff --git a/enterprise/com/src/main/java/org/neo4j/com/storecopy/FileMoveAction.java b/enterprise/com/src/main/java/org/neo4j/com/storecopy/FileMoveAction.java index 27cdefa8d8b49..da15bd6a41004 100644 --- a/enterprise/com/src/main/java/org/neo4j/com/storecopy/FileMoveAction.java +++ b/enterprise/com/src/main/java/org/neo4j/com/storecopy/FileMoveAction.java @@ -74,7 +74,6 @@ public void move( File toDir, CopyOption... copyOptions ) throws IOException Path resolvedPath = toDir.toPath().resolve( relativePath ); Files.createDirectories( resolvedPath.getParent() ); Files.copy( originalPath, resolvedPath, copyOptions ); - Files.delete( originalPath ); } @Override diff --git a/enterprise/com/src/main/java/org/neo4j/com/storecopy/FileMoveProvider.java b/enterprise/com/src/main/java/org/neo4j/com/storecopy/FileMoveProvider.java index 62e04a8d93af1..02620d250f60d 100644 --- a/enterprise/com/src/main/java/org/neo4j/com/storecopy/FileMoveProvider.java +++ b/enterprise/com/src/main/java/org/neo4j/com/storecopy/FileMoveProvider.java @@ -126,7 +126,7 @@ private Stream expandTraverseFiles( File dir, File basePath ) File base = basePath; // Capture effectively-final base path snapshot. Stream files = listing.stream().filter( this::isFile ); Stream dirs = listing.stream().filter( this::isDirectory ); - Stream moveFiles = files.map( f -> moveFileCorrectly( f, base ) ); + Stream moveFiles = files.map( f -> copyFileCorrectly( f, base ) ); Stream traverseDirectories = dirs.flatMap( d -> traverseForMoving( d, base ) ); return Stream.concat( moveFiles, traverseDirectories ); } @@ -171,7 +171,7 @@ private List listFiles( File dir ) * contains the logic for handling files between * the 2 systems */ - private FileMoveAction moveFileCorrectly( File fileToMove, File basePath ) + private FileMoveAction copyFileCorrectly( File fileToMove, File basePath ) { if ( fileMoveActionInformer.shouldBeManagedByPageCache( fileToMove.getName() ) ) { diff --git a/enterprise/com/src/test/java/org/neo4j/com/storecopy/FileMoveActionTest.java b/enterprise/com/src/test/java/org/neo4j/com/storecopy/FileMoveActionTest.java deleted file mode 100644 index 08ba010f9fe93..0000000000000 --- a/enterprise/com/src/test/java/org/neo4j/com/storecopy/FileMoveActionTest.java +++ /dev/null @@ -1,98 +0,0 @@ -/* - * Copyright (c) 2002-2018 "Neo4j," - * Neo4j Sweden AB [http://neo4j.com] - * - * This file is part of Neo4j Enterprise Edition. The included source - * code can be redistributed and/or modified under the terms of the - * GNU AFFERO GENERAL PUBLIC LICENSE Version 3 - * (http://www.fsf.org/licensing/licenses/agpl-3.0.html) with the - * Commons Clause, as found in the associated LICENSE.txt file. - * - * 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 Affero General Public License for more details. - * - * Neo4j object code can be licensed independently from the source - * under separate terms from the AGPL. Inquiries can be directed to: - * licensing@neo4j.com - * - * More information is also available at: - * https://neo4j.com/licensing/ - */ -package org.neo4j.com.storecopy; - -import org.junit.Rule; -import org.junit.Test; - -import java.io.File; -import java.io.IOException; -import java.nio.file.StandardOpenOption; - -import org.neo4j.io.fs.DefaultFileSystemAbstraction; -import org.neo4j.io.fs.FileSystemAbstraction; -import org.neo4j.io.pagecache.PageCache; -import org.neo4j.kernel.configuration.Config; -import org.neo4j.kernel.impl.pagecache.ConfigurableStandalonePageCacheFactory; -import org.neo4j.test.rule.TestDirectory; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -public class FileMoveActionTest -{ - @Rule - public final TestDirectory testDirectory = TestDirectory.testDirectory(); - - private FileSystemAbstraction fileSystemAbstraction = new DefaultFileSystemAbstraction(); - private Config config = Config.defaults(); - - @Test - public void pageCacheFilesMovedDoNotLeaveOriginal() throws IOException - { - // given - PageCache pageCache = aPageCache(); - - // and - File pageCacheFile = new File( "page-cache-file" ); - pageCache.map( pageCacheFile, 100, StandardOpenOption.CREATE ); - - // when - File targetRename = new File( "target" ); - FileMoveAction.copyViaPageCache( pageCacheFile, pageCache ).move( targetRename ); - - // then - assertFalse( pageCacheFile.exists() ); - assertTrue( targetRename.exists() ); - } - - @Test - public void nonPageCacheFilesMovedDoNotLeaveOriginal() throws IOException - { - // given - File baseDirectory = testDirectory.directory(); - File sourceDirectory = new File( baseDirectory, "source" ); - File targetDirectory = new File( baseDirectory, "target" ); - File sourceFile = new File( sourceDirectory, "theFileName" ); - File targetFile = new File( targetDirectory, "theFileName" ); - sourceFile.getParentFile().mkdirs(); - targetDirectory.mkdirs(); - - // and sanity check - assertTrue( sourceFile.createNewFile() ); - assertTrue( sourceFile.exists() ); - assertFalse( targetFile.exists() ); - - // when - FileMoveAction.copyViaFileSystem( sourceFile, sourceDirectory ).move( targetDirectory ); - - // then - assertTrue( targetFile.exists() ); - assertFalse( sourceFile.exists() ); - } - - private PageCache aPageCache() - { - return ConfigurableStandalonePageCacheFactory.createPageCache( fileSystemAbstraction, config ); - } -} diff --git a/enterprise/kernel/src/test/java/org/neo4j/util/TestHelpers.java b/enterprise/kernel/src/test/java/org/neo4j/util/TestHelpers.java index 922d8e2115f31..e9e96eaaee367 100644 --- a/enterprise/kernel/src/test/java/org/neo4j/util/TestHelpers.java +++ b/enterprise/kernel/src/test/java/org/neo4j/util/TestHelpers.java @@ -107,5 +107,6 @@ public static String backupAddressHa( GraphDatabaseAPI graphDatabase ) return hostnamePort.toString(); } + }