From 9a9ef89cb4d49fb184466b918e9c132db82bced3 Mon Sep 17 00:00:00 2001 From: gregw Date: Tue, 25 Jun 2024 12:00:40 +1000 Subject: [PATCH 1/2] Fix flaky test --- .../eclipse/jetty/ee10/servlet/ContextScopeListenerTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ContextScopeListenerTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ContextScopeListenerTest.java index 66874707141..b35fa93c1a3 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ContextScopeListenerTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ContextScopeListenerTest.java @@ -17,6 +17,7 @@ import java.net.URI; import java.util.Arrays; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; import jakarta.servlet.AsyncContext; @@ -24,6 +25,7 @@ import jakarta.servlet.DispatcherType; import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.awaitility.Awaitility; import org.eclipse.jetty.client.ContentResponse; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.http.HttpStatus; @@ -118,6 +120,7 @@ public class ContextScopeListenerTest URI uri = URI.create("http://localhost:" + _connector.getLocalPort() + "/initialPath"); ContentResponse response = _client.GET(uri); assertThat(response.getStatus(), equalTo(HttpStatus.OK_200)); + Awaitility.waitAtMost(5, TimeUnit.SECONDS).pollInterval(100, TimeUnit.MILLISECONDS).until(() -> _history.size() == 9); assertHistory( "enterScope /initialPath", "doGet", From 718c6fce514053fd5583e7e40ee9a85acbad48f0 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 25 Jun 2024 12:32:29 +1000 Subject: [PATCH 2/2] Content.Source from methods (#11949) Introduce Content.Source.from methods These isolate code from specific implementations (which could even be made internal) --- .../org/eclipse/jetty/http/MultiPart.java | 3 +- .../jetty/http/MultiPartByteRanges.java | 50 ++++++- .../org/eclipse/jetty/io/ByteBufferPool.java | 1 + .../java/org/eclipse/jetty/io/Content.java | 137 ++++++++++++++++++ .../org/eclipse/jetty/io/IOResources.java | 38 +---- .../io/content/InputStreamContentSource.java | 4 +- .../ByteChannelContentSource.java | 18 +-- .../eclipse/jetty/io/ContentSourceTest.java | 2 +- 8 files changed, 198 insertions(+), 55 deletions(-) rename jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/{content => internal}/ByteChannelContentSource.java (93%) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java index dce75b4dca8..452cd394bbd 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java @@ -36,7 +36,6 @@ import java.util.concurrent.ThreadLocalRandom; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.content.ByteBufferContentSource; -import org.eclipse.jetty.io.content.ByteChannelContentSource; import org.eclipse.jetty.io.content.ChunksContentSource; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.IO; @@ -476,7 +475,7 @@ public class MultiPart public Content.Source newContentSource() { // TODO: use a ByteBuffer pool and direct ByteBuffers? - return new ByteChannelContentSource.PathContentSource(getPath()); + return Content.Source.from(getPath()); } @Override diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartByteRanges.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartByteRanges.java index 94aced30bf0..8651bc37a39 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartByteRanges.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartByteRanges.java @@ -24,7 +24,6 @@ import java.util.concurrent.CompletableFuture; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.IOResources; -import org.eclipse.jetty.io.content.ByteChannelContentSource; import org.eclipse.jetty.io.content.ContentSourceCompletableFuture; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.thread.AutoLock; @@ -164,14 +163,55 @@ public class MultiPartByteRanges } /** - *

A specialized {@link org.eclipse.jetty.io.content.ByteChannelContentSource.PathContentSource} - * whose content is sliced by a byte range.

+ *

A specialized {@link Content.Source} + * whose {@link Path} content is sliced by a byte range.

+ * + * @deprecated use {@link Content.Source#from(ByteBufferPool.Sized, Path, long, long)} */ - public static class PathContentSource extends ByteChannelContentSource.PathContentSource + @Deprecated(forRemoval = true, since = "12.0.11") + public static class PathContentSource implements Content.Source { + private final Content.Source contentSource; + public PathContentSource(Path path, ByteRange byteRange) { - super(new ByteBufferPool.Sized(null), path, byteRange.first(), byteRange.getLength()); + contentSource = Content.Source.from(null, path, byteRange.first(), byteRange.getLength()); + } + + @Override + public void demand(Runnable demandCallback) + { + contentSource.demand(demandCallback); + } + + @Override + public void fail(Throwable failure) + { + contentSource.fail(failure); + } + + @Override + public void fail(Throwable failure, boolean last) + { + contentSource.fail(failure, last); + } + + @Override + public long getLength() + { + return contentSource.getLength(); + } + + @Override + public Content.Chunk read() + { + return contentSource.read(); + } + + @Override + public boolean rewind() + { + return contentSource.rewind(); } } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java index 671e174ba00..2233ae72bf4 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java @@ -47,6 +47,7 @@ import org.eclipse.jetty.util.BufferUtil; public interface ByteBufferPool { ByteBufferPool NON_POOLING = new NonPooling(); + ByteBufferPool.Sized SIZED_NON_POOLING = new Sized(ByteBufferPool.NON_POOLING); /** *

Acquires a {@link RetainableByteBuffer} from this pool.

diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java index abde44ec95a..e491e3e34bc 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java @@ -17,19 +17,25 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.nio.ByteBuffer; +import java.nio.channels.ByteChannel; +import java.nio.channels.SeekableByteChannel; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.nio.file.Path; import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Flow; import java.util.function.Consumer; import org.eclipse.jetty.io.content.BufferedContentSink; +import org.eclipse.jetty.io.content.ByteBufferContentSource; import org.eclipse.jetty.io.content.ContentSinkOutputStream; import org.eclipse.jetty.io.content.ContentSinkSubscriber; import org.eclipse.jetty.io.content.ContentSourceInputStream; import org.eclipse.jetty.io.content.ContentSourcePublisher; +import org.eclipse.jetty.io.content.InputStreamContentSource; import org.eclipse.jetty.io.internal.ByteBufferChunk; +import org.eclipse.jetty.io.internal.ByteChannelContentSource; import org.eclipse.jetty.io.internal.ContentCopier; import org.eclipse.jetty.io.internal.ContentSourceByteBuffer; import org.eclipse.jetty.io.internal.ContentSourceConsumer; @@ -151,6 +157,137 @@ public class Content */ public interface Source { + /** + * Create a {@code Content.Source} from zero or more {@link ByteBuffer}s + * @param byteBuffers The {@link ByteBuffer}s to use as the source. + * @return A {@code Content.Source} + */ + static Content.Source from(ByteBuffer... byteBuffers) + { + return new ByteBufferContentSource(byteBuffers); + } + + /** + * Create a {@code Content.Source} from a {@link Path}. + * @param path The {@link Path}s to use as the source. + * @return A {@code Content.Source} + */ + static Content.Source from(Path path) + { + return from(null, path, 0, -1); + } + + /** + * Create a {@code Content.Source} from a {@link Path}. + * @param path The {@link Path}s to use as the source. + * @param offset The offset in bytes from which to start the source + * @param length The length in bytes of the source. + * @return A {@code Content.Source} + */ + static Content.Source from(Path path, long offset, long length) + { + return from(null, path, offset, length); + } + + /** + * Create a {@code Content.Source} from a {@link Path}. + * @param byteBufferPool The {@link org.eclipse.jetty.io.ByteBufferPool.Sized} to use for any internal buffers. + * @param path The {@link Path}s to use as the source. + * @return A {@code Content.Source} + */ + static Content.Source from(ByteBufferPool.Sized byteBufferPool, Path path) + { + return from(byteBufferPool, path, 0, -1); + } + + /** + * Create a {@code Content.Source} from a {@link Path}. + * @param byteBufferPool The {@link org.eclipse.jetty.io.ByteBufferPool.Sized} to use for any internal buffers. + * @param path The {@link Path}s to use as the source. + * @param offset The offset in bytes from which to start the source + * @param length The length in bytes of the source. + * @return A {@code Content.Source} + */ + static Content.Source from(ByteBufferPool.Sized byteBufferPool, Path path, long offset, long length) + { + return new ByteChannelContentSource.PathContentSource(byteBufferPool, path, offset, length); + } + + /** + * Create a {@code Content.Source} from a {@link ByteChannel}. + * @param byteBufferPool The {@link org.eclipse.jetty.io.ByteBufferPool.Sized} to use for any internal buffers. + * @param byteChannel The {@link ByteChannel}s to use as the source. + * @return A {@code Content.Source} + */ + static Content.Source from(ByteBufferPool.Sized byteBufferPool, ByteChannel byteChannel) + { + return new ByteChannelContentSource(byteBufferPool, byteChannel); + } + + /** + * Create a {@code Content.Source} from a {@link ByteChannel}. + * @param byteBufferPool The {@link org.eclipse.jetty.io.ByteBufferPool.Sized} to use for any internal buffers. + * @param seekableByteChannel The {@link ByteChannel}s to use as the source. + * @param offset The offset in bytes from which to start the source + * @param length The length in bytes of the source. + * @return A {@code Content.Source} + */ + static Content.Source from(ByteBufferPool.Sized byteBufferPool, SeekableByteChannel seekableByteChannel, long offset, long length) + { + return new ByteChannelContentSource(byteBufferPool, seekableByteChannel, offset, length); + } + + static Content.Source from(InputStream inputStream) + { + return from(null, inputStream); + } + + /** + * Create a {@code Content.Source} from a {@link Path}. + * @param byteBufferPool The {@link org.eclipse.jetty.io.ByteBufferPool.Sized} to use for any internal buffers. + * @param inputStream The {@link InputStream}s to use as the source. + * @return A {@code Content.Source} + */ + static Content.Source from(ByteBufferPool.Sized byteBufferPool, InputStream inputStream) + { + return new InputStreamContentSource(inputStream, byteBufferPool); + } + + /** + * Create a {@code Content.Source} from a {@link Path}. + * @param byteBufferPool The {@link org.eclipse.jetty.io.ByteBufferPool.Sized} to use for any internal buffers. + * @param inputStream The {@link InputStream}s to use as the source. + * @param offset The offset in bytes from which to start the source + * @param length The length in bytes of the source. + * @return A {@code Content.Source} + */ + static Content.Source from(ByteBufferPool.Sized byteBufferPool, InputStream inputStream, long offset, long length) + { + return new InputStreamContentSource(inputStream, byteBufferPool) + { + private long skip = offset; + private long toRead = length; + + @Override + protected int fillBufferFromInputStream(InputStream inputStream, byte[] buffer) throws IOException + { + if (skip > 0) + { + inputStream.skipNBytes(skip); + skip = 0; + } + + if (toRead == 0) + return -1; + int toReadInt = (int)Math.min(Integer.MAX_VALUE, toRead); + int len = toReadInt > -1 ? Math.min(toReadInt, buffer.length) : buffer.length; + int read = inputStream.read(buffer, 0, len); + toRead -= read; + return read; + } + }; + } + /** *

Reads, non-blocking, the whole content source into a {@link ByteBuffer}.

* diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/IOResources.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/IOResources.java index f0d52ffbf65..eb811845516 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/IOResources.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/IOResources.java @@ -21,7 +21,6 @@ import java.nio.file.Files; import java.nio.file.Path; import org.eclipse.jetty.io.content.ByteBufferContentSource; -import org.eclipse.jetty.io.content.ByteChannelContentSource; import org.eclipse.jetty.io.content.InputStreamContentSource; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; @@ -137,7 +136,7 @@ public class IOResources Path path = resource.getPath(); if (path != null) { - return new ByteChannelContentSource.PathContentSource(new ByteBufferPool.Sized(bufferPool, direct, bufferSize), path); + return Content.Source.from(new ByteBufferPool.Sized(bufferPool, direct, bufferSize), path, 0, -1); } if (resource instanceof MemoryResource memoryResource) { @@ -181,12 +180,12 @@ public class IOResources Path path = resource.getPath(); if (path != null) { - return new ByteChannelContentSource.PathContentSource(new ByteBufferPool.Sized(bufferPool, direct, bufferSize), path, first, length); + return Content.Source.from(new ByteBufferPool.Sized(bufferPool, direct, bufferSize), path, first, length); } // Try an optimization for MemoryResource. if (resource instanceof MemoryResource memoryResource) - return new ByteBufferContentSource(ByteBuffer.wrap(memoryResource.getBytes())); + return Content.Source.from(ByteBuffer.wrap(memoryResource.getBytes())); // Fallback to InputStream. try @@ -194,7 +193,7 @@ public class IOResources InputStream inputStream = resource.newInputStream(); if (inputStream == null) throw new IllegalArgumentException("Resource does not support InputStream: " + resource); - return new RangedInputStreamContentSource(inputStream, new ByteBufferPool.Sized(bufferPool, direct, bufferSize), first, length); + return Content.Source.from(new ByteBufferPool.Sized(bufferPool, direct, bufferSize), inputStream, first, length); } catch (IOException e) { @@ -411,33 +410,4 @@ public class IOResources super.onCompleteFailure(x); } } - - /** - *

A specialized {@link InputStreamContentSource} - * whose content is sliced by a byte range.

- */ - private static class RangedInputStreamContentSource extends InputStreamContentSource - { - private long toRead; - - public RangedInputStreamContentSource(InputStream inputStream, ByteBufferPool bufferPool, long first, long length) throws IOException - { - super(inputStream, bufferPool); - inputStream.skipNBytes(first); - // TODO perform sanity checks on length? - this.toRead = length; - } - - @Override - protected int fillBufferFromInputStream(InputStream inputStream, byte[] buffer) throws IOException - { - if (toRead == 0) - return -1; - int toReadInt = (int)Math.min(Integer.MAX_VALUE, toRead); - int len = toReadInt > -1 ? Math.min(toReadInt, buffer.length) : buffer.length; - int read = inputStream.read(buffer, 0, len); - toRead -= read; - return read; - } - } } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/InputStreamContentSource.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/InputStreamContentSource.java index 61d0785deb7..bc14ba9d9e9 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/InputStreamContentSource.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/InputStreamContentSource.java @@ -47,7 +47,7 @@ public class InputStreamContentSource implements Content.Source public InputStreamContentSource(InputStream inputStream) { - this(inputStream, new ByteBufferPool.Sized(null)); + this(inputStream, null); } public InputStreamContentSource(InputStream inputStream, ByteBufferPool bufferPool) @@ -58,7 +58,7 @@ public class InputStreamContentSource implements Content.Source public InputStreamContentSource(InputStream inputStream, ByteBufferPool.Sized bufferPool) { this.inputStream = Objects.requireNonNull(inputStream); - this.bufferPool = Objects.requireNonNull(bufferPool); + this.bufferPool = Objects.requireNonNullElse(bufferPool, ByteBufferPool.SIZED_NON_POOLING); } public int getBufferSize() diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ByteChannelContentSource.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java similarity index 93% rename from jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ByteChannelContentSource.java rename to jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java index db0bed464f6..65336ac6520 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ByteChannelContentSource.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteChannelContentSource.java @@ -11,7 +11,7 @@ // ======================================================================== // -package org.eclipse.jetty.io.content; +package org.eclipse.jetty.io.internal; import java.io.IOException; import java.nio.ByteBuffer; @@ -51,7 +51,7 @@ public class ByteChannelContentSource implements Content.Source public ByteChannelContentSource(SeekableByteChannel seekableByteChannel, long offset, long length) { - this(new ByteBufferPool.Sized(null), seekableByteChannel, offset, length); + this(null, seekableByteChannel, offset, length); } public ByteChannelContentSource(ByteBufferPool.Sized byteBufferPool, SeekableByteChannel seekableByteChannel, long offset, long length) @@ -73,7 +73,7 @@ public class ByteChannelContentSource implements Content.Source public ByteChannelContentSource(ByteChannel byteChannel) { - this(new ByteBufferPool.Sized(null), byteChannel, -1L, -1L); + this(null, byteChannel, -1L, -1L); } public ByteChannelContentSource(ByteBufferPool.Sized byteBufferPool, ByteChannel byteChannel) @@ -83,7 +83,7 @@ public class ByteChannelContentSource implements Content.Source private ByteChannelContentSource(ByteBufferPool.Sized byteBufferPool, ByteChannel byteChannel, long offset, long length) { - _byteBufferPool = Objects.requireNonNull(byteBufferPool); + _byteBufferPool = Objects.requireNonNullElse(byteBufferPool, ByteBufferPool.SIZED_NON_POOLING); _byteChannel = byteChannel; _offset = offset < 0 ? 0 : offset; _length = length; @@ -246,28 +246,24 @@ public class ByteChannelContentSource implements Content.Source /** * A {@link ByteChannelContentSource} for a {@link Path} - * @deprecated To be replaced by an updated {@link org.eclipse.jetty.io.content.PathContentSource} in 12.1.0 */ - @Deprecated(forRemoval = true, since = "12.0.11") public static class PathContentSource extends ByteChannelContentSource { private final Path _path; public PathContentSource(Path path) { - super(new ByteBufferPool.Sized(null), null, 0, size(path)); - _path = path; + this(null, path, 0, -1); } public PathContentSource(ByteBufferPool.Sized byteBufferPool, Path path) { - super(byteBufferPool, null, 0, size(path)); - _path = path; + this(byteBufferPool, path, 0, -1); } public PathContentSource(ByteBufferPool.Sized byteBufferPool, Path path, long offset, long length) { - super(byteBufferPool, null, offset, length); + super(byteBufferPool, null, offset, length < 0 ? size(path) : length); _path = path; } diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTest.java index ae20cf57d00..daf6f5967c6 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTest.java @@ -37,11 +37,11 @@ import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jetty.io.content.AsyncContent; import org.eclipse.jetty.io.content.ByteBufferContentSource; -import org.eclipse.jetty.io.content.ByteChannelContentSource; import org.eclipse.jetty.io.content.ContentSourceInputStream; import org.eclipse.jetty.io.content.ContentSourceTransformer; import org.eclipse.jetty.io.content.InputStreamContentSource; import org.eclipse.jetty.io.content.PathContentSource; +import org.eclipse.jetty.io.internal.ByteChannelContentSource; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback;