Skip to content

Commit

Permalink
[tutasdk] use a struct for keying the blob access token cache
Browse files Browse the repository at this point in the history
  • Loading branch information
ganthern committed Nov 5, 2024
1 parent f859f68 commit 6f765e6
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 39 deletions.
58 changes: 44 additions & 14 deletions tuta-sdk/rust/sdk/src/blobs/blob_access_token_cache.rs
Original file line number Diff line number Diff line change
@@ -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)

This comment has been minimized.

Copy link
@charlag

charlag Nov 5, 2024

Contributor

why do we convert id to string? Wouldn't it make sense to just copy the id?

This comment has been minimized.

Copy link
@charlag

charlag Nov 5, 2024

Contributor

ah they are strings anyway under the hood aren't they, nvm

}
}

pub(super) struct BlobAccessTokenCache {
cache: RwLock<HashMap<String, BlobServerAccessInfo>>,
cache: RwLock<HashMap<BlobWriteTokenKey, BlobServerAccessInfo>>,
date_provider: Arc<dyn DateProvider>,
}

Expand All @@ -19,7 +35,7 @@ impl BlobAccessTokenCache {

pub async fn try_get_token<F, E, Loader>(
&self,
key: &String,
key: &BlobWriteTokenKey,
loader: Loader,
) -> Result<BlobServerAccessInfo, E>
where
Expand All @@ -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);
}
Expand All @@ -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 {
Expand All @@ -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()
};
Expand All @@ -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()
Expand Down Expand Up @@ -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));
Expand Down
25 changes: 4 additions & 21 deletions tuta-sdk/rust/sdk/src/blobs/blob_access_token_facade.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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::*;
Expand Down
8 changes: 6 additions & 2 deletions tuta-sdk/rust/sdk/src/blobs/blob_facade.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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!(
Expand Down
4 changes: 2 additions & 2 deletions tuta-sdk/rust/sdk/src/tutanota_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 6f765e6

Please sign in to comment.