Repository navigation
Fix COG tile layers with s3:// URLs being treated as mission-relative - #1099
Merged
tariqksoliman merged 1850 commits intoOct 1, 2026
Merged
tariqksoliman merged 1850 commits into
tariqksoliman merged 1850 commits into
Conversation
Each of the seven built-in attachments now owns its own make() plus the lifecycle ops its behavior needs; core builds attachments by iterating LayerAttachmentRegistry in manifest-declared order, deferring the ones that need their siblings. Pairings owns the peer-feature/az-el calculation Photosphere used to reach in for, and PathGradient owns its globe primitive's add/remove. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…entity Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…dinary plugins `paths` is the tools/interactions concept — a key there is the export name a mission config names in `"js"`. Layer types were borrowing the field for render surfaces and attachments for a constant `"plugin"` wrapper, so the same key meant three things. Layer types now declare `modules` (nested per globe engine rather than dotted string keys) and attachments a single `module`. Also aligns the two families with the older four: they carry `uuid`/`id`, they version as `"core"` like every other plugin shipped here (rejecting anything that is neither `"core"` nor semver), and they are discovered by `plugin-cli`, so they list, show `info`, validate and destroy like any plugin — core's stay undisableable through `overridable: false` rather than through being invisible. `activate` regenerates their registries too, so installing one no longer needs a full build. Removes `settings`: declared in the contract, validated, resolved by the generator and exposed as `getSettings()`, but no plugin ever used it. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
… build Velocity's fade-on-move subscription lived in `make`, which re-runs on every show and time change, so listeners accumulated for the life of the page. Subscribe once per layer; the handler already resolves the current layer. A path gradient built its Cesium geometry twice on a host's first toggle-on: core now makes the map layer before the attachment loop, so the loop shows the attachment, and the deferred globe branch (keyed on hadToMake) showed it again. Defer only when that loop didn't run. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
`withOp` still read `mods[id].plugin`, the pre-rename key, so it matched nothing: an attachment's per-feature ops were never found and clearing a clicked image overlay or model silently did nothing. Guard both sides of the generated boundary in a test, since a mismatch here fails as a no-op rather than an error. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…dation The code grew config/filter/time surfaces, classification capabilities and a whole attachment family; the authoring materials still described the render contract. Closes that gap, and moves the CLI out of /plugins so the directory is containers only. - plugin-cli/ at the root: cli.js, lib/, and the six scaffolds as real files under scaffolds/<type>/, token-substituted into paths and contents. They are valid JSON/JS as they sit, so the validator checks them (a broken scaffold fails CI, not someone's first plugin). registries.json is CLI config and moves with it; plugin-state.json stays in /plugins, where the server reads it. - create layerattachment, and a layerattachments README with the contract and a worked example — it was the one family with neither. - Validate classification capabilities against a schema: a wrong type or an out-of-enum value is an error, an unknown key warns, and an omission core acts on (map.stacking, host.order) warns. These previously failed silently as a mis-ordered or un-pickable layer. - Document the config/filter/time surfaces, drop the capture surface and the onHostToggle op (neither has a runtime implementation). - Destroy a layer's attachments when its host is removed for good. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
The leaf name repeated what the plugin directory already said (Vector/map/vector.js), and globe/config.js was not the config surface at all but a helper shared by the two engine modules, one directory away from the real config/vector.js. - Vector/map.js, config.js, filter.js, time.js; globe/cesium.js, globe/lithosphere.js. - globe/config.js → globe/layerConfig.js: it builds the engine-neutral globe layer descriptor, which is not the `config` surface. - Non-surface helpers go in lib/ (VectorTile/lib/SimplifiedVectorGrid.js). - Drop the export default from the layerConfig helpers; every importer uses the named exports. - Vector/map.js declares its ops as named functions like the other 29 modules instead of inline methods. Paths are convention only — the manifest key is what names a surface — so this is manifests, imports and regenerated registries. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
The scaffold's bare `make` is shorthand for `{ main }`, which made Vector's
phase object look like a different contract.
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…ne map `load` was in LAYER_OPS, the validator, the README table and the scaffold, but core never dispatched it and no type implemented it: data acquisition (including the dynamic-extent refetch) is core's, in LayerCapturer. The op table also omitted `render` and `onToggle`, which core does dispatch. `create`'s valid types and the discovery directories were two hand-kept lists, so the help text still advertised five families after layerattachment landed. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
src/pre/*.js and configure/public/*Configs.json are single copies in the repo, so a worker running the CLI could regenerate away a fixture another worker had just written and was asserting on (~1 in 3 runs locally; CI uses one worker). Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Eight sessions each wrote a plugin against this branch with no help beyond the docs. Every one of them tripped over the same three things: - `create layertype` printed "implement make in map/<name>.js", a path that stopped existing when the layout collapsed to one segment per surface. - A scaffolded plugin is untracked, since a container is normally its own repo. Said so where `create` is documented, rather than leaving it to git status. - `plugin-registries.json" is `plugin-cli/registries.json`. Also: a type that `extends` another was warned for capabilities its parent declares, and the two layer families' templates weren't linked from the README's Creating Plugins section that lists the other four. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
The plugins README points at this file for extends; this file never mentioned it. The one child of the eight who was told extends existed found out how it works by reading LayerTypeRegistry and pluginValidation; the others would not have found it at all, and a new data source is exactly the case it exists for. Also states what an expanded sublayer must carry, since that was copied from Vector's STAC path by guess. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Every one of these was found by an author who followed the doc and then found the code disagreed: icons are MDI, not Ionicons (the scaffold itself emits `mdi mdi-`); the toolbar priority bands are 1-4/101-102/1001-1002, not '~1001-1020'; `create` takes six types, not four; and the documented way to run a plugin's own tests fails in global setup without PLAYWRIGHT_TEST_UNIT_ONLY, which only appeared in a package.json script. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…d tests `applicableLayerTypes` was descriptive: a mission config, a kind preset or a type's `defaultInteractions` could name an interaction the interaction says it doesn't handle. updateInteractions now emits APPLICABLE_LAYER_TYPES and the runner filters the whole pipeline — preamble and postamble included — against the layer's type chain, so a type that `extends` vector keeps vector's interactions. The scaffolded tests were CommonJS assertions about plugin.json sitting beside ESM modules, and no npm script ran them. They are now ESM, import the module where the module can load outside the bundler (attachment, interaction, backend), check that every declared module path resolves — the usual cause of a plugin that validates but never appears — and run under `test:plugins:unit`. Backend was the one family with no contract document: adds plugins/core/backend/README.md (the `s` object, lifecycle ordering, ensureAdmin's whitelist, envs, models) and gives the scaffold a deliberate auth gate. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
The Configure form a manifest declares was validated only as "is an object": Maker skips a component type it cannot render and writes wherever `field` points, so a typo'd type or an attachment field outside its own `configPath` yields a form that looks right and configures nothing. validateMetaconfig now checks component types against what Maker renders, field presence, dropdown options, objectarray shape, grid widths, and that an attachment only writes inside its configPath. `validate` also now reports what it could not see before: an enabled plugin absent from src/pre (valid, but the app never loads it) and a registry import whose plugin is gone — both meaning activate has not been run. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Four surfaces the eight authoring experiments asked for, each replacing something core was deciding on their behalf: - layertype `source.fetch(layerObj, ctx) -> GeoJSON`, dispatched from LayerCapturer for every acquisition. Core keeps the whole dynamic-extent policy (extent, debounce/settling, zoom gate, move threshold, request staleness, clear/update, reload subscribers) — extracted out of the two duplicated branches into capture/dynamicExtent.js so it is testable — so `fetch` is a pure "given this, give me GeoJSON" for data that doesn't come out of a url core can fetch (POST bodies, headers, pagination, an SDK). - layertype `legend.derive(layerObj)`, replacing the `['image','tile'].includes(type) && cogTransform || type === 'velocity' || (type === 'data' && shader === 'colorize')` list that both the LayersTool and the LegendTool carried. Tile/Image/Velocity/Data now declare the surface. - attachment `onConfigChange(ctx)`, with `mmgisAPI.setLayerAttachmentConfig()` as its dispatcher: core writes the new settings into the host's live config at the declared configPath, then lets the attachment retune in place rather than paying for the default host rebuild. - interaction `configPath`, so an interaction with per-layer settings is handed its own subtree as `ctx.config` instead of digging through layerData. Also: `raw` (the engine namespace) on both globe contexts, matching `mctx.raw`; `test:plugins:unit` no longer fails when no plugin ships a @Unit spec. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Eight plugins were written against this branch by separate agents; these are the things they had to guess at or work around. - The layertypes README still said data acquisition is not in the vocabulary and belongs in `make`, 75 lines above the `source` surface that replaced that advice. - `gctx.raw` was absent from the gctx table, which still sent globe authors to `gctx.renderer` for engine-specific work. Documented, including that the two engines' namespaces are symmetric in name only. - Globe operations are dispatched by layer *name* (only `make` gets a layer object), and `setVisibility`/`setOpacity` take the new value positionally. None of that was written down; two authors guessed. - Nothing said how a layer opts into dynamic extent, so `source.fetch` saw a null `ctx.view` with no explanation. - `legend.derive` is handed config, not data; say so, and where render state actually lives, instead of leaving authors to stash on the layer. - `onConfigChange` is passed the built attachment, which the ctx table omitted. - The layertype scaffold's own test asserted `modules.globe[key]` over `Object.keys(renderers.globe)`, so it failed for every globe layertype, and it assumed `modules` where a single-module `extends` plugin has `module`. - The scaffold and the attachment README both captured `window.L` at import time, which is undefined in the unit test the same scaffold ships. Read the global per call instead, and drop `F_` from the example: importing an MMGIS singleton pulls jQuery and makes a module un-importable in Node, which browser-globals now states rather than implying it protects against that. - `create` printed `npm run build` for interactions instead of `activate`, said nothing when a name collided with an existing plugin in another family (a `Curtain` layertype silently shadowed the `Curtain` tool), and left the author to discover that a non-core container is untracked here. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
A plugin whose main setting is written in another language — the Overpass QL query one of the authoring experiments built its layertype around — got a single-line text field, because every free-text component Maker renders is one. `textarea` is multiline and monospace, sized by an optional `rows`. The component vocabulary itself was only discoverable by declaring something invalid and reading the validator's error, so plugins/README.md now has the metaconfig section that error was standing in for: what a row is, what each component field does, and what all 23 component types render. Also: the interaction runner assigned `ctx.config` only for interactions that declare a `configPath`, so an interaction without one was handed whichever config the interaction before it in the pipeline had. It now gets null, which is also what a layer that was never configured yields — the previous test asserted the leak, so it changes with it. For the globe, `gctx.raw` on LithoSphere is the globe class and nothing else (the package exports one thing), so the README now says where the work actually happens: `renderer` for the live globe and `window.THREE` for geometry, which MMGIS vendors and puts on the window. An author looked for THREE on `raw`, didn't find it, and dropped LithoSphere support. `create layertype` scaffolds a standalone renderer, which is also now called out as the wrong starting point for a type that draws like one MMGIS already has: extend it instead. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
An interaction could be handed `ctx.config` but had no way to declare what an admin should be able to put there, so its settings existed only as prose in a README. It now declares `config.rows` in the same metaconfig vocabulary an attachment uses, and Maker renders those rows on the interaction's card in a layer's Interactions tab, behind a gear — beside where the interaction was chosen and dragged, rather than in a tab of its own. Validation follows from where the form lives: rows need a `configPath` (the runner has nowhere else to read them back from), every `field` must sit inside it, and `tab`/`tabs` are rejected. A row's `default` remains a form default — nothing is written until an admin touches the field — so plugins still default their own values, which the scaffold and README now spell out. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Closing a layer modal rebuilds the layer from the fields its layer type's tabs declare, which is why attachment config paths were already exempted. An interaction's settings come from its own manifest, so they were written, rendered, and then silently dropped on close. Interaction manifests now load into the store alongside the attachment ones, and their config paths are preserved by the same pass. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…per user under AUTH=local Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…IN_MISSION guard, and e2e tests Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…UTH=local Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…ache per-user lookup, document cross-mission note in Users UI Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…n on mission mutations Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…equests Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…es; plain 403 for non-browser clients Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Per-user mission viewing permissions (missions_viewing) under AUTH=local
Revert "Per-user mission viewing permissions (missions_viewing) under AUTH=local" (#130)
…3049-sync-upstream-development Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com> # Conflicts: # configure/package.json # package.json
…-development Sync upstream NASA-AMMOS/MMGIS development into development
…lAbsolute Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…source Globe layer config passed the raw COG source (s3://..., ../../Missions/...) to Cesium/LithoSphere instead of the /titiler/cog/tiles endpoint the 2D map builds. Extract buildCogTileUrl into Tile/cog.js and use it from both, and encode the source as a single url= value (also in the Configure cog/info and Identifier cog/point requests) so sources with their own query strings (?versionId=...&...) survive. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
This reverts commit 0161c70.
…the COG source" This reverts commit 0c56dee.
…4801-cog-s3-scheme Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com> # Conflicts: # configure/package.json # package.json
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔍 Devin Review: 1 flag
Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With Devin: JPL-Devin#147
Purpose
A Tile layer with
sourceType: COGand ans3://bucket/key.tifURL did not work; users had to hand-write a fullhttps://.../titiler/cog/tiles/...?url=s3://...URL instead.Root cause:
F_.isUrlAbsolute(and its copy inconfigure/src/core/utils.js) used^(?:[a-z]+:)?//, so any scheme containing a digit (s3:) was not recognized as absolute.L_.getUrlthen prepended the mission path, producing tile requests like:Proposed Changes
isUrlAbsolutenow accepts any RFC 3986 scheme (^(?:[a-z][a-z0-9+.-]*:)?//) in bothFormulae_.jsandconfigure/src/core/utils.js, sos3://,gs://,az://,svn+ssh://etc. count as absolute.tests/unit/formulae.spec.js.(The Cesium TiTiler-wrapping and
url=encoding changes from review were reverted as out of scope for this PR; they are pre-existing behaviours affecting https:// COGs too and can be a follow-up.)Issues
Testing
sourceType: COG,url: s3://my-bucket/path/file.tiftile layer: before the fix tile requests carriedurl=../../Missions/CogTest/s3://...; after they are.../titiler/cog/tiles/WebMercatorQuad/{z}/{x}/{y}.webp?url=s3://my-bucket/path/file.tif.PLAYWRIGHT_TEST_UNIT_ONLY=true npx playwright test tests/unit/formulae.spec.js tests/unit/layerSource.spec.js— all pass.Note for deployers:
TITILER_ALLOWED_URL_PATTERNS(default only allows^https://and^/Missions/) must also permit^s3://...for the proxy to forward s3 URLs to TiTiler.