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

Dirk Rudolph edited comment on SLING-6655 at 3/17/17 7:19 AM:
--------------------------------------------------------------

I see your point now, thanks. 

To summarise the discussion from my perspective, there are 3 things implemented:

1. Exporter which obviously doesn't really need the model to resouceType 
binding. It only leverages the resourceType to register the {{ExportServlet}} 
accordingly 
2. The {{ResourceTypeBasedResourcePicker}} responsible for picking the right 
implementation of an adapter based on the nearest resourceType if the adaptable 
is {{Resource}} or {{SlingHttpServletRequest}}.
3. The methods {{Object getModelFromResource(...)}} and {{Object 
getModelFromRequest(...)}} used for scripting.

Still, IMHO, *3* doesn't belong to the {{ModelFactory}} but more to something 
scripting related. Having said that the implementation of those methods could 
also just use createModel(resource, Object.class) if the models itself are 
registered with adapater = Object.class, or even better using a marker 
interface and having *2* in place to pick the right implementation. This would 
better fit the semantics of the ModelFactory's purpose. Wdyt? Though the 
"caching" currently done in {{AdapterImplementations}} would need to somehow be 
handled by the {{ImplementationPicker}} then. 


was (Author: diru):
I see your point now, thanks. 

To summarise the discussion from my perspective, there are 3 things implemented:

1. Exporter which obviously doesn't really need the model to resouceType 
binding. It only leverages the resourceType to register the {{ExportServlet}} 
accordingly 
2. The {{ResourceTypeBasedResourcePicker}} responsible for picking the right 
implementation of an adapter based on the nearest resourceType if the adaptable 
is {{Resource}} or {{SlingHttpServletRequest}}.
3. The methods {{Object getModelFromResource(...)}} and {{Object 
getModelFromRequest(...)}} used for scripting.

Still, IMHO, 3 doesn't belong to the {{ModelFactory}} but more to something 
scripting related. Having said that the implementation of those methods could 
also just use createModel(resource, Object.class) if the models itself are 
registered with adapater = Object.class, or even better using a marker 
interface. This would better fit the semantics of the ModelFactory's purpose. 
Wdyt?

> Remove untyped getModelFrom* methods from ModelFactory
> ------------------------------------------------------
>
>                 Key: SLING-6655
>                 URL: https://issues.apache.org/jira/browse/SLING-6655
>             Project: Sling
>          Issue Type: Wish
>    Affects Versions: Sling Models Impl 1.3.0
>            Reporter: Dirk Rudolph
>
> As far as I understood the the whole story behind Sling Models API is/was 
> that it can be used to adapt ordinary objects to typed objects. With adding 
> {{Object getModelFromResource(...)}} and {{getModelFromRequest(...)}} this 
> paradigm has been broken. Just looking at the API, what is the purpose of 
> that methods? I agree that it makes much sense to have a binding between the 
> resourceType of {{Resource}} and maybe even {{SlingHttpServletRequest}} and a 
> model, but this resulting into an ordinary object from API perspective makes 
> it kind of pointless. 
> I propose:
> * mark them as deprecated as already done in SLING-6652
> * let them throw {{UnsupportedOperationException}} to prevent them being used
> * remove them in the next major API release
> * move the logic of "finding the nearest implementation by resourceType* to 
> {{ResourceTypeBasedResourcePicker}}
> and for the exporter:
> * if a {{@Model}} has an {{@Exporter}} implicitly registered it with the 
> {{implType}} as {{adapterType}}, if not already done
> * use the {{implType}} in the {{ExporterServlet}} to adapt from request or 
> {{Resource}} to the model object



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to