From 455401b181dd1cb3dc1eceef00bcbbbd42fe9d5d Mon Sep 17 00:00:00 2001 From: danielmarbach Date: Tue, 5 Mar 2024 17:12:49 +0100 Subject: [PATCH 1/5] Small cleanup --- .../Api/MessageRedirectsController.cs | 40 ++++++++++--------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs b/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs index 80b9c987a9..0e2d5a8084 100644 --- a/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs +++ b/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Linq; using System.Net; + using System.Text.Json.Serialization; using System.Threading.Tasks; using Contracts.MessageRedirects; using Infrastructure.DomainEvents; @@ -16,7 +17,6 @@ using DeterministicGuid = Infrastructure.DeterministicGuid; - // TODO Re-evaluate the use of status codes in this controller [ApiController] [Route("api")] public class MessageRedirectsController( @@ -29,15 +29,15 @@ public class MessageRedirectsController( [HttpPost] public async Task NewRedirects(MessageRedirectRequest request) { - if (string.IsNullOrWhiteSpace(request.fromphysicaladdress) || string.IsNullOrWhiteSpace(request.tophysicaladdress)) + if (string.IsNullOrWhiteSpace(request.FromPhysicalAddress) || string.IsNullOrWhiteSpace(request.ToPhysicalAddress)) { return BadRequest(); } var messageRedirect = new MessageRedirect { - FromPhysicalAddress = request.fromphysicaladdress, - ToPhysicalAddress = request.tophysicaladdress, + FromPhysicalAddress = request.FromPhysicalAddress, + ToPhysicalAddress = request.ToPhysicalAddress, LastModifiedTicks = DateTime.UtcNow.Ticks }; @@ -47,18 +47,16 @@ public async Task NewRedirects(MessageRedirectRequest request) if (existing != null) { - //TODO verify both of these return the same as the previous code return existing.ToPhysicalAddress == messageRedirect.ToPhysicalAddress // not using Created here because that would be turned by the HttpNoContentOutputFormatter into a 204 ? StatusCode((int)HttpStatusCode.Created) : Conflict("Duplicate"); } - var dependents = collection.Redirects.Where(r => r.ToPhysicalAddress == request.fromphysicaladdress).ToList(); + var dependents = collection.Redirects.Where(r => r.ToPhysicalAddress == request.FromPhysicalAddress).ToList(); if (dependents.Any()) { - //TODO verify this returns the same as the previous code return Conflict("Dependents"); } @@ -73,7 +71,7 @@ public async Task NewRedirects(MessageRedirectRequest request) ToPhysicalAddress = messageRedirect.ToPhysicalAddress }); - if (request.retryexisting) + if (request.RetryExisting) { await session.SendLocal(new RetryPendingMessages { @@ -87,11 +85,11 @@ public async Task NewRedirects(MessageRedirectRequest request) return StatusCode((int)HttpStatusCode.Created); } - [Route("redirects/{messageredirectid:guid}")] + [Route("redirects/{messageRedirectId:guid}")] [HttpPut] public async Task UpdateRedirect(Guid messageRedirectId, MessageRedirectRequest request) { - if (string.IsNullOrWhiteSpace(request.tophysicaladdress)) + if (string.IsNullOrWhiteSpace(request.ToPhysicalAddress)) { return BadRequest(); } @@ -105,7 +103,7 @@ public async Task UpdateRedirect(Guid messageRedirectId, MessageR return NotFound(); } - var toMessageRedirectId = DeterministicGuid.MakeId(request.tophysicaladdress); + var toMessageRedirectId = DeterministicGuid.MakeId(request.ToPhysicalAddress); if (redirects[toMessageRedirectId] != null) { @@ -117,7 +115,7 @@ public async Task UpdateRedirect(Guid messageRedirectId, MessageR MessageRedirectId = messageRedirectId, PreviousToPhysicalAddress = messageRedirect.ToPhysicalAddress, FromPhysicalAddress = messageRedirect.FromPhysicalAddress, - ToPhysicalAddress = messageRedirect.ToPhysicalAddress = request.tophysicaladdress + ToPhysicalAddress = messageRedirect.ToPhysicalAddress = request.ToPhysicalAddress }; messageRedirect.LastModifiedTicks = DateTime.UtcNow.Ticks; @@ -129,7 +127,7 @@ public async Task UpdateRedirect(Guid messageRedirectId, MessageR return NoContent(); } - [Route("redirects/{messageredirectid:guid}")] + [Route("redirects/{messageRedirectId:guid}")] [HttpDelete] public async Task DeleteRedirect(Guid messageRedirectId) { @@ -189,15 +187,19 @@ public async Task> Redirects(string sort, stri return queryResult; } - public record class RedirectsQueryResult(Guid MessageRedirectId, string FromPhysicalAddress, string ToPhysicalAddress, DateTime LastModified); + public record RedirectsQueryResult(Guid MessageRedirectId, string FromPhysicalAddress, string ToPhysicalAddress, DateTime LastModified); + // Input models from the API are for some odd reasons not snake cased public class MessageRedirectRequest { -#pragma warning disable IDE1006 // Naming Styles - public string fromphysicaladdress { get; set; } - public string tophysicaladdress { get; set; } - public bool retryexisting { get; set; } -#pragma warning restore IDE1006 // Naming Styles + [JsonPropertyName("fromphysicaladdress")] + public string FromPhysicalAddress { get; set; } + + [JsonPropertyName("tophysicaladdress")] + public string ToPhysicalAddress { get; set; } + + [JsonPropertyName("retryexisting")] + public bool RetryExisting { get; set; } } } } \ No newline at end of file From 79236f5d38a37f3fd4f26b74fd0c542c9da228a9 Mon Sep 17 00:00:00 2001 From: danielmarbach Date: Wed, 6 Mar 2024 10:51:58 +0100 Subject: [PATCH 2/5] Introducing a simple header for now --- .../Api/MessageRedirectsController.cs | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs b/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs index 0e2d5a8084..f31fcaeba4 100644 --- a/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs +++ b/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs @@ -47,17 +47,25 @@ public async Task NewRedirects(MessageRedirectRequest request) if (existing != null) { - return existing.ToPhysicalAddress == messageRedirect.ToPhysicalAddress - // not using Created here because that would be turned by the HttpNoContentOutputFormatter into a 204 - ? StatusCode((int)HttpStatusCode.Created) - : Conflict("Duplicate"); + if (existing.ToPhysicalAddress == messageRedirect.ToPhysicalAddress) + { + return StatusCode((int)HttpStatusCode.Created, messageRedirect); + } + + // Setting the ReasonPhrase is no longer supported in HTTP/2. We could be using application/problem+json instead + // but that would require a lot of changes in the client code. For now, we are using the X-Particular-Reason header + Response.Headers["X-Particular-Reason"] = "Duplicate"; + return StatusCode((int)HttpStatusCode.Conflict, existing); } var dependents = collection.Redirects.Where(r => r.ToPhysicalAddress == request.FromPhysicalAddress).ToList(); if (dependents.Any()) { - return Conflict("Dependents"); + // Setting the ReasonPhrase is no longer supported in HTTP/2. We could be using application/problem+json instead + // but that would require a lot of changes in the client code. For now, we are using the X-Particular-Reason header + Response.Headers["X-Particular-Reason"] = "Dependents"; + return StatusCode((int)HttpStatusCode.Conflict, dependents); } collection.Redirects.Add(messageRedirect); @@ -132,7 +140,7 @@ public async Task UpdateRedirect(Guid messageRedirectId, MessageR public async Task DeleteRedirect(Guid messageRedirectId) { var redirects = await store.GetOrCreate(); - + var messageRedirect = redirects[messageRedirectId]; if (messageRedirect == null) From e7464874fc0c2deef29cb638f04e41ca02ebab7b Mon Sep 17 00:00:00 2001 From: danielmarbach Date: Wed, 6 Mar 2024 11:04:27 +0100 Subject: [PATCH 3/5] Consistently returning the redirect --- .../MessageRedirects/Api/MessageRedirectsController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs b/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs index f31fcaeba4..9f0608e4fd 100644 --- a/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs +++ b/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs @@ -90,7 +90,7 @@ public async Task NewRedirects(MessageRedirectRequest request) } // not using Created here because that would be turned by the HttpNoContentOutputFormatter into a 204 - return StatusCode((int)HttpStatusCode.Created); + return StatusCode((int)HttpStatusCode.Created, messageRedirect); } [Route("redirects/{messageRedirectId:guid}")] From 099b9ab9018310ca2e2128df5aad806fbf172757 Mon Sep 17 00:00:00 2001 From: danielmarbach Date: Wed, 6 Mar 2024 11:38:13 +0100 Subject: [PATCH 4/5] Formatting --- .../MessageRedirects/Api/MessageRedirectsController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs b/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs index 9f0608e4fd..4899498683 100644 --- a/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs +++ b/src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs @@ -140,7 +140,7 @@ public async Task UpdateRedirect(Guid messageRedirectId, MessageR public async Task DeleteRedirect(Guid messageRedirectId) { var redirects = await store.GetOrCreate(); - + var messageRedirect = redirects[messageRedirectId]; if (messageRedirect == null) From 89520aab185850a9d3611eebcb2ea386d7f7f66a Mon Sep 17 00:00:00 2001 From: danielmarbach Date: Wed, 6 Mar 2024 13:08:17 +0100 Subject: [PATCH 5/5] Approval --- .../ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt index 774916efa8..d9f1fdbc10 100644 --- a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt +++ b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt @@ -67,6 +67,6 @@ DELETE /recoverability/unacknowledgedgroups/{groupId:required:minlength(1)} => S HEAD /redirect => ServiceControl.MessageRedirects.Api.MessageRedirectsController:CountRedirects() GET /redirects => ServiceControl.MessageRedirects.Api.MessageRedirectsController:Redirects(String sort, String direction, PagingInfo pagingInfo) POST /redirects => ServiceControl.MessageRedirects.Api.MessageRedirectsController:NewRedirects(MessageRedirectRequest request) -DELETE /redirects/{messageredirectid:guid} => ServiceControl.MessageRedirects.Api.MessageRedirectsController:DeleteRedirect(Guid messageRedirectId) -PUT /redirects/{messageredirectid:guid} => ServiceControl.MessageRedirects.Api.MessageRedirectsController:UpdateRedirect(Guid messageRedirectId, MessageRedirectRequest request) +DELETE /redirects/{messageRedirectId:guid} => ServiceControl.MessageRedirects.Api.MessageRedirectsController:DeleteRedirect(Guid messageRedirectId) +PUT /redirects/{messageRedirectId:guid} => ServiceControl.MessageRedirects.Api.MessageRedirectsController:UpdateRedirect(Guid messageRedirectId, MessageRedirectRequest request) GET /sagas/{id} => ServiceControl.SagaAudit.SagasController:Sagas(PagingInfo pagingInfo, Guid id)