Throw when foreign record cannot be found (#1001)

atk4 · May 31, 2022 · 07b1c72 · 07b1c72
1 parent 9e4c71f
commit 07b1c72
Show file tree

Hide file tree

Showing 9 changed files with 46 additions and 32 deletions.
diff --git a/src/Model.php b/src/Model.php
@@ -1463,12 +1463,16 @@ public function withPersistence(Persistence $persistence, $id = null, string $cl
     /**
      * @param mixed $value
      *
-     * @return static|null
+     * @return ($fromTryLoad is true ? static|null : static)
      */
-    private function _loadBy(bool $isTryLoad, string $fieldName, $value)
+    private function _loadBy(bool $fromTryLoad, string $fieldName, $value)
     {
         $this->assertIsModel();
 
+        if ($fieldName === $this->id_field) { // optimization only
+            return $this->{$fromTryLoad ? 'tryLoad' : 'load'}($value);
+        }
+
         $field = $this->getField($fieldName);
 
         $scopeBak = $this->scope;
@@ -1478,7 +1482,7 @@ private function _loadBy(bool $isTryLoad, string $fieldName, $value)
             $this->scope = clone $this->scope;
             $this->addCondition($field, $value);
 
-            return $this->{$isTryLoad ? 'tryLoadOne' : 'loadOne'}();
+            return $this->{$fromTryLoad ? 'tryLoadOne' : 'loadOne'}();
         } finally {
             $this->scope = $scopeBak;
             $field->system = $systemBak;

diff --git a/src/Reference.php b/src/Reference.php
@@ -126,9 +126,9 @@ final protected function getOurFieldValue(Model $ourEntity)
         return $this->getOurModel($ourEntity)->get($this->getOurFieldName());
     }
 
-    public function getTheirFieldName(): string
+    public function getTheirFieldName(Model $theirModel = null): string
     {
-        return $this->their_field ?? Model::assertInstanceOf($this->model)->id_field;
+        return $this->their_field ?? ($theirModel ?? Model::assertInstanceOf($this->model))->id_field;
     }
 
     protected function onHookToOurModel(Model $model, string $spot, \Closure $fx, array $args = [], int $priority = 5): int

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

diff --git a/src/Reference/HasMany.php b/src/Reference/HasMany.php
@@ -20,7 +20,7 @@ private function getModelTableString(Model $model): string
         return $model->table;
     }
 
-    public function getTheirFieldName(): string
+    public function getTheirFieldName(Model $theirModel = null): string
     {
         if ($this->their_field) {
             return $this->their_field;

diff --git a/src/Reference/HasOne.php b/src/Reference/HasOne.php
@@ -89,16 +89,26 @@ public function ref(Model $ourModel, array $defaults = []): Model
 
         if ($ourModel->isEntity()) {
             $ourValue = $this->getOurFieldValue($ourModel);
-            $this->assertReferenceValueNotNull($ourValue);
+
+            if ($this->getOurFieldName() === $ourModel->id_field) {
+                $this->assertReferenceValueNotNull($ourValue);
+                $tryLoad = true;
+            } else {
+                $tryLoad = false;
+            }
 
             $theirModelOrig = $theirModel;
-            if ($this->their_field) {
-                $theirModel = $theirModel->tryLoadBy($this->their_field, $ourValue);
+            if ($ourValue === null) {
+                $theirModel = null;
             } else {
-                $theirModel = $theirModel->tryLoad($ourValue);
+                $theirModel->addCondition($this->getTheirFieldName($theirModel), $ourValue);
+
+                $theirModel = $tryLoad
+                    ? $theirModel->tryLoadOne()
+                    : $theirModel->loadOne();
             }
 
-            if ($theirModel === null) { // TODO or implement tryRef?
+            if ($theirModel === null) {
                 $theirModel = $theirModelOrig->createEntity();
             }
         }

diff --git a/src/Reference/HasOneSql.php b/src/Reference/HasOneSql.php
@@ -99,10 +99,7 @@ public function refLink(Model $ourModel, array $defaults = []): Model
     {
         $theirModel = $this->createTheirModel($defaults);
 
-        $theirModel->addCondition(
-            $this->their_field ?: $theirModel->id_field,
-            $this->referenceOurValue()
-        );
+        $theirModel->addCondition($this->getTheirFieldName($theirModel), $this->referenceOurValue());
 
         return $theirModel;
     }
@@ -115,17 +112,14 @@ public function ref(Model $ourModel, array $defaults = []): Model
         $theirModel = parent::ref($ourModel, $defaults);
         $ourModel = $this->getOurModel($ourModel);
 
-        $theirFieldName = $this->their_field ?? $theirModel->id_field; // TODO why not $this->getTheirFieldName() ?
-
-        if ($ourModel->isEntity()) {
-            $theirModel->getModel()
-                ->addCondition($theirFieldName, $this->getOurFieldValue($ourModel));
+        if ($ourModel->isEntity() && $this->getOurFieldValue($ourModel) !== null) {
+            // materialized condition already added in parent/HasOne class
         } else {
-            // handles the deep traversal using an expression
+            // handle deep traversal using an expression
             $ourFieldExpression = $ourModel->action('field', [$this->getOurField()]);
 
             $theirModel->getModel(true)
-                ->addCondition($theirFieldName, $ourFieldExpression);
+                ->addCondition($this->getTheirFieldName($theirModel), $ourFieldExpression);
         }
 
         return $theirModel;

diff --git a/src/Schema/Migrator.php b/src/Schema/Migrator.php
@@ -211,11 +211,7 @@ protected function getReferenceField(Field $field): ?Field
     {
         $reference = $field->getReference();
         if ($reference instanceof HasOne) {
-            $referenceTheirField = \Closure::bind(function () use ($reference) {
-                return $reference->their_field;
-            }, null, \Atk4\Data\Reference::class)();
-
-            $referenceField = $referenceTheirField ?? $reference->getOwner()->id_field; // TODO migrate to $reference->getTheirFieldName()
+            $referenceField = $reference->getTheirFieldName($reference->createTheirModel());
 
             $modelSeed = is_array($reference->model)
                 ? $reference->model

diff --git a/tests/ReferenceSqlTest.php b/tests/ReferenceSqlTest.php
@@ -417,11 +417,21 @@ public function testReferenceHasOneTraversing(): void
         ], $userEntity->getModel()->ref('Company')->ref('Orders')->export());
     }
 
-    public function testUnloadedEntityTraversingHasOnedEx(): void
+    public function testUnloadedEntityTraversingHasOne(): void
     {
         $user = $this->setupDbForTraversing();
         $userEntity = $user->createEntity();
 
+        $companyEntity = $userEntity->ref('Company');
+        $this->assertFalse($companyEntity->isLoaded());
+    }
+
+    public function testUnloadedEntityTraversingHasOneEx(): void
+    {
+        $user = $this->setupDbForTraversing();
+        $user->getRef('Company')->setDefaults(['our_field' => 'id']);
+        $userEntity = $user->createEntity();
+
         $this->expectException(Exception::class);
         $this->expectExceptionMessage('Unable to traverse on null value');
         $userEntity->ref('Company');
@@ -465,14 +475,14 @@ public function testReferenceHook(): void
         $uu = $u->load(2);
         $this->assertNull($uu->get('address'));
         $this->assertNull($uu->get('contact_id'));
+        $this->assertNull($uu->ref('contact_id')->get('address'));
 
         $uu = $u->load(3);
         $this->assertSame('Joe contact', $uu->get('address'));
         $this->assertSame('Joe contact', $uu->ref('contact_id')->get('address'));
 
         $uu = $u->load(2);
-        $cc = $uu->getModel()->ref('contact_id')->createEntity()->save(['address' => 'Peters new contact']);
-        $uu->set('contact_id', $cc->getId());
+        $uu->ref('contact_id')->save(['address' => 'Peters new contact']);
 
         $this->assertNotNull($uu->get('contact_id'));
         $this->assertSame('Peters new contact', $uu->ref('contact_id')->get('address'));

diff --git a/tests/ReferenceTest.php b/tests/ReferenceTest.php
@@ -69,7 +69,7 @@ public function testModelProperty(): void
         $user = $user->createEntity();
         $user->setId(1);
         $user->getModel()->hasOne('order_id', ['model' => [Model::class, 'table' => 'order']]);
-        $o = $user->getModel()->ref('order_id');
+        $o = $user->ref('order_id');
         $this->assertSame('order', $o->table);
     }