[pve-devel] [RFC PATCH 2/2] add ver get parameter for js/css files to improve cache behaviour

[Thomas Lamprecht]

On 08/09/2017 02:08 PM, Dominik Csapak wrote:
> we do not set an Expire header for our js files, and certain browsers
> do not even make the request then (on which we would answer
> with a correct 304 not modified)
> 
> so to force the browser to load a new version of the gui when we change
> something, we add the package version as a get parameter
> 
> when doing this, the browsers still cache the file, until the get
> parameter changes, which is exactly what we want
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/Service/pveproxy.pm | 5 +++++
>  www/index.html.tpl      | 6 +++---
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm
> index 0904127f..7d39900a 100755
> --- a/PVE/Service/pveproxy.pm
> +++ b/PVE/Service/pveproxy.pm
> @@ -17,6 +17,7 @@ use PVE::APIServer::Formatter::Standard;
>  use PVE::APIServer::Formatter::HTML;
>  use PVE::APIServer::AnyEvent;
>  use PVE::HTTPServer;
> +use PVE::pvecfg;
>  
>  use Template;
>  
> @@ -200,6 +201,9 @@ sub get_index {
>  	$langfile = 1;
>      }
>  
> +    my $ver = PVE::pvecfg::version();
> +    my $release = PVE::pvecfg::release();
> +
>      my $vars = {
>  	lang => $lang,
>  	langfile => $langfile,
> @@ -208,6 +212,7 @@ sub get_index {
>  	console => $args->{console},
>  	nodename => $nodename,
>  	debug => $server->{debug},
> +	version => "$ver-$release",
>      };
>  
>      # by default, load the normal index
> diff --git a/www/index.html.tpl b/www/index.html.tpl
> index 9abe09ef..6bbdea18 100644
> --- a/www/index.html.tpl
> +++ b/www/index.html.tpl
> @@ -10,9 +10,9 @@
>      <link rel="stylesheet" type="text/css" href="/pve2/ext6/theme-crisp/resources/theme-crisp-all.css" />
>      <link rel="stylesheet" type="text/css" href="/pve2/ext6/crisp/resources/charts-all.css" />
>      <link rel="stylesheet" type="text/css" href="/pve2/fa/css/font-awesome.css" />
> -    <link rel="stylesheet" type="text/css" href="/pve2/css/ext6-pve.css" />
> +    <link rel="stylesheet" type="text/css" href="/pve2/css/ext6-pve.css?ver=[% version %]" />
>      [% IF langfile %]
> -    <script type='text/javascript' src='/pve2/locale/pve-lang-[% lang %].js'></script>
> +    <script type='text/javascript' src='/pve2/locale/pve-lang-[% lang %].js?ver=[% version %]'></script>
>      [%- ELSE %]
>      <script type='text/javascript'>function gettext(buf) { return buf; }</script>
>      [% END %]
> @@ -23,7 +23,7 @@
>      <script type="text/javascript" src="/pve2/ext6/ext-all.js"></script>
>      <script type="text/javascript" src="/pve2/ext6/charts.js"></script>
>      [% END %]
> -    <script type="text/javascript" src="/pve2/js/pvemanagerlib.js"></script>
> +    <script type="text/javascript" src="/pve2/js/pvemanagerlib.js?ver=[% version %]"></script>
>      <script type="text/javascript" src="/pve2/ext6/locale/locale-[% lang %].js"></script>
>  
>      <script type="text/javascript">
> 

On 11/06/2017 01:01 PM, Dominik Csapak wrote:
> any comments? i believe this will vastly improve the cache behavior of the gui, so it i would be a shame if this does not get applied in one form or another
> 

Looks good to me, agree with you, would be good to have.

Reviewed-by: Thomas Lamprecht <t.lamprecht at proxmox.com>