Stuart / Shing,
I agree that removing the auto initialization of the query string makes
this API clearer.
I know there are some apps/frameworks that will have key values in the
query that need to be added to every associated push request, but they are
likely to be very specific and not generally all the query - this it is
best to handle explicitly.
cheers
On 19 August 2015 at 08:04, Stuart Douglas <sdouglas_at_redhat.com> wrote:
>
>
> ----- Original Message -----
> > From: "Shing Wai Chan" <shing.wai.chan_at_oracle.com>
> > To: jsr369-experts_at_servlet-spec.java.net
> > Sent: Wednesday, 19 August, 2015 7:57:16 AM
> > Subject: [servlet-spec users] [jsr369-experts] Re: Re:
> [SERVLET_SPEC-134] Push API vJetty
> >
> > Besides queryString, other http headers also have a similar issue.
> > So, we can do one of the following (as in queryString):
> > a) remove the initialized for headers
> > b) add removeHeader API
>
> I think removeHeader makes sense. Unlike the query string the
> automatically added headers are likely to be wanted in the pushed response.
>
> Stuart
>
> >
> > Any comments?
> > Thanks.
> > Shing Wai Chan
> >
> > On 8/17/15, 3:52 PM, Stuart Douglas wrote:
> > >
> > > ----- Original Message -----
> > >> From: "Shing Wai Chan"<shing.wai.chan_at_oracle.com>
> > >> To: jsr369-experts_at_servlet-spec.java.net
> > >> Cc: "Stuart Douglas"<sdouglas_at_redhat.com>
> > >> Sent: Tuesday, 18 August, 2015 2:48:42 AM
> > >> Subject: [jsr369-experts] Re: [servlet-spec users] Re:
> [SERVLET_SPEC-134]
> > >> Push API vJetty
> > >>
> > >> However, from the javadoc, the queryString is "initialized as
> follows":
> > >> * The query string from HttpServletRequest.getQueryString().
> > >>
> > >> So, this implies that the path() call inherits the query string from
> the
> > >> original request.
> > >> That is why I ask the question.
> > >>
> > >> To fix the issue, we can do one of the following:
> > >> a) remove the initialized for queryString from above
> > >> b) add remove/replace queryString API
> > >>
> > >> For the queryString issue, (a) is better.
> > >> But then we also need to consider similar issues for headers.
> > >
> > > Oops, I totally miss read that.
> > >
> > > I don't think that it makes any sense to initialise the query string
> to be
> > > the same as the original request. I think in the majority of cases the
> new
> > > request will have a different query string, so I think your option a)
> is
> > > the way to go.
> > >
> > > Stuart
> > >
> > >
> > >
> > >> Thanks.
> > >> Shing Wai Chan
> > >>
> > >> On 8/16/15, 6:01 PM, Stuart Douglas wrote:
> > >>> ----- Original Message -----
> > >>>> From: "Shing Wai Chan"<shing.wai.chan_at_oracle.com>
> > >>>> To: jsr369-experts_at_servlet-spec.java.net
> > >>>> Sent: Monday, 17 August, 2015 2:33:19 AM
> > >>>> Subject: [jsr369-experts] Re: [servlet-spec users]
> [SERVLET_SPEC-134]
> > >>>> Push
> > >>>> API vJetty
> > >>>>
> > >>>> From javadoc,
> > >>>> "The instance is initialized as follows:
> > >>>> ...
> > >>>> * The query string from HttpServletRequest.getQueryString()
> > >>>>
> > >>>> PushBuilder queryString(String queryString)
> > >>>> Set the query string to be used for the push. Will be appended to
> any
> > >>>> query String included in a call to path(String).
> > >>>> "
> > >>>>
> > >>>> So, this means we can only add more query String, but cannot modify
> or
> > >>>> remove the query string from the httpServletRequest.
> > >>>> Is it an issue here? Do we need APIs to remove or clean up or
> replace
> > >>>> the query string?
> > >>>> (Just consider the case of push a jpg file which may/do not need
> query
> > >>>> string. Or in some case, we need to "replace" the query string.)
> > >>>> (The same may be for other headers.)
> > >>> This is the query string from the .path() call, not the query string
> from
> > >>> the original
> > >>> request. I think in this case it makes sense, if you don't want a
> query
> > >>> string just don't
> > >>> include one in the call to path().
> > >>>
> > >>> Stuart
> > >>>
> > >>>> Shing Wai Chan
> > >>>>
> > >>>>
> > >>>> On 8/14/15, 4:09 PM, Edward Burns wrote:
> > >>>>>>>>>> On Wed, 12 Aug 2015 09:16:19 +1000, Greg
> > >>>>>>>>>> Wilkins<gregw_at_webtide.com>
> > >>>>>>>>>> said:
> > >>>>> GW> We've not done a lot of work or analysis on this
> > >>>>> GW> recently.... however it is included in our stable release
> and
> > >>>>> here
> > >>>>> GW> is the API that we are currently providing:
> > >>>>>
> > >>>>> GW>
> > >>>>>
> https://github.com/eclipse/jetty.project/blob/master/jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilder.java
> > >>>>>
> > >>>>> Thanks Greg,
> > >>>>>
> > >>>>> I've taken this as a starting point and created a branch on which
> we
> > >>>>> can
> > >>>>> iterate. I've published the artifact and the javadoc is at [1].
> > >>>>>
> > >>>>> I've added getPushBuilder() to HttpServletRequest [2].
> > >>>>>
> > >>>>> I've reworded your text somewhat, and am still in the process of
> doing
> > >>>>> that. The biggest and only normative change to what you had is in
> the
> > >>>>> final sentance of the class javadoc for PushBuilder. I've called
> out
> > >>>>> some specific areas for discussion here.
> > >>>>>
> > >>>>> A PushBuilder can be customized by chained calls to mutator
> > >>>>> methods
> > >>>>> before the push() method is called to *initiate an
> asynchronous
> > >>>>> push
> > >>>>> request with the current state of the builder.* After the
> call to
> > >>>>> push(), the builder may be reused for another push, *however
> the
> > >>>>> implementation must make it so the path(String), etag(String)
> and
> > >>>>> lastModified(String) values are cleared before returning from
> > >>>>> push()*. All other values are retained over calls to push().
> > >>>>>
> > >>>>> GW> However, while it is being used, I would not characterize
> this
> > >>>>> as
> > >>>>> GW> extensively tested or analysed. To my knowledge nobody has
> used
> > >>>>> the
> > >>>>> API
> > >>>>> GW> directly in a framework, but some have deployed the push
> filter
> > >>>>> in
> > >>>>> their
> > >>>>> GW> applications.
> > >>>>>
> > >>>>> GW> Note that while I like the idea of the simple degenerate
> case,
> > >>>>> my
> > >>>>> limited
> > >>>>> GW> experience is that building the synthetic request is
> actually a
> > >>>>> lot
> > >>>>> more
> > >>>>> GW> complex than you'd think and there are lots of corner/edge
> > >>>>> cases.
> > >>>>> Even the
> > >>>>> GW> PushBuilder needs to synthesize/validate headers and the
> rules
> > >>>>> for
> > >>>>> that
> > >>>>> GW> will need to be carefully crafted and explained to achieve
> > >>>>> portability.
> > >>>>>
> > >>>>> You know, I think I can live without the dispatcher that I showed
> at
> > >>>>> GeekOut on slide 70 of [3]:
> > >>>>>
> > >>>>> It could just as well be
> > >>>>>
> > >>>>>
> ((HttpServletRequest)request).getRequestBuilder().path(url).push();
> > >>>>>
> > >>>>> Let's get some discussion going here!
> > >>>>>
> > >>>>> Ed
> > >>>>>
> >
>
--
Greg Wilkins <gregw@webtide.com> CTO http://webtide.com