Move popgetter cache to lib with cache feature flag

Moves the constructor that uses cached metadata from cli to lib crates to enable its use from the Python bindings crate.
Urban-Analytics-Technology-Platform · Sep 5, 2024 · 93e5a81 · 93e5a81
1 parent 4a95ec2
commit 93e5a81
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 39 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/popgetter/Cargo.toml b/popgetter/Cargo.toml
@@ -25,11 +25,12 @@ thiserror = { workspace = true }
 tokio = { workspace = true, features = ["full"] }
 wkb = { workspace = true }
 wkt = { workspace = true }
+xdg = { workspace = true, optional = true}
 
 [dev-dependencies]
 tempfile = { workspace = true }
 
 [features]
 default = ["cache", "formatters"]
-cache = []
+cache = ["dep:xdg"]
 formatters = ["dep:geojson"]
diff --git a/popgetter/src/lib.rs b/popgetter/src/lib.rs
@@ -1,8 +1,10 @@
 use std::path::Path;
 
+#[cfg(feature = "cache")]
+use anyhow::Context;
 use anyhow::Result;
 use data_request_spec::DataRequestSpec;
-use log::debug;
+use log::{debug, error};
 use metadata::Metadata;
 use polars::frame::DataFrame;
 use search::{Params, SearchParams, SearchResults};
@@ -47,8 +49,45 @@ impl Popgetter {
     // Only include method with "cache" feature since it requires a filesystem
     #[cfg(feature = "cache")]
     /// Setup the Popgetter object with custom configuration from cache
-    pub fn new_with_config_and_cache<P: AsRef<Path>>(config: Config, cache: P) -> Result<Self> {
-        let metadata = Metadata::from_cache(cache)?;
+    pub async fn new_with_config_and_cache(config: Config) -> Result<Self> {
+        let xdg_dirs = xdg::BaseDirectories::with_prefix("popgetter")?;
+        let path = xdg_dirs.get_cache_home();
+        Popgetter::new_with_config_and_cache_path(config, path).await
+    }
+
+    // Only include method with "cache" feature since it requires a filesystem
+    #[cfg(feature = "cache")]
+    async fn new_with_config_and_cache_path<P: AsRef<Path>>(
+        config: Config,
+        path: P,
+    ) -> Result<Self> {
+        // Try to read metadata from cache
+        if path.as_ref().exists() {
+            match Popgetter::new_from_cache_path(config.clone(), &path) {
+                Ok(popgetter) => return Ok(popgetter),
+                Err(err) => {
+                    // Log error, continue without cache and attempt to create one
+                    error!("Failed to read metadata from cache with error: {err}");
+                }
+            }
+        }
+        // If no metadata cache, get metadata and try to cache
+        std::fs::create_dir_all(&path)?;
+        let popgetter = Popgetter::new_with_config(config).await?;
+        // If error creating cache, remove cache path
+        if let Err(err) = popgetter.metadata.write_cache(&path) {
+            std::fs::remove_dir_all(&path).with_context(|| {
+                "Failed to remove cache dir following error writing cache: {err}"
+            })?;
+            Err(err)?
+        }
+        Ok(popgetter)
+    }
+
+    // Only include method with "cache" feature since it requires a filesystem
+    #[cfg(feature = "cache")]
+    fn new_from_cache_path<P: AsRef<Path>>(config: Config, path: P) -> Result<Self> {
+        let metadata = Metadata::from_cache(path)?;
         Ok(Self { metadata, config })
     }
 
@@ -94,7 +133,8 @@ mod tests {
         let config = Config::default();
         let popgetter = Popgetter::new_with_config(config.clone()).await?;
         popgetter.metadata.write_cache(&tempdir)?;
-        let popgetter_from_cache = Popgetter::new_with_config_and_cache(config, &tempdir)?;
+        let popgetter_from_cache =
+            Popgetter::new_with_config_and_cache_path(config, tempdir).await?;
         assert_eq!(popgetter, popgetter_from_cache);
         Ok(())
     }

diff --git a/popgetter_cli/src/cli.rs b/popgetter_cli/src/cli.rs
@@ -1,9 +1,9 @@
 // FromStr is required by EnumString. The compiler seems to not be able to
 // see that and so is giving a warning. Dont remove it
-use anyhow::{Context, Result};
+use anyhow::Result;
 use clap::{Args, Parser, Subcommand};
 use enum_dispatch::enum_dispatch;
-use log::{debug, error, info};
+use log::{debug, info};
 use nonempty::nonempty;
 use polars::frame::DataFrame;
 use popgetter::{
@@ -144,33 +144,6 @@ impl From<OutputFormat> for OutputFormatter {
     }
 }
 
-async fn read_popgetter(config: Config) -> anyhow::Result<Popgetter> {
-    let xdg_dirs = xdg::BaseDirectories::with_prefix("popgetter")?;
-    // TODO: enable cache to be optional
-    let path = xdg_dirs.get_cache_home();
-    // Try to read metadata from cache
-    if path.exists() {
-        match Popgetter::new_with_config_and_cache(config.clone(), &path) {
-            Ok(popgetter) => return Ok(popgetter),
-            Err(err) => {
-                // Log error, continue without cache and attempt to create one
-                error!("Failed to read metadata from cache with error: {err}");
-            }
-        }
-    }
-    // If no metadata cache, get metadata and try to cache
-    std::fs::create_dir_all(&path)?;
-    let popgetter = Popgetter::new_with_config(config).await?;
-
-    // If error creating cache, remove cache path
-    if let Err(err) = popgetter.metadata.write_cache(&path) {
-        std::fs::remove_dir_all(&path)
-            .with_context(|| "Failed to remove cache dir following error writing cache: {err}")?;
-        Err(err)?
-    }
-    Ok(popgetter)
-}
-
 impl RunCommand for DataCommand {
     async fn run(&self, config: Config) -> Result<()> {
         info!("Running `data` subcommand");
@@ -180,7 +153,7 @@ impl RunCommand for DataCommand {
                 DOWNLOADING_SEARCHING_STRING.to_string() + RUNNING_TAIL_STRING,
             )
         });
-        let popgetter = read_popgetter(config).await?;
+        let popgetter = Popgetter::new_with_config_and_cache(config).await?;
         let search_params: SearchParams = self.search_params_args.clone().into();
         let search_results = popgetter.search(&search_params);
 
@@ -387,7 +360,7 @@ impl RunCommand for MetricsCommand {
                 DOWNLOADING_SEARCHING_STRING.into(),
             )
         });
-        let popgetter = read_popgetter(config).await?;
+        let popgetter = Popgetter::new_with_config_and_cache(config).await?;
         let search_results = popgetter.search(&self.search_params_args.to_owned().into());
         if let Some(mut s) = sp {
             s.stop_with_symbol(COMPLETE_PROGRESS_STRING);
@@ -427,7 +400,7 @@ impl RunCommand for CountriesCommand {
                 spinner_message.to_string() + RUNNING_TAIL_STRING,
             )
         });
-        let popgetter = read_popgetter(config).await?;
+        let popgetter = Popgetter::new_with_config_and_cache(config).await?;
         if let Some(mut s) = sp {
             s.stop_with_symbol(COMPLETE_PROGRESS_STRING);
         }

diff --git a/popgetter_py/src/lib.rs b/popgetter_py/src/lib.rs
@@ -1,6 +1,7 @@
 use std::default::Default;
 
 use ::popgetter::{
+    config::Config,
     data_request_spec::DataRequestSpec,
     search::{SearchParams, SearchText},
     COL,
@@ -32,7 +33,9 @@ fn convert_py_dict<T: DeserializeOwned>(obj: &Bound<'_, PyAny>) -> PyResult<T> {
 
 /// Returns search results as a `DataFrame` from given `SearchParams`.
 async fn _search(search_params: SearchParams) -> DataFrame {
-    let popgetter = ::popgetter::Popgetter::new().await.unwrap();
+    let popgetter = ::popgetter::Popgetter::new_with_config_and_cache(Config::default())
+        .await
+        .unwrap();
     let search_results = popgetter.search(&search_params);
     search_results
         .0
@@ -75,7 +78,9 @@ fn get_search_params(obj: &Bound<'_, PyAny>) -> PyResult<SearchParams> {
 
 /// Downloads data as a `DataFrame` for a given `DataRequestSpec`.
 async fn _download_data_request_spec(data_request: DataRequestSpec) -> DataFrame {
-    let popgetter = ::popgetter::Popgetter::new().await.unwrap();
+    let popgetter = ::popgetter::Popgetter::new_with_config_and_cache(Config::default())
+        .await
+        .unwrap();
     popgetter
         .download_data_request_spec(&data_request)
         .await