Skip to content

Commit cb6058f

Browse files
ypeckstadtypeckstadt
committed
Re-add deprecated CLI arguments
1 parent 53383a9 commit cb6058f

File tree

7 files changed

+262
-2
lines changed

7 files changed

+262
-2
lines changed

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ public class ExportCommand extends ExportCommandOptions implements Callable<Inte
5151

5252
@Override
5353
public Integer call() throws Exception {
54+
validateDeprecatedOptions();
55+
applyDeprecatedOptions();
5456
String scalarDbPropertiesFilePath = getScalarDbPropertiesFilePath();
5557

5658
try {
@@ -107,6 +109,39 @@ public Integer call() throws Exception {
107109
return 0;
108110
}
109111

112+
/**
113+
* Validates that deprecated and new options are not both specified.
114+
*
115+
* @throws CommandLine.ParameterException if both old and new options are specified
116+
*/
117+
private void validateDeprecatedOptions() {
118+
boolean hasDeprecatedStartExclusive =
119+
spec.commandLine().getParseResult().hasMatchedOption(DEPRECATED_START_EXCLUSIVE_OPTION);
120+
boolean hasNewStartInclusive =
121+
spec.commandLine().getParseResult().hasMatchedOption(START_INCLUSIVE_OPTION)
122+
|| spec.commandLine().getParseResult().hasMatchedOption(START_INCLUSIVE_OPTION_SHORT);
123+
124+
if (hasDeprecatedStartExclusive && hasNewStartInclusive) {
125+
throw new CommandLine.ParameterException(
126+
spec.commandLine(),
127+
DataLoaderError.DEPRECATED_AND_NEW_OPTION_BOTH_SPECIFIED.buildMessage(
128+
DEPRECATED_START_EXCLUSIVE_OPTION, START_INCLUSIVE_OPTION, START_INCLUSIVE_OPTION));
129+
}
130+
131+
boolean hasDeprecatedEndExclusive =
132+
spec.commandLine().getParseResult().hasMatchedOption(DEPRECATED_END_EXCLUSIVE_OPTION);
133+
boolean hasNewEndInclusive =
134+
spec.commandLine().getParseResult().hasMatchedOption(END_INCLUSIVE_OPTION)
135+
|| spec.commandLine().getParseResult().hasMatchedOption(END_INCLUSIVE_OPTION_SHORT);
136+
137+
if (hasDeprecatedEndExclusive && hasNewEndInclusive) {
138+
throw new CommandLine.ParameterException(
139+
spec.commandLine(),
140+
DataLoaderError.DEPRECATED_AND_NEW_OPTION_BOTH_SPECIFIED.buildMessage(
141+
DEPRECATED_END_EXCLUSIVE_OPTION, END_INCLUSIVE_OPTION, END_INCLUSIVE_OPTION));
142+
}
143+
}
144+
110145
private String getScalarDbPropertiesFilePath() {
111146
if (StringUtils.isBlank(configFilePath)) {
112147
throw new IllegalArgumentException(DataLoaderError.CONFIG_FILE_PATH_BLANK.buildMessage());

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@
1010
public class ExportCommandOptions {
1111

1212
protected static final String DEFAULT_CONFIG_FILE_NAME = "scalardb.properties";
13+
public static final String START_INCLUSIVE_OPTION = "--start-inclusive";
14+
public static final String START_INCLUSIVE_OPTION_SHORT = "-si";
15+
public static final String DEPRECATED_START_EXCLUSIVE_OPTION = "--start-exclusive";
16+
public static final String END_INCLUSIVE_OPTION = "--end-inclusive";
17+
public static final String END_INCLUSIVE_OPTION_SHORT = "-ei";
18+
public static final String DEPRECATED_END_EXCLUSIVE_OPTION = "--end-exclusive";
1319

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

96+
// Deprecated option - kept for backward compatibility
97+
@CommandLine.Option(
98+
names = {DEPRECATED_START_EXCLUSIVE_OPTION},
99+
description = "Deprecated: Use --start-inclusive instead (inverted logic)",
100+
hidden = true)
101+
@Deprecated
102+
protected Boolean startExclusiveDeprecated;
103+
90104
@CommandLine.Option(
91105
names = {"--end-key", "-ek"},
92106
paramLabel = "<KEY=VALUE>",
@@ -100,6 +114,14 @@ public class ExportCommandOptions {
100114
defaultValue = "true")
101115
protected boolean scanEndInclusive;
102116

117+
// Deprecated option - kept for backward compatibility
118+
@CommandLine.Option(
119+
names = {DEPRECATED_END_EXCLUSIVE_OPTION},
120+
description = "Deprecated: Use --end-inclusive instead (inverted logic)",
121+
hidden = true)
122+
@Deprecated
123+
protected Boolean endExclusiveDeprecated;
124+
103125
@CommandLine.Option(
104126
names = {"--sort-by", "-s"},
105127
paramLabel = "<SORT_ORDER>",
@@ -144,4 +166,23 @@ public class ExportCommandOptions {
144166
description = "Size of the data chunk to process in a single task (default: 200)",
145167
defaultValue = "200")
146168
protected int dataChunkSize;
169+
170+
/**
171+
* Applies deprecated option values if they are set.
172+
*
173+
* <p>This method is called AFTER validateDeprecatedOptions(), so we are guaranteed that both the
174+
* deprecated and new options were not specified together. If we reach this point, only the
175+
* deprecated option was provided by the user.
176+
*/
177+
public void applyDeprecatedOptions() {
178+
// If the deprecated option is set, use its value (inverted logic)
179+
if (startExclusiveDeprecated != null) {
180+
scanStartInclusive = !startExclusiveDeprecated;
181+
}
182+
183+
// If the deprecated option is set, use its value (inverted logic)
184+
if (endExclusiveDeprecated != null) {
185+
scanEndInclusive = !endExclusiveDeprecated;
186+
}
187+
}
147188
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ public class ImportCommand extends ImportCommandOptions implements Callable<Inte
5252

5353
@Override
5454
public Integer call() throws Exception {
55+
validateDeprecatedOptions();
56+
applyDeprecatedOptions();
5557
validateImportTarget(controlFilePath, namespace, tableName);
5658
validateLogDirectory(logDirectory);
5759
validatePositiveValue(
@@ -272,6 +274,29 @@ private Optional<ControlFile> parseControlFileFromPath(String controlFilePath) {
272274
}
273275
}
274276

277+
/**
278+
* Validates that deprecated and new options are not both specified.
279+
*
280+
* @throws ParameterException if both old and new options are specified
281+
*/
282+
private void validateDeprecatedOptions() {
283+
// Using getParseResult allows us to check if the argument was specified or not, regardless of
284+
// the default option
285+
boolean hasDeprecatedThreads =
286+
spec.commandLine().getParseResult().hasMatchedOption(DEPRECATED_THREADS_OPTION);
287+
boolean hasNewMaxThreads =
288+
spec.commandLine().getParseResult().hasMatchedOption(MAX_THREADS_OPTION)
289+
|| spec.commandLine().getParseResult().hasMatchedOption(MAX_THREADS_OPTION_SHORT);
290+
291+
// Throw exception if both are specified
292+
if (hasDeprecatedThreads && hasNewMaxThreads) {
293+
throw new ParameterException(
294+
spec.commandLine(),
295+
DataLoaderError.DEPRECATED_AND_NEW_OPTION_BOTH_SPECIFIED.buildMessage(
296+
DEPRECATED_THREADS_OPTION, MAX_THREADS_OPTION, MAX_THREADS_OPTION));
297+
}
298+
}
299+
275300
/**
276301
* Generate import options object from provided cli parameter data
277302
*

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
public class ImportCommandOptions {
1010

1111
public static final String FILE_OPTION_NAME_LONG_FORMAT = "--file";
12+
public static final String MAX_THREADS_OPTION = "--max-threads";
13+
public static final String MAX_THREADS_OPTION_SHORT = "-mt";
14+
public static final String DEPRECATED_THREADS_OPTION = "--threads";
1215

1316
@CommandLine.Option(
1417
names = {"--mode", "-m"},
@@ -39,6 +42,15 @@ public class ImportCommandOptions {
3942
defaultValue = "16")
4043
protected int maxThreads;
4144

45+
// Deprecated option - kept for backward compatibility
46+
@CommandLine.Option(
47+
names = {DEPRECATED_THREADS_OPTION},
48+
paramLabel = "<THREADS>",
49+
description = "Deprecated: Use --max-threads instead",
50+
hidden = true)
51+
@Deprecated
52+
protected Integer threadsDeprecated;
53+
4254
@CommandLine.Option(
4355
names = {"--namespace", "-ns"},
4456
paramLabel = "<NAMESPACE>",
@@ -158,4 +170,18 @@ public class ImportCommandOptions {
158170
description = "Maximum number of data chunks that can be kept at a time for processing",
159171
defaultValue = "256")
160172
protected int dataChunkQueueSize;
173+
174+
/**
175+
* Applies deprecated option values if they are set.
176+
*
177+
* <p>This method is called AFTER validateDeprecatedOptions(), so we are guaranteed that both the
178+
* deprecated and new options were not specified together. If we reach this point, only the
179+
* deprecated option was provided by the user.
180+
*/
181+
public void applyDeprecatedOptions() {
182+
// If the deprecated option is set, use its value
183+
if (threadsDeprecated != null) {
184+
maxThreads = threadsDeprecated;
185+
}
186+
}
161187
}

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

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,27 @@
88
import java.nio.file.Paths;
99
import org.junit.jupiter.api.AfterEach;
1010
import org.junit.jupiter.api.Assertions;
11+
import org.junit.jupiter.api.BeforeEach;
1112
import org.junit.jupiter.api.Test;
13+
import picocli.CommandLine;
1214

1315
class ExportCommandTest {
1416

17+
private ExportCommand exportCommand;
18+
19+
@BeforeEach
20+
void setUp() {
21+
exportCommand = new ExportCommand();
22+
CommandLine cmd = new CommandLine(exportCommand);
23+
// Parse empty args to properly initialize spec.commandLine() reference
24+
try {
25+
cmd.parseArgs();
26+
} catch (Exception e) {
27+
// Ignore parse errors for missing required options - we'll set fields directly in tests
28+
}
29+
exportCommand.spec = cmd.getCommandSpec();
30+
}
31+
1532
@AfterEach
1633
void removeFileIfCreated() {
1734
// To remove generated file if it is present
@@ -24,7 +41,6 @@ void removeFileIfCreated() {
2441

2542
@Test
2643
void call_withBlankScalarDBConfigurationFile_shouldThrowException() {
27-
ExportCommand exportCommand = new ExportCommand();
2844
exportCommand.configFilePath = "";
2945
exportCommand.dataChunkSize = 100;
3046
exportCommand.maxThreads = 4;
@@ -44,7 +60,6 @@ void call_withBlankScalarDBConfigurationFile_shouldThrowException() {
4460

4561
@Test
4662
void call_withInvalidScalarDBConfigurationFile_shouldReturnOne() throws Exception {
47-
ExportCommand exportCommand = new ExportCommand();
4863
exportCommand.configFilePath = "scalardb.properties";
4964
exportCommand.dataChunkSize = 100;
5065
exportCommand.maxThreads = 4;
@@ -55,4 +70,70 @@ void call_withInvalidScalarDBConfigurationFile_shouldReturnOne() throws Exceptio
5570
exportCommand.outputFormat = FileFormat.JSON;
5671
Assertions.assertEquals(1, exportCommand.call());
5772
}
73+
74+
@Test
75+
void call_withBothStartExclusiveAndStartInclusive_shouldThrowException() {
76+
// Simulate command line parsing with both deprecated and new options
77+
String[] args = {
78+
"--config",
79+
"scalardb.properties",
80+
"--namespace",
81+
"scalar",
82+
"--table",
83+
"asset",
84+
"--format",
85+
"JSON",
86+
"--start-exclusive=false",
87+
"--start-inclusive=false"
88+
};
89+
ExportCommand command = new ExportCommand();
90+
CommandLine cmd = new CommandLine(command);
91+
// Parse args - this will trigger our validation
92+
cmd.parseArgs(args);
93+
94+
// Now call the command, which should throw the validation error
95+
CommandLine.ParameterException thrown =
96+
assertThrows(
97+
CommandLine.ParameterException.class,
98+
command::call,
99+
"Expected to throw ParameterException when both deprecated and new options are specified");
100+
Assertions.assertTrue(
101+
thrown
102+
.getMessage()
103+
.contains(
104+
"Cannot specify both deprecated option '--start-exclusive' and new option '--start-inclusive'"));
105+
}
106+
107+
@Test
108+
void call_withBothEndExclusiveAndEndInclusive_shouldThrowException() {
109+
// Simulate command line parsing with both deprecated and new options
110+
String[] args = {
111+
"--config",
112+
"scalardb.properties",
113+
"--namespace",
114+
"scalar",
115+
"--table",
116+
"asset",
117+
"--format",
118+
"JSON",
119+
"--end-exclusive=false",
120+
"--end-inclusive=false"
121+
};
122+
ExportCommand command = new ExportCommand();
123+
CommandLine cmd = new CommandLine(command);
124+
// Parse args - this will trigger our validation
125+
cmd.parseArgs(args);
126+
127+
// Now call the command, which should throw the validation error
128+
CommandLine.ParameterException thrown =
129+
assertThrows(
130+
CommandLine.ParameterException.class,
131+
command::call,
132+
"Expected to throw ParameterException when both deprecated and new options are specified");
133+
Assertions.assertTrue(
134+
thrown
135+
.getMessage()
136+
.contains(
137+
"Cannot specify both deprecated option '--end-exclusive' and new option '--end-inclusive'"));
138+
}
58139
}

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ public class ImportCommandTest {
2727
void setUp() {
2828
importCommand = new ImportCommand();
2929
CommandLine cmd = new CommandLine(importCommand);
30+
// Parse empty args to properly initialize spec.commandLine() reference
31+
try {
32+
cmd.parseArgs();
33+
} catch (Exception e) {
34+
// Ignore parse errors for missing required options - we'll set fields directly in tests
35+
}
3036
importCommand.spec = cmd.getCommandSpec();
3137
}
3238

@@ -67,4 +73,44 @@ private void deleteFile(Path file) {
6773
LOGGER.error("Failed to delete file: {}", file, e);
6874
}
6975
}
76+
77+
@Test
78+
void call_withBothThreadsAndMaxThreads_shouldThrowException() throws Exception {
79+
Path configFile = tempDir.resolve("config.properties");
80+
Files.createFile(configFile);
81+
Path importFile = tempDir.resolve("import.json");
82+
Files.createFile(importFile);
83+
84+
// Simulate command line parsing with both deprecated and new options
85+
String[] args = {
86+
"--config",
87+
configFile.toString(),
88+
"--file",
89+
importFile.toString(),
90+
"--namespace",
91+
"sample",
92+
"--table",
93+
"table",
94+
"--threads",
95+
"8",
96+
"--max-threads",
97+
"16"
98+
};
99+
ImportCommand command = new ImportCommand();
100+
CommandLine cmd = new CommandLine(command);
101+
// Parse args - this will trigger our validation
102+
cmd.parseArgs(args);
103+
104+
// Now call the command, which should throw the validation error
105+
CommandLine.ParameterException thrown =
106+
assertThrows(
107+
CommandLine.ParameterException.class,
108+
command::call,
109+
"Expected to throw ParameterException when both deprecated and new options are specified");
110+
org.junit.jupiter.api.Assertions.assertTrue(
111+
thrown
112+
.getMessage()
113+
.contains(
114+
"Cannot specify both deprecated option '--threads' and new option '--max-threads'"));
115+
}
70116
}

‎data-loader/core/src/main/java/com/scalar/db/dataloader/core/DataLoaderError.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,12 @@ public enum DataLoaderError implements ScalarDbError {
210210
Category.USER_ERROR, "0055", "Transaction size must be greater than 0", "", ""),
211211
INVALID_MAX_THREADS(
212212
Category.USER_ERROR, "0056", "Number of max threads must be greater than 0", "", ""),
213+
DEPRECATED_AND_NEW_OPTION_BOTH_SPECIFIED(
214+
Category.USER_ERROR,
215+
"0057",
216+
"Cannot specify both deprecated option '%s' and new option '%s'. Please use '%s' instead",
217+
"",
218+
""),
213219

214220
//
215221
// Errors for the internal error category

0 commit comments

Comments
 (0)