feat(history): record first registrant as default

With no default in settings, the registry resolves one from
registration order and never records it, so the choice is remade on
every start. startPlugins does not await plugin.start(), so a provider
that registers once its database answers loses the slot to one that
registers straight from start(), and the switch is silent because the
unavailable warning covers configured providers only.

Record the first provider to register, through the same settings write
the POST route uses, so a provider installed later cannot take the
default from the one already serving. The resources API does this for
resource providers. Nothing is recorded while settings do not
represent the configured state.

A failed write is reported once per run rather than per attempt: the
retry stays, but console.error reaches the log ring the Admin UI
subscribes to, and a plugin reconnecting in a loop would push the
errors explaining the loop out of it.

Author: mairas

docs/develop/rest-api/history_api.md

@@ -110,7 +110,7 @@ Returns an array of path strings.
 
 The History API supports the registration of multiple history provider plugins.
 
-The first plugin registered is set as the _default_ provider and all requests will be directed to it.
+The first plugin to register becomes the _default_ provider and all requests are directed to it. When settings name no default, and the settings the server started with are safe to save, it records that first provider — so the choice holds across restarts and a plugin installed later does not take it over. A default already named in settings is never replaced, and a server started with `--data` or sample data records nothing, keeping the first provider for that run only. Change the default in the Admin UI under Apps & Plugins -> Configuration.
 
 Requests can be directed to a specific provider by using the `provider` parameter in the request with the _id_ of the provider plugin.
 

src/api/history/index.ts

@@ -66,6 +66,15 @@ export class HistoryApiHttpRegistry {
    * the HISTORYPROVIDERS serverevent. */
   private unavailableGraceExpired = false
   private unavailableGraceTimer: ReturnType<typeof setTimeout> | null = null
+  /** True once a failed recording has been reported for this run. */
+  private recordFailureReported = false
+  /** The provider chosen as default whose write has not landed yet.
+   * Held across later registrations so a failed write is retried by the
+   * next one: without it the arrival of a second provider ends the
+   * attempts, and the default reverts to registration order on every
+   * start. Cleared when that provider unregisters, since recording a
+   * provider that has gone would be worse than the fallback. */
+  private pendingDefaultProviderId?: string
   proxy: HistoryApi
 
   /** The configured provider when it is registered, otherwise the first
@@ -136,6 +145,7 @@ export class HistoryApiHttpRegistry {
       this.unavailableGraceExpired = false
       this.notifyConfiguredAvailable()
     }
+    this.recordDefaultIfUnconfigured(pluginId)
     this.emitProvidersState()
     debug(
       `Registered history api provider ${pluginId},`,
@@ -144,11 +154,66 @@ export class HistoryApiHttpRegistry {
     )
   }
 
+  /** Persist the first provider to register when settings name none.
+   *
+   * Unrecorded, the default is resolved from registration order on
+   * every start, and `startPlugins` does not await `start()`: a plugin
+   * that registers once its database answers loses the slot to one that
+   * registers from `start()` itself. Recording it makes the first
+   * outcome the lasting one, so a provider installed later cannot take
+   * the default from the one already serving.
+   */
+  private recordDefaultIfUnconfigured(pluginId: string): void {
+    if (this.configuredProviderId !== undefined) {
+      return
+    }
+    if (!this.app.config.safeToPersistSettings) {
+      debug.enabled &&
+        debug(
+          `Not recording ${pluginId}: settings are not the configured state`
+        )
+      return
+    }
+    if (this.pendingDefaultProviderId === undefined) {
+      // Only the first provider claims the slot; later ones just give a
+      // failed write another chance at the one already claimed.
+      if (this.historyProviders.size !== 1) {
+        return
+      }
+      this.pendingDefaultProviderId = pluginId
+    }
+    const candidate = this.pendingDefaultProviderId
+    this.saveConfiguredProvider(candidate, (err?: Error) => {
+      if (!err) {
+        this.pendingDefaultProviderId = undefined
+        return
+      }
+      // A plugin that reconnects in a loop registers each time, and
+      // console.error reaches the 100-entry log ring the admin UI
+      // subscribes to. Reporting every attempt would push out the
+      // plugin errors that explain the loop, so the retry stays and
+      // only the first report does.
+      if (!this.recordFailureReported) {
+        this.recordFailureReported = true
+        console.error(
+          `Failed to record ${candidate} as the default history provider:`,
+          err.message
+        )
+      } else {
+        debug.enabled &&
+          debug(`Failed to record ${candidate} again: ${err.message}`)
+      }
+    })
+  }
+
   unregisterHistoryApiProvider(pluginId: string): void {
     if (!pluginId || !this.historyProviders.has(pluginId)) {
       return
     }
     this.historyProviders.delete(pluginId)
+    if (pluginId === this.pendingDefaultProviderId) {
+      this.pendingDefaultProviderId = undefined
+    }
     if (pluginId === this.configuredProviderId) {
       this.armUnavailableGrace()
     }

src/api/history/openApi.ts

@@ -356,7 +356,7 @@ historyApiDoc.paths = {
                   configured: {
                     type: 'string',
                     description:
-                      'Provider identifier persisted in server settings. May differ from `id` when the configured provider is not currently registered.'
+                      'Provider identifier persisted in server settings, set either by a client through this API or by the server itself, which records the first provider to register when no default is configured and its settings are safe to save. May differ from `id` when the configured provider is not currently registered, and is absent on a server that has recorded nothing.'
                   }
                 },
                 example: { id: 'signalk-to-influxdb2' }

src/config/config.ts

@@ -191,7 +191,9 @@ export interface Config {
        * Applied whenever the provider is registered, so the default does
        * not depend on plugin load order. When the configured provider is
        * not registered (e.g. plugin disabled), the first registered
-       * provider serves as fallback. */
+       * provider serves as fallback. Set through the History API, or by
+       * the server itself when a provider registers while this key is
+       * absent and safeToPersistSettings is set. */
       defaultProvider?: string
     }
     notifications?: {

test/history-api.ts

@@ -297,7 +297,14 @@ describe('History API v2', () => {
     }
 
     interface TestApp extends WithHistoryApi {
-      config: { settings: { historyApi?: { defaultProvider?: string } } }
+      config: {
+        safeToPersistSettings: boolean
+        settings: {
+          historyApi?: { defaultProvider?: string }
+          pipedProviders?: unknown[]
+          interfaces?: Record<string, boolean>
+        }
+      }
       handleMessage: (id: string, delta: unknown) => void
       /** Notification values captured from handleMessage */
       notifications: NotificationValue[]
@@ -311,7 +318,12 @@ describe('History API v2', () => {
       const serverEvents: HistoryProvidersEventData[] = []
       return {
         config: {
+          safeToPersistSettings: true,
           settings: {
+            // Unrelated keys so the assertions can show the recorded
+            // write carries the rest of the user's settings with it.
+            pipedProviders: [{ id: 'gps' }],
+            interfaces: { nmea0183: true },
             historyApi: configuredDefault
               ? { defaultProvider: configuredDefault }
               : undefined
@@ -354,10 +366,47 @@ describe('History API v2', () => {
       return registry
     }
 
+    // Recording the first provider writes settings, so every test in
+    // this block goes through a stubbed writeSettingsFile: it captures
+    // what would be persisted, and its callback runs inline so the
+    // assertions need no timing guess. Stubbing also keeps the block
+    // independent of whether an earlier suite built a Server, which
+    // disables settings writes for the whole process.
+    let settingsWrites: Array<{
+      historyApi?: { defaultProvider?: string }
+      pipedProviders?: unknown[]
+      interfaces?: Record<string, boolean>
+    }>
+    let writeOutcome: (cb: (err?: Error) => void) => void
+    let restoreWriteSettings: () => void
+
+    beforeEach(() => {
+      settingsWrites = []
+      writeOutcome = (cb) => cb()
+      // eslint-disable-next-line @typescript-eslint/no-require-imports
+      const config = require('../dist/config/config')
+      const original = config.writeSettingsFile
+      config.writeSettingsFile = (
+        _app: unknown,
+        settings: (typeof settingsWrites)[number],
+        cb: (err?: Error) => void
+      ) => {
+        settingsWrites.push(settings)
+        writeOutcome(cb)
+      }
+      restoreWriteSettings = () => {
+        config.writeSettingsFile = original
+      }
+    })
+
     afterEach(() => {
+      restoreWriteSettings()
       registries.splice(0).forEach((r) => r.stop())
     })
 
+    const recordedDefault = (app: TestApp) =>
+      app.config.settings.historyApi?.defaultProvider
+
     const VALUES_QUERY: ValuesRequest = {
       duration: Temporal.Duration.from({ minutes: 15 }),
       pathSpecs: []
@@ -403,14 +452,142 @@ describe('History API v2', () => {
       ;(await defaultOf(app)).should.equal(providerContext('kip'))
     })
 
-    it('defaults to the first registered provider without configuration', async function () {
+    it('serves the first registered provider when settings name none', async function () {
       const app = makeApp()
       const registry = makeRegistry(app)
       registry.registerHistoryApiProvider('kip', provider('kip'))
       registry.registerHistoryApiProvider('questdb', provider('questdb'))
       ;(await defaultOf(app)).should.equal(providerContext('kip'))
     })
 
+    it('records the first registered provider as the configured default', function () {
+      const app = makeApp()
+      const registry = makeRegistry(app)
+      registry.registerHistoryApiProvider('questdb', provider('questdb'))
+      settingsWrites.length.should.equal(1)
+      settingsWrites[0].historyApi!.defaultProvider!.should.equal('questdb')
+      recordedDefault(app)!.should.equal('questdb')
+    })
+
+    it('records without dropping the rest of the settings', function () {
+      const app = makeApp()
+      const registry = makeRegistry(app)
+      registry.registerHistoryApiProvider('questdb', provider('questdb'))
+      settingsWrites[0].should.deep.equal({
+        pipedProviders: [{ id: 'gps' }],
+        interfaces: { nmea0183: true },
+        historyApi: { defaultProvider: 'questdb' }
+      })
+    })
+
+    it('keeps the recorded default when a second provider registers', async function () {
+      const app = makeApp()
+      const registry = makeRegistry(app)
+      registry.registerHistoryApiProvider('questdb', provider('questdb'))
+      registry.registerHistoryApiProvider('kip', provider('kip'))
+      settingsWrites.length.should.equal(1)
+      recordedDefault(app)!.should.equal('questdb')
+      ;(await defaultOf(app)).should.equal(providerContext('questdb'))
+    })
+
+    it('honours the recorded default on the next start', async function () {
+      const app = makeApp()
+      const first = makeRegistry(app)
+      first.registerHistoryApiProvider('questdb', provider('questdb'))
+
+      // A fresh registry over the settings the first one wrote, with the
+      // providers registering in the order that used to decide it.
+      const restarted = makeRegistry(app)
+      restarted.registerHistoryApiProvider('kip', provider('kip'))
+      restarted.registerHistoryApiProvider('questdb', provider('questdb'))
+      ;(await defaultOf(app)).should.equal(providerContext('questdb'))
+    })
+
+    it('leaves a configured default alone', function () {
+      const app = makeApp('kip')
+      const registry = makeRegistry(app)
+      registry.registerHistoryApiProvider('questdb', provider('questdb'))
+      settingsWrites.length.should.equal(0)
+      recordedDefault(app)!.should.equal('kip')
+    })
+
+    it('treats an empty configured default as none', function () {
+      const app = makeApp()
+      app.config.settings.historyApi = { defaultProvider: '' }
+      const registry = makeRegistry(app)
+      registry.registerHistoryApiProvider('questdb', provider('questdb'))
+      recordedDefault(app)!.should.equal('questdb')
+    })
+
+    it('records nothing while settings hold runtime overrides', async function () {
+      const app = makeApp()
+      app.config.safeToPersistSettings = false
+      const registry = makeRegistry(app)
+      registry.registerHistoryApiProvider('questdb', provider('questdb'))
+      settingsWrites.length.should.equal(0)
+      chai.expect(recordedDefault(app)).to.equal(undefined)
+      ;(await defaultOf(app)).should.equal(providerContext('questdb'))
+    })
+
+    it('serves the provider even when recording it fails', async function () {
+      const app = makeApp()
+      writeOutcome = (cb) => cb(new Error('disk full'))
+      const registry = makeRegistry(app)
+      registry.registerHistoryApiProvider('questdb', provider('questdb'))
+      settingsWrites.length.should.equal(1)
+      chai.expect(recordedDefault(app)).to.equal(undefined)
+      ;(await defaultOf(app)).should.equal(providerContext('questdb'))
+    })
+
+    it('retries the first provider when a later one registers', async function () {
+      const app = makeApp()
+      writeOutcome = (cb) => cb(new Error('disk full'))
+      const registry = makeRegistry(app)
+      registry.registerHistoryApiProvider('questdb', provider('questdb'))
+      // The slot belongs to questdb even though its write failed, so the
+      // arrival of kip must retry questdb rather than end the attempts.
+      writeOutcome = (cb) => cb()
+      registry.registerHistoryApiProvider('kip', provider('kip'))
+      settingsWrites.length.should.equal(2)
+      settingsWrites[1].historyApi!.defaultProvider!.should.equal('questdb')
+      recordedDefault(app)!.should.equal('questdb')
+      ;(await defaultOf(app)).should.equal(providerContext('questdb'))
+    })
+
+    it('drops the pending default when that provider unregisters', function () {
+      const app = makeApp()
+      writeOutcome = (cb) => cb(new Error('disk full'))
+      const registry = makeRegistry(app)
+      registry.registerHistoryApiProvider('questdb', provider('questdb'))
+      registry.unregisterHistoryApiProvider('questdb')
+      // questdb is gone, so nothing may record it. kip arrives as the
+      // only provider and claims the slot itself.
+      writeOutcome = (cb) => cb()
+      registry.registerHistoryApiProvider('kip', provider('kip'))
+      recordedDefault(app)!.should.equal('kip')
+    })
+
+    it('reports a failed recording once per run', function () {
+      const app = makeApp()
+      writeOutcome = (cb) => cb(new Error('disk full'))
+      const registry = makeRegistry(app)
+      const reported: unknown[][] = []
+      const originalError = console.error
+      console.error = (...args: unknown[]) => {
+        reported.push(args)
+      }
+      try {
+        // A plugin that reconnects registers again; the retry must not
+        // report again, or it floods the log ring the admin UI reads.
+        registry.registerHistoryApiProvider('questdb', provider('questdb'))
+        registry.registerHistoryApiProvider('questdb', provider('questdb'))
+      } finally {
+        console.error = originalError
+      }
+      settingsWrites.length.should.equal(2)
+      reported.length.should.equal(1)
+    })
+
     it('rejects when no provider is registered', async function () {
       const app = makeApp('questdb')
       makeRegistry(app)
@@ -451,14 +628,14 @@ describe('History API v2', () => {
       app.notifications.length.should.equal(0)
     })
 
-    it('reports no configured provider for an empty configured id', function () {
+    it('records over an empty configured id', function () {
       const app = makeApp()
       // makeApp takes a truthy id, so the empty value goes in directly.
       app.config.settings.historyApi = { defaultProvider: '' }
       const registry = makeRegistry(app)
       registry.registerHistoryApiProvider('questdb', provider('questdb'))
       const event = app.serverEvents[app.serverEvents.length - 1]
-      chai.expect(event.configuredId).to.equal(undefined)
+      event.configuredId!.should.equal('questdb')
       event.defaultId!.should.equal('questdb')
     })
 
@@ -472,6 +649,19 @@ describe('History API v2', () => {
       const lastEvent = (app: TestApp) =>
         app.serverEvents[app.serverEvents.length - 1]
 
+      it('reports the recorded provider as configured on an unconfigured server', function () {
+        const app = makeApp()
+        const registry = makeRegistry(app)
+
+        registry.registerHistoryApiProvider('questdb', provider('questdb'))
+        lastEvent(app).should.deep.equal({
+          ids: ['questdb'],
+          defaultId: 'questdb',
+          configuredId: 'questdb',
+          configuredAvailable: true
+        })
+      })
+
       it('emits full state on register and unregister', function () {
         const app = makeApp('questdb')
         const registry = makeRegistry(app)