WordPress.org

Make WordPress Core

Opened 5 months ago

Last modified 3 months ago

#51757 new enhancement

REST API: Add const validation

Reported by: ajlende Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: REST API Keywords: has-patch json-schema has-unit-tests
Focuses: Cc:

Description

The const keyword is a useful shorthand for pattern matching with oneOf.

Use of this keyword is functionally equivalent to an "enum" with a single value.

See https://json-schema.org/draft/2019-09/json-schema-validation.html#rfc.section.6.1.3

Attachments (1)

51757.patch (2.0 KB) - added by ajlende 5 months ago.

Download all attachments as: .zip

Change History (9)

@ajlende
5 months ago

#1 @ajlende
5 months ago

  • Keywords has-patch added

#2 @TimothyBlynJacobs
5 months ago

  • Keywords json-schema added

Thanks for the ticket @ajlende! I totally agree with the value of supporting this keyword.

The one thing to consider, though, is that we advertise as JSON Schema v4. const is a v6 keyword. There are breaking changes in v6. So I'm not sure what this process would look like yet.

https://json-schema.org/draft-06/json-schema-release-notes.html

#3 @desrosj
5 months ago

  • Version trunk deleted

#4 @ajlende
5 months ago

I didn't realize that it was a v6 thing. In my opinion, we're already not strictly spec compliant for v4—there are gotchas like assuming the top-level array is properties, assuming 'type' => 'object' at the top-level, and using the required boolean for individual properties—so I don't see any harm in adding a keyword which will eventually be spec compliant when we already have these quirks.

However, it would be nice if we did implement the $schema keyword and require a value of http://json-schema.org/draft-04/schema# for a spec-compliant version of v4 and http://json-schema.org/draft-06/schema# for things with v6 features like const.

I trust your decision making on whether or not to add it, and I can still just use the enum keyword, so this is in no way a blocker for anything.

#5 @TimothyBlynJacobs
5 months ago

we're already not strictly spec compliant for v4—there are gotchas like assuming the top-level array is properties, assuming 'type' => 'object' at the top-level

An important thing about this, is that this assumption is only baked into rest_get_endpoint_args_for_schema(). Our underlying schema validators don't make this assumption.

and using the required boolean for individual properties

Yeah this is the v3 syntax. We do support the v4 syntax now at a nested level. But I'd like to support it at the controller schema level too.

One of the things I'd like to get to is for the route to actually be aware of the schema as an args definition. Right now we validate using whatever is in args and that only contains the properties definition. So we don't have the $schema keyword available, or the actual top level type of the resource, etc... If we were able to capture that properly in the schema, we could then make rest_validate_value_from_schema and friends aware of the schema version.

I'd like to try and tackle that in WP 5.7. Both so we could explore things like v6 features, but also so we could support some really important v4 features like $ref which also require the whole schema as context.

This ticket was mentioned in PR #919 on WordPress/wordpress-develop by yakimun.


3 months ago

  • Keywords has-unit-tests added

Added support for the const JSON Schema keyword.

Trac ticket: https://core.trac.wordpress.org/ticket/51757

#7 @yakimun
3 months ago

The const keyword acts in the same way as enum with a single value.

Our implementation of validation of enum was improved in #51911 to handle values of different types correctly. These changes bring the new function rest_are_values_equal, which correctly checks the equality of two values.

I suggest implementing const validation using the rest_are_values_equal because it will provide more consistent results. The implementation of such a proposal is attached as a PR.

#8 @TimothyBlynJacobs
3 months ago

Thanks for the patch @yakimun. I'm going to try and take a look at this for 5.8 to see how we might handle supporting JSON Schema v6.

Note: See TracTickets for help on using tickets.