Merge pull request apache#531 from afs/fuseki-web-query

JENA-1671: Fix general query servlet.

jena-fuseki2/jena-fuseki-access/src/main/java/org/apache/jena/fuseki/access/AccessCtl_SPARQL_QueryDataset.java

@@ -25,6 +25,7 @@
 import java.util.List;
 import java.util.function.Function;
 
+import org.apache.jena.atlas.lib.Pair;
 import org.apache.jena.fuseki.servlets.ActionService;
 import org.apache.jena.fuseki.servlets.HttpAction;
 import org.apache.jena.fuseki.servlets.SPARQL_QueryDataset;
@@ -44,27 +45,28 @@ public AccessCtl_SPARQL_QueryDataset(Function<HttpAction, String> requestUser) {
     }
 
     private static boolean ALLOW_FROM = true; 
-    
+
     @Override
     protected Collection<String> customParams() {
         // The additional ?user.
         return Collections.singletonList("user");
     }
-    
+
     /** Decide the dataset - this modifies the query
      *  If the query has a dataset description.
      */
     @Override
-    protected DatasetGraph decideDataset(HttpAction action, Query query, String queryStringLog) {
+    protected Pair<DatasetGraph, Query> decideDataset(HttpAction action, Query query, String queryStringLog) {
         DatasetGraph dsg = action.getActiveDSG();
         if ( ! DataAccessCtl.isAccessControlled(dsg) )
             return super.decideDataset(action, query, queryStringLog);
-        
+
         DatasetDescription dsDesc0 = getDatasetDescription(action, query);
         SecurityContext sCxt = DataAccessLib.getSecurityContext(action, dsg, requestUser);
-        return dynamicDataset(action, query, dsg, dsDesc0, sCxt);
+        DatasetGraph dsg2 = dynamicDataset(action, query, dsg, dsDesc0, sCxt);
+        return Pair.create(dsg2,  query);
     }
-        
+
     private DatasetGraph dynamicDataset(HttpAction action, Query query, DatasetGraph dsg0, DatasetDescription dsDesc0, SecurityContext sCxt) {
         if ( dsDesc0 == null )
             return dsg0;

jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/server/Operation.java

@@ -27,10 +27,15 @@
  */
 public class Operation {
 
-    // Create intern'ed symbols. 
+    /** Create/intern. */ 
     static private NameMgr<Operation> mgr = new NameMgr<>(); 
     static public Operation register(String name, String description) {
-        return mgr.register(name, (x)->new Operation(x, description));
+        return mgr.register(name, (x)->create(x, description));
+    }
+
+    /** Create; not registered */
+    static private Operation create(String name, String description) {
+        return new Operation(name, description);
     }
 
     public static final Operation Query          = register("Query", "SPARQL Query");

jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ActionService.java

@@ -25,6 +25,8 @@
 
 import java.io.IOException;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.function.BiFunction;
 
 import javax.servlet.ServletException;
 
@@ -38,8 +40,26 @@
 
 /** Service request lifecycle */
 public abstract class ActionService extends ActionBase {
+
+    private final BiFunction<HttpAction, Operation, ActionService> selectProcessor;
+
+    /**
+     * ActionService for a directly called serlvet. The request handler will be
+     * {@code this.executeLifecycle}.
+     */
     protected ActionService() {
         super(Fuseki.actionLog);
+        selectProcessor = (action, operation)->this;
+    }
+
+    /**
+     * ActionService to redirect to another ActionService to be request handler will be
+     * {@code selectProcessor.executeLifecycle}.
+     * @see ServiceRouter
+     */
+    protected ActionService(BiFunction<HttpAction, Operation, ActionService> selectProcessor) {
+        super(Fuseki.actionLog);
+        this.selectProcessor = selectProcessor;
     }
 
     protected abstract void validate(HttpAction action);
@@ -85,7 +105,7 @@ protected void execCommonWorker(HttpAction action) {
         String endpointName = mapRequestToOperation(action, dataAccessPoint);
 
         // ServiceRouter dispatch
-        Operation operation = null;
+        Operation operation;
         if ( !endpointName.isEmpty() ) {
             operation = chooseOperation(action, dSrv, endpointName);
             if ( operation == null )
@@ -96,8 +116,6 @@ protected void execCommonWorker(HttpAction action) {
         } else {
             // Endpoint ""
             operation = chooseOperation(action, dSrv);
-            if ( operation == null )
-                ServletOps.errorBadRequest(format("dataset=%s", dataAccessPoint.getName()));
         }
 
         // ---- Auth checking.
@@ -120,15 +138,18 @@ protected void execCommonWorker(HttpAction action) {
             // No Endpoint name given; there may be several endpoints for the operation.
             // authorization is the AND of all endpoints.
             Collection<Endpoint> x = getEndpoints(dSrv, operation);
-            if ( x.isEmpty() )
+            if ( operation != null && x.isEmpty() )
                 throw new InternalErrorException("Inconsistent: no endpoints for "+operation);
             x.forEach(ep->{
                 Auth.allow(user, ep.getAuthPolicy(), ServletOps::errorForbidden);
             });
         }
         // ---- End auth checking.
-
-        ActionService handler = action.getServiceDispatchRegistry().findHandler(operation);
+
+        // Decide the code to execute the request.
+        // For ServiceRouter, this involves a lookup in the service dispatch registry.
+        // For directly called services, they override the operations called by this.executeLifecycle. 
+        ActionService handler = selectProcessor.apply(action, operation);
         if ( handler == null )
             ServletOps.errorBadRequest(format("dataset=%s: op=%s", dataAccessPoint.getName(), operation.getName()));
         handler.executeLifecycle(action);
@@ -140,6 +161,8 @@ protected void execCommonWorker(HttpAction action) {
     // If asked for GSP_R and there are no endpoints for GSP_R, try GSP_RW.
     // Ditto Quads_R -> Quads_RW.
     private Collection<Endpoint> getEndpoints(DataService dSrv, Operation operation) {
+        if ( operation == null )
+            return Collections.emptySet();
         Collection<Endpoint> x = dSrv.getEndpoints(operation);
         if ( x == null || x.isEmpty() ) {
             if ( operation == Operation.GSP_R )

jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/SPARQL_Query.java

@@ -37,6 +37,7 @@
 import org.apache.jena.atlas.io.IO ;
 import org.apache.jena.atlas.io.IndentedLineBuffer ;
 import org.apache.jena.atlas.json.JsonObject;
+import org.apache.jena.atlas.lib.Pair;
 import org.apache.jena.atlas.web.ContentType ;
 import org.apache.jena.fuseki.Fuseki ;
 import org.apache.jena.fuseki.system.FusekiNetLib;
@@ -279,8 +280,13 @@ protected void execute(String queryString, HttpAction action) {
         // Assumes finished whole thing by end of sendResult.
         try {
             action.beginRead() ;
-            DatasetGraph dataset = decideDataset(action, query, queryStringLog) ;
-            try ( QueryExecution qExec = createQueryExecution(action, query, dataset) ; ) {
+            Pair<DatasetGraph, Query> p = decideDataset(action, query, queryStringLog) ;
+            DatasetGraph dataset = p.getLeft();
+            Query q = p.getRight();
+            if ( q == null )
+                q = query;
+
+            try ( QueryExecution qExec = createQueryExecution(action, q, dataset) ; ) {
                 SPARQLResult result = executeQuery(action, qExec, query, queryStringLog) ;
                 // Deals with exceptions itself.
                 sendResults(action, result, query.getPrologue()) ;
@@ -390,9 +396,9 @@ private void setAnyProtocolTimeouts(QueryExecution qExec, HttpAction action) {
      * @param action
      * @param query  Query - this may be modified to remove a DatasetDescription.
      * @param queryStringLog
-     * @return {@link Dataset}
+     * @return Pair of {@link Dataset} and {@link Query}. 
      */
-    protected abstract DatasetGraph decideDataset(HttpAction action, Query query, String queryStringLog) ;
+    protected abstract Pair<DatasetGraph, Query> decideDataset(HttpAction action, Query query, String queryStringLog) ;
 
     /** Ship the results to the remote caller.
      * @param action

jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/SPARQL_QueryDataset.java

@@ -18,6 +18,7 @@
 
 package org.apache.jena.fuseki.servlets;
 
+import org.apache.jena.atlas.lib.Pair;
 import org.apache.jena.query.Query ;
 import org.apache.jena.sparql.core.DatasetDescription ;
 import org.apache.jena.sparql.core.DatasetGraph ;
@@ -42,7 +43,7 @@ protected void validateQuery(HttpAction action, Query query)
      *  If the query has a dataset description.
      */
     @Override
-    protected DatasetGraph decideDataset(HttpAction action, Query query, String queryStringLog) {
+    protected Pair<DatasetGraph, Query> decideDataset(HttpAction action, Query query, String queryStringLog) {
         DatasetGraph dsg = action.getActiveDSG() ;
         DatasetDescription dsDesc = getDatasetDescription(action, query) ;
         if ( dsDesc != null ) {
@@ -52,6 +53,6 @@ protected DatasetGraph decideDataset(HttpAction action, Query query, String quer
                 query.getNamedGraphURIs().clear() ;
             }
         }
-        return dsg ;
+        return Pair.create(dsg, query);
     }
 }

jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/SPARQL_QueryGeneral.java

@@ -23,6 +23,7 @@
 import java.util.List ;
 
 import org.apache.jena.atlas.lib.InternalErrorException ;
+import org.apache.jena.atlas.lib.Pair;
 import org.apache.jena.fuseki.server.DataService;
 import org.apache.jena.fuseki.server.Operation;
 import org.apache.jena.fuseki.system.GraphLoadUtils;
@@ -37,6 +38,11 @@
 import org.apache.jena.sparql.core.DatasetGraphZero;
 
 public class SPARQL_QueryGeneral extends SPARQL_Query {
+    // In order to use as much of the main SPARQL code as possible, 
+    // this freestanding servlet mimics being a dataset service, 
+    // and inserts the loaded dataset at decideDataset.
+    // ActionService understands this - the operation is null. 
+
     final static int MaxTriples = 100 * 1000 ;
 
     public SPARQL_QueryGeneral() {
@@ -54,24 +60,28 @@ protected String mapRequestToDataset(HttpAction action) {
         return null ;
     }
 
-    /** SPARQL_QueryGeneral is a servlet to be called directly.
-     * It declares it is own {@code Operation} to fit into {@link ActionService#execCommonWorker}.
-     * The Fuseki service handling continues; 
-     * {@link SPARQL_Query} will ask for a dataset which will return the fixed, empty dataset.
-     * 
-     */
+    /** SPARQL_QueryGeneral is a servlet to be called directly. */
     @Override
     protected Operation chooseOperation(HttpAction action, DataService dataService) {
-        return Operation.Query;
+        return null;
     }
 
     @Override
-    protected DatasetGraph decideDataset(HttpAction action, Query query, String queryStringLog) {
-        DatasetDescription datasetDesc = getDatasetDescription(action, query) ;
-        if ( datasetDesc == null )
+    protected Pair<DatasetGraph, Query> decideDataset(HttpAction action, Query query, String queryStringLog) {
+        DatasetDescription datasetDesc = getDatasetDescription(action, query);
+        if ( datasetDesc == null ) {
             //ServletOps.errorBadRequest("No dataset description in protocol request or in the query string") ;
-            return new DatasetGraphZero();
-        return datasetFromDescriptionWeb(action, datasetDesc) ;
+            return Pair.create(new DatasetGraphZero(), query);
+        }
+
+        // These will have been taken care of by the "getDatasetDescription"
+        if ( query.hasDatasetDescription() ) {
+            // Don't modify input.
+            query = query.cloneQuery();
+            query.getNamedGraphURIs().clear();
+            query.getGraphURIs().clear();
+        }
+        return Pair.create(datasetFromDescriptionWeb(action, datasetDesc), query) ;
     }
 
     /**

jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ServiceRouter.java

@@ -55,7 +55,7 @@
 public class ServiceRouter extends ActionService {
 
     public ServiceRouter() {
-        super();
+        super((action, operation)->action.getServiceDispatchRegistry().findHandler(operation));
     }
 
     // These calls should not happen because ActionService calls chooseOperation(),

jena-fuseki2/jena-fuseki-main/src/main/java/org/apache/jena/fuseki/main/cmds/FusekiMain.java

@@ -168,9 +168,8 @@ protected FusekiMain(String... argv) {
             add(argPasswdFile, "--passwd=FILE", "Password file");
             // put in the configuration file
 //            add(argRealm, "--realm=REALM", "Realm name");
-
-//            add(argWithPing,    "--ping",   "Enable /$/ping");
-//            add(argWithStats,   "--stats",  "Enable /$/stats");
+           add(argWithPing,    "--ping",   "Enable /$/ping");
+           add(argWithStats,   "--stats",  "Enable /$/stats");
 
             super.modVersion.addClass(Fuseki.class);
         }

jena-fuseki2/jena-fuseki-main/src/test/java/org/apache/jena/fuseki/main/CustomService.java → jena-fuseki2/jena-fuseki-main/src/test/java/org/apache/jena/fuseki/main/CustomTestService.java

@@ -26,7 +26,7 @@
 import org.apache.jena.riot.WebContent;
 import org.apache.jena.web.HttpSC;
 
-public class CustomService extends ActionREST {
+public class CustomTestService extends ActionREST {
 
     // do* -- the operations to accept
 

jena-fuseki2/jena-fuseki-main/src/test/java/org/apache/jena/fuseki/main/TC_Fuseki.java

@@ -24,7 +24,7 @@
 
 @RunWith(Suite.class)
 @Suite.SuiteClasses( {
-  TS_EmbeddedFuseki.class,
+  TS_FusekiMain.class,
   TS_SecurityFuseki.class
 })
 public class TC_Fuseki {

jena-fuseki2/jena-fuseki-main/src/test/java/org/apache/jena/fuseki/main/TS_EmbeddedFuseki.java → jena-fuseki2/jena-fuseki-main/src/test/java/org/apache/jena/fuseki/main/TS_FusekiMain.java

@@ -32,20 +32,14 @@
   , TestFusekiTestServer.class
   , TestFusekiTestAuth.class
   , TestFusekiCustomOperation.class
+  , TestFusekiMainCmd.class
 })
-public class TS_EmbeddedFuseki {
+public class TS_FusekiMain {
     @BeforeClass public static void setupForFusekiServer() {
         LogCtl.setLevel(Fuseki.serverLogName,        "WARN");
         LogCtl.setLevel(Fuseki.actionLogName,        "WARN");
         LogCtl.setLevel(Fuseki.requestLogName,       "WARN");
         LogCtl.setLevel(Fuseki.adminLogName,         "WARN");
         LogCtl.setLevel("org.eclipse.jetty",         "WARN");
-
-        // Shouldn't see these in the embedded server.
-//        LogCtl.setLevel("org.apache.shiro",          "WARN") ;
-//        LogCtl.setLevel(Fuseki.configLogName,        "WARN");
-
-//        LogCtl.setLevel(Fuseki.builderLogName,       "WARN");
-//        LogCtl.setLevel(Fuseki.servletRequestLogName,"WARN");
     }
 }

jena-fuseki2/jena-fuseki-main/src/test/java/org/apache/jena/fuseki/main/TestFusekiCustomOperation.java

@@ -35,12 +35,15 @@
 import org.apache.jena.fuseki.server.DataService;
 import org.apache.jena.fuseki.server.Operation;
 import org.apache.jena.fuseki.servlets.ActionService;
+import org.apache.jena.fuseki.servlets.HttpAction;
 import org.apache.jena.query.QueryExecution;
 import org.apache.jena.rdfconnection.RDFConnection;
 import org.apache.jena.rdfconnection.RDFConnectionFactory;
+import org.apache.jena.riot.WebContent;
 import org.apache.jena.riot.web.HttpOp;
 import org.apache.jena.sparql.core.DatasetGraph;
 import org.apache.jena.sparql.core.DatasetGraphFactory;
+import org.apache.jena.web.HttpSC;
 import org.junit.Test;
 
 /** Test for adding a new operation */
@@ -49,7 +52,38 @@ public class TestFusekiCustomOperation {
     private static final String contentType = "application/special";
     private static final String endpointName = "special";
 
-    private final ActionService customHandler = new CustomService();
+    private final ActionService customHandler = new CustomTestService() {
+        @Override
+        protected void doGet(HttpAction action) {
+            action.response.setStatus(HttpSC.OK_200);
+            try {
+                action.response.setContentType(WebContent.contentTypeTextPlain);
+                action.response.getOutputStream().println("    ** Hello world (GET) **");
+            }
+            catch (IOException e) {
+                e.printStackTrace();
+            }
+        }
+
+        @Override
+        protected void doHead(HttpAction action) {
+            action.response.setStatus(HttpSC.OK_200);
+            action.response.setContentType(WebContent.contentTypeTextPlain);
+        }
+
+        @Override
+        protected void doPost(HttpAction action) {
+            action.response.setStatus(HttpSC.OK_200);
+            try {
+                action.response.setContentType(WebContent.contentTypeTextPlain);
+                action.response.getOutputStream().println("    ** Hello world (POST) **");
+            }
+            catch (IOException e) {
+                e.printStackTrace();
+            }
+        }
+    };
+
     private final int port = WebLib.choosePort();
     private final String url = "http://localhost:"+port;