Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
import React, { useState, useEffect, useCallback } from 'react'
import Alert from 'react-bootstrap/Alert'
import Card from 'react-bootstrap/Card'
import Col from 'react-bootstrap/Col'
import Form from 'react-bootstrap/Form'
import Row from 'react-bootstrap/Row'
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'
import { faClockRotateLeft } from '@fortawesome/free-solid-svg-icons/faClockRotateLeft'

const PROVIDERS_PATH = '/signalk/v2/api/history/_providers'

interface ProvidersState {
ids: string[]
/** Provider currently serving unqualified History API requests */
defaultId: string | null
/** Provider persisted in server settings; may not be registered */
configuredId: string | null
}

const HistoryProviderSettings: React.FC = () => {
const [providers, setProviders] = useState<ProvidersState | null>(null)
const [saveError, setSaveError] = useState<string | null>(null)
const [saved, setSaved] = useState(false)

const fetchProviders = useCallback(async () => {
try {
const [providersRes, defaultRes] = await Promise.all([
fetch(PROVIDERS_PATH, { credentials: 'include' }),
fetch(`${PROVIDERS_PATH}/_default`, { credentials: 'include' })
])
if (!providersRes.ok || !defaultRes.ok) {
return
}
const providersBody = (await providersRes.json()) as Record<
string,
{ isDefault: boolean }
>
const defaultBody = (await defaultRes.json()) as {
id?: string
configured?: string
}
setProviders({
ids: Object.keys(providersBody),
defaultId: defaultBody.id ?? null,
configuredId: defaultBody.configured ?? null
})
} catch (e) {
console.error('Failed to fetch history providers:', e)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}, [])

useEffect(() => {
fetchProviders()
}, [fetchProviders])

const handleChange = useCallback(
async (id: string) => {
setSaveError(null)
setSaved(false)
try {
const res = await fetch(`${PROVIDERS_PATH}/_default/${id}`, {
method: 'POST',
credentials: 'include'
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
if (res.ok) {
setSaved(true)
setTimeout(() => setSaved(false), 3000)
} else {
const body = await res.json()
setSaveError(body.message || `Save failed (HTTP ${res.status})`)
}
} catch (e) {
setSaveError(e instanceof Error ? e.message : 'Save failed')
}
await fetchProviders()
},
[fetchProviders]
)

if (providers === null) {
return null
}

const configuredButUnavailable =
providers.configuredId !== null &&
!providers.ids.includes(providers.configuredId)

return (
<Card>
<Card.Header>
<FontAwesomeIcon icon={faClockRotateLeft} />{' '}
<strong>History Provider</strong>
</Card.Header>
<Card.Body>
<Form.Group as={Row} className="mb-0">
<Col md={2}>
<Form.Label>Default Provider</Form.Label>
</Col>
<Col xs="12" md={10}>
{providers.ids.length === 0 ? (
<Form.Text className="text-muted">
No history providers are registered. Enable a plugin that
provides the History API to select a default.
</Form.Text>
) : (
<>
<Form.Select
value={providers.defaultId ?? ''}
onChange={(e: React.ChangeEvent<HTMLSelectElement>) =>
handleChange(e.target.value)
}
style={{ maxWidth: '300px' }}
>
{providers.ids.map((id) => (
<option key={id} value={id}>
{id}
</option>
))}
</Form.Select>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
<Form.Text className="text-muted">
Serves History API requests that do not specify a provider.
The setting persists across restarts and does not depend on
plugin load order.
</Form.Text>
</>
)}
{configuredButUnavailable && (
<Alert
variant="warning"
style={{ marginTop: '10px', marginBottom: 0 }}
>
The configured default provider &quot;{providers.configuredId}
&quot; is not currently available
{providers.defaultId
? ` — using "${providers.defaultId}" as fallback`
: ''}
. It will become the default again when its plugin is enabled.
</Alert>
)}
{saved && (
<Alert
variant="success"
style={{ marginTop: '10px', marginBottom: 0 }}
>
Default history provider saved.
</Alert>
)}
{saveError && (
<Alert
variant="danger"
style={{ marginTop: '10px', marginBottom: 0 }}
>
{saveError}
</Alert>
)}
</Col>
</Form.Group>
</Card.Body>
</Card>
)
}

export default HistoryProviderSettings
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import React from 'react'
import GnssPositionSettings from './GnssPositionSettings'
import UnitPreferencesSettings from './UnitPreferencesSettings'
import HistoryProviderSettings from './HistoryProviderSettings'

const PreferencesPage: React.FC = () => {
return (
<div className="animated fadeIn">
<GnssPositionSettings />
<UnitPreferencesSettings />
<HistoryProviderSettings />
</div>
)
}
Expand Down
62 changes: 47 additions & 15 deletions src/api/history/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,39 @@ import { Context, Path, SourceRef } from '@signalk/server-api'
import { createDebug } from '../../debug'
import { Request, Response } from 'express'
import { WithSecurityStrategy } from '../../security'
import { ConfigApp, writeSettingsFile } from '../../config/config'

import { Responses } from '../'

const debug = createDebug('signalk-server:api:history')

const HISTORY_API_PATH = `/signalk/v2/api/history`

interface HistoryApplication extends WithSecurityStrategy, IRouter {}
export interface HistoryApplication
extends WithSecurityStrategy, IRouter, ConfigApp, WithHistoryApi {}

export class HistoryApiHttpRegistry {
private historyProviders: Map<string, HistoryProvider> = new Map()
private defaultProviderId?: string
/** Persisted user choice; may reference a provider that is not
* currently registered (e.g. plugin disabled). */
private configuredProviderId?: string
proxy: HistoryApi

constructor(private app: HistoryApplication & WithHistoryApi) {
/** The configured provider when it is registered, otherwise the first
* registered provider as fallback. Keeps the default independent of
* plugin load order. */
private get defaultProviderId(): string | undefined {
if (
this.configuredProviderId &&
this.historyProviders.has(this.configuredProviderId)
) {
return this.configuredProviderId
}
return this.historyProviders.keys().next().value
}

constructor(private app: HistoryApplication) {
this.configuredProviderId = app.config.settings.historyApi?.defaultProvider
this.proxy = {
getValues: (query: ValuesRequest): Promise<ValuesResponse> => {
return this.defaultProvider().getValues(query)
Expand Down Expand Up @@ -72,9 +90,6 @@ export class HistoryApiHttpRegistry {
if (!this.historyProviders.has(pluginId)) {
this.historyProviders.set(pluginId, provider)
}
if (this.historyProviders.size === 1) {
this.defaultProviderId = pluginId
}
debug(
`Registered history api provider ${pluginId},`,
`total=${this.historyProviders.size},`,
Expand All @@ -87,9 +102,6 @@ export class HistoryApiHttpRegistry {
return
}
this.historyProviders.delete(pluginId)
if (pluginId === this.defaultProviderId) {
this.defaultProviderId = this.historyProviders.keys().next().value
}
debug(
`Unregistered history api provider ${pluginId},`,
`total=${this.historyProviders.size},`,
Expand Down Expand Up @@ -128,7 +140,8 @@ export class HistoryApiHttpRegistry {
debug(`**route = ${req.method} ${req.path}`)
try {
res.status(200).json({
id: this.defaultProviderId
id: this.defaultProviderId,
configured: this.configuredProviderId
})
} catch (err: unknown) {
res.status(400).json({
Expand Down Expand Up @@ -161,11 +174,21 @@ export class HistoryApiHttpRegistry {
throw new Error('Provider id not supplied!')
}
if (this.historyProviders.has(req.params.id)) {
this.defaultProviderId = req.params.id
res.status(200).json({
statusCode: 200,
state: 'COMPLETED',
message: `Default provider set to ${req.params.id}.`
this.configuredProviderId = req.params.id
this.saveConfiguredProvider((err) => {
if (err) {
res.status(500).json({
statusCode: 500,
state: 'FAILED',
message: `Failed to save settings: ${err.message}`
})
} else {
res.status(200).json({
statusCode: 200,
state: 'COMPLETED',
message: `Default provider set to ${req.params.id}.`
})
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Commit the provider selection only after persistence succeeds.

The code updates both configuredProviderId and app.config.settings before the write completes. On failure, the API returns 500 but the failed selection remains active in memory; concurrent POSTs can also leave runtime state different from disk.

Pass the requested ID into saveConfiguredProvider, write an immutable settings snapshot, and update runtime state only in the successful callback. Add a write-failure regression test.

Proposed fix
-            this.configuredProviderId = req.params.id
-            this.saveConfiguredProvider((err) => {
+            const providerId = req.params.id
+            this.saveConfiguredProvider(providerId, (err) => {
               if (err) {
                 res.status(500).json({
                   statusCode: 500,
                   state: 'FAILED',
                   message: `Failed to save settings: ${err.message}`
                 })
               } else {
+                this.configuredProviderId = providerId
+                this.app.config.settings.historyApi = {
+                  ...this.app.config.settings.historyApi,
+                  defaultProvider: providerId
+                }
                 res.status(200).json({
                   statusCode: 200,
                   state: 'COMPLETED',
-                  message: `Default provider set to ${req.params.id}.`
+                  message: `Default provider set to ${providerId}.`
                 })
               }
             })
-  private saveConfiguredProvider(cb: (err?: Error) => void) {
-    const settings = this.app.config.settings
-    settings.historyApi = {
-      ...settings.historyApi,
-      defaultProvider: this.configuredProviderId
+  private saveConfiguredProvider(
+    providerId: string,
+    cb: (err?: Error) => void
+  ) {
+    const settings = {
+      ...this.app.config.settings,
+      historyApi: {
+        ...this.app.config.settings.historyApi,
+        defaultProvider: providerId
+      }
     }
     writeSettingsFile(this.app, settings, cb)
   }

Also applies to: 247-253

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/api/history/index.ts` around lines 177 - 191, Update the
provider-selection flow around configuredProviderId and saveConfiguredProvider
so the requested ID is passed into persistence, the write uses an immutable
settings snapshot, and runtime state is committed only from the successful
callback. Keep failed writes from changing the active provider, including under
concurrent POSTs, and add a regression test covering the write-failure case.

})
} else {
throw new Error(`Provider ${req.params.id} not found!`)
Expand Down Expand Up @@ -221,6 +244,15 @@ export class HistoryApiHttpRegistry {
)
}

private saveConfiguredProvider(cb: (err?: Error) => void) {
const settings = this.app.config.settings
settings.historyApi = {
...settings.historyApi,
defaultProvider: this.configuredProviderId
}
writeSettingsFile(this.app, settings, cb)
}

private defaultProvider(): HistoryProvider {
if (
this.defaultProviderId &&
Expand Down
8 changes: 7 additions & 1 deletion src/api/history/openApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,11 @@ historyApiDoc.paths = {
id: {
type: 'string',
description: 'Provider identifier.'
},
configured: {
type: 'string',
description:
'Provider identifier persisted in server settings. May differ from `id` when the configured provider is not currently registered.'
}
},
example: { id: 'signalk-to-influxdb2' }
Expand All @@ -360,7 +365,8 @@ historyApiDoc.paths = {
post: {
tags: ['Provider'],
summary: 'Sets the default history provider.',
description: 'Sets the provider with the supplied `id` as the default.',
description:
'Sets the provider with the supplied `id` as the default and persists the choice in server settings.',
responses: {
default: { $ref: '#/components/responses/ErrorResponse' },
'200': { $ref: '#/components/responses/200OKResponse' }
Expand Down
6 changes: 4 additions & 2 deletions src/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { ResourcesApi } from './resources'
import { WeatherApi } from './weather'
import { AutopilotApi } from './autopilot'
import { RadarApi } from './radar'
import { HistoryApiHttpRegistry } from './history'
import { HistoryApiHttpRegistry, HistoryApplication } from './history'
import { SignalKApiId, WithFeatures } from '@signalk/server-api'
import { NotificationApi, NotificationApplication } from './notifications'
import {
Expand Down Expand Up @@ -96,7 +96,9 @@ export const startApis = (

const featuresApi = new FeaturesApi(app)

const historyApiHttpRegistry = new HistoryApiHttpRegistry(app)
const historyApiHttpRegistry = new HistoryApiHttpRegistry(
app as HistoryApplication
)
// eslint-disable-next-line @typescript-eslint/no-explicit-any
;(app as any).historyApiHttpRegistry = historyApiHttpRegistry
apiList.push('history')
Expand Down
8 changes: 8 additions & 0 deletions src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,14 @@ export interface Config {
courseApi?: {
apiOnly?: boolean
}
historyApi?: {
/** Plugin id of the History API provider to use as the default.
* 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. */
defaultProvider?: string
}
notifications?: {
manageNotifications?: boolean
}
Expand Down
Loading
Loading