Merge pull request #470 from ddotthomas/FIX/463_results_from_different_search_engines_get_cached_as_the_same_key

🐛 Different cache keys when different search engines are selected

src/models/server_models.rs

@@ -1,7 +1,11 @@
 //! This module provides the models to parse cookies and search parameters from the search
 //! engine website.
+use std::borrow::Cow;
+
 use serde::Deserialize;
 
+use super::parser_models::Style;
+
 /// A named struct which deserializes all the user provided search parameters and stores them.
 #[derive(Deserialize)]
 pub struct SearchParams {
@@ -21,11 +25,24 @@ pub struct SearchParams {
 #[derive(Deserialize)]
 pub struct Cookie<'a> {
     /// It stores the theme name used in the website.
-    pub theme: &'a str,
+    pub theme: Cow<'a, str>,
     /// It stores the colorscheme name used for the website theme.
-    pub colorscheme: &'a str,
+    pub colorscheme: Cow<'a, str>,
     /// It stores the user selected upstream search engines selected from the UI.
-    pub engines: Vec<&'a str>,
+    pub engines: Cow<'a, Vec<Cow<'a, str>>>,
     /// It stores the user selected safe search level from the UI.
     pub safe_search_level: u8,
 }
+
+impl<'a> Cookie<'a> {
+    /// server_models::Cookie contructor function
+    pub fn build(style: &'a Style, mut engines: Vec<Cow<'a, str>>, safe_search_level: u8) -> Self {
+        engines.sort();
+        Self {
+            theme: Cow::Borrowed(&style.theme),
+            colorscheme: Cow::Borrowed(&style.colorscheme),
+            engines: Cow::Owned(engines),
+            safe_search_level,
+        }
+    }
+}

src/server/routes/search.rs

@@ -6,14 +6,15 @@ use crate::{
     handler::{file_path, FileType},
     models::{
         aggregation_models::SearchResults,
-        engine_models::{EngineError, EngineHandler},
+        engine_models::EngineHandler,
-        server_models::{Cookie, SearchParams},
+        server_models::{self, SearchParams},
     },
     results::aggregator::aggregate,
 };
 use actix_web::{get, http::header::ContentType, web, HttpRequest, HttpResponse};
 use regex::Regex;
 use std::{
+    borrow::Cow,
     fs::File,
     io::{BufRead, BufReader, Read},
 };
@@ -48,16 +49,33 @@ pub async fn search(
                     .finish());
             }
 
-            let get_results = |page| {
+            let cookie = req.cookie("appCookie");
-                results(
+
-                    &config,
+            // Get search settings using the user's cookie or from the server's config
-                    &cache,
+            let mut search_settings: server_models::Cookie<'_> = cookie
-                    query,
+                .and_then(|cookie_value| serde_json::from_str(cookie_value.value()).ok())
-                    page,
+                .unwrap_or_else(|| {
-                    req.clone(),
+                    server_models::Cookie::build(
-                    &params.safesearch,
+                        &config.style,
-                )
+                        config
-            };
+                            .upstream_search_engines
+                            .iter()
+                            .filter_map(|(engine, enabled)| {
+                                enabled.then_some(Cow::Borrowed(engine.as_str()))
+                            })
+                            .collect(),
+                        config.safe_search,
+                    )
+                });
+
+            search_settings.safe_search_level = get_safesearch_level(
+                &Some(search_settings.safe_search_level),
+                &params.safesearch,
+                config.safe_search,
+            );
+
+            // Closure wrapping the results function capturing local references
+            let get_results = |page| results(&config, &cache, query, page, &search_settings);
 
             // .max(1) makes sure that the page >= 0.
             let page = params.page.unwrap_or(1).max(1) - 1;
