Merge pull request #97 from ArchipelagoMW/better_parse_failure_manage…

…ment

Fix parsing of UTF8 across 1kb boundaries + propagate package parse errors

Archipelago.MultiClient.Net.Tests/BaseArchipelagoSocketHelperFixture.cs

@@ -0,0 +1,175 @@
+#if NET47 || NET48 || NET6_0
+using Archipelago.MultiClient.Net.Helpers;
+using Archipelago.MultiClient.Net.Packets;
+using Newtonsoft.Json;
+using NUnit.Framework;
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Net.WebSockets;
+using System.Text;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Archipelago.MultiClient.Net.Tests
+{
+	[TestFixture]
+	class BaseArchipelagoSocketHelperFixture
+	{
+		[TestCase("some message", 100, Description = "Buffer bigger than message")]
+		[TestCase("some message", 10, Description = "Buffer smaller than message")]
+		[TestCase("🃁🃂🃃🃄🃅🃆🃇🃈🃉🃊",  10, Description = "UTF8 complex character breaking A")]
+		[TestCase("🃁🃂🃃🃄🃅🃆🃇🃈🃉🃊",  11, Description = "UTF8 complex character breaking B")]
+		[TestCase("🃁🃂🃃🃄🃅🃆🃇🃈🃉🃊",  12, Description = "UTF8 complex character breaking C")]
+		[TestCase("🃁🃂🃃🃄🃅🃆🃇🃈🃉🃊",  13, Description = "UTF8 complex character breaking D")]
+		public async Task Should_read_message_from_websocket_and_parse_archipelago_package(string message, int bufferSize)
+		{
+			var sayPacketJson = @"[{ ""cmd"":""Say"", ""text"": ""$MESSAGE"" }]".Replace("$MESSAGE", message);
+			var sut = new BaseArchipelagoSocketHelper<TestWebSocket>(new TestWebSocket(sayPacketJson), bufferSize);
+
+			Exception error = null;
+			ArchipelagoPacketBase receivedPacket = null;
+
+			sut.PacketReceived += packet => receivedPacket = packet;
+			sut.ErrorReceived += (e, _) => error = e;
+
+			sut.StartPolling();
+
+			int maxRetries = 100;
+			int retryCount = 0;
+			while (receivedPacket == null && retryCount++ < maxRetries)
+				await Task.Delay(10);
+
+			var sayPacket = receivedPacket as SayPacket;
+
+			Assert.That(error, Is.Null);
+			Assert.That(sayPacket, Is.Not.Null);
+			Assert.That(sayPacket.Text, Is.EqualTo(message));
+		}
+
+		[Test]
+		public async Task Should_throw_error_failed_parse()
+		{
+			var faultyJson = @"[{ ""cmd"":""Say"": ""text"": ""Incorrect json"" }]";
+
+			var sut = new BaseArchipelagoSocketHelper<TestWebSocket>(new TestWebSocket(faultyJson), 100);
+
+			Exception error = null;
+			ArchipelagoPacketBase receivedPacket = null;
+
+			sut.PacketReceived += packet => receivedPacket = packet;
+			sut.ErrorReceived += (e, _) => error = e;
+
+			sut.StartPolling();
+
+			int maxRetries = 100;
+			int retryCount = 0;
+			while (error == null && retryCount++ < maxRetries)
+				await Task.Delay(10);
+
+			Assert.That(receivedPacket, Is.Null);
+			Assert.That(error, Is.Not.Null);
+			Assert.That(error, Is.TypeOf(typeof(JsonReaderException)));
+		}
+
+		[TestCase("some message", 100, Description = "Buffer bigger than message")]
+		[TestCase("some message", 10, Description = "Buffer smaller than message")]
+		[TestCase("🃁🃂🃃🃄🃅🃆🃇🃈🃉🃊", 10, Description = "UTF8 complex character breaking A")]
+		[TestCase("🃁🃂🃃🃄🃅🃆🃇🃈🃉🃊", 11, Description = "UTF8 complex character breaking B")]
+		[TestCase("🃁🃂🃃🃄🃅🃆🃇🃈🃉🃊", 12, Description = "UTF8 complex character breaking C")]
+		[TestCase("🃁🃂🃃🃄🃅🃆🃇🃈🃉🃊", 13, Description = "UTF8 complex character breaking D")]
+		public async Task Should_serialize_and_send_message_to_websocket(string message, int bufferSize)
+		{
+			var sayPacket = new SayPacket { Text = message };
+
+			var testSocket = new TestWebSocket();
+
+			var sut = new BaseArchipelagoSocketHelper<TestWebSocket>(testSocket, bufferSize);
+
+			ArchipelagoPacketBase[] sendPackets = null;
+
+			sut.PacketsSent += packet => sendPackets = packet;
+
+			sut.StartPolling();
+
+			await sut.SendPacketAsync(sayPacket);
+
+			int maxRetries = 100;
+			int retryCount = 0;
+			while (sendPackets == null && retryCount++ < maxRetries)
+				await Task.Delay(10);
+
+			Assert.That(sendPackets, Is.Not.Null);
+			Assert.That(sendPackets[0], Is.EqualTo(sayPacket));
+
+			var sendMessages = testSocket.GetWrittenMessages();
+
+			Assert.That(sendMessages, Is.Not.Null);
+
+			var json = @"[{""cmd"":""Say"",""text"":""$MESSAGE""}]".Replace("$MESSAGE", message);
+			Assert.That(sendMessages[0], Is.EqualTo(json));
+		}
+	}
+
+	class TestWebSocket : WebSocket
+	{
+		MemoryStream incommingBytes;
+
+		int currentOutIndex;
+		readonly List<MemoryStream> outBytes = new List<MemoryStream> { new MemoryStream() };
+
+		public List<string> GetWrittenMessages() =>
+			outBytes.Select(bytes => Encoding.UTF8.GetString(bytes.ToArray())).ToList();
+
+		public TestWebSocket() : this(string.Empty)
+		{
+		}
+
+		public TestWebSocket(string inMessage)
+		{
+			incommingBytes = new MemoryStream(Encoding.UTF8.GetBytes(inMessage));
+		}
+
+		public override void Abort() { }
+
+		public override Task CloseAsync(WebSocketCloseStatus closeStatus, string statusDescription, CancellationToken cancellationToken) 
+			=> Task.CompletedTask;
+
+		public override Task CloseOutputAsync(WebSocketCloseStatus closeStatus, string statusDescription, CancellationToken cancellationToken) 
+			=> Task.CompletedTask;
+
+		public override void Dispose() { }
+
+		public override async Task<WebSocketReceiveResult> ReceiveAsync(ArraySegment<byte> outBuffer, CancellationToken cancellationToken)
+		{
+			// ReSharper disable once AssignNullToNotNullAttribute
+			var readCount = await incommingBytes.ReadAsync(outBuffer.Array, 0, outBuffer.Count, cancellationToken);
+
+			return new WebSocketReceiveResult(readCount, WebSocketMessageType.Text, incommingBytes.Position == incommingBytes.Length);
+		}
+
+		public override async Task SendAsync(ArraySegment<byte> buffer, WebSocketMessageType messageType, bool endOfMessage,
+			CancellationToken cancellationToken)
+		{
+			if (messageType != WebSocketMessageType.Text)
+				return;
+
+			outBytes[currentOutIndex].Write(buffer.Array, buffer.Offset, buffer.Count);
+
+			if (endOfMessage)
+			{
+				outBytes.Add(new MemoryStream());
+				currentOutIndex++;
+			}
+
+			await Task.CompletedTask;
+		}
+
+		public override WebSocketCloseStatus? CloseStatus => null;
+		public override string CloseStatusDescription => null;
+		public override string SubProtocol => null;
+		public override WebSocketState State => WebSocketState.Open;
+	}
+}
+#endif

Archipelago.MultiClient.Net.Tests/LocationCheckHelperFixture.cs

@@ -8,6 +8,7 @@
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.Linq;
+using System.Net.Sockets;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -477,7 +478,7 @@ public void Should_not_fail_when_room_update_is_missing_location_checks()
 
 #if !NET471
 		[Test]
-        public async Task Should_scout_locations_async()
+        public void Should_scout_locations_async_but_only_existing_ones()
         {
 	        var socket = Substitute.For<IArchipelagoSocketHelper>();
 	        var itemInfoResolver = Substitute.For<IItemInfoResolver>();
@@ -497,27 +498,150 @@ public async Task Should_scout_locations_async()
 			        { 1, new ReadOnlyCollection<PlayerInfo>(new List<PlayerInfo> { null, null, new PlayerInfo() }) }
 				}));
 #endif
-
 			ILocationCheckHelper sut = new LocationCheckHelper(socket, itemInfoResolver, connectionInfo, players);
 
+			var connected = new ConnectedPacket
+			{
+				LocationsChecked = new long[0],
+				MissingChecks = new[] { 1L }
+			};
+			socket.PacketReceived += Raise.Event<ArchipelagoSocketHelperDelagates.PacketReceivedHandler>(connected);
+
 			var locationScoutResponse = new LocationInfoPacket()
             {
                 Locations = new [] { new NetworkItem { Location = 1 } }
             };
 
-            var scoutTask = sut.ScoutLocationsAsync(1);
+            var scoutTask = sut.ScoutLocationsAsync(1, 2);
 
             Assert.That(scoutTask.IsCompleted, Is.False);
 
             socket.PacketReceived += Raise.Event<ArchipelagoSocketHelperDelagates.PacketReceivedHandler>(locationScoutResponse);
 
-            Assert.That(scoutTask.IsCompleted, Is.True);
+            scoutTask.Wait();
+
+            socket.Received().SendPacketAsync(Arg.Is<LocationScoutsPacket>(p => p.Locations.Length == 1 && p.Locations[0] == 1));
+
+			Assert.That(scoutTask.IsCompleted, Is.True);
+			Assert.That(scoutTask.Result, Is.Not.Null);
+			Assert.That(scoutTask.Result.Count, Is.EqualTo(1));
+			Assert.That(scoutTask.Result.First().Key, Is.EqualTo(1));
+		}
+
+		[Test]
+		public void Should_scout_locations_should_still_call_callback_if_no_locations_exist()
+		{
+			var socket = Substitute.For<IArchipelagoSocketHelper>();
+			var itemInfoResolver = Substitute.For<IItemInfoResolver>();
+			var connectionInfo = Substitute.For<IConnectionInfoProvider>();
+			connectionInfo.Team.Returns(1);
+			connectionInfo.Slot.Returns(2);
+
+			var players = Substitute.For<IPlayerHelper>();
+#if NET472
+	        players.Players.Returns(
+		        new Dictionary<int, ReadOnlyCollection<PlayerInfo>> {
+			        { 1, new ReadOnlyCollection<PlayerInfo>(new List<PlayerInfo> { null, null, new PlayerInfo() }) }
+		        });
+#else
+			players.Players.Returns(new ReadOnlyDictionary<int, ReadOnlyCollection<PlayerInfo>>(
+				new Dictionary<int, ReadOnlyCollection<PlayerInfo>> {
+					{ 1, new ReadOnlyCollection<PlayerInfo>(new List<PlayerInfo> { null, null, new PlayerInfo() }) }
+				}));
+#endif
+			ILocationCheckHelper sut = new LocationCheckHelper(socket, itemInfoResolver, connectionInfo, players);
+
+			var locationScoutResponse = new LocationInfoPacket()
+			{
+				Locations = new[] { new NetworkItem { Location = 1 } }
+			};
+
+			var scoutTask = sut.ScoutLocationsAsync(1, 2);
+
+			socket.PacketReceived += Raise.Event<ArchipelagoSocketHelperDelagates.PacketReceivedHandler>(locationScoutResponse);
+
+			scoutTask.Wait();
+
+			socket.DidNotReceive().SendPacketAsync(Arg.Any<LocationScoutsPacket>());
+
+			Assert.That(scoutTask.IsCompleted, Is.True);
+			Assert.That(scoutTask.Result, Is.Not.Null);
+			Assert.That(scoutTask.Result.Count, Is.EqualTo(0));
+		}
+#else
+		[Test]
+        public void Should_scout_locations_async_but_only_existing_ones()
+        {
+	        var socket = Substitute.For<IArchipelagoSocketHelper>();
+	        var itemInfoResolver = Substitute.For<IItemInfoResolver>();
+			var connectionInfo = Substitute.For<IConnectionInfoProvider>();
+			connectionInfo.Team.Returns(1);
+			connectionInfo.Slot.Returns(2);
 
-			await scoutTask;
+			var players = Substitute.For<IPlayerHelper>();
+
+	        players.Players.Returns(
+		        new Dictionary<int, ReadOnlyCollection<PlayerInfo>> {
+			        { 1, new ReadOnlyCollection<PlayerInfo>(new List<PlayerInfo> { null, null, new PlayerInfo() }) }
+		        });
+
+			ILocationCheckHelper sut = new LocationCheckHelper(socket, itemInfoResolver, connectionInfo, players);
+
+			var connected = new ConnectedPacket
+			{
+				LocationsChecked = new long[0], 
+				MissingChecks = new[] { 1L }
+			};
+			socket.PacketReceived += Raise.Event<ArchipelagoSocketHelperDelagates.PacketReceivedHandler>(connected);
+
+			Dictionary<long, ScoutedItemInfo> scoutedLocations = null;
+
+            sut.ScoutLocationsAsync(scouted => { scoutedLocations = scouted; }, 1, 2);
+
+            var locationScoutResponse = new LocationInfoPacket
+            {
+	            Locations = new[] { new NetworkItem { Location = 1 } }
+            };
+
+            socket.PacketReceived += Raise.Event<ArchipelagoSocketHelperDelagates.PacketReceivedHandler>(locationScoutResponse);
+
+			socket.Received().SendPacketAsync(Arg.Is<LocationScoutsPacket>(p => p.Locations.Length == 1 && p.Locations[0] == 1));
+
+			Assert.That(scoutedLocations, Is.Not.Null);
+            Assert.That(scoutedLocations.Count, Is.EqualTo(1));
+			Assert.That(scoutedLocations.First().Key, Is.EqualTo(1));
+		}
+
+        [Test]
+        public void Should_scout_locations_should_still_call_callback_if_no_locations_exist()
+        {
+	        var socket = Substitute.For<IArchipelagoSocketHelper>();
+	        var itemInfoResolver = Substitute.For<IItemInfoResolver>();
+	        var connectionInfo = Substitute.For<IConnectionInfoProvider>();
+	        connectionInfo.Team.Returns(1);
+	        connectionInfo.Slot.Returns(2);
+
+	        var players = Substitute.For<IPlayerHelper>();
+
+	        players.Players.Returns(
+		        new Dictionary<int, ReadOnlyCollection<PlayerInfo>> {
+			        { 1, new ReadOnlyCollection<PlayerInfo>(new List<PlayerInfo> { null, null, new PlayerInfo() }) }
+		        });
+
+	        ILocationCheckHelper sut = new LocationCheckHelper(socket, itemInfoResolver, connectionInfo, players);
+
+	        Dictionary<long, ScoutedItemInfo> scoutedLocations = null;
+
+	        sut.ScoutLocationsAsync(scouted => { scoutedLocations = scouted; }, 999);
+
+	        socket.DidNotReceive().SendPacketAsync(Arg.Any<LocationScoutsPacket>());
+
+	        Assert.That(scoutedLocations, Is.Not.Null);
+	        Assert.That(scoutedLocations.Count, Is.EqualTo(0));
         }
 #endif
-		
-	    [Test]
+
+		[Test]
 	    public void Should_ignore_non_existing_locations()
 	    {
 		    var socket = Substitute.For<IArchipelagoSocketHelper>();