Re: [PR] SOLR-17518: Deprecate UpdateRequest.getXml() and replace it with XMLRequestWriter [solr]

2025-02-25 Thread via GitHub


psalagnac merged PR #3200:
URL: https://github.com/apache/solr/pull/3200


-- 
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]



Re: [PR] SOLR-17518: Deprecate UpdateRequest.getXml() and replace it with XMLRequestWriter [solr]

2025-02-24 Thread via GitHub


psalagnac commented on PR #3200:
URL: https://github.com/apache/solr/pull/3200#issuecomment-2679343190

   > I assume you moved some things around but didn't really write code here 
(i.e. all code, variable names was the choice of original authors)?
   
   Yes, just moved things around.
   


-- 
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]



Re: [PR] SOLR-17518: Deprecate UpdateRequest.getXml() and replace it with XMLRequestWriter [solr]

2025-02-24 Thread via GitHub


psalagnac commented on code in PR #3200:
URL: https://github.com/apache/solr/pull/3200#discussion_r1968216711


##
solr/solrj/src/java/org/apache/solr/client/solrj/impl/XMLRequestWriter.java:
##
@@ -0,0 +1,216 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.impl;
+
+import java.io.BufferedWriter;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.request.RequestWriter;
+import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.client.solrj.util.ClientUtils;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.util.ContentStream;
+import org.apache.solr.common.util.XML;
+
+public class XMLRequestWriter extends RequestWriter {
+
+  /**
+   * Use this to do a push writing instead of pull. If this method returns 
null {@link

Review Comment:
   That's not new code, not sure on history. This javadoc is also in base class 
`ContentWriter`.
   I agree it does not make sense... I'll update it with this PR.



-- 
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]



Re: [PR] SOLR-17518: Deprecate UpdateRequest.getXml() and replace it with XMLRequestWriter [solr]

2025-02-24 Thread via GitHub


epugh commented on code in PR #3200:
URL: https://github.com/apache/solr/pull/3200#discussion_r1968218397


##
solr/solrj/src/java/org/apache/solr/client/solrj/impl/XMLRequestWriter.java:
##
@@ -0,0 +1,216 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.impl;
+
+import java.io.BufferedWriter;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.request.RequestWriter;
+import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.client.solrj.util.ClientUtils;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.util.ContentStream;
+import org.apache.solr.common.util.XML;
+
+public class XMLRequestWriter extends RequestWriter {
+
+  /**
+   * Use this to do a push writing instead of pull. If this method returns 
null {@link

Review Comment:
   awesome!   



-- 
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]



Re: [PR] SOLR-17518: Deprecate UpdateRequest.getXml() and replace it with XMLRequestWriter [solr]

2025-02-24 Thread via GitHub


psalagnac commented on code in PR #3200:
URL: https://github.com/apache/solr/pull/3200#discussion_r1968216711


##
solr/solrj/src/java/org/apache/solr/client/solrj/impl/XMLRequestWriter.java:
##
@@ -0,0 +1,216 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.impl;
+
+import java.io.BufferedWriter;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.request.RequestWriter;
+import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.client.solrj.util.ClientUtils;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.util.ContentStream;
+import org.apache.solr.common.util.XML;
+
+public class XMLRequestWriter extends RequestWriter {
+
+  /**
+   * Use this to do a push writing instead of pull. If this method returns 
null {@link

Review Comment:
   That's not now code, not sure on history. This javadoc is also in base class 
`ContentWriter`.
   I agree it does not make sense... I'll update it with this PR.



-- 
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]



Re: [PR] SOLR-17518: Deprecate UpdateRequest.getXml() and replace it with XMLRequestWriter [solr]

2025-02-24 Thread via GitHub


psalagnac commented on code in PR #3200:
URL: https://github.com/apache/solr/pull/3200#discussion_r1968216711


##
solr/solrj/src/java/org/apache/solr/client/solrj/impl/XMLRequestWriter.java:
##
@@ -0,0 +1,216 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.impl;
+
+import java.io.BufferedWriter;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.request.RequestWriter;
+import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.client.solrj.util.ClientUtils;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.util.ContentStream;
+import org.apache.solr.common.util.XML;
+
+public class XMLRequestWriter extends RequestWriter {
+
+  /**
+   * Use this to do a push writing instead of pull. If this method returns 
null {@link

Review Comment:
   That's not new code, not sure on history. This javadoc is also in base class 
`RequestWriter`.
   I agree it does not make sense... I'll update it with this PR.



-- 
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]



Re: [PR] SOLR-17518: Deprecate UpdateRequest.getXml() and replace it with XMLRequestWriter [solr]

2025-02-24 Thread via GitHub


psalagnac commented on code in PR #3200:
URL: https://github.com/apache/solr/pull/3200#discussion_r1968195273


##
solr/solrj/src/java/org/apache/solr/client/solrj/request/UpdateRequest.java:
##
@@ -368,147 +368,39 @@ public void setDeleteQuery(List deleteQuery) {
   // --
   // --
 
+  /**
+   * @deprecated Method will be removed in Solr 10.0. Use {@link 
XMLRequestWriter} instead.
+   */
+  @Deprecated(since = "9.9")
   @Override
   public Collection getContentStreams() throws IOException {
 return ClientUtils.toContentStreams(getXML(), ClientUtils.TEXT_XML);
   }
 
+  /**
+   * @deprecated Method will be removed in Solr 10.0. Use {@link 
XMLRequestWriter} instead.
+   */
+  @Deprecated(since = "9.9")
   public String getXML() throws IOException {
 StringWriter writer = new StringWriter();
 writeXML(writer);
 writer.flush();
 
 // If action is COMMIT or OPTIMIZE, it is sent with params
 String xml = writer.toString();
-// System.out.println( "SEND:"+xml );
 return (xml.length() > 0) ? xml : null;
   }
 
-  private List>> getDocLists(
-  Map> documents) {
-List>> docLists = new 
ArrayList<>();
-Map> docList = null;
-if (this.documents != null) {
-
-  Boolean lastOverwrite = true;
-  Integer lastCommitWithin = -1;
-
-  Set>> entries = 
this.documents.entrySet();
-  for (Entry> entry : entries) {
-Map map = entry.getValue();
-Boolean overwrite = null;
-Integer commitWithin = null;
-if (map != null) {
-  overwrite = (Boolean) entry.getValue().get(OVERWRITE);
-  commitWithin = (Integer) entry.getValue().get(COMMIT_WITHIN);
-}
-if (!Objects.equals(overwrite, lastOverwrite)
-|| !Objects.equals(commitWithin, lastCommitWithin)
-|| docLists.isEmpty()) {
-  docList = new LinkedHashMap<>();
-  docLists.add(docList);
-}
-docList.put(entry.getKey(), entry.getValue());
-lastCommitWithin = commitWithin;
-lastOverwrite = overwrite;
-  }
-}
-
-if (docIterator != null) {
-  docList = new LinkedHashMap<>();
-  docLists.add(docList);
-  while (docIterator.hasNext()) {
-SolrInputDocument doc = docIterator.next();
-if (doc != null) {
-  docList.put(doc, null);
-}
-  }
-}
-
-return docLists;
-  }
-
   /**
-   * @since solr 1.4
+   * @deprecated Method will be removed in Solr 10.0. Use {@link 
XMLRequestWriter} instead.
*/
+  @Deprecated(since = "9.9")
   public UpdateRequest writeXML(Writer writer) throws IOException {
-List>> getDocLists = 
getDocLists(documents);
-
-for (Map> docs : getDocLists) {
-
-  if ((docs != null && docs.size() > 0)) {
-Entry> firstDoc = 
docs.entrySet().iterator().next();
-Map map = firstDoc.getValue();
-Integer cw = null;
-Boolean ow = null;
-if (map != null) {
-  cw = (Integer) firstDoc.getValue().get(COMMIT_WITHIN);
-  ow = (Boolean) firstDoc.getValue().get(OVERWRITE);
-}
-if (ow == null) ow = true;
-int commitWithin = (cw != null && cw != -1) ? cw : this.commitWithin;
-boolean overwrite = ow;
-if (commitWithin > -1 || overwrite != true) {
-  writer.write(
-  "");
-} else {
-  writer.write("");
-}
-
-Set>> entries = 
docs.entrySet();
-for (Entry> entry : entries) {
-  ClientUtils.writeXML(entry.getKey(), writer);
-}
-
-writer.write("");
-  }
-}
-
-// Add the delete commands
-boolean deleteI = deleteById != null && deleteById.size() > 0;
-boolean deleteQ = deleteQuery != null && deleteQuery.size() > 0;
-if (deleteI || deleteQ) {
-  if (commitWithin > 0) {
-writer.append("");
-  } else {
-writer.append("");
-  }
-  if (deleteI) {
-for (Map.Entry> entry : 
deleteById.entrySet()) {
-  writer.append(" map = entry.getValue();
-  if (map != null) {
-Long version = (Long) map.get(VER);
-String route = (String) map.get(_ROUTE_);
-if (version != null) {
-  writer.append(" 
version=\"").append(String.valueOf(version)).append('"');
-}
-
-if (route != null) {
-  writer.append(" _route_=\"").append(route).append('"');
-}
-  }
-  writer.append(">");
-
-  XML.escapeCharData(entry.getKey(), writer);
-  writer.append("");
-}
-  }
-  if (deleteQ) {
-for (String q : deleteQuery) {
-  writer.append("");
-  XML.escapeCharData(q, writer);
-  writer.append("");
-}
-  }
-  writer.append("");
-}
+XMLRequestWriter requestWriter = new XM

Re: [PR] SOLR-17518: Deprecate UpdateRequest.getXml() and replace it with XMLRequestWriter [solr]

2025-02-22 Thread via GitHub


dsmiley commented on code in PR #3200:
URL: https://github.com/apache/solr/pull/3200#discussion_r1966671050


##
solr/solrj/src/java/org/apache/solr/client/solrj/impl/XMLRequestWriter.java:
##
@@ -0,0 +1,216 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.impl;
+
+import java.io.BufferedWriter;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.request.RequestWriter;
+import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.client.solrj.util.ClientUtils;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.util.ContentStream;
+import org.apache.solr.common.util.XML;
+
+public class XMLRequestWriter extends RequestWriter {
+
+  /**
+   * Use this to do a push writing instead of pull. If this method returns 
null {@link
+   * 
org.apache.solr.client.solrj.request.RequestWriter#getContentStreams(SolrRequest)}
 is invoked
+   * to do a pull write.
+   */
+  @Override
+  public RequestWriter.ContentWriter getContentWriter(SolrRequest req) {
+if (req instanceof UpdateRequest updateRequest) {
+  if (isEmpty(updateRequest)) return null;
+  return new RequestWriter.ContentWriter() {
+@Override
+public void write(OutputStream os) throws IOException {
+  OutputStreamWriter writer = new OutputStreamWriter(os, 
StandardCharsets.UTF_8);
+  writeXML(updateRequest, writer);
+  writer.flush();
+}
+
+@Override
+public String getContentType() {
+  return ClientUtils.TEXT_XML;
+}
+  };
+}
+return req.getContentWriter(ClientUtils.TEXT_XML);
+  }
+
+  @Override
+  public Collection getContentStreams(SolrRequest req) 
throws IOException {
+if (req instanceof UpdateRequest) {
+  return null;
+}
+return req.getContentStreams();
+  }
+
+  @Override
+  public void write(SolrRequest request, OutputStream os) throws 
IOException {
+if (request instanceof UpdateRequest updateRequest) {
+  BufferedWriter writer =
+  new BufferedWriter(new OutputStreamWriter(os, 
StandardCharsets.UTF_8));
+  writeXML(updateRequest, writer);
+  writer.flush();
+}
+  }
+
+  @Override
+  public String getUpdateContentType() {
+return ClientUtils.TEXT_XML;
+  }
+
+  public void writeXML(UpdateRequest request, Writer writer) throws 
IOException {
+List>> getDocLists = 
getDocLists(request);
+
+for (Map> docs : getDocLists) {
+
+  if (docs != null && !docs.isEmpty()) {
+Map.Entry> firstDoc =
+docs.entrySet().iterator().next();
+Map map = firstDoc.getValue();
+Integer cw = null;

Review Comment:
   he didn't write this code



-- 
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]



Re: [PR] SOLR-17518: Deprecate UpdateRequest.getXml() and replace it with XMLRequestWriter [solr]

2025-02-22 Thread via GitHub


dsmiley commented on code in PR #3200:
URL: https://github.com/apache/solr/pull/3200#discussion_r198902


##
solr/solrj/src/java/org/apache/solr/client/solrj/request/UpdateRequest.java:
##
@@ -368,147 +368,39 @@ public void setDeleteQuery(List deleteQuery) {
   // --
   // --
 
+  /**
+   * @deprecated Method will be removed in Solr 10.0. Use {@link 
XMLRequestWriter} instead.
+   */
+  @Deprecated(since = "9.9")
   @Override
   public Collection getContentStreams() throws IOException {
 return ClientUtils.toContentStreams(getXML(), ClientUtils.TEXT_XML);
   }
 
+  /**
+   * @deprecated Method will be removed in Solr 10.0. Use {@link 
XMLRequestWriter} instead.
+   */
+  @Deprecated(since = "9.9")
   public String getXML() throws IOException {
 StringWriter writer = new StringWriter();
 writeXML(writer);
 writer.flush();
 
 // If action is COMMIT or OPTIMIZE, it is sent with params
 String xml = writer.toString();
-// System.out.println( "SEND:"+xml );
 return (xml.length() > 0) ? xml : null;
   }
 
-  private List>> getDocLists(
-  Map> documents) {
-List>> docLists = new 
ArrayList<>();
-Map> docList = null;
-if (this.documents != null) {
-
-  Boolean lastOverwrite = true;
-  Integer lastCommitWithin = -1;
-
-  Set>> entries = 
this.documents.entrySet();
-  for (Entry> entry : entries) {
-Map map = entry.getValue();
-Boolean overwrite = null;
-Integer commitWithin = null;
-if (map != null) {
-  overwrite = (Boolean) entry.getValue().get(OVERWRITE);
-  commitWithin = (Integer) entry.getValue().get(COMMIT_WITHIN);
-}
-if (!Objects.equals(overwrite, lastOverwrite)
-|| !Objects.equals(commitWithin, lastCommitWithin)
-|| docLists.isEmpty()) {
-  docList = new LinkedHashMap<>();
-  docLists.add(docList);
-}
-docList.put(entry.getKey(), entry.getValue());
-lastCommitWithin = commitWithin;
-lastOverwrite = overwrite;
-  }
-}
-
-if (docIterator != null) {
-  docList = new LinkedHashMap<>();
-  docLists.add(docList);
-  while (docIterator.hasNext()) {
-SolrInputDocument doc = docIterator.next();
-if (doc != null) {
-  docList.put(doc, null);
-}
-  }
-}
-
-return docLists;
-  }
-
   /**
-   * @since solr 1.4
+   * @deprecated Method will be removed in Solr 10.0. Use {@link 
XMLRequestWriter} instead.
*/
+  @Deprecated(since = "9.9")
   public UpdateRequest writeXML(Writer writer) throws IOException {
-List>> getDocLists = 
getDocLists(documents);
-
-for (Map> docs : getDocLists) {
-
-  if ((docs != null && docs.size() > 0)) {
-Entry> firstDoc = 
docs.entrySet().iterator().next();
-Map map = firstDoc.getValue();
-Integer cw = null;
-Boolean ow = null;
-if (map != null) {
-  cw = (Integer) firstDoc.getValue().get(COMMIT_WITHIN);
-  ow = (Boolean) firstDoc.getValue().get(OVERWRITE);
-}
-if (ow == null) ow = true;
-int commitWithin = (cw != null && cw != -1) ? cw : this.commitWithin;
-boolean overwrite = ow;
-if (commitWithin > -1 || overwrite != true) {
-  writer.write(
-  "");
-} else {
-  writer.write("");
-}
-
-Set>> entries = 
docs.entrySet();
-for (Entry> entry : entries) {
-  ClientUtils.writeXML(entry.getKey(), writer);
-}
-
-writer.write("");
-  }
-}
-
-// Add the delete commands
-boolean deleteI = deleteById != null && deleteById.size() > 0;
-boolean deleteQ = deleteQuery != null && deleteQuery.size() > 0;
-if (deleteI || deleteQ) {
-  if (commitWithin > 0) {
-writer.append("");
-  } else {
-writer.append("");
-  }
-  if (deleteI) {
-for (Map.Entry> entry : 
deleteById.entrySet()) {
-  writer.append(" map = entry.getValue();
-  if (map != null) {
-Long version = (Long) map.get(VER);
-String route = (String) map.get(_ROUTE_);
-if (version != null) {
-  writer.append(" 
version=\"").append(String.valueOf(version)).append('"');
-}
-
-if (route != null) {
-  writer.append(" _route_=\"").append(route).append('"');
-}
-  }
-  writer.append(">");
-
-  XML.escapeCharData(entry.getKey(), writer);
-  writer.append("");
-}
-  }
-  if (deleteQ) {
-for (String q : deleteQuery) {
-  writer.append("");
-  XML.escapeCharData(q, writer);
-  writer.append("");
-}
-  }
-  writer.append("");
-}
+XMLRequestWriter requestWriter = new XMLR

Re: [PR] SOLR-17518: Deprecate UpdateRequest.getXml() and replace it with XMLRequestWriter [solr]

2025-02-20 Thread via GitHub


epugh commented on code in PR #3200:
URL: https://github.com/apache/solr/pull/3200#discussion_r1964253721


##
solr/solrj/src/java/org/apache/solr/client/solrj/impl/XMLRequestWriter.java:
##
@@ -0,0 +1,216 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.impl;
+
+import java.io.BufferedWriter;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.request.RequestWriter;
+import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.client.solrj.util.ClientUtils;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.util.ContentStream;
+import org.apache.solr.common.util.XML;
+
+public class XMLRequestWriter extends RequestWriter {
+
+  /**
+   * Use this to do a push writing instead of pull. If this method returns 
null {@link
+   * 
org.apache.solr.client.solrj.request.RequestWriter#getContentStreams(SolrRequest)}
 is invoked
+   * to do a pull write.
+   */
+  @Override
+  public RequestWriter.ContentWriter getContentWriter(SolrRequest req) {
+if (req instanceof UpdateRequest updateRequest) {
+  if (isEmpty(updateRequest)) return null;
+  return new RequestWriter.ContentWriter() {
+@Override
+public void write(OutputStream os) throws IOException {
+  OutputStreamWriter writer = new OutputStreamWriter(os, 
StandardCharsets.UTF_8);
+  writeXML(updateRequest, writer);
+  writer.flush();
+}
+
+@Override
+public String getContentType() {
+  return ClientUtils.TEXT_XML;
+}
+  };
+}
+return req.getContentWriter(ClientUtils.TEXT_XML);
+  }
+
+  @Override
+  public Collection getContentStreams(SolrRequest req) 
throws IOException {
+if (req instanceof UpdateRequest) {
+  return null;
+}
+return req.getContentStreams();
+  }
+
+  @Override
+  public void write(SolrRequest request, OutputStream os) throws 
IOException {
+if (request instanceof UpdateRequest updateRequest) {
+  BufferedWriter writer =
+  new BufferedWriter(new OutputStreamWriter(os, 
StandardCharsets.UTF_8));
+  writeXML(updateRequest, writer);
+  writer.flush();
+}
+  }
+
+  @Override
+  public String getUpdateContentType() {
+return ClientUtils.TEXT_XML;
+  }
+
+  public void writeXML(UpdateRequest request, Writer writer) throws 
IOException {
+List>> getDocLists = 
getDocLists(request);
+
+for (Map> docs : getDocLists) {
+
+  if (docs != null && !docs.isEmpty()) {
+Map.Entry> firstDoc =
+docs.entrySet().iterator().next();
+Map map = firstDoc.getValue();
+Integer cw = null;

Review Comment:
   seems odd to have both `cw` and later `commitWithin`, and since we don't 
normally abbriveate variables, do `commitWithin` and `overwrite` everywhere?



##
solr/solrj/src/java/org/apache/solr/client/solrj/request/UpdateRequest.java:
##
@@ -368,147 +368,39 @@ public void setDeleteQuery(List deleteQuery) {
   // --
   // --
 
+  /**
+   * @deprecated Method will be removed in Solr 10.0. Use {@link 
XMLRequestWriter} instead.
+   */
+  @Deprecated(since = "9.9")

Review Comment:
   thanks for the `since`, I like seeing those to help remind us when we can 
clean them up.   Plus, you added a reminder in the docs!



##
solr/solrj/src/java/org/apache/solr/client/solrj/impl/XMLRequestWriter.java:
##
@@ -0,0 +1,216 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for ad