Repository navigation
Conversation
mridang
force-pushed
the
feat/better-enhanced-sdks
branch
from
June 13, 2026 11:28
06849e1 to
fde8eea
Compare
Contributor
Qodana for PHP2 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at qodana-support@jetbrains.com
|
mridang
force-pushed
the
feat/better-enhanced-sdks
branch
2 times, most recently
from
June 13, 2026 13:19
a17022e to
5b7276b
Compare
mridang
changed the title
mridang
force-pushed
the
feat/better-enhanced-sdks
branch
9 times, most recently
from
June 14, 2026 13:59
bf254ab to
ddf46c0
Compare
Regenerate the client from openapi-generator-plus with modernized templates, authenticators ported to the new interfaces, and house tooling aligned to the generator's output. BREAKING CHANGE: new generated API surface, a raised minimum runtime, and updated dependencies; not source-compatible with the prior release.
Reorganize .openapi-generator-ignore into the shared eight-section layout and sort each section alphabetically. Also fix the facade to use the generated TransportOptions constructor instead of the removed ::defaults() helper.
Move the bespoke auth tests, ZitadelTest, and fixtures from test/ into tests/ so there is a single test root matching the generator's emitted unit tests. Wire mrpunyapal/peststan into phpstan.neon and drop the test-dir exclusion so the suite is now statically analysed.
…am) and stale test path
The twelve generators had drifted into twelve dialects describing the same SDK. Each sentence now has one wording across every language, varying only in a token the language genuinely calls by another name.
The package manifest declared MIT beside an Apache-2.0 LICENSE file. It is maintained by hand, so the generator could not correct it; it now declares Apache-2.0, and proc.yml records the same for the generated metadata.
Adds the network and timeout errors, drops the one-off error types and keeps the Zitadel root name through errorPrefix.
OpenID discovery and token requests now run lazily through the injected ApiClient. Caller mistakes raise the built-in argument or state error, a rejected token request OAuth2Server, an unusable token response OAuth2Token. The private-key spec now uses the local stack instead of zitadel.cloud.
Every SDK error now lives in Zitadel\Client\Errors, so the hand-written auth, tests and specs import from there. Drop the local OAuth2 exception copies now that the generator always emits them, and let OpenId discovery raise through ApiException::fromResponse instead of keeping its own status table. Add the open-telemetry/sdk dev dependency and the proxy-auth fixture the regenerated transport tests need.
The repo carried no contributor guidance, so the keep-list, the error contract and where a fix belongs were only discoverable by reading code.
Errors, client contract and configuration now match the other eleven languages; the process-wide default configuration is gone.
The generator now records which dev dependencies its tests import, and this repo keeps its own package manifest, so the two must stay in step.
…anifest The generator now records which dev dependencies its tests import, and its linter configs apply to the generated tree instead of being kept locally.
Wait for both 3128 and 3129 to listen before reading their mapped ports so the bootstrap no longer aborts on an empty getMappedPort(3129), and add the 407 and credentialed proxy tests.
…ts auth port The proxy fixture failed in CI: squid's auth port (3129) never became available, so both the ZitadelTest suite and the Setup bootstrap timed out. ubuntu/squid declares VOLUME /var/log/squid and /var/spool/squid and drops to the unprivileged proxy user before opening its logs; the other SDKs mount both as tmpfs so the log daemon can start. testcontainers-php has no tmpfs helper, so the container is extended with one to match the siblings. The port read now blocks on squid's own "authport" readiness line instead of a hand-rolled inspect poll, which also removes the getMappedPort-on-null and Docker-inspect union-type errors that failed phpstan.
Waiting on squid's readiness log was not enough: getMappedPort() memoises the first inspect it reads, and on CI that snapshot is taken while the container is already running but before Docker has surfaced the published host ports, so the mapped port stayed empty and aborted the suite. Poll a fresh StartedGenericContainer each iteration until both ports are published and accepting connections; the tmpfs mounts keep squid alive so the bindings actually appear. Squid's logs are surfaced if the ports never do.
Squid is confirmed up on both ports in CI, but the mapped-port wait still times out. Record whether getMappedPort returned a host port and whether that port accepts a TCP connection, so the failure says which of the two it is instead of only dumping squid's logs.
The root cause of the CI-only proxy failure: testcontainers-php sets host
PortBindings but never Config.ExposedPorts, so the daemon only publishes ports
the image already EXPOSEs. ubuntu/squid exposes 3128 but not the 3129 auth port,
so on CI's strict daemon the 3129 binding was dropped (getMappedPort returned
empty); local Docker Desktop published it anyway, which hid the bug.
Override createContainerConfig to expose every requested port. An empty
ExposedPorts item serialises to [] and the daemon wants {}, so a single ignored
entry forces object serialisation.
…y diagnostics The 407 test asserted the base ApiException; tighten it to ClientException, the 4xx bucket the transport actually raises, matching the other SDKs. Drop the one-off dumpPorts diagnostic and verbose state string so the port wait matches the lean helper in spec/Setup.php.
…rounds
The proxy fixture pinned testcontainers 1.0.8, which lacked withTmpfs and
serialised an empty ExposedPorts item as [] (the daemon wants {}). That forced a
hand-rolled tmpfs Mount and a dummy-key ContainerConfigExposedPortsItem. 1.1.0
adds withTmpfs and wraps ExposedPorts values in a JsonObject, so both fixtures
now use native withTmpfs and set ExposedPorts to empty arrays, matching the
generator's golden bootstrap. Behaviour is unchanged; the suite still passes.
Setup started a wiremock+squid proxy and published PROXY_URL/PROXY_AUTH_URL, but nothing reads them: ZitadelTest starts its own containers and the sanity specs do not use a proxy. So the unit suite stood up two proxies and used one. Remove the dead fixture so the extension only loads .env and wires the JUnit reporter, leaving ZitadelTest's setUpBeforeClass as the single place that manages the proxy containers, matching the java, dotnet, ruby and node SDKs.
composer test filtered to --testsuite unit, so the docker-compose-backed sanity specs (Session/User service checks) never ran in CI even though the Setup bootstrap and AbstractIntegrationTest bring the stack up themselves. Run both suites like the other SDKs' single test command does.
mridang
force-pushed
the
feat/better-enhanced-sdks
branch
from
September 28, 2026 00:37
ecc7a59 to
dfc84ac
Compare
Rename the four sanity-check tests to the concise form the other SDKs use so all six read line for line.
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
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.
Regenerates the SDK from the enhanced openapi-generator-plus, with bespoke authenticators ported to the new interfaces and full unit + integration suites passing locally.
Closes #21
Closes #58