apache / apache/cloudstack

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

Đang mở
#13,893 3 bình luận 0 reaction 1 người được giao Được @DaanHoogland nhận Xem trên GitHub
type:technical-debt
Ngôn ngữ chính
Java
Star
3.1k
Fork
1.4k
Merge trung bình
6 ngày 19 giờ
Pull request đã merge (30 ngày)
32

Mô tả

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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.