diff --git a/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/parsingcontext/build.go b/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/parsingcontext/build.go index 722d46a188..441da5b55a 100644 --- a/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/parsingcontext/build.go +++ b/tools/importer-rest-api-specs/internal/components/apidefinitions/parser/parsingcontext/build.go @@ -11,8 +11,6 @@ import ( ) func BuildFromFile(filePath string) (*Context, error) { - //directory := filepath.Dir(filePath) - // parsing this twice looks silly, so why are we doing this? // we want the name and the properties of the objects, and the "expanded" version // removes the names - it could be we can patch this to also include this, but @@ -23,11 +21,6 @@ func BuildFromFile(filePath string) (*Context, error) { return nil, fmt.Errorf("loading swagger file %q: %+v", filePath, err) } - //swaggerDocWithReferences, err = findAndMergeLocalMixins(swaggerDocWithReferences, directory, filePath) - //if err != nil { - // return nil, fmt.Errorf("could not mixin remote swagger files referenced by %q: %+v", filePath, err) - //} - // walk the refs in the loaded spec and resolve any external references localMixins, err := resolveExternalSwaggerReferences(swaggerDocWithReferences, filePath) if err != nil { @@ -70,13 +63,6 @@ func BuildFromFile(filePath string) (*Context, error) { // mix-in the refs from any external swagger documents, analysis.Mixin(expandedSwaggerDoc.Spec(), localMixins) - //expandedSwaggerDoc, err = findAndMergeLocalMixins(expandedSwaggerDoc, directory, filePath) - //if err != nil { - // return nil, fmt.Errorf("could not mixin remote swagger files referenced by %q: %+v", filePath, err) - //} - // - //analysis.Mixin(expandedSwaggerDoc.Spec(), localMixins) - // flatten the spec, inlining any refs basePath = expandedSwaggerDoc.SpecFilePath() flattenedExpandedOpts := &analysis.FlattenOpts{ @@ -105,42 +91,11 @@ func BuildFromFile(filePath string) (*Context, error) { }, nil } -//func findAndMergeLocalMixins(input *loads.Document, basePath string, baseFile string) (*loads.Document, error) { -// //if len(strings.Split(baseFile, fmt.Sprintf("%c", filepath.Separator))) != 2 { // We only care about local files, not sub-folders -// // return input, nil -// //} -// pathsToMixin := make([]string, 0) -// if input.Analyzer != nil { -// allRefs := input.Analyzer.AllRefs() -// for _, v := range allRefs { -// if path := v.Ref.GetURL(); path != nil && path.Path != "" { //} && len(strings.Split(path.Path, "/")) == 2 { // Check if we have a reference in the CWD. -// pathsToMixin = append(pathsToMixin, path.Path) -// } -// } -// } -// -// if len(pathsToMixin) > 0 { -// uniqueFilter := make(map[string]bool) -// uniquePaths := make([]string, 0) -// for _, path := range pathsToMixin { -// if _, ok := uniqueFilter[path]; !ok { -// uniqueFilter[path] = true -// uniquePaths = append(uniquePaths, path) -// } -// } -// mixins := make([]*spec.Swagger, 0) -// for _, v := range uniquePaths { -// doc, err := loads.Spec(fmt.Sprintf("%s/%s", basePath, v)) -// if err != nil { -// return nil, fmt.Errorf("could not load remote ref %q for mixin in %q: %+v", v, baseFile, err) -// } -// mixins = append(mixins, doc.Spec()) -// } -// _ = analysis.Mixin(input.Spec(), mixins...) -// } -// return input, nil -//} - +// resolveExternalSwaggerReferences attempts to resolve all external refs in the current document. each external ref +// will be parsed and resolved recursively, until all possible references have been collated and returned in a *spec.Swagger +// containing all these definitions. +// we are going this because the go-openapi parser has very buggy relative path resolution logic, and is unable to +// resolve relative paths in external referenced swagger files that are not located in the current directory. func resolveExternalSwaggerReferences(input *loads.Document, filePath string) (*spec.Swagger, error) { externalReferences := make(map[string]spec.Schema) @@ -159,12 +114,6 @@ func resolveExternalSwaggerReferences(input *loads.Document, filePath string) (* return nil, fmt.Errorf("resolving external ref %q in %q: %+v", ref.String(), filePath, err) } } - - //for _, ref := range refs { - // if !strings.HasPrefix(ref.GetURL().Fragment, "/parameters/") { - // ref.Ref.ReferenceURL.Path = "" - // } - //} } return &spec.Swagger{ @@ -175,18 +124,17 @@ func resolveExternalSwaggerReferences(input *loads.Document, filePath string) (* } func resolveRefsForModel(ref spec.Ref, filePath, topLevelDir string, doc *loads.Document, refs map[string]spec.Schema) error { - //if path := ref.GetURL(); path == nil || (path.Path != "" && len(strings.Split(path.Path, "/")) != 2) { - // Check if we have a reference in the CWD (otherwise there will be issue resolving relative path in analysis.Flatten). if path.Path is empty string, it refers to the same file - //return nil - //} - + // ignore parameters as we don't use these. we might need to include these at some point, if the go-openapi parser + // is unable to resolve them internally (which has not happened yet) if strings.HasPrefix(ref.GetURL().Fragment, "/parameters/") { return nil } + // determine the model name, and a swagger doc where it should be defined modelName, modelFilePath := modelNamePathFromRef(ref, filePath, topLevelDir) - // bye bye path + // the path in the ref will no longer be needed, as we are going to resolve it ourselves + // we can do this because `ReferenceURL` is a *url.URL, so this will thankfully propagate upstream ref.Ref.ReferenceURL.Path = "" if _, ok := refs[modelName]; ok { @@ -199,6 +147,7 @@ func resolveRefsForModel(ref spec.Ref, filePath, topLevelDir string, doc *loads. return nil } + // decide whether to look for the model definition in the current file, or another referenced file refDoc := doc if modelFilePath != filePath { newDoc, err := loads.Spec(modelFilePath) @@ -216,14 +165,12 @@ func resolveRefsForModel(ref spec.Ref, filePath, topLevelDir string, doc *loads. } refs[modelName] = resolvedModel - // bye bye path - ref.Ref.ReferenceURL.Path = "" - // look for any refs in the properties (fields) of the resolved model if err := resolveRefsForProperties(resolvedModel.Properties, modelFilePath, topLevelDir, refDoc, refs); err != nil { return err } + // some models use `additionalProperties`, these may contain refs if resolvedModel.AdditionalProperties != nil && resolvedModel.AdditionalProperties.Schema != nil { if resolvedModel.AdditionalProperties.Schema.Ref.String() != "" { if err := resolveRefsForModel(resolvedModel.AdditionalProperties.Schema.Ref, modelFilePath, topLevelDir, refDoc, refs); err != nil { @@ -232,23 +179,12 @@ func resolveRefsForModel(ref spec.Ref, filePath, topLevelDir string, doc *loads. } } + // some models use a top-level `items`, i.e. in the case of a type array(object) if items := resolvedModel.Items; items != nil && items.Schema != nil && items.Schema.Ref.String() != "" { if err := resolveRefsForModel(items.Schema.Ref, modelFilePath, topLevelDir, refDoc, refs); err != nil { return err } } - //for _, prop := range resolvedModel.Properties { - // if prop.Ref.String() != "" { - // if err := resolveRefsForModel(prop.Ref, modelFilePath, topLevelDir, refDoc, refs); err != nil { - // return err - // } - // } - // if items := prop.Items; items != nil && items.Schema != nil && items.Schema.Ref.String() != "" { - // if err := resolveRefsForModel(items.Schema.Ref, modelFilePath, topLevelDir, refDoc, refs); err != nil { - // return err - // } - // } - //} // if this is a parent model, look for implementations if resolvedModel.Discriminator != "" { @@ -264,8 +200,13 @@ func resolveRefsForModel(ref spec.Ref, filePath, topLevelDir string, doc *loads. allOfRefModelName, _ := modelNamePathFromRef(allOf.Ref, modelFilePath, topLevelDir) if modelName == allOfRefModelName { - // this is an implementation of our parent, so resolve it - if err := resolveRefsForModel(allOf.Ref, modelFilePath, topLevelDir, refDoc, refs); err != nil { + // this is an implementation of our parent, so construct a Ref for it and attempt to resolve it + childRef, err := spec.NewRef("#/definitions/" + defName) + if err != nil { + return fmt.Errorf("constructing a Ref for %q: %v", defName, err) + } + + if err = resolveRefsForModel(childRef, modelFilePath, topLevelDir, refDoc, refs); err != nil { return err } break @@ -275,9 +216,7 @@ func resolveRefsForModel(ref spec.Ref, filePath, topLevelDir string, doc *loads. } } - // if this is a child model, look for a parent - //_, isImplementation := resolvedModel.Extensions.GetString("x-ms-discriminator-value") - //if isImplementation { + // look for parent models - this could be a plain child model, or a discriminated implementation for _, allOf := range resolvedModel.AllOf { if allOf.Ref.String() != "" { if err := resolveRefsForModel(allOf.Ref, modelFilePath, topLevelDir, refDoc, refs); err != nil { @@ -285,13 +224,14 @@ func resolveRefsForModel(ref spec.Ref, filePath, topLevelDir string, doc *loads. } } } - //} return nil } +// resolveRefsForProperties attempts to resolve any references contained within the properties for a model func resolveRefsForProperties(properties spec.SchemaProperties, modelFilePath, topLevelDir string, refDoc *loads.Document, refs map[string]spec.Schema) error { for _, prop := range properties { + // some property types inherit from another model for _, allOf := range prop.AllOf { if allOf.Ref.String() != "" { if err := resolveRefsForModel(allOf.Ref, modelFilePath, topLevelDir, refDoc, refs); err != nil { @@ -299,23 +239,40 @@ func resolveRefsForProperties(properties spec.SchemaProperties, modelFilePath, t } } } + + // some properties use `additionalProperties` + if prop.AdditionalProperties != nil && prop.AdditionalProperties.Schema != nil { + if prop.AdditionalProperties.Schema.Ref.String() != "" { + if err := resolveRefsForModel(prop.AdditionalProperties.Schema.Ref, modelFilePath, topLevelDir, refDoc, refs); err != nil { + return err + } + } + } + + // look for a model referenced directly by the property if prop.Ref.String() != "" { if err := resolveRefsForModel(prop.Ref, modelFilePath, topLevelDir, refDoc, refs); err != nil { return err } } + + // look for a model referenced directly by the items of a property (i.e. for array elements) if items := prop.Items; items != nil && items.Schema != nil && items.Schema.Ref.String() != "" { if err := resolveRefsForModel(items.Schema.Ref, modelFilePath, topLevelDir, refDoc, refs); err != nil { return err } } + + // recursively resolve any refs for properties which have inlined models if err := resolveRefsForProperties(prop.Properties, modelFilePath, topLevelDir, refDoc, refs); err != nil { return err } } + return nil } +// modelNamePathFromRef determines the model name, and the path to the swagger doc that defines it, from a supplied ref func modelNamePathFromRef(ref spec.Ref, sourcePath, topLevelDir string) (modelName string, modelFilePath string) { refUrl := ref.GetURL() if refUrl == nil { @@ -329,17 +286,10 @@ func modelNamePathFromRef(ref spec.Ref, sourcePath, topLevelDir string) (modelNa } else { // external reference, so prepend the directory from the sourcePath // note: there is a known swagger issue where refs have invalid paths like `.../../cosmos-db.json`, fortunately - // `filepath.Join` does a good job of cleaning these up and so far these continue to resolve to the actual + // `filepath.Join` does a good job of cleaning these up and _so far_ these continue to resolve to the actual // correct path. if this problem gets worse, this is a good place to add any manual workarounds. sourceDir, _ := filepath.Split(sourcePath) modelFilePath = filepath.Join(sourceDir, modelFilePath) - - // if the model file resides in a directory higher than the source file directory, then skip it. these paths - // are often broken and so far we don't need anything in the common specs from outside the service anyway. - //modelFileDir, _ := filepath.Split(modelFilePath) - //if len(modelFileDir) < len(topLevelDir) { - // modelFilePath = "" - //} } // get the modelName from the last URL fragment