Skip to content

Commit e80b2ff

Browse files
committed
fix: address pr review comments
1 parent 49ce050 commit e80b2ff

File tree

3 files changed

+10
-15
lines changed

3 files changed

+10
-15
lines changed

services/network/Http.cpp

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
#include "infra/stream/StringInputStream.hpp"
33
#include "infra/stream/StringOutputStream.hpp"
44
#include "infra/util/Tokenizer.hpp"
5-
#include <string>
65

76
namespace services
87
{
@@ -206,12 +205,9 @@ namespace services
206205
infra::Tokenizer tokenizer(statusLine, ' ');
207206

208207
auto versionValid = HttpVersionValid(tokenizer.Token(0));
209-
auto optionalStatusCode = HttpStatusCodeFromString(tokenizer.Token(1));
210-
if (versionValid && optionalStatusCode)
211-
{
212-
statusCode = *optionalStatusCode;
208+
auto statusCode = HttpStatusCodeFromString(tokenizer.Token(1));
209+
if (versionValid)
213210
observer.StatusAvailable(statusCode, statusLine);
214-
}
215211
else
216212
{
217213
error = true;
@@ -409,7 +405,7 @@ namespace services
409405
return std::nullopt;
410406
}
411407

412-
std::optional<HttpStatusCode> HttpStatusCodeFromString(infra::BoundedConstString statusCode)
408+
HttpStatusCode HttpStatusCodeFromString(infra::BoundedConstString statusCode)
413409
{
414410
std::underlying_type<services::HttpStatusCode>::type value = 0;
415411

@@ -537,7 +533,7 @@ namespace services
537533
return "NetworkAuthenticationRequired";
538534
}
539535

540-
return "";
536+
return "Unknown HTTP status";
541537
}
542538
}
543539

@@ -559,11 +555,10 @@ namespace infra
559555

560556
TextOutputStream& operator<<(TextOutputStream& stream, services::HttpStatusCode statusCode)
561557
{
562-
auto statusString = services::HttpStatusCodeToString(statusCode);
563-
if (statusString.empty())
564-
stream << static_cast<int>(statusCode);
565-
else
566-
stream << statusString;
558+
auto status = services::HttpStatusCodeToString(statusCode);
559+
stream << status;
560+
if (status == "Unknown HTTP status")
561+
stream << ": " << static_cast<int>(statusCode);
567562

568563
return stream;
569564
}

services/network/Http.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ namespace services
193193
infra::BoundedConstString HttpVerbToString(HttpVerb verb);
194194
std::optional<HttpVerb> HttpVerbFromString(infra::BoundedConstString verb);
195195

196-
std::optional<HttpStatusCode> HttpStatusCodeFromString(infra::BoundedConstString statusCode);
196+
HttpStatusCode HttpStatusCodeFromString(infra::BoundedConstString statusCode);
197197
infra::BoundedConstString HttpStatusCodeToString(services::HttpStatusCode statusCode);
198198
}
199199

services/network/test/TestHttpClient.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ INSTANTIATE_TEST_SUITE_P(HttpStatusMessageTest, HttpStatusMessageFormattingTest,
107107
HttpStatusCodeWithString{ services::HttpStatusCode::LoopDetected, "LoopDetected" },
108108
HttpStatusCodeWithString{ services::HttpStatusCode::NotExtended, "NotExtended" },
109109
HttpStatusCodeWithString{ services::HttpStatusCode::NetworkAuthenticationRequired, "NetworkAuthenticationRequired" },
110-
HttpStatusCodeWithString{ static_cast<services::HttpStatusCode>(900), "900" }));
110+
HttpStatusCodeWithString{ static_cast<services::HttpStatusCode>(900), "Unknown HTTP status: 900" }));
111111

112112
struct HttpStatusCodeAndString
113113
{

0 commit comments

Comments
 (0)