From: Marko Luukkainen Date: Fri, 25 Aug 2023 12:04:52 +0000 (+0300) Subject: Graph Comparison / Change management fixes. X-Git-Url: https://gerrit.simantics.org/r/gitweb?a=commitdiff_plain;h=a43434a612fa0aafd0d4f17ebed2b770c4b1d585;p=simantics%2Finterop.git Graph Comparison / Change management fixes. Graph comparator could create conflicting path matches. Graph comparator failed to find matches, if lowest difference match was conflicting for some objects, while other objects contained unique higher difference matches. GraphChanges did not take account property additions. gitlab #43 Change-Id: If29407759e7ee26c55adf9d2c6e4a8ad035b8d85 --- diff --git a/org.simantics.interop/src/org/simantics/interop/test/GraphChanges.java b/org.simantics.interop/src/org/simantics/interop/test/GraphChanges.java index e52ad97..f906507 100644 --- a/org.simantics.interop/src/org/simantics/interop/test/GraphChanges.java +++ b/org.simantics.interop/src/org/simantics/interop/test/GraphChanges.java @@ -47,11 +47,11 @@ public class GraphChanges { } public boolean isLeftAsserted() { - return !leftSub.equals(leftStm.getSubject()); + return leftStm != null && !leftSub.equals(leftStm.getSubject()); } public boolean isRightAsserted() { - return !rightSub.equals(rightStm.getSubject()); + return rightStm != null && !rightSub.equals(rightStm.getSubject()); } public Resource getLeftSub() { @@ -301,49 +301,54 @@ public class GraphChanges { Statement del = mod.leftStm; Resource sub1 = mod.getLeftSub(); Resource sub2 = bijection.getComparable().getRight(sub1); - if (sub2 == null) { - throw new DatabaseException("Did not find matching resource from bijection for " + sub1); - } - Resource s1 = del.getSubject(); - Resource s2 = bijection.getComparable().getRight(s1); - Resource p1 = del.getPredicate(); + Resource p1 = del != null ? del.getPredicate() : mod.getPredicate(); Resource p2 = bijection.getComparable().getRight(p1); if (p2 == null) p2 = p1; - Resource o1 = del.getObject(); + Resource o1 = del != null ? del.getObject() : null; Resource o2 = bijection.getComparable().getRight(o1); + + if (mod.isLeftAsserted()) { + if (sub2 == null) + sub2 = sub1; + + } + if (o2 == null && p2 != null) + o2 = graph.getPossibleObject(sub2, p2); if (mod.isLeftAsserted()) { - if (s2 == null) - s2 = s1; if (o2 == null) o2 = o1; - } - - if (s2 == null || p2 == null) { - throw new DatabaseException("Did not find matching statement from bijection for (" + s1 +","+p1+","+o1+"), got (" + s2 +","+p2+","+o2+")"); } - Collection stms = graph.getStatements(s2, p2); - Statement stm2 = null; - if (o2 == null) { - // Matching literal resources are not included in bijection map. - if (stms.size() == 1) { - stm2 = stms.iterator().next(); - } else { - throw new DatabaseException("Did not find matching statement from bijection for (" + s1 +","+p1+","+o1+"), got (" + s2 +","+p2+","+o2+")"); - } + if (sub2 == null || p2 == null) { + throw new DatabaseException("Did not find matching statement from bijection for (" + sub1 +","+p1+","+o1+"), got (" + sub2 +","+p2+","+o2+")"); + } + if (del == null) { + Modification mod2 = new Modification(sub2, mod.rightSub, null, mod.rightStm); + modifications.add(mod2); } else { - for (Statement s : stms) { - if (s.getObject().equals(o2)) { - stm2 = s; - break; + Collection stms = graph.getStatements(sub2, p2); + Statement stm2 = null; + if (o2 == null) { + // Matching literal resources are not included in bijection map. + if (stms.size() == 1) { + stm2 = stms.iterator().next(); + } else { + throw new DatabaseException("Did not find matching statement from bijection for (" + sub1 +","+p1+","+o1+"), got (" + sub2 +","+p2+","+o2+")"); + } + } else { + for (Statement s : stms) { + if (s.getObject().equals(o2)) { + stm2 = s; + break; + } } } + if (stm2 == null) { + throw new DatabaseException("Did not find matching statement from bijection for (" + sub1 +","+p1+","+o1+"), got (" + sub2 +","+p2+","+o2+"), but it is not in DB!"); + } + Modification mod2 = new Modification(sub2, mod.rightSub, stm2, mod.rightStm); + modifications.add(mod2); } - if (stm2 == null) { - throw new DatabaseException("Did not find matching statement from bijection for (" + s1 +","+p1+","+o1+"), got (" + s2 +","+p2+","+o2+"), but it is not in DB!"); - } - Modification mod2 = new Modification(sub2, mod.rightSub, stm2, mod.rightStm); - modifications.add(mod2); } } diff --git a/org.simantics.interop/src/org/simantics/interop/test/GraphComparator.java b/org.simantics.interop/src/org/simantics/interop/test/GraphComparator.java index 7ea8f4d..6064d49 100644 --- a/org.simantics.interop/src/org/simantics/interop/test/GraphComparator.java +++ b/org.simantics.interop/src/org/simantics/interop/test/GraphComparator.java @@ -109,9 +109,6 @@ public class GraphComparator { this.comparator = comparator; } - ArrayList ss1 = new ArrayList(); - ArrayList ss2 = new ArrayList(); - public Comparator getResourceComparator() { return rcomp; @@ -241,8 +238,6 @@ public class GraphComparator { nonMatchedRight = null; nonTested = null; nonTraversed = null; - ss1 = null; - ss2 = null; strong = null; tested = null; traversed = null; @@ -379,15 +374,21 @@ public class GraphComparator { private boolean compareIter(ReadGraph graph, Stack objectsLeft, Stack objectsRight, Set unreliableLeft, Set unreliableRight) throws DatabaseException { // process compares objects that are identified and searches for more resources to process. iter = 0; - boolean c = process(objectsLeft, objectsRight, unreliableLeft, unreliableRight); + boolean c = process(objectsLeft, objectsRight, unreliableLeft, unreliableRight,false); if (objectsLeft.isEmpty()) { // process unreliable handles cases where unidentified statements subject and object have been identified c |= processUnreliable(unreliableLeft, unreliableRight); // process unreliable handles cases where unidentified resources have path of length one to identified resource if (!c) { - c |= processUnreliable(unreliableLeft, unreliableRight,objectsLeft,objectsRight); + c |= processUnreliable(unreliableLeft, unreliableRight,objectsLeft,objectsRight,false); + if (!c && objectsLeft.isEmpty() && unreliableLeft.size() > 0 && unreliableRight.size() > 0) { + c |= processUnreliable(unreliableLeft, unreliableRight,objectsLeft,objectsRight,true); + } if (!c && objectsLeft.isEmpty() && unreliableLeft.size() > 0 && unreliableRight.size() > 0) { - c |= processUnreliable2(unreliableLeft, unreliableRight,objectsLeft,objectsRight); + c |= processUnreliable2(unreliableLeft, unreliableRight,objectsLeft,objectsRight,false); + } + if (!c && objectsLeft.isEmpty() && unreliableLeft.size() > 0 && unreliableRight.size() > 0) { + c |= processUnreliable2(unreliableLeft, unreliableRight,objectsLeft,objectsRight,true); } if (!c && objectsLeft.isEmpty() && unreliableLeft.size() > 0 && unreliableRight.size() > 0) { // comparison is ending, but we have still unprocessed unidentified resources left. @@ -401,6 +402,8 @@ public class GraphComparator { } } } + if (!c) + process(objectsLeft, objectsRight, unreliableLeft, unreliableRight,true); return c; } @@ -411,7 +414,7 @@ public class GraphComparator { return "Graph compare " + (cr + ch) + " / " + (cr+ch+(Math.max(objectsLeft.size(), objectsRight.size())+Math.max(unreliableLeft.size(), unreliableRight.size()))); } - private boolean process(Stack objectsLeft, Stack objectsRight, Set unreliableLeft, Set unreliableRight) throws DatabaseException { + private boolean process(Stack objectsLeft, Stack objectsRight, Set unreliableLeft, Set unreliableRight, boolean useBest) throws DatabaseException { List ss1 = new ArrayList(); List ss2 = new ArrayList(); boolean didSomething = false; @@ -446,7 +449,7 @@ public class GraphComparator { ss1 = filter(Collections.singletonList(b.HasProperty), ss1); ss2 = filter(Collections.singletonList(b.HasProperty), ss2); - compareStatements(ss1, ss2, null, null,null,null); + compareStatements(ss1, ss2, null, null,null,null, useBest); ss1.clear(); ss2.clear(); } @@ -458,7 +461,7 @@ public class GraphComparator { ss2 = filterAsserted(r2, ss2); ss1 = filterNonTraversed(ss1); ss2 = filterNonTraversed(ss2); - compareStatements(ss1, ss2, objectsLeft, objectsRight,unreliableLeft,unreliableRight); + compareStatements(ss1, ss2, objectsLeft, objectsRight,unreliableLeft,unreliableRight, useBest); ss1.clear(); ss2.clear(); @@ -515,7 +518,7 @@ public class GraphComparator { return didSomething; } - private boolean processUnreliable(Set unreliableLeft, Set unreliableRight, Stack objectsLeft, Stack objectsRight) throws DatabaseException { + private boolean processUnreliable(Set unreliableLeft, Set unreliableRight, Stack objectsLeft, Stack objectsRight, boolean useBest) throws DatabaseException { MapList subjectLeft = new MapList(); MapList subjectRight = new MapList(); MapList objectLeft = new MapList(); @@ -555,8 +558,6 @@ public class GraphComparator { } } - compareStatements(ss1, ss2, objectsLeft, objectsRight,unreliableLeft,unreliableRight); - for (Resource similarOl : comparableOLeft.getKeys()) { List similarLeft = comparableOLeft.getValues(similarOl); if (similarLeft.size() == left.size()) { @@ -701,7 +702,7 @@ public class GraphComparator { } - private boolean processUnreliable2(Set unreliableLeft, Set unreliableRight, Stack objectsLeft, Stack objectsRight) throws DatabaseException { + private boolean processUnreliable2(Set unreliableLeft, Set unreliableRight, Stack objectsLeft, Stack objectsRight, boolean useBest) throws DatabaseException { MapList subjectLeft = new MapList(); MapList subjectRight = new MapList(); MapList objectLeft = new MapList(); @@ -752,7 +753,7 @@ public class GraphComparator { int lcount = changes1.size(); int rcount = changes2.size(); - compareStatements(ss1, ss2, objectsLeft, objectsRight,unreliableLeft,unreliableRight); + compareStatements(ss1, ss2, objectsLeft, objectsRight,unreliableLeft,unreliableRight, useBest); if (comparableStatements.size() > ccount) { didSomething = true; for (Entry entry : comparableStatements.getEntries()) { @@ -836,25 +837,62 @@ public class GraphComparator { } if (matchingPaths.size() > 0) { if (matchingPaths.size() == 1) { - didSomething = true; Resource or = matchingPaths.keySet().iterator().next(); - - objectsLeft.add(ol); - objectsRight.add(or); - addComparable(ol, or); - Collection statementsLeft = objectLeft.getValues(ol); - Collection statementsRight = objectRight.getValues(or); - unreliableLeft.removeAll(statementsLeft); - unreliableRight.removeAll(statementsRight); BijectionMap map = getMatchingPaths(pathsLeft, matchingPaths.get(or)); + BijectionMap toMap = new BijectionMap(); + toMap.map(ol, or); + boolean pass = true; + // Path matching may create conflicting results, prevent selecting them. for (Path left : map.getLeftSet()) { Path right = map.getRight(left); for (int i = 0; i < left.getLength(); i++) { - addComparable(left.getStatements().get(i),right.getStatements().get(i)); - iter++; + Resource l = left.getStatements().get(i).getObject(); + Resource r = right.getStatements().get(i).getObject(); + if (comparableResources.contains(l, r)) + continue; + if (comparableResources.containsLeft(l)) { + pass = false; + break; + } + if (comparableResources.containsRight(r)) { + pass = false; + break; + } + if (toMap.contains(l,r)) + continue; + if (toMap.containsLeft(l)) { + pass = false; + break; + } + if (toMap.containsRight(r)) { + pass = false; + break; + } + toMap.map(l, r); + } } - iter++; + if (pass) { + didSomething = true; + + objectsLeft.add(ol); + objectsRight.add(or); + addComparable(ol, or); + Collection statementsLeft = objectLeft.getValues(ol); + Collection statementsRight = objectRight.getValues(or); + unreliableLeft.removeAll(statementsLeft); + unreliableRight.removeAll(statementsRight); + for (Path left : map.getLeftSet()) { + Path right = map.getRight(left); + for (int i = 0; i < left.getLength(); i++) { + addComparable(left.getStatements().get(i),right.getStatements().get(i)); + iter++; + } + } + iter++; + } else { + iter++; + } } } } @@ -1080,11 +1118,13 @@ public class GraphComparator { } } - private void addModification(Resource left, Statement leftstm, Resource right, Statement rightstm) { + private void addModification(Resource left, Statement leftstm, Resource right, Statement rightstm) throws DatabaseException { Modification mod = new Modification(left, right, leftstm, rightstm); if (!modificationsSet.contains(mod)) { modificationsSet.add(mod); modifications.add(mod); + if (mapLiterals && leftstm != null && rightstm != null) + addComparable(leftstm, rightstm); } } @@ -1166,7 +1206,7 @@ public class GraphComparator { } } - private void compareStatements(List ss1, List ss2, Stack objectsLeft, Stack objectsRight, Collection unreliableLeft, Collection unreliableRight) throws DatabaseException { + private void compareStatements(List ss1, List ss2, Stack objectsLeft, Stack objectsRight, Collection unreliableLeft, Collection unreliableRight, boolean useBest) throws DatabaseException { sortStatement(ss1, ss2); int i1 = 0; @@ -1197,7 +1237,7 @@ public class GraphComparator { int same2 = sameRel(ss2, i2); int c = rcomp.compare(ss1.get(i1).getPredicate(),ss2.get(i2).getPredicate()); if (c == 0) { - compareStatements(ss1, i1, same1, ss2, i2, same2,objectsLeft,objectsRight,unreliableLeft,unreliableRight); + compareStatements(ss1, i1, same1, ss2, i2, same2,objectsLeft,objectsRight,unreliableLeft,unreliableRight, useBest); i1+=same1; i2+=same2; } else if (c < 0) { @@ -1250,7 +1290,7 @@ public class GraphComparator { return comparator.compare(g, o1, o2); } - private void compareStatements(List ss1, int off1, int len1, List ss2, int off2, int len2, Collection objectsLeft, Collection objectsRight, Collection unreliableLeft, Collection unreliableRight) throws DatabaseException { + private void compareStatements(List ss1, int off1, int len1, List ss2, int off2, int len2, Collection objectsLeft, Collection objectsRight, Collection unreliableLeft, Collection unreliableRight, boolean useBest) throws DatabaseException { boolean[] used1 = new boolean[len1]; for (int i = 0; i < used1.length; i++) { used1[i] = false; @@ -1293,10 +1333,13 @@ public class GraphComparator { Integer[] pris = priorities.getKeys(new Integer[]{}); Arrays.sort(pris); boolean matchFail = priorityMatching(ss1, off1, len1, ss2, off2, len2, pris, differences, priorities, used1, used2, objectsLeft, objectsRight, false); - if (matchFail) - matchFail = priorityMatching(ss1, off1, len1, ss2, off2, len2, pris, differences, priorities, used1, used2, objectsLeft, objectsRight, objectsLeft == null); + if (matchFail && objectsLeft == null) + matchFail = priorityMatching(ss1, off1, len1, ss2, off2, len2, pris, differences, priorities, used1, used2, objectsLeft, objectsRight,true); + if (unreliableLeft != null) { if (matchFail) { + if (useBest) + bestMatching(ss1, off1, len1, ss2, off2, len2, differences, priorities, used1, used2, objectsLeft, objectsRight); // With match failure, statement matching was aborted. We must move all unmatched statements to unreliable. for (Integer pri : pris) { if (pri == 0 || pri == Integer.MAX_VALUE) @@ -1446,6 +1489,63 @@ public class GraphComparator { } return matchFail; } + + /** + * priority matching has a flaw, if lowest priority produces conflicting results, it is not able to match anything. + * This matching logic looks for a best match, and if it is non conflicting applies the match. + * + * TODO: now this applies all unique lowest priority matches for each object. Should this pick only the object with global lowest priority, + * so that new matches improve situation with conflicting (non unique) matches? + * + * @param ss1 + * @param off1 + * @param len1 + * @param ss2 + * @param off2 + * @param len2 + * @param differences + * @param priorities + * @param used1 + * @param used2 + * @param objectsLeft + * @param objectsRight + * @throws DatabaseException + */ + private void bestMatching(List ss1, int off1, int len1, List ss2, int off2, int len2, List> differences, MapList priorities, boolean used1[],boolean used2[], Collection objectsLeft, Collection objectsRight) throws DatabaseException { + for (int i1 = 0; i1 < differences.size(); i1++) { + if (used1[i1]) + continue; + List diffs = differences.get(i1); + int min = Integer.MAX_VALUE; + int minIndex = -1; + for (int j = 0; j < diffs.size(); j++) { + int p = diffs.get(j); + if (p < 0) + continue; + if (p < min) { + min = p; + minIndex = j; + } else if (p == min) { + minIndex = -1; + } + } + if (minIndex >= 0 && min > 0) { + int i2 = minIndex; + if (!used2[i2]) { + used1[i1] = true; + used2[i2] = true; + Statement s1 = ss1.get(i1+off1); + Statement s2 = ss2.get(i2+off2); + + if (objectsLeft != null) { + objectsLeft.add(s1.getObject()); + objectsRight.add(s2.getObject()); + } + addComparable(s1, s2); + } + } + } + } /**