From afa709dd08a01057a7e657c3dc3f49751368f2ba Mon Sep 17 00:00:00 2001 From: Sami Dokus Date: Thu, 24 Sep 2026 14:39:10 -0600 Subject: [PATCH 1/3] Prepare delete_many IDs as placeholders and ignore non-positive integer IDs --- .../Traits/Custom_Table_Query_Methods.php | 45 +++++------ .../Traits/Custom_Table_Query_MethodsTest.php | 74 +++++++++++++++++++ 2 files changed, 97 insertions(+), 22 deletions(-) diff --git a/src/Schema/Traits/Custom_Table_Query_Methods.php b/src/Schema/Traits/Custom_Table_Query_Methods.php index 4e88682..26de539 100644 --- a/src/Schema/Traits/Custom_Table_Query_Methods.php +++ b/src/Schema/Traits/Custom_Table_Query_Methods.php @@ -177,6 +177,7 @@ public static function delete( int $uid, string $column = '' ): bool { * Deletes multiple rows from the table. * * @since 3.0.0 + * @since TBD IDs are passed to the query as placeholders, and non-positive IDs are ignored for integer columns. * * @param array $ids The IDs of the rows to delete. * @param string $column The column to use for the delete query. @@ -185,38 +186,38 @@ public static function delete( int $uid, string $column = '' ): bool { * @return bool|int The number of rows affected, or `false` on failure. */ public static function delete_many( array $ids, string $column = '', string $more_where = '' ) { - $ids = array_filter( - array_map( - fn( $id ) => is_numeric( $id ) ? (int) $id : "'{$id}'", - $ids - ) - ); + $columns = static::get_columns(); + $wheres = []; + $values = []; - if ( empty( $ids ) ) { - return false; + foreach ( $column ? [ $column ] : static::primary_columns() as $column_name ) { + $column_object = $columns->get( $column_name ); + $is_int = $column_object && PHP_Types::INT === $column_object->get_php_type(); + + $column_ids = $is_int ? + array_filter( array_map( fn( $id ) => filter_var( $id, FILTER_VALIDATE_INT, [ 'options' => [ 'min_range' => 1 ] ] ), $ids ) ) : + array_filter( array_map( 'strval', $ids ), fn( $id ) => '' !== $id ); + + if ( empty( $column_ids ) ) { + return false; + } + + $wheres[] = '%i IN (' . implode( ', ', array_fill( 0, count( $column_ids ), $is_int ? '%d' : '%s' ) ) . ')'; + array_push( $values, $column_name, ...array_values( $column_ids ) ); } - $database = Config::get_db(); - $prepared_ids = implode( ', ', $ids ); - $column = $column ? - "{$column} IN ({$prepared_ids})" : - implode( - ' AND ', - array_map( - function ( $c ) use ( $prepared_ids ) { - return "{$c} IN ({$prepared_ids})"; - }, - static::primary_columns() - ) - ); + $database = Config::get_db(); + $where = implode( ' AND ', $wheres ); return $database::query( $database::prepare( - "DELETE FROM %i WHERE {$column} {$more_where}", + "DELETE FROM %i WHERE {$where} {$more_where}", static::table_name( true ), + ...$values ) ); } + /** * Prepares the statements and values for the insert and update queries. * diff --git a/tests/wpunit/Traits/Custom_Table_Query_MethodsTest.php b/tests/wpunit/Traits/Custom_Table_Query_MethodsTest.php index bbbb003..db19850 100644 --- a/tests/wpunit/Traits/Custom_Table_Query_MethodsTest.php +++ b/tests/wpunit/Traits/Custom_Table_Query_MethodsTest.php @@ -1029,6 +1029,80 @@ public function should_handle_empty_sub_where_group() { /** * Get a test table for query method testing. */ + /** + * @test + */ + public function should_delete_many_by_int_and_numeric_string_ids() { + $table = $this->get_query_test_table(); + Register::table( $table ); + + $ids = $this->insert_delete_fixtures( $table, 3 ); + + $this->assertSame( 2, $table::delete_many( [ $ids[0], (string) $ids[1] ] ) ); + $this->assertNull( $table::get_by_id( $ids[0] ) ); + $this->assertNull( $table::get_by_id( $ids[1] ) ); + $this->assertNotNull( $table::get_by_id( $ids[2] ) ); + $this->assertTrue( $table::delete( $ids[2] ) ); + $this->assertNull( $table::get_by_id( $ids[2] ) ); + } + + /** + * @test + */ + public function should_not_delete_anything_for_non_positive_ids() { + $table = $this->get_query_test_table(); + Register::table( $table ); + + $ids = $this->insert_delete_fixtures( $table, 2 ); + + $this->assertFalse( $table::delete_many( [ -$ids[0], 0, '-' . $ids[1], '0' ] ) ); + $this->assertFalse( $table::delete( -$ids[0] ) ); + $this->assertFalse( $table::delete( 0 ) ); + $this->assertEquals( 2, $table::get_total_items() ); + } + + /** + * @test + */ + public function should_not_interpolate_string_ids_into_the_query() { + $table = $this->get_query_test_table(); + Register::table( $table ); + + $this->insert_delete_fixtures( $table, 2 ); + + $this->assertFalse( $table::delete_many( [ '1 OR 1=1', "1) OR (1=1" ] ) ); + $this->assertSame( 0, $table::delete_many( [ "x' OR '1'='1" ], 'slug' ) ); + $this->assertEquals( 2, $table::get_total_items() ); + } + + /** + * @test + */ + public function should_delete_many_by_custom_column_with_quoted_values() { + $table = $this->get_query_test_table(); + Register::table( $table ); + + $this->insert_delete_fixtures( $table, 2 ); + $table::insert( [ 'name' => 'Quoted', 'slug' => "it's-quoted", 'status' => 1 ] ); + + $this->assertSame( 1, $table::delete_many( [ "it's-quoted" ], 'slug' ) ); + $this->assertNull( $table::get_first_by( 'slug', "it's-quoted" ) ); + $this->assertSame( 1, $table::delete_many( [ 'test-delete-0' ], 'slug' ) ); + $this->assertEquals( 1, $table::get_total_items() ); + $this->assertNotNull( $table::get_first_by( 'slug', 'test-delete-1' ) ); + } + + private function insert_delete_fixtures( $table, int $count ): array { + $ids = []; + + for ( $i = 0; $i < $count; $i++ ) { + $table::insert( [ 'name' => "Test {$i}", 'slug' => "test-delete-{$i}", 'status' => 1 ] ); + $ids[] = DB::last_insert_id(); + } + + return $ids; + } + private function get_query_test_table() { return new class extends Table { const SCHEMA_VERSION = '1.0.0'; From b184a280738ff9fe1a9e4cac35e6288572e6477b Mon Sep 17 00:00:00 2001 From: Sami Dokus Date: Thu, 24 Sep 2026 15:13:38 -0600 Subject: [PATCH 2/3] Accept zero-padded numeric string IDs in delete_many --- src/Schema/Traits/Custom_Table_Query_Methods.php | 2 +- tests/wpunit/Traits/Custom_Table_Query_MethodsTest.php | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Schema/Traits/Custom_Table_Query_Methods.php b/src/Schema/Traits/Custom_Table_Query_Methods.php index 26de539..a445380 100644 --- a/src/Schema/Traits/Custom_Table_Query_Methods.php +++ b/src/Schema/Traits/Custom_Table_Query_Methods.php @@ -195,7 +195,7 @@ public static function delete_many( array $ids, string $column = '', string $mor $is_int = $column_object && PHP_Types::INT === $column_object->get_php_type(); $column_ids = $is_int ? - array_filter( array_map( fn( $id ) => filter_var( $id, FILTER_VALIDATE_INT, [ 'options' => [ 'min_range' => 1 ] ] ), $ids ) ) : + array_filter( array_map( fn( $id ) => filter_var( is_string( $id ) && ctype_digit( $id ) ? ltrim( $id, '0' ) : $id, FILTER_VALIDATE_INT, [ 'options' => [ 'min_range' => 1 ] ] ), $ids ) ) : array_filter( array_map( 'strval', $ids ), fn( $id ) => '' !== $id ); if ( empty( $column_ids ) ) { diff --git a/tests/wpunit/Traits/Custom_Table_Query_MethodsTest.php b/tests/wpunit/Traits/Custom_Table_Query_MethodsTest.php index db19850..eca13d1 100644 --- a/tests/wpunit/Traits/Custom_Table_Query_MethodsTest.php +++ b/tests/wpunit/Traits/Custom_Table_Query_MethodsTest.php @@ -1036,9 +1036,11 @@ public function should_delete_many_by_int_and_numeric_string_ids() { $table = $this->get_query_test_table(); Register::table( $table ); - $ids = $this->insert_delete_fixtures( $table, 3 ); + $ids = $this->insert_delete_fixtures( $table, 4 ); $this->assertSame( 2, $table::delete_many( [ $ids[0], (string) $ids[1] ] ) ); + $this->assertSame( 1, $table::delete_many( [ '000' . $ids[3] ] ) ); + $this->assertNull( $table::get_by_id( $ids[3] ) ); $this->assertNull( $table::get_by_id( $ids[0] ) ); $this->assertNull( $table::get_by_id( $ids[1] ) ); $this->assertNotNull( $table::get_by_id( $ids[2] ) ); From 8bcd22b5eb8c79bad60503afadb6b60fa2465102 Mon Sep 17 00:00:00 2001 From: Sami Dokus Date: Fri, 25 Sep 2026 15:38:28 -0600 Subject: [PATCH 3/3] Keep numeric IDs, cast them, and drop duplicates in delete_many --- src/Schema/Traits/Custom_Table_Query_Methods.php | 10 ++++++---- tests/wpunit/Traits/Custom_Table_Query_MethodsTest.php | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Schema/Traits/Custom_Table_Query_Methods.php b/src/Schema/Traits/Custom_Table_Query_Methods.php index a445380..6345106 100644 --- a/src/Schema/Traits/Custom_Table_Query_Methods.php +++ b/src/Schema/Traits/Custom_Table_Query_Methods.php @@ -177,7 +177,7 @@ public static function delete( int $uid, string $column = '' ): bool { * Deletes multiple rows from the table. * * @since 3.0.0 - * @since TBD IDs are passed to the query as placeholders, and non-positive IDs are ignored for integer columns. + * @since TBD IDs are passed to the query as placeholders, non-numeric IDs are ignored for integer columns, and duplicates are removed. * * @param array $ids The IDs of the rows to delete. * @param string $column The column to use for the delete query. @@ -194,9 +194,11 @@ public static function delete_many( array $ids, string $column = '', string $mor $column_object = $columns->get( $column_name ); $is_int = $column_object && PHP_Types::INT === $column_object->get_php_type(); - $column_ids = $is_int ? - array_filter( array_map( fn( $id ) => filter_var( is_string( $id ) && ctype_digit( $id ) ? ltrim( $id, '0' ) : $id, FILTER_VALIDATE_INT, [ 'options' => [ 'min_range' => 1 ] ] ), $ids ) ) : - array_filter( array_map( 'strval', $ids ), fn( $id ) => '' !== $id ); + $column_ids = array_unique( + $is_int ? + array_map( 'intval', array_filter( $ids, 'is_numeric' ) ) : + array_filter( array_map( 'strval', $ids ), fn( $id ) => '' !== $id ) + ); if ( empty( $column_ids ) ) { return false; diff --git a/tests/wpunit/Traits/Custom_Table_Query_MethodsTest.php b/tests/wpunit/Traits/Custom_Table_Query_MethodsTest.php index eca13d1..ac2f8aa 100644 --- a/tests/wpunit/Traits/Custom_Table_Query_MethodsTest.php +++ b/tests/wpunit/Traits/Custom_Table_Query_MethodsTest.php @@ -1038,7 +1038,7 @@ public function should_delete_many_by_int_and_numeric_string_ids() { $ids = $this->insert_delete_fixtures( $table, 4 ); - $this->assertSame( 2, $table::delete_many( [ $ids[0], (string) $ids[1] ] ) ); + $this->assertSame( 2, $table::delete_many( [ $ids[0], (string) $ids[1], $ids[0] ] ) ); $this->assertSame( 1, $table::delete_many( [ '000' . $ids[3] ] ) ); $this->assertNull( $table::get_by_id( $ids[3] ) ); $this->assertNull( $table::get_by_id( $ids[0] ) ); @@ -1057,7 +1057,7 @@ public function should_not_delete_anything_for_non_positive_ids() { $ids = $this->insert_delete_fixtures( $table, 2 ); - $this->assertFalse( $table::delete_many( [ -$ids[0], 0, '-' . $ids[1], '0' ] ) ); + $this->assertSame( 0, $table::delete_many( [ -$ids[0], 0, '-' . $ids[1], '0' ] ) ); $this->assertFalse( $table::delete( -$ids[0] ) ); $this->assertFalse( $table::delete( 0 ) ); $this->assertEquals( 2, $table::get_total_items() );