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

Parse messages and events based on byte arrays instead of strings #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ajgolledge
Copy link

Parse responses encoded in multibyte character sets (e.g. UTF-8) correctly by applying the content length given in the response to the length of the raw byte array and not the encoded string. The length of the string and the length of the byte array are not necessarily equivalent.

…of a UTF-8 string. This byte array is then used for further parsing in the derived EventMessage class to save converting back again to a byte array from a UTF-8 string. Tests have been extended to include a DetectedSpeech event. There are two variants of this event which are used in the tests, one containing a response with English text in the body and another containing German text and umlaut characters. Add Logger initializer to MessageParsingTests class to allow its test cases to be run independently. Add new test case to MessageParsingTests to test for the successful extraction of body payload from the EventMessage.
@ajgolledge ajgolledge mentioned this pull request May 9, 2023
@danbarua
Copy link

danbarua commented May 9, 2023

This is a valid request, and the implementation needs to be looked at carefully to avoid thrashing the Garbage Collector in high-traffic applications.

If my memory serves me correctly I have looked into it in the past and hit an obstacle with cleanup.

@danbarua
Copy link

danbarua commented May 9, 2023

And we are in Modern DotNet where we now have Span<T> and other useful things in the base class libraries.

The original NEventSocket was intended to run on Mono as well as DotNet, we can make some big improvements now that dotnet is properly cross-platform.

{
var parser = this;

foreach (var c in next)
foreach (var b in next)
{
parser = parser.Append(next);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sidenote: this recurses, should be parser.Append(c) or now parser.Append(b). This had no effect so far, because Append(byte[]) is not actually used.

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.

3 participants