This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 1f05e73c203 SOLR-17706: SolrJ DocumentObjectBinder, make a singleton
(#3271)
1f05e73c203 is described below
commit 1f05e73c20379317a25b587a9c541702b86575f8
Author: David Smiley <[email protected]>
AuthorDate: Sat Mar 22 12:14:44 2025 -0400
SOLR-17706: SolrJ DocumentObjectBinder, make a singleton (#3271)
DocumentObjectBinder should be singleton.
Remove from SolrClient, and thus update QueryResponse to not get it from
there.
Use Java SPI/ServiceLoader to allow someone to customize. (Manually tested)
Use ConcurrentHashMap.computeIfAbsent internally.
---
solr/CHANGES.txt | 3 +++
.../solr/cloud/DistribJoinFromCollectionTest.java | 6 ++---
.../apache/solr/cloud/TestRandomFlRTGCloud.java | 2 +-
.../transform/TestSubQueryTransformerDistrib.java | 3 +--
.../solr/search/join/ShardToShardJoinAbstract.java | 6 ++---
.../org/apache/solr/client/solrj/SolrClient.java | 28 +++++-----------------
.../client/solrj/beans/DocumentObjectBinder.java | 26 +++++++++++---------
.../solr/client/solrj/request/QueryRequest.java | 2 +-
.../solr/client/solrj/response/QueryResponse.java | 20 +++-------------
.../solrj/beans/TestDocumentObjectBinder.java | 4 ++--
.../client/solrj/response/QueryResponseTest.java | 10 ++++----
.../solrj/response/TestClusteringResponse.java | 2 +-
12 files changed, 44 insertions(+), 68 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 812ab8243a7..48883e6c7fd 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -161,6 +161,9 @@ Other Changes
Stopped using RawResponseWriter when not needed. Rename
BinaryResponseWriter to JavaBinResponseWriter.
(David Smiley)
+* SOLR-17706: SolrJ DocumentObjectBinder is now a global singleton via an
INSTANCE field and that
+ which is pluggable via Java SPI/ServiceLoader. SolrClient.getBinder is
gone. (David Smiley)
+
================== 9.9.0 ==================
New Features
---------------------
diff --git
a/solr/core/src/test/org/apache/solr/cloud/DistribJoinFromCollectionTest.java
b/solr/core/src/test/org/apache/solr/cloud/DistribJoinFromCollectionTest.java
index e22770d4222..0d2d7bef066 100644
---
a/solr/core/src/test/org/apache/solr/cloud/DistribJoinFromCollectionTest.java
+++
b/solr/core/src/test/org/apache/solr/cloud/DistribJoinFromCollectionTest.java
@@ -147,7 +147,7 @@ public class DistribJoinFromCollectionTest extends
SolrCloudTestCase {
+ 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);
SolrDocumentList hits = rsp.getResults();
assertEquals("Expected 1 doc, got " + hits, 1, hits.getNumFound());
SolrDocument doc = hits.get(0);
@@ -173,7 +173,7 @@ public class DistribJoinFromCollectionTest extends
SolrCloudTestCase {
+ fromQ;
final QueryRequest qr =
new QueryRequest(params("collection", toColl, "q", joinQ, "fl",
"id,get_s,score"));
- final QueryResponse rsp = new QueryResponse(client.request(qr), client);
+ final QueryResponse rsp = qr.process(client);
final SolrDocumentList hits = rsp.getResults();
assertEquals("Expected 1 doc", 1, hits.getNumFound());
SolrDocument doc = hits.get(0);
@@ -195,7 +195,7 @@ public class DistribJoinFromCollectionTest extends
SolrCloudTestCase {
+ " to=join_s}match_s:d";
final QueryRequest qr =
new QueryRequest(params("collection", toColl, "q", joinQ, "fl",
"id,get_s,score"));
- final QueryResponse rsp = new QueryResponse(client.request(qr), client);
+ final QueryResponse rsp = qr.process(client);
final SolrDocumentList hits = rsp.getResults();
assertEquals("Expected no hits", 0, hits.getNumFound());
}
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java
b/solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java
index ecaeae8b64f..9def712aeab 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java
@@ -624,7 +624,7 @@ public class TestRandomFlRTGCloud extends SolrCloudTestCase
{
return getDocsFromRTGResponse(
expectList,
new QueryResponse(
- new RawCapableXMLResponseParser().processResponse(new
StringReader(rsp)), null));
+ new RawCapableXMLResponseParser().processResponse(new
StringReader(rsp))));
}
/**
diff --git
a/solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformerDistrib.java
b/solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformerDistrib.java
index 5d626446e58..e4cb7ae836f 100644
---
a/solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformerDistrib.java
+++
b/solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformerDistrib.java
@@ -182,8 +182,7 @@ public class TestSubQueryTransformerDistrib extends
SolrCloudTestCase {
final SolrDocumentList hits;
{
final QueryRequest qr = withBasicAuth(new QueryRequest(params));
- final QueryResponse rsp = new QueryResponse();
- rsp.setResponse(cluster.getSolrClient().request(qr, people + "," +
depts));
+ final QueryResponse rsp = qr.process(cluster.getSolrClient(), people +
"," + depts);
hits = rsp.getResults();
assertEquals(peopleMultiplier, hits.getNumFound());
diff --git
a/solr/core/src/test/org/apache/solr/search/join/ShardToShardJoinAbstract.java
b/solr/core/src/test/org/apache/solr/search/join/ShardToShardJoinAbstract.java
index ece5e5338a6..b2b3f4be01e 100644
---
a/solr/core/src/test/org/apache/solr/search/join/ShardToShardJoinAbstract.java
+++
b/solr/core/src/test/org/apache/solr/search/join/ShardToShardJoinAbstract.java
@@ -194,7 +194,7 @@ public class ShardToShardJoinAbstract extends
SolrCloudTestCase {
+ " to=id}"
+ fromQ;
QueryRequest qr = new QueryRequest(params("collection", toColl,
"q", joinQ, "fl", "*"));
- QueryResponse rsp = new QueryResponse(client.request(qr), client);
+ QueryResponse rsp = qr.process(client);
SolrDocumentList hits = rsp.getResults();
assertEquals("Expected 1 doc, got " + hits, 1, hits.getNumFound());
SolrDocument doc = hits.get(0);
@@ -216,7 +216,7 @@ public class ShardToShardJoinAbstract extends
SolrCloudTestCase {
QueryRequest qr =
new QueryRequest(
params("collection", fromColl, "q", joinQ, "fl", "*",
"rows", "20"));
- QueryResponse rsp = new QueryResponse(client.request(qr), client);
+ QueryResponse rsp = new QueryResponse(client.request(qr));
SolrDocumentList hits = rsp.getResults();
assertEquals("Expected 11 doc, got " + hits, 10 + 1,
hits.getNumFound());
for (SolrDocument doc : hits) {
@@ -246,7 +246,7 @@ public class ShardToShardJoinAbstract extends
SolrCloudTestCase {
+ " to=id}"
+ fromQ;
QueryRequest qr = new QueryRequest(params("collection", toColl, "q",
joinQ, "fl", "*"));
- QueryResponse rsp = new QueryResponse(client.request(qr), client);
+ QueryResponse rsp = qr.process(client);
SolrDocumentList hits = rsp.getResults();
final Set<String> expect =
new HashSet<>(
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java
index 240f9750ed3..acdf6b013b3 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java
@@ -52,7 +52,6 @@ public abstract class SolrClient implements Serializable,
Closeable {
private static final long serialVersionUID = 1L;
- private DocumentObjectBinder binder;
protected String defaultCollection;
/**
@@ -261,7 +260,7 @@ public abstract class SolrClient implements Serializable,
Closeable {
*/
public UpdateResponse addBean(String collection, Object obj, int
commitWithinMs)
throws IOException, SolrServerException {
- return add(collection, getBinder().toSolrInputDocument(obj),
commitWithinMs);
+ return add(collection,
DocumentObjectBinder.INSTANCE.toSolrInputDocument(obj), commitWithinMs);
}
/**
@@ -277,7 +276,7 @@ public abstract class SolrClient implements Serializable,
Closeable {
*/
public UpdateResponse addBean(Object obj, int commitWithinMs)
throws IOException, SolrServerException {
- return add(null, getBinder().toSolrInputDocument(obj), commitWithinMs);
+ return add(null, DocumentObjectBinder.INSTANCE.toSolrInputDocument(obj),
commitWithinMs);
}
/**
@@ -324,15 +323,14 @@ public abstract class SolrClient implements Serializable,
Closeable {
* @return an {@link org.apache.solr.client.solrj.response.UpdateResponse}
from the server
* @throws IOException if there is a communication error with the server
* @throws SolrServerException if there is an error on the server
- * @see SolrClient#getBinder()
+ * @see DocumentObjectBinder
* @since solr 5.1
*/
public UpdateResponse addBeans(String collection, Collection<?> beans, int
commitWithinMs)
throws SolrServerException, IOException {
- DocumentObjectBinder binder = this.getBinder();
ArrayList<SolrInputDocument> docs = new ArrayList<>(beans.size());
for (Object bean : beans) {
- docs.add(binder.toSolrInputDocument(bean));
+ docs.add(DocumentObjectBinder.INSTANCE.toSolrInputDocument(bean));
}
return add(collection, docs, commitWithinMs);
}
@@ -348,7 +346,7 @@ public abstract class SolrClient implements Serializable,
Closeable {
* @return an {@link org.apache.solr.client.solrj.response.UpdateResponse}
from the server
* @throws IOException if there is a communication error with the server
* @throws SolrServerException if there is an error on the server
- * @see SolrClient#getBinder()
+ * @see DocumentObjectBinder
* @since solr 3.5
*/
public UpdateResponse addBeans(Collection<?> beans, int commitWithinMs)
@@ -380,7 +378,7 @@ public abstract class SolrClient implements Serializable,
Closeable {
public SolrInputDocument next() {
Object o = beanIterator.next();
if (o == null) return null;
- return getBinder().toSolrInputDocument(o);
+ return DocumentObjectBinder.INSTANCE.toSolrInputDocument(o);
}
@Override
@@ -1195,20 +1193,6 @@ public abstract class SolrClient implements
Serializable, Closeable {
return request(request, null);
}
- /**
- * Get the {@link org.apache.solr.client.solrj.beans.DocumentObjectBinder}
for this client.
- *
- * @return a DocumentObjectBinder
- * @see SolrClient#addBean
- * @see SolrClient#addBeans
- */
- public DocumentObjectBinder getBinder() {
- if (binder == null) {
- binder = new DocumentObjectBinder();
- }
- return binder;
- }
-
/**
* This method defines the context in which this Solr client is being used
(e.g. for internal
* communication between Solr nodes or as an external client). The default
value is {@code
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/beans/DocumentObjectBinder.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/beans/DocumentObjectBinder.java
index 7cd139ef21e..fabcfee587f 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/beans/DocumentObjectBinder.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/beans/DocumentObjectBinder.java
@@ -33,6 +33,7 @@ import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.ServiceLoader;
import java.util.concurrent.ConcurrentHashMap;
import java.util.regex.Pattern;
import org.apache.solr.common.SolrDocument;
@@ -41,13 +42,22 @@ import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.util.SuppressForbidden;
/**
- * A class to map objects to and from solr documents.
+ * Maps objects to and from Solr documents.
*
* @since solr 1.3
*/
public class DocumentObjectBinder {
- private final Map<Class<?>, List<DocField>> infocache = new
ConcurrentHashMap<>();
+ /**
+ * The singleton/default instance. The implementation is loaded via {@link
ServiceLoader}, so it's
+ * customizable.
+ */
+ public static final DocumentObjectBinder INSTANCE =
+ ServiceLoader.load(DocumentObjectBinder.class)
+ .findFirst()
+ .orElseGet(DocumentObjectBinder::new);
+
+ private final Map<Class<?>, List<DocField>> infoCache = new
ConcurrentHashMap<>();
public DocumentObjectBinder() {}
@@ -126,18 +136,12 @@ public class DocumentObjectBinder {
}
private List<DocField> getDocFields(Class<?> clazz) {
- List<DocField> fields = infocache.get(clazz);
- if (fields == null) {
- synchronized (infocache) {
- infocache.put(clazz, fields = collectInfo(clazz));
- }
- }
- return fields;
+ return infoCache.computeIfAbsent(clazz, this::collectInfo);
}
@SuppressWarnings("removal")
@SuppressForbidden(reason = "Needs access to possibly private @Field
annotated fields/methods")
- private List<DocField> collectInfo(Class<?> clazz) {
+ protected List<DocField> collectInfo(Class<?> clazz) {
List<DocField> fields = new ArrayList<>();
Class<?> superClazz = clazz;
List<AccessibleObject> members = new ArrayList<>();
@@ -169,7 +173,7 @@ public class DocumentObjectBinder {
return fields;
}
- private class DocField {
+ protected class DocField {
private Field annotation;
private String name;
private java.lang.reflect.Field field;
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/request/QueryRequest.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/request/QueryRequest.java
index c8652fa3887..2c26b2ac81d 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/QueryRequest.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/QueryRequest.java
@@ -62,7 +62,7 @@ public class QueryRequest extends
CollectionRequiringSolrRequest<QueryResponse>
@Override
protected QueryResponse createResponse(SolrClient client) {
- return new QueryResponse(client);
+ return new QueryResponse();
}
@Override
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/response/QueryResponse.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/response/QueryResponse.java
index e09a073f4d4..11cea6f4587 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/response/QueryResponse.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/response/QueryResponse.java
@@ -23,7 +23,6 @@ import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
-import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.beans.DocumentObjectBinder;
import org.apache.solr.client.solrj.response.json.NestableJsonFacet;
import org.apache.solr.common.SolrDocumentList;
@@ -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) {
this.setResponse(res);
- this.solrClient = solrClient;
- }
-
- public QueryResponse(SolrClient solrClient) {
- this.solrClient = solrClient;
}
@Override
@@ -643,9 +631,7 @@ public class QueryResponse extends SolrResponseBase {
}
public <T> List<T> getBeans(Class<T> type) {
- return solrClient == null
- ? new DocumentObjectBinder().getBeans(type, _results)
- : solrClient.getBinder().getBeans(type, _results);
+ return DocumentObjectBinder.INSTANCE.getBeans(type, _results);
}
public Map<String, FieldStatsInfo> getFieldStatsInfo() {
diff --git
a/solr/solrj/src/test/org/apache/solr/client/solrj/beans/TestDocumentObjectBinder.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/beans/TestDocumentObjectBinder.java
index 2740ead1d93..0c746012825 100644
---
a/solr/solrj/src/test/org/apache/solr/client/solrj/beans/TestDocumentObjectBinder.java
+++
b/solr/solrj/src/test/org/apache/solr/client/solrj/beans/TestDocumentObjectBinder.java
@@ -38,7 +38,7 @@ public class TestDocumentObjectBinder extends SolrTestCase {
DocumentObjectBinder binder = new DocumentObjectBinder();
XMLResponseParser parser = new XMLResponseParser();
NamedList<Object> nl = parser.processResponse(new StringReader(xml));
- QueryResponse res = new QueryResponse(nl, null);
+ QueryResponse res = new QueryResponse(nl);
SolrDocumentList solDocList = res.getResults();
List<Item> l = binder.getBeans(Item.class, res.getResults());
@@ -83,7 +83,7 @@ public class TestDocumentObjectBinder extends SolrTestCase {
DocumentObjectBinder binder = new DocumentObjectBinder();
XMLResponseParser parser = new XMLResponseParser();
NamedList<Object> nl = parser.processResponse(new StringReader(xml));
- QueryResponse res = new QueryResponse(nl, null);
+ QueryResponse res = new QueryResponse(nl);
List<Item> l = binder.getBeans(Item.class, res.getResults());
assertArrayEquals(
diff --git
a/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseTest.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseTest.java
index a85a3cccebb..40602b73ea6 100644
---
a/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseTest.java
+++
b/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseTest.java
@@ -55,7 +55,7 @@ public class QueryResponseTest extends SolrTestCase {
}
}
- QueryResponse qr = new QueryResponse(response, null);
+ QueryResponse qr = new QueryResponse(response);
assertNotNull(qr);
int counter = 0;
@@ -119,7 +119,7 @@ public class QueryResponseTest extends SolrTestCase {
}
}
- QueryResponse qr = new QueryResponse(response, null);
+ QueryResponse qr = new QueryResponse(response);
assertNotNull(qr);
GroupResponse groupResponse = qr.getGroupResponse();
assertNotNull(groupResponse);
@@ -225,7 +225,7 @@ public class QueryResponseTest extends SolrTestCase {
}
}
- QueryResponse qr = new QueryResponse(response, null);
+ QueryResponse qr = new QueryResponse(response);
assertNotNull(qr);
GroupResponse groupResponse = qr.getGroupResponse();
assertNotNull(groupResponse);
@@ -267,7 +267,7 @@ public class QueryResponseTest extends SolrTestCase {
NamedList<Object> response = parser.processResponse(in);
in.close();
- QueryResponse qr = new QueryResponse(response, null);
+ QueryResponse qr = new QueryResponse(response);
assertNotNull(qr);
assertNotNull(qr.getIntervalFacets());
assertEquals(2, qr.getIntervalFacets().size());
@@ -311,7 +311,7 @@ public class QueryResponseTest extends SolrTestCase {
}
}
- QueryResponse qr = new QueryResponse(response, null);
+ QueryResponse qr = new QueryResponse(response);
assertNotNull(qr);
Map<String, Object> explainMap = qr.getExplainMap();
diff --git
a/solr/solrj/src/test/org/apache/solr/client/solrj/response/TestClusteringResponse.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/response/TestClusteringResponse.java
index 8e1a2631e59..caed38d12f1 100644
---
a/solr/solrj/src/test/org/apache/solr/client/solrj/response/TestClusteringResponse.java
+++
b/solr/solrj/src/test/org/apache/solr/client/solrj/response/TestClusteringResponse.java
@@ -45,7 +45,7 @@ public class TestClusteringResponse extends SolrJettyTestBase
{
response = parser.processResponse(in);
}
}
- QueryResponse qr = new QueryResponse(response, null);
+ QueryResponse qr = new QueryResponse(response);
ClusteringResponse clusteringResponse = qr.getClusteringResponse();
List<Cluster> clusters = clusteringResponse.getClusters();
assertEquals(4, clusters.size());
