[jira] [Comment Edited] (SLING-4447) Improve JavaUseProvider to not fall back to a simple Pojo instanciation in case a Java class with @Model annotation cannot be instanciated

["Radu Cotescu (JIRA)" <ji...@apache.org>]

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

Radu Cotescu edited comment on SLING-4447 at 2/25/15 6:32 PM:
--------------------------------------------------------------

I briefly reviewed the patch. Here are my comments:

* make the provider as lazy as possible: first check if the class exists by querying the DCLM, then do all the checks
* be consistent when errors occur: return a {{ProviderOutcome}} initialised with a failure cause  - {{ProviderOutcome#failure(Throwable cause)}}
* use a provider lower than 100 and higher than 90 - we want this {{UseProvider}} to kick in before the {{JavaUseProvider}} but after the {{RenderUnitProvider}}, for performance reasons

To overcome [~jsedding]'s concern we should mark Sightly as an optional package import.


was (Author: radu.cotescu):
I briefly reviewed the patch. Here are my comments:

* make the provider as lazy as possible: first check if the class exists by querying the DCLM, then do all the checks
* be consistent when errors occur: return a {{ProviderOutcome}} initialised with a failure cause  - {{ProviderOutcome#failure(Throwable cause)}}
* use a provider lower than 100 and higher than 90 - we want this {{UseProvider}} to kick in before the {{JavaUseProvider}} but after the {{RenderUnitProvider}}, for performance reasons

> Improve JavaUseProvider to not fall back to a simple Pojo instanciation in case a Java class with @Model annotation cannot be instanciated
> ------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: SLING-4447
>                 URL: https://issues.apache.org/jira/browse/SLING-4447
>             Project: Sling
>          Issue Type: Improvement
>          Components: Scripting
>    Affects Versions: Scripting Sightly Engine 1.0.0
>            Reporter: Konrad Windszus
>            Assignee: Konrad Windszus
>         Attachments: SLING-4447-v01.patch
>
>
> Currently in case a Java class is a Sling Model (i.e. has the Model annotation) and cannot be instanciated (e.g. required injections not possible) Sightly falls back to instanciate those as simple Pojos.
> This is never good, since a lot of NullPointerException happen then, because all injections have not been performed then.
> Therefore in the following code should be used instead 
> {code}
> if (modelFactory.isModelClass(resource, cls)) {
>   if (modelFactory.canCreateFromAdaptable(resource, cls)) {
>     obj = modelFactory.createModel(resource, cls);
>   } else if (modelFactory.canCreateFromAdaptable(request, cls)) {
>     obj = modelFactory.createModel(request, cls);
>   } else {
>     throw new IllegalStateException("Could not adapt the given Sling Model from neither resource nor request: " + cls);
>   }
> }
> {code}
> That way, exceptions would be propagated in case a Sling model cannot be instanciated and developers more easily see why the Sling Model does not work (instead of running into NullPointerExceptions in their model because it was instanciated as simple 



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