apache / apache/cloudstack

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

未关闭
#13,893 3 条评论 0 个 reaction 已指派 1 人 已被 @DaanHoogland 认领 在 GitHub 查看
type:technical-debt
主要语言
Java
星标
3.1k
派生
1.4k
平均合并
6 天 19 小时
30 天内合并 PR
32

描述

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.

贡献指南

打开贡献指南

评估

这个 Issue 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。