Skip to content

Commit 5ba96ea

Browse files
ypeckstadtypeckstadt
committed
Move repeated validation to util class method
1 parent c607435 commit 5ba96ea

File tree

4 files changed

+168
-40
lines changed

4 files changed

+168
-40
lines changed

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

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.scalar.db.dataloader.cli.command.dataexport;
22

3+
import static com.scalar.db.dataloader.cli.util.CommandLineInputUtils.validateDeprecatedOptionPair;
34
import static com.scalar.db.dataloader.cli.util.CommandLineInputUtils.validatePositiveValue;
45
import static java.nio.file.StandardOpenOption.APPEND;
56
import static java.nio.file.StandardOpenOption.CREATE;
@@ -115,31 +116,16 @@ public Integer call() throws Exception {
115116
* @throws CommandLine.ParameterException if both old and new options are specified
116117
*/
117118
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-
}
119+
validateDeprecatedOptionPair(
120+
spec.commandLine(),
121+
DEPRECATED_START_EXCLUSIVE_OPTION,
122+
START_INCLUSIVE_OPTION,
123+
START_INCLUSIVE_OPTION_SHORT);
124+
validateDeprecatedOptionPair(
125+
spec.commandLine(),
126+
DEPRECATED_END_EXCLUSIVE_OPTION,
127+
END_INCLUSIVE_OPTION,
128+
END_INCLUSIVE_OPTION_SHORT);
143129
}
144130

145131
private String getScalarDbPropertiesFilePath() {

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

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.scalar.db.dataloader.cli.command.dataimport;
22

3+
import static com.scalar.db.dataloader.cli.util.CommandLineInputUtils.validateDeprecatedOptionPair;
34
import static com.scalar.db.dataloader.cli.util.CommandLineInputUtils.validatePositiveValue;
45

56
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -280,21 +281,11 @@ private Optional<ControlFile> parseControlFileFromPath(String controlFilePath) {
280281
* @throws ParameterException if both old and new options are specified
281282
*/
282283
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-
}
284+
validateDeprecatedOptionPair(
285+
spec.commandLine(),
286+
DEPRECATED_THREADS_OPTION,
287+
MAX_THREADS_OPTION,
288+
MAX_THREADS_OPTION_SHORT);
298289
}
299290

300291
/**

‎data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/util/CommandLineInputUtils.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,30 @@ public static void validatePositiveValue(
6161
throw new CommandLine.ParameterException(commandLine, error.buildMessage());
6262
}
6363
}
64+
65+
/**
66+
* Validates that a deprecated option and its replacement are not both specified. If both options
67+
* are detected, it throws a {@link picocli.CommandLine.ParameterException} with an appropriate
68+
* error message.
69+
*
70+
* @param commandLine the {@link CommandLine} instance used to provide context for the exception
71+
* @param deprecatedOption the deprecated option name
72+
* @param newOption the new option name
73+
* @param newOptionShort the short form of the new option name
74+
* @throws CommandLine.ParameterException if both deprecated and new options are specified
75+
*/
76+
public static void validateDeprecatedOptionPair(
77+
CommandLine commandLine, String deprecatedOption, String newOption, String newOptionShort) {
78+
boolean hasDeprecated = commandLine.getParseResult().hasMatchedOption(deprecatedOption);
79+
boolean hasNew =
80+
commandLine.getParseResult().hasMatchedOption(newOption)
81+
|| commandLine.getParseResult().hasMatchedOption(newOptionShort);
82+
83+
if (hasDeprecated && hasNew) {
84+
throw new CommandLine.ParameterException(
85+
commandLine,
86+
DataLoaderError.DEPRECATED_AND_NEW_OPTION_BOTH_SPECIFIED.buildMessage(
87+
deprecatedOption, newOption, newOption));
88+
}
89+
}
6490
}

‎data-loader/cli/src/test/java/com/scalar/db/dataloader/cli/util/CommandLineInputUtilsTest.java‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import static org.junit.jupiter.api.Assertions.assertThrows;
77
import static org.junit.jupiter.api.Assertions.assertTrue;
88
import static org.mockito.Mockito.mock;
9+
import static org.mockito.Mockito.when;
910

1011
import com.scalar.db.dataloader.core.DataLoaderError;
1112
import java.util.Map;
@@ -185,4 +186,128 @@ public void validatePositiveValue_differentErrorTypes_shouldUseCorrectErrorMessa
185186
.getMessage()
186187
.contains(DataLoaderError.INVALID_DATA_CHUNK_QUEUE_SIZE.buildMessage()));
187188
}
189+
190+
@Test
191+
public void validateDeprecatedOptionPair_onlyDeprecatedSpecified_shouldNotThrowException() {
192+
// Arrange
193+
CommandLine commandLine = mock(CommandLine.class);
194+
CommandLine.ParseResult parseResult = mock(CommandLine.ParseResult.class);
195+
when(commandLine.getParseResult()).thenReturn(parseResult);
196+
when(parseResult.hasMatchedOption("--old-option")).thenReturn(true);
197+
when(parseResult.hasMatchedOption("--new-option")).thenReturn(false);
198+
when(parseResult.hasMatchedOption("-n")).thenReturn(false);
199+
200+
// Act & Assert - No exception should be thrown
201+
assertDoesNotThrow(
202+
() ->
203+
CommandLineInputUtils.validateDeprecatedOptionPair(
204+
commandLine, "--old-option", "--new-option", "-n"));
205+
}
206+
207+
@Test
208+
public void validateDeprecatedOptionPair_onlyNewOptionSpecified_shouldNotThrowException() {
209+
// Arrange
210+
CommandLine commandLine = mock(CommandLine.class);
211+
CommandLine.ParseResult parseResult = mock(CommandLine.ParseResult.class);
212+
when(commandLine.getParseResult()).thenReturn(parseResult);
213+
when(parseResult.hasMatchedOption("--old-option")).thenReturn(false);
214+
when(parseResult.hasMatchedOption("--new-option")).thenReturn(true);
215+
when(parseResult.hasMatchedOption("-n")).thenReturn(false);
216+
217+
// Act & Assert - No exception should be thrown
218+
assertDoesNotThrow(
219+
() ->
220+
CommandLineInputUtils.validateDeprecatedOptionPair(
221+
commandLine, "--old-option", "--new-option", "-n"));
222+
}
223+
224+
@Test
225+
public void
226+
validateDeprecatedOptionPair_onlyNewOptionShortFormSpecified_shouldNotThrowException() {
227+
// Arrange
228+
CommandLine commandLine = mock(CommandLine.class);
229+
CommandLine.ParseResult parseResult = mock(CommandLine.ParseResult.class);
230+
when(commandLine.getParseResult()).thenReturn(parseResult);
231+
when(parseResult.hasMatchedOption("--old-option")).thenReturn(false);
232+
when(parseResult.hasMatchedOption("--new-option")).thenReturn(false);
233+
when(parseResult.hasMatchedOption("-n")).thenReturn(true);
234+
235+
// Act & Assert - No exception should be thrown
236+
assertDoesNotThrow(
237+
() ->
238+
CommandLineInputUtils.validateDeprecatedOptionPair(
239+
commandLine, "--old-option", "--new-option", "-n"));
240+
}
241+
242+
@Test
243+
public void validateDeprecatedOptionPair_neitherSpecified_shouldNotThrowException() {
244+
// Arrange
245+
CommandLine commandLine = mock(CommandLine.class);
246+
CommandLine.ParseResult parseResult = mock(CommandLine.ParseResult.class);
247+
when(commandLine.getParseResult()).thenReturn(parseResult);
248+
when(parseResult.hasMatchedOption("--old-option")).thenReturn(false);
249+
when(parseResult.hasMatchedOption("--new-option")).thenReturn(false);
250+
when(parseResult.hasMatchedOption("-n")).thenReturn(false);
251+
252+
// Act & Assert - No exception should be thrown
253+
assertDoesNotThrow(
254+
() ->
255+
CommandLineInputUtils.validateDeprecatedOptionPair(
256+
commandLine, "--old-option", "--new-option", "-n"));
257+
}
258+
259+
@Test
260+
public void validateDeprecatedOptionPair_bothSpecified_shouldThrowException() {
261+
// Arrange
262+
CommandLine commandLine = mock(CommandLine.class);
263+
CommandLine.ParseResult parseResult = mock(CommandLine.ParseResult.class);
264+
when(commandLine.getParseResult()).thenReturn(parseResult);
265+
when(parseResult.hasMatchedOption("--old-option")).thenReturn(true);
266+
when(parseResult.hasMatchedOption("--new-option")).thenReturn(true);
267+
when(parseResult.hasMatchedOption("-n")).thenReturn(false);
268+
269+
// Act & Assert
270+
CommandLine.ParameterException exception =
271+
assertThrows(
272+
CommandLine.ParameterException.class,
273+
() ->
274+
CommandLineInputUtils.validateDeprecatedOptionPair(
275+
commandLine, "--old-option", "--new-option", "-n"));
276+
277+
// Verify the exception message contains the error message
278+
assertTrue(
279+
exception
280+
.getMessage()
281+
.contains(
282+
DataLoaderError.DEPRECATED_AND_NEW_OPTION_BOTH_SPECIFIED.buildMessage(
283+
"--old-option", "--new-option", "--new-option")));
284+
}
285+
286+
@Test
287+
public void
288+
validateDeprecatedOptionPair_deprecatedAndNewShortFormSpecified_shouldThrowException() {
289+
// Arrange
290+
CommandLine commandLine = mock(CommandLine.class);
291+
CommandLine.ParseResult parseResult = mock(CommandLine.ParseResult.class);
292+
when(commandLine.getParseResult()).thenReturn(parseResult);
293+
when(parseResult.hasMatchedOption("--old-option")).thenReturn(true);
294+
when(parseResult.hasMatchedOption("--new-option")).thenReturn(false);
295+
when(parseResult.hasMatchedOption("-n")).thenReturn(true);
296+
297+
// Act & Assert
298+
CommandLine.ParameterException exception =
299+
assertThrows(
300+
CommandLine.ParameterException.class,
301+
() ->
302+
CommandLineInputUtils.validateDeprecatedOptionPair(
303+
commandLine, "--old-option", "--new-option", "-n"));
304+
305+
// Verify the exception message contains the error message
306+
assertTrue(
307+
exception
308+
.getMessage()
309+
.contains(
310+
DataLoaderError.DEPRECATED_AND_NEW_OPTION_BOTH_SPECIFIED.buildMessage(
311+
"--old-option", "--new-option", "--new-option")));
312+
}
188313
}

0 commit comments

Comments
 (0)