Skip to content

Adding the a test for combined tokens #200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 15, 2021
Merged

Adding the a test for combined tokens #200

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3ec11ff
Adding the a test for combined tokens
mamazu Nov 30, 2021
3315407
Adding tests for unquoted strings in square brackets
mamazu Dec 7, 2021
f8b891e
Fixing tests for brackets
mamazu Dec 11, 2021
ae813ad
Handling strings inside brackets
mamazu Dec 15, 2021
f5b28b6
Simplifying the next character logic
mamazu Dec 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion src/PHPCR/Util/QOM/Sql2Scanner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,9 @@ protected function scan($sql2)
$stringStartCharacter = false;
$isEscaped = false;
$escapedQuotesCount = 0;
foreach (\str_split($sql2) as $index => $character) {
$splitString = \str_split($sql2);
for ($index = 0; $index < count($splitString); $index++) {
$character = $splitString[$index];
if (!$stringStartCharacter && in_array($character, [' ', "\t", "\n"], true)) {
if ($currentToken !== '') {
$tokens[] = $currentToken;
Expand All @@ -165,6 +167,19 @@ protected function scan($sql2)
$currentToken = '';
continue;
}

// Handling the squared brackets in queries
if (!$isEscaped && $character === '[') {
if ($currentToken !== '') {
$tokens[] = $currentToken;
}
$stringSize = $this->parseBrackets($sql2, $index);
$tokens[] = substr($sql2, $index, $stringSize);
// We need to subtract one here because the for loop will automatically increment the index
$index += $stringSize - 1;
continue;
}

$currentToken .= $character;

if (!$isEscaped && in_array($character, ['"', "'"], true)) {
Expand All @@ -188,6 +203,12 @@ protected function scan($sql2)
} elseif (!$stringStartCharacter) {
// If there is no start character already we have found the beginning of a new string
$stringStartCharacter = $character;

// When tokenizing `AS"abc"` add the current token (AS) as token already
if (strlen($currentToken) > 1) {
$tokens[] = substr($currentToken, 0, strlen($currentToken) - 1);
$currentToken = $character;
}
}
}
$isEscaped = $character === '\\';
Expand All @@ -211,4 +232,11 @@ private function getCharacterAtIndex($string, $index)

return '';
}

private function parseBrackets(string $query, int $index): int
{
$endPosition = strpos($query, ']', $index) + 1;

return $endPosition - $index;
}
}
57 changes: 54 additions & 3 deletions tests/PHPCR/Tests/Util/QOM/Sql2ScannerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ public function testToken()
/**
* @dataProvider dataTestStringTokenization
*/
public function testStringTokenization()
public function testStringTokenization(string $query)
{
$scanner = new Sql2Scanner('SELECT page.* FROM [nt:unstructured] AS page WHERE name ="Hello world"');
$scanner = new Sql2Scanner($query);
$expected = [
'SELECT',
'page',
Expand All @@ -49,7 +49,7 @@ public function testStringTokenization()
$this->expectTokensFromScanner($scanner, $expected);
}

public function dataTestStringTokenization()
public function dataTestStringTokenization(): array
{
$multilineQuery = <<<'SQL'
SELECT page.*
Expand Down Expand Up @@ -124,6 +124,57 @@ public function testSQLEscapedStrings2()
$this->expectTokensFromScanner($scanner, $expected);
}

public function testSquareBrackets()
{
$sql = 'WHERE ISSAMENODE(file, ["/home node"])';

$scanner = new Sql2Scanner($sql);
$expected = [
'WHERE',
'ISSAMENODE',
'(',
'file',
',',
'["/home node"]',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think the quotes need to completely be removed while scanning (not sure exactly what the old logic did). maybe we need to backport the test to a version with the old scanner and make the test assert the result of that scanner, and then make the new scanner do the same behaviour?

when i run the tests with this branch, i get:

There were 2 failures:

1) PHPCR\Tests\Query\QOM\ConvertQueriesBackAndForthTest::testBackAndForth with data set "6.7.20.SameNode.Selector_Space" ('6.7.20.SameNode.Selector_Space', array('SELECT * FROM [nt:file] AS fi...ode"])', 'SELECT * FROM [nt:file] AS fi...node])'))
QOM-->SQL2->QOM: Query variation 6.7.20.SameNode.Selector_Space resulted in SQL2 that is not found: array (
  0 => 'SELECT * FROM [nt:file] AS file WHERE ISSAMENODE(file, ["/home node"])',
  1 => 'SELECT * FROM [nt:file] AS file WHERE ISSAMENODE(file, [/home node])',
)

jackalope-doctrine-dbal/vendor/phpcr/phpcr-api-tests/tests/Query/QOM/ConvertQueriesBackAndForthTest.php:84

2) PHPCR\Tests\Query\QOM\Sql2ToQomConverterTest::testQueries with data set "6.7.20.SameNode.Selector_Space" ('6.7.20.SameNode.Selector_Space', array('SELECT * FROM [nt:file] AS fi...ode"])', 'SELECT * FROM [nt:file] AS fi...node])'))
Original query = SELECT * FROM [nt:file] AS file WHERE ISSAMENODE(file, ["/home node"])
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
     'source' => Jackalope\Query\QOM\Selector Object (...)
     'constraint' => Jackalope\Query\QOM\SameNode Object (
         'selectorName' => 'file'
-        'path' => '/home node'
+        'path' => '"/home node"'
     )
     'orderings' => Array ()
     'columns' => Array ()

jackalope-doctrine-dbal/vendor/phpcr/phpcr-api-tests/tests/Query/QOM/Sql2ToQomConverterTest.php:96

Copy link
Contributor Author

@mamazu mamazu Dec 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we didn't have tests before. I couldn't tell that I changed the logic. Would have probably made more sense to create the tests first. I can check the previous implementation though.

But I that shouldn't be too hard to implement. The question still remains then though, what about invalid strings inside those brackets. Should they be validated?

')',
];

$this->expectTokensFromScanner($scanner, $expected);
}

public function testSquareBracketsWithoutQuotes()
{
$sql = 'WHERE ISSAMENODE(file, [/home node])';

$scanner = new Sql2Scanner($sql);
$expected = [
'WHERE',
'ISSAMENODE',
'(',
'file',
',',
'[/home node]',
')',
];

$this->expectTokensFromScanner($scanner, $expected);
}

public function testTokenizingWithMissingSpaces()
{
$sql = 'SELECT * AS"all"';

$scanner = new Sql2Scanner($sql);
$expected = [
'SELECT',
'*',
'AS',
'"all"',
];

$this->expectTokensFromScanner($scanner, $expected);
}

public function testThrowingErrorOnUnclosedString()
{
$this->expectException(InvalidQueryException::class);
Expand Down