Skip to content

Introduces ApiUrlResolver #707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Draft

Introduces ApiUrlResolver #707

wants to merge 2 commits into from

Conversation

oguzkocer
Copy link
Contributor

@oguzkocer oguzkocer commented May 2, 2025

Introduces ApiUrlResolver trait that replaces the static api_root_url argument for api clients.

Up until now, WpApiClient has had the argument api_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 uses https://public-api.wordpress.com/$namespace/sites/$site_url/$endpoint which is not possible to pass to the api client with just an api_root_url.

To address this, here is the how the first iteration of ApiUrlResolver trait looks like:

#[uniffi::export(with_foreign)]
pub trait ApiUrlResolver: Send + Sync {
    fn resolve(&self, namespace: String, endpoint_segments: Vec<String>) -> Arc<ParsedUrl>;
}

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:

#[uniffi::export]
impl ApiUrlResolver for WpOrgSiteApiUrlResolver {
    fn resolve(&self, namespace: String, endpoint_segments: Vec<String>) -> Arc<ParsedUrl> {
        Arc::new(
            self.api_root_url
                .by_extending_and_splitting_by_forward_slash(
                    [namespace].into_iter().chain(endpoint_segments),
                )
                .into(),
        )
    }
}

This does exactly what we used to internally do. However, here is the (shortened version of) WpComDotOrgApiUrlResolver which is a new addition that allows WpApiClient to be used for WordPress.com API:

impl ApiUrlResolver for WpComDotOrgApiUrlResolver {
    fn resolve(&self, namespace: String, endpoint_segments: Vec<String>) -> Arc<ParsedUrl> {
        Arc::new(
            self.base_url
                .by_extending_and_splitting_by_forward_slash(
                    vec![namespace, "sites".to_string(), self.site_url.to_string()]
                        .into_iter()
                        .chain(endpoint_segments),
                )
                .into(),
        )
    }
}

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 by WpComApiClient and looks very similar to WpOrgSiteApiUrlResolver with the exception that, instead of the api_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 for wp_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.

@oguzkocer oguzkocer added the Rust label May 2, 2025
@oguzkocer oguzkocer changed the title Adds ApiUrlResolver Introduces ApiUrlResolver May 2, 2025
@oguzkocer oguzkocer requested a review from a team May 3, 2025 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant