Simplify ResponseParser.getContentType processing#4528
Conversation
Especially helps HttpSolrClient and implementations.
dsmiley
left a comment
There was a problem hiding this comment.
Changelog?. As it's just a refactoring, not thinking a JIRA.
| * @return the MIME types that this parser is capable of parsing. Never null. | ||
| */ | ||
| public abstract Collection<String> getContentTypes(); | ||
| public abstract Set<String> getContentTypes(); |
There was a problem hiding this comment.
This is a backwards incompatible change... and it it's only caller should be our own code, and I think very few people would have a custom one of these.
| assert validateContentTypes(); | ||
| } | ||
|
|
||
| private boolean validateContentTypes() { |
There was a problem hiding this comment.
The best place for validation is here when instantiating. These are new rules... if someone actually has a custom non-compliant impl, they'll find out.
| } | ||
|
|
||
| @Test | ||
| public void testProcessorMimeTypes() throws Exception { |
There was a problem hiding this comment.
from Claude:
The deleted test was really checking the behavior of processorAcceptsMimeType — specifically, that it correctly stripped the ; charset=UTF-8 from "application/xml;
charset=UTF-8" before comparing. That was non-trivial because the parser used to return full content-type strings.
Now that getContentTypes() returns pre-normalized MIME types and checkContentType just does contains(mimeType.toLowerCase(...)), the interesting logic has moved into the
parsers themselves. A rewritten test in the base class would basically just be:
assertTrue(new XMLResponseParser().getContentTypes().contains("application/xml"));
assertFalse(new XMLResponseParser().getContentTypes().contains("application/json"));
That's testing the parser, not the client. It would belong in a XMLResponseParserTest or similar, not in an HTTP client integration test. And since it's just verifying
static constants, it's of marginal value — the constructor assertion in ResponseParser already catches malformed values.
I'd leave it deleted.
|
I have a changelog and plan to merge Monday subject to feedback. |
Especially helps HttpSolrClient and implementations.