Extractor runs if supported

Author: bruno-garcia

src/Sentry.AspNetCore/BaseRequestPayloadExtractor.cs

@@ -0,0 +1,33 @@
+using Microsoft.AspNetCore.Http;
+
+namespace Sentry.AspNetCore
+{
+    public abstract class BaseRequestPayloadExtractor : IRequestPayloadExtractor
+    {
+        public object ExtractPayload(HttpRequest request)
+        {
+            if (!request.Body.CanSeek
+                || !request.Body.CanRead
+                || !IsSupported(request))
+            {
+                return null;
+            }
+
+            var originalPosition = request.Body.Position;
+            try
+            {
+                request.Body.Position = 0;
+
+                return DoExtractPayLoad(request);
+            }
+            finally
+            {
+                request.Body.Position = originalPosition;
+            }
+        }
+
+        protected abstract bool IsSupported(HttpRequest request);
+
+        protected abstract object DoExtractPayLoad(HttpRequest request);
+    }
+}

src/Sentry.AspNetCore/DefaultRequestPayloadExtractor.cs

@@ -4,38 +4,25 @@
 
 namespace Sentry.AspNetCore
 {
-    public class DefaultRequestPayloadExtractor : IRequestPayloadExtractor
+    public class DefaultRequestPayloadExtractor : BaseRequestPayloadExtractor
     {
-        public object ExtractPayload(HttpRequest request)
-        {
-            if (!request.Body.CanSeek || !request.Body.CanRead)
-            {
-                return null;
-            }
+        protected override bool IsSupported(HttpRequest request) => true;
 
-            var originalPosition = request.Body.Position;
-            try
-            {
-                request.Body.Position = 0;
-
-                // https://github.com/dotnet/corefx/blob/master/src/Common/src/CoreLib/System/IO/StreamReader.cs#L186
-                // Default parameters other than 'leaveOpen'
-                using (var reader = new StreamReader(request.Body, Encoding.UTF8, true, 1024,
-                    // Make sure StreamReader does not close the stream:
-                    leaveOpen: true))
-                {
-                    // This could be turned into async but at this point the stream should be buffered so no real value
-                    // A custom serializer that would take the stream and read from it into the output stream would add more value
-                    // as it would avoid the need to create the following (possibly huge) string
-                    var body = reader.ReadToEnd();
-                    return body.Length == 0
-                        ? null
-                        : body;
-                }
-            }
-            finally
+        protected override object DoExtractPayLoad(HttpRequest request)
+        {
+            // https://github.com/dotnet/corefx/blob/master/src/Common/src/CoreLib/System/IO/StreamReader.cs#L186
+            // Default parameters other than 'leaveOpen'
+            using (var reader = new StreamReader(request.Body, Encoding.UTF8, true, 1024,
+                // Make sure StreamReader does not close the stream:
+                leaveOpen: true))
             {
-                request.Body.Position = originalPosition;
+                // This could be turned into async but at this point the stream should be buffered so no real value
+                // A custom serializer that would take the stream and read from it into the output stream would add more value
+                // as it would avoid the need to create the following (possibly huge) string
+                var body = reader.ReadToEnd();
+                return body.Length == 0
+                    ? null
+                    : body;
             }
         }
     }

src/Sentry.AspNetCore/FormRequestPayloadExtractor.cs

@@ -4,16 +4,15 @@
 
 namespace Sentry.AspNetCore
 {
-    public class FormRequestPayloadExtractor : IRequestPayloadExtractor
+    public class FormRequestPayloadExtractor : BaseRequestPayloadExtractor
     {
         private const string SupportedContentType = "application/x-www-form-urlencoded";
 
-        public object ExtractPayload(HttpRequest request)
-        {
-            return SupportedContentType
-                .Equals(request.ContentType, StringComparison.InvariantCulture)
-                ? request.Form.ToDictionary(k => k.Key, v => v.Value)
-                : null;
-        }
+        protected override bool IsSupported(HttpRequest request)
+            => SupportedContentType
+                .Equals(request.ContentType, StringComparison.InvariantCulture);
+
+        protected override object DoExtractPayLoad(HttpRequest request)
+            => request.Form?.ToDictionary(k => k.Key, v => v.Value);
     }
 }

test/Sentry.AspNetCore.Tests/BaseRequestPayloadExtractorTests.cs

@@ -0,0 +1,78 @@
+using System.IO;
+using Microsoft.AspNetCore.Http;
+using NSubstitute;
+using Xunit;
+
+namespace Sentry.AspNetCore.Tests
+{
+    public abstract class BaseRequestPayloadExtractorTests<TExtractor>
+        where TExtractor : BaseRequestPayloadExtractor, new()
+    {
+        protected class Fixture
+        {
+            public HttpRequest HttpRequest { get; set; } = Substitute.For<HttpRequest>();
+            public Stream Stream { get; set; } = Substitute.For<Stream>();
+
+            public Fixture()
+            {
+                Stream.CanSeek.Returns(true);
+                Stream.CanRead.Returns(true);
+            }
+
+            public TExtractor GetSut()
+            {
+                HttpRequest.Body.Returns(Stream);
+                return new TExtractor();
+            }
+        }
+
+        protected Fixture TestFixture = new Fixture();
+
+        [Fact]
+        public void ExtractPayload_OriginalStreamPosition_Reset()
+        {
+            const int originalPosition = 100;
+            TestFixture.Stream.Position.Returns(originalPosition);
+
+            var sut = TestFixture.GetSut();
+
+            sut.ExtractPayload(TestFixture.HttpRequest);
+
+            TestFixture.Stream.Received().Position = originalPosition;
+        }
+
+        [Fact]
+        public void ExtractPayload_OriginalStream_NotClosed()
+        {
+            var sut = TestFixture.GetSut();
+
+            sut.ExtractPayload(TestFixture.HttpRequest);
+
+            TestFixture.Stream.DidNotReceive().Close();
+        }
+
+        [Fact]
+        public void ExtractPayload_CantSeakStream_DoesNotChangePosition()
+        {
+            TestFixture.Stream.CanSeek.Returns(false);
+
+            var sut = TestFixture.GetSut();
+
+            Assert.Null(sut.ExtractPayload(TestFixture.HttpRequest));
+
+            TestFixture.Stream.DidNotReceive().Position = Arg.Any<int>();
+        }
+
+        [Fact]
+        public void ExtractPayload_CantReadStream_DoesNotChangePosition()
+        {
+            TestFixture.Stream.CanRead.Returns(false);
+
+            var sut = TestFixture.GetSut();
+
+            Assert.Null(sut.ExtractPayload(TestFixture.HttpRequest));
+
+            TestFixture.Stream.DidNotReceive().Position = Arg.Any<int>();
+        }
+    }
+}

test/Sentry.AspNetCore.Tests/DefaultRequestPayloadExtractorTests.cs

@@ -1,55 +1,10 @@
 using System.IO;
-using Microsoft.AspNetCore.Http;
-using NSubstitute;
 using Xunit;
 
 namespace Sentry.AspNetCore.Tests
 {
-    public class DefaultRequestPayloadExtractorTests
+    public class DefaultRequestPayloadExtractorTests : BaseRequestPayloadExtractorTests<DefaultRequestPayloadExtractor>
     {
-        private class Fixture
-        {
-            public HttpRequest HttpRequest { get; set; } = Substitute.For<HttpRequest>();
-            public Stream Stream { get; set; } = Substitute.For<Stream>();
-
-            public Fixture()
-            {
-                Stream.CanSeek.Returns(true);
-                Stream.CanRead.Returns(true);
-            }
-
-            public DefaultRequestPayloadExtractor GetSut()
-            {
-                HttpRequest.Body.Returns(Stream);
-                return new DefaultRequestPayloadExtractor();
-            }
-        }
-
-        private readonly Fixture _fixture = new Fixture();
-
-        [Fact]
-        public void ExtractPayload_OriginalStreamPosition_Reset()
-        {
-            const int originalPosition = 100;
-            _fixture.Stream.Position.Returns(originalPosition);
-
-            var sut = _fixture.GetSut();
-
-            sut.ExtractPayload(_fixture.HttpRequest);
-
-            _fixture.Stream.Received().Position = originalPosition;
-        }
-
-        [Fact]
-        public void ExtractPayload_OriginalStream_NotClosed()
-        {
-            var sut = _fixture.GetSut();
-
-            sut.ExtractPayload(_fixture.HttpRequest);
-
-            _fixture.Stream.DidNotReceive().Close();
-        }
-
         [Fact]
         public void ExtractPayload_StringData_ReadCorrectly()
         {
@@ -58,37 +13,13 @@ public void ExtractPayload_StringData_ReadCorrectly()
             var writer = new StreamWriter(stream);
             writer.Write(expected);
             writer.Flush();
-            _fixture.Stream = stream;
+            TestFixture.Stream = stream;
 
-            var sut = _fixture.GetSut();
+            var sut = TestFixture.GetSut();
 
-            var actual = sut.ExtractPayload(_fixture.HttpRequest);
+            var actual = sut.ExtractPayload(TestFixture.HttpRequest);
 
             Assert.Equal(expected, actual);
         }
-
-        [Fact]
-        public void ExtractPayload_CantSeakStream_DoesNotChangePosition()
-        {
-            _fixture.Stream.CanSeek.Returns(false);
-
-            var sut = _fixture.GetSut();
-
-            Assert.Null(sut.ExtractPayload(_fixture.HttpRequest));
-
-            _fixture.Stream.DidNotReceive().Position = Arg.Any<int>();
-        }
-
-        [Fact]
-        public void ExtractPayload_CantReadStream_DoesNotChangePosition()
-        {
-            _fixture.Stream.CanRead.Returns(false);
-
-            var sut = _fixture.GetSut();
-
-            Assert.Null(sut.ExtractPayload(_fixture.HttpRequest));
-
-            _fixture.Stream.DidNotReceive().Position = Arg.Any<int>();
-        }
     }
 }

test/Sentry.AspNetCore.Tests/FormRequestPayloadExtractorTests.cs

@@ -0,0 +1,47 @@
+using System.Collections.Generic;
+using Microsoft.AspNetCore.Http;
+using Microsoft.Extensions.Primitives;
+using NSubstitute;
+using Xunit;
+
+namespace Sentry.AspNetCore.Tests
+{
+    public class FormRequestPayloadExtractorTests : BaseRequestPayloadExtractorTests<FormRequestPayloadExtractor>
+    {
+        public FormRequestPayloadExtractorTests()
+        {
+            TestFixture = new Fixture();
+            TestFixture.HttpRequest.ContentType.Returns("application/x-www-form-urlencoded");
+        }
+
+        [Fact]
+        public void ExtractPayload_SuportedContentType_ReadForm()
+        {
+            var expected = new Dictionary<string, StringValues> { { "key", new StringValues("val") } };
+            var f = new FormCollection(expected);
+            TestFixture.HttpRequest.Form.Returns(f);
+
+            var sut = TestFixture.GetSut();
+
+            var actual = sut.ExtractPayload(TestFixture.HttpRequest);
+            Assert.NotNull(actual);
+
+            var actualDic = actual as IDictionary<string, StringValues>;
+            Assert.NotNull(actualDic);
+
+            Assert.Equal(expected, actualDic);
+        }
+
+        [Fact]
+        public void ExtractPayload_UnsuportedContentType_DoesNotReadStream()
+        {
+            TestFixture.HttpRequest.ContentType.Returns("application/json");
+
+            var sut = TestFixture.GetSut();
+
+            Assert.Null(sut.ExtractPayload(TestFixture.HttpRequest));
+
+            TestFixture.Stream.DidNotReceive().Position = Arg.Any<int>();
+        }
+    }
+}