You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@jena.apache.org by GitBox <gi...@apache.org> on 2020/09/09 20:02:20 UTC

[GitHub] [jena] afs opened a new pull request #794: JENA-1960: Clean up Fuseki dispatch

afs opened a new pull request #794:
URL: https://github.com/apache/jena/pull/794


   Covers:
   
   [JENA-1960](https://issues.apache.org/jira/browse/JENA-1960) Clean up Fuseki dispatch 
   
   and the small:
   
   [JENA-1961](https://issues.apache.org/jira/browse/JENA-1961) Support /$/metrics in Fuseki Main
   [JENA-1962](https://issues.apache.org/jira/browse/JENA-1962) Assembler for a dataset of a single graph taken from another dataset.
   


----------------------------------------------------------------
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.

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


[GitHub] [jena] afs merged pull request #794: JENA-1960: Clean up Fuseki dispatch

Posted by GitBox <gi...@apache.org>.
afs merged pull request #794:
URL: https://github.com/apache/jena/pull/794






----------------------------------------------------------------
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.

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


[GitHub] [jena] afs commented on a change in pull request #794: JENA-1960: Clean up Fuseki dispatch

Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #794:
URL: https://github.com/apache/jena/pull/794#discussion_r486203790



##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/server/Dispatcher.java
##########
@@ -144,19 +146,16 @@ private static ActionProcessor chooseProcessor(HttpAction action) {
 
         // ---- Determine Endpoint.
         String endpointName = mapRequestToEndpointName(action, dataAccessPoint);
+        // Main step of choosing the endpoint for the dispatch of the request.
+        // An endpoint is a (name, operation).
+        // There may be multiple operations for an endpointName of this data service.
 
         Endpoint endpoint = chooseEndpoint(action, dataService, endpointName);
-        if ( endpoint == null ) {
-            if ( isEmpty(endpointName) )
-                ServletOps.errorBadRequest("No operation for request: "+action.getActionURI());
-            else {
-                // No dispatch - the filter passes these through if the ActionProcessor is null.
-                return null;
-                // If this is used, resources (servlets, sttaic files) under "/dataset/" are not accessible.
-                //ServletOps.errorNotFound("No endpoint: "+action.getActionURI());
-            }
+        if ( endpoint == null )
+            // Named service, no such endpoint.
+            // Allows for resources under /dataset/
+            // Does to Jetty's default handling (404 for GET, 405 other methods).

Review comment:
       This is actually out of date. This PR also changes the default handling of this case, replacing the Jetty default handling because Jetty is different to other servers.
   




----------------------------------------------------------------
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.

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


[GitHub] [jena] afs merged pull request #794: JENA-1960: Clean up Fuseki dispatch

Posted by GitBox <gi...@apache.org>.
afs merged pull request #794:
URL: https://github.com/apache/jena/pull/794


   


----------------------------------------------------------------
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.

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


[GitHub] [jena] rvesse commented on a change in pull request #794: JENA-1960: Clean up Fuseki dispatch

Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #794:
URL: https://github.com/apache/jena/pull/794#discussion_r486171130



##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/build/FusekiConfig.java
##########
@@ -455,17 +456,54 @@ private static DataService buildDataService(Resource fusekiService, DatasetDescr
         accEndpointOldStyle(endpoints1, Operation.GSP_R,    fusekiService,  pServiceReadGraphStoreEP);
         accEndpointOldStyle(endpoints1, Operation.GSP_RW,   fusekiService,  pServiceReadWriteGraphStoreEP);
 
+        // ---- Legacy for old style: a request wouls also try the dataset (i.e. no endpoint name).

Review comment:
       `wouls` -> `would`

##########
File path: jena-arq/src/main/java/org/apache/jena/sparql/core/assembler/AssemblerUtils.java
##########
@@ -77,7 +79,7 @@ static public void registerModel(Resource r, Assembler a) {
         register(ConstAssembler.general(), r, a, JA.Model) ;
     }
 
-    /** Register an addition assembler */  
+    /** Register an addition assembler */

Review comment:
       `additional` ?

##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/server/Dispatcher.java
##########
@@ -203,151 +190,91 @@ private static ActionProcessor chooseProcessor(HttpAction action) {
         return processor;
     }
 
-    /**
-     * Map request to operation name.
-     * Returns the service name (the part after the "/" of the dataset part) or "".
-     */
-    protected static String mapRequestToEndpointName(HttpAction action, DataAccessPoint dataAccessPoint) {
-        return ActionLib.mapRequestToEndpointName(action, dataAccessPoint);
-    }
 
-    // Find the endpoints for an operation.
-    // This is GSP_R/GSP_RW aware.
-    // If asked for GSP_R and there are no endpoints for GSP_R, try GSP_RW.
-    private static Collection<Endpoint> getEndpoints(DataService dataService, Operation operation) {
-        Collection<Endpoint> x = dataService.getEndpoints(operation);
-        if ( x == null || x.isEmpty() ) {
-            if ( operation == GSP_R )
-                x = dataService.getEndpoints(GSP_RW);
-        }
-        return x;
-    }
 
-    /**
-     * Choose an endpoint. This can be with or without endpointName.
-     * If there is no endpoint and the action is on the data service itself (unnamed endpoint)
-     * look for a named endpoint that supplies the operation.
-     */
-    private static Endpoint chooseEndpoint(HttpAction action, DataService dataService, String endpointName) {
-        try {
-            Endpoint ep = chooseEndpointNoLegacy(action, dataService, endpointName);
-            if ( ep != null )
-                return ep;
-            // No dispatch so far.
-
-            if ( ! isEmpty(endpointName) )
-                return ep;
-            // [DISPATCH LEGACY]
-
-            // When it is a unnamed service request (operation on the dataset) and there
-            // is no match, search the named services.
-            Operation operation = chooseOperation(action);
-            // Search for an endpoint that provides the operation.
-            // No guarantee it has the access controls for the operation
-            // but in this case, access control will validate against all possible endpoints.
-            ep = findEndpointForOperation(action, dataService, operation, true);
-            return ep;
-        } catch (ActionErrorException ex) {
-            throw ex;
-        } catch (RuntimeException ex) {
-            // Example: Jetty throws BadMessageException when it is an HTML form and it is too big.
-            ServletOps.errorBadRequest(ex.getMessage());
-            return null;
-        }
-    }
+//    // Find the endpoints for an operation.
+//    // This is GSP_R/GSP_RW aware.
+//    // If asked for GSP_R and there are no endpoints for GSP_R, try GSP_RW.
+//    private static Collection<Endpoint> getEndpoints(DataService dataService, Operation operation) {
+//        Collection<Endpoint> x = dataService.getEndpoints(operation);
+//        if ( x == null || x.isEmpty() ) {
+//            if ( operation == GSP_R ) // [GSP Promote]
+//                x = dataService.getEndpoints(GSP_RW);
+//        }
+//        return x;
+//    }
 
     /**
      * Choose an endpoint.
+     * An endpoint is a name and an operation.
      * <ul>
      * <li>Look by service name to get the EndpointSet</li>
-     * <li>If empty set, return null.</li>
+     * <li>If empty set, respond with error</li>
      * <li>If there is only one choice, return that (may even be the wrong operation
-     *       - processor implmentations must be defensive).</li>
+     *       - processor implementations must be defensive).</li>
      * <li>If multiple choices, classify the operation
      *     (includes custom content-type) and look up by operation.</li>
-     * <li>Return a match wit a r
+     * <li>If not suitable, respond with error
+     * <li>Return an endpoint.
      * </ul>
+     * The endpoint chosen may not be suitable, the operation must do checking.
      */
-    private static Endpoint chooseEndpointNoLegacy(HttpAction action, DataService dataService, String endpointName) {
+    private static Endpoint chooseEndpoint(HttpAction action, DataService dataService, String endpointName) {
         EndpointSet epSet = isEmpty(endpointName) ? dataService.getEndpointSet() : dataService.getEndpointSet(endpointName);
-
-        if ( epSet == null || epSet.isEmpty() )
+        if ( epSet == null || epSet.isEmpty() ) {
             // No matches by name.
-            return null;
+            if ( ! StringUtils.isAnyEmpty(endpointName) )
+                // There was a service name, not found.
+                // But it may be a URL for static resource.
+                return null;
+//                // No service endpoint so 404.
+//                ServletOps.errorNotFound();
+            // Dataset URL - "exists" (even if no services) so 404 is wrong.
+            ServletOps.errorBadRequest("No endpoint for request");
+            return null; // Unreachable.
+        }
 
         // If there is one endpoint, dispatch there directly.
         Endpoint ep = epSet.getExactlyOne();
         if ( ep != null )
+            // Single dispatch, may not be valid.
             return ep;
         // No single direct dispatch. Multiple choices (different operation, same endpoint name)
         // Work out which operation we are looking for.
         Operation operation = chooseOperation(action);
         ep = epSet.get(operation);
-        // This also happens in findEndpointForOperation
-        // If a GSP-R request, try for GSP-RW
-        if ( ep == null && Operation.GSP_R.equals(operation) )
-            ep = epSet.get(Operation.GSP_RW);
-        return ep;
-    }
-
-    /**
-     *  Find an endpoint for an operation.
-     *  This searches all endpoints of a {@link DataService} that provide the {@link Operation}.
-     *  This understands that GSP_RW can service GSP_R.
-     *  Used for legacy dispatch.
-     */
-    private static Endpoint findEndpointForOperation(HttpAction action, DataService dataService, Operation operation, boolean preferUnnamed) {
-        Endpoint ep = findEndpointForOperationExact(dataService, operation, preferUnnamed);
-        if ( ep != null )
-            return ep;
-        // Try to find "R" functionality from an RW.
-        if ( GSP_R.equals(operation) )
-            return findEndpointForOperationExact(dataService, GSP_RW, preferUnnamed);
-        // Instead of 404, return 405 if asked for RW but only R available.
-        if ( GSP_RW.equals(operation) && dataService.hasOperation(GSP_R) )
-            ServletOps.errorMethodNotAllowed(action.getMethod());
-        return null;
-    }
-
-    /** Find a matching endpoint for exactly this operation.
-     * If multiple choices, prefer either named or unnamed according
-     * to the flag {@code preferUnnamed}.
-     */
-    private static Endpoint findEndpointForOperationExact(DataService dataService, Operation operation, boolean preferUnnamed) {
-        List<Endpoint> eps = dataService.getEndpoints(operation);
-        if ( eps == null || eps.isEmpty() )
-            return null;
-        // ==== Legacy compatibility.
-        // Find a named service, with preference for named/unnamed.
-        Endpoint epAlt = null;
-        for ( Endpoint ep : eps ) {
-            if ( operation.equals(ep.getOperation()) ) {
-                if ( ep.isUnnamed() && preferUnnamed )
-                    return ep;
-                if ( ! ep.isUnnamed() && ! preferUnnamed )
-                    return ep;
-                epAlt = ep;
+        if ( ep == null ) {
+            if ( GSP_R.equals(operation) )
+                // If asking for GSP_R, and GSP_RW available, pass that back.
+                ep = epSet.get(GSP_RW); // [GSP Promote]
+            else if ( GSP_RW.equals(operation) ) {
+                // If asking for GSP_RW, only GSP_R available -> 405.
+                if ( epSet.contains(GSP_R) )
+                    ServletOps.errorMethodNotAllowed(action.getMethod());
             }
         }
-        // Did not find a preferred one.
-        return epAlt;
+
+        // There are multiple endpoints; if none are suitable, then 400.
+        if ( ep == null )
+            ServletOps.errorBadRequest("No operation for request: "+action.getActionURI());
+        return ep;
     }
 
     /**
      * Identify the operation being requested.
      * It is analysing the HTTP request using global configuration.
      * The decision is based on
      * <ul>
-     * <li>Query parameters (URL query string or HTML form)</li>
-     * <li>Content-Type header</li>
-     * <li>Otherwise it is a plain REST (quads) operation.chooseOperation</li>
+     * <li>HTTP query string parameters (URL query string or HTML form)</li>
+     * <li>Registered Content-Type header</li>
+     * <li>Otherwise it is a plain REST (quads)</li>
      * </ul>
      * The HTTP Method is not considered.
      * <p>
      * The operation is not guaranteed to be supported on every {@link DataService}
      * nor that access control will allow it to be performed.
      */
-    public static Operation chooseOperation(HttpAction action) {
+    private static Operation chooseOperation(HttpAction action) {

Review comment:
       Any particular reason to lock things down to `private` ?

##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/server/Dispatcher.java
##########
@@ -144,19 +146,16 @@ private static ActionProcessor chooseProcessor(HttpAction action) {
 
         // ---- Determine Endpoint.
         String endpointName = mapRequestToEndpointName(action, dataAccessPoint);
+        // Main step of choosing the endpoint for the dispatch of the request.
+        // An endpoint is a (name, operation).
+        // There may be multiple operations for an endpointName of this data service.
 
         Endpoint endpoint = chooseEndpoint(action, dataService, endpointName);
-        if ( endpoint == null ) {
-            if ( isEmpty(endpointName) )
-                ServletOps.errorBadRequest("No operation for request: "+action.getActionURI());
-            else {
-                // No dispatch - the filter passes these through if the ActionProcessor is null.
-                return null;
-                // If this is used, resources (servlets, sttaic files) under "/dataset/" are not accessible.
-                //ServletOps.errorNotFound("No endpoint: "+action.getActionURI());
-            }
+        if ( endpoint == null )
+            // Named service, no such endpoint.
+            // Allows for resources under /dataset/
+            // Does to Jetty's default handling (404 for GET, 405 other methods).

Review comment:
       `Does` -> `Goes`




----------------------------------------------------------------
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.

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


[GitHub] [jena] kinow commented on a change in pull request #794: JENA-1960: Clean up Fuseki dispatch

Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #794:
URL: https://github.com/apache/jena/pull/794#discussion_r485947244



##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/build/FusekiConfig.java
##########
@@ -455,17 +456,54 @@ private static DataService buildDataService(Resource fusekiService, DatasetDescr
         accEndpointOldStyle(endpoints1, Operation.GSP_R,    fusekiService,  pServiceReadGraphStoreEP);
         accEndpointOldStyle(endpoints1, Operation.GSP_RW,   fusekiService,  pServiceReadWriteGraphStoreEP);
 
+        // ---- Legacy for old style: a request wouls also try the dataset (i.e. no endpoint name).
+        // If "sparql" then allow /dataset?query=
+        // Instead, for old style declarations, add new endpoints to put on the dataset
+        // Only complication is that authorization is the AND (all say "yes") of named service authorization.
+        {
+            Collection<Endpoint> endpointsCompat = oldStyleCompat(dataService, endpoints1);
+            endpointsCompat.forEach(dataService::addEndpoint);
+        }
+        endpoints1.forEach(dataService::addEndpoint);
+
         // New (2019) style
         // fuseki:endpoint [ fuseki:operation fuseki:query ; fuseki:name "" ; fuseki:allowedUsers (....) ] ;
         //   and more.
-        accFusekiEndpoints(endpoints2, fusekiService, dsDescMap);
 
-        endpoints1.forEach(dataService::addEndpoint);
+        accFusekiEndpoints(endpoints2, fusekiService, dsDescMap);
         // This will overwrite old style entries of the same fuseki:name.
         endpoints2.forEach(dataService::addEndpoint);
+
         return dataService;
     }
 
+    /**
+     *  Old style compatibility.
+     *  For each endpoint in "endpoints1", ensure there is a endpoint on the dataset 9endpoint name "") itself.

Review comment:
       "an endpoint", and s/9endpoint/(endpoint.




----------------------------------------------------------------
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.

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


[GitHub] [jena] afs merged pull request #794: JENA-1960: Clean up Fuseki dispatch

Posted by GitBox <gi...@apache.org>.
afs merged pull request #794:
URL: https://github.com/apache/jena/pull/794


   


----------------------------------------------------------------
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.

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


[GitHub] [jena] afs commented on a change in pull request #794: JENA-1960: Clean up Fuseki dispatch

Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #794:
URL: https://github.com/apache/jena/pull/794#discussion_r486207205



##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/server/Dispatcher.java
##########
@@ -203,151 +190,91 @@ private static ActionProcessor chooseProcessor(HttpAction action) {
         return processor;
     }
 
-    /**
-     * Map request to operation name.
-     * Returns the service name (the part after the "/" of the dataset part) or "".
-     */
-    protected static String mapRequestToEndpointName(HttpAction action, DataAccessPoint dataAccessPoint) {
-        return ActionLib.mapRequestToEndpointName(action, dataAccessPoint);
-    }
 
-    // Find the endpoints for an operation.
-    // This is GSP_R/GSP_RW aware.
-    // If asked for GSP_R and there are no endpoints for GSP_R, try GSP_RW.
-    private static Collection<Endpoint> getEndpoints(DataService dataService, Operation operation) {
-        Collection<Endpoint> x = dataService.getEndpoints(operation);
-        if ( x == null || x.isEmpty() ) {
-            if ( operation == GSP_R )
-                x = dataService.getEndpoints(GSP_RW);
-        }
-        return x;
-    }
 
-    /**
-     * Choose an endpoint. This can be with or without endpointName.
-     * If there is no endpoint and the action is on the data service itself (unnamed endpoint)
-     * look for a named endpoint that supplies the operation.
-     */
-    private static Endpoint chooseEndpoint(HttpAction action, DataService dataService, String endpointName) {
-        try {
-            Endpoint ep = chooseEndpointNoLegacy(action, dataService, endpointName);
-            if ( ep != null )
-                return ep;
-            // No dispatch so far.
-
-            if ( ! isEmpty(endpointName) )
-                return ep;
-            // [DISPATCH LEGACY]
-
-            // When it is a unnamed service request (operation on the dataset) and there
-            // is no match, search the named services.
-            Operation operation = chooseOperation(action);
-            // Search for an endpoint that provides the operation.
-            // No guarantee it has the access controls for the operation
-            // but in this case, access control will validate against all possible endpoints.
-            ep = findEndpointForOperation(action, dataService, operation, true);
-            return ep;
-        } catch (ActionErrorException ex) {
-            throw ex;
-        } catch (RuntimeException ex) {
-            // Example: Jetty throws BadMessageException when it is an HTML form and it is too big.
-            ServletOps.errorBadRequest(ex.getMessage());
-            return null;
-        }
-    }
+//    // Find the endpoints for an operation.
+//    // This is GSP_R/GSP_RW aware.
+//    // If asked for GSP_R and there are no endpoints for GSP_R, try GSP_RW.
+//    private static Collection<Endpoint> getEndpoints(DataService dataService, Operation operation) {
+//        Collection<Endpoint> x = dataService.getEndpoints(operation);
+//        if ( x == null || x.isEmpty() ) {
+//            if ( operation == GSP_R ) // [GSP Promote]
+//                x = dataService.getEndpoints(GSP_RW);
+//        }
+//        return x;
+//    }
 
     /**
      * Choose an endpoint.
+     * An endpoint is a name and an operation.
      * <ul>
      * <li>Look by service name to get the EndpointSet</li>
-     * <li>If empty set, return null.</li>
+     * <li>If empty set, respond with error</li>
      * <li>If there is only one choice, return that (may even be the wrong operation
-     *       - processor implmentations must be defensive).</li>
+     *       - processor implementations must be defensive).</li>
      * <li>If multiple choices, classify the operation
      *     (includes custom content-type) and look up by operation.</li>
-     * <li>Return a match wit a r
+     * <li>If not suitable, respond with error
+     * <li>Return an endpoint.
      * </ul>
+     * The endpoint chosen may not be suitable, the operation must do checking.
      */
-    private static Endpoint chooseEndpointNoLegacy(HttpAction action, DataService dataService, String endpointName) {
+    private static Endpoint chooseEndpoint(HttpAction action, DataService dataService, String endpointName) {
         EndpointSet epSet = isEmpty(endpointName) ? dataService.getEndpointSet() : dataService.getEndpointSet(endpointName);
-
-        if ( epSet == null || epSet.isEmpty() )
+        if ( epSet == null || epSet.isEmpty() ) {
             // No matches by name.
-            return null;
+            if ( ! StringUtils.isAnyEmpty(endpointName) )
+                // There was a service name, not found.
+                // But it may be a URL for static resource.
+                return null;
+//                // No service endpoint so 404.
+//                ServletOps.errorNotFound();
+            // Dataset URL - "exists" (even if no services) so 404 is wrong.
+            ServletOps.errorBadRequest("No endpoint for request");
+            return null; // Unreachable.
+        }
 
         // If there is one endpoint, dispatch there directly.
         Endpoint ep = epSet.getExactlyOne();
         if ( ep != null )
+            // Single dispatch, may not be valid.
             return ep;
         // No single direct dispatch. Multiple choices (different operation, same endpoint name)
         // Work out which operation we are looking for.
         Operation operation = chooseOperation(action);
         ep = epSet.get(operation);
-        // This also happens in findEndpointForOperation
-        // If a GSP-R request, try for GSP-RW
-        if ( ep == null && Operation.GSP_R.equals(operation) )
-            ep = epSet.get(Operation.GSP_RW);
-        return ep;
-    }
-
-    /**
-     *  Find an endpoint for an operation.
-     *  This searches all endpoints of a {@link DataService} that provide the {@link Operation}.
-     *  This understands that GSP_RW can service GSP_R.
-     *  Used for legacy dispatch.
-     */
-    private static Endpoint findEndpointForOperation(HttpAction action, DataService dataService, Operation operation, boolean preferUnnamed) {
-        Endpoint ep = findEndpointForOperationExact(dataService, operation, preferUnnamed);
-        if ( ep != null )
-            return ep;
-        // Try to find "R" functionality from an RW.
-        if ( GSP_R.equals(operation) )
-            return findEndpointForOperationExact(dataService, GSP_RW, preferUnnamed);
-        // Instead of 404, return 405 if asked for RW but only R available.
-        if ( GSP_RW.equals(operation) && dataService.hasOperation(GSP_R) )
-            ServletOps.errorMethodNotAllowed(action.getMethod());
-        return null;
-    }
-
-    /** Find a matching endpoint for exactly this operation.
-     * If multiple choices, prefer either named or unnamed according
-     * to the flag {@code preferUnnamed}.
-     */
-    private static Endpoint findEndpointForOperationExact(DataService dataService, Operation operation, boolean preferUnnamed) {
-        List<Endpoint> eps = dataService.getEndpoints(operation);
-        if ( eps == null || eps.isEmpty() )
-            return null;
-        // ==== Legacy compatibility.
-        // Find a named service, with preference for named/unnamed.
-        Endpoint epAlt = null;
-        for ( Endpoint ep : eps ) {
-            if ( operation.equals(ep.getOperation()) ) {
-                if ( ep.isUnnamed() && preferUnnamed )
-                    return ep;
-                if ( ! ep.isUnnamed() && ! preferUnnamed )
-                    return ep;
-                epAlt = ep;
+        if ( ep == null ) {
+            if ( GSP_R.equals(operation) )
+                // If asking for GSP_R, and GSP_RW available, pass that back.
+                ep = epSet.get(GSP_RW); // [GSP Promote]
+            else if ( GSP_RW.equals(operation) ) {
+                // If asking for GSP_RW, only GSP_R available -> 405.
+                if ( epSet.contains(GSP_R) )
+                    ServletOps.errorMethodNotAllowed(action.getMethod());
             }
         }
-        // Did not find a preferred one.
-        return epAlt;
+
+        // There are multiple endpoints; if none are suitable, then 400.
+        if ( ep == null )
+            ServletOps.errorBadRequest("No operation for request: "+action.getActionURI());
+        return ep;
     }
 
     /**
      * Identify the operation being requested.
      * It is analysing the HTTP request using global configuration.
      * The decision is based on
      * <ul>
-     * <li>Query parameters (URL query string or HTML form)</li>
-     * <li>Content-Type header</li>
-     * <li>Otherwise it is a plain REST (quads) operation.chooseOperation</li>
+     * <li>HTTP query string parameters (URL query string or HTML form)</li>
+     * <li>Registered Content-Type header</li>
+     * <li>Otherwise it is a plain REST (quads)</li>
      * </ul>
      * The HTTP Method is not considered.
      * <p>
      * The operation is not guaranteed to be supported on every {@link DataService}
      * nor that access control will allow it to be performed.
      */
-    public static Operation chooseOperation(HttpAction action) {
+    private static Operation chooseOperation(HttpAction action) {

Review comment:
       Only that this is an internal operation. From what I can tell, it is a left over from when dispatch was split across multiple classes.
   




----------------------------------------------------------------
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.

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


[GitHub] [jena] afs commented on pull request #794: JENA-1960: Clean up Fuseki dispatch

Posted by GitBox <gi...@apache.org>.
afs commented on pull request #794:
URL: https://github.com/apache/jena/pull/794#issuecomment-690120470


   > as far as I can follow the changes
   
   Yes! Dispatch, in all its cases and all its extensibility is complicated. It's been though a number of rewrites. The test coverage is good and that has been essential in this PR which is a simplification of the dispatch process.
   
   Thanks to you both for looking.


----------------------------------------------------------------
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.

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