From 7475083a0999438ba9539cb7008d870c2cd376e9 Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Thu, 26 Feb 2026 13:47:14 +0000 Subject: [PATCH 1/4] `ByteBufBsonDocument` and `RawBsonDocument` simplifications ### Rationale `ByteBufBsonDocument#clone` used to return a `RawBsonDocument`. The recent changes returned a normal `BsonDocument`, which is potentially expensive depending on its usage. The `ByteBufBsonDocument` changes also added complex iterator logic, when `RawBsonDocument` deferred to `BsonDocument` iterators. As iteration is essentially a hydrating mechanism, there is opportunity for improvements for both implementations. By changing the `RawBsonDocument` iterators to be more efficient, `ByteBufBsonDocument` can now utilize these efficiency gains by proxy, relying on the `cachedDocument` iterators. This change both reduces the complexity of `ByteBufBsonDocument` and relies on an improved `RawBsonDocument` implementation. ### Summary of changes * **`ByteBufBsonDocument`**: * Simplify by returning `RawBsonDocument` from `toBsonDocument`, avoiding full BSON deserialization. When there are no sequence fields, the body bytes are cloned directly. When sequence fields exist, `BsonBinaryWriter.pipe()` merges the body with sequence arrays efficiently. * Use `toBsonDocument` for iterators. This eliminates the need for custom iterators (`IteratorMode`, `CombinedIterator`, `createBodyIterator`, and sequence iterators) since `entrySet`/`values`/`keySet` now delegate to the cached `RawBsonDocument`. * **`RawBsonDocument`**: * Renamed `toBaseBsonDocument` to override the default `toBsonDocument` implementation. * Implemented the iterators so that they don't need to fully convert the document to a `BsonDocument`. * **Tests**: * Updated `ByteBufBsonDocumentTest` iteration tests. * Updated `ByteBufBsonArrayTest#fromValues` as `entrySet` now returns `RawBsonDocument` instances. JAVA-6010 Co-Authored-By: Claude Opus 4.6 --- bson/src/main/org/bson/RawBsonDocument.java | 53 ++- .../connection/ByteBufBsonDocument.java | 311 ++---------------- .../connection/ByteBufBsonArrayTest.java | 24 +- .../connection/ByteBufBsonDocumentTest.java | 54 +-- 4 files changed, 102 insertions(+), 340 deletions(-) diff --git a/bson/src/main/org/bson/RawBsonDocument.java b/bson/src/main/org/bson/RawBsonDocument.java index eb672bcef8d..9f9fa650d54 100644 --- a/bson/src/main/org/bson/RawBsonDocument.java +++ b/bson/src/main/org/bson/RawBsonDocument.java @@ -35,7 +35,11 @@ import java.io.StringWriter; import java.nio.ByteBuffer; import java.nio.ByteOrder; +import java.util.AbstractMap; +import java.util.ArrayList; import java.util.Collection; +import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Set; @@ -225,17 +229,42 @@ public int size() { @Override public Set> entrySet() { - return toBaseBsonDocument().entrySet(); + List> entries = new ArrayList<>(); + try (BsonBinaryReader bsonReader = createReader()) { + bsonReader.readStartDocument(); + while (bsonReader.readBsonType() != BsonType.END_OF_DOCUMENT) { + String key = bsonReader.readName(); + BsonValue value = RawBsonValueHelper.decode(bytes, bsonReader); + entries.add(new AbstractMap.SimpleImmutableEntry<>(key, value)); + } + } + return new LinkedHashSet<>(entries); } @Override public Collection values() { - return toBaseBsonDocument().values(); + List values = new ArrayList<>(); + try (BsonBinaryReader bsonReader = createReader()) { + bsonReader.readStartDocument(); + while (bsonReader.readBsonType() != BsonType.END_OF_DOCUMENT) { + bsonReader.skipName(); + values.add(RawBsonValueHelper.decode(bytes, bsonReader)); + } + } + return new LinkedHashSet<>(values); } @Override public Set keySet() { - return toBaseBsonDocument().keySet(); + List keys = new ArrayList<>(); + try (BsonBinaryReader bsonReader = createReader()) { + bsonReader.readStartDocument(); + while (bsonReader.readBsonType() != BsonType.END_OF_DOCUMENT) { + keys.add(bsonReader.readName()); + bsonReader.skipValue(); + } + } + return new LinkedHashSet<>(keys); } @Override @@ -318,12 +347,19 @@ public String toJson(final JsonWriterSettings settings) { @Override public boolean equals(final Object o) { - return toBaseBsonDocument().equals(o); + return toBsonDocument().equals(o); } @Override public int hashCode() { - return toBaseBsonDocument().hashCode(); + return toBsonDocument().hashCode(); + } + + @Override + public BsonDocument toBsonDocument() { + try (BsonBinaryReader bsonReader = createReader()) { + return new BsonDocumentCodec().decode(bsonReader, DecoderContext.builder().build()); + } } @Override @@ -335,13 +371,6 @@ private BsonBinaryReader createReader() { return new BsonBinaryReader(new ByteBufferBsonInput(getByteBuffer())); } - // Transform to an org.bson.BsonDocument instance - private BsonDocument toBaseBsonDocument() { - try (BsonBinaryReader bsonReader = createReader()) { - return new BsonDocumentCodec().decode(bsonReader, DecoderContext.builder().build()); - } - } - /** * Write the replacement object. * diff --git a/driver-core/src/main/com/mongodb/internal/connection/ByteBufBsonDocument.java b/driver-core/src/main/com/mongodb/internal/connection/ByteBufBsonDocument.java index 4d4ebcc1169..639bf7654e2 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ByteBufBsonDocument.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ByteBufBsonDocument.java @@ -24,13 +24,15 @@ import com.mongodb.lang.Nullable; import org.bson.BsonArray; import org.bson.BsonBinaryReader; +import org.bson.BsonBinaryWriter; import org.bson.BsonDocument; +import org.bson.BsonElement; import org.bson.BsonReader; import org.bson.BsonType; import org.bson.BsonValue; import org.bson.ByteBuf; -import org.bson.codecs.BsonDocumentCodec; -import org.bson.codecs.DecoderContext; +import org.bson.RawBsonDocument; +import org.bson.io.BasicOutputBuffer; import org.bson.io.ByteBufferBsonInput; import org.bson.json.JsonMode; import org.bson.json.JsonWriterSettings; @@ -41,13 +43,9 @@ import java.io.ObjectInputStream; import java.io.UnsupportedEncodingException; import java.nio.charset.StandardCharsets; -import java.util.AbstractCollection; -import java.util.AbstractMap; -import java.util.AbstractSet; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; -import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -186,7 +184,6 @@ public final class ByteBufBsonDocument extends BsonDocument implements Closeable */ private transient boolean closed; - /** * Creates a {@code ByteBufBsonDocument} from an OP_MSG command message. * @@ -382,60 +379,17 @@ public String getFirstKey() { @Override public Set> entrySet() { - ensureOpen(); - if (cachedDocument != null) { - return cachedDocument.entrySet(); - } - return new AbstractSet>() { - @Override - public Iterator> iterator() { - // Combine body entries with sequence entries - return new CombinedIterator<>(createBodyIterator(IteratorMode.ENTRIES), createSequenceEntryIterator()); - } - - @Override - public int size() { - return ByteBufBsonDocument.this.size(); - } - }; + return toBsonDocument().entrySet(); } @Override public Collection values() { - ensureOpen(); - if (cachedDocument != null) { - return cachedDocument.values(); - } - return new AbstractCollection() { - @Override - public Iterator iterator() { - return new CombinedIterator<>(createBodyIterator(IteratorMode.VALUES), createSequenceValueIterator()); - } - - @Override - public int size() { - return ByteBufBsonDocument.this.size(); - } - }; + return toBsonDocument().values(); } @Override public Set keySet() { - ensureOpen(); - if (cachedDocument != null) { - return cachedDocument.keySet(); - } - return new AbstractSet() { - @Override - public Iterator iterator() { - return new CombinedIterator<>(createBodyIterator(IteratorMode.KEYS), sequenceFields.keySet().iterator()); - } - - @Override - public int size() { - return ByteBufBsonDocument.this.size(); - } - }; + return toBsonDocument().keySet(); } // ==================== Conversion Methods ==================== @@ -452,9 +406,8 @@ public BsonReader asBsonReader() { * *

