Change *FileMappedHttpContent to delegate last modified to wrapped resource (#13889)

#13850 change *FileMappedHttpContent to delegate last modified to wrapped resource

Signed-off-by: Ludovic Orban <[email protected]>

Author: lorban

jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java

@@ -16,7 +16,6 @@
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.file.Path;
-import java.time.Instant;
 import java.util.Objects;
 
 import org.eclipse.jetty.http.HttpField;
@@ -93,8 +92,6 @@ private static class SingleBufferFileMappedHttpContent extends HttpContent.Wrapp
     {
         private final ByteBuffer _buffer;
         private final HttpField _contentLength;
-        private final HttpField _lastModified;
-        private final Instant _lastModifiedInstant;
 
         private SingleBufferFileMappedHttpContent(HttpContent content) throws IOException
         {
@@ -106,16 +103,14 @@ private SingleBufferFileMappedHttpContent(HttpContent content) throws IOExceptio
             if (_buffer == null)
                 throw new IOException("Cannot memory map Content (not supported by underlying FileSystem): " + content.getResource());
             _contentLength = new HttpField(HttpHeader.CONTENT_LENGTH, Integer.toString(_buffer.remaining()));
-            _lastModified = content.getLastModified();
-            _lastModifiedInstant = content.getLastModifiedInstant();
         }
 
         @Override
         public void writeTo(Content.Sink sink, long offset, long length, Callback callback)
         {
             try
             {
-                sink.write(true, BufferUtil.slice(_buffer, (int)offset, (int)length), callback);
+                sink.write(true, BufferUtil.slice(_buffer, Math.toIntExact(offset), Math.toIntExact(length)), callback);
             }
             catch (Throwable x)
             {
@@ -134,18 +129,6 @@ public long getContentLengthValue()
         {
             return _buffer.remaining();
         }
-
-        @Override
-        public Instant getLastModifiedInstant()
-        {
-            return _lastModifiedInstant;
-        }
-
-        @Override
-        public HttpField getLastModified()
-        {
-            return _lastModified;
-        }
     }
 
     private static class MultiBufferFileMappedHttpContent extends HttpContent.Wrapper
@@ -154,8 +137,6 @@ private static class MultiBufferFileMappedHttpContent extends HttpContent.Wrappe
         private final int maxBufferSize;
         private final HttpField _contentLength;
         private final long _contentLengthValue;
-        private final HttpField _lastModified;
-        private final Instant _lastModifiedInstant;
 
         private MultiBufferFileMappedHttpContent(HttpContent content, int maxBufferSize) throws IOException
         {
@@ -187,8 +168,6 @@ private MultiBufferFileMappedHttpContent(HttpContent content, int maxBufferSize)
             }
             _contentLengthValue = total;
             _contentLength = new HttpField(HttpHeader.CONTENT_LENGTH, Long.toString(total));
-            _lastModified = content.getLastModified();
-            _lastModifiedInstant = content.getLastModifiedInstant();
         }
 
         @Override
@@ -260,17 +239,5 @@ public long getContentLengthValue()
         {
             return _contentLengthValue;
         }
-
-        @Override
-        public Instant getLastModifiedInstant()
-        {
-            return _lastModifiedInstant;
-        }
-
-        @Override
-        public HttpField getLastModified()
-        {
-            return _lastModified;
-        }
     }
 }

jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/HttpContent.java

@@ -90,6 +90,7 @@ public interface HttpContent
 
     /**
      * Get the last modified instant of this resource.
+     * Always return the most up-to-date value.
      *
      * @return the last modified instant, or null if that instant of this content is not known.
      * @see #getLastModified()

jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/ValidatingCachingHttpContentFactory.java

@@ -38,6 +38,10 @@
  * a positive value indicates the number of milliseconds of the minimum time between validation checks.
  * </p>
  * <p>
+ * When the wrapped {@link HttpContent}'s {@link HttpContent#getLastModifiedInstant() last modification time} did not change,
+ * it is considered that the resource did not change too and is therefore not refreshed.
+ * </p>
+ * <p>
  * This also remember a missed entry for the time set by {@code validationTime}ms. After this has
  * elapsed the entry will be invalid and will be evicted from the cache at the next access.
  * </p>

jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactoryTest.java

@@ -20,6 +20,8 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
+import java.nio.file.attribute.FileTime;
+import java.time.Instant;
 
 import org.eclipse.jetty.http.MimeTypes;
 import org.eclipse.jetty.io.ByteBufferPool;
@@ -37,13 +39,47 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.lessThan;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 @ExtendWith(WorkDirExtension.class)
 public class FileMappingHttpContentFactoryTest
 {
     public WorkDir workDir;
 
+    @Test
+    public void testSingleBufferFileMappedLastModified() throws Exception
+    {
+        Path file = Files.writeString(workDir.getEmptyPathDir().resolve("file.txt"), "0123456789abcdefghijABCDEFGHIJ");
+        FileMappingHttpContentFactory fileMappingHttpContentFactory = new FileMappingHttpContentFactory(
+            new ResourceHttpContentFactory(ResourceFactory.root().newResource(file.getParent()), MimeTypes.DEFAULTS, ByteBufferPool.SIZED_NON_POOLING));
+
+        HttpContent content = fileMappingHttpContentFactory.getContent("file.txt");
+        Instant originalLastModifiedInstant = content.getLastModifiedInstant();
+
+        // Set the file's last modified time to 10s into the future.
+        Files.setLastModifiedTime(file, FileTime.from(Instant.now().plusSeconds(10)));
+
+        assertThat(originalLastModifiedInstant, lessThan(content.getLastModifiedInstant()));
+    }
+
+    @Test
+    public void testMultiBufferFileMappedLastModified() throws Exception
+    {
+        Path file = Files.writeString(workDir.getEmptyPathDir().resolve("file.txt"), "0123456789abcdefghijABCDEFGHIJ");
+        FileMappingHttpContentFactory fileMappingHttpContentFactory = new FileMappingHttpContentFactory(
+            new ResourceHttpContentFactory(ResourceFactory.root().newResource(file.getParent()), MimeTypes.DEFAULTS, ByteBufferPool.SIZED_NON_POOLING),
+            0, 10);
+
+        HttpContent content = fileMappingHttpContentFactory.getContent("file.txt");
+        Instant originalLastModifiedInstant = content.getLastModifiedInstant();
+
+        // Set the file's last modified time to 10s into the future.
+        Files.setLastModifiedTime(file, FileTime.from(Instant.now().plusSeconds(10)));
+
+        assertThat(originalLastModifiedInstant, lessThan(content.getLastModifiedInstant()));
+    }
+
     @Test
     public void testMultiBufferFileMappedOffsetAndLength() throws Exception
     {