Chanqes to QueryIterSort -- ensure that when a QueryCancelledException
is thrown through SortedBindingIterator, it closes the parent
QueryIterSort.
diff --git
a/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterSort.java
b/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterSort.java
index 173da34..04c2655 100644
---
a/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterSort.java
+++
b/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterSort.java
@@ -92,6 +92,7 @@ public class QueryIterSort extends QueryIterPlainWrapper {
// iterator in a try/finally block, and thus will call
// close() themselves.
catch (QueryCancelledException e) {
+ QueryIterSort.this.close();
close();
throw e;
}
Changes to TestQueryIterSort: three tests; one to ensure that
directly close()ing the QueryIterSort also closes the underlying
source iterator; one to ensure that when the QueryIterSort is
run to completion, the source iterator is self-closed; and one to
ensure that when an exception is thrown part-way through
running the QueryIterSort, the source iterator is closed.
Plus changes to CallbackIterator to track whether close has
been called and to make it self-closing:
diff --git
a/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestQueryIterSort.java
b/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestQueryIterSort.java
index dd1b08c..1f78a7f 100644
---
a/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestQueryIterSort.java
+++
b/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestQueryIterSort.java
@@ -18,9 +18,7 @@
package org.apache.jena.sparql.engine.iterator;
-import static org.junit.Assert.assertEquals ;
-import static org.junit.Assert.assertNotNull ;
-import static org.junit.Assert.assertTrue ;
+import static org.junit.Assert.*;
import java.util.ArrayList ;
import java.util.Iterator ;
@@ -133,6 +131,37 @@ public class TestQueryIterSort {
}
assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)) ;
+ }
+
+ @Test public void testCloseClosesSourceIterator() {
+ Context context = new Context() ;
+ ExecutionContext ec = new ExecutionContext(context, (Graph) null,
(DatasetGraph) null, (OpExecutorFactory) null);
+ QueryIterSort qis = new QueryIterSort(iterator, comparator, ec);
+ qis.close();
+ assertTrue("source iterator should have been closed",
iterator.isClosed());
+ }
+
+ @Test public void testExhaustionClosesSourceIterator() {
+ iterator.setCallback(new Callback()
+ { @Override public void call() { /* do nothing */ }
+ });
+ Context context = new Context() ;
+ ExecutionContext ec = new ExecutionContext(context, (Graph) null,
(DatasetGraph) null, (OpExecutorFactory) null);
+ QueryIterSort qis = new QueryIterSort(iterator, comparator, ec);
+ while (qis.hasNext()) qis.next();
+ assertTrue("source iterator should have been closed",
iterator.isClosed());
+ }
+
+ @Test public void testCancelClosesSourceIterator() {
+ Context context = new Context() ;
+ ExecutionContext ec = new ExecutionContext(context, (Graph) null,
(DatasetGraph) null, (OpExecutorFactory) null);
+ QueryIterSort qis = new QueryIterSort(iterator, comparator, ec);
+ try {
+ while (qis.hasNext()) qis.next();
+ fail("query should have been cancelled by trigger");
+ } catch (QueryCancelledException q) {
+ assertTrue("source iterator should have been closed",
iterator.isClosed());
+ }
}
@Test
@@ -286,6 +315,7 @@ public class TestQueryIterSort {
int trigger ;
Iterator<Binding> delegate ;
boolean canceled = false ;
+ boolean closed = false;
public CallbackIterator(Iterator<Binding> delegate, int trigger,
Callback callback)
{
@@ -294,7 +324,7 @@ public class TestQueryIterSort {
this.trigger = trigger ;
}
- public void setCallback(Callback callback)
+ public void setCallback(Callback callback)
{
this.callback = callback ;
}
@@ -302,7 +332,10 @@ public class TestQueryIterSort {
@Override
public boolean hasNext()
{
- return delegate.hasNext() ;
+ // self-closing
+ boolean has = delegate.hasNext() ;
+ if (has == false) closed = true;
+ return has ;
}
@Override
@@ -325,6 +358,10 @@ public class TestQueryIterSort {
{
return elementsReturned ;
}
+
+ public boolean isClosed() {
+ return closed;
+ }
public boolean isCanceled() {
return canceled ;
@@ -341,7 +378,7 @@ public class TestQueryIterSort {
public void cancel() { canceled = true ; }
@Override
- public void close() { }
+ public void close() { closed = true ; }
@Override
public void output(IndentedWriter out, SerializationContext sCxt)
{ throw new ARQNotImplemented() ;
Chris
On 11 April 2017 at 14:48, Chris Dollin <[email protected]>
wrote:
> working on those now ...
>
> On 11 April 2017 at 14:30, afs <[email protected]> wrote:
>
>> Github user afs commented on the issue:
>>
>> https://github.com/apache/jena/pull/236
>>
>> This PR covers the "Bug in SortedDataBag?" report as well. Waiting
>> on tests for that.
>>
>>
>> ---
>> If your project is set up for it, you can reply to this email and have
>> your
>> reply appear on GitHub as well. If your project does not have this feature
>> enabled and wishes so, or if the feature is enabled but not working,
>> please
>> contact infrastructure at [email protected] or file a JIRA ticket
>> with INFRA.
>> ---
>>
>
>
>
> --
> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
> Affair/
>
> Epimorphics Ltd, http://www.epimorphics.com
> Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
> 6PT
> Epimorphics Ltd. is a limited company registered in England (number
> 7016688)
>
--
"What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
Affair/
Epimorphics Ltd, http://www.epimorphics.com
Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
6PT
Epimorphics Ltd. is a limited company registered in England (number 7016688)