@@ -105,25 +123,19 @@ async fn results(
     cache: &web::Data<SharedCache>,
     query: &str,
     page: u32,
-    req: HttpRequest,
+    search_settings: &server_models::Cookie<'_>,
-    safe_search: &Option<u8>,
 ) -> Result<SearchResults, Box<dyn std::error::Error>> {
     // eagerly parse cookie value to evaluate safe search level
-    let cookie_value = req.cookie("appCookie");
+    let safe_search_level = search_settings.safe_search_level;
-
-    let cookie_value: Option<Cookie<'_>> = cookie_value
-        .as_ref()
-        .and_then(|cv| serde_json::from_str(cv.name_value().1).ok());
-
-    let safe_search_level = get_safesearch_level(
-        safe_search,
-        &cookie_value.as_ref().map(|cv| cv.safe_search_level),
-        config.safe_search,
-    );
 
     let cache_key = format!(
-        "http://{}:{}/search?q={}&page={}&safesearch={}",
+        "http://{}:{}/search?q={}&page={}&safesearch={}&engines={}",
-        config.binding_ip, config.port, query, page, safe_search_level
+        config.binding_ip,
+        config.port,
+        query,
+        page,
+        safe_search_level,
+        search_settings.engines.join(",")
     );
 
     // fetch the cached results json.
@@ -151,51 +163,28 @@ async fn results(
             // default selected upstream search engines from the config file otherwise
             // parse the non-empty cookie and grab the user selected engines from the
             // UI and use that.
-            let mut results: SearchResults = match cookie_value {
+            let mut results: SearchResults = match search_settings.engines.is_empty() {
-                Some(cookie_value) => {
+                false => {
-                    let engines: Vec<EngineHandler> = cookie_value
+                    aggregate(
-                        .engines
+                        query,
-                        .iter()
+                        page,
-                        .filter_map(|name| EngineHandler::new(name).ok())
+                        config.aggregator.random_delay,
-                        .collect();
+                        config.debug,
-
+                        &search_settings
-                    match engines.is_empty() {
+                            .engines
-                        false => {
+                            .iter()
-                            aggregate(
+                            .filter_map(|engine| EngineHandler::new(&engine).ok())
-                                query,
+                            .collect::<Vec<EngineHandler>>(),
-                                page,
+                        config.request_timeout,
-                                config.aggregator.random_delay,
+                        safe_search_level,
-                                config.debug,
+                    )
-                                &engines,
+                    .await?
-                                config.request_timeout,
+                }
-                                safe_search_level,
+                true => {
-                            )
+                    let mut search_results = SearchResults::default();
-                            .await?
+                    search_results.set_no_engines_selected();
-                        }
+                    search_results
-                        true => {
-                            let mut search_results = SearchResults::default();
-                            search_results.set_no_engines_selected();
-                            search_results
-                        }
-                    }
                 }
-                None => aggregate(
-                    query,
-                    page,
-                    config.aggregator.random_delay,
-                    config.debug,
-                    &config
-                        .upstream_search_engines
-                        .clone()
-                        .into_iter()
-                        .filter_map(|(key, value)| value.then_some(key))
-                        .map(|engine| EngineHandler::new(&engine))
-                        .collect::<Result<Vec<EngineHandler>, error_stack::Report<EngineError>>>(
-                        )?,
-                    config.request_timeout,
-                    safe_search_level,
-                )
-                .await?,
             };
             if results.engine_errors_info().is_empty()
                 && results.results().is_empty()
@@ -237,23 +226,24 @@ fn is_match_from_filter_list(
     Ok(false)
 }
 
-/// A helper function which returns the safe search level based on the url params
+/// A helper function to modify the safe search level based on the url params.
-/// and cookie value.
+/// The `safe_search` is the one in the user's cookie or
+/// the default set by the server config if the cookie was missing.
 ///
 /// # Argurments
 ///
-/// * `safe_search` - Safe search level from the url.
+/// * `url_level` - Safe search level from the url.
-/// * `cookie` - User's cookie
+/// * `safe_search` - User's cookie, or the safe search level set by the server
-/// * `default` - Safe search level to fall back to
+/// * `config_level` - Safe search level to fall back to
-fn get_safesearch_level(safe_search: &Option<u8>, cookie: &Option<u8>, default: u8) -> u8 {
+fn get_safesearch_level(cookie_level: &Option<u8>, url_level: &Option<u8>, config_level: u8) -> u8 {
-    match safe_search {
+    match url_level {
-        Some(ss) => {
+        Some(url_level) => {
-            if *ss >= 3 {
+            if *url_level >= 3 {
-                default
+                config_level
             } else {
-                *ss
+                *url_level
             }
         }
-        None => cookie.unwrap_or(default),
+        None => cookie_level.unwrap_or(config_level),
     }
 }