From 9e8a299b0ebbe721ccda70cf125cb7a8ae95a1c0 Mon Sep 17 00:00:00 2001 From: Raffaele Florio Date: Thu, 16 Jan 2025 18:29:42 +0100 Subject: [PATCH 1/5] fix JAVA-5764 --- .../org/bson/codecs/pojo/PojoCodecImpl.java | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/bson/src/main/org/bson/codecs/pojo/PojoCodecImpl.java b/bson/src/main/org/bson/codecs/pojo/PojoCodecImpl.java index 96853000198..5de7d240798 100644 --- a/bson/src/main/org/bson/codecs/pojo/PojoCodecImpl.java +++ b/bson/src/main/org/bson/codecs/pojo/PojoCodecImpl.java @@ -76,13 +76,10 @@ public void encode(final BsonWriter writer, final T value, final EncoderContext writer.writeStartDocument(); encodeIdProperty(writer, value, encoderContext, classModel.getIdPropertyModelHolder()); - - if (classModel.useDiscriminator()) { - writer.writeString(classModel.getDiscriminatorKey(), classModel.getDiscriminator()); - } + encodeDiscriminatorProperty(writer); for (PropertyModel propertyModel : classModel.getPropertyModels()) { - if (propertyModel.equals(classModel.getIdPropertyModel())) { + if (idProperty(propertyModel) || discriminatorProperty(propertyModel)) { continue; } encodeProperty(writer, value, encoderContext, propertyModel); @@ -140,6 +137,23 @@ private void encodeIdProperty(final BsonWriter writer, final T instance, fin } } + private boolean idProperty(final PropertyModel propertyModel) { + return propertyModel.equals(classModel.getIdPropertyModel()); + } + + private void encodeDiscriminatorProperty(final BsonWriter writer) { + if (classModel.useDiscriminator()) { + writer.writeString(classModel.getDiscriminatorKey(), classModel.getDiscriminator()); + } + } + + private boolean discriminatorProperty(final PropertyModel propertyModel) { + if (classModel.useDiscriminator()) { + return propertyModel.getReadName().equals(classModel.getDiscriminatorKey()); + } + return false; + } + private void encodeProperty(final BsonWriter writer, final T instance, final EncoderContext encoderContext, final PropertyModel propertyModel) { if (propertyModel != null && propertyModel.isReadable()) { From 067da501a3bca16a3b31a1d94fb81a3ef019df97 Mon Sep 17 00:00:00 2001 From: Raffaele Florio Date: Wed, 22 Jan 2025 12:51:54 +0100 Subject: [PATCH 2/5] adds test about single discriminator serialization --- .../pojo/PojoCodecDiscriminatorTest.java | 50 +++++++++++++++++++ .../pojo/entities/DiscriminatorModel.java | 31 ++++++++++++ .../DiscriminatorWithGetterModel.java | 35 +++++++++++++ 3 files changed, 116 insertions(+) create mode 100644 bson/src/test/unit/org/bson/codecs/pojo/PojoCodecDiscriminatorTest.java create mode 100644 bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorModel.java create mode 100644 bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorWithGetterModel.java diff --git a/bson/src/test/unit/org/bson/codecs/pojo/PojoCodecDiscriminatorTest.java b/bson/src/test/unit/org/bson/codecs/pojo/PojoCodecDiscriminatorTest.java new file mode 100644 index 00000000000..7e3ca55e5d2 --- /dev/null +++ b/bson/src/test/unit/org/bson/codecs/pojo/PojoCodecDiscriminatorTest.java @@ -0,0 +1,50 @@ +/* + * 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.pojo; + +import org.bson.codecs.pojo.entities.DiscriminatorModel; +import org.bson.codecs.pojo.entities.DiscriminatorWithGetterModel; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; + +public final class PojoCodecDiscriminatorTest extends PojoTestCase { + + @Test + public void testDiscriminatorEncodedOnceWhenItIsAlsoAGetter() { + byte[] encodedDiscriminatorWithoutGetter = encode( + getCodec(DiscriminatorModel.class), + new DiscriminatorModel(), + false + ).toByteArray(); + byte[] encodedDiscriminatorWithGetter = encode( + getCodec(DiscriminatorWithGetterModel.class), + new DiscriminatorWithGetterModel(), + false + ).toByteArray(); + assertArrayEquals(encodedDiscriminatorWithoutGetter, encodedDiscriminatorWithGetter); + } + + @Test + public void testDiscriminatorEncodingWhenItIsAlsoAGetter() { + encodesTo( + getCodec(DiscriminatorWithGetterModel.class), + new DiscriminatorWithGetterModel(), + "{discriminatorKey:'discriminatorValue'}" + ); + } +} diff --git a/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorModel.java b/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorModel.java new file mode 100644 index 00000000000..c8f204d2c10 --- /dev/null +++ b/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorModel.java @@ -0,0 +1,31 @@ +/* + * 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.pojo.entities; + +import org.bson.codecs.pojo.annotations.BsonDiscriminator; + +@BsonDiscriminator(key = "discriminatorKey", value = "discriminatorValue") +public class DiscriminatorModel { + + public DiscriminatorModel() { + } + + @Override + public String toString() { + return "DiscriminatorWithoutGetterModel{}"; + } +} diff --git a/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorWithGetterModel.java b/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorWithGetterModel.java new file mode 100644 index 00000000000..fe8ba245bd1 --- /dev/null +++ b/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorWithGetterModel.java @@ -0,0 +1,35 @@ +/* + * 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.pojo.entities; + +import org.bson.codecs.pojo.annotations.BsonDiscriminator; + +@BsonDiscriminator(key = "discriminatorKey", value = "discriminatorValue") +public class DiscriminatorWithGetterModel { + + public DiscriminatorWithGetterModel() { + } + + public String getDiscriminatorKey() { + return "discriminatorValue"; + } + + @Override + public String toString() { + return "DiscriminatorWithGetterModel{}"; + } +} From 00db424166d66ee69dd2cf43ef151b680b10fc20 Mon Sep 17 00:00:00 2001 From: Raffaele Florio Date: Wed, 22 Jan 2025 13:06:55 +0100 Subject: [PATCH 3/5] improves PojoCodecDiscriminatorTest by replacing the encodesTo test with the roundTrip one --- .../codecs/pojo/PojoCodecDiscriminatorTest.java | 5 ++--- .../codecs/pojo/entities/DiscriminatorModel.java | 5 ----- .../entities/DiscriminatorWithGetterModel.java | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/bson/src/test/unit/org/bson/codecs/pojo/PojoCodecDiscriminatorTest.java b/bson/src/test/unit/org/bson/codecs/pojo/PojoCodecDiscriminatorTest.java index 7e3ca55e5d2..c6261ffe2d2 100644 --- a/bson/src/test/unit/org/bson/codecs/pojo/PojoCodecDiscriminatorTest.java +++ b/bson/src/test/unit/org/bson/codecs/pojo/PojoCodecDiscriminatorTest.java @@ -40,9 +40,8 @@ public void testDiscriminatorEncodedOnceWhenItIsAlsoAGetter() { } @Test - public void testDiscriminatorEncodingWhenItIsAlsoAGetter() { - encodesTo( - getCodec(DiscriminatorWithGetterModel.class), + public void testDiscriminatorRoundTripWhenItIsAlsoAGetter() { + roundTrip( new DiscriminatorWithGetterModel(), "{discriminatorKey:'discriminatorValue'}" ); diff --git a/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorModel.java b/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorModel.java index c8f204d2c10..1ef419540bd 100644 --- a/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorModel.java +++ b/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorModel.java @@ -23,9 +23,4 @@ public class DiscriminatorModel { public DiscriminatorModel() { } - - @Override - public String toString() { - return "DiscriminatorWithoutGetterModel{}"; - } } diff --git a/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorWithGetterModel.java b/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorWithGetterModel.java index fe8ba245bd1..79532daa6c7 100644 --- a/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorWithGetterModel.java +++ b/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorWithGetterModel.java @@ -18,6 +18,8 @@ import org.bson.codecs.pojo.annotations.BsonDiscriminator; +import java.util.Objects; + @BsonDiscriminator(key = "discriminatorKey", value = "discriminatorValue") public class DiscriminatorWithGetterModel { @@ -28,6 +30,18 @@ public String getDiscriminatorKey() { return "discriminatorValue"; } + @Override + public boolean equals(final Object o) { + if (o == null || getClass() != o.getClass()) return false; + final DiscriminatorWithGetterModel that = (DiscriminatorWithGetterModel) o; + return Objects.equals(getDiscriminatorKey(), that.getDiscriminatorKey()); + } + + @Override + public int hashCode() { + return Objects.hashCode(getDiscriminatorKey()); + } + @Override public String toString() { return "DiscriminatorWithGetterModel{}"; From 70052f44140c8e1219a815dbdc4bcb9539d6ed70 Mon Sep 17 00:00:00 2001 From: Raffaele Florio Date: Thu, 23 Jan 2025 07:13:14 +0100 Subject: [PATCH 4/5] adds readName test about single discriminator serialization --- .../pojo/PojoCodecDiscriminatorTest.java | 28 +++++++++- .../entities/DiscriminatorWithProperty.java | 51 +++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorWithProperty.java diff --git a/bson/src/test/unit/org/bson/codecs/pojo/PojoCodecDiscriminatorTest.java b/bson/src/test/unit/org/bson/codecs/pojo/PojoCodecDiscriminatorTest.java index c6261ffe2d2..60cb94d4e87 100644 --- a/bson/src/test/unit/org/bson/codecs/pojo/PojoCodecDiscriminatorTest.java +++ b/bson/src/test/unit/org/bson/codecs/pojo/PojoCodecDiscriminatorTest.java @@ -18,6 +18,7 @@ import org.bson.codecs.pojo.entities.DiscriminatorModel; import org.bson.codecs.pojo.entities.DiscriminatorWithGetterModel; +import org.bson.codecs.pojo.entities.DiscriminatorWithProperty; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertArrayEquals; @@ -26,7 +27,7 @@ public final class PojoCodecDiscriminatorTest extends PojoTestCase { @Test public void testDiscriminatorEncodedOnceWhenItIsAlsoAGetter() { - byte[] encodedDiscriminatorWithoutGetter = encode( + byte[] encodedDiscriminatorModel = encode( getCodec(DiscriminatorModel.class), new DiscriminatorModel(), false @@ -36,7 +37,7 @@ public void testDiscriminatorEncodedOnceWhenItIsAlsoAGetter() { new DiscriminatorWithGetterModel(), false ).toByteArray(); - assertArrayEquals(encodedDiscriminatorWithoutGetter, encodedDiscriminatorWithGetter); + assertArrayEquals(encodedDiscriminatorModel, encodedDiscriminatorWithGetter); } @Test @@ -46,4 +47,27 @@ public void testDiscriminatorRoundTripWhenItIsAlsoAGetter() { "{discriminatorKey:'discriminatorValue'}" ); } + + @Test + public void testDiscriminatorEncodedOnceWhenItIsAlsoAProperty() { + byte[] encodedDiscriminatorModel = encode( + getCodec(DiscriminatorModel.class), + new DiscriminatorModel(), + false + ).toByteArray(); + byte[] encodedDiscriminatorWithProperty = encode( + getCodec(DiscriminatorWithProperty.class), + new DiscriminatorWithProperty(), + false + ).toByteArray(); + assertArrayEquals(encodedDiscriminatorModel, encodedDiscriminatorWithProperty); + } + + @Test + public void testDiscriminatorRoundTripWhenItIsAlsoAProperty() { + roundTrip( + new DiscriminatorWithProperty(), + "{discriminatorKey:'discriminatorValue'}" + ); + } } diff --git a/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorWithProperty.java b/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorWithProperty.java new file mode 100644 index 00000000000..806e8ae123a --- /dev/null +++ b/bson/src/test/unit/org/bson/codecs/pojo/entities/DiscriminatorWithProperty.java @@ -0,0 +1,51 @@ +/* + * 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.pojo.entities; + +import org.bson.codecs.pojo.annotations.BsonDiscriminator; +import org.bson.codecs.pojo.annotations.BsonProperty; + +import java.util.Objects; + +@BsonDiscriminator(key = "discriminatorKey", value = "discriminatorValue") +public class DiscriminatorWithProperty { + + public DiscriminatorWithProperty() { + } + + @BsonProperty("discriminatorKey") + public String getDiscriminator() { + return "discriminatorValue"; + } + + @Override + public boolean equals(final Object o) { + if (o == null || getClass() != o.getClass()) return false; + final DiscriminatorWithProperty that = (DiscriminatorWithProperty) o; + return Objects.equals(getDiscriminator(), that.getDiscriminator()); + } + + @Override + public int hashCode() { + return Objects.hashCode(getDiscriminator()); + } + + @Override + public String toString() { + return "DiscriminatorWithProperty{}"; + } +} From 9ee6496bd6d8f860241c78fd825bd94704618e55 Mon Sep 17 00:00:00 2001 From: Raffaele Florio Date: Thu, 30 Jan 2025 18:40:18 +0100 Subject: [PATCH 5/5] moves the duplicate discriminator keys check in ConventionAnnotationImpl --- .../bson/codecs/pojo/ConventionAnnotationImpl.java | 13 +++++++++++++ .../main/org/bson/codecs/pojo/PojoCodecImpl.java | 9 +-------- .../org/bson/codecs/pojo/PojoCodecProvider.java | 2 +- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/bson/src/main/org/bson/codecs/pojo/ConventionAnnotationImpl.java b/bson/src/main/org/bson/codecs/pojo/ConventionAnnotationImpl.java index f27e025cd2b..83da176b563 100644 --- a/bson/src/main/org/bson/codecs/pojo/ConventionAnnotationImpl.java +++ b/bson/src/main/org/bson/codecs/pojo/ConventionAnnotationImpl.java @@ -25,6 +25,8 @@ import org.bson.codecs.pojo.annotations.BsonIgnore; import org.bson.codecs.pojo.annotations.BsonProperty; import org.bson.codecs.pojo.annotations.BsonRepresentation; +import org.bson.diagnostics.Logger; +import org.bson.diagnostics.Loggers; import java.lang.annotation.Annotation; import java.lang.reflect.Constructor; @@ -41,6 +43,8 @@ final class ConventionAnnotationImpl implements Convention { + private static final Logger LOGGER = Loggers.getLogger("ConventionAnnotation"); + @Override public void apply(final ClassModelBuilder classModelBuilder) { for (final Annotation annotation : classModelBuilder.getAnnotations()) { @@ -240,6 +244,15 @@ private void cleanPropertyBuilders(final ClassModelBuilder classModelBuilder) if (!propertyModelBuilder.isReadable() && !propertyModelBuilder.isWritable()) { propertiesToRemove.add(propertyModelBuilder.getName()); } + if (classModelBuilder.useDiscriminator() && propertyModelBuilder.getReadName().equals(classModelBuilder.getDiscriminatorKey())) { + propertiesToRemove.add(propertyModelBuilder.getName()); + LOGGER.warn( + format( + "Removed the property '%s' from the model because the discriminator has the same key", + classModelBuilder.getDiscriminatorKey() + ) + ); + } } for (String propertyName : propertiesToRemove) { classModelBuilder.removeProperty(propertyName); diff --git a/bson/src/main/org/bson/codecs/pojo/PojoCodecImpl.java b/bson/src/main/org/bson/codecs/pojo/PojoCodecImpl.java index 5de7d240798..cbcfc99b20d 100644 --- a/bson/src/main/org/bson/codecs/pojo/PojoCodecImpl.java +++ b/bson/src/main/org/bson/codecs/pojo/PojoCodecImpl.java @@ -79,7 +79,7 @@ public void encode(final BsonWriter writer, final T value, final EncoderContext encodeDiscriminatorProperty(writer); for (PropertyModel propertyModel : classModel.getPropertyModels()) { - if (idProperty(propertyModel) || discriminatorProperty(propertyModel)) { + if (idProperty(propertyModel)) { continue; } encodeProperty(writer, value, encoderContext, propertyModel); @@ -147,13 +147,6 @@ private void encodeDiscriminatorProperty(final BsonWriter writer) { } } - private boolean discriminatorProperty(final PropertyModel propertyModel) { - if (classModel.useDiscriminator()) { - return propertyModel.getReadName().equals(classModel.getDiscriminatorKey()); - } - return false; - } - private void encodeProperty(final BsonWriter writer, final T instance, final EncoderContext encoderContext, final PropertyModel propertyModel) { if (propertyModel != null && propertyModel.isReadable()) { diff --git a/bson/src/main/org/bson/codecs/pojo/PojoCodecProvider.java b/bson/src/main/org/bson/codecs/pojo/PojoCodecProvider.java index b62364b1b4b..255b520aabb 100644 --- a/bson/src/main/org/bson/codecs/pojo/PojoCodecProvider.java +++ b/bson/src/main/org/bson/codecs/pojo/PojoCodecProvider.java @@ -80,7 +80,7 @@ private PojoCodec createCodec(final Class clazz, final CodecRegistry r } else if (automatic || (clazz.getPackage() != null && packages.contains(clazz.getPackage().getName()))) { try { classModel = createClassModel(clazz, conventions); - if (clazz.isInterface() || !classModel.getPropertyModels().isEmpty()) { + if (clazz.isInterface() || !classModel.getPropertyModels().isEmpty() || classModel.useDiscriminator()) { discriminatorLookup.addClassModel(classModel); return new AutomaticPojoCodec<>(createCodec(classModel, registry, propertyCodecProviders, discriminatorLookup));