Don't validate readonly field for write requests or vice-versa#532
Don't validate readonly field for write requests or vice-versa#532CTrando wants to merge 2 commits into
Conversation
CTrando
changed the title
fenollp
left a comment
fenollp
left a comment
There was a problem hiding this comment.
From https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.0.md#schema-object
cc #246 I'm not sure I understand your issue.
There's already VisitAsRequest / VisitAsResponse options which you clone in this PR. Why not reuse them?
Your tests only mention openapi3filter.ValidateRequest for which you introduced EndpointType. Why not use ``openapi3filter.ValidateResponse` as well?
Also if your tests could reuse as much as possible of the same openapi document that'd be a great help to make sense of them please.
|
@CTrando can you share the exact error output you receive before this change? Just trying to make sure we have all the context here |
codyaray
left a comment
codyaray
left a comment
There was a problem hiding this comment.
I don't know the codebase at all, so just took a prelim look here
| router, err := legacyrouter.NewRouter(doc) | ||
| require.NoError(t, err) | ||
|
|
||
| // Set no id because id is a required readonly field, but this is a write request |
There was a problem hiding this comment.
ID is marked writeOnly in this test, not readOnly...
| Route: route, | ||
| Options: &Options{EndpointType: ReadEndpoint}, | ||
| }) | ||
| require.Error(t, err) |
There was a problem hiding this comment.
wait, why would this fail due to "insufficient length ID" if this is a read endpoint and a read-only field (i.e., it shouldn't be validated)?
|
@CTrando Kind greetings :) Is there anything you'd like to add to this PR / explain about the issue you're solving? Thanks! |
3 participants
What
Don't validate readonly field for write requests
Why
In my work, we were using the same struct for many of our openapi endpoints, and one of those struct fields was
readOnly. We were getting validation errors whenever we tried to make a post request with that struct because we weren't filling in the field.For example, we were using the common struct:
For all our endpoints.
When we tried to create an account, we wouldn't fill in the ID, but we were still getting validation errors. The issue is because in the library there's no way (to my knowledge) to give the validator context on whether the endpoint is a read endpoint or a write endpoint. The behavior should be if the endpoint is a write endpoint, then we shouldn't validate a readonly field.
cc @codyaray who knows more about this than me.