Caching data response for complete event parsing #21
Conversation
This solves issue processing chunk of data without full event data
Event is completed only when double new line is present, otherwise we don't process partial event
Insert in buffer only not complete messages reducing amount of data needed to store
Recouse
left a comment
Recouse
left a comment
There was a problem hiding this comment.
Thank you for the PR!
I left some comments. Apart making ServerEventParser an actor, everything else seems fine.
|
|
||
| public protocol EventParser: Sendable { | ||
| func parse(_ data: Data) -> [EVEvent] | ||
| func parse(_ data: Data) async -> [EVEvent] |
There was a problem hiding this comment.
Should this function be marked as async?
There was a problem hiding this comment.
I allowed async function in EventParser protocol to allow conformance for an actor that run asynchronous
| /// ``ServerEventParser`` is used to parse text data into ``ServerEvent``. | ||
| public struct ServerEventParser: EventParser { | ||
| let mode: EventSource.Mode | ||
| actor ServerEventParser: EventParser { |
There was a problem hiding this comment.
What is the purpose of making this an actor?
There was a problem hiding this comment.
ServerEventParser has a mutable property buffer, so its better to use reference type such as class, then to provide exclusive access to modifying buffer property and ensuring data-race safety I make ServerEventParser an actor, which ensures only one thread at a time could access this data. But I am still new to those concepts in Swift so if you have a better solution in mind please share 🙂
Co-authored-by: Firdavs Khaydarov <recouse.web@gmail.com>
Recouse
left a comment
Recouse
left a comment
There was a problem hiding this comment.
Since EventParser and all its implementations are Sendable and thread safe, there's no need to make ServerEventParser an actor, and therefore the parse(_:) function does not need to be async.
I've made some changes. You'll also need to update the tests based on the compiler's suggestions. Once that's done, I can merge it
|
|
||
| public protocol EventParser: Sendable { | ||
| func parse(_ data: Data) -> [EVEvent] | ||
| func parse(_ data: Data) async -> [EVEvent] |
There was a problem hiding this comment.
| func parse(_ data: Data) async -> [EVEvent] | |
| mutating func parse(_ data: Data) async -> [EVEvent] |
| } else { | ||
| rawMessages = data.split(by: [Self.lf, Self.lf]) | ||
| } | ||
| func parse(_ data: Data) -> [EVEvent] { |
There was a problem hiding this comment.
| func parse(_ data: Data) -> [EVEvent] { | |
| mutating func parse(_ data: Data) -> [EVEvent] { |
| } else { | ||
| rawMessages = data.split(by: [Self.lf, Self.lf]) | ||
| } | ||
| func parse(_ data: Data) -> [EVEvent] { |
There was a problem hiding this comment.
| func parse(_ data: Data) -> [EVEvent] { | |
| mutating func parse(_ data: Data) -> [EVEvent] { |
| handleSessionResponse(response, completionHandler: completionHandler) | ||
| case let .didReceiveData(data): | ||
| parseMessages(from: data) | ||
| await parseMessages(from: data) |
There was a problem hiding this comment.
| await parseMessages(from: data) | |
| parseMessages(from: data) |
| } | ||
|
|
||
| private func parseMessages(from data: Data) { | ||
| private func parseMessages(from data: Data) async { |
There was a problem hiding this comment.
| private func parseMessages(from data: Data) async { | |
| private func parseMessages(from data: Data) { |
| } | ||
|
|
||
| let events = eventParser.parse(data) | ||
| let events = await eventParser.parse(data) |
There was a problem hiding this comment.
| let events = await eventParser.parse(data) | |
| let events = eventParser.parse(data) |
|
Thank you for the contribution, this is a great improvement to the package! |
2 participants
Buffer
Add buffer in parser to store parts of the messages that are not complete, as mentioned in #19
Now parser would store received data parse full messages and retain remaining of the message to parse with next chunk
If the connection closes and buffer is not empty events are not processed, this behaviour is according to the SSE documentation
Thread safety
To provide thread-safe access to buffer, parser is isolated to an actor.
Test Cases Adjustments:
Tests/EventSourceTests/EventParserTests.swift: Modified test cases to support asynchronous parsing by updating theparsemethod calls to useawait. Added new test cases to handle incomplete and separated messages. [1] [2] [3] [4] [5] [6] [7] [8]Typo
Fixed a typo in the
sessionDelegateTaskvariable name. [1] [2] [3] [4]