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 tons of useful libraries out there, if we would add all of them to the resource api, I guess we would create one of the biggest apis in the world :)
But I guess it has already been considered whether this should go somewhere else or not. So just my 2cents Regards Carsten Jason E Bailey wrote > 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 traversal about 2 - 3 > times a month, sometimes as part of a new component, or when pulling reports > and operational queries. > > We use it, when writing code, more then get/list children of a resource. > > I do think the ability to do a traversal should be part of a core library, > the reasoning for that is that there are currently multiple instances of a > some sort of tree traversal in multiple bundles now. From Sling Query to the > default GET servlets. When I start seeing the same thing over and over again > that tells me it should be centralized. > > I did just commit one change based on this conversation, since their tends to > be a focus on "whole subtree" which is something I don't actually use, I > removed the method > > - stream(); > > leaving just > > - stream(Predicate<Resource>) > > That leaves open the possibility of static predicates to do things like > > stream(CHILDREN); > stream(ALL_DESCENDANTS) > stream(PAGES) > > As for extensions of existing methods, I think there would be an interest in > that. maybe something like > > Stream<Resource> listChildren(Predicate<Resource> childSelector); > > which I think would make a great patch. > > I appreciate the feedback. > > Thanks! > - Jason > > On Tue, Jun 5, 2018, at 6:30 PM, Jörg Hoh wrote: >> 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 qualifies >> for an extension of the API. Personally I implemented it once or twice over >> the course of some years, but that's not something I need to have in the >> public API. I consider it as a convenience function I can easily implement >> on top of the already existing Resource API. >> >> I would rather provide versions of the listChildren method which return >> streams (or any other method of the Resource API which returns a list or >> an iterator over resources). >> >> Jörg >> >> >> 2018-06-04 20:24 GMT+02:00 Jason E Bailey <j...@apache.org>: >> >>> At this point I'm looking for feedback to say whether there is a consensus >>> that the proposed API >>> >>> - stream() >>> - stream(Predicate<Resource>) >>> >>> 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 encountering stream() on a resource would believe that it's only a >>> stream of the immediate children. >>> >>> I believe, he would prefer something like >>> >>> - treeStream() >>> >>> I'm inclined to see just stream() as the base method, as that's the >>> standard convention for a method that generates a stream. >>> >>> feedback appreciated. >>> Also there is a new pull request now that I've rolled everything back >>> https://github.com/apache/sling-org-apache-sling-api/pull/5 >>> >>> >>> >>> - Jason >>> >>> On Mon, Jun 4, 2018, at 2:02 PM, Jason E Bailey wrote: >>>> 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 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 redundancy would allow you to traverse a structure like the >>> following: >>>>> >>>>> - myasset.jpg (dam:Asset) >>>>> - renditions (nt:folder) >>>>> - original (nt:file) >>>>> - rendition1.jpg (nt:file) >>>>> >>>>> so if you wanted to get just a stream of the nt:file resources you >>> could do: >>>>> >>>>> myassetResource.stream(maxDepth(2)).filter(isResourceType("nt:file")). >>> forEach(e >>>>> -> {DO_SOMETHING}); >>>>> >>>>> I like the idea of adding the predicates either in the API or as a >>> separate >>>>> dependency as there are likely a few common patterns that would be used >>>>> quite a bit. >>>>> >>>>> -Dan >>>>> >>>>> On Mon, Jun 4, 2018 at 1:31 PM, Jason E Bailey <j...@apache.org> wrote: >>>>> >>>>>> >>>>>> >>>>>> - 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 very useful to have a predicate which would only allow >>> for >>>>>> both >>>>>>> direct children or children up to a particular depth. I'd see it >>> useful >>>>>> to >>>>>>> provide 2-3 different default predicates to help with common >>> activities: >>>>>>> >>>>>>> ResourceChildrenPredicate - filter the stream of resources based >>> on their >>>>>>> child status >>>>>>> ResourceTypePredicate - filter the stream of resource based on >>> their >>>>>>> resource type >>>>>>> ValuePredicate - filter the stream of resources based on a value >>> in the >>>>>>> resource's ValueMap >>>>>>> >>>>>>> >>>>>> >>>>>> Funny you should mention this. The original ResourceStream object >>> came >>>>>> from the whiteboard streams project. Which has a whole package >>> dedicated >>>>>> to defining static predicates. There's even an expression languages >>> in >>>>>> their as well. However, for filtering, there's already a filter >>> method that >>>>>> a Stream provides. It's redundant to have a pre-filtered stream. >>>>>> >>>>>> However, as a limitation on the stream, you can do the same thing. It >>>>>> would something like >>>>>> >>>>>> - resource.stream(CHILD_RESOURCES); >>>>>> >>>>>> or >>>>>> >>>>>> - resource.stream(maxDepth(1)); >>>>>> >>>>>> maxDepth being a static method that you've imported from the >>> appropriate >>>>>> library that provides a Predicate >>>>>> >>> >> >> >> >> -- >> Cheers, >> Jörg Hoh, >> >> http://cqdump.wordpress.com >> Twitter: @joerghoh -- Carsten Ziegeler Adobe Research Switzerland cziege...@apache.org
