From d0d929bc55f84151f0950dbd510120c804a1da68 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Tue, 1 Jun 2021 12:04:57 +0200 Subject: [PATCH] Align the remaining exception expectations and fix MemSegBuffer and CompositeBuffer to throw the correct exceptions --- .../io/netty/buffer/api/CompositeBuffer.java | 5 +- .../netty/buffer/api/memseg/MemSegBuffer.java | 6 ++ .../api/tests/BufferCompositionTest.java | 7 +-- .../buffer/api/tests/BufferTestSupport.java | 58 +++++++++---------- 4 files changed, 41 insertions(+), 35 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 8d9a097..c5c56e2 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 @@ -808,8 +808,11 @@ public final class CompositeBuffer extends ResourceSupport extension) { Objects.requireNonNull(extension, "Extension buffer cannot be null."); Buffer buffer = extension.receive(); - if (!isOwned()) { + if (!isAccessible() || !isOwned()) { buffer.close(); + if (!isAccessible()) { + throw bufferIsClosed(this); + } throw new IllegalStateException("This buffer cannot be extended because it is not in an owned state."); } if (bufs.length > 0 && buffer.order() != order()) { 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 472d3bf..140969d 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 @@ -214,6 +214,9 @@ class MemSegBuffer extends ResourceSupport implements Buff @Override public Buffer fill(byte value) { + if (!isAccessible()) { + throw bufferIsClosed(this); + } checkSet(0, capacity()); seg.fill(value); return this; @@ -288,6 +291,9 @@ class MemSegBuffer extends ResourceSupport implements Buff @Override public long nativeAddress() { + if (!isAccessible()) { + throw bufferIsClosed(this); + } if (seg.isNative()) { return seg.address().toRawLongValue(); } 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 a01ed25..2814b77 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 @@ -17,6 +17,7 @@ package io.netty.buffer.api.tests; import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; +import io.netty.buffer.api.BufferClosedException; import io.netty.buffer.api.BufferReadOnlyException; import io.netty.buffer.api.CompositeBuffer; import io.netty.buffer.api.Send; @@ -69,7 +70,7 @@ public class BufferCompositionTest extends BufferTestSupport { CompositeBuffer bufA = CompositeBuffer.compose(allocator, allocator.allocate(4).send()); Send sendA = bufA.send(); try { - assertThrows(IllegalStateException.class, () -> bufA.extendWith(sendA)); + assertThrows(BufferClosedException.class, () -> bufA.extendWith(sendA)); } finally { sendA.discard(); } @@ -151,9 +152,7 @@ public class BufferCompositionTest extends BufferTestSupport { composite = CompositeBuffer.compose(allocator, a.send()); } try (composite) { - var exc = assertThrows(IllegalStateException.class, - () -> composite.extendWith(composite.send())); - assertThat(exc).hasMessageContaining("cannot be extended"); + assertThrows(BufferClosedException.class, () -> composite.extendWith(composite.send())); } } } 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 4b5e23f..9a5ccff 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 @@ -332,15 +332,14 @@ public abstract class BufferTestSupport { assertThrows(BufferClosedException.class, () -> buf.copyInto(0, new byte[1], 0, 1)); assertThrows(BufferClosedException.class, () -> buf.copyInto(0, ByteBuffer.allocate(1), 0, 1)); if (CompositeBuffer.isComposite(buf)) { - assertThrows(IllegalStateException.class, () -> ((CompositeBuffer) buf).extendWith(target.send())); + assertThrows(BufferClosedException.class, () -> ((CompositeBuffer) buf).extendWith(target.send())); } } assertThrows(BufferClosedException.class, () -> buf.split()); assertThrows(BufferClosedException.class, () -> buf.send()); - assertThrows(UnsupportedOperationException.class, () -> acquire((ResourceSupport) buf)); - final RuntimeException c1e = assertThrows(RuntimeException.class, () -> buf.copy()); - assertTrue(c1e instanceof UnsupportedOperationException || c1e instanceof IllegalStateException); + assertThrows(BufferClosedException.class, () -> acquire((ResourceSupport) buf)); + assertThrows(BufferClosedException.class, () -> buf.copy()); assertThrows(BufferClosedException.class, () -> buf.openCursor()); assertThrows(BufferClosedException.class, () -> buf.openCursor(0, 0)); assertThrows(BufferClosedException.class, () -> buf.openReverseCursor()); @@ -348,31 +347,31 @@ public abstract class BufferTestSupport { } public static void verifyReadInaccessible(Buffer buf) { - assertThrows(UnsupportedOperationException.class, () -> buf.readByte()); - assertThrows(UnsupportedOperationException.class, () -> buf.readUnsignedByte()); - assertThrows(UnsupportedOperationException.class, () -> buf.readChar()); - assertThrows(UnsupportedOperationException.class, () -> buf.readShort()); - assertThrows(UnsupportedOperationException.class, () -> buf.readUnsignedShort()); - assertThrows(UnsupportedOperationException.class, () -> buf.readMedium()); - assertThrows(UnsupportedOperationException.class, () -> buf.readUnsignedMedium()); - assertThrows(UnsupportedOperationException.class, () -> buf.readInt()); - assertThrows(UnsupportedOperationException.class, () -> buf.readUnsignedInt()); - assertThrows(UnsupportedOperationException.class, () -> buf.readFloat()); - assertThrows(UnsupportedOperationException.class, () -> buf.readLong()); - assertThrows(UnsupportedOperationException.class, () -> buf.readDouble()); + assertThrows(BufferClosedException.class, () -> buf.readByte()); + assertThrows(BufferClosedException.class, () -> buf.readUnsignedByte()); + assertThrows(BufferClosedException.class, () -> buf.readChar()); + assertThrows(BufferClosedException.class, () -> buf.readShort()); + assertThrows(BufferClosedException.class, () -> buf.readUnsignedShort()); + assertThrows(BufferClosedException.class, () -> buf.readMedium()); + assertThrows(BufferClosedException.class, () -> buf.readUnsignedMedium()); + assertThrows(BufferClosedException.class, () -> buf.readInt()); + assertThrows(BufferClosedException.class, () -> buf.readUnsignedInt()); + assertThrows(BufferClosedException.class, () -> buf.readFloat()); + assertThrows(BufferClosedException.class, () -> buf.readLong()); + assertThrows(BufferClosedException.class, () -> buf.readDouble()); - assertThrows(UnsupportedOperationException.class, () -> buf.getByte(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getUnsignedByte(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getChar(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getShort(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getUnsignedShort(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getMedium(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getUnsignedMedium(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getInt(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getUnsignedInt(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getFloat(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getLong(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getDouble(0)); + assertThrows(BufferClosedException.class, () -> buf.getByte(0)); + assertThrows(BufferClosedException.class, () -> buf.getUnsignedByte(0)); + assertThrows(BufferClosedException.class, () -> buf.getChar(0)); + assertThrows(BufferClosedException.class, () -> buf.getShort(0)); + assertThrows(BufferClosedException.class, () -> buf.getUnsignedShort(0)); + assertThrows(BufferClosedException.class, () -> buf.getMedium(0)); + assertThrows(BufferClosedException.class, () -> buf.getUnsignedMedium(0)); + assertThrows(BufferClosedException.class, () -> buf.getInt(0)); + assertThrows(BufferClosedException.class, () -> buf.getUnsignedInt(0)); + assertThrows(BufferClosedException.class, () -> buf.getFloat(0)); + assertThrows(BufferClosedException.class, () -> buf.getLong(0)); + assertThrows(BufferClosedException.class, () -> buf.getDouble(0)); } public static void verifyWriteInaccessible(Buffer buf, Class expected) { @@ -403,8 +402,7 @@ public abstract class BufferTestSupport { assertThrows(expected, () -> buf.setDouble(0, 32)); assertThrows(expected, () -> buf.ensureWritable(1)); - final RuntimeException f1e = assertThrows(RuntimeException.class, () -> buf.fill((byte) 0)); - assertTrue(f1e instanceof IllegalStateException || f1e instanceof UnsupportedOperationException); + assertThrows(expected, () -> buf.fill((byte) 0)); try (BufferAllocator allocator = BufferAllocator.heap(); Buffer source = allocator.allocate(8)) { assertThrows(expected, () -> source.copyInto(0, buf, 0, 1));