fix: correct resolver configuration merge in ValidationService
Description
Fixes external file resolution via $ref that broke in v3.3.0. The issue was caused by incorrect object spread order in the ValidationService constructor.
Problem:
Since the 3.3.0 refactor, the ValidationService constructor has been merging __unstable config in the wrong order. The spread of parserOptions.__unstable?.resolver overwrites the resolver array, breaking local file reference resolution
BEFORE (bug):
__unstable: {
resolver: {
resolvers: [createHttpWithAuthResolver(), ...] // Was added
},
...parserOptions.__unstable?.resolver, // Overwrites above
}
Solution: Reordered spreads so base config is spread first, then custom resolvers are set last:
AFTER (fixed):
__unstable: {
...parserOptions.__unstable,
resolver: {
...parserOptions.__unstable?.resolver,
resolvers: [createHttpWithAuthResolver(), ...] // Set last
}
}
Testing:
- Done All existing tests pass (200 passing)
- Local file references now resolve correctly
- GitHub resolver with auth still works
Related issue(s)
Fixes #1839
⚠️ No Changeset found
Latest commit: 81d12760a711124fcce028393740175803207bbd
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
Changeset has been generated for this PR as part of auto-changeset workflow.
Please review the changeset before merging the PR.
---
'@asyncapi/cli': patch
---
fix: correct resolver configuration merge in ValidationService
- 29c22f1: fix: correct resolver configuration merge in ValidationService
Fixes #1839
- b4529e5: test: add regression test for resolver config merge bug
- 8246049: test: add integration test for external file reference resolution
If you are a maintainer or the author of the PR, you can change the changeset by clicking here
[!TIP] If you don't want auto-changeset to run on this PR, you can add the label
skip-changesetto the PR or remove the changeset and change PR title to something other thanfix:orfeat:.
Thanks for finding the root cause @Kartikayy007. Could you possibly add a test targeting #1839 so that such scenarios don't happen later?
Thanks for finding the root cause @Kartikayy007. Could you possibly add a test targeting #1839 so that such scenarios don't happen later?
ready for another review @Shurtu-gal
@Kartikayy007 What I meant was having a set of asyncapi files with refs between them. That should provide us good sense of security.
@Shurtu-gal please review, also please take a look at #1887 whenever you get time
@Shurtu-gal ready to merge?
/rtm
I believe after PR https://github.com/asyncapi/cli/pull/1875 adding the custom resolver support, the spread of parserOptions.__unstable?.resolver was overwriting the custom resolvers array that was breaking local file reference resolution causing bug.
Just curious, PR #1875 was merged on Oct 27 but the issue #1839 was reported on Aug 22.
Just curious, PR #1875 was merged on Oct 27 but the issue #1839 was reported on Aug 22.
Thank you for that the bug came in the v3.3.0 itself not in the #1875 the PR inherited the issue, ill update the PR description
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code