From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 42BC46671D for ; Fri, 6 Nov 2020 14:22:49 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 30538220E9 for ; Fri, 6 Nov 2020 14:22:19 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 6F4A1220DF for ; Fri, 6 Nov 2020 14:22:18 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 3454D46055 for ; Fri, 6 Nov 2020 14:22:18 +0100 (CET) Date: Fri, 6 Nov 2020 14:22:17 +0100 From: Wolfgang Bumiller To: Dominik Csapak Cc: pbs-devel@lists.proxmox.com Message-ID: <20201106132217.5onspcmr2kgxkuvc@olga.proxmox.com> References: <20201106100343.12161-1-d.csapak@proxmox.com> <20201106100343.12161-5-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201106100343.12161-5-d.csapak@proxmox.com> User-Agent: NeoMutt/20180716 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.010 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox-backup-proxy.rs, rest.rs, config.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup 4/6] server/config: add ability to get mtime of files for template X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Nov 2020 13:22:49 -0000 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 > --- > 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, > env_type: RpcEnvironmentType, > templates: RwLock>, > - template_files: RwLock>, > + template_files: RwLock)>>, ^ 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>>, > } > > @@ -76,7 +76,7 @@ impl ApiConfig { > self.env_type > } > > - pub fn register_template

(&self, name: &str, path: P) -> Result<(), Error> > + pub fn register_template

(&self, name: &str, path: P, files_to_check: &[&str]) -> Result<(), Error> > where > P: Into > { > @@ -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(&self, name: &str, data: &T) -> Result > - where > - T: Serialize, > + pub fn render_template(&self, name: &str, mut data: Value) -> Result > { > - let path; > + fn get_timestamps(files: &Vec) -> 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