ConfigKeys left unused while their read site still does raw _configDao.getConfiguration() map lookups

Abierto

#13,898 abierto el 17/8/2026 2 comentarios 0 reacciones 1 asignado Ver en GitHub

@DaanHoogland ya está trabajando en esto.

Desde el 18/8/2026.

Evaluación

Este issue todavía no se ha evaluado.

Descripción

type:technical-debt

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 ConfigKeys:

_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<String,String> _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.

Lenguaje dominante: Java
Estrellas: 3.1k
Forks: 1.4k
Merge medio: 6 d 20 h
PR fusionados (30 d): 27