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

Next version – various refactors and cleanups #6

Merged
merged 19 commits into from
Jul 11, 2023
Merged

Conversation

tmaxmax
Copy link
Owner

@tmaxmax tmaxmax commented Jul 11, 2023

See changelog and commits.

tmaxmax added 19 commits July 11, 2023 23:29
This improves parser performance a little and cleans up the code.
SSE is a text-based protocol, which uses the UTF-8 encoding. Therefore
it makes no sense to use raw byte slices in the API and the
implementation, reasons being:
- Giving a false impression to users that raw data can be safely sent
using this protocol
- Necessity of unsafe string to byte slice conversions of user input for
performance – immutability in userland is impossible to enforce
- Necessity of specifying ownership of byte slices in comments – with
safely created strings, one must not worry about copying it, to prevent
it being mutated at source
- More memory footprint of sse.Message
- More code

The following changes were made:
- Remove ability to add raw bytes as data to server events;
AppendText is removed, in favour of AppendData taking in strings
- Change the client event's data type to string
- Remove fmt.Stringer impl of client event (unnecessary and subjective)
- Many internal usages of []byte have been replaced with string
Also make cloning of chunks lazy.
This refactors chunk handling to not include newlines aswell.
This also modifies the FieldParser to output comments.
- Extract single-line string logic into separate type, messageField
- Rename EventID constructors: NewEventID -> NewID, MustEventID -> ID
- Create EventName type with constructors named similarly to EventID's
constructors
- Use these new types and constructors in Message and other code
To adhere to the spec nomenclature.
@tmaxmax tmaxmax self-assigned this Jul 11, 2023
@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Patch coverage: 93.54% and project coverage change: -1.37 ⚠️

Comparison is base (4938f99) 96.27% compared to head (eb740f5) 94.91%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master       #6      +/-   ##
==========================================
- Coverage   96.27%   94.91%   -1.37%     
==========================================
  Files          13       13              
  Lines        1020      963      -57     
==========================================
- Hits          982      914      -68     
- Misses         30       37       +7     
- Partials        8       12       +4     
Impacted Files Coverage Δ
joe.go 99.20% <ø> (ø)
message.go 81.25% <88.88%> (-5.85%) ⬇️
message_fields.go 90.90% <90.90%> (ø)
client_connection.go 98.92% <100.00%> (+1.01%) ⬆️
internal/parser/chunk.go 100.00% <100.00%> (ø)
internal/parser/field.go 100.00% <100.00%> (ø)
internal/parser/field_parser.go 100.00% <100.00%> (ø)
internal/parser/parser.go 100.00% <100.00%> (ø)
replay_buffer.go 100.00% <100.00%> (ø)
replay_provider.go 100.00% <100.00%> (ø)
... and 1 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tmaxmax tmaxmax merged commit eb740f5 into master Jul 11, 2023
@tmaxmax tmaxmax deleted the refactors branch July 11, 2023 21:22
@tmaxmax tmaxmax mentioned this pull request Jul 11, 2023
29 tasks
@tmaxmax tmaxmax added this to the v1 milestone Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant