refactor(auditlog): consolidate SiteCall DTO mapper into Communication

Extract the verbatim-duplicated SiteCallOperationalDto -> SiteCall mapper
into a single public SiteCallDtoMapper static class in
ScadaLink.Communication.Grpc, mirroring AuditEventDtoMapper. Replaces three
identical private copies (SiteStreamGrpcServer.MapSiteCallFromDto,
ClusterClientSiteAuditClient.MapSiteCall, and the test-infra
DirectActorSiteStreamAuditClient.MapSiteCallFromDto), removes the now-stale
doc comment that justified the duplication, and drops the using directives
that became unused. Adds SiteCallDtoMapperTests for field-by-field coverage.

Only the FromDto direction is provided: nothing maps SiteCall back onto the
wire, so a ToDto would be dead code.
This commit is contained in:
Joseph Doherty
2026-05-21 04:00:20 -04:00
parent fdd1a4b886
commit 6f0d2ca499
5 changed files with 211 additions and 86 deletions

View File

@@ -1,7 +1,6 @@
using Akka.Actor; using Akka.Actor;
using ScadaLink.Commons.Entities.Audit; using ScadaLink.Commons.Entities.Audit;
using ScadaLink.Commons.Messages.Audit; using ScadaLink.Commons.Messages.Audit;
using ScadaLink.Commons.Types;
using ScadaLink.Communication.Grpc; using ScadaLink.Communication.Grpc;
namespace ScadaLink.AuditLog.Site.Telemetry; namespace ScadaLink.AuditLog.Site.Telemetry;
@@ -92,7 +91,7 @@ public sealed class ClusterClientSiteAuditClient : ISiteStreamAuditClient
foreach (var packet in batch.Packets) foreach (var packet in batch.Packets)
{ {
var audit = AuditEventDtoMapper.FromDto(packet.AuditEvent); var audit = AuditEventDtoMapper.FromDto(packet.AuditEvent);
var siteCall = MapSiteCall(packet.Operational); var siteCall = SiteCallDtoMapper.FromDto(packet.Operational);
entries.Add(new CachedTelemetryEntry(audit, siteCall)); entries.Add(new CachedTelemetryEntry(audit, siteCall));
} }
@@ -115,31 +114,4 @@ public sealed class ClusterClientSiteAuditClient : ISiteStreamAuditClient
} }
return ack; return ack;
} }
/// <summary>
/// Translates a <see cref="SiteCallOperationalDto"/> into the
/// <see cref="SiteCall"/> persistence entity. Mirrors
/// <c>SiteStreamGrpcServer.MapSiteCallFromDto</c> — there is no shared
/// mapper because that lives in <c>ScadaLink.Communication</c> as a private
/// helper. <see cref="SiteCall.IngestedAtUtc"/> is a placeholder; the
/// central <c>AuditLogIngestActor</c> overwrites it inside the dual-write
/// transaction so the AuditLog and SiteCalls rows share one instant.
/// </summary>
private static SiteCall MapSiteCall(SiteCallOperationalDto dto) => new()
{
TrackedOperationId = TrackedOperationId.Parse(dto.TrackedOperationId),
Channel = dto.Channel,
Target = dto.Target,
SourceSite = dto.SourceSite,
Status = dto.Status,
RetryCount = dto.RetryCount,
LastError = string.IsNullOrEmpty(dto.LastError) ? null : dto.LastError,
HttpStatus = dto.HttpStatus,
CreatedAtUtc = DateTime.SpecifyKind(dto.CreatedAtUtc.ToDateTime(), DateTimeKind.Utc),
UpdatedAtUtc = DateTime.SpecifyKind(dto.UpdatedAtUtc.ToDateTime(), DateTimeKind.Utc),
TerminalAtUtc = dto.TerminalAtUtc is null
? null
: DateTime.SpecifyKind(dto.TerminalAtUtc.ToDateTime(), DateTimeKind.Utc),
IngestedAtUtc = DateTime.UtcNow, // overwritten by AuditLogIngestActor
};
} }

View File

@@ -0,0 +1,70 @@
using ScadaLink.Commons.Entities.Audit;
using ScadaLink.Commons.Types;
namespace ScadaLink.Communication.Grpc;
/// <summary>
/// Canonical bridge for Site Call Audit (#22) operational rows between the
/// wire-format <see cref="SiteCallOperationalDto"/> exchanged on the
/// <c>CachedCallTelemetry</c> packet and the in-process <see cref="SiteCall"/>
/// persistence entity central writes into the <c>SiteCalls</c> table.
/// </summary>
/// <remarks>
/// <para>
/// This mapper lives in <c>ScadaLink.Communication</c> (which owns the generated
/// <see cref="SiteCallOperationalDto"/> and references <c>Commons</c> for
/// <see cref="SiteCall"/>) so both <c>SiteStreamGrpcServer</c> and
/// <c>ScadaLink.AuditLog</c> can share one implementation without the
/// project-reference cycle that would result from hosting it in
/// <c>ScadaLink.AuditLog</c> (AuditLog → Communication, never the reverse).
/// Mirrors the sibling <see cref="AuditEventDtoMapper"/>.
/// </para>
/// <para>
/// Only the DTO→entity direction is provided: nothing in the system maps a
/// <see cref="SiteCall"/> back onto the wire (sites emit the operational state
/// from <c>SiteCallOperational</c>, never from the central <see cref="SiteCall"/>
/// entity), so an entity→DTO method would be dead code.
/// </para>
/// <para>
/// String nullability convention: proto3 scalar strings cannot be absent, so the
/// optional <see cref="SiteCall.LastError"/> rehydrates from an empty string back
/// to null. The optional <c>HttpStatus</c> and <c>TerminalAtUtc</c> use proto
/// wrappers so they preserve true null semantics.
/// </para>
/// </remarks>
public static class SiteCallDtoMapper
{
/// <summary>
/// Reconstructs a <see cref="SiteCall"/> persistence entity from its
/// wire-format DTO. An empty <c>LastError</c> rehydrates as null; absent
/// <c>HttpStatus</c>/<c>TerminalAtUtc</c> wrappers stay null.
/// </summary>
/// <remarks>
/// <see cref="SiteCall.IngestedAtUtc"/> is stamped here as a placeholder
/// (<see cref="DateTime.UtcNow"/>); the central ingest actor overwrites it
/// inside the dual-write transaction so the AuditLog and SiteCalls rows
/// share one instant. The value sent on the wire is informational only.
/// </remarks>
public static SiteCall FromDto(SiteCallOperationalDto dto)
{
ArgumentNullException.ThrowIfNull(dto);
return new SiteCall
{
TrackedOperationId = TrackedOperationId.Parse(dto.TrackedOperationId),
Channel = dto.Channel,
Target = dto.Target,
SourceSite = dto.SourceSite,
Status = dto.Status,
RetryCount = dto.RetryCount,
LastError = string.IsNullOrEmpty(dto.LastError) ? null : dto.LastError,
HttpStatus = dto.HttpStatus,
CreatedAtUtc = DateTime.SpecifyKind(dto.CreatedAtUtc.ToDateTime(), DateTimeKind.Utc),
UpdatedAtUtc = DateTime.SpecifyKind(dto.UpdatedAtUtc.ToDateTime(), DateTimeKind.Utc),
TerminalAtUtc = dto.TerminalAtUtc is null
? null
: DateTime.SpecifyKind(dto.TerminalAtUtc.ToDateTime(), DateTimeKind.Utc),
IngestedAtUtc = DateTime.UtcNow, // overwritten by AuditLogIngestActor
};
}
}

View File

@@ -7,7 +7,6 @@ using Microsoft.Extensions.Options;
using ScadaLink.Commons.Entities.Audit; using ScadaLink.Commons.Entities.Audit;
using ScadaLink.Commons.Interfaces.Services; using ScadaLink.Commons.Interfaces.Services;
using ScadaLink.Commons.Messages.Audit; using ScadaLink.Commons.Messages.Audit;
using ScadaLink.Commons.Types;
using GrpcStatus = Grpc.Core.Status; using GrpcStatus = Grpc.Core.Status;
namespace ScadaLink.Communication.Grpc; namespace ScadaLink.Communication.Grpc;
@@ -326,7 +325,7 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
foreach (var packet in request.Packets) foreach (var packet in request.Packets)
{ {
var auditEvent = AuditEventDtoMapper.FromDto(packet.AuditEvent); var auditEvent = AuditEventDtoMapper.FromDto(packet.AuditEvent);
var siteCall = MapSiteCallFromDto(packet.Operational); var siteCall = SiteCallDtoMapper.FromDto(packet.Operational);
entries.Add(new CachedTelemetryEntry(auditEvent, siteCall)); entries.Add(new CachedTelemetryEntry(auditEvent, siteCall));
} }
@@ -451,31 +450,6 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
return response; return response;
} }
/// <summary>
/// Translates a <see cref="SiteCallOperationalDto"/> into the persistence
/// entity. <see cref="SiteCall.IngestedAtUtc"/> is stamped here as a
/// placeholder; the central ingest actor overwrites it inside the
/// dual-write transaction so the AuditLog and SiteCalls rows share one
/// instant.
/// </summary>
private static SiteCall MapSiteCallFromDto(SiteCallOperationalDto dto) => new()
{
TrackedOperationId = TrackedOperationId.Parse(dto.TrackedOperationId),
Channel = dto.Channel,
Target = dto.Target,
SourceSite = dto.SourceSite,
Status = dto.Status,
RetryCount = dto.RetryCount,
LastError = string.IsNullOrEmpty(dto.LastError) ? null : dto.LastError,
HttpStatus = dto.HttpStatus,
CreatedAtUtc = DateTime.SpecifyKind(dto.CreatedAtUtc.ToDateTime(), DateTimeKind.Utc),
UpdatedAtUtc = DateTime.SpecifyKind(dto.UpdatedAtUtc.ToDateTime(), DateTimeKind.Utc),
TerminalAtUtc = dto.TerminalAtUtc is null
? null
: DateTime.SpecifyKind(dto.TerminalAtUtc.ToDateTime(), DateTimeKind.Utc),
IngestedAtUtc = DateTime.UtcNow, // overwritten by AuditLogIngestActor
};
/// <summary> /// <summary>
/// Tracks a single active stream so cleanup only removes its own entry. /// Tracks a single active stream so cleanup only removes its own entry.
/// </summary> /// </summary>

View File

@@ -2,7 +2,6 @@ using Akka.Actor;
using ScadaLink.AuditLog.Site.Telemetry; using ScadaLink.AuditLog.Site.Telemetry;
using ScadaLink.Commons.Entities.Audit; using ScadaLink.Commons.Entities.Audit;
using ScadaLink.Commons.Messages.Audit; using ScadaLink.Commons.Messages.Audit;
using ScadaLink.Commons.Types;
using ScadaLink.Communication.Grpc; using ScadaLink.Communication.Grpc;
namespace ScadaLink.AuditLog.Tests.Integration.Infrastructure; namespace ScadaLink.AuditLog.Tests.Integration.Infrastructure;
@@ -113,10 +112,9 @@ public sealed class DirectActorSiteStreamAuditClient : ISiteStreamAuditClient
/// back into the proto ack. /// back into the proto ack.
/// </summary> /// </summary>
/// <remarks> /// <remarks>
/// Uses the shared <see cref="AuditEventDtoMapper.FromDto"/> for the audit half; /// Uses the shared <see cref="AuditEventDtoMapper.FromDto"/> for the audit half
/// the SiteCall DTO is decoded inline because the AuditLog mapper does not /// and <see cref="SiteCallDtoMapper.FromDto"/> for the SiteCall half — the same
/// (and should not) know about <see cref="SiteCallOperationalDto"/> — the /// canonical mappers the production <c>SiteStreamGrpcServer</c> uses.
/// production gRPC server (Bundle D) uses the same inline shape.
/// </remarks> /// </remarks>
public async Task<IngestAck> IngestCachedTelemetryAsync(CachedTelemetryBatch batch, CancellationToken ct) public async Task<IngestAck> IngestCachedTelemetryAsync(CachedTelemetryBatch batch, CancellationToken ct)
{ {
@@ -132,7 +130,7 @@ public sealed class DirectActorSiteStreamAuditClient : ISiteStreamAuditClient
foreach (var packet in batch.Packets) foreach (var packet in batch.Packets)
{ {
var audit = AuditEventDtoMapper.FromDto(packet.AuditEvent); var audit = AuditEventDtoMapper.FromDto(packet.AuditEvent);
var siteCall = MapSiteCallFromDto(packet.Operational); var siteCall = SiteCallDtoMapper.FromDto(packet.Operational);
entries.Add(new CachedTelemetryEntry(audit, siteCall)); entries.Add(new CachedTelemetryEntry(audit, siteCall));
} }
@@ -149,28 +147,4 @@ public sealed class DirectActorSiteStreamAuditClient : ISiteStreamAuditClient
} }
return ack; return ack;
} }
/// <summary>
/// Mirrors <c>SiteStreamGrpcServer.MapSiteCallFromDto</c> — keep the two in
/// sync. The placeholder <see cref="SiteCall.IngestedAtUtc"/> stamped here
/// is overwritten by the central ingest actor inside the dual-write
/// transaction, so the value sent on the wire is informational only.
/// </summary>
private static SiteCall MapSiteCallFromDto(SiteCallOperationalDto dto) => new()
{
TrackedOperationId = TrackedOperationId.Parse(dto.TrackedOperationId),
Channel = dto.Channel,
Target = dto.Target,
SourceSite = dto.SourceSite,
Status = dto.Status,
RetryCount = dto.RetryCount,
LastError = string.IsNullOrEmpty(dto.LastError) ? null : dto.LastError,
HttpStatus = dto.HttpStatus,
CreatedAtUtc = DateTime.SpecifyKind(dto.CreatedAtUtc.ToDateTime(), DateTimeKind.Utc),
UpdatedAtUtc = DateTime.SpecifyKind(dto.UpdatedAtUtc.ToDateTime(), DateTimeKind.Utc),
TerminalAtUtc = dto.TerminalAtUtc is null
? null
: DateTime.SpecifyKind(dto.TerminalAtUtc.ToDateTime(), DateTimeKind.Utc),
IngestedAtUtc = DateTime.UtcNow,
};
} }

View File

