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

Refactor request body validation using decorator #5920

Merged
merged 2 commits into from
Jun 21, 2023

Conversation

Aadesh-Baral
Copy link
Contributor

@Aadesh-Baral Aadesh-Baral commented Jun 15, 2023

This PR adds the necessary component to fix Task 1 of #5910

  • Creates a decorator which can be used to validate the request body against a DTO class.
    • This decorator can be applied to the route handler that requires request validation.
    • This decorator validates the request using the provided DTO class and stores the validated DTO instance in request.validated_dto.
  • Adds a function to fetch validation errors from Schematics DataError exception class so that we can return what and where error occurred during request validation.

Sample validation error :

@validate_request(ProjectCreateDTO)
def post():
validated_dto = request.validated_dto # request.validated_dto will be set in the decorator after validating request against dto ProjectCreateDTO

If the validation error occurs due to missing project description and instruction we will have error response like:

{
  "error":{
    "code": 400,
    "sub_code": "BAD_REQUEST",
    "message": "The request was invalid or cannot be otherwise served."
    "details": {
      "field_errors": {
        "field": "description", "message": "This field is required",
        "field": "instruction", "message": "This field is required"
      }
    }
  }
}

Benefits:

  1. Now we can just add a decorator on the route handler (like in the example above) instead of following code to validate the request body.
try:
    dto = ProjectCreateDTO(request.get_json())
    dto.validate()
except DataError as e:
   return {"Error": str(e), "SubCode": "BAD_REQUEST"}
  1. The error message will have more information like what and where the issue occured
  2. This approach improves code organization, promotes code reuse, and enhances readability.

NOTE: This decorator must me only used after decorator token_auth.login_required if both decorator are to be used.

@token_auth.login_required
@validate_request(ProjectCreateDTO)
def post():
validated_dto = request.validated_dto 

--------------------------------------------------

- Refactored the validation of the request body against a DTO class by implementing a decorator, validate_request_body.
- This decorator will be applied to the route handler that requires request body validation.
- This decorator validates the request body using the provided DTO class and stores the validated DTO instance in request.validated_dto.
- This approach improves code organization, promotes code reuse, and enhances readability.
- Added documentation to the decorator to clarify its purpose and behavior.
This commit improves the validation process for request bodies and enhances the maintainability and readability of the codebase.
Copy link
Contributor

@eternaltyro eternaltyro left a comment

Choose a reason for hiding this comment

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

Just need some context to understand the change.

backend/models/dtos/__init__.py Show resolved Hide resolved
Copy link
Contributor

@d-rita d-rita left a comment

Choose a reason for hiding this comment

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

Given that this will give more context for validation errors, LGTM

…ng in views.

---------------------------------------------------------
The previous implementation of the request body validation decorator has been refactored to provide better flexibility and handling of requests. The new implementation introduces the following changes:

- Renamed the decorator from validate_request_body to validate_request to reflect its broader functionality of validating the entire request, including request body, query parameters, and path parameters.

- Added a note in the decorator's docstring regarding the recommended order of applying decorators, emphasizing that it should come after the token_auth decorator if used. This ensures that the authenticated user ID can be set on the DTO if it has a user_id field.

- Modified the decorator to dynamically set attribute values on the DTO based on the request's body, query parameters, and path parameters. This enables more flexible assignment of values to DTO attributes, accommodating different request sources.

- Introduced the use of request.is_json to check if the request body is in JSON format before accessing its attributes. This helps avoid errors when handling non-JSON request bodies.

- Added support for setting the authenticated user ID on the DTO if it contains a user_id field. The user ID is obtained from the token_auth service's current_user() method.

This decorator should only be applied after the token_auth decorator (if token_auth is used) so that this decorator can access token_auth.current_user.
@sonarcloud
Copy link

sonarcloud bot commented Jun 21, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Aadesh-Baral
Copy link
Contributor Author

Aadesh-Baral commented Jun 21, 2023

@d-rita @eternaltyro I modified the decorator, so it now validates the DTO class using all of the path, query, and body parameters rather than just the body parameters. Could you kindly re-review the PR? The decorator is now more broadly applicable and may be used on views that takes path and query params. Take a look at the commit message for further details.

I apologize for making changes after the review.

@eternaltyro eternaltyro merged commit 63a624a into develop Jun 21, 2023
8 checks passed
@eternaltyro eternaltyro deleted the enhance/5910-validation-error branch June 21, 2023 08:37
@Aadesh-Baral Aadesh-Baral linked an issue Jun 21, 2023 that may be closed by this pull request
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return detailed request validation error messages
3 participants