From a1ee097dd4a3d69f3043d8b4bf79c23b1c27fcf4 Mon Sep 17 00:00:00 2001 From: Chris Schaefer Date: Mon, 10 Feb 2014 09:46:28 -0500 Subject: [PATCH] * During pre-parsing of XML look for references to the same bean and if multiple matches found, create new bean definitions and update the reference in the preprocessed XML to allow for unique instances. * Use job scope for job level listeners * Remove call to application context close in JsrJobOperator.restart Fixes TCK tests: -- testTransitionElementOnAttrValuesWithRestartJobParamOverrides testChunkArtifactInstanceUniqueness testOneArtifactIsJobAndStepListener --- .../BatchPropertyBeanPostProcessor.java | 15 +-- .../xml/JsrBeanDefinitionDocumentReader.java | 47 ++++++- .../jsr/configuration/xml/ListenerParser.java | 5 +- .../batch/core/jsr/launch/JsrJobOperator.java | 2 - .../JsrBeanDefinitionDocumentReaderTests.java | 119 ++++++++++++++++++ .../xml/ListenerParserTests.java | 4 +- .../ExitStatusSettingStepListener.java | 40 ++++++ .../batch-jobs/jsrSpringInstanceTests.xml | 45 +++++++ .../batch-jobs/jsrUniqueInstanceTests.xml | 53 ++++++++ .../src/test/resources/META-INF/batch.xml | 1 + 10 files changed, 313 insertions(+), 18 deletions(-) create mode 100644 spring-batch-core/src/test/java/org/springframework/batch/core/jsr/step/listener/ExitStatusSettingStepListener.java create mode 100644 spring-batch-core/src/test/resources/META-INF/batch-jobs/jsrSpringInstanceTests.xml create mode 100644 spring-batch-core/src/test/resources/META-INF/batch-jobs/jsrUniqueInstanceTests.xml diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/support/BatchPropertyBeanPostProcessor.java b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/support/BatchPropertyBeanPostProcessor.java index 9ef662dc0..7a644b6a7 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/support/BatchPropertyBeanPostProcessor.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/support/BatchPropertyBeanPostProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2014 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. @@ -94,18 +94,19 @@ public class BatchPropertyBeanPostProcessor implements BeanPostProcessor, BeanFa } private Properties getArtifactProperties(String artifactName) { + String originalArtifactName = artifactName; + + if(originalArtifactName.startsWith(SCOPED_TARGET_BEAN_PREFIX)) { + originalArtifactName = artifactName.substring(SCOPED_TARGET_BEAN_PREFIX.length()); + } + StepContext stepContext = StepSynchronizationManager.getContext(); if (stepContext != null) { - String originalArtifactName = artifactName; - if(originalArtifactName.startsWith(SCOPED_TARGET_BEAN_PREFIX)) { - originalArtifactName = artifactName.substring(SCOPED_TARGET_BEAN_PREFIX.length()); - } - return batchPropertyContext.getStepArtifactProperties(stepContext.getStepName(), originalArtifactName); } - return batchPropertyContext.getArtifactProperties(artifactName); + return batchPropertyContext.getArtifactProperties(originalArtifactName); } private void injectBatchProperties(final Object artifact, final Properties artifactProperties) { diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/xml/JsrBeanDefinitionDocumentReader.java b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/xml/JsrBeanDefinitionDocumentReader.java index 25d4baac9..80f52bf52 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/xml/JsrBeanDefinitionDocumentReader.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/xml/JsrBeanDefinitionDocumentReader.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2014 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. @@ -24,6 +24,8 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.batch.core.jsr.configuration.support.JsrExpressionParser; import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.support.AbstractBeanDefinition; +import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.xml.DefaultBeanDefinitionDocumentReader; import org.w3c.dom.Element; @@ -37,11 +39,12 @@ import org.w3c.dom.traversal.NodeIterator; /** *

- * {@link DefaultBeanDefinitionDocumentReader} extension to hook into the pre/post processing of the provided + * {@link DefaultBeanDefinitionDocumentReader} extension to hook into the pre processing of the provided * XML document, ensuring any references to property operators such as jobParameters and jobProperties are * resolved prior to loading the context. Since we know these initial values upfront, doing this transformation * allows us to ensure values are retrieved in their resolved form prior to loading the context and property - * operators can be used on any element. + * operators can be used on any element. This document reader will also look for references to artifacts by + * the same name and create new bean definitions to provide the ability to create new instances. *

* * @author Chris Schaefer @@ -214,17 +217,51 @@ public class JsrBeanDefinitionDocumentReader extends DefaultBeanDefinitionDocume DocumentTraversal traversal = (DocumentTraversal) root.getOwnerDocument(); NodeIterator iterator = traversal.createNodeIterator(root, NodeFilter.SHOW_ELEMENT, null, true); + BeanDefinitionRegistry registry = getBeanDefinitionRegistry(); + Map referenceCountMap = new HashMap(); + for (Node n = iterator.nextNode(); n != null; n = iterator.nextNode()) { NamedNodeMap map = n.getAttributes(); if (map.getLength() > 0) { for (int i = 0; i < map.getLength(); i++) { Node node = map.item(i); + + String nodeName = node.getNodeName(); String nodeValue = node.getNodeValue(); String resolvedValue = resolveValue(nodeValue); + String newNodeValue = resolvedValue; - if(!nodeValue.equals(resolvedValue)) { - node.setNodeValue(resolvedValue); + if("ref".equals(nodeName)) { + if(!referenceCountMap.containsKey(resolvedValue)) { + referenceCountMap.put(resolvedValue, 0); + } + + // possibly fully qualified class name in ref tag in the jobXML + if(!registry.containsBeanDefinition(resolvedValue)) { + AbstractBeanDefinition beanDefinition = BeanDefinitionBuilder.genericBeanDefinition(resolvedValue) + .getBeanDefinition(); + beanDefinition.setScope("step"); + registry.registerBeanDefinition(resolvedValue, beanDefinition); + newNodeValue = resolvedValue; + } + + if (referenceCountMap.containsKey(resolvedValue)) { + Integer referenceCount = referenceCountMap.get(resolvedValue); + referenceCount++; + referenceCountMap.put(resolvedValue, referenceCount); + + newNodeValue = resolvedValue + referenceCount; + + if(registry.containsBeanDefinition(resolvedValue)) { + BeanDefinition beanDefinition = registry.getBeanDefinition(resolvedValue); + registry.registerBeanDefinition(newNodeValue, beanDefinition); + } + } + } + + if(!nodeValue.equals(newNodeValue)) { + node.setNodeValue(newNodeValue); } } } else { diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/xml/ListenerParser.java b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/xml/ListenerParser.java index f2e10b089..c09561f2d 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/xml/ListenerParser.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/xml/ListenerParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2014 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. @@ -43,6 +43,7 @@ public class ListenerParser { private static final String LISTENER_ELEMENT = "listener"; private static final String LISTENERS_ELEMENT = "listeners"; private static final String SCOPE_STEP = "step"; + private static final String SCOPE_JOB = "job"; @SuppressWarnings("rawtypes") private Class listenerType; @@ -128,7 +129,7 @@ public class ListenerParser { private String getListenerScope() { if (listenerType == JobListenerFactoryBean.class) { - return BeanDefinition.SCOPE_SINGLETON; + return SCOPE_JOB; } return SCOPE_STEP; diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/launch/JsrJobOperator.java b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/launch/JsrJobOperator.java index 597857981..6932fe566 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/launch/JsrJobOperator.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/launch/JsrJobOperator.java @@ -540,8 +540,6 @@ public class JsrJobOperator implements JobOperator, InitializingBean { jobRepository.update(jobExecution); throw new JobRestartException(e); - } finally { - batchContext.close(); } return jobExecution.getId(); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/configuration/xml/JsrBeanDefinitionDocumentReaderTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/configuration/xml/JsrBeanDefinitionDocumentReaderTests.java index 239970779..66e6e5474 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/configuration/xml/JsrBeanDefinitionDocumentReaderTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/configuration/xml/JsrBeanDefinitionDocumentReaderTests.java @@ -6,15 +6,21 @@ import java.util.Properties; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.junit.Test; +import org.springframework.batch.core.jsr.JsrTestUtils; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.xml.DefaultDocumentLoader; import org.springframework.beans.factory.xml.DelegatingEntityResolver; import org.springframework.beans.factory.xml.DocumentLoader; import org.springframework.core.io.ClassPathResource; +import org.springframework.util.StringUtils; import org.springframework.util.xml.SimpleSaxErrorHandler; import org.w3c.dom.Document; import org.xml.sax.ErrorHandler; import org.xml.sax.InputSource; + +import javax.batch.api.Batchlet; +import javax.batch.runtime.JobExecution; + import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertNotNull; import static junit.framework.Assert.assertTrue; @@ -124,6 +130,107 @@ public class JsrBeanDefinitionDocumentReaderTests { assertEquals("myfile.txt", resolvedProperties.getProperty("jobProperty3")); } + @Test + public void testGenerationOfBeanDefinitionsForMultipleReferences() throws Exception { + JsrXmlApplicationContext applicationContext = new JsrXmlApplicationContext(new Properties()); + applicationContext.setValidating(false); + applicationContext.load(new ClassPathResource("baseContext.xml"), + new ClassPathResource("/META-INF/batch.xml"), + new ClassPathResource("/META-INF/batch-jobs/jsrUniqueInstanceTests.xml")); + applicationContext.refresh(); + + assertTrue("exitStatusSettingStepListener bean definition not found", applicationContext.containsBeanDefinition("exitStatusSettingStepListener")); + assertTrue("exitStatusSettingStepListener1 bean definition not found", applicationContext.containsBeanDefinition("exitStatusSettingStepListener1")); + assertTrue("exitStatusSettingStepListener2 bean definition not found", applicationContext.containsBeanDefinition("exitStatusSettingStepListener2")); + assertTrue("exitStatusSettingStepListener3 bean definition not found", applicationContext.containsBeanDefinition("exitStatusSettingStepListener3")); + assertTrue("exitStatusSettingStepListenerClassBeanDefinition bean definition not found", applicationContext.containsBeanDefinition("org.springframework.batch.core.jsr.step.listener.ExitStatusSettingStepListener")); + assertTrue("exitStatusSettingStepListener1ClassBeanDefinition bean definition not found", applicationContext.containsBeanDefinition("org.springframework.batch.core.jsr.step.listener.ExitStatusSettingStepListener1")); + assertTrue("exitStatusSettingStepListener2ClassBeanDefinition bean definition not found", applicationContext.containsBeanDefinition("org.springframework.batch.core.jsr.step.listener.ExitStatusSettingStepListener2")); + assertTrue("exitStatusSettingStepListener3ClassBeanDefinition bean definition not found", applicationContext.containsBeanDefinition("org.springframework.batch.core.jsr.step.listener.ExitStatusSettingStepListener3")); + assertTrue("testBatchlet bean definition not found", applicationContext.containsBeanDefinition("testBatchlet")); + assertTrue("testBatchlet1 bean definition not found", applicationContext.containsBeanDefinition("testBatchlet1")); + assertTrue("testBatchlet2 bean definition not found", applicationContext.containsBeanDefinition("testBatchlet2")); + } + + @Test + public void testArtifactUniqueness() throws Exception { + JobExecution jobExecution = JsrTestUtils.runJob("jsrUniqueInstanceTests", new Properties(), 10000L); + String exitStatus = jobExecution.getExitStatus(); + + assertTrue("Exit status must contain listener3", exitStatus.contains("listener3")); + exitStatus = exitStatus.replace("listener3", ""); + + assertTrue("Exit status must contain listener2", exitStatus.contains("listener2")); + exitStatus = exitStatus.replace("listener2", ""); + + assertTrue("Exit status must contain listener1", exitStatus.contains("listener1")); + exitStatus = exitStatus.replace("listener1", ""); + + assertTrue("Exit status must contain listener0", exitStatus.contains("listener0")); + exitStatus = exitStatus.replace("listener0", ""); + + assertTrue("Exit status must contain listener7", exitStatus.contains("listener7")); + exitStatus = exitStatus.replace("listener7", ""); + + assertTrue("Exit status must contain listener6", exitStatus.contains("listener6")); + exitStatus = exitStatus.replace("listener6", ""); + + assertTrue("Exit status must contain listener5", exitStatus.contains("listener5")); + exitStatus = exitStatus.replace("listener5", ""); + + assertTrue("Exit status must contain listener4", exitStatus.contains("listener4")); + exitStatus = exitStatus.replace("listener4", ""); + + assertTrue("exitStatus must be empty", "".equals(exitStatus)); + } + + @Test + public void testGenerationOfSpringBeanDefinitionsForMultipleReferences() { + JsrXmlApplicationContext applicationContext = new JsrXmlApplicationContext(new Properties()); + applicationContext.setValidating(false); + applicationContext.load(new ClassPathResource("baseContext.xml"), + new ClassPathResource("/META-INF/batch-jobs/jsrSpringInstanceTests.xml")); + + applicationContext.refresh(); + + assertTrue("exitStatusSettingStepListener bean definition not found", applicationContext.containsBeanDefinition("exitStatusSettingStepListener")); + assertTrue("scopedTarget.exitStatusSettingStepListener bean definition not found", applicationContext.containsBeanDefinition("scopedTarget.exitStatusSettingStepListener")); + + BeanDefinition exitStatusSettingStepListenerBeanDefinition = applicationContext.getBeanDefinition("scopedTarget.exitStatusSettingStepListener"); + assertTrue("step".equals(exitStatusSettingStepListenerBeanDefinition.getScope())); + + assertTrue("Should not contain bean definition for exitStatusSettingStepListener1", !applicationContext.containsBeanDefinition("exitStatusSettingStepListener1")); + assertTrue("Should not contain bean definition for exitStatusSettingStepListener2", !applicationContext.containsBeanDefinition("exitStatusSettingStepListener2")); + assertTrue("Should not contain bean definition for exitStatusSettingStepListener3", !applicationContext.containsBeanDefinition("exitStatusSettingStepListener3")); + + assertTrue("Should not contain bean definition for testBatchlet1", !applicationContext.containsBeanDefinition("testBatchlet1")); + assertTrue("Should not contain bean definition for testBatchlet2", !applicationContext.containsBeanDefinition("testBatchlet2")); + + assertTrue("testBatchlet bean definition not found", applicationContext.containsBeanDefinition("testBatchlet")); + + BeanDefinition testBatchletBeanDefinition = applicationContext.getBeanDefinition("testBatchlet"); + assertTrue("singleton".equals(testBatchletBeanDefinition.getScope())); + } + + @Test + public void testSpringArtifactUniqueness() throws Exception { + JobExecution jobExecution = JsrTestUtils.runJob("jsrSpringInstanceTests", new Properties(), 10000L); + String exitStatus = jobExecution.getExitStatus(); + + assertEquals("listener1listener1listener4listener4", exitStatus); + + assertTrue("Exit status must contain listener1", exitStatus.contains("listener1")); + assertTrue("exitStatus must contain 2 listener1 values", StringUtils.countOccurrencesOf(exitStatus, "listener1") == 2); + + exitStatus = exitStatus.replace("listener1", ""); + + assertTrue("Exit status must contain listener4", exitStatus.contains("listener4")); + assertTrue("exitStatus must contain 2 listener4 values", StringUtils.countOccurrencesOf(exitStatus, "listener4") == 2); + exitStatus = exitStatus.replace("listener4", ""); + + assertTrue("exitStatus must be empty", "".equals(exitStatus)); + } + private Document getDocument(String location) { InputStream inputStream = ClassLoader.class.getResourceAsStream(location); @@ -138,4 +245,16 @@ public class JsrBeanDefinitionDocumentReaderTests { } catch (IOException e) { } } } + + public static class TestBatchlet implements Batchlet { + @Override + public String process() throws Exception { + return null; + } + + @Override + public void stop() throws Exception { + + } + } } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/configuration/xml/ListenerParserTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/configuration/xml/ListenerParserTests.java index 56026c4d0..fde9dcb48 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/configuration/xml/ListenerParserTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/configuration/xml/ListenerParserTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2014 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. @@ -60,6 +60,6 @@ public class ListenerParserTests { listenerParser.applyListenerScope("jobListener", applicationContext); BeanDefinition beanDefinition = applicationContext.getBeanDefinition("jobListener"); - assertEquals("singleton", beanDefinition.getScope()); + assertEquals("job", beanDefinition.getScope()); } } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/step/listener/ExitStatusSettingStepListener.java b/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/step/listener/ExitStatusSettingStepListener.java new file mode 100644 index 000000000..63d341b14 --- /dev/null +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/step/listener/ExitStatusSettingStepListener.java @@ -0,0 +1,40 @@ +package org.springframework.batch.core.jsr.step.listener; + +import javax.batch.api.BatchProperty; +import javax.batch.api.listener.StepListener; +import javax.batch.runtime.context.JobContext; +import javax.inject.Inject; + +/** + *

+ * {@link StepListener} for testing. Sets or appends the value of the + * testProperty field to the {@link JobContext} exit status on afterStep. + *

+ * + * @author Chris Schaefer + * @since 3.0 + */ +public class ExitStatusSettingStepListener implements StepListener { + @Inject + @BatchProperty + private String testProperty; + + @Inject + private JobContext jobContext; + + @Override + public void beforeStep() throws Exception { + + } + + @Override + public void afterStep() throws Exception { + String exitStatus = jobContext.getExitStatus(); + + if("".equals(exitStatus) || exitStatus == null) { + jobContext.setExitStatus(testProperty); + } else { + jobContext.setExitStatus(exitStatus + testProperty); + } + } +} diff --git a/spring-batch-core/src/test/resources/META-INF/batch-jobs/jsrSpringInstanceTests.xml b/spring-batch-core/src/test/resources/META-INF/batch-jobs/jsrSpringInstanceTests.xml new file mode 100644 index 000000000..8535e3a9c --- /dev/null +++ b/spring-batch-core/src/test/resources/META-INF/batch-jobs/jsrSpringInstanceTests.xml @@ -0,0 +1,45 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/spring-batch-core/src/test/resources/META-INF/batch-jobs/jsrUniqueInstanceTests.xml b/spring-batch-core/src/test/resources/META-INF/batch-jobs/jsrUniqueInstanceTests.xml new file mode 100644 index 000000000..aab71ad34 --- /dev/null +++ b/spring-batch-core/src/test/resources/META-INF/batch-jobs/jsrUniqueInstanceTests.xml @@ -0,0 +1,53 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/spring-batch-core/src/test/resources/META-INF/batch.xml b/spring-batch-core/src/test/resources/META-INF/batch.xml index 14ac315ab..347468b4c 100644 --- a/spring-batch-core/src/test/resources/META-INF/batch.xml +++ b/spring-batch-core/src/test/resources/META-INF/batch.xml @@ -2,4 +2,5 @@ +