Headers parsing #63
Headers parsing #63
Conversation
| { | ||
| var offset = 0; |
There was a problem hiding this comment.
Removed precalculated offset +=... code since BufferWriter.Advance() method (which also marked with AggressiveInlining) does just that anyway and makes it much easier comprehend.
| @@ -8,94 +6,6 @@ public class SubscriptionTest | |||
|
|
|||
| public SubscriptionTest(ITestOutputHelper output) => _output = output; | |||
|
|
|||
| [Fact] | |||
| public async Task Subscription_with_same_subject() | |||
| { | |||
There was a problem hiding this comment.
This is moved to the ProtocolTest
src/NATS.Client.Core/NatsSub.cs
Outdated
| @@ -9,6 +11,7 @@ internal NatsSubBase(NatsConnection connection, SubscriptionManager manager, str | |||
| { | |||
| Connection = connection; | |||
| Manager = manager; | |||
| HeaderParser = new HeaderParser(Encoding.UTF8); | |||
There was a problem hiding this comment.
Header Parser is thread safe, right? Should we allocate 1 per connection as opposed to 1 per subscription.
Also, the default encoding is technically ASCII since this follows the HTTP RFC
Certain clients enforce ASCII while others do not though. For example the Go implementation defaults to UTF-8.
So I think that we should add HeaderEncoding to NatsOptions, default it to Encoding.ASCII there but let someone change it if they want.
Co-authored-by: Caleb Lloyd <2414837+caleblloyd@users.noreply.github.com>
Co-authored-by: Caleb Lloyd <2414837+caleblloyd@users.noreply.github.com>
Also, test resilience improvements.
This was caused by recent nats-server fix.
GitHub CI seems to have slowed down!? On my local ubuntu vm runs under a minute.
Separated server version read.
CI was failing because of a change on the server reporting WS port instead of standard port which broke cluster reconnect test. There was also an issue reading the server version in tests. All issues in test code and they're fixed now. |
Implementations of
HPUBandHMSGmessages.Also includes minor code clean-up and some test improvements to reduce false positives.