apache / apache/cloudstack

remove redundant/diverging hardcoded defaults in parseInt/parseLong wrapping ConfigKey reads

Ouverte
#13,893 3 commentaires 0 réactions 1 personne assignée Réclamée par @DaanHoogland Voir sur GitHub
type:technical-debt
Langage dominant
Java
Étoiles
3.1k
Forks
1.4k
Merge moyen
6 j 19 h
PR mergées (30 j)
32

Description

Many call sites read a `ConfigKey` via the raw DAO and wrap the result in `NumbersUtil.parseInt(value, someLiteral)` / `NumbersUtil.parseLong(...)` / `Integer.parseInt(...)`, supplying a hardcoded fallback even though the `ConfigKey` already carries its own default via `.defaultValue()` (and `.value()` applies that default automatically when no DB row exists). Most of the time the literal happens to match the `ConfigKey`'s declared default, so it's just redundant. But it doesn't always match — which is a latent bug, not just noise, since the two numbers silently diverge depending on which code path executes.

Found during the `.value()` migration pass (issue #10752):

- `DeploymentPlanningManagerImpl.java` — `HostReservationReleasePeriod`: `ConfigKey` default is `"300000"`, but the field's Java-level initializer is `60L * 60L * 1000L` (3,600,000), and the code only falls back to the `ConfigKey` default when the persisted value is `<= 0` — not when the row is simply missing (null), in which case it silently keeps 3,600,000 instead of 300,000.
- `StorageCacheManagerImpl.java` — `ExpungeWorkers`: `ConfigKey` default is `"1"`, but the read is `NumbersUtil.parseInt(configDao.getValue(...), 10)` — a hardcoded fallback of 10 vs. the registered default of 1.
- `UcsManagerImpl.java` — `UCSSyncBladeInterval`: `ConfigKey` default is `"3600"`, but a leftover `catch (NumberFormatException e) { syncBladeInterval = 600; }` uses 600. Low risk in practice since `.value()` no longer throws on parse, but the mismatched literal is still latent debt.

These three were left un-migrated to `.value()` specifically because of this discrepancy — picking either number without a decision would silently change fresh-install/edge-case behavior. Worth a repo-wide sweep for the same `parseInt`/`parseLong`-with-hardcoded-default pattern, since these three were only found incidentally while migrating unrelated call sites, not through an exhaustive search.

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.