From d3969de101e018515b715f32e1e189a8fabda97c Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 7 Feb 2013 14:11:24 +0100 Subject: [PATCH 01/11] Allow for ordering of mixed AspectJ before/after advices Issue: SPR-9438 --- .../AspectJPrecedenceComparator.java | 21 +++++-------------- .../AspectJPrecedenceComparatorTests.java | 11 +++++----- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparator.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparator.java index 0dd176354eb..31fdb83f7d8 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparator.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -53,7 +53,6 @@ class AspectJPrecedenceComparator implements Comparator { private static final int HIGHER_PRECEDENCE = -1; private static final int SAME_PRECEDENCE = 0; private static final int LOWER_PRECEDENCE = 1; - private static final int NOT_COMPARABLE = 0; private final Comparator advisorComparator; @@ -85,21 +84,11 @@ class AspectJPrecedenceComparator implements Comparator { Advisor advisor1 = (Advisor) o1; Advisor advisor2 = (Advisor) o2; - - boolean oneOrOtherIsAfterAdvice = - (AspectJAopUtils.isAfterAdvice(advisor1) || AspectJAopUtils.isAfterAdvice(advisor2)); - boolean oneOrOtherIsBeforeAdvice = - (AspectJAopUtils.isBeforeAdvice(advisor1) || AspectJAopUtils.isBeforeAdvice(advisor2)); - if (oneOrOtherIsAfterAdvice && oneOrOtherIsBeforeAdvice) { - return NOT_COMPARABLE; - } - else { - int advisorPrecedence = this.advisorComparator.compare(advisor1, advisor2); - if (advisorPrecedence == SAME_PRECEDENCE && declaredInSameAspect(advisor1, advisor2)) { - advisorPrecedence = comparePrecedenceWithinAspect(advisor1, advisor2); - } - return advisorPrecedence; + int advisorPrecedence = this.advisorComparator.compare(advisor1, advisor2); + if (advisorPrecedence == SAME_PRECEDENCE && declaredInSameAspect(advisor1, advisor2)) { + advisorPrecedence = comparePrecedenceWithinAspect(advisor1, advisor2); } + return advisorPrecedence; } private int comparePrecedenceWithinAspect(Advisor advisor1, Advisor advisor2) { diff --git a/spring-aop/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparatorTests.java b/spring-aop/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparatorTests.java index 08d95808edc..96313821a35 100644 --- a/spring-aop/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparatorTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparatorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -16,12 +16,11 @@ package org.springframework.aop.aspectj.autoproxy; -import static org.junit.Assert.*; - import java.lang.reflect.Method; import org.junit.Before; import org.junit.Test; + import org.springframework.aop.Advisor; import org.springframework.aop.AfterReturningAdvice; import org.springframework.aop.BeforeAdvice; @@ -35,11 +34,13 @@ import org.springframework.aop.aspectj.AspectJMethodBeforeAdvice; import org.springframework.aop.aspectj.AspectJPointcutAdvisor; import org.springframework.aop.support.DefaultPointcutAdvisor; +import static org.junit.Assert.*; + /** * @author Adrian Colyer * @author Chris Beams */ -public final class AspectJPrecedenceComparatorTests { +public class AspectJPrecedenceComparatorTests { private static final int HIGH_PRECEDENCE_ADVISOR_ORDER = 100; private static final int LOW_PRECEDENCE_ADVISOR_ORDER = 200; @@ -89,7 +90,7 @@ public final class AspectJPrecedenceComparatorTests { public void testSameAspectOneOfEach() { Advisor advisor1 = createAspectJAfterAdvice(HIGH_PRECEDENCE_ADVISOR_ORDER, EARLY_ADVICE_DECLARATION_ORDER, "someAspect"); Advisor advisor2 = createAspectJBeforeAdvice(HIGH_PRECEDENCE_ADVISOR_ORDER, LATE_ADVICE_DECLARATION_ORDER, "someAspect"); - assertEquals("advisor1 and advisor2 not comparable", 0, this.comparator.compare(advisor1, advisor2)); + assertEquals("advisor1 and advisor2 not comparable", 1, this.comparator.compare(advisor1, advisor2)); } @Test From 0d66df26da0ec220b67c0ac90f57b58b28fd77b8 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 7 Feb 2013 15:27:43 +0100 Subject: [PATCH 02/11] "depends-on" attribute on lang namespace element actually respected at runtime now Issue: SPR-8625 --- .../config/ScriptBeanDefinitionParser.java | 12 ++++++++- .../groovy/GroovyScriptFactoryTests.java | 27 +++++++++---------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/scripting/config/ScriptBeanDefinitionParser.java b/spring-context/src/main/java/org/springframework/scripting/config/ScriptBeanDefinitionParser.java index 23cfe64423f..f12e4004bf7 100644 --- a/spring-context/src/main/java/org/springframework/scripting/config/ScriptBeanDefinitionParser.java +++ b/spring-context/src/main/java/org/springframework/scripting/config/ScriptBeanDefinitionParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -26,6 +26,7 @@ import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionDefaults; import org.springframework.beans.factory.support.GenericBeanDefinition; import org.springframework.beans.factory.xml.AbstractBeanDefinitionParser; +import org.springframework.beans.factory.xml.BeanDefinitionParserDelegate; import org.springframework.beans.factory.xml.ParserContext; import org.springframework.beans.factory.xml.XmlReaderContext; import org.springframework.scripting.support.ScriptFactoryPostProcessor; @@ -64,6 +65,8 @@ class ScriptBeanDefinitionParser extends AbstractBeanDefinitionParser { private static final String DEPENDENCY_CHECK_ATTRIBUTE = "dependency-check"; + private static final String DEPENDS_ON_ATTRIBUTE = "depends-on"; + private static final String INIT_METHOD_ATTRIBUTE = "init-method"; private static final String DESTROY_METHOD_ATTRIBUTE = "destroy-method"; @@ -138,6 +141,13 @@ class ScriptBeanDefinitionParser extends AbstractBeanDefinitionParser { String dependencyCheck = element.getAttribute(DEPENDENCY_CHECK_ATTRIBUTE); bd.setDependencyCheck(parserContext.getDelegate().getDependencyCheck(dependencyCheck)); + // Parse depends-on list of bean names. + String dependsOn = element.getAttribute(DEPENDS_ON_ATTRIBUTE); + if (StringUtils.hasLength(dependsOn)) { + bd.setDependsOn(StringUtils.tokenizeToStringArray( + dependsOn, BeanDefinitionParserDelegate.MULTI_VALUE_ATTRIBUTE_DELIMITERS)); + } + // Retrieve the defaults for bean definitions within this parser context BeanDefinitionDefaults beanDefinitionDefaults = parserContext.getDelegate().getBeanDefinitionDefaults(); diff --git a/spring-context/src/test/java/org/springframework/scripting/groovy/GroovyScriptFactoryTests.java b/spring-context/src/test/java/org/springframework/scripting/groovy/GroovyScriptFactoryTests.java index 96d3db7fdf2..cca1625d3df 100644 --- a/spring-context/src/test/java/org/springframework/scripting/groovy/GroovyScriptFactoryTests.java +++ b/spring-context/src/test/java/org/springframework/scripting/groovy/GroovyScriptFactoryTests.java @@ -16,31 +16,22 @@ package org.springframework.scripting.groovy; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; -import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.mock; -import groovy.lang.DelegatingMetaClass; -import groovy.lang.GroovyObject; - import java.io.FileNotFoundException; import java.util.Arrays; import java.util.Map; +import groovy.lang.DelegatingMetaClass; +import groovy.lang.GroovyObject; import org.junit.Before; import org.junit.Ignore; import org.junit.Test; + import org.springframework.aop.support.AopUtils; import org.springframework.aop.target.dynamic.Refreshable; -import org.springframework.tests.sample.beans.TestBean; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.FactoryBean; import org.springframework.beans.factory.UnsatisfiedDependencyException; +import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.context.ApplicationContext; import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.core.NestedRuntimeException; @@ -56,6 +47,12 @@ import org.springframework.scripting.support.ScriptFactoryPostProcessor; import org.springframework.stereotype.Component; import org.springframework.tests.Assume; import org.springframework.tests.TestGroup; +import org.springframework.tests.sample.beans.TestBean; +import org.springframework.util.ObjectUtils; + +import static org.junit.Assert.*; +import static org.mockito.BDDMockito.*; +import static org.mockito.Mockito.mock; /** * @author Rob Harrop @@ -350,7 +347,9 @@ public class GroovyScriptFactoryTests { @Test public void testInlineScriptFromTag() throws Exception { - ApplicationContext ctx = new ClassPathXmlApplicationContext("groovy-with-xsd.xml", getClass()); + ClassPathXmlApplicationContext ctx = new ClassPathXmlApplicationContext("groovy-with-xsd.xml", getClass()); + BeanDefinition bd = ctx.getBeanFactory().getBeanDefinition("calculator"); + assertTrue(ObjectUtils.containsElement(bd.getDependsOn(), "messenger")); Calculator calculator = (Calculator) ctx.getBean("calculator"); assertNotNull(calculator); assertFalse(calculator instanceof Refreshable); From 9ffbee332c242cb52310dad6b8653910e415bb16 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 7 Feb 2013 15:28:25 +0100 Subject: [PATCH 03/11] Fixed documentation for "depends-on" attribute --- .../scripting/config/spring-lang-2.5.xsd | 14 +++++++------- .../scripting/config/spring-lang-3.0.xsd | 14 +++++++------- .../scripting/config/spring-lang-3.1.xsd | 12 ++++++------ .../scripting/config/spring-lang-3.2.xsd | 12 ++++++------ 4 files changed, 26 insertions(+), 26 deletions(-) diff --git a/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-2.5.xsd b/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-2.5.xsd index ac7c3174183..e6288503896 100644 --- a/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-2.5.xsd +++ b/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-2.5.xsd @@ -82,7 +82,7 @@ @@ -137,13 +137,13 @@ diff --git a/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-3.0.xsd b/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-3.0.xsd index 574fb05b432..5a34213de74 100644 --- a/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-3.0.xsd +++ b/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-3.0.xsd @@ -82,7 +82,7 @@ @@ -127,13 +127,13 @@ diff --git a/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-3.1.xsd b/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-3.1.xsd index edce1324f4d..54ae90e9e12 100644 --- a/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-3.1.xsd +++ b/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-3.1.xsd @@ -147,13 +147,13 @@ diff --git a/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-3.2.xsd b/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-3.2.xsd index 34e9775fe24..5bd66ded436 100644 --- a/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-3.2.xsd +++ b/spring-context/src/main/resources/org/springframework/scripting/config/spring-lang-3.2.xsd @@ -147,13 +147,13 @@ From e3c83bb7699f50641bdf9b673c9dc64b176ae399 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 7 Feb 2013 15:36:19 +0100 Subject: [PATCH 04/11] Minor javadoc and source layout polishing --- .../xml/BeanDefinitionDocumentReader.java | 27 ++++++++++--------- .../DefaultBeanDefinitionDocumentReader.java | 23 ++++++++-------- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/xml/BeanDefinitionDocumentReader.java b/spring-beans/src/main/java/org/springframework/beans/factory/xml/BeanDefinitionDocumentReader.java index 37ef1b40904..ffe9d294d9b 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/xml/BeanDefinitionDocumentReader.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/xml/BeanDefinitionDocumentReader.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -16,11 +16,11 @@ package org.springframework.beans.factory.xml; +import org.w3c.dom.Document; + import org.springframework.beans.factory.BeanDefinitionStoreException; import org.springframework.core.env.Environment; -import org.w3c.dom.Document; - /** * SPI for parsing an XML document that contains Spring bean definitions. * Used by XmlBeanDefinitionReader for actually parsing a DOM document. @@ -38,20 +38,21 @@ import org.w3c.dom.Document; public interface BeanDefinitionDocumentReader { /** - * Read bean definitions from the given DOM document, - * and register them with the given bean factory. + * Set the Environment to use when reading bean definitions. + *

Used for evaluating profile information to determine whether a + * {@code } document/element should be included or ignored. + */ + void setEnvironment(Environment environment); + + /** + * Read bean definitions from the given DOM document and + * register them with the registry in the given reader context. * @param doc the DOM document - * @param readerContext the current context of the reader. Includes the resource being parsed + * @param readerContext the current context of the reader + * (includes the target registry and the resource being parsed) * @throws BeanDefinitionStoreException in case of parsing errors */ void registerBeanDefinitions(Document doc, XmlReaderContext readerContext) throws BeanDefinitionStoreException; - /** - * Set the Environment to use when reading bean definitions. Used for evaluating - * profile information to determine whether a {@code } document/element should - * be included or omitted. - */ - void setEnvironment(Environment environment); - } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/xml/DefaultBeanDefinitionDocumentReader.java b/spring-beans/src/main/java/org/springframework/beans/factory/xml/DefaultBeanDefinitionDocumentReader.java index 9900aa16517..a422d5b4fcd 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/xml/DefaultBeanDefinitionDocumentReader.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/xml/DefaultBeanDefinitionDocumentReader.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -72,16 +72,15 @@ public class DefaultBeanDefinitionDocumentReader implements BeanDefinitionDocume public static final String RESOURCE_ATTRIBUTE = "resource"; - /** @see org.springframework.context.annotation.Profile */ public static final String PROFILE_ATTRIBUTE = "profile"; protected final Log logger = LogFactory.getLog(getClass()); - private XmlReaderContext readerContext; - private Environment environment; + private XmlReaderContext readerContext; + private BeanDefinitionParserDelegate delegate; @@ -104,13 +103,12 @@ public class DefaultBeanDefinitionDocumentReader implements BeanDefinitionDocume */ public void registerBeanDefinitions(Document doc, XmlReaderContext readerContext) { this.readerContext = readerContext; - logger.debug("Loading bean definitions"); Element root = doc.getDocumentElement(); - doRegisterBeanDefinitions(root); } + /** * Register each bean definition within the given root {@code } element. * @throws IllegalStateException if {@code elements will cause recursion in this method. In + // Any nested elements will cause recursion in this method. In // order to propagate and preserve default-* attributes correctly, // keep track of the current (parent) delegate, which may be null. Create // the new (child) delegate with a reference to the parent for fallback purposes, // then ultimately reset this.delegate back to its original (parent) reference. // this behavior emulates a stack of delegates without actually necessitating one. BeanDefinitionParserDelegate parent = this.delegate; - this.delegate = createHelper(readerContext, root, parent); + this.delegate = createHelper(this.readerContext, root, parent); preProcessXml(root); parseBeanDefinitions(root, this.delegate); @@ -143,7 +142,9 @@ public class DefaultBeanDefinitionDocumentReader implements BeanDefinitionDocume this.delegate = parent; } - protected BeanDefinitionParserDelegate createHelper(XmlReaderContext readerContext, Element root, BeanDefinitionParserDelegate parentDelegate) { + protected BeanDefinitionParserDelegate createHelper( + XmlReaderContext readerContext, Element root, BeanDefinitionParserDelegate parentDelegate) { + BeanDefinitionParserDelegate delegate = new BeanDefinitionParserDelegate(readerContext, environment); delegate.initDefaults(root, parentDelegate); return delegate; From 2aaa66f86b10e8146bae9518e4139dd33913b1f7 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 7 Feb 2013 22:02:40 +0100 Subject: [PATCH 05/11] Allow for ordering of mixed AspectJ before/after advices Issue: SPR-9438 --- .../AspectJAwareAdvisorAutoProxyCreator.java | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJAwareAdvisorAutoProxyCreator.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJAwareAdvisorAutoProxyCreator.java index dbfcbd380e3..271d961088b 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJAwareAdvisorAutoProxyCreator.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJAwareAdvisorAutoProxyCreator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -16,8 +16,8 @@ package org.springframework.aop.aspectj.autoproxy; +import java.util.ArrayList; import java.util.Comparator; -import java.util.LinkedList; import java.util.List; import org.aopalliance.aop.Advice; @@ -67,29 +67,24 @@ public class AspectJAwareAdvisorAutoProxyCreator extends AbstractAdvisorAutoProx @Override @SuppressWarnings("unchecked") protected List sortAdvisors(List advisors) { - // build list for sorting List partiallyComparableAdvisors = - new LinkedList(); + new ArrayList(advisors.size()); for (Advisor element : advisors) { partiallyComparableAdvisors.add( new PartiallyComparableAdvisorHolder(element, DEFAULT_PRECEDENCE_COMPARATOR)); } - - // sort it List sorted = PartialOrder.sort(partiallyComparableAdvisors); - if (sorted == null) { - // TODO: work harder to give a better error message here. - throw new IllegalArgumentException("Advice precedence circularity error"); + if (sorted != null) { + List result = new ArrayList(advisors.size()); + for (PartiallyComparableAdvisorHolder pcAdvisor : sorted) { + result.add(pcAdvisor.getAdvisor()); + } + return result; } - - // extract results again - List result = new LinkedList(); - for (PartiallyComparableAdvisorHolder pcAdvisor : sorted) { - result.add(pcAdvisor.getAdvisor()); + else { + return super.sortAdvisors(advisors); } - - return result; } /** From 7bbb4ec7aff9ca171f2d34f747d7e0d96a6ce1b0 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 7 Feb 2013 13:35:31 -0800 Subject: [PATCH 06/11] Fix Assert.instanceOf exception message Update the exception message used when Assert.instanceOf fails such that it expects the provided message to end with '.'. This reverts commit 5874383ef081bb52a872dd49d63e5b542fbf20ca which caused the implementation to be at odds with the JavaDoc and the previous release. The updated code also has the benefit of protecting against a null message. Issue: SPR-10269 --- .../java/org/springframework/util/Assert.java | 7 +++--- .../org/springframework/util/AssertTests.java | 23 ++++++++++++++++++- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/Assert.java b/spring-core/src/main/java/org/springframework/util/Assert.java index 05c1c4babe2..193d461a054 100644 --- a/spring-core/src/main/java/org/springframework/util/Assert.java +++ b/spring-core/src/main/java/org/springframework/util/Assert.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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 @@ -334,8 +334,9 @@ public abstract class Assert { public static void isInstanceOf(Class type, Object obj, String message) { notNull(type, "Type to check against must not be null"); if (!type.isInstance(obj)) { - throw new IllegalArgumentException(message + - ". Object of class [" + (obj != null ? obj.getClass().getName() : "null") + + throw new IllegalArgumentException( + (StringUtils.hasLength(message) ? message + " " : "") + + "Object of class [" + (obj != null ? obj.getClass().getName() : "null") + "] must be an instance of " + type); } } diff --git a/spring-core/src/test/java/org/springframework/util/AssertTests.java b/spring-core/src/test/java/org/springframework/util/AssertTests.java index 744b492f617..0dfe86f1dce 100644 --- a/spring-core/src/test/java/org/springframework/util/AssertTests.java +++ b/spring-core/src/test/java/org/springframework/util/AssertTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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,7 +24,9 @@ import java.util.List; import java.util.Map; import java.util.Set; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; /** * Unit tests for the {@link Assert} class. @@ -36,6 +38,9 @@ import org.junit.Test; */ public class AssertTests { + @Rule + public ExpectedException thrown = ExpectedException.none(); + @Test(expected = IllegalArgumentException.class) public void instanceOf() { final Set set = new HashSet(); @@ -43,6 +48,22 @@ public class AssertTests { Assert.isInstanceOf(HashMap.class, set); } + @Test + public void instanceOfNoMessage() throws Exception { + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("Object of class [java.lang.Object] must be an instance " + + "of interface java.util.Set"); + Assert.isInstanceOf(Set.class, new Object(), null); + } + + @Test + public void instanceOfMessage() throws Exception { + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("Custom message. Object of class [java.lang.Object] must " + + "be an instance of interface java.util.Set"); + Assert.isInstanceOf(Set.class, new Object(), "Custom message."); + } + @Test public void isNullDoesNotThrowExceptionIfArgumentIsNullWithMessage() { Assert.isNull(null, "Bla"); From b3c9a11bd1141f644859eb0023f4d6f4ae0c9910 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 7 Feb 2013 23:22:30 +0100 Subject: [PATCH 07/11] Folded a FactoryBean-specific check into predictBeanType now This change means that we effectively revert SPR-8954's code change in favor of the isFactoryBean implementation simply relying on predictBeanType to sort it out, filtering a post-processed predictedType for FactoryBean applicability. Issue: SPR-9177 Issue: SPR-9143 --- .../AbstractAutowireCapableBeanFactory.java | 9 +- .../factory/support/AbstractBeanFactory.java | 14 +- .../beans/factory/support/Spr8954Tests.java | 31 +++- .../ConfigurationClassSpr8954Tests.java | 132 ++++++++++++++++++ 4 files changed, 173 insertions(+), 13 deletions(-) create mode 100644 spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassSpr8954Tests.java diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java index c112676ea1d..9d11eeb520c 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -586,9 +586,10 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac for (BeanPostProcessor bp : getBeanPostProcessors()) { if (bp instanceof SmartInstantiationAwareBeanPostProcessor) { SmartInstantiationAwareBeanPostProcessor ibp = (SmartInstantiationAwareBeanPostProcessor) bp; - Class processedType = ibp.predictBeanType(beanClass, beanName); - if (processedType != null) { - return processedType; + Class predictedType = ibp.predictBeanType(beanClass, beanName); + if (predictedType != null && (typesToMatch.length > 1 || + !FactoryBean.class.equals(typesToMatch[0]) || FactoryBean.class.isAssignableFrom(predictedType))) { + return predictedType; } } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java index 6ec60e6e61b..9e97ba2fafa 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -497,18 +497,21 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp // Retrieve corresponding bean definition. RootBeanDefinition mbd = getMergedLocalBeanDefinition(beanName); + Class[] typesToMatch = (FactoryBean.class.equals(typeToMatch) ? + new Class[] {typeToMatch} : new Class[] {FactoryBean.class, typeToMatch}); + // Check decorated bean definition, if any: We assume it'll be easier // to determine the decorated bean's type than the proxy's type. BeanDefinitionHolder dbd = mbd.getDecoratedDefinition(); if (dbd != null && !BeanFactoryUtils.isFactoryDereference(name)) { RootBeanDefinition tbd = getMergedBeanDefinition(dbd.getBeanName(), dbd.getBeanDefinition(), mbd); - Class targetClass = predictBeanType(dbd.getBeanName(), tbd, FactoryBean.class, typeToMatch); + Class targetClass = predictBeanType(dbd.getBeanName(), tbd, typesToMatch); if (targetClass != null && !FactoryBean.class.isAssignableFrom(targetClass)) { return typeToMatch.isAssignableFrom(targetClass); } } - Class beanClass = predictBeanType(beanName, mbd, FactoryBean.class, typeToMatch); + Class beanClass = predictBeanType(beanName, mbd, typesToMatch); if (beanClass == null) { return false; } @@ -1332,9 +1335,8 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp * @param mbd the corresponding bean definition */ protected boolean isFactoryBean(String beanName, RootBeanDefinition mbd) { - Class predictedType = predictBeanType(beanName, mbd, FactoryBean.class); - return (predictedType != null && FactoryBean.class.isAssignableFrom(predictedType)) || - (mbd.hasBeanClass() && FactoryBean.class.isAssignableFrom(mbd.getBeanClass())); + Class beanClass = predictBeanType(beanName, mbd, FactoryBean.class); + return (beanClass != null && FactoryBean.class.isAssignableFrom(beanClass)); } /** diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/support/Spr8954Tests.java b/spring-beans/src/test/java/org/springframework/beans/factory/support/Spr8954Tests.java index 5c1368c4610..411f5380451 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/support/Spr8954Tests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/support/Spr8954Tests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -22,6 +22,8 @@ import static org.junit.Assert.*; import java.util.Map; import org.junit.Test; + +import org.springframework.beans.BeansException; import org.springframework.beans.factory.FactoryBean; import org.springframework.beans.factory.config.InstantiationAwareBeanPostProcessorAdapter; import org.springframework.beans.factory.support.DefaultListableBeanFactory; @@ -49,6 +51,8 @@ public class Spr8954Tests { assertThat(bf.getBean("foo"), instanceOf(Foo.class)); assertThat(bf.getBean("&foo"), instanceOf(FooFactoryBean.class)); + assertThat(bf.isTypeMatch("&foo", FactoryBean.class), is(true)); + @SuppressWarnings("rawtypes") Map fbBeans = bf.getBeansOfType(FactoryBean.class); assertThat(1, equalTo(fbBeans.size())); @@ -59,6 +63,25 @@ public class Spr8954Tests { assertThat("&foo", equalTo(aiBeans.keySet().iterator().next())); } + @Test + public void findsBeansByTypeIfNotInstantiated() { + DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); + bf.registerBeanDefinition("foo", new RootBeanDefinition(FooFactoryBean.class)); + bf.addBeanPostProcessor(new PredictingBPP()); + + assertThat(bf.isTypeMatch("&foo", FactoryBean.class), is(true)); + + @SuppressWarnings("rawtypes") + Map fbBeans = bf.getBeansOfType(FactoryBean.class); + assertThat(1, equalTo(fbBeans.size())); + assertThat("&foo", equalTo(fbBeans.keySet().iterator().next())); + + Map aiBeans = bf.getBeansOfType(AnInterface.class); + assertThat(1, equalTo(aiBeans.size())); + assertThat("&foo", equalTo(aiBeans.keySet().iterator().next())); + } + + static class FooFactoryBean implements FactoryBean, AnInterface { @Override @@ -84,7 +107,9 @@ public class Spr8954Tests { } interface PredictedType { + } + static class PredictedTypeImpl implements PredictedType { } static class PredictingBPP extends InstantiationAwareBeanPostProcessorAdapter { @@ -92,8 +117,8 @@ public class Spr8954Tests { @Override public Class predictBeanType(Class beanClass, String beanName) { return FactoryBean.class.isAssignableFrom(beanClass) ? - PredictedType.class : - super.predictBeanType(beanClass, beanName); + PredictedType.class : null; } } + } diff --git a/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassSpr8954Tests.java b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassSpr8954Tests.java new file mode 100644 index 00000000000..1a7a5b1b33e --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassSpr8954Tests.java @@ -0,0 +1,132 @@ +/* + * Copyright 2002-2013 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.context.annotation; + +import java.util.Map; + +import org.junit.Test; + +import org.springframework.beans.factory.FactoryBean; +import org.springframework.beans.factory.config.InstantiationAwareBeanPostProcessorAdapter; +import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.beans.factory.support.RootBeanDefinition; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + +/** + * Unit tests for SPR-8954, in which a custom {@link InstantiationAwareBeanPostProcessor} + * forces the predicted type of a FactoryBean, effectively preventing retrieval of the + * bean from calls to #getBeansOfType(FactoryBean.class). The implementation of + * {@link AbstractBeanFactory#isFactoryBean(String, RootBeanDefinition)} now ensures + * that not only the predicted bean type is considered, but also the original bean + * definition's beanClass. + * + * @author Chris Beams + * @author Oliver Gierke + */ +public class ConfigurationClassSpr8954Tests { + + @Test + public void repro() { + AnnotationConfigApplicationContext bf = new AnnotationConfigApplicationContext(); + bf.registerBeanDefinition("fooConfig", new RootBeanDefinition(FooConfig.class)); + bf.getBeanFactory().addBeanPostProcessor(new PredictingBPP()); + bf.refresh(); + + assertThat(bf.getBean("foo"), instanceOf(Foo.class)); + assertThat(bf.getBean("&foo"), instanceOf(FooFactoryBean.class)); + + assertThat(bf.isTypeMatch("&foo", FactoryBean.class), is(true)); + + @SuppressWarnings("rawtypes") + Map fbBeans = bf.getBeansOfType(FactoryBean.class); + assertThat(1, equalTo(fbBeans.size())); + assertThat("&foo", equalTo(fbBeans.keySet().iterator().next())); + + Map aiBeans = bf.getBeansOfType(AnInterface.class); + assertThat(1, equalTo(aiBeans.size())); + assertThat("&foo", equalTo(aiBeans.keySet().iterator().next())); + } + + @Test + public void findsBeansByTypeIfNotInstantiated() { + AnnotationConfigApplicationContext bf = new AnnotationConfigApplicationContext(); + bf.registerBeanDefinition("fooConfig", new RootBeanDefinition(FooConfig.class)); + bf.getBeanFactory().addBeanPostProcessor(new PredictingBPP()); + bf.refresh(); + + assertThat(bf.isTypeMatch("&foo", FactoryBean.class), is(true)); + + @SuppressWarnings("rawtypes") + Map fbBeans = bf.getBeansOfType(FactoryBean.class); + assertThat(1, equalTo(fbBeans.size())); + assertThat("&foo", equalTo(fbBeans.keySet().iterator().next())); + + Map aiBeans = bf.getBeansOfType(AnInterface.class); + assertThat(1, equalTo(aiBeans.size())); + assertThat("&foo", equalTo(aiBeans.keySet().iterator().next())); + } + + + static class FooConfig { + + @Bean FooFactoryBean foo() { + return new FooFactoryBean(); + } + } + + static class FooFactoryBean implements FactoryBean, AnInterface { + + @Override + public Foo getObject() throws Exception { + return new Foo(); + } + + @Override + public Class getObjectType() { + return Foo.class; + } + + @Override + public boolean isSingleton() { + return true; + } + } + + interface AnInterface { + } + + static class Foo { + } + + interface PredictedType { + } + + static class PredictedTypeImpl implements PredictedType { + } + + static class PredictingBPP extends InstantiationAwareBeanPostProcessorAdapter { + + @Override + public Class predictBeanType(Class beanClass, String beanName) { + return FactoryBean.class.isAssignableFrom(beanClass) ? + PredictedType.class : null; + } + } + +} From 9255d3038ff4bfbc7530eec74e551613ebd03725 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 8 Feb 2013 00:58:39 +0100 Subject: [PATCH 08/11] @Scheduled provides String variants of fixedDelay, fixedRate, initialDelay for placeholder support Issue: SPR-8067 --- .../scheduling/annotation/Scheduled.java | 27 +++- .../ScheduledAnnotationBeanPostProcessor.java | 152 ++++++++++++------ .../scheduling/config/IntervalTask.java | 9 +- .../support/CronSequenceGenerator.java | 43 +++-- ...duledAnnotationBeanPostProcessorTests.java | 114 ++++++++++--- 5 files changed, 255 insertions(+), 90 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/scheduling/annotation/Scheduled.java b/spring-context/src/main/java/org/springframework/scheduling/annotation/Scheduled.java index dea788ce088..383e4adc5de 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/annotation/Scheduled.java +++ b/spring-context/src/main/java/org/springframework/scheduling/annotation/Scheduled.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -64,18 +64,41 @@ public @interface Scheduled { */ long fixedDelay() default -1; + /** + * Execute the annotated method with a fixed period between the end + * of the last invocation and the start of the next. + * @return the delay in milliseconds as a String value, e.g. a placeholder + * @since 3.2.2 + */ + String fixedDelayString() default ""; + /** * Execute the annotated method with a fixed period between invocations. * @return the period in milliseconds */ long fixedRate() default -1; + /** + * Execute the annotated method with a fixed period between invocations. + * @return the period in milliseconds as a String value, e.g. a placeholder + * @since 3.2.2 + */ + String fixedRateString() default ""; + /** * Number of milliseconds to delay before the first execution of a * {@link #fixedRate()} or {@link #fixedDelay()} task. * @return the initial delay in milliseconds * @since 3.2 */ - long initialDelay() default 0; + long initialDelay() default -1; + + /** + * Number of milliseconds to delay before the first execution of a + * {@link #fixedRate()} or {@link #fixedDelay()} task. + * @return the initial delay in milliseconds as a String value, e.g. a placeholder + * @since 3.2.2 + */ + String initialDelayString() default ""; } diff --git a/spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java b/spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java index f5f236d2f9d..754afc5bd67 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java +++ b/spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -17,7 +17,6 @@ package org.springframework.scheduling.annotation; import java.lang.reflect.Method; - import java.util.HashMap; import java.util.Map; import java.util.concurrent.ScheduledExecutorService; @@ -111,53 +110,115 @@ public class ScheduledAnnotationBeanPostProcessor public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException { Scheduled annotation = AnnotationUtils.getAnnotation(method, Scheduled.class); if (annotation != null) { - Assert.isTrue(void.class.equals(method.getReturnType()), - "Only void-returning methods may be annotated with @Scheduled."); - Assert.isTrue(method.getParameterTypes().length == 0, - "Only no-arg methods may be annotated with @Scheduled."); - if (AopUtils.isJdkDynamicProxy(bean)) { - try { - // found a @Scheduled method on the target class for this JDK proxy -> is it - // also present on the proxy itself? - method = bean.getClass().getMethod(method.getName(), method.getParameterTypes()); + try { + Assert.isTrue(void.class.equals(method.getReturnType()), + "Only void-returning methods may be annotated with @Scheduled"); + Assert.isTrue(method.getParameterTypes().length == 0, + "Only no-arg methods may be annotated with @Scheduled"); + if (AopUtils.isJdkDynamicProxy(bean)) { + try { + // found a @Scheduled method on the target class for this JDK proxy -> is it + // also present on the proxy itself? + method = bean.getClass().getMethod(method.getName(), method.getParameterTypes()); + } + catch (SecurityException ex) { + ReflectionUtils.handleReflectionException(ex); + } + catch (NoSuchMethodException ex) { + throw new IllegalStateException(String.format( + "@Scheduled method '%s' found on bean target class '%s', " + + "but not found in any interface(s) for bean JDK proxy. Either " + + "pull the method up to an interface or switch to subclass (CGLIB) " + + "proxies by setting proxy-target-class/proxyTargetClass " + + "attribute to 'true'", method.getName(), targetClass.getSimpleName())); + } } - catch (SecurityException ex) { - ReflectionUtils.handleReflectionException(ex); + Runnable runnable = new ScheduledMethodRunnable(bean, method); + boolean processedSchedule = false; + String errorMessage = "Exactly one of the 'cron', 'fixedDelay(String)', or 'fixedRate(String)' attributes is required"; + // Determine initial delay + long initialDelay = annotation.initialDelay(); + String initialDelayString = annotation.initialDelayString(); + if (!"".equals(initialDelayString)) { + Assert.isTrue(initialDelay < 0, "Specify 'initialDelay' or 'initialDelayString', not both"); + if (embeddedValueResolver != null) { + initialDelayString = embeddedValueResolver.resolveStringValue(initialDelayString); + } + try { + initialDelay = Integer.parseInt(initialDelayString); + } + catch (NumberFormatException ex) { + throw new IllegalArgumentException( + "Invalid initialDelayString value \"" + initialDelayString + "\" - cannot parse into integer"); + } } - catch (NoSuchMethodException ex) { - throw new IllegalStateException(String.format( - "@Scheduled method '%s' found on bean target class '%s', " + - "but not found in any interface(s) for bean JDK proxy. Either " + - "pull the method up to an interface or switch to subclass (CGLIB) " + - "proxies by setting proxy-target-class/proxyTargetClass " + - "attribute to 'true'", method.getName(), targetClass.getSimpleName())); + // Check cron expression + String cron = annotation.cron(); + if (!"".equals(cron)) { + Assert.isTrue(initialDelay == -1, "'initialDelay' not supported for cron triggers"); + processedSchedule = true; + if (embeddedValueResolver != null) { + cron = embeddedValueResolver.resolveStringValue(cron); + } + registrar.addCronTask(new CronTask(runnable, cron)); } - } - Runnable runnable = new ScheduledMethodRunnable(bean, method); - boolean processedSchedule = false; - String errorMessage = "Exactly one of the 'cron', 'fixedDelay', or 'fixedRate' attributes is required."; - String cron = annotation.cron(); - if (!"".equals(cron)) { - processedSchedule = true; - if (embeddedValueResolver != null) { - cron = embeddedValueResolver.resolveStringValue(cron); + // At this point we don't need to differentiate between initial delay set or not anymore + if (initialDelay < 0) { + initialDelay = 0; } - registrar.addCronTask(new CronTask(runnable, cron)); + // Check fixed delay + long fixedDelay = annotation.fixedDelay(); + if (fixedDelay >= 0) { + Assert.isTrue(!processedSchedule, errorMessage); + processedSchedule = true; + registrar.addFixedDelayTask(new IntervalTask(runnable, fixedDelay, initialDelay)); + } + String fixedDelayString = annotation.fixedDelayString(); + if (!"".equals(fixedDelayString)) { + Assert.isTrue(!processedSchedule, errorMessage); + processedSchedule = true; + if (embeddedValueResolver != null) { + fixedDelayString = embeddedValueResolver.resolveStringValue(fixedDelayString); + } + try { + fixedDelay = Integer.parseInt(fixedDelayString); + } + catch (NumberFormatException ex) { + throw new IllegalArgumentException( + "Invalid fixedDelayString value \"" + fixedDelayString + "\" - cannot parse into integer"); + } + registrar.addFixedDelayTask(new IntervalTask(runnable, fixedDelay, initialDelay)); + } + // Check fixed rate + long fixedRate = annotation.fixedRate(); + if (fixedRate >= 0) { + Assert.isTrue(!processedSchedule, errorMessage); + processedSchedule = true; + registrar.addFixedRateTask(new IntervalTask(runnable, fixedRate, initialDelay)); + } + String fixedRateString = annotation.fixedRateString(); + if (!"".equals(fixedRateString)) { + Assert.isTrue(!processedSchedule, errorMessage); + processedSchedule = true; + if (embeddedValueResolver != null) { + fixedRateString = embeddedValueResolver.resolveStringValue(fixedRateString); + } + try { + fixedRate = Integer.parseInt(fixedRateString); + } + catch (NumberFormatException ex) { + throw new IllegalArgumentException( + "Invalid fixedRateString value \"" + fixedRateString + "\" - cannot parse into integer"); + } + registrar.addFixedRateTask(new IntervalTask(runnable, fixedRate, initialDelay)); + } + // Check whether we had any attribute set + Assert.isTrue(processedSchedule, errorMessage); } - long initialDelay = annotation.initialDelay(); - long fixedDelay = annotation.fixedDelay(); - if (fixedDelay >= 0) { - Assert.isTrue(!processedSchedule, errorMessage); - processedSchedule = true; - registrar.addFixedDelayTask(new IntervalTask(runnable, fixedDelay, initialDelay)); + catch (IllegalArgumentException ex) { + throw new IllegalStateException( + "Encountered invalid @Scheduled method '" + method.getName() + "': " + ex.getMessage()); } - long fixedRate = annotation.fixedRate(); - if (fixedRate >= 0) { - Assert.isTrue(!processedSchedule, errorMessage); - processedSchedule = true; - registrar.addFixedRateTask(new IntervalTask(runnable, fixedRate, initialDelay)); - } - Assert.isTrue(processedSchedule, errorMessage); } } }); @@ -168,18 +229,14 @@ public class ScheduledAnnotationBeanPostProcessor if (event.getApplicationContext() != this.applicationContext) { return; } - Map configurers = this.applicationContext.getBeansOfType(SchedulingConfigurer.class); - if (this.scheduler != null) { this.registrar.setScheduler(this.scheduler); } - for (SchedulingConfigurer configurer : configurers.values()) { configurer.configureTasks(this.registrar); } - if (this.registrar.hasTasks() && this.registrar.getScheduler() == null) { Map schedulers = new HashMap(); schedulers.putAll(applicationContext.getBeansOfType(TaskScheduler.class)); @@ -199,7 +256,6 @@ public class ScheduledAnnotationBeanPostProcessor "configureTasks() callback. Found the following beans: " + schedulers.keySet()); } } - this.registrar.afterPropertiesSet(); } diff --git a/spring-context/src/main/java/org/springframework/scheduling/config/IntervalTask.java b/spring-context/src/main/java/org/springframework/scheduling/config/IntervalTask.java index 4b39927e8f7..567f9fdbfe1 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/config/IntervalTask.java +++ b/spring-context/src/main/java/org/springframework/scheduling/config/IntervalTask.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -44,8 +44,8 @@ public class IntervalTask extends Task { */ public IntervalTask(Runnable runnable, long interval, long initialDelay) { super(runnable); - this.initialDelay = initialDelay; this.interval = interval; + this.initialDelay = initialDelay; } /** @@ -59,10 +59,11 @@ public class IntervalTask extends Task { public long getInterval() { - return interval; + return this.interval; } public long getInitialDelay() { - return initialDelay; + return this.initialDelay; } + } diff --git a/spring-context/src/main/java/org/springframework/scheduling/support/CronSequenceGenerator.java b/spring-context/src/main/java/org/springframework/scheduling/support/CronSequenceGenerator.java index b3530ebf785..5f5cf4e2809 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/support/CronSequenceGenerator.java +++ b/spring-context/src/main/java/org/springframework/scheduling/support/CronSequenceGenerator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -69,6 +69,7 @@ public class CronSequenceGenerator { private final TimeZone timeZone; + /** * Construct a {@link CronSequenceGenerator} from the pattern provided. * @param expression a space-separated list of time fields @@ -81,6 +82,7 @@ public class CronSequenceGenerator { parse(expression); } + /** * Get the next {@link Date} in the sequence matching the Cron pattern and * after the value provided. The return value will have a whole number of @@ -135,7 +137,8 @@ public class CronSequenceGenerator { int updateMinute = findNext(this.minutes, minute, calendar, Calendar.MINUTE, Calendar.HOUR_OF_DAY, resets); if (minute == updateMinute) { resets.add(Calendar.MINUTE); - } else { + } + else { doNext(calendar, dot); } @@ -143,7 +146,8 @@ public class CronSequenceGenerator { int updateHour = findNext(this.hours, hour, calendar, Calendar.HOUR_OF_DAY, Calendar.DAY_OF_WEEK, resets); if (hour == updateHour) { resets.add(Calendar.HOUR_OF_DAY); - } else { + } + else { doNext(calendar, dot); } @@ -152,7 +156,8 @@ public class CronSequenceGenerator { int updateDayOfMonth = findNextDay(calendar, this.daysOfMonth, dayOfMonth, daysOfWeek, dayOfWeek, resets); if (dayOfMonth == updateDayOfMonth) { resets.add(Calendar.DAY_OF_MONTH); - } else { + } + else { doNext(calendar, dot); } @@ -160,7 +165,8 @@ public class CronSequenceGenerator { int updateMonth = findNext(this.months, month, calendar, Calendar.MONTH, Calendar.YEAR, resets); if (month != updateMonth) { if (calendar.get(Calendar.YEAR) - dot > 4) { - throw new IllegalStateException("Invalid cron expression led to runaway search for next trigger"); + throw new IllegalArgumentException("Invalid cron expression \"" + this.expression + + "\" led to runaway search for next trigger"); } doNext(calendar, dot); } @@ -181,7 +187,7 @@ public class CronSequenceGenerator { reset(calendar, resets); } if (count >= max) { - throw new IllegalStateException("Overflow in day for expression=" + this.expression); + throw new IllegalArgumentException("Overflow in day for expression \"" + this.expression + "\""); } return dayOfMonth; } @@ -222,7 +228,8 @@ public class CronSequenceGenerator { } } - // Parsing logic invoked by the constructor. + + // Parsing logic invoked by the constructor /** * Parse the given pattern expression. @@ -230,8 +237,8 @@ public class CronSequenceGenerator { private void parse(String expression) throws IllegalArgumentException { String[] fields = StringUtils.tokenizeToStringArray(expression, " "); if (fields.length != 6) { - throw new IllegalArgumentException(String.format("" - + "cron expression must consist of 6 fields (found %d in %s)", fields.length, expression)); + throw new IllegalArgumentException(String.format( + "Cron expression must consist of 6 fields (found %d in \"%s\")", fields.length, expression)); } setNumberHits(this.seconds, fields[0], 0, 60); setNumberHits(this.minutes, fields[1], 0, 60); @@ -296,10 +303,12 @@ public class CronSequenceGenerator { // Not an incrementer so it must be a range (possibly empty) int[] range = getRange(field, min, max); bits.set(range[0], range[1] + 1); - } else { + } + else { String[] split = StringUtils.delimitedListToStringArray(field, "/"); if (split.length > 2) { - throw new IllegalArgumentException("Incrementer has more than two fields: " + field); + throw new IllegalArgumentException("Incrementer has more than two fields: '" + + field + "' in expression \"" + this.expression + "\""); } int[] range = getRange(split[0], min, max); if (!split[0].contains("-")) { @@ -322,19 +331,23 @@ public class CronSequenceGenerator { } if (!field.contains("-")) { result[0] = result[1] = Integer.valueOf(field); - } else { + } + else { String[] split = StringUtils.delimitedListToStringArray(field, "-"); if (split.length > 2) { - throw new IllegalArgumentException("Range has more than two fields: " + field); + throw new IllegalArgumentException("Range has more than two fields: '" + + field + "' in expression \"" + this.expression + "\""); } result[0] = Integer.valueOf(split[0]); result[1] = Integer.valueOf(split[1]); } if (result[0] >= max || result[1] >= max) { - throw new IllegalArgumentException("Range exceeds maximum (" + max + "): " + field); + throw new IllegalArgumentException("Range exceeds maximum (" + max + "): '" + + field + "' in expression \"" + this.expression + "\""); } if (result[0] < min || result[1] < min) { - throw new IllegalArgumentException("Range less than minimum (" + min + "): " + field); + throw new IllegalArgumentException("Range less than minimum (" + min + "): '" + + field + "' in expression \"" + this.expression + "\""); } return result; } diff --git a/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java b/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java index e100da3d47d..91bf0f697e6 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java +++ b/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java @@ -26,6 +26,7 @@ import java.util.List; import java.util.Properties; import org.junit.Test; + import org.springframework.beans.DirectFieldAccessor; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.config.BeanDefinition; @@ -52,8 +53,7 @@ public class ScheduledAnnotationBeanPostProcessorTests { public void fixedDelayTask() { StaticApplicationContext context = new StaticApplicationContext(); BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class); - BeanDefinition targetDefinition = new RootBeanDefinition( - ScheduledAnnotationBeanPostProcessorTests.FixedDelayTestBean.class); + BeanDefinition targetDefinition = new RootBeanDefinition(FixedDelayTestBean.class); context.registerBeanDefinition("postProcessor", processorDefinition); context.registerBeanDefinition("target", targetDefinition); context.refresh(); @@ -106,8 +106,7 @@ public class ScheduledAnnotationBeanPostProcessorTests { public void fixedRateTaskWithInitialDelay() { StaticApplicationContext context = new StaticApplicationContext(); BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class); - BeanDefinition targetDefinition = new RootBeanDefinition( - ScheduledAnnotationBeanPostProcessorTests.FixedRateWithInitialDelayTestBean.class); + BeanDefinition targetDefinition = new RootBeanDefinition(FixedRateWithInitialDelayTestBean.class); context.registerBeanDefinition("postProcessor", processorDefinition); context.registerBeanDefinition("target", targetDefinition); context.refresh(); @@ -162,8 +161,7 @@ public class ScheduledAnnotationBeanPostProcessorTests { public void metaAnnotationWithFixedRate() { StaticApplicationContext context = new StaticApplicationContext(); BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class); - BeanDefinition targetDefinition = new RootBeanDefinition( - ScheduledAnnotationBeanPostProcessorTests.MetaAnnotationFixedRateTestBean.class); + BeanDefinition targetDefinition = new RootBeanDefinition(MetaAnnotationFixedRateTestBean.class); context.registerBeanDefinition("postProcessor", processorDefinition); context.registerBeanDefinition("target", targetDefinition); context.refresh(); @@ -211,7 +209,7 @@ public class ScheduledAnnotationBeanPostProcessorTests { } @Test - public void propertyPlaceholderWithCronExpression() { + public void propertyPlaceholderWithCron() { String businessHoursCronExpression = "0 0 9-17 * * MON-FRI"; StaticApplicationContext context = new StaticApplicationContext(); BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class); @@ -219,8 +217,7 @@ public class ScheduledAnnotationBeanPostProcessorTests { Properties properties = new Properties(); properties.setProperty("schedules.businessHours", businessHoursCronExpression); placeholderDefinition.getPropertyValues().addPropertyValue("properties", properties); - BeanDefinition targetDefinition = new RootBeanDefinition( - ScheduledAnnotationBeanPostProcessorTests.PropertyPlaceholderTestBean.class); + BeanDefinition targetDefinition = new RootBeanDefinition(PropertyPlaceholderWithCronTestBean.class); context.registerBeanDefinition("placeholder", placeholderDefinition); context.registerBeanDefinition("postProcessor", processorDefinition); context.registerBeanDefinition("target", targetDefinition); @@ -242,6 +239,70 @@ public class ScheduledAnnotationBeanPostProcessorTests { assertEquals(businessHoursCronExpression, task.getExpression()); } + @Test + public void propertyPlaceholderWithFixedDelay() { + StaticApplicationContext context = new StaticApplicationContext(); + BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class); + BeanDefinition placeholderDefinition = new RootBeanDefinition(PropertyPlaceholderConfigurer.class); + Properties properties = new Properties(); + properties.setProperty("fixedDelay", "5000"); + properties.setProperty("initialDelay", "1000"); + placeholderDefinition.getPropertyValues().addPropertyValue("properties", properties); + BeanDefinition targetDefinition = new RootBeanDefinition(PropertyPlaceholderWithFixedDelayTestBean.class); + context.registerBeanDefinition("placeholder", placeholderDefinition); + context.registerBeanDefinition("postProcessor", processorDefinition); + context.registerBeanDefinition("target", targetDefinition); + context.refresh(); + Object postProcessor = context.getBean("postProcessor"); + Object target = context.getBean("target"); + ScheduledTaskRegistrar registrar = (ScheduledTaskRegistrar) + new DirectFieldAccessor(postProcessor).getPropertyValue("registrar"); + @SuppressWarnings("unchecked") + List fixedDelayTasks = (List) + new DirectFieldAccessor(registrar).getPropertyValue("fixedDelayTasks"); + assertEquals(1, fixedDelayTasks.size()); + IntervalTask task = fixedDelayTasks.get(0); + ScheduledMethodRunnable runnable = (ScheduledMethodRunnable) task.getRunnable(); + Object targetObject = runnable.getTarget(); + Method targetMethod = runnable.getMethod(); + assertEquals(target, targetObject); + assertEquals("fixedDelay", targetMethod.getName()); + assertEquals(1000L, task.getInitialDelay()); + assertEquals(5000L, task.getInterval()); + } + + @Test + public void propertyPlaceholderWithFixedRate() { + StaticApplicationContext context = new StaticApplicationContext(); + BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class); + BeanDefinition placeholderDefinition = new RootBeanDefinition(PropertyPlaceholderConfigurer.class); + Properties properties = new Properties(); + properties.setProperty("fixedRate", "3000"); + properties.setProperty("initialDelay", "1000"); + placeholderDefinition.getPropertyValues().addPropertyValue("properties", properties); + BeanDefinition targetDefinition = new RootBeanDefinition(PropertyPlaceholderWithFixedRateTestBean.class); + context.registerBeanDefinition("placeholder", placeholderDefinition); + context.registerBeanDefinition("postProcessor", processorDefinition); + context.registerBeanDefinition("target", targetDefinition); + context.refresh(); + Object postProcessor = context.getBean("postProcessor"); + Object target = context.getBean("target"); + ScheduledTaskRegistrar registrar = (ScheduledTaskRegistrar) + new DirectFieldAccessor(postProcessor).getPropertyValue("registrar"); + @SuppressWarnings("unchecked") + List fixedRateTasks = (List) + new DirectFieldAccessor(registrar).getPropertyValue("fixedRateTasks"); + assertEquals(1, fixedRateTasks.size()); + IntervalTask task = fixedRateTasks.get(0); + ScheduledMethodRunnable runnable = (ScheduledMethodRunnable) task.getRunnable(); + Object targetObject = runnable.getTarget(); + Method targetMethod = runnable.getMethod(); + assertEquals(target, targetObject); + assertEquals("fixedRate", targetMethod.getName()); + assertEquals(1000L, task.getInitialDelay()); + assertEquals(3000L, task.getInterval()); + } + @Test public void propertyPlaceholderForMetaAnnotation() { String businessHoursCronExpression = "0 0 9-17 * * MON-FRI"; @@ -285,7 +346,7 @@ public class ScheduledAnnotationBeanPostProcessorTests { context.refresh(); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = BeanCreationException.class) public void invalidCron() throws Throwable { StaticApplicationContext context = new StaticApplicationContext(); BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class); @@ -293,12 +354,7 @@ public class ScheduledAnnotationBeanPostProcessorTests { ScheduledAnnotationBeanPostProcessorTests.InvalidCronTestBean.class); context.registerBeanDefinition("postProcessor", processorDefinition); context.registerBeanDefinition("target", targetDefinition); - try { - context.refresh(); - fail("expected exception"); - } catch (BeanCreationException ex) { - throw ex.getRootCause(); - } + context.refresh(); } @Test(expected = BeanCreationException.class) @@ -342,7 +398,7 @@ public class ScheduledAnnotationBeanPostProcessorTests { static class FixedRateWithInitialDelayTestBean { - @Scheduled(initialDelay=1000, fixedRate=3000) + @Scheduled(fixedRate=3000, initialDelay=1000) public void fixedRate() { } } @@ -395,13 +451,13 @@ public class ScheduledAnnotationBeanPostProcessorTests { } - @Scheduled(fixedRate = 5000) + @Scheduled(fixedRate=5000) @Target(ElementType.METHOD) @Retention(RetentionPolicy.RUNTIME) private static @interface EveryFiveSeconds {} - @Scheduled(cron = "0 0 * * * ?") + @Scheduled(cron="0 0 * * * ?") @Target(ElementType.METHOD) @Retention(RetentionPolicy.RUNTIME) private static @interface Hourly {} @@ -423,7 +479,7 @@ public class ScheduledAnnotationBeanPostProcessorTests { } - static class PropertyPlaceholderTestBean { + static class PropertyPlaceholderWithCronTestBean { @Scheduled(cron = "${schedules.businessHours}") public void x() { @@ -431,7 +487,23 @@ public class ScheduledAnnotationBeanPostProcessorTests { } - @Scheduled(cron = "${schedules.businessHours}") + static class PropertyPlaceholderWithFixedDelayTestBean { + + @Scheduled(fixedDelayString="${fixedDelay}", initialDelayString="${initialDelay}") + public void fixedDelay() { + } + } + + + static class PropertyPlaceholderWithFixedRateTestBean { + + @Scheduled(fixedRateString="${fixedRate}", initialDelayString="${initialDelay}") + public void fixedRate() { + } + } + + + @Scheduled(cron="${schedules.businessHours}") @Target(ElementType.METHOD) @Retention(RetentionPolicy.RUNTIME) private static @interface BusinessHours {} From f9bac48d84bf1c632410aae10e4e2a70a0fbeb7c Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 8 Feb 2013 00:59:45 +0100 Subject: [PATCH 09/11] Further preparations for 3.2.2 --- src/dist/changelog.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/dist/changelog.txt b/src/dist/changelog.txt index d033dfbc534..7cd00264a9d 100644 --- a/src/dist/changelog.txt +++ b/src/dist/changelog.txt @@ -6,10 +6,13 @@ http://www.springsource.org Changes in version 3.2.2 (2013-03-07) -------------------------------------- -* official support for Hibernate 4.2 +* official support for Hibernate 4.2 (SPR-10255) * ConfigurationClassPostProcessor consistently uses ClassLoader, not loading core JDK annotations via ASM (SPR-10249) * ConfigurationClassPostProcessor allows for overriding of scoped-proxy bean definitions (SPR-10265) +* "depends-on" attribute on lang namespace element actually respected at runtime now (SPR-8625) +* allow for ordering of mixed AspectJ before/after advices (SPR-9438) * added "maximumAutoGrowSize" property to SpelParserConfiguration (SPR-10229) +* @Scheduled provides String variants of fixedDelay, fixedRate, initialDelay for placeholder support (SPR-8067) * SQLErrorCodeSQLExceptionTranslator tries to find SQLException with actual error code among causes (SPR-10260) * DefaultMessageListenerContainer invokes specified ExceptionListener for recovery exceptions as well (SPR-10230) * DefaultMessageListenerContainer logs recovery failures at error level and exposes "isRecovering()" method (SPR-10230) From 0058503cf0d0bd552d9938c467dd83a287831997 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 8 Feb 2013 01:13:08 +0100 Subject: [PATCH 10/11] @Scheduled provides String variants of fixedDelay, fixedRate, initialDelay for placeholder support Issue: SPR-8067 --- .../springframework/scheduling/support/CronTriggerTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spring-context/src/test/java/org/springframework/scheduling/support/CronTriggerTests.java b/spring-context/src/test/java/org/springframework/scheduling/support/CronTriggerTests.java index 49574ae29d5..a99bffa1ed8 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/support/CronTriggerTests.java +++ b/spring-context/src/test/java/org/springframework/scheduling/support/CronTriggerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2013 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. @@ -487,7 +487,7 @@ public class CronTriggerTests { assertEquals(calendar.getTime(), date = trigger.nextExecutionTime(context2)); } - @Test(expected=IllegalStateException.class) + @Test(expected = IllegalArgumentException.class) public void testNonExistentSpecificDate() throws Exception { // TODO: maybe try and detect this as a special case in parser? CronTrigger trigger = new CronTrigger("0 0 0 31 6 *", timeZone); From 584e79c677e69853a983667521a20b66331d52a6 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 7 Feb 2013 17:22:09 -0800 Subject: [PATCH 11/11] Promote use of @PostConstruct and @PreDestroy Update reference documentation to promote the use of the JSR-250 @PostConstruct and @PreDestroy annotations. Issue: SPR-8493 --- src/reference/docbook/beans-customizing.xml | 27 +++++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/reference/docbook/beans-customizing.xml b/src/reference/docbook/beans-customizing.xml index acca5514a74..9e5c4284b98 100644 --- a/src/reference/docbook/beans-customizing.xml +++ b/src/reference/docbook/beans-customizing.xml @@ -13,18 +13,25 @@
Lifecycle callbacks - - To interact with the container's management of the bean lifecycle, you can implement the Spring InitializingBean and DisposableBean interfaces. The container calls afterPropertiesSet() for the former and destroy() for the latter to allow the bean to perform certain actions upon initialization and destruction of - your beans. You can also achieve the same integration with the container - without coupling your classes to Spring interfaces through the use of - init-method and destroy method object definition metadata. + your beans. + + + The JSR-250 @PostConstruct and + @PreDestroy annotations are generally + considered best practice for receiving lifecycle callbacks in a modern + Spring application. Using these annotations means that your beans are not + coupled to Spring specific interfaces. For details see + . + If you don't want to use the JSR-250 annotations but you are still + looking to remove coupling consider the use of init-method and destroy-method + object definition metadata. + Internally, the Spring Framework uses BeanPostProcessor implementations to @@ -57,7 +64,9 @@ It is recommended that you do not use the InitializingBean interface because it - unnecessarily couples the code to Spring. Alternatively, specify a POJO + unnecessarily couples the code to Spring. Alternatively, use the + + @PostConstruct annotation or specify a POJO initialization method. In the case of XML-based configuration metadata, you use the init-method attribute to specify the name of the method that has a void no-argument signature. For example, the @@ -99,7 +108,9 @@ It is recommended that you do not use the DisposableBean callback interface because - it unnecessarily couples the code to Spring. Alternatively, specify a + it unnecessarily couples the code to Spring. Alternatively, use the + + @PreDestroy annotation or specify a generic method that is supported by bean definitions. With XML-based configuration metadata, you use the destroy-method attribute on the <bean/>. For example, the