Embedded spec sometimes omits schemas from external refs, race-like

[mgabeler-lee-6rs]

I have a couple OAS specs in my app. One of them loads a schema from another spec file, using something like this:

components:
  specs:
    MyFoo:
      $ref: '../otherdir/otherspec.yaml#/components/schemas/Foo'

I have oapi-codegen configured thus:

package: mythingy
generate:
  models: true
  client: true
  gin-server: true
  embedded-spec: true
import-mapping:
  "../otherdir/otherspec.yaml": "mymodule/otherthingy"
output: generated.go

I added a CI step to verify the committed generated code was up to date, and started noticing random failures. In the failure cases, the schemas from the externally referenced spec were missing.

I was able to reproduce this locally by running a loop:

( set -x ; while go run github.com/deepmap/oapi-codegen/v2/cmd/oapi-codegen -config oapi-codegen.yaml ./openapi.yaml ; do git diff --exit-code || break ; done )

This fails within 10 loops at most for me.

This looks a lot like #1055, but that was closed as "using an old version". I'm reproducing this with the latest release (v2.1.0).

[jamietanna]

Interesting 🤔 are you able to share what the diff is when generation fails? Is it that there's subtle differences in the generated spec?

[percivalalb]

I think I'm also experiencing the same on v2.1, running the codegen a dozen times produces 2 or 3 distinct diffs. In each case the base64 data in var swaggerSpec = []string{ is slightly different.

I'm working on a minimal spec to reproduce.

[percivalalb]

The spec in https://github.com/percivalalb/oapi-codegen-issue-1572 reproduces this issue

[jamietanna]

Thanks so much @percivalalb :raised_hands:

Via our very new contribution guideline, mind also sticking an Apache-2..0 license on that repo?

[percivalalb]

Thanks so much @percivalalb 🙌

Via our very new contribution guideline, mind also sticking an Apache-2..0 license on that repo?

Sure - done

[mgabeler-lee-6rs]

Interesting 🤔 are you able to share what the diff is when generation fails? Is it that there's subtle differences in the generated spec?

I can't share the diff (internal proprietary stuff), but I can characterize it. In the bad generation, I'm generally seeing the externally referenced schema having come over, but any other schemas it referenced within that external file are missing.

Unlike Percival's sample, I'm seeing the bad result being the same each time, at least out of 3 failing runs, after normalizing away any document ordering differences via VSCode's JSON: Sort document command.

[jamietanna]

Thanks! I can also see this in https://github.com/deepmap/oapi-codegen/pull/1481/ which has the two specs:

{"components":{"schemas":{"Category":{"properties":{"id":{"example":1,"format":"int64","type":"integer"},"name":{"example":"Dogs","type":"string"}},"type":"object","xml":{"name":"category"}},"Container":{"properties":{"object_a":{"$ref":"#/components/schemas/ObjectA"},"object_b":{"$ref":"#/components/schemas/ObjectB"},"object_c":{"$ref":"#/components/schemas/object_c"},"pet":{"$ref":"#/components/schemas/Pet"}}},"ObjectA":{"properties":{"name":{"type":"string"},"object_b":{"$ref":"#/components/schemas/ObjectB"}}},"ObjectB":{"properties":{"name":{"type":"string"}}},"Pet":{"properties":{"category":{"$ref":"#/components/schemas/Category"},"id":{"example":10,"format":"int64","type":"integer"},"name":{"example":"doggie","type":"string"},"photoUrls":{"items":{"type":"string","xml":{"name":"photoUrl"}},"type":"array","xml":{"wrapped":true}},"status":{"description":"pet status in the store","enum":["available","pending","sold"],"type":"string"},"tags":{"items":{"$ref":"#/components/schemas/Tag"},"type":"array","xml":{"wrapped":true}}},"required":["name","photoUrls"],"type":"object","xml":{"name":"pet"}},"Tag":{"properties":{"id":{"format":"int64","type":"integer"},"name":{"type":"string"}},"type":"object","xml":{"name":"tag"}},"object_c":{"additionalProperties":true,"type":"object"}}},"info":{"title":"","version":""},"openapi":"3.0.0","paths":{}}

