Skip to content

Consolidate tile-URL building into shared tileUrlUtils - #213

Open
sandesh-sp wants to merge 9 commits into
developmentfrom
feat/tile-url-consolidation
Open

Consolidate tile-URL building into shared tileUrlUtils#213
sandesh-sp wants to merge 9 commits into
developmentfrom
feat/tile-url-consolidation

Conversation

@sandesh-sp

@sandesh-sp sandesh-sp commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Consolidates tile-URL construction into shared modules used by every rendering path — the Leaflet per-tile middleware, the DeckGL static URL builder, and the TimeControl time-change reload — so all engines apply the same source resolution, time formatting, and STAC/COG/TMS parameter logic instead of each maintaining its own copy.

This replaces the narrower cogUrlUtils helper and removes duplicated substitution logic that had drifted between paths.

What changed

  • New tileUrlUtils with a clear two-stage contract:
    • buildTileUrlOptions — formats a layer's time values once and resolves tms → tileFormat.
    • compileTileUrl — a pure substituter: replaces {time} / {starttime} / {endtime} / {customtime.N}, then injects datetime= and the STAC/COG/TMS params. It never re-formats time values.
  • New tileLayerSourceresolveTileLayerSource turns a layer config into its base URL (active tile level → L_.getUrlstac-collection: / COG: / titiler-url: handling). Layer creation and time-driven reload both call it, so they cannot resolve to different sources.
  • Leaflet middleware now calls compileTileUrl per tile from getTileUrl; the WMS path keeps its own substitution but shares the formatted time values. refresh() re-applies the two creation-time URL transformations ({t} rewriting, the WMS base/params split) that a plain _url assignment would skip.
  • DeckGL path compiles a complete URL once at layer creation and rebuilds it on time change.
  • TimeControl reload resolves the URL through resolveTileLayerSource, in the same order creation does, and pushes the result into the active engine.
  • Removed cogUrlUtils and its spec; added specs for tileUrlUtils, tileLayerSource, and the Leaflet middleware.
  • Docs: adds src/essence/Basics/Layers_/tile-url-pipeline.md describing the full pipeline, the three call sites, and the WMS/globe exceptions.

Why

Four real defects, all previously silent:

  • DeckGL threw on every time change. TimeControl.reloadLayer called refresh() unconditionally and wrote l.options.time on layers that have neither — a deck layer has props, not options. Time changes on the deck engine now work rather than throwing.
  • Creation-time options carried raw, unformatted times. Map_.makeTileLayer set time: layerObj.time.end straight from config — including a literal undefined in the URL when unset — until the first time change replaced them. They are now formatted from the moment the layer is created.
  • A bogus datetime=/ was appended for STAC/COG/titiler layers with no time configured (the old guard was != null, which treats '' as a real value). Now omitted. This is a server-visible URL change, and an improvement.
  • Unparseable dates produced d3's 0NaN-NaN-… in the URL. They now yield an empty string.

Beyond the fixes, one source of truth for source resolution, time formatting, and param injection across Leaflet, DeckGL, and WMS — which also aligns with the engine-agnostic-core direction of moving apply/refresh behind the engine-adapter boundary.

Design intent, not a bug fix: compileTileUrl is a pure substituter and must stay one. Leaflet calls it per tile, so formatting there would re-bite on every tile fetch and shift dates across timezones, while DeckGL's single URL bake would hide it. (An earlier version of this description claimed the base code had such a double-format bug. It did not — the old per-tile path substituted this.options verbatim. The invariant is a going-forward guarantee.)

Notes

Testing

  • tests/unit/tileUrlUtils.spec.js — the formatting invariant, empty/unparseable time handling, STAC/COG/TMS param injection, the TMS de-duplication branches, the global mosaic limits, the closed key set, and placeholders inside query strings.
  • tests/unit/tileLayerSource.spec.js — tile-level selection, the service-prefix branches, and elevation resolution.
  • tests/unit/leafletTileLayerMiddleware.spec.jsrefresh()'s {t} rewriting and WMS base/params split, against a minimal Leaflet stub.

🤖 Generated with Claude Code

https://claude.ai/code/session_011ZkGkzkgvKXv9p8uiP5hwp

sandesh-sp and others added 3 commits July 16, 2026 15:41
Rename cogUrlUtils -> tileUrlUtils and grow it into compileTileUrl(),
shared by the Leaflet per-tile middleware and the DeckGL URL builder so
time placeholders and STAC/COG params resolve identically per engine.
@github-actions

Copy link
Copy Markdown

🤖 Version Auto-Bumped

The version has been automatically incremented to 4.2.15-20260722

This commit was added to your PR branch. When you merge this PR, the new version will be included.


If you want a different version, update package.json manually and push to this PR.

@sandesh-sp

Copy link
Copy Markdown
Collaborator Author

#: 1 🔴
Bug: DeckGL path never restores layer.url
What was missed: Deck branch did return true mid-function (TimeControl.js:299), exiting before the layer.url = originalUrl restore at the tail. Leaflet fell through and restored; deck
didn't. Since performTimeUrlReplacements isn't idempotent, {key} placeholders were consumed permanently (values frozen after the first time step) and nocache= accumulated forever.
Fix as landed: Removed the early return; deck falls through to the shared tail. Comment records why it must not early-return.
────────────────────────────────────────
#: 2 🟡
Bug: Entire layer config leaked into this.options
What was missed: buildTileUrlOptions returned {...layerObj}, passed whole to refresh(), which copies every key onto this.options — raw minZoom/maxZoom overwrote the parseInt'd creation
values, surviving only on JS coercion.
Fix as landed: Closed key set at the source: dropped the ...layerObj spread; buildTileUrlOptions now returns only tile-URL keys, with the nine COG fields listed explicitly.
Map_.makeTileLayer spreads that same object into creation options (Leaflet-only options follow, so they win on overlap), so creation and refresh can't diverge. Passing it whole to
refresh() is now safe by construction.
────────────────────────────────────────
#: 3 🟡
Bug: Empty-time placeholders left literal
What was missed: New compileTileUrl guarded each replace with if (timeStr), leaving a literal {time} → guaranteed 404. Old middleware substituted '' unconditionally.
Fix as landed: .replace(/{time}/g, timeStr || '') in step 2. Step 1's datetime= empty-skip left alone — correct there.
────────────────────────────────────────
#: 4 🟡
Bug: setLayerWmsParams manufactured options on deck layers
What was missed: if (!l.options) l.options = {} grew an options bag on foreign DeckGL objects (which use props), duplicating the Leaflet refresh merge.
Fix as landed: Guard on l.options != null — skips deck rather than growing it an options nothing reads.
────────────────────────────────────────
#: 5 🟢
Bug: Minors
What was missed: 'deckgl' string literal; _syncLayers comment claimed cloning removed in e44f649; processExpression untyped in a .ts file; docs asserted compileTileUrl re-formatted in
"old code" though it's new in this PR.
Fix as landed: MAP_ENGINE.DECKGL (imported from MapEngines/types/engine); comment reworded to the array-copy reality; (expression: string): string; docs reframed as a forward-looking
invariant, plus line-refs refreshed and a "closed key set" section added.

buildTileUrlOptions now returns a closed set of tile-URL keys instead of
spreading the whole layer config, so refresh() can no longer overwrite
creation options like the parseInt'd min/maxZoom. Map_ and TimeControl
share that one object. Also fixes the DeckGL branch's early return,
empty time placeholder substitution, and the Deck setLayerWmsParams path.
…Url →

  stac-collection:/COG:/titiler-url: prefix handling) used by both
  Map_.makeTileLayer and TimeControl.reloadLayer, so a time-driven reload
  can no longer resolve a layer onto a different source than creation did
- TimeControl: resolve the tile source before time URL replacements so
  nocache= lands on the tile request instead of inside TiTiler's url=
  param for COG: layers; leave layer.url unmutated for tile layers
- leaflet-tilelayer-middleware: normalize the {t} placeholder on every
  _url assignment (creation and refresh) and extract WMS URL splitting
- tileUrlUtils: apply {time}/{starttime}/{endtime} replacements before
  param injection, since URLSearchParams percent-encodes the braces and
  DeckGL (unlike Leaflet) has no earlier substitution step
- Move tile-url-pipeline.md next to the code; add unit tests for
  tileLayerSource, the middleware, and tileUrlUtils
| `titiler-url` | strip prefix, absolutize against `L_.missionPath` |
| plain template | falls through untouched |

Output: a real base `url`, the `splitColonType` (the stripped prefix, which

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we rename splitcolontype to something that actually indicated the value? such as layerType (if that ends up making sense)

`compileTileUrl` later keys off of), the tile level's `tileElevation`, and the
resolved `tileFormat` (forced to `wmts` for `stac-collection` sources).

> The resolver is **pure**. The `layerObj.tileformat` write for stac layers is

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is tileformat not camelCase?

Comment on lines +75 to +76
> **Both** `Map_.makeTileLayer` and `TimeControl.reloadLayer` call this. They
> used to each carry their own copy of the logic, and the reload copy silently

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to claude: future agents don't need to know the details of how something used to work -- it is no longer relevant. documentation like this should only describe what literally happens or the motivation behind the existing architecture.

> dropped the tile-level selection — a time change would swap the layer back to
> its default source. Keep this single implementation.

> **Note:** `makeTileLayer` calls `TimeControl.performTimeUrlReplacements(...)`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't call it that then, lmao

> **not** part of the `buildTileUrlOptions` / `compileTileUrl` pair — don't
> confuse it for Stage 3.

### Stage 2 — Option building (`buildTileUrlOptions`)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meta question here: this is basically formatting per each arg type. is there a structural reason to group them together (such as many downstream tasks want them to be bundled -- or each of theme needs a shared util supplied by the parent class) or does it make more sense to define them at the level of the arg itself?

| ----------- | -------------------- | ------------------------- | --------------------------------------- |
| **DeckGL** | compile **once** | `Map_.makeTileLayer` | rebuild URL + `Map_.engine.updateLayer` |
| **Leaflet** | compile **per tile** | `Map_.makeTileLayer` | `tileLayer.refresh(...)` |
| **WMS** | own substitution | `L.tileLayer.colorFilter` | reads shared `this.options` times |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wms is an engine???

// that ordering for the three standard tokens (L.Util.template substitutes
// them from `options` before this function is ever called); DeckGL has no
// such step, so a placeholder inside a query string reached the server raw.
nextUrl = nextUrl.replace(/{time}/g, timeStr || '')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

processedUrl? populatedUrl?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants