diff --git a/src/ServiceControl.UnitTests/ScatterGather/RemoteEndpointDetailsDeserializationTests.cs b/src/ServiceControl.UnitTests/ScatterGather/RemoteEndpointDetailsDeserializationTests.cs new file mode 100644 index 0000000000..5bf2429474 --- /dev/null +++ b/src/ServiceControl.UnitTests/ScatterGather/RemoteEndpointDetailsDeserializationTests.cs @@ -0,0 +1,86 @@ +namespace ServiceControl.UnitTests.ScatterGather; + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Text; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using CompositeViews.Messages; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Logging.Abstractions; +using NUnit.Framework; +using ServiceBus.Management.Infrastructure.Settings; +using ServiceControl.Infrastructure.WebApi; +using ServiceControl.Operations; +using ServiceControl.Infrastructure; +using ServiceControl.Persistence.Infrastructure; + +/// +/// Audit instances serialize their API with WhenWritingNull, so an endpoint whose host (or name) is not +/// known is sent without that property at all. and +/// became C# required members in 6.20, which System.Text.Json enforces on deserialization. A single such message +/// must not make the primary drop every message the audit instance returned. +/// +[TestFixture] +class RemoteEndpointDetailsDeserializationTests +{ + const string RemoteAddress = "http://audit-1/api"; + + // The shape an audit instance answers with for a message whose sending endpoint has no host, + // e.g. one parsed from an originating address without a machine part. + const string AuditPageWithoutSendingHost = """ + [ + { + "id": "audit-msg-1", + "message_id": "audit-msg-1", + "message_type": "Sales.PlaceOrder", + "sending_endpoint": { "name": "Sales", "host_id": "8f6b7a0e-6f0c-4c39-9a57-7d5f2e6b8a11" }, + "receiving_endpoint": { "name": "Billing", "host_id": "1d2c3b4a-0000-4c39-9a57-7d5f2e6b8a11", "host": "BILLING01" }, + "processed_at": "2026-10-07T07:00:00Z", + "headers": [] + } + ] + """; + + [Test] + public void An_endpoint_without_a_host_deserializes() + { + var endpoint = JsonSerializer.Deserialize("""{ "name": "Sales", "host_id": "8f6b7a0e-6f0c-4c39-9a57-7d5f2e6b8a11" }""", SerializerOptions.Default); + + Assert.That(endpoint.Name, Is.EqualTo("Sales")); + Assert.That(endpoint.Host, Is.Null); + } + + [Test] + public async Task A_remote_message_whose_endpoint_has_no_host_is_kept_and_the_result_is_complete() + { + var settings = new Settings { RemoteInstances = [new RemoteInstanceSetting(RemoteAddress)] }; + var factory = new FakeHttpClientFactory(); + factory.Register(settings.RemoteInstances[0], (_, _) => + { + var response = new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(AuditPageWithoutSendingHost, Encoding.UTF8, "application/json") }; + response.Headers.TryAddWithoutValidation("Total-Count", "1"); + response.Headers.TryAddWithoutValidation("ETag", "\"remote-etag\""); + return Task.FromResult(response); + }); + + var api = new TestApi(settings, factory); + + var result = await api.Execute(new ScatterGatherApiMessageViewContext(new PagingInfo(), new SortInfo("time_sent", "desc")), "/api/messages"); + + Assert.That(result.IncompleteInstances, Is.Empty, "the audit instance answered; its page must not be reported (and dropped) as a failure"); + Assert.That(result.Results.Select(m => m.MessageId), Is.EqualTo(["audit-msg-1"])); + Assert.That(result.Results.Single().SendingEndpoint.Name, Is.EqualTo("Sales")); + } + + class TestApi(Settings settings, IHttpClientFactory factory) + : ScatterGatherApiMessageView(new object(), settings, factory, new HttpContextAccessor(), NullLogger.Instance) + { + protected override Task>> LocalQuery(ScatterGatherApiMessageViewContext input, CancellationToken cancellationToken = default) => + Task.FromResult(new QueryResult>([], new QueryStatsInfo(DataVersion.FromToken("local-etag"), 0))); + } +} diff --git a/src/ServiceControl/Infrastructure/WebApi/SerializerOptions.cs b/src/ServiceControl/Infrastructure/WebApi/SerializerOptions.cs index 966798089f..6420a65017 100644 --- a/src/ServiceControl/Infrastructure/WebApi/SerializerOptions.cs +++ b/src/ServiceControl/Infrastructure/WebApi/SerializerOptions.cs @@ -2,6 +2,7 @@ { using System.Text.Json; using System.Text.Json.Serialization; + using System.Text.Json.Serialization.Metadata; public static class SerializerOptions { @@ -13,7 +14,21 @@ public static JsonSerializerOptions CustomizeDefaults(this JsonSerializerOptions options.WriteIndented = false; options.PropertyNamingPolicy = JsonNamingPolicy.SnakeCaseLower; options.Converters.Add(new JsonStringEnumConverter(JsonNamingPolicy.CamelCase)); + options.TypeInfoResolver = (options.TypeInfoResolver ?? new DefaultJsonTypeInfoResolver()).WithAddedModifier(DoNotEnforceRequiredMembers); return options; } + + // C# 'required' members document what our own code must set when it creates these types. The JSON + // we read comes from other instances, often of other versions, that write with WhenWritingNull (as + // above), so a value they do not know is simply absent. System.Text.Json enforces 'required' on + // deserialization, and one such message would fail an audit instance's whole response, dropping it + // from the scatter-gather. + static void DoNotEnforceRequiredMembers(JsonTypeInfo typeInfo) + { + foreach (var property in typeInfo.Properties) + { + property.IsRequired = false; + } + } } } \ No newline at end of file