From d18bb76c07bebee46adb447f0b11f614f2851ce4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Dybvik=20Langfors?= Date: Thu, 21 Nov 2024 15:16:07 +0100 Subject: [PATCH] fix: Reenable party list cache, log party name look failure with negative cache TTL (#1395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Party list cache was by a mistake disabled, this reenables it. Also, add a log entry when party name lookup fails due to remote API response indicates no name (usually due to missing party profile). Cache negative responses for just 10 seconds (instead of the default 24 hours). ## Related Issue(s) - N/A ## Verification - [x] **Your** code builds clean without any errors or warnings - [x] Manual testing done (required) - [ ] Relevant automated test added (if you find this hard, leave it and we'll help out) ## Summary by CodeRabbit - **New Features** - Enhanced caching for retrieving authorized parties, improving efficiency. - Introduced logging capabilities in the Party Name Registry client for better error tracking. - **Bug Fixes** - Improved error handling in the Party Name Registry client to log issues with missing names. - **Refactor** - Updated caching strategy in the Party Name Registry client for optimized performance. Co-authored-by: Ole Jørgen Skogstad --- .../AltinnAuthorizationClient.cs | 7 ++--- .../NameRegistry/PartyNameRegistryClient.cs | 28 ++++++++++++++++--- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs index a10b0b4e6..1bc6d9280 100644 --- a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs +++ b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs @@ -81,10 +81,9 @@ public async Task GetAuthorizedParties(IPartyIdentifier CancellationToken cancellationToken = default) { var authorizedPartiesRequest = new AuthorizedPartiesRequest(authenticatedParty); - // Disabled until https://github.com/digdir/dialogporten/issues/1226 is fixed - // var authorizedParties = await _partiesCache.GetOrSetAsync(authorizedPartiesRequest.GenerateCacheKey(), async token - // => await PerformAuthorizedPartiesRequest(authorizedPartiesRequest, token), token: cancellationToken); - var authorizedParties = await PerformAuthorizedPartiesRequest(authorizedPartiesRequest, cancellationToken); + var authorizedParties = await _partiesCache.GetOrSetAsync(authorizedPartiesRequest.GenerateCacheKey(), async token + => await PerformAuthorizedPartiesRequest(authorizedPartiesRequest, token), token: cancellationToken); + return flatten ? GetFlattenedAuthorizedParties(authorizedParties) : authorizedParties; } diff --git a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/NameRegistry/PartyNameRegistryClient.cs b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/NameRegistry/PartyNameRegistryClient.cs index 7646f30f8..0f4f70454 100644 --- a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/NameRegistry/PartyNameRegistryClient.cs +++ b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/NameRegistry/PartyNameRegistryClient.cs @@ -4,6 +4,7 @@ using Digdir.Domain.Dialogporten.Application.Externals; using Digdir.Domain.Dialogporten.Domain.Parties; using Digdir.Domain.Dialogporten.Domain.Parties.Abstractions; +using Microsoft.Extensions.Logging; using ZiggyCreatures.Caching.Fusion; namespace Digdir.Domain.Dialogporten.Infrastructure.Altinn.NameRegistry; @@ -12,6 +13,7 @@ internal sealed class PartyNameRegistryClient : IPartyNameRegistry { private readonly IFusionCache _cache; private readonly HttpClient _client; + private readonly ILogger _logger; private static readonly JsonSerializerOptions SerializerOptions = new() { @@ -19,17 +21,28 @@ internal sealed class PartyNameRegistryClient : IPartyNameRegistry DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault }; - public PartyNameRegistryClient(HttpClient client, IFusionCacheProvider cacheProvider) + public PartyNameRegistryClient(HttpClient client, IFusionCacheProvider cacheProvider, ILogger logger) { _client = client ?? throw new ArgumentNullException(nameof(client)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _cache = cacheProvider.GetCache(nameof(NameRegistry)) ?? throw new ArgumentNullException(nameof(cacheProvider)); } public async Task GetName(string externalIdWithPrefix, CancellationToken cancellationToken) { - return await _cache.GetOrSetAsync( + return await _cache.GetOrSetAsync( $"Name_{externalIdWithPrefix}", - ct => GetNameFromRegister(externalIdWithPrefix, ct), + async (ctx, ct) => + { + var name = await GetNameFromRegister(externalIdWithPrefix, ct); + if (name is null) + { + // Short negative cache + ctx.Options.Duration = TimeSpan.FromSeconds(10); + } + + return name; + }, token: cancellationToken); } @@ -48,7 +61,14 @@ public PartyNameRegistryClient(HttpClient client, IFusionCacheProvider cacheProv serializerOptions: SerializerOptions, cancellationToken: cancellationToken); - return nameLookupResult.PartyNames.FirstOrDefault()?.Name; + var name = nameLookupResult.PartyNames.FirstOrDefault()?.Name; + if (name is null) + { + // This is PII, but this is an error condition (probably due to missing Altinn profile) + _logger.LogError("Failed to get name from party name registry for external id {ExternalId}", externalIdWithPrefix); + } + + return name; } private static bool TryParse(string externalIdWithPrefix, [NotNullWhen(true)] out NameLookup? nameLookup)