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

wrap $ref in allOf when readonly is set #1034

Merged
merged 3 commits into from
Oct 26, 2021
Merged

wrap $ref in allOf when readonly is set #1034

merged 3 commits into from
Oct 26, 2021

Conversation

josephzidell
Copy link
Contributor

Describe the PR
This PR adds functionality to support readonly $refs. It is supported by swagger, yet requires it to be wrapped in an allOf.

Relation issue
#1033

Additional context
Unfortunately, using readonly as a sibling to a $ref is ignored. The workaround is to wrap the $ref in an allOf. See this StackOverflow answer. I have confirmed that manually changing the swagger.json makes this work as expected.

@josephzidell
Copy link
Contributor Author

I added a simple test confirming that setting readonly:"true" propagates (which we already know is the case, but seemed to have been missing a test case).

Can someone point me in the right direction as to a full test for this feature?

@codecov
Copy link

codecov bot commented Oct 22, 2021

Codecov Report

Merging #1034 (3a62e1c) into master (0548f60) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 3a62e1c differs from pull request most recent head eebd162. Consider uploading reports for the commit eebd162 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1034      +/-   ##
==========================================
+ Coverage   93.01%   93.02%   +0.01%     
==========================================
  Files           7        7              
  Lines        2005     2008       +3     
==========================================
+ Hits         1865     1868       +3     
  Misses         77       77              
  Partials       63       63              
Impacted Files Coverage Δ
parser.go 93.50% <100.00%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0548f60...eebd162. Read the comment docs.

@ubogdan
Copy link
Contributor

ubogdan commented Oct 22, 2021

@josephzidell, you need to write an additional unit test that will call parseStructField method to cover Line 1094 of parser.go.

@ubogdan
Copy link
Contributor

ubogdan commented Oct 25, 2021

@josephzidell We can't merge this PR until we have the unit tests and code coverage passing.

@josephzidell
Copy link
Contributor Author

josephzidell commented Oct 25, 2021

@ubogdan Understood. Unfortunately, I am feeling lost w/r to the style of unit tests in this repo. If someone can offer some guidance, I'd be happy to wrap this up.

@josephzidell
Copy link
Contributor Author

@ubogdan Is this test in the right vicinity?

Copy link
Contributor

@ubogdan ubogdan left a comment

Choose a reason for hiding this comment

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

LGTM

@ubogdan
Copy link
Contributor

ubogdan commented Oct 26, 2021

@josephzidell Yes, it looks good enough to me. 😃
Thanks for your contribution.

@ubogdan ubogdan merged commit b5ca0bb into swaggo:master Oct 26, 2021
@josephzidell josephzidell deleted the wrap-ref-in-allof-when-readonly branch October 26, 2021 18:25
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