From 6624433607d9e1f87f7f42ff9c33f679b8162bc8 Mon Sep 17 00:00:00 2001 From: Emile Rolley Date: Thu, 30 May 2024 12:48:46 +0200 Subject: [PATCH 1/4] refactor(migration)!: deep simplification of the migrateSituation code --- src/commons.ts | 28 +- src/migration/index.ts | 59 ++--- src/migration/migrateSituation.ts | 245 +++++++++++------- .../deleteKeyFromSituationAndFoldedSteps.ts | 24 -- .../migrateSituation/getValueWithoutQuotes.ts | 19 -- .../handleSituationKeysMigration.ts | 97 ------- .../handleSituationValuesMigration.ts | 110 -------- .../migrateSituation/handleSpecialCases.ts | 55 ---- src/optims/index.ts | 4 +- test/migration/migrateSituation.test.ts | 86 +++--- typedoc.json | 7 +- 11 files changed, 250 insertions(+), 484 deletions(-) delete mode 100644 src/migration/migrateSituation/deleteKeyFromSituationAndFoldedSteps.ts delete mode 100644 src/migration/migrateSituation/getValueWithoutQuotes.ts delete mode 100644 src/migration/migrateSituation/handleSituationKeysMigration.ts delete mode 100644 src/migration/migrateSituation/handleSituationValuesMigration.ts delete mode 100644 src/migration/migrateSituation/handleSpecialCases.ts diff --git a/src/commons.ts b/src/commons.ts index 7a82072..f5d6d2b 100644 --- a/src/commons.ts +++ b/src/commons.ts @@ -1,5 +1,12 @@ import { basename } from 'path' -import { Rule, Logger, ExprAST, reduceAST, ASTNode } from 'publicodes' +import { + Rule, + Logger, + ExprAST, + reduceAST, + ASTNode, + Evaluation, +} from 'publicodes' import yaml from 'yaml' /** @@ -233,3 +240,22 @@ Avec : ${yaml.stringify(secondDef, { indent: 2 })}`, ) } + +/** + * Unquote a string value. + * + * @param value - The value to parse. + * + * @returns The value without quotes if it is a string, null otherwise. + */ +export function getValueWithoutQuotes(value: Evaluation) { + if ( + typeof value !== 'string' || + !value.startsWith("'") || + value === 'oui' || + value === 'non' + ) { + return null + } + return value.slice(1, -1) +} diff --git a/src/migration/index.ts b/src/migration/index.ts index 1392a39..0ee6d57 100644 --- a/src/migration/index.ts +++ b/src/migration/index.ts @@ -1,45 +1,46 @@ /** @packageDocumentation -## Migrate a situation +## Situation migration -{@link migrateSituation | `migrateSituation`} allows to migrate situation and foldedSteps based on migration instructions. It's useful in forms when a model is updated and we want old answers to be kept and taken into account in the new model. +### Why? -### Usage +In time, the `publicodes` models evolve. When a model is updated (e.g. a rule +is renamed, a value is changed, a new rule is added, etc.), we want to ensure +that the previous situations (i.e. answers to questions) are still valid. -For instance, we have a simple set of rules: +This is where the sitation migration comes in. -```yaml -age: - question: "Quel est votre âge ?" -```` +### Usage -and the following situation: -```json -{ - age: 25 -} -``` +{@link migrateSituation | `migrateSituation`} allows to migrate a siuation from +an old version of a model to a new version according to the provided _migration +instructions_. -If I change my model because I want to fix the accent to: -```yaml -âge: - question: "Quel est votre âge ?" -``` +```typescript +import { migrateSituation } from '@publicodes/tools/migration' -I don't want to lose the previous answer, so I can use `migrateSituation` with the following migration instructions: +const oldSituation = { + "age": 25 + "job": "developer", +} -```yaml -keysToMigrate: - age: âge -``` +// In the new model version, the rule `age` has been renamed to `âge` and the +// value `developer` has been translated to `développeur`. +const migrationInstructions = { + keysToMigrate: { age: 'âge' } + valuesToMigrate: { + job: { developer: 'développeur' } + } +} -Then, calling `migrateSituation` with the situation and the migration instructions will return: +console.log(migrateSituation(oldSituation, migrationInstructions)) -```json -{ - âge: 25 -} +// Output: +// { +// "âge": 25, +// "job": "développeur" +// } ``` */ diff --git a/src/migration/migrateSituation.ts b/src/migration/migrateSituation.ts index 837131b..8f76d77 100644 --- a/src/migration/migrateSituation.ts +++ b/src/migration/migrateSituation.ts @@ -1,113 +1,178 @@ -import { getValueWithoutQuotes } from './migrateSituation/getValueWithoutQuotes' -import { handleSituationKeysMigration } from './migrateSituation/handleSituationKeysMigration' -import { handleSituationValuesMigration } from './migrateSituation/handleSituationValuesMigration' -import { handleSpecialCases } from './migrateSituation/handleSpecialCases' import { Evaluation } from 'publicodes' +import { getValueWithoutQuotes, RuleName } from '../commons' -export type NodeValue = Evaluation - -export type Situation = { - [key: string]: NodeValue -} +/** + * A situation object containing all answers for a given simulation. + */ +export type Situation = Record -export type DottedName = string +/** + * Associate a old value to a new value. + */ +export type ValueMigration = Record -export type MigrationType = { - keysToMigrate: Record - valuesToMigrate: Record> +/** + * Migration instructions. It contains the rules and values to migrate. + */ +export type Migration = { + rulesToMigrate: Record + valuesToMigrate: Record } /** - * Migrate rules and answers from a situation which used to work with an old version of a model to a new version according to the migration instructions. + * Migrate a situation from an old version of a model to a new version + * according to the provided migration instructions. * - * @param {Object} options - The options object. - * @param {Situation} options.situation - The `situation` as Publicodes object containing all answers for a given simulation. - * @param {DottedName[]} [options.foldedSteps=[]] - In case of form app, an array containing answered questions. - * @param {MigrationType} options.migrationInstructions - An object containing keys and values to migrate formatted as follows: + * @param situation - The situation object containing all answers for a given simulation. + * @param instructions - The migration instructions object. * - * @example - * ``` - * { - * keysToMigrate: { - * oldKey: newKey - * } - * valuesToMigrate: { - * key: { - * oldValue: newValue - * } - * } - * ``` - * An example can be found in {@link https://github.com/incubateur-ademe/nosgestesclimat/blob/preprod/migration/migration.yaml | nosgestesclimat repository}. - * @returns {Object} The migrated situation (and foldedSteps if specified). + * @returns The migrated situation (and foldedSteps if specified). + * + * TODO: exemple of instructions (empty string for deletion, new key name for renaming, new value for updating) + * + * An example of instructions can be found {@link https://github.com/incubateur-ademe/nosgestesclimat/blob/preprod/migration/migration.yaml | here}. */ -export function migrateSituation({ - situation, - foldedSteps = [], - migrationInstructions, -}: { - situation: Situation - foldedSteps?: DottedName[] - migrationInstructions: MigrationType -}) { - let situationMigrated = { ...situation } - let foldedStepsMigrated = [...foldedSteps] - - Object.entries(situationMigrated).map(([ruleName, nodeValue]) => { - situationMigrated = handleSpecialCases({ - ruleName, - nodeValue, - situation: situationMigrated, - }) - - // We check if the non supported ruleName is a key to migrate. - // Ex: "logement . chauffage . bois . type . bûche . consommation": "xxx" which is now ""logement . chauffage . bois . type . bûches . consommation": "xxx" - if (Object.keys(migrationInstructions.keysToMigrate).includes(ruleName)) { - const result = handleSituationKeysMigration({ - ruleName, - nodeValue, - situation: situationMigrated, - foldedSteps: foldedStepsMigrated, - migrationInstructions, - }) - - situationMigrated = result.situationMigrated - foldedStepsMigrated = result.foldedStepsMigrated - } +export function migrateSituation( + situation: Situation, + instructions: Migration, +): Situation { + let newSituation = { ...situation } + const currentRules = Object.keys(situation) + const valueKeysToMigrate = Object.keys(instructions.valuesToMigrate) - const matchingValueToMigrateObject = - migrationInstructions.valuesToMigrate[ - Object.keys(migrationInstructions.valuesToMigrate).find((key) => - ruleName.includes(key), - ) as any - ] + Object.entries(situation).map(([rule, value]) => { + handleSpecialCases(rule, value, newSituation) + + if (currentRules.includes(rule)) { + updateKey(rule, value, newSituation, instructions.rulesToMigrate[rule]) + } - const formattedNodeValue = - getValueWithoutQuotes(nodeValue) || (nodeValue as string) + const formattedValue = getValueWithoutQuotes(value) ?? (value as string) + const valuesMigration = + instructions.valuesToMigrate[ + valueKeysToMigrate.find((key) => rule.includes(key)) + ] ?? {} + const oldValuesName = Object.keys(valuesMigration) if ( // We check if the value of the non supported ruleName value is a value to migrate. // Ex: answer "logement . chauffage . bois . type": "bûche" changed to "bûches" // If a value is specified but empty, we consider it to be deleted (we need to ask the question again) // Ex: answer "transport . boulot . commun . type": "vélo" - matchingValueToMigrateObject && - Object.keys(matchingValueToMigrateObject).includes( - // If the string start with a ', we remove it along with the last character - // Ex: "'bûche'" => "bûche" - formattedNodeValue, - ) + oldValuesName.includes(formattedValue) ) { - const result = handleSituationValuesMigration({ - ruleName, - nodeValue: formattedNodeValue, - situation: situationMigrated, - foldedSteps: foldedStepsMigrated, - migrationInstructions, - }) - - situationMigrated = result.situationMigrated - foldedStepsMigrated = result.foldedStepsMigrated + updateValue(rule, valuesMigration[formattedValue], newSituation) } }) - return { situationMigrated, foldedStepsMigrated } + return newSituation +} + +// Handle migration of old value format : an object { valeur: number, unité: string } +/** + * Handles special cases during the migration of old value formats. + * + * @example + * ```` +{ valeur: number, unité: string } +``` + * + * @param rule - The name of the rule. + * @param oldValue - The node value. + * @param situation - The situation object. + * @returns - The updated situation object. + */ +function handleSpecialCases( + rule: RuleName, + oldValue: Evaluation, + situation: Situation, +): void { + // Special case, number store as a string, we have to convert it to a number + if ( + oldValue && + typeof oldValue === 'string' && + !isNaN(parseFloat(oldValue)) + ) { + situation[rule] = parseFloat(oldValue) + } + + // Special case : wrong value format, legacy from previous publicodes version + // handle the case where valeur is a string "2.33" + if (oldValue && oldValue['valeur'] !== undefined) { + situation[rule] = + typeof oldValue['valeur'] === 'string' && + !isNaN(parseFloat(oldValue['valeur'])) + ? parseFloat(oldValue['valeur']) + : (oldValue['valeur'] as number) + } + // Special case : other wrong value format, legacy from previous publicodes version + // handle the case where nodeValue is a string "2.33" + if (oldValue && oldValue['nodeValue'] !== undefined) { + situation[rule] = + typeof oldValue['nodeValue'] === 'string' && + !isNaN(parseFloat(oldValue['nodeValue'])) + ? parseFloat(oldValue['nodeValue']) + : (oldValue['nodeValue'] as number) + } +} + +/** + */ +function updateKey( + rule: RuleName, + oldValue: Evaluation, + situation: Situation, + ruleToMigrate: RuleName | undefined, +): void { + if (ruleToMigrate === undefined) { + return + } + + delete situation[rule] + + if (ruleToMigrate !== '') { + situation[ruleToMigrate] = + typeof oldValue === 'object' ? (oldValue as any)?.valeur : oldValue + } +} + +/** + */ +export function updateValue( + rule: RuleName, + value: string, + situation: Situation, +): void { + // The value is not a value to migrate and the key has to be deleted + if (value === '') { + delete situation[rule] + } else { + // The value is renamed and needs to be migrated + situation[rule] = getMigratedValue(value) + } +} + +function getMigratedValue(value: string): string { + if (typeof value === 'string' && value !== 'oui' && value !== 'non') { + return `'${value}'` + } + + // FIXME: I'm not sure if it's necessary to check if the value is a number, + // as valuesToMigrate is a ValueMigration object (Record). + // Is it possible to have objects in valuesToMigrate? + // if ( + // ( + // value as unknown as { + // valeur: number + // } + // )?.valeur !== undefined + // ) { + // return ( + // value as unknown as { + // valeur: number + // } + // ).valeur as unknown as string + // } + + return value } diff --git a/src/migration/migrateSituation/deleteKeyFromSituationAndFoldedSteps.ts b/src/migration/migrateSituation/deleteKeyFromSituationAndFoldedSteps.ts deleted file mode 100644 index 393779d..0000000 --- a/src/migration/migrateSituation/deleteKeyFromSituationAndFoldedSteps.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { DottedName, Situation } from '../migrateSituation' - -/** - * Delete a key from the situation and from the foldedSteps if it exists. - * @param ruleName - The rulename to delete. - * @param situation - The situation object. - * @param foldedSteps - The foldedSteps array. - */ -export function deleteKeyFromSituationAndFoldedSteps({ - ruleName, - situation, - foldedSteps, -}: { - ruleName: string - situation: Situation - foldedSteps: DottedName[] -}) { - delete situation[ruleName] - const index = foldedSteps?.indexOf(ruleName) - - if (index > -1) { - foldedSteps.splice(index, 1) - } -} diff --git a/src/migration/migrateSituation/getValueWithoutQuotes.ts b/src/migration/migrateSituation/getValueWithoutQuotes.ts deleted file mode 100644 index 9cd8ef8..0000000 --- a/src/migration/migrateSituation/getValueWithoutQuotes.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { NodeValue } from '../migrateSituation' - -/** - * Returns the value without quotes if it is a string. - * @param value - The value to parse. - * - * @returns The value without quotes if it is a string, null otherwise. - */ -export function getValueWithoutQuotes(value: NodeValue) { - if ( - typeof value !== 'string' || - !value.startsWith("'") || - value === 'oui' || - value === 'non' - ) { - return null - } - return value.slice(1, -1) -} diff --git a/src/migration/migrateSituation/handleSituationKeysMigration.ts b/src/migration/migrateSituation/handleSituationKeysMigration.ts deleted file mode 100644 index 9e77d3f..0000000 --- a/src/migration/migrateSituation/handleSituationKeysMigration.ts +++ /dev/null @@ -1,97 +0,0 @@ -import { - DottedName, - MigrationType, - NodeValue, - Situation, -} from '../migrateSituation' -import { deleteKeyFromSituationAndFoldedSteps } from './deleteKeyFromSituationAndFoldedSteps' - -type Props = { - ruleName: string - nodeValue: NodeValue - situation: Situation - foldedSteps: DottedName[] - migrationInstructions: MigrationType -} - -/** - * Updates a key in the situation object and foldedSteps. - * @param ruleName - The name of the rule to update. - * @param nodeValue - The new value for the rule. - * @param situation - The situation object. - * @param foldedSteps - The array of foldedSteps. - * @param migrationInstructions - The migration instructions. - */ -function updateKeyInSituationAndFoldedSteps({ - ruleName, - nodeValue, - situation, - foldedSteps, - migrationInstructions, -}: { - ruleName: string - nodeValue: NodeValue - situation: Situation - foldedSteps: DottedName[] - migrationInstructions: MigrationType -}) { - situation[migrationInstructions.keysToMigrate[ruleName]] = - (nodeValue as any)?.valeur ?? nodeValue - - delete situation[ruleName] - - const index = foldedSteps?.indexOf(ruleName) - - if (index > -1) { - foldedSteps[index] = migrationInstructions.keysToMigrate[ruleName] - } -} - -/** - * Updates a key in the situation and foldedSteps based on migration instructions. - * If the key is not a key to migrate but a key to delete, it will be removed from the situation and foldedSteps. - * If the key is renamed and needs to be migrated, it will be updated in the situation and foldedSteps. - * - * @param ruleName - The name of the rule/key to update. - * @param nodeValue - The new value for the rule/key. - * @param situation - The current situation object. - * @param foldedSteps - The current foldedSteps array. - * @param migrationInstructions - The migration instructions object. - * - * @returns An object containing the migrated situation and foldedSteps. - */ -export function handleSituationKeysMigration({ - ruleName, - nodeValue, - situation, - foldedSteps, - migrationInstructions, -}: Props): { situationMigrated: Situation; foldedStepsMigrated: DottedName[] } { - const situationMigrated = { ...situation } - const foldedStepsMigrated = [...foldedSteps] - - // The key is not a key to migrate but a key to delete - if (migrationInstructions.keysToMigrate[ruleName] === '') { - deleteKeyFromSituationAndFoldedSteps({ - ruleName, - situation: situationMigrated, - foldedSteps: foldedStepsMigrated, - }) - return { situationMigrated, foldedStepsMigrated } - } - - if (!migrationInstructions.keysToMigrate[ruleName]) { - return - } - - // The key is renamed and needs to be migrated - updateKeyInSituationAndFoldedSteps({ - ruleName, - nodeValue, - situation: situationMigrated, - foldedSteps: foldedStepsMigrated, - migrationInstructions, - }) - - return { situationMigrated, foldedStepsMigrated } -} diff --git a/src/migration/migrateSituation/handleSituationValuesMigration.ts b/src/migration/migrateSituation/handleSituationValuesMigration.ts deleted file mode 100644 index 4bf9df5..0000000 --- a/src/migration/migrateSituation/handleSituationValuesMigration.ts +++ /dev/null @@ -1,110 +0,0 @@ -import { - DottedName, - MigrationType, - NodeValue, - Situation, -} from '../migrateSituation' -import { deleteKeyFromSituationAndFoldedSteps } from './deleteKeyFromSituationAndFoldedSteps' - -type Props = { - ruleName: DottedName - nodeValue: NodeValue - situation: Situation - foldedSteps: DottedName[] - migrationInstructions: MigrationType -} - -/** - * Get the migrated value. - * - * @param ruleName - The name of the rule to update. - * @param nodeValue - The new value for the rule. - * @param migrationInstructions - The migration instructions. - */ -function getMigratedValue({ - ruleName, - nodeValue, - migrationInstructions, -}: { - ruleName: DottedName - nodeValue: NodeValue - migrationInstructions: MigrationType -}): NodeValue { - if ( - typeof migrationInstructions.valuesToMigrate[ruleName][ - nodeValue as string - ] === 'string' && - migrationInstructions.valuesToMigrate[ruleName][nodeValue as string] !== - 'oui' && - migrationInstructions.valuesToMigrate[ruleName][nodeValue as string] !== - 'non' - ) { - return `'${migrationInstructions.valuesToMigrate[ruleName][nodeValue as string]}'` - } - - if ( - ( - migrationInstructions.valuesToMigrate[ruleName][nodeValue as string] as { - valeur: number - } - )?.valeur !== undefined - ) { - return ( - migrationInstructions.valuesToMigrate[ruleName][nodeValue as string] as { - valeur: number - } - ).valeur - } - - return migrationInstructions.valuesToMigrate[ruleName][ - nodeValue as string - ] as string | number -} - -/** - * Handles the migration of situation values based on the provided migration instructions. - * - * @param ruleName - The name of the rule/key to update. - * @param nodeValue - The new value for the rule/key. - * @param situation - The current situation object. - * @param foldedSteps - The current foldedSteps array. - * @param migrationInstructions - The migration instructions object. - * - * @returns An object containing the migrated situation and foldedSteps. - */ -export function handleSituationValuesMigration({ - ruleName, - nodeValue, - situation, - foldedSteps, - migrationInstructions, -}: Props): { situationMigrated: Situation; foldedStepsMigrated: DottedName[] } { - if (!migrationInstructions.valuesToMigrate[ruleName]) { - return - } - - const situationMigrated = { ...situation } - const foldedStepsMigrated = [...foldedSteps] - - // The value is not a value to migrate and the key has to be deleted - if ( - migrationInstructions.valuesToMigrate[ruleName][nodeValue as string] === '' - ) { - deleteKeyFromSituationAndFoldedSteps({ - ruleName, - situation: situationMigrated, - foldedSteps: foldedStepsMigrated, - }) - - return { situationMigrated, foldedStepsMigrated } - } - - // The value is renamed and needs to be migrated - situationMigrated[ruleName] = getMigratedValue({ - ruleName, - nodeValue, - migrationInstructions, - }) - - return { situationMigrated, foldedStepsMigrated } -} diff --git a/src/migration/migrateSituation/handleSpecialCases.ts b/src/migration/migrateSituation/handleSpecialCases.ts deleted file mode 100644 index ace4870..0000000 --- a/src/migration/migrateSituation/handleSpecialCases.ts +++ /dev/null @@ -1,55 +0,0 @@ -import { DottedName, NodeValue, Situation } from '../migrateSituation' - -type Props = { - ruleName: DottedName - nodeValue: NodeValue - situation: Situation -} - -// Handle migration of old value format : an object { valeur: number, unité: string } -/** - * Handles special cases during the migration of old value formats. - * - * @example - * ```` -{ valeur: number, unité: string } -``` - * - * @param ruleName - The name of the rule. - * @param nodeValue - The node value. - * @param situation - The situation object. - * @returns - The updated situation object. - */ -export function handleSpecialCases({ ruleName, nodeValue, situation }: Props) { - const situationUpdated = { ...situation } - - // Special case, number store as a string, we have to convert it to a number - if ( - nodeValue && - typeof nodeValue === 'string' && - !isNaN(parseFloat(nodeValue)) - ) { - situationUpdated[ruleName] = parseFloat(nodeValue) - } - - // Special case : wrong value format, legacy from previous publicodes version - // handle the case where valeur is a string "2.33" - if (nodeValue && nodeValue['valeur'] !== undefined) { - situationUpdated[ruleName] = - typeof nodeValue['valeur'] === 'string' && - !isNaN(parseFloat(nodeValue['valeur'])) - ? parseFloat(nodeValue['valeur']) - : (nodeValue['valeur'] as number) - } - // Special case : other wrong value format, legacy from previous publicodes version - // handle the case where nodeValue is a string "2.33" - if (nodeValue && nodeValue['nodeValue'] !== undefined) { - situationUpdated[ruleName] = - typeof nodeValue['nodeValue'] === 'string' && - !isNaN(parseFloat(nodeValue['nodeValue'])) - ? parseFloat(nodeValue['nodeValue']) - : (nodeValue['nodeValue'] as number) - } - - return situationUpdated -} diff --git a/src/optims/index.ts b/src/optims/index.ts index 611c27a..c2e053c 100644 --- a/src/optims/index.ts +++ b/src/optims/index.ts @@ -1,5 +1,5 @@ /** @packageDocumentation - + ## The constant folding pass Currently, only one optimisation pass is available: the constant folding one. @@ -43,7 +43,7 @@ alimentation . déchets . niveau moyen: somme: - 96.0151712 - 48.512508999999994 - - 49.9611611 + - 49.9611611 - gestes description: | Ce niveau correspond à la moyenne française. diff --git a/test/migration/migrateSituation.test.ts b/test/migration/migrateSituation.test.ts index 8a87069..b028ac5 100644 --- a/test/migration/migrateSituation.test.ts +++ b/test/migration/migrateSituation.test.ts @@ -1,87 +1,61 @@ -import { migrateSituation } from '../../src/migration/migrateSituation' +import { + migrateSituation, + Migration, + Situation, +} from '../../src/migration/migrateSituation' -const migrationInstructions = { - keysToMigrate: { age: 'âge', 'année de naissance': '' }, +const instructions: Migration = { + rulesToMigrate: { age: 'âge', 'année de naissance': '' }, valuesToMigrate: { prénom: { jean: 'Jean avec un J', michel: '' } }, } +const migrateSituationWithInstructions = (situation: Situation) => + migrateSituation(situation, instructions) + describe('migrateSituation', () => { it('should migrate key', () => { - expect( - migrateSituation({ - situation: { age: 27 }, - foldedSteps: ['age'], - migrationInstructions, - }), - ).toEqual({ situationMigrated: { âge: 27 }, foldedStepsMigrated: ['âge'] }) + expect(migrateSituationWithInstructions({ age: 27 })).toEqual({ âge: 27 }) }), it('should migrate value', () => { - expect( - migrateSituation({ - situation: { prénom: 'jean' }, - foldedSteps: ['prénom'], - migrationInstructions, - }), - ).toEqual({ - situationMigrated: { prénom: "'Jean avec un J'" }, - foldedStepsMigrated: ['prénom'], + expect(migrateSituationWithInstructions({ prénom: 'jean' })).toEqual({ + prénom: "'Jean avec un J'", }) }), it('should delete key', () => { expect( - migrateSituation({ - situation: { 'année de naissance': 1997 }, - foldedSteps: ['année de naissance'], - migrationInstructions, - }), - ).toEqual({ - foldedStepsMigrated: [], - situationMigrated: {}, - }) + migrateSituationWithInstructions({ 'année de naissance': 1997 }), + ).toEqual({}) }), it('should delete value', () => { expect( - migrateSituation({ - situation: { prénom: 'michel' }, - foldedSteps: ['prénom'], - migrationInstructions, + migrateSituationWithInstructions({ + prénom: 'michel', }), - ).toEqual({ - foldedStepsMigrated: [], - situationMigrated: {}, - }) + ).toEqual({}) }), it('should support old situations (1)', () => { expect( - migrateSituation({ - situation: { âge: { valeur: 27, unité: 'an' } }, - foldedSteps: ['âge'], - migrationInstructions, + migrateSituationWithInstructions({ + âge: { valeur: 27, unité: 'an' }, }), ).toEqual({ - foldedStepsMigrated: ['âge'], - situationMigrated: { âge: 27 }, + âge: 27, }) }), it('should support old situations (2)', () => { expect( - migrateSituation({ - situation: { - âge: { - type: 'number', - fullPrecision: true, - isNullable: false, - nodeValue: 27, - nodeKind: 'constant', - rawNode: 27, - }, + migrateSituationWithInstructions({ + âge: { + type: 'number', + fullPrecision: true, + isNullable: false, + nodeValue: 27, + nodeKind: 'constant', + rawNode: 27, }, - foldedSteps: ['âge'], - migrationInstructions, }), ).toEqual({ - foldedStepsMigrated: ['âge'], - situationMigrated: { âge: 27 }, + âge: 27, }) }) }) diff --git a/typedoc.json b/typedoc.json index 9ae18cb..7c589c7 100644 --- a/typedoc.json +++ b/typedoc.json @@ -1,7 +1,12 @@ { "$schema": "https://typedoc.org/schema.json", "name": "@publicodes/tools API", - "entryPoints": ["./src", "./src/optims/", "./src/compilation/"], + "entryPoints": [ + "./src", + "./src/optims/", + "./src/compilation/", + "./src/migration/" + ], "navigationLinks": { "GitHub": "https://github.com/publicodes/tools" }, From 6c731422a32787fcb33d8530624a18940c093094 Mon Sep 17 00:00:00 2001 From: Emile Rolley Date: Thu, 30 May 2024 18:12:29 +0200 Subject: [PATCH 2/4] refactor(migration): cleaning --- src/migration/index.ts | 31 +++--- src/migration/migrateSituation.ts | 92 +++++++-------- test/migration/migrateSituation.test.ts | 142 +++++++++++++++++------- 3 files changed, 155 insertions(+), 110 deletions(-) diff --git a/src/migration/index.ts b/src/migration/index.ts index 0ee6d57..d52f626 100644 --- a/src/migration/index.ts +++ b/src/migration/index.ts @@ -12,7 +12,7 @@ This is where the sitation migration comes in. ### Usage -{@link migrateSituation | `migrateSituation`} allows to migrate a siuation from +{@link migrateSituation | `migrateSituation`} allows to migrate a situation from an old version of a model to a new version according to the provided _migration instructions_. @@ -20,27 +20,28 @@ instructions_. ```typescript import { migrateSituation } from '@publicodes/tools/migration' -const oldSituation = { - "age": 25 +const situation = { + "age": 25, "job": "developer", + "city": "Paris" } -// In the new model version, the rule `age` has been renamed to `âge` and the -// value `developer` has been translated to `développeur`. -const migrationInstructions = { - keysToMigrate: { age: 'âge' } +const instructions = { + keysToMigrate: { + // The rule `age` has been renamed to `âge`. + age: 'âge', + // The rule `city` has been removed. + city: '' + }, valuesToMigrate: { - job: { developer: 'développeur' } + job: { + // The value `developer` has been translated to `développeur`. + developer: 'développeur' + } } } -console.log(migrateSituation(oldSituation, migrationInstructions)) - -// Output: -// { -// "âge": 25, -// "job": "développeur" -// } +migrateSituation(situation, instructions) // { "âge": 25, "job": "'développeur'" } ``` */ diff --git a/src/migration/migrateSituation.ts b/src/migration/migrateSituation.ts index 8f76d77..ed7031b 100644 --- a/src/migration/migrateSituation.ts +++ b/src/migration/migrateSituation.ts @@ -15,7 +15,7 @@ export type ValueMigration = Record * Migration instructions. It contains the rules and values to migrate. */ export type Migration = { - rulesToMigrate: Record + keysToMigrate: Record valuesToMigrate: Record } @@ -28,9 +28,35 @@ export type Migration = { * * @returns The migrated situation (and foldedSteps if specified). * - * TODO: exemple of instructions (empty string for deletion, new key name for renaming, new value for updating) + * @example + * ```typescript + * import { migrateSituation } from '@publicodes/tools/migration' + * + * const situation = { + * "age": 25 + * "job": "developer", + * "city": "Paris" + * } + * + * const instructions = { + * keysToMigrate: { + * // The rule `age` will be renamed to `âge`. + * age: 'âge', + * // The rule `city` will be removed. + * city: '' + * }, + * valuesToMigrate: { + * job: { + * // The value `developer` will be translated to `développeur`. + * developer: 'développeur' + * } + * } + * } + * + * migrateSituation(situation, instructions) // { "âge": 25, "job": "'développeur'" } + * ``` * - * An example of instructions can be found {@link https://github.com/incubateur-ademe/nosgestesclimat/blob/preprod/migration/migration.yaml | here}. + * @note An example of instructions can be found {@link https://github.com/incubateur-ademe/nosgestesclimat/blob/preprod/migration/migration.yaml | here}. */ export function migrateSituation( situation: Situation, @@ -44,7 +70,7 @@ export function migrateSituation( handleSpecialCases(rule, value, newSituation) if (currentRules.includes(rule)) { - updateKey(rule, value, newSituation, instructions.rulesToMigrate[rule]) + updateKey(rule, value, newSituation, instructions.keysToMigrate[rule]) } const formattedValue = getValueWithoutQuotes(value) ?? (value as string) @@ -54,13 +80,7 @@ export function migrateSituation( ] ?? {} const oldValuesName = Object.keys(valuesMigration) - if ( - // We check if the value of the non supported ruleName value is a value to migrate. - // Ex: answer "logement . chauffage . bois . type": "bûche" changed to "bûches" - // If a value is specified but empty, we consider it to be deleted (we need to ask the question again) - // Ex: answer "transport . boulot . commun . type": "vélo" - oldValuesName.includes(formattedValue) - ) { + if (oldValuesName.includes(formattedValue)) { updateValue(rule, valuesMigration[formattedValue], newSituation) } }) @@ -68,19 +88,13 @@ export function migrateSituation( return newSituation } -// Handle migration of old value format : an object { valeur: number, unité: string } /** - * Handles special cases during the migration of old value formats. + * Handle migration of old value format : an object { valeur: number, unité: string }. * * @example - * ```` -{ valeur: number, unité: string } -``` - * - * @param rule - The name of the rule. - * @param oldValue - The node value. - * @param situation - The situation object. - * @returns - The updated situation object. + * ```json + * { valeur: number, unité: string } + * ``` */ function handleSpecialCases( rule: RuleName, @@ -116,8 +130,6 @@ function handleSpecialCases( } } -/** - */ function updateKey( rule: RuleName, oldValue: Evaluation, @@ -136,9 +148,7 @@ function updateKey( } } -/** - */ -export function updateValue( +function updateValue( rule: RuleName, value: string, situation: Situation, @@ -148,31 +158,9 @@ export function updateValue( delete situation[rule] } else { // The value is renamed and needs to be migrated - situation[rule] = getMigratedValue(value) - } -} - -function getMigratedValue(value: string): string { - if (typeof value === 'string' && value !== 'oui' && value !== 'non') { - return `'${value}'` + situation[rule] = + typeof value === 'string' && value !== 'oui' && value !== 'non' + ? `'${value}'` + : value } - - // FIXME: I'm not sure if it's necessary to check if the value is a number, - // as valuesToMigrate is a ValueMigration object (Record). - // Is it possible to have objects in valuesToMigrate? - // if ( - // ( - // value as unknown as { - // valeur: number - // } - // )?.valeur !== undefined - // ) { - // return ( - // value as unknown as { - // valeur: number - // } - // ).valeur as unknown as string - // } - - return value } diff --git a/test/migration/migrateSituation.test.ts b/test/migration/migrateSituation.test.ts index b028ac5..97abb24 100644 --- a/test/migration/migrateSituation.test.ts +++ b/test/migration/migrateSituation.test.ts @@ -5,7 +5,7 @@ import { } from '../../src/migration/migrateSituation' const instructions: Migration = { - rulesToMigrate: { age: 'âge', 'année de naissance': '' }, + keysToMigrate: { age: 'âge', 'année de naissance': '' }, valuesToMigrate: { prénom: { jean: 'Jean avec un J', michel: '' } }, } @@ -15,47 +15,103 @@ const migrateSituationWithInstructions = (situation: Situation) => describe('migrateSituation', () => { it('should migrate key', () => { expect(migrateSituationWithInstructions({ age: 27 })).toEqual({ âge: 27 }) - }), - it('should migrate value', () => { - expect(migrateSituationWithInstructions({ prénom: 'jean' })).toEqual({ - prénom: "'Jean avec un J'", - }) - }), - it('should delete key', () => { - expect( - migrateSituationWithInstructions({ 'année de naissance': 1997 }), - ).toEqual({}) - }), - it('should delete value', () => { - expect( - migrateSituationWithInstructions({ - prénom: 'michel', - }), - ).toEqual({}) - }), - it('should support old situations (1)', () => { - expect( - migrateSituationWithInstructions({ - âge: { valeur: 27, unité: 'an' }, - }), - ).toEqual({ - âge: 27, - }) - }), - it('should support old situations (2)', () => { - expect( - migrateSituationWithInstructions({ - âge: { - type: 'number', - fullPrecision: true, - isNullable: false, - nodeValue: 27, - nodeKind: 'constant', - rawNode: 27, - }, - }), - ).toEqual({ - âge: 27, - }) + }) + + it('should migrate value', () => { + expect(migrateSituationWithInstructions({ prénom: 'jean' })).toEqual({ + prénom: "'Jean avec un J'", + }) + }) + + it('should delete key', () => { + expect( + migrateSituationWithInstructions({ 'année de naissance': 1997 }), + ).toEqual({}) + }) + + it('should delete value', () => { + expect( + migrateSituationWithInstructions({ + prénom: 'michel', + }), + ).toEqual({}) + }) + + it('should support old situations (1)', () => { + expect( + migrateSituationWithInstructions({ + âge: { valeur: 27, unité: 'an' }, + }), + ).toEqual({ + âge: 27, + }) + }) + + it('should support old situations (2)', () => { + expect( + migrateSituationWithInstructions({ + âge: { + type: 'number', + fullPrecision: true, + isNullable: false, + nodeValue: 27, + nodeKind: 'constant', + rawNode: 27, + }, + }), + ).toEqual({ + âge: 27, + }) + }) + + it('should migrate the API example', () => { + const oldSituation = { + age: 25, + job: 'developer', + city: 'Paris', + } + + const instructions = { + keysToMigrate: { + age: 'âge', + city: '', + }, + valuesToMigrate: { + job: { + developer: 'développeur', + }, + }, + } + expect(migrateSituation(oldSituation, instructions)).toEqual({ + âge: 25, + job: "'développeur'", + }) + }) + + it('should not modify the original situation', () => { + const situation = { + job: 'developer', + âge: { + type: 'number', + fullPrecision: true, + isNullable: false, + nodeValue: 27, + nodeKind: 'constant', + rawNode: 27, + }, + } + + migrateSituation(situation, instructions) + expect(situation).toEqual({ + âge: { + type: 'number', + fullPrecision: true, + isNullable: false, + nodeValue: 27, + nodeKind: 'constant', + rawNode: 27, + }, + job: 'developer', }) + }) }) From 71c78302cc64ef599a3b3355248ab7e91d01b642 Mon Sep 17 00:00:00 2001 From: Emile Rolley Date: Mon, 3 Jun 2024 17:15:04 +0200 Subject: [PATCH 3/4] pkg: upgrade publicodes --- package.json | 2 +- src/migration/migrateSituation.ts | 17 ++++++----------- test/optims/constantFolding.test.ts | 4 ++-- test/serializeParsedRules.test.ts | 2 +- test/utils.test.ts | 2 +- yarn.lock | 8 ++++---- 6 files changed, 15 insertions(+), 20 deletions(-) diff --git a/package.json b/package.json index d4f005b..8c80e60 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,7 @@ "license": "MIT", "dependencies": { "@types/node": "^18.11.18", - "publicodes": "^1.1.1" + "publicodes": "^1.3.0" }, "devDependencies": { "@types/jest": "^29.2.5", diff --git a/src/migration/migrateSituation.ts b/src/migration/migrateSituation.ts index ed7031b..d2ac94d 100644 --- a/src/migration/migrateSituation.ts +++ b/src/migration/migrateSituation.ts @@ -1,11 +1,6 @@ -import { Evaluation } from 'publicodes' +import { Evaluation, Situation } from 'publicodes' import { getValueWithoutQuotes, RuleName } from '../commons' -/** - * A situation object containing all answers for a given simulation. - */ -export type Situation = Record - /** * Associate a old value to a new value. */ @@ -59,9 +54,9 @@ export type Migration = { * @note An example of instructions can be found {@link https://github.com/incubateur-ademe/nosgestesclimat/blob/preprod/migration/migration.yaml | here}. */ export function migrateSituation( - situation: Situation, + situation: Situation, instructions: Migration, -): Situation { +): Situation { let newSituation = { ...situation } const currentRules = Object.keys(situation) const valueKeysToMigrate = Object.keys(instructions.valuesToMigrate) @@ -99,7 +94,7 @@ export function migrateSituation( function handleSpecialCases( rule: RuleName, oldValue: Evaluation, - situation: Situation, + situation: Situation, ): void { // Special case, number store as a string, we have to convert it to a number if ( @@ -133,7 +128,7 @@ function handleSpecialCases( function updateKey( rule: RuleName, oldValue: Evaluation, - situation: Situation, + situation: Situation, ruleToMigrate: RuleName | undefined, ): void { if (ruleToMigrate === undefined) { @@ -151,7 +146,7 @@ function updateKey( function updateValue( rule: RuleName, value: string, - situation: Situation, + situation: Situation, ): void { // The value is not a value to migrate and the key has to be deleted if (value === '') { diff --git a/test/optims/constantFolding.test.ts b/test/optims/constantFolding.test.ts index 02d965c..33fd112 100644 --- a/test/optims/constantFolding.test.ts +++ b/test/optims/constantFolding.test.ts @@ -35,7 +35,7 @@ describe('Constant folding [meta]', () => { } const engine = new Engine(rawRules, { logger: disabledLogger, - allowOrphanRules: true, + strict: { noOrphanRule: false }, }) const baseParsedRules = engine.getParsedRules() const serializedBaseParsedRules = serializeParsedRules(baseParsedRules) @@ -71,7 +71,7 @@ describe('Constant folding [meta]', () => { } const engine = new Engine(rawRules, { logger: disabledLogger, - allowOrphanRules: true, + strict: { noOrphanRule: false }, }) const foldedRules = serializeParsedRules( constantFolding(engine, { diff --git a/test/serializeParsedRules.test.ts b/test/serializeParsedRules.test.ts index 550e361..1780815 100644 --- a/test/serializeParsedRules.test.ts +++ b/test/serializeParsedRules.test.ts @@ -894,7 +894,7 @@ describe('More complexe cases', () => { }, } const parsedRules = new Engine(rules, { - allowOrphanRules: true, + strict: { noOrphanRule: false }, }).getParsedRules() expect(serializeParsedRules(parsedRules)).toStrictEqual( diff --git a/test/utils.test.ts b/test/utils.test.ts index 31ea578..8f434b6 100644 --- a/test/utils.test.ts +++ b/test/utils.test.ts @@ -5,7 +5,7 @@ import type { ParsedRules } from 'publicodes' export function callWithEngine(fn: (engine: Engine) => R, rawRules: any): R { const engine = new Engine(rawRules, { logger: disabledLogger, - allowOrphanRules: true, + strict: { noOrphanRule: false }, }) return fn(engine) } diff --git a/yarn.lock b/yarn.lock index 1ee370c..51b6c87 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2498,10 +2498,10 @@ prompts@^2.0.1: kleur "^3.0.3" sisteransi "^1.0.5" -publicodes@^1.1.1: - version "1.2.0" - resolved "https://registry.yarnpkg.com/publicodes/-/publicodes-1.2.0.tgz#2856891da07633315eff419402bb61109045504d" - integrity sha512-qit3KsTqwZct5Xt6uyJ83yXNXxxScKuSRvemUgCRWEczP/mPAX/tiDQUgwwOC2fDoiCuQuZ9EiQwU5HfzHFIJQ== +publicodes@^1.3.0: + version "1.3.0" + resolved "https://registry.yarnpkg.com/publicodes/-/publicodes-1.3.0.tgz#5530e029ef6aaea2373c56a846d78732c56abc66" + integrity sha512-VBicg3+/SZUe49LzM77CpxWEjPra7sWDnbJyGVShDd2Tuo/p3Krb7ebe/kNM5Z1WDwuqCYLT+X75Co7fi/tMAA== punycode@^2.1.0: version "2.3.1" From af8380e2f2b8708ea12185fb8e561d680ce6972c Mon Sep 17 00:00:00 2001 From: Emile Rolley Date: Mon, 3 Jun 2024 17:17:29 +0200 Subject: [PATCH 4/4] fix: use the publicodes Situation type --- test/migration/migrateSituation.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/migration/migrateSituation.test.ts b/test/migration/migrateSituation.test.ts index 97abb24..297034d 100644 --- a/test/migration/migrateSituation.test.ts +++ b/test/migration/migrateSituation.test.ts @@ -1,7 +1,8 @@ +import { Situation } from 'publicodes' +import { RuleName } from '../../src' import { migrateSituation, Migration, - Situation, } from '../../src/migration/migrateSituation' const instructions: Migration = { @@ -9,7 +10,7 @@ const instructions: Migration = { valuesToMigrate: { prénom: { jean: 'Jean avec un J', michel: '' } }, } -const migrateSituationWithInstructions = (situation: Situation) => +const migrateSituationWithInstructions = (situation: Situation) => migrateSituation(situation, instructions) describe('migrateSituation', () => {