RUST-591 Use separate ConnectionPoolOptions types for monitoring and internal usage (#326)

Author: patrickfreed

src/cmap/mod.rs

@@ -78,7 +78,7 @@ impl ConnectionPool {
         if let Some(ref handler) = event_handler {
             handler.handle_pool_created_event(PoolCreatedEvent {
                 address: address.clone(),
-                options,
+                options: options.map(|o| o.to_event_options()),
             });
         };
 

src/cmap/options.rs

@@ -7,44 +7,43 @@ use typed_builder::TypedBuilder;
 use crate::{
     bson_util,
     client::{auth::Credential, options::ServerApi},
-    event::cmap::CmapEventHandler,
+    event::cmap::{CmapEventHandler, ConnectionPoolOptions as EventOptions},
     options::{ClientOptions, DriverInfo, StreamAddress, TlsOptions},
 };
 
-/// Contains the options for creating a connection pool. While these options are specified at the
-/// client-level, `ConnectionPoolOptions` is exposed for the purpose of CMAP event handling.
+/// Contains the options for creating a connection pool.
 #[derive(Clone, Default, Deserialize, TypedBuilder, Derivative)]
 #[derivative(Debug, PartialEq)]
 #[builder(field_defaults(default, setter(strip_option)))]
 #[serde(rename_all = "camelCase")]
-pub struct ConnectionPoolOptions {
+pub(crate) struct ConnectionPoolOptions {
     /// The application name specified by the user. This is sent to the server as part of the
     /// handshake that each connection makes when it's created.
-    pub app_name: Option<String>,
+    pub(crate) app_name: Option<String>,
 
-    /// The connect timeout passed to each underlying TcpStream when attemtping to connect to the
+    /// The connect timeout passed to each underlying TcpStream when attempting to connect to the
     /// server.
     #[serde(skip)]
-    pub connect_timeout: Option<Duration>,
+    pub(crate) connect_timeout: Option<Duration>,
 
     /// The credential to use for authenticating connections in this pool.
     #[serde(skip)]
-    pub credential: Option<Credential>,
+    pub(crate) credential: Option<Credential>,
 
     /// Extra information to append to the driver version in the metadata of the handshake with the
     /// server. This should be used by libraries wrapping the driver, e.g. ODMs.
     #[serde(skip)]
-    pub driver_info: Option<DriverInfo>,
+    pub(crate) driver_info: Option<DriverInfo>,
 
     /// Processes all events generated by the pool.
     #[derivative(Debug = "ignore", PartialEq = "ignore")]
     #[serde(skip)]
-    pub event_handler: Option<Arc<dyn CmapEventHandler>>,
+    pub(crate) event_handler: Option<Arc<dyn CmapEventHandler>>,
 
     /// How often the background thread performs its maintenance (e.g. ensure minPoolSize).
     #[cfg(test)]
     #[serde(skip)]
-    pub maintenance_frequency: Option<Duration>,
+    pub(crate) maintenance_frequency: Option<Duration>,
 
     /// Connections that have been ready for usage in the pool for longer than `max_idle_time` will
     /// not be used.
@@ -53,20 +52,20 @@ pub struct ConnectionPoolOptions {
     #[serde(rename = "maxIdleTimeMS")]
     #[serde(default)]
     #[serde(deserialize_with = "bson_util::deserialize_duration_from_u64_millis")]
-    pub max_idle_time: Option<Duration>,
+    pub(crate) max_idle_time: Option<Duration>,
 
     /// The maximum number of connections that the pool can have at a given time. This includes
     /// connections which are currently checked out of the pool.
     ///
     /// The default is 100.
-    pub max_pool_size: Option<u32>,
+    pub(crate) max_pool_size: Option<u32>,
 
     /// The minimum number of connections that the pool can have at a given time. This includes
     /// connections which are currently checked out of the pool. If fewer than `min_pool_size`
     /// connections are in the pool, connections will be added to the pool in the background.
     ///
     /// The default is that no minimum is enforced
-    pub min_pool_size: Option<u32>,
+    pub(crate) min_pool_size: Option<u32>,
 
     /// Whether to start the pool as "ready" or not.
     /// For tests only.
@@ -85,7 +84,7 @@ pub struct ConnectionPoolOptions {
     ///
     /// The default is not to use TLS for connections.
     #[serde(skip)]
-    pub tls_options: Option<TlsOptions>,
+    pub(crate) tls_options: Option<TlsOptions>,
 
     /// Rather than wait indefinitely for a connection to become available, instead return an error
     /// after the given duration.
@@ -94,23 +93,42 @@ pub struct ConnectionPoolOptions {
     #[serde(rename = "waitQueueTimeoutMS")]
     #[serde(default)]
     #[serde(deserialize_with = "bson_util::deserialize_duration_from_u64_millis")]
-    pub wait_queue_timeout: Option<Duration>,
+    pub(crate) wait_queue_timeout: Option<Duration>,
 }
 
 impl ConnectionPoolOptions {
     pub(crate) fn from_client_options(options: &ClientOptions) -> Self {
-        let mut pool_options = Self::builder().build();
-        pool_options.app_name = options.app_name.clone();
-        pool_options.connect_timeout = options.connect_timeout;
-        pool_options.credential = options.credential.clone();
-        pool_options.driver_info = options.driver_info.clone();
-        pool_options.event_handler = options.cmap_event_handler.clone();
-        pool_options.max_idle_time = options.max_idle_time;
-        pool_options.min_pool_size = options.min_pool_size;
-        pool_options.tls_options = options.tls_options();
-        pool_options.wait_queue_timeout = options.wait_queue_timeout;
-
-        pool_options
+        Self {
+            app_name: options.app_name.clone(),
+            connect_timeout: options.connect_timeout,
+            driver_info: options.driver_info.clone(),
+            max_idle_time: options.max_idle_time,
+            min_pool_size: options.min_pool_size,
+            max_pool_size: options.max_pool_size,
+            server_api: options.server_api.clone(),
+            tls_options: options.tls_options(),
+            wait_queue_timeout: options.wait_queue_timeout,
+            credential: options.credential.clone(),
+            event_handler: options.cmap_event_handler.clone(),
+            #[cfg(test)]
+            maintenance_frequency: None,
+            #[cfg(test)]
+            ready: None,
+        }
+    }
+
+    pub(crate) fn to_event_options(&self) -> EventOptions {
+        EventOptions {
+            app_name: self.app_name.clone(),
+            connect_timeout: self.connect_timeout,
+            driver_info: self.driver_info.clone(),
+            max_idle_time: self.max_idle_time,
+            min_pool_size: self.min_pool_size,
+            max_pool_size: self.max_pool_size,
+            server_api: self.server_api.clone(),
+            tls_options: self.tls_options.clone(),
+            wait_queue_timeout: self.wait_queue_timeout,
+        }
     }
 }
 

src/cmap/test/file.rs

@@ -17,7 +17,7 @@ pub struct TestFile {
     version: u8,
     style: TestStyle,
     pub description: String,
-    pub pool_options: Option<ConnectionPoolOptions>,
+    pub(crate) pool_options: Option<ConnectionPoolOptions>,
     pub operations: Vec<ThreadedOperation>,
     pub error: Option<Error>,
     pub events: Vec<Event>,

src/cmap/test/mod.rs

@@ -12,8 +12,9 @@ use self::{
 };
 
 use crate::{
-    cmap::{options::ConnectionPoolOptions, Connection, ConnectionPool},
+    cmap::{Connection, ConnectionPool, ConnectionPoolOptions},
     error::{Error, Result},
+    event::cmap::ConnectionPoolOptions as EventOptions,
     options::TlsOptions,
     runtime::AsyncJoinHandle,
     sdam::{ServerUpdate, ServerUpdateSender},
@@ -329,11 +330,10 @@ impl Matchable for TlsOptions {
     }
 }
 
-impl Matchable for ConnectionPoolOptions {
-    fn content_matches(&self, expected: &ConnectionPoolOptions) -> bool {
+impl Matchable for EventOptions {
+    fn content_matches(&self, expected: &EventOptions) -> bool {
         self.app_name.matches(&expected.app_name)
             && self.connect_timeout.matches(&expected.connect_timeout)
-            && self.credential.matches(&expected.credential)
             && self.max_idle_time.matches(&expected.max_idle_time)
             && self.max_pool_size.matches(&expected.max_pool_size)
             && self.min_pool_size.matches(&expected.min_pool_size)

src/event/cmap.rs

@@ -1,10 +1,14 @@
 //! Contains the events and functionality for monitoring behavior of the connection pooling of a
 //! `Client`.
 
+use std::time::Duration;
+
 use serde::Deserialize;
 
-pub use crate::cmap::options::ConnectionPoolOptions;
-use crate::options::StreamAddress;
+use crate::{
+    client::options::{DriverInfo, ServerApi, TlsOptions},
+    options::StreamAddress,
+};
 
 /// We implement `Deserialize` for all of the event types so that we can more easily parse the CMAP
 /// spec tests. However, we have no need to parse the address field from the JSON files (if it's
@@ -31,6 +35,70 @@ pub struct PoolCreatedEvent {
     pub options: Option<ConnectionPoolOptions>,
 }
 
+/// Contains the options for creating a connection pool. While these options are specified at the
+/// client-level, `ConnectionPoolOptions` is exposed for the purpose of CMAP event handling.
+#[derive(Clone, Default, Deserialize, Debug, PartialEq)]
+#[serde(rename_all = "camelCase")]
+#[non_exhaustive]
+pub struct ConnectionPoolOptions {
+    /// The application name specified by the user. This is sent to the server as part of the
+    /// handshake that each connection makes when it's created.
+    pub app_name: Option<String>,
+
+    /// The connect timeout passed to each underlying TcpStream when attempting to connect to the
+    /// server.
+    #[serde(skip)]
+    pub connect_timeout: Option<Duration>,
+
+    /// Extra information to append to the driver version in the metadata of the handshake with the
+    /// server. This should be used by libraries wrapping the driver, e.g. ODMs.
+    #[serde(skip)]
+    pub driver_info: Option<DriverInfo>,
+
+    /// Connections that have been ready for usage in the pool for longer than `max_idle_time` will
+    /// not be used.
+    ///
+    /// The default is that connections will not be closed due to being idle.
+    #[serde(rename = "maxIdleTimeMS")]
+    #[serde(default)]
+    #[serde(deserialize_with = "crate::bson_util::deserialize_duration_from_u64_millis")]
+    pub max_idle_time: Option<Duration>,
+
+    /// The maximum number of connections that the pool can have at a given time. This includes
+    /// connections which are currently checked out of the pool.
+    ///
+    /// The default is 100.
+    pub max_pool_size: Option<u32>,
+
+    /// The minimum number of connections that the pool can have at a given time. This includes
+    /// connections which are currently checked out of the pool. If fewer than `min_pool_size`
+    /// connections are in the pool, connections will be added to the pool in the background.
+    ///
+    /// The default is that no minimum is enforced
+    pub min_pool_size: Option<u32>,
+
+    /// The declared API version
+    ///
+    /// The default value is to have no declared API version
+    pub(crate) server_api: Option<ServerApi>,
+
+    /// The options specifying how a TLS connection should be configured. If `tls_options` is
+    /// `None`, then TLS will not be used for the connections.
+    ///
+    /// The default is not to use TLS for connections.
+    #[serde(skip)]
+    pub tls_options: Option<TlsOptions>,
+
+    /// Rather than wait indefinitely for a connection to become available, instead return an error
+    /// after the given duration.
+    ///
+    /// The default is to block indefinitely until a connection becomes available.
+    #[serde(rename = "waitQueueTimeoutMS")]
+    #[serde(default)]
+    #[serde(deserialize_with = "crate::bson_util::deserialize_duration_from_u64_millis")]
+    pub wait_queue_timeout: Option<Duration>,
+}
+
 /// Event emitted when a connection pool becomes ready.
 #[derive(Clone, Debug, Deserialize, PartialEq)]
 #[non_exhaustive]