Pass Persistence to their model closure factory only (#1185)

Author: mvorisek

docs/advanced.md

@@ -471,11 +471,13 @@ Next we need to define reference. Inside Model_Invoice add:
 ```
 $this->hasMany('InvoicePayment', ['model' => [Model_InvoicePayment::class]]);
 
-$this->hasMany('Payment', ['model' => function (self $m) {
-    $p = new Model_Payment($m->getPersistence());
+$this->hasMany('Payment', ['model' => static function (Persistence $persistence) {
+    $p = new Model_Payment($persistence);
     $j = $p->join('invoice_payment.payment_id');
     $j->addField('amount_closed');
     $j->hasOne('invoice_id', ['model' => [Model_Invoice::class]]);
+
+    return $p;
 }, 'theirField' => 'invoice_id']);
 
 $this->onHookShort(Model::HOOK_BEFORE_DELETE, function () {
@@ -724,7 +726,7 @@ $this->hasOne('client_id', ['model' => [$this->client_class]]);
 Alternatively you can replace model in the init() method of Model_Invoice:
 
 ```
-$this->getReference('client_id')->model = 'Model_Client';
+$this->getReference('client_id')->model = [Model_Client::class];
 ```
 
 You can also use array here if you wish to pass additional information into
@@ -737,53 +739,25 @@ $this->getReference('client_id')->model = ['Model_Client', 'no_audit' => true];
 Combined with our "Audit" handler above, this should allow you to relate
 with deleted clients.
 
-The final use case is when some value inside the existing model should be
-passed into the related model. Let's say we have 'Model_Invoice' and we want to
-add 'payment_invoice_id' that points to 'Model_Payment'. However we want this
-field only to offer payments made by the same client. Inside Model_Invoice add:
-
-```
-$this->hasOne('client_id', ['model' => [Model_Client::class]]);
-
-$this->hasOne('payment_invoice_id', ['model' => function (self $m) {
-    return $m->ref('client_id')->ref('Payment');
-}]);
-
-/// how to use
-
-$m = new Model_Invoice($db);
-$m->set('client_id', 123);
-
-$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')->loadOne()->getId());
-    $m->save();
-}
-```
-
 ## Narrowing Down Existing References
 
 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' => [Model_Invoice::class]]);
-$this->hasMany('OverdueInvoice', ['model' => function (self $m) {
-    return $m->ref('Invoice')->addCondition('due', '<', date('Y-m-d'))
+
+$this->hasMany('OverdueInvoice', ['model' => static function (Persistence $persistence) {
+    return (new Model_Invoice($persistence))
+        ->addCondition('due', '<', new \DateTime());
 }]);
 ```
 
 This way if you extend your class into 'Model_Client' and modify the 'Invoice'
 reference to use different model:
 
 ```
-$this->getReference('Invoice')->model = 'Model_Invoice_Sale';
+$this->getReference('Invoice')->model = [Model_Invoice_Sale::class];
 ```
 
 The 'OverdueInvoice' reference will be also properly adjusted.

docs/model.md

@@ -197,8 +197,8 @@ you use Persistence's "afterAdd" hook. This will not affect ALL models but just
 which are associated with said persistence:
 
 ```
-$db->onHook(Persistence::HOOK_AFTER_ADD, function (Persistence $p, Model $m) use ($acl) {
-    $fields = $m->getFields();
+$db->onHook(Persistence::HOOK_AFTER_ADD, function (Persistence $persistence, Model $model) use ($acl) {
+    $fields = $model->getFields();
 
     $acl->disableRestrictedFields($fields);
 });

docs/references.md

@@ -118,7 +118,7 @@ There are several ways how to link models with hasMany:
 ```
 $m->hasMany('Orders', ['model' => [Model_Order::class]]); // using seed
 
