Generic Cache-Aware Data Adapter

Question

I've written a generic data adapter that can make use of a cache, for use in a data access layer. It presents an interface allowing CRUD operations, as well as retrieving all objects of a certain type.

IEntity.cs:

using System;
namespace GenericDataAdapter
{
 public interface IEntity
 {
 Guid Id { get; set; }
 }
}

IDataAdapter.cs:

using System;
using System.Collections.Generic;
using System.Threading.Tasks;
namespace GenericDataAdapter
{
 public interface IDataAdapter<T> where T : IEntity
 {
 Task<IEnumerable<T>> ReadAllAsync();
 Task<(bool, T)> ReadAsync(Guid id); // bool indicates whether entity was present in data source
 Task SaveAsync(T newData); // update if present, create if not present
 Task DeleteAsync(Guid id);
 }
}

CacheDataAdapter.cs:

using System;
using System.Collections.Generic;
using System.Threading.Tasks;
namespace GenericDataAdapter
{
 public class CacheDataAdapter<T> : IDataAdapter<T> where T : IEntity
 {
 private IDataAdapter<T> PrimaryDataSource;
 private IDataAdapter<T> Cache;
 public CacheDataAdapter(IDataAdapter<T> primaryDataSource, IDataAdapter<T> cache)
 {
 PrimaryDataSource = primaryDataSource;
 Cache = cache;
 }
 public Task<IEnumerable<T>> ReadAllAsync()
 {
 return PrimaryDataSource.ReadAllAsync();
 }
 // can potentially return stale data, due to SaveAsync()/DeleteAsync() not being atomic
 public async Task<(bool, T)> ReadAsync(Guid id)
 {
 var (presentInCache, cacheData) = await Cache.ReadAsync(id);
 if (presentInCache)
 {
 return (true, cacheData);
 }
 var (presentInPrimary, primaryData) = await PrimaryDataSource.ReadAsync(id);
 if (presentInPrimary)
 {
 await Cache.SaveAsync(primaryData);
 return (true, primaryData);
 }
 else
 {
 return (false, default(T));
 }
 }
 public async Task SaveAsync(T newData)
 {
 await Cache.SaveAsync(newData);
 await PrimaryDataSource.SaveAsync(newData);
 }
 public async Task DeleteAsync(Guid id)
 {
 await Cache.DeleteAsync(id);
 await PrimaryDataSource.DeleteAsync(id);
 } 
 }
}

There are two points I'd like feedback on, though all comments are welcome:

• In CacheDataAdapter.ReadAllAsync(), should I use await? It doesn't seem like I should, since I'm just passing through the return value of PrimaryDataSource.ReadAllAsync(), but I'm not sure.
• I'm not sure whether unifying the create and update operations into SaveAsync() is a good idea; it puts the responsibility for fully initializing objects on the application. That's why I'm using GUIDs as the identifier, so the application can generate unique IDs without having to consult the database first.

• AnotherGuy: In your SaveAsync and DeleteAsync methods I would switch around the order of writing to the primary datasource first then the cache. Looking at this from a defensive principle the current implementation could be an issue. Should the save/delete to the primary datasource fail the cache would cover this until it expires. The other way around allows your ReadAsync method to handle the case and retry save to the cache transparently.
• dfhwze: @DylanSp Having a unique identifier, whether it's a business or technical id, is never a bad idea, regardless whether it represents the primary key in a database.

Answer 1

Typically this scenario is done with a decorator pattern where you would wrap just the one and have the cache functionality in the one class. You would put the code in the CacheDataAdapter that I assume is currently in the code of the IDataAdapter cache. Most of the time this will simplify your code as you can call base method to get the values if not in the backing store for the cache.

For example if you are using the ConcurrentDictionary or MemoryCache they will have methods to GetOrCreate or AddOrUpdate that will take a lamba expression that you can call into your base method to get the value if missing. Since you are doing async work while caching you should check out the AsyncLazy

If you don't want to do the decorator pattern I would swap Save and Delete to do it in the primary store before removing them from the cache. You could have a situation where the save fails in the primary but your cache is updated to say it's good.

As far if you should await the ReadAllAsync or not it doesn't matter a lot. I personally wouldn't await it but either way is fine. See answer on SO

• DylanSp: I figured I'd separate the implementation of caching logic into CacheDataAdapter, while another implementation of IDataAdapter would handle the lower-level logic of connecting to a specific cache solution like Redis.