-
Notifications
You must be signed in to change notification settings - Fork 1
[SPLIT] assertResourceAvailable() + buildResourceUrl() DRY violation (87 methods) → #158 + #159 + #160 #145
Description
⚠️ This issue has been split into three parts for safe execution
Per the Phase 7 Scope & Split Assessment, this issue requires rewriting all 87 public method bodies in a 2,490-line file. 87 method rewrites in a single pass is the highest-risk item in the Phase 7 plan. The methods group naturally by resource type.
Split Issues
| Part | Issue | Scope |
|---|---|---|
| Part 1 | #158 | Create build() helper + rewrite Systems (16) + Deployments (9) + Procedures (8) = 33 methods |
| Part 2 | #159 | Rewrite SamplingFeatures (8) + Properties (6) + Datastreams (11) = 25 methods |
| Part 3 | #160 | Rewrite Observations (8) + ControlStreams (11) + Commands (10) = 29 methods — also resolves #111 |
Execution Order
- #100 Part 1: Remove
assertResourceAvailablefrom per-ID methods — Systems, Deployments, Procedures, SamplingFeatures (33 methods) #156, #100 Part 2: RemoveassertResourceAvailablefrom per-ID methods — Properties, Datastreams, Observations, ControlStreams, Commands (39 methods) #157 ([SPLIT] assertResourceAvailable() overly strict for per-ID methods → #156 + #157 #100 Parts 1 & 2) — remove asserts from per-ID methods FIRST - #145 Part 1: Create
build()helper + rewrite Systems, Deployments, Procedures (33 methods) #158 (Part 1) — createbuild()helper + first batch - #145 Part 2: Rewrite SamplingFeatures, Properties, Datastreams to
build()(25 methods) #159 (Part 2) — second batch - #145 Part 3: Rewrite Observations, ControlStreams, Commands to
build()(29 methods) — resolves #111 #160 (Part 3) — final batch + DEFERRED —getCommandStatus()uses string concatenation instead ofbuildResourceUrl()for query string (F45) #111 auto-resolution
This parent issue should be closed when #158, #159, and #160 are all resolved.
Original issue body (preserved for reference)
Finding
Every one of the 90 public URL-building methods in CSAPIQueryBuilder repeats the exact same two-line guard-then-build pattern. The only variation across all 90 methods is the resource type string, optional ID, optional sub-path, and optional options — no method adds logic between the guard and the URL construction.
Review Source: Senior developer code review of clean-pr — docs/code-review/009-pending-p2-assert-resource-paired-pattern.md
Severity: P2-Important
Category: Code Quality / DRY / Architecture
Ownership: Ours
Problem Statement
CSAPIQueryBuilder in url_builder.ts (2,490 lines) contains 90 public methods that all follow an identical two-line pattern:
this.assertResourceAvailable('resourceType');
return this.buildResourceUrl('resourceType', id, subPath, options);This is repeated with zero variation in logic. The file is nearly 2,500 lines long, but the actual URL-building logic lives entirely in buildResourceUrl() (~20 lines) and buildQueryString() (~40 lines). The remaining ~2,000 lines are 90 copies of the same two-line pattern.
Exception: getCommandStatus() concatenates buildQueryString(options) separately rather than passing options through buildResourceUrl() — tracked in issue #111 and resolved by Part 3 (#160).