{"components":{"schemas":{"Category":{"properties":{"id":{"properties":{"id":{"example":1,"format":"int64","type":"integer"},"name":{"example":"Dogs","type":"string"}},"type":"object","xml":{"name":"category"}},"name":{"properties":{"id":{"example":1,"format":"int64","type":"integer"},"name":{"example":"Dogs","type":"string"}},"type":"object","xml":{"name":"category"}}},"type":"object","xml":{"name":"category"}},"Container":{"properties":{"object_a":{"$ref":"#/components/schemas/ObjectA"},"object_b":{"$ref":"#/components/schemas/ObjectB"},"object_c":{"$ref":"#/components/schemas/object_c"},"pet":{"$ref":"#/components/schemas/Pet"}}},"ObjectA":{"properties":{"name":{"type":"string"},"object_b":{"$ref":"#/components/schemas/ObjectB"}}},"ObjectB":{"properties":{"name":{"type":"string"}}},"Pet":{"properties":{"category":{"properties":{"category":{"$ref":"#/components/schemas/Category"},"id":{"example":10,"format":"int64","type":"integer"},"name":{"example":"doggie","type":"string"},"photoUrls":{"items":{"type":"string","xml":{"name":"photoUrl"}},"type":"array","xml":{"wrapped":true}},"status":{"description":"pet status in the store","enum":["available","pending","sold"],"type":"string"},"tags":{"items":{"$ref":"#/components/schemas/Tag"},"type":"array","xml":{"wrapped":true}}},"required":["name","photoUrls"],"type":"object","xml":{"name":"pet"}},"id":{"properties":{"category":{"$ref":"#/components/schemas/Category"},"id":{"example":10,"format":"int64","type":"integer"},"name":{"example":"doggie","type":"string"},"photoUrls":{"items":{"type":"string","xml":{"name":"photoUrl"}},"type":"array","xml":{"wrapped":true}},"status":{"description":"pet status in the store","enum":["available","pending","sold"],"type":"string"},"tags":{"items":{"$ref":"#/components/schemas/Tag"},"type":"array","xml":{"wrapped":true}}},"required":["name","photoUrls"],"type":"object","xml":{"name":"pet"}},"name":{"properties":{"category":{"$ref":"#/components/schemas/Category"},"id":{"example":10,"format":"int64","type":"integer"},"name":{"example":"doggie","type":"string"},"photoUrls":{"items":{"type":"string","xml":{"name":"photoUrl"}},"type":"array","xml":{"wrapped":true}},"status":{"description":"pet status in the store","enum":["available","pending","sold"],"type":"string"},"tags":{"items":{"$ref":"#/components/schemas/Tag"},"type":"array","xml":{"wrapped":true}}},"required":["name","photoUrls"],"type":"object","xml":{"name":"pet"}},"photoUrls":{"properties":{"category":{"$ref":"#/components/schemas/Category"},"id":{"example":10,"format":"int64","type":"integer"},"name":{"example":"doggie","type":"string"},"photoUrls":{"items":{"type":"string","xml":{"name":"photoUrl"}},"type":"array","xml":{"wrapped":true}},"status":{"description":"pet status in the store","enum":["available","pending","sold"],"type":"string"},"tags":{"items":{"$ref":"#/components/schemas/Tag"},"type":"array","xml":{"wrapped":true}}},"required":["name","photoUrls"],"type":"object","xml":{"name":"pet"}},"status":{"properties":{"category":{"$ref":"#/components/schemas/Category"},"id":{"example":10,"format":"int64","type":"integer"},"name":{"example":"doggie","type":"string"},"photoUrls":{"items":{"type":"string","xml":{"name":"photoUrl"}},"type":"array","xml":{"wrapped":true}},"status":{"description":"pet status in the store","enum":["available","pending","sold"],"type":"string"},"tags":{"items":{"$ref":"#/components/schemas/Tag"},"type":"array","xml":{"wrapped":true}}},"required":["name","photoUrls"],"type":"object","xml":{"name":"pet"}},"tags":{"properties":{"category":{"$ref":"#/components/schemas/Category"},"id":{"example":10,"format":"int64","type":"integer"},"name":{"example":"doggie","type":"string"},"photoUrls":{"items":{"type":"string","xml":{"name":"photoUrl"}},"type":"array","xml":{"wrapped":true}},"status":{"description":"pet status in the store","enum":["available","pending","sold"],"type":"string"},"tags":{"items":{"$ref":"#/components/schemas/Tag"},"type":"array","xml":{"wrapped":true}}},"required":["name","photoUrls"],"type":"object","xml":{"name":"pet"}}},"required":["name","photoUrls"],"type":"object","xml":{"name":"pet"}},"Tag":{"properties":{"id":{"properties":{"id":{"format":"int64","type":"integer"},"name":{"type":"string"}},"type":"object","xml":{"name":"tag"}},"name":{"properties":{"id":{"format":"int64","type":"integer"},"name":{"type":"string"}},"type":"object","xml":{"name":"tag"}}},"type":"object","xml":{"name":"tag"}},"object_c":{"additionalProperties":true,"type":"object"}}},"info":{"title":"","version":""},"openapi":"3.0.0","paths":{}}

[percivalalb]

The issue stems from InternalizeRefs call not being particular clever about the names it gives the new schemas/components it adds. The default behaviour is:

// If a reference points to an element inside a document, it returns the last // element in the reference using filepath.Base. Otherwise if the reference points // to a file, it returns the file name trimmed of all extensions.

So in my example spec the $refs ../../components/responses/successes/message/records.yaml & ../../components/responses/successes/number/records.yaml both are converted to a schema with name "records". There is some non-deterministic logic during internalised, so the schema with name "records" that is included in the seralised spec flip-flops between the "message records" schema and "number records" schema.

So it's an issue more generally downstream with kin-openapi.

[jamietanna]

Thanks very much for the work on this, @percivalalb :raised_hands: Very appreciated!