From 6f765e63086e8ec9e82629abe99feba22f915505 Mon Sep 17 00:00:00 2001 From: nig Date: Tue, 5 Nov 2024 13:42:53 +0100 Subject: [PATCH] [tutasdk] use a struct for keying the blob access token cache --- .../sdk/src/blobs/blob_access_token_cache.rs | 58 ++++++++++++++----- .../sdk/src/blobs/blob_access_token_facade.rs | 25 ++------ tuta-sdk/rust/sdk/src/blobs/blob_facade.rs | 8 ++- tuta-sdk/rust/sdk/src/tutanota_constants.rs | 4 +- 4 files changed, 56 insertions(+), 39 deletions(-) diff --git a/tuta-sdk/rust/sdk/src/blobs/blob_access_token_cache.rs b/tuta-sdk/rust/sdk/src/blobs/blob_access_token_cache.rs index b6d59a4efff..e1079e27375 100644 --- a/tuta-sdk/rust/sdk/src/blobs/blob_access_token_cache.rs +++ b/tuta-sdk/rust/sdk/src/blobs/blob_access_token_cache.rs @@ -1,11 +1,27 @@ use crate::date::DateProvider; use crate::entities::storage::BlobServerAccessInfo; +use crate::generated_id::GeneratedId; +use crate::tutanota_constants::ArchiveDataType; use std::collections::HashMap; use std::future::Future; use std::sync::{Arc, RwLock}; +/// still missing ReadArchive(archive_id) and ReadBlob(archive_id, blob_id) +/// (see TS impl of the cache) +/// once we have those, it should be made even more type safe by restricting the input to +/// reading/writing functions on blob facade to the right kind of token. +#[derive(Clone, Hash, PartialEq, Eq)] +#[cfg_attr(test, derive(Debug))] +pub(crate) struct BlobWriteTokenKey(String, ArchiveDataType); + +impl BlobWriteTokenKey { + pub fn new(id: &GeneratedId, data_type: ArchiveDataType) -> Self { + Self(id.to_string(), data_type) + } +} + pub(super) struct BlobAccessTokenCache { - cache: RwLock>, + cache: RwLock>, date_provider: Arc, } @@ -19,7 +35,7 @@ impl BlobAccessTokenCache { pub async fn try_get_token( &self, - key: &String, + key: &BlobWriteTokenKey, loader: Loader, ) -> Result where @@ -36,18 +52,18 @@ impl BlobAccessTokenCache { } } let loaded = loader().await?; - self.insert(key, loaded.clone()); + self.insert(key.to_owned(), loaded.clone()); Ok(loaded) } - fn insert(&self, key: &str, value: BlobServerAccessInfo) { + fn insert(&self, key: BlobWriteTokenKey, value: BlobServerAccessInfo) { let mut cache = self.cache.write().expect("poisoned lock"); // someone else might have inserted something while we were loading. // we're just replacing + dropping that value. - let _previous = cache.insert(key.to_owned(), value); + let _previous = cache.insert(key, value); } - pub fn evict(&self, key: &String) { + pub fn evict(&self, key: &BlobWriteTokenKey) { let mut cache = self.cache.write().expect("poisoned lock"); cache.remove(key); } @@ -65,23 +81,28 @@ fn can_be_used_for_another_request( #[cfg(test)] mod tests { use crate::blobs::blob_access_token_cache::{ - can_be_used_for_another_request, BlobAccessTokenCache, + can_be_used_for_another_request, BlobAccessTokenCache, BlobWriteTokenKey, }; use crate::date::date_provider::stub::DateProviderStub; use crate::date::DateTime; use crate::entities::storage::BlobServerAccessInfo; + use crate::tutanota_constants::ArchiveDataType; use crate::util::test_utils::create_test_entity; + use crate::GeneratedId; use std::sync::Arc; #[tokio::test] async fn get_cached() { let cache = BlobAccessTokenCache::new(Arc::new(DateProviderStub::new(0))); - let key = "key".to_owned(); + let key = BlobWriteTokenKey::new( + &GeneratedId("group".to_owned()), + ArchiveDataType::Attachments, + ); let test_token = BlobServerAccessInfo { expires: DateTime::from_millis(10), ..create_test_entity() }; - cache.insert(&key, test_token.clone()); + cache.insert(key.clone(), test_token.clone()); let loaded = cache.try_get_token(&key, || async { // helps type inference if true { @@ -95,7 +116,10 @@ mod tests { #[tokio::test] async fn get_uncached() { let cache = BlobAccessTokenCache::new(Arc::new(DateProviderStub::new(0))); - let key = "key".to_owned(); + let key = BlobWriteTokenKey::new( + &GeneratedId("group".to_owned()), + ArchiveDataType::Attachments, + ); let test_token = BlobServerAccessInfo { ..create_test_entity() }; @@ -109,12 +133,15 @@ mod tests { #[tokio::test] async fn get_expired() { let cache = BlobAccessTokenCache::new(Arc::new(DateProviderStub::new(20))); - let key = "key".to_owned(); + let key = BlobWriteTokenKey::new( + &GeneratedId("group".to_owned()), + ArchiveDataType::Attachments, + ); let expired_token = BlobServerAccessInfo { expires: DateTime::from_millis(10), ..create_test_entity() }; - cache.insert(&key, expired_token.clone()); + cache.insert(key.clone(), expired_token.clone()); let new_token = BlobServerAccessInfo { expires: DateTime::from_millis(30), ..create_test_entity() @@ -148,12 +175,15 @@ mod tests { #[test] fn evict() { let cache = BlobAccessTokenCache::new(Arc::new(DateProviderStub::new(20))); - let key = "key".to_owned(); + let key = BlobWriteTokenKey::new( + &GeneratedId("group".to_owned()), + ArchiveDataType::Attachments, + ); let expired_token = BlobServerAccessInfo { expires: DateTime::from_millis(10), ..create_test_entity() }; - cache.insert(&key, expired_token.clone()); + cache.insert(key.clone(), expired_token.clone()); cache.evict(&key); assert!(!cache.cache.read().unwrap().contains_key(&key)); diff --git a/tuta-sdk/rust/sdk/src/blobs/blob_access_token_facade.rs b/tuta-sdk/rust/sdk/src/blobs/blob_access_token_facade.rs index 199939bdaf9..a1383e5c8b5 100644 --- a/tuta-sdk/rust/sdk/src/blobs/blob_access_token_facade.rs +++ b/tuta-sdk/rust/sdk/src/blobs/blob_access_token_facade.rs @@ -1,4 +1,4 @@ -use crate::blobs::blob_access_token_cache::BlobAccessTokenCache; +use crate::blobs::blob_access_token_cache::{BlobAccessTokenCache, BlobWriteTokenKey}; use crate::crypto::randomizer_facade::RandomizerFacade; use crate::custom_id::CustomId; use crate::date::DateProvider; @@ -66,34 +66,17 @@ impl BlobAccessTokenFacade { self.cache .try_get_token( - &make_write_cache_key(owner_group_id, archive_data_type), + &BlobWriteTokenKey::new(owner_group_id, archive_data_type), loader, ) .await } /// Remove a given write token from the cache. - pub fn evict_write_token( - &self, - archive_data_type: ArchiveDataType, - owner_group_id: &GeneratedId, - ) { - let key = make_write_cache_key(owner_group_id, archive_data_type); - self.cache.evict(&key); + pub fn evict_access_token(&self, key: &BlobWriteTokenKey) { + self.cache.evict(key); } } - -pub(crate) fn make_write_cache_key( - owner_group_id: &GeneratedId, - archive_data_type: ArchiveDataType, -) -> String { - format!( - "{}{}", - owner_group_id.as_str(), - archive_data_type.discriminant() - ) -} - #[cfg(test)] mod tests { use super::*; diff --git a/tuta-sdk/rust/sdk/src/blobs/blob_facade.rs b/tuta-sdk/rust/sdk/src/blobs/blob_facade.rs index c6612154383..11cbc925863 100644 --- a/tuta-sdk/rust/sdk/src/blobs/blob_facade.rs +++ b/tuta-sdk/rust/sdk/src/blobs/blob_facade.rs @@ -1,3 +1,4 @@ +use crate::blobs::blob_access_token_cache::BlobWriteTokenKey; #[cfg_attr(test, mockall_double::double)] use crate::blobs::blob_access_token_facade::BlobAccessTokenFacade; use crate::crypto::aes::Iv; @@ -73,7 +74,10 @@ impl BlobFacade { source: HttpError::NotAuthorizedError, }) => { self.blob_access_token_facade - .evict_write_token(archive_data_type, owner_group_id); + .evict_access_token(&BlobWriteTokenKey::new( + owner_group_id, + archive_data_type, + )); self.encrypt_and_upload_chunk( archive_data_type, owner_group_id, @@ -374,7 +378,7 @@ mod tests { #[test] fn encode_query_params_works() { - assert_eq!("", encode_query_params([] as [(&str, &str); 0])); + assert_eq!("", encode_query_params([("", ""); 0])); assert_eq!("", encode_query_params([("", "b"), ("c", "")])); assert_eq!("?c=d+d+d", encode_query_params([("", "b"), ("c", "d d d")])); assert_eq!( diff --git a/tuta-sdk/rust/sdk/src/tutanota_constants.rs b/tuta-sdk/rust/sdk/src/tutanota_constants.rs index 78fa0b8583e..abda1859867 100644 --- a/tuta-sdk/rust/sdk/src/tutanota_constants.rs +++ b/tuta-sdk/rust/sdk/src/tutanota_constants.rs @@ -10,8 +10,8 @@ pub enum PublicKeyIdentifierType { #[allow(dead_code)] #[repr(i64)] -#[derive(Copy, Clone)] -#[cfg_attr(test, derive(Debug, PartialEq))] +#[derive(Copy, Clone, Hash, PartialEq, Eq)] +#[cfg_attr(test, derive(Debug))] pub enum ArchiveDataType { AuthorityRequests = 0, Attachments = 1,