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

Allow for normal bean wiring semantics for types assignable to Map [SPR-7915] #12570

Closed
spring-projects-issues opened this issue Jan 27, 2011 · 3 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jan 27, 2011

Costin Leau opened SPR-7915 and commented

When injecting an entity that extends Map, the container automatically assumes the bean names/instances need to be injected even if that's not the case.
Example and discussion here: https://jira.springsource.org/browse/SGF-22


Affects: 3.0.5

Attachments:

Issue Links:

Referenced from: commits 4a0fa69

2 votes, 3 watchers

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Sep 22, 2011

Olaf Otto commented

I am also affected by this issue. Types compatible to Collection / Map lead to exceptions during autowiring.

The origin (as of 3.0.6.release) is in the DefaultListableBeanFactorie's method

doResolveDependency(DependencyDescriptor descriptor, Class<?> type, String beanName, Set<String> autowiredBeanNames, TypeConverter typeConverter):

(beanName being the name of the bean who's members are being autowired, type the Class of the field that is autowired)

  // -- snipp
else if (Collection.class.isAssignableFrom(type) && type.isInterface()) {
	Class elementType = descriptor.getCollectionType();
	if (elementType == null) {
		if (descriptor.isRequired()) {
			throw new FatalBeanException("No element type declared for collection [" + type.getName() + "]");
		}
		return null;
	}
	Map<String, Object> matchingBeans = findAutowireCandidates(beanName, elementType, descriptor);
	if (matchingBeans.isEmpty()) {
		if (descriptor.isRequired()) {
			raiseNoSuchBeanDefinitionException(elementType, "collection of " + elementType.getName(), descriptor);
		}
		return null;
	}
	if (autowiredBeanNames != null) {
		autowiredBeanNames.addAll(matchingBeans.keySet());
	}
	TypeConverter converter = (typeConverter != null ? typeConverter : getTypeConverter());
	return converter.convertIfNecessary(matchingBeans.values(), type);
}
else if (Map.class.isAssignableFrom(type) && type.isInterface()) {
	Class keyType = descriptor.getMapKeyType();
	if (keyType == null || !String.class.isAssignableFrom(keyType)) {
		if (descriptor.isRequired()) {
			throw new FatalBeanException("Key type [" + keyType + "] of map [" + type.getName() +
					"] must be assignable to [java.lang.String]");
		}
		return null;
	}
	Class valueType = descriptor.getMapValueType();
	if (valueType == null) {
		if (descriptor.isRequired()) {
			throw new FatalBeanException("No value type declared for map [" + type.getName() + "]");
		}
		return null;
	}
	Map<String, Object> matchingBeans = findAutowireCandidates(beanName, valueType, descriptor);
	if (matchingBeans.isEmpty()) {
		if (descriptor.isRequired()) {
			raiseNoSuchBeanDefinitionException(valueType, "map with value type " + valueType.getName(), descriptor);
		}
		return null;
	}
	if (autowiredBeanNames != null) {
		autowiredBeanNames.addAll(matchingBeans.keySet());
	}
	return matchingBeans;
}
else {

I think there are two issues with this:

  1. IMO wer're checking for a Bean here that is compatible to the field type that is to be autowired, i.e. it must be
type.isAssignableFrom(Collection.class);
and
type.isAssignableFrom(Map.class);

and not the other way around

  1. I think this should not be implicit behavior nut rather explicit. It's rather obvious one can get into a use case where this type of autowiring is not desirable but cannot be suppressed by other means than changing the field type to something that is not a collection.

In general, I think this is a rather esoteric use case...

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Sep 22, 2011

Olaf Otto commented

Attached a maven project verifying the bug against 3.0.6.RELEASE

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Dec 11, 2011

Chris Beams commented

This deserves more careful attention and testing than we can afford given the close proximity of 3.1 GA; slating for 3.2 Backlog.

In the meantime, for the use cases listed, switching to @Resource(name="beanName") should achieve the desired effect.

I do agree, however, that in cases such as @Autowired AssignableToMap foo; that the container should first beans matching the declared type before falling back to current @Autowired Map semantics, which is to to populate the map with beanName/beanInstance pairs.

Also note that I've added the reproduction case attached here to the spring-framework-issues project. See spring-attic/spring-framework-issues@f398411

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants