Skip to content

Commit 46eba3d

Browse files
jhoellerjhoeller
committed
Nullability fine-tuning around declaration inconsistencies
Issue: SPR-15720 Issue: SPR-15792
1 parent 68e6b14 commit 46eba3d

File tree

186 files changed

+986
-619
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

186 files changed

+986
-619
lines changed

‎spring-aop/src/main/java/org/springframework/aop/Advisor.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,18 @@
3131
* implemented using interception.
3232
*
3333
* @author Rod Johnson
34+
* @author Juergen Hoeller
3435
*/
3536
public interface Advisor {
3637

38+
/**
39+
* Common placeholder for an empty {@code Advice} to be returned from
40+
* {@link #getAdvice()} if no proper advice has been configured (yet).
41+
* @since 5.0
42+
*/
43+
Advice EMPTY_ADVICE = new Advice() {};
44+
45+
3746
/**
3847
* Return the advice part of this aspect. An advice may be an
3948
* interceptor, a before advice, a throws advice, etc.

‎spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJPointcutAdvisor.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.springframework.core.Ordered;
2424
import org.springframework.lang.Nullable;
2525
import org.springframework.util.Assert;
26-
import org.springframework.util.ObjectUtils;
2726

2827
/**
2928
* AspectJPointcutAdvisor that adapts an {@link AbstractAspectJAdvice}
@@ -53,11 +52,11 @@ public AspectJPointcutAdvisor(AbstractAspectJAdvice advice) {
5352
this.pointcut = advice.buildSafePointcut();
5453
}
5554

55+
5656
public void setOrder(int order) {
5757
this.order = order;
5858
}
5959

60-
6160
@Override
6261
public boolean isPerInstance() {
6362
return true;
@@ -93,12 +92,12 @@ public boolean equals(Object other) {
9392
return false;
9493
}
9594
AspectJPointcutAdvisor otherAdvisor = (AspectJPointcutAdvisor) other;
96-
return (ObjectUtils.nullSafeEquals(this.advice, otherAdvisor.advice));
95+
return this.advice.equals(otherAdvisor.advice);
9796
}
9897

9998
@Override
10099
public int hashCode() {
101-
return AspectJPointcutAdvisor.class.hashCode();
100+
return AspectJPointcutAdvisor.class.hashCode() * 29 + this.advice.hashCode();
102101
}
103102

104103
}

‎spring-aop/src/main/java/org/springframework/aop/config/AdvisorComponentDefinition.java

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,11 @@ public class AdvisorComponentDefinition extends AbstractComponentDefinition {
3939

4040
private final BeanDefinition advisorDefinition;
4141

42-
private String description;
42+
private final String description;
4343

44-
private BeanReference[] beanReferences;
44+
private final BeanReference[] beanReferences;
4545

46-
private BeanDefinition[] beanDefinitions;
46+
private final BeanDefinition[] beanDefinitions;
4747

4848

4949
public AdvisorComponentDefinition(String advisorBeanName, BeanDefinition advisorDefinition) {
@@ -57,11 +57,7 @@ public AdvisorComponentDefinition(
5757
Assert.notNull(advisorDefinition, "'advisorDefinition' must not be null");
5858
this.advisorBeanName = advisorBeanName;
5959
this.advisorDefinition = advisorDefinition;
60-
unwrapDefinitions(advisorDefinition, pointcutDefinition);
61-
}
62-
6360

64-
private void unwrapDefinitions(BeanDefinition advisorDefinition, @Nullable BeanDefinition pointcutDefinition) {
6561
MutablePropertyValues pvs = advisorDefinition.getPropertyValues();
6662
BeanReference adviceReference = (BeanReference) pvs.get("adviceBeanName");
6763
Assert.state(adviceReference != null, "Missing 'adviceBeanName' property");

‎spring-aop/src/main/java/org/springframework/aop/config/SimpleBeanFactoryAwareAspectInstanceFactory.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2012 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -21,6 +21,8 @@
2121
import org.springframework.beans.factory.BeanFactoryAware;
2222
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
2323
import org.springframework.core.Ordered;
24+
import org.springframework.lang.Nullable;
25+
import org.springframework.util.Assert;
2426
import org.springframework.util.ClassUtils;
2527
import org.springframework.util.StringUtils;
2628

@@ -34,8 +36,10 @@
3436
*/
3537
public class SimpleBeanFactoryAwareAspectInstanceFactory implements AspectInstanceFactory, BeanFactoryAware {
3638

39+
@Nullable
3740
private String aspectBeanName;
3841

42+
@Nullable
3943
private BeanFactory beanFactory;
4044

4145

@@ -50,9 +54,7 @@ public void setAspectBeanName(String aspectBeanName) {
5054
@Override
5155
public void setBeanFactory(BeanFactory beanFactory) {
5256
this.beanFactory = beanFactory;
53-
if (!StringUtils.hasText(this.aspectBeanName)) {
54-
throw new IllegalArgumentException("'aspectBeanName' is required");
55-
}
57+
Assert.notNull(this.aspectBeanName, "'aspectBeanName' is required");
5658
}
5759

5860

@@ -62,6 +64,8 @@ public void setBeanFactory(BeanFactory beanFactory) {
6264
*/
6365
@Override
6466
public Object getAspectInstance() {
67+
Assert.state(this.beanFactory != null, "No BeanFactory set");
68+
Assert.state(this.aspectBeanName != null, "No 'aspectBeanName' set");
6569
return this.beanFactory.getBean(this.aspectBeanName);
6670
}
6771

@@ -77,7 +81,8 @@ public ClassLoader getAspectClassLoader() {
7781

7882
@Override
7983
public int getOrder() {
80-
if (this.beanFactory.isSingleton(this.aspectBeanName) &&
84+
if (this.beanFactory != null && this.aspectBeanName != null &&
85+
this.beanFactory.isSingleton(this.aspectBeanName) &&
8186
this.beanFactory.isTypeMatch(this.aspectBeanName, Ordered.class)) {
8287
return ((Ordered) this.beanFactory.getBean(this.aspectBeanName)).getOrder();
8388
}

‎spring-aop/src/main/java/org/springframework/aop/framework/AbstractAdvisingBeanPostProcessor.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -22,6 +22,7 @@
2222
import org.springframework.aop.Advisor;
2323
import org.springframework.aop.support.AopUtils;
2424
import org.springframework.beans.factory.config.BeanPostProcessor;
25+
import org.springframework.lang.Nullable;
2526

2627
/**
2728
* Base class for {@link BeanPostProcessor} implementations that apply a
@@ -33,6 +34,7 @@
3334
@SuppressWarnings("serial")
3435
public abstract class AbstractAdvisingBeanPostProcessor extends ProxyProcessorSupport implements BeanPostProcessor {
3536

37+
@Nullable
3638
protected Advisor advisor;
3739

3840
protected boolean beforeExistingAdvisors = false;
@@ -61,7 +63,7 @@ public Object postProcessBeforeInitialization(Object bean, String beanName) {
6163

6264
@Override
6365
public Object postProcessAfterInitialization(Object bean, String beanName) {
64-
if (bean instanceof AopInfrastructureBean) {
66+
if (bean instanceof AopInfrastructureBean || this.advisor == null) {
6567
// Ignore AOP infrastructure such as scoped proxies.
6668
return bean;
6769
}
@@ -125,6 +127,9 @@ protected boolean isEligible(Class<?> targetClass) {
125127
if (eligible != null) {
126128
return eligible;
127129
}
130+
if (this.advisor == null) {
131+
return false;
132+
}
128133
eligible = AopUtils.canApply(this.advisor, targetClass);
129134
this.eligibleBeans.put(targetClass, eligible);
130135
return eligible;

‎spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public class AdvisedSupport extends ProxyConfig implements Advised {
107107
* No-arg constructor for use as a JavaBean.
108108
*/
109109
public AdvisedSupport() {
110-
initMethodCache();
110+
this.methodCache = new ConcurrentHashMap<>(32);
111111
}
112112

113113
/**
@@ -119,13 +119,6 @@ public AdvisedSupport(Class<?>... interfaces) {
119119
setInterfaces(interfaces);
120120
}
121121

122-
/**
123-
* Initialize the method cache.
124-
*/
125-
private void initMethodCache() {
126-
this.methodCache = new ConcurrentHashMap<>(32);
127-
}
128-
129122

130123
/**
131124
* Set the given object as target.
@@ -558,7 +551,7 @@ private void readObject(ObjectInputStream ois) throws IOException, ClassNotFound
558551
ois.defaultReadObject();
559552

560553
// Initialize transient fields.
561-
initMethodCache();
554+
this.methodCache = new ConcurrentHashMap<>(32);
562555
}
563556

564557

‎spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractAdvisorAutoProxyCreator.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
2626
import org.springframework.core.annotation.AnnotationAwareOrderComparator;
2727
import org.springframework.lang.Nullable;
28+
import org.springframework.util.Assert;
2829

2930
/**
3031
* Generic auto proxy creator that builds AOP proxies for specific beans
@@ -48,6 +49,7 @@
4849
@SuppressWarnings("serial")
4950
public abstract class AbstractAdvisorAutoProxyCreator extends AbstractAutoProxyCreator {
5051

52+
@Nullable
5153
private BeanFactoryAdvisorRetrievalHelper advisorRetrievalHelper;
5254

5355

@@ -101,6 +103,7 @@ protected List<Advisor> findEligibleAdvisors(Class<?> beanClass, String beanName
101103
* @return the List of candidate Advisors
102104
*/
103105
protected List<Advisor> findCandidateAdvisors() {
106+
Assert.state(this.advisorRetrievalHelper != null, "No BeanFactoryAdvisorRetrievalHelper available");
104107
return this.advisorRetrievalHelper.findAdvisorBeans();
105108
}
106109

‎spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractAutoProxyCreator.java

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ public void setApplyCommonInterceptorsFirst(boolean applyCommonInterceptorsFirst
203203
}
204204

205205
@Override
206-
public void setBeanFactory(@Nullable BeanFactory beanFactory) {
206+
public void setBeanFactory(BeanFactory beanFactory) {
207207
this.beanFactory = beanFactory;
208208
}
209209

@@ -241,10 +241,10 @@ public Object getEarlyBeanReference(Object bean, String beanName) throws BeansEx
241241
}
242242

243243
@Override
244-
public Object postProcessBeforeInstantiation(Class<?> beanClass, @Nullable String beanName) throws BeansException {
244+
public Object postProcessBeforeInstantiation(Class<?> beanClass, String beanName) throws BeansException {
245245
Object cacheKey = getCacheKey(beanClass, beanName);
246246

247-
if (beanName == null || !this.targetSourcedBeans.contains(beanName)) {
247+
if (!StringUtils.hasLength(beanName) || !this.targetSourcedBeans.contains(beanName)) {
248248
if (this.advisedBeans.containsKey(cacheKey)) {
249249
return null;
250250
}
@@ -257,15 +257,15 @@ public Object postProcessBeforeInstantiation(Class<?> beanClass, @Nullable Strin
257257
// Create proxy here if we have a custom TargetSource.
258258
// Suppresses unnecessary default instantiation of the target bean:
259259
// The TargetSource will handle target instances in a custom fashion.
260-
if (beanName != null) {
261-
TargetSource targetSource = getCustomTargetSource(beanClass, beanName);
262-
if (targetSource != null) {
260+
TargetSource targetSource = getCustomTargetSource(beanClass, beanName);
261+
if (targetSource != null) {
262+
if (StringUtils.hasLength(beanName)) {
263263
this.targetSourcedBeans.add(beanName);
264-
Object[] specificInterceptors = getAdvicesAndAdvisorsForBean(beanClass, beanName, targetSource);
265-
Object proxy = createProxy(beanClass, beanName, specificInterceptors, targetSource);
266-
this.proxyTypes.put(cacheKey, proxy.getClass());
267-
return proxy;
268264
}
265+
Object[] specificInterceptors = getAdvicesAndAdvisorsForBean(beanClass, beanName, targetSource);
266+
Object proxy = createProxy(beanClass, beanName, specificInterceptors, targetSource);
267+
this.proxyTypes.put(cacheKey, proxy.getClass());
268+
return proxy;
269269
}
270270

271271
return null;
@@ -333,8 +333,8 @@ protected Object getCacheKey(Class<?> beanClass, @Nullable String beanName) {
333333
* @param cacheKey the cache key for metadata access
334334
* @return a proxy wrapping the bean, or the raw bean instance as-is
335335
*/
336-
protected Object wrapIfNecessary(Object bean, @Nullable String beanName, Object cacheKey) {
337-
if (beanName != null && this.targetSourcedBeans.contains(beanName)) {
336+
protected Object wrapIfNecessary(Object bean, String beanName, Object cacheKey) {
337+
if (StringUtils.hasLength(beanName) && this.targetSourcedBeans.contains(beanName)) {
338338
return bean;
339339
}
340340
if (Boolean.FALSE.equals(this.advisedBeans.get(cacheKey))) {
@@ -391,7 +391,7 @@ protected boolean isInfrastructureClass(Class<?> beanClass) {
391391
* @param beanName the name of the bean
392392
* @return whether to skip the given bean
393393
*/
394-
protected boolean shouldSkip(Class<?> beanClass, @Nullable String beanName) {
394+
protected boolean shouldSkip(Class<?> beanClass, String beanName) {
395395
return false;
396396
}
397397

@@ -582,8 +582,7 @@ protected void customizeProxyFactory(ProxyFactory proxyFactory) {
582582
* @see #PROXY_WITHOUT_ADDITIONAL_INTERCEPTORS
583583
*/
584584
@Nullable
585-
protected abstract Object[] getAdvicesAndAdvisorsForBean(Class<?> beanClass,
586-
@Nullable String beanName, @Nullable TargetSource customTargetSource)
587-
throws BeansException;
585+
protected abstract Object[] getAdvicesAndAdvisorsForBean(Class<?> beanClass, String beanName,
586+
@Nullable TargetSource customTargetSource) throws BeansException;
588587

589588
}

‎spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/BeanNameAutoProxyCreator.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@ public void setBeanNames(String... beanNames) {
7676
*/
7777
@Override
7878
@Nullable
79-
protected Object[] getAdvicesAndAdvisorsForBean(Class<?> beanClass, String beanName, @Nullable TargetSource targetSource) {
79+
protected Object[] getAdvicesAndAdvisorsForBean(
80+
Class<?> beanClass, String beanName, @Nullable TargetSource targetSource) {
81+
8082
if (this.beanNames != null) {
8183
for (String mappedName : this.beanNames) {
8284
if (FactoryBean.class.isAssignableFrom(beanClass)) {

‎spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/InfrastructureAdvisorAutoProxyCreator.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2012 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@
1818

1919
import org.springframework.beans.factory.config.BeanDefinition;
2020
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
21+
import org.springframework.lang.Nullable;
2122

2223
/**
2324
* Auto-proxy creator that considers infrastructure Advisor beans only,
@@ -29,6 +30,7 @@
2930
@SuppressWarnings("serial")
3031
public class InfrastructureAdvisorAutoProxyCreator extends AbstractAdvisorAutoProxyCreator {
3132

33+
@Nullable
3234
private ConfigurableListableBeanFactory beanFactory;
3335

3436

@@ -40,7 +42,7 @@ protected void initBeanFactory(ConfigurableListableBeanFactory beanFactory) {
4042

4143
@Override
4244
protected boolean isEligibleAdvisorBean(String beanName) {
43-
return (this.beanFactory.containsBeanDefinition(beanName) &&
45+
return (this.beanFactory != null && this.beanFactory.containsBeanDefinition(beanName) &&
4446
this.beanFactory.getBeanDefinition(beanName).getRole() == BeanDefinition.ROLE_INFRASTRUCTURE);
4547
}
4648

‎spring-aop/src/main/java/org/springframework/aop/support/AbstractExpressionPointcut.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ protected void onSetExpression(@Nullable String expression) throws IllegalArgume
8989
* Return this pointcut's expression.
9090
*/
9191
@Override
92+
@Nullable
9293
public String getExpression() {
9394
return this.expression;
9495
}

‎spring-aop/src/main/java/org/springframework/aop/support/AbstractGenericPointcutAdvisor.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2012 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,7 +19,8 @@
1919
import org.aopalliance.aop.Advice;
2020

2121
/**
22-
* Abstract generic PointcutAdvisor that allows for any Advice to be configured.
22+
* Abstract generic {@link org.springframework.aop.PointcutAdvisor}
23+
* that allows for any {@link Advice} to be configured.
2324
*
2425
* @author Juergen Hoeller
2526
* @since 2.0
@@ -29,7 +30,7 @@
2930
@SuppressWarnings("serial")
3031
public abstract class AbstractGenericPointcutAdvisor extends AbstractPointcutAdvisor {
3132

32-
private Advice advice;
33+
private Advice advice = EMPTY_ADVICE;
3334

3435

3536
/**

‎spring-aop/src/main/java/org/springframework/aop/support/DelegatingIntroductionInterceptor.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ public class DelegatingIntroductionInterceptor extends IntroductionInfoSupport
5757
* Object that actually implements the interfaces.
5858
* May be "this" if a subclass implements the introduced interfaces.
5959
*/
60+
@Nullable
6061
private Object delegate;
6162

6263

0 commit comments

Comments
 (0)