From 7fe5db0f827ba33311aec6369badd45d618e422c Mon Sep 17 00:00:00 2001 From: Max Loeb Date: Mon, 25 Sep 2023 11:13:23 -0700 Subject: [PATCH 1/4] modernize update_all --- lib/Model.php | 57 +++++++--------------------------- lib/Relation.php | 29 +++++++++++++++++ lib/SQLBuilder.php | 12 +++---- lib/Table.php | 16 +++++----- test/ActiveRecordWriteTest.php | 24 +++++++------- 5 files changed, 66 insertions(+), 72 deletions(-) diff --git a/lib/Model.php b/lib/Model.php index 8dfefc1f..07d7946b 100644 --- a/lib/Model.php +++ b/lib/Model.php @@ -1031,8 +1031,15 @@ private function update($validate = true) return false; } - $dirty = $this->dirty_attributes(); - static::table()->update($dirty, $pk); + $attributes = $this->dirty_attributes(); + + $options = [ + 'conditions' => [ + new WhereClause([$this->table()->pk[0] => $pk], []) + ] + ]; + + static::table()->update($attributes, $options); $this->invoke_callback('after_update', false); $this->update_cache(); } @@ -1076,52 +1083,12 @@ public static function delete_all(): int } /** - * Updates records using set in $options - * - * Does not instantiate models and therefore does not invoke callbacks - * - * Update all using a hash: - * - * ``` - * YourModel::update_all(['set' => ['name' => "Bob"]]); - * ``` - * - * Update all using a string: - * - * ``` - * YourModel::update_all(['set' => 'name = "Bob"']); - * ``` - * - * An options array takes the following parameters: + * @see Relation::update_all() * - * @param QueryOptions $options - * - * @return int Number of rows affected */ - public static function update_all(array $options = []): int + public static function update_all(string|array $attributes): int { - $table = static::table(); - $conn = static::connection(); - $sql = new SQLBuilder($conn, $table->get_fully_qualified_table_name()); - - isset($options['set']) && $sql->update($options['set']); - - if (isset($options['conditions']) && ($conditions = $options['conditions'])) { - $sql->where([WhereClause::from_arg($conditions)]); - } - - if (isset($options['limit'])) { - $sql->limit($options['limit']); - } - - if (isset($options['order'])) { - $sql->order($options['order']); - } - - $values = $sql->bind_values(); - $ret = $conn->query($table->last_sql = $sql->to_s(), $values); - - return $ret->rowCount(); + return static::Relation()->update_all($attributes); } /** diff --git a/lib/Relation.php b/lib/Relation.php index ba02dee0..f65ab8c3 100644 --- a/lib/Relation.php +++ b/lib/Relation.php @@ -15,6 +15,7 @@ * @template TModel of Model * * @phpstan-import-type RelationOptions from Types + * @phpstan-import-type Attributes from Types */ class Relation implements \Iterator { @@ -846,6 +847,34 @@ public function to_a(): array return $this->_to_a($this->options); } + /** + * Updates records using set in $options + * + * Does not instantiate models and therefore does not invoke callbacks + * + * Update all using a hash: + * + * ``` + * YourModel::update_all(['set' => ['name' => "Bob"]]); + * ``` + * + * Update all using a string: + * + * ``` + * YourModel::update_all(['set' => 'name = "Bob"']); + * ``` + * + * An options array takes the following parameters: + * + * @param Attributes $options + * + * @return int Number of rows affected + */ + public function update_all(array|string $attributes): int + { + return $this->table()->update($attributes, $this->options); + } + /** * Deletes the records without instantiating the records * first, and without invoking callbacks. diff --git a/lib/SQLBuilder.php b/lib/SQLBuilder.php index 98ba3f65..a6edf602 100644 --- a/lib/SQLBuilder.php +++ b/lib/SQLBuilder.php @@ -216,18 +216,18 @@ public function insert(array $hash, mixed $pk = null, string $sequence_name = nu } /** - * @param Attributes|string $mixed + * @param Attributes|string $attributes * * @throws ActiveRecordException */ - public function update(array|string $mixed): static + public function update(array|string $data): static { $this->operation = 'UPDATE'; - if (is_hash($mixed)) { - $this->data = $mixed; - } elseif (is_string($mixed)) { - $this->update = $mixed; + if (is_hash($data)) { + $this->data = $data; + } elseif (is_string($data)) { + $this->update = $data; } return $this; diff --git a/lib/Table.php b/lib/Table.php index c0b72cf2..d08af160 100644 --- a/lib/Table.php +++ b/lib/Table.php @@ -422,21 +422,19 @@ public function insert(array &$data, string|int $pk = null, string $sequence_nam } /** - * @param Attributes $data - * @param Attributes $where + * @param string|Attributes $attributes + * @param Attributes $options * * @throws Exception\ActiveRecordException */ - public function update(array &$data, array $where): \PDOStatement + public function update(string|array $attributes, array $options): int { - $data = $this->process_data($data); - - $sql = new SQLBuilder($this->conn, $this->get_fully_qualified_table_name()); - $sql->update($data)->where([new WhereClause($where, [])], []); - + $sql = $this->options_to_sql($options); + $sql->update($attributes); $values = $sql->bind_values(); + $ret = $this->conn->query($this->last_sql = $sql->to_s(), $values); - return $this->conn->query($this->last_sql = $sql->to_s(), $values); + return $ret->rowCount(); } /** diff --git a/test/ActiveRecordWriteTest.php b/test/ActiveRecordWriteTest.php index 1b925320..251d0cfd 100644 --- a/test/ActiveRecordWriteTest.php +++ b/test/ActiveRecordWriteTest.php @@ -395,18 +395,18 @@ public function testDeleteAllWithDistinct() Author::distinct()->where('parent_author_id = 2')->delete_all(); } - public function testUpdateAllWithSetAsString() + public function testUpdateAllWithString() { - Author::update_all(['set' => 'parent_author_id = 2']); + Author::update_all('parent_author_id = 2'); $ids = Author::pluck('parent_author_id'); foreach ($ids as $id) { $this->assertEquals(2, $id); } } - public function testUpdateAllWithSetAsHash() + public function testUpdateAllWithHash() { - Author::update_all(['set' => ['parent_author_id' => 2]]); + Author::update_all(['parent_author_id' => 2]); $ids = Author::pluck('parent_author_id'); foreach ($ids as $id) { $this->assertEquals(2, $id); @@ -427,18 +427,15 @@ public function testUpdateAllWithConditionsAsString() public function testUpdateAllWithConditionsAsHash() { - $num_affected = Author::update_all([ - 'set' => 'parent_author_id = 2', - 'conditions' => [ - 'name' => 'Tito' - ] - ]); + $num_affected = Author::where([ + 'name' => 'Tito' + ])->update_all('parent_author_id = 2'); $this->assertEquals(2, $num_affected); } public function testUpdateAllWithConditionsAsArray() { - $num_affected = Author::update_all(['set' => 'parent_author_id = 2', 'conditions' => ['name = ?', 'Tito']]); + $num_affected = Author::where(['name = ?', 'Tito'])->update_all('parent_author_id = 2'); $this->assertEquals(2, $num_affected); } @@ -448,7 +445,10 @@ public function testUpdateAllWithLimitAndOrder() $this->markTestSkipped('Only MySQL & Sqlite accept limit/order with UPDATE clause'); } - $num_affected = Author::update_all(['set' => 'parent_author_id = 2', 'limit' => 1, 'order' => 'name asc']); + $num_affected = Author::limit(1) + ->order('name asc') + ->update_all('parent_author_id = 2'); + $this->assertEquals(1, $num_affected); $this->assertTrue(false !== strpos(Author::table()->last_sql, 'ORDER BY name asc LIMIT 1')); } From b63ed273e5637c09c8250cbc284feedfc4d7ca45 Mon Sep 17 00:00:00 2001 From: Max Loeb Date: Mon, 25 Sep 2023 11:27:41 -0700 Subject: [PATCH 2/4] lint and fixes --- lib/Model.php | 1 - lib/Relation.php | 2 -- lib/SQLBuilder.php | 2 -- lib/Table.php | 5 ++++- test/ActiveRecordWriteTest.php | 5 +---- 5 files changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/Model.php b/lib/Model.php index 07d7946b..86886a33 100644 --- a/lib/Model.php +++ b/lib/Model.php @@ -1084,7 +1084,6 @@ public static function delete_all(): int /** * @see Relation::update_all() - * */ public static function update_all(string|array $attributes): int { diff --git a/lib/Relation.php b/lib/Relation.php index f65ab8c3..653a83e8 100644 --- a/lib/Relation.php +++ b/lib/Relation.php @@ -866,8 +866,6 @@ public function to_a(): array * * An options array takes the following parameters: * - * @param Attributes $options - * * @return int Number of rows affected */ public function update_all(array|string $attributes): int diff --git a/lib/SQLBuilder.php b/lib/SQLBuilder.php index a6edf602..c6b95ac3 100644 --- a/lib/SQLBuilder.php +++ b/lib/SQLBuilder.php @@ -216,8 +216,6 @@ public function insert(array $hash, mixed $pk = null, string $sequence_name = nu } /** - * @param Attributes|string $attributes - * * @throws ActiveRecordException */ public function update(array|string $data): static diff --git a/lib/Table.php b/lib/Table.php index d08af160..2ede9f2f 100644 --- a/lib/Table.php +++ b/lib/Table.php @@ -423,13 +423,16 @@ public function insert(array &$data, string|int $pk = null, string $sequence_nam /** * @param string|Attributes $attributes - * @param Attributes $options + * @param Attributes $options * * @throws Exception\ActiveRecordException */ public function update(string|array $attributes, array $options): int { $sql = $this->options_to_sql($options); + if (is_hash($attributes)) { + $attributes = $this->process_data($attributes); + } $sql->update($attributes); $values = $sql->bind_values(); $ret = $this->conn->query($this->last_sql = $sql->to_s(), $values); diff --git a/test/ActiveRecordWriteTest.php b/test/ActiveRecordWriteTest.php index 251d0cfd..048dbb9a 100644 --- a/test/ActiveRecordWriteTest.php +++ b/test/ActiveRecordWriteTest.php @@ -418,10 +418,7 @@ public function testUpdateAllWithHash() */ public function testUpdateAllWithConditionsAsString() { - $num_affected = Author::update_all([ - 'set' => 'parent_author_id = 2', - 'conditions' => ['name = ?', 'Tito'] - ]); + $num_affected = Author::where(['name = ?', 'Tito'])->update_all('parent_author_id = 2'); $this->assertEquals(2, $num_affected); } From 92790fd0c15fa6b797ca93c34cfdb363a450c548 Mon Sep 17 00:00:00 2001 From: Max Loeb Date: Mon, 25 Sep 2023 11:38:00 -0700 Subject: [PATCH 3/4] stanning --- lib/Model.php | 4 +++- lib/Relation.php | 36 ++++++++++++++++++++++++------------ lib/SQLBuilder.php | 2 ++ 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/lib/Model.php b/lib/Model.php index 86886a33..cb055457 100644 --- a/lib/Model.php +++ b/lib/Model.php @@ -1013,7 +1013,7 @@ private function insert($validate = true) * * @return bool True if the model was saved to the database otherwise false */ - private function update($validate = true) + private function update(bool $validate = true) { $this->verify_not_readonly('update'); @@ -1083,6 +1083,8 @@ public static function delete_all(): int } /** + * @param string|Attributes $attributes + * * @see Relation::update_all() */ public static function update_all(string|array $attributes): int diff --git a/lib/Relation.php b/lib/Relation.php index 653a83e8..dcf98a3b 100644 --- a/lib/Relation.php +++ b/lib/Relation.php @@ -848,25 +848,37 @@ public function to_a(): array } /** - * Updates records using set in $options + * Updates all records in the current relation with provided attributes. + * This method constructs a single SQL UPDATE statement and sends it + * straight to the database. It does not instantiate the involved models + * and it does not trigger Active Record callbacks or validations. + * However, values passed to #update_all will still go through Active + * Record's normal type casting and serialization. Returns the number of + * rows affected. * - * Does not instantiate models and therefore does not invoke callbacks + * Note: As Active Record callbacks are not triggered, this method will + * not automatically update updated_at+/+updated_on columns. * - * Update all using a hash: + * // Update all customers with the given attributes + * Customer::update_all( ['wants_email'] => true) * - * ``` - * YourModel::update_all(['set' => ['name' => "Bob"]]); - * ``` + * // Update all books with 'Rails' in their title + * Book::where('title LIKE ?', '%Rails%') + * ->update_all(['author', 'David']) * - * Update all using a string: + * // Update all books that match conditions, but limit it to 5 ordered by date + * Book::where('title LIKE ?', '%Rails%') + * ->order('created_at') + * ->limit(5) + * ->update_all(['author'=> 'David']) * - * ``` - * YourModel::update_all(['set' => 'name = "Bob"']); - * ``` + * // Update all invoices and set the number column to its id value. + * Invoice::update_all('number = id') * - * An options array takes the following parameters: + * @param string|Attributes $attributes A string or hash representing the SET part of + * an SQL statement. * - * @return int Number of rows affected + * @return int number of affected records */ public function update_all(array|string $attributes): int { diff --git a/lib/SQLBuilder.php b/lib/SQLBuilder.php index c6b95ac3..81a32cd9 100644 --- a/lib/SQLBuilder.php +++ b/lib/SQLBuilder.php @@ -216,6 +216,8 @@ public function insert(array $hash, mixed $pk = null, string $sequence_name = nu } /** + * @param string|Attributes $data + * * @throws ActiveRecordException */ public function update(array|string $data): static From fe61dbb5c30ee3130b047c9af6c3a1c3211b3b02 Mon Sep 17 00:00:00 2001 From: Max Loeb Date: Mon, 25 Sep 2023 11:40:27 -0700 Subject: [PATCH 4/4] lint --- lib/Relation.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Relation.php b/lib/Relation.php index dcf98a3b..ecebb346 100644 --- a/lib/Relation.php +++ b/lib/Relation.php @@ -875,8 +875,8 @@ public function to_a(): array * // Update all invoices and set the number column to its id value. * Invoice::update_all('number = id') * - * @param string|Attributes $attributes A string or hash representing the SET part of - * an SQL statement. + * @param string|Attributes $attributes a string or hash representing the SET part of + * an SQL statement * * @return int number of affected records */