Skip to content
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

Fix for DATAREST-1012 - support polymorphic inline collections on PUT… #368

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix for DATAREST-1012 - support polymorphic inline collections on PUT… #368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
39 changes: 29 additions & 10 deletions ...st-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,9 @@
import lombok.RequiredArgsConstructor;

import java.io.InputStream;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.Map;
import java.util.*;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.stream.Collectors;

import org.springframework.beans.PropertyAccessor;
import org.springframework.beans.PropertyAccessorFactory;
Expand Down Expand Up @@ -62,6 +57,7 @@
* @author Mark Paluch
* @author Craig Andrews
* @author Mathias Düsterhöft
* @author Stefan Reichert
* @since 2.2
*/
@RequiredArgsConstructor
Expand Down Expand Up @@ -451,12 +447,19 @@ private Optional<Collection<Object>> mergeCollections(PersistentProperty<?> prop
: CollectionFactory.createApproximateCollection(targetCollection, sourceCollection.size());

Iterator<Object> sourceIterator = sourceCollection.iterator();
Iterator<Object> targetIterator = targetCollection == null ? Collections.emptyIterator()
: targetCollection.iterator();
// FIX DATAREST-1012: map the target collection by type to avoid incompatible type on heterogeneous
// polymorphic collections. This map is used to retrieve matching merge targets for the incoming elements
Map<Class<?>, Iterator<Object>> targetIteratorByTypeMap = createIteratorByTypeMap(targetCollection);

while (sourceIterator.hasNext()) {

Object sourceElement = sourceIterator.next();
boolean sourceElementTypeExists = targetIteratorByTypeMap.containsKey(sourceElement.getClass());

// FIX DATAREST-1012: identify an object of matching type as merge target
Iterator<Object> targetIterator = sourceElementTypeExists
? targetIteratorByTypeMap.get(sourceElement.getClass())
: Collections.emptyListIterator();
Object targetElement = targetIterator.hasNext() ? targetIterator.next() : null;

result.add(mergeForPut(sourceElement, targetElement, mapper));
Expand All @@ -479,6 +482,23 @@ private Optional<Collection<Object>> mergeCollections(PersistentProperty<?> prop
});
}

/**
* Returns a map containing target collection elements grouped by their type. The key is represented by the
* type's {@link Class} mapping a collection of the respective matching elements.
*
* @param targetCollection
* @return
*/
private Map<Class<?>, Iterator<Object>> createIteratorByTypeMap(Collection<Object> targetCollection) {
Map<Class<?>, Iterator<Object>> targetIteratorByTypeMap = new HashMap<>();
if (targetCollection != null) {
targetCollection.stream()
.collect(Collectors.groupingBy(Object::getClass))
.forEach((key, value) -> targetIteratorByTypeMap.put(key, value.iterator()));
}
return targetIteratorByTypeMap;
}

