Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#45635 closed defect (bug) (duplicate)

Rest-API broken on trunk

Reported by: hjanuschka's profile hjanuschka Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.0
Component: REST API Keywords:
Focuses: Cc:


current trunk, i think this commit is the culprit:

is broken, as it changes the "known" default behaviour, we have a quiet large installation at (600.000+ posts)

the commit - if we upgrade to 5.0 - would force use to find and modify ALL wp-json call's that we are making (which are a lot).

i can get the idea of _fields - but in that particular case it think features like this should be bumped in the v3 version namespace.

and about the _fields, maybe a graphql would be the better alternative, if one has problem's with rest.

Change History (5)

#1 @dlh
4 years ago

  • Component changed from General to REST API
  • Keywords reporter-feedback added
  • Version set to 5.0

@hjanuschka can you add more detail about what has broken as a result of [43736] and [43986]?

Do note that if the _fields parameter isn't specified, then all fields defined in the schema will be still returned (see #43874, which was added in 4.9.8).

#2 @hjanuschka
4 years ago

ok i bisect't it down (5.0.1 (good) to master), to exactly this commit.

in fact, commenting out the continue, where fields get removed, make my unittest's pass.

what we test:

  • load all plugins (alot)
  • create some post via WP_RestRequest
  • make a WPRestRequest
  • validate response fields
  • after, above commit, our registered fields are not returned anymore.

the thing is, we are not setting a schema definition on register_rest_fields - would be a huge refactor on our side (high potential for regressions).
however -> state's it is optional, and it does not say it will hide the field.

i can understand, that relying on the schema is a good way, however i beg you to find an alternate solution, since it is kinda BC (resulting from a non-strict past) like:

  • adding filter before continue wp_rest_response_fields($object, $fields)
  • also include non-schema fields. (unsure at first sight howto do)

let me know if there is acceptance for a change, and wich direction would be the best, and i am going to send a patch.

right now - we have stopped updating to 5.x - and evaluating options.


#3 @dlh
4 years ago

Thanks for providing those details, @hjanuschka.

At first glance, what you describe sounds similar to #45220, which was fixed in [43908]. That commit has now been merged into trunk in [44254]. Has the expected behavior been restored after [44254]?

#4 @hjanuschka
4 years ago

@dlh i can confirm that [43908] restores known behaviour. 🤘

i found a central place in my code where i could add like a "dummy" schema entry, forcing "title" => "dummy", type => "object"

is there any documentation, what side effects that could have, where schema is used, and what exactly for? i am a bit afraid introducing a huge regression doing this. but creating a schema for each field (many k's) is too much work-force!

#5 @dlh
4 years ago

  • Keywords reporter-feedback removed
  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Glad to hear it, @hjanuschka. I'll close this as a duplicate of #45220.

To your question, the best "official" documentation I know of is the REST API Handbook and its section on schema. The REST API team might know of other good external documentation. (Personally, I have also learned a bit by browsing how core uses the rest_sanitize_value_from_schema() and rest_validate_value_from_schema() functions.)

Note: See TracTickets for help on using tickets.