Skip to content
This repository was archived by the owner on Sep 24, 2018. It is now read-only.

Conversation

@rmccue
Copy link
Member

@rmccue rmccue commented May 24, 2015

This is an alternate approach to #577 that we discussed.

Right now, we have the ability to take a site and discover whether it has the API enabled (and where it is). From there, you can see what routes are available on the site, but you can't actually tell what the site supports. For example, how do I know if the site has WooCommerce enabled? How do I know what versions of the API are supported?

In #577, we talked about a possible solution to this, which is to list out the core objects and where they're available in the API. This would allow for some feature detection based on what objects are available. However, this makes it a bit of a pain to actually use, since you still might not be able to understand the API. It doesn't really solve the core problem of discovery.

This PR tackles it from a different angle, and uses the new concept of namespaces that we have in version 2. The index now contains a list of the available namespaces, and namespaces each have their own index (that you can override with your own index if needed). This allows for simple feature detection.

Let's say we later come out with version 3 of the REST API, and you want to assess what's available on the site. You can grab the index and check namespaces for wp/v3, which would indicate it's supported on the site. If that's not there, you could then fall back to version 2, and check for wp/v2 instead.

You could do the same for checking for plugins. Simply check namespaces for woocommerce or woocommerce/v2 or something like that.

@rmccue
Copy link
Member Author

rmccue commented May 24, 2015

(Needs some unit tests.)

@rachelbaker
Copy link
Member

@rmccue with test coverage, I am +1 on this.
What do you think about including a public method for listing all registered namespaces? I didn't see a way to do that.

@rachelbaker
Copy link
Member

Clarifying:

What do you think about including a public method for listing all registered namespaces? I didn't see a way to do that.

I see how the namespaces are listed in the index (pasted below), and that is great for external clients. I am asking if it would make sense to include a public get_all_namespaces() method for internal (plugins, themes, etc) use?

{
"name": "Keeping it Local",
"description": "Just another WP Sessions Demo",
"url": "http://local.wpapi.dev",
"namespaces": [
"wp/v2",
"sessions/v1"
],
"authentication": [],
...
}

@rmccue
Copy link
Member Author

rmccue commented May 25, 2015

Great idea. :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielbachhuber This should help with #1262, since now you can specify "options" on the routes too. Should fix the repetition of the schema.

@rachelbaker
Copy link
Member

@rmccue ready for #reviewmerge?

@rmccue
Copy link
Member Author

rmccue commented May 25, 2015

@rachelbaker Still needs tests :)

@rmccue
Copy link
Member Author

rmccue commented May 25, 2015

Now we're ready to go. :)

@WP-API/amigos #reviewmerge

rachelbaker added a commit that referenced this pull request May 25, 2015
@rachelbaker rachelbaker merged commit f961453 into develop May 25, 2015
@rachelbaker
Copy link
Member

Merged #1283

@rachelbaker rachelbaker deleted the list-namespaces branch May 25, 2015 21:30
@sc0ttkclark
Copy link

Code missing phpDoc on new parameter added to register_route

@rmccue
Copy link
Member Author

rmccue commented May 28, 2015

Good catch, thanks.

@sc0ttkclark
Copy link

Heyyy I got you this time, get me next time ;)

@rmccue
Copy link
Member Author

rmccue commented May 28, 2015

Actually! Worth mentioning that namespace is meant to be an internally-used parameter, since it's set for you. Setting it in the route registration itself won't work, unless you're directly filtering. Maybe we explicitly don't want to document it because of this.

@sc0ttkclark
Copy link

Always document, even internal IMO

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants