You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@jena.apache.org by "afs (via GitHub)" <gi...@apache.org> on 2023/10/22 19:24:53 UTC
[PR] Fix IRIs.asBase [jena]
afs opened a new pull request, #2059:
URL: https://github.com/apache/jena/pull/2059
Better fix for 41acb38a34
There are two problems to be fixed, not one!
--
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
Re: [PR] Fix IRIs.asBase [jena]
Posted by "afs (via GitHub)" <gi...@apache.org>.
afs commented on code in PR #2059:
URL: https://github.com/apache/jena/pull/2059#discussion_r1369806112
##########
jena-fuseki2/jena-fuseki-geosparql/src/main/java/org/apache/jena/ext/io/github/galbiston/rdf_tables/file/FileConverter.java:
##########
@@ -101,6 +101,7 @@ public static void writeToModel(File inputFile, Model model, char delimiter, Boo
} catch (IOException | RuntimeException | CsvValidationException ex) {
LOGGER.error("FileConverter: Line - {}, File - {}, Exception - {}", lineNumber, inputFile.getAbsolutePath(), ex.getMessage());
+ ex.printStackTrace();
Review Comment:
Good point. Moved the exception into the `AssertionError`. Previously, it was hiding the information.
--
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
Re: [PR] Fix IRIs.asBase [jena]
Posted by "afs (via GitHub)" <gi...@apache.org>.
afs merged PR #2059:
URL: https://github.com/apache/jena/pull/2059
--
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
Re: [PR] Fix IRIs.asBase [jena]
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on code in PR #2059:
URL: https://github.com/apache/jena/pull/2059#discussion_r1369319707
##########
jena-core/src/main/java/org/apache/jena/irix/IRIs.java:
##########
@@ -76,24 +76,32 @@ public static String getBaseStr() {
/**
* Given a candidate baseURI string, which may be a filename,
* turn it into a IRI suitable as a base IRI.
+ * This includes encoding characters in a filename (e.g. spaces).
*/
- public static String toBase(String baseURI) {
- if ( baseURI == null )
+ public static String toBase(String uriForBase) {
+ if ( uriForBase == null )
return getBaseStr();
- String scheme = scheme(baseURI);
+ String scheme = scheme(uriForBase);
if ( Sys.isWindows ) {
// Assume a scheme of one letter is a Windows drive letter.
if ( scheme != null && scheme.length() == 1 )
scheme = "file";
}
- // File scheme if the URI is "file:" or there is no scheme
- // and the system base is a "file:" URI
- boolean isFile = ( scheme != null )
- ? scheme.equals("file")
- : IRIs.getSystemBase().hasScheme("file");
- if ( isFile )
- baseURI = IRILib.encodeFileURL(baseURI);
- return IRIs.getSystemBase().resolve(baseURI).toString();
+ if ( scheme == null ) {
+ // Relative name: it the base is a file: URI, encode the relative
+ // name if it does not look like it is already encoded.
+ boolean isFileBase = ( scheme == null ) && IRIs.getSystemBase().hasScheme("file");
Review Comment:
This `( scheme == null )` will always be `true`, because of the `if ( scheme == null )` above.
##########
jena-fuseki2/jena-fuseki-geosparql/src/main/java/org/apache/jena/ext/io/github/galbiston/rdf_tables/file/FileConverter.java:
##########
@@ -101,6 +101,7 @@ public static void writeToModel(File inputFile, Model model, char delimiter, Boo
} catch (IOException | RuntimeException | CsvValidationException ex) {
LOGGER.error("FileConverter: Line - {}, File - {}, Exception - {}", lineNumber, inputFile.getAbsolutePath(), ex.getMessage());
+ ex.printStackTrace();
Review Comment:
Currently debugging it? Or if intended to leave in the code maybe better to be wrapped in another exception thrown?
--
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
Re: [PR] Fix IRIs.asBase [jena]
Posted by "afs (via GitHub)" <gi...@apache.org>.
afs commented on PR #2059:
URL: https://github.com/apache/jena/pull/2059#issuecomment-1776082479
This makes the test suite run when the current directory, or a potential base URI, has a space in it.
There is an unrelated issue with jena-fuseki-geosparql; resource URLs return a string space in them that do not the original resource with a space in its name.
--
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
Re: [PR] Fix IRIs.asBase [jena]
Posted by "afs (via GitHub)" <gi...@apache.org>.
afs commented on PR #2059:
URL: https://github.com/apache/jena/pull/2059#issuecomment-1774192963
Good catch.
The Jenkins jobs are showing some double encoding going on which I haven't been able to produce locally.
There are two problems - but seems I've only found one of them :-)
--
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
Re: [PR] Fix IRIs.asBase [jena]
Posted by "afs (via GitHub)" <gi...@apache.org>.
afs commented on code in PR #2059:
URL: https://github.com/apache/jena/pull/2059#discussion_r1369801256
##########
jena-core/src/main/java/org/apache/jena/irix/IRIs.java:
##########
@@ -76,24 +76,32 @@ public static String getBaseStr() {
/**
* Given a candidate baseURI string, which may be a filename,
* turn it into a IRI suitable as a base IRI.
+ * This includes encoding characters in a filename (e.g. spaces).
*/
- public static String toBase(String baseURI) {
- if ( baseURI == null )
+ public static String toBase(String uriForBase) {
+ if ( uriForBase == null )
return getBaseStr();
- String scheme = scheme(baseURI);
+ String scheme = scheme(uriForBase);
if ( Sys.isWindows ) {
// Assume a scheme of one letter is a Windows drive letter.
if ( scheme != null && scheme.length() == 1 )
scheme = "file";
}
- // File scheme if the URI is "file:" or there is no scheme
- // and the system base is a "file:" URI
- boolean isFile = ( scheme != null )
- ? scheme.equals("file")
- : IRIs.getSystemBase().hasScheme("file");
- if ( isFile )
- baseURI = IRILib.encodeFileURL(baseURI);
- return IRIs.getSystemBase().resolve(baseURI).toString();
+ if ( scheme == null ) {
+ // Relative name: it the base is a file: URI, encode the relative
+ // name if it does not look like it is already encoded.
+ boolean isFileBase = ( scheme == null ) && IRIs.getSystemBase().hasScheme("file");
Review Comment:
Fixed!
--
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