* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal