Skip to content

Resolve combinations of built in types #72

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

Closed
ENikS opened this issue Mar 26, 2018 · 8 comments
Closed

Resolve combinations of built in types #72

ENikS opened this issue Mar 26, 2018 · 8 comments

Comments

@ENikS
Copy link
Contributor

ENikS commented Mar 26, 2018

Problem

Unity supports IEnumerable, Array, and Lazy. Theoretically it should be able to distinguish these types:

IEnumerable<T>
IEunmerable<Lazy<T>>
Lazy<T>
Lazy<IEnumerable<T>>
Lazy<T>[]
@ENikS ENikS closed this as completed in dfc3afa Mar 26, 2018
@ENikS
Copy link
Contributor Author

ENikS commented Mar 27, 2018

The container should be able to resolve chain of generics like this: IEnumerable<Lazy<IList<T>>> if T is registered.

@ENikS ENikS reopened this Mar 27, 2018
@patrickrobbins
Copy link

The solutions break existing strategies. Please consider adding a default null for the additional parameters. This is the second breaking API in the 5.x branch.

@ENikS
Copy link
Contributor Author

ENikS commented Mar 27, 2018

Would be more productive if you could provide more context or a sample of what you are talking about. Where would you like me to add null?

I know a lot of Unity bugs became 'features' and there are solutions to work around them. If you want to stay with these solutions, just do not upgrade.

@patrickrobbins
Copy link

The following would allow anything using these public methods to continue using them without breaking the packages. (I.E. I can upgrade to the latest even if I have a 3rd dependency that is calling the single param constructor)

public ArrayResolveStrategy(MethodInfo method, MethodInfo lazy) => public ArrayResolveStrategy(MethodInfo method, MethodInfo lazy = null)

public ArrayResolveStrategy(MethodInfo method, MethodInfo lazy) => public ArrayResolveStrategy(MethodInfo method, MethodInfo lazy = null)

public EnumerableResolveStrategy(MethodInfo method, MethodInfo lazy) => public EnumerableResolveStrategy(MethodInfo method, MethodInfo lazy = null)

@ENikS
Copy link
Contributor Author

ENikS commented Mar 27, 2018

Unfortunately adding null would not solve the issue. It has to be provided now.
... and this is indeed breaking change and I should have increased minor version instead of patch, sorry for that.

@ENikS ENikS closed this as completed in 34db976 Mar 27, 2018
@pksorensen
Copy link
Contributor

looks like this also broke some working aspnet core applications

Here is everything that I could find on it.
unitycontainer/microsoft-dependency-injection#15

I think what you describe here is exactly the problem

The container should be able to resolve chain of generics like this: IEnumerable<Lazy<IList>> if T is registered.

maybe we need a way to disable this behavior @ENikS ?

@pksorensen
Copy link
Contributor

Rethinking, the feature is ok. no need for disabling. It must be a bug that GetFinalType returns IdentityServerOptions when resovling IEnumerable<IPostConfigureOptions>
due to IdentityServerOptions beign registered.

@pksorensen
Copy link
Contributor

Would you accept a PR that changes this according to comment below?

 [SuppressMessage("ReSharper", "InconsistentlySynchronizedField")]
        internal Type GetFinalType(Type argType)
        {
            Type next;
            for (var type = argType; null != type; type = next)
            {
                var info = type.GetTypeInfo();
                
                if (info.IsGenericType) //this should be IsEnumerable || IsLazy only ? not all other generics
                {
                    
                    if (_isTypeExplicitlyRegistered(type)) return type;

                    var definition = info.GetGenericTypeDefinition();
                    if (_isTypeExplicitlyRegistered(definition)) return definition;

                    next = info.GenericTypeArguments[0];
                    if (_isTypeExplicitlyRegistered(next)) return next;
                }
                else if (type.IsArray)
                {
                    next = type.GetElementType();
                    if (_isTypeExplicitlyRegistered(next)) return next;
                }
                else
                {
                    return type;
                }
            }

            return argType;
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants