Fix bug when using a ctor with interfaces in MEF with unbound generic…

[TMiNus]

…s #10627

[dnfclas]

Hi @TMiNus, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

[dnfclas]

@TMiNus, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

[TMiNus]

This System.IO.DirectoryNotFoundException in the Windows_NT Release Build is not related with the changes at all O.o, prob. something going foobar in the CI server

[stephentoub]

cc: @dsplaisted

[Petermarcu]

@davkean

[karelz]

@dsplaisted @davkean can you please review the change?

[ericstj]

I'm not super familiar with MEF 2 but it looks like this change is modifying this code that previously looked for a public non static default constructor to look for that or a constructor that only takes Interfaces. My question would be, why only interfaces? Can't MEF also create parts that aren't interfaces?

/cc @nblumhardt

[nblumhardt]

@ericstj that's correct, MEF2 can create/consume non-interface contracts.

@TMiNus I just checked out your bug report linked above, I think the missing piece of information is that when no [ImportingConstructor] is specified, the MEF "typed part" model will only fall back to parameterless constructors. There is no policy for selecting a "best" constructor among those accepting parameters. If you want a non-default constructor used, you need to specify which one (even if it's the only one present).

You can make your example work by adding the [ImportingConstructor] attribute to the constructor on ILogger, or do something like this with SelectConstructor():

conventions.ForType(typeof(Logger<>))
           .SelectConstructor(ci => ci.Single())
           .Export();

Hope this helps,
Nick

[TMiNus]

@nblumhardt I already knew about the [ImportingConstructor] mark, but that solution is not good enough if you don't have access to the source code to modify that part, also I wanted to use a convention based syntax. Here is the link to the whole process http://stackoverflow.com/questions/38797527/default-constructor-and-open-generics-in-mef-2-using-conventions

[TMiNus]

@ericstj @nblumhardt The bug manifest when you don't have the mark.

MEF2 requires the mark if you don't have a parameter-less constructor, which is completely wrong because you could have a constructor with only interfaces. With the change MEF2 will accept the constructor with interfaces as a good one and it will try to resolve the contract for those interfaces

[nblumhardt]

Hi @TMiNus - what I mean is, there's no bug here, it's by design.

If you want MEF to choose constructors using a strategy different from the built-in one, you can use the SelectConstructor() method to do it, as shown above. You don't need access to the part's source code for this.

Instead of calling SelectConstructor() directly, you can create your own extension method to describe the policy:

conventions.ForType(typeof(Logger<>))
           .SelectConstructorAcceptingInterfaces()
           .Export();

Where SelectConstructorAcceptingInterfaces() is:

static class PartConventionBuilderExtensions
{
    public static PartConventionBuilder SelectConstructorAcceptingInterfaces<T>(this PartConventionBuilder @this)
    {
        return @this.SelectConstructor(constructors =>
            constructors.Single(ci => ci.GetParameters().All(pi => pi.ParameterType.IsInterface)));
    }
}

[TMiNus]

@nblumhardt Did you read the stackoverflow link that I put in my comment, it seems like you didn´t. SelectConstructor doesn´t work. I personally tried it.

[TMiNus]

Just to be 200% sure I already tried your code and as I said before is a no go.

As you can read in the title of the bug report, this is all related to unbound generics. I want MEF2 to close the generic for me. And if you see the Test that I enhanced, it clearly states that the export should be done as an unbound generic so your code will change to something like this:

conventions.ForType(typeof(Logger<>))
           .SelectConstructorAcceptingInterfaces()
           .Export(t => t.AsContractType(typeof(ILogger<>)))
           .Shared();

And the realization of all this should be something like this:

var test = container.GetExport<ILogger<int>>()

And as stated in the stackoverflow link SelectConstructor doesn't work on this case.

[nblumhardt]

Ah I see. The problem then is that SelectConstructor() is not having the required effect. It should be able to handle this scenario, but somewhere down the line "attributes" applied by the convention to the desired constructor on the open type aren't being projected across to the constructor on the closed version of the type.

This PR is not the solution, unfortunately, but the bug report is valid. I'll add some notes over there in https://github.com/dotnet/corefx/issues/10627.

Cheers!

[TMiNus]

Well you can argue that it is a matter of perspective, if the MEF2 guys consider that SelectConstructor should cover this scenario then SelectConstructor should be looked. On the other hand if it is decided that in MEF2 terms a constructor with only interfaces should be treated as a parameter-less constructor then this PR is a valid one because it solves that problem.

As I said it is a matter of perspective and what the MEF2 team decide.

[Petermarcu]

@TMiNus @nblumhardt , sounds like we're going to look at making a different change for this based on this conversation and what I see here: #10627 . Should we close this PR?

[TMiNus]

@Petermarcu The change that I talk about in my last comment in #10627 would cover a more wide scenario. This PR will fix the problem if you have a constructor with only interfaces

[Petermarcu]

ok, @nblumhardt what do you want to do with this PR?

[nblumhardt]

@Petermarcu if the SelectConstructor() bug is fixed, the "constructor-with-only-interfaces" policy can be plugged in through the existing API surface, so I think it makes sense to close this one.

[Petermarcu]

ok, sounds good. I'm going to close this one and we can bring it back if there is still a gap after #10627 .