From 74d7f6d4ca84d81de873611aafd64832e0715e0b Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 10 May 2018 14:35:26 +0200 Subject: [PATCH 1/4] Add a QueryBuilder Mapper Signed-off-by: Roeland Jago Douma --- lib/public/AppFramework/Db/Mapper.php | 15 +- lib/public/AppFramework/Db/QBMapper.php | 270 ++++++++++++++++++++++++ 2 files changed, 284 insertions(+), 1 deletion(-) create mode 100644 lib/public/AppFramework/Db/QBMapper.php diff --git a/lib/public/AppFramework/Db/Mapper.php b/lib/public/AppFramework/Db/Mapper.php index b008702ba5429..6910757add0da 100644 --- a/lib/public/AppFramework/Db/Mapper.php +++ b/lib/public/AppFramework/Db/Mapper.php @@ -34,6 +34,7 @@ * Simple parent class for inheriting your data access layer from. This class * may be subject to change in the future * @since 7.0.0 + * @deprecated 14.0.0 Move over to QBMapper */ abstract class Mapper { @@ -47,6 +48,7 @@ abstract class Mapper { * @param string $entityClass the name of the entity that the sql should be * mapped to queries without using sql * @since 7.0.0 + * @deprecated 14.0.0 Move over to QBMapper */ public function __construct(IDBConnection $db, $tableName, $entityClass=null){ $this->db = $db; @@ -65,6 +67,7 @@ public function __construct(IDBConnection $db, $tableName, $entityClass=null){ /** * @return string the table name * @since 7.0.0 + * @deprecated 14.0.0 Move over to QBMapper */ public function getTableName(){ return $this->tableName; @@ -76,6 +79,7 @@ public function getTableName(){ * @param Entity $entity the entity that should be deleted * @return Entity the deleted entity * @since 7.0.0 - return value added in 8.1.0 + * @deprecated 14.0.0 Move over to QBMapper */ public function delete(Entity $entity){ $sql = 'DELETE FROM `' . $this->tableName . '` WHERE `id` = ?'; @@ -90,6 +94,7 @@ public function delete(Entity $entity){ * @param Entity $entity the entity that should be created * @return Entity the saved entity with the set id * @since 7.0.0 + * @deprecated 14.0.0 Move over to QBMapper */ public function insert(Entity $entity){ // get updated fields to save, fields have to be set using a setter to @@ -139,6 +144,7 @@ public function insert(Entity $entity){ * @param Entity $entity the entity that should be created * @return Entity the saved entity with the set id * @since 7.0.0 - return value was added in 8.0.0 + * @deprecated 14.0.0 Move over to QBMapper */ public function update(Entity $entity){ // if entity wasn't changed it makes no sense to run a db query @@ -195,6 +201,7 @@ public function update(Entity $entity){ * @param array $array * @return bool true if associative * @since 8.1.0 + * @deprecated 14.0.0 Move over to QBMapper */ private function isAssocArray(array $array) { return array_values($array) !== $array; @@ -205,6 +212,7 @@ private function isAssocArray(array $array) { * @param $value * @return int PDO constant * @since 8.1.0 + * @deprecated 14.0.0 Move over to QBMapper */ private function getPDOType($value) { switch (gettype($value)) { @@ -226,6 +234,7 @@ private function getPDOType($value) { * @param int $offset from which row we want to start * @return \PDOStatement the database query result * @since 7.0.0 + * @deprecated 14.0.0 Move over to QBMapper */ protected function execute($sql, array $params=[], $limit=null, $offset=null){ $query = $this->db->prepare($sql, $limit, $offset); @@ -249,7 +258,6 @@ protected function execute($sql, array $params=[], $limit=null, $offset=null){ return $query; } - /** * Returns an db result and throws exceptions when there are more or less * results @@ -262,6 +270,7 @@ protected function execute($sql, array $params=[], $limit=null, $offset=null){ * @throws MultipleObjectsReturnedException if more than one item exist * @return array the result as row * @since 7.0.0 + * @deprecated 14.0.0 Move over to QBMapper */ protected function findOneQuery($sql, array $params=[], $limit=null, $offset=null){ $stmt = $this->execute($sql, $params, $limit, $offset); @@ -297,6 +306,7 @@ protected function findOneQuery($sql, array $params=[], $limit=null, $offset=nul * @param int $offset from which row we want to start * @return string formatted error message string * @since 9.1.0 + * @deprecated 14.0.0 Move over to QBMapper */ private function buildDebugMessage($msg, $sql, array $params=[], $limit=null, $offset=null) { return $msg . @@ -313,6 +323,7 @@ private function buildDebugMessage($msg, $sql, array $params=[], $limit=null, $o * @param array $row the row which should be converted to an entity * @return Entity the entity * @since 7.0.0 + * @deprecated 14.0.0 Move over to QBMapper */ protected function mapRowToEntity($row) { return call_user_func($this->entityClass .'::fromRow', $row); @@ -327,6 +338,7 @@ protected function mapRowToEntity($row) { * @param int $offset from which row we want to start * @return array all fetched entities * @since 7.0.0 + * @deprecated 14.0.0 Move over to QBMapper */ protected function findEntities($sql, array $params=[], $limit=null, $offset=null) { $stmt = $this->execute($sql, $params, $limit, $offset); @@ -354,6 +366,7 @@ protected function findEntities($sql, array $params=[], $limit=null, $offset=nul * @throws MultipleObjectsReturnedException if more than one item exist * @return Entity the entity * @since 7.0.0 + * @deprecated 14.0.0 Move over to QBMapper */ protected function findEntity($sql, array $params=[], $limit=null, $offset=null){ return $this->mapRowToEntity($this->findOneQuery($sql, $params, $limit, $offset)); diff --git a/lib/public/AppFramework/Db/QBMapper.php b/lib/public/AppFramework/Db/QBMapper.php new file mode 100644 index 0000000000000..0eaa7601f20af --- /dev/null +++ b/lib/public/AppFramework/Db/QBMapper.php @@ -0,0 +1,270 @@ + + * + * @author Roeland Jago Douma + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCP\AppFramework\Db; + +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; + +/** + * Simple parent class for inheriting your data access layer from. This class + * may be subject to change in the future + * + * @since 14.0.0 + */ +abstract class QBMapper { + + /** @var string */ + protected $tableName; + + /** @var string */ + protected $entityClass; + + /** @var IDBConnection */ + protected $db; + + /** + * @param IDBConnection $db Instance of the Db abstraction layer + * @param string $tableName the name of the table. set this to allow entity + * @param string $entityClass the name of the entity that the sql should be + * mapped to queries without using sql + * @since 14.0.0 + */ + public function __construct(IDBConnection $db, string $tableName, string $entityClass=null){ + $this->db = $db; + $this->tableName = $tableName; + + // if not given set the entity name to the class without the mapper part + // cache it here for later use since reflection is slow + if($entityClass === null) { + $this->entityClass = str_replace('Mapper', '', \get_class($this)); + } else { + $this->entityClass = $entityClass; + } + } + + + /** + * @return string the table name + * @since 14.0.0 + */ + public function getTableName(): string { + return $this->tableName; + } + + + /** + * Deletes an entity from the table + * @param Entity $entity the entity that should be deleted + * @return Entity the deleted entity + * @since 14.0.0 + */ + public function delete(Entity $entity): Entity { + $qb = $this->db->getQueryBuilder(); + + $qb->delete($this->tableName) + ->where( + $qb->expr()->eq('id', $qb->createNamedParameter($entity->getId())) + ); + $qb->execute(); + return $entity; + } + + + /** + * Creates a new entry in the db from an entity + * @param Entity $entity the entity that should be created + * @return Entity the saved entity with the set id + * @since 14.0.0 + */ + public function insert(Entity $entity): Entity { + // get updated fields to save, fields have to be set using a setter to + // be saved + $properties = $entity->getUpdatedFields(); + + $qb = $this->db->getQueryBuilder(); + $qb->insert($this->tableName); + + // build the fields + foreach($properties as $property => $updated) { + $column = $entity->propertyToColumn($property); + $getter = 'get' . ucfirst($property); + $value = $entity->$getter(); + + $qb->setValue($column, $qb->createNamedParameter($value)); + } + + $qb->execute(); + + $entity->setId((int) $qb->getLastInsertId()); + + return $entity; + } + + + + /** + * Updates an entry in the db from an entity + * @throws \InvalidArgumentException if entity has no id + * @param Entity $entity the entity that should be created + * @return Entity the saved entity with the set id + * @since 14.0.0 + */ + public function update(Entity $entity): Entity { + // if entity wasn't changed it makes no sense to run a db query + $properties = $entity->getUpdatedFields(); + if(\count($properties) === 0) { + return $entity; + } + + // entity needs an id + $id = $entity->getId(); + if($id === null){ + throw new \InvalidArgumentException( + 'Entity which should be updated has no id'); + } + + // get updated fields to save, fields have to be set using a setter to + // be saved + // do not update the id field + unset($properties['id']); + + $qb = $this->db->getQueryBuilder(); + $qb->update($this->tableName); + + // build the fields + foreach($properties as $property => $updated) { + $column = $entity->propertyToColumn($property); + $getter = 'get' . ucfirst($property); + $value = $entity->$getter(); + + $qb->set($column, $qb->createNamedParameter($value)); + } + + $qb->where( + $qb->expr()->eq('id', $qb->createNamedParameter($id)) + ); + $qb->execute(); + + return $entity; + } + + /** + * Returns an db result and throws exceptions when there are more or less + * results + * + * @see findEntity + * + * @param IQueryBuilder $query + * @throws DoesNotExistException if the item does not exist + * @throws MultipleObjectsReturnedException if more than one item exist + * @return array the result as row + * @since 14.0.0 + */ + protected function findOneQuery(IQueryBuilder $query): array { + $cursor = $query->execute(); + + $row = $cursor->fetch(); + if($row === false) { + $cursor->closeCursor(); + $msg = $this->buildDebugMessage( + 'Did expect one result but found none when executing', $query + ); + throw new DoesNotExistException($msg); + } + + $row2 = $cursor->fetch(); + $cursor->closeCursor(); + if($row2 !== false ) { + $msg = $this->buildDebugMessage( + 'Did not expect more than one result when executing', $query + ); + throw new MultipleObjectsReturnedException($msg); + } + + return $row; + } + + /** + * @param string $msg + * @param IQueryBuilder $sql + * @return string + * @since 14.0.0 + */ + private function buildDebugMessage(string $msg, IQueryBuilder $sql): string { + return $msg . + ': query "' . $sql->getSQL() . '"; '; + } + + + /** + * Creates an entity from a row. Automatically determines the entity class + * from the current mapper name (MyEntityMapper -> MyEntity) + * + * @param array $row the row which should be converted to an entity + * @return Entity the entity + * @since 14.0.0 + */ + protected function mapRowToEntity(array $row): Entity { + return \call_user_func($this->entityClass .'::fromRow', $row); + } + + + /** + * Runs a sql query and returns an array of entities + * + * @param IQueryBuilder $query + * @return Entity[] all fetched entities + * @since 14.0.0 + */ + protected function findEntities(IQueryBuilder $query): array { + $cursor = $query->execute(); + + $entities = []; + + while($row = $cursor->fetch()){ + $entities[] = $this->mapRowToEntity($row); + } + + $cursor->closeCursor(); + + return $entities; + } + + + /** + * Returns an db result and throws exceptions when there are more or less + * results + * + * @param IQueryBuilder $query + * @throws DoesNotExistException if the item does not exist + * @throws MultipleObjectsReturnedException if more than one item exist + * @return Entity the entity + * @since 14.0.0 + */ + protected function findEntity(IQueryBuilder $query): Entity { + return $this->mapRowToEntity($this->findOneQuery($query)); + } + +} From 6d299883ba7af08a7a39fde2b09339bf10a8d501 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 10 May 2018 14:47:17 +0200 Subject: [PATCH 2/4] Bump autoloader Signed-off-by: Roeland Jago Douma --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index bf223204e2bc4..d37cab8277a8d 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -23,6 +23,7 @@ 'OCP\\AppFramework\\Db\\Entity' => $baseDir . '/lib/public/AppFramework/Db/Entity.php', 'OCP\\AppFramework\\Db\\Mapper' => $baseDir . '/lib/public/AppFramework/Db/Mapper.php', 'OCP\\AppFramework\\Db\\MultipleObjectsReturnedException' => $baseDir . '/lib/public/AppFramework/Db/MultipleObjectsReturnedException.php', + 'OCP\\AppFramework\\Db\\QBMapper' => $baseDir . '/lib/public/AppFramework/Db/QBMapper.php', 'OCP\\AppFramework\\Http' => $baseDir . '/lib/public/AppFramework/Http.php', 'OCP\\AppFramework\\Http\\ContentSecurityPolicy' => $baseDir . '/lib/public/AppFramework/Http/ContentSecurityPolicy.php', 'OCP\\AppFramework\\Http\\DataDisplayResponse' => $baseDir . '/lib/public/AppFramework/Http/DataDisplayResponse.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index cb723ef70ee31..ac68a91621be9 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -53,6 +53,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OCP\\AppFramework\\Db\\Entity' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/Entity.php', 'OCP\\AppFramework\\Db\\Mapper' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/Mapper.php', 'OCP\\AppFramework\\Db\\MultipleObjectsReturnedException' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/MultipleObjectsReturnedException.php', + 'OCP\\AppFramework\\Db\\QBMapper' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/QBMapper.php', 'OCP\\AppFramework\\Http' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http.php', 'OCP\\AppFramework\\Http\\ContentSecurityPolicy' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/ContentSecurityPolicy.php', 'OCP\\AppFramework\\Http\\DataDisplayResponse' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/DataDisplayResponse.php', From 610c66520ba1d498d327158a377dc150bfa1b388 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 10 May 2018 14:47:26 +0200 Subject: [PATCH 3/4] Move over TokenMapper Signed-off-by: Roeland Jago Douma --- lib/private/Authentication/Token/DefaultTokenMapper.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/private/Authentication/Token/DefaultTokenMapper.php b/lib/private/Authentication/Token/DefaultTokenMapper.php index 41d1b9f203db9..55494d7237057 100644 --- a/lib/private/Authentication/Token/DefaultTokenMapper.php +++ b/lib/private/Authentication/Token/DefaultTokenMapper.php @@ -30,11 +30,12 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\Mapper; +use OCP\AppFramework\Db\QBMapper; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\IUser; -class DefaultTokenMapper extends Mapper { +class DefaultTokenMapper extends QBMapper { public function __construct(IDBConnection $db) { parent::__construct($db, 'authtoken'); From ed7b4839d9ac543799e291ad2d338db559c86354 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 14 May 2018 14:46:33 +0200 Subject: [PATCH 4/4] The column is not user input so suppress the phan warning Signed-off-by: Roeland Jago Douma --- lib/public/AppFramework/Db/QBMapper.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/public/AppFramework/Db/QBMapper.php b/lib/public/AppFramework/Db/QBMapper.php index 0eaa7601f20af..a9b38732a3033 100644 --- a/lib/public/AppFramework/Db/QBMapper.php +++ b/lib/public/AppFramework/Db/QBMapper.php @@ -97,6 +97,7 @@ public function delete(Entity $entity): Entity { * @param Entity $entity the entity that should be created * @return Entity the saved entity with the set id * @since 14.0.0 + * @suppress SqlInjectionChecker */ public function insert(Entity $entity): Entity { // get updated fields to save, fields have to be set using a setter to @@ -130,6 +131,7 @@ public function insert(Entity $entity): Entity { * @param Entity $entity the entity that should be created * @return Entity the saved entity with the set id * @since 14.0.0 + * @suppress SqlInjectionChecker */ public function update(Entity $entity): Entity { // if entity wasn't changed it makes no sense to run a db query