diff --git a/src/Core/Grand.Data/Audit/IAuditRecordReader.cs b/src/Core/Grand.Data/Audit/IAuditRecordReader.cs index 9b73bb20d..47edbee91 100644 --- a/src/Core/Grand.Data/Audit/IAuditRecordReader.cs +++ b/src/Core/Grand.Data/Audit/IAuditRecordReader.cs @@ -1,7 +1,7 @@ namespace Grand.Data.Audit; /// -/// Reads one stored record as an audit snapshot, to name the record a changed field refers to +/// Reads stored records as audit snapshots, to name the records changed fields refer to /// public interface IAuditRecordReader { @@ -11,4 +11,11 @@ public interface IAuditRecordReader /// the snapshot may hold only that top-level field. /// Task> Read(Type entityType, string value, string key = null, string field = null); + + /// + /// The records whose is one of , in one read, by that value; a + /// value without a record is left out. Otherwise as . + /// + Task>> ReadMany(Type entityType, IReadOnlyCollection values, + string key = null, string field = null); } diff --git a/src/Core/Grand.Data/Audit/RepositoryAudit.cs b/src/Core/Grand.Data/Audit/RepositoryAudit.cs index 7424f40dd..c6fbc58cb 100644 --- a/src/Core/Grand.Data/Audit/RepositoryAudit.cs +++ b/src/Core/Grand.Data/Audit/RepositoryAudit.cs @@ -11,9 +11,14 @@ public sealed class RepositoryAudit(IAuditTracker tracker, Func tracker.IsActive && tracker.IsAudited(typeof(T)); - public Task Inserted(T entity) + public async Task Inserted(T entity) { - return Tracking ? tracker.BeforeWriteAsync(entity.Id, true, loadMany) : Task.CompletedTask; + if (!Tracking) + return; + + await tracker.BeforeWriteAsync(entity.Id, true, loadMany); + //the inserted entity is what was stored, so the flush does not read it back + tracker.AfterFullUpdate(entity); } /// diff --git a/src/Core/Grand.Data/LiteDb/LiteDBAuditRecordReader.cs b/src/Core/Grand.Data/LiteDb/LiteDBAuditRecordReader.cs index 6227f59c7..0cab29dce 100644 --- a/src/Core/Grand.Data/LiteDb/LiteDBAuditRecordReader.cs +++ b/src/Core/Grand.Data/LiteDb/LiteDBAuditRecordReader.cs @@ -22,4 +22,15 @@ public Task> Read(Type entityType, string value, str var entity = document == null ? null : db.Mapper.ToObject(entityType, document); return Task.FromResult(entity == null ? null : AuditSnapshot.Take(entity, entityType)); } + + //an embedded database has no round trip to save, so each record is read on its own + public async Task>> ReadMany(Type entityType, + IReadOnlyCollection values, string key = null, string field = null) + { + var records = new Dictionary>(); + foreach (var value in values.Distinct()) + if (await Read(entityType, value, key, field) is { } record) + records[value] = record; + return records; + } } diff --git a/src/Core/Grand.Data/Mongo/MongoAuditRecordReader.cs b/src/Core/Grand.Data/Mongo/MongoAuditRecordReader.cs index 8e5580609..59aa20bce 100644 --- a/src/Core/Grand.Data/Mongo/MongoAuditRecordReader.cs +++ b/src/Core/Grand.Data/Mongo/MongoAuditRecordReader.cs @@ -13,12 +13,36 @@ public class MongoAuditRecordReader(Func database) : IAuditRecor public async Task> Read(Type entityType, string value, string key = null, string field = null) { - //a key other than the id may hold a number, stored as one - BsonValue match = key != null && int.TryParse(value, out var number) ? number : value; - var find = database().GetCollection(entityType.Name).Find(new BsonDocument(key ?? "_id", match)); + var find = database().GetCollection(entityType.Name) + .Find(new BsonDocument(key ?? "_id", Match(value, key))); var document = field == null ? await find.FirstOrDefaultAsync() : await find.Project(Builders.Projection.Include(field)).FirstOrDefaultAsync(); return AuditSnapshot.Flatten(document, entityType == typeof(Setting)); } + + public async Task>> ReadMany(Type entityType, + IReadOnlyCollection values, string key = null, string field = null) + { + var name = key ?? "_id"; + var find = database().GetCollection(entityType.Name) + .Find(Builders.Filter.In(name, values.Select(v => Match(v, key)))); + //the key stays in the projection: it tells which value a record answers (the id always does) + var documents = field == null + ? await find.ToListAsync() + : await find.Project(key == null || key == field + ? Builders.Projection.Include(field) + : Builders.Projection.Include(field).Include(key)).ToListAsync(); + + var records = new Dictionary>(); + foreach (var document in documents) + records.TryAdd(document[name].ToString()!, AuditSnapshot.Flatten(document, entityType == typeof(Setting))); + return records; + } + + //a key other than the id may hold a number, stored as one + private static BsonValue Match(string value, string key) + { + return key != null && int.TryParse(value, out var number) ? number : value; + } } diff --git a/src/Core/Grand.Infrastructure/Audit/AuditReferences.cs b/src/Core/Grand.Infrastructure/Audit/AuditReferences.cs index 60f7e9869..35f505b68 100644 --- a/src/Core/Grand.Infrastructure/Audit/AuditReferences.cs +++ b/src/Core/Grand.Infrastructure/Audit/AuditReferences.cs @@ -17,8 +17,11 @@ public partial class AuditReferences(IAuditRecordReader reader) private static readonly ConcurrentDictionary<(Type, string), PropertyInfo> Properties = new(); private static readonly ConcurrentDictionary Owners = new(); - private readonly Dictionary<(Type Target, string List, string Key, string Owner, string Id), string> _names = new(); - private readonly Dictionary<(Type Target, string Id), string> _ownerStores = new(); + //the records read for these entries, null when there is none + private readonly Dictionary> _records = new(); + + //set while Prefetch walks the entries: the records they need are noted instead of read + private HashSet _wanted; [GeneratedRegex(@"\[[^\]]*\]")] private static partial Regex ElementKeys(); @@ -32,6 +35,56 @@ public static string NameProperty(Type type) t.GetProperties().FirstOrDefault(p => p.IsDefined(typeof(AuditNameAttribute)))?.Name ?? "Name"); } + /// + /// Reads every record the entries refer to with one query per kind of record instead of one per id, so + /// describing them afterwards reads nothing. A record that could not be read this way is read when needed. + /// + public async Task Prefetch( + IEnumerable<(Type Type, AuditEntry Entry, IDictionary Before, IDictionary After)> entries) + { + var wanted = _wanted = []; + try + { + foreach (var (type, entry, before, after) in entries) + try + { + await DescribeEntry(type, entry, before, after); + } + catch (Exception) + { + //fails again when described + } + } + finally + { + _wanted = null; + } + + foreach (var group in wanted.Where(k => !_records.ContainsKey(k)).GroupBy(k => (k.Target, k.Key, k.Field))) + try + { + var read = await reader.ReadMany(group.Key.Target, group.Select(k => k.Value).ToList(), group.Key.Key, + group.Key.Field); + foreach (var key in group) + _records[key] = read.TryGetValue(key.Value, out var record) ? record : null; + } + catch (Exception) + { + //read one by one when described + } + } + + /// + /// Names the list elements, the references and the owner of one entry + /// + public async Task DescribeEntry(Type entityType, AuditEntry entry, IDictionary before, + IDictionary after) + { + await NameElements(entityType, entry.Changes, before, after); + await Describe(entityType, entry.Changes, before, after); + await LinkOwner(entityType, entry, after ?? before); + } + public async Task Describe(Type entityType, IEnumerable changes, IDictionary before, IDictionary after) { @@ -45,9 +98,12 @@ public async Task Describe(Type entityType, IEnumerable changes, ID //a name that cannot be found leaves the ids, never the change out try { - var described = AuditDiff.Change(change.Path, - await Describe(property, change.Path, change.OldValue, before), - await Describe(property, change.Path, change.NewValue, after)); + var oldValue = await Describe(property, change.Path, change.OldValue, before); + var newValue = await Describe(property, change.Path, change.NewValue, after); + if (_wanted != null) + continue; + + var described = AuditDiff.Change(change.Path, oldValue, newValue); change.OldValue = described.OldValue; change.NewValue = described.NewValue; change.Truncated = described.Truncated; @@ -93,7 +149,7 @@ public async Task NameElements(Type entityType, IEnumerable changes type = elementType ?? property.PropertyType; } - if (names.Any(n => n != "")) + if (_wanted == null && names.Any(n => n != "")) change.ElementNames = names; } catch (Exception) @@ -121,7 +177,7 @@ private async Task ElementName(Type elementType, string path, IDictionar return value; var owner = reference.Owner == null ? null : SiblingValue(snapshot, field, reference.Owner); - return await NameOrNull(reference, owner, value); + return await NameOf(reference, owner, value); } /// @@ -137,22 +193,17 @@ public async Task LinkOwner(Type entityType, AuditEntry entry, IDictionary()!.Target; - entry.OwnerType = target.Name; - entry.OwnerId = ownerId; + if (_wanted == null) + (entry.OwnerType, entry.OwnerId) = (target.Name, ownerId); if (entry.EntityStoreId != null) return; //a store that cannot be read leaves the entry without one, never the entry out try { - if (!_ownerStores.TryGetValue((target, ownerId), out var storeId)) - { - var record = await reader.Read(target, ownerId, null, "StoreId"); - _ownerStores[(target, ownerId)] = storeId = - record != null && record.TryGetValue("StoreId", out var stored) ? stored : null; - } - - entry.EntityStoreId = storeId; + var record = await Record(target, ownerId, null, "StoreId"); + if (_wanted == null) + entry.EntityStoreId = record != null && record.TryGetValue("StoreId", out var stored) ? stored : null; } catch (Exception) { @@ -242,23 +293,11 @@ private static string SiblingValue(IDictionary snapshot, string private async Task Label(AuditReferenceAttribute reference, string owner, string id) { - var name = await NameOrNull(reference, owner, id); + var name = await NameOf(reference, owner, id); return name == null ? id : $"{name} ({id})"; } - private async Task NameOrNull(AuditReferenceAttribute reference, string owner, string id) - { - var key = (reference.Target, reference.List, reference.Key, owner, id); - if (!_names.TryGetValue(key, out var name)) - { - name = await NameOf(reference, owner, id); - _names[key] = name; - } - - return string.IsNullOrEmpty(name) ? null : name; - } - - //the name in the snapshot of the referenced record, or of the element of its list + //the name in the snapshot of the referenced record, or of the element of its list; null when there is none private async Task NameOf(AuditReferenceAttribute reference, string owner, string id) { var recordId = reference.List == null ? id : owner; @@ -267,11 +306,28 @@ private async Task NameOf(AuditReferenceAttribute reference, string owne //only the field holding the name is read, not the whole record var record = reference.List == null - ? await reader.Read(reference.Target, recordId, reference.Key, NameProperty(reference.Target)) - : await reader.Read(reference.Target, recordId, null, reference.List); + ? await Record(reference.Target, recordId, reference.Key, NameProperty(reference.Target)) + : await Record(reference.Target, recordId, null, reference.List); var path = reference.List == null ? NameProperty(reference.Target) : $"{reference.List}[id={id}].{NameProperty(ElementType(reference.Target.GetProperty(reference.List)!.PropertyType)!)}"; return record != null && record.TryGetValue(path, out var name) && name != "" ? name : null; } + + //each record is read once for all the entries; while Prefetch collects, it is only noted + private async Task> Record(Type target, string value, string key, string field) + { + var recordKey = new RecordKey(target, key, field, value); + if (_wanted != null) + { + _wanted.Add(recordKey); + return null; + } + + if (!_records.TryGetValue(recordKey, out var record)) + _records[recordKey] = record = await reader.Read(target, value, key, field); + return record; + } + + private readonly record struct RecordKey(Type Target, string Key, string Field, string Value); } diff --git a/src/Core/Grand.Infrastructure/Audit/AuditTracker.cs b/src/Core/Grand.Infrastructure/Audit/AuditTracker.cs index e190ab311..4cedceb6a 100644 --- a/src/Core/Grand.Infrastructure/Audit/AuditTracker.cs +++ b/src/Core/Grand.Infrastructure/Audit/AuditTracker.cs @@ -270,7 +270,9 @@ private static async Task> BuildEntries( Dictionary, Task>>>> loaders, AuditReferences references) { - var entries = new List(); + //an entry of null: its changes could not be computed + var built = new List<(Type Type, string Id, AuditEntry Entry, IDictionary Before, + IDictionary After)>(); foreach (var group in pending.GroupBy(p => p.Key.Type)) { var toRead = group.Where(p => p.Value.AfterHint == null).Select(p => p.Key.Id).ToList(); @@ -281,32 +283,44 @@ private static async Task> BuildEntries( foreach (var item in group) { var after = item.Value.AfterHint ?? (read.TryGetValue(item.Key.Id, out var stored) ? stored : null); - AuditEntry entry; try { - entry = Build(group.Key, item.Key.Id, item.Value, after); + var entry = Build(group.Key, item.Key.Id, item.Value, after); if (entry != null) - { - await references.NameElements(group.Key, entry.Changes, item.Value.Before, after); - await references.Describe(group.Key, entry.Changes, item.Value.Before, after); - await references.LinkOwner(group.Key, entry, after ?? item.Value.Before); - } + built.Add((group.Key, item.Key.Id, entry, item.Value.Before, after)); } catch (Exception) { - entry = new AuditEntry { - Kind = after == null ? AuditKind.Delete : AuditKind.Update, - EntityType = group.Key.Name, - EntityId = item.Key.Id, - Details = "changes could not be computed" - }; + built.Add((group.Key, item.Key.Id, null, item.Value.Before, after)); } - - if (entry != null) - entries.Add(entry); } } + //the names of all entries in a few reads, instead of one read per referenced record + await references.Prefetch(built.Where(b => b.Entry != null).Select(b => (b.Type, b.Entry, b.Before, b.After))); + + var entries = new List(); + foreach (var item in built) + { + var entry = item.Entry; + if (entry != null) + try + { + await references.DescribeEntry(item.Type, entry, item.Before, item.After); + } + catch (Exception) + { + entry = null; + } + + entries.Add(entry ?? new AuditEntry { + Kind = item.After == null ? AuditKind.Delete : AuditKind.Update, + EntityType = item.Type.Name, + EntityId = item.Id, + Details = "changes could not be computed" + }); + } + return entries; } diff --git a/src/Tests/Grand.Infrastructure.Tests/Audit/AuditReferencesTests.cs b/src/Tests/Grand.Infrastructure.Tests/Audit/AuditReferencesTests.cs index 39cc58a55..515f25f15 100644 --- a/src/Tests/Grand.Infrastructure.Tests/Audit/AuditReferencesTests.cs +++ b/src/Tests/Grand.Infrastructure.Tests/Audit/AuditReferencesTests.cs @@ -282,4 +282,100 @@ public async Task UnmarkedOrMissing_KeepsTheStoredValue() Assert.AreEqual("b", unmarked.NewValue); Assert.AreEqual("b9", missing.NewValue); } + + private void ReadManyFromRecords() + { + _reader.Setup(r => r.ReadMany(It.IsAny(), It.IsAny>(), It.IsAny(), + It.IsAny())) + .Returns, string, string>((type, values, _, _) => + Task.FromResult>>(values + .Where(v => _records.ContainsKey((type, v))).ToDictionary(v => v, v => _records[(type, v)]))); + } + + private async Task DescribeAll( + List<(Type Type, AuditEntry Entry, IDictionary Before, IDictionary After)> entries) + { + foreach (var (type, entry, before, after) in entries) + await _references.DescribeEntry(type, entry, before, after); + } + + [TestMethod] + public async Task Prefetch_OneReadPerKindOfRecord_ThenDescribingReadsNothing() + { + _records[(typeof(Brand), "b1")] = new Dictionary { ["Name"] = "Aurora" }; + _records[(typeof(Brand), "b2")] = new Dictionary { ["Name"] = "Nordic" }; + _records[(typeof(Category), "c1")] = new Dictionary { ["Name"] = "Lamps" }; + ReadManyFromRecords(); + var first = new AuditEntry { Changes = [new AuditChange { Path = "BrandId", OldValue = "b1", NewValue = "b2" }] }; + var second = new AuditEntry { + Changes = [ + new AuditChange { Path = "BrandId", NewValue = "b1" }, + new AuditChange { Path = "ProductCategories[id=x].DisplayOrder", NewValue = "2" } + ] + }; + var product = new Dictionary { ["ProductCategories[id=x].CategoryId"] = "c1" }; + var entries = new List<(Type, AuditEntry, IDictionary, IDictionary)> { + (typeof(Product), first, null, null), + (typeof(Product), second, null, product) + }; + + await _references.Prefetch(entries); + + //collecting changes nothing + Assert.AreEqual("b1", first.Changes[0].OldValue); + Assert.IsNull(second.Changes[1].ElementNames); + + await DescribeAll(entries); + + Assert.AreEqual("Aurora (b1)", first.Changes[0].OldValue); + Assert.AreEqual("Nordic (b2)", first.Changes[0].NewValue); + Assert.AreEqual("Aurora (b1)", second.Changes[0].NewValue); + CollectionAssert.AreEqual(new[] { "Lamps" }, second.Changes[1].ElementNames.ToArray()); + _reader.Verify(r => r.ReadMany(typeof(Brand), It.Is>(v => v.Count == 2), null, + "Name"), Times.Once); + _reader.Verify(r => r.ReadMany(typeof(Category), It.IsAny>(), null, "Name"), + Times.Once); + _reader.Verify(r => r.Read(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + + [TestMethod] + public async Task Prefetch_OwnerStoresReadTogether_LinkedWhenDescribed() + { + _records[(typeof(MerchandiseReturn), "r1")] = new Dictionary { ["StoreId"] = "s1" }; + _records[(typeof(MerchandiseReturn), "r2")] = new Dictionary { ["StoreId"] = "s2" }; + ReadManyFromRecords(); + var entries = new List<(Type, AuditEntry, IDictionary, IDictionary)>(); + foreach (var id in new[] { "r1", "r2" }) + entries.Add((typeof(MerchandiseReturnNote), new AuditEntry(), null, + new Dictionary { ["MerchandiseReturnId"] = id })); + + await _references.Prefetch(entries); + Assert.IsNull(entries[0].Item2.OwnerId); + await DescribeAll(entries); + + Assert.AreEqual("r1", entries[0].Item2.OwnerId); + Assert.AreEqual("s1", entries[0].Item2.EntityStoreId); + Assert.AreEqual("s2", entries[1].Item2.EntityStoreId); + _reader.Verify(r => r.ReadMany(typeof(MerchandiseReturn), It.Is>(v => v.Count == 2), + null, "StoreId"), Times.Once); + _reader.Verify(r => r.Read(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + + [TestMethod] + public async Task Prefetch_ReadFails_NamesReadOneByOne() + { + _records[(typeof(Brand), "b1")] = new Dictionary { ["Name"] = "Aurora" }; + _reader.Setup(r => r.ReadMany(It.IsAny(), It.IsAny>(), It.IsAny(), + It.IsAny())).ThrowsAsync(new InvalidOperationException()); + var entries = new List<(Type, AuditEntry, IDictionary, IDictionary)> { + (typeof(Product), new AuditEntry { Changes = [new AuditChange { Path = "BrandId", NewValue = "b1" }] }, null, null) + }; + + await _references.Prefetch(entries); + await DescribeAll(entries); + + Assert.AreEqual("Aurora (b1)", entries[0].Item2.Changes[0].NewValue); + } } diff --git a/src/Tests/Grand.Infrastructure.Tests/Audit/AuditTrackerTests.cs b/src/Tests/Grand.Infrastructure.Tests/Audit/AuditTrackerTests.cs index e31c88545..df7aa40e2 100644 --- a/src/Tests/Grand.Infrastructure.Tests/Audit/AuditTrackerTests.cs +++ b/src/Tests/Grand.Infrastructure.Tests/Audit/AuditTrackerTests.cs @@ -220,6 +220,21 @@ public async Task Insert_NoBeforeRead_KindInsert() Assert.IsTrue(entry.Changes.Any(c => c.Path == "Name" && c.NewValue == "Shoes")); } + [TestMethod] + public async Task Insert_ThroughTheRepository_NotReadBack() + { + _tracker.Activate(Actor); + var thing = Put("1"); + + await new RepositoryAudit(_tracker, _access).Inserted(thing); + await _tracker.FlushAsync(); + + Assert.AreEqual(0, _loadCalls); + var entry = Written.Single(); + Assert.AreEqual(AuditKind.Insert, entry.Kind); + Assert.IsTrue(entry.Changes.Any(c => c.Path == "Name" && c.NewValue == "Shoes")); + } + [TestMethod] public async Task Delete_KindDelete() { diff --git a/src/Tests/Grand.Infrastructure.Tests/Audit/AuditingLiteDBRepositoryTests.cs b/src/Tests/Grand.Infrastructure.Tests/Audit/AuditingLiteDBRepositoryTests.cs index 0a6c70518..296d2db26 100644 --- a/src/Tests/Grand.Infrastructure.Tests/Audit/AuditingLiteDBRepositoryTests.cs +++ b/src/Tests/Grand.Infrastructure.Tests/Audit/AuditingLiteDBRepositoryTests.cs @@ -188,6 +188,21 @@ public async Task ExcludedType_NoEntry() Assert.AreEqual(0, _written.Count); } + [TestMethod] + public async Task RecordReader_ReadMany_ByIdWithoutMissing() + { + var lamp = await Seed(); + var desk = new Gadget { Name = "Desk" }; + await _repository.InsertAsync(desk); + + var records = await new LiteDBAuditRecordReader(() => _database) + .ReadMany(typeof(Gadget), [lamp.Id, desk.Id, "missing"], null, "Name"); + + Assert.AreEqual(2, records.Count); + Assert.AreEqual("Lamp", records[lamp.Id]["Name"]); + Assert.AreEqual("Desk", records[desk.Id]["Name"]); + } + public class Gadget : BaseEntity { public string Name { get; set; }