-
Notifications
You must be signed in to change notification settings - Fork 458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed first-chance HandlerException for optional parameters #450
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
// Copyright 2018 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. | ||
|
||
#if !NETCOREAPP1_0 // FirstChanceException event was added in .NET Core 2.0 | ||
namespace CastleTests | ||
{ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Runtime.ExceptionServices; | ||
using System.Text; | ||
|
||
using NUnit.Framework; | ||
|
||
public static class TestUtils | ||
{ | ||
public static void AssertNoFirstChanceExceptions(Action action) | ||
{ | ||
var firstChanceExceptions = new List<Exception>(); | ||
|
||
var handler = new EventHandler<FirstChanceExceptionEventArgs>((sender, e) => | ||
firstChanceExceptions.Add(e.Exception)); | ||
|
||
AppDomain.CurrentDomain.FirstChanceException += handler; | ||
try | ||
{ | ||
action.Invoke(); | ||
} | ||
finally | ||
{ | ||
AppDomain.CurrentDomain.FirstChanceException -= handler; | ||
} | ||
|
||
if (firstChanceExceptions.Any()) | ||
{ | ||
var message = new StringBuilder(); | ||
for (var i = 0; i < firstChanceExceptions.Count; i++) | ||
{ | ||
message.Append("First-chance exception ").Append(i + 1).Append(" of ").Append(firstChanceExceptions.Count).AppendLine(":"); | ||
message.AppendLine(firstChanceExceptions[i].ToString()); | ||
message.AppendLine(); | ||
} | ||
|
||
message.Append("Expected: no first-chance exceptions."); | ||
|
||
Assert.Fail(message.ToString()); | ||
} | ||
} | ||
} | ||
} | ||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -361,12 +361,7 @@ private object ResolveFromKernelByName(CreationContext context, ComponentModel m | |
|
||
private object ResolveFromKernelByType(CreationContext context, ComponentModel model, DependencyModel dependency) | ||
{ | ||
IHandler handler; | ||
try | ||
{ | ||
handler = TryGetHandlerFromKernel(dependency, context); | ||
} | ||
catch (HandlerException exception) | ||
if (!TryGetHandlerFromKernel(dependency, context, out var handler)) | ||
{ | ||
if (dependency.HasDefaultValue) | ||
{ | ||
|
@@ -377,8 +372,7 @@ private object ResolveFromKernelByType(CreationContext context, ComponentModel m | |
"Missing dependency.{2}Component {0} has a dependency on {1}, which could not be resolved.{2}Make sure the dependency is correctly registered in the container as a service, or provided as inline argument.", | ||
model.Name, | ||
dependency.TargetItemType, | ||
Environment.NewLine), | ||
exception); | ||
Environment.NewLine)); | ||
} | ||
|
||
if (handler == null) | ||
|
@@ -424,23 +418,37 @@ private object ResolveFromParameter(CreationContext context, ComponentModel mode | |
} | ||
} | ||
|
||
private IHandler TryGetHandlerFromKernel(DependencyModel dependency, CreationContext context) | ||
private bool TryGetHandlerFromKernel(DependencyModel dependency, CreationContext context, out IHandler handler) | ||
{ | ||
// we are doing it in two stages because it is likely to be faster to a lookup | ||
// by key than a linear search | ||
var handler = kernel.LoadHandlerByType(dependency.DependencyKey, dependency.TargetItemType, context.AdditionalArguments); | ||
if (handler == null) | ||
try | ||
{ | ||
handler = kernel.LoadHandlerByType(dependency.DependencyKey, dependency.TargetItemType, context.AdditionalArguments); | ||
} | ||
catch (HandlerException) | ||
{ | ||
throw new HandlerException(string.Format("Handler for {0} was not found.", dependency.TargetItemType), null); | ||
handler = null; | ||
} | ||
if (handler == null) return false; | ||
|
||
if (handler.IsBeingResolvedInContext(context) == false) | ||
{ | ||
return handler; | ||
return true; | ||
} | ||
|
||
// make a best effort to find another one that fit | ||
|
||
var handlers = kernel.GetHandlers(dependency.TargetItemType); | ||
IHandler[] handlers; | ||
try | ||
{ | ||
handlers = kernel.GetHandlers(dependency.TargetItemType); | ||
} | ||
catch (HandlerException) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To make this a valid refactoring, we have to be sure that this method can't throw There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
{ | ||
return false; | ||
} | ||
|
||
foreach (var maybeCorrectHandler in handlers) | ||
{ | ||
if (maybeCorrectHandler.IsBeingResolvedInContext(context) == false) | ||
|
@@ -449,7 +457,7 @@ private IHandler TryGetHandlerFromKernel(DependencyModel dependency, CreationCon | |
break; | ||
} | ||
} | ||
return handler; | ||
return true; | ||
} | ||
|
||
private static bool IsHandlerInValidState(IHandler handler) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
return false
case looks almost identical to thehandler = null; return true
case below it.There's a potentially a better factoring to indicate which of the two messages should be used?