Properly handle SSE comments in RESTEasy Reactive client and server code

Fixes: quarkusio#30169
geoand · Jan 4, 2023 · 9a7a560 · 9a7a560
1 parent 3e7eb23
commit 9a7a560
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 12 deletions.
diff --git a/...t/src/test/java/io/quarkus/resteasy/reactive/jsonb/deployment/test/sse/SseParserTest.java b/...t/src/test/java/io/quarkus/resteasy/reactive/jsonb/deployment/test/sse/SseParserTest.java
@@ -43,8 +43,8 @@ public void testParser() {
 
         // all fields
         testParser("data:DATA\nid:ID\n:COMMENT\nretry:23\nevent:NAME\n\n", "DATA", "COMMENT", "ID", "NAME", 23);
-        // all fields and no data: no event
-        testParser("id:ID\n:COMMENT\nretry:23\nevent:NAME\n\n", null, null, null, null, SseEvent.RECONNECT_NOT_SET);
+        // all fields and no data
+        testParser("id:ID\n:COMMENT\nretry:23\nevent:NAME\n\n", null, "COMMENT", "ID", "NAME", 23);
 
         // optional space after colon
         testParser("data:foo\n\n", "foo", null, null, null, SseEvent.RECONNECT_NOT_SET);
@@ -147,6 +147,13 @@ private void testParser(String event, String data, String comment, String lastId
                     .setId(lastId)
                     .setName(name)
                     .setReconnectDelay(reconnectDelay)));
+        } else if (comment != null) {
+            testParser(Collections.singletonList(event), Collections.singletonList(new InboundSseEventImpl(null, null)
+                    .setData(null)
+                    .setComment(comment)
+                    .setId(lastId)
+                    .setName(name)
+                    .setReconnectDelay(reconnectDelay)));
         } else {
             testParser(Collections.singletonList(event), Collections.emptyList());
         }

diff --git a/...loyment/src/test/java/io/quarkus/resteasy/reactive/server/test/stream/StreamResource.java b/...loyment/src/test/java/io/quarkus/resteasy/reactive/server/test/stream/StreamResource.java
@@ -160,7 +160,8 @@ public Multi<String> sseThrows() {
     @GET
     @Produces(MediaType.SERVER_SENT_EVENTS)
     public Multi<OutboundSseEvent> sseRaw(@Context Sse sse) {
-        return Multi.createFrom().items(sse.newEventBuilder().id("one").data("uno").name("eins").build(),
+        return Multi.createFrom().items(sse.newEventBuilder().comment("dummy").build(),
+                sse.newEventBuilder().id("one").data("uno").name("eins").build(),
                 sse.newEventBuilder().id("two").data("dos").name("zwei").build(),
                 sse.newEventBuilder().id("three").data("tres").name("drei").build());
     }

diff --git a/...loyment/src/test/java/io/quarkus/resteasy/reactive/server/test/stream/StreamTestCase.java b/...loyment/src/test/java/io/quarkus/resteasy/reactive/server/test/stream/StreamTestCase.java
@@ -3,7 +3,6 @@
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
 import java.time.Duration;
-import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -211,7 +210,7 @@ public void testSse() throws InterruptedException {
             });
             sse.open();
             Assertions.assertTrue(latch.await(20, TimeUnit.SECONDS));
-            Assertions.assertEquals(Arrays.asList("a", "b", "c"), results);
+            org.assertj.core.api.Assertions.assertThat(results).containsExactly("a", "b", "c");
             Assertions.assertEquals(0, errors.size());
         }
     }
@@ -248,7 +247,9 @@ public void testSseForMultiWithOutboundSseEvent() throws InterruptedException {
             List<String> results = new CopyOnWriteArrayList<>();
             List<String> ids = new CopyOnWriteArrayList<>();
             List<String> names = new CopyOnWriteArrayList<>();
+            List<String> comments = new CopyOnWriteArrayList<>();
             sse.register(event -> {
+                comments.add(event.getComment());
                 results.add(event.readData());
                 ids.add(event.getId());
                 names.add(event.getName());
@@ -259,9 +260,10 @@ public void testSseForMultiWithOutboundSseEvent() throws InterruptedException {
             });
             sse.open();
             Assertions.assertTrue(latch.await(20, TimeUnit.SECONDS));
-            Assertions.assertEquals(Arrays.asList("uno", "dos", "tres"), results);
-            Assertions.assertEquals(Arrays.asList("one", "two", "three"), ids);
-            Assertions.assertEquals(Arrays.asList("eins", "zwei", "drei"), names);
+            org.assertj.core.api.Assertions.assertThat(results).containsExactly(null, "uno", "dos", "tres");
+            org.assertj.core.api.Assertions.assertThat(ids).containsExactly(null, "one", "two", "three");
+            org.assertj.core.api.Assertions.assertThat(names).containsExactly(null, "eins", "zwei", "drei");
+            org.assertj.core.api.Assertions.assertThat(comments).containsExactly("dummy", null, null, null);
             Assertions.assertEquals(0, errors.size());
         }
     }

diff --git a/...ctive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/SseParser.java b/...ctive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/SseParser.java
@@ -145,7 +145,7 @@ private void parseEvent() {
 
     private void dispatchEvent() {
         // ignore empty events
-        if (dataBuffer.length() == 0)
+        if (dataBuffer.length() == 0 && commentBuffer.length() == 0)
             return;
         WebTargetImpl webTarget = sseEventSource.getWebTarget();
         InboundSseEventImpl event;
@@ -159,7 +159,7 @@ private void dispatchEvent() {
         event.setComment(commentBuffer.length() == 0 ? null : commentBuffer.toString());
         // SSE spec says empty string is the default, but JAX-RS says null if not specified
         event.setId(lastEventId);
-        event.setData(dataBuffer.toString());
+        event.setData(dataBuffer.length() == 0 ? null : dataBuffer.toString());
         // SSE spec says "message" is the default, but JAX-RS says null if not specified
         event.setName(eventType);
         event.setReconnectDelay(eventReconnectTime);

diff --git a/...eactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/SseUtil.java b/...eactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/SseUtil.java
@@ -69,8 +69,10 @@ private static String serialiseEvent(ResteasyReactiveRequestContext context, Out
             if (event.getReconnectDelay() >= 0)
                 serialiseField(context, sb, "retry", Long.toString(event.getReconnectDelay()), false);
         }
-        String data = serialiseDataToString(context, event, eventMediaType);
-        serialiseField(context, sb, "data", data, true);
+        if (event.getData() != null) {
+            String data = serialiseDataToString(context, event, eventMediaType);
+            serialiseField(context, sb, "data", data, true);
+        }
         sb.append(NL);
         // return a UTF8 buffer
         return sb.toString();