Improve the search handler.

Mainly the inclusion/exclusion of facettes got a major rewrite.

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.
 
 

src/models.rs

@@ -257,10 +257,10 @@ impl Photo {
 
 #[async_trait]
 pub trait Facet {
-    async fn by_slug(
+    async fn load_slugs(
-        slug: &str,
+        slugs: &[String],
         db: &mut AsyncPgConnection,
-    ) -> Result<Self, Error>
+    ) -> Result<Vec<Self>, Error>
     where
         Self: Sized;
 }
@@ -274,11 +274,11 @@ pub struct Tag {
 
 #[async_trait]
 impl Facet for Tag {
-    async fn by_slug(
+    async fn load_slugs(
-        slug: &str,
+        slugs: &[String],
         db: &mut AsyncPgConnection,
-    ) -> Result<Tag, Error> {
+    ) -> Result<Vec<Tag>, Error> {
-        t::tags.filter(t::slug.eq(slug)).first(db).await
+        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(
+    async fn load_slugs(
-        slug: &str,
+        slugs: &[String],
         db: &mut AsyncPgConnection,
-    ) -> Result<Person, Error> {
+    ) -> Result<Vec<Person>, Error> {
-        h::people.filter(h::slug.eq(slug)).first(db).await
+        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(
+    async fn load_slugs(
-        slug: &str,
+        slugs: &[String],
         db: &mut AsyncPgConnection,
-    ) -> Result<Place, Error> {
+    ) -> Result<Vec<Place>, Error> {
-        l::places.filter(l::slug.eq(slug)).first(db).await
+        l::places.filter(l::slug.eq_any(slugs)).load(db).await
     }
 }
 

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<Response> {
     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));
+            .filter(pt::tag_id.eq(tag.id));
-        photos = if tag.inc {
+        photos = photos.filter(p::id.eq_any(ids));
-            photos.filter(p::id.eq_any(ids))
-        } else {
-            photos.filter(p::id.ne_all(ids))
-        };
     }
-    for location in &query.l {
+    if !query.t.exclude.is_empty() {
+        let ids = query.t.exclude.iter().map(|t| t.id).collect::<Vec<_>>();
+        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));
+            .filter(pl::place_id.eq(location.id));
-        photos = if location.inc {
+        photos = photos.filter(p::id.eq_any(ids));
-            photos.filter(p::id.eq_any(ids))
-        } else {
-            photos.filter(p::id.ne_all(ids))
-        };
     }
-    for person in &query.p {
+    if !query.l.exclude.is_empty() {
+        let ids = query.l.exclude.iter().map(|t| t.id).collect::<Vec<_>>();
+        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));
+            .filter(pp::person_id.eq(person.id));
-        photos = if person.inc {
+        photos = photos.filter(p::id.eq_any(ids));
-            photos.filter(p::id.eq_any(ids))
-        } else {
-            photos.filter(p::id.ne_all(ids))
-        }
     }
+    if !query.p.exclude.is_empty() {
+        let ids = query.p.exclude.iter().map(|t| t.id).collect::<Vec<_>>();
+        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<String>,
+    people: InclExcl<String>,
+    locations: InclExcl<String>,
+    pos: Option<bool>,
+    q: String,
+    since: DateTimeImg,
+    until: DateTimeImg,
+}
+
+impl TryFrom<Vec<(String, String)>> for RawQuery {
+    type Error = ViewError;
+
+    fn try_from(value: Vec<(String, String)>) -> Result<Self, Self::Error> {
+        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<NaiveDate>,
+    time: Option<NaiveTime>,
+    img: Option<i32>,
+}
+
 /// A `Vec` that automatically flattens an iterator of options when extended.
 struct SomeVec<T>(Vec<T>);
 
@@ -105,11 +185,11 @@ impl<T> Extend<Option<T>> for SomeVec<T> {
 #[derive(Debug, Default)]
 pub struct SearchQuery {
     /// Keys
-    pub t: Vec<Filter<Tag>>,
+    pub t: InclExcl<Tag>,
     /// People
-    pub p: Vec<Filter<Person>>,
+    pub p: InclExcl<Person>,
     /// Places (locations)
-    pub l: Vec<Filter<Place>>,
+    pub l: InclExcl<Place>,
     pub since: QueryDateTime,
     pub until: QueryDateTime,
     pub pos: Option<bool>,
@@ -117,107 +197,41 @@ pub struct SearchQuery {
     pub q: String,
 }
 
-#[derive(Debug)]
-pub struct Filter<T> {
-    pub inc: bool,
-    pub item: T,
-}
-
-impl<T: Facet> Filter<T> {
-    async fn load(val: &str, db: &mut AsyncPgConnection) -> Option<Filter<T>> {
-        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<Self> {
-        let mut result = SearchQuery::default();
+        Ok(SearchQuery {
-        let (mut s_d, mut s_t, mut u_d, mut u_t) = (None, None, None, None);
+            t: InclExcl::load(query.tags, db).await?,
-        for (key, val) in &query {
+            p: InclExcl::load(query.people, db).await?,
-            match key.as_ref() {
+            l: InclExcl::load(query.locations, db).await?,
-                "since_date" => s_d = Some(val.as_ref()),
+            pos: query.pos,
-                "since_time" => s_t = Some(val.as_ref()),
+            q: query.q,
-                "until_date" => u_d = Some(val.as_ref()),
+            since: QueryDateTime::from_raw(
-                "until_time" => u_t = Some(val.as_ref()),
+                &query.since,
-                _ => (),
+                NaiveTime::from_hms_opt(0, 0, 0).unwrap(),
-            }
+                db,
-        }
+            )
-        result.since = QueryDateTime::since_from_parts(s_d, s_t);
+            .await?,
-        result.until = QueryDateTime::until_from_parts(u_d, u_t);
+            until: QueryDateTime::from_raw(
-        for (key, val) in query {
+                &query.until,
-            match key.as_ref() {
+                NaiveTime::from_hms_milli_opt(23, 59, 59, 999).unwrap(),
-                "q" => {
+                db,
-                    if val.contains("!pos") {
+            )
-                        result.pos = Some(false);
+            .await?,
-                    } 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)
     }
     fn to_base_url(&self) -> UrlString {
         let mut result = UrlString::new("/search/");
-        for i in &self.t {
+        for (t, i) in &self.t {
-            result.cond_query("t", i.inc, &i.item.slug);
+            result.cond_query("t", i, &t.slug);
         }
-        for i in &self.l {
+        for (l, i) in &self.l {
-            result.cond_query("l", i.inc, &i.item.slug);
+            result.cond_query("l", i, &l.slug);
         }
-        for i in &self.p {
+        for (p, i) in &self.p {
-            result.cond_query("p", i.inc, &i.item.slug);
+            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<T> {
+    include: Vec<T>,
+    exclude: Vec<T>,
+}
+impl<T> Default for InclExcl<T> {
+    fn default() -> Self {
+        InclExcl {
+            include: Vec::new(),
+            exclude: Vec::new(),
+        }
+    }
+}
+impl<'a, T> IntoIterator for &'a InclExcl<T> {
+    type Item = (&'a T, bool);
+    type IntoIter = Box<dyn Iterator<Item = Self::Item> + '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<String> {
+    // 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<T: Facet> InclExcl<T> {
+    async fn load(
+        val: InclExcl<String>,
+        db: &mut AsyncPgConnection,
+    ) -> Result<Self> {
+        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<NaiveDateTime>,
 }
 
 impl QueryDateTime {
-    fn new(val: Option<NaiveDateTime>) -> Self {
+    async fn from_raw(
-        QueryDateTime { val }
+        raw: &DateTimeImg,
-    }
+        def_time: NaiveTime,
-    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,
         db: &mut AsyncPgConnection,
     ) -> Result<Self> {
-        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<NaiveDateTime> {
-    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),
-            )
-        })
-}

templates/search.rs.html

@@ -10,14 +10,14 @@
   <form class="search" action="/search/" method="get">
     <label for="s_q" accesskey="s" title="Search">🔍</label>
     <div class="refs">
-      @for p in &query.p {
+      @for (p, inc) in &query.p {
-        <label class="@if !p.inc {not }p">@p.item.person_name <input type="checkbox" name="p" value="@if !p.inc {!}@p.item.slug" checked/></label>
+        <label class="@if !inc {not }p">@p.person_name <input type="checkbox" name="p" value="@if !inc {!}@p.slug" checked/></label>
       }
-      @for t in &query.t {
+      @for (t, inc) in &query.t {
-        <label class="@if !t.inc {not }t">@t.item.tag_name <input type="checkbox" name="t" value="@if !t.inc {!}@t.item.slug" checked/></label>
+        <label class="@if !inc {not }t">@t.tag_name <input type="checkbox" name="t" value="@if !inc {!}@t.slug" checked/></label>
       }
-      @for l in &query.l {
+      @for (l, inc) in &query.l {
-        <label class="@if !l.inc {not }l">@l.item.place_name <input type="checkbox" name="l" value="@if !l.inc {!}@l.item.slug" checked/></label>
+        <label class="@if !inc {not }l">@l.place_name <input type="checkbox" name="l" value="@if !inc {!}@l.slug" checked/></label>
       }
       @if let Some(pos) = &query.pos {
         <label@if !pos { class="not"}>pos <input type="checkbox" name="pos" value="@if !pos {!}t" checked/></label>