[jsr369-experts] [SERVLET_SPEC-134] Push API vJetty2

From: Edward Burns <edward.burns_at_oracle.com>
Date: Wed, 19 Aug 2015 09:52:19 -0700

Thanks for the review and discussion on this. Let's keep the momentum

>>>>> On Sun, 16 Aug 2015 09:33:19 -0700, Shing Wai Chan <shing.wai.chan_at_oracle.com> said:

Spec> "The instance is initialized as follows:
Spec> ...
Spec> * The query string from HttpServletRequest.getQueryString()

Spec> PushBuilder queryString(String queryString) Set the query string
Spec> to be used for the push. Will be appended to any query String
Spec> included in a call to path(String).

SWC> So, this means we can only add more query String, but cannot modify
SWC> or remove the query string from the httpServletRequest. Is it an
SWC> issue here?

Yes, I do think this in an issue.

SWC> Do we need APIs to remove or clean up or replace the query string?
SWC> (Just consider the case of push a jpg file which may/do not need
SWC> query string. Or in some case, we need to "replace" the query
SWC> string.) (The same may be for other headers.)

>>>>> On Sun, 16 Aug 2015 21:01:39 -0400 (EDT), Stuart Douglas <sdouglas_at_redhat.com> said:

SD> This is the query string from the .path() call, not the query string
SD> from the original request. I think in this case it makes sense, if
SD> you don't want a query string just don't include one in the call to
SD> path().

No, Shing-wai is correct with the way it is currently worded in the vJetty

>>>>> On Mon, 17 Aug 2015 09:48:42 -0700, Shing Wai Chan <shing.wai.chan_at_oracle.com> said:

SWC> However, from the javadoc, the queryString is "initialized as follows":
SWC> * The query string from HttpServletRequest.getQueryString().

SWC> So, this implies that the path() call inherits the query string
SWC> from the original request. That is why I ask the question.

SWC> To fix the issue, we can do one of the following:
SWC> a) remove the initialized for queryString from above
SWC> b) add remove/replace queryString API

SWC> For the queryString issue, (a) is better. But then we also need to
SWC> consider similar issues for headers.

The proposal provides for headers:

Spec> The existing headers of the current HttpServletRequest are added
Spec> to the builder, except for:

Spec> Conditional headers (eg. If-Modified-Since)
Spec> Range headers
Spec> Expect headers
Spec> Authorization headers
Spec> Referrer headers

>>>>> On Mon, 17 Aug 2015 18:52:02 -0400 (EDT), Stuart Douglas <sdouglas_at_redhat.com> said:

SD> I don't think that it makes any sense to initialise the query string
SD> to be the same as the original request. I think in the majority of
SD> cases the new request will have a different query string, so I think
SD> your option a) is the way to go.

I agree.

>>>>> On Tue, 18 Aug 2015 14:57:16 -0700, Shing Wai Chan <shing.wai.chan_at_oracle.com> said:

SWC> Besides queryString, other http headers also have a similar issue.
SWC> So, we can do one of the following (as in queryString):
SWC> a) remove the initialized for headers
SWC> b) add removeHeader API

I like b) here.

>>>>> On Tue, 18 Aug 2015 18:04:59 -0400 (EDT), Stuart Douglas <sdouglas_at_redhat.com> said:

SD> I think removeHeader makes sense. Unlike the query string the
SD> automatically added headers are likely to be wanted in the pushed
SD> response.

>>>>> On Wed, 19 Aug 2015 08:52:45 +1000, Greg Wilkins <gregw_at_webtide.com> said:

GW> I agree that removing the auto initialization of the query string makes
GW> this API clearer.

GW> I know there are some apps/frameworks that will have key values in the
GW> query that need to be added to every associated push request, but they are
GW> likely to be very specific and not generally all the query - this it is
GW> best to handle explicitly.

I have removed "The query string from

I have added removeHeader().

Also, I don't see the point of having both setHeader() and addHeader()
so I have removed addHeader() and modified setHeader() to clarify that
if there is an existing header with that name, it is overwritten.

The vJetty2 revision of the proposal is at [1]. Please review and



| edward.burns_at_oracle.com | office: +1 407 458 0017
| 54 Business days til JavaOne 2015
| 69 Business days til DOAG 2015
[1] https://maven.java.net/service/local/repositories/snapshots/archive/javax/servlet/javax.servlet-api/4.0.0-b01-SERVLET_SPEC-134-SNAPSHOT/javax.servlet-api-4.0.0-b01-SERVLET_SPEC-134-20150819.164748-2-javadoc.jar/!/javax/servlet/http/PushBuilder.html