osu-capstone-cs72 / cs-applied-plan-portal

A portal that streamlines the planning process for OSU CS Applied students and advisors
https://applied-plan-portal.herokuapp.com
MIT License
2 stars 5 forks source link

Schema validation ignores fields that are not required #62

Closed silverware13 closed 4 years ago

silverware13 commented 4 years ago

Schema validation currently ignores fields that are set to not required, as if the field was not specified.

However the comments explain the required field being set to false as allowing patch requests to function properly.

For example if I have a field called planName and I have the required field set to false, then it should mean that it is not a violation to submit a PATCH request without the planName field in the body. However if I do send the planName field and it does not match the schema then there should be a violation.

Using the above example currently sending the planName not formatted in the way the schema expects is met with no errors, when set to "required: false".

philectron commented 4 years ago

You're right. The schema validator does ignore fields that are not required. However, when the isPartialObj parameter is set to true, the function will treat the input object as a PATCH object. All fields are optional, but if you include any of the fields it has to satisfy the schema. Once the isPartialObj parameter is set to true, the validator doesn't care about the required flag anymore---as all fields are now treated as optional.

For your example, since the request is a PATCH, you send in a planName which is one of the field names of the planSchema, the validator must validate this planName regardless of the requirement because if it didn't, it would patch the object with the invalid planName to the existing object.

In short, my intention when I implemented the schema validator to work with PATCH requests is that "All fields are optional, and if a field is included in the request body, it must satisfy the schema."

silverware13 commented 4 years ago

Thank you for the clarification, with that in mind I realize there is not a bug here. I am closing this issue now.