Optimize the SSE delimiting of streamed messages (#112)

Author: numanelahi

src/include/grpc_transcoding/response_to_json_translator.h

@@ -69,12 +69,12 @@ struct JsonResponseTranslateOptions {
   // If set to true, newline "\n" is used to separate streaming messages.
   // If set to false, all streaming messages are treated as a JSON array and
   // separated by comma.
-  bool stream_newline_delimited;
+  bool stream_newline_delimited = false;
 
   // If true, enforces Server-Sent Events (SSE) message framing (`data: <message>\n\n`)
   // and, `stream_newline_delimited` is ignored.
   // If false, message framing is determined by `stream_newline_delimited`.
-  bool stream_sse_style_delimited;
+  bool stream_sse_style_delimited = false;
 };
 
 class ResponseToJsonTranslator : public MessageStream {

src/response_to_json_translator.cc

@@ -128,19 +128,10 @@ bool ResponseToJsonTranslator::TranslateMessage(
   ::google::protobuf::io::StringOutputStream json_stream(json_out);
 
   if (streaming_ && options_.stream_sse_style_delimited) {
-    if (first_) {
-      if (!WriteString(&json_stream, "data: ")) {
-        status_ = absl::Status(absl::StatusCode::kInternal,
-                               "Failed to build the response message.");
-        return false;
-      }
-      first_ = false;
-    } else {
-      if (!WriteString(&json_stream, "\n\ndata: ")) {
-        status_ = absl::Status(absl::StatusCode::kInternal,
-                               "Failed to build the response message.");
-        return false;
-      }
+    if (!WriteString(&json_stream, "data: ")) {
+      status_ = absl::Status(absl::StatusCode::kInternal,
+                             "Failed to build the response message.");
+      return false;
     }
   } else if (streaming_ && !options_.stream_newline_delimited) {
     if (first_) {
@@ -174,7 +165,13 @@ bool ResponseToJsonTranslator::TranslateMessage(
   }
 
   // Append a newline delimiter after the message if needed.
-  if (streaming_ && options_.stream_newline_delimited && !options_.stream_sse_style_delimited) {
+  if (streaming_ && options_.stream_sse_style_delimited) {
+    if (!WriteString(&json_stream, "\n\n")) {
+      status_ = absl::Status(absl::StatusCode::kInternal,
+                             "Failed to build the response message.");
+      return false;
+    }
+  } else if (streaming_ && options_.stream_newline_delimited) {
     if (!WriteChar(&json_stream, '\n')) {
       status_ = absl::Status(absl::StatusCode::kInternal,
                              "Failed to build the response message.");

test/response_to_json_translator_test.cc

@@ -944,7 +944,7 @@ TEST_F(ResponseToJsonTranslatorTest, StreamingSSEStyleDelimitedDirectTest) {
 
   // Now we should have the test_message1 translated
   EXPECT_TRUE(translator.NextMessage(&message));
-  EXPECT_EQ("data: {\"name\":\"1\",\"theme\":\"Fiction\"}", message);
+  EXPECT_EQ("data: {\"name\":\"1\",\"theme\":\"Fiction\"}\n\n", message);
 
   // No more messages, but not finished yet
   EXPECT_FALSE(translator.NextMessage(&message));
@@ -957,10 +957,10 @@ TEST_F(ResponseToJsonTranslatorTest, StreamingSSEStyleDelimitedDirectTest) {
 
   // Now we should have test_message2 & test_message3 translated
   EXPECT_TRUE(translator.NextMessage(&message));
-  EXPECT_EQ("\n\ndata: {\"name\":\"2\",\"theme\":\"Fantasy\"}", message);
+  EXPECT_EQ("data: {\"name\":\"2\",\"theme\":\"Fantasy\"}\n\n", message);
 
   EXPECT_TRUE(translator.NextMessage(&message));
-  EXPECT_EQ("\n\ndata: {\"name\":\"3\",\"theme\":\"Children\"}", message);
+  EXPECT_EQ("data: {\"name\":\"3\",\"theme\":\"Children\"}\n\n", message);
 
   // No more messages, but not finished yet
   EXPECT_FALSE(translator.NextMessage(&message));
@@ -971,7 +971,7 @@ TEST_F(ResponseToJsonTranslatorTest, StreamingSSEStyleDelimitedDirectTest) {
 
   // Now we should have the test_message4 translated
   EXPECT_TRUE(translator.NextMessage(&message));
-  EXPECT_EQ("\n\ndata: {\"name\":\"4\",\"theme\":\"Classics\"}", message);
+  EXPECT_EQ("data: {\"name\":\"4\",\"theme\":\"Classics\"}\n\n", message);
 
   // No more messages, but not finished yet
   EXPECT_FALSE(translator.NextMessage(&message));