Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 23b883d

Browse files
authoredApr 9, 2025··
Merge pull request #16735 from mpadinhabrandao/improve-clone-models
Add null attribute check in cloneResultMap method
2 parents c99cd56 + a43ed82 commit 23b883d

File tree

7 files changed

+306
-3
lines changed

7 files changed

+306
-3
lines changed
 

Diff for: ‎phalcon/Mvc/Model.zep

+16-3
Original file line numberDiff line numberDiff line change
@@ -924,9 +924,16 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface,
924924
*/
925925
public static function cloneResultMap(var base, array! data, var columnMap, int dirtyState = 0, bool keepSnapshots = null) -> <ModelInterface>
926926
{
927-
var instance, attribute, key, value, castValue, attributeName, metaData, reverseMap;
927+
var instance, attribute, key, value, castValue, attributeName, metaData, reverseMap, notNullAttributes;
928928

929929
let instance = clone base;
930+
if instance instanceof Model {
931+
let metaData = instance->getModelsMetaData();
932+
let notNullAttributes = metaData->getNotNullAttributes(instance);
933+
} else {
934+
let metaData = null;
935+
let notNullAttributes = [];
936+
}
930937

931938
// Change the dirty state to persistent
932939
instance->setDirtyState(dirtyState);
@@ -940,6 +947,10 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface,
940947
continue;
941948
}
942949

950+
if value === null && in_array(key, notNullAttributes) {
951+
continue;
952+
}
953+
943954
if typeof columnMap !== "array" {
944955
let instance->{key} = value;
945956

@@ -949,8 +960,10 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface,
949960
// Every field must be part of the column map
950961
if !fetch attribute, columnMap[key] {
951962
if typeof columnMap === "array" && !empty columnMap {
952-
let metaData = instance->getModelsMetaData();
953-
963+
if metaData === null {
964+
let metaData = instance->getModelsMetaData();
965+
}
966+
954967
let reverseMap = metaData->getReverseColumnMap(instance);
955968
if !fetch attribute, reverseMap[key] {
956969
if unlikely !globals_get("orm.ignore_unknown_columns") {

Diff for: ‎tests/_data/assets/schemas/mysql.sql

+13
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,17 @@ create index co_invoices_inv_created_at_index
173173

174174

175175

176+
drop table if exists `co_manufacturers`;
177+
CREATE TABLE `co_manufacturers` (
178+
`id` int(10) unsigned NOT NULL AUTO_INCREMENT,
179+
`name` VARCHAR(100) NOT NULL,
180+
`country` VARCHAR(100) NULL,
181+
`founded_year` INT NOT NULL,
182+
PRIMARY KEY (`id`)
183+
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
184+
185+
186+
176187
drop table if exists objects;
177188

178189
create table objects
@@ -239,6 +250,8 @@ CREATE TABLE `co_products` (
239250
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
240251

241252

253+
DROP TABLE IF EXISTS co_rb_test_model;CREATE TABLE co_rb_test_model (id SMALLINT, name VARCHAR(10) NOT NULL);
254+
242255

243256
drop table if exists `co_setters`;
244257

Diff for: ‎tests/_data/assets/schemas/pgsql.sql

+15
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,19 @@ create index co_invoices_inv_status_flag_index
9898

9999

100100

101+
drop table if exists co_manufacturers;
102+
create table co_manufacturers
103+
(
104+
id serial not null
105+
constraint manufacturers_pk
106+
primary key,
107+
name varchar(100) not null,
108+
country varchar(100) null,
109+
founded_year int not null
110+
);
111+
112+
113+
101114
drop table if exists objects;
102115

103116
create table objects
@@ -145,6 +158,8 @@ create table co_products
145158
);
146159

147160

161+
DROP TABLE IF EXISTS co_rb_test_model;CREATE TABLE co_rb_test_model (id SMALLINT, name VARCHAR(10) NOT NULL);
162+
148163

149164
drop table if exists co_setters;
150165

Diff for: ‎tests/_data/assets/schemas/sqlite.sql

+12
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,16 @@ create index co_invoices_inv_created_at_index
7575

7676

7777

78+
drop table if exists co_manufacturers;
79+
create table co_manufacturers (
80+
id integer constraint co_manufacturers_pk primary key autoincrement,
81+
name text not null,
82+
country text null,
83+
founded_year integer not null
84+
);
85+
86+
87+
7888
drop table if exists `objects`;
7989

8090
create table objects
@@ -94,6 +104,8 @@ create table objects
94104

95105

96106

107+
108+
97109
drop table if exists co_setters;
98110

99111
create table co_setters
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Phalcon\Tests\Fixtures\Migrations;
6+
7+
/**
8+
* Class ManufacturersMigration
9+
*/
10+
class ManufacturersMigration extends AbstractMigration
11+
{
12+
protected $table = "co_manufacturers";
13+
14+
/**
15+
* @param int $id
16+
* @param string $name
17+
* @param string|null $country
18+
* @param int $foundedYear
19+
*
20+
* @return int
21+
*/
22+
public function insert(
23+
$id,
24+
string $name,
25+
?string $country,
26+
int $foundedYear
27+
): int {
28+
$id = $id ?: 'null';
29+
$country = $country ? "'{$country}'" : 'null';
30+
$sql = <<<SQL
31+
insert into co_manufacturers (
32+
id, name, country, founded_year
33+
) values (
34+
{$id}, '{$name}', {$country}, {$foundedYear}
35+
)
36+
SQL;
37+
38+
return $this->connection->exec($sql);
39+
}
40+
41+
protected function getSqlMysql(): array
42+
{
43+
return [
44+
"
45+
drop table if exists `co_manufacturers`;",
46+
"
47+
CREATE TABLE `co_manufacturers` (
48+
`id` int(10) unsigned NOT NULL AUTO_INCREMENT,
49+
`name` VARCHAR(100) NOT NULL,
50+
`country` VARCHAR(100) NULL,
51+
`founded_year` INT NOT NULL,
52+
PRIMARY KEY (`id`)
53+
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
54+
"
55+
];
56+
}
57+
58+
protected function getSqlSqlite(): array
59+
{
60+
return [
61+
"
62+
drop table if exists co_manufacturers;",
63+
"
64+
create table co_manufacturers (
65+
id integer constraint co_manufacturers_pk primary key autoincrement,
66+
name text not null,
67+
country text null,
68+
founded_year integer not null
69+
);
70+
"
71+
];
72+
}
73+
74+
protected function getSqlPgsql(): array
75+
{
76+
return [
77+
"
78+
drop table if exists co_manufacturers;",
79+
"
80+
create table co_manufacturers
81+
(
82+
id serial not null
83+
constraint manufacturers_pk
84+
primary key,
85+
name varchar(100) not null,
86+
country varchar(100) null,
87+
founded_year int not null
88+
);
89+
"
90+
];
91+
}
92+
93+
protected function getSqlSqlsrv(): array
94+
{
95+
return [];
96+
}
97+
}

Diff for: ‎tests/_data/fixtures/models/Manufacturer.php

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Phalcon\Tests\Models;
6+
7+
use Phalcon\Mvc\Model;
8+
9+
/**
10+
* Class Manufacturer
11+
*
12+
* @property int $id;
13+
* @property string $name;
14+
* @property string $country;
15+
* @property int $founded_year;
16+
*/
17+
class Manufacturer extends Model
18+
{
19+
public ?int $id;
20+
21+
public string $name;
22+
23+
public string $country = 'UK';
24+
25+
public ?int $founded_year;
26+
27+
public function initialize()
28+
{
29+
$this->setSource('co_manufacturers');
30+
}
31+
32+
public function columnMap()
33+
{
34+
return [
35+
'id' => 'id',
36+
'name' => 'name',
37+
'country' => 'country',
38+
'founded_year' => 'founded_year',
39+
];
40+
}
41+
}

Diff for: ‎tests/database/Mvc/Model/CloneResultMapCest.php

+112
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Phalcon\Tests\Fixtures\Migrations\InvoicesMigration;
2121
use Phalcon\Tests\Fixtures\Traits\DiTrait;
2222
use Phalcon\Tests\Models\InvoicesMap;
23+
use Phalcon\Tests\Models\Manufacturer;
2324

2425
/**
2526
* Class CloneResultMapCest
@@ -210,6 +211,89 @@ public function cloneResultMapWithCasting(DatabaseTester $I, Example $example)
210211
);
211212
}
212213

214+
/**
215+
* Tests Phalcon\Mvc\Model :: cloneResultMapModelTyped()
216+
*
217+
* @dataProvider modelManufactureDataProvider
218+
*
219+
* @param DatabaseTester $I
220+
* @param Example $example
221+
*
222+
* @author Phalcon Team <team@phalcon.io>
223+
* @since 2020-10-05
224+
*
225+
* @group mysql
226+
* @group pgsql
227+
* @group sqlite
228+
*/
229+
public function cloneResultMapModelTyped(DatabaseTester $I, Example $example)
230+
{
231+
$I->wantToTest('Mvc\Model - cloneResultMapModelTyped() - with model arguments typed');
232+
233+
$base = new Manufacturer();
234+
235+
/**
236+
* @var Model\MetaData $metaData
237+
*/
238+
$metaData = $base->getModelsMetaData();
239+
$columnMap = $metaData->getColumnMap($base);
240+
241+
$data = [
242+
'id' => $example['id'],
243+
'name' => $example['name'],
244+
'country' => $example['country'],
245+
'founded_year' => $example['founded_year'],
246+
];
247+
248+
$data = array_filter($data);
249+
250+
/**
251+
* @var Manufacturer $manufacturer
252+
*/
253+
$manufacturer = Model::cloneResultMap(
254+
$base,
255+
$data,
256+
$columnMap
257+
);
258+
259+
260+
if ($example['founded_year'] === null) {
261+
$I->expectThrowable(
262+
\Error::class,
263+
function () use ($manufacturer) {
264+
$manufacturer->founded_year;
265+
}
266+
);
267+
} else {
268+
$I->assertEquals(
269+
$example['founded_year'],
270+
$manufacturer->founded_year
271+
);
272+
}
273+
274+
if ($example['country'] === null) {
275+
$I->assertEquals(
276+
[
277+
'id' => $example['id'],
278+
'name' => $example['name'],
279+
'country' => 'UK',
280+
'founded_year' => $example['founded_year'],
281+
],
282+
$manufacturer->toArray()
283+
);
284+
} else {
285+
$I->assertEquals(
286+
[
287+
'id' => $example['id'],
288+
'name' => $example['name'],
289+
'country' => $example['country'],
290+
'founded_year' => $example['founded_year'],
291+
],
292+
$manufacturer->toArray()
293+
);
294+
}
295+
}
296+
213297
/**
214298
* @return array
215299
*/
@@ -234,4 +318,32 @@ protected function modelDataProvider(): array
234318
]
235319
];
236320
}
321+
322+
/**
323+
* @return array
324+
*/
325+
protected function modelManufactureDataProvider(): array
326+
{
327+
return [
328+
[
329+
'id' => 1,
330+
'name' => 'Test Manufacturer',
331+
'founded_year' => 1990,
332+
'country' => 'UK'
333+
],
334+
[
335+
'id' => 2,
336+
'name' => 'Another Manufacturer',
337+
'founded_year' => 2000,
338+
'country' => null
339+
],
340+
[
341+
'id' => 3,
342+
'name' => 'Third Manufacturer',
343+
'founded_year' => null,
344+
'country' => 'CA'
345+
]
346+
347+
];
348+
}
237349
}

0 commit comments

Comments
 (0)
Please sign in to comment.