From fdd1a4b88696ba6494e4940f733ae097b94c5c63 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 21 May 2026 03:51:51 -0400 Subject: [PATCH] refactor(auditlog): consolidate AuditEvent DTO mappers into Communication --- .../Telemetry/ClusterClientSiteAuditClient.cs | 7 +- .../Site/Telemetry/SiteAuditTelemetryActor.cs | 3 +- .../Grpc/AuditEventDtoMapper.cs} | 20 ++- .../Grpc/SiteStreamGrpcServer.cs | 123 +----------------- .../CombinedTelemetryIdempotencyTests.cs | 3 +- .../CombinedTelemetryDispatcher.cs | 3 +- .../DirectActorSiteStreamAuditClient.cs | 7 +- .../ClusterClientSiteAuditClientTests.cs | 7 +- .../AuditEventDtoMapperTests.cs} | 25 ++-- 9 files changed, 45 insertions(+), 153 deletions(-) rename src/{ScadaLink.AuditLog/Telemetry/AuditEventMapper.cs => ScadaLink.Communication/Grpc/AuditEventDtoMapper.cs} (84%) rename tests/{ScadaLink.AuditLog.Tests/Telemetry/AuditEventMapperTests.cs => ScadaLink.Communication.Tests/AuditEventDtoMapperTests.cs} (92%) diff --git a/src/ScadaLink.AuditLog/Site/Telemetry/ClusterClientSiteAuditClient.cs b/src/ScadaLink.AuditLog/Site/Telemetry/ClusterClientSiteAuditClient.cs index dcf2fc4..2bf5f43 100644 --- a/src/ScadaLink.AuditLog/Site/Telemetry/ClusterClientSiteAuditClient.cs +++ b/src/ScadaLink.AuditLog/Site/Telemetry/ClusterClientSiteAuditClient.cs @@ -1,5 +1,4 @@ using Akka.Actor; -using ScadaLink.AuditLog.Telemetry; using ScadaLink.Commons.Entities.Audit; using ScadaLink.Commons.Messages.Audit; using ScadaLink.Commons.Types; @@ -35,7 +34,7 @@ namespace ScadaLink.AuditLog.Site.Telemetry; /// The batches arrive as proto DTOs ( / /// ) because the /// builds them with -/// . This client converts them back into +/// . This client converts them back into /// the / entities the Akka /// command messages carry — the same DTO→entity translation the /// SiteStreamGrpcServer performs for the gRPC reconciliation path. @@ -71,7 +70,7 @@ public sealed class ClusterClientSiteAuditClient : ISiteStreamAuditClient var events = new List(batch.Events.Count); foreach (var dto in batch.Events) { - events.Add(AuditEventMapper.FromDto(dto)); + events.Add(AuditEventDtoMapper.FromDto(dto)); } // Ask throws AskTimeoutException on timeout and rethrows a @@ -92,7 +91,7 @@ public sealed class ClusterClientSiteAuditClient : ISiteStreamAuditClient var entries = new List(batch.Packets.Count); foreach (var packet in batch.Packets) { - var audit = AuditEventMapper.FromDto(packet.AuditEvent); + var audit = AuditEventDtoMapper.FromDto(packet.AuditEvent); var siteCall = MapSiteCall(packet.Operational); entries.Add(new CachedTelemetryEntry(audit, siteCall)); } diff --git a/src/ScadaLink.AuditLog/Site/Telemetry/SiteAuditTelemetryActor.cs b/src/ScadaLink.AuditLog/Site/Telemetry/SiteAuditTelemetryActor.cs index 724e1d1..e903e0a 100644 --- a/src/ScadaLink.AuditLog/Site/Telemetry/SiteAuditTelemetryActor.cs +++ b/src/ScadaLink.AuditLog/Site/Telemetry/SiteAuditTelemetryActor.cs @@ -1,7 +1,6 @@ using Akka.Actor; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using ScadaLink.AuditLog.Telemetry; using ScadaLink.Commons.Entities.Audit; using ScadaLink.Commons.Interfaces.Services; using ScadaLink.Communication.Grpc; @@ -136,7 +135,7 @@ public class SiteAuditTelemetryActor : ReceiveActor var batch = new AuditEventBatch(); foreach (var e in events) { - batch.Events.Add(AuditEventMapper.ToDto(e)); + batch.Events.Add(AuditEventDtoMapper.ToDto(e)); } return batch; } diff --git a/src/ScadaLink.AuditLog/Telemetry/AuditEventMapper.cs b/src/ScadaLink.Communication/Grpc/AuditEventDtoMapper.cs similarity index 84% rename from src/ScadaLink.AuditLog/Telemetry/AuditEventMapper.cs rename to src/ScadaLink.Communication/Grpc/AuditEventDtoMapper.cs index d821db0..ed5fb22 100644 --- a/src/ScadaLink.AuditLog/Telemetry/AuditEventMapper.cs +++ b/src/ScadaLink.Communication/Grpc/AuditEventDtoMapper.cs @@ -1,16 +1,24 @@ using ScadaLink.Commons.Entities.Audit; using ScadaLink.Commons.Types.Enums; -using ScadaLink.Communication.Grpc; using Timestamp = Google.Protobuf.WellKnownTypes.Timestamp; -namespace ScadaLink.AuditLog.Telemetry; +namespace ScadaLink.Communication.Grpc; /// -/// Bridges Audit Log (#23) rows between the in-process record -/// and the wire-format exchanged over the -/// IngestAuditEvents RPC. +/// Canonical bridge for Audit Log (#23) rows between the in-process +/// record and the wire-format +/// exchanged over the IngestAuditEvents, IngestCachedTelemetry and +/// PullAuditEvents RPCs. /// /// +/// +/// This mapper lives in ScadaLink.Communication (which owns the generated +/// and references Commons for +/// ) so both SiteStreamGrpcServer and +/// ScadaLink.AuditLog can share one implementation without the +/// project-reference cycle that would result from hosting it in +/// ScadaLink.AuditLog (AuditLog → Communication, never the reverse). +/// /// Lossy by design: the proto contract intentionally omits two fields. /// /// — site-local SQLite state, never travels. @@ -22,7 +30,7 @@ namespace ScadaLink.AuditLog.Telemetry; /// Int32Value wrapper so they preserve true null semantics. /// /// -public static class AuditEventMapper +public static class AuditEventDtoMapper { /// /// Projects an into its wire-format DTO. Null reference diff --git a/src/ScadaLink.Communication/Grpc/SiteStreamGrpcServer.cs b/src/ScadaLink.Communication/Grpc/SiteStreamGrpcServer.cs index 8a92027..23a19d8 100644 --- a/src/ScadaLink.Communication/Grpc/SiteStreamGrpcServer.cs +++ b/src/ScadaLink.Communication/Grpc/SiteStreamGrpcServer.cs @@ -8,7 +8,6 @@ using ScadaLink.Commons.Entities.Audit; using ScadaLink.Commons.Interfaces.Services; using ScadaLink.Commons.Messages.Audit; using ScadaLink.Commons.Types; -using ScadaLink.Commons.Types.Enums; using GrpcStatus = Grpc.Core.Status; namespace ScadaLink.Communication.Grpc; @@ -224,13 +223,10 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase /// /// /// - /// The DTO→entity conversion is inlined here (rather than calling the - /// AuditLog mapper) to avoid a project-reference cycle: - /// ScadaLink.AuditLog already references - /// ScadaLink.Communication, so the gRPC server cannot reach back - /// into AuditLog for its mapper. The shape mirrors - /// AuditEventMapper.FromDto in ScadaLink.AuditLog.Telemetry; - /// the two must evolve together. + /// The DTO→entity conversion uses the shared + /// (hosted in ScadaLink.Communication so both this server and + /// ScadaLink.AuditLog share one implementation without a + /// project-reference cycle). /// /// /// When is not yet wired (host startup @@ -262,36 +258,10 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase return new IngestAck(); } - // Inlined FromDto. Keep in sync with AuditEventMapper.FromDto in - // ScadaLink.AuditLog.Telemetry — there is no shared mapper because - // doing so would create a project-reference cycle (AuditLog → Communication). var entities = new List(request.Events.Count); foreach (var dto in request.Events) { - entities.Add(new AuditEvent - { - EventId = Guid.Parse(dto.EventId), - OccurredAtUtc = DateTime.SpecifyKind(dto.OccurredAtUtc.ToDateTime(), DateTimeKind.Utc), - IngestedAtUtc = null, - Channel = Enum.Parse(dto.Channel), - Kind = Enum.Parse(dto.Kind), - CorrelationId = string.IsNullOrEmpty(dto.CorrelationId) ? null : Guid.Parse(dto.CorrelationId), - SourceSiteId = NullIfEmpty(dto.SourceSiteId), - SourceInstanceId = NullIfEmpty(dto.SourceInstanceId), - SourceScript = NullIfEmpty(dto.SourceScript), - Actor = NullIfEmpty(dto.Actor), - Target = NullIfEmpty(dto.Target), - Status = Enum.Parse(dto.Status), - HttpStatus = dto.HttpStatus, - DurationMs = dto.DurationMs, - ErrorMessage = NullIfEmpty(dto.ErrorMessage), - ErrorDetail = NullIfEmpty(dto.ErrorDetail), - RequestSummary = NullIfEmpty(dto.RequestSummary), - ResponseSummary = NullIfEmpty(dto.ResponseSummary), - PayloadTruncated = dto.PayloadTruncated, - Extra = NullIfEmpty(dto.Extra), - ForwardState = null, - }); + entities.Add(AuditEventDtoMapper.FromDto(dto)); } var cmd = new IngestAuditEventsCommand(entities); @@ -355,7 +325,7 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase var entries = new List(request.Packets.Count); foreach (var packet in request.Packets) { - var auditEvent = MapAuditEventFromDto(packet.AuditEvent); + var auditEvent = AuditEventDtoMapper.FromDto(packet.AuditEvent); var siteCall = MapSiteCallFromDto(packet.Operational); entries.Add(new CachedTelemetryEntry(auditEvent, siteCall)); } @@ -450,7 +420,7 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase }; foreach (var evt in events) { - response.Events.Add(AuditEventToDto(evt)); + response.Events.Add(AuditEventDtoMapper.ToDto(evt)); } // Flip to Reconciled AFTER projecting the response so a fault below the @@ -481,85 +451,6 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase return response; } - /// - /// Inlined audit-event entity→DTO translation. Keep in sync with - /// AuditEventMapper.ToDto in ScadaLink.AuditLog.Telemetry — - /// the project-reference cycle (AuditLog → Communication) prevents calling - /// the AuditLog mapper directly. The shape mirrors the FromDto pair above. - /// - private static AuditEventDto AuditEventToDto(AuditEvent evt) - { - var dto = new AuditEventDto - { - EventId = evt.EventId.ToString(), - OccurredAtUtc = Google.Protobuf.WellKnownTypes.Timestamp.FromDateTime(EnsureUtc(evt.OccurredAtUtc)), - Channel = evt.Channel.ToString(), - Kind = evt.Kind.ToString(), - CorrelationId = evt.CorrelationId?.ToString() ?? string.Empty, - SourceSiteId = evt.SourceSiteId ?? string.Empty, - SourceInstanceId = evt.SourceInstanceId ?? string.Empty, - SourceScript = evt.SourceScript ?? string.Empty, - Actor = evt.Actor ?? string.Empty, - Target = evt.Target ?? string.Empty, - Status = evt.Status.ToString(), - ErrorMessage = evt.ErrorMessage ?? string.Empty, - ErrorDetail = evt.ErrorDetail ?? string.Empty, - RequestSummary = evt.RequestSummary ?? string.Empty, - ResponseSummary = evt.ResponseSummary ?? string.Empty, - PayloadTruncated = evt.PayloadTruncated, - Extra = evt.Extra ?? string.Empty, - }; - - if (evt.HttpStatus.HasValue) dto.HttpStatus = evt.HttpStatus.Value; - if (evt.DurationMs.HasValue) dto.DurationMs = evt.DurationMs.Value; - - return dto; - } - - private static DateTime EnsureUtc(DateTime value) => - value.Kind == DateTimeKind.Utc - ? value - : DateTime.SpecifyKind(value.ToUniversalTime(), DateTimeKind.Utc); - - private static string? NullIfEmpty(string? value) => - string.IsNullOrEmpty(value) ? null : value; - - /// - /// Inlined audit-event DTO→entity translation, kept in sync with the - /// handler above. Extracted to a private - /// helper so the M3 dual-write RPC can reuse it without duplicating yet - /// another copy. The shape still mirrors - /// AuditEventMapper.FromDto in ScadaLink.AuditLog.Telemetry; - /// the two must evolve together (the project-reference cycle that - /// prevents calling the AuditLog mapper directly is documented on - /// ). - /// - private static AuditEvent MapAuditEventFromDto(AuditEventDto dto) => - new() - { - EventId = Guid.Parse(dto.EventId), - OccurredAtUtc = DateTime.SpecifyKind(dto.OccurredAtUtc.ToDateTime(), DateTimeKind.Utc), - IngestedAtUtc = null, - Channel = Enum.Parse(dto.Channel), - Kind = Enum.Parse(dto.Kind), - CorrelationId = NullIfEmpty(dto.CorrelationId) is { } cid ? Guid.Parse(cid) : null, - SourceSiteId = NullIfEmpty(dto.SourceSiteId), - SourceInstanceId = NullIfEmpty(dto.SourceInstanceId), - SourceScript = NullIfEmpty(dto.SourceScript), - Actor = NullIfEmpty(dto.Actor), - Target = NullIfEmpty(dto.Target), - Status = Enum.Parse(dto.Status), - HttpStatus = dto.HttpStatus, - DurationMs = dto.DurationMs, - ErrorMessage = NullIfEmpty(dto.ErrorMessage), - ErrorDetail = NullIfEmpty(dto.ErrorDetail), - RequestSummary = NullIfEmpty(dto.RequestSummary), - ResponseSummary = NullIfEmpty(dto.ResponseSummary), - PayloadTruncated = dto.PayloadTruncated, - Extra = NullIfEmpty(dto.Extra), - ForwardState = null, - }; - /// /// Translates a into the persistence /// entity. is stamped here as a diff --git a/tests/ScadaLink.AuditLog.Tests/Integration/CombinedTelemetryIdempotencyTests.cs b/tests/ScadaLink.AuditLog.Tests/Integration/CombinedTelemetryIdempotencyTests.cs index 1d67ed7..63d8574 100644 --- a/tests/ScadaLink.AuditLog.Tests/Integration/CombinedTelemetryIdempotencyTests.cs +++ b/tests/ScadaLink.AuditLog.Tests/Integration/CombinedTelemetryIdempotencyTests.cs @@ -1,7 +1,6 @@ using Akka.TestKit.Xunit2; using Microsoft.EntityFrameworkCore; using ScadaLink.AuditLog.Tests.Integration.Infrastructure; -using ScadaLink.AuditLog.Telemetry; using ScadaLink.Commons.Entities.Audit; using ScadaLink.Commons.Messages.Integration; using ScadaLink.Commons.Types; @@ -55,7 +54,7 @@ public class CombinedTelemetryIdempotencyTests : TestKit, IClassFixture(batch.Events.Count); foreach (var dto in batch.Events) { - events.Add(AuditEventMapper.FromDto(dto)); + events.Add(AuditEventDtoMapper.FromDto(dto)); } // Ask the central actor; the reply carries the accepted EventIds. @@ -114,7 +113,7 @@ public sealed class DirectActorSiteStreamAuditClient : ISiteStreamAuditClient /// back into the proto ack. /// /// - /// Uses the shared for the audit half; + /// Uses the shared for the audit half; /// the SiteCall DTO is decoded inline because the AuditLog mapper does not /// (and should not) know about — the /// production gRPC server (Bundle D) uses the same inline shape. @@ -132,7 +131,7 @@ public sealed class DirectActorSiteStreamAuditClient : ISiteStreamAuditClient var entries = new List(batch.Packets.Count); foreach (var packet in batch.Packets) { - var audit = AuditEventMapper.FromDto(packet.AuditEvent); + var audit = AuditEventDtoMapper.FromDto(packet.AuditEvent); var siteCall = MapSiteCallFromDto(packet.Operational); entries.Add(new CachedTelemetryEntry(audit, siteCall)); } diff --git a/tests/ScadaLink.AuditLog.Tests/Site/Telemetry/ClusterClientSiteAuditClientTests.cs b/tests/ScadaLink.AuditLog.Tests/Site/Telemetry/ClusterClientSiteAuditClientTests.cs index d9cbe82..b1005d3 100644 --- a/tests/ScadaLink.AuditLog.Tests/Site/Telemetry/ClusterClientSiteAuditClientTests.cs +++ b/tests/ScadaLink.AuditLog.Tests/Site/Telemetry/ClusterClientSiteAuditClientTests.cs @@ -2,7 +2,6 @@ using Akka.Actor; using Akka.TestKit.Xunit2; using Google.Protobuf.WellKnownTypes; using ScadaLink.AuditLog.Site.Telemetry; -using ScadaLink.AuditLog.Telemetry; using ScadaLink.Commons.Entities.Audit; using ScadaLink.Commons.Messages.Audit; using ScadaLink.Commons.Types.Enums; @@ -46,7 +45,7 @@ public class ClusterClientSiteAuditClientTests : TestKit var batch = new AuditEventBatch(); foreach (var e in events) { - batch.Events.Add(AuditEventMapper.ToDto(e)); + batch.Events.Add(AuditEventDtoMapper.ToDto(e)); } return batch; } @@ -158,7 +157,7 @@ public class ClusterClientSiteAuditClientTests : TestKit { batch.Packets.Add(new CachedTelemetryPacket { - AuditEvent = AuditEventMapper.ToDto(e), + AuditEvent = AuditEventDtoMapper.ToDto(e), Operational = NewOperationalDto(), }); } @@ -190,7 +189,7 @@ public class ClusterClientSiteAuditClientTests : TestKit var batch = new CachedTelemetryBatch(); batch.Packets.Add(new CachedTelemetryPacket { - AuditEvent = AuditEventMapper.ToDto(NewEvent()), + AuditEvent = AuditEventDtoMapper.ToDto(NewEvent()), Operational = NewOperationalDto(), }); diff --git a/tests/ScadaLink.AuditLog.Tests/Telemetry/AuditEventMapperTests.cs b/tests/ScadaLink.Communication.Tests/AuditEventDtoMapperTests.cs similarity index 92% rename from tests/ScadaLink.AuditLog.Tests/Telemetry/AuditEventMapperTests.cs rename to tests/ScadaLink.Communication.Tests/AuditEventDtoMapperTests.cs index 6901361..a247ec6 100644 --- a/tests/ScadaLink.AuditLog.Tests/Telemetry/AuditEventMapperTests.cs +++ b/tests/ScadaLink.Communication.Tests/AuditEventDtoMapperTests.cs @@ -1,18 +1,17 @@ using Google.Protobuf.WellKnownTypes; -using ScadaLink.AuditLog.Telemetry; using ScadaLink.Commons.Entities.Audit; using ScadaLink.Commons.Types.Enums; using ScadaLink.Communication.Grpc; -namespace ScadaLink.AuditLog.Tests.Telemetry; +namespace ScadaLink.Communication.Tests; /// -/// Round-trip + edge tests for the that bridges +/// Round-trip + edge tests for the that bridges /// (Commons) ↔ (proto). /// ForwardState is site-local and IngestedAtUtc is central-set, so neither survives /// the proto round-trip. /// -public class AuditEventMapperTests +public class AuditEventDtoMapperTests { [Fact] public void ToDto_FromDto_Roundtrip_FullyPopulated_PreservesAllFields() @@ -47,8 +46,8 @@ public class AuditEventMapperTests ForwardState = AuditForwardState.Pending }; - var dto = AuditEventMapper.ToDto(original); - var roundTripped = AuditEventMapper.FromDto(dto); + var dto = AuditEventDtoMapper.ToDto(original); + var roundTripped = AuditEventDtoMapper.FromDto(dto); Assert.Equal(original.EventId, roundTripped.EventId); Assert.Equal(original.OccurredAtUtc, roundTripped.OccurredAtUtc); @@ -88,7 +87,7 @@ public class AuditEventMapperTests // all string? fields left null; CorrelationId null }; - var dto = AuditEventMapper.ToDto(evt); + var dto = AuditEventDtoMapper.ToDto(evt); Assert.Equal(string.Empty, dto.CorrelationId); Assert.Equal(string.Empty, dto.SourceSiteId); @@ -126,7 +125,7 @@ public class AuditEventMapperTests Extra = string.Empty }; - var evt = AuditEventMapper.FromDto(dto); + var evt = AuditEventDtoMapper.FromDto(dto); Assert.Null(evt.CorrelationId); Assert.Null(evt.SourceSiteId); @@ -154,8 +153,8 @@ public class AuditEventMapperTests Status = AuditStatus.Delivered }; - var dto = AuditEventMapper.ToDto(evt); - var roundTripped = AuditEventMapper.FromDto(dto); + var dto = AuditEventDtoMapper.ToDto(evt); + var roundTripped = AuditEventDtoMapper.FromDto(dto); Assert.Equal(DateTimeKind.Utc, roundTripped.OccurredAtUtc.Kind); Assert.Equal(occurredAt, roundTripped.OccurredAtUtc); @@ -175,7 +174,7 @@ public class AuditEventMapperTests DurationMs = null }; - var dto = AuditEventMapper.ToDto(evt); + var dto = AuditEventDtoMapper.ToDto(evt); Assert.Null(dto.HttpStatus); Assert.Null(dto.DurationMs); @@ -197,7 +196,7 @@ public class AuditEventMapperTests Assert.Null(dto.HttpStatus); Assert.Null(dto.DurationMs); - var evt = AuditEventMapper.FromDto(dto); + var evt = AuditEventDtoMapper.FromDto(dto); Assert.Null(evt.HttpStatus); Assert.Null(evt.DurationMs); @@ -215,7 +214,7 @@ public class AuditEventMapperTests Status = AuditStatus.Parked }; - var dto = AuditEventMapper.ToDto(evt); + var dto = AuditEventDtoMapper.ToDto(evt); Assert.Equal("ApiOutbound", dto.Channel); Assert.Equal("ApiCallCached", dto.Kind);