Skip to content

Use the request's API version in URL mode when the connector has one #567

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.

Sign up for GitHub

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

Open
Cbrad24 wants to merge 1 commit into
base: v4
Choose a base branch
from
Open

Use the request's API version in URL mode when the connector has one #567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/Traits/Plugins/HasApiVersion.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,21 @@ public function bootHasApiVersion(PendingRequest $pendingRequest): void
VersionMode::Header => $pendingRequest->headers()->add($this->versionKey, $this->apiVersion),
VersionMode::QueryParam => $pendingRequest->query()->add($this->versionKey, $this->apiVersion),
VersionMode::Url => $pendingRequest->setUrl(
str_replace('{version}', $this->ensureVersionIsSafeForUrl($this->apiVersion), $pendingRequest->getUrl())
str_replace('{version}', $this->ensureVersionIsSafeForUrl($this->getRequestApiVersion($pendingRequest) ?? $this->apiVersion), $pendingRequest->getUrl())

@JonPurvis JonPurvis Oct 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

),
};
}

/**
* Get the request's API version, which takes precedence in the URL because the connector boots first and replaces the placeholder before the request can.
*/
protected function getRequestApiVersion(PendingRequest $pendingRequest): ?string
{
$request = $pendingRequest->getRequest();

return method_exists($request, 'getApiVersion') ? $request->getApiVersion() : null;
}
Comment on lines +72 to +77

@JonPurvis JonPurvis Oct 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 JonPurvis Oct 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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;
}


/**
* Ensure the version can only ever form a single subdomain label or path segment.
*
Expand Down
13 changes: 13 additions & 0 deletions tests/Unit/Concerns/HasApiVersionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,19 @@
return true;
});
});

test('the request can override the connector api version in the url', function () {
$mockClient = sendVersionedRequest(
makeVersionedConnector('https://api.provider.com/{version}', VersionMode::Url, 'v1'),
makeVersionedRequest('beta', VersionMode::Url),
);

$mockClient->assertSent(function (Request $request, $response) {
expect($response->getPendingRequest()->getUrl())->toBe('https://api.provider.com/beta');

return true;
});
});

@JonPurvis JonPurvis Oct 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

});

describe('Without A Version', function () {
Expand Down
Loading