Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Address TODOs in redirect controller #3990

Merged
merged 5 commits into from Mar 7, 2024
Merged

Address TODOs in redirect controller #3990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
455401b
Small cleanup
danielmarbach Mar 5, 2024
79236f5
Introducing a simple header for now
danielmarbach Mar 6, 2024
e746487
Consistently returning the redirect
danielmarbach Mar 6, 2024
099b9ab
Formatting
danielmarbach Mar 6, 2024
89520aa
Approval
danielmarbach Mar 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt
View file
Open in desktop
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
Open in desktop
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; }
}
}
}