From d5db319fc2c400a8c76249e1ed7ffd9deb9f7609 Mon Sep 17 00:00:00 2001 From: Reino Ruusu Date: Mon, 24 Feb 2020 13:56:41 +0200 Subject: [PATCH] Thread safety changes in objmap2 gitlab #483 Change-Id: I3a831732961ff7fc3ac6db3a2fdb94c2e9361d7a --- .../objmap/forward/IForwardMappingRule.java | 7 ++- .../factories/UpdateMethodFactory.java | 6 +++ .../simantics/objmap/graph/impl/Mapping.java | 17 +++---- .../objmap/graph/impl/RangeUpdateRequest.java | 44 +++++++++++-------- .../objmap/graph/rules/MappedElementRule.java | 11 +++++ .../graph/rules/MappedElementsRule.java | 20 +++++++-- .../objmap/graph/rules/ValueRule.java | 12 ++++- .../objmap/graph/schema/AdaptedLinkType.java | 9 ++++ .../objmap/graph/schema/SimpleLinkType.java | 9 ++++ .../factories/UpdateMethodFactory.java | 6 +++ .../structural/schema/AdaptedLinkType.java | 9 ++++ .../structural/schema/SimpleLinkType.java | 9 ++++ 12 files changed, 127 insertions(+), 32 deletions(-) diff --git a/bundles/org.simantics.objmap2/src/org/simantics/objmap/forward/IForwardMappingRule.java b/bundles/org.simantics.objmap2/src/org/simantics/objmap/forward/IForwardMappingRule.java index d6c707e02..27a1aeb1a 100644 --- a/bundles/org.simantics.objmap2/src/org/simantics/objmap/forward/IForwardMappingRule.java +++ b/bundles/org.simantics.objmap2/src/org/simantics/objmap/forward/IForwardMappingRule.java @@ -15,6 +15,11 @@ import org.simantics.db.ReadGraph; import org.simantics.objmap.exceptions.MappingException; public interface IForwardMappingRule { + /** + * Check whether a range element needs to be updated due to a change in the range element, without modifying the range element. + */ + boolean checkChanges(ReadGraph graph, IForwardMapping mapping, Domain domainElement, Range rangeElement) throws MappingException; + /** * Modifies the range element so that it corresponds to the domain element. * @param g read transaction @@ -24,6 +29,6 @@ public interface IForwardMappingRule { * @return true if the rule made some modifications * @throws MappingException */ - boolean updateRange(ReadGraph graph, IForwardMapping mapping, Domain domainElement, Range rangeElement) throws MappingException; + boolean updateRange(ReadGraph graph, IForwardMapping mapping, Domain domainElement, Range rangeElement) throws MappingException; void createRange(ReadGraph graph, IForwardMapping mapping, Domain domainElement, Range rangeElement) throws MappingException; } diff --git a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/annotations/factories/UpdateMethodFactory.java b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/annotations/factories/UpdateMethodFactory.java index e1de9f20c..79c39ecfa 100644 --- a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/annotations/factories/UpdateMethodFactory.java +++ b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/annotations/factories/UpdateMethodFactory.java @@ -58,6 +58,12 @@ public class UpdateMethodFactory implements IMethodRuleFactory map, Domain domainElement, + Range rangeElement) throws MappingException { + return false; + } + public void createDomain(WriteGraph g, IBackwardMapping map, Domain domainElement, Range rangeElement) throws MappingException { updateDomain(g, map, domainElement, rangeElement); }; diff --git a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/impl/Mapping.java b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/impl/Mapping.java index 1589e9432..4820dae01 100644 --- a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/impl/Mapping.java +++ b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/impl/Mapping.java @@ -12,8 +12,6 @@ package org.simantics.objmap.graph.impl; -import gnu.trove.map.hash.THashMap; - import java.util.AbstractSet; import java.util.ArrayList; import java.util.Collection; @@ -21,12 +19,9 @@ import java.util.Iterator; import java.util.List; import java.util.Set; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.simantics.db.ReadGraph; import org.simantics.db.WriteGraph; import org.simantics.db.exception.DatabaseException; - import org.simantics.objmap.backward.IBackwardMapping; import org.simantics.objmap.exceptions.MappingException; import org.simantics.objmap.forward.IForwardMapping; @@ -34,6 +29,10 @@ import org.simantics.objmap.graph.IMapping; import org.simantics.objmap.graph.IMappingListener; import org.simantics.objmap.graph.schema.ILinkType; import org.simantics.objmap.graph.schema.IMappingSchema; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import gnu.trove.map.hash.THashMap; /** * An implementation of IMapping. The class should not be created @@ -306,13 +305,15 @@ public class Mapping implements IMapping { if(listensDomain) { RangeUpdateRequest request = new RangeUpdateRequest(link, map, this); + boolean changes; try { - g.syncRequest(request, request); + changes = g.syncRequest(request, request) > 0; } catch (DatabaseException e) { throw new MappingException(e); } - // TODO check if really modified - updated.add(link.rangeElement); + + if (changes) + updated.add(link.rangeElement); } else if(link.type.updateRange(g, map, link.domainElement, link.rangeElement)) diff --git a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/impl/RangeUpdateRequest.java b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/impl/RangeUpdateRequest.java index b84b9c94f..d62435cd1 100644 --- a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/impl/RangeUpdateRequest.java +++ b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/impl/RangeUpdateRequest.java @@ -21,7 +21,7 @@ import org.simantics.objmap.forward.IForwardMapping; import org.simantics.objmap.graph.impl.Link; -public class RangeUpdateRequest implements Read, SyncListener { +public class RangeUpdateRequest implements Read, SyncListener { Link link; /* @@ -30,27 +30,24 @@ public class RangeUpdateRequest implements Read, SyncList */ IForwardMapping map; // map==null is used to flag that request is performed once Mapping mapping; // mapping==null is used as a flag the request disposed + int counter; public RangeUpdateRequest(Link link, IForwardMapping map, Mapping mapping) { this.link = link; this.map = map; this.mapping = mapping; + this.counter = 0; } @Override - public Boolean perform(ReadGraph g) throws DatabaseException { - if(map != null) { - link.type.updateRange(g, map, link.domainElement, link.rangeElement); - map = null; - return Boolean.TRUE; - } - else if(mapping != null) { - mapping.domainModified(link); - mapping = null; - return Boolean.FALSE; - } - else - return null; + public Integer perform(ReadGraph g) throws DatabaseException { + boolean changed = false; + if (map != null) + changed = link.type.checkChanges(g, map, link.domainElement, link.rangeElement); + else if (mapping != null) + changed = link.type.checkChanges(g, mapping, link.domainElement, link.rangeElement); + + return changed ? counter + 1 : counter; } @Override @@ -63,15 +60,24 @@ public class RangeUpdateRequest implements Read, SyncList } @Override - public void execute(ReadGraph graph, Boolean result) - throws DatabaseException { + public void execute(ReadGraph graph, Integer result) + throws DatabaseException { + boolean changed = result != counter; + counter = result; + + if (map != null) { + if (changed) + link.type.updateRange(graph, map, link.domainElement, link.rangeElement); + map = null; + } + else if (mapping != null && changed) { + mapping.domainModified(link); + mapping = null; + } } @Override public boolean isDisposed() { return mapping == null || link.removed || mapping.isDisposed(); } - - - } diff --git a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/rules/MappedElementRule.java b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/rules/MappedElementRule.java index 9761f5555..a0b7befed 100644 --- a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/rules/MappedElementRule.java +++ b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/rules/MappedElementRule.java @@ -11,6 +11,8 @@ *******************************************************************************/ package org.simantics.objmap.graph.rules; +import java.util.Objects; + import org.simantics.db.ReadGraph; import org.simantics.db.WriteGraph; import org.simantics.objmap.backward.IBackwardMapping; @@ -61,10 +63,19 @@ public class MappedElementRule implements IBidirectionalMappingRu return rangeAccessor.set(rangeElement, mappedValue); } + @Override + public boolean checkChanges(ReadGraph g, IForwardMapping map, Domain domainElement, Range rangeElement) throws MappingException { + Domain value = domainAccessor.get(g, domainElement); + Range mappedValue = value == null ? null : map.map(g, value); + return mappedValue == rangeAccessor.get(rangeElement); + } + + @Override public void createDomain(WriteGraph g, IBackwardMapping map, Domain domainElement, Range rangeElement) throws MappingException { updateDomain(g, map, domainElement, rangeElement); }; + @Override public void createRange(ReadGraph g, IForwardMapping map, Domain domainElement, Range rangeElement) throws MappingException { updateRange(g, map, domainElement, rangeElement); }; diff --git a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/rules/MappedElementsRule.java b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/rules/MappedElementsRule.java index 7f3c13a10..45fe70641 100644 --- a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/rules/MappedElementsRule.java +++ b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/rules/MappedElementsRule.java @@ -13,6 +13,7 @@ package org.simantics.objmap.graph.rules; import java.util.ArrayList; import java.util.Collection; +import java.util.Objects; import org.simantics.db.ReadGraph; import org.simantics.db.WriteGraph; @@ -66,12 +67,25 @@ public class MappedElementsRule implements IBidirectionalMappingR Domain domainElement, Range rangeElement) throws MappingException { LOGGER.trace(" MappedElementsRule.updateRange"); - Collection value = domainAccessor.get(g, domainElement); + ArrayList mappedValue = getMappedValue(g, map, domainElement); + return rangeAccessor.set(rangeElement, mappedValue); + } + + @Override + public boolean checkChanges(ReadGraph g, IForwardMapping map, Domain domainElement, Range rangeElement) throws MappingException { + LOGGER.trace(" MappedElementsRule.updateRange"); + ArrayList mappedValue = getMappedValue(g, map, domainElement); + return Objects.equals(mappedValue, rangeAccessor.get(rangeElement)); + } + + private ArrayList getMappedValue(ReadGraph g, IForwardMapping map, Domain domainElement) + throws MappingException { + Collection value = domainAccessor.get(g, domainElement); ArrayList mappedValue = new ArrayList(value.size()); for(Domain r : value) mappedValue.add(map.map(g, r));//map.get(r)); - return rangeAccessor.set(rangeElement, mappedValue); - } + return mappedValue; + } public void createDomain(WriteGraph g, IBackwardMapping map, Domain domainElement, Range rangeElement) throws MappingException { updateDomain(g, map, domainElement, rangeElement); diff --git a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/rules/ValueRule.java b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/rules/ValueRule.java index 9ba9dba11..cbea3253a 100644 --- a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/rules/ValueRule.java +++ b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/rules/ValueRule.java @@ -13,6 +13,9 @@ package org.simantics.objmap.graph.rules; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + +import java.util.Objects; + import org.simantics.db.ReadGraph; import org.simantics.db.WriteGraph; import org.simantics.objmap.backward.IBackwardMapping; @@ -57,7 +60,14 @@ public class ValueRule implements IBidirectionalMappingRule map, Domain domainElement, + Range rangeElement) throws MappingException { + Object value = rangeAccessor.get(rangeElement); + return Objects.equals(value, domainAccessor.get(g, domainElement)); + } public void createDomain(WriteGraph g, IBackwardMapping map, Domain domainElement, Range rangeElement) throws MappingException { updateDomain(g, map, domainElement, rangeElement); diff --git a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/schema/AdaptedLinkType.java b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/schema/AdaptedLinkType.java index 9b02d5d50..b0d8d965f 100644 --- a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/schema/AdaptedLinkType.java +++ b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/schema/AdaptedLinkType.java @@ -69,18 +69,27 @@ public class AdaptedLinkType implements ILinkType { } } + @Override public void createDomain(WriteGraph graph, IBackwardMapping mapping, Resource domainElement, Range rangeElement) throws MappingException { }; + @Override public void createRange(ReadGraph graph, org.simantics.objmap.forward.IForwardMapping mapping, Resource domainElement, Range rangeElement) throws MappingException { }; + @Override + public boolean checkChanges(ReadGraph g, IForwardMapping map, Resource domainElement, Range rangeElement) throws MappingException { + return false; + } + + @Override public boolean updateDomain(WriteGraph g, IBackwardMapping map, Resource domainElement, Range rangeElement) throws MappingException { return false; } + @Override public boolean updateRange(ReadGraph g, IForwardMapping map, Resource domainElement, Range rangeElement) throws MappingException { return false; } diff --git a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/schema/SimpleLinkType.java b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/schema/SimpleLinkType.java index 9ddc43716..12dd4c9c0 100644 --- a/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/schema/SimpleLinkType.java +++ b/bundles/org.simantics.objmap2/src/org/simantics/objmap/graph/schema/SimpleLinkType.java @@ -104,6 +104,15 @@ public class SimpleLinkType implements ILinkType { updateRange(graph, mapping, domainElement, rangeElement); }; + @Override + public boolean checkChanges(ReadGraph g, IForwardMapping map, Resource domainElement, + Range rangeElement) throws MappingException { + boolean updated = false; + for(IBidirectionalMappingRule rule : rules) + updated |= rule.checkChanges(g, map, domainElement, rangeElement); + return updated; + } + public boolean updateDomain(WriteGraph g, IBackwardMapping map, Resource domainElement, Range rangeElement) throws MappingException { if(LOGGER.isTraceEnabled()) try { diff --git a/bundles/org.simantics.objmap2/src/org/simantics/objmap/structural/annotations/factories/UpdateMethodFactory.java b/bundles/org.simantics.objmap2/src/org/simantics/objmap/structural/annotations/factories/UpdateMethodFactory.java index a2be5fa42..c0fee3da6 100644 --- a/bundles/org.simantics.objmap2/src/org/simantics/objmap/structural/annotations/factories/UpdateMethodFactory.java +++ b/bundles/org.simantics.objmap2/src/org/simantics/objmap/structural/annotations/factories/UpdateMethodFactory.java @@ -58,6 +58,12 @@ public class UpdateMethodFactory implements IMethodRuleFactory map, Domain domainElement, + Range rangeElement) throws MappingException { + return false; + } + public void createDomain(WriteGraph g, IBackwardMapping map, Domain domainElement, Range rangeElement) throws MappingException { updateDomain(g, map, domainElement, rangeElement); }; diff --git a/bundles/org.simantics.objmap2/src/org/simantics/objmap/structural/schema/AdaptedLinkType.java b/bundles/org.simantics.objmap2/src/org/simantics/objmap/structural/schema/AdaptedLinkType.java index 203863a6c..21d1d8990 100644 --- a/bundles/org.simantics.objmap2/src/org/simantics/objmap/structural/schema/AdaptedLinkType.java +++ b/bundles/org.simantics.objmap2/src/org/simantics/objmap/structural/schema/AdaptedLinkType.java @@ -72,18 +72,27 @@ public class AdaptedLinkType implements ILinkType mapping, StructuralResource domainElement, IStructuralObject rangeElement) throws MappingException { }; + @Override public void createRange(ReadGraph graph, org.simantics.objmap.forward.IForwardMapping mapping, StructuralResource domainElement, IStructuralObject rangeElement) throws MappingException { }; + @Override + public boolean checkChanges(ReadGraph graph, IForwardMapping mapping, StructuralResource domainElement, IStructuralObject rangeElement) throws MappingException { + return false; + } + + @Override public boolean updateDomain(WriteGraph g, IBackwardMapping map, StructuralResource domainElement, IStructuralObject rangeElement) throws MappingException { return false; } + @Override public boolean updateRange(ReadGraph g, IForwardMapping map, StructuralResource domainElement, IStructuralObject rangeElement) throws MappingException { return false; } diff --git a/bundles/org.simantics.objmap2/src/org/simantics/objmap/structural/schema/SimpleLinkType.java b/bundles/org.simantics.objmap2/src/org/simantics/objmap/structural/schema/SimpleLinkType.java index 77b2ab094..5df6064b8 100644 --- a/bundles/org.simantics.objmap2/src/org/simantics/objmap/structural/schema/SimpleLinkType.java +++ b/bundles/org.simantics.objmap2/src/org/simantics/objmap/structural/schema/SimpleLinkType.java @@ -154,6 +154,15 @@ public class SimpleLinkType implements ILinkType map, + StructuralResource domainElement, IStructuralObject rangeElement) throws MappingException { + boolean updated = false; + for(IBidirectionalMappingRule rule : rules) + updated |= rule.checkChanges(g, map, domainElement, rangeElement); + return updated; + } + public boolean updateDomain(WriteGraph g, IBackwardMapping map, StructuralResource domainElement, IStructuralObject rangeElement) throws MappingException { if(LOGGER.isTraceEnabled()) try { -- 2.47.1