Skip to content

Commit

Permalink
Add settings validation functions (#798)
Browse files Browse the repository at this point in the history
  • Loading branch information
lyonsil authored Mar 21, 2024
1 parent e0c3e96 commit c7bd644
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 6 deletions.
36 changes: 36 additions & 0 deletions lib/papi-dts/papi.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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: <SettingName extends keyof SettingTypes>(
key: SettingName,
validator: SettingValidator<SettingName>,
) => Promise<UnsubscriberAsync>;
}>;
/**
* SettingDataTypes handles getting and setting Platform.Bible core application and extension
Expand All @@ -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<SettingName extends SettingNames> = (
newValue: SettingTypes[SettingName],
currentValue: SettingTypes[SettingName],
allChanges: Partial<SettingTypes>,
) => Promise<boolean>;
/** Validators for all settings. Keys are setting keys, values are functions to validate new settings */
export type AllSettingsValidators = {
[SettingName in SettingNames]: SettingValidator<SettingName>;
};
module 'papi-shared-types' {
interface DataProviders {
[settingsServiceDataProviderName]: ISettingsService;
Expand Down Expand Up @@ -5026,6 +5050,18 @@ declare module 'shared/services/settings.service-model' {
callback: (newSetting: SettingTypes[SettingName]) => void,
options?: DataProviderSubscriberOptions,
): Promise<UnsubscriberAsync>;
/**
*
* 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<SettingName extends SettingNames>(
key: SettingName,
validator: SettingValidator<SettingName>,
): Promise<UnsubscriberAsync>;
} & OnDidDispose &
IDataProvider<SettingDataTypes> &
typeof settingsServiceObjectToProxy;
Expand Down
39 changes: 37 additions & 2 deletions src/main/data/core-settings-info.data.ts
Original file line number Diff line number Diff line change
@@ -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<SettingName extends SettingNames> = {
Expand All @@ -11,9 +13,42 @@ export type AllSettingsInfo = {
};

/** Info about all settings built into core. Does not contain info for extensions' settings */
const coreSettingsInfo: Partial<AllSettingsInfo> = {
export const coreSettingsInfo: Partial<AllSettingsInfo> = {
'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<boolean> => {
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<boolean> => {
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<AllSettingsValidators> = {
'platform.verseRef': verseRefSettingsValidator,
'platform.interfaceLanguage': interfaceLanguageValidator,
};
25 changes: 23 additions & 2 deletions src/main/services/settings.service-host.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean> => {
// Reject all attempts to set the verse ref
return false;
},
'platform.interfaceLanguage': async (): Promise<boolean> => {
// Accept all attempts to set the interface language
return true;
},
},
}));

test('Get verseRef returns default value', async () => {
Expand Down Expand Up @@ -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,
Expand All @@ -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",
);
});
42 changes: 40 additions & 2 deletions src/main/services/settings.service-host.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as networkService from '@shared/services/network.service';
import IDataProviderEngine, { DataProviderEngine } from '@shared/models/data-provider-engine.model';
import {
DataProviderDataType,
Expand All @@ -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');

Expand Down Expand Up @@ -48,6 +56,33 @@ function getDefaultValueForKey<SettingName extends SettingNames>(
return settingInfo.default;
}

async function validateSetting<SettingName extends SettingNames>(
key: SettingName,
newValue: SettingTypes[SettingName],
currentValue: SettingTypes[SettingName],
allChanges?: Partial<SettingTypes>,
): Promise<boolean> {
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 & {
Expand Down Expand Up @@ -86,6 +121,9 @@ class SettingDataProviderEngine
newSetting: SettingTypes[SettingName],
): Promise<DataProviderUpdateInstructions<SettingDataTypes>> {
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) {
Expand Down
42 changes: 42 additions & 0 deletions src/shared/services/settings.service-model.ts
Original file line number Diff line number Diff line change
@@ -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({
Expand All @@ -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 <SettingName extends SettingNames>(
key: SettingName,
validator: SettingValidator<SettingName>,
): Promise<UnsubscriberAsync> => {
return networkService.registerRequestHandler(
serializeRequestType(CATEGORY_EXTENSION_SETTING_VALIDATOR, key),
validator,
);
},
});
/**
* SettingDataTypes handles getting and setting Platform.Bible core application and extension
Expand All @@ -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<SettingName extends SettingNames> = (
newValue: SettingTypes[SettingName],
currentValue: SettingTypes[SettingName],
allChanges: Partial<SettingTypes>,
) => Promise<boolean>;

/** Validators for all settings. Keys are setting keys, values are functions to validate new settings */
export type AllSettingsValidators = {
[SettingName in SettingNames]: SettingValidator<SettingName>;
};

declare module 'papi-shared-types' {
export interface DataProviders {
[settingsServiceDataProviderName]: ISettingsService;
Expand Down Expand Up @@ -94,6 +130,12 @@ export type ISettingsService = {
callback: (newSetting: SettingTypes[SettingName]) => void,
options?: DataProviderSubscriberOptions,
): Promise<UnsubscriberAsync>;

/** JSDOC DESTINATION settingsServiceRegisterValidator */
registerValidator<SettingName extends SettingNames>(
key: SettingName,
validator: SettingValidator<SettingName>,
): Promise<UnsubscriberAsync>;
} & OnDidDispose &
IDataProvider<SettingDataTypes> &
typeof settingsServiceObjectToProxy;

0 comments on commit c7bd644

Please sign in to comment.