Skip to content

Commit

Permalink
fix(237738): improve redis caching edge cases (#920)
Browse files Browse the repository at this point in the history
* fix: Perform redis caching at repository level and fix dependencies

* chore: Linted code for plan-technology-for-your-school.sln solution

* fix: fix not found pages being in cache forever

* fix: move new deps into background job

* chore: Linted code for plan-technology-for-your-school.sln solution

* fix: fix cache test

* fix: remove duplicate dep storage

* chore: Linted code for plan-technology-for-your-school.sln solution

* tests: add tests for improved redis cache invalidation

* feat: clear redis cache on deployment

* fix: format json redis key for better viewing

* chore: separate content repository from cached content repository

* chore: simplify redis format

* fix: cache fetch by id and add tests

* chore: change order of key

* chore: Linted code for plan-technology-for-your-school.sln solution

* tests: fix tests for reordered key

* fix: content type should start with lower case

* tests: update tests for casing

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
katie-gardner and github-actions[bot] authored Jan 9, 2025
1 parent 9c3840c commit 891b5fc
Show file tree
Hide file tree
Showing 29 changed files with 381 additions and 151 deletions.
29 changes: 28 additions & 1 deletion .github/workflows/deploy-image.yml
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,40 @@ jobs:
--output none
&> /dev/null
flush-redis-cache:
runs-on: ubuntu-22.04
name: Flush Redis Cache on ${{ inputs.environment }}
environment: ${{ inputs.environment }}
needs: [upgrade-database, pull-image-from-gcr-and-publish-to-acr, deploy-image]

steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.ref }}

- name: Azure CLI Login
uses: ./.github/actions/azure-login
with:
az_tenant_id: ${{ secrets.AZ_TENANT_ID }}
az_subscription_id: ${{ secrets.AZ_SUBSCRIPTION_ID }}
az_client_id: ${{ secrets.AZ_CLIENT_ID }}
az_client_secret: ${{ secrets.AZ_CLIENT_SECRET }}

- name: Flush Redis Cache
uses: azure/CLI@v1
id: azure
with:
azcliversion: 2.67.0
inlineScript: |
az redis flush --name ${{ secrets.AZ_ENVIRONMENT }}${{ secrets.DFE_PROJECT_NAME }} --resource-group ${{ secrets.AZ_ENVIRONMENT }}${{ secrets.DFE_PROJECT_NAME }} --yes
run-e2e-testing:
runs-on: ubuntu-22.04
name: Clear Database & Run E2E Tests on ${{ inputs.environment }}
if: ${{ inputs.environment == 'tst' || inputs.environment == 'staging' }}
environment: ${{ inputs.environment }}
needs:
[upgrade-database, pull-image-from-gcr-and-publish-to-acr, deploy-image]
[upgrade-database, pull-image-from-gcr-and-publish-to-acr, deploy-image, flush-redis-cache]
env:
az_keyvault_name: ${{ secrets.AZ_ENVIRONMENT }}${{ secrets.DFE_PROJECT_NAME }}-kv
az_keyvault_database_connectionstring_name: ${{ secrets.AZ_KEYVAULT_DATABASE_CONNECTIONSTRING_NAME }}
Expand Down
3 changes: 2 additions & 1 deletion src/Dfe.PlanTech.Application/Caching/Interfaces/ICmsCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public interface ICmsCache : IDistributedCache
/// Then removes the dependency array itself
/// </summary>
/// <param name="contentComponentId">Id of component to invalidate dependencies of</param>
/// <param name="contentType">Name of the content type of the component being invalidated</param>
/// <returns></returns>
Task InvalidateCacheAsync(string contentComponentId);
Task InvalidateCacheAsync(string contentComponentId, string contentType);
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using Dfe.PlanTech.Application.Caching.Interfaces;
using Dfe.PlanTech.Application.Core;
using Dfe.PlanTech.Application.Persistence.Interfaces;
using Dfe.PlanTech.Domain.Content.Interfaces;
Expand All @@ -16,21 +15,18 @@ public class GetEntityFromContentfulQuery : ContentRetriever, IGetEntityFromCont
public const string ExceptionMessageEntityContentful = "Error fetching Entity from Contentful";

private readonly ILogger<GetEntityFromContentfulQuery> _logger;
private readonly ICmsCache _cache;

public GetEntityFromContentfulQuery(ILogger<GetEntityFromContentfulQuery> logger, IContentRepository repository, ICmsCache cache) : base(repository)
public GetEntityFromContentfulQuery(ILogger<GetEntityFromContentfulQuery> logger, IContentRepository repository) : base(repository)
{
_logger = logger;
_cache = cache;
}

