client filtering done in query (#629)

atuin-client/src/database.rs

@@ -25,6 +25,17 @@ pub struct Context {
     pub hostname: String,
 }
 
+#[derive(Default, Clone)]
+pub struct OptFilters {
+    pub exit: Option<i64>,
+    pub exclude_exit: Option<i64>,
+    pub cwd: Option<String>,
+    pub exclude_cwd: Option<String>,
+    pub before: Option<String>,
+    pub after: Option<String>,
+    pub limit: Option<i64>,
+}
+
 pub fn current_context() -> Context {
     let Ok(session) = env::var("ATUIN_SESSION") else {
         eprintln!("ERROR: Failed to find $ATUIN_SESSION in the environment. Check that you have correctly set up your shell.");
@@ -79,9 +90,7 @@ pub trait Database: Send + Sync + 'static {
         filter: FilterMode,
         context: &Context,
         query: &str,
-        limit: Option<i64>,
+        filter_options: OptFilters,
-        before: Option<i64>,
-        after: Option<i64>,
     ) -> Result<Vec<History>>;
 
     async fn query_history(&self, query: &str) -> Result<Vec<History>>;
@@ -340,9 +349,7 @@ impl Database for Sqlite {
         filter: FilterMode,
         context: &Context,
         query: &str,
-        limit: Option<i64>,
+        filter_options: OptFilters,
-        before: Option<i64>,
-        after: Option<i64>,
     ) -> Result<Vec<History>> {
         let mut sql = SqlBuilder::select_from("history");
 
@@ -350,18 +357,10 @@ impl Database for Sqlite {
             .having("max(timestamp)")
             .order_desc("timestamp");
 
-        if let Some(limit) = limit {
+        if let Some(limit) = filter_options.limit {
             sql.limit(limit);
         }
 
-        if let Some(after) = after {
-            sql.and_where_gt("timestamp", after);
-        }
-
-        if let Some(before) = before {
-            sql.and_where_lt("timestamp", before);
-        }
-
         match filter {
             FilterMode::Global => &mut sql,
             FilterMode::Host => sql.and_where_eq("hostname", quote(&context.hostname)),
@@ -421,6 +420,32 @@ impl Database for Sqlite {
             }
         };
 
+        filter_options
+            .exit
+            .map(|exit| sql.and_where_eq("exit", exit));
+
+        filter_options
+            .exclude_exit
+            .map(|exclude_exit| sql.and_where_ne("exit", exclude_exit));
+
+        filter_options
+            .cwd
+            .map(|cwd| sql.and_where_eq("cwd", quote(cwd)));
+
+        filter_options
+            .exclude_cwd
+            .map(|exclude_cwd| sql.and_where_ne("cwd", quote(exclude_cwd)));
+
+        filter_options.before.map(|before| {
+            interim::parse_date_string(before.as_str(), Utc::now(), interim::Dialect::Uk)
+                .map(|before| sql.and_where_lt("timestamp", quote(before.timestamp_nanos())))
+        });
+
+        filter_options.after.map(|after| {
+            interim::parse_date_string(after.as_str(), Utc::now(), interim::Dialect::Uk)
+                .map(|after| sql.and_where_gt("timestamp", quote(after.timestamp_nanos())))
+        });
+
         let query = sql.sql().expect("bug in search query. please report");
 
         let res = sqlx::query(&query)
@@ -508,7 +533,15 @@ mod test {
         };
 
         let results = db
-            .search(mode, filter_mode, &context, query, None, None, None)
+            .search(
+                mode,
+                filter_mode,
+                &context,
+                query,
+                OptFilters {
+                    ..Default::default()
+                },
+            )
             .await?;
 
         assert_eq!(
@@ -718,9 +751,9 @@ mod test {
                 FilterMode::Global,
                 &context,
                 "",
-                None,
+                OptFilters {
-                None,
+                    ..Default::default()
-                None,
+                },
             )
             .await
             .unwrap();

src/command/client/search.rs

@@ -1,11 +1,10 @@
 use atuin_common::utils;
-use chrono::Utc;
 use clap::Parser;
 use eyre::Result;
 
 use atuin_client::{
-    database::current_context,
     database::Database,
+    database::{current_context, OptFilters},
     history::History,
     settings::{FilterMode, SearchMode, Settings},
 };
@@ -103,19 +102,18 @@ impl Cmd {
         } else {
             let list_mode = ListMode::from_flags(self.human, self.cmd_only);
 
-            let mut entries = run_non_interactive(
+            let opt_filter = OptFilters {
-                settings,
+                exit: self.exit,
-                self.cwd.clone(),
+                exclude_exit: self.exclude_exit,
-                self.exit,
+                cwd: self.cwd,
-                self.exclude_exit,
+                exclude_cwd: self.exclude_cwd,
-                self.exclude_cwd.clone(),
+                before: self.before,
-                self.before.clone(),
+                after: self.after,
-                self.after.clone(),
+                limit: self.limit,
-                self.limit,
+            };
-                &self.query,
+
-                &mut db,
+            let mut entries =
-            )
+                run_non_interactive(settings, opt_filter.clone(), &self.query, &mut db).await?;
-            .await?;
 
             if entries.is_empty() {
                 std::process::exit(1)
@@ -132,18 +130,8 @@ impl Cmd {
                         db.delete(entry.clone()).await?;
                     }
 
-                    entries = run_non_interactive(
+                    entries =
-                        settings,
+                        run_non_interactive(settings, opt_filter.clone(), &self.query, &mut db)
-                        self.cwd.clone(),
-                        self.exit,
-                        self.exclude_exit,
-                        self.exclude_cwd.clone(),
-                        self.before.clone(),
-                        self.after.clone(),
-                        self.limit,
-                        &self.query,
-                        &mut db,
-                    )
                             .await?;
                 }
             } else {
@@ -159,33 +147,22 @@ impl Cmd {
 #[allow(clippy::too_many_arguments)]
 async fn run_non_interactive(
     settings: &Settings,
-    cwd: Option<String>,
+    filter_options: OptFilters,
-    exit: Option<i64>,
-    exclude_exit: Option<i64>,
-    exclude_cwd: Option<String>,
-    before: Option<String>,
-    after: Option<String>,
-    limit: Option<i64>,
     query: &[String],
     db: &mut impl Database,
 ) -> Result<Vec<History>> {
-    let dir = if cwd.as_deref() == Some(".") {
+    let dir = if filter_options.cwd.as_deref() == Some(".") {
         Some(utils::get_current_dir())
     } else {
-        cwd
+        filter_options.cwd
     };
 
     let context = current_context();
 
-    let before = before.and_then(|b| {
+    let opt_filter = OptFilters {
-        interim::parse_date_string(b.as_str(), Utc::now(), interim::Dialect::Uk)
+        cwd: dir,
-            .map_or(None, |d| Some(d.timestamp_nanos()))
+        ..filter_options
-    });
+    };
-
-    let after = after.and_then(|a| {
-        interim::parse_date_string(a.as_str(), Utc::now(), interim::Dialect::Uk)
-            .map_or(None, |d| Some(d.timestamp_nanos()))
-    });
 
     let results = db
         .search(
@@ -193,45 +170,9 @@ async fn run_non_interactive(
             settings.filter_mode,
             &context,
             query.join(" ").as_str(),
-            limit,
+            opt_filter,
-            before,
-            after,
         )
         .await?;
 
-    // TODO: This filtering would be better done in the SQL query, I just
-    // need a nice way of building queries.
-    let results: Vec<History> = results
-        .iter()
-        .filter(|h| {
-            if let Some(exit) = exit {
-                if h.exit != exit {
-                    return false;
-                }
-            }
-
-            if let Some(exit) = exclude_exit {
-                if h.exit == exit {
-                    return false;
-                }
-            }
-
-            if let Some(cwd) = &exclude_cwd {
-                if h.cwd.as_str() == cwd.as_str() {
-                    return false;
-                }
-            }
-
-            if let Some(cwd) = &dir {
-                if h.cwd.as_str() != cwd.as_str() {
-                    return false;
-                }
-            }
-
-            true
-        })
-        .map(std::borrow::ToOwned::to_owned)
-        .collect();
-
     Ok(results)
 }

src/command/client/search/engines/db.rs

@@ -1,5 +1,7 @@
 use async_trait::async_trait;
-use atuin_client::{database::Database, history::History, settings::SearchMode};
+use atuin_client::{
+    database::Database, database::OptFilters, history::History, settings::SearchMode,
+};
 use eyre::Result;
 
 use super::{SearchEngine, SearchState};
@@ -19,9 +21,10 @@ impl SearchEngine for Search {
                 state.filter_mode,
                 &state.context,
                 state.input.as_str(),
-                Some(200),
+                OptFilters {
-                None,
+                    limit: Some(200),
-                None,
+                    ..Default::default()
+                },
             )
             .await?
             .into_iter()

src/command/client/search/interactive.rs

@@ -13,8 +13,7 @@ use semver::Version;
 use unicode_width::UnicodeWidthStr;
 
 use atuin_client::{
-    database::current_context,
+    database::{current_context, Database},
-    database::Database,
     history::History,
     settings::{ExitMode, FilterMode, SearchMode, Settings},
 };