Resource.lastModified() propagates 0 value if target resource exists

Includes use of Files.getLastModifiedTime for NIO Paths, preservation of NIO-based resolution on createRelative, deprecation of PathResource, and consistent use of getContentLengthLong over getContentLength.

Issue: SPR-17320

Author: jhoeller

spring-core/src/main/java/org/springframework/core/io/AbstractFileResolvingResource.java

@@ -65,7 +65,7 @@ else if (code == HttpURLConnection.HTTP_NOT_FOUND) {
 						return false;
 					}
 				}
-				if (con.getContentLength() >= 0) {
+				if (con.getContentLengthLong() > 0) {
 					return true;
 				}
 				if (httpCon != null) {
@@ -106,7 +106,7 @@ public boolean isReadable() {
 						return false;
 					}
 				}
-				int contentLength = con.getContentLength();
+				long contentLength = con.getContentLengthLong();
 				if (contentLength > 0) {
 					return true;
 				}
@@ -226,23 +226,35 @@ public long contentLength() throws IOException {
 		URL url = getURL();
 		if (ResourceUtils.isFileURL(url)) {
 			// Proceed with file system resolution
-			return getFile().length();
+			File file = getFile();
+			long length = file.length();
+			if (length == 0L && !file.exists()) {
+				throw new FileNotFoundException(getDescription() +
+						" cannot be resolved in the file system for checking its content length");
+			}
+			return length;
 		}
 		else {
 			// Try a URL connection content-length header
 			URLConnection con = url.openConnection();
 			customizeConnection(con);
-			return con.getContentLength();
+			return con.getContentLengthLong();
 		}
 	}
 
 	@Override
 	public long lastModified() throws IOException {
 		URL url = getURL();
+		boolean fileCheck = false;
 		if (ResourceUtils.isFileURL(url) || ResourceUtils.isJarURL(url)) {
 			// Proceed with file system resolution
+			fileCheck = true;
 			try {
-				return super.lastModified();
+				File fileToCheck = getFileForLastModifiedCheck();
+				long lastModified = fileToCheck.lastModified();
+				if (lastModified > 0L || fileToCheck.exists()) {
+					return lastModified;
+				}
 			}
 			catch (FileNotFoundException ex) {
 				// Defensively fall back to URL connection check instead
@@ -251,7 +263,12 @@ public long lastModified() throws IOException {
 		// Try a URL connection last-modified header
 		URLConnection con = url.openConnection();
 		customizeConnection(con);
-		return con.getLastModified();
+		long lastModified = con.getLastModified();
+		if (fileCheck && lastModified == 0 && con.getContentLengthLong() <= 0) {
+			throw new FileNotFoundException(getDescription() +
+					" cannot be resolved in the file system for checking its last-modified timestamp");
+		}
+		return lastModified;
 	}
 
 	/**

spring-core/src/main/java/org/springframework/core/io/AbstractResource.java

@@ -146,7 +146,7 @@ public long contentLength() throws IOException {
 		InputStream is = getInputStream();
 		try {
 			long size = 0;
-			byte[] buf = new byte[255];
+			byte[] buf = new byte[256];
 			int read;
 			while ((read = is.read(buf)) != -1) {
 				size += read;
@@ -169,10 +169,11 @@ public long contentLength() throws IOException {
 	 */
 	@Override
 	public long lastModified() throws IOException {
-		long lastModified = getFileForLastModifiedCheck().lastModified();
-		if (lastModified == 0L) {
+		File fileToCheck = getFileForLastModifiedCheck();
+		long lastModified = fileToCheck.lastModified();
+		if (lastModified == 0L && !fileToCheck.exists()) {
 			throw new FileNotFoundException(getDescription() +
-					" cannot be resolved in the file system for resolving its last-modified timestamp");
+					" cannot be resolved in the file system for checking its last-modified timestamp");
 		}
 		return lastModified;
 	}

spring-core/src/main/java/org/springframework/core/io/FileSystemResource.java

@@ -26,6 +26,7 @@
 import java.nio.channels.FileChannel;
 import java.nio.channels.ReadableByteChannel;
 import java.nio.channels.WritableByteChannel;
+import java.nio.file.FileSystem;
 import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
@@ -86,10 +87,10 @@ public FileSystemResource(String path) {
 	 * <p>Note: When building relative resources via {@link #createRelative},
 	 * the relative path will apply <i>at the same directory level</i>:
 	 * e.g. new File("C:/dir1"), relative path "dir2" -> "C:/dir2"!
-	 * If you prefer to have relative paths built underneath the given root
-	 * directory, use the {@link #FileSystemResource(String) constructor with a file path}
-	 * to append a trailing slash to the root path: "C:/dir1/", which
-	 * indicates this directory as root for all relative paths.
+	 * If you prefer to have relative paths built underneath the given root directory,
+	 * use the {@link #FileSystemResource(String) constructor with a file path}
+	 * to append a trailing slash to the root path: "C:/dir1/", which indicates
+	 * this directory as root for all relative paths.
 	 * @param file a File handle
 	 * @see #FileSystemResource(Path)
 	 * @see #getFile()
@@ -102,20 +103,38 @@ public FileSystemResource(File file) {
 	}
 
 	/**
-	 * Create a new {@code FileSystemResource} from a {@link Path} handle.
+	 * Create a new {@code FileSystemResource} from a {@link Path} handle,
+	 * performing all file system interactions via NIO.2 instead of {@link File}.
 	 * <p>In contrast to {@link PathResource}, this variant strictly follows the
 	 * general {@link FileSystemResource} conventions, in particular in terms of
 	 * path cleaning and {@link #createRelative(String)} handling.
 	 * @param filePath a Path handle to a file
 	 * @since 5.1
 	 * @see #FileSystemResource(File)
-	 * @see PathResource
 	 */
 	public FileSystemResource(Path filePath) {
 		Assert.notNull(filePath, "Path must not be null");
+		this.path = StringUtils.cleanPath(filePath.toString());
+		this.file = null;
 		this.filePath = filePath;
+	}
+
+	/**
+	 * Create a new {@code FileSystemResource} from a {@link FileSystem} handle,
+	 * locating the specified path.
+	 * <p>This is an alternative to {@link #FileSystemResource(String)},
+	 * performing all file system interactions via NIO.2 instead of {@link File}.
+	 * @param fileSystem the FileSystem to locate the path within
+	 * @param path a file path
+	 * @since 5.1.1
+	 * @see #FileSystemResource(File)
+	 */
+	public FileSystemResource(FileSystem fileSystem, String path) {
+		Assert.notNull(fileSystem, "FileSystem must not be null");
+		Assert.notNull(path, "Path must not be null");
+		this.path = StringUtils.cleanPath(path);
 		this.file = null;
-		this.path = StringUtils.cleanPath(filePath.toString());
+		this.filePath = fileSystem.getPath(this.path).normalize();
 	}
 
 
@@ -240,11 +259,44 @@ public WritableByteChannel writableChannel() throws IOException {
 	}
 
 	/**
-	 * This implementation returns the underlying File's length.
+	 * This implementation returns the underlying File/Path length.
 	 */
 	@Override
 	public long contentLength() throws IOException {
-		return (this.file != null ? this.file.length() : Files.size(this.filePath));
+		if (this.file != null) {
+			long length = this.file.length();
+			if (length == 0L && !this.file.exists()) {
+				throw new FileNotFoundException(getDescription() +
+						" cannot be resolved in the file system for checking its content length");
+			}
+			return length;
+		}
+		else {
+			try {
+				return Files.size(this.filePath);
+			}
+			catch (NoSuchFileException ex) {
+				throw new FileNotFoundException(ex.getMessage());
+			}
+		}
+	}
+
+	/**
+	 * This implementation returns the underlying File/Path last-modified time.
+	 */
+	@Override
+	public long lastModified() throws IOException {
+		if (this.file != null) {
+			return super.lastModified();
+		}
+		else {
+			try {
+				return Files.getLastModifiedTime(this.filePath).toMillis();
+			}
+			catch (NoSuchFileException ex) {
+				throw new FileNotFoundException(ex.getMessage());
+			}
+		}
 	}
 
 	/**
@@ -255,7 +307,8 @@ public long contentLength() throws IOException {
 	@Override
 	public Resource createRelative(String relativePath) {
 		String pathToUse = StringUtils.applyRelativePath(this.path, relativePath);
-		return new FileSystemResource(pathToUse);
+		return (this.file != null ? new FileSystemResource(pathToUse) :
+				new FileSystemResource(this.filePath.getFileSystem(), pathToUse));
 	}
 
 	/**

spring-core/src/main/java/org/springframework/core/io/FileUrlResource.java

@@ -37,9 +37,8 @@
  * <p>This is the class resolved by {@link DefaultResourceLoader} for a "file:..."
  * URL location, allowing a downcast to {@link WritableResource} for it.
  *
- * <p>Alternatively, for direct construction from a {@link java.io.File} handle,
- * consider using {@link FileSystemResource}. For an NIO {@link java.nio.file.Path},
- * consider using {@link PathResource} instead.
+ * <p>Alternatively, for direct construction from a {@link java.io.File} handle
+ * or NIO {@link java.nio.file.Path}, consider using {@link FileSystemResource}.
  *
  * @author Juergen Hoeller
  * @since 5.0.2

spring-core/src/main/java/org/springframework/core/io/PathResource.java

@@ -48,10 +48,11 @@
  * @author Philippe Marschall
  * @author Juergen Hoeller
  * @since 4.0
- * @see FileSystemResource#FileSystemResource(Path)
  * @see java.nio.file.Path
  * @see java.nio.file.Files
+ * @deprecated as of 5.1.1, in favor of {@link FileSystemResource#FileSystemResource(Path)}
  */
+@Deprecated
 public class PathResource extends AbstractResource implements WritableResource {
 
 	private final Path path;
@@ -105,7 +106,7 @@ public final String getPath() {
 
 	/**
 	 * This implementation returns whether the underlying file exists.
-	 * @see org.springframework.core.io.PathResource#exists()
+	 * @see java.nio.file.Files#exists(Path, java.nio.file.LinkOption...)
 	 */
 	@Override
 	public boolean exists() {

spring-core/src/main/java/org/springframework/core/io/Resource.java

@@ -43,9 +43,9 @@
  * @see WritableResource
  * @see ContextResource
  * @see UrlResource
- * @see ClassPathResource
+ * @see FileUrlResource
  * @see FileSystemResource
- * @see PathResource
+ * @see ClassPathResource
  * @see ByteArrayResource
  * @see InputStreamResource
  */

spring-core/src/test/java/org/springframework/core/io/ResourceTests.java

@@ -123,11 +123,10 @@ public void testClassPathResourceWithClass() throws IOException {
 
 	@Test
 	public void testFileSystemResource() throws IOException {
-		Resource resource = new FileSystemResource(getClass().getResource("Resource.class").getFile());
+		String file = getClass().getResource("Resource.class").getFile();
+		Resource resource = new FileSystemResource(file);
 		doTestResource(resource);
-		assertEquals(new FileSystemResource(getClass().getResource("Resource.class").getFile()), resource);
-		Resource resource2 = new FileSystemResource("core/io/Resource.class");
-		assertEquals(resource2, new FileSystemResource("core/../core/io/./Resource.class"));
+		assertEquals(new FileSystemResource(file), resource);
 	}
 
 	@Test
@@ -136,8 +135,12 @@ public void testFileSystemResourceWithFilePath() throws Exception {
 		Resource resource = new FileSystemResource(filePath);
 		doTestResource(resource);
 		assertEquals(new FileSystemResource(filePath), resource);
-		Resource resource2 = new FileSystemResource("core/io/Resource.class");
-		assertEquals(resource2, new FileSystemResource("core/../core/io/./Resource.class"));
+	}
+
+	@Test
+	public void testFileSystemResourceWithPlainPath() {
+		Resource resource = new FileSystemResource("core/io/Resource.class");
+		assertEquals(resource, new FileSystemResource("core/../core/io/./Resource.class"));
 	}
 
 	@Test
@@ -157,23 +160,52 @@ public void testUrlResource() throws IOException {
 	private void doTestResource(Resource resource) throws IOException {
 		assertEquals("Resource.class", resource.getFilename());
 		assertTrue(resource.getURL().getFile().endsWith("Resource.class"));
+		assertTrue(resource.exists());
+		assertTrue(resource.isReadable());
+		assertTrue(resource.contentLength() > 0);
+		assertTrue(resource.lastModified() > 0);
 
 		Resource relative1 = resource.createRelative("ClassPathResource.class");
 		assertEquals("ClassPathResource.class", relative1.getFilename());
 		assertTrue(relative1.getURL().getFile().endsWith("ClassPathResource.class"));
 		assertTrue(relative1.exists());
+		assertTrue(relative1.isReadable());
+		assertTrue(relative1.contentLength() > 0);
+		assertTrue(relative1.lastModified() > 0);
 
 		Resource relative2 = resource.createRelative("support/ResourcePatternResolver.class");
 		assertEquals("ResourcePatternResolver.class", relative2.getFilename());
 		assertTrue(relative2.getURL().getFile().endsWith("ResourcePatternResolver.class"));
 		assertTrue(relative2.exists());
+		assertTrue(relative2.isReadable());
+		assertTrue(relative2.contentLength() > 0);
+		assertTrue(relative2.lastModified() > 0);
 
-		/*
 		Resource relative3 = resource.createRelative("../SpringVersion.class");
 		assertEquals("SpringVersion.class", relative3.getFilename());
 		assertTrue(relative3.getURL().getFile().endsWith("SpringVersion.class"));
 		assertTrue(relative3.exists());
-		*/
+		assertTrue(relative3.isReadable());
+		assertTrue(relative3.contentLength() > 0);
+		assertTrue(relative3.lastModified() > 0);
+
+		Resource relative4 = resource.createRelative("X.class");
+		assertFalse(relative4.exists());
+		assertFalse(relative4.isReadable());
+		try {
+			relative4.contentLength();
+			fail("Should have thrown FileNotFoundException");
+		}
+		catch (FileNotFoundException ex) {
+			// expected
+		}
+		try {
+			relative4.lastModified();
+			fail("Should have thrown FileNotFoundException");
+		}
+		catch (FileNotFoundException ex) {
+			// expected
+		}
 	}
 
 	@Test