fix: Registry cache clearing bug preventing effective caching #29
Conversation
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
The Registry::load() method was incorrectly calling clear() which deleted the cache before attempting to read from it, making caching completely ineffective. Modified clear() to accept an optional $includeCache parameter (default: true) to maintain backward compatibility while allowing load() to clear only local elements without destroying the cache.
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.
Problem
The
Registry::load()method contained a critical bug where it calledclear()which:This meant the cache was always empty when loading, making the entire caching system ineffective and causing performance issues for applications with many discovered elements, and discoverability isses for all servers using discovered elements.
Solution
Modified the
Registry::clear()method to accept an optional$includeCacheparameter:clear()- Default behavior unchanged (clears cache + local elements)clear(false)- New option to clear only local elements, preserving cacheload()- Now callsclear(false)to preserve cache during loadingBackward Compatibility
✅ No breaking changes - existing
clear()calls work identically✅ Default parameter maintains existing API behavior
✅ Safe for patch release - 3.0.2