From 020e0c17b8495f46b6748921bbc2a1005cd4767f Mon Sep 17 00:00:00 2001 From: cneuwirt Date: Fri, 28 Sep 2007 21:13:38 +0000 Subject: [PATCH] Fixed IOC-80 by considering parameter dependencies for valid handler states. git-svn-id: https://svn.castleproject.org/svn/castle/trunk@4361 73e77b4c-caa6-f847-a29a-24ab75ae54b6 --- .../Model/ConstructorCandidateCollection.cs | 17 ++++- .../Startable/StartableFacilityTestCase.cs | 2 +- .../RuntimeParametersTestCase.cs | 3 +- .../UnsatisfiedDependenciesTestCase.cs | 8 ++- .../Castle.MicroKernel/DefaultKernel.cs | 18 +++-- .../Facilities/Startable/StartableFacility.cs | 83 ++++++++++++---------- .../Castle.MicroKernel/Handlers/AbstractHandler.cs | 8 ++- .../ModelBuilder/DefaultComponentModelBuilder.cs | 2 +- .../ConstructorDependenciesModelInspector.cs | 24 +++++-- .../Proxy/FactorySupportTestCase.cs | 67 +++++++++++++++++ .../Proxy/ProxyBehaviorInvalidTestCase.cs | 2 +- .../Proxy/ProxyBehaviorTestCase.cs | 2 +- .../Proxy/TypedFactoryFacilityTestCase.cs | 2 +- .../Castle.Windsor.Tests/Proxy/typedFactory.xml | 2 +- .../Proxy/typedFactoryCreateWithoutId.xml | 2 +- 15 files changed, 176 insertions(+), 66 deletions(-) create mode 100644 InversionOfControl/Castle.Windsor.Tests/Proxy/FactorySupportTestCase.cs diff --git a/Core/Castle.Core/Model/ConstructorCandidateCollection.cs b/Core/Castle.Core/Model/ConstructorCandidateCollection.cs index 1a4b54292..be0b42f4d 100644 --- a/Core/Castle.Core/Model/ConstructorCandidateCollection.cs +++ b/Core/Castle.Core/Model/ConstructorCandidateCollection.cs @@ -23,6 +23,7 @@ namespace Castle.Core [Serializable] public class ConstructorCandidateCollection : ReadOnlyCollectionBase { + private bool hasAmbiguousFewerArgumentsCandidate; private ConstructorCandidate fewerArgumentsCandidate; /// @@ -34,14 +35,21 @@ namespace Castle.Core if (fewerArgumentsCandidate == null) { fewerArgumentsCandidate = candidate; + hasAmbiguousFewerArgumentsCandidate = false; } else { - if (candidate.Constructor.GetParameters().Length < - fewerArgumentsCandidate.Constructor.GetParameters().Length) + int constructorParamCount = candidate.Constructor.GetParameters().Length; + int fewerArgumentsCount = fewerArgumentsCandidate.Constructor.GetParameters().Length; + + if (constructorParamCount < fewerArgumentsCount) { fewerArgumentsCandidate = candidate; } + else if (constructorParamCount == fewerArgumentsCount) + { + hasAmbiguousFewerArgumentsCandidate = true; + } } InnerList.Add(candidate); @@ -56,6 +64,11 @@ namespace Castle.Core get { return fewerArgumentsCandidate; } } + public bool HasAmbiguousFewerArgumentsCandidate + { + get { return hasAmbiguousFewerArgumentsCandidate; } + } + /// /// Clears this instance. /// diff --git a/InversionOfControl/Castle.MicroKernel.Tests/Facilities/Startable/StartableFacilityTestCase.cs b/InversionOfControl/Castle.MicroKernel.Tests/Facilities/Startable/StartableFacilityTestCase.cs index 15c6c742e..ffdc54598 100644 --- a/InversionOfControl/Castle.MicroKernel.Tests/Facilities/Startable/StartableFacilityTestCase.cs +++ b/InversionOfControl/Castle.MicroKernel.Tests/Facilities/Startable/StartableFacilityTestCase.cs @@ -83,7 +83,7 @@ namespace Castle.Facilities.Startable.Tests Assert.IsTrue(component.Stopped); } - [Test, Ignore("Pending resolution")] + [Test] public void TestStartableWithCustomDependencies() { IKernel kernel = new DefaultKernel(); diff --git a/InversionOfControl/Castle.MicroKernel.Tests/RuntimeParametersTestCase.cs b/InversionOfControl/Castle.MicroKernel.Tests/RuntimeParametersTestCase.cs index be4f97a45..d74c50a33 100644 --- a/InversionOfControl/Castle.MicroKernel.Tests/RuntimeParametersTestCase.cs +++ b/InversionOfControl/Castle.MicroKernel.Tests/RuntimeParametersTestCase.cs @@ -46,7 +46,8 @@ namespace Castle.MicroKernel.Tests [Test] [ExpectedException(typeof(HandlerException), "Can't create component 'compb' as it has " + "dependencies to be satisfied. \r\ncompb is waiting for the following dependencies: \r\n\r\n" + - "Services: \r\n- Castle.MicroKernel.Tests.RuntimeParameters.CompC which was not registered. \r\n" + "Services: \r\n- Castle.MicroKernel.Tests.RuntimeParameters.CompC which was not registered. \r\n\r\n" + + "Keys (components with specific keys)\r\n- myArgument which was not registered. \r\n" )] public void WithoutParameters() { diff --git a/InversionOfControl/Castle.MicroKernel.Tests/UnsatisfiedDependenciesTestCase.cs b/InversionOfControl/Castle.MicroKernel.Tests/UnsatisfiedDependenciesTestCase.cs index 73909028e..e6e43899b 100644 --- a/InversionOfControl/Castle.MicroKernel.Tests/UnsatisfiedDependenciesTestCase.cs +++ b/InversionOfControl/Castle.MicroKernel.Tests/UnsatisfiedDependenciesTestCase.cs @@ -46,9 +46,10 @@ namespace Castle.MicroKernel.Tests } [Test] - [ExpectedException(typeof(DependencyResolverException), - "Could not resolve non-optional dependency for 'key' (Castle.MicroKernel.Tests.ClassComponents.CustomerImpl2). Parameter 'name' type 'System.String'" - )] + [ExpectedException(typeof(HandlerException), + "Can't create component 'key' as it has dependencies to be satisfied. \r\nkey is waiting for the following dependencies: \r\n\r\n" + + "Keys (components with specific keys)\r\n- name which was not registered. \r\n- address which was not registered. \r\n" + + "- age which was not registered. \r\n")] public void UnsatisfiedConfigValues() { MutableConfiguration config = new MutableConfiguration("component"); @@ -61,6 +62,7 @@ namespace Castle.MicroKernel.Tests kernel.ConfigurationStore.AddComponentConfiguration("customer", config); kernel.AddComponent("key", typeof(CustomerImpl2)); + object instance = kernel["key"]; } diff --git a/InversionOfControl/Castle.MicroKernel/DefaultKernel.cs b/InversionOfControl/Castle.MicroKernel/DefaultKernel.cs index 6a7f9f206..286d9d7d2 100644 --- a/InversionOfControl/Castle.MicroKernel/DefaultKernel.cs +++ b/InversionOfControl/Castle.MicroKernel/DefaultKernel.cs @@ -913,17 +913,17 @@ namespace Castle.MicroKernel { IHandler[] genericResult = NamingSubSystem.GetHandlers(service.GetGenericTypeDefinition()); - if (result.Length == 0) - { - result = genericResult; - } - else + if (result.Length > 0) { IHandler[] mergedResult = new IHandler[result.Length + genericResult.Length]; result.CopyTo(mergedResult, 0); genericResult.CopyTo(mergedResult, result.Length); result = mergedResult; } + else + { + result = genericResult; + } } // If a parent kernel exists, we merge both results @@ -931,13 +931,11 @@ namespace Castle.MicroKernel { IHandler[] parentResult = Parent.GetHandlers(service); - if (parentResult.Length != 0) + if (parentResult.Length > 0) { IHandler[] newResult = new IHandler[result.Length + parentResult.Length]; - - Array.Copy(result, newResult, result.Length); - Array.Copy(parentResult, 0, newResult, result.Length, parentResult.Length); - + result.CopyTo(newResult, 0); + parentResult.CopyTo(newResult, result.Length); result = newResult; } } diff --git a/InversionOfControl/Castle.MicroKernel/Facilities/Startable/StartableFacility.cs b/InversionOfControl/Castle.MicroKernel/Facilities/Startable/StartableFacility.cs index 4770b0577..762e2bda1 100644 --- a/InversionOfControl/Castle.MicroKernel/Facilities/Startable/StartableFacility.cs +++ b/InversionOfControl/Castle.MicroKernel/Facilities/Startable/StartableFacility.cs @@ -16,64 +16,73 @@ namespace Castle.Facilities.Startable { using System; using System.Collections; - using Castle.MicroKernel; using Castle.MicroKernel.Facilities; using Castle.MicroKernel.SubSystems.Conversion; - using Castle.Core; - - public class StartableFacility : AbstractFacility + public class StartableFacility : AbstractFacility { private ArrayList waitList = new ArrayList(); - private ITypeConverter converter; - + private ITypeConverter converter; + protected override void Init() { converter = (ITypeConverter) Kernel.GetSubSystem(SubSystemConstants.ConversionManagerKey); - Kernel.ComponentModelCreated += + Kernel.ComponentModelCreated += new ComponentModelDelegate(OnComponentModelCreated); - Kernel.ComponentRegistered += + Kernel.ComponentRegistered += new ComponentDataDelegate(OnComponentRegistered); } private void OnComponentModelCreated(ComponentModel model) { - bool startable = + bool startable = CheckIfComponentImplementsIStartable(model) || HasStartableAttributeSet(model); model.ExtendedProperties["startable"] = startable; if (startable) { - model.LifecycleSteps.Add( - LifecycleStepType.Commission, StartConcern.Instance ); - model.LifecycleSteps.Add( - LifecycleStepType.Decommission, StopConcern.Instance ); + model.LifecycleSteps.Add( + LifecycleStepType.Commission, StartConcern.Instance); + model.LifecycleSteps.Add( + LifecycleStepType.Decommission, StopConcern.Instance); } } - private void OnComponentRegistered(String key, IHandler handler) + private void OnComponentRegistered(String key, IHandler handler) { bool startable = (bool) handler.ComponentModel.ExtendedProperties["startable"]; - + if (startable) { if (handler.CurrentState == HandlerState.WaitingDependency) { - waitList.Add( handler ); + AddHandlerToWaitingList(handler); } else { - Start( key ); + Start(key); } } CheckWaitingList(); } + private void OnHandlerStateChanged(object source, EventArgs args) + { + CheckWaitingList(); + } + + private void AddHandlerToWaitingList(IHandler handler) + { + waitList.Add(handler); + + handler.OnHandlerStateChanged += new HandlerStateDelegate(OnHandlerStateChanged); + } + /// /// For each new component registered, /// some components in the WaitingDependency @@ -81,7 +90,7 @@ namespace Castle.Facilities.Startable /// private void CheckWaitingList() { - IHandler[] handlers = (IHandler[]) waitList.ToArray( typeof(IHandler) ); + IHandler[] handlers = (IHandler[]) waitList.ToArray(typeof(IHandler)); IList validList = new ArrayList(); @@ -91,12 +100,14 @@ namespace Castle.Facilities.Startable { validList.Add(handler); waitList.Remove(handler); + + handler.OnHandlerStateChanged -= new HandlerStateDelegate(OnHandlerStateChanged); } } foreach(IHandler handler in validList) { - Start( handler.ComponentModel.Name ); + Start(handler.ComponentModel.Name); } } @@ -109,26 +120,26 @@ namespace Castle.Facilities.Startable object instance = Kernel[key]; } - private bool CheckIfComponentImplementsIStartable(ComponentModel model) - { - return typeof(IStartable).IsAssignableFrom(model.Implementation); - } - + private bool CheckIfComponentImplementsIStartable(ComponentModel model) + { + return typeof(IStartable).IsAssignableFrom(model.Implementation); + } + private bool HasStartableAttributeSet(ComponentModel model) - { - bool result = false; - - if (model.Configuration != null) - { - String startable = model.Configuration.Attributes["startable"]; - + { + bool result = false; + + if (model.Configuration != null) + { + String startable = model.Configuration.Attributes["startable"]; + if (startable != null) { result = (bool) converter.PerformConversion(startable, typeof(bool)); } - } - - return result; - } + } + + return result; + } } -} +} \ No newline at end of file diff --git a/InversionOfControl/Castle.MicroKernel/Handlers/AbstractHandler.cs b/InversionOfControl/Castle.MicroKernel/Handlers/AbstractHandler.cs index 1eaa67621..d4790a319 100644 --- a/InversionOfControl/Castle.MicroKernel/Handlers/AbstractHandler.cs +++ b/InversionOfControl/Castle.MicroKernel/Handlers/AbstractHandler.cs @@ -406,6 +406,12 @@ namespace Castle.MicroKernel.Handlers { AddDependency(dependency); } + else if (dependency.DependencyType == DependencyType.Parameter && + !ComponentModel.Constructors.HasAmbiguousFewerArgumentsCandidate && + !ComponentModel.Parameters.Contains(dependency.DependencyKey)) + { + AddDependency(dependency); + } } if (state == HandlerState.Valid) @@ -460,7 +466,7 @@ namespace Castle.MicroKernel.Handlers DependenciesByService.Add(dependency.TargetType, dependency); } - else + else if (!DependenciesByKey.Contains(dependency.DependencyKey)) { DependenciesByKey.Add(dependency.DependencyKey, dependency); } diff --git a/InversionOfControl/Castle.MicroKernel/ModelBuilder/DefaultComponentModelBuilder.cs b/InversionOfControl/Castle.MicroKernel/ModelBuilder/DefaultComponentModelBuilder.cs index 9a699615f..7935bae93 100644 --- a/InversionOfControl/Castle.MicroKernel/ModelBuilder/DefaultComponentModelBuilder.cs +++ b/InversionOfControl/Castle.MicroKernel/ModelBuilder/DefaultComponentModelBuilder.cs @@ -109,11 +109,11 @@ namespace Castle.MicroKernel.ModelBuilder { AddContributor(new GenericInspector()); AddContributor(new ConfigurationModelInspector()); + AddContributor(new ConfigurationParametersInspector()); AddContributor(new LifestyleModelInspector()); AddContributor(new ConstructorDependenciesModelInspector()); AddContributor(new PropertiesDependenciesModelInspector()); AddContributor(new LifecycleModelInspector()); - AddContributor(new ConfigurationParametersInspector()); AddContributor(new InterceptorInspector()); AddContributor(new ComponentActivatorInspector()); AddContributor(new ComponentProxyInspector()); diff --git a/InversionOfControl/Castle.MicroKernel/ModelBuilder/Inspectors/ConstructorDependenciesModelInspector.cs b/InversionOfControl/Castle.MicroKernel/ModelBuilder/Inspectors/ConstructorDependenciesModelInspector.cs index 1f75befaa..541ceb89b 100644 --- a/InversionOfControl/Castle.MicroKernel/ModelBuilder/Inspectors/ConstructorDependenciesModelInspector.cs +++ b/InversionOfControl/Castle.MicroKernel/ModelBuilder/Inspectors/ConstructorDependenciesModelInspector.cs @@ -18,6 +18,7 @@ namespace Castle.MicroKernel.ModelBuilder.Inspectors using System.Reflection; using Castle.Core; using Castle.MicroKernel.SubSystems.Conversion; + using Castle.MicroKernel.Util; /// /// This implementation of @@ -53,12 +54,11 @@ namespace Castle.MicroKernel.ModelBuilder.Inspectors // We register each public constructor // and let the ComponentFactory select an // eligible amongst the candidates later - - model.Constructors.Add(CreateConstructorCandidate(constructor)); + model.Constructors.Add(CreateConstructorCandidate(model, constructor)); } } - protected virtual ConstructorCandidate CreateConstructorCandidate(ConstructorInfo constructor) + protected virtual ConstructorCandidate CreateConstructorCandidate(ComponentModel model, ConstructorInfo constructor) { ParameterInfo[] parameters = constructor.GetParameters(); @@ -77,10 +77,22 @@ namespace Castle.MicroKernel.ModelBuilder.Inspectors dependencies[i] = new DependencyModel( DependencyType.Parameter, parameter.Name, paramType, false); } - else + else { - dependencies[i] = new DependencyModel( - DependencyType.Service, parameter.Name, paramType, false); + ParameterModel modelParameter = model.Parameters[parameter.Name]; + + if (modelParameter != null && ReferenceExpressionUtil.IsReference(modelParameter.Value)) + { + String key = ReferenceExpressionUtil.ExtractComponentKey(modelParameter.Value); + + dependencies[i] = new DependencyModel( + DependencyType.ServiceOverride, key, paramType, false); + } + else + { + dependencies[i] = new DependencyModel( + DependencyType.Service, parameter.Name, paramType, false); + } } } diff --git a/InversionOfControl/Castle.Windsor.Tests/Proxy/FactorySupportTestCase.cs b/InversionOfControl/Castle.Windsor.Tests/Proxy/FactorySupportTestCase.cs new file mode 100644 index 000000000..5c25569af --- /dev/null +++ b/InversionOfControl/Castle.Windsor.Tests/Proxy/FactorySupportTestCase.cs @@ -0,0 +1,67 @@ +// Copyright 2004-2007 Castle Project - http://www.castleproject.org/ +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +namespace Castle.Windsor.Tests.Proxy +{ + using System; + using Castle.Core; + using Castle.Core.Configuration; + using Castle.Core.Interceptor; + using Castle.Facilities.FactorySupport; + using Castle.Windsor.Tests.Components; + using NUnit.Framework; + + [TestFixture] + public class FactorySupportTestCase + { + private WindsorContainer container; + + [SetUp] + public void SetUp() + { + container = new WindsorContainer(); + } + + [Test] + public void FactorySupport_UsingProxiedFactory_WorksFine() + { + container.AddFacility("factories", new FactorySupportFacility()); + container.AddComponent("standard.interceptor", typeof(StandardInterceptor)); + container.AddComponent("factory", typeof(CalulcatorFactory)); + + AddComponent("calculator", typeof(ICalcService), typeof(CalculatorService), "Create"); + + ICalcService service = (ICalcService) container["calculator"]; + + Assert.IsNotNull(service); + } + + private void AddComponent(string key, Type service, Type type, string factoryMethod) + { + MutableConfiguration config = new MutableConfiguration(key); + config.Attributes["factoryId"] = "factory"; + config.Attributes["factoryCreate"] = factoryMethod; + container.Kernel.ConfigurationStore.AddComponentConfiguration(key, config); + container.Kernel.AddComponent(key, service, type); + } + + [Interceptor(typeof(StandardInterceptor))] + public class CalulcatorFactory + { + public virtual ICalcService Create() + { + return new CalculatorService(); + } + } + } +} diff --git a/InversionOfControl/Castle.Windsor.Tests/Proxy/ProxyBehaviorInvalidTestCase.cs b/InversionOfControl/Castle.Windsor.Tests/Proxy/ProxyBehaviorInvalidTestCase.cs index cfa41dd88..dc5cd373d 100644 --- a/InversionOfControl/Castle.Windsor.Tests/Proxy/ProxyBehaviorInvalidTestCase.cs +++ b/InversionOfControl/Castle.Windsor.Tests/Proxy/ProxyBehaviorInvalidTestCase.cs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -namespace Castle.Windsor.Tests +namespace Castle.Windsor.Tests.Proxy { using System.Configuration; using Castle.Core.Interceptor; diff --git a/InversionOfControl/Castle.Windsor.Tests/Proxy/ProxyBehaviorTestCase.cs b/InversionOfControl/Castle.Windsor.Tests/Proxy/ProxyBehaviorTestCase.cs index 43157338a..773166707 100644 --- a/InversionOfControl/Castle.Windsor.Tests/Proxy/ProxyBehaviorTestCase.cs +++ b/InversionOfControl/Castle.Windsor.Tests/Proxy/ProxyBehaviorTestCase.cs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -namespace Castle.Windsor.Tests +namespace Castle.Windsor.Tests.Proxy { using System; using Castle.Core.Interceptor; diff --git a/InversionOfControl/Castle.Windsor.Tests/Proxy/TypedFactoryFacilityTestCase.cs b/InversionOfControl/Castle.Windsor.Tests/Proxy/TypedFactoryFacilityTestCase.cs index 7a87a7eb4..1c7fee6f1 100644 --- a/InversionOfControl/Castle.Windsor.Tests/Proxy/TypedFactoryFacilityTestCase.cs +++ b/InversionOfControl/Castle.Windsor.Tests/Proxy/TypedFactoryFacilityTestCase.cs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -namespace Castle.Windsor.Tests +namespace Castle.Windsor.Tests.Proxy { using System; using Castle.Core.Interceptor; diff --git a/InversionOfControl/Castle.Windsor.Tests/Proxy/typedFactory.xml b/InversionOfControl/Castle.Windsor.Tests/Proxy/typedFactory.xml index 50e0e6e07..1b942cede 100644 --- a/InversionOfControl/Castle.Windsor.Tests/Proxy/typedFactory.xml +++ b/InversionOfControl/Castle.Windsor.Tests/Proxy/typedFactory.xml @@ -7,7 +7,7 @@ diff --git a/InversionOfControl/Castle.Windsor.Tests/Proxy/typedFactoryCreateWithoutId.xml b/InversionOfControl/Castle.Windsor.Tests/Proxy/typedFactoryCreateWithoutId.xml index 626e4f1af..5efa1e3a9 100644 --- a/InversionOfControl/Castle.Windsor.Tests/Proxy/typedFactoryCreateWithoutId.xml +++ b/InversionOfControl/Castle.Windsor.Tests/Proxy/typedFactoryCreateWithoutId.xml @@ -7,7 +7,7 @@ -- 2.11.4.GIT