Fix SpringFactoriesLoader cache key when using default classloader

Update `SpringFactoriesLoader` so that `null` is never used for the
cache key. Prior to this commit, calling `forDefaultResourceLocation`
with `null` and `ClassUtils.getDefaultClassLoader()` would provide
different `SpringFactoriesLoader` instances rather than making use
of a single shared cached instance.

See gh-28416

Author: philwebb

spring-core/src/main/java/org/springframework/core/io/support/SpringFactoriesLoader.java

@@ -114,43 +114,17 @@ public class SpringFactoriesLoader {
 	private final Map<String, List<String>> factories;
 
 
-	private SpringFactoriesLoader(@Nullable ClassLoader classLoader, String resourceLocation) {
-		this.classLoader = classLoader;
-		this.factories = loadFactoriesResource((classLoader != null) ? classLoader
-				: SpringFactoriesLoader.class.getClassLoader(), resourceLocation);
-	}
-
+	/**
+	 * Create a new {@link SpringFactoriesLoader} instance.
+	 * @param classLoader the classloader used to instantiate the factories
+	 * @param factories a map of factory class name to implementation class names
+	 */
 	protected SpringFactoriesLoader(@Nullable ClassLoader classLoader, Map<String, List<String>> factories) {
 		this.classLoader = classLoader;
 		this.factories = factories;
 	}
 
 
-	private Map<String, List<String>> loadFactoriesResource(ClassLoader classLoader, String resourceLocation) {
-		Map<String, List<String>> result = new LinkedHashMap<>();
-		try {
-			Enumeration<URL> urls = classLoader.getResources(resourceLocation);
-			while (urls.hasMoreElements()) {
-				UrlResource resource = new UrlResource(urls.nextElement());
-				Properties properties = PropertiesLoaderUtils.loadProperties(resource);
-				properties.forEach((name, value) -> {
-					List<String> implementations = result.computeIfAbsent(((String) name).trim(), key -> new ArrayList<>());
-					Arrays.stream(StringUtils.commaDelimitedListToStringArray((String) value))
-						.map(String::trim).forEach(implementations::add);
-				});
-			}
-			result.replaceAll(this::toDistinctUnmodifiableList);
-		}
-		catch (IOException ex) {
-			throw new IllegalArgumentException("Unable to load factories from location [" + resourceLocation + "]", ex);
-		}
-		return Collections.unmodifiableMap(result);
-	}
-
-	private List<String> toDistinctUnmodifiableList(String factoryType, List<String> implementations) {
-		return implementations.stream().distinct().toList();
-	}
-
 	/**
 	 * Load and instantiate the factory implementations of the given type from
 	 * {@value #FACTORIES_RESOURCE_LOCATION}, using the given class loader and
@@ -343,19 +317,47 @@ public static SpringFactoriesLoader forResourceLocation(String resourceLocation)
 	 */
 	public static SpringFactoriesLoader forResourceLocation(@Nullable ClassLoader classLoader, String resourceLocation) {
 		Assert.hasText(resourceLocation, "'resourceLocation' must not be empty");
-		Map<String, SpringFactoriesLoader> loaders = SpringFactoriesLoader.cache.get(classLoader);
+		ClassLoader resourceClassLoader = (classLoader != null) ? classLoader
+				: SpringFactoriesLoader.class.getClassLoader();
+		Map<String, SpringFactoriesLoader> loaders = SpringFactoriesLoader.cache.get(resourceClassLoader);
 		if (loaders == null) {
 			loaders = new ConcurrentReferenceHashMap<>();
-			SpringFactoriesLoader.cache.put(classLoader, loaders);
+			SpringFactoriesLoader.cache.put(resourceClassLoader, loaders);
 		}
 		SpringFactoriesLoader loader = loaders.get(resourceLocation);
 		if (loader == null) {
-			loader = new SpringFactoriesLoader(classLoader, resourceLocation);
+			Map<String, List<String>> factories = loadFactoriesResource(resourceClassLoader, resourceLocation);
+			loader = new SpringFactoriesLoader(classLoader, factories);
 			loaders.put(resourceLocation, loader);
 		}
 		return loader;
 	}
 
+	private static Map<String, List<String>> loadFactoriesResource(ClassLoader classLoader, String resourceLocation) {
+		Map<String, List<String>> result = new LinkedHashMap<>();
+		try {
+			Enumeration<URL> urls = classLoader.getResources(resourceLocation);
+			while (urls.hasMoreElements()) {
+				UrlResource resource = new UrlResource(urls.nextElement());
+				Properties properties = PropertiesLoaderUtils.loadProperties(resource);
+				properties.forEach((name, value) -> {
+					List<String> implementations = result.computeIfAbsent(((String) name).trim(), key -> new ArrayList<>());
+					Arrays.stream(StringUtils.commaDelimitedListToStringArray((String) value))
+						.map(String::trim).forEach(implementations::add);
+				});
+			}
+			result.replaceAll(SpringFactoriesLoader::toDistinctUnmodifiableList);
+		}
+		catch (IOException ex) {
+			throw new IllegalArgumentException("Unable to load factories from location [" + resourceLocation + "]", ex);
+		}
+		return Collections.unmodifiableMap(result);
+	}
+
+	private static List<String> toDistinctUnmodifiableList(String factoryType, List<String> implementations) {
+		return implementations.stream().distinct().toList();
+	}
+
 
 	/**
 	 * Internal instantiator used to create the factory instance.

spring-core/src/test/java/org/springframework/core/io/support/SpringFactoriesLoaderTests.java

@@ -34,6 +34,7 @@
 import org.springframework.core.io.support.SpringFactoriesLoader.FactoryInstantiator;
 import org.springframework.core.io.support.SpringFactoriesLoader.FailureHandler;
 import org.springframework.core.log.LogMessage;
+import org.springframework.util.ClassUtils;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -169,6 +170,13 @@ void loadForResourceLocationLoadsFactories() {
 		assertThat(factories.get(0)).isInstanceOf(MyDummyFactory1.class);
 	}
 
+	@Test
+	void sameCachedResultIsUsedForDefaultClassLoaderAndNullClassLoader() {
+		SpringFactoriesLoader forNull = SpringFactoriesLoader.forDefaultResourceLocation(null);
+		SpringFactoriesLoader forDefault = SpringFactoriesLoader.forDefaultResourceLocation(ClassUtils.getDefaultClassLoader());
+		assertThat(forNull).isSameAs(forDefault);
+	}
+
 
 	@Nested
 	class FailureHandlerTests {