Skip to content

Fix NPE when reading/mapping null value inside collection. #3691

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Fix NPE when reading/mapping null value inside collection. #3691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e1651fa
Prepare issue branch.
christophstrobl Jul 1, 2021
5197834
Fix NPE when reading/mapping null value inside collection.
christophstrobl Jul 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>3.3.0-SNAPSHOT</version>
<version>3.3.0-GH-3686-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Spring Data MongoDB</name>
Expand Down
2 changes: 1 addition & 1 deletion spring-data-mongodb-benchmarks/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>3.3.0-SNAPSHOT</version>
<version>3.3.0-GH-3686-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
2 changes: 1 addition & 1 deletion spring-data-mongodb-distribution/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>3.3.0-SNAPSHOT</version>
<version>3.3.0-GH-3686-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
2 changes: 1 addition & 1 deletion spring-data-mongodb/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>3.3.0-SNAPSHOT</version>
<version>3.3.0-GH-3686-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
8 changes: 6 additions & 2 deletions ...db/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2006,12 +2006,16 @@ protected static class ConversionContext {
/**
* Converts a source object into {@link TypeInformation target}.
*
* @param source must not be {@literal null}.
* @param source can be {@literal null}.
* @param typeHint must not be {@literal null}.
* @return the converted object.
*/
@SuppressWarnings("unchecked")
public <S extends Object> S convert(Object source, TypeInformation<? extends S> typeHint) {
public <S extends Object> S convert(@Nullable Object source, TypeInformation<? extends S> typeHint) {

if(source == null) {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't that be rather caught in the map and collection iterations as we basically make convert nullable?

Copy link
Member Author

Choose a reason for hiding this comment

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

The MappingMongoConverter does not follow the o.s.c.c.converter.Converter contract and accepted null in previous versions, though the documentation did state it differently. Since it's a public API, I'd rather restore the original behaviour and alter the documentation, than spread null checks in various places.

return null;
}

Assert.notNull(typeHint, "TypeInformation must not be null");

Expand Down
39 changes: 39 additions & 0 deletions ...st/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2532,6 +2532,41 @@ void shouldWriteNullPropertyCorrectly() {
assertThat(document).containsEntry("writeAlwaysPerson", null).doesNotContainKey("writeNonNullPerson");
}

@Test // GH-3686
void readsCollectionContainingNullValue() {

org.bson.Document source = new org.bson.Document("items", Arrays.asList(new org.bson.Document("itemKey", "i1"), null, new org.bson.Document("itemKey", "i3")));

Order target = converter.read(Order.class, source);

assertThat(target.items)
.map(it -> it != null ? it.itemKey : null)
.containsExactly("i1", null, "i3");
}

@Test // GH-3686
void readsArrayContainingNullValue() {

org.bson.Document source = new org.bson.Document("arrayOfStrings", Arrays.asList("i1", null, "i3"));

WithArrays target = converter.read(WithArrays.class, source);

assertThat(target.arrayOfStrings).containsExactly("i1", null, "i3");
}

@Test // GH-3686
void readsMapContainingNullValue() {

org.bson.Document source = new org.bson.Document("mapOfObjects", new org.bson.Document("item1", "i1").append("item2", null).append("item3", "i3"));

ClassWithMapProperty target = converter.read(ClassWithMapProperty.class, source);

assertThat(target.mapOfObjects)
.containsEntry("item1", "i1")
.containsEntry("item2", null)
.containsEntry("item3", "i3");
}

static class GenericType<T> {
T content;
}
Expand Down Expand Up @@ -2893,6 +2928,10 @@ static class WithArrayInConstructor {

}

static class WithArrays {
String[] arrayOfStrings;
}

// DATAMONGO-1898

// DATACMNS-1278
Expand Down