From 4938dab0ae8b508a4933f14f18e39c3d84aa6e69 Mon Sep 17 00:00:00 2001 From: Michel Schudel Date: Sun, 6 Jan 2019 22:35:44 +0100 Subject: [PATCH] Fix XML parser default value handling 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 --- .../xml/BeanDefinitionParserDelegate.java | 20 ++++---- ...edBeansElementAttributeRecursionTests.java | 48 ++++++++++++++++++- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/xml/BeanDefinitionParserDelegate.java b/spring-beans/src/main/java/org/springframework/beans/factory/xml/BeanDefinitionParserDelegate.java index 5da0bf53b2..80cb940426 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/xml/BeanDefinitionParserDelegate.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/xml/BeanDefinitionParserDelegate.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 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. @@ -320,21 +320,21 @@ public class BeanDefinitionParserDelegate { */ 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 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 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 sections, otherwise falling back to 'no'. autowire = (parentDefaults != null ? parentDefaults.getAutowire() : AUTOWIRE_NO_VALUE); } @@ -572,7 +572,7 @@ public class BeanDefinitionParserDelegate { } 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)); @@ -586,7 +586,7 @@ public class BeanDefinitionParserDelegate { } 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); @@ -661,7 +661,7 @@ public class BeanDefinitionParserDelegate { @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; @@ -1341,7 +1341,7 @@ public class BeanDefinitionParserDelegate { */ 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); @@ -1481,6 +1481,10 @@ public class BeanDefinitionParserDelegate { 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()))); } diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/xml/NestedBeansElementAttributeRecursionTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/xml/NestedBeansElementAttributeRecursionTests.java index 106e1d79b6..6d86208b66 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/xml/NestedBeansElementAttributeRecursionTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/xml/NestedBeansElementAttributeRecursionTests.java @@ -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. @@ -41,6 +41,21 @@ public class NestedBeansElementAttributeRecursionTests { 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"); @@ -61,6 +76,22 @@ public class NestedBeansElementAttributeRecursionTests { 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) topLevel.getSomeList(), hasItems("charlie", "delta")); @@ -84,6 +115,21 @@ public class NestedBeansElementAttributeRecursionTests { 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));