[jira] [Comment Edited] (SLING-8279) Having a Resource + ResourceMetadata should be sufficient for roundtrip link mapping.

2019-05-29 Thread Konrad Windszus (JIRA)


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

Konrad Windszus edited comment on SLING-8279 at 5/29/19 5:33 PM:
-

The issue with supporting more adaptables for injectors which currently only 
support {{Resource}} look at SLING-5726.


was (Author: kwin):
The issue with supporting more adaptables for injectors which currently only 
support {{Resource}} look at SLING-5276.

> Having a Resource + ResourceMetadata should be sufficient for roundtrip link 
> mapping.
> -
>
> Key: SLING-8279
> URL: https://issues.apache.org/jira/browse/SLING-8279
> Project: Sling
>  Issue Type: Wish
>  Components: ResourceResolver
>Reporter: Mark Adamcin
>Priority: Major
>
> With Sling Models, it is very easy to construct composite model types from 
> more than one resource, usually addressable as a subtree of the repository.
> However, a pattern is emerging where mapping is being performed to create 
> related links within the model, which mandates that SlingHttpServletRequest 
> be used as the adaptable type, because a Resource adaptable would not provide 
> sufficient context for ResourceResolver.map(). Without a request for context, 
> .map() would likely return incorrect links based on a default base request 
> URL of "http://localhost/;.
> While on one hand, it might be argued that link mapping should only occur in 
> the view, after the model graph has been constructed, on the other hand, link 
> resolution and resource mapping are so fundamental to the correct behavior of 
> a web application that we should take advantage of every opportunity to make 
> these activities more convenient and less error prone.
> If you trace the code for resource resolution and mapping, you will find that 
> it relies on just four discrete contextual properties that are currently 
> available only from a request object (i.e. not available from a Resource or 
> its ResourceMetadata):
>  # scheme
>  # host
>  # port
>  # contextPath
> In addition, given that the ResourceResolver used by servlets when handling a 
> request is generally retrieved from the Sling Request itself using 
> getResourceResolver(), it seems redundant in concept, not to mention clumsy 
> in practice, to require passing the request as an argument back to the 
> resource resolver (that was created specifically for the request in question) 
> in order to render links for any resources resolved while servicing that 
> request.
> I think it is time to change the expected behavior of 
> ResourceResolver.resolve(String), ResourceResolver.map(String path), and 
> other ResourceResolver methods that return resources without an explicit 
> HttpServletRequest parameter, such that:
>  # request.getResourceResolver().resolve(path) returns the same Resource as 
> (any ResourceResolver).resolve(request, path)
>  # request.getResourceResolver().map(path) returns the same String as (any 
> ResourceResolver).map(request, path)
>  # 
> request.getResourceResolver().getResource(somePath).getResourceResolver().resolve(path)
>  returns the same Resource as request.getResourceResolver().resolve(path)
>  # 
> request.getResourceResolver().findResources(someQuery).next().getResourceResolver().resolve(path)
>  returns the same Resource as request.getResourceResolver().resolve(path)
>  # etc.
>  # ResourceResolverFactory.getResourceResolver(Map) and 
> ResourceResolverFactory.getServiceResourceResolver(Map) would return 
> ResourceResolvers that continue to use 
> [http://localhost:80|http://localhost/] as the default context url.
> If these constraints can not be satisfied reasonably using the existing 
> resolve(String) and map(String) methods, I would propose adding overloads 
> that accept a context Resource in place of the context HttpServletRequest, 
> with additional properties added to ResourceMetadata during request resource 
> resolution that persist the four request context properties listed above 
> (scheme, host, port, contextPath).



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Comment Edited] (SLING-8279) Having a Resource + ResourceMetadata should be sufficient for roundtrip link mapping.

2019-02-19 Thread Brett Birschbach (JIRA)


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

Brett Birschbach edited comment on SLING-8279 at 2/19/19 2:29 PM:
--

Though we could debate the merits of where code should live and probably never 
come to an agreement...

Here's my opinion, in agreement with Mark.

1) The option to construct a URL in a model is already possible since the 
`request` is available - this is just making it more streamlined.

2) The `requestResolver.map(resource)` and other similar functions that take a 
resource give a potentially unexpected behavior of ignoring the current request 
(since the resource conceivably is tied to the request, per Mark's notes) - 
requiring a developer to explicitly pass `null` in the case that they 
*actually* want the function to be evaluated outside the context of a request 
seems like a better coding pattern.

3) Constructing the URL often has additional java logic in a CMS - perhaps you 
want to inject some logic to translate external URLs that your translation 
provider doesn't translate (just an example).  Ideally that code can stay in 
the model rather than requiring HTL to jump thru another hoop.  I know we can 
use a `Use` object to do this from the model, but at that point it just feels 
we are adding code complexity for no objective benefit.

4) For sling model exporters, there is no explicit "view" layer that the 
developer creates "above" the model - it's a generic request handler.  If we 
don't do the externalization in the model, where would we do it?

 


was (Author: brett.birschbach):
Though we could debate the merits of where code should live and probably never 
come to an agreement...

Here's my opinion, in agreement with Mark.

1) The option to construct a URL in a model is already possible since the 
`request` is available - this is just making it more streamlined

2) The `requestResolver.map(resource)` and other similar functions that take a 
resource give a potentially unexpected behavior of ignoring the current request 
(since the resource conceivably is tied to the request, per Mark's notes) - 
requiring a developer to explicitly pass `null` in the case that they 
*actually* want the function to be evaluated outside the context of a request 
seems like a better coding pattern.

3) Constructing the URL often has additional java logic in a CMS - perhaps you 
want to inject some logic to translate external URLs that your translation 
provider doesn't translate (just an example).  Ideally that code can stay in 
the model rather than requiring HTL to jump thru another hoop.

4) For sling model exporters, there is no explicit "view" layer that the 
developer creates "above" the model - it's a generic request handler.  If we 
don't do the externalization in the model, where would we do it?

 

> Having a Resource + ResourceMetadata should be sufficient for roundtrip link 
> mapping.
> -
>
> Key: SLING-8279
> URL: https://issues.apache.org/jira/browse/SLING-8279
> Project: Sling
>  Issue Type: Wish
>  Components: ResourceResolver
>Reporter: Mark Adamcin
>Priority: Major
>
> With Sling Models, it is very easy to construct composite model types from 
> more than one resource, usually addressable as a subtree of the repository.
> However, a pattern is emerging where mapping is being performed to create 
> related links within the model, which mandates that SlingHttpServletRequest 
> be used as the adaptable type, because a Resource adaptable would not provide 
> sufficient context for ResourceResolver.map(). Without a request for context, 
> .map() would likely return incorrect links based on a default base request 
> URL of "http://localhost/;.
> While on one hand, it might be argued that link mapping should only occur in 
> the view, after the model graph has been constructed, on the other hand, link 
> resolution and resource mapping are so fundamental to the correct behavior of 
> a web application that we should take advantage of every opportunity to make 
> these activities more convenient and less error prone.
> If you trace the code for resource resolution and mapping, you will find that 
> it relies on just four discrete contextual properties that are currently 
> available only from a request object (i.e. not available from a Resource or 
> its ResourceMetadata):
>  # scheme
>  # host
>  # port
>  # contextPath
> In addition, given that the ResourceResolver used by servlets when handling a 
> request is generally retrieved from the Sling Request itself using 
> getResourceResolver(), it seems redundant in concept, not to mention clumsy 
> in practice, to require passing the request as an argument back to the 
> resource resolver (that