refactor: Remove numerous clones()

get_container_name_by_id clone, init_all_log refactor to remove clones,
remove id.clone() by using SpawnId.get_id()
This commit is contained in:
Jack Wills
2024-12-03 16:37:38 +00:00
parent de72ab47cd
commit e5927f781a
6 changed files with 93 additions and 92 deletions
+6 -3
View File
@@ -110,10 +110,10 @@ pub struct ContainerPorts {
pub public: Option<u16>,
}
impl From<&Port> for ContainerPorts {
fn from(value: &Port) -> Self {
impl From<Port> for ContainerPorts {
fn from(value: Port) -> Self {
Self {
ip: value.ip.clone(),
ip: value.ip,
private: value.private_port,
public: value.public_port,
}
@@ -258,9 +258,12 @@ pub enum State {
}
impl State {
/// The container is alive if the start is Running, either healthy or unhealthy
pub const fn is_alive(self) -> bool {
matches!(self, Self::Running(_))
}
/// Color of the state for the containers section
/// TODO allow usable editable colours
pub const fn get_color(self) -> Color {
match self {
Self::Paused => Color::Yellow,
+25 -19
View File
@@ -251,7 +251,7 @@ impl AppData {
self.filter_containers();
}
// change the filter_by option
/// change the filter_by option
pub fn filter_by_next(&mut self) {
if let Some(by) = self.filter.by.next() {
self.filter.by = by;
@@ -259,7 +259,7 @@ impl AppData {
}
}
// change the filter_by option
/// change the filter_by option
pub fn filter_by_prev(&mut self) {
if let Some(by) = self.filter.by.prev() {
self.filter.by = by;
@@ -393,6 +393,14 @@ impl AppData {
self.containers.items.len()
}
pub fn get_all_id_state(&self) -> Vec<(State, ContainerId)> {
self.containers
.items
.iter()
.map(|i| (i.state, i.id.clone()))
.collect::<Vec<_>>()
}
/// Get all the ContainerItems
pub fn get_container_items(&self) -> &[ContainerItem] {
&self.containers.items
@@ -478,11 +486,10 @@ impl AppData {
}
/// Get Option of the current selected container's ports, sorted by private port
pub fn get_selected_ports(&mut self) -> Option<(Vec<ContainerPorts>, State)> {
pub fn get_selected_ports(&mut self) -> Option<(&[ContainerPorts], State)> {
if let Some(item) = self.get_mut_selected_container() {
let mut ports = item.ports.clone();
ports.sort_by(|a, b| a.private.cmp(&b.private));
return Some((ports, item.state));
item.ports.sort_by(|a, b| a.private.cmp(&b.private));
return Some((&item.ports, item.state));
}
None
}
@@ -506,12 +513,12 @@ impl AppData {
}
/// Get the ContainerName of by ID
pub fn get_container_name_by_id(&mut self, id: &ContainerId) -> Option<ContainerName> {
pub fn get_container_name_by_id(&mut self, id: &ContainerId) -> Option<&ContainerName> {
self.containers
.items
.iter_mut()
.find(|i| &i.id == id)
.map(|i| i.name.clone())
.map(|i| &i.name)
}
/// Find the id of the currently selected container.
@@ -692,7 +699,6 @@ impl AppData {
let mut columns = Columns::new();
let count = |x: &str| u8::try_from(x.chars().count()).unwrap_or(12);
// Should probably find a refactor here somewhere
for container in [&self.containers.items, &self.hidden_containers] {
for container in container {
let cpu_count = container.cpu_stats.back().map_or_else(
@@ -759,12 +765,11 @@ impl AppData {
container.tx.update(tx);
container.mem_limit.update(mem_limit);
}
// need to benchmark this?
self.sort_containers();
}
/// Update, or insert, containers
pub fn update_containers(&mut self, all_containers: &mut [ContainerSummary]) {
pub fn update_containers(&mut self, mut all_containers: Vec<ContainerSummary>) {
let all_ids = self
.containers
.items
@@ -799,7 +804,7 @@ impl AppData {
}
}
for i in all_containers {
for mut i in all_containers {
if let Some(id) = i.id.as_ref() {
let name = i.names.as_mut().map_or(String::new(), |names| {
names.first_mut().map_or(String::new(), |f| {
@@ -810,8 +815,8 @@ impl AppData {
})
});
let ports = i.ports.as_ref().map_or(vec![], |i| {
i.iter().map(ContainerPorts::from).collect::<Vec<_>>()
let ports = i.ports.map_or(vec![], |i| {
i.into_iter().map(ContainerPorts::from).collect::<Vec<_>>()
});
let id = ContainerId::from(id.as_str());
@@ -1466,7 +1471,7 @@ mod tests {
let mut app_data = gen_appdata(&containers);
let result = app_data.get_container_name_by_id(&ContainerId::from("2"));
assert_eq!(result, Some(ContainerName::from("container_2")));
assert_eq!(result, Some(&ContainerName::from("container_2")));
}
#[test]
@@ -2173,7 +2178,8 @@ mod tests {
private: 8001,
public: None
}
],
]
.as_slice(),
State::Running(RunningState::Healthy),
))
);
@@ -2185,7 +2191,7 @@ mod tests {
assert_eq!(
result,
Some((vec![], State::Running(RunningState::Healthy)))
Some((vec![].as_slice(), State::Running(RunningState::Healthy)))
);
}
@@ -2220,12 +2226,12 @@ mod tests {
let (_ids, containers) = gen_containers();
let mut app_data = gen_appdata(&containers);
let result_pre = app_data.get_container_items().to_owned();
let mut input = [
let input = vec![
gen_container_summary(1, "paused"),
gen_container_summary(2, "dead"),
];
app_data.update_containers(&mut input);
app_data.update_containers(input);
let result_post = app_data.get_container_items().to_owned();
assert_ne!(result_pre, result_post);
assert_eq!(result_post[0].state, State::Paused);
+46 -59
View File
@@ -19,7 +19,7 @@ use tokio::{
use uuid::Uuid;
use crate::{
app_data::{AppData, ContainerId, ContainerStatus, DockerCommand, State},
app_data::{AppData, ContainerId, DockerCommand, State},
app_error::AppError,
parse_args::CliArgs,
ui::{GuiState, Status},
@@ -34,6 +34,15 @@ enum SpawnId {
Log(ContainerId),
}
impl SpawnId {
/// Extract the &ContainerId out of self
const fn get_id(&self) -> &ContainerId {
match self {
Self::Log(id) | Self::Stats((id, _)) => id,
}
}
}
/// Cpu & Mem stats take twice as long as the update interval to get a value, so will have two being executed at the same time
/// SpawnId::Stats takes container_id and binate value to enable both cycles of the same container_id to be inserted into the hashmap
/// Binate value is toggled when all handles have been spawned off
@@ -104,12 +113,12 @@ impl DockerData {
async fn update_container_stat(
app_data: Arc<Mutex<AppData>>,
docker: Arc<Docker>,
id: ContainerId,
state: State,
spawn_id: SpawnId,
spawns: Arc<Mutex<HashMap<SpawnId, JoinHandle<()>>>>,
) {
if state.is_alive() {
let id = spawn_id.get_id();
let mut stream = docker
.stats(
id.get(),
@@ -162,31 +171,27 @@ impl DockerData {
app_data
.lock()
.update_stats_by_id(&id, cpu_stats, mem_stat, mem_limit, rx, tx);
.update_stats_by_id(id, cpu_stats, mem_stat, mem_limit, rx, tx);
}
}
spawns.lock().remove(&spawn_id);
}
/// Update all stats, spawn each container into own tokio::spawn thread
fn update_all_container_stats(&mut self, all_ids: &[(State, ContainerId)]) {
fn update_all_container_stats(&mut self) {
let all_ids = self.app_data.lock().get_all_id_state();
for (state, id) in all_ids {
let docker = Arc::clone(&self.docker);
let app_data = Arc::clone(&self.app_data);
let spawns = Arc::clone(&self.spawns);
let spawn_id = SpawnId::Stats((id.clone(), self.binate));
let spawn_id = SpawnId::Stats((id, self.binate));
self.spawns
.lock()
.entry(spawn_id.clone())
.or_insert_with(|| {
tokio::spawn(Self::update_container_stat(
app_data,
docker,
id.clone(),
*state,
Arc::clone(&self.app_data),
Arc::clone(&self.docker),
state,
spawn_id,
spawns,
Arc::clone(&self.spawns),
))
});
}
@@ -196,7 +201,7 @@ impl DockerData {
/// Get all current containers, handle into ContainerItem in the app_data struct rather than here
/// Just make sure that items sent are guaranteed to have an id
/// If in a containerised runtime, will ignore any container that uses the `/app/oxker` as an entry point, unless the `-s` flag is set
async fn update_all_containers(&self) -> Vec<(State, ContainerId)> {
async fn update_all_containers(&self) {
let containers = self
.docker
.list_containers(Some(ListContainersOptions::<String> {
@@ -206,7 +211,7 @@ impl DockerData {
.await
.unwrap_or_default();
let mut output = containers
let output = containers
.into_iter()
.filter_map(|f| match f.id {
Some(_) => {
@@ -225,23 +230,7 @@ impl DockerData {
})
.collect::<Vec<ContainerSummary>>();
self.app_data.lock().update_containers(&mut output);
// Just get the containers that are currently running, or being restarted, no point updating info on paused or dead containers
output
.into_iter()
.filter_map(|i| {
i.id.map(|id| {
(
State::from((
i.state,
&ContainerStatus::from(i.status.map_or_else(String::new, |i| i)),
)),
ContainerId::from(id.as_str()),
)
})
})
.collect::<Vec<_>>()
self.app_data.lock().update_containers(output);
}
/// Update single container logs
@@ -250,7 +239,6 @@ impl DockerData {
app_data: Arc<Mutex<AppData>>,
docker: Arc<Docker>,
id: ContainerId,
init: Option<Arc<AtomicUsize>>,
since: u64,
spawns: Arc<Mutex<HashMap<SpawnId, JoinHandle<()>>>>,
stderr: bool,
@@ -272,28 +260,29 @@ impl DockerData {
output.push(data);
}
}
spawns.lock().remove(&SpawnId::Log(id.clone()));
app_data.lock().update_log_by_id(output, &id);
init.map(|i| i.fetch_add(1, std::sync::atomic::Ordering::SeqCst));
spawns.lock().remove(&SpawnId::Log(id));
}
/// Update all logs, spawn each container into own tokio::spawn thread
fn init_all_logs(&self, all_ids: &[(State, ContainerId)], init: Option<&Arc<AtomicUsize>>) {
fn init_all_logs(&self, all_ids: Vec<(State, ContainerId)>) -> Arc<AtomicUsize> {
let init = Arc::new(AtomicUsize::new(0));
for (_, id) in all_ids {
// let init = init.map(|i|Arc::clone(i));
let app_data: Arc<parking_lot::lock_api::Mutex<parking_lot::RawMutex, AppData>> =
Arc::clone(&self.app_data);
let docker = Arc::clone(&self.docker);
let spawns = Arc::clone(&self.spawns);
let std_err = self.args.std_err;
let init = Arc::clone(&init);
self.spawns.lock().insert(
SpawnId::Log(id.clone()),
tokio::spawn(Self::update_log(
Arc::clone(&self.app_data),
Arc::clone(&self.docker),
id.clone(),
init.map(Arc::clone),
0,
Arc::clone(&self.spawns),
self.args.std_err,
)),
tokio::spawn(async move {
Self::update_log(app_data, docker, id, 0, spawns, std_err).await;
init.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
}),
);
}
init
}
/// Initialize docker container data, before any messages are received
@@ -301,14 +290,13 @@ impl DockerData {
self.gui_state.lock().status_push(Status::Init);
let loading_uuid = Uuid::new_v4();
GuiState::start_loading_animation(&self.gui_state, loading_uuid);
let all_ids = self.update_all_containers().await;
self.update_all_containers().await;
let all_ids = self.app_data.lock().get_all_id_state();
let all_ids_len = all_ids.len();
let init = self.init_all_logs(all_ids);
self.update_all_container_stats();
self.update_all_container_stats(&all_ids);
let init = Arc::new(AtomicUsize::new(0));
self.init_all_logs(&all_ids, Some(&init));
while init.load(std::sync::atomic::Ordering::SeqCst) != all_ids.len() {
while init.load(std::sync::atomic::Ordering::SeqCst) != all_ids_len {
self.app_data.lock().sort_containers();
tokio::time::sleep(std::time::Duration::from_millis(10)).await;
}
@@ -318,7 +306,7 @@ impl DockerData {
/// Update all cpu_mem, and selected container log (if a log update join_handle isn't currently being executed)
async fn update_everything(&mut self) {
let all_ids = self.update_all_containers().await;
self.update_all_containers().await;
if let Some(container) = self.app_data.lock().get_selected_container() {
let last_updated = container.last_updated;
self.spawns
@@ -329,14 +317,13 @@ impl DockerData {
Arc::clone(&self.app_data),
Arc::clone(&self.docker),
container.id.clone(),
None,
last_updated,
Arc::clone(&self.spawns),
self.args.std_err,
))
});
};
self.update_all_container_stats(&all_ids);
self.update_all_container_stats();
self.app_data.lock().sort_containers();
}
@@ -438,7 +425,7 @@ impl DockerData {
if app_data.lock().get_error().is_none() {
let mut inner = Self {
app_data,
args: args.clone(),
args,
binate: Binate::One,
docker: Arc::new(docker),
gui_state,
@@ -446,7 +433,7 @@ impl DockerData {
spawns: Arc::new(Mutex::new(HashMap::new())),
};
inner.initialise_container_data().await;
Self::heartbeat(&args, docker_tx);
Self::heartbeat(&inner.args, docker_tx);
inner.message_handler().await;
}
}
+10 -7
View File
@@ -144,9 +144,9 @@ impl TerminalSize {
#[derive(Debug, Clone)]
pub enum ExecMode {
// use Bollard Rust library
Internal((ContainerId, Arc<Docker>)),
Internal((Arc<ContainerId>, Arc<Docker>)),
// use the external `docker-cli`
External(ContainerId),
External(Arc<ContainerId>),
}
impl ExecMode {
@@ -186,7 +186,10 @@ impl ExecMode {
{
if let Some(Ok(msg)) = output.next().await {
if !msg.to_string().starts_with(OCI_ERROR) {
return Some(Self::Internal((id.clone(), Arc::clone(docker))));
return Some(Self::Internal((
Arc::new(id),
Arc::clone(docker),
)));
}
}
}
@@ -199,7 +202,7 @@ impl ExecMode {
{
if let Ok(output) = String::from_utf8(output.stdout) {
if !output.starts_with(OCI_ERROR) {
return Some(Self::External(id.clone()));
return Some(Self::External(Arc::new(id)));
}
}
}
@@ -302,9 +305,9 @@ impl ExecMode {
Ok(())
}
// This is the fix for key pressed not being handled correctly on quit
// It writes a special message to the stdout, and then listens out for a valid response
// afterwhich it's assumes that we're completely done with TTY
/// This is the fix for key pressed not being handled correctly on quit
/// It writes a special message to the stdout, and then listens out for a valid response
/// afterwhich it's assumes that we're completely done with TTY
fn internal_cleanup(&self) -> Result<(), AppError> {
match self {
Self::External(_) => Ok(()),
+5 -3
View File
@@ -294,7 +294,8 @@ pub fn ports(
app_data: &Arc<Mutex<AppData>>,
max_lens: (usize, usize, usize),
) {
let ports = app_data.lock().get_selected_ports();
let mut data = app_data.lock();
let ports = data.get_selected_ports();
if let Some(ports) = ports {
let block = Block::default()
.borders(Borders::ALL)
@@ -318,6 +319,7 @@ pub fn ports(
.alignment(Alignment::Center)
.block(block);
f.render_widget(paragraph, area);
drop(data);
} else {
let mut output = vec![Line::from(
Span::from(format!(
@@ -326,7 +328,7 @@ pub fn ports(
))
.fg(Color::Yellow),
)];
for item in &ports.0 {
for item in ports.0 {
let fg = Color::White;
let strings = item.print();
@@ -1244,7 +1246,7 @@ mod tests {
setup
.app_data
.lock()
.update_containers(&mut vec![gen_container_summary(1, "paused")]);
.update_containers(vec![gen_container_summary(1, "paused")]);
setup.app_data.lock().docker_controls_next();
let expected = [
+1 -1
View File
@@ -326,7 +326,7 @@ fn draw_frame(f: &mut Frame, app_data: &Arc<Mutex<AppData>>, gui_state: &Arc<Mut
gui_state.lock().set_delete_container(None);
},
|name| {
draw_blocks::delete_confirm(f, gui_state, &name);
draw_blocks::delete_confirm(f, gui_state, name);
},
);
}