diff --git a/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs b/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs index 7e756fa..691bc8a 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs @@ -116,9 +116,12 @@ public sealed class MxAccessGatewayService( return bulkConstraintPlan.CreateDeniedReply(request); } - MxCommandRequest invokeRequest = request.Clone(); - invokeRequest.Command = commandToInvoke; - WorkerCommand workerCommand = mapper.MapCommand(invokeRequest); + // Map from the command alone: cloning the whole request only to overwrite its command with + // commandToInvoke deep-cloned the (potentially large) original payload for nothing, since + // MapCommand reads nothing but the command (GWC-29). The one clone that matters still + // happens inside MapCommand, which is what keeps the worker-bound graph unaliased from + // commandToInvoke — the caller still reads it below via TrackCommandReply. + WorkerCommand workerCommand = mapper.MapCommand(commandToInvoke); WorkerCommandReply workerReply = await sessionManager .InvokeAsync(request.SessionId, workerCommand, context.CancellationToken) .ConfigureAwait(false); diff --git a/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGrpcMapper.cs b/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGrpcMapper.cs index ea6d0f7..fa5e254 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGrpcMapper.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGrpcMapper.cs @@ -29,9 +29,27 @@ public sealed class MxAccessGrpcMapper ArgumentNullException.ThrowIfNull(request); ArgumentNullException.ThrowIfNull(request.Command); + return MapCommand(request.Command); + } + + /// + /// Maps a gRPC MX command to a worker command. Callers that already hold the command — including + /// the constraint pipeline, whose rewritten command is not the one on the request — use this + /// overload rather than cloning a whole request to carry a single field (GWC-29); nothing outside + /// the command is read here. + /// + /// Command payload. + /// The mapped ready for worker dispatch. + public WorkerCommand MapCommand(MxCommand command) + { + ArgumentNullException.ThrowIfNull(command); + + // The clone is required and must stay: the caller may hand us the gRPC-owned request command, + // and the caller keeps reading it after dispatch (TrackCommandReply). Cloning here is what makes + // WorkerClient.CreateCommandEnvelope's no-aliasing invariant true. return new WorkerCommand { - Command = request.Command.Clone(), + Command = command.Clone(), EnqueueTimestamp = Timestamp.FromDateTimeOffset(_timeProvider.GetUtcNow()), }; } diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGrpcMapperTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGrpcMapperTests.cs index b7249ba..4bda8ae 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGrpcMapperTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGrpcMapperTests.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.Time.Testing; using ZB.MOM.WW.MxGateway.Contracts.Proto; using ZB.MOM.WW.MxGateway.Server.Grpc; @@ -38,6 +39,48 @@ public sealed class MxAccessGrpcMapperTests Assert.NotNull(workerCommand.EnqueueTimestamp); } + /// + /// The command-only overload exists so Invoke does not deep-clone the whole request just to + /// overwrite and discard its command (GWC-29). It must still perform the one clone that keeps the + /// worker-bound graph unaliased from the caller-owned gRPC command, and must produce the same + /// as the request overload. + /// + [Fact] + public void MapCommandFromCommandClonesPayload() + { + FakeTimeProvider timeProvider = new(new DateTimeOffset(2026, 8, 7, 12, 0, 0, TimeSpan.Zero)); + MxAccessGrpcMapper mapper = new(timeProvider); + MxCommand command = new() + { + Kind = MxCommandKind.Write, + Write = new WriteCommand + { + ServerHandle = 10, + ItemHandle = 20, + UserId = 30, + Value = new MxValue + { + DataType = MxDataType.String, + StringValue = "value", + }, + }, + }; + MxCommandRequest request = new() + { + SessionId = "session-1", + Command = command.Clone(), + }; + + WorkerCommand fromCommand = mapper.MapCommand(command); + WorkerCommand fromRequest = mapper.MapCommand(request); + command.Write.Value.StringValue = "changed"; + + Assert.Equal(MxCommandKind.Write, fromCommand.Command.Kind); + Assert.Equal("value", fromCommand.Command.Write.Value.StringValue); + Assert.NotNull(fromCommand.EnqueueTimestamp); + Assert.Equal(fromRequest, fromCommand); + } + /// Verifies that command reply mapping preserves HRESULT and status information. [Fact] public void MapCommandReply_PreservesHresultStatusesAndPayload()