Merge pull request #6456 from eclipse/jetty-9.4.x-6383-FileBufferedResponseHandler

Merge fix to BufferedResponseHandlerTest to 9.4.x

Author: lachlan-roberts

jetty-server/src/main/java/org/eclipse/jetty/server/handler/BufferedResponseHandler.java

@@ -209,7 +209,7 @@ protected interface BufferedInterceptor extends HttpOutput.Interceptor
     {
     }
 
-    private class ArrayBufferedInterceptor implements BufferedInterceptor
+    class ArrayBufferedInterceptor implements BufferedInterceptor
     {
         private final Interceptor _next;
         private final HttpChannel _channel;

jetty-server/src/main/java/org/eclipse/jetty/server/handler/FileBufferedResponseHandler.java

@@ -76,7 +76,7 @@ protected BufferedInterceptor newBufferedInterceptor(HttpChannel httpChannel, In
         return new FileBufferedInterceptor(httpChannel, interceptor);
     }
 
-    private class FileBufferedInterceptor implements BufferedResponseHandler.BufferedInterceptor
+    class FileBufferedInterceptor implements BufferedResponseHandler.BufferedInterceptor
     {
         private static final int MAX_MAPPED_BUFFER_SIZE = Integer.MAX_VALUE / 2;
 
@@ -111,7 +111,7 @@ public void resetBuffer()
             BufferedInterceptor.super.resetBuffer();
         }
 
-        private void dispose()
+        protected void dispose()
         {
             IO.close(_fileOutputStream);
             _fileOutputStream = null;

jetty-server/src/test/java/org/eclipse/jetty/server/FileBufferedResponseHandlerTest.java renamed to jetty-server/src/test/java/org/eclipse/jetty/server/handler/FileBufferedResponseHandlerTest.java

@@ -16,7 +16,7 @@
 //  ========================================================================
 //
 
-package org.eclipse.jetty.server;
+package org.eclipse.jetty.server.handler;
 
 import java.io.File;
 import java.io.IOException;
@@ -30,6 +30,7 @@
 import java.time.Duration;
 import java.util.Random;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
 import javax.servlet.ServletException;
@@ -39,9 +40,14 @@
 
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpTester;
-import org.eclipse.jetty.server.handler.AbstractHandler;
-import org.eclipse.jetty.server.handler.FileBufferedResponseHandler;
-import org.eclipse.jetty.server.handler.HandlerCollection;
+import org.eclipse.jetty.server.HttpChannel;
+import org.eclipse.jetty.server.HttpConfiguration;
+import org.eclipse.jetty.server.HttpConnectionFactory;
+import org.eclipse.jetty.server.HttpOutput;
+import org.eclipse.jetty.server.LocalConnector;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.toolchain.test.FS;
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.eclipse.jetty.util.Callback;
@@ -63,6 +69,7 @@ public class FileBufferedResponseHandlerTest
 {
     private static final Logger LOG = Log.getLogger(FileBufferedResponseHandlerTest.class);
 
+    private final CountDownLatch _disposeLatch = new CountDownLatch(1);
     private Server _server;
     private LocalConnector _localConnector;
     private ServerConnector _serverConnector;
@@ -86,7 +93,22 @@ public void before() throws Exception
         _serverConnector = new ServerConnector(_server, new HttpConnectionFactory(config));
         _server.addConnector(_serverConnector);
 
-        _bufferedHandler = new FileBufferedResponseHandler();
+        _bufferedHandler = new FileBufferedResponseHandler()
+        {
+            @Override
+            protected BufferedInterceptor newBufferedInterceptor(HttpChannel httpChannel, HttpOutput.Interceptor interceptor)
+            {
+                return new FileBufferedInterceptor(httpChannel, interceptor)
+                {
+                    @Override
+                    protected void dispose()
+                    {
+                        super.dispose();
+                        _disposeLatch.countDown();
+                    }
+                };
+            }
+        };
         _bufferedHandler.setTempDir(_testDir);
         _bufferedHandler.getPathIncludeExclude().include("/include/*");
         _bufferedHandler.getPathIncludeExclude().exclude("*.exclude");
@@ -157,6 +179,8 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
         assertThat(response.getStatus(), is(HttpStatus.OK_200));
         assertThat(responseContent, containsString("Committed: false"));
         assertThat(responseContent, containsString("NumFiles: 1"));
+
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }
 
@@ -249,6 +273,8 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
         assertThat(responseContent, containsString("NumFilesBeforeFlush: 0"));
         assertThat(responseContent, containsString("Committed: false"));
         assertThat(responseContent, containsString("NumFiles: 1"));
+
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }
 
@@ -279,6 +305,8 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
         assertThat(response.getStatus(), is(HttpStatus.OK_200));
         assertThat(responseContent, not(containsString("writtenAfterClose")));
         assertThat(responseContent, containsString("NumFiles: 1"));
+
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }
 
@@ -339,6 +367,8 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
         // The flush should not create the file unless there is content to write.
         assertThat(response.getStatus(), is(HttpStatus.OK_200));
         assertThat(responseContent, containsString("NumFiles: 0"));
+
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }
 
@@ -378,6 +408,8 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
         assertThat(responseContent, containsString("NumFilesBeforeReset: 1"));
         assertThat(responseContent, containsString("NumFilesAfterReset: 0"));
         assertThat(responseContent, containsString("NumFilesAfterWrite: 1"));
+
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }
 
@@ -451,6 +483,8 @@ public boolean content(ByteBuffer ref)
         assertThat(response.get("NumFiles"), is("1"));
         assertThat(response.get("FileSize"), is(Long.toString(fileSize)));
         assertThat(received.get(), is(fileSize));
+
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }
 
@@ -531,6 +565,7 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
         assertThat(error.getMessage(), containsString("intentionally throwing from interceptor"));
 
         // All files were deleted.
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }
 
@@ -579,6 +614,7 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
         assertThat(error, instanceOf(NoSuchFileException.class));
 
         // No files were created.
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }