dsmiley commented on code in PR #3207:
URL: https://github.com/apache/solr/pull/3207#discussion_r1976523606
##
solr/solrj/src/java/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java:
##
@@ -49,15 +42,6 @@
* @since solr 1.4
*/
public class JavaBinUpdateRequestCodec {
- private boolean readStringAsCharSeq = false;
-
- public JavaBinUpdateRequestCodec setReadStringAsCharSeq(boolean flag) {
Review Comment:
unused; always false
##
solr/solrj/src/java/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java:
##
@@ -277,67 +208,52 @@ public List readIterator(DataInputInputStream
fis) throws IOException {
// special treatment for first outermost Iterator
// (the list of documents)
seenOuterMostDocIterator = true;
- return readOuterMostDocIterator(fis);
+ readDocs(fis);
+ return List.of(); // bogus; already processed
}
-private List readOuterMostDocIterator(DataInputInputStream fis)
throws IOException {
- if (namedList[0] == null) namedList[0] = new NamedList<>();
- NamedList params = (NamedList) namedList[0].get("params");
- if (params == null) params = new NamedList<>();
- updateRequest.setParams(new ModifiableSolrParams(params.toSolrParams()));
- if (handler == null) return super.readIterator(fis);
- Integer commitWithin = null;
- Boolean overwrite = null;
- Object o = null;
- super.readStringAsCharSeq =
JavaBinUpdateRequestCodec.this.readStringAsCharSeq;
- try {
-while (true) {
- if (o == null) {
-o = readVal(fis);
- }
-
- if (o == END_OBJ) {
-break;
- }
+private void readDocs(DataInputInputStream fis) throws IOException {
+ if (resultNamedList == null) resultNamedList = new NamedList<>();
- SolrInputDocument sdoc = null;
- if (o instanceof List) {
-@SuppressWarnings("unchecked")
-List> list = (List>) o;
-sdoc = listToSolrInputDocument(list);
- } else if (o instanceof NamedList) {
-UpdateRequest req = new UpdateRequest();
-req.setParams(new ModifiableSolrParams(((NamedList)
o).toSolrParams()));
-handler.update(null, req, null, null);
- } else if (o instanceof Map.Entry) {
-@SuppressWarnings("unchecked")
-Map.Entry> entry =
-(Map.Entry>) o;
-sdoc = entry.getKey();
-Map p = entry.getValue();
-if (p != null) {
- commitWithin = (Integer) p.get(UpdateRequest.COMMIT_WITHIN);
- overwrite = (Boolean) p.get(UpdateRequest.OVERWRITE);
-}
- } else if (o instanceof SolrInputDocument) {
-sdoc = (SolrInputDocument) o;
- } else if (o instanceof Map) {
-sdoc = convertMapToSolrInputDoc((Map) o);
- }
+ UpdateRequest updateRequest = new UpdateRequest();
+ NamedList params = (NamedList) resultNamedList.get("params"); //
always precedes docs
+ if (params != null) {
+
updateRequest.setParams(ModifiableSolrParams.of(params.toSolrParams()));
+ }
- // peek at the next object to see if we're at the end
- o = readVal(fis);
- if (o == END_OBJ) {
-// indicate that we've hit the last doc in the batch, used to
enable optimizations when
-// doing replication
-updateRequest.lastDocInBatch();
+ Object o = readVal(fis);
+ while (o != END_OBJ) {
+Integer commitWithin = null;
+Boolean overwrite = null;
+
+SolrInputDocument sdoc;
+if (o instanceof Map.Entry) { // doc + options. UpdateRequest
"docsMap"
+ @SuppressWarnings("unchecked")
+ Map.Entry> entry =
+ (Map.Entry>) o;
+ sdoc = entry.getKey();
+ Map p = entry.getValue();
+ if (p != null) {
+commitWithin = (Integer) p.get(UpdateRequest.COMMIT_WITHIN);
+overwrite = (Boolean) p.get(UpdateRequest.OVERWRITE);
}
+} else if (o instanceof SolrInputDocument d) { // doc. UpdateRequest
"docs""
+ sdoc = d;
+} else if (o instanceof Map m) { // doc. To imitate JSON style.
SOLR-13731
+ sdoc = convertMapToSolrInputDoc(m);
+} else {
+ throw new IllegalStateException("Unexpected data type: " +
o.getClass());
Review Comment:
Not being silent about the unexpected! I'll change this to
`SolrException(ErrorCode.BAD_REQUEST`
##
solr/solrj/src/test/org/apache/solr/client/solrj/request/TestUpdateRequestCodec.java:
##
@@ -247,13 +247,14 @@ public void testBackCompat4_5() throws IOException {
InputStream is =
getClass().getResourceAsStream("/solrj/updateReq_4_5.bin");
assertNotNull("updateReq_4_5.bin was not found", is);
+List unmarshalledDocs = new ArrayList<>();
UpdateReq
