-
Notifications
You must be signed in to change notification settings - Fork 1
#145 Part 1: Create build() helper + rewrite Systems, Deployments, Procedures (33 methods) #158
Description
Parent Issue
Split from #145 — assertResourceAvailable() + buildResourceUrl() two-line pattern repeated 90 times in url_builder.ts — DRY violation.
This is Part 1 of 3. See also: Part 2 and Part 3 (issues to follow).
Scope — Part 1 Only
Create the private build() helper method and rewrite Systems (16) + Deployments (9) + Procedures (8) = 33 methods to delegate to it.
Context: State of the File After Dependencies
This issue is sequenced after #156 and #157 (#100 Parts 1 & 2), which remove assertResourceAvailable() from 72 per-ID methods. After those are done, the file has two patterns:
Pattern A — 15 list/create methods (guard retained):
getSystems(options?: SystemQueryOptions): string {
this.assertResourceAvailable('systems');
return this.buildResourceUrl('systems', undefined, undefined, options);
}Pattern B — 72 per-ID methods (guard removed by #100):
getSystem(id: string, options?: QueryOptions): string {
return this.buildResourceUrl('systems', id, undefined, options);
}The build() helper must handle both patterns. The recommended approach: build() always calls assertResourceAvailable(), but per-ID methods simply bypass build() and continue calling buildResourceUrl() directly. Only the 15 list/create methods need build().
Alternative: build() takes an optional guard: boolean parameter, or we create two helpers (buildGuarded() for list/create, and the existing buildResourceUrl() for per-ID). The implementer should choose the cleanest approach.
Implementation
Step 1 — Create the build() helper:
/**
* Guards resource availability and constructs the URL in one step.
* Used by list/create methods that require the resource to be discovered.
*
* @param resourceType - The resource type key (e.g., 'systems', 'datastreams').
* @param id - Optional resource ID.
* @param subPath - Optional sub-path (e.g., 'schema', 'observations').
* @param options - Optional query parameters.
* @returns Fully constructed URL string.
* @throws {EndpointError} If the resource type is not available.
*/
private build(
resourceType: string,
id?: string,
subPath?: string,
options?: QueryOptions
): string {
this.assertResourceAvailable(resourceType);
return this.buildResourceUrl(resourceType, id, subPath, options);
}Step 2 — Rewrite 33 methods (3 resource types):
For list/create methods (guard needed):
// Before:
getSystems(options?: SystemQueryOptions): string {
this.assertResourceAvailable('systems');
return this.buildResourceUrl('systems', undefined, undefined, options);
}
// After:
getSystems(options?: SystemQueryOptions): string {
return this.build('systems', undefined, undefined, options);
}For per-ID methods (guard already removed by #100):
// Before:
getSystem(id: string, options?: QueryOptions): string {
return this.buildResourceUrl('systems', id, undefined, options);
}
// After:
getSystem(id: string, options?: QueryOptions): string {
return this.build('systems', id, undefined, options);
}Wait — design decision needed here. If build() always calls assertResourceAvailable(), then wrapping per-ID methods in build() would RE-INTRODUCE the guard that #100 removed. Two approaches:
- Approach A:
build()only used by list/create methods. Per-ID methods stay asbuildResourceUrl()calls unchanged. This means Part 1 only touches ~6 list/create methods for these 3 types + createsbuild(). The 27 per-ID methods stay as-is. - Approach B:
build()takes an optionalskipGuardflag. Per-ID methods passskipGuard: true. - Approach C: Per-ID methods also go through
build(), butbuild()conditionally guards based on whetheridis present (noid= collection method = guard needed;idpresent = per-ID = skip guard).
Recommended: Approach C — build() conditionally guards: if no id is passed, it calls assertResourceAvailable(); if id is present, it skips the guard. This makes ALL methods delegate to build() uniformly, and the guard logic lives in one place. This is the cleanest DRY outcome.
private build(
resourceType: string,
id?: string,
subPath?: string,
options?: QueryOptions
): string {
if (!id) {
this.assertResourceAvailable(resourceType);
}
return this.buildResourceUrl(resourceType, id, subPath, options);
}With this design, all 33 methods in scope become return this.build(...) one-liners.
Methods to Rewrite (33)
Systems (16): getSystems, getSystem, createSystem, updateSystem, deleteSystem, getSystemDataStreams, getSystemControlStreams, getSystemDeployments, getSystemSamplingFeatures, getSystemProcedure, getSystemHistory, getSystemDetails, plus any other system-related methods.
Deployments (9): getDeployments, getDeployment, createDeployment, updateDeployment, deleteDeployment, getDeploymentSubdeployments, getDeploymentSystems, plus others.
Procedures (8): getProcedures, getProcedure, createProcedure, updateProcedure, deleteProcedure, getProcedureSystems, getProcedureDataStreams, plus others.
File to Modify
| File | Action |
|---|---|
src/ogc-api/csapi/url_builder.ts |
Add build() helper + rewrite 33 methods |
What NOT to Touch
- ❌ SamplingFeatures, Properties, Datastreams, Observations, ControlStreams, Commands — Parts 2 and 3
- ❌ Public method signatures — names, parameters, return types must remain identical
- ❌ JSDoc on public methods — preserve all documentation
- ❌
assertResourceAvailable()orbuildResourceUrl()definitions - ❌ Test files — existing tests must pass unchanged
- ❌ Any file outside
url_builder.ts
Acceptance Criteria (Part 1)
- Private
build()method added toCSAPIQueryBuilder - All Systems methods (16) delegate to
build() - All Deployments methods (9) delegate to
build() - All Procedures methods (8) delegate to
build() - No direct
this.assertResourceAvailable()calls remain in these 33 methods - Guard behavior preserved: list/create methods still throw when resource not available
- Per-ID methods still work without guard (consistent with [SPLIT] assertResourceAvailable() overly strict for per-ID methods → #156 + #157 #100 fix)
-
tsc --noEmit— zero errors -
npm test— all tests pass -
npm run lint— zero errors
Dependencies
Blocked by: #156, #157 (#100 Parts 1 & 2 — assert removal must be done first)
Blocks: Part 2, Part 3
References
- [SPLIT]
assertResourceAvailable()+buildResourceUrl()DRY violation (87 methods) → #158 + #159 + #160 #145 — parent issue with full analysis and all 38 references - #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 split (assert removal) — must complete before this - DEFERRED —
getCommandStatus()uses string concatenation instead ofbuildResourceUrl()for query string (F45) #111 —getCommandStatus()concatenation — resolved in Part 3 url_builder.tslines 256–273 (buildResourceUrl), 355–365 (assertResourceAvailable)- Phase 7 Scope & Split Assessment
Operational Constraints
⚠️ MANDATORY: Before starting work on this issue, reviewdocs/governance/AI_OPERATIONAL_CONSTRAINTS.md.
Upstream Isolation Constraint
Per the upstream maintainer's comment on PR #136: url_builder.ts is within src/ogc-api/csapi/ — purely internal to the isolated CSAPI module.