Skip to content

Commit 69e6759

Browse files
authored
chore(web): direct type usage in templates instead of copy (#1218)
## Description In many cases in Web we create intermediate data structure and copy data there without any necessity. It is simpler just use filters and RPC types. This PR removes some such places. ## Type of Change - [ ] **Add** - New feature or capability - [ ] **Change** - Changes in existing functionality - [ ] **Fix** - Bug fixes - [ ] **Remove** - Removed features or deprecated functionality - [x] **Internal** - Internal changes (refactoring, tests, docs, etc.) ## Related Issues (Optional) ## Breaking Changes - [ ] This PR contains breaking changes ## Testing - [ ] Unit tests added/updated - [ ] Integration tests added/updated - [ ] Manual testing performed - [x] No testing required (docs, internal refactor, etc.) ## Additional Notes Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
1 parent ea8dc3d commit 69e6759

15 files changed

Lines changed: 100 additions & 260 deletions

crates/api/src/web/domain.rs

Lines changed: 5 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -24,35 +24,13 @@ use axum::response::{Html, IntoResponse, Response};
2424
use hyper::http::StatusCode;
2525
use rpc::forge::forge_server::Forge;
2626

27+
use super::filters;
2728
use crate::api::Api;
2829

2930
#[derive(Template)]
3031
#[template(path = "domain_show.html")]
3132
struct DomainShow {
32-
domains: Vec<DomainRowDisplay>,
33-
}
34-
35-
struct DomainRowDisplay {
36-
id: String,
37-
name: String,
38-
created: String,
39-
updated: String,
40-
deleted: String,
41-
}
42-
43-
impl From<::rpc::protos::dns::Domain> for DomainRowDisplay {
44-
fn from(d: ::rpc::protos::dns::Domain) -> Self {
45-
Self {
46-
id: d.id.unwrap_or_default().to_string(),
47-
name: d.name,
48-
created: d.created.unwrap_or_default().to_string(),
49-
updated: d.updated.unwrap_or_default().to_string(),
50-
deleted: d
51-
.deleted
52-
.map(|x| x.to_string())
53-
.unwrap_or("Not Deleted".to_string()),
54-
}
55-
}
33+
domains: Vec<::rpc::protos::dns::Domain>,
5634
}
5735

5836
/// List domains
@@ -65,15 +43,11 @@ pub async fn show_html(AxumState(state): AxumState<Arc<Api>>) -> Response {
6543
}
6644
};
6745

68-
let mut out = Vec::new();
69-
for domain in domains.domains.into_iter() {
70-
out.push(domain.into());
71-
}
72-
73-
let tmpl = DomainShow { domains: out };
46+
let tmpl = DomainShow {
47+
domains: domains.domains,
48+
};
7449
(StatusCode::OK, Html(tmpl.render().unwrap())).into_response()
7550
}
76-
7751
pub async fn show_all_json(AxumState(state): AxumState<Arc<Api>>) -> Response {
7852
let domains = match fetch_domains(state).await {
7953
Ok(m) => m,

crates/api/src/web/dpa.rs

Lines changed: 6 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -26,33 +26,14 @@ use rpc::forge as forgerpc;
2626
use rpc::forge::forge_server::Forge;
2727
use uuid::Uuid;
2828

29+
use super::filters;
2930
use super::state_history::StateHistoryTable;
3031
use crate::api::Api;
3132

3233
#[derive(Template)]
3334
#[template(path = "dpa_show.html")]
3435
struct DpaShow {
35-
dpas: Vec<DpaRowDisplay>,
36-
}
37-
38-
struct DpaRowDisplay {
39-
id: String,
40-
machine_id: String,
41-
state: String,
42-
created: String,
43-
macaddr: String,
44-
}
45-
46-
impl From<forgerpc::DpaInterface> for DpaRowDisplay {
47-
fn from(dpa: forgerpc::DpaInterface) -> Self {
48-
Self {
49-
id: dpa.id.map(|i| i.to_string()).unwrap_or_default(),
50-
machine_id: dpa.machine_id.map(|i| i.to_string()).unwrap_or_default(),
51-
created: dpa.created.map(|c| c.to_string()).unwrap_or_default(),
52-
macaddr: dpa.mac_addr,
53-
state: dpa.controller_state,
54-
}
55-
}
36+
dpas: Vec<forgerpc::DpaInterface>,
5637
}
5738

5839
/// List DPAs
@@ -65,9 +46,7 @@ pub async fn show_dpas_html(AxumState(state): AxumState<Arc<Api>>) -> Response {
6546
}
6647
};
6748

68-
let tmpl = DpaShow {
69-
dpas: dpas.into_iter().map(Into::into).collect(),
70-
};
49+
let tmpl = DpaShow { dpas };
7150
(StatusCode::OK, Html(tmpl.render().unwrap())).into_response()
7251
}
7352

