[GitHub] jena pull request #463: Cleanup
Github user afs commented on a diff in the pull request: https://github.com/apache/jena/pull/463#discussion_r211266636 --- Diff: jena-arq/src/main/java/org/apache/jena/sparql/core/DynamicDatasets.java --- @@ -56,8 +55,8 @@ public static DatasetGraph dynamicDataset(DatasetDescription description, Datase if (description.isEmpty() ) return dsg; -Set defaultGraphs = NodeUtils.convertToNodes(description.getDefaultGraphURIs()) ; -Set namedGraphs = NodeUtils.convertToNodes(description.getNamedGraphURIs()) ; +Collection defaultGraphs = NodeUtils.convertToNodes(description.getDefaultGraphURIs()) ; --- End diff -- There are various places duplicate removal may happen but it is confusing and with all the routes may be depending on here. Better to maintain compatibility. PR #464 restores the explicit behaviour by having "to list" and "to set" versions and a deprecated "NodeUtils.convertToNodes" with the Set semantics. Does that look good? ---
[GitHub] jena pull request #463: Cleanup
Github user rvesse commented on a diff in the pull request: https://github.com/apache/jena/pull/463#discussion_r211191248 --- Diff: jena-arq/src/main/java/org/apache/jena/sparql/core/DynamicDatasets.java --- @@ -56,8 +55,8 @@ public static DatasetGraph dynamicDataset(DatasetDescription description, Datase if (description.isEmpty() ) return dsg; -Set defaultGraphs = NodeUtils.convertToNodes(description.getDefaultGraphURIs()) ; -Set namedGraphs = NodeUtils.convertToNodes(description.getNamedGraphURIs()) ; +Collection defaultGraphs = NodeUtils.convertToNodes(description.getDefaultGraphURIs()) ; --- End diff -- Isn't the point of using sets here to remove duplicates or is that now done in the `NodeUtils.convertToNodes()` method? ---
[GitHub] jena pull request #463: Cleanup
Github user asfgit closed the pull request at: https://github.com/apache/jena/pull/463 ---
[GitHub] jena pull request #463: Cleanup
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/463#discussion_r210921706
--- Diff:
jena-fuseki2/jena-fuseki-embedded/src/main/java/org/apache/jena/fuseki/embedded/FusekiServer.java
---
@@ -455,30 +527,36 @@ private static void
setMimeTypes(ServletContextHandler context) {
private void servlets(ServletContextHandler context) {
// Fuseki dataset services filter
+// This goes as the filter at the end of any filter chaining.
FusekiFilter ff = new FusekiFilter();
-FilterHolder h = new FilterHolder(ff);
-context.addFilter(h, "/*", null);
-
-other.forEach(p->addServlet(context, p.getLeft(),
p.getRight()));
+addFilter(context, "/*", ff);
if ( withStats )
addServlet(context, "/$/stats", new ActionStats());
if ( withPing )
addServlet(context, "/$/ping", new ActionPing());
+servlets.forEach(p->addServlet(context, p.getLeft(),
p.getRight()));
+filters.forEach (p-> addFilter(context, p.getLeft(),
p.getRight()));
+
if ( staticContentDir != null ) {
DefaultServlet staticServlet = new DefaultServlet();
ServletHolder staticContent = new
ServletHolder(staticServlet);
staticContent.setInitParameter("resourceBase",
staticContentDir);
context.addServlet(staticContent, "/");
}
}
-
+
--- End diff --
These leading space arise in Eclipse. After a closing "}" the cursor is
padded to the same depth as the "}". Is there a way to change that without
using a save action?
---
[GitHub] jena pull request #463: Cleanup
Github user kinow commented on a diff in the pull request:
https://github.com/apache/jena/pull/463#discussion_r210896418
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/lib/ListUtils.java
---
@@ -34,49 +34,40 @@
{
private ListUtils() {}
-public static
-List unique(List list)
-{
- return toList(list.stream().distinct()) ;
+public static List unique(List list) {
+return toList(list.stream().distinct());
}
-
-public static
-List asList(int... values)
-{
-List x = new ArrayList<>(values.length) ;
+
+public static List asList(int...values) {
--- End diff --
Oh, my mistake :) :+1:
---
[GitHub] jena pull request #463: Cleanup
Github user kinow commented on a diff in the pull request:
https://github.com/apache/jena/pull/463#discussion_r210893804
--- Diff:
jena-fuseki2/jena-fuseki-embedded/src/main/java/org/apache/jena/fuseki/embedded/FusekiServer.java
---
@@ -455,30 +527,36 @@ private static void
setMimeTypes(ServletContextHandler context) {
private void servlets(ServletContextHandler context) {
// Fuseki dataset services filter
+// This goes as the filter at the end of any filter chaining.
FusekiFilter ff = new FusekiFilter();
-FilterHolder h = new FilterHolder(ff);
-context.addFilter(h, "/*", null);
-
-other.forEach(p->addServlet(context, p.getLeft(),
p.getRight()));
+addFilter(context, "/*", ff);
if ( withStats )
addServlet(context, "/$/stats", new ActionStats());
if ( withPing )
addServlet(context, "/$/ping", new ActionPing());
+servlets.forEach(p->addServlet(context, p.getLeft(),
p.getRight()));
+filters.forEach (p-> addFilter(context, p.getLeft(),
p.getRight()));
+
if ( staticContentDir != null ) {
DefaultServlet staticServlet = new DefaultServlet();
ServletHolder staticContent = new
ServletHolder(staticServlet);
staticContent.setInitParameter("resourceBase",
staticContentDir);
context.addServlet(staticContent, "/");
}
}
-
+
--- End diff --
>simply turning on the save action to remove trailing spaces is likely to
cause such formatting changes on files with functional changes.
Agreed, I used the save action in Eclipse for a while and regretted later.
>Previously, keeping formatting changes separate from functional changes
has been requested. (I'm neutral.)
I'm ok with either too. But my point here was that this case wasn't like
the save action... at least from looking at the github diff, it appears there
was no spaces before, and these were added in this commit only. But as I said,
nit-pick. And nit-picks like this one can cause distraction, so feel free to
ignore or hide :-)
---
[GitHub] jena pull request #463: Cleanup
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/463#discussion_r210891477
--- Diff:
jena-fuseki2/jena-fuseki-embedded/src/main/java/org/apache/jena/fuseki/embedded/FusekiServer.java
---
@@ -455,30 +527,36 @@ private static void
setMimeTypes(ServletContextHandler context) {
private void servlets(ServletContextHandler context) {
// Fuseki dataset services filter
+// This goes as the filter at the end of any filter chaining.
FusekiFilter ff = new FusekiFilter();
-FilterHolder h = new FilterHolder(ff);
-context.addFilter(h, "/*", null);
-
-other.forEach(p->addServlet(context, p.getLeft(),
p.getRight()));
+addFilter(context, "/*", ff);
if ( withStats )
addServlet(context, "/$/stats", new ActionStats());
if ( withPing )
addServlet(context, "/$/ping", new ActionPing());
+servlets.forEach(p->addServlet(context, p.getLeft(),
p.getRight()));
+filters.forEach (p-> addFilter(context, p.getLeft(),
p.getRight()));
+
if ( staticContentDir != null ) {
DefaultServlet staticServlet = new DefaultServlet();
ServletHolder staticContent = new
ServletHolder(staticServlet);
staticContent.setInitParameter("resourceBase",
staticContentDir);
context.addServlet(staticContent, "/");
}
}
-
+
--- End diff --
There is a tradeoff here - simply turning on the save action to remove
trailing spaces is likely to cause such formatting changes on files with
functional changes. Previously, keeping formatting changes separate from
functional changes has been requested. (I'm neutral.)
---
[GitHub] jena pull request #463: Cleanup
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/463#discussion_r210889580
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/lib/ListUtils.java
---
@@ -34,49 +34,40 @@
{
private ListUtils() {}
-public static
-List unique(List list)
-{
- return toList(list.stream().distinct()) ;
+public static List unique(List list) {
+return toList(list.stream().distinct());
}
-
-public static
-List asList(int... values)
-{
-List x = new ArrayList<>(values.length) ;
+
+public static List asList(int...values) {
--- End diff --
The signature of `Arrays.asList` is `List asList(T... a)` and
auto-boxing does not work when the argument is `int[]` or `int ...`).
---
[GitHub] jena pull request #463: Cleanup
Github user kinow commented on a diff in the pull request:
https://github.com/apache/jena/pull/463#discussion_r210856558
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/lib/ListUtils.java
---
@@ -34,49 +34,40 @@
{
private ListUtils() {}
-public static
-List unique(List list)
-{
- return toList(list.stream().distinct()) ;
+public static List unique(List list) {
+return toList(list.stream().distinct());
}
-
-public static
-List asList(int... values)
-{
-List x = new ArrayList<>(values.length) ;
+
+public static List asList(int...values) {
--- End diff --
Couldn't we replace this method by something like `Arrays.asList(1, 2, 3);`
? Or maybe deprecate it?
---
[GitHub] jena pull request #463: Cleanup
Github user kinow commented on a diff in the pull request: https://github.com/apache/jena/pull/463#discussion_r210855762 --- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/NodeUtils.java --- @@ -18,12 +18,9 @@ package org.apache.jena.sparql.util; -import java.util.Collection ; -import java.util.HashSet ; -import java.util.Iterator ; -import java.util.Objects; -import java.util.Set ; +import java.util.*; --- End diff -- Isn't it a bit better to keep what we have in import statements, expanding to what's used instead of importing the whole package? ---
[GitHub] jena pull request #463: Cleanup
Github user kinow commented on a diff in the pull request:
https://github.com/apache/jena/pull/463#discussion_r210855919
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/NodeUtils.java
---
@@ -105,15 +102,23 @@ public String accept(Node x) {
Iterator conv = new MapFilterIterator<>(mapper, eIter) ;
return conv ;
}
-
-/** Convert IRI String to Node */
-public static Set convertToNodes(Collection uris) {
-Set nodes = new HashSet<>() ;
-for ( String x : uris )
-nodes.add(NodeFactory.createURI(x)) ;
-return nodes ;
+
--- End diff --
Nit-pick! Some spaces were left behind.
---
[GitHub] jena pull request #463: Cleanup
Github user kinow commented on a diff in the pull request:
https://github.com/apache/jena/pull/463#discussion_r210857578
--- Diff:
jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ServiceRouter.java
---
@@ -120,10 +120,12 @@ protected boolean allowQuads_RW(HttpAction action) {
return isEnabled(action, Operation.Quads_RW);
}
-// Test whether there is a configuration that allows this action
as the operation
-// given.
-// Ignores the operation in the action (set due to parsing - it
might be "quads"
-// which is the generic operation when just the dataset is
specificed.
+/**
+ * Test whether there is a configuration that allows this action
as the operation
+ * given. Ignores the operation in the action which is set due to
parsing - it
+ * might be "quads" which is the generic operation when just the
dataset is
+ * specificed.
--- End diff --
s/specificed/specified
---
[GitHub] jena pull request #463: Cleanup
Github user kinow commented on a diff in the pull request: https://github.com/apache/jena/pull/463#discussion_r210857731 --- Diff: jena-fuseki2/jena-fuseki-embedded/src/main/java/org/apache/jena/fuseki/embedded/FusekiServer.java --- @@ -20,10 +20,9 @@ import static java.util.Objects.requireNonNull; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; +import java.util.*; --- End diff -- Ditto first comment, about maintaining the current standard in the code (I think) ---
[GitHub] jena pull request #463: Cleanup
Github user kinow commented on a diff in the pull request:
https://github.com/apache/jena/pull/463#discussion_r210857986
--- Diff:
jena-fuseki2/jena-fuseki-embedded/src/main/java/org/apache/jena/fuseki/embedded/FusekiServer.java
---
@@ -455,30 +527,36 @@ private static void
setMimeTypes(ServletContextHandler context) {
private void servlets(ServletContextHandler context) {
// Fuseki dataset services filter
+// This goes as the filter at the end of any filter chaining.
FusekiFilter ff = new FusekiFilter();
-FilterHolder h = new FilterHolder(ff);
-context.addFilter(h, "/*", null);
-
-other.forEach(p->addServlet(context, p.getLeft(),
p.getRight()));
+addFilter(context, "/*", ff);
if ( withStats )
addServlet(context, "/$/stats", new ActionStats());
if ( withPing )
addServlet(context, "/$/ping", new ActionPing());
+servlets.forEach(p->addServlet(context, p.getLeft(),
p.getRight()));
+filters.forEach (p-> addFilter(context, p.getLeft(),
p.getRight()));
+
if ( staticContentDir != null ) {
DefaultServlet staticServlet = new DefaultServlet();
ServletHolder staticContent = new
ServletHolder(staticServlet);
staticContent.setInitParameter("resourceBase",
staticContentDir);
context.addServlet(staticContent, "/");
}
}
-
+
--- End diff --
Nit-picking again! Unnecessary spaces...
---
[GitHub] jena pull request #463: Cleanup
GitHub user afs opened a pull request: https://github.com/apache/jena/pull/463 Cleanup This is a collection of small items, accumulated while working with the source code attached to a project. * Various comments/javadoc added and fixed and other code cleanup. * make subclassing of the Fuseki query servlet better * building Fuseki embedded using the command line syntax * deprecate TDB ops "isBackedBy" in favour of "isTDB" for better naming. I've left it as each individual commit for the details. You can merge this pull request into a Git repository by running: $ git pull https://github.com/afs/jena cleanup Alternatively you can review and apply these changes as the patch at: https://github.com/apache/jena/pull/463.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #463 commit 8d0c1310a89c115553889208c97cbdfdfb1958b2 Author: Andy Seaborne Date: 2018-08-14T12:02:38Z Typos and comments. commit d061885fae1ca4ba86e0603e7b7a45d21c739802 Author: Andy Seaborne Date: 2018-08-14T12:12:23Z Expose the command line argument processing to build a FusekiServer. commit 0335b26f11825843f17053501a07c185f28571a5 Author: Andy Seaborne Date: 2018-08-14T12:13:46Z Deprecate isBackedByTDB in favour of isTDB2. commit d576e607e6f578a57f7049a4e2dc6ad81d707ebc Author: Andy Seaborne Date: 2018-08-15T14:42:48Z Deprecate isBackedByTDB in favour of isTDB1 commit 833cdb686b6035fd63afbfb0b92bfe7f9c77e493 Author: Andy Seaborne Date: 2018-08-15T15:54:56Z Fix javadoc. commit 8206b7c6cd57868a6aba4021647c10b221986cc1 Author: Andy Seaborne Date: 2018-08-15T15:55:53Z Provision for custom parameters so request validation does not WARN. commit caf572a1081bb4ab84337c7553ff0181a5052c8b Author: Andy Seaborne Date: 2018-08-16T08:56:25Z Convert to single format. commit cb6d9b5c63e0cfcb5bb600c4e5af9948c77b41ec Author: Andy Seaborne Date: 2018-08-16T08:56:48Z Strings to Node_URIs. commit 21050fc9300cc29eda22744c35466aae9f5b01c0 Author: Andy Seaborne Date: 2018-08-16T11:03:08Z Put attribute names constants in Fuseki. commit f6bac07b64c500472091efb7a83ca486b89e321e Author: Andy Seaborne Date: 2018-08-16T11:03:59Z Switch naming of setters to non-"set" style. commit 99180e5831bb8d0ff94c0e98fd3a375f02c20af5 Author: Andy Seaborne Date: 2018-08-16T15:11:59Z Support for ServletFilters. commit ae74347f3607427e88561c854f26903cc880f310 Author: Andy Seaborne Date: 2018-08-16T15:12:44Z Comments and internal rename. commit aa3518ee48d53a4bcd4d63c66b36cb8e0f3a93bd Author: Andy Seaborne Date: 2018-08-16T15:13:05Z Remove unused and out-of-date class. commit 9dace245d39a7c1a977fa34e423502c5ed39afe9 Author: Andy Seaborne Date: 2018-08-16T22:21:49Z Better attribute naming - org.apache.jena.fuseki: commit 3b153d526d18626e6ad3ea4414d1dc8f138213ba Author: Andy Seaborne Date: 2018-08-16T22:22:07Z Typos ---
