From 12e70327c04384ba3b41a2d4794ecd5d1bf04ce0 Mon Sep 17 00:00:00 2001 From: Gordon Wang Date: Mon, 16 Oct 2023 18:44:14 +1100 Subject: [PATCH 1/4] Support stored nulls for nullable fields in Java records --- .../src/main/org/bson/codecs/record/RecordCodec.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/bson-record-codec/src/main/org/bson/codecs/record/RecordCodec.java b/bson-record-codec/src/main/org/bson/codecs/record/RecordCodec.java index 8a304760f31..2dd7353756e 100644 --- a/bson-record-codec/src/main/org/bson/codecs/record/RecordCodec.java +++ b/bson-record-codec/src/main/org/bson/codecs/record/RecordCodec.java @@ -62,6 +62,7 @@ private static final class ComponentModel { private final Codec codec; private final int index; private final String fieldName; + private final boolean isNullable; private ComponentModel(final List typeParameters, final RecordComponent component, final CodecRegistry codecRegistry, final int index) { @@ -70,6 +71,7 @@ private ComponentModel(final List typeParameters, final RecordComponent co this.codec = computeCodec(typeParameters, component, codecRegistry); this.index = index; this.fieldName = computeFieldName(component); + this.isNullable = checkFieldIsNullable(component); } String getComponentName() { @@ -156,6 +158,14 @@ private static String computeFieldName(final RecordComponent component) { return component.getName(); } + private static boolean checkFieldIsNullable(final RecordComponent component) { + try { + return component.getDeclaringRecord().getDeclaredField(component.getName()).isAnnotationPresent(Nullable.class); + } catch (NoSuchFieldException e) { + throw new AssertionError(format("Unexpectedly missing the declared field for recordComponent %s", component), e); + } + } + private static boolean isAnnotationPresentOnField(final RecordComponent component, final Class annotation) { try { @@ -275,6 +285,8 @@ public T decode(final BsonReader reader, final DecoderContext decoderContext) { if (LOGGER.isTraceEnabled()) { LOGGER.trace(format("Found property not present in the ClassModel: %s", fieldName)); } + } else if (reader.getCurrentBsonType() == BsonType.NULL && componentModel.isNullable) { + reader.readNull(); } else { constructorArguments[componentModel.index] = decoderContext.decodeWithChildContext(componentModel.codec, reader); } From a32d6aecfe971ed3f0486c1fad5d1ec25cf4732f Mon Sep 17 00:00:00 2001 From: Gordon Wang Date: Mon, 16 Oct 2023 19:01:51 +1100 Subject: [PATCH 2/4] Implement test --- .../bson/codecs/record/RecordCodecTest.java | 18 ++++++++++++++++++ .../TestRecordWithNullableAnnotation.java | 9 +++++++++ 2 files changed, 27 insertions(+) create mode 100644 bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableAnnotation.java diff --git a/bson-record-codec/src/test/unit/org/bson/codecs/record/RecordCodecTest.java b/bson-record-codec/src/test/unit/org/bson/codecs/record/RecordCodecTest.java index 606bc68e59a..c0e0ea4a68d 100644 --- a/bson-record-codec/src/test/unit/org/bson/codecs/record/RecordCodecTest.java +++ b/bson-record-codec/src/test/unit/org/bson/codecs/record/RecordCodecTest.java @@ -22,6 +22,7 @@ import org.bson.BsonDocumentWriter; import org.bson.BsonDouble; import org.bson.BsonInt32; +import org.bson.BsonNull; import org.bson.BsonObjectId; import org.bson.BsonString; import org.bson.codecs.DecoderContext; @@ -49,6 +50,7 @@ import org.bson.codecs.record.samples.TestRecordWithMapOfRecords; import org.bson.codecs.record.samples.TestRecordWithNestedParameterized; import org.bson.codecs.record.samples.TestRecordWithNestedParameterizedRecord; +import org.bson.codecs.record.samples.TestRecordWithNullableAnnotation; import org.bson.codecs.record.samples.TestRecordWithParameterizedRecord; import org.bson.codecs.record.samples.TestRecordWithPojoAnnotations; import org.bson.codecs.record.samples.TestSelfReferentialHolderRecord; @@ -325,6 +327,22 @@ public void testRecordWithNulls() { assertEquals(testRecord, decoded); } + @Test + public void testRecordWithStoredNulls() { + var codec = createRecordCodec(TestRecordWithNullableAnnotation.class, Bson.DEFAULT_CODEC_REGISTRY); + var identifier = new ObjectId(); + var testRecord = new TestRecordWithNullableAnnotation(identifier, null); + + var document = new BsonDocument("_id", new BsonObjectId(identifier)) + .append("name", new BsonNull()); + + // when + var decoded = codec.decode(new BsonDocumentReader(document), DecoderContext.builder().build()); + + // then + assertEquals(testRecord, decoded); + } + @Test public void testRecordWithExtraData() { var codec = createRecordCodec(TestRecordWithDeprecatedAnnotations.class, Bson.DEFAULT_CODEC_REGISTRY); diff --git a/bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableAnnotation.java b/bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableAnnotation.java new file mode 100644 index 00000000000..f8b7918ad34 --- /dev/null +++ b/bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableAnnotation.java @@ -0,0 +1,9 @@ +package org.bson.codecs.record.samples; + +import org.bson.codecs.pojo.annotations.BsonId; +import org.bson.types.ObjectId; + +import javax.annotation.Nullable; + +public record TestRecordWithNullableAnnotation (@BsonId ObjectId id, @Nullable String name) { +} From 1f4c3679441c856123b61f6bdc1f0b267446f42e Mon Sep 17 00:00:00 2001 From: Gordon Wang Date: Sun, 22 Oct 2023 15:05:34 +1100 Subject: [PATCH 3/4] Address comments --- .../org/bson/codecs/record/RecordCodec.java | 10 +------- .../bson/codecs/record/RecordCodecTest.java | 6 ++--- .../TestRecordWithNullableAnnotation.java | 9 -------- .../samples/TestRecordWithNullableField.java | 23 +++++++++++++++++++ 4 files changed, 27 insertions(+), 21 deletions(-) delete mode 100644 bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableAnnotation.java create mode 100644 bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableField.java diff --git a/bson-record-codec/src/main/org/bson/codecs/record/RecordCodec.java b/bson-record-codec/src/main/org/bson/codecs/record/RecordCodec.java index 2dd7353756e..39b75e8cf3d 100644 --- a/bson-record-codec/src/main/org/bson/codecs/record/RecordCodec.java +++ b/bson-record-codec/src/main/org/bson/codecs/record/RecordCodec.java @@ -71,7 +71,7 @@ private ComponentModel(final List typeParameters, final RecordComponent co this.codec = computeCodec(typeParameters, component, codecRegistry); this.index = index; this.fieldName = computeFieldName(component); - this.isNullable = checkFieldIsNullable(component); + this.isNullable = !component.getType().isPrimitive(); } String getComponentName() { @@ -158,14 +158,6 @@ private static String computeFieldName(final RecordComponent component) { return component.getName(); } - private static boolean checkFieldIsNullable(final RecordComponent component) { - try { - return component.getDeclaringRecord().getDeclaredField(component.getName()).isAnnotationPresent(Nullable.class); - } catch (NoSuchFieldException e) { - throw new AssertionError(format("Unexpectedly missing the declared field for recordComponent %s", component), e); - } - } - private static boolean isAnnotationPresentOnField(final RecordComponent component, final Class annotation) { try { diff --git a/bson-record-codec/src/test/unit/org/bson/codecs/record/RecordCodecTest.java b/bson-record-codec/src/test/unit/org/bson/codecs/record/RecordCodecTest.java index c0e0ea4a68d..176ba33c294 100644 --- a/bson-record-codec/src/test/unit/org/bson/codecs/record/RecordCodecTest.java +++ b/bson-record-codec/src/test/unit/org/bson/codecs/record/RecordCodecTest.java @@ -50,7 +50,7 @@ import org.bson.codecs.record.samples.TestRecordWithMapOfRecords; import org.bson.codecs.record.samples.TestRecordWithNestedParameterized; import org.bson.codecs.record.samples.TestRecordWithNestedParameterizedRecord; -import org.bson.codecs.record.samples.TestRecordWithNullableAnnotation; +import org.bson.codecs.record.samples.TestRecordWithNullableField; import org.bson.codecs.record.samples.TestRecordWithParameterizedRecord; import org.bson.codecs.record.samples.TestRecordWithPojoAnnotations; import org.bson.codecs.record.samples.TestSelfReferentialHolderRecord; @@ -329,9 +329,9 @@ public void testRecordWithNulls() { @Test public void testRecordWithStoredNulls() { - var codec = createRecordCodec(TestRecordWithNullableAnnotation.class, Bson.DEFAULT_CODEC_REGISTRY); + var codec = createRecordCodec(TestRecordWithNullableField.class, Bson.DEFAULT_CODEC_REGISTRY); var identifier = new ObjectId(); - var testRecord = new TestRecordWithNullableAnnotation(identifier, null); + var testRecord = new TestRecordWithNullableField(identifier, null); var document = new BsonDocument("_id", new BsonObjectId(identifier)) .append("name", new BsonNull()); diff --git a/bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableAnnotation.java b/bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableAnnotation.java deleted file mode 100644 index f8b7918ad34..00000000000 --- a/bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableAnnotation.java +++ /dev/null @@ -1,9 +0,0 @@ -package org.bson.codecs.record.samples; - -import org.bson.codecs.pojo.annotations.BsonId; -import org.bson.types.ObjectId; - -import javax.annotation.Nullable; - -public record TestRecordWithNullableAnnotation (@BsonId ObjectId id, @Nullable String name) { -} diff --git a/bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableField.java b/bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableField.java new file mode 100644 index 00000000000..eb098305dde --- /dev/null +++ b/bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableField.java @@ -0,0 +1,23 @@ +/* + * 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.codecs.record.samples; + +import org.bson.codecs.pojo.annotations.BsonId; +import org.bson.types.ObjectId; + +public record TestRecordWithNullableField(@BsonId ObjectId id, String name) { +} From e3e8b11a868fc04dad4f499915e27563674eb365 Mon Sep 17 00:00:00 2001 From: Gordon Wang Date: Mon, 23 Oct 2023 11:08:55 +1100 Subject: [PATCH 4/4] Address comments --- .../org/bson/codecs/record/RecordCodec.java | 6 +++++- .../bson/codecs/record/RecordCodecTest.java | 18 ++++++++++++++++-- .../samples/TestRecordWithNullableField.java | 2 +- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/bson-record-codec/src/main/org/bson/codecs/record/RecordCodec.java b/bson-record-codec/src/main/org/bson/codecs/record/RecordCodec.java index 39b75e8cf3d..5bce0560233 100644 --- a/bson-record-codec/src/main/org/bson/codecs/record/RecordCodec.java +++ b/bson-record-codec/src/main/org/bson/codecs/record/RecordCodec.java @@ -16,6 +16,7 @@ package org.bson.codecs.record; +import org.bson.BsonInvalidOperationException; import org.bson.BsonReader; import org.bson.BsonType; import org.bson.BsonWriter; @@ -277,7 +278,10 @@ public T decode(final BsonReader reader, final DecoderContext decoderContext) { if (LOGGER.isTraceEnabled()) { LOGGER.trace(format("Found property not present in the ClassModel: %s", fieldName)); } - } else if (reader.getCurrentBsonType() == BsonType.NULL && componentModel.isNullable) { + } else if (reader.getCurrentBsonType() == BsonType.NULL) { + if (!componentModel.isNullable) { + throw new BsonInvalidOperationException(format("Null value on primitive field: %s", componentModel.fieldName)); + } reader.readNull(); } else { constructorArguments[componentModel.index] = decoderContext.decodeWithChildContext(componentModel.codec, reader); diff --git a/bson-record-codec/src/test/unit/org/bson/codecs/record/RecordCodecTest.java b/bson-record-codec/src/test/unit/org/bson/codecs/record/RecordCodecTest.java index 176ba33c294..636554443fd 100644 --- a/bson-record-codec/src/test/unit/org/bson/codecs/record/RecordCodecTest.java +++ b/bson-record-codec/src/test/unit/org/bson/codecs/record/RecordCodecTest.java @@ -22,6 +22,7 @@ import org.bson.BsonDocumentWriter; import org.bson.BsonDouble; import org.bson.BsonInt32; +import org.bson.BsonInvalidOperationException; import org.bson.BsonNull; import org.bson.BsonObjectId; import org.bson.BsonString; @@ -331,10 +332,11 @@ public void testRecordWithNulls() { public void testRecordWithStoredNulls() { var codec = createRecordCodec(TestRecordWithNullableField.class, Bson.DEFAULT_CODEC_REGISTRY); var identifier = new ObjectId(); - var testRecord = new TestRecordWithNullableField(identifier, null); + var testRecord = new TestRecordWithNullableField(identifier, null, 42); var document = new BsonDocument("_id", new BsonObjectId(identifier)) - .append("name", new BsonNull()); + .append("name", new BsonNull()) + .append("age", new BsonInt32(42)); // when var decoded = codec.decode(new BsonDocumentReader(document), DecoderContext.builder().build()); @@ -343,6 +345,18 @@ public void testRecordWithStoredNulls() { assertEquals(testRecord, decoded); } + @Test + public void testExceptionsWithStoredNullsOnPrimitiveField() { + var codec = createRecordCodec(TestRecordWithNullableField.class, Bson.DEFAULT_CODEC_REGISTRY); + + var document = new BsonDocument("_id", new BsonObjectId(new ObjectId())) + .append("name", new BsonString("Felix")) + .append("age", new BsonNull()); + + assertThrows(BsonInvalidOperationException.class, () -> + codec.decode(new BsonDocumentReader(document), DecoderContext.builder().build())); + } + @Test public void testRecordWithExtraData() { var codec = createRecordCodec(TestRecordWithDeprecatedAnnotations.class, Bson.DEFAULT_CODEC_REGISTRY); diff --git a/bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableField.java b/bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableField.java index eb098305dde..f2329c8170e 100644 --- a/bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableField.java +++ b/bson-record-codec/src/test/unit/org/bson/codecs/record/samples/TestRecordWithNullableField.java @@ -19,5 +19,5 @@ import org.bson.codecs.pojo.annotations.BsonId; import org.bson.types.ObjectId; -public record TestRecordWithNullableField(@BsonId ObjectId id, String name) { +public record TestRecordWithNullableField(@BsonId ObjectId id, String name, int age) { }