Skip to content

Commit

Permalink
Address TODOs in redirect controller (#3990)
Browse files Browse the repository at this point in the history 
* Small cleanup

* Introducing a simple header for now

* Consistently returning the redirect

* Formatting

* Approval
  • Loading branch information
@danielmarbach
danielmarbach committed Mar 7, 2024
1 parent a8f80d7 commit c37c4ed
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 27 deletions.
4 changes: 2 additions & 2 deletions src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt
View file
Expand Up @@ -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)
60 changes: 35 additions & 25 deletions src/ServiceControl/MessageRedirects/Api/MessageRedirectsController.cs
View file
Expand Up @@ -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;
Expand All @@ -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(
Expand All @@ -29,15 +29,15 @@ public class MessageRedirectsController(
[HttpPost]
public async Task<IActionResult> 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
};

Expand All @@ -47,19 +47,25 @@ public async Task<IActionResult> 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");
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();
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");
// 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);
Expand All @@ -73,7 +79,7 @@ public async Task<IActionResult> NewRedirects(MessageRedirectRequest request)
ToPhysicalAddress = messageRedirect.ToPhysicalAddress
});

if (request.retryexisting)
if (request.RetryExisting)
{
await session.SendLocal(new RetryPendingMessages
{
Expand All @@ -84,14 +90,14 @@ public async Task<IActionResult> 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}")]
[Route("redirects/{messageRedirectId:guid}")]
[HttpPut]
public async Task<IActionResult> UpdateRedirect(Guid messageRedirectId, MessageRedirectRequest request)
{
if (string.IsNullOrWhiteSpace(request.tophysicaladdress))
if (string.IsNullOrWhiteSpace(request.ToPhysicalAddress))
{
return BadRequest();
}
Expand All @@ -105,7 +111,7 @@ public async Task<IActionResult> UpdateRedirect(Guid messageRedirectId, MessageR
return NotFound();
}

var toMessageRedirectId = DeterministicGuid.MakeId(request.tophysicaladdress);
var toMessageRedirectId = DeterministicGuid.MakeId(request.ToPhysicalAddress);

if (redirects[toMessageRedirectId] != null)
{
Expand All @@ -117,7 +123,7 @@ public async Task<IActionResult> 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;
Expand All @@ -129,7 +135,7 @@ public async Task<IActionResult> UpdateRedirect(Guid messageRedirectId, MessageR
return NoContent();
}

[Route("redirects/{messageredirectid:guid}")]
[Route("redirects/{messageRedirectId:guid}")]
[HttpDelete]
public async Task<IActionResult> DeleteRedirect(Guid messageRedirectId)
{
Expand Down Expand Up @@ -189,15 +195,19 @@ public async Task<IEnumerable<RedirectsQueryResult>> 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; }
}
}
}

0 comments on commit c37c4ed

Please sign in to comment.