Skip to content

Commit

Permalink
Fix description memoization in bundle schema (#1409)
Browse files Browse the repository at this point in the history
## Changes
Fixes #559

The CLI generation is now stable and does not produce a diff for the
`bundle_descriptions.json` file.

Before a pointer to the schema was stored in the memo, which would be
mutated later to include the description. This lead to duplicate
documentation for schema components that were used in multiple places.
This PR fixes this issue.

Eg: Before all references of `pause_status` would have the same
description.

## Tests
Added regression test.
  • Loading branch information
shreyas-goenka authored May 1, 2024
1 parent 507053e commit 3021586
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 39 deletions.
2 changes: 1 addition & 1 deletion bundle/schema/docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func UpdateBundleDescriptions(openapiSpecPath string) (*Docs, error) {
}
openapiReader := &OpenapiReader{
OpenapiSpec: spec,
Memo: make(map[string]*jsonschema.Schema),
memo: make(map[string]jsonschema.Schema),
}

// Generate descriptions for the "resources" field
Expand Down
28 changes: 14 additions & 14 deletions bundle/schema/docs/bundle_descriptions.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 27 additions & 16 deletions bundle/schema/openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,40 @@ import (
)

type OpenapiReader struct {
// OpenAPI spec to read schemas from.
OpenapiSpec *openapi.Specification
Memo map[string]*jsonschema.Schema

// In-memory cache of schemas read from the OpenAPI spec.
memo map[string]jsonschema.Schema
}

const SchemaPathPrefix = "#/components/schemas/"

func (reader *OpenapiReader) readOpenapiSchema(path string) (*jsonschema.Schema, error) {
// Read a schema directly from the OpenAPI spec.
func (reader *OpenapiReader) readOpenapiSchema(path string) (jsonschema.Schema, error) {
schemaKey := strings.TrimPrefix(path, SchemaPathPrefix)

// return early if we already have a computed schema
memoSchema, ok := reader.Memo[schemaKey]
memoSchema, ok := reader.memo[schemaKey]
if ok {
return memoSchema, nil
}

// check path is present in openapi spec
openapiSchema, ok := reader.OpenapiSpec.Components.Schemas[schemaKey]
if !ok {
return nil, fmt.Errorf("schema with path %s not found in openapi spec", path)
return jsonschema.Schema{}, fmt.Errorf("schema with path %s not found in openapi spec", path)
}

// convert openapi schema to the native schema struct
bytes, err := json.Marshal(*openapiSchema)
if err != nil {
return nil, err
return jsonschema.Schema{}, err
}
jsonSchema := &jsonschema.Schema{}
err = json.Unmarshal(bytes, jsonSchema)
jsonSchema := jsonschema.Schema{}
err = json.Unmarshal(bytes, &jsonSchema)
if err != nil {
return nil, err
return jsonschema.Schema{}, err
}

// A hack to convert a map[string]interface{} to *Schema
Expand All @@ -49,23 +53,28 @@ func (reader *OpenapiReader) readOpenapiSchema(path string) (*jsonschema.Schema,
if ok {
b, err := json.Marshal(jsonSchema.AdditionalProperties)
if err != nil {
return nil, err
return jsonschema.Schema{}, err
}
additionalProperties := &jsonschema.Schema{}
err = json.Unmarshal(b, additionalProperties)
if err != nil {
return nil, err
return jsonschema.Schema{}, err
}
jsonSchema.AdditionalProperties = additionalProperties
}

// store read schema into memo
reader.Memo[schemaKey] = jsonSchema
reader.memo[schemaKey] = jsonSchema

return jsonSchema, nil
}

// safe againt loops in refs
// Resolve all nested "$ref" references in the schema. This function unrolls a single
// level of "$ref" in the schema and calls into traverseSchema to resolve nested references.
// Thus this function and traverseSchema are mutually recursive.
//
// This function is safe against reference loops. If a reference loop is detected, an error
// is returned.
func (reader *OpenapiReader) safeResolveRefs(root *jsonschema.Schema, tracker *tracker) (*jsonschema.Schema, error) {
if root.Reference == nil {
return reader.traverseSchema(root, tracker)
Expand All @@ -91,12 +100,12 @@ func (reader *OpenapiReader) safeResolveRefs(root *jsonschema.Schema, tracker *t
// in the memo
root.Reference = nil

// unroll one level of reference
// unroll one level of reference.
selfRef, err := reader.readOpenapiSchema(ref)
if err != nil {
return nil, err
}
root = selfRef
root = &selfRef
root.Description = description

// traverse again to find new references
Expand All @@ -108,6 +117,8 @@ func (reader *OpenapiReader) safeResolveRefs(root *jsonschema.Schema, tracker *t
return root, err
}

// Traverse the nested properties of the schema to resolve "$ref" references. This function
// and safeResolveRefs are mutually recursive.
func (reader *OpenapiReader) traverseSchema(root *jsonschema.Schema, tracker *tracker) (*jsonschema.Schema, error) {
// case primitive (or invalid)
if root.Type != jsonschema.ObjectType && root.Type != jsonschema.ArrayType {
Expand Down Expand Up @@ -154,11 +165,11 @@ func (reader *OpenapiReader) readResolvedSchema(path string) (*jsonschema.Schema
}
tracker := newTracker()
tracker.push(path, path)
root, err = reader.safeResolveRefs(root, tracker)
resolvedRoot, err := reader.safeResolveRefs(&root, tracker)
if err != nil {
return nil, tracker.errWithTrace(err.Error(), "")
}
return root, nil
return resolvedRoot, nil
}

func (reader *OpenapiReader) jobsDocs() (*Docs, error) {
Expand Down
Loading

0 comments on commit 3021586

Please sign in to comment.