Merge pull request #373 from POPWorldMedia/settings-cache-refactor-370

Settings cache refactor #370

src/PopForums.AzureKit.Functions/CacheHelper.cs

@@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using PopForums.Configuration;
 
 namespace PopForums.AzureKit.Functions;
@@ -34,4 +35,11 @@ public List<T> GetPagedListCacheObject<T>(string rootKey, int page)
 	{
 		return null;
 	}
+
+	public event Action<string> OnRemoveCacheKey;
+
+	public string GetEffectiveCacheKey(string key)
+	{
+		return key;
+	}
 }

src/PopForums.AzureKit/Redis/CacheHelper.cs

@@ -61,13 +61,17 @@ public CacheHelper(IErrorLog errorLog, ITenantService tenantService, IConfig con
 				{
 					if (_cache == null)
 						return;
-					_cache.Remove(value.ToString());
+					var valueString = value.ToString();
+					_cache.Remove(valueString);
+					OnRemoveCacheKey?.Invoke(valueString);
 				});
 			}
 		}
 	}
+
+	public event Action<string> OnRemoveCacheKey;
 
-	private string PrefixTenantOnKey(string key)
+	public string GetEffectiveCacheKey(string key)
 	{
 		var tenantID = _tenantService.GetTenant();
 		return $"{tenantID}:{key}";
@@ -89,7 +93,7 @@ public void SetCacheObject(string key, object value)
 
 	public void SetCacheObject(string key, object value, double seconds)
 	{
-		key = PrefixTenantOnKey(key);
+		key = GetEffectiveCacheKey(key);
 		var timeSpan = TimeSpan.FromSeconds(seconds);
 		var options = new MemoryCacheEntryOptions { AbsoluteExpirationRelativeToNow = timeSpan };
 		_cacheTelemetry.Start();
@@ -112,7 +116,7 @@ public void SetCacheObject(string key, object value, double seconds)
 
 	public void SetLongTermCacheObject(string key, object value)
 	{
-		key = PrefixTenantOnKey(key);
+		key = GetEffectiveCacheKey(key);
 		var options = new MemoryCacheEntryOptions { SlidingExpiration = TimeSpan.FromMinutes(60) };
 		_cacheTelemetry.Start();
 		_cache.Set(key, value, options);
@@ -134,7 +138,7 @@ public void SetLongTermCacheObject(string key, object value)
 
 	public void SetPagedListCacheObject<T>(string rootKey, int page, List<T> value)
 	{
-		rootKey = PrefixTenantOnKey(rootKey);
+		rootKey = GetEffectiveCacheKey(rootKey);
 		_cache.TryGetValue(rootKey, out Dictionary<int, List<T>> rootPages);
 		if (rootPages == null)
 			rootPages = new Dictionary<int, List<T>>();
@@ -150,7 +154,7 @@ public void SetPagedListCacheObject<T>(string rootKey, int page, List<T> value)
 
 	public void RemoveCacheObject(string key)
 	{
-		key = PrefixTenantOnKey(key);
+		key = GetEffectiveCacheKey(key);
 		_cache.Remove(key);
 		try
 		{
@@ -167,7 +171,7 @@ public void RemoveCacheObject(string key)
 
 	public T GetCacheObject<T>(string key)
 	{
-		key = PrefixTenantOnKey(key);
+		key = GetEffectiveCacheKey(key);
 		_cacheTelemetry.Start();
 		var cacheObject = _cache.Get(key);
 		if (cacheObject != null)
@@ -205,7 +209,7 @@ public T GetCacheObject<T>(string key)
 
 	public List<T> GetPagedListCacheObject<T>(string rootKey, int page)
 	{
-		rootKey = PrefixTenantOnKey(rootKey);
+		rootKey = GetEffectiveCacheKey(rootKey);
 		_cacheTelemetry.Start();
 		_cache.TryGetValue(rootKey, out Dictionary<int, List<T>> rootPages);
 		if (rootPages == null)

src/PopForums.AzureKit/ServiceCollectionExtensions.cs

@@ -19,7 +19,7 @@ public static IServiceCollection AddPopForumsRedisCache(this IServiceCollection
 		var config = serviceProvider.GetService<IConfig>();
 		if (config.ForceLocalOnly)
 			return services;
-		services.Replace(ServiceDescriptor.Transient<ICacheHelper, PopForums.AzureKit.Redis.CacheHelper>());
+		services.Replace(ServiceDescriptor.Singleton<ICacheHelper, PopForums.AzureKit.Redis.CacheHelper>());
 		return services;
 	}
 

src/PopForums.Sql/CacheHelper.cs

@@ -61,6 +61,7 @@ public void RemoveCacheObject(string key)
 	{
 		key = PrefixTenantOnKey(key);
 		_cache.Remove(key);
+		OnRemoveCacheKey?.Invoke(key);
 	}
 
 	public T GetCacheObject<T>(string key)
@@ -80,4 +81,12 @@ public List<T> GetPagedListCacheObject<T>(string rootKey, int page)
 			return rootPages[page];
 		return null;
 	}
+
+	public event Action<string> OnRemoveCacheKey;
+
+	public string GetEffectiveCacheKey(string key)
+	{
+		var effectiveKey = PrefixTenantOnKey(key);
+		return effectiveKey;
+	}
 }

src/PopForums.Sql/Repositories/SettingsRepository.cs

@@ -6,12 +6,20 @@ public SettingsRepository(ISqlObjectFactory sqlObjectFactory, ICacheHelper cache
 	{
 		_sqlObjectFactory = sqlObjectFactory;
 		_cacheHelper = cacheHelper;
+		_cacheHelper.OnRemoveCacheKey += key =>
+		{
+			var effectiveCacheKey = _cacheHelper.GetEffectiveCacheKey(CacheKey);
+			if (key == effectiveCacheKey)
+				OnSettingsInvalidated?.Invoke();
+		};
 	}
 
 	private readonly ISqlObjectFactory _sqlObjectFactory;
 	private readonly ICacheHelper _cacheHelper;
 	private const string CacheKey = "pf.settings";
 
+	public event Action OnSettingsInvalidated;
+
 	public Dictionary<string, string> Get()
 	{
 		var cached = _cacheHelper.GetCacheObject<Dictionary<string, string>>(CacheKey);
@@ -34,9 +42,4 @@ public void Save(Dictionary<string, object> dictionary)
 		});
 		_cacheHelper.RemoveCacheObject(CacheKey);
 	}
-
-	public bool IsStale(DateTime lastLoad)
-	{
-		return false;
-	}
 }

src/PopForums.Test/Configuration/SettingsTests.cs

@@ -71,12 +71,12 @@ public void LoadFromRepoWhenStale()
 	{
 		var settingsRepo = Substitute.For<ISettingsRepository>();
 		settingsRepo.Get().Returns(new Dictionary<string, string>());
-		settingsRepo.IsStale(Arg.Any<DateTime>()).Returns(true);
 		var errorLog = Substitute.For<IErrorLog>();
 		var settingsManager = new SettingsManager(settingsRepo, errorLog);
 
-		var settings = settingsManager.Current;
-		settings = settingsManager.Current;
+		_ = settingsManager.Current;
+		settingsRepo.OnSettingsInvalidated += Raise.Event<Action>();
+		_ = settingsManager.Current;
 		settingsRepo.Received(2).Get();
 	}
 
@@ -85,13 +85,12 @@ public void DoNotLoadFromRepoWhenNotStale()
 	{
 		var settingsRepo = Substitute.For<ISettingsRepository>();
 		settingsRepo.Get().Returns(new Dictionary<string, string>());
-		settingsRepo.IsStale(Arg.Any<DateTime>()).Returns(false);
 		var errorLog = Substitute.For<IErrorLog>();
 		var settingsManager = new SettingsManager(settingsRepo, errorLog);
 
-		var settings = settingsManager.Current;
-		settings = settingsManager.Current;
-		settingsRepo.Received().Get();
+		_ = settingsManager.Current;
+		_ = settingsManager.Current;
+		settingsRepo.Received(1).Get();
 	}
 
 	[Fact]

src/PopForums/Configuration/ICacheHelper.cs

@@ -37,12 +37,23 @@ public interface ICacheHelper
 	/// <param name="key"></param>
 	void RemoveCacheObject(string key);
 	/// <summary>
-	/// Get an object from cache.
+	/// Removes an object from cache.
 	/// </summary>
-	/// <typeparam name="T"></typeparam>
-	/// <param name="key"></param>
-	/// <returns></returns>
+	/// <param name="key">Key of the cache item to remove.</param>
+	/// <remarks>
+	/// Implementations of this method should call the <see cref="OnRemoveCacheKey"/> event.
+	/// </remarks>
 	T GetCacheObject<T>(string key);
 
 	List<T> GetPagedListCacheObject<T>(string rootKey, int page);
+
+	/// <summary>
+	/// Can be used as a notification mechanism for downstream actions when cache is invalidated.
+	/// </summary>
+	/// <remarks>
+	/// Implementations of ICacheHelper should call this event whenever they invalidate a cached object.
+	/// </remarks>
+	event Action<string> OnRemoveCacheKey;
+
+	string GetEffectiveCacheKey(string key);
 }

src/PopForums/Configuration/SettingsManager.cs

@@ -13,19 +13,21 @@ public SettingsManager(ISettingsRepository settingsRepository, IErrorLog errorLo
 	{
 		_settingsRepository = settingsRepository;
 		_errorLog = errorLog;
+		_settingsRepository.OnSettingsInvalidated += () =>
+		{
+			_settings = null;
+		};
 	}
 
 	private readonly ISettingsRepository _settingsRepository;
 	private readonly IErrorLog _errorLog;
-	private static DateTime _lastLoad;
-	private static readonly object SyncRoot = new object();
 	private Settings _settings;
 
 	public Settings Current
 	{
 		get
 		{
-			if (_settings == null || _settingsRepository.IsStale(_lastLoad))
+			if (_settings == null)
 				LoadSettings();
 			return _settings;
 		}
@@ -67,7 +69,6 @@ private void LoadSettings()
 			}
 		}
 		_settings = settings;
-		_lastLoad = DateTime.UtcNow;
 	}
 
 	public void Save(Settings settings)
@@ -79,14 +80,11 @@ public void Save(Settings settings)
 	public void SaveCurrent()
 	{
 		var dictionary = new Dictionary<string, object>();
-		lock (SyncRoot)
+		var properties = Current.GetType().GetProperties();
+		foreach (var property in properties)
 		{
-			var properties = Current.GetType().GetProperties();
-			foreach (var property in properties)
-			{
-				dictionary.Add(property.Name, property.GetValue(Current, null));
-			}
-			_settingsRepository.Save(dictionary);
+			dictionary.Add(property.Name, property.GetValue(Current, null));
 		}
+		_settingsRepository.Save(dictionary);
 	}
 }

src/PopForums/Extensions/ServiceCollections.cs

@@ -7,7 +7,7 @@ public static void AddPopForumsBase(this IServiceCollection services)
 		// config
 		services.AddTransient<IConfig, Config>();
 		services.AddTransient<IErrorLog, ErrorLog>();
-		services.AddTransient<ISettingsManager, SettingsManager>();
+		services.AddSingleton<ISettingsManager, SettingsManager>();
 		services.AddTransient<ITenantService, TenantService>();
 
 		// composers

src/PopForums/Repositories/ISettingsRepository.cs

@@ -4,5 +4,5 @@ public interface ISettingsRepository
 {
 	Dictionary<string, string> Get();
 	void Save(Dictionary<string, object> dictionary);
-	bool IsStale(DateTime lastLoad);
+	event Action OnSettingsInvalidated;
 }