diff --git a/bson/src/main/org/bson/codecs/pojo/ClassModelBuilder.java b/bson/src/main/org/bson/codecs/pojo/ClassModelBuilder.java index 018cd150347..9b7e336cde1 100644 --- a/bson/src/main/org/bson/codecs/pojo/ClassModelBuilder.java +++ b/bson/src/main/org/bson/codecs/pojo/ClassModelBuilder.java @@ -43,7 +43,7 @@ * @see ClassModel */ public class ClassModelBuilder { - private static final String ID_PROPERTY_NAME = "_id"; + static final String ID_PROPERTY_NAME = "_id"; private final List> propertyModelBuilders = new ArrayList>(); private InstanceCreatorFactory instanceCreatorFactory; private Class type; diff --git a/bson/src/main/org/bson/codecs/pojo/ConventionAnnotationImpl.java b/bson/src/main/org/bson/codecs/pojo/ConventionAnnotationImpl.java index bdba6e36766..ae2ec291f4b 100644 --- a/bson/src/main/org/bson/codecs/pojo/ConventionAnnotationImpl.java +++ b/bson/src/main/org/bson/codecs/pojo/ConventionAnnotationImpl.java @@ -136,24 +136,64 @@ private void processCreatorAnnotation(final ClassModelBuilder classModelB throw creatorExecutable.getError(clazz, "All parameters must be annotated with a @BsonProperty in %s"); } for (int i = 0; i < properties.size(); i++) { - BsonProperty bsonProperty = properties.get(i); + // BsonId may be missing, and thus creatorExecutable.getIdPropertyIndex() may be null. + boolean isIdProperty = Integer.valueOf(i).equals(creatorExecutable.getIdPropertyIndex()); Class parameterType = parameterTypes.get(i); - PropertyModelBuilder propertyModelBuilder = classModelBuilder.getProperty(bsonProperty.value()); - if (propertyModelBuilder == null) { - addCreatorPropertyToClassModelBuilder(classModelBuilder, bsonProperty.value(), parameterType); - } else if (!propertyModelBuilder.getTypeData().isAssignableFrom(parameterType)) { + PropertyModelBuilder propertyModelBuilder = null; + + if (isIdProperty) { + // This handles the BsonId annotation on the BsonCreator parameter. + propertyModelBuilder = classModelBuilder.getProperty(classModelBuilder.getIdPropertyName()); + } else { + BsonProperty bsonProperty = properties.get(i); + + // Find the property using write name and falls back to read name + for (PropertyModelBuilder builder : classModelBuilder.getPropertyModelBuilders()) { + if (bsonProperty.value().equals(builder.getWriteName())) { + // When there is a property that matches the write name of the parameter, use it and stop looking + propertyModelBuilder = builder; + break; + } + + if (bsonProperty.value().equals(builder.getReadName())) { + // When there is a property that matches the read name of the parameter, save it but continue to look + // This is so just in case there is another property that matches the write name. + propertyModelBuilder = builder; + } + } + + if (propertyModelBuilder == null) { + // BsonProperty value should be the BSON property name (write name). However, in legacy, when BsonProperty is used + // in BsonCreator parameters, it is mapped to the actual POJO property name (e.g. method name or field name). + // To support it, we still need to look it up. + propertyModelBuilder = classModelBuilder.getProperty(bsonProperty.value()); + } + + if (propertyModelBuilder == null) { + propertyModelBuilder = addCreatorPropertyToClassModelBuilder(classModelBuilder, bsonProperty.value(), + parameterType); + } else { + // An existing property is found, set its write name + propertyModelBuilder.writeName(bsonProperty.value()); + } + } + + if (!propertyModelBuilder.getTypeData().isAssignableFrom(parameterType)) { throw creatorExecutable.getError(clazz, format("Invalid Property type for '%s'. Expected %s, found %s.", - bsonProperty.value(), propertyModelBuilder.getTypeData().getType(), parameterType)); + propertyModelBuilder.getWriteName(), propertyModelBuilder.getTypeData().getType(), parameterType)); } } classModelBuilder.instanceCreatorFactory(new InstanceCreatorFactoryImpl(creatorExecutable)); } } - private void addCreatorPropertyToClassModelBuilder(final ClassModelBuilder classModelBuilder, final String name, - final Class clazz) { - classModelBuilder.addProperty(createPropertyModelBuilder(new PropertyMetadata(name, classModelBuilder.getType().getSimpleName(), - TypeData.builder(clazz).build())).readName(null).writeName(name)); + private PropertyModelBuilder addCreatorPropertyToClassModelBuilder(final ClassModelBuilder classModelBuilder, + final String name, + final Class clazz) { + PropertyModelBuilder propertyModelBuilder = createPropertyModelBuilder(new PropertyMetadata(name, + classModelBuilder.getType().getSimpleName(), TypeData.builder(clazz).build())).readName(null).writeName(name); + classModelBuilder.addProperty(propertyModelBuilder); + return propertyModelBuilder; } private void cleanPropertyBuilders(final ClassModelBuilder classModelBuilder) { diff --git a/bson/src/main/org/bson/codecs/pojo/CreatorExecutable.java b/bson/src/main/org/bson/codecs/pojo/CreatorExecutable.java index e2fcf1383a9..afda59bf4a7 100644 --- a/bson/src/main/org/bson/codecs/pojo/CreatorExecutable.java +++ b/bson/src/main/org/bson/codecs/pojo/CreatorExecutable.java @@ -17,6 +17,7 @@ package org.bson.codecs.pojo; import org.bson.codecs.configuration.CodecConfigurationException; +import org.bson.codecs.pojo.annotations.BsonId; import org.bson.codecs.pojo.annotations.BsonProperty; import java.lang.annotation.Annotation; @@ -33,6 +34,7 @@ final class CreatorExecutable { private final Constructor constructor; private final Method method; private final List properties = new ArrayList(); + private final Integer idPropertyIndex; private final List> parameterTypes = new ArrayList>(); CreatorExecutable(final Class clazz, final Constructor constructor) { @@ -47,21 +49,32 @@ private CreatorExecutable(final Class clazz, final Constructor constructor this.clazz = clazz; this.constructor = constructor; this.method = method; + Integer idPropertyIndex = null; if (constructor != null || method != null) { parameterTypes.addAll(asList(constructor != null ? constructor.getParameterTypes() : method.getParameterTypes())); Annotation[][] parameterAnnotations = constructor != null ? constructor.getParameterAnnotations() : method.getParameterAnnotations(); - for (Annotation[] parameterAnnotation : parameterAnnotations) { + for (int i = 0; i < parameterAnnotations.length; ++i) { + Annotation[] parameterAnnotation = parameterAnnotations[i]; + for (Annotation annotation : parameterAnnotation) { if (annotation.annotationType().equals(BsonProperty.class)) { properties.add((BsonProperty) annotation); break; } + + if (annotation.annotationType().equals(BsonId.class)) { + properties.add(null); + idPropertyIndex = i; + break; + } } } } + + this.idPropertyIndex = idPropertyIndex; } public Class getType() { @@ -72,6 +85,10 @@ List getProperties() { return properties; } + Integer getIdPropertyIndex() { + return idPropertyIndex; + } + public List> getParameterTypes() { return parameterTypes; } diff --git a/bson/src/main/org/bson/codecs/pojo/InstanceCreatorImpl.java b/bson/src/main/org/bson/codecs/pojo/InstanceCreatorImpl.java index 07c99ca17e6..9b41ff67abc 100644 --- a/bson/src/main/org/bson/codecs/pojo/InstanceCreatorImpl.java +++ b/bson/src/main/org/bson/codecs/pojo/InstanceCreatorImpl.java @@ -41,9 +41,18 @@ final class InstanceCreatorImpl implements InstanceCreator { } else { this.cachedValues = new HashMap, Object>(); this.properties = new HashMap(); + + if (creatorExecutable.getIdPropertyIndex() != null) { + this.properties.put(ClassModelBuilder.ID_PROPERTY_NAME, creatorExecutable.getIdPropertyIndex()); + } + for (int i = 0; i < creatorExecutable.getProperties().size(); i++) { - this.properties.put(creatorExecutable.getProperties().get(i).value(), i); + if (!Integer.valueOf(i).equals(creatorExecutable.getIdPropertyIndex())) { + // Skip the ID property + this.properties.put(creatorExecutable.getProperties().get(i).value(), i); + } } + this.params = new Object[properties.size()]; } } @@ -54,11 +63,20 @@ public void set(final S value, final PropertyModel propertyModel) { propertyModel.getPropertyAccessor().set(newInstance, value); } else { if (!properties.isEmpty()) { - Integer index = properties.get(propertyModel.getName()); + String propertyName = propertyModel.getWriteName(); + + if (!properties.containsKey(propertyName)) { + // If the property name cannot be found using write name, falls back to the property name + // This is to provide backward compatibility with the legacy BsonProperty annotation on + // BsonCreator parameters + propertyName = propertyModel.getName(); + } + + Integer index = properties.get(propertyName); if (index != null) { params[index] = value; } - properties.remove(propertyModel.getName()); + properties.remove(propertyName); } if (properties.isEmpty()) { diff --git a/bson/src/main/org/bson/codecs/pojo/annotations/BsonId.java b/bson/src/main/org/bson/codecs/pojo/annotations/BsonId.java index 84be90294a7..b7f3871629e 100644 --- a/bson/src/main/org/bson/codecs/pojo/annotations/BsonId.java +++ b/bson/src/main/org/bson/codecs/pojo/annotations/BsonId.java @@ -32,6 +32,6 @@ */ @Documented @Retention(RetentionPolicy.RUNTIME) -@Target({ElementType.FIELD, ElementType.METHOD}) +@Target({ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER}) public @interface BsonId { } diff --git a/bson/src/test/unit/org/bson/codecs/pojo/PojoRoundTripTest.java b/bson/src/test/unit/org/bson/codecs/pojo/PojoRoundTripTest.java index e7931ed968f..4a224cdd6c8 100644 --- a/bson/src/test/unit/org/bson/codecs/pojo/PojoRoundTripTest.java +++ b/bson/src/test/unit/org/bson/codecs/pojo/PojoRoundTripTest.java @@ -55,7 +55,9 @@ import org.bson.codecs.pojo.entities.SimpleNestedPojoModel; import org.bson.codecs.pojo.entities.UpperBoundsConcreteModel; import org.bson.codecs.pojo.entities.conventions.CreatorAllFinalFieldsModel; +import org.bson.codecs.pojo.entities.conventions.CreatorConstructorIdModel; import org.bson.codecs.pojo.entities.conventions.CreatorConstructorModel; +import org.bson.codecs.pojo.entities.conventions.CreatorConstructorRenameModel; import org.bson.codecs.pojo.entities.conventions.CreatorMethodModel; import org.bson.codecs.pojo.entities.conventions.CreatorNoArgsConstructorModel; import org.bson.codecs.pojo.entities.conventions.CreatorNoArgsMethodModel; @@ -231,6 +233,14 @@ private static List testCases() { getPojoCodecProviderBuilder(CreatorConstructorModel.class), "{'integersField': [10, 11], 'stringField': 'twelve', 'longField': {$numberLong: '13'}}")); + data.add(new TestData("Creator constructor with rename", new CreatorConstructorRenameModel(asList(10, 11), "twelve", 13), + getPojoCodecProviderBuilder(CreatorConstructorRenameModel.class), + "{'integerList': [10, 11], 'stringField': 'twelve', 'longField': {$numberLong: '13'}}")); + + data.add(new TestData("Creator constructor with ID", new CreatorConstructorIdModel("1234-34567-890", asList(10, 11), "twelve", 13), + getPojoCodecProviderBuilder(CreatorConstructorIdModel.class), + "{'_id': '1234-34567-890', 'integersField': [10, 11], 'stringField': 'twelve', 'longField': {$numberLong: '13'}}")); + data.add(new TestData("Creator no-args constructor", new CreatorNoArgsConstructorModel(40, "one", 42), getPojoCodecProviderBuilder(CreatorNoArgsConstructorModel.class), "{'integerField': 40, 'stringField': 'one', 'longField': {$numberLong: '42'}}")); diff --git a/bson/src/test/unit/org/bson/codecs/pojo/entities/conventions/CreatorConstructorIdModel.java b/bson/src/test/unit/org/bson/codecs/pojo/entities/conventions/CreatorConstructorIdModel.java new file mode 100644 index 00000000000..db6809412d5 --- /dev/null +++ b/bson/src/test/unit/org/bson/codecs/pojo/entities/conventions/CreatorConstructorIdModel.java @@ -0,0 +1,112 @@ +/* + * Copyright 2017 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.BsonId; +import org.bson.codecs.pojo.annotations.BsonProperty; + +import java.util.List; + +public class CreatorConstructorIdModel { + private final String id; + private final List integersField; + private String stringField; + public long longField; + + @BsonCreator + public CreatorConstructorIdModel(final @BsonId String id, @BsonProperty("integersField") final List integerField, + @BsonProperty("longField") final long longField) { + this.id = id; + this.integersField = integerField; + this.longField = longField; + } + + public CreatorConstructorIdModel(final String id, final List integersField, final String stringField, final long longField) { + this.id = id; + this.integersField = integersField; + this.stringField = stringField; + this.longField = longField; + } + + public String getId() { + return id; + } + + public List getIntegersField() { + return integersField; + } + + public String getStringField() { + return stringField; + } + + public void setStringField(final String stringField) { + this.stringField = stringField; + } + + public long getLongField() { + return longField; + } + + public void setLongField(final long longField) { + this.longField = longField; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + CreatorConstructorIdModel that = (CreatorConstructorIdModel) o; + + if (getLongField() != that.getLongField()) { + return false; + } + if (getId() != null ? !getId().equals(that.getId()) : that.getId() != null) { + return false; + } + if (getIntegersField() != null ? !getIntegersField().equals(that.getIntegersField()) + : that.getIntegersField() != null) { + return false; + } + return getStringField() != null ? getStringField().equals(that.getStringField()) : that.getStringField() == null; + } + + @Override + public int hashCode() { + int result = getId() != null ? getId().hashCode() : 0; + result = 31 * result + (getIntegersField() != null ? getIntegersField().hashCode() : 0); + result = 31 * result + (getStringField() != null ? getStringField().hashCode() : 0); + result = 31 * result + (int) (getLongField() ^ (getLongField() >>> 32)); + return result; + } + + @Override + public String toString() { + return "CreatorConstructorIdModel{" + + "id='" + id + '\'' + + ", integersField=" + integersField + + ", stringField='" + stringField + '\'' + + ", longField=" + longField + + '}'; + } +} diff --git a/bson/src/test/unit/org/bson/codecs/pojo/entities/conventions/CreatorConstructorRenameModel.java b/bson/src/test/unit/org/bson/codecs/pojo/entities/conventions/CreatorConstructorRenameModel.java new file mode 100644 index 00000000000..5466dad3195 --- /dev/null +++ b/bson/src/test/unit/org/bson/codecs/pojo/entities/conventions/CreatorConstructorRenameModel.java @@ -0,0 +1,103 @@ +/* + * Copyright 2017 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.BsonProperty; + +import java.util.List; + +public class CreatorConstructorRenameModel { + private final List integersField; + private String stringField; + public long longField; + + @BsonCreator + public CreatorConstructorRenameModel(@BsonProperty("integerList") final List integerField, + @BsonProperty("longField") final long longField) { + this.integersField = integerField; + this.longField = longField; + } + + public CreatorConstructorRenameModel(final List integersField, final String stringField, final long longField) { + this.integersField = integersField; + this.stringField = stringField; + this.longField = longField; + } + + @BsonProperty("integerList") + public List getIntegersField() { + return integersField; + } + + public String getStringField() { + return stringField; + } + + public void setStringField(final String stringField) { + this.stringField = stringField; + } + + public long getLongField() { + return longField; + } + + public void setLongField(final long longField) { + this.longField = longField; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + CreatorConstructorRenameModel that = (CreatorConstructorRenameModel) o; + + if (getLongField() != that.getLongField()) { + return false; + } + if (getIntegersField() != null ? !getIntegersField().equals(that.getIntegersField()) : that.getIntegersField() != null) { + return false; + } + if (getStringField() != null ? !getStringField().equals(that.getStringField()) : that.getStringField() != null) { + return false; + } + + return true; + } + + @Override + public int hashCode() { + int result = getIntegersField() != null ? getIntegersField().hashCode() : 0; + result = 31 * result + (getStringField() != null ? getStringField().hashCode() : 0); + result = 31 * result + (int) (getLongField() ^ (getLongField() >>> 32)); + return result; + } + + @Override + public String toString() { + return "CreatorConstructorRenameModel{" + + "integersField=" + integersField + + ", stringField='" + stringField + "'" + + ", longField=" + longField + + "}"; + } +}