Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ public class ExportCommand extends ExportCommandOptions implements Callable<Inte

@Override
public Integer call() throws Exception {
validateDeprecatedOptions();
applyDeprecatedOptions();
String scalarDbPropertiesFilePath = getScalarDbPropertiesFilePath();

try {
Expand Down Expand Up @@ -107,6 +109,39 @@ public Integer call() throws Exception {
return 0;
}

/**
* Validates that deprecated and new options are not both specified.
*
* @throws CommandLine.ParameterException if both old and new options are specified
*/
private void validateDeprecatedOptions() {
boolean hasDeprecatedStartExclusive =
spec.commandLine().getParseResult().hasMatchedOption(DEPRECATED_START_EXCLUSIVE_OPTION);
boolean hasNewStartInclusive =
spec.commandLine().getParseResult().hasMatchedOption(START_INCLUSIVE_OPTION)
|| spec.commandLine().getParseResult().hasMatchedOption(START_INCLUSIVE_OPTION_SHORT);

if (hasDeprecatedStartExclusive && hasNewStartInclusive) {
throw new CommandLine.ParameterException(
spec.commandLine(),
DataLoaderError.DEPRECATED_AND_NEW_OPTION_BOTH_SPECIFIED.buildMessage(
DEPRECATED_START_EXCLUSIVE_OPTION, START_INCLUSIVE_OPTION, START_INCLUSIVE_OPTION));
}

boolean hasDeprecatedEndExclusive =
spec.commandLine().getParseResult().hasMatchedOption(DEPRECATED_END_EXCLUSIVE_OPTION);
boolean hasNewEndInclusive =
spec.commandLine().getParseResult().hasMatchedOption(END_INCLUSIVE_OPTION)
|| spec.commandLine().getParseResult().hasMatchedOption(END_INCLUSIVE_OPTION_SHORT);

if (hasDeprecatedEndExclusive && hasNewEndInclusive) {
throw new CommandLine.ParameterException(
spec.commandLine(),
DataLoaderError.DEPRECATED_AND_NEW_OPTION_BOTH_SPECIFIED.buildMessage(
DEPRECATED_END_EXCLUSIVE_OPTION, END_INCLUSIVE_OPTION, END_INCLUSIVE_OPTION));
}
}

private String getScalarDbPropertiesFilePath() {
if (StringUtils.isBlank(configFilePath)) {
throw new IllegalArgumentException(DataLoaderError.CONFIG_FILE_PATH_BLANK.buildMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@
public class ExportCommandOptions {

protected static final String DEFAULT_CONFIG_FILE_NAME = "scalardb.properties";
public static final String START_INCLUSIVE_OPTION = "--start-inclusive";
public static final String START_INCLUSIVE_OPTION_SHORT = "-si";
public static final String DEPRECATED_START_EXCLUSIVE_OPTION = "--start-exclusive";
public static final String END_INCLUSIVE_OPTION = "--end-inclusive";
public static final String END_INCLUSIVE_OPTION_SHORT = "-ei";
public static final String DEPRECATED_END_EXCLUSIVE_OPTION = "--end-exclusive";

@CommandLine.Option(
names = {"--config", "-c"},
Expand Down Expand Up @@ -87,6 +93,14 @@ public class ExportCommandOptions {
// TODO: test that -si false, works
protected boolean scanStartInclusive;

// Deprecated option - kept for backward compatibility
@CommandLine.Option(
names = {DEPRECATED_START_EXCLUSIVE_OPTION},
description = "Deprecated: Use --start-inclusive instead (inverted logic)",
hidden = true)
@Deprecated
protected Boolean startExclusiveDeprecated;

@CommandLine.Option(
names = {"--end-key", "-ek"},
paramLabel = "<KEY=VALUE>",
Expand All @@ -100,6 +114,14 @@ public class ExportCommandOptions {
defaultValue = "true")
protected boolean scanEndInclusive;

// Deprecated option - kept for backward compatibility
@CommandLine.Option(
names = {DEPRECATED_END_EXCLUSIVE_OPTION},
description = "Deprecated: Use --end-inclusive instead (inverted logic)",
hidden = true)
@Deprecated
protected Boolean endExclusiveDeprecated;

@CommandLine.Option(
names = {"--sort-by", "-s"},
paramLabel = "<SORT_ORDER>",
Expand Down Expand Up @@ -144,4 +166,23 @@ public class ExportCommandOptions {
description = "Size of the data chunk to process in a single task (default: 200)",
defaultValue = "200")
protected int dataChunkSize;

/**
* Applies deprecated option values if they are set.
*
* <p>This method is called AFTER validateDeprecatedOptions(), so we are guaranteed that both the
* deprecated and new options were not specified together. If we reach this point, only the
* deprecated option was provided by the user.
*/
public void applyDeprecatedOptions() {
// If the deprecated option is set, use its value (inverted logic)
if (startExclusiveDeprecated != null) {
scanStartInclusive = !startExclusiveDeprecated;
}

// If the deprecated option is set, use its value (inverted logic)
if (endExclusiveDeprecated != null) {
scanEndInclusive = !endExclusiveDeprecated;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ public class ImportCommand extends ImportCommandOptions implements Callable<Inte

@Override
public Integer call() throws Exception {
validateDeprecatedOptions();
applyDeprecatedOptions();
validateImportTarget(controlFilePath, namespace, tableName);
validateLogDirectory(logDirectory);
validatePositiveValue(
Expand Down Expand Up @@ -272,6 +274,29 @@ private Optional<ControlFile> parseControlFileFromPath(String controlFilePath) {
}
}

/**
* Validates that deprecated and new options are not both specified.
*
* @throws ParameterException if both old and new options are specified
*/
private void validateDeprecatedOptions() {
// Using getParseResult allows us to check if the argument was specified or not, regardless of
// the default option
boolean hasDeprecatedThreads =
spec.commandLine().getParseResult().hasMatchedOption(DEPRECATED_THREADS_OPTION);
boolean hasNewMaxThreads =
spec.commandLine().getParseResult().hasMatchedOption(MAX_THREADS_OPTION)
|| spec.commandLine().getParseResult().hasMatchedOption(MAX_THREADS_OPTION_SHORT);

// Throw exception if both are specified
if (hasDeprecatedThreads && hasNewMaxThreads) {
throw new ParameterException(
spec.commandLine(),
DataLoaderError.DEPRECATED_AND_NEW_OPTION_BOTH_SPECIFIED.buildMessage(
DEPRECATED_THREADS_OPTION, MAX_THREADS_OPTION, MAX_THREADS_OPTION));
}
}

/**
* Generate import options object from provided cli parameter data
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
public class ImportCommandOptions {

public static final String FILE_OPTION_NAME_LONG_FORMAT = "--file";
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";

@CommandLine.Option(
names = {"--mode", "-m"},
Expand Down Expand Up @@ -39,6 +42,15 @@ public class ImportCommandOptions {
defaultValue = "16")
protected int maxThreads;

// Deprecated option - kept for backward compatibility
@CommandLine.Option(
names = {DEPRECATED_THREADS_OPTION},
paramLabel = "<THREADS>",
description = "Deprecated: Use --max-threads instead",
hidden = true)
@Deprecated
protected Integer threadsDeprecated;

@CommandLine.Option(
names = {"--namespace", "-ns"},
paramLabel = "<NAMESPACE>",
Expand Down Expand Up @@ -158,4 +170,18 @@ public class ImportCommandOptions {
description = "Maximum number of data chunks that can be kept at a time for processing",
defaultValue = "256")
protected int dataChunkQueueSize;

/**
* Applies deprecated option values if they are set.
*
* <p>This method is called AFTER validateDeprecatedOptions(), so we are guaranteed that both the
* deprecated and new options were not specified together. If we reach this point, only the
* deprecated option was provided by the user.
*/
public void applyDeprecatedOptions() {
// If the deprecated option is set, use its value
if (threadsDeprecated != null) {
maxThreads = threadsDeprecated;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,27 @@
import java.nio.file.Paths;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import picocli.CommandLine;

class ExportCommandTest {

private ExportCommand exportCommand;

@BeforeEach
void setUp() {
exportCommand = new ExportCommand();
CommandLine cmd = new CommandLine(exportCommand);
// Parse empty args to properly initialize spec.commandLine() reference
try {
cmd.parseArgs();
} catch (Exception e) {
// Ignore parse errors for missing required options - we'll set fields directly in tests
}
exportCommand.spec = cmd.getCommandSpec();
}

@AfterEach
void removeFileIfCreated() {
// To remove generated file if it is present
Expand All @@ -24,7 +41,6 @@ void removeFileIfCreated() {

@Test
void call_withBlankScalarDBConfigurationFile_shouldThrowException() {
ExportCommand exportCommand = new ExportCommand();
exportCommand.configFilePath = "";
exportCommand.dataChunkSize = 100;
exportCommand.maxThreads = 4;
Expand All @@ -44,7 +60,6 @@ void call_withBlankScalarDBConfigurationFile_shouldThrowException() {

@Test
void call_withInvalidScalarDBConfigurationFile_shouldReturnOne() throws Exception {
ExportCommand exportCommand = new ExportCommand();
exportCommand.configFilePath = "scalardb.properties";
exportCommand.dataChunkSize = 100;
exportCommand.maxThreads = 4;
Expand All @@ -55,4 +70,70 @@ void call_withInvalidScalarDBConfigurationFile_shouldReturnOne() throws Exceptio
exportCommand.outputFormat = FileFormat.JSON;
Assertions.assertEquals(1, exportCommand.call());
}

@Test
void call_withBothStartExclusiveAndStartInclusive_shouldThrowException() {
// Simulate command line parsing with both deprecated and new options
String[] args = {
"--config",
"scalardb.properties",
"--namespace",
"scalar",
"--table",
"asset",
"--format",
"JSON",
"--start-exclusive=false",
"--start-inclusive=false"
};
ExportCommand command = new ExportCommand();
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");
Assertions.assertTrue(
thrown
.getMessage()
.contains(
"Cannot specify both deprecated option '--start-exclusive' and new option '--start-inclusive'"));
}

@Test
void call_withBothEndExclusiveAndEndInclusive_shouldThrowException() {
// Simulate command line parsing with both deprecated and new options
String[] args = {
"--config",
"scalardb.properties",
"--namespace",
"scalar",
"--table",
"asset",
"--format",
"JSON",
"--end-exclusive=false",
"--end-inclusive=false"
};
ExportCommand command = new ExportCommand();
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");
Assertions.assertTrue(
thrown
.getMessage()
.contains(
"Cannot specify both deprecated option '--end-exclusive' and new option '--end-inclusive'"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ public class ImportCommandTest {
void setUp() {
importCommand = new ImportCommand();
CommandLine cmd = new CommandLine(importCommand);
// Parse empty args to properly initialize spec.commandLine() reference
try {
cmd.parseArgs();
} catch (Exception e) {
// Ignore parse errors for missing required options - we'll set fields directly in tests
}
importCommand.spec = cmd.getCommandSpec();
}

Expand Down Expand Up @@ -67,4 +73,44 @@ private void deleteFile(Path file) {
LOGGER.error("Failed to delete file: {}", file, e);
}
}

@Test
void call_withBothThreadsAndMaxThreads_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 = {
"--config",
configFile.toString(),
"--file",
importFile.toString(),
"--namespace",
"sample",
"--table",
"table",
"--threads",
"8",
"--max-threads",
"16"
};
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");
org.junit.jupiter.api.Assertions.assertTrue(
thrown
.getMessage()
.contains(
"Cannot specify both deprecated option '--threads' and new option '--max-threads'"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,12 @@ public enum DataLoaderError implements ScalarDbError {
Category.USER_ERROR, "0055", "Transaction size must be greater than 0", "", ""),
INVALID_MAX_THREADS(
Category.USER_ERROR, "0056", "Number of max threads must be greater than 0", "", ""),
DEPRECATED_AND_NEW_OPTION_BOTH_SPECIFIED(
Category.USER_ERROR,
"0057",
"Cannot specify both deprecated option '%s' and new option '%s'. Please use '%s' instead",
"",
""),

//
// Errors for the internal error category
Expand Down