From 7a016e49937edf19fd67b61cc95ee12955551bb2 Mon Sep 17 00:00:00 2001
From: Jonas Kalderstam <jonas@kalderstam.se>
Date: Tue, 1 Nov 2016 11:46:10 +0100
Subject: [PATCH] Add missing cc-report-dir option to BackupCommand

---
 .../org/neo4j/backup/OnlineBackupCommand.java | 75 ++++++-------------
 .../neo4j/backup/OnlineBackupCommandTest.java |  5 +-
 2 files changed, 26 insertions(+), 54 deletions(-)

diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/OnlineBackupCommand.java b/enterprise/backup/src/main/java/org/neo4j/backup/OnlineBackupCommand.java
index de3c395921e05..3704623be1055 100644
--- a/enterprise/backup/src/main/java/org/neo4j/backup/OnlineBackupCommand.java
+++ b/enterprise/backup/src/main/java/org/neo4j/backup/OnlineBackupCommand.java
@@ -29,10 +29,12 @@
 import org.neo4j.commandline.admin.CommandFailed;
 import org.neo4j.commandline.admin.IncorrectUsage;
 import org.neo4j.commandline.admin.OutsideWorld;
-import org.neo4j.consistency.ConsistencyCheckService;
 import org.neo4j.commandline.arguments.Arguments;
 import org.neo4j.commandline.arguments.MandatoryNamedArg;
+import org.neo4j.commandline.arguments.OptionalBooleanArg;
 import org.neo4j.commandline.arguments.OptionalNamedArg;
+import org.neo4j.commandline.arguments.common.OptionalCanonicalPath;
+import org.neo4j.consistency.ConsistencyCheckService;
 import org.neo4j.consistency.ConsistencyCheckSettings;
 import org.neo4j.graphdb.factory.GraphDatabaseSettings;
 import org.neo4j.helpers.Args;
@@ -41,15 +43,8 @@
 import org.neo4j.kernel.configuration.Config;
 import org.neo4j.server.configuration.ConfigLoader;
 
-import static java.lang.String.format;
 import static java.util.Arrays.asList;
-
-import static org.neo4j.kernel.impl.util.Converters.mandatory;
-import static org.neo4j.kernel.impl.util.Converters.optional;
-import static org.neo4j.kernel.impl.util.Converters.toFile;
 import static org.neo4j.kernel.impl.util.Converters.toHostnamePort;
