Hi
2017-01-29 10:50 GMT-05:00 arjan tijms <arjan.tijms_at_gmail.com>:
> Hi,
>
> I'm reviewing the search expression API classes a little. First of all,
> I'm *very* impressed by the quality of the work, this is really well done!
>
>
Yes, it took a lot of time and a lot of iterations to get it done.
> I did found a tiny tiny issue in the API class
> javax.faces.component.search.ComponentNotFoundException though. It's
> almost too small to mention, but for completeness: it doesn't declare a
> static final serialVersionUID field.
>
> Another small issue that I noticed is in the Javadoc of:
>
> javax.faces.application.Application.addSearchKeywordResolver(
> SearchKeywordResolver)
>
> It currently says:
>
> /**
> * <p class="changed_added_2_3">Cause an the argument
> <code>resolver</code> to be added to the resolver chain.
> *
> * It is not possible to remove an {_at_link SearchKeywordResolver}
> * registered with this method, once it has been registered.</p>
> *
> * <p>The default implementation throws
> * <code>UnsupportedOperationException</code> and is provided
> * for the sole purpose of not breaking existing applications that
> extend
> * {_at_link Application}.</p>
> *
> * @throws IllegalStateException if called after the first
> * request to the {_at_link javax.faces.webapp.FacesServlet} has
> been serviced.
> * @throws NullPointerException when resolver is null.
> *
> * @param resolver the SearchKeywordResolver to add.
> * @since 2.3
> */
>
> There's a small typo -> "Cause an the ..."
>
> But more importantly, it doesn't say if the new SearchKeywordResolver is
> added in front of the chain or at the end.
>
>
In the implementation it's very explicitly added to the front, so it can
> override existing resolvers. Perhaps a small clarification here would help?
>
>
New SearchKeywordResolver instances must be put on top. The idea is if the
developer wants to override an existing keyword, it should be able to do it
just adding it to the chain, just like with EL resolvers.
regards,
Leonardo Uribe
> Kind regards,
> Arjan Tijms
>
>
>
>
>
>
>
>