Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ClientLibrariesImpl should not always open a new ResourceResolver #2446

Open
joerghoh opened this issue Feb 9, 2023 · 1 comment · May be fixed by #2528
Open

ClientLibrariesImpl should not always open a new ResourceResolver #2446

joerghoh opened this issue Feb 9, 2023 · 1 comment · May be fixed by #2528

Comments

@joerghoh
Copy link
Collaborator

joerghoh commented Feb 9, 2023

Feature Request

The ClientLibrariesImpl sling model uses a dedicated Service ResourceResolver to resolve the categories; but such a model is used within many components, I found it used in 12 snippets of the v1 page component. That means that in case a page uses all of these snippets, 12 times a resource resolver is opened and closed, which is quite some overhead (in the range of miliseconds).

It should be possible to share a single instance of such a ResourceResolver amongst all of these snippets in the context of a single request, which would reduce overhead and page rendering time.

Describe the solution you'd like

The ClientLibrariesImpl should store an opened ResourceResolver within the context of the ResourceResolver of the request (in the ResourceResolver.getPropertyMap()). When such a RR is available in that map, it should be used (instead of a new one being created).

The API contract of ResourceResolver.getPropertyMap()) ensures that the Service RR is closed when the page rendering is finished and its RR is closed.

@joerghoh
Copy link
Collaborator Author

joerghoh commented Feb 9, 2023

Need to postpone it until this API is available in AEM 6.5.x ...

ky940819 added a commit to ky940819/aem-core-wcm-components that referenced this issue Jun 11, 2023
…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
ky940819 added a commit to ky940819/aem-core-wcm-components that referenced this issue Jun 11, 2023
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
ky940819 added a commit to ky940819/aem-core-wcm-components that referenced this issue Jun 11, 2023
Updates the `ClientLibrariesImpl` model by removing the clientlib lookup
logic and making use of the `ClientLibrariesLookupService` instead.

----
refs issue adobe#2446
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant