[Cbrad24]

PR #564 introduces a HasApiVersion trait for API versioning, including a {version} placeholder in the URL with VersionMode::Url.

I tried to use this with Microsoft Graph's API, as it's a good fit for switching between Graph's v1.0 and beta APIs. The problem I ran into was that with VersionMode::Url, the connector's version always wins when it has one, and $request->setApiVersion() is ignored.

This is because the connector boots first and replaces {version} on its pass, so when the request's plugin boots there is no {version} left to replace:

• $connector->send($request) creates a PendingRequest.
• The connector plugin boots and replaces {version}.
• The request plugin boots and finds nothing to replace.
• The override on the request does nothing.

This PR adds a resolveUrlApiVersion method, which the connector uses when it replaces {version}. If the request also uses HasApiVersion in VersionMode::Url and has a version, the request's version wins, as #564's description puts it: "Prefer the request override when only that one call should differ." A request in VersionMode::Header or VersionMode::QueryParam keeps its version out of the URL. To support this, the trait gains a getVersionMode() getter.

New tests cover:

• a URL-mode request overriding the connector's version in the URL
• a header-mode request leaving the connector's version in the URL, with its own version sent in the header
• a request with its own getApiVersion() and getVersionMode() methods, but not the trait, leaving the connector's version in the URL

Only VersionMode::Url is affected by the boot order, since VersionMode::Header and VersionMode::QueryParam both use add() which replaces the connector's value under the same key.

Example failing code:

class GraphConnector extends Connector
{
    use HasApiVersion;

    public function __construct()
    {
        $this->setApiVersion('v1.0');
        $this->versionMode = VersionMode::Url;
    }

    public function resolveBaseUrl(): string
    {
        return 'https://graph.microsoft.com/{version}';
    }
}

class ListSignInsRequest extends Request
{
    use HasApiVersion;

    protected Method $method = Method::GET;

    public function __construct()
    {
        $this->versionMode = VersionMode::Url;
    }

    public function resolveEndpoint(): string
    {
        return '/auditLogs/signIns';
    }
}

(new GraphConnector)->send((new ListSignInsRequest)->setApiVersion('beta'));
// Sends https://graph.microsoft.com/v1.0/auditLogs/signIns, not /beta/

[JonPurvis]

Thanks for this @Cbrad24 🚀

The connector boots HasApiVersion before the request, so the connector replaces {version} first and the request never gets a chance.

I've left a few comment but nothing too major. If those could be addressed, that'd be great. Regarding a point you made in your PR:

Side note: #564 description lists UrlPath and Subdomain as enum cases, but the merged enum has a single Url case covering both.

This was a very late change made pre merge because UrlPath and Subdomain both involved replacing part of the URL, so we decided to just make one Url case to handle both.

[JonPurvis]

Small readability tweak: call a resolveUrlApiVersion($pendingRequest) helper here (see my next comment for possible implementation). Same behaviour you’re going for, we still substitute the request’s version when the connector boots first, just not inlined as getRequestApiVersion() ?? $this->apiVersion.

[JonPurvis]

Please could you rename this to resolveUrlApiVersion() so it’s more obvious it’s for the {version} placeholder, not header/query.

Couple of other suggestions too:

Only look at the request when the connector is booting ($pendingRequest->getConnector() === $this). When the request boots URL mode later, $this is already the request, so we can just use $this->apiVersion.

Only use the request’s version if the request is also in VersionMode::Url (needs a small getVersionMode() getter on the trait). If someone calls setApiVersion('beta') on a request that’s still in default Header mode, we should not change the URL path, only the header.

Detect HasApiVersion with Helpers::classUsesRecursive($request) instead of method_exists($request, 'getApiVersion'), so unrelated getApiVersion() methods don’t affect URLs.

[JonPurvis]

Something like:

/**
 * Resolve which version to substitute into `{version}` for URL mode.
 */
protected function resolveUrlApiVersion(PendingRequest $pendingRequest): string
{
    if ($pendingRequest->getConnector() === $this) {
        $request = $pendingRequest->getRequest();

        if (in_array(self::class, Helpers::classUsesRecursive($request), true)
            && $request->getVersionMode() === VersionMode::Url) {
            $requestVersion = $request->getApiVersion();

            if (is_string($requestVersion) && $requestVersion !== '') {
                return $requestVersion;
            }
        }
    }

    return $this->apiVersion;
}

Then a getter:

public function getVersionMode(): VersionMode
{
    return $this->versionMode;
}

[JonPurvis]

Could you add a test for this scenario?

Connector still uses VersionMode::Url with v1, but the request only overrides via header, makeVersionedRequest('beta', VersionMode::Header). Here setApiVersion('beta') must not change the URL; the path should stay https://api.provider.com/v1, and the header should be api-version: beta.

[Cbrad24]

Thanks @JonPurvis, all fixed.

Your snippet mostly worked out, just had to change self::class as that will reference the class that instantiates the trait, not the trait itself. Also added the extra test coverage.

[JonPurvis]

Thanks for sorting, @Cbrad24 👍

I'll be catching up with Sam next week, so this will most likely be merged and tagged then.

Cheers

[JonPurvis]

👍