diff --git a/src/authenticator.rs b/src/authenticator.rs index ae66fda..75f6888 100644 --- a/src/authenticator.rs +++ b/src/authenticator.rs @@ -35,8 +35,8 @@ where where T: AsRef, { - let scope_key = storage::ScopeHash::new(scopes); - match self.storage.get(scope_key, scopes) { + let hashed_scopes = storage::HashedScopes::from(scopes); + match self.storage.get(hashed_scopes) { Some(t) if !t.expired() => { // unexpired token found Ok(t) @@ -59,9 +59,7 @@ where } Ok(token) => token, }; - self.storage - .set(scope_key, scopes, Some(token.clone())) - .await; + self.storage.set(hashed_scopes, Some(token.clone())).await; Ok(token) } None @@ -74,7 +72,7 @@ where .auth_flow .token(&self.hyper_client, &self.app_secret, scopes) .await?; - self.storage.set(scope_key, scopes, Some(t.clone())).await; + self.storage.set(hashed_scopes, Some(t.clone())).await; Ok(t) } } diff --git a/src/service_account.rs b/src/service_account.rs index e88022a..c81b6cc 100644 --- a/src/service_account.rs +++ b/src/service_account.rs @@ -263,9 +263,9 @@ where where T: AsRef, { - let hash = storage::ScopeHash::new(scopes); + let hashed_scopes = storage::HashedScopes::from(scopes); let cache = &self.cache; - match cache.get(hash, scopes) { + match cache.get(hashed_scopes) { Some(token) if !token.expired() => return Ok(token), _ => {} } @@ -277,7 +277,7 @@ where scopes, ) .await?; - cache.set(hash, scopes, Some(token.clone())).await; + cache.set(hashed_scopes, Some(token.clone())).await; Ok(token) } /// Send a request for a new Bearer token to the OAuth provider. @@ -399,12 +399,9 @@ mod tests { assert!(acc .cache - .get( - dbg!(storage::ScopeHash::new(&[ - "https://www.googleapis.com/auth/pubsub" - ])), - &["https://www.googleapis.com/auth/pubsub"], - ) + .get(storage::HashedScopes::from(&[ + "https://www.googleapis.com/auth/pubsub" + ])) .is_some()); // Test that token is in cache (otherwise mock will tell us) let tok = acc diff --git a/src/storage.rs b/src/storage.rs index 7192f77..0dd8720 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -13,24 +13,53 @@ use std::sync::Mutex; use serde::{Deserialize, Serialize}; -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] -pub struct ScopeHash(u64); +#[derive(Debug)] +pub struct HashedScopes<'a, T> { + hash: u64, + scopes: &'a [T], +} -impl ScopeHash { - /// Calculate a hash value describing the scopes. The order of the scopes in the - /// list does not change the hash value. i.e. two lists that contains the exact - /// same scopes, but in different order will return the same hash value. - pub fn new(scopes: &[T]) -> Self - where - T: AsRef, - { - let mut hash_sum = DefaultHasher::new().finish(); +// Implement Clone manually. Auto derive fails to work correctly because we want +// Clone to be implemented regardless of whether T is Clone or not. +impl<'a, T> Clone for HashedScopes<'a, T> { + fn clone(&self) -> Self { + HashedScopes { + hash: self.hash, + scopes: self.scopes, + } + } +} +impl<'a, T> Copy for HashedScopes<'a, T> {} + +impl<'a, T> From<&'a [T]> for HashedScopes<'a, T> +where + T: AsRef, +{ + fn from(scopes: &'a [T]) -> Self { + // Calculate a hash value describing the scopes. The order of the scopes in the + // list does not change the hash value. i.e. two lists that contains the exact + // same scopes, but in different order will return the same hash value. + let mut hash = DefaultHasher::new().finish(); for scope in scopes { let mut hasher = DefaultHasher::new(); scope.as_ref().hash(&mut hasher); - hash_sum ^= hasher.finish(); + hash ^= hasher.finish(); } - ScopeHash(hash_sum) + HashedScopes { hash, scopes } + } +} + +impl<'a, T> HashedScopes<'a, T> +where + T: AsRef, +{ + // implement an inherent from method even though From is implemented. This + // is because passing an array ref like &[&str; 1] (&["foo"]) will be auto + // deref'd to a slice on function boundaries, but it will not implement the + // From trait. This inherent method just serves to auto deref from array + // refs to slices and proxy to the From impl. + pub fn from(scopes: &'a [T]) -> Self { + >::from(scopes) } } @@ -40,23 +69,23 @@ pub(crate) enum Storage { } impl Storage { - pub(crate) async fn set(&self, h: ScopeHash, scopes: &[T], token: Option) + pub(crate) async fn set(&self, scopes: HashedScopes<'_, T>, token: Option) where T: AsRef, { match self { - Storage::Memory { tokens } => tokens.lock().unwrap().set(h, scopes, token), - Storage::Disk(disk_storage) => disk_storage.set(h, scopes, token).await, + Storage::Memory { tokens } => tokens.lock().unwrap().set(scopes, token), + Storage::Disk(disk_storage) => disk_storage.set(scopes, token).await, } } - pub(crate) fn get(&self, h: ScopeHash, scopes: &[T]) -> Option + pub(crate) fn get(&self, scopes: HashedScopes) -> Option where T: AsRef, { match self { - Storage::Memory { tokens } => tokens.lock().unwrap().get(h, scopes), - Storage::Disk(disk_storage) => disk_storage.get(h, scopes), + Storage::Memory { tokens } => tokens.lock().unwrap().get(scopes), + Storage::Disk(disk_storage) => disk_storage.get(scopes), } } } @@ -64,7 +93,7 @@ impl Storage { /// A single stored token. #[derive(Debug, Clone, Serialize, Deserialize)] struct JSONToken { - pub hash: ScopeHash, + pub hash: u64, pub scopes: Option>, pub token: Token, } @@ -107,38 +136,45 @@ impl JSONTokens { Ok(container) } - fn get(&self, h: ScopeHash, scopes: &[T]) -> Option + fn get(&self, scopes: HashedScopes) -> Option where T: AsRef, { for t in self.tokens.iter() { if let Some(token_scopes) = &t.scopes { if scopes + .scopes .iter() .all(|s| token_scopes.iter().any(|t| t == s.as_ref())) { return Some(t.token.clone()); } - } else if h == t.hash { + } else if scopes.hash == t.hash { return Some(t.token.clone()); } } None } - fn set(&mut self, h: ScopeHash, scopes: &[T], token: Option) + fn set(&mut self, scopes: HashedScopes, token: Option) where T: AsRef, { - eprintln!("setting: {:?}, {:?}", h, token); - self.tokens.retain(|x| x.hash != h); + eprintln!("setting: {:?}, {:?}", scopes.hash, token); + self.tokens.retain(|x| x.hash != scopes.hash); match token { None => (), Some(t) => { self.tokens.push(JSONToken { - hash: h, - scopes: Some(scopes.iter().map(|x| x.as_ref().to_string()).collect()), + hash: scopes.hash, + scopes: Some( + scopes + .scopes + .iter() + .map(|x| x.as_ref().to_string()) + .collect(), + ), token: t, }); } @@ -183,13 +219,13 @@ impl DiskStorage { }) } - async fn set(&self, h: ScopeHash, scopes: &[T], token: Option) + async fn set(&self, scopes: HashedScopes<'_, T>, token: Option) where T: AsRef, { let cloned_tokens = { let mut tokens = self.tokens.lock().unwrap(); - tokens.set(h, scopes, token); + tokens.set(scopes, token); tokens.clone() }; self.write_tx @@ -199,11 +235,11 @@ impl DiskStorage { .expect("disk storage task not running"); } - pub(crate) fn get(&self, h: ScopeHash, scopes: &[T]) -> Option + pub(crate) fn get(&self, scopes: HashedScopes) -> Option where T: AsRef, { - self.tokens.lock().unwrap().get(h, scopes) + self.tokens.lock().unwrap().get(scopes) } } @@ -215,24 +251,24 @@ mod tests { fn test_hash_scopes() { // Idential list should hash equal. assert_eq!( - ScopeHash::new(&["foo", "bar"]), - ScopeHash::new(&["foo", "bar"]) + HashedScopes::from(&["foo", "bar"]).hash, + HashedScopes::from(&["foo", "bar"]).hash, ); // The hash should be order independent. assert_eq!( - ScopeHash::new(&["bar", "foo"]), - ScopeHash::new(&["foo", "bar"]) + HashedScopes::from(&["bar", "foo"]).hash, + HashedScopes::from(&["foo", "bar"]).hash, ); assert_eq!( - ScopeHash::new(&["bar", "baz", "bat"]), - ScopeHash::new(&["baz", "bar", "bat"]) + HashedScopes::from(&["bar", "baz", "bat"]).hash, + HashedScopes::from(&["baz", "bar", "bat"]).hash, ); // Ensure hashes differ when the contents are different by more than // just order. assert_ne!( - ScopeHash::new(&["foo", "bar", "baz"]), - ScopeHash::new(&["foo", "bar"]) + HashedScopes::from(&["foo", "bar", "baz"]).hash, + HashedScopes::from(&["foo", "bar"]).hash, ); } }