Skip to content

Commit c685525

Browse files
committed
Revise TargetSource implementations for proper nullability
Includes hashCode optimization in AbstractBeanFactoryBasedTargetSource. Includes ThreadLocal naming fix in ThreadLocalTargetSource. Closes gh-30576 Closes gh-30581
1 parent b738a20 commit c685525

File tree

7 files changed

+35
-24
lines changed

7 files changed

+35
-24
lines changed

spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/target/AbstractBeanFactoryBasedTargetSourceCreator.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
3535
import org.springframework.beans.factory.support.GenericBeanDefinition;
3636
import org.springframework.lang.Nullable;
37+
import org.springframework.util.Assert;
3738

3839
/**
3940
* Convenient superclass for
@@ -82,6 +83,11 @@ protected final BeanFactory getBeanFactory() {
8283
return this.beanFactory;
8384
}
8485

86+
private ConfigurableBeanFactory getConfigurableBeanFactory() {
87+
Assert.state(this.beanFactory != null, "BeanFactory not set");
88+
return this.beanFactory;
89+
}
90+
8591

8692
//---------------------------------------------------------------------
8793
// Implementation of the TargetSourceCreator interface
@@ -105,7 +111,7 @@ public final TargetSource getTargetSource(Class<?> beanClass, String beanName) {
105111
// We need to override just this bean definition, as it may reference other beans
106112
// and we're happy to take the parent's definition for those.
107113
// Always use prototype scope if demanded.
108-
BeanDefinition bd = this.beanFactory.getMergedBeanDefinition(beanName);
114+
BeanDefinition bd = getConfigurableBeanFactory().getMergedBeanDefinition(beanName);
109115
GenericBeanDefinition bdCopy = new GenericBeanDefinition(bd);
110116
if (isPrototypeBased()) {
111117
bdCopy.setScope(BeanDefinition.SCOPE_PROTOTYPE);
@@ -127,7 +133,7 @@ public final TargetSource getTargetSource(Class<?> beanClass, String beanName) {
127133
protected DefaultListableBeanFactory getInternalBeanFactoryForBean(String beanName) {
128134
synchronized (this.internalBeanFactories) {
129135
return this.internalBeanFactories.computeIfAbsent(beanName,
130-
name -> buildInternalBeanFactory(this.beanFactory));
136+
name -> buildInternalBeanFactory(getConfigurableBeanFactory()));
131137
}
132138
}
133139

spring-aop/src/main/java/org/springframework/aop/target/AbstractBeanFactoryBasedTargetSource.java

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.springframework.beans.factory.BeanFactory;
2626
import org.springframework.beans.factory.BeanFactoryAware;
2727
import org.springframework.lang.Nullable;
28+
import org.springframework.util.Assert;
2829
import org.springframework.util.ObjectUtils;
2930

3031
/**
@@ -58,16 +59,18 @@ public abstract class AbstractBeanFactoryBasedTargetSource implements TargetSour
5859
protected final transient Log logger = LogFactory.getLog(getClass());
5960

6061
/** Name of the target bean we will create on each invocation. */
62+
@Nullable
6163
private String targetBeanName;
6264

6365
/** Class of the target. */
66+
@Nullable
6467
private volatile Class<?> targetClass;
6568

6669
/**
6770
* BeanFactory that owns this TargetSource. We need to hold onto this
6871
* reference so that we can create new prototype instances as necessary.
6972
*/
70-
@SuppressWarnings("serial")
73+
@Nullable
7174
private BeanFactory beanFactory;
7275

7376

@@ -88,6 +91,7 @@ public void setTargetBeanName(String targetBeanName) {
8891
* Return the name of the target bean in the factory.
8992
*/
9093
public String getTargetBeanName() {
94+
Assert.state(this.targetBeanName != null, "Target bean name not set");
9195
return this.targetBeanName;
9296
}
9397

@@ -117,11 +121,13 @@ public void setBeanFactory(BeanFactory beanFactory) {
117121
* Return the owning BeanFactory.
118122
*/
119123
public BeanFactory getBeanFactory() {
124+
Assert.state(this.beanFactory != null, "BeanFactory not set");
120125
return this.beanFactory;
121126
}
122127

123128

124129
@Override
130+
@Nullable
125131
public Class<?> getTargetClass() {
126132
Class<?> targetClass = this.targetClass;
127133
if (targetClass != null) {
@@ -130,7 +136,7 @@ public Class<?> getTargetClass() {
130136
synchronized (this) {
131137
// Full check within synchronization, entering the BeanFactory interaction algorithm only once...
132138
targetClass = this.targetClass;
133-
if (targetClass == null && this.beanFactory != null) {
139+
if (targetClass == null && this.beanFactory != null && this.targetBeanName != null) {
134140
// Determine type of the target bean.
135141
targetClass = this.beanFactory.getType(this.targetBeanName);
136142
if (targetClass == null) {
@@ -184,18 +190,16 @@ public boolean equals(@Nullable Object other) {
184190

185191
@Override
186192
public int hashCode() {
187-
int hashCode = getClass().hashCode();
188-
hashCode = 13 * hashCode + ObjectUtils.nullSafeHashCode(this.beanFactory);
189-
hashCode = 13 * hashCode + ObjectUtils.nullSafeHashCode(this.targetBeanName);
190-
return hashCode;
193+
return getClass().hashCode() * 13 + ObjectUtils.nullSafeHashCode(this.targetBeanName);
191194
}
192195

193196
@Override
194197
public String toString() {
195198
StringBuilder sb = new StringBuilder(getClass().getSimpleName());
196199
sb.append(" for target bean '").append(this.targetBeanName).append('\'');
197-
if (this.targetClass != null) {
198-
sb.append(" of type [").append(this.targetClass.getName()).append(']');
200+
Class<?> targetClass = this.targetClass;
201+
if (targetClass != null) {
202+
sb.append(" of type [").append(targetClass.getName()).append(']');
199203
}
200204
return sb.toString();
201205
}

spring-aop/src/main/java/org/springframework/aop/target/AbstractLazyCreationTargetSource.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2023 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.
@@ -46,6 +46,7 @@ public abstract class AbstractLazyCreationTargetSource implements TargetSource {
4646
protected final Log logger = LogFactory.getLog(getClass());
4747

4848
/** The lazily initialized target object. */
49+
@Nullable
4950
private Object lazyTarget;
5051

5152

spring-aop/src/main/java/org/springframework/aop/target/EmptyTargetSource.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ public static EmptyTargetSource forClass(@Nullable Class<?> targetClass, boolean
7070
// Instance implementation
7171
//---------------------------------------------------------------------
7272

73+
@Nullable
7374
private final Class<?> targetClass;
7475

7576
private final boolean isStatic;

spring-aop/src/main/java/org/springframework/aop/target/HotSwappableTargetSource.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,11 @@ public synchronized Object swap(Object newTarget) throws IllegalArgumentExceptio
9797

9898

9999
/**
100-
* Two HotSwappableTargetSources are equal if the current target
101-
* objects are equal.
100+
* Two HotSwappableTargetSources are equal if the current target objects are equal.
102101
*/
103102
@Override
104-
public boolean equals(@Nullable Object obj) {
105-
return (this == obj || (obj instanceof HotSwappableTargetSource that &&
103+
public boolean equals(@Nullable Object other) {
104+
return (this == other || (other instanceof HotSwappableTargetSource that &&
106105
this.target.equals(that.target)));
107106
}
108107

spring-aop/src/main/java/org/springframework/aop/target/SingletonTargetSource.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,8 @@ public boolean isStatic() {
8484
*/
8585
@Override
8686
public boolean equals(@Nullable Object other) {
87-
if (this == other) {
88-
return true;
89-
}
90-
if (!(other instanceof SingletonTargetSource otherTargetSource)) {
91-
return false;
92-
}
93-
return this.target.equals(otherTargetSource.target);
87+
return (this == other || (other instanceof SingletonTargetSource that &&
88+
this.target.equals(that.target)));
9489
}
9590

9691
/**

spring-aop/src/main/java/org/springframework/aop/target/ThreadLocalTargetSource.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-2023 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.
@@ -58,7 +58,12 @@ public class ThreadLocalTargetSource extends AbstractPrototypeBasedTargetSource
5858
* is meant to be per thread per instance of the ThreadLocalTargetSource class.
5959
*/
6060
private final ThreadLocal<Object> targetInThread =
61-
new NamedThreadLocal<>("Thread-local instance of bean '" + getTargetBeanName() + "'");
61+
new NamedThreadLocal<>("Thread-local instance of bean") {
62+
@Override
63+
public String toString() {
64+
return super.toString() + " '" + getTargetBeanName() + "'";
65+
}
66+
};
6267

6368
/**
6469
* Set of managed targets, enabling us to keep track of the targets we've created.

0 commit comments

Comments
 (0)