trquth / autofac

Automatically exported from code.google.com/p/autofac
Other
0 stars 0 forks source link

Empty Lists are create for types not registered in the container. #471

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?

1. Create an empty container
2. Call container.InjectProperties
3. dep1.List shouldn't be resolved as no types of Dep2 have been registered in 
the container.

What is the expected output? What do you see instead?

In 3.1.1 the attached code works as expected. In 3.1.3 the list gets set to an 
empty list 

What version of Autofac are you using? On what version of .NET/Silverlight?

Version 3.1.1  & .net 4.5 

Please provide any additional information below.

Original issue reported on code.google.com by toddby...@gmail.com on 13 Nov 2013 at 1:58

Attachments:

GoogleCodeExporter commented 8 years ago

Original comment by travis.illig on 14 Nov 2013 at 4:29

GoogleCodeExporter commented 8 years ago
This appears to be related to the root cause of Issue #430. I won't combine 
them quite yet since there's a workaround for Issue #430 that won't solve this; 
however, if we decide to fix the root issue, we can combine.

Original comment by travis.illig on 16 Dec 2013 at 7:22

GoogleCodeExporter commented 8 years ago

Original comment by travis.illig on 16 Dec 2013 at 7:23

GoogleCodeExporter commented 8 years ago
This changed because the standard collection support 
(https://code.google.com/p/autofac/wiki/RelationshipTypes) was enhanced to 
support not only IEnumerable<T> but also IList<T> and ICollection<T>.

The collection support will always return an empty list if you ask it for a 
list of items that aren't registered in the container. The major use case is 
for things like handlers - a class might need to iterate through a list of, 
say, message handlers, and it's totally valid that no handlers might be 
registered. Thus, returning an empty list rather than a null value is by design.

It is recommended that you update your code to only ask the container for 
things you expect it to resolve.

I have updated the documentation on the 
https://code.google.com/p/autofac/wiki/RelationshipTypes page to include a 
warning that this is the expected behavior.

Original comment by travis.illig on 16 Dec 2013 at 9:54

GoogleCodeExporter commented 8 years ago
Hi Travis,

We are using Autofac to resolve our presenters and service classes in a c# 
Webforms project.

Before this change anything that was not registered in AutoFac wasn't called. 
Now we have Autofac setting lists of our domain objects, in our views. 

Even if we rewrite the Webforms injection code we still won't be able to tell 
if autofac really manages a resource or not without calling resolve twice. 

I am not sure making it so someone doesn't need to check for null on a IList<T> 
is worth breaking so much existing code that relies on autofac.

1) Can't we meet half way and make it so the new IEnumerable<T>,IList<T>, and 
ICollection<T> Resolver only returns empty lists if enabled?

2) Open up the new Autofac.Features.Collections.* so they can be overriden 
  a) Make it easier to just override a single Autofac.Features.* with custom code with out having to register all of the feature again.

Original comment by toddby...@gmail.com on 16 Dec 2013 at 10:28

GoogleCodeExporter commented 8 years ago
The only real change that was made was to add support for IList<T> and 
ICollection<T> that lives alongside IEnumerable<T>. The behavior for 
IEnumerable<T> didn't actually change - it has always returned an empty 
enumerable.

The change for IList<T> and ICollection<T> was made as part of the fix for 
Issue #464 in revision 0a2004001e2b 
(https://code.google.com/p/autofac/source/browse/Core/Source/Autofac/Features/Co
llections/CollectionRegistrationSource.cs?spec=svn54a7de7c99d1c61d178733e2715a1c
f59a185002&r=0a2004001e2b6c3d24af96042128296169b7ed4a).

The implicit collection support is implemented by a registration source, not a 
new "resolver" and it's always on - there's no "switch" to turn it off, so we 
can't offer that.

We're not ready to make the stuff in that namespace part of the publicly 
supported API, either. As soon as we make it public, it means we'll always have 
to support extensions based on the fact that it's public - larger API footprint 
and all. I don't think we want to support that at this time.

If you take the tack of only asking the container for things you want it to 
manage, you won't need to call resolve twice. I might be misunderstanding, but:

* If it's a constructor parameter, it's safe to assume you want the container 
to try to resolve it.
* If it's a null property and you call property injection on the object, it's 
safe to assume you want the container to try to resolve it.
* If it's NOT a null property and you call property injection on the object, 
you may or may not want the container to override it.

It's that third case that is the tricky one, but it can be handled. For web 
forms, use the UnsetPropertyInjectionModule rather than the 
PropertyInjectionModule in web.config. For other objects you might register, 
register them using the "PropertyWiringOptions.PreserveSetValues" option:

builder
  .RegisterType<MyType>()
  .PropertiesAutowired(PropertyWiringOptions.PreserveSetValues);

Original comment by travis.illig on 16 Dec 2013 at 10:45

GoogleCodeExporter commented 8 years ago
Yeah the problem is pages can have allot of properties that should not be 
resolved from the container, and we can't use construction injection like we do 
for the other pieces. 

I had hoped there was something along the lines of 
builder.Build(ContainerBuildOptions.ExcludeDefaultModules) but more selective 
instead of all of them.

The easiest way forward then is to just write my own PropertyInjectionModule & 
InjectProperties methods and call resolve twice to avoid getting empty lists.

Thanks for the help.

Todd

Original comment by toddby...@gmail.com on 16 Dec 2013 at 11:17