-
Notifications
You must be signed in to change notification settings - Fork 328
docs: Add documentation for SSE serialization, reconnection, and heartbeat #612
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
Conversation
WalkthroughThis update introduces Kotlin serialization support to both the client and server SSE (Server-Sent Events) code examples by adding the Kotlin serialization plugin and Ktor JSON serialization dependencies to their respective build scripts. New serializable data classes ( Changes
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
codeSnippets/snippets/server-sse/src/test/kotlin/com/example/ApplicationTest.kt (1)
53-80
: Consider adding assertions for heartbeat event countsThe heartbeat test captures both regular events and heartbeat events and cancels after receiving at least 3 of each. Consider adding assertions at the end of the test to verify that you received the expected number of events before cancellation.
withTimeout(5_000) { client.sse("/heartbeat") { incoming.collect { event -> when (event.data) { "Hello" -> hellos++ "heartbeat" -> heartbeats++ } if (hellos > 3 && heartbeats > 3) { cancel() } } } } + // Verify we received the expected number of events + assert(hellos > 3) { "Expected more than 3 Hello events, got $hellos" } + assert(heartbeats > 3) { "Expected more than 3 heartbeat events, got $heartbeats" } }codeSnippets/snippets/client-sse/src/main/kotlin/com.example/Application.kt (1)
43-51
: Consider using the deserialized objectsThe current implementation deserializes the events but doesn't use the resulting objects. For documentation purposes, consider adding code that demonstrates how to use the deserialized objects.
incoming.collect { event: TypedServerSentEvent<String> -> when (event.event) { "customer" -> { val customer: Customer? = deserialize<Customer>(event.data) + customer?.let { + println("Received customer: ${it.firstName} ${it.lastName}") + } } "product" -> { val product: Product? = deserialize<Product>(event.data) + product?.let { + println("Received product with ID: ${it.id}, prices: ${it.prices}") + } } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
codeSnippets/snippets/client-sse/build.gradle.kts
(2 hunks)codeSnippets/snippets/client-sse/src/main/kotlin/com.example/Application.kt
(2 hunks)codeSnippets/snippets/server-sse/build.gradle.kts
(2 hunks)codeSnippets/snippets/server-sse/src/main/kotlin/com/example/Application.kt
(2 hunks)codeSnippets/snippets/server-sse/src/test/kotlin/com/example/ApplicationTest.kt
(2 hunks)topics/client-server-sent-events.topic
(2 hunks)topics/server-server-sent-events.topic
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
codeSnippets/snippets/server-sse/src/test/kotlin/com/example/ApplicationTest.kt (1)
codeSnippets/snippets/server-sse/src/main/kotlin/com/example/Application.kt (1)
module
(21-54)
🔇 Additional comments (17)
codeSnippets/snippets/server-sse/build.gradle.kts (2)
8-8
: Appropriate Kotlin serialization plugin addition.Adding the Kotlin serialization plugin is necessary for enabling JSON serialization in the SSE examples. This aligns with the PR objective to add documentation for SSE serialization.
25-25
: Correct Ktor JSON serialization dependency.The addition of
ktor-serialization-kotlinx-json
dependency ensures that the server can properly serialize objects to JSON. This matches the existing Ktor version used in the project.codeSnippets/snippets/client-sse/build.gradle.kts (2)
9-9
: Appropriate Kotlin serialization plugin addition.Adding the serialization plugin enables the client-side deserialization capabilities required for the SSE example. This matches the server-side configuration approach.
29-29
: Correct Ktor JSON serialization dependency.The addition of
ktor-serialization-kotlinx-json
dependency is necessary for the client to deserialize JSON data from SSE events. This supports the PR objective of documenting SSE serialization.codeSnippets/snippets/server-sse/src/main/kotlin/com/example/Application.kt (4)
8-11
: Appropriate serialization imports.The added imports are necessary for supporting JSON serialization in the SSE examples. These imports enable the use of
@Serializable
annotation and JSON encoding.
13-17
: Well-structured serializable data classes.The
Customer
andProduct
data classes are properly marked with@Serializable
annotation, making them suitable for JSON serialization. They provide good example models for demonstrating SSE serialization.
32-39
: Good implementation of SSE with serialization.The
/json
endpoint demonstrates how to use custom serialization with SSE. The implementation correctly:
- Obtains the appropriate serializer dynamically using reflection
- Encodes objects to JSON strings
- Sends different types of serialized objects as events
This implementation addresses the PR objective of documenting SSE serialization (KTOR-7776).
41-52
: Well-implemented heartbeat functionality.The
/heartbeat
endpoint effectively demonstrates:
- Setting up periodic heartbeat events with a 10ms interval
- Configuring a custom heartbeat event message
- Sending additional events alongside heartbeats
This implementation addresses the PR objective of adding heartbeat functionality to SSE (KTOR-7954).
topics/server-server-sent-events.topic (4)
94-96
: Improved code formatting in documentation.The
ServerSSESession
class name is now properly wrapped in a<code>
tag within its hyperlink, improving the consistency and readability of the documentation.
127-129
: Useful link to complete example.Adding a link to the full example repository helps users understand the implementation in context and provides a reference for implementing SSE in their own applications.
131-150
: Comprehensive documentation for SSE heartbeat.The new section on SSE heartbeat:
- Clearly explains the purpose of heartbeats in maintaining SSE connections
- Provides a well-documented code example showing how to configure heartbeats
- Links to the relevant API documentation
This documentation addresses the PR objective related to heartbeat functionality (KTOR-7954).
151-165
: Clear and informative serialization documentation.The serialization section:
- Explains how to enable serialization in SSE routes
- Shows a practical example of implementing serialization
- References the appropriate API class
- Clarifies how serialized data is handled in SSE events
This documentation addresses the PR objective related to SSE serialization (KTOR-7776).
codeSnippets/snippets/server-sse/src/test/kotlin/com/example/ApplicationTest.kt (1)
31-51
: Implementation of JSON serialization tests looks goodThe test correctly verifies that the server emits properly serialized JSON events by validating the structure of two different event types (Customer and Product). This test appropriately complements the server-side serialization implementation.
codeSnippets/snippets/client-sse/src/main/kotlin/com.example/Application.kt (1)
12-16
: Data class implementation is well structuredThe Customer and Product data classes are correctly annotated with @serializable and have appropriate fields for serialization/deserialization purposes.
topics/client-server-sent-events.topic (3)
52-72
: Clear documentation on reconnection functionalityThe reconnection section effectively explains the configuration options available for automatic reconnection, including the maximum number of attempts and reconnection delay time. The note about OkHttp not supporting this feature is also valuable information for users.
73-80
: Well-documented event filtering examplesThe event filtering section clearly explains how to include comment-only and retry-only events in the incoming flow, with appropriate code references.
167-188
: Comprehensive documentation on deserializationThe deserialization section effectively explains how to deserialize SSE events into type-safe Kotlin objects, including the necessary configuration and usage of the ClientSSESessionWithDeserialization class. The included code example from the snippets provides a clear illustration of real-world usage.
// example with deserialization | ||
client.sse({ | ||
url("http://localhost:8080/serverSentEvents") | ||
}, deserialize = { | ||
typeInfo, jsonString -> | ||
val serializer = Json.serializersModule.serializer(typeInfo.kotlinType!!) | ||
Json.decodeFromString(serializer, jsonString)!! | ||
}) { // `this` is `ClientSSESessionWithDeserialization` | ||
incoming.collect { event: TypedServerSentEvent<String> -> | ||
when (event.event) { | ||
"customer" -> { | ||
val customer: Customer? = deserialize<Customer>(event.data) | ||
} | ||
"product" -> { | ||
val product: Product? = deserialize<Product>(event.data) | ||
} | ||
} | ||
} | ||
} | ||
client.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove nullable operator (!!) in deserialization function
The use of !!
in the deserialization function could cause a NullPointerException if deserialization fails. It's best to handle this case more gracefully.
client.sse({
url("http://localhost:8080/serverSentEvents")
}, deserialize = {
typeInfo, jsonString ->
val serializer = Json.serializersModule.serializer(typeInfo.kotlinType!!)
- Json.decodeFromString(serializer, jsonString)!!
+ Json.decodeFromString(serializer, jsonString)
}) { // `this` is `ClientSSESessionWithDeserialization`
Also consider adding error handling for the deserialization process:
client.sse({
url("http://localhost:8080/serverSentEvents")
}, deserialize = {
typeInfo, jsonString ->
val serializer = Json.serializersModule.serializer(typeInfo.kotlinType!!)
- Json.decodeFromString(serializer, jsonString)!!
+ try {
+ Json.decodeFromString(serializer, jsonString)
+ } catch (e: Exception) {
+ println("Failed to deserialize: $jsonString")
+ null
+ }
}) { // `this` is `ClientSSESessionWithDeserialization`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good and clear documentation :)
I just left a few small notes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
topics/server-server-sent-events.topic (1)
151-169
: Good documentation of serialization functionality.The serialization section clearly explains how to enable and use serialization with SSE, which addresses another PR objective.
However, as noted in a previous review comment, it would be helpful to mention the
serialize
parameter in the main "Handle SSE sessions" section (around line 65), where you describe the ways to set up an SSE route. This would make readers aware of this capability earlier.
🧹 Nitpick comments (1)
topics/server-server-sent-events.topic (1)
130-150
: Great addition of heartbeat documentation.This new section clearly explains the purpose and implementation of the heartbeat functionality, which was one of the PR objectives.
One suggestion: The example uses a 10-millisecond interval, which is extremely short. Consider adding a note that real-world applications would typically use longer intervals (seconds or minutes) to avoid unnecessary network traffic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
topics/client-server-sent-events.topic
(3 hunks)topics/server-server-sent-events.topic
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- topics/client-server-sent-events.topic
🔇 Additional comments (3)
topics/server-server-sent-events.topic (3)
94-96
: Good improvement to formatting.Adding the
<code>
tag aroundServerSSESession
improves readability and properly indicates this is a code entity.
126-129
: Clear reference to the full example.Adding a link to the complete example is valuable for readers who want to see the implementation in context.
163-164
: Clear explanation of the serialization function.This line effectively addresses the previous review comment by clarifying that the example converts objects specifically to JSON.
KTOR-7776 Documentation for Add serialization for SSE
KTOR-8181 Documentation for Add reconnection in ClientSSESession
KTOR-7954 Documentation for Add heartbeat to SSE
Preview on staging:
Client SSE
Server SSE