Fix: disabled plugins no longer have an active API interface (#1684)

src/interfaces/plugins.ts

@@ -27,7 +27,7 @@ import {
 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
 // @ts-ignore
 import { getLogger } from '@signalk/streams/logging'
-import express, { Request, Response } from 'express'
+import express, { Request, Response, Router } from 'express'
 import fs from 'fs'
 import _ from 'lodash'
 import path from 'path'
@@ -423,10 +423,12 @@ module.exports = (theApp: any) => {
     plugin: PluginInfo,
     location: string,
     configuration: any,
+    router: Router,
     restart: (newConfiguration: any) => void
   ) {
     debug('Starting plugin %s from %s', plugin.name, location)
     try {
+      doRegisterWithRouter(app, plugin, router)
       app.setPluginStatus(plugin.id, null)
 
       if (plugin.enableDebug) {
@@ -609,7 +611,14 @@ module.exports = (theApp: any) => {
           console.error(err)
         } else {
           stopPlugin(plugin)
-          doPluginStart(app, plugin, location, newConfiguration, restart)
+          doPluginStart(
+            app,
+            plugin,
+            location,
+            newConfiguration,
+            router,
+            restart
+          )
         }
       })
     }
@@ -626,20 +635,21 @@ module.exports = (theApp: any) => {
     plugin.keywords = metadata.keywords
     plugin.packageLocation = location
 
+    plugin.enableLogging = startupOptions.enableLogging
+    app.plugins.push(plugin)
+    app.pluginsMap[plugin.id] = plugin
+
+    const router = express.Router()
     if (startupOptions && startupOptions.enabled) {
       doPluginStart(
         app,
         plugin,
         location,
         startupOptions.configuration,
+        router,
         restart
       )
     }
-    plugin.enableLogging = startupOptions.enableLogging
-    app.plugins.push(plugin)
-    app.pluginsMap[plugin.id] = plugin
-
-    const router = express.Router()
     router.get('/', (req: Request, res: Response) => {
       const currentOptions = getPluginOptions(plugin.id)
       const enabledByDefault = isEnabledByPackageEnableDefault(
@@ -669,7 +679,14 @@ module.exports = (theApp: any) => {
         plugin.enableLogging = options.enableLogging
         plugin.enableDebug = options.enableDebug
         if (options.enabled) {
-          doPluginStart(app, plugin, location, options.configuration, restart)
+          doPluginStart(
+            app,
+            plugin,
+            location,
+            options.configuration,
+            router,
+            restart
+          )
         }
       })
     })
@@ -678,17 +695,27 @@ module.exports = (theApp: any) => {
       res.json(getPluginOptions(plugin.id))
     })
 
-    if (typeof plugin.registerWithRouter == 'function') {
+    if (typeof plugin.signalKApiRoutes === 'function') {
+      app.use('/signalk/v1/api', plugin.signalKApiRoutes(express.Router()))
+    }
+  }
+
+  function doRegisterWithRouter(app: any, plugin: PluginInfo, router: Router) {
+    if (typeof plugin.registerWithRouter === 'function') {
+      debug('Activating routing for ' + plugin.id)
+      router.use((req, res, next) => {
+        const stopHandlers = onStopHandlers[plugin.id]
+
+        if (stopHandlers.length > 0) next()
+        else res.status(500).send(plugin.id + ' has stopped')
+      })
       plugin.registerWithRouter(router)
       if (typeof plugin.getOpenApi == 'function') {
         app.setPluginOpenApi(plugin.id, plugin.getOpenApi())
       }
     }
-    app.use(backwardsCompat('/plugins/' + plugin.id), router)
 
-    if (typeof plugin.signalKApiRoutes === 'function') {
-      app.use('/signalk/v1/api', plugin.signalKApiRoutes(express.Router()))
-    }
+    app.use(backwardsCompat('/plugins/' + plugin.id), router)
   }
 }
 

test/plugins.js

@@ -6,26 +6,31 @@ const Server = require('../lib/')
 const fs = require('fs')
 const path = require('path')
 
+let pluginConfig;
+
 describe('Demo plugin ', () => {
-  it('works', async () => {
+  beforeEach(() => {
     process.env.SIGNALK_NODE_CONFIG_DIR = require('path').join(
       __dirname,
       'plugin-test-config'
     )
-    const pluginConfig = {
+     pluginConfig = {
       enabled: true,
       configuration: {
         testOption: 'testValue'
       }
     }
-
     mkDirSync(path.join(`${__dirname}/plugin-test-config/plugin-config-data`))
+  })
+
+  it('works', async () => {
     writePluginConfig(pluginConfig)
 
     const port = await freeport()
     const server = new Server({
       config: { settings: { port } }
     })
+
     await server.start()
     const plugins = await fetch(`http://0.0.0.0:${port}/skServer/plugins`).then(res =>
       res.json()
@@ -90,10 +95,78 @@ describe('Demo plugin ', () => {
     assert.equal(outputValues[2], 3)
 
     await server.stop()
-  })
+  }),
+
+    it('registerWithRouter only on plugin Start', async () => {
+      pluginConfig.enabled = false;
+      writePluginConfig(pluginConfig)
+
+      const port = await freeport()
+      const server = new Server({
+        config: { settings: { port } }
+      })
+      await server.start()
+      const plugins = await fetch(`http://0.0.0.0:${port}/skServer/plugins/`).then(res =>
+        res.json()
+      )
+
+      assert(plugins.find(plugin => plugin.id === 'testplugin'))
+      const plugin = server.app.plugins.find(plugin => plugin.id === 'testplugin')
+      assert(plugin)
+      assert(!plugin.started)
+
+      const response = await fetch(`http://0.0.0.0:${port}/skServer/plugins/testplugin/demopluginGet`, {
+        method: 'GET',
+      }).then(res => res.ok
+      )
+
+      assert(!response)
+    }),
+
+    it('deactivates API interface when plugin has stopped', async () => {
+      writePluginConfig(pluginConfig)
+
+      const port = await freeport()
+      const server = new Server({
+        config: { settings: { port } }
+      })
+      await server.start()
+      const plugins = await fetch(`http://0.0.0.0:${port}/skServer/plugins/`).then(res =>
+        res.json()
+      )
+
+      assert(plugins.find(plugin => plugin.id === 'testplugin'))
+      const plugin = server.app.plugins.find(plugin => plugin.id === 'testplugin')
+      assert(plugin)
+      assert(plugin.started)
+
+      const response = await fetch(`http://0.0.0.0:${port}/skServer/plugins/testplugin/demopluginGet`, {
+        method: 'GET',
+      }).then(res => res.ok
+      )
+
+      assert(response)
+
+      pluginConfig.enabled = false
+      await postPluginConfig(port, pluginConfig)
+
+
+      const stoppedPlugin = server.app.plugins.find(plugin => plugin.id === 'testplugin')
+      assert(stoppedPlugin)
+
+      const status = await fetch(`http://0.0.0.0:${port}/skServer/plugins/testplugin/demopluginGet`, {
+        method: 'GET',
+      }).then(res => {
+        return res.status
+      })
+
+      assert.equal(status, 500)
+
+
+    })
 })
 
-function mkDirSync (dirPath) {
+function mkDirSync(dirPath) {
   try {
     fs.mkdirSync(dirPath)
   } catch (err) {
@@ -103,7 +176,7 @@ function mkDirSync (dirPath) {
   }
 }
 
-function writePluginConfig (config) {
+function writePluginConfig(config) {
   fs.writeFileSync(
     path.join(
       `${__dirname}/plugin-test-config/plugin-config-data/testplugin.json`
@@ -112,7 +185,7 @@ function writePluginConfig (config) {
   )
 }
 
-async function postPluginConfig (port, config) {
+async function postPluginConfig(port, config) {
   await fetch(`http://0.0.0.0:${port}/skServer/plugins/testplugin/config`, {
     method: 'POST',
     headers: {