dsmiley commented on code in PR #3271:
URL: https://github.com/apache/solr/pull/3271#discussion_r1999323063
##########
solr/core/src/test/org/apache/solr/cloud/DistribJoinFromCollectionTest.java:
##########
@@ -147,7 +147,7 @@ private void testJoins(String toColl, String fromColl,
String toDocId, boolean i
+ fromQ;
QueryRequest qr =
new QueryRequest(params("collection", toColl, "q", joinQ, "fl",
"id,get_s,score"));
- QueryResponse rsp = new QueryResponse(client.request(qr), client);
+ QueryResponse rsp = qr.process(client);
Review Comment:
cool; in this and similar cases, I was just using SolrJ idiomatically
irrespective of this PR's objective. Given a request, if you want it's typed
response, you use the `process` method. That's the *right* way to do it.
##########
solr/solrj/src/java/org/apache/solr/client/solrj/response/QueryResponse.java:
##########
@@ -102,21 +101,10 @@ public class QueryResponse extends SolrResponseBase {
private Map<String, Object> _debugMap = null;
private Map<String, Object> _explainMap = null;
- // utility variable used for automatic binding -- it should not be serialized
- private final transient SolrClient solrClient;
+ public QueryResponse() {}
- public QueryResponse() {
- solrClient = null;
- }
-
- /** Utility constructor to set the solrServer and namedList */
- public QueryResponse(NamedList<Object> res, SolrClient solrClient) {
+ public QueryResponse(NamedList<Object> res) {
Review Comment:
Users shouldn't be using this constructor; it's for SolrJ internals.
##########
solr/CHANGES.txt:
##########
@@ -160,6 +160,9 @@ Other Changes
thus BinaryQueryResponseWriter is gone. Added TextQueryResponseWriter for
the text ones.
Stopped using RawResponseWriter when not needed. (David Smiley)
+* SOLR-17706: SolrJ DocumentObjectBinder is now a global singleton via an
INSTANCE field and that
Review Comment:
I will test it manually before merging
##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/QueryRequest.java:
##########
@@ -62,7 +62,7 @@ public String getPath() {
@Override
protected QueryResponse createResponse(SolrClient client) {
- return new QueryResponse(client);
+ return new QueryResponse();
Review Comment:
No. Only one place in Solr does it use it, aside from this spot. So *not*
using it is in fact the norm. IMO it shouldn't even be on the `createResponse`
API contract in the first place. My other PR created yesterday makes that
change.
##########
solr/solrj/src/java/org/apache/solr/client/solrj/beans/DocumentObjectBinder.java:
##########
@@ -126,18 +136,12 @@ private void addChild(Object obj, DocField field,
SolrInputDocument doc) {
}
private List<DocField> getDocFields(Class<?> clazz) {
Review Comment:
Yeah I "had" to make this change; was begging for it.
--
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]