@@ -118,50 +97,17 @@ async fn fetch_dpas(api: Arc<Api>) -> Result<Vec<forgerpc::DpaInterface>, tonic:
11897
#[derive(Template)]
11998
#[template(path = "dpa_detail.html")]
12099
struct DpaDetail {
121-
id: String,
122-
machine_id: String,
123-
macaddr: String,
124-
created: String,
125-
updated: String,
126-
deleted: String,
127-
underlay_ip: String,
128-
overlay_ip: String,
129-
state: String,
130-
state_version: String,
131-
network_config: String,
132-
network_config_version: String,
133-
network_status_observation: String,
100+
dpa: forgerpc::DpaInterface,
134101
history: StateHistoryTable,
135102
}
136103

137104
impl From<forgerpc::DpaInterface> for DpaDetail {
138105
fn from(dpa: forgerpc::DpaInterface) -> Self {
139-
let mut history_records = Vec::new();
140-
141-
for record in dpa.history.into_iter().rev() {
142-
history_records.push(record.into());
143-
}
144-
145106
let history = StateHistoryTable {
146-
records: history_records,
107+
records: dpa.history.iter().rev().cloned().map(Into::into).collect(),
147108
};
148109

149-
Self {
150-
id: dpa.id.map(|i| i.to_string()).unwrap_or_default(),
151-
machine_id: dpa.machine_id.map(|i| i.to_string()).unwrap_or_default(),
152-
macaddr: dpa.mac_addr,
153-
created: dpa.created.map(|c| c.to_string()).unwrap_or_default(),
154-
updated: dpa.updated.map(|c| c.to_string()).unwrap_or_default(),
155-
deleted: dpa.deleted.map(|c| c.to_string()).unwrap_or_default(),
156-
underlay_ip: dpa.underlay_ip,
157-
overlay_ip: dpa.overlay_ip,
158-
state: dpa.controller_state,
159-
state_version: dpa.controller_state_version,
160-
network_config: dpa.network_config,
161-
network_config_version: dpa.network_config_version,
162-
network_status_observation: dpa.network_status_observation,
163-
history,
164-
}
110+
Self { dpa, history }
165111
}
166112
}
167113

crates/api/src/web/filters.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,13 @@ pub fn option_fmt(value: &Option<impl Display>) -> askama::Result<String> {
222222
})
223223
}
224224

225+
pub fn option_fmt_or(value: &Option<impl Display>, default: &str) -> askama::Result<String> {
226+
Ok(match value {
227+
Some(value) => value.to_string(),
228+
None => default.to_string(),
229+
})
230+
}
231+
225232
pub(crate) fn normalize_state_label(state: impl Display) -> String {
226233
state_and_substate_labels(state).0
227234
}

crates/api/src/web/ib_fabric.rs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,7 @@ use crate::api::Api;
3030
#[derive(Template)]
3131
#[template(path = "ib_fabric_show.html")]
3232
struct IbFabricShow {
33-
fabrics: Vec<IbFabricRowDisplay>,
34-
}
35-
36-
struct IbFabricRowDisplay {
37-
id: String,
33+
fabrics: Vec<String>,
3834
}
3935

4036
/// List fabrics
@@ -51,12 +47,7 @@ pub async fn show_html(AxumState(state): AxumState<Arc<Api>>) -> Response {
5147
}
5248
};
5349

54-
let tmpl = IbFabricShow {
55-
fabrics: fabrics
56-
.into_iter()
57-
.map(|id| IbFabricRowDisplay { id })
58-
.collect(),
59-
};
50+
let tmpl = IbFabricShow { fabrics };
6051
(StatusCode::OK, Html(tmpl.render().unwrap())).into_response()
6152
}
6253

crates/api/src/web/ipxe_template.rs

Lines changed: 19 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
* limitations under the License.
1616
*/
1717

18+
use std::borrow::Cow;
1819
use std::sync::Arc;
1920

2021
use askama::Template;
@@ -25,37 +26,26 @@ use hyper::http::StatusCode;
2526
use rpc::forge as forgerpc;
2627
use rpc::forge::forge_server::Forge;
2728

29+
fn ipxe_template_scope_fmt(scope: &i32) -> Cow<'static, str> {
30+
rpc::forge::IpxeTemplateScope::try_from(*scope)
31+
.map(|scope| Cow::Owned(format!("{scope:?}")))
32+
.unwrap_or(Cow::Borrowed("Unknown"))
33+
}
34+
35+
mod filters {
36+
pub use super::super::filters::option_fmt;
37+
38+
pub fn ipxe_template_scope_fmt(scope: &i32) -> askama::Result<super::Cow<'static, str>> {
39+
Ok(super::ipxe_template_scope_fmt(scope))
40+
}
41+
}
42+
2843
use crate::api::Api;
2944

