From 36d0c0504e7fc552ee1437bb6ce25cec70d68bf2 Mon Sep 17 00:00:00 2001 From: Nadja Reitzenstein Date: Mon, 6 Feb 2023 12:08:41 +0100 Subject: [PATCH 1/3] Implementing capability caching --- src/client.rs | 96 ++++++++++++++++++++++++++++++++++++++- src/types/capabilities.rs | 22 +++++++++ 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/src/client.rs b/src/client.rs index 7969497..7526848 100644 --- a/src/client.rs +++ b/src/client.rs @@ -138,6 +138,11 @@ fn validate_sequence_set( #[derive(Debug)] pub struct Session { conn: Connection, + + // Capabilities are almost guaranteed to chance if encryption state or authentication state + // changes, so caching them in `Connection` is inappropiate. + capability_cache: Option, + pub(crate) unsolicited_responses_tx: mpsc::Sender, /// Server responses that are not related to the current command. See also the note on @@ -153,6 +158,10 @@ pub struct Session { #[derive(Debug)] pub struct Client { conn: Connection, + + // Capabilities are almost guaranteed to chance if encryption state or authentication state + // changes, so caching them in `Connection` is inappropiate. + capability_cache: Option, } /// The underlying primitives type. Both `Client`(unauthenticated) and `Session`(after succesful @@ -333,6 +342,7 @@ impl Client { debug: false, greeting_read: false, }, + capability_cache: None, } } @@ -345,6 +355,47 @@ impl Client { Ok(res) } + /// The [`CAPABILITY` command](https://tools.ietf.org/html/rfc3501#section-6.1.1) requests a + /// listing of capabilities that the server supports. The server will include "IMAP4rev1" as + /// one of the listed capabilities. See [`Capabilities`] for further details. + /// + /// This method will always bypass the local capabilities cache and send a `CAPABILITY` command + /// to the server. The [`Self::capabilities()`] method can be used when returning a cached + /// response is acceptable. + pub fn capabilities_refresh(&mut self) -> Result<&Capabilities> { + let (mut tx, _rx) = mpsc::channel(); + let caps = self.run_command_and_read_response("CAPABILITY") + .and_then(|lines| Capabilities::parse(lines, &mut tx))?; + self.capability_cache = Some(caps); + + self.capability_cache.as_ref() + // This path will not be hit; if the cache is not populated the above calls will either + // populate it or return with an early error. + .ok_or_else(|| panic!("CAPABILITY call did not populate capability cache!")) + } + + /// The [`CAPABILITY` command](https://tools.ietf.org/html/rfc3501#section-6.1.1) requests a + /// listing of capabilities that the server supports. The server will include "IMAP4rev1" as + /// one of the listed capabilities. See [`Capabilities`] for further details. + /// + /// This function will not query the server if a set of capabilities was cached, but request + /// and cache capabilities from the server otherwise. The [`Self::capabilities_refresh`] method + /// can be used to refresh the cache by forcing a `CAPABILITY` command to be send. + pub fn capabilities_ref(&mut self) -> Result<&Capabilities> { + let (mut tx, _rx) = mpsc::channel(); + if self.capability_cache.is_none() { + let caps = self.run_command_and_read_response("CAPABILITY") + .and_then(|lines| Capabilities::parse(lines, &mut tx))?; + + self.capability_cache = Some(caps); + } + + self.capability_cache.as_ref() + // This path will not be hit; if the cache is not populated the above `if` will either + // populate it or return with an early error. + .ok_or_else(|| panic!("CAPABILITY call did not populate capability cache!")) + } + /// Log in to the IMAP server. Upon success a [`Session`](struct.Session.html) instance is /// returned; on error the original `Client` instance is returned in addition to the error. /// This is because `login` takes ownership of `self`, so in order to try again (e.g. after @@ -502,6 +553,7 @@ impl Session { let (tx, rx) = mpsc::channel(); Session { conn, + capability_cache: None, unsolicited_responses: rx, unsolicited_responses_tx: tx, } @@ -771,12 +823,52 @@ impl Session { self.run_command_and_check_ok(&format!("UNSUBSCRIBE {}", quote!(mailbox.as_ref()))) } + /// The [`CAPABILITY` command](https://tools.ietf.org/html/rfc3501#section-6.1.1) requests a + /// listing of capabilities that the server supports. The server will include "IMAP4rev1" as + /// one of the listed capabilities. See [`Capabilities`] for further details. + /// + /// This method will always bypass the local capabilities cache and send a `CAPABILITY` command + /// to the server. The [`Self::capabilities()`] method can be used when returning a cached + /// response is acceptable. + pub fn capabilities_refresh(&mut self) -> Result<&Capabilities> { + let caps = self.run_command_and_read_response("CAPABILITY") + .and_then(|lines| Capabilities::parse(lines, &mut self.unsolicited_responses_tx))?; + self.capability_cache = Some(caps); + + self.capability_cache.as_ref() + // This path will not be hit; if the cache is not populated the above calls will either + // populate it or return with an early error. + .ok_or_else(|| panic!("CAPABILITY call did not populate capability cache!")) + } + + /// The [`CAPABILITY` command](https://tools.ietf.org/html/rfc3501#section-6.1.1) requests a + /// listing of capabilities that the server supports. The server will include "IMAP4rev1" as + /// one of the listed capabilities. See [`Capabilities`] for further details. + /// + /// This function will not query the server if a set of capabilities was cached, but request + /// and cache capabilities from the server otherwise. The [`Self::capabilities_refresh`] method + /// can be used to refresh the cache by forcing a `CAPABILITY` command to be send. + pub fn capabilities_ref(&mut self) -> Result<&Capabilities> { + if self.capability_cache.is_none() { + let caps = self.run_command_and_read_response("CAPABILITY") + .and_then(|lines| Capabilities::parse(lines, &mut self.unsolicited_responses_tx))?; + + self.capability_cache = Some(caps); + } + + self.capability_cache.as_ref() + // This path will not be hit; if the cache is not populated the above `if` will either + // populate it or return with an early error. + .ok_or_else(|| panic!("CAPABILITY call did not populate capability cache!")) + } + /// The [`CAPABILITY` command](https://tools.ietf.org/html/rfc3501#section-6.1.1) requests a /// listing of capabilities that the server supports. The server will include "IMAP4rev1" as /// one of the listed capabilities. See [`Capabilities`] for further details. pub fn capabilities(&mut self) -> Result { - self.run_command_and_read_response("CAPABILITY") - .and_then(|lines| Capabilities::parse(lines, &mut self.unsolicited_responses_tx)) + // TODO: This emulated the same behaviour as before, with each call issuing a command. + // It may be sensible to allow hitting the cache here. + self.capabilities_refresh().map(|caps| caps.clone()) } /// The [`EXPUNGE` command](https://tools.ietf.org/html/rfc3501#section-6.4.3) permanently diff --git a/src/types/capabilities.rs b/src/types/capabilities.rs index 966ce15..a11d9c6 100644 --- a/src/types/capabilities.rs +++ b/src/types/capabilities.rs @@ -5,6 +5,7 @@ use imap_proto::{Capability, Response}; use ouroboros::self_referencing; use std::collections::hash_set::Iter; use std::collections::HashSet; +use std::fmt; use std::sync::mpsc; const IMAP4REV1_CAPABILITY: &str = "IMAP4rev1"; @@ -43,6 +44,17 @@ pub struct Capabilities { pub(crate) capabilities: HashSet>, } +impl Clone for Capabilities { + fn clone(&self) -> Self { + // Give _rx a name so it's not immediately dropped. Otherwise any unsolicited responses + // that would be send there will return a SendError instead of the parsed response simply + // being dropped later. + let (mut tx, _rx) = mpsc::channel(); + Self::parse(self.borrow_data().clone(), &mut tx) + .expect("failed to parse capabilities from data which was already successfully parse before") + } +} + impl Capabilities { /// Parse the given input into one or more [`Capabilitity`] responses. pub(crate) fn parse( @@ -98,3 +110,13 @@ impl Capabilities { self.borrow_capabilities().is_empty() } } + +impl fmt::Debug for Capabilities { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut dbg = f.debug_tuple("Capabilities"); + for x in self.borrow_capabilities() { + dbg.field(x); + } + dbg.finish() + } +} \ No newline at end of file From 15d3f482bb70097e61bc32f5684d731957e416b4 Mon Sep 17 00:00:00 2001 From: Nadja Reitzenstein Date: Sun, 12 Feb 2023 13:39:48 +0100 Subject: [PATCH 2/3] Implement changes & suggestions by @jonhoo --- src/client.rs | 31 +++++++++++-------------------- src/types/capabilities.rs | 3 ++- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/src/client.rs b/src/client.rs index 7526848..28be62c 100644 --- a/src/client.rs +++ b/src/client.rs @@ -139,7 +139,7 @@ fn validate_sequence_set( pub struct Session { conn: Connection, - // Capabilities are almost guaranteed to chance if encryption state or authentication state + // Capabilities are almost guaranteed to change if encryption state or authentication state // changes, so caching them in `Connection` is inappropiate. capability_cache: Option, @@ -159,7 +159,7 @@ pub struct Session { pub struct Client { conn: Connection, - // Capabilities are almost guaranteed to chance if encryption state or authentication state + // Capabilities are almost guaranteed to change if encryption state or authentication state // changes, so caching them in `Connection` is inappropiate. capability_cache: Option, } @@ -362,16 +362,16 @@ impl Client { /// This method will always bypass the local capabilities cache and send a `CAPABILITY` command /// to the server. The [`Self::capabilities()`] method can be used when returning a cached /// response is acceptable. - pub fn capabilities_refresh(&mut self) -> Result<&Capabilities> { + pub fn current_capabilities(&mut self) -> Result<&Capabilities> { let (mut tx, _rx) = mpsc::channel(); let caps = self.run_command_and_read_response("CAPABILITY") .and_then(|lines| Capabilities::parse(lines, &mut tx))?; self.capability_cache = Some(caps); - self.capability_cache.as_ref() + Ok(self.capability_cache.as_ref() // This path will not be hit; if the cache is not populated the above calls will either // populate it or return with an early error. - .ok_or_else(|| panic!("CAPABILITY call did not populate capability cache!")) + .expect("CAPABILITY call did not populate capability cache!")) } /// The [`CAPABILITY` command](https://tools.ietf.org/html/rfc3501#section-6.1.1) requests a @@ -381,7 +381,7 @@ impl Client { /// This function will not query the server if a set of capabilities was cached, but request /// and cache capabilities from the server otherwise. The [`Self::capabilities_refresh`] method /// can be used to refresh the cache by forcing a `CAPABILITY` command to be send. - pub fn capabilities_ref(&mut self) -> Result<&Capabilities> { + pub fn capabilities(&mut self) -> Result<&Capabilities> { let (mut tx, _rx) = mpsc::channel(); if self.capability_cache.is_none() { let caps = self.run_command_and_read_response("CAPABILITY") @@ -390,10 +390,10 @@ impl Client { self.capability_cache = Some(caps); } - self.capability_cache.as_ref() + Ok(self.capability_cache.as_ref() // This path will not be hit; if the cache is not populated the above `if` will either // populate it or return with an early error. - .ok_or_else(|| panic!("CAPABILITY call did not populate capability cache!")) + .expect("CAPABILITY call did not populate capability cache!")) } /// Log in to the IMAP server. Upon success a [`Session`](struct.Session.html) instance is @@ -830,7 +830,7 @@ impl Session { /// This method will always bypass the local capabilities cache and send a `CAPABILITY` command /// to the server. The [`Self::capabilities()`] method can be used when returning a cached /// response is acceptable. - pub fn capabilities_refresh(&mut self) -> Result<&Capabilities> { + pub fn current_capabilities(&mut self) -> Result<&Capabilities> { let caps = self.run_command_and_read_response("CAPABILITY") .and_then(|lines| Capabilities::parse(lines, &mut self.unsolicited_responses_tx))?; self.capability_cache = Some(caps); @@ -848,7 +848,7 @@ impl Session { /// This function will not query the server if a set of capabilities was cached, but request /// and cache capabilities from the server otherwise. The [`Self::capabilities_refresh`] method /// can be used to refresh the cache by forcing a `CAPABILITY` command to be send. - pub fn capabilities_ref(&mut self) -> Result<&Capabilities> { + pub fn capabilities(&mut self) -> Result<&Capabilities> { if self.capability_cache.is_none() { let caps = self.run_command_and_read_response("CAPABILITY") .and_then(|lines| Capabilities::parse(lines, &mut self.unsolicited_responses_tx))?; @@ -862,15 +862,6 @@ impl Session { .ok_or_else(|| panic!("CAPABILITY call did not populate capability cache!")) } - /// The [`CAPABILITY` command](https://tools.ietf.org/html/rfc3501#section-6.1.1) requests a - /// listing of capabilities that the server supports. The server will include "IMAP4rev1" as - /// one of the listed capabilities. See [`Capabilities`] for further details. - pub fn capabilities(&mut self) -> Result { - // TODO: This emulated the same behaviour as before, with each call issuing a command. - // It may be sensible to allow hitting the cache here. - self.capabilities_refresh().map(|caps| caps.clone()) - } - /// The [`EXPUNGE` command](https://tools.ietf.org/html/rfc3501#section-6.4.3) permanently /// removes all messages that have [`Flag::Deleted`] set from the currently selected mailbox. /// The message sequence number of each message that is removed is returned. @@ -2826,7 +2817,7 @@ a1 OK completed\r ]; let mock_stream = MockStream::new(response); let mut session = mock_session!(mock_stream); - let capabilities = session.capabilities().unwrap(); + let capabilities = session.capabilities().cloned().unwrap(); assert!( session.stream.get_ref().written_buf == b"a1 CAPABILITY\r\n".to_vec(), "Invalid capability command" diff --git a/src/types/capabilities.rs b/src/types/capabilities.rs index a11d9c6..3b990bf 100644 --- a/src/types/capabilities.rs +++ b/src/types/capabilities.rs @@ -48,7 +48,8 @@ impl Clone for Capabilities { fn clone(&self) -> Self { // Give _rx a name so it's not immediately dropped. Otherwise any unsolicited responses // that would be send there will return a SendError instead of the parsed response simply - // being dropped later. + // being dropped later. Those responses being dropped is safe as they were already parsed + // and sent to a consumer when this capabilities response was parsed the first time. let (mut tx, _rx) = mpsc::channel(); Self::parse(self.borrow_data().clone(), &mut tx) .expect("failed to parse capabilities from data which was already successfully parse before") From 04f5c9e88f923931f633aa5a8375811fe3e265eb Mon Sep 17 00:00:00 2001 From: Nadja Reitzenstein Date: Mon, 13 Feb 2023 00:25:55 +0100 Subject: [PATCH 3/3] fix capabilities methods doclinks --- src/client.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client.rs b/src/client.rs index 28be62c..732c7cd 100644 --- a/src/client.rs +++ b/src/client.rs @@ -379,7 +379,7 @@ impl Client { /// one of the listed capabilities. See [`Capabilities`] for further details. /// /// This function will not query the server if a set of capabilities was cached, but request - /// and cache capabilities from the server otherwise. The [`Self::capabilities_refresh`] method + /// and cache capabilities from the server otherwise. The [`Self::current_capabilities`] method /// can be used to refresh the cache by forcing a `CAPABILITY` command to be send. pub fn capabilities(&mut self) -> Result<&Capabilities> { let (mut tx, _rx) = mpsc::channel(); @@ -846,7 +846,7 @@ impl Session { /// one of the listed capabilities. See [`Capabilities`] for further details. /// /// This function will not query the server if a set of capabilities was cached, but request - /// and cache capabilities from the server otherwise. The [`Self::capabilities_refresh`] method + /// and cache capabilities from the server otherwise. The [`Self::current_capabilities`] method /// can be used to refresh the cache by forcing a `CAPABILITY` command to be send. pub fn capabilities(&mut self) -> Result<&Capabilities> { if self.capability_cache.is_none() {