Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-12 Thread Bertrand Delacretaz
On Tue, Jun 12, 2018 at 12:37 PM Julian Sedding wrote: > ...then came > ResourceUtil.getValueMap(resource), which returns a modifiable empty > ValueMap even if resource == null. And finally, a lot later, > resource.getValueMap() was added, because ValueMaps are used a lot > when dealing with

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-12 Thread Julian Sedding
I share Dan's opinion that it makes sense to offer the functionality via a utility of some sort (be it adaptTo or static methods or even constructors ;)). In the past that was also how the Sling API evolved. E.g. first there was resource.adaptTo(ValueMap.class), then came

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-07 Thread Daniel Klco
On Thu, Jun 7, 2018 at 9:05 AM, Bertrand Delacretaz wrote: > On Thu, Jun 7, 2018 at 3:00 PM, Jason E Bailey wrote: > > ...I really don't think creating a dedicated bundle that consists of a > single class > > is really appropriate > > That's not necessarily a problem, especially if it's

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-07 Thread Bertrand Delacretaz
On Thu, Jun 7, 2018 at 3:00 PM, Jason E Bailey wrote: > ...I really don't think creating a dedicated bundle that consists of a single > class > is really appropriate That's not necessarily a problem, especially if it's meant to avoid touching our api bundle, which we're always quite

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-07 Thread Jason E Bailey
Or, you know, new Streamer(resource) :) I really don't think creating a dedicated bundle that consists of a single class is really appropriate. I've gone ahead and closed the pull request because I've come to realize that I'm doing an extremely poor job at explaining the vision behind this

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-06 Thread Bertrand Delacretaz
On Wed, Jun 6, 2018 at 7:23 PM, Jörg Hoh wrote: > ...Streamer stream = resource.adaptTo(Streamer.class); > could do the trick. Then you can have everything in a dedicated bundle and > there is no need to change any of the sling core APIs or bundles +1 -Bertrand

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-06 Thread Jörg Hoh
Hi 2018-06-06 17:01 GMT+02:00 Jason E Bailey : > This started out as a separate Object which was part of a bigger set of > code, from which the feedback eventually had it end up as a single method > in the Resource interface. > Resource implements the very flexible Adapter interface, so I

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-06 Thread Bertrand Delacretaz
On Wed, Jun 6, 2018 at 3:53 PM, Carsten Ziegeler wrote: > ...I see the value of having this as a library > everyone can use. But I think we usually should keep library code > separate from the pure (in lack of a better word) api like the resource > api... I agree with that. IIUC the current

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-06 Thread Carsten Ziegeler
Because the resource resolver bundle is a provider (not consumer) of the resource package and therefore has a narrow version import range, basically limited to a specific minor version. It implements a couple of interfaces from that package. So adding anything to that package comes with this

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-06 Thread Jason E Bailey
1. good to know. 2. why? - Jason On Wed, Jun 6, 2018, at 9:54 AM, Carsten Ziegeler wrote: > Forgot one thing: each time we change the resource api package, we need > to re-release the resource resolver (although we might not need to > implement anything there) - which is another indicator

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-06 Thread Jason E Bailey
This started out as a separate Object which was part of a bigger set of code, from which the feedback eventually had it end up as a single method in the Resource interface. The libraries I mentioned would be entirely separate, all of this came from here:

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-06 Thread Carsten Ziegeler
Forgot one thing: each time we change the resource api package, we need to re-release the resource resolver (although we might not need to implement anything there) - which is another indicator whether the resource api package is a good candidate or not Regards Carsten Carsten Ziegeler wrote >

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-06 Thread Carsten Ziegeler
I haven't really followed the whole discussion but the keyword in your a response to me is "library". I see the value of having this as a library everyone can use. But I think we usually should keep library code separate from the pure (in lack of a better word) api like the resource api. There are

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-06 Thread Jason E Bailey
Hi Jörg, I think this is, in part, a matter of personal experience. My team and I have found traversing a sub tree traversal so useful that I've created libraries of predicates and an expression language to ease the creation of predicates for those traversals. I would estimate that we use a

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-05 Thread Jörg Hoh
Hi, sorry for joining the game a bit late ... At the moment it's not clear to me, why we actually want to extend the API with this functionality; while I totally understand the value and beauty of streams, I wonder if the usecase "traverse the whole subtree" happens really that often that it

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-05 Thread Jason E Bailey
Robert - I've implemented a version of the traversal using a stack of iterators. Conceivably less memory consumption depending on the underlying implementation, but it shouldn't be more then what I had. I'll leave the pull request open for two more days, and then merge and do the release.

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-05 Thread Jason E Bailey
Realized I wasn't clear in how the method declaration would be problematic. example: > > - stream(Predicate, Set... ) take the invocation - stream(predicate, CHILDREN_ONLY); since the first predicate is a structural predicate in the manner that it determines what child resources to traverse

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-05 Thread Daniel Klco
+1 for the proposed methods I would also encourage adding the predicate classes into the API. On Mon, Jun 4, 2018 at 2:24 PM, Jason E Bailey wrote: > At this point I'm looking for feedback to say whether there is a consensus > that the proposed API > > - stream() > - stream(Predicate) > >

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-04 Thread Jason E Bailey
At this point I'm looking for feedback to say whether there is a consensus that the proposed API - stream() - stream(Predicate) where each one attempts to iterate through the tree is *sufficient enough for the initial release*. I believe Stefan's concern was that he believed someone

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-04 Thread Jason E Bailey
You got it. Thats's what I was attempting to describe and that's the type of filtering we do a lot of. - Jason On Mon, Jun 4, 2018, at 1:58 PM, Daniel Klco wrote: > It seems redundant, but I think they're somewhat different concepts. The > Predicate passed into the stream method is for

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-04 Thread Daniel Klco
It seems redundant, but I think they're somewhat different concepts. The Predicate passed into the stream method is for evaluating the Resources relevant for traversal, whereas (at least I'm assuming) that any subsequent filters would filter the subsequent stream of resources. Having that sort of

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-04 Thread Jason E Bailey
- Jason On Mon, Jun 4, 2018, at 12:35 PM, Daniel Klco wrote: > > Rather than having another parameter, what about providing a > ResourceChildrenPredicate? Based on the current API it looks like you'd > have to provide the current resource to make this work, but it seems like > it would be

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-04 Thread Jason E Bailey
Thanks Robert! - Jason On Mon, Jun 4, 2018, at 11:45 AM, Robert Munteanu wrote: > There are two questions I want to raise: > > 1. API related > > We have two methods: > > - stream() > - stream(Predicate) > > which traverse the whole tree. the base stream() would traverse the whole tree,

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-04 Thread Daniel Klco
On Mon, Jun 4, 2018 at 11:45 AM, Robert Munteanu wrote: > Hi Jason, > > On Thu, 2018-05-31 at 10:39 -0400, Jason E Bailey wrote: > > https://github.com/apache/sling-org-apache-sling-api/pull/4 > > > > I've created a pull request to add a new class to the Sling API > > bundle that would allow you

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-04 Thread Robert Munteanu
Hi Jason, On Thu, 2018-05-31 at 10:39 -0400, Jason E Bailey wrote: > https://github.com/apache/sling-org-apache-sling-api/pull/4 > > I've created a pull request to add a new class to the Sling API > bundle that would allow you to create a Stream object which > traverses a resource tree. > >

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-06-04 Thread Jason E Bailey
Went with adding the Stream to the Resource interface directly as a default method. This is more intuitive and results in less code. There has been a concern expressed about the naming methodology as to whether Resource.stream() should by default be the children or the entire subtree. - Jason

Re: [DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-05-31 Thread Jason E Bailey
Per some feedback from Stefan. I added the streaming capability directly to the Resource interface as default methods. Which is just really, really, cool. The current pull request has both the separate class and the Resource modifications. Once I've gathered feedback I'll remove the option

[DISCUSSION][API][PROPOSAL] Adding Stream generator to Sling API

2018-05-31 Thread Jason E Bailey
https://github.com/apache/sling-org-apache-sling-api/pull/4 I've created a pull request to add a new class to the Sling API bundle that would allow you to create a Stream object which traverses a resource tree. Additionally, the ResourceStream class has convenience methods to specify a