Working through test coverage

This will be a bit of a slog, new code built in the hot path will need some love and attention.

Signed-off-by: quobix <dave@quobix.com>

datamodel/low/model_interfaces.go

@@ -96,8 +96,6 @@ type OpenAPIParameter interface {
 //TODO: this needs to be fixed, move returns to pointers.
 
 type SharedOperations interface {
-	//HasDescription
-	//HasExternalDocs
 	GetOperationId() NodeReference[string]
 	GetExternalDocs() NodeReference[any]
 	GetDescription() NodeReference[string]

document_examples_test.go

@@ -101,7 +101,7 @@ func ExampleNewDocument_fromWithDocumentConfigurationFailure() {
 	if len(errors) > 0 {
 		fmt.Println("Error building Digital Ocean spec errors reported")
 	}
-	// Output: There are 475 errors logged
+	// Output: There are 474 errors logged
 	//Error building Digital Ocean spec errors reported
 }
 

index/extract_refs.go

@@ -54,10 +54,6 @@ func (index *SpecIndex) ExtractRefs(node, parent *yaml.Node, seenPath []string,
 					definitionPath = fmt.Sprintf("#/%s", strings.Join(loc, "/"))
 					fullDefinitionPath = fmt.Sprintf("%s#/%s", index.specAbsolutePath, strings.Join(loc, "/"))
 					_, jsonPath = utils.ConvertComponentIdIntoFriendlyPathSearch(definitionPath)
-				} else {
-					definitionPath = fmt.Sprintf("#/%s", n.Value)
-					fullDefinitionPath = fmt.Sprintf("%s#/%s", index.specAbsolutePath, n.Value)
-					_, jsonPath = utils.ConvertComponentIdIntoFriendlyPathSearch(definitionPath)
 				}
 				ref := &Reference{
 					FullDefinition: fullDefinitionPath,
@@ -110,12 +106,7 @@ func (index *SpecIndex) ExtractRefs(node, parent *yaml.Node, seenPath []string,
 						definitionPath = fmt.Sprintf("#/%s", strings.Join(loc, "/"))
 						fullDefinitionPath = fmt.Sprintf("%s#/%s", index.specAbsolutePath, strings.Join(loc, "/"))
 						_, jsonPath = utils.ConvertComponentIdIntoFriendlyPathSearch(definitionPath)
-					} else {
-						definitionPath = fmt.Sprintf("#/%s", n.Value)
-						fullDefinitionPath = fmt.Sprintf("%s#/%s", index.specAbsolutePath, n.Value)
-						_, jsonPath = utils.ConvertComponentIdIntoFriendlyPathSearch(definitionPath)
 					}
-
 					ref := &Reference{
 						FullDefinition: fullDefinitionPath,
 						Definition:     definitionPath,
@@ -201,35 +192,16 @@ func (index *SpecIndex) ExtractRefs(node, parent *yaml.Node, seenPath []string,
 				copy(fp, seenPath)
 
 				value := node.Content[i+1].Value
-
 				segs := strings.Split(value, "/")
 				name := segs[len(segs)-1]
-
-				var p string
 				uri := strings.Split(value, "#/")
 
-				if strings.HasPrefix(value, "http") || filepath.IsAbs(value) {
-					if len(uri) == 2 {
-						_, p = utils.ConvertComponentIdIntoFriendlyPathSearch(fmt.Sprintf("#/%s", uri[1]))
-					} else {
-						_, p = utils.ConvertComponentIdIntoFriendlyPathSearch(uri[0])
-					}
-				} else {
-					if len(uri) == 2 {
-						_, p = utils.ConvertComponentIdIntoFriendlyPathSearch(fmt.Sprintf("#/%s", uri[1]))
-					} else {
-						_, p = utils.ConvertComponentIdIntoFriendlyPathSearch(value)
-					}
-				}
-
 				// determine absolute path to this definition
-
-				// TODO: come and clean this mess up.
-				var iroot string
+				var defRoot string
 				if strings.HasPrefix(index.specAbsolutePath, "http") {
-					iroot = index.specAbsolutePath
+					defRoot = index.specAbsolutePath
 				} else {
-					iroot = filepath.Dir(index.specAbsolutePath)
+					defRoot = filepath.Dir(index.specAbsolutePath)
 				}
 
 				var componentName string
@@ -255,8 +227,8 @@ func (index *SpecIndex) ExtractRefs(node, parent *yaml.Node, seenPath []string,
 								// if the index has a base path, use that to resolve the path
 								if index.config.BasePath != "" {
 									abs, _ := filepath.Abs(filepath.Join(index.config.BasePath, uri[0]))
-									if abs != iroot {
-										abs, _ = filepath.Abs(filepath.Join(iroot, uri[0]))
+									if abs != defRoot {
+										abs, _ = filepath.Abs(filepath.Join(defRoot, uri[0]))
 									}
 									fullDefinitionPath = fmt.Sprintf("%s#/%s", abs, uri[1])
 									componentName = fmt.Sprintf("#/%s", uri[1])
@@ -269,13 +241,6 @@ func (index *SpecIndex) ExtractRefs(node, parent *yaml.Node, seenPath []string,
 										u.Path = abs
 										fullDefinitionPath = fmt.Sprintf("%s#/%s", u.String(), uri[1])
 										componentName = fmt.Sprintf("#/%s", uri[1])
-
-									} else {
-
-										abs, _ := filepath.Abs(filepath.Join(iroot, uri[0]))
-										fullDefinitionPath = fmt.Sprintf("%s#/%s", abs, uri[1])
-										componentName = fmt.Sprintf("#/%s", uri[1])
-
 									}
 								}
 							}
@@ -287,29 +252,23 @@ func (index *SpecIndex) ExtractRefs(node, parent *yaml.Node, seenPath []string,
 						fullDefinitionPath = value
 					} else {
 						// is it a relative file include?
-						if strings.Contains(uri[0], "#") {
-							fullDefinitionPath = fmt.Sprintf("%s#/%s", iroot, uri[0])
-							componentName = fmt.Sprintf("#/%s", uri[0])
-						} else {
+						if !strings.Contains(uri[0], "#") {
 
-							if strings.HasPrefix(iroot, "http") {
+							if strings.HasPrefix(defRoot, "http") {
 								if !filepath.IsAbs(uri[0]) {
-									u, _ := url.Parse(iroot)
+									u, _ := url.Parse(defRoot)
 									pathDir := filepath.Dir(u.Path)
 									pathAbs, _ := filepath.Abs(filepath.Join(pathDir, uri[0]))
 									u.Path = pathAbs
 									fullDefinitionPath = u.String()
 								}
 							} else {
-								if filepath.IsAbs(uri[0]) {
-									fullDefinitionPath = uri[0]
-								} else {
-
+								if !filepath.IsAbs(uri[0]) {
 									// if the index has a base path, use that to resolve the path
 									if index.config.BasePath != "" {
 										abs, _ := filepath.Abs(filepath.Join(index.config.BasePath, uri[0]))
-										if abs != iroot {
-											abs, _ = filepath.Abs(filepath.Join(iroot, uri[0]))
+										if abs != defRoot {
+											abs, _ = filepath.Abs(filepath.Join(defRoot, uri[0]))
 										}
 										fullDefinitionPath = abs
 										componentName = uri[0]
@@ -325,7 +284,7 @@ func (index *SpecIndex) ExtractRefs(node, parent *yaml.Node, seenPath []string,
 
 										} else {
 
-											abs, _ := filepath.Abs(filepath.Join(iroot, uri[0]))
+											abs, _ := filepath.Abs(filepath.Join(defRoot, uri[0]))
 											fullDefinitionPath = abs
 											componentName = uri[0]
 
@@ -338,6 +297,8 @@ func (index *SpecIndex) ExtractRefs(node, parent *yaml.Node, seenPath []string,
 					}
 				}
 
+				_, p := utils.ConvertComponentIdIntoFriendlyPathSearch(componentName)
+
 				ref := &Reference{
 					FullDefinition: fullDefinitionPath,
 					Definition:     componentName,
@@ -598,10 +559,6 @@ func (index *SpecIndex) ExtractComponentsFromRefs(refs []*Reference) []*Referenc
 		located := index.FindComponent(ref.FullDefinition, ref.Node)
 		if located != nil {
 
-			if located.Index == nil {
-				index.logger.Warn("located component has no index", "component", located.FullDefinition)
-			}
-
 			index.refLock.Lock()
 			// have we already mapped this?
 			if index.allMappedRefs[ref.FullDefinition] == nil {
@@ -617,24 +574,6 @@ func (index *SpecIndex) ExtractComponentsFromRefs(refs []*Reference) []*Referenc
 					FullDefinition: ref.FullDefinition,
 				}
 				sequence[refIndex] = rm
-			} else {
-				// it exists, but is it a component with the same ID?
-				d := index.allMappedRefs[ref.FullDefinition]
-
-				// if the full definition matches, we're good and can skip this.
-				if d.FullDefinition != ref.FullDefinition {
-					found = append(found, located)
-					if located.FullDefinition != ref.FullDefinition {
-						located.FullDefinition = ref.FullDefinition
-					}
-					index.allMappedRefs[ref.FullDefinition] = located
-					rm := &ReferenceMapped{
-						Reference:      located,
-						Definition:     ref.Definition,
-						FullDefinition: ref.FullDefinition,
-					}
-					sequence[refIndex] = rm
-				}
 			}
 			index.refLock.Unlock()
 		} else {

index/find_component.go

@@ -34,19 +34,16 @@ func (index *SpecIndex) FindComponent(componentId string, parent *yaml.Node) *Re
 			return index.FindComponentInRoot(fmt.Sprintf("#/%s", uri[1]))
 		}
 	} else {
-		if !strings.Contains(componentId, "#") {
 
-			// does it contain a file extension?
-			fileExt := filepath.Ext(componentId)
-			if fileExt != "" {
-				return index.lookupRolodex(uri)
-			}
+		// does it contain a file extension?
+		fileExt := filepath.Ext(componentId)
+		if fileExt != "" {
+			return index.lookupRolodex(uri)
+		}
 
-			// root search
-			return index.FindComponentInRoot(componentId)
+		// root search
+		return index.FindComponentInRoot(componentId)
 
-		}
-		return index.FindComponentInRoot(fmt.Sprintf("#/%s", uri[0]))
 	}
 }
 
@@ -72,12 +69,6 @@ func FindComponent(root *yaml.Node, componentId, absoluteFilePath string, index
 
 		fullDef := fmt.Sprintf("%s%s", absoluteFilePath, componentId)
 
-		// TODO: clean this shit up
-
-		newIndexWithUpdatedPath := *index
-		newIndexWithUpdatedPath.specAbsolutePath = absoluteFilePath
-		newIndexWithUpdatedPath.AbsoluteFile = absoluteFilePath
-
 		// extract properties
 		ref := &Reference{
 			FullDefinition:        fullDef,
@@ -86,7 +77,7 @@ func FindComponent(root *yaml.Node, componentId, absoluteFilePath string, index
 			Node:                  resNode,
 			Path:                  friendlySearch,
 			RemoteLocation:        absoluteFilePath,
-			Index:                 &newIndexWithUpdatedPath,
+			Index:                 index,
 			RequiredRefProperties: extractDefinitionRequiredRefProperties(resNode, map[string][]string{}, fullDef),
 		}
 
@@ -121,24 +112,12 @@ func (index *SpecIndex) lookupRolodex(uri []string) *Reference {
 			absoluteFileLocation = file
 		} else {
 			if index.specAbsolutePath != "" {
-				if index.config.BaseURL != nil {
-
-					// extract the base path from the specAbsolutePath for this index.
-					sap, _ := url.Parse(index.specAbsolutePath)
-					newPath, _ := filepath.Abs(filepath.Join(filepath.Dir(sap.Path), file))
-
-					sap.Path = newPath
-					f := sap.String()
-					absoluteFileLocation = f
-
-				} else {
+				if index.config.BaseURL == nil {
 
 					// consider the file local
 					dir := filepath.Dir(index.config.SpecAbsolutePath)
 					absoluteFileLocation, _ = filepath.Abs(filepath.Join(dir, file))
 				}
-			} else {
-				absoluteFileLocation = file
 			}
 		}
 

index/find_component_test.go

@@ -71,6 +71,58 @@ func TestSpecIndex_CheckCircularIndex(t *testing.T) {
 	assert.Nil(t, c)
 }
 
+func TestFindComponent_RolodexFileParseError(t *testing.T) {
+
+	badData := "I cannot be parsed: \"I am not a YAML file or a JSON file"
+	_ = os.WriteFile("bad.yaml", []byte(badData), 0644)
+	defer os.Remove("bad.yaml")
+
+	badRef := `openapi: 3.1.0
+components:
+  schemas:
+    thing:
+      type: object
+      properties:
+        thong:
+          $ref: 'bad.yaml'
+`
+	var rootNode yaml.Node
+	_ = yaml.Unmarshal([]byte(badRef), &rootNode)
+
+	cf := CreateOpenAPIIndexConfig()
+	cf.AvoidCircularReferenceCheck = true
+	cf.BasePath = "."
+
+	rolo := NewRolodex(cf)
+	rolo.SetRootNode(&rootNode)
+	cf.Rolodex = rolo
+
+	fsCfg := LocalFSConfig{
+		BaseDirectory: cf.BasePath,
+		FileFilters:   []string{"bad.yaml"},
+		DirFS:         os.DirFS(cf.BasePath),
+	}
+
+	fileFS, err := NewLocalFSWithConfig(&fsCfg)
+
+	assert.NoError(t, err)
+	rolo.AddLocalFS(cf.BasePath, fileFS)
+
+	indexedErr := rolo.IndexTheRolodex()
+	rolo.BuildIndexes()
+
+	// should error
+	assert.Error(t, indexedErr)
+
+	index := rolo.GetRootIndex()
+
+	assert.Nil(t, index.uri)
+
+	// can't be found.
+	a, _ := index.SearchIndexForReference("bad.yaml")
+	assert.Nil(t, a)
+}
+
 func TestSpecIndex_performExternalLookup_invalidURL(t *testing.T) {
 	yml := `openapi: 3.1.0
 components:
@@ -105,6 +157,13 @@ components:
 	assert.Len(t, index.GetReferenceIndexErrors(), 0)
 }
 
+func TestSpecIndex_FailFindComponentInRoot(t *testing.T) {
+
+	index := &SpecIndex{}
+	assert.Nil(t, index.FindComponentInRoot("does it even matter? of course not. no"))
+
+}
+
 func TestSpecIndex_LocateRemoteDocsWithRemoteURLHandler(t *testing.T) {
 
 	// This test will push the index to do try and locate remote references that use relative references

index/index_utils.go

@@ -30,21 +30,6 @@ func isHttpMethod(val string) bool {
 	return false
 }
 
-func DetermineReferenceResolveType(ref string) int {
-	if ref != "" && ref[0] == '#' {
-		return LocalResolve
-	}
-	if ref != "" && len(ref) >= 5 && (ref[:5] == "https" || ref[:5] == "http:") {
-		return HttpResolve
-	}
-	if strings.Contains(ref, ".json") ||
-		strings.Contains(ref, ".yaml") ||
-		strings.Contains(ref, ".yml") {
-		return FileResolve
-	}
-	return -1
-}
-
 func boostrapIndexCollections(rootNode *yaml.Node, index *SpecIndex) {
 	index.root = rootNode
 	index.allRefs = make(map[string]*Reference)

what-changed/model/operation.go

@@ -405,7 +405,6 @@ func CompareOperations(l, r any) *OperationChanges {
 		oc.ServerChanges = checkServers(lOperation.Servers, rOperation.Servers)
 		oc.ExtensionChanges = CompareExtensions(lOperation.Extensions, rOperation.Extensions)
 
-		// todo: callbacks
 	}
 	CheckProperties(props)
 	oc.PropertyChanges = NewPropertyChanges(changes)