Fix reverse join inconsistencies and defaults

src/Model/Join.php

@@ -167,30 +167,39 @@ protected function init(): void
     {
         $this->_init();
 
-        // handle foreign table containing a dot
+        // owner model should have id_field set
+        $id_field = $this->getOwner()->id_field;
+        if (!$id_field) {
+            throw (new Exception('Joins owner model should have id_field set'))
+                ->addMoreInfo('model', $this->getOwner());
+        }
+
+        // handle foreign table containing a dot - that will be reverse join
         if (is_string($this->foreign_table) && strpos($this->foreign_table, '.') !== false) {
+            // split by LAST dot in foreign_table name
+            [$this->foreign_table, $this->foreign_field] = preg_split('~\.+(?=[^.]+$)~', $this->foreign_table);
+
             if (!isset($this->reverse)) {
                 $this->reverse = true;
-                if (isset($this->master_field)) {
-                    // both master and foreign fields are set
-
-                    // master_field exists, no we will use that
-                    // if (!is_object($this->master_field) && !$this->getOwner()->hasField($this->master_field)) {
-                    throw (new Exception('You are trying to link tables on non-id fields. This is not implemented yet'))
-                        ->addMoreInfo('condition', $this->getOwner()->table . '.' . $this->master_field . ' = ' . $this->foreign_table);
-                    // } $this->reverse = 'link';
-                }
             }
+        }
 
-            // split by LAST dot in foreign_table name
-            [$this->foreign_table, $this->foreign_field] = preg_split('/\.+(?=[^\.]+$)/', $this->foreign_table);
+        if ($this->reverse === true) {
+            if (isset($this->master_field) && $this->master_field !== $id_field) {
+                throw (new Exception('Joining tables on non-id fields is not implemented yet'))
+                    ->addMoreInfo('condition', $this->getOwner()->table . '.' . $this->master_field . ' = ' . $this->foreign_table . '.' . $this->foreign_field);
+            }
 
             if (!$this->master_field) {
-                $this->master_field = 'id';
+                $this->master_field = $id_field;
+            }
+
+            if (!$this->foreign_field) {
+                $this->foreign_field = $this->getOwner()->table . '_' . $id_field;
             }
         } else {
             $this->reverse = false;
-            $id_field = $this->getOwner()->id_field ?: 'id';
+
             if (!$this->master_field) {
                 $this->master_field = $this->foreign_table . '_' . $id_field;
             }

tests/JoinArrayTest.php

@@ -44,6 +44,7 @@ public function testDirection()
         $this->assertSame('test_id', $this->getProtected($j, 'master_field'));
         $this->assertSame('id', $this->getProtected($j, 'foreign_field'));
 
+        $this->expectException(Exception::class); // TODO not implemented yet, see https://github.com/atk4/data/pull/802
         $j = $m->join('contact4.foo_id', ['test_id', 'reverse' => true]);
         $this->assertTrue($this->getProtected($j, 'reverse'));
         $this->assertSame('test_id', $this->getProtected($j, 'master_field'));

tests/JoinSqlTest.php

@@ -41,6 +41,7 @@ public function testDirection()
         $this->assertSame('test_id', $this->getProtected($j, 'master_field'));
         $this->assertSame('id', $this->getProtected($j, 'foreign_field'));
 
+        $this->expectException(Exception::class); // TODO not implemented yet, see https://github.com/atk4/data/pull/802
         $j = $m->join('contact4.foo_id', ['test_id', 'reverse' => true]);
         $this->assertTrue($this->getProtected($j, 'reverse'));
         $this->assertSame('test_id', $this->getProtected($j, 'master_field'));
@@ -599,4 +600,76 @@ public function testJoinHasOneHasMany()
             ['id' => 41, 'contact_id' => 10, 'address' => '[email protected]'],
         ], $m_u->ref('Email')->export());
     }
+
+    public function testJoinReverseOneOnOne()
+    {
+        $this->setDb([
+            'user' => [
+                10 => ['id' => 10, 'name' => 'John'],
+                20 => ['id' => 20, 'name' => 'Peter'],
+            ], 'detail' => [
+                100 => ['id' => 100, 'my_user_id' => 10, 'notes' => 'first note'],
+                200 => ['id' => 200, 'my_user_id' => 20, 'notes' => 'second note'],
+            ],
+        ]);
+
+        $db = new Persistence\Sql($this->db->connection);
+        $m_user = new Model($db, 'user');
+        $m_user->addField('name');
+        $j = $m_user->join('detail.my_user_id', [
+            //'reverse' => true, // this will be reverse join by default
+            // also no need to set these (will be done automatically), but still let's do that for test sake
+            'master_field' => 'id',
+            'foreign_field' => 'my_user_id',
+        ]);
+        $j->addField('notes');
+
+        // try load one record
+        $m = (clone $m_user)->tryLoad(20);
+        $this->assertTrue($m->loaded());
+        $this->assertEquals(['id' => 20, 'name' => 'Peter', 'notes' => 'second note'], $m->get());
+
+        // try to update loaded record
+        $m->save(['name' => 'Mark', 'notes' => '2nd note']);
+        $m = (clone $m_user)->tryLoad(20);
+        $this->assertTrue($m->loaded());
+        $this->assertEquals(['id' => 20, 'name' => 'Mark', 'notes' => '2nd note'], $m->get());
+
+        // insert new record
+        $m = (clone $m_user)->save(['name' => 'Emily', 'notes' => '3rd note']);
+        $m = (clone $m_user)->tryLoad(21);
+        $this->assertTrue($m->loaded());
+        $this->assertEquals(['id' => 21, 'name' => 'Emily', 'notes' => '3rd note'], $m->get());
+
+        // now test reverse join defined differently
+        $m_user = new Model($db, 'user');
+        $m_user->addField('name');
+        $j = $m_user->join('detail', [ // here we just set foreign table name without dot and foreign_field
+            'reverse' => true, // and set it as revers join
+            'foreign_field' => 'my_user_id', // this is custome name so we have to set it here otherwise it will generate user_id
+        ]);
+        $j->addField('notes');
+
+        // insert new record
+        $m = (clone $m_user)->save(['name' => 'Olaf', 'notes' => '4th note']);
+        $m = (clone $m_user)->tryLoad(22);
+        $this->assertTrue($m->loaded());
+        $this->assertEquals(['id' => 22, 'name' => 'Olaf', 'notes' => '4th note'], $m->get());
+
+        // now test reverse join with table_alias and foreign_alias
+        $m_user = new Model($db, ['user', 'table_alias' => 'u']);
+        $m_user->addField('name');
+        $j = $m_user->join('detail', [
+            'reverse' => true,
+            'foreign_field' => 'my_user_id',
+            'foreign_alias' => 'a',
+        ]);
+        $j->addField('notes');
+
+        // insert new record
+        $m = (clone $m_user)->save(['name' => 'Chris', 'notes' => '5th note']);
+        $m = (clone $m_user)->tryLoad(23);
+        $this->assertTrue($m->loaded());
+        $this->assertEquals(['id' => 23, 'name' => 'Chris', 'notes' => '5th note'], $m->get());
+    }
 }