diff --git a/src/i18n/en.json b/src/i18n/en.json index a83442f..d9c3b81 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -353,6 +353,7 @@ "failedObject": "The problem is in {{object}}.", "noAnswer": "The server did not answer.", "notConfirmed": "The server did not confirm the reload.", + "notReloaded": "The server did not reload its settings.", "openObject": "Open it", "stillRunning": "The server keeps running on the settings it had. Your changes are saved and apply once this is fixed." }, diff --git a/src/lib/settingsApply.test.ts b/src/lib/settingsApply.test.ts index 07faa5d..2fc5bcd 100644 --- a/src/lib/settingsApply.test.ts +++ b/src/lib/settingsApply.test.ts @@ -8,10 +8,12 @@ import { describe, it, expect } from 'vitest'; import { describeApplyFailure, describeRequestFailure, + describeServerReload, + registryWrites, reloadActionFor, reloadActionsFor, + serverAppliesWrite, writesRegistry, - writtenRegistryTypes, } from './settingsApply'; import type { JmapMethodResponse } from '@/types/jmap'; @@ -38,8 +40,26 @@ describe('reloadActionFor', () => { expect(reloadActionFor('x:Authentication')).toBeNull(); }); + it('reloads allowed IPs in full: they are part of the settings, not the blocked list', () => { + expect(reloadActionFor('x:AllowedIp')).toBe('ReloadSettings'); + }); + it('skips data read live, operations and stores', () => { - for (const type of ['Account', 'Domain', 'DkimSignature', 'Tenant', 'Action', 'QueuedMessage', 'DataStore']) { + for (const type of [ + 'Account', + 'Alert', + 'DnsServer', + 'Domain', + 'DkimSignature', + 'Enterprise', + 'SpamLlm', + 'Tenant', + 'Action', + 'QueuedMessage', + 'DataStore', + 'MetricsStore', + 'TracingStore', + ]) { expect(reloadActionFor(`x:${type}`)).toBeNull(); } }); @@ -78,7 +98,62 @@ describe('registry writes', () => { ['x:Certificate/set', { destroyed: ['c1'] }, '2'], ['error', { type: 'serverFail' }, '3'], ]; - expect(writtenRegistryTypes(responses)).toEqual(['x:MtaRoute', 'x:Certificate']); + expect(registryWrites(responses)).toEqual([{ objectName: 'x:MtaRoute' }, { objectName: 'x:Certificate' }]); + }); + + it("reads the server's own reload report where there is one", () => { + const responses: JmapMethodResponse[] = [ + ['x:MtaRoute/set', { updated: { a: null }, 'x:settingsReload': { applied: true } }, '0'], + [ + 'x:Tracer/set', + { + created: { t: { id: 't1' } }, + 'x:settingsReload': { applied: false, description: 'Saved, but the running settings were not reloaded. x' }, + }, + '1', + ], + ['x:Domain/set', { created: { d: { id: 'd1' } } }, '2'], + ['x:MtaHook/set', { updated: { h: null }, 'x:settingsReload': 'yes' }, '3'], + ]; + expect(registryWrites(responses)).toEqual([ + { objectName: 'x:MtaRoute', serverReload: { applied: true } }, + { + objectName: 'x:Tracer', + serverReload: { applied: false, description: 'Saved, but the running settings were not reloaded. x' }, + }, + { objectName: 'x:Domain' }, + // Not the shape a server sends: treated as absent. + { objectName: 'x:MtaHook' }, + ]); + }); + + it('leaves applying to the server when it reported, except for allowed IPs', () => { + expect(serverAppliesWrite({ objectName: 'x:MtaRoute', serverReload: { applied: true } })).toBe(true); + expect(serverAppliesWrite({ objectName: 'x:MtaRoute', serverReload: { applied: false } })).toBe(true); + expect(serverAppliesWrite({ objectName: 'x:MtaRoute' })).toBe(false); + expect(serverAppliesWrite({ objectName: 'x:AllowedIp', serverReload: { applied: true } })).toBe(false); + }); +}); + +describe('describeServerReload', () => { + it('drops the lead-in the banner already says and names the object', () => { + expect( + describeServerReload({ + applied: false, + description: + 'Saved, but the running settings were not reloaded. Tracer with id b: Only one console tracer is allowed', + }), + ).toEqual({ + message: 'Tracer with id b: Only one console tracer is allowed', + object: { object: 'Tracer', id: 'b' }, + }); + }); + + it('keeps a description it does not recognize, and copes with none', () => { + expect(describeServerReload({ applied: false, description: 'Store unavailable' })).toEqual({ + message: 'Store unavailable', + }); + expect(describeServerReload({ applied: false })).toEqual({ message: 'The server did not reload its settings.' }); }); }); diff --git a/src/lib/settingsApply.ts b/src/lib/settingsApply.ts index bb52df5..c24450c 100644 --- a/src/lib/settingsApply.ts +++ b/src/lib/settingsApply.ts @@ -11,9 +11,15 @@ * A write to the registry is stored at once, but most settings only take * effect when the server rebuilds its configuration from the registry: the * x:Action ReloadSettings action, which also carries the change to every - * node of a cluster. The server does that by itself on a write for a few - * types only (directories and the default authentication). Everything else - * waited for someone to open Management > Actions and reload by hand. + * node of a cluster. Older servers do that by themselves on a write for a few + * types only (directories and the default authentication); everything else + * waits for a reload, which the admin sends (settingsApplyStore). + * + * Newer servers reload after every write that needs it and say how it went + * in the set response's `x:settingsReload` ({applied, description}), absent + * when the write needed no reload. A write that carries it needs nothing from + * the admin; the table below is what an older server, which never sends it, + * still needs, and it follows the server's own list of what each type needs. * * A reload is all or nothing: the new configuration replaces the running one * only when every settings object builds, so applying straight after a save @@ -43,6 +49,8 @@ const OWN_ACTION: Record = { MemoryLookupKey: 'ReloadLookupStores', MemoryLookupKeyValue: 'ReloadLookupStores', BlockedIp: 'ReloadBlockedIps', + // Not AllowedIp: allowed addresses are part of the full settings, and only + // ReloadSettings rebuilds them. }; /** Types that need no reload after a write. Anything not listed here is reloaded. */ @@ -54,15 +62,19 @@ const NOTHING_TO_APPLY = new Set([ 'Account', 'AccountPassword', 'AccountSettings', + 'Alert', 'ApiKey', 'AppPassword', + 'DnsServer', 'Domain', 'DkimSignature', + 'Enterprise', 'MailingList', 'MaskedEmail', 'OAuthClient', 'PublicKey', 'Role', + 'SpamLlm', 'Tenant', // Operations, records and telemetry rather than settings. 'Action', @@ -86,7 +98,9 @@ const NOTHING_TO_APPLY = new Set([ 'Coordinator', 'DataStore', 'InMemoryStore', + 'MetricsStore', 'SearchStore', + 'TracingStore', // Applications are unpacked by their own manager, which no reload reaches. 'Application', ]); @@ -110,9 +124,43 @@ export function writesRegistry(methodCalls: JmapMethodCall[]): boolean { return methodCalls.some(([name]) => REGISTRY_SET.test(name)); } -/** The registry types a response says were created, changed or destroyed. */ -export function writtenRegistryTypes(methodResponses: JmapMethodResponse[]): string[] { - const types = new Set(); +/** What a newer server says about applying a registry write (`x:settingsReload`). */ +export interface ServerReload { + /** The running settings, on every node, include the write. */ + applied: boolean; + /** Why they don't, when they don't. */ + description?: string; +} + +/** A registry type a request created, changed or destroyed, and what the server said about applying it. */ +export interface RegistryWrite { + objectName: string; + /** Absent from older servers, and from newer ones when the write needed no reload. */ + serverReload?: ServerReload; +} + +/** + * Types whose `x:settingsReload` doesn't tell the whole story. The server + * answers an AllowedIp write with the blocked-IP reload, but allowed + * addresses are only rebuilt by a full reload, so the admin still sends one. + */ +const SERVER_RELOAD_INCOMPLETE = new Set(['x:AllowedIp']); + +/** Whether a write's `x:settingsReload` means the admin has nothing to send for it. */ +export function serverAppliesWrite(write: RegistryWrite): boolean { + return write.serverReload !== undefined && !SERVER_RELOAD_INCOMPLETE.has(write.objectName); +} + +function readServerReload(value: unknown): ServerReload | undefined { + if (!value || typeof value !== 'object') return undefined; + const { applied, description } = value as { applied?: unknown; description?: unknown }; + if (typeof applied !== 'boolean') return undefined; + return typeof description === 'string' && description ? { applied, description } : { applied }; +} + +/** The registry writes in a response: one per `x:/set` that created, changed or destroyed something. */ +export function registryWrites(methodResponses: JmapMethodResponse[]): RegistryWrite[] { + const writes: RegistryWrite[] = []; for (const [name, result] of methodResponses) { const match = REGISTRY_SET.exec(name); if (!match || !result) continue; @@ -124,10 +172,11 @@ export function writtenRegistryTypes(methodResponses: JmapMethodResponse[]): str (updated && Object.keys(updated).length > 0) || (destroyed && destroyed.length > 0) ) { - types.add(match[1]); + const serverReload = readServerReload(result['x:settingsReload']); + writes.push(serverReload ? { objectName: match[1], serverReload } : { objectName: match[1] }); } } - return [...types]; + return writes; } /** The actions due for a set of written types, in the order they run. */ @@ -172,3 +221,19 @@ export function describeRequestFailure(err: unknown): ApplyFailure { } return { message: i18n.t('settingsApply.noAnswer', 'The server did not answer.') }; } + +/** How the server starts a refused reload's description; the banner says the same in its own words. */ +const SERVER_RELOAD_PREFIX = 'Saved, but the running settings were not reloaded. '; +/** How the server names the object that didn't build: " with id : ". */ +const SERVER_RELOAD_OBJECT = /^([A-Z][A-Za-z0-9]*) with id ([^\s:]+): /; + +/** Reads a server's refused reload (`x:settingsReload` with applied: false) into something to show. */ +export function describeServerReload(reload: ServerReload): ApplyFailure { + let message = reload.description?.trim() ?? ''; + if (message.startsWith(SERVER_RELOAD_PREFIX)) message = message.slice(SERVER_RELOAD_PREFIX.length).trim(); + if (!message) { + return { message: i18n.t('settingsApply.notReloaded', 'The server did not reload its settings.') }; + } + const match = SERVER_RELOAD_OBJECT.exec(message); + return match ? { message, object: { object: match[1], id: match[2] } } : { message }; +} diff --git a/src/services/jmap/client.ts b/src/services/jmap/client.ts index 2218a89..f5fc007 100644 --- a/src/services/jmap/client.ts +++ b/src/services/jmap/client.ts @@ -12,7 +12,7 @@ import { apiFetch } from '@/services/api'; import { logJmapExchange } from '@/lib/debug'; import type { JmapMethodCall, JmapMethodResponse, JmapQueryResponse, JmapResponse } from '@/types/jmap'; import type { Schema } from '@/types/schema'; -import { writesRegistry, writtenRegistryTypes } from '@/lib/settingsApply'; +import { registryWrites, writesRegistry, type RegistryWrite } from '@/lib/settingsApply'; const JMAP_USING = [ 'urn:ietf:params:jmap:core', @@ -39,11 +39,12 @@ export function getAccountId(objectType: string): string { /** * inbuxa: told about every registry write, so that saved settings can be * applied on the server (settingsApplyStore). `started` comes before the - * request goes out and `finished` after it settles, with the types it changed. + * request goes out and `finished` after it settles, with the types it changed + * and what the server said about applying them. */ export interface RegistryWriteListener { started(): void; - finished(objectNames: string[]): void; + finished(writes: RegistryWrite[]): void; } let registryWriteListener: RegistryWriteListener | null = null; @@ -60,10 +61,10 @@ export async function jmapRequest( const listener = writesRegistry(methodCalls) ? registryWriteListener : null; if (!listener) return sendJmapRequest(methodCalls, signal, extraUsing); listener.started(); - let written: string[] = []; + let written: RegistryWrite[] = []; try { const responses = await sendJmapRequest(methodCalls, signal, extraUsing); - written = writtenRegistryTypes(responses); + written = registryWrites(responses); return responses; } finally { listener.finished(written); diff --git a/src/stores/settingsApplyStore.test.ts b/src/stores/settingsApplyStore.test.ts index 897480f..1f3ed2d 100644 --- a/src/stores/settingsApplyStore.test.ts +++ b/src/stores/settingsApplyStore.test.ts @@ -6,6 +6,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import type { RegistryWriteListener } from '@/services/jmap/client'; +import type { RegistryWrite } from '@/lib/settingsApply'; import type { JmapMethodCall, JmapMethodResponse } from '@/types/jmap'; const mocks = vi.hoisted(() => ({ @@ -26,11 +27,22 @@ vi.mock('@/hooks/use-toast', () => ({ toast: mocks.toast })); import { APPLY_DELAY_MS, resetSettingsApplyForTests, useSettingsApplyStore } from './settingsApplyStore'; +// A save on an older server: no x:settingsReload in the response. function save(...types: string[]) { mocks.listener!.started(); - mocks.listener!.finished(types); + mocks.listener!.finished(types.map((objectName) => ({ objectName }))); } +// A save on a newer server, which applied it or said why not. +function serverSave(objectName: string, applied: boolean, description?: string) { + const write: RegistryWrite = { objectName, serverReload: description ? { applied, description } : { applied } }; + mocks.listener!.started(); + mocks.listener!.finished([write]); +} + +const REFUSED = + 'Saved, but the running settings were not reloaded. Tracer with id b: Only one console tracer is allowed'; + function reloadCreates(call: JmapMethodCall): unknown[] { expect(call[0]).toBe('x:Action/set'); return Object.values((call[1] as { create: Record }).create); @@ -80,7 +92,7 @@ describe('settingsApplyStore', () => { mocks.listener!.started(); await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS * 5); expect(mocks.jmapRequest).not.toHaveBeenCalled(); - mocks.listener!.finished(['x:Certificate']); + mocks.listener!.finished([{ objectName: 'x:Certificate' }]); await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); expect(mocks.jmapRequest).toHaveBeenCalledTimes(1); @@ -164,4 +176,174 @@ describe('settingsApplyStore', () => { useSettingsApplyStore.getState().dismiss(); expect(useSettingsApplyStore.getState()).toMatchObject({ failure: null, pending: ['ReloadSettings'] }); }); + describe('a server that applies writes itself', () => { + it('sends nothing and says "Saved and applied" once per burst', async () => { + serverSave('x:MtaDeliverySchedule', true); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS / 2); + serverSave('x:MtaRoute', true); + serverSave('x:Certificate', true); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + + expect(mocks.jmapRequest).not.toHaveBeenCalled(); + expect(mocks.toast).toHaveBeenCalledTimes(1); + expect(mocks.toast).toHaveBeenCalledWith( + expect.objectContaining({ title: 'Saved and applied', variant: 'success' }), + ); + expect(useSettingsApplyStore.getState()).toMatchObject({ pending: [], failure: null }); + }); + + it("leaves the toast to the form for types it reloads that the admin doesn't", async () => { + serverSave('x:Directory', true); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS * 2); + expect(mocks.jmapRequest).not.toHaveBeenCalled(); + expect(mocks.toast).not.toHaveBeenCalled(); + }); + + it("shows the server's reason, and Apply now sends ReloadSettings", async () => { + serverSave('x:Tracer', false, REFUSED); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS * 2); + + expect(mocks.jmapRequest).not.toHaveBeenCalled(); + expect(mocks.toast).not.toHaveBeenCalled(); + expect(useSettingsApplyStore.getState()).toMatchObject({ + pending: ['ReloadSettings'], + failure: { + message: 'Tracer with id b: Only one console tracer is allowed', + object: { object: 'Tracer', id: 'b' }, + }, + }); + + mocks.jmapRequest.mockResolvedValueOnce(answer(['reload-0'])); + await useSettingsApplyStore.getState().applyNow(); + expect(reloadCreates(mocks.jmapRequest.mock.calls[0][0][0])).toEqual([{ '@type': 'ReloadSettings' }]); + expect(useSettingsApplyStore.getState()).toMatchObject({ pending: [], failure: null }); + expect(mocks.toast).toHaveBeenCalledWith(expect.objectContaining({ title: 'Saved and applied' })); + }); + + it('lets a later write in the burst that applied have the last word', async () => { + serverSave('x:Tracer', false, REFUSED); + serverSave('x:Tracer', true); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + expect(useSettingsApplyStore.getState()).toMatchObject({ pending: [], failure: null }); + expect(mocks.toast).toHaveBeenCalledTimes(1); + }); + + it('clears an earlier failure once a later write applies', async () => { + serverSave('x:Tracer', false, REFUSED); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + expect(useSettingsApplyStore.getState().failure).not.toBeNull(); + + serverSave('x:Tracer', true); + expect(useSettingsApplyStore.getState()).toMatchObject({ pending: [], failure: null }); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + expect(mocks.jmapRequest).not.toHaveBeenCalled(); + expect(mocks.toast).toHaveBeenCalledTimes(1); + }); + + it('does not retry a refused reload on an unrelated save', async () => { + serverSave('x:Tracer', false, REFUSED); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + save('x:Account'); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS * 2); + expect(mocks.jmapRequest).not.toHaveBeenCalled(); + expect(useSettingsApplyStore.getState().failure).not.toBeNull(); + }); + + it('still reloads allowed IPs in full: the report only covers the blocked list', async () => { + mocks.jmapRequest.mockResolvedValueOnce(answer(['reload-0'])); + serverSave('x:AllowedIp', true); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + expect(reloadCreates(mocks.jmapRequest.mock.calls[0][0][0])).toEqual([{ '@type': 'ReloadSettings' }]); + expect(mocks.toast).toHaveBeenCalledTimes(1); + }); + }); + + describe('a burst mixing both kinds of answer', () => { + it('sends only what the server left to the admin, and says so once', async () => { + mocks.jmapRequest.mockResolvedValueOnce(answer(['reload-0'])); + serverSave('x:MtaRoute', true); + serverSave('x:Certificate', true); + // A type the server doesn't answer for, as an older node would. + save('x:SomethingNew'); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + + expect(mocks.jmapRequest).toHaveBeenCalledTimes(1); + expect(reloadCreates(mocks.jmapRequest.mock.calls[0][0][0])).toEqual([{ '@type': 'ReloadSettings' }]); + expect(mocks.toast).toHaveBeenCalledTimes(1); + expect(useSettingsApplyStore.getState()).toMatchObject({ pending: [], failure: null }); + }); + + it("doesn't take back a reload queued after the server applied one", async () => { + mocks.jmapRequest.mockResolvedValueOnce(answer(['reload-0'])); + save('x:MtaRoute'); + serverSave('x:MtaHook', true); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + // The server's reload came after the first write and covers it. + expect(mocks.jmapRequest).not.toHaveBeenCalled(); + expect(mocks.toast).toHaveBeenCalledTimes(1); + // Nothing is left over to send with a later refusal either. + serverSave('x:Tracer', false, REFUSED); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + expect(mocks.jmapRequest).not.toHaveBeenCalled(); + serverSave('x:Tracer', true); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + expect(mocks.toast).toHaveBeenCalledTimes(2); + + serverSave('x:MtaHook', true); + save('x:MtaRoute'); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + expect(mocks.jmapRequest).toHaveBeenCalledTimes(1); + expect(mocks.toast).toHaveBeenCalledTimes(3); + }); + + it("lets the admin's reload settle a server refusal in the same burst", async () => { + mocks.jmapRequest.mockResolvedValueOnce(answer(['reload-0'])); + serverSave('x:Tracer', false, REFUSED); + save('x:SomethingNew'); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + + expect(mocks.jmapRequest).toHaveBeenCalledTimes(1); + expect(useSettingsApplyStore.getState()).toMatchObject({ pending: [], failure: null }); + expect(mocks.toast).toHaveBeenCalledTimes(1); + }); + + it('shows a refusal once when the admin reload fails too', async () => { + mocks.jmapRequest.mockResolvedValueOnce( + answer([], { 'reload-0': { type: 'validationFailed', description: 'Only one console tracer is allowed' } }), + ); + serverSave('x:Tracer', false, REFUSED); + save('x:SomethingNew'); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS * 2); + + expect(mocks.jmapRequest).toHaveBeenCalledTimes(1); + expect(mocks.toast).not.toHaveBeenCalled(); + expect(useSettingsApplyStore.getState()).toMatchObject({ + pending: ['ReloadSettings'], + failure: { message: 'Only one console tracer is allowed' }, + }); + }); + + it('reports a server answer that lands while a reload is out once it is back', async () => { + let answerReload: (r: JmapMethodResponse[]) => void = () => {}; + mocks.jmapRequest.mockReturnValueOnce( + new Promise((resolve) => { + answerReload = resolve; + }), + ); + save('x:MtaRoute'); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + expect(useSettingsApplyStore.getState().applying).toBe(true); + + serverSave('x:Certificate', false, 'Saved, but the running settings were not reloaded. Bad key'); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + answerReload(answer(['reload-0'])); + await vi.advanceTimersByTimeAsync(APPLY_DELAY_MS); + + expect(mocks.jmapRequest).toHaveBeenCalledTimes(1); + expect(useSettingsApplyStore.getState()).toMatchObject({ + pending: ['ReloadTlsCertificates'], + failure: { message: 'Bad key' }, + }); + }); + }); }); diff --git a/src/stores/settingsApplyStore.ts b/src/stores/settingsApplyStore.ts index 7694d00..8f2d17b 100644 --- a/src/stores/settingsApplyStore.ts +++ b/src/stores/settingsApplyStore.ts @@ -7,15 +7,18 @@ /** * inbuxa: applies saved settings on the running server. * - * Every registry write that needs it queues a reload action (see - * lib/settingsApply). Once no write has been in flight for APPLY_DELAY_MS the - * queued actions go out together in one x:Action/set, so a bulk edit, or a - * page that saves several objects in a row, costs one reload rather than one - * per object. A reload that fails stays queued and shows in - * SettingsApplyBanner until it is applied, by "Apply now" or by the next save. + * A newer server applies a registry write itself and says how it went in the + * set response (`x:settingsReload`); the admin only reports that. An older + * server doesn't, so every write there that needs it queues a reload action + * (see lib/settingsApply). Once no write has been in flight for APPLY_DELAY_MS + * the queued actions go out together in one x:Action/set, so a bulk edit, or + * a page that saves several objects in a row, costs one reload rather than + * one per object. * - * If the server one day applies these writes by itself, the reload sent here - * is a second, harmless one: one per burst of saves, never one per object. + * Either way a burst of saves ends in one result: a "Saved and applied" toast, + * or SettingsApplyBanner with why the settings weren't applied. What wasn't + * applied stays queued until it is, by "Apply now", by the next save that + * needs the same reload, or by the server applying a later write of that kind. */ import { create } from 'zustand'; @@ -26,8 +29,11 @@ import { RELOAD_ORDER, describeApplyFailure, describeRequestFailure, - reloadActionsFor, + describeServerReload, + reloadActionFor, + serverAppliesWrite, type ApplyFailure, + type RegistryWrite, type ReloadAction, } from '@/lib/settingsApply'; import type { JmapSetError } from '@/types/jmap'; @@ -35,13 +41,13 @@ import type { JmapSetError } from '@/types/jmap'; export const APPLY_DELAY_MS = 600; interface SettingsApplyState { - /** Actions waiting to be sent: queued by saves, or left over from a failed attempt. */ + /** Actions not known to be applied: queued by saves, or left over from a failed attempt. */ pending: ReloadAction[]; applying: boolean; /** Why the last attempt didn't apply, until one does. */ failure: ApplyFailure | null; - /** Queues the actions the written types need, and applies them once writes settle. */ - noteWrites: (objectNames: string[]) => void; + /** Takes note of registry writes: what the server applied, and what the admin has to. */ + noteWrites: (writes: RegistryWrite[]) => void; /** Sends whatever is queued now, without waiting. */ applyNow: () => Promise; /** Hides the failure. What failed stays queued for the next save. */ @@ -55,6 +61,12 @@ let applyAgain = false; // reload, only a save that needs one tries again by itself; editing an // account, say, doesn't rerun a reload that is known to fail. let freshlyQueued = false; +// What the server reported for the writes since the burst last settled: that +// it applied a settings object, and the reloads it couldn't apply (the last +// word per action). +let serverReported = false; +let serverApplied = false; +const serverFailures = new Map(); function merge(a: ReloadAction[], b: ReloadAction[]): ReloadAction[] { const all = new Set([...a, ...b]); @@ -64,25 +76,78 @@ function merge(a: ReloadAction[], b: ReloadAction[]): ReloadAction[] { function schedule() { if (timer) clearTimeout(timer); timer = null; + if (writesInFlight > 0) return; const { pending, failure } = useSettingsApplyStore.getState(); - if (pending.length === 0 || writesInFlight > 0) return; - if (failure && !freshlyQueued) return; + const send = pending.length > 0 && (freshlyQueued || (!failure && serverFailures.size === 0)); + if (!send && !serverReported) return; timer = setTimeout(() => { timer = null; - void useSettingsApplyStore.getState().applyNow(); + settle(); }, APPLY_DELAY_MS); } +/** Ends a burst of saves: sends what the admin has to, or reports what the server did. */ +function settle() { + const store = useSettingsApplyStore.getState(); + if (store.applying) { + // Report once the reload that is out comes back. + applyAgain = true; + return; + } + const serverFailure = [...serverFailures.values()].pop() ?? null; + const announce = serverApplied; + serverReported = false; + serverApplied = false; + serverFailures.clear(); + + if (serverFailure) useSettingsApplyStore.setState({ failure: serverFailure }); + const { pending, failure } = useSettingsApplyStore.getState(); + if (pending.length > 0 && (freshlyQueued || !failure)) { + // An older server's writes, or a reload a dismissed failure left queued: + // the admin's reload speaks for the whole burst. + void store.applyNow(); + return; + } + if (announce && !failure) { + toast({ title: i18n.t('settingsApply.applied', 'Saved and applied'), variant: 'success' }); + } +} + export const useSettingsApplyStore = create()((set, get) => ({ pending: [], applying: false, failure: null, - noteWrites: (objectNames) => { - const due = reloadActionsFor(objectNames); - if (due.length > 0) { - freshlyQueued = true; - set({ pending: merge(get().pending, due) }); + noteWrites: (writes) => { + let pending = get().pending; + for (const write of writes) { + const own = reloadActionFor(write.objectName); + if (write.serverReload && serverAppliesWrite(write)) { + // The server has applied this write, or tried to: nothing to send. + // A type the admin sends nothing for, a directory say, was a full reload. + const action = own ?? 'ReloadSettings'; + serverReported = true; + if (write.serverReload.applied) { + serverFailures.delete(action); + pending = pending.filter((a) => a !== action); + // Only settings objects say "Saved and applied"; the rest have their form's toast. + if (own) serverApplied = true; + } else { + serverFailures.set(action, describeServerReload(write.serverReload)); + pending = merge(pending, [action]); + } + } else if (own) { + // An older server: the admin applies it. + freshlyQueued = true; + pending = merge(pending, [own]); + } + } + if (pending.length === 0) { + // The server has applied everything queued, and whatever failed before. + freshlyQueued = false; + set({ pending, failure: null }); + } else { + set({ pending }); } schedule(); }, @@ -99,6 +164,9 @@ export const useSettingsApplyStore = create()((set, get) => if (actions.length === 0) return; freshlyQueued = false; + // This reload comes after every write so far, so it has the last word on + // what the server said about them. + for (const action of actions) serverFailures.delete(action); set({ applying: true, pending: [] }); let notApplied: ReloadAction[] = []; let failure: ApplyFailure | null = null; @@ -133,10 +201,11 @@ export const useSettingsApplyStore = create()((set, get) => set({ applying: false, pending: merge(get().pending, notApplied), failure }); if (!failure) { + serverApplied = false; toast({ title: i18n.t('settingsApply.applied', 'Saved and applied'), variant: 'success' }); } - if (applyAgain) { + if (applyAgain || serverReported) { applyAgain = false; schedule(); } @@ -151,9 +220,9 @@ setRegistryWriteListener({ if (timer) clearTimeout(timer); timer = null; }, - finished(objectNames) { + finished(writes) { writesInFlight = Math.max(0, writesInFlight - 1); - useSettingsApplyStore.getState().noteWrites(objectNames); + useSettingsApplyStore.getState().noteWrites(writes); }, }); @@ -164,5 +233,8 @@ export function resetSettingsApplyForTests() { writesInFlight = 0; applyAgain = false; freshlyQueued = false; + serverReported = false; + serverApplied = false; + serverFailures.clear(); useSettingsApplyStore.setState({ pending: [], applying: false, failure: null }); }