SOLR-17518: Deprecate UpdateRequest.getXml() and replace it with XMLRequestWriter #3200
Conversation
There was a problem hiding this comment.
Sharper eyes than I need to weigh in, but I like how it all looks. I love that we are slowly putting XML on the same playing field as JSON and other formats, and getting away from the "special treatment". I tried to add Avro as a request writer once and totally got lost in the path of how writers worked, and that xml was different than other formats!
| public class XMLRequestWriter extends RequestWriter { | ||
|
|
||
| /** | ||
| * Use this to do a push writing instead of pull. If this method returns null {@link |
There was a problem hiding this comment.
kind of nit, but what does a "push writing" mean versus pull? Maybe if I read other javadocs it would be clear what that concept is?
There was a problem hiding this comment.
maybe just looking for this javadoc to work a bit harder in explaining things!
| Map.Entry<SolrInputDocument, Map<String, Object>> firstDoc = | ||
| docs.entrySet().iterator().next(); | ||
| Map<String, Object> map = firstDoc.getValue(); | ||
| Integer cw = null; |
There was a problem hiding this comment.
seems odd to have both cw and later commitWithin, and since we don't normally abbriveate variables, do commitWithin and overwrite everywhere?
| @@ -50,36 +47,14 @@ public interface ContentWriter { | |||
| * org.apache.solr.client.solrj.request.RequestWriter#getContentStreams(SolrRequest)} is invoked | |||
| * to do a pull write. | |||
| */ | |||
| public ContentWriter getContentWriter(SolrRequest<?> req) { | |||
| /** | ||
| * @deprecated Method will be removed in Solr 10.0. Use {@link XMLRequestWriter} instead. | ||
| */ | ||
| @Deprecated(since = "9.9") |
There was a problem hiding this 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!
https://issues.apache.org/jira/browse/SOLR-17518
Description
Update request in XML are written directly by
UpdateRequest. It would be much cleaner to have a dedicated class, specific to XML, similar to existingBinaryRequestWriter.Solution
Add
XMLRequestWriter.Methods
UpdateRequest.getXML()andUpdateRequest.writeXML()are flagged as deprecated, so the change may be merged in 9x branch. I plan to fully remove them from 10 in another PR.