JEP 321 surface-level comments

[Tobias Thierer]

On Tue, May 8, 2018 at 12:35 PM Chris Povirk <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 of JDK-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 for accept-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).

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.

>
>    - 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.

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?

Tobias
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20180510/0952da9a/attachment.html>