feat(lib): add CactiRequest, CactiLogger, CactiFilesystem wrappers#7126
Closed
somethingwithproof wants to merge 6 commits into
Closed
feat(lib): add CactiRequest, CactiLogger, CactiFilesystem wrappers#7126somethingwithproof wants to merge 6 commits into
somethingwithproof wants to merge 6 commits into
Conversation
Thin wrappers over symfony/http-foundation, psr/log, symfony/process, and symfony/filesystem that give the Cacti code base a single test-friendly call surface for request parsing, structured logging, external process execution, and file system operations. Each class is intentionally small; behaviour stays in the underlying Symfony components. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds PSR-4 wrapper classes under lib/ to provide a stable seam over HTTP request access, logging, process execution, and filesystem operations (primarily to enable test doubles and constructor injection), along with initial unit tests for each wrapper.
Changes:
- Introduces
Cacti\Http\CactiRequest,Cacti\Log\CactiLogger,Cacti\Process\CactiProcess, andCacti\Filesystem\CactiFilesystemwrappers delegating to Symfony/PSR components. - Adds new Pest unit tests covering the wrapper surfaces.
- Adds basic reset/static-injection mechanisms for test isolation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/CactiRequestTest.php | Adds unit tests for the request wrapper (currently broken bootstrap path). |
| tests/Unit/CactiProcessTest.php | Adds unit tests for process splitting/execution (currently broken bootstrap path). |
| tests/Unit/CactiLoggerTest.php | Adds unit tests for PSR-3 forwarding and legacy fallback (currently broken bootstrap path). |
| tests/Unit/CactiFilesystemTest.php | Adds unit tests for filesystem wrapper operations (currently broken bootstrap path). |
| lib/Http/CactiRequest.php | Adds Symfony Request-based wrapper, including AJAX detection and session helpers. |
| lib/Process/CactiProcess.php | Adds Symfony Process-based wrapper for run/start and a simple tokenizer. |
| lib/Log/CactiLogger.php | Adds PSR-3 logger wrapper with legacy cacti_log() fallback and level mapping. |
| lib/Filesystem/CactiFilesystem.php | Adds Symfony Filesystem-based wrapper with error logging on IO exceptions. |
Same as PR-A: appends 11 PHPStan ignoreErrors entries that exist on upstream develop but are not yet baselined, so this branch's CI does not regress on phpstan analyse --level 6. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Contributor
Author
Drop lib/Process/CactiProcess.php and tests/Unit/CactiProcessTest.php to avoid colliding with Cacti#7088 / Cacti#7073, which already provide the canonical flat lib/CactiProcess.php. Move the remaining wrappers to the un-namespaced lib/CactiX.php convention used by every other lib/Cacti*.php class on develop: lib/Filesystem/CactiFilesystem.php -> lib/CactiFilesystem.php lib/Http/CactiRequest.php -> lib/CactiRequest.php lib/Log/CactiLogger.php -> lib/CactiLogger.php Tests load the classes via require_once instead of the dropped PSR-4 autoloader. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
somethingwithproof
changed the title
Contributor
Author
2 tasks
Contributor
Author
|
Queue note: deferring this from the active review path for now. It is currently red and the generic Request/Logger/Filesystem wrappers overlap the broader Symfony/helper direction without blocking the current bug/security queue. |
Contributor
Author
|
Closing for now to keep the active queue focused. The generic Request/Logger/Filesystem wrappers are not blocking the current bug/security work and overlap the broader Symfony/helper direction. We can reopen or split out CactiFilesystem later if a concrete consumer needs it. |
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.
Part C of six PRs splitting #7123. Depends on #7124.
Adds three flat un-namespaced infra wrappers:
The CactiProcess wrapper that was previously in this PR has been
dropped; #7088 (and #7073) already provide the canonical
lib/CactiProcess.php.
Aligns with the flat lib/CactiX.php convention used by #7088 / #7073 /
#7077 / #7075. Tests load classes via require_once, not via PSR-4
autoload.
symfony/http-foundation, symfony/filesystem, and psr/log are added by
#7124. symfony/process is intentionally not duplicated here; it lands
via #7073.