David Smiley commented on SOLR-11914:

Here's a patch.
 * toMap, toMultiMap: Marked deprecated. There is some code affected in 
streaming expressions: FacetStream, SqlStream, TimeSeriesStream, 
GatherNodesStream. Most of the cases here looked very odd to me – a params was 
being converted to a Map to a NamedList and then back to a SolrParams. 
Oooookaaay. [~dpgove] could you take a look? You touched some of this before. I 
removed that round-tripping, and in several places took advantage of the new 
SolrParams.stream() method.
 * toFilteredSolrParams: Marked deprecated. Modified the only caller in 
SolrCore.preDecorateResponse to do filtering on the fly via a SolrParams inner 
class impl. The notion of filtering params based on the param name does sound 
useful but I think it could be added in a better way in the future as a view 
and with a more flexible predicate.
 * getAll: Marked deprecated. Copied these to CollectionsHandler as static 
package-protected methods called "copy". I moved the test to 
TestCollectionAPIs.java which is a so-so spot (none others in the package 
looked better); I didn't feel like creating a new test class just for this test.
 ** CollectionsHandler.forEach: removed this utility method which I added a 
month or two ago. Now that SolrParams implement Iterable, the desire for this 
utility is low.
 * toSolrParams(NamedList): Marked deprecated and moved the implementation to 
NamedList as an instance method. The impl now redirects to NamedList. I left a 
nocommit to remind me to adjust all callers (of which there are a ton – 40?) 
but didn't want to make this patch hard to read. I added some javadocs to 
toSolrParams... the null handling seems inconsistent.
 * MapWriter: added some docs & comments technically unrelated to the issue

[~noble.paul] I'd appreciate a review since I know you were involved in some of 
these methods.

> Remove/move questionable SolrParams methods
> -------------------------------------------
>                 Key: SOLR-11914
>                 URL: https://issues.apache.org/jira/browse/SOLR-11914
>             Project: Solr
>          Issue Type: Improvement
>      Security Level: Public(Default Security Level. Issues are Public) 
>          Components: SolrJ
>            Reporter: David Smiley
>            Priority: Minor
>              Labels: newdev
>         Attachments: SOLR-11914.patch
> {{Map<String, Object> getAll(Map<String, Object> sink, Collection<String> 
> params)}} 
> Is only used by the CollectionsHandler, and has particular rules about how it 
> handles multi-valued data that make it not very generic, and thus I think 
> doesn't belong here.  Furthermore the existence of this method is confusing 
> in that it gives the user another choice against it use versus toMap (there 
> are two overloaded variants).
> {{SolrParams toFilteredSolrParams(List<String> names)}}
> Is only called in one place, and something about it bothers me, perhaps just 
> the name or that it ought to be a view maybe.
> {{static Map<String,String> toMap(NamedList params)}}
> Isn't used and I don't like it; it doesn't even involve a SolrParams!  Legacy 
> of 2006.
> {{static Map<String,String[]> toMultiMap(NamedList params)}}
> It doesn't even involve a SolrParams! Legacy of 2006 with some updates since. 
> Used in some places. Perhaps should be moved to NamedList as an instance 
> method.

This message was sent by Atlassian JIRA

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

Reply via email to