From 7d149911885dbb62e8a3c3cc1c19053a1797e906 Mon Sep 17 00:00:00 2001 From: DarkSide Date: Thu, 10 Dec 2020 17:30:15 +0200 Subject: [PATCH 01/11] fix issues in join --- src/Model/Join.php | 22 ++++++++++++++-------- tests/JoinSqlTest.php | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/src/Model/Join.php b/src/Model/Join.php index 3c88b122d..f4dbc8cb1 100644 --- a/src/Model/Join.php +++ b/src/Model/Join.php @@ -167,30 +167,36 @@ protected function init(): void { $this->_init(); + // owner model should have id_field set + if (!$this->getOwner()->id_field) { + throw (new Exception('Joins owner model should have id_field set')) + ->addMoreInfo('model', $this->getOwner()); + } + $id_field = $this->getOwner()->id_field; + // handle foreign table containing a dot 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)) { + if (isset($this->master_field) && $this->master_field !== $id_field) { // both master and foreign fields are set - // master_field exists, no we will use that + // master_field exists, now 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); + ->addMoreInfo('condition', $this->getOwner()->table . '.' . $this->master_field . ' = ' . $this->foreign_table . '.' . $this->foreign_field); // } $this->reverse = 'link'; } } - // split by LAST dot in foreign_table name - [$this->foreign_table, $this->foreign_field] = preg_split('/\.+(?=[^\.]+$)/', $this->foreign_table); - if (!$this->master_field) { - $this->master_field = 'id'; + $this->master_field = $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; } diff --git a/tests/JoinSqlTest.php b/tests/JoinSqlTest.php index 90f46e783..5a3f274f0 100644 --- a/tests/JoinSqlTest.php +++ b/tests/JoinSqlTest.php @@ -599,4 +599,45 @@ public function testJoinHasOneHasMany() ['id' => 41, 'contact_id' => 10, 'address' => 'johnny@foo.net'], ], $m_u->ref('Email')->export()); } + + public function testJoinOneOnOne() + { + $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()); + } } From ca64444c548f12f25e0bbbe865ca2eecb12d9bcf Mon Sep 17 00:00:00 2001 From: DarkSide Date: Thu, 10 Dec 2020 17:44:12 +0200 Subject: [PATCH 02/11] fix join defaults --- src/Model/Join.php | 28 ++++++++++++++++------------ tests/JoinArrayTest.php | 1 + tests/JoinSqlTest.php | 1 + 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/Model/Join.php b/src/Model/Join.php index f4dbc8cb1..621b71517 100644 --- a/src/Model/Join.php +++ b/src/Model/Join.php @@ -168,35 +168,39 @@ protected function init(): void $this->_init(); // owner model should have id_field set - if (!$this->getOwner()->id_field) { + $id_field = $this->getOwner()->id_field; + if (!$id_field) { throw (new Exception('Joins owner model should have id_field set')) ->addMoreInfo('model', $this->getOwner()); } - $id_field = $this->getOwner()->id_field; - // handle foreign table containing a dot + // 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) && $this->master_field !== $id_field) { - // both master and foreign fields are set - - // master_field exists, now 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->foreign_field); - // } $this->reverse = 'link'; - } + } + } + + if ($this->reverse === true) { + + if (isset($this->master_field) && $this->master_field !== $id_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->foreign_field); } if (!$this->master_field) { $this->master_field = $id_field; } + + if (!$this->foreign_field) { + $this->foreign_field = $this->getOwner()->table . '_' . $id_field; + } } else { $this->reverse = false; + if (!$this->master_field) { $this->master_field = $this->foreign_table . '_' . $id_field; } diff --git a/tests/JoinArrayTest.php b/tests/JoinArrayTest.php index d4e224ea6..f0da4ce6b 100644 --- a/tests/JoinArrayTest.php +++ b/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); $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')); diff --git a/tests/JoinSqlTest.php b/tests/JoinSqlTest.php index 5a3f274f0..fb35a5260 100644 --- a/tests/JoinSqlTest.php +++ b/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); $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')); From f0b2ae8902e90b3d90abfa6394bb8b61fc06916a Mon Sep 17 00:00:00 2001 From: DarkSide Date: Thu, 10 Dec 2020 18:45:29 +0200 Subject: [PATCH 03/11] finalize tests --- src/Model/Join.php | 1 - tests/JoinSqlTest.php | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/Model/Join.php b/src/Model/Join.php index 621b71517..0f0fd3d09 100644 --- a/src/Model/Join.php +++ b/src/Model/Join.php @@ -185,7 +185,6 @@ protected function init(): void } if ($this->reverse === true) { - if (isset($this->master_field) && $this->master_field !== $id_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->foreign_field); diff --git a/tests/JoinSqlTest.php b/tests/JoinSqlTest.php index fb35a5260..04b426767 100644 --- a/tests/JoinSqlTest.php +++ b/tests/JoinSqlTest.php @@ -601,7 +601,7 @@ public function testJoinHasOneHasMany() ], $m_u->ref('Email')->export()); } - public function testJoinOneOnOne() + public function testJoinReverseOneOnOne() { $this->setDb([ 'user' => [ @@ -640,5 +640,20 @@ public function testJoinOneOnOne() $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 without + $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()); } } From e6bc11517aa68fc6ec92b5433b2e8f268b27bc68 Mon Sep 17 00:00:00 2001 From: DarkSide Date: Thu, 10 Dec 2020 19:11:18 +0200 Subject: [PATCH 04/11] add test with aliases --- tests/JoinSqlTest.php | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/JoinSqlTest.php b/tests/JoinSqlTest.php index 04b426767..4de97391a 100644 --- a/tests/JoinSqlTest.php +++ b/tests/JoinSqlTest.php @@ -641,7 +641,7 @@ public function testJoinReverseOneOnOne() $this->assertTrue($m->loaded()); $this->assertEquals(['id' => 21, 'name' => 'Emily', 'notes' => '3rd note'], $m->get()); - // now test reverse join without + // 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 @@ -655,5 +655,21 @@ public function testJoinReverseOneOnOne() $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()); } } From 64568e2fe38ba947f395f1d91b095deb83bbfc68 Mon Sep 17 00:00:00 2001 From: Imants Horsts Date: Thu, 10 Dec 2020 19:13:45 +0200 Subject: [PATCH 05/11] Update src/Model/Join.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michael Voříšek --- src/Model/Join.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Model/Join.php b/src/Model/Join.php index 0f0fd3d09..72aa56d1c 100644 --- a/src/Model/Join.php +++ b/src/Model/Join.php @@ -186,7 +186,7 @@ protected function init(): void if ($this->reverse === true) { if (isset($this->master_field) && $this->master_field !== $id_field) { - throw (new Exception('You are trying to link tables on non-id fields. This is not implemented yet')) + 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); } From ae8474d0c3d11b52bc7784d8cd68a39ed52affda Mon Sep 17 00:00:00 2001 From: Imants Horsts Date: Thu, 10 Dec 2020 19:13:57 +0200 Subject: [PATCH 06/11] Update src/Model/Join.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michael Voříšek --- src/Model/Join.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Model/Join.php b/src/Model/Join.php index 72aa56d1c..baed0bfec 100644 --- a/src/Model/Join.php +++ b/src/Model/Join.php @@ -177,7 +177,7 @@ protected function init(): void // 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); + [$this->foreign_table, $this->foreign_field] = preg_split('~\.+(?=[^.]+$)~', $this->foreign_table); if (!isset($this->reverse)) { $this->reverse = true; From 1f43cac40d35b61909b473ef63c0ad1b0a877373 Mon Sep 17 00:00:00 2001 From: Imants Horsts Date: Thu, 10 Dec 2020 19:14:14 +0200 Subject: [PATCH 07/11] Update tests/JoinArrayTest.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michael Voříšek --- tests/JoinArrayTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/JoinArrayTest.php b/tests/JoinArrayTest.php index f0da4ce6b..5d46fd46a 100644 --- a/tests/JoinArrayTest.php +++ b/tests/JoinArrayTest.php @@ -44,7 +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); + $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')); From 1973658f61f8b955949ac96f5919c1d06757fb5e Mon Sep 17 00:00:00 2001 From: Imants Horsts Date: Thu, 10 Dec 2020 19:18:00 +0200 Subject: [PATCH 08/11] Update JoinSqlTest.php --- tests/JoinSqlTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/JoinSqlTest.php b/tests/JoinSqlTest.php index 4de97391a..e1bf73845 100644 --- a/tests/JoinSqlTest.php +++ b/tests/JoinSqlTest.php @@ -41,7 +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); + $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')); From 1b522be45f57317330bd49025c43c4246820153d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Thu, 10 Dec 2020 20:14:17 +0100 Subject: [PATCH 09/11] Update tests/JoinArrayTest.php --- tests/JoinArrayTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/JoinArrayTest.php b/tests/JoinArrayTest.php index 5d46fd46a..0a819b76a 100644 --- a/tests/JoinArrayTest.php +++ b/tests/JoinArrayTest.php @@ -44,7 +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 + $this->expectException(Exception::class); // TODO not implemented yet, see https://github.com/atk4/data/issues/803 $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')); From 11af3032fabe7f59fb2bd3c5747fab9511a16f91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Thu, 10 Dec 2020 20:14:36 +0100 Subject: [PATCH 10/11] Update tests/JoinSqlTest.php --- tests/JoinSqlTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/JoinSqlTest.php b/tests/JoinSqlTest.php index e1bf73845..d706b8a11 100644 --- a/tests/JoinSqlTest.php +++ b/tests/JoinSqlTest.php @@ -41,7 +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 + $this->expectException(Exception::class); // TODO not implemented yet, see https://github.com/atk4/data/issues/803 $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')); From 7158fc7f3dc60a84c18fc34f23e3c98c1a61212e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Thu, 10 Dec 2020 20:15:06 +0100 Subject: [PATCH 11/11] Update src/Model/Join.php --- src/Model/Join.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Model/Join.php b/src/Model/Join.php index baed0bfec..d0c6065b5 100644 --- a/src/Model/Join.php +++ b/src/Model/Join.php @@ -185,7 +185,7 @@ protected function init(): void } if ($this->reverse === true) { - if (isset($this->master_field) && $this->master_field !== $id_field) { + if (isset($this->master_field) && $this->master_field !== $id_field) { // TODO not implemented yet, see https://github.com/atk4/data/issues/803 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); }