Review of SampleFacetPredicateEvaluator

[davidjgonzalez]

Hi @alexkli Could you take a look at:

https://github.com/Adobe-Consulting-Services/acs-aem-samples/blob/master/bundle/src/main/java/com/adobe/acs/samples/querybuilder/facets/impl/SampleFacetPredicateEvaluator.java

And let me know if it makes sense, or if there are any efficiencies.

[alexkli]

As a sample, I think it might be confusing to have a predicate evaluator that just does facet extraction and is named "facet" (both predicate registration name and class name).

Rather, it would be better to have a predicate that does xpath (and filtering) plus a facet extraction for that same concept. And name it after that concept.

Not sure what a good minimal example would be - maybe something that checks 2 properties together or so (so it leads to a shorter query than using 2 normal property predicates).

Also, for the sake of performance, I'd avoid switching to the sling resource api - facet extractors and filtering are highly performance critical pieces of code. There should also be a warning about this since depending on the query and the content they might run for all nodes in the repository with every single query execution (if you are doing it wrong, but this can easily happen).

[davidjgonzalez]

@alexkli Thanks for the quick feedback.

For my own clarification; you can't attach a new FacetExtractor (FE) to an existing PredicateEvaluator (PE), correct? If not, can you point how i'd register to an existing PredicateEvaluator? I combined the PE with the FE since you need (AFIAK) a custom PE to use a custom FE; granted you could use a custom FE across multiple custom PEs. This was a point of confusion for me when trying to figure out FE fit into QueryBuilder.

Ill work on breaking this FacetExtractor out into its own Sample and create another PredicateEvaluator
Ill remove the Sling API and note why we wouldn't use it (performance).

[alexkli]

FE's are always tied to a PE. They are adressed the same way in queries - in fact, you just say "extract facets = true" and then it will do facets for all predicates of the query that are "empty" (i.e. don't specify a value to search for) and for which a FE exists.

[davidjgonzalez]

@alexkli Trying to find documentation for extract facets = true; is this ~ same as calling query.extractFacets(); (after query.getResults();) ? Can a custom FE be added to an existing PE?

Im thinking about how to reflect these during the sample "split". Thanks again for all the feedback.

[alexkli]

Yes, query.extractFacets(). I was referring to the feature of the /bin/querybuilder.json servlet where you pass p.facets=true to have it call extract facets and include them in the json result.

No, you can't attach a new FE to some existing PE that you cannot change.

[acollign]

Hi guys,

I wrote a simple predicate evaluator sample [0] and documented it as part of our official documentation [1]. I'll be happy to contribute it to this project. However I want to keep an upstream repo since we have a different approach which consists of installable code samples.

Btw, the code was reviewed by @alexkli before publishing.

[0] https://github.com/Adobe-Marketing-Cloud/aem-docs-custom-predicate-evaluator/blob/master/src/main/java/com/adobe/aem/docs/search/ReplicationPredicateEvaluator.java
[1] http://docs.adobe.com/content/docs/en/aem/6-0/develop/search/implementing-custom-predicate-evaluator.html

[davidjgonzalez]

@acollign sure! Nice simple examples are welcome! I believe this should implement a filter criteria though. Also we are trying to ensure the ACS Samples have a narrative in comments that explain why certain code choices were made and what it's doing.