[pbs-devel] [PATCH proxmox-backup 0/6] improve caching behaviour for html resources

[pbs-devel] [PATCH proxmox-backup 0/6] improve caching behaviour for html resources

[Dominik Csapak]

modeled after pve's caching:
* for static files, we set the 'last-modified' header
* for all js/css files in the index, we add a ?t= parameter that
  is the mtime of the file
  (this should get updated when the respective package gets updated)

note: the first patch is not really related, and can be applied individually

Dominik Csapak (6):
  proxmox-backup-proxy: remove unnecessary alias
  tools/fs: add helpers to get the mtime of a file
  server/rest: set last-modified for static files
  server/config: add ability to get mtime of files for template
  proxmox-backup-proxy: add cache parameter to index
  ui: set also extjs language

 src/bin/proxmox-backup-proxy.rs | 13 ++++++--
 src/server/config.rs            | 55 +++++++++++++++++++++++----------
 src/server/rest.rs              | 25 +++++++++++++--
 src/tools/fs.rs                 | 22 +++++++++++++
 www/index.hbs                   | 33 ++++++++++----------
 5 files changed, 109 insertions(+), 39 deletions(-)

-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 1/6] proxmox-backup-proxy: remove unnecessary alias

[Dominik Csapak]

the basedir is already /usr/share/javascript/proxmox-backup/
so adding a subdir of that as alias is not needed

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/bin/proxmox-backup-proxy.rs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 5803f024..3f07db89 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -90,7 +90,6 @@ async fn run() -> Result<(), Error> {
     config.add_alias("xtermjs", "/usr/share/pve-xtermjs");
     config.add_alias("locale", "/usr/share/pbs-i18n");
     config.add_alias("widgettoolkit", "/usr/share/javascript/proxmox-widget-toolkit");
-    config.add_alias("css", "/usr/share/javascript/proxmox-backup/css");
     config.add_alias("docs", "/usr/share/doc/proxmox-backup/html");
 
     let mut indexpath = PathBuf::from(buildcfg::JS_DIR);
-- 
2.20.1

[pbs-devel] applied: [PATCH proxmox-backup 1/6] proxmox-backup-proxy: remove unnecessary alias

[Thomas Lamprecht]

On 06.11.20 11:03, Dominik Csapak wrote:
> the basedir is already /usr/share/javascript/proxmox-backup/
> so adding a subdir of that as alias is not needed
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/bin/proxmox-backup-proxy.rs | 1 -
>  1 file changed, 1 deletion(-)
> 
>

applied, thanks!

[pbs-devel] [PATCH proxmox-backup 2/6] tools/fs: add helpers to get the mtime of a file

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/fs.rs | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/src/tools/fs.rs b/src/tools/fs.rs
index 72a530d8..d7ab934a 100644
--- a/src/tools/fs.rs
+++ b/src/tools/fs.rs
@@ -312,3 +312,25 @@ fn do_lock_dir_noblock(
 
     Ok(handle)
 }
+
+pub fn get_file_mtime<P: AsRef<std::path::Path>>(path: P) -> Result<i64, Error> {
+    let time = std::fs::metadata(path)?.modified()?;
+
+    let mtime: i64 = if time < std::time::SystemTime::UNIX_EPOCH {
+        -(std::time::SystemTime::UNIX_EPOCH.duration_since(time)?.as_secs() as i64)
+    } else {
+        time.duration_since(std::time::SystemTime::UNIX_EPOCH)?.as_secs() as i64
+    };
+    Ok(mtime)
+}
+
+pub async fn get_async_file_mtime<P: AsRef<std::path::Path>>(path: P) -> Result<i64, Error> {
+    let time = tokio::fs::metadata(path).await?.modified()?;
+
+    let mtime: i64 = if time < std::time::SystemTime::UNIX_EPOCH {
+        -(std::time::SystemTime::UNIX_EPOCH.duration_since(time)?.as_secs() as i64)
+    } else {
+        time.duration_since(std::time::SystemTime::UNIX_EPOCH)?.as_secs() as i64
+    };
+    Ok(mtime)
+}
-- 
2.20.1

Re: [pbs-devel] [PATCH proxmox-backup 2/6] tools/fs: add helpers to get the mtime of a file

[Thomas Lamprecht]

On 06.11.20 11:03, Dominik Csapak wrote:
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/tools/fs.rs | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/src/tools/fs.rs b/src/tools/fs.rs
> index 72a530d8..d7ab934a 100644
> --- a/src/tools/fs.rs
> +++ b/src/tools/fs.rs
> @@ -312,3 +312,25 @@ fn do_lock_dir_noblock(
>  
>      Ok(handle)
>  }
> +
> +pub fn get_file_mtime<P: AsRef<std::path::Path>>(path: P) -> Result<i64, Error> {
> +    let time = std::fs::metadata(path)?.modified()?;
> +
> +    let mtime: i64 = if time < std::time::SystemTime::UNIX_EPOCH {
> +        -(std::time::SystemTime::UNIX_EPOCH.duration_since(time)?.as_secs() as i64)
> +    } else {
> +        time.duration_since(std::time::SystemTime::UNIX_EPOCH)?.as_secs() as i64
> +    };
> +    Ok(mtime)
> +}
> +
> +pub async fn get_async_file_mtime<P: AsRef<std::path::Path>>(path: P) -> Result<i64, Error> {
> +    let time = tokio::fs::metadata(path).await?.modified()?;
> +
> +    let mtime: i64 = if time < std::time::SystemTime::UNIX_EPOCH {
> +        -(std::time::SystemTime::UNIX_EPOCH.duration_since(time)?.as_secs() as i64)
> +    } else {
> +        time.duration_since(std::time::SystemTime::UNIX_EPOCH)?.as_secs() as i64
> +    };
> +    Ok(mtime)
> +}
> 

wouldn't it be more sensible to implement just a system_time_to_i64 and use that
in combination with a "manual" modified call on use-site.
Avoids instancing multiple generics, and more flexible as one can have a file handle
or a path.

[pbs-devel] [PATCH proxmox-backup 3/6] server/rest: set last-modified for static files

[Dominik Csapak]

this way the browser can cache them

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/server/rest.rs | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/server/rest.rs b/src/server/rest.rs
index ea87c9c8..d0169ba5 100644
--- a/src/server/rest.rs
+++ b/src/server/rest.rs
@@ -492,10 +492,17 @@ async fn simple_static_file_download(filename: PathBuf) -> Result<Response<Body>
 
     use tokio::io::AsyncReadExt;
 
-    let mut file = File::open(filename)
+    let mut file = File::open(&filename)
         .await
         .map_err(|err| http_err!(BAD_REQUEST, "File open failed: {}", err))?;
 
+    let mtime = crate::tools::fs::get_async_file_mtime(filename)
+        .await
+        .map_err(|err| http_err!(INTERNAL_SERVER_ERROR, "File stat failed: {}", err))?;
+
+    let last_modified = proxmox::tools::time::strftime_utc("%a, %d %b %Y %T GMT", mtime)
+        .map_err(|err| http_err!(INTERNAL_SERVER_ERROR, "strftime failed: {}", err))?;
+
     let mut data: Vec<u8> = Vec::new();
     file.read_to_end(&mut data)
         .await
@@ -505,16 +512,27 @@ async fn simple_static_file_download(filename: PathBuf) -> Result<Response<Body>
     response.headers_mut().insert(
         header::CONTENT_TYPE,
         header::HeaderValue::from_static(content_type));
+    response.headers_mut().insert(
+        header::LAST_MODIFIED,
+        header::HeaderValue::from_str(&last_modified)?);
     Ok(response)
 }
 
 async fn chuncked_static_file_download(filename: PathBuf) -> Result<Response<Body>, Error> {
     let (content_type, _nocomp) = extension_to_content_type(&filename);
 
-    let file = File::open(filename)
+    let file = File::open(&filename)
         .await
         .map_err(|err| http_err!(BAD_REQUEST, "File open failed: {}", err))?;
 
+    let mtime = crate::tools::fs::get_async_file_mtime(filename)
+        .await
+        .map_err(|err| http_err!(INTERNAL_SERVER_ERROR, "File stat failed: {}", err))?;
+
+    let last_modified = proxmox::tools::time::strftime_utc("%a, %d %b %Y %T GMT", mtime)
+        .map_err(|err| http_err!(INTERNAL_SERVER_ERROR, "strftime failed: {}", err))?;
+
+
     let payload = tokio_util::codec::FramedRead::new(file, tokio_util::codec::BytesCodec::new())
         .map_ok(|bytes| hyper::body::Bytes::from(bytes.freeze()));
     let body = Body::wrap_stream(payload);
@@ -523,6 +541,7 @@ async fn chuncked_static_file_download(filename: PathBuf) -> Result<Response<Bod
     Ok(Response::builder()
        .status(StatusCode::OK)
        .header(header::CONTENT_TYPE, content_type)
+       .header(header::LAST_MODIFIED, &last_modified)
        .body(body)
        .unwrap()
     )
-- 
2.20.1

Re: [pbs-devel] [PATCH proxmox-backup 3/6] server/rest: set last-modified for static files

[Wolfgang Bumiller]

On Fri, Nov 06, 2020 at 11:03:40AM +0100, Dominik Csapak wrote:
> this way the browser can cache them
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/server/rest.rs | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/src/server/rest.rs b/src/server/rest.rs
> index ea87c9c8..d0169ba5 100644
> --- a/src/server/rest.rs
> +++ b/src/server/rest.rs
> @@ -492,10 +492,17 @@ async fn simple_static_file_download(filename: PathBuf) -> Result<Response<Body>
>  
>      use tokio::io::AsyncReadExt;
>  
> -    let mut file = File::open(filename)
> +    let mut file = File::open(&filename)
>          .await
>          .map_err(|err| http_err!(BAD_REQUEST, "File open failed: {}", err))?;
>  
> +    let mtime = crate::tools::fs::get_async_file_mtime(filename)

We already have an open file handle here, so please don't use the path
again. Split the helper in something to which you can pass either the
metadata you get from `file.metadata().await?`, or the file directly.

> +        .await
> +        .map_err(|err| http_err!(INTERNAL_SERVER_ERROR, "File stat failed: {}", err))?;
> +
> +    let last_modified = proxmox::tools::time::strftime_utc("%a, %d %b %Y %T GMT", mtime)
> +        .map_err(|err| http_err!(INTERNAL_SERVER_ERROR, "strftime failed: {}", err))?;
> +
>      let mut data: Vec<u8> = Vec::new();
>      file.read_to_end(&mut data)
>          .await
> @@ -505,16 +512,27 @@ async fn simple_static_file_download(filename: PathBuf) -> Result<Response<Body>
>      response.headers_mut().insert(
>          header::CONTENT_TYPE,
>          header::HeaderValue::from_static(content_type));
> +    response.headers_mut().insert(
> +        header::LAST_MODIFIED,
> +        header::HeaderValue::from_str(&last_modified)?);
>      Ok(response)
>  }
>  
>  async fn chuncked_static_file_download(filename: PathBuf) -> Result<Response<Body>, Error> {
>      let (content_type, _nocomp) = extension_to_content_type(&filename);
>  
> -    let file = File::open(filename)
> +    let file = File::open(&filename)
>          .await
>          .map_err(|err| http_err!(BAD_REQUEST, "File open failed: {}", err))?;
>  
> +    let mtime = crate::tools::fs::get_async_file_mtime(filename)

^ same

> +        .await
> +        .map_err(|err| http_err!(INTERNAL_SERVER_ERROR, "File stat failed: {}", err))?;
> +
> +    let last_modified = proxmox::tools::time::strftime_utc("%a, %d %b %Y %T GMT", mtime)
> +        .map_err(|err| http_err!(INTERNAL_SERVER_ERROR, "strftime failed: {}", err))?;
> +
> +
>      let payload = tokio_util::codec::FramedRead::new(file, tokio_util::codec::BytesCodec::new())
>          .map_ok(|bytes| hyper::body::Bytes::from(bytes.freeze()));
>      let body = Body::wrap_stream(payload);
> @@ -523,6 +541,7 @@ async fn chuncked_static_file_download(filename: PathBuf) -> Result<Response<Bod
>      Ok(Response::builder()
>         .status(StatusCode::OK)
>         .header(header::CONTENT_TYPE, content_type)
> +       .header(header::LAST_MODIFIED, &last_modified)
>         .body(body)
>         .unwrap()
>      )
> -- 
> 2.20.1

[pbs-devel] [PATCH proxmox-backup 4/6] server/config: add ability to get mtime of files for template

[Dominik Csapak]

this will help us to circumvent browser caching issues, by
giving the server config a list of files that it will stat for their mtime
which we can the use in the template to append a "GET" parameter
(this lets the browser refresh the resource if the parameter is different)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/bin/proxmox-backup-proxy.rs |  4 +--
 src/server/config.rs            | 55 +++++++++++++++++++++++----------
 src/server/rest.rs              |  2 +-
 3 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 3f07db89..d4425adc 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -94,8 +94,8 @@ async fn run() -> Result<(), Error> {
 
     let mut indexpath = PathBuf::from(buildcfg::JS_DIR);
     indexpath.push("index.hbs");
-    config.register_template("index", &indexpath)?;
-    config.register_template("console", "/usr/share/pve-xtermjs/index.html.hbs")?;
+    config.register_template("index", &indexpath, &[])?;
+    config.register_template("console", "/usr/share/pve-xtermjs/index.html.hbs", &[])?;
 
     let mut commando_sock = server::CommandoSocket::new(server::our_ctrl_sock());
 
diff --git a/src/server/config.rs b/src/server/config.rs
index c7300668..846725a3 100644
--- a/src/server/config.rs
+++ b/src/server/config.rs
@@ -7,12 +7,12 @@ use std::sync::{Arc, Mutex, RwLock};
 use anyhow::{bail, Error, format_err};
 use hyper::Method;
 use handlebars::Handlebars;
-use serde::Serialize;
+use serde_json::{Value, json};
 
 use proxmox::api::{ApiMethod, Router, RpcEnvironmentType};
 use proxmox::tools::fs::{create_path, CreateOptions};
 
-use crate::tools::{FileLogger, FileLogOptions};
+use crate::tools::{FileLogger, FileLogOptions, self};
 
 pub struct ApiConfig {
     basedir: PathBuf,
@@ -20,7 +20,7 @@ pub struct ApiConfig {
     aliases: HashMap<String, PathBuf>,
     env_type: RpcEnvironmentType,
     templates: RwLock<Handlebars<'static>>,
-    template_files: RwLock<HashMap<String, (SystemTime, PathBuf)>>,
+    template_files: RwLock<HashMap<String, (SystemTime, PathBuf, Vec<PathBuf>)>>,
     request_log: Option<Arc<Mutex<FileLogger>>>,
 }
 
@@ -76,7 +76,7 @@ impl ApiConfig {
         self.env_type
     }
 
-    pub fn register_template<P>(&self, name: &str, path: P) -> Result<(), Error>
+    pub fn register_template<P>(&self, name: &str, path: P, files_to_check: &[&str]) -> Result<(), Error>
     where
         P: Into<PathBuf>
     {
@@ -85,42 +85,63 @@ impl ApiConfig {
         }
 
         let path: PathBuf = path.into();
-        let metadata = metadata(&path)?;
-        let mtime = metadata.modified()?;
+        let mtime = metadata(&path)?.modified()?;
+
+        let mut files = Vec::new();
+
+        for file in files_to_check {
+            let (_, components) = crate::tools::normalize_uri_path(file)?;
+            let path = self.find_alias(&components);
+            if path.file_name().is_none() {
+                bail!("error registering template: has no filename: {:?}", file);
+            }
+            files.push(path);
+        }
 
         self.templates.write().unwrap().register_template_file(name, &path)?;
-        self.template_files.write().unwrap().insert(name.to_string(), (mtime, path));
+        self.template_files.write().unwrap().insert(name.to_string(), (mtime, path, files));
 
         Ok(())
     }
 
     /// Checks if the template was modified since the last rendering
     /// if yes, it loads a the new version of the template
-    pub fn render_template<T>(&self, name: &str, data: &T) -> Result<String, Error>
-    where
-        T: Serialize,
+    pub fn render_template(&self, name: &str, mut data: Value) -> Result<String, Error>
     {
-        let path;
+        fn get_timestamps(files: &Vec<PathBuf>) -> Value {
+            let mut value = json!({});
+
+            for file in files {
+                let mtime = tools::fs::get_file_mtime(file).unwrap_or_else(|_| 0);
+                let filename = file.file_name().unwrap().to_string_lossy();
+                value[filename.into_owned()] = Value::from(mtime);
+            }
+            value
+        }
+
         let mtime;
         {
             let template_files = self.template_files.read().unwrap();
-            let (old_mtime, old_path) = template_files.get(name).ok_or_else(|| format_err!("template not found"))?;
+            let (old_mtime, old_path, files) = template_files.get(name).ok_or_else(|| format_err!("template not found"))?;
 
             mtime = metadata(old_path)?.modified()?;
             if mtime <= *old_mtime {
-                return self.templates.read().unwrap().render(name, data).map_err(|err| format_err!("{}", err));
+                data["times".to_string()] = get_timestamps(files);
+                return self.templates.read().unwrap().render(name, &data).map_err(|err| format_err!("{}", err));
             }
-            path = old_path.to_path_buf();
         }
 
         {
             let mut template_files = self.template_files.write().unwrap();
             let mut templates = self.templates.write().unwrap();
 
-            templates.register_template_file(name, &path)?;
-            template_files.insert(name.to_string(), (mtime, path));
+            let (time, path, files) = template_files.get_mut(name).ok_or_else(|| format_err!("template not found"))?;
+
+            templates.register_template_file(name, path)?;
+            *time = mtime;
 
-            templates.render(name, data).map_err(|err| format_err!("{}", err))
+            data["times".to_string()] = get_timestamps(files);
+            templates.render(name, &data).map_err(|err| format_err!("{}", err))
         }
     }
 
diff --git a/src/server/rest.rs b/src/server/rest.rs
index d0169ba5..fa01522d 100644
--- a/src/server/rest.rs
+++ b/src/server/rest.rs
@@ -437,7 +437,7 @@ fn get_index(
         "debug": debug,
     });
 
-    let (ct, index) = match api.render_template(template_file, &data) {
+    let (ct, index) = match api.render_template(template_file, data) {
         Ok(index) => ("text/html", index),
         Err(err) => {
             ("text/plain", format!("Error rendering template: {}", err))
-- 
2.20.1

Re: [pbs-devel] [PATCH proxmox-backup 4/6] server/config: add ability to get mtime of files for template

[Wolfgang Bumiller]

On Fri, Nov 06, 2020 at 11:03:41AM +0100, Dominik Csapak wrote:
> this will help us to circumvent browser caching issues, by
> giving the server config a list of files that it will stat for their mtime
> which we can the use in the template to append a "GET" parameter
> (this lets the browser refresh the resource if the parameter is different)
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/bin/proxmox-backup-proxy.rs |  4 +--
>  src/server/config.rs            | 55 +++++++++++++++++++++++----------
>  src/server/rest.rs              |  2 +-
>  3 files changed, 41 insertions(+), 20 deletions(-)
> 
> diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
> index 3f07db89..d4425adc 100644
> --- a/src/bin/proxmox-backup-proxy.rs
> +++ b/src/bin/proxmox-backup-proxy.rs
> @@ -94,8 +94,8 @@ async fn run() -> Result<(), Error> {
>  
>      let mut indexpath = PathBuf::from(buildcfg::JS_DIR);
>      indexpath.push("index.hbs");
> -    config.register_template("index", &indexpath)?;
> -    config.register_template("console", "/usr/share/pve-xtermjs/index.html.hbs")?;
> +    config.register_template("index", &indexpath, &[])?;
> +    config.register_template("console", "/usr/share/pve-xtermjs/index.html.hbs", &[])?;
>  
>      let mut commando_sock = server::CommandoSocket::new(server::our_ctrl_sock());
>  
> diff --git a/src/server/config.rs b/src/server/config.rs
> index c7300668..846725a3 100644
> --- a/src/server/config.rs
> +++ b/src/server/config.rs
> @@ -7,12 +7,12 @@ use std::sync::{Arc, Mutex, RwLock};
>  use anyhow::{bail, Error, format_err};
>  use hyper::Method;
>  use handlebars::Handlebars;
> -use serde::Serialize;
> +use serde_json::{Value, json};
>  
>  use proxmox::api::{ApiMethod, Router, RpcEnvironmentType};
>  use proxmox::tools::fs::{create_path, CreateOptions};
>  
> -use crate::tools::{FileLogger, FileLogOptions};
> +use crate::tools::{FileLogger, FileLogOptions, self};
>  
>  pub struct ApiConfig {
>      basedir: PathBuf,
> @@ -20,7 +20,7 @@ pub struct ApiConfig {
>      aliases: HashMap<String, PathBuf>,
>      env_type: RpcEnvironmentType,
>      templates: RwLock<Handlebars<'static>>,
> -    template_files: RwLock<HashMap<String, (SystemTime, PathBuf)>>,
> +    template_files: RwLock<HashMap<String, (SystemTime, PathBuf, Vec<PathBuf>)>>,

^ not very self explanatory as a tuple. If this were a struct you could
also move the 'get_timestamps' helper from inside `render_template` to
the struct as a method.

>      request_log: Option<Arc<Mutex<FileLogger>>>,
>  }
>  
> @@ -76,7 +76,7 @@ impl ApiConfig {
>          self.env_type
>      }
>  
> -    pub fn register_template<P>(&self, name: &str, path: P) -> Result<(), Error>
> +    pub fn register_template<P>(&self, name: &str, path: P, files_to_check: &[&str]) -> Result<(), Error>
>      where
>          P: Into<PathBuf>
>      {
> @@ -85,42 +85,63 @@ impl ApiConfig {
>          }
>  
>          let path: PathBuf = path.into();
> -        let metadata = metadata(&path)?;
> -        let mtime = metadata.modified()?;
> +        let mtime = metadata(&path)?.modified()?;
> +
> +        let mut files = Vec::new();
> +
> +        for file in files_to_check {
> +            let (_, components) = crate::tools::normalize_uri_path(file)?;
> +            let path = self.find_alias(&components);
> +            if path.file_name().is_none() {
> +                bail!("error registering template: has no filename: {:?}", file);
> +            }
> +            files.push(path);
> +        }
>  
>          self.templates.write().unwrap().register_template_file(name, &path)?;
> -        self.template_files.write().unwrap().insert(name.to_string(), (mtime, path));
> +        self.template_files.write().unwrap().insert(name.to_string(), (mtime, path, files));
>  
>          Ok(())
>      }
>  
>      /// Checks if the template was modified since the last rendering
>      /// if yes, it loads a the new version of the template
> -    pub fn render_template<T>(&self, name: &str, data: &T) -> Result<String, Error>
> -    where
> -        T: Serialize,
> +    pub fn render_template(&self, name: &str, mut data: Value) -> Result<String, Error>
>      {
> -        let path;
> +        fn get_timestamps(files: &Vec<PathBuf>) -> Value {
> +            let mut value = json!({});
> +
> +            for file in files {
> +                let mtime = tools::fs::get_file_mtime(file).unwrap_or_else(|_| 0);

This could just use `.unwrap_or(0)` ;-)

> +                let filename = file.file_name().unwrap().to_string_lossy();
> +                value[filename.into_owned()] = Value::from(mtime);
> +            }
> +            value
> +        }
> +
>          let mtime;
>          {
>              let template_files = self.template_files.read().unwrap();
> -            let (old_mtime, old_path) = template_files.get(name).ok_or_else(|| format_err!("template not found"))?;
> +            let (old_mtime, old_path, files) = template_files.get(name).ok_or_else(|| format_err!("template not found"))?;
>  
>              mtime = metadata(old_path)?.modified()?;
>              if mtime <= *old_mtime {
> -                return self.templates.read().unwrap().render(name, data).map_err(|err| format_err!("{}", err));
> +                data["times".to_string()] = get_timestamps(files);
> +                return self.templates.read().unwrap().render(name, &data).map_err(|err| format_err!("{}", err));

while you're at it - can this not just be `.map_err(Error::from)`?
Alternatively without mapping the error you can `Ok`-wrap it with a `?`
operator: `return Ok(foo.render()?);` that implies the `error.into()`.

>              }
> -            path = old_path.to_path_buf();
>          }
>  
>          {
>              let mut template_files = self.template_files.write().unwrap();
>              let mut templates = self.templates.write().unwrap();
>  
> -            templates.register_template_file(name, &path)?;
> -            template_files.insert(name.to_string(), (mtime, path));
> +            let (time, path, files) = template_files.get_mut(name).ok_or_else(|| format_err!("template not found"))?;
> +
> +            templates.register_template_file(name, path)?;
> +            *time = mtime;
>  
> -            templates.render(name, data).map_err(|err| format_err!("{}", err))
> +            data["times".to_string()] = get_timestamps(files);
> +            templates.render(name, &data).map_err(|err| format_err!("{}", err))

Same ^

>          }
>      }
>  
> diff --git a/src/server/rest.rs b/src/server/rest.rs
> index d0169ba5..fa01522d 100644
> --- a/src/server/rest.rs
> +++ b/src/server/rest.rs
> @@ -437,7 +437,7 @@ fn get_index(
>          "debug": debug,
>      });
>  
> -    let (ct, index) = match api.render_template(template_file, &data) {
> +    let (ct, index) = match api.render_template(template_file, data) {
>          Ok(index) => ("text/html", index),
>          Err(err) => {
>              ("text/plain", format!("Error rendering template: {}", err))
> -- 
> 2.20.1

[pbs-devel] [PATCH proxmox-backup 5/6] proxmox-backup-proxy: add cache parameter to index

[Dominik Csapak]

for efficiency, use only one mtime value per debian package
(this should change when a new version gets installed)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/bin/proxmox-backup-proxy.rs | 10 +++++++++-
 www/index.hbs                   | 26 +++++++++++++-------------
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index d4425adc..336eacd1 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -94,7 +94,15 @@ async fn run() -> Result<(), Error> {
 
     let mut indexpath = PathBuf::from(buildcfg::JS_DIR);
     indexpath.push("index.hbs");
-    config.register_template("index", &indexpath, &[])?;
+
+    // TODO: get filenames automatically from template?
+    config.register_template("index", &indexpath, &[
+        "/js/proxmox-backup-gui.js",
+        "/widgettoolkit/proxmoxlib.js",
+        "/extjs/ext-all.js",
+        "/fontawesome/css/font-awesome.css",
+        "/locale/pbs-lang-de.js", // we have to choose a file that we know exists
+    ])?;
     config.register_template("console", "/usr/share/pve-xtermjs/index.html.hbs", &[])?;
 
     let mut commando_sock = server::CommandoSocket::new(server::our_ctrl_sock());
diff --git a/www/index.hbs b/www/index.hbs
index 008e2410..93df4fc0 100644
--- a/www/index.hbs
+++ b/www/index.hbs
@@ -7,22 +7,22 @@
     <title>{{ NodeName }} - Proxmox Backup Server</title>
     <link rel="icon" sizes="128x128" href="/images/logo-128.png" />
     <link rel="apple-touch-icon" sizes="128x128" href="/pve2/images/logo-128.png" />
-    <link rel="stylesheet" type="text/css" href="/extjs/theme-crisp/resources/theme-crisp-all.css" />
-    <link rel="stylesheet" type="text/css" href="/extjs/crisp/resources/charts-all.css" />
-    <link rel="stylesheet" type="text/css" href="/fontawesome/css/font-awesome.css" />
-    <link rel="stylesheet" type="text/css" href="/widgettoolkit/css/ext6-pmx.css" />
-    <link rel="stylesheet" type="text/css" href="/css/ext6-pbs.css" />
+    <link rel="stylesheet" type="text/css" href="/extjs/theme-crisp/resources/theme-crisp-all.css?t={{lookup times "ext-all.js"}}" />
+    <link rel="stylesheet" type="text/css" href="/extjs/crisp/resources/charts-all.css?t={{lookup times "ext-all.js"}}" />
+    <link rel="stylesheet" type="text/css" href="/fontawesome/css/font-awesome.css?t={{lookup times "font-awesome.css"}}" />
+    <link rel="stylesheet" type="text/css" href="/widgettoolkit/css/ext6-pmx.css?t={{lookup times "proxmoxlib.js"}}" />
+    <link rel="stylesheet" type="text/css" href="/css/ext6-pbs.css?t={{lookup times "proxmox-backup-gui.js"}}" />
     {{#if language}}
-    <script type='text/javascript' src='/locale/pbs-lang-{{ language }}.js'></script>
+	<script type='text/javascript' src='/locale/pbs-lang-{{ language }}.js?t={{lookup times "pbs-lang-de.js"}}'></script>
     {{else}}
     <script type='text/javascript'> function gettext(buf) { return buf; } </script>
     {{/if}}
     {{#if debug}}
-    <script type="text/javascript" src="/extjs/ext-all-debug.js"></script>
-    <script type="text/javascript" src="/extjs/charts-debug.js"></script>
+    <script type="text/javascript" src="/extjs/ext-all-debug.js?t={{lookup times "ext-all.js"}}"></script>
+    <script type="text/javascript" src="/extjs/charts-debug.js?t={{lookup times "ext-all.js"}}"></script>
     {{else}}
-    <script type="text/javascript" src="/extjs/ext-all.js"></script>
-    <script type="text/javascript" src="/extjs/charts.js"></script>
+    <script type="text/javascript" src="/extjs/ext-all.js?t={{lookup times "ext-all.js"}}"></script>
+    <script type="text/javascript" src="/extjs/charts.js?t={{lookup times "ext-all.js"}}"></script>
     {{/if}}
     <script type="text/javascript">
     Proxmox = {
@@ -32,12 +32,12 @@
 	CSRFPreventionToken: "{{ CSRFPreventionToken }}",
     };
     </script>
-    <script type="text/javascript" src="/widgettoolkit/proxmoxlib.js"></script>
-    <script type="text/javascript" src="/extjs/locale/locale-en.js"></script>
+    <script type="text/javascript" src="/widgettoolkit/proxmoxlib.js?t={{lookup times "proxmoxlib.js"}}"></script>
+    <script type="text/javascript" src="/extjs/locale/locale-en.js?t={{lookup times "ext-all.js"}}"></script>
     <script type="text/javascript">
       Ext.History.fieldid = 'x-history-field';
     </script>
-    <script type="text/javascript" src="/js/proxmox-backup-gui.js"></script>
+      <script type="text/javascript" src="/js/proxmox-backup-gui.js?t={{lookup times "proxmox-backup-gui.js"}}"></script>
   </head>
   <body>
     <!-- Fields required for history management -->
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 6/6] ui: set also extjs language

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/index.hbs | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/www/index.hbs b/www/index.hbs
index 93df4fc0..346e000c 100644
--- a/www/index.hbs
+++ b/www/index.hbs
@@ -12,11 +12,6 @@
     <link rel="stylesheet" type="text/css" href="/fontawesome/css/font-awesome.css?t={{lookup times "font-awesome.css"}}" />
     <link rel="stylesheet" type="text/css" href="/widgettoolkit/css/ext6-pmx.css?t={{lookup times "proxmoxlib.js"}}" />
     <link rel="stylesheet" type="text/css" href="/css/ext6-pbs.css?t={{lookup times "proxmox-backup-gui.js"}}" />
-    {{#if language}}
-	<script type='text/javascript' src='/locale/pbs-lang-{{ language }}.js?t={{lookup times "pbs-lang-de.js"}}'></script>
-    {{else}}
-    <script type='text/javascript'> function gettext(buf) { return buf; } </script>
-    {{/if}}
     {{#if debug}}
     <script type="text/javascript" src="/extjs/ext-all-debug.js?t={{lookup times "ext-all.js"}}"></script>
     <script type="text/javascript" src="/extjs/charts-debug.js?t={{lookup times "ext-all.js"}}"></script>
@@ -24,6 +19,13 @@
     <script type="text/javascript" src="/extjs/ext-all.js?t={{lookup times "ext-all.js"}}"></script>
     <script type="text/javascript" src="/extjs/charts.js?t={{lookup times "ext-all.js"}}"></script>
     {{/if}}
+    {{#if language}}
+    <script type='text/javascript' src='/locale/pbs-lang-{{ language }}.js?t={{lookup times "pbs-lang-de.js"}}'></script>
+    <script type="text/javascript" src="/extjs/locale/locale-{{ language }}.js?t={{lookup times "ext-all.js"}}"></script>
+    {{else}}
+    <script type='text/javascript'> function gettext(buf) { return buf; } </script>
+    <script type="text/javascript" src="/extjs/locale/locale-en.js?t={{lookup times "ext-all.js"}}"></script>
+    {{/if}}
     <script type="text/javascript">
     Proxmox = {
 	Setup: { auth_cookie_name: 'PBSAuthCookie' },
@@ -33,7 +35,6 @@
     };
     </script>
     <script type="text/javascript" src="/widgettoolkit/proxmoxlib.js?t={{lookup times "proxmoxlib.js"}}"></script>
-    <script type="text/javascript" src="/extjs/locale/locale-en.js?t={{lookup times "ext-all.js"}}"></script>
     <script type="text/javascript">
       Ext.History.fieldid = 'x-history-field';
     </script>
-- 
2.20.1