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

Fixing issue #309 - References outside components and paths are not resolved #645

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

erfemega
Copy link
Contributor

@erfemega erfemega commented Nov 24, 2022

This PR fixes issue #309

  • The fix was extended to dereference all the elements that are out of components element
  • It was added support for the case when the user references all/individual paths from '#/components/pathItems' in OAS 3.1
  • Added support to circular reference cases (when there is a circular ref in schemas)

It was added a suite of tests to validate that all referenceable elements:

  • Responses individually
  • Schemas
  • RequestBodies individually
  • Examples individually
  • Paths individually and all together
  • Parameters individually
  • Headers individually
    are well resolved in both cases:
  • When user references from out of components
  • When user references from in of components

@VShingala
Copy link
Member

@erfemega There are some conflicts that need resolution. Also, if you see deref.js file, inside resolveRefs() function at line no. 212, we have handling that restricts such usage based on no. of segments. Can we also add handling there? Current PR doesn't support such refs. (i.e. refs inside schema components)

Copy link
Member

@VShingala VShingala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is not addressing all the places we're r4esolving references. See here.

Do take a look at all places where we resolve $ref objects and add appropriate changes.

- Referencing from out of components
- Referencing paths from pathItems in oas 3.1
- Adding circular references in dereference process
- Adding test scenarios
The method and implementation were changed to other location
lib/deref.js Show resolved Hide resolved
lib/deref.js Show resolved Hide resolved
Copy link
Member

@VShingala VShingala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR looks good overall, there are some conflicts and one comment pending. Let's address it.

* @returns {boolean} whether is circular reference or not.
*/
isCircularReference: function (traverseContext, contentFromTrace) {
return traverseContext.parents.find((parent) => { return parent.node === contentFromTrace; }) !== undefined;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erfemega Is this a deep comparison?

@eumenhofer
Copy link

Is there anything I can do to get this cleared out?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants