fix different ID column across nested models

atk4 · Jan 11, 2022 · 8684a3d · 8684a3d
1 parent aab0bf9
commit 8684a3d
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 77 deletions.
diff --git a/src/Field.php b/src/Field.php
@@ -499,7 +499,7 @@ public function __debugInfo(): array
         ];
 
         foreach ([
-            'system', 'never_persist', 'never_save', 'read_only', 'ui', 'joinName',
+            'actual', 'system', 'never_persist', 'never_save', 'read_only', 'ui', 'joinName',
         ] as $key) {
             if ($this->{$key} !== null) {
                 $arr[$key] = $this->{$key};

diff --git a/src/Model.php b/src/Model.php
@@ -1612,10 +1612,6 @@ public function save(array $data = [])
         });
     }
 
-    /**
-     * This is a temporary method to avoid code duplication, but insert / import should
-     * be implemented differently.
-     */
     protected function _insert(array $row): void
     {
         // Find any row values that do not correspond to fields, and they may correspond to
@@ -1658,10 +1654,6 @@ protected function _insert(array $row): void
     }
 
     /**
-     * Faster method to add data, that does not modify active record.
-     *
-     * Will be further optimized in the future.
-     *
      * @return mixed
      */
     public function insert(array $row)
@@ -1673,11 +1665,6 @@ public function insert(array $row)
     }
 
     /**
-     * Even more faster method to add data, does not modify your
-     * current record and will not return anything.
-     *
-     * Will be further optimized in the future.
-     *
      * @return $this
      */
     public function import(array $rows)

diff --git a/src/Persistence.php b/src/Persistence.php
@@ -116,18 +116,6 @@ public function getDatabasePlatform(): Platforms\AbstractPlatform
         return new Persistence\GenericPlatform();
     }
 
-    private function assertSameIdField(Model $model): void
-    {
-        $modelIdFieldName = $model->getField($model->id_field)->getPersistenceName();
-        $tableIdFieldName = $model->table->id_field;
-
-        if ($modelIdFieldName !== $tableIdFieldName) {
-            throw (new Exception('Table model with different ID field persistence name is not supported'))
-                ->addMoreInfo('model_id_field', $modelIdFieldName)
-                ->addMoreInfo('table_id_field', $tableIdFieldName);
-        }
-    }
-
     /**
      * Tries to load data record, but will not fail if record can't be loaded.
      *
@@ -174,13 +162,27 @@ public function insert(Model $model, array $data)
         unset($data);
 
         if (is_object($model->table)) {
-            $this->assertSameIdField($model);
+            $innerInsertId = $model->table->insert($this->typecastLoadRow($model->table, $dataRaw));
+            if (!$model->id_field) {
+                return false;
+            }
+
+            $insertId = $this->typecastLoadField(
+                $model->getField($model->id_field),
+                $model->getField($model->id_field)->getPersistenceName() === $model->table->id_field
+                    ? $this->typecastSaveField($model->table->getField($model->table->id_field), $innerInsertId)
+                    : $dataRaw[$model->getField($model->id_field)->getPersistenceName()]
+            );
 
-            return $model->table->insert($model->table->persistence->typecastLoadRow($model->table, $dataRaw));
+            return $insertId;
         }
 
         $idRaw = $this->insertRaw($model, $dataRaw);
-        $id = $model->id_field ? $this->typecastLoadField($model->getField($model->id_field), $idRaw) : new \stdClass();
+        if (!$model->id_field) {
+            return false;
+        }
+
+        $id = $this->typecastLoadField($model->getField($model->id_field), $idRaw);
 
         return $id;
     }
@@ -201,6 +203,7 @@ protected function insertRaw(Model $model, array $dataRaw)
     public function update(Model $model, $id, array $data): void
     {
         $idRaw = $model->id_field ? $this->typecastSaveField($model->getField($model->id_field), $id) : null;
+        unset($id);
         if ($idRaw === null || (array_key_exists($model->id_field, $data) && $data[$model->id_field] === null)) {
             throw new Exception('Model id_field is not set. Unable to update record.');
         }
@@ -213,15 +216,16 @@ public function update(Model $model, $id, array $data): void
         }
 
         if (is_object($model->table)) {
-            $this->assertSameIdField($model);
-
-            $model->table->load($id)->save($model->table->persistence->typecastLoadRow($model->table, $dataRaw));
+            $idPersistenceName = $model->getField($model->id_field)->getPersistenceName();
+            $innerModel = $model->table->loadBy(
+                $idPersistenceName,
+                $this->typecastLoadField($model->table->getField($idPersistenceName), $idRaw)
+            );
+            $innerModel->save($this->typecastLoadRow($model->table, $dataRaw));
 
             return;
         }
 
-        unset($id);
-
         $this->updateRaw($model, $idRaw, $dataRaw);
     }
 
@@ -241,20 +245,22 @@ protected function updateRaw(Model $model, $idRaw, array $dataRaw): void
     public function delete(Model $model, $id): void
     {
         $idRaw = $model->id_field ? $this->typecastSaveField($model->getField($model->id_field), $id) : null;
+        unset($id);
         if ($idRaw === null) {
             throw new Exception('Model id_field is not set. Unable to delete record.');
         }
 
         if (is_object($model->table)) {
-            $this->assertSameIdField($model);
-
-            $model->table->delete($id);
+            $idPersistenceName = $model->getField($model->id_field)->getPersistenceName();
+            $innerModel = $model->table->loadBy(
+                $idPersistenceName,
+                $this->typecastLoadField($model->table->getField($idPersistenceName), $idRaw)
+            );
+            $innerModel->delete();
 
             return;
         }
 
-        unset($id);
-
         $this->deleteRaw($model, $idRaw);
     }
 

diff --git a/src/Persistence/Sql.php b/src/Persistence/Sql.php
@@ -422,7 +422,8 @@ public function action(Model $model, string $type, array $args = [])
                 $this->setLimitOrder($model, $query);
 
                 if ($model->isEntity() && $model->isLoaded()) {
-                    $query->where($model->getField($model->id_field), $model->getId());
+                    $idRaw = $this->typecastSaveField($model->getField($model->id_field), $model->getId());
+                    $query->where($model->getField($model->id_field), $idRaw);
                 }
 
                 return $query;
@@ -478,7 +479,9 @@ public function tryLoad(Model $model, $id): ?array
                 throw (new Exception('Unable to load field by "id" when Model->id_field is not defined.'))
                     ->addMoreInfo('id', $id);
             }
-            $query->where($model->getField($model->id_field), $id);
+
+            $idRaw = $this->typecastSaveField($model->getField($model->id_field), $id);
+            $query->where($model->getField($model->id_field), $idRaw);
         }
         $query->limit($id === self::ID_LOAD_ANY ? 1 : 2);
 

diff --git a/tests/ModelNestedTest.php b/tests/ModelNestedTest.php
@@ -20,8 +20,8 @@ protected function setUp(): void
 
         $this->setDb([
             'user' => [
-                ['_id' => 1, 'name' => 'John', '_birthday' => '1980-2-1'],
-                ['_id' => 2, 'name' => 'Sue', '_birthday' => '2005-4-3'],
+                ['_id' => 1, 'name' => 'John', '_birthday' => '1980-02-01'],
+                ['_id' => 2, 'name' => 'Sue', '_birthday' => '2005-04-03'],
             ],
         ]);
     }
@@ -70,8 +70,8 @@ public function hook(string $spot, array $args = [], HookBreaker &$brokenBy = nu
             'table' => 'user',
         ]);
         $mInner->removeField('id');
-        $mInner->addField('_id', ['type' => 'integer']);
-        $mInner->id_field = '_id';
+        $mInner->id_field = 'uid';
+        $mInner->addField('uid', ['actual' => '_id', 'type' => 'integer']);
         $mInner->addField('name');
         $mInner->addField('y', ['actual' => '_birthday', 'type' => 'date']);
 
@@ -81,8 +81,7 @@ public function hook(string $spot, array $args = [], HookBreaker &$brokenBy = nu
             'table' => $mInner,
         ]);
         $m->removeField('id');
-        $m->addField('_id', ['type' => 'integer']);
-        $m->id_field = '_id';
+        $m->id_field = 'birthday';
         $m->addField('name');
         $m->addField('birthday', ['actual' => 'y', 'type' => 'date']);
 
@@ -100,15 +99,14 @@ public function testSelectSql(): void
             ($this->db->connection->dsql())
                 ->table(
                     ($this->db->connection->dsql())
-                        ->field('_id')
+                        ->field('_id', 'uid')
                         ->field('name')
                         ->field('_birthday', 'y')
                         ->table('user')
                         ->order('name', true)
                         ->limit(5),
                     '_tm'
                 )
-                ->field('_id')
                 ->field('name')
                 ->field('y', 'birthday')
                 ->order('y')
@@ -127,8 +125,8 @@ public function testSelectExport(): void
         $m = $this->createTestModel();
 
         $this->assertSameExportUnordered([
-            ['_id' => 1, 'name' => 'John', 'birthday' => new \DateTime('1980-2-1')],
-            ['_id' => 2, 'name' => 'Sue', 'birthday' => new \DateTime('2005-4-3')],
+            ['name' => 'John', 'birthday' => new \DateTime('1980-2-1')],
+            ['name' => 'Sue', 'birthday' => new \DateTime('2005-4-3')],
         ], $m->export());
 
         $this->assertSame([
@@ -141,89 +139,93 @@ public function testInsert(): void
     {
         $m = $this->createTestModel();
 
-        $m->createEntity()->setMulti([
-            'name' => 'Karl',
-            'birthday' => new \DateTime('2000-6-1'),
-        ])->save();
+        $entity = $m->createEntity()
+            ->setMulti([
+                'name' => 'Karl',
+                'birthday' => new \DateTime('2000-6-1'),
+            ])->save();
 
         $this->assertSame([
             ['main', Model::HOOK_VALIDATE, ['save']],
             ['main', Model::HOOK_BEFORE_SAVE, [false]],
-            ['main', Model::HOOK_BEFORE_INSERT, [['_id' => null, 'name' => 'Karl', 'birthday' => \DateTime::class]]],
+            ['main', Model::HOOK_BEFORE_INSERT, [['name' => 'Karl', 'birthday' => \DateTime::class]]],
             ['inner', Model::HOOK_VALIDATE, ['save']],
             ['inner', Model::HOOK_BEFORE_SAVE, [false]],
-            ['inner', Model::HOOK_BEFORE_INSERT, [['_id' => null, 'name' => 'Karl', 'y' => \DateTime::class]]],
+            ['inner', Model::HOOK_BEFORE_INSERT, [['uid' => null, 'name' => 'Karl', 'y' => \DateTime::class]]],
             ['inner', Persistence\Sql::HOOK_BEFORE_INSERT_QUERY, [Query::class]],
             ['inner', Persistence\Sql::HOOK_AFTER_INSERT_QUERY, [Query::class, DbalResult::class]],
             ['inner', Model::HOOK_AFTER_INSERT, []],
             ['inner', Model::HOOK_AFTER_SAVE, [false]],
             ['main', Model::HOOK_AFTER_INSERT, []],
             ['main', Model::HOOK_BEFORE_UNLOAD, []],
             ['main', Model::HOOK_AFTER_UNLOAD, []],
-            ['main', Model::HOOK_BEFORE_LOAD, [3]],
+            ['main', Model::HOOK_BEFORE_LOAD, [\DateTime::class]],
             ['inner', Persistence\Sql::HOOK_INIT_SELECT_QUERY, [Query::class, 'select']],
             ['main', Persistence\Sql::HOOK_INIT_SELECT_QUERY, [Query::class, 'select']],
             ['main', Model::HOOK_AFTER_LOAD, []],
             ['main', Model::HOOK_AFTER_SAVE, [false]],
         ], $this->hookLog);
 
+        $this->assertSame(3, $m->table->loadBy('name', 'Karl')->getId());
+        $this->assertSameExportUnordered([[new \DateTime('2000-6-1')]], [[$entity->getId()]]);
+
         $this->assertSameExportUnordered([
-            ['_id' => 1, 'name' => 'John', 'birthday' => new \DateTime('1980-2-1')],
-            ['_id' => 2, 'name' => 'Sue', 'birthday' => new \DateTime('2005-4-3')],
-            ['_id' => 3, 'name' => 'Karl', 'birthday' => new \DateTime('2000-6-1')],
+            ['name' => 'John', 'birthday' => new \DateTime('1980-2-1')],
+            ['name' => 'Sue', 'birthday' => new \DateTime('2005-4-3')],
+            ['name' => 'Karl', 'birthday' => new \DateTime('2000-6-1')],
         ], $m->export());
     }
 
     public function testUpdate(): void
     {
         $m = $this->createTestModel();
 
-        $m->load(2)->setMulti([
-            'name' => 'Susan',
-            'birthday' => new \DateTime('2020-10-10'),
-        ])->save();
+        $m->load(new \DateTime('2005-4-3'))
+            ->setMulti([
+                'name' => 'Susan',
+            ])->save();
 
         $this->assertSame([
-            ['main', Model::HOOK_BEFORE_LOAD, [2]],
+            ['main', Model::HOOK_BEFORE_LOAD, [\DateTime::class]],
             ['inner', Persistence\Sql::HOOK_INIT_SELECT_QUERY, [Query::class, 'select']],
             ['main', Persistence\Sql::HOOK_INIT_SELECT_QUERY, [Query::class, 'select']],
             ['main', Model::HOOK_AFTER_LOAD, []],
             ['main', Model::HOOK_VALIDATE, ['save']],
             ['main', Model::HOOK_BEFORE_SAVE, [true]],
-            ['main', Model::HOOK_BEFORE_UPDATE, [['name' => 'Susan', 'birthday' => \DateTime::class]]],
-            ['inner', Model::HOOK_BEFORE_LOAD, [2]],
+            ['main', Model::HOOK_BEFORE_UPDATE, [['name' => 'Susan']]],
+            ['inner', Model::HOOK_BEFORE_LOAD, [null]],
             ['inner', Persistence\Sql::HOOK_INIT_SELECT_QUERY, [Query::class, 'select']],
             ['inner', Model::HOOK_AFTER_LOAD, []],
             ['inner', Model::HOOK_VALIDATE, ['save']],
             ['inner', Model::HOOK_BEFORE_SAVE, [true]],
-            ['inner', Model::HOOK_BEFORE_UPDATE, [['name' => 'Susan', 'y' => \DateTime::class]]],
+            ['inner', Model::HOOK_BEFORE_UPDATE, [['name' => 'Susan']]],
             ['inner', Persistence\Sql::HOOK_BEFORE_UPDATE_QUERY, [Query::class]],
             ['inner', Persistence\Sql::HOOK_AFTER_UPDATE_QUERY, [Query::class, DbalResult::class]],
-            ['inner', Model::HOOK_AFTER_UPDATE, [['name' => 'Susan', 'y' => \DateTime::class]]],
+            ['inner', Model::HOOK_AFTER_UPDATE, [['name' => 'Susan']]],
             ['inner', Model::HOOK_AFTER_SAVE, [true]],
-            ['main', Model::HOOK_AFTER_UPDATE, [['name' => 'Susan', 'birthday' => \DateTime::class]]],
+            ['main', Model::HOOK_AFTER_UPDATE, [['name' => 'Susan']]],
             ['main', Model::HOOK_AFTER_SAVE, [true]],
         ], $this->hookLog);
 
         $this->assertSameExportUnordered([
-            ['_id' => 1, 'name' => 'John', 'birthday' => new \DateTime('1980-2-1')],
-            ['_id' => 2, 'name' => 'Susan', 'birthday' => new \DateTime('2020-10-10')],
+            ['name' => 'John', 'birthday' => new \DateTime('1980-2-1')],
+            ['name' => 'Susan', 'birthday' => new \DateTime('2005-4-3')],
         ], $m->export());
     }
 
     public function testDelete(): void
     {
         $m = $this->createTestModel();
 
-        $m->load(2)->delete();
+        $m->delete(new \DateTime('2005-4-3'));
 
         $this->assertSame([
-            ['main', Model::HOOK_BEFORE_LOAD, [2]],
+            ['main', Model::HOOK_BEFORE_LOAD, [\DateTime::class]],
             ['inner', Persistence\Sql::HOOK_INIT_SELECT_QUERY, [Query::class, 'select']],
             ['main', Persistence\Sql::HOOK_INIT_SELECT_QUERY, [Query::class, 'select']],
             ['main', Model::HOOK_AFTER_LOAD, []],
             ['main', Model::HOOK_BEFORE_DELETE, []],
-            ['inner', Model::HOOK_BEFORE_LOAD, [2]],
+            ['inner', Model::HOOK_BEFORE_LOAD, [null]],
             ['inner', Persistence\Sql::HOOK_INIT_SELECT_QUERY, [Query::class, 'select']],
             ['inner', Model::HOOK_AFTER_LOAD, []],
             ['inner', Model::HOOK_BEFORE_DELETE, []],
@@ -238,7 +240,7 @@ public function testDelete(): void
         ], $this->hookLog);
 
         $this->assertSameExportUnordered([
-            ['_id' => 1, 'name' => 'John', 'birthday' => new \DateTime('1980-2-1')],
+            ['name' => 'John', 'birthday' => new \DateTime('1980-2-1')],
         ], $m->export());
     }
 }