From b6c6e425343925b9f7a464ee43c3f80cf93c8cec Mon Sep 17 00:00:00 2001 From: Immo Wache Date: Wed, 14 Oct 2009 22:35:25 +0200 Subject: [PATCH] Nasty bug in RubyEngineSpec and RubyControllerFactorySpec.cs fixed. When RubyEngine is created with RubyEngine.InitializeIronRubyMvc, the original IControllerFactory from MVC must be restored after every observation. Otherwise you get with every observation a chain of ControllerFactories with undeterminated behaviour. Tests in Path RubyControllerFactorySpec.cs refactored to reduce inheritance, and make the tests more straightward. RubyEngine.RequireRubyFile(path) implementation extended with use of PathProvider.MapPath. Empty RubyEngine constructor removed, no longer needed for xunit test. --- .../Controllers/RubyControllerFactorySpec.cs | 277 +++++++++--------- IronRubyMvc.Tests/Core/RubyEngineSpec.cs | 23 +- .../Controllers/RubyControllerFactory.cs | 228 +++++++------- IronRubyMvc/Core/RubyEngine.cs | 13 +- 4 files changed, 275 insertions(+), 266 deletions(-) diff --git a/IronRubyMvc.Tests/Controllers/RubyControllerFactorySpec.cs b/IronRubyMvc.Tests/Controllers/RubyControllerFactorySpec.cs index 9a69daf..5f840b2 100644 --- a/IronRubyMvc.Tests/Controllers/RubyControllerFactorySpec.cs +++ b/IronRubyMvc.Tests/Controllers/RubyControllerFactorySpec.cs @@ -14,36 +14,29 @@ namespace System.Web.Mvc.IronRuby.Tests.Controllers { - public abstract class with_ironruby_mvc_and_routes_file : InstanceContextSpecification + [Concern(typeof(RubyControllerFactory))] + public abstract class with_ironruby_mvc_and_routes_and_controller_file : InstanceContextSpecification { - protected IPathProvider _pathProvider; - protected IRubyEngine _rubyEngine; - protected RequestContext _requestContext; - - protected override void EstablishContext() + /// + /// Create a text file with content from value + /// + /// full text file path + /// text file content + protected void CreateFile(string path, string value) { - base.EstablishContext(); - - //create a routes.rb file in current directory - createRoutesFile("routes.rb"); - - _pathProvider = An(); - _pathProvider.WhenToldTo(pp => pp.ApplicationPhysicalPath).Return(Environment.CurrentDirectory); - _pathProvider.WhenToldTo(pp => pp.FileExists("~/routes.rb")).Return(true); - _pathProvider.WhenToldTo(pp => pp.MapPath("~/routes.rb")).Return("routes.rb"); - RouteTable.Routes.Clear(); - - _requestContext = new RequestContext(new Mock().Object, new RouteData()); - - RubyEngine _theRubyEngine = RubyEngine.InitializeIronRubyMvc(_pathProvider, "~/routes.rb"); - _rubyEngine = _theRubyEngine; + FileStream fs = new FileStream(path, FileMode.Create); + BinaryWriter bw = new BinaryWriter(fs); + bw.Write(ASCIIEncoding.Default.GetBytes(value)); + bw.Flush(); + bw.Close(); + fs.Close(); } - + /// /// create a default routes file in path /// /// full path name for the routes file to create - private void createRoutesFile(string path) + protected virtual void CreateRoutesFile(string path) { var script = new StringBuilder(); @@ -60,68 +53,76 @@ private void createRoutesFile(string path) } /// - /// Creates a file with content value in path + /// Creates a ruby controller file with content value in path /// /// file full path name /// file content as string - protected void CreateFile(string path, string value) + protected virtual void CreateControllerFile(string path, string controllerName) { - FileStream fs = new FileStream(path, FileMode.Create); - BinaryWriter bw = new BinaryWriter(fs); - bw.Write(ASCIIEncoding.Default.GetBytes(value)); - bw.Flush(); - bw.Close(); - fs.Close(); - } - } + var script = new StringBuilder(); + script.AppendLine("class {0} < Controller".FormattedWith(controllerName)); + script.AppendLine(" def my_action"); + script.AppendLine(" $counter = $counter + 5"); + script.AppendLine(" \"Can't see ninjas\".to_clr_string"); + script.AppendLine(" end"); + script.AppendLine("end"); + string value = script.ToString(); - [Concern(typeof(RubyControllerFactory))] - public abstract class with_ironruby_mvc_and_routes_and_controller_file : with_ironruby_mvc_and_routes_file - { - protected const string _controllerName = "My"; - protected const string _controllerClassName = _controllerName + "Controller"; + CreateFile(path, value); + } - protected string _mappedControllerPath = _controllerClassName + ".rb"; - protected string _virtualControllerPath = @"~\Controllers\{0}.rb" - .FormattedWith(_controllerClassName); + private IControllerFactory originalFactory; - protected IControllerFactory _controllerFactory; - protected IController _controller; + protected IRubyEngine _rubyEngine; + protected IPathProvider _pathProvider; + protected RequestContext _requestContext; + + protected const string _controllerName = "My"; + protected const string _controllerClassName = "MyController"; + protected const string _mappedControllerPath = "MyController.rb"; + protected const string _virtualControllerPath = "~\\Controllers\\MyController.rb"; protected override void EstablishContext() { - base.EstablishContext(); - - createControllerFile(_mappedControllerPath, _controllerClassName); + //create a routes.rb and a ruby controller file in current directory + CreateRoutesFile("routes.rb"); + CreateControllerFile(_mappedControllerPath, _controllerClassName); + _pathProvider = An(); + //routes.rb + _pathProvider.WhenToldTo(pp => pp.ApplicationPhysicalPath).Return(Environment.CurrentDirectory); + _pathProvider.WhenToldTo(pp => pp.FileExists("~/routes.rb")).Return(true); + _pathProvider.WhenToldTo(pp => pp.MapPath("~/routes.rb")).Return("routes.rb"); + //MyController.rb _pathProvider.WhenToldTo(pp => pp.FileExists(_virtualControllerPath)).Return(true); _pathProvider.WhenToldTo(pp => pp.MapPath(_virtualControllerPath)).Return(_mappedControllerPath); + + RouteTable.Routes.Clear(); + _requestContext = new RequestContext(new Mock().Object, new RouteData()); + + //save the original controller factory to avoid chaining all test factories + originalFactory = ControllerBuilder.Current.GetControllerFactory(); + + _rubyEngine = RubyEngine.InitializeIronRubyMvc(_pathProvider, "~/routes.rb"); } - protected virtual void createControllerFile(string path, string controllerName) + protected override RubyControllerFactory CreateSut() { - var script = new StringBuilder(); - script.AppendLine("class {0} < Controller".FormattedWith(controllerName)); - script.AppendLine(" def my_action"); - script.AppendLine(" $counter = $counter + 5"); - script.AppendLine(" \"Can't see ninjas\".to_clr_string"); - script.AppendLine(" end"); - script.AppendLine("end"); - string value = script.ToString(); + return (RubyControllerFactory)ControllerBuilder.Current.GetControllerFactory(); + } - CreateFile(path, value); + protected override void AfterEachObservation() + { + //restore the original controller factory to avoid chaining of all test factories + ControllerBuilder.Current.SetControllerFactory(originalFactory); } } - [Concern(typeof(RubyControllerFactory))] public class when_a_ruby_controller_needs_to_be_resolved : with_ironruby_mvc_and_routes_and_controller_file { - protected override RubyControllerFactory CreateSut() - { - return (RubyControllerFactory)ControllerBuilder.Current.GetControllerFactory(); - } - + private IController _controller; + protected override void Because() { _controller = Sut.CreateController(_requestContext, _controllerName); @@ -150,18 +151,6 @@ public void should_have_the_correct_controller_class_name() { (_controller as RubyController).ControllerClassName.ShouldBeEqualTo(_controllerClassName); } - - //[Observation] - //public void it_should_have_called_the_ruby_engine() - //{ - // _rubyEngine.WasToldTo(eng => eng.GetRubyClass(_controllerName)).OnlyOnce(); - //} - - //[Observation] - //public void should_have_called_the_inner_controller_factory() - //{ - // _controllerFactory.WasToldTo(factory => factory.CreateController(_requestContext, _controllerName)).OnlyOnce(); - //} } [Concern(typeof(RubyControllerFactory))] @@ -169,19 +158,16 @@ public class when_a_ruby_controller_was_resolved_twice : with_ironruby_mvc_and_r { private const string methodToFilter = "index"; - private int actionFiltersCountFirst; - private int actionFiltersCountSecond; - - protected override void createControllerFile(string path, string controllerName) + protected override void CreateControllerFile(string path, string controllerName) { var script = new StringBuilder(); - script.AppendLine("class {0} < Controller".FormattedWith(controllerName)); + script.AppendLine("class {0} < Controller".FormattedWith(_controllerClassName)); script.AppendLine(""); script.AppendLine(" before_action :index do |context|"); script.AppendLine(" context.request_context.http_context.response.write(\"Hello world
\")"); script.AppendLine(" end"); script.AppendLine(""); - script.AppendLine(" def {0}".FormattedWith(methodToFilter)); + script.AppendLine(" def index"); script.AppendLine(" $counter = $counter + 5"); script.AppendLine(" \"Can't see ninjas\".to_clr_string"); script.AppendLine(" end"); @@ -191,10 +177,9 @@ protected override void createControllerFile(string path, string controllerName) CreateFile(path, value); } - protected override RubyControllerFactory CreateSut() - { - return (RubyControllerFactory) ControllerBuilder.Current.GetControllerFactory(); - } + private int actionFiltersCountFirst; + private int actionFiltersCountSecond; + private IController _controller; protected override void Because() { @@ -222,64 +207,72 @@ public void action_filters_count_should_be_equal() } - -// [Concern(typeof(RubyControllerFactory))] -// public class when_a_ruby_controller_needs_to_be_resolved : InstanceContextSpecification -// { -// private IRubyEngine _rubyEngine; -// private IControllerFactory _controllerFactory; -// private RequestContext _requestContext; -// private const string _controllerName = "my_controller"; -// private IController _controller; -// -// protected override void EstablishContext() -// { -// _rubyEngine = Dependency(); -// _controllerFactory = Dependency(); -// _requestContext = new RequestContext(new HttpContextMock().Object, new RouteData()); -// -// _controllerFactory -// .WhenToldTo(factory => factory.CreateController(_requestContext, _controllerName)) -// .Throw(new InvalidOperationException()); -// -// _rubyEngine.WhenToldTo(eng => eng.LoadController(_requestContext, _controllerName)).Return(Dependency()); -// -// } -// -// protected override RubyControllerFactory CreateSut() -// { -// return new RubyControllerFactory(_controllerFactory, _rubyEngine); -// } -// -// protected override void Because() -// { -// _controller = Sut.CreateController(_requestContext, _controllerName); -// } -// -// [Observation] -// public void should_have_returned_a_result() -// { -// _controller.ShouldNotBeNull(); -// } -// -// [Observation] -// public void should_have_returned_a_controller() -// { -// _controller.ShouldBeAnInstanceOf(); -// } -// -// [Observation] -// public void it_should_have_called_the_ruby_engine() -// { -// _rubyEngine.WasToldTo(eng => eng.LoadController(_requestContext, _controllerName)).OnlyOnce(); -// } -// -// [Observation] -// public void should_have_called_the_inner_controller_factory() -// { -// _controllerFactory.WasToldTo(factory => factory.CreateController(_requestContext, _controllerName)).OnlyOnce(); -// } -// } + //[Concern(typeof(RubyControllerFactory))] + //public class when_a_controller_needs_to_be_resolved : InstanceContextSpecification + //{ + // private IRubyEngine _rubyEngine; + // private IControllerFactory _controllerFactory; + // private IPathProvider _pathProvider; + // private RequestContext _requestContext; + // private const string _controllerName = "my_controller"; + // private IController _controller; + + // private string requirePath; + + // protected override void EstablishContext() + // { + // _pathProvider = An(); + // _rubyEngine = An(); + // _controllerFactory = An(); + // _requestContext = new RequestContext(new HttpContextMock().Object, new RouteData()); + + // _controllerFactory + // .WhenToldTo(factory => factory.CreateController(_requestContext, _controllerName)) + // .Throw(new InvalidOperationException()); + + // _rubyEngine.WhenToldTo(eng => eng.RequireRubyFile(requirePath)); + // } + + // protected override RubyControllerFactory CreateSut() + // { + // return new RubyControllerFactory(_pathProvider, _controllerFactory, _rubyEngine); + // } + + // protected override void Because() + // { + // _controller = Sut.CreateController(_requestContext, _controllerName); + // } + + // [Observation] + // public void should_have_returned_a_result() + // { + // _controller.ShouldNotBeNull(); + // } + + // [Observation] + // public void should_have_returned_a_controller() + // { + // _controller.ShouldBeAnInstanceOf(); + // } + + // [Observation] + // public void it_should_have_called_the_ruby_engine() + // { + // _rubyEngine.WasToldTo(eng => eng.RequireRubyFile(requirePath)).OnlyOnce(); + // } + + // [Observation] + // public void it_should_have_require_path_from_ruby_engine() + // { + // requirePath.ShouldBeEqualTo("gaga_gaga"); + // } + + // [Observation] + // public void should_have_called_the_inner_controller_factory() + // { + // _controllerFactory.WasToldTo(factory => factory.CreateController(_requestContext, _controllerName)).OnlyOnce(); + // } + //} [Concern(typeof(RubyControllerFactory))] public class when_a_ruby_controller_needs_to_be_disposed: InstanceContextSpecification diff --git a/IronRubyMvc.Tests/Core/RubyEngineSpec.cs b/IronRubyMvc.Tests/Core/RubyEngineSpec.cs index 23fd8ef..5073402 100644 --- a/IronRubyMvc.Tests/Core/RubyEngineSpec.cs +++ b/IronRubyMvc.Tests/Core/RubyEngineSpec.cs @@ -24,6 +24,8 @@ namespace System.Web.Mvc.IronRuby.Tests.Core [Concern(typeof(RubyEngine))] public class when_asked_to_initialize_ironruby_mvc_with_existing_routes_file : StaticContextSpecification { + private IControllerFactory originalFactory; + private RubyEngine _engine; private IPathProvider _pathProvider; @@ -40,6 +42,9 @@ protected override void EstablishContext() _pathProvider.WhenToldTo(pp => pp.MapPath("~/routes.rb")).Return("routes.rb"); // SetupResult.For(RouteTable.Routes).Return(new RouteCollection()); RouteTable.Routes.Clear(); + + //save the original controller factory to avoid chaining all test factories + originalFactory = ControllerBuilder.Current.GetControllerFactory(); } /// @@ -114,11 +119,20 @@ public void should_have_a_ruby_view_engine() }); passes.ShouldBeTrue(); } + + protected override void AfterEachObservation() + { + //restore the original controller factory to avoid chaining of all test factories + ControllerBuilder.Current.SetControllerFactory(originalFactory); + } + } [Concern(typeof(RubyEngine))] public class when_asked_to_initialize_ironruby_mvc_with_non_existing_routes_file : StaticContextSpecification { + private IControllerFactory originalFactory; + private RubyEngine _engine; private IPathProvider _pathProvider; @@ -127,6 +141,9 @@ protected override void EstablishContext() _pathProvider = An(); _pathProvider.WhenToldTo(pp => pp.ApplicationPhysicalPath).Return(Environment.CurrentDirectory); _pathProvider.WhenToldTo(pp => pp.FileExists("~/routes.rb")).Return(false); + + //save the original controller factory to avoid chaining all test factories + originalFactory = ControllerBuilder.Current.GetControllerFactory(); } protected override void Because() @@ -141,7 +158,11 @@ public void it_should_not_have_a_global_routes_variable() _engine.Context.GetGlobalVariable("routes").ShouldBeNull(); } - + protected override void AfterEachObservation() + { + //restore the original controller factory to avoid chaining of all test factories + ControllerBuilder.Current.SetControllerFactory(originalFactory); + } } public abstract class with_an_engine_initialized : with_ironruby_initialized diff --git a/IronRubyMvc/Controllers/RubyControllerFactory.cs b/IronRubyMvc/Controllers/RubyControllerFactory.cs index c75f31d..3819c38 100644 --- a/IronRubyMvc/Controllers/RubyControllerFactory.cs +++ b/IronRubyMvc/Controllers/RubyControllerFactory.cs @@ -1,115 +1,115 @@ -#region Usings - -using System.Web.Mvc.IronRuby.Core; -using System.Web.Mvc.IronRuby.Extensions; -using System.Web.Routing; -using IronRuby.Builtins; - -#endregion - -namespace System.Web.Mvc.IronRuby.Controllers -{ - public class RubyControllerFactory : IControllerFactory - { - private readonly IRubyEngine _engine; - private readonly IControllerFactory _innerFactory; - private readonly IPathProvider _pathProvider; - - public RubyControllerFactory(IPathProvider pathProvider, IControllerFactory innerFactory, IRubyEngine engine) - { - _pathProvider = pathProvider; - _innerFactory = innerFactory; - _engine = engine; - } - - #region IControllerFactory Members - - public IController CreateController(RequestContext requestContext, string controllerName) - { - try - { - return _innerFactory.CreateController(requestContext, controllerName); - } - catch (InvalidOperationException) - { - } - catch (HttpException) - { - } - - return LoadController(requestContext, controllerName); - } - - - public void ReleaseController(IController controller) - { - var disposable = controller as IDisposable; - - if (disposable != null) - disposable.Dispose(); - } - - #endregion - - /// - /// Loads the controller. - /// - /// The request context. - /// Name of the controller. - /// - private RubyController LoadController(RequestContext requestContext, string controllerName) - { - var controllerFilePath = GetControllerFilePath(controllerName); - var controllerClassName = GetControllerClassName(controllerName); - -//_engine.RemoveClassFromGlobals(controllerClassName); - - if (controllerFilePath.IsNullOrBlank()) - return null; - - _engine.RequireRubyFile(controllerFilePath); - - var controllerClass = _engine.GetRubyClass(controllerClassName); - var controller = ConfigureController(controllerClass, requestContext); - - return controller; - } - - - /// - /// Configures the controller. - /// - /// The ruby class. - /// The request context. - /// - private RubyController ConfigureController(RubyClass rubyClass, RequestContext requestContext) - { - var controller = (RubyController)_engine.CreateInstance(rubyClass); - controller.InternalInitialize(new ControllerConfiguration {Context = requestContext, Engine = _engine, RubyClass = rubyClass}); - return controller; - } - - /// - /// Gets the name of the controller class. - /// - /// Name of the controller. - /// - private static string GetControllerClassName(string controllerName) - { - return (controllerName.EndsWith("Controller", StringComparison.OrdinalIgnoreCase) - ? controllerName - : Constants.ControllerclassFormat.FormattedWith(controllerName)).Pascalize(); - } - - private string GetControllerFilePath(string controllerName) - { - var fileName = Constants.ControllerPascalPathFormat.FormattedWith(controllerName.Pascalize()); - if (_pathProvider.FileExists(fileName)) - return fileName; - - fileName = Constants.ControllerUnderscorePathFormat.FormattedWith(controllerName.Underscore()); - - return _pathProvider.FileExists(fileName) ? fileName : string.Empty; - } - } +#region Usings + +using System.Web.Mvc.IronRuby.Core; +using System.Web.Mvc.IronRuby.Extensions; +using System.Web.Routing; +using IronRuby.Builtins; + +#endregion + +namespace System.Web.Mvc.IronRuby.Controllers +{ + public class RubyControllerFactory : IControllerFactory + { + private readonly IRubyEngine _engine; + private readonly IControllerFactory _innerFactory; + private readonly IPathProvider _pathProvider; + + public RubyControllerFactory(IPathProvider pathProvider, IControllerFactory innerFactory, IRubyEngine engine) + { + _pathProvider = pathProvider; + _innerFactory = innerFactory; + _engine = engine; + } + + #region IControllerFactory Members + + public IController CreateController(RequestContext requestContext, string controllerName) + { + try + { + return _innerFactory.CreateController(requestContext, controllerName); + } + catch (InvalidOperationException) + { + } + catch (HttpException) + { + } + + return LoadController(requestContext, controllerName); + } + + + public void ReleaseController(IController controller) + { + var disposable = controller as IDisposable; + + if (disposable != null) + disposable.Dispose(); + } + + #endregion + + /// + /// Loads the controller. + /// + /// The request context. + /// Name of the controller. + /// + private RubyController LoadController(RequestContext requestContext, string controllerName) + { + var controllerFilePath = GetControllerFilePath(controllerName); + var controllerClassName = GetControllerClassName(controllerName); + +//_engine.RemoveClassFromGlobals(controllerClassName); + + if (controllerFilePath.IsNullOrBlank()) + return null; + + _engine.RequireRubyFile(controllerFilePath); + + var controllerClass = _engine.GetRubyClass(controllerClassName); + var controller = ConfigureController(controllerClass, requestContext); + + return controller; + } + + + /// + /// Configures the controller. + /// + /// The ruby class. + /// The request context. + /// + private RubyController ConfigureController(RubyClass rubyClass, RequestContext requestContext) + { + var controller = (RubyController)_engine.CreateInstance(rubyClass); + controller.InternalInitialize(new ControllerConfiguration {Context = requestContext, Engine = _engine, RubyClass = rubyClass}); + return controller; + } + + /// + /// Gets the name of the controller class. + /// + /// Name of the controller. + /// + private static string GetControllerClassName(string controllerName) + { + return (controllerName.EndsWith("Controller", StringComparison.OrdinalIgnoreCase) + ? controllerName + : Constants.ControllerclassFormat.FormattedWith(controllerName)).Pascalize(); + } + + private string GetControllerFilePath(string controllerName) + { + var fileName = Constants.ControllerPascalPathFormat.FormattedWith(controllerName.Pascalize()); + if (_pathProvider.FileExists(fileName)) + return fileName; + + fileName = Constants.ControllerUnderscorePathFormat.FormattedWith(controllerName.Underscore()); + + return _pathProvider.FileExists(fileName) ? fileName : string.Empty; + } + } } \ No newline at end of file diff --git a/IronRubyMvc/Core/RubyEngine.cs b/IronRubyMvc/Core/RubyEngine.cs index 442af2b..2b80bd3 100644 --- a/IronRubyMvc/Core/RubyEngine.cs +++ b/IronRubyMvc/Core/RubyEngine.cs @@ -31,11 +31,6 @@ public class RubyEngine : IRubyEngine { private readonly string _routesPath; - //added for xunit testing to create a "The" moc - public RubyEngine() - { - } - /// /// Initializes a new instance of the class. /// @@ -237,7 +232,7 @@ public void ExecuteFile(string path, bool throwIfNotExist) if (!PathProvider.FileExists(path)) return; - HandleError(() => Engine.ExecuteFile(PathProvider.MapPath(path), CurrentScope)); + HandleError(() => Engine.ExecuteFile(PathProvider.MapPath(path), CurrentScope)); //HandleError(() => Engine.CreateOperations(CurrentScope).InvokeMember(null, "require", path)); } @@ -290,8 +285,9 @@ public void LoadAssemblies(params Type[] assemblies) /// The path. public void RequireRubyFile(string path) { - //Engine.RequireRubyFile(path); - ExecuteScript("require '" + path.Replace("\\","/").Replace("~", string.Empty) + "'", CurrentScope); + //Engine.RequireRubyFile(path); + path = PathProvider.MapPath(path).Replace('\\', '/').Replace("~", string.Empty); + ExecuteScript(String.Format("require '{0}'", path), CurrentScope); } /// @@ -415,7 +411,6 @@ private static RubyEngine InitializeIronRuby(IPathProvider vpp, string routesPat return new RubyEngine(runtime, vpp, routesPath); } - public T GetGlobalVariable(string name) { return (T)GetGlobalVariable(name);