sling-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robert Munteanu <romb...@apache.org>
Subject Resource Filter API ( was: [VOTE] Release Apache Sling Resource Filter version 1.0.0)
Date Thu, 23 Aug 2018 09:19:36 GMT
Hi Jason,

On Tue, 2018-08-21 at 08:30 -0400, Jason E Bailey wrote:
> Hi Robert,
> 
> I'll need to clarify the documentation. 
> 
> The ResourceFilter is the object that converts a String to a
> Predicate<Resource> , there are cases where you want the
> ResourceFilter Script to evaluate against a provided java object, The
> ResourceFilter object has methods to add objects to the context that
> the parsed script is ran against.
> 
> Because of that, I choose to make the ResourceFilter Object maintain
> state. Since it maintains state, I needed a provider to create a
> unique instance of ResourceFilter. I couldn't just do new
> ResourceFilter() because I need to encapsulate and separate the
> parser references and I eventually will be adding an ability to
> define new functions for the ResourceFilter Script by way of
> additional services.

I took a look again at the exposed API classes and try to explain to
myself what they do. Again, this is subjective and might be incorrect,
so take it with a grain of salt. 

1. ResourceFilter

This looks like it has the job of building Predicate<Resource>
instances.

2. ResourceFilterException

Checked exception, signals parsing errors

3. ResourceFilterStream

Utility class to create a stream from a resource + filter

4. ResourceStream

Class that is able to create a stream of of resources.

Is that correct?

------

My notes are as follows:

a. Do we need custom exceptions? I.e., do we expect consumers to
try/catch when parsing and then do something about the error? If yes,
we keep the exception, otherwise we could resort to something like an
IllegalArgumentException

b. The ResourceFilter seems to conflate building instances (parse())
and accumulating parameters (addParameter). Not a big deal but can be a
bit hard to parse at first ( excuse the pun ).

Typically I'd see these as builder classes, with an API such as

    ResourcePredicates.withParameter(...).parse(....)

c. The ResourceFilterStream is an almost 1:1 wrapper over the
ResourceStream. Do we really need both?

d. Adapting a resource to a ResourceFilter is a bit confusing, as the
ResourceFilter does not have any state based on the resource it adapts
from. This also tied into b. above.

Thanks for your patience with the release and the review :-)

Best,

Robert


Mime
View raw message