Refactor Config and ConfigBuilder (#71)

* Replace &str with Into<String> in ConfigBuilder

Co-Authored-By: Paul Horn <dev@knutwalker.engineer>

* Use AsRef<str> in Graph constructor

Co-Authored-By: Paul Horn <dev@knutwalker.engineer>

* Implement Default for ConfigBuilder

* Improve docs in config.rs

* Fix docs in Graph

* Improve ConfigBuilder::build

* Use `neo4j` as default database name

* Add GraphConfig::new and improve docs a bit more.

* Remove unnecessary 'static qualifier from const

* Remove Option from parameters where we provide a default

* Replace async tests with sync tests

* Use same Into<String> bound in Graph as in Config

---------

Co-authored-by: Paul Horn <dev@knutwalker.engineer>

Author: s1ck

lib/src/config.rs

@@ -1,9 +1,10 @@
 pub use crate::errors::*;
 
+const DEFAULT_DATABASE: &str = "neo4j";
 const DEFAULT_FETCH_SIZE: usize = 200;
 const DEFAULT_MAX_CONNECTIONS: usize = 16;
 
-/// The configuration used to connect to the database, see [`crate::Graph::connect`]
+/// The configuration used to connect to the database, see [`crate::Graph::connect`].
 #[derive(Debug, Clone)]
 pub struct Config {
     pub(crate) uri: String,
@@ -14,97 +15,101 @@ pub struct Config {
     pub(crate) fetch_size: usize,
 }
 
-/// A builder to override default configurations and build the [`Config`]
+/// A builder to override default configurations and build the [`Config`].
 pub struct ConfigBuilder {
     uri: Option<String>,
     user: Option<String>,
     password: Option<String>,
-    db: Option<String>,
-    fetch_size: Option<usize>,
-    max_connections: Option<usize>,
+    db: String,
+    fetch_size: usize,
+    max_connections: usize,
 }
 
 impl ConfigBuilder {
-    ///the uri of the neo4j server
-    pub fn uri(mut self, uri: &str) -> Self {
-        self.uri = Some(uri.to_owned());
+    /// Creates a new `ConfigBuilder` with default values.
+    pub fn new() -> Self {
+        Self::default()
+    }
+
+    /// The uri of the Neo4j server, e.g. "127.0.0.1:7687".
+    pub fn uri(mut self, uri: impl Into<String>) -> Self {
+        self.uri = Some(uri.into());
         self
     }
 
-    ///username for authentication
-    pub fn user(mut self, user: &str) -> Self {
-        self.user = Some(user.to_owned());
+    /// The username for authenticating with the Neo4j server.
+    pub fn user(mut self, user: impl Into<String>) -> Self {
+        self.user = Some(user.into());
         self
     }
 
-    ///password for authentication
-    pub fn password(mut self, password: &str) -> Self {
-        self.password = Some(password.to_owned());
+    /// The password for authenticating with the Neo4j server.
+    pub fn password(mut self, password: impl Into<String>) -> Self {
+        self.password = Some(password.into());
         self
     }
 
-    ///the name of the database, defaults to "neo4j" if not configured.
-    pub fn db(mut self, db: &str) -> Self {
-        self.db = Some(db.to_owned());
+    /// The name of the database to connect to.
+    ///
+    /// Defaults to "neo4j" if not set.
+    pub fn db(mut self, db: impl Into<String>) -> Self {
+        self.db = db.into();
         self
     }
 
-    ///fetch_size indicates the number of rows to fetch from server in one request, it is
-    ///recommended to use a large fetch_size if you are working with large data sets.
-    ///default fetch_size is 200
+    /// `fetch_size` indicates the number of rows to fetch from server in one request.
+    /// It is recommended to use a large `fetch_size` if you are working with large data sets.
+    ///
+    /// Defaults to 200 if not set.
     pub fn fetch_size(mut self, fetch_size: usize) -> Self {
-        self.fetch_size = Some(fetch_size);
+        self.fetch_size = fetch_size;
         self
     }
 
-    ///maximum number of connections in the connection pool
+    /// The maximum number of connections in the connection pool.
+    ///
+    /// Defaults to 16 if not set.
     pub fn max_connections(mut self, max_connections: usize) -> Self {
-        self.max_connections = Some(max_connections);
+        self.max_connections = max_connections;
         self
     }
 
     pub fn build(self) -> Result<Config> {
-        if self.uri.is_none()
-            || self.user.is_none()
-            || self.password.is_none()
-            || self.fetch_size.is_none()
-            || self.max_connections.is_none()
-            || self.db.is_none()
-        {
-            Err(Error::InvalidConfig)
-        } else {
-            //The config attributes are validated before unwrapping
+        if let (Some(uri), Some(user), Some(password)) = (self.uri, self.user, self.password) {
             Ok(Config {
-                uri: self.uri.unwrap(),
-                user: self.user.unwrap(),
-                password: self.password.unwrap(),
-                fetch_size: self.fetch_size.unwrap(),
-                max_connections: self.max_connections.unwrap(),
-                db: self.db.unwrap(),
+                uri,
+                user,
+                password,
+                fetch_size: self.fetch_size,
+                max_connections: self.max_connections,
+                db: self.db,
             })
+        } else {
+            Err(Error::InvalidConfig)
         }
     }
 }
 
-/// Creates a config builder with reasonable default values wherever appropriate.
-pub fn config() -> ConfigBuilder {
-    ConfigBuilder {
-        uri: None,
-        user: None,
-        password: None,
-        db: Some("".to_owned()),
-        max_connections: Some(DEFAULT_MAX_CONNECTIONS),
-        fetch_size: Some(DEFAULT_FETCH_SIZE),
+impl Default for ConfigBuilder {
+    fn default() -> Self {
+        ConfigBuilder {
+            uri: None,
+            user: None,
+            password: None,
+            db: DEFAULT_DATABASE.into(),
+            max_connections: DEFAULT_MAX_CONNECTIONS,
+            fetch_size: DEFAULT_FETCH_SIZE,
+        }
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
 
-    #[tokio::test]
-    async fn should_build_config() {
-        let config = config()
+    #[test]
+    fn should_build_config() {
+        let config = ConfigBuilder::default()
             .uri("127.0.0.1:7687")
             .user("some_user")
             .password("some_password")
@@ -121,9 +126,9 @@ mod tests {
         assert_eq!(config.max_connections, 5);
     }
 
-    #[tokio::test]
-    async fn should_build_with_defaults() {
-        let config = config()
+    #[test]
+    fn should_build_with_defaults() {
+        let config = ConfigBuilder::default()
             .uri("127.0.0.1:7687")
             .user("some_user")
             .password("some_password")
@@ -132,26 +137,26 @@ mod tests {
         assert_eq!(config.uri, "127.0.0.1:7687");
         assert_eq!(config.user, "some_user");
         assert_eq!(config.password, "some_password");
-        assert_eq!(config.db, "");
+        assert_eq!(config.db, "neo4j");
         assert_eq!(config.fetch_size, 200);
         assert_eq!(config.max_connections, 16);
     }
 
-    #[tokio::test]
-    async fn should_reject_invalid_config() {
-        assert!(config()
+    #[test]
+    fn should_reject_invalid_config() {
+        assert!(ConfigBuilder::default()
             .user("some_user")
             .password("some_password")
             .build()
             .is_err());
 
-        assert!(config()
+        assert!(ConfigBuilder::default()
             .uri("127.0.0.1:7687")
             .password("some_password")
             .build()
             .is_err());
 
-        assert!(config()
+        assert!(ConfigBuilder::default()
             .uri("127.0.0.1:7687")
             .user("some_user")
             .build()

lib/src/graph.rs

@@ -1,4 +1,4 @@
-use crate::config::{config, Config};
+use crate::config::{Config, ConfigBuilder};
 use crate::errors::*;
 use crate::pool::{create_pool, ConnectionPool};
 use crate::query::Query;
@@ -19,16 +19,25 @@ pub fn query(q: &str) -> Query {
 }
 
 impl Graph {
-    /// Connects to the database with configurations provided, you can build a config using
-    /// [`config`]
+    /// Connects to the database with configurations provided.
+    ///
+    /// You can build a config using [`ConfigBuilder::default()`].
     pub async fn connect(config: Config) -> Result<Self> {
         let pool = create_pool(&config).await?;
         Ok(Graph { config, pool })
     }
 
     /// Connects to the database with default configurations
-    pub async fn new(uri: &str, user: &str, password: &str) -> Result<Self> {
-        let config = config().uri(uri).user(user).password(password).build()?;
+    pub async fn new(
+        uri: impl Into<String>,
+        user: impl Into<String>,
+        password: impl Into<String>,
+    ) -> Result<Self> {
+        let config = ConfigBuilder::default()
+            .uri(uri)
+            .user(user)
+            .password(password)
+            .build()?;
         Self::connect(config).await
     }
 

lib/src/lib.rs

@@ -66,7 +66,7 @@
 //!
 //! #[tokio::main]
 //! async fn main() {
-//!    let config = config()
+//!    let config = ConfigBuilder::default()
 //!        .uri("127.0.0.1:7687")
 //!        .user("neo4j")
 //!        .password("neo")
@@ -172,7 +172,7 @@
 //!
 //! #[tokio::main]
 //! async fn main() {
-//!    let config = config()
+//!    let config = ConfigBuilder::default()
 //!        .uri("127.0.0.1:7687")
 //!        .user("neo4j")
 //!        .password("neo")
@@ -764,7 +764,7 @@ mod txn;
 mod types;
 mod version;
 
-pub use crate::config::{config, Config, ConfigBuilder};
+pub use crate::config::{Config, ConfigBuilder};
 pub use crate::errors::*;
 pub use crate::graph::{query, Graph};
 pub use crate::query::Query;

lib/tests/configurations.rs

@@ -4,7 +4,10 @@ mod container;
 
 #[tokio::test]
 async fn configurations() {
-    let config = config().db("neo4j").fetch_size(500).max_connections(10);
+    let config = ConfigBuilder::default()
+        .db("neo4j")
+        .fetch_size(500)
+        .max_connections(10);
     let neo4j = container::Neo4jContainer::from_config(config).await;
     let graph = neo4j.graph();
 

lib/tests/container.rs

@@ -1,5 +1,5 @@
 use lenient_semver::Version;
-use neo4rs::{config, ConfigBuilder, Graph};
+use neo4rs::{ConfigBuilder, Graph};
 use testcontainers::{clients::Cli, core::WaitFor, Container, Image};
 
 use std::{collections::HashMap, sync::Arc};
@@ -18,7 +18,7 @@ impl Neo4jContainer {
 
     #[allow(dead_code)]
     pub async fn from_version(version: impl Into<String>) -> Self {
-        Self::from_config_and_version(config(), version).await
+        Self::from_config_and_version(ConfigBuilder::default(), version).await
     }
 
     #[allow(dead_code)]

lib/tests/streams_within_a_transaction.rs

@@ -5,7 +5,7 @@ mod container;
 
 #[tokio::test]
 async fn streams_within_a_transaction() {
-    let config = config().fetch_size(1);
+    let config = ConfigBuilder::default().fetch_size(1);
     let neo4j = container::Neo4jContainer::from_config(config).await;
     let graph = neo4j.graph();