Skip to content

Commit bde5290

Browse files
mvorisekmvorisek
authored
Model::tryLoad returns null when not found (#996)
1 parent d650c11 commit bde5290

24 files changed

+154
-156
lines changed

‎composer.json

+3-3
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,20 @@
3737
"php": ">=7.4 <8.2",
3838
"atk4/core": "dev-develop",
3939
"doctrine/dbal": "^3.3",
40-
"mvorisek/atk4-hintable": "~1.7.1"
40+
"mvorisek/atk4-hintable": "~1.8.0"
4141
},
4242
"require-release": {
4343
"php": ">=7.4 <8.2",
4444
"atk4/core": "~3.2.0",
4545
"doctrine/dbal": "^3.3",
46-
"mvorisek/atk4-hintable": "~1.7.1"
46+
"mvorisek/atk4-hintable": "~1.8.0"
4747
},
4848
"require-dev": {
4949
"ergebnis/composer-normalize": "^2.13",
5050
"friendsofphp/php-cs-fixer": "^3.0",
5151
"johnkary/phpunit-speedtrap": "^3.3",
5252
"phpstan/extension-installer": "^1.1",
53-
"phpstan/phpstan": "^1.0",
53+
"phpstan/phpstan": "^1.6",
5454
"phpstan/phpstan-deprecation-rules": "^1.0",
5555
"phpunit/phpunit": "^9.5.5"
5656
},

‎docs/advanced.rst

+16-17
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ which I want to define like this::
166166

167167
$this->getOwner()->addField('updated_dts', ['type' => 'datetime']);
168168

169-
$this->getOwner()->onHook(Model::HOOK_BEFORE_UPDATE, function($m, $data) {
169+
$this->getOwner()->onHook(Model::HOOK_BEFORE_UPDATE, function ($m, $data) {
170170
if(isset($this->getApp()->user) && $this->getApp()->user->isLoaded()) {
171171
$data['updated_by'] = $this->getApp()->user->getId();
172172
}
@@ -440,7 +440,7 @@ inside your model are unique::
440440
$mm->addCondition($mm->id_field != $this->id);
441441
$mm = $mm->tryLoadBy($field, $m->get($field));
442442

443-
if ($mm->isLoaded()) {
443+
if ($mm !== null) {
444444
throw (new \Atk4\Core\Exception('Duplicate record exists'))
445445
->addMoreInfo('field', $field)
446446
->addMoreInfo('value', $m->get($field));
@@ -514,7 +514,7 @@ Next we need to define reference. Inside Model_Invoice add::
514514

515515
$this->hasMany('InvoicePayment');
516516

517-
$this->hasMany('Payment', ['model' => function($m) {
517+
$this->hasMany('Payment', ['model' => function ($m) {
518518
$p = new Model_Payment($m->persistence);
519519
$j = $p->join('invoice_payment.payment_id');
520520
$j->addField('amount_closed');
@@ -570,24 +570,23 @@ payment towards a most suitable invoice::
570570

571571
while($this->get('amount_due') > 0) {
572572

573-
// See if any invoices match by 'reference';
574-
$invoices = $invoices->tryLoadBy('reference', $this->get('reference'));
573+
// see if any invoices match by 'reference'
574+
$invoice = $invoices->tryLoadBy('reference', $this->get('reference'));
575575

576-
if (!$invoices->isLoaded()) {
576+
if ($invoice === null) {
577577

578578
// otherwise load any unpaid invoice
579-
$invoices = $invoices->tryLoadAny();
579+
$invoice = $invoices->tryLoadAny();
580580

581-
if(!$invoices->isLoaded()) {
582-
583-
// couldn't load any invoice.
581+
if ($invoice === null) {
582+
// couldn't load any invoice
584583
return;
585584
}
586585
}
587586

588587
// How much we can allocate to this invoice
589-
$alloc = min($this->get('amount_due'), $invoices->get('amount_due'))
590-
$this->ref('InvoicePayment')->insert(['amount_closed' => $alloc, 'invoice_id' => $invoices->getId()]);
588+
$alloc = min($this->get('amount_due'), $invoice->get('amount_due'))
589+
$this->ref('InvoicePayment')->insert(['amount_closed' => $alloc, 'invoice_id' => $invoice->getId()]);
591590

592591
// Reload ourselves to refresh amount_due
593592
$this->reload();
@@ -744,7 +743,7 @@ section. Add this into your Invoice Model::
744743
Next both payment and lines need to be added after invoice is actually created,
745744
so::
746745

747-
$this->onHookShort(Model::HOOK_AFTER_SAVE, function($is_update) {
746+
$this->onHookShort(Model::HOOK_AFTER_SAVE, function ($is_update) {
748747
if($this->_isset('payment')) {
749748
$this->ref('Payment')->insert($this->get('payment'));
750749
}
@@ -800,7 +799,7 @@ field only to offer payments made by the same client. Inside Model_Invoice add::
800799

801800
$this->hasOne('client_id', 'Client');
802801

803-
$this->hasOne('payment_invoice_id', ['model' => function($m) {
802+
$this->hasOne('payment_invoice_id', ['model' => function ($m) {
804803
return $m->ref('client_id')->ref('Payment');
805804
}]);
806805

@@ -809,13 +808,13 @@ field only to offer payments made by the same client. Inside Model_Invoice add::
809808
$m = new Model_Invoice($db);
810809
$m->set('client_id', 123);
811810

812-
$m->set('payment_invoice_id', $m->ref('payment_invoice_id')->tryLoadOne()->getId());
811+
$m->set('payment_invoice_id', $m->ref('payment_invoice_id')->loadOne()->getId());
813812

814813
In this case the payment_invoice_id will be set to ID of any payment by client
815814
123. There also may be some better uses::
816815

817816
foreach ($cl->ref('Invoice') as $m) {
818-
$m->set('payment_invoice_id', $m->ref('payment_invoice_id')->tryLoadOne()->getId());
817+
$m->set('payment_invoice_id', $m->ref('payment_invoice_id')->loadOne()->getId());
819818
$m->save();
820819
}
821820

@@ -826,7 +825,7 @@ Agile Data allow you to define multiple references between same entities, but
826825
sometimes that can be quite useful. Consider adding this inside your Model_Contact::
827826

828827
$this->hasMany('Invoice', 'Model_Invoice');
829-
$this->hasMany('OverdueInvoice', ['model' => function($m) {
828+
$this->hasMany('OverdueInvoice', ['model' => function ($m) {
830829
return $m->ref('Invoice')->addCondition('due', '<', date('Y-m-d'))
831830
}]);
832831

‎docs/hooks.rst

+6-6
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ Example with beforeSave
5252
The next code snippet demonstrates a basic usage of a `beforeSave` hook.
5353
This one will update field values just before record is saved::
5454

55-
$m->onHook(Model::HOOK_BEFORE_SAVE, function($m) {
55+
$m->onHook(Model::HOOK_BEFORE_SAVE, function ($m) {
5656
$m->set('name', strtoupper($m->get('name')));
5757
$m->set('surname', strtoupper($m->get('surname')));
5858
});
@@ -136,7 +136,7 @@ of save.
136136

137137
You may actually drop validation exception inside save, insert or update hooks::
138138

139-
$m->onHook(Model::HOOK_BEFORE_SAVE, function($m) {
139+
$m->onHook(Model::HOOK_BEFORE_SAVE, function ($m) {
140140
if ($m->get('name') === 'Yagi') {
141141
throw new \Atk4\Data\ValidationException(['name' => "We don't serve like you"]);
142142
}
@@ -193,7 +193,7 @@ and your update() may not actually update anything. This does not normally
193193
generate an error, however if you want to actually make sure that update() was
194194
effective, you can implement this through a hook::
195195

196-
$m->onHook(Persistence\Sql::HOOK_AFTER_UPDATE_QUERY, function($m, $update, $c) {
196+
$m->onHook(Persistence\Sql::HOOK_AFTER_UPDATE_QUERY, function ($m, $update, $c) {
197197
if ($c === 0) {
198198
throw (new \Atk4\Core\Exception('Update didn\'t affect any records'))
199199
->addMoreInfo('query', $update->getDebugQuery())
@@ -210,7 +210,7 @@ In some cases you want to prevent default actions from executing.
210210
Suppose you want to check 'memcache' before actually loading the record from
211211
the database. Here is how you can implement this functionality::
212212

213-
$m->onHook(Model::HOOK_BEFORE_LOAD, function($m, $id) {
213+
$m->onHook(Model::HOOK_BEFORE_LOAD, function ($m, $id) {
214214
$data = $m->getApp()->cacheFetch($m->table, $id);
215215
if ($data) {
216216
$dataRef = &$m->getDataRef();
@@ -237,15 +237,15 @@ This can be used in various situations.
237237

238238
Save information into auditLog about failure:
239239

240-
$m->onHook(Model::HOOK_ROLLBACK, function($m) {
240+
$m->onHook(Model::HOOK_ROLLBACK, function ($m) {
241241
$m->auditLog->registerFailure();
242242
});
243243

244244
Upgrade schema:
245245

246246
use Atk4\Data\Persistence\Sql\Exception as SqlException;
247247

248-
$m->onHook(Model::HOOK_ROLLBACK, function($m, $exception) {
248+
$m->onHook(Model::HOOK_ROLLBACK, function ($m, $exception) {
249249
if ($exception instanceof SqlException) {
250250
$m->schema->upgrade();
251251
$m->breakHook(false); // exception will not be thrown

‎docs/model.rst

+1-1
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ To invoke code from `init()` methods of ALL models (for example soft-delete logi
180180
you use Persistence's "afterAdd" hook. This will not affect ALL models but just models
181181
which are associated with said persistence::
182182

183-
$db->onHook(Persistence::HOOK_AFTER_ADD, function($p, $m) use($acl) {
183+
$db->onHook(Persistence::HOOK_AFTER_ADD, function ($p, $m) use ($acl) {
184184

185185
$fields = $m->getFields();
186186

‎docs/overview.rst

+2-2
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ If your persistence does not support expressions (e.g. you are using Redis or
166166
MongoDB), you would need to define the field differently::
167167

168168
$model->addField('gross');
169-
$model->onHook(Model::HOOK_BEFORE_SAVE, function($m) {
169+
$model->onHook(Model::HOOK_BEFORE_SAVE, function ($m) {
170170
$m->set('gross', $m->get('net') + $m->get('vat'));
171171
});
172172

@@ -186,7 +186,7 @@ you want it to work with NoSQL, then your solution might be::
186186

187187
// persistence does not support expressions
188188
$model->addField('gross');
189-
$model->onHook(Model::HOOK_BEFORE_SAVE, function($m) {
189+
$model->onHook(Model::HOOK_BEFORE_SAVE, function ($m) {
190190
$m->set('gross', $m->get('net') + $m->get('vat'));
191191
});
192192

‎docs/persistence.rst

+9-11
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ There are several ways to link your model up with the persistence::
6666

6767
.. php:method:: tryLoad
6868
69-
Same as load() but will silently fail if record is not found::
69+
Same as load() but will return null if record is not found::
7070

7171
$m = $m->tryLoad(10);
7272
$m->setMulti($data);
@@ -482,7 +482,7 @@ Start by creating a beforeSave handler for Order::
482482
->addCondition('client_id', $this->get('client_id')) // same client
483483
->addCondition($this->id_field, '!=', $this->getId()) // has another order
484484
->tryLoadBy('ref', $this->get('ref')) // with same ref
485-
->isLoaded()
485+
!== null
486486
) {
487487
throw (new Exception('Order with ref already exists for this client'))
488488
->addMoreInfo('client', $this->get('client_id'))
@@ -580,20 +580,19 @@ application::
580580
// first, try to load it from MemCache
581581
$m = $this->mdb->add(clone $class)->tryLoad($id);
582582

583-
if (!$m->isLoaded()) {
584-
583+
if ($m === null) {
585584
// fall-back to load from SQL
586585
$m = $this->sql->add(clone $class)->load($id);
587586

588587
// store into MemCache too
589588
$m = $m->withPersistence($this->mdb)->save();
590589
}
591590

592-
$m->onHook(Model::HOOK_BEFORE_SAVE, function($m) {
591+
$m->onHook(Model::HOOK_BEFORE_SAVE, function ($m) {
593592
$m->withPersistence($this->sql)->save();
594593
});
595594

596-
$m->onHook(Model::HOOK_BEFORE_DELETE, function($m) {
595+
$m->onHook(Model::HOOK_BEFORE_DELETE, function ($m) {
597596
$m->withPersistence($this->sql)->delete();
598597
});
599598

@@ -618,8 +617,7 @@ use a string). It will first be associated with the MemCache DB persistence and
618617
we will attempt to load a corresponding ID. Next, if no record is found in the
619618
cache::
620619

621-
if (!$m->isLoaded()) {
622-
620+
if ($m === null) {
623621
// fall-back to load from SQL
624622
$m = $this->sql->add(clone $class)->load($id);
625623

@@ -634,11 +632,11 @@ records.
634632
The last two hooks are in order to replicate any changes into the SQL database
635633
also::
636634

637-
$m->onHook(Model::HOOK_BEFORE_SAVE, function($m) {
635+
$m->onHook(Model::HOOK_BEFORE_SAVE, function ($m) {
638636
$m->withPersistence($this->sql)->save();
639637
});
640638

641-
$m->onHook(Model::HOOK_BEFORE_DELETE, function($m) {
639+
$m->onHook(Model::HOOK_BEFORE_DELETE, function ($m) {
642640
$m->withPersistence($this->sql)->delete();
643641
});
644642

@@ -690,7 +688,7 @@ Archive Copies into different persistence
690688
If you wish that every time you save your model the copy is also stored inside
691689
some other database (for archive purposes) you can implement it like this::
692690

693-
$m->onHook(Model::HOOK_BEFORE_SAVE, function($m) {
691+
$m->onHook(Model::HOOK_BEFORE_SAVE, function ($m) {
694692
$arc = $this->withPersistence($m->getApp()->archive_db, false);
695693

696694
// add some audit fields

‎docs/references.rst

+3-3
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ There are several ways how to link models with hasMany::
106106

107107
$m->hasMany('Orders', ['model' => [Model_Order::class]]); // using seed
108108

109-
$m->hasMany('Order', ['model' => function($m, $r) { // using callback
109+
$m->hasMany('Order', ['model' => function ($m, $r) { // using callback
110110
return new Model_Order();
111111
}]);
112112

@@ -433,7 +433,7 @@ User-defined Reference
433433
Sometimes you would want to have a different type of relation between models,
434434
so with `addRef` you can define whatever reference you want::
435435

436-
$m->addRef('Archive', ['model' => function($m) {
436+
$m->addRef('Archive', ['model' => function ($m) {
437437
return $m->newInstance(null, ['table' => $m->table . '_archive']);
438438
}]);
439439

@@ -447,7 +447,7 @@ Note that you can create one-to-many or many-to-one relations, by using your
447447
custom logic.
448448
No condition will be applied by default so it's all up to you::
449449

450-
$m->addRef('Archive', ['model' => function($m) {
450+
$m->addRef('Archive', ['model' => function ($m) {
451451
$archive = $m->newInstance(null, ['table' => $m->table . '_archive']);
452452

453453
$m->addField('original_id', ['type' => 'integer']);

‎docs/types.rst

+1-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ ATK Data prior to 1.5 supports the following types:
9292

9393
In ATK Data the number of supported types has been extended with:
9494

95-
- percent (34.2%) ([':php:class:`Number`', 'format' => function($v) { return $v * 100; }, 'postfix' => '%'])
95+
- percent (34.2%) ([':php:class:`Number`', 'format' => function ($v) { return $v * 100; }, 'postfix' => '%'])
9696
- rating (3 out of 5) ([':php:class:`Number`', 'max' => 5, 'precision' => 0])
9797
- uuid (xxxxxxxx-xxxx-...) ([':php:class:`Number`', 'base' => 16, 'mask' => '########-##..'])
9898
- hex (number with base 16) ([':php:class:`Number`', 'base' => 16])

‎src/Model.php

+23-17
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,9 @@ class Model implements \IteratorAggregate
9191
public const HOOK_ONLY_FIELDS = self::class . '@onlyFields';
9292

9393
/** @const string */
94-
protected const ID_LOAD_ONE = self::class . '@idLoadOne';
94+
protected const ID_LOAD_ONE = self::class . '@idLoadOne-h7axmDNBB3qVXjVv';
9595
/** @const string */
96-
protected const ID_LOAD_ANY = self::class . '@idLoadAny';
96+
protected const ID_LOAD_ANY = self::class . '@idLoadAny-h7axmDNBB3qVXjVv';
9797

9898
// {{{ Properties of the class
9999

@@ -1234,9 +1234,9 @@ private function remapIdLoadToPersistence($id)
12341234
/**
12351235
* @param mixed $id
12361236
*
1237-
* @return $this
1237+
* @return ($fromTryLoad is true ? $this|null : $this)
12381238
*/
1239-
private function _loadThis(bool $isTryLoad, $id)
1239+
private function _loadThis(bool $fromTryLoad, $id)
12401240
{
12411241
$this->assertIsEntity();
12421242
if ($this->isLoaded()) {
@@ -1250,21 +1250,27 @@ private function _loadThis(bool $isTryLoad, $id)
12501250
return $this;
12511251
}
12521252
$dataRef = &$this->getDataRef();
1253-
$dataRef = $this->persistence->{$isTryLoad ? 'tryLoad' : 'load'}($this->getModel(), $this->remapIdLoadToPersistence($id));
1254-
if ($isTryLoad && $dataRef === null) {
1255-
$dataRef = [];
1256-
$this->unload();
1257-
} else {
1258-
if ($this->id_field) {
1259-
$this->setId($this->getId());
1260-
}
1253+
$dataRef = $this->persistence->{$fromTryLoad ? 'tryLoad' : 'load'}($this->getModel(), $this->remapIdLoadToPersistence($id));
12611254

1262-
$ret = $this->hook(self::HOOK_AFTER_LOAD);
1263-
if ($ret === false) {
1264-
$this->unload();
1265-
} elseif (is_object($ret)) {
1266-
return $ret; // @phpstan-ignore-line
1255+
if ($dataRef === null) {
1256+
// $fromTryLoad is always true here
1257+
1258+
return null;
1259+
}
1260+
1261+
if ($this->id_field) {
1262+
$this->setId($this->getId());
1263+
}
1264+
1265+
$ret = $this->hook(self::HOOK_AFTER_LOAD);
1266+
if ($ret === false) {
1267+
if ($fromTryLoad) {
1268+
return null;
12671269
}
1270+
1271+
$this->unload();
1272+
} elseif (is_object($ret)) {
1273+
return $ret; // @phpstan-ignore-line
12681274
}
12691275

12701276
return $this;

‎src/Reference/ContainsOne.php

+8-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,14 @@ public function ref(Model $ourModel, array $defaults = []): Model
9494
});
9595
}
9696

97-
$theirModel = $theirModel->tryLoadOne();
97+
if ($ourModel->isEntity()) {
98+
$theirModelOrig = $theirModel;
99+
$theirModel = $theirModel->tryLoadOne();
100+
101+
if ($theirModel === null) { // TODO or implement tryRef?
102+
$theirModel = $theirModelOrig->createEntity();
103+
}
104+
}
98105

99106
return $theirModel;
100107
}

0 commit comments

Comments
 (0)