From 289c9ebba186e5ec70e8124794fe4c68f670fc8a Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Tue, 1 Jun 2021 11:35:01 +0200 Subject: [PATCH] Make Buffer.ensureWritable throw exceptions consistent with the rest of the API --- .../main/java/io/netty/buffer/api/CompositeBuffer.java | 3 +++ .../java/io/netty/buffer/api/bytebuffer/NioBuffer.java | 3 +++ .../java/io/netty/buffer/api/unsafe/UnsafeBuffer.java | 3 +++ .../java/io/netty/buffer/api/memseg/MemSegBuffer.java | 3 +++ .../io/netty/buffer/api/tests/BufferCompositionTest.java | 2 +- .../io/netty/buffer/api/tests/BufferReadOnlyTest.java | 8 ++++---- .../java/io/netty/buffer/api/tests/BufferTestSupport.java | 7 +++---- 7 files changed, 20 insertions(+), 9 deletions(-) diff --git a/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java b/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java index cf10acd..8d9a097 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java @@ -731,6 +731,9 @@ public final class CompositeBuffer extends ResourceSupport implements Buffer, Re @Override public void ensureWritable(int size, int minimumGrowth, boolean allowCompaction) { + if (!isAccessible()) { + throw bufferIsClosed(this); + } if (!isOwned()) { throw attachTrace(new IllegalStateException( "Buffer is not owned. Only owned buffers can call ensureWritable.")); diff --git a/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java b/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java index 115ecd2..d0920e9 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java @@ -431,6 +431,9 @@ class UnsafeBuffer extends ResourceSupport implements Buff @Override public void ensureWritable(int size, int minimumGrowth, boolean allowCompaction) { + if (!isAccessible()) { + throw bufferIsClosed(this); + } if (!isOwned()) { throw attachTrace(new IllegalStateException( "Buffer is not owned. Only owned buffers can call ensureWritable.")); diff --git a/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java b/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java index c181e48..472d3bf 100644 --- a/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java +++ b/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java @@ -518,6 +518,9 @@ class MemSegBuffer extends ResourceSupport implements Buff @Override public void ensureWritable(int size, int minimumGrowth, boolean allowCompaction) { + if (!isAccessible()) { + throw bufferIsClosed(this); + } if (!isOwned()) { throw attachTrace(new IllegalStateException( "Buffer is not owned. Only owned buffers can call ensureWritable.")); diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java index bc37a46..a01ed25 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java @@ -392,7 +392,7 @@ public class BufferCompositionTest extends BufferTestSupport { Buffer b = allocator.allocate(4).makeReadOnly(); Buffer composite = CompositeBuffer.compose(allocator, a.send(), b.send())) { assertTrue(composite.readOnly()); - verifyWriteInaccessible(composite, BufferReadOnlyException.class, BufferReadOnlyException.class); + verifyWriteInaccessible(composite, BufferReadOnlyException.class); } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java index 78961fe..edffd70 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java @@ -44,7 +44,7 @@ public class BufferReadOnlyTest extends BufferTestSupport { Buffer buf = allocator.allocate(8)) { var b = buf.makeReadOnly(); assertThat(b).isSameAs(buf); - verifyWriteInaccessible(buf, BufferReadOnlyException.class, BufferReadOnlyException.class); + verifyWriteInaccessible(buf, BufferReadOnlyException.class); } } @@ -67,12 +67,12 @@ public class BufferReadOnlyTest extends BufferTestSupport { assertFalse(buf.readOnly()); buf.makeReadOnly(); assertTrue(buf.readOnly()); - verifyWriteInaccessible(buf, BufferReadOnlyException.class, BufferReadOnlyException.class); + verifyWriteInaccessible(buf, BufferReadOnlyException.class); buf.makeReadOnly(); assertTrue(buf.readOnly()); - verifyWriteInaccessible(buf, BufferReadOnlyException.class, BufferReadOnlyException.class); + verifyWriteInaccessible(buf, BufferReadOnlyException.class); } } @@ -85,7 +85,7 @@ public class BufferReadOnlyTest extends BufferTestSupport { var send = buf.send(); try (Buffer receive = send.receive()) { assertTrue(receive.readOnly()); - verifyWriteInaccessible(receive, BufferReadOnlyException.class, BufferReadOnlyException.class); + verifyWriteInaccessible(receive, BufferReadOnlyException.class); } } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java index 7535481..4b5e23f 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java @@ -324,7 +324,7 @@ public abstract class BufferTestSupport { public static void verifyInaccessible(Buffer buf) { verifyReadInaccessible(buf); - verifyWriteInaccessible(buf, BufferClosedException.class, IllegalStateException.class); + verifyWriteInaccessible(buf, BufferClosedException.class); try (BufferAllocator allocator = BufferAllocator.heap(); Buffer target = allocator.allocate(24)) { @@ -375,8 +375,7 @@ public abstract class BufferTestSupport { assertThrows(UnsupportedOperationException.class, () -> buf.getDouble(0)); } - public static void verifyWriteInaccessible(Buffer buf, Class expected, - Class expectedEnsureWritable) { + public static void verifyWriteInaccessible(Buffer buf, Class expected) { assertThrows(expected, () -> buf.writeByte((byte) 32)); assertThrows(expected, () -> buf.writeUnsignedByte(32)); assertThrows(expected, () -> buf.writeChar('3')); @@ -403,7 +402,7 @@ public abstract class BufferTestSupport { assertThrows(expected, () -> buf.setLong(0, 32)); assertThrows(expected, () -> buf.setDouble(0, 32)); - assertThrows(expectedEnsureWritable, () -> buf.ensureWritable(1)); + assertThrows(expected, () -> buf.ensureWritable(1)); final RuntimeException f1e = assertThrows(RuntimeException.class, () -> buf.fill((byte) 0)); assertTrue(f1e instanceof IllegalStateException || f1e instanceof UnsupportedOperationException); try (BufferAllocator allocator = BufferAllocator.heap();