Skip to content

JAVA-6224 Ensure Kotlin can encode ByteArrays correctly - #2019

Open
rozza wants to merge 4 commits into
mongodb:mainfrom
rozza:JAVA-6224
Open

JAVA-6224 Ensure Kotlin can encode ByteArrays correctly#2019
rozza wants to merge 4 commits into
mongodb:mainfrom
rozza:JAVA-6224

Conversation

@rozza

@rozza rozza commented Jul 20, 2026

Copy link
Copy Markdown
Member
  • Fixes regression in bson-kotlin
  • Adds explicit ByteArray support to bson-kotlinx

JAVA-6224

rozza added 2 commits July 20, 2026 14:08
Narrow the DataClassCodec array interception to exclude ByteArray so it
falls through to ByteArrayCodec (BSON Binary), restoring pre-5.1.3
behavior. ArrayCodecProvider is registered before ByteArrayCodec in both
the test registry and the production KotlinCodecProvider, so it must
apply the same ByteArray exclusion and return null to let the registry
fall through to ByteArrayCodec. Object arrays still route through
ArrayCodec (JAVA-5122).

JAVA-6224
kotlinx.serialization encodes a ByteArray as a BSON array of int32
elements by default, which differs from bson-kotlin's DataClassCodec
(compact BSON Binary). Add an opt-in ByteArrayAsBsonBinary KSerializer
so users can store ByteArray fields as BsonBinary via
@serializable(with = ByteArrayAsBsonBinary::class) or @contextual plus
its serializersModule. It is not registered in defaultSerializersModule,
so existing behavior and on-disk format are unchanged.

JAVA-6224
@rozza
rozza requested a review from a team as a code owner July 20, 2026 14:55
@rozza
rozza requested review from Copilot and nhachicha and removed request for nhachicha July 20, 2026 14:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses ByteArray encoding differences/regressions across the Kotlin codecs by ensuring bson-kotlin encodes ByteArray as compact BSON Binary and by adding an opt-in kotlinx.serialization serializer to encode ByteArray as BSON Binary when desired.

Changes:

  • Update bson-kotlin codec selection so ByteArray uses ByteArrayCodec (BSON Binary) instead of the generic array codec.
  • Add an opt-in kotlinx.serialization KSerializer<ByteArray> (ByteArrayAsBsonBinary) for compact BSON Binary encoding in bson-kotlinx.
  • Extend tests and sample data classes in both modules to cover ByteArray fields and nested ByteArray arrays.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/samples/DataClasses.kt Adds sample serializable data classes exercising ByteArray and nested arrays with opt-in binary serialization.
bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodecTest.kt Adds round-trip tests documenting default ByteArray behavior and verifying opt-in binary encoding via ByteArrayAsBsonBinary.
bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/ByteArrayAsBsonBinary.kt Introduces a KSerializer<ByteArray> that encodes/decodes as compact BsonBinary and provides a contextual SerializersModule.
bson-kotlin/src/test/kotlin/org/bson/codecs/kotlin/samples/DataClasses.kt Extends Kotlin (non-kotlinx) sample data classes to include ByteArray and nested ByteArray arrays with correct equality semantics.
bson-kotlin/src/test/kotlin/org/bson/codecs/kotlin/DataClassCodecTest.kt Adds/updates tests to assert ByteArray encodes as BSON Binary and remains size-efficient; extends array test coverage.
bson-kotlin/src/main/kotlin/org/bson/codecs/kotlin/DataClassCodec.kt Prevents ByteArray from being treated as a generic array so the registry can select ByteArrayCodec.
bson-kotlin/src/main/kotlin/org/bson/codecs/kotlin/ArrayCodecProvider.kt Ensures ByteArray falls through to ByteArrayCodec by returning null for ByteArray rather than an ArrayCodec.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@rozza
rozza requested a review from nhachicha July 27, 2026 08:08

@nhachicha nhachicha left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Claude review revealed some minor things, the important one to consider is the migration/breaking existing users with this fix

Review — blocking & important items

bson-kotlin/src/main/kotlin/org/bson/codecs/kotlin/DataClassCodec.kt

  • 🟡 [important] Decoding data written by the regressed versions is now a hard failure with no migration story. Affected releases persisted ByteArray fields as BSON arrays of int32; after this fix those documents decode via ByteArrayCodec, which throws BsonInvalidOperationException ("readBinaryData can only be called when CurrentBSONType is BINARY"). Either add a lenient fallback that reads a BSON array of int32 when currentBsonType == ARRAY, or explicitly decide against leniency and document the break in JAVA-6224 / release notes (naming which released versions produced the array form). A test pinning whichever behavior is chosen would document it.

bson-kotlin/src/test/kotlin/org/bson/codecs/kotlin/DataClassCodecTest.kt

  • 🔴 [blocking] testDataClassWithObjectArrayEncodesAsBsonArray is a byte-for-byte duplicate of the updated testDataClassWithArrays — identical DataClassWithArrays instance and semantically identical expected JSON (differences are whitespace only). It adds no coverage and doubles maintenance. Suggest keeping one: either revert the byte-array additions to testDataClassWithArrays and keep the new focused test, or delete the new one.
  • 🟡 [important] In testDataClassWithByteArrayDoesNotExpandDocumentSize, document.toBsonDocument() is redundant — document is already a BsonDocument (the call returns this). Use document.getBinary("byteArray").data.size directly.

bson-kotlin/src/main/kotlin/org/bson/codecs/kotlin/ArrayCodecProvider.kt

  • 🟡 [important] Returning null for ByteArray assumes the registry contains a ByteArrayCodec. Users composing ArrayCodecProvider in a custom registry without ValueCodecProvider previously got a (verbose) working codec and will now get CodecConfigurationException. This is the intended pre-regression behavior, but worth a line in the release notes.

bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/ByteArrayAsBsonBinary.kt

  • 🟡 [important] The public serializersModule property has no KDoc. In an explicitApi() module — and referenced from the class KDoc as one of the two opt-in mechanisms — it should document what registering it does (@Contextual ByteArray fields use this serializer), with its own @since 5.10.

bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodecTest.kt

  • 🟡 [important] The @Contextual + ByteArrayAsBsonBinary.serializersModule path advertised in the KDoc has no test. Add one (data class with @Contextual val byteArray: ByteArray, codec built with the module) — contextual resolution for primitive-array types has kotlinx-serialization edge cases worth pinning. Also missing: empty/nullable ByteArray and a negative test that deserialize fails on a non-binary BSON value.
  • 🟡 [important] Comment wrapping in the two new tests is broken mid-clause ("(subType / 00),", "per-type-argument / annotation") far short of the 120-col limit, and much of the second comment narrates the change rather than stating a constraint. Suggest trimming to the one non-obvious point — the annotation inside Array<@Serializable(...) ByteArray> applies per element — and reflowing the rest.

@nhachicha nhachicha changed the title Ensure Kotlin can encode ByteArrays correctly JAVA-6224 Ensure Kotlin can encode ByteArrays correctly Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants