From 285f379da40e5ec8eb7768b6834f4ac2629f2132 Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 27 Nov 2023 16:41:06 +0900 Subject: [PATCH 01/12] docs: fix PHPDoc types --- system/BaseModel.php | 28 ++++++++++++++++------------ system/Model.php | 5 +++-- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/system/BaseModel.php b/system/BaseModel.php index 1d854256a63c..f37cfab1fa28 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -824,10 +824,11 @@ public function insert($data = null, bool $returnID = true) /** * Compiles batch insert runs the queries, validating each row prior. * - * @param array|null $set an associative array of insert values - * @param bool|null $escape Whether to escape values - * @param int $batchSize The size of the batch to run - * @param bool $testing True means only number of records is returned, false will execute the query + * @param list|null $set an associative array of insert values + * @phpstan-param list|null $set + * @param bool|null $escape Whether to escape values + * @param int $batchSize The size of the batch to run + * @param bool $testing True means only number of records is returned, false will execute the query * * @return bool|int Number of rows inserted or FALSE on failure * @@ -912,6 +913,7 @@ public function insertBatch(?array $set = null, ?bool $escape = null, int $batch * * @param array|int|string|null $id * @param array|object|null $data + * @phpstan-param row_array|object|null $data * * @throws ReflectionException */ @@ -973,7 +975,8 @@ public function update($id = null, $data = null): bool /** * Compiles an update and runs the query. * - * @param array|null $set An associative array of update values + * @param list|null $set an associative array of insert values + * @phpstan-param list|null $set * @param string|null $index The where key * @param int $batchSize The size of the batch to run * @param bool $returnSQL True means SQL is returned, false will execute the query @@ -1656,9 +1659,9 @@ public function asObject(string $class = 'object') * This method uses objectToRawArray() internally and does conversion * to string on all Time instances * - * @param object|string $data Data - * @param bool $onlyChanged Only Changed Property - * @param bool $recursive If true, inner entities will be cast as array as well + * @param object|null $data Data + * @param bool $onlyChanged Only Changed Property + * @param bool $recursive If true, inner entities will be cast as array as well * * @return array Array * @@ -1688,9 +1691,9 @@ protected function objectToArray($data, bool $onlyChanged = true, bool $recursiv * Takes a class and returns an array of its public and protected * properties as an array with raw values. * - * @param object|string $data Data - * @param bool $onlyChanged Only Changed Property - * @param bool $recursive If true, inner entities will be casted as array as well + * @param object|null $data Data + * @param bool $onlyChanged Only Changed Property + * @param bool $recursive If true, inner entities will be casted as array as well * * @return array|null Array * @@ -1722,7 +1725,8 @@ protected function objectToRawArray($data, bool $onlyChanged = true, bool $recur * Transform data to array. * * @param array|object|null $data Data - * @param string $type Type of data (insert|update) + * @phpstan-param row_array|object|null $data + * @param string $type Type of data (insert|update) * * @throws DataException * @throws InvalidArgumentException diff --git a/system/Model.php b/system/Model.php index 0a8cd9dbec59..a0dfe33dfafb 100644 --- a/system/Model.php +++ b/system/Model.php @@ -778,6 +778,7 @@ protected function doProtectFieldsForInsert(array $data): array * * @param array|int|string|null $id * @param array|object|null $data + * @phpstan-param row_array|object|null $data * * @throws ReflectionException */ @@ -802,8 +803,8 @@ public function update($id = null, $data = null): bool * Takes a class and returns an array of its public and protected * properties as an array with raw values. * - * @param object|string $data - * @param bool $recursive If true, inner entities will be cast as array as well + * @param object|null $data + * @param bool $recursive If true, inner entities will be cast as array as well * * @return array|null Array * From be70e74c4ea5259c9ea40b6521858780c1707f24 Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 27 Nov 2023 16:49:35 +0900 Subject: [PATCH 02/12] refactor: rename parameter name to $object --- system/BaseModel.php | 18 +++++++++--------- system/Model.php | 6 +++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/system/BaseModel.php b/system/BaseModel.php index f37cfab1fa28..d88af3c82557 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -1659,7 +1659,7 @@ public function asObject(string $class = 'object') * This method uses objectToRawArray() internally and does conversion * to string on all Time instances * - * @param object|null $data Data + * @param object|null $object Object * @param bool $onlyChanged Only Changed Property * @param bool $recursive If true, inner entities will be cast as array as well * @@ -1667,9 +1667,9 @@ public function asObject(string $class = 'object') * * @throws ReflectionException */ - protected function objectToArray($data, bool $onlyChanged = true, bool $recursive = false): array + protected function objectToArray($object, bool $onlyChanged = true, bool $recursive = false): array { - $properties = $this->objectToRawArray($data, $onlyChanged, $recursive); + $properties = $this->objectToRawArray($object, $onlyChanged, $recursive); assert(is_array($properties)); @@ -1691,7 +1691,7 @@ protected function objectToArray($data, bool $onlyChanged = true, bool $recursiv * Takes a class and returns an array of its public and protected * properties as an array with raw values. * - * @param object|null $data Data + * @param object|null $object Object * @param bool $onlyChanged Only Changed Property * @param bool $recursive If true, inner entities will be casted as array as well * @@ -1699,12 +1699,12 @@ protected function objectToArray($data, bool $onlyChanged = true, bool $recursiv * * @throws ReflectionException */ - protected function objectToRawArray($data, bool $onlyChanged = true, bool $recursive = false): ?array + protected function objectToRawArray($object, bool $onlyChanged = true, bool $recursive = false): array { - if (method_exists($data, 'toRawArray')) { - $properties = $data->toRawArray($onlyChanged, $recursive); + if (method_exists($object, 'toRawArray')) { + $properties = $object->toRawArray($onlyChanged, $recursive); } else { - $mirror = new ReflectionClass($data); + $mirror = new ReflectionClass($object); $props = $mirror->getProperties(ReflectionProperty::IS_PUBLIC | ReflectionProperty::IS_PROTECTED); $properties = []; @@ -1714,7 +1714,7 @@ protected function objectToRawArray($data, bool $onlyChanged = true, bool $recur foreach ($props as $prop) { // Must make protected values accessible. $prop->setAccessible(true); - $properties[$prop->getName()] = $prop->getValue($data); + $properties[$prop->getName()] = $prop->getValue($object); } } diff --git a/system/Model.php b/system/Model.php index a0dfe33dfafb..ce821df9f77c 100644 --- a/system/Model.php +++ b/system/Model.php @@ -803,16 +803,16 @@ public function update($id = null, $data = null): bool * Takes a class and returns an array of its public and protected * properties as an array with raw values. * - * @param object|null $data + * @param object|null $object Object * @param bool $recursive If true, inner entities will be cast as array as well * * @return array|null Array * * @throws ReflectionException */ - protected function objectToRawArray($data, bool $onlyChanged = true, bool $recursive = false): ?array + protected function objectToRawArray($object, bool $onlyChanged = true, bool $recursive = false): array { - return parent::objectToRawArray($data, $onlyChanged); + return parent::objectToRawArray($object, $onlyChanged); } /** From c602c4f67a7ce745e679fa84c366e260f2a4c128 Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 27 Nov 2023 17:29:31 +0900 Subject: [PATCH 03/12] docs: fix PHPDoc types --- system/BaseModel.php | 15 ++++++++------- system/Model.php | 6 +++--- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/system/BaseModel.php b/system/BaseModel.php index d88af3c82557..bad60dc559b4 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -1659,9 +1659,9 @@ public function asObject(string $class = 'object') * This method uses objectToRawArray() internally and does conversion * to string on all Time instances * - * @param object|null $object Object - * @param bool $onlyChanged Only Changed Property - * @param bool $recursive If true, inner entities will be cast as array as well + * @param object $object Object + * @param bool $onlyChanged Only Changed Property + * @param bool $recursive If true, inner entities will be cast as array as well * * @return array Array * @@ -1691,16 +1691,17 @@ protected function objectToArray($object, bool $onlyChanged = true, bool $recurs * Takes a class and returns an array of its public and protected * properties as an array with raw values. * - * @param object|null $object Object - * @param bool $onlyChanged Only Changed Property - * @param bool $recursive If true, inner entities will be casted as array as well + * @param object $object Object + * @param bool $onlyChanged Only Changed Property + * @param bool $recursive If true, inner entities will be casted as array as well * - * @return array|null Array + * @return array * * @throws ReflectionException */ protected function objectToRawArray($object, bool $onlyChanged = true, bool $recursive = false): array { + // Entity::toRawArray() returns array. if (method_exists($object, 'toRawArray')) { $properties = $object->toRawArray($onlyChanged, $recursive); } else { diff --git a/system/Model.php b/system/Model.php index ce821df9f77c..c6dbf5c699d9 100644 --- a/system/Model.php +++ b/system/Model.php @@ -803,10 +803,10 @@ public function update($id = null, $data = null): bool * Takes a class and returns an array of its public and protected * properties as an array with raw values. * - * @param object|null $object Object - * @param bool $recursive If true, inner entities will be cast as array as well + * @param object $object Object + * @param bool $recursive If true, inner entities will be cast as array as well * - * @return array|null Array + * @return array * * @throws ReflectionException */ From 512ba40c3b023c5648956ef82906cbc5c32905c6 Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 27 Nov 2023 17:29:44 +0900 Subject: [PATCH 04/12] refactor: remove unneeded assert() --- system/BaseModel.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/system/BaseModel.php b/system/BaseModel.php index bad60dc559b4..e71a73cbe789 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -1671,8 +1671,6 @@ protected function objectToArray($object, bool $onlyChanged = true, bool $recurs { $properties = $this->objectToRawArray($object, $onlyChanged, $recursive); - assert(is_array($properties)); - // Convert any Time instances to appropriate $dateFormat if ($properties !== []) { $properties = array_map(function ($value) { From 58fce8b8fff3775310ebc7d439adf14fbf1b1396 Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 27 Nov 2023 18:46:26 +0900 Subject: [PATCH 05/12] docs: fix PHPDoc types --- system/BaseModel.php | 2 +- system/Model.php | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/system/BaseModel.php b/system/BaseModel.php index e71a73cbe789..06a78e3f2601 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -1663,7 +1663,7 @@ public function asObject(string $class = 'object') * @param bool $onlyChanged Only Changed Property * @param bool $recursive If true, inner entities will be cast as array as well * - * @return array Array + * @return array * * @throws ReflectionException */ diff --git a/system/Model.php b/system/Model.php index c6dbf5c699d9..deffa4a9a042 100644 --- a/system/Model.php +++ b/system/Model.php @@ -212,6 +212,7 @@ protected function doFind(bool $singleton, $id = null) * @param string $columnName Column Name * * @return array|null The resulting row of data, or null if no data found. + * @phpstan-return list|null */ protected function doFindColumn(string $columnName) { @@ -715,7 +716,7 @@ protected function shouldUpdate($data): bool * @phpstan-param row_array|object|null $data * @param bool $returnID Whether insert ID should be returned or not. * - * @return false|int|object|string + * @return bool|int|string * @phpstan-return ($returnID is true ? int|string|false : bool) * * @throws ReflectionException From c43037fba8a29d47da027dec2d3dfdceccf43c9c Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 27 Nov 2023 18:59:20 +0900 Subject: [PATCH 06/12] refactor: extract timeToString() --- system/BaseModel.php | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/system/BaseModel.php b/system/BaseModel.php index 06a78e3f2601..94f710d01fe1 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -1672,17 +1672,29 @@ protected function objectToArray($object, bool $onlyChanged = true, bool $recurs $properties = $this->objectToRawArray($object, $onlyChanged, $recursive); // Convert any Time instances to appropriate $dateFormat - if ($properties !== []) { - $properties = array_map(function ($value) { - if ($value instanceof Time) { - return $this->timeToDate($value); - } + return $this->timeToString($properties); + } - return $value; - }, $properties); + /** + * Convert any Time instances to appropriate $dateFormat. + * + * @param array $properties + * + * @return array + */ + protected function timeToString(array $properties): array + { + if ($properties === []) { + return []; } - return $properties; + return array_map(function ($value) { + if ($value instanceof Time) { + return $this->timeToDate($value); + } + + return $value; + }, $properties); } /** From 6b77de2edb493d12ea7f2b8406c031328fa97c95 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 28 Nov 2023 09:13:22 +0900 Subject: [PATCH 07/12] docs: add PHPDoc types --- system/BaseModel.php | 10 ++++++---- system/Model.php | 5 +++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/system/BaseModel.php b/system/BaseModel.php index 94f710d01fe1..94fe410bfce7 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -478,8 +478,9 @@ abstract protected function doOnlyDeleted(); * Compiles a replace and runs the query. * This method works only with dbCalls. * - * @param array|null $data Data - * @param bool $returnSQL Set to true to return Query String + * @param array|null $data Data + * @phpstan-param row_array|null $data + * @param bool $returnSQL Set to true to return Query String * * @return BaseResult|false|Query|string */ @@ -1151,8 +1152,9 @@ public function onlyDeleted() /** * Compiles a replace and runs the query. * - * @param array|null $data Data - * @param bool $returnSQL Set to true to return Query String + * @param array|null $data Data + * @phpstan-param row_array|null $data + * @param bool $returnSQL Set to true to return Query String * * @return BaseResult|false|Query|string */ diff --git a/system/Model.php b/system/Model.php index deffa4a9a042..8d8645fa9572 100644 --- a/system/Model.php +++ b/system/Model.php @@ -481,8 +481,9 @@ protected function doOnlyDeleted() * Compiles a replace into string and runs the query * This method works only with dbCalls. * - * @param array|null $data Data - * @param bool $returnSQL Set to true to return Query String + * @param array|null $data Data + * @phpstan-param row_array|null $data + * @param bool $returnSQL Set to true to return Query String * * @return BaseResult|false|Query|string */ From 872beb905649cbdba9657f3973e1ce9e72abfed9 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 28 Nov 2023 09:15:18 +0900 Subject: [PATCH 08/12] refactor: fix non-boolean if condition --- system/BaseModel.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/BaseModel.php b/system/BaseModel.php index 94fe410bfce7..38f63c5f189a 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -1161,7 +1161,7 @@ public function onlyDeleted() public function replace(?array $data = null, bool $returnSQL = false) { // Validate data before saving. - if ($data && ! $this->skipValidation && ! $this->validate($data)) { + if (($data !== null) && ! $this->skipValidation && ! $this->validate($data)) { return false; } From a2c7b3f18c251d74a124b55da54cb2f7d0360da2 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 28 Nov 2023 09:15:51 +0900 Subject: [PATCH 09/12] chore: update phpstan-baseline --- phpstan-baseline.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/phpstan-baseline.php b/phpstan-baseline.php index 039eedaaccdc..d1fd9361ac01 100644 --- a/phpstan-baseline.php +++ b/phpstan-baseline.php @@ -51,11 +51,6 @@ 'count' => 1, 'path' => __DIR__ . '/system/BaseModel.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Only booleans are allowed in &&, array\\|null given on the left side\\.$#', - 'count' => 1, - 'path' => __DIR__ . '/system/BaseModel.php', -]; $ignoreErrors[] = [ 'message' => '#^Only booleans are allowed in a ternary operator condition, array\\|null given\\.$#', 'count' => 1, From f4d17d24e499f65bb85ae7218b21f662c17a133c Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 28 Nov 2023 09:20:27 +0900 Subject: [PATCH 10/12] refactor: extract setCreatedField() and setUpdatedField() --- system/BaseModel.php | 62 ++++++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/system/BaseModel.php b/system/BaseModel.php index 38f63c5f189a..bd4d0ab59b18 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -783,14 +783,8 @@ public function insert($data = null, bool $returnID = true) // Set created_at and updated_at with same time $date = $this->setDate(); - - if ($this->useTimestamps && $this->createdField !== '' && ! array_key_exists($this->createdField, $data)) { - $data[$this->createdField] = $date; - } - - if ($this->useTimestamps && $this->updatedField !== '' && ! array_key_exists($this->updatedField, $data)) { - $data[$this->updatedField] = $date; - } + $data = $this->setCreatedField($data, $date); + $data = $this->setUpdatedField($data, $date); $eventData = ['data' => $data]; @@ -822,6 +816,36 @@ public function insert($data = null, bool $returnID = true) return $returnID ? $this->insertID : $result; } + /** + * Set datetime to created field. + * + * @phpstan-param row_array $row + * @param int|string $date timestamp or datetime string + */ + protected function setCreatedField(array $row, $date): array + { + if ($this->useTimestamps && $this->createdField !== '' && ! array_key_exists($this->createdField, $row)) { + $row[$this->createdField] = $date; + } + + return $row; + } + + /** + * Set datetime to updated field. + * + * @phpstan-param row_array $row + * @param int|string $date timestamp or datetime string + */ + protected function setUpdatedField(array $row, $date): array + { + if ($this->useTimestamps && $this->updatedField !== '' && ! array_key_exists($this->updatedField, $row)) { + $row[$this->updatedField] = $date; + } + + return $row; + } + /** * Compiles batch insert runs the queries, validating each row prior. * @@ -871,14 +895,8 @@ public function insertBatch(?array $set = null, ?bool $escape = null, int $batch // Set created_at and updated_at with same time $date = $this->setDate(); - - if ($this->useTimestamps && $this->createdField !== '' && ! array_key_exists($this->createdField, $row)) { - $row[$this->createdField] = $date; - } - - if ($this->useTimestamps && $this->updatedField !== '' && ! array_key_exists($this->updatedField, $row)) { - $row[$this->updatedField] = $date; - } + $row = $this->setCreatedField($row, $date); + $row = $this->setUpdatedField($row, $date); } } @@ -945,9 +963,7 @@ public function update($id = null, $data = null): bool throw DataException::forEmptyDataset('update'); } - if ($this->useTimestamps && $this->updatedField !== '' && ! array_key_exists($this->updatedField, $data)) { - $data[$this->updatedField] = $this->setDate(); - } + $data = $this->setUpdatedField($data, $this->setDate()); $eventData = [ 'id' => $id, @@ -1031,9 +1047,7 @@ public function updateBatch(?array $set = null, ?string $index = null, int $batc $row[$index] = $updateIndex; } - if ($this->useTimestamps && $this->updatedField !== '' && ! array_key_exists($this->updatedField, $row)) { - $row[$this->updatedField] = $this->setDate(); - } + $row = $this->setUpdatedField($row, $this->setDate()); } } @@ -1165,9 +1179,7 @@ public function replace(?array $data = null, bool $returnSQL = false) return false; } - if ($this->useTimestamps && $this->updatedField !== '' && ! array_key_exists($this->updatedField, (array) $data)) { - $data[$this->updatedField] = $this->setDate(); - } + $data = $this->setUpdatedField((array) $data, $this->setDate()); return $this->doReplace($data, $returnSQL); } From b2bf84d720f1bef3de8e9a2cfe2a4b9fc5175425 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 28 Nov 2023 09:29:58 +0900 Subject: [PATCH 11/12] refactor: replace $data with $row --- system/BaseModel.php | 76 ++++++++++++++++++++++---------------------- system/Model.php | 40 +++++++++++------------ 2 files changed, 58 insertions(+), 58 deletions(-) diff --git a/system/BaseModel.php b/system/BaseModel.php index bd4d0ab59b18..a0faaddefe0b 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -478,13 +478,13 @@ abstract protected function doOnlyDeleted(); * Compiles a replace and runs the query. * This method works only with dbCalls. * - * @param array|null $data Data - * @phpstan-param row_array|null $data + * @param array|null $row Row data + * @phpstan-param row_array|null $row * @param bool $returnSQL Set to true to return Query String * * @return BaseResult|false|Query|string */ - abstract protected function doReplace(?array $data = null, bool $returnSQL = false); + abstract protected function doReplace(?array $row = null, bool $returnSQL = false); /** * Grabs the last error(s) that occurred from the Database connection. @@ -741,8 +741,8 @@ public function getInsertID() * Inserts data into the database. If an object is provided, * it will attempt to convert it to an array. * - * @param array|object|null $data Data - * @phpstan-param row_array|object|null $data + * @param array|object|null $row Row data + * @phpstan-param row_array|object|null $row * @param bool $returnID Whether insert ID should be returned or not. * * @return bool|int|string insert ID or true on success. false on failure. @@ -750,7 +750,7 @@ public function getInsertID() * * @throws ReflectionException */ - public function insert($data = null, bool $returnID = true) + public function insert($row = null, bool $returnID = true) { $this->insertID = 0; @@ -758,10 +758,10 @@ public function insert($data = null, bool $returnID = true) $cleanValidationRules = $this->cleanValidationRules; $this->cleanValidationRules = false; - $data = $this->transformDataToArray($data, 'insert'); + $row = $this->transformDataToArray($row, 'insert'); // Validate data before saving. - if (! $this->skipValidation && ! $this->validate($data)) { + if (! $this->skipValidation && ! $this->validate($row)) { // Restore $cleanValidationRules $this->cleanValidationRules = $cleanValidationRules; @@ -773,20 +773,20 @@ public function insert($data = null, bool $returnID = true) // Must be called first, so we don't // strip out created_at values. - $data = $this->doProtectFieldsForInsert($data); + $row = $this->doProtectFieldsForInsert($row); // doProtectFields() can further remove elements from // $data so we need to check for empty dataset again - if (! $this->allowEmptyInserts && empty($data)) { + if (! $this->allowEmptyInserts && empty($row)) { throw DataException::forEmptyDataset('insert'); } // Set created_at and updated_at with same time $date = $this->setDate(); - $data = $this->setCreatedField($data, $date); - $data = $this->setUpdatedField($data, $date); + $row = $this->setCreatedField($row, $date); + $row = $this->setUpdatedField($row, $date); - $eventData = ['data' => $data]; + $eventData = ['data' => $row]; if ($this->tempAllowCallbacks) { $eventData = $this->trigger('beforeInsert', $eventData); @@ -931,12 +931,12 @@ public function insertBatch(?array $set = null, ?bool $escape = null, int $batch * it will attempt to convert it into an array. * * @param array|int|string|null $id - * @param array|object|null $data - * @phpstan-param row_array|object|null $data + * @param array|object|null $row Row data + * @phpstan-param row_array|object|null $row * * @throws ReflectionException */ - public function update($id = null, $data = null): bool + public function update($id = null, $row = null): bool { if (is_bool($id)) { throw new InvalidArgumentException('update(): argument #1 ($id) should not be boolean.'); @@ -946,28 +946,28 @@ public function update($id = null, $data = null): bool $id = [$id]; } - $data = $this->transformDataToArray($data, 'update'); + $row = $this->transformDataToArray($row, 'update'); // Validate data before saving. - if (! $this->skipValidation && ! $this->validate($data)) { + if (! $this->skipValidation && ! $this->validate($row)) { return false; } // Must be called first, so we don't // strip out updated_at values. - $data = $this->doProtectFields($data); + $row = $this->doProtectFields($row); // doProtectFields() can further remove elements from // $data, so we need to check for empty dataset again - if (empty($data)) { + if (empty($row)) { throw DataException::forEmptyDataset('update'); } - $data = $this->setUpdatedField($data, $this->setDate()); + $row = $this->setUpdatedField($row, $this->setDate()); $eventData = [ 'id' => $id, - 'data' => $data, + 'data' => $row, ]; if ($this->tempAllowCallbacks) { @@ -1166,22 +1166,22 @@ public function onlyDeleted() /** * Compiles a replace and runs the query. * - * @param array|null $data Data - * @phpstan-param row_array|null $data + * @param array|null $row Row data + * @phpstan-param row_array|null $row * @param bool $returnSQL Set to true to return Query String * * @return BaseResult|false|Query|string */ - public function replace(?array $data = null, bool $returnSQL = false) + public function replace(?array $row = null, bool $returnSQL = false) { // Validate data before saving. - if (($data !== null) && ! $this->skipValidation && ! $this->validate($data)) { + if (($row !== null) && ! $this->skipValidation && ! $this->validate($row)) { return false; } - $data = $this->setUpdatedField((array) $data, $this->setDate()); + $row = $this->setUpdatedField((array) $row, $this->setDate()); - return $this->doReplace($data, $returnSQL); + return $this->doReplace($row, $returnSQL); } /** @@ -1749,48 +1749,48 @@ protected function objectToRawArray($object, bool $onlyChanged = true, bool $rec /** * Transform data to array. * - * @param array|object|null $data Data - * @phpstan-param row_array|object|null $data + * @param array|object|null $row Row data + * @phpstan-param row_array|object|null $row * @param string $type Type of data (insert|update) * * @throws DataException * @throws InvalidArgumentException * @throws ReflectionException */ - protected function transformDataToArray($data, string $type): array + protected function transformDataToArray($row, string $type): array { if (! in_array($type, ['insert', 'update'], true)) { throw new InvalidArgumentException(sprintf('Invalid type "%s" used upon transforming data to array.', $type)); } - if (! $this->allowEmptyInserts && empty($data)) { + if (! $this->allowEmptyInserts && empty($row)) { throw DataException::forEmptyDataset($type); } // If $data is using a custom class with public or protected // properties representing the collection elements, we need to grab // them as an array. - if (is_object($data) && ! $data instanceof stdClass) { + if (is_object($row) && ! $row instanceof stdClass) { // If it validates with entire rules, all fields are needed. $onlyChanged = ($this->skipValidation === false && $this->cleanValidationRules === false) ? false : ($type === 'update'); - $data = $this->objectToArray($data, $onlyChanged, true); + $row = $this->objectToArray($row, $onlyChanged, true); } // If it's still a stdClass, go ahead and convert to // an array so doProtectFields and other model methods // don't have to do special checks. - if (is_object($data)) { - $data = (array) $data; + if (is_object($row)) { + $row = (array) $row; } // If it's still empty here, means $data is no change or is empty object - if (! $this->allowEmptyInserts && empty($data)) { + if (! $this->allowEmptyInserts && empty($row)) { throw DataException::forEmptyDataset($type); } - return $data; + return $row; } /** diff --git a/system/Model.php b/system/Model.php index 8d8645fa9572..91bbc94a6172 100644 --- a/system/Model.php +++ b/system/Model.php @@ -481,15 +481,15 @@ protected function doOnlyDeleted() * Compiles a replace into string and runs the query * This method works only with dbCalls. * - * @param array|null $data Data - * @phpstan-param row_array|null $data + * @param array|null $row Data + * @phpstan-param row_array|null $row * @param bool $returnSQL Set to true to return Query String * * @return BaseResult|false|Query|string */ - protected function doReplace(?array $data = null, bool $returnSQL = false) + protected function doReplace(?array $row = null, bool $returnSQL = false) { - return $this->builder()->testMode($returnSQL)->replace($data); + return $this->builder()->testMode($returnSQL)->replace($row); } /** @@ -713,8 +713,8 @@ protected function shouldUpdate($data): bool * Inserts data into the database. If an object is provided, * it will attempt to convert it to an array. * - * @param array|object|null $data - * @phpstan-param row_array|object|null $data + * @param array|object|null $row + * @phpstan-param row_array|object|null $row * @param bool $returnID Whether insert ID should be returned or not. * * @return bool|int|string @@ -722,21 +722,21 @@ protected function shouldUpdate($data): bool * * @throws ReflectionException */ - public function insert($data = null, bool $returnID = true) + public function insert($row = null, bool $returnID = true) { if (! empty($this->tempData['data'])) { - if (empty($data)) { - $data = $this->tempData['data']; + if (empty($row)) { + $row = $this->tempData['data']; } else { - $data = $this->transformDataToArray($data, 'insert'); - $data = array_merge($this->tempData['data'], $data); + $row = $this->transformDataToArray($row, 'insert'); + $row = array_merge($this->tempData['data'], $row); } } $this->escape = $this->tempData['escape'] ?? []; $this->tempData = []; - return parent::insert($data, $returnID); + return parent::insert($row, $returnID); } /** @@ -779,26 +779,26 @@ protected function doProtectFieldsForInsert(array $data): array * it will attempt to convert it into an array. * * @param array|int|string|null $id - * @param array|object|null $data - * @phpstan-param row_array|object|null $data + * @param array|object|null $row + * @phpstan-param row_array|object|null $row * * @throws ReflectionException */ - public function update($id = null, $data = null): bool + public function update($id = null, $row = null): bool { if (! empty($this->tempData['data'])) { - if (empty($data)) { - $data = $this->tempData['data']; + if (empty($row)) { + $row = $this->tempData['data']; } else { - $data = $this->transformDataToArray($data, 'update'); - $data = array_merge($this->tempData['data'], $data); + $row = $this->transformDataToArray($row, 'update'); + $row = array_merge($this->tempData['data'], $row); } } $this->escape = $this->tempData['escape'] ?? []; $this->tempData = []; - return parent::update($id, $data); + return parent::update($id, $row); } /** From 7ad3c38ee9f46936d8673148bfb4ce88477ee569 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 28 Nov 2023 13:25:21 +0900 Subject: [PATCH 12/12] refactor: replace $data with $row --- system/BaseModel.php | 107 +++++++++++++++++++++++-------------------- system/Model.php | 68 ++++++++++++++------------- 2 files changed, 94 insertions(+), 81 deletions(-) diff --git a/system/BaseModel.php b/system/BaseModel.php index a0faaddefe0b..0d3df3d465a1 100644 --- a/system/BaseModel.php +++ b/system/BaseModel.php @@ -400,11 +400,12 @@ abstract protected function doFirst(); * Inserts data into the current database. * This method works only with dbCalls. * - * @param array $data Data + * @param array $row Row data + * @phpstan-param row_array $row * * @return bool */ - abstract protected function doInsert(array $data); + abstract protected function doInsert(array $row); /** * Compiles batch insert and runs the queries, validating each row prior. @@ -423,10 +424,11 @@ abstract protected function doInsertBatch(?array $set = null, ?bool $escape = nu * Updates a single record in the database. * This method works only with dbCalls. * - * @param array|int|string|null $id ID - * @param array|null $data Data + * @param array|int|string|null $id ID + * @param array|null $row Row data + * @phpstan-param row_array|null $row */ - abstract protected function doUpdate($id = null, $data = null): bool; + abstract protected function doUpdate($id = null, $row = null): bool; /** * Compiles an update and runs the query. @@ -509,15 +511,16 @@ abstract protected function idValue($data); * Public getter to return the id value using the idValue() method. * For example with SQL this will return $data->$this->primaryKey. * - * @param array|object $data + * @param array|object $row Row data + * @phpstan-param row_array|object $row * * @return array|int|string|null * * @todo: Make abstract in version 5.0 */ - public function getIdValue($data) + public function getIdValue($row) { - return $this->idValue($data); + return $this->idValue($row); } /** @@ -693,20 +696,21 @@ public function first() * you must ensure that the class will provide access to the class * variables, even if through a magic method. * - * @param array|object $data Data + * @param array|object $row Row data + * @phpstan-param row_array|object $row * * @throws ReflectionException */ - public function save($data): bool + public function save($row): bool { - if (empty($data)) { + if (empty($row)) { return true; } - if ($this->shouldUpdate($data)) { - $response = $this->update($this->getIdValue($data), $data); + if ($this->shouldUpdate($row)) { + $response = $this->update($this->getIdValue($row), $row); } else { - $response = $this->insert($data, false); + $response = $this->insert($row, false); if ($response !== false) { $response = true; @@ -720,11 +724,12 @@ public function save($data): bool * This method is called on save to determine if entry have to be updated. * If this method returns false insert operation will be executed * - * @param array|object $data Data + * @param array|object $row Row data + * @phpstan-param row_array|object $row */ - protected function shouldUpdate($data): bool + protected function shouldUpdate($row): bool { - return ! empty($this->getIdValue($data)); + return ! empty($this->getIdValue($row)); } /** @@ -776,7 +781,7 @@ public function insert($row = null, bool $returnID = true) $row = $this->doProtectFieldsForInsert($row); // doProtectFields() can further remove elements from - // $data so we need to check for empty dataset again + // $row, so we need to check for empty dataset again if (! $this->allowEmptyInserts && empty($row)) { throw DataException::forEmptyDataset('insert'); } @@ -867,7 +872,7 @@ public function insertBatch(?array $set = null, ?bool $escape = null, int $batch if (is_array($set)) { foreach ($set as &$row) { - // If $data is using a custom class with public or protected + // If $row is using a custom class with public or protected // properties representing the collection elements, we need to grab // them as an array. if (is_object($row) && ! $row instanceof stdClass) { @@ -958,7 +963,7 @@ public function update($id = null, $row = null): bool $row = $this->doProtectFields($row); // doProtectFields() can further remove elements from - // $data, so we need to check for empty dataset again + // $row, so we need to check for empty dataset again if (empty($row)) { throw DataException::forEmptyDataset('update'); } @@ -1007,7 +1012,7 @@ public function updateBatch(?array $set = null, ?string $index = null, int $batc { if (is_array($set)) { foreach ($set as &$row) { - // If $data is using a custom class with public or protected + // If $row is using a custom class with public or protected // properties representing the collection elements, we need to grab // them as an array. if (is_object($row) && ! $row instanceof stdClass) { @@ -1269,46 +1274,48 @@ public function protect(bool $protect = true) * Ensures that only the fields that are allowed to be updated are * in the data array. * - * Used by update() and updateBatch() to protect against mass assignment - * vulnerabilities. + * @used-by update() to protect against mass assignment vulnerabilities. + * @used-by updateBatch() to protect against mass assignment vulnerabilities. * - * @param array $data Data + * @param array $row Row data + * @phpstan-param row_array $row * * @throws DataException */ - protected function doProtectFields(array $data): array + protected function doProtectFields(array $row): array { if (! $this->protectFields) { - return $data; + return $row; } if (empty($this->allowedFields)) { throw DataException::forInvalidAllowedFields(static::class); } - foreach (array_keys($data) as $key) { + foreach (array_keys($row) as $key) { if (! in_array($key, $this->allowedFields, true)) { - unset($data[$key]); + unset($row[$key]); } } - return $data; + return $row; } /** * Ensures that only the fields that are allowed to be inserted are in * the data array. * - * Used by insert() and insertBatch() to protect against mass assignment - * vulnerabilities. + * @used-by insert() to protect against mass assignment vulnerabilities. + * @used-by insertBatch() to protect against mass assignment vulnerabilities. * - * @param array $data Data + * @param array $row Row data + * @phpstan-param row_array $row * * @throws DataException */ - protected function doProtectFieldsForInsert(array $data): array + protected function doProtectFieldsForInsert(array $row): array { - return $this->doProtectFields($data); + return $this->doProtectFields($row); } /** @@ -1493,25 +1500,26 @@ public function cleanRules(bool $choice = false) } /** - * Validate the data against the validation rules (or the validation group) + * Validate the row data against the validation rules (or the validation group) * specified in the class property, $validationRules. * - * @param array|object $data Data + * @param array|object $row Row data + * @phpstan-param row_array|object $row */ - public function validate($data): bool + public function validate($row): bool { $rules = $this->getValidationRules(); - if ($this->skipValidation || empty($rules) || empty($data)) { + if ($this->skipValidation || empty($rules) || empty($row)) { return true; } // Validation requires array, so cast away. - if (is_object($data)) { - $data = (array) $data; + if (is_object($row)) { + $row = (array) $row; } - $rules = $this->cleanValidationRules ? $this->cleanValidationRules($rules, $data) : $rules; + $rules = $this->cleanValidationRules ? $this->cleanValidationRules($rules, $row) : $rules; // If no data existed that needs validation // our job is done here. @@ -1521,7 +1529,7 @@ public function validate($data): bool $this->validation->reset()->setRules($rules, $this->validationMessages); - return $this->validation->run($data, null, $this->DBGroup); + return $this->validation->run($row, null, $this->DBGroup); } /** @@ -1565,17 +1573,18 @@ public function getValidationMessages(): array * currently so that rules don't block updating when only updating * a partial row. * - * @param array $rules Array containing field name and rule - * @param array|null $data Data + * @param array $rules Array containing field name and rule + * @param array $row Row data (@TODO Remove null in param type) + * @phpstan-param row_array $row */ - protected function cleanValidationRules(array $rules, ?array $data = null): array + protected function cleanValidationRules(array $rules, ?array $row = null): array { - if (empty($data)) { + if (empty($row)) { return []; } foreach (array_keys($rules) as $field) { - if (! array_key_exists($field, $data)) { + if (! array_key_exists($field, $row)) { unset($rules[$field]); } } @@ -1767,7 +1776,7 @@ protected function transformDataToArray($row, string $type): array throw DataException::forEmptyDataset($type); } - // If $data is using a custom class with public or protected + // If $row is using a custom class with public or protected // properties representing the collection elements, we need to grab // them as an array. if (is_object($row) && ! $row instanceof stdClass) { @@ -1785,7 +1794,7 @@ protected function transformDataToArray($row, string $type): array $row = (array) $row; } - // If it's still empty here, means $data is no change or is empty object + // If it's still empty here, means $row is no change or is empty object if (! $this->allowEmptyInserts && empty($row)) { throw DataException::forEmptyDataset($type); } diff --git a/system/Model.php b/system/Model.php index 91bbc94a6172..4c55aac937bf 100644 --- a/system/Model.php +++ b/system/Model.php @@ -274,29 +274,30 @@ protected function doFirst() * Inserts data into the current table. * This method works only with dbCalls. * - * @param array $data Data + * @param array $row Row data + * @phpstan-param row_array $row * * @return bool */ - protected function doInsert(array $data) + protected function doInsert(array $row) { $escape = $this->escape; $this->escape = []; // Require non-empty primaryKey when // not using auto-increment feature - if (! $this->useAutoIncrement && empty($data[$this->primaryKey])) { + if (! $this->useAutoIncrement && empty($row[$this->primaryKey])) { throw DataException::forEmptyPrimaryKey('insert'); } $builder = $this->builder(); // Must use the set() method to ensure to set the correct escape flag - foreach ($data as $key => $val) { + foreach ($row as $key => $val) { $builder->set($key, $val, $escape[$key] ?? null); } - if ($this->allowEmptyInserts && empty($data)) { + if ($this->allowEmptyInserts && empty($row)) { $table = $this->db->protectIdentifiers($this->table, true, null, false); if ($this->db->getPlatform() === 'MySQLi') { $sql = 'INSERT INTO ' . $table . ' VALUES ()'; @@ -327,7 +328,7 @@ protected function doInsert(array $data) // If insertion succeeded then save the insert ID if ($result) { - $this->insertID = ! $this->useAutoIncrement ? $data[$this->primaryKey] : $this->db->insertID(); + $this->insertID = ! $this->useAutoIncrement ? $row[$this->primaryKey] : $this->db->insertID(); } return $result; @@ -364,9 +365,10 @@ protected function doInsertBatch(?array $set = null, ?bool $escape = null, int $ * This method works only with dbCalls. * * @param array|int|string|null $id - * @param array|null $data + * @param array|null $row Row data + * @phpstan-param row_array|null $row */ - protected function doUpdate($id = null, $data = null): bool + protected function doUpdate($id = null, $row = null): bool { $escape = $this->escape; $this->escape = []; @@ -378,7 +380,7 @@ protected function doUpdate($id = null, $data = null): bool } // Must use the set() method to ensure to set the correct escape flag - foreach ($data as $key => $val) { + foreach ($row as $key => $val) { $builder->set($key, $val, $escape[$key] ?? null); } @@ -529,33 +531,34 @@ protected function idValue($data) /** * Returns the id value for the data array or object * - * @param array|object $data Data + * @param array|object $row Row data + * @phpstan-param row_array|object $row * * @return array|int|string|null */ - public function getIdValue($data) + public function getIdValue($row) { - if (is_object($data) && isset($data->{$this->primaryKey})) { + if (is_object($row) && isset($row->{$this->primaryKey})) { // Get the raw primary key value of the Entity. - if ($data instanceof Entity) { - $cast = $data->cast(); + if ($row instanceof Entity) { + $cast = $row->cast(); // Disable Entity casting, because raw primary key value is needed for database. - $data->cast(false); + $row->cast(false); - $primaryKey = $data->{$this->primaryKey}; + $primaryKey = $row->{$this->primaryKey}; // Restore Entity casting setting. - $data->cast($cast); + $row->cast($cast); return $primaryKey; } - return $data->{$this->primaryKey}; + return $row->{$this->primaryKey}; } - if (is_array($data) && ! empty($data[$this->primaryKey])) { - return $data[$this->primaryKey]; + if (is_array($row) && ! empty($row[$this->primaryKey])) { + return $row[$this->primaryKey]; } return null; @@ -692,11 +695,11 @@ public function set($key, $value = '', ?bool $escape = null) * This method is called on save to determine if entry have to be updated * If this method return false insert operation will be executed * - * @param array|object $data Data + * @param array|object $row Data */ - protected function shouldUpdate($data): bool + protected function shouldUpdate($row): bool { - if (parent::shouldUpdate($data) === false) { + if (parent::shouldUpdate($row) === false) { return false; } @@ -706,7 +709,7 @@ protected function shouldUpdate($data): bool // When useAutoIncrement feature is disabled, check // in the database if given record already exists - return $this->where($this->primaryKey, $this->getIdValue($data))->countAllResults() === 1; + return $this->where($this->primaryKey, $this->getIdValue($row))->countAllResults() === 1; } /** @@ -743,35 +746,36 @@ public function insert($row = null, bool $returnID = true) * Ensures that only the fields that are allowed to be inserted are in * the data array. * - * Used by insert() and insertBatch() to protect against mass assignment - * vulnerabilities. + * @used-by insert() to protect against mass assignment vulnerabilities. + * @used-by insertBatch() to protect against mass assignment vulnerabilities. * - * @param array $data Data + * @param array $row Row data + * @phpstan-param row_array $row * * @throws DataException */ - protected function doProtectFieldsForInsert(array $data): array + protected function doProtectFieldsForInsert(array $row): array { if (! $this->protectFields) { - return $data; + return $row; } if (empty($this->allowedFields)) { throw DataException::forInvalidAllowedFields(static::class); } - foreach (array_keys($data) as $key) { + foreach (array_keys($row) as $key) { // Do not remove the non-auto-incrementing primary key data. if ($this->useAutoIncrement === false && $key === $this->primaryKey) { continue; } if (! in_array($key, $this->allowedFields, true)) { - unset($data[$key]); + unset($row[$key]); } } - return $data; + return $row; } /**