Skip to content
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

Migrate away from Managed Extensibility Framework (MEF) #400

Closed
JamieMagee opened this issue Jan 5, 2023 · 0 comments · Fixed by #412
Closed

Migrate away from Managed Extensibility Framework (MEF) #400

JamieMagee opened this issue Jan 5, 2023 · 0 comments · Fixed by #412
Assignees
Labels
breaking change Breaking change, requires major version bump status:in-progress Someone is working on implementation type:refactor Refactoring or improving of existing code

Comments

@JamieMagee
Copy link
Member

JamieMagee commented Jan 5, 2023

Currently, we use Managed Extensibility Framework (MEF) to handle injection of dependencies through Component Detection. MEF was the best way to do Dependency Injection (DI) in .NET Framework, and still is the best way to do application extensibility via plugins. However, in Modern .NET, the most common way to do DI is via the Microsoft.Extensions.DependencyInjection package. I suggest migrating away from MEF and to Microsoft.Extensions.DependencyInjection-style DI for a few main reasons:

  1. We don't need the full functionality of MEF
  2. Microsoft.Extensions.DependencyInjection has better support, documentation, and developer mindshare
  3. Microsoft.Extensions.DependencyInjection has less 'magic'
    1. No loading random DLLs
    2. No decorating classes with [Export] and properties with [Import]

The pattern I suggest migrating to is outline in the Options pattern guidance for .NET library authors documentation. I suggest performing this migration in a few key steps

1. Add dependencies and scaffolding #370

We need to add a reference to Microsoft.Extensions.DependencyInjection.Abstractions in Microsoft.ComponentDetection.Orchestrator and Microsoft.Extensions.DependencyInjection to Microsoft.ComponentDetection. The Microsoft.Extensions.DependencyInjection.Abstractions package contains interfaces and abstract classes required for Dependency Injection which can be implemented by any library. We need to use this package specifically in our library (Microsoft.ComponentDetection.Orchestrator) because we don't know which DI library our caller will be using. In our CLI tool, Microsoft.ComponentDetection, we'll use the Microsoft implementation (Microsoft.Extensions.DependencyInjection) of the abstractions provided in Microsoft.Extensions.DependencyInjection.Abstractions

We also need to add an extension method to IServiceCollection called AddComponentDetection. This is where we'll eventually configure all the required services. This extension should initially be private to our assembly, so we can rapidly iterate on the design, without the changes being semver breaking changes.

internal static class ServiceCollectionExtensions
{
    /// <summary>
    /// Register Component Detection services.
    /// </summary>
    /// <param name="services">The <see cref="IServiceCollection" /> to register the services with.</param>
    /// <returns>The <see cref="IServiceCollection" /> so that additional calls can be chained.</returns>
    public static IServiceCollection AddComponentDetection(this IServiceCollection services) => services;
}

This is already complete in #370

2. Add class constructors

Microsoft.Extensions.DependencyInjection works on constructor injection, whereas MEF works on property injection. As we currently use MEF, most of our classes don't have the required constructors (only the default parameterless constructors created by Roslyn).

For example, the DockerService class would need to have the following constructor added

[Export(typeof(IDockerService))]
public class DockerService : IDockerService
{
    public DockerService(ILogger logger)
    {
        this.Logger = logger;
    }

    [Import]
    public ILogger Logger { get; set; }
}

This needs to be done for all classes that are currently annotated with the [Export] attribute.

3. Replace DetectorTestUtility

DetectorTestUtility provides a fluent API to easily set up detector unit tests. Unfortunately, it uses property injection instead of constructor injection to set up the classes.

We need to refactor DetectorTestUtility to use constructor injection, while still providing the mocking capabilities and fluent API if possible.

4. Wire up classes

In our ServiceCollectionExtensions class, we need to wire up all the classes that are currently annotated with the [Export] attribute. For example:

public static IServiceCollection AddComponentDetection(this IServiceCollection services)
{
    services.AddSingleton<IComponentDetector, DockerfileComponentDetector>();
    services.AddSingleton<IComponentDetector, NpmComponentDetector>();
    // etc.

    return services;
}

4. Migrate Microsoft.ComponentDetection

At this point we can migrate Microsoft.ComponentDetection to use ServiceCollection and AddComponentDetection. For example:

var serviceCollection = new ServiceCollection()
    .AddLogging(configure =>
        // ...
    )
    .AddComponentDetection();

See Dependency injection in .NET for more guidance.

5. Make ServiceCollectionExtensions public

At this point Microsoft.ComponentDetection should be using Microsoft.Extensions.DependencyInjection to handle dependency injection. So, if not done already, we should expose ServiceCollectionExtensions (and AddComponentDetection as part of our public API, so consumers of Component Detection (like https://github.com/microsoft/sbom-tool) can use it too.

6. Remove MEF attributes

We should remove any MEF attributes, like [Export] and [Import].

7. Remove public properties

At this point we need to make a breaking API change by removing the public properties that MEF used to configure dependencies. For example, DockerService would change from:

[Export(typeof(IDockerService))]
public class DockerService : IDockerService
{
    public DockerService(ILogger logger)
    {
        this.Logger = logger;
    }

    [Import]
    public ILogger Logger { get; set; }
}

to

[Export(typeof(IDockerService))]
public class DockerService : IDockerService
{
    private readonly ILogger logger;

    public DockerService(ILogger logger)
    {
        this.logger = logger;
    }
}

8. Remove MEF dependencies

Finally, we need to remove the nuget packages that provide the MEF functionality. These are:

  • System.Composition.AttributedModel
  • System.Composition.Convention
  • System.Composition.Hosting
  • System.Composition.Runtime
  • System.Composition.TypedParts
@JamieMagee JamieMagee added status:ready Ready to start implementation type:refactor Refactoring or improving of existing code breaking change Breaking change, requires major version bump labels Jan 5, 2023
JamieMagee added a commit that referenced this issue Jan 15, 2023
Currently, Managed Extensibility Framework (MEF) does property injection. In migrating to .NET's own Dependency Injection framework `Microsoft.Extensions.DependencyInjection` we need to migrate to constructor injection. This change adds 2 constructors for every class that has properties injected into it:

- A parameterless constructor (for MEF)
- A constructor with parameters for all the required dependencies (for .NET DI)

This is an intermediate step that allows MEF to work, while work continues on the migration to MEF.

See #400 for more information
JamieMagee added a commit that referenced this issue Jan 15, 2023
This is the equivalent of `DetectorTestUtilityCreator` and `DetectorTestUtility`, but for .NET DI.

See #400
@JamieMagee JamieMagee self-assigned this Jan 25, 2023
@JamieMagee JamieMagee added status:in-progress Someone is working on implementation and removed status:ready Ready to start implementation labels Jan 25, 2023
JamieMagee added a commit that referenced this issue Jan 27, 2023
Currently, Managed Extensibility Framework (MEF) does property injection. In migrating to .NET's own Dependency Injection framework `Microsoft.Extensions.DependencyInjection` we need to migrate to constructor injection. This change adds 2 constructors for every class that has properties injected into it:

- A parameterless constructor (for MEF)
- A constructor with parameters for all the required dependencies (for .NET DI)

This is an intermediate step that allows MEF to work, while work continues on the migration to MEF.

See #400 for more information
JamieMagee added a commit that referenced this issue Jan 27, 2023
This is the equivalent of `DetectorTestUtilityCreator` and `DetectorTestUtility`, but for .NET DI.

See #400
JamieMagee added a commit that referenced this issue Jan 30, 2023
Currently, Managed Extensibility Framework (MEF) does property injection. In migrating to .NET's own Dependency Injection framework `Microsoft.Extensions.DependencyInjection` we need to migrate to constructor injection. This change adds 2 constructors for every class that has properties injected into it:

- A parameterless constructor (for MEF)
- A constructor with parameters for all the required dependencies (for .NET DI)

This is an intermediate step that allows MEF to work, while work continues on the migration to MEF.

See #400 for more information
JamieMagee added a commit that referenced this issue Jan 30, 2023
This is the equivalent of `DetectorTestUtilityCreator` and `DetectorTestUtility`, but for .NET DI.

See #400
JamieMagee added a commit that referenced this issue Feb 6, 2023
Currently, Managed Extensibility Framework (MEF) does property injection. In migrating to .NET's own Dependency Injection framework `Microsoft.Extensions.DependencyInjection` we need to migrate to constructor injection. This change adds 2 constructors for every class that has properties injected into it:

- A parameterless constructor (for MEF)
- A constructor with parameters for all the required dependencies (for .NET DI)

This is an intermediate step that allows MEF to work, while work continues on the migration to MEF.

See #400 for more information
JamieMagee added a commit that referenced this issue Feb 6, 2023
This is the equivalent of `DetectorTestUtilityCreator` and `DetectorTestUtility`, but for .NET DI.

See #400
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Breaking change, requires major version bump status:in-progress Someone is working on implementation type:refactor Refactoring or improving of existing code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant