Skip to content

Commit

Permalink
The player cache was not getting cleared correctly because a trailing…
Browse files Browse the repository at this point in the history
… / or page name was treated as a different cache key #627
  • Loading branch information
sussexrick committed Oct 29, 2023
1 parent 9c29991 commit 8ade9c0
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 22 deletions.
Original file line number Diff line number Diff line change
@@ -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<IReadThroughCache> _cache = new();
private readonly Mock<IRouteNormaliser> _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);

Expand All @@ -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<ArgumentNullException>(() => cacheClearer.InvalidateCacheForPlayer(null));
#nullable enable
}

[Theory]
Expand All @@ -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<ArgumentException>(() => cacheClearer.InvalidateCacheForPlayer(player));
}
Expand Down
2 changes: 1 addition & 1 deletion Stoolball.Data.MemoryCache/CachedCompetitionDataSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public async Task<List<Competition>> ReadCompetitions(CompetitionFilter filter)
}

/// <inheritdoc />
public async Task<Competition> ReadCompetitionByRoute(string route)
public async Task<Competition?> ReadCompetitionByRoute(string route)
{
return await _competitionDataSource.ReadCompetitionByRoute(route).ConfigureAwait(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public async Task<List<MatchLocation>> ReadMatchLocations(MatchLocationFilter fi
return await _readThroughCache.ReadThroughCacheAsync(async () => await _matchLocationDataSource.ReadMatchLocations(filter).ConfigureAwait(false), CachePolicy.MatchLocationsExpiration(), cacheKey, dependentCacheKey);
}

public async Task<MatchLocation> ReadMatchLocationByRoute(string route, bool includeRelated = false)
public async Task<MatchLocation?> ReadMatchLocationByRoute(string route, bool includeRelated = false)
{
return await _matchLocationDataSource.ReadMatchLocationByRoute(route, includeRelated).ConfigureAwait(false);
}
Expand Down
20 changes: 17 additions & 3 deletions Stoolball.Data.MemoryCache/CachedPlayerDataSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -14,48 +15,61 @@ 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));
}

/// <inheritdoc />
public async Task<Player?> ReadPlayerByMemberKey(Guid key)
{
var cacheKey = nameof(IPlayerDataSource) + nameof(ReadPlayerByMemberKey) + key;
return await _readThroughCache.ReadThroughCacheAsync(async () => await _playerDataSource.ReadPlayerByMemberKey(key), CachePolicy.StatisticsExpiration(), cacheKey, cacheKey);
}

/// <inheritdoc />
public async Task<Player?> 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);
}

/// <inheritdoc />
public async Task<List<PlayerIdentity>> ReadPlayerIdentities(PlayerFilter filter)
{
var cacheKey = nameof(IPlayerDataSource) + nameof(ReadPlayerIdentities) + GranularCacheKey(filter);
var dependentCacheKey = cacheKey + _playerFilterSerializer.Serialize(filter);
return await _readThroughCache.ReadThroughCacheAsync(async () => await _playerDataSource.ReadPlayerIdentities(filter).ConfigureAwait(false), CachePolicy.StatisticsExpiration(), cacheKey, dependentCacheKey);
}

/// <inheritdoc />
public async Task<PlayerIdentity?> ReadPlayerIdentityByRoute(string route)
{
// only used in edit scenarios - do not cache
return await _playerDataSource.ReadPlayerIdentityByRoute(route);
}

/// <inheritdoc />
public async Task<List<Player>> 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);
}

/// <inheritdoc />
public async Task<List<Player>> ReadPlayers(PlayerFilter filter, IDbConnection connection)
{
var cacheKey = nameof(IPlayerDataSource) + nameof(ReadPlayers) + _playerFilterSerializer.Serialize(filter);
Expand Down
14 changes: 9 additions & 5 deletions Stoolball.Data.MemoryCache/PlayerCacheInvalidator.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Linq;
using Stoolball.Data.Abstractions;
using Stoolball.Routing;
using Stoolball.Statistics;
using Stoolball.Teams;

Expand All @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit 8ade9c0

Please sign in to comment.