Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize ID in array persistence and fix type guessing in static persistence #911

Merged
merged 7 commits into from
Nov 12, 2021
Merged

Normalize ID in array persistence and fix type guessing in static persistence #911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ec4ffde
fix seeding with empty table array persistence data
mvorisek Nov 12, 2021
0093834
do not use hook in static persistence
mvorisek Nov 12, 2021
ba955ca
fix id field type guessing & setup in static persistence
mvorisek Nov 12, 2021
b91ce41
Normalize ID in array persistence
mvorisek Nov 5, 2021
23f0ac5
adjust tests
mvorisek Nov 12, 2021
319d064
fix stan
mvorisek Nov 12, 2021
3d5823d
improve test
mvorisek Nov 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/Persistence/Array_.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public function __construct(array $data = [])
// and put all persistence data in there 1/2
if (count($this->seedData) > 0 && !isset($this->seedData['data'])) {
$rowSample = reset($this->seedData);
if (is_array($rowSample) && !is_array(reset($rowSample))) {
if (is_array($rowSample) && $rowSample !== [] && !is_array(reset($rowSample))) {
$this->seedData = ['data' => $this->seedData];
}
}
Expand Down Expand Up @@ -111,9 +111,9 @@ public function getRawDataByTable(Model $model, string $table): array
* @param int|string|null $idFromRow
* @param int|string $id
*/
private function assertNoIdMismatch($idFromRow, $id): void
private function assertNoIdMismatch(Model $model, $idFromRow, $id): void
{
if ($idFromRow !== null && (is_int($idFromRow) ? (string) $idFromRow : $idFromRow) !== (is_int($id) ? (string) $id : $id)) {
if ($idFromRow !== null && !$model->getField($model->id_field)->compare($idFromRow, $id)) {
throw (new Exception('Row constains ID column, but it does not match the row ID'))
->addMoreInfo('idFromKey', $id)
->addMoreInfo('idFromData', $idFromRow);
Expand All @@ -127,9 +127,10 @@ private function saveRow(Model $model, array $rowData, $id): void
{
if ($model->id_field) {
$idField = $model->getField($model->id_field);
$id = $idField->normalize($id);
$idColumnName = $idField->getPersistenceName();
if (array_key_exists($idColumnName, $rowData)) {
$this->assertNoIdMismatch($rowData[$idColumnName], $id);
$this->assertNoIdMismatch($model, $rowData[$idColumnName], $id);
unset($rowData[$idColumnName]);
}

Expand Down Expand Up @@ -179,7 +180,7 @@ public function add(Model $model, array $defaults = []): Model

if ($model->id_field && $model->hasField($model->id_field)) {
$f = $model->getField($model->id_field);
if (!$f->type) {
if ($f->type === null) {
$f->type = 'integer';
}
}
Expand Down
49 changes: 39 additions & 10 deletions src/Persistence/Static_.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,23 @@ public function __construct(array $data = null)
// chomp off first row, we will use it to deduct fields
$row1 = reset($data);

$this->onHookShort(self::HOOK_AFTER_ADD, function (...$args) {
$this->afterAdd(...$args);
});

if (!is_array($row1)) {
// convert array of strings into array of hashes
$allKeysInt = true;
foreach ($data as $k => $str) {
$data[$k] = ['name' => $str];

if (!is_int($k)) {
$allKeysInt = false;
}
}
unset($str);

$this->titleForModel = 'name';
$this->fieldsForModel = ['name' => []];
$this->fieldsForModel = [
'id' => ['type' => $allKeysInt ? 'integer' : 'string'],
'name' => ['type' => 'string'], // TODO type should be guessed as well
];

parent::__construct($data);

Expand Down Expand Up @@ -89,7 +93,7 @@ public function __construct(array $data = null)
} elseif (is_object($value)) {
$def_types[] = ['type' => 'object'];
} else {
$def_types[] = [];
$def_types[] = ['type' => 'string'];
}

// if title is not set, use first key
Expand Down Expand Up @@ -132,12 +136,38 @@ public function __construct(array $data = null)
parent::__construct($data);
}

public function add(Model $model, array $defaults = []): Model
{
if ($model->id_field && !$model->hasField($model->id_field)) {
// init model, but prevent array persistence data seeding, id field with correct type must be setup first
\Closure::bind(function () use ($model, $defaults) {
$hadData = true;
if (!isset($this->data[$model->table])) {
$hadData = false;
$this->data[$model->table] = true; // @phpstan-ignore-line
}
try {
parent::add($model, $defaults);
} finally {
if (!$hadData) {
unset($this->data[$model->table]);
}
}
}, $this, Array_::class)();

if (isset($this->fieldsForModel[$model->id_field])) {
$model->getField($model->id_field)->type = $this->fieldsForModel[$model->id_field]['type'];
}
}
$this->addMissingFieldsToModel($model);

return parent::add($model, $defaults);
}

/**
* Automatically adds missing model fields.
*
* Called by HOOK_AFTER_ADD hook.
*/
public function afterAdd(Model $model): void
protected function addMissingFieldsToModel(Model $model): void
{
if ($this->titleForModel) {
$model->title_field = $this->titleForModel;
Expand All @@ -148,7 +178,6 @@ public function afterAdd(Model $model): void
continue;
}

// add new field
$model->addField($field, $def);
}
}
Expand Down
27 changes: 0 additions & 27 deletions tests/FieldHereditaryTest.php
View file
Open in desktop

This file was deleted.

18 changes: 9 additions & 9 deletions tests/Persistence/ArrayTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,10 +700,10 @@ public function testLimit(): void
{
// order by one field ascending
$p = new Persistence\Array_([
['f1' => 'A'],
['f1' => 'D'],
['f1' => 'E'],
['f1' => 'C'],
1 => ['f1' => 'A'],
2 => ['f1' => 'D'],
3 => ['f1' => 'E'],
4 => ['f1' => 'C'],
]);
$m = new Model($p);
$m->addField('f1');
Expand All @@ -713,16 +713,16 @@ public function testLimit(): void
$m->setLimit(3);
$this->assertSame(3, $m->action('count')->getOne());
$this->assertSame([
['id' => 0, 'f1' => 'A'],
['id' => 1, 'f1' => 'D'],
['id' => 2, 'f1' => 'E'],
['id' => 1, 'f1' => 'A'],
['id' => 2, 'f1' => 'D'],
['id' => 3, 'f1' => 'E'],
], array_values($m->export()));

$m->setLimit(2, 1);
$this->assertSame(2, $m->action('count')->getOne());
$this->assertSame([
['id' => 1, 'f1' => 'D'],
['id' => 2, 'f1' => 'E'],
['id' => 2, 'f1' => 'D'],
['id' => 3, 'f1' => 'E'],
], array_values($m->export()));

// well, this is strange, that you can actually change limit on-the-fly and then previous
Expand Down
81 changes: 63 additions & 18 deletions tests/Persistence/StaticTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,25 @@ class StaticTest extends TestCase
{
public function testBasicStatic(): void
{
$p = new Persistence\Static_(['hello', 'world']);
$p = new Persistence\Static_([1 => 'hello', 'world']);

// default title field
$m = new Model($p);
$m = $m->load(1);
$m = $m->load(2);
$this->assertSame('world', $m->get('name'));

// custom title field and try loading from same static twice
$m = new Model($p); //, ['title_field' => 'foo']);
$m = $m->load(1);
$m = $m->load(2);
$this->assertSame('world', $m->get('name')); // still 'name' here not 'foo'
}

public function testArrayOfArrays(): void
{
$p = new Persistence\Static_([['hello', 'xx', true], ['world', 'xy', false]]);
$p = new Persistence\Static_([1 => ['hello', 'xx', true], ['world', 'xy', false]]);
$m = new Model($p);

$m = $m->load(1);
$m = $m->load(2);

$this->assertSame('world', $m->get('name'));
$this->assertSame('xy', $m->get('field1'));
Expand All @@ -39,10 +39,10 @@ public function testArrayOfArrays(): void

public function testArrayOfHashes(): void
{
$p = new Persistence\Static_([['foo' => 'hello'], ['foo' => 'world']]);
$p = new Persistence\Static_([1 => ['foo' => 'hello'], ['foo' => 'world']]);
$m = new Model($p);

$m = $m->load(1);
$m = $m->load(2);

$this->assertSame('world', $m->get('foo'));
}
Expand All @@ -67,6 +67,32 @@ public function testIdKey(): void
$this->assertSame('world', $m->get('foo'));
}

public function testZeroIdAllowed(): void
{
$p = new Persistence\Static_(['hello', 'world']);
$m = new class($p) extends Model {
protected function init(): void
{
parent::init();

$this->getField('id')->required = false;
$this->getField('id')->mandatory = true;
}
};

$this->assertSame('hello', $m->load(0)->get('name'));
$this->assertSame('world', $m->load(1)->get('name'));
}

public function testZeroIdNotAllowed(): void
{
$p = new Persistence\Static_(['hello', 'world']);

$this->expectException(\Atk4\Data\Exception::class);
$this->expectExceptionMessage('Must not be a zero');
$m = new Model($p);
}

public function testEmpty(): void
{
$p = new Persistence\Static_([]);
Expand All @@ -79,34 +105,34 @@ public function testEmpty(): void

public function testCustomField(): void
{
$p = new Persistence\Static_([['foo' => 'hello'], ['foo' => 'world']]);
$p = new Persistence\Static_([1 => ['foo' => 'hello'], ['foo' => 'world']]);
$m = new StaticPersistenceModel($p);

$this->assertSame('custom field', $m->getField('foo')->caption);

$p = new Persistence\Static_([['foo' => 'hello', 'bar' => 'world']]);
$p = new Persistence\Static_([1 => ['foo' => 'hello', 'bar' => 'world']]);
$m = new StaticPersistenceModel($p);
$this->assertSame('foo', $m->title_field);
}

public function testTitleOrName(): void
{
$p = new Persistence\Static_([['foo' => 'hello', 'bar' => 'world']]);
$p = new Persistence\Static_([1 => ['foo' => 'hello', 'bar' => 'world']]);
$m = new Model($p);
$this->assertSame('foo', $m->title_field);

$p = new Persistence\Static_([['foo' => 'hello', 'name' => 'x']]);
$p = new Persistence\Static_([1 => ['foo' => 'hello', 'name' => 'x']]);
$m = new Model($p);
$this->assertSame('name', $m->title_field);

$p = new Persistence\Static_([['foo' => 'hello', 'title' => 'x']]);
$p = new Persistence\Static_([1 => ['foo' => 'hello', 'title' => 'x']]);
$m = new Model($p);
$this->assertSame('title', $m->title_field);
}

public function testFieldTypes(): void
{
$p = new Persistence\Static_([[
$p = new Persistence\Static_([1 => [
'name' => 'hello',
'test_int' => 123,
'test_float' => 123.45,
Expand All @@ -119,17 +145,36 @@ public function testFieldTypes(): void
]]);
$m = new Model($p);

$this->assertSame('integer', $m->getField('id')->type);
$this->assertSame('integer', $m->getField('test_int')->type);
$this->assertSame('float', $m->getField('test_float')->type);
// $this->assertSame('datetime', $m->getField('test_date')->type);
// $this->assertSame('json', $m->getField('test_array')->type);
// $this->assertSame('object', $m->getField('test_object')->type);

// string is default type, so it is null
$this->assertNull($m->getField('name')->type);
$this->assertNull($m->getField('test_str_1')->type);
$this->assertNull($m->getField('test_str_2')->type);
$this->assertNull($m->getField('test_str_3')->type);
// string is default type
$this->assertSame('string', $m->getField('name')->type);
$this->assertSame('string', $m->getField('test_str_1')->type);
$this->assertSame('string', $m->getField('test_str_2')->type);
$this->assertSame('string', $m->getField('test_str_3')->type);
}

public function testFieldTypesBasicInteger(): void
{
$p = new Persistence\Static_([1 => 'hello', 'world']);
$m = new Model($p);

$this->assertSame('integer', $m->getField('id')->type);
$this->assertSame('string', $m->getField('name')->type);
}

public function testFieldTypesBasicString(): void
{
$p = new Persistence\Static_(['test' => 'hello', 10 => 'world']);
$m = new Model($p);

$this->assertSame('string', $m->getField('id')->type);
$this->assertSame('string', $m->getField('name')->type);
}
}

Expand Down
15 changes: 15 additions & 0 deletions tests/RandomTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,21 @@ public function testSameTable3(): void
$this->assertSame('John', $m->load(2)->ref('parent_item_id', ['table_alias' => 'pp'])->get('name'));
}

public function testDirty2(): void
{
$p = new Persistence\Static_([1 => 'hello', 'world']);

// default title field
$m = new Model($p);
$m->addExpression('caps', function ($m) {
return strtoupper($m->get('name'));
});

$m = $m->load(2);
$this->assertSame('world', $m->get('name'));
$this->assertSame('WORLD', $m->get('caps'));
}

public function testUpdateCondition(): void
{
$db = new Persistence\Sql($this->db->connection);
Expand Down