From 8ade9c0e11b1fbe8db179b57875c0b0488affbcc Mon Sep 17 00:00:00 2001 From: Rick Mason Date: Sun, 29 Oct 2023 12:23:59 +0000 Subject: [PATCH] The player cache was not getting cleared correctly because a trailing / or page name was treated as a different cache key #627 --- ...ests.cs => PlayerCacheInvalidatorTests.cs} | 33 ++++++++++++------- .../CachedCompetitionDataSource.cs | 2 +- .../CachedMatchLocationDataSource.cs | 2 +- .../CachedPlayerDataSource.cs | 20 +++++++++-- .../PlayerCacheInvalidator.cs | 14 +++++--- 5 files changed, 49 insertions(+), 22 deletions(-) rename Stoolball.Data.MemoryCache.UnitTests/{PlayerCacheClearerTests.cs => PlayerCacheInvalidatorTests.cs} (65%) diff --git a/Stoolball.Data.MemoryCache.UnitTests/PlayerCacheClearerTests.cs b/Stoolball.Data.MemoryCache.UnitTests/PlayerCacheInvalidatorTests.cs similarity index 65% rename from Stoolball.Data.MemoryCache.UnitTests/PlayerCacheClearerTests.cs rename to Stoolball.Data.MemoryCache.UnitTests/PlayerCacheInvalidatorTests.cs index 5498cd8b..6f1e7f8e 100644 --- a/Stoolball.Data.MemoryCache.UnitTests/PlayerCacheClearerTests.cs +++ b/Stoolball.Data.MemoryCache.UnitTests/PlayerCacheInvalidatorTests.cs @@ -1,35 +1,42 @@ using System; using Moq; using Stoolball.Data.Abstractions; +using Stoolball.Routing; using Stoolball.Statistics; using Xunit; namespace Stoolball.Data.MemoryCache.UnitTests { - public class PlayerCacheClearerTests + public class PlayerCacheInvalidatorTests { private readonly Mock _cache = new(); + private readonly Mock _normaliser = new(); - [Fact] - public void Clears_cache_for_reading_players_by_PlayerRoute_and_MemberKey_and_for_player_summary_statistics() + [Theory] + [InlineData("/players/example")] + [InlineData("/players/example/")] + [InlineData("/players/example/some-page")] + public void Clears_cache_for_reading_players_by_normalised_PlayerRoute_and_MemberKey_and_for_player_summary_statistics(string route) { - var player = new Player { PlayerRoute = "/players/example", MemberKey = Guid.NewGuid() }; - var cacheClearer = new PlayerCacheInvalidator(_cache.Object); + var player = new Player { PlayerRoute = route, MemberKey = Guid.NewGuid() }; + const string normalisedRoute = "/players/example"; + _normaliser.Setup(x => x.NormaliseRouteToEntity(player.PlayerRoute, "players")).Returns(normalisedRoute); + var cacheClearer = new PlayerCacheInvalidator(_cache.Object, _normaliser.Object); cacheClearer.InvalidateCacheForPlayer(player); - _cache.Verify(x => x.InvalidateCache(nameof(IPlayerDataSource) + nameof(IPlayerDataSource.ReadPlayerByRoute) + player.PlayerRoute), Times.Once); + _cache.Verify(x => x.InvalidateCache(nameof(IPlayerDataSource) + nameof(IPlayerDataSource.ReadPlayerByRoute) + normalisedRoute), Times.Once); _cache.Verify(x => x.InvalidateCache(nameof(IPlayerDataSource) + nameof(IPlayerDataSource.ReadPlayerByMemberKey) + player.MemberKey), Times.Once); - _cache.Verify(x => x.InvalidateCache(nameof(IPlayerSummaryStatisticsDataSource) + nameof(IPlayerSummaryStatisticsDataSource.ReadBattingStatistics) + player.PlayerRoute), Times.Once); - _cache.Verify(x => x.InvalidateCache(nameof(IPlayerSummaryStatisticsDataSource) + nameof(IPlayerSummaryStatisticsDataSource.ReadBowlingStatistics) + player.PlayerRoute), Times.Once); - _cache.Verify(x => x.InvalidateCache(nameof(IPlayerSummaryStatisticsDataSource) + nameof(IPlayerSummaryStatisticsDataSource.ReadFieldingStatistics) + player.PlayerRoute), Times.Once); + _cache.Verify(x => x.InvalidateCache(nameof(IPlayerSummaryStatisticsDataSource) + nameof(IPlayerSummaryStatisticsDataSource.ReadBattingStatistics) + normalisedRoute), Times.Once); + _cache.Verify(x => x.InvalidateCache(nameof(IPlayerSummaryStatisticsDataSource) + nameof(IPlayerSummaryStatisticsDataSource.ReadBowlingStatistics) + normalisedRoute), Times.Once); + _cache.Verify(x => x.InvalidateCache(nameof(IPlayerSummaryStatisticsDataSource) + nameof(IPlayerSummaryStatisticsDataSource.ReadFieldingStatistics) + normalisedRoute), Times.Once); } [Fact] public void Skips_clearing_ReadPlayerByMemberKey_cache_for_missing_MemberKey() { var player = new Player { PlayerRoute = "/players/example", MemberKey = null }; - var cacheClearer = new PlayerCacheInvalidator(_cache.Object); + var cacheClearer = new PlayerCacheInvalidator(_cache.Object, _normaliser.Object); cacheClearer.InvalidateCacheForPlayer(player); @@ -39,9 +46,11 @@ public void Skips_clearing_ReadPlayerByMemberKey_cache_for_missing_MemberKey() [Fact] public void Throws_ArgumentNullException_for_missing_Player() { - var cacheClearer = new PlayerCacheInvalidator(_cache.Object); + var cacheClearer = new PlayerCacheInvalidator(_cache.Object, _normaliser.Object); +#nullable disable Assert.Throws(() => cacheClearer.InvalidateCacheForPlayer(null)); +#nullable enable } [Theory] @@ -50,7 +59,7 @@ public void Throws_ArgumentNullException_for_missing_Player() public void Throws_ArgumentException_for_missing_PlayerRoute(string route) { var player = new Player { PlayerRoute = route, MemberKey = Guid.NewGuid() }; - var cacheClearer = new PlayerCacheInvalidator(_cache.Object); + var cacheClearer = new PlayerCacheInvalidator(_cache.Object, _normaliser.Object); Assert.Throws(() => cacheClearer.InvalidateCacheForPlayer(player)); } diff --git a/Stoolball.Data.MemoryCache/CachedCompetitionDataSource.cs b/Stoolball.Data.MemoryCache/CachedCompetitionDataSource.cs index f926a3fc..34b53bf0 100644 --- a/Stoolball.Data.MemoryCache/CachedCompetitionDataSource.cs +++ b/Stoolball.Data.MemoryCache/CachedCompetitionDataSource.cs @@ -40,7 +40,7 @@ public async Task> ReadCompetitions(CompetitionFilter filter) } /// - public async Task ReadCompetitionByRoute(string route) + public async Task ReadCompetitionByRoute(string route) { return await _competitionDataSource.ReadCompetitionByRoute(route).ConfigureAwait(false); } diff --git a/Stoolball.Data.MemoryCache/CachedMatchLocationDataSource.cs b/Stoolball.Data.MemoryCache/CachedMatchLocationDataSource.cs index 227ae30d..0c638ed2 100644 --- a/Stoolball.Data.MemoryCache/CachedMatchLocationDataSource.cs +++ b/Stoolball.Data.MemoryCache/CachedMatchLocationDataSource.cs @@ -39,7 +39,7 @@ public async Task> ReadMatchLocations(MatchLocationFilter fi return await _readThroughCache.ReadThroughCacheAsync(async () => await _matchLocationDataSource.ReadMatchLocations(filter).ConfigureAwait(false), CachePolicy.MatchLocationsExpiration(), cacheKey, dependentCacheKey); } - public async Task ReadMatchLocationByRoute(string route, bool includeRelated = false) + public async Task ReadMatchLocationByRoute(string route, bool includeRelated = false) { return await _matchLocationDataSource.ReadMatchLocationByRoute(route, includeRelated).ConfigureAwait(false); } diff --git a/Stoolball.Data.MemoryCache/CachedPlayerDataSource.cs b/Stoolball.Data.MemoryCache/CachedPlayerDataSource.cs index d2105dda..0a2b5841 100644 --- a/Stoolball.Data.MemoryCache/CachedPlayerDataSource.cs +++ b/Stoolball.Data.MemoryCache/CachedPlayerDataSource.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Threading.Tasks; using Stoolball.Data.Abstractions; +using Stoolball.Routing; using Stoolball.Statistics; namespace Stoolball.Data.MemoryCache @@ -14,29 +15,39 @@ public class CachedPlayerDataSource : IPlayerDataSource private readonly ICacheablePlayerDataSource _playerDataSource; private readonly IPlayerFilterSerializer _playerFilterSerializer; private readonly IStatisticsFilterQueryStringSerializer _statisticsFilterSerialiser; + private readonly IRouteNormaliser _routeNormaliser; - public CachedPlayerDataSource(IReadThroughCache readThroughCache, ICacheablePlayerDataSource playerDataSource, IPlayerFilterSerializer playerFilterSerializer, IStatisticsFilterQueryStringSerializer statisticsFilterSerialiser) + public CachedPlayerDataSource(IReadThroughCache readThroughCache, + ICacheablePlayerDataSource playerDataSource, + IPlayerFilterSerializer playerFilterSerializer, + IStatisticsFilterQueryStringSerializer statisticsFilterSerialiser, + IRouteNormaliser routeNormaliser) { _readThroughCache = readThroughCache ?? throw new ArgumentNullException(nameof(readThroughCache)); _playerDataSource = playerDataSource ?? throw new ArgumentNullException(nameof(playerDataSource)); _playerFilterSerializer = playerFilterSerializer ?? throw new ArgumentNullException(nameof(playerFilterSerializer)); _statisticsFilterSerialiser = statisticsFilterSerialiser ?? throw new ArgumentNullException(nameof(statisticsFilterSerialiser)); + _routeNormaliser = routeNormaliser ?? throw new ArgumentNullException(nameof(routeNormaliser)); } + /// public async Task ReadPlayerByMemberKey(Guid key) { var cacheKey = nameof(IPlayerDataSource) + nameof(ReadPlayerByMemberKey) + key; return await _readThroughCache.ReadThroughCacheAsync(async () => await _playerDataSource.ReadPlayerByMemberKey(key), CachePolicy.StatisticsExpiration(), cacheKey, cacheKey); } + /// public async Task ReadPlayerByRoute(string route, StatisticsFilter? filter = null) { if (filter == null) { filter = new StatisticsFilter(); } - var cacheKey = nameof(IPlayerDataSource) + nameof(ReadPlayerByRoute) + route; + var normalisedRoute = _routeNormaliser.NormaliseRouteToEntity(route, "players"); + var cacheKey = nameof(IPlayerDataSource) + nameof(ReadPlayerByRoute) + normalisedRoute; var dependentCacheKey = cacheKey + _statisticsFilterSerialiser.Serialize(filter); - return await _readThroughCache.ReadThroughCacheAsync(async () => await _playerDataSource.ReadPlayerByRoute(route, filter), CachePolicy.StatisticsExpiration(), cacheKey, dependentCacheKey); + return await _readThroughCache.ReadThroughCacheAsync(async () => await _playerDataSource.ReadPlayerByRoute(normalisedRoute, filter), CachePolicy.StatisticsExpiration(), cacheKey, dependentCacheKey); } + /// public async Task> ReadPlayerIdentities(PlayerFilter filter) { var cacheKey = nameof(IPlayerDataSource) + nameof(ReadPlayerIdentities) + GranularCacheKey(filter); @@ -44,18 +55,21 @@ public async Task> ReadPlayerIdentities(PlayerFilter filter return await _readThroughCache.ReadThroughCacheAsync(async () => await _playerDataSource.ReadPlayerIdentities(filter).ConfigureAwait(false), CachePolicy.StatisticsExpiration(), cacheKey, dependentCacheKey); } + /// public async Task ReadPlayerIdentityByRoute(string route) { // only used in edit scenarios - do not cache return await _playerDataSource.ReadPlayerIdentityByRoute(route); } + /// public async Task> ReadPlayers(PlayerFilter filter) { var cacheKey = nameof(IPlayerDataSource) + nameof(ReadPlayers) + _playerFilterSerializer.Serialize(filter); return await _readThroughCache.ReadThroughCacheAsync(async () => await _playerDataSource.ReadPlayers(filter).ConfigureAwait(false), CachePolicy.StatisticsExpiration(), cacheKey, cacheKey); } + /// public async Task> ReadPlayers(PlayerFilter filter, IDbConnection connection) { var cacheKey = nameof(IPlayerDataSource) + nameof(ReadPlayers) + _playerFilterSerializer.Serialize(filter); diff --git a/Stoolball.Data.MemoryCache/PlayerCacheInvalidator.cs b/Stoolball.Data.MemoryCache/PlayerCacheInvalidator.cs index 7e1638c2..3225bcac 100644 --- a/Stoolball.Data.MemoryCache/PlayerCacheInvalidator.cs +++ b/Stoolball.Data.MemoryCache/PlayerCacheInvalidator.cs @@ -1,6 +1,7 @@ using System; using System.Linq; using Stoolball.Data.Abstractions; +using Stoolball.Routing; using Stoolball.Statistics; using Stoolball.Teams; @@ -9,10 +10,12 @@ namespace Stoolball.Data.MemoryCache public class PlayerCacheInvalidator : IPlayerCacheInvalidator { private readonly IReadThroughCache _readThroughCache; + private readonly IRouteNormaliser _routeNormaliser; - public PlayerCacheInvalidator(IReadThroughCache readThroughCache) + public PlayerCacheInvalidator(IReadThroughCache readThroughCache, IRouteNormaliser routeNormaliser) { _readThroughCache = readThroughCache ?? throw new ArgumentNullException(nameof(readThroughCache)); + _routeNormaliser = routeNormaliser ?? throw new ArgumentNullException(nameof(routeNormaliser)); } public void InvalidateCacheForPlayer(Player cacheable) @@ -27,14 +30,15 @@ public void InvalidateCacheForPlayer(Player cacheable) throw new ArgumentException($"{nameof(cacheable.PlayerRoute)} cannot be null or empty string"); } - _readThroughCache.InvalidateCache(nameof(IPlayerDataSource) + nameof(IPlayerDataSource.ReadPlayerByRoute) + cacheable.PlayerRoute); + var normalisedRoute = _routeNormaliser.NormaliseRouteToEntity(cacheable.PlayerRoute, "players"); + _readThroughCache.InvalidateCache(nameof(IPlayerDataSource) + nameof(IPlayerDataSource.ReadPlayerByRoute) + normalisedRoute); if (cacheable.MemberKey.HasValue) { _readThroughCache.InvalidateCache(nameof(IPlayerDataSource) + nameof(IPlayerDataSource.ReadPlayerByMemberKey) + cacheable.MemberKey); } - _readThroughCache.InvalidateCache(nameof(IPlayerSummaryStatisticsDataSource) + nameof(IPlayerSummaryStatisticsDataSource.ReadBattingStatistics) + cacheable.PlayerRoute); - _readThroughCache.InvalidateCache(nameof(IPlayerSummaryStatisticsDataSource) + nameof(IPlayerSummaryStatisticsDataSource.ReadBowlingStatistics) + cacheable.PlayerRoute); - _readThroughCache.InvalidateCache(nameof(IPlayerSummaryStatisticsDataSource) + nameof(IPlayerSummaryStatisticsDataSource.ReadFieldingStatistics) + cacheable.PlayerRoute); + _readThroughCache.InvalidateCache(nameof(IPlayerSummaryStatisticsDataSource) + nameof(IPlayerSummaryStatisticsDataSource.ReadBattingStatistics) + normalisedRoute); + _readThroughCache.InvalidateCache(nameof(IPlayerSummaryStatisticsDataSource) + nameof(IPlayerSummaryStatisticsDataSource.ReadBowlingStatistics) + normalisedRoute); + _readThroughCache.InvalidateCache(nameof(IPlayerSummaryStatisticsDataSource) + nameof(IPlayerSummaryStatisticsDataSource.ReadFieldingStatistics) + normalisedRoute); } public void InvalidateCacheForTeams(params Team[] teams)