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

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

@@ -0,0 +1,82 @@
+/*
+ * Copyright 2002-present 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.core.type.classreading;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+
+import org.jspecify.annotations.Nullable;
+
+import org.springframework.core.io.DefaultResourceLoader;
+import org.springframework.core.io.Resource;
+import org.springframework.core.io.ResourceLoader;
+import org.springframework.util.ClassUtils;
+
+abstract class AbstractMetadataReaderFactory implements MetadataReaderFactory {
+
+	private final ResourceLoader resourceLoader;
+
+
+	public AbstractMetadataReaderFactory(@Nullable ResourceLoader resourceLoader) {
+		this.resourceLoader = (resourceLoader != null ? resourceLoader : new DefaultResourceLoader());
+	}
+
+	public AbstractMetadataReaderFactory(@Nullable ClassLoader classLoader) {
+		this.resourceLoader =
+				(classLoader != null ? new DefaultResourceLoader(classLoader) : new DefaultResourceLoader());
+	}
+
+	public AbstractMetadataReaderFactory() {
+		this.resourceLoader = new DefaultResourceLoader();
+	}
+
+	/**
+	 * Return the ResourceLoader that this MetadataReaderFactory has been
+	 * constructed with.
+	 */
+	@Override
+	public ResourceLoader getResourceLoader() {
+		return this.resourceLoader;
+	}
+
+	@Override
+	public MetadataReader getMetadataReader(String className) throws IOException {
+		try {
+			String resourcePath = ResourceLoader.CLASSPATH_URL_PREFIX +
+					ClassUtils.convertClassNameToResourcePath(className) + ClassUtils.CLASS_FILE_SUFFIX;
+			Resource resource = this.resourceLoader.getResource(resourcePath);
+			return getMetadataReader(resource);
+		}
+		catch (FileNotFoundException ex) {
+			// Maybe an inner class name using the dot name syntax? Need to use the dollar syntax here...
+			// ClassUtils.forName has an equivalent check for resolution into Class references later on.
+			int lastDotIndex = className.lastIndexOf('.');
+			if (lastDotIndex != -1) {
+				String innerClassName =
+						className.substring(0, lastDotIndex) + '$' + className.substring(lastDotIndex + 1);
+				String innerClassResourcePath = ResourceLoader.CLASSPATH_URL_PREFIX +
+						ClassUtils.convertClassNameToResourcePath(innerClassName) + ClassUtils.CLASS_FILE_SUFFIX;
+				Resource innerClassResource = this.resourceLoader.getResource(innerClassResourcePath);
+				if (innerClassResource.exists()) {
+					return getMetadataReader(innerClassResource);
+				}
+			}
+			throw ex;
+		}
+	}
+
+}

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

@@ -34,9 +34,10 @@ import org.springframework.core.io.ResourceLoader;
  *
  * @author Juergen Hoeller
  * @author Costin Leau
+ * @author Brian Clozel
  * @since 2.5
  */
-public class CachingMetadataReaderFactory implements MetadataReaderFactory {
+public class CachingMetadataReaderFactory extends AbstractMetadataReaderFactory {
 
 	/** Default maximum number of entries for a local MetadataReader cache: 256. */
 	public static final int DEFAULT_CACHE_LIMIT = 256;
@@ -52,8 +53,7 @@ public class CachingMetadataReaderFactory implements MetadataReaderFactory {
 	 * using a local resource cache.
 	 */
 	public CachingMetadataReaderFactory() {
-		this.delegate = MetadataReaderFactory.create((ClassLoader) null);
+		this(MetadataReaderFactory.create((ClassLoader) null));
-		setCacheLimit(DEFAULT_CACHE_LIMIT);
 	}
 
 	/**
@@ -62,8 +62,7 @@ public class CachingMetadataReaderFactory implements MetadataReaderFactory {
 	 * @param classLoader the ClassLoader to use
 	 */
 	public CachingMetadataReaderFactory(@Nullable ClassLoader classLoader) {
-		this.delegate = MetadataReaderFactory.create(classLoader);
+		this(MetadataReaderFactory.create(classLoader));
-		setCacheLimit(DEFAULT_CACHE_LIMIT);
 	}
 
 	/**
@@ -74,8 +73,13 @@ public class CachingMetadataReaderFactory implements MetadataReaderFactory {
 	 * @see DefaultResourceLoader#getResourceCache
 	 */
 	public CachingMetadataReaderFactory(@Nullable ResourceLoader resourceLoader) {
-		this.delegate = MetadataReaderFactory.create(resourceLoader);
+		this(MetadataReaderFactory.create(resourceLoader));
-		if (resourceLoader instanceof DefaultResourceLoader defaultResourceLoader) {
+	}
+
+	CachingMetadataReaderFactory(MetadataReaderFactory delegate) {
+		super(delegate.getResourceLoader());
+		this.delegate = delegate;
+		if (getResourceLoader() instanceof DefaultResourceLoader defaultResourceLoader) {
 			this.metadataReaderCache = defaultResourceLoader.getResourceCache(MetadataReader.class);
 		}
 		else {
@@ -83,6 +87,7 @@ public class CachingMetadataReaderFactory implements MetadataReaderFactory {
 		}
 	}
 
+
 	/**
 	 * Specify the maximum number of entries for the MetadataReader cache.
 	 * <p>Default is 256 for a local cache, whereas a shared cache is
@@ -113,11 +118,6 @@ public class CachingMetadataReaderFactory implements MetadataReaderFactory {
 		}
 	}
 
-	@Override
-	public MetadataReader getMetadataReader(String className) throws IOException {
-		return this.delegate.getMetadataReader(className);
-	}
-
 	@Override
 	public MetadataReader getMetadataReader(Resource resource) throws IOException {
 		if (this.metadataReaderCache instanceof ConcurrentMap) {

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

@@ -53,6 +53,13 @@ public interface MetadataReaderFactory {
 	 */
 	MetadataReader getMetadataReader(Resource resource) throws IOException;
 
+	/**
+	 * Return the ResourceLoader that this MetadataReaderFactory has been
+	 * constructed with.
+	 * @since 7.0
+	 */
+	ResourceLoader getResourceLoader();
+
 	/**
 	 * Create a default {@link MetadataReaderFactory} implementation that's suitable
 	 * for the current JVM.

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

@@ -16,15 +16,12 @@
 
 package org.springframework.core.type.classreading;
 
-import java.io.FileNotFoundException;
 import java.io.IOException;
 
 import org.jspecify.annotations.Nullable;
 
-import org.springframework.core.io.DefaultResourceLoader;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.ResourceLoader;
-import org.springframework.util.ClassUtils;
 
 /**
  * Simple implementation of the {@link MetadataReaderFactory} interface,
@@ -33,16 +30,14 @@ import org.springframework.util.ClassUtils;
  * @author Juergen Hoeller
  * @since 2.5
  */
-public class SimpleMetadataReaderFactory implements MetadataReaderFactory {
+public class SimpleMetadataReaderFactory extends AbstractMetadataReaderFactory {
-
-	private final ResourceLoader resourceLoader;
 
 
 	/**
 	 * Create a new SimpleMetadataReaderFactory for the default class loader.
 	 */
 	public SimpleMetadataReaderFactory() {
-		this.resourceLoader = new DefaultResourceLoader();
+		super();
 	}
 
 	/**
@@ -51,7 +46,7 @@ public class SimpleMetadataReaderFactory implements MetadataReaderFactory {
 	 * (also determines the ClassLoader to use)
 	 */
 	public SimpleMetadataReaderFactory(@Nullable ResourceLoader resourceLoader) {
-		this.resourceLoader = (resourceLoader != null ? resourceLoader : new DefaultResourceLoader());
+		super(resourceLoader);
 	}
 
 	/**
@@ -59,49 +54,12 @@ public class SimpleMetadataReaderFactory implements MetadataReaderFactory {
 	 * @param classLoader the ClassLoader to use
 	 */
 	public SimpleMetadataReaderFactory(@Nullable ClassLoader classLoader) {
-		this.resourceLoader =
+		super(classLoader);
-				(classLoader != null ? new DefaultResourceLoader(classLoader) : new DefaultResourceLoader());
-	}
-
-
-	/**
-	 * Return the ResourceLoader that this MetadataReaderFactory has been
-	 * constructed with.
-	 */
-	public final ResourceLoader getResourceLoader() {
-		return this.resourceLoader;
-	}
-
-
-	@Override
-	public MetadataReader getMetadataReader(String className) throws IOException {
-		try {
-			String resourcePath = ResourceLoader.CLASSPATH_URL_PREFIX +
-					ClassUtils.convertClassNameToResourcePath(className) + ClassUtils.CLASS_FILE_SUFFIX;
-			Resource resource = this.resourceLoader.getResource(resourcePath);
-			return getMetadataReader(resource);
-		}
-		catch (FileNotFoundException ex) {
-			// Maybe an inner class name using the dot name syntax? Need to use the dollar syntax here...
-			// ClassUtils.forName has an equivalent check for resolution into Class references later on.
-			int lastDotIndex = className.lastIndexOf('.');
-			if (lastDotIndex != -1) {
-				String innerClassName =
-						className.substring(0, lastDotIndex) + '$' + className.substring(lastDotIndex + 1);
-				String innerClassResourcePath = ResourceLoader.CLASSPATH_URL_PREFIX +
-						ClassUtils.convertClassNameToResourcePath(innerClassName) + ClassUtils.CLASS_FILE_SUFFIX;
-				Resource innerClassResource = this.resourceLoader.getResource(innerClassResourcePath);
-				if (innerClassResource.exists()) {
-					return getMetadataReader(innerClassResource);
-				}
-			}
-			throw ex;
-		}
 	}
 
 	@Override
 	public MetadataReader getMetadataReader(Resource resource) throws IOException {
-		return new SimpleMetadataReader(resource, this.resourceLoader.getClassLoader());
+		return new SimpleMetadataReader(resource, getResourceLoader().getClassLoader());
 	}
 
 }

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

@@ -16,15 +16,12 @@
 
 package org.springframework.core.type.classreading;
 
-import java.io.FileNotFoundException;
 import java.io.IOException;
 
 import org.jspecify.annotations.Nullable;
 
-import org.springframework.core.io.DefaultResourceLoader;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.ResourceLoader;
-import org.springframework.util.ClassUtils;
 
 /**
  * Implementation of the {@link MetadataReaderFactory} interface,
@@ -33,17 +30,14 @@ import org.springframework.util.ClassUtils;
  * @author Brian Clozel
  * @since 7.0
  */
-public class ClassFileMetadataReaderFactory implements MetadataReaderFactory {
+public class ClassFileMetadataReaderFactory extends AbstractMetadataReaderFactory {
-
-
-	private final ResourceLoader resourceLoader;
 
 
 	/**
 	 * Create a new ClassFileMetadataReaderFactory for the default class loader.
 	 */
 	public ClassFileMetadataReaderFactory() {
-		this.resourceLoader = new DefaultResourceLoader();
+		super();
 	}
 
 	/**
@@ -52,7 +46,7 @@ public class ClassFileMetadataReaderFactory implements MetadataReaderFactory {
 	 * (also determines the ClassLoader to use)
 	 */
 	public ClassFileMetadataReaderFactory(@Nullable ResourceLoader resourceLoader) {
-		this.resourceLoader = (resourceLoader != null ? resourceLoader : new DefaultResourceLoader());
+		super(resourceLoader);
 	}
 
 	/**
@@ -60,46 +54,11 @@ public class ClassFileMetadataReaderFactory implements MetadataReaderFactory {
 	 * @param classLoader the ClassLoader to use
 	 */
 	public ClassFileMetadataReaderFactory(@Nullable ClassLoader classLoader) {
-		this.resourceLoader =
+		super(classLoader);
-				(classLoader != null ? new DefaultResourceLoader(classLoader) : new DefaultResourceLoader());
-	}
-
-	/**
-	 * Return the ResourceLoader that this MetadataReaderFactory has been
-	 * constructed with.
-	 */
-	public final ResourceLoader getResourceLoader() {
-		return this.resourceLoader;
-	}
-
-	@Override
-	public MetadataReader getMetadataReader(String className) throws IOException {
-		try {
-			String resourcePath = ResourceLoader.CLASSPATH_URL_PREFIX +
-					ClassUtils.convertClassNameToResourcePath(className) + ClassUtils.CLASS_FILE_SUFFIX;
-			Resource resource = this.resourceLoader.getResource(resourcePath);
-			return getMetadataReader(resource);
-		}
-		catch (FileNotFoundException ex) {
-			// Maybe an inner class name using the dot name syntax? Need to use the dollar syntax here...
-			// ClassUtils.forName has an equivalent check for resolution into Class references later on.
-			int lastDotIndex = className.lastIndexOf('.');
-			if (lastDotIndex != -1) {
-				String innerClassName =
-						className.substring(0, lastDotIndex) + '$' + className.substring(lastDotIndex + 1);
-				String innerClassResourcePath = ResourceLoader.CLASSPATH_URL_PREFIX +
-						ClassUtils.convertClassNameToResourcePath(innerClassName) + ClassUtils.CLASS_FILE_SUFFIX;
-				Resource innerClassResource = this.resourceLoader.getResource(innerClassResourcePath);
-				if (innerClassResource.exists()) {
-					return getMetadataReader(innerClassResource);
-				}
-			}
-			throw ex;
-		}
 	}
 
 	@Override
 	public MetadataReader getMetadataReader(Resource resource) throws IOException {
-		return new ClassFileMetadataReader(resource, this.resourceLoader.getClassLoader());
+		return new ClassFileMetadataReader(resource, getResourceLoader().getClassLoader());
 	}
 }

spring-core/src/test/java/org/springframework/core/type/CachingMetadataReaderLeakTests.java

@@ -1,78 +0,0 @@
-/*
- * Copyright 2002-present 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.
- * You may obtain a copy of the License at
- *
- *      https://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.springframework.core.type;
-
-import java.net.URL;
-
-import org.jspecify.annotations.Nullable;
-import org.junit.jupiter.api.Test;
-
-import org.springframework.core.io.Resource;
-import org.springframework.core.io.UrlResource;
-import org.springframework.core.testfixture.EnabledForTestGroups;
-import org.springframework.core.type.classreading.CachingMetadataReaderFactory;
-import org.springframework.core.type.classreading.MetadataReader;
-import org.springframework.core.type.classreading.MetadataReaderFactory;
-
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.springframework.core.testfixture.TestGroup.LONG_RUNNING;
-
-/**
- * Tests for checking the behaviour of {@link CachingMetadataReaderFactory} under
- * load. If the cache is not controlled, this test should fail with an out of memory
- * exception around entry 5k.
- *
- * @author Costin Leau
- * @author Sam Brannen
- */
-@EnabledForTestGroups(LONG_RUNNING)
-class CachingMetadataReaderLeakTests {
-
-	private static final int ITEMS_TO_LOAD = 9999;
-
-	private final MetadataReaderFactory mrf = new CachingMetadataReaderFactory();
-
-	@Test
-	void significantLoad() throws Exception {
-		// the biggest public class in the JDK (>60k)
-		URL url = getClass().getResource("/java/awt/Component.class");
-		assertThat(url).isNotNull();
-
-		// look at a LOT of items
-		for (int i = 0; i < ITEMS_TO_LOAD; i++) {
-			Resource resource = new UrlResource(url) {
-
-				@Override
-				public boolean equals(@Nullable Object obj) {
-					return (obj == this);
-				}
-
-				@Override
-				public int hashCode() {
-					return System.identityHashCode(this);
-				}
-			};
-
-			MetadataReader reader = mrf.getMetadataReader(resource);
-			assertThat(reader).isNotNull();
-		}
-
-		// useful for profiling to take snapshots
-		// System.in.read();
-	}
-
-}

spring-core/src/test/java/org/springframework/core/type/classreading/CachingMetadataReaderFactoryTests.java

@@ -0,0 +1,50 @@
+/*
+ * Copyright 2002-present 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.core.type.classreading;
+
+
+import org.junit.jupiter.api.Test;
+
+import org.springframework.core.io.Resource;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Tests for {@link CachingMetadataReaderFactory}.
+ */
+class CachingMetadataReaderFactoryTests {
+
+	@Test
+	void shouldCacheClassNameCalls() throws Exception {
+		MetadataReaderFactory delegate = mock(MetadataReaderFactory.class);
+		when(delegate.getMetadataReader(any(Resource.class))).thenReturn(mock(MetadataReader.class));
+
+		CachingMetadataReaderFactory readerFactory = new CachingMetadataReaderFactory(delegate);
+		MetadataReader metadataReader = readerFactory.getMetadataReader(TestClass.class.getName());
+		metadataReader = readerFactory.getMetadataReader(TestClass.class.getName());
+
+		verify(delegate, times(1)).getMetadataReader(any(Resource.class));
+	}
+
+	public static class TestClass {
+	}
+
+}