Remove CatchAll and body ID manipulation since it should no longer be…

… needed (#3987) * No longer replace the message id since the body will be loaded using the unique message id which is a deterministic ID that doesn't contain slashes Co-authored-by: Mauro Servienti <mauro.servienti@gmail.com> * Remove the catch all part because it was only handling /body which should not contain slashes anymore Co-authored-by: Mauro Servienti <mauro.servienti@gmail.com> * Remove the replacement of forward slashes since that could never have worked Co-authored-by: Mauro Servienti <mauro.servienti@gmail.com> --------- Co-authored-by: Mauro Servienti <mauro.servienti@gmail.com>
Particular · Mar 12, 2024 · f5db181 · f5db181
1 parent 5f3f0e0
commit f5db181
Show file tree

Hide file tree

Showing 4 changed files with 3 additions and 42 deletions.
diff --git a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt
@@ -9,7 +9,6 @@ GET /endpoints/{endpoint}/messages/search/{keyword} => ServiceControl.Audit.Audi
 GET /endpoints/known => ServiceControl.Audit.Monitoring.KnownEndpointsController:GetAll(PagingInfo pagingInfo)
 GET /instance-info => ServiceControl.Audit.Infrastructure.WebApi.RootController:Config()
 GET /messages => ServiceControl.Audit.Auditing.MessagesView.GetMessagesController:GetAllMessages(PagingInfo pagingInfo, SortInfo sortInfo, Boolean includeSystemMessages)
-GET /messages/{*catchAll} => ServiceControl.Audit.Auditing.MessagesView.GetMessagesController:CatchAll(String catchAll)
 GET /messages/{id}/body => ServiceControl.Audit.Auditing.MessagesView.GetMessagesController:Get(String id)
 GET /messages/search => ServiceControl.Audit.Auditing.MessagesView.GetMessagesController:Search(PagingInfo pagingInfo, SortInfo sortInfo, String q)
 GET /messages/search/{keyword} => ServiceControl.Audit.Auditing.MessagesView.GetMessagesController:SearchByKeyWord(PagingInfo pagingInfo, SortInfo sortInfo, String keyword)

diff --git a/src/ServiceControl.Audit/Auditing/MessagesView/GetMessagesController.cs b/src/ServiceControl.Audit/Auditing/MessagesView/GetMessagesController.cs
@@ -8,8 +8,6 @@ namespace ServiceControl.Audit.Auditing.MessagesView
     using Microsoft.AspNetCore.Mvc;
     using Persistence;
 
-    // All routes matching `messages/*` must be in this controller as WebAPI cannot figure out the overlapping routes
-    // from `messages/{*catchAll}` if they're in separate controllers.
     [ApiController]
     [Route("api")]
     public class GetMessagesController(IAuditDataStore dataStore) : ControllerBase
@@ -45,10 +43,7 @@ public async Task<IList<AuditCount>> GetEndpointAuditCounts([FromQuery] PagingIn
         [HttpGet]
         public async Task<IActionResult> Get(string id)
         {
-            var messageId = id;
-            messageId = messageId?.Replace("/", @"\");
-
-            var result = await dataStore.GetMessageBody(messageId);
+            var result = await dataStore.GetMessageBody(id);
 
             if (result.Found == false)
             {
@@ -62,30 +57,14 @@ public async Task<IActionResult> Get(string id)
 
             if (result.StringContent == null && result.StreamContent == null)
             {
-                throw new Exception($"Metadata for message '{messageId}' indicated that a body was present but no content could be found in storage");
+                throw new Exception($"Metadata for message '{id}' indicated that a body was present but no content could be found in storage");
             }
 
             Response.Headers.ETag = result.ETag;
             var contentType = result.ContentType ?? "text/*";
             return result.StringContent != null ? Content(result.StringContent, contentType) : File(result.StreamContent, contentType);
         }
 
-        // TODO: Verify if this catch all approach is still relevant today with Kestrel
-        // Possible a message may contain a slash or backslash, either way http.sys will rewrite it to forward slash,
-        // and then the "normal" route above will not activate, resulting in 404 if this route is not present.
-        [Route("messages/{*catchAll}")]
-        [HttpGet]
-        public async Task<IActionResult> CatchAll(string catchAll)
-        {
-            if (!string.IsNullOrEmpty(catchAll) && catchAll.EndsWith("/body"))
-            {
-                var id = catchAll.Substring(0, catchAll.Length - 5);
-                return await Get(id);
-            }
-
-            return NotFound();
-        }
-
         [Route("messages/search")]
         [HttpGet]
         public async Task<IList<MessagesView>> Search([FromQuery] PagingInfo pagingInfo, [FromQuery] SortInfo sortInfo, string q)
@@ -99,7 +78,7 @@ public async Task<IList<MessagesView>> Search([FromQuery] PagingInfo pagingInfo,
         [HttpGet]
         public async Task<IList<MessagesView>> SearchByKeyWord([FromQuery] PagingInfo pagingInfo, [FromQuery] SortInfo sortInfo, string keyword)
         {
-            var result = await dataStore.QueryMessages(keyword?.Replace("/", @"\"), pagingInfo, sortInfo);
+            var result = await dataStore.QueryMessages(keyword, pagingInfo, sortInfo);
             Response.WithQueryStatsAndPagingInfo(result.QueryStats, pagingInfo);
             return result.Results;
         }

diff --git a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt
@@ -40,7 +40,6 @@ GET /heartbeats/stats => ServiceControl.Monitoring.EndpointsMonitoringController
 GET /instance-info => ServiceControl.Infrastructure.WebApi.RootController:Config()
 GET /license => ServiceControl.Licensing.LicenseController:License(Boolean refresh)
 GET /messages => ServiceControl.CompositeViews.Messages.GetMessagesController:Messages(PagingInfo pagingInfo, SortInfo sortInfo, Boolean includeSystemMessages)
-GET /messages/{*catchAll} => ServiceControl.CompositeViews.Messages.GetMessagesController:CatchAll(String catchAll, String instanceId)
 GET /messages/{id}/body => ServiceControl.CompositeViews.Messages.GetMessagesController:Get(String id, String instanceId)
 GET /messages/search => ServiceControl.CompositeViews.Messages.GetMessagesController:Search(PagingInfo pagingInfo, SortInfo sortInfo, String q)
 GET /messages/search/{keyword} => ServiceControl.CompositeViews.Messages.GetMessagesController:SearchByKeyWord(PagingInfo pagingInfo, SortInfo sortInfo, String keyword)

diff --git a/src/ServiceControl/CompositeViews/Messages/GetMessagesController.cs b/src/ServiceControl/CompositeViews/Messages/GetMessagesController.cs
@@ -87,22 +87,6 @@ public async Task<IActionResult> Get(string id, [FromQuery(Name = "instance_id")
             return Empty;
         }
 
-        // TODO Is this still needed?
-        // Possible a message may contain a slash or backslash, either way http.sys will rewrite it to forward slash,
-        // and then the "normal" route above will not activate, resulting in 404 if this route is not present.
-        [Route("messages/{*catchAll}")]
-        [HttpGet]
-        public async Task<IActionResult> CatchAll(string catchAll, [FromQuery(Name = "instance_id")] string instanceId)
-        {
-            if (!string.IsNullOrEmpty(catchAll) && catchAll.EndsWith("/body"))
-            {
-                var id = catchAll[..^5];
-                return await Get(id, instanceId);
-            }
-
-            return NotFound();
-        }
-
         [Route("messages/search")]
         [HttpGet]
         public Task<IList<MessagesView>> Search([FromQuery] PagingInfo pagingInfo, [FromQuery] SortInfo sortInfo, string q) => api.Execute(new(pagingInfo, sortInfo, q));