After this method is called:

*
    - *
  • The result is cached for future calls
  • + *
  • The result is cached as a {@link RawBsonDocument} for future calls
  • *
  • All underlying byte buffers are released
  • - *
  • Sequence field documents are hydrated to regular {@code BsonDocument} instances
  • *
  • All subsequent read operations use the cached document
  • *
* @@ -464,21 +417,29 @@ public BsonReader asBsonReader() { public BsonDocument toBsonDocument() { ensureOpen(); if (cachedDocument == null) { - ByteBuf dup = bodyByteBuf.duplicate(); - try (BsonBinaryReader reader = new BsonBinaryReader(new ByteBufferBsonInput(dup))) { - // Decode body document - BsonDocument doc = new BsonDocumentCodec().decode(reader, DecoderContext.builder().build()); - // Add hydrated sequence fields - for (Map.Entry entry : sequenceFields.entrySet()) { - doc.put(entry.getKey(), entry.getValue().toHydratedArray()); + if (sequenceFields.isEmpty()) { + // No sequence fields: clone body bytes directly + byte[] clonedBytes = new byte[bodyByteBuf.remaining()]; + bodyByteBuf.get(bodyByteBuf.position(), clonedBytes); + cachedDocument = new RawBsonDocument(clonedBytes); + } else { + // With sequence fields: pipe body + extra elements + BasicOutputBuffer buffer = new BasicOutputBuffer(); + ByteBuf dup = bodyByteBuf.duplicate(); + try (BsonBinaryWriter writer = new BsonBinaryWriter(buffer); + BsonBinaryReader reader = new BsonBinaryReader(new ByteBufferBsonInput(dup))) { + List extraElements = new ArrayList<>(); + for (Map.Entry entry : sequenceFields.entrySet()) { + extraElements.add(new BsonElement(entry.getKey(), entry.getValue().asArray())); + } + writer.pipe(reader, extraElements); + } finally { + dup.release(); } - cachedDocument = doc; - closed = true; - // Release buffers since we no longer need them - releaseResources(); - } finally { - dup.release(); + cachedDocument = new RawBsonDocument(buffer.getInternalBuffer(), 0, buffer.getPosition()); } + closed = true; + releaseResources(); } return cachedDocument; } @@ -504,7 +465,7 @@ public String toString() { @Override public BsonDocument clone() { ensureOpen(); - return toBsonDocument().clone(); + return toBsonDocument(); } @SuppressWarnings("EqualsDoesntCheckParameterClass") @@ -645,8 +606,8 @@ private BsonValue getValueFromBody(final String key) { } /** - * Gets the first key from the body, or from sequence fields if body is empty. - * Throws NoSuchElementException if the document is completely empty. + * Gets the first key from the body. + * Throws NoSuchElementException if the body document is completely empty. */ private String getFirstKeyFromBody() { ByteBuf dup = bodyByteBuf.duplicate(); @@ -655,10 +616,6 @@ private String getFirstKeyFromBody() { if (reader.readBsonType() != BsonType.END_OF_DOCUMENT) { return reader.readName(); } - // Body is empty, try sequence fields - if (!sequenceFields.isEmpty()) { - return sequenceFields.keySet().iterator().next(); - } throw new NoSuchElementException(); } finally { dup.release(); @@ -697,147 +654,6 @@ private int countBodyFields() { } } - // ==================== Iterator Support ==================== - - /** - * Mode for the body iterator, determining what type of elements it produces. - */ - private enum IteratorMode { ENTRIES, KEYS, VALUES } - - /** - * Creates an iterator over the body document fields. - * - *

The iterator creates a duplicated ByteBuf that is temporarily tracked for safety. - * When iteration completes normally, the buffer is released immediately and removed from tracking. - * This prevents accumulation of finished iterator buffers while ensuring cleanup if the parent - * document is closed before iteration completes.

- * - * @param mode Determines whether to return entries, keys, or values - * @return An iterator of the appropriate type - */ - @SuppressWarnings("unchecked") - private Iterator createBodyIterator(final IteratorMode mode) { - return new Iterator() { - private final Closeable resourceHandle; - private ByteBuf duplicatedByteBuf; - private BsonBinaryReader reader; - private boolean started; - private boolean finished; - - { - // Create duplicate buffer for iteration and track it temporarily - duplicatedByteBuf = bodyByteBuf.duplicate(); - resourceHandle = () -> { - if (duplicatedByteBuf != null) { - try { - if (reader != null) { - reader.close(); - } - } catch (Exception e) { - // Ignore - } - duplicatedByteBuf.release(); - duplicatedByteBuf = null; - reader = null; - } - }; - trackedResources.add(resourceHandle); - reader = new BsonBinaryReader(new ByteBufferBsonInput(duplicatedByteBuf)); - } - - @Override - public boolean hasNext() { - if (finished) { - return false; - } - ensureOpen(); - if (!started) { - reader.readStartDocument(); - reader.readBsonType(); - started = true; - } - boolean hasNext = reader.getCurrentBsonType() != BsonType.END_OF_DOCUMENT; - if (!hasNext) { - cleanup(); - } - return hasNext; - } - - @Override - public T next() { - if (!hasNext()) { - throw new NoSuchElementException(); - } - ensureOpen(); - String key = reader.readName(); - BsonValue value = readBsonValue(duplicatedByteBuf, reader, trackedResources); - reader.readBsonType(); - - switch (mode) { - case ENTRIES: - return (T) new AbstractMap.SimpleImmutableEntry<>(key, value); - case KEYS: - return (T) key; - case VALUES: - return (T) value; - default: - throw new IllegalStateException("Unknown iterator mode: " + mode); - } - } - - private void cleanup() { - if (!finished) { - finished = true; - // Remove from tracked resources since we're cleaning up immediately - trackedResources.remove(resourceHandle); - try { - resourceHandle.close(); - } catch (Exception e) { - // Ignore - } - } - } - }; - } - - /** - * Creates an iterator over sequence fields as map entries. - * Each entry contains the field name and its array value. - */ - private Iterator> createSequenceEntryIterator() { - Iterator> iter = sequenceFields.entrySet().iterator(); - return new Iterator>() { - @Override - public boolean hasNext() { - return iter.hasNext(); - } - - @Override - public Entry next() { - Entry entry = iter.next(); - return new AbstractMap.SimpleImmutableEntry<>(entry.getKey(), entry.getValue().asArray()); - } - }; - } - - /** - * Creates an iterator over sequence field values (arrays). - */ - private Iterator createSequenceValueIterator() { - Iterator iter = sequenceFields.values().iterator(); - return new Iterator() { - @Override - public boolean hasNext() { - return iter.hasNext(); - } - - @Override - public BsonValue next() { - return iter.next().asArray(); - } - }; - } - // ==================== Resource Management Helpers ==================== /** @@ -977,69 +793,6 @@ boolean containsValue(final Object value) { return value instanceof BsonValue && asArray().asArray().contains(value); } - /** - * Converts this sequence to a BsonArray of regular BsonDocument instances. - * - *

Used by {@link ByteBufBsonDocument#toBsonDocument()} to fully hydrate the document. - * Unlike {@link #asArray()}, this creates regular BsonDocument instances, not - * ByteBufBsonDocument wrappers.

- * - * @return A BsonArray containing fully deserialized BsonDocument instances - */ - BsonArray toHydratedArray() { - ByteBuf dup = sequenceByteBuf.duplicate(); - try { - List hydratedDocs = new ArrayList<>(); - while (dup.hasRemaining()) { - int docStart = dup.position(); - int docSize = dup.getInt(); - int docEnd = docStart + docSize; - ByteBuf docBuf = dup.duplicate().position(docStart).limit(docEnd); - try (BsonBinaryReader reader = new BsonBinaryReader(new ByteBufferBsonInput(docBuf))) { - hydratedDocs.add(new BsonDocumentCodec().decode(reader, DecoderContext.builder().build())); - } finally { - docBuf.release(); - } - dup.position(docEnd); - } - return new BsonArray(hydratedDocs); - } finally { - dup.release(); - } - } } - /** - * An iterator that combines two iterators sequentially. - * - *

Used to merge body field iteration with sequence field iteration, - * presenting a unified view of all document fields.

- * - * @param The type of elements returned by the iterator - */ - private static final class CombinedIterator implements Iterator { - private final Iterator primary; - private final Iterator secondary; - - CombinedIterator(final Iterator primary, final Iterator secondary) { - this.primary = primary; - this.secondary = secondary; - } - - @Override - public boolean hasNext() { - return primary.hasNext() || secondary.hasNext(); - } - - @Override - public T next() { - if (primary.hasNext()) { - return primary.next(); - } - if (secondary.hasNext()) { - return secondary.next(); - } - throw new NoSuchElementException(); - } - } } diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufBsonArrayTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufBsonArrayTest.java index 637f89cb347..4b05607a56f 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufBsonArrayTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufBsonArrayTest.java @@ -49,8 +49,6 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; -import java.io.ByteArrayOutputStream; -import java.io.IOException; import java.nio.ByteBuffer; import java.util.Date; import java.util.Iterator; @@ -351,17 +349,19 @@ void testAllBsonTypes() { } static ByteBufBsonArray fromBsonValues(final List values) { - BsonDocument document = new BsonDocument() - .append("a", new BsonArray(values)); + BsonDocument document = new BsonDocument("a", new BsonArray(values)); BasicOutputBuffer buffer = new BasicOutputBuffer(); new BsonDocumentCodec().encode(new BsonBinaryWriter(buffer), document, EncoderContext.builder().build()); - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - try { - buffer.pipe(baos); - } catch (IOException e) { - throw new RuntimeException("impossible!"); - } - ByteBuf documentByteBuf = new ByteBufNIO(ByteBuffer.wrap(baos.toByteArray())); - return (ByteBufBsonArray) new ByteBufBsonDocument(documentByteBuf).entrySet().iterator().next().getValue(); + byte[] bytes = new byte[buffer.getPosition()]; + System.arraycopy(buffer.getInternalBuffer(), 0, bytes, 0, bytes.length); + // Skip past the outer document header to the array value bytes. + // Document format: [4-byte size][type byte (0x04)][field name "a\0"][array bytes...][0x00] + int arrayOffset = 4 + 1 + 2; // doc size + type byte + "a" + null terminator + int arraySize = (bytes[arrayOffset] & 0xFF) + | ((bytes[arrayOffset + 1] & 0xFF) << 8) + | ((bytes[arrayOffset + 2] & 0xFF) << 16) + | ((bytes[arrayOffset + 3] & 0xFF) << 24); + ByteBuf arrayByteBuf = new ByteBufNIO(ByteBuffer.wrap(bytes, arrayOffset, arraySize)); + return new ByteBufBsonArray(arrayByteBuf); } } diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufBsonDocumentTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufBsonDocumentTest.java index 1f61f309d14..7db52c16312 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufBsonDocumentTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufBsonDocumentTest.java @@ -438,52 +438,32 @@ void deeplyNestedClosedRecursively() { } @Test - @DisplayName("Iteration tracks resources correctly") - void iterationTracksResources() { + @DisplayName("Iterators work as expected") + void iteratorsWorksAsExpected() { BsonDocument doc = new BsonDocument() .append("doc1", new BsonDocument("a", new BsonInt32(1))) .append("arr1", new BsonArray(asList(new BsonInt32(2), new BsonInt32(3)))) .append("primitive", new BsonString("test")); - ByteBuf buf = createByteBufFromDocument(doc); - ByteBufBsonDocument byteBufDoc = new ByteBufBsonDocument(buf); - - int count = 0; - for (Map.Entry entry : byteBufDoc.entrySet()) { - assertNotNull(entry.getKey()); - assertNotNull(entry.getValue()); - count++; - } - assertEquals(3, count); - - byteBufDoc.close(); - assertThrows(IllegalStateException.class, byteBufDoc::size); - } - - @Test - @DisplayName("Iterators ensure the resource is still open") - void iteratorsEnsureResourceIsStillOpen() { - BsonDocument doc = new BsonDocument() - .append("doc1", new BsonDocument("a", new BsonInt32(1))) - .append("arr1", new BsonArray(asList(new BsonInt32(2), new BsonInt32(3)))) - .append("primitive", new BsonString("test")); - - ByteBuf buf = createByteBufFromDocument(doc); - ByteBufBsonDocument byteBufDoc = new ByteBufBsonDocument(buf); + try (ByteBufBsonDocument byteBufDoc = new ByteBufBsonDocument(createByteBufFromDocument(doc))) { - Iterator keysIterator = byteBufDoc.keySet().iterator(); - assertDoesNotThrow(keysIterator::hasNext); + int count = 0; + for (Map.Entry entry : byteBufDoc.entrySet()) { + assertNotNull(entry.getKey()); + assertNotNull(entry.getValue()); + count++; + } + assertEquals(3, count); - Iterator nestedKeysIterator = byteBufDoc.getDocument("doc1").keySet().iterator(); - assertDoesNotThrow(nestedKeysIterator::hasNext); + Iterator keysIterator = byteBufDoc.keySet().iterator(); + assertDoesNotThrow(keysIterator::hasNext); - Iterator arrayIterator = byteBufDoc.getArray("arr1").iterator(); - assertDoesNotThrow(arrayIterator::hasNext); + Iterator nestedKeysIterator = byteBufDoc.getDocument("doc1").keySet().iterator(); + assertDoesNotThrow(nestedKeysIterator::hasNext); - byteBufDoc.close(); - assertThrows(IllegalStateException.class, keysIterator::hasNext); - assertThrows(IllegalStateException.class, nestedKeysIterator::hasNext); - assertThrows(IllegalStateException.class, arrayIterator::hasNext); + Iterator arrayIterator = byteBufDoc.getArray("arr1").iterator(); + assertDoesNotThrow(arrayIterator::hasNext); + } } @Test From 74a86d34d78b5598203903ead010473d89562497 Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Mon, 9 Mar 2026 11:46:39 +0000 Subject: [PATCH 2/4] PR updates --- bson/src/main/org/bson/RawBsonDocument.java | 15 +- .../bson/RawBsonDocumentSpecification.groovy | 494 ------------------ .../unit/org/bson/RawBsonDocumentTest.java | 420 +++++++++++++++ .../connection/ByteBufBsonDocument.java | 26 +- .../connection/ByteBufBsonDocumentTest.java | 4 + 5 files changed, 447 insertions(+), 512 deletions(-) delete mode 100644 bson/src/test/unit/org/bson/RawBsonDocumentSpecification.groovy create mode 100644 bson/src/test/unit/org/bson/RawBsonDocumentTest.java diff --git a/bson/src/main/org/bson/RawBsonDocument.java b/bson/src/main/org/bson/RawBsonDocument.java index 9f9fa650d54..00e4508f10f 100644 --- a/bson/src/main/org/bson/RawBsonDocument.java +++ b/bson/src/main/org/bson/RawBsonDocument.java @@ -127,12 +127,13 @@ public RawBsonDocument(final byte[] bytes, final int offset, final int length) { public RawBsonDocument(final T document, final Codec codec) { notNull("document", document); notNull("codec", codec); - BasicOutputBuffer buffer = new BasicOutputBuffer(); - try (BsonBinaryWriter writer = new BsonBinaryWriter(buffer)) { - codec.encode(writer, document, EncoderContext.builder().build()); - this.bytes = buffer.getInternalBuffer(); - this.offset = 0; - this.length = buffer.getPosition(); + try (BasicOutputBuffer buffer = new BasicOutputBuffer()) { + try (BsonBinaryWriter writer = new BsonBinaryWriter(buffer)) { + codec.encode(writer, document, EncoderContext.builder().build()); + this.bytes = buffer.getInternalBuffer(); + this.offset = 0; + this.length = buffer.getPosition(); + } } } @@ -251,7 +252,7 @@ public Collection values() { values.add(RawBsonValueHelper.decode(bytes, bsonReader)); } } - return new LinkedHashSet<>(values); + return values; } @Override diff --git a/bson/src/test/unit/org/bson/RawBsonDocumentSpecification.groovy b/bson/src/test/unit/org/bson/RawBsonDocumentSpecification.groovy deleted file mode 100644 index a23ec06dedb..00000000000 --- a/bson/src/test/unit/org/bson/RawBsonDocumentSpecification.groovy +++ /dev/null @@ -1,494 +0,0 @@ -/* - * 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 org.bson - -import org.bson.codecs.BsonDocumentCodec -import org.bson.codecs.DecoderContext -import org.bson.codecs.DocumentCodec -import org.bson.codecs.EncoderContext -import org.bson.codecs.RawBsonDocumentCodec -import org.bson.io.BasicOutputBuffer -import org.bson.json.JsonMode -import org.bson.json.JsonReader -import org.bson.json.JsonWriter -import org.bson.json.JsonWriterSettings -import spock.lang.Specification - -import java.nio.ByteOrder - -import static java.util.Arrays.asList -import static util.GroovyHelpers.areEqual - -class RawBsonDocumentSpecification extends Specification { - - static emptyDocument = new BsonDocument() - static emptyRawDocument = new RawBsonDocument(emptyDocument, new BsonDocumentCodec()) - static document = new BsonDocument() - .append('a', new BsonInt32(1)) - .append('b', new BsonInt32(2)) - .append('c', new BsonDocument('x', BsonBoolean.TRUE)) - .append('d', new BsonArray(asList(new BsonDocument('y', BsonBoolean.FALSE), new BsonArray(asList(new BsonInt32(1)))))) - - def 'constructors should throw if parameters are invalid'() { - when: - new RawBsonDocument(null) - - then: - thrown(IllegalArgumentException) - - when: - new RawBsonDocument(null, 0, 5) - - then: - thrown(IllegalArgumentException) - - when: - new RawBsonDocument(new byte[5], -1, 5) - - then: - thrown(IllegalArgumentException) - - when: - new RawBsonDocument(new byte[5], 5, 5) - - then: - thrown(IllegalArgumentException) - - when: - new RawBsonDocument(new byte[5], 0, 0) - - then: - thrown(IllegalArgumentException) - - when: - new RawBsonDocument(new byte[10], 6, 5) - - then: - thrown(IllegalArgumentException) - - when: - new RawBsonDocument(null, new DocumentCodec()) - - then: - thrown(IllegalArgumentException) - - when: - new RawBsonDocument(new Document(), null) - - then: - thrown(IllegalArgumentException) - } - - def 'byteBuffer should contain the correct bytes'() { - when: - def byteBuf = rawDocument.getByteBuffer() - - then: - rawDocument == document - byteBuf.asNIO().order() == ByteOrder.LITTLE_ENDIAN - byteBuf.remaining() == 66 - - when: - def actualBytes = new byte[66] - byteBuf.get(actualBytes) - - then: - actualBytes == getBytesFromDocument() - - where: - rawDocument << createRawDocumentVariants() - } - - def 'parse should through if parameter is invalid'() { - when: - RawBsonDocument.parse(null) - - then: - thrown(IllegalArgumentException) - } - - def 'should parse json'() { - expect: - RawBsonDocument.parse('{a : 1}') == new BsonDocument('a', new BsonInt32(1)) - } - - def 'containKey should throw if the key name is null'() { - when: - rawDocument.containsKey(null) - - then: - thrown(IllegalArgumentException) - - where: - rawDocument << createRawDocumentVariants() - } - - def 'containsKey should find an existing key'() { - expect: - rawDocument.containsKey('a') - rawDocument.containsKey('b') - rawDocument.containsKey('c') - rawDocument.containsKey('d') - - where: - rawDocument << createRawDocumentVariants() - } - - def 'containsKey should not find a non-existing key'() { - expect: - !rawDocument.containsKey('e') - !rawDocument.containsKey('x') - !rawDocument.containsKey('y') - rawDocument.get('e') == null - rawDocument.get('x') == null - rawDocument.get('y') == null - - where: - rawDocument << createRawDocumentVariants() - } - - def 'should return RawBsonDocument for sub documents and RawBsonArray for arrays'() { - expect: - rawDocument.get('a') instanceof BsonInt32 - rawDocument.get('b') instanceof BsonInt32 - rawDocument.get('c') instanceof RawBsonDocument - rawDocument.get('d') instanceof RawBsonArray - rawDocument.get('d').asArray().get(0) instanceof RawBsonDocument - rawDocument.get('d').asArray().get(1) instanceof RawBsonArray - - and: - rawDocument.getDocument('c').getBoolean('x').value - !rawDocument.get('d').asArray().get(0).asDocument().getBoolean('y').value - rawDocument.get('d').asArray().get(1).asArray().get(0).asInt32().value == 1 - - where: - rawDocument << createRawDocumentVariants() - } - - def 'containValue should find an existing value'() { - expect: - rawDocument.containsValue(document.get('a')) - rawDocument.containsValue(document.get('b')) - rawDocument.containsValue(document.get('c')) - rawDocument.containsValue(document.get('d')) - - where: - rawDocument << createRawDocumentVariants() - } - - def 'containValue should not find a non-existing value'() { - expect: - !rawDocument.containsValue(new BsonInt32(3)) - !rawDocument.containsValue(new BsonDocument('e', BsonBoolean.FALSE)) - !rawDocument.containsValue(new BsonArray(asList(new BsonInt32(2), new BsonInt32(4)))) - - where: - rawDocument << createRawDocumentVariants() - } - - def 'isEmpty should return false when the document is not empty'() { - expect: - !rawDocument.isEmpty() - - where: - rawDocument << createRawDocumentVariants() - } - - def 'isEmpty should return true when the document is empty'() { - expect: - emptyRawDocument.isEmpty() - } - - def 'should get correct size when the document is empty'() { - expect: - emptyRawDocument.size() == 0 - } - - def 'should get correct key set when the document is empty'() { - expect: - emptyRawDocument.keySet().isEmpty() - } - - def 'should get correct values set when the document is empty'() { - expect: - emptyRawDocument.values().isEmpty() - } - - def 'should get correct entry set when the document is empty'() { - expect: - emptyRawDocument.entrySet().isEmpty() - } - - def 'should get correct size'() { - expect: - createRawDocumenFromDocument().size() == 4 - - where: - rawDocument << createRawDocumentVariants() - } - - def 'should get correct key set'() { - expect: - rawDocument.keySet() == ['a', 'b', 'c', 'd'] as Set - - where: - rawDocument << createRawDocumentVariants() - } - - def 'should get correct values set'() { - expect: - rawDocument.values() as Set == [document.get('a'), document.get('b'), document.get('c'), document.get('d')] as Set - - where: - rawDocument << createRawDocumentVariants() - } - - def 'should get correct entry set'() { - expect: - rawDocument.entrySet() == [new TestEntry('a', document.get('a')), - new TestEntry('b', document.get('b')), - new TestEntry('c', document.get('c')), - new TestEntry('d', document.get('d'))] as Set - - where: - rawDocument << createRawDocumentVariants() - } - - def 'should get first key'() { - expect: - document.getFirstKey() == 'a' - - where: - rawDocument << createRawDocumentVariants() - } - - def 'getFirstKey should throw NoSuchElementException if the document is empty'() { - when: - emptyRawDocument.getFirstKey() - - then: - thrown(NoSuchElementException) - } - - def 'should create BsonReader'() { - when: - def reader = document.asBsonReader() - - then: - new BsonDocumentCodec().decode(reader, DecoderContext.builder().build()) == document - - cleanup: - reader.close() - } - - def 'toJson should return equivalent JSON'() { - expect: - new RawBsonDocumentCodec().decode(new JsonReader(rawDocument.toJson()), DecoderContext.builder().build()) == document - - where: - rawDocument << createRawDocumentVariants() - } - - def 'toJson should respect default JsonWriterSettings'() { - given: - def writer = new StringWriter() - - when: - new BsonDocumentCodec().encode(new JsonWriter(writer), document, EncoderContext.builder().build()) - - then: - writer.toString() == rawDocument.toJson() - - where: - rawDocument << createRawDocumentVariants() - } - - def 'toJson should respect JsonWriterSettings'() { - given: - def jsonWriterSettings = JsonWriterSettings.builder().outputMode(JsonMode.SHELL).build() - def writer = new StringWriter() - - when: - new RawBsonDocumentCodec().encode(new JsonWriter(writer, jsonWriterSettings), rawDocument, EncoderContext.builder().build()) - - then: - writer.toString() == rawDocument.toJson(jsonWriterSettings) - - where: - rawDocument << createRawDocumentVariants() - } - - def 'all write methods should throw UnsupportedOperationException'() { - given: - def rawDocument = createRawDocumenFromDocument() - - when: - rawDocument.clear() - - then: - thrown(UnsupportedOperationException) - - when: - rawDocument.put('x', BsonNull.VALUE) - - then: - thrown(UnsupportedOperationException) - - when: - rawDocument.append('x', BsonNull.VALUE) - - then: - thrown(UnsupportedOperationException) - - when: - rawDocument.putAll(new BsonDocument('x', BsonNull.VALUE)) - - then: - thrown(UnsupportedOperationException) - - when: - rawDocument.remove(BsonNull.VALUE) - - then: - thrown(UnsupportedOperationException) - } - - def 'should decode'() { - rawDocument.decode(new BsonDocumentCodec()) == document - - where: - rawDocument << createRawDocumentVariants() - } - - def 'hashCode should equal hash code of identical BsonDocument'() { - expect: - rawDocument.hashCode() == document.hashCode() - - where: - rawDocument << createRawDocumentVariants() - } - - def 'equals should equal identical BsonDocument'() { - expect: - areEqual(rawDocument, document) - areEqual(document, rawDocument) - areEqual(rawDocument, rawDocument) - !areEqual(rawDocument, emptyRawDocument) - - where: - rawDocument << createRawDocumentVariants() - } - - def 'clone should make a deep copy'() { - when: - RawBsonDocument cloned = rawDocument.clone() - - then: - !cloned.getByteBuffer().array().is(createRawDocumenFromDocument().getByteBuffer().array()) - cloned.getByteBuffer().remaining() == rawDocument.getByteBuffer().remaining() - cloned == createRawDocumenFromDocument() - - where: - rawDocument << [ - createRawDocumenFromDocument(), - createRawDocumentFromByteArray(), - createRawDocumentFromByteArrayOffsetLength() - ] - } - - def 'should serialize and deserialize'() { - given: - def baos = new ByteArrayOutputStream() - def oos = new ObjectOutputStream(baos) - - when: - oos.writeObject(localRawDocument) - def bais = new ByteArrayInputStream(baos.toByteArray()) - def ois = new ObjectInputStream(bais) - def deserializedDocument = ois.readObject() - - then: - document == deserializedDocument - - where: - localRawDocument << createRawDocumentVariants() - } - - private static List createRawDocumentVariants() { - [ - createRawDocumenFromDocument(), - createRawDocumentFromByteArray(), - createRawDocumentFromByteArrayOffsetLength() - ] - } - - private static RawBsonDocument createRawDocumenFromDocument() { - new RawBsonDocument(document, new BsonDocumentCodec()) - } - - private static RawBsonDocument createRawDocumentFromByteArray() { - byte[] strippedBytes = getBytesFromDocument() - new RawBsonDocument(strippedBytes) - } - - private static byte[] getBytesFromDocument() { - def (int size, byte[] bytes) = getBytesFromOutputBuffer() - def strippedBytes = new byte[size] - System.arraycopy(bytes, 0, strippedBytes, 0, size) - strippedBytes - } - - private static List getBytesFromOutputBuffer() { - def outputBuffer = new BasicOutputBuffer(1024) - new BsonDocumentCodec().encode(new BsonBinaryWriter(outputBuffer), document, EncoderContext.builder().build()) - def bytes = outputBuffer.getInternalBuffer() - [outputBuffer.position, bytes] - } - - private static RawBsonDocument createRawDocumentFromByteArrayOffsetLength() { - def (int size, byte[] bytes) = getBytesFromOutputBuffer() - def unstrippedBytes = new byte[size + 2] - System.arraycopy(bytes, 0, unstrippedBytes, 1, size) - new RawBsonDocument(unstrippedBytes, 1, size) - } - - class TestEntry implements Map.Entry { - - private final String key - private BsonValue value - - TestEntry(String key, BsonValue value) { - this.key = key - this.value = value - } - - @Override - String getKey() { - key - } - - @Override - BsonValue getValue() { - value - } - - @Override - BsonValue setValue(final BsonValue value) { - this.value = value - } - } -} diff --git a/bson/src/test/unit/org/bson/RawBsonDocumentTest.java b/bson/src/test/unit/org/bson/RawBsonDocumentTest.java new file mode 100644 index 00000000000..6ebb716e91f --- /dev/null +++ b/bson/src/test/unit/org/bson/RawBsonDocumentTest.java @@ -0,0 +1,420 @@ +/* + * 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 org.bson; + +import org.bson.codecs.BsonDocumentCodec; +import org.bson.codecs.DecoderContext; +import org.bson.codecs.DocumentCodec; +import org.bson.codecs.EncoderContext; +import org.bson.codecs.RawBsonDocumentCodec; +import org.bson.io.BasicOutputBuffer; +import org.bson.json.JsonMode; +import org.bson.json.JsonReader; +import org.bson.json.JsonWriter; +import org.bson.json.JsonWriterSettings; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.io.StringWriter; +import java.nio.ByteOrder; +import java.util.AbstractMap; +import java.util.HashSet; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Set; +import java.util.stream.Stream; + +import static java.util.Arrays.asList; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@DisplayName("RawBsonDocument") +class RawBsonDocumentTest { + + private static final BsonDocument EMPTY_DOCUMENT = new BsonDocument(); + private static final RawBsonDocument EMPTY_RAW_DOCUMENT = new RawBsonDocument(EMPTY_DOCUMENT, new BsonDocumentCodec()); + private static final BsonDocument DOCUMENT = new BsonDocument() + .append("a", new BsonInt32(1)) + .append("b", new BsonInt32(1)) + .append("c", new BsonDocument("x", BsonBoolean.TRUE)) + .append("d", new BsonArray(asList(new BsonDocument("y", BsonBoolean.FALSE), new BsonArray(asList(new BsonInt32(1)))))); + + // Constructor Validation + + @Test + @DisplayName("constructors should throw if parameters are invalid") + void constructorsShouldThrowForInvalidParameters() { + assertThrows(IllegalArgumentException.class, () -> new RawBsonDocument((byte[]) null)); + assertThrows(IllegalArgumentException.class, () -> new RawBsonDocument(null, 0, 5)); + assertThrows(IllegalArgumentException.class, () -> new RawBsonDocument(new byte[5], -1, 5)); + assertThrows(IllegalArgumentException.class, () -> new RawBsonDocument(new byte[5], 5, 5)); + assertThrows(IllegalArgumentException.class, () -> new RawBsonDocument(new byte[5], 0, 0)); + assertThrows(IllegalArgumentException.class, () -> new RawBsonDocument(new byte[10], 6, 5)); + assertThrows(IllegalArgumentException.class, () -> new RawBsonDocument(null, new DocumentCodec())); + assertThrows(IllegalArgumentException.class, () -> new RawBsonDocument(new Document(), null)); + } + + // Byte Buffer + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("byteBuffer should contain the correct bytes") + void byteBufferShouldContainCorrectBytes(final RawBsonDocument rawDocument) { + ByteBuf byteBuf = rawDocument.getByteBuffer(); + + assertEquals(DOCUMENT, rawDocument); + assertEquals(ByteOrder.LITTLE_ENDIAN, byteBuf.asNIO().order()); + assertEquals(66, byteBuf.remaining()); + + byte[] actualBytes = new byte[66]; + byteBuf.get(actualBytes); + assertArrayEquals(getBytesFromDocument(), actualBytes); + } + + // Parse + + @Test + @DisplayName("parse() should throw if parameter is invalid") + void parseShouldThrowForInvalidParameter() { + assertThrows(IllegalArgumentException.class, () -> RawBsonDocument.parse(null)); + } + + @Test + @DisplayName("parse() should parse JSON") + void parseShouldParseJson() { + assertEquals(new BsonDocument("a", new BsonInt32(1)), RawBsonDocument.parse("{a : 1}")); + } + + // Basic Operations + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("containsKey() throws IllegalArgumentException for null key") + void containsKeyShouldThrowForNullKey(final RawBsonDocument rawDocument) { + assertThrows(IllegalArgumentException.class, () -> rawDocument.containsKey(null)); + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("containsKey() finds existing keys") + void containsKeyShouldFindExistingKeys(final RawBsonDocument rawDocument) { + assertTrue(rawDocument.containsKey("a")); + assertTrue(rawDocument.containsKey("b")); + assertTrue(rawDocument.containsKey("c")); + assertTrue(rawDocument.containsKey("d")); + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("containsKey() does not find non-existing keys") + void containsKeyShouldNotFindNonExistingKeys(final RawBsonDocument rawDocument) { + assertFalse(rawDocument.containsKey("e")); + assertFalse(rawDocument.containsKey("x")); + assertFalse(rawDocument.containsKey("y")); + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("get() returns null for non-existing keys") + void getShouldReturnNullForNonExistingKeys(final RawBsonDocument rawDocument) { + assertEquals(null, rawDocument.get("e")); + assertEquals(null, rawDocument.get("x")); + assertEquals(null, rawDocument.get("y")); + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("get() returns RawBsonDocument for sub documents and RawBsonArray for arrays") + void getShouldReturnCorrectTypes(final RawBsonDocument rawDocument) { + assertInstanceOf(BsonInt32.class, rawDocument.get("a")); + assertInstanceOf(BsonInt32.class, rawDocument.get("b")); + assertInstanceOf(RawBsonDocument.class, rawDocument.get("c")); + assertInstanceOf(RawBsonArray.class, rawDocument.get("d")); + assertInstanceOf(RawBsonDocument.class, rawDocument.get("d").asArray().get(0)); + assertInstanceOf(RawBsonArray.class, rawDocument.get("d").asArray().get(1)); + + assertTrue(rawDocument.getDocument("c").getBoolean("x").getValue()); + assertFalse(rawDocument.get("d").asArray().get(0).asDocument().getBoolean("y").getValue()); + assertEquals(1, rawDocument.get("d").asArray().get(1).asArray().get(0).asInt32().getValue()); + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("containsValue() finds existing values") + void containsValueShouldFindExistingValues(final RawBsonDocument rawDocument) { + assertTrue(rawDocument.containsValue(DOCUMENT.get("a"))); + assertTrue(rawDocument.containsValue(DOCUMENT.get("b"))); + assertTrue(rawDocument.containsValue(DOCUMENT.get("c"))); + assertTrue(rawDocument.containsValue(DOCUMENT.get("d"))); + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("containsValue() does not find non-existing values") + void containsValueShouldNotFindNonExistingValues(final RawBsonDocument rawDocument) { + assertFalse(rawDocument.containsValue(new BsonInt32(3))); + assertFalse(rawDocument.containsValue(new BsonDocument("e", BsonBoolean.FALSE))); + assertFalse(rawDocument.containsValue(new BsonArray(asList(new BsonInt32(2), new BsonInt32(4))))); + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("isEmpty() returns false when the document is not empty") + void isEmptyShouldReturnFalseForNonEmptyDocument(final RawBsonDocument rawDocument) { + assertFalse(rawDocument.isEmpty()); + } + + @Test + @DisplayName("isEmpty() returns true when the document is empty") + void isEmptyShouldReturnTrueForEmptyDocument() { + assertTrue(EMPTY_RAW_DOCUMENT.isEmpty()); + } + + @Test + @DisplayName("size() returns 0 for empty document") + void sizeShouldReturnZeroForEmptyDocument() { + assertEquals(0, EMPTY_RAW_DOCUMENT.size()); + } + + @Test + @DisplayName("keySet() is empty for empty document") + void keySetShouldBeEmptyForEmptyDocument() { + assertTrue(EMPTY_RAW_DOCUMENT.keySet().isEmpty()); + } + + @Test + @DisplayName("values() is empty for empty document") + void valuesShouldBeEmptyForEmptyDocument() { + assertTrue(EMPTY_RAW_DOCUMENT.values().isEmpty()); + } + + @Test + @DisplayName("entrySet() is empty for empty document") + void entrySetShouldBeEmptyForEmptyDocument() { + assertTrue(EMPTY_RAW_DOCUMENT.entrySet().isEmpty()); + } + + // Collection Views + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("size() returns correct count") + void sizeShouldReturnCorrectCount(final RawBsonDocument rawDocument) { + assertEquals(4, rawDocument.size()); + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("keySet() returns all keys") + void keySetShouldReturnAllKeys(final RawBsonDocument rawDocument) { + assertEquals(new HashSet<>(asList("a", "b", "c", "d")), rawDocument.keySet()); + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("values() returns all values") + void valuesShouldReturnAllValues(final RawBsonDocument rawDocument) { + assertEquals( + asList(DOCUMENT.get("a"), DOCUMENT.get("b"), DOCUMENT.get("c"), DOCUMENT.get("d")), + rawDocument.values()); + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("entrySet() returns all entries") + void entrySetShouldReturnAllEntries(final RawBsonDocument rawDocument) { + Set> expected = new HashSet<>(asList( + new AbstractMap.SimpleImmutableEntry<>("a", DOCUMENT.get("a")), + new AbstractMap.SimpleImmutableEntry<>("b", DOCUMENT.get("b")), + new AbstractMap.SimpleImmutableEntry<>("c", DOCUMENT.get("c")), + new AbstractMap.SimpleImmutableEntry<>("d", DOCUMENT.get("d")) + )); + assertEquals(expected, rawDocument.entrySet()); + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("getFirstKey() returns first key") + void getFirstKeyShouldReturnFirstKey(final RawBsonDocument rawDocument) { + assertEquals("a", rawDocument.getFirstKey()); + } + + @Test + @DisplayName("getFirstKey() throws NoSuchElementException for empty document") + void getFirstKeyShouldThrowForEmptyDocument() { + assertThrows(NoSuchElementException.class, () -> EMPTY_RAW_DOCUMENT.getFirstKey()); + } + + // Conversion and Serialization + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("asBsonReader() creates valid reader") + void asBsonReaderShouldWork(final RawBsonDocument rawDocument) { + try (BsonReader reader = rawDocument.asBsonReader()) { + BsonDocument decoded = new BsonDocumentCodec().decode(reader, DecoderContext.builder().build()); + assertEquals(DOCUMENT, decoded); + } + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("toJson() returns equivalent JSON") + void toJsonShouldReturnEquivalentJson(final RawBsonDocument rawDocument) { + RawBsonDocument reparsed = new RawBsonDocumentCodec().decode( + new JsonReader(rawDocument.toJson()), DecoderContext.builder().build()); + assertEquals(DOCUMENT, reparsed); + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("toJson() respects default JsonWriterSettings") + void toJsonShouldRespectDefaultSettings(final RawBsonDocument rawDocument) { + StringWriter writer = new StringWriter(); + new BsonDocumentCodec().encode(new JsonWriter(writer), DOCUMENT, EncoderContext.builder().build()); + assertEquals(writer.toString(), rawDocument.toJson()); + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("toJson() respects JsonWriterSettings") + void toJsonShouldRespectCustomSettings(final RawBsonDocument rawDocument) { + JsonWriterSettings settings = JsonWriterSettings.builder().outputMode(JsonMode.SHELL).build(); + StringWriter writer = new StringWriter(); + new RawBsonDocumentCodec().encode(new JsonWriter(writer, settings), rawDocument, EncoderContext.builder().build()); + assertEquals(writer.toString(), rawDocument.toJson(settings)); + } + + // Immutability + + @Test + @DisplayName("All write methods throw UnsupportedOperationException") + void writeMethodsShouldThrow() { + RawBsonDocument rawDocument = createRawDocumentFromDocument(); + assertThrows(UnsupportedOperationException.class, () -> rawDocument.clear()); + assertThrows(UnsupportedOperationException.class, () -> rawDocument.put("x", BsonNull.VALUE)); + assertThrows(UnsupportedOperationException.class, () -> rawDocument.append("x", BsonNull.VALUE)); + assertThrows(UnsupportedOperationException.class, () -> rawDocument.putAll(new BsonDocument("x", BsonNull.VALUE))); + assertThrows(UnsupportedOperationException.class, () -> rawDocument.remove(BsonNull.VALUE)); + } + + // Decode + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("decode() returns equivalent document") + void decodeShouldWork(final RawBsonDocument rawDocument) { + assertEquals(DOCUMENT, rawDocument.decode(new BsonDocumentCodec())); + } + + // Equality and HashCode + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("hashCode() equals hash code of identical BsonDocument") + void hashCodeShouldEqualBsonDocumentHashCode(final RawBsonDocument rawDocument) { + assertEquals(DOCUMENT.hashCode(), rawDocument.hashCode()); + } + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("equals() works correctly") + void equalsShouldWork(final RawBsonDocument rawDocument) { + assertEquals(DOCUMENT, rawDocument); + assertEquals(DOCUMENT, rawDocument); + assertEquals(rawDocument, rawDocument); + assertNotEquals(EMPTY_RAW_DOCUMENT, rawDocument); + } + + // Clone + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("clone() creates deep copy") + void cloneShouldMakeDeepCopy(final RawBsonDocument rawDocument) { + RawBsonDocument cloned = (RawBsonDocument) rawDocument.clone(); + RawBsonDocument reference = createRawDocumentFromDocument(); + + assertNotSame(cloned.getByteBuffer().array(), reference.getByteBuffer().array()); + assertEquals(rawDocument.getByteBuffer().remaining(), cloned.getByteBuffer().remaining()); + assertEquals(reference, cloned); + } + + // Serialization + + @ParameterizedTest + @MethodSource("rawDocumentVariants") + @DisplayName("Java serialization works correctly") + void serializationShouldWork(final RawBsonDocument rawDocument) throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + new ObjectOutputStream(baos).writeObject(rawDocument); + Object deserialized = new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray())).readObject(); + assertEquals(DOCUMENT, deserialized); + } + + // --- Helper Methods --- + + static Stream rawDocumentVariants() { + return Stream.of( + createRawDocumentFromDocument(), + createRawDocumentFromByteArray(), + createRawDocumentFromByteArrayOffsetLength() + ); + } + + private static RawBsonDocument createRawDocumentFromDocument() { + return new RawBsonDocument(DOCUMENT, new BsonDocumentCodec()); + } + + private static RawBsonDocument createRawDocumentFromByteArray() { + return new RawBsonDocument(getBytesFromDocument()); + } + + private static RawBsonDocument createRawDocumentFromByteArrayOffsetLength() { + BasicOutputBuffer outputBuffer = new BasicOutputBuffer(1024); + new BsonDocumentCodec().encode(new BsonBinaryWriter(outputBuffer), DOCUMENT, EncoderContext.builder().build()); + byte[] bytes = outputBuffer.getInternalBuffer(); + int size = outputBuffer.getPosition(); + + byte[] unstrippedBytes = new byte[size + 2]; + System.arraycopy(bytes, 0, unstrippedBytes, 1, size); + return new RawBsonDocument(unstrippedBytes, 1, size); + } + + private static byte[] getBytesFromDocument() { + BasicOutputBuffer outputBuffer = new BasicOutputBuffer(1024); + new BsonDocumentCodec().encode(new BsonBinaryWriter(outputBuffer), DOCUMENT, EncoderContext.builder().build()); + byte[] bytes = outputBuffer.getInternalBuffer(); + int size = outputBuffer.getPosition(); + + byte[] strippedBytes = new byte[size]; + System.arraycopy(bytes, 0, strippedBytes, 0, size); + return strippedBytes; + } +} diff --git a/driver-core/src/main/com/mongodb/internal/connection/ByteBufBsonDocument.java b/driver-core/src/main/com/mongodb/internal/connection/ByteBufBsonDocument.java index 639bf7654e2..edcc15934e3 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ByteBufBsonDocument.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ByteBufBsonDocument.java @@ -424,19 +424,20 @@ public BsonDocument toBsonDocument() { cachedDocument = new RawBsonDocument(clonedBytes); } else { // With sequence fields: pipe body + extra elements - BasicOutputBuffer buffer = new BasicOutputBuffer(); - ByteBuf dup = bodyByteBuf.duplicate(); - try (BsonBinaryWriter writer = new BsonBinaryWriter(buffer); - BsonBinaryReader reader = new BsonBinaryReader(new ByteBufferBsonInput(dup))) { - List extraElements = new ArrayList<>(); - for (Map.Entry entry : sequenceFields.entrySet()) { - extraElements.add(new BsonElement(entry.getKey(), entry.getValue().asArray())); + try (BasicOutputBuffer buffer = new BasicOutputBuffer()) { + ByteBuf dup = bodyByteBuf.duplicate(); + try (BsonBinaryWriter writer = new BsonBinaryWriter(buffer); + BsonBinaryReader reader = new BsonBinaryReader(new ByteBufferBsonInput(dup))) { + List extraElements = new ArrayList<>(); + for (Entry entry : sequenceFields.entrySet()) { + extraElements.add(new BsonElement(entry.getKey(), entry.getValue().asArray())); + } + writer.pipe(reader, extraElements); + } finally { + dup.release(); } - writer.pipe(reader, extraElements); - } finally { - dup.release(); + cachedDocument = new RawBsonDocument(buffer.getInternalBuffer(), 0, buffer.getPosition()); } - cachedDocument = new RawBsonDocument(buffer.getInternalBuffer(), 0, buffer.getPosition()); } closed = true; releaseResources(); @@ -465,6 +466,9 @@ public String toString() { @Override public BsonDocument clone() { ensureOpen(); + if (cachedDocument != null) { + return cachedDocument.clone(); + } return toBsonDocument(); } diff --git a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufBsonDocumentTest.java b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufBsonDocumentTest.java index 7db52c16312..f3744057a18 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufBsonDocumentTest.java +++ b/driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufBsonDocumentTest.java @@ -62,6 +62,7 @@ import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -321,7 +322,10 @@ void toStringShouldWork() { void cloneShouldWork() { try (ByteBufBsonDocument byteBufDocument = new ByteBufBsonDocument(documentByteBuf)) { BsonDocument cloned = byteBufDocument.clone(); + assertNotSame(byteBufDocument, cloned); assertEquals(byteBufDocument, cloned); + + assertNotSame(byteBufDocument.clone(), byteBufDocument.clone()); } } From eec286aa9653fd4a9e0ebe49584deb6d8f1872ee Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Mon, 9 Mar 2026 11:55:01 +0000 Subject: [PATCH 3/4] Removed doubled ensureOpen guards. --- .claude/settings.local.json | 8 ++++++++ .../internal/connection/ByteBufBsonDocument.java | 15 ++++++--------- 2 files changed, 14 insertions(+), 9 deletions(-) create mode 100644 .claude/settings.local.json diff --git a/.claude/settings.local.json b/.claude/settings.local.json new file mode 100644 index 00000000000..3443e359782 --- /dev/null +++ b/.claude/settings.local.json @@ -0,0 +1,8 @@ +{ + "permissions": { + "allow": [ + "Bash(./gradlew :bson:compileTestJava:*)", + "Bash(ls:*)" + ] + } +} diff --git a/driver-core/src/main/com/mongodb/internal/connection/ByteBufBsonDocument.java b/driver-core/src/main/com/mongodb/internal/connection/ByteBufBsonDocument.java index edcc15934e3..1953154bf34 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/ByteBufBsonDocument.java +++ b/driver-core/src/main/com/mongodb/internal/connection/ByteBufBsonDocument.java @@ -299,12 +299,14 @@ public boolean isEmpty() { @Override public boolean containsKey(final Object key) { ensureOpen(); - if (cachedDocument != null) { - return cachedDocument.containsKey(key); - } if (key == null) { throw new IllegalArgumentException("key can not be null"); } + + if (cachedDocument != null) { + return cachedDocument.containsKey(key); + } + // Check sequence fields first (fast HashMap lookup), then scan body if (sequenceFields.containsKey(key)) { return true; @@ -396,7 +398,6 @@ public Set keySet() { @Override public BsonReader asBsonReader() { - ensureOpen(); // Must hydrate first since we need to include sequence fields return toBsonDocument().asBsonReader(); } @@ -452,20 +453,17 @@ public String toJson() { @Override public String toJson(final JsonWriterSettings settings) { - ensureOpen(); return toBsonDocument().toJson(settings); } @Override public String toString() { - ensureOpen(); return toBsonDocument().toString(); } @SuppressWarnings("MethodDoesntCallSuperMethod") @Override public BsonDocument clone() { - ensureOpen(); if (cachedDocument != null) { return cachedDocument.clone(); } @@ -475,13 +473,11 @@ public BsonDocument clone() { @SuppressWarnings("EqualsDoesntCheckParameterClass") @Override public boolean equals(final Object o) { - ensureOpen(); return toBsonDocument().equals(o); } @Override public int hashCode() { - ensureOpen(); return toBsonDocument().hashCode(); } @@ -537,6 +533,7 @@ public void clear() { // ==================== Private Body Field Operations ==================== // These methods read from bodyByteBuf using a temporary duplicate buffer + // Must be guarded by `ensureOpen` /** * Searches the body for a field with the given key. From 0c9895275df4147e93a5896ec44e735ee27c4cca Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Mon, 9 Mar 2026 15:19:22 +0000 Subject: [PATCH 4/4] Removed claude --- .claude/settings.local.json | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 .claude/settings.local.json diff --git a/.claude/settings.local.json b/.claude/settings.local.json deleted file mode 100644 index 3443e359782..00000000000 --- a/.claude/settings.local.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "permissions": { - "allow": [ - "Bash(./gradlew :bson:compileTestJava:*)", - "Bash(ls:*)" - ] - } -}