Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduces
ApiUrlResolver
trait that replaces the staticapi_root_url
argument for api clients.Up until now,
WpApiClient
has had the argumentapi_root_url: Arc<ParsedUrl>
which was used to built the endpoint urls with$api_root_url/$namespace/$endpoint
formula. The problem is that different WordPress hosts may have a different formulas. For example, WordPress.com useshttps://public-api.wordpress.com/$namespace/sites/$site_url/$endpoint
which is not possible to pass to the api client with just anapi_root_url
.To address this, here is the how the first iteration of
ApiUrlResolver
trait looks like:This function signature allows putting together the endpoint url in all sorts of different ways. For example, the
WpOrgSiteApiUrlResolver
which is the replacement for the older method looks like this:This does exactly what we used to internally do. However, here is the (shortened version of)
WpComDotOrgApiUrlResolver
which is a new addition that allowsWpApiClient
to be used for WordPress.com API:Notice how in this case we were able to add the
/sites/$site
part in the middle of the url.This PR also introduces the
WpComApiClientInternalUrlResolver
which is used byWpComApiClient
and looks very similar toWpOrgSiteApiUrlResolver
with the exception that, instead of theapi_root_url
, it uses the base wp.com REST API url:https://public-api.wordpress.com
. This resolver is kept private as it shouldn't be necessary to directly construct it or interact with it.The trait is available in Rust, Kotlin & Swift, so if someone wants to build an API client for a different WordPress host, they can easily implement a custom resolver. Also notice that even though Kotlin implementation now supports this, there is very minimal change to the wrapper. That's because by adding a second constructor, we can easily support the existing implementation without any change from the consumers.
For WordPress.com resolver, I've added panic statements to ensure that they are only used for their intended namespaces. We currently don't have a way to return a
Result
for url resolution and unless we end up needing it forwp_api
crate, I don't think we should introduce the extra complexity for the sake of WordPress.com. These panics should help developers avoid configuration errors, but it should never interfere with actual runtime. Having said that, if we don't like the idea of panics, we can convert them to warning logs and let the requests fail. Either way is fine with me.Note that this trait implementation introduces some minor memory inefficiency due to the trait being exposed through FFI. It might be possible to get around this, likely in exchange for increased complexity. I don't think this is worth worrying over at the moment, but I wanted to at least note it.