Release/2.1.0 - #16
Merged
Merged
Conversation
…14 events This bundles the work accumulated for the v2.1.0 pre-release, validated together via the full CI suite (1067 tests, 0 failures): Response emission model (bug fix): - Response::json()/text()/html() no longer auto-emit. Emission now happens exactly once, at Application::run() — the only place that's guaranteed to run after the full middleware pipeline (including post-handler middleware) has finished. Previously, auto-emit inside the handler sent bytes before post-handler middleware could act on the response, and every request logged a spurious "body already sent" warning from the redundant emit() call in run(). - Application::run() now guards its emit() call with $response->isSent(). - set_exception_handler (registered in configureBasicErrorHandling() and configureErrorHandling()) previously discarded handleException()'s return value, relying entirely on the auto-emit side effect to get the error response to the client. Now emits explicitly — otherwise removing auto-emit would have silently swallowed error responses for exceptions that escape the handle()/run() flow entirely (e.g. bootstrap errors). - disableAutoEmit() kept as a no-op for backward compatibility with existing callers. v2.1.0 deprecation cycle (docs/technical/DEPRECATION_AND_REMOVAL_PLAN.md, ITEM-001 through ITEM-008, all completed 2026-05-29): - Core\Container, Request::getIp(), LoadShedder, Performance\RateLimitMiddleware and Str::startsWith/endsWith/contains marked @deprecated with trigger_error(E_USER_DEPRECATED), scheduled for removal in v3.0.0. - Providers\Logger replaced by Logging\PsrLogger (PSR-3); Providers\Logger kept as a deprecated shim. Dead Logging\Logger/FileHandler/ LogHandlerInterface removed. - Providers\EventDispatcher and Providers\ListenerProvider deprecated in favor of Events\EventDispatcher (now PSR-14 compliant) and Events\ListenerProvider. Other fixes captured in tasks/ (2026-05-29): Request body-parsing JSON array fallback, HeaderRequest missing strict_types, Validator required false-negative on integer zero, Psr7Pool header-iteration mutation on reset, static cached-input leaking across requests, missing CustomHeaderCollection import in Request. Housekeeping: react/http moved from require to suggest (only needed by the pivotphp/reactphp extension); obsolete v1.1.4 examples removed; tests/Services/* merged into tests/Http/*; ContainerTestSimple removed (superseded by ContainerTest).
…ch() task Confirmed HookManager doesn't touch fire()/listen() at all — it manages its own prioritized listeners and wires them into ListenerProvider directly via PSR-14, dispatching through the Hook event class. fire()/listen() remain a deliberately separate, lightweight string-based mechanism, but had zero test coverage until now (grep confirmed no production caller either). - Added tests/Events/EventDispatcherTest.php: covers dispatch() via ListenerProviderInterface, fire()/listen() independently, propagation stop on false return, and confirms the two listener sets don't leak into each other. - Documented the dispatch(string,array) -> fire(string,array) rename (breaking since 2.0.0, no BC alias exists — dispatch() must stay PSR-14-only per EventDispatcherInterface) in CHANGELOG.md. - Marked tasks/2026-05-29-event-dispatcher-*.md acceptance criteria resolved/explained.
Psr7Pool::resetServerRequest() received $serverParams but never applied it — ServerRequestInterface (PSR-7) doesn't define a with*() method for server params since they're meant to be read-only, so there was no way to reset them on a reused instance. Headers were already being cleared/reapplied correctly (previous fix), but server params (REMOTE_ADDR, HTTPS, auth data injected by the SAPI, etc.) from the previous request could leak into the next one served by the same pooled object — a real data-leak risk in concurrent/pooled environments (Swoole, ReactPHP, FrankenPHP). Added ServerRequest::withServerParams() (practical extension beyond the formal PSR-7 interface, same pattern as the existing withCookieParams()) and wired it into resetServerRequest(). Covered by Psr7PoolTest::testResetServerRequestDoesNotLeakHeadersOrServerParamsBetweenReuses(). Also updated docs/technical/INCONSISTENCIES_REPORT.md: marked C-01 (dual IoC containers) as resolved via the v2.1.0 deprecation cycle, C-02 (getCachedInput bypass in parseBody) as already fixed, and C-03 (this fix) as resolved.
Audited all 6 high-impact items against current code: - A-01 (three incompatible rate limiters): resolved — RateLimitMiddleware and LoadShedder already @deprecated v2.1.0 in favor of RateLimiter. - A-02 (MiddlewareStack::warmupCommonPipelines() calling undefined Response::setHeader()): resolved — the method no longer exists. - A-03 (Request::ip() vs getIp(), the latter unvalidated and spoofable via X-Forwarded-For): resolved — getIp() is deprecated and now delegates to ip(), inheriting its validation. - A-04 (ApiDocumentationMiddleware instantiating the Express.js Response instead of a PSR-7 type): resolved — it uses Psr7Response directly now. - A-05 (Validator::FILTER_VALIDATE_INT truthiness bug rejecting 0): resolved — already uses strict `=== false` comparison. - A-06 (MiddlewareStack's 5 static properties unsafe under async servers): was undocumented — added an explicit warning to the class docblock covering which properties are affected, the practical impact under Swoole/ReactPHP/FrankenPHP, and that clearCache() must be called explicitly between requests in those environments. The recommendation only asked for documentation, not a refactor to instance state — that's a larger architectural change out of scope here. Every item's report entry now has a Status note recording what was verified and why. No behavior changes except A-06's docblock addition.
…r closure (B-04, B-05) B-05: handleException() hardcoded `$statusCode === 404 ? 'Not Found' : 'Internal Server Error'` for the production (non-debug) error message — any status other than 404 (403, 401, 400, 405, 503...) always showed "Internal Server Error", discarding the exception's real HTTP semantics. Response::error() already had a proper status->message map, just not reusable. Extracted it into Response::defaultErrorMessage(), used by both error() and Application::handleException(). Covered by testExceptionHandlingProductionModeUsesStatusSpecificMessage() — it would have failed against the old hardcoded ternary. B-04: configureBasicErrorHandling() and configureErrorHandling() each registered set_exception_handler() with an identical anonymous closure (handleException() + isSent()-guarded emit()). Extracted into a public handleUncaughtException() method, now unit-testable in isolation (testHandleUncaughtExceptionEmitsErrorResponse()) instead of being an anonymous closure nobody could exercise directly. Also updated docs/technical/INCONSISTENCIES_REPORT.md: A-06 and A-07 were already fully resolved in earlier commits on this branch but the status table still said "Pendente Sprint 4" / "Parcialmente Corrigido" — corrected both entries, plus B-04 and B-05.
…-07) ExtensionManager held a hard dependency on Application, but never actually calls any Application-specific method — it only stores the reference and passes it along to registered extension callbacks/classes, which are the ones that use the real API surface. Added Core\ApplicationInterface as a marker interface (intentionally empty: there's no concrete method ExtensionManager itself needs) and made Application implement it. ExtensionManager now type-hints against the interface instead of the 1000+ line concrete class. Runtime behavior is unchanged — the object passed to extensions is still the real Application instance with its full API, only the declared type inside ExtensionManager changed.
Http\Pool\PoolManager (instance-based, generic rent()/return()/borrow() on
arbitrary named pools) and Http\Psr7\Pool\PoolManager (100% static,
coordinates the specific PSR-7 pools: ResponsePool/HeaderPool/
OperationsCache) share a class name but not a purpose or a shape.
Full unification behind a single PoolManagerInterface — the report's
original recommendation — turns out to not be viable without rewriting
one of the two from scratch: PHP interfaces only cover instance methods,
and Http\Psr7\Pool\PoolManager has none (everything is static). Confirmed
neither class is actually used on the framework's hot path today (real
pooling goes through HttpPoolFacade/Psr7Pool), so a disproportionate
rewrite isn't justified just to satisfy an interface.
Documented the distinction explicitly in both classes' docblocks instead —
that removes the actual reported impact ("ambiguity in which pool to
use"). Full architectural unification is out of scope.
EventDispatcher and Logger were already moved out of Providers/ to Events/ and Logging/ respectively, with deprecated shims left behind (prior commits on this branch). Container is the one item from M-06 still in Providers/ — but it's the active, canonical container (Application uses it exclusively; Core\Container is the deprecated one, per C-01). Moving it now would be a third identity change for the same class in a short span (Core\Container -> Providers\Container -> yet another namespace) with no real benefit to consumers. Documented that reasoning directly in the class docblock instead of moving it.
Multi-line addListener()/listen() calls didn't follow the project's one-argument-per-line convention for multi-line calls. Caught by composer cs:check while doing a final quality pass on this branch.
code-cfernandes
added
bug
🚀 Release Readiness Report - PivotPHP Core v2.0.0✅ All Checks Passed!
📦 Ready for PublicationThe project is ready to be tagged and released! Next Steps:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prepares the codebase for the “Release/2.1.0” line by tightening type-safety (strict_types=1), introducing/standardizing new PSR-aligned components (PSR-3 logger, PSR-14 events), improving request/response pooling correctness, and updating the test suite to reflect new deprecations and emission behavior.
Changes:
- Add
strict_types=1across multiple core/util/http files and refine type handling in validator/utils/helpers. - Introduce PSR-3
Logging\PsrLogger, migrate service providers to prefer newEvents\*andLogging\*components, and deprecate legacy provider classes. - Improve PSR-7 pooling safety (resetting headers/serverParams), remove Response auto-emit behavior, and update/relocate HTTP-related tests accordingly.
Reviewed changes
Copilot reviewed 83 out of 83 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Support/StrTest.php | Suppresses deprecation notices from deprecated Str methods under test. |
| tests/Support/HttpTestCase.php | Adds a base HTTP test case that resets superglobals between tests. |
| tests/Services/ResponseTest.php | Removes old service-layer response tests (moved to HTTP tests). |
| tests/Services/RequestTest.php | Removes old service-layer request tests (moved to HTTP tests). |
| tests/Services/HeaderRequestTest.php | Removes old service-layer header tests (moved to HTTP tests). |
| tests/Middleware/SimpleLoadShedderTest.php | Suppresses deprecation notices for deprecated LoadShedder tests. |
| tests/Middleware/RateLimiterTest.php | Ensures Response is in explicit test mode to avoid output side effects. |
| tests/Middleware/Performance/RateLimitMiddlewareTest.php | Suppresses deprecation notices for deprecated rate-limit middleware tests. |
| tests/Middleware/Core/BaseMiddlewareTest.php | Ensures Response is in explicit test mode to avoid output side effects. |
| tests/Http/ResponseTest.php | Expands HTTP response tests and centralizes setup with explicit test mode. |
| tests/Http/RequestTest.php | Expands HTTP request tests and adds superglobal resets for isolation. |
| tests/Http/Pool/Psr7PoolTest.php | Adds regression test to prevent header/serverParams leakage across pooled request reuse. |
| tests/Http/HeaderRequestTest.php | Expands header handling tests and manages $_SERVER fixtures in setup/teardown. |
| tests/Events/EventDispatcherTest.php | Adds coverage for PSR-14 dispatch and the separate string-based fire()/listen() path. |
| tests/Core/RateLimitMiddlewareTestPsr15.php | Updates import path for the deprecated PSR-15 rate-limit middleware class. |
| tests/Core/ContainerTestSimple.php | Removes simplified/debug-only container test. |
| tests/Core/ContainerTest.php | Suppresses deprecation notices when testing deprecated Core\Container. |
| tests/Core/ApplicationTest.php | Adds tests for status-specific production error messages and uncaught-exception emission. |
| tasks/2026-05-29-validator-required-false-negative-integer-zero.md | Adds task note about required falsy handling pitfalls. |
| tasks/2026-05-29-static-cached-input-isolacao-entre-requests.md | Adds task note about request body cache isolation across requests/processes. |
| tasks/2026-05-29-request-missing-use-import-custom-header-collection.md | Adds task note about missing import for CustomHeaderCollection. |
| tasks/2026-05-29-psrlogger-nome-hardcoded-express-php-log.md | Adds task note about legacy “express-php” naming in logger defaults. |
| tasks/2026-05-29-psr7pool-reset-header-iteration-mutacao.md | Adds task note about safe header reset patterns in pooled PSR-7 objects. |
| tasks/2026-05-29-parsebody-logic-bug-json-array-fallback.md | Adds task note about JSON array handling consistency in request parsing. |
| tasks/2026-05-29-headerrequest-falta-strict-types.md | Adds task note about strict types and typing expectations for HeaderRequest. |
| tasks/2026-05-29-event-dispatcher-fire-renomeacao-sem-retrocompatibilidade.md | Adds task note documenting the dispatch/fire breaking change and mitigation via docs. |
| src/Validation/Validator.php | Enables strict types and refines required/integer checks and message casting. |
| src/Utils/Utils.php | Enables strict types and routes casing helpers through Support\Str. |
| src/Utils/Arr.php | Enables strict types and adds union typing to only/except. |
| src/Support/Str.php | Enables strict types and deprecates string helpers in favor of PHP natives. |
| src/Support/HookManager.php | Updates docblock type annotation to the new Events\ListenerProvider. |
| src/Providers/LoggingServiceProvider.php | Switches PSR logger binding to Logging\PsrLogger. |
| src/Providers/Logger.php | Marks legacy provider logger as deprecated and emits deprecation warning. |
| src/Providers/ListenerProvider.php | Marks legacy provider listener provider as deprecated and emits deprecation warning. |
| src/Providers/ExtensionManager.php | Types extensions against new ApplicationInterface and avoids registering non-callables. |
| src/Providers/EventServiceProvider.php | Binds PSR-14 interfaces to new Events\ListenerProvider and Events\EventDispatcher. |
| src/Providers/EventDispatcher.php | Marks legacy provider event dispatcher as deprecated and emits deprecation warning. |
| src/Providers/Container.php | Adds architectural note explaining why Container remains in Providers namespace. |
| src/Middleware/RateLimiter.php | Updates default key generator to use Request::ip() (not deprecated getIp()). |
| src/Middleware/Performance/RateLimitMiddleware.php | Marks as deprecated, emits deprecation warning, and hardens JSON body writing. |
| src/Middleware/MiddlewareStack.php | Enables strict types, adds warning doc for async runtimes, and removes warmup method. |
| src/Middleware/LoadShedder.php | Marks as deprecated, emits deprecation warning, and modernizes body creation. |
| src/Middleware/Http/ApiDocumentationMiddleware.php | Switches to PSR-7 response/stream types and clarifies OpenAPI generation behavior. |
| src/Logging/PsrLogger.php | Adds new PSR-3 logger implementation with file writing + fallback behavior. |
| src/Logging/LogHandlerInterface.php | Removes legacy log handler interface. |
| src/Logging/Logger.php | Removes legacy logging system. |
| src/Logging/FileHandler.php | Removes legacy file handler. |
| src/Http/Response.php | Enables strict types, removes auto test-environment detection and auto-emit, centralizes default error messages. |
| src/Http/Request.php | Enables strict types, switches cached input to per-instance, introduces CustomHeaderCollection, improves PSR-7 parsedBody typing, deprecates getIp(). |
| src/Http/Psr7/ServerRequest.php | Adds withServerParams() for pooling resets and tightens URI path fallback. |
| src/Http/Psr7/Pool/PoolManager.php | Clarifies purpose and distinguishes from generic pool manager. |
| src/Http/Psr7/Adapters/HeaderPoolAdapter.php | Fixes non-array header value handling to use string casting. |
| src/Http/Pool/Psr7Pool.php | Clears headers before reuse and resets serverParams; guards stream truncation during pooling. |
| src/Http/Pool/PoolManager.php | Adds doc clarifying difference vs PSR-7 static pool manager. |
| src/Http/HeaderRequest.php | Enables strict types and centralizes header camel-case conversion helper. |
| src/Http/CustomHeaderCollection.php | Adds a concrete header collection for overriding headers in tests/custom requests. |
| src/Http/Adapters/Psr7PoolAdapter.php | Ensures cookies param is always an array when calling Psr7Pool. |
| src/Http/Adapters/GlobalsToServerRequestAdapter.php | Tightens URI path fallback and hardens uploaded file stream creation. |
| src/Events/ListenerProvider.php | Adds new PSR-14 listener provider implementation. |
| src/Events/EventDispatcher.php | Adds PSR-14 dispatcher implementation plus separate string-based event mechanism. |
| src/Core/Container.php | Enables strict types, deprecates Core container, and hardens reflection type checks. |
| src/Core/ApplicationInterface.php | Adds marker interface to decouple extensions from concrete Application type. |
| src/Core/Application.php | Enables strict types, emits uncaught exceptions properly, improves middleware wrapping, and uses status-specific production messages. |
| src/aliases.php | Guards class_alias calls with class_exists() for optional dependencies. |
| src/aliases-performance-tools.php | Moves strict_types declaration to the proper location for the file. |
| README.md | Updates version references, clarifies OpenAPI middleware behavior, fixes script paths. |
| phpstan.neon | Replaces broad ignore patterns with narrower/specific ignores. |
| examples/README.md | Updates examples documentation to v2.0.0 messaging and removes obsolete references. |
| examples/09-error-handling/enhanced-errors-v114.php | Removes obsolete legacy example. |
| examples/07-advanced/performance-v1.1.3.php | Removes obsolete legacy example. |
| examples/07-advanced/array-callables-v114.php | Removes obsolete legacy example. |
| examples/05-performance/high-performance.php | Updates performance example messaging to v2.0.0+. |
| examples/01-basics/hello-world.php | Updates hello-world example messaging and labels to v2.0.0+. |
| composer.json | Removes react/http from hard requirements; documents it under suggest for async integration. |
| CLAUDE.md | Updates example commands and architecture notes to match current structure and deprecations. |
| CHANGELOG.md | Documents the clarified dispatch() vs fire() breaking change context in Unreleased. |
Comments suppressed due to low confidence (2)
src/Utils/Arr.php:169
- Arr::only() currently breaks when $keys is passed as a string: it assigns func_get_args()[1] (still a string) and then immediately returns [] because $keys is not an array. Since the signature explicitly allows array|string, string input should be handled.
src/Utils/Arr.php:195 - Arr::except() currently treats a string $keys as "no keys" and returns the full array, because $keys stays a string and the method returns early when !is_array($keys). With the array|string signature, string input should be converted to a one-element array.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
🚀 Release Readiness Report - PivotPHP Core v2.0.0✅ All Checks Passed!
📦 Ready for PublicationThe project is ready to be tagged and released! Next Steps:
|
2 similar comments
🚀 Release Readiness Report - PivotPHP Core v2.0.0✅ All Checks Passed!
📦 Ready for PublicationThe project is ready to be tagged and released! Next Steps:
|
🚀 Release Readiness Report - PivotPHP Core v2.0.0✅ All Checks Passed!
📦 Ready for PublicationThe project is ready to be tagged and released! Next Steps:
|
…succeeded
Flagged by Copilot code review. The environment-header fallback checked
getallheaders() only to decide whether to *skip* merging — the merge loop
lived inside `if (empty($existingHeaders))`, so it only ever ran when
getallheaders() returned nothing. Whenever it returned real headers (the
normal case under Apache/PHP-FPM), they were fetched and then silently
discarded: Request::setHeaders() would end up with only the explicit
custom overrides, dropping Authorization, Cookie, and everything else
from the actual request.
Extracted the merge logic (skip keys already set by an explicit override)
into mergeMissingHeaders(), now called unconditionally with whichever
source has data — getallheaders() when non-empty, otherwise the parsed
$_SERVER['HTTP_*'] fallback (extracted into parseServerHeaders()). There's
now a single merge code path instead of two copies where only one worked,
so the bug can't be silently reintroduced by editing just one branch.
getallheaders() can't be meaningfully mocked in tests: CustomHeaderCollection
guards the call with function_exists('getallheaders'), which always resolves
against the global function regardless of any same-namespace override, and
the CLI SAPI PHPUnit runs under doesn't define it at all. Added
tests/Http/CustomHeaderCollectionTest.php covering mergeMissingHeaders()
directly via reflection (the exact logic that was broken) plus the real,
exercisable $_SERVER fallback path and override precedence.
🚀 Release Readiness Report - PivotPHP Core v2.0.0✅ All Checks Passed!
📦 Ready for PublicationThe project is ready to be tagged and released! Next Steps:
|
…Stream)
Flagged by Copilot code review. truncate() isn't part of StreamInterface
(PSR-7) — resetStream() guarded the call with method_exists() and, when
absent, fell through to write() anyway. write() only overwrites the bytes
matching the new content's length; if the new content is shorter than
whatever the reused stream held from its previous request/response,
the leftover trailing bytes stay in the stream. Same class of bug as the
serverParams pool leak (C-03) — data from one request leaking into the
next via a pooled object, just at the stream/body level this time.
The framework's own Stream implementation always has truncate(), so this
was latent (the pool never actually saw a stream without it) but real for
any other StreamInterface implementation the pool might be handed. Now
resetStream() only reuses the stream when both isWritable() and
method_exists(..., 'truncate') hold; otherwise it falls back to a fresh
Stream::createFromString($content), same as the non-writable case already
did.
Added WritableSeekableStreamWithoutTruncate to Psr7PoolTest — a minimal
StreamInterface implementation that's writable/seekable but deliberately
has no truncate(). Verified the new test fails without the fix (residual
bytes leak: 'Hi' + pool return('Original long content here') produced
'Hiiginal long content here') and passes with it.
🚀 Release Readiness Report - PivotPHP Core v2.0.0✅ All Checks Passed!
📦 Ready for PublicationThe project is ready to be tagged and released! Next Steps:
|
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.
Pull Request Template
📋 Descrição
Descreva resumidamente as mudanças feitas neste PR.
🎯 Tipo de Mudança
🧪 Como foi testado?
Descreva os testes que você executou para verificar suas mudanças.
📝 Checklist
🔗 Issues Relacionadas
Fixes #(número da issue)
📸 Screenshots (se aplicável)
Cole screenshots aqui para demonstrar as mudanças visuais.
📝 Notas Adicionais
Adicione qualquer informação adicional relevante para os reviewers.