You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martin Grigorov <mg...@apache.org> on 2016/08/15 15:35:45 UTC
Re: wicket git commit: WICKET-6194 PushHeaderItem handles client side cache
Hi Tobias,
On Thu, Aug 11, 2016 at 8:45 PM, <ts...@apache.org> wrote:
> Repository: wicket
> Updated Branches:
> refs/heads/master 061c0e903 -> 76116aa46
>
>
> WICKET-6194 PushHeaderItem handles client side cache
>
> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/76116aa4
> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/76116aa4
> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/76116aa4
>
> Branch: refs/heads/master
> Commit: 76116aa467590378ae6b5d5acc8a153246fc9ee4
> Parents: 061c0e9
> Author: Tobias Soloschenko <ts...@apache.org>
> Authored: Thu Aug 11 20:43:52 2016 +0200
> Committer: Tobias Soloschenko <ts...@apache.org>
> Committed: Thu Aug 11 20:45:00 2016 +0200
>
> ----------------------------------------------------------------------
> .../http2/markup/head/PushHeaderItem.java | 148 ++++++++++++++++++-
> .../src/docs/guide/http2push/http2push_1.gdoc | 44 +++++-
> 2 files changed, 183 insertions(+), 9 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/
> 76116aa4/wicket-experimental/wicket-http2/wicket-http2-
> core/src/main/java/org/apache/wicket/http2/markup/head/PushHeaderItem.java
> ----------------------------------------------------------------------
> diff --git a/wicket-experimental/wicket-http2/wicket-http2-core/src/
> main/java/org/apache/wicket/http2/markup/head/PushHeaderItem.java
> b/wicket-experimental/wicket-http2/wicket-http2-core/src/
> main/java/org/apache/wicket/http2/markup/head/PushHeaderItem.java
> index 5e88a2f..2238c61 100644
> --- a/wicket-experimental/wicket-http2/wicket-http2-core/src/
> main/java/org/apache/wicket/http2/markup/head/PushHeaderItem.java
> +++ b/wicket-experimental/wicket-http2/wicket-http2-core/src/
> main/java/org/apache/wicket/http2/markup/head/PushHeaderItem.java
> @@ -16,6 +16,11 @@
> */
> package org.apache.wicket.http2.markup.head;
>
> +import java.io.IOException;
> +import java.net.URL;
> +import java.text.ParseException;
> +import java.text.SimpleDateFormat;
> +import java.util.Date;
> import java.util.List;
> import java.util.Set;
> import java.util.TreeSet;
> @@ -27,19 +32,26 @@ import org.apache.wicket.Page;
> import org.apache.wicket.WicketRuntimeException;
> import org.apache.wicket.http2.Http2Settings;
> import org.apache.wicket.markup.head.HeaderItem;
> +import org.apache.wicket.markup.html.WebPage;
> import org.apache.wicket.request.IRequestHandler;
> import org.apache.wicket.request.Request;
> import org.apache.wicket.request.Response;
> import org.apache.wicket.request.Url;
> import org.apache.wicket.request.cycle.RequestCycle;
> +import org.apache.wicket.request.http.WebRequest;
> +import org.apache.wicket.request.http.WebResponse;
> import org.apache.wicket.request.mapper.parameter.PageParameters;
> import org.apache.wicket.request.mapper.parameter.PageParametersEncoder;
> import org.apache.wicket.request.resource.ResourceReference;
> import org.apache.wicket.util.collections.ConcurrentHashSet;
> +import org.apache.wicket.util.time.Time;
>
> /**
> * A push header item to be used in the http/2 context and to reduce the
> latency of the web
> - * application
> + * application. Follow these steps for your page:<br><br>
> + * - Override the setHeaders method and don't call super.setHeaders to
> disable caching<br>
> + * - Get the page request / response and store them as transient fields
> that are given into the PushHeaderItem<br>
> + * - Ensure a valid https connection (not self signed), because otherwise
> no caching information are accepted from Chrome or other browsers
> *
> * @author Tobias Soloschenko
> *
> @@ -49,6 +61,12 @@ public class PushHeaderItem extends HeaderItem
> private static final long serialVersionUID = 1L;
>
> /**
> + * The header date format for if-modified-since / last-modified
> + */
> + private static final SimpleDateFormat headerDateFormat = new
> SimpleDateFormat(
> + "EEE, dd MMM yyyy HH:mm:ss zzz");
>
SimpleDateFormat is not thread safe.
Better use the new java.time.DateTimeFormatter
> +
> + /**
> * The http2 protocol string
> */
> public static final String HTTP2_PROTOCOL = "http/2";
> @@ -62,6 +80,47 @@ public class PushHeaderItem extends HeaderItem
> * The URLs of resources to be pushed to the client
> */
> private Set<String> urls = new ConcurrentHashSet<String>(new
> TreeSet<String>());
> + /**
> + * The web response of the page to apply the caching information to
> + */
> + private WebResponse pageWebResponse;
> +
> + /**
> + * The web request of the page to get the caching information from
> + */
> + private WebRequest pageWebRequest;
> +
> + /**
> + * The page to get the modification time of
> + */
> + private Page page;
> +
> + /**
> + * Creates a push header item based on the given page and the
> corresponding page request / page
> + * response. To get the request and response
> + *
> + *
> + * @param page
> + * the page this header item is applied to
> + * @param pageRequest
> + * the page request this header item is applied to
> + * @param pageResponse
> + * the page response this header item is applied to
> + */
> + public PushHeaderItem(Page page, Request pageRequest, Response
> pageResponse)
> + {
> + if (page == null || !(page instanceof WebPage) ||
> pageResponse == null ||
> + !(pageResponse instanceof WebResponse))
> + {
> + throw new WicketRuntimeException(
> + "Please hand over the web page, the web
> response and the web request to the push header item like \"new
> PushHeaderItem(this, yourWebPageReponse, yourWebPageRequest)\" - " +
> + "The webPageResponse /
> webPageRequest can be obtained via \"getRequestCycle().getResponse()\"
> and placed into the page as field " +
> + "\"private transient Response
> webPageResponse;\" / \"private transient Request webPageRequest;\"");
> + }
> + this.pageWebResponse = (WebResponse)pageResponse;
> + this.pageWebRequest = (WebRequest)pageRequest;
> + this.page = page;
> + }
>
> /**
> * Uses the URLs that has already been pushed to the client to
> ensure not to push them again
> @@ -78,23 +137,102 @@ public class PushHeaderItem extends HeaderItem
> }
>
> /**
> + * Gets the time the page of this header item has been modified
> + *
> + * @return the time the page of this header item has been modified
> + */
> + private Time getPageModificationTime()
> + {
> + URL resource = page.getClass().getResource(
> page.getClass().getSimpleName() + ".html");
>
What is the .class is modified but the markup is still the same ?
What if the resource itself is modified ?
> + if (resource == null)
> + {
> + throw new WicketRuntimeException(
> + "The markup to the page couldn't be found:
> " + page.getClass().getName());
> + }
> + try
> + {
> + return Time.valueOf(new
> Date(resource.openConnection().getLastModified()));
> + }
> + catch (IOException e)
> + {
> + throw new WicketRuntimeException(
> + "The time couln't be determined of the
> markup file of the page: " +
> + page.getClass().getName(),
> + e);
> + }
> + }
> +
> + /**
> + * Applies the cache header item to the response
> + */
> + private void applyPageCacheHeader()
> + {
> + Time pageModificationTime = getPageModificationTime();
> + // check modification of page html
> + pageWebResponse.setLastModifiedTime(pageModificationTime);
> + pageWebResponse.setDateHeader("Expires",
> pageModificationTime);
> + pageWebResponse.setHeader("Cache-Control",
> + "max-age=31536000, public, must-revalidate,
> proxy-revalidate");
> + pageWebResponse.setHeader("Pragma", "public");
>
This sets caching response headers for the whole page.
Usually Wicket pages are stateful, thus it is better to not cache them.
See org.apache.wicket.markup.html.WebPage#setHeaders()
Now with this logic here it becomes harder for the application developer to
find out why the browser stopped making requests to the page.
(I haven't read the discussion at Jetty's issue tracker yet. It is in my
mail queue.)
> + }
> +
> + /**
> * Pushes the previously created URLs to the client
> */
> @Override
> public void render(Response response)
> {
> + // applies the caching header to the actual page request
> + applyPageCacheHeader();
> +
> HttpServletRequest request = getContainerRequest(
> RequestCycle.get().getRequest());
> // Check if the protocol is http/2 or http/2.0 to only
> push the resources in this case
> if (isHttp2(request))
> {
> - // Receives the vendor specific push builder
> - Http2Settings http2Settings =
> Http2Settings.Holder.get(Application.get());
> - PushBuilder pushBuilder =
> http2Settings.getPushBuilder();
> - pushBuilder.push(request, urls.toArray(new
> String[urls.size()]));
> + try
> + {
> + Time pageModificationTime =
> getPageModificationTime();
> + String ifModifiedSinceHeader =
> pageWebRequest.getHeader("If-Modified-Since");
> +
> + if (ifModifiedSinceHeader != null)
> + {
> + Time
> ifModifiedSinceFromRequestTime = Time
> + .valueOf(headerDateFormat.
> parse(ifModifiedSinceHeader));
> + // If the client modification time
> is before the page modification time -
> + // don't push
> + if (ifModifiedSinceFromRequestTime
> .before(pageModificationTime))
> + {
> + push(request);
> + }
> + }
> + else
> + {
> +
> + }
> + }
> + catch (ParseException e)
> + {
> + // If the If-Modified-Since time can't be
> parsed - the push handling is going to be
> + // skipped
> + }
> }
> }
>
> /**
> + * Pushed all URLs of this header item to the client
> + *
> + * @param request
> + * the request to push the URLs to
> + */
> + private void push(HttpServletRequest request)
> + {
> + // Receives the vendor specific push builder
> + Http2Settings http2Settings = Http2Settings.Holder.get(
> Application.get());
> + PushBuilder pushBuilder = http2Settings.getPushBuilder();
> + pushBuilder.push(request, urls.toArray(new
> String[urls.size()]));
> + }
> +
> + /**
> * Creates a URL and pushes the resource to the client - this is
> only supported if http2 is
> * enabled
> *
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/
> 76116aa4/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> ----------------------------------------------------------------------
> diff --git a/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> b/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> index d6888fd..59d53fe 100644
> --- a/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> +++ b/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> @@ -21,12 +21,48 @@ For the setup you need to follow those steps:
> </dependency>
> {code}
>
> -3. Add the following PushHeaderItem to the renderHead method in your
> component.
> +3. Use the PushHeader Item like in this example page:
> Example:
> {code}
> -TestResourceReference instance = TestResourceReference.getInstance();
> -response.render(CssHeaderItem.forReference(instance));
> -response.render(new PushHeaderItem().push(Arrays.asList(new
> PushItem(instance))));
> +public class HTTP2Page extends WebPage
> +{
> + private static final long serialVersionUID = 1L;
> +
> + private transient Response webPageResponse;
> +
> + private transient Request webPageRequest;
>
IMO this example is bad.
#renderHead() is called for each request cycle.
If the page is stateful then the chance for NullPointerException is high
with those transient fields.
> +
> + public HTTP2Page()
> + {
> + webPageResponse = getRequestCycle().getResponse();
> + webPageRequest = getRequestCycle().getRequest();
> + add(new Label("label", "Label"));
> + }
> +
> + @Override
> + public void renderHead(IHeaderResponse response)
> + {
> + super.renderHead(response);
> + TestResourceReference instance = TestResourceReference.
> getInstance();
> + response.render(CssHeaderItem.forReference(instance));
> + response.render(new PushHeaderItem(this, webPageResponse,
> webPageRequest)
>
Isn't it better if PushHeaderItem has overrideable methods like
#getResponse() and #getRequest() which by default use
RequestCycle.get().getXyz() ?
This way the application developers will type less in most cases and
override those methods only when really needed.
> + .push(Arrays.asList(new PushItem(instance))));
> + }
> +
> + @Override
> + protected void setHeaders(WebResponse response)
> + {
> + // NOOP just disable caching
> + }
> +}
> {code}
>
> Basically the resource is pushed before the actual response of the
> component is send to the client (browser) and because of this the client
> does not need to send an additional request.
> +
> +The PushHeaderItem behaves like explained in the following steps:
> +* When a browser requests the page with an initial commit everything is
> going to be pushed with (200)
> +* When a browser requests the page a second time resources are not pushed
> (304) not modified, because of the actual ResourceReferences headers
> +* When a browser requests the page a second time and the markup of the
> page has changed everything is going to be pushed again (200)
> +* When a browser requests the page a second time and resource references
> has been changed but not the page markup, all changed resource references
> are shipped via separate requests
> +
> +Note: Chrome does not set cache headers if the https connection is not
> secure (self signed) / valid - so ensure that a valid https connection is
> available with your server.
> \ No newline at end of file
>
>
Fwd: wicket git commit: WICKET-6194 PushHeaderItem handles client
side cache
Posted by Tobias Soloschenko <to...@googlemail.com>.
Hi,
yesterday I did some research and made some notes - here they are:
I had a look at your suggestions and I found out that within a header item
we can't use RequestCycle.get(), because it does not return the actual page
request / response within the renderHead method.
Thats the reason why I used to get the Request / Response within the
constructor of the page currently (example). Are there other ways to get
the last page request / response?
For the date I use this, now:
private static final DateTimeFormatter headerDateFormat =
DateTimeFormatter.ofPattern("EEE, dd MMM yyyy HH:mm:ss zzz");
...
Time ifModifiedSinceFromRequestTime = Time.valueOf(Date.from(
LocalDateTime.parse(ifModifiedSinceHeader, headerDateFormat).toInstant(
ZoneOffset.UTC)));
for http/2 push we actually need to set a hint when the index page has been
changed the last time and because of this date the other resources are
cached. (see jetty ticket for that)
Anyway I think we can handle the cache this way, because of must-revalidate:
pageWebResponse.setHeader("Cache-Control", "max-age=31536000, public,
must-revalidate, proxy-revalidate");
pageWebResponse.setHeader("Pragma", "public");
Even if the client holds a cache entry the request is made and the server
responds with the page content - or am I wrong here?
kind regards
Tobias
---------- Forwarded message ----------
From: Tobias Soloschenko <to...@googlemail.com>
Date: 2016-08-16 7:44 GMT+02:00
Subject: Re: wicket git commit: WICKET-6194 PushHeaderItem handles client
side cache
To: "dev@wicket.apache.org" <de...@wicket.apache.org>
Hi Martin,
2016-08-15 17:35 GMT+02:00 Martin Grigorov <mg...@apache.org>:
> Hi Tobias,
>
> On Thu, Aug 11, 2016 at 8:45 PM, <ts...@apache.org> wrote:
>
> > Repository: wicket
> > Updated Branches:
> > refs/heads/master 061c0e903 -> 76116aa46
> >
> >
> > WICKET-6194 PushHeaderItem handles client side cache
> >
> > Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/76116aa4
> > Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/76116aa4
> > Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/76116aa4
> >
> > Branch: refs/heads/master
> > Commit: 76116aa467590378ae6b5d5acc8a153246fc9ee4
> > Parents: 061c0e9
> > Author: Tobias Soloschenko <ts...@apache.org>
> > Authored: Thu Aug 11 20:43:52 2016 +0200
> > Committer: Tobias Soloschenko <ts...@apache.org>
> > Committed: Thu Aug 11 20:45:00 2016 +0200
> >
> > ----------------------------------------------------------------------
> > .../http2/markup/head/PushHeaderItem.java | 148
> ++++++++++++++++++-
> > .../src/docs/guide/http2push/http2push_1.gdoc | 44 +++++-
> > 2 files changed, 183 insertions(+), 9 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> > http://git-wip-us.apache.org/repos/asf/wicket/blob/
> > 76116aa4/wicket-experimental/wicket-http2/wicket-http2-
> > core/src/main/java/org/apache/wicket/http2/markup/head/PushH
> eaderItem.java
> > ----------------------------------------------------------------------
> > diff --git a/wicket-experimental/wicket-http2/wicket-http2-core/src/
> > main/java/org/apache/wicket/http2/markup/head/PushHeaderItem.java
> > b/wicket-experimental/wicket-http2/wicket-http2-core/src/
> > main/java/org/apache/wicket/http2/markup/head/PushHeaderItem.java
> > index 5e88a2f..2238c61 100644
> > --- a/wicket-experimental/wicket-http2/wicket-http2-core/src/
> > main/java/org/apache/wicket/http2/markup/head/PushHeaderItem.java
> > +++ b/wicket-experimental/wicket-http2/wicket-http2-core/src/
> > main/java/org/apache/wicket/http2/markup/head/PushHeaderItem.java
> > @@ -16,6 +16,11 @@
> > */
> > package org.apache.wicket.http2.markup.head;
> >
> > +import java.io.IOException;
> > +import java.net.URL;
> > +import java.text.ParseException;
> > +import java.text.SimpleDateFormat;
> > +import java.util.Date;
> > import java.util.List;
> > import java.util.Set;
> > import java.util.TreeSet;
> > @@ -27,19 +32,26 @@ import org.apache.wicket.Page;
> > import org.apache.wicket.WicketRuntimeException;
> > import org.apache.wicket.http2.Http2Settings;
> > import org.apache.wicket.markup.head.HeaderItem;
> > +import org.apache.wicket.markup.html.WebPage;
> > import org.apache.wicket.request.IRequestHandler;
> > import org.apache.wicket.request.Request;
> > import org.apache.wicket.request.Response;
> > import org.apache.wicket.request.Url;
> > import org.apache.wicket.request.cycle.RequestCycle;
> > +import org.apache.wicket.request.http.WebRequest;
> > +import org.apache.wicket.request.http.WebResponse;
> > import org.apache.wicket.request.mapper.parameter.PageParameters;
> > import org.apache.wicket.request.mapper.parameter.PageParametersEnc
> oder;
> > import org.apache.wicket.request.resource.ResourceReference;
> > import org.apache.wicket.util.collections.ConcurrentHashSet;
> > +import org.apache.wicket.util.time.Time;
> >
> > /**
> > * A push header item to be used in the http/2 context and to reduce the
> > latency of the web
> > - * application
> > + * application. Follow these steps for your page:<br><br>
> > + * - Override the setHeaders method and don't call super.setHeaders to
> > disable caching<br>
> > + * - Get the page request / response and store them as transient fields
> > that are given into the PushHeaderItem<br>
> > + * - Ensure a valid https connection (not self signed), because
> otherwise
> > no caching information are accepted from Chrome or other browsers
> > *
> > * @author Tobias Soloschenko
> > *
> > @@ -49,6 +61,12 @@ public class PushHeaderItem extends HeaderItem
> > private static final long serialVersionUID = 1L;
> >
> > /**
> > + * The header date format for if-modified-since / last-modified
> > + */
> > + private static final SimpleDateFormat headerDateFormat = new
> > SimpleDateFormat(
> > + "EEE, dd MMM yyyy HH:mm:ss zzz");
> >
>
> SimpleDateFormat is not thread safe.
> Better use the new java.time.DateTimeFormatter
>
>
Okay, we should use this instead - right.
>
>
> > +
> > + /**
> > * The http2 protocol string
> > */
> > public static final String HTTP2_PROTOCOL = "http/2";
> > @@ -62,6 +80,47 @@ public class PushHeaderItem extends HeaderItem
> > * The URLs of resources to be pushed to the client
> > */
> > private Set<String> urls = new ConcurrentHashSet<String>(new
> > TreeSet<String>());
> > + /**
> > + * The web response of the page to apply the caching information
> to
> > + */
> > + private WebResponse pageWebResponse;
> > +
> > + /**
> > + * The web request of the page to get the caching information
> from
> > + */
> > + private WebRequest pageWebRequest;
> > +
> > + /**
> > + * The page to get the modification time of
> > + */
> > + private Page page;
> > +
> > + /**
> > + * Creates a push header item based on the given page and the
> > corresponding page request / page
> > + * response. To get the request and response
> > + *
> > + *
> > + * @param page
> > + * the page this header item is applied to
> > + * @param pageRequest
> > + * the page request this header item is applied to
> > + * @param pageResponse
> > + * the page response this header item is applied to
> > + */
> > + public PushHeaderItem(Page page, Request pageRequest, Response
> > pageResponse)
> > + {
> > + if (page == null || !(page instanceof WebPage) ||
> > pageResponse == null ||
> > + !(pageResponse instanceof WebResponse))
> > + {
> > + throw new WicketRuntimeException(
> > + "Please hand over the web page, the web
> > response and the web request to the push header item like \"new
> > PushHeaderItem(this, yourWebPageReponse, yourWebPageRequest)\" - " +
> > + "The webPageResponse /
> > webPageRequest can be obtained via \"getRequestCycle().getResponse()\"
> > and placed into the page as field " +
> > + "\"private transient Response
> > webPageResponse;\" / \"private transient Request webPageRequest;\"");
> > + }
> > + this.pageWebResponse = (WebResponse)pageResponse;
> > + this.pageWebRequest = (WebRequest)pageRequest;
> > + this.page = page;
> > + }
> >
> > /**
> > * Uses the URLs that has already been pushed to the client to
> > ensure not to push them again
> > @@ -78,23 +137,102 @@ public class PushHeaderItem extends HeaderItem
> > }
> >
> > /**
> > + * Gets the time the page of this header item has been modified
> > + *
> > + * @return the time the page of this header item has been
> modified
> > + */
> > + private Time getPageModificationTime()
> > + {
> > + URL resource = page.getClass().getResource(
> > page.getClass().getSimpleName() + ".html");
> >
>
> What is the .class is modified but the markup is still the same ?
> What if the resource itself is modified ?
>
>
as of the jetty issue the decision if resources are going to be pushed
relies on the index page - this was my first thought about how we could
handle it. Are there any better solutions to find out if the page the push
header item is placed into has changed?
>
> > + if (resource == null)
> > + {
> > + throw new WicketRuntimeException(
> > + "The markup to the page couldn't be
> found:
> > " + page.getClass().getName());
> > + }
> > + try
> > + {
> > + return Time.valueOf(new
> > Date(resource.openConnection().getLastModified()));
> > + }
> > + catch (IOException e)
> > + {
> > + throw new WicketRuntimeException(
> > + "The time couln't be determined of the
> > markup file of the page: " +
> > + page.getClass().getName(),
> > + e);
> > + }
> > + }
> > +
> > + /**
> > + * Applies the cache header item to the response
> > + */
> > + private void applyPageCacheHeader()
> > + {
> > + Time pageModificationTime = getPageModificationTime();
> > + // check modification of page html
> > + pageWebResponse.setLastModifi
> edTime(pageModificationTime);
> > + pageWebResponse.setDateHeader("Expires",
> > pageModificationTime);
> > + pageWebResponse.setHeader("Cache-Control",
> > + "max-age=31536000, public, must-revalidate,
> > proxy-revalidate");
> > + pageWebResponse.setHeader("Pragma", "public");
> >
>
> This sets caching response headers for the whole page.
> Usually Wicket pages are stateful, thus it is better to not cache them.
> See org.apache.wicket.markup.html.WebPage#setHeaders()
> Now with this logic here it becomes harder for the application developer to
> find out why the browser stopped making requests to the page.
>
> (I haven't read the discussion at Jetty's issue tracker yet. It is in my
> mail queue.)
>
>
Yes that is right, but as previously mentioned the caching of the resources
relies on the index page. So if the index page has not been updated the
other resources are not going to be pushed. Because as of the push API it
only pushes, but not handles the cache for the resource files.
It behaves like this:
- When a browser requests the page with an initial commit everything is
going to be pushed with (200)
- When a browser requests the page a second time resources are not pushed
(304) not modified - I think in this case separate requests are made,
because of the actual ResourceReferences headers
- When a browser requests the page a second time and the markup of the page
has changed everything is going to be pushed again (200)
- When a browser requests the page a second time and resource references
has been changed but not the page markup, all changed resource references
are shipped via separate requests
Maybe we can find a way to handle the caching without modification of the
cache header?!
>
> > + }
> > +
> > + /**
> > * Pushes the previously created URLs to the client
> > */
> > @Override
> > public void render(Response response)
> > {
> > + // applies the caching header to the actual page request
> > + applyPageCacheHeader();
> > +
> > HttpServletRequest request = getContainerRequest(
> > RequestCycle.get().getRequest());
> > // Check if the protocol is http/2 or http/2.0 to only
> > push the resources in this case
> > if (isHttp2(request))
> > {
> > - // Receives the vendor specific push builder
> > - Http2Settings http2Settings =
> > Http2Settings.Holder.get(Application.get());
> > - PushBuilder pushBuilder =
> > http2Settings.getPushBuilder();
> > - pushBuilder.push(request, urls.toArray(new
> > String[urls.size()]));
> > + try
> > + {
> > + Time pageModificationTime =
> > getPageModificationTime();
> > + String ifModifiedSinceHeader =
> > pageWebRequest.getHeader("If-Modified-Since");
> > +
> > + if (ifModifiedSinceHeader != null)
> > + {
> > + Time
> > ifModifiedSinceFromRequestTime = Time
> > +
> .valueOf(headerDateFormat.
> > parse(ifModifiedSinceHeader));
> > + // If the client modification
> time
> > is before the page modification time -
> > + // don't push
> > + if (ifModifiedSinceFromRequestTim
> e
> > .before(pageModificationTime))
> > + {
> > + push(request);
> > + }
> > + }
> > + else
> > + {
> > +
> > + }
> > + }
> > + catch (ParseException e)
> > + {
> > + // If the If-Modified-Since time can't be
> > parsed - the push handling is going to be
> > + // skipped
> > + }
> > }
> > }
> >
> > /**
> > + * Pushed all URLs of this header item to the client
> > + *
> > + * @param request
> > + * the request to push the URLs to
> > + */
> > + private void push(HttpServletRequest request)
> > + {
> > + // Receives the vendor specific push builder
> > + Http2Settings http2Settings = Http2Settings.Holder.get(
> > Application.get());
> > + PushBuilder pushBuilder = http2Settings.getPushBuilder()
> ;
> > + pushBuilder.push(request, urls.toArray(new
> > String[urls.size()]));
> > + }
> > +
> > + /**
> > * Creates a URL and pushes the resource to the client - this is
> > only supported if http2 is
> > * enabled
> > *
> >
> > http://git-wip-us.apache.org/repos/asf/wicket/blob/
> > 76116aa4/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> > ----------------------------------------------------------------------
> > diff --git a/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> > b/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> > index d6888fd..59d53fe 100644
> > --- a/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> > +++ b/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> > @@ -21,12 +21,48 @@ For the setup you need to follow those steps:
> > </dependency>
> > {code}
> >
> > -3. Add the following PushHeaderItem to the renderHead method in your
> > component.
> > +3. Use the PushHeader Item like in this example page:
> > Example:
> > {code}
> > -TestResourceReference instance = TestResourceReference.getInstance();
> > -response.render(CssHeaderItem.forReference(instance));
> > -response.render(new PushHeaderItem().push(Arrays.asList(new
> > PushItem(instance))));
> > +public class HTTP2Page extends WebPage
> > +{
> > + private static final long serialVersionUID = 1L;
> > +
> > + private transient Response webPageResponse;
> > +
> > + private transient Request webPageRequest;
> >
>
> IMO this example is bad.
> #renderHead() is called for each request cycle.
> If the page is stateful then the chance for NullPointerException is high
> with those transient fields.
>
>
okay, then let us find a better example :-)
>
> > +
> > + public HTTP2Page()
> > + {
> > + webPageResponse = getRequestCycle().getResponse();
> > + webPageRequest = getRequestCycle().getRequest();
> > + add(new Label("label", "Label"));
> > + }
> > +
> > + @Override
> > + public void renderHead(IHeaderResponse response)
> > + {
> > + super.renderHead(response);
> > + TestResourceReference instance = TestResourceReference.
> > getInstance();
> > + response.render(CssHeaderItem.forReference(instance));
> > + response.render(new PushHeaderItem(this, webPageResponse,
> > webPageRequest)
> >
>
> Isn't it better if PushHeaderItem has overrideable methods like
> #getResponse() and #getRequest() which by default use
> RequestCycle.get().getXyz() ?
> This way the application developers will type less in most cases and
> override those methods only when really needed.
>
>
Can you help me with this? I was using RequestCycle.get(), but within the
ResourceReference it was not the RequestCycle of the Page which I used to
compare the modification date.
>
> > + .push(Arrays.asList(new PushItem(instance))));
> > + }
> > +
> > + @Override
> > + protected void setHeaders(WebResponse response)
> > + {
> > + // NOOP just disable caching
> > + }
> > +}
> > {code}
> >
> > Basically the resource is pushed before the actual response of the
> > component is send to the client (browser) and because of this the client
> > does not need to send an additional request.
> > +
> > +The PushHeaderItem behaves like explained in the following steps:
> > +* When a browser requests the page with an initial commit everything is
> > going to be pushed with (200)
> > +* When a browser requests the page a second time resources are not
> pushed
> > (304) not modified, because of the actual ResourceReferences headers
> > +* When a browser requests the page a second time and the markup of the
> > page has changed everything is going to be pushed again (200)
> > +* When a browser requests the page a second time and resource references
> > has been changed but not the page markup, all changed resource references
> > are shipped via separate requests
> > +
> > +Note: Chrome does not set cache headers if the https connection is not
> > secure (self signed) / valid - so ensure that a valid https connection is
> > available with your server.
> > \ No newline at end of file
> >
> >
>
Re: wicket git commit: WICKET-6194 PushHeaderItem handles client side cache
Posted by Tobias Soloschenko <to...@googlemail.com>.
Hi Martin,
2016-08-15 17:35 GMT+02:00 Martin Grigorov <mg...@apache.org>:
> Hi Tobias,
>
> On Thu, Aug 11, 2016 at 8:45 PM, <ts...@apache.org> wrote:
>
> > Repository: wicket
> > Updated Branches:
> > refs/heads/master 061c0e903 -> 76116aa46
> >
> >
> > WICKET-6194 PushHeaderItem handles client side cache
> >
> > Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/76116aa4
> > Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/76116aa4
> > Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/76116aa4
> >
> > Branch: refs/heads/master
> > Commit: 76116aa467590378ae6b5d5acc8a153246fc9ee4
> > Parents: 061c0e9
> > Author: Tobias Soloschenko <ts...@apache.org>
> > Authored: Thu Aug 11 20:43:52 2016 +0200
> > Committer: Tobias Soloschenko <ts...@apache.org>
> > Committed: Thu Aug 11 20:45:00 2016 +0200
> >
> > ----------------------------------------------------------------------
> > .../http2/markup/head/PushHeaderItem.java | 148
> ++++++++++++++++++-
> > .../src/docs/guide/http2push/http2push_1.gdoc | 44 +++++-
> > 2 files changed, 183 insertions(+), 9 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> > http://git-wip-us.apache.org/repos/asf/wicket/blob/
> > 76116aa4/wicket-experimental/wicket-http2/wicket-http2-
> > core/src/main/java/org/apache/wicket/http2/markup/head/
> PushHeaderItem.java
> > ----------------------------------------------------------------------
> > diff --git a/wicket-experimental/wicket-http2/wicket-http2-core/src/
> > main/java/org/apache/wicket/http2/markup/head/PushHeaderItem.java
> > b/wicket-experimental/wicket-http2/wicket-http2-core/src/
> > main/java/org/apache/wicket/http2/markup/head/PushHeaderItem.java
> > index 5e88a2f..2238c61 100644
> > --- a/wicket-experimental/wicket-http2/wicket-http2-core/src/
> > main/java/org/apache/wicket/http2/markup/head/PushHeaderItem.java
> > +++ b/wicket-experimental/wicket-http2/wicket-http2-core/src/
> > main/java/org/apache/wicket/http2/markup/head/PushHeaderItem.java
> > @@ -16,6 +16,11 @@
> > */
> > package org.apache.wicket.http2.markup.head;
> >
> > +import java.io.IOException;
> > +import java.net.URL;
> > +import java.text.ParseException;
> > +import java.text.SimpleDateFormat;
> > +import java.util.Date;
> > import java.util.List;
> > import java.util.Set;
> > import java.util.TreeSet;
> > @@ -27,19 +32,26 @@ import org.apache.wicket.Page;
> > import org.apache.wicket.WicketRuntimeException;
> > import org.apache.wicket.http2.Http2Settings;
> > import org.apache.wicket.markup.head.HeaderItem;
> > +import org.apache.wicket.markup.html.WebPage;
> > import org.apache.wicket.request.IRequestHandler;
> > import org.apache.wicket.request.Request;
> > import org.apache.wicket.request.Response;
> > import org.apache.wicket.request.Url;
> > import org.apache.wicket.request.cycle.RequestCycle;
> > +import org.apache.wicket.request.http.WebRequest;
> > +import org.apache.wicket.request.http.WebResponse;
> > import org.apache.wicket.request.mapper.parameter.PageParameters;
> > import org.apache.wicket.request.mapper.parameter.
> PageParametersEncoder;
> > import org.apache.wicket.request.resource.ResourceReference;
> > import org.apache.wicket.util.collections.ConcurrentHashSet;
> > +import org.apache.wicket.util.time.Time;
> >
> > /**
> > * A push header item to be used in the http/2 context and to reduce the
> > latency of the web
> > - * application
> > + * application. Follow these steps for your page:<br><br>
> > + * - Override the setHeaders method and don't call super.setHeaders to
> > disable caching<br>
> > + * - Get the page request / response and store them as transient fields
> > that are given into the PushHeaderItem<br>
> > + * - Ensure a valid https connection (not self signed), because
> otherwise
> > no caching information are accepted from Chrome or other browsers
> > *
> > * @author Tobias Soloschenko
> > *
> > @@ -49,6 +61,12 @@ public class PushHeaderItem extends HeaderItem
> > private static final long serialVersionUID = 1L;
> >
> > /**
> > + * The header date format for if-modified-since / last-modified
> > + */
> > + private static final SimpleDateFormat headerDateFormat = new
> > SimpleDateFormat(
> > + "EEE, dd MMM yyyy HH:mm:ss zzz");
> >
>
> SimpleDateFormat is not thread safe.
> Better use the new java.time.DateTimeFormatter
>
>
Okay, we should use this instead - right.
>
>
> > +
> > + /**
> > * The http2 protocol string
> > */
> > public static final String HTTP2_PROTOCOL = "http/2";
> > @@ -62,6 +80,47 @@ public class PushHeaderItem extends HeaderItem
> > * The URLs of resources to be pushed to the client
> > */
> > private Set<String> urls = new ConcurrentHashSet<String>(new
> > TreeSet<String>());
> > + /**
> > + * The web response of the page to apply the caching information
> to
> > + */
> > + private WebResponse pageWebResponse;
> > +
> > + /**
> > + * The web request of the page to get the caching information
> from
> > + */
> > + private WebRequest pageWebRequest;
> > +
> > + /**
> > + * The page to get the modification time of
> > + */
> > + private Page page;
> > +
> > + /**
> > + * Creates a push header item based on the given page and the
> > corresponding page request / page
> > + * response. To get the request and response
> > + *
> > + *
> > + * @param page
> > + * the page this header item is applied to
> > + * @param pageRequest
> > + * the page request this header item is applied to
> > + * @param pageResponse
> > + * the page response this header item is applied to
> > + */
> > + public PushHeaderItem(Page page, Request pageRequest, Response
> > pageResponse)
> > + {
> > + if (page == null || !(page instanceof WebPage) ||
> > pageResponse == null ||
> > + !(pageResponse instanceof WebResponse))
> > + {
> > + throw new WicketRuntimeException(
> > + "Please hand over the web page, the web
> > response and the web request to the push header item like \"new
> > PushHeaderItem(this, yourWebPageReponse, yourWebPageRequest)\" - " +
> > + "The webPageResponse /
> > webPageRequest can be obtained via \"getRequestCycle().getResponse()\"
> > and placed into the page as field " +
> > + "\"private transient Response
> > webPageResponse;\" / \"private transient Request webPageRequest;\"");
> > + }
> > + this.pageWebResponse = (WebResponse)pageResponse;
> > + this.pageWebRequest = (WebRequest)pageRequest;
> > + this.page = page;
> > + }
> >
> > /**
> > * Uses the URLs that has already been pushed to the client to
> > ensure not to push them again
> > @@ -78,23 +137,102 @@ public class PushHeaderItem extends HeaderItem
> > }
> >
> > /**
> > + * Gets the time the page of this header item has been modified
> > + *
> > + * @return the time the page of this header item has been
> modified
> > + */
> > + private Time getPageModificationTime()
> > + {
> > + URL resource = page.getClass().getResource(
> > page.getClass().getSimpleName() + ".html");
> >
>
> What is the .class is modified but the markup is still the same ?
> What if the resource itself is modified ?
>
>
as of the jetty issue the decision if resources are going to be pushed
relies on the index page - this was my first thought about how we could
handle it. Are there any better solutions to find out if the page the push
header item is placed into has changed?
>
> > + if (resource == null)
> > + {
> > + throw new WicketRuntimeException(
> > + "The markup to the page couldn't be
> found:
> > " + page.getClass().getName());
> > + }
> > + try
> > + {
> > + return Time.valueOf(new
> > Date(resource.openConnection().getLastModified()));
> > + }
> > + catch (IOException e)
> > + {
> > + throw new WicketRuntimeException(
> > + "The time couln't be determined of the
> > markup file of the page: " +
> > + page.getClass().getName(),
> > + e);
> > + }
> > + }
> > +
> > + /**
> > + * Applies the cache header item to the response
> > + */
> > + private void applyPageCacheHeader()
> > + {
> > + Time pageModificationTime = getPageModificationTime();
> > + // check modification of page html
> > + pageWebResponse.setLastModifiedTime(
> pageModificationTime);
> > + pageWebResponse.setDateHeader("Expires",
> > pageModificationTime);
> > + pageWebResponse.setHeader("Cache-Control",
> > + "max-age=31536000, public, must-revalidate,
> > proxy-revalidate");
> > + pageWebResponse.setHeader("Pragma", "public");
> >
>
> This sets caching response headers for the whole page.
> Usually Wicket pages are stateful, thus it is better to not cache them.
> See org.apache.wicket.markup.html.WebPage#setHeaders()
> Now with this logic here it becomes harder for the application developer to
> find out why the browser stopped making requests to the page.
>
> (I haven't read the discussion at Jetty's issue tracker yet. It is in my
> mail queue.)
>
>
Yes that is right, but as previously mentioned the caching of the resources
relies on the index page. So if the index page has not been updated the
other resources are not going to be pushed. Because as of the push API it
only pushes, but not handles the cache for the resource files.
It behaves like this:
- When a browser requests the page with an initial commit everything is
going to be pushed with (200)
- When a browser requests the page a second time resources are not pushed
(304) not modified - I think in this case separate requests are made,
because of the actual ResourceReferences headers
- When a browser requests the page a second time and the markup of the page
has changed everything is going to be pushed again (200)
- When a browser requests the page a second time and resource references
has been changed but not the page markup, all changed resource references
are shipped via separate requests
Maybe we can find a way to handle the caching without modification of the
cache header?!
>
> > + }
> > +
> > + /**
> > * Pushes the previously created URLs to the client
> > */
> > @Override
> > public void render(Response response)
> > {
> > + // applies the caching header to the actual page request
> > + applyPageCacheHeader();
> > +
> > HttpServletRequest request = getContainerRequest(
> > RequestCycle.get().getRequest());
> > // Check if the protocol is http/2 or http/2.0 to only
> > push the resources in this case
> > if (isHttp2(request))
> > {
> > - // Receives the vendor specific push builder
> > - Http2Settings http2Settings =
> > Http2Settings.Holder.get(Application.get());
> > - PushBuilder pushBuilder =
> > http2Settings.getPushBuilder();
> > - pushBuilder.push(request, urls.toArray(new
> > String[urls.size()]));
> > + try
> > + {
> > + Time pageModificationTime =
> > getPageModificationTime();
> > + String ifModifiedSinceHeader =
> > pageWebRequest.getHeader("If-Modified-Since");
> > +
> > + if (ifModifiedSinceHeader != null)
> > + {
> > + Time
> > ifModifiedSinceFromRequestTime = Time
> > +
> .valueOf(headerDateFormat.
> > parse(ifModifiedSinceHeader));
> > + // If the client modification
> time
> > is before the page modification time -
> > + // don't push
> > + if (
> ifModifiedSinceFromRequestTime
> > .before(pageModificationTime))
> > + {
> > + push(request);
> > + }
> > + }
> > + else
> > + {
> > +
> > + }
> > + }
> > + catch (ParseException e)
> > + {
> > + // If the If-Modified-Since time can't be
> > parsed - the push handling is going to be
> > + // skipped
> > + }
> > }
> > }
> >
> > /**
> > + * Pushed all URLs of this header item to the client
> > + *
> > + * @param request
> > + * the request to push the URLs to
> > + */
> > + private void push(HttpServletRequest request)
> > + {
> > + // Receives the vendor specific push builder
> > + Http2Settings http2Settings = Http2Settings.Holder.get(
> > Application.get());
> > + PushBuilder pushBuilder = http2Settings.getPushBuilder()
> ;
> > + pushBuilder.push(request, urls.toArray(new
> > String[urls.size()]));
> > + }
> > +
> > + /**
> > * Creates a URL and pushes the resource to the client - this is
> > only supported if http2 is
> > * enabled
> > *
> >
> > http://git-wip-us.apache.org/repos/asf/wicket/blob/
> > 76116aa4/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> > ----------------------------------------------------------------------
> > diff --git a/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> > b/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> > index d6888fd..59d53fe 100644
> > --- a/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> > +++ b/wicket-user-guide/src/docs/guide/http2push/http2push_1.gdoc
> > @@ -21,12 +21,48 @@ For the setup you need to follow those steps:
> > </dependency>
> > {code}
> >
> > -3. Add the following PushHeaderItem to the renderHead method in your
> > component.
> > +3. Use the PushHeader Item like in this example page:
> > Example:
> > {code}
> > -TestResourceReference instance = TestResourceReference.getInstance();
> > -response.render(CssHeaderItem.forReference(instance));
> > -response.render(new PushHeaderItem().push(Arrays.asList(new
> > PushItem(instance))));
> > +public class HTTP2Page extends WebPage
> > +{
> > + private static final long serialVersionUID = 1L;
> > +
> > + private transient Response webPageResponse;
> > +
> > + private transient Request webPageRequest;
> >
>
> IMO this example is bad.
> #renderHead() is called for each request cycle.
> If the page is stateful then the chance for NullPointerException is high
> with those transient fields.
>
>
okay, then let us find a better example :-)
>
> > +
> > + public HTTP2Page()
> > + {
> > + webPageResponse = getRequestCycle().getResponse();
> > + webPageRequest = getRequestCycle().getRequest();
> > + add(new Label("label", "Label"));
> > + }
> > +
> > + @Override
> > + public void renderHead(IHeaderResponse response)
> > + {
> > + super.renderHead(response);
> > + TestResourceReference instance = TestResourceReference.
> > getInstance();
> > + response.render(CssHeaderItem.forReference(instance));
> > + response.render(new PushHeaderItem(this, webPageResponse,
> > webPageRequest)
> >
>
> Isn't it better if PushHeaderItem has overrideable methods like
> #getResponse() and #getRequest() which by default use
> RequestCycle.get().getXyz() ?
> This way the application developers will type less in most cases and
> override those methods only when really needed.
>
>
Can you help me with this? I was using RequestCycle.get(), but within the
ResourceReference it was not the RequestCycle of the Page which I used to
compare the modification date.
>
> > + .push(Arrays.asList(new PushItem(instance))));
> > + }
> > +
> > + @Override
> > + protected void setHeaders(WebResponse response)
> > + {
> > + // NOOP just disable caching
> > + }
> > +}
> > {code}
> >
> > Basically the resource is pushed before the actual response of the
> > component is send to the client (browser) and because of this the client
> > does not need to send an additional request.
> > +
> > +The PushHeaderItem behaves like explained in the following steps:
> > +* When a browser requests the page with an initial commit everything is
> > going to be pushed with (200)
> > +* When a browser requests the page a second time resources are not
> pushed
> > (304) not modified, because of the actual ResourceReferences headers
> > +* When a browser requests the page a second time and the markup of the
> > page has changed everything is going to be pushed again (200)
> > +* When a browser requests the page a second time and resource references
> > has been changed but not the page markup, all changed resource references
> > are shipped via separate requests
> > +
> > +Note: Chrome does not set cache headers if the https connection is not
> > secure (self signed) / valid - so ensure that a valid https connection is
> > available with your server.
> > \ No newline at end of file
> >
> >
>