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
This commit is contained in:
Ellie Huxtable 2023-03-08 23:45:14 +00:00 committed by GitHub
parent afd1113b3b
commit b91d4f4806
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 53 additions and 25 deletions

View File

@ -71,13 +71,19 @@ pub trait Database: Send + Sync {
async fn last(&self) -> Result<History>; async fn last(&self) -> Result<History>;
async fn before(&self, timestamp: chrono::DateTime<Utc>, count: i64) -> Result<Vec<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( async fn search(
&self, &self,
limit: Option<i64>,
search_mode: SearchMode, search_mode: SearchMode,
filter: FilterMode, filter: FilterMode,
context: &Context, context: &Context,
query: &str, query: &str,
limit: Option<i64>,
before: Option<i64>,
after: Option<i64>,
) -> Result<Vec<History>>; ) -> Result<Vec<History>>;
async fn query_history(&self, query: &str) -> Result<Vec<History>>; async fn query_history(&self, query: &str) -> Result<Vec<History>>;
@ -385,11 +391,13 @@ impl Database for Sqlite {
async fn search( async fn search(
&self, &self,
limit: Option<i64>,
search_mode: SearchMode, search_mode: SearchMode,
filter: FilterMode, filter: FilterMode,
context: &Context, context: &Context,
query: &str, query: &str,
limit: Option<i64>,
before: Option<i64>,
after: Option<i64>,
) -> Result<Vec<History>> { ) -> Result<Vec<History>> {
let mut sql = SqlBuilder::select_from("history"); let mut sql = SqlBuilder::select_from("history");
@ -401,6 +409,14 @@ impl Database for Sqlite {
sql.limit(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 { match filter {
FilterMode::Global => &mut sql, FilterMode::Global => &mut sql,
FilterMode::Host => sql.and_where_eq("hostname", quote(&context.hostname)), FilterMode::Host => sql.and_where_eq("hostname", quote(&context.hostname)),
@ -498,7 +514,9 @@ mod test {
cwd: "/home/ellie".to_string(), 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!( assert_eq!(
results.len(), results.len(),
@ -701,7 +719,15 @@ mod test {
} }
let start = Instant::now(); let start = Instant::now();
let _results = db let _results = db
.search(None, SearchMode::Fuzzy, FilterMode::Global, &context, "") .search(
SearchMode::Fuzzy,
FilterMode::Global,
&context,
"",
None,
None,
None,
)
.await .await
.unwrap(); .unwrap();
let duration = start.elapsed(); let duration = start.elapsed();

View File

@ -148,13 +148,25 @@ async fn run_non_interactive(
let context = current_context(); 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 let results = db
.search( .search(
limit,
settings.search_mode, settings.search_mode,
settings.filter_mode, settings.filter_mode,
&context, &context,
query.join(" ").as_str(), query.join(" ").as_str(),
limit,
before,
after,
) )
.await?; .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 true
}) })
.map(std::borrow::ToOwned::to_owned) .map(std::borrow::ToOwned::to_owned)

View File

@ -57,7 +57,15 @@ impl State {
db.list(self.filter_mode, &self.context, Some(200), true) db.list(self.filter_mode, &self.context, Some(200), true)
.await? .await?
} else { } else {
db.search(Some(200), search_mode, self.filter_mode, &self.context, i) db.search(
search_mode,
self.filter_mode,
&self.context,
i,
Some(200),
None,
None,
)
.await? .await?
}; };