From 572efd918fdae4de65de556a012319c15c8e2205 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 19 Feb 2025 17:43:36 -0800 Subject: [PATCH 01/13] Optimize writing numeric values with putInt, putLong, and putDouble. --- bson/src/main/org/bson/ByteBuf.java | 33 ++++++++++ bson/src/main/org/bson/ByteBufNIO.java | 27 +++++++- bson/src/main/org/bson/io/OutputBuffer.java | 20 ++++-- .../connection/ByteBufferBsonOutput.java | 62 +++++++++++++++++-- .../internal/connection/CompositeByteBuf.java | 15 +++++ .../connection/netty/NettyByteBuf.java | 18 ++++++ 6 files changed, 163 insertions(+), 12 deletions(-) diff --git a/bson/src/main/org/bson/ByteBuf.java b/bson/src/main/org/bson/ByteBuf.java index e44a97dfc67..382adb618ad 100644 --- a/bson/src/main/org/bson/ByteBuf.java +++ b/bson/src/main/org/bson/ByteBuf.java @@ -106,6 +106,39 @@ public interface ByteBuf { */ ByteBuf put(byte b); + /** + * Writes the given int value into this buffer at the current position, + * using the current byte order, and increments the position by 4. + * + * @param b the int value to be written + * @return this buffer + * @throws java.nio.BufferOverflowException if there are fewer than 4 bytes remaining in this buffer + * @throws java.nio.ReadOnlyBufferException if this buffer is read-only + */ + ByteBuf putInt(int b); + + /** + * Writes the given double value into this buffer at the current position, + * using the current byte order, and increments the position by 8. + * + * @param b the double value to be written + * @return this buffer + * @throws java.nio.BufferOverflowException if there are fewer than 8 bytes remaining in this buffer + * @throws java.nio.ReadOnlyBufferException if this buffer is read-only + */ + ByteBuf putDouble(double b); + + /** + * Writes the given long value into this buffer at the current position, + * using the current byte order, and increments the position by 8. + * + * @param b the long value to be written + * @return this buffer + * @throws java.nio.BufferOverflowException if there are fewer than 8 bytes remaining in this buffer + * @throws java.nio.ReadOnlyBufferException if this buffer is read-only + */ + ByteBuf putLong(long b); + /** *

Flips this buffer. The limit is set to the current position and then the position is set to zero. If the mark is defined then it * is discarded.

diff --git a/bson/src/main/org/bson/ByteBufNIO.java b/bson/src/main/org/bson/ByteBufNIO.java index 83bfa7d893a..24168b90763 100644 --- a/bson/src/main/org/bson/ByteBufNIO.java +++ b/bson/src/main/org/bson/ByteBufNIO.java @@ -97,6 +97,24 @@ public ByteBuf put(final byte b) { return this; } + @Override + public ByteBuf putInt(final int b) { + buf.putInt(b); + return this; + } + + @Override + public ByteBuf putDouble(final double b) { + buf.putDouble(b); + return this; + } + + @Override + public ByteBuf putLong(final long b) { + buf.putLong(b); + return this; + } + @Override public ByteBuf flip() { ((Buffer) buf).flip(); @@ -160,8 +178,13 @@ public ByteBuf get(final byte[] bytes, final int offset, final int length) { @Override public ByteBuf get(final int index, final byte[] bytes, final int offset, final int length) { - for (int i = 0; i < length; i++) { - bytes[offset + i] = buf.get(index + i); + if (buf.hasArray()) { + System.arraycopy(buf.array(), index, bytes, offset, length); + } else { + // Fallback to per-byte copying if no backing array is available. + for (int i = 0; i < length; i++) { + bytes[offset + i] = buf.get(index + i); + } } return this; } diff --git a/bson/src/main/org/bson/io/OutputBuffer.java b/bson/src/main/org/bson/io/OutputBuffer.java index 00f88cea706..ee45beae38e 100644 --- a/bson/src/main/org/bson/io/OutputBuffer.java +++ b/bson/src/main/org/bson/io/OutputBuffer.java @@ -23,6 +23,7 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.nio.charset.StandardCharsets; import java.util.List; import static java.lang.String.format; @@ -94,12 +95,21 @@ public void writeDouble(final double x) { writeLong(Double.doubleToRawLongBits(x)); } - @Override + public void writeString(final String str) { - writeInt(0); // making space for size - int strLen = writeCharacters(str, false); - writeInt32(getPosition() - strLen - 4, strLen); - } + byte[] utf8Bytes = str.getBytes(StandardCharsets.UTF_8); + int length = utf8Bytes.length + 1; // include null terminator + writeInt32(length); + writeBytes(utf8Bytes); + writeByte(0); + } + +// @Override +// public void writeString(final String str) { +// writeInt(0); // making space for size +// int strLen = writeCharacters(str, false); +// writeInt32(getPosition() - strLen - 4, strLen); +// } @Override public void writeCString(final String value) { diff --git a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java index 40df1b867fd..a89048a359a 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java @@ -18,6 +18,7 @@ import org.bson.ByteBuf; import org.bson.io.OutputBuffer; +import org.bson.types.ObjectId; import java.io.IOException; import java.io.OutputStream; @@ -82,6 +83,58 @@ public void writeBytes(final byte[] bytes, final int offset, final int length) { position += length; } + @Override + public void writeInt32(final int value) { + ByteBuf buf = getCurrentByteBuffer(); + if (buf.remaining() >= 4) { + ensureOpen(); + buf.putInt(value); + position += 4; + } else { + // fallback for edge cases + super.writeInt32(value); + } + } + + @Override + public void writeDouble(final double value) { + ByteBuf buf = getCurrentByteBuffer(); + if (buf.remaining() >= 8) { + ensureOpen(); + buf.putDouble(value); + position += 8; + } else { + // fallback for edge cases + super.writeDouble(value); + } + } + + @Override + public void writeLong(final long value) { + ByteBuf buf = getCurrentByteBuffer(); + if (buf.remaining() >= 8) { + ensureOpen(); + buf.putLong(value); + position += 8; + } else { + // fallback for edge cases + super.writeInt64(value); + } + } + + @Override + public void writeObjectId(final ObjectId value) { + ByteBuf byteBufferAtIndex = getByteBufferAtIndex(curBufferIndex); + if (byteBufferAtIndex.remaining() >= 12) { + ensureOpen(); + value.putToByteBuffer(byteBufferAtIndex.asNIO()); + position += 12; + } else { + // fallback for edge cases + writeBytes(value.toByteArray()); + } + } + @Override public void writeByte(final int value) { ensureOpen(); @@ -156,13 +209,12 @@ public int pipe(final OutputStream out) throws IOException { int total = 0; for (final ByteBuf cur : getByteBuffers()) { - ByteBuf dup = cur.duplicate(); - while (dup.hasRemaining()) { - int numBytesToCopy = Math.min(dup.remaining(), tmp.length); - dup.get(tmp, 0, numBytesToCopy); + while (cur.hasRemaining()) { + int numBytesToCopy = Math.min(cur.remaining(), tmp.length); + cur.get(tmp, 0, numBytesToCopy); out.write(tmp, 0, numBytesToCopy); } - total += dup.limit(); + total += cur.limit(); } return total; } diff --git a/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java b/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java index fa8cde2e517..664d53e7c3e 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java +++ b/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java @@ -237,6 +237,21 @@ public ByteBuf put(final byte b) { throw new UnsupportedOperationException(); } + @Override + public ByteBuf putInt(final int b) { + throw new UnsupportedOperationException(); + } + + @Override + public ByteBuf putDouble(final double b) { + throw new UnsupportedOperationException(); + } + + @Override + public ByteBuf putLong(final long b) { + throw new UnsupportedOperationException(); + } + @Override public ByteBuf flip() { throw new UnsupportedOperationException(); diff --git a/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java b/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java index 074e77de04f..335ce823d92 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java +++ b/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java @@ -89,6 +89,24 @@ public ByteBuf put(final byte b) { return this; } + @Override + public ByteBuf putInt(final int b) { + proxied.writeInt(b); + return this; + } + + @Override + public ByteBuf putDouble(final double b) { + proxied.writeDouble(b); + return this; + } + + @Override + public ByteBuf putLong(final long b) { + proxied.writeLong(b); + return this; + } + @Override public ByteBuf flip() { isWriting = !isWriting; From c1d73acdce01e6628881a1fbda4bc491c5f536fc Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 19 Feb 2025 17:48:35 -0800 Subject: [PATCH 02/13] Remove commented code. --- bson/src/main/org/bson/io/OutputBuffer.java | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/bson/src/main/org/bson/io/OutputBuffer.java b/bson/src/main/org/bson/io/OutputBuffer.java index ee45beae38e..78e6d95bd59 100644 --- a/bson/src/main/org/bson/io/OutputBuffer.java +++ b/bson/src/main/org/bson/io/OutputBuffer.java @@ -95,21 +95,12 @@ public void writeDouble(final double x) { writeLong(Double.doubleToRawLongBits(x)); } - + @Override public void writeString(final String str) { - byte[] utf8Bytes = str.getBytes(StandardCharsets.UTF_8); - int length = utf8Bytes.length + 1; // include null terminator - writeInt32(length); - writeBytes(utf8Bytes); - writeByte(0); - } - -// @Override -// public void writeString(final String str) { -// writeInt(0); // making space for size -// int strLen = writeCharacters(str, false); -// writeInt32(getPosition() - strLen - 4, strLen); -// } + writeInt(0); // making space for size + int strLen = writeCharacters(str, false); + writeInt32(getPosition() - strLen - 4, strLen); + } @Override public void writeCString(final String value) { From 0e90c12e5bcc1806d00703f589ecfdf3bdd379f5 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 19 Feb 2025 17:49:02 -0800 Subject: [PATCH 03/13] Remove unused imports. --- bson/src/main/org/bson/io/OutputBuffer.java | 1 - 1 file changed, 1 deletion(-) diff --git a/bson/src/main/org/bson/io/OutputBuffer.java b/bson/src/main/org/bson/io/OutputBuffer.java index 78e6d95bd59..00f88cea706 100644 --- a/bson/src/main/org/bson/io/OutputBuffer.java +++ b/bson/src/main/org/bson/io/OutputBuffer.java @@ -23,7 +23,6 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; -import java.nio.charset.StandardCharsets; import java.util.List; import static java.lang.String.format; From c6935642bcd86b4562828e8fb6a8810f49ee6041 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 26 Feb 2025 22:10:13 -0800 Subject: [PATCH 04/13] Optimize writing values at absolute buffer position. --- bson/src/main/org/bson/ByteBuf.java | 11 ++++ bson/src/main/org/bson/ByteBufNIO.java | 6 ++ .../connection/ByteBufferBsonOutput.java | 61 +++++++++++++++++-- .../internal/connection/CompositeByteBuf.java | 5 ++ .../connection/netty/NettyByteBuf.java | 6 ++ 5 files changed, 83 insertions(+), 6 deletions(-) diff --git a/bson/src/main/org/bson/ByteBuf.java b/bson/src/main/org/bson/ByteBuf.java index 382adb618ad..fdef08f3b0b 100644 --- a/bson/src/main/org/bson/ByteBuf.java +++ b/bson/src/main/org/bson/ByteBuf.java @@ -117,6 +117,17 @@ public interface ByteBuf { */ ByteBuf putInt(int b); + /** + * Writes the given int value into this buffer at the current position, + * using the current byte order, and increments the position by 4. + * + * @param b the int value to be written + * @return this buffer + * @throws java.nio.BufferOverflowException if there are fewer than 4 bytes remaining in this buffer + * @throws java.nio.ReadOnlyBufferException if this buffer is read-only + */ + ByteBuf putInt(int index, int b); + /** * Writes the given double value into this buffer at the current position, * using the current byte order, and increments the position by 8. diff --git a/bson/src/main/org/bson/ByteBufNIO.java b/bson/src/main/org/bson/ByteBufNIO.java index 24168b90763..ffb6584ac64 100644 --- a/bson/src/main/org/bson/ByteBufNIO.java +++ b/bson/src/main/org/bson/ByteBufNIO.java @@ -103,6 +103,12 @@ public ByteBuf putInt(final int b) { return this; } + @Override + public ByteBuf putInt(final int index, final int b) { + buf.putInt(index, b); + return this; + } + @Override public ByteBuf putDouble(final double b) { buf.putDouble(b); diff --git a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java index a89048a359a..0cf6b55c159 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java @@ -92,7 +92,47 @@ public void writeInt32(final int value) { position += 4; } else { // fallback for edge cases - super.writeInt32(value); + write(value); + write(value >> 8); + write(value >> 16); + write(value >> 24); + } + } + + + @Override + public void writeInt32(final int absolutePosition, final int value) { + ensureOpen(); + + if (absolutePosition < 0) { + throw new IllegalArgumentException(String.format("position must be >= 0 but was %d", absolutePosition)); + } + + if (absolutePosition + 3 > position - 1) { + throw new IllegalArgumentException(String.format("Integer at specified position must fit within limit <= %d, " + + "but attempted to write at %d (occupies 4 bytes)", position - 1, absolutePosition + 3)); + } + + BufferPositionPair bufferPositionPair = getBufferPositionPair(absolutePosition); + ByteBuf byteBuffer = getByteBufferAtIndex(bufferPositionPair.bufferIndex); + int capacity = bufferPositionPair.capacity; + if (bufferPositionPair.capacity >= 4) { + byteBuffer.putInt(bufferPositionPair.position, value); + } else { + // fallback for edge cases + int valueToWrite = value; + int pos = bufferPositionPair.position; + int bufferIndex = bufferPositionPair.bufferIndex; + + for (int i = 0; i < 4; i++) { + byteBuffer.put(pos++, (byte) valueToWrite); + valueToWrite = valueToWrite >> 8; + if (--capacity == 0) { + byteBuffer = getByteBufferAtIndex(++bufferIndex); + pos = 0; + capacity = byteBuffer.position(); + } + } } } @@ -105,12 +145,12 @@ public void writeDouble(final double value) { position += 8; } else { // fallback for edge cases - super.writeDouble(value); + writeInt64(Double.doubleToRawLongBits(value)); } } @Override - public void writeLong(final long value) { + public void writeInt64(final long value) { ByteBuf buf = getCurrentByteBuffer(); if (buf.remaining() >= 8) { ensureOpen(); @@ -118,7 +158,14 @@ public void writeLong(final long value) { position += 8; } else { // fallback for edge cases - super.writeInt64(value); + write((byte) (0xFFL & (value))); + write((byte) (0xFFL & (value >> 8))); + write((byte) (0xFFL & (value >> 16))); + write((byte) (0xFFL & (value >> 24))); + write((byte) (0xFFL & (value >> 32))); + write((byte) (0xFFL & (value >> 40))); + write((byte) (0xFFL & (value >> 48))); + write((byte) (0xFFL & (value >> 56))); } } @@ -277,7 +324,7 @@ private BufferPositionPair getBufferPositionPair(final int absolutePosition) { bufferSize = bufferList.get(bufferIndex).position(); } - return new BufferPositionPair(bufferIndex, positionInBuffer); + return new BufferPositionPair(bufferIndex, positionInBuffer, bufferSize - positionInBuffer); } private void ensureOpen() { @@ -328,10 +375,12 @@ public void close() { private static final class BufferPositionPair { private final int bufferIndex; private int position; + private int capacity; - BufferPositionPair(final int bufferIndex, final int position) { + BufferPositionPair(final int bufferIndex, final int position, final int capacity) { this.bufferIndex = bufferIndex; this.position = position; + this.capacity = capacity; } } } diff --git a/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java b/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java index 664d53e7c3e..47545753367 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java +++ b/driver-core/src/main/com/mongodb/internal/connection/CompositeByteBuf.java @@ -242,6 +242,11 @@ public ByteBuf putInt(final int b) { throw new UnsupportedOperationException(); } + @Override + public ByteBuf putInt(final int index, final int b) { + throw new UnsupportedOperationException(); + } + @Override public ByteBuf putDouble(final double b) { throw new UnsupportedOperationException(); diff --git a/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java b/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java index 335ce823d92..cb6ba587419 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java +++ b/driver-core/src/main/com/mongodb/internal/connection/netty/NettyByteBuf.java @@ -95,6 +95,12 @@ public ByteBuf putInt(final int b) { return this; } + @Override + public ByteBuf putInt(final int index, final int b) { + proxied.setInt(index, b); + return this; + } + @Override public ByteBuf putDouble(final double b) { proxied.writeDouble(b); From c7d6d4d1180bc4eb6d5a614de87c5e5ce6ccd6b8 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Tue, 4 Mar 2025 00:23:09 -0800 Subject: [PATCH 05/13] Add unit tests. --- bson/src/main/org/bson/io/OutputBuffer.java | 4 +- .../connection/ByteBufferBsonOutput.java | 46 ++------ .../connection/ByteBufferBsonOutputTest.java | 105 +++++++++++++++++- 3 files changed, 116 insertions(+), 39 deletions(-) diff --git a/bson/src/main/org/bson/io/OutputBuffer.java b/bson/src/main/org/bson/io/OutputBuffer.java index 00f88cea706..17c1931e208 100644 --- a/bson/src/main/org/bson/io/OutputBuffer.java +++ b/bson/src/main/org/bson/io/OutputBuffer.java @@ -63,7 +63,7 @@ public void writeBytes(final byte[] bytes) { @Override public void writeInt32(final int value) { - write(value >> 0); + write(value); write(value >> 8); write(value >> 16); write(value >> 24); @@ -79,7 +79,7 @@ public void writeInt32(final int position, final int value) { @Override public void writeInt64(final long value) { - write((byte) (0xFFL & (value >> 0))); + write((byte) (0xFFL & (value))); write((byte) (0xFFL & (value >> 8))); write((byte) (0xFFL & (value >> 16))); write((byte) (0xFFL & (value >> 24))); diff --git a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java index 0cf6b55c159..6149c9a2637 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java @@ -18,7 +18,6 @@ import org.bson.ByteBuf; import org.bson.io.OutputBuffer; -import org.bson.types.ObjectId; import java.io.IOException; import java.io.OutputStream; @@ -92,10 +91,7 @@ public void writeInt32(final int value) { position += 4; } else { // fallback for edge cases - write(value); - write(value >> 8); - write(value >> 16); - write(value >> 24); + super.writeInt32(value); } } @@ -109,15 +105,17 @@ public void writeInt32(final int absolutePosition, final int value) { } if (absolutePosition + 3 > position - 1) { - throw new IllegalArgumentException(String.format("Integer at specified position must fit within limit <= %d, " + - "but attempted to write at %d (occupies 4 bytes)", position - 1, absolutePosition + 3)); + throw new IllegalArgumentException(String.format("Cannot write 4 bytes starting at position %d: current size is %d bytes", + position - 1, + absolutePosition + 3)); } BufferPositionPair bufferPositionPair = getBufferPositionPair(absolutePosition); ByteBuf byteBuffer = getByteBufferAtIndex(bufferPositionPair.bufferIndex); - int capacity = bufferPositionPair.capacity; - if (bufferPositionPair.capacity >= 4) { - byteBuffer.putInt(bufferPositionPair.position, value); + int capacity = byteBuffer.position() - bufferPositionPair.position; + + if (capacity >= 4) { + byteBuffer.putInt(bufferPositionPair.position, value); } else { // fallback for edge cases int valueToWrite = value; @@ -158,27 +156,7 @@ public void writeInt64(final long value) { position += 8; } else { // fallback for edge cases - write((byte) (0xFFL & (value))); - write((byte) (0xFFL & (value >> 8))); - write((byte) (0xFFL & (value >> 16))); - write((byte) (0xFFL & (value >> 24))); - write((byte) (0xFFL & (value >> 32))); - write((byte) (0xFFL & (value >> 40))); - write((byte) (0xFFL & (value >> 48))); - write((byte) (0xFFL & (value >> 56))); - } - } - - @Override - public void writeObjectId(final ObjectId value) { - ByteBuf byteBufferAtIndex = getByteBufferAtIndex(curBufferIndex); - if (byteBufferAtIndex.remaining() >= 12) { - ensureOpen(); - value.putToByteBuffer(byteBufferAtIndex.asNIO()); - position += 12; - } else { - // fallback for edge cases - writeBytes(value.toByteArray()); + super.writeInt64(value); } } @@ -324,7 +302,7 @@ private BufferPositionPair getBufferPositionPair(final int absolutePosition) { bufferSize = bufferList.get(bufferIndex).position(); } - return new BufferPositionPair(bufferIndex, positionInBuffer, bufferSize - positionInBuffer); + return new BufferPositionPair(bufferIndex, positionInBuffer); } private void ensureOpen() { @@ -375,12 +353,10 @@ public void close() { private static final class BufferPositionPair { private final int bufferIndex; private int position; - private int capacity; - BufferPositionPair(final int bufferIndex, final int position, final int capacity) { + BufferPositionPair(final int bufferIndex, final int position) { this.bufferIndex = bufferIndex; this.position = position; - this.capacity = capacity; } } } diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java index 3a8a2c83acb..dbe0caf676d 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java @@ -16,14 +16,17 @@ package com.mongodb.internal.connection; -import com.mongodb.assertions.Assertions; import org.bson.BsonSerializationException; import org.bson.ByteBuf; +import org.bson.ByteBufNIO; import org.bson.types.ObjectId; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import java.io.ByteArrayOutputStream; @@ -31,10 +34,13 @@ import java.nio.ByteBuffer; import java.nio.charset.CharacterCodingException; import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.List; import java.util.concurrent.ThreadLocalRandom; import java.util.function.BiConsumer; import java.util.function.Consumer; +import static com.mongodb.assertions.Assertions.fail; import static com.mongodb.internal.connection.ByteBufferBsonOutput.INITIAL_BUFFER_SIZE; import static com.mongodb.internal.connection.ByteBufferBsonOutput.MAX_BUFFER_SIZE; import static java.util.Arrays.asList; @@ -82,7 +88,7 @@ void positionAndSizeShouldBe0AfterConstructor(final String branchState) { break; } default: { - throw Assertions.fail(branchState); + throw fail(branchState); } } assertEquals(0, out.getPosition()); @@ -622,4 +628,99 @@ void shouldHandleMixedBranchingAndTruncating(final int reps) throws CharacterCod assertEquals(expected.toString(), StandardCharsets.UTF_8.newDecoder().decode(ByteBuffer.wrap(out.toByteArray())).toString()); } } + + @Test + @DisplayName("Test that calling writeInt32 at an absolute position where the integer would not fit throws an exception") + void shouldThrowExceptionWhenIntegerDoesNotFitWriteInt32() { + try (ByteBufferBsonOutput output = new ByteBufferBsonOutput(new SimpleBufferProvider())) { + // Write 10 bytes (position becomes 10) + for (int i = 0; i < 10; i++) { + output.writeByte(0); + } + + // absolutePosition = 7 would require bytes at positions 7,8,9,10, but the last written element was at 9. + assertThrows(IllegalArgumentException.class, () -> + output.writeInt32(7, 5678) + ); + } + } + + @Test + @DisplayName("Test that calling writeInt32 with a negative absolute position throws an exception") + void shouldThrowExceptionWhenAbsolutePositionIsNegative() { + try (ByteBufferBsonOutput output = new ByteBufferBsonOutput(new SimpleBufferProvider())) { + Assertions.assertThrows(IllegalArgumentException.class, () -> + output.writeInt32(-1, 5678) + ); + } + } + + static java.util.stream.Stream shouldWriteInt32WithinSpanningBuffers() { + return java.util.stream.Stream.of( + Arguments.of(0, 0x09080706, + Arrays.asList( + new byte[]{0, 1, 2, 3}, + new byte[]{4, 5, 6, 7}), + Arrays.asList( + new byte[]{0x06, 0x07, 0x08, 0x09}, + new byte[]{4, 5, 6, 7})), + Arguments.of(1, 0x09080706, + Arrays.asList( + new byte[]{0, 1, 2, 3}, + new byte[]{4, 5, 6, 7}), + Arrays.asList( + new byte[]{0, 0x06, 0x07, 0x08}, + new byte[]{0x09, 5, 6, 7})), + Arguments.of(2, 0x09080706, + Arrays.asList( + new byte[]{0, 1, 2, 3}, + new byte[]{4, 5, 6, 7}), + Arrays.asList( + new byte[]{0, 1, 0x06, 0x07}, + new byte[]{0x08, 0x09, 6, 7}) + ), + Arguments.of(3, 0x09080706, + Arrays.asList( + new byte[]{0, 1, 2, 3}, + new byte[]{4, 5, 6, 7}), + Arrays.asList( + new byte[]{0, 1, 2, 0x06}, + new byte[]{0x07, 0x08, 0x09, 7}) + ), + Arguments.of(4, 0x09080706, + Arrays.asList( + new byte[]{0, 1, 2, 3}, + new byte[]{4, 5, 6, 7}), + Arrays.asList( + new byte[]{0, 1, 2, 3}, + new byte[]{0x06, 0x07, 0x08, 0x09}) + )); + } + + @ParameterizedTest + @MethodSource + void shouldWriteInt32WithinSpanningBuffers( + int absolutePosition, + int intValue, + List initialData, + List expectedBuffers) { + + try (ByteBufferBsonOutput output = + new ByteBufferBsonOutput(size -> new ByteBufNIO(ByteBuffer.allocate(4)))) { + + //given + initialData.forEach(output::writeBytes); + + //when + output.writeInt32(absolutePosition, intValue); + + //then + List buffers = output.getByteBuffers(); + assertEquals(expectedBuffers.size(), buffers.size(), "Number of buffers mismatch"); + for (int i = 0; i < expectedBuffers.size(); i++) { + assertArrayEquals(expectedBuffers.get(i), buffers.get(i).array(), + "Buffer " + i + " contents mismatch"); + } + } + } } From 01530aa1eac70c8dfa8958a023871ab398d03cd2 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Tue, 4 Mar 2025 19:09:00 -0800 Subject: [PATCH 06/13] Add unit tests. --- .../connection/ByteBufferBsonOutputTest.java | 234 +++++++++++++++--- 1 file changed, 202 insertions(+), 32 deletions(-) diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java index dbe0caf676d..acec636f4b1 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java @@ -34,7 +34,6 @@ import java.nio.ByteBuffer; import java.nio.charset.CharacterCodingException; import java.nio.charset.StandardCharsets; -import java.util.Arrays; import java.util.List; import java.util.concurrent.ThreadLocalRandom; import java.util.function.BiConsumer; @@ -630,7 +629,7 @@ void shouldHandleMixedBranchingAndTruncating(final int reps) throws CharacterCod } @Test - @DisplayName("Test that calling writeInt32 at an absolute position where the integer would not fit throws an exception") + @DisplayName("should throw exception when calling writeInt32 at absolute position where integer would not fit") void shouldThrowExceptionWhenIntegerDoesNotFitWriteInt32() { try (ByteBufferBsonOutput output = new ByteBufferBsonOutput(new SimpleBufferProvider())) { // Write 10 bytes (position becomes 10) @@ -646,7 +645,7 @@ void shouldThrowExceptionWhenIntegerDoesNotFitWriteInt32() { } @Test - @DisplayName("Test that calling writeInt32 with a negative absolute position throws an exception") + @DisplayName("should throw exception when calling writeInt32 with negative absolute position") void shouldThrowExceptionWhenAbsolutePositionIsNegative() { try (ByteBufferBsonOutput output = new ByteBufferBsonOutput(new SimpleBufferProvider())) { Assertions.assertThrows(IllegalArgumentException.class, () -> @@ -655,51 +654,39 @@ void shouldThrowExceptionWhenAbsolutePositionIsNegative() { } } - static java.util.stream.Stream shouldWriteInt32WithinSpanningBuffers() { + static java.util.stream.Stream shouldWriteInt32AbsoluteValueWithinSpanningBuffers() { return java.util.stream.Stream.of( - Arguments.of(0, 0x09080706, - Arrays.asList( + Arguments.of( + 0, // absolute position + 0x09080706, // int value + asList( + // initial data new byte[]{0, 1, 2, 3}, new byte[]{4, 5, 6, 7}), - Arrays.asList( + asList( + // expected BsonByteBufferOutput data new byte[]{0x06, 0x07, 0x08, 0x09}, new byte[]{4, 5, 6, 7})), Arguments.of(1, 0x09080706, - Arrays.asList( - new byte[]{0, 1, 2, 3}, - new byte[]{4, 5, 6, 7}), - Arrays.asList( - new byte[]{0, 0x06, 0x07, 0x08}, - new byte[]{0x09, 5, 6, 7})), + asList(new byte[]{0, 1, 2, 3}, new byte[]{4, 5, 6, 7}), + asList(new byte[]{0, 0x06, 0x07, 0x08}, new byte[]{0x09, 5, 6, 7})), Arguments.of(2, 0x09080706, - Arrays.asList( - new byte[]{0, 1, 2, 3}, - new byte[]{4, 5, 6, 7}), - Arrays.asList( - new byte[]{0, 1, 0x06, 0x07}, - new byte[]{0x08, 0x09, 6, 7}) + asList(new byte[]{0, 1, 2, 3}, new byte[]{4, 5, 6, 7}), + asList(new byte[]{0, 1, 0x06, 0x07}, new byte[]{0x08, 0x09, 6, 7}) ), Arguments.of(3, 0x09080706, - Arrays.asList( - new byte[]{0, 1, 2, 3}, - new byte[]{4, 5, 6, 7}), - Arrays.asList( - new byte[]{0, 1, 2, 0x06}, - new byte[]{0x07, 0x08, 0x09, 7}) + asList(new byte[]{0, 1, 2, 3}, new byte[]{4, 5, 6, 7}), + asList(new byte[]{0, 1, 2, 0x06}, new byte[]{0x07, 0x08, 0x09, 7}) ), Arguments.of(4, 0x09080706, - Arrays.asList( - new byte[]{0, 1, 2, 3}, - new byte[]{4, 5, 6, 7}), - Arrays.asList( - new byte[]{0, 1, 2, 3}, - new byte[]{0x06, 0x07, 0x08, 0x09}) + asList(new byte[]{0, 1, 2, 3}, new byte[]{4, 5, 6, 7}), + asList(new byte[]{0, 1, 2, 3}, new byte[]{0x06, 0x07, 0x08, 0x09}) )); } @ParameterizedTest @MethodSource - void shouldWriteInt32WithinSpanningBuffers( + void shouldWriteInt32AbsoluteValueWithinSpanningBuffers( int absolutePosition, int intValue, List initialData, @@ -723,4 +710,187 @@ void shouldWriteInt32WithinSpanningBuffers( } } } + + static java.util.stream.Stream shouldWriteInt32WithinSpanningBuffers() { + return java.util.stream.Stream.of( + // Test case 1: No initial data; entire int written into one buffer. + Arguments.of(0x09080706, + asList( + // No initial data + ), + asList( + // expected BsonByteBufferOutput data + new byte[]{0x06, 0x07, 0x08, 0x09}), + 4, // expected overall position after write (0 + 4) + 4 // expected last buffer position (buffer fully written) + ), + Arguments.of(0x09080706, + asList(new byte[]{0}), + asList(new byte[]{0, 0x06, 0x07, 0x08}, new byte[]{0x09, 0, 0, 0}), 5, 1 + ), + Arguments.of(0x09080706, + asList(new byte[]{0, 1}), + asList(new byte[]{0, 1, 0x06, 0x07}, new byte[]{0x08, 0x09, 0, 0}), 6, 2 + ), + Arguments.of(0x09080706, + asList(new byte[]{0, 1, 2}), + asList(new byte[]{0, 1, 2, 0x06}, new byte[]{0x07, 0x08, 0x09, 0}), 7, 3 + ), + Arguments.of(0x09080706, + asList(new byte[]{0, 1, 2, 3}), + asList(new byte[]{0, 1, 2, 3}, new byte[]{0x06, 0x07, 0x08, 0x09}), 8, 4 + )); + } + + static java.util.stream.Stream shouldWriteInt64WithinSpanningBuffers() { + return java.util.stream.Stream.of( + // Test case 1: No initial data; entire long written into one buffer. + Arguments.of(0x0A0B0C0D0E0F1011L, + asList( + // No initial data + ), + asList( + // expected BsonByteBufferOutput data + new byte[]{0x11, 0x10, 0x0F, 0x0E, 0x0D, 0x0C, 0x0B, 0x0A} + ), + 8, // expected overall position after write (0 + 8) + 8 // expected last buffer position (buffer fully written) + ), + Arguments.of(0x0A0B0C0D0E0F1011L, + asList(new byte[]{0}), + asList(new byte[]{0, 0x11, 0x10, 0x0F, 0x0E, 0x0D, 0x0C, 0x0B}, new byte[]{0x0A, 0, 0, 0, 0, 0, 0, 0}), + 9, 1 + ), + Arguments.of(0x0A0B0C0D0E0F1011L, + asList(new byte[]{0, 1}), + asList(new byte[]{0, 1, 0x11, 0x10, 0x0F, 0x0E, 0x0D, 0x0C}, new byte[]{0x0B, 0x0A, 0, 0, 0, 0, 0, 0}), + 10, 2 + ), + Arguments.of(0x0A0B0C0D0E0F1011L, + asList(new byte[]{0, 1, 2}), + asList(new byte[]{0, 1, 2, 0x11, 0x10, 0x0F, 0x0E, 0x0D}, new byte[]{0x0C, 0x0B, 0x0A, 0, 0, 0, 0, 0}), + 11, 3 + ), + Arguments.of(0x0A0B0C0D0E0F1011L, + asList(new byte[]{0, 1, 2, 3}), + asList(new byte[]{0, 1, 2, 3, 0x11, 0x10, 0x0F, 0x0E}, new byte[]{0x0D, 0x0C, 0x0B, 0x0A, 0, 0, 0, 0}), + 12, 4 + ), + Arguments.of(0x0A0B0C0D0E0F1011L, + asList(new byte[]{0, 1, 2, 3, 4}), + asList(new byte[]{0, 1, 2, 3, 4, 0x11, 0x10, 0x0F}, new byte[]{0x0E, 0x0D, 0x0C, 0x0B, 0x0A, 0, 0, 0}), + 13, 5 + ), + Arguments.of(0x0A0B0C0D0E0F1011L, + asList(new byte[]{0, 1, 2, 3, 4, 5}), + asList(new byte[]{0, 1, 2, 3, 4, 5, 0x11, 0x10}, new byte[]{0x0F, 0x0E, 0x0D, 0x0C, 0x0B, 0x0A, 0, 0}), + 14, 6 + ), Arguments.of(0x0A0B0C0D0E0F1011L, + asList(new byte[]{0, 1, 2, 3, 4, 5, 6}), + asList(new byte[]{0, 1, 2, 3, 4, 5, 6, 0x11}, new byte[]{0x10, 0x0F, 0x0E, 0x0D, 0x0C, 0x0B, 0x0A, 0}), + 15, 7 + ), + Arguments.of(0x0A0B0C0D0E0F1011L, + asList(new byte[]{0, 1, 2, 3, 4, 5, 6, 7}), + asList(new byte[]{0, 1, 2, 3, 4, 5, 6, 7}, new byte[]{0x11, 0x10, 0x0F, 0x0E, 0x0D, 0x0C, 0x0B, 0x0A}), + 16, 8 + ) + ); + } + + @ParameterizedTest + @MethodSource("shouldWriteInt32WithinSpanningBuffers") + void shouldWriteInt32WithinSpanningBuffers( + final int intValue, + final List initialData, + final List expectedBuffers, + final int expectedOutputPosition, + final int expectedLastBufferPosition) { + + try (ByteBufferBsonOutput output = + new ByteBufferBsonOutput(size -> new ByteBufNIO(ByteBuffer.allocate(4)))) { + + //given + initialData.forEach(output::writeBytes); + + //when + output.writeInt32(intValue); + + //then + //getByteBuffers returns ByteBuffers with limit() set to position, position set to 0. + List buffers = output.getByteBuffers(); + assertEquals(expectedBuffers.size(), buffers.size(), "Number of buffers mismatch"); + for (int i = 0; i < expectedBuffers.size(); i++) { + assertArrayEquals(expectedBuffers.get(i), buffers.get(i).array(), + "Buffer " + i + " contents mismatch"); + } + + assertEquals(expectedLastBufferPosition, buffers.get(buffers.size() - 1).limit()); + assertEquals(expectedOutputPosition, output.getPosition()); + } + } + + @ParameterizedTest + @MethodSource("shouldWriteInt64WithinSpanningBuffers") + void shouldWriteInt64WithinSpanningBuffers( + final long intValue, + final List initialData, + final List expectedBuffers, + final int expectedOutputPosition, + final int expectedLastBufferPosition) { + + try (ByteBufferBsonOutput output = + new ByteBufferBsonOutput(size -> new ByteBufNIO(ByteBuffer.allocate(8)))) { + + //given + initialData.forEach(output::writeBytes); + + //when + output.writeInt64(intValue); + + //then + //getByteBuffers returns ByteBuffers with limit() set to position, position set to 0. + List buffers = output.getByteBuffers(); + assertEquals(expectedBuffers.size(), buffers.size(), "Number of buffers mismatch"); + for (int i = 0; i < expectedBuffers.size(); i++) { + assertArrayEquals(expectedBuffers.get(i), buffers.get(i).array(), + "Buffer " + i + " contents mismatch"); + } + + assertEquals(expectedLastBufferPosition, buffers.get(buffers.size() - 1).limit()); + assertEquals(expectedOutputPosition, output.getPosition()); + } + } + + @ParameterizedTest + @MethodSource("shouldWriteInt64WithinSpanningBuffers") + void shouldWriteDoubleWithinSpanningBuffers( + final long intValue, + final List initialData, + final List expectedBuffers, + final int expectedOutputPosition, + final int expectedLastBufferPosition) { + + try (ByteBufferBsonOutput output = + new ByteBufferBsonOutput(size -> new ByteBufNIO(ByteBuffer.allocate(8)))) { + + //given + initialData.forEach(output::writeBytes); + + //when + output.writeDouble(Double.longBitsToDouble(intValue)); + + //then + //getByteBuffers returns ByteBuffers with limit() set to position, position set to 0. + List buffers = output.getByteBuffers(); + assertEquals(expectedBuffers.size(), buffers.size(), "Number of buffers mismatch"); + for (int i = 0; i < expectedBuffers.size(); i++) { + assertArrayEquals(expectedBuffers.get(i), buffers.get(i).array(), + "Buffer " + i + " contents mismatch"); + } + + assertEquals(expectedLastBufferPosition, buffers.get(buffers.size() - 1).limit()); + assertEquals(expectedOutputPosition, output.getPosition()); + } + } } From 53cbe5f63609af6c52eb1e17f0e5b6a8cdb5f862 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Tue, 4 Mar 2025 19:31:29 -0800 Subject: [PATCH 07/13] Add unit tests for ByteBuf. --- .../internal/connection/ByteBufTest.java | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufTest.java diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufTest.java new file mode 100644 index 00000000000..4622246d3a6 --- /dev/null +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufTest.java @@ -0,0 +1,84 @@ +package com.mongodb.internal.connection; + + +import org.bson.ByteBuf; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertEquals; + + +class ByteBufTest { + + static Stream bufferProviders() { + return Stream.of(new ByteBufSpecification.NettyBufferProvider(), new SimpleBufferProvider()); + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldPutInt(BufferProvider provider) { + ByteBuf buffer = provider.getBuffer(1024); + try { + buffer.putInt(42); + buffer.flip(); + assertEquals(42, buffer.getInt()); + } finally { + buffer.release(); + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldPutLong(BufferProvider provider) { + ByteBuf buffer = provider.getBuffer(1024); + try { + buffer.putLong(42L); + buffer.flip(); + assertEquals(42L, buffer.getLong()); + } finally { + buffer.release(); + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldPutDouble(BufferProvider provider) { + ByteBuf buffer = provider.getBuffer(1024); + try { + buffer.putDouble(42.0D); + buffer.flip(); + assertEquals(42.0D, buffer.getDouble()); + } finally { + buffer.release(); + } + } + + @ParameterizedTest + @MethodSource("bufferProviders") + void shouldPutIntAtIndex(BufferProvider provider) { + ByteBuf buffer = provider.getBuffer(1024); + try { + buffer.putInt(0); + buffer.putInt(0); + buffer.putInt(0); + buffer.putInt(0); + buffer.put((byte) 43); + buffer.put((byte) 44); + buffer.putInt(0, 22); + buffer.putInt(4, 23); + buffer.putInt(8, 24); + buffer.putInt(12, 25); + buffer.flip(); + assertEquals(22, buffer.getInt()); + assertEquals(23, buffer.getInt()); + assertEquals(24, buffer.getInt()); + assertEquals(25, buffer.getInt()); + assertEquals(43, buffer.get()); + assertEquals(44, buffer.get()); + } finally { + buffer.release(); + } + } +} \ No newline at end of file From 4ae828adf24f38e4f8d1e0176c7479aaa68cf50c Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Tue, 4 Mar 2025 20:16:51 -0800 Subject: [PATCH 08/13] Fix static check issues. --- bson/src/main/org/bson/io/OutputBuffer.java | 4 +-- .../connection/ByteBufferBsonOutput.java | 9 ++++--- .../internal/connection/ByteBufTest.java | 27 +++++++++++++++---- .../connection/ByteBufferBsonOutputTest.java | 18 ++++++------- 4 files changed, 38 insertions(+), 20 deletions(-) diff --git a/bson/src/main/org/bson/io/OutputBuffer.java b/bson/src/main/org/bson/io/OutputBuffer.java index 17c1931e208..00f88cea706 100644 --- a/bson/src/main/org/bson/io/OutputBuffer.java +++ b/bson/src/main/org/bson/io/OutputBuffer.java @@ -63,7 +63,7 @@ public void writeBytes(final byte[] bytes) { @Override public void writeInt32(final int value) { - write(value); + write(value >> 0); write(value >> 8); write(value >> 16); write(value >> 24); @@ -79,7 +79,7 @@ public void writeInt32(final int position, final int value) { @Override public void writeInt64(final long value) { - write((byte) (0xFFL & (value))); + write((byte) (0xFFL & (value >> 0))); write((byte) (0xFFL & (value >> 8))); write((byte) (0xFFL & (value >> 16))); write((byte) (0xFFL & (value >> 24))); diff --git a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java index 6149c9a2637..0856f8fec53 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java @@ -234,12 +234,13 @@ public int pipe(final OutputStream out) throws IOException { int total = 0; for (final ByteBuf cur : getByteBuffers()) { - while (cur.hasRemaining()) { - int numBytesToCopy = Math.min(cur.remaining(), tmp.length); - cur.get(tmp, 0, numBytesToCopy); + ByteBuf dup = cur.duplicate(); + while (dup.hasRemaining()) { + int numBytesToCopy = Math.min(dup.remaining(), tmp.length); + dup.get(tmp, 0, numBytesToCopy); out.write(tmp, 0, numBytesToCopy); } - total += cur.limit(); + total += dup.limit(); } return total; } diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufTest.java index 4622246d3a6..722d7d62fa4 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufTest.java @@ -1,3 +1,19 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.mongodb.internal.connection; @@ -18,7 +34,7 @@ static Stream bufferProviders() { @ParameterizedTest @MethodSource("bufferProviders") - void shouldPutInt(BufferProvider provider) { + void shouldPutInt(final BufferProvider provider) { ByteBuf buffer = provider.getBuffer(1024); try { buffer.putInt(42); @@ -31,7 +47,7 @@ void shouldPutInt(BufferProvider provider) { @ParameterizedTest @MethodSource("bufferProviders") - void shouldPutLong(BufferProvider provider) { + void shouldPutLong(final BufferProvider provider) { ByteBuf buffer = provider.getBuffer(1024); try { buffer.putLong(42L); @@ -44,7 +60,7 @@ void shouldPutLong(BufferProvider provider) { @ParameterizedTest @MethodSource("bufferProviders") - void shouldPutDouble(BufferProvider provider) { + void shouldPutDouble(final BufferProvider provider) { ByteBuf buffer = provider.getBuffer(1024); try { buffer.putDouble(42.0D); @@ -57,7 +73,7 @@ void shouldPutDouble(BufferProvider provider) { @ParameterizedTest @MethodSource("bufferProviders") - void shouldPutIntAtIndex(BufferProvider provider) { + void shouldPutIntAtIndex(final BufferProvider provider) { ByteBuf buffer = provider.getBuffer(1024); try { buffer.putInt(0); @@ -71,6 +87,7 @@ void shouldPutIntAtIndex(BufferProvider provider) { buffer.putInt(8, 24); buffer.putInt(12, 25); buffer.flip(); + assertEquals(22, buffer.getInt()); assertEquals(23, buffer.getInt()); assertEquals(24, buffer.getInt()); @@ -81,4 +98,4 @@ void shouldPutIntAtIndex(BufferProvider provider) { buffer.release(); } } -} \ No newline at end of file +} diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java index acec636f4b1..ad1bf422213 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java @@ -687,10 +687,10 @@ static java.util.stream.Stream shouldWriteInt32AbsoluteValueWithinSpa @ParameterizedTest @MethodSource void shouldWriteInt32AbsoluteValueWithinSpanningBuffers( - int absolutePosition, - int intValue, - List initialData, - List expectedBuffers) { + final int absolutePosition, + final int intValue, + final List initialData, + final List expectedBuffers) { try (ByteBufferBsonOutput output = new ByteBufferBsonOutput(size -> new ByteBufNIO(ByteBuffer.allocate(4)))) { @@ -711,7 +711,7 @@ void shouldWriteInt32AbsoluteValueWithinSpanningBuffers( } } - static java.util.stream.Stream shouldWriteInt32WithinSpanningBuffers() { + static java.util.stream.Stream int32SpanningBuffersData() { return java.util.stream.Stream.of( // Test case 1: No initial data; entire int written into one buffer. Arguments.of(0x09080706, @@ -742,7 +742,7 @@ static java.util.stream.Stream shouldWriteInt32WithinSpanningBuffers( )); } - static java.util.stream.Stream shouldWriteInt64WithinSpanningBuffers() { + static java.util.stream.Stream int64SpanningBuffersData() { return java.util.stream.Stream.of( // Test case 1: No initial data; entire long written into one buffer. Arguments.of(0x0A0B0C0D0E0F1011L, @@ -799,7 +799,7 @@ static java.util.stream.Stream shouldWriteInt64WithinSpanningBuffers( } @ParameterizedTest - @MethodSource("shouldWriteInt32WithinSpanningBuffers") + @MethodSource("int32SpanningBuffersData") void shouldWriteInt32WithinSpanningBuffers( final int intValue, final List initialData, @@ -831,7 +831,7 @@ void shouldWriteInt32WithinSpanningBuffers( } @ParameterizedTest - @MethodSource("shouldWriteInt64WithinSpanningBuffers") + @MethodSource("int64SpanningBuffersData") void shouldWriteInt64WithinSpanningBuffers( final long intValue, final List initialData, @@ -863,7 +863,7 @@ void shouldWriteInt64WithinSpanningBuffers( } @ParameterizedTest - @MethodSource("shouldWriteInt64WithinSpanningBuffers") + @MethodSource("int64SpanningBuffersData") void shouldWriteDoubleWithinSpanningBuffers( final long intValue, final List initialData, From 5983d1fb620fea649d3b982228749da6cfd614d2 Mon Sep 17 00:00:00 2001 From: Viacheslav Babanin Date: Thu, 6 Mar 2025 12:38:15 -0800 Subject: [PATCH 09/13] Update driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java Co-authored-by: Ross Lawley --- .../com/mongodb/internal/connection/ByteBufferBsonOutput.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java index 0856f8fec53..cf2cb23f241 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java @@ -84,9 +84,9 @@ public void writeBytes(final byte[] bytes, final int offset, final int length) { @Override public void writeInt32(final int value) { + ensureOpen(); ByteBuf buf = getCurrentByteBuffer(); if (buf.remaining() >= 4) { - ensureOpen(); buf.putInt(value); position += 4; } else { From ee72dbc52cfe036aacab4fc54a1ff18c56e5a796 Mon Sep 17 00:00:00 2001 From: Viacheslav Babanin Date: Thu, 6 Mar 2025 12:38:33 -0800 Subject: [PATCH 10/13] Update driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java Co-authored-by: Ross Lawley --- .../com/mongodb/internal/connection/ByteBufferBsonOutput.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java index cf2cb23f241..9ad6a409803 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java @@ -136,9 +136,9 @@ public void writeInt32(final int absolutePosition, final int value) { @Override public void writeDouble(final double value) { + ensureOpen(); ByteBuf buf = getCurrentByteBuffer(); if (buf.remaining() >= 8) { - ensureOpen(); buf.putDouble(value); position += 8; } else { From 28351b45102d8cb9d9e6f0b03021a5a77a29ce87 Mon Sep 17 00:00:00 2001 From: Viacheslav Babanin Date: Thu, 6 Mar 2025 12:38:47 -0800 Subject: [PATCH 11/13] Update driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java Co-authored-by: Ross Lawley --- .../com/mongodb/internal/connection/ByteBufferBsonOutput.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java index 9ad6a409803..d53ffe7c683 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ByteBufferBsonOutput.java @@ -149,9 +149,9 @@ public void writeDouble(final double value) { @Override public void writeInt64(final long value) { + ensureOpen(); ByteBuf buf = getCurrentByteBuffer(); if (buf.remaining() >= 8) { - ensureOpen(); buf.putLong(value); position += 8; } else { From 4b37c946e5d8fe8907354735cf480a44817d97c4 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Thu, 6 Mar 2025 14:03:27 -0800 Subject: [PATCH 12/13] Update javadoc. --- bson/src/main/org/bson/ByteBuf.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bson/src/main/org/bson/ByteBuf.java b/bson/src/main/org/bson/ByteBuf.java index fdef08f3b0b..089bb67885c 100644 --- a/bson/src/main/org/bson/ByteBuf.java +++ b/bson/src/main/org/bson/ByteBuf.java @@ -114,6 +114,7 @@ public interface ByteBuf { * @return this buffer * @throws java.nio.BufferOverflowException if there are fewer than 4 bytes remaining in this buffer * @throws java.nio.ReadOnlyBufferException if this buffer is read-only + * @since 5.4 */ ByteBuf putInt(int b); @@ -125,6 +126,7 @@ public interface ByteBuf { * @return this buffer * @throws java.nio.BufferOverflowException if there are fewer than 4 bytes remaining in this buffer * @throws java.nio.ReadOnlyBufferException if this buffer is read-only + * @since 5.4 */ ByteBuf putInt(int index, int b); @@ -136,6 +138,7 @@ public interface ByteBuf { * @return this buffer * @throws java.nio.BufferOverflowException if there are fewer than 8 bytes remaining in this buffer * @throws java.nio.ReadOnlyBufferException if this buffer is read-only + * @since 5.4 */ ByteBuf putDouble(double b); @@ -147,6 +150,7 @@ public interface ByteBuf { * @return this buffer * @throws java.nio.BufferOverflowException if there are fewer than 8 bytes remaining in this buffer * @throws java.nio.ReadOnlyBufferException if this buffer is read-only + * @since 5.4 */ ByteBuf putLong(long b); From 183c0815d9695706f355cbeff581c175c99b4a91 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Mon, 7 Apr 2025 22:21:55 -0700 Subject: [PATCH 13/13] Add tests. --- bson/src/main/org/bson/io/OutputBuffer.java | 1 + .../connection/ByteBufferBsonOutputTest.java | 45 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/bson/src/main/org/bson/io/OutputBuffer.java b/bson/src/main/org/bson/io/OutputBuffer.java index 00f88cea706..7c1a64b2f85 100644 --- a/bson/src/main/org/bson/io/OutputBuffer.java +++ b/bson/src/main/org/bson/io/OutputBuffer.java @@ -70,6 +70,7 @@ public void writeInt32(final int value) { } @Override + @Deprecated public void writeInt32(final int position, final int value) { write(position, value >> 0); write(position + 1, value >> 8); diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java index ad1bf422213..560e3177360 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonOutputTest.java @@ -114,6 +114,51 @@ void shouldWriteByte(final boolean useBranch) { } } + @DisplayName("should write byte at position") + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void shouldWriteByteAtPosition(final boolean useBranch) { + for (int offset = 0; offset < 5; offset++) { + try (ByteBufferBsonOutput out = new ByteBufferBsonOutput(new SimpleBufferProvider())) { + byte v = 11; + byte[] byteToWrite = {1, 2, 3, 4, 5}; + if (useBranch) { + try (ByteBufferBsonOutput.Branch branch = out.branch()) { + branch.writeBytes(byteToWrite); + branch.write(offset, v); + } + } else { + out.writeBytes(byteToWrite); + out.write(offset, v); + } + byteToWrite[offset] = v; + assertArrayEquals(byteToWrite, out.toByteArray()); + assertEquals(5, out.getPosition()); + assertEquals(5, out.size()); + + } + } + } + + @DisplayName("should throw exception when writing byte at invalid position") + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void shouldThrowExceptionWhenWriteByteAtInvalidPosition(final boolean useBranch) { + try (ByteBufferBsonOutput out = new ByteBufferBsonOutput(new SimpleBufferProvider())) { + byte v = 11; + byte[] byteToWrite = {1, 2, 3, 4, 5}; + if (useBranch) { + try (ByteBufferBsonOutput.Branch branch = out.branch()) { + out.writeBytes(byteToWrite); + assertThrows(IllegalArgumentException.class, () -> branch.write(-1, v)); + } + } else { + out.writeBytes(byteToWrite); + assertThrows(IllegalArgumentException.class, () -> out.write(-1, v)); + } + } + } + @DisplayName("should write a bytes") @ParameterizedTest @ValueSource(booleans = {false, true})