On 8/19/15, 1:59 PM, Greg Wilkins wrote:
> Ed,
>
> Should we also be explicit about how query string included in setPath
> and one in setQueryString are handled?
>
> I'm assuming that they should be merged and the key difference is that
> setPath is reset each cycle, but the setQueryString is not.
There are two ways of merging for parameters with the same name. (Let us
take a simple example: path with a=1, query with a=2.)
a) replace a=2
b) aggregate, a=1, a=2
It should be (b).
Any comment?
Shing Wai Chan
>
> cheers
>
>
> On 20 August 2015 at 02:52, Edward Burns <edward.burns_at_oracle.com
> <mailto:edward.burns_at_oracle.com>> wrote:
>
> Thanks for the review and discussion on this. Let's keep the momentum
> going!
>
> >>>>> On Sun, 16 Aug 2015 09:33:19 -0700, Shing Wai Chan
> <shing.wai.chan_at_oracle.com <mailto: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 <mailto: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 <mailto: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 <mailto: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 <tel:16%20-0700>, Shing
> Wai Chan <shing.wai.chan_at_oracle.com
> <mailto: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 <mailto: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 <mailto: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
> HttpServletRequest.getQueryString()".
>
> 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
> comment.
>
> Thanks,
>
> Ed
>
> --
> | edward.burns_at_oracle.com <mailto:edward.burns_at_oracle.com> |
> office: +1 407 458 0017 <tel:%2B1%20407%20458%200017>
> | 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
> <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/%21/javax/servlet/http/PushBuilder.html>
>
>
>
>
> --
> Greg Wilkins <gregw_at_webtide.com <mailto:gregw_at_webtide.com>> CTO
> http://webtide.com