Skip to content

Commit c4cb0e1

Browse files
author
vividcoder
committed
fix(config): write CLI storage params directly to Storage object
applyCLIParams() wrote 7 storage-related values to intermediate fields on CommonParameter instead of the Storage object that is actually used at runtime. Since applyConfigParams() runs first, these CLI values were silently ignored. - Remove 7 intermediate fields from CommonParameter - Simplify applyConfigParams() to read directly from config - Fix applyCLIParams() to write directly to PARAMETER.storage - Add tests for CLI-overrides-config and config-defaults scenarios
1 parent 8cdda54 commit c4cb0e1

3 files changed

Lines changed: 68 additions & 59 deletions

File tree

common/src/main/java/org/tron/common/parameter/CommonParameter.java

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -76,27 +76,6 @@ public class CommonParameter {
7676
@Getter
7777
public List<String> seedNodes = new ArrayList<>();
7878
@Getter
79-
@Setter
80-
public String storageDbDirectory = "";
81-
@Getter
82-
@Setter
83-
public String storageDbEngine = "";
84-
@Getter
85-
@Setter
86-
public String storageDbSynchronous = "";
87-
@Getter
88-
@Setter
89-
public String contractParseEnable = "";
90-
@Getter
91-
@Setter
92-
public String storageIndexDirectory = "";
93-
@Getter
94-
@Setter
95-
public String storageIndexSwitch = "";
96-
@Getter
97-
@Setter
98-
public String storageTransactionHistorySwitch = "";
99-
@Getter
10079
public boolean fastForward = false;
10180
// ── Network / P2P ─────────────────────────────
10281
@Getter

framework/src/main/java/org/tron/core/config/args/Args.java

Lines changed: 15 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -225,37 +225,14 @@ public static void applyConfigParams(
225225

226226
PARAMETER.storage = new Storage();
227227

228-
PARAMETER.storage.setDbEngine(Optional.ofNullable(PARAMETER.storageDbEngine)
229-
.filter(StringUtils::isNotEmpty)
230-
.orElse(Storage.getDbEngineFromConfig(config)));
231-
232-
PARAMETER.storage.setDbSync(Optional.ofNullable(PARAMETER.storageDbSynchronous)
233-
.filter(StringUtils::isNotEmpty)
234-
.map(Boolean::valueOf)
235-
.orElse(Storage.getDbVersionSyncFromConfig(config)));
236-
237-
PARAMETER.storage.setContractParseSwitch(Optional.ofNullable(PARAMETER.contractParseEnable)
238-
.filter(StringUtils::isNotEmpty)
239-
.map(Boolean::valueOf)
240-
.orElse(Storage.getContractParseSwitchFromConfig(config)));
241-
242-
PARAMETER.storage.setDbDirectory(Optional.ofNullable(PARAMETER.storageDbDirectory)
243-
.filter(StringUtils::isNotEmpty)
244-
.orElse(Storage.getDbDirectoryFromConfig(config)));
245-
246-
PARAMETER.storage.setIndexDirectory(Optional.ofNullable(PARAMETER.storageIndexDirectory)
247-
.filter(StringUtils::isNotEmpty)
248-
.orElse(Storage.getIndexDirectoryFromConfig(config)));
249-
250-
PARAMETER.storage.setIndexSwitch(Optional.ofNullable(PARAMETER.storageIndexSwitch)
251-
.filter(StringUtils::isNotEmpty)
252-
.orElse(Storage.getIndexSwitchFromConfig(config)));
253-
254-
PARAMETER.storage
255-
.setTransactionHistorySwitch(
256-
Optional.ofNullable(PARAMETER.storageTransactionHistorySwitch)
257-
.filter(StringUtils::isNotEmpty)
258-
.orElse(Storage.getTransactionHistorySwitchFromConfig(config)));
228+
PARAMETER.storage.setDbEngine(Storage.getDbEngineFromConfig(config));
229+
PARAMETER.storage.setDbSync(Storage.getDbVersionSyncFromConfig(config));
230+
PARAMETER.storage.setContractParseSwitch(Storage.getContractParseSwitchFromConfig(config));
231+
PARAMETER.storage.setDbDirectory(Storage.getDbDirectoryFromConfig(config));
232+
PARAMETER.storage.setIndexDirectory(Storage.getIndexDirectoryFromConfig(config));
233+
PARAMETER.storage.setIndexSwitch(Storage.getIndexSwitchFromConfig(config));
234+
PARAMETER.storage.setTransactionHistorySwitch(
235+
Storage.getTransactionHistorySwitchFromConfig(config));
259236

260237
PARAMETER.storage
261238
.setCheckpointVersion(Storage.getCheckpointVersionFromConfig(config));
@@ -1057,25 +1034,25 @@ private static void applyCLIParams(CLIParameter cmd, JCommander jc) {
10571034
PARAMETER.maxHttpConnectNumber = cmd.maxHttpConnectNumber;
10581035
}
10591036
if (assigned.containsKey("--storage-db-directory")) {
1060-
PARAMETER.storageDbDirectory = cmd.storageDbDirectory;
1037+
PARAMETER.storage.setDbDirectory(cmd.storageDbDirectory);
10611038
}
10621039
if (assigned.containsKey("--storage-db-engine")) {
1063-
PARAMETER.storageDbEngine = cmd.storageDbEngine;
1040+
PARAMETER.storage.setDbEngine(cmd.storageDbEngine);
10641041
}
10651042
if (assigned.containsKey("--storage-db-synchronous")) {
1066-
PARAMETER.storageDbSynchronous = cmd.storageDbSynchronous;
1043+
PARAMETER.storage.setDbSync(Boolean.valueOf(cmd.storageDbSynchronous));
10671044
}
10681045
if (assigned.containsKey("--contract-parse-enable")) {
1069-
PARAMETER.contractParseEnable = cmd.contractParseEnable;
1046+
PARAMETER.storage.setContractParseSwitch(Boolean.valueOf(cmd.contractParseEnable));
10701047
}
10711048
if (assigned.containsKey("--storage-index-directory")) {
1072-
PARAMETER.storageIndexDirectory = cmd.storageIndexDirectory;
1049+
PARAMETER.storage.setIndexDirectory(cmd.storageIndexDirectory);
10731050
}
10741051
if (assigned.containsKey("--storage-index-switch")) {
1075-
PARAMETER.storageIndexSwitch = cmd.storageIndexSwitch;
1052+
PARAMETER.storage.setIndexSwitch(cmd.storageIndexSwitch);
10761053
}
10771054
if (assigned.containsKey("--storage-transactionHistory-switch")) {
1078-
PARAMETER.storageTransactionHistorySwitch = cmd.storageTransactionHistorySwitch;
1055+
PARAMETER.storage.setTransactionHistorySwitch(cmd.storageTransactionHistorySwitch);
10791056
}
10801057
if (assigned.containsKey("--fast-forward")) {
10811058
PARAMETER.fastForward = cmd.fastForward;

framework/src/test/java/org/tron/core/config/args/ArgsTest.java

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,5 +282,58 @@ public void testInitService() {
282282

283283
Args.clearParam();
284284
}
285+
286+
/**
287+
* Verify that CLI storage parameters correctly override config file values.
288+
*
289+
* <p>config-test.conf defines: db.directory = "database", db.engine = "LEVELDB".
290+
* When CLI passes different values (e.g. --storage-db-directory cli-db-dir),
291+
* the Storage object should reflect the CLI values, not the config file values.
292+
*
293+
* <p>This ensures the three-layer override chain works:
294+
* Storage defaults -> config file -> CLI arguments.
295+
*/
296+
@Test
297+
public void testCliOverridesStorageConfig() {
298+
Args.setParam(new String[] {
299+
"--storage-db-directory", "cli-db-dir",
300+
"--storage-db-engine", "ROCKSDB",
301+
"--storage-db-synchronous", "true",
302+
"--storage-index-directory", "cli-index-dir",
303+
"--storage-index-switch", "cli-index-switch",
304+
"--storage-transactionHistory-switch", "off",
305+
"--contract-parse-enable", "false"
306+
}, TestConstants.TEST_CONF);
307+
308+
CommonParameter parameter = Args.getInstance();
309+
310+
Assert.assertEquals("cli-db-dir", parameter.getStorage().getDbDirectory());
311+
Assert.assertEquals("ROCKSDB", parameter.getStorage().getDbEngine());
312+
Assert.assertTrue(parameter.getStorage().isDbSync());
313+
Assert.assertEquals("cli-index-dir", parameter.getStorage().getIndexDirectory());
314+
Assert.assertEquals("cli-index-switch", parameter.getStorage().getIndexSwitch());
315+
Assert.assertEquals("off", parameter.getStorage().getTransactionHistorySwitch());
316+
Assert.assertFalse(parameter.getStorage().isContractParseSwitch());
317+
318+
Args.clearParam();
319+
}
320+
321+
/**
322+
* Verify that config file storage values are applied when no CLI override is present.
323+
*
324+
* <p>config-test.conf defines: db.directory = "database", db.engine = "LEVELDB".
325+
* Without any CLI storage arguments, the Storage object should use these config values.
326+
*/
327+
@Test
328+
public void testConfigStorageDefaults() {
329+
Args.setParam(new String[] {}, TestConstants.TEST_CONF);
330+
331+
CommonParameter parameter = Args.getInstance();
332+
333+
Assert.assertEquals("database", parameter.getStorage().getDbDirectory());
334+
Assert.assertEquals("LEVELDB", parameter.getStorage().getDbEngine());
335+
336+
Args.clearParam();
337+
}
285338
}
286339

0 commit comments

Comments
 (0)