REST API: Inconsistent type for 'media_details' and 'sizes' in attachments controller

[SergeyBiryukov]

Background: #50913, #50640.

While working on the tickets above, I've discovered an inconsistency in WP_REST_Attachments_Controller.

In the ::prepare_item_for_response() method, $data['media_details'] is treated as an array if not empty, but is initialized as an stdClass object if empty. Same for $data['media_details']['sizes'].

This was introduced along with the class itself in [38832].

This is currently causing ​two errors on PHP 8:

1) WP_Test_REST_Attachments_Controller::test_get_item_sizes
Error: Cannot use object of type stdClass as array

/var/www/tests/phpunit/tests/rest-api/rest-attachments-controller.php:607

2) WP_Test_REST_Attachments_Controller::test_get_item_sizes_with_no_url
Error: Cannot use object of type stdClass as array

/var/www/tests/phpunit/tests/rest-api/rest-attachments-controller.php:635

The errors are caused by $data['media_details']['sizes'] being initialized as an empty object, due to missing GD library in the PHP 8 build in the Docker image used for the tests. See comment:10:ticket:50640 for some more context.

The same errors can also be reproduced on PHP 7 or 5.6 if the GD library is not loaded.

The tests should probably be updated to check if $data['media_details']['sizes'] is not empty before proceeding, or to require the GD library, but the main issue remains.

Could we switch these properties to be initialized as an empty array instead of an object? See the attached patch.

[SergeyBiryukov]

Removing the keyword. Although this was discovered while working on PHP 8 support, it's not specific to PHP 8 after all, and can be reproduced on any PHP version if the GD library is not loaded.

[TimothyBlynJacobs]

I _think_ this was implemented this way so that when the result is json_encoded it is encoded as an empty json object, instead of a json array. Ie {} instead of [].

Regardless, the correct schema type is an object. See ​https://developer.wordpress.org/rest-api/extending-the-rest-api/schema/#primitive-types for reference.

The tests should probably be updated to check if $data['media_details']['sizes'] is not empty before proceeding

I think this is the correct fix. Changing the data values in the controller would be a BC break.

[SergeyBiryukov]

Thanks for the clarification! This makes more sense to me now :)

Let's just fix the tests as part of #50913 or #50640 then, I agree with that being the correct fix here.

[SergeyBiryukov]

In 49044:

Tests: Check if image sizes were successfully retrieved in some REST API attachments controller tests.

This outputs a proper message in case of failure, instead of an obscure PHP error further in the test.

Props TimothyBlynJacobs.
See #50913, #51393.

[SergeyBiryukov]

In 49046:

Tests: Correct the check for image sizes in some REST API attachments controller tests.

If the sizes data could not be retrieved, the controller returns an empty object instead of an array.

This makes sure that the value is in fact an array before proceeding, and outputs a proper message in case of failure, instead of an obscure PHP error further in the test.

Follow-up to [49044].

See #50913, #51393.