ConfigKeys left unused while their read site still does raw _configDao.getConfiguration() map lookups
- Langage dominant
- Java
- Étoiles
- 3.1k
- Forks
- 1.4k
- Merge moyen
- 6 j 19 h
- PR mergées (30 j)
- 32
Description
Found while writing ConfigKey-wiring unit tests for PR #13884 (issue #10752, phase out enum `Config`).
`ManagementServerImpl.configure()` (server/src/main/java/com/cloud/server/ManagementServerImpl.java:1141-1148) still reads two settings through the raw configuration map instead of their migrated `ConfigKey`s:
```java
_configs = _configDao.getConfiguration();
final String value = _configs.get("event.purge.interval");
final int cleanup = NumbersUtil.parseInt(value, 60 * 60 * 24); // 1 day.
_purgeDelay = NumbersUtil.parseInt(_configs.get("event.purge.delay"), 0);
```
`ManagementServer.EventPurgeInterval` and `ManagementServer.EventPurgeDelay` both already exist on the interface — three lines below, `AlertPurgeInterval`/`AlertPurgeDelay` (the exact same purge-scheduling pattern) were correctly migrated to `.value()` in the same method, so these two are a clear miss rather than a deliberate choice.
This is a different variant from the pattern in #13893 (redundant/diverging hardcoded defaults passed to `parseInt`/`parseLong` around an already-migrated `.value()` call): here the read site was never migrated to `.value()` (or even to `_configDao.getValue(key.key())`) at all — it still goes through a raw `Map _configs = _configDao.getConfiguration()` lookup by literal key string, bypassing the `ConfigKey` entirely. The earlier `.value()`-replacement sweep only searched for the `_configDao.getValue(key.key())` call shape, so it structurally couldn't catch this one.
It also happens to carry the same kind of default-mismatch risk as #13893: `EventPurgeDelay`'s registered default is `"15"`, but the hardcoded fallback here is `0`, and `0` is the sentinel that disables purging (`if (_purgeDelay != 0) { schedule... }`). So migrating this site isn't a pure no-op — it needs the same default-reconciliation judgment call as the #13893 cases before switching it over.
Only these two sites were found in `ManagementServerImpl`; this was spotted incidentally while scoping unit tests for the `.value()` migration, not through an exhaustive sweep — worth a repo-wide grep for other `_configDao.getConfiguration()`-backed raw map reads (`_configs.get("literal.key")`) that have a matching `ConfigKey` sitting unused nearby.
Guide de contribution
Ouvrir le guide de contribution
Évaluation
Cette issue n'a pas encore été évaluée.