Skip to content

feat: modify Dashboard and Filters type to HashMap #1172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions src/users/dashboards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*
*/

use std::sync::RwLock;
use std::{collections::HashMap, sync::RwLock};

use once_cell::sync::Lazy;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -108,7 +108,7 @@ pub struct Dashboard {
}

#[derive(Default, Debug)]
pub struct Dashboards(RwLock<Vec<Dashboard>>);
pub struct Dashboards(RwLock<HashMap<String, Dashboard>>);

impl Dashboards {
pub async fn load(&self) -> anyhow::Result<()> {
Expand Down Expand Up @@ -173,20 +173,29 @@ impl Dashboards {
}

let mut s = self.0.write().expect(LOCK_EXPECT);
s.append(&mut this);
s.extend(
this.into_iter()
.map(|d| (d.dashboard_id.clone().unwrap_or_else(|| d.name.clone()), d)),
);
Comment on lines +176 to +179
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we do this using HashMap::inserts, with dashboard_id as always the key!


Ok(())
}

pub fn update(&self, dashboard: &Dashboard) {
let mut s = self.0.write().expect(LOCK_EXPECT);
s.retain(|d| d.dashboard_id != dashboard.dashboard_id);
s.push(dashboard.clone());
s.retain(|_, d| d.dashboard_id != dashboard.dashboard_id);
s.insert(
dashboard
.dashboard_id
.clone()
.unwrap_or(dashboard.name.clone()),
dashboard.clone(),
);
Comment on lines +186 to +193
Copy link
Contributor

@de-sh de-sh Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a simple insert will work? Insert is equivalent to an update in the case of existing keys.

}

pub fn delete_dashboard(&self, dashboard_id: &str) {
let mut s = self.0.write().expect(LOCK_EXPECT);
s.retain(|d| d.dashboard_id != Some(dashboard_id.to_string()));
s.remove(dashboard_id);
}

pub fn get_dashboard(&self, dashboard_id: &str, user_id: &str) -> Option<Dashboard> {
Expand All @@ -195,19 +204,19 @@ impl Dashboards {
.expect(LOCK_EXPECT)
.iter()
.find(|d| {
d.dashboard_id == Some(dashboard_id.to_string())
&& d.user_id == Some(user_id.to_string())
d.1.dashboard_id == Some(dashboard_id.to_string())
&& d.1.user_id == Some(user_id.to_string())
Comment on lines 205 to +208
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we do this with a simple HashMap::get?

})
.cloned()
.map(|(_, d)| d.clone())
}

pub fn list_dashboards_by_user(&self, user_id: &str) -> Vec<Dashboard> {
pub fn list_dashboards_by_user(&self, user_id: &str) -> HashMap<String, Dashboard> {
self.0
.read()
.expect(LOCK_EXPECT)
.iter()
.filter(|d| d.user_id == Some(user_id.to_string()))
.cloned()
.filter(|(_, d)| d.user_id == Some(user_id.to_string()))
.map(|(k, d)| (k.clone(), d.clone()))
.collect()
}
}
Expand Down
33 changes: 24 additions & 9 deletions src/users/filters.rs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar suggestions here

Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*
*/

use datafusion::common::HashMap;
use once_cell::sync::Lazy;
use serde::{Deserialize, Serialize};
use serde_json::Value;
Expand Down Expand Up @@ -70,7 +71,7 @@ pub struct Rules {
}

#[derive(Debug, Default)]
pub struct Filters(RwLock<Vec<Filter>>);
pub struct Filters(RwLock<HashMap<String, Filter>>);

impl Filters {
pub async fn load(&self) -> anyhow::Result<()> {
Expand Down Expand Up @@ -125,40 +126,54 @@ impl Filters {
}

let mut s = self.0.write().expect(LOCK_EXPECT);
s.append(&mut this);
s.extend(this.into_iter().map(|filter| {
(
filter
.filter_id
.clone()
.unwrap_or_else(|| filter.clone().filter_name),
filter,
)
}));

Ok(())
}

pub fn update(&self, filter: &Filter) {
let mut s = self.0.write().expect(LOCK_EXPECT);
s.retain(|f| f.filter_id != filter.filter_id);
s.push(filter.clone());
s.retain(|_, f| f.filter_id != filter.filter_id);
s.insert(
filter
.filter_id
.clone()
.unwrap_or_else(|| filter.clone().filter_name),
filter.clone(),
);
}

pub fn delete_filter(&self, filter_id: &str) {
let mut s = self.0.write().expect(LOCK_EXPECT);
s.retain(|f| f.filter_id != Some(filter_id.to_string()));
s.retain(|_, f| f.filter_id != Some(filter_id.to_string()));
}

pub fn get_filter(&self, filter_id: &str, user_id: &str) -> Option<Filter> {
self.0
.read()
.expect(LOCK_EXPECT)
.iter()
.find(|f| {
.find(|(_, f)| {
f.filter_id == Some(filter_id.to_string()) && f.user_id == Some(user_id.to_string())
})
.cloned()
.map(|(_, f)| f.clone())
}

pub fn list_filters_by_user(&self, user_id: &str) -> Vec<Filter> {
self.0
.read()
.expect(LOCK_EXPECT)
.iter()
.filter(|f| f.user_id == Some(user_id.to_string()))
.cloned()
.filter(|(_, f)| f.user_id == Some(user_id.to_string()))
.map(|(_, f)| f.clone())
.collect()
}
}
Expand Down
Loading