diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/StepParser.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/StepParser.java index a0b308522..76b8ff86a 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/StepParser.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/StepParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2008 the original author or authors. + * Copyright 2006-2009 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. @@ -418,21 +418,8 @@ public class StepParser { String id = listenerElement.getAttribute("id"); String listenerRef = listenerElement.getAttribute("ref"); String className = listenerElement.getAttribute("class"); - if ((StringUtils.hasText(id) || StringUtils.hasText(className)) - && StringUtils.hasText(listenerRef)) { - NamedNodeMap attributeNodes = listenerElement.getAttributes(); - StringBuilder attributes = new StringBuilder(); - for (int i = 0; i < attributeNodes.getLength(); i++) { - if (i > 0) { - attributes.append(" "); - } - attributes.append(attributeNodes.item(i)); - } - parserContext.getReaderContext().error("Both 'ref' and " + - (StringUtils.hasText(id) ? "'id'" : "'class'") + - " specified; use 'class' with an optional 'id' or just 'ref' for <" + - listenerElement.getTagName() + "> element specified with attributes: " + attributes, element); - } + checkListenerElementAttributes(parserContext, element, + listenerElement, id, listenerRef, className); if (StringUtils.hasText(listenerRef)) { BeanReference bean = new RuntimeBeanReference(listenerRef); beans.add(bean); @@ -465,21 +452,8 @@ public class StepParser { String id = listenerElement.getAttribute("id"); String listenerRef = listenerElement.getAttribute("ref"); String className = listenerElement.getAttribute("class"); - if ((StringUtils.hasText(id) || StringUtils.hasText(className)) - && StringUtils.hasText(listenerRef)) { - NamedNodeMap attributeNodes = listenerElement.getAttributes(); - StringBuilder attributes = new StringBuilder(); - for (int i = 0; i < attributeNodes.getLength(); i++) { - if (i > 0) { - attributes.append(" "); - } - attributes.append(attributeNodes.item(i)); - } - parserContext.getReaderContext().error("Both 'ref' and " + - (StringUtils.hasText(id) ? "'id'" : "'class'") + - " specified; use 'class' with an optional 'id' or just 'ref' for <" + - listenerElement.getTagName() + "> element specified with attributes: " + attributes, element); - } + checkListenerElementAttributes(parserContext, element, + listenerElement, id, listenerRef, className); if (StringUtils.hasText(listenerRef)) { listenerBuilder.addPropertyReference("delegate", listenerRef); } @@ -531,6 +505,26 @@ public class StepParser { } } + private void checkListenerElementAttributes(ParserContext parserContext, + Element element, Element listenerElement, String id, + String listenerRef, String className) { + if ((StringUtils.hasText(id) || StringUtils.hasText(className)) + && StringUtils.hasText(listenerRef)) { + NamedNodeMap attributeNodes = listenerElement.getAttributes(); + StringBuilder attributes = new StringBuilder(); + for (int i = 0; i < attributeNodes.getLength(); i++) { + if (i > 0) { + attributes.append(" "); + } + attributes.append(attributeNodes.item(i)); + } + parserContext.getReaderContext().error("Both 'ref' and " + + (StringUtils.hasText(id) ? "'id'" : "'class'") + + " specified; use 'class' with an optional 'id' or just 'ref' for <" + + listenerElement.getTagName() + "> element specified with attributes: " + attributes, element); + } + } + @SuppressWarnings("unchecked") private void handleStreamsElement(Element element, RootBeanDefinition bd, ParserContext parserContext) { Element streamsElement = diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/RepositoryJobParserTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/RepositoryJobParserTests.java index 0b36af31e..2542b4e63 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/RepositoryJobParserTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/RepositoryJobParserTests.java @@ -54,7 +54,7 @@ public class RepositoryJobParserTests { } @Test - public void testJobWithRepository() throws Exception { + public void testTaskletStepWithBadListener() throws Exception { assertNotNull(job); JobExecution jobExecution = jobRepository.createJobExecution(job.getName(), new JobParameters()); job.execute(jobExecution); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/StepParserTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/StepParserTests.java new file mode 100644 index 000000000..59a9ddab2 --- /dev/null +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/StepParserTests.java @@ -0,0 +1,51 @@ +/* + * Copyright 2006-2009 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.batch.core.configuration.xml; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import org.junit.Test; +import org.springframework.beans.factory.parsing.BeanDefinitionParsingException; +import org.springframework.context.support.ClassPathXmlApplicationContext; + + +/** + * @author Thomas Risberg + */ +public class StepParserTests { + + @Test + public void testTaskletStepWithBadStepListener() throws Exception { + loadContextWithBadListener("org/springframework/batch/core/configuration/xml/StepParserBadStepListenerTests-context.xml"); + } + + @Test + public void testTaskletStepWithBadRetryListener() throws Exception { + loadContextWithBadListener("org/springframework/batch/core/configuration/xml/StepParserBadRetryListenerTests-context.xml"); + } + + private void loadContextWithBadListener(String contextLocation) { + try { + new ClassPathXmlApplicationContext(contextLocation); + fail("Context should not load!"); + } + catch (BeanDefinitionParsingException e) { + assertTrue(e.getMessage().contains("'ref' and 'class'")); + } + } + +} diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepParserBadRetryListenerTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepParserBadRetryListenerTests-context.xml new file mode 100644 index 000000000..f8972126c --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepParserBadRetryListenerTests-context.xml @@ -0,0 +1,45 @@ + + + + + + + + + + + + + + + + org.springframework.dao.DataIntegrityViolationException, + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepParserBadStepListenerTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepParserBadStepListenerTests-context.xml new file mode 100644 index 000000000..cae1ad809 --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepParserBadStepListenerTests-context.xml @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + + + + + \ No newline at end of file