Skip to content

Commit

Permalink
fix: Reenable party list cache, log party name look failure with nega…
Browse files Browse the repository at this point in the history
…tive cache TTL (#1395)

## 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)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## 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.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Co-authored-by: Ole Jørgen Skogstad <skogstad@softis.net>
  • Loading branch information
elsand and oskogstad authored Nov 21, 2024
1 parent 304f4da commit d18bb76
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,9 @@ public async Task<AuthorizedPartiesResult> 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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -12,24 +13,36 @@ internal sealed class PartyNameRegistryClient : IPartyNameRegistry
{
private readonly IFusionCache _cache;
private readonly HttpClient _client;
private readonly ILogger<PartyNameRegistryClient> _logger;

private static readonly JsonSerializerOptions SerializerOptions = new()
{
PropertyNameCaseInsensitive = true,
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault
};

public PartyNameRegistryClient(HttpClient client, IFusionCacheProvider cacheProvider)
public PartyNameRegistryClient(HttpClient client, IFusionCacheProvider cacheProvider, ILogger<PartyNameRegistryClient> 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<string?> GetName(string externalIdWithPrefix, CancellationToken cancellationToken)
{
return await _cache.GetOrSetAsync(
return await _cache.GetOrSetAsync<string?>(
$"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);
}

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

0 comments on commit d18bb76

Please sign in to comment.