From 42d2ec8119b66aa655f2448be2de5101c471f96b Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Tue, 19 Nov 2024 16:52:34 +0100 Subject: [PATCH] [Bug #275] Handle comments when parsing SPARQL queries. --- .../jopa/query/sparql/SparqlQueryParser.java | 86 +++++--- .../query/sparql/SparqlQueryParserTest.java | 184 ++++++++++++------ 2 files changed, 192 insertions(+), 78 deletions(-) diff --git a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/query/sparql/SparqlQueryParser.java b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/query/sparql/SparqlQueryParser.java index 4f82bfa65..5e21951fc 100644 --- a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/query/sparql/SparqlQueryParser.java +++ b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/query/sparql/SparqlQueryParser.java @@ -54,6 +54,8 @@ public class SparqlQueryParser implements QueryParser { private ParamType currentParamType; private StringBuilder currentWord; private boolean inProjection; + private boolean inComment; + private boolean inUri; public SparqlQueryParser(ParameterValueFactory parameterValueFactory) { this.parameterValueFactory = parameterValueFactory; @@ -67,45 +69,57 @@ private enum ParamType { @Override public SparqlQueryHolder parseQuery(String query) { this.query = query; - this.queryParts = new ArrayList<>(); - this.uniqueParams = new HashMap<>(); - this.positionalCounter = 1; - this.parameters = new ArrayList<>(); - this.inSQString = false; - // In double-quoted string - this.inDQString = false; - this.inParam = false; - this.lastParamEndIndex = 0; - this.paramStartIndex = 0; - this.currentParamType = null; - this.currentWord = new StringBuilder(); + resetParser(); int i; for (i = 0; i < query.length(); i++) { final char c = query.charAt(i); switch (c) { + case '#': + startComment(i, c); + break; case '\'': - inSQString = !inSQString; + // inComment ? inSQString : !inSQString + inSQString = inComment == inSQString; break; case '"': - inDQString = !inDQString; + // inComment ? inDQString : !inDQString + inDQString = inComment == inDQString; break; case '$': - parameterStart(i, ParamType.POSITIONAL); + if (!inComment) { + parameterStart(i, ParamType.POSITIONAL); + } break; case '?': - if (inParam) { - parameterEnd(i); // Property path zero or one - } else { - parameterStart(i, ParamType.NAMED); + if (!inComment) { + if (inParam) { + parameterEnd(i); // Property path zero or one + } else { + parameterStart(i, ParamType.NAMED); + } } break; - case '{': - this.inProjection = false; // Intentional fall-through case '<': + this.inUri = true; + if (inParam) { + parameterEnd(i); + } + wordEnd(); + break; case '>': - case ',': + this.inUri = false; + if (inParam) { + parameterEnd(i); + } + wordEnd(); + break; case '\n': + this.inComment = false; // Intentional fall-through + case '{': + // inComment ? inProjection : !inProjection + this.inProjection = inComment && inProjection; // Intentional fall-through case '\r': + case ',': case ')': case ' ': case '.': @@ -135,6 +149,34 @@ public SparqlQueryHolder parseQuery(String query) { return new SparqlQueryHolder(query, queryParts, parameters); } + private void resetParser() { + this.queryParts = new ArrayList<>(); + this.uniqueParams = new HashMap<>(); + this.positionalCounter = 1; + this.parameters = new ArrayList<>(); + this.inSQString = false; + // In double-quoted string + this.inDQString = false; + this.inParam = false; + this.inUri = false; + this.inComment = false; + this.lastParamEndIndex = 0; + this.paramStartIndex = 0; + this.currentParamType = null; + this.currentWord = new StringBuilder(); + } + + private void startComment(int index, char c) { + if (!inUri) { + if (inParam && !inComment) { + parameterEnd(index); + } + inComment = true; + } else { + currentWord.append(c); + } + } + private void parameterStart(int index, ParamType paramType) { if (!inSQString && !inDQString) { queryParts.add(query.substring(lastParamEndIndex, index)); diff --git a/jopa-impl/src/test/java/cz/cvut/kbss/jopa/query/sparql/SparqlQueryParserTest.java b/jopa-impl/src/test/java/cz/cvut/kbss/jopa/query/sparql/SparqlQueryParserTest.java index 4695cd925..1cb1a2670 100644 --- a/jopa-impl/src/test/java/cz/cvut/kbss/jopa/query/sparql/SparqlQueryParserTest.java +++ b/jopa-impl/src/test/java/cz/cvut/kbss/jopa/query/sparql/SparqlQueryParserTest.java @@ -70,12 +70,12 @@ public void testParseQueryWithThreeParametersInSingleBGP() { @Test public void testParseQueryWithMultiplePatternsAndParameters() { - final String query = "PREFIX foaf: \n" + - "SELECT ?craft\n" + - "{\n" + - "?craft foaf:name \"Apollo 7\" .\n" + - "?craft foaf:homepage ?homepage\n" + - "}"; + final String query = """ + PREFIX foaf: + SELECT ?craft { + ?craft foaf:name "Apollo 7" . + ?craft foaf:homepage ?homepage + }"""; final QueryHolder holder = queryParser.parseQuery(query); assertEquals(2, holder.getParameters().size()); assertTrue(holder.getParameters().contains(new QueryParameter<>("craft", valueFactory))); @@ -84,11 +84,12 @@ public void testParseQueryWithMultiplePatternsAndParameters() { @Test public void testParseQueryWithFilterAndRegex() { - final String query = "PREFIX dc: \n" + - "SELECT ?title\n" + - "WHERE { ?x dc:title ?title\n" + - " FILTER regex(?title, \"^SPARQL\") \n" + - " }"; + final String query = """ + PREFIX dc: + SELECT ?title + WHERE { ?x dc:title ?title + FILTER regex(?title, "^SPARQL")\s + }"""; final QueryHolder holder = queryParser.parseQuery(query); assertEquals(2, holder.getParameters().size()); assertTrue(holder.getParameters().contains(new QueryParameter<>("title", valueFactory))); @@ -97,12 +98,13 @@ public void testParseQueryWithFilterAndRegex() { @Test public void testParseQueryWithFilterAndMultipleParameters() { - final String query = "PREFIX dc: \n" + - "PREFIX ns: \n" + - "SELECT ?title ?price\n" + - "WHERE { ?x ns:price ?price .\n" + - " FILTER (?price < 30.5)\n" + - " ?x dc:title ?title . }"; + final String query = """ + PREFIX dc: + PREFIX ns: + SELECT ?title ?price + WHERE { ?x ns:price ?price . + FILTER (?price < 30.5) + ?x dc:title ?title . }"""; final QueryHolder holder = queryParser.parseQuery(query); assertEquals(3, holder.getParameters().size()); assertTrue(holder.getParameters().contains(new QueryParameter<>("title", valueFactory))); @@ -112,11 +114,12 @@ public void testParseQueryWithFilterAndMultipleParameters() { @Test public void testParseConstructQuery() { - final String query = "PREFIX foaf: \n" + - "PREFIX org: \n" + - "\n" + - "CONSTRUCT { ?x foaf:name ?name }\n" + - "WHERE { ?x org:employeeName ?name }"; + final String query = """ + PREFIX foaf: + PREFIX org: + + CONSTRUCT { ?x foaf:name ?name } + WHERE { ?x org:employeeName ?name }"""; final QueryHolder holder = queryParser.parseQuery(query); assertEquals(2, holder.getParameters().size()); assertTrue(holder.getParameters().contains(new QueryParameter<>("x", valueFactory))); @@ -131,12 +134,13 @@ public void parserThrowsExceptionWhenParameterNameIsMissing() { @Test public void testParseQueryWithNumberedPositionalParams() { - final String query = "PREFIX foaf: \n" + - "SELECT ?craft\n" + - "{\n" + - "?craft foaf:name $1 .\n" + - "?craft foaf:homepage $2\n" + - "}"; + final String query = """ + PREFIX foaf: + SELECT ?craft + { + ?craft foaf:name $1 . + ?craft foaf:homepage $2 + }"""; final QueryHolder holder = queryParser.parseQuery(query); assertEquals(3, holder.getParameters().size()); assertTrue(holder.getParameters().contains(new QueryParameter<>("craft", valueFactory))); @@ -146,12 +150,12 @@ public void testParseQueryWithNumberedPositionalParams() { @Test public void testParseQueryWithUnnumberedPositionalParams() { - final String query = "PREFIX foaf: \n" + - "SELECT ?craft\n" + - "{\n" + - "?craft foaf:name $ .\n" + - "?craft foaf:homepage $\n" + - "}"; + final String query = """ + PREFIX foaf: + SELECT ?craft { + ?craft foaf:name $ . + ?craft foaf:homepage $ + }"""; final QueryHolder holder = queryParser.parseQuery(query); assertEquals(3, holder.getParameters().size()); assertTrue(holder.getParameters().contains(new QueryParameter<>("craft", valueFactory))); @@ -161,12 +165,12 @@ public void testParseQueryWithUnnumberedPositionalParams() { @Test public void testParseQueryWithMixedNumberedAndUnnumberedPositionalParams() { - final String query = "PREFIX foaf: \n" + - "SELECT ?craft\n" + - "{\n" + - "?craft foaf:name $1 .\n" + - "?craft foaf:homepage $\n" + - "}"; + final String query = """ + PREFIX foaf: + SELECT ?craft { + ?craft foaf:name $1 . + ?craft foaf:homepage $ + }"""; final QueryHolder holder = queryParser.parseQuery(query); assertEquals(3, holder.getParameters().size()); assertTrue(holder.getParameters().contains(new QueryParameter<>("craft", valueFactory))); @@ -176,12 +180,12 @@ public void testParseQueryWithMixedNumberedAndUnnumberedPositionalParams() { @Test public void parsingQueryWithUsedParameterPositionThrowsException() { - final String query = "PREFIX foaf: \n" + - "SELECT ?craft\n" + - "{\n" + - "?craft foaf:name $1 .\n" + - "?craft foaf:homepage $1\n" + - "}"; + final String query = """ + PREFIX foaf: + SELECT ?craft { + ?craft foaf:name $1 . + ?craft foaf:homepage $1 + }"""; assertThrows(QueryParserException.class, () -> queryParser.parseQuery(query)); } @@ -200,12 +204,13 @@ public void parsingQueryWithIncorrectlyMixedNumberedAndUnnumberedPositionsThrows @Test public void parsingQueryWithPositionalParameterNotNumberThrowsException() { - final String query = "PREFIX foaf: \n" + - "SELECT ?craft\n" + - "{\n" + - "?craft foaf:name $1 .\n" + - "?craft foaf:homepage $notanumber\n" + - "}"; + final String query = """ + PREFIX foaf: + SELECT ?craft + { + ?craft foaf:name $1 . + ?craft foaf:homepage $notanumber + }"""; assertThrows(QueryParserException.class, () -> queryParser.parseQuery(query)); } @@ -304,10 +309,11 @@ public void parsesQueryWithPropertyPathAlternative() { @Test void parseQuerySupportsWindowsLikeLineEnds() { - final String query = "SELECT * WHERE {\r\n" + - " GRAPH ?contextUri\r\n" + - " { ?s ?p ?o . }\r" + - "}"; + final String query = """ + SELECT * WHERE {\r + GRAPH ?contextUri\r + { ?s ?p ?o . }\r\ + }"""; final QueryHolder holder = queryParser.parseQuery(query); assertNotNull(holder.getParameter("contextUri")); assertNotNull(holder.getParameter("s")); @@ -352,4 +358,70 @@ void parseQueryDoesNotRequireWhereKeywordInAskQuery() { final QueryParameter labelVar = (QueryParameter) holder.getParameter("label"); assertFalse(labelVar.isProjected()); } + + @Test + void parseQueryIgnoresQueryCommentsAtStringStart() { + final String query = """ + # Selects ?y that have label "Test" - variables name is different to test ignoring variables in comments + SELECT ?x WHERE { + ?x "Test" . + }"""; + final QueryHolder holder = queryParser.parseQuery(query); + assertEquals(1, holder.getParameters().size()); + assertNotNull(holder.getParameter("x")); + } + + @Test + void parseQueryIgnoresQueryCommentsBetweenQueryLines() { + final String query = """ + SELECT ?x WHERE { + # Selects ?y that have label "Test" - variables name is different to test ignoring variables in comments + ?x "Test" . + }"""; + final QueryHolder holder = queryParser.parseQuery(query); + assertEquals(1, holder.getParameters().size()); + assertNotNull(holder.getParameter("x")); + } + + @Test + void parseQueryIgnoresQueryCommentsStartingInQueryLine() { + final String query = """ + SELECT ?x WHERE { + ?x a ?t#ype . is still a comment so ?y is ignored + }"""; + final QueryHolder holder = queryParser.parseQuery(query); + assertEquals(2, holder.getParameters().size()); + assertNotNull(holder.getParameter("x")); + assertNotNull(holder.getParameter("t")); + } + + @Test + void parseQueryIgnoresQueryCommentsStartingInFunction() { + final String query = """ + SELECT ?x WHERE { + ?x a ?t#ype . is still a comment so ?y is ignored + BIND (STR(?t) as ?typeStr)# as ?y) + }"""; + final QueryHolder holder = queryParser.parseQuery(query); + assertEquals(3, holder.getParameters().size()); + assertNotNull(holder.getParameter("x")); + assertNotNull(holder.getParameter("t")); + assertNotNull(holder.getParameter("typeStr")); + } + + @Test + void parseQueryHandlesAsk() { + final String query = """ + ASK { + ?c ?hasContainer ?container . + ?container ?hasMember ?a . + FILTER (STRSTARTS(STR(?hasMember), "rdf:_")) } + """; + final QueryHolder holder = queryParser.parseQuery(query); + assertNotNull(holder.getParameter("c")); + assertNotNull(holder.getParameter("hasContainer")); + assertNotNull(holder.getParameter("container")); + assertNotNull(holder.getParameter("hasMember")); + assertNotNull(holder.getParameter("a")); + } }