SOLR-17743: V2 API: use correct content-type #3326
Conversation
| @@ -133,7 +133,6 @@ public class {{classname}} { | |||
| {{#isBodyParam}} | |||
| {{#vendorExtensions.x-genericEntity}} | |||
| this.requestBody = requestBody; | |||
| addHeader("Content-type", "application/json"); | |||
There was a problem hiding this comment.
This doesn't belong here. It's the ContentWriter that's responsible for the type.
There was a problem hiding this comment.
I do like that better from an abstraction perspective, but I remember putting this line here only after noticing that relying on the ContentWriter was leaving the header unset in some cases. I wish I remembered the context there better.
But let's not worry about that now - if it crops up again I'll reinvestigate at that point...
| @@ -205,26 +203,23 @@ public class {{classname}} { | |||
|
|
|||
| @Override | |||
| @SuppressWarnings("unchecked") | |||
| public RequestWriter.ContentWriter getContentWriter(String expectedType) { | |||
| public RequestWriter.ContentWriter getContentWriter(String _expectedTypeIGNORE) { | |||
There was a problem hiding this comment.
getContentWriter is passed an "expectedType" from the SolrClient.requestWriter which defaults to "javabin". But V2 should ignore that requestWriter as it's opinionated on how to write -- only supports JSON via Jackson or raw writing. Perhaps that field in SolrClient should be named defaultRequestWriter... although we're not choosing a different RequestWriter so.... 🤷
There was a problem hiding this comment.
[0] Maybe some version of this rationale should live as a comment in the code/template, so that we can notice when those preconditions/assumptions change?
e.g.
// v2 currently only supports reading JSON request-bodies, so we ignore the expected type suggestion provided by callers.
| @@ -205,26 +203,23 @@ public class {{classname}} { | |||
|
|
|||
| @Override | |||
| @SuppressWarnings("unchecked") | |||
| public RequestWriter.ContentWriter getContentWriter(String expectedType) { | |||
| public RequestWriter.ContentWriter getContentWriter(String _expectedTypeIGNORE) { | |||
There was a problem hiding this comment.
[0] Maybe some version of this rationale should live as a comment in the code/template, so that we can notice when those preconditions/assumptions change?
e.g.
// v2 currently only supports reading JSON request-bodies, so we ignore the expected type suggestion provided by callers.
https://issues.apache.org/jira/browse/SOLR-17743
I welcome input on testing (if needed).