[jsr369-experts] Re: [servlet-spec users] Re: Re: PushBuilder

From: Greg Wilkins <gregw_at_webtide.com>
Date: Sat, 12 Mar 2016 07:28:59 +1100

On 12 March 2016 at 03:19, Edward Burns <edward.burns_at_oracle.com> wrote:

> GW> So a simple fix for 2. is that we simply say that isPushSupported
> always
> GW> returns false for pushed resources - ie it will always return false
> when
> GW> handling B, so B will never attempt to push C. Frameworks will need
> to do
> GW> transitive dependencies so the handling of A will push both B and C.
> With the EDR API I don't even see how this can happen. I thought we
> abandoned the synthetic GET approach? On HttpServletRequest A we call
> getPushBuilder(). This returns the PushBuilder that allows us to push
> B. There is no way in the API to get an HttpServletRequest for B, so
> there is no way even for there to be a request C. Can you please help
> me understand?

ServletA receives a request.
ServletB pushes resource B.
ServletB receives the pushed request (which to it looks pretty much the
same as a normal request).
ServletB pushes resource C.

We now have the problem! We are not allowed to push C onto the stream for
B by the protocol. If Stream A still exists, we can push C onto A, but
that is a race as stream A can complete normally at any time during the
handling of ServletB

Hence my preference to just say to ServletB that push is not supported.
Frameworks will have to adapt and when they receive the request to A, push
both B and C (ie the framework has to work out that B depends on C).

> Here is what we currently have.
> HttpServletRequest.getPushBuilder
> Returns a PushBuilder for issuing server push responses from the
> current request. If the current connection does not support server
> push, or server push has been disabled by the client via a
> SETTINGS_ENABLE_PUSH settings frame value of 0 (zero), a valid
> PushBuilder must still be returned, but calling any of the methods on
> it will have no effect.

Ah I thought we had gone for ISE if somebody called getPushBuilder after
isPushSupported returned false.

If we have kept the possibility that PushBuilders can be noops, then that
is good
for avoiding these kinds of races/issues

> PushBuilder.push()
> void push()
> Push a resource given the current state of the builder, returning
> immediately without blocking.
> Push a resource based on the current state of the PushBuilder. If
> isConditional() is true and an etag or lastModified value is provided,
> then an appropriate conditional header will be generated. If both an
> etag and lastModified value are provided only an If-None-Match header
> will be generated. If the builder has a session ID, then the pushed
> request will include the session ID either as a Cookie or as a URI
> parameter as appropriate. The builders query string is merged with any
> passed query string.
> Before returning from this method, the builder has its path, etag and
> lastModified fields nulled. All other fields are left as is for
> possible reuse in another push.
> Throws:
> IllegalArgumentException - if the method set expects a request body
> (eg POST)
> IllegalStateException - if there was no call to
> path(java.lang.String) on this instance either between its
> instantiation or the last call to push() that did not throw an
> IllegalStateException.
> The only way I can see us returning a boolean from push() is like this:
> @returns
> false if and only if calling isPushSupported() as the first action
> within the push() implementation returns false. Returns true otherwise.
> What do you think of that?

Actually as PB said, there are places that the race and/or failure can
happen even after
a true return. Perhaps it is just better to return void, but to note in
the javadoc that
a push may not happen, or may not be accepted. Ie don't write code that
expecting there will definitely be a request dispatched for each push.


> Ed
> --
> | edward.burns_at_oracle.com | office: +1 407 458 0017

Greg Wilkins <gregw@webtide.com> CTO http://webtide.com