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

Added implementation + tests for context aware via injection. #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

niekraaijmakers
Copy link

Makes the following possible:

`@Model(adaptables = {SlingHttpServletRequest.class, Resource.class}, defaultInjectionStrategy = DefaultInjectionStrategy.OPTIONAL)
public class SampleModel {

@Via(type= ContextAwareConfigResource.class, value = "config")
@ValueMapValue
private String propertyTest;

public String getPropertyTest() {
    return propertyTest;
}

}`

Use via to point to context aware config resource, to inject valuemap values.
Tested it locally and works fine with latest versions.
What do you think?

@stefanseifert
Copy link
Member

there is already an a JIRA ticket for this topic https://issues.apache.org/jira/browse/SLING-7256

in your implementation you introduced a thread local to cache things resolved by this injector. i think this is a bad idea in this context. if a cache is required because caconfig retrieval is slow, it should be added to caconfig in general and not only to the injector. please not that SlingAdaptable and sling models itself has also some caching capabilities, we should not add another here.

but before going into those details there are open general questions where this type of extension should be placed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants