Skip to content

Commit

Permalink
Add ByteString.FromStream and ByteString.FromStreamAsync in C#
Browse files Browse the repository at this point in the history
Fixes #2088.

We now have separate tests for netcoreapp and net45 to test the two branches here.
(netstandard10 doesn't have MemoryStream.GetBuffer)

Although most of this library doesn't have any async functionality,
this feels like a natural place to locally add it.
  • Loading branch information
jskeet committed Jan 10, 2017
1 parent e76d91a commit fb71df9
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 1 deletion.
55 changes: 54 additions & 1 deletion csharp/src/Google.Protobuf.Test/ByteStringTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
using System;
using System.Text;
using NUnit.Framework;
using System.IO;
#if !DOTNET35
using System.Threading.Tasks;
#endif

namespace Google.Protobuf
{
Expand Down Expand Up @@ -168,6 +172,56 @@ public void FromBase64_Empty()
Assert.AreSame(ByteString.Empty, ByteString.FromBase64(""));
}

[Test]
public void FromStream_Seekable()
{
var stream = new MemoryStream(new byte[] { 1, 2, 3, 4, 5 });
// Consume the first byte, just to test that it's "from current position"
stream.ReadByte();
var actual = ByteString.FromStream(stream);
ByteString expected = ByteString.CopyFrom(2, 3, 4, 5);
Assert.AreEqual(expected, actual, $"{expected.ToBase64()} != {actual.ToBase64()}");
}

[Test]
public void FromStream_NotSeekable()
{
var stream = new MemoryStream(new byte[] { 1, 2, 3, 4, 5 });
// Consume the first byte, just to test that it's "from current position"
stream.ReadByte();
// Wrap the original stream in LimitedInputStream, which has CanSeek=false
var limitedStream = new LimitedInputStream(stream, 3);
var actual = ByteString.FromStream(limitedStream);
ByteString expected = ByteString.CopyFrom(2, 3, 4);
Assert.AreEqual(expected, actual, $"{expected.ToBase64()} != {actual.ToBase64()}");
}

#if !DOTNET35
[Test]
public async Task FromStreamAsync_Seekable()
{
var stream = new MemoryStream(new byte[] { 1, 2, 3, 4, 5 });
// Consume the first byte, just to test that it's "from current position"
stream.ReadByte();
var actual = await ByteString.FromStreamAsync(stream);
ByteString expected = ByteString.CopyFrom(2, 3, 4, 5);
Assert.AreEqual(expected, actual, $"{expected.ToBase64()} != {actual.ToBase64()}");
}

[Test]
public async Task FromStreamAsync_NotSeekable()
{
var stream = new MemoryStream(new byte[] { 1, 2, 3, 4, 5 });
// Consume the first byte, just to test that it's "from current position"
stream.ReadByte();
// Wrap the original stream in LimitedInputStream, which has CanSeek=false
var limitedStream = new LimitedInputStream(stream, 3);
var actual = await ByteString.FromStreamAsync(limitedStream);
ByteString expected = ByteString.CopyFrom(2, 3, 4);
Assert.AreEqual(expected, actual, $"{expected.ToBase64()} != {actual.ToBase64()}");
}
#endif

[Test]
public void GetHashCode_Regression()
{
Expand All @@ -179,6 +233,5 @@ public void GetHashCode_Regression()
ByteString b2 = ByteString.CopyFrom(200, 1, 2, 3, 4);
Assert.AreNotEqual(b1.GetHashCode(), b2.GetHashCode());
}

}
}
1 change: 1 addition & 0 deletions csharp/src/Google.Protobuf.Test/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"testRunner": "nunit",

"frameworks": {
"net451": {},
"netcoreapp1.0": {
"imports" : [ "dnxcore50", "netcoreapp1.0", "portable-net45+win8" ],
"buildOptions": {
Expand Down
53 changes: 53 additions & 0 deletions csharp/src/Google.Protobuf/ByteString.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@
using System.Collections.Generic;
using System.IO;
using System.Text;
#if !DOTNET35
using System.Threading;
using System.Threading.Tasks;
#endif

namespace Google.Protobuf
{
Expand Down Expand Up @@ -141,6 +145,55 @@ public static ByteString FromBase64(string bytes)
return bytes == "" ? Empty : new ByteString(Convert.FromBase64String(bytes));
}

/// <summary>
/// Constructs a <see cref="ByteString"/> from data in the given stream, synchronously.
/// </summary>
/// <remarks>If successful, <paramref name="stream"/> will be read completely, from the position
/// at the start of the call.</remarks>
/// <param name="stream">The stream to copy into a ByteString.</param>
/// <returns>A ByteString with content read from the given stream.</returns>
public static ByteString FromStream(Stream stream)
{
ProtoPreconditions.CheckNotNull(stream, nameof(stream));
int capacity = stream.CanSeek ? checked((int) (stream.Length - stream.Position)) : 0;

This comment has been minimized.

Copy link
@aaronhudon

aaronhudon Jun 2, 2017

Should we be concerned that this MemoryStream is not being disposed?
var memoryStream = new MemoryStream(capacity);

This comment has been minimized.

Copy link
@jskeet

jskeet Jun 2, 2017

Author Contributor

Nope - it would do basically nothing.

var memoryStream = new MemoryStream(capacity);

This comment has been minimized.

Copy link
@patspam

patspam Feb 13, 2017

This is outside of DOTNET35 conditional compilation, but .NET 3.5 `System.IO.Stream' doesn't support CopyTo, which leads to:

ByteString.cs(160,20): error CS1061: Type `System.IO.Stream' does not contain a definition for `CopyTo' and no extension method `CopyTo' of type `System.IO.Stream' could be found. Are you missing an assembly reference?
stream.CopyTo(memoryStream);
#if NETSTANDARD1_0
byte[] bytes = memoryStream.ToArray();
#else
// Avoid an extra copy if we can.
byte[] bytes = memoryStream.Length == memoryStream.Capacity ? memoryStream.GetBuffer() : memoryStream.ToArray();
#endif
return AttachBytes(bytes);
}

#if !DOTNET35
/// <summary>
/// Constructs a <see cref="ByteString"/> from data in the given stream, asynchronously.
/// </summary>
/// <remarks>If successful, <paramref name="stream"/> will be read completely, from the position
/// at the start of the call.</remarks>
/// <param name="stream">The stream to copy into a ByteString.</param>
/// <param name="cancellationToken">The cancellation token to use when reading from the stream, if any.</param>
/// <returns>A ByteString with content read from the given stream.</returns>
public async static Task<ByteString> FromStreamAsync(Stream stream, CancellationToken cancellationToken = default(CancellationToken))
{
ProtoPreconditions.CheckNotNull(stream, nameof(stream));
int capacity = stream.CanSeek ? checked((int) (stream.Length - stream.Position)) : 0;
var memoryStream = new MemoryStream(capacity);
// We have to specify the buffer size here, as there's no overload accepting the cancellation token
// alone. But it's documented to use 81920 by default if not specified.
await stream.CopyToAsync(memoryStream, 81920, cancellationToken);
#if NETSTANDARD1_0
byte[] bytes = memoryStream.ToArray();
#else
// Avoid an extra copy if we can.
byte[] bytes = memoryStream.Length == memoryStream.Capacity ? memoryStream.GetBuffer() : memoryStream.ToArray();
#endif
return AttachBytes(bytes);
}
#endif

/// <summary>
/// Constructs a <see cref="ByteString" /> from the given array. The contents
/// are copied, so further modifications to the array will not
Expand Down

0 comments on commit fb71df9

Please sign in to comment.