SOLR-16368: Use SolrClient type instead of overly specific subclasses

[epugh]

trafficstars

https://issues.apache.org/jira/browse/SOLR-16368

Description

Where can we use SolrClient or SolrCloudClient instead of the sub classes?

Solution

hunting through source!

Tests

Make the change, run the tests...

Checklist

Please review the following and check all that apply:

• [ ] I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
• [ ] I have created a Jira issue and added the issue ID to my pull request title.
• [ ] I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
• [ ] I have developed this patch against the main branch.
• [ ] I have run ./gradlew check.
• [ ] I have added tests for my changes.
• [ ] I have added documentation for the Reference Guide

[epugh]

One thing I've noticed is a lot of places where we cast to a specific client just to get the httpClient. We do it all over the place in the tests...

solrClient = getCloudSolrClient(cluster);
httpClient = (CloseableHttpClient) ((CloudLegacySolrClient) solrClient).getHttpClient();

Thoughts on introducing a getHttp1Client method in the base test class that would take a SolrClient and could interrogate it and return the httpClient? That would remove a ton of places where we refer to CloudLegacySolrClient.

[dsmiley]

Thoughts on introducing a getHttp1Client method in the base test class that would take a SolrClient and could interrogate it and return the httpClient? That would remove a ton of places where we refer to CloudLegacySolrClient.

I spot checked some tests to see why Apache HttpClient is used.

• In some tests, the test just needs any Http client really. These can be migrated to Java's new built-in Http client.
• In some tests, another SolrClient is built using the same backing Apache HttpClient. I suspect this is merely a convenience. If there was a SolrClient.clone() method, say, then this could work?
• In some tests, the objective is merely to change the Collection being targeted. Maybe a SolrClient.forCollection(String) returning SolrClient? Just an idea.

Despite these ideas, implementing them is out of scope of this issue, I think, even though they would influence each other. They could be tackled first or after.

[epugh]

THanks for sharing those thoughts @dsmiley ..... I'm going to keep plugging away, and try and stay away from refactoring things in this PR!

[sonatype-lift[bot]]

:warning: 311 God Classes were detected by Lift in this project. Visit the Lift web console for more details.

[epugh]

@dsmiley I think I am done! Only had to touch 105 files ;-). I have a list of improvements I'd like to make, but those can be for follow up PR's... Can I get another quick review and then I'll merge?

[dsmiley]

BTW, there are other needlessly specific declarations of SolrClient subclasses as well. I can see this PR focuses on HttpSolrClient. Maybe you'll do others (like Http2SolrClient etc.) in other PRs?

[epugh]

I went ahead and looked at the other specific subclasses of SolrClient and SolrCloudClient... I think at THIS POINT, I am now done with this PR. I am running all the tests again, and assuming they pass and a I get LGTM, then I'll merge. I will start a thread on Dev about other refactoring/cleanup areas, maybe newdev candidates?