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

fix: fix issue with JSON ref resolver producing duplicate results #29

Merged
merged 2 commits into from
Mar 29, 2024

Conversation

semcc
Copy link
Collaborator

@semcc semcc commented Mar 29, 2024

Problem

When attempting to resolve JSON refs for an array of components of length > 1, and if none of the components have a ref property, it effectively overwrites the result object with the "current" result over every iteration in the map which leads to the final result being an array of duplicates of whatever the last component is.

This becomes an issue when validating an OpenAPI schema with required = true parameters because the resulting resolved spec will contain duplicate required = true parameters of the same name, which is a schema violation.

Solution

Pass an empty object to the simpleSpecResolveInternal call in the array mapping because we should be generating "new" results per map iteration, not using whatever the previous "result" is.

Testing

New test added to utils.spec.mts

@semcc semcc requested a review from jfrconley March 29, 2024 20:28
Copy link

changeset-bot bot commented Mar 29, 2024

🦋 Changeset detected

Latest commit: 345a030

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@nornir/rest Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

Coverage report

St.
Category Percentage Covered / Total
🔴 Statements 9.22% 133/1442
🔴 Branches 3.39% 18/531
🔴 Functions 16.61% 49/295
🔴 Lines 9.44% 130/1377

Test suite run success

43 tests passing in 6 suites.

Report generated by 🧪jest coverage report action from 345a030

@jfrconley jfrconley merged commit 61e8469 into main Mar 29, 2024
6 checks passed
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.

2 participants