From d2a5aeb616f801d64917149c139eaa6fe1880cb3 Mon Sep 17 00:00:00 2001 From: dhgarrette Date: Sat, 14 Mar 2009 02:58:20 +0000 Subject: [PATCH] BATCH-1080: BATCH-1145: Since both and set directly to the *StepFactoryBean, their "parent" attributes were overwriting each other. As a solution, the top-level element has been removed. To achieve the same result, the user can use an abstract top-level step with nothing declared but the . --- .../configuration/xml/AbstractStepParser.java | 37 +++++++++--- .../xml/CoreNamespaceHandler.java | 1 - .../xml/StandaloneStepParser.java | 1 - .../xml/TaskletElementParser.java | 60 ++++++------------- .../xml/TopLevelTaskletElementParser.java | 46 -------------- .../configuration/xml/spring-batch-2.0.xsd | 19 ------ ...mentParentAttributeParserTests-context.xml | 42 +++++++------ .../src/main/resources/jobs/skipSampleJob.xml | 29 ++++++--- 8 files changed, 85 insertions(+), 150 deletions(-) delete mode 100644 spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/TopLevelTaskletElementParser.java diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/AbstractStepParser.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/AbstractStepParser.java index 2767d6712..165b2461a 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/AbstractStepParser.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/AbstractStepParser.java @@ -50,15 +50,16 @@ public abstract class AbstractStepParser { private StepListenerParser stepListenerParser = new StepListenerParser(); /** - * @param element + * @param stepElement * @param parserContext * @return a BeanDefinition if possible */ - protected AbstractBeanDefinition parseTasklet(Element element, ParserContext parserContext, String jobRepositoryRef) { + protected AbstractBeanDefinition parseTasklet(Element stepElement, ParserContext parserContext, + String jobRepositoryRef) { - String taskletRef = element.getAttribute("tasklet"); + String taskletRef = stepElement.getAttribute("tasklet"); @SuppressWarnings("unchecked") - List taskletElements = (List) DomUtils.getChildElementsByTagName(element, "tasklet"); + List taskletElements = (List) DomUtils.getChildElementsByTagName(stepElement, "tasklet"); boolean taskletElementExists = taskletElements.size() > 0; AbstractBeanDefinition bd = null; if (StringUtils.hasText(taskletRef)) { @@ -66,20 +67,35 @@ public abstract class AbstractStepParser { parserContext.getReaderContext().error( "The <" + taskletElements.get(0).getNodeName() + "> element can't be combined with the 'tasklet=\"" + taskletRef - + "\"' attribute specification for <" + element.getNodeName() + ">", element); + + "\"' attribute specification for <" + stepElement.getNodeName() + ">", stepElement); } - bd = parseTaskletRef(element, taskletRef, parserContext, jobRepositoryRef); - setUpBeanDefinition(element, bd, parserContext, jobRepositoryRef); + bd = parseTaskletRef(stepElement, taskletRef, parserContext, jobRepositoryRef); } else if (taskletElementExists) { Element taskElement = taskletElements.get(0); - bd = taskletElementParser.parse(taskElement, parserContext); - setUpBeanDefinition(element, bd, parserContext, jobRepositoryRef); + bd = taskletElementParser.parse(taskElement, parserContext, stepUnderspecified(stepElement)); } + + if (bd != null) { + setUpBeanDefinition(stepElement, bd, parserContext, jobRepositoryRef); + } + return bd; } + /** + * Should this step should be treated as incomplete? If it has a parent or + * is abstract, then it may not have all properties. + * + * @param stepElement + * @return TRUE if + */ + private boolean stepUnderspecified(Element stepElement) { + return Boolean.valueOf(stepElement.getAttribute("abstract")) + || StringUtils.hasText(stepElement.getAttribute("parent")); + } + /** * @param stepElement * @param taskletRef @@ -105,6 +121,8 @@ public abstract class AbstractStepParser { String jobRepositoryRef) { checkStepAttributes(stepElement, bd); + bd.setAbstract(stepElement.hasAttribute("abstract") && Boolean.valueOf(stepElement.getAttribute("abstract"))); + RuntimeBeanReference jobRepositoryBeanRef = new RuntimeBeanReference(jobRepositoryRef); bd.getPropertyValues().addPropertyValue("jobRepository", jobRepositoryBeanRef); @@ -131,6 +149,7 @@ public abstract class AbstractStepParser { bd.setRole(BeanDefinition.ROLE_SUPPORT); bd.setSource(parserContext.extractSource(stepElement)); + } private void checkStepAttributes(Element stepElement, AbstractBeanDefinition bd) { diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/CoreNamespaceHandler.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/CoreNamespaceHandler.java index 7e963fccd..9765c9fe0 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/CoreNamespaceHandler.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/CoreNamespaceHandler.java @@ -32,7 +32,6 @@ public class CoreNamespaceHandler extends NamespaceHandlerSupport { public void init() { this.registerBeanDefinitionParser("job", new JobParser()); this.registerBeanDefinitionParser("step", new TopLevelStepParser()); - this.registerBeanDefinitionParser("tasklet", new TopLevelTaskletElementParser()); this.registerBeanDefinitionParser("job-repository", new JobRepositoryParser()); this.registerBeanDefinitionParser("job-listener", new TopLevelJobListenerParser()); this.registerBeanDefinitionParser("step-listener", new TopLevelStepListenerParser()); diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/StandaloneStepParser.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/StandaloneStepParser.java index adfef7a3d..84bb8d097 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/StandaloneStepParser.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/StandaloneStepParser.java @@ -44,7 +44,6 @@ public class StandaloneStepParser extends AbstractStepParser { bd = new GenericBeanDefinition(); setUpBeanDefinition(element, bd, parserContext, element.getAttribute("job-repository")); } - bd.setAbstract(Boolean.valueOf(element.getAttribute("abstract"))); return bd; } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/TaskletElementParser.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/TaskletElementParser.java index 8f1c0f978..f79891241 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/TaskletElementParser.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/TaskletElementParser.java @@ -48,48 +48,27 @@ public class TaskletElementParser { * @param element * @param parserContext */ - protected AbstractBeanDefinition parse(Element element, ParserContext parserContext) { - - boolean isFaultTolerant = false; + protected AbstractBeanDefinition parse(Element element, ParserContext parserContext, boolean underspecified) { String skipLimit = element.getAttribute("skip-limit"); - if (!isFaultTolerant) { - isFaultTolerant = checkIntValueForFaultToleranceNeeded(skipLimit); - } String retryLimit = element.getAttribute("retry-limit"); - if (!isFaultTolerant) { - isFaultTolerant = checkIntValueForFaultToleranceNeeded(retryLimit); - } String cacheCapacity = element.getAttribute("cache-capacity"); - if (!isFaultTolerant) { - isFaultTolerant = checkIntValueForFaultToleranceNeeded(cacheCapacity); - } String isReaderTransactionalQueue = element.getAttribute("is-reader-transactional-queue"); - if (!isFaultTolerant && StringUtils.hasText(isReaderTransactionalQueue)) { - if ("true".equals(isReaderTransactionalQueue)) { - isFaultTolerant = true; - } - } - checkExceptionElementForFaultToleranceNeeded(element, "skippable-exception-classes"); - checkExceptionElementForFaultToleranceNeeded(element, "retryable-exception-classes"); - checkExceptionElementForFaultToleranceNeeded(element, "fatal-exception-classes"); + + boolean useFaultTolerant = underspecified + || (StringUtils.hasText(isReaderTransactionalQueue) && Boolean.valueOf(isReaderTransactionalQueue)) + || isPositive(skipLimit) || isPositive(retryLimit) || isPositive(cacheCapacity) + || hasElement(element, "skippable-exception-classes") + || hasElement(element, "retryable-exception-classes") || hasElement(element, "fatal-exception-classes"); GenericBeanDefinition bd = new GenericBeanDefinition(); - if (isFaultTolerant) { + if (useFaultTolerant) { bd.setBeanClass(FaultTolerantStepFactoryBean.class); } else { bd.setBeanClass(SimpleStepFactoryBean.class); } - boolean isAbstract = Boolean.valueOf(element.getAttribute("abstract")); - bd.setAbstract(isAbstract); - - String parentRef = element.getAttribute("parent"); - if (StringUtils.hasText(parentRef)) { - bd.setParentName(parentRef); - } - MutablePropertyValues propertyValues = bd.getPropertyValues(); String readerBeanId = element.getAttribute("reader"); @@ -127,7 +106,7 @@ public class TaskletElementParser { propertyValues.addPropertyValue("chunkCompletionPolicy", completionPolicy); } - if (!isAbstract + if (!underspecified && propertyValues.contains("commitInterval") == propertyValues.contains("chunkCompletionPolicy")) { parserContext.getReaderContext().error( "The 'tasklet' element must contain either 'commit-interval' " @@ -147,19 +126,19 @@ public class TaskletElementParser { } if (StringUtils.hasText(isReaderTransactionalQueue)) { - if (isFaultTolerant) { + if (useFaultTolerant) { propertyValues.addPropertyValue("isReaderTransactionalQueue", isReaderTransactionalQueue); } } handleExceptionElement(element, parserContext, bd, "skippable-exception-classes", "skippableExceptionClasses", - isFaultTolerant, isAbstract); + useFaultTolerant, underspecified); handleExceptionElement(element, parserContext, bd, "retryable-exception-classes", "retryableExceptionClasses", - isFaultTolerant, isAbstract); + useFaultTolerant, underspecified); handleExceptionElement(element, parserContext, bd, "fatal-exception-classes", "fatalExceptionClasses", - isFaultTolerant, isAbstract); + useFaultTolerant, underspecified); handleRetryListenersElement(element, bd, parserContext); @@ -169,22 +148,17 @@ public class TaskletElementParser { } - private boolean checkIntValueForFaultToleranceNeeded(String stringValue) { + private boolean isPositive(String stringValue) { if (StringUtils.hasText(stringValue)) { - int value = Integer.valueOf(stringValue); - if (value > 0) { + if (Integer.valueOf(stringValue) > 0) { return true; } } return false; } - private boolean checkExceptionElementForFaultToleranceNeeded(Element element, String subElementName) { - String exceptions = DomUtils.getChildElementValueByTagName(element, subElementName); - if (StringUtils.hasLength(exceptions)) { - return true; - } - return false; + private boolean hasElement(Element element, String subElementName) { + return StringUtils.hasLength(DomUtils.getChildElementValueByTagName(element, subElementName)); } @SuppressWarnings("unchecked") diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/TopLevelTaskletElementParser.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/TopLevelTaskletElementParser.java deleted file mode 100644 index 1ed6b9f8c..000000000 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/TopLevelTaskletElementParser.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright 2006-2008 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 org.springframework.beans.factory.support.AbstractBeanDefinition; -import org.springframework.beans.factory.xml.AbstractBeanDefinitionParser; -import org.springframework.beans.factory.xml.ParserContext; -import org.w3c.dom.Element; - -/** - * Parser for the lt;tasklet/gt; top level element in the Batch namespace. Sets - * up and returns a bean definition. - * - * @author Dan Garrette - * @since 2.0 - */ -public class TopLevelTaskletElementParser extends AbstractBeanDefinitionParser { - - private TaskletElementParser taskletElementParser = new TaskletElementParser(); - - @Override - protected AbstractBeanDefinition parseInternal(Element element, ParserContext parserContext) { - - if (!Boolean.valueOf(element.getAttribute("abstract"))) { - parserContext.getReaderContext().error( - "The element, when not contained with a , must be abstract", element); - } - - return taskletElementParser.parse(element, parserContext); - - } - -} diff --git a/spring-batch-core/src/main/resources/org/springframework/batch/core/configuration/xml/spring-batch-2.0.xsd b/spring-batch-core/src/main/resources/org/springframework/batch/core/configuration/xml/spring-batch-2.0.xsd index eab4b3526..421541a06 100644 --- a/spring-batch-core/src/main/resources/org/springframework/batch/core/configuration/xml/spring-batch-2.0.xsd +++ b/spring-batch-core/src/main/resources/org/springframework/batch/core/configuration/xml/spring-batch-2.0.xsd @@ -111,24 +111,6 @@ - - - - A bean definition for a tasklet that can be injected into a Step. - Useful for creating a "base" - tasklet from which others can extend. - - - - - - - - - - - - - diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/TaskletElementParentAttributeParserTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/TaskletElementParentAttributeParserTests-context.xml index c78f1d958..873b64bc6 100644 --- a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/TaskletElementParentAttributeParserTests-context.xml +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/TaskletElementParentAttributeParserTests-context.xml @@ -7,10 +7,8 @@ - - + + java.lang.NullPointerException @@ -26,10 +24,8 @@ - - + + java.lang.NullPointerException @@ -46,20 +42,22 @@ - - - java.lang.ArithmeticException - - - org.springframework.dao.DeadlockLoserDataAccessException - - - - - - - - + + + + java.lang.ArithmeticException + + + org.springframework.dao.DeadlockLoserDataAccessException + + + + + + + + + diff --git a/spring-batch-samples/src/main/resources/jobs/skipSampleJob.xml b/spring-batch-samples/src/main/resources/jobs/skipSampleJob.xml index b31173d45..f18eca14c 100644 --- a/spring-batch-samples/src/main/resources/jobs/skipSampleJob.xml +++ b/spring-batch-samples/src/main/resources/jobs/skipSampleJob.xml @@ -35,23 +35,34 @@ - - - - - org.springframework.batch.item.validator.ValidationException - java.lang.RuntimeException + + + + + java.lang.RuntimeException - + - + + + + + + + org.springframework.batch.item.validator.ValidationException + + + + + +