From 677153737019632f9e00162d6fe733459b3bfe87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustav=20Siljam=C3=A4ki?= Date: Wed, 12 Aug 2026 23:23:44 +0200 Subject: [PATCH] fix: pass the cache key to Mongo as an id, not as a JSON filter Fixes #57. DeleteOneAsync and UpdateOneAsync on DiskRepositoryCollectionBase have no TKey overload, so `collection.DeleteOneAsync(key.Value)` bound to the DeleteOneAsync(FilterDefinition) overload through the driver's implicit string -> JsonFilterDefinition conversion. It compiled cleanly and then made Mongo parse the cache key as a JSON filter document, throwing on every single drop: System.FormatException: JSON reader was expecting a value but found 'FeedStatusDto' CacheBase prefixes every key with typeof(T).Name, so the first bareword in the message is the cached type - which makes it read like a serialization problem far from the actual call. Nothing backed by the MongoDB persist layer could be dropped or evicted from 0.4.1 onwards. GetOneAsync was unaffected because that id overload does exist, so reads kept working and the cache simply went permanently stale. In our production this ran unnoticed for three months. Build the filter explicitly instead, at all three call sites (DropAsync, the stale-entry delete in GetAsync, and SetUpdateTime, which backs Invalidate and BuyMoreTime and had the same latent misbinding). A key can now only ever be read as an id. The regression test renders the filter that reaches the collection and asserts it is an _id equality - that is what tells an id apart from a parsed JSON document without needing a server, so it runs on CI where the Integration category does not. Tharga.Cache.MongoDB.Tests needed InternalsVisibleTo from Tharga.Cache to construct the provider. --- .../MongoDBKeyFilterTests.cs | 68 +++++++++++++++++++ Tharga.Cache.MongoDB/MongoDB.cs | 24 +++++-- Tharga.Cache/Tharga.Cache.csproj | 1 + 3 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 Tharga.Cache.MongoDB.Tests/MongoDBKeyFilterTests.cs diff --git a/Tharga.Cache.MongoDB.Tests/MongoDBKeyFilterTests.cs b/Tharga.Cache.MongoDB.Tests/MongoDBKeyFilterTests.cs new file mode 100644 index 0000000..ab161d7 --- /dev/null +++ b/Tharga.Cache.MongoDB.Tests/MongoDBKeyFilterTests.cs @@ -0,0 +1,68 @@ +using FluentAssertions; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using MongoDB.Bson; +using MongoDB.Bson.Serialization; +using MongoDB.Driver; +using Moq; +using Tharga.Cache.Core; +using Tharga.MongoDB; +using Xunit; +using Sut = Tharga.Cache.MongoDB.MongoDB; + +namespace Tharga.Cache.MongoDB.Tests; + +/// +/// The cache key has to reach Mongo as an id, never as a filter document. +/// +/// +/// DeleteOneAsync and UpdateOneAsync have no TKey overload, so a bare string is picked up by the +/// FilterDefinition overload through the driver's implicit string -> JsonFilterDefinition conversion. +/// That compiles cleanly and then fails at runtime, once per drop, with +/// "JSON reader was expecting a value but found '<TypeName>'" - because CacheBase prefixes every +/// key with the type name. Rendering the filter here is what tells the two apart. +/// +public class MongoDBKeyFilterTests +{ + private const string CacheKey = "FeedStatusDto.my-feed-key"; + + private static BsonDocument Render(FilterDefinition filter) + { + return filter.Render(new RenderArgs(BsonSerializer.SerializerRegistry.GetSerializer(), BsonSerializer.SerializerRegistry)); + } + + private static (Sut Sut, Mock Collection) BuildSut() + { + var collection = new Mock(); + + var collectionProvider = new Mock(); + collectionProvider + .Setup(x => x.GetCollection(It.IsAny())) + .Returns(collection.Object); + + var sut = new Sut(collectionProvider.Object, Mock.Of(), Options.Create(new MongoDBCacheOptions()), Mock.Of>()); + return (sut, collection); + } + + [Fact] + public async Task DropAsync_FiltersOnTheIdRatherThanParsingTheKeyAsJson() + { + var (sut, collection) = BuildSut(); + FilterDefinition captured = null; + collection + .Setup(x => x.DeleteOneAsync(It.IsAny>(), It.IsAny>(), It.IsAny())) + .Callback, OneOption, IClientSessionHandle>((f, _, _) => captured = f) + .ReturnsAsync((CacheEntity)null); + + await sut.DropAsync(CacheKey); + + captured.Should().NotBeNull(); + var rendered = Render(captured); + rendered.ElementCount.Should().Be(1); + rendered.GetElement(0).Name.Should().Be("_id"); + rendered.GetElement(0).Value.AsString.Should().Be(CacheKey); + } + +} + + diff --git a/Tharga.Cache.MongoDB/MongoDB.cs b/Tharga.Cache.MongoDB/MongoDB.cs index b6720f5..5f05e1b 100644 --- a/Tharga.Cache.MongoDB/MongoDB.cs +++ b/Tharga.Cache.MongoDB/MongoDB.cs @@ -42,7 +42,7 @@ public async Task> GetAsync(Key key) { if (!item.StaleWhileRevalidate && item.FreshSpan.HasValue && item.FreshSpan.Value != TimeSpan.MaxValue && item.CreateTime.Add(item.FreshSpan.Value) < DateTime.UtcNow) { - await collection.DeleteOneAsync(item.Id); + await collection.DeleteOneAsync(ById(item.Id)); return null; } @@ -104,15 +104,30 @@ public Task Invalidate(Key key) public async Task DropAsync(Key key) { var collection = GetCollection(); - var item = await collection.DeleteOneAsync(key.Value); + var item = await collection.DeleteOneAsync(ById(key.Value)); return item != null; } + /// + /// The id of a cache entity as an explicit filter. + /// + /// + /// There is no DeleteOneAsync(TKey) overload, so a bare string is picked up by + /// DeleteOneAsync(FilterDefinition<CacheEntity>) through the driver's implicit + /// string -> JsonFilterDefinition conversion. That compiles, and then Mongo parses the + /// cache key as a JSON filter document: "JSON reader was expecting a value but found 'MyType'". + /// Building the filter here means the key can only ever be read as an id. + /// + private static FilterDefinition ById(string id) + { + return new FilterDefinitionBuilder().Eq(x => x.Id, id); + } + private async Task SetUpdateTime(Key key, DateTime updateTime) { var collection = GetCollection(); var update = new UpdateDefinitionBuilder().Set(x => x.UpdateTime, updateTime); - var result = await collection.UpdateOneAsync(key.Value, update, OneOption.SingleOrDefault); + var result = await collection.UpdateOneAsync(ById(key.Value), update, OneOption.SingleOrDefault); return result.Before != null; } @@ -150,4 +165,5 @@ public ValueTask DisposeAsync() { return ValueTask.CompletedTask; } -} \ No newline at end of file +} + diff --git a/Tharga.Cache/Tharga.Cache.csproj b/Tharga.Cache/Tharga.Cache.csproj index 59f110d..56306a2 100644 --- a/Tharga.Cache/Tharga.Cache.csproj +++ b/Tharga.Cache/Tharga.Cache.csproj @@ -48,6 +48,7 @@ +