Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup of ByteBufferAccumulator (approach 2) #11094

Closed
wants to merge 57 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
061d8ac
Review ByteBufferAccumulator #10541
gregw Dec 13, 2023
59a9307
Review ByteBufferAccumulator #10541
gregw Dec 14, 2023
854e92c
Review ByteBufferAccumulator #10541
gregw Dec 14, 2023
f75ded5
Review ByteBufferAccumulator #10541
gregw Dec 14, 2023
2970ce6
Review ByteBufferAccumulator #10541
gregw Dec 14, 2023
4e0a106
Merge branch 'jetty-12.0.x' into fix/jetty-12/10541/byteBufferAccumul…
gregw Dec 17, 2023
30102db
Review ByteBufferAccumulator #10541
gregw Dec 17, 2023
9740820
Review ByteBufferAccumulator #10541
gregw Dec 19, 2023
2ea7810
Review ByteBufferAccumulator #10541
gregw Dec 20, 2023
c70df80
Review ByteBufferAccumulator #10541
gregw Dec 21, 2023
8e43335
Merge branch 'jetty-12.0.x' into fix/jetty-12/10541/byteBufferAccumul…
gregw Dec 23, 2023
2faa60e
Review ByteBufferAccumulator #10541
gregw Dec 24, 2023
aeb164a
Review ByteBufferAccumulator #10541
gregw Dec 24, 2023
b514f19
Merge branch 'jetty-12.0.x' into fix/jetty-12/10541/byteBufferAccumul…
gregw Jan 16, 2024
46ec1f8
Merge branch 'jetty-12.0.x' into fix/jetty-12/10541/byteBufferAccumul…
gregw Jan 16, 2024
1fa20a4
Merge branch 'fix/jetty-12/10541/byteBufferAccumulator' into fix/jett…
gregw Jan 16, 2024
158b108
Review ByteBufferAccumulator #10541
gregw Jan 16, 2024
c68da10
Review ByteBufferAccumulator #10541
gregw Jan 17, 2024
7a91e77
New implementation of BufferedContentSink
gregw Jan 20, 2024
7339dcd
javadoc
gregw Jan 20, 2024
089145b
do not clear on release
gregw Jan 20, 2024
df3d950
More fixes to BufferedContentSink
gregw Jan 21, 2024
475e570
forced flush
gregw Jan 21, 2024
8a30dce
Fixed bug in BufferUtil
gregw Jan 21, 2024
4b6b2a8
document long to int conversion
gregw Jan 21, 2024
58ca19f
Merge branch 'jetty-12.0.x' into fix/jetty-12/10541/byteBufferAccumul…
gregw Feb 19, 2024
4852058
Merge branch 'jetty-12.0.x' into fix/jetty-12/10541/byteBufferAccumul…
gregw Mar 1, 2024
6f8a2d9
removed byte[] append API
gregw Mar 1, 2024
44dcac8
removed writeTo(OutputStream)
gregw Mar 1, 2024
c48fae4
updates from review
gregw Mar 1, 2024
ed608ed
Merge branch 'jetty-12.0.x' into fix/jetty-12/10541/byteBufferAccumul…
gregw Mar 5, 2024
6c9e40f
Don't modify deprecated class
gregw Mar 5, 2024
3970a3a
More updates after review
gregw Mar 5, 2024
eb3097a
Merge branch 'jetty-12.0.x' into fix/jetty-12/10541/byteBufferAccumul…
gregw Mar 7, 2024
22ec9b0
updated after merge
gregw Mar 8, 2024
10dd573
updated after merge
gregw Mar 8, 2024
2707777
replace ChunkAccumulator usages with ContentSourceRetainableByteBuffe…
lorban Mar 8, 2024
e06c10d
fix maxSize check, add test, add javadoc
lorban Mar 8, 2024
1461ec6
fix Content.Sink.from(OutputStream) and add test
lorban Mar 8, 2024
07c8cbf
merged copy and getByteBuffer implementation
gregw Mar 8, 2024
7dae7b7
handle 0 capacity
gregw Mar 9, 2024
9a91601
do not copy in append()
lorban Mar 18, 2024
75236c1
add tests for append(RBB) variant
lorban Mar 18, 2024
00c08eb
add tests for copy + fix bugs
lorban Mar 18, 2024
a56693f
improve javadoc
lorban Mar 18, 2024
9a41edc
fix javadoc
lorban Mar 18, 2024
e7eec0e
improve tests
lorban Mar 18, 2024
0adadf8
fix releasing of appended RBB
lorban Mar 19, 2024
ccca9aa
fix leaks and leak detection in tests
lorban Mar 19, 2024
938408a
Merge branch 'jetty-12.0.x' into fix/jetty-12/10541/byteBufferAccumul…
gregw Mar 20, 2024
48a6c65
Update from review
gregw Mar 20, 2024
fb5c5a3
Update from review
gregw Mar 20, 2024
5d39b18
Merge branch 'jetty-12.0.x' into fix/jetty-12/10541/byteBufferAccumul…
gregw Mar 22, 2024
d1f86d0
Merge branch 'jetty-12.0.x' into fix/jetty-12/10541/byteBufferAccumul…
gregw Mar 27, 2024
5936d4d
improved javadoc and removed asReadOnly
gregw Mar 27, 2024
3c36697
Merge remote-tracking branch 'origin/jetty-12.0.x' into fix/jetty-12/…
lorban Mar 28, 2024
6252858
Merge branch 'jetty-12.0.x' into fix/jetty-12/10541/byteBufferAccumul…
gregw Mar 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -889,52 +889,6 @@ default Throwable getFailure()
*/
boolean isLast();

