From 558ed242b3cfe5b45d10f6a1b23e04a69e7a6180 Mon Sep 17 00:00:00 2001 From: Michael Hunger Date: Tue, 15 Mar 2011 13:37:53 +0100 Subject: [PATCH] fixed a concurrent-modification-checking related bug, it is only necessary when the entity was previously attached --- .../fieldaccess/DetachedEntityState.java | 51 +++++++++++++------ 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/DetachedEntityState.java b/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/DetachedEntityState.java index a4646ec41..59d328795 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/DetachedEntityState.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/DetachedEntityState.java @@ -35,7 +35,7 @@ import static org.springframework.data.graph.neo4j.fieldaccess.DoReturn.unwrap; * @since 15.09.2010 */ public class DetachedEntityState, STATE> implements EntityState { - private final Map dirty = new HashMap(); + private final Map dirty = new HashMap(); protected final EntityState delegate; private final static Log log = LogFactory.getLog(DetachedEntityState.class); private GraphDatabaseContext graphDatabaseContext; @@ -87,18 +87,37 @@ public class DetachedEntityState, STATE> imple return getGraphDatabaseContext().transactionIsRunning(); } + static class ExistingValue { + public final Object value; + private final boolean fromGraph; + + ExistingValue(Object value, boolean fromGraph) { + this.value = value; + this.fromGraph = fromGraph; + } + + @Override + public String toString() { + return String.format("ExistingValue{value=%s, fromGraph=%s}", value, fromGraph); + } + + private boolean mustCheckConcurrentModification() { + return fromGraph; + } + } @Override public Object setValue(final Field field, final Object newVal) { if (isDetached()) { - final ENTITY entity = getEntity(); if (!isDirty(field) && isWritable(field)) { Object existingValue; - if (entity.getPersistentState()!=null) existingValue = unwrap(delegate.getValue(field)); - else { - existingValue = getValueFromEntity(field); - if (existingValue == null) existingValue = getDefaultValue(field.getType()); + if (hasPersistentState()) { + addDirty(field, unwrap(delegate.getValue(field)), true); + } + else { + // existingValue = getValueFromEntity(field); + // if (existingValue == null) existingValue = getDefaultValue(field.getType()); + addDirty(field, newVal, false); } - addDirty(field, existingValue); } return newVal; } @@ -133,7 +152,7 @@ public class DetachedEntityState, STATE> imple throw new IllegalStateException("Flushing detached entity without a persistent state, this had to be created first."); } if (isDirty()) { - for (final Map.Entry entry : dirty.entrySet()) { + for (final Map.Entry entry : dirty.entrySet()) { final Field field = entry.getKey(); if (log.isDebugEnabled()) log.debug("Flushing dirty Entity new node " + entity.getPersistentState() + " field " + field+ " with value "+getValueFromEntity(field)); checkConcurrentModification(entity, entry, field); @@ -159,11 +178,13 @@ public class DetachedEntityState, STATE> imple } } - private void checkConcurrentModification(final ENTITY entity, final Map.Entry entry, final Field field) { - final Object nodeValue = unwrap(delegate.getValue(field)); - final Object previousValue = entry.getValue(); - if (!ObjectUtils.nullSafeEquals(nodeValue, previousValue)) { - throw new ConcurrentModificationException("Node " + entity.getPersistentState() + " field " + field + " changed in between previous " + previousValue + " current " + nodeValue); // todo or just overwrite + private void checkConcurrentModification(final ENTITY entity, final Map.Entry entry, final Field field) { + final ExistingValue previousValue = entry.getValue(); + if (previousValue.mustCheckConcurrentModification()) { + final Object nodeValue = unwrap(delegate.getValue(field)); + if (!ObjectUtils.nullSafeEquals(nodeValue, previousValue.value)) { + throw new ConcurrentModificationException("Node " + entity.getPersistentState() + " field " + field + " changed in between previous " + previousValue + " current " + nodeValue); // todo or just overwrite + } } } @@ -179,8 +200,8 @@ public class DetachedEntityState, STATE> imple this.dirty.clear(); } - private void addDirty(final Field f, final Object previousValue) { - this.dirty.put(f, previousValue); + private void addDirty(final Field f, final Object previousValue, boolean fromGraph) { + this.dirty.put(f, new ExistingValue(previousValue,fromGraph)); }