public async Task<TContent?> GetEntityById<TContent>(string contentId, CancellationToken cancellationToken = default)
where TContent : ContentComponent
{
try
{
return await _cache.GetOrCreateAsync($"Entity:{contentId}",
() => repository.GetEntityById<TContent>(contentId, cancellationToken: cancellationToken));
return await repository.GetEntityById<TContent>(contentId, cancellationToken: cancellationToken);
}
catch (Exception ex)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@

using Dfe.PlanTech.Application.Caching.Interfaces;
using Dfe.PlanTech.Application.Core;
using Dfe.PlanTech.Application.Persistence.Interfaces;
using Dfe.PlanTech.Domain.Content.Interfaces;
Expand All @@ -16,20 +14,17 @@ public class GetNavigationQuery : ContentRetriever, IGetNavigationQuery
public const string ExceptionMessageContentful = "Error getting navigation links from Contentful";

private readonly ILogger<GetNavigationQuery> _logger;
private readonly ICmsCache _cache;

public GetNavigationQuery(ILogger<GetNavigationQuery> logger, IContentRepository repository, ICmsCache cache) : base(repository)
public GetNavigationQuery(ILogger<GetNavigationQuery> logger, IContentRepository repository) : base(repository)
{
_logger = logger;
_cache = cache;
}

public async Task<IEnumerable<INavigationLink>> GetNavigationLinks(CancellationToken cancellationToken = default)
{
try
{
var navigationLinks = await _cache.GetOrCreateAsync("NavigationLinks", () => repository.GetEntities<NavigationLink>(cancellationToken)) ?? [];
return navigationLinks;
return await repository.GetEntities<NavigationLink>(cancellationToken);
}
catch (Exception ex)
{
Expand All @@ -42,7 +37,7 @@ public async Task<IEnumerable<INavigationLink>> GetNavigationLinks(CancellationT
{
try
{
return await _cache.GetOrCreateAsync($"NavigationLink:{contentId}", () => repository.GetEntityById<NavigationLink?>(contentId, cancellationToken: cancellationToken));
return await repository.GetEntityById<NavigationLink?>(contentId, cancellationToken: cancellationToken);
}
catch (Exception ex)
{
Expand Down
10 changes: 3 additions & 7 deletions src/Dfe.PlanTech.Application/Content/Queries/GetPageQuery.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using Dfe.PlanTech.Application.Caching.Interfaces;
using Dfe.PlanTech.Application.Exceptions;
using Dfe.PlanTech.Application.Persistence.Interfaces;
using Dfe.PlanTech.Application.Persistence.Models;
Expand All @@ -12,14 +11,12 @@ namespace Dfe.PlanTech.Application.Content.Queries;

public class GetPageQuery : IGetPageQuery
{
private readonly ICmsCache _cache;
private readonly IContentRepository _repository;
private readonly ILogger<GetPageQuery> _logger;
private readonly GetPageFromContentfulOptions _options;

public GetPageQuery(ICmsCache cache, IContentRepository repository, ILogger<GetPageQuery> logger, GetPageFromContentfulOptions options)
public GetPageQuery(IContentRepository repository, ILogger<GetPageQuery> logger, GetPageFromContentfulOptions options)
{
_cache = cache;
_repository = repository;
_logger = logger;
_options = options;
Expand Down Expand Up @@ -52,8 +49,7 @@ public GetPageQuery(ICmsCache cache, IContentRepository repository, ILogger<GetP
{
try
{
return await _cache.GetOrCreateAsync($"Page:{pageId}", () =>
_repository.GetEntityById<Page?>(pageId, cancellationToken: cancellationToken));
return await _repository.GetEntityById<Page?>(pageId, cancellationToken: cancellationToken);
}
catch (Exception ex)
{
Expand All @@ -67,7 +63,7 @@ public GetPageQuery(ICmsCache cache, IContentRepository repository, ILogger<GetP
{
try
{
var pages = await _cache.GetOrCreateAsync($"Page:{slug}", () => _repository.GetEntities<Page?>(options, cancellationToken)) ?? [];
var pages = await _repository.GetEntities<Page?>(options, cancellationToken);

var page = pages.FirstOrDefault();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using Dfe.PlanTech.Application.Caching.Interfaces;
using Dfe.PlanTech.Application.Persistence.Interfaces;
using Dfe.PlanTech.Application.Persistence.Models;
using Dfe.PlanTech.Domain.Content.Interfaces;
Expand All @@ -11,14 +10,12 @@
namespace Dfe.PlanTech.Application.Content.Queries;

public class GetSubTopicRecommendationQuery(IContentRepository repository,
ILogger<GetSubTopicRecommendationQuery> logger,
ICmsCache cache) : IGetSubTopicRecommendationQuery
ILogger<GetSubTopicRecommendationQuery> logger) : IGetSubTopicRecommendationQuery
{
public async Task<SubtopicRecommendation?> GetSubTopicRecommendation(string subtopicId, CancellationToken cancellationToken = default)
{
var options = CreateGetEntityOptions(subtopicId);
var subTopicRecommendations = await cache.GetOrCreateAsync($"SubtopicRecommendation:{subtopicId}",
() => repository.GetEntities<SubtopicRecommendation>(options, cancellationToken)) ?? [];
var subTopicRecommendations = await repository.GetEntities<SubtopicRecommendation>(options, cancellationToken);

var subtopicRecommendation = subTopicRecommendations.FirstOrDefault();

Expand All @@ -35,8 +32,7 @@ public class GetSubTopicRecommendationQuery(IContentRepository repository,
var options = CreateGetEntityOptions(subtopicId, 2);
options.Select = ["fields.intros", "sys"];

var subtopicRecommendations = await cache.GetOrCreateAsync($"RecommendationViewDto:{subtopicId}",
() => repository.GetEntities<SubtopicRecommendation>(options, cancellationToken)) ?? [];
var subtopicRecommendations = await repository.GetEntities<SubtopicRecommendation>(options, cancellationToken);

var subtopicRecommendation = subtopicRecommendations.FirstOrDefault();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public async Task<IServiceBusResult> ProcessMessage(string subject, string body,
{
var payload = MapMessageToPayload(body);

await cache.InvalidateCacheAsync(payload.Sys.Id);
await cache.InvalidateCacheAsync(payload.Sys.Id, payload.ContentType);
return new ServiceBusSuccessResult();
}
catch (Exception ex) when (ex is JsonException)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Dfe.PlanTech.Application.Persistence.Models;
using Dfe.PlanTech.Domain.Persistence.Interfaces;

namespace Dfe.PlanTech.Application.Persistence.Interfaces;
Expand All @@ -18,14 +19,12 @@ public interface IContentRepository
Task<TEntity?> GetEntityById<TEntity>(string id, int include = 2, CancellationToken cancellationToken = default);

/// <summary>
/// Get all entities of the specified type.
/// Get options to use for fetching an Entity by Id
/// </summary>
/// <param name="entityTypeId"></param>
/// <param name="queries">Additional filtere</param>
/// <param name="cancellationToken"></param>
/// <typeparam name="TEntity"></typeparam>
/// <param name="id"></param>
/// <param name="include"></param>
/// <returns></returns>
Task<IEnumerable<TEntity>> GetEntities<TEntity>(string entityTypeId, IGetEntitiesOptions? options, CancellationToken cancellationToken = default);
GetEntitiesOptions GetEntityByIdOptions(string id, int include = 2);

/// <summary>
/// Get all entities of the specified type, using the name of the generic parameter's type as the entity type id (to lower case).
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System.Text;
using Dfe.PlanTech.Domain.Persistence.Interfaces;
using Dfe.PlanTech.Infrastructure.Application.Models;

namespace Dfe.PlanTech.Application.Persistence.Models;

Expand Down Expand Up @@ -29,4 +31,35 @@ public GetEntitiesOptions()
public IEnumerable<IContentQuery>? Queries { get; init; }

public int Include { get; init; } = 2;

public string SerializeToRedisFormat()
{
var builder = new StringBuilder();

builder.Append(":Include=");
builder.Append(Include);

if (Select != null && Select.Any())
builder.Append($":Select=[{string.Join(",", Select)}]");

if (Queries != null && Queries.Any())
{
builder.Append(":Queries=[");
foreach (var query in Queries)
{
builder.Append(query.Field);

if (query is ContentQueryEquals queryEquals)
builder.Append($"={queryEquals.Value}");
else if (query is ContentQueryIncludes queryIncludes)
builder.Append($"=[{string.Join(',', queryIncludes.Value)}]");

builder.Append(',');
}
builder.Length--;
builder.Append(']');
}

return builder.ToString();
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using Dfe.PlanTech.Application.Caching.Interfaces;
using Dfe.PlanTech.Application.Core;
using Dfe.PlanTech.Application.Exceptions;
using Dfe.PlanTech.Application.Persistence.Interfaces;
Expand All @@ -12,11 +11,9 @@ namespace Dfe.PlanTech.Application.Questionnaire.Queries;
public class GetSectionQuery : ContentRetriever, IGetSectionQuery
{
public const string SlugFieldPath = "fields.interstitialPage.fields.slug";
private readonly ICmsCache _cache;

public GetSectionQuery(IContentRepository repository, ICmsCache cache) : base(repository)
public GetSectionQuery(IContentRepository repository) : base(repository)
{
_cache = cache;
}

public async Task<Section?> GetSectionBySlug(string sectionSlug, CancellationToken cancellationToken = default)
Expand All @@ -40,7 +37,7 @@ public GetSectionQuery(IContentRepository repository, ICmsCache cache) : base(re

try
{
var sections = await _cache.GetOrCreateAsync($"Section:{sectionSlug}", () => repository.GetEntities<Section>(options, cancellationToken)) ?? [];
var sections = await repository.GetEntities<Section>(options, cancellationToken);
return sections.FirstOrDefault();
}
catch (Exception ex)
Expand All @@ -56,13 +53,9 @@ public GetSectionQuery(IContentRepository repository, ICmsCache cache) : base(re
{
try
{
var sections = await _cache.GetOrCreateAsync("Sections", async () =>
{
var options = new GetEntitiesOptions(include: 3);
var sections = await repository.GetEntities<Section>(options, cancellationToken);
return sections.Select(MapSection);
}) ?? [];
return sections;
var options = new GetEntitiesOptions(include: 3);
var sections = await repository.GetEntities<Section>(options, cancellationToken);
return sections.Select(MapSection);
}
catch (Exception ex)
{
Expand Down
2 changes: 2 additions & 0 deletions src/Dfe.PlanTech.Domain/Extensions/StringHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,6 @@ private static string ReplaceWhitespaceCharacters(this string text, string repla

private static string ReplaceNonSlugCharacters(this string text, string replacement)
=> MatchNonAlphaNumericExceptHyphensPattern().Replace(text, replacement);

public static string FirstCharToLower(this string input) => char.ToLower(input[0]) + input[1..];
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,6 @@ public interface IGetEntitiesOptions
/// If null, return all
/// </remarks>
public IEnumerable<string>? Select { get; set; }

public string SerializeToRedisFormat();
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ public static IServiceCollection SetupContentfulClient(this IServiceCollection s

services.AddTransient<IGetSubmissionStatusesQuery, GetSubmissionStatusesQuery>();
services.AddScoped<IContentfulClient, ContentfulClient>();
services.AddScoped<IContentRepository, ContentfulRepository>();
services.AddKeyedScoped<IContentRepository, ContentfulRepository>("contentfulRepository");
services.AddScoped<IContentRepository, CachedContentfulRepository>();


services.SetupRichTextRenderer();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
using Dfe.PlanTech.Application.Caching.Interfaces;
using Dfe.PlanTech.Application.Persistence.Interfaces;
using Dfe.PlanTech.Application.Persistence.Models;
using Dfe.PlanTech.Domain.Persistence.Interfaces;
using Dfe.PlanTech.Infrastructure.Contentful.Helpers;
using Microsoft.Extensions.DependencyInjection;

namespace Dfe.PlanTech.Infrastructure.Contentful.Persistence;

/// <summary>
/// Encapsulates ContentfulClient functionality, whilst abstracting through the IEntityRepository interface
/// </summary>
/// <see href="IEntityRepository"/>
public class CachedContentfulRepository : IContentRepository
{
private readonly IContentRepository _contentRepository;
private readonly ICmsCache _cache;

public CachedContentfulRepository([FromKeyedServices("contentfulRepository")] IContentRepository contentRepository, ICmsCache cache)
{
_contentRepository = contentRepository ?? throw new ArgumentNullException(nameof(contentRepository));
_cache = cache ?? throw new ArgumentNullException(nameof(cache));
}

public async Task<IEnumerable<TEntity>> GetEntities<TEntity>(CancellationToken cancellationToken = default)
{
string contentType = GetContentTypeName<TEntity>();
var key = $"{contentType}s";

return await _cache.GetOrCreateAsync(key, async () => await _contentRepository.GetEntities<TEntity>(cancellationToken)) ?? [];
}

public async Task<IEnumerable<TEntity>> GetEntities<TEntity>(IGetEntitiesOptions options, CancellationToken cancellationToken = default)
{
var contentType = GetContentTypeName<TEntity>();
var jsonOptions = options.SerializeToRedisFormat();
var key = $"{contentType}{jsonOptions}";

return await _cache.GetOrCreateAsync(key, async () => await _contentRepository.GetEntities<TEntity>(options, cancellationToken)) ?? [];
}

public async Task<TEntity?> GetEntityById<TEntity>(string id, int include = 2, CancellationToken cancellationToken = default)
{
var options = GetEntityByIdOptions(id, include);
var entities = (await GetEntities<TEntity>(options, cancellationToken)).ToList();

if (entities.Count > 1)
throw new GetEntitiesException($"Found more than 1 entity with id {id}");

return entities.FirstOrDefault();
}

public GetEntitiesOptions GetEntityByIdOptions(string id, int include = 2)
{
return _contentRepository.GetEntityByIdOptions(id, include);
}

private static string GetContentTypeName<TEntity>()
{
var name = typeof(TEntity).Name;
return name == "ContentSupportPage" ? name : name.FirstCharToLower();
}
}
Loading

0 comments on commit 891b5fc

Please sign in to comment.