/**
* <p>Copies the bytes from this Chunk to the given byte array.</p>
*
* @param bytes the byte array to copy the bytes into
* @param offset the offset within the byte array
* @param length the maximum number of bytes to copy
* @return the number of bytes actually copied
*/
default int get(byte[] bytes, int offset, int length)
{
ByteBuffer b = getByteBuffer();
if (b == null || !b.hasRemaining())
return 0;
length = Math.min(length, b.remaining());
b.get(bytes, offset, length);
return length;
}

/**
* <p>Skips, advancing the ByteBuffer position, the given number of bytes.</p>
*
* @param length the maximum number of bytes to skip
* @return the number of bytes actually skipped
*/
default int skip(int length)
{
if (length == 0)
return 0;
ByteBuffer byteBuffer = getByteBuffer();
length = Math.min(byteBuffer.remaining(), length);
byteBuffer.position(byteBuffer.position() + length);
return length;
}

/**
* @return an immutable version of this Chunk
*/
default Chunk asReadOnly()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I think this method can only be deprecated for removal, as it is currently part of the API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good point

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... unless we make this PR a 12.1 thing.... in which case we can deprecate in 12.0.x and remove here.

{
if (getByteBuffer().isReadOnly())
return this;
if (canRetain())
return asChunk(getByteBuffer().asReadOnlyBuffer(), isLast(), this);
return from(getByteBuffer().asReadOnlyBuffer(), isLast());
}

/**
* <p>Implementations of this interface may process {@link Chunk}s being copied by the
* {@link Content#copy(Source, Sink, Processor, Callback)} method, so that
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package org.eclipse.jetty.io;

import java.nio.ByteBuffer;
import java.nio.ReadOnlyBufferException;
import java.util.ArrayList;
import java.util.List;

Expand Down Expand Up @@ -221,9 +222,10 @@ default void clear()
* Copies the contents of the given byte buffer at the end of this buffer.
* @param bytes the byte buffer to copy from.
* @return true if all bytes of the given buffer were copied, false otherwise.
* @throws ReadOnlyBufferException if the buffer is read only
* @see BufferUtil#append(ByteBuffer, ByteBuffer)
*/
default boolean append(ByteBuffer bytes)
default boolean append(ByteBuffer bytes) throws ReadOnlyBufferException
{
BufferUtil.append(getByteBuffer(), bytes);
return !bytes.hasRemaining();
Expand All @@ -233,13 +235,48 @@ default boolean append(ByteBuffer bytes)
* Copies the contents of the given retainable byte buffer at the end of this buffer.
* @param bytes the retainable byte buffer to copy from.
* @return true if all bytes of the given buffer were copied, false otherwise.
* @throws ReadOnlyBufferException if the buffer is read only
* @see BufferUtil#append(ByteBuffer, ByteBuffer)
*/
default boolean append(RetainableByteBuffer bytes)
default boolean append(RetainableByteBuffer bytes) throws ReadOnlyBufferException
{
return bytes.remaining() == 0 || append(bytes.getByteBuffer());
}

/**
* <p>Copies the bytes from this Chunk to the given byte array.</p>
*
* @param bytes the byte array to copy the bytes into
* @param offset the offset within the byte array
* @param length the maximum number of bytes to copy
* @return the number of bytes actually copied
*/
default int get(byte[] bytes, int offset, int length)
{
ByteBuffer b = getByteBuffer();
if (b == null || !b.hasRemaining())
return 0;
length = Math.min(length, b.remaining());
b.get(bytes, offset, length);
return length;
}

/**
* <p>Skips, advancing the ByteBuffer position, the given number of bytes.</p>
*
* @param length the maximum number of bytes to skip
* @return the number of bytes actually skipped
*/
default int skip(int length)
{
if (length == 0)
return 0;
ByteBuffer byteBuffer = getByteBuffer();
length = Math.min(byteBuffer.remaining(), length);
byteBuffer.position(byteBuffer.position() + length);
return length;
}

/**
* Copies the contents of this retainable byte buffer at the end of the given byte buffer.
* @param toInfillMode the destination buffer.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,43 +14,18 @@
package org.eclipse.jetty.io;

import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;

import org.eclipse.jetty.util.BufferUtil;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.sameInstance;

public class ContentTest
{
@Test
public void testAsReadOnly()
{
assertThat(Content.Chunk.EOF.asReadOnly(), sameInstance(Content.Chunk.EOF));
assertThat(Content.Chunk.EMPTY.asReadOnly(), sameInstance(Content.Chunk.EMPTY));

assertThat(Content.Chunk.from(BufferUtil.EMPTY_BUFFER, true).asReadOnly(), sameInstance(Content.Chunk.EOF));
assertThat(Content.Chunk.from(BufferUtil.EMPTY_BUFFER, false).asReadOnly(), sameInstance(Content.Chunk.EMPTY));

Content.Chunk failureChunk = Content.Chunk.from(new NumberFormatException());
assertThat(failureChunk.asReadOnly(), sameInstance(failureChunk));

Content.Chunk chunk = Content.Chunk.from(ByteBuffer.wrap(new byte[1]).asReadOnlyBuffer(), false);
assertThat(chunk.asReadOnly(), sameInstance(chunk));

Content.Chunk rwChunk = Content.Chunk.from(ByteBuffer.wrap("abc".getBytes(StandardCharsets.US_ASCII)), false);
Content.Chunk roChunk = rwChunk.asReadOnly();
assertThat(rwChunk, not(sameInstance(roChunk)));
assertThat(BufferUtil.toString(rwChunk.getByteBuffer(), StandardCharsets.US_ASCII), equalTo(BufferUtil.toString(roChunk.getByteBuffer(), StandardCharsets.US_ASCII)));
}

@Test
public void testFromEmptyByteBufferWithoutReleaser()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ private void notifyOnRequestRead(Request wrapped, Content.Chunk chunk)
{
try
{
onRequestRead(wrapped, chunk == null ? null : chunk.asReadOnly());
onRequestRead(wrapped, chunk);
}
catch (Throwable x)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.nio.BufferOverflowException;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.nio.ReadOnlyBufferException;
import java.nio.channels.FileChannel;
import java.nio.channels.FileChannel.MapMode;
import java.nio.channels.ReadableByteChannel;
Expand Down Expand Up @@ -431,8 +432,9 @@ public static boolean compact(ByteBuffer buffer)
* @param from Buffer to take bytes from in flush mode, whose position is modified with the bytes taken.
* @param to Buffer to put bytes to in fill mode.
* @return number of bytes moved
* @throws ReadOnlyBufferException if the buffer is read only
*/
public static int put(ByteBuffer from, ByteBuffer to)
public static int put(ByteBuffer from, ByteBuffer to) throws ReadOnlyBufferException
{
int length = from.remaining();
if (length == 0)
Expand Down Expand Up @@ -469,8 +471,9 @@ public static int put(ByteBuffer from, ByteBuffer to)
* @param off offset into byte
* @param len length to append
* @throws BufferOverflowException if unable to append buffer due to space limits
* @throws ReadOnlyBufferException if the buffer is read only
*/
public static void append(ByteBuffer to, byte[] b, int off, int len) throws BufferOverflowException
public static void append(ByteBuffer to, byte[] b, int off, int len) throws BufferOverflowException, ReadOnlyBufferException
{
int pos = flipToFill(to);
try
Expand All @@ -489,8 +492,9 @@ public static void append(ByteBuffer to, byte[] b, int off, int len) throws Buff
* @param to Buffer is flush mode
* @param b bytes to append
* @throws BufferOverflowException if unable to append buffer due to space limits
* @throws ReadOnlyBufferException if the buffer is read only
*/
public static void append(ByteBuffer to, byte[] b) throws BufferOverflowException
public static void append(ByteBuffer to, byte[] b) throws BufferOverflowException, ReadOnlyBufferException
{
append(to, b, 0, b.length);
}
Expand All @@ -501,8 +505,9 @@ public static void append(ByteBuffer to, byte[] b) throws BufferOverflowExceptio
* @param to Buffer is flush mode
* @param s String to append as UTF8
* @throws BufferOverflowException if unable to append buffer due to space limits
* @throws ReadOnlyBufferException if the buffer is read only
*/
public static void append(ByteBuffer to, String s) throws BufferOverflowException
public static void append(ByteBuffer to, String s) throws BufferOverflowException, ReadOnlyBufferException
{
byte[] b = s.getBytes(StandardCharsets.UTF_8);
append(to, b, 0, b.length);
Expand All @@ -514,8 +519,9 @@ public static void append(ByteBuffer to, String s) throws BufferOverflowExceptio
* @param to Buffer is flush mode
* @param b byte to append
* @throws BufferOverflowException if unable to append buffer due to space limits
* @throws ReadOnlyBufferException if the buffer is read only
*/
public static void append(ByteBuffer to, byte b)
public static void append(ByteBuffer to, byte b) throws BufferOverflowException, ReadOnlyBufferException
{
int pos = flipToFill(to);
try
Expand All @@ -534,8 +540,9 @@ public static void append(ByteBuffer to, byte b)
* @param to Buffer in flush mode, whose position will be incremented by the number of bytes appended
* @param b buffer to append to in flush mode, whose limit will be incremented by the number of bytes appended.
* @return The number of bytes appended.
* @throws ReadOnlyBufferException if the buffer is read only
*/
public static int append(ByteBuffer to, ByteBuffer b)
public static int append(ByteBuffer to, ByteBuffer b) throws ReadOnlyBufferException
{
int pos = flipToFill(to);
try
Expand Down
Loading