Skip to content

Implement our own Peek() which blocks #49

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.

Sign up for GitHub

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

Merged
xPaw merged 5 commits into from
Jan 21, 2022
Merged

Implement our own Peek() which blocks #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
70c6411
Implement our own Peek() which blocks
xPaw Jan 14, 2022
7f27fc3
Reimplement ReadComment()
xPaw Jan 14, 2022
95fd05f
Add unit tests for #48
JustArchi Jan 14, 2022
56239be
Add test that comments with a carriage return do not stop the line
xPaw Jan 20, 2022
7c9f586
Typo
xPaw Jan 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions ValveKeyValue/ValveKeyValue.Test/Text/CommentWithCrOnlyTestCase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
using System.Linq;
using System.Text;
using NUnit.Framework;

namespace ValveKeyValue.Test
{
class CommentWithCrOnlyTestCase
{
[Test]
public void CommentWithCarriageReturn()
{
var text = new StringBuilder();
text.AppendLine(@"""test_kv""");
text.AppendLine("{");
text.AppendLine("// this is a comment that contains a carriage return: \r [$INVALID] which should continue parsing");
text.AppendLine(@"""test"" ""hello""");
text.AppendLine("}");

var data = KVSerializer.Create(KVSerializationFormat.KeyValues1Text).Deserialize(text.ToString());

Assert.Multiple(() =>
{
Assert.That(data.Children.Count(), Is.EqualTo(1));
Assert.That((string)data["test"], Is.EqualTo("hello"));
});
}
}
}
99 changes: 99 additions & 0 deletions ValveKeyValue/ValveKeyValue.Test/Text/StreamsTestCase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
using System;
using System.Globalization;
using System.IO;
using System.Text;
using NUnit.Framework;

namespace ValveKeyValue.Test;

internal class StreamsTestCase
{
class BlockingStream : Stream
{
private int _backingPosition;

internal BlockingStream(byte[] testData, int maxReadAtOnce = 1)
{
ArgumentNullException.ThrowIfNull(testData);

if (maxReadAtOnce <= 0) throw new ArgumentOutOfRangeException(nameof(maxReadAtOnce));

TestData = testData;
MaxReadAtOnce = maxReadAtOnce;
}

public byte[] TestData { get; }
public int MaxReadAtOnce { get; }

public override bool CanRead => true;
public override bool CanSeek => false;
public override bool CanWrite => false;
public override long Length => TestData.Length;

public override long Position
{
get => _backingPosition;
set => throw new NotSupportedException();
}

public override void Flush()
{
}

public override int Read(byte[] buffer, int offset, int count)
{
ArgumentNullException.ThrowIfNull(buffer);

if (offset < 0) throw new ArgumentOutOfRangeException(nameof(offset));

if (count <= 0) throw new ArgumentOutOfRangeException(nameof(count));

if (_backingPosition >= TestData.Length) return 0;

// Never read more than we have
if (count > TestData.Length - _backingPosition) count = TestData.Length - _backingPosition;

// Never read more than user-configured limit
if (count > MaxReadAtOnce) count = MaxReadAtOnce;

Buffer.BlockCopy(TestData, _backingPosition, buffer, offset, count);

_backingPosition += count;

return count;
}

public override long Seek(long offset, SeekOrigin origin)
{
throw new NotSupportedException();
}

public override void SetLength(long value)
{
throw new NotSupportedException();
}

public override void Write(byte[] buffer, int offset, int count)
{
throw new NotSupportedException();
}
}

[TestCase(1)]
[TestCase(10)]
public void CanHandleBlockingStreams(int maxReadAtOnce)
{
var testData = Encoding.UTF8.GetBytes(
@"""test_kv""
{
""test"" ""1337""
}"
);

using var blockingStream = new BlockingStream(testData, maxReadAtOnce);

var data = KVSerializer.Create(KVSerializationFormat.KeyValues1Text).Deserialize(blockingStream);

Assert.That(data["test"].ToInt32(CultureInfo.InvariantCulture), Is.EqualTo(1337));
}
}
55 changes: 50 additions & 5 deletions ValveKeyValue/ValveKeyValue/Deserialization/KV1TokenReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public KV1TokenReader(TextReader textReader, KVSerializerOptions options)
readonly KVSerializerOptions options;
TextReader textReader;
bool disposed;
int? peekedNext;

public KVToken ReadNextToken()
{
Expand Down Expand Up @@ -84,12 +85,34 @@ KVToken ReadComment()
{
ReadChar(CommentBegin);

if (Peek() == (char)CommentBegin)
var sb = new StringBuilder();
var next = Next();

// Some keyvalues implementations have a bug where only a single slash is needed for a comment
if (next != CommentBegin)
{
Next();
sb.Append(next);
}

while (true)
{
next = Next();

if (next == '\n')
{
break;
}

sb.Append(next);
}

if (sb[^1] == '\r')
{
sb.Remove(sb.Length - 1, 1);
}

var text = textReader.ReadLine();
var text = sb.ToString();
Copy link
Member

@yaakov-h yaakov-h Jan 20, 2022

Choose a reason for hiding this comment

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

Why this loop? Is there something wrong with textReader.ReadLine() from the old code? If its the case that we might have something in our little peekedNext field, can we not deal with that in a simpler manner?

Copy link
Member

@yaakov-h yaakov-h Jan 20, 2022

Choose a reason for hiding this comment

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

I wonder if it would be simpler to build a TextReader wrapper.

Copy link
Member Author

@xPaw xPaw Jan 20, 2022

Choose a reason for hiding this comment

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

I initially wanted to just use Stream, but that had its own problems.

I think ReadLine may return just for \r without being followed by \n which isn't how Valve deals with it (always looking for \n only).


return new KVToken(KVTokenType.Comment, text);
}

Expand Down Expand Up @@ -122,7 +145,18 @@ KVToken ReadInclusion()

char Next()
{
var next = textReader.Read();
int next;

if (peekedNext.HasValue)
{
next = peekedNext.Value;
peekedNext = null;
}
else
{
next = textReader.Read();
}

if (next == -1)
{
throw new EndOfStreamException();
Expand All @@ -131,7 +165,18 @@ char Next()
return (char)next;
}

int Peek() => textReader.Peek();
int Peek()
{
if (peekedNext.HasValue)
{
return peekedNext.Value;
}

var next = textReader.Read();
peekedNext = next;

return next;
}

void ReadChar(char expectedChar)
{
Expand Down