[
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)
