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

Felix Meschberger commented on SLING-4275:
------------------------------------------

bq. 1. There are several exceptions, like SightlyException, 
RuntimeExtensionException, SightlyUseException - are these really required? 

* SightlyException is the "root" exception of all of Sightly.
* SightlyUseException may be thrown by the UseProvider API. Respective JavaDoc 
mention is missing
* RuntimeExtensionException may be thrown by the RuntimeExtension API. 
Respective JavaDoc is missing

bq. 2. RuntimeExtension and ExtensionInstance

RuntimeExtension.provide is called once per script evaluation while 
ExtensionInstance objects may be called for each occurrence of the extension's 
use in the script. As such this is kind of a caching mechanism.

bq. 3. The purpose of the Record interface is a little bit unclear to me.

The Record interface supports for simplified property access in a much simpler 
way than the Map interface would allow. Also Pojo's in use 
attributes/statements may implement the Record interface to actually inject 
properties into the current evaluation scope.

I think this is another case of incomplete JavaDoc.

bq. 4. ResourceResolution seems to contain methods for getting scripts. Isn't 
this some functionalty the existing ServletResolver component provides? Or if 
not, shouldn't we move this to the Sling API?

Yes, this should probably be moved to the Sling API. But we did not want to tie 
the release of Sightly to an update of the Sling API: The Sling API bundle 
cannot alwas be simply updated in a running system unless you want to accept 
ripple effects due to extended API in other packages, such as the resource API.

bq. 5. Many of the methods in the RenderContext are Object traversal and value 
conversion methods. I think this needs to go into a separate service (and maybe 
package)

The RenderContext is created for script evaluation and handed down the 
evaluation stack and extensions. I don't think this needs to be exposed as a 
service.

In general: JavaDoc on all the exported API is not complete yet and must still 
be worked on.

> Review of the Sightly API
> -------------------------
>
>                 Key: SLING-4275
>                 URL: https://issues.apache.org/jira/browse/SLING-4275
>             Project: Sling
>          Issue Type: Task
>          Components: Scripting
>            Reporter: Carsten Ziegeler
>             Fix For: Scripting Sightly Engine 1.0.0
>
>
> I have some comments about the public Sightly API:
> 1. There are several exceptions, like SightlyException, 
> RuntimeExtensionException, SightlyUseException - are these really required? 
> The API does not mention any method/interface throwing such an exception
> 2.  RuntimeExtension and ExtensionInstance - I think we can get rid of the 
> RuntimeExtension interface and simply pass the RenderContext in 
> ExtensionInstance#call.
> 3. The purpose of the Record interface is a little bit unclear to me. In 
> addition it has a T get(String) method while properties returns a 
> Set<String>. I guess the engine supports a Map, so maybe we can get rid of 
> this interface?
> 4. ResourceResolution seems to contain methods for getting scripts. Isn't 
> this some functionalty the existing ServletResolver component provides? Or if 
> not, shouldn't we move this to the Sling API?
> 5. Many of the methods in the RenderContext are Object traversal and value 
> conversion methods. I think this needs to go into a separate service (and 
> maybe package)



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

Reply via email to