Hang when expanding circular $ref

[fredbi]

This PR is not fully satisfying in the sense that it does not fix once and for all the circular ref story. It improves spec expansion which no more hangs on some complex circular ref patterns. However, it is not able to produce stable expanded schemas when it used to hang.

All produced expansions are correct, but may vary in the way they expand cycles, when several cycles do collide. Since we walk the spec at random and this spec is mutating during the expansion process, we stand no chance to come out with a clean resolution (i.e. resolve shortest cycles first)

What it does:

• Fixes #76 (absolute path left behind in resolved circular $ref)
• Fixes #74 (isCircular no more checks on basePath!="")
• Contributes #75 (hang is solved, but the (always correct) expanded result may differ from one run to another)
• Contributes go-swagger/go-swagger#957 (hang on circular spec)

Even though you chose not to merge this, and maybe rightly so, there are some advances with this problem, like a minimal reproducible test case.

Cheers,

Fred

[codecov[bot]]

Codecov Report

Merging #79 into master will increase coverage by 0.69%. The diff coverage is 78.72%.

@@            Coverage Diff             @@
##           master      #79      +/-   ##
==========================================
+ Coverage   40.27%   40.96%   +0.69%     
==========================================
  Files          18       19       +1     
  Lines        1959     1982      +23     
==========================================
+ Hits          789      812      +23     
  Misses        995      995              
  Partials      175      175

Impacted Files	Coverage Δ
debug.go	100% <100%> (ø)
expander.go	55.59% <74.35%> (+1.14%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 16284f9...af4c70f. Read the comment docs.

[pytlesk4]

This looks good to me, I am going to check out your changes and test this against some problematic specs we run into at Stoplight.

[fredbi]

How about the unstable aspect of this? I mean, if you expand to validate, this is okay (all cycles are expanded in a somewhat correct way), but if you want, say to get documentation, that's no good.

Is that okay with you?

[pytlesk4]

For our use case at Stoplight and Prism, that is fine. We are expanding the spec to do contract testing of request/responses. It would take a bigger refactor to make it stable imo, and it is just nice that it doesn't hang anymore.

[fredbi]

We have a problem with that one.

When I attempted to integrate with go-swagger, I could see that this breaks validate.

Validate expands JSON-schema meta schema and this is broken.

[casualjim]

I reverted this one

[fredbi]

I am fixing this

[fredbi]

@pytlesk4 . Oh just realized you push a fix yourself. I am trying to test it with complete go-swagger integration then