From 5d3e269ec9e00c1ebbaf4d25dd9f837b9fada711 Mon Sep 17 00:00:00 2001 From: Jolie Rabideau Date: Mon, 25 Mar 2024 10:03:07 -0400 Subject: [PATCH 1/6] implement isValid, add project setting validator, fix test, not working --- lib/papi-dts/papi.d.ts | 43 +++++++++++++++++-- src/declarations/papi-shared-types.ts | 4 +- .../data/core-project-settings-info.data.ts | 30 ++++++++++++- .../project-settings.service-host.test.ts | 10 ++--- .../services/project-settings.service-host.ts | 41 ++++++++++++++++-- .../project-settings.service-model.ts | 36 ++++++++++++++-- .../services/project-settings.service.ts | 4 ++ 7 files changed, 151 insertions(+), 17 deletions(-) diff --git a/lib/papi-dts/papi.d.ts b/lib/papi-dts/papi.d.ts index cf55164bdb..4e9ab43087 100644 --- a/lib/papi-dts/papi.d.ts +++ b/lib/papi-dts/papi.d.ts @@ -2467,6 +2467,16 @@ declare module 'papi-shared-types' { * @example 'World English Bible' */ 'platform.fullName': string; + /** + * Which books are present in this Scripture project. Represented as a string with 0 or 1 for + * each possible book by [standardized book + * code](https://github.com/sillsdev/libpalaso/blob/master/SIL.Scripture/Canon.cs#L226) (123 + * characters long) + * + * @example + * '100111000000000000110000001000000000010111111111111111111111111111000000000000000000000000000000000000000000100000000000000' + */ + 'platformScripture.booksPresent': string; } /** * Names for each user setting available on the papi. @@ -4709,6 +4719,10 @@ declare module 'renderer/hooks/papi-hooks/use-dialog-callback.hook' { } declare module 'shared/services/project-settings.service-model' { import { ProjectSettingNames, ProjectSettingTypes, ProjectTypes } from 'papi-shared-types'; + import { UnsubscriberAsync } from 'platform-bible-utils'; + /** Name prefix for registered commands that call project settings validators */ + export const CATEGORY_EXTENSION_PROJECT_SETTING_VALIDATOR = 'extensionProjectSettingValidator'; + export const projectSettingsServiceNetworkObjectName = 'ProjectSettingsService'; /** * * Provides utility functions that project storage interpreters should call when handling project @@ -4730,11 +4744,11 @@ declare module 'shared/services/project-settings.service-model' { * @returns `true` if change is valid, `false` otherwise */ isValid( + key: ProjectSettingName, newValue: ProjectSettingTypes[ProjectSettingName], currentValue: ProjectSettingTypes[ProjectSettingName], - key: ProjectSettingName, - allChanges: SimultaneousProjectSettingsChanges, projectType: ProjectTypes, + allChanges?: SimultaneousProjectSettingsChanges, ): Promise; /** * Gets default value for a project setting @@ -4753,6 +4767,17 @@ declare module 'shared/services/project-settings.service-model' { key: ProjectSettingName, projectType: ProjectTypes, ): 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: ProjectSettingName, + validator: ProjectSettingValidator, + ): Promise; } /** * All project settings changes being set in one batch @@ -4768,7 +4793,19 @@ declare module 'shared/services/project-settings.service-model' { currentValue: ProjectSettingTypes[ProjectSettingName]; }; }; - export const projectSettingsServiceNetworkObjectName = 'ProjectSettingsService'; + /** Function that validates whether a new project setting value should be allowed to be set */ + export type ProjectSettingValidator = ( + newValue: ProjectSettingTypes[ProjectSettingName], + currentValue: ProjectSettingTypes[ProjectSettingName], + allChanges: SimultaneousProjectSettingsChanges, + ) => Promise; + /** + * Validators for all project settings. Keys are setting keys, values are functions to validate new + * settings + */ + export type AllProjectSettingsValidators = { + [ProjectSettingName in ProjectSettingNames]: ProjectSettingValidator; + }; } declare module '@papi/core' { /** Exporting empty object so people don't have to put 'type' in their import statements */ diff --git a/src/declarations/papi-shared-types.ts b/src/declarations/papi-shared-types.ts index c475b5b1a5..a4b0cf8421 100644 --- a/src/declarations/papi-shared-types.ts +++ b/src/declarations/papi-shared-types.ts @@ -172,7 +172,9 @@ declare module 'papi-shared-types' { TProjectDataTypes extends DataProviderDataTypes, > = { /** - * Set the value of the specified project setting on this project. + * Set the value of the specified project setting on this project. `setSetting` must call + * `papi.projectSettings.isValid(newValue, currentValue, key, allChanges, projectType)` before + * allowing the setting change. * * @param key The string id of the project setting to change * @param newSetting The value that is to be set to the project setting. diff --git a/src/main/data/core-project-settings-info.data.ts b/src/main/data/core-project-settings-info.data.ts index 94843ebb45..5b065038eb 100644 --- a/src/main/data/core-project-settings-info.data.ts +++ b/src/main/data/core-project-settings-info.data.ts @@ -1,4 +1,9 @@ +import { + AllProjectSettingsValidators, + ProjectSettingValidator, +} from '@shared/services/project-settings.service-model'; import { ProjectSettingNames, ProjectSettingTypes } from 'papi-shared-types'; +import { stringLength } from 'platform-bible-utils'; /** Information about one project setting */ type ProjectSettingInfo = { @@ -14,7 +19,7 @@ export type AllProjectSettingsInfo = { }; /** Info about all project settings built into core. Does not contain info for extensions' settings */ -const coreProjectSettingsInfo: Partial = { +export const coreProjectSettingsInfo: Partial = { 'platform.fullName': { default: '%project_full_name_missing%' }, 'platform.language': { default: '%project_language_missing%' }, 'platformScripture.booksPresent': { @@ -27,4 +32,27 @@ const coreProjectSettingsInfo: Partial = { 'platformScripture.versification': { default: 4 }, }; +// Based on https://github.com/paranext/paranext-core/blob/5c403e272b002ddd8970f735bc119f335c78c509/extensions/src/usfm-data-provider/index.d.ts#L401 +// Should be 123 characters long +// todo Check that it only includes 1 or 0 +export const booksPresentSettingsValidator: ProjectSettingValidator< + 'platformScripture.booksPresent' +> = async (newValue: string): Promise => { + return stringLength(newValue) === 123; +}; + +// Based on https://github.com/paranext/paranext-core/blob/5c403e272b002ddd8970f735bc119f335c78c509/extensions/src/usfm-data-provider/index.d.ts#L391 +// There are 7 options in the enum +export const versificationSettingsValidator: ProjectSettingValidator< + 'platformScripture.versification' +> = async (newValue: number): Promise => { + return newValue >= 0 && newValue <= 6; +}; + +/** Info about all settings built into core. Does not contain info for extensions' settings */ +export const coreProjectSettingsValidators: Partial = { + 'platformScripture.booksPresent': booksPresentSettingsValidator, + 'platformScripture.versification': versificationSettingsValidator, +}; + export default coreProjectSettingsInfo; diff --git a/src/main/services/project-settings.service-host.test.ts b/src/main/services/project-settings.service-host.test.ts index 92fdbbf481..2fb017913b 100644 --- a/src/main/services/project-settings.service-host.test.ts +++ b/src/main/services/project-settings.service-host.test.ts @@ -13,12 +13,12 @@ jest.mock('@main/data/core-project-settings-info.data', () => ({ })); describe('isValid', () => { - it('should return true always - TEMP. TODO: Fix when we implement validation #511', async () => { + it('should return true if new value has 123 characters', async () => { + const projectSettingKey = 'platformScripture.booksPresent'; const isSettingChangeValid = await testingProjectSettingsService.isValid( - '', - '', - 'platform.fullName', - {}, + projectSettingKey, + '000000011110000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000', + '000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000', 'ParatextStandard', ); expect(isSettingChangeValid).toBe(true); diff --git a/src/main/services/project-settings.service-host.ts b/src/main/services/project-settings.service-host.ts index 2a8348492f..aee97b5907 100644 --- a/src/main/services/project-settings.service-host.ts +++ b/src/main/services/project-settings.service-host.ts @@ -1,12 +1,20 @@ -import coreProjectSettingsInfo, { +import * as networkService from '@shared/services/network.service'; +import { AllProjectSettingsInfo, + coreProjectSettingsValidators, + coreProjectSettingsInfo, } from '@main/data/core-project-settings-info.data'; import networkObjectService from '@shared/services/network-object.service'; import { + CATEGORY_EXTENSION_PROJECT_SETTING_VALIDATOR, IProjectSettingsService, + ProjectSettingValidator, + SimultaneousProjectSettingsChanges, projectSettingsServiceNetworkObjectName, } from '@shared/services/project-settings.service-model'; +import { serializeRequestType } from '@shared/utils/util'; import { ProjectSettingNames, ProjectSettingTypes, ProjectTypes } from 'papi-shared-types'; +import { UnsubscriberAsync } from 'platform-bible-utils'; /** * Our internal list of project settings information for each setting. Theoretically this should not @@ -35,9 +43,23 @@ async function initialize(): Promise { return initializationPromise; } -// TODO: Implement validators in https://github.com/paranext/paranext-core/issues/511 -async function isValid(): Promise { - return true; +async function isValid( + key: ProjectSettingName, + newValue: ProjectSettingTypes[ProjectSettingName], + currentValue: ProjectSettingTypes[ProjectSettingName], + projectType: ProjectTypes, + allChanges?: SimultaneousProjectSettingsChanges, +): Promise { + try { + if (key in coreProjectSettingsValidators) { + const projectSettingValidator = coreProjectSettingsValidators[key]; + if (projectSettingValidator) + return projectSettingValidator(newValue, currentValue, allChanges ?? {}, projectType); + } + return true; + } catch (error) { + throw new Error(`Error validating setting for key '${key}': ${error}`); + } } async function getDefault( @@ -55,9 +77,20 @@ async function getDefault( return projectSettingInfo.default; } +async function registerValidator( + key: ProjectSettingName, + validatorCallback: ProjectSettingValidator, +): Promise { + return networkService.registerRequestHandler( + serializeRequestType(CATEGORY_EXTENSION_PROJECT_SETTING_VALIDATOR, key), + validatorCallback, + ); +} + const projectSettingsService: IProjectSettingsService = { isValid, getDefault, + registerValidator, }; /** This is an internal-only export for testing purposes, and should not be used in development */ diff --git a/src/shared/services/project-settings.service-model.ts b/src/shared/services/project-settings.service-model.ts index 14ee71c864..8d547561bf 100644 --- a/src/shared/services/project-settings.service-model.ts +++ b/src/shared/services/project-settings.service-model.ts @@ -1,4 +1,10 @@ import { ProjectSettingNames, ProjectSettingTypes, ProjectTypes } from 'papi-shared-types'; +import { UnsubscriberAsync } from 'platform-bible-utils'; + +/** Name prefix for registered commands that call project settings validators */ +export const CATEGORY_EXTENSION_PROJECT_SETTING_VALIDATOR = 'extensionProjectSettingValidator'; + +export const projectSettingsServiceNetworkObjectName = 'ProjectSettingsService'; /** * JSDOC SOURCE projectSettingsService @@ -22,11 +28,11 @@ export interface IProjectSettingsService { * @returns `true` if change is valid, `false` otherwise */ isValid( + key: ProjectSettingName, newValue: ProjectSettingTypes[ProjectSettingName], currentValue: ProjectSettingTypes[ProjectSettingName], - key: ProjectSettingName, - allChanges: SimultaneousProjectSettingsChanges, projectType: ProjectTypes, + allChanges?: SimultaneousProjectSettingsChanges, ): Promise; /** * Gets default value for a project setting @@ -45,6 +51,17 @@ export interface IProjectSettingsService { key: ProjectSettingName, projectType: ProjectTypes, ): 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: ProjectSettingName, + validatorCallback: ProjectSettingValidator, + ): Promise; } /** @@ -61,5 +78,18 @@ export type SimultaneousProjectSettingsChanges = { currentValue: ProjectSettingTypes[ProjectSettingName]; }; }; +/** Function that validates whether a new project setting value should be allowed to be set */ +export type ProjectSettingValidator = ( + newValue: ProjectSettingTypes[ProjectSettingName], + currentValue: ProjectSettingTypes[ProjectSettingName], + allChanges: SimultaneousProjectSettingsChanges, + projectType: ProjectTypes, +) => Promise; -export const projectSettingsServiceNetworkObjectName = 'ProjectSettingsService'; +/** + * Validators for all project settings. Keys are setting keys, values are functions to validate new + * settings + */ +export type AllProjectSettingsValidators = { + [ProjectSettingName in ProjectSettingNames]: ProjectSettingValidator; +}; diff --git a/src/shared/services/project-settings.service.ts b/src/shared/services/project-settings.service.ts index 5040d78a06..e0185f99b1 100644 --- a/src/shared/services/project-settings.service.ts +++ b/src/shared/services/project-settings.service.ts @@ -40,6 +40,10 @@ const projectSettingsService: IProjectSettingsService = { await initialize(); return networkObject.isValid(newValue, currentValue, key, allChanges, projectType); }, + async registerValidator(key, validator) { + await initialize(); + return networkObject.registerValidator(key, validator); + }, }; export default projectSettingsService; From 424c6a890c5794bd8bca8ec66d34734ea64274a2 Mon Sep 17 00:00:00 2001 From: Jolie Rabideau Date: Mon, 25 Mar 2024 15:33:38 -0400 Subject: [PATCH 2/6] add mock for validators in test --- lib/papi-dts/papi.d.ts | 17 +++++------------ .../data/core-project-settings-info.data.ts | 2 -- .../project-settings.service-host.test.ts | 10 ++++++++-- .../services/project-settings.service-host.ts | 1 + 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/lib/papi-dts/papi.d.ts b/lib/papi-dts/papi.d.ts index 4e9ab43087..617dde636b 100644 --- a/lib/papi-dts/papi.d.ts +++ b/lib/papi-dts/papi.d.ts @@ -2467,16 +2467,6 @@ declare module 'papi-shared-types' { * @example 'World English Bible' */ 'platform.fullName': string; - /** - * Which books are present in this Scripture project. Represented as a string with 0 or 1 for - * each possible book by [standardized book - * code](https://github.com/sillsdev/libpalaso/blob/master/SIL.Scripture/Canon.cs#L226) (123 - * characters long) - * - * @example - * '100111000000000000110000001000000000010111111111111111111111111111000000000000000000000000000000000000000000100000000000000' - */ - 'platformScripture.booksPresent': string; } /** * Names for each user setting available on the papi. @@ -2495,7 +2485,9 @@ declare module 'papi-shared-types' { TProjectDataTypes extends DataProviderDataTypes, > = { /** - * Set the value of the specified project setting on this project. + * Set the value of the specified project setting on this project. `setSetting` must call + * `papi.projectSettings.isValid(newValue, currentValue, key, allChanges, projectType)` before + * allowing the setting change. * * @param key The string id of the project setting to change * @param newSetting The value that is to be set to the project setting. @@ -4776,7 +4768,7 @@ declare module 'shared/services/project-settings.service-model' { */ registerValidator( key: ProjectSettingName, - validator: ProjectSettingValidator, + validatorCallback: ProjectSettingValidator, ): Promise; } /** @@ -4798,6 +4790,7 @@ declare module 'shared/services/project-settings.service-model' { newValue: ProjectSettingTypes[ProjectSettingName], currentValue: ProjectSettingTypes[ProjectSettingName], allChanges: SimultaneousProjectSettingsChanges, + projectType: ProjectTypes, ) => Promise; /** * Validators for all project settings. Keys are setting keys, values are functions to validate new diff --git a/src/main/data/core-project-settings-info.data.ts b/src/main/data/core-project-settings-info.data.ts index 5b065038eb..bee8462344 100644 --- a/src/main/data/core-project-settings-info.data.ts +++ b/src/main/data/core-project-settings-info.data.ts @@ -54,5 +54,3 @@ export const coreProjectSettingsValidators: Partial ({ __esModule: true, - default: { + coreProjectSettingsInfo: { 'platform.fullName': { default: '%test_project_full_name_missing%' }, 'platform.language': { default: '%test_project_language_missing%' }, 'platformScripture.booksPresent': { @@ -10,10 +10,16 @@ jest.mock('@main/data/core-project-settings-info.data', () => ({ }, // Not present! Should throw error 'platformScripture.versification': { default: 1629326 }, }, + coreProjectSettingsValidators: { + 'platformScripture.booksPresent': async (): Promise => { + // Accept all attempts to set the books present + return true; + }, + }, })); describe('isValid', () => { - it('should return true if new value has 123 characters', async () => { + it('should return true', async () => { const projectSettingKey = 'platformScripture.booksPresent'; const isSettingChangeValid = await testingProjectSettingsService.isValid( projectSettingKey, diff --git a/src/main/services/project-settings.service-host.ts b/src/main/services/project-settings.service-host.ts index aee97b5907..1ebe37c324 100644 --- a/src/main/services/project-settings.service-host.ts +++ b/src/main/services/project-settings.service-host.ts @@ -56,6 +56,7 @@ async function isValid( if (projectSettingValidator) return projectSettingValidator(newValue, currentValue, allChanges ?? {}, projectType); } + // If there is no validator let the change go through return true; } catch (error) { throw new Error(`Error validating setting for key '${key}': ${error}`); From 9ba9f351d74e9951c65e572002c64115dcb50ba5 Mon Sep 17 00:00:00 2001 From: Jolie Rabideau Date: Tue, 26 Mar 2024 17:26:15 -0400 Subject: [PATCH 3/6] revert some unnecessary changes --- lib/papi-dts/papi.d.ts | 3 +-- src/declarations/papi-shared-types.ts | 3 +-- src/main/data/core-project-settings-info.data.ts | 4 +++- src/main/services/project-settings.service-host.ts | 3 +-- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/papi-dts/papi.d.ts b/lib/papi-dts/papi.d.ts index 9974826036..8ac582d69b 100644 --- a/lib/papi-dts/papi.d.ts +++ b/lib/papi-dts/papi.d.ts @@ -2486,8 +2486,7 @@ declare module 'papi-shared-types' { > = { /** * Set the value of the specified project setting on this project. `setSetting` must call - * `papi.projectSettings.isValid(newValue, currentValue, key, allChanges, projectType)` before - * allowing the setting change. + * `papi.projectSettings.isValid` before allowing the setting change. * * @param key The string id of the project setting to change * @param newSetting The value that is to be set to the project setting. diff --git a/src/declarations/papi-shared-types.ts b/src/declarations/papi-shared-types.ts index a4b0cf8421..fad7b5e050 100644 --- a/src/declarations/papi-shared-types.ts +++ b/src/declarations/papi-shared-types.ts @@ -173,8 +173,7 @@ declare module 'papi-shared-types' { > = { /** * Set the value of the specified project setting on this project. `setSetting` must call - * `papi.projectSettings.isValid(newValue, currentValue, key, allChanges, projectType)` before - * allowing the setting change. + * `papi.projectSettings.isValid` before allowing the setting change. * * @param key The string id of the project setting to change * @param newSetting The value that is to be set to the project setting. diff --git a/src/main/data/core-project-settings-info.data.ts b/src/main/data/core-project-settings-info.data.ts index bee8462344..f2fac0e858 100644 --- a/src/main/data/core-project-settings-info.data.ts +++ b/src/main/data/core-project-settings-info.data.ts @@ -19,7 +19,7 @@ export type AllProjectSettingsInfo = { }; /** Info about all project settings built into core. Does not contain info for extensions' settings */ -export const coreProjectSettingsInfo: Partial = { +const coreProjectSettingsInfo: Partial = { 'platform.fullName': { default: '%project_full_name_missing%' }, 'platform.language': { default: '%project_language_missing%' }, 'platformScripture.booksPresent': { @@ -54,3 +54,5 @@ export const coreProjectSettingsValidators: Partial Date: Wed, 27 Mar 2024 10:16:45 -0400 Subject: [PATCH 4/6] update tests --- .../project-settings.service-host.test.ts | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/main/services/project-settings.service-host.test.ts b/src/main/services/project-settings.service-host.test.ts index ca49b8e4ed..a2824952f4 100644 --- a/src/main/services/project-settings.service-host.test.ts +++ b/src/main/services/project-settings.service-host.test.ts @@ -1,8 +1,16 @@ import { testingProjectSettingsService } from '@main/services/project-settings.service-host'; +import { ProjectSettingValidator } from '@shared/services/project-settings.service-model'; +jest.mock('@shared/services/network.service', () => ({ + ...jest.requireActual('@shared/services/network.service'), + registerRequestHandler: () => { + return {}; + }, +})); jest.mock('@main/data/core-project-settings-info.data', () => ({ + ...jest.requireActual('@main/data/core-project-settings-info.data'), __esModule: true, - coreProjectSettingsInfo: { + default: { 'platform.fullName': { default: '%test_project_full_name_missing%' }, 'platform.language': { default: '%test_project_language_missing%' }, 'platformScripture.booksPresent': { @@ -47,4 +55,21 @@ describe('getDefault', () => { testingProjectSettingsService.getDefault(projectSettingKey, 'ParatextStandard'), ).rejects.toThrow(new RegExp(`default value for project setting ${projectSettingKey}`)); }); + + describe('registerValidator', () => { + it('should resolve', async () => { + const projectSettingKey = 'platform.fullName'; + const fullNameSettingsValidator: ProjectSettingValidator< + 'platform.fullName' + > = async (): Promise => { + return true; + }; + await expect( + testingProjectSettingsService.registerValidator( + projectSettingKey, + fullNameSettingsValidator, + ), + ).resolves.toStrictEqual({}); + }); + }); }); From d8d962c578439803a354704c6c59ed8ea772d5ab Mon Sep 17 00:00:00 2001 From: Jolie Rabideau Date: Wed, 27 Mar 2024 10:17:58 -0400 Subject: [PATCH 5/6] fix test nesting --- .../project-settings.service-host.test.ts | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/main/services/project-settings.service-host.test.ts b/src/main/services/project-settings.service-host.test.ts index a2824952f4..efe996e92e 100644 --- a/src/main/services/project-settings.service-host.test.ts +++ b/src/main/services/project-settings.service-host.test.ts @@ -55,21 +55,18 @@ describe('getDefault', () => { testingProjectSettingsService.getDefault(projectSettingKey, 'ParatextStandard'), ).rejects.toThrow(new RegExp(`default value for project setting ${projectSettingKey}`)); }); +}); - describe('registerValidator', () => { - it('should resolve', async () => { - const projectSettingKey = 'platform.fullName'; - const fullNameSettingsValidator: ProjectSettingValidator< - 'platform.fullName' - > = async (): Promise => { - return true; - }; - await expect( - testingProjectSettingsService.registerValidator( - projectSettingKey, - fullNameSettingsValidator, - ), - ).resolves.toStrictEqual({}); - }); +describe('registerValidator', () => { + it('should resolve', async () => { + const projectSettingKey = 'platform.fullName'; + const fullNameSettingsValidator: ProjectSettingValidator< + 'platform.fullName' + > = async (): Promise => { + return true; + }; + await expect( + testingProjectSettingsService.registerValidator(projectSettingKey, fullNameSettingsValidator), + ).resolves.toStrictEqual({}); }); }); From 98fd2060d9d660d02b5939d67a320f051238d874 Mon Sep 17 00:00:00 2001 From: Jolie Rabideau Date: Tue, 2 Apr 2024 20:10:06 -0400 Subject: [PATCH 6/6] Fix comments, object to proxy --- lib/papi-dts/papi.d.ts | 24 +++++++++-- src/declarations/papi-shared-types.ts | 7 +++- .../project-settings.service-host.test.ts | 21 +++++++--- .../services/project-settings.service-host.ts | 41 ++++++++++--------- .../project-settings.service-model.ts | 18 +++++++- .../services/project-settings.service.ts | 20 +++------ 6 files changed, 85 insertions(+), 46 deletions(-) diff --git a/lib/papi-dts/papi.d.ts b/lib/papi-dts/papi.d.ts index 8ac582d69b..e32f449b35 100644 --- a/lib/papi-dts/papi.d.ts +++ b/lib/papi-dts/papi.d.ts @@ -2485,13 +2485,16 @@ declare module 'papi-shared-types' { TProjectDataTypes extends DataProviderDataTypes, > = { /** - * Set the value of the specified project setting on this project. `setSetting` must call - * `papi.projectSettings.isValid` before allowing the setting change. + * Set the value of the specified project setting on this project. + * + * Note: `setSetting` must call `papi.projectSettings.isValid` before allowing the setting + * change. * * @param key The string id of the project setting to change * @param newSetting The value that is to be set to the project setting. * @returns Information that papi uses to interpret whether to send out updates. Defaults to * `true` (meaning send updates only for this data type). + * @throws If the setting validator failed. * @see {@link DataProviderUpdateInstructions} for more info on what to return */ setSetting: ( @@ -4714,6 +4717,20 @@ declare module 'shared/services/project-settings.service-model' { /** Name prefix for registered commands that call project settings validators */ export const CATEGORY_EXTENSION_PROJECT_SETTING_VALIDATOR = 'extensionProjectSettingValidator'; export const projectSettingsServiceNetworkObjectName = 'ProjectSettingsService'; + export const projectSettingsServiceObjectToProxy: Readonly<{ + /** + * + * Registers a function that validates whether a new project 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: ProjectSettingName, + validator: ProjectSettingValidator, + ) => Promise; + }>; /** * * Provides utility functions that project storage interpreters should call when handling project @@ -4759,7 +4776,8 @@ declare module 'shared/services/project-settings.service-model' { projectType: ProjectTypes, ): Promise; /** - * Registers a function that validates whether a new setting value is allowed to be set. + * + * Registers a function that validates whether a new project 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 diff --git a/src/declarations/papi-shared-types.ts b/src/declarations/papi-shared-types.ts index fad7b5e050..c50841fb8d 100644 --- a/src/declarations/papi-shared-types.ts +++ b/src/declarations/papi-shared-types.ts @@ -172,13 +172,16 @@ declare module 'papi-shared-types' { TProjectDataTypes extends DataProviderDataTypes, > = { /** - * Set the value of the specified project setting on this project. `setSetting` must call - * `papi.projectSettings.isValid` before allowing the setting change. + * Set the value of the specified project setting on this project. + * + * Note: `setSetting` must call `papi.projectSettings.isValid` before allowing the setting + * change. * * @param key The string id of the project setting to change * @param newSetting The value that is to be set to the project setting. * @returns Information that papi uses to interpret whether to send out updates. Defaults to * `true` (meaning send updates only for this data type). + * @throws If the setting validator failed. * @see {@link DataProviderUpdateInstructions} for more info on what to return */ setSetting: ( diff --git a/src/main/services/project-settings.service-host.test.ts b/src/main/services/project-settings.service-host.test.ts index efe996e92e..307a36ceb3 100644 --- a/src/main/services/project-settings.service-host.test.ts +++ b/src/main/services/project-settings.service-host.test.ts @@ -19,24 +19,33 @@ jest.mock('@main/data/core-project-settings-info.data', () => ({ // Not present! Should throw error 'platformScripture.versification': { default: 1629326 }, }, coreProjectSettingsValidators: { - 'platformScripture.booksPresent': async (): Promise => { - // Accept all attempts to set the books present - return true; + 'platform.language': async (newValue: string): Promise => { + return newValue === 'eng' || newValue === 'fre'; }, }, })); describe('isValid', () => { it('should return true', async () => { - const projectSettingKey = 'platformScripture.booksPresent'; + const projectSettingKey = 'platform.language'; const isSettingChangeValid = await testingProjectSettingsService.isValid( projectSettingKey, - '000000011110000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000', - '000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000', + 'eng', + '%test_project_language_missing%', 'ParatextStandard', ); expect(isSettingChangeValid).toBe(true); }); + it('should return false', async () => { + const projectSettingKey = 'platform.language'; + const isSettingChangeValid = await testingProjectSettingsService.isValid( + projectSettingKey, + 'ger', + '%test_project_language_missing%', + 'ParatextStandard', + ); + expect(isSettingChangeValid).toBe(false); + }); }); describe('getDefault', () => { diff --git a/src/main/services/project-settings.service-host.ts b/src/main/services/project-settings.service-host.ts index ba62203cdd..bd7154ec23 100644 --- a/src/main/services/project-settings.service-host.ts +++ b/src/main/services/project-settings.service-host.ts @@ -7,13 +7,13 @@ import networkObjectService from '@shared/services/network-object.service'; import { CATEGORY_EXTENSION_PROJECT_SETTING_VALIDATOR, IProjectSettingsService, - ProjectSettingValidator, SimultaneousProjectSettingsChanges, projectSettingsServiceNetworkObjectName, + projectSettingsServiceObjectToProxy, } from '@shared/services/project-settings.service-model'; import { serializeRequestType } from '@shared/utils/util'; import { ProjectSettingNames, ProjectSettingTypes, ProjectTypes } from 'papi-shared-types'; -import { UnsubscriberAsync } from 'platform-bible-utils'; +import { includes } from 'platform-bible-utils'; /** * Our internal list of project settings information for each setting. Theoretically this should not @@ -49,16 +49,26 @@ async function isValid( projectType: ProjectTypes, allChanges?: SimultaneousProjectSettingsChanges, ): Promise { - try { - if (key in coreProjectSettingsValidators) { - const projectSettingValidator = coreProjectSettingsValidators[key]; - if (projectSettingValidator) - return projectSettingValidator(newValue, currentValue, allChanges ?? {}, projectType); - } - // If there is no validator let the change go through + if (key in coreProjectSettingsValidators) { + const projectSettingValidator = coreProjectSettingsValidators[key]; + if (projectSettingValidator) + return projectSettingValidator(newValue, currentValue, allChanges ?? {}, projectType); + // If key exists in coreProjectSettingsValidators but there is no validator, let the change go through return true; + } + try { + return networkService.request( + serializeRequestType(CATEGORY_EXTENSION_PROJECT_SETTING_VALIDATOR, key), + newValue, + currentValue, + allChanges ?? {}, + projectType, + ); } catch (error) { - throw new Error(`Error validating setting for key '${key}': ${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; } } @@ -77,16 +87,7 @@ async function getDefault( return projectSettingInfo.default; } -async function registerValidator( - key: ProjectSettingName, - validatorCallback: ProjectSettingValidator, -): Promise { - return networkService.registerRequestHandler( - serializeRequestType(CATEGORY_EXTENSION_PROJECT_SETTING_VALIDATOR, key), - validatorCallback, - ); -} - +const { registerValidator } = projectSettingsServiceObjectToProxy; const projectSettingsService: IProjectSettingsService = { isValid, getDefault, diff --git a/src/shared/services/project-settings.service-model.ts b/src/shared/services/project-settings.service-model.ts index 8d547561bf..e0eef065db 100644 --- a/src/shared/services/project-settings.service-model.ts +++ b/src/shared/services/project-settings.service-model.ts @@ -1,3 +1,5 @@ +import { serializeRequestType } from '@shared/utils/util'; +import * as networkService from '@shared/services/network.service'; import { ProjectSettingNames, ProjectSettingTypes, ProjectTypes } from 'papi-shared-types'; import { UnsubscriberAsync } from 'platform-bible-utils'; @@ -5,6 +7,18 @@ import { UnsubscriberAsync } from 'platform-bible-utils'; export const CATEGORY_EXTENSION_PROJECT_SETTING_VALIDATOR = 'extensionProjectSettingValidator'; export const projectSettingsServiceNetworkObjectName = 'ProjectSettingsService'; +export const projectSettingsServiceObjectToProxy = Object.freeze({ + /** JSDOC DESTINATION projectSettingsServiceRegisterValidator */ + registerValidator: async ( + key: ProjectSettingName, + validator: ProjectSettingValidator, + ): Promise => { + return networkService.registerRequestHandler( + serializeRequestType(CATEGORY_EXTENSION_PROJECT_SETTING_VALIDATOR, key), + validator, + ); + }, +}); /** * JSDOC SOURCE projectSettingsService @@ -52,7 +66,9 @@ export interface IProjectSettingsService { projectType: ProjectTypes, ): Promise; /** - * Registers a function that validates whether a new setting value is allowed to be set. + * JSDOC SOURCE projectSettingsServiceRegisterValidator + * + * Registers a function that validates whether a new project 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 diff --git a/src/shared/services/project-settings.service.ts b/src/shared/services/project-settings.service.ts index e0185f99b1..b9fec0d3f9 100644 --- a/src/shared/services/project-settings.service.ts +++ b/src/shared/services/project-settings.service.ts @@ -1,8 +1,10 @@ import { projectSettingsServiceNetworkObjectName, IProjectSettingsService, + projectSettingsServiceObjectToProxy, } from '@shared/services/project-settings.service-model'; import networkObjectService from '@shared/services/network-object.service'; +import { createSyncProxyForAsyncObject } from 'platform-bible-utils'; let networkObject: IProjectSettingsService; let initializationPromise: Promise; @@ -31,19 +33,9 @@ async function initialize(): Promise { return initializationPromise; } -const projectSettingsService: IProjectSettingsService = { - async getDefault(key, projectType) { - await initialize(); - return networkObject.getDefault(key, projectType); - }, - async isValid(newValue, currentValue, key, allChanges, projectType) { - await initialize(); - return networkObject.isValid(newValue, currentValue, key, allChanges, projectType); - }, - async registerValidator(key, validator) { - await initialize(); - return networkObject.registerValidator(key, validator); - }, -}; +const projectSettingsService = createSyncProxyForAsyncObject(async () => { + await initialize(); + return networkObject; +}, projectSettingsServiceObjectToProxy); export default projectSettingsService;