From c7bd6444d25ce03903be39257f66b06497e98afb Mon Sep 17 00:00:00 2001 From: Matt Lyons Date: Thu, 21 Mar 2024 13:40:59 -0500 Subject: [PATCH] Add settings validation functions (#798) --- lib/papi-dts/papi.d.ts | 36 ++++++++++++++++ src/main/data/core-settings-info.data.ts | 39 ++++++++++++++++- .../services/settings.service-host.test.ts | 25 ++++++++++- src/main/services/settings.service-host.ts | 42 ++++++++++++++++++- src/shared/services/settings.service-model.ts | 42 +++++++++++++++++++ 5 files changed, 178 insertions(+), 6 deletions(-) diff --git a/lib/papi-dts/papi.d.ts b/lib/papi-dts/papi.d.ts index cf55164bdb..5271315e53 100644 --- a/lib/papi-dts/papi.d.ts +++ b/lib/papi-dts/papi.d.ts @@ -4941,6 +4941,8 @@ declare module 'shared/services/settings.service-model' { DataProviderUpdateInstructions, IDataProvider, } from '@papi/core'; + /** Name prefix for registered commands that call settings validators */ + export const CATEGORY_EXTENSION_SETTING_VALIDATOR = 'extensionSettingValidator'; /** * * This name is used to register the settings service data provider on the papi. You can use this @@ -4954,6 +4956,18 @@ declare module 'shared/services/settings.service-model' { * name to find the data provider when accessing it using the useData hook */ dataProviderName: 'platform.settingsServiceDataProvider'; + /** + * + * Registers a function that validates whether a new setting value is allowed to be set. + * + * @param key The string id of the setting to validate + * @param validator Function to call to validate the new setting value + * @returns Unsubscriber that should be called whenever the providing extension is deactivated + */ + registerValidator: ( + key: SettingName, + validator: SettingValidator, + ) => Promise; }>; /** * SettingDataTypes handles getting and setting Platform.Bible core application and extension @@ -4976,6 +4990,16 @@ declare module 'shared/services/settings.service-model' { export type AllSettingsData = { [SettingName in SettingNames]: SettingTypes[SettingName]; }; + /** Function that validates whether a new setting value should be allowed to be set */ + export type SettingValidator = ( + newValue: SettingTypes[SettingName], + currentValue: SettingTypes[SettingName], + allChanges: Partial, + ) => Promise; + /** Validators for all settings. Keys are setting keys, values are functions to validate new settings */ + export type AllSettingsValidators = { + [SettingName in SettingNames]: SettingValidator; + }; module 'papi-shared-types' { interface DataProviders { [settingsServiceDataProviderName]: ISettingsService; @@ -5026,6 +5050,18 @@ declare module 'shared/services/settings.service-model' { callback: (newSetting: SettingTypes[SettingName]) => void, options?: DataProviderSubscriberOptions, ): Promise; + /** + * + * Registers a function that validates whether a new setting value is allowed to be set. + * + * @param key The string id of the setting to validate + * @param validator Function to call to validate the new setting value + * @returns Unsubscriber that should be called whenever the providing extension is deactivated + */ + registerValidator( + key: SettingName, + validator: SettingValidator, + ): Promise; } & OnDidDispose & IDataProvider & typeof settingsServiceObjectToProxy; diff --git a/src/main/data/core-settings-info.data.ts b/src/main/data/core-settings-info.data.ts index bcea7f827a..578fe0f0ff 100644 --- a/src/main/data/core-settings-info.data.ts +++ b/src/main/data/core-settings-info.data.ts @@ -1,4 +1,6 @@ +import { AllSettingsValidators, SettingValidator } from '@shared/services/settings.service-model'; import { SettingNames, SettingTypes } from 'papi-shared-types'; +import { ScriptureReference } from 'platform-bible-utils'; /** Information about one setting */ type SettingInfo = { @@ -11,9 +13,42 @@ export type AllSettingsInfo = { }; /** Info about all settings built into core. Does not contain info for extensions' settings */ -const coreSettingsInfo: Partial = { +export const coreSettingsInfo: Partial = { 'platform.verseRef': { default: { bookNum: 1, chapterNum: 1, verseNum: 1 } }, 'platform.interfaceLanguage': { default: ['eng'] }, }; -export default coreSettingsInfo; +// TODO: Add range checking of BCV numbers given the current versification +export const verseRefSettingsValidator: SettingValidator<'platform.verseRef'> = async ( + newValue: ScriptureReference, +): Promise => { + return ( + 'bookNum' in newValue && + 'chapterNum' in newValue && + 'verseNum' in newValue && + typeof newValue.bookNum === 'number' && + typeof newValue.chapterNum === 'number' && + typeof newValue.verseNum === 'number' && + newValue.bookNum >= 0 && + newValue.chapterNum >= 0 && + newValue.verseNum >= 0 + ); +}; + +// TODO: Validate that strings in the array to match BCP 47 values once the i18n code is ready +export const interfaceLanguageValidator: SettingValidator<'platform.interfaceLanguage'> = async ( + newValue: string[], +): Promise => { + return ( + typeof newValue === 'object' && + Array.isArray(newValue) && + newValue.length > 0 && + typeof newValue[0] === 'string' + ); +}; + +/** Info about all settings built into core. Does not contain info for extensions' settings */ +export const coreSettingsValidators: Partial = { + 'platform.verseRef': verseRefSettingsValidator, + 'platform.interfaceLanguage': interfaceLanguageValidator, +}; diff --git a/src/main/services/settings.service-host.test.ts b/src/main/services/settings.service-host.test.ts index 5d315f33e8..ee748af0a8 100644 --- a/src/main/services/settings.service-host.test.ts +++ b/src/main/services/settings.service-host.test.ts @@ -28,11 +28,21 @@ jest.mock('@node/services/node-file-system.service', () => ({ jest.mock('@main/data/core-settings-info.data', () => ({ ...jest.requireActual('@main/data/core-settings-info.data'), __esModule: true, - default: { + coreSettingsInfo: { 'platform.verseRef': { default: { bookNum: 1, chapterNum: 1, verseNum: 1 } }, 'platform.interfaceLanguage': { default: ['eng'] }, 'settingsTest.noDefaultExists': {}, }, + coreSettingsValidators: { + 'platform.verseRef': async (): Promise => { + // Reject all attempts to set the verse ref + return false; + }, + 'platform.interfaceLanguage': async (): Promise => { + // Accept all attempts to set the interface language + return true; + }, + }, })); test('Get verseRef returns default value', async () => { @@ -68,7 +78,7 @@ test('No default specified for key', async () => { ); }); -test('Set verseRef returns true', async () => { +test('Set interfaceLanguage returns true', async () => { const result = await settingsProviderEngine.set( 'platform.interfaceLanguage', NEW_INTERFACE_LANGUAGE, @@ -85,3 +95,14 @@ test('Reset verseRef returns false', async () => { const result = await settingsProviderEngine.reset('platform.verseRef'); expect(result).toBe(false); }); + +test('Set verseRef throws', async () => { + const result = settingsProviderEngine.set('platform.verseRef', { + bookNum: 2, + chapterNum: 1, + verseNum: 1, + }); + await expect(result).rejects.toThrow( + "Error setting value for key 'platform.verseRef': Error: validation failed", + ); +}); diff --git a/src/main/services/settings.service-host.ts b/src/main/services/settings.service-host.ts index cd8ff44b30..7ce87e27e6 100644 --- a/src/main/services/settings.service-host.ts +++ b/src/main/services/settings.service-host.ts @@ -1,3 +1,4 @@ +import * as networkService from '@shared/services/network.service'; import IDataProviderEngine, { DataProviderEngine } from '@shared/models/data-provider-engine.model'; import { DataProviderDataType, @@ -6,16 +7,23 @@ import { import dataProviderService from '@shared/services/data-provider.service'; import { AllSettingsData, + CATEGORY_EXTENSION_SETTING_VALIDATOR, ISettingsService, SettingDataTypes, settingsServiceDataProviderName, settingsServiceObjectToProxy, } from '@shared/services/settings.service-model'; -import coreSettingsInfo from '@main/data/core-settings-info.data'; +import { coreSettingsInfo, coreSettingsValidators } from '@main/data/core-settings-info.data'; import { SettingNames, SettingTypes } from 'papi-shared-types'; -import { createSyncProxyForAsyncObject, deserialize, serialize } from 'platform-bible-utils'; +import { + createSyncProxyForAsyncObject, + deserialize, + includes, + serialize, +} from 'platform-bible-utils'; import { joinUriPaths } from '@node/utils/util'; import * as nodeFS from '@node/services/node-file-system.service'; +import { serializeRequestType } from '@shared/utils/util'; const SETTINGS_FILE_URI = joinUriPaths('data://', 'settings.json'); @@ -48,6 +56,33 @@ function getDefaultValueForKey( return settingInfo.default; } +async function validateSetting( + key: SettingName, + newValue: SettingTypes[SettingName], + currentValue: SettingTypes[SettingName], + allChanges?: Partial, +): Promise { + if (key in coreSettingsValidators) { + const settingValidator = coreSettingsValidators[key]; + if (settingValidator) return settingValidator(newValue, currentValue, allChanges ?? {}); + // If there is no validator just let the change go through + return true; + } + try { + return networkService.request( + serializeRequestType(CATEGORY_EXTENSION_SETTING_VALIDATOR, key), + newValue, + currentValue, + allChanges ?? {}, + ); + } catch (error) { + // If there is no validator just let the change go through + const missingValidatorMsg = `No handler was found to process the request of type`; + if (includes(`${error}`, missingValidatorMsg)) return true; + throw error; + } +} + class SettingDataProviderEngine extends DataProviderEngine< SettingDataTypes & { @@ -86,6 +121,9 @@ class SettingDataProviderEngine newSetting: SettingTypes[SettingName], ): Promise> { try { + if (!(await validateSetting(key, newSetting, await this.get(key)))) + throw new Error('validation failed'); + this.settingsData[key] = newSetting; await writeSettingsDataToFile(this.settingsData); } catch (error) { diff --git a/src/shared/services/settings.service-model.ts b/src/shared/services/settings.service-model.ts index 0561264a89..279e5a59ad 100644 --- a/src/shared/services/settings.service-model.ts +++ b/src/shared/services/settings.service-model.ts @@ -1,11 +1,16 @@ +import * as networkService from '@shared/services/network.service'; import { SettingNames, SettingTypes } from 'papi-shared-types'; import { OnDidDispose, UnsubscriberAsync } from 'platform-bible-utils'; +import { serializeRequestType } from '@shared/utils/util'; import { DataProviderSubscriberOptions, DataProviderUpdateInstructions, IDataProvider, } from './papi-core.service'; +/** Name prefix for registered commands that call settings validators */ +export const CATEGORY_EXTENSION_SETTING_VALIDATOR = 'extensionSettingValidator'; + /** JSDOC DESTINATION settingsServiceDataProviderName */ export const settingsServiceDataProviderName = 'platform.settingsServiceDataProvider'; export const settingsServiceObjectToProxy = Object.freeze({ @@ -16,6 +21,25 @@ export const settingsServiceObjectToProxy = Object.freeze({ * name to find the data provider when accessing it using the useData hook */ dataProviderName: settingsServiceDataProviderName, + + /** + * JSDOC SOURCE settingsServiceRegisterValidator + * + * Registers a function that validates whether a new setting value is allowed to be set. + * + * @param key The string id of the setting to validate + * @param validator Function to call to validate the new setting value + * @returns Unsubscriber that should be called whenever the providing extension is deactivated + */ + registerValidator: async ( + key: SettingName, + validator: SettingValidator, + ): Promise => { + return networkService.registerRequestHandler( + serializeRequestType(CATEGORY_EXTENSION_SETTING_VALIDATOR, key), + validator, + ); + }, }); /** * SettingDataTypes handles getting and setting Platform.Bible core application and extension @@ -40,6 +64,18 @@ export type AllSettingsData = { [SettingName in SettingNames]: SettingTypes[SettingName]; }; +/** Function that validates whether a new setting value should be allowed to be set */ +export type SettingValidator = ( + newValue: SettingTypes[SettingName], + currentValue: SettingTypes[SettingName], + allChanges: Partial, +) => Promise; + +/** Validators for all settings. Keys are setting keys, values are functions to validate new settings */ +export type AllSettingsValidators = { + [SettingName in SettingNames]: SettingValidator; +}; + declare module 'papi-shared-types' { export interface DataProviders { [settingsServiceDataProviderName]: ISettingsService; @@ -94,6 +130,12 @@ export type ISettingsService = { callback: (newSetting: SettingTypes[SettingName]) => void, options?: DataProviderSubscriberOptions, ): Promise; + + /** JSDOC DESTINATION settingsServiceRegisterValidator */ + registerValidator( + key: SettingName, + validator: SettingValidator, + ): Promise; } & OnDidDispose & IDataProvider & typeof settingsServiceObjectToProxy;