nats-io / nats-kafka

NATS to Kafka Bridging
Apache License 2.0
131 stars 32 forks source link

Write message indexes correctly in protobuf #62

Closed drnushooz closed 2 years ago

drnushooz commented 2 years ago

An issue exists in the protobuf serializer where the message indexes array length and contents are not written correctly causing the deserialization to fail. This PR correctly sets the array length as well as the message indexes array contents using varints and also changes the serializer test to decode the message and check for validity.

codecov-commenter commented 2 years ago

Codecov Report

Merging #62 (e3f927c) into main (d10eb43) will increase coverage by 0.07%. The diff coverage is 86.66%.

:exclamation: Current head e3f927c differs from pull request most recent head 3e6d1c9. Consider uploading reports for the commit 3e6d1c9 to get more accurate results Impacted file tree graph

@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
+ Coverage   67.19%   67.27%   +0.07%     
==========================================
  Files          27       27              
  Lines        1759     1763       +4     
==========================================
+ Hits         1182     1186       +4     
  Misses        448      448              
  Partials      129      129              
Impacted Files Coverage Δ
server/kafka/pb_deserializer.go 57.44% <ø> (ø)
server/kafka/pb_serializer.go 68.29% <86.66%> (+3.42%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d10eb43...3e6d1c9. Read the comment docs.

variadico commented 2 years ago

Thanks for the fix! Bug fix release coming soon.

drnushooz commented 2 years ago

Thanks for the fix! Bug fix release coming soon.

Sounds great! No rush on this one.