Skip to content

Commit

Permalink
Support implicit conversion of FHIR.Period to functions that accept I…
Browse files Browse the repository at this point in the history 
…nterval<Any>, but would therefore accept.

This fixes a big where the implicit conversion logic would only succeed for FHIR.Period -> Interval<System.DateTime> conversions, so there would be no match for `start of encounter.period` because the `start of` function was looking for Interval<System.Any>.

PiperOrigin-RevId: 676466662
  • Loading branch information
@copybara-github
Googler authored and copybara-github committed Sep 19, 2024
1 parent 598656f commit def266a
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 6 deletions.
2 changes: 1 addition & 1 deletion internal/convert/convert.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
15 changes: 15 additions & 0 deletions internal/convert/convert_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,21 @@ func TestOperandImplicitConverter(t *testing.T) {
},
},
},
{
name: "FHIR ModelInfo Conversion to target that accepts Interval<Any>",
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,
Expand Down
20 changes: 19 additions & 1 deletion internal/modelinfo/modelinfo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Any> can accept an Interval<DateTime>.
OutputType types.IType
}

// IsImplicitlyConvertible uses model info conversionInfo to determine if one type can be converted
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<System.Any>"}] = ci
}
}

return mi, nil
Expand Down Expand Up @@ -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}
}
19 changes: 17 additions & 2 deletions internal/modelinfo/modelinfo_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ func TestIsImplicitlyConvertible(t *testing.T) {
IsConvertible: true,
Library: "SYSTEM",
Function: "ToDecimal",
OutputType: types.Decimal,
},
},
{
Expand All @@ -273,6 +274,7 @@ func TestIsImplicitlyConvertible(t *testing.T) {
IsConvertible: true,
Library: "SYSTEM",
Function: "ToDateTime",
OutputType: types.DateTime,
},
},
{
Expand All @@ -283,6 +285,7 @@ func TestIsImplicitlyConvertible(t *testing.T) {
IsConvertible: true,
Library: "FHIRHelpers",
Function: "ToCode",
OutputType: types.Code,
},
},
{
Expand All @@ -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<System.Any>",
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},
},
},
}
Expand All @@ -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)
}
})
}
Expand Down
47 changes: 45 additions & 2 deletions parser/query_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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`,
Expand Down Expand Up @@ -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)
}
Expand Down
28 changes: 28 additions & 0 deletions tests/enginetests/query_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(`
Expand Down

0 comments on commit def266a

Please sign in to comment.