Fix load of imported fields in reverse array join

src/Model/Join.php

@@ -270,6 +270,7 @@ protected function init(): void
 
     protected function initJoinHooks(): void
     {
+        $this->onHookToOwnerEntity(Model::HOOK_AFTER_LOAD, \Closure::fromCallable([$this, 'afterLoad']));
         $this->onHookToOwnerEntity(Model::HOOK_AFTER_UNLOAD, \Closure::fromCallable([$this, 'afterUnload']));
 
         if ($this->reverse) {
@@ -280,8 +281,6 @@ protected function initJoinHooks(): void
             $this->onHookToOwnerEntity(Model::HOOK_BEFORE_INSERT, \Closure::fromCallable([$this, 'beforeInsert']), [], -5);
             $this->onHookToOwnerEntity(Model::HOOK_BEFORE_UPDATE, \Closure::fromCallable([$this, 'beforeUpdate']), [], -5);
             $this->onHookToOwnerEntity(Model::HOOK_AFTER_DELETE, \Closure::fromCallable([$this, 'doDelete']));
-
-            $this->onHookToOwnerEntity(Model::HOOK_AFTER_LOAD, \Closure::fromCallable([$this, 'afterLoad']));
         }
     }
 
@@ -462,7 +461,7 @@ protected function issetSaveBuffer(Model $entity): bool
      *
      * @internal should be not used outside atk4/data
      */
-    protected function getReindexAndUnsetSaveBuffer(Model $entity): array
+    protected function getReindexedDataAndUnsetSaveBuffer(Model $entity): array
     {
         $resOur = $this->saveBufferByOid[spl_object_id($entity)];
         $this->unsetSaveBuffer($entity);
@@ -497,14 +496,16 @@ public function setSaveBufferValue(Model $entity, string $fieldName, $value): vo
         $this->saveBufferByOid[spl_object_id($entity)][$fieldName] = $value;
     }
 
+    public function afterLoad(Model $entity): void
+    {
+    }
+
     protected function afterUnload(Model $entity): void
     {
         $this->unsetId($entity);
         $this->unsetSaveBuffer($entity);
     }
 
-    abstract public function afterLoad(Model $entity): void;
-
     /**
      * @param array<string, mixed> $data
      */
@@ -523,7 +524,7 @@ public function beforeInsert(Model $entity, array &$data): void
 
         $foreignModel = $this->getForeignModel();
         $foreignEntity = $foreignModel->createEntity()
-            ->setMulti($this->getReindexAndUnsetSaveBuffer($entity))
+            ->setMulti($this->getReindexedDataAndUnsetSaveBuffer($entity))
             /* ->set($this->foreignField, null) */;
         $foreignEntity->save();
 
@@ -550,7 +551,7 @@ public function afterInsert(Model $entity): void
 
         $foreignModel = $this->getForeignModel();
         $foreignEntity = $foreignModel->createEntity()
-            ->setMulti($this->getReindexAndUnsetSaveBuffer($entity))
+            ->setMulti($this->getReindexedDataAndUnsetSaveBuffer($entity))
             ->set($this->foreignField, $id);
         $foreignEntity->save();
 
@@ -573,7 +574,7 @@ public function beforeUpdate(Model $entity, array &$data): void
         $foreignModel = $this->getForeignModel();
         $foreignId = $this->reverse ? $entity->getId() : $entity->get($this->masterField);
         $this->assertReferenceIdNotNull($foreignId);
-        $saveBuffer = $this->getReindexAndUnsetSaveBuffer($entity);
+        $saveBuffer = $this->getReindexedDataAndUnsetSaveBuffer($entity);
         $foreignModel->atomic(function () use ($foreignModel, $foreignId, $saveBuffer) {
             $foreignModel = (clone $foreignModel)->addCondition($this->foreignField, $foreignId);
             foreach ($foreignModel as $foreignEntity) {

src/Persistence/Array_/Join.php

@@ -12,21 +12,27 @@ class Join extends Model\Join
 {
     public function afterLoad(Model $entity): void
     {
-        // we need to collect ID
-        $this->setId($entity, $entity->getDataRef()[$this->masterField]);
-        if ($this->getId($entity) === null) {
+        $model = $this->getOwner();
+
+        $foreignId = $entity->getDataRef()[$this->masterField];
+        if ($foreignId === null) {
             return;
         }
 
         try {
-            $data = Persistence\Array_::assertInstanceOf($this->getOwner()->getPersistence())
-                ->load($this->createFakeForeignModel(), $this->getId($entity));
+            $foreignData = Persistence\Array_::assertInstanceOf($model->getPersistence())
+                ->load($this->createFakeForeignModel(), $foreignId);
         } catch (Exception $e) {
             throw (new Exception('Unable to load joined record', $e->getCode(), $e))
                 ->addMoreInfo('table', $this->foreignTable)
-                ->addMoreInfo('id', $this->getId($entity));
+                ->addMoreInfo('id', $foreignId);
         }
+
         $dataRef = &$entity->getDataRef();
-        $dataRef = array_merge($data, $entity->getDataRef());
+        foreach ($model->getFields() as $field) {
+            if ($field->hasJoin() && $field->getJoin()->shortName === $this->shortName) {
+                $dataRef[$field->shortName] = $foreignData[$field->getPersistenceName()];
+            }
+        }
     }
 }

src/Persistence/Sql/Join.php

@@ -77,22 +77,5 @@ public function initSelectQuery(Model $model, Query $query): void
             $this->kind,
             $this->foreignAlias
         );
-
-        /*
-        if ($this->reverse) {
-            $query->field([$this->shortName => $this->join ?? ($model->tableAlias ?? $model->table) . '.' . $this->masterField]);
-        } else {
-            $query->field([$this->shortName => $this->foreignAlias . '.' . $this->foreignField]);
-        }
-        */
-    }
-
-    public function afterLoad(Model $entity): void
-    {
-        // we need to collect ID
-        if (isset($entity->getDataRef()[$this->shortName])) {
-            $this->setId($entity, $entity->getDataRef()[$this->shortName]);
-            unset($entity->getDataRef()[$this->shortName]);
-        }
     }
 }

src/Schema/TestCase.php

@@ -10,6 +10,7 @@
 use Atk4\Data\Persistence\Sql\Expression;
 use Doctrine\DBAL\ParameterType;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Platforms\MySQLPlatform;
 use Doctrine\DBAL\Platforms\OraclePlatform;
 use Doctrine\DBAL\Platforms\SQLitePlatform;
 use Doctrine\DBAL\Platforms\SQLServerPlatform;
@@ -363,14 +364,13 @@ public function dropCreatedDb(): void
         }
     }
 
-    public function markTestIncompleteWhenCreateUniqueIndexIsNotSupportedByPlatform(): void
+    protected function markTestIncompleteOnMySQL56PlatformAsCreateUniqueStringIndexHasLengthLimit(): void
     {
-        if ($this->getDatabasePlatform() instanceof SQLServerPlatform) {
-            // https://github.com/doctrine/dbal/issues/5507
-            self::markTestIncomplete('TODO MSSQL: DBAL must setup unique index without WHERE clause');
-        } elseif ($this->getDatabasePlatform() instanceof OraclePlatform) {
-            // https://github.com/doctrine/dbal/issues/5508
-            self::markTestIncomplete('TODO Oracle: DBAL must setup unique index on table column too');
+        if ($this->getDatabasePlatform() instanceof MySQLPlatform) {
+            $serverVersion = $this->getConnection()->getConnection()->getWrappedConnection()->getServerVersion(); // @phpstan-ignore-line
+            if (preg_match('~^5\.6~', $serverVersion)) {
+                self::markTestIncomplete('TODO MySQL 5.6: Unique key exceed max key (767 bytes) length');
+            }
         }
     }
 }

tests/JoinArrayTest.php

@@ -80,6 +80,10 @@ public function testJoinSaving1(): void
         $user2->set('contact_phone', '+123');
         $user2->save();
 
+        self::assertSame(1, $user2->getId());
+        self::assertSame('John', $user2->get('name'));
+        self::assertSame('+123', $user2->get('contact_phone'));
+
         self::assertSame([
             'user' => [1 => ['name' => 'John', 'contact_id' => 1]],
             'contact' => [1 => ['contact_phone' => '+123']],
@@ -134,6 +138,10 @@ public function testJoinSaving2(): void
         $user2->set('contact_phone', '+123');
         $user2->save();
 
+        self::assertSame(1, $user2->getId());
+        self::assertSame('John', $user2->get('name'));
+        self::assertSame('+123', $user2->get('contact_phone'));
+
         self::assertSame([
             'user' => [1 => ['name' => 'John']],
             'contact' => [1 => ['contact_phone' => '+123', 'test_id' => 1]],
@@ -375,7 +383,7 @@ public function testJoinDelete(): void
         ], $this->getInternalPersistenceData($db));
     }
 
-    public function testLoadMissing(): void
+    public function testLoadMissingException(): void
     {
         $db = new Persistence\Array_([
             'user' => [
@@ -396,6 +404,7 @@ public function testLoadMissing(): void
         $j->addField('contact_phone');
 
         $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Unable to load joined record');
         $user->load(2);
     }
 

tests/JoinSqlTest.php

@@ -7,6 +7,7 @@
 use Atk4\Data\Exception;
 use Atk4\Data\Model;
 use Atk4\Data\Schema\TestCase;
+use Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException;
 use Doctrine\DBAL\Platforms\MySQLPlatform;
 
 class JoinSqlTest extends TestCase
@@ -81,6 +82,10 @@ public function testJoinSaving1(): void
 
         $user2->save();
 
+        self::assertSame(1, $user2->getId());
+        self::assertSame('John', $user2->get('name'));
+        self::assertSame('+123', $user2->get('contact_phone'));
+
         self::assertSame([
             'user' => [
                 1 => ['id' => 1, 'name' => 'John', 'contact_id' => 1],
@@ -129,6 +134,10 @@ public function testJoinSaving2(): void
         $user2->set('contact_phone', '+123');
         $user2->save();
 
+        self::assertSame(1, $user2->getId());
+        self::assertSame('John', $user2->get('name'));
+        self::assertSame('+123', $user2->get('contact_phone'));
+
         self::assertSame([
             'user' => [
                 1 => ['id' => 1, 'name' => 'John'],
@@ -356,23 +365,35 @@ public function testJoinDelete(): void
         $user->addField('contact_id', ['type' => 'integer']);
         $user->addField('name');
         $j = $user->join('contact');
-        // TODO persist order is broken $this->createMigrator()->createForeignKey($j);
+        $this->createMigrator()->createForeignKey($j);
         $j->addField('contact_phone');
 
-        $user = $user->load(1);
+        $user = $user->load(3);
         $user->delete();
 
         self::assertSame([
             'user' => [
-                2 => ['id' => 2, 'name' => 'Peter', 'contact_id' => 1],
-                ['id' => 3, 'name' => 'XX', 'contact_id' => 2],
-                ['id' => 4, 'name' => 'YYY', 'contact_id' => 3],
+                1 => ['id' => 1, 'name' => 'John 2', 'contact_id' => 1],
+                ['id' => 2, 'name' => 'Peter', 'contact_id' => 1],
+                4 => ['id' => 4, 'name' => 'YYY', 'contact_id' => 3],
             ],
             'contact' => [
-                2 => ['id' => 2, 'contact_phone' => '+999'],
-                ['id' => 3, 'contact_phone' => '+777'],
+                1 => ['id' => 1, 'contact_phone' => '+555'],
+                3 => ['id' => 3, 'contact_phone' => '+777'],
             ],
         ], $this->getDb());
+
+        $user = $user->getModel()->load(1);
+
+        $this->expectException(Exception::class);
+        try {
+            $user->delete();
+        } catch (Exception $e) {
+            $dbalException = $e->getPrevious()->getPrevious();
+            self::assertInstanceOf(ForeignKeyConstraintViolationException::class, $dbalException);
+
+            throw $e;
+        }
     }
 
     public function testDoubleSaveHook(): void
@@ -592,8 +613,6 @@ public function testJoinHasOneHasMany(): void
             ['id' => 31, 'user_id' => 1, 'token' => 'DEF'],
         ], $user2->ref('Token')->export());
 
-        $this->markTestIncompleteWhenCreateUniqueIndexIsNotSupportedByPlatform();
-
         // hasMany email model (uses custom ourField, theirField)
         $email = new Model($this->db, ['table' => 'email']);
         $email->addField('contact_id', ['type' => 'integer']);

tests/ReferenceSqlTest.php

@@ -8,7 +8,6 @@
 use Atk4\Data\Model;
 use Atk4\Data\Schema\TestCase;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
-use Doctrine\DBAL\Platforms\MySQLPlatform;
 use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
 use Doctrine\DBAL\Platforms\SQLServerPlatform;
 use Doctrine\DBAL\Types as DbalTypes;
@@ -113,13 +112,7 @@ public function testBasic2(): void
         $c->addField('currency');
         $c->addField('name');
 
-        if ($this->getDatabasePlatform() instanceof MySQLPlatform) {
-            $serverVersion = $this->getConnection()->getConnection()->getWrappedConnection()->getServerVersion(); // @phpstan-ignore-line
-            if (preg_match('~^5\.6~', $serverVersion)) {
-                self::markTestIncomplete('TODO MySQL 5.6: Unique key exceed max key (767 bytes) length');
-            }
-        }
-        $this->markTestIncompleteWhenCreateUniqueIndexIsNotSupportedByPlatform();
+        $this->markTestIncompleteOnMySQL56PlatformAsCreateUniqueStringIndexHasLengthLimit();
 
         $u->hasOne('cur', ['model' => $c, 'ourField' => 'currency', 'theirField' => 'currency']);
         $this->createMigrator()->createForeignKey($u->getReference('cur'));
@@ -697,8 +690,6 @@ public function testHasOneIdFieldAsOurField(): void
         $s->addField('name');
         $s->addField('player_id', ['type' => 'integer']);
 
-        $this->markTestIncompleteWhenCreateUniqueIndexIsNotSupportedByPlatform();
-
         $p = new Model($this->db, ['table' => 'player']);
         $p->addField('name');
         $p->delete(2);

tests/Schema/MigratorFkTest.php

@@ -10,7 +10,6 @@
 use Doctrine\DBAL\Exception as DbalException;
 use Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException;
 use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
-use Doctrine\DBAL\Platforms\MySQLPlatform;
 use Doctrine\DBAL\Platforms\SQLServerPlatform;
 use Doctrine\DBAL\Schema\ForeignKeyConstraint;
 use Doctrine\DBAL\Schema\Index;
@@ -99,12 +98,7 @@ public function testCreateIndexUnique(): void
 
         $this->createMigrator($client)->create();
 
-        if ($this->getDatabasePlatform() instanceof MySQLPlatform) {
-            $serverVersion = $this->getConnection()->getConnection()->getWrappedConnection()->getServerVersion(); // @phpstan-ignore-line
-            if (preg_match('~^5\.6~', $serverVersion)) {
-                self::markTestIncomplete('TODO MySQL 5.6: Unique key exceed max key (767 bytes) length');
-            }
-        }
+        $this->markTestIncompleteOnMySQL56PlatformAsCreateUniqueStringIndexHasLengthLimit();
 
         $this->createMigrator()->createIndex([$client->getField('name')], true);
         self::assertSame([[['name'], true]], $this->listTableIndexes('client'));
@@ -145,12 +139,7 @@ public function testCreateIndexMultipleFields(): void
         $this->createMigrator($client)->create();
         self::assertSame([], $this->listTableIndexes('client'));
 
-        if ($this->getDatabasePlatform() instanceof MySQLPlatform) {
-            $serverVersion = $this->getConnection()->getConnection()->getWrappedConnection()->getServerVersion(); // @phpstan-ignore-line
-            if (preg_match('~^5\.6~', $serverVersion)) {
-                self::markTestIncomplete('TODO MySQL 5.6: Unique key exceed max key (767 bytes) length');
-            }
-        }
+        $this->markTestIncompleteOnMySQL56PlatformAsCreateUniqueStringIndexHasLengthLimit();
 
         $this->createMigrator($client)->createIndex([$client->getField('a'), $client->getField('b')], true);
         self::assertSame([[['a', 'b'], true]], $this->listTableIndexes('client'));
@@ -246,12 +235,7 @@ public function testForeignKeyViolationWithoutPk(): void
         $currency->insert(['code' => 'USD', 'name' => 'United States dollar']);
         $currency->insert(['code' => 'CZK', 'name' => 'Česká koruna']);
 
-        if ($this->getDatabasePlatform() instanceof MySQLPlatform) {
-            $serverVersion = $this->getConnection()->getConnection()->getWrappedConnection()->getServerVersion(); // @phpstan-ignore-line
-            if (preg_match('~^5\.6~', $serverVersion)) {
-                self::markTestIncomplete('TODO MySQL 5.6: Unique key exceed max key (767 bytes) length');
-            }
-        }
+        $this->markTestIncompleteOnMySQL56PlatformAsCreateUniqueStringIndexHasLengthLimit();
 
         $this->createMigrator()->createForeignKey([$price->getField('currency'), $currency->getField('code')]);