Skip to content

Commit

Permalink
Fix npe from subcommands (#4954)
Browse files Browse the repository at this point in the history
* Add getDefaultSyncModeIfNotSet and clean up method

* Add unit test to prevent a new NPE

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
  • Loading branch information
gfukushima authored Jan 18, 2023
1 parent f0c2380 commit 3e8cf77
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
11 changes: 6 additions & 5 deletions besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -2005,13 +2005,13 @@ private void issueOptionWarnings() {
CommandLineUtils.failIfOptionDoesntMeetRequirement(
commandLine,
"--fast-sync-min-peers can't be used with FULL sync-mode",
!SyncMode.isFullSync(getDefaultSyncModeIfNotSet(syncMode)),
!SyncMode.isFullSync(getDefaultSyncModeIfNotSet()),
singletonList("--fast-sync-min-peers"));

CommandLineUtils.failIfOptionDoesntMeetRequirement(
commandLine,
"--Xcheckpoint-post-merge-enabled can only be used with X_CHECKPOINT sync-mode",
SyncMode.X_CHECKPOINT.equals(getDefaultSyncModeIfNotSet(syncMode)),
SyncMode.X_CHECKPOINT.equals(getDefaultSyncModeIfNotSet()),
singletonList("--Xcheckpoint-post-merge-enabled"));

if (!securityModuleName.equals(DEFAULT_SECURITY_MODULE)
Expand All @@ -2033,7 +2033,7 @@ private void issueOptionWarnings() {
private void configure() throws Exception {
checkPortClash();
checkIfRequiredPortsAreAvailable();
syncMode = getDefaultSyncModeIfNotSet(syncMode);
syncMode = getDefaultSyncModeIfNotSet();

ethNetworkConfig = updateNetworkConfig(network);

Expand Down Expand Up @@ -2167,7 +2167,8 @@ public BesuController buildController() {
public BesuControllerBuilder getControllerBuilder() {
final KeyValueStorageProvider storageProvider = keyValueStorageProvider(keyValueStorageName);
return controllerBuilderFactory
.fromEthNetworkConfig(updateNetworkConfig(network), genesisConfigOverrides, syncMode)
.fromEthNetworkConfig(
updateNetworkConfig(network), genesisConfigOverrides, getDefaultSyncModeIfNotSet())
.synchronizerConfiguration(buildSyncConfig())
.ethProtocolConfiguration(unstableEthProtocolOptions.toDomainObject())
.dataDirectory(dataDir())
Expand Down Expand Up @@ -3436,7 +3437,7 @@ public static List<String> getJDKEnabledProtocols() {
}
}

private SyncMode getDefaultSyncModeIfNotSet(final SyncMode syncMode) {
private SyncMode getDefaultSyncModeIfNotSet() {
return Optional.ofNullable(syncMode)
.orElse(
genesisFile == null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.isNotNull;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -487,4 +488,13 @@ private void createDbDirectory(final boolean createDataFiles) throws IOException
assertThat(success).isTrue();
}
}

@Test
public void blocksImportWithNoSyncModeDoesNotRaiseNPE() throws IOException {
final File fileToImport = temp.newFile("blocks.file");
parseCommand(
BLOCK_SUBCOMMAND_NAME, BLOCK_IMPORT_SUBCOMMAND_NAME, "--from", fileToImport.getPath());

verify(mockControllerBuilderFactory).fromEthNetworkConfig(any(), any(), isNotNull());
}
}

0 comments on commit 3e8cf77

Please sign in to comment.