bugfix: gracefully handle empty docs/tags when attempting to apply ta…

…gs (#304)

* bugfix: gracefully handle empty docs/tags when attempting to apply tags

* bugfix: gracefully handle regex creation failures

* fix test

crates/entities/Cargo.toml

@@ -17,6 +17,7 @@ shared = { path = "../shared" }
 strum = "0.24"
 strum_macros = "0.24"
 tantivy = "0.18"
+thiserror = "1.0.38"
 tokio = { version = "1", features = ["full"] }
 url = "2.2"
 

crates/entities/src/models/crawl_queue.rs

@@ -1,5 +1,3 @@
-use std::collections::HashSet;
-
 use regex::RegexSet;
 use sea_orm::entity::prelude::*;
 use sea_orm::sea_query::{OnConflict, Query, SqliteQueryBuilder};
@@ -8,6 +6,8 @@ use sea_orm::{
     QueryOrder, QueryTrait, Set, Statement,
 };
 use serde::{Deserialize, Serialize};
+use std::collections::HashSet;
+use thiserror::Error;
 use url::Url;
 
 use super::crawl_tag;
@@ -19,6 +19,14 @@ use shared::regex::{regex_for_domain, regex_for_prefix};
 
 const MAX_RETRIES: u8 = 5;
 
+#[derive(Debug, Error)]
+pub enum EnqueueError {
+    #[error("Database error: {0}")]
+    DbError(#[from] sea_orm::DbErr),
+    #[error("other enqueue error: {0}")]
+    Other(#[from] anyhow::Error),
+}
+
 #[derive(Debug, Clone, PartialEq, EnumIter, DeriveActiveEnum, Serialize, Deserialize, Eq)]
 #[sea_orm(rs_type = "String", db_type = "String(None)")]
 pub enum TaskErrorType {
@@ -431,7 +439,7 @@ fn filter_urls(
     settings: &UserSettings,
     overrides: &EnqueueSettings,
     urls: &[String],
-) -> Vec<String> {
+) -> anyhow::Result<Vec<String>> {
     let mut allow_list: Vec<String> = Vec::new();
     let mut skip_list: Vec<String> = Vec::new();
     let mut restrict_list: Vec<String> = Vec::new();
@@ -447,12 +455,13 @@ fn filter_urls(
         restrict_list.extend(ruleset.restrict_list);
     }
 
-    let allow_list = RegexSet::new(allow_list).expect("Unable to create allow list");
-    let skip_list = RegexSet::new(skip_list).expect("Unable to create skip list");
-    let restrict_list = RegexSet::new(restrict_list).expect("Unable to create restrict list");
+    let allow_list = RegexSet::new(allow_list)?;
+    let skip_list = RegexSet::new(skip_list)?;
+    let restrict_list = RegexSet::new(restrict_list)?;
 
     // Ignore invalid URLs
-    urls.iter()
+    let res = urls
+        .iter()
         .filter_map(|url| {
             if let Ok(mut parsed) = Url::parse(url) {
                 // Check that we can handle this scheme
@@ -496,7 +505,9 @@ fn filter_urls(
 
             None
         })
-        .collect::<Vec<String>>()
+        .collect::<Vec<String>>();
+
+    Ok(res)
 }
 
 pub async fn enqueue_local_files(
@@ -558,9 +569,9 @@ pub async fn enqueue_all(
     settings: &UserSettings,
     overrides: &EnqueueSettings,
     pipeline: Option<String>,
-) -> anyhow::Result<(), sea_orm::DbErr> {
+) -> anyhow::Result<(), EnqueueError> {
     // Filter URLs
-    let urls = filter_urls(lenses, settings, overrides, urls);
+    let urls = filter_urls(lenses, settings, overrides, urls).unwrap_or_default();
 
     // Ignore urls already indexed
     let mut is_indexed: HashSet<String> = HashSet::with_capacity(urls.len());
@@ -586,7 +597,7 @@ pub async fn enqueue_all(
                 if let Ok(parsed) = Url::parse(&url) {
                     let domain = match parsed.scheme() {
                         "file" => "localhost",
-                        _ => parsed.host_str().expect("Invalid URL host"),
+                        _ => parsed.host_str()?,
                     };
 
                     result = Some(ActiveModel {
@@ -1191,7 +1202,8 @@ mod test {
             "https://www.reddit.com/submit?title=The%20Epic%20of%20Humanity&url=https://bahaiworld.bahai.org/library/the-epic-of-humanity".into()
         ];
 
-        let mut filtered = filter_urls(&[lens], &settings, &overrides, &to_enqueue);
+        let mut filtered = filter_urls(&[lens], &settings, &overrides, &to_enqueue)
+            .expect("Unable to filter urls");
         assert_eq!(filtered.len(), 1);
         assert_eq!(
             filtered.pop(),

crates/entities/src/models/indexed_document.rs

@@ -151,7 +151,12 @@ pub async fn insert_tags_for_docs<C: ConnectionTrait>(
     db: &C,
     docs: &[Model],
     tags: &[i64],
-) -> Result<InsertResult<document_tag::ActiveModel>, DbErr> {
+) -> Result<(), DbErr> {
+    // Nothing to do if we have no docs or tags
+    if docs.is_empty() || tags.is_empty() {
+        return Ok(());
+    }
+
     // Remove dupes before adding
     let tags: HashSet<i64> = HashSet::from_iter(tags.iter().cloned());
     let doc_ids: Vec<i64> = docs.iter().map(|m| m.id).collect();
@@ -187,7 +192,9 @@ pub async fn insert_tags_for_docs<C: ConnectionTrait>(
             .to_owned(),
         )
         .exec(db)
-        .await
+        .await?;
+
+    Ok(())
 }
 
 /// Inserts an entry into the tag table for each document and
@@ -196,7 +203,7 @@ pub async fn insert_tags_many<C: ConnectionTrait>(
     db: &C,
     docs: &[Model],
     tags: &[TagPair],
-) -> Result<InsertResult<document_tag::ActiveModel>, DbErr> {
+) -> Result<(), DbErr> {
     let mut tag_ids: Vec<i64> = Vec::new();
     for (label, value) in tags.iter() {
         match get_or_create(db, label.to_owned(), value).await {

crates/spyglass/src/api/route.rs

@@ -7,15 +7,14 @@ use entities::models::{
     bootstrap_queue, connection::get_all_connections, crawl_queue, fetch_history, indexed_document,
     lens, tag,
 };
-use entities::schema::{DocFields, SearchDocument};
 use entities::sea_orm;
 use entities::sea_orm::{prelude::*, sea_query, sea_query::Expr, QueryOrder, Set};
 use entities::sea_orm::{FromQueryResult, JoinType, QuerySelect};
 use jsonrpsee::core::Error;
 use libspyglass::connection::{self, credentials, handle_authorize_connection};
 use libspyglass::filesystem;
 use libspyglass::plugin::PluginCommand;
-use libspyglass::search::Searcher;
+use libspyglass::search::{document_to_struct, Searcher};
 use libspyglass::state::AppState;
 use libspyglass::task::{AppPause, CleanupTask, ManagerCommand};
 use shared::config::Config;
@@ -406,8 +405,6 @@ pub async fn search(
         .await;
 
     let start = SystemTime::now();
-    let fields = DocFields::as_fields();
-
     let index = &state.index;
     let searcher = index.reader.searcher();
 
@@ -427,59 +424,52 @@ pub async fn search(
 
     let mut results: Vec<SearchResult> = Vec::new();
     let mut missing: Vec<(String, String)> = Vec::new();
+
+    let terms = search_req.query.split_whitespace().collect::<HashSet<_>>();
     for (score, doc_addr) in docs {
-        if let Ok(retrieved) = searcher.doc(doc_addr) {
-            let doc_id = retrieved
-                .get_first(fields.id)
-                .expect("Missing doc_id in schema");
-            let domain = retrieved
-                .get_first(fields.domain)
-                .expect("Missing domain in schema");
-            let title = retrieved
-                .get_first(fields.title)
-                .expect("Missing title in schema");
-            let description = retrieved
-                .get_first(fields.description)
-                .expect("Missing description in schema");
-            let url = retrieved
-                .get_first(fields.url)
-                .expect("Missing url in schema");
-
-            log::debug!("Got id with url {:?} {:?}", doc_id, url);
-            if let Some(doc_id) = doc_id.as_text() {
-                let indexed = indexed_document::Entity::find()
-                    .filter(indexed_document::Column::DocId.eq(doc_id))
-                    .one(&state.db)
-                    .await;
-
-                let crawl_uri = url.as_text().unwrap_or_default().to_string();
-                match indexed {
-                    Ok(Some(indexed)) => {
-                        let tags = indexed
-                            .find_related(tag::Entity)
-                            .all(&state.db)
-                            .await
-                            .unwrap_or_default()
-                            .iter()
-                            .map(|tag| (tag.label.as_ref().to_string(), tag.value.clone()))
-                            .collect::<Vec<(String, String)>>();
-
-                        let result = SearchResult {
-                            doc_id: doc_id.to_string(),
-                            domain: domain.as_text().unwrap_or_default().to_string(),
-                            title: title.as_text().unwrap_or_default().to_string(),
-                            crawl_uri: crawl_uri.clone(),
-                            description: description.as_text().unwrap_or_default().to_string(),
-                            url: indexed.open_url.unwrap_or(crawl_uri),
-                            tags,
-                            score,
-                        };
-
-                        results.push(result);
-                    }
-                    _ => {
-                        missing.push((doc_id.to_owned(), crawl_uri.to_owned()));
-                    }
+        if let Ok(Ok(doc)) = searcher.doc(doc_addr).map(|doc| document_to_struct(&doc)) {
+            log::debug!("Got id with url {} {}", doc.doc_id, doc.url);
+            let indexed = indexed_document::Entity::find()
+                .filter(indexed_document::Column::DocId.eq(doc.doc_id.clone()))
+                .one(&state.db)
+                .await;
+
+            let crawl_uri = doc.url;
+            match indexed {
+                Ok(Some(indexed)) => {
+                    let tags = indexed
+                        .find_related(tag::Entity)
+                        .all(&state.db)
+                        .await
+                        .unwrap_or_default()
+                        .iter()
+                        .map(|tag| (tag.label.as_ref().to_string(), tag.value.clone()))
+                        .collect::<Vec<(String, String)>>();
+
+                    let matched_indices = doc
+                        .content
+                        .split_whitespace()
+                        .enumerate()
+                        .filter(|(_, w)| terms.contains(w))
+                        .map(|(idx, _)| idx)
+                        .collect::<Vec<_>>();
+
+                    dbg!(matched_indices);
+                    let result = SearchResult {
+                        doc_id: doc.doc_id.clone(),
+                        domain: doc.domain,
+                        title: doc.title,
+                        crawl_uri: crawl_uri.clone(),
+                        description: doc.description,
+                        url: indexed.open_url.unwrap_or(crawl_uri),
+                        tags,
+                        score,
+                    };
+
+                    results.push(result);
+                }
+                _ => {
+                    missing.push((doc.doc_id.to_owned(), crawl_uri.to_owned()));
                 }
             }
         }

crates/spyglass/src/search/mod.rs

@@ -4,6 +4,7 @@ use std::path::PathBuf;
 use std::sync::{Arc, Mutex};
 use std::time::Instant;
 
+use anyhow::anyhow;
 use migration::{Expr, Func};
 use tantivy::collector::TopDocs;
 use tantivy::directory::MmapDirectory;
@@ -320,6 +321,45 @@ async fn get_tag_checks(db: &DatabaseConnection, search: &str) -> Option<Vec<i64
     None
 }
 
+pub struct RetrievedDocument {
+    pub doc_id: String,
+    pub domain: String,
+    pub title: String,
+    pub description: String,
+    pub content: String,
+    pub url: String,
+}
+
+fn field_to_string(doc: &Document, field: Field) -> String {
+    doc.get_first(field)
+        .map(|x| x.as_text().unwrap_or_default())
+        .map(|x| x.to_string())
+        .unwrap_or_default()
+}
+
+pub fn document_to_struct(doc: &Document) -> anyhow::Result<RetrievedDocument> {
+    let fields = DocFields::as_fields();
+    let doc_id = field_to_string(doc, fields.id);
+    if doc_id.is_empty() {
+        return Err(anyhow!("Invalid doc_id"));
+    }
+
+    let domain = field_to_string(doc, fields.domain);
+    let title = field_to_string(doc, fields.title);
+    let description = field_to_string(doc, fields.description);
+    let url = field_to_string(doc, fields.url);
+    let content = field_to_string(doc, fields.content);
+
+    Ok(RetrievedDocument {
+        doc_id,
+        domain,
+        title,
+        description,
+        content,
+        url,
+    })
+}
+
 #[cfg(test)]
 mod test {
     use crate::search::{DocumentUpdate, IndexPath, Searcher};