From 3e65aa723394d0bad6245bbb38a1ee70f0137719 Mon Sep 17 00:00:00 2001 From: Marwan Tammam Date: Thu, 30 Oct 2025 11:55:16 -0400 Subject: [PATCH 1/4] Fix some issues with fhirpath expression evaluation - Properly handle delimited identifiers - Propagate the `permissive` option properly and through children() and descendants() by adding it to the context - Handle primitive Xhtml type - Fix IsCodeField() to return true for CodeType fields by checking annotation instead of suffix (i.e. DocumentReference_RelatesTo_CodeType) --- fhirpath/fhirpath_test.go | 35 +++++++++++++++++++++- fhirpath/internal/expr/context.go | 5 ++++ fhirpath/internal/expr/expressions.go | 8 +++-- fhirpath/internal/expr/expressions_test.go | 6 ++++ fhirpath/internal/funcs/impl/navigation.go | 2 +- fhirpath/internal/parser/visitor.go | 22 +++++++++----- fhirpath/system/types.go | 4 ++- fhirpath/system/types_test.go | 6 ++++ internal/fhir/elements_primitive.go | 9 ++++++ internal/fhir/elements_primitive_test.go | 10 +++++++ internal/protofields/fields.go | 9 +++--- 11 files changed, 100 insertions(+), 16 deletions(-) diff --git a/fhirpath/fhirpath_test.go b/fhirpath/fhirpath_test.go index 2a60de2..77637f5 100644 --- a/fhirpath/fhirpath_test.go +++ b/fhirpath/fhirpath_test.go @@ -47,7 +47,10 @@ type evaluateTestCase struct { var ( patientChu = &ppb.Patient{ - Id: fhir.ID("123"), + Id: fhir.ID("123"), + Text: &dtpb.Narrative{ + Div: &dtpb.Xhtml{Value: "patient chu record"}, + }, Active: fhir.Boolean(true), Gender: &ppb.Patient_GenderCode{ Value: cpb.AdministrativeGenderCode_FEMALE, @@ -135,6 +138,13 @@ var ( }, }, }, + RelatesTo: []*drpb.DocumentReference_RelatesTo{ + { + Code: &drpb.DocumentReference_RelatesTo_CodeType{ + Value: cpb.DocumentRelationshipTypeCode_APPENDS, + }, + }, + }, } questionnaireRef, _ = reference.Typed("Questionnaire", "1234") obsWithRef = &opb.Observation{ @@ -455,6 +465,12 @@ func TestEvaluate_PathSelection_ReturnsResult(t *testing.T) { inputCollection: []fhirpath.Resource{task}, wantCollection: system.Collection{system.String(fhirconv.DateTimeToString(end.ToProtoDateTime()))}, }, + { + name: "delimited identifier", + inputPath: "Patient.text.`div`", + inputCollection: []fhirpath.Resource{patientChu}, + wantCollection: system.Collection{fhir.Xhtml("patient chu record")}, + }, } testEvaluate(t, testCases) } @@ -1068,6 +1084,13 @@ func TestFunctionInvocation_Evaluates(t *testing.T) { inputCollection: []fhirpath.Resource{patientChu}, wantCollection: system.Collection{system.Boolean(false), system.Boolean(true)}, }, + { + name: "(legacy) filtering nested fields by field name", + inputPath: "descendants().family", + inputCollection: []fhirpath.Resource{patientChu}, + compileOptions: []fhirpath.CompileOption{compopts.Permissive()}, + wantCollection: system.Collection{patientChu.Name[0].Family, patientChu.Name[1].Family, patientChu.Contact[0].Name.Family}, + }, { name: "filters child fields with ofType()", inputPath: "children().ofType(string)", @@ -1215,6 +1238,12 @@ func TestTypeExpression_Evaluates(t *testing.T) { inputCollection: []fhirpath.Resource{}, wantCollection: system.Collection{system.MustParseDate("2000-12-05")}, }, + { + name: "passes through as code", + inputPath: "relatesTo.code as code", + inputCollection: []fhirpath.Resource{docRef}, + wantCollection: system.Collection{docRef.RelatesTo[0].Code}, + }, } testEvaluate(t, testCases) @@ -1425,6 +1454,10 @@ func TestCompile_ReturnsError(t *testing.T) { name: "resolving invalid type specifier", inputPath: "1 is System.Patient", }, + { + name: "reserved keyword not delimited", + inputPath: "Patient.text.div", + }, } for _, tc := range testCases { diff --git a/fhirpath/internal/expr/context.go b/fhirpath/internal/expr/context.go index 6f8b788..69df5d0 100644 --- a/fhirpath/internal/expr/context.go +++ b/fhirpath/internal/expr/context.go @@ -28,6 +28,10 @@ type Context struct { // Resolver is an optional mechanism for resolving FHIR Resources that // is used in the 'resolve()' FHIRPath function. Resolver resolver.Resolver + + // Permissive is a legacy option to allow FHIRpaths with *invalid* fields to be + // compiled (to reduce breakages). + Permissive bool } // Clone copies this Context object to produce a new instance. @@ -37,6 +41,7 @@ func (c *Context) Clone() *Context { ExternalConstants: c.ExternalConstants, LastResult: c.LastResult, Resolver: c.Resolver, + Permissive: c.Permissive, } } diff --git a/fhirpath/internal/expr/expressions.go b/fhirpath/internal/expr/expressions.go index e97b861..353ad9a 100644 --- a/fhirpath/internal/expr/expressions.go +++ b/fhirpath/internal/expr/expressions.go @@ -161,6 +161,9 @@ func (e *FieldExpression) Evaluate(ctx *Context, input system.Collection) (syste fieldName = fieldName + "_value" field = reflect.Descriptor().Fields().ByName(protoreflect.Name(fieldName)) if field == nil { + if e.Permissive { + continue + } return nil, fmt.Errorf("%w: %s not a field on %T", ErrInvalidField, fieldName, message) } } @@ -442,8 +445,9 @@ var _ Expression = (*EqualityExpression)(nil) // FunctionExpression enables evaluation of Function Invocation expressions. // It holds the function and function arguments. type FunctionExpression struct { - Fn func(*Context, system.Collection, ...Expression) (system.Collection, error) - Args []Expression + Fn func(*Context, system.Collection, ...Expression) (system.Collection, error) + Args []Expression + Permissive bool } // Evaluate evaluates the function with respect to its arguments. Returns the result diff --git a/fhirpath/internal/expr/expressions_test.go b/fhirpath/internal/expr/expressions_test.go index e905c15..54be64f 100644 --- a/fhirpath/internal/expr/expressions_test.go +++ b/fhirpath/internal/expr/expressions_test.go @@ -210,6 +210,12 @@ func TestFieldExpression_Gets_DesiredField(t *testing.T) { input: system.Collection{patientMissingName}, wantCollection: system.Collection{patientBirthDay}, }, + { + name: "(Legacy) input item doesn't have field", + fieldExp: &expr.FieldExpression{FieldName: "given", Permissive: true}, + input: system.Collection{patientFirstHumanName, patientContactPoint[0]}, + wantCollection: system.Collection{patientFirstHumanName.Given[0], patientFirstHumanName.Given[1]}, + }, { name: "(Legacy) input contains non-resource items", fieldExp: &expr.FieldExpression{FieldName: "birthDate", Permissive: true}, diff --git a/fhirpath/internal/funcs/impl/navigation.go b/fhirpath/internal/funcs/impl/navigation.go index cfd0f6f..c16e4e1 100644 --- a/fhirpath/internal/funcs/impl/navigation.go +++ b/fhirpath/internal/funcs/impl/navigation.go @@ -44,7 +44,7 @@ func Children(ctx *expr.Context, input system.Collection, args ...expr.Expressio } } for _, f := range fields { - fe := expr.FieldExpression{FieldName: f} + fe := expr.FieldExpression{FieldName: f, Permissive: ctx.Permissive} messages, err := fe.Evaluate(ctx, system.Collection{base}) if err != nil { return nil, err diff --git a/fhirpath/internal/parser/visitor.go b/fhirpath/internal/parser/visitor.go index a1b23a1..559eb19 100644 --- a/fhirpath/internal/parser/visitor.go +++ b/fhirpath/internal/parser/visitor.go @@ -17,10 +17,11 @@ import ( ) var ( - errNotSupported = errors.New("expression not currently supported") - errTooManyQualifiers = errors.New("too many type qualifiers") - errVisitingChildren = errors.New("error while visiting child expressions") - errUnresolvedFunction = errors.New("function identifier can't be resolved") + errNotSupported = errors.New("expression not currently supported") + errTooManyQualifiers = errors.New("too many type qualifiers") + errVisitingChildren = errors.New("error while visiting child expressions") + errUnresolvedFunction = errors.New("function identifier can't be resolved") + errInvalidDelimitedIdentifier = errors.New("invalid delimited identifier") ) type FHIRPathVisitor struct { @@ -421,6 +422,12 @@ func (v *FHIRPathVisitor) VisitExternalConstant(ctx *grammar.ExternalConstantCon // root of the expression. If so, it will return a TypeExpression. Otherwise, it returns a FieldExpression. func (v *FHIRPathVisitor) VisitMemberInvocation(ctx *grammar.MemberInvocationContext) interface{} { identifier := ctx.GetText() + if ctx.Identifier().DELIMITEDIDENTIFIER() != nil { + if len(identifier) < 2 || identifier[0] != '`' || identifier[len(identifier)-1] != '`' { + return &VisitResult{nil, fmt.Errorf("%w: %s", errInvalidDelimitedIdentifier, identifier)} + } + identifier = identifier[1 : len(identifier)-1] + } var expression expr.Expression if resource.IsType(identifier) && !v.visitedRoot { @@ -480,8 +487,9 @@ func (v *FHIRPathVisitor) VisitFunction(ctx *grammar.FunctionContext) interface{ return v.transformedVisitResult( &expr.FunctionExpression{ - Fn: fn.Func, - Args: []expr.Expression{&expr.TypeExpression{Type: typeSpecifier.String()}}, + Fn: fn.Func, + Args: []expr.Expression{&expr.TypeExpression{Type: typeSpecifier.String()}}, + Permissive: v.Permissive, }, ) } @@ -500,7 +508,7 @@ func (v *FHIRPathVisitor) VisitFunction(ctx *grammar.FunctionContext) interface{ if len(expressions) < fn.MinArity || len(expressions) > fn.MaxArity { return &VisitResult{nil, fmt.Errorf("%w: input arity outside of function arity bounds", impl.ErrWrongArity)} } - return v.transformedVisitResult(&expr.FunctionExpression{Fn: fn.Func, Args: expressions}) + return v.transformedVisitResult(&expr.FunctionExpression{Fn: fn.Func, Args: expressions, Permissive: v.Permissive}) } func (v *FHIRPathVisitor) VisitParamList(ctx *grammar.ParamListContext) interface{} { diff --git a/fhirpath/system/types.go b/fhirpath/system/types.go index a5aae33..31cb2d1 100644 --- a/fhirpath/system/types.go +++ b/fhirpath/system/types.go @@ -52,7 +52,7 @@ func IsPrimitive(input any) bool { switch v := input.(type) { case *dtpb.Boolean, *dtpb.String, *dtpb.Uri, *dtpb.Url, *dtpb.Canonical, *dtpb.Code, *dtpb.Oid, *dtpb.Id, *dtpb.Uuid, *dtpb.Markdown, *dtpb.Base64Binary, *dtpb.Integer, *dtpb.UnsignedInt, *dtpb.PositiveInt, *dtpb.Decimal, *dtpb.Date, - *dtpb.Time, *dtpb.DateTime, *dtpb.Instant, *dtpb.Quantity, Any: + *dtpb.Time, *dtpb.DateTime, *dtpb.Instant, *dtpb.Quantity, *dtpb.Xhtml, Any: return true case fhir.Base: return protofields.IsCodeField(v) @@ -94,6 +94,8 @@ func From(input any) (Any, error) { return Integer(v.Value), nil case *dtpb.PositiveInt: return Integer(v.Value), nil + case *dtpb.Xhtml: + return String(v.Value), nil case *dtpb.Decimal: value, err := decimal.NewFromString(v.Value) if err != nil { diff --git a/fhirpath/system/types_test.go b/fhirpath/system/types_test.go index d1089e4..c5df775 100644 --- a/fhirpath/system/types_test.go +++ b/fhirpath/system/types_test.go @@ -97,6 +97,12 @@ var testCases []testCase = []testCase{ want: system.String("aGVsbG8gd29ybGQ="), shouldCast: true, }, + { + name: "converts xhtml", + input: fhir.Xhtml("xhtml"), + want: system.String("xhtml"), + shouldCast: true, + }, { name: "converts integer", input: fhir.Integer(123), diff --git a/internal/fhir/elements_primitive.go b/internal/fhir/elements_primitive.go index 37fa5bf..79c0244 100644 --- a/internal/fhir/elements_primitive.go +++ b/internal/fhir/elements_primitive.go @@ -341,3 +341,12 @@ func UUID(value string) *dtpb.Uuid { func RandomUUID() *dtpb.Uuid { return UUID(uuid.NewString()) } + +// Xhtml creates an R4 FHIR XHTML element from a string value. +// +// See: https://hl7.org/fhir/R4/narrative.html#xhtml +func Xhtml(value string) *dtpb.Xhtml { + return &dtpb.Xhtml{ + Value: value, + } +} diff --git a/internal/fhir/elements_primitive_test.go b/internal/fhir/elements_primitive_test.go index 9375dc1..523bcc2 100644 --- a/internal/fhir/elements_primitive_test.go +++ b/internal/fhir/elements_primitive_test.go @@ -388,3 +388,13 @@ func TestURIFromUUID(t *testing.T) { }) } } + +func TestXhtml(t *testing.T) { + want := "xhtml" + + sut := fhir.Xhtml(want) + + if got := sut.GetValue(); !cmp.Equal(got, want) { + t.Errorf("Xhtml: got %v, want %v", got, want) + } +} diff --git a/internal/protofields/fields.go b/internal/protofields/fields.go index 5d2a097..1158365 100644 --- a/internal/protofields/fields.go +++ b/internal/protofields/fields.go @@ -1,8 +1,7 @@ package protofields import ( - "strings" - + apb "github.com/google/fhir/go/proto/google/fhir/proto/annotations_go_proto" dtpb "github.com/google/fhir/go/proto/google/fhir/proto/r4/core/datatypes_go_proto" bcrpb "github.com/google/fhir/go/proto/google/fhir/proto/r4/core/resources/bundle_and_contained_resource_go_proto" "github.com/iancoleman/strcase" @@ -103,12 +102,14 @@ func UnwrapOneofField(element proto.Message, fieldName string) proto.Message { // Codes with enum values and string values are both considered valid. func IsCodeField(message proto.Message) bool { reflect := message.ProtoReflect() - name := string(reflect.Descriptor().Name()) + if !proto.HasExtension(reflect.Descriptor().Options(), apb.E_FhirValuesetUrl) { + return false + } field := reflect.Descriptor().Fields().ByName(protoreflect.Name("value")) if field != nil { allowedKinds := []protoreflect.Kind{protoreflect.EnumKind, protoreflect.StringKind} isValidFieldType := slices.Includes(allowedKinds, field.Kind()) - return strings.HasSuffix(name, "Code") && isValidFieldType + return isValidFieldType } return false } From 5930775ea5f4ba27a32d07d735ace1b4a8aab081 Mon Sep 17 00:00:00 2001 From: Marwan Tammam Date: Mon, 3 Nov 2025 12:20:56 -0500 Subject: [PATCH 2/4] Fix a bug in bundleresolver's getLatestResource() --- fhirpath/resolver/bundleresolver.go | 2 +- fhirpath/resolver/bundleresolver_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fhirpath/resolver/bundleresolver.go b/fhirpath/resolver/bundleresolver.go index 63a9dbf..ef5ddf1 100644 --- a/fhirpath/resolver/bundleresolver.go +++ b/fhirpath/resolver/bundleresolver.go @@ -173,7 +173,7 @@ func getLatestResource(resources []fhir.Resource) (fhir.Resource, error) { return nil, ErrMissingMetaOrLastUpdated } resLastUpdated := res.GetMeta().GetLastUpdated().GetValueUs() - if resolvedLastUpdated > resLastUpdated { + if resolvedLastUpdated < resLastUpdated { resolvedResource = res resolvedLastUpdated = resLastUpdated } diff --git a/fhirpath/resolver/bundleresolver_test.go b/fhirpath/resolver/bundleresolver_test.go index c0e4779..a907b61 100644 --- a/fhirpath/resolver/bundleresolver_test.go +++ b/fhirpath/resolver/bundleresolver_test.go @@ -56,7 +56,7 @@ func TestBundleResolver_Resolve(t *testing.T) { patient123Latest := &ppb.Patient{ Id: fhir.ID("123"), Meta: &dtpb.Meta{ - LastUpdated: &dtpb.Instant{ValueUs: 500}, + LastUpdated: &dtpb.Instant{ValueUs: 5000}, }, } From 08a41e51e0ef6a6debc0e6526346e8a72851237e Mon Sep 17 00:00:00 2001 From: Marwan Tammam Date: Tue, 11 Nov 2025 10:39:23 -0500 Subject: [PATCH 3/4] address comments#1 --- fhirpath/fhirpath_test.go | 6 ++++++ fhirpath/internal/parser/visitor.go | 6 ++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/fhirpath/fhirpath_test.go b/fhirpath/fhirpath_test.go index ac1268d..d74c0c2 100644 --- a/fhirpath/fhirpath_test.go +++ b/fhirpath/fhirpath_test.go @@ -471,6 +471,12 @@ func TestEvaluate_PathSelection_ReturnsResult(t *testing.T) { inputCollection: []fhirpath.Resource{patientChu}, wantCollection: system.Collection{fhir.Xhtml("patient chu record")}, }, + { + name: "escaping backticks", + inputPath: "Patient.text.`div`.contains('\\`')", + inputCollection: []fhirpath.Resource{patientChu}, + wantCollection: system.Collection{system.Boolean(false)}, + }, } testEvaluate(t, testCases) } diff --git a/fhirpath/internal/parser/visitor.go b/fhirpath/internal/parser/visitor.go index 04f0304..656c672 100644 --- a/fhirpath/internal/parser/visitor.go +++ b/fhirpath/internal/parser/visitor.go @@ -445,10 +445,8 @@ func (v *FHIRPathVisitor) VisitExternalConstant(ctx *grammar.ExternalConstantCon func (v *FHIRPathVisitor) VisitMemberInvocation(ctx *grammar.MemberInvocationContext) interface{} { identifier := ctx.GetText() if ctx.Identifier().DELIMITEDIDENTIFIER() != nil { - if len(identifier) < 2 || identifier[0] != '`' || identifier[len(identifier)-1] != '`' { - return &VisitResult{nil, fmt.Errorf("%w: %s", errInvalidDelimitedIdentifier, identifier)} - } - identifier = identifier[1 : len(identifier)-1] + identifier = strings.TrimPrefix(identifier, "`") + identifier = strings.TrimSuffix(identifier, "`") } var expression expr.Expression From bfa750d5846941606aa1c76c5ca8d691b4f1fe27 Mon Sep 17 00:00:00 2001 From: Marwan Tammam Date: Wed, 12 Nov 2025 10:19:56 -0500 Subject: [PATCH 4/4] add skipunknownfields option --- fhirpath/compopts/compopts.go | 9 +++++ fhirpath/fhirpath.go | 5 ++- fhirpath/fhirpath_test.go | 4 +- fhirpath/internal/expr/context.go | 6 +++ fhirpath/internal/expr/expressions.go | 14 ++++--- fhirpath/internal/expr/expressions_test.go | 4 +- fhirpath/internal/funcs/impl/navigation.go | 2 +- fhirpath/internal/funcs/impl/projection.go | 2 +- .../internal/funcs/impl/projection_test.go | 6 +++ fhirpath/internal/opts/opts.go | 4 ++ fhirpath/internal/parser/visitor.go | 38 ++++++++++--------- 11 files changed, 62 insertions(+), 32 deletions(-) diff --git a/fhirpath/compopts/compopts.go b/fhirpath/compopts/compopts.go index 3e171bc..a8239f3 100644 --- a/fhirpath/compopts/compopts.go +++ b/fhirpath/compopts/compopts.go @@ -57,6 +57,15 @@ func Permissive() opts.CompileOption { }) } +// SkipUnknownFields is an option that allows for skipping unknown fields and returning +// an empty collection instead of throwing an error. +func SkipUnknownFields() opts.CompileOption { + return opts.Transform(func(cfg *opts.CompileConfig) error { + cfg.SkipUnknownFields = true + return nil + }) +} + // WithExperimentalFuncs is an option that enables experimental functions not // in the N1 Normative specification. func WithExperimentalFuncs() opts.CompileOption { diff --git a/fhirpath/fhirpath.go b/fhirpath/fhirpath.go index 8a0d43e..8eebed3 100644 --- a/fhirpath/fhirpath.go +++ b/fhirpath/fhirpath.go @@ -47,8 +47,9 @@ func Compile(expr string, options ...CompileOption) (*Expression, error) { } visitor := &parser.FHIRPathVisitor{ - Functions: config.Table, - Permissive: config.Permissive, + Functions: config.Table, + Permissive: config.Permissive, + SkipUnknownFields: config.SkipUnknownFields, } vr, ok := visitor.Visit(tree).(*parser.VisitResult) if !ok { diff --git a/fhirpath/fhirpath_test.go b/fhirpath/fhirpath_test.go index d74c0c2..70ee509 100644 --- a/fhirpath/fhirpath_test.go +++ b/fhirpath/fhirpath_test.go @@ -1091,10 +1091,10 @@ func TestFunctionInvocation_Evaluates(t *testing.T) { wantCollection: system.Collection{system.Boolean(false), system.Boolean(true)}, }, { - name: "(legacy) filtering nested fields by field name", + name: "filtering nested fields by field name", inputPath: "descendants().family", inputCollection: []fhirpath.Resource{patientChu}, - compileOptions: []fhirpath.CompileOption{compopts.Permissive()}, + compileOptions: []fhirpath.CompileOption{compopts.SkipUnknownFields()}, wantCollection: system.Collection{patientChu.Name[0].Family, patientChu.Name[1].Family, patientChu.Contact[0].Name.Family}, }, { diff --git a/fhirpath/internal/expr/context.go b/fhirpath/internal/expr/context.go index 21bb2cc..7d7983a 100644 --- a/fhirpath/internal/expr/context.go +++ b/fhirpath/internal/expr/context.go @@ -34,6 +34,11 @@ type Context struct { // Permissive is a legacy option to allow FHIRpaths with *invalid* fields to be // compiled (to reduce breakages). Permissive bool + + // SkipUnknownFields is an option that allows for skipping unknown fields and returning + // an empty collection instead of throwing an error. + SkipUnknownFields bool + // Service is an optional mechanism for providing a terminology service // which can be used to validate code in valueSet TermService terminology.Service @@ -70,6 +75,7 @@ func (c *Context) Clone() *Context { LastResult: c.LastResult, Resolver: c.Resolver, Permissive: c.Permissive, + SkipUnknownFields: c.SkipUnknownFields, TermService: c.TermService, GoContext: c.GoContext, } diff --git a/fhirpath/internal/expr/expressions.go b/fhirpath/internal/expr/expressions.go index 14d21f0..cb154d9 100644 --- a/fhirpath/internal/expr/expressions.go +++ b/fhirpath/internal/expr/expressions.go @@ -77,8 +77,9 @@ var _ Expression = (*IdentityExpression)(nil) // FieldExpression is the expression that accesses the specified // FieldName in the input collection. type FieldExpression struct { - FieldName string - Permissive bool + FieldName string + Permissive bool + SkipUnknownFields bool } // Evaluate filters the input collections by those that contain @@ -161,7 +162,7 @@ func (e *FieldExpression) Evaluate(ctx *Context, input system.Collection) (syste fieldName = fieldName + "_value" field = reflect.Descriptor().Fields().ByName(protoreflect.Name(fieldName)) if field == nil { - if e.Permissive { + if e.SkipUnknownFields { continue } return nil, fmt.Errorf("%w: %s not a field on %T", ErrInvalidField, fieldName, message) @@ -445,9 +446,10 @@ var _ Expression = (*EqualityExpression)(nil) // FunctionExpression enables evaluation of Function Invocation expressions. // It holds the function and function arguments. type FunctionExpression struct { - Fn func(*Context, system.Collection, ...Expression) (system.Collection, error) - Args []Expression - Permissive bool + Fn func(*Context, system.Collection, ...Expression) (system.Collection, error) + Args []Expression + Permissive bool + SkipUnknownFields bool } // Evaluate evaluates the function with respect to its arguments. Returns the result diff --git a/fhirpath/internal/expr/expressions_test.go b/fhirpath/internal/expr/expressions_test.go index 8ba6d60..487a56c 100644 --- a/fhirpath/internal/expr/expressions_test.go +++ b/fhirpath/internal/expr/expressions_test.go @@ -211,8 +211,8 @@ func TestFieldExpression_Gets_DesiredField(t *testing.T) { wantCollection: system.Collection{patientBirthDay}, }, { - name: "(Legacy) input item doesn't have field", - fieldExp: &expr.FieldExpression{FieldName: "given", Permissive: true}, + name: "input item doesn't have field - skipped", + fieldExp: &expr.FieldExpression{FieldName: "given", SkipUnknownFields: true}, input: system.Collection{patientFirstHumanName, patientContactPoint[0]}, wantCollection: system.Collection{patientFirstHumanName.Given[0], patientFirstHumanName.Given[1]}, }, diff --git a/fhirpath/internal/funcs/impl/navigation.go b/fhirpath/internal/funcs/impl/navigation.go index c16e4e1..69956e8 100644 --- a/fhirpath/internal/funcs/impl/navigation.go +++ b/fhirpath/internal/funcs/impl/navigation.go @@ -44,7 +44,7 @@ func Children(ctx *expr.Context, input system.Collection, args ...expr.Expressio } } for _, f := range fields { - fe := expr.FieldExpression{FieldName: f, Permissive: ctx.Permissive} + fe := expr.FieldExpression{FieldName: f, Permissive: ctx.Permissive, SkipUnknownFields: ctx.SkipUnknownFields} messages, err := fe.Evaluate(ctx, system.Collection{base}) if err != nil { return nil, err diff --git a/fhirpath/internal/funcs/impl/projection.go b/fhirpath/internal/funcs/impl/projection.go index 9d13e74..b677457 100644 --- a/fhirpath/internal/funcs/impl/projection.go +++ b/fhirpath/internal/funcs/impl/projection.go @@ -31,7 +31,7 @@ func Select(ctx *expr.Context, input system.Collection, args ...expr.Expression) result = append(result, output...) } // Raise field errors if one was raised for each input. - if len(input) > 0 && len(fieldErrs) == len(input) { + if len(input) > 0 && len(fieldErrs) == len(input) && !ctx.SkipUnknownFields { return nil, errors.Join(fieldErrs...) } return result, nil diff --git a/fhirpath/internal/funcs/impl/projection_test.go b/fhirpath/internal/funcs/impl/projection_test.go index 0a94552..6c91ed0 100644 --- a/fhirpath/internal/funcs/impl/projection_test.go +++ b/fhirpath/internal/funcs/impl/projection_test.go @@ -74,6 +74,12 @@ func TestSelect_Evaluates(t *testing.T) { inputArgs: []expr.Expression{&expr.FieldExpression{FieldName: "state"}}, wantCollection: system.Collection{address[0].GetState()}, }, + { + name: "does not raise error if all fields are invalid but SkipUnknownFields is true", + inputCollection: system.Collection{address[0], fhir.String("string")}, + inputArgs: []expr.Expression{&expr.FieldExpression{FieldName: "foo", SkipUnknownFields: true}}, + wantCollection: system.Collection{}, + }, } for _, tc := range testCases { diff --git a/fhirpath/internal/opts/opts.go b/fhirpath/internal/opts/opts.go index 5a39f05..93e3d54 100644 --- a/fhirpath/internal/opts/opts.go +++ b/fhirpath/internal/opts/opts.go @@ -23,6 +23,10 @@ type CompileConfig struct { // Permissive is a legacy option to allow FHIRpaths with *invalid* fields to be // compiled (to reduce breakages). Permissive bool + + // SkipUnknownFields is an option that allows for skipping unknown fields and returning + // an empty collection instead of throwing an error. + SkipUnknownFields bool } // EvaluateConfig provides the configuration values for the Evaluate command. diff --git a/fhirpath/internal/parser/visitor.go b/fhirpath/internal/parser/visitor.go index 656c672..f335071 100644 --- a/fhirpath/internal/parser/visitor.go +++ b/fhirpath/internal/parser/visitor.go @@ -17,19 +17,19 @@ import ( ) var ( - errNotSupported = errors.New("expression not currently supported") - errTooManyQualifiers = errors.New("too many type qualifiers") - errVisitingChildren = errors.New("error while visiting child expressions") - errUnresolvedFunction = errors.New("function identifier can't be resolved") - errInvalidDelimitedIdentifier = errors.New("invalid delimited identifier") + errNotSupported = errors.New("expression not currently supported") + errTooManyQualifiers = errors.New("too many type qualifiers") + errVisitingChildren = errors.New("error while visiting child expressions") + errUnresolvedFunction = errors.New("function identifier can't be resolved") ) type FHIRPathVisitor struct { *grammar.BasefhirpathVisitor - visitedRoot bool - Functions funcs.FunctionTable - Transform VisitorTransform - Permissive bool + visitedRoot bool + Functions funcs.FunctionTable + Transform VisitorTransform + Permissive bool + SkipUnknownFields bool } type VisitResult struct { @@ -45,10 +45,11 @@ type typeResult struct { // clone produces a shallow-clone of the visitor, to be used when visiting sub-expressions. func (v *FHIRPathVisitor) clone() *FHIRPathVisitor { return &FHIRPathVisitor{ - Functions: v.Functions, - Transform: v.Transform, - Permissive: v.Permissive, - visitedRoot: false, + Functions: v.Functions, + Transform: v.Transform, + Permissive: v.Permissive, + SkipUnknownFields: v.SkipUnknownFields, + visitedRoot: false, } } @@ -454,7 +455,7 @@ func (v *FHIRPathVisitor) VisitMemberInvocation(ctx *grammar.MemberInvocationCon expression = &expr.TypeExpression{Type: identifier} v.visitedRoot = true } else { - expression = &expr.FieldExpression{FieldName: identifier, Permissive: v.Permissive} + expression = &expr.FieldExpression{FieldName: identifier, Permissive: v.Permissive, SkipUnknownFields: v.SkipUnknownFields} } return v.transformedVisitResult(expression) @@ -507,9 +508,10 @@ func (v *FHIRPathVisitor) VisitFunction(ctx *grammar.FunctionContext) interface{ return v.transformedVisitResult( &expr.FunctionExpression{ - Fn: fn.Func, - Args: []expr.Expression{&expr.TypeExpression{Type: typeSpecifier.String()}}, - Permissive: v.Permissive, + Fn: fn.Func, + Args: []expr.Expression{&expr.TypeExpression{Type: typeSpecifier.String()}}, + Permissive: v.Permissive, + SkipUnknownFields: v.SkipUnknownFields, }, ) } @@ -528,7 +530,7 @@ func (v *FHIRPathVisitor) VisitFunction(ctx *grammar.FunctionContext) interface{ if len(expressions) < fn.MinArity || len(expressions) > fn.MaxArity { return &VisitResult{nil, fmt.Errorf("%w: input arity outside of function arity bounds", impl.ErrWrongArity)} } - return v.transformedVisitResult(&expr.FunctionExpression{Fn: fn.Func, Args: expressions, Permissive: v.Permissive}) + return v.transformedVisitResult(&expr.FunctionExpression{Fn: fn.Func, Args: expressions, Permissive: v.Permissive, SkipUnknownFields: v.SkipUnknownFields}) } func (v *FHIRPathVisitor) VisitParamList(ctx *grammar.ParamListContext) interface{} {