Backport to branch(3.16) : Deprecate --log-success argument and introduce new replacement option --enable-log-success (#3144)

Co-authored-by: inv-jishnu <[email protected]>
Co-authored-by: Peckstadt Yves <[email protected]>

Author: 3 people

data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommand.java

@@ -286,6 +286,11 @@ private void validateDeprecatedOptions() {
         DEPRECATED_THREADS_OPTION,
         MAX_THREADS_OPTION,
         MAX_THREADS_OPTION_SHORT);
+    validateDeprecatedOptionPair(
+        spec.commandLine(),
+        DEPRECATED_LOG_SUCCESS_RECORDS_OPTION,
+        ENABLE_LOG_SUCCESS_RECORDS_OPTION,
+        ENABLE_LOG_SUCCESS_RECORDS_OPTION_SHORT);
   }
 
   /**
@@ -303,7 +308,7 @@ private ImportOptions createImportOptions(ControlFile controlFile) {
             .controlFile(controlFile)
             .controlFileValidationLevel(controlFileValidation)
             .logRawRecord(logRawRecord)
-            .logSuccessRecords(logSuccessRecords)
+            .logSuccessRecords(enableLogSuccessRecords)
             .ignoreNullValues(ignoreNullValues)
             .namespace(namespace)
             .dataChunkSize(dataChunkSize)

data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommandOptions.java

@@ -13,6 +13,10 @@ public class ImportCommandOptions {
   public static final String MAX_THREADS_OPTION_SHORT = "-mt";
   public static final String DEPRECATED_THREADS_OPTION = "--threads";
 
+  public static final String ENABLE_LOG_SUCCESS_RECORDS_OPTION = "--enable-log-success";
+  public static final String ENABLE_LOG_SUCCESS_RECORDS_OPTION_SHORT = "-ls";
+  public static final String DEPRECATED_LOG_SUCCESS_RECORDS_OPTION = "--log-success";
+
   @CommandLine.Option(
       names = {"--mode", "-m"},
       description = "ScalarDB mode (STORAGE, TRANSACTION) (default: STORAGE)",
@@ -69,11 +73,22 @@ public class ImportCommandOptions {
       description = "Path to the JSON control file for data mapping")
   protected String controlFilePath;
 
+  /**
+   * @deprecated As of release 3.6.2. Will be removed in release 4.0.0. Use --enable-log-success
+   *     instead
+   */
+  @Deprecated
+  @CommandLine.Option(
+      names = {"--log-success"},
+      description = "Deprecated: Use --enable-log-success",
+      hidden = true)
+  protected boolean logSuccessRecordsDeprecated;
+
   @CommandLine.Option(
-      names = {"--log-success", "-ls"},
+      names = {"--enable-log-success", "-ls"},
       description = "Enable logging of successfully processed records (default: false)",
       defaultValue = "false")
-  protected boolean logSuccessRecords;
+  protected boolean enableLogSuccessRecords;
 
   @CommandLine.Option(
       names = {"--log-dir", "-ld"},
@@ -183,5 +198,8 @@ public void applyDeprecatedOptions() {
     if (threadsDeprecated != null) {
       maxThreads = threadsDeprecated;
     }
+    if (logSuccessRecordsDeprecated) {
+      enableLogSuccessRecords = true;
+    }
   }
 }

data-loader/cli/src/test/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommandTest.java

@@ -149,4 +149,84 @@ void call_withOnlyDeprecatedThreads_shouldApplyValue() throws Exception {
     // Verify the value was applied to maxThreads
     assertEquals(12, command.maxThreads);
   }
+
+  @Test
+  void call_withBothLogSuccessAndEnableLogSuccess_shouldThrowException() throws Exception {
+    Path configFile = tempDir.resolve("config.properties");
+    Files.createFile(configFile);
+    Path importFile = tempDir.resolve("import.json");
+    Files.createFile(importFile);
+
+    // Simulate command line parsing with both deprecated and new options
+    String[] args = {"--file", importFile.toString(), "--log-success", "--enable-log-success"};
+    ImportCommand command = new ImportCommand();
+    CommandLine cmd = new CommandLine(command);
+    // Parse args - this will trigger our validation
+    cmd.parseArgs(args);
+
+    // Now call the command, which should throw the validation error
+    CommandLine.ParameterException thrown =
+        assertThrows(
+            CommandLine.ParameterException.class,
+            command::call,
+            "Expected to throw ParameterException when both deprecated and new options are specified");
+    assertTrue(
+        thrown
+            .getMessage()
+            .contains(
+                "Cannot specify both deprecated option '--log-success' and new option '--enable-log-success'"));
+  }
+
+  @Test
+  void call_withOnlyDeprecatedLogSuccess_shouldApplyValue() throws Exception {
+    Path configFile = tempDir.resolve("config.properties");
+    Files.createFile(configFile);
+    Path importFile = tempDir.resolve("import.json");
+    Files.createFile(importFile);
+
+    // Simulate command line parsing with only deprecated option
+    String[] args = {"--file", importFile.toString(), "--log-success"};
+    ImportCommand command = new ImportCommand();
+    CommandLine cmd = new CommandLine(command);
+    cmd.parseArgs(args);
+
+    // Verify the deprecated value was parsed
+    assertTrue(command.logSuccessRecordsDeprecated);
+
+    // Apply deprecated options (this is what the command does after validation)
+    command.applyDeprecatedOptions();
+
+    // Verify the value was applied to enable-log-success
+    assertTrue(command.enableLogSuccessRecords);
+  }
+
+  @Test
+  void call_withEnableLogSuccess_shouldSetToTrueWithoutValue() throws Exception {
+    Path importFile = tempDir.resolve("import.json");
+    Files.createFile(importFile);
+
+    // Simulate command line parsing with the new flag without providing true/false value
+    String[] args = {"--file", importFile.toString(), "--enable-log-success"};
+    ImportCommand command = new ImportCommand();
+    CommandLine cmd = new CommandLine(command);
+    cmd.parseArgs(args);
+
+    // Verify the flag was parsed correctly without requiring a value
+    assertTrue(command.enableLogSuccessRecords);
+  }
+
+  @Test
+  void call_withEnableLogSuccessShortForm_shouldSetToTrueWithoutValue() throws Exception {
+    Path importFile = tempDir.resolve("import.json");
+    Files.createFile(importFile);
+
+    // Simulate command line parsing with the short form flag without providing true/false value
+    String[] args = {"--file", importFile.toString(), "-ls"};
+    ImportCommand command = new ImportCommand();
+    CommandLine cmd = new CommandLine(command);
+    cmd.parseArgs(args);
+
+    // Verify the short form flag was parsed correctly without requiring a value
+    assertTrue(command.enableLogSuccessRecords);
+  }
 }