nick-boss-tech opened a new pull request, #5132: URL: https://github.com/apache/solr/pull/5132
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)* https://issues.apache.org/jira/browse/SOLR-6207 ## What happens today **After `setParams` is called, `getParamString()` still shows the original request parameters.** [`SolrQueryRequestBase.getParamString()`](https://github.com/nick-boss-tech/solr/blob/cabedd1d968059215188f4e7563fb303241899ed/solr/core/src/java/org/apache/solr/request/SolrQueryRequestBase.java#L171-L174) returns `origParams`. [`setParams`](https://github.com/nick-boss-tech/solr/blob/cabedd1d968059215188f4e7563fb303241899ed/solr/core/src/java/org/apache/solr/request/SolrQueryRequestBase.java#L92-L95) replaces only the current parameters. A caller that reads a new `q` through `getParams()` gets the new value. A log line built from `getParamString()` still shows the old one. The request log uses this string ([SolrCore.java line 2927](https://github.com/nick-boss-tech/solr/blob/cabedd1d968059215188f4e7563fb303241899ed/solr/core/src/java/org/apache/solr/core/SolrCore.java#L2927)). The interface javadoc says only that the string shows "all the important parameters," so it does not say which set it shows. The ticket reports this. The links in this paragraph point at the base commit, b efore this change. ## What this change does **The javadoc now states what the method returns and what it leaves out.** The change is to the interface javadoc of [`SolrQueryRequest.getParamString()`](https://github.com/nick-boss-tech/solr/blob/b099a9f1be5a3fb529767ca625f42e7860e62f13/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java#L128-L134). The only production edit is javadoc. On the request-handler path, defaults, appends and invariants are merged in with `setParams` ([RequestUtil.java line 182](https://github.com/nick-boss-tech/solr/blob/b099a9f1be5a3fb529767ca625f42e7860e62f13/solr/core/src/java/org/apache/solr/request/json/RequestUtil.java#L182)), so they are not part of the original parameters. ## Proof **This test records existing behavior. It does not fail on the base code, and it is not meant to.** [SolrQueryRequestBaseTest](https://github.com/nick-boss-tech/solr/blob/b099a9f1be5a3fb529767ca625f42e7860e62f13/solr/core/src/test/org/apache/solr/request/SolrQueryRequestBaseTest.java#L25-L39) checks that `getParamString()` still returns the original parameters after `setParams`. The ticket treats that behavior as intended. The test also passes with the production edit reverted to base, because that edit is javadoc only. At head `b099a9f1be5a3fb529767ca625f42e7860e62f13`: SolrQueryRequestBaseTest 1 of 1 passes. ## A choice to check **The change documents the method and keeps it. It does not deprecate it.** The options are to document what the method returns, as this change does, or to deprecate the method. A 2014 comment on the ticket suggests deprecating and removing it, because readers must check the javadoc to learn which parameters it returns. Deprecation would change the API for code that calls the method. Was documenting the method the right call, or should it be deprecated? ## Limits **The javadoc describes one path, and the method stays as it is.** - The javadoc describes the request-handler path. Other code that builds a request from already merged parameters would show defaults in the original parameters. Those paths were not checked. - The method is not deprecated or removed. See the choice above. Changelog: [changelog/unreleased/SOLR-6207-getparamstring-javadoc.yml](https://github.com/nick-boss-tech/solr/blob/b099a9f1be5a3fb529767ca625f42e7860e62f13/changelog/unreleased/SOLR-6207-getparamstring-javadoc.yml) ### AI assistance AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