-import static org.neo4j.kernel.impl.util.Converters.toPath;
-import static org.neo4j.kernel.impl.util.Converters.withDefault;
 
 public class OnlineBackupCommand implements AdminCommand
 {
@@ -60,12 +55,13 @@ public class OnlineBackupCommand implements AdminCommand
             .withArgument( new MandatoryNamedArg( "to", "backup-path",
                     "Directory where the backup will be made; if there is already a backup present an " +
                             "incremental backup will be attempted." ) )
-            .withArgument( new OptionalNamedArg( "check-consistency", "true|false", "true", "If a consistency" +
-                    " check should be made." ) )
+            .withArgument( new OptionalBooleanArg( "check-consistency", true,
+                    "If a consistency check should be made." ) )
+            .withArgument( new OptionalCanonicalPath( "cc-report-dir", "directory", ".",
+                    "Directory where consistency report will be written.") )
             .withAdditionalConfig()
             .withArgument( new OptionalNamedArg( "timeout", "timeout", "20m",
                     "Timeout in the form <time>[ms|s|m|h], where the default unit is seconds." ) );
-    private static final String checkConsistencyArg = "check-consistency";
 
     public static class Provider extends AdminCommand.Provider
     {
@@ -120,19 +116,25 @@ public OnlineBackupCommand( BackupTool backupTool, Path homeDir, Path configDir,
     @Override
     public void execute( String[] args ) throws IncorrectUsage, CommandFailed
     {
-        Args parsedArgs = Args.withFlags( checkConsistencyArg ).parse( args );
         HostnamePort address;
         File destination;
-        ConsistencyCheck consistencyCheck;
+        ConsistencyCheck consistencyCheck = ConsistencyCheck.NONE;
         Optional<Path> additionalConfig;
         long timeout;
         try
         {
-            address = parseAddress( parsedArgs );
-            destination = parseDestination( parsedArgs );
-            consistencyCheck = parseConsistencyCheck( parsedArgs );
-            additionalConfig = parseAdditionalConfig( parsedArgs );
-            timeout = parseTimeout( parsedArgs );
+            address = toHostnamePort( new HostnamePort( "localhost", 6362 ) )
+                    .apply( arguments.parse( "from", args ) );
+            destination = arguments.parseMandatoryPath( "to", args ).toFile();
+            timeout = parseTimeout( args );
+            additionalConfig = arguments.parseOptionalPath( "additional-config", args );
+            if ( arguments.parseBoolean( "check-consistency", args ) )
+            {
+                Path reportDir = arguments.parseOptionalPath( "cc-report-dir", args )
+                        .orElseThrow( () ->
+                        new IllegalArgumentException( "cc-report-dir must be a path" ) );
+                consistencyCheck = ConsistencyCheck.full( reportDir.toFile(), consistencyCheckService );
+            }
         }
         catch ( IllegalArgumentException e )
         {
@@ -150,42 +152,9 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed
         }
     }
 
-    private HostnamePort parseAddress( Args args )
-    {
-        HostnamePort defaultAddress = new HostnamePort( "localhost", 6362 );
-        return args.interpretOption( "from", withDefault( defaultAddress ), toHostnamePort( defaultAddress ) );
-    }
-
-    private File parseDestination( Args parsedArgs )
-    {
-        return parsedArgs.interpretOption( "to", mandatory(), toFile() );
-    }
-
-    private ConsistencyCheck parseConsistencyCheck( Args args ) throws CommandFailed
-    {
-        File reportDir;
-        try
-        {
-            reportDir = args.interpretOption( "cc-report-dir", withDefault( new File(".") ), File::new )
-                    .getCanonicalFile();
-        }
-        catch ( IOException e )
-        {
-            throw new CommandFailed( format( "cannot access report directory: %s: %s",
-                    e.getClass().getSimpleName(), e.getMessage() ), e );
-        }
-        return args.getBoolean( checkConsistencyArg, true, true ) ?
-                ConsistencyCheck.full( reportDir, consistencyCheckService ) : ConsistencyCheck.NONE;
-    }
-
-    private Optional<Path> parseAdditionalConfig( Args args )
-    {
-        return Optional.ofNullable( args.interpretOption( "additional-config", optional(), toPath() ) );
-    }
-
-    private long parseTimeout( Args parsedArgs )
+    private long parseTimeout( String[] args )
     {
-        return parsedArgs.getDuration( "timeout", TimeUnit.MINUTES.toMillis( 20 ) );
+        return Args.parse( args ).getDuration( "timeout", TimeUnit.MINUTES.toMillis( 20 ) );
     }
 
     private Config loadConfig( Optional<Path> additionalConfig ) throws CommandFailed
diff --git a/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandTest.java b/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandTest.java
index 73d2c4e7b707e..a71f41367149a 100644
--- a/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandTest.java
+++ b/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandTest.java
@@ -286,7 +286,8 @@ public void shouldPrintNiceHelp() throws Throwable
         usage.printUsageForCommand( new OnlineBackupCommand.Provider(), out );
 
         verify( out ).accept( "usage: neo4j-admin backup [--from=<address>] --to=<backup-path>\n" +
-                "                          [--check-consistency=<true|false>]\n" +
+                "                          [--check-consistency[=<true|false>]]\n" +
+                "                          [--cc-report-dir=<directory>]\n" +
                 "                          [--additional-config=<config-file-path>]\n" +
                 "                          [--timeout=<timeout>]" );
         verify( out ).accept( "" );
@@ -304,6 +305,8 @@ public void shouldPrintNiceHelp() throws Throwable
                 "                                           attempted.\n" +
                 "  --check-consistency=<true|false>         If a consistency check should be\n" +
                 "                                           made. [default:true]\n" +
+                "  --cc-report-dir=<directory>              Directory where consistency report\n" +
+                "                                           will be written. [default:.]\n" +
                 "  --additional-config=<config-file-path>   Configuration file to supply\n" +
                 "                                           additional configuration in.\n" +
                 "                                           [default:]\n" +