Skip to content

Invoke Build Callbacks at Lifetime Scope Creation #1054

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

Merged
merged 5 commits into from
Dec 4, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions nullabilitynotes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@

# Notes on nullability

- Anywhere that we have a 'Try....' method, we now apply [NotNullWhen(returnValue: true)] on the out parameter.
This should probably be enforced via code review in the future.

ReflectionExtensions
```

public static bool TryGetDeclaringProperty(this ParameterInfo pi, [NotNullWhen(returnValue: true)] out PropertyInfo? prop)

```

- Null annotation attributes only inform the caller if used in a straight if declaration.

ResolutionExtensions.TryResolve

- Registration methods where we accept a 'limit' now constrain the limit to where T: notnull.

The only impact I can think of is if someone was using ``Nullable<int>`` as the limit for a service.

In addition, TryResolve is now constrained to 'class'.

- Types that overload == and != need to specify the overload parameters as nullable, otherwise we can't do null checks on those values.

Service
```
/// <summary>
/// Implements the operator ==.
/// </summary>
/// <param name="left">The left operand.</param>
/// <param name="right">The right operand.</param>
/// <returns>The result of the operator.</returns>
public static bool operator ==(Service? left, Service? right)
{
return Equals(left, right);
}
```

- Need to make 'optional' methods return TService? to indicate to the caller that the response may be null.

- Upgraded StyleCop analyzers to a beta version (for now), to understand nullable modifiers on arrays.

- The Roslyn compiler reports a class field as being uninitialized (and potentially null) if it can't tell it's been initialised at the end of the constructor.
This happens even if the constructor calls a method or property that always instantiates the field. So you can set 'default!' or 'null!' on the field to get round this (not ideal, but explainable).

ResolveOperation
```
// _successfulActivations can never be null, but the roslyn compiler doesn't look deeper than
// the initial constructor methods yet.
private List<InstanceLookup> _successfulActivations = default!;
```

DeferredCallbacks
```
// _callback set to default! to get around initialisation detection problem in rosyln.
private Action<IComponentRegistry> _callback = default!;
```

There are a couple of open rosyln issues that are related to this, some discussion about adding flow analysis to check that constructors
do in fact finish having initialised those properties.

This is also a massive pain whenever I have a class that I want to instantiate using 'Property' notation, e.g. DecoratorContext:

```
private DecoratorContext() // error on this line
{

}

internal static DecoratorContext Create(Type implementationType, Type serviceType, object implementationInstance)
{
var context = new DecoratorContext
{
ImplementationType = implementationType,
ServiceType = serviceType,
AppliedDecorators = new List<object>(0),
AppliedDecoratorTypes = new List<Type>(0),
CurrentInstance = implementationInstance
};
return context;
}


```

https://github.com/dotnet/roslyn/issues/39291
https://github.com/dotnet/roslyn/issues/39090
https://github.com/dotnet/roslyn/issues/37975

- If a method is being passed as an expression for reflection only, it's better to mark the arguments as default! rather than
change the signature of the type:

FactoryGenerator
```
// The explicit '!' default is ok because the code is never executed, it's just used by
// the expression tree.
private static readonly ConstructorInfo RequestConstructor
= ReflectionExtensions.GetConstructor(() => new ResolveRequest(default!, default!, default!));
```

- In a couple of specific cases, some generic methods now need to state explicitly 'class' as a type constraint, where they didn't before; if I don't do that
then duplicate overloads for class and struct need to be defined to remove ambiguity.

RegistrationExtensions.RegisterDecorator
9 changes: 9 additions & 0 deletions src/Autofac/Autofac.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@
</ItemGroup>

<ItemGroup>
<Compile Update="Builder\BuildCallbackServiceResources.Designer.cs">
<DesignTime>True</DesignTime>
<AutoGen>True</AutoGen>
<DependentUpon>BuildCallbackServiceResources.resx</DependentUpon>
</Compile>
<Compile Update="Builder\RegistrationBuilderResources.Designer.cs">
<DesignTime>True</DesignTime>
<AutoGen>True</AutoGen>
Expand Down Expand Up @@ -267,6 +272,10 @@
</ItemGroup>

<ItemGroup>
<EmbeddedResource Update="Builder\BuildCallbackServiceResources.resx">
<Generator>ResXFileCodeGenerator</Generator>
<LastGenOutput>BuildCallbackServiceResources.Designer.cs</LastGenOutput>
</EmbeddedResource>
<EmbeddedResource Update="Builder\RegistrationBuilderResources.resx">
<Generator>ResXFileCodeGenerator</Generator>
<LastGenOutput>RegistrationBuilderResources.Designer.cs</LastGenOutput>
Expand Down
63 changes: 63 additions & 0 deletions src/Autofac/Builder/BuildCallbackManager.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// This software is part of the Autofac IoC container
// Copyright © 2019 Autofac Contributors
// https://autofac.org
//
// Permission is hereby granted, free of charge, to any person
// obtaining a copy of this software and associated documentation
// files (the "Software"), to deal in the Software without
// restriction, including without limitation the rights to use,
// copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the
// Software is furnished to do so, subject to the following
// conditions:
//
// The above copyright notice and this permission notice shall be
// included in all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
// OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
// HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
// WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
// OTHER DEALINGS IN THE SOFTWARE.

using System.Linq;
using Autofac.Core;

namespace Autofac.Builder
{
internal static class BuildCallbackManager
{
private static readonly TypedService CallbackServiceType = new TypedService(typeof(BuildCallbackService));

private const string BuildCallbacksExecutedKey = nameof(BuildCallbacksExecutedKey);

/// <summary>
/// Executes the newly-registered build callbacks for a given scope/container..
/// </summary>
/// <param name="scope">The new scope/container.</param>
internal static void RunBuildCallbacks(ILifetimeScope scope)
{
var buildCallbackServices = scope.ComponentRegistry.RegistrationsFor(CallbackServiceType);

foreach (var srv in buildCallbackServices)
{
// Use the metadata to track whether we've executed already, to avoid issuing a resolve request.
if (srv.Metadata.ContainsKey(BuildCallbacksExecutedKey))
{
continue;
}

var request = new ResolveRequest(CallbackServiceType, srv, Enumerable.Empty<Parameter>());
var component = scope.ResolveComponent(request) as BuildCallbackService;

srv.Metadata[BuildCallbacksExecutedKey] = true;

// Run the callbacks for the relevant scope.
component.Execute(scope);
}
}
}
}
75 changes: 75 additions & 0 deletions src/Autofac/Builder/BuildCallbackService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// This software is part of the Autofac IoC container
// Copyright © 2019 Autofac Contributors
// https://autofac.org
//
// Permission is hereby granted, free of charge, to any person
// obtaining a copy of this software and associated documentation
// files (the "Software"), to deal in the Software without
// restriction, including without limitation the rights to use,
// copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the
// Software is furnished to do so, subject to the following
// conditions:
//
// The above copyright notice and this permission notice shall be
// included in all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
// OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
// HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
// WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
// OTHER DEALINGS IN THE SOFTWARE.

using System;
using System.Collections.Generic;
using System.Linq;

namespace Autofac.Builder
{
/// <summary>
/// Represent a collection of callbacks that can be run after a container or scope is 'built'.
/// </summary>
internal class BuildCallbackService
{
private List<Action<ILifetimeScope>> _callbacks = new List<Action<ILifetimeScope>>();

/// <summary>
/// Add a callback to the set that will get executed.
/// </summary>
/// <param name="callback">The callback to run.</param>
public void AddCallback(Action<ILifetimeScope> callback)
{
if (callback == null) throw new ArgumentNullException(nameof(callback));

_callbacks.Add(callback);
}

public void Execute(ILifetimeScope scope)
{
if (scope == null) throw new ArgumentNullException(nameof(scope));

if (_callbacks == null)
{
throw new InvalidOperationException(BuildCallbackServiceResources.BuildCallbacksAlreadyRun);
}

try
{
foreach (var callback in _callbacks)
{
callback(scope);
}
}
finally
{
// Clear the reference to the callbacks to release any held scopes (and function as a do-not-run flag)
// This object will be a singleton instance in the container/scope, so the initial function scopes that
// define the callbacks could be holding onto closure resources. We want the GC to take that back.
_callbacks = null;
}
}
}
}
72 changes: 72 additions & 0 deletions src/Autofac/Builder/BuildCallbackServiceResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading