From f4de1ea1472f6c7ca9b34067d58e2d9a08ad1b5f Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 22 Feb 2017 15:32:19 +0100 Subject: [PATCH] Polishing --- .../aop/framework/CglibAopProxy.java | 14 +-- .../ConfigurationClassPostProcessorTests.java | 12 +-- .../context/annotation/Spr11202Tests.java | 29 +++--- .../context/annotation/Spr6602Tests.java | 11 ++- .../ConfigurationClassProcessingTests.java | 54 +++++------ .../core/annotation/AnnotationUtilsTests.java | 10 +- .../common/TemplateAwareExpressionParser.java | 96 ++++++++----------- src/asciidoc/core-beans.adoc | 4 +- 8 files changed, 102 insertions(+), 128 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java b/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java index a708e7bb88..9e7d0d25a1 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 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. @@ -202,19 +202,13 @@ class CglibAopProxy implements AopProxy, Serializable { // Generate the proxy class and create a proxy instance. return createProxyClassAndInstance(enhancer, callbacks); } - catch (CodeGenerationException ex) { + catch (CodeGenerationException | IllegalArgumentException ex) { throw new AopConfigException("Could not generate CGLIB subclass of class [" + this.advised.getTargetClass() + "]: " + "Common causes of this problem include using a final class or a non-visible class", ex); } - catch (IllegalArgumentException ex) { - throw new AopConfigException("Could not generate CGLIB subclass of class [" + - this.advised.getTargetClass() + "]: " + - "Common causes of this problem include using a final class or a non-visible class", - ex); - } - catch (Exception ex) { + catch (Throwable ex) { // TargetSource.getTarget() failed throw new AopConfigException("Unexpected AOP exception", ex); } @@ -256,7 +250,7 @@ class CglibAopProxy implements AopProxy, Serializable { * methods across ClassLoaders, and writes warnings to the log for each one found. */ private void doValidateClass(Class proxySuperClass, ClassLoader proxyClassLoader) { - if (Object.class != proxySuperClass) { + if (proxySuperClass != Object.class) { Method[] methods = proxySuperClass.getDeclaredMethods(); for (Method method : methods) { int mod = method.getModifiers(); diff --git a/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java index a8eec500df..d194ede6f9 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java @@ -66,18 +66,17 @@ import static org.junit.Assert.*; */ public class ConfigurationClassPostProcessorTests { - private DefaultListableBeanFactory beanFactory; + private final DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); @Before - public void setUp() { - DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); + public void setup() { QualifierAnnotationAutowireCandidateResolver acr = new QualifierAnnotationAutowireCandidateResolver(); - acr.setBeanFactory(bf); - bf.setAutowireCandidateResolver(acr); - this.beanFactory = bf; + acr.setBeanFactory(this.beanFactory); + this.beanFactory.setAutowireCandidateResolver(acr); } + /** * Enhanced {@link Configuration} classes are only necessary for respecting * certain bean semantics, like singleton-scoping, scoped proxies, etc. @@ -950,7 +949,6 @@ public class ConfigurationClassPostProcessorTests { } } - @Configuration public static class RawRepositoryConfiguration { diff --git a/spring-context/src/test/java/org/springframework/context/annotation/Spr11202Tests.java b/spring-context/src/test/java/org/springframework/context/annotation/Spr11202Tests.java index 7cc1d2ca41..53d231d951 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/Spr11202Tests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/Spr11202Tests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2017 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,11 +22,11 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -import org.junit.After; import org.junit.Test; import org.springframework.beans.factory.FactoryBean; import org.springframework.beans.factory.InitializingBean; +import org.springframework.context.ApplicationContext; import org.springframework.core.type.AnnotatedTypeMetadata; import org.springframework.core.type.AnnotationMetadata; import org.springframework.util.Assert; @@ -38,24 +38,15 @@ import static org.junit.Assert.*; */ public class Spr11202Tests { - private AnnotationConfigApplicationContext context; - - @After - public void close() { - if (context != null) { - context.close(); - } - } - - @Test // Fails + @Test public void testWithImporter() { - context = new AnnotationConfigApplicationContext(Wrapper.class); + ApplicationContext context = new AnnotationConfigApplicationContext(Wrapper.class); assertEquals("foo", context.getBean("value")); } - @Test // Passes + @Test public void testWithoutImporter() { - context = new AnnotationConfigApplicationContext(Config.class); + ApplicationContext context = new AnnotationConfigApplicationContext(Config.class); assertEquals("foo", context.getBean("value")); } @@ -65,6 +56,7 @@ public class Spr11202Tests { protected static class Wrapper { } + protected static class Selector implements ImportSelector { @Override @@ -73,6 +65,7 @@ public class Spr11202Tests { } } + @Configuration protected static class Config { @@ -95,6 +88,7 @@ public class Spr11202Tests { } } + protected static class NoBarCondition implements Condition { @Override @@ -106,12 +100,14 @@ public class Spr11202Tests { } } + @Retention(RetentionPolicy.RUNTIME) @Documented @Target(ElementType.TYPE) - protected static @interface Bar { + protected @interface Bar { } + protected static class FooFactoryBean implements FactoryBean, InitializingBean { private Foo foo = new Foo(); @@ -137,6 +133,7 @@ public class Spr11202Tests { } } + protected static class Foo { private String name; diff --git a/spring-context/src/test/java/org/springframework/context/annotation/Spr6602Tests.java b/spring-context/src/test/java/org/springframework/context/annotation/Spr6602Tests.java index 5e7f4d60ee..a7e57071db 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/Spr6602Tests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/Spr6602Tests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2017 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. @@ -32,6 +32,7 @@ import static org.junit.Assert.*; * @author Chris Beams */ public class Spr6602Tests { + @Test public void testXmlBehavior() throws Exception { doAssertions(new ClassPathXmlApplicationContext("Spr6602Tests-context.xml", Spr6602Tests.class)); @@ -59,8 +60,10 @@ public class Spr6602Tests { assertThat(bar3, is(not(bar4))); } + @Configuration public static class FooConfig { + @Bean public Foo foo() throws Exception { return new Foo(barFactory().getObject()); @@ -72,7 +75,9 @@ public class Spr6602Tests { } } + public static class Foo { + final Bar bar; public Foo(Bar bar) { @@ -80,9 +85,11 @@ public class Spr6602Tests { } } + public static class Bar { } + public static class BarFactory implements FactoryBean { @Override @@ -102,4 +109,4 @@ public class Spr6602Tests { } -} \ No newline at end of file +} diff --git a/spring-context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationClassProcessingTests.java b/spring-context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationClassProcessingTests.java index 88d27eaca7..e34684e7ee 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationClassProcessingTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationClassProcessingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 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. @@ -63,8 +63,8 @@ import org.springframework.tests.sample.beans.TestBean; import static org.junit.Assert.*; /** - * Miscellaneous system tests covering {@link Bean} naming, aliases, scoping and error - * handling within {@link Configuration} class definitions. + * Miscellaneous system tests covering {@link Bean} naming, aliases, scoping and + * error handling within {@link Configuration} class definitions. * * @author Chris Beams * @author Juergen Hoeller @@ -72,30 +72,6 @@ import static org.junit.Assert.*; */ public class ConfigurationClassProcessingTests { - /** - * Creates a new {@link BeanFactory}, populates it with a {@link BeanDefinition} for - * each of the given {@link Configuration} configClasses, and then - * post-processes the factory using JavaConfig's {@link ConfigurationClassPostProcessor}. - * When complete, the factory is ready to service requests for any {@link Bean} methods - * declared by configClasses. - */ - private DefaultListableBeanFactory initBeanFactory(Class... configClasses) { - DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); - for (Class configClass : configClasses) { - String configBeanName = configClass.getName(); - factory.registerBeanDefinition(configBeanName, new RootBeanDefinition(configClass)); - } - ConfigurationClassPostProcessor ccpp = new ConfigurationClassPostProcessor(); - ccpp.postProcessBeanDefinitionRegistry(factory); - ccpp.postProcessBeanFactory(factory); - RequiredAnnotationBeanPostProcessor rapp = new RequiredAnnotationBeanPostProcessor(); - rapp.setBeanFactory(factory); - factory.addBeanPostProcessor(rapp); - factory.freezeConfiguration(); - return factory; - } - - @Rule public final ExpectedException exception = ExpectedException.none(); @@ -267,6 +243,30 @@ public class ConfigurationClassProcessingTests { } + /** + * Creates a new {@link BeanFactory}, populates it with a {@link BeanDefinition} + * for each of the given {@link Configuration} {@code configClasses}, and then + * post-processes the factory using JavaConfig's {@link ConfigurationClassPostProcessor}. + * When complete, the factory is ready to service requests for any {@link Bean} methods + * declared by {@code configClasses}. + */ + private DefaultListableBeanFactory initBeanFactory(Class... configClasses) { + DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); + for (Class configClass : configClasses) { + String configBeanName = configClass.getName(); + factory.registerBeanDefinition(configBeanName, new RootBeanDefinition(configClass)); + } + ConfigurationClassPostProcessor ccpp = new ConfigurationClassPostProcessor(); + ccpp.postProcessBeanDefinitionRegistry(factory); + ccpp.postProcessBeanFactory(factory); + RequiredAnnotationBeanPostProcessor rapp = new RequiredAnnotationBeanPostProcessor(); + rapp.setBeanFactory(factory); + factory.addBeanPostProcessor(rapp); + factory.freezeConfiguration(); + return factory; + } + + @Configuration static class ConfigWithBeanWithCustomName { diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationUtilsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationUtilsTests.java index 81e795295d..07cd1f103b 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationUtilsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationUtilsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-20167the 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. @@ -1538,14 +1538,8 @@ public class AnnotationUtilsTests { assertArrayEquals(new char[] { 'x', 'y', 'z' }, chars); } + @SafeVarargs - // The following "varargs" suppression is necessary for javac from OpenJDK - // (1.8.0_60-b27); however, Eclipse warns that it's unnecessary. See the following - // Eclipse issues for details. - // - // https://bugs.eclipse.org/bugs/show_bug.cgi?id=344783 - // https://bugs.eclipse.org/bugs/show_bug.cgi?id=349669#c10 - // @SuppressWarnings("varargs") static T[] asArray(T... arr) { return arr; } diff --git a/spring-expression/src/main/java/org/springframework/expression/common/TemplateAwareExpressionParser.java b/spring-expression/src/main/java/org/springframework/expression/common/TemplateAwareExpressionParser.java index bbdc2bc747..92afac4cd9 100644 --- a/spring-expression/src/main/java/org/springframework/expression/common/TemplateAwareExpressionParser.java +++ b/spring-expression/src/main/java/org/springframework/expression/common/TemplateAwareExpressionParser.java @@ -40,31 +40,28 @@ public abstract class TemplateAwareExpressionParser implements ExpressionParser * Default ParserContext instance for non-template expressions. */ private static final ParserContext NON_TEMPLATE_PARSER_CONTEXT = new ParserContext() { - @Override public String getExpressionPrefix() { return null; } - @Override public String getExpressionSuffix() { return null; } - @Override public boolean isTemplate() { return false; } }; + @Override public Expression parseExpression(String expressionString) throws ParseException { return parseExpression(expressionString, NON_TEMPLATE_PARSER_CONTEXT); } @Override - public Expression parseExpression(String expressionString, ParserContext context) - throws ParseException { + public Expression parseExpression(String expressionString, ParserContext context) throws ParseException { if (context == null) { context = NON_TEMPLATE_PARSER_CONTEXT; } @@ -77,11 +74,12 @@ public abstract class TemplateAwareExpressionParser implements ExpressionParser } } - private Expression parseTemplate(String expressionString, ParserContext context) - throws ParseException { + + private Expression parseTemplate(String expressionString, ParserContext context) throws ParseException { if (expressionString.isEmpty()) { return new LiteralExpression(""); } + Expression[] expressions = parseExpressions(expressionString, context); if (expressions.length == 1) { return expressions[0]; @@ -109,8 +107,7 @@ public abstract class TemplateAwareExpressionParser implements ExpressionParser * @return the parsed expressions * @throws ParseException when the expressions cannot be parsed */ - private Expression[] parseExpressions(String expressionString, ParserContext context) - throws ParseException { + private Expression[] parseExpressions(String expressionString, ParserContext context) throws ParseException { List expressions = new LinkedList<>(); String prefix = context.getExpressionPrefix(); String suffix = context.getExpressionSuffix(); @@ -120,35 +117,29 @@ public abstract class TemplateAwareExpressionParser implements ExpressionParser if (prefixIndex >= startIdx) { // an inner expression was found - this is a composite if (prefixIndex > startIdx) { - expressions.add(createLiteralExpression(context, - expressionString.substring(startIdx, prefixIndex))); + expressions.add(new LiteralExpression(expressionString.substring(startIdx, prefixIndex))); } int afterPrefixIndex = prefixIndex + prefix.length(); - int suffixIndex = skipToCorrectEndSuffix(prefix, suffix, - expressionString, afterPrefixIndex); - + int suffixIndex = skipToCorrectEndSuffix(suffix, expressionString, afterPrefixIndex); if (suffixIndex == -1) { throw new ParseException(expressionString, prefixIndex, - "No ending suffix '" + suffix - + "' for expression starting at character " - + prefixIndex + ": " - + expressionString.substring(prefixIndex)); + "No ending suffix '" + suffix + "' for expression starting at character " + + prefixIndex + ": " + expressionString.substring(prefixIndex)); } if (suffixIndex == afterPrefixIndex) { throw new ParseException(expressionString, prefixIndex, - "No expression defined within delimiter '" + prefix + suffix - + "' at character " + prefixIndex); + "No expression defined within delimiter '" + prefix + suffix + + "' at character " + prefixIndex); } - String expr = expressionString.substring(prefixIndex + prefix.length(), - suffixIndex); + String expr = expressionString.substring(prefixIndex + prefix.length(), suffixIndex); expr = expr.trim(); if (expr.isEmpty()) { throw new ParseException(expressionString, prefixIndex, - "No expression defined within delimiter '" + prefix + suffix - + "' at character " + prefixIndex); + "No expression defined within delimiter '" + prefix + suffix + + "' at character " + prefixIndex); } expressions.add(doParseExpression(expr, context)); @@ -156,18 +147,13 @@ public abstract class TemplateAwareExpressionParser implements ExpressionParser } else { // no more ${expressions} found in string, add rest as static text - expressions.add(createLiteralExpression(context, - expressionString.substring(startIdx))); + expressions.add(new LiteralExpression(expressionString.substring(startIdx))); startIdx = expressionString.length(); } } return expressions.toArray(new Expression[expressions.size()]); } - private Expression createLiteralExpression(ParserContext context, String text) { - return new LiteralExpression(text); - } - /** * Return true if the specified suffix can be found at the supplied position in the * supplied expression string. @@ -192,15 +178,15 @@ public abstract class TemplateAwareExpressionParser implements ExpressionParser /** * Copes with nesting, for example '${...${...}}' where the correct end for the first * ${ is the final }. - * @param prefix the prefix * @param suffix the suffix * @param expressionString the expression string * @param afterPrefixIndex the most recently found prefix location for which the - * matching end suffix is being sought + * matching end suffix is being sought * @return the position of the correct matching nextSuffix or -1 if none can be found */ - private int skipToCorrectEndSuffix(String prefix, String suffix, - String expressionString, int afterPrefixIndex) throws ParseException { + private int skipToCorrectEndSuffix(String suffix, String expressionString, int afterPrefixIndex) + throws ParseException { + // Chew on the expression text - relying on the rules: // brackets must be in pairs: () [] {} // string literals are "..." or '...' and these may contain unmatched brackets @@ -226,16 +212,15 @@ public abstract class TemplateAwareExpressionParser implements ExpressionParser case ']': case ')': if (stack.isEmpty()) { - throw new ParseException(expressionString, pos, "Found closing '" - + ch + "' at position " + pos + " without an opening '" - + Bracket.theOpenBracketFor(ch) + "'"); + throw new ParseException(expressionString, pos, "Found closing '" + ch + + "' at position " + pos + " without an opening '" + + Bracket.theOpenBracketFor(ch) + "'"); } Bracket p = stack.pop(); if (!p.compatibleWithCloseBracket(ch)) { - throw new ParseException(expressionString, pos, "Found closing '" - + ch + "' at position " + pos - + " but most recent opening is '" + p.bracket - + "' at position " + p.pos); + throw new ParseException(expressionString, pos, "Found closing '" + ch + + "' at position " + pos + " but most recent opening is '" + p.bracket + + "' at position " + p.pos); } break; case '\'': @@ -244,8 +229,7 @@ public abstract class TemplateAwareExpressionParser implements ExpressionParser int endLiteral = expressionString.indexOf(ch, pos + 1); if (endLiteral == -1) { throw new ParseException(expressionString, pos, - "Found non terminating string literal starting at position " - + pos); + "Found non terminating string literal starting at position " + pos); } pos = endLiteral; break; @@ -254,9 +238,8 @@ public abstract class TemplateAwareExpressionParser implements ExpressionParser } if (!stack.isEmpty()) { Bracket p = stack.pop(); - throw new ParseException(expressionString, p.pos, "Missing closing '" - + Bracket.theCloseBracketFor(p.bracket) + "' for '" + p.bracket - + "' at position " + p.pos); + throw new ParseException(expressionString, p.pos, "Missing closing '" + + Bracket.theCloseBracketFor(p.bracket) + "' for '" + p.bracket + "' at position " + p.pos); } if (!isSuffixHere(expressionString, pos, suffix)) { return -1; @@ -265,6 +248,17 @@ public abstract class TemplateAwareExpressionParser implements ExpressionParser } + /** + * Actually parse the expression string and return an Expression object. + * @param expressionString the raw expression string to parse + * @param context a context for influencing this expression parsing routine (optional) + * @return an evaluator for the parsed expression + * @throws ParseException an exception occurred during parsing + */ + protected abstract Expression doParseExpression(String expressionString, ParserContext context) + throws ParseException; + + /** * This captures a type of bracket and the position in which it occurs in the * expression. The positional information is used if an error has to be reported @@ -313,14 +307,4 @@ public abstract class TemplateAwareExpressionParser implements ExpressionParser } } - /** - * Actually parse the expression string and return an Expression object. - * @param expressionString the raw expression string to parse - * @param context a context for influencing this expression parsing routine (optional) - * @return an evaluator for the parsed expression - * @throws ParseException an exception occurred during parsing - */ - protected abstract Expression doParseExpression(String expressionString, - ParserContext context) throws ParseException; - } diff --git a/src/asciidoc/core-beans.adoc b/src/asciidoc/core-beans.adoc index 9dae9a36c9..1bc2a1ff0f 100644 --- a/src/asciidoc/core-beans.adoc +++ b/src/asciidoc/core-beans.adoc @@ -1361,7 +1361,7 @@ element. - + ---- @@ -1375,7 +1375,7 @@ following snippet: - + ----