[ https://issues.apache.org/jira/browse/SOLR-11914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16439410#comment-16439410 ]
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 (v7.6.3#76005) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org