[jira] [Commented] (SLING-5726) Allow ValueMapInjector and ResourcePathInjector to act directly on SlingHttpServletRequest

["Konrad Windszus (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SLING-5726?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15312385#comment-15312385 ] 

Konrad Windszus commented on SLING-5726:
----------------------------------------

Coming back to this

bq. I'm entirely supportive of the notion of using models in multiple contexts, which is the basis for this request. My concern here is one of separation of concerns. AbstractInjector is not API, it is an internal helper. 

Yes, you are right. This patch is actually modifying a helper which is only used in two injectors (and not exported). So this is not at all about changing any API or the framework!

bq. Putting additional expectations on Injectors just seems wrong to me.

This patch has the opposite intent. It will make the injectors just easier to use, because the developer shouldn't care to much on which adaptable the injector is working and all injectors we ship with Sling Models should support IMHO as many adaptables as possible (to make them easier to use in all contexts). We are talking about a restriction of two injectors we want to lift here!

What about the currently non-documented restriction of the ResourcePathInjector (to only act on Resources but not on Requests). We should definitely do something about it.

For that we have two options
# Apply the given patch, or
# Document that ResourcePathInjector will not support the adaptable SlingHttpServletRequest (in http://sling.apache.org/documentation/bundles/models.html#available-injectors)

In addition we could think about supporting "via" on the annotation {{@ResourcePath}} (https://github.com/apache/sling/blob/trunk/bundles/extensions/models/api/src/main/java/org/apache/sling/models/annotations/injectorspecific/ResourcePath.java) and make that have the default "resource" in case the adaptable is a SlingHttpServletRequest. The default value would only make sense of course if we choose option 2.



> Allow ValueMapInjector and ResourcePathInjector to act directly on SlingHttpServletRequest
> ------------------------------------------------------------------------------------------
>
>                 Key: SLING-5726
>                 URL: https://issues.apache.org/jira/browse/SLING-5726
>             Project: Sling
>          Issue Type: Bug
>          Components: Extensions
>    Affects Versions: Sling Models Impl 1.2.8
>            Reporter: Konrad Windszus
>            Assignee: Konrad Windszus
>         Attachments: SLING-5726.patch
>
>
> Currently both injectors only work correctly on objects being adaptable to a {{ValueMap}}. For {{ValueMapInjector}} this is documented but for {{ResourcePathInjector}} it is not (http://sling.apache.org/documentation/bundles/models.html#available-injectors), although for the latter it does only matter if the path is given through a resource property and not in a static way.
> This should be relaxed that both also work with {{SlingHttpServletRequest}} as that always carries the current resource from which the {{ValueMap}} can be easily retrieved.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)