GH-1409: Unnecessary parsing of IRIs every time an RDF parser is used #1410
Conversation
…arser is used Signed-off-by: Pavel Mihaylov <pavel@ontotext.com>
|
@rdf4j-bot whitelist |
|
@rdf4j-bot run tests |
|
Thanks for the fix @avataar. It appears a regression test is failing. There may be a corner case overlooked here. |
…arser is used (fixed failing test) Signed-off-by: Pavel Mihaylov <pavel@ontotext.com>
…arser is used (fixed formatting from prev commit) Signed-off-by: Pavel Mihaylov <pavel@ontotext.com>
| @@ -440,8 +440,7 @@ public void test780IRISpace() throws Exception { | |||
| assertEquals(1, model.size()); | |||
| model.filter(null, RDF.TYPE, null) | |||
| .objects() | |||
| .forEach(obj -> assertEquals("http://purl.bioontology.org/ontology/UATC/%20SERINE%20%20", | |||
| obj.stringValue())); | |||
| .forEach(obj -> assertEquals("http://purl.bioontology.org/ontology/UATC/ SERINE ", obj.stringValue())); | |||
There was a problem hiding this comment.
This changes the test case to conform to the changed behaviour, and will result in syntactically illegal IRIs being produced. I know that this only occurs when VERIFY_URI_SYNTAX has been disabled, but looking at the original fix that introduced this test case (#780, see PR #839 ) the %-encoding of encoded spaces when syntax validation is disabled was a deliberate feature. Are you quite sure you're correct to modify this test?
There was a problem hiding this comment.
Looking a bit further, the original issue complained about the round-trip: the writer serializing such IRIs with the spaces un-encoded. I've just verified that the TurtleWriter has been fixed to deal with that. So I guess the motivation for this test case (and the behavior that it tested) has changed, and you may be right that this is the correct behavior.
Signed-off-by: Pavel Mihaylov pavel@ontotext.com
This PR addresses GitHub issue: #1409
Also fixes an issue in AbstractRDFParser.createURI() where non-fatal validation of IRIs didn't return null (this was masked by old code in resolveURI()).