Skip to content

Commit

Permalink
Fix XML parser default value handling
Browse files Browse the repository at this point in the history
The xml parser does not fill in defaults provided in the XSD when
validation is disabled. As a result, attributes like default-lazy-init
will not receive the value "default" but an empty string.

With this commit, BeanDefinitionParserDelegate now takes this into
account, checking default values against empty string as well as
"default".

As a consequence, default-lazy-init attribute should now work correctly
even when the XSD validation is disabled.

Issue: SPR-8335
  • Loading branch information
MichelSchudel authored and sdeleuze committed Jan 8, 2019
1 parent 124e817 commit cba355a
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2017 the original author or authors.
* Copyright 2002-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -320,21 +320,21 @@ public void initDefaults(Element root, @Nullable BeanDefinitionParserDelegate pa
*/
protected void populateDefaults(DocumentDefaultsDefinition defaults, @Nullable DocumentDefaultsDefinition parentDefaults, Element root) {
String lazyInit = root.getAttribute(DEFAULT_LAZY_INIT_ATTRIBUTE);
if (DEFAULT_VALUE.equals(lazyInit)) {
if (isDefaultValue(lazyInit)) {
// Potentially inherited from outer <beans> sections, otherwise falling back to false.
lazyInit = (parentDefaults != null ? parentDefaults.getLazyInit() : FALSE_VALUE);
}
defaults.setLazyInit(lazyInit);

String merge = root.getAttribute(DEFAULT_MERGE_ATTRIBUTE);
if (DEFAULT_VALUE.equals(merge)) {
if (isDefaultValue(merge)) {
// Potentially inherited from outer <beans> sections, otherwise falling back to false.
merge = (parentDefaults != null ? parentDefaults.getMerge() : FALSE_VALUE);
}
defaults.setMerge(merge);

String autowire = root.getAttribute(DEFAULT_AUTOWIRE_ATTRIBUTE);
if (DEFAULT_VALUE.equals(autowire)) {
if (isDefaultValue(autowire)) {
// Potentially inherited from outer <beans> sections, otherwise falling back to 'no'.
autowire = (parentDefaults != null ? parentDefaults.getAutowire() : AUTOWIRE_NO_VALUE);
}
Expand Down Expand Up @@ -572,7 +572,7 @@ else if (containingBean != null) {
}

String lazyInit = ele.getAttribute(LAZY_INIT_ATTRIBUTE);
if (DEFAULT_VALUE.equals(lazyInit)) {
if (isDefaultValue(lazyInit)) {
lazyInit = this.defaults.getLazyInit();
}
bd.setLazyInit(TRUE_VALUE.equals(lazyInit));
Expand All @@ -586,7 +586,7 @@ else if (containingBean != null) {
}

String autowireCandidate = ele.getAttribute(AUTOWIRE_CANDIDATE_ATTRIBUTE);
if ("".equals(autowireCandidate) || DEFAULT_VALUE.equals(autowireCandidate)) {
if (isDefaultValue(autowireCandidate)) {
String candidatePattern = this.defaults.getAutowireCandidates();
if (candidatePattern != null) {
String[] patterns = StringUtils.commaDelimitedListToStringArray(candidatePattern);
Expand Down Expand Up @@ -661,7 +661,7 @@ public void parseMetaElements(Element ele, BeanMetadataAttributeAccessor attribu
@SuppressWarnings("deprecation")
public int getAutowireMode(String attValue) {
String att = attValue;
if (DEFAULT_VALUE.equals(att)) {
if (isDefaultValue(att)) {
att = this.defaults.getAutowire();
}
int autowire = AbstractBeanDefinition.AUTOWIRE_NO;
Expand Down Expand Up @@ -1341,7 +1341,7 @@ public Properties parsePropsElement(Element propsEle) {
*/
public boolean parseMergeAttribute(Element collectionElement) {
String value = collectionElement.getAttribute(MERGE_ATTRIBUTE);
if (DEFAULT_VALUE.equals(value)) {
if (isDefaultValue(value)) {
value = this.defaults.getMerge();
}
return TRUE_VALUE.equals(value);
Expand Down Expand Up @@ -1481,6 +1481,10 @@ public boolean isDefaultNamespace(Node node) {
return isDefaultNamespace(getNamespaceURI(node));
}

private boolean isDefaultValue(String value) {
return (DEFAULT_VALUE.equals(value) || "".equals(value));
}

private boolean isCandidateElement(Node node) {
return (node instanceof Element && (isDefaultNamespace(node) || !isDefaultNamespace(node.getParentNode())));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -41,6 +41,21 @@ public void defaultLazyInit() {
new XmlBeanDefinitionReader(bf).loadBeanDefinitions(
new ClassPathResource("NestedBeansElementAttributeRecursionTests-lazy-context.xml", this.getClass()));

assertLazyInits(bf);
}

@Test
public void defaultLazyInitWithNonValidatingParser() {
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
XmlBeanDefinitionReader xmlBeanDefinitionReader = new XmlBeanDefinitionReader(bf);
xmlBeanDefinitionReader.setValidating(false);
xmlBeanDefinitionReader.loadBeanDefinitions(
new ClassPathResource("NestedBeansElementAttributeRecursionTests-lazy-context.xml", this.getClass()));

assertLazyInits(bf);
}

private void assertLazyInits(DefaultListableBeanFactory bf) {
BeanDefinition foo = bf.getBeanDefinition("foo");
BeanDefinition bar = bf.getBeanDefinition("bar");
BeanDefinition baz = bf.getBeanDefinition("baz");
Expand All @@ -61,6 +76,22 @@ public void defaultMerge() {
new XmlBeanDefinitionReader(bf).loadBeanDefinitions(
new ClassPathResource("NestedBeansElementAttributeRecursionTests-merge-context.xml", this.getClass()));

assertMerge(bf);
}

@Test
@SuppressWarnings("unchecked")
public void defaultMergeWithNonValidatingParser() {
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
XmlBeanDefinitionReader xmlBeanDefinitionReader = new XmlBeanDefinitionReader(bf);
xmlBeanDefinitionReader.setValidating(false);
xmlBeanDefinitionReader.loadBeanDefinitions(
new ClassPathResource("NestedBeansElementAttributeRecursionTests-merge-context.xml", this.getClass()));

assertMerge(bf);
}

private void assertMerge(DefaultListableBeanFactory bf) {
TestBean topLevel = bf.getBean("topLevelConcreteTestBean", TestBean.class);
// has the concrete child bean values
assertThat((Iterable<String>) topLevel.getSomeList(), hasItems("charlie", "delta"));
Expand All @@ -84,6 +115,21 @@ public void defaultAutowireCandidates() {
new XmlBeanDefinitionReader(bf).loadBeanDefinitions(
new ClassPathResource("NestedBeansElementAttributeRecursionTests-autowire-candidates-context.xml", this.getClass()));

assertAutowireCandidates(bf);
}

@Test
public void defaultAutowireCandidatesWithNonValidatingParser() {
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
XmlBeanDefinitionReader xmlBeanDefinitionReader = new XmlBeanDefinitionReader(bf);
xmlBeanDefinitionReader.setValidating(false);
xmlBeanDefinitionReader.loadBeanDefinitions(
new ClassPathResource("NestedBeansElementAttributeRecursionTests-autowire-candidates-context.xml", this.getClass()));

assertAutowireCandidates(bf);
}

private void assertAutowireCandidates(DefaultListableBeanFactory bf) {
assertThat(bf.getBeanDefinition("fooService").isAutowireCandidate(), is(true));
assertThat(bf.getBeanDefinition("fooRepository").isAutowireCandidate(), is(true));
assertThat(bf.getBeanDefinition("other").isAutowireCandidate(), is(false));
Expand Down

0 comments on commit cba355a

Please sign in to comment.