[GitHub] [jena] kinow commented on a diff in pull request #1960: Fuseki improvements for access control, servlet context, and configurations with shared datasets.

["kinow (via GitHub)" <gi...@apache.org>]

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


##########
jena-arq/src/main/java/org/apache/jena/query/DatasetFactory.java:
##########
@@ -254,16 +254,48 @@ public static Dataset assemble(String filename, String resourceURI) {
 	}
 
 	/**
-	 * Assemble a dataset from the model
+     * Assemble a dataset from a specific resource in a model.
+     *
+     * @param model The Model
+     * @param resourceURI The URI for the the dataset in the assembler model.
+     * @return Dataset
+     */
+	public static Dataset assemble(Model model, String resourceURI) {
+        Objects.requireNonNull(model, "model can not be null") ;
+        Objects.requireNonNull(resourceURI, "resourceURI can not be null") ;
+        Resource r = model.createResource(resourceURI);
+        return assemble(r);
+	}

Review Comment:
   The indenting appears to be wrong looking at the GitHub UI diff. Probably tabs & spaces used in the same file, so no need to change I think.



##########
jena-fuseki2/jena-fuseki-main/src/main/java/org/apache/jena/fuseki/main/FusekiServer.java:
##########
@@ -867,6 +874,25 @@ private static boolean argBoolean(Resource r, Property p, boolean dftValue) {
             }
         }
 
+        private static String argString(Resource r, Property p, String dftValue) {
+            try { GraphUtils.atmostOneProperty(r, p); }
+            catch (NotUniqueException ex) {
+                throw new FusekiConfigException(ex.getMessage());

Review Comment:
   Is it worth to include the `ex` in the `FusekiConfigException`, to have more context in the error?



##########
jena-fuseki2/jena-fuseki-main/src/main/java/org/apache/jena/fuseki/main/FusekiServer.java:
##########
@@ -867,6 +874,25 @@ private static boolean argBoolean(Resource r, Property p, boolean dftValue) {
             }
         }
 
+        private static String argString(Resource r, Property p, String dftValue) {
+            try { GraphUtils.atmostOneProperty(r, p); }
+            catch (NotUniqueException ex) {
+                throw new FusekiConfigException(ex.getMessage());
+            }
+            Statement stmt = r.getProperty(p);
+            if ( stmt == null )
+                return dftValue;
+            try {
+                Node n = stmt.getObject().asLiteral().asNode();
+                if ( ! G.isString(n) )
+                    throw new FusekiConfigException("Not a string for '"+p+"' : "+stmt.getObject());
+                return n.getLiteralLexicalForm();
+            } catch (JenaException ex) {
+                throw new FusekiConfigException("Not a string for '"+p+"' : "+stmt.getObject());

Review Comment:
   Ditto here



##########
jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/build/FusekiConfig.java:
##########
@@ -635,19 +637,24 @@ private static void accEndpoint(Collection<Endpoint> endpoints, Operation operat
 
     public static Dataset getDataset(Resource datasetDesc, DatasetDescriptionMap dsDescMap) {
         // check if this one already built
+        // This is absolute and does not require on NamedDatasetAssembler;

Review Comment:
   s/on/the ? s/on/one? or s/does not require/is not required?



##########
jena-fuseki2/jena-fuseki-main/src/test/java/org/apache/jena/fuseki/main/access/TestAuthorized.java:
##########
@@ -53,8 +55,28 @@ public class TestAuthorized {
         assertTrue(auth.isAllowed("user1"));
     }
 
+    @Test public void auth_anyLoggedIn_3() {
+        AuthPolicy auth = Auth.policyAllowSpecific("!");
+        assertFalse(auth.isAllowed(null));
+        assertFalse(auth.isAllowed("user1"));
+    }
+
+    @Test public void auth_anyLoggedIn_4() {
+        String level = LogCtl.getLevel(Fuseki.configLog);
+        try {
+            LogCtl.set(Fuseki.configLog, "Error");
+            // "!" anbd anythign else causes a warning.

Review Comment:
   s/anbd/and



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