kinow commented on code in PR #2359:
URL: https://github.com/apache/jena/pull/2359#discussion_r1529286253


##########
jena-extras/jena-querybuilder/src/main/java/org/apache/jena/arq/querybuilder/handlers/DatasetHandler.java:
##########
@@ -54,24 +57,42 @@ public void addAll(DatasetHandler datasetHandler) {
     }
 
     /**
-     * Add a graph name to the from named list.
-     *
-     * @param graphName The graph name to add.
+     * Converts an object into a graph name.
+     * @param graphName the object that represents the graph name.
+     * @return the string that is the graph name.
      */
-    public void fromNamed(String graphName) {
-        query.addNamedGraphURI(graphName);
+    String asGraphName(Object graphName) {
+        // package private for testing access
+        if (graphName instanceof String) {
+            return (String) graphName;
+        }
+        if (graphName instanceof FrontsNode) {
+            return asGraphName(((FrontsNode)graphName).asNode());
+        }
+        if (graphName instanceof Node_URI) {
+            return ((Node_URI)graphName).getURI();
+        }
+        if (graphName instanceof Node_Literal) {
+            return asGraphName(((Node_Literal)graphName).getLiteralValue());
+        }
+
+        return graphName.toString();
     }
 
     /**
-     * Add the graph names to the from named list.
-     *
-     * The names are ordered in as defined in the collection.
+     * Add a graph name to the from named list.
      *
-     * @param graphNames The from names to add.
+     * @param graphName The graph name to add.
      */
-    public void fromNamed(Collection<String> graphNames) {
-        for (String uri : graphNames) {
-            query.addNamedGraphURI(uri);
+    public void fromNamed(Object graphName) {
+        if (graphName instanceof Collection) {
+            ((Collection<?>)graphName).forEach(this::fromNamed);

Review Comment:
   Couldn't this be used to create one of those request bombs that keep 
iterating until it causes a StackOverflowError? That's probably fine since 
external users won't have access to this methods, I think, only application 
developers... but looks a bit dangerous (maybe worth adding a note in the docs 
that it's a recursive method... or replace the recursion by a stack+while, 
etc...). Same the `from` method.



##########
jena-extras/jena-querybuilder/src/main/java/org/apache/jena/arq/querybuilder/UpdateBuilder.java:
##########
@@ -122,6 +122,14 @@ private ExtendedIterator<Quad> 
getQuads(Collection<QuadHolder> holders) {
         return result;
     }
 
+    /**
+     * Checks that deletes or inserts have been added to the builder.
+     * @return true if there are any delete or insert statements.

Review Comment:
   Returns true if there are no deletes and no inserts? Because the return is 
checking `isEmpty` for both.... 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to