Delay the validation of PropertyModel type data

rozza · rozza · commit eb4a4796e747 · 2019-08-09T16:31:00.000+01:00
Now ensures the conventions have been applied before validation takes place. JAVA-3286
diff --git a/bson/src/main/org/bson/codecs/pojo/ClassModelBuilder.java b/bson/src/main/org/bson/codecs/pojo/ClassModelBuilder.java
@@ -321,6 +321,9 @@ private void validatePropertyModels(final String declaringClass, final List<Prop
         Map<String, Integer> propertyWriteNameMap = new HashMap<String, Integer>();
 
         for (PropertyModel<?> propertyModel : propertyModels) {
+            if (propertyModel.hasError()) {
+                throw new CodecConfigurationException(propertyModel.getError());
+            }
             checkForDuplicates("property", propertyModel.getName(), propertyNameMap, declaringClass);
             if (propertyModel.isReadable()) {
                 checkForDuplicates("read property", propertyModel.getReadName(), propertyReadNameMap, declaringClass);
diff --git a/bson/src/main/org/bson/codecs/pojo/PojoBuilderHelper.java b/bson/src/main/org/bson/codecs/pojo/PojoBuilderHelper.java
@@ -16,8 +16,6 @@
 
 package org.bson.codecs.pojo;
 
-import org.bson.codecs.configuration.CodecConfigurationException;
-
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
@@ -38,11 +36,12 @@
 import static java.util.Arrays.asList;
 import static java.util.Collections.reverse;
 import static org.bson.assertions.Assertions.notNull;
-import static org.bson.codecs.pojo.PropertyReflectionUtils.isGetter;
 import static org.bson.codecs.pojo.PropertyReflectionUtils.getPropertyMethods;
+import static org.bson.codecs.pojo.PropertyReflectionUtils.isGetter;
 import static org.bson.codecs.pojo.PropertyReflectionUtils.toPropertyName;
 
 final class PojoBuilderHelper {
+
     @SuppressWarnings("unchecked")
     static <T> void configureClassModelBuilder(final ClassModelBuilder<T> classModelBuilder, final Class<T> clazz) {
         classModelBuilder.type(notNull("clazz", clazz));
@@ -158,11 +157,10 @@ private static <T, S> PropertyMetadata<T> getOrCreateMethodPropertyMetadata(fina
                                                                   final Type genericType) {
         PropertyMetadata<T> propertyMetadata = getOrCreatePropertyMetadata(propertyName, declaringClassName, propertyNameMap, typeData);
         if (!isAssignableClass(propertyMetadata.getTypeData().getType(), typeData.getType())) {
-            throw new CodecConfigurationException(format("Property '%s' in %s, has differing data types: %s and %s", propertyName,
+            propertyMetadata.setError(format("Property '%s' in %s, has differing data types: %s and %s.", propertyName,
                     declaringClassName, propertyMetadata.getTypeData(), typeData));
         }
-        cachePropertyTypeData(propertyMetadata, propertyTypeParameterMap, parentClassTypeData, genericTypeNames,
-                genericType);
+        cachePropertyTypeData(propertyMetadata, propertyTypeParameterMap, parentClassTypeData, genericTypeNames, genericType);
         return propertyMetadata;
     }
 
@@ -223,7 +221,8 @@ static <T> PropertyModelBuilder<T> createPropertyModelBuilder(final PropertyMeta
                 .readAnnotations(propertyMetadata.getReadAnnotations())
                 .writeAnnotations(propertyMetadata.getWriteAnnotations())
                 .propertySerialization(new PropertyModelSerializationImpl<T>())
-                .propertyAccessor(new PropertyAccessorImpl<T>(propertyMetadata));
+                .propertyAccessor(new PropertyAccessorImpl<T>(propertyMetadata))
+                .setError(propertyMetadata.getError());
 
         if (propertyMetadata.getTypeParameters() != null) {
             specializePropertyModelBuilder(propertyModelBuilder, propertyMetadata);
diff --git a/bson/src/main/org/bson/codecs/pojo/PojoCodecImpl.java b/bson/src/main/org/bson/codecs/pojo/PojoCodecImpl.java
@@ -332,7 +332,7 @@ private <V> PropertyModel<V> getSpecializedPropertyModel(final PropertyModel<V>
 
         return new PropertyModel<V>(propertyModel.getName(), propertyModel.getReadName(), propertyModel.getWriteName(),
                 specializedPropertyType, null, propertyModel.getPropertySerialization(), propertyModel.useDiscriminator(),
-                propertyModel.getPropertyAccessor());
+                propertyModel.getPropertyAccessor(), propertyModel.getError());
     }
 
     @SuppressWarnings("unchecked")
diff --git a/bson/src/main/org/bson/codecs/pojo/PropertyMetadata.java b/bson/src/main/org/bson/codecs/pojo/PropertyMetadata.java
@@ -41,6 +41,7 @@ final class PropertyMetadata<T> {
     private TypeParameterMap typeParameterMap;
     private List<TypeData<?>> typeParameters;
 
+    private String error;
     private Field field;
     private Method getter;
     private Method setter;
@@ -136,6 +137,14 @@ public <S> PropertyMetadata<T> typeParameterInfo(final TypeParameterMap typePara
         return this;
     }
 
+    String getError() {
+        return error;
+    }
+
+    void setError(final String error) {
+        this.error = error;
+    }
+
     public boolean isSerializable() {
         if (getter != null) {
             return field == null || notStaticOrTransient(field.getModifiers());
diff --git a/bson/src/main/org/bson/codecs/pojo/PropertyModel.java b/bson/src/main/org/bson/codecs/pojo/PropertyModel.java
@@ -33,11 +33,12 @@ public final class PropertyModel<T> {
     private final PropertySerialization<T> propertySerialization;
     private final Boolean useDiscriminator;
     private final PropertyAccessor<T> propertyAccessor;
+    private final String error;
     private volatile Codec<T> cachedCodec;
 
     PropertyModel(final String name, final String readName, final String writeName, final TypeData<T> typeData,
                   final Codec<T> codec, final PropertySerialization<T> propertySerialization, final Boolean useDiscriminator,
-                  final PropertyAccessor<T> propertyAccessor) {
+                  final PropertyAccessor<T> propertyAccessor, final String error) {
         this.name = name;
         this.readName = readName;
         this.writeName = writeName;
@@ -47,6 +48,7 @@ public final class PropertyModel<T> {
         this.propertySerialization = propertySerialization;
         this.useDiscriminator = useDiscriminator;
         this.propertyAccessor = propertyAccessor;
+        this.error = error;
     }
 
     /**
@@ -182,6 +184,11 @@ public boolean equals(final Object o) {
                 : that.getPropertyAccessor() != null) {
             return false;
         }
+
+        if (getError() != null ? !getError().equals(that.getError()) : that.getError() != null) {
+            return false;
+        }
+
         if (getCachedCodec() != null ? !getCachedCodec().equals(that.getCachedCodec()) : that.getCachedCodec() != null) {
             return false;
         }
@@ -199,10 +206,19 @@ public int hashCode() {
         result = 31 * result + (getPropertySerialization() != null ? getPropertySerialization().hashCode() : 0);
         result = 31 * result + (useDiscriminator != null ? useDiscriminator.hashCode() : 0);
         result = 31 * result + (getPropertyAccessor() != null ? getPropertyAccessor().hashCode() : 0);
+        result = 31 * result + (getError() != null ? getError().hashCode() : 0);
         result = 31 * result + (getCachedCodec() != null ? getCachedCodec().hashCode() : 0);
         return result;
     }
 
+    boolean hasError() {
+        return error != null;
+    }
+
+    String getError() {
+        return error;
+    }
+
     PropertySerialization<T> getPropertySerialization() {
         return propertySerialization;
     }
diff --git a/bson/src/main/org/bson/codecs/pojo/PropertyModelBuilder.java b/bson/src/main/org/bson/codecs/pojo/PropertyModelBuilder.java
@@ -45,6 +45,7 @@ public final class PropertyModelBuilder<T> {
     private List<Annotation> readAnnotations = emptyList();
     private List<Annotation> writeAnnotations = emptyList();
     private Boolean discriminatorEnabled;
+    private String error;
 
     PropertyModelBuilder() {
     }
@@ -246,7 +247,8 @@ public PropertyModel<T> build() {
                 codec,
                 stateNotNull("propertySerialization", propertySerialization),
                 discriminatorEnabled,
-                stateNotNull("propertyAccessor", propertyAccessor));
+                stateNotNull("propertyAccessor", propertyAccessor),
+                error);
     }
 
     @Override
@@ -267,4 +269,9 @@ PropertyModelBuilder<T> typeData(final TypeData<T> typeData) {
         this.typeData = notNull("typeData", typeData);
         return this;
     }
+
+    PropertyModelBuilder<T> setError(final String error) {
+        this.error = error;
+        return this;
+    }
 }
diff --git a/bson/src/test/unit/org/bson/codecs/pojo/ConventionsTest.java b/bson/src/test/unit/org/bson/codecs/pojo/ConventionsTest.java
@@ -24,6 +24,7 @@
 import org.bson.codecs.pojo.entities.conventions.AnnotationNameCollision;
 import org.bson.codecs.pojo.entities.conventions.AnnotationWithObjectIdModel;
 import org.bson.codecs.pojo.entities.conventions.AnnotationWriteCollision;
+import org.bson.codecs.pojo.entities.conventions.BsonIgnoreDuplicatePropertyMultipleTypes;
 import org.bson.codecs.pojo.entities.conventions.CreatorInvalidConstructorModel;
 import org.bson.codecs.pojo.entities.conventions.CreatorInvalidMethodModel;
 import org.bson.codecs.pojo.entities.conventions.CreatorInvalidMethodReturnTypeModel;
@@ -40,6 +41,7 @@
 import static org.bson.codecs.pojo.Conventions.ANNOTATION_CONVENTION;
 import static org.bson.codecs.pojo.Conventions.CLASS_AND_PROPERTY_CONVENTION;
 import static org.bson.codecs.pojo.Conventions.DEFAULT_CONVENTIONS;
+import static org.bson.codecs.pojo.Conventions.NO_CONVENTIONS;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -203,6 +205,12 @@ public void testCreatorInvalidTypeMethodModel() {
                 .conventions(singletonList(ANNOTATION_CONVENTION)).build();
     }
 
+    @Test(expected = CodecConfigurationException.class)
+    public void testBsonIgnoreDuplicatePropertyMultipleTypesModel() {
+        ClassModel.builder(BsonIgnoreDuplicatePropertyMultipleTypes.class)
+                .conventions(NO_CONVENTIONS).build();
+    }
+
     private class PropertyAccessorTest<T> implements PropertyAccessor<T> {
 
         @Override
diff --git a/bson/src/test/unit/org/bson/codecs/pojo/PojoRoundTripTest.java b/bson/src/test/unit/org/bson/codecs/pojo/PojoRoundTripTest.java
@@ -67,6 +67,7 @@
 import org.bson.codecs.pojo.entities.TreeWithIdModel;
 import org.bson.codecs.pojo.entities.UpperBoundsConcreteModel;
 import org.bson.codecs.pojo.entities.conventions.AnnotationBsonPropertyIdModel;
+import org.bson.codecs.pojo.entities.conventions.BsonIgnoreDuplicatePropertyMultipleTypes;
 import org.bson.codecs.pojo.entities.conventions.BsonIgnoreInvalidMapModel;
 import org.bson.codecs.pojo.entities.conventions.BsonIgnoreSyntheticProperty;
 import org.bson.codecs.pojo.entities.conventions.CollectionDiscriminatorAbstractClassesModel;
@@ -427,6 +428,11 @@ private static List<TestData> testCases() {
                 getPojoCodecProviderBuilder(DuplicateAnnotationAllowedModel.class).conventions(Conventions.DEFAULT_CONVENTIONS),
                 "{'_id': 'abc'}"));
 
+        data.add(new TestData("BsonIgnore duplicate property with multiple types",
+                new BsonIgnoreDuplicatePropertyMultipleTypes("string value"),
+                getPojoCodecProviderBuilder(BsonIgnoreDuplicatePropertyMultipleTypes.class).conventions(Conventions.DEFAULT_CONVENTIONS),
+                "{stringField: 'string value'}"));
+
         return data;
     }
 
diff --git a/bson/src/test/unit/org/bson/codecs/pojo/entities/conventions/BsonIgnoreDuplicatePropertyMultipleTypes.java b/bson/src/test/unit/org/bson/codecs/pojo/entities/conventions/BsonIgnoreDuplicatePropertyMultipleTypes.java
@@ -0,0 +1,82 @@
+/*
+ * 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.conventions;
+
+import org.bson.codecs.pojo.annotations.BsonCreator;
+import org.bson.codecs.pojo.annotations.BsonIgnore;
+import org.bson.codecs.pojo.annotations.BsonProperty;
+
+public class BsonIgnoreDuplicatePropertyMultipleTypes {
+    private final String stringField;
+    private String altStringField;
+
+    @BsonCreator
+    public BsonIgnoreDuplicatePropertyMultipleTypes(@BsonProperty("stringField") final String stringField) {
+        this.stringField = stringField;
+    }
+
+    public String getStringField() {
+        return stringField;
+    }
+
+    @BsonIgnore
+    public String getAltStringField() {
+        return altStringField;
+    }
+
+    @BsonIgnore
+    public void setAltStringField(String altStringField) {
+        this.altStringField = altStringField;
+    }
+
+    @BsonIgnore
+    public void setAltStringField(Integer i) {
+        this.altStringField = i.toString();
+    }
+
+    @Override
+    public String toString() {
+        return "BsonIgnoreDuplicatePropertyMultipleTypes{"
+                + "stringField='" + stringField + '\''
+                + ", altStringField='" + altStringField + '\''
+                + '}';
+    }
+
+    @Override
+    public boolean equals(final Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+
+        BsonIgnoreDuplicatePropertyMultipleTypes that = (BsonIgnoreDuplicatePropertyMultipleTypes) o;
+
+        if (stringField != null ? !stringField.equals(that.stringField) : that.stringField != null) {
+            return false;
+        }
+        return altStringField != null ? altStringField.equals(that.altStringField) : that.altStringField == null;
+    }
+
+    @Override
+    public int hashCode() {
+        int result = stringField != null ? stringField.hashCode() : 0;
+        result = 31 * result + (altStringField != null ? altStringField.hashCode() : 0);
+        return result;
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -332,7 +332,7 @@ private <V> PropertyModel<V> getSpecializedPropertyModel(final PropertyModel<V>`
`332`	`332`
`333`	`333`	`return new PropertyModel<V>(propertyModel.getName(), propertyModel.getReadName(), propertyModel.getWriteName(),`
`334`	`334`	`specializedPropertyType, null, propertyModel.getPropertySerialization(), propertyModel.useDiscriminator(),`
`335`		`- propertyModel.getPropertyAccessor());`
	`335`	`+ propertyModel.getPropertyAccessor(), propertyModel.getError());`
`336`	`336`	`}`
`337`	`337`
`338`	`338`	`@SuppressWarnings("unchecked")`
Original file line number	Diff line number	Diff line change
`@@ -45,6 +45,7 @@ public final class PropertyModelBuilder<T> {`
`45`	`45`	`private List<Annotation> readAnnotations = emptyList();`
`46`	`46`	`private List<Annotation> writeAnnotations = emptyList();`
`47`	`47`	`private Boolean discriminatorEnabled;`
	`48`	`+ private String error;`
`48`	`49`
`49`	`50`	`PropertyModelBuilder() {`
`50`	`51`	`}`
`@@ -246,7 +247,8 @@ public PropertyModel<T> build() {`
`246`	`247`	`codec,`
`247`	`248`	`stateNotNull("propertySerialization", propertySerialization),`
`248`	`249`	`discriminatorEnabled,`
`249`		`- stateNotNull("propertyAccessor", propertyAccessor));`
	`250`	`+ stateNotNull("propertyAccessor", propertyAccessor),`
	`251`	`+ error);`
`250`	`252`	`}`
`251`	`253`
`252`	`254`	`@Override`
`@@ -267,4 +269,9 @@ PropertyModelBuilder<T> typeData(final TypeData<T> typeData) {`
`267`	`269`	`this.typeData = notNull("typeData", typeData);`
`268`	`270`	`return this;`
`269`	`271`	`}`
	`272`	`+`
	`273`	`+ PropertyModelBuilder<T> setError(final String error) {`
	`274`	`+ this.error = error;`
	`275`	`+ return this;`
	`276`	`+ }`
`270`	`277`	`}`