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

[MS.DI] Incorrect resolving for generic types. #267

Closed
dvabuzyarov opened this issue Apr 29, 2020 · 7 comments · Fixed by #268
Closed

[MS.DI] Incorrect resolving for generic types. #267

dvabuzyarov opened this issue Apr 29, 2020 · 7 comments · Fixed by #268
Assignees
Labels
bug Something isn't working good first issue Good for newcomers
Milestone

Comments

@dvabuzyarov
Copy link

DryIoc.dll : 4.1.1
.net core 3.1

This scenario is relative to Options pattern in .net core. Due the issue DryIoc returns incorrect options.

In the test there are 2 types of TestOptions, one of them is a generic type. When we are using the generic type DryIoc resolves NonGeneric type instead of requested generic type.

The container resolves

        public class TestService
        {
            
        }
        public class TestOptions
        {
        }

        public class TestOptions<TService> : TestOptions where TService : class
        {
        }

        public class TestOptionsSetup : IConfigureOptions<TestOptions>
        {
            public void Configure(TestOptions options)
            {
                throw new System.NotImplementedException();
            }
        }

        [TestMethod]
        public async Task DryIocIssue()
        {
            var container = new Container();
            container.Register<IConfigureOptions<TestOptions>, TestOptionsSetup>();
            var actual = container.Resolve<IEnumerable<IConfigureOptions<TestOptions<TestService>>>>();
         
           // but returns 1 item: {TestOptionsSetup}
            Assert.AreEqual(0, actual.Count());
        }
@dadhi
Copy link
Owner

dadhi commented Apr 29, 2020

@dvabuzyarov ,

I see a single registration only. So if that's it, the Resolve of collection will return a single service as well.

What other service do you expect to be resolved in this example?

@dvabuzyarov
Copy link
Author

@dadhi Actually I am expecting it returns 0, as I am not asking for

IConfigureOptions<TestOptions>> 

but for

IConfigureOptions<TestOptions<TestService>>>

moreover I extended a little the test case and IsRegistered behaves as I would expect.

        [TestMethod]
        public async Task DryIocIssue()
        {
            var container = new Container();
            container.Register<IConfigureOptions<TestOptions>, TestOptionsSetup>();

            var isRegisteredForNonGeneric = container.IsRegistered(typeof(IConfigureOptions<TestOptions>));
            var isRegisteredForGeneric = container.IsRegistered(typeof(IConfigureOptions<TestOptions<TestService>>));
            var actual = container.ResolveMany(typeof(IConfigureOptions<TestOptions<TestService>>));
            
            Assert.IsTrue(isRegisteredForNonGeneric);
            Assert.IsFalse(isRegisteredForGeneric);
            Assert.AreEqual(0, actual.Count());
        }

@dadhi
Copy link
Owner

dadhi commented Apr 29, 2020

Apparently, you hit the Variance in IConfigureOptions<in TOptions>.
DryIoc supports the variance in the resolved collection by Default and seems like MS.DI does not (need a fact check).

Here is the working example where I turning variance off https://dotnetfiddle.net/6OOYUi
The whole code:

using System;
using System.Linq;

using DryIoc;
					
public class Program
{
	public static void Main()
	{
		var container = new Container(rules =>									 
			rules.WithoutVariantGenericTypesInResolvedCollection());
		
		container.Register<IConfigureOptions<TestOptions>, TestOptionsSetup>();

		var isRegisteredForNonGeneric = container.IsRegistered(typeof(IConfigureOptions<TestOptions>));
		var isRegisteredForGeneric = container.IsRegistered(typeof(IConfigureOptions<TestOptions<TestService>>));
		var actual = container.ResolveMany(typeof(IConfigureOptions<TestOptions<TestService>>));

		Console.WriteLine($"{isRegisteredForNonGeneric} is true, {isRegisteredForGeneric} is false, {actual.Count()} is 0?");
	}
	
	
	public interface IConfigureOptions<in TOptions> where TOptions : class
    {
        void Configure(TOptions options);
    }
		
	public class TestService
	{

	}
	public class TestOptions
	{
	}

	public class TestOptions<TService> : TestOptions where TService : class
	{
	}

	public class TestOptionsSetup : IConfigureOptions<TestOptions>
	{
		public void Configure(TestOptions options)
		{
			throw new System.NotImplementedException();
		}
	}
}

If MS.DI does not support the variance, then I need to turn it off here:

public static readonly Rules MicrosoftDependencyInjectionRules = new Rules(

@dvabuzyarov
Copy link
Author

@dadhi Thanks for you help, the settings completely fix the issue with the options and the test passed.

The only thing is should not IsRegistred respect the variance options and behave consistently with ResolveMany? I mean the case when IsRegistred return false for a type, but ResolveMany returns an instance for this type.

According to default behaviour of MS.DI I met this on my project working with Grpc interceptor configuration and can confirm that default behaviour for MS.DI and DryIoc is different (until apply the rule). I can provide more info or I can create a test project, so you can check it out.

@dadhi
Copy link
Owner

dadhi commented Apr 29, 2020

The only thing is should not IsRegistred respect the variance options

It is a hard question. Currently, IsRegistered checks exactly what you're asking for. In this case, IsRegistered can help you to find the discrepancies like this issue.

I can provide more info or I can create a test project, so you can check it out.

Yeah. More samples is better

I will keep this issue open - because we need to fix MS.DI rules anyway. PRs is welcome ;-)

@dadhi dadhi added good first issue Good for newcomers bug Something isn't working labels Apr 29, 2020
@dadhi dadhi added this to the v4.1.5 milestone Apr 29, 2020
@dadhi dadhi changed the title Incorrect resolving for generic types. [MS.DI] Incorrect resolving for generic types. Apr 29, 2020
@dvabuzyarov
Copy link
Author

I've prepared an example where you can test the issue. https://github.com/dvabuzyarov/dryioc_issue_example
git clone git@github.com:dvabuzyarov/dryioc_issue_example.git

You can easily switch between default implementation with MS.DI and DryIoc just enabling/disabling line 22 in Program.cs file.

@dadhi
Copy link
Owner

dadhi commented Apr 29, 2020

Good, until the fix is released you may pass a preconfigured container to new DryIocServiceProviderFactory(containerWithMyRules).

dadhi added a commit that referenced this issue May 17, 2020
…: #267 [MS.DI] Incorrect resolving for generic types
@dadhi dadhi linked a pull request May 17, 2020 that will close this issue
@dadhi dadhi self-assigned this May 17, 2020
dadhi added a commit that referenced this issue May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants