From c7e6630492d34f615990fb747ed76868b9046aba Mon Sep 17 00:00:00 2001 From: Frederik Zimmer Date: Fri, 26 Feb 2016 17:36:13 +0100 Subject: [PATCH] OLINGO-848: getting a property of an entity flags the entity as changed Signed-off-by: Christian Amend --- .../AbstractCollectionInvocationHandler.java | 9 - .../AbstractStructuredInvocationHandler.java | 98 ++++++--- .../commons/ComplexInvocationHandler.java | 7 +- .../commons/EntityInvocationHandler.java | 14 +- ...onTransactionalPersistenceManagerImpl.java | 28 ++- .../TransactionalPersistenceManagerImpl.java | 22 +- .../fit/proxy/ChangeDetectionTestITCase.java | 188 ++++++++++++++++++ 7 files changed, 305 insertions(+), 61 deletions(-) create mode 100644 fit/src/test/java/org/apache/olingo/fit/proxy/ChangeDetectionTestITCase.java diff --git a/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/AbstractCollectionInvocationHandler.java b/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/AbstractCollectionInvocationHandler.java index dd10b7543..e46572205 100644 --- a/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/AbstractCollectionInvocationHandler.java +++ b/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/AbstractCollectionInvocationHandler.java @@ -64,8 +64,6 @@ public abstract class AbstractCollectionInvocationHandler, Object> annotationsByTerm = new HashMap, Object>(); - private boolean changed = false; - public AbstractCollectionInvocationHandler( final AbstractService service, final Collection items, @@ -174,7 +172,6 @@ public abstract class AbstractCollectionInvocationHandler collection) { - changed = true; return items.addAll(collection); } @@ -324,8 +319,4 @@ public abstract class AbstractCollectionInvocationHandler linkCache = new HashMap(); - protected int propertiesTag = 0; - - protected int linksTag = 0; - protected final Map streamedPropertyChanges = new HashMap(); protected final Map streamedPropertyCache = new HashMap(); @@ -386,7 +382,6 @@ public abstract class AbstractStructuredInvocationHandler extends AbstractInvoca } if (res != null) { - addPropertyChanges(name, res); propertyCache.put(name, res); } @@ -526,7 +521,79 @@ public abstract class AbstractStructuredInvocationHandler extends AbstractInvoca } public Map getPropertyChanges() { - return propertyChanges; + Map changedProperties = new HashMap(); + changedProperties.putAll(propertyChanges); + + for (Map.Entry propertyCacheEntry : propertyCache.entrySet()) { + if (hasCachedPropertyChanged(propertyCacheEntry.getValue())) { + changedProperties.put(propertyCacheEntry.getKey(), propertyCacheEntry.getValue()); + } + } + + return changedProperties; + } + + protected boolean hasCachedPropertyChanged(final Object cachedValue) { + AbstractStructuredInvocationHandler structuredInvocationHandler = getStructuredInvocationHandler(cachedValue); + if (structuredInvocationHandler != null) { + return structuredInvocationHandler.isChanged(); + } + + return false; + } + + public boolean isChanged() { + return !linkChanges.isEmpty() + || hasPropertyChanges(); + } + + protected boolean hasPropertyChanges() { + return !propertyChanges.isEmpty() || hasDeepPropertyChanges(); + } + + protected boolean hasDeepPropertyChanges() { + for (Object propertyValue : propertyCache.values()) { + if (hasCachedPropertyChanged(propertyValue)) { + return true; + } + } + + return false; + } + + public void applyChanges() { + streamedPropertyCache.putAll(streamedPropertyChanges); + streamedPropertyChanges.clear(); + propertyCache.putAll(propertyChanges); + propertyChanges.clear(); + linkCache.putAll(linkChanges); + linkChanges.clear(); + + applyChangesOnChildren(); + } + + protected void applyChangesOnChildren() { + for (Object propertyValue : propertyCache.values()) { + applyChanges(propertyValue); + } + } + + protected void applyChanges(final Object cachedValue) { + AbstractStructuredInvocationHandler structuredInvocationHandler = getStructuredInvocationHandler(cachedValue); + if (structuredInvocationHandler != null) { + structuredInvocationHandler.applyChanges(); + } + } + + protected AbstractStructuredInvocationHandler getStructuredInvocationHandler(final Object value) { + if (value != null && Proxy.isProxyClass(value.getClass())) { + InvocationHandler invocationHandler = Proxy.getInvocationHandler(value); + if (invocationHandler instanceof AbstractStructuredInvocationHandler) { + return (AbstractStructuredInvocationHandler) invocationHandler; + } + } + + return null; } public Collection readAdditionalPropertyNames() { @@ -567,14 +634,11 @@ public abstract class AbstractStructuredInvocationHandler extends AbstractInvoca } protected void addPropertyChanges(final String name, final Object value) { - final int checkpoint = propertyChanges.hashCode(); - updatePropertiesTag(checkpoint); + propertyCache.remove(name); propertyChanges.put(name, value); } protected void addLinkChanges(final NavigationProperty navProp, final Object value) { - final int checkpoint = linkChanges.hashCode(); - updateLinksTag(checkpoint); linkChanges.put(navProp, value); if (linkCache.containsKey(navProp)) { @@ -582,18 +646,6 @@ public abstract class AbstractStructuredInvocationHandler extends AbstractInvoca } } - protected void updatePropertiesTag(final int checkpoint) { - if (propertiesTag == 0 || checkpoint == propertiesTag) { - propertiesTag = propertyChanges.hashCode(); - } - } - - protected void updateLinksTag(final int checkpoint) { - if (linksTag == 0 || checkpoint == linksTag) { - linksTag = linkChanges.hashCode(); - } - } - public Map getStreamedPropertyChanges() { return streamedPropertyChanges; } @@ -642,8 +694,6 @@ public abstract class AbstractStructuredInvocationHandler extends AbstractInvoca protected abstract void load(); - public abstract boolean isChanged(); - protected abstract List getInternalProperties(); protected abstract ClientProperty getInternalProperty(final String name); diff --git a/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/ComplexInvocationHandler.java b/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/ComplexInvocationHandler.java index 1e2197c56..aac32ace6 100644 --- a/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/ComplexInvocationHandler.java +++ b/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/ComplexInvocationHandler.java @@ -27,10 +27,10 @@ import org.apache.commons.lang3.tuple.ImmutablePair; import org.apache.commons.lang3.tuple.Pair; import org.apache.olingo.client.api.communication.request.retrieve.ODataPropertyRequest; import org.apache.olingo.client.api.communication.response.ODataRetrieveResponse; -import org.apache.olingo.client.api.uri.URIBuilder; import org.apache.olingo.client.api.domain.ClientComplexValue; import org.apache.olingo.client.api.domain.ClientLinked; import org.apache.olingo.client.api.domain.ClientProperty; +import org.apache.olingo.client.api.uri.URIBuilder; import org.apache.olingo.commons.api.edm.FullQualifiedName; import org.apache.olingo.ext.proxy.AbstractService; import org.apache.olingo.ext.proxy.api.annotations.ComplexType; @@ -144,11 +144,6 @@ public class ComplexInvocationHandler extends AbstractStructuredInvocationHandle return retrieveNavigationProperty(property, getter); } - @Override - public boolean isChanged() { - return getEntityHandler() == null ? false : getEntityHandler().isChanged(); - } - @Override protected void load() { try { diff --git a/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/EntityInvocationHandler.java b/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/EntityInvocationHandler.java index e99314037..2fea9bd28 100644 --- a/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/EntityInvocationHandler.java +++ b/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/EntityInvocationHandler.java @@ -229,13 +229,12 @@ public class EntityInvocationHandler extends AbstractStructuredInvocationHandler this.streamedPropertyChanges.clear(); this.streamedPropertyCache.clear(); this.propertyChanges.clear(); + this.propertyCache.clear(); this.linkChanges.clear(); this.linkCache.clear(); - this.propertiesTag = 0; - this.linksTag = 0; this.annotations.clear(); } - + public EntityUUID getUUID() { return uuid; } @@ -301,11 +300,10 @@ public class EntityInvocationHandler extends AbstractStructuredInvocationHandler return isChanged(true); } - public boolean isChanged(final boolean deep) { - return this.linkChanges.hashCode() != this.linksTag - || this.propertyChanges.hashCode() != this.propertiesTag - || (deep && (this.stream != null - || !this.streamedPropertyChanges.isEmpty())); + public boolean isChanged(final boolean considerStreamProperties) { + return super.isChanged() + || (considerStreamProperties && (stream != null + || !streamedPropertyChanges.isEmpty())); } public void uploadStream(final EdmStreamValue stream) { diff --git a/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/NonTransactionalPersistenceManagerImpl.java b/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/NonTransactionalPersistenceManagerImpl.java index 91f8cbee1..b123d1e4c 100644 --- a/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/NonTransactionalPersistenceManagerImpl.java +++ b/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/NonTransactionalPersistenceManagerImpl.java @@ -74,15 +74,27 @@ public class NonTransactionalPersistenceManagerImpl extends AbstractPersistenceM } if (entry.getValue() != null - && response instanceof ODataEntityCreateResponse && response.getStatusCode() == 201) { - entry.getValue().setEntity(((ODataEntityCreateResponse) response).getBody()); - responses.put(index, entry.getValue().getEntityURI()); - LOG.debug("Upgrade created object '{}'", entry.getValue()); + && response instanceof ODataEntityCreateResponse && (response.getStatusCode() == 201 || response + .getStatusCode() == 204)) { + if (response.getStatusCode() == 201) { + entry.getValue().setEntity(((ODataEntityCreateResponse) response).getBody()); + responses.put(index, entry.getValue().getEntityURI()); + LOG.debug("Upgrade created object '{}'", entry.getValue()); + } else { + entry.getValue().applyChanges(); + responses.put(index, null); + } } else if (entry.getValue() != null - && response instanceof ODataEntityUpdateResponse && response.getStatusCode() == 200) { - entry.getValue().setEntity(((ODataEntityUpdateResponse) response).getBody()); - responses.put(index, entry.getValue().getEntityURI()); - LOG.debug("Upgrade updated object '{}'", entry.getValue()); + && response instanceof ODataEntityUpdateResponse && (response.getStatusCode() == 200 || response + .getStatusCode() == 204)) { + if (response.getStatusCode() == 200) { + entry.getValue().setEntity(((ODataEntityUpdateResponse) response).getBody()); + responses.put(index, entry.getValue().getEntityURI()); + LOG.debug("Upgrade updated object '{}'", entry.getValue()); + } else { + entry.getValue().applyChanges(); + responses.put(index, null); + } } else { responses.put(index, null); } diff --git a/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/TransactionalPersistenceManagerImpl.java b/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/TransactionalPersistenceManagerImpl.java index e43a4e159..e24796efa 100644 --- a/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/TransactionalPersistenceManagerImpl.java +++ b/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/TransactionalPersistenceManagerImpl.java @@ -116,12 +116,22 @@ public class TransactionalPersistenceManagerImpl extends AbstractPersistenceMana final EntityInvocationHandler handler = items.get(changesetItemId); if (handler != null) { - if (res instanceof ODataEntityCreateResponse && res.getStatusCode() == 201) { - handler.setEntity(((ODataEntityCreateResponse) res).getBody()); - LOG.debug("Upgrade created object '{}'", handler); - } else if (res instanceof ODataEntityUpdateResponse && res.getStatusCode() == 200) { - handler.setEntity(((ODataEntityUpdateResponse) res).getBody()); - LOG.debug("Upgrade updated object '{}'", handler); + if (res instanceof ODataEntityCreateResponse && (res.getStatusCode() == 201 || res + .getStatusCode() == 204)) { + if (res.getStatusCode() == 201) { + handler.setEntity(((ODataEntityCreateResponse) res).getBody()); + LOG.debug("Upgrade created object '{}'", handler); + } else { + handler.applyChanges(); + } + } else if (res instanceof ODataEntityUpdateResponse && (res.getStatusCode() == 200 || res + .getStatusCode() == 204)) { + if (res.getStatusCode() == 201) { + handler.setEntity(((ODataEntityUpdateResponse) res).getBody()); + LOG.debug("Upgrade updated object '{}'", handler); + } else { + handler.applyChanges(); + } } } } diff --git a/fit/src/test/java/org/apache/olingo/fit/proxy/ChangeDetectionTestITCase.java b/fit/src/test/java/org/apache/olingo/fit/proxy/ChangeDetectionTestITCase.java new file mode 100644 index 000000000..33897e724 --- /dev/null +++ b/fit/src/test/java/org/apache/olingo/fit/proxy/ChangeDetectionTestITCase.java @@ -0,0 +1,188 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.olingo.fit.proxy; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.lang.reflect.Proxy; +import java.sql.Timestamp; +import java.util.Calendar; + +import org.apache.commons.lang3.RandomUtils; +import org.apache.olingo.ext.proxy.api.ComplexType; +import org.apache.olingo.ext.proxy.api.EntityCollection; +import org.apache.olingo.ext.proxy.api.EntityType; +import org.apache.olingo.ext.proxy.commons.ComplexInvocationHandler; +import org.apache.olingo.ext.proxy.commons.EntityCollectionInvocationHandler; +import org.apache.olingo.ext.proxy.commons.EntityInvocationHandler; +// CHECKSTYLE:OFF (Maven checkstyle) +import org.apache.olingo.fit.proxy.staticservice.microsoft.test.odata.services.odatawcfservice.InMemoryEntities; +import org.apache.olingo.fit.proxy.staticservice.microsoft.test.odata.services.odatawcfservice.types.Account; +import org.apache.olingo.fit.proxy.staticservice.microsoft.test.odata.services.odatawcfservice.types.Address; +import org.apache.olingo.fit.proxy.staticservice.microsoft.test.odata.services.odatawcfservice.types.Customer; +import org.apache.olingo.fit.proxy.staticservice.microsoft.test.odata.services.odatawcfservice.types.Order; +import org.apache.olingo.fit.proxy.staticservice.microsoft.test.odata.services.odatawcfservice.types.OrderCollection; +import org.apache.olingo.fit.proxy.staticservice.microsoft.test.odata.services.odatawcfservice.types.PaymentInstrument; +import org.apache.olingo.fit.proxy.staticservice.microsoft.test.odata.services.odatawcfservice.types.PaymentInstrumentCollection; +// CHECKSTYLE:ON (Maven checkstyle) +import org.junit.Test; + +public class ChangeDetectionTestITCase extends AbstractTestITCase { + + @Test + public void entityUnchangedOnGetProperty() { + final Customer customer = getContainer().getCustomers().getByKey(1).load(); + assertFalse(isChanged(customer)); + + customer.getLastName(); + + assertFalse(isChanged(customer)); + } + + @Test + public void entityChangedOnSetProperty() { + final Customer customer = getContainer().getCustomers().getByKey(1).load(); + assertFalse(isChanged(customer)); + + customer.setLastName("Test"); + + assertTrue(isChanged(customer)); + + getContainer().flush(); + assertFalse(isChanged(customer)); + } + + @Test + public void entityUnchangedOnGetComplexProperty() { + final Customer customer = getContainer().getCustomers().getByKey(1).load(); + assertFalse(isChanged(customer)); + + final Address homeAddress = customer.getHomeAddress(); + assertFalse(isChanged(customer)); + + homeAddress.getCity(); + assertFalse(isChanged(homeAddress)); + assertFalse(isChanged(customer)); + } + + @Test + public void entityChangedOnSetComplexProperty() { + final Customer customer = getContainer().getCustomers().getByKey(2).load(); + assertFalse(isChanged(customer)); + + final Address newAdress = getContainer().newComplexInstance(Address.class); + customer.setHomeAddress(newAdress); + + assertTrue(isChanged(customer)); + + getContainer().flush(); + assertFalse(isChanged(customer)); + } + + @Test + public void entityChangedOnSetPropertyOfComplexProperty() { + final Customer customer = getContainer().getCustomers().getByKey(1).load(); + assertFalse(isChanged(customer)); + + final Address homeAddress = customer.getHomeAddress(); + homeAddress.setCity("Test"); + + assertTrue(isChanged(customer)); + + getContainer().flush(); + assertFalse(isChanged(customer)); + } + + @Test + public void entityUnchangedOnGetNavigationProperty() { + final Customer customer = getContainer().getCustomers().getByKey(1).load(); + assertFalse(isChanged(customer)); + + customer.getOrders(); + + assertFalse(isChanged(customer)); + } + + @Test + public void entityChangedOnAddNavigationProperty() { + final Account account = getContainer().getAccounts().getByKey(101).load(); + assertFalse(isChanged(account)); + + final PaymentInstrumentCollection instruments = account.getMyPaymentInstruments().execute(); + assertFalse(isChanged(account)); + + final PaymentInstrument instrument = getContainer().newEntityInstance(PaymentInstrument.class); + final int id = RandomUtils.nextInt(101999, 105000); + instrument.setPaymentInstrumentID(id); + instrument.setFriendlyName("New one"); + instrument.setCreatedDate(new Timestamp(Calendar.getInstance().getTimeInMillis())); + instruments.add(instrument); + + assertTrue(isChanged(instrument)); + assertFalse(isChanged(account)); + + getContainer().flush(); + assertFalse(isChanged(instrument)); + } + + @Test + public void entityCollectionUnchangedOnGet() { + final Customer customer = getContainer().getCustomers().getByKey(1).load(); + assertFalse(isChanged(customer)); + + final OrderCollection orders = customer.getOrders().execute(); + assertFalse(isChanged(customer)); + + for (Order order : orders) { + assertFalse(isChanged(order)); + order.getOrderDate(); + assertFalse(isChanged(order)); + } + + assertFalse(isChanged(customer)); + } + + protected InMemoryEntities getContainer() { + return container; + } + + protected boolean isChanged(final EntityType entity) { + EntityInvocationHandler invocationHandler = getInvocationHandler(entity); + return invocationHandler.isChanged(); + } + + protected boolean isChanged(final ComplexType complex) { + ComplexInvocationHandler invocationHandler = getInvocationHandler(complex); + return invocationHandler.isChanged(); + } + + protected EntityInvocationHandler getInvocationHandler(final EntityType entity) { + return (EntityInvocationHandler) Proxy.getInvocationHandler(entity); + } + + protected ComplexInvocationHandler getInvocationHandler(final ComplexType complex) { + return (ComplexInvocationHandler) Proxy.getInvocationHandler(complex); + } + + protected EntityCollectionInvocationHandler getInvocationHandler( + final EntityCollection complex) { + return (EntityCollectionInvocationHandler) Proxy.getInvocationHandler(complex); + } +}