From 4c856c9a43fc18132b8d72c78aeabbb7cc5d6dbb Mon Sep 17 00:00:00 2001 From: KazuyaUchida Date: Tue, 17 Dec 2024 18:20:17 +0900 Subject: [PATCH 01/10] =?UTF-8?q?=E3=82=B3=E3=83=A1=E3=83=B3=E3=83=88?= =?UTF-8?q?=E3=82=92=E4=BB=98=E4=B8=8E=E3=81=97=E3=81=9FSQL=E3=81=8C?= =?UTF-8?q?=E5=AE=9F=E8=A1=8C=E3=81=95=E3=82=8C=E3=81=AA=E3=81=84=E5=95=8F?= =?UTF-8?q?=E9=A1=8C=E3=82=92=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/SqlQueryRowList.php | 9 +++++++-- tests/Fake/sql/todo_item_by_id_with_comment.sql | 7 +++++++ tests/SqlQueryTest.php | 12 ++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 tests/Fake/sql/todo_item_by_id_with_comment.sql diff --git a/src/SqlQueryRowList.php b/src/SqlQueryRowList.php index fa62e10..8c754fc 100644 --- a/src/SqlQueryRowList.php +++ b/src/SqlQueryRowList.php @@ -12,6 +12,8 @@ use function array_pop; use function count; use function explode; +use function preg_replace; +use function str_starts_with; use function strpos; use function strtolower; use function trim; @@ -52,8 +54,11 @@ public function __invoke(array ...$queries): iterable } $lastQuery = $result - ? strtolower(trim((string) $result->queryString, "\\ \t\n\r\0\x0B")) : ''; - if ($result instanceof PDOStatement && strpos($lastQuery, 'select') === 0) { + ? strtolower(trim( + (string) preg_replace('/\/\*.*?\*\//', '', (string) $result->queryString), + "\\ \t\n\r\0\x0B", + )) : ''; + if ($result instanceof PDOStatement && str_starts_with($lastQuery, 'select')) { return (array) $result->fetchAll(PDO::FETCH_ASSOC); } diff --git a/tests/Fake/sql/todo_item_by_id_with_comment.sql b/tests/Fake/sql/todo_item_by_id_with_comment.sql new file mode 100644 index 0000000..658a4e5 --- /dev/null +++ b/tests/Fake/sql/todo_item_by_id_with_comment.sql @@ -0,0 +1,7 @@ +/* todo_item_by_id_with_comment.sql */ +SELECT + * +FROM + todo /* table */ +WHERE + id = :id /* conditions */ \ No newline at end of file diff --git a/tests/SqlQueryTest.php b/tests/SqlQueryTest.php index 1f41012..37c9b78 100644 --- a/tests/SqlQueryTest.php +++ b/tests/SqlQueryTest.php @@ -66,4 +66,16 @@ public function testMultipleQuery(): void $this->assertSame('test', $row['title']); $this->assertSame('2', $row['id']); } + + public function testWithComment(): void + { + $sql = (string) file_get_contents(__DIR__ . '/Fake/sql/todo_item_by_id_with_comment.sql'); + $query = new SqlQueryRowList($this->pdo, $sql); + $row = ((array) $query(['id' => 1]))[0]; + assert(is_array($row)); + assert(isset($row['title'])); + assert(isset($row['id'])); + $this->assertSame('run', $row['title']); + $this->assertSame('1', $row['id']); + } } From f8182280b67b54ad522c5b6eb43afe46327926c9 Mon Sep 17 00:00:00 2001 From: KazuyaUchida Date: Tue, 17 Dec 2024 18:46:36 +0900 Subject: [PATCH 02/10] Feedback coderabbitai review --- src/SqlQueryRowList.php | 2 +- .../sql/todo_item_by_id_with_line_comment.sql | 2 ++ .../todo_item_by_id_with_multiple_comment.sql | 1 + tests/SqlQueryTest.php | 25 +++++++++++++------ 4 files changed, 22 insertions(+), 8 deletions(-) create mode 100644 tests/Fake/sql/todo_item_by_id_with_line_comment.sql create mode 100644 tests/Fake/sql/todo_item_by_id_with_multiple_comment.sql diff --git a/src/SqlQueryRowList.php b/src/SqlQueryRowList.php index 8c754fc..db7f6b4 100644 --- a/src/SqlQueryRowList.php +++ b/src/SqlQueryRowList.php @@ -55,7 +55,7 @@ public function __invoke(array ...$queries): iterable $lastQuery = $result ? strtolower(trim( - (string) preg_replace('/\/\*.*?\*\//', '', (string) $result->queryString), + (string) preg_replace('/\/\*.*?\*\/|--.*$/m', '', (string) $result->queryString), "\\ \t\n\r\0\x0B", )) : ''; if ($result instanceof PDOStatement && str_starts_with($lastQuery, 'select')) { diff --git a/tests/Fake/sql/todo_item_by_id_with_line_comment.sql b/tests/Fake/sql/todo_item_by_id_with_line_comment.sql new file mode 100644 index 0000000..1fd2b42 --- /dev/null +++ b/tests/Fake/sql/todo_item_by_id_with_line_comment.sql @@ -0,0 +1,2 @@ +-- todo_item_by_id_with_line_comment.sql +SELECT * FROM todo WHERE id = :id -- comment \ No newline at end of file diff --git a/tests/Fake/sql/todo_item_by_id_with_multiple_comment.sql b/tests/Fake/sql/todo_item_by_id_with_multiple_comment.sql new file mode 100644 index 0000000..4d491e8 --- /dev/null +++ b/tests/Fake/sql/todo_item_by_id_with_multiple_comment.sql @@ -0,0 +1 @@ +/* todo_item_by_id_with_multiple_comment.sql */ SELECT * FROM todo WHERE id = :id -- conditions \ No newline at end of file diff --git a/tests/SqlQueryTest.php b/tests/SqlQueryTest.php index 37c9b78..890cb12 100644 --- a/tests/SqlQueryTest.php +++ b/tests/SqlQueryTest.php @@ -32,13 +32,7 @@ protected function setUp(): void public function testInvoke(): void { $sql = (string) file_get_contents(__DIR__ . '/Fake/sql/todo_item_by_id.sql'); - $query = new SqlQueryRowList($this->pdo, $sql); - $row = ((array) $query(['id' => 1]))[0]; - assert(is_array($row)); - assert(isset($row['title'])); - assert(isset($row['id'])); - $this->assertSame('run', $row['title']); - $this->assertSame('1', $row['id']); + $this->testSql($sql); } public function testNotFound(): void @@ -70,6 +64,23 @@ public function testMultipleQuery(): void public function testWithComment(): void { $sql = (string) file_get_contents(__DIR__ . '/Fake/sql/todo_item_by_id_with_comment.sql'); + $this->testSql($sql); + } + + public function testWithLineComment(): void + { + $sql = (string) file_get_contents(__DIR__ . '/Fake/sql/todo_item_by_id_with_line_comment.sql'); + $this->testSql($sql); + } + + public function testWithMultipleComment(): void + { + $sql = (string) file_get_contents(__DIR__ . '/Fake/sql/todo_item_by_id_with_multiple_comment.sql'); + $this->testSql($sql); + } + + private function testSql(string $sql): void + { $query = new SqlQueryRowList($this->pdo, $sql); $row = ((array) $query(['id' => 1]))[0]; assert(is_array($row)); From c34dc4a36e8c4a3c5fe7d7815569b55df4073c76 Mon Sep 17 00:00:00 2001 From: KazuyaUchida Date: Tue, 17 Dec 2024 18:59:15 +0900 Subject: [PATCH 03/10] Feedback coderabbitai review / use phpunit function --- tests/SqlQueryTest.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/SqlQueryTest.php b/tests/SqlQueryTest.php index 890cb12..8ad5dc8 100644 --- a/tests/SqlQueryTest.php +++ b/tests/SqlQueryTest.php @@ -82,10 +82,12 @@ public function testWithMultipleComment(): void private function testSql(string $sql): void { $query = new SqlQueryRowList($this->pdo, $sql); - $row = ((array) $query(['id' => 1]))[0]; - assert(is_array($row)); - assert(isset($row['title'])); - assert(isset($row['id'])); + $result = (array) $query(['id' => 1]); + $this->assertNotEmpty($result); + $row = $result[0]; + $this->assertIsArray($row); + $this->assertArrayHasKey('title', $row); + $this->assertArrayHasKey('id', $row); $this->assertSame('run', $row['title']); $this->assertSame('1', $row['id']); } From b310e55ce8e90f2ee7dfab25ea03c6304a582172 Mon Sep 17 00:00:00 2001 From: KazuyaUchida Date: Wed, 18 Dec 2024 09:26:56 +0900 Subject: [PATCH 04/10] rollback str_starts_with -> strpos --- src/SqlQueryRowList.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/SqlQueryRowList.php b/src/SqlQueryRowList.php index db7f6b4..01f5579 100644 --- a/src/SqlQueryRowList.php +++ b/src/SqlQueryRowList.php @@ -13,7 +13,6 @@ use function count; use function explode; use function preg_replace; -use function str_starts_with; use function strpos; use function strtolower; use function trim; @@ -58,7 +57,7 @@ public function __invoke(array ...$queries): iterable (string) preg_replace('/\/\*.*?\*\/|--.*$/m', '', (string) $result->queryString), "\\ \t\n\r\0\x0B", )) : ''; - if ($result instanceof PDOStatement && str_starts_with($lastQuery, 'select')) { + if ($result instanceof PDOStatement && strpos($lastQuery, 'select') === 0) { return (array) $result->fetchAll(PDO::FETCH_ASSOC); } From eaf52fa1d47af23dbf4b718fe07890de1eaf07d6 Mon Sep 17 00:00:00 2001 From: KazuyaUchida <86758002+KazuyaUchida@users.noreply.github.com> Date: Wed, 18 Dec 2024 12:40:12 +0900 Subject: [PATCH 05/10] Apply suggestions from code review Co-authored-by: Akihito Koriyama --- tests/Fake/sql/todo_item_by_id_with_comment.sql | 2 +- tests/Fake/sql/todo_item_by_id_with_line_comment.sql | 2 +- tests/Fake/sql/todo_item_by_id_with_multiple_comment.sql | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Fake/sql/todo_item_by_id_with_comment.sql b/tests/Fake/sql/todo_item_by_id_with_comment.sql index 658a4e5..d13cf2d 100644 --- a/tests/Fake/sql/todo_item_by_id_with_comment.sql +++ b/tests/Fake/sql/todo_item_by_id_with_comment.sql @@ -4,4 +4,4 @@ SELECT FROM todo /* table */ WHERE - id = :id /* conditions */ \ No newline at end of file + id = :id /* conditions */ diff --git a/tests/Fake/sql/todo_item_by_id_with_line_comment.sql b/tests/Fake/sql/todo_item_by_id_with_line_comment.sql index 1fd2b42..355dc27 100644 --- a/tests/Fake/sql/todo_item_by_id_with_line_comment.sql +++ b/tests/Fake/sql/todo_item_by_id_with_line_comment.sql @@ -1,2 +1,2 @@ -- todo_item_by_id_with_line_comment.sql -SELECT * FROM todo WHERE id = :id -- comment \ No newline at end of file +SELECT * FROM todo WHERE id = :id -- comment diff --git a/tests/Fake/sql/todo_item_by_id_with_multiple_comment.sql b/tests/Fake/sql/todo_item_by_id_with_multiple_comment.sql index 4d491e8..eb9f05f 100644 --- a/tests/Fake/sql/todo_item_by_id_with_multiple_comment.sql +++ b/tests/Fake/sql/todo_item_by_id_with_multiple_comment.sql @@ -1 +1 @@ -/* todo_item_by_id_with_multiple_comment.sql */ SELECT * FROM todo WHERE id = :id -- conditions \ No newline at end of file +/* todo_item_by_id_with_multiple_comment.sql */ SELECT * FROM todo WHERE id = :id -- conditions From 0354483f26f02565760be30d9208121b7cc962d7 Mon Sep 17 00:00:00 2001 From: Akihito Koriyama Date: Wed, 18 Dec 2024 12:58:57 +0900 Subject: [PATCH 06/10] Add type annotations to QueryInterceptor methods Added precise type annotations for MethodInvocation and other parameters in QueryInterceptor. This improves code readability and aids in static analysis. No functional changes were made. --- src/QueryInterceptor.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/QueryInterceptor.php b/src/QueryInterceptor.php index 52944b0..3379656 100644 --- a/src/QueryInterceptor.php +++ b/src/QueryInterceptor.php @@ -46,6 +46,7 @@ public function invoke(MethodInvocation $invocation) } /** + * @param MethodInvocation $invocation * @param array $param * * @return mixed @@ -62,7 +63,10 @@ private function getQueryResult(MethodInvocation $invocation, QueryInterface $qu return $result; } - /** @param mixed $result */ + /** + * @param MethodInvocation $invocation + * @param mixed $result + */ private function returnRo(ResourceObject $ro, MethodInvocation $invocation, $result): ResourceObject { if (! $result) { From bfe51067dec8729f1e91006a563c5d408d02309b Mon Sep 17 00:00:00 2001 From: Akihito Koriyama Date: Wed, 18 Dec 2024 13:01:30 +0900 Subject: [PATCH 07/10] Update src/QueryInterceptor.php Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- src/QueryInterceptor.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/QueryInterceptor.php b/src/QueryInterceptor.php index 3379656..2c32d19 100644 --- a/src/QueryInterceptor.php +++ b/src/QueryInterceptor.php @@ -64,8 +64,9 @@ private function getQueryResult(MethodInvocation $invocation, QueryInterface $qu } /** - * @param MethodInvocation $invocation - * @param mixed $result + * @param MethodInvocation $invocation + * @param array|object|scalar|null $result + * @return ResourceObject */ private function returnRo(ResourceObject $ro, MethodInvocation $invocation, $result): ResourceObject { From 7f5e3021c7132bbbe2f6e9b4d903d995af97c4ab Mon Sep 17 00:00:00 2001 From: Akihito Koriyama Date: Wed, 18 Dec 2024 13:03:25 +0900 Subject: [PATCH 08/10] fixup! Add type annotations to QueryInterceptor methods --- src/QueryInterceptor.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/QueryInterceptor.php b/src/QueryInterceptor.php index 2c32d19..8a71e31 100644 --- a/src/QueryInterceptor.php +++ b/src/QueryInterceptor.php @@ -47,7 +47,7 @@ public function invoke(MethodInvocation $invocation) /** * @param MethodInvocation $invocation - * @param array $param + * @param array $param * * @return mixed */ @@ -64,9 +64,8 @@ private function getQueryResult(MethodInvocation $invocation, QueryInterface $qu } /** - * @param MethodInvocation $invocation + * @param MethodInvocation $invocation * @param array|object|scalar|null $result - * @return ResourceObject */ private function returnRo(ResourceObject $ro, MethodInvocation $invocation, $result): ResourceObject { From c02772e7425c41b17eae38e6de93429530a5f570 Mon Sep 17 00:00:00 2001 From: Akihito Koriyama Date: Wed, 18 Dec 2024 13:15:10 +0900 Subject: [PATCH 09/10] Refactor SQL cleanup by introducing reusable constants Introduce `QUERY_CLEANUP_REGEX` and `TRIM_CHARACTERS_REGEX` constants to improve code readability and reduce redundancy. Replace hardcoded regex strings with these constants in SQL trimming and cleanup operations. This makes future maintenance and updates to these patterns simpler and more centralized. --- src/SqlQueryRowList.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/SqlQueryRowList.php b/src/SqlQueryRowList.php index 01f5579..fa4015f 100644 --- a/src/SqlQueryRowList.php +++ b/src/SqlQueryRowList.php @@ -19,6 +19,9 @@ class SqlQueryRowList implements RowListInterface { + public const QUERY_CLEANUP_REGEX = '/\/\*.*?\*\/|--.*$/m'; + public const TRIM_CHARACTERS_REGEX = "\\ \t\n\r\0\x0B"; + /** @var ExtendedPdoInterface */ private $pdo; @@ -38,7 +41,7 @@ public function __invoke(array ...$queries): iterable $this->sql .= ';'; } - $sqls = explode(';', trim($this->sql, "\\ \t\n\r\0\x0B")); + $sqls = explode(';', trim($this->sql, self::TRIM_CHARACTERS_REGEX)); array_pop($sqls); $numQueris = count($queries); if (count($sqls) !== $numQueris) { @@ -54,8 +57,8 @@ public function __invoke(array ...$queries): iterable $lastQuery = $result ? strtolower(trim( - (string) preg_replace('/\/\*.*?\*\/|--.*$/m', '', (string) $result->queryString), - "\\ \t\n\r\0\x0B", + (string) preg_replace(self::QUERY_CLEANUP_REGEX, '', (string) $result->queryString), + self::TRIM_CHARACTERS_REGEX, )) : ''; if ($result instanceof PDOStatement && strpos($lastQuery, 'select') === 0) { return (array) $result->fetchAll(PDO::FETCH_ASSOC); From 20a90ae5239ba4abc4d9b62757cdcdb8a38ff7ab Mon Sep 17 00:00:00 2001 From: Akihito Koriyama Date: Wed, 18 Dec 2024 13:26:38 +0900 Subject: [PATCH 10/10] Refactor type annotations for query result handling. Updated type annotations to ensure compatibility with diverse data structures and improve clarity. This change enhances maintainability and conforms to best practices for type declarations. --- src/QueryInterceptor.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/QueryInterceptor.php b/src/QueryInterceptor.php index 8a71e31..8911e4a 100644 --- a/src/QueryInterceptor.php +++ b/src/QueryInterceptor.php @@ -57,6 +57,7 @@ private function getQueryResult(MethodInvocation $invocation, QueryInterface $qu $result = $query($param); $object = $invocation->getThis(); if ($object instanceof ResourceObject) { + /** @var array|object|scalar|null $result */ return $this->returnRo($object, $invocation, $result); } @@ -65,7 +66,7 @@ private function getQueryResult(MethodInvocation $invocation, QueryInterface $qu /** * @param MethodInvocation $invocation - * @param array|object|scalar|null $result + * @param mixed $result */ private function returnRo(ResourceObject $ro, MethodInvocation $invocation, $result): ResourceObject {