diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/json/SpiJsonReader.java b/ebean-core/src/main/java/io/ebeaninternal/api/json/SpiJsonReader.java index 1aaaf12fe..613b105cc 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/api/json/SpiJsonReader.java +++ b/ebean-core/src/main/java/io/ebeaninternal/api/json/SpiJsonReader.java @@ -34,5 +34,5 @@ public interface SpiJsonReader { Object readValueUsingObjectMapper(Class propertyType) throws IOException; - boolean intercept(); + boolean update(); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorJsonHelp.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorJsonHelp.java index 0e323114b..720ef92ca 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorJsonHelp.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorJsonHelp.java @@ -133,7 +133,12 @@ final class BeanDescriptorJsonHelp { String key = parser.getCurrentName(); BeanProperty p = desc.beanProperty(key); if (p != null) { - p.jsonRead(readJson, bean); + if (p.isVersion() && readJson.update() ) { + // skip version prop during update + p.jsonRead(readJson); + } else { + p.jsonRead(readJson, bean); + } } else { // read an unmapped property if (unmappedProperties == null) { diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java index 97d1b790e..319ce4cf1 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java @@ -20,9 +20,9 @@ import io.ebeaninternal.api.SpiExpressionRequest; import io.ebeaninternal.api.SpiQuery; import io.ebeaninternal.api.json.SpiJsonReader; import io.ebeaninternal.api.json.SpiJsonWriter; +import io.ebeaninternal.server.bind.DataBind; import io.ebeaninternal.server.core.EncryptAlias; import io.ebeaninternal.server.core.InternString; -import io.ebeaninternal.server.bind.DataBind; import io.ebeaninternal.server.deploy.generatedproperty.GeneratedProperty; import io.ebeaninternal.server.deploy.generatedproperty.GeneratedWhenCreated; import io.ebeaninternal.server.deploy.generatedproperty.GeneratedWhenModified; @@ -1408,15 +1408,20 @@ public class BeanProperty implements ElPropertyValue, Property, STreeProperty { } public void jsonRead(SpiJsonReader ctx, EntityBean bean) throws IOException { + Object objValue = jsonRead(ctx); + if (jsonDeserialize) { + if (ctx.update()) { + setValueIntercept(bean, objValue); + } else { + setValue(bean, objValue); + } + } + } + + public Object jsonRead(SpiJsonReader ctx) throws IOException { JsonToken event = ctx.nextToken(); if (JsonToken.VALUE_NULL == event) { - if (jsonDeserialize) { - if (ctx.intercept()) { - setValueIntercept(bean, null); - } else { - setValue(bean, null); - } - } + return null; } else { // expect to read non-null json value Object objValue; @@ -1433,13 +1438,7 @@ public class BeanProperty implements ElPropertyValue, Property, STreeProperty { CoreLog.log.log(ERROR, msg, e); } } - if (jsonDeserialize) { - if (ctx.intercept()) { - setValueIntercept(bean, objValue); - } else { - setValue(bean, objValue); - } - } + return objValue; } } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocMany.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocMany.java index feec8f8ce..0cd13f64b 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocMany.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocMany.java @@ -2,6 +2,8 @@ package io.ebeaninternal.server.deploy; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.core.JsonToken; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ObjectNode; import io.ebean.SqlUpdate; import io.ebean.Transaction; import io.ebean.bean.BeanCollection; @@ -27,6 +29,7 @@ import java.io.StringWriter; import java.util.*; import static java.lang.System.Logger.Level.ERROR; +import static java.lang.System.Logger.Level.WARNING; /** * Property mapped to a List Set or Map. @@ -937,7 +940,7 @@ public class BeanPropertyAssocMany extends BeanPropertyAssoc implements ST if (elementDescriptor != null) { elementDescriptor.jsonWriteMapEntry(ctx, entry); } else { - targetDescriptor.jsonWrite(ctx, (EntityBean)entry.getValue()); + targetDescriptor.jsonWrite(ctx, (EntityBean) entry.getValue()); } } @@ -1003,7 +1006,7 @@ public class BeanPropertyAssocMany extends BeanPropertyAssoc implements ST if (JsonToken.VALUE_NULL == event) { return null; } - return jsonReadCollection(ctx, null); + return jsonReadCollection(ctx, null, null); } /** @@ -1016,15 +1019,16 @@ public class BeanPropertyAssocMany extends BeanPropertyAssoc implements ST /** * Read the collection (JSON Array) containing entity beans. */ - public Object jsonReadCollection(SpiJsonReader readJson, EntityBean parentBean) throws IOException { + public Object jsonReadCollection(SpiJsonReader readJson, EntityBean parentBean, Object targets) throws IOException { if (elementDescriptor != null && elementDescriptor.isJsonReadCollection()) { return elementDescriptor.jsonReadCollection(readJson, parentBean); } BeanCollection collection = createEmpty(parentBean); BeanCollectionAdd add = beanCollectionAdd(collection); + Map existingBeans = extractBeans(targets); do { - // CHECKME: Update existing list entry here? - EntityBean detailBean = (EntityBean) targetDescriptor.jsonRead(readJson, name, null); + + EntityBean detailBean = getDetailBean(readJson, existingBeans); if (detailBean == null) { // read the entire array break; @@ -1039,6 +1043,63 @@ public class BeanPropertyAssocMany extends BeanPropertyAssoc implements ST return collection; } + /** + * Find bean in the target collection and reuse it for JSON update. + */ + private EntityBean getDetailBean(SpiJsonReader readJson, Map targets) throws IOException { + BeanProperty idProperty = targetDescriptor.idProperty(); + if (targets == null || idProperty == null) { + return (EntityBean) targetDescriptor.jsonRead(readJson, name, null); + } else { + JsonToken token = readJson.parser().nextToken(); + if (JsonToken.VALUE_NULL == token || JsonToken.END_ARRAY == token) { + return null; + } + // extract the id. We have to buffer the JSON; + ObjectNode node = readJson.mapper().readTree(readJson.parser()); + SpiJsonReader jsonReader = readJson.forJson(node.traverse()); + JsonNode idNode = node.get(idProperty.name()); + Object id = idNode == null ? null : idProperty.jsonRead(readJson.forJson(idNode.traverse())); + return (EntityBean) targetDescriptor.jsonRead(jsonReader, name, targets.get(id)); + } + } + + /** + * Extract beans, that are currently in the target collection. (targets can be a List/Set/Map) + */ + private Map extractBeans(Object targets) { + Collection beans; + + if (targets == null) { + return null; + } else if (targets instanceof Map) { + if (((Map) targets).isEmpty()) { + return null; + } + beans = ((Map) targets).values(); + } else if (targets instanceof Collection) { + if (((Collection) targets).isEmpty()) { + return null; + } + beans = (Collection) targets; + } else { + CoreLog.log.log(WARNING, "Found non collection value " + targets.getClass().getSimpleName()); + return null; + } + + BeanProperty idProp = targetDescriptor.idProperty(); + Map ret = new HashMap<>(); + for (T bean : beans) { + if (bean instanceof EntityBean) { + Object id = idProp.getValue((EntityBean) bean); + if (id != null) { + ret.put(id, bean); + } + } + } + return ret.isEmpty() ? null : ret; + } + /** * Bind all the property values to the SqlUpdate. */ diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocManyJsonHelp.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocManyJsonHelp.java index f09bc4dc6..d561f3e68 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocManyJsonHelp.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocManyJsonHelp.java @@ -50,10 +50,10 @@ class BeanPropertyAssocManyJsonHelp { if (JsonToken.START_ARRAY != event && JsonToken.START_OBJECT != event) { throw new JsonParseException(parser, "Unexpected token " + event + " - expecting start array or object"); } - if (readJson.intercept()) { - many.setValueIntercept(parentBean, many.jsonReadCollection(readJson, parentBean)); + if (readJson.update()) { + many.setValueIntercept(parentBean, many.jsonReadCollection(readJson, parentBean, many.value(parentBean))); } else { - many.setValue(parentBean, many.jsonReadCollection(readJson, parentBean)); + many.setValue(parentBean, many.jsonReadCollection(readJson, parentBean, null)); } } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java index bcc75aa4c..01fc670e5 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java @@ -787,9 +787,10 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc implements STr @Override public void jsonRead(SpiJsonReader readJson, EntityBean bean) throws IOException { if (jsonDeserialize && targetDescriptor != null) { - T target = (T) value(bean); + // CHECKME: may we skip reading the object from the json stream? + T target = readJson.update() ? (T) value(bean) : null; T assocBean = targetDescriptor.jsonRead(readJson, name, target); - if (readJson.intercept()) { + if (readJson.update()) { setValueIntercept(bean, assocBean); } else { setValue(bean, assocBean); @@ -797,6 +798,11 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc implements STr } } + @Override + public Object jsonRead(SpiJsonReader readJson) throws IOException { + return targetDescriptor.jsonRead(readJson, name, null); + } + public boolean isReference(Object detailBean) { EntityBean eb = (EntityBean) detailBean; return targetDescriptor.isReference(eb._ebean_getIntercept()); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertySimpleCollection.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertySimpleCollection.java index 2fbf41dcc..dc170974d 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertySimpleCollection.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertySimpleCollection.java @@ -19,7 +19,7 @@ public final class BeanPropertySimpleCollection extends BeanPropertyAssocMany } @Override - public Object jsonReadCollection(SpiJsonReader readJson, EntityBean parentBean) throws IOException { + public Object jsonReadCollection(SpiJsonReader readJson, EntityBean parentBean, Object collectionValue) throws IOException { return elementDescriptor.jsonReadCollection(readJson, parentBean); } } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/json/ReadJson.java b/ebean-core/src/main/java/io/ebeaninternal/server/json/ReadJson.java index 65db79c08..ab9100c9f 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/json/ReadJson.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/json/ReadJson.java @@ -32,13 +32,13 @@ public final class ReadJson implements SpiJsonReader { private final Object objectMapper; private final PersistenceContext persistenceContext; private final LoadContext loadContext; - private final boolean intercept; + private final boolean update; private final boolean enableLazyLoading; /** * Construct with parser and readOptions. */ - public ReadJson(BeanDescriptor desc, JsonParser parser, JsonReadOptions readOptions, Object objectMapper, boolean intercept) { + public ReadJson(BeanDescriptor desc, JsonParser parser, JsonReadOptions readOptions, Object objectMapper, boolean update) { this.rootDesc = desc; this.parser = parser; this.objectMapper = objectMapper; @@ -48,7 +48,7 @@ public final class ReadJson implements SpiJsonReader { // only create visitorMap, pathStack if needed ... this.visitorMap = (readOptions == null) ? null : readOptions.getVisitorMap(); this.pathStack = (visitorMap == null && loadContext == null) ? null : new PathStack(); - this.intercept = intercept; + this.update = update; } /** @@ -62,7 +62,7 @@ public final class ReadJson implements SpiJsonReader { this.objectMapper = source.objectMapper; this.persistenceContext = source.persistenceContext; this.loadContext = source.loadContext; - this.intercept = source.intercept; + this.update = source.update; this.enableLazyLoading = source.enableLazyLoading; } @@ -213,10 +213,10 @@ public final class ReadJson implements SpiJsonReader { } /** - * Do we have to set values via intercept or not. + * Do we update an existing bean? This meeans we have to set values via intercept and handle collections. */ @Override - public boolean intercept() { - return intercept; + public boolean update() { + return update; } } diff --git a/ebean-test/src/test/java/org/tests/text/json/TestJsonBeanDescriptorParse.java b/ebean-test/src/test/java/org/tests/text/json/TestJsonBeanDescriptorParse.java index 9a7acd1a5..7225f69e4 100644 --- a/ebean-test/src/test/java/org/tests/text/json/TestJsonBeanDescriptorParse.java +++ b/ebean-test/src/test/java/org/tests/text/json/TestJsonBeanDescriptorParse.java @@ -12,14 +12,19 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.tests.model.basic.Address; +import org.tests.model.basic.Contact; +import org.tests.model.basic.ContactNote; import org.tests.model.basic.Customer; import java.io.IOException; import java.io.StringReader; +import java.util.Comparator; import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; public class TestJsonBeanDescriptorParse extends BaseTestCase { @@ -32,6 +37,17 @@ public class TestJsonBeanDescriptorParse extends BaseTestCase { address.setLine1("foo"); DB.save(address); customer.setBillingAddress(address); + + Contact alfred = new Contact("Alfred", "P"); + alfred.setId(789); // set some deterministic id + ContactNote note = new ContactNote("Drinks", "Order 100l beer"); + note.setId(890); + alfred.getNotes().add(note); + customer.getContacts().add(alfred); + Contact anton = new Contact("Anton", "P"); + anton.setId(790); + anton.getNotes().add(new ContactNote("Equipment", "Organize barbecue")); + customer.getContacts().add(anton); DB.save(customer); } @@ -59,6 +75,38 @@ public class TestJsonBeanDescriptorParse extends BaseTestCase { assertThat(beanState.dirtyValues()).isEmpty(); } + @Test + public void testJsonManyUpdate() throws IOException { + + Customer customer = DB.find(Customer.class, 234); + String json = + "{\"contacts\": [ " + + " {\"id\": 789, \"lastName\": \"Praml\", \"notes\": [" + + " {\"id\": 890,\"title\": \"Beer\",\"note\": \"Order 200l beer\",\"version\": 17}," + + " {\"title\": \"Food\",\"note\": \"Order 20 steaks\"}" + + " ]}," + + " {\"id\": 790, \"firstName\": \"Anton\", \"lastName\": null, \"notes\" : null } " + + "]}"; + DB.json().toBean(customer, json); + DB.save(customer); + + customer = DB.find(Customer.class, 234); + assertThat(customer.getContacts()).hasSize(2); + customer.getContacts().sort(Comparator.comparing(Contact::getId)); + + Contact contact = customer.getContacts().get(0); + assertThat(contact.getFirstName()).isEqualTo("Alfred"); + assertThat(contact.getLastName()).isEqualTo("Praml"); + assertThat(contact.getNotes()).hasSize(2).extracting(ContactNote::getNote) + .containsExactlyInAnyOrder("Order 200l beer", "Order 20 steaks"); + + contact = customer.getContacts().get(1); + assertThat(contact.getFirstName()).isEqualTo("Anton"); + assertThat(contact.getLastName()).isEqualTo(null); + assertThat(contact.getNotes()).hasSize(1); + + } + @Test public void testJsonUpdate() throws IOException { Customer customer = DB.find(Customer.class, 234);