sling-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Julian Sedding (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (SLING-7759) add other "patterns" than path for filters
Date Wed, 04 Jul 2018 14:45:00 GMT

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

Julian Sedding commented on SLING-7759:
---------------------------------------

[~npeltier] looks mostly fine to me. A few remarks below.

- I would rename "sling.filter.pattern.suffix" to "sling.filter.suffix.pattern", because it's
a pattern to match the suffix, rather than a suffix for the pattern.
- I like factoring the checks out into {{FilterPredicate}}, nice separation of concerns!
- It's always better to avoid new dependencies on core modules, but you already said in the
PR that you'd remove commons-lang3 again.
- As you remark, "sling.filter.paths" is probably not terribly useful. Furthermore, it can
also be achieved via "sling.filter.pattern". So i would drop it.
- Handling between path and suffix matching is slightly different, as path is set to "/" if
it is blank. Is this desired or accidental?
- I took a stab at simplifying {{FilterPredicate}}, I didn't like the loop with lots of if
statements. See https://github.com/npeltier/sling-org-apache-sling-engine/compare/SLING-7759...jsedding:npeltier/SLING-7759

> add other "patterns" than path for filters
> ------------------------------------------
>
>                 Key: SLING-7759
>                 URL: https://issues.apache.org/jira/browse/SLING-7759
>             Project: Sling
>          Issue Type: Improvement
>          Components: Engine
>    Affects Versions: Engine 2.6.12
>            Reporter: Nicolas Peltier
>            Assignee: Nicolas Peltier
>            Priority: Major
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> adding additional patterns to filters (right now {{sling.filter.pattern}} allows to include
them based on path only):
> - path passes if sling.filter.pattern (i propose we keep it for legacy &  add sling.filter.pattern.path
for consistency with what follow) regexp matches current path,
> - selector passes if sling.filter.pattern.selector is in the selector chain,
> - method passes if sling.filter.pattern.method is equals to curren method,
> - suffix  passes if sling.filter.pattern.suffix regep matches current suffix



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message