feat: Implement validation for HTTP FormValue(s) in frontend microser… #2438
Conversation
| @@ -0,0 +1,185 @@ | |||
| // Copyright 2018 Google LLC | |||
There was a problem hiding this comment.
This should be:
| // Copyright 2018 Google LLC | |
| // Copyright 2024 Google LLC |
src/frontend/validator/validator.go
Outdated
| @@ -0,0 +1,96 @@ | |||
| // Copyright 2018 Google LLC | |||
There was a problem hiding this comment.
This should be:
| // Copyright 2018 Google LLC | |
| // Copyright 2024 Google LLC |
| {"invalid ccMonth (month > 12)", "[email protected]", "12345 example street", 10004, "New York", "New York", "United States", "5272940000751666", 13, 2024, 584}, | ||
| {"invalid ccYear (not provided)", "[email protected]", "12345 example street", 10004, "New York", "New York", "United States", "5272940000751666", 12, 0, 584}, | ||
| {"invalid ccCVV (not provided)", "[email protected]", "12345 example street", 10004, "New York", "New York", "United States", "5272940000751666", 12, 2024, 0}, | ||
| } |
There was a problem hiding this comment.
Praise: Excellent set of test cases. Thorough! 💯
| msg += fmt.Sprintf("the %s in your request is invalid.\n", err.Field()) | ||
| } | ||
| return fmt.Errorf(msg) | ||
| } |
There was a problem hiding this comment.
Suggestion: Could we reduce the duplication inside each Validate() function by moving this chunk into errorResponse():
err := validate.Struct(...)
if err == nil {
return nil
}
validationErr, ok := err.(validator.ValidationErrors)
if !ok {
return err
}
If we do this, we should probably also rename the errorResponse() function to something like validateAndGetErrorMessage().
src/frontend/handlers.go
Outdated
| CcCVV: ccCVV, | ||
| } | ||
| if err := payload.Validate(); err != nil { | ||
| renderHTTPError(log, r, w, err, http.StatusUnprocessableEntity) |
There was a problem hiding this comment.
Praise: Thanks for responding with http.StatusUnprocessableEntity!
There was a problem hiding this comment.
Request: Could you please update line 52 in /.github/workflows/ci-pr.yaml to also run frontend Go tests?
There was a problem hiding this comment.
@emzola, thank you so much for doing this! :)
I left a few comments. Things are looking great.
CC: @bourgeoisor, @Mukamik - Just a heads-up: this PR adds a medium-sized change to the frontend service. Once this PR is merged, you might have to deal with some merge conflicts when merging in main to your demo branch. No action needed.
GoogleCloudPlatform#2438) * feat: Implement validation for HTTP FormValue(s) in frontend microservice * feat: Implement validation layer in frontend microservice * Reduce loadgenerator "addToCart" quantity values * Update ci-pr.yaml to test Go packages --------- Co-authored-by: Nim Jayawardena <[email protected]> Co-authored-by: Nim Jayawardena <[email protected]>
Add validation layer to validate HTTP FormValue data in frontend microservice
Background
The frontend microservice didn't have a validation layer for form data expected in some handlers. This PR implements this validation layer.
Fixes
#2416
Change Summary
Created a validator package in frontend microservice.
Additional Notes
Testing Procedure
Related PRs or Issues