From b91d4f4806a0cf3059db08f6dcfc4c1bb4cf992c Mon Sep 17 00:00:00 2001
From: Ellie Huxtable <ellie@elliehuxtable.com>
Date: Wed, 8 Mar 2023 23:45:14 +0000
Subject: [PATCH] Fix before/after combined with limit (#770)

* Fix before/after combined with limit

Mixing filters done in Rust with filters done in SQL is _no bueno_.

Been meaning to do this for a while anyways. Search params are getting a
bit fat but oh well!

* Make an excuse for a big function sig

* Do options map_or not if

* Fix tests
---
 atuin-client/src/database.rs             | 34 +++++++++++++++++++++---
 src/command/client/search.rs             | 32 +++++++++-------------
 src/command/client/search/interactive.rs | 12 +++++++--
 3 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/atuin-client/src/database.rs b/atuin-client/src/database.rs
index d0e696c..d19a0b7 100644
--- a/atuin-client/src/database.rs
+++ b/atuin-client/src/database.rs
@@ -71,13 +71,19 @@ pub trait Database: Send + Sync {
     async fn last(&self) -> Result<History>;
     async fn before(&self, timestamp: chrono::DateTime<Utc>, count: i64) -> Result<Vec<History>>;
 
+    // Yes I know, it's a lot.
+    // Could maybe break it down to a searchparams struct or smth but that feels a little... pointless.
+    // Been debating maybe a DSL for search? eg "before:time limit:1 the query"
+    #[allow(clippy::too_many_arguments)]
     async fn search(
         &self,
-        limit: Option<i64>,
         search_mode: SearchMode,
         filter: FilterMode,
         context: &Context,
         query: &str,
+        limit: Option<i64>,
+        before: Option<i64>,
+        after: Option<i64>,
     ) -> Result<Vec<History>>;
 
     async fn query_history(&self, query: &str) -> Result<Vec<History>>;
@@ -385,11 +391,13 @@ impl Database for Sqlite {
 
     async fn search(
         &self,
-        limit: Option<i64>,
         search_mode: SearchMode,
         filter: FilterMode,
         context: &Context,
         query: &str,
+        limit: Option<i64>,
+        before: Option<i64>,
+        after: Option<i64>,
     ) -> Result<Vec<History>> {
         let mut sql = SqlBuilder::select_from("history");
 
@@ -401,6 +409,14 @@ impl Database for Sqlite {
             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)),
@@ -498,7 +514,9 @@ mod test {
             cwd: "/home/ellie".to_string(),
         };
 
-        let results = db.search(None, mode, filter_mode, &context, query).await?;
+        let results = db
+            .search(mode, filter_mode, &context, query, None, None, None)
+            .await?;
 
         assert_eq!(
             results.len(),
@@ -701,7 +719,15 @@ mod test {
         }
         let start = Instant::now();
         let _results = db
-            .search(None, SearchMode::Fuzzy, FilterMode::Global, &context, "")
+            .search(
+                SearchMode::Fuzzy,
+                FilterMode::Global,
+                &context,
+                "",
+                None,
+                None,
+                None,
+            )
             .await
             .unwrap();
         let duration = start.elapsed();
diff --git a/src/command/client/search.rs b/src/command/client/search.rs
index f437783..3098267 100644
--- a/src/command/client/search.rs
+++ b/src/command/client/search.rs
@@ -148,13 +148,25 @@ async fn run_non_interactive(
 
     let context = current_context();
 
+    let before = before.and_then(|b| {
+        interim::parse_date_string(b.as_str(), Utc::now(), interim::Dialect::Uk)
+            .map_or(None, |d| Some(d.timestamp_nanos()))
+    });
+
+    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(
-            limit,
             settings.search_mode,
             settings.filter_mode,
             &context,
             query.join(" ").as_str(),
+            limit,
+            before,
+            after,
         )
         .await?;
 
@@ -187,24 +199,6 @@ async fn run_non_interactive(
                 }
             }
 
-            if let Some(before) = &before {
-                let before =
-                    interim::parse_date_string(before.as_str(), Utc::now(), interim::Dialect::Uk);
-
-                if before.is_err() || h.timestamp.gt(&before.unwrap()) {
-                    return false;
-                }
-            }
-
-            if let Some(after) = &after {
-                let after =
-                    interim::parse_date_string(after.as_str(), Utc::now(), interim::Dialect::Uk);
-
-                if after.is_err() || h.timestamp.lt(&after.unwrap()) {
-                    return false;
-                }
-            }
-
             true
         })
         .map(std::borrow::ToOwned::to_owned)
diff --git a/src/command/client/search/interactive.rs b/src/command/client/search/interactive.rs
index 05ab03e..4a29ee4 100644
--- a/src/command/client/search/interactive.rs
+++ b/src/command/client/search/interactive.rs
@@ -57,8 +57,16 @@ impl State {
             db.list(self.filter_mode, &self.context, Some(200), true)
                 .await?
         } else {
-            db.search(Some(200), search_mode, self.filter_mode, &self.context, i)
-                .await?
+            db.search(
+                search_mode,
+                self.filter_mode,
+                &self.context,
+                i,
+                Some(200),
+                None,
+                None,
+            )
+            .await?
         };
 
         self.results_state.select(0);