[GitHub] [jena] afs commented on a diff in pull request #1388: GH-1387 Improved custom service executor extension system

[GitBox <gi...@apache.org>]

afs commented on code in PR #1388:
URL: https://github.com/apache/jena/pull/1388#discussion_r899936276


##########
jena-arq/src/main/java/org/apache/jena/sparql/ARQConstants.java:
##########
@@ -292,6 +292,10 @@
     public static final Symbol registryServiceExecutors =
         SystemARQ.allocSymbol("registryServiceExecutors") ;
 
+    /** The service executor library registry key */

Review Comment:
   Are two different keys necessary?



##########
jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterRepeatApply.java:
##########
@@ -18,99 +18,26 @@
 
 package org.apache.jena.sparql.engine.iterator;
 
-import java.util.NoSuchElementException;
-
-import org.apache.jena.atlas.lib.Lib;
-import org.apache.jena.atlas.logging.Log;
 import org.apache.jena.sparql.engine.ExecutionContext;
 import org.apache.jena.sparql.engine.QueryIterator;
 import org.apache.jena.sparql.engine.binding.Binding;
 
 /**
  * Repeatedly execute the subclass operation for each Binding in the input iterator.
  */
-public abstract class QueryIterRepeatApply extends QueryIter1 {
-    private int count = 0;
-    private QueryIterator currentStage;
-    private volatile boolean cancelRequested = false;
+public abstract class QueryIterRepeatApply extends QueryIterRepeatApplyBulk {
 
     public QueryIterRepeatApply(QueryIterator input, ExecutionContext context) {
         super(input, context);
-        this.currentStage = null;
-
-        if ( input == null ) {
-            Log.error(this, "[QueryIterRepeatApply] Repeated application to null input iterator");
-            return;
-        }
-    }
-
-    protected QueryIterator getCurrentStage() {
-        return currentStage;
     }
 
     protected abstract QueryIterator nextStage(Binding binding);
 
+    /** The caller guarantees that input has at least one more item */
     @Override
-    protected boolean hasNextBinding() {
-        if ( isFinished() )
-            return false;
-
-        for ( ;; ) {
-            if ( currentStage == null )
-                currentStage = makeNextStage();
-
-            if ( currentStage == null )
-                return false;
-
-            if ( cancelRequested )
-                // Pass on the cancelRequest to the active stage.
-                performRequestCancel(currentStage);
-
-            if ( currentStage.hasNext() )
-                return true;
-
-            // finish this step
-            currentStage.close();
-            currentStage = null;
-            // loop
-        }
-        // Unreachable
-    }
-
-    @Override
-    protected Binding moveToNextBinding() {
-        if ( !hasNextBinding() )
-            throw new NoSuchElementException(Lib.className(this) + ".next()/finished");
-        return currentStage.nextBinding();
-
-    }
-
-    private QueryIterator makeNextStage() {
-        count++;
-
-        if ( getInput() == null )
-            return null;
-
-        if ( !getInput().hasNext() ) {
-            getInput().close();
-            return null;
-        }
-
+    protected QueryIterator nextStage(QueryIterator input) {

Review Comment:
   Please explain what is going on here. 
   
   If a change is needed, then having a superclass is not necessary. Just two `nextStage` methods with different signatures.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org