On Wed, 6 Jun 2018 at 21:03, Chris Hegarty <chris.hega...@oracle.com> wrote:
>
> > On 4 Jun 2018, at 03:48, James Roper <ja...@lightbend.com> wrote:
> >
> > ...
> >
> > +1 on variants of getContentType that give you either the media type or
> the charset parsed from the content type header. Without them, we're going
> to end up with a lot of regexps doing ad hoc parsing to get the information
> needed out of the Content-Type header. For example, if you want to know if
> the incoming request is application/json, text/plain, or application/xml,
> in order to parse that correctly, you'll need to do things like
> .matches("text/plain(?;.*)?"). Whereas, if you had a getMediaType() you
> could just do a .equals("text/plain").
>
> I agree that it can be a little cumbersome, but still possible, to get
> to the meat of specific header values like Content-Type. There is a
> mini-design space around this; should a single method be exposed to
> return the Charset, or a higher-level interface like ContentType that
> exposes the media-type and the list of parameters, exceptions/defaults,
> etc?
>
Completely agreed that this is an important discussion to have. Personally
I don't have a strong opinion, I've had experience maintaining both styles
of API and for me there's no clear winner in which approach is best. Here's
a few additional considerations:
* Scope is always an issue with these types of features. Why stop at just
content type? Why not also add high level modelling around accept headers?
And so on... I think we need to carefully decide what the criteria is for
making a header a first class concept in the API. Some questions to ask
there (which I'm guessing you already have a good understanding of, but I
don't) is how much we expect this API to be used directly by end users, and
how much we expect it to be used underneath a higher level API, either as a
third party open source library, or perhaps we expect users to create their
own use case specific abstractions on top? The lower level it will sit (on
average), the less demand for higher level concepts in the API.
* If we model it using a higher level interface like ContentType, and maybe
we decide to model a few other headers using a higher level interface too,
then why not model all headers using a higher level interface, and use some
sort of strongly typed map API to get the header values? For example:
interface HeaderKey<V> {
String getHeaderName();
V parse(String value);
}
class ContentType {
...
}
class ContentTypeKey implements HeaderKey<ContentType> {
String getHeaderName() {
return "Content-Type";
}
ContentType parse(String value) {
// ...
}
}
class HttpHeaders {
<V> V getHeader(HeaderKey<V> key) {
String value = getHeader(key.getHeaderName());
if (value != null) {
return key.parse(value);
} else {
return null;
}
}
...
}
The above uses a parse on the fly approach (which would result in multiple
parses on each access), other approaches might involve registering header
keys when creating the client (and this can be taken advantage of to do
caching of parsed values, particularly with an HTTP2 underlying impl) etc.
Anyway, the above obviously means introducing a lot more API, and wouldn't
be worth it if the only thing we're interested in adding a convenience for
is the content type, but may be more useful if we want to add convenience
methods for more headers, since it means other libraries can provide their
own convenience methods using the same mechanism for parsing and accessing
headers.
I filed the following enhancement request to track this:
> https://bugs.openjdk.java.net/browse/JDK-8204470
>
> Since this request is more of a convenience API, and is additive, it may
> not happen in time for 11. There are a number of other higher priority
> issues that need to be resolved before JDK 11 RDP 1 [1] . Some of these
> have minor impact on the existing API, and as such need to be resolved
> in 11.
>
> -Chris.
>
> [1] http://openjdk.java.net/projects/jdk/11/
>
>
>
>
--
*James Roper*
*Senior Developer, Office of the CTO*
Lightbend <https://www.lightbend.com/> – Build reactive apps!
Twitter: @jroper <https://twitter.com/jroper>
