Polish 'Avoid parsing nested jar URLs for cached resource lookups'

See gh-51827

loader/spring-boot-loader/src/main/java/org/springframework/boot/loader/net/protocol/jar/JarUrlConnection.java

@@ -45,6 +45,7 @@ import org.springframework.boot.loader.net.util.UrlDecoder;
  * @author Phillip Webb
  * @author Andy Wilkinson
  * @author Rostyslav Dudka
+ * @author Sharan Bharmshetty
  */
 final class JarUrlConnection extends java.net.JarURLConnection {
 
@@ -335,20 +336,11 @@ final class JarUrlConnection extends java.net.JarURLConnection {
 		String spec = url.getFile();
 		if (spec.startsWith("nested:")) {
 			int separator = spec.indexOf("!/");
-			boolean specHasEntry = (separator != -1) && (separator + 2 != spec.length());
-			if (specHasEntry) {
+			boolean specHasJarEntry = (separator != -1) && (separator + 2 != spec.length());
+			if (specHasJarEntry) {
 				String jarFileSpec = spec.substring(0, separator);
 				boolean runtimeRef = "runtime".equals(url.getRef());
-				JarFileUrlKey urlKey = new JarFileUrlKey("jar", "", -1, jarFileSpec, runtimeRef);
-				JarFile jarFile = jarFiles.getCached(urlKey);
-				if (jarFile == null) {
-					URL jarFileUrl = new URL(jarFileSpec);
-					if (runtimeRef) {
-						jarFileUrl = new URL(jarFileUrl, "#runtime");
-					}
-					jarFile = jarFiles.getOrCreate(true, jarFileUrl);
-					jarFiles.cacheIfAbsent(true, jarFileUrl, jarFile);
-				}
+				JarFile jarFile = jarFiles.getOrCreateAndCache(jarFileSpec, runtimeRef);
 				String entryName = UrlDecoder.decode(spec.substring(separator + 2));
 				if (!hasEntry(jarFile, entryName)) {
 					return notFoundConnection(jarFile.getName(), entryName);

loader/spring-boot-loader/src/main/java/org/springframework/boot/loader/net/protocol/jar/UrlJarFiles.java

@@ -31,6 +31,7 @@ import java.util.jar.JarFile;
  * instances.
  *
  * @author Phillip Webb
+ * @author Sharan Bharmshetty
  */
 class UrlJarFiles {
 
@@ -53,6 +54,20 @@ class UrlJarFiles {
 		this.factory = factory;
 	}
 
+	JarFile getOrCreateAndCache(String jarFileSpec, boolean runtimeRef) throws IOException {
+		JarFileUrlKey jarFileUrlKey = new JarFileUrlKey("jar", "", -1, jarFileSpec, runtimeRef);
+		JarFile jarFile = this.cache.get(jarFileUrlKey);
+		if (jarFile == null) {
+			URL jarFileUrl = new URL(jarFileSpec);
+			if (runtimeRef) {
+				jarFileUrl = new URL(jarFileUrl, "#runtime");
+			}
+			jarFile = this.factory.createJarFile(jarFileUrl, this::onClose);
+			this.cache.putIfAbsent(jarFileUrlKey, jarFile, jarFileUrl);
+		}
+		return jarFile;
+	}
+
 	/**
 	 * Get an existing {@link JarFile} instance from the cache, or create a new
 	 * {@link JarFile} instance that can be {@link #cacheIfAbsent(boolean, URL, JarFile)
@@ -81,15 +96,6 @@ class UrlJarFiles {
 		return this.cache.get(jarFileUrl);
 	}
 
-	/**
-	 * Return the cached {@link JarFile} if available.
-	 * @param urlKey the jar file URL key
-	 * @return the cached jar or {@code null}
-	 */
-	JarFile getCached(JarFileUrlKey urlKey) {
-		return this.cache.get(urlKey);
-	}
-
 	/**
 	 * Cache the given {@link JarFile} if caching can be used and there is no existing
 	 * entry.
@@ -168,12 +174,12 @@ class UrlJarFiles {
 
 		/**
 		 * Get a {@link JarFile} from the cache given a jar file URL key.
-		 * @param urlKey the jar file URL key
+		 * @param jarFileUrlKey the jar file URL key
 		 * @return the cached {@link JarFile} or {@code null}
 		 */
-		JarFile get(JarFileUrlKey urlKey) {
+		JarFile get(JarFileUrlKey jarFileUrlKey) {
 			synchronized (this) {
-				return this.jarFileUrlToJarFile.get(urlKey);
+				return this.jarFileUrlToJarFile.get(jarFileUrlKey);
 			}
 		}
 
@@ -197,11 +203,23 @@ class UrlJarFiles {
 		 * they were already there
 		 */
 		boolean putIfAbsent(URL jarFileUrl, JarFile jarFile) {
-			JarFileUrlKey urlKey = new JarFileUrlKey(jarFileUrl);
+			return putIfAbsent(new JarFileUrlKey(jarFileUrl), jarFile, jarFileUrl);
+		}
+
+		/**
+		 * Put the given jar file URL and jar file into the cache if they aren't already
+		 * there.
+		 * @param jarFileUrlKey the jar file URL key
+		 * @param jarFileUrl the jar file URL
+		 * @param jarFile the jar file
+		 * @return {@code true} if the items were added to the cache or {@code false} if
+		 * they were already there
+		 */
+		boolean putIfAbsent(JarFileUrlKey jarFileUrlKey, JarFile jarFile, URL jarFileUrl) {
 			synchronized (this) {
-				JarFile cached = this.jarFileUrlToJarFile.get(urlKey);
+				JarFile cached = this.jarFileUrlToJarFile.get(jarFileUrlKey);
 				if (cached == null) {
-					this.jarFileUrlToJarFile.put(urlKey, jarFile);
+					this.jarFileUrlToJarFile.put(jarFileUrlKey, jarFile);
 					this.jarFileToJarFileUrl.put(jarFile, jarFileUrl);
 					return true;
 				}

loader/spring-boot-loader/src/test/java/org/springframework/boot/loader/net/protocol/jar/JarFileUrlKeyTests.java

@@ -30,6 +30,7 @@ import static org.assertj.core.api.Assertions.assertThat;
  * Tests for {@link JarFileUrlKey}.
  *
  * @author Phillip Webb
+ * @author Sharan Bharmshetty
  */
 class JarFileUrlKeyTests {