JEP 321 surface-level comments

[Chris Hegarty]

Hi Tobias,

On 10/05/18 19:51, Tobias Thierer wrote:
> On Tue, May 8, 2018 at 12:35 PM Chris Povirk <cpovirk at google.com 
> <mailto:cpovirk at google.com>> wrote:
> 
> Commenting inline on just a couple of points.
> 
>       * I second the idea of making HttpHeaders a final class, as
>         proposed as part ofJDK-8184274
>         <https://bugs.openjdk.java.net/browse/JDK-8184274>. While "final
>         class" was part of that suggestion, it doesn't seem to have been
>         specifically shot down. Tobias suggested that the goal of an
>         extensible class was to let users plug in a lazily parsed map
>         for HTTP 2, but he didn't seem to think this was likely to work,
>         given the HTTP 2 spec. The pros I see for a final class, besides
>         the usual advantages of simplicity, are: It could ensure that
>         instances use a Map that retains key order and doesn't contain
>         nulls. It could also implement case insensitivity, which the
>         current implementation claims to do but doesn't. (Maybe it's
>         relying on internal callers to do normalize strings before
>         calling it? But that doesn't help the external caller who asks
>         foraccept-encoding instead of Accept-Encoding.)
> 
> To give more background: The argument of allowing different 
> implementations that could do stuff such as parse headers lazily was 
> suggested to me (verbally, I think) by either Chris Hegarty or Michael 
> McMahon. Like you (Chris Povirk) say, after carefully reading RFC 7541 
> (HPACK), I didn't find the argument convincing because I didn't see a 
> way for how header parsing could to a substantial extent be done 
> "lazily" - any RFC 7541 compliant implementation will need to keep track 
> of the dynamic table state, at which point there's not much work left 
> that could be done lazily. I believe I gave this feedback to Chris and 
> Michael, too (perhaps even on this list).

Already answered in my reply to Chris Povirk.

> The additional points you (Chris Povirk) raise about guaranteeing a key 
> order and consistent behavior with respects to nulls make sense to me. 
> Since HttpHeaders is a pure value type, I think that further supports 
> the argument that it should be either a final class, or at least not 
> allow custom implementations by application developers.

Already answered in my reply to Chris Povirk.

>       * I'm a little confused by HttpRequest. At first, I thought it was
>         a value type like HttpHeaders, so I was going to have the same
>         suggestion of making it a final type (and also its builder). But
>         the implementation appears to be mutable and to contain
>         additional fields. It feels like the type is pulling double duty
>         as both a value type in the public API (which could be nice to
>         make final) and as a stateful internal type (which might contain
>         an instance of the public type), and it's not clear if different
>         implementations could successfully interchange HttpRequest
>         instances. (Tobias said that he'd raised this point earlier, and
>         it sounds like it may realistically have to stay how it is, but
>         since this was my reaction and it turned out to match what he'd
>         said, I wanted to at least mention it again.)
> 
> The mix of concerns (business logic vs. value type) did also strike me 
> as making the API more difficult to use because it closely couples the 
> business logic and value type semantics - making it harder/impossible 
> for an application to reimplement e.g. only some business logic. This 
> was discussed in more detailed feedback that Google provided a long time 
> ago but it wasn't clear (to me, at least) whether/how we'd be able to 
> provide additional value beyond making sure that Chris and Michael were 
> aware of this perspective.

We did discuss this a long time ago, and we addressed the substantive
points at the time, i.e the sendXXX methods were previously virtual
methods on HttpRequest, thus mixing function and state within
HttpRequest. There were other comments, some relating to extensibility,
etc, that were also addressed in the first round of feedback from
Google. The second round of feedback was transcribed into JIRA items,
retrievable at https://bugs.openjdk.java.net/issues/?filter=3D31878

> Also, my impression was that Chris and Michael have been trying to 
> finalize the API for a while and that it might be too late for such changes?

The primary goal of JEP 321 [1] is to standarize the API. The JEP has
been targeted to JDK 11 [2]. The JDK 11 schedule is available on the
JDK 11 release specific section of the JDK project page [3].


-Chris.

[1] http://openjdk.java.net/jeps/321
[2] http://mail.openjdk.java.net/pipermail/jdk-dev/2018-April/001072.html
[3] http://openjdk.java.net/projects/jdk/11/