This is an automated email from the ASF dual-hosted git repository. tabish pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/qpid-protonj2.git
The following commit(s) were added to refs/heads/main by this push: new fba2e778 PROTON-2894 Add an optimized peekByte to buffer APIs fba2e778 is described below commit fba2e7787374840ed9b0b38c061333b7135acf24 Author: Timothy Bish <tabish...@gmail.com> AuthorDate: Thu May 1 16:34:47 2025 -0400 PROTON-2894 Add an optimized peekByte to buffer APIs Allows many of the decoders to more easily peek ahead and check for null encodings while decoding the elements of a performative. --- .../protonj2/buffer/ProtonBufferAccessors.java | 10 ++++ .../buffer/impl/ProtonByteArrayBuffer.java | 16 +++++ .../buffer/impl/ProtonCompositeBufferImpl.java | 22 +++++++ .../buffer/netty/Netty4ToProtonBufferAdapter.java | 16 +++++ .../decoders/messaging/HeaderTypeDecoder.java | 5 +- .../decoders/messaging/PropertiesTypeDecoder.java | 5 +- .../decoders/transport/AttachTypeDecoder.java | 3 +- .../codec/decoders/transport/BeginTypeDecoder.java | 3 +- .../decoders/transport/DetachTypeDecoder.java | 3 +- .../decoders/transport/DispositionTypeDecoder.java | 2 +- .../codec/decoders/transport/FlowTypeDecoder.java | 2 +- .../codec/decoders/transport/OpenTypeDecoder.java | 3 +- .../decoders/transport/TransferTypeDecoder.java | 2 +- .../protonj2/buffer/ProtonAbstractBufferTest.java | 70 ++++++++++++++++++++++ 14 files changed, 146 insertions(+), 16 deletions(-) diff --git a/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/ProtonBufferAccessors.java b/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/ProtonBufferAccessors.java index da254772..bf681cb0 100644 --- a/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/ProtonBufferAccessors.java +++ b/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/ProtonBufferAccessors.java @@ -24,6 +24,16 @@ package org.apache.qpid.protonj2.buffer; */ public interface ProtonBufferAccessors { + /** + * Look ahead an return the next byte that would be read from a call to readByte or + * a call to getByte at the current read offset. + * + * @return the next readable byte without advancing the read offset. + * + * @throws IndexOutOfBoundsException if there is no readable bytes left in the buffer. + */ + byte peekByte(); + /** * Reads a single byte at the given index and returns it without modification to the target * buffer read offset. diff --git a/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/impl/ProtonByteArrayBuffer.java b/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/impl/ProtonByteArrayBuffer.java index 5bdc422e..bae7e493 100644 --- a/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/impl/ProtonByteArrayBuffer.java +++ b/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/impl/ProtonByteArrayBuffer.java @@ -351,6 +351,12 @@ public final class ProtonByteArrayBuffer extends SharedResource<ProtonBuffer> im //----- Indexed Get operations + @Override + public byte peekByte() { + checkPeek(); + return ProtonBufferUtils.readByte(array, offset(readOffset)); + } + @Override public byte getByte(int index) { checkGet(index, Byte.BYTES); @@ -965,6 +971,16 @@ public final class ProtonByteArrayBuffer extends SharedResource<ProtonBuffer> im } } + private void checkPeek() { + if (readOffset == writeOffset) { + if (closed) { + throw ProtonBufferUtils.genericBufferIsClosed(this); + } else { + throw ProtonBufferUtils.genericOutOfBounds(this, readOffset); + } + } + } + private void checkRead(int index, int size) { if (index < 0 || writeOffset < index + size || closed) { if (closed) { diff --git a/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/impl/ProtonCompositeBufferImpl.java b/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/impl/ProtonCompositeBufferImpl.java index 8035e7af..2eb2faaf 100644 --- a/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/impl/ProtonCompositeBufferImpl.java +++ b/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/impl/ProtonCompositeBufferImpl.java @@ -758,6 +758,12 @@ public final class ProtonCompositeBufferImpl extends SharedResource<ProtonBuffer //----- Offset based get operations + @Override + public byte peekByte() { + checkPeek(); + return findIndexedAccessor(readOffset, Byte.BYTES).getByte(readOffset); + } + @Override public byte getByte(int index) { checkGetBounds(index, Byte.BYTES); @@ -1426,6 +1432,12 @@ public final class ProtonCompositeBufferImpl extends SharedResource<ProtonBuffer //----- Internal API for composite buffer + private void checkPeek() { + if (readOffset == writeOffset) { + throw generateIndexOutOfBounds(readOffset, false); + } + } + private void checkGetBounds(int index, int size) { if (index < 0 || capacity < index + size) { throw generateIndexOutOfBounds(index, false); @@ -1701,6 +1713,11 @@ public final class ProtonCompositeBufferImpl extends SharedResource<ProtonBuffer // fall within the available portion of the buffer in this chunk or // an IOOBE will again be thrown. + @Override + public byte peekByte() { + return parent.buffers[chunkIndex].peekByte(); + } + @Override public byte getByte(int index) { return parent.buffers[chunkIndex].getByte(offset(index)); @@ -1817,6 +1834,11 @@ public final class ProtonCompositeBufferImpl extends SharedResource<ProtonBuffer return this; } + @Override + public byte peekByte() { + throw new UnsupportedOperationException(); + } + @Override public byte getByte(int index) { throw new UnsupportedOperationException(); diff --git a/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/netty/Netty4ToProtonBufferAdapter.java b/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/netty/Netty4ToProtonBufferAdapter.java index 7cd2bfc5..3cdb34de 100644 --- a/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/netty/Netty4ToProtonBufferAdapter.java +++ b/protonj2/src/main/java/org/apache/qpid/protonj2/buffer/netty/Netty4ToProtonBufferAdapter.java @@ -516,6 +516,12 @@ public final class Netty4ToProtonBufferAdapter extends SharedResource<ProtonBuff //----- Primitive Get Methods + @Override + public byte peekByte() { + checkPeek(); + return resource.getByte(readOffset); + } + @Override public byte getByte(int index) { checkGet(index, Byte.BYTES); @@ -1035,6 +1041,16 @@ public final class Netty4ToProtonBufferAdapter extends SharedResource<ProtonBuff //----- Internal utilities for mapping to netty + private void checkPeek() { + if (readOffset == writeOffset) { + if (closed) { + throw ProtonBufferUtils.genericBufferIsClosed(this); + } else { + throw ProtonBufferUtils.genericOutOfBounds(this, readOffset); + } + } + } + private void checkRead(int index, int size) { if (index < 0 || writeOffset < index + size || closed) { if (closed) { diff --git a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/messaging/HeaderTypeDecoder.java b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/messaging/HeaderTypeDecoder.java index 0b07e57f..49a84999 100644 --- a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/messaging/HeaderTypeDecoder.java +++ b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/messaging/HeaderTypeDecoder.java @@ -95,9 +95,8 @@ public final class HeaderTypeDecoder extends AbstractDescribedListTypeDecoder<He // Peek ahead and see if there is a null in the next slot, if so we don't call // the setter for that entry to ensure the returned type reflects the encoded // state in the modification entry. - boolean nullValue = buffer.getByte(buffer.getReadOffset()) == EncodingCodes.NULL; - if (nullValue) { - buffer.readByte(); + if (buffer.peekByte() == EncodingCodes.NULL) { + buffer.advanceReadOffset(1); continue; } diff --git a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/messaging/PropertiesTypeDecoder.java b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/messaging/PropertiesTypeDecoder.java index 62bcbde5..63517014 100644 --- a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/messaging/PropertiesTypeDecoder.java +++ b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/messaging/PropertiesTypeDecoder.java @@ -95,9 +95,8 @@ public final class PropertiesTypeDecoder extends AbstractDescribedListTypeDecode // Peek ahead and see if there is a null in the next slot, if so we don't call // the setter for that entry to ensure the returned type reflects the encoded // state in the modification entry. - boolean nullValue = buffer.getByte(buffer.getReadOffset()) == EncodingCodes.NULL; - if (nullValue) { - buffer.readByte(); + if (buffer.peekByte() == EncodingCodes.NULL) { + buffer.advanceReadOffset(1); continue; } diff --git a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/AttachTypeDecoder.java b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/AttachTypeDecoder.java index e722905b..c1ce9d30 100644 --- a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/AttachTypeDecoder.java +++ b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/AttachTypeDecoder.java @@ -100,8 +100,7 @@ public final class AttachTypeDecoder extends AbstractDescribedListTypeDecoder<At // Peek ahead and see if there is a null in the next slot, if so we don't call // the setter for that entry to ensure the returned type reflects the encoded // state in the modification entry. - final boolean nullValue = buffer.getByte(buffer.getReadOffset()) == EncodingCodes.NULL; - if (nullValue) { + if (buffer.peekByte() == EncodingCodes.NULL) { // Ensure mandatory fields are set if (index < MIN_ATTACH_LIST_ENTRIES) { throw new DecodeException(errorForMissingRequiredFields(index)); diff --git a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/BeginTypeDecoder.java b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/BeginTypeDecoder.java index 7ccd3e4e..80a37251 100644 --- a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/BeginTypeDecoder.java +++ b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/BeginTypeDecoder.java @@ -95,8 +95,7 @@ public final class BeginTypeDecoder extends AbstractDescribedListTypeDecoder<Beg // Peek ahead and see if there is a null in the next slot, if so we don't call // the setter for that entry to ensure the returned type reflects the encoded // state in the modification entry. - final boolean nullValue = buffer.getByte(buffer.getReadOffset()) == EncodingCodes.NULL; - if (nullValue) { + if (buffer.peekByte() == EncodingCodes.NULL) { // Ensure mandatory fields are set if (index > 0 && index < MIN_BEGIN_LIST_ENTRIES) { throw new DecodeException(errorForMissingRequiredFields(index)); diff --git a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/DetachTypeDecoder.java b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/DetachTypeDecoder.java index f92570ae..4811c78d 100644 --- a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/DetachTypeDecoder.java +++ b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/DetachTypeDecoder.java @@ -96,8 +96,7 @@ public final class DetachTypeDecoder extends AbstractDescribedListTypeDecoder<De // Peek ahead and see if there is a null in the next slot, if so we don't call // the setter for that entry to ensure the returned type reflects the encoded // state in the modification entry. - final boolean nullValue = buffer.getByte(buffer.getReadOffset()) == EncodingCodes.NULL; - if (nullValue) { + if (buffer.peekByte() == EncodingCodes.NULL) { if (index == 0) { throw new DecodeException("The handle field is mandatory in a Detach"); } diff --git a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/DispositionTypeDecoder.java b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/DispositionTypeDecoder.java index 5f527ee7..5350a3f0 100644 --- a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/DispositionTypeDecoder.java +++ b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/DispositionTypeDecoder.java @@ -97,7 +97,7 @@ public final class DispositionTypeDecoder extends AbstractDescribedListTypeDecod // Peek ahead and see if there is a null in the next slot, if so we don't call // the setter for that entry to ensure the returned type reflects the encoded // state in the modification entry. - if (buffer.getByte(buffer.getReadOffset()) == EncodingCodes.NULL) { + if (buffer.peekByte() == EncodingCodes.NULL) { // Ensure mandatory fields are set if (index < MIN_DISPOSITION_LIST_ENTRIES) { throw new DecodeException(errorForMissingRequiredFields(index)); diff --git a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/FlowTypeDecoder.java b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/FlowTypeDecoder.java index 51bc9f35..595f1b08 100644 --- a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/FlowTypeDecoder.java +++ b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/FlowTypeDecoder.java @@ -96,7 +96,7 @@ public final class FlowTypeDecoder extends AbstractDescribedListTypeDecoder<Flow // Peek ahead and see if there is a null in the next slot, if so we don't call // the setter for that entry to ensure the returned type reflects the encoded // state in the modification entry. - if (buffer.getByte(buffer.getReadOffset()) == EncodingCodes.NULL) { + if (buffer.peekByte() == EncodingCodes.NULL) { // Ensure mandatory fields are set if (index > 0 && index < MIN_FLOW_LIST_ENTRIES) { throw new DecodeException(errorForMissingRequiredFields(index)); diff --git a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/OpenTypeDecoder.java b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/OpenTypeDecoder.java index b01e7dc0..377efa48 100644 --- a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/OpenTypeDecoder.java +++ b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/OpenTypeDecoder.java @@ -95,10 +95,11 @@ public final class OpenTypeDecoder extends AbstractDescribedListTypeDecoder<Open // Peek ahead and see if there is a null in the next slot, if so we don't call // the setter for that entry to ensure the returned type reflects the encoded // state in the modification entry. - if (buffer.getByte(buffer.getReadOffset()) == EncodingCodes.NULL) { + if (buffer.peekByte() == EncodingCodes.NULL) { if (index == 0) { throw new DecodeException("The container-id field cannot be omitted from the Open"); } + buffer.advanceReadOffset(1); continue; } diff --git a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/TransferTypeDecoder.java b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/TransferTypeDecoder.java index 2e4b69d2..1fbe7a81 100644 --- a/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/TransferTypeDecoder.java +++ b/protonj2/src/main/java/org/apache/qpid/protonj2/codec/decoders/transport/TransferTypeDecoder.java @@ -99,7 +99,7 @@ public final class TransferTypeDecoder extends AbstractDescribedListTypeDecoder< // Peek ahead and see if there is a null in the next slot, if so we don't call // the setter for that entry to ensure the returned type reflects the encoded // state in the modification entry. - if (buffer.getByte(buffer.getReadOffset()) == EncodingCodes.NULL) { + if (buffer.peekByte() == EncodingCodes.NULL) { if (index == 0) { throw new DecodeException("The handle field cannot be omitted from the Transfer"); } diff --git a/protonj2/src/test/java/org/apache/qpid/protonj2/buffer/ProtonAbstractBufferTest.java b/protonj2/src/test/java/org/apache/qpid/protonj2/buffer/ProtonAbstractBufferTest.java index fc1c7f9b..d90f1e69 100644 --- a/protonj2/src/test/java/org/apache/qpid/protonj2/buffer/ProtonAbstractBufferTest.java +++ b/protonj2/src/test/java/org/apache/qpid/protonj2/buffer/ProtonAbstractBufferTest.java @@ -5548,6 +5548,76 @@ public abstract class ProtonAbstractBufferTest { } } + @Test + public void testPeekByte() { + try (ProtonBufferAllocator allocator = createTestCaseAllocator()) { + ProtonBuffer buffer = allocator.allocate(128); + + assertThrows(IndexOutOfBoundsException.class, () -> buffer.peekByte()); + + final byte value = 0x01; + + buffer.writeByte(value); + assertEquals(value, buffer.peekByte()); + + buffer.advanceReadOffset(1); + assertThrows(IndexOutOfBoundsException.class, () -> buffer.peekByte()); + + buffer.writeByte(value); + buffer.close(); + + assertThrows(IllegalStateException.class, () -> buffer.peekByte()); + } + } + + @Test + public void testPeekByteOnCopiedBuffer() { + try (ProtonBufferAllocator allocator = createTestCaseAllocator()) { + ProtonBuffer buffer = allocator.allocate(128); + + final byte value = 0x01; + + buffer.writeByte(value); + assertEquals(value, buffer.peekByte()); + + final ProtonBuffer copiedBuffer = buffer.copy(); + + assertEquals(value, buffer.peekByte()); + + copiedBuffer.advanceReadOffset(1); + assertThrows(IndexOutOfBoundsException.class, () -> copiedBuffer.peekByte()); + + copiedBuffer.writeByte(value); + copiedBuffer.close(); + + assertThrows(IllegalStateException.class, () -> copiedBuffer.peekByte()); + } + } + + @Test + public void testPeekByteOnSplitBuffer() { + try (ProtonBufferAllocator allocator = createTestCaseAllocator()) { + ProtonBuffer buffer = allocator.allocate(128); + + final byte value1 = 0x01; + final byte value2 = 0x02; + + buffer.writeByte(value1); + buffer.writeByte(value2); + + assertEquals(value1, buffer.peekByte()); + assertEquals(value1, buffer.readByte()); + + final ProtonBuffer split = buffer.readSplit(1); + + assertEquals(value2, split.peekByte()); + + split.close(); + + assertThrows(IllegalStateException.class, () -> split.peekByte()); + } + } + protected static void verifyInaccessible(ProtonBuffer buf) { verifyReadInaccessible(buf); --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org