diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ca5acc..dcf2fd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ The format is based on - Use `diesel-async` with deadpool feature for database access. - A bunch of previously synchronous handlers are now async. - Some `.map` and simliar replaced with `if` blocks or `for` loops. +* Refactored query parsing and facet handling in search (PR #11). + Should be more efficient now, especially for negative facets. * Avoid an extra query for the positions in search. diff --git a/src/models.rs b/src/models.rs index 005fbb4..f40b18d 100644 --- a/src/models.rs +++ b/src/models.rs @@ -257,10 +257,10 @@ impl Photo { #[async_trait] pub trait Facet { - async fn by_slug( - slug: &str, + async fn load_slugs( + slugs: &[String], db: &mut AsyncPgConnection, - ) -> Result + ) -> Result, Error> where Self: Sized; } @@ -274,11 +274,11 @@ pub struct Tag { #[async_trait] impl Facet for Tag { - async fn by_slug( - slug: &str, + async fn load_slugs( + slugs: &[String], db: &mut AsyncPgConnection, - ) -> Result { - t::tags.filter(t::slug.eq(slug)).first(db).await + ) -> Result, Error> { + t::tags.filter(t::slug.eq_any(slugs)).load(db).await } } @@ -319,11 +319,11 @@ impl Person { #[async_trait] impl Facet for Person { - async fn by_slug( - slug: &str, + async fn load_slugs( + slugs: &[String], db: &mut AsyncPgConnection, - ) -> Result { - h::people.filter(h::slug.eq(slug)).first(db).await + ) -> Result, Error> { + h::people.filter(h::slug.eq_any(slugs)).load(db).await } } @@ -345,11 +345,11 @@ pub struct Place { #[async_trait] impl Facet for Place { - async fn by_slug( - slug: &str, + async fn load_slugs( + slugs: &[String], db: &mut AsyncPgConnection, - ) -> Result { - l::places.filter(l::slug.eq(slug)).first(db).await + ) -> Result, Error> { + l::places.filter(l::slug.eq_any(slugs)).load(db).await } } diff --git a/src/server/search.rs b/src/server/search.rs index cb9a37c..298c312 100644 --- a/src/server/search.rs +++ b/src/server/search.rs @@ -1,4 +1,4 @@ -use super::error::ViewResult; +use super::error::{ViewError, ViewResult}; use super::splitlist::split_to_group_links; use super::urlstring::UrlString; use super::{Context, RenderRucte, Result}; @@ -20,7 +20,7 @@ pub async fn search( query: Vec<(String, String)>, ) -> Result { let mut db = context.db().await?; - let query = SearchQuery::load(query, &mut db).await?; + let query = SearchQuery::load(query.try_into()?, &mut db).await?; let mut photos = Photo::query(context.is_authorized()); if let Some(since) = query.since.as_ref() { @@ -29,36 +29,46 @@ pub async fn search( if let Some(until) = query.until.as_ref() { photos = photos.filter(p::date.le(until)); } - for tag in &query.t { + for tag in &query.t.include { let ids = pt::photo_tags .select(pt::photo_id) - .filter(pt::tag_id.eq(tag.item.id)); - photos = if tag.inc { - photos.filter(p::id.eq_any(ids)) - } else { - photos.filter(p::id.ne_all(ids)) - }; + .filter(pt::tag_id.eq(tag.id)); + photos = photos.filter(p::id.eq_any(ids)); } - for location in &query.l { + if !query.t.exclude.is_empty() { + let ids = query.t.exclude.iter().map(|t| t.id).collect::>(); + let ids = pt::photo_tags + .select(pt::photo_id) + .filter(pt::tag_id.eq_any(ids)); + photos = photos.filter(p::id.ne_all(ids)); + } + for location in &query.l.include { let ids = pl::photo_places .select(pl::photo_id) - .filter(pl::place_id.eq(location.item.id)); - photos = if location.inc { - photos.filter(p::id.eq_any(ids)) - } else { - photos.filter(p::id.ne_all(ids)) - }; + .filter(pl::place_id.eq(location.id)); + photos = photos.filter(p::id.eq_any(ids)); } - for person in &query.p { + if !query.l.exclude.is_empty() { + let ids = query.l.exclude.iter().map(|t| t.id).collect::>(); + let ids = pl::photo_places + .select(pl::photo_id) + .filter(pl::place_id.eq_any(ids)); + photos = photos.filter(p::id.ne_all(ids)); + } + for person in &query.p.include { let ids = pp::photo_people .select(pp::photo_id) - .filter(pp::person_id.eq(person.item.id)); - photos = if person.inc { - photos.filter(p::id.eq_any(ids)) - } else { - photos.filter(p::id.ne_all(ids)) - } + .filter(pp::person_id.eq(person.id)); + photos = photos.filter(p::id.eq_any(ids)); } + if !query.p.exclude.is_empty() { + let ids = query.p.exclude.iter().map(|t| t.id).collect::>(); + let ids = pp::photo_people + .select(pp::photo_id) + .filter(pp::person_id.eq_any(ids)); + photos = photos.filter(p::id.ne_all(ids)); + } + use crate::schema::positions::dsl as pos; if let Some(pos) = query.pos { let pos_ids = pos::positions.select(pos::photo_id); @@ -88,6 +98,76 @@ pub async fn search( })?) } +#[derive(Default, Debug)] +struct RawQuery { + tags: InclExcl, + people: InclExcl, + locations: InclExcl, + pos: Option, + q: String, + since: DateTimeImg, + until: DateTimeImg, +} + +impl TryFrom> for RawQuery { + type Error = ViewError; + + fn try_from(value: Vec<(String, String)>) -> Result { + let mut to = RawQuery::default(); + for (key, val) in value { + match key.as_ref() { + "q" => { + if val.contains("!pos") { + to.pos = Some(false); + } else if val.contains("pos") { + to.pos = Some(true); + } + to.q = val; + } + "t" => to.tags.add(val), + "p" => to.people.add(val), + "l" => to.locations.add(val), + "pos" => { + to.pos = match val.as_str() { + "t" => Some(true), + "!t" => Some(false), + "" => None, + val => { + warn!("Bad value for \"pos\": {:?}", val); + None + } + } + } + "since_date" if !val.is_empty() => { + to.since.date = Some(val.parse().req("since_date")?) + } + "since_time" if !val.is_empty() => { + to.since.time = Some(val.parse().req("since_time")?) + } + "until_date" if !val.is_empty() => { + to.until.date = Some(val.parse().req("until_date")?) + } + "until_time" if !val.is_empty() => { + to.until.time = Some(val.parse().req("until_time")?) + } + "from" => to.since.img = Some(val.parse().req("from")?), + "to" => to.until.img = Some(val.parse().req("to")?), + _ => (), // ignore unknown query parameters + } + } + Ok(to) + } +} + +/// A since or until time, can either be given as a date (optionally +/// with time) or as an image id to take the datetime from. +#[derive(Default, Debug)] +struct DateTimeImg { + date: Option, + time: Option, + img: Option, +} + /// A `Vec` that automatically flattens an iterator of options when extended. struct SomeVec(Vec); @@ -105,11 +185,11 @@ impl Extend> for SomeVec { #[derive(Debug, Default)] pub struct SearchQuery { /// Keys - pub t: Vec>, + pub t: InclExcl, /// People - pub p: Vec>, + pub p: InclExcl, /// Places (locations) - pub l: Vec>, + pub l: InclExcl, pub since: QueryDateTime, pub until: QueryDateTime, pub pos: Option, @@ -117,107 +197,41 @@ pub struct SearchQuery { pub q: String, } -#[derive(Debug)] -pub struct Filter { - pub inc: bool, - pub item: T, -} - -impl Filter { - async fn load(val: &str, db: &mut AsyncPgConnection) -> Option> { - let (inc, slug) = match val.strip_prefix('!') { - Some(val) => (false, val), - None => (true, val), - }; - match T::by_slug(slug, db).await { - Ok(item) => Some(Filter { inc, item }), - Err(err) => { - warn!("No filter {:?}: {:?}", slug, err); - None - } - } - } -} - impl SearchQuery { async fn load( - query: Vec<(String, String)>, + query: RawQuery, db: &mut AsyncPgConnection, ) -> Result { - let mut result = SearchQuery::default(); - let (mut s_d, mut s_t, mut u_d, mut u_t) = (None, None, None, None); - for (key, val) in &query { - match key.as_ref() { - "since_date" => s_d = Some(val.as_ref()), - "since_time" => s_t = Some(val.as_ref()), - "until_date" => u_d = Some(val.as_ref()), - "until_time" => u_t = Some(val.as_ref()), - _ => (), - } - } - result.since = QueryDateTime::since_from_parts(s_d, s_t); - result.until = QueryDateTime::until_from_parts(u_d, u_t); - for (key, val) in query { - match key.as_ref() { - "q" => { - if val.contains("!pos") { - result.pos = Some(false); - } else if val.contains("pos") { - result.pos = Some(true); - } - result.q = val; - } - "t" => { - if let Some(f) = Filter::load(&val, db).await { - result.t.push(f); - } - } - "p" => { - if let Some(f) = Filter::load(&val, db).await { - result.p.push(f); - } - } - "l" => { - if let Some(f) = Filter::load(&val, db).await { - result.l.push(f); - } - } - "pos" => { - result.pos = match val.as_str() { - "t" => Some(true), - "!t" => Some(false), - "" => None, - val => { - warn!("Bad value for \"pos\": {:?}", val); - None - } - } - } - "from" => { - result.since = - QueryDateTime::from_img(val.parse().req("from")?, db) - .await?; - } - "to" => { - result.until = - QueryDateTime::from_img(val.parse().req("to")?, db) - .await?; - } - _ => (), // ignore unknown query parameters - } - } - Ok(result) + Ok(SearchQuery { + t: InclExcl::load(query.tags, db).await?, + p: InclExcl::load(query.people, db).await?, + l: InclExcl::load(query.locations, db).await?, + pos: query.pos, + q: query.q, + since: QueryDateTime::from_raw( + &query.since, + NaiveTime::from_hms_opt(0, 0, 0).unwrap(), + db, + ) + .await?, + until: QueryDateTime::from_raw( + &query.until, + NaiveTime::from_hms_milli_opt(23, 59, 59, 999).unwrap(), + db, + ) + .await?, + }) } fn to_base_url(&self) -> UrlString { let mut result = UrlString::new("/search/"); - for i in &self.t { - result.cond_query("t", i.inc, &i.item.slug); + for (t, i) in &self.t { + result.cond_query("t", i, &t.slug); } - for i in &self.l { - result.cond_query("l", i.inc, &i.item.slug); + for (l, i) in &self.l { + result.cond_query("l", i, &l.slug); } - for i in &self.p { - result.cond_query("p", i.inc, &i.item.slug); + for (p, i) in &self.p { + result.cond_query("p", i, &p.slug); } for i in &self.pos { result.cond_query("pos", *i, "t"); @@ -226,35 +240,90 @@ impl SearchQuery { } } +#[derive(Debug)] +pub struct InclExcl { + include: Vec, + exclude: Vec, +} +impl Default for InclExcl { + fn default() -> Self { + InclExcl { + include: Vec::new(), + exclude: Vec::new(), + } + } +} +impl<'a, T> IntoIterator for &'a InclExcl { + type Item = (&'a T, bool); + type IntoIter = Box + 'a>; + + fn into_iter(self) -> Self::IntoIter { + Box::new( + self.include + .iter() + .map(|t| (t, true)) + .chain(self.exclude.iter().map(|t| (t, false))), + ) + } +} + +impl InclExcl { + // TODO: Check that data (after optional bang) is a valid slug. Return result. + fn add(&mut self, data: String) { + match data.strip_prefix('!') { + Some(val) => self.exclude.push(val.into()), + None => self.include.push(data), + }; + } +} + +impl InclExcl { + async fn load( + val: InclExcl, + db: &mut AsyncPgConnection, + ) -> Result { + Ok(InclExcl { + include: if val.include.is_empty() { + Vec::new() + } else { + T::load_slugs(&val.include, db).await? + }, + exclude: if val.exclude.is_empty() { + Vec::new() + } else { + T::load_slugs(&val.exclude, db).await? + }, + }) + } +} + #[derive(Debug, Default)] pub struct QueryDateTime { val: Option, } impl QueryDateTime { - fn new(val: Option) -> Self { - QueryDateTime { val } - } - fn since_from_parts(date: Option<&str>, time: Option<&str>) -> Self { - let since_midnight = NaiveTime::from_hms_opt(0, 0, 0).unwrap(); - QueryDateTime::new(datetime_from_parts(date, time, since_midnight)) - } - fn until_from_parts(date: Option<&str>, time: Option<&str>) -> Self { - let until_midnight = - NaiveTime::from_hms_milli_opt(23, 59, 59, 999).unwrap(); - QueryDateTime::new(datetime_from_parts(date, time, until_midnight)) - } - async fn from_img( - photo_id: i32, + async fn from_raw( + raw: &DateTimeImg, + def_time: NaiveTime, db: &mut AsyncPgConnection, ) -> Result { - Ok(QueryDateTime::new( + let val = if let Some(img_id) = raw.img { p::photos .select(p::date) - .filter(p::id.eq(photo_id)) + .filter(p::id.eq(img_id)) .first(db) - .await?, - )) + .await? + } else { + None + }; + + Ok(QueryDateTime { + val: val.or_else(|| { + raw.date + .map(|date| date.and_time(raw.time.unwrap_or(def_time))) + }), + }) } fn as_ref(&self) -> Option<&NaiveDateTime> { self.val.as_ref() @@ -289,19 +358,3 @@ impl<'a> templates::ToHtml for QueryTimeFmt<'a> { } } } - -fn datetime_from_parts( - date: Option<&str>, - time: Option<&str>, - defaulttime: NaiveTime, -) -> Option { - date.and_then(|date| NaiveDate::parse_from_str(date, "%Y-%m-%d").ok()) - .map(|date| { - date.and_time( - time.and_then(|s| { - NaiveTime::parse_from_str(s, "%H:%M:%S").ok() - }) - .unwrap_or(defaulttime), - ) - }) -} diff --git a/templates/search.rs.html b/templates/search.rs.html index 30ab333..5bc46f7 100644 --- a/templates/search.rs.html +++ b/templates/search.rs.html @@ -10,14 +10,14 @@