From 8f735ef1217aa55d2b61b063b13050c9fc8f0a3e Mon Sep 17 00:00:00 2001 From: Dennis Eichhorn Date: Sat, 27 Jan 2024 10:10:43 +0000 Subject: [PATCH] continue with db mapper optimizations --- DataStorage/Database/Mapper/ReadMapper.php | 152 ++----------------- DataStorage/Database/Mapper/UpdateMapper.php | 2 +- DataStorage/Database/Mapper/WriteMapper.php | 11 +- Module/ModuleAbstract.php | 4 + 4 files changed, 31 insertions(+), 138 deletions(-) diff --git a/DataStorage/Database/Mapper/ReadMapper.php b/DataStorage/Database/Mapper/ReadMapper.php index 22fc24ae6..f12b9b735 100755 --- a/DataStorage/Database/Mapper/ReadMapper.php +++ b/DataStorage/Database/Mapper/ReadMapper.php @@ -294,7 +294,7 @@ final class ReadMapper extends DataMapperAbstract } if (!empty($this->with) && !empty($objs)) { - $this->loadHasManyRelationsTest($objs); + $this->loadHasManyRelations($objs); } if (!empty($this->indexedBy)) { @@ -335,7 +335,7 @@ final class ReadMapper extends DataMapperAbstract $obj = $this->populateAbstract($row, $obj); if (!empty($this->with)) { - $this->loadHasManyRelationsTest([$obj]); + $this->loadHasManyRelations([$obj]); } yield $obj; @@ -1114,117 +1114,6 @@ final class ReadMapper extends DataMapperAbstract return $belongsToMapper->populateAbstract($result, $mapper::createBaseModel($result)); } - /** - * Fill object with relations - * - * @param object $obj Object to fill - * - * @return void - * - * @since 1.0.0 - */ - public function loadHasManyRelationsOld(object $obj) : void - { - if (empty($this->with)) { - return; - } - - // @todo only accept array and then perform this work on the array here - // this allows us to better load data for all objects at the same time! - - $primaryKey = $this->mapper::getObjectId($obj); - if (empty($primaryKey)) { - return; - } - - $refClass = null; - - // @todo check if there are more cases where the relation is already loaded with joins etc. - // there can be pseudo hasMany elements like localizations. They are hasMany but these are already loaded with joins! - foreach ($this->with as $member => $withData) { - if (isset($this->mapper::HAS_MANY[$member])) { - $many = $this->mapper::HAS_MANY[$member]; - if (isset($many['column'])) { - continue; - } - - $isPrivate = $withData['private'] ?? false; - - $objectMapper = $this->createRelationMapper($many['mapper']::get(db: $this->db), $member); - if ($many['external'] === null) { - $objectMapper->where($many['mapper']::COLUMNS[$many['self']]['internal'], $primaryKey); - } else { - $query = new Builder($this->db, true); - $query->leftJoin($many['table']) - ->on($many['mapper']::TABLE . '_d1.' . $many['mapper']::PRIMARYFIELD, '=', $many['table'] . '.' . $many['external']) - ->where($many['table'] . '.' . $many['self'], '=', $primaryKey); - - // Cannot use join, because join only works on members and we don't have members for a relation table - // This is why we need to create a "base" query which contains the join on table columns - $objectMapper->query($query); - } - - // @todo This right here is the problem for performing this on an array of primary keys. - // In case of a relation table there is no relation info available in the obj or the objects - // Since we don't retrieve the relation table information (we only use it in the select) we cannot assign - // the objects to the correct parent obj. For this reason we need to perform the loadHasManyRelations on an individual - // obj. - // Maybe we split this function in owns_one/belongs_to and hasMany. This way we could at least perform the action on an array - // for owns_one/belongs_to. - // Idea: somehow make the query->execute() return an array indexed by the key the object belongs to? This however would result - // not in a simple [array] but an array => array. - // For this we might have to create an internal function or variable called ->indexedBy(whatever_column_to_use_for_index) - $objects = $objectMapper->execute(); - if (empty($objects) || (!\is_array($objects) && $objects->id === 0)) { - continue; - } - - if ($isPrivate) { - if ($refClass === null) { - $refClass = new \ReflectionClass($obj); - } - - $refProp = $refClass->getProperty($member); - $refProp->setValue($obj, !\is_array($objects) && ($many['conditional'] ?? false) === false - ? [$many['mapper']::getObjectId($objects) => $objects] - : $objects // if conditional === true the obj will be assigned (e.g. hasMany localizations but only one is loaded for the model) - ); - } else { - $obj->{$member} = !\is_array($objects) && ($many['conditional'] ?? false) === false - ? [$many['mapper']::getObjectId($objects) => $objects] - : $objects; // if conditional === true the obj will be assigned (e.g. hasMany localizations but only one is loaded for the model) - } - - continue; - } elseif (isset($this->mapper::OWNS_ONE[$member]) - || isset($this->mapper::BELONGS_TO[$member]) - ) { - $relation = isset($this->mapper::OWNS_ONE[$member]) - ? $this->mapper::OWNS_ONE[$member] - : $this->mapper::BELONGS_TO[$member]; - - if (\count($withData) < 2) { - continue; - } - - /** @var ReadMapper $relMapper */ - $relMapper = $this->createRelationMapper($relation['mapper']::reader($this->db), $member); - - $isPrivate = $withData['private'] ?? false; - if ($isPrivate) { - if ($refClass === null) { - $refClass = new \ReflectionClass($obj); - } - - $refProp = $refClass->getProperty($member); - $relMapper->loadHasManyRelationsOld($refProp->getValue($obj)); - } else { - $relMapper->loadHasManyRelationsOld($obj->{$member}); - } - } - } - } - /** * Fill object with relations * @@ -1234,15 +1123,8 @@ final class ReadMapper extends DataMapperAbstract * * @since 1.0.0 */ - public function loadHasManyRelationsTest(array $objs) : void + public function loadHasManyRelations(array $objs) : void { - if (empty($this->with)) { - return; - } - - // @todo only accept array and then perform this work on the array here - // this allows us to better load data for all objects at the same time! - $primaryKeys = []; foreach ($objs as $idx => $obj) { $key = $this->mapper::getObjectId($obj); @@ -1258,8 +1140,6 @@ final class ReadMapper extends DataMapperAbstract $refClass = null; - $cachedKeys = []; - // @todo check if there are more cases where the relation is already loaded with joins etc. // there can be pseudo hasMany elements like localizations. They are hasMany but these are already loaded with joins! foreach ($this->with as $member => $withData) { @@ -1326,18 +1206,18 @@ final class ReadMapper extends DataMapperAbstract } elseif (isset($this->mapper::OWNS_ONE[$member]) || isset($this->mapper::BELONGS_TO[$member]) ) { - $relation = isset($this->mapper::OWNS_ONE[$member]) - ? $this->mapper::OWNS_ONE[$member] - : $this->mapper::BELONGS_TO[$member]; - if (\count($withData) < 2) { continue; } + $relation = isset($this->mapper::OWNS_ONE[$member]) + ? $this->mapper::OWNS_ONE[$member] + : $this->mapper::BELONGS_TO[$member]; + /** @var ReadMapper $relMapper */ $relMapper = $this->createRelationMapper($relation['mapper']::reader($this->db), $member); - $isPrivate = $withData['private'] ?? false; + $isPrivate = $relation['private'] ?? false; if ($isPrivate) { if ($refClass === null) { $refClass = new \ReflectionClass($obj); @@ -1350,14 +1230,14 @@ final class ReadMapper extends DataMapperAbstract $tempObjs[] = $refProp->getValue($obj); } - $relMapper->loadHasManyRelationsTest($tempObjs); + $relMapper->loadHasManyRelations($tempObjs); } else { $tempObjs = []; foreach ($objs as $obj) { $tempObjs[] = $obj->{$member}; } - $relMapper->loadHasManyRelationsTest($tempObjs); + $relMapper->loadHasManyRelations($tempObjs); } } } @@ -1395,7 +1275,7 @@ final class ReadMapper extends DataMapperAbstract } // @todo withData doesn't store this directly, it is in [0]['private] ?!?! - $isPrivate = $withData['private'] ?? false; + $isPrivate = $many['private'] ?? false; $objectMapper = $this->createRelationMapper($many['mapper']::exists(db: $this->db), $member); if ($many['external'] === null/* same as $many['table'] !== $many['mapper']::TABLE */) { @@ -1417,18 +1297,18 @@ final class ReadMapper extends DataMapperAbstract } elseif (isset($this->mapper::OWNS_ONE[$member]) || isset($this->mapper::BELONGS_TO[$member]) ) { - $relation = isset($this->mapper::OWNS_ONE[$member]) - ? $this->mapper::OWNS_ONE[$member] - : $this->mapper::BELONGS_TO[$member]; - if (\count($withData) < 2) { continue; } + $relation = isset($this->mapper::OWNS_ONE[$member]) + ? $this->mapper::OWNS_ONE[$member] + : $this->mapper::BELONGS_TO[$member]; + /** @var ReadMapper $relMapper */ $relMapper = $this->createRelationMapper($relation['mapper']::reader($this->db), $member); - $isPrivate = $withData['private'] ?? false; + $isPrivate = $relation['private'] ?? false; if ($isPrivate) { if ($refClass === null) { $refClass = new \ReflectionClass($obj); diff --git a/DataStorage/Database/Mapper/UpdateMapper.php b/DataStorage/Database/Mapper/UpdateMapper.php index 2b1cb76b9..3b7f07a85 100755 --- a/DataStorage/Database/Mapper/UpdateMapper.php +++ b/DataStorage/Database/Mapper/UpdateMapper.php @@ -388,7 +388,7 @@ final class UpdateMapper extends DataMapperAbstract $this->mapper::remover(db: $this->db)->deleteRelationTable($member, $removes, $objId); } - if (!empty($adds)) { + if (!empty($adds) && isset($this->mapper::HAS_MANY[$member]['external'])) { $this->mapper::writer(db: $this->db)->createRelationTable($member, $adds, $objId); } } diff --git a/DataStorage/Database/Mapper/WriteMapper.php b/DataStorage/Database/Mapper/WriteMapper.php index 0c12fda7a..1ac994088 100755 --- a/DataStorage/Database/Mapper/WriteMapper.php +++ b/DataStorage/Database/Mapper/WriteMapper.php @@ -353,10 +353,17 @@ final class WriteMapper extends DataMapperAbstract } } + // @todo This inserts one element at a time. SQL allows to insert multiple rows. + // The problem with this is, that we then need to manually calculate the objIds + // since lastInsertId returns the first generated id. + // However, the current use case in Jingga only rarely has multiple hasMany during the creation + // since we are calling the API individually. $objsIds[$key] = $mapper::create(db: $this->db)->execute($value); } - $this->createRelationTable($propertyName, $objsIds, $objId); + if (!empty($objsIds) && isset($this->mapper::HAS_MANY[$propertyName]['external'])) { + $this->createRelationTable($propertyName, $objsIds, $objId); + } } } @@ -374,9 +381,11 @@ final class WriteMapper extends DataMapperAbstract public function createRelationTable(string $propertyName, array $objsIds, mixed $objId) : void { try { + /* This check got pulled out to avoid function call to begin with if (empty($objsIds) || !isset($this->mapper::HAS_MANY[$propertyName]['external'])) { return; } + */ $relQuery = new Builder($this->db); $relQuery->into($this->mapper::HAS_MANY[$propertyName]['table']) diff --git a/Module/ModuleAbstract.php b/Module/ModuleAbstract.php index 5c967cfba..7f5b767a5 100755 --- a/Module/ModuleAbstract.php +++ b/Module/ModuleAbstract.php @@ -942,6 +942,10 @@ abstract class ModuleAbstract string $ip ) : void { + if (empty($rel1) || empty($rel2)) { + return; + } + $trigger = static::NAME . '-' . $trigger . '-relation-create'; $this->app->eventManager->triggerSimilar('PRE:Module:' . $trigger, '', $rel1);