I've felt we need one or two less maybe.  MultiMapSolrParams can often
be substituted by ModifiableSolrParams.  Javadocs could be improved in
general.

RequiredSolrParams and DefaultSolrParams (plus subclasses) could be
package scope so as to discourage direct use, which I think is never
needed.

On Wed, Apr 3, 2024 at 10:52 AM Gus Heck <gus.h...@gmail.com> wrote:
>
> We have quite a few, including MapSolrParams which seems to explicitly defy
> the contract of SolrParams being a multimap...
>
> Has anyone spent any time considering if we really need all these variants:
>
>    - AppendedSolrParams
>    - DefaultSolrParams
>    - DocRowParams
>    - MapSolrParams
>    - ModifiableSolrParams
>    - MultiMapSolrParams
>    - RequiredSolrParams
>    - SolrQuery
>    - VersionedParams
>
> Additionally there seem to be 3 places where we create anonymous subclasses
> of SolrParams...
>
> Most of these seem to have some angle or additional fields, so they aren't
> useless per-se but it does make it hard to reason about the behavior of a
> method that accepts SolrParams... for example RequestUtil.processParams()
> has the suspicious code:
>
> public static void processParams(
>     SolrRequestHandler handler,
>     SolrQueryRequest req,
>     SolrParams defaults,
>     SolrParams appends,
>     SolrParams invariants) {
>
>   boolean searchHandler = handler instanceof SearchHandler;
>   SolrParams params = req.getParams();
>
>   // Handle JSON stream for search requests
>   if (searchHandler && req.getContentStreams() != null) {
>
>     Map<String, String[]> map = MultiMapSolrParams.asMultiMap(params, false);
>
>     if (!(params instanceof MultiMapSolrParams || params instanceof
> ModifiableSolrParams)) {
>       // need to set params on request since we weren't able to access
> the original map
>       params = new MultiMapSolrParams(map);
>       req.setParams(params);
>     }
>
>
> This seems to A) possibly fail to restrict defaults and appends to the
> actual subclasses that are associated with that functionality (and if that
> is not a good idea, why do we have them?), and B) discard the request
> parameters if someone uses anything other than the expected two subclasses
> of SolrParams in the request.
>
> Has this been considered before?
>
> Archives search only brought up my previous irritations with SolrParams
> implementations...
>
> https://lists.apache.org/thread/tkoj75z736x1nzotgh2xsn7wdnnsoc8g
>
> -Gus
>
> --
> http://www.needhamsoftware.com (work)
> https://a.co/d/b2sZLD9 (my fantasy fiction book)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@solr.apache.org
For additional commands, e-mail: dev-h...@solr.apache.org

Reply via email to