Add peekInteger API to ByteBuffer #3157

RaghavRoy145 · 2025-03-25T12:43:06Z

Motivation:

The existing getInteger API is often used incorrectly, leading to verbose or error-prone code as mentioned in #2034 and #2736. Users may unintentionally access data without correctly managing the reader index. To address this, I added a peekInteger API that reads integers using the current readerIndex without modifying it.

Modifications:

Added a peekInteger method to ByteBuffer, which provides a nonmutating way to read integers using the current readerIndex.
Implemented peekInteger using the existing getInteger function.
Added comprehensive unit tests to validate correct behavior, including scenarios for different integer types, insufficient bytes, and multi-write scenarios.

Result:

Users now have access to a safer API for inspecting integers in a buffer without modifying state.
Added tests for integer reading scenarios.

…pple#2736 and apple#2034, leading to verbose or error-prone code. Users may unintentionally access data without correctly managing the reader index. To address this, I introduce a nonmutating peekInteger API that reads integers using the current readerIndex without modifying it. * Added a peekInteger method to ByteBuffer, which provides a nonmutating way to read integers using the current readerIndex. * Implemented peekInteger using the existing getInteger function. * Added some simple unit tests to validate correct behavior, including scenarios for different integer types, insufficient bytes, and multi-write scenarios. * Users now have access to a safer, more intuitive API for inspecting integers in a buffer without modifying state. * Existing getInteger functionality remains unchanged. * Added tests for integer reading scenarios.

glbrntt

Couple of doc nits but otherwise this looks great, thank you!

Sources/NIOCore/ByteBuffer-int.swift

glbrntt · 2025-03-25T13:40:45Z

Sources/NIOCore/ByteBuffer-int.swift

@@ -113,6 +113,22 @@ extension ByteBuffer {
            self.setBytes(ptr, at: index)
        }
    }
+
+    /// Peeks at the integer at the current reader index without advancing it.


Suggested change

/// Peeks at the integer at the current reader index without advancing it.

/// Returns the integer at the current reader index without advancing it.

Just to echo my comment on one of the issues referred to in the description. Since this kind of a change can be extrapolated to every type that has a get* API, can the scope of this PR increase to consider them or should they separate PRs?
Thanks!

Given that this PR is effectively good to go I'd bias towards merging this and opening a separate PR for other methods.

glbrntt

Oops, looks like the formatter failed:

diff --git a/Sources/NIOCore/ByteBuffer-int.swift b/Sources/NIOCore/ByteBuffer-int.swift
index b228801..161eff7 100644
--- a/Sources/NIOCore/ByteBuffer-int.swift
+++ b/Sources/NIOCore/ByteBuffer-int.swift
@@ -127,7 +127,7 @@ extension ByteBuffer {
         endianness: Endianness = .big,
         as: T.Type = T.self
     ) -> T? {
-        return self.getInteger(at: self.readerIndex, endianness: endianness, as: `as`)
+        self.getInteger(at: self.readerIndex, endianness: endianness, as: `as`)
     }
 }
 
diff --git a/Tests/NIOCoreTests/ByteBufferTest.swift b/Tests/NIOCoreTests/ByteBufferTest.swift
index 460810d..dcdaa11 100644
--- a/Tests/NIOCoreTests/ByteBufferTest.swift
+++ b/Tests/NIOCoreTests/ByteBufferTest.swift
@@ -3876,7 +3876,7 @@ extension ByteBufferTest {
         let second: UInt8 = 0x34
         _ = buffer.writeInteger(first)
         _ = buffer.writeInteger(second)
-        
+
         // First peek to check the first integer.
         guard let peeked1: UInt8 = buffer.peekInteger(as: UInt8.self) else {
             XCTFail("peekInteger failed to return a value on first call")
@@ -3884,7 +3884,7 @@ extension ByteBufferTest {
         }
         XCTAssertEqual(peeked1, first)
         XCTAssertEqual(buffer.readerIndex, 0)
-        
+
         // Second peek should return the same value, confirming readerIndex is unchanged.
         guard let peeked2: UInt8 = buffer.peekInteger(as: UInt8.self) else {
             XCTFail("peekInteger failed to return a value on second call")

glbrntt · 2025-03-25T17:00:27Z

Sources/NIOCore/ByteBuffer-int.swift

+        guard let result = self.getInteger(at: self.readerIndex, endianness: endianness, as: T.self) else {
+            return nil
+        }
+        return result


We don't need the guard here. The formatter was complaining because of the explicit return where it wasn't necessary:

Suggested change

guard let result = self.getInteger(at: self.readerIndex, endianness: endianness, as: T.self) else {

return nil

}

return result

self.getInteger(at: self.readerIndex, endianness: endianness, as: T.self)

glbrntt

Thank you!

RaghavRoy145 · 2025-03-25T17:33:23Z

I'm sorry, I need to be more careful with the format checks. However, there's some unit tests that fail, are those supposed to be taken care of as well?

glbrntt · 2025-03-27T09:29:24Z

I'm sorry, I need to be more careful with the format checks.

No worries! For what it's worth you should be able to run swift format --parallel --recursive --in-place Sources Tests to run the formatter locally (assuming you have a Swift 6 toolchain installed).

However, there's some unit tests that fail, are those supposed to be taken care of as well?

The failing unit tests are an issue with a nightly build of the compiler, so nothing more you need to do here.

### Motivation: Existing get APIs require passing an explicit index and can be misused, leading to verbose and error-prone code. Adding peek variants that automatically use the current readerIndex improves safety and clarity. This aims to address #2034 and #2736, and is a continuation of PR #3157 ### Modifications: - Introduced peekBytes(), peekString(), peekNullTerminatedString(), peekUTF8ValidatedString() and peekDispatchData(). - Added tests for each peek API covering normal, empty, repeated, and partial read scenarios. ### Result: Developers can now use nonmutating peek APIs to inspect ByteBuffer contents without altering the reader index. ***PS**: I have focussed on the ByteBuffer-aux get\* implementations, so I'm not sure if I am missing any other important/error-prone get\* elsewhere, if so, please point me to them and I'll be happy to add respective peek\*'s!*

Existing get APIs require passing an explicit index and can be misused, leading to verbose and error-prone code. Adding peek variants that automatically use the current readerIndex improves safety and clarity. This aims to address issue apple#2034 and issue apple#2736, and is a continuation of PR apple#3157 Modifications: Introduced peekSlice(), peekLengthPrefixedSlice(), peekData(), peekUUIDBytes() and peekWebSocketErrorCode(). Added tests for each peek API covering normal, empty and repeated peek scenarios. Result: Developers can now use nonmutating peek APIs to inspect ByteBuffer contents without altering the reader index.

### Motivation: Existing get APIs require passing an explicit index and can be misused, leading to verbose and error-prone code. Adding peek variants that automatically use the current readerIndex improves safety and clarity. This aims to address issue #2034 and issue #2736, and is a continuation of PR #3157 ### Modifications: Introduced peekSlice(), peekLengthPrefixedSlice(), peekData(), peekUUIDBytes() and peekWebSocketErrorCode(). Added tests for each peek API covering normal, empty and repeated peek scenarios. ### Result: Developers can now use nonmutating peek APIs to inspect ByteBuffer contents without altering the reader index.

glbrntt reviewed Mar 25, 2025

View reviewed changes

glbrntt added the 🆕 semver/minor Adds new public API. label Mar 25, 2025

RaghavRoy145 and others added 2 commits March 25, 2025 15:16

Resolved Doc nits

2c87825

Merge branch 'main' into peek

c99b201

glbrntt approved these changes Mar 25, 2025

View reviewed changes

glbrntt requested changes Mar 25, 2025

View reviewed changes

Resolved formatting issues

ad2317b

glbrntt reviewed Mar 25, 2025

View reviewed changes

Removed explicit return

046a1ff

RaghavRoy145 mentioned this pull request Mar 25, 2025

We should offer the same [get/set]MultipleIntegers as we offer for [read/write]MultipleIntegers methods on ByteBuffer #2736

Open

glbrntt approved these changes Mar 25, 2025

View reviewed changes

More formatting changes

1020124

Merge branch 'main' into peek

e3331a5

glbrntt enabled auto-merge (squash) March 27, 2025 09:29

glbrntt merged commit e8260eb into apple:main Mar 27, 2025
39 of 41 checks passed

RaghavRoy145 deleted the peek branch March 27, 2025 11:01

RaghavRoy145 mentioned this pull request Mar 29, 2025

Add Peek API variants for ByteBuffer #3160

Merged

RaghavRoy145 mentioned this pull request Apr 3, 2025

Add More Peek API Variants for ByteBuffer #3174

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add peekInteger API to ByteBuffer #3157

Add peekInteger API to ByteBuffer #3157

Uh oh!

RaghavRoy145 commented Mar 25, 2025

Uh oh!

glbrntt left a comment

Uh oh!

Uh oh!

glbrntt Mar 25, 2025

Uh oh!

RaghavRoy145 Mar 25, 2025

Uh oh!

glbrntt Mar 25, 2025

Uh oh!

glbrntt left a comment

Uh oh!

glbrntt Mar 25, 2025

Uh oh!

glbrntt left a comment

Uh oh!

RaghavRoy145 commented Mar 25, 2025 •

edited

Loading

Uh oh!

glbrntt commented Mar 27, 2025

Uh oh!

Uh oh!

Uh oh!

	/// Peeks at the integer at the current reader index without advancing it.
	/// Returns the integer at the current reader index without advancing it.

Add peekInteger API to ByteBuffer #3157

Add peekInteger API to ByteBuffer #3157

Uh oh!

Conversation

RaghavRoy145 commented Mar 25, 2025

Motivation:

Modifications:

Result:

Uh oh!

glbrntt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

glbrntt Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

RaghavRoy145 Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

glbrntt Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

glbrntt left a comment

Choose a reason for hiding this comment

Uh oh!

glbrntt Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

glbrntt left a comment

Choose a reason for hiding this comment

Uh oh!

RaghavRoy145 commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glbrntt commented Mar 27, 2025

Uh oh!

Uh oh!

Uh oh!

RaghavRoy145 commented Mar 25, 2025 •

edited

Loading