You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jena.apache.org by an...@apache.org on 2016/09/05 12:29:40 UTC

[04/10] jena git commit: Added some QueryIterDistinct tests:

Added some QueryIterDistinct tests:

- that cancelling a QID cancels its base iterator
- likewise if we're serialising to disc
- closing an unbagged QID doesn't make a bag
- closing a bagged QID closes the bag


Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/ed7bbb65
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/ed7bbb65
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/ed7bbb65

Branch: refs/heads/master
Commit: ed7bbb65ae2af16c63bc0b6a70be9eb712d38d85
Parents: efd4dfd
Author: Chris Dollin <eh...@googlemail.com>
Authored: Thu Sep 1 15:04:33 2016 +0100
Committer: Chris Dollin <eh...@googlemail.com>
Committed: Thu Sep 1 15:04:33 2016 +0100

----------------------------------------------------------------------
 .../apache/jena/atlas/data/SortedDataBag.java   |   8 ++
 .../engine/iterator/QueryIterDistinct.java      |   5 +-
 .../engine/iterator/QueryIteratorBase.java      |   7 +-
 .../engine/iterator/TestCancelDistinct.java     | 140 +++++++++++++++++++
 4 files changed, 157 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/jena/blob/ed7bbb65/jena-arq/src/main/java/org/apache/jena/atlas/data/SortedDataBag.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/org/apache/jena/atlas/data/SortedDataBag.java b/jena-arq/src/main/java/org/apache/jena/atlas/data/SortedDataBag.java
index 26ba388..1728129 100644
--- a/jena-arq/src/main/java/org/apache/jena/atlas/data/SortedDataBag.java
+++ b/jena-arq/src/main/java/org/apache/jena/atlas/data/SortedDataBag.java
@@ -103,6 +103,14 @@ public class SortedDataBag<E> extends AbstractDataBag<E> {
     public boolean isCancelled() {
         return comparator.cancelled;
     }
+    
+    /**
+     * isClosed returns true iff this bag has been closed.
+     * (Used in testing.)
+     */
+    public boolean isClosed() {
+        return closed;
+    }
 
     protected void checkClosed() {
         if (closed)

http://git-wip-us.apache.org/repos/asf/jena/blob/ed7bbb65/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterDistinct.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterDistinct.java b/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterDistinct.java
index ee088ec..1860226 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterDistinct.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterDistinct.java
@@ -43,7 +43,7 @@ import org.apache.jena.sparql.engine.binding.BindingProjectNamed ;
 public class QueryIterDistinct extends QueryIter1
 {
     private long memThreshold = Long.MAX_VALUE ;    // Default "off" value.
-    private DistinctDataBag<Binding> db = null ;
+    protected DistinctDataBag<Binding> db = null ;
     private Iterator<Binding> iterator = null ;
     private Set<Binding> seen = new HashSet<>() ;
     private Binding slot = null ;
@@ -141,6 +141,9 @@ public class QueryIterDistinct extends QueryIter1
         db = null ;
     }
 
+    // We don't need to do anything. We're a QueryIter1
+    // and that handles the cancellation of the wrapped
+    // iterator.
     @Override
     protected void requestSubCancel()
     { }

http://git-wip-us.apache.org/repos/asf/jena/blob/ed7bbb65/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIteratorBase.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIteratorBase.java b/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIteratorBase.java
index dee0661..7c1f836 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIteratorBase.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIteratorBase.java
@@ -52,8 +52,11 @@ public abstract class QueryIteratorBase
     // ONLY the requestingCancel variable needs to be volatile. The abortIterator is guaranteed to 
     // be visible because it is written to before requestingCancel, and read from after.
 
-    /** In the process of requesting a cancel, or one has been done */  
-    private volatile boolean requestingCancel = false;
+    /** 
+        In the process of requesting a cancel, or one has been done.
+        `protected` to allow tests to read it.
+    */  
+    protected volatile boolean requestingCancel = false;
 
     /* If set, any hasNext/next throws QueryAbortedException
      * In normal operation, this is the same setting as requestingCancel.

http://git-wip-us.apache.org/repos/asf/jena/blob/ed7bbb65/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestCancelDistinct.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestCancelDistinct.java b/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestCancelDistinct.java
new file mode 100644
index 0000000..7774d3b
--- /dev/null
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestCancelDistinct.java
@@ -0,0 +1,140 @@
+package org.apache.jena.sparql.engine.iterator;
+
+import static org.junit.Assert.*;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+
+import org.apache.jena.atlas.data.DistinctDataBag;
+import org.apache.jena.atlas.io.IndentedWriter;
+import org.apache.jena.graph.Graph;
+import org.apache.jena.query.ARQ;
+import org.apache.jena.sparql.core.DatasetGraph;
+import org.apache.jena.sparql.engine.ExecutionContext;
+import org.apache.jena.sparql.engine.binding.Binding;
+import org.apache.jena.sparql.engine.binding.BindingFactory;
+import org.apache.jena.sparql.engine.main.OpExecutorFactory;
+import org.apache.jena.sparql.serializer.SerializationContext;
+import org.apache.jena.sparql.util.Context;
+import org.junit.Test;
+
+public class TestCancelDistinct {
+
+    private final class MockQueryIterator extends QueryIteratorBase {
+        
+        Iterator<Binding> bindings;
+        
+        MockQueryIterator() {
+            this(new ArrayList<Binding>());
+        }
+        
+        MockQueryIterator(Binding ... bindings) {
+            this(Arrays.asList(bindings));
+        }
+        
+        MockQueryIterator(List<Binding> bindings) {
+            this.bindings = bindings.iterator();
+        }
+        
+        @Override
+        public void output(IndentedWriter out, SerializationContext sCxt) {
+            
+        }
+
+        @Override
+        protected boolean hasNextBinding() {
+            return bindings.hasNext();
+        }
+
+        @Override
+        protected Binding moveToNextBinding() {
+            return bindings.next();
+        }
+
+        @Override
+        protected void closeIterator() {
+            
+        }
+
+        @Override
+        protected void requestCancel() {
+            
+        }
+    }
+
+    /**
+       test that of a QueryIterDistinct is cancelled, so is the
+       iterator that it wraps.
+    */
+    @Test public void testUnbaggedCancelPropagates() {
+        // Something better than null would be good. But making
+        // an ExecutionContext is non-trivial.
+        ExecutionContext c = null;
+        QueryIteratorBase base = new MockQueryIterator();
+            
+        QueryIterDistinct d = new QueryIterDistinct(base, c);
+        assertFalse(base.requestingCancel);
+        d.cancel();
+        assertTrue(base.requestingCancel);
+    }
+    
+    final Context params = new Context();
+
+    final Graph activeGraph = null;
+    final DatasetGraph dataset = null;
+    final OpExecutorFactory factory = null;
+
+    final ExecutionContext c = new ExecutionContext(params, activeGraph, dataset, factory);
+
+    /**
+       test that of a QueryIterDistinct with an active databag is 
+       cancelled, so is the iterator that it wraps.
+    */
+    @Test public void testBaggedCancelPropagates() {        
+        params.set(ARQ.spillToDiskThreshold, 0);
+        
+        QueryIteratorBase base = new MockQueryIterator(BindingFactory.create());
+        QueryIterDistinct d = new QueryIterDistinct(base, c);
+        
+        assertNull(d.db);
+       
+        Binding b = d.next();
+       
+        assertNotNull(d.db);      
+        DistinctDataBag<Binding> db = d.db;
+        
+        assertFalse(base.requestingCancel);
+        d.cancel();
+        assertTrue(base.requestingCancel);
+        
+    }    
+    
+    @Test public void testCloseWhenNoBag() {        
+        params.set(ARQ.spillToDiskThreshold, 0);
+        
+        QueryIteratorBase base = new MockQueryIterator(BindingFactory.create());
+        QueryIterDistinct d = new QueryIterDistinct(base, c);
+        
+        // when there is no databag, close leaves it null
+        assertNull(d.db);
+        d.close();
+        assertNull(d.db);
+    }    
+    
+    @Test public void testCloseWhenBagPresent() {        
+        params.set(ARQ.spillToDiskThreshold, 0);
+        
+        QueryIteratorBase base = new MockQueryIterator(BindingFactory.create());
+        QueryIterDistinct d = new QueryIterDistinct(base, c);
+        
+        assertNull(d.db);
+        Binding ignored = d.next();
+        assertNotNull(d.db); 
+        DistinctDataBag<Binding> bag = d.db;
+        d.close();
+        assertTrue(bag.isClosed());
+        assertNull(d.db);
+    }  
+}