Hi Carsten, Our emails crossed. Yours essentially answering my question, thanks!
I had a look at SLING-3868 which has added an accessor for a per thread resource resolver. Regarding supporting SLING-3868 for Sling feature flags, the only missing part would be to leverage ResourceResolverFactory#getThreadResourceResolver in the FeatureManager. As a result and as mentioned by Felix, we would be able to evaluate contextual feature state from services. One question though. In SLING-3868, it seems the purpose for the addition of the per thread resource resolver accessor is to solve the cases where "it's not possible to pass the current resource resolver done the call stack". IMO, it is indeed quite useful in those cases, but it comes with added level of magic and may not be the best way to pass data to methods when the possibility to change API is available. In the case of using the Feature API from a service, if leveraging SLING-3868 it would imply the following steps: 1. Acquire a resource resolver (the resource resolver is set in the thread context implicitly) 2. Invoke Features.isEnabled(featureName) (the feature evaluation will use the latest resource resolver form the thread context) This involves some magic and enforces some limitations: * Between the points 1. and 2. the thread must of course not acquire another resource resolver (otherwise it would change the reference in the thread context) * From the context, only the resource can be specified (it would not be possible to pass a request implementation) Assuming there is a possibility to change the Feature API in a backward compatible way, which would allow an application developer to provide his own Context implementation in a direct fashion, wouldn't it make sense to allow it ? I think this could be done by: 1. Removing the comment in [0] that explicitly tells application developer not to use the method; and/or 2. Doubling the method isEnabled in Features API to allow passing a Context Regards, Timothee [0] https://github.com/apache/sling/blob/trunk/bundles/extensions/feature-flags/src/main/java/org/apache/sling/featureflags/Feature.java#L60 2014-09-03 9:08 GMT+01:00 Timothée Maret <[email protected]>: > Hi Felix, > > Thanks for your input. > > > 2014-09-03 4:26 GMT+01:00 Felix Meschberger <[email protected]>: > > Hi >> >> The Features service can be used by non-request processing threads, e.g. >> by background threads/tasks. >> >> > ok > > Currently, though, it will only global feature state and not context >> sensitive state. >> >> > ok, that's what I felt. My service would require contextual data to figure > out feature states. > > >> With the proposed addition of the ResourceResolverHelper, which may place >> the ResourceResolver created from a ResourceResolverFactory into the thread >> context, the FeatureManager could leverage that and create a context with >> the ResourceResolver directly if the HttpServletRequest is not available, >> such as in background contexts. > > > Considering the FeatureManager alone, I think the evaluation of feature > status from services will always end up dealing with meaningless context. > Whether the context contains null entities (the current case) or contains > instances defined by the FeatureManger would not change would not change > much from an application POV. > > I think we need an API to allow application to provide whatever context > they want for evaluating the status of a feature. > As I understand the idea is to use a ResourceResolverHelper for that > purpose. > > Is there a discussion thread or jira issue I could look at to > understand/track the addition of the ResourceResolverHelper ? > > >> With such a ResourceResolver you can then provide the Tenant information >> for your own Feature implementation. >> >> Regards >> Felix >> >> Am 02.09.2014 um 05:58 schrieb Timothée Maret <[email protected]>: >> >> > Hi, >> > >> > In our service, we provide a set of features which can be >> enabled/disabled >> > per tenant. >> > Our tenants implementation is based on the Sling Tenant API and we are >> > thinking to leverage the Sling Feature flags API to represent our >> features. >> > From our application POV, we would need to implement this logical >> signature >> > >> > status = enable(tenantId, featureName) >> > >> > Looking at the Sling feature flags API, the signature we should >> implement >> > looks like [0] >> > >> > status = feature(featureName).enable(request, resolver) >> > >> > where the request and resolver are essentially the context provided by >> the >> > features manager implementation. >> > >> > So, implementing our use case would be possible as we can derive a >> tenantId >> > from a resolver >> > >> > status = feature(featureName).enable(tenantId(resolver)) >> > >> > With the current features manager implementation, this would be possible >> > only when features are checked upon a HTTP request as the features >> manager >> > would be able to provide a meaningful context. >> > >> > However, with background services, the features manager would naturally >> not >> > be able to provide a meaningful context (currently the resolver and >> request >> > are null), thus our implementation would be useless for those use cases. >> > According to the javadoc in [2], it would also not be possible to >> > explicitly provide our own context implementation (that would embed the >> > tenantId, or a resource resolver that can be reduced to the tenantId) >> using >> > the API. >> > >> > An option would be to implement our own features manager which would >> take >> > the tenantId in consideration, however I believe it would make more >> sense >> > to leverage the existing implementation and allow to pass custom >> contexts >> > to it (essentially removing the comment in [2]). >> > >> > Another improvement may be to allow passing a map of properties via the >> > context. This way, a background service could pass directly the relevant >> > information to the Feature, instead of having to leverage a proxy >> > ResourceResolver or HttpServletRequest instance. >> > >> > Anyway, are feature flags supposed to be accessible from background >> > services (as wished in [1]) ? >> > If yes, what is the recommended way to use contextual feature flags from >> > background services ? >> > >> > Regards, >> > >> > Timothee >> > >> > [0] >> > >> https://github.com/apache/sling/blob/trunk/bundles/extensions/feature-flags/src/main/java/org/apache/sling/featureflags/Feature.java#L70 >> > [1] http://markmail.org/message/rueoiuacmft5fdet >> > [2] >> > >> https://github.com/apache/sling/blob/trunk/bundles/extensions/feature-flags/src/main/java/org/apache/sling/featureflags/Feature.java#L60 >> >> Regards, > > Timothee > -- Timothée Maret
