FindRequestByMappingGuidAsync #1043
Conversation
There was a problem hiding this comment.
This review comes a bit late, sorry about that... I didn't pay enough attention when you first sent the PR.
I realize changing it now would be a breaking change, but I think it's worth it, because at the moment it's broken (it will throw if there's more than one matching request)
| /// <param name="mappingGuid">The Mapping Guid</param> | ||
| /// <param name="cancellationToken">The optional cancellationToken.</param> | ||
| [Get("requests/find")] | ||
| Task<LogEntryModel?> FindRequestByMappingGuidAsync([Query] Guid mappingGuid, CancellationToken cancellationToken = default); |
There was a problem hiding this comment.
It should probably be FindRequestsByMappingGuidAsync, and return a collection
| Guid.TryParse(value.ToString(), out var mappingGuid) | ||
| ) | ||
| { | ||
| var logEntry = LogEntries.SingleOrDefault(le => !le.RequestMessage.Path.StartsWith("/__admin/") && le.MappingGuid == mappingGuid); |
There was a problem hiding this comment.
There could be more than one matching request, and in this case SingleOrDefault will throw.
| return ToJson(logEntryMapper.Map(logEntry)); | ||
| } | ||
|
|
||
| return ResponseMessageBuilder.Create(HttpStatusCode.OK); |
There was a problem hiding this comment.
Returning 200 with an empty response is probably not what the client would expect. If the status code is 200, the client will expect the body to contain a LogEntryModel.
But anyway, I think this endpoint should return a collection (which will be empty if no match is found)
No description provided.