-
-
Notifications
You must be signed in to change notification settings - Fork 91
Address https://github.com/daveshanley/vacuum/issues/485 #491
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #491 +/- ##
==========================================
- Coverage 99.59% 99.59% -0.01%
==========================================
Files 175 179 +4
Lines 21700 22013 +313
==========================================
+ Hits 21613 21924 +311
- Misses 55 57 +2
Partials 32 32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes issue #485 by preserving hash characters (#) in OpenAPI component names instead of incorrectly replacing them with dollar signs ($). The leading # in JSON Pointer notation (e.g., #/components/...) is already stripped during path splitting, so any # characters within actual component names (like async_search.submit#wait_for_completion_timeout from Elasticsearch specs) should be preserved literally in the generated JSONPath queries.
Key Changes
- Removed the
#to$character replacement logic from the final path assembly inConvertComponentIdIntoFriendlyPathSearch - Updated existing tests to reflect the corrected behavior where
#is preserved in component names - Added comprehensive test coverage for issue #485 with 7 new test functions covering various edge cases
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| utils/utils.go | Removed character replacement loops that converted # to $ in the final path assembly, added clarifying comments explaining why # should be preserved |
| utils_test.go | Updated 9 existing test assertions to expect # instead of $ in paths, added 7 new test functions specifically for issue #485 covering real-world cases and edge scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`$id` is now fully supported, and will also perform the correct resolution chain. Bringing `libopenain` completely inline with JSON Schema 2020-12
When schemas declare $id, relative $ref values should resolve against that $id per JSON Schema 2020-12 spec. Previously, libopenapi would try to find files directly instead of resolving against the $id base URI.
components:
schemas:
s1:
$id: "https://example.com/a.json"
type: string
s2:
$ref: "a.json" # Should resolve to s1 via its $id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
some code can’t be reached, therefore it should not exist.
the rest are trash, but this is legit.
I am not going to re-build the functions myself, there is little gain, however I have documented the behavior clearly.
Address #309
Address #471