Skip to content

Commit

Permalink
Can now include libraries without specifying a version when the libra…
Browse files Browse the repository at this point in the history 
…ry itself has a version.

#44

PiperOrigin-RevId: 650878420
  • Loading branch information
@evan-gordon @copybara-github
evan-gordon authored and copybara-github committed Jul 10, 2024
1 parent 0fad5f7 commit b5e79e6
Show file tree
Hide file tree
Showing 4 changed files with 245 additions and 39 deletions.
22 changes: 19 additions & 3 deletions internal/reference/reference.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package reference
import (
"errors"
"fmt"
"strings"

"github.com/google/cql/internal/convert"
"github.com/google/cql/internal/modelinfo"
Expand Down Expand Up @@ -138,9 +139,24 @@ func (r *Resolver[T, F]) IncludeLibrary(m *model.LibraryIdentifier, validateIsUn
}
}

lib := namedLibKey{qualified: m.Qualified, version: m.Version}
if _, ok := r.libs[lib]; !ok {
return fmt.Errorf("library %s %s was included, but does not exist", m.Qualified, m.Version)
libToInclude := namedLibKey{qualified: m.Qualified, version: m.Version}
if _, ok := r.libs[libToInclude]; !ok {
// If the library is not found, and the version is not specified, attempt to use the latest
// version. This mimics the behavior found in the parser's topologicalSortLibraries function.
if libToInclude.version == "" {
for libKey := range r.libs {
if libKey.qualified != libToInclude.qualified {
continue
}
if strings.Compare(libToInclude.version, libKey.version) == -1 {
libToInclude.version = libKey.version
m.Version = libKey.version
}
}
}
if _, ok := r.libs[libToInclude]; !ok {
return fmt.Errorf("library %s %s was included, but does not exist", m.Qualified, m.Version)
}
}

r.includedLibs[includeKey{localID: m.Local, includedBy: r.currLib}] = m
Expand Down
244 changes: 208 additions & 36 deletions parser/library_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ package parser
import (
"context"
"errors"
"slices"
"strings"
"testing"

"github.com/google/cql/model"
"github.com/google/cql/result"
"github.com/google/cql/types"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/lithammer/dedent"
)

Expand Down Expand Up @@ -264,16 +264,6 @@ func TestMalformedCQLSingleLibrary(t *testing.T) {
}
}

func TestNoLibraries(t *testing.T) {
_, err := newFHIRParser(t).Libraries(context.Background(), []string{}, Config{})
if err == nil {
t.Fatal("Parse succeeded, wanted error")
}
want := "no CQL libraries were provided"
if !strings.Contains(err.Error(), want) {
t.Errorf("Returned error (%s) did not contain expected string (%s)", err.Error(), want)
}
}
func TestMalformedCQLMultipleLibraries(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -345,6 +335,20 @@ func TestMalformedIncludeDependenciesMultipleLibraries(t *testing.T) {
},
wantErr: "found circular dependencies",
},
{
name: "library includes non-existent library",
cqlLibs: []string{
"include lib1",
},
wantErr: "lib1 does not exist",
},
{
name: "library includes non-existent library with version specified",
cqlLibs: []string{
"include lib1 version '1.0'",
},
wantErr: "lib1 1.0 does not exist",
},
{
name: "repeated library identifier",
cqlLibs: []string{
Expand Down Expand Up @@ -408,6 +412,17 @@ func TestMalformedIncludeDependenciesMultipleLibraries(t *testing.T) {
}
}

func TestNoLibraries(t *testing.T) {
_, err := newFHIRParser(t).Libraries(context.Background(), []string{}, Config{})
if err == nil {
t.Fatal("Parse succeeded, wanted error")
}
want := "no CQL libraries were provided"
if !strings.Contains(err.Error(), want) {
t.Errorf("Returned error (%s) did not contain expected string (%s)", err.Error(), want)
}
}

func TestParserTopologicalSortMultipleLibraries(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -507,6 +522,24 @@ func TestParserTopologicalSortMultipleLibraries(t *testing.T) {
},
},
},
{
name: "Can include a versioned library without specifying the version",
cqlLibs: []string{
"library lib1 version '1.0'",
"include lib1",
},
want: []*model.Library{
&model.Library{
Identifier: &model.LibraryIdentifier{Local: "lib1", Qualified: "lib1", Version: "1.0"},
},
&model.Library{
// Unnamed Library
Includes: []*model.Include{
{Identifier: &model.LibraryIdentifier{Local: "lib1", Qualified: "lib1", Version: "1.0"}},
},
},
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
Expand All @@ -522,36 +555,175 @@ func TestParserTopologicalSortMultipleLibraries(t *testing.T) {
}

func TestParserTopologicalSortMultipleTopLevelLibraries(t *testing.T) {
cqlLibs := []string{
dedent.Dedent(`
library lib3
include lib2
include lib1`),
dedent.Dedent(`
library measure1
include lib1
include lib2
include lib3`),
dedent.Dedent(`
library measure2
include lib3`),
"library lib2",
"library lib1",
tests := []struct {
name string
cqlLibs []string
wantTopLevelLibs []model.LibraryIdentifier
}{
{
name: "Simple case with multiple top level libraries",
cqlLibs: []string{
dedent.Dedent(`
library lib3
include lib2
include lib1`),
dedent.Dedent(`
library measure1
include lib1
include lib2
include lib3`),
dedent.Dedent(`
library measure2
include lib3`),
"library lib2",
"library lib1",
},
wantTopLevelLibs: []model.LibraryIdentifier{
{
Qualified: "measure1",
Local: "measure1",
Version: "",
},
{
Qualified: "measure2",
Local: "measure2",
Version: "",
},
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
parsedLibs, err := newFHIRParser(t).Libraries(context.Background(), test.cqlLibs, Config{})
if err != nil {
t.Fatalf("Parse returned unexpected error: %v", err)
}

parsedLibs, err := newFHIRParser(t).Libraries(context.Background(), cqlLibs, Config{})
if err != nil {
t.Fatalf("Parse returned unexpected error: %v", err)
lastTwoIDs := []model.LibraryIdentifier{
*parsedLibs[len(parsedLibs)-1].Identifier,
*parsedLibs[len(parsedLibs)-2].Identifier,
}
slices.SortStableFunc(lastTwoIDs, func(a, b model.LibraryIdentifier) int {
if a.Qualified != b.Qualified {
return strings.Compare(a.Qualified, b.Qualified)
}
return strings.Compare(a.Version, b.Version)
})
// Topological sort isn't 100% deterministic so assert the last values are what we want.
if diff := cmp.Diff(test.wantTopLevelLibs, lastTwoIDs); diff != "" {
t.Errorf("%v\nLibraries(%v) parsing diff (-want +got):\n%v", test.wantTopLevelLibs, lastTwoIDs, diff)
}
})
}
}

lastTwoIDs := []string{
parsedLibs[len(parsedLibs)-1].Identifier.Qualified,
parsedLibs[len(parsedLibs)-2].Identifier.Qualified,
func TestParserTopologicalSortLibrariesIncludeCorrectVersion(t *testing.T) {
tests := []struct {
name string
cqlLibs []string
wantTopLevelLibs []*model.Library
}{
{
name: "Can parse includes with no version specified and multiple versions for the same library",
cqlLibs: []string{
"library lib1 version '1.0'",
"library lib1 version '1.2'",
dedent.Dedent(`
library measure1
include lib1`),
},
wantTopLevelLibs: []*model.Library{
&model.Library{
Identifier: &model.LibraryIdentifier{
Qualified: "lib1",
Local: "lib1",
Version: "1.0",
},
},
&model.Library{
Identifier: &model.LibraryIdentifier{
Qualified: "lib1",
Local: "lib1",
Version: "1.2",
},
},
&model.Library{
Identifier: &model.LibraryIdentifier{
Local: "measure1",
Qualified: "measure1",
Version: "",
},
Includes: []*model.Include{
{
Identifier: &model.LibraryIdentifier{
Local: "lib1",
Qualified: "lib1",
Version: "1.2",
},
},
},
},
},
},
{
name: "Can parse includes with no version specified and multiple versions for the same library where one has no version.",
cqlLibs: []string{
"library lib1 version '1.0'",
"library lib1",
dedent.Dedent(`
library measure1
include lib1`),
},
wantTopLevelLibs: []*model.Library{
&model.Library{
Identifier: &model.LibraryIdentifier{
Qualified: "lib1",
Local: "lib1",
},
},
&model.Library{
Identifier: &model.LibraryIdentifier{
Qualified: "lib1",
Local: "lib1",
Version: "1.0",
},
},
&model.Library{
Identifier: &model.LibraryIdentifier{
Local: "measure1",
Qualified: "measure1",
Version: "",
},
Includes: []*model.Include{
{
Identifier: &model.LibraryIdentifier{
Local: "lib1",
Qualified: "lib1",
},
},
},
},
},
},
}
measureIDs := []string{"measure1", "measure2"}
// Topological sort isn't 100% deterministic so assert the last values are what we want.
if diff := cmp.Diff(measureIDs, lastTwoIDs, cmpopts.SortSlices(func(a, b string) bool { return a < b })); diff != "" {
t.Errorf("%v\nLibraries(%v) parsing diff (-want +got):\n%v", measureIDs, lastTwoIDs, diff)
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
parsedLibs, err := newFHIRParser(t).Libraries(context.Background(), test.cqlLibs, Config{})
if err != nil {
t.Fatalf("Parse returned unexpected error: %v", err)
}

// Topological sort isn't 100% deterministic so we need to sort the libraries.
slices.SortStableFunc(parsedLibs, func(a, b *model.Library) int {
if a.Identifier.Qualified != b.Identifier.Qualified {
return strings.Compare(a.Identifier.Qualified, b.Identifier.Qualified)
}
return strings.Compare(a.Identifier.Version, b.Identifier.Version)
})
if diff := cmp.Diff(test.wantTopLevelLibs, parsedLibs); diff != "" {
t.Errorf("%v\nLibraries(%v) parsing diff (-want +got):\n%v", test.wantTopLevelLibs, parsedLibs, diff)
}
})
}
}

Expand Down
15 changes: 15 additions & 0 deletions parser/parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ type lexedLib struct {

// topologicalSortLibraries parses the CQL libraries into ANTLR, topologically sorts their
// dependencies and returns a sorted list of lexedLib.
// Note: In cases where a library is included without a version, it attempts to find the latest
// version of the library with a string value comparison. This is a naive approach and may not work
// with non numerical versioning systems.
func (p *Parser) topologicalSortLibraries(cqlLibs []string) ([]lexedLib, error) {
// lexedLibraries maps graph ID to library that has been lexed.
lexedLibraries := make(map[string]lexedLib, len(cqlLibs))
Expand Down Expand Up @@ -149,6 +152,18 @@ func (p *Parser) topologicalSortLibraries(cqlLibs []string) ([]lexedLib, error)
for libID, deps := range includeDependencies {
libNode := goraph.NewNode(libID.Key())
for _, includedID := range deps {
// If the version is not specified, use the latest version. This mimics the behavior found in
// the reference resolver.
if includedID.Version == "" {
for libKey := range includeDependencies {
if libKey.Name != includedID.Name {
continue
}
if strings.Compare(includedID.Version, libKey.Version) == -1 {
includedID = libKey
}
}
}
includedNode := goraph.NewNode(includedID.Key())
if err := graph.AddEdge(includedNode.ID(), libNode.ID(), 1); err != nil {
return nil, fmt.Errorf("failed to import library %q, dependency graph could not resolve with error: %w", includedID, err)
Expand Down
3 changes: 3 additions & 0 deletions result/result.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ func LibKeyFromModel(lib *model.LibraryIdentifier) LibKey {
// Key returns a unique string key representation of the LibKey.
// A space is added between the name and version to avoid naming clashes.
func (l LibKey) Key() string {
if l.Version == "" {
return l.Name
}
// TODO b/301606416 - Since identifiers can contain spaces, this is not a unique key.
return l.Name + " " + l.Version
}
Expand Down

0 comments on commit b5e79e6

Please sign in to comment.