REST API: WP_REST_Controller::get_public_item_schema() assumes that the schema has properties.

[pento]

My understanding of JSON Schema is that the base type doesn't necessarily need to be an object, it can be any type, so it's possible to define an endpoint that returns a string:

array(
        '$schema'     => 'http://json-schema.org/draft-04/schema#',
        'title'       => 'foo',
        'type'        => 'string',
        'description' => __( 'This is my magical endpoint that just returns a string.' ),
);

Other methods on WP_REST_Controller seem to check that properties is defined on the schema, but ::get_public_item_schema() doesn't.

[dkarfa]

Hi @pento
Yes, ::get_item_schema() returing the properties

[TimothyBlynJacobs]

Yep, this would be good to fix. I've run into the issue as well.

[TimothyBlynJacobs]

Thanks for working on a patch for this @dhavalkasvala! Do you want to try tackling a ​unit test for this? Coding standards wise, we should also have ​spaces around the parentheses.

[TimothyBlynJacobs]

Thanks for updating the patch @dhavalkasvala, this looks good! Do you want to work on a unit test for this? No worries if not.

[dhavalkasvala]

@TimothyBlynJacobs I want to work on unit test but don't how to do that right now.

[TimothyBlynJacobs]

No worries @dhavalkasvala! There is some documentation available on WordPress.org: ​https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/

I sent you a message on Slack as well. Feel free to ping me about getting started with it.

[johnwatkins0]

Includes the previously attached change with unit tests covering the change as well as get_public_item_schema generally

[johnwatkins0]

Made unit test PHP5-compatible

[TimothyBlynJacobs]

Thanks for working on a test for this @johnwatkins0!

I think it'd be better if we created a new WP_REST_Test_Controller that accepted in its constructor a schema. That way we'd be able to reuse the controller for any schema related controller tests.

Perhaps something along the lines of WP_REST_Test_Configurable_Controller?

[johnwatkins0]

Thanks, @TimothyBlynJacobs. I like that Idea. Updated in 48785.3.diff.

[TimothyBlynJacobs]

That looks great @johnwatkins0! Let's just split that test into two. One for checking against the string type, and another for the object type with properties.

[johnwatkins0]

Separate tests makes sense. Updated in 48785.4.diff, @TimothyBlynJacobs .

[TimothyBlynJacobs]

Thanks for refreshing @johnwatkins0, looks great!

[birgire]

The restapi group test on 48785.4.diff​ seems to run successfully on my install.

In 48785.4.diff​ I noticed the:

assertTrue( array_key_exists( ... ) )
assertFalse( array_key_exists(  ... ) )

I think alternatively one could use:

assertArrayHasKey
assertArrayNotHasKey

I don't think we need to use __() in test method (no locale assertions).

[TimothyBlynJacobs]

That’s a good point @birgire, do you want to add an updated patch?

[birgire]

Replying to TimothyBlynJacobs:

That’s a good point @birgire, do you want to add an updated patch?

Sure, thanks, 48785.5.diff​ uses

assertArrayHasKey
assertArrayNotHasKey

and also:

assertEqualSetsWithIndex

It also removes __() function dependency.

And adds @since 5.4.0 in the new test class file (+ minor inline docs additions).

[TimothyBlynJacobs]

In 47328:

REST API: Don't assume all item schemas have properties.

All schema types, not just objects, are permitted as the base type of a resource. A future patch could add validation support for those types, but this fix only prevents a PHP warning from being issued.

Props dhavalkasvala, johnwatkins0, birgire.
Fixes #48785.

[ajaaskel]

Would you please explain a little bit more. It's nice to hear "it's fixed" but it's not obvious for a system maintainer how to fix the thing. Where/how the fix is available?
I faced the problem when running search/replace at WP client software (a special tool for maintaining WordPress on command line). The purpose is to change the complete setup to a new domain. The tool does not change anything but gives up at that error.
A sample:

from_domain=domain1.com
to_domain=domain2.com

wp --path=/home/www/domain/"$our_domain"/html search-replace "$from_domain" "$our_domain"

PHP Warning: Invalid argument supplied for foreach() in /home/www/domain/domain2.com/html/wp-includes/rest-api/endpoints/class-wp-rest-controller.php on line 287
Warning: Invalid argument supplied for foreach() in /home/www/domain/domain2.com/html/wp-includes/rest-api/endpoints/class-wp-rest-controller.php on line 287

[TimothyBlynJacobs]

Hi @ajaaskel,

By "fixed" this means that the fix was included in the version of WordPress being worked on at the time. That is listed in the "Milestone:" section of the trac ticket.

So if you are running WordPress 5.4, you shouldn't see that warning any longer.