@@ -0,0 +1,135 @@
using Google.Protobuf.WellKnownTypes;
using ScadaLink.Communication.Grpc;
namespace ScadaLink.Communication.Tests;
/// <summary>
/// Field-coverage + edge tests for the <see cref="SiteCallDtoMapper"/> that
/// decodes <see cref="SiteCallOperationalDto"/> (proto) into the
/// <see cref="ScadaLink.Commons.Entities.Audit.SiteCall"/> persistence entity.
/// Only the DTO→entity direction exists — nothing in the system maps a
/// <c>SiteCall</c> back onto the wire — so there is no round-trip test.
/// <c>IngestedAtUtc</c> is a site-side placeholder the central ingest actor
/// overwrites, so it is asserted as "recent UTC" rather than a fixed value.
/// </summary>
public class SiteCallDtoMapperTests
{
[Fact]
public void FromDto_FullyPopulated_MapsEveryField()
{
var trackedOperationId = Guid.NewGuid();
var createdAt = new DateTime(2026, 5, 20, 10, 0, 0, DateTimeKind.Utc);
var updatedAt = new DateTime(2026, 5, 20, 10, 5, 0, DateTimeKind.Utc);
var terminalAt = new DateTime(2026, 5, 20, 10, 10, 0, DateTimeKind.Utc);
var dto = new SiteCallOperationalDto
{
TrackedOperationId = trackedOperationId.ToString(),
Channel = "ApiOutbound",
Target = "ERP.GetOrder",
SourceSite = "site-melbourne",
Status = "Delivered",
RetryCount = 3,
LastError = "transient 503",
HttpStatus = 200,
CreatedAtUtc = Timestamp.FromDateTime(createdAt),
UpdatedAtUtc = Timestamp.FromDateTime(updatedAt),
TerminalAtUtc = Timestamp.FromDateTime(terminalAt),
};
var entity = SiteCallDtoMapper.FromDto(dto);
Assert.Equal(trackedOperationId, entity.TrackedOperationId.Value);
Assert.Equal("ApiOutbound", entity.Channel);
Assert.Equal("ERP.GetOrder", entity.Target);
Assert.Equal("site-melbourne", entity.SourceSite);
Assert.Equal("Delivered", entity.Status);
Assert.Equal(3, entity.RetryCount);
Assert.Equal("transient 503", entity.LastError);
Assert.Equal(200, entity.HttpStatus);
Assert.Equal(createdAt, entity.CreatedAtUtc);
Assert.Equal(updatedAt, entity.UpdatedAtUtc);
Assert.Equal(terminalAt, entity.TerminalAtUtc);
}
[Fact]
public void FromDto_EmptyLastError_BecomesNull()
{
var dto = NewMinimalDto();
dto.LastError = string.Empty;
var entity = SiteCallDtoMapper.FromDto(dto);
Assert.Null(entity.LastError);
}
[Fact]
public void FromDto_AbsentHttpStatus_StaysNull()
{
// Int32Value wrapper unset on the wire — preserves true null semantics
// for non-API cached writes.
var dto = NewMinimalDto();
Assert.Null(dto.HttpStatus);
var entity = SiteCallDtoMapper.FromDto(dto);
Assert.Null(entity.HttpStatus);
}
[Fact]
public void FromDto_AbsentTerminalAt_StaysNull()
{
// Timestamp wrapper unset while the call is still active.
var dto = NewMinimalDto();
Assert.Null(dto.TerminalAtUtc);
var entity = SiteCallDtoMapper.FromDto(dto);
Assert.Null(entity.TerminalAtUtc);
}
[Fact]
public void FromDto_Timestamps_RehydrateAsUtcKind()
{
var dto = NewMinimalDto();
var entity = SiteCallDtoMapper.FromDto(dto);
Assert.Equal(DateTimeKind.Utc, entity.CreatedAtUtc.Kind);
Assert.Equal(DateTimeKind.Utc, entity.UpdatedAtUtc.Kind);
}
[Fact]
public void FromDto_IngestedAtUtc_StampedAsRecentPlaceholder()
{
// IngestedAtUtc is a site-side DateTime.UtcNow placeholder; the central
// ingest actor overwrites it inside the dual-write transaction.
var before = DateTime.UtcNow;
var entity = SiteCallDtoMapper.FromDto(NewMinimalDto());
var after = DateTime.UtcNow;
Assert.InRange(entity.IngestedAtUtc, before, after);
Assert.Equal(DateTimeKind.Utc, entity.IngestedAtUtc.Kind);
}
[Fact]
public void FromDto_Null_Throws()
{
Assert.Throws<ArgumentNullException>(() => SiteCallDtoMapper.FromDto(null!));
}
private static SiteCallOperationalDto NewMinimalDto() => new()
{
TrackedOperationId = Guid.NewGuid().ToString(),
Channel = "DbOutbound",
Target = "warehouse.dbo.WriteOrder",
SourceSite = "site-brisbane",
Status = "Submitted",
RetryCount = 0,
CreatedAtUtc = Timestamp.FromDateTime(DateTime.UtcNow),
UpdatedAtUtc = Timestamp.FromDateTime(DateTime.UtcNow),
};
}