diff --git a/API/updateTools.js b/API/updateTools.js index c50aa980e..c217a3231 100644 --- a/API/updateTools.js +++ b/API/updateTools.js @@ -4,6 +4,29 @@ const path = require("path"); const logger = require("./logger"); const { isLean } = require("./Backend/Utils/deploymentMode"); +// Module binding (a `paths` key) -> the tool's address: the name it answers to +// on the bus, in the modern controller's registries and in teardown events. +// toolCanonicalId (src/essence/Basics/ToolController_/ToolMetadataUtils.js) +// applies the same derivation in the browser, so the build and the frontend +// agree on what a tool is called. Two bindings can derive one address (`Foo` +// and `FooTool`), so a collision throws here and fails the build. +function buildToolIds(tools) { + const ids = {}; + const claimedBy = new Map(); + for (const t in tools) { + for (const p in tools[t].paths) { + const id = p.replace(/Tool$/, "").toLowerCase(); + if (claimedBy.has(id) && claimedBy.get(id) !== p) + throw new Error( + `Tool bindings "${claimedBy.get(id)}" and "${p}" both derive the address "${id}"` + ); + claimedBy.set(id, p); + ids[p] = id; + } + } + return ids; +} + function updateTools() { let tools = {}; @@ -189,6 +212,9 @@ function updateTools() { toolConfigs += `export const toolModules = ${JSON.stringify( toolModules ).replace(/"/g, "")}\n`; + toolConfigs += `export const toolIds = ${JSON.stringify( + buildToolIds(tools) + )}\n`; toolConfigs += `export const testModules = ${JSON.stringify( testModules ).replace(/"/g, "")}\n`; @@ -383,4 +409,9 @@ function updateComponents() { } } -module.exports = { updateTools, updateComponents, bakeStaticConfig }; +module.exports = { + updateTools, + updateComponents, + bakeStaticConfig, + buildToolIds, +}; diff --git a/docs/pages/APIs/JavaScript/Main/Event-Bus-API.md b/docs/pages/APIs/JavaScript/Main/Event-Bus-API.md index 22b2acfb5..8ef04f666 100644 --- a/docs/pages/APIs/JavaScript/Main/Event-Bus-API.md +++ b/docs/pages/APIs/JavaScript/Main/Event-Bus-API.md @@ -155,9 +155,11 @@ anything there right now. ## Plugin Scoped API -MMGIS automatically injects a scoped API into each tool as `this.api`. This API automatically prefixes event and provider names with `plugin:{address}:`, where `address` is derived from the tool's module name (e.g., `DrawTool` → `draw`). +MMGIS injects a scoped API into each tool the modern layout loads as `this.api`. This API automatically prefixes event and provider names with `plugin:{address}:`, where `address` is derived at build time from the tool's module binding (e.g., `DrawTool` → `draw`). A tool never mints its own handle — there is no public way to — so a plugin reaches its own and no other's. -> **Note:** Each plugin must have a unique ID. Multiple instances of the same plugin in a mission are not currently supported. If two plugins share the same ID, their events and providers will collide. This constraint is not currently enforced at runtime but may be in a future version. +The controller mints the handle before the tool's `initialize()` runs and releases it after the tool's `destroy()` returns, unregistering every provider and subscription made through it. Anything a tool registers straight on `window.mmgisAPI` sits outside the handle and stays the tool's own to remove: the React-based tools work that way today, so their requests carry no caller and LayerManager's unprefixed providers outlive a release. + +> **Note:** An address comes from a tool's module binding, and two bindings that derive the same address fail the build. One tool still cannot run as two instances: the second would answer for the first's events and providers. The scoped API is available on `this.api` in your tool's `initialize()` and `make()` functions: @@ -179,13 +181,13 @@ const MyTool = { Emit an event with auto-prefixed name. ```javascript -const api = window.mmgisAPI.forPlugin('myPlugin') +const api = this.api // injected; address 'myplugin' -// This emits 'plugin:myPlugin:dataUpdated' +// This emits 'plugin:myplugin:dataUpdated' api.emit('dataUpdated', { value: 42 }) // Subscribers listen using the full path -window.mmgisAPI.on('plugin:myPlugin:dataUpdated', (data) => { +window.mmgisAPI.on('plugin:myplugin:dataUpdated', (data) => { console.log(data.value) // 42 }) ``` @@ -197,7 +199,7 @@ Subscribe to an event. Like `request`, names are **not** prefixed — a subscrip The handle tracks the subscription, so `release()` drops it along with the plugin's providers. The returned unsubscribe is there for letting one go sooner. ```javascript -const api = window.mmgisAPI.forPlugin('myPlugin') +const api = this.api const off = api.on('layer:visibilityChange', handleLayerChange) @@ -211,15 +213,15 @@ Register a provider with auto-prefixed name. **Returns:** Cleanup function ```javascript -const api = window.mmgisAPI.forPlugin('myPlugin') +const api = this.api // injected; address 'myplugin' -// This registers 'plugin:myPlugin:getData' +// This registers 'plugin:myplugin:getData' const cleanup = api.provide('getData', (params) => { return { result: params.input * 2 } }) // Callers request using the full path -const data = await window.mmgisAPI.request('plugin:myPlugin:getData', { input: 21 }) +const data = await window.mmgisAPI.request('plugin:myplugin:getData', { input: 21 }) console.log(data.result) // 42 // Later, remove the provider @@ -231,9 +233,9 @@ cleanup() Request another provider, stamped with this plugin's address. Names are **not** prefixed: a request addresses someone else's provider, so it takes the full name. ```javascript -const api = window.mmgisAPI.forPlugin('myPlugin') +const api = this.api // injected; address 'myplugin' -// The provider is called with ({ input: 21 }, { caller: 'myPlugin' }) +// The provider is called with ({ input: 21 }, { caller: 'myplugin' }) await api.request('plugin:other:getData', { input: 21 }) ``` @@ -248,11 +250,11 @@ Hand every registration this handle made back to core — its own `getVars` prov After release the handle is inert: `emit`, `provide` and `on` do nothing, `request` resolves to `null`, and releasing again changes nothing. ```javascript -const api = window.mmgisAPI.forPlugin('myPlugin') -api.provide('getData', () => data) // 'plugin:myPlugin:getData' +const api = this.api // injected; address 'myplugin' +api.provide('getData', () => data) // 'plugin:myplugin:getData' api.release() -window.mmgisAPI.hasHandler('plugin:myPlugin:getData') // false +window.mmgisAPI.hasHandler('plugin:myplugin:getData') // false ``` ### Metadata Properties @@ -260,10 +262,10 @@ window.mmgisAPI.hasHandler('plugin:myPlugin:getData') // false The scoped API also exposes metadata: ```javascript -const api = window.mmgisAPI.forPlugin('myPlugin') +const api = this.api -console.log(api.address) // 'myPlugin' -console.log(api.prefix) // 'plugin:myPlugin:' +console.log(api.address) // 'myplugin' +console.log(api.prefix) // 'plugin:myplugin:' ``` ### Complete Plugin Example @@ -386,6 +388,8 @@ window.mmgisAPI.on('legend:made', ({ layerName, legendData }) => { |-------|---------|-------------| | `panels:changed` | `{ panels }` | Fired whenever the panel layout changes — a panel registered or unregistered, changed state, lost a tool, or was resized — and once with an empty listing when the layout is torn down | | `plugins:changed` | `{ plugins }` | Fired whenever a plugin is shown, hidden, loaded or unloaded by command, once after a batch of plugins loads with the layout, and once with an empty listing when the layout is torn down | +| `plugins:destroyed` | `{ pluginId }` | Fired as one plugin is torn down, after its own `destroy()` has run and its bus handle has been released | +| `plugins:allDestroyed` | `{ pluginIds }` | Fired once when a layout teardown destroyed at least one plugin, after each plugin's own `plugins:destroyed` | `panels` carries the same listing [`panels:getAll`](#panel-and-plugin-providers) returns, and `plugins` the same listing `plugins:getAll` returns, so there is @@ -416,6 +420,16 @@ A component that seeds from `panels:getAll` and also subscribes to `panels:changed` must guard the seed so it cannot overwrite state an event has already delivered — the request can resolve after a later event lands. +`plugins:destroyed` and `plugins:allDestroyed` report the teardown itself +rather than the listing that results from it. Both are signals a core service +releases shared resources on. `pluginId` is the departing plugin's address — +the identity it spoke to services under — so a release matched against it +reaches only what that plugin held, and a surviving plugin's stays put. The +collective signal releases outright, because with every plugin destroyed the +resource's owner is among them and no bystander pays for the release. A +teardown a command asked for is followed by `plugins:changed` carrying the new +listing. + ### WebSocket Events | Event | Payload | Description | diff --git a/src/essence/Basics/Layers_/Layers_.js b/src/essence/Basics/Layers_/Layers_.js index b94e8ad89..ee68b1ab1 100644 --- a/src/essence/Basics/Layers_/Layers_.js +++ b/src/essence/Basics/Layers_/Layers_.js @@ -5,6 +5,7 @@ import Search from '../../Ancillary/Search' import Attributions from '../../Ancillary/Attributions' import CursorInfo from '../../Ancillary/CursorInfo' import ToolController_ from '../../Basics/ToolController_/ToolController_' +import { toolCanonicalId } from '../ToolController_/ToolMetadataUtils' import LayerGeologic from './LayerGeologic/LayerGeologic' import ServiceUrls from '../ServiceUrls/ServiceUrls' import { isRasterTileLayerType } from '../MapEngines/types/engine' @@ -2486,22 +2487,15 @@ const L_ = { L_.Map_.resetView(L_.configData.msv.view) L_.Globe_.litho.setCenter(L_.configData.msv.view) }, - hasTool: function (toolName) { - for (var i = 0; i < L_.tools.length; i++) { - if ( - L_.tools[i].hasOwnProperty('name') && - L_.tools[i].name.toLowerCase() == toolName - ) - return true - } - return false - }, getToolVars: function (toolName, withVarsFromLayers, showWarnings) { let vars = {} for (var i = 0; i < L_.tools.length; i++) { + // Matched on the tool's address first, with the lowercased display + // name kept as a fallback for callers that ask by that instead. if ( - L_.tools[i].hasOwnProperty('name') && - L_.tools[i].name.toLowerCase() == toolName && + (toolCanonicalId(L_.tools[i]) === toolName || + (L_.tools[i].hasOwnProperty('name') && + L_.tools[i].name.toLowerCase() == toolName)) && L_.tools[i].hasOwnProperty('variables') ) { vars = L_.tools[i].variables diff --git a/src/essence/Basics/ToolController_/TOOL_RENDERING_PIPELINE.md b/src/essence/Basics/ToolController_/TOOL_RENDERING_PIPELINE.md index 381bbdcba..d7b024ae0 100644 --- a/src/essence/Basics/ToolController_/TOOL_RENDERING_PIPELINE.md +++ b/src/essence/Basics/ToolController_/TOOL_RENDERING_PIPELINE.md @@ -21,6 +21,8 @@ The validated dashboard configuration declares available UI panels (e.g., left, Tools defined in the mission configuration are processed to generate normalized metadata. The `buildToolConfigMap` function parses properties like layout orientation, preferred positions, and custom icons. This step produces a clean `ToolMetadata` object used for capability matching, separating visual logic from core tool behavior. +Metadata carries two names for a tool. `module` is the binding that reaches its class in the generated registry (`src/pre/tools.js`); `id` is its address — what it is called on the bus, in the controller's registries, in `data-tool` attributes and in teardown events. The address is derived from the binding at build time (`AOITool` → `aoi`), so a panel may name a tool by its display name, its address or its binding and reach the same tool. + ## 4. Tool Assignment **Module:** `src/essence/Basics/ToolController_/ToolControllerModern_.js` @@ -42,4 +44,4 @@ The `UserInterfaceModern_.render()` method constructs the physical HTML structur To prevent DOM race conditions and ensure CSS layout calculations are finalized, the pending `toolLoadQueue` is executed asynchronously using `setTimeout(fn, 0)`. -Once triggered, `ToolControllerModern_.loadTool()` is called for each pending tool. This method locates the tool module, calls its `initialize()` method, and delegates DOM injection by invoking the tool's `make(targetId)` method inside its assigned placeholder container. +Once triggered, `ToolControllerModern_.loadTool()` is called for each pending tool. This method locates the tool module, mints the tool's plugin-scoped bus handle and assigns it to the instance as `api`, calls its `initialize()` method, and delegates DOM injection by invoking the tool's `make(targetId)` method inside its assigned placeholder container. `destroyTool` releases that handle after the tool's own `destroy()` has run, then announces the teardown as `plugins:destroyed` with the tool's address. diff --git a/src/essence/Basics/ToolController_/ToolControllerModern_.js b/src/essence/Basics/ToolController_/ToolControllerModern_.js index 79af70c69..893881588 100644 --- a/src/essence/Basics/ToolController_/ToolControllerModern_.js +++ b/src/essence/Basics/ToolController_/ToolControllerModern_.js @@ -2,13 +2,14 @@ import PanelManager_ from '../PanelManager_/PanelManager_' import { toolModules } from '../../../pre/tools' import { generateToolMetadata } from './ToolMetadataUtils' import { createLogger } from '../Logger_/Logger_' -import { mmgisAPI } from '../../mmgisAPI/mmgisAPI' +import { mmgisAPI, mintHandle } from '../../mmgisAPI/mmgisAPI' const logger = createLogger('ToolControllerModern') // --- Module-Level State --- /** - * Map of loaded tool instances: targetId -> { toolInstance, toolName, toolId, toolMetadata } + * Map of loaded tool instances: + * targetId -> { toolInstance, toolName, toolId, toolMetadata, targetId, api } */ const loadedTools = new Map() @@ -29,8 +30,27 @@ const hiddenTools = new Set() */ const deferredTools = new Map() +/** + * The other two ways a mission config names a tool: its display name, and the + * registry binding its `js` carries. Both are rebuilt by buildToolConfigMap. + */ +const toolNameToId = new Map() // name -> address +const toolModuleToId = new Map() // registry binding -> address + // --- Internal Helper Functions --- +/** + * The address for however a config named a tool — its display name, its + * registry binding, or the address itself. An unrecognised string passes + * through so the caller reports it under the name the config used. + * + * @param {string} nameOrId - Tool name, registry binding, or address + * @returns {string} Tool address + */ +function resolvePluginId(nameOrId) { + return toolNameToId.get(nameOrId) || toolModuleToId.get(nameOrId) || nameOrId +} + /** * Finds a compatible panel for a tool, trying preferred position first * @param {Object} metadata Tool metadata @@ -142,8 +162,9 @@ const ToolControllerModern_ = { * @returns {Object} Object containing toolConfigMap and getToolData helper */ buildToolConfigMap: function (tools) { - const toolConfigMap = new Map() // id -> { config, metadata } - const toolNameToId = new Map() // name -> id + const toolConfigMap = new Map() // address -> { config, metadata } + toolNameToId.clear() + toolModuleToId.clear() tools.forEach(toolConfig => { const toolMetadata = generateToolMetadata(toolConfig) @@ -161,13 +182,13 @@ const ToolControllerModern_ = { } else { toolNameToId.set(toolMetadata.name, toolMetadata.id) } + if (toolMetadata.module) toolModuleToId.set(toolMetadata.module, toolMetadata.id) }) - // Helper function to get tool data by name or ID - const getToolData = (nameOrId) => { - const id = toolNameToId.get(nameOrId) || nameOrId - return toolConfigMap.get(id) - } + // A panel names its tools however the mission config author wrote + // them: the display name, the address, or the registry binding the + // config's `js` carries. + const getToolData = (nameOrId) => toolConfigMap.get(resolvePluginId(nameOrId)) return { toolConfigMap, getToolData } }, @@ -304,7 +325,8 @@ const ToolControllerModern_ = { * Load and instantiate a tool in a specific target container * * @param {Object} toolMetadata - Tool metadata object - * @param {string} toolMetadata.id - Tool identifier (e.g., 'TitleTool') + * @param {string} toolMetadata.id - Tool address (e.g., 'title') + * @param {string} toolMetadata.module - Registry binding (e.g., 'TitleTool') * @param {string} toolMetadata.name - Tool display name (e.g., 'Title') * @param {string} targetId - DOM element ID where tool should render * @returns {Object|null} Tool instance or null if failed @@ -333,13 +355,20 @@ const ToolControllerModern_ = { return null } + // Held outside the try so a load that throws part-way can hand back + // the handle it was already given. + let ToolClass = null + let api = null + let tracked = false + try { - // Find the tool module in pre/tools.js exports - const ToolClass = toolModules[toolMetadata.id] + // The class is reached by the module binding; everything else about + // this tool is keyed by its address. + ToolClass = toolModules[toolMetadata.module] if (!ToolClass) { logger.error( - `Tool module "${toolMetadata.id}" not found in toolModules.`, + `Tool module "${toolMetadata.module}" (id "${toolMetadata.id}") not found in toolModules.`, 'Available tools:', Object.keys(toolModules) ) @@ -357,6 +386,13 @@ const ToolControllerModern_ = { this.destroyTool(targetId) } + // Mint the tool's bus handle before it can run any of its own code, + // so initialize()/make() already have somewhere to hang providers + // and events. It is kept beside the instance for destroyTool to + // release: whoever hands a handle out owns taking it back. + api = mintHandle(toolMetadata.id) + ToolClass.api = api + // Initialize tool if it has an initialize method if (typeof ToolClass.initialize === 'function') { ToolClass.initialize() @@ -375,8 +411,10 @@ const ToolControllerModern_ = { toolName: toolMetadata.name, toolId: toolMetadata.id, toolMetadata: toolMetadata, - targetId: targetId + targetId: targetId, + api: api }) + tracked = true // Register reverse lookup (toolId -> targetId) for show/hide/unload by toolId toolIdToTargetId.set(toolMetadata.id, targetId) @@ -396,6 +434,14 @@ const ToolControllerModern_ = { return ToolClass } catch (error) { logger.error(`Failed to load tool "${toolMetadata.name}":`, error) + + // An untracked instance is one no destroyTool will ever come for, + // so its handle's registrations would answer for a tool that isn't + // there for the rest of the session. + if (api && !tracked) { + api.release() + if (ToolClass.api === api) ToolClass.api = null + } return null } }, @@ -404,18 +450,19 @@ const ToolControllerModern_ = { * Destroy a tool instance in a specific container * * @param {string} targetId - DOM element ID of the tool container - * @returns {boolean} True if destroyed successfully + * @returns {boolean} False when the tool's own destroy() or its handle's + * release threw; the lifecycle registries are cleared either way. */ destroyTool: function (targetId) { if (!loadedTools.has(targetId)) { return false } - const { toolInstance, toolName, toolId } = loadedTools.get(targetId) + const { toolInstance, toolName, toolId, api } = loadedTools.get(targetId) let destroyed = true try { - // Call destroy() if available + // destroy() runs before the handle is taken back, so a tool can still speak to core. if (typeof toolInstance.destroy === 'function') { toolInstance.destroy() } @@ -423,8 +470,7 @@ const ToolControllerModern_ = { logger.error(`Error destroying tool in container "${targetId}":`, error) destroyed = false } finally { - // Always remove from tracking, even if destroy() threw, so a - // misbehaving plugin can't leave stale/zombie lifecycle state + // Untracked even if destroy() threw, so no zombie lifecycle state. loadedTools.delete(targetId) // Clean up reverse lookup and hidden state @@ -432,8 +478,24 @@ const ToolControllerModern_ = { toolIdToTargetId.delete(toolId) hiddenTools.delete(toolId) } + + // Released last and guarded so the teardown announcement below + // still runs; a reload's fresher `api` on the singleton is kept. + if (api) { + try { + api.release() + } catch (error) { + logger.error(`Error releasing the bus handle for "${toolId}":`, error) + destroyed = false + } + if (toolInstance.api === api) toolInstance.api = null + } } + // Announced on the bus, not by direct call, so this controller stays + // ignorant of the services that match held resources to `pluginId`. + mmgisAPI.emit('plugins:destroyed', Object.freeze({ pluginId: toolId })) + if (destroyed) { logger.debug(`Destroyed tool "${toolName}" from container "${targetId}"`) } @@ -446,6 +508,7 @@ const ToolControllerModern_ = { */ destroyAllTools: function () { const targetIds = Array.from(loadedTools.keys()) + const pluginIds = targetIds.map((targetId) => loadedTools.get(targetId).toolId) const hadPlugins = toolIdToTargetId.size > 0 || deferredTools.size > 0 logger.debug(`Destroying ${targetIds.length} loaded tools`) @@ -457,6 +520,15 @@ const ToolControllerModern_ = { // Clear deferred registry (destroyTool already clears toolIdToTargetId and hiddenTools) deferredTools.clear() + // One collective signal after the per-tool announcements, for services + // holding a resource shared across plugins; nothing loaded, nothing held. + if (pluginIds.length > 0) { + mmgisAPI.emit( + 'plugins:allDestroyed', + Object.freeze({ pluginIds: Object.freeze(pluginIds) }) + ) + } + if (hadPlugins) this.notifyPluginsChanged() }, @@ -525,13 +597,14 @@ const ToolControllerModern_ = { * Show a plugin that was hidden via hidePlugin or startHidden config. * The plugin must already be loaded — its instance and state are preserved. * - * @param {string} pluginId - Tool ID (e.g., 'TitleTool') + * @param {string} nameOrId - Tool address (e.g., 'title'), display name, or registry binding * @returns {boolean} True if shown successfully, false if not found */ - showPlugin: function (pluginId) { + showPlugin: function (nameOrId) { + const pluginId = resolvePluginId(nameOrId) const targetId = toolIdToTargetId.get(pluginId) if (!targetId) { - logger.warn(`showPlugin: "${pluginId}" is not a loaded plugin`) + logger.warn(`showPlugin: "${nameOrId}" is not a loaded plugin`) return false } document.getElementById(targetId)?.classList.remove('plugin-hidden') @@ -545,13 +618,14 @@ const ToolControllerModern_ = { * Hide a plugin without destroying it. Its instance and internal state are preserved; * calling showPlugin later restores it exactly as left. * - * @param {string} pluginId - Tool ID + * @param {string} nameOrId - Tool address, display name, or registry binding * @returns {boolean} True if hidden successfully, false if not found */ - hidePlugin: function (pluginId) { + hidePlugin: function (nameOrId) { + const pluginId = resolvePluginId(nameOrId) const targetId = toolIdToTargetId.get(pluginId) if (!targetId) { - logger.warn(`hidePlugin: "${pluginId}" is not a loaded plugin`) + logger.warn(`hidePlugin: "${nameOrId}" is not a loaded plugin`) return false } document.getElementById(targetId)?.classList.add('plugin-hidden') @@ -565,17 +639,18 @@ const ToolControllerModern_ = { * Load a plugin that is currently deferred (startUnloaded at init, or previously unloaded). * Calls make() on the existing DOM container. The plugin starts visible after load. * - * @param {string} pluginId - Tool ID + * @param {string} nameOrId - Tool address, display name, or registry binding * @returns {boolean} True if loaded (or already loaded), false if not found / load failed */ - loadPlugin: function (pluginId) { + loadPlugin: function (nameOrId) { + const pluginId = resolvePluginId(nameOrId) if (toolIdToTargetId.has(pluginId)) { - logger.warn(`loadPlugin: "${pluginId}" is already loaded`) + logger.warn(`loadPlugin: "${nameOrId}" is already loaded`) return true } const deferred = deferredTools.get(pluginId) if (!deferred) { - logger.warn(`loadPlugin: "${pluginId}" not found in deferred registry`) + logger.warn(`loadPlugin: "${nameOrId}" not found in deferred registry`) return false } const instance = this.loadTool(deferred.toolMetadata, deferred.targetId) @@ -594,18 +669,19 @@ const ToolControllerModern_ = { * Fully unload a plugin, releasing its instance and resources. * The DOM container is emptied but kept in place so loadPlugin can recreate it later. * - * @param {string} pluginId - Tool ID + * @param {string} nameOrId - Tool address, display name, or registry binding * @returns {boolean} True if unloaded successfully, false if not found or already unloaded */ - unloadPlugin: function (pluginId) { + unloadPlugin: function (nameOrId) { + const pluginId = resolvePluginId(nameOrId) const targetId = toolIdToTargetId.get(pluginId) if (!targetId) { - logger.warn(`unloadPlugin: "${pluginId}" is not loaded`) + logger.warn(`unloadPlugin: "${nameOrId}" is not loaded`) return false } const toolData = loadedTools.get(targetId) if (!toolData) { - logger.warn(`unloadPlugin: "${pluginId}" data not found`) + logger.warn(`unloadPlugin: "${nameOrId}" data not found`) return false } const savedMetadata = toolData.toolMetadata @@ -631,11 +707,11 @@ const ToolControllerModern_ = { /** * Check if a plugin is currently loaded (make() has been called and not yet destroyed) * - * @param {string} pluginId - Tool ID + * @param {string} nameOrId - Tool address, display name, or registry binding * @returns {boolean} */ - isPluginLoaded: function (pluginId) { - return toolIdToTargetId.has(pluginId) + isPluginLoaded: function (nameOrId) { + return toolIdToTargetId.has(resolvePluginId(nameOrId)) }, /** @@ -643,10 +719,11 @@ const ToolControllerModern_ = { * (loaded but not visible), or because it's deferred/unloaded (startUnloaded, * or unloadPlugin) * - * @param {string} pluginId - Tool ID + * @param {string} nameOrId - Tool address, display name, or registry binding * @returns {boolean} */ - isPluginHidden: function (pluginId) { + isPluginHidden: function (nameOrId) { + const pluginId = resolvePluginId(nameOrId) return hiddenTools.has(pluginId) || deferredTools.has(pluginId) }, @@ -656,10 +733,11 @@ const ToolControllerModern_ = { * - hidden: loaded, instance and state intact, not visible * - visible: loaded and on screen * - * @param {string} pluginId - Tool ID + * @param {string} nameOrId - Tool address, display name, or registry binding * @returns {'unloaded'|'hidden'|'visible'|null} null when the id is unknown */ - getPluginState: function (pluginId) { + getPluginState: function (nameOrId) { + const pluginId = resolvePluginId(nameOrId) if (deferredTools.has(pluginId)) return 'unloaded' if (!toolIdToTargetId.has(pluginId)) return null return hiddenTools.has(pluginId) ? 'hidden' : 'visible' @@ -670,15 +748,16 @@ const ToolControllerModern_ = { * transition needs — asking for 'visible' on an unloaded plugin loads it. * Idempotent: asking for the state a plugin already holds changes nothing. * - * @param {string} pluginId - Tool ID + * @param {string} nameOrId - Tool address, display name, or registry binding * @param {'unloaded'|'hidden'|'visible'} state - Target state * @returns {object} { ok: true, state, changed } or { ok: false, reason } */ - setPluginState: function (pluginId, state) { + setPluginState: function (nameOrId, state) { if (!['unloaded', 'hidden', 'visible'].includes(state)) { return { ok: false, reason: 'bad-request' } } + const pluginId = resolvePluginId(nameOrId) const current = this.getPluginState(pluginId) if (current === null) return { ok: false, reason: 'not-found' } if (current === state) return { ok: true, state, changed: false } diff --git a/src/essence/Basics/ToolController_/ToolController_.js b/src/essence/Basics/ToolController_/ToolController_.js index 485dd4c76..073317bee 100644 --- a/src/essence/Basics/ToolController_/ToolController_.js +++ b/src/essence/Basics/ToolController_/ToolController_.js @@ -483,11 +483,6 @@ let ToolController_ = { ToolController_.toolModuleNames.forEach((t) => { const tool = ToolController_.toolModules[t] if (tool) { - // Inject scoped API based on tool name (e.g., "DrawTool" -> "draw") - if (window.mmgisAPI && !tool.api) { - const pluginId = t.replace(/Tool$/, '').toLowerCase() - tool.api = window.mmgisAPI.forPlugin(pluginId) - } if (typeof tool.initialize === 'function') { tool.initialize() } diff --git a/src/essence/Basics/ToolController_/ToolMetadataUtils.js b/src/essence/Basics/ToolController_/ToolMetadataUtils.js index 7ae162dd4..2013b637e 100644 --- a/src/essence/Basics/ToolController_/ToolMetadataUtils.js +++ b/src/essence/Basics/ToolController_/ToolMetadataUtils.js @@ -3,6 +3,7 @@ */ import DOMPurify from 'dompurify' +import { toolIds as generatedToolIds } from '../../../pre/tools' import { TOOL_ORIENTATION } from './types/tool' import { PANEL_POSITION } from '../PanelManager_/types/layout' import { createLogger } from '../Logger_/Logger_' @@ -204,6 +205,12 @@ export function sanitizeToolMetadata(metadata) { // Sanitize critical string fields that are used in DOM sanitized.id = sanitizeValue(metadata.id, 'id') sanitized.name = sanitizeValue(metadata.name, 'text') + // generateToolMetadata always sets this, to '' for a config naming no + // module. Metadata assembled by hand carries no binding at all and comes + // back without the key rather than with an empty one. + if (metadata.module !== undefined) { + sanitized.module = sanitizeValue(metadata.module, 'id') + } sanitized.icon = getValidIconClass(metadata.icon, sanitized.id) // Verify critical fields didn't become empty @@ -512,6 +519,29 @@ export function getValidIconClass(iconClass, toolId) { return result.normalized } +/** + * The address a configured tool entry answers to: what it is called on the + * bus, in the modern controller's registries, in the DOM, in teardown events + * and as the key its configured variables resolve under. It comes from the + * generated registry, or is derived from `js` the way the build derives it + * (buildToolIds in API/updateTools.js), or from the entry's own name when + * there is no module. + * + * @param {Object} toolConfig - Tool configuration entry, read for { js, name } + * @returns {string} Tool address + */ +export function toolCanonicalId(toolConfig) { + const toolModule = (toolConfig && toolConfig.js) || '' + if (toolModule) { + const ids = generatedToolIds ?? {} + return Object.prototype.hasOwnProperty.call(ids, toolModule) + ? ids[toolModule] + : toolModule.replace(/Tool$/, '').toLowerCase() + } + const toolName = (toolConfig && toolConfig.name) || 'Unknown' + return toolName.toLowerCase().replace(/\s+/g, '-') +} + /** * Generate tool metadata from tool configuration * Consolidates metadata from both root-level (legacy) and nested metadata object. @@ -522,7 +552,8 @@ export function getValidIconClass(iconClass, toolId) { */ export function generateToolMetadata(toolConfig) { const toolName = toolConfig.name || 'Unknown' - const toolId = toolConfig.js || toolName.toLowerCase().replace(/\s+/g, '-') + const toolModule = toolConfig.js || '' + const toolId = toolCanonicalId(toolConfig) // Read metadata from nested object (preferred location) const declaredMetadata = toolConfig.metadata || {} @@ -531,6 +562,7 @@ export function generateToolMetadata(toolConfig) { // Nested metadata takes precedence const rawMetadata = { id: toolId, + module: toolModule, name: toolName, // Icon can be at root as 'defaultIcon' (legacy) or in metadata as 'icon' icon: declaredMetadata.icon || toolConfig.icon || toolConfig.defaultIcon || 'cog', diff --git a/src/essence/Basics/ToolController_/types/tool.ts b/src/essence/Basics/ToolController_/types/tool.ts index 80a72c6d2..82b7fb51b 100644 --- a/src/essence/Basics/ToolController_/types/tool.ts +++ b/src/essence/Basics/ToolController_/types/tool.ts @@ -19,9 +19,12 @@ export type ToolOrientation = (typeof TOOL_ORIENTATION)[keyof typeof TOOL_ORIENT * Layout and UI-related properties should be defined here. */ export interface ToolMetadata { - /** Unique identifier for the tool */ + /** The tool's address: its identity on the bus, in the DOM and in events */ id: string; + /** Binding that reaches the tool's class in the generated tool registry */ + module?: string; + /** Display name */ name: string; diff --git a/src/essence/Tools/AOI/AOITool.js b/src/essence/Tools/AOI/AOITool.js index a68e6ae51..f627313d5 100644 --- a/src/essence/Tools/AOI/AOITool.js +++ b/src/essence/Tools/AOI/AOITool.js @@ -3,7 +3,10 @@ * * Pluggable contract (see specs/012-aoi-plugin/plan.md and PLUGIN-DEVELOPMENT-GUIDE.md): * - * pluginId: 'aoi' + * pluginId: 'aoi' — derived from this plugin's binding at build time. The + * tool controller mints the plugin-scoped bus handle from it and injects it + * as `AOITool.api` before make() runs, which is what prefixes every emit and + * provide below with `plugin:aoi:`. * * Emits (auto-prefixed plugin:aoi:): * - areaDrawn { feature, source: 'search'|'draw'|'upload'|'inspect' } @@ -19,9 +22,9 @@ * - map:drawstart / drawvertex / * drawcomplete / drawcancel (engine bus) * - map:featureClick (inspect-mode boundary clicks, filtered by layerId) - * - plugin:fetch-stats:analysisProgress { done, total } - * - plugin:fetch-stats:analysisReady { analysisData } - * - plugin:fetch-stats:analysisSkipped { reason } + * - plugin:fetchstats:analysisProgress { done, total } + * - plugin:fetchstats:analysisReady { analysisData } + * - plugin:fetchstats:analysisSkipped { reason } * * Requests: * - map:createLayer / map:removeLayer @@ -52,8 +55,7 @@ import { } from './aoiHelpers' import { loadBoundaries } from './aoiBoundaryLoader' -// ── Plugin identity / layer ids ──────────────────────────────────────────────── -const PLUGIN_ID = 'aoi' +// ── Draw shapes / layer ids ─────────────────────────────────────────────────── const DEFAULT_DRAW_SHAPES = ['polygon', 'rectangle', 'circle'] const VALID_DRAW_SHAPES = new Set(['point', 'linestring', 'polygon', 'rectangle', 'circle']) const SELECTION_LAYER_ID = 'aoi:selection' @@ -137,7 +139,6 @@ const AOITool = { _reactRoot: null, _state: initialState(), _cleanups: [], - _api: null, _analysisErrorTimeout: null, _drawKeyHandler: null, @@ -152,13 +153,13 @@ const AOITool = { } this._reactRoot = createRoot(container) - this._api = - (typeof window !== 'undefined' && window.mmgisAPI?.forPlugin?.(PLUGIN_ID)) || - { emit: () => { }, provide: () => () => { } } - - this._cleanups.push( - this._api.provide('getCurrentSelection', () => this._state.currentAOI) + // The controller minted this tool's bus handle and injected it before + // make() ran; handing it back is the controller's job, not destroy()'s. + const offSelection = this.api?.provide( + 'getCurrentSelection', + () => this._state.currentAOI ) + if (offSelection) this._cleanups.push(offSelection) this._state.searchLoading = true loadBoundaries() @@ -188,7 +189,7 @@ const AOITool = { subscribe('map:drawcomplete', (e) => this._onDrawComplete(e)) subscribe('map:drawcancel', () => this._onDrawCancelEvent()) subscribe('map:featureClick', (info) => this._onMapFeatureClick(info)) - subscribe('plugin:fetch-stats:analysisProgress', ({ done, total }) => { + subscribe('plugin:fetchstats:analysisProgress', ({ done, total }) => { if (done === 0) { this._setState({ analysisStatus: 'running', @@ -200,10 +201,10 @@ const AOITool = { this._setState({ analysisDone: done }) } }) - subscribe('plugin:fetch-stats:analysisReady', () => { + subscribe('plugin:fetchstats:analysisReady', () => { this._setState({ analysisStatus: 'idle' }) }) - subscribe('plugin:fetch-stats:analysisSkipped', ({ reason } = {}) => { + subscribe('plugin:fetchstats:analysisSkipped', ({ reason } = {}) => { this._showAnalysisError(this._messageForSkipReason(reason)) }) } @@ -242,7 +243,6 @@ const AOITool = { this.targetId = null this._state = initialState() - this._api = null this.made = false }, @@ -265,7 +265,7 @@ const AOITool = { * unset or empty. */ _resolveDrawShapes() { - const raw = this._api?.getVars?.()?.drawShapes + const raw = this.api?.getVars?.()?.drawShapes const list = Array.isArray(raw) ? raw : typeof raw === 'string' @@ -304,7 +304,7 @@ const AOITool = { }, /** - * Map a `plugin:fetch-stats:analysisSkipped.reason` to a user-facing message. + * Map a `plugin:fetchstats:analysisSkipped.reason` to a user-facing message. * Unknown reasons get a generic fallback. */ _messageForSkipReason(reason) { @@ -370,7 +370,7 @@ const AOITool = { }, _onClose() { - mmgisSetPluginState('AOITool', 'unloaded') + mmgisSetPluginState('aoi', 'unloaded') .then((result) => { if (!result.ok) { console.warn(`[AOI] unload refused: ${result.reason}`) @@ -581,7 +581,7 @@ const AOITool = { }).catch((err) => console.warn('[AOI] failed to add selection layer', err)) this._state.currentAOI = { feature, source, label } - this._api?.emit('areaDrawn', { feature, source }) + this.api?.emit('areaDrawn', { feature, source }) const c = featureCentroid(feature) // `view` keeps the tooltip on-screen when the camera does not move; omit @@ -649,7 +649,7 @@ const AOITool = { this._removeSelectionLayer() this._hideTooltip() this._state.currentAOI = null - this._api?.emit('drawingCleared', {}) + this.api?.emit('drawingCleared', {}) this._render() }, @@ -697,12 +697,12 @@ const AOITool = { _onAnalyze() { const aoi = this._state.currentAOI if (!aoi) return - this._api?.emit('analysisAOIReady', { feature: aoi.feature }) + this.api?.emit('analysisAOIReady', { feature: aoi.feature }) this._hideTooltip() }, _onCancel() { - this._api?.emit('drawingCancelled', {}) + this.api?.emit('drawingCancelled', {}) this._clearSelection() }, diff --git a/src/essence/Tools/AddTempLayer/MMGISAddTempLayerAdapter.tsx b/src/essence/Tools/AddTempLayer/MMGISAddTempLayerAdapter.tsx index 18aa4ab07..dcf97fb1a 100644 --- a/src/essence/Tools/AddTempLayer/MMGISAddTempLayerAdapter.tsx +++ b/src/essence/Tools/AddTempLayer/MMGISAddTempLayerAdapter.tsx @@ -14,7 +14,8 @@ import { * Submits become `layers:addLayer` requests (session-only add; lost on reload). */ -const TOOL_ID = 'AddTempLayerTool' +/** This plugin's address, derived from its binding at build time. */ +const TOOL_ID = 'addtemplayer' /** * Dismisses this tool, reporting a refusal rather than dropping it: a silent diff --git a/src/essence/Tools/AddTempLayer/__tests__/MMGISAddTempLayerAdapter.spec.tsx b/src/essence/Tools/AddTempLayer/__tests__/MMGISAddTempLayerAdapter.spec.tsx index 0d572ab14..d42ed7333 100644 --- a/src/essence/Tools/AddTempLayer/__tests__/MMGISAddTempLayerAdapter.spec.tsx +++ b/src/essence/Tools/AddTempLayer/__tests__/MMGISAddTempLayerAdapter.spec.tsx @@ -9,7 +9,7 @@ import { mount, click } from '../../_shared/__tests__/reactHarness' * a closed form sitting over the map, without an error anyone would notice. */ -const TOOL_ID = 'AddTempLayerTool' +const TOOL_ID = 'addtemplayer' let request: ReturnType diff --git a/src/essence/Tools/AddTempLayer/config.json b/src/essence/Tools/AddTempLayer/config.json index 76408b2c2..1d02e35c6 100644 --- a/src/essence/Tools/AddTempLayer/config.json +++ b/src/essence/Tools/AddTempLayer/config.json @@ -5,7 +5,7 @@ "defaultIcon": "add", "description": "Add an external layer (WMS/WMTS/XYZ/GeoJSON) to the map for the current session.", "descriptionFull": { - "title": "An 'Add layer from URL' form in a floating panel over the map. It starts hidden — reveal it with the core action 'plugins:show:AddTempLayerTool'; its close button hides it again. Added layers are session-only and lost on reload." + "title": "An 'Add layer from URL' form in a floating panel over the map. It starts hidden — reveal it with the core action 'plugins:show:addtemplayer'; its close button hides it again. Added layers are session-only and lost on reload." }, "hasVars": false, "name": "AddTempLayer", diff --git a/src/essence/Tools/Chart/ChartTool.js b/src/essence/Tools/Chart/ChartTool.js index e9edadebc..d00f3a50a 100644 --- a/src/essence/Tools/Chart/ChartTool.js +++ b/src/essence/Tools/Chart/ChartTool.js @@ -2,13 +2,13 @@ * Chart plugin — MMGIS wrapper. * * Receiver-only. Subscribes (at module scope) to - * `plugin:fetch-stats:analysisReady` and renders the per-layer stats payload + * `plugin:fetchstats:analysisReady` and renders the per-layer stats payload * via ChartComponent. * * pluginId: 'chart' * * Listens to (module scope, survives Chart's own mount/unmount): - * - plugin:fetch-stats:analysisReady { analysisData: { [layerName]: } } + * - plugin:fetchstats:analysisReady { analysisData: { [layerName]: } } * - plugin:aoi:analysisAOIReady { feature } — clears stale data * when a new analysis starts * @@ -34,7 +34,7 @@ let _subscribed = false function _onAnalysisReady(payload) { _latestAnalysisData = payload?.analysisData ?? null if (_instance && _instance._reactRoot) _instance._render() - mmgisShowPlugin('ChartTool') + mmgisShowPlugin('chart') .then((result) => { if (!result.ok) { console.warn(`[Chart] showPlugin refused: ${result.reason}`) @@ -55,7 +55,7 @@ function _subscribeBus() { if (_subscribed) return true const api = typeof window !== 'undefined' ? window.mmgisAPI : null if (!api?.on) return false - api.on('plugin:fetch-stats:analysisReady', _onAnalysisReady) + api.on('plugin:fetchstats:analysisReady', _onAnalysisReady) api.on('plugin:aoi:analysisAOIReady', _onAnalysisStart) _subscribed = true return true @@ -124,7 +124,7 @@ const ChartTool = { _onClose() { // Fully unload (not just hide) so a later analysisReady re-mounts a // fresh instance via _onAnalysisReady's showPlugin call. - mmgisSetPluginState('ChartTool', 'unloaded') + mmgisSetPluginState('chart', 'unloaded') .then((result) => { if (!result.ok) { console.warn(`[Chart] unload refused: ${result.reason}`) diff --git a/src/essence/Tools/Chart/__tests__/ChartTool.spec.tsx b/src/essence/Tools/Chart/__tests__/ChartTool.spec.tsx index bc40e39d8..1d9f3cd57 100644 --- a/src/essence/Tools/Chart/__tests__/ChartTool.spec.tsx +++ b/src/essence/Tools/Chart/__tests__/ChartTool.spec.tsx @@ -82,15 +82,15 @@ describe('ChartTool hand-offs', () => { }) test('a result with no panel yet asks the loader for one', async () => { - fire('plugin:fetch-stats:analysisReady', { analysisData: ANALYSIS_DATA }) + fire('plugin:fetchstats:analysisReady', { analysisData: ANALYSIS_DATA }) expect(requested('plugins:show')).toEqual([ - { name: 'plugins:show', params: { pluginId: 'ChartTool' } }, + { name: 'plugins:show', params: { pluginId: 'chart' } }, ]) }) test('the panel it opens renders the result it was handed', async () => { - fire('plugin:fetch-stats:analysisReady', { analysisData: ANALYSIS_DATA }) + fire('plugin:fetchstats:analysisReady', { analysisData: ANALYSIS_DATA }) await open() expect(chartProps.analysisData).toEqual(ANALYSIS_DATA) @@ -99,7 +99,7 @@ describe('ChartTool hand-offs', () => { test('a result reaches an open panel in place', async () => { await open() - fire('plugin:fetch-stats:analysisReady', { analysisData: ANALYSIS_DATA }) + fire('plugin:fetchstats:analysisReady', { analysisData: ANALYSIS_DATA }) expect(chartProps.analysisData).toEqual(ANALYSIS_DATA) }) @@ -108,14 +108,14 @@ describe('ChartTool hand-offs', () => { await open() const before = requested('plugins:show').length - fire('plugin:fetch-stats:analysisReady', { analysisData: ANALYSIS_DATA }) + fire('plugin:fetchstats:analysisReady', { analysisData: ANALYSIS_DATA }) expect(requested('plugins:show').length).toBe(before + 1) }) test('a new analysis clears stale results without opening the panel', async () => { await open() - fire('plugin:fetch-stats:analysisReady', { analysisData: ANALYSIS_DATA }) + fire('plugin:fetchstats:analysisReady', { analysisData: ANALYSIS_DATA }) const before = requested('plugins:show').length fire('plugin:aoi:analysisAOIReady', { feature: {} }) @@ -125,7 +125,7 @@ describe('ChartTool hand-offs', () => { }) test('closing unloads the plugin so the next result re-mounts a panel', async () => { - fire('plugin:fetch-stats:analysisReady', { analysisData: ANALYSIS_DATA }) + fire('plugin:fetchstats:analysisReady', { analysisData: ANALYSIS_DATA }) await open() await act(async () => { @@ -135,7 +135,7 @@ describe('ChartTool hand-offs', () => { expect(requested('plugins:setState')).toEqual([ { name: 'plugins:setState', - params: { pluginId: 'ChartTool', state: 'unloaded' }, + params: { pluginId: 'chart', state: 'unloaded' }, }, ]) }) diff --git a/src/essence/Tools/Chart/config.json b/src/essence/Tools/Chart/config.json index b269e7f84..5b44447ad 100644 --- a/src/essence/Tools/Chart/config.json +++ b/src/essence/Tools/Chart/config.json @@ -5,7 +5,7 @@ "defaultIcon": "chart-bar", "description": "Render per-layer analysis results published by the FetchStats plugin.", "descriptionFull": { - "title": "Subscribes to plugin:fetch-stats:analysisReady on the mmgisAPI Event Bus and renders one card per (layer, asset) with a headline mean, histogram, and stats grid. No per-layer configuration required — analysis-supported layers are configured on the AOI plugin side, and FetchStats runs the actual statistics requests.", + "title": "Subscribes to plugin:fetchstats:analysisReady on the mmgisAPI Event Bus and renders one card per (layer, asset) with a headline mean, histogram, and stats grid. No per-layer configuration required — analysis-supported layers are configured on the AOI plugin side, and FetchStats runs the actual statistics requests.", "example": {} }, "hasVars": false, diff --git a/src/essence/Tools/Comparison/ComparisonTool.tsx b/src/essence/Tools/Comparison/ComparisonTool.tsx index 7211ccb91..0bbf102ce 100644 --- a/src/essence/Tools/Comparison/ComparisonTool.tsx +++ b/src/essence/Tools/Comparison/ComparisonTool.tsx @@ -35,7 +35,8 @@ import { mmgisShowPlugin, } from '../_shared/adapters/mmgisAPI' -const PLUGIN_ID = 'ComparisonTool' +/** This plugin's address, derived from its binding at build time. */ +const PLUGIN_ID = 'comparison' // ── Module-level state ──────────────────────────────────────────────────────── // A hand-off can fire before make() has ever run, so the bus listeners live at diff --git a/src/essence/Tools/Comparison/__tests__/ComparisonTool.spec.tsx b/src/essence/Tools/Comparison/__tests__/ComparisonTool.spec.tsx index 57a66872c..94837bf90 100644 --- a/src/essence/Tools/Comparison/__tests__/ComparisonTool.spec.tsx +++ b/src/essence/Tools/Comparison/__tests__/ComparisonTool.spec.tsx @@ -86,7 +86,7 @@ describe('ComparisonTool hand-offs', () => { fire('plugin:comparison:startWithDates') expect(requested('plugins:show')).toEqual([ - { name: 'plugins:show', params: { pluginId: 'ComparisonTool' } }, + { name: 'plugins:show', params: { pluginId: 'comparison' } }, ]) }) @@ -149,7 +149,7 @@ describe('ComparisonTool hand-offs', () => { expect(requested('plugins:setState')).toEqual([ { name: 'plugins:setState', - params: { pluginId: 'ComparisonTool', state: 'unloaded' }, + params: { pluginId: 'comparison', state: 'unloaded' }, }, ]) }) diff --git a/src/essence/Tools/FetchStats/FetchStatsTool.js b/src/essence/Tools/FetchStats/FetchStatsTool.js index 50ab4d3cf..26367c1a1 100644 --- a/src/essence/Tools/FetchStats/FetchStatsTool.js +++ b/src/essence/Tools/FetchStats/FetchStatsTool.js @@ -1,12 +1,14 @@ /** * FetchStats plugin — no-UI background plugin. * - * pluginId: 'fetch-stats' + * pluginId: 'fetchstats' — derived from this plugin's binding at build time. + * The tool controller mints the bus handle from it and injects it as + * `FetchStatsTool.api` before initialize() runs. * * Listens to: * - plugin:aoi:analysisAOIReady { feature } * - * Emits (auto-prefixed plugin:fetch-stats:): + * Emits (auto-prefixed plugin:fetchstats:): * - analysisProgress { done: number, total: number } * - analysisReady { analysisData: { [layerDisplayName]: } } * - analysisSkipped { reason: 'no-eligible-layers' } @@ -16,8 +18,6 @@ * message to the user. */ -const PLUGIN_ID = 'fetch-stats' - /** * Build the statistics POST URL for a layer's `variables.analysis` block. * `assets` and `bidx` repeat; `nodata` is set when present. @@ -35,7 +35,6 @@ const FetchStatsTool = { height: 0, width: 0, MMGISInterface: null, - _api: null, _cleanups: [], made: false, @@ -49,10 +48,6 @@ const FetchStatsTool = { if (this.made) return this.made = true this.MMGISInterface = new interfaceWithMMGIS(this, targetId) - this._api = - (typeof window !== 'undefined' && - window.mmgisAPI?.forPlugin?.(PLUGIN_ID)) || - {} const api = window.mmgisAPI if (api?.on) { @@ -73,9 +68,6 @@ const FetchStatsTool = { this._cleanups.forEach((fn) => fn()) this._cleanups = [] this.MMGISInterface?.separateFromMMGIS() - // An analysis already awaiting the network cannot be cancelled, so - // dropping the handle is what keeps its remaining emits off the bus. - this._api = null }, async _getAnalyzableVisibleLayers() { @@ -99,13 +91,22 @@ const FetchStatsTool = { async _runAnalysisForVisibleLayers(feature) { if (!feature?.geometry || !window.mmgisAPI?.request) return + // The tool controller injects this plugin's bus handle before its own + // code runs and clears it once teardown releases it. The classic + // layout has no controller to inject one at all. An analysis already + // awaiting the network cannot be cancelled, so in both cases the run + // finishes and reports its results to nobody. + const emit = (event, data) => { + if (this.api) this.api.emit(event, data) + } + const layers = await this._getAnalyzableVisibleLayers() if (!layers.length) { - this._api?.emit('analysisSkipped', { reason: 'no-eligible-layers' }) + emit('analysisSkipped', { reason: 'no-eligible-layers' }) return } - this._api?.emit('analysisProgress', { done: 0, total: layers.length }) + emit('analysisProgress', { done: 0, total: layers.length }) const body = JSON.stringify({ type: 'Feature', @@ -119,13 +120,13 @@ const FetchStatsTool = { const displayName = layer.display_name || layer.name const result = await this._postStatsForLayer(layer, body) done += 1 - this._api?.emit('analysisProgress', { done, total: layers.length }) + emit('analysisProgress', { done, total: layers.length }) return [displayName, result] }) ) const analysisData = Object.fromEntries(entries) - this._api?.emit('analysisReady', { analysisData }) + emit('analysisReady', { analysisData }) }, async _postStatsForLayer(layer, body) { diff --git a/src/essence/Tools/FetchStats/__tests__/fetchStatsLifecycle.spec.js b/src/essence/Tools/FetchStats/__tests__/fetchStatsLifecycle.spec.js index ec84ad10c..f6ac8b776 100644 --- a/src/essence/Tools/FetchStats/__tests__/fetchStatsLifecycle.spec.js +++ b/src/essence/Tools/FetchStats/__tests__/fetchStatsLifecycle.spec.js @@ -34,12 +34,14 @@ const flush = () => new Promise((resolve) => setTimeout(resolve)) let subscriptions let handlers let emit +let release let deferConfig beforeEach(() => { subscriptions = [] handlers = {} emit = vi.fn() + release = vi.fn() deferConfig = null window.mmgisAPI = { on: (event, handler) => { @@ -47,7 +49,6 @@ beforeEach(() => { handlers[event] = handler return vi.fn() }, - forPlugin: () => ({ emit }), request: (name) => { if (name === 'layers:getVisible') return Promise.resolve({ uuid: true }) if (name === 'layers:getAll') return Promise.resolve(['uuid']) @@ -58,10 +59,21 @@ beforeEach(() => { }) }, } + // Stands in for the handle the tool controller mints and injects before + // initialize() runs. + FetchStatsTool.api = { + address: 'fetchstats', + on: vi.fn(() => vi.fn()), + emit, + provide: vi.fn(() => vi.fn()), + request: vi.fn(() => Promise.resolve(null)), + release, + } }) afterEach(() => { FetchStatsTool.destroy() + FetchStatsTool.api = null delete window.mmgisAPI vi.unstubAllGlobals() vi.restoreAllMocks() @@ -75,7 +87,7 @@ test('the classic layout subscribes from initialize() alone', () => { test('the modern layout, calling both start hooks, still subscribes once', () => { FetchStatsTool.initialize() - FetchStatsTool.make('fetch-stats-target') + FetchStatsTool.make('fetchstats-target') expect(subscriptions).toEqual([AOI_READY]) }) @@ -102,12 +114,18 @@ test('an analysis resolving after teardown announces nothing', async () => { handlers[AOI_READY]({ feature: AOI }) await flush() + // What the tool controller does on teardown: destroy(), then release the + // handle and clear it off the instance. FetchStatsTool.destroy() + FetchStatsTool.api.release() + FetchStatsTool.api = null + deferConfig(ANALYSIS_LAYER) await flush() + expect(release).toHaveBeenCalled() expect(emit).not.toHaveBeenCalled() - // Reaching a dropped handle through a bare .emit throws, and the + // Reaching a cleared handle through a bare .emit throws, and the // subscription's catch would log that away instead of failing the // assertion above. expect(warn).not.toHaveBeenCalled() diff --git a/src/essence/Tools/LayerFilterThemes/MMGISThemeRailAdapter.tsx b/src/essence/Tools/LayerFilterThemes/MMGISThemeRailAdapter.tsx index 1410826bb..c09cd328d 100644 --- a/src/essence/Tools/LayerFilterThemes/MMGISThemeRailAdapter.tsx +++ b/src/essence/Tools/LayerFilterThemes/MMGISThemeRailAdapter.tsx @@ -18,10 +18,9 @@ import type { ThemeSummary } from './lib/types' // replacement panel can subscribe without inheriting the old panel's name. const SELECTED_THEME_EVENT = 'plugin:layerfilterthemes:selectedThemeChanged' -// How the layout identifies this plugin — the `js` id a mission config gives -// the tool, which is what a panel lists in `toolIds`. Distinct from the -// lowercased tool name `tool:getVars` is keyed by. -const TOOL_ID = 'LayerFilterThemesTool' +// How the layout identifies this plugin — its address, derived from its +// binding at build time, which is what a panel lists in `toolIds`. +const TOOL_ID = 'layerfilterthemes' type ThemeRailVars = { themes?: unknown diff --git a/src/essence/Tools/LayerFilterThemes/__tests__/MMGISThemeRailAdapter.spec.tsx b/src/essence/Tools/LayerFilterThemes/__tests__/MMGISThemeRailAdapter.spec.tsx index 227a76ded..f414226b4 100644 --- a/src/essence/Tools/LayerFilterThemes/__tests__/MMGISThemeRailAdapter.spec.tsx +++ b/src/essence/Tools/LayerFilterThemes/__tests__/MMGISThemeRailAdapter.spec.tsx @@ -24,7 +24,7 @@ const RAIL_PANEL = { state: 'expanded', // The `js` id a mission config gives the tool, which is what the layout // lists — not the lowercased name `tool:getVars` is keyed by. - toolIds: ['LayerFilterThemesTool'], + toolIds: ['layerfilterthemes'], } const NEIGHBOUR = { id: 'filters', diff --git a/src/essence/Tools/LayerManager/__tests__/handlers.spec.js b/src/essence/Tools/LayerManager/__tests__/handlers.spec.js index 241163e09..692e6829d 100644 --- a/src/essence/Tools/LayerManager/__tests__/handlers.spec.js +++ b/src/essence/Tools/LayerManager/__tests__/handlers.spec.js @@ -287,7 +287,7 @@ test.describe('handlers', () => { await flush() expect(requests).toEqual([ - { name: 'plugins:show', params: { pluginId: 'AddTempLayerTool' } }, + { name: 'plugins:show', params: { pluginId: 'addtemplayer' } }, ]) expect(emitCalls).toHaveLength(0) }) diff --git a/src/essence/Tools/LayerManager/adapters/handlers.ts b/src/essence/Tools/LayerManager/adapters/handlers.ts index a43a4438c..28375e72a 100644 --- a/src/essence/Tools/LayerManager/adapters/handlers.ts +++ b/src/essence/Tools/LayerManager/adapters/handlers.ts @@ -74,7 +74,8 @@ export const compareLayer = (layerId: string): void => { mmgisEmit('plugin:comparison:startWithLayer', { layerId }) } -export const ADD_LAYER_PLUGIN_ID = 'AddTempLayerTool' +/** AddTempLayer's address, derived from that plugin's binding at build time. */ +export const ADD_LAYER_PLUGIN_ID = 'addtemplayer' /** Reveals the "add layer from URL" form. */ export const showAddLayer = (): void => { diff --git a/src/essence/Tools/MODERN_TOOL_PATTERN.md b/src/essence/Tools/MODERN_TOOL_PATTERN.md index 3847b8a98..7445d7907 100644 --- a/src/essence/Tools/MODERN_TOOL_PATTERN.md +++ b/src/essence/Tools/MODERN_TOOL_PATTERN.md @@ -168,3 +168,5 @@ If your tool requires specific placement options, add a `metadata` object to you } } ``` + +The `paths` key names the generated import binding and, with a trailing `Tool` dropped and the rest lowercased, the tool's address (`AOITool` → `aoi`). That address is the tool's identity everywhere but the import — the key its configured `variables` resolve under, the target `plugins:show:aoi` takes, the prefix on the bus handle the controller injects as `this.api` (`plugin:aoi:ready`), and the `pluginId` its teardown announces. diff --git a/src/essence/Tools/Title/config.json b/src/essence/Tools/Title/config.json index 376e643f6..3ca29f9bc 100644 --- a/src/essence/Tools/Title/config.json +++ b/src/essence/Tools/Title/config.json @@ -69,7 +69,7 @@ { "field": "variables.actionButtonLink", "name": "Button Link", - "description": "An external http(s) URL (opened in a new tab); one of 'panels:show:', 'panels:hide:', 'plugins:show:', 'plugins:hide:' (e.g. 'panels:hide:left-panel', 'plugins:show:DrawTool'); or a custom event name, which is emitted as-is. Event names should be namespaced (e.g. 'plugin:title:refresh'). Leave empty to hide the button.", + "description": "An external http(s) URL (opened in a new tab); one of 'panels:show:', 'panels:hide:', 'plugins:show:', 'plugins:hide:' (e.g. 'panels:hide:left-panel', 'plugins:show:draw'); or a custom event name, which is emitted as-is. Event names should be namespaced (e.g. 'plugin:title:refresh'). Leave empty to hide the button.", "type": "text", "width": 6 } diff --git a/src/essence/mmgisAPI/mmgisAPI.js b/src/essence/mmgisAPI/mmgisAPI.js index e18e810f2..64968d851 100644 --- a/src/essence/mmgisAPI/mmgisAPI.js +++ b/src/essence/mmgisAPI/mmgisAPI.js @@ -995,17 +995,6 @@ var mmgisAPI = { return handlers.has(name) }, - // ============ PLUGIN-SCOPED API ============ - - /** - * Get a plugin's bus handle by address. See `mintHandle`. - * @param {string} address - Plugin address (e.g., 'draw', 'info', 'aoi') - * @returns {Object} The plugin's handle - */ - forPlugin(address) { - return mintHandle(address) - }, - // Formulae_ utils: { ...F_ }, } @@ -1021,6 +1010,8 @@ const readVars = (address) => { * Mint a plugin's bus handle: `emit` and `provide` prefix names with the * plugin's address, `request` takes a full name and is stamped with the token * minted here, and `release()` hands back everything the handle registered. + * Core-internal: the tool controller mints one per tool it loads and injects + * it, so a plugin cannot reach another plugin's handle by naming its address. * @param {string} address - Plugin address (e.g., 'aoi') * @returns {Object} The plugin's handle */ diff --git a/tests/e2e/eventbus-integration.spec.js b/tests/e2e/eventbus-integration.spec.js index b3d1ba8fe..03bbcffbf 100644 --- a/tests/e2e/eventbus-integration.spec.js +++ b/tests/e2e/eventbus-integration.spec.js @@ -366,98 +366,3 @@ test.describe('mmgisAPI Events - Dual Emission Integration', () => { expect(result.allCalled).toBe(true) }) }) - -test.describe('Plugin Scoped API (this.api)', () => { - - test('scoped API has emit and provide methods', async ({ page }) => { - const result = await page.evaluate(() => { - const api = window.mmgisAPI.forPlugin('testPlugin') - return { - hasEmit: typeof api.emit === 'function', - hasProvide: typeof api.provide === 'function', - hasAddress: api.address === 'testPlugin', - hasPrefix: api.prefix === 'plugin:testPlugin:', - } - }) - - expect(result.hasEmit).toBe(true) - expect(result.hasProvide).toBe(true) - expect(result.hasAddress).toBe(true) - expect(result.hasPrefix).toBe(true) - }) - - test('emit auto-prefixes event names', async ({ page }) => { - const result = await page.evaluate(() => { - return new Promise((resolve) => { - let receivedData = null - - // Subscribe using global API with full path - window.mmgisAPI.on('plugin:myPlugin:dataUpdated', (data) => { - receivedData = data - }) - - // Emit using scoped API - auto-prefixed - const api = window.mmgisAPI.forPlugin('myPlugin') - api.emit('dataUpdated', { value: 123 }) - - setTimeout(() => { - resolve({ - received: receivedData !== null, - valueMatches: receivedData?.value === 123, - }) - }, 10) - }) - }) - - expect(result.received).toBe(true) - expect(result.valueMatches).toBe(true) - }) - - test('provide auto-prefixes handler names', async ({ page }) => { - const result = await page.evaluate(async () => { - const api = window.mmgisAPI.forPlugin('dataPlugin') - - // Provide using scoped API - auto-prefixed - api.provide('getData', (params) => { - return { result: params.input * 3 } - }) - - // Request using global API with full path - const response = await window.mmgisAPI.request( - 'plugin:dataPlugin:getData', - { input: 14 } - ) - - return { result: response?.result } - }) - - expect(result.result).toBe(42) - }) - - test('cleanup function from scoped provide() removes handler', async ({ - page, - }) => { - const result = await page.evaluate(() => { - const api = window.mmgisAPI.forPlugin('cleanupPlugin') - - // Provide and get cleanup function - const cleanup = api.provide('tempHandler', () => 'temp') - - const existsBefore = window.mmgisAPI.hasHandler( - 'plugin:cleanupPlugin:tempHandler' - ) - - // Cleanup - cleanup() - - const existsAfter = window.mmgisAPI.hasHandler( - 'plugin:cleanupPlugin:tempHandler' - ) - - return { existsBefore, existsAfter } - }) - - expect(result.existsBefore).toBe(true) - expect(result.existsAfter).toBe(false) - }) -}) diff --git a/tests/unit/__mocks__/preTools.js b/tests/unit/__mocks__/preTools.js index 595e4ebfe..99ace2a3c 100644 --- a/tests/unit/__mocks__/preTools.js +++ b/tests/unit/__mocks__/preTools.js @@ -9,4 +9,5 @@ export const Kinds = {} export const toolConfigs = {} export const toolModules = {} +export const toolIds = {} export const testModules = {} diff --git a/tests/unit/aoiDrawKeys.spec.js b/tests/unit/aoiDrawKeys.spec.js index 57d63715d..184ed9565 100644 --- a/tests/unit/aoiDrawKeys.spec.js +++ b/tests/unit/aoiDrawKeys.spec.js @@ -45,6 +45,7 @@ afterEach(() => { AOITool._removeDrawKeys() AOITool._state.isDrawing = false document.body.innerHTML = '' + delete AOITool.api delete window.mmgisAPI }) @@ -192,10 +193,14 @@ test.describe('AOI draw-session keys', () => { handlers[event] = handler return () => delete handlers[event] } - window.mmgisAPI.forPlugin = () => ({ + // The controller injects the plugin-scoped handle before make() runs. + AOITool.api = { + on: () => () => { }, emit: () => { }, provide: () => () => { }, - }) + request: () => Promise.resolve(null), + release: () => { }, + } appendTo(document.body, 'div', { id: 'toolPanel' }) AOITool.make('toolPanel') diff --git a/tests/unit/aoiToolClose.spec.js b/tests/unit/aoiToolClose.spec.js index 0a5156507..92a98e27a 100644 --- a/tests/unit/aoiToolClose.spec.js +++ b/tests/unit/aoiToolClose.spec.js @@ -19,7 +19,7 @@ test('closing the panel unloads the AOI plugin', () => { AOITool._onClose() expect(request).toHaveBeenCalledWith('plugins:setState', { - pluginId: 'AOITool', + pluginId: 'aoi', state: 'unloaded', }) }) diff --git a/tests/unit/chartToolPlugin.spec.js b/tests/unit/chartToolPlugin.spec.js index d56265faf..d36702694 100644 --- a/tests/unit/chartToolPlugin.spec.js +++ b/tests/unit/chartToolPlugin.spec.js @@ -33,11 +33,11 @@ afterEach(() => { test('a fresh analysisReady payload asks the bus to show the Chart plugin', async () => { await import('../../src/essence/Tools/Chart/ChartTool.js') - expect(listeners['plugin:fetch-stats:analysisReady']).toBeTypeOf('function') + expect(listeners['plugin:fetchstats:analysisReady']).toBeTypeOf('function') - listeners['plugin:fetch-stats:analysisReady']({ analysisData: { layerA: {} } }) + listeners['plugin:fetchstats:analysisReady']({ analysisData: { layerA: {} } }) - expect(request).toHaveBeenCalledWith('plugins:show', { pluginId: 'ChartTool' }) + expect(request).toHaveBeenCalledWith('plugins:show', { pluginId: 'chart' }) }) test('closing the panel unloads the Chart plugin', async () => { @@ -46,7 +46,7 @@ test('closing the panel unloads the Chart plugin', async () => { ChartTool._onClose() expect(request).toHaveBeenCalledWith('plugins:setState', { - pluginId: 'ChartTool', + pluginId: 'chart', state: 'unloaded', }) }) diff --git a/tests/unit/getToolVars.spec.js b/tests/unit/getToolVars.spec.js new file mode 100644 index 000000000..389b9152b --- /dev/null +++ b/tests/unit/getToolVars.spec.js @@ -0,0 +1,70 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' + +// Viewer_ pulls in Photosphere/ModelViewer/PDFViewer, which are JSX written in +// .js files that vite's import-analysis can't parse. Nothing here needs the +// real viewers, so stub the aggregator to keep the import chain parseable. +vi.mock('../../src/essence/Basics/Viewer_/Viewer_', () => ({ default: {} })) + +// Stand in the generated registry the build writes, so the lookup resolves +// addresses the way it does in a browser rather than only through the +// frontend's fallback derivation. +vi.mock('../../src/pre/tools', () => ({ + Kinds: {}, + toolConfigs: {}, + toolModules: {}, + toolIds: { FetchStatsTool: 'fetchstats' }, + testModules: {}, +})) + +const { default: L_ } = await import('../../src/essence/Basics/Layers_/Layers_') + +/** + * A tool's variables are configured under its entry in the mission config, but + * the address it is known by everywhere else comes from its module binding. + * Those two only coincide when the configured display name happens to be the + * binding minus its "Tool", so the lookup matches the address first and keeps + * the lowercased name as a fallback. + */ +describe('getToolVars', () => { + beforeEach(() => { + L_.tools = [] + }) + + it('finds a tool by its address, whatever it was named', () => { + L_.tools = [ + { name: 'Statistics', js: 'FetchStatsTool', variables: { url: '/s' } }, + ] + + expect(L_.getToolVars('fetchstats')).toEqual({ url: '/s' }) + }) + + // The fallback is what keeps callers that ask by display name working. + it('still finds a tool by its lowercased name', () => { + L_.tools = [ + { name: 'Layers', js: 'LayersTool', variables: { search: true } }, + ] + + expect(L_.getToolVars('layers')).toEqual({ search: true }) + }) + + // An entry with no module binding is named after itself, hyphenated — the + // same address the controller would mint its handle under. + it('names an entry with no binding after itself', () => { + L_.tools = [{ name: 'Legacy Thing', variables: { b: 2 } }] + + expect(L_.getToolVars('legacy-thing')).toEqual({ b: 2 }) + }) + + // External consumers reach this through the `tool:getVars` provider, which + // hands the marker straight back; only a plugin handle's getVars maps it to + // an empty object. + it('answers a miss with the no-vars marker', () => { + L_.tools = [ + { name: 'Statistics', js: 'FetchStatsTool', variables: { url: '/s' } }, + ] + + expect(L_.getToolVars('nothing-configured')).toEqual({ __noVars: true }) + // The module binding is not an address and never was one. + expect(L_.getToolVars('FetchStatsTool')).toEqual({ __noVars: true }) + }) +}) diff --git a/tests/unit/mmgisApiCallerStamp.spec.js b/tests/unit/mmgisApiCallerStamp.spec.js index 9a370d063..17d25abe1 100644 --- a/tests/unit/mmgisApiCallerStamp.spec.js +++ b/tests/unit/mmgisApiCallerStamp.spec.js @@ -5,7 +5,7 @@ import { describe, it, expect, afterEach, vi } from 'vitest' // real viewers, so stub the aggregator to keep the import chain parseable. vi.mock('../../src/essence/Basics/Viewer_/Viewer_', () => ({ default: {} })) -import { mmgisAPI } from '../../src/essence/mmgisAPI/mmgisAPI' +import { mmgisAPI, mintHandle } from '../../src/essence/mmgisAPI/mmgisAPI' const cleanups = [] @@ -40,7 +40,7 @@ describe('the caller a request arrives with', () => { // any shape through unchanged — an object, a scalar, or none at all. it('is the address of the handle the request went through', async () => { const calls = recorder('test:stamped') - api = mmgisAPI.forPlugin('aoi') + api = mintHandle('aoi') await api.request('test:stamped', { a: 1 }) await api.request('test:stamped', 'some text') diff --git a/tests/unit/pluginHandleRelease.spec.js b/tests/unit/pluginHandleRelease.spec.js index 266505bc1..784b3c66b 100644 --- a/tests/unit/pluginHandleRelease.spec.js +++ b/tests/unit/pluginHandleRelease.spec.js @@ -5,7 +5,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' // real viewers, so stub the aggregator to keep the import chain parseable. vi.mock('../../src/essence/Basics/Viewer_/Viewer_', () => ({ default: {} })) -import { mmgisAPI } from '../../src/essence/mmgisAPI/mmgisAPI' +import { mmgisAPI, mintHandle } from '../../src/essence/mmgisAPI/mmgisAPI' import L_ from '../../src/essence/Basics/Layers_/Layers_' // Issue #414 - a handle registers providers on a bus that outlives the plugin @@ -21,7 +21,7 @@ describe('plugin handle release', () => { // getToolVars walks the configured tool list; an empty one is the // no-vars case every unconfigured plugin is in. L_.tools = [] - api = mmgisAPI.forPlugin('rel-test') + api = mintHandle('rel-test') }) // The bus is a module singleton, so a handle left registered would answer @@ -59,7 +59,7 @@ describe('plugin handle release', () => { // name alone would unregister the live successor here. it('a stale release leaves a successor holding the same name alone', async () => { api.provide('answer', () => 1) - next = mmgisAPI.forPlugin('rel-test') + next = mintHandle('rel-test') next.provide('answer', () => 42) api.release() diff --git a/tests/unit/providers/busReconciliation.spec.js b/tests/unit/providers/busReconciliation.spec.js index 1ffe9a474..4102641e8 100644 --- a/tests/unit/providers/busReconciliation.spec.js +++ b/tests/unit/providers/busReconciliation.spec.js @@ -1,8 +1,10 @@ import { test, expect, beforeEach, afterEach, vi } from 'vitest' vi.mock('../../../src/essence/Basics/Viewer_/Viewer_', () => ({ default: {} })) // Reaching 'hidden' from 'unloaded' loads the plugin first, and loadPlugin -// looks tool ids up in the real registry; stand in a minimal module. +// reaches its class through the module binding in the real registry; stand in +// a minimal module. vi.mock('../../../src/pre/tools', () => ({ + toolIds: {}, toolModules: { ReconcileTool: { make: () => {}, destroy: () => {} } }, })) @@ -73,12 +75,12 @@ test('the client subscribes to the plugin event core actually emits', () => { try { ToolControllerModern_.registerDeferred( - { id: 'ReconcileTool', name: 'Reconcile' }, 'reconcile-target' + { id: 'reconcile', module: 'ReconcileTool', name: 'Reconcile' }, 'reconcile-target' ) - ToolControllerModern_.setPluginState('ReconcileTool', 'hidden') + ToolControllerModern_.setPluginState('reconcile', 'hidden') expect(seen).toHaveLength(1) - expect(seen[0]).toContainEqual({ id: 'ReconcileTool', state: 'hidden' }) + expect(seen[0]).toContainEqual({ id: 'reconcile', state: 'hidden' }) } finally { off() ToolControllerModern_.destroyAllTools() diff --git a/tests/unit/providers/plugins.spec.js b/tests/unit/providers/plugins.spec.js index 2fad5cd8d..b7274cb6f 100644 --- a/tests/unit/providers/plugins.spec.js +++ b/tests/unit/providers/plugins.spec.js @@ -1,10 +1,12 @@ import { test, expect, beforeEach, afterEach, vi } from 'vitest' vi.mock('../../../src/essence/Basics/Viewer_/Viewer_', () => ({ default: {} })) -// loadPlugin looks tool ids up in the real tool registry; stand in a minimal -// module so plugins:show has something to load. Its destroy is a spy: whether -// a tool tore itself down is not visible in the plugin listing. +// loadPlugin reaches a tool's class through its module binding in the real +// tool registry; stand in a minimal module so plugins:show has something to +// load. Its destroy is a spy: whether a tool tore itself down is not visible +// in the plugin listing. const { destroySpy } = vi.hoisted(() => ({ destroySpy: vi.fn() })) vi.mock('../../../src/pre/tools', () => ({ + toolIds: {}, toolModules: { FakeTool: { make: () => {}, destroy: destroySpy } }, })) @@ -14,7 +16,10 @@ import ToolControllerModern_ from '../../../src/essence/Basics/ToolController_/T beforeEach(() => { destroySpy.mockClear() document.body.innerHTML = '
' - ToolControllerModern_.registerDeferred({ id: 'FakeTool', name: 'Fake' }, 'fake-target') + // 'FakeTool' reaches the module, 'fake' is the address commands name it by. + ToolControllerModern_.registerDeferred( + { id: 'fake', module: 'FakeTool', name: 'Fake' }, 'fake-target' + ) mmgisAPI_._pluginController = ToolControllerModern_ }) @@ -37,7 +42,7 @@ test('all four handlers are registered at module load', () => { test('getAll returns the public projection', async () => { const plugins = await mmgisAPI.request('plugins:getAll') - expect(plugins).toContainEqual({ id: 'FakeTool', state: 'unloaded' }) + expect(plugins).toContainEqual({ id: 'fake', state: 'unloaded' }) }) test('a malformed payload is a bad-request', async () => { @@ -45,12 +50,12 @@ test('a malformed payload is a bad-request', async () => { expect(await mmgisAPI.request('plugins:hide', {})).toEqual({ ok: false, reason: 'bad-request' }) expect(await mmgisAPI.request('plugins:hide', { pluginId: 42 })) .toEqual({ ok: false, reason: 'bad-request' }) - expect(await mmgisAPI.request('plugins:setState', { pluginId: 'FakeTool' })) + expect(await mmgisAPI.request('plugins:setState', { pluginId: 'fake' })) .toEqual({ ok: false, reason: 'bad-request' }) }) test('setState with a well-formed but unrecognised state is a bad-request', async () => { - expect(await mmgisAPI.request('plugins:setState', { pluginId: 'FakeTool', state: 'sideways' })) + expect(await mmgisAPI.request('plugins:setState', { pluginId: 'fake', state: 'sideways' })) .toEqual({ ok: false, reason: 'bad-request' }) }) @@ -62,47 +67,47 @@ test('an unknown plugin is not-found', async () => { test('without a controller the layout is inactive', async () => { mmgisAPI_._pluginController = null expect(await mmgisAPI.request('plugins:getAll')).toEqual([]) - expect(await mmgisAPI.request('plugins:hide', { pluginId: 'FakeTool' })) + expect(await mmgisAPI.request('plugins:hide', { pluginId: 'fake' })) .toEqual({ ok: false, reason: 'layout-inactive' }) }) test('show maps to visible and hide maps to hidden', async () => { - expect(await mmgisAPI.request('plugins:show', { pluginId: 'FakeTool' })) + expect(await mmgisAPI.request('plugins:show', { pluginId: 'fake' })) .toEqual({ ok: true, state: 'visible', changed: true }) - expect(await mmgisAPI.request('plugins:hide', { pluginId: 'FakeTool' })) + expect(await mmgisAPI.request('plugins:hide', { pluginId: 'fake' })) .toEqual({ ok: true, state: 'hidden', changed: true }) }) test('a hidden plugin can be shown again', async () => { // The instance survives hiding, so restoring it is a reveal rather than a // fresh load. - await mmgisAPI.request('plugins:show', { pluginId: 'FakeTool' }) - await mmgisAPI.request('plugins:hide', { pluginId: 'FakeTool' }) + await mmgisAPI.request('plugins:show', { pluginId: 'fake' }) + await mmgisAPI.request('plugins:hide', { pluginId: 'fake' }) const card = document.querySelector('#fake-target') expect(card.classList.contains('plugin-hidden')).toBe(true) - expect(await mmgisAPI.request('plugins:show', { pluginId: 'FakeTool' })) + expect(await mmgisAPI.request('plugins:show', { pluginId: 'fake' })) .toEqual({ ok: true, state: 'visible', changed: true }) expect(card.classList.contains('plugin-hidden')).toBe(false) expect(await mmgisAPI.request('plugins:getAll')) - .toContainEqual({ id: 'FakeTool', state: 'visible' }) + .toContainEqual({ id: 'fake', state: 'visible' }) expect(destroySpy).not.toHaveBeenCalled() }) test('setState forwards a valid state and returns the controller result unmodified', async () => { - expect(await mmgisAPI.request('plugins:setState', { pluginId: 'FakeTool', state: 'visible' })) + expect(await mmgisAPI.request('plugins:setState', { pluginId: 'fake', state: 'visible' })) .toEqual({ ok: true, state: 'visible', changed: true }) }) test('setState can unload a loaded plugin, tearing down its instance', async () => { - await mmgisAPI.request('plugins:setState', { pluginId: 'FakeTool', state: 'visible' }) + await mmgisAPI.request('plugins:setState', { pluginId: 'fake', state: 'visible' }) - expect(await mmgisAPI.request('plugins:setState', { pluginId: 'FakeTool', state: 'unloaded' })) + expect(await mmgisAPI.request('plugins:setState', { pluginId: 'fake', state: 'unloaded' })) .toEqual({ ok: true, state: 'unloaded', changed: true }) const plugins = await mmgisAPI.request('plugins:getAll') - expect(plugins).toContainEqual({ id: 'FakeTool', state: 'unloaded' }) + expect(plugins).toContainEqual({ id: 'fake', state: 'unloaded' }) // The listing reports bookkeeping; the tool's own teardown is what // releases its DOM and listeners. expect(destroySpy).toHaveBeenCalled() @@ -114,8 +119,8 @@ test('a refused command logs nothing', async () => { await mmgisAPI.request('plugins:hide') await mmgisAPI.request('plugins:hide', { pluginId: 'Ghost' }) - await mmgisAPI.request('plugins:setState', { pluginId: 'FakeTool', state: 42 }) - await mmgisAPI.request('plugins:setState', { pluginId: 'FakeTool', state: 'sideways' }) + await mmgisAPI.request('plugins:setState', { pluginId: 'fake', state: 42 }) + await mmgisAPI.request('plugins:setState', { pluginId: 'fake', state: 'sideways' }) expect(warn).not.toHaveBeenCalled() expect(error).not.toHaveBeenCalled() diff --git a/tests/unit/toolControllerModern.pluginState.spec.js b/tests/unit/toolControllerModern.pluginState.spec.js index 0210dd315..84d52267c 100644 --- a/tests/unit/toolControllerModern.pluginState.spec.js +++ b/tests/unit/toolControllerModern.pluginState.spec.js @@ -1,16 +1,20 @@ import { test, expect, vi, beforeEach, afterEach } from 'vitest' vi.mock('../../src/essence/Basics/Viewer_/Viewer_', () => ({ default: {} })) -// loadPlugin looks tool ids up in the real tool registry; stand in a minimal -// module so the load-then-show/hide paths in setPluginState have something to -// load. 'GhostTool' is deliberately absent so a test can force a load failure. +// loadPlugin reaches a tool's class through its module binding in the real +// tool registry; stand in a minimal module so the load-then-show/hide paths in +// setPluginState have something to load. 'GhostTool' is deliberately absent so +// a test can force a load failure. vi.mock('../../src/pre/tools', () => ({ + toolIds: {}, toolModules: { FakeTool: { make: () => {}, destroy: () => {} } }, })) import ToolControllerModern_ from '../../src/essence/Basics/ToolController_/ToolControllerModern_' import { mmgisAPI } from '../../src/essence/mmgisAPI/mmgisAPI' -const metadata = { id: 'FakeTool', name: 'Fake Tool' } +// Written by hand rather than generated, so it states both names outright: +// 'FakeTool' reaches the module, 'fake' is the address every command names it by. +const metadata = { id: 'fake', module: 'FakeTool', name: 'Fake Tool' } // Set by a test that wants to observe plugins:changed; always torn down in // afterEach rather than at the end of the test body, so a failed assertion @@ -42,7 +46,7 @@ afterEach(() => { }) test('a deferred plugin reads as unloaded', () => { - expect(ToolControllerModern_.getPluginState('FakeTool')).toBe('unloaded') + expect(ToolControllerModern_.getPluginState('fake')).toBe('unloaded') }) test('an unknown plugin has no state', () => { @@ -55,13 +59,13 @@ test('setting an unknown plugin is not-found', () => { }) test('an unrecognised state is a bad-request', () => { - expect(ToolControllerModern_.setPluginState('FakeTool', 'sideways')) + expect(ToolControllerModern_.setPluginState('fake', 'sideways')) .toEqual({ ok: false, reason: 'bad-request' }) }) test('setting the current state is a quiet no-op', () => { const seen = listenForChanges() - expect(ToolControllerModern_.setPluginState('FakeTool', 'unloaded')) + expect(ToolControllerModern_.setPluginState('fake', 'unloaded')) .toEqual({ ok: true, state: 'unloaded', changed: false }) expect(seen).toEqual([]) }) @@ -69,24 +73,24 @@ test('setting the current state is a quiet no-op', () => { test('a successful transition emits plugins:changed', () => { const seen = listenForChanges() - expect(ToolControllerModern_.setPluginState('FakeTool', 'visible')) + expect(ToolControllerModern_.setPluginState('fake', 'visible')) .toEqual({ ok: true, state: 'visible', changed: true }) expect(seen).toHaveLength(1) - expect(seen[0].plugins).toContainEqual({ id: 'FakeTool', state: 'visible' }) + expect(seen[0].plugins).toContainEqual({ id: 'fake', state: 'visible' }) }) test('unloaded to hidden loads the plugin and lands it hidden, not visible', () => { - expect(ToolControllerModern_.setPluginState('FakeTool', 'hidden')) + expect(ToolControllerModern_.setPluginState('fake', 'hidden')) .toEqual({ ok: true, state: 'hidden', changed: true }) - expect(ToolControllerModern_.getPluginState('FakeTool')).toBe('hidden') + expect(ToolControllerModern_.getPluginState('fake')).toBe('hidden') expect(document.getElementById('fake-target').classList.contains('plugin-hidden')).toBe(true) }) test('visible to hidden hides an already-loaded plugin', () => { - ToolControllerModern_.setPluginState('FakeTool', 'visible') + ToolControllerModern_.setPluginState('fake', 'visible') - expect(ToolControllerModern_.setPluginState('FakeTool', 'hidden')) + expect(ToolControllerModern_.setPluginState('fake', 'hidden')) .toEqual({ ok: true, state: 'hidden', changed: true }) expect(document.getElementById('fake-target').classList.contains('plugin-hidden')).toBe(true) }) @@ -102,22 +106,22 @@ test('a plugin missing from the tool registry fails to load', () => { test('listPlugins unions plugins that are in different states', () => { document.body.innerHTML += '
' ToolControllerModern_.registerDeferred({ id: 'SecondTool', name: 'Second' }, 'second-target') - ToolControllerModern_.setPluginState('FakeTool', 'visible') + ToolControllerModern_.setPluginState('fake', 'visible') const plugins = ToolControllerModern_.listPlugins() - expect(plugins).toContainEqual({ id: 'FakeTool', state: 'visible' }) + expect(plugins).toContainEqual({ id: 'fake', state: 'visible' }) expect(plugins).toContainEqual({ id: 'SecondTool', state: 'unloaded' }) }) test('the listing is frozen and cloneable', () => { const plugins = ToolControllerModern_.listPlugins() - expect(plugins).toContainEqual({ id: 'FakeTool', state: 'unloaded' }) + expect(plugins).toContainEqual({ id: 'fake', state: 'unloaded' }) expect(Object.isFrozen(plugins[0])).toBe(true) expect(() => structuredClone(plugins)).not.toThrow() }) test('a mutator that refuses reports the transition, not a missing plugin', () => { - ToolControllerModern_.setPluginState('FakeTool', 'visible') + ToolControllerModern_.setPluginState('fake', 'visible') const seen = listenForChanges() // destroyTool keeps the lifecycle registries in step, so the only way a // mutator refuses a plugin getPluginState just resolved is those registries @@ -125,7 +129,7 @@ test('a mutator that refuses reports the transition, not a missing plugin', () = // no test can reach. vi.spyOn(ToolControllerModern_, 'hidePlugin').mockReturnValue(false) - expect(ToolControllerModern_.setPluginState('FakeTool', 'hidden')) + expect(ToolControllerModern_.setPluginState('fake', 'hidden')) .toEqual({ ok: false, reason: 'transition-failed' }) expect(seen).toEqual([]) }) @@ -147,7 +151,7 @@ test('a load batch broadcasts once, after every plugin in it has loaded', () => // One event, not one per plugin, and it carries the settled listing rather // than the partial one a subscriber seeding mid-batch would have captured. expect(seen).toHaveLength(1) - expect(seen[0].plugins).toContainEqual({ id: 'FakeTool', state: 'visible' }) + expect(seen[0].plugins).toContainEqual({ id: 'fake', state: 'visible' }) expect(seen[0].plugins).toContainEqual({ id: 'SecondTool', state: 'unloaded' }) }) diff --git a/tests/unit/toolIdentity.spec.js b/tests/unit/toolIdentity.spec.js new file mode 100644 index 000000000..61f7f9d30 --- /dev/null +++ b/tests/unit/toolIdentity.spec.js @@ -0,0 +1,184 @@ +import { describe, test, expect, afterEach, vi } from 'vitest' + +// Viewer_ pulls in Photosphere/ModelViewer/PDFViewer, which are JSX written in +// .js files that vite's import-analysis can't parse. Nothing here needs the +// real viewers, so stub the aggregator to keep the import chain parseable. +vi.mock('../../src/essence/Basics/Viewer_/Viewer_', () => ({ default: {} })) + +// Stand in the generated registry: bindings mapped to the addresses the build +// derived from them, and the classes those bindings reach. `PlainTool` is +// deliberately missing from `toolIds` so the frontend's own fallback +// derivation is exercised beside the generated map. +const { calls } = vi.hoisted(() => ({ calls: [] })) +vi.mock('../../src/pre/tools', () => ({ + Kinds: {}, + toolConfigs: {}, + testModules: {}, + toolIds: { FetchStatsTool: 'fetchstats' }, + toolModules: { + // Records the address its injected handle carries at each lifecycle + // step, so a test can see whether the handle was there before the tool + // ran any of its own code. + FetchStatsTool: { + initialize() { + calls.push(['initialize', this.api?.address]) + this.api.provide('getSelection', () => 'a selection') + }, + make() { + calls.push(['make', this.api?.address]) + }, + destroy() { + calls.push(['destroy', this.api?.address]) + }, + }, + // Puts a provider up through its handle and then throws, so a test can + // see what a load that never finished hands back. + FailingTool: { + initialize() { + this.api.provide('getSelection', () => 'a selection') + throw new Error('initialize threw') + }, + make() {}, + destroy() {}, + }, + PlainTool: { make: () => {}, destroy: () => {} }, + }, +})) + +const { toolModules } = await import('../../src/pre/tools') +const { mmgisAPI } = await import('../../src/essence/mmgisAPI/mmgisAPI') +const { generateToolMetadata } = await import( + '../../src/essence/Basics/ToolController_/ToolMetadataUtils' +) +const { default: ToolControllerModern_ } = await import( + '../../src/essence/Basics/ToolController_/ToolControllerModern_' +) + +/** + * A tool has two names and they are not interchangeable. `module` reaches its + * class in the generated registry; `id` is its address — what it is called + * everywhere else, including on the bus. These specs pin that the two stay in + * their lanes, and that the controller hands each tool a handle minted under + * its address and takes it back on the way out. + */ + +const statsConfig = { name: 'Statistics', js: 'FetchStatsTool' } + +function loadStatsTool() { + const target = document.createElement('div') + target.id = 'stats-target' + document.body.appendChild(target) + ToolControllerModern_.loadTool(generateToolMetadata(statsConfig), 'stats-target') + return 'stats-target' +} + +afterEach(() => { + // loadedTools and the lifecycle registries are module-level singletons, so + // a tool one test leaves loaded answers for the next test's. So are the + // name -> address and binding -> address maps, which an empty config map + // empties. + ToolControllerModern_.destroyAllTools() + ToolControllerModern_.buildToolConfigMap([]) + document.body.innerHTML = '' + calls.length = 0 +}) + +describe('resolving a tool config to its two names', () => { + test('metadata carries the address beside the binding', () => { + expect(generateToolMetadata(statsConfig)).toMatchObject({ + id: 'fetchstats', + module: 'FetchStatsTool', + }) + // A binding the generated registry does not carry derives the same way + // the build would have derived it. + expect(generateToolMetadata({ name: 'Plain', js: 'PlainTool' })).toMatchObject({ + id: 'plain', + module: 'PlainTool', + }) + }) + + // A panel's `panelTools` names its tools however the config author wrote + // them: the display name, the address, or the registry binding. + test('a panel finds a tool by name, by address, or by module binding', () => { + const { getToolData } = ToolControllerModern_.buildToolConfigMap([statsConfig]) + + expect(getToolData('Statistics').metadata.id).toBe('fetchstats') + expect(getToolData('fetchstats').metadata.id).toBe('fetchstats') + expect(getToolData('FetchStatsTool').metadata.id).toBe('fetchstats') + expect(getToolData('Nothing Named This')).toBe(undefined) + }) +}) + +describe('a tool the controller loads', () => { + test('already holds its bus handle when its own code first runs', () => { + loadStatsTool() + + // Both lifecycle hooks saw a handle, minted under the tool's address + // rather than the binding that reached its class. + expect(calls).toEqual([ + ['initialize', 'fetchstats'], + ['make', 'fetchstats'], + ]) + }) + + test('has its handle taken back when its own load throws part-way', () => { + const target = document.createElement('div') + target.id = 'failing-target' + document.body.appendChild(target) + + const loaded = ToolControllerModern_.loadTool( + generateToolMetadata({ name: 'Failing', js: 'FailingTool' }), + 'failing-target' + ) + + // No destroyTool will ever come for an instance that never finished + // loading, so the failed load itself is what hands the handle back. + expect(loaded).toBe(null) + expect(mmgisAPI.hasHandler('plugin:failing:getSelection')).toBe(false) + expect(toolModules.FailingTool.api).toBe(null) + }) + + test('has the providers it put up through that handle answering', async () => { + loadStatsTool() + + expect(mmgisAPI.hasHandler('plugin:fetchstats:getSelection')).toBe(true) + expect(await mmgisAPI.request('plugin:fetchstats:getSelection')).toBe( + 'a selection' + ) + }) +}) + +describe('a tool the controller destroys', () => { + test('is announced by its address, and its registrations handed back', () => { + const targetId = loadStatsTool() + const seen = [] + const off = mmgisAPI.on('plugins:destroyed', (payload) => seen.push(payload)) + + try { + ToolControllerModern_.destroyTool(targetId) + } finally { + off() + } + + expect(seen).toEqual([{ pluginId: 'fetchstats' }]) + // The tool's own destroy() ran while it still had the handle; the + // release comes after, so nothing it wanted to say was cut off. + expect(calls).toContainEqual(['destroy', 'fetchstats']) + expect(mmgisAPI.hasHandler('plugin:fetchstats:getSelection')).toBe(false) + expect(toolModules.FetchStatsTool.api).toBe(null) + }) + + test('is named in the collective signal a full teardown ends with', () => { + loadStatsTool() + const seen = [] + const off = mmgisAPI.on('plugins:allDestroyed', (payload) => seen.push(payload)) + + try { + ToolControllerModern_.destroyAllTools() + } finally { + off() + } + + expect(seen).toEqual([{ pluginIds: ['fetchstats'] }]) + }) +}) diff --git a/tests/unit/updateToolsIds.spec.js b/tests/unit/updateToolsIds.spec.js new file mode 100644 index 000000000..36dcf415c --- /dev/null +++ b/tests/unit/updateToolsIds.spec.js @@ -0,0 +1,77 @@ +import { describe, it, expect } from 'vitest' +import fs from 'node:fs' +import path from 'node:path' + +import { buildToolIds } from '../../API/updateTools' +import { toolCanonicalId } from '../../src/essence/Basics/ToolController_/ToolMetadataUtils' + +// Issue #350: a tool's address is derived from its module binding at build +// time and written into the generated registry. buildToolIds is the pure half +// of the generator, so it can be pinned directly. + +describe('buildToolIds', () => { + it('derives an address from each binding', () => { + expect(buildToolIds({ Draw: { paths: { DrawTool: 'x' } } })).toEqual({ + DrawTool: 'draw', + }) + }) + + // Kinds is special-cased out of `toolModules` — it is imported under its + // own export rather than loaded as a tool — but it is still a binding with + // an address, and nothing about the derivation treats it differently. + it('names the Kinds binding after itself', () => { + expect(buildToolIds({ Kinds: { paths: { Kinds: 'x' } } })).toEqual({ + Kinds: 'kinds', + }) + }) + + // Legacy 3D tools list helper modules beside their entry point. Only a + // trailing "Tool" is dropped, so a helper keeps the underscore that makes + // it a separate address from the tool it belongs to. + it('keeps the underscore in a helper binding', () => { + expect( + buildToolIds({ + Viewshed: { + paths: { ViewshedTool: 'x', ViewshedTool_Manager: 'y' }, + }, + }) + ).toEqual({ + ViewshedTool: 'viewshed', + ViewshedTool_Manager: 'viewshedtool_manager', + }) + }) + + // Dropping a trailing "Tool" lets two bindings land on one address, where + // the second would quietly take over the first's events and providers. + // The generator throws instead, so the collision fails the build. + it('rejects two bindings that derive the same address', () => { + expect(() => + buildToolIds({ + Foo: { paths: { Foo: 'x', FooTool: 'y' } }, + }) + ).toThrow(/"Foo" and "FooTool" both derive the address "foo"/) + }) + + // The derivation is written twice — here for the build, and in + // toolCanonicalId for the browser, which falls back to it for a binding + // the generated registry does not carry. The two run in different + // processes and must land on the same string, or a tool the registry names + // one thing answers the bus as another. cwd is the repo root under vitest. + it('agrees with the frontend derivation on the checked-in manifests', () => { + const toolsDir = 'src/essence/Tools' + const tools = {} + for (const dir of fs.readdirSync(toolsDir)) { + if (dir[0] === '_' || dir[0] === '.') continue + const configPath = path.join(toolsDir, dir, 'config.json') + if (!fs.existsSync(configPath)) continue + tools[dir] = JSON.parse(fs.readFileSync(configPath, 'utf8')) + } + + const ids = buildToolIds(tools) + expect(Object.keys(ids).length).toBeGreaterThan(0) + expect(ids).toMatchObject({ AOITool: 'aoi', FetchStatsTool: 'fetchstats' }) + for (const binding of Object.keys(ids)) { + expect(toolCanonicalId({ js: binding })).toBe(ids[binding]) + } + }) +})