[jira] [Reopened] (DIRAPI-165) Add a FilterBuillder

["lucas theisen (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/DIRAPI-165?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

lucas theisen reopened DIRAPI-165:
----------------------------------

[~elecharny]
{quote}
Currently, the {{FilterBuilder}} class is the only one that has everything but the ASF header, and I must admit it's quite well documented. It would be cool if the other classes have a bit of doco, too. Not necessarily that extensive, but still. For test, we don't really care about Javadoc, except for specific tests.
{quote}
Thank you for adding the headers, I completely forgot that.  The reason I only javadoc'd FilterBuilder is because it is the only public interface to the FilterBuilder (a facade).  All the other classes are package only classes and as such, internal only.  Do we still javadoc those?
{quote}
Regarding the {{Operator}} enum, we do have an enum in the ldap-model module that could have been used, or at least improved : AssertionType. I think there is some room for improvement in this enum. I also think that there is no need to declare two inner Operator - in UnaryFilter and setOfFiltersFilter -.
{quote}
I am not sure I understand this comment.  I initially had a single enum, but split it up to provide compile time assurance that the wrong enum value is impossible to supply (cannot give an {{Operator.NOT}} to an {{AttributeValueAssertion}} constructor).  However, as I ended up leaving the constructors private, this probably is an unnecessary protection.  I could improve the {{AssertionType}} enum you mentioned, but it has some strange values: {{EXTENSIBLE, SCOPE, ASSERTION, UNDEFINED, OBJECTCLASS}}.  The don't actually have operators, so it would not be possibly to have the operator method in there.  Any suggestion on this?  Could we remove those 5 values (i assume you put them there for a reason so probably not)?  I can look to see where they are used if you think i should.
{quote}
Last, not least, the Filter.build(...) is clearly missing a lot of things : the attribute value is to be escaped wrt RFC 4515  (assertionvalue) :
{quote}
I thought about adding escaping, but at the same time I thought it may be confusing to the user.  Anybody familiar with LDAP searches knows that {{*}} is a glob so I assume they would want to do:
{code}
    equals( "givenName", "emman*" );
{code}
or something similar.  If we internally escape that during the {{build()}} method, it would break their search.  I understand that they _should_ be using a substring (which I left out because I assumed they would just put in their own {{*}} on an {{equals}}).  Anyway, I saw that you added a {{substring}} and, given that, I could certainly add the proper escapes to the build method.  Do you think doing escaping would confuse others?  It is certainly more correct, and safer, but I could see people missing this quirk.  Last, I noticed in your implementation of substring has the signiture:
{code}
    public static FilterBuilder substring( String attribute, String initial, String end, String... any )
{code}
Which has middle after the end.  I understand this was done to leverage the varargs, but using it is very weird.  What do you think of breaking the {{substring}} function up into:
{code}
    /** results in [part_0](*[part_n]) */
    public static FilterBuilder substring( String attribute,  String... parts)
    /** results in [part_0](*[part_n])* */
    public static FilterBuilder startsWith( String attribute, String... parts)
    /** results in *[part_0](*[part_n]) */
    public static FilterBuilder endsWith( String attribute, String... parts)
    /** results in *[part_0](*[part_n])* */
    public static FilterBuilder contains( String attribute, String... parts)
{code}
This way the parts always show up in order.

What do you think?


> Add a FilterBuillder
> --------------------
>
>                 Key: DIRAPI-165
>                 URL: https://issues.apache.org/jira/browse/DIRAPI-165
>             Project: Directory Client API
>          Issue Type: New Feature
>    Affects Versions: 1.0.0-M21
>            Reporter: lucas theisen
>            Priority: Minor
>             Fix For: 1.0.0-M29
>
>
> Looking for something Fluent, in the _spirit_ of Hibernate Criteria.  May not seem like much, but can drastically reduce query syntax issues. Also, you would likely be using a StringBuilder anyway, so its not much different. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)