From 83aea65d27cc0f572d2e64a9ba6f3707f8f60b63 Mon Sep 17 00:00:00 2001 From: Mark Rendle Date: Thu, 12 Sep 2013 12:37:55 +0100 Subject: [PATCH] Fixed Cookie handling in Pipeline and removed HandlerRunnerBuilder --- src/Simple.Web.Mocks/MockResponse.cs | 6 ++ src/Simple.Web.Tests/CookieTests.cs | 81 ++++++++----------- src/Simple.Web.sln | 17 +--- .../CodeGeneration/AsyncPipeline.cs | 19 +++++ .../CodeGeneration/CookiePropertySetter.cs | 4 +- .../CodeGeneration/PipelineFunctionFactory.cs | 41 ++++++++-- .../Hosting/HandlerRunnerFactory.cs | 50 ------------ src/Simple.Web/Simple.Web.csproj | 2 - 8 files changed, 97 insertions(+), 123 deletions(-) delete mode 100644 src/Simple.Web/Hosting/HandlerRunnerFactory.cs diff --git a/src/Simple.Web.Mocks/MockResponse.cs b/src/Simple.Web.Mocks/MockResponse.cs index efb1a99..32f6f46 100644 --- a/src/Simple.Web.Mocks/MockResponse.cs +++ b/src/Simple.Web.Mocks/MockResponse.cs @@ -1,3 +1,5 @@ +using System.Runtime.Remoting.Messaging; + namespace Simple.Web.Mocks { using System; @@ -10,6 +12,10 @@ namespace Simple.Web.Mocks public class MockResponse : IResponse { + public MockResponse() + { + Headers = new Dictionary(); + } public Status Status { get; set; } public Func WriteFunction { get; set; } public IDictionary Headers { get; set; } diff --git a/src/Simple.Web.Tests/CookieTests.cs b/src/Simple.Web.Tests/CookieTests.cs index 466adbe..79188d4 100644 --- a/src/Simple.Web.Tests/CookieTests.cs +++ b/src/Simple.Web.Tests/CookieTests.cs @@ -2,9 +2,11 @@ using System.Collections.Generic; using Simple.Web.Behaviors; using Simple.Web.CodeGeneration; +using Simple.Web.Hosting; using Simple.Web.Http; using Simple.Web.Mocks; using Xunit; +using Xunit.Sdk; namespace Simple.Web.Tests { @@ -18,17 +20,8 @@ public void LoadsSingleStringCookieUsingPropertyName() var request = new MockRequest {Headers = new Dictionary {{"Cookie", new[] {"Test=Pass"}}}}; var context = new MockContext {Request = request}; - var runner = new HandlerRunnerBuilder(typeof (SingleStringCookieHandler), "GET").BuildRunner(); - var target = new SingleStringCookieHandler(); - try - { - runner(target, context); - } - catch (ArgumentNullException) - { - // Content-type handling is going to throw an exception here. - } - Assert.Equal("Pass", target.Test); + Run(context); + Assert.Equal("Pass", SingleStringCookieHandler.TestValue); } [Fact] @@ -39,17 +32,8 @@ public void LoadsSingleGuidCookieUsingPropertyName() {Headers = new Dictionary {{"Cookie", new[] {"Test=" + expected.ToString()}}}}; var context = new MockContext {Request = request}; - var runner = new HandlerRunnerBuilder(typeof (SingleGuidCookieHandler), "GET").BuildRunner(); - var target = new SingleGuidCookieHandler(); - try - { - runner(target, context); - } - catch (ArgumentNullException) - { - // Content-type handling is going to throw an exception here. - } - Assert.Equal(expected, target.Test); + Run(context); + Assert.Equal(expected, SingleGuidCookieHandler.TestValue); } [Fact] @@ -60,17 +44,8 @@ public void LoadsSingleNullableGuidCookieUsingPropertyName() {Headers = new Dictionary {{"Cookie", new[] {"Test=" + expected.ToString()}}}}; var context = new MockContext {Request = request}; - var runner = new HandlerRunnerBuilder(typeof (SingleNullableGuidCookieHandler), "GET").BuildRunner(); - var target = new SingleNullableGuidCookieHandler(); - try - { - runner(target, context); - } - catch (ArgumentNullException) - { - // Content-type handling is going to throw an exception here. - } - Assert.Equal(expected, target.Test); + Run(context); + Assert.Equal(expected, SingleNullableGuidCookieHandler.TestValue); } [Fact] @@ -79,17 +54,8 @@ public void LoadsSingleNullGuidCookieUsingPropertyName() var request = new MockRequest(); var context = new MockContext {Request = request}; - var runner = new HandlerRunnerBuilder(typeof (SingleNullableGuidCookieHandler), "GET").BuildRunner(); - var target = new SingleNullableGuidCookieHandler(); - try - { - runner(target, context); - } - catch (ArgumentNullException) - { - // Content-type handling is going to throw an exception here. - } - Assert.False(target.Test.HasValue); + Run(context); + Assert.False(SingleNullableGuidCookieHandler.TestValue.HasValue); } //[Fact] @@ -121,8 +87,7 @@ public void SetsCookieFromSingleValueProperty() var request = new MockRequest(); var response = new MockResponse(); var context = new MockContext {Request = request, Response = response}; - var handler = new SingleStringCookieHandler {Test = "Pass"}; - Run(handler, context); + Run(context); string[] cookies; Assert.True(response.Headers.TryGetValue(HeaderKeys.SetCookie, out cookies)); Assert.True(cookies.Any(c => c.StartsWith("Test=Pass;"))); @@ -136,12 +101,13 @@ public void ParsesCookieValue() Assert.Equal("bar", value); } - private static void Run(T handler, IContext context) + private static void Run(IContext context) { - var runner = new HandlerRunnerBuilder(typeof(T), "GET").BuildRunner(); + var runner = new PipelineFunctionFactory(typeof (T)).BuildAsyncRunMethod("GET"); + var info = new HandlerInfo(typeof(T), "GET"); try { - runner(handler, context); + runner(context, info).Wait(); } catch (ArgumentNullException) { @@ -152,8 +118,10 @@ private static void Run(T handler, IContext context) class SingleStringCookieHandler : IGet { + public static string TestValue; public Status Get() { + TestValue = Test; return 200; } @@ -161,10 +129,23 @@ public Status Get() public string Test { get; set; } } + class SingleStringCookieSetHandler : IGet + { + public Status Get() + { + return 200; + } + + [Cookie] + public string Test { get { return "Pass"; } } + } + class SingleGuidCookieHandler : IGet { + public static Guid TestValue; public Status Get() { + TestValue = Test; return 200; } @@ -174,8 +155,10 @@ public Status Get() class SingleNullableGuidCookieHandler : IGet { + public static Guid? TestValue; public Status Get() { + TestValue = Test; return 200; } diff --git a/src/Simple.Web.sln b/src/Simple.Web.sln index 9a6b369..f1327ae 100644 --- a/src/Simple.Web.sln +++ b/src/Simple.Web.sln @@ -1,6 +1,8 @@  Microsoft Visual Studio Solution File, Format Version 12.00 -# Visual Studio 2012 +# Visual Studio 2013 +VisualStudioVersion = 12.0.20827.3 +MinimumVisualStudioVersion = 10.0.40219.1 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Tests", "Tests", "{71E6E108-5645-45D1-B128-E6225253924D}" EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Simple.Web", "Simple.Web\Simple.Web.csproj", "{903C289D-4CAE-4259-80DA-79D74CE06DCE}" @@ -17,8 +19,6 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Simple.Web.Ninject", "Simpl EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Simple.Web.Ninject.Tests", "Simple.Web.Ninject.Tests\Simple.Web.Ninject.Tests.csproj", "{428EAAAC-67EB-4385-A134-20E7D41CC3BE}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Simple.Web.CodeGeneration.Tests", "Simple.Web.CodeGeneration.Tests\Simple.Web.CodeGeneration.Tests.csproj", "{BC652FC8-4F9B-4E6F-86CE-1196D8D37B09}" -EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Simple.Web.Mocks", "Simple.Web.Mocks\Simple.Web.Mocks.csproj", "{D50161E6-44C7-4527-825F-9EC3755ED2BD}" EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Simple.Web.AspNet", "Simple.Web.AspNet\Simple.Web.AspNet.csproj", "{266D3482-C95C-4B2F-B7EA-F08F849084D3}" @@ -171,16 +171,6 @@ Global {428EAAAC-67EB-4385-A134-20E7D41CC3BE}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU {428EAAAC-67EB-4385-A134-20E7D41CC3BE}.Release|Mixed Platforms.Build.0 = Release|Any CPU {428EAAAC-67EB-4385-A134-20E7D41CC3BE}.Release|x86.ActiveCfg = Release|Any CPU - {BC652FC8-4F9B-4E6F-86CE-1196D8D37B09}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {BC652FC8-4F9B-4E6F-86CE-1196D8D37B09}.Debug|Any CPU.Build.0 = Debug|Any CPU - {BC652FC8-4F9B-4E6F-86CE-1196D8D37B09}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU - {BC652FC8-4F9B-4E6F-86CE-1196D8D37B09}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU - {BC652FC8-4F9B-4E6F-86CE-1196D8D37B09}.Debug|x86.ActiveCfg = Debug|Any CPU - {BC652FC8-4F9B-4E6F-86CE-1196D8D37B09}.Release|Any CPU.ActiveCfg = Release|Any CPU - {BC652FC8-4F9B-4E6F-86CE-1196D8D37B09}.Release|Any CPU.Build.0 = Release|Any CPU - {BC652FC8-4F9B-4E6F-86CE-1196D8D37B09}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU - {BC652FC8-4F9B-4E6F-86CE-1196D8D37B09}.Release|Mixed Platforms.Build.0 = Release|Any CPU - {BC652FC8-4F9B-4E6F-86CE-1196D8D37B09}.Release|x86.ActiveCfg = Release|Any CPU {D50161E6-44C7-4527-825F-9EC3755ED2BD}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {D50161E6-44C7-4527-825F-9EC3755ED2BD}.Debug|Any CPU.Build.0 = Debug|Any CPU {D50161E6-44C7-4527-825F-9EC3755ED2BD}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU @@ -399,7 +389,6 @@ Global {29FBE3C3-4CB4-4625-8AA7-C748E1F0D2BA} = {71E6E108-5645-45D1-B128-E6225253924D} {FD018C10-D36C-4C40-8403-07D1F0CCA328} = {71E6E108-5645-45D1-B128-E6225253924D} {428EAAAC-67EB-4385-A134-20E7D41CC3BE} = {71E6E108-5645-45D1-B128-E6225253924D} - {BC652FC8-4F9B-4E6F-86CE-1196D8D37B09} = {71E6E108-5645-45D1-B128-E6225253924D} {D50161E6-44C7-4527-825F-9EC3755ED2BD} = {71E6E108-5645-45D1-B128-E6225253924D} {730BBC21-8495-4825-988B-FCF4D2AE5742} = {71E6E108-5645-45D1-B128-E6225253924D} {81F43A2B-238C-43D1-9D11-621DDD1A7817} = {71E6E108-5645-45D1-B128-E6225253924D} diff --git a/src/Simple.Web/CodeGeneration/AsyncPipeline.cs b/src/Simple.Web/CodeGeneration/AsyncPipeline.cs index 0778ce5..a7cda8f 100644 --- a/src/Simple.Web/CodeGeneration/AsyncPipeline.cs +++ b/src/Simple.Web/CodeGeneration/AsyncPipeline.cs @@ -33,6 +33,11 @@ public static MethodInfo ContinueWithAsyncBlockMethod(Type handlerType) return GetMethod("ContinueWithAsyncBlock", handlerType); } + public static MethodInfo ContinueWithActionMethod(Type handlerType) + { + return GetMethod("ContinueWithAction", handlerType); + } + private static MethodInfo GetMethod(string name, Type handlerType) { return typeof (AsyncPipeline).GetMethod(name, BindingFlags.NonPublic | BindingFlags.Static).MakeGenericMethod(handlerType); @@ -75,6 +80,20 @@ private static Task ContinueWithHandler(Task task, Func ContinueWithAction(Task task, Action continuation, IContext context, + THandler handler) + { + return task.ContinueWith(t => + { + if (t.Result) + { + continuation(handler, context); + return true; + } + return false; + }, TaskContinuationOptions.OnlyOnRanToCompletion); + } + private static Task ContinueWithAsyncHandler(Task task, Func> continuation, IContext context, THandler handler) { return task.ContinueWith(t => diff --git a/src/Simple.Web/CodeGeneration/CookiePropertySetter.cs b/src/Simple.Web/CodeGeneration/CookiePropertySetter.cs index 2d3f1d2..b3297d0 100644 --- a/src/Simple.Web/CodeGeneration/CookiePropertySetter.cs +++ b/src/Simple.Web/CodeGeneration/CookiePropertySetter.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Linq.Expressions; using System.Reflection; @@ -15,7 +16,7 @@ internal class CookiePropertySetter internal static IEnumerable GetCookiePropertySetters(Type type, ParameterExpression handler, ParameterExpression context) { - foreach (var cookieProperty in type.GetProperties().Where(p => Attribute.GetCustomAttribute(p, typeof(CookieAttribute)) != null)) + foreach (var cookieProperty in type.GetProperties().Where(p => p.CanWrite && Attribute.GetCustomAttribute(p, typeof(CookieAttribute)) != null)) { var attribute = (CookieAttribute)Attribute.GetCustomAttribute(cookieProperty, typeof(CookieAttribute)); var property = Expression.Property(handler, cookieProperty); @@ -92,6 +93,7 @@ private static string GetCookieValue(IContext context, string name) { string value; context.Request.TryGetCookieValue(name, out value); + Trace.WriteLine("GetCookieValue"); return value; } } diff --git a/src/Simple.Web/CodeGeneration/PipelineFunctionFactory.cs b/src/Simple.Web/CodeGeneration/PipelineFunctionFactory.cs index b64dc7e..2295d4e 100644 --- a/src/Simple.Web/CodeGeneration/PipelineFunctionFactory.cs +++ b/src/Simple.Web/CodeGeneration/PipelineFunctionFactory.cs @@ -1,5 +1,6 @@ using System.Collections; using System.Collections.Concurrent; +using System.Web.Handlers; using Simple.Web.Behaviors.Implementations; namespace Simple.Web.CodeGeneration @@ -52,7 +53,8 @@ public static Func Get(Type handlerType, string htt } } - // It's not really worth all the locking palaver here, worst case scenario the AsyncRunMethod gets built more than once. + // It's not really worth all the locking palaver here, + // worst case scenario the AsyncRunMethod gets built more than once. Func method; if (!handlerCache.TryGetValue(httpMethod, out method)) { @@ -80,13 +82,25 @@ public Func BuildAsyncRunMethod(string httpMethod) var blocks = new List(); blocks.AddRange(CreateBlocks(GetSetupBehaviorInfos())); - var setCookieProperties = CookiePropertySetter.GetCookiePropertySetters(_handlerType, _handler, _context); - blocks.AddRange(setCookieProperties); + + var buildAction = GetType().GetMethod("BuildAction", BindingFlags.NonPublic | BindingFlags.Instance).MakeGenericMethod(_handlerType); + + var setCookieProperties = CookiePropertySetter.GetCookiePropertySetters(_handlerType, _handler, _context).ToList(); + if (setCookieProperties.Any()) + { + var setCookieDelegate = buildAction.Invoke(this, new object[] {setCookieProperties}); + blocks.Add(setCookieDelegate); + } var second = new HandlerBlock(_handlerType, GetRunMethod(httpMethod)); blocks.Add(second); - var setPropertyCookies = PropertyCookieSetter.GetPropertyCookieSetters(_handlerType, _handler, _context); - blocks.AddRange(setPropertyCookies); + + var setPropertyCookies = PropertyCookieSetter.GetPropertyCookieSetters(_handlerType, _handler, _context).ToList(); + if (setPropertyCookies.Any()) + { + var setPropertyDelegate = buildAction.Invoke(this, new object[] {setPropertyCookies}); + blocks.Add(setPropertyDelegate); + } var redirectBehavior = new ResponseBehaviorInfo(typeof (object), typeof (Redirect2), Priority.High) { Universal = true }; @@ -152,6 +166,9 @@ private static IEnumerable CreateBlocks(IEnumerable private Expression BuildCallExpression(IEnumerable blocks) { + HandlerBlock handlerBlock; + PipelineBlock pipelineBlock; + Expression call = Expression.Call(AsyncPipeline.DefaultStartMethod); foreach (var block in blocks) @@ -165,12 +182,17 @@ private Expression BuildCallExpression(IEnumerable blocks) { call = BuildCallHandlerExpression(block, call); } - else + else if ((pipelineBlock = block as PipelineBlock) != null) { call = Expression.Call(AsyncPipeline.ContinueWithAsyncBlockMethod(_handlerType), call, - Expression.Constant(((PipelineBlock)block).Generate(_handlerType)), + Expression.Constant((pipelineBlock).Generate(_handlerType)), _context, _handler); } + else + { + call = Expression.Call(AsyncPipeline.ContinueWithActionMethod(_handlerType), call, + Expression.Constant(block), _context, _handler); + } } return call; } @@ -228,5 +250,10 @@ private bool HandlerHasBehavior(BehaviorInfo behaviorInfo) } return behaviorInfo.BehaviorType.IsAssignableFrom(_handlerType); } + + private Action BuildAction(IEnumerable blocks) + { + return Expression.Lambda>(Expression.Block(blocks), _handler, _context).Compile(); + } } } \ No newline at end of file diff --git a/src/Simple.Web/Hosting/HandlerRunnerFactory.cs b/src/Simple.Web/Hosting/HandlerRunnerFactory.cs deleted file mode 100644 index 8f32cc5..0000000 --- a/src/Simple.Web/Hosting/HandlerRunnerFactory.cs +++ /dev/null @@ -1,50 +0,0 @@ -namespace Simple.Web.Hosting -{ - using System; - using System.Collections.Concurrent; - using Simple.Web.CodeGeneration; - using Simple.Web.Http; - - /// - /// Factory class for building runners. - /// - internal class HandlerRunnerFactory - { - private readonly ConcurrentDictionary> _cache = - new ConcurrentDictionary>(); - - private readonly ConcurrentDictionary _asyncCache = - new ConcurrentDictionary(); - - /// - /// Singleton instance. - /// - public static readonly HandlerRunnerFactory Instance = new HandlerRunnerFactory(); - - private HandlerRunnerFactory() - { - } - - /// - /// Gets the synchronous runner for the specified handler type. - /// - /// Type of the handler. - /// The HTTP method. - /// - public Action Get(Type handlerType, string httpMethod) - { - return _cache.GetOrAdd(handlerType, t => new HandlerRunnerBuilder(t, httpMethod).BuildRunner()); - } - - /// - /// Gets the asynchronous runner for the specified handler type. - /// - /// Type of the handler. - /// The HTTP method. - /// - public AsyncRunner GetAsync(Type handlerType, string httpMethod) - { - return _asyncCache.GetOrAdd(handlerType, t => new HandlerRunnerBuilder(t, httpMethod).BuildAsyncRunner()); - } - } -} diff --git a/src/Simple.Web/Simple.Web.csproj b/src/Simple.Web/Simple.Web.csproj index f6e6c3b..008d955 100644 --- a/src/Simple.Web/Simple.Web.csproj +++ b/src/Simple.Web/Simple.Web.csproj @@ -127,7 +127,6 @@ - @@ -147,7 +146,6 @@ -