Modernize internal hashes to xxh128#404
Conversation
Check for CacheFailedOver listeners before constructing and dispatching the event from FailoverStore. This brings the failover event path in line with Hypervel's guarded framework-event pattern. It avoids event construction and dispatch work when no listener is registered, while preserving the existing per-coroutine failure tracking and one-event-per-failure-window behavior. Add a focused regression test that makes hasListeners(CacheFailedOver::class) return false and asserts dispatch is not called. Tighten the existing multi-store failover test so it verifies the listener check on the dispatching path too. Verification: ./vendor/bin/phpunit --no-progress tests/Integration/Cache/FailoverStoreTest.php passed.
Rename the worker-count environment variable from SERVER_WORKERS_NUMBER to SERVER_WORKERS so the setting reads naturally and matches the user-facing server config style. Add SERVER_HTTP2 as an optional config hook for Swoole's HTTP/2 protocol setting while preserving the existing default of enabled. Add SERVER_MAX_REQUESTS as an optional operational tuning knob for Swoole worker recycling, preserving the existing default of 100000 requests per worker. Update the deployment and Reverb docs to use the new SERVER_WORKERS name and document the server tuning variables exposed through config/server.php. Verification: php -l src/foundation/config/server.php passed.
Replace opaque internal auth guard hashes with xxh128 while keeping the same input data and key prefixes. Updates session, token, JWT, Sanctum, and eloquent user-provider paths that only need fast internal identity hashes. The auth regression test now asserts the exact remember/login key names so the intended key format stays locked.
Switch file-store key paths and tagged-cache namespaces from legacy hashes to xxh128 for faster opaque internal key generation. Keeps the existing namespace inputs intact, updates repository documentation, and refreshes cache store/tagged-cache tests to assert the new internal key contract.
Make the Redis cache Doctor derive tagged namespaces through the real tagged-cache path instead of reimplementing tag ID hashing. This preserves Laravel-compatible order-sensitive tag namespaces, removes the sorted-tag mismatch, and adds a regression test proving DoctorContext matches TaggedCache::taggedItemKey for the requested tag order.
Change Filesystem::hash() to use xxh128 by default for faster app-internal file checks while preserving the explicit algorithm argument for protocol-specific callers. Updates the File facade docblock and filesystem tests so the documented default and asserted behavior match the new Hypervel default.
Replace watcher-specific MD5 terminology and direct file-content hashing with Filesystem::hash(), so scan state follows the framework hash default. Renames internal hash state to neutral file-hash wording and updates watcher docs, config comments, fixtures, and tests to match the generalized hash behavior.
Remove the unused dynamic connection name cache and its facade annotation, since Hypervel's connection architecture does not use Laravel's dynamic connection hashing path. Simplifies config lookup through the container while preserving support for both full config repositories and Capsule Fluent config, with integration coverage for the shared in-memory SQLite path.
Switch opaque request fingerprints from sha1 to xxh128 while keeping the same route, IP, and method inputs. Adds coverage for the new fingerprint value and the existing no-route failure path so the behavior remains explicit.
Modernize throttle middleware's opaque formatted identifiers from sha1 to xxh128 for faster internal rate-limit key generation. Keeps the same identifier inputs and updates the throttle middleware tests to assert the new hash algorithm.
Replace queue middleware and Redis test isolation hash generation with xxh128 where the values are opaque internal keys. The input values and surrounding behavior stay unchanged; only the internal hash algorithm changes.
Modernize scheduler mutex and description hashing to xxh128 for faster opaque internal schedule identifiers. Updates callback and event scheduling tests to cover the new hash output while preserving the existing input formulas and mutex behavior.
Switch anonymous component path prefix hashing to xxh128 for faster internal view key generation. Adds explicit coverage for the new prefix hash and fixes the existing base_path fixture typo in the same test file.
Modernize Vite's opaque generated key hashing to xxh128 while preserving the same manifest inputs and preload behavior. Updates the Vite tests so the expected generated values match the new internal hash algorithm.
Replace opaque Horizon, Telescope, and Sentry integration hashes with xxh128 where the values are internal fingerprints or grouping keys. Keeps external protocol hashes untouched and preserves the existing inputs for each observability path.
Remove the now-unused DoctorContext tagId wrapper after namespacedKey was moved to the real tagged-cache namespace path. Refresh remaining all-mode cache namespace comments from sha1 to xxh128, including Doctor, benchmark, StoreContext, and Redis integration-test prose, so the comments match the current key format and requested-order tag semantics.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR switches many digest and key-generation paths to ChangesRuntime identifier hashes
Cache keying and tagged namespaces
Utility hash defaults
Watcher file-hash polling
DatabaseManager config access
Server settings and docs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR modernizes Hypervel's internal non-cryptographic hashing from
Confidence Score: 4/5Safe to merge after fixing the Mockery call-count expectation in the new DoctorCommand test; all other changes are well-covered and logically consistent. The new test testDoctorRunsChecksWhileRedisConnectionIsBorrowed asserts withConnection->twice(), but the refactored handle() consolidates everything into a single withConnection call. Mockery verifies exact call counts at teardown, so this test will fail in the suite despite all inline assertions passing during the test body. tests/Cache/Redis/Console/DoctorCommandTest.php - the withConnection call-count expectation in the new connection-borrowing test Important Files Changed
Reviews (2): Last reviewed commit: "fix(cache): address hash review feedback" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/cache/src/FileStore.php (1)
400-405: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valuePath-derivation change orphans existing file-cache entries on upgrade.
Switching the key digest from
sha1toxxh128changes the derived directory/file path for every key, so entries written by a prior version become unreachable (cold cache) and the old files won't be cleaned byforget()/flush()until a full directory flush. This is acceptable for cache semantics, but call it out in upgrade notes so operators don't rely on a warm file cache across the deploy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cache/src/FileStore.php` around lines 400 - 405, The FileStore::path digest switch from sha1 to xxh128 changes every derived cache location, so existing on-disk entries from prior versions will be orphaned after upgrade. Keep the new FileStore::path behavior as-is, but add an upgrade note/changelog entry documenting that file-cache entries will not be reused across the deploy and may need a warm-up or manual cleanup, and mention the impact on forget() and flush() semantics for previously written keys.src/auth/src/SessionGuard.php (1)
102-104: 🧹 Nitpick | 🔵 TrivialDeploy impact: switching the class hash invalidates all active sessions and remember-me cookies.
$classHashfeeds bothhashedName(the session key) andhashedRecallerName(the recaller cookie name). Moving fromsha1toxxh128changes both derived names, so on rollout every currently logged-in user's session lookup and "remember me" cookie will no longer resolve, effectively forcing a global logout. This is inherent to the rename and the test contract confirms the new values, but it should be called out in migration/release notes so it isn't mistaken for a regression.The change itself is otherwise correct and consistent with the updated
AuthGuardTestassertions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/auth/src/SessionGuard.php` around lines 102 - 104, Call out the SessionGuard class-hash change in release/migration notes because $classHash in SessionGuard::__construct now changes both hashedName and hashedRecallerName, which invalidates existing sessions and remember-me cookies on deployment. Add a brief upgrade note near the SessionGuard/AuthGuardTest change explaining that switching from the old hash to xxh128 causes a one-time global logout so operators are not surprised by the behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/watcher/src/Driver/ScanFileDriver.php`:
- Around line 32-33: Update ScanFileDriver::scanFile so the baseline check uses
an explicit null comparison instead of a truthy test on $this->lastFileHashes;
this ensures an empty array snapshot is treated as a valid prior state and
transitions from no files to newly created files are detected. Keep the change
localized to the current-file comparison logic around getWatchFileHashes and the
$this->lastFileHashes guard.
In `@tests/Cache/Redis/Console/DoctorCommandTest.php`:
- Around line 265-280: The regression test is using ArrayStore, so it is not
validating the Redis all-mode namespace behavior that
DoctorContext::namespacedKey() is meant to mirror. Update the test to use the
actual Redis tagged store path or assert directly against the Redis namespace
format, and keep the existing tagged-key comparisons anchored on
DoctorContext::namespacedKey() so ordering and Redis all-mode naming are
exercised correctly.
---
Nitpick comments:
In `@src/auth/src/SessionGuard.php`:
- Around line 102-104: Call out the SessionGuard class-hash change in
release/migration notes because $classHash in SessionGuard::__construct now
changes both hashedName and hashedRecallerName, which invalidates existing
sessions and remember-me cookies on deployment. Add a brief upgrade note near
the SessionGuard/AuthGuardTest change explaining that switching from the old
hash to xxh128 causes a one-time global logout so operators are not surprised by
the behavior.
In `@src/cache/src/FileStore.php`:
- Around line 400-405: The FileStore::path digest switch from sha1 to xxh128
changes every derived cache location, so existing on-disk entries from prior
versions will be orphaned after upgrade. Keep the new FileStore::path behavior
as-is, but add an upgrade note/changelog entry documenting that file-cache
entries will not be reused across the deploy and may need a warm-up or manual
cleanup, and mention the impact on forget() and flush() semantics for previously
written keys.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 71f2004e-3f67-4b40-b130-1cbef3a858c4
📒 Files selected for processing (55)
src/auth/src/EloquentUserProvider.phpsrc/auth/src/SessionGuard.phpsrc/auth/src/TokenGuard.phpsrc/boost/docs/deployment.mdsrc/boost/docs/reverb.mdsrc/cache/src/FailoverStore.phpsrc/cache/src/FileStore.phpsrc/cache/src/Redis/AllTaggedCache.phpsrc/cache/src/Redis/Console/Benchmark/BenchmarkContext.phpsrc/cache/src/Redis/Console/Doctor/Checks/ForeverStorageCheck.phpsrc/cache/src/Redis/Console/Doctor/Checks/TaggedOperationsCheck.phpsrc/cache/src/Redis/Console/Doctor/DoctorContext.phpsrc/cache/src/Redis/Support/StoreContext.phpsrc/cache/src/Repository.phpsrc/cache/src/TaggedCache.phpsrc/console/src/Scheduling/CallbackEvent.phpsrc/console/src/Scheduling/Event.phpsrc/database/src/DatabaseManager.phpsrc/filesystem/src/Filesystem.phpsrc/foundation/config/server.phpsrc/foundation/src/Testing/Concerns/InteractsWithRedis.phpsrc/foundation/src/Vite.phpsrc/horizon/src/Notifications/LongWaitDetected.phpsrc/http/src/Request.phpsrc/jwt/src/JwtGuard.phpsrc/queue/src/Middleware/RateLimited.phpsrc/routing/src/Middleware/ThrottleRequests.phpsrc/sanctum/src/SanctumGuard.phpsrc/sentry/src/Features/ConsoleSchedulingFeature.phpsrc/support/src/Facades/DB.phpsrc/support/src/Facades/File.phpsrc/telescope/src/IncomingExceptionEntry.phpsrc/telescope/src/Watchers/QueryWatcher.phpsrc/view/src/Compilers/BladeCompiler.phpsrc/watcher/README.mdsrc/watcher/config/watcher.phpsrc/watcher/src/Driver/ScanFileDriver.phptests/Auth/AuthGuardTest.phptests/Cache/CacheFileStoreTest.phptests/Cache/CacheSwooleStoreTest.phptests/Cache/Redis/AllTaggedCacheTest.phptests/Cache/Redis/Console/DoctorCommandTest.phptests/Console/Scheduling/CallbackEventTest.phptests/Console/Scheduling/EventTest.phptests/Filesystem/FilesystemTest.phptests/Foundation/FoundationViteTest.phptests/Http/HttpRequestTest.phptests/Integration/Cache/FailoverStoreTest.phptests/Integration/Cache/Redis/KeyNamingIntegrationTest.phptests/Integration/Cache/Redis/TaggedOperationsIntegrationTest.phptests/Integration/Database/Sqlite/InMemorySqliteSharedPdoTest.phptests/Routing/ThrottleRequestsTest.phptests/View/Blade/BladeComponentTagCompilerTest.phptests/Watcher/Driver/ScanFileDriverTest.phptests/Watcher/Fixtures/ScanFileDriverStub.php
💤 Files with no reviewable changes (1)
- src/support/src/Facades/DB.php
Handle empty watcher baselines with an explicit null check so a transition from no watched files to a new file is reported correctly. Normalize failed watched-file hash reads to null and skip those entries instead of storing false in the watcher hash snapshot. This keeps watcher state typed as path-to-hash strings while preserving the normal Filesystem default. Move the Doctor namespace regression from an ArrayStore unit shape into the Redis all-mode integration suite so it verifies real tagged-cache key storage and order-sensitive all-mode namespaces. Keep Redis doctor checks inside the borrowed Redis connection callback for the full DoctorContext lifetime, preventing released pooled connections from being reused after checkout. Also update Redis connector tests to inspect raw phpredis clients while the pool connection is still borrowed. Tests: focused watcher/cache/Redis suites passed; composer fix passed with csfixer, phpstan, and the full parallel suite.
|
CodeRabbit: The The The docstring coverage warning is not being acted on. It is not part of this repository’s review criteria for this PR, and adding docstrings just to satisfy that generated threshold would add noise rather than improve the code. |
|
Greptile:
The Removing The session-cookie, file-cache, tagged-cache, and scheduler-key invalidation points are accurate as upgrade impacts, but they do not require code changes. These are runtime-derived internal keys, and Hypervel 0.4 is not carrying compatibility layers for previous hash formats. |
1 participant
Summary
Modernizes Hypervel's opaque internal hash usage from legacy
md5/sha1calls toxxh128where the value is used only as an internal lookup key, namespace, fingerprint, mutex identifier, generated prefix, or grouping key.This keeps protocol-required and security-sensitive hashes unchanged, including Redis
EVALSHA, email verification, cookie HMAC prefixes, Gravatar, HIBP, WebSocket handshakes, Pusherbody_md5, and SQS MD5 fixtures.What changed
xxh128.Filesystem::hash()to default toxxh128, while preserving the explicit algorithm parameter for callers that need protocol-specific hashes.Filesystem::hash()instead of direct MD5 file-content hashing.Fluentconfig objects.sha1toxxh128.Performance
xxh128is faster than MD5/SHA1 for these internal non-cryptographic paths, while still providing a wide 128-bit output for low collision risk. The change avoids applying cryptographic hashes where Hypervel only needs stable opaque internal identifiers.The remaining MD5/SHA1 sites were intentionally kept because they are dictated by external protocols, security primitives, or tests for those explicit algorithms.
Tests
composer fixsuccessfully:Summary by CodeRabbit
xxh128scheme (key formats may change, impacting existing cache entries).