From c8ba00dd4fab5fe11b10f08f1f550f708d84cfb2 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Thu, 27 Nov 2025 11:20:38 +0100 Subject: [PATCH 01/14] Add distributed caching for GCP ID tokens --- Directory.Packages.props | 1 + .../TelemetryConstants.cs | 6 + .../Aws/IParameterProvider.cs | 4 + .../Caching/DynamoDbDistributedCache.cs | 152 ++++++++++++++++++ .../Caching/IDistributedCache.cs | 29 ++++ .../Caching/InMemoryDistributedCache.cs | 55 +++++++ .../Caching/MultiLayerCache.cs | 129 +++++++++++++++ ...ic.Documentation.Api.Infrastructure.csproj | 1 + .../Gcp/GcpIdTokenProvider.cs | 49 ++++-- .../OpenTelemetry/OpenTelemetryExtensions.cs | 1 + .../ServicesExtension.cs | 59 ++++++- 11 files changed, 471 insertions(+), 15 deletions(-) create mode 100644 src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs create mode 100644 src/api/Elastic.Documentation.Api.Infrastructure/Caching/IDistributedCache.cs create mode 100644 src/api/Elastic.Documentation.Api.Infrastructure/Caching/InMemoryDistributedCache.cs create mode 100644 src/api/Elastic.Documentation.Api.Infrastructure/Caching/MultiLayerCache.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index 566dca579..6d43beffa 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -23,6 +23,7 @@ + diff --git a/src/api/Elastic.Documentation.Api.Core/TelemetryConstants.cs b/src/api/Elastic.Documentation.Api.Core/TelemetryConstants.cs index 6a8d2683c..e6d8d4bd8 100644 --- a/src/api/Elastic.Documentation.Api.Core/TelemetryConstants.cs +++ b/src/api/Elastic.Documentation.Api.Core/TelemetryConstants.cs @@ -31,4 +31,10 @@ public static class TelemetryConstants /// Used to trace frontend telemetry proxying. /// public const string OtlpProxySourceName = "Elastic.Documentation.Api.OtlpProxy"; + + /// + /// ActivitySource name for distributed cache operations. + /// Used to trace cache hits, misses, and performance. + /// + public const string CacheSourceName = "Elastic.Documentation.Api.Cache"; } diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Aws/IParameterProvider.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Aws/IParameterProvider.cs index c35dac454..b911b856d 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Aws/IParameterProvider.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Aws/IParameterProvider.cs @@ -4,6 +4,10 @@ namespace Elastic.Documentation.Api.Infrastructure.Aws; +/// +/// Abstraction for retrieving configuration parameters. +/// Infrastructure concern: Used by other Infrastructure adapters to get configuration. +/// public interface IParameterProvider { Task GetParam(string name, bool withDecryption = true, Cancel ctx = default); diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs new file mode 100644 index 000000000..923727224 --- /dev/null +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs @@ -0,0 +1,152 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Diagnostics; +using System.Globalization; +using Amazon.DynamoDBv2; +using Amazon.DynamoDBv2.Model; +using Elastic.Documentation.Api.Core; +using Microsoft.Extensions.Logging; + +namespace Elastic.Documentation.Api.Infrastructure.Caching; + +/// +/// DynamoDB implementation of for Lambda environments. +/// Provides distributed caching across all Lambda containers using DynamoDB as backing store. +/// Clean Code: Constructor injection (Dependency Inversion), small focused methods. +/// +public sealed class DynamoDbDistributedCache(IAmazonDynamoDB dynamoDb, string tableName, ILogger logger) : IDistributedCache +{ + private static readonly ActivitySource ActivitySource = new(TelemetryConstants.CacheSourceName); + + private readonly IAmazonDynamoDB _dynamoDb = dynamoDb; + private readonly string _tableName = tableName; + private readonly ILogger _logger = logger; + + // DynamoDB attribute names + private const string AttributeCacheKey = "CacheKey"; + private const string AttributeValue = "Value"; + private const string AttributeExpiresAt = "ExpiresAt"; + private const string AttributeTtl = "TTL"; + + public async Task GetAsync(string key, Cancel ct = default) + { + using var activity = ActivitySource.StartActivity("get cache", ActivityKind.Client); + _ = (activity?.SetTag("cache.key", key)); + _ = (activity?.SetTag("cache.table", _tableName)); + _ = (activity?.SetTag("cache.backend", "dynamodb")); + + try + { + var response = await _dynamoDb.GetItemAsync(new GetItemRequest + { + TableName = _tableName, + Key = new Dictionary + { + [AttributeCacheKey] = new AttributeValue { S = key } + } + }, ct); + + if (!response.IsItemSet) + { + _ = (activity?.SetTag("cache.hit", false)); + _logger.LogDebug("Cache miss for key: {CacheKey}", key); + return null; + } + + // Check if expired (application-level check, DynamoDB TTL is for cleanup) + if (IsExpired(response.Item)) + { + _ = (activity?.SetTag("cache.hit", false)); + _ = (activity?.SetTag("cache.expired", true)); + _logger.LogDebug("Cache expired for key: {CacheKey}", key); + return null; + } + + var value = response.Item.TryGetValue(AttributeValue, out var valueAttr) + ? valueAttr.S + : null; + + _ = (activity?.SetTag("cache.hit", value != null)); + if (value != null) + { + _logger.LogDebug("Cache hit for key: {CacheKey}", key); + } + + return value; + } + catch (ResourceNotFoundException ex) + { + // Table doesn't exist - return null gracefully + // Infrastructure should create table, but don't fail hard in dev + _ = (activity?.SetTag("cache.error", "table_not_found")); + _logger.LogWarning(ex, "DynamoDB table {TableName} not found. Cache operations will fail gracefully.", _tableName); + return null; + } + catch (Exception ex) + { + _ = (activity?.SetStatus(ActivityStatusCode.Error, ex.Message)); + _logger.LogError(ex, "Error retrieving cache key {CacheKey} from DynamoDB", key); + return null; // Fail gracefully + } + } + + public async Task SetAsync(string key, string value, TimeSpan ttl, Cancel ct = default) + { + using var activity = ActivitySource.StartActivity("set cache", ActivityKind.Client); + _ = (activity?.SetTag("cache.key", key)); + _ = (activity?.SetTag("cache.table", _tableName)); + _ = (activity?.SetTag("cache.backend", "dynamodb")); + _ = (activity?.SetTag("cache.ttl", ttl.TotalSeconds)); + + try + { + var expiresAt = DateTimeOffset.UtcNow.Add(ttl); + var ttlTimestamp = expiresAt.ToUnixTimeSeconds(); + + _ = await _dynamoDb.PutItemAsync(new PutItemRequest + { + TableName = _tableName, + Item = new Dictionary + { + [AttributeCacheKey] = new AttributeValue { S = key }, + [AttributeValue] = new AttributeValue { S = value }, + [AttributeExpiresAt] = new AttributeValue { N = ttlTimestamp.ToString(CultureInfo.InvariantCulture) }, + [AttributeTtl] = new AttributeValue { N = ttlTimestamp.ToString(CultureInfo.InvariantCulture) } + } + }, ct); + + _logger.LogDebug("Cache set for key: {CacheKey}, TTL: {TTL}s", key, ttl.TotalSeconds); + } + catch (ResourceNotFoundException ex) + { + // Table doesn't exist - fail silently in dev, log in production + // Infrastructure should create table before deployment + _ = (activity?.SetTag("cache.error", "table_not_found")); + _logger.LogWarning(ex, "DynamoDB table {TableName} not found. Unable to cache key {CacheKey}.", _tableName, key); + } + catch (Exception ex) + { + _ = (activity?.SetStatus(ActivityStatusCode.Error, ex.Message)); + _logger.LogError(ex, "Error setting cache key {CacheKey} in DynamoDB", key); + // Fail gracefully - don't throw + } + } + + /// + /// Checks if a DynamoDB item has expired based on ExpiresAt attribute. + /// Clean Code: Single-purpose helper method with intention-revealing name. + /// + private static bool IsExpired(Dictionary item) + { + if (!item.TryGetValue(AttributeExpiresAt, out var expiresAtAttr)) + return true; // No expiration timestamp = treat as expired + + if (!long.TryParse(expiresAtAttr.N, out var expiresAtUnix)) + return true; // Invalid timestamp = treat as expired + + var expiresAt = DateTimeOffset.FromUnixTimeSeconds(expiresAtUnix); + return expiresAt <= DateTimeOffset.UtcNow; + } +} diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/IDistributedCache.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/IDistributedCache.cs new file mode 100644 index 000000000..20669354e --- /dev/null +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/IDistributedCache.cs @@ -0,0 +1,29 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +namespace Elastic.Documentation.Api.Infrastructure.Caching; + +/// +/// Abstraction for distributed caching across Lambda invocations. +/// Infrastructure concern: Used by other Infrastructure adapters for caching. +/// +public interface IDistributedCache +{ + /// + /// Retrieves a cached value by key. + /// + /// Cache key following pattern: {category}:{identifier} (e.g., "idtoken:https://example.com") + /// Cancellation token + /// Cached value as string, or null if not found or expired + Task GetAsync(string key, Cancel ct = default); + + /// + /// Stores a value in the cache with a time-to-live. + /// + /// Cache key following pattern: {category}:{identifier} + /// Value to cache (typically JSON-serialized data) + /// Time-to-live duration + /// Cancellation token + Task SetAsync(string key, string value, TimeSpan ttl, Cancel ct = default); +} diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/InMemoryDistributedCache.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/InMemoryDistributedCache.cs new file mode 100644 index 000000000..50256cc05 --- /dev/null +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/InMemoryDistributedCache.cs @@ -0,0 +1,55 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Collections.Concurrent; + +namespace Elastic.Documentation.Api.Infrastructure.Caching; + +/// +/// In-memory implementation of for local development. +/// Uses ConcurrentDictionary for thread-safe storage with TTL-based expiration. +/// Clean Code: Sealed class (not meant for inheritance), single responsibility. +/// +public sealed class InMemoryDistributedCache : IDistributedCache +{ + private readonly ConcurrentDictionary _cache = new(); + + /// + /// Immutable cache entry with value and expiration timestamp. + /// Clean Code: Record type ensures immutability. + /// + private sealed record CacheEntry(string Value, DateTimeOffset ExpiresAt); + + public Task GetAsync(string key, Cancel ct = default) + { + if (_cache.TryGetValue(key, out var entry)) + { + if (IsExpired(entry)) + { + // Remove expired entry + _ = _cache.TryRemove(key, out _); + return Task.FromResult(null); + } + + return Task.FromResult(entry.Value); + } + + return Task.FromResult(null); + } + + public Task SetAsync(string key, string value, TimeSpan ttl, Cancel ct = default) + { + var expiresAt = DateTimeOffset.UtcNow.Add(ttl); + var entry = new CacheEntry(value, expiresAt); + _ = _cache.AddOrUpdate(key, entry, (_, _) => entry); + return Task.CompletedTask; + } + + /// + /// Checks if a cache entry has expired. + /// Clean Code: Single-purpose helper method with intention-revealing name. + /// + private static bool IsExpired(CacheEntry entry) => + entry.ExpiresAt <= DateTimeOffset.UtcNow; +} diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/MultiLayerCache.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/MultiLayerCache.cs new file mode 100644 index 000000000..434b1ce9f --- /dev/null +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/MultiLayerCache.cs @@ -0,0 +1,129 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Collections.Concurrent; +using System.Diagnostics; +using Elastic.Documentation.Api.Core; +using Microsoft.Extensions.Logging; + +namespace Elastic.Documentation.Api.Infrastructure.Caching; + +/// +/// Multi-layer cache decorator implementing L1 (in-memory) + L2 (distributed) caching strategy. +/// Optimizes Lambda warm-start performance while maintaining cross-container cache sharing. +/// Design Pattern: Decorator - Adds L1 layer to existing cache implementation. +/// Caching Strategy: Cache-aside (read-through) and write-through patterns. +/// +public sealed class MultiLayerCache(IDistributedCache l2Cache, ILogger logger) : IDistributedCache +{ + private static readonly ActivitySource ActivitySource = new(TelemetryConstants.CacheSourceName); + + // L1 Cache: Static in-memory cache survives across Lambda warm starts + // Thread-safe for concurrent requests within same Lambda container + private static readonly ConcurrentDictionary L1Cache = new(); + + // L2 Cache: DynamoDB for cross-container sharing + private readonly IDistributedCache _l2Cache = l2Cache; + private readonly ILogger _logger = logger; + + /// + /// Immutable L1 cache entry with value and expiration timestamp. + /// Clean Code: Record type ensures immutability, intention-revealing name. + /// + private sealed record L1CacheEntry(string Value, DateTimeOffset ExpiresAt); + + public async Task GetAsync(string key, Cancel ct = default) + { + using var activity = ActivitySource.StartActivity("get cache", ActivityKind.Internal); + _ = (activity?.SetTag("cache.key", key)); + _ = (activity?.SetTag("cache.backend", "multilayer")); + + // L1: Check in-memory cache first (fastest) + if (TryGetFromL1(key, out var value)) + { + _ = (activity?.SetTag("cache.l1.hit", true)); + _ = (activity?.SetTag("cache.l2.hit", false)); + _logger.LogDebug("L1 cache hit for key: {CacheKey}", key); + return value; + } + + _ = (activity?.SetTag("cache.l1.hit", false)); + + // L2: Check distributed cache (DynamoDB) + var l2Value = await _l2Cache.GetAsync(key, ct); + + if (l2Value != null) + { + _ = (activity?.SetTag("cache.l2.hit", true)); + _logger.LogDebug("L2 cache hit for key: {CacheKey}, populating L1", key); + + // Populate L1 cache for future requests in this container + // Use a reasonable TTL for L1 (1 hour) to match ID token lifetime + PopulateL1(key, l2Value, TimeSpan.FromHours(1)); + } + else + { + _ = (activity?.SetTag("cache.l2.hit", false)); + _logger.LogDebug("Cache miss (L1 and L2) for key: {CacheKey}", key); + } + + return l2Value; + } + + public async Task SetAsync(string key, string value, TimeSpan ttl, Cancel ct = default) + { + using var activity = ActivitySource.StartActivity("set cache", ActivityKind.Internal); + _ = (activity?.SetTag("cache.key", key)); + _ = (activity?.SetTag("cache.backend", "multilayer")); + _ = (activity?.SetTag("cache.ttl", ttl.TotalSeconds)); + + // Write-through: Update both L1 and L2 + PopulateL1(key, value, ttl); + _logger.LogDebug("Writing to L1 and L2 cache for key: {CacheKey}, TTL: {TTL}s", key, ttl.TotalSeconds); + + await _l2Cache.SetAsync(key, value, ttl, ct); + } + + /// + /// Attempts to retrieve value from L1 cache. + /// Clean Code: Single Responsibility - only handles L1 retrieval logic. + /// + private static bool TryGetFromL1(string key, out string? value) + { + if (L1Cache.TryGetValue(key, out var entry)) + { + if (IsExpired(entry)) + { + // Remove expired entry from L1 + _ = L1Cache.TryRemove(key, out _); + value = null; + return false; + } + + value = entry.Value; + return true; + } + + value = null; + return false; + } + + /// + /// Populates L1 cache with value and expiration. + /// Clean Code: Single Responsibility - only handles L1 population logic. + /// + private static void PopulateL1(string key, string value, TimeSpan ttl) + { + var expiresAt = DateTimeOffset.UtcNow.Add(ttl); + var entry = new L1CacheEntry(value, expiresAt); + _ = L1Cache.AddOrUpdate(key, entry, (_, _) => entry); + } + + /// + /// Checks if L1 cache entry has expired. + /// Clean Code: Single-purpose helper with intention-revealing name. + /// + private static bool IsExpired(L1CacheEntry entry) => + entry.ExpiresAt <= DateTimeOffset.UtcNow; +} diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Elastic.Documentation.Api.Infrastructure.csproj b/src/api/Elastic.Documentation.Api.Infrastructure/Elastic.Documentation.Api.Infrastructure.csproj index 7724ea935..58e3c4880 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Elastic.Documentation.Api.Infrastructure.csproj +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Elastic.Documentation.Api.Infrastructure.csproj @@ -17,6 +17,7 @@ + diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Gcp/GcpIdTokenProvider.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Gcp/GcpIdTokenProvider.cs index e6df30897..4de088990 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Gcp/GcpIdTokenProvider.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Gcp/GcpIdTokenProvider.cs @@ -2,29 +2,37 @@ // Elasticsearch B.V licenses this file to you under the Apache 2.0 License. // See the LICENSE file in the project root for more information -using System.Collections.Concurrent; using System.Security.Cryptography; using System.Text; using System.Text.Json; using System.Text.Json.Serialization; +using Elastic.Documentation.Api.Infrastructure.Caching; namespace Elastic.Documentation.Api.Infrastructure.Gcp; // This is a custom implementation to create an ID token for GCP. // Because Google.Api.Auth.OAuth2 is not compatible with AOT -public class GcpIdTokenProvider(HttpClient httpClient) : IGcpIdTokenProvider +// Clean Architecture: Depends on IDistributedCache abstraction from Core layer +public class GcpIdTokenProvider(IHttpClientFactory httpClientFactory, IDistributedCache cache) : IGcpIdTokenProvider { - // Cache tokens by target audience to avoid regenerating them on every request - private static readonly ConcurrentDictionary TokenCache = new(); - - private sealed record CachedToken(string Token, DateTimeOffset ExpiresAt); + private readonly IHttpClientFactory _httpClientFactory = httpClientFactory; + private readonly IDistributedCache _cache = cache; public async Task GenerateIdTokenAsync(string serviceAccount, string targetAudience, Cancel cancellationToken = default) { - // Check if we have a valid cached token - if (TokenCache.TryGetValue(targetAudience, out var cachedToken) && - cachedToken.ExpiresAt > DateTimeOffset.UtcNow.AddMinutes(1)) // Refresh 1 minute before expiry - return cachedToken.Token; + // Check distributed cache first (works across all Lambda containers) + var cacheKey = $"idtoken:{targetAudience}"; + var cachedJson = await _cache.GetAsync(cacheKey, cancellationToken); + + if (cachedJson != null) + { + var cachedToken = JsonSerializer.Deserialize(cachedJson, IdTokenCacheJsonContext.Default.CachedIdToken); + + // Check if token is still valid (refresh 1 minute before expiry) + var expiresAt = DateTimeOffset.FromUnixTimeSeconds(cachedToken.ExpiresAtUnix); + if (expiresAt > DateTimeOffset.UtcNow.AddMinutes(1)) + return cachedToken.Token; + } // Read and parse service account key file using System.Text.Json source generation (AOT compatible) var serviceAccountJson = JsonSerializer.Deserialize(serviceAccount, GcpJsonContext.Default.ServiceAccountKey); @@ -72,10 +80,12 @@ public async Task GenerateIdTokenAsync(string serviceAccount, string tar // Exchange JWT for ID token var idToken = await ExchangeJwtForIdToken(jwt, targetAudience, cancellationToken); - var expiresAt = expirationTime.Subtract(TimeSpan.FromMinutes(1)); - _ = TokenCache.AddOrUpdate(targetAudience, - new CachedToken(idToken, expiresAt), - (_, _) => new CachedToken(idToken, expiresAt)); + // Cache the token in distributed cache (shared across all Lambda containers) + // Use 15-minute buffer for maximum safety against clock skew and edge cases + var cacheExpiry = expirationTime.Subtract(TimeSpan.FromMinutes(15)); + var cacheEntry = new CachedIdToken(idToken, cacheExpiry.ToUnixTimeSeconds()); + var cacheJson = JsonSerializer.Serialize(cacheEntry, IdTokenCacheJsonContext.Default.CachedIdToken); + await _cache.SetAsync(cacheKey, cacheJson, TimeSpan.FromHours(1), cancellationToken); return idToken; } @@ -89,6 +99,7 @@ private async Task ExchangeJwtForIdToken(string jwt, string targetAudien new KeyValuePair("target_audience", targetAudience) ]); + var httpClient = _httpClientFactory.CreateClient(); var response = await httpClient.PostAsync("https://oauth2.googleapis.com/token", requestContent, cancellationToken); _ = response.EnsureSuccessStatusCode(); @@ -133,6 +144,12 @@ internal readonly record struct JwtPayload( string TargetAudience ); +/// +/// Cached ID token structure for distributed cache storage. +/// AOT-compatible: Uses source-generated JSON serialization. +/// +internal readonly record struct CachedIdToken(string Token, long ExpiresAtUnix); + [JsonSerializable(typeof(ServiceAccountKey))] [JsonSerializable(typeof(JwtPayload))] [JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.SnakeCaseLower)] @@ -141,3 +158,7 @@ internal sealed partial class GcpJsonContext : JsonSerializerContext; [JsonSerializable(typeof(JwtHeader))] [JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase)] internal sealed partial class JwtHeaderJsonContext : JsonSerializerContext; + +[JsonSerializable(typeof(CachedIdToken))] +[JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase)] +internal sealed partial class IdTokenCacheJsonContext : JsonSerializerContext; diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/OpenTelemetry/OpenTelemetryExtensions.cs b/src/api/Elastic.Documentation.Api.Infrastructure/OpenTelemetry/OpenTelemetryExtensions.cs index d6572a751..26af4ee00 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/OpenTelemetry/OpenTelemetryExtensions.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/OpenTelemetry/OpenTelemetryExtensions.cs @@ -36,6 +36,7 @@ public static TracerProviderBuilder AddDocsApiTracing(this TracerProviderBuilder .AddSource(TelemetryConstants.AskAiSourceName) .AddSource(TelemetryConstants.StreamTransformerSourceName) .AddSource(TelemetryConstants.OtlpProxySourceName) + .AddSource(TelemetryConstants.CacheSourceName) .AddAspNetCoreInstrumentation(aspNetCoreOptions => { // Don't trace root API endpoint (health check) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/ServicesExtension.cs b/src/api/Elastic.Documentation.Api.Infrastructure/ServicesExtension.cs index dea5311e0..88ebaba3d 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/ServicesExtension.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/ServicesExtension.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information using System.ComponentModel.DataAnnotations; +using Amazon.DynamoDBv2; using Elastic.Documentation.Api.Core; using Elastic.Documentation.Api.Core.AskAi; using Elastic.Documentation.Api.Core.Search; @@ -11,6 +12,7 @@ using Elastic.Documentation.Api.Infrastructure.Adapters.Search; using Elastic.Documentation.Api.Infrastructure.Adapters.Telemetry; using Elastic.Documentation.Api.Infrastructure.Aws; +using Elastic.Documentation.Api.Infrastructure.Caching; using Elastic.Documentation.Api.Infrastructure.Gcp; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; @@ -71,6 +73,7 @@ private static void AddElasticDocsApiUsecases(this IServiceCollection services, }); // Register AppEnvironment as a singleton for dependency injection _ = services.AddSingleton(new AppEnvironment { Current = appEnv }); + AddDistributedCache(services, appEnv); AddParameterProvider(services, appEnv); AddAskAiUsecase(services, appEnv); AddSearchUsecase(services, appEnv); @@ -121,6 +124,58 @@ private static void AddParameterProvider(IServiceCollection services, AppEnv app } } + private static void AddDistributedCache(IServiceCollection services, AppEnv appEnv) + { + var logger = GetLogger(services); + + switch (appEnv) + { + case AppEnv.Dev: + { + logger?.LogInformation("Configuring InMemoryDistributedCache for environment {AppEnvironment}", appEnv); + _ = services.AddSingleton(); + logger?.LogInformation("InMemoryDistributedCache registered for local development"); + break; + } + case AppEnv.Prod: + case AppEnv.Staging: + case AppEnv.Edge: + { + logger?.LogInformation("Configuring DynamoDB distributed cache for environment {AppEnvironment}", appEnv); + try + { + // Register AWS DynamoDB client + _ = services.AddSingleton(); + logger?.LogInformation("AmazonDynamoDB client registered"); + + // Register multi-layer cache (L1: in-memory + L2: DynamoDB) + _ = services.AddSingleton(sp => + { + var dynamoDb = sp.GetRequiredService(); + var tableName = $"docs-api-cache-{appEnv.ToStringFast()}"; + var dynamoLogger = sp.GetRequiredService>(); + var multiLogger = sp.GetRequiredService>(); + + var dynamoCache = new DynamoDbDistributedCache(dynamoDb, tableName, dynamoLogger); + var multiLayerCache = new MultiLayerCache(dynamoCache, multiLogger); + logger?.LogInformation("Multi-layer cache registered with DynamoDB table: {TableName}", tableName); + return multiLayerCache; + }); + } + catch (Exception ex) + { + logger?.LogError(ex, "Failed to configure distributed cache for environment {AppEnvironment}", appEnv); + throw; + } + break; + } + default: + { + throw new ArgumentOutOfRangeException(nameof(appEnv), appEnv, "Unsupported environment for distributed cache."); + } + } + } + private static void AddAskAiUsecase(IServiceCollection services, AppEnv appEnv) { var logger = GetLogger(services); @@ -128,8 +183,10 @@ private static void AddAskAiUsecase(IServiceCollection services, AppEnv appEnv) try { + // Register GcpIdTokenProvider with distributed cache dependency + // Clean: Let DI handle everything automatically _ = services.AddSingleton(); - logger?.LogInformation("GcpIdTokenProvider registered successfully"); + logger?.LogInformation("GcpIdTokenProvider registered with distributed cache support"); _ = services.AddScoped(); logger?.LogInformation("LlmGatewayOptions registered successfully"); From e8de291c93e11a992c802ac42d909e5b28aeb214 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Thu, 27 Nov 2025 11:34:51 +0100 Subject: [PATCH 02/14] Add tests --- .../Caching/DistributedCacheTests.cs | 333 ++++++++++++++++++ ...umentation.Api.Infrastructure.Tests.csproj | 1 + 2 files changed, 334 insertions(+) create mode 100644 tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs new file mode 100644 index 000000000..07dd79521 --- /dev/null +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs @@ -0,0 +1,333 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Text.Json; +using Amazon.DynamoDBv2; +using Amazon.DynamoDBv2.Model; +using Elastic.Documentation.Api.Infrastructure.Caching; +using Elastic.Documentation.Api.Infrastructure.Gcp; +using FakeItEasy; +using FluentAssertions; +using Microsoft.Extensions.Logging.Abstractions; + +namespace Elastic.Documentation.Api.Infrastructure.Tests.Caching; + +public class InMemoryDistributedCacheTests +{ + private readonly InMemoryDistributedCache _cache = new(); + + [Fact] + public async Task GetAsync_WhenKeyDoesNotExist_ReturnsNull() + { + // Act + var result = await _cache.GetAsync("nonexistent-key"); + + // Assert + result.Should().BeNull(); + } + + [Fact] + public async Task SetAndGet_WhenKeyIsSet_ReturnsValue() + { + // Arrange + const string key = "test-key"; + const string value = "test-value"; + + // Act + await _cache.SetAsync(key, value, TimeSpan.FromMinutes(1)); + var result = await _cache.GetAsync(key); + + // Assert + result.Should().Be(value); + } + + [Fact] + public async Task GetAsync_WhenEntryExpired_ReturnsNull() + { + // Arrange + const string key = "expiring-key"; + + // Act + await _cache.SetAsync(key, "value", TimeSpan.FromMilliseconds(10)); + await Task.Delay(50); + var result = await _cache.GetAsync(key); + + // Assert + result.Should().BeNull("expired entries should be removed"); + } + + [Fact] + public async Task SetAsync_OverwritesExistingValue() + { + // Arrange + const string key = "key"; + + // Act + await _cache.SetAsync(key, "first", TimeSpan.FromMinutes(1)); + await _cache.SetAsync(key, "second", TimeSpan.FromMinutes(1)); + var result = await _cache.GetAsync(key); + + // Assert + result.Should().Be("second"); + } +} + +public class MultiLayerCacheTests +{ + [Fact] + public async Task GetAsync_WhenL1Hit_DoesNotCallL2Again() + { + // Arrange + var fakeL2 = A.Fake(); + var cache = new MultiLayerCache(fakeL2, NullLogger.Instance); + + // Pre-populate L1 by setting a value + await cache.SetAsync("key", "value", TimeSpan.FromMinutes(1)); + + // Act - Second get should hit L1 + var result1 = await cache.GetAsync("key"); + var result2 = await cache.GetAsync("key"); + + // Assert + result1.Should().Be("value"); + result2.Should().Be("value"); + // L2 should only be called once (during SetAsync), not on subsequent Gets + A.CallTo(() => fakeL2.GetAsync(A._, A._)) + .MustNotHaveHappened(); + } + + [Fact] + public async Task GetAsync_WhenL1Miss_CallsL2AndPopulatesL1() + { + // Arrange + var fakeL2 = A.Fake(); + A.CallTo(() => fakeL2.GetAsync("key", A._)) + .Returns("l2-value"); + + var cache = new MultiLayerCache(fakeL2, NullLogger.Instance); + + // Act - First call misses L1, hits L2 + var result1 = await cache.GetAsync("key"); + // Second call should hit L1 (populated from previous call) + var result2 = await cache.GetAsync("key"); + + // Assert + result1.Should().Be("l2-value"); + result2.Should().Be("l2-value"); + A.CallTo(() => fakeL2.GetAsync("key", A._)) + .MustHaveHappenedOnceExactly(); + } + + [Fact] + public async Task SetAsync_WritesToBothL1AndL2() + { + // Arrange + var fakeL2 = A.Fake(); + var cache = new MultiLayerCache(fakeL2, NullLogger.Instance); + + // Act + await cache.SetAsync("key", "value", TimeSpan.FromMinutes(1)); + + // Get from cache (should hit L1) + var result = await cache.GetAsync("key"); + + // Assert + result.Should().Be("value", "L1 should have the value"); + A.CallTo(() => fakeL2.SetAsync("key", "value", TimeSpan.FromMinutes(1), A._)) + .MustHaveHappenedOnceExactly(); + } + + [Fact] + public async Task GetAsync_WhenBothCachesMiss_ReturnsNull() + { + // Arrange + var fakeL2 = A.Fake(); + A.CallTo(() => fakeL2.GetAsync(A._, A._)) + .Returns((string?)null); + + var cache = new MultiLayerCache(fakeL2, NullLogger.Instance); + + // Act + var result = await cache.GetAsync("missing-key"); + + // Assert + result.Should().BeNull(); + } +} + +public class DynamoDbDistributedCacheTests +{ + [Fact] + public async Task GetAsync_WhenItemExists_ReturnsValue() + { + // Arrange + var fakeDynamoDb = A.Fake(); + var expiresAt = DateTimeOffset.UtcNow.AddMinutes(30).ToUnixTimeSeconds(); + + var response = new GetItemResponse + { + Item = new Dictionary + { + ["CacheKey"] = new AttributeValue { S = "test-key" }, + ["Value"] = new AttributeValue { S = "test-value" }, + ["ExpiresAt"] = new AttributeValue { N = expiresAt.ToString() } + }, + IsItemSet = true + }; + + A.CallTo(() => fakeDynamoDb.GetItemAsync(A._, A._)) + .Returns(response); + + var cache = new DynamoDbDistributedCache(fakeDynamoDb, "test-table", NullLogger.Instance); + + // Act + var result = await cache.GetAsync("test-key"); + + // Assert + result.Should().Be("test-value"); + } + + [Fact] + public async Task GetAsync_WhenItemExpired_ReturnsNull() + { + // Arrange + var fakeDynamoDb = A.Fake(); + var expiresAt = DateTimeOffset.UtcNow.AddMinutes(-5).ToUnixTimeSeconds(); // Expired 5 min ago + + var response = new GetItemResponse + { + Item = new Dictionary + { + ["CacheKey"] = new AttributeValue { S = "test-key" }, + ["Value"] = new AttributeValue { S = "test-value" }, + ["ExpiresAt"] = new AttributeValue { N = expiresAt.ToString() } + }, + IsItemSet = true + }; + + A.CallTo(() => fakeDynamoDb.GetItemAsync(A._, A._)) + .Returns(response); + + var cache = new DynamoDbDistributedCache(fakeDynamoDb, "test-table", NullLogger.Instance); + + // Act + var result = await cache.GetAsync("test-key"); + + // Assert + result.Should().BeNull("expired items should not be returned"); + } + + [Fact] + public async Task GetAsync_WhenItemDoesNotExist_ReturnsNull() + { + // Arrange + var fakeDynamoDb = A.Fake(); + var response = new GetItemResponse { IsItemSet = false }; + + A.CallTo(() => fakeDynamoDb.GetItemAsync(A._, A._)) + .Returns(response); + + var cache = new DynamoDbDistributedCache(fakeDynamoDb, "test-table", NullLogger.Instance); + + // Act + var result = await cache.GetAsync("missing-key"); + + // Assert + result.Should().BeNull(); + } + + [Fact] + public async Task SetAsync_CallsDynamoDbPutItem() + { + // Arrange + var fakeDynamoDb = A.Fake(); + var cache = new DynamoDbDistributedCache(fakeDynamoDb, "test-table", NullLogger.Instance); + + // Act + await cache.SetAsync("key", "value", TimeSpan.FromMinutes(30)); + + // Assert + A.CallTo(() => fakeDynamoDb.PutItemAsync( + A.That.Matches(r => + r.TableName == "test-table" && + r.Item["CacheKey"].S == "key" && + r.Item["Value"].S == "value" + ), + A._ + )).MustHaveHappenedOnceExactly(); + } + + [Fact] + public async Task GetAsync_WhenTableNotFound_ReturnsNullGracefully() + { + // Arrange + var fakeDynamoDb = A.Fake(); + var exception = new ResourceNotFoundException("Table not found"); + A.CallTo(() => fakeDynamoDb.GetItemAsync(A._, A._)) + .Throws(exception); + + var cache = new DynamoDbDistributedCache(fakeDynamoDb, "missing-table", NullLogger.Instance); + + // Act + var result = await cache.GetAsync("key"); + + // Assert + result.Should().BeNull("should handle missing table gracefully"); + } +} + +public class GcpIdTokenProviderCachingIntegrationTests +{ + [Fact] + public async Task GenerateIdTokenAsync_UsesCachedToken_WhenValid() + { + // Arrange + var fakeHttpClientFactory = A.Fake(); + var cache = new InMemoryDistributedCache(); + + var provider = new GcpIdTokenProvider(fakeHttpClientFactory, cache); + const string targetAudience = "https://test-audience.googleapis.com"; + + // Pre-populate cache with valid token (expires in 50 minutes) + var cachedToken = new + { + token = "fake-cached-token", + expiresAtUnix = DateTimeOffset.UtcNow.AddMinutes(50).ToUnixTimeSeconds() + }; + var cacheJson = JsonSerializer.Serialize(cachedToken); + await cache.SetAsync($"idtoken:{targetAudience}", cacheJson, TimeSpan.FromHours(1)); + + // Act + var result = await provider.GenerateIdTokenAsync("{}", targetAudience); + + // Assert + result.Should().Be("fake-cached-token", "should return cached token without calling Google OAuth"); + A.CallTo(() => fakeHttpClientFactory.CreateClient()).MustNotHaveHappened(); + } + + [Fact] + public async Task GenerateIdTokenAsync_IgnoresExpiredCachedToken() + { + // Arrange + var cache = new InMemoryDistributedCache(); + + // Pre-populate cache with expired token (already past 45-minute threshold) + var expiredToken = new + { + token = "expired-token", + expiresAtUnix = DateTimeOffset.UtcNow.AddSeconds(30).ToUnixTimeSeconds() // Only 30s left + }; + var cacheJson = JsonSerializer.Serialize(expiredToken); + await cache.SetAsync("idtoken:https://test.com", cacheJson, TimeSpan.FromHours(1)); + + // Act - Try to get the expired token + var cachedValue = await cache.GetAsync("idtoken:https://test.com"); + var parsedToken = JsonSerializer.Deserialize(cachedValue!); + var expiresAt = DateTimeOffset.FromUnixTimeSeconds(parsedToken.GetProperty("expiresAtUnix").GetInt64()); + + // Assert - Token should be considered expired (less than 1 minute buffer) + (expiresAt <= DateTimeOffset.UtcNow.AddMinutes(1)).Should().BeTrue( + "tokens with less than 1 minute remaining should be refreshed"); + } +} diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Elastic.Documentation.Api.Infrastructure.Tests.csproj b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Elastic.Documentation.Api.Infrastructure.Tests.csproj index 6513b2c7d..3e8bc29f7 100644 --- a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Elastic.Documentation.Api.Infrastructure.Tests.csproj +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Elastic.Documentation.Api.Infrastructure.Tests.csproj @@ -10,6 +10,7 @@ + From d9e79bdf2a1438f8977d5cac34376b237ad15dc1 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Thu, 27 Nov 2025 11:37:37 +0100 Subject: [PATCH 03/14] Formatting --- .../Caching/DistributedCacheTests.cs | 82 +++++++++---------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs index 07dd79521..fd4fe6db1 100644 --- a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs @@ -18,55 +18,55 @@ public class InMemoryDistributedCacheTests private readonly InMemoryDistributedCache _cache = new(); [Fact] - public async Task GetAsync_WhenKeyDoesNotExist_ReturnsNull() + public async Task GetAsyncWhenKeyDoesNotExistReturnsNull() { // Act - var result = await _cache.GetAsync("nonexistent-key"); + var result = await _cache.GetAsync("nonexistent-key", TestContext.Current.CancellationToken); // Assert result.Should().BeNull(); } [Fact] - public async Task SetAndGet_WhenKeyIsSet_ReturnsValue() + public async Task SetAndGetWhenKeyIsSetReturnsValue() { // Arrange const string key = "test-key"; const string value = "test-value"; // Act - await _cache.SetAsync(key, value, TimeSpan.FromMinutes(1)); - var result = await _cache.GetAsync(key); + await _cache.SetAsync(key, value, TimeSpan.FromMinutes(1), TestContext.Current.CancellationToken); + var result = await _cache.GetAsync(key, TestContext.Current.CancellationToken); // Assert result.Should().Be(value); } [Fact] - public async Task GetAsync_WhenEntryExpired_ReturnsNull() + public async Task GetAsyncWhenEntryExpiredReturnsNull() { // Arrange const string key = "expiring-key"; // Act - await _cache.SetAsync(key, "value", TimeSpan.FromMilliseconds(10)); - await Task.Delay(50); - var result = await _cache.GetAsync(key); + await _cache.SetAsync(key, "value", TimeSpan.FromMilliseconds(10), TestContext.Current.CancellationToken); + await Task.Delay(50, TestContext.Current.CancellationToken); + var result = await _cache.GetAsync(key, TestContext.Current.CancellationToken); // Assert result.Should().BeNull("expired entries should be removed"); } [Fact] - public async Task SetAsync_OverwritesExistingValue() + public async Task SetAsyncOverwritesExistingValue() { // Arrange const string key = "key"; // Act - await _cache.SetAsync(key, "first", TimeSpan.FromMinutes(1)); - await _cache.SetAsync(key, "second", TimeSpan.FromMinutes(1)); - var result = await _cache.GetAsync(key); + await _cache.SetAsync(key, "first", TimeSpan.FromMinutes(1), TestContext.Current.CancellationToken); + await _cache.SetAsync(key, "second", TimeSpan.FromMinutes(1), TestContext.Current.CancellationToken); + var result = await _cache.GetAsync(key, TestContext.Current.CancellationToken); // Assert result.Should().Be("second"); @@ -76,18 +76,18 @@ public async Task SetAsync_OverwritesExistingValue() public class MultiLayerCacheTests { [Fact] - public async Task GetAsync_WhenL1Hit_DoesNotCallL2Again() + public async Task GetAsyncWhenL1HitDoesNotCallL2Again() { // Arrange var fakeL2 = A.Fake(); var cache = new MultiLayerCache(fakeL2, NullLogger.Instance); // Pre-populate L1 by setting a value - await cache.SetAsync("key", "value", TimeSpan.FromMinutes(1)); + await cache.SetAsync("key", "value", TimeSpan.FromMinutes(1), TestContext.Current.CancellationToken); // Act - Second get should hit L1 - var result1 = await cache.GetAsync("key"); - var result2 = await cache.GetAsync("key"); + var result1 = await cache.GetAsync("key", TestContext.Current.CancellationToken); + var result2 = await cache.GetAsync("key", TestContext.Current.CancellationToken); // Assert result1.Should().Be("value"); @@ -98,7 +98,7 @@ public async Task GetAsync_WhenL1Hit_DoesNotCallL2Again() } [Fact] - public async Task GetAsync_WhenL1Miss_CallsL2AndPopulatesL1() + public async Task GetAsyncWhenL1MissCallsL2AndPopulatesL1() { // Arrange var fakeL2 = A.Fake(); @@ -108,9 +108,9 @@ public async Task GetAsync_WhenL1Miss_CallsL2AndPopulatesL1() var cache = new MultiLayerCache(fakeL2, NullLogger.Instance); // Act - First call misses L1, hits L2 - var result1 = await cache.GetAsync("key"); + var result1 = await cache.GetAsync("key", TestContext.Current.CancellationToken); // Second call should hit L1 (populated from previous call) - var result2 = await cache.GetAsync("key"); + var result2 = await cache.GetAsync("key", TestContext.Current.CancellationToken); // Assert result1.Should().Be("l2-value"); @@ -120,17 +120,17 @@ public async Task GetAsync_WhenL1Miss_CallsL2AndPopulatesL1() } [Fact] - public async Task SetAsync_WritesToBothL1AndL2() + public async Task SetAsyncWritesToBothL1AndL2() { // Arrange var fakeL2 = A.Fake(); var cache = new MultiLayerCache(fakeL2, NullLogger.Instance); // Act - await cache.SetAsync("key", "value", TimeSpan.FromMinutes(1)); + await cache.SetAsync("key", "value", TimeSpan.FromMinutes(1), TestContext.Current.CancellationToken); // Get from cache (should hit L1) - var result = await cache.GetAsync("key"); + var result = await cache.GetAsync("key", TestContext.Current.CancellationToken); // Assert result.Should().Be("value", "L1 should have the value"); @@ -139,7 +139,7 @@ public async Task SetAsync_WritesToBothL1AndL2() } [Fact] - public async Task GetAsync_WhenBothCachesMiss_ReturnsNull() + public async Task GetAsyncWhenBothCachesMissReturnsNull() { // Arrange var fakeL2 = A.Fake(); @@ -149,7 +149,7 @@ public async Task GetAsync_WhenBothCachesMiss_ReturnsNull() var cache = new MultiLayerCache(fakeL2, NullLogger.Instance); // Act - var result = await cache.GetAsync("missing-key"); + var result = await cache.GetAsync("missing-key", TestContext.Current.CancellationToken); // Assert result.Should().BeNull(); @@ -159,7 +159,7 @@ public async Task GetAsync_WhenBothCachesMiss_ReturnsNull() public class DynamoDbDistributedCacheTests { [Fact] - public async Task GetAsync_WhenItemExists_ReturnsValue() + public async Task GetAsyncWhenItemExistsReturnsValue() { // Arrange var fakeDynamoDb = A.Fake(); @@ -182,14 +182,14 @@ public async Task GetAsync_WhenItemExists_ReturnsValue() var cache = new DynamoDbDistributedCache(fakeDynamoDb, "test-table", NullLogger.Instance); // Act - var result = await cache.GetAsync("test-key"); + var result = await cache.GetAsync("test-key", TestContext.Current.CancellationToken); // Assert result.Should().Be("test-value"); } [Fact] - public async Task GetAsync_WhenItemExpired_ReturnsNull() + public async Task GetAsyncWhenItemExpiredReturnsNull() { // Arrange var fakeDynamoDb = A.Fake(); @@ -212,14 +212,14 @@ public async Task GetAsync_WhenItemExpired_ReturnsNull() var cache = new DynamoDbDistributedCache(fakeDynamoDb, "test-table", NullLogger.Instance); // Act - var result = await cache.GetAsync("test-key"); + var result = await cache.GetAsync("test-key", TestContext.Current.CancellationToken); // Assert result.Should().BeNull("expired items should not be returned"); } [Fact] - public async Task GetAsync_WhenItemDoesNotExist_ReturnsNull() + public async Task GetAsyncWhenItemDoesNotExistReturnsNull() { // Arrange var fakeDynamoDb = A.Fake(); @@ -231,21 +231,21 @@ public async Task GetAsync_WhenItemDoesNotExist_ReturnsNull() var cache = new DynamoDbDistributedCache(fakeDynamoDb, "test-table", NullLogger.Instance); // Act - var result = await cache.GetAsync("missing-key"); + var result = await cache.GetAsync("missing-key", TestContext.Current.CancellationToken); // Assert result.Should().BeNull(); } [Fact] - public async Task SetAsync_CallsDynamoDbPutItem() + public async Task SetAsyncCallsDynamoDbPutItem() { // Arrange var fakeDynamoDb = A.Fake(); var cache = new DynamoDbDistributedCache(fakeDynamoDb, "test-table", NullLogger.Instance); // Act - await cache.SetAsync("key", "value", TimeSpan.FromMinutes(30)); + await cache.SetAsync("key", "value", TimeSpan.FromMinutes(30), TestContext.Current.CancellationToken); // Assert A.CallTo(() => fakeDynamoDb.PutItemAsync( @@ -259,7 +259,7 @@ public async Task SetAsync_CallsDynamoDbPutItem() } [Fact] - public async Task GetAsync_WhenTableNotFound_ReturnsNullGracefully() + public async Task GetAsyncWhenTableNotFoundReturnsNullGracefully() { // Arrange var fakeDynamoDb = A.Fake(); @@ -270,7 +270,7 @@ public async Task GetAsync_WhenTableNotFound_ReturnsNullGracefully() var cache = new DynamoDbDistributedCache(fakeDynamoDb, "missing-table", NullLogger.Instance); // Act - var result = await cache.GetAsync("key"); + var result = await cache.GetAsync("key", TestContext.Current.CancellationToken); // Assert result.Should().BeNull("should handle missing table gracefully"); @@ -280,7 +280,7 @@ public async Task GetAsync_WhenTableNotFound_ReturnsNullGracefully() public class GcpIdTokenProviderCachingIntegrationTests { [Fact] - public async Task GenerateIdTokenAsync_UsesCachedToken_WhenValid() + public async Task GenerateIdTokenAsyncUsesCachedTokenWhenValid() { // Arrange var fakeHttpClientFactory = A.Fake(); @@ -296,10 +296,10 @@ public async Task GenerateIdTokenAsync_UsesCachedToken_WhenValid() expiresAtUnix = DateTimeOffset.UtcNow.AddMinutes(50).ToUnixTimeSeconds() }; var cacheJson = JsonSerializer.Serialize(cachedToken); - await cache.SetAsync($"idtoken:{targetAudience}", cacheJson, TimeSpan.FromHours(1)); + await cache.SetAsync($"idtoken:{targetAudience}", cacheJson, TimeSpan.FromHours(1), TestContext.Current.CancellationToken); // Act - var result = await provider.GenerateIdTokenAsync("{}", targetAudience); + var result = await provider.GenerateIdTokenAsync("{}", targetAudience, TestContext.Current.CancellationToken); // Assert result.Should().Be("fake-cached-token", "should return cached token without calling Google OAuth"); @@ -307,7 +307,7 @@ public async Task GenerateIdTokenAsync_UsesCachedToken_WhenValid() } [Fact] - public async Task GenerateIdTokenAsync_IgnoresExpiredCachedToken() + public async Task GenerateIdTokenAsyncIgnoresExpiredCachedToken() { // Arrange var cache = new InMemoryDistributedCache(); @@ -319,10 +319,10 @@ public async Task GenerateIdTokenAsync_IgnoresExpiredCachedToken() expiresAtUnix = DateTimeOffset.UtcNow.AddSeconds(30).ToUnixTimeSeconds() // Only 30s left }; var cacheJson = JsonSerializer.Serialize(expiredToken); - await cache.SetAsync("idtoken:https://test.com", cacheJson, TimeSpan.FromHours(1)); + await cache.SetAsync("idtoken:https://test.com", cacheJson, TimeSpan.FromHours(1), TestContext.Current.CancellationToken); // Act - Try to get the expired token - var cachedValue = await cache.GetAsync("idtoken:https://test.com"); + var cachedValue = await cache.GetAsync("idtoken:https://test.com", TestContext.Current.CancellationToken); var parsedToken = JsonSerializer.Deserialize(cachedValue!); var expiresAt = DateTimeOffset.FromUnixTimeSeconds(parsedToken.GetProperty("expiresAtUnix").GetInt64()); From 67d000415bb3a8b1d14ec7b4ffc2e97529e2ae4c Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Thu, 27 Nov 2025 11:47:11 +0100 Subject: [PATCH 04/14] Fix tests --- .../Caching/DistributedCacheTests.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs index fd4fe6db1..6ce39a27f 100644 --- a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs @@ -2,6 +2,7 @@ // Elasticsearch B.V licenses this file to you under the Apache 2.0 License. // See the LICENSE file in the project root for more information +using System.Globalization; using System.Text.Json; using Amazon.DynamoDBv2; using Amazon.DynamoDBv2.Model; @@ -171,7 +172,7 @@ public async Task GetAsyncWhenItemExistsReturnsValue() { ["CacheKey"] = new AttributeValue { S = "test-key" }, ["Value"] = new AttributeValue { S = "test-value" }, - ["ExpiresAt"] = new AttributeValue { N = expiresAt.ToString() } + ["ExpiresAt"] = new AttributeValue { N = expiresAt.ToString(CultureInfo.InvariantCulture) } }, IsItemSet = true }; @@ -201,7 +202,7 @@ public async Task GetAsyncWhenItemExpiredReturnsNull() { ["CacheKey"] = new AttributeValue { S = "test-key" }, ["Value"] = new AttributeValue { S = "test-value" }, - ["ExpiresAt"] = new AttributeValue { N = expiresAt.ToString() } + ["ExpiresAt"] = new AttributeValue { N = expiresAt.ToString(CultureInfo.InvariantCulture) } }, IsItemSet = true }; From 4f5d67c3eb49135e9eabd676d0c858f96eb85552 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Thu, 27 Nov 2025 12:06:42 +0100 Subject: [PATCH 05/14] Fix tests --- .../Caching/DistributedCacheTests.cs | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs index 6ce39a27f..88feca13f 100644 --- a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs @@ -82,13 +82,14 @@ public async Task GetAsyncWhenL1HitDoesNotCallL2Again() // Arrange var fakeL2 = A.Fake(); var cache = new MultiLayerCache(fakeL2, NullLogger.Instance); + var uniqueKey = $"test-key-{Guid.NewGuid()}"; // Use unique key to avoid L1 cache pollution from other tests // Pre-populate L1 by setting a value - await cache.SetAsync("key", "value", TimeSpan.FromMinutes(1), TestContext.Current.CancellationToken); + await cache.SetAsync(uniqueKey, "value", TimeSpan.FromMinutes(1), TestContext.Current.CancellationToken); // Act - Second get should hit L1 - var result1 = await cache.GetAsync("key", TestContext.Current.CancellationToken); - var result2 = await cache.GetAsync("key", TestContext.Current.CancellationToken); + var result1 = await cache.GetAsync(uniqueKey, TestContext.Current.CancellationToken); + var result2 = await cache.GetAsync(uniqueKey, TestContext.Current.CancellationToken); // Assert result1.Should().Be("value"); @@ -103,20 +104,21 @@ public async Task GetAsyncWhenL1MissCallsL2AndPopulatesL1() { // Arrange var fakeL2 = A.Fake(); - A.CallTo(() => fakeL2.GetAsync("key", A._)) + var uniqueKey = $"test-key-{Guid.NewGuid()}"; // Use unique key to avoid L1 cache pollution from other tests + A.CallTo(() => fakeL2.GetAsync(uniqueKey, A._)) .Returns("l2-value"); var cache = new MultiLayerCache(fakeL2, NullLogger.Instance); // Act - First call misses L1, hits L2 - var result1 = await cache.GetAsync("key", TestContext.Current.CancellationToken); + var result1 = await cache.GetAsync(uniqueKey, TestContext.Current.CancellationToken); // Second call should hit L1 (populated from previous call) - var result2 = await cache.GetAsync("key", TestContext.Current.CancellationToken); + var result2 = await cache.GetAsync(uniqueKey, TestContext.Current.CancellationToken); // Assert result1.Should().Be("l2-value"); result2.Should().Be("l2-value"); - A.CallTo(() => fakeL2.GetAsync("key", A._)) + A.CallTo(() => fakeL2.GetAsync(uniqueKey, A._)) .MustHaveHappenedOnceExactly(); } @@ -126,16 +128,17 @@ public async Task SetAsyncWritesToBothL1AndL2() // Arrange var fakeL2 = A.Fake(); var cache = new MultiLayerCache(fakeL2, NullLogger.Instance); + var uniqueKey = $"test-key-{Guid.NewGuid()}"; // Use unique key to avoid L1 cache pollution from other tests // Act - await cache.SetAsync("key", "value", TimeSpan.FromMinutes(1), TestContext.Current.CancellationToken); + await cache.SetAsync(uniqueKey, "value", TimeSpan.FromMinutes(1), TestContext.Current.CancellationToken); // Get from cache (should hit L1) - var result = await cache.GetAsync("key", TestContext.Current.CancellationToken); + var result = await cache.GetAsync(uniqueKey, TestContext.Current.CancellationToken); // Assert result.Should().Be("value", "L1 should have the value"); - A.CallTo(() => fakeL2.SetAsync("key", "value", TimeSpan.FromMinutes(1), A._)) + A.CallTo(() => fakeL2.SetAsync(uniqueKey, "value", TimeSpan.FromMinutes(1), A._)) .MustHaveHappenedOnceExactly(); } @@ -304,7 +307,8 @@ public async Task GenerateIdTokenAsyncUsesCachedTokenWhenValid() // Assert result.Should().Be("fake-cached-token", "should return cached token without calling Google OAuth"); - A.CallTo(() => fakeHttpClientFactory.CreateClient()).MustNotHaveHappened(); + // Note: Can't verify CreateClient() wasn't called because it's an extension method (not interceptable by FakeItEasy) + // But the test still validates that cached token is returned correctly } [Fact] From 98e5fbde8233fd223b4a755ca69363ed46f6ef35 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Thu, 27 Nov 2025 12:59:01 +0100 Subject: [PATCH 06/14] Use hashed cache keys --- .../Caching/CacheKey.cs | 45 ++++++++++++++++ .../Caching/DynamoDbDistributedCache.cs | 28 +++++----- .../Caching/IDistributedCache.cs | 17 ++++-- .../Caching/InMemoryDistributedCache.cs | 12 +++-- .../Caching/MultiLayerCache.cs | 24 +++++---- .../Gcp/GcpIdTokenProvider.cs | 6 ++- .../Caching/DistributedCacheTests.cs | 53 +++++++++++-------- 7 files changed, 129 insertions(+), 56 deletions(-) create mode 100644 src/api/Elastic.Documentation.Api.Infrastructure/Caching/CacheKey.cs diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/CacheKey.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/CacheKey.cs new file mode 100644 index 000000000..67e565463 --- /dev/null +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/CacheKey.cs @@ -0,0 +1,45 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Security.Cryptography; +using System.Text; + +namespace Elastic.Documentation.Api.Infrastructure.Caching; + +/// +/// Represents a cache key with automatic hashing of sensitive identifiers. +/// Prevents exposing sensitive data in cache keys (CodeQL security requirement). +/// +public sealed class CacheKey +{ + /// + /// Gets the hashed key string for use in cache operations. + /// + public string Value { get; } + + private CacheKey(string category, string identifier) + { + // Hash the identifier to prevent exposing sensitive data (CodeQL security requirement) + var bytes = Encoding.UTF8.GetBytes(identifier); + var hash = SHA256.HashData(bytes); + var hashBase64 = Convert.ToBase64String(hash); + // Use base64url encoding for cache key (URL-safe) + var hashBase64Url = hashBase64.Replace('+', '-').Replace('/', '_').TrimEnd('='); + Value = $"{category}:{hashBase64Url}"; + } + + /// + /// Creates a cache key from a category and identifier. + /// The identifier is automatically hashed to prevent exposing sensitive data. + /// + /// Cache category (e.g., "idtoken", "search") + /// Identifier that may contain sensitive data (will be hashed) + /// A CacheKey instance with the hashed key + public static CacheKey Create(string category, string identifier) => new(category, identifier); + + /// + /// Implicit conversion to string for convenience. + /// + public static implicit operator string(CacheKey key) => key.Value; +} diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs index 923727224..679aa9a9f 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs @@ -30,10 +30,11 @@ public sealed class DynamoDbDistributedCache(IAmazonDynamoDB dynamoDb, string ta private const string AttributeExpiresAt = "ExpiresAt"; private const string AttributeTtl = "TTL"; - public async Task GetAsync(string key, Cancel ct = default) + public async Task GetAsync(CacheKey key, Cancel ct = default) { + var hashedKey = key.Value; using var activity = ActivitySource.StartActivity("get cache", ActivityKind.Client); - _ = (activity?.SetTag("cache.key", key)); + _ = (activity?.SetTag("cache.key", hashedKey)); _ = (activity?.SetTag("cache.table", _tableName)); _ = (activity?.SetTag("cache.backend", "dynamodb")); @@ -44,14 +45,14 @@ public sealed class DynamoDbDistributedCache(IAmazonDynamoDB dynamoDb, string ta TableName = _tableName, Key = new Dictionary { - [AttributeCacheKey] = new AttributeValue { S = key } + [AttributeCacheKey] = new AttributeValue { S = hashedKey } } }, ct); if (!response.IsItemSet) { _ = (activity?.SetTag("cache.hit", false)); - _logger.LogDebug("Cache miss for key: {CacheKey}", key); + _logger.LogDebug("Cache miss for key: {CacheKey}", hashedKey); return null; } @@ -60,7 +61,7 @@ public sealed class DynamoDbDistributedCache(IAmazonDynamoDB dynamoDb, string ta { _ = (activity?.SetTag("cache.hit", false)); _ = (activity?.SetTag("cache.expired", true)); - _logger.LogDebug("Cache expired for key: {CacheKey}", key); + _logger.LogDebug("Cache expired for key: {CacheKey}", hashedKey); return null; } @@ -71,7 +72,7 @@ public sealed class DynamoDbDistributedCache(IAmazonDynamoDB dynamoDb, string ta _ = (activity?.SetTag("cache.hit", value != null)); if (value != null) { - _logger.LogDebug("Cache hit for key: {CacheKey}", key); + _logger.LogDebug("Cache hit for key: {CacheKey}", hashedKey); } return value; @@ -87,15 +88,16 @@ public sealed class DynamoDbDistributedCache(IAmazonDynamoDB dynamoDb, string ta catch (Exception ex) { _ = (activity?.SetStatus(ActivityStatusCode.Error, ex.Message)); - _logger.LogError(ex, "Error retrieving cache key {CacheKey} from DynamoDB", key); + _logger.LogError(ex, "Error retrieving cache key {CacheKey} from DynamoDB", hashedKey); return null; // Fail gracefully } } - public async Task SetAsync(string key, string value, TimeSpan ttl, Cancel ct = default) + public async Task SetAsync(CacheKey key, string value, TimeSpan ttl, Cancel ct = default) { + var hashedKey = key.Value; using var activity = ActivitySource.StartActivity("set cache", ActivityKind.Client); - _ = (activity?.SetTag("cache.key", key)); + _ = (activity?.SetTag("cache.key", hashedKey)); _ = (activity?.SetTag("cache.table", _tableName)); _ = (activity?.SetTag("cache.backend", "dynamodb")); _ = (activity?.SetTag("cache.ttl", ttl.TotalSeconds)); @@ -110,26 +112,26 @@ public async Task SetAsync(string key, string value, TimeSpan ttl, Cancel ct = d TableName = _tableName, Item = new Dictionary { - [AttributeCacheKey] = new AttributeValue { S = key }, + [AttributeCacheKey] = new AttributeValue { S = hashedKey }, [AttributeValue] = new AttributeValue { S = value }, [AttributeExpiresAt] = new AttributeValue { N = ttlTimestamp.ToString(CultureInfo.InvariantCulture) }, [AttributeTtl] = new AttributeValue { N = ttlTimestamp.ToString(CultureInfo.InvariantCulture) } } }, ct); - _logger.LogDebug("Cache set for key: {CacheKey}, TTL: {TTL}s", key, ttl.TotalSeconds); + _logger.LogDebug("Cache set for key: {CacheKey}, TTL: {TTL}s", hashedKey, ttl.TotalSeconds); } catch (ResourceNotFoundException ex) { // Table doesn't exist - fail silently in dev, log in production // Infrastructure should create table before deployment _ = (activity?.SetTag("cache.error", "table_not_found")); - _logger.LogWarning(ex, "DynamoDB table {TableName} not found. Unable to cache key {CacheKey}.", _tableName, key); + _logger.LogWarning(ex, "DynamoDB table {TableName} not found. Unable to cache key {CacheKey}.", _tableName, hashedKey); } catch (Exception ex) { _ = (activity?.SetStatus(ActivityStatusCode.Error, ex.Message)); - _logger.LogError(ex, "Error setting cache key {CacheKey} in DynamoDB", key); + _logger.LogError(ex, "Error setting cache key {CacheKey} in DynamoDB", hashedKey); // Fail gracefully - don't throw } } diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/IDistributedCache.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/IDistributedCache.cs index 20669354e..de875cb36 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/IDistributedCache.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/IDistributedCache.cs @@ -8,22 +8,31 @@ namespace Elastic.Documentation.Api.Infrastructure.Caching; /// Abstraction for distributed caching across Lambda invocations. /// Infrastructure concern: Used by other Infrastructure adapters for caching. /// +/// +/// +/// Cache keys should be created using to automatically hash +/// sensitive identifiers and prevent exposing sensitive data in cache keys (CodeQL security requirement). +/// +/// +/// Key format: {category}:{hashed-identifier} (e.g., "idtoken:{hash}" where hash is SHA256 of the identifier) +/// +/// public interface IDistributedCache { /// /// Retrieves a cached value by key. /// - /// Cache key following pattern: {category}:{identifier} (e.g., "idtoken:https://example.com") + /// Cache key created using (format: {category}:{hashed-identifier}) /// Cancellation token /// Cached value as string, or null if not found or expired - Task GetAsync(string key, Cancel ct = default); + Task GetAsync(CacheKey key, Cancel ct = default); /// /// Stores a value in the cache with a time-to-live. /// - /// Cache key following pattern: {category}:{identifier} + /// Cache key created using (format: {category}:{hashed-identifier}) /// Value to cache (typically JSON-serialized data) /// Time-to-live duration /// Cancellation token - Task SetAsync(string key, string value, TimeSpan ttl, Cancel ct = default); + Task SetAsync(CacheKey key, string value, TimeSpan ttl, Cancel ct = default); } diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/InMemoryDistributedCache.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/InMemoryDistributedCache.cs index 50256cc05..9733ff574 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/InMemoryDistributedCache.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/InMemoryDistributedCache.cs @@ -21,14 +21,15 @@ public sealed class InMemoryDistributedCache : IDistributedCache /// private sealed record CacheEntry(string Value, DateTimeOffset ExpiresAt); - public Task GetAsync(string key, Cancel ct = default) + public Task GetAsync(CacheKey key, Cancel ct = default) { - if (_cache.TryGetValue(key, out var entry)) + var hashedKey = key.Value; + if (_cache.TryGetValue(hashedKey, out var entry)) { if (IsExpired(entry)) { // Remove expired entry - _ = _cache.TryRemove(key, out _); + _ = _cache.TryRemove(hashedKey, out _); return Task.FromResult(null); } @@ -38,11 +39,12 @@ private sealed record CacheEntry(string Value, DateTimeOffset ExpiresAt); return Task.FromResult(null); } - public Task SetAsync(string key, string value, TimeSpan ttl, Cancel ct = default) + public Task SetAsync(CacheKey key, string value, TimeSpan ttl, Cancel ct = default) { + var hashedKey = key.Value; var expiresAt = DateTimeOffset.UtcNow.Add(ttl); var entry = new CacheEntry(value, expiresAt); - _ = _cache.AddOrUpdate(key, entry, (_, _) => entry); + _ = _cache.AddOrUpdate(hashedKey, entry, (_, _) => entry); return Task.CompletedTask; } diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/MultiLayerCache.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/MultiLayerCache.cs index 434b1ce9f..273dd1edb 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/MultiLayerCache.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/MultiLayerCache.cs @@ -33,18 +33,19 @@ public sealed class MultiLayerCache(IDistributedCache l2Cache, ILogger private sealed record L1CacheEntry(string Value, DateTimeOffset ExpiresAt); - public async Task GetAsync(string key, Cancel ct = default) + public async Task GetAsync(CacheKey key, Cancel ct = default) { + var hashedKey = key.Value; using var activity = ActivitySource.StartActivity("get cache", ActivityKind.Internal); - _ = (activity?.SetTag("cache.key", key)); + _ = (activity?.SetTag("cache.key", hashedKey)); _ = (activity?.SetTag("cache.backend", "multilayer")); // L1: Check in-memory cache first (fastest) - if (TryGetFromL1(key, out var value)) + if (TryGetFromL1(hashedKey, out var value)) { _ = (activity?.SetTag("cache.l1.hit", true)); _ = (activity?.SetTag("cache.l2.hit", false)); - _logger.LogDebug("L1 cache hit for key: {CacheKey}", key); + _logger.LogDebug("L1 cache hit for key: {CacheKey}", hashedKey); return value; } @@ -56,31 +57,32 @@ private sealed record L1CacheEntry(string Value, DateTimeOffset ExpiresAt); if (l2Value != null) { _ = (activity?.SetTag("cache.l2.hit", true)); - _logger.LogDebug("L2 cache hit for key: {CacheKey}, populating L1", key); + _logger.LogDebug("L2 cache hit for key: {CacheKey}, populating L1", hashedKey); // Populate L1 cache for future requests in this container // Use a reasonable TTL for L1 (1 hour) to match ID token lifetime - PopulateL1(key, l2Value, TimeSpan.FromHours(1)); + PopulateL1(hashedKey, l2Value, TimeSpan.FromHours(1)); } else { _ = (activity?.SetTag("cache.l2.hit", false)); - _logger.LogDebug("Cache miss (L1 and L2) for key: {CacheKey}", key); + _logger.LogDebug("Cache miss (L1 and L2) for key: {CacheKey}", hashedKey); } return l2Value; } - public async Task SetAsync(string key, string value, TimeSpan ttl, Cancel ct = default) + public async Task SetAsync(CacheKey key, string value, TimeSpan ttl, Cancel ct = default) { + var hashedKey = key.Value; using var activity = ActivitySource.StartActivity("set cache", ActivityKind.Internal); - _ = (activity?.SetTag("cache.key", key)); + _ = (activity?.SetTag("cache.key", hashedKey)); _ = (activity?.SetTag("cache.backend", "multilayer")); _ = (activity?.SetTag("cache.ttl", ttl.TotalSeconds)); // Write-through: Update both L1 and L2 - PopulateL1(key, value, ttl); - _logger.LogDebug("Writing to L1 and L2 cache for key: {CacheKey}, TTL: {TTL}s", key, ttl.TotalSeconds); + PopulateL1(hashedKey, value, ttl); + _logger.LogDebug("Writing to L1 and L2 cache for key: {CacheKey}, TTL: {TTL}s", hashedKey, ttl.TotalSeconds); await _l2Cache.SetAsync(key, value, ttl, ct); } diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Gcp/GcpIdTokenProvider.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Gcp/GcpIdTokenProvider.cs index 4de088990..69e5fea2c 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Gcp/GcpIdTokenProvider.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Gcp/GcpIdTokenProvider.cs @@ -12,7 +12,7 @@ namespace Elastic.Documentation.Api.Infrastructure.Gcp; // This is a custom implementation to create an ID token for GCP. // Because Google.Api.Auth.OAuth2 is not compatible with AOT -// Clean Architecture: Depends on IDistributedCache abstraction from Core layer +// Clean Architecture: Depends on IDistributedCache abstraction from Infrastructure layer public class GcpIdTokenProvider(IHttpClientFactory httpClientFactory, IDistributedCache cache) : IGcpIdTokenProvider { private readonly IHttpClientFactory _httpClientFactory = httpClientFactory; @@ -21,7 +21,8 @@ public class GcpIdTokenProvider(IHttpClientFactory httpClientFactory, IDistribut public async Task GenerateIdTokenAsync(string serviceAccount, string targetAudience, Cancel cancellationToken = default) { // Check distributed cache first (works across all Lambda containers) - var cacheKey = $"idtoken:{targetAudience}"; + // CacheKey automatically hashes the identifier to prevent exposing sensitive data + var cacheKey = CacheKey.Create("idtoken", targetAudience); var cachedJson = await _cache.GetAsync(cacheKey, cancellationToken); if (cachedJson != null) @@ -118,6 +119,7 @@ private static string Base64UrlEncode(byte[] input) // Convert base64 to base64url encoding return base64.Replace('+', '-').Replace('/', '_').TrimEnd('='); } + } internal readonly record struct ServiceAccountKey( diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs index 88feca13f..ca181b583 100644 --- a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs @@ -21,8 +21,11 @@ public class InMemoryDistributedCacheTests [Fact] public async Task GetAsyncWhenKeyDoesNotExistReturnsNull() { + // Arrange + var key = CacheKey.Create("test", "nonexistent-key"); + // Act - var result = await _cache.GetAsync("nonexistent-key", TestContext.Current.CancellationToken); + var result = await _cache.GetAsync(key, TestContext.Current.CancellationToken); // Assert result.Should().BeNull(); @@ -32,7 +35,7 @@ public async Task GetAsyncWhenKeyDoesNotExistReturnsNull() public async Task SetAndGetWhenKeyIsSetReturnsValue() { // Arrange - const string key = "test-key"; + var key = CacheKey.Create("test", "test-key"); const string value = "test-value"; // Act @@ -47,7 +50,7 @@ public async Task SetAndGetWhenKeyIsSetReturnsValue() public async Task GetAsyncWhenEntryExpiredReturnsNull() { // Arrange - const string key = "expiring-key"; + var key = CacheKey.Create("test", "expiring-key"); // Act await _cache.SetAsync(key, "value", TimeSpan.FromMilliseconds(10), TestContext.Current.CancellationToken); @@ -62,7 +65,7 @@ public async Task GetAsyncWhenEntryExpiredReturnsNull() public async Task SetAsyncOverwritesExistingValue() { // Arrange - const string key = "key"; + var key = CacheKey.Create("test", "key"); // Act await _cache.SetAsync(key, "first", TimeSpan.FromMinutes(1), TestContext.Current.CancellationToken); @@ -82,7 +85,7 @@ public async Task GetAsyncWhenL1HitDoesNotCallL2Again() // Arrange var fakeL2 = A.Fake(); var cache = new MultiLayerCache(fakeL2, NullLogger.Instance); - var uniqueKey = $"test-key-{Guid.NewGuid()}"; // Use unique key to avoid L1 cache pollution from other tests + var uniqueKey = CacheKey.Create("test", $"test-key-{Guid.NewGuid()}"); // Use unique key to avoid L1 cache pollution from other tests // Pre-populate L1 by setting a value await cache.SetAsync(uniqueKey, "value", TimeSpan.FromMinutes(1), TestContext.Current.CancellationToken); @@ -95,7 +98,7 @@ public async Task GetAsyncWhenL1HitDoesNotCallL2Again() result1.Should().Be("value"); result2.Should().Be("value"); // L2 should only be called once (during SetAsync), not on subsequent Gets - A.CallTo(() => fakeL2.GetAsync(A._, A._)) + A.CallTo(() => fakeL2.GetAsync(A._, A._)) .MustNotHaveHappened(); } @@ -104,7 +107,7 @@ public async Task GetAsyncWhenL1MissCallsL2AndPopulatesL1() { // Arrange var fakeL2 = A.Fake(); - var uniqueKey = $"test-key-{Guid.NewGuid()}"; // Use unique key to avoid L1 cache pollution from other tests + var uniqueKey = CacheKey.Create("test", $"test-key-{Guid.NewGuid()}"); // Use unique key to avoid L1 cache pollution from other tests A.CallTo(() => fakeL2.GetAsync(uniqueKey, A._)) .Returns("l2-value"); @@ -128,7 +131,7 @@ public async Task SetAsyncWritesToBothL1AndL2() // Arrange var fakeL2 = A.Fake(); var cache = new MultiLayerCache(fakeL2, NullLogger.Instance); - var uniqueKey = $"test-key-{Guid.NewGuid()}"; // Use unique key to avoid L1 cache pollution from other tests + var uniqueKey = CacheKey.Create("test", $"test-key-{Guid.NewGuid()}"); // Use unique key to avoid L1 cache pollution from other tests // Act await cache.SetAsync(uniqueKey, "value", TimeSpan.FromMinutes(1), TestContext.Current.CancellationToken); @@ -147,13 +150,14 @@ public async Task GetAsyncWhenBothCachesMissReturnsNull() { // Arrange var fakeL2 = A.Fake(); - A.CallTo(() => fakeL2.GetAsync(A._, A._)) + A.CallTo(() => fakeL2.GetAsync(A._, A._)) .Returns((string?)null); var cache = new MultiLayerCache(fakeL2, NullLogger.Instance); + var key = CacheKey.Create("test", "missing-key"); // Act - var result = await cache.GetAsync("missing-key", TestContext.Current.CancellationToken); + var result = await cache.GetAsync(key, TestContext.Current.CancellationToken); // Assert result.Should().BeNull(); @@ -167,13 +171,14 @@ public async Task GetAsyncWhenItemExistsReturnsValue() { // Arrange var fakeDynamoDb = A.Fake(); + var key = CacheKey.Create("test", "test-key"); var expiresAt = DateTimeOffset.UtcNow.AddMinutes(30).ToUnixTimeSeconds(); var response = new GetItemResponse { Item = new Dictionary { - ["CacheKey"] = new AttributeValue { S = "test-key" }, + ["CacheKey"] = new AttributeValue { S = key.Value }, ["Value"] = new AttributeValue { S = "test-value" }, ["ExpiresAt"] = new AttributeValue { N = expiresAt.ToString(CultureInfo.InvariantCulture) } }, @@ -186,7 +191,7 @@ public async Task GetAsyncWhenItemExistsReturnsValue() var cache = new DynamoDbDistributedCache(fakeDynamoDb, "test-table", NullLogger.Instance); // Act - var result = await cache.GetAsync("test-key", TestContext.Current.CancellationToken); + var result = await cache.GetAsync(key, TestContext.Current.CancellationToken); // Assert result.Should().Be("test-value"); @@ -197,13 +202,14 @@ public async Task GetAsyncWhenItemExpiredReturnsNull() { // Arrange var fakeDynamoDb = A.Fake(); + var key = CacheKey.Create("test", "test-key"); var expiresAt = DateTimeOffset.UtcNow.AddMinutes(-5).ToUnixTimeSeconds(); // Expired 5 min ago var response = new GetItemResponse { Item = new Dictionary { - ["CacheKey"] = new AttributeValue { S = "test-key" }, + ["CacheKey"] = new AttributeValue { S = key.Value }, ["Value"] = new AttributeValue { S = "test-value" }, ["ExpiresAt"] = new AttributeValue { N = expiresAt.ToString(CultureInfo.InvariantCulture) } }, @@ -216,7 +222,7 @@ public async Task GetAsyncWhenItemExpiredReturnsNull() var cache = new DynamoDbDistributedCache(fakeDynamoDb, "test-table", NullLogger.Instance); // Act - var result = await cache.GetAsync("test-key", TestContext.Current.CancellationToken); + var result = await cache.GetAsync(key, TestContext.Current.CancellationToken); // Assert result.Should().BeNull("expired items should not be returned"); @@ -227,6 +233,7 @@ public async Task GetAsyncWhenItemDoesNotExistReturnsNull() { // Arrange var fakeDynamoDb = A.Fake(); + var key = CacheKey.Create("test", "missing-key"); var response = new GetItemResponse { IsItemSet = false }; A.CallTo(() => fakeDynamoDb.GetItemAsync(A._, A._)) @@ -235,7 +242,7 @@ public async Task GetAsyncWhenItemDoesNotExistReturnsNull() var cache = new DynamoDbDistributedCache(fakeDynamoDb, "test-table", NullLogger.Instance); // Act - var result = await cache.GetAsync("missing-key", TestContext.Current.CancellationToken); + var result = await cache.GetAsync(key, TestContext.Current.CancellationToken); // Assert result.Should().BeNull(); @@ -247,15 +254,16 @@ public async Task SetAsyncCallsDynamoDbPutItem() // Arrange var fakeDynamoDb = A.Fake(); var cache = new DynamoDbDistributedCache(fakeDynamoDb, "test-table", NullLogger.Instance); + var key = CacheKey.Create("test", "key"); // Act - await cache.SetAsync("key", "value", TimeSpan.FromMinutes(30), TestContext.Current.CancellationToken); + await cache.SetAsync(key, "value", TimeSpan.FromMinutes(30), TestContext.Current.CancellationToken); // Assert A.CallTo(() => fakeDynamoDb.PutItemAsync( A.That.Matches(r => r.TableName == "test-table" && - r.Item["CacheKey"].S == "key" && + r.Item["CacheKey"].S == key.Value && r.Item["Value"].S == "value" ), A._ @@ -267,6 +275,7 @@ public async Task GetAsyncWhenTableNotFoundReturnsNullGracefully() { // Arrange var fakeDynamoDb = A.Fake(); + var key = CacheKey.Create("test", "key"); var exception = new ResourceNotFoundException("Table not found"); A.CallTo(() => fakeDynamoDb.GetItemAsync(A._, A._)) .Throws(exception); @@ -274,7 +283,7 @@ public async Task GetAsyncWhenTableNotFoundReturnsNullGracefully() var cache = new DynamoDbDistributedCache(fakeDynamoDb, "missing-table", NullLogger.Instance); // Act - var result = await cache.GetAsync("key", TestContext.Current.CancellationToken); + var result = await cache.GetAsync(key, TestContext.Current.CancellationToken); // Assert result.Should().BeNull("should handle missing table gracefully"); @@ -300,7 +309,8 @@ public async Task GenerateIdTokenAsyncUsesCachedTokenWhenValid() expiresAtUnix = DateTimeOffset.UtcNow.AddMinutes(50).ToUnixTimeSeconds() }; var cacheJson = JsonSerializer.Serialize(cachedToken); - await cache.SetAsync($"idtoken:{targetAudience}", cacheJson, TimeSpan.FromHours(1), TestContext.Current.CancellationToken); + var cacheKey = CacheKey.Create("idtoken", targetAudience); + await cache.SetAsync(cacheKey, cacheJson, TimeSpan.FromHours(1), TestContext.Current.CancellationToken); // Act var result = await provider.GenerateIdTokenAsync("{}", targetAudience, TestContext.Current.CancellationToken); @@ -324,10 +334,11 @@ public async Task GenerateIdTokenAsyncIgnoresExpiredCachedToken() expiresAtUnix = DateTimeOffset.UtcNow.AddSeconds(30).ToUnixTimeSeconds() // Only 30s left }; var cacheJson = JsonSerializer.Serialize(expiredToken); - await cache.SetAsync("idtoken:https://test.com", cacheJson, TimeSpan.FromHours(1), TestContext.Current.CancellationToken); + var cacheKey = CacheKey.Create("idtoken", "https://test.com"); + await cache.SetAsync(cacheKey, cacheJson, TimeSpan.FromHours(1), TestContext.Current.CancellationToken); // Act - Try to get the expired token - var cachedValue = await cache.GetAsync("idtoken:https://test.com", TestContext.Current.CancellationToken); + var cachedValue = await cache.GetAsync(cacheKey, TestContext.Current.CancellationToken); var parsedToken = JsonSerializer.Deserialize(cachedValue!); var expiresAt = DateTimeOffset.FromUnixTimeSeconds(parsedToken.GetProperty("expiresAtUnix").GetInt64()); From 3ce7873f6cfce6779e0bcac1685ae93a50d3606e Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Thu, 27 Nov 2025 13:09:41 +0100 Subject: [PATCH 07/14] Potential fix for pull request finding 'Generic catch clause' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- .../Caching/DynamoDbDistributedCache.cs | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs index 679aa9a9f..f3e94206f 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs @@ -85,12 +85,25 @@ public sealed class DynamoDbDistributedCache(IAmazonDynamoDB dynamoDb, string ta _logger.LogWarning(ex, "DynamoDB table {TableName} not found. Cache operations will fail gracefully.", _tableName); return null; } - catch (Exception ex) + catch (ProvisionedThroughputExceededException ex) + { + _ = (activity?.SetTag("cache.error", "provisioned_throughput_exceeded")); + _logger.LogWarning(ex, "Provisioned throughput exceeded for DynamoDB cache table {TableName}.", _tableName); + return null; + } + catch (InternalServerErrorException ex) + { + _ = (activity?.SetTag("cache.error", "internal_server_error")); + _logger.LogError(ex, "Internal server error retrieving cache key {CacheKey} from DynamoDB", hashedKey); + return null; + } + catch (Exception ex) when (ex is not OperationCanceledException && ex is not TaskCanceledException) { _ = (activity?.SetStatus(ActivityStatusCode.Error, ex.Message)); _logger.LogError(ex, "Error retrieving cache key {CacheKey} from DynamoDB", hashedKey); return null; // Fail gracefully } + // Allow cancellation exceptions to propagate to respect request lifetimes } public async Task SetAsync(CacheKey key, string value, TimeSpan ttl, Cancel ct = default) @@ -128,7 +141,18 @@ public async Task SetAsync(CacheKey key, string value, TimeSpan ttl, Cancel ct = _ = (activity?.SetTag("cache.error", "table_not_found")); _logger.LogWarning(ex, "DynamoDB table {TableName} not found. Unable to cache key {CacheKey}.", _tableName, hashedKey); } - catch (Exception ex) + catch (ProvisionedThroughputExceededException ex) + _ = (activity?.SetTag("cache.error", "provisioned_throughput_exceeded")); + _logger.LogWarning(ex, "Provisioned throughput exceeded for DynamoDB cache table {TableName}. Unable to cache key {CacheKey}.", _tableName, hashedKey); + } + catch (InternalServerErrorException ex) + { + _ = (activity?.SetTag("cache.error", "internal_server_error")); + _logger.LogError(ex, "Internal server error setting cache key {CacheKey} in DynamoDB", hashedKey); + } + catch (Exception ex) when (ex is not OperationCanceledException && ex is not TaskCanceledException) + { + // Allow cancellation exceptions to propagate to respect request lifetimes { _ = (activity?.SetStatus(ActivityStatusCode.Error, ex.Message)); _logger.LogError(ex, "Error setting cache key {CacheKey} in DynamoDB", hashedKey); From db22e6d7627753f2672a7df800f52fa4befaf31f Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Thu, 27 Nov 2025 13:20:09 +0100 Subject: [PATCH 08/14] Fix formatting --- .../Caching/DynamoDbDistributedCache.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs index f3e94206f..89085a21d 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs @@ -97,7 +97,7 @@ public sealed class DynamoDbDistributedCache(IAmazonDynamoDB dynamoDb, string ta _logger.LogError(ex, "Internal server error retrieving cache key {CacheKey} from DynamoDB", hashedKey); return null; } - catch (Exception ex) when (ex is not OperationCanceledException && ex is not TaskCanceledException) + catch (Exception ex) when (ex is not OperationCanceledException and not TaskCanceledException) { _ = (activity?.SetStatus(ActivityStatusCode.Error, ex.Message)); _logger.LogError(ex, "Error retrieving cache key {CacheKey} from DynamoDB", hashedKey); @@ -143,22 +143,22 @@ public async Task SetAsync(CacheKey key, string value, TimeSpan ttl, Cancel ct = } catch (ProvisionedThroughputExceededException ex) _ = (activity?.SetTag("cache.error", "provisioned_throughput_exceeded")); - _logger.LogWarning(ex, "Provisioned throughput exceeded for DynamoDB cache table {TableName}. Unable to cache key {CacheKey}.", _tableName, hashedKey); + _logger.LogWarning(ex, "Provisioned throughput exceeded for DynamoDB cache table {TableName}. Unable to cache key {CacheKey}.", _tableName, hashedKey); } catch (InternalServerErrorException ex) { _ = (activity?.SetTag("cache.error", "internal_server_error")); _logger.LogError(ex, "Internal server error setting cache key {CacheKey} in DynamoDB", hashedKey); } - catch (Exception ex) when (ex is not OperationCanceledException && ex is not TaskCanceledException) + catch (Exception ex) when (ex is not OperationCanceledException and not TaskCanceledException) { - // Allow cancellation exceptions to propagate to respect request lifetimes - { - _ = (activity?.SetStatus(ActivityStatusCode.Error, ex.Message)); - _logger.LogError(ex, "Error setting cache key {CacheKey} in DynamoDB", hashedKey); - // Fail gracefully - don't throw + // Allow cancellation exceptions to propagate to respect request lifetimes + { + _ = (activity?.SetStatus(ActivityStatusCode.Error, ex.Message)); + _logger.LogError(ex, "Error setting cache key {CacheKey} in DynamoDB", hashedKey); + // Fail gracefully - don't throw + } } - } /// /// Checks if a DynamoDB item has expired based on ExpiresAt attribute. From 64a172e5089971ac0a897760f8760d0574fd8b77 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Thu, 27 Nov 2025 13:31:29 +0100 Subject: [PATCH 09/14] Fix syntax error and cleanup --- .../Caching/DynamoDbDistributedCache.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs index 89085a21d..80f015208 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs @@ -142,8 +142,9 @@ public async Task SetAsync(CacheKey key, string value, TimeSpan ttl, Cancel ct = _logger.LogWarning(ex, "DynamoDB table {TableName} not found. Unable to cache key {CacheKey}.", _tableName, hashedKey); } catch (ProvisionedThroughputExceededException ex) + { _ = (activity?.SetTag("cache.error", "provisioned_throughput_exceeded")); - _logger.LogWarning(ex, "Provisioned throughput exceeded for DynamoDB cache table {TableName}. Unable to cache key {CacheKey}.", _tableName, hashedKey); + _logger.LogWarning(ex, "Provisioned throughput exceeded for DynamoDB cache table {TableName}. Unable to cache key {CacheKey}.", _tableName, hashedKey); } catch (InternalServerErrorException ex) { @@ -153,12 +154,11 @@ public async Task SetAsync(CacheKey key, string value, TimeSpan ttl, Cancel ct = catch (Exception ex) when (ex is not OperationCanceledException and not TaskCanceledException) { // Allow cancellation exceptions to propagate to respect request lifetimes - { - _ = (activity?.SetStatus(ActivityStatusCode.Error, ex.Message)); - _logger.LogError(ex, "Error setting cache key {CacheKey} in DynamoDB", hashedKey); - // Fail gracefully - don't throw - } + _ = activity?.SetStatus(ActivityStatusCode.Error, ex.Message); + _logger.LogError(ex, "Error setting cache key {CacheKey} in DynamoDB", hashedKey); + // Fail gracefully - don't throw } + } /// /// Checks if a DynamoDB item has expired based on ExpiresAt attribute. From e3a2b170131293dbd4bf84738aee3d3298a3b207 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Thu, 27 Nov 2025 13:46:48 +0100 Subject: [PATCH 10/14] Simplify --- .../Gcp/GcpIdTokenProvider.cs | 30 ++++--------- .../Caching/DistributedCacheTests.cs | 44 +++++++++---------- 2 files changed, 30 insertions(+), 44 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Gcp/GcpIdTokenProvider.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Gcp/GcpIdTokenProvider.cs index 69e5fea2c..bb731d8f8 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Gcp/GcpIdTokenProvider.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Gcp/GcpIdTokenProvider.cs @@ -22,17 +22,15 @@ public async Task GenerateIdTokenAsync(string serviceAccount, string tar { // Check distributed cache first (works across all Lambda containers) // CacheKey automatically hashes the identifier to prevent exposing sensitive data + // DynamoDB ExpiresAt attribute handles expiration checking var cacheKey = CacheKey.Create("idtoken", targetAudience); - var cachedJson = await _cache.GetAsync(cacheKey, cancellationToken); + var cachedToken = await _cache.GetAsync(cacheKey, cancellationToken); - if (cachedJson != null) + if (cachedToken != null) { - var cachedToken = JsonSerializer.Deserialize(cachedJson, IdTokenCacheJsonContext.Default.CachedIdToken); - - // Check if token is still valid (refresh 1 minute before expiry) - var expiresAt = DateTimeOffset.FromUnixTimeSeconds(cachedToken.ExpiresAtUnix); - if (expiresAt > DateTimeOffset.UtcNow.AddMinutes(1)) - return cachedToken.Token; + // Cache implementation (DynamoDbDistributedCache) already checked ExpiresAt + // If we get here, the token is still valid + return cachedToken; } // Read and parse service account key file using System.Text.Json source generation (AOT compatible) @@ -83,10 +81,10 @@ public async Task GenerateIdTokenAsync(string serviceAccount, string tar // Cache the token in distributed cache (shared across all Lambda containers) // Use 15-minute buffer for maximum safety against clock skew and edge cases + // DynamoDB will use ExpiresAt attribute for expiration checking and TTL for cleanup var cacheExpiry = expirationTime.Subtract(TimeSpan.FromMinutes(15)); - var cacheEntry = new CachedIdToken(idToken, cacheExpiry.ToUnixTimeSeconds()); - var cacheJson = JsonSerializer.Serialize(cacheEntry, IdTokenCacheJsonContext.Default.CachedIdToken); - await _cache.SetAsync(cacheKey, cacheJson, TimeSpan.FromHours(1), cancellationToken); + var cacheTtl = cacheExpiry - now; + await _cache.SetAsync(cacheKey, idToken, cacheTtl, cancellationToken); return idToken; } @@ -146,12 +144,6 @@ internal readonly record struct JwtPayload( string TargetAudience ); -/// -/// Cached ID token structure for distributed cache storage. -/// AOT-compatible: Uses source-generated JSON serialization. -/// -internal readonly record struct CachedIdToken(string Token, long ExpiresAtUnix); - [JsonSerializable(typeof(ServiceAccountKey))] [JsonSerializable(typeof(JwtPayload))] [JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.SnakeCaseLower)] @@ -160,7 +152,3 @@ internal sealed partial class GcpJsonContext : JsonSerializerContext; [JsonSerializable(typeof(JwtHeader))] [JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase)] internal sealed partial class JwtHeaderJsonContext : JsonSerializerContext; - -[JsonSerializable(typeof(CachedIdToken))] -[JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase)] -internal sealed partial class IdTokenCacheJsonContext : JsonSerializerContext; diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs index ca181b583..99f45fc64 100644 --- a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs @@ -302,15 +302,10 @@ public async Task GenerateIdTokenAsyncUsesCachedTokenWhenValid() var provider = new GcpIdTokenProvider(fakeHttpClientFactory, cache); const string targetAudience = "https://test-audience.googleapis.com"; - // Pre-populate cache with valid token (expires in 50 minutes) - var cachedToken = new - { - token = "fake-cached-token", - expiresAtUnix = DateTimeOffset.UtcNow.AddMinutes(50).ToUnixTimeSeconds() - }; - var cacheJson = JsonSerializer.Serialize(cachedToken); + // Pre-populate cache with valid token (TTL of 45 minutes - matches cache expiry logic) + const string cachedToken = "fake-cached-token"; var cacheKey = CacheKey.Create("idtoken", targetAudience); - await cache.SetAsync(cacheKey, cacheJson, TimeSpan.FromHours(1), TestContext.Current.CancellationToken); + await cache.SetAsync(cacheKey, cachedToken, TimeSpan.FromMinutes(45), TestContext.Current.CancellationToken); // Act var result = await provider.GenerateIdTokenAsync("{}", targetAudience, TestContext.Current.CancellationToken); @@ -325,25 +320,28 @@ public async Task GenerateIdTokenAsyncUsesCachedTokenWhenValid() public async Task GenerateIdTokenAsyncIgnoresExpiredCachedToken() { // Arrange + var fakeHttpClientFactory = A.Fake(); var cache = new InMemoryDistributedCache(); - // Pre-populate cache with expired token (already past 45-minute threshold) - var expiredToken = new - { - token = "expired-token", - expiresAtUnix = DateTimeOffset.UtcNow.AddSeconds(30).ToUnixTimeSeconds() // Only 30s left - }; - var cacheJson = JsonSerializer.Serialize(expiredToken); - var cacheKey = CacheKey.Create("idtoken", "https://test.com"); - await cache.SetAsync(cacheKey, cacheJson, TimeSpan.FromHours(1), TestContext.Current.CancellationToken); + var provider = new GcpIdTokenProvider(fakeHttpClientFactory, cache); + const string targetAudience = "https://test.com"; - // Act - Try to get the expired token + // Pre-populate cache with token that has very short TTL (will expire quickly) + // InMemoryDistributedCache will remove it when expired + const string expiredToken = "expired-token"; + var cacheKey = CacheKey.Create("idtoken", targetAudience); + await cache.SetAsync(cacheKey, expiredToken, TimeSpan.FromMilliseconds(10), TestContext.Current.CancellationToken); + + // Wait for expiration + await Task.Delay(50, TestContext.Current.CancellationToken); + + // Act - Try to get the expired token (should be null, triggering new token generation) var cachedValue = await cache.GetAsync(cacheKey, TestContext.Current.CancellationToken); - var parsedToken = JsonSerializer.Deserialize(cachedValue!); - var expiresAt = DateTimeOffset.FromUnixTimeSeconds(parsedToken.GetProperty("expiresAtUnix").GetInt64()); - // Assert - Token should be considered expired (less than 1 minute buffer) - (expiresAt <= DateTimeOffset.UtcNow.AddMinutes(1)).Should().BeTrue( - "tokens with less than 1 minute remaining should be refreshed"); + // Assert - Expired cache entry should return null (cache handles expiration via TTL) + cachedValue.Should().BeNull("expired cache entries should return null"); + + // Since cache returned null, provider should generate a new token + // This test verifies that expired cache entries don't prevent new token generation } } From 901f2041fb6deccd26d8b956cafcb5ab8d084fa4 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Thu, 27 Nov 2025 14:00:19 +0100 Subject: [PATCH 11/14] Simplify test --- .../Caching/DistributedCacheTests.cs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs index 99f45fc64..e8ed9ddd5 100644 --- a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs @@ -320,10 +320,7 @@ public async Task GenerateIdTokenAsyncUsesCachedTokenWhenValid() public async Task GenerateIdTokenAsyncIgnoresExpiredCachedToken() { // Arrange - var fakeHttpClientFactory = A.Fake(); var cache = new InMemoryDistributedCache(); - - var provider = new GcpIdTokenProvider(fakeHttpClientFactory, cache); const string targetAudience = "https://test.com"; // Pre-populate cache with token that has very short TTL (will expire quickly) @@ -335,13 +332,12 @@ public async Task GenerateIdTokenAsyncIgnoresExpiredCachedToken() // Wait for expiration await Task.Delay(50, TestContext.Current.CancellationToken); - // Act - Try to get the expired token (should be null, triggering new token generation) + // Act - Try to get the expired token var cachedValue = await cache.GetAsync(cacheKey, TestContext.Current.CancellationToken); - // Assert - Expired cache entry should return null (cache handles expiration via TTL) - cachedValue.Should().BeNull("expired cache entries should return null"); - - // Since cache returned null, provider should generate a new token - // This test verifies that expired cache entries don't prevent new token generation + // Assert - Expired cache entry should return null + // GcpIdTokenProvider checks `if (cachedToken != null)` - when cache returns null, + // it will generate a new token, effectively ignoring the expired cached token + cachedValue.Should().BeNull("expired cache entries should return null, allowing provider to generate new token"); } } From 54adcba4428a232563937b51a132318aeeee6c07 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Thu, 27 Nov 2025 23:34:54 +0100 Subject: [PATCH 12/14] Also apply simplification to the gateway --- .../Caching/DynamoDbDistributedCache.cs | 29 ++------------- .../Caching/DistributedCacheTests.cs | 35 ++----------------- 2 files changed, 4 insertions(+), 60 deletions(-) diff --git a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs index 80f015208..3b6f70791 100644 --- a/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs +++ b/src/api/Elastic.Documentation.Api.Infrastructure/Caching/DynamoDbDistributedCache.cs @@ -27,7 +27,6 @@ public sealed class DynamoDbDistributedCache(IAmazonDynamoDB dynamoDb, string ta // DynamoDB attribute names private const string AttributeCacheKey = "CacheKey"; private const string AttributeValue = "Value"; - private const string AttributeExpiresAt = "ExpiresAt"; private const string AttributeTtl = "TTL"; public async Task GetAsync(CacheKey key, Cancel ct = default) @@ -56,15 +55,8 @@ public sealed class DynamoDbDistributedCache(IAmazonDynamoDB dynamoDb, string ta return null; } - // Check if expired (application-level check, DynamoDB TTL is for cleanup) - if (IsExpired(response.Item)) - { - _ = (activity?.SetTag("cache.hit", false)); - _ = (activity?.SetTag("cache.expired", true)); - _logger.LogDebug("Cache expired for key: {CacheKey}", hashedKey); - return null; - } - + // DynamoDB TTL handles expiration automatically + // Items may still be returned briefly after expiration until DynamoDB deletes them var value = response.Item.TryGetValue(AttributeValue, out var valueAttr) ? valueAttr.S : null; @@ -127,7 +119,6 @@ public async Task SetAsync(CacheKey key, string value, TimeSpan ttl, Cancel ct = { [AttributeCacheKey] = new AttributeValue { S = hashedKey }, [AttributeValue] = new AttributeValue { S = value }, - [AttributeExpiresAt] = new AttributeValue { N = ttlTimestamp.ToString(CultureInfo.InvariantCulture) }, [AttributeTtl] = new AttributeValue { N = ttlTimestamp.ToString(CultureInfo.InvariantCulture) } } }, ct); @@ -159,20 +150,4 @@ public async Task SetAsync(CacheKey key, string value, TimeSpan ttl, Cancel ct = // Fail gracefully - don't throw } } - - /// - /// Checks if a DynamoDB item has expired based on ExpiresAt attribute. - /// Clean Code: Single-purpose helper method with intention-revealing name. - /// - private static bool IsExpired(Dictionary item) - { - if (!item.TryGetValue(AttributeExpiresAt, out var expiresAtAttr)) - return true; // No expiration timestamp = treat as expired - - if (!long.TryParse(expiresAtAttr.N, out var expiresAtUnix)) - return true; // Invalid timestamp = treat as expired - - var expiresAt = DateTimeOffset.FromUnixTimeSeconds(expiresAtUnix); - return expiresAt <= DateTimeOffset.UtcNow; - } } diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs index e8ed9ddd5..1a6771d82 100644 --- a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs @@ -172,7 +172,7 @@ public async Task GetAsyncWhenItemExistsReturnsValue() // Arrange var fakeDynamoDb = A.Fake(); var key = CacheKey.Create("test", "test-key"); - var expiresAt = DateTimeOffset.UtcNow.AddMinutes(30).ToUnixTimeSeconds(); + // Note: We no longer use ExpiresAt - DynamoDB TTL handles expiration automatically var response = new GetItemResponse { @@ -180,7 +180,7 @@ public async Task GetAsyncWhenItemExistsReturnsValue() { ["CacheKey"] = new AttributeValue { S = key.Value }, ["Value"] = new AttributeValue { S = "test-value" }, - ["ExpiresAt"] = new AttributeValue { N = expiresAt.ToString(CultureInfo.InvariantCulture) } + ["TTL"] = new AttributeValue { N = DateTimeOffset.UtcNow.AddMinutes(30).ToUnixTimeSeconds().ToString(CultureInfo.InvariantCulture) } }, IsItemSet = true }; @@ -197,37 +197,6 @@ public async Task GetAsyncWhenItemExistsReturnsValue() result.Should().Be("test-value"); } - [Fact] - public async Task GetAsyncWhenItemExpiredReturnsNull() - { - // Arrange - var fakeDynamoDb = A.Fake(); - var key = CacheKey.Create("test", "test-key"); - var expiresAt = DateTimeOffset.UtcNow.AddMinutes(-5).ToUnixTimeSeconds(); // Expired 5 min ago - - var response = new GetItemResponse - { - Item = new Dictionary - { - ["CacheKey"] = new AttributeValue { S = key.Value }, - ["Value"] = new AttributeValue { S = "test-value" }, - ["ExpiresAt"] = new AttributeValue { N = expiresAt.ToString(CultureInfo.InvariantCulture) } - }, - IsItemSet = true - }; - - A.CallTo(() => fakeDynamoDb.GetItemAsync(A._, A._)) - .Returns(response); - - var cache = new DynamoDbDistributedCache(fakeDynamoDb, "test-table", NullLogger.Instance); - - // Act - var result = await cache.GetAsync(key, TestContext.Current.CancellationToken); - - // Assert - result.Should().BeNull("expired items should not be returned"); - } - [Fact] public async Task GetAsyncWhenItemDoesNotExistReturnsNull() { From 05643bf666a8a242e88d3f8ffe73eddd02fd7a81 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Thu, 27 Nov 2025 23:47:25 +0100 Subject: [PATCH 13/14] Fix test --- .../Caching/DistributedCacheTests.cs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs index 1a6771d82..13ac4b78b 100644 --- a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs @@ -265,7 +265,14 @@ public class GcpIdTokenProviderCachingIntegrationTests public async Task GenerateIdTokenAsyncUsesCachedTokenWhenValid() { // Arrange + // Use HttpMessageHandler directly with method name matching to verify ExchangeJwtForIdToken was not called + // See: https://fakeiteasy.github.io/docs/8.1.0/Recipes/faking-http-client/ + var fakeHandler = A.Fake(); + var httpClient = new HttpClient(fakeHandler); var fakeHttpClientFactory = A.Fake(); + A.CallTo(() => fakeHttpClientFactory.CreateClient(A._)) + .Returns(httpClient); + var cache = new InMemoryDistributedCache(); var provider = new GcpIdTokenProvider(fakeHttpClientFactory, cache); @@ -281,8 +288,12 @@ public async Task GenerateIdTokenAsyncUsesCachedTokenWhenValid() // Assert result.Should().Be("fake-cached-token", "should return cached token without calling Google OAuth"); - // Note: Can't verify CreateClient() wasn't called because it's an extension method (not interceptable by FakeItEasy) - // But the test still validates that cached token is returned correctly + // Verify that ExchangeJwtForIdToken was not called (PostAsync -> SendAsync) + // Using method name matching since SendAsync is protected + A.CallTo(fakeHandler) + .WithReturnType>() + .Where(call => call.Method.Name == "SendAsync") + .MustNotHaveHappened(); } [Fact] From 80f17668b5017f7785e27c80bef6ecc8001ba181 Mon Sep 17 00:00:00 2001 From: Jan Calanog Date: Thu, 27 Nov 2025 23:55:18 +0100 Subject: [PATCH 14/14] Potential fix for pull request finding 'Missing Dispose call on local IDisposable' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- .../Caching/DistributedCacheTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs index 13ac4b78b..a980e1b63 100644 --- a/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs +++ b/tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs @@ -268,7 +268,7 @@ public async Task GenerateIdTokenAsyncUsesCachedTokenWhenValid() // Use HttpMessageHandler directly with method name matching to verify ExchangeJwtForIdToken was not called // See: https://fakeiteasy.github.io/docs/8.1.0/Recipes/faking-http-client/ var fakeHandler = A.Fake(); - var httpClient = new HttpClient(fakeHandler); + using var httpClient = new HttpClient(fakeHandler); var fakeHttpClientFactory = A.Fake(); A.CallTo(() => fakeHttpClientFactory.CreateClient(A._)) .Returns(httpClient);