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

Carsten Ziegeler commented on SLING-4275:
-----------------------------------------

{quote}
* 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
{quote}
Right, but do we really need these different runtime exceptions? Each service 
having it's own runtime exception seems like a vaste to me, especially as the 
caller does not do anything with such exceptions.

bq. 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.
As ExtensionInstance has a single method, I think we could go without this 
extra layer of object caching and go with a singleton

bq. 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.
bq. I think this is another case of incomplete JavaDoc.

So I'll wait for the javadoc update - what about ValueMap?

bq. 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.

So we start with something which will be added to the Sling API later on and 
then we deprecate this stuff again? Maybe we should at least put this in a 
separate package so we can deprecate the whole package later on

bq. 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.

I was not refering to making the RenderContext itself a service - I'm refering 
to the utility methods in that interface that handle Object traversal and value 
conversion - I don't think these methods belong to RenderContext - they can 
either be moved to an utility class or service.



> 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