From b9947af295aad00be323c88402fcae0c14674684 Mon Sep 17 00:00:00 2001 From: Lowell Manners Date: Wed, 2 Dec 2020 12:01:02 +0100 Subject: [PATCH 1/3] Add AppendKeyValueAscii methods --- src/ZeroLog.Tests/LogEventTests.KeyValues.cs | 71 +++++++++++++++++++- src/ZeroLog/LogEvent.cs | 54 +++++++++++++++ 2 files changed, 123 insertions(+), 2 deletions(-) diff --git a/src/ZeroLog.Tests/LogEventTests.KeyValues.cs b/src/ZeroLog.Tests/LogEventTests.KeyValues.cs index d2843f7c..3f204329 100644 --- a/src/ZeroLog.Tests/LogEventTests.KeyValues.cs +++ b/src/ZeroLog.Tests/LogEventTests.KeyValues.cs @@ -1,4 +1,5 @@ using System; +using System.Text; using NUnit.Framework; namespace ZeroLog.Tests @@ -27,14 +28,80 @@ public void should_append_multiple_key_values() [Test] public void should_append_formatted_string_mixed_with_key_values() { - // TODO(lmanners): There are more edge cases here. _logEvent.AppendKeyValue("myKey", "myValue"); _logEvent.AppendFormat("Some {} message"); _logEvent.Append("formatted"); _logEvent.AppendKeyValue("otherKey", 2); + _logEvent.Append("..."); _logEvent.WriteToStringBuffer(_output); - Assert.AreEqual("Some formatted message ~~ { \"myKey\": \"myValue\", \"otherKey\": 2 }", _output.ToString()); + Assert.AreEqual("Some formatted message... ~~ { \"myKey\": \"myValue\", \"otherKey\": 2 }", _output.ToString()); + } + + [Test] + public void should_append_key_byte_array_value() + { + var bytes = Encoding.ASCII.GetBytes("myValue"); + _logEvent.AppendKeyValueAscii("myKey", bytes, bytes.Length); + _logEvent.WriteToStringBuffer(_output); + + Assert.AreEqual(" ~~ { \"myKey\": \"myValue\" }", _output.ToString()); + } + + [Test] + public void should_append_key_byte_span_value() + { + var bytes = Encoding.ASCII.GetBytes("myValue"); + _logEvent.AppendKeyValueAscii("myKey", bytes.AsSpan()); + _logEvent.WriteToStringBuffer(_output); + + Assert.AreEqual(" ~~ { \"myKey\": \"myValue\" }", _output.ToString()); + } + + [Test] + public void should_append_key_char_span_value() + { + _logEvent.AppendKeyValueAscii("myKey", "myValue".AsSpan()); + _logEvent.WriteToStringBuffer(_output); + + Assert.AreEqual(" ~~ { \"myKey\": \"myValue\" }", _output.ToString()); + } + + [Test] + public void should_ignore_byte_array_value_with_negative_length() + { + var bytes = Encoding.Default.GetBytes("myValue"); + _logEvent.AppendKeyValueAscii("myKey", bytes, -1); + _logEvent.WriteToStringBuffer(_output); + + Assert.AreEqual("", _output.ToString()); + } + + [Test] + public void should_ignore_byte_array_value_that_is_empty() + { + _logEvent.AppendKeyValueAscii("myKey", new byte[0], 0); + _logEvent.WriteToStringBuffer(_output); + + Assert.AreEqual("", _output.ToString()); + } + + [Test] + public void should_ignore_byte_span_value_that_is_empty() + { + _logEvent.AppendKeyValueAscii("myKey", ReadOnlySpan.Empty); + _logEvent.WriteToStringBuffer(_output); + + Assert.AreEqual("", _output.ToString()); + } + + [Test] + public void should_ignore_char_span_value_that_is_empty() + { + _logEvent.AppendKeyValueAscii("myKey", ReadOnlySpan.Empty); + _logEvent.WriteToStringBuffer(_output); + + Assert.AreEqual("", _output.ToString()); } [Test] diff --git a/src/ZeroLog/LogEvent.cs b/src/ZeroLog/LogEvent.cs index d5c80978..3cd6e0ef 100644 --- a/src/ZeroLog/LogEvent.cs +++ b/src/ZeroLog/LogEvent.cs @@ -151,6 +151,60 @@ public ILogEvent AppendKeyValue(string key, T? value) return this; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public ILogEvent AppendKeyValueAscii(string key, byte[]? bytes, int length) + { + if (length <= 0 || !PrepareAppend(sizeof(ArgumentType) + sizeof(byte) + sizeof(ArgumentType) + sizeof(int) + length, 2)) + return this; + + AppendArgumentType(ArgumentType.KeyString); + AppendString(key); + + AppendAsciiString(bytes, length); + return this; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public ILogEvent AppendKeyValueAscii(string key, byte* bytes, int length) + { + if (length <= 0 || !PrepareAppend(sizeof(ArgumentType) + sizeof(byte) + sizeof(ArgumentType) + sizeof(int) + length, 2)) + return this; + + AppendArgumentType(ArgumentType.KeyString); + AppendString(key); + + AppendAsciiString(bytes, length); + return this; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public ILogEvent AppendKeyValueAscii(string key, ReadOnlySpan bytes) + { + var length = bytes.Length; + if (length <= 0 || !PrepareAppend(sizeof(ArgumentType) + sizeof(byte) + sizeof(ArgumentType) + sizeof(int) + length, 2)) + return this; + + AppendArgumentType(ArgumentType.KeyString); + AppendString(key); + + AppendAsciiString(bytes); + return this; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public ILogEvent AppendKeyValueAscii(string key, ReadOnlySpan bytes) + { + var length = bytes.Length; + if (length <= 0 || !PrepareAppend(sizeof(ArgumentType) + sizeof(byte) + sizeof(ArgumentType) + sizeof(int) + length, 2)) + return this; + + AppendArgumentType(ArgumentType.KeyString); + AppendString(key); + + AppendAsciiString(bytes); + return this; + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public ILogEvent AppendAsciiString(byte[]? bytes, int length) { From 5722f9f1742b03069f31378007cde6135e05f29e Mon Sep 17 00:00:00 2001 From: Lowell Manners Date: Thu, 3 Dec 2020 11:11:48 +0100 Subject: [PATCH 2/3] Allow empty byte values and add truncation tests --- src/ZeroLog.Tests/LogEventTests.EdgeCases.cs | 51 +++++++++++++++++++- src/ZeroLog.Tests/LogEventTests.KeyValues.cs | 29 ++++++++--- src/ZeroLog/LogEvent.cs | 36 ++++++++++++-- 3 files changed, 104 insertions(+), 12 deletions(-) diff --git a/src/ZeroLog.Tests/LogEventTests.EdgeCases.cs b/src/ZeroLog.Tests/LogEventTests.EdgeCases.cs index a17162ec..3f409798 100644 --- a/src/ZeroLog.Tests/LogEventTests.EdgeCases.cs +++ b/src/ZeroLog.Tests/LogEventTests.EdgeCases.cs @@ -41,8 +41,7 @@ public void should_truncate_ascii_string_if_buffer_is_not_large_enough() [Test] public void should_ignore_ascii_string_if_buffer_is_not_large_enough_for_header( - [Range(_bufferSize - 2 * _asciiHeaderSize, _bufferSize)] - int firstStringLength) + [Range(_bufferSize - 2 * _asciiHeaderSize, _bufferSize)] int firstStringLength) { var largeString1 = new string('a', firstStringLength); var asciiBytes1 = Encoding.ASCII.GetBytes(largeString1); @@ -214,6 +213,54 @@ public void should_ignore_append_time_span_if_buffer_is_full() Check.That(string.IsNullOrWhiteSpace(_output.ToString())); } + [Test] + public void should_truncate_key_value_ascii_if_too_long_bytes() + { + var largeString = new string('a', 2000); + var bytes = Encoding.ASCII.GetBytes(largeString); + _logEvent.AppendKeyValue("key1", "value1"); + _logEvent.AppendKeyValueAscii("key2", bytes, bytes.Length); + _logEvent.WriteToStringBuffer(_output); + Assert.AreEqual(" ~~ { \"key1\": \"value1\" } [TRUNCATED]", _output.ToString()); + } + + [Test] + public void should_truncate_key_value_ascii_if_too_long_raw_bytes() + { + var largeString = new string('a', 2000); + var asciiBytes = Encoding.ASCII.GetBytes(largeString); + _logEvent.AppendKeyValue("key1", "value1"); + + fixed (byte* pAsciiBytes = asciiBytes) + { + _logEvent.AppendKeyValueAscii("key2", pAsciiBytes, asciiBytes.Length); + } + + _logEvent.WriteToStringBuffer(_output); + Assert.AreEqual(" ~~ { \"key1\": \"value1\" } [TRUNCATED]", _output.ToString()); + } + + [Test] + public void should_truncate_key_value_ascii_if_too_long_byte_span() + { + var largeString = new string('a', 2000); + var bytes = Encoding.ASCII.GetBytes(largeString); + _logEvent.AppendKeyValue("key1", "value1"); + _logEvent.AppendKeyValueAscii("key2", bytes.AsSpan()); + _logEvent.WriteToStringBuffer(_output); + Assert.AreEqual(" ~~ { \"key1\": \"value1\" } [TRUNCATED]", _output.ToString()); + } + + [Test] + public void should_truncate_key_value_ascii_if_too_long_char_span() + { + var largeString = new string('a', 2000); + _logEvent.AppendKeyValue("key1", "value1"); + _logEvent.AppendKeyValueAscii("key2", largeString.AsSpan()); + _logEvent.WriteToStringBuffer(_output); + Assert.AreEqual(" ~~ { \"key1\": \"value1\" } [TRUNCATED]", _output.ToString()); + } + [Test] public void should_ignore_append_key_values_if_buffer_is_full() { diff --git a/src/ZeroLog.Tests/LogEventTests.KeyValues.cs b/src/ZeroLog.Tests/LogEventTests.KeyValues.cs index 3f204329..3fe32be4 100644 --- a/src/ZeroLog.Tests/LogEventTests.KeyValues.cs +++ b/src/ZeroLog.Tests/LogEventTests.KeyValues.cs @@ -78,30 +78,47 @@ public void should_ignore_byte_array_value_with_negative_length() } [Test] - public void should_ignore_byte_array_value_that_is_empty() + public void should_support_byte_array_value_that_is_empty() { _logEvent.AppendKeyValueAscii("myKey", new byte[0], 0); _logEvent.WriteToStringBuffer(_output); - Assert.AreEqual("", _output.ToString()); + Assert.AreEqual(" ~~ { \"myKey\": \"\" }", _output.ToString()); + } + + [Test] + public void should_support_raw_byte_value_that_is_empty() + { + var bytes = new byte[0]; + unsafe + { + fixed (byte* pBytes = bytes) + { + _logEvent.AppendKeyValueAscii("myKey", pBytes, 0); + } + } + + _logEvent.WriteToStringBuffer(_output); + + Assert.AreEqual(" ~~ { \"myKey\": \"\" }", _output.ToString()); } [Test] - public void should_ignore_byte_span_value_that_is_empty() + public void should_support_byte_span_value_that_is_empty() { _logEvent.AppendKeyValueAscii("myKey", ReadOnlySpan.Empty); _logEvent.WriteToStringBuffer(_output); - Assert.AreEqual("", _output.ToString()); + Assert.AreEqual(" ~~ { \"myKey\": \"\" }", _output.ToString()); } [Test] - public void should_ignore_char_span_value_that_is_empty() + public void should_support_char_span_value_that_is_empty() { _logEvent.AppendKeyValueAscii("myKey", ReadOnlySpan.Empty); _logEvent.WriteToStringBuffer(_output); - Assert.AreEqual("", _output.ToString()); + Assert.AreEqual(" ~~ { \"myKey\": \"\" }", _output.ToString()); } [Test] diff --git a/src/ZeroLog/LogEvent.cs b/src/ZeroLog/LogEvent.cs index 3cd6e0ef..3cc651d1 100644 --- a/src/ZeroLog/LogEvent.cs +++ b/src/ZeroLog/LogEvent.cs @@ -154,12 +154,19 @@ public ILogEvent AppendKeyValue(string key, T? value) [MethodImpl(MethodImplOptions.AggressiveInlining)] public ILogEvent AppendKeyValueAscii(string key, byte[]? bytes, int length) { - if (length <= 0 || !PrepareAppend(sizeof(ArgumentType) + sizeof(byte) + sizeof(ArgumentType) + sizeof(int) + length, 2)) + if (length < 0 || !PrepareAppend(sizeof(ArgumentType) + sizeof(byte) + sizeof(ArgumentType) + sizeof(int) + length, 2)) return this; AppendArgumentType(ArgumentType.KeyString); AppendString(key); + if (length == 0) + { + AppendArgumentType(ArgumentType.AsciiString); + AppendInt32(0); + return this; + } + AppendAsciiString(bytes, length); return this; } @@ -167,12 +174,19 @@ public ILogEvent AppendKeyValueAscii(string key, byte[]? bytes, int length) [MethodImpl(MethodImplOptions.AggressiveInlining)] public ILogEvent AppendKeyValueAscii(string key, byte* bytes, int length) { - if (length <= 0 || !PrepareAppend(sizeof(ArgumentType) + sizeof(byte) + sizeof(ArgumentType) + sizeof(int) + length, 2)) + if (length < 0 || !PrepareAppend(sizeof(ArgumentType) + sizeof(byte) + sizeof(ArgumentType) + sizeof(int) + length, 2)) return this; AppendArgumentType(ArgumentType.KeyString); AppendString(key); + if (length == 0) + { + AppendArgumentType(ArgumentType.AsciiString); + AppendInt32(0); + return this; + } + AppendAsciiString(bytes, length); return this; } @@ -181,12 +195,19 @@ public ILogEvent AppendKeyValueAscii(string key, byte* bytes, int length) public ILogEvent AppendKeyValueAscii(string key, ReadOnlySpan bytes) { var length = bytes.Length; - if (length <= 0 || !PrepareAppend(sizeof(ArgumentType) + sizeof(byte) + sizeof(ArgumentType) + sizeof(int) + length, 2)) + if (!PrepareAppend(sizeof(ArgumentType) + sizeof(byte) + sizeof(ArgumentType) + sizeof(int) + length, 2)) return this; AppendArgumentType(ArgumentType.KeyString); AppendString(key); + if (length == 0) + { + AppendArgumentType(ArgumentType.AsciiString); + AppendInt32(0); + return this; + } + AppendAsciiString(bytes); return this; } @@ -195,12 +216,19 @@ public ILogEvent AppendKeyValueAscii(string key, ReadOnlySpan bytes) public ILogEvent AppendKeyValueAscii(string key, ReadOnlySpan bytes) { var length = bytes.Length; - if (length <= 0 || !PrepareAppend(sizeof(ArgumentType) + sizeof(byte) + sizeof(ArgumentType) + sizeof(int) + length, 2)) + if (!PrepareAppend(sizeof(ArgumentType) + sizeof(byte) + sizeof(ArgumentType) + sizeof(int) + length, 2)) return this; AppendArgumentType(ArgumentType.KeyString); AppendString(key); + if (length == 0) + { + AppendArgumentType(ArgumentType.AsciiString); + AppendInt32(0); + return this; + } + AppendAsciiString(bytes); return this; } From 4d149b5a135c78d7cb80d656ea3a7bfe241b9709 Mon Sep 17 00:00:00 2001 From: Lucas Trzesniewski Date: Thu, 3 Dec 2020 11:42:12 +0100 Subject: [PATCH 3/3] Remove calls to AppendAsciiString, handle null --- src/ZeroLog.Tests/LogEventTests.KeyValues.cs | 22 ++++++++- src/ZeroLog/LogEvent.cs | 48 ++++++++++---------- 2 files changed, 43 insertions(+), 27 deletions(-) diff --git a/src/ZeroLog.Tests/LogEventTests.KeyValues.cs b/src/ZeroLog.Tests/LogEventTests.KeyValues.cs index 3fe32be4..e5de1288 100644 --- a/src/ZeroLog.Tests/LogEventTests.KeyValues.cs +++ b/src/ZeroLog.Tests/LogEventTests.KeyValues.cs @@ -87,9 +87,18 @@ public void should_support_byte_array_value_that_is_empty() } [Test] - public void should_support_raw_byte_value_that_is_empty() + public void should_support_null_byte_array() { - var bytes = new byte[0]; + _logEvent.AppendKeyValueAscii("myKey", (byte[])null, 0); + _logEvent.WriteToStringBuffer(_output); + + Assert.AreEqual(" ~~ { \"myKey\": null }", _output.ToString()); + } + + [Test] + public void should_support_empty_byte_pointer() + { + var bytes = new byte[1]; unsafe { fixed (byte* pBytes = bytes) @@ -103,6 +112,15 @@ public void should_support_raw_byte_value_that_is_empty() Assert.AreEqual(" ~~ { \"myKey\": \"\" }", _output.ToString()); } + [Test] + public unsafe void should_support_null_byte_pointer() + { + _logEvent.AppendKeyValueAscii("myKey", (byte*)null, 0); + _logEvent.WriteToStringBuffer(_output); + + Assert.AreEqual(" ~~ { \"myKey\": null }", _output.ToString()); + } + [Test] public void should_support_byte_span_value_that_is_empty() { diff --git a/src/ZeroLog/LogEvent.cs b/src/ZeroLog/LogEvent.cs index 3cc651d1..c8a67008 100644 --- a/src/ZeroLog/LogEvent.cs +++ b/src/ZeroLog/LogEvent.cs @@ -160,14 +160,18 @@ public ILogEvent AppendKeyValueAscii(string key, byte[]? bytes, int length) AppendArgumentType(ArgumentType.KeyString); AppendString(key); - if (length == 0) + if (bytes is null) { - AppendArgumentType(ArgumentType.AsciiString); - AppendInt32(0); + AppendArgumentType(ArgumentType.Null); return this; } - AppendAsciiString(bytes, length); + AppendArgumentType(ArgumentType.AsciiString); + AppendInt32(length); + + if (length != 0) + AppendBytes(bytes, length); + return this; } @@ -180,14 +184,15 @@ public ILogEvent AppendKeyValueAscii(string key, byte* bytes, int length) AppendArgumentType(ArgumentType.KeyString); AppendString(key); - if (length == 0) + if (bytes == null) { - AppendArgumentType(ArgumentType.AsciiString); - AppendInt32(0); + AppendArgumentType(ArgumentType.Null); return this; } - AppendAsciiString(bytes, length); + AppendArgumentType(ArgumentType.AsciiString); + AppendInt32(length); + AppendBytes(bytes, length); return this; } @@ -201,35 +206,28 @@ public ILogEvent AppendKeyValueAscii(string key, ReadOnlySpan bytes) AppendArgumentType(ArgumentType.KeyString); AppendString(key); - if (length == 0) - { - AppendArgumentType(ArgumentType.AsciiString); - AppendInt32(0); - return this; - } - - AppendAsciiString(bytes); + AppendArgumentType(ArgumentType.AsciiString); + AppendInt32(length); + AppendBytes(bytes); return this; } [MethodImpl(MethodImplOptions.AggressiveInlining)] - public ILogEvent AppendKeyValueAscii(string key, ReadOnlySpan bytes) + public ILogEvent AppendKeyValueAscii(string key, ReadOnlySpan chars) { - var length = bytes.Length; + var length = chars.Length; if (!PrepareAppend(sizeof(ArgumentType) + sizeof(byte) + sizeof(ArgumentType) + sizeof(int) + length, 2)) return this; AppendArgumentType(ArgumentType.KeyString); AppendString(key); - if (length == 0) - { - AppendArgumentType(ArgumentType.AsciiString); - AppendInt32(0); - return this; - } + AppendArgumentType(ArgumentType.AsciiString); + AppendInt32(length); + + foreach (var c in chars) + *_dataPointer++ = (byte)c; - AppendAsciiString(bytes); return this; }