BaseController::prepare_item_for_response only works on the first level depth

[nima-karimi]

The BaseController::prepare_item_for_response method is usually run in our API controllers to prepare a data item for the response. It formats the data according to the schema defined for each endpoint.

Some properties, however, are defined as objects in the endpoint schemas and can introduce another level of properties to the schema. At the moment BaseController::prepare_item_for_response only checks the top-level properties of a data item and doesn't traverse through all of the sublevel properties. It should instead loop through all of the properties of an item if it's defined as an object and confirm that the properties are according to the schema.

Related PR that sparked this: https://github.com/woocommerce/google-listings-and-ads/pull/1284

[mikkamp]

Just wanted to add that we want to do this for both object and array. Here is a list of the primitive types for reference.

[jorgemd24]

After the discussion with @mikkamp about possible solutions using WP validation schema functions such as rest_sanitize_value_from_schema and with the goal to improve the following points:

1. The sanitization of each field.
2. Cast empty arrays to objects when the schema type is "Object".
3. Using the default property in any sublevels, is currently only for the top-level properties.

We could create a new function based on the rest_sanitize_value_from_schema and adding the following functionalities:

1. Casting objects.
2. The ability to check the default value if the value is not set.

The advantage of this approach is that we are going to have more control over the data sent to the client.

I looked for other ways to achieve the same points that we mentioned before, but in any case, it is required to transverse all the data and the schema.

If we decide that this is the right direction, I am happy to implement this solution.

[mikkamp]

I tried to look into this a bit more and find examples of how it's being handled in other extensions. From the examples I found the validation / sanitation based on schema is done mostly for data sent to the API. For responses they mostly ensure it's sanitized as needed but it's not compared to the schema to make sure it matches. The closest I could find is the StoreAPI which has a function get_item_response which ensures the response is formatted as expected (but it doesn't do this by walking through the schema).

Adding a generic check for validation/sanitation in BaseController::prepare_item_for_response seems to be because we don't trust how the controller created the response, so we double check it matches the schema, sounds like some of this can be done in unit testing instead. So I wonder if it's more useful to have an inherited function like get_item_response which sanitizes all the fields as needed, instead of walking through the response recursively and double checking it matches the schema.

Also we aren't very strict in the controllers so not all responses use prepare_item_for_response and just return a response directly, especially in cases where we share multiple routes within a controller.

Using the default property in any sublevels, is currently only for the top-level properties.

If I look through all the controllers I only see the default being used for the fields we are passing in the request, but it's not used in any responses, so I wonder if we really need this.

Cast empty arrays to objects when the schema type is "Object"

I don't see any better example for this. We only need it in Shipping Rates so far, if we want to keep this generic we could add a simple recursive function that loops through the response and fixes this, but not have it do much else.

[mikkamp]

Additional details on preparing responses: pb0Spc-2pd-p2

I'm going to relabel this as technical debt, as we need some further discussion on how to align this across all controllers. It's not currently a bug that is causing issues.