diff --git a/python/fpml/core/extract.py b/python/fpml/core/extract.py index 1ed170a..38241d0 100644 --- a/python/fpml/core/extract.py +++ b/python/fpml/core/extract.py @@ -42,7 +42,7 @@ def resolve_template( context (Optional[Context], optional): Additional context data. Defaults to None. fp_options (Optional[FPOptions], optional): Options for controlling FHIRPath evaluation. Defaults to None. strict (bool, optional): Whether to enforce strict mode. Defaults to False. - See more details on + See more details on [strict mode](https://github.com/beda-software/FHIRPathMappingLanguage/tree/main?tab=readme-ov-file#strict-mode). Returns: @@ -55,7 +55,7 @@ def resolve_template( FHIRPathMappingLanguage Specification: https://github.com/beda-software/FHIRPathMappingLanguage/tree/main?tab=readme-ov-file#specification """ # noqa: E501 - return resolve_template_recur( + result = resolve_template_recur( [], guarded_resource if strict else resource, template, @@ -64,6 +64,8 @@ def resolve_template( fp_options=fp_options, ) + return None if result == undefined else result + def resolve_template_recur( start_path: Path, @@ -72,12 +74,16 @@ def resolve_template_recur( context: Context, fp_options: Optional[FPOptions] = None, ) -> Any: - return iterate_node( + result = iterate_node( start_path, {root_node_key: template}, context or {}, lambda path, node, context: process_node(path, resource, node, context, fp_options), - ).get(root_node_key, None) + ) + if isinstance(result, dict): + return result.get(root_node_key, undefined) + + return undefined def process_node( @@ -113,7 +119,7 @@ def process_node( def iterate_node(start_path: Path, node: Node, context: Context, transform: Transformer) -> Node: if isinstance(node, list): # Arrays are flattened and null/undefined values are removed here - return flatten( + cleaned_array = flatten( [ value for value in [ @@ -127,9 +133,11 @@ def iterate_node(start_path: Path, node: Node, context: Context, transform: Tran if value is not None and value is not undefined ] ) + + return cleaned_array or undefined if isinstance(node, dict): # undefined values are removed from dicts, but nulls are preserved - return { + cleaned_object = { key: value for key, value in { key: iterate_node( @@ -142,6 +150,8 @@ def iterate_node(start_path: Path, node: Node, context: Context, transform: Tran if value is not undefined } + return cleaned_object or undefined + return transform(start_path, node, context)[0] @@ -354,20 +364,24 @@ def process_assign_block( raise FPMLValidationError( "Assign block must accept only one key per object", path ) - result = resolve_template_recur(path, resource, obj, extended_context, fp_options) + result = { + key: resolve_template_recur( + [*path, key], resource, obj_value, extended_context, fp_options + ) + for key, obj_value in obj.items() + } key = next(iter(obj.keys())) - extended_context.update({key: result.get(key, None)}) + extended_context.update({key: result[key] if result[key] != undefined else None}) elif isinstance(node[assign_key], dict) and len(node[assign_key]) == 1: obj = node[assign_key] - result = resolve_template_recur( - path, - resource, - obj, - extended_context, - fp_options, - ) + result = { + key: resolve_template_recur( + [*path, key], resource, obj_value, extended_context, fp_options + ) + for key, obj_value in obj.items() + } key = next(iter(obj.keys())) - extended_context.update({key: result.get(key, None)}) + extended_context.update({key: result[key] if result[key] != undefined else None}) else: raise FPMLValidationError("Assign block must accept array or object", path) return omit_key(node, assign_key), extended_context diff --git a/python/tests/core/test_extract.py b/python/tests/core/test_extract.py index b5b6796..c095807 100644 --- a/python/tests/core/test_extract.py +++ b/python/tests/core/test_extract.py @@ -58,11 +58,11 @@ def test_transformation_works_on_accessing_props_of_implicit_context_in_strict_m def test_transformation_for_empty_object_return_empty_object() -> None: - assert resolve_template({}, {}) == {} + assert resolve_template({}, {}) is None def test_transformation_for_empty_array_return_empty_array() -> None: - assert resolve_template({}, []) == [] + assert resolve_template({}, []) is None def test_transformation_for_array_of_arrays_returns_flattened_array() -> None: @@ -82,7 +82,7 @@ def test_transformation_for_object_with_null_keys_returns_null_keys() -> None: def test_transformation_for_object_with_undefined_keys_clears_undefined_keys() -> None: - assert resolve_template({}, {"key": undefined}) == {} + assert resolve_template({}, {"key": undefined}) is None def test_transformation_for_object_with_non_null_keys_returns_non_null_keys() -> None: @@ -123,7 +123,7 @@ def test_transformation_for_non_empty_array_expression_return_first_element() -> def test_transformation_for_empty_array_expression_clears_undefined_keys() -> None: resource: Resource = {"list": []} - assert resolve_template(resource, {"result": "{{ list.where($this = 0) }}"}) == {} + assert resolve_template(resource, {"result": "{{ list.where($this = 0) }}"}) is None def test_transformation_for_empty_array_nullable_expression_returns_null() -> None: @@ -141,23 +141,26 @@ def test_transformation_for_template_expression_returns_resolved_template() -> N ) -def test_transformation_for_empty_array_template_expression_clears_undefined_keys() -> None: +def test_transformation_for_empty_array_template_expression_returns_undefined() -> None: resource: Resource = {"list": [{"key": 1}, {"key": 2}, {"key": 3}]} assert ( resolve_template( resource, - {"result": "/Patient/{{ list.where($this = 0) }}/_history/{{ list.last() }}"}, + "/Patient/{{ list.where($this = 0) }}/_history/{{ list.last() }}", ) - == {} + is None ) def test_transformation_for_empty_array_nullable_template_expression_returns_null() -> None: resource: Resource = {"list": [{"key": 1}, {"key": 2}, {"key": 3}]} - assert resolve_template( - resource, - {"result": "/Patient/{{+ list.where($this = 0) +}}/_history/{{ list.last() }}"}, - ) == {"result": None} + assert ( + resolve_template( + resource, + "/Patient/{{+ list.where($this = 0) +}}/_history/{{ list.last() }}", + ) + is None + ) def test_transformation_for_multiline_template_works_properly() -> None: @@ -234,7 +237,7 @@ def test_assign_block_with_undefined_intermediate_values() -> None: "valueA": "{{ %varB }}", }, ) - == {} + is None ) @@ -508,7 +511,7 @@ def test_if_block_clears_undefined_keys_for_falsy_condition_without_else_branch( }, }, ) - assert result == {} + assert result is None def test_if_block_returns_null_for_falsy_condition_with_nullable_else_branch() -> None: @@ -622,7 +625,7 @@ def test_if_block_fails_on_implicit_merge_with_non_object_returned_from_if_branc { "result": { "myKey": 1, - "{% if key = 'value' %}": [], + "{% if key = 'value' %}": [{"key1": True}], }, }, ) @@ -639,7 +642,7 @@ def test_if_block_fails_on_implicit_merge_with_non_object_returned_from_else_bra "result": { "myKey": 1, "{% if key != 'value' %}": {}, - "{% else %}": [], + "{% else %}": [{"key1": True}], }, }, ) @@ -760,3 +763,17 @@ def test_merge_block_fails_on_merge_with_non_object() -> None: "{% merge %}": [1, 2], }, ) + + +@pytest.mark.skip(reason="https://github.com/beda-software/FHIRPathMappingLanguage/issues/30") +def test_null_values_are_not_removed_from_array() -> None: + resource_with_empty_array_nullable = { + "root": {"resourceType": "Example", "list": [None, {"nested": None}, None]}, + } + + result = resolve_template( + resource_with_empty_array_nullable, + {"root": "{{ %Resource.root }}"}, + {"Resource": resource_with_empty_array_nullable}, + ) + assert result == {"root": {"resourceType": "Example", "list": [None, {"nested": None}]}} diff --git a/ts/server/src/core/extract.spec.ts b/ts/server/src/core/extract.spec.ts index 6c51d8d..a42ef4b 100644 --- a/ts/server/src/core/extract.spec.ts +++ b/ts/server/src/core/extract.spec.ts @@ -3,6 +3,39 @@ import { FPMLValidationError, resolveTemplate } from './extract'; describe('Transformation', () => { const resource = { list: [{ key: 1 }, { key: 2 }, { key: 3 }] } as any; + // https://github.com/beda-software/FHIRPathMappingLanguage/issues/30 + test.skip('null values are not removed', () => { + const resourceWithEmptyArrayNullable = { + root: { resourceType: 'Example', list: [null, { nested: null }, null] }, + } as any; + expect( + resolveTemplate( + resourceWithEmptyArrayNullable, + { root: '{{ %Resource.root }}' }, + { Resource: resourceWithEmptyArrayNullable }, + null, + null, + true, + ), + ).toStrictEqual({ root: { resourceType: 'Example', list: [null, { nested: null }] } }); + }); + + test('undefined value are removed', () => { + const resourceWithEmptyArray = { + root: { resourceType: 'Example', list: [undefined, { nested: undefined }] }, + } as any; + expect( + resolveTemplate( + resourceWithEmptyArray, + { root: '{{ %Resource.root }}' }, + { Resource: resourceWithEmptyArray }, + null, + null, + true, + ), + ).toStrictEqual({ root: { resourceType: 'Example' } }); + }); + test('fails on accessing props of resource in strict mode', () => { expect(() => resolveTemplate(resource, { key: '{{ list.key }}' }, {}, null, null, true), @@ -61,12 +94,12 @@ describe('Transformation', () => { ).toStrictEqual({ key: 1 }); }); - test('for empty object return empty object', () => { - expect(resolveTemplate(resource, {})).toStrictEqual({}); + test('for empty object return undefined', () => { + expect(resolveTemplate(resource, {})).toBeNull(); }); - test('for empty array return empty array', () => { - expect(resolveTemplate(resource, [])).toStrictEqual([]); + test('for empty array return undefined', () => { + expect(resolveTemplate(resource, [])).toBeNull(); }); test('for array of arrays returns flattened array', () => { @@ -92,8 +125,12 @@ describe('Transformation', () => { expect(resolveTemplate(resource, { key: null })).toStrictEqual({ key: null }); }); - test('for object with undefined keys returns undefined keys', () => { - expect(resolveTemplate(resource, { key: undefined })).toStrictEqual({ key: undefined }); + test('for object with undefined keys returns undefined', () => { + expect(resolveTemplate(resource, { key: undefined })).toBeNull(); + }); + + test('for object with undefined keys returns only defined keys', () => { + expect(resolveTemplate(resource, { key: undefined, key2: 1 })).toStrictEqual({ key2: 1 }); }); test('for object with non-null keys returns non-null keys', () => { @@ -131,7 +168,7 @@ describe('Transformation', () => { }); test('for empty array expression returns undefined', () => { - expect(resolveTemplate(resource, '{{ list.where($this = 0) }}')).toStrictEqual(undefined); + expect(resolveTemplate(resource, '{{ list.where($this = 0) }}')).toBeNull(); }); test('for empty array nullable expression returns null', () => { @@ -150,7 +187,7 @@ describe('Transformation', () => { resource, '/Patient/{{ list.where($this = 0) }}/_history/{{ list.last() }}', ), - ).toStrictEqual(undefined); + ).toBeNull(); }); test('for empty array nullable template expression returns null', () => { @@ -230,13 +267,18 @@ describe('Assign block', () => { test('works with undefined intermediate values', () => { expect( - resolveTemplate(resource, { - '{% assign %}': [{ varA: '{{ {} }}' }, { varB: '{{ %varA }}' }], - valueA: '{{ %varB }}', - }), - ).toStrictEqual({ - valueA: undefined, - }); + resolveTemplate( + resource, + { + '{% assign %}': [{ varA: '{{ {} }}' }, { varB: '{{ %varA }}' }], + valueA: '{{ %varB }}', + }, + null, + null, + null, + true, + ), + ).toBeNull(); }); test('works with multiple vars as array of objects', () => { @@ -478,9 +520,7 @@ describe('If block', () => { "{% if key != 'value' %}": { nested: "{{ 'true' + key }}" }, }, }), - ).toStrictEqual({ - result: undefined, - }); + ).toBeNull(); }); test('returns null for falsy condition with nullable else branch', () => { @@ -600,7 +640,7 @@ describe('If block', () => { resolveTemplate(resource, { result: { myKey: 1, - "{% if key = 'value' %}": [], + "{% if key = 'value' %}": [{ key1: true }], }, }), ).toThrow(FPMLValidationError); @@ -612,7 +652,7 @@ describe('If block', () => { result: { myKey: 1, "{% if key != 'value' %}": {}, - '{% else %}': [], + '{% else %}': [{ key1: true }], }, }), ).toThrow(FPMLValidationError); diff --git a/ts/server/src/core/extract.ts b/ts/server/src/core/extract.ts index a1633ea..94e88bd 100644 --- a/ts/server/src/core/extract.ts +++ b/ts/server/src/core/extract.ts @@ -50,7 +50,7 @@ export function resolveTemplate( fpOptions?: FPOptions, strict?: boolean, ): any { - return resolveTemplateRecur( + const result = resolveTemplateRecur( [], strict ? guardedResourceFactory(resource) : resource, template, @@ -58,6 +58,9 @@ export function resolveTemplate( model, fpOptions, ); + + // NOTE: for synchronization with Python implementation + return result === undefined ? null : result; } function resolveTemplateRecur( @@ -106,7 +109,7 @@ function resolveTemplateRecur( return { node, context }; }, - )[rootNodeKey]; + )?.[rootNodeKey]; } function processTemplateString( @@ -164,7 +167,6 @@ function processAssignBlock( ): { node: any; context: Context } { const extendedContext = { ...context }; const keys = Object.keys(node); - const assignRegExp = /{%\s*assign\s*%}/; const assignKey = keys.find((k) => k.match(assignRegExp)); if (assignKey) { @@ -177,8 +179,17 @@ function processAssignBlock( ); } - Object.entries( - resolveTemplateRecur(path, resource, obj, extendedContext, model, fpOptions), + return Object.entries( + mapValues(obj, (objValue, objKey) => + resolveTemplateRecur( + [...path, objKey], + resource, + objValue, + extendedContext, + model, + fpOptions, + ), + ), ).forEach(([key, value]) => { extendedContext[key] = value; }); @@ -191,13 +202,15 @@ function processAssignBlock( ); } Object.entries( - resolveTemplateRecur( - path, - resource, - node[assignKey], - extendedContext, - model, - fpOptions, + mapValues(node[assignKey], (objValue, objKey) => + resolveTemplateRecur( + [...path, objKey], + resource, + objValue, + extendedContext, + model, + fpOptions, + ), ), ).forEach(([key, value]) => { extendedContext[key] = value; @@ -398,19 +411,26 @@ type Transformer = (path: Path, node: any, context: Context) => { node: any; con function iterateObject(startPath: Path, obj: any, context: Context, transform: Transformer): any { if (Array.isArray(obj)) { // Arrays are flattened and null/undefined values are removed here - return obj + const cleanedArray = obj .flatMap((value, index) => { const result = transform([...startPath, index], value, context); return iterateObject([...startPath, index], result.node, result.context, transform); }) .filter((x) => x !== null && x !== undefined); + return cleanedArray.length ? cleanedArray : undefined; } else if (isPlainObject(obj)) { - return mapValues(obj, (value, key) => { + const objResult = mapValues(obj, (value, key) => { const result = transform([...startPath, key], value, context); return iterateObject([...startPath, key], result.node, result.context, transform); }); + + const cleanedObject = Object.entries(objResult).filter(([, value]) => value !== undefined); + if (!cleanedObject.length) { + return undefined; + } + return Object.fromEntries(cleanedObject); } return transform(startPath, obj, context).node;