Re: [PATCH yew-comp 1/3] task (viewer): add task download button

[Maximiliano Sandoval <m.sandoval@proxmox.com>]

Dominik Csapak <d.csapak@proxmox.com> writes:

> similar to what we already have in the ExtJS gui. Currently the default
> is to construct a data URL from the downloaded json in the UI, since not
> all APIs support the 'download=1' parameter (and some can't be easily
> extended).
>
> The download api can be enabled with a setting though, so if at some
> point all consumers use that, we can drop the compatibility code path.
>
> Adds also a small helper to create a temporary 'a' element to click.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> Longer explanation which api calls do not support the 'download'
> parameter: on PDM, we tunnel the api call to the remote, but this is
> autogenerated code that expects json output. With the 'download'
> parameter, the content type gets set to text/plain, which confuses the
> client.
>
> To fix it, we'd either have to rewrite the auto-code generation to
> handle this case, or expose the underlying client and doit manually, or
> rewrite the remote task api call to to something similar what we do here
> in the ui, collecting json output and returning a final log txt file.
>
> Since having this download button is useful anyway, and implementing any
> of these solutions would take way longer than doing this here, I opted
> for doing that now, and fixing the backend later.
>
>  src/lib.rs         |  21 ++++++++
>  src/log_view.rs    |   7 +--
>  src/task_viewer.rs | 118 +++++++++++++++++++++++++++++++++++++++++----
>  src/tasks.rs       |   6 +++
>  4 files changed, 140 insertions(+), 12 deletions(-)
>
> diff --git a/src/lib.rs b/src/lib.rs
> index 62aa571..65dae68 100644
> --- a/src/lib.rs
> +++ b/src/lib.rs
> @@ -234,6 +234,8 @@ pub mod utils;
>  mod xtermjs;
>  pub use xtermjs::{ConsoleType, ProxmoxXTermJs, XTermJs};
>  
> +use anyhow::{format_err, Error};
> +
>  use pwt::gettext_noop;
>  use pwt::state::{LanguageInfo, TextDirection};
>  
> @@ -271,6 +273,25 @@ mod panic_wrapper {
>      }
>  }
>  
> +/// Helper to download the given 'source' url via a simulated click on a hidden 'a' element.
> +pub fn download_as_file(source: &str, file_name: &str) -> Result<(), Error> {
> +    let el = gloo_utils::document()
> +        .create_element("a")
> +        .map_err(|_| format_err!("unable to create element 'a'"))?;
> +    let html = el
> +        .dyn_into::<web_sys::HtmlElement>()

Is it possible a `use wasm_bindgen::JsCast;`

is missing in this function or file? This does not compile.


> +        .map_err(|_| format_err!("unable to cast to html element"))?;
> +    html.set_attribute("href", source)
> +        .map_err(|_| format_err!("unable to set href attribute"))?;
> +    html.set_attribute("target", "_blank")
> +        .map_err(|_| format_err!("unable to set target attribute"))?;
> +    html.set_attribute("download", file_name)
> +        .map_err(|_| format_err!("unable to set download attribute"))?;
> +    html.click();
> +    html.remove();
> +    Ok(())
> +}
> +
>  pub fn store_csrf_token(crsf_token: &str) {
>      if let Some(store) = pwt::state::session_storage() {
>          if store.set_item("CSRFToken", crsf_token).is_err() {
> diff --git a/src/log_view.rs b/src/log_view.rs
> index 20f8f77..46bc758 100644
> --- a/src/log_view.rs
> +++ b/src/log_view.rs
> @@ -37,10 +37,11 @@ const MAX_PHYSICAL: f64 = 17_000_000.0;
>  const DEFAULT_LINE_HEIGHT: u64 = 18;
>  
>  #[derive(Deserialize)]
> -struct LogEntry {
> +/// a single Log entry line
> +pub struct LogEntry {
>      #[allow(dead_code)]
> -    n: u64,
> -    t: String,
> +    pub n: u64,
> +    pub t: String,
>  }
>  
>  pub struct LogPage {
> diff --git a/src/task_viewer.rs b/src/task_viewer.rs
> index e78454e..decf721 100644
> --- a/src/task_viewer.rs
> +++ b/src/task_viewer.rs
> @@ -1,6 +1,6 @@
>  use std::rc::Rc;
>  
> -use serde_json::Value;
> +use serde_json::{json, Value};
>  
>  use gloo_timers::callback::Timeout;
>  
> @@ -9,15 +9,19 @@ use yew::prelude::*;
>  use yew::virtual_dom::{Key, VComp, VNode};
>  
>  use pwt::state::Loader;
> -use pwt::widget::{Button, Column, Dialog, TabBarItem, TabPanel, Toolbar};
> +use pwt::widget::{AlertDialog, Button, Column, Container, Dialog, TabBarItem, TabPanel, Toolbar};
>  use pwt::{prelude::*, AsyncPool};
>  
> +use crate::common_api_types::ProxmoxUpid;
> +use crate::log_view::LogEntry;
>  use crate::percent_encoding::percent_encode_component;
>  use crate::utils::{format_duration_human, format_upid, render_epoch};
> -use crate::{KVGrid, KVGridRow, LogView};
> +use crate::{download_as_file, KVGrid, KVGridRow, LogView};
>  
>  use pwt_macros::builder;
>  
> +use proxmox_time::epoch_to_rfc3339_utc;
> +
>  #[derive(Properties, PartialEq, Clone)]
>  #[builder]
>  pub struct TaskViewer {
> @@ -39,6 +43,12 @@ pub struct TaskViewer {
>      #[builder(IntoPropValue, into_prop_value)]
>      /// The base url for
>      pub base_url: AttrValue,
> +
> +    #[prop_or(false)]
> +    #[builder(IntoPropValue, into_prop_value)]
> +    /// Use the 'download' API parameter for downloading task logs.
> +    /// By default the task log will be downloaded as json, and converted to a data URL.
> +    pub download_api: bool,
>  }
>  
>  impl TaskViewer {
> @@ -55,6 +65,7 @@ pub enum Msg {
>      DataChange,
>      Reload,
>      StopTask,
> +    DownloadErr(Option<String>),
>  }
>  
>  pub struct PwtTaskViewer {
> @@ -63,6 +74,7 @@ pub struct PwtTaskViewer {
>      active: bool,
>      endtime: Option<i64>,
>      async_pool: AsyncPool,
> +    download_err: Option<String>,
>  }
>  
>  impl Component for PwtTaskViewer {
> @@ -99,6 +111,7 @@ impl Component for PwtTaskViewer {
>              active: props.endtime.is_none(),
>              endtime: props.endtime,
>              async_pool: AsyncPool::new(),
> +            download_err: None,
>          }
>      }
>  
> @@ -138,12 +151,23 @@ impl Component for PwtTaskViewer {
>                  }
>                  true
>              }
> +            Msg::DownloadErr(err) => {
> +                self.download_err = err;
> +                true
> +            }
>          }
>      }
>  
>      fn view(&self, ctx: &Context<Self>) -> Html {
>          let props = ctx.props();
>  
> +        if let Some(err) = &self.download_err {
> +            return AlertDialog::new(err)
> +                .title(tr!("Download Error"))
> +                .on_close(ctx.link().callback(|_| Msg::DownloadErr(None)))
> +                .into();
> +        }
> +
>          let panel = self.loader.render(|data| {
>              TabPanel::new()
>                  .class("pwt-flex-fit")
> @@ -273,18 +297,94 @@ impl PwtTaskViewer {
>          let active = self.active;
>          let link = ctx.link();
>  
> -        let toolbar = Toolbar::new().class("pwt-border-bottom").with_child(
> -            Button::new(tr!("Stop"))
> -                .disabled(!active)
> -                .onclick(link.callback(|_| Msg::StopTask)),
> -        );
> -
>          let url = format!(
>              "{}/{}/log",
>              props.base_url,
>              percent_encode_component(&task_id),
>          );
>  
> +        let filename = match props.task_id.parse::<ProxmoxUpid>() {
> +            Ok(upid) => {
> +                format!(
> +                    "task-{}-{}-{}.log",
> +                    upid.node,
> +                    upid.worker_type,
> +                    epoch_to_rfc3339_utc(upid.starttime).unwrap_or(upid.starttime.to_string())
> +                )
> +            }
> +            Err(_) => format!("task-{}.log", props.task_id),
> +        };
> +
> +        let download_btn: Html = if props.download_api {
> +            Container::from_tag("a")
> +                .attribute("disabled", active.then_some(""))
> +                .attribute("target", "_blank")
> +                .attribute("href", format!("/api2/extjs{url}?download=1"))
> +                .attribute("filename", filename)
> +                .with_child(
> +                    Button::new(tr!("Download"))
> +                        .disabled(active)
> +                        .icon_class("fa fa-download"),
> +                )
> +                .into()
> +        } else {
> +            Button::new(tr!("Download"))
> +                .icon_class("fa fa-download")
> +                .disabled(active)
> +                .on_activate({
> +                    let url = url.clone();
> +                    let link = link.clone();
> +                    let async_pool = self.async_pool.clone();
> +                    move |_| {
> +                        let url = url.clone();
> +                        let filename = filename.clone();
> +                        let link = link.clone();
> +                        async_pool.spawn(async move {
> +                            let param = json!({
> +                                "start": 0,
> +                                "limit": 0,
> +                            });
> +                            let resp =
> +                                crate::http_get_full::<Vec<LogEntry>>(url, Some(param)).await;
> +                            match resp {
> +                                Ok(resp) => {
> +                                    let string = resp
> +                                        .data
> +                                        .into_iter()
> +                                        .map(|entry| entry.t)
> +                                        .collect::<Vec<_>>()
> +                                        .join("\n");
> +
> +                                    if let Err(err) = download_as_file(
> +                                        &format!(
> +                                            "data:text/plain;charset=utf-8,{}",
> +                                            percent_encode_component(&string)
> +                                        ),
> +                                        &filename,
> +                                    ) {
> +                                        link.send_message(Msg::DownloadErr(Some(err.to_string())))
> +                                    }
> +                                }
> +                                Err(err) => {
> +                                    link.send_message(Msg::DownloadErr(Some(err.to_string())))
> +                                }
> +                            }
> +                        });
> +                    }
> +                })
> +                .into()
> +        };
> +
> +        let toolbar = Toolbar::new()
> +            .class("pwt-border-bottom")
> +            .with_child(
> +                Button::new(tr!("Stop"))
> +                    .disabled(!active)
> +                    .onclick(link.callback(|_| Msg::StopTask)),
> +            )
> +            .with_flex_spacer()
> +            .with_child(download_btn);
> +
>          Column::new()
>              .class("pwt-flex-fit")
>              .with_child(toolbar)
> diff --git a/src/tasks.rs b/src/tasks.rs
> index b49de98..fb3ed39 100644
> --- a/src/tasks.rs
> +++ b/src/tasks.rs
> @@ -78,6 +78,11 @@ pub struct Tasks {
>      #[prop_or_default]
>      /// An optional column configuration that overwrites the default one.
>      pub columns: Option<Rc<Vec<DataTableHeader<TaskListItem>>>>,
> +
> +    #[prop_or(false)]
> +    #[builder(IntoPropValue, into_prop_value)]
> +    /// Use the 'download' API parameter for downloading task logs.
> +    pub download_api: bool,
>  }
>  
>  impl Default for Tasks {
> @@ -493,6 +498,7 @@ impl LoadableComponent for ProxmoxTasks {
>          match view_state {
>              ViewDialog::TaskViewer => {
>                  let mut dialog = TaskViewer::new(&*selected_key)
> +                    .download_api(props.download_api)
>                      .endtime(selected_item.endtime)
>                      .on_close(ctx.link().change_view_callback(|_| None));
>                  if let Some(base_url) = &props.base_url {

-- 
Maximiliano