[Issue #2446] Don't open new ResourceResolver to resolve clientlib categories
#2528
Conversation
This file contains 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
…ceType(String); Updates `Utils#getSuperType` to use `resourceResolver.getParentResourceType(superType)` instead of `resourceResolver.getResource(superType).getResourceSuperType`. This change is made because any `ResourceResolver` can always resolve the resource super type hierarchy via `ResourceResolver#getParentResourceType(String)`, even if `ResrouceResolver#getResource(String)` would be unable to resolve the super type resource due to permissions. After this change, the only requirement for the `ResourceResolver` passed to `Utils#getSuperTypes()` is that it be an active `ResourceResolver` - it does not need to be a special `ResourceResolver` with any elevated permissions. ---- refs issue adobe#2446
Adds a new service `ClientLibrariesLookupService`. This service provides a means by which to resolve ClientLibraries by resource type. The service will internally cache `resourceType` -> `ClientLibrary` mappings such that future lookups for the same `resourceType` will not require costly lookup or service-user sessions. This service listens to the event topic: `com/adobe/granite/ui/librarymanager/INVALIDATED`. When an event matching this topic occurs, then the cache items are cleared. ---- refs issue adobe#2446
Updates the `ClientLibrariesImpl` model by removing the clientlib lookup logic and making use of the `ClientLibrariesLookupService` instead. ---- refs issue adobe#2446
|
Kudos, SonarCloud Quality Gate passed!
|
Codecov Report
@@ Coverage Diff @@
## main #2528 +/- ##
============================================
- Coverage 87.25% 87.23% -0.02%
- Complexity 2609 2614 +5
============================================
Files 229 230 +1
Lines 6926 6950 +24
Branches 1049 1047 -2
============================================
+ Hits 6043 6063 +20
- Misses 354 357 +3
- Partials 529 530 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
This PR is a Proof of Concept on how to resolve issue #2446.
The solution outlined in issue #2446 is to pin an open service-user session to the user request so that the service user session does not need to be re-opened multiple times (which is costly). The problem with this solution is that the Sling API included in AEM 6.5.x does not currently support this.
The PR proposes an alternative solution.
This PR builds on the fact that all resource type -> client library category resolution is performed using a service user session.
Since the same service user is used to perform this resolution for every request, the results of this resolution will not differ for different user sessions - therefore the results are suitable to be cached between multiple requests.
To accomplish this, a new service (
ClientLibraryLookupService) is added.This service performs the client library category resolution, and then uses a simple
HashMapas a cache.Initial requests to a clean cache may still cause multiple service user session logins during a single user request; however, once the cache is populated, there are no service user session logins at all.
This has an advantage over pinning the service user session to the user request because (with the exception of the first few requests) there are 0 service user sessions created (instead of 1), and additionally the entire logic and repository access associated with the resource type -> client library category resolution is also eliminated and replaced with a simple hashmap lookup - reducing repository access in general.
To ensure that the cache is kept in sync with the repository, the cache is cleared every time a
com/adobe/granite/ui/librarymanager/INVALIDATEDevent is issued - which occurs every time a client library is modified, deleted, or added.When this event is issued, the entire cache is invalidated. This is easier than trying to figure out which cache items are no longer valid and selectively clearing just those entries. Changes to the client libraries on a running AEM instance should be infrequent enough that clearing the entire cache will have a negligible performance impact.