feat: add missing API methods, local-auth client_credentials support & bug fixes#93
Open
flovntp wants to merge 43 commits into
Open
feat: add missing API methods, local-auth client_credentials support & bug fixes#93flovntp wants to merge 43 commits into
flovntp wants to merge 43 commits into
Conversation
Add methods identified as missing from task classes after comparing
against the OpenAPI spec and low-level generated APIs:
- MetricsTask: inject HttpTrafficApi, BlackfireMonitoringApi and
ContinuousProfilingApi; add httpMetricsTimeline*, blackfireServer*
and getContinuousProfiling* methods
- CertificatesTask: getProvisioner(), listProvisioners(),
updateProvisioner()
- EnvironmentsTask: deploy(), maintenanceRedeploy()
- OrganizationsTask: listReferencedOrgs/Projects, queryCarbon,
listSubscriptions and related billing/MFA helpers
- ProjectsTask: DeploymentTarget CRUD, Alerts, DomainClaims, carbon
query, usage alerts
- RegionsTask: listReferencedRegions() via ReferencesApi
- RepositoriesTask: listGitDiffs() via DiffApi
- ResourcesTask: patchAutoscalerSettings()
- TeamsTask: listReferencedTeams()
- UsersTask: getCurrentUserDeprecated(), createProfilePicture(),
getAccessDocument(), listReferencedUsers()
UpsunClient: instantiate and inject HttpTrafficApi,
BlackfireMonitoringApi, ContinuousProfilingApi, DiffApi for the
updated task constructors.
fix(templates): add missing FormDataProcessor import and 'object' deserialize case
- api.mustache: add use {{invokerPackage}}\FormDataProcessor so all
generated API classes that use FormDataProcessor have the correct
import after regeneration
- ObjectSerializer.mustache: add case 'object': return (object)$data
in deserialize() switch to handle generic object return types
Fix FormDataProcessor missing import in UserProfilesApi (generated).
Fix ObjectSerializer missing 'object' case in deserialize() (generated).
Tests: add 31 new test methods across 11 existing test files to cover
all added methods; fix MetricsTaskTest, TaskContainersTaskTest and
SystemOperationsTaskTest which were mocking final classes.
636 tests pass, 0 failures.
Collaborator
🧾 API Coverage ReportLast updated: 10949ec • Run #247
📋 Full JSON report |
Upsun SDK checker reportDisplay raw output |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Upsun SDK checker reportDisplay raw output |
1 similar comment
Upsun SDK checker reportDisplay raw output |
Upsun SDK checker reportDisplay raw output |
Upsun SDK checker reportDisplay raw output |
Upsun SDK checker reportDisplay raw output |
Upsun SDK checker reportDisplay raw output |
… buffer, refreshEndpoint, re-entrance guard
FIX 1 — 401 retry middleware calls forceRefresh() instead of exchangeCodeForToken()
AbstractApi::sendAuthenticatedRequest() intercepts 401, calls
OAuthProvider::forceRefresh() and retries once (RFC 6750 §3.1).
FIX 2 — refreshAccessToken() sends Authorization: Basic header
Required for confidential clients per RFC 6749 §3.2.1.
FIX 3 — Proactive refresh buffer raised from 60 s to 120 s
Mitigates clock skew between client and auth server.
FIX 4 — Configurable refreshEndpoint (distinct from tokenEndpoint)
OAuthProvider accepts an optional refreshEndpoint parameter (default =
tokenEndpoint). UpsunClient passes auth_url + refresh_endpoint.
FIX 5 — Re-entrance guard (thundering-herd protection)
acquiringToken bool prevents recursive acquisition in PHP Fiber
contexts. Multi-process FPM protection requires external lock (out of scope).
Also adds:
- doAcquireToken(): prefers refresh_token grant, falls back to api_token
- forceRefresh(): clears access token without touching refreshToken or acquiringToken
- storeTokenData() now persists refresh_token and token_type from response
- 9 new unit tests covering all 5 fixes (567 tests, 3395 assertions — all green)
- Minor CI workflow label fixes and composer clean script
… remove backward-compat alias
…case tests, expand coverage to AbstractApi
doAcquireToken() already guards $this->refreshToken before calling refreshAccessToken(), making the inner check dead code (never reachable). Removing it achieves 100% line + method coverage on OAuthProvider. Also syncs template (oauth_provider.mustache) and commits phpunit.xml coverage-scope update (added src/Api/AbstractApi.php).
…double-retry Change 1 — Bearer-only mode + \Closure tokenProvider in AbstractApi - AbstractApi: replace OAuthProvider $oauthProvider with \Closure $tokenProvider - getAuthorizationHeader() → ($this->tokenProvider)() - refreshToken() → ($this->tokenProvider)() - All ~62 concrete API files: sed OAuthProvider → \Closure tokenProvider - api.mustache + abstract_api.mustache: updated to match - UpsunClient: $auth nullable (?OAuthProvider), only created when apiToken !== '' - new setBearerToken(string $token): void - getToken() three-way: auth → bearerToken → throw RuntimeException - $tokenProvider closure passed to all concrete APIs instead of $this->auth Change 2 — Fiber-based thundering-herd guard in OAuthProvider::ensureValidToken() - Replace bool $acquiringToken with ?\Fiber $acquiringFiber - Concurrent Fiber callers suspend (via \Fiber::getCurrent()?->suspend()) until the first acquisition completes, then return without a second HTTP call - In sync (FPM) contexts \Fiber::getCurrent() is null, so acquiringFiber is always null and the guard is a no-op — identical behaviour to the old bool flag Change 3 — $_retried guard in sendAuthenticatedRequest() prevents double forceRefresh - Add bool $_retried = false param; on 401 call tokenProvider(true) then recurse with $_retried=true so a second 401 goes straight to ApiException without a redundant force-refresh call Tests: - AbstractApiTest: rewritten to use closure tracking (tokenCallCount/forceRefreshCount) instead of OAuthProvider mock; 401-retry $_retried guard verified - UpsunClientTest: removed testGetTokenReturnsApiToken (now HTTP-backed); added testGetTokenDelegatesToOAuthProvider, testGetTokenReturnsBearerToken, testGetTokenThrowsWhenNoAuthMethodSet - OAuthProviderTest: replaced reflection-based acquiringToken test with proper Fiber concurrency test (testFiberGuardPreventsDoubleAcquisitionUnderConcurrency) - BaseTestCase + all ~25 TaskTests: OAuthProvider mock → \Closure tokenProvider Result: 584/584 tests pass, lint clean
…uisition Replaces bare Closure with a proper TokenProvider interface, aligning naming with the Node SDK's `type TokenProvider = (force?: boolean) => string`. - src/Core/TokenProvider.php: new interface with __invoke(bool $force=false): string - OAuthProvider: implements TokenProvider, adds __invoke() covering force-refresh path - AbstractApi: TokenProvider type hint replacing Closure - UpsunClient: $tokenProvider declared as named variable before $taskParams array (facade does not implement the interface — clean separation of concerns) - All 62 concrete API files + mustache templates updated - All tests updated: anonymous class implements TokenProvider (Closure != interface in PHP) - 25 task test files: multi-line anonymous class format (PHPCS compliant)
…d file
Alphabetical use-statement order (matching PHPCS rule) was broken after
introducing TokenProvider — `use {{invokerPackage}}\Core\TokenProvider;`
was sitting between JsonException and Http\ imports.
Now matches the order produced by phpcbf on AbstractApi.php.
…code fix:all (phpcbf + rector + php-cs-fixer) applied: - OAuthProvider: remove redundant `use Upsun\Core\TokenProvider;` (same namespace) - 25 task test files: FQNS `\Upsun\Core\TokenProvider` → `use` import + short name oauth_provider.mustache synced with OAuthProvider.php: - use Fiber; import (was \Fiber FQNS) - class OAuthProvider implements TokenProvider - short Fiber name throughout (not \Fiber) - __invoke(bool $force = false): string method added
…lsafe instance call
Fiber::suspend() is a static method. Calling it via a nullsafe instance
reference ($fiber?->suspend()) is technically valid but a PHP anti-pattern
(calling static methods on instances). The correct form is:
if (Fiber::getCurrent() !== null) { Fiber::suspend(); }
This is explicit about the guard intent and avoids potential deprecation
warnings from static-via-instance call patterns.
Add methods identified as missing from task classes after comparing
against the OpenAPI spec and low-level generated APIs:
- MetricsTask: inject HttpTrafficApi, BlackfireMonitoringApi and
ContinuousProfilingApi; add httpMetricsTimeline*, blackfireServer*
and getContinuousProfiling* methods
- CertificatesTask: getProvisioner(), listProvisioners(),
updateProvisioner()
- EnvironmentsTask: deploy(), maintenanceRedeploy()
- OrganizationsTask: listReferencedOrgs/Projects, queryCarbon,
listSubscriptions and related billing/MFA helpers
- ProjectsTask: DeploymentTarget CRUD, Alerts, DomainClaims, carbon
query, usage alerts
- RegionsTask: listReferencedRegions() via ReferencesApi
- RepositoriesTask: listGitDiffs() via DiffApi
- ResourcesTask: patchAutoscalerSettings()
- TeamsTask: listReferencedTeams()
- UsersTask: getCurrentUserDeprecated(), createProfilePicture(),
getAccessDocument(), listReferencedUsers()
UpsunClient: instantiate and inject HttpTrafficApi,
BlackfireMonitoringApi, ContinuousProfilingApi, DiffApi for the
updated task constructors.
fix(templates): add missing FormDataProcessor import and 'object' deserialize case
- api.mustache: add use {{invokerPackage}}\FormDataProcessor so all
generated API classes that use FormDataProcessor have the correct
import after regeneration
- ObjectSerializer.mustache: add case 'object': return (object)$data
in deserialize() switch to handle generic object return types
Fix FormDataProcessor missing import in UserProfilesApi (generated).
Fix ObjectSerializer missing 'object' case in deserialize() (generated).
Tests: add 31 new test methods across 11 existing test files to cover
all added methods; fix MetricsTaskTest, TaskContainersTaskTest and
SystemOperationsTaskTest which were mocking final classes.
636 tests pass, 0 failures.
Upsun SDK checker reportDisplay raw output |
Upsun SDK checker reportDisplay raw output |
Upsun SDK checker reportDisplay raw output |
1 similar comment
Upsun SDK checker reportDisplay raw output |
Upsun SDK checker reportDisplay raw output |
flovntp
changed the title
Upsun SDK checker reportDisplay raw output |
Upsun SDK checker reportDisplay raw output |
Upsun SDK checker reportDisplay raw output |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Summary
This PR completes the Upsun API coverage in the PHP SDK: it adds the missing methods across several
Taskclasses, introduces a newclient_credentialsauthentication mode (local in-container token service), hardens the OAuth/auth layer, and fixes several bugs. It also includes a full SDK regeneration and a significant increase in test coverage.What's new
🔌 API coverage
Taskclasses (Projects, Organizations, Metrics, Resources, SupportTickets, Teams, Users, Certificates, SystemOperations, TaskContainers, …).->taskContainersaccess on the client.ResponseObjectinitialization.🔐 Authentication
client_credentialsmode, auto-enabled viaUpsunConfig::LOCAL_AUTH(local token service, e.g. inside an Upsun container).x-token-ttlheader (clamped to 60–900 s) via thetokenTtlparameter.TokenProviderinterface (typed token acquisition).refresh_token/forceRefreshwith a 120s buffer.APIConfiguration→ApiConfiguration(backward-compat alias removed).🐛 Bug fixes
ResourcesTask::updateAutoscalerSettings(): no moreTypeErrorwhen$servicesis omitted.SupportTicketsTask::listCategories()/listPriorities(): correct handling ofnullarguments (no moreparse_url(null)nor validation on a null id).TeamsTask::grantTeamProjectAccessToTeam(): removed a duplicatecheckProjectIdvalidation (wrong error message).ApiExceptionhandler.📝 Docs & templates
oauth_provider,abstract_api,README,ApiException, …) with the generated code.✅ Tests
Taskclasses and the OAuth layer.phpcsexit 0.Notes
APIConfigurationwas renamed toApiConfigurationwith no backward-compat alias.src/Model/*files) comes from the automatic SDK regeneration.