-$m->hasMany('Order', ['model' => static function (Model $m, Reference $r, array $defaults) { // using callback
+$m->hasMany('Order', ['model' => static function (Persistence $persistence, array $defaults) { // using callback
     return new Model_Order();
 }]);
 ```
@@ -471,7 +471,7 @@ Sometimes you would want to have a different type of relation between models,
 so with `addReference` you can define whatever reference you want:
 
 ```
-$m->addReference('Archive', ['model' => function (Model $m) {
+$m->addReference('Archive', ['model' => static function (Persistence $persistence) use ($m) {
     return new $m(null, ['table' => $m->table . '_archive']);
 }]);
 ```
@@ -489,7 +489,7 @@ custom logic.
 No condition will be applied by default so it's all up to you:
 
 ```
-$m->addReference('Archive', ['model' => function (Model $m) {
+$m->addReference('Archive', ['model' => static function (Persistence $persistence) use ($m) {
     $archive = new $m(null, ['table' => $m->table . '_archive']);
 
     $m->addField('original_id', ['type' => 'integer']);

src/Reference.php

@@ -47,7 +47,7 @@ class Reference
      * Seed of their model. If it is a Model instance, self::createTheirModel() must
      * always clone it to return a new instance.
      *
-     * @var Model|\Closure(object, static, array<string, mixed>): Model|array<mixed>
+     * @var array<mixed>|\Closure(Persistence, array<string, mixed>): Model|Model
      */
     protected $model;
 
@@ -231,7 +231,7 @@ protected function initTableAlias(): void
      *
      * @return Persistence|false
      */
-    protected function getDefaultPersistence(Model $theirModel)
+    protected function getDefaultPersistence()
     {
         $ourModel = $this->getOurModel();
 
@@ -256,7 +256,8 @@ protected function createTheirModelBeforeInit(array $defaults): Model
 
         // if model is Closure, then call the closure and it should return a model
         if ($this->model instanceof \Closure) {
-            $m = ($this->model)($this->getOurModel(), $this, $defaults);
+            $persistence = Persistence::assertInstanceOf($this->getDefaultPersistence());
+            $m = ($this->model)($persistence, $defaults);
         } else {
             $m = $this->model;
         }
@@ -278,7 +279,7 @@ protected function createTheirModelBeforeInit(array $defaults): Model
     protected function createTheirModelSetPersistence(Model $theirModel): void
     {
         if (!$theirModel->issetPersistence()) {
-            $persistence = $this->getDefaultPersistence($theirModel);
+            $persistence = $this->getDefaultPersistence();
             if ($persistence !== false) {
                 $theirModel->setPersistence($persistence);
             }

src/Reference/ContainsBase.php

@@ -71,7 +71,7 @@ protected function init(): void
     }
 
     #[\Override]
-    protected function getDefaultPersistence(Model $theirModel): Persistence
+    protected function getDefaultPersistence(): Persistence
     {
         return new Persistence\Array_();
     }

tests/ReferenceSqlTest.php

@@ -6,6 +6,7 @@
 
 use Atk4\Data\Exception;
 use Atk4\Data\Model;
+use Atk4\Data\Persistence;
 use Atk4\Data\Schema\TestCase;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Platforms\SQLServerPlatform;
@@ -357,18 +358,17 @@ public function getValue(): int
         $integerWrappedTypeName = $integerWrappedType->getName(); // @phpstan-ignore-line
 
         $this->executeFxWithTemporaryType($integerWrappedTypeName, $integerWrappedType, function () use ($integerWrappedType, $integerWrappedTypeName) {
-            $file = new Model($this->db, ['table' => 'file']);
-            $file->getField('id')->type = $integerWrappedTypeName;
-            $file->addField('name');
-            $file->hasOne('parentDirectory', [
-                'model' => $file,
-                'type' => $integerWrappedTypeName,
-                'ourField' => 'parentDirectoryId',
-            ]);
-            $file->hasMany('childFiles', [
-                'model' => $file,
-                'theirField' => 'parentDirectoryId',
-            ]);
+            $createFileModelFx = static function (Persistence $persistence) use ($integerWrappedTypeName, &$createFileModelFx) {
+                $m = new Model($persistence, ['table' => 'file']);
+                $m->getField('id')->type = $integerWrappedTypeName;
+                $m->addField('name');
+                $m->hasOne('parentDirectory', ['model' => $createFileModelFx, 'ourField' => 'parentDirectoryId', 'type' => $integerWrappedTypeName]);
+                $m->hasMany('childFiles', ['model' => $createFileModelFx, 'theirField' => 'parentDirectoryId']);
+
+                return $m;
+            };
+
+            $file = $createFileModelFx($this->db);
 
             $fileEntity = $file->loadBy('name', 'v');
             self::assertSame(3, $fileEntity->getId()->getValue());

tests/ReferenceTest.php

@@ -114,7 +114,7 @@ public function testHasOneDuplicateNameException(): void
     public function testCustomReference(): void
     {
         $m = new Model($this->db, ['table' => 'user']);
-        $m->addReference('archive', ['model' => static function (Model $m) {
+        $m->addReference('archive', ['model' => static function () use ($m) {
             return new $m(null, ['table' => $m->table . '_archive']);
         }]);