3045
#[derive(Template)]
3146
#[template(path = "ipxe_template_show.html")]
3247
struct IpxeTemplateShow {
33-
templates: Vec<IpxeTemplateRowDisplay>,
34-
}
35-
36-
struct IpxeTemplateRowDisplay {
37-
id: String,
38-
name: String,
39-
description: String,
40-
scope: String,
41-
required_params_count: usize,
42-
required_artifacts_count: usize,
43-
}
44-
45-
impl From<&forgerpc::IpxeTemplate> for IpxeTemplateRowDisplay {
46-
fn from(tmpl: &forgerpc::IpxeTemplate) -> Self {
47-
let scope = forgerpc::IpxeTemplateScope::try_from(tmpl.scope)
48-
.map(|s| format!("{s:?}"))
49-
.unwrap_or_else(|_| "Unknown".to_string());
50-
Self {
51-
id: tmpl.id.as_ref().map(|u| u.to_string()).unwrap_or_default(),
52-
name: tmpl.name.clone(),
53-
description: tmpl.description.clone(),
54-
scope,
55-
required_params_count: tmpl.required_params.len(),
56-
required_artifacts_count: tmpl.required_artifacts.len(),
57-
}
58-
}
48+
templates: Vec<forgerpc::IpxeTemplate>,
5949
}
6050

6151
pub async fn show_html(AxumState(state): AxumState<Arc<Api>>) -> Response {
@@ -71,9 +61,7 @@ pub async fn show_html(AxumState(state): AxumState<Arc<Api>>) -> Response {
7161
}
7262
};
7363

74-
let tmpl = IpxeTemplateShow {
75-
templates: templates.iter().map(Into::into).collect(),
76-
};
64+
let tmpl = IpxeTemplateShow { templates };
7765
(StatusCode::OK, Html(tmpl.render().unwrap())).into_response()
7866
}
7967

@@ -103,29 +91,12 @@ async fn fetch_templates(api: Arc<Api>) -> Result<Vec<forgerpc::IpxeTemplate>, t
10391
#[derive(Template)]
10492
#[template(path = "ipxe_template_detail.html")]
10593
struct IpxeTemplateDetail {
106-
name: String,
107-
description: String,
108-
scope: String,
109-
required_params: Vec<String>,
110-
reserved_params: Vec<String>,
111-
required_artifacts: Vec<String>,
112-
template_text: String,
94+
tmpl: forgerpc::IpxeTemplate,
11395
}
11496

11597
impl From<forgerpc::IpxeTemplate> for IpxeTemplateDetail {
11698
fn from(tmpl: forgerpc::IpxeTemplate) -> Self {
117-
let scope = forgerpc::IpxeTemplateScope::try_from(tmpl.scope)
118-
.map(|s| format!("{s:?}"))
119-
.unwrap_or_else(|_| "Unknown".to_string());
120-
Self {
121-
name: tmpl.name,
122-
description: tmpl.description,
123-
scope,
124-
required_params: tmpl.required_params,
125-
reserved_params: tmpl.reserved_params,
126-
required_artifacts: tmpl.required_artifacts,
127-
template_text: tmpl.template,
128-
}
99+
Self { tmpl }
129100
}
130101
}
131102

crates/api/src/web/rack.rs

Lines changed: 2 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,7 @@ use crate::api::Api;
3232
#[derive(Template)]
3333
#[template(path = "rack_show.html")]
3434
struct Racks {
35-
racks: Vec<RackRecord>,
36-
}
37-
38-
#[derive(Debug, serde::Serialize)]
39-
struct RackRecord {
40-
id: String,
41-
rack_state: String,
42-
compute_trays: String,
43-
power_shelves: String,
44-
switches: String,
35+
racks: Vec<rpc::forge::Rack>,
4536
}
4637

4738
#[derive(Template)]
@@ -74,31 +65,7 @@ pub async fn show_html(state: AxumState<Arc<Api>>) -> Response {
7465
}
7566
};
7667

77-
let racks = racks
78-
.racks
79-
.into_iter()
80-
.map(|rack| RackRecord {
81-
id: rack.id.map(|id| id.to_string()).unwrap_or_default(),
82-
rack_state: rack.rack_state,
83-
compute_trays: format!(
84-
"{} / {}",
85-
rack.compute_trays.len(),
86-
rack.expected_compute_trays.len()
87-
),
88-
power_shelves: format!(
89-
"{} / {}",
90-
rack.power_shelves.len(),
91-
rack.expected_power_shelves.len()
92-
),
93-
switches: format!(
94-
"{} / {}",
95-
rack.switches.len(),
96-
rack.expected_nvlink_switches.len()
97-
),
98-
})
99-
.collect();
100-
101-
let display = Racks { racks };
68+
let display = Racks { racks: racks.racks };
10269
(StatusCode::OK, Html(display.render().unwrap())).into_response()
10370
}
10471

0 commit comments

Comments
 (0)