Allow ID wrappers as path variables #33
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sgrimm
force-pushed
the
wrapper-params
branch
from
9b2ad32 to
5496487
Compare
Previously, if we wanted to include an ID value in the URL path, we had to declare the path variable as `Long` in the controller and then explicitly create an ID wrapper object. This was needed because Spring MVC didn't know how to convert a string to an instance of an ID wrapper class. So the simple fix is to add a constructor to the wrapper classes that takes a string argument. If the client uses a non-numeric value, it will fail with HTTP 400 the same way it fails with `Long` parameters. Update all the controller methods that used to take `Long` arguments to instead use the appropriate wrapper classes. This caused us to hit swagger-api/swagger-core#3904 so upgrade the OpenAPI library to get the fix for that.
sgrimm
force-pushed
the
wrapper-params
branch
from
5496487 to
bff8811
Compare
|
In addition to running the test suite, I also tested this by running the seedbank-app Cypress test suite to confirm this doesn't introduce any behavior changes that break the front end. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, if we wanted to include an ID value in the URL path, we had to declare
the path variable as
Longin the controller and then explicitly create an IDwrapper object.
This was needed because Spring MVC didn't know how to convert a string to an
instance of an ID wrapper class. So the simple fix is to add a constructor to the
wrapper classes that takes a string argument. If the client uses a non-numeric
value, it will fail with HTTP 400 the same way it fails with
Longparameters.Update all the controller methods that used to take
Longarguments to insteaduse the appropriate wrapper classes.
This caused us to hit swagger-api/swagger-core#3904 so
upgrade the OpenAPI library to get the fix for that.