Model::tryLoad returns null when not found (#996)

atk4 · May 29, 2022 · bde5290 · bde5290
1 parent d650c11
commit bde5290
Show file tree

Hide file tree

Showing 24 changed files with 154 additions and 156 deletions.
diff --git a/composer.json b/composer.json
@@ -37,20 +37,20 @@
         "php": ">=7.4 <8.2",
         "atk4/core": "dev-develop",
         "doctrine/dbal": "^3.3",
-        "mvorisek/atk4-hintable": "~1.7.1"
+        "mvorisek/atk4-hintable": "~1.8.0"
     },
     "require-release": {
         "php": ">=7.4 <8.2",
         "atk4/core": "~3.2.0",
         "doctrine/dbal": "^3.3",
-        "mvorisek/atk4-hintable": "~1.7.1"
+        "mvorisek/atk4-hintable": "~1.8.0"
     },
     "require-dev": {
         "ergebnis/composer-normalize": "^2.13",
         "friendsofphp/php-cs-fixer": "^3.0",
         "johnkary/phpunit-speedtrap": "^3.3",
         "phpstan/extension-installer": "^1.1",
-        "phpstan/phpstan": "^1.0",
+        "phpstan/phpstan": "^1.6",
         "phpstan/phpstan-deprecation-rules": "^1.0",
         "phpunit/phpunit": "^9.5.5"
     },

diff --git a/docs/advanced.rst b/docs/advanced.rst
@@ -166,7 +166,7 @@ which I want to define like this::
 
         $this->getOwner()->addField('updated_dts', ['type' => 'datetime']);
 
-        $this->getOwner()->onHook(Model::HOOK_BEFORE_UPDATE, function($m, $data) {
+        $this->getOwner()->onHook(Model::HOOK_BEFORE_UPDATE, function ($m, $data) {
             if(isset($this->getApp()->user) && $this->getApp()->user->isLoaded()) {
                 $data['updated_by'] = $this->getApp()->user->getId();
             }
@@ -440,7 +440,7 @@ inside your model are unique::
                     $mm->addCondition($mm->id_field != $this->id);
                     $mm = $mm->tryLoadBy($field, $m->get($field));
 
-                    if ($mm->isLoaded()) {
+                    if ($mm !== null) {
                         throw (new \Atk4\Core\Exception('Duplicate record exists'))
                             ->addMoreInfo('field', $field)
                             ->addMoreInfo('value', $m->get($field));
@@ -514,7 +514,7 @@ Next we need to define reference. Inside Model_Invoice add::
 
     $this->hasMany('InvoicePayment');
 
-    $this->hasMany('Payment', ['model' => function($m) {
+    $this->hasMany('Payment', ['model' => function ($m) {
         $p = new Model_Payment($m->persistence);
         $j = $p->join('invoice_payment.payment_id');
         $j->addField('amount_closed');
@@ -570,24 +570,23 @@ payment towards a most suitable invoice::
 
         while($this->get('amount_due') > 0) {
 
-            // See if any invoices match by 'reference';
-            $invoices = $invoices->tryLoadBy('reference', $this->get('reference'));
+            // see if any invoices match by 'reference'
+            $invoice = $invoices->tryLoadBy('reference', $this->get('reference'));
 
-            if (!$invoices->isLoaded()) {
+            if ($invoice === null) {
 
                 // otherwise load any unpaid invoice
-                $invoices = $invoices->tryLoadAny();
+                $invoice = $invoices->tryLoadAny();
 
-                if(!$invoices->isLoaded()) {
-
-                    // couldn't load any invoice.
+                if ($invoice === null) {
+                    // couldn't load any invoice
                     return;
                 }
             }
 
             // How much we can allocate to this invoice
-            $alloc = min($this->get('amount_due'), $invoices->get('amount_due'))
-            $this->ref('InvoicePayment')->insert(['amount_closed' => $alloc, 'invoice_id' => $invoices->getId()]);
+            $alloc = min($this->get('amount_due'), $invoice->get('amount_due'))
+            $this->ref('InvoicePayment')->insert(['amount_closed' => $alloc, 'invoice_id' => $invoice->getId()]);
 
             // Reload ourselves to refresh amount_due
             $this->reload();
@@ -744,7 +743,7 @@ section. Add this into your Invoice Model::
 Next both payment and lines need to be added after invoice is actually created,
 so::
 
-    $this->onHookShort(Model::HOOK_AFTER_SAVE, function($is_update) {
+    $this->onHookShort(Model::HOOK_AFTER_SAVE, function ($is_update) {
         if($this->_isset('payment')) {
             $this->ref('Payment')->insert($this->get('payment'));
         }
@@ -800,7 +799,7 @@ field only to offer payments made by the same client. Inside Model_Invoice add::
 
     $this->hasOne('client_id', 'Client');
 
-    $this->hasOne('payment_invoice_id', ['model' => function($m) {
+    $this->hasOne('payment_invoice_id', ['model' => function ($m) {
         return $m->ref('client_id')->ref('Payment');
     }]);
 
@@ -809,13 +808,13 @@ field only to offer payments made by the same client. Inside Model_Invoice add::
     $m = new Model_Invoice($db);
     $m->set('client_id', 123);
 
-    $m->set('payment_invoice_id', $m->ref('payment_invoice_id')->tryLoadOne()->getId());
+    $m->set('payment_invoice_id', $m->ref('payment_invoice_id')->loadOne()->getId());
 
 In this case the payment_invoice_id will be set to ID of any payment by client
 123. There also may be some better uses::
 
     foreach ($cl->ref('Invoice') as $m) {
-        $m->set('payment_invoice_id', $m->ref('payment_invoice_id')->tryLoadOne()->getId());
+        $m->set('payment_invoice_id', $m->ref('payment_invoice_id')->loadOne()->getId());
         $m->save();
     }
 
@@ -826,7 +825,7 @@ Agile Data allow you to define multiple references between same entities, but
 sometimes that can be quite useful. Consider adding this inside your Model_Contact::
 
     $this->hasMany('Invoice', 'Model_Invoice');
-    $this->hasMany('OverdueInvoice', ['model' => function($m) {
+    $this->hasMany('OverdueInvoice', ['model' => function ($m) {
         return $m->ref('Invoice')->addCondition('due', '<', date('Y-m-d'))
     }]);
 

diff --git a/docs/hooks.rst b/docs/hooks.rst
@@ -52,7 +52,7 @@ Example with beforeSave
 The next code snippet demonstrates a basic usage of a `beforeSave` hook.
 This one will update field values just before record is saved::
 
-    $m->onHook(Model::HOOK_BEFORE_SAVE, function($m) {
+    $m->onHook(Model::HOOK_BEFORE_SAVE, function ($m) {
         $m->set('name', strtoupper($m->get('name')));
         $m->set('surname', strtoupper($m->get('surname')));
     });
@@ -136,7 +136,7 @@ of save.
 
 You may actually drop validation exception inside save, insert or update hooks::
 
-    $m->onHook(Model::HOOK_BEFORE_SAVE, function($m) {
+    $m->onHook(Model::HOOK_BEFORE_SAVE, function ($m) {
         if ($m->get('name') === 'Yagi') {
             throw new \Atk4\Data\ValidationException(['name' => "We don't serve like you"]);
         }
@@ -193,7 +193,7 @@ and your update() may not actually update anything. This does not normally
 generate an error, however if you want to actually make sure that update() was
 effective, you can implement this through a hook::
 
-    $m->onHook(Persistence\Sql::HOOK_AFTER_UPDATE_QUERY, function($m, $update, $c) {
+    $m->onHook(Persistence\Sql::HOOK_AFTER_UPDATE_QUERY, function ($m, $update, $c) {
         if ($c === 0) {
             throw (new \Atk4\Core\Exception('Update didn\'t affect any records'))
                 ->addMoreInfo('query', $update->getDebugQuery())
@@ -210,7 +210,7 @@ In some cases you want to prevent default actions from executing.
 Suppose you want to check 'memcache' before actually loading the record from
 the database. Here is how you can implement this functionality::
 
-    $m->onHook(Model::HOOK_BEFORE_LOAD, function($m, $id) {
+    $m->onHook(Model::HOOK_BEFORE_LOAD, function ($m, $id) {
         $data = $m->getApp()->cacheFetch($m->table, $id);
         if ($data) {
             $dataRef = &$m->getDataRef();
@@ -237,15 +237,15 @@ This can be used in various situations.
 
 Save information into auditLog about failure:
 
-    $m->onHook(Model::HOOK_ROLLBACK, function($m) {
+    $m->onHook(Model::HOOK_ROLLBACK, function ($m) {
         $m->auditLog->registerFailure();
     });
 
 Upgrade schema:
 
     use Atk4\Data\Persistence\Sql\Exception as SqlException;
 
-    $m->onHook(Model::HOOK_ROLLBACK, function($m, $exception) {
+    $m->onHook(Model::HOOK_ROLLBACK, function ($m, $exception) {
         if ($exception instanceof SqlException) {
             $m->schema->upgrade();
             $m->breakHook(false); // exception will not be thrown

diff --git a/docs/model.rst b/docs/model.rst
@@ -180,7 +180,7 @@ To invoke code from `init()` methods of ALL models (for example soft-delete logi
 you use Persistence's "afterAdd" hook. This will not affect ALL models but just models
 which are associated with said persistence::
 
-   $db->onHook(Persistence::HOOK_AFTER_ADD, function($p, $m) use($acl) {
+   $db->onHook(Persistence::HOOK_AFTER_ADD, function ($p, $m) use ($acl) {
 
       $fields = $m->getFields();
 

diff --git a/docs/overview.rst b/docs/overview.rst
@@ -166,7 +166,7 @@ If your persistence does not support expressions (e.g. you are using Redis or
 MongoDB), you would need to define the field differently::
 
     $model->addField('gross');
-    $model->onHook(Model::HOOK_BEFORE_SAVE, function($m) {
+    $model->onHook(Model::HOOK_BEFORE_SAVE, function ($m) {
         $m->set('gross', $m->get('net') + $m->get('vat'));
     });
 
@@ -186,7 +186,7 @@ you want it to work with NoSQL, then your solution might be::
 
         // persistence does not support expressions
         $model->addField('gross');
-        $model->onHook(Model::HOOK_BEFORE_SAVE, function($m) {
+        $model->onHook(Model::HOOK_BEFORE_SAVE, function ($m) {
             $m->set('gross', $m->get('net') + $m->get('vat'));
         });
 

diff --git a/docs/persistence.rst b/docs/persistence.rst
@@ -66,7 +66,7 @@ There are several ways to link your model up with the persistence::
 
 .. php:method:: tryLoad
 
-    Same as load() but will silently fail if record is not found::
+    Same as load() but will return null if record is not found::
 
         $m = $m->tryLoad(10);
         $m->setMulti($data);
@@ -482,7 +482,7 @@ Start by creating a beforeSave handler for Order::
                     ->addCondition('client_id', $this->get('client_id'))  // same client
                     ->addCondition($this->id_field, '!=', $this->getId()) // has another order
                     ->tryLoadBy('ref', $this->get('ref'))                 // with same ref
-                    ->isLoaded()
+                    !== null
             ) {
                 throw (new Exception('Order with ref already exists for this client'))
                     ->addMoreInfo('client', $this->get('client_id'))
@@ -580,20 +580,19 @@ application::
         // first, try to load it from MemCache
         $m = $this->mdb->add(clone $class)->tryLoad($id);
 
-        if (!$m->isLoaded()) {
-
+        if ($m === null) {
             // fall-back to load from SQL
             $m = $this->sql->add(clone $class)->load($id);
 
             // store into MemCache too
             $m = $m->withPersistence($this->mdb)->save();
         }
 
-        $m->onHook(Model::HOOK_BEFORE_SAVE, function($m) {
+        $m->onHook(Model::HOOK_BEFORE_SAVE, function ($m) {
             $m->withPersistence($this->sql)->save();
         });
 
-        $m->onHook(Model::HOOK_BEFORE_DELETE, function($m) {
+        $m->onHook(Model::HOOK_BEFORE_DELETE, function ($m) {
             $m->withPersistence($this->sql)->delete();
         });
 
@@ -618,8 +617,7 @@ use a string). It will first be associated with the MemCache DB persistence and
 we will attempt to load a corresponding ID. Next, if no record is found in the
 cache::
 
-    if (!$m->isLoaded()) {
-
+    if ($m === null) {
         // fall-back to load from SQL
         $m = $this->sql->add(clone $class)->load($id);
 
@@ -634,11 +632,11 @@ records.
 The last two hooks are in order to replicate any changes into the SQL database
 also::
 
-    $m->onHook(Model::HOOK_BEFORE_SAVE, function($m) {
+    $m->onHook(Model::HOOK_BEFORE_SAVE, function ($m) {
         $m->withPersistence($this->sql)->save();
     });
 
-    $m->onHook(Model::HOOK_BEFORE_DELETE, function($m) {
+    $m->onHook(Model::HOOK_BEFORE_DELETE, function ($m) {
         $m->withPersistence($this->sql)->delete();
     });
 
@@ -690,7 +688,7 @@ Archive Copies into different persistence
 If you wish that every time you save your model the copy is also stored inside
 some other database (for archive purposes) you can implement it like this::
 
-    $m->onHook(Model::HOOK_BEFORE_SAVE, function($m) {
+    $m->onHook(Model::HOOK_BEFORE_SAVE, function ($m) {
         $arc = $this->withPersistence($m->getApp()->archive_db, false);
 
         // add some audit fields

diff --git a/docs/references.rst b/docs/references.rst
@@ -106,7 +106,7 @@ There are several ways how to link models with hasMany::
 
     $m->hasMany('Orders', ['model' => [Model_Order::class]]); // using seed
 
-    $m->hasMany('Order', ['model' => function($m, $r) {   // using callback
+    $m->hasMany('Order', ['model' => function ($m, $r) {   // using callback
         return new Model_Order();
     }]);
 
@@ -433,7 +433,7 @@ User-defined Reference
 Sometimes you would want to have a different type of relation between models,
 so with `addRef` you can define whatever reference you want::
 
-    $m->addRef('Archive', ['model' => function($m) {
+    $m->addRef('Archive', ['model' => function ($m) {
         return $m->newInstance(null, ['table' => $m->table . '_archive']);
     }]);
 
@@ -447,7 +447,7 @@ Note that you can create one-to-many or many-to-one relations, by using your
 custom logic.
 No condition will be applied by default so it's all up to you::
 
-    $m->addRef('Archive', ['model' => function($m) {
+    $m->addRef('Archive', ['model' => function ($m) {
         $archive = $m->newInstance(null, ['table' => $m->table . '_archive']);
 
         $m->addField('original_id', ['type' => 'integer']);

diff --git a/docs/types.rst b/docs/types.rst
@@ -92,7 +92,7 @@ ATK Data prior to 1.5 supports the following types:
 
 In ATK Data the number of supported types has been extended with:
 
- - percent (34.2%) ([':php:class:`Number`', 'format' => function($v) { return $v * 100; }, 'postfix' => '%'])
+ - percent (34.2%) ([':php:class:`Number`', 'format' => function ($v) { return $v * 100; }, 'postfix' => '%'])
  - rating (3 out of 5) ([':php:class:`Number`', 'max' => 5, 'precision' => 0])
  - uuid (xxxxxxxx-xxxx-...) ([':php:class:`Number`', 'base' => 16, 'mask' => '########-##..'])
  - hex (number with base 16) ([':php:class:`Number`', 'base' => 16])

diff --git a/src/Model.php b/src/Model.php
@@ -91,9 +91,9 @@ class Model implements \IteratorAggregate
     public const HOOK_ONLY_FIELDS = self::class . '@onlyFields';
 
     /** @const string */
-    protected const ID_LOAD_ONE = self::class . '@idLoadOne';
+    protected const ID_LOAD_ONE = self::class . '@idLoadOne-h7axmDNBB3qVXjVv';
     /** @const string */
-    protected const ID_LOAD_ANY = self::class . '@idLoadAny';
+    protected const ID_LOAD_ANY = self::class . '@idLoadAny-h7axmDNBB3qVXjVv';
 
     // {{{ Properties of the class
 
@@ -1234,9 +1234,9 @@ private function remapIdLoadToPersistence($id)
     /**
      * @param mixed $id
      *
-     * @return $this
+     * @return ($fromTryLoad is true ? $this|null : $this)
      */
-    private function _loadThis(bool $isTryLoad, $id)
+    private function _loadThis(bool $fromTryLoad, $id)
     {
         $this->assertIsEntity();
         if ($this->isLoaded()) {
@@ -1250,21 +1250,27 @@ private function _loadThis(bool $isTryLoad, $id)
             return $this;
         }
         $dataRef = &$this->getDataRef();
-        $dataRef = $this->persistence->{$isTryLoad ? 'tryLoad' : 'load'}($this->getModel(), $this->remapIdLoadToPersistence($id));
-        if ($isTryLoad && $dataRef === null) {
-            $dataRef = [];
-            $this->unload();
-        } else {
-            if ($this->id_field) {
-                $this->setId($this->getId());
-            }
+        $dataRef = $this->persistence->{$fromTryLoad ? 'tryLoad' : 'load'}($this->getModel(), $this->remapIdLoadToPersistence($id));
 
-            $ret = $this->hook(self::HOOK_AFTER_LOAD);
-            if ($ret === false) {
-                $this->unload();
-            } elseif (is_object($ret)) {
-                return $ret; // @phpstan-ignore-line
+        if ($dataRef === null) {
+            // $fromTryLoad is always true here
+
+            return null;
+        }
+
+        if ($this->id_field) {
+            $this->setId($this->getId());
+        }
+
+        $ret = $this->hook(self::HOOK_AFTER_LOAD);
+        if ($ret === false) {
+            if ($fromTryLoad) {
+                return null;
             }
+
+            $this->unload();
+        } elseif (is_object($ret)) {
+            return $ret; // @phpstan-ignore-line
         }
 
         return $this;

diff --git a/src/Reference/ContainsOne.php b/src/Reference/ContainsOne.php
@@ -94,7 +94,14 @@ public function ref(Model $ourModel, array $defaults = []): Model
             });
         }
 
-        $theirModel = $theirModel->tryLoadOne();
+        if ($ourModel->isEntity()) {
+            $theirModelOrig = $theirModel;
+            $theirModel = $theirModel->tryLoadOne();
+
+            if ($theirModel === null) { // TODO or implement tryRef?
+                $theirModel = $theirModelOrig->createEntity();
+            }
+        }
 
         return $theirModel;
     }