From 6cacf0dc8745c618faa1a6098020b5b64a5ea75a Mon Sep 17 00:00:00 2001 From: Carson Davis Date: Thu, 10 Sep 2026 15:40:15 -0500 Subject: [PATCH 1/3] Give each tool one address derived at build time, and have core mint and release its bus handle A tool's address comes from its binding name in the generated manifest, so there is nothing to declare and nothing to collide. The modern tool controller mints the tool's bus handle before its own code runs, injects it, releases it after destroy, and announces each teardown on the bus. The public accessor for minting a handle by name is gone, the classic controller's unused mint is deleted, and layer variable lookup keys on the address. --- API/updateTools.js | 26 ++- .../APIs/JavaScript/Main/Event-Bus-API.md | 48 +++-- src/essence/Basics/Layers_/Layers_.js | 21 +-- .../TOOL_RENDERING_PIPELINE.md | 4 +- .../ToolController_/ToolControllerModern_.js | 173 ++++++++++++++---- .../Basics/ToolController_/ToolController_.js | 5 - .../ToolController_/ToolMetadataUtils.js | 35 +++- .../Basics/ToolController_/types/tool.ts | 5 +- src/essence/Tools/AOI/AOITool.js | 36 ++-- .../AddTempLayer/MMGISAddTempLayerAdapter.tsx | 3 +- .../MMGISAddTempLayerAdapter.spec.tsx | 2 +- src/essence/Tools/AddTempLayer/config.json | 2 +- src/essence/Tools/Chart/ChartTool.js | 10 +- .../Tools/Chart/__tests__/ChartTool.spec.tsx | 16 +- src/essence/Tools/Chart/config.json | 2 +- .../Tools/Comparison/ComparisonTool.tsx | 3 +- .../__tests__/ComparisonTool.spec.tsx | 4 +- .../Tools/FetchStats/FetchStatsTool.js | 28 +-- .../MMGISThemeRailAdapter.tsx | 7 +- .../__tests__/MMGISThemeRailAdapter.spec.tsx | 2 +- .../LayerManager/__tests__/handlers.spec.js | 2 +- .../Tools/LayerManager/adapters/handlers.ts | 3 +- src/essence/Tools/MODERN_TOOL_PATTERN.md | 2 + src/essence/Tools/Title/config.json | 2 +- src/essence/mmgisAPI/mmgisAPI.js | 13 +- tests/e2e/eventbus-integration.spec.js | 95 ---------- tests/unit/__mocks__/preTools.js | 1 + tests/unit/aoiDrawKeys.spec.js | 9 +- tests/unit/aoiToolClose.spec.js | 2 +- tests/unit/chartToolPlugin.spec.js | 8 +- tests/unit/getToolVars.spec.js | 70 +++++++ tests/unit/mmgisApiCallerStamp.spec.js | 4 +- tests/unit/pluginHandleRelease.spec.js | 6 +- .../unit/providers/busReconciliation.spec.js | 10 +- tests/unit/providers/plugins.spec.js | 45 +++-- .../toolControllerModern.pluginState.spec.js | 42 +++-- tests/unit/toolIdentity.spec.js | 157 ++++++++++++++++ tests/unit/updateToolsIds.spec.js | 66 +++++++ 38 files changed, 673 insertions(+), 296 deletions(-) create mode 100644 tests/unit/getToolVars.spec.js create mode 100644 tests/unit/toolIdentity.spec.js create mode 100644 tests/unit/updateToolsIds.spec.js diff --git a/API/updateTools.js b/API/updateTools.js index c50aa980e..994ec7232 100644 --- a/API/updateTools.js +++ b/API/updateTools.js @@ -4,6 +4,22 @@ 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. Addresses are unique because bindings are: +// two bindings sharing one address would have to be the same import name. +function buildToolIds(tools) { + const ids = {}; + for (const t in tools) { + for (const p in tools[t].paths) { + ids[p] = p.replace(/Tool$/, "").toLowerCase(); + } + } + return ids; +} + function updateTools() { let tools = {}; @@ -189,6 +205,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 +402,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 3cda49c58..e18e02202 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 it 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, so no two tools can share one. 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 }, '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..a84d209ec 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,18 @@ 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 address the tool is known by everywhere else — + // the one its bus handle is minted under — so a tool configured + // under a display name that is not its address is still reachable. + // The lowercased name stays as a fallback for callers that ask by + // display name 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..26466e1f2 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,22 @@ 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} True if destroyed successfully. False covers both a + * tool whose own destroy() threw and a bus handle whose release did — in + * either case the lifecycle registries are still cleared, but something the + * tool held was not given back. */ 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 + // Call destroy() if available. It runs before the handle is taken + // back, so a tool can still speak to core while shutting down. if (typeof toolInstance.destroy === 'function') { toolInstance.destroy() } @@ -432,8 +482,31 @@ const ToolControllerModern_ = { toolIdToTargetId.delete(toolId) hiddenTools.delete(toolId) } + + // Hand back what the minted handle registered — last, and guarded + // so a throwing release cannot swallow the teardown announcement + // below. The instance is a module singleton, so its `api` is only + // cleared while it still points at this load's handle: a reload + // that already minted a fresh one keeps it. + 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 + } } + // The teardown is announced on the bus rather than by any direct call, + // which is what keeps this controller from knowing anything about the + // services plugins reach for. `pluginId` is the tool's address — the + // one it spoke to core services under — so a service holding a + // resource for this plugin can match the announcement to what it holds + // and let go of it, while a surviving plugin's stays untouched. + mmgisAPI.emit('plugins:destroyed', Object.freeze({ pluginId: toolId })) + if (destroyed) { logger.debug(`Destroyed tool "${toolName}" from container "${targetId}"`) } @@ -446,6 +519,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 +531,18 @@ const ToolControllerModern_ = { // Clear deferred registry (destroyTool already clears toolIdToTargetId and hiddenTools) deferredTools.clear() + // After the per-tool announcements, one collective signal that the + // plugins went down together — what a core service releases a shared + // resource on when it holds one for whichever plugin asked. Emitted + // only when there were instances to destroy: with nothing loaded, + // nothing was held on a plugin's behalf. + if (pluginIds.length > 0) { + mmgisAPI.emit( + 'plugins:allDestroyed', + Object.freeze({ pluginIds: Object.freeze(pluginIds) }) + ) + } + if (hadPlugins) this.notifyPluginsChanged() }, @@ -525,13 +611,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 +632,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 +653,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 +683,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 +721,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 +733,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 +747,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 +762,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..910ed9f65 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,30 @@ 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. `js` names the + * import binding and nothing else; the build derives the same address from it + * (buildToolIds in API/updateTools.js), so a registry generated before this + * export existed still lands on the same string. An entry with no module at + * all is named after itself. + * + * @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 +553,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 +563,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..caa34a57a 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' @@ -152,13 +154,15 @@ const AOITool = { } this._reactRoot = createRoot(container) - this._api = - (typeof window !== 'undefined' && window.mmgisAPI?.forPlugin?.(PLUGIN_ID)) || - { emit: () => { }, provide: () => () => { } } + // 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. + this._api = AOITool.api - this._cleanups.push( - this._api.provide('getCurrentSelection', () => this._state.currentAOI) + const offSelection = this._api?.provide( + 'getCurrentSelection', + () => this._state.currentAOI ) + if (offSelection) this._cleanups.push(offSelection) this._state.searchLoading = true loadBoundaries() @@ -188,7 +192,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 +204,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)) }) } @@ -304,7 +308,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 +374,7 @@ const AOITool = { }, _onClose() { - mmgisSetPluginState('AOITool', 'unloaded') + mmgisSetPluginState('aoi', 'unloaded') .then((result) => { if (!result.ok) { console.warn(`[AOI] unload refused: ${result.reason}`) 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 e94158b27..ecad0a63f 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: [], initialize() { @@ -44,10 +43,6 @@ const FetchStatsTool = { make(targetId) { this.MMGISInterface = new interfaceWithMMGIS(this, targetId) - this._api = - (typeof window !== 'undefined' && - window.mmgisAPI?.forPlugin?.(PLUGIN_ID)) || - {} const api = window.mmgisAPI if (api?.on) { @@ -90,13 +85,20 @@ 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; the classic layout has no controller to do so, and a run + // there 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', @@ -110,13 +112,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/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 3e11ce009..11309d2a2 100644 --- a/src/essence/mmgisAPI/mmgisAPI.js +++ b/src/essence/mmgisAPI/mmgisAPI.js @@ -992,17 +992,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_ }, } @@ -1018,6 +1007,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 6c5be52b7..a5a9b4b10 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 d662a0155..f2dd7bf7e 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..2b679d62f --- /dev/null +++ b/tests/unit/toolIdentity.spec.js @@ -0,0 +1,157 @@ +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]) + }, + }, + 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 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..83be792c1 --- /dev/null +++ b/tests/unit/updateToolsIds.spec.js @@ -0,0 +1,66 @@ +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', + }) + }) + + // 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]) + } + }) +}) From e4cdc4c6f56a513ee4127a6555eadc5485f7c974 Mon Sep 17 00:00:00 2001 From: Carson Davis Date: Thu, 10 Sep 2026 15:58:15 -0500 Subject: [PATCH 2/3] Pin the mid-load release, trim controller comments, and read AOI's handle directly --- .../APIs/JavaScript/Main/Event-Bus-API.md | 2 +- src/essence/Basics/Layers_/Layers_.js | 7 ++-- .../ToolController_/ToolControllerModern_.js | 34 ++++++------------- .../ToolController_/ToolMetadataUtils.js | 9 +++-- src/essence/Tools/AOI/AOITool.js | 16 ++++----- tests/unit/toolIdentity.spec.js | 27 +++++++++++++++ 6 files changed, 50 insertions(+), 45 deletions(-) diff --git a/docs/pages/APIs/JavaScript/Main/Event-Bus-API.md b/docs/pages/APIs/JavaScript/Main/Event-Bus-API.md index 5b88fe9a4..88383b562 100644 --- a/docs/pages/APIs/JavaScript/Main/Event-Bus-API.md +++ b/docs/pages/APIs/JavaScript/Main/Event-Bus-API.md @@ -155,7 +155,7 @@ anything there right now. ## Plugin Scoped API -MMGIS injects a scoped API into each tool it 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. +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. 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. diff --git a/src/essence/Basics/Layers_/Layers_.js b/src/essence/Basics/Layers_/Layers_.js index a84d209ec..ee68b1ab1 100644 --- a/src/essence/Basics/Layers_/Layers_.js +++ b/src/essence/Basics/Layers_/Layers_.js @@ -2490,11 +2490,8 @@ const L_ = { getToolVars: function (toolName, withVarsFromLayers, showWarnings) { let vars = {} for (var i = 0; i < L_.tools.length; i++) { - // Matched on the address the tool is known by everywhere else — - // the one its bus handle is minted under — so a tool configured - // under a display name that is not its address is still reachable. - // The lowercased name stays as a fallback for callers that ask by - // display name instead. + // Matched on the tool's address first, with the lowercased display + // name kept as a fallback for callers that ask by that instead. if ( (toolCanonicalId(L_.tools[i]) === toolName || (L_.tools[i].hasOwnProperty('name') && diff --git a/src/essence/Basics/ToolController_/ToolControllerModern_.js b/src/essence/Basics/ToolController_/ToolControllerModern_.js index 26466e1f2..893881588 100644 --- a/src/essence/Basics/ToolController_/ToolControllerModern_.js +++ b/src/essence/Basics/ToolController_/ToolControllerModern_.js @@ -450,10 +450,8 @@ 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. False covers both a - * tool whose own destroy() threw and a bus handle whose release did — in - * either case the lifecycle registries are still cleared, but something the - * tool held was not given back. + * @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)) { @@ -464,8 +462,7 @@ const ToolControllerModern_ = { let destroyed = true try { - // Call destroy() if available. It runs before the handle is taken - // back, so a tool can still speak to core while shutting down. + // destroy() runs before the handle is taken back, so a tool can still speak to core. if (typeof toolInstance.destroy === 'function') { toolInstance.destroy() } @@ -473,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 @@ -483,11 +479,8 @@ const ToolControllerModern_ = { hiddenTools.delete(toolId) } - // Hand back what the minted handle registered — last, and guarded - // so a throwing release cannot swallow the teardown announcement - // below. The instance is a module singleton, so its `api` is only - // cleared while it still points at this load's handle: a reload - // that already minted a fresh one keeps it. + // 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() @@ -499,12 +492,8 @@ const ToolControllerModern_ = { } } - // The teardown is announced on the bus rather than by any direct call, - // which is what keeps this controller from knowing anything about the - // services plugins reach for. `pluginId` is the tool's address — the - // one it spoke to core services under — so a service holding a - // resource for this plugin can match the announcement to what it holds - // and let go of it, while a surviving plugin's stays untouched. + // 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) { @@ -531,11 +520,8 @@ const ToolControllerModern_ = { // Clear deferred registry (destroyTool already clears toolIdToTargetId and hiddenTools) deferredTools.clear() - // After the per-tool announcements, one collective signal that the - // plugins went down together — what a core service releases a shared - // resource on when it holds one for whichever plugin asked. Emitted - // only when there were instances to destroy: with nothing loaded, - // nothing was held on a plugin's behalf. + // 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', diff --git a/src/essence/Basics/ToolController_/ToolMetadataUtils.js b/src/essence/Basics/ToolController_/ToolMetadataUtils.js index 910ed9f65..2013b637e 100644 --- a/src/essence/Basics/ToolController_/ToolMetadataUtils.js +++ b/src/essence/Basics/ToolController_/ToolMetadataUtils.js @@ -522,11 +522,10 @@ export function getValidIconClass(iconClass, toolId) { /** * 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. `js` names the - * import binding and nothing else; the build derives the same address from it - * (buildToolIds in API/updateTools.js), so a registry generated before this - * export existed still lands on the same string. An entry with no module at - * all is named after itself. + * 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 diff --git a/src/essence/Tools/AOI/AOITool.js b/src/essence/Tools/AOI/AOITool.js index caa34a57a..f627313d5 100644 --- a/src/essence/Tools/AOI/AOITool.js +++ b/src/essence/Tools/AOI/AOITool.js @@ -139,7 +139,6 @@ const AOITool = { _reactRoot: null, _state: initialState(), _cleanups: [], - _api: null, _analysisErrorTimeout: null, _drawKeyHandler: null, @@ -156,9 +155,7 @@ const AOITool = { // 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. - this._api = AOITool.api - - const offSelection = this._api?.provide( + const offSelection = this.api?.provide( 'getCurrentSelection', () => this._state.currentAOI ) @@ -246,7 +243,6 @@ const AOITool = { this.targetId = null this._state = initialState() - this._api = null this.made = false }, @@ -269,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' @@ -585,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 @@ -653,7 +649,7 @@ const AOITool = { this._removeSelectionLayer() this._hideTooltip() this._state.currentAOI = null - this._api?.emit('drawingCleared', {}) + this.api?.emit('drawingCleared', {}) this._render() }, @@ -701,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/tests/unit/toolIdentity.spec.js b/tests/unit/toolIdentity.spec.js index 2b679d62f..61f7f9d30 100644 --- a/tests/unit/toolIdentity.spec.js +++ b/tests/unit/toolIdentity.spec.js @@ -31,6 +31,16 @@ vi.mock('../../src/pre/tools', () => ({ 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: () => {} }, }, })) @@ -111,6 +121,23 @@ describe('a tool the controller loads', () => { ]) }) + 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() From c264a6d2650ea68df8b6ab32859b0adc8d5fd152 Mon Sep 17 00:00:00 2001 From: Carson Davis Date: Thu, 10 Sep 2026 16:04:41 -0500 Subject: [PATCH 3/3] Fail the build when two tool bindings derive one address --- API/updateTools.js | 13 ++++++++++--- docs/pages/APIs/JavaScript/Main/Event-Bus-API.md | 2 +- tests/unit/updateToolsIds.spec.js | 11 +++++++++++ 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/API/updateTools.js b/API/updateTools.js index 994ec7232..c217a3231 100644 --- a/API/updateTools.js +++ b/API/updateTools.js @@ -8,13 +8,20 @@ const { isLean } = require("./Backend/Utils/deploymentMode"); // 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. Addresses are unique because bindings are: -// two bindings sharing one address would have to be the same import name. +// 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) { - ids[p] = p.replace(/Tool$/, "").toLowerCase(); + 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; diff --git a/docs/pages/APIs/JavaScript/Main/Event-Bus-API.md b/docs/pages/APIs/JavaScript/Main/Event-Bus-API.md index 88383b562..3d25bf397 100644 --- a/docs/pages/APIs/JavaScript/Main/Event-Bus-API.md +++ b/docs/pages/APIs/JavaScript/Main/Event-Bus-API.md @@ -159,7 +159,7 @@ MMGIS injects a scoped API into each tool the modern layout loads as `this.api`. 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, so no two tools can share one. One tool still cannot run as two instances: the second would answer for the first's events and providers. +> **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: diff --git a/tests/unit/updateToolsIds.spec.js b/tests/unit/updateToolsIds.spec.js index 83be792c1..36dcf415c 100644 --- a/tests/unit/updateToolsIds.spec.js +++ b/tests/unit/updateToolsIds.spec.js @@ -41,6 +41,17 @@ describe('buildToolIds', () => { }) }) + // 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