Skip to content

Commit

Permalink
Working through test cases
Browse files Browse the repository at this point in the history
There are still gaps to be found based on various combinations of crazy references.

Signed-off-by: quobix <dave@quobix.com>
  • Loading branch information
daveshanley committed Oct 30, 2023
1 parent 3ee631c commit d8dfafd
Show file tree
Hide file tree
Showing 9 changed files with 167 additions and 39 deletions.
2 changes: 1 addition & 1 deletion document_examples_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func ExampleNewDocument_fromWithDocumentConfigurationFailure() {
if len(errors) > 0 {
fmt.Println("Error building Digital Ocean spec errors reported")
}
// Output: There are 474 errors logged
// Output: There are 475 errors logged
//Error building Digital Ocean spec errors reported
}

Expand Down
10 changes: 8 additions & 2 deletions index/extract_refs.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func (index *SpecIndex) ExtractRefs(node, parent *yaml.Node, seenPath []string,
} else {

// if the index has a base path, use that to resolve the path
if index.config.BasePath != "" {
if index.config.BasePath != "" && index.config.BaseURL == nil {
abs, _ := filepath.Abs(filepath.Join(index.config.BasePath, uri[0]))
if abs != defRoot {
abs, _ = filepath.Abs(filepath.Join(defRoot, uri[0]))
Expand All @@ -234,13 +234,19 @@ func (index *SpecIndex) ExtractRefs(node, parent *yaml.Node, seenPath []string,
componentName = fmt.Sprintf("#/%s", uri[1])
} else {
// if the index has a base URL, use that to resolve the path.
if index.config.BaseURL != nil {
if index.config.BaseURL != nil && !filepath.IsAbs(defRoot) {

u := *index.config.BaseURL
abs, _ := filepath.Abs(filepath.Join(u.Path, uri[0]))
u.Path = abs
fullDefinitionPath = fmt.Sprintf("%s#/%s", u.String(), uri[1])
componentName = fmt.Sprintf("#/%s", uri[1])

} else {

abs, _ := filepath.Abs(filepath.Join(defRoot, uri[0]))
fullDefinitionPath = fmt.Sprintf("%s#/%s", abs, uri[1])
componentName = fmt.Sprintf("#/%s", uri[1])
}
}
}
Expand Down
1 change: 1 addition & 0 deletions index/find_component.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func (index *SpecIndex) FindComponent(componentId string) *Reference {
// root search
return index.FindComponentInRoot(componentId)
}
return nil
}

func FindComponent(root *yaml.Node, componentId, absoluteFilePath string, index *SpecIndex) *Reference {
Expand Down
101 changes: 96 additions & 5 deletions index/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,8 @@ func (resolver *Resolver) isInfiniteCircularDependency(ref *Reference, visitedDe
return false, visitedDefinitions
}

// TODO: pick up here, required ref properties are not extracted correctly.

for refDefinition := range ref.RequiredRefProperties {
r, _ := resolver.specIndex.SearchIndexForReference(refDefinition)
if initialRef != nil && initialRef.Definition == r.Definition {
Expand Down Expand Up @@ -399,6 +401,7 @@ func (resolver *Resolver) extractRelatives(ref *Reference, node, parent *yaml.No
u, _ := url.Parse(httpExp[0])
abs, _ := filepath.Abs(filepath.Join(filepath.Dir(u.Path), exp[0]))
u.Path = abs
u.Fragment = ""
fullDef = fmt.Sprintf("%s#/%s", u.String(), exp[1])

} else {
Expand All @@ -412,7 +415,9 @@ func (resolver *Resolver) extractRelatives(ref *Reference, node, parent *yaml.No
fileDef := strings.Split(ref.FullDefinition, "#/")

// extract the location of the ref and build a full def path.
fullDef, _ = filepath.Abs(filepath.Join(filepath.Dir(fileDef[0]), exp[0]))
abs, _ := filepath.Abs(filepath.Join(filepath.Dir(fileDef[0]), exp[0]))
fullDef = fmt.Sprintf("%s#/%s", abs, exp[1])

}

}
Expand Down Expand Up @@ -518,11 +523,30 @@ func (resolver *Resolver) extractRelatives(ref *Reference, node, parent *yaml.No
if _, v := utils.FindKeyNodeTop("items", node.Content[i+1].Content); v != nil {
if utils.IsNodeMap(v) {
if d, _, l := utils.IsNodeRefValue(v); d {
mappedRefs := resolver.specIndex.GetMappedReferences()[l]

// create full definition lookup based on ref.
def := ref.FullDefinition
exp := strings.Split(ref.FullDefinition, "#/")
if len(exp) == 2 {
if exp[0] != "" {
if !strings.HasPrefix(ref.FullDefinition, "http") {
if !filepath.IsAbs(exp[0]) {
abs, _ := filepath.Abs(fmt.Sprintf("%s#/%s", filepath.Dir(ref.FullDefinition), exp[0]))
def = fmt.Sprintf("%s#/%s", abs, l)

Check warning on line 535 in index/resolver.go

View check run for this annotation

Codecov / codecov/patch

index/resolver.go#L532-L535

Added lines #L532 - L535 were not covered by tests
}
}
}
} else {
if !strings.Contains(ref.FullDefinition, "#") {

Check warning on line 540 in index/resolver.go

View check run for this annotation

Codecov / codecov/patch

index/resolver.go#L540

Added line #L540 was not covered by tests

}
}

mappedRefs := resolver.specIndex.GetMappedReferences()[def]
if mappedRefs != nil && !mappedRefs.Circular {
circ := false
for f := range journey {
if journey[f].Definition == mappedRefs.Definition {
if journey[f].FullDefinition == mappedRefs.FullDefinition {
circ = true
break
}
Expand Down Expand Up @@ -560,11 +584,78 @@ func (resolver *Resolver) extractRelatives(ref *Reference, node, parent *yaml.No
v := node.Content[i+1].Content[q]
if utils.IsNodeMap(v) {
if d, _, l := utils.IsNodeRefValue(v); d {
mappedRefs := resolver.specIndex.GetMappedReferences()[l]

// create full definition lookup based on ref.
def := l
exp := strings.Split(l, "#/")
if len(exp) == 2 {
if exp[0] != "" {
if !strings.HasPrefix(exp[0], "http") {
if !filepath.IsAbs(exp[0]) {

if strings.HasPrefix(ref.FullDefinition, "http") {

u, _ := url.Parse(ref.FullDefinition)
p, _ := filepath.Abs(filepath.Join(filepath.Dir(u.Path), exp[0]))
u.Path = p
def = fmt.Sprintf("%s#/%s", u.String(), exp[1])

} else {
abs, _ := filepath.Abs(filepath.Join(filepath.Dir(ref.FullDefinition), exp[0]))
def = fmt.Sprintf("%s#/%s", abs, exp[1])
}
}
} else {
panic("mummmmma mia")

Check warning on line 609 in index/resolver.go

View check run for this annotation

Codecov / codecov/patch

index/resolver.go#L609

Added line #L609 was not covered by tests
}

} else {
if strings.HasPrefix(ref.FullDefinition, "http") {
u, _ := url.Parse(ref.FullDefinition)
u.Fragment = ""
def = fmt.Sprintf("%s#/%s", u.String(), exp[1])

} else {
if strings.HasPrefix(ref.FullDefinition, "#/") {
def = fmt.Sprintf("#/%s", exp[1])
} else {
def = fmt.Sprintf("%s#/%s", ref.FullDefinition, exp[1])
}
}
}
} else {

if strings.HasPrefix(l, "http") {
def = l

Check warning on line 629 in index/resolver.go

View check run for this annotation

Codecov / codecov/patch

index/resolver.go#L629

Added line #L629 was not covered by tests
} else {
if filepath.IsAbs(l) {
def = l

Check warning on line 632 in index/resolver.go

View check run for this annotation

Codecov / codecov/patch

index/resolver.go#L632

Added line #L632 was not covered by tests
} else {

// check if were dealing with a remote file
if strings.HasPrefix(ref.FullDefinition, "http") {

// split the url.
u, _ := url.Parse(ref.FullDefinition)
abs, _ := filepath.Abs(filepath.Join(filepath.Dir(u.Path), l))
u.Path = abs
u.Fragment = ""
def = u.String()
} else {
lookupRef := strings.Split(ref.FullDefinition, "#/")
abs, _ := filepath.Abs(filepath.Join(filepath.Dir(lookupRef[0]), l))
def = abs
}
}
}
//panic("oh no")
}

mappedRefs, _ := resolver.specIndex.SearchIndexForReference(def)
if mappedRefs != nil && !mappedRefs.Circular {
circ := false
for f := range journey {
if journey[f].Definition == mappedRefs.Definition {
if journey[f].FullDefinition == mappedRefs.FullDefinition {
circ = true
break
}
Expand Down
33 changes: 21 additions & 12 deletions index/rolodex.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"net/url"
"os"
"path/filepath"
"sort"
"sync"
"time"
)
Expand Down Expand Up @@ -272,21 +273,24 @@ func (r *Rolodex) IndexTheRolodex() error {
copiedConfig.AvoidBuildIndex = true // we will build out everything in two steps.
idx, err := idxFile.Index(&copiedConfig)

// for each index, we need a resolver
resolver := NewResolver(idx)

// check if the config has been set to ignore circular references in arrays and polymorphic schemas
if copiedConfig.IgnoreArrayCircularReferences {
resolver.IgnoreArrayCircularReferences()
}
if copiedConfig.IgnorePolymorphicCircularReferences {
resolver.IgnorePolymorphicCircularReferences()
}

if err != nil {
errChan <- err
}
indexChan <- idx

if err == nil {
// for each index, we need a resolver
resolver := NewResolver(idx)

// check if the config has been set to ignore circular references in arrays and polymorphic schemas
if copiedConfig.IgnoreArrayCircularReferences {
resolver.IgnoreArrayCircularReferences()
}
if copiedConfig.IgnorePolymorphicCircularReferences {
resolver.IgnorePolymorphicCircularReferences()
}
indexChan <- idx
}

}

if lfs, ok := fs.(RolodexFS); ok {
Expand Down Expand Up @@ -339,6 +343,11 @@ func (r *Rolodex) IndexTheRolodex() error {
// now that we have indexed all the files, we can build the index.
r.indexes = indexBuildQueue
//if !r.indexConfig.AvoidBuildIndex {

sort.Slice(indexBuildQueue, func(i, j int) bool {
return indexBuildQueue[i].specAbsolutePath < indexBuildQueue[j].specAbsolutePath
})

for _, idx := range indexBuildQueue {
idx.BuildIndex()
if r.indexConfig.AvoidCircularReferenceCheck {
Expand Down
22 changes: 14 additions & 8 deletions index/rolodex_remote_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,17 @@ func (i *RemoteFS) Open(remoteURL string) (fs.File, error) {
// can we block threads.
if runtime.GOMAXPROCS(-1) > 2 {
i.logger.Debug("waiting for existing fetch to complete", "file", remoteURL, "remoteURL", remoteParsedURL.String())

Check warning on line 266 in index/rolodex_remote_loader.go

View check run for this annotation

Codecov / codecov/patch

index/rolodex_remote_loader.go#L266

Added line #L266 was not covered by tests
for {
if wf, ko := i.Files.Load(remoteParsedURL.Path); ko {
return wf.(*RemoteFile), nil

f := make(chan *RemoteFile)
fwait := func(path string, c chan *RemoteFile) {
for {
if wf, ko := i.Files.Load(remoteParsedURL.Path); ko {
c <- wf.(*RemoteFile)

Check warning on line 272 in index/rolodex_remote_loader.go

View check run for this annotation

Codecov / codecov/patch

index/rolodex_remote_loader.go#L268-L272

Added lines #L268 - L272 were not covered by tests
}
}
}
go fwait(remoteParsedURL.Path, f)
return <-f, nil

Check warning on line 277 in index/rolodex_remote_loader.go

View check run for this annotation

Codecov / codecov/patch

index/rolodex_remote_loader.go#L276-L277

Added lines #L276 - L277 were not covered by tests
}
}

Expand Down Expand Up @@ -366,12 +372,16 @@ func (i *RemoteFS) Open(remoteURL string) (fs.File, error) {
copiedCfg.BaseURL = newBaseURL
}
copiedCfg.SpecAbsolutePath = remoteParsedURL.String()
idx, idxError := remoteFile.Index(&copiedCfg)

if len(remoteFile.data) > 0 {
i.logger.Debug("successfully loaded file", "file", absolutePath)
}
//i.seekRelatives(remoteFile)
// remove from processing
i.ProcessingFiles.Delete(remoteParsedURL.Path)
i.Files.Store(absolutePath, remoteFile)

idx, idxError := remoteFile.Index(&copiedCfg)

if idxError != nil && idx == nil {
i.remoteErrors = append(i.remoteErrors, idxError)

Check warning on line 387 in index/rolodex_remote_loader.go

View check run for this annotation

Codecov / codecov/patch

index/rolodex_remote_loader.go#L387

Added line #L387 was not covered by tests
Expand All @@ -383,10 +393,6 @@ func (i *RemoteFS) Open(remoteURL string) (fs.File, error) {
idx.BuildIndex()
}

// remove from processing
i.ProcessingFiles.Delete(remoteParsedURL.Path)
i.Files.Store(absolutePath, remoteFile)

//if !i.remoteRunning {
return remoteFile, errors.Join(i.remoteErrors...)
// } else {
Expand Down
15 changes: 8 additions & 7 deletions index/rolodex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ components:
first := `openapi: 3.1.0
components:
schemas:
CircleTest:
StartTest:
type: object
required:
- muffins
Expand All @@ -268,9 +268,9 @@ components:
BaseDirectory: baseDir,
DirFS: os.DirFS(baseDir),
FileFilters: []string{
// "first.yaml",
// "second.yaml",
// "third.yaml",
"first.yaml",
"second.yaml",
"third.yaml",
"fourth.yaml",
},
}
Expand Down Expand Up @@ -300,10 +300,11 @@ components:
assert.NoError(t, err)
assert.Len(t, rolodex.GetCaughtErrors(), 0)

// there are two circles. Once when reading the journey from first.yaml, and then a second internal look in second.yaml
// there are three circles. Once when reading the journey from first.yaml, and then a second internal look in second.yaml
// the index won't find three, because by the time that 'three' has been read, it's already been indexed and the journey
// discovered.
assert.Len(t, rolodex.GetIgnoredCircularReferences(), 2)
// discovered. The third is the entirely 'new' circle that is sucked down via `fourth.yaml` from the simulated remote server, which contains
// all the same specs, it's just they are now being sucked in remotely.
assert.Len(t, rolodex.GetIgnoredCircularReferences(), 3)
}

func test_rolodexDeepRefServer(a, b, c []byte) *httptest.Server {
Expand Down
13 changes: 12 additions & 1 deletion index/search_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,18 @@ func (index *SpecIndex) SearchIndexForReferenceByReferenceWithContext(ctx contex
// does component exist in the root?
node, _ := rFile.GetContentAsYAMLNode()
if node != nil {
found := idx.FindComponent(ref)
var found *Reference
exp := strings.Split(ref, "#/")
compId := ref

if len(exp) == 2 {
compId = fmt.Sprintf("#/%s", exp[1])
found = FindComponent(node, compId, exp[0], idx)
}
if found == nil {
found = idx.FindComponent(ref)
}

if found != nil {
idx.cache.Store(ref, found)
index.cache.Store(ref, found)
Expand Down
9 changes: 6 additions & 3 deletions index/utility_methods.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,20 +103,22 @@ func extractDefinitionRequiredRefProperties(schemaNode *yaml.Node, reqRefProps m

// extractRequiredReferenceProperties returns a map of definition names to the property or properties which reference it within a node
func extractRequiredReferenceProperties(fulldef string, requiredPropDefNode *yaml.Node, propName string, reqRefProps map[string][]string) map[string][]string {
isRef, _, defPath := utils.IsNodeRefValue(requiredPropDefNode)
isRef, _, _ := utils.IsNodeRefValue(requiredPropDefNode)
if !isRef {
_, defItems := utils.FindKeyNodeTop("items", requiredPropDefNode.Content)
if defItems != nil {
isRef, _, defPath = utils.IsNodeRefValue(defItems)
isRef, _, _ = utils.IsNodeRefValue(defItems)
}
}

if /* still */ !isRef {
return reqRefProps
}

defPath := fulldef

// explode defpath
exp := strings.Split(defPath, "#/")
exp := strings.Split(fulldef, "#/")
if len(exp) == 2 {
if exp[0] != "" {
if !strings.HasPrefix(exp[0], "http") {
Expand All @@ -141,6 +143,7 @@ func extractRequiredReferenceProperties(fulldef string, requiredPropDefNode *yam

}
}

} else {

if strings.HasPrefix(exp[0], "http") {
Expand Down

0 comments on commit d8dfafd

Please sign in to comment.