Skip to content

refactor!: Cache instead of Arc<Cache> in public APIs #433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
m4tx merged 1 commit into from
Dec 28, 2025
Merged

refactor!: Cache instead of Arc<Cache> in public APIs #433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 23 additions & 13 deletions cot/src/cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,13 @@ pub type CacheResult<T> = Result<T, CacheError>;
/// ```
#[derive(Debug, Clone)]
pub struct Cache {
inner: Arc<CacheImpl>,
}

#[derive(Debug)]
struct CacheImpl {
#[debug("..")]
store: Arc<dyn BoxCacheStore>,
store: Box<dyn BoxCacheStore>,
Copy link
Contributor

@ElijahAhianyo ElijahAhianyo Dec 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious about this. What's the benefit of doing it this way(boxing the store in another struct and wrapping it in an Arc)?

Copy link
Member Author

@m4tx m4tx Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need an indirection here (either Box, Arc, or some other heap-based container) because we store a trait object (dyn) here. Arc is not necessary, because we have it above anyway, and Box is sligthly less expensive because it doesn't have reference counting.

An ideal situation would be to have the exact cache type specified using some generic type T in CacheImpl and make the CacheImpl implement some trait that would be used directly by Cache, but I think the extra pointer dereference won't hurt the performance and it's much cleaner to implement.

prefix: Option<String>,
expiry: Timeout,
}
Expand All @@ -219,17 +224,19 @@ impl Cache {
/// );
/// ```
pub fn new(store: impl CacheStore, prefix: Option<String>, expiry: Timeout) -> Self {
let store: Arc<dyn BoxCacheStore> = Arc::new(store);
let store: Box<dyn BoxCacheStore> = Box::new(store);
Self {
store,
prefix,
expiry,
inner: Arc::new(CacheImpl {
store,
prefix,
expiry,
}),
}
}

fn format_key<K: AsRef<str>>(&self, key: K) -> String {
let k = key.as_ref();
if let Some(pref) = &self.prefix {
if let Some(pref) = &self.inner.prefix {
return format!("{pref}:{k}");
}
k.to_string()
Expand Down Expand Up @@ -276,6 +283,7 @@ impl Cache {
{
let k = self.format_key(key.as_ref());
let result = self
.inner
.store
.get(&k)
.await?
Expand Down Expand Up @@ -334,8 +342,9 @@ impl Cache {
V: Serialize,
{
let k = self.format_key(key.into());
self.store
.insert(k, serde_json::to_value(value)?, self.expiry)
self.inner
.store
.insert(k, serde_json::to_value(value)?, self.inner.expiry)
.await?;
Ok(())
}
Expand Down Expand Up @@ -387,7 +396,8 @@ impl Cache {
V: Serialize,
{
let k = self.format_key(key.into());
self.store
self.inner
.store
.insert(k, serde_json::to_value(value)?, expiry)
.await?;
Ok(())
Expand Down Expand Up @@ -424,7 +434,7 @@ impl Cache {
/// ```
pub async fn remove<K: AsRef<str>>(&self, key: K) -> CacheResult<()> {
let k = self.format_key(key.as_ref());
self.store.remove(&k).await?;
self.inner.store.remove(&k).await?;
Ok(())
}

Expand Down Expand Up @@ -465,7 +475,7 @@ impl Cache {
/// # }
/// ```
pub async fn clear(&self) -> CacheResult<()> {
self.store.clear().await?;
self.inner.store.clear().await?;
Ok(())
}

Expand Down Expand Up @@ -505,7 +515,7 @@ impl Cache {
/// # }
/// ```
pub async fn approx_size(&self) -> CacheResult<usize> {
let result = self.store.approx_size().await?;
let result = self.inner.store.approx_size().await?;
Ok(result)
}

Expand Down Expand Up @@ -542,7 +552,7 @@ impl Cache {
/// ```
pub async fn contains_key<K: AsRef<str>>(&self, key: K) -> CacheResult<bool> {
let k = self.format_key(key.as_ref());
let result = self.store.contains_key(&k).await?;
let result = self.inner.store.contains_key(&k).await?;
Ok(result)
}

Expand Down
16 changes: 8 additions & 8 deletions cot/src/project.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1334,8 +1334,8 @@ impl Bootstrapper<WithDatabase> {
}

#[cfg(feature = "cache")]
async fn init_cache(config: &CacheConfig) -> cot::Result<Arc<Cache>> {
let cache = Cache::from_config(config).await.map(Arc::new)?;
async fn init_cache(config: &CacheConfig) -> cot::Result<Cache> {
let cache = Cache::from_config(config).await?;
Ok(cache)
}
}
Expand Down Expand Up @@ -1652,7 +1652,7 @@ impl BootstrapPhase for WithCache {
type Database = Option<Arc<Database>>;
type AuthBackend = <WithApps as BootstrapPhase>::AuthBackend;
#[cfg(feature = "cache")]
type Cache = Arc<Cache>;
type Cache = Cache;
}

/// The final phase of bootstrapping a Cot project, the initialized phase.
Expand Down Expand Up @@ -1808,7 +1808,7 @@ impl ProjectContext<WithApps> {

impl ProjectContext<WithDatabase> {
#[must_use]
fn with_cache(self, #[cfg(feature = "cache")] cache: Arc<Cache>) -> ProjectContext<WithCache> {
fn with_cache(self, #[cfg(feature = "cache")] cache: Cache) -> ProjectContext<WithCache> {
ProjectContext {
config: self.config,
apps: self.apps,
Expand Down Expand Up @@ -1908,7 +1908,7 @@ impl<S: BootstrapPhase<AuthBackend = Arc<dyn AuthBackend>>> ProjectContext<S> {
}

#[cfg(feature = "cache")]
impl<S: BootstrapPhase<Cache = Arc<Cache>>> ProjectContext<S> {
impl<S: BootstrapPhase<Cache = Cache>> ProjectContext<S> {
/// Returns the cache for the project.
///
/// # Examples
Expand All @@ -1925,7 +1925,7 @@ impl<S: BootstrapPhase<Cache = Arc<Cache>>> ProjectContext<S> {
/// ```
#[must_use]
#[cfg(feature = "cache")]
pub fn cache(&self) -> &Arc<Cache> {
pub fn cache(&self) -> &Cache {
&self.cache
}
}
Expand Down Expand Up @@ -2521,11 +2521,11 @@ mod tests {

#[cot::test]
async fn default_auth_backend() {
let cache_memory = Arc::new(Cache::new(
let cache_memory = Cache::new(
cache::store::memory::Memory::new(),
None,
Timeout::default(),
));
);

let context = ProjectContext::new()
.with_config(
Expand Down
13 changes: 5 additions & 8 deletions cot/src/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ pub struct TestRequestBuilder {
json_data: Option<String>,
static_files: Vec<StaticFile>,
#[cfg(feature = "cache")]
cache: Option<Arc<Cache>>,
cache: Option<Cache>,
}

/// A wrapper over an auth backend that is cloneable.
Expand Down Expand Up @@ -774,7 +774,7 @@ impl TestRequestBuilder {
#[cfg(feature = "cache")]
self.cache
.clone()
.unwrap_or_else(|| Arc::new(Cache::new(Memory::new(), None, Timeout::default()))),
.unwrap_or_else(|| Cache::new(Memory::new(), None, Timeout::default())),
);
prepare_request(&mut request, Arc::new(context));

Expand Down Expand Up @@ -1776,17 +1776,14 @@ enum CacheKind {
#[cfg(feature = "cache")]
#[derive(Debug, Clone)]
pub struct TestCache {
cache: Arc<Cache>,
cache: Cache,
kind: CacheKind,
}

#[cfg(feature = "cache")]
impl TestCache {
fn new(cache: Cache, kind: CacheKind) -> Self {
Self {
cache: Arc::new(cache),
kind,
}
Self { cache, kind }
}

/// Create a new in-memory test cache.
Expand Down Expand Up @@ -1905,7 +1902,7 @@ impl TestCache {
/// # }
/// ```
#[must_use]
pub fn cache(&self) -> Arc<Cache> {
pub fn cache(&self) -> Cache {
self.cache.clone()
}

Expand Down
Loading