@SuppressWarnings("unchecked")
private static Collection<Object> asCollection(Object source) {

Expand All @@ -497,7 +517,6 @@ private static Collection<Object> asCollection(Object source) {
* Returns the given source instance as {@link Collection} or creates a new one for the given type.
*
* @param source can be {@literal null}.
* @param type must not be {@literal null} in case {@code source} is null.
* @return
*/
@SuppressWarnings("unchecked")
Expand Down
77 changes: 76 additions & 1 deletion .../src/test/java/org/springframework/data/rest/webmvc/json/DomainObjectReaderUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
package org.springframework.data.rest.webmvc.json;

import static com.fasterxml.jackson.annotation.JsonProperty.Access.*;
import static com.fasterxml.jackson.annotation.JsonTypeInfo.As.*;
import static javax.persistence.CascadeType.*;
import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;

Expand All @@ -26,6 +28,10 @@
import lombok.RequiredArgsConstructor;
import lombok.Value;

import javax.persistence.Basic;
import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType;
import javax.persistence.OneToMany;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.util.*;
Expand Down Expand Up @@ -55,6 +61,8 @@
import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.DeserializationContext;
Expand All @@ -64,6 +72,7 @@
import com.fasterxml.jackson.databind.PropertyNamingStrategy;
import com.fasterxml.jackson.databind.module.SimpleModule;
import com.fasterxml.jackson.databind.node.ObjectNode;

import com.google.common.base.Charsets;

/**
Expand All @@ -73,11 +82,13 @@
* @author Craig Andrews
* @author Mathias Düsterhöft
* @author Ken Dombeck
* @author Stefan Reichert
*/
@RunWith(MockitoJUnitRunner.class)
public class DomainObjectReaderUnitTests {

@Mock ResourceMappings mappings;
@Mock
ResourceMappings mappings;

DomainObjectReader reader;
PersistentEntities entities;
Expand All @@ -102,6 +113,12 @@ public void setUp() {
mappingContext.getPersistentEntity(SampleWithReference.class);
mappingContext.getPersistentEntity(Note.class);
mappingContext.getPersistentEntity(WithNullCollection.class);
mappingContext.getPersistentEntity(Basket.class);
mappingContext.getPersistentEntity(Fruit.class);
mappingContext.getPersistentEntity(Apple.class);
mappingContext.getPersistentEntity(Pear.class);
mappingContext.getPersistentEntity(Orange.class);
mappingContext.getPersistentEntity(Banana.class);
mappingContext.afterPropertiesSet();

this.entities = new PersistentEntities(Collections.singleton(mappingContext));
Expand Down Expand Up @@ -516,6 +533,64 @@ public void mergesAssociationsAndKeepsMutableCollection() {
assertThat(result.nested).isSameAs(originalCollection);
}

@Test // DATAREST-1012
public void writesPolymorphicArrayWithSwitchedItemForPut() throws Exception {

Basket basket = new Basket();
Apple apple = new Apple();
Pear pear = new Pear();
basket.fruits = new ArrayList<>();
basket.fruits.add(new Banana());
basket.fruits.add(apple);
basket.fruits.add(pear);

JsonNode node = new ObjectMapper().readTree("{ \"fruits\" : [ "
+ "{ \"@class\" : \"Pear\", \"color\" : \"green\" }," + "{ \"@class\" : \"Orange\", \"color\" : \"orange\" },"
+ "{ \"@class\" : \"Apple\", \"color\" : \"red\" } ] }");

Basket result = reader.readPut((ObjectNode) node, basket, new ObjectMapper());

assertThat(result.fruits.size()).isEqualTo(3);
assertThat(result.fruits.get(0)).isSameAs(pear);
assertThat(result.fruits.get(0).color).isEqualTo("green");
assertThat(result.fruits.get(1)).isInstanceOfAny(Orange.class);
assertThat(result.fruits.get(1).color).isEqualTo("orange");
assertThat(result.fruits.get(2)).isSameAs(apple);
assertThat(result.fruits.get(2).color).isEqualTo("red");
}

@JsonAutoDetect(fieldVisibility = Visibility.ANY)
static class Basket {
@Id @GeneratedValue(strategy = GenerationType.AUTO) Long id;

@OneToMany(cascade = ALL, orphanRemoval = true) List<Fruit> fruits;
}

@JsonAutoDetect(fieldVisibility = Visibility.ANY)
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = PROPERTY, property = "@class")
@JsonSubTypes({ @JsonSubTypes.Type(name = "Apple", value = Apple.class),
@JsonSubTypes.Type(name = "Pear", value = Pear.class),
@JsonSubTypes.Type(name = "Orange", value = Orange.class) })
static class Fruit {
@Id @GeneratedValue(strategy = GenerationType.AUTO) Long id;

@Basic
String color;
}

@JsonAutoDetect(fieldVisibility = Visibility.ANY)
static class Apple extends Fruit {}

@JsonAutoDetect(fieldVisibility = Visibility.ANY)
static class Pear extends Fruit {}

@JsonAutoDetect(fieldVisibility = Visibility.ANY)
static class Orange extends Fruit {}

@JsonAutoDetect(fieldVisibility = Visibility.ANY)
static class Banana extends Fruit {}


@Test // DATAREST-1030
public void patchWithReferenceToRelatedEntityIsResolvedCorrectly() throws Exception {

Expand Down