From 701f54b185af1207ad80ebf2ae584bd1aeb86a5d Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 24 Mar 2020 01:23:07 +0100 Subject: [PATCH] Polishing --- .../aop/framework/ProxyFactoryBean.java | 85 +++++++------------ .../context/annotation/Configuration.java | 11 +-- .../context/annotation/PropertySource.java | 51 ++++++----- .../io/support/PropertySourceFactory.java | 4 +- .../org/springframework/util/ClassUtils.java | 15 ++-- .../DataSourceTransactionManager.java | 11 +-- 6 files changed, 78 insertions(+), 99 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/ProxyFactoryBean.java b/spring-aop/src/main/java/org/springframework/aop/framework/ProxyFactoryBean.java index ff8c0e10e857..8d50bc0a479f 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/ProxyFactoryBean.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/ProxyFactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2020 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. @@ -21,9 +21,7 @@ import java.io.Serializable; import java.util.ArrayList; import java.util.Arrays; -import java.util.HashMap; import java.util.List; -import java.util.Map; import org.aopalliance.aop.Advice; import org.aopalliance.intercept.Interceptor; @@ -342,11 +340,8 @@ private synchronized Object newPrototypeInstance() { // an independent instance of the configuration. // In this case, no proxy will have an instance of this object's configuration, // but will have an independent copy. - if (logger.isTraceEnabled()) { - logger.trace("Creating copy of prototype ProxyFactoryBean config: " + this); - } - ProxyCreatorSupport copy = new ProxyCreatorSupport(getAopProxyFactory()); + // The copy needs a fresh advisor chain, and a fresh TargetSource. TargetSource targetSource = freshTargetSource(); copy.copyConfigurationFrom(this, targetSource, freshAdvisorChain()); @@ -359,9 +354,6 @@ private synchronized Object newPrototypeInstance() { } copy.setFrozen(this.freezeProxy); - if (logger.isTraceEnabled()) { - logger.trace("Using ProxyCreatorSupport copy: " + copy); - } return getProxy(copy.createAopProxy()); } @@ -395,9 +387,7 @@ private void checkInterceptorNames() { logger.debug("Bean with name '" + finalName + "' concluding interceptor chain " + "is not an advisor class: treating it as a target or TargetSource"); } - String[] newNames = new String[this.interceptorNames.length - 1]; - System.arraycopy(this.interceptorNames, 0, newNames, 0, newNames.length); - this.interceptorNames = newNames; + this.interceptorNames = Arrays.copyOf(this.interceptorNames, this.interceptorNames.length - 1); } } } @@ -449,16 +439,12 @@ private synchronized void initializeAdvisorChain() throws AopConfigException, Be // Materialize interceptor chain from bean names. for (String name : this.interceptorNames) { - if (logger.isTraceEnabled()) { - logger.trace("Configuring advisor or advice '" + name + "'"); - } - if (name.endsWith(GLOBAL_SUFFIX)) { if (!(this.beanFactory instanceof ListableBeanFactory)) { throw new AopConfigException( "Can only use global advisors or interceptors with a ListableBeanFactory"); } - addGlobalAdvisor((ListableBeanFactory) this.beanFactory, + addGlobalAdvisors((ListableBeanFactory) this.beanFactory, name.substring(0, name.length() - GLOBAL_SUFFIX.length())); } @@ -475,7 +461,7 @@ private synchronized void initializeAdvisorChain() throws AopConfigException, Be // Avoid unnecessary creation of prototype bean just for advisor chain initialization. advice = new PrototypePlaceholderAdvisor(name); } - addAdvisorOnChainCreation(advice, name); + addAdvisorOnChainCreation(advice); } } } @@ -498,11 +484,10 @@ private List freshAdvisorChain() { if (logger.isDebugEnabled()) { logger.debug("Refreshing bean named '" + pa.getBeanName() + "'"); } - // Replace the placeholder with a fresh prototype instance resulting - // from a getBean() lookup + // Replace the placeholder with a fresh prototype instance resulting from a getBean lookup if (this.beanFactory == null) { - throw new IllegalStateException("No BeanFactory available anymore (probably due to serialization) " + - "- cannot resolve prototype advisor '" + pa.getBeanName() + "'"); + throw new IllegalStateException("No BeanFactory available anymore (probably due to " + + "serialization) - cannot resolve prototype advisor '" + pa.getBeanName() + "'"); } Object bean = this.beanFactory.getBean(pa.getBeanName()); Advisor refreshedAdvisor = namedBeanToAdvisor(bean); @@ -519,28 +504,26 @@ private List freshAdvisorChain() { /** * Add all global interceptors and pointcuts. */ - private void addGlobalAdvisor(ListableBeanFactory beanFactory, String prefix) { + private void addGlobalAdvisors(ListableBeanFactory beanFactory, String prefix) { String[] globalAdvisorNames = BeanFactoryUtils.beanNamesForTypeIncludingAncestors(beanFactory, Advisor.class); String[] globalInterceptorNames = BeanFactoryUtils.beanNamesForTypeIncludingAncestors(beanFactory, Interceptor.class); - List beans = new ArrayList<>(globalAdvisorNames.length + globalInterceptorNames.length); - Map names = new HashMap<>(beans.size()); - for (String name : globalAdvisorNames) { - Object bean = beanFactory.getBean(name); - beans.add(bean); - names.put(bean, name); - } - for (String name : globalInterceptorNames) { - Object bean = beanFactory.getBean(name); - beans.add(bean); - names.put(bean, name); - } - AnnotationAwareOrderComparator.sort(beans); - for (Object bean : beans) { - String name = names.get(bean); - if (name.startsWith(prefix)) { - addAdvisorOnChainCreation(bean, name); + if (globalAdvisorNames.length > 0 || globalInterceptorNames.length > 0) { + List beans = new ArrayList<>(globalAdvisorNames.length + globalInterceptorNames.length); + for (String name : globalAdvisorNames) { + if (name.startsWith(prefix)) { + beans.add(beanFactory.getBean(name)); + } + } + for (String name : globalInterceptorNames) { + if (name.startsWith(prefix)) { + beans.add(beanFactory.getBean(name)); + } + } + AnnotationAwareOrderComparator.sort(beans); + for (Object bean : beans) { + addAdvisorOnChainCreation(bean); } } } @@ -551,17 +534,11 @@ private void addGlobalAdvisor(ListableBeanFactory beanFactory, String prefix) { * Because of these three possibilities, we can't type the signature * more strongly. * @param next advice, advisor or target object - * @param name bean name from which we obtained this object in our owning - * bean factory */ - private void addAdvisorOnChainCreation(Object next, String name) { + private void addAdvisorOnChainCreation(Object next) { // We need to convert to an Advisor if necessary so that our source reference // matches what we find from superclass interceptors. - Advisor advisor = namedBeanToAdvisor(next); - if (logger.isTraceEnabled()) { - logger.trace("Adding advisor with name '" + name + "'"); - } - addAdvisor(advisor); + addAdvisor(namedBeanToAdvisor(next)); } /** @@ -572,9 +549,7 @@ private void addAdvisorOnChainCreation(Object next, String name) { */ private TargetSource freshTargetSource() { if (this.targetName == null) { - if (logger.isTraceEnabled()) { - logger.trace("Not refreshing target: Bean name not specified in 'interceptorNames'."); - } + // Not refreshing target: bean name not specified in 'interceptorNames' return this.targetSource; } else { @@ -602,8 +577,8 @@ private Advisor namedBeanToAdvisor(Object next) { // We expected this to be an Advisor or Advice, // but it wasn't. This is a configuration error. throw new AopConfigException("Unknown advisor type " + next.getClass() + - "; Can only include Advisor or Advice type beans in interceptorNames chain except for last entry," + - "which may also be target or TargetSource", ex); + "; can only include Advisor or Advice type beans in interceptorNames chain " + + "except for last entry which may also be target instance or TargetSource", ex); } } @@ -614,7 +589,7 @@ private Advisor namedBeanToAdvisor(Object next) { protected void adviceChanged() { super.adviceChanged(); if (this.singleton) { - logger.debug("Advice has changed; recaching singleton instance"); + logger.debug("Advice has changed; re-caching singleton instance"); synchronized (this) { this.singletonInstance = null; } diff --git a/spring-context/src/main/java/org/springframework/context/annotation/Configuration.java b/spring-context/src/main/java/org/springframework/context/annotation/Configuration.java index 077c0ec3a687..084bd29f37d9 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/Configuration.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/Configuration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -344,15 +344,16 @@ * *

By default, {@code @Bean} methods will be eagerly instantiated at container * bootstrap time. To avoid this, {@code @Configuration} may be used in conjunction with - * the {@link Lazy @Lazy} annotation to indicate that all {@code @Bean} methods declared within - * the class are by default lazily initialized. Note that {@code @Lazy} may be used on - * individual {@code @Bean} methods as well. + * the {@link Lazy @Lazy} annotation to indicate that all {@code @Bean} methods declared + * within the class are by default lazily initialized. Note that {@code @Lazy} may be used + * on individual {@code @Bean} methods as well. * *

Testing support for {@code @Configuration} classes

* *

The Spring TestContext framework available in the {@code spring-test} module * provides the {@code @ContextConfiguration} annotation which can accept an array of - * {@code @Configuration} {@code Class} objects: + * component class references — typically {@code @Configuration} or + * {@code @Component} classes. * * 

  * @RunWith(SpringRunner.class)
diff --git a/spring-context/src/main/java/org/springframework/context/annotation/PropertySource.java b/spring-context/src/main/java/org/springframework/context/annotation/PropertySource.java
index f1ae3463e193..384197cb4e56 100644
--- a/spring-context/src/main/java/org/springframework/context/annotation/PropertySource.java
+++ b/spring-context/src/main/java/org/springframework/context/annotation/PropertySource.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -54,25 +54,30 @@
  *     }
  * }
* - * Notice that the {@code Environment} object is + *

Notice that the {@code Environment} object is * {@link org.springframework.beans.factory.annotation.Autowired @Autowired} into the * configuration class and then used when populating the {@code TestBean} object. Given * the configuration above, a call to {@code testBean.getName()} will return "myTestBean". * - *

Resolving ${...} placeholders in {@code } and {@code @Value} annotations

- * - * In order to resolve ${...} placeholders in {@code } definitions or {@code @Value} - * annotations using properties from a {@code PropertySource}, one must register - * a {@code PropertySourcesPlaceholderConfigurer}. This happens automatically when using - * {@code } in XML, but must be explicitly registered using - * a {@code static} {@code @Bean} method when using {@code @Configuration} classes. See - * the "Working with externalized values" section of @{@link Configuration}'s javadoc and - * "a note on BeanFactoryPostProcessor-returning @Bean methods" of @{@link Bean}'s javadoc - * for details and examples. + *

Resolving ${...} placeholders in {@code } and {@code @Value} annotations

+ * + *

In order to resolve ${...} placeholders in {@code } definitions or {@code @Value} + * annotations using properties from a {@code PropertySource}, you must ensure that an + * appropriate embedded value resolver is registered in the {@code BeanFactory} + * used by the {@code ApplicationContext}. This happens automatically when using + * {@code } in XML. When using {@code @Configuration} classes + * this can be achieved by explicitly registering a {@code PropertySourcesPlaceholderConfigurer} + * via a {@code static} {@code @Bean} method. Note, however, that explicit registration + * of a {@code PropertySourcesPlaceholderConfigurer} via a {@code static} {@code @Bean} + * method is typically only required if you need to customize configuration such as the + * placeholder syntax, etc. See the "Working with externalized values" section of + * {@link Configuration @Configuration}'s javadocs and "a note on + * BeanFactoryPostProcessor-returning {@code @Bean} methods" of {@link Bean @Bean}'s + * javadocs for details and examples. * *

Resolving ${...} placeholders within {@code @PropertySource} resource locations

* - * Any ${...} placeholders present in a {@code @PropertySource} {@linkplain #value() + *

Any ${...} placeholders present in a {@code @PropertySource} {@linkplain #value() * resource location} will be resolved against the set of property sources already * registered against the environment. For example: * @@ -92,7 +97,7 @@ * } * } * - * Assuming that "my.placeholder" is present in one of the property sources already + *

Assuming that "my.placeholder" is present in one of the property sources already * registered, e.g. system properties or environment variables, the placeholder will * be resolved to the corresponding value. If not, then "default/path" will be used as a * default. Expressing a default value (delimited by colon ":") is optional. If no @@ -101,10 +106,10 @@ * *

A note on property overriding with @PropertySource

* - * In cases where a given property key exists in more than one {@code .properties} + *

In cases where a given property key exists in more than one {@code .properties} * file, the last {@code @PropertySource} annotation processed will 'win' and override. * - * For example, given two properties files {@code a.properties} and + *

For example, given two properties files {@code a.properties} and * {@code b.properties}, consider the following two configuration classes * that reference them with {@code @PropertySource} annotations: * @@ -118,7 +123,7 @@ * public class ConfigB { } * * - * The override ordering depends on the order in which these classes are registered + *

The override ordering depends on the order in which these classes are registered * with the application context. * * 

@@ -128,12 +133,12 @@
  * ctx.refresh();
  *
* - * In the scenario above, the properties in {@code b.properties} will override any + *

In the scenario above, the properties in {@code b.properties} will override any * duplicates that exist in {@code a.properties}, because {@code ConfigB} was registered * last. * *

In certain situations, it may not be possible or practical to tightly control - * property source ordering when using {@code @ProperySource} annotations. For example, + * property source ordering when using {@code @PropertySource} annotations. For example, * if the {@code @Configuration} classes above were registered via component-scanning, * the ordering is difficult to predict. In such cases - and if overriding is important - * it is recommended that the user fall back to using the programmatic PropertySource API. @@ -150,6 +155,7 @@ * @author Chris Beams * @author Juergen Hoeller * @author Phillip Webb + * @author Sam Brannen * @since 3.1 * @see PropertySources * @see Configuration @@ -164,8 +170,11 @@ public @interface PropertySource { /** - * Indicate the name of this property source. If omitted, a name will - * be generated based on the description of the underlying resource. + * Indicate the name of this property source. If omitted, the {@link #factory()} + * will generate a name based on the underlying resource (in the case of + * {@link org.springframework.core.io.support.DefaultPropertySourceFactory}: + * derived from the resource description through a corresponding name-less + * {@link org.springframework.core.io.support.ResourcePropertySource} constructor). * @see org.springframework.core.env.PropertySource#getName() * @see org.springframework.core.io.Resource#getDescription() */ diff --git a/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceFactory.java b/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceFactory.java index 925c9a3513b6..21cba7596cb8 100644 --- a/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceFactory.java +++ b/spring-core/src/main/java/org/springframework/core/io/support/PropertySourceFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2020 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. @@ -33,6 +33,8 @@ public interface PropertySourceFactory { /** * Create a {@link PropertySource} that wraps the given resource. * @param name the name of the property source + * (can be {@code null} in which case the factory implementation + * will have to generate a name based on the given resource) * @param resource the resource (potentially encoded) to wrap * @return the new {@link PropertySource} (never {@code null}) * @throws IOException if resource resolution failed diff --git a/spring-core/src/main/java/org/springframework/util/ClassUtils.java b/spring-core/src/main/java/org/springframework/util/ClassUtils.java index 5a0310eac102..799ad7f5299c 100644 --- a/spring-core/src/main/java/org/springframework/util/ClassUtils.java +++ b/spring-core/src/main/java/org/springframework/util/ClassUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -506,7 +506,7 @@ public static Class resolvePrimitiveIfNecessary(Class clazz) { * @param lhsType the target type * @param rhsType the value type that should be assigned to the target type * @return if the target type is assignable from the value type - * @see TypeUtils#isAssignable + * @see TypeUtils#isAssignable(java.lang.reflect.Type, java.lang.reflect.Type) */ public static boolean isAssignable(Class lhsType, Class rhsType) { Assert.notNull(lhsType, "Left-hand side type must not be null"); @@ -516,17 +516,12 @@ public static boolean isAssignable(Class lhsType, Class rhsType) { } if (lhsType.isPrimitive()) { Class resolvedPrimitive = primitiveWrapperTypeMap.get(rhsType); - if (lhsType == resolvedPrimitive) { - return true; - } + return (lhsType == resolvedPrimitive); } else { Class resolvedWrapper = primitiveTypeToWrapperMap.get(rhsType); - if (resolvedWrapper != null && lhsType.isAssignableFrom(resolvedWrapper)) { - return true; - } + return (resolvedWrapper != null && lhsType.isAssignableFrom(resolvedWrapper)); } - return false; } /** @@ -1038,7 +1033,7 @@ public static String getQualifiedMethodName(Method method, @Nullable Class cl * @param clazz the clazz to analyze * @param paramTypes the parameter types of the method * @return whether the class has a corresponding constructor - * @see Class#getMethod + * @see Class#getConstructor */ public static boolean hasConstructor(Class clazz, Class... paramTypes) { return (getConstructorIfAvailable(clazz, paramTypes) != null); diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceTransactionManager.java b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceTransactionManager.java index 447198f00c6e..0ce9f4125b9e 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceTransactionManager.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceTransactionManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2020 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. @@ -129,7 +129,7 @@ public DataSourceTransactionManager() { /** * Create a new DataSourceTransactionManager instance. - * @param dataSource JDBC DataSource to manage transactions for + * @param dataSource the JDBC DataSource to manage transactions for */ public DataSourceTransactionManager(DataSource dataSource) { this(); @@ -137,6 +137,7 @@ public DataSourceTransactionManager(DataSource dataSource) { afterPropertiesSet(); } + /** * Set the JDBC DataSource that this instance should manage transactions for. *

This will typically be a locally defined DataSource, for example an @@ -408,13 +409,9 @@ protected void prepareTransactionalConnection(Connection con, TransactionDefinit throws SQLException { if (isEnforceReadOnly() && definition.isReadOnly()) { - Statement stmt = con.createStatement(); - try { + try (Statement stmt = con.createStatement()) { stmt.executeUpdate("SET TRANSACTION READ ONLY"); } - finally { - stmt.close(); - } } }