From e524a404062f86618ec3740870fb74caa0b97641 Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 9 Sep 2024 16:37:17 -0700 Subject: [PATCH] Support implicit conversion of FHIR.Period to functions that accept Interval, but would therefore accept. This fixes a big where the implicit conversion logic would only succeed for FHIR.Period -> Interval conversions, so there would be no match for `start of encounter.period` because the `start of` function was looking for Interval. PiperOrigin-RevId: 672716681 --- internal/convert/convert.go | 2 +- internal/convert/convert_test.go | 15 +++++++++ internal/modelinfo/modelinfo.go | 20 +++++++++++- internal/modelinfo/modelinfo_test.go | 19 +++++++++-- parser/query_test.go | 47 ++++++++++++++++++++++++++-- tests/enginetests/query_test.go | 28 +++++++++++++++++ 6 files changed, 125 insertions(+), 6 deletions(-) diff --git a/internal/convert/convert.go b/internal/convert/convert.go index 169c424..db9ed3e 100644 --- a/internal/convert/convert.go +++ b/internal/convert/convert.go @@ -324,7 +324,7 @@ func OperandImplicitConverter(invokedType types.IType, declaredType types.IType, LibraryName: res.Library, Name: res.Function, Operands: []model.IExpression{opToWrap}, - Expression: model.ResultType(declaredType), + Expression: model.ResultType(res.OutputType), } score := implicitConversionScore(declaredType) diff --git a/internal/convert/convert_test.go b/internal/convert/convert_test.go index a86cd43..ad67065 100644 --- a/internal/convert/convert_test.go +++ b/internal/convert/convert_test.go @@ -566,6 +566,21 @@ func TestOperandImplicitConverter(t *testing.T) { }, }, }, + { + name: "FHIR ModelInfo Conversion to target that accepts Interval", + invokedType: &types.Named{TypeName: "FHIR.Period"}, + declaredType: &types.Interval{PointType: types.Any}, + want: ConvertedOperand{ + Matched: true, + Score: 5, + WrappedOperand: &model.FunctionRef{ + LibraryName: "FHIRHelpers", + Name: "ToInterval", + Expression: model.ResultType(&types.Interval{PointType: types.DateTime}), + Operands: []model.IExpression{model.NewLiteral("operand", types.String)}, + }, + }, + }, { name: "System ModelInfo Conversion to Class", invokedType: types.Integer, diff --git a/internal/modelinfo/modelinfo.go b/internal/modelinfo/modelinfo.go index b540fd7..3385ce1 100644 --- a/internal/modelinfo/modelinfo.go +++ b/internal/modelinfo/modelinfo.go @@ -250,6 +250,9 @@ type Convertible struct { // ex FHIRHelpers.ToString. Library string Function string + // OutputType defines the result type of the conversion. This can be a compatible subtype of the + // declared type, e.g., a declared Interval can accept an Interval. + OutputType types.IType } // IsImplicitlyConvertible uses model info conversionInfo to determine if one type can be converted @@ -277,7 +280,12 @@ func (m *ModelInfos) IsImplicitlyConvertible(from, to types.IType) (Convertible, if len(splitNames) != 2 { return Convertible{}, fmt.Errorf("invalid conversion function name %v", ci.FunctionName) } - return Convertible{IsConvertible: true, Library: splitNames[0], Function: splitNames[1]}, nil + return Convertible{ + IsConvertible: true, + Library: splitNames[0], + Function: splitNames[1], + OutputType: typeSpecifierFromElementType(ci.ToType), + }, nil } // BaseTypes returns all of the BaseTypes (aka Parents) of a type excluding Any (or for nested types @@ -614,6 +622,11 @@ func load(miXML *modelInfoXML) (*modelInfo, error) { for _, ci := range miXML.ConversionInfos { mi.conversionMap[conversionKey{fromType: ci.FromType, toType: ci.ToType}] = ci + + // TODO(b/317008490): Find a more robust approach to getting all compatible conversion targets. + if strings.HasPrefix(ci.ToType, "Interval") { + mi.conversionMap[conversionKey{fromType: ci.FromType, toType: "Interval"}] = ci + } } return mi, nil @@ -706,5 +719,10 @@ func typeSpecifierFromElementType(modelInfoType string) types.IType { return types.ToSystem(modelInfoType) } + // TODO(b/317008490): Do we need to handle other container types, e.g. List<>? + if strings.HasPrefix(modelInfoType, "Interval<") && strings.HasSuffix(modelInfoType, ">") { + pointType := strings.TrimPrefix(strings.TrimSuffix(modelInfoType, ">"), "Interval<") + return &types.Interval{PointType: types.ToSystem(pointType)} + } return &types.Named{TypeName: modelInfoType} } diff --git a/internal/modelinfo/modelinfo_test.go b/internal/modelinfo/modelinfo_test.go index 9d679f1..be35987 100644 --- a/internal/modelinfo/modelinfo_test.go +++ b/internal/modelinfo/modelinfo_test.go @@ -263,6 +263,7 @@ func TestIsImplicitlyConvertible(t *testing.T) { IsConvertible: true, Library: "SYSTEM", Function: "ToDecimal", + OutputType: types.Decimal, }, }, { @@ -273,6 +274,7 @@ func TestIsImplicitlyConvertible(t *testing.T) { IsConvertible: true, Library: "SYSTEM", Function: "ToDateTime", + OutputType: types.DateTime, }, }, { @@ -283,6 +285,7 @@ func TestIsImplicitlyConvertible(t *testing.T) { IsConvertible: true, Library: "FHIRHelpers", Function: "ToCode", + OutputType: types.Code, }, }, { @@ -293,6 +296,18 @@ func TestIsImplicitlyConvertible(t *testing.T) { IsConvertible: true, Library: "FHIRHelpers", Function: "ToInterval", + OutputType: &types.Interval{PointType: types.DateTime}, + }, + }, + { + name: "FHIR.Period -> Interval", + from: &types.Named{TypeName: "FHIR.Period"}, + to: &types.Interval{PointType: types.Any}, + want: Convertible{ + IsConvertible: true, + Library: "FHIRHelpers", + Function: "ToInterval", + OutputType: &types.Interval{PointType: types.DateTime}, }, }, } @@ -305,8 +320,8 @@ func TestIsImplicitlyConvertible(t *testing.T) { if err != nil { t.Fatalf("IsImplicitlyConvertible(%s, %s) failed unexpectedly: %v", tc.from, tc.to, err) } - if got != tc.want { - t.Errorf("IsImplicitlyConvertible(%s, %s) got: %v, want: %v", tc.from, tc.to, got, tc.want) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("IsImplicitlyConvertible(%s, %s): %s", tc.from, tc.to, diff) } }) } diff --git a/parser/query_test.go b/parser/query_test.go index 3b7b4a4..6e4c645 100644 --- a/parser/query_test.go +++ b/parser/query_test.go @@ -429,7 +429,7 @@ func TestQuery(t *testing.T) { { name: "Relationship and Aggregate have the same alias name", cql: dedent.Dedent(` - define TESTRESULT: ({1, 2, 3}) N + define TESTRESULT: ({1, 2, 3}) N with ({4, 5}) R such that R = N aggregate R starting 1: R * N`), want: &model.Query{ @@ -500,6 +500,46 @@ func TestQuery(t *testing.T) { }, }, }, + { + name: "Query start with implicit interval", + cql: `define TESTRESULT: [Encounter] E return start of E.period`, + want: &model.Query{ + Expression: model.ResultType(&types.List{ElementType: types.DateTime}), + Source: []*model.AliasedSource{ + { + Alias: "E", + Source: &model.Retrieve{ + Expression: model.ResultType(&types.List{ElementType: &types.Named{TypeName: "FHIR.Encounter"}}), + DataType: "{http://hl7.org/fhir}Encounter", + TemplateID: "http://hl7.org/fhir/StructureDefinition/Encounter", + CodeProperty: "type", + }, + Expression: model.ResultType(&types.List{ElementType: &types.Named{TypeName: "FHIR.Encounter"}}), + }, + }, + Return: &model.ReturnClause{ + Element: &model.Element{ResultType: types.DateTime}, + Expression: &model.Start{ + UnaryExpression: &model.UnaryExpression{ + Expression: model.ResultType(types.DateTime), + Operand: &model.FunctionRef{ + Expression: model.ResultType(&types.Interval{PointType: types.DateTime}), + Name: "ToInterval", + LibraryName: "FHIRHelpers", + Operands: []model.IExpression{ + &model.Property{ + Expression: model.ResultType(&types.Named{TypeName: "FHIR.Period"}), + Source: &model.AliasRef{Name: "E", Expression: model.ResultType(&types.Named{TypeName: "FHIR.Encounter"})}, + Path: "period", + }, + }, + }, + }, + }, + Distinct: true, + }, + }, + }, { name: "Query with All Return", cql: `define TESTRESULT: [Observation] o return all 4`, @@ -580,10 +620,13 @@ func TestQuery(t *testing.T) { cqlLib := dedent.Dedent(fmt.Sprintf(` valueset "Blood pressure": 'https://test/file1' using FHIR version '4.0.1' + include FHIRHelpers version '4.0.1' called FHIRHelpers context Patient %v`, test.cql)) - parsedLibs, err := newFHIRParser(t).Libraries(context.Background(), []string{cqlLib}, Config{}) + libs := addFHIRHelpersLib(t, cqlLib) + + parsedLibs, err := newFHIRParser(t).Libraries(context.Background(), libs, Config{}) if err != nil { t.Fatalf("Parse returned unexpected error: %v", err) } diff --git a/tests/enginetests/query_test.go b/tests/enginetests/query_test.go index 956b67e..c363aef 100644 --- a/tests/enginetests/query_test.go +++ b/tests/enginetests/query_test.go @@ -151,6 +151,34 @@ func TestQuery(t *testing.T) { define TESTRESULT: [Observation] O where null`), wantResult: newOrFatal(t, result.List{Value: []result.Value{}, StaticType: &types.List{ElementType: &types.Named{TypeName: "FHIR.Observation"}}}), }, + { + name: "Where filters by date", + cql: dedent.Dedent(` + using FHIR version '4.0.1' + include FHIRHelpers version '4.0.1' called FHIRHelpers + + define TESTRESULT: [Encounter] E where start of E.period < @2022-01-01 + `), + wantResult: newOrFatal(t, result.List{ + Value: []result.Value{ + newOrFatal(t, result.Named{Value: RetrieveFHIRResource(t, "Encounter", "1"), RuntimeType: &types.Named{TypeName: "FHIR.Encounter"}}), + }, + StaticType: &types.List{ElementType: &types.Named{TypeName: "FHIR.Encounter"}}, + }), + }, + { + name: "Where filters by all results", + cql: dedent.Dedent(` + using FHIR version '4.0.1' + include FHIRHelpers version '4.0.1' called FHIRHelpers + + define TESTRESULT: [Encounter] E where start of E.period < @1999-01-01 + `), + wantResult: newOrFatal(t, result.List{ + Value: []result.Value{}, + StaticType: &types.List{ElementType: &types.Named{TypeName: "FHIR.Encounter"}}, + }), + }, { name: "Query retrieves empy list", cql: dedent.Dedent(`