Polish contribution

See gh-29697

spring-beans/src/main/java/org/springframework/beans/factory/xml/ResourceEntityResolver.java

@@ -120,18 +120,20 @@ public class ResourceEntityResolver extends DelegatingEntityResolver {
 	/**
 	 * A fallback method for {@link #resolveEntity(String, String)} that is used when a
 	 * "schema" entity (DTD or XSD) cannot be resolved as a local resource. The default
-	 * behavior is to perform a remote resolution over HTTPS.
+	 * behavior is to perform remote resolution over HTTPS.
 	 * <p>Subclasses can override this method to change the default behavior.
 	 * <ul>
 	 * <li>Return {@code null} to fall back to the parser's
 	 * {@linkplain org.xml.sax.EntityResolver#resolveEntity(String, String) default behavior}.</li>
-	 * <li>Throw an exception to prevent remote resolution of the XSD or DTD.</li>
+	 * <li>Throw an exception to prevent remote resolution of the DTD or XSD.</li>
 	 * </ul>
 	 * @param publicId the public identifier of the external entity being referenced,
 	 * or null if none was supplied
-	 * @param systemId the system identifier of the external entity being referenced
+	 * @param systemId the system identifier of the external entity being referenced,
+	 * representing the URL of the DTD or XSD
 	 * @return an InputSource object describing the new input source, or null to request
-	 * that the parser open a regular URI connection to the system identifier.
+	 * that the parser open a regular URI connection to the system identifier
+	 * @since 6.0.4
 	 */
 	@Nullable
 	protected InputSource resolveSchemaEntity(@Nullable String publicId, String systemId) {

spring-beans/src/test/java/org/springframework/beans/factory/xml/ResourceEntityResolverTests.java

@@ -16,88 +16,68 @@
 
 package org.springframework.beans.factory.xml;
 
-import java.io.IOException;
+import org.junit.jupiter.params.ParameterizedTest;
-
+import org.junit.jupiter.params.provider.ValueSource;
-import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 import org.xml.sax.InputSource;
-import org.xml.sax.SAXException;
 
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.ResourceLoader;
 import org.springframework.lang.Nullable;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 
 /**
+ * Unit tests for ResourceEntityResolver.
+ *
  * @author Simon Baslé
+ * @author Sam Brannen
+ * @since 6.0.4
  */
 class ResourceEntityResolverTests {
 
-	@Test
+	@ParameterizedTest
-	void resolveEntityCallsFallbackWithNullOnDtd() throws IOException, SAXException {
+	@ValueSource(strings = { "https://example.org/schema/", "https://example.org/schema.xml" })
-		ResourceEntityResolver resolver = new FallingBackEntityResolver(false, null);
+	void resolveEntityDoesNotCallFallbackIfNotSchema(String systemId) throws Exception {
-
+		ConfigurableFallbackEntityResolver resolver = new ConfigurableFallbackEntityResolver(true);
-		assertThat(resolver.resolveEntity("testPublicId", "https://example.org/exampleschema.dtd"))
-				.isNull();
-	}
-
-	@Test
-	void resolveEntityCallsFallbackWithNullOnXsd() throws IOException, SAXException {
-		ResourceEntityResolver resolver = new FallingBackEntityResolver(false, null);
 
-		assertThat(resolver.resolveEntity("testPublicId", "https://example.org/exampleschema.xsd"))
+		assertThat(resolver.resolveEntity("testPublicId", systemId)).isNull();
-				.isNull();
+		assertThat(resolver.fallbackInvoked).isFalse();
 	}
 
-	@Test
+	@ParameterizedTest
-	void resolveEntityCallsFallbackWithThrowOnDtd() {
+	@ValueSource(strings = { "https://example.org/schema.dtd", "https://example.org/schema.xsd" })
-		ResourceEntityResolver resolver = new FallingBackEntityResolver(true, null);
+	void resolveEntityCallsFallbackThatReturnsNull(String systemId) throws Exception {
+		ConfigurableFallbackEntityResolver resolver = new ConfigurableFallbackEntityResolver(null);
 
-		assertThatIllegalStateException().isThrownBy(
+		assertThat(resolver.resolveEntity("testPublicId", systemId)).isNull();
-						() -> resolver.resolveEntity("testPublicId", "https://example.org/exampleschema.dtd"))
+		assertThat(resolver.fallbackInvoked).isTrue();
-				.withMessage("FallingBackEntityResolver that throws");
 	}
 
-	@Test
+	@ParameterizedTest
-	void resolveEntityCallsFallbackWithThrowOnXsd() {
+	@ValueSource(strings = { "https://example.org/schema.dtd", "https://example.org/schema.xsd" })
-		ResourceEntityResolver resolver = new FallingBackEntityResolver(true, null);
+	void resolveEntityCallsFallbackThatThrowsException(String systemId) {
+		ConfigurableFallbackEntityResolver resolver = new ConfigurableFallbackEntityResolver(true);
 
-		assertThatIllegalStateException().isThrownBy(
+		assertThatExceptionOfType(ResolutionRejectedException.class)
-						() -> resolver.resolveEntity("testPublicId", "https://example.org/exampleschema.xsd"))
+				.isThrownBy(() -> resolver.resolveEntity("testPublicId", systemId));
-				.withMessage("FallingBackEntityResolver that throws");
+		assertThat(resolver.fallbackInvoked).isTrue();
 	}
 
-	@Test
+	@ParameterizedTest
-	void resolveEntityCallsFallbackWithInputSourceOnDtd() throws IOException, SAXException {
+	@ValueSource(strings = { "https://example.org/schema.dtd", "https://example.org/schema.xsd" })
+	void resolveEntityCallsFallbackThatReturnsInputSource(String systemId) throws Exception {
 		InputSource expected = Mockito.mock(InputSource.class);
-		ResourceEntityResolver resolver = new FallingBackEntityResolver(false, expected);
+		ConfigurableFallbackEntityResolver resolver = new ConfigurableFallbackEntityResolver(expected);
 
-		assertThat(resolver.resolveEntity("testPublicId", "https://example.org/exampleschema.dtd"))
+		assertThat(resolver.resolveEntity("testPublicId", systemId)).isSameAs(expected);
-				.isNotNull()
+		assertThat(resolver.fallbackInvoked).isTrue();
-				.isSameAs(expected);
 	}
 
-	@Test
-	void resolveEntityCallsFallbackWithInputSourceOnXsd() throws IOException, SAXException {
-		InputSource expected = Mockito.mock(InputSource.class);
-		ResourceEntityResolver resolver = new FallingBackEntityResolver(false, expected);
-
-		assertThat(resolver.resolveEntity("testPublicId", "https://example.org/exampleschema.xsd"))
-				.isNotNull()
-				.isSameAs(expected);
-	}
-
-	@Test
-	void resolveEntityDoesntCallFallbackIfNotSchema() throws IOException, SAXException {
-		ResourceEntityResolver resolver = new FallingBackEntityResolver(true, null);
-
-		assertThat(resolver.resolveEntity("testPublicId", "https://example.org/example.xml"))
-				.isNull();
-	}
 
 	private static final class NoOpResourceLoader implements ResourceLoader {
+
 		@Override
 		public Resource getResource(String location) {
 			return null;
@@ -109,23 +89,40 @@ class ResourceEntityResolverTests {
 		}
 	}
 
-	private static class FallingBackEntityResolver extends ResourceEntityResolver {
+	private static class ConfigurableFallbackEntityResolver extends ResourceEntityResolver {
 
 		private final boolean shouldThrow;
+
 		@Nullable
 		private final InputSource returnValue;
 
-		private FallingBackEntityResolver(boolean shouldThrow, @Nullable InputSource returnValue) {
+		boolean fallbackInvoked = false;
+
+
+		private ConfigurableFallbackEntityResolver(boolean shouldThrow) {
 			super(new NoOpResourceLoader());
 			this.shouldThrow = shouldThrow;
+			this.returnValue = null;
+		}
+
+		private ConfigurableFallbackEntityResolver(@Nullable InputSource returnValue) {
+			super(new NoOpResourceLoader());
+			this.shouldThrow = false;
 			this.returnValue = returnValue;
 		}
 
+
 		@Nullable
 		@Override
 		protected InputSource resolveSchemaEntity(String publicId, String systemId) {
-			if (shouldThrow) throw new IllegalStateException("FallingBackEntityResolver that throws");
+			this.fallbackInvoked = true;
+			if (this.shouldThrow) {
+				throw new ResolutionRejectedException();
+			}
 			return this.returnValue;
 		}
 	}
+
+	static class ResolutionRejectedException extends RuntimeException {}
+
 }