* [pbs-devel] [PATCH proxmox-backup 0/6] improve caching behaviour for html resources
@ 2020-11-06 10:03 Dominik Csapak
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 1/6] proxmox-backup-proxy: remove unnecessary alias Dominik Csapak
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Dominik Csapak @ 2020-11-06 10:03 UTC (permalink / raw)
To: pbs-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 1/6] proxmox-backup-proxy: remove unnecessary alias
2020-11-06 10:03 [pbs-devel] [PATCH proxmox-backup 0/6] improve caching behaviour for html resources Dominik Csapak
@ 2020-11-06 10:03 ` Dominik Csapak
2020-11-06 18:05 ` [pbs-devel] applied: " Thomas Lamprecht
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 2/6] tools/fs: add helpers to get the mtime of a file Dominik Csapak
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Dominik Csapak @ 2020-11-06 10:03 UTC (permalink / raw)
To: pbs-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/6] tools/fs: add helpers to get the mtime of a file
2020-11-06 10:03 [pbs-devel] [PATCH proxmox-backup 0/6] improve caching behaviour for html resources Dominik Csapak
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 1/6] proxmox-backup-proxy: remove unnecessary alias Dominik Csapak
@ 2020-11-06 10:03 ` Dominik Csapak
2020-11-06 17:24 ` Thomas Lamprecht
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 3/6] server/rest: set last-modified for static files Dominik Csapak
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Dominik Csapak @ 2020-11-06 10:03 UTC (permalink / raw)
To: pbs-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 3/6] server/rest: set last-modified for static files
2020-11-06 10:03 [pbs-devel] [PATCH proxmox-backup 0/6] improve caching behaviour for html resources Dominik Csapak
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 1/6] proxmox-backup-proxy: remove unnecessary alias Dominik Csapak
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 2/6] tools/fs: add helpers to get the mtime of a file Dominik Csapak
@ 2020-11-06 10:03 ` Dominik Csapak
2020-11-06 13:14 ` Wolfgang Bumiller
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 4/6] server/config: add ability to get mtime of files for template Dominik Csapak
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Dominik Csapak @ 2020-11-06 10:03 UTC (permalink / raw)
To: pbs-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 4/6] server/config: add ability to get mtime of files for template
2020-11-06 10:03 [pbs-devel] [PATCH proxmox-backup 0/6] improve caching behaviour for html resources Dominik Csapak
` (2 preceding siblings ...)
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 3/6] server/rest: set last-modified for static files Dominik Csapak
@ 2020-11-06 10:03 ` Dominik Csapak
2020-11-06 13:22 ` Wolfgang Bumiller
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 5/6] proxmox-backup-proxy: add cache parameter to index Dominik Csapak
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 6/6] ui: set also extjs language Dominik Csapak
5 siblings, 1 reply; 11+ messages in thread
From: Dominik Csapak @ 2020-11-06 10:03 UTC (permalink / raw)
To: pbs-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 5/6] proxmox-backup-proxy: add cache parameter to index
2020-11-06 10:03 [pbs-devel] [PATCH proxmox-backup 0/6] improve caching behaviour for html resources Dominik Csapak
` (3 preceding siblings ...)
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 4/6] server/config: add ability to get mtime of files for template Dominik Csapak
@ 2020-11-06 10:03 ` Dominik Csapak
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 6/6] ui: set also extjs language Dominik Csapak
5 siblings, 0 replies; 11+ messages in thread
From: Dominik Csapak @ 2020-11-06 10:03 UTC (permalink / raw)
To: pbs-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 6/6] ui: set also extjs language
2020-11-06 10:03 [pbs-devel] [PATCH proxmox-backup 0/6] improve caching behaviour for html resources Dominik Csapak
` (4 preceding siblings ...)
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 5/6] proxmox-backup-proxy: add cache parameter to index Dominik Csapak
@ 2020-11-06 10:03 ` Dominik Csapak
5 siblings, 0 replies; 11+ messages in thread
From: Dominik Csapak @ 2020-11-06 10:03 UTC (permalink / raw)
To: pbs-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 3/6] server/rest: set last-modified for static files
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 3/6] server/rest: set last-modified for static files Dominik Csapak
@ 2020-11-06 13:14 ` Wolfgang Bumiller
0 siblings, 0 replies; 11+ messages in thread
From: Wolfgang Bumiller @ 2020-11-06 13:14 UTC (permalink / raw)
To: Dominik Csapak; +Cc: pbs-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 4/6] server/config: add ability to get mtime of files for template
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 4/6] server/config: add ability to get mtime of files for template Dominik Csapak
@ 2020-11-06 13:22 ` Wolfgang Bumiller
0 siblings, 0 replies; 11+ messages in thread
From: Wolfgang Bumiller @ 2020-11-06 13:22 UTC (permalink / raw)
To: Dominik Csapak; +Cc: pbs-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/6] tools/fs: add helpers to get the mtime of a file
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 2/6] tools/fs: add helpers to get the mtime of a file Dominik Csapak
@ 2020-11-06 17:24 ` Thomas Lamprecht
0 siblings, 0 replies; 11+ messages in thread
From: Thomas Lamprecht @ 2020-11-06 17:24 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pbs-devel] applied: [PATCH proxmox-backup 1/6] proxmox-backup-proxy: remove unnecessary alias
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 1/6] proxmox-backup-proxy: remove unnecessary alias Dominik Csapak
@ 2020-11-06 18:05 ` Thomas Lamprecht
0 siblings, 0 replies; 11+ messages in thread
From: Thomas Lamprecht @ 2020-11-06 18:05 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
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!
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-11-06 18:05 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-06 10:03 [pbs-devel] [PATCH proxmox-backup 0/6] improve caching behaviour for html resources Dominik Csapak
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 1/6] proxmox-backup-proxy: remove unnecessary alias Dominik Csapak
2020-11-06 18:05 ` [pbs-devel] applied: " Thomas Lamprecht
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 2/6] tools/fs: add helpers to get the mtime of a file Dominik Csapak
2020-11-06 17:24 ` Thomas Lamprecht
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 3/6] server/rest: set last-modified for static files Dominik Csapak
2020-11-06 13:14 ` Wolfgang Bumiller
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 4/6] server/config: add ability to get mtime of files for template Dominik Csapak
2020-11-06 13:22 ` Wolfgang Bumiller
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 5/6] proxmox-backup-proxy: add cache parameter to index Dominik Csapak
2020-11-06 10:03 ` [pbs-devel] [PATCH proxmox-backup 6/6] ui: set also extjs language Dominik Csapak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox