Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allows the creation of empty SSE events #9207

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

spericas
Copy link
Member

@spericas spericas commented Aug 27, 2024

Description

Allows an SSE event to be created using no data. Uses a static constant SseEvent.NO_DATA to avoid returning null in getters. Updates serialization code and tests. See issue #9165.

@spericas spericas added webserver 4.x Version 4.x labels Aug 27, 2024
@spericas spericas added this to the 4.1.1 milestone Aug 27, 2024
@spericas spericas requested a review from barchetta August 27, 2024 13:37
@spericas spericas self-assigned this Aug 27, 2024
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Aug 27, 2024
@spericas spericas closed this Aug 27, 2024
@spericas spericas reopened this Aug 27, 2024
@spericas spericas marked this pull request as draft August 27, 2024 13:40
@spericas spericas removed the request for review from barchetta August 27, 2024 13:41
@spericas spericas marked this pull request as ready for review August 27, 2024 19:26
Copy link
Member

@tjquinno tjquinno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look fine to me, although I do have one question.

I see the writer of the issue cited the SSE spec saying that "empty" events are legal.

I know very little about SSE, but is it in any way a potential problem that the SseEvent data for an empty event would contain this special constant value (as opposed to having no value at all)? For example, does this distort some representation of the event size or length or something like that that could otherwise be used to infer an "empty" event?

@spericas
Copy link
Member Author

The changes look fine to me, although I do have one question.

I see the writer of the issue cited the SSE spec saying that "empty" events are legal.

I know very little about SSE, but is it in any way a potential problem that the SseEvent data for an empty event would contain this special constant value (as opposed to having no value at all)? For example, does this distort some representation of the event size or length or something like that that could otherwise be used to infer an "empty" event?

The code in SseSink currently checks for that constant and does not serialize anything into the stream. Would that answer your question?

@tjquinno
Copy link
Member

Yes. Thanks.

tjquinno
tjquinno previously approved these changes Aug 27, 2024
…nt SseEvent.NO_DATA to avoid returning null in getters. Updates serialization code and tests.
@spericas spericas merged commit af86d0f into helidon-io:main Aug 28, 2024
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x Version 4.x OCA Verified All contributors have signed the Oracle Contributor Agreement. webserver
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants