From 1fbd865bb0bba9b35a570abd6dfe9f21ad51e8b1 Mon Sep 17 00:00:00 2001 From: Arne Hamann Date: Mon, 23 Jan 2023 20:57:04 +0100 Subject: [PATCH 1/9] Replace mapper by QBMapper Signed-off-by: Arne Hamann --- lib/DB/GeophotoMapper.php | 193 +++++++++++++++++++++++++----- lib/Service/PhotofilesService.php | 10 +- 2 files changed, 171 insertions(+), 32 deletions(-) diff --git a/lib/DB/GeophotoMapper.php b/lib/DB/GeophotoMapper.php index c0c9beab3..75ed842df 100644 --- a/lib/DB/GeophotoMapper.php +++ b/lib/DB/GeophotoMapper.php @@ -12,61 +12,200 @@ namespace OCA\Maps\DB; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; -use OCP\AppFramework\Db\Mapper; +use OCP\AppFramework\Db\QBMapper; -class GeophotoMapper extends Mapper { +class GeophotoMapper extends QBMapper { public function __construct(IDBConnection $db) { parent::__construct($db, 'maps_photos'); } + /** + * @param $id + * @return mixed|\OCP\AppFramework\Db\Entity + * @throws \OCP\AppFramework\Db\DoesNotExistException + * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException + * @throws \OCP\DB\Exception + */ public function find($id) { - $sql = 'SELECT * FROM `*PREFIX*maps_photos` ' . - 'WHERE `id` = ?'; - return $this->findEntity($sql, [$id]); + $qb = $this->db->getQueryBuilder(); + + $qb->select("*") + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_STR)) + ); + + return $this->findEntity($qb); } - public function findByFileId($userId, $fileId) { - try { - $sql = 'SELECT * FROM `*PREFIX*maps_photos` ' . - 'WHERE `user_id` = ? ' . - 'AND `file_id` = ? '; - return $this->findEntity($sql, [$userId, $fileId]); - } - catch (\Throwable $e) { - return null; - } + /** + * @param $fileId + * @param $userId + * @return mixed|\OCP\AppFramework\Db\Entity + * @throws \OCP\AppFramework\Db\DoesNotExistException + * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException + * @throws \OCP\DB\Exception + */ + public function findByFileIdUserId($fileId, $userId) { + $qb = $this->db->getQueryBuilder(); + + $qb->select("*") + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)) + )->andWhere( + $qb->expr()->eq('file_id', $qb->createNamedParameter($fileId, IQueryBuilder::PARAM_STR)) + ); + + return $this->findEntity($qb); } + /** + * @param $fileId + * @return mixed|\OCP\AppFramework\Db\Entity + * @throws \OCP\AppFramework\Db\DoesNotExistException + * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException + * @throws \OCP\DB\Exception + */ + public function findByFileId($fileId) { + $qb = $this->db->getQueryBuilder(); + + $qb->select("*") + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('file_id', $qb->createNamedParameter($fileId, IQueryBuilder::PARAM_STR)) + ); + + return $this->findEntity($qb); + } + + /** + * @param $userId + * @param $limit + * @param $offset + * @return array|\OCP\AppFramework\Db\Entity[] + * @throws \OCP\DB\Exception + */ public function findAll($userId, $limit=null, $offset=null) { - $sql = 'SELECT * FROM `*PREFIX*maps_photos` where `user_id` = ? and `lat` is not null and `lng` is not null ORDER BY `date_taken` ASC'; - return $this->findEntities($sql, [$userId], $limit, $offset); + $qb = $this->db->getQueryBuilder(); + + $qb->select("*") + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)) + )->andWhere( + $qb->expr()->isNotNull('lat') + )->andWhere( + $qb->expr()->isNotNull('lng') + )->orderBy('date_taken', 'ASC'); + if (!is_null($offset)) { + $qb->setFirstResult($offset); + } + if (!is_null($limit)) { + $qb->setMaxResults($limit); + } + return $this->findEntities($qb); } + /** + * @param $userId + * @param $limit + * @param $offset + * @return array|\OCP\AppFramework\Db\Entity[] + * @throws \OCP\DB\Exception + */ public function findAllNonLocalized($userId, $limit=null, $offset=null) { - $sql = 'SELECT * FROM `*PREFIX*maps_photos` where `user_id` = ? and (`lat` is null or `lng` is null) ORDER BY `date_taken` ASC'; - return $this->findEntities($sql, [$userId], $limit, $offset); + $qb = $this->db->getQueryBuilder(); + + $qb->select("*") + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)) + )->andWhere( + $qb->expr()->orX( + $qb->expr()->isNull('lat'), + $qb->expr()->isNull('lng') + ) + )->orderBy('date_taken', 'ASC'); + if (!is_null($offset)) { + $qb->setFirstResult($offset); + } + if (!is_null($limit)) { + $qb->setMaxResults($limit); + } + return $this->findEntities($qb); } + /** + * @param $fileId + * @return false|void + * @throws \OCP\DB\Exception + */ public function deleteByFileId($fileId) { - $sql = 'DELETE FROM `*PREFIX*maps_photos` where `file_id` = ?'; - return $this->execute($sql, [$fileId]); + $qb = $this->db->getQueryBuilder(); + + $qb->delete() + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('file_id', $qb->createNamedParameter($fileId, IQueryBuilder::PARAM_STR)) + ); + + return $qb->executeStatement(); } + /** + * @param $fileId + * @param $userId + * @return bool + * @throws \OCP\DB\Exception + */ public function deleteByFileIdUserId($fileId, $userId) { - $sql = 'DELETE FROM `*PREFIX*maps_photos` where `file_id` = ? and `user_id` = ?'; - return $this->execute($sql, [$fileId, $userId]); + try { + $entity = $this->findByFileIdUserId( $fileId, $userId); + } catch (DoesNotExistException | MultipleObjectsReturnedException $e) { + return false; + } + $this->delete($entity); + return true; } + /** + * @param $userId + * @return bool + * @throws \OCP\DB\Exception + */ public function deleteAll($userId) { - $sql = 'DELETE FROM `*PREFIX*maps_photos` where `user_id` = ?'; - return $this->execute($sql, [$userId]); + $qb = $this->db->getQueryBuilder(); + + $qb->delete() + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)) + ); + return $qb->executeStatement(); } + /** + * @param $fileId + * @param $lat + * @param $lng + * @return int + * @throws \OCP\DB\Exception + */ public function updateByFileId($fileId, $lat, $lng) { - $sql = 'UPDATE `*PREFIX*maps_photos` set `lat` = ? , `lng` = ? where `file_id` = ?'; - return $this->execute($sql, [$lat, $lng, $fileId]); + $qb = $this->db->getQueryBuilder(); + + $qb->update( + )->set('lat', $qb->createNamedParameter($lat) + )->set('lng', $qb->createNamedParameter($lng) + )->where('file_id', $qb->createNamedParameter($fileId)); + + return $qb->executeStatement(); } } diff --git a/lib/Service/PhotofilesService.php b/lib/Service/PhotofilesService.php index f1e025973..e95f9d41f 100644 --- a/lib/Service/PhotofilesService.php +++ b/lib/Service/PhotofilesService.php @@ -157,7 +157,7 @@ public function updateByFileNow(Node $file) { if (!is_null($exif)) { $ownerId = $file->getOwner()->getUID(); // in case there is no entry for this file yet (normally there is because non-localized photos are added) - if ($this->photoMapper->findByFileId($ownerId, $file->getId()) === null) { + if ($this->photoMapper->findByFileIdUserId($file->getId(), $ownerId) === null) { // TODO insert for all users having access to this file, not just the owner $this->insertPhoto($file, $ownerId, $exif); } @@ -227,7 +227,7 @@ private function setDirectoriesCoords($userId, $paths, $lats, $lngs) { $nodes = $dir->getDirectoryListing(); foreach($nodes as $node) { if ($this->isPhoto($node) && $node->isUpdateable()) { - $photo = $this->photoMapper->findByFileId($userId, $node->getId()); + $photo = $this->photoMapper->findByFileIdUserId($node->getId(), $userId); $done[] = [ 'path' => preg_replace('/^files/', '', $node->getInternalPath()), 'lat' => $lat, @@ -256,7 +256,7 @@ private function setFilesCoords($userId, $paths, $lats, $lngs) { if ($this->isPhoto($file) && $file->isUpdateable()) { $lat = (count($lats) > $i) ? $lats[$i] : $lats[0]; $lng = (count($lngs) > $i) ? $lngs[$i] : $lngs[0]; - $photo = $this->photoMapper->findByFileId($userId, $file->getId()); + $photo = $this->photoMapper->findByFileIdUserId($file->getId(), $userId); $done[] = [ 'path' => preg_replace('/^files/', '', $file->getInternalPath()), 'lat' => $lat, @@ -281,7 +281,7 @@ public function resetPhotosFilesCoords($userId, $paths) { if ($userFolder->nodeExists($cleanpath)) { $file = $userFolder->get($cleanpath); if ($this->isPhoto($file) && $file->isUpdateable()) { - $photo = $this->photoMapper->findByFileId($userId, $file->getId()); + $photo = $this->photoMapper->findByFileIdUserId($file->getId(), $userId); $done[] = [ 'path' => preg_replace('/^files/', '', $file->getInternalPath()), 'lat' => null, @@ -309,7 +309,7 @@ public function addPhotoNow($photo, $userId) { // so we need to be sure it's not inserted several times // by checking if it already exists in DB // OR by using file_id in primary key - if ($this->photoMapper->findByFileId($userId, $photo->getId()) === null) { + if ($this->photoMapper->findByFileIdUserId($photo->getId(), $userId) === null) { $this->insertPhoto($photo, $userId, $exif); } $this->photosCache->clear($userId); From 2bb67222d77894c7fe53c74df0e6e65c4df25def Mon Sep 17 00:00:00 2001 From: Arne Hamann Date: Mon, 23 Jan 2023 21:12:52 +0100 Subject: [PATCH 2/9] Update lib/DB/GeophotoMapper.php Co-authored-by: Joas Schilling <213943+nickvergessen@users.noreply.github.com> Signed-off-by: Arne Hamann --- lib/DB/GeophotoMapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/DB/GeophotoMapper.php b/lib/DB/GeophotoMapper.php index 75ed842df..b5892ae78 100644 --- a/lib/DB/GeophotoMapper.php +++ b/lib/DB/GeophotoMapper.php @@ -143,7 +143,7 @@ public function findAllNonLocalized($userId, $limit=null, $offset=null) { /** * @param $fileId - * @return false|void + * @return int * @throws \OCP\DB\Exception */ public function deleteByFileId($fileId) { From d7a4a953a867c18bfec762ecb2b8d7606b5d29ae Mon Sep 17 00:00:00 2001 From: Arne Hamann Date: Mon, 23 Jan 2023 21:13:02 +0100 Subject: [PATCH 3/9] Update lib/DB/GeophotoMapper.php Co-authored-by: Joas Schilling <213943+nickvergessen@users.noreply.github.com> Signed-off-by: Arne Hamann --- lib/DB/GeophotoMapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/DB/GeophotoMapper.php b/lib/DB/GeophotoMapper.php index b5892ae78..7762cdeeb 100644 --- a/lib/DB/GeophotoMapper.php +++ b/lib/DB/GeophotoMapper.php @@ -176,7 +176,7 @@ public function deleteByFileIdUserId($fileId, $userId) { /** * @param $userId - * @return bool + * @return int * @throws \OCP\DB\Exception */ public function deleteAll($userId) { From ac0f8e3614b02f000df76fd1a2b5255c04941d56 Mon Sep 17 00:00:00 2001 From: Arne Hamann Date: Mon, 23 Jan 2023 21:16:25 +0100 Subject: [PATCH 4/9] Some fixes Signed-off-by: Arne Hamann --- lib/DB/GeophotoMapper.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/DB/GeophotoMapper.php b/lib/DB/GeophotoMapper.php index 7762cdeeb..f831cad1d 100644 --- a/lib/DB/GeophotoMapper.php +++ b/lib/DB/GeophotoMapper.php @@ -200,8 +200,7 @@ public function deleteAll($userId) { public function updateByFileId($fileId, $lat, $lng) { $qb = $this->db->getQueryBuilder(); - $qb->update( - )->set('lat', $qb->createNamedParameter($lat) + $qb->update($this->getTableName())->set('lat', $qb->createNamedParameter($lat) )->set('lng', $qb->createNamedParameter($lng) )->where('file_id', $qb->createNamedParameter($fileId)); From 4436b614548d2f652b5919c935f225df3f1185e0 Mon Sep 17 00:00:00 2001 From: Arne Hamann Date: Mon, 23 Jan 2023 21:18:20 +0100 Subject: [PATCH 5/9] Delete directly Signed-off-by: Arne Hamann --- lib/DB/GeophotoMapper.php | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/DB/GeophotoMapper.php b/lib/DB/GeophotoMapper.php index f831cad1d..03babaa99 100644 --- a/lib/DB/GeophotoMapper.php +++ b/lib/DB/GeophotoMapper.php @@ -165,13 +165,16 @@ public function deleteByFileId($fileId) { * @throws \OCP\DB\Exception */ public function deleteByFileIdUserId($fileId, $userId) { - try { - $entity = $this->findByFileIdUserId( $fileId, $userId); - } catch (DoesNotExistException | MultipleObjectsReturnedException $e) { - return false; - } - $this->delete($entity); - return true; + $qb = $this->db->getQueryBuilder(); + + $qb->delete() + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)) + )->andWhere( + $qb->expr()->eq('file_id', $qb->createNamedParameter($fileId, IQueryBuilder::PARAM_STR)) + ); + return $qb->executeStatement(); } /** From 751697923b89c05718d7a179318d72cad94d301d Mon Sep 17 00:00:00 2001 From: Arne Hamann Date: Mon, 23 Jan 2023 21:21:22 +0100 Subject: [PATCH 6/9] Delete needs table_name Signed-off-by: Arne Hamann --- lib/DB/GeophotoMapper.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/DB/GeophotoMapper.php b/lib/DB/GeophotoMapper.php index 03babaa99..63fbfa55b 100644 --- a/lib/DB/GeophotoMapper.php +++ b/lib/DB/GeophotoMapper.php @@ -149,8 +149,7 @@ public function findAllNonLocalized($userId, $limit=null, $offset=null) { public function deleteByFileId($fileId) { $qb = $this->db->getQueryBuilder(); - $qb->delete() - ->from($this->getTableName()) + $qb->delete($this->getTableName()) ->where( $qb->expr()->eq('file_id', $qb->createNamedParameter($fileId, IQueryBuilder::PARAM_STR)) ); @@ -167,8 +166,7 @@ public function deleteByFileId($fileId) { public function deleteByFileIdUserId($fileId, $userId) { $qb = $this->db->getQueryBuilder(); - $qb->delete() - ->from($this->getTableName()) + $qb->delete($this->getTableName()) ->where( $qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)) )->andWhere( @@ -185,8 +183,7 @@ public function deleteByFileIdUserId($fileId, $userId) { public function deleteAll($userId) { $qb = $this->db->getQueryBuilder(); - $qb->delete() - ->from($this->getTableName()) + $qb->delete($this->getTableName()) ->where( $qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)) ); From 4ce6f58659e61d90e4ac2b2997a2b15c2659a82e Mon Sep 17 00:00:00 2001 From: Arne Hamann Date: Mon, 23 Jan 2023 21:34:49 +0100 Subject: [PATCH 7/9] Use try statements Signed-off-by: Arne Hamann --- lib/Service/PhotofilesService.php | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/Service/PhotofilesService.php b/lib/Service/PhotofilesService.php index e95f9d41f..6e01ec710 100644 --- a/lib/Service/PhotofilesService.php +++ b/lib/Service/PhotofilesService.php @@ -16,6 +16,7 @@ use OCA\Maps\Helper\ExifDataInvalidException; use OCA\Maps\Helper\ExifDataNoLocationException; use OCA\Maps\Helper\ExifGeoData; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\Files\FileInfo; use OCP\ICacheFactory; use OCP\IL10N; @@ -157,15 +158,14 @@ public function updateByFileNow(Node $file) { if (!is_null($exif)) { $ownerId = $file->getOwner()->getUID(); // in case there is no entry for this file yet (normally there is because non-localized photos are added) - if ($this->photoMapper->findByFileIdUserId($file->getId(), $ownerId) === null) { - // TODO insert for all users having access to this file, not just the owner - $this->insertPhoto($file, $ownerId, $exif); - } - else { - $this->updatePhoto($file, $exif); + try { + $this->photoMapper->findByFileIdUserId($file->getId(), $ownerId); + $this->updatePhoto($file, $exif); $this->photosCache->clear($ownerId); $this->nonLocalizedPhotosCache->clear($ownerId); - } + } catch (DoesNotExistException) { + $this->insertPhoto($file, $ownerId, $exif); + } } } } @@ -309,7 +309,9 @@ public function addPhotoNow($photo, $userId) { // so we need to be sure it's not inserted several times // by checking if it already exists in DB // OR by using file_id in primary key - if ($this->photoMapper->findByFileIdUserId($photo->getId(), $userId) === null) { + try { + $this->photoMapper->findByFileIdUserId($photo->getId(), $userId); + } catch (DoesNotExistException) { $this->insertPhoto($photo, $userId, $exif); } $this->photosCache->clear($userId); From 59c0efb65268fad1515b7106a42f54fa3ce33da0 Mon Sep 17 00:00:00 2001 From: Arne Hamann Date: Mon, 23 Jan 2023 21:38:28 +0100 Subject: [PATCH 8/9] Update lib/DB/GeophotoMapper.php Co-authored-by: Joas Schilling <213943+nickvergessen@users.noreply.github.com> Signed-off-by: Arne Hamann --- lib/DB/GeophotoMapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/DB/GeophotoMapper.php b/lib/DB/GeophotoMapper.php index 63fbfa55b..247bbd418 100644 --- a/lib/DB/GeophotoMapper.php +++ b/lib/DB/GeophotoMapper.php @@ -160,7 +160,7 @@ public function deleteByFileId($fileId) { /** * @param $fileId * @param $userId - * @return bool + * @return int * @throws \OCP\DB\Exception */ public function deleteByFileIdUserId($fileId, $userId) { From eb71aa9784174af0647634fb9ed939f680b60ba7 Mon Sep 17 00:00:00 2001 From: Arne Hamann Date: Mon, 23 Jan 2023 21:56:13 +0100 Subject: [PATCH 9/9] Fix Update Signed-off-by: Arne Hamann --- lib/DB/GeophotoMapper.php | 7 ++++--- lib/Service/PhotofilesService.php | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/DB/GeophotoMapper.php b/lib/DB/GeophotoMapper.php index 247bbd418..d6c05a841 100644 --- a/lib/DB/GeophotoMapper.php +++ b/lib/DB/GeophotoMapper.php @@ -200,9 +200,10 @@ public function deleteAll($userId) { public function updateByFileId($fileId, $lat, $lng) { $qb = $this->db->getQueryBuilder(); - $qb->update($this->getTableName())->set('lat', $qb->createNamedParameter($lat) - )->set('lng', $qb->createNamedParameter($lng) - )->where('file_id', $qb->createNamedParameter($fileId)); + $qb->update($this->getTableName()) + ->set('lat', $qb->createNamedParameter($lat)) + ->set('lng', $qb->createNamedParameter($lng)) + ->where($qb->expr()->eq('file_id', $qb->createNamedParameter($fileId))); return $qb->executeStatement(); } diff --git a/lib/Service/PhotofilesService.php b/lib/Service/PhotofilesService.php index 6e01ec710..500a81faa 100644 --- a/lib/Service/PhotofilesService.php +++ b/lib/Service/PhotofilesService.php @@ -163,7 +163,7 @@ public function updateByFileNow(Node $file) { $this->updatePhoto($file, $exif); $this->photosCache->clear($ownerId); $this->nonLocalizedPhotosCache->clear($ownerId); - } catch (DoesNotExistException) { + } catch (DoesNotExistException $exception) { $this->insertPhoto($file, $ownerId, $exif); } } @@ -311,7 +311,7 @@ public function addPhotoNow($photo, $userId) { // OR by using file_id in primary key try { $this->photoMapper->findByFileIdUserId($photo->getId(), $userId); - } catch (DoesNotExistException) { + } catch (DoesNotExistException $exception) { $this->insertPhoto($photo, $userId, $exif); } $this->photosCache->clear($userId);