WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 3 years ago

#23099 closed feature request (wontfix)

Add JSON-RPC support using existing XML-RPC methods

Reported by: maxcutler Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.5
Component: XML-RPC Keywords:
Focuses: Cc:

Description

Many people have expressed interest in a JSON API for WordPress core. Until a full REST API can be implemented, the existing XML-RPC method implementations can be re-used by wrapping them in JSON-RPC serialization/de-serialization logic.

Attachments (1)

jsonrpc.diff (14.1 KB) - added by maxcutler 5 years ago.

Download all attachments as: .zip

Change History (45)

@maxcutler
5 years ago

#1 follow-up: @maxcutler
5 years ago

This is a prototype implementation that allows for pluggable serialization engines, and includes a simple JSON-RPC 2.0 server implementation as a proof-of-concept.

This works by changing the wp_xmlrpc_server class to no longer extend IXR_Server, and instead determine which server class to use in serve_request. If the request has 'Content-Type: application/json' header, it uses JSON-RPC, otherwise it uses normal XML-RPC. Plugins can use the filter to inspect the request and use their own serialization engines if they want.

If this approach is acceptable, then more work can be put into the JSON-RPC implementation, or else a third-party library can be used as a base like IXR was used for XML-RPC. But I didn't want to put in work unnecessarily, so feedback would be appreciated.

#2 @SergeyBiryukov
5 years ago

  • Version changed from trunk to 3.5

#3 follow-up: @wonderboymusic
5 years ago

Thanks for doing this. Just so I understand, 'wp_rpc_request_handler_class' can be a class that extends IXR_Server? I am not wildly in love with POST'ing JSON for read-only APIs, but it's a good forward step for WP.

For anyone who cares, here's a non-RPC/non-REST thing I made: https://github.com/staylor/json-dot-php

#4 in reply to: ↑ 3 @maxcutler
5 years ago

Replying to wonderboymusic:

Thanks for doing this. Just so I understand, 'wp_rpc_request_handler_class' can be a class that extends IXR_Server?

It can be any class with a constructor that takes a list of method callbacks.

I am not wildly in love with POST'ing JSON for read-only APIs, but it's a good forward step for WP.

This is absolutely necessary for security purposes. Otherwise you'd have to include username/passwords in the URL for GET requests, which makes them visible in server/proxy logs and other places that POST contents aren't logged.

Another enhancement would be to add more auth options, so that it can piggy-back on an admin cookie or use oauth or other options that don't include the password in plaintext. But that's for another ticket.

#5 in reply to: ↑ 1 @MikeSchinkel
5 years ago

  • Cc mike@… added

Replying to maxcutler:

This is a prototype implementation that allows for pluggable serialization engines, and includes a simple JSON-RPC 2.0 server implementation as a proof-of-concept.

+1. Nice idea.

#6 @wonderboymusic
5 years ago

This is absolutely necessary for security purposes.

I keep forgetting about the built-in methods, all of my experience using it has been exposing read-only data. So yes, that makes sense.

#7 follow-ups: @rmccue
5 years ago

I wonder, since this appears to be a stop-gap until we have a JSON API, why not just implement a JSON API straight out?

#8 in reply to: ↑ 7 ; follow-up: @maxcutler
5 years ago

Replying to rmccue:

I wonder, since this appears to be a stop-gap until we have a JSON API, why not just implement a JSON API straight out?

It's an enormous project, and there are many diverging opinions about how to go about doing it. Automattic has made an attempt, there are several plugins on WP.org with varying approaches; I've yet to see anything that would be suitable for core.

In the meantime, there are many existing XML-RPC clients (like the mobile apps) that would prefer to use JSON if they could to reduce over-the-wire bandwidth and because programming clients is easier than XML-RPC (JSON parsing >> XML parsing).

The method implementations have existed for several releases, so only the new request/response serialization layer has to be validated and tested. This should make it easy to include and support compared to an entire new API subsystem with a brand new implementation of post, taxonomy, user, etc. functionality.

#9 in reply to: ↑ 7 @nacin
5 years ago

