Skip to content

Commit 2fa25b5

Browse files
bclozelbclozel
committed
Fix caching operations in CachingMetadataReaderFactory
gh-33616 refactored `CachingMetadataReaderFactory` and broke the behavior as it bypassed the cache for `getMetadataReader(String className)` operations. This commit restores the original behavior. Fixes gh-35112
1 parent b3a5473 commit 2fa25b5

File tree

7 files changed

+161
-183
lines changed

7 files changed

+161
-183
lines changed

‎spring-core/src/main/java/org/springframework/core/type/classreading/AbstractMetadataReaderFactory.java

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Copyright 2002-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.core.type.classreading;
18+
19+
import java.io.FileNotFoundException;
20+
import java.io.IOException;
21+
22+
import org.jspecify.annotations.Nullable;
23+
24+
import org.springframework.core.io.DefaultResourceLoader;
25+
import org.springframework.core.io.Resource;
26+
import org.springframework.core.io.ResourceLoader;
27+
import org.springframework.util.ClassUtils;
28+
29+
abstract class AbstractMetadataReaderFactory implements MetadataReaderFactory {
30+
31+
private final ResourceLoader resourceLoader;
32+
33+
34+
public AbstractMetadataReaderFactory(@Nullable ResourceLoader resourceLoader) {
35+
this.resourceLoader = (resourceLoader != null ? resourceLoader : new DefaultResourceLoader());
36+
}
37+
38+
public AbstractMetadataReaderFactory(@Nullable ClassLoader classLoader) {
39+
this.resourceLoader =
40+
(classLoader != null ? new DefaultResourceLoader(classLoader) : new DefaultResourceLoader());
41+
}
42+
43+
public AbstractMetadataReaderFactory() {
44+
this.resourceLoader = new DefaultResourceLoader();
45+
}
46+
47+
/**
48+
* Return the ResourceLoader that this MetadataReaderFactory has been
49+
* constructed with.
50+
*/
51+
@Override
52+
public ResourceLoader getResourceLoader() {
53+
return this.resourceLoader;
54+
}
55+
56+
@Override
57+
public MetadataReader getMetadataReader(String className) throws IOException {
58+
try {
59+
String resourcePath = ResourceLoader.CLASSPATH_URL_PREFIX +
60+
ClassUtils.convertClassNameToResourcePath(className) + ClassUtils.CLASS_FILE_SUFFIX;
61+
Resource resource = this.resourceLoader.getResource(resourcePath);
62+
return getMetadataReader(resource);
63+
}
64+
catch (FileNotFoundException ex) {
65+
// Maybe an inner class name using the dot name syntax? Need to use the dollar syntax here...
66+
// ClassUtils.forName has an equivalent check for resolution into Class references later on.
67+
int lastDotIndex = className.lastIndexOf('.');
68+
if (lastDotIndex != -1) {
69+
String innerClassName =
70+
className.substring(0, lastDotIndex) + '$' + className.substring(lastDotIndex + 1);
71+
String innerClassResourcePath = ResourceLoader.CLASSPATH_URL_PREFIX +
72+
ClassUtils.convertClassNameToResourcePath(innerClassName) + ClassUtils.CLASS_FILE_SUFFIX;
73+
Resource innerClassResource = this.resourceLoader.getResource(innerClassResourcePath);
74+
if (innerClassResource.exists()) {
75+
return getMetadataReader(innerClassResource);
76+
}
77+
}
78+
throw ex;
79+
}
80+
}
81+
82+
}

‎spring-core/src/main/java/org/springframework/core/type/classreading/CachingMetadataReaderFactory.java

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,10 @@
3434
*
3535
* @author Juergen Hoeller
3636
* @author Costin Leau
37+
* @author Brian Clozel
3738
* @since 2.5
3839
*/
39-
public class CachingMetadataReaderFactory implements MetadataReaderFactory {
40+
public class CachingMetadataReaderFactory extends AbstractMetadataReaderFactory {
4041

4142
/** Default maximum number of entries for a local MetadataReader cache: 256. */
4243
public static final int DEFAULT_CACHE_LIMIT = 256;
@@ -52,8 +53,7 @@ public class CachingMetadataReaderFactory implements MetadataReaderFactory {
5253
* using a local resource cache.
5354
*/
5455
public CachingMetadataReaderFactory() {
55-
this.delegate = MetadataReaderFactory.create((ClassLoader) null);
56-
setCacheLimit(DEFAULT_CACHE_LIMIT);
56+
this(MetadataReaderFactory.create((ClassLoader) null));
5757
}
5858

5959
/**
@@ -62,8 +62,7 @@ public CachingMetadataReaderFactory() {
6262
* @param classLoader the ClassLoader to use
6363
*/
6464
public CachingMetadataReaderFactory(@Nullable ClassLoader classLoader) {
65-
this.delegate = MetadataReaderFactory.create(classLoader);
66-
setCacheLimit(DEFAULT_CACHE_LIMIT);
65+
this(MetadataReaderFactory.create(classLoader));
6766
}
6867

6968
/**
@@ -74,15 +73,21 @@ public CachingMetadataReaderFactory(@Nullable ClassLoader classLoader) {
7473
* @see DefaultResourceLoader#getResourceCache
7574
*/
7675
public CachingMetadataReaderFactory(@Nullable ResourceLoader resourceLoader) {
77-
this.delegate = MetadataReaderFactory.create(resourceLoader);
78-
if (resourceLoader instanceof DefaultResourceLoader defaultResourceLoader) {
76+
this(MetadataReaderFactory.create(resourceLoader));
77+
}
78+
79+
CachingMetadataReaderFactory(MetadataReaderFactory delegate) {
80+
super(delegate.getResourceLoader());
81+
this.delegate = delegate;
82+
if (getResourceLoader() instanceof DefaultResourceLoader defaultResourceLoader) {
7983
this.metadataReaderCache = defaultResourceLoader.getResourceCache(MetadataReader.class);
8084
}
8185
else {
8286
setCacheLimit(DEFAULT_CACHE_LIMIT);
8387
}
8488
}
8589

90+
8691
/**
8792
* Specify the maximum number of entries for the MetadataReader cache.
8893
* <p>Default is 256 for a local cache, whereas a shared cache is
@@ -113,11 +118,6 @@ public int getCacheLimit() {
113118
}
114119
}
115120

116-
@Override
117-
public MetadataReader getMetadataReader(String className) throws IOException {
118-
return this.delegate.getMetadataReader(className);
119-
}
120-
121121
@Override
122122
public MetadataReader getMetadataReader(Resource resource) throws IOException {
123123
if (this.metadataReaderCache instanceof ConcurrentMap) {

‎spring-core/src/main/java/org/springframework/core/type/classreading/MetadataReaderFactory.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,13 @@ public interface MetadataReaderFactory {
5353
*/
5454
MetadataReader getMetadataReader(Resource resource) throws IOException;
5555

56+
/**
57+
* Return the ResourceLoader that this MetadataReaderFactory has been
58+
* constructed with.
59+
* @since 7.0
60+
*/
61+
ResourceLoader getResourceLoader();
62+
5663
/**
5764
* Create a default {@link MetadataReaderFactory} implementation that's suitable
5865
* for the current JVM.

‎spring-core/src/main/java/org/springframework/core/type/classreading/SimpleMetadataReaderFactory.java

Lines changed: 5 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,12 @@
1616

1717
package org.springframework.core.type.classreading;
1818

19-
import java.io.FileNotFoundException;
2019
import java.io.IOException;
2120

2221
import org.jspecify.annotations.Nullable;
2322

24-
import org.springframework.core.io.DefaultResourceLoader;
2523
import org.springframework.core.io.Resource;
2624
import org.springframework.core.io.ResourceLoader;
27-
import org.springframework.util.ClassUtils;
2825

2926
/**
3027
* Simple implementation of the {@link MetadataReaderFactory} interface,
@@ -33,16 +30,14 @@
3330
* @author Juergen Hoeller
3431
* @since 2.5
3532
*/
36-
public class SimpleMetadataReaderFactory implements MetadataReaderFactory {
37-
38-
private final ResourceLoader resourceLoader;
33+
public class SimpleMetadataReaderFactory extends AbstractMetadataReaderFactory {
3934

4035

4136
/**
4237
* Create a new SimpleMetadataReaderFactory for the default class loader.
4338
*/
4439
public SimpleMetadataReaderFactory() {
45-
this.resourceLoader = new DefaultResourceLoader();
40+
super();
4641
}
4742

4843
/**
@@ -51,57 +46,20 @@ public SimpleMetadataReaderFactory() {
5146
* (also determines the ClassLoader to use)
5247
*/
5348
public SimpleMetadataReaderFactory(@Nullable ResourceLoader resourceLoader) {
54-
this.resourceLoader = (resourceLoader != null ? resourceLoader : new DefaultResourceLoader());
49+
super(resourceLoader);
5550
}
5651

5752
/**
5853
* Create a new SimpleMetadataReaderFactory for the given class loader.
5954
* @param classLoader the ClassLoader to use
6055
*/
6156
public SimpleMetadataReaderFactory(@Nullable ClassLoader classLoader) {
62-
this.resourceLoader =
63-
(classLoader != null ? new DefaultResourceLoader(classLoader) : new DefaultResourceLoader());
64-
}
65-
66-
67-
/**
68-
* Return the ResourceLoader that this MetadataReaderFactory has been
69-
* constructed with.
70-
*/
71-
public final ResourceLoader getResourceLoader() {
72-
return this.resourceLoader;
73-
}
74-
75-
76-
@Override
77-
public MetadataReader getMetadataReader(String className) throws IOException {
78-
try {
79-
String resourcePath = ResourceLoader.CLASSPATH_URL_PREFIX +
80-
ClassUtils.convertClassNameToResourcePath(className) + ClassUtils.CLASS_FILE_SUFFIX;
81-
Resource resource = this.resourceLoader.getResource(resourcePath);
82-
return getMetadataReader(resource);
83-
}
84-
catch (FileNotFoundException ex) {
85-
// Maybe an inner class name using the dot name syntax? Need to use the dollar syntax here...
86-
// ClassUtils.forName has an equivalent check for resolution into Class references later on.
87-
int lastDotIndex = className.lastIndexOf('.');
88-
if (lastDotIndex != -1) {
89-
String innerClassName =
90-
className.substring(0, lastDotIndex) + '$' + className.substring(lastDotIndex + 1);
91-
String innerClassResourcePath = ResourceLoader.CLASSPATH_URL_PREFIX +
92-
ClassUtils.convertClassNameToResourcePath(innerClassName) + ClassUtils.CLASS_FILE_SUFFIX;
93-
Resource innerClassResource = this.resourceLoader.getResource(innerClassResourcePath);
94-
if (innerClassResource.exists()) {
95-
return getMetadataReader(innerClassResource);
96-
}
97-
}
98-
throw ex;
99-
}
57+
super(classLoader);
10058
}
10159

10260
@Override
10361
public MetadataReader getMetadataReader(Resource resource) throws IOException {
104-
return new SimpleMetadataReader(resource, this.resourceLoader.getClassLoader());
62+
return new SimpleMetadataReader(resource, getResourceLoader().getClassLoader());
10563
}
10664

10765
}

‎spring-core/src/main/java24/org/springframework/core/type/classreading/ClassFileMetadataReaderFactory.java

Lines changed: 5 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,12 @@
1616

1717
package org.springframework.core.type.classreading;
1818

19-
import java.io.FileNotFoundException;
2019
import java.io.IOException;
2120

2221
import org.jspecify.annotations.Nullable;
2322

24-
import org.springframework.core.io.DefaultResourceLoader;
2523
import org.springframework.core.io.Resource;
2624
import org.springframework.core.io.ResourceLoader;
27-
import org.springframework.util.ClassUtils;
2825

2926
/**
3027
* Implementation of the {@link MetadataReaderFactory} interface,
@@ -33,17 +30,14 @@
3330
* @author Brian Clozel
3431
* @since 7.0
3532
*/
36-
public class ClassFileMetadataReaderFactory implements MetadataReaderFactory {
37-
38-
39-
private final ResourceLoader resourceLoader;
33+
public class ClassFileMetadataReaderFactory extends AbstractMetadataReaderFactory {
4034

4135

4236
/**
4337
* Create a new ClassFileMetadataReaderFactory for the default class loader.
4438
*/
4539
public ClassFileMetadataReaderFactory() {
46-
this.resourceLoader = new DefaultResourceLoader();
40+
super();
4741
}
4842

4943
/**
@@ -52,54 +46,19 @@ public ClassFileMetadataReaderFactory() {
5246
* (also determines the ClassLoader to use)
5347
*/
5448
public ClassFileMetadataReaderFactory(@Nullable ResourceLoader resourceLoader) {
55-
this.resourceLoader = (resourceLoader != null ? resourceLoader : new DefaultResourceLoader());
49+
super(resourceLoader);
5650
}
5751

5852
/**
5953
* Create a new ClassFileMetadataReaderFactory for the given class loader.
6054
* @param classLoader the ClassLoader to use
6155
*/
6256
public ClassFileMetadataReaderFactory(@Nullable ClassLoader classLoader) {
63-
this.resourceLoader =
64-
(classLoader != null ? new DefaultResourceLoader(classLoader) : new DefaultResourceLoader());
65-
}
66-
67-
/**
68-
* Return the ResourceLoader that this MetadataReaderFactory has been
69-
* constructed with.
70-
*/
71-
public final ResourceLoader getResourceLoader() {
72-
return this.resourceLoader;
73-
}
74-
75-
@Override
76-
public MetadataReader getMetadataReader(String className) throws IOException {
77-
try {
78-
String resourcePath = ResourceLoader.CLASSPATH_URL_PREFIX +
79-
ClassUtils.convertClassNameToResourcePath(className) + ClassUtils.CLASS_FILE_SUFFIX;
80-
Resource resource = this.resourceLoader.getResource(resourcePath);
81-
return getMetadataReader(resource);
82-
}
83-
catch (FileNotFoundException ex) {
84-
// Maybe an inner class name using the dot name syntax? Need to use the dollar syntax here...
85-
// ClassUtils.forName has an equivalent check for resolution into Class references later on.
86-
int lastDotIndex = className.lastIndexOf('.');
87-
if (lastDotIndex != -1) {
88-
String innerClassName =
89-
className.substring(0, lastDotIndex) + '$' + className.substring(lastDotIndex + 1);
90-
String innerClassResourcePath = ResourceLoader.CLASSPATH_URL_PREFIX +
91-
ClassUtils.convertClassNameToResourcePath(innerClassName) + ClassUtils.CLASS_FILE_SUFFIX;
92-
Resource innerClassResource = this.resourceLoader.getResource(innerClassResourcePath);
93-
if (innerClassResource.exists()) {
94-
return getMetadataReader(innerClassResource);
95-
}
96-
}
97-
throw ex;
98-
}
57+
super(classLoader);
9958
}
10059

10160
@Override
10261
public MetadataReader getMetadataReader(Resource resource) throws IOException {
103-
return new ClassFileMetadataReader(resource, this.resourceLoader.getClassLoader());
62+
return new ClassFileMetadataReader(resource, getResourceLoader().getClassLoader());
10463
}
10564
}

0 commit comments

Comments
 (0)