Log warning if --include-metadata is used

Author: ypeckstadt

data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommand.java

@@ -55,6 +55,7 @@ public class ExportCommand extends ExportCommandOptions implements Callable<Inte
   public Integer call() throws Exception {
     validateDeprecatedOptions();
     applyDeprecatedOptions();
+    warnAboutIgnoredDeprecatedOptions();
     String scalarDbPropertiesFilePath = getScalarDbPropertiesFilePath();
 
     try {
@@ -142,6 +143,29 @@ private void validateDeprecatedOptions() {
         MAX_THREADS_OPTION_SHORT);
   }
 
+  /** Warns about deprecated options that are no longer used and have been completely ignored. */
+  private void warnAboutIgnoredDeprecatedOptions() {
+    boolean hasIncludeMetadata =
+        spec.commandLine().getParseResult().hasMatchedOption(DEPRECATED_INCLUDE_METADATA_OPTION)
+            || spec.commandLine()
+                .getParseResult()
+                .hasMatchedOption(DEPRECATED_INCLUDE_METADATA_OPTION_SHORT);
+
+    if (hasIncludeMetadata) {
+      // Use picocli's ANSI support for colored warning output
+      CommandLine.Help.Ansi ansi = CommandLine.Help.Ansi.AUTO;
+      String warning =
+          ansi.string(
+              "@|bold,yellow The "
+                  + DEPRECATED_INCLUDE_METADATA_OPTION
+                  + " option is deprecated and no longer has any effect. "
+                  + "Use the 'scalar.db.consensus_commit.include_metadata.enabled' configuration property "
+                  + "in your ScalarDB properties file to control whether transaction metadata is included in full scans.|@");
+
+      logger.warn(warning);
+    }
+  }
+
   private String getScalarDbPropertiesFilePath() {
     if (StringUtils.isBlank(configFilePath)) {
       throw new IllegalArgumentException(DataLoaderError.CONFIG_FILE_PATH_BLANK.buildMessage());
@@ -161,8 +185,7 @@ private void validateOutputDirectory() throws DirectoryValidationException {
 
   private ExportManager createExportManager(
       TransactionFactory transactionFactory, ScalarDbDao scalarDbDao, FileFormat fileFormat) {
-    ProducerTaskFactory taskFactory =
-        new ProducerTaskFactory(delimiter, prettyPrintJson);
+    ProducerTaskFactory taskFactory = new ProducerTaskFactory(delimiter, prettyPrintJson);
     DistributedTransactionManager manager = transactionFactory.getTransactionManager();
     switch (fileFormat) {
       case JSON:

data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommandOptions.java

@@ -19,6 +19,8 @@ public class ExportCommandOptions {
   public static final String MAX_THREADS_OPTION = "--max-threads";
   public static final String MAX_THREADS_OPTION_SHORT = "-mt";
   public static final String DEPRECATED_THREADS_OPTION = "--threads";
+  public static final String DEPRECATED_INCLUDE_METADATA_OPTION = "--include-metadata";
+  public static final String DEPRECATED_INCLUDE_METADATA_OPTION_SHORT = "-m";
 
   @CommandLine.Option(
       names = {"--config", "-c"},
@@ -70,13 +72,14 @@ public class ExportCommandOptions {
   protected FileFormat outputFormat;
 
   /**
-   * @deprecated As of release 3.16.2 This option is no longer used and will be removed in release
-   *     4.0.0.
+   * @deprecated As of release 3.16.2 This option is no longer used and will be removed in release.
+   *     The option is not fully removed as it will break backwards 4.0.0.
    */
   @Deprecated
   @CommandLine.Option(
       names = {"--include-metadata", "-m"},
-      description = "Deprecated: This option is no longer used",
+      description =
+          "Deprecated: This option is no longer used. Please use scalar.db.consensus_commit.include_metadata.enabled to control whether transaction metadata is included in full scans.",
       defaultValue = "false",
       hidden = true)
   protected boolean includeTransactionMetadata;

data-loader/cli/src/test/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommandTest.java

@@ -273,4 +273,125 @@ void call_withoutMaxThreads_shouldDefaultToAvailableProcessors() {
     // Verify it was set to available processors
     assertEquals(Runtime.getRuntime().availableProcessors(), command.maxThreads);
   }
+
+  @Test
+  void call_withDeprecatedIncludeMetadataOption_shouldParseWithoutError() {
+    // Simulate command line parsing with deprecated --include-metadata option
+    String[] args = {
+      "--config",
+      "scalardb.properties",
+      "--namespace",
+      "scalar",
+      "--table",
+      "asset",
+      "--format",
+      "JSON",
+      "--include-metadata"
+    };
+    ExportCommand command = new ExportCommand();
+    CommandLine cmd = new CommandLine(command);
+    cmd.parseArgs(args);
+
+    // Verify the deprecated option was parsed
+    // Since it has defaultValue = "false", using the flag sets it to true
+    assertEquals(true, command.includeTransactionMetadata);
+
+    // Verify that the spec was set correctly
+    command.spec = cmd.getCommandSpec();
+
+    // Verify that the command line has the deprecated option matched
+    assertTrue(
+        cmd.getParseResult()
+            .hasMatchedOption(ExportCommandOptions.DEPRECATED_INCLUDE_METADATA_OPTION));
+  }
+
+  @Test
+  void call_withDeprecatedIncludeMetadataShortOption_shouldParseWithoutError() {
+    // Simulate command line parsing with deprecated -m short option
+    String[] args = {
+      "--config",
+      "scalardb.properties",
+      "--namespace",
+      "scalar",
+      "--table",
+      "asset",
+      "--format",
+      "JSON",
+      "-m"
+    };
+    ExportCommand command = new ExportCommand();
+    CommandLine cmd = new CommandLine(command);
+    cmd.parseArgs(args);
+
+    // Verify the deprecated option was parsed
+    assertEquals(true, command.includeTransactionMetadata);
+
+    // Verify that the spec was set correctly
+    command.spec = cmd.getCommandSpec();
+
+    // Verify that the command line has the deprecated short option matched
+    assertTrue(
+        cmd.getParseResult()
+            .hasMatchedOption(ExportCommandOptions.DEPRECATED_INCLUDE_METADATA_OPTION_SHORT));
+  }
+
+  @Test
+  void call_withDeprecatedIncludeMetadataFalse_shouldParseWithoutError() {
+    // Simulate command line parsing with deprecated --include-metadata=false option
+    String[] args = {
+      "--config",
+      "scalardb.properties",
+      "--namespace",
+      "scalar",
+      "--table",
+      "asset",
+      "--format",
+      "JSON",
+      "--include-metadata=false"
+    };
+    ExportCommand command = new ExportCommand();
+    CommandLine cmd = new CommandLine(command);
+    cmd.parseArgs(args);
+
+    // Verify the deprecated option was parsed with explicit false value
+    assertEquals(false, command.includeTransactionMetadata);
+
+    // Verify that the spec was set correctly
+    command.spec = cmd.getCommandSpec();
+
+    // Verify that the command line has the deprecated option matched
+    assertTrue(
+        cmd.getParseResult()
+            .hasMatchedOption(ExportCommandOptions.DEPRECATED_INCLUDE_METADATA_OPTION));
+  }
+
+  @Test
+  void call_withoutDeprecatedIncludeMetadataOption_shouldHaveDefaultValue() {
+    // Simulate command line parsing without the deprecated option
+    String[] args = {
+      "--config",
+      "scalardb.properties",
+      "--namespace",
+      "scalar",
+      "--table",
+      "asset",
+      "--format",
+      "JSON"
+    };
+    ExportCommand command = new ExportCommand();
+    CommandLine cmd = new CommandLine(command);
+    cmd.parseArgs(args);
+
+    // Verify the option has its default value (false)
+    assertEquals(false, command.includeTransactionMetadata);
+
+    // Verify that the spec was set correctly
+    command.spec = cmd.getCommandSpec();
+
+    // Verify that the command line does NOT have the deprecated option matched
+    assertEquals(
+        false,
+        cmd.getParseResult()
+            .hasMatchedOption(ExportCommandOptions.DEPRECATED_INCLUDE_METADATA_OPTION));
+  }
 }

data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/ExportManager.java

@@ -11,7 +11,6 @@
 import com.scalar.db.dataloader.core.dataexport.validation.ExportOptionsValidator;
 import com.scalar.db.dataloader.core.dataimport.dao.ScalarDbDao;
 import com.scalar.db.dataloader.core.dataimport.dao.ScalarDbDaoException;
-import com.scalar.db.dataloader.core.util.TableMetadataUtil;
 import com.scalar.db.exception.transaction.CrudException;
 import com.scalar.db.exception.transaction.UnknownTransactionStatusException;
 import com.scalar.db.io.DataType;

data-loader/core/src/test/java/com/scalar/db/dataloader/core/dataexport/CsvExportManagerTest.java

@@ -88,7 +88,7 @@ void startExport_givenValidDataWithoutPartitionKey_shouldGenerateOutputFile()
   @Test
   void startExport_givenPartitionKey_shouldGenerateOutputFile()
       throws IOException, ScalarDbDaoException {
-      producerTaskFactory = new ProducerTaskFactory(",", false);
+    producerTaskFactory = new ProducerTaskFactory(",", false);
     exportManager = new CsvExportManager(manager, dao, producerTaskFactory);
     TransactionManagerCrudOperable.Scanner scanner =
         Mockito.mock(TransactionManagerCrudOperable.Scanner.class);
@@ -152,7 +152,7 @@ void startExport_givenHeaderRequired_shouldGenerateOutputFileWithHeader() throws
   private void runExportAndAssertFirstLine(boolean excludeHeader, String expectedFirstLine)
       throws Exception {
     // Arrange
-      producerTaskFactory = new ProducerTaskFactory(",", false);
+    producerTaskFactory = new ProducerTaskFactory(",", false);
     exportManager = new CsvExportManager(manager, dao, producerTaskFactory);
     TransactionManagerCrudOperable.Scanner scanner =
         Mockito.mock(TransactionManagerCrudOperable.Scanner.class);