Replying to rmccue:

I wonder, since this appears to be a stop-gap until we have a JSON API, why not just implement a JSON API straight out?

Because this means we avoid needing to design and create a new implementation from scratch, not to mention individual API methods. It's an easy win to allow people to use JSON instead of XML (which rightfully scares away pretty much everyone).

This wouldn't be pitched as a "New JSON API" in the release notes. It would be much softer, like "JSON-RPC support added to the traditional XML-RPC server".

#10 in reply to: ↑ 8 ; follow-ups: @rmccue
5 years ago

Replying to maxcutler:

In the meantime, there are many existing XML-RPC clients (like the mobile apps) that would prefer to use JSON if they could to reduce over-the-wire bandwidth and because programming clients is easier than XML-RPC (JSON parsing >> XML parsing).

My fear is that this will be called Good Enough and we'll never get a first-class JSON-based REST API.

As far as I know, there's been no real big push for this. Maybe it's something we want to get a group of people working on for 3.6 as a semi-major feature?

#11 in reply to: ↑ 10 @nacin
5 years ago

Replying to rmccue:

My fear is that this will be called Good Enough and we'll never get a first-class JSON-based REST API.

Let's not kid ourselves. This isn't even close to good enough.

As far as I know, there's been no real big push for this. Maybe it's something we want to get a group of people working on for 3.6 as a semi-major feature?

There have been a number of conversations, but it's too early to push for this. Definitely not something for 3.6.

#12 in reply to: ↑ 10 @maxcutler
5 years ago

Replying to rmccue:

As far as I know, there's been no real big push for this. Maybe it's something we want to get a group of people working on for 3.6 as a semi-major feature?

A proper REST API for core is a huge project, and while there have been some discussions about it, it's likely a year-plus effort of planning and implementation, which pushes it beyond 3.6. We need something in the interim, and JSON-RPC is a low-cost low-risk attempt to improve the lives of client devs.

I know nacin has been talking to people (including me) about REST API ideas, which I hope will result in some group discussions at some point in the not too distant future. But, in any case, a REST API is beyond the scope of this ticket and that discussion should take place elsewhere.

#13 @scribu
5 years ago

Previously: #14618

#14 follow-up: @scribu
5 years ago

I think it could be a good intermediate solution. Could someone provide a sample request and response?

#15 @markoheijnen
5 years ago

If this isn't for 3.6 then I guess this code should be a plugin and that we now start having public discussions about how a REST API in core should look like.

#16 in reply to: ↑ 14 ; follow-up: @maxcutler
5 years ago

Replying to scribu:

I think it could be a good intermediate solution. Could someone provide a sample request and response?

Here's a sample for getting a post: https://gist.github.com/4435628

#17 @scribu
5 years ago

That's actually not bad. Bonus points if we actually start using it ourselves in Core.

#18 @betzster
5 years ago

  • Cc j@… added

#19 in reply to: ↑ 16 @MikeSchinkel
5 years ago

Replying to maxcutler:

Here's a sample for getting a post: https://gist.github.com/4435628

Sooo much nicer to work with.

#20 @redsweater
5 years ago

If this idea is adopted you will probably want to put some thought into exposing to clients whether a particular WP installation supports the JSON endpoint or not. Maybe something to add to the (unfortunately named) wlwmanifest.xml file?

#21 @johnbillion
5 years ago

  • Cc johnbillion added

#22 @josephscott
5 years ago

  • Cc josephscott added

#23 @wycks
5 years ago

  • Cc bob.ellison@… added

#24 @scribu
5 years ago

#14618 was marked as a duplicate.

#25 @scribu
5 years ago

  • Keywords has-patch added

#26 @scribu
5 years ago

  • Keywords needs-patch needs-unit-tests added; has-patch removed
  • Milestone changed from Awaiting Review to 3.6

Nacin brought up a salient point about the patch: it changes the callback syntax from 'this:method_name' to array( $this, 'method_name' ) which will screw over plugins using the 'xmlrpc_methods' filter.

So, we need to fix that.

Also, some unit tests would be nice.

#27 @markoheijnen
5 years ago

Is this something that we really want to include in 3.6? I rather work on getting a RESTfull API for 3.7

#28 follow-up: @scribu
5 years ago

Well, the patch for this is mostly written, so it's not much effort to get it in.

If you really want to get a RESTfull API by 3.7, you'd better start now.

#29 in reply to: ↑ 28 @rmccue
5 years ago

Replying to scribu:

If you really want to get a RESTfull API by 3.7, you'd better start now.

Already have, https://gist.github.com/5022591d312952d1245a is the last version that I pushed up.

I still don't think we should implement a stop-gap like this patch. JSON-RPC looks much worse to me than XML-RPC since it seems like we're squashing two incompatible technologies together. It's also something that will need to be supported moving forward, which again, is not something I think we should do for something that should be replaced in the next release.

#30 @scribu
5 years ago

JSON-RPC looks much worse to me than XML-RPC since it seems like we're squashing two incompatible technologies together.

That's very hand-wavy. Any pragmatic arguments for why JSON-RPC is worse than XML-RPC?

#31 follow-up: @markoheijnen
5 years ago

For me it doesn't really matter why it is worse. We should not implement this in this way if we will get a RESTfull API in 3.7 or 3.8. We should look how we want stuff in the future instead of looking only to the next release.

We could make the current implementation a little bit different so that it can be extended with a plugin. So don't extend IXR_Server and change serve_request() part that Max has in his patch.

#32 in reply to: ↑ 31 ; follow-up: @maxcutler
5 years ago

Replying to markoheijnen:

For me it doesn't really matter why it is worse. We should not implement this in this way if we will get a RESTfull API in 3.7 or 3.8. We should look how we want stuff in the future instead of looking only to the next release.

Could you and rmccue put together a proposal for a REST API? I'd love to see a full breakdown of the work involved, the design, the testing plan, the apps you'll build to verify that it meets real-world needs, and a schedule for all of that.

Forgive me for being skeptical, but based on my experience with the core community over the past several releases, I have a truly hard time believing that you could land a proper REST API even by 3.8.

In the meantime, there's a demonstrable need for an API that is better than XML-RPC, and I feel JSON-RPC is a way to get there quickly with minimal code churn. As scribu said, if you have real technical or maintainability concerns, I'd love to hear them.

#33 follow-up: @redsweater
5 years ago

maxcutler: as the major proponent of the plan, it seems the onus is more on you to provide extensive evidence for why such a stop-gap solution should be implemented, than to put it on markoheijnen and rmccue to develop an elaborate plan for developing a proper JSON-based API.

Am I right that all the client benefits of the JSON-RPC wrapper you propose, short of the bandwidth savings, would be achieved by implementing a similar wrapper on the client side? Maybe that is the right area to focus on for the short term, to prototype how a JSON-based API should behave and give you the luxury of dealing in JSON for the majority of your client code, while exploring the mechanics of how the JSON API should ideally work.

My general impression of the JSON-RPC wrapper proposal on the server is that it's an example of "junking up" the code base for a short-term gain. I think that's the gist of the reaction you are getting from markoheijnen and rmccue as well. The onus is on proponents to argue that the benefits of this patch are so great that they override the stop-gap, junking-up impression it leaves on some of us.

#34 in reply to: ↑ 33 ; follow-ups: @maxcutler
5 years ago

Replying to redsweater:

My general impression of the JSON-RPC wrapper proposal on the server is that it's an example of "junking up" the code base for a short-term gain. I think that's the gist of the reaction you are getting from markoheijnen and rmccue as well. The onus is on proponents to argue that the benefits of this patch are so great that they override the stop-gap, junking-up impression it leaves on some of us.

Let's put some perspective on this: the supplied patch is only about 110 lines of code if you ignore the change in method declarations (re: scribu/nacin comment above). It also adds the ability for plugins to add whatever serialization format they might prefer: YAML, MessagePack, jsonp, plist, custom XML schemas, etc, etc.

To call that "junking up the code base" is a bit hyperbolic considering the state of the WordPress code base...

As someone who writes and maintains a WordPress XML-RPC client, you are probably familiar with the pains of properly implementing said APIs. If you'd like, I can pull up a list of several dozen bugs in the official WP mobile apps from just the last six months that have been caused by poor/buggy XML-RPC implementations.

The fact of the matter is that JSON is much easier to work with than XML for almost all modern development environments, from mobile to desktop to web. I've written WP XML-RPC clients for multiple languages, and it's a pain everywhere.

There are also many scenarios where it would be easier for WP plugins to use a JSON-RPC endpoint than to muck around with the current AJAX hooks. See #14618 for more discussion.

I agree with everyone that a "proper" RESTful API would be ideal, but the reality of the situation is that it's going to take a long time for it to land in core. In the intervening year+ of time, it would be so nice to make the lives of the mobile and plugin devs better by having a JSON-RPC API in core that can be depended on for talking to all WP.org sites.

That's just my 2 cents. My goal is simply to broaden WordPress' reach by making it easier to integrate with other systems, and in my experience XML-RPC is a really big negative point for outside developers considering such integrations. I understand that the core community's perspective and priorities may be different, but I want this to be a realistic and not an idealistic discussion.

#35 in reply to: ↑ 34 @redsweater
5 years ago

Replying to maxcutler:

To call that "junking up the code base" is a bit hyperbolic considering the state of the WordPress code base...

I'm sorry, you're right, that came across as more inflammatory than I meant. I was trying to paint a picture of the kind of mentality that estabslishes the basis for resisting changes like this that feel, for whatever reasons, like half-solutions.

You make a pretty good case for the utility of being able to specify the serialization method. Perhaps that is the right way to frame this patch, than it's not a stop-gap API solution but an affordance for allowing clients to communicate with the canonical WordPress API using alternate serializations.

#36 in reply to: ↑ 32 @rmccue
5 years ago

Replying to maxcutler:

Could you and rmccue put together a proposal for a REST API? I'd love to see a full breakdown of the work involved, the design, the testing plan, the apps you'll build to verify that it meets real-world needs, and a schedule for all of that.

I'd be happy to draw up a full plan for an API if there's consensus that we want it. At the moment I've merely been working on a proof of concept so that there's something to push forward.

The main gripe I have with this patch is that it will leave us supporting JSON-RPC far into the future. I'd be more than happy if the filters in serve_request() go in, as that would allow a plugin to easily add this if needed.

JSON isn't the solution to every problem, and if the problem with implementing XML-RPC in other languages lies with the XML parsing (I'd be surprised if libxml isn't available in those) then that's something that needs to be solved in those languages. Implementing RPC in JSON feels as if we're attaching wheels to a boat. It may have some utility, but in reality, I don't think it's useful.

#37 in reply to: ↑ 34 @scribu
5 years ago

Replying to maxcutler:

If you'd like, I can pull up a list of several dozen bugs in the official WP mobile apps from just the last six months that have been caused by poor/buggy XML-RPC implementations.

Perhaps you should, so we could see if the bugs are caused by XML or by something else.

#38 @MZAWeb
5 years ago

  • Cc wordpress@… added

#39 @aniketpant
5 years ago

  • Cc me@… added

#40 @jazbek
5 years ago

  • Cc j.yzbek@… added

#41 @nacin
5 years ago

  • Keywords 3.7-early added
  • Milestone changed from 3.6 to Future Release

Good discussion. Let's discuss again early 3.7.

#42 @wonderboymusic
4 years ago

  • Milestone changed from Future Release to 3.7

these are all marked 3.7-early

#43 @nacin
4 years ago

  • Milestone changed from 3.7 to Future Release

Seems like there is not a huge desire for this at this time.

#44 @danielbachhuber
3 years ago

  • Keywords needs-patch needs-unit-tests 3.7-early removed
  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed

WP-API all the way! Let's put our weight behind that.

Note: See TracTickets for help on using tickets.