Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expander: expanded document is not self-contained with some circular $ref patterns #94

Open
fredbi opened this issue Jul 4, 2019 · 0 comments · May be fixed by #137
Open

Expander: expanded document is not self-contained with some circular $ref patterns #94

fredbi opened this issue Jul 4, 2019 · 0 comments · May be fixed by #137

Comments

@fredbi
Copy link
Member

fredbi commented Jul 4, 2019

Let's make things official about this limitation:

  • whenever $ref form a cycle remotely (i.e. the cycle is out of the root document)

Then the expanded result leaves some $ref items to the remote document with the cycle.
This means the expanded result is not always self-contained.

At this moment, this is not blocking, and libraries such as go-openapi/validate or go-openapi/analysis still work with this.

@fredbi fredbi self-assigned this Dec 9, 2020
fredbi added a commit to fredbi/spec that referenced this issue Jan 4, 2021
* fixes go-openapi#94

* now expanded $ref are always contained in the resulting document.
  All circular $ref that used to resolve to a remote $ref now resolve
  as a json pointer inside the expanded document.
* schema IDs are removed from the expanded spec: schemas expanded from some schema ID reference
  now refer to the new expanded root document.
* fixed circular ID resolution

Signed-off-by: Frederic BIDON <[email protected]>
fredbi added a commit to fredbi/spec that referenced this issue Jan 5, 2021
* fixes go-openapi#94

* now expanded $ref's are always contained in the resulting document.
  All circular $ref that used to resolve to a remote $ref now resolve
  as a json pointer inside the expanded document. Pointer resolution
  prefers pointers to definitions.

* added additional test case for remote cyclical $ref, from azure API

* schema IDs are removed from the expanded spec: schemas expanded from some schema ID reference
  now refer to the new expanded root document.

* circular IDs are resolved against the corresponding root document.

> NOTE(1): uncovered pre-existing issue with nested schema ID involving cyclical references.
> This case remains unsupported and is illustrated by test case: circular_test.go#L198 ("withID")

> NOTE(2): pre-existing issue with non-deterministic expansion remains unsolved,
> although the election of the replacing pointer inside the root document
> somewhat reduces the scope of this problem.
>
> This case remains illustrated by a minimal test case: circular_test.go#L46 ("minimal"),
> which expands correctly, but with changing results.

> NOTE(3): notice that expansion is still not an idempotent transform, in the presence
> of cyclical $ref's: another run on an expanded spec with remaining cyclical $ref
> will expand further down and detect again the cycle.
>
> The result remains functionally correct, as illustrated by test case: circular_test.go#L168 ("CircularID").
> Notice that this test case reproduces a validation fixture from jsonschema test (passed by go-openapi/validate).

Signed-off-by: Frederic BIDON <[email protected]>
@fredbi fredbi linked a pull request Jan 5, 2021 that will close this issue
@fredbi fredbi added the expand label Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant