RYA-148 Fixed a bug where Joins would write the same binding set with different visibility orders depending on which child node the new result appeared on.
Project: http://git-wip-us.apache.org/repos/asf/incubator-rya/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-rya/commit/573aedf2 Tree: http://git-wip-us.apache.org/repos/asf/incubator-rya/tree/573aedf2 Diff: http://git-wip-us.apache.org/repos/asf/incubator-rya/diff/573aedf2 Branch: refs/heads/master Commit: 573aedf202cc2366be66715f09f33409b8a6ecd5 Parents: 5e241aa Author: Kevin Chilton <[email protected]> Authored: Wed Aug 10 18:47:35 2016 -0400 Committer: Aaron Mihalik <[email protected]> Committed: Tue Aug 23 10:43:47 2016 -0400 ---------------------------------------------------------------------- .../pcj/fluo/app/JoinResultUpdater.java | 31 +++++-- .../pcj/fluo/app/IterativeJoinTest.java | 88 ++++++++++++++++++++ 2 files changed, 110 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-rya/blob/573aedf2/extras/rya.pcj.fluo/pcj.fluo.app/src/main/java/org/apache/rya/indexing/pcj/fluo/app/JoinResultUpdater.java ---------------------------------------------------------------------- diff --git a/extras/rya.pcj.fluo/pcj.fluo.app/src/main/java/org/apache/rya/indexing/pcj/fluo/app/JoinResultUpdater.java b/extras/rya.pcj.fluo/pcj.fluo.app/src/main/java/org/apache/rya/indexing/pcj/fluo/app/JoinResultUpdater.java index 8439360..dfa5b62 100644 --- a/extras/rya.pcj.fluo/pcj.fluo.app/src/main/java/org/apache/rya/indexing/pcj/fluo/app/JoinResultUpdater.java +++ b/extras/rya.pcj.fluo/pcj.fluo.app/src/main/java/org/apache/rya/indexing/pcj/fluo/app/JoinResultUpdater.java @@ -330,7 +330,7 @@ public class JoinResultUpdater { checkNotNull(rightResults); // Both sides are required, so if there are no right results, then do not emit anything. - return new LazyJoiningIterator(newLeftResult, rightResults); + return new LazyJoiningIterator(Side.LEFT, newLeftResult, rightResults); } @Override @@ -339,7 +339,7 @@ public class JoinResultUpdater { checkNotNull(newRightResult); // Both sides are required, so if there are no left reuslts, then do not emit anything. - return new LazyJoiningIterator(newRightResult, leftResults); + return new LazyJoiningIterator(Side.RIGHT, newRightResult, leftResults); } } @@ -365,7 +365,7 @@ public class JoinResultUpdater { // Otherwise, return an iterator that holds the new required result // joined with the right results. - return new LazyJoiningIterator(newLeftResult, rightResults); + return new LazyJoiningIterator(Side.LEFT, newLeftResult, rightResults); } @Override @@ -375,7 +375,7 @@ public class JoinResultUpdater { // The right result is optional, so if it does not join with anything // on the left, then do not emit anything. - return new LazyJoiningIterator(newRightResult, leftResults); + return new LazyJoiningIterator(Side.RIGHT, newRightResult, leftResults); } } @@ -388,18 +388,21 @@ public class JoinResultUpdater { */ private static final class LazyJoiningIterator implements Iterator<VisibilityBindingSet> { + private final Side newResultSide; private final VisibilityBindingSet newResult; private final Iterator<VisibilityBindingSet> joinedResults; /** * Constructs an instance of {@link LazyJoiningIterator}. * + * @param newResultSide - Indicates which side of the join the {@code newResult} arrived on. (not null) * @param newResult - A binding set that will be joined with some other binding sets. (not null) - * @param joinResults - The binding sets that will be joined with {@code newResult}. (not null) + * @param joinedResults - The binding sets that will be joined with {@code newResult}. (not null) */ - public LazyJoiningIterator(final VisibilityBindingSet newResult, final Iterator<VisibilityBindingSet> joinResults) { + public LazyJoiningIterator(final Side newResultSide, final VisibilityBindingSet newResult, final Iterator<VisibilityBindingSet> joinedResults) { + this.newResultSide = checkNotNull(newResultSide); this.newResult = checkNotNull(newResult); - joinedResults = checkNotNull(joinResults); + this.joinedResults = checkNotNull(joinedResults); } @Override @@ -420,10 +423,20 @@ public class JoinResultUpdater { bs.addBinding(binding); } + // We want to make sure the visibilities are always written the same way, + // so figure out which are on the left side and which are on the right side. + final String leftVisi; + final String rightVisi; + if(newResultSide == Side.LEFT) { + leftVisi = newResult.getVisibility(); + rightVisi = joinResult.getVisibility(); + } else { + leftVisi = joinResult.getVisibility(); + rightVisi = newResult.getVisibility(); + } + String visibility = ""; final Joiner join = Joiner.on(")&("); - final String leftVisi = newResult.getVisibility(); - final String rightVisi = joinResult.getVisibility(); if(leftVisi.isEmpty() || rightVisi.isEmpty()) { visibility = (leftVisi + rightVisi).trim(); } else { http://git-wip-us.apache.org/repos/asf/incubator-rya/blob/573aedf2/extras/rya.pcj.fluo/pcj.fluo.app/src/test/java/org/apache/rya/indexing/pcj/fluo/app/IterativeJoinTest.java ---------------------------------------------------------------------- diff --git a/extras/rya.pcj.fluo/pcj.fluo.app/src/test/java/org/apache/rya/indexing/pcj/fluo/app/IterativeJoinTest.java b/extras/rya.pcj.fluo/pcj.fluo.app/src/test/java/org/apache/rya/indexing/pcj/fluo/app/IterativeJoinTest.java new file mode 100644 index 0000000..51393d6 --- /dev/null +++ b/extras/rya.pcj.fluo/pcj.fluo.app/src/test/java/org/apache/rya/indexing/pcj/fluo/app/IterativeJoinTest.java @@ -0,0 +1,88 @@ +/* + * 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.rya.indexing.pcj.fluo.app; + +import static org.junit.Assert.assertEquals; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; + +import org.apache.rya.indexing.pcj.fluo.app.JoinResultUpdater.IterativeJoin; +import org.apache.rya.indexing.pcj.fluo.app.JoinResultUpdater.LeftOuterJoin; +import org.apache.rya.indexing.pcj.fluo.app.JoinResultUpdater.NaturalJoin; +import org.apache.rya.indexing.pcj.storage.accumulo.VisibilityBindingSet; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; +import org.openrdf.model.ValueFactory; +import org.openrdf.model.impl.ValueFactoryImpl; +import org.openrdf.query.impl.MapBindingSet; + +/** + * Tests the methods of {@link IterativeJoin}. + */ +@RunWith(Parameterized.class) +public class IterativeJoinTest { + + @Parameters + public static Collection<Object[]> data() { + return Arrays.asList(new Object[][] { + {new NaturalJoin()}, + {new LeftOuterJoin()} + }); + } + + @Parameter + public IterativeJoin join; + + /** + * This test ensures the same binding sets are created as the result of a + * {@link IterativeJoin} regardless of which side the notification is triggered on. + */ + @Test + public void naturalJoin_sideDoesNotMatter() { + // Create the binding sets that will be joined. + final ValueFactory vf = new ValueFactoryImpl(); + + final MapBindingSet bs1 = new MapBindingSet(); + bs1.addBinding("id", vf.createLiteral("some_uid")); + bs1.addBinding("name", vf.createLiteral("Alice")); + final VisibilityBindingSet vbs1 = new VisibilityBindingSet(bs1, "a"); + + final MapBindingSet bs2 = new MapBindingSet(); + bs2.addBinding("id", vf.createLiteral("some_uid")); + bs2.addBinding("hair", vf.createLiteral("brown")); + final VisibilityBindingSet vbs2 = new VisibilityBindingSet(bs2, "b"); + + // new vbs1 shows up on the left, matches vbs2 on the right + final Iterator<VisibilityBindingSet> newLeftIt = join.newLeftResult(vbs1, Collections.singleton(vbs2).iterator()); + final VisibilityBindingSet newLeftResult = newLeftIt.next(); + + // new vbs2 shows up on the right, matches vbs1 on the left + final Iterator<VisibilityBindingSet> newRightIt = join.newRightResult(Collections.singleton(vbs1).iterator(), vbs2); + final VisibilityBindingSet newRightResult = newRightIt.next(); + + // Ensure those two results are the same. + assertEquals(newLeftResult, newRightResult); + } +} \ No newline at end of file
