From 1dfb69e536012550024dcf4777ef9a0d71cbbcdf Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 11 Jun 2026 16:06:26 +0200 Subject: [PATCH 1/5] refactor(db): Introduce repository concept Signed-off-by: Carl Schwan --- lib/AppFramework/Db/Attribute/Column.php | 41 ++ lib/AppFramework/Db/Attribute/Entity.php | 36 ++ lib/AppFramework/Db/Attribute/Id.php | 40 ++ lib/AppFramework/Db/Repository.php | 459 ++++++++++++++++++ lib/Controller/DevicesController.php | 29 +- lib/DB/DeviceShare.php | 41 +- lib/DB/DeviceShareMapper.php | 229 --------- lib/DB/DeviceShareRepository.php | 91 ++++ lib/Listener/ShareListener.php | 2 + .../Unit/Controller/DevicesControllerTest.php | 4 +- vendor-bin/psalm/composer.json | 2 +- vendor-bin/psalm/composer.lock | 16 +- 12 files changed, 712 insertions(+), 278 deletions(-) create mode 100644 lib/AppFramework/Db/Attribute/Column.php create mode 100644 lib/AppFramework/Db/Attribute/Entity.php create mode 100644 lib/AppFramework/Db/Attribute/Id.php create mode 100644 lib/AppFramework/Db/Repository.php delete mode 100644 lib/DB/DeviceShareMapper.php create mode 100644 lib/DB/DeviceShareRepository.php diff --git a/lib/AppFramework/Db/Attribute/Column.php b/lib/AppFramework/Db/Attribute/Column.php new file mode 100644 index 000000000..ee5909273 --- /dev/null +++ b/lib/AppFramework/Db/Attribute/Column.php @@ -0,0 +1,41 @@ + $generatorClass */ + public ?string $generatorClass = null, + ) { + } +} diff --git a/lib/AppFramework/Db/Repository.php b/lib/AppFramework/Db/Repository.php new file mode 100644 index 000000000..404012bb5 --- /dev/null +++ b/lib/AppFramework/Db/Repository.php @@ -0,0 +1,459 @@ + */ + private array $_mappingColumnToTypes = []; + + /** @var array */ + private array $_mappingColumnToProperty = []; + + /** @var array */ + private array $_mappingPropertyToColumn = []; + + /** @var \ReflectionClass */ + private \ReflectionClass $reflection; + + private string $idProperty; + + /** + * @param IDBConnection $connection + * @param class-string $entityClass + * @throws \ReflectionException + */ + public function __construct( + protected readonly IDBConnection $connection, + protected readonly string $entityClass, + ) { + $this->reflection = new \ReflectionClass($this->entityClass); + + $entities = $this->reflection->getAttributes(Entity::class, \ReflectionAttribute::IS_INSTANCEOF); + if (count($entities) !== 1) { + throw new \InvalidArgumentException('The given entity is missing or has too many of the required #[Entity] attribute'); + } + + $this->tableName = $entities[0]->newInstance()->name; + + foreach ($this->reflection->getProperties() as $property) { + $columnAttributes = $property->getAttributes(Column::class, \ReflectionAttribute::IS_INSTANCEOF); + if (count($columnAttributes) === 0) { + continue; + } + + /** @var Column $columnAttribute */ + $columnAttribute = $columnAttributes[0]->newInstance(); + $this->_mappingColumnToTypes[$columnAttribute->name] = $columnAttribute->type; + $this->_mappingColumnToProperty[$columnAttribute->name] = $property->getName(); + $this->_mappingPropertyToColumn[$property->getName()] = $columnAttribute->name; + + /** @var list<\ReflectionAttribute> $ids */ + $ids = $property->getAttributes(Id::class, \ReflectionAttribute::IS_INSTANCEOF); + if (!empty($ids)) { + $this->idProperty = $property->getName(); + } + } + } + + /** + * Runs a sql query and yields each resulting entity to obtain database entries in a memory-efficient way + * + * @param IQueryBuilder $query + * @return Generator Generator of fetched entities + * @psalm-return Generator Generator of fetched entities + * @throws Exception + */ + public function yieldEntities(IQueryBuilder $query): Generator { + $result = $query->executeQuery(); + try { + while ($row = $result->fetch()) { + yield $this->mapRowToEntity($row); + } + } finally { + $result->closeCursor(); + } + } + + /** + * Runs a sql query and returns an array of entities + * + * @param IQueryBuilder $query + * @psalm-return list all fetched entities + * @throws Exception + */ + public function findEntities(IQueryBuilder $query): array { + return iterator_to_array($this->yieldEntities($query)); + } + + private function buildDebugMessage(string $msg, IQueryBuilder $sql): string { + return $msg . ': query "' . $sql->getSQL() . '"; '; + } + + /** + * @param array $row + * @return T + */ + private function mapRowToEntity(mixed $row): object { + $entity = new $this->entityClass(); + foreach ($row as $column => $value) { + $property = $this->_mappingColumnToProperty[$column]; + $type = $this->_mappingColumnToTypes[$column]; + if ($type === Types::BLOB) { + // (B)LOB is treated as string when we read from the DB + if (is_resource($value)) { + $value = stream_get_contents($value); + } + $type = Types::STRING; + } + + if ($column === $this->idProperty) { + $entity->$property = (string)$value; + continue; + } + + switch ($type) { + case Types::BIGINT: + case Types::SMALLINT: + settype($value, Types::INTEGER); + break; + case Types::BINARY: + case Types::DECIMAL: + case Types::TEXT: + settype($value, Types::STRING); + break; + case Types::TIME: + case Types::DATE: + case Types::DATETIME: + case Types::DATETIME_TZ: + if (!$value instanceof \DateTime) { + $value = new \DateTime($value); + } + break; + case Types::TIME_IMMUTABLE: + case Types::DATE_IMMUTABLE: + case Types::DATETIME_IMMUTABLE: + case Types::DATETIME_TZ_IMMUTABLE: + if (!$value instanceof \DateTimeImmutable) { + $value = new \DateTimeImmutable($value); + } + break; + case Types::JSON: + if (!is_array($value)) { + $value = json_decode($value, true); + } + break; + } + $entity->$property = $value; + } + return $entity; + } + + /** + * Insert the entity in the database. + * + * This will additionally generate a value for the primary key. + * + * @psalm-param T $entity + * @return T + */ + public function insert(object $entity): object { + $insert = $this->connection->getQueryBuilder(); + + $isSnowflake = false; + $primaryProperty = null; + $values = []; + foreach ($this->reflection->getProperties() as $property) { + /** @var list<\ReflectionAttribute> $columns */ + $columns = $property->getAttributes(Column::class, \ReflectionAttribute::IS_INSTANCEOF); + if (empty($columns)) { + continue; // Not in the DB + } + + $column = $columns[0]->newInstance(); + + /** @var list<\ReflectionAttribute> $ids */ + $ids = $property->getAttributes(Id::class, \ReflectionAttribute::IS_INSTANCEOF); + if (count($ids) > 0 && $property->getValue($entity) === null) { + $primaryProperty = $property; + $generatorClass = $ids[0]->newInstance()->generatorClass; + $generator = Server::get($generatorClass); + /** @psalm-suppress UndefinedClass NC 33 and above */ + if (class_exists(ISnowflakeGenerator::class) && $generator instanceof ISnowflakeGenerator) { + $isSnowflake = true; + /** @psalm-suppress UndefinedClass */ + $values[$column->name] = $generator->nextId(); + $property->setValue($entity, $insert->createNamedParameter($values[$column->name])); + } + } else { + $type = $this->getParameterType($column->type, false); + $values[$column->name] = $insert->createNamedParameter($property->getValue($entity), $type); + } + } + + $insert->insert($this->tableName) + ->values($values) + ->executeStatement(); + + if (!$isSnowflake) { + $primaryProperty->setValue($entity, $insert->getLastInsertId()); + } + return $entity; + } + + /** + * @psalm-param T $entity + * @return T + */ + public function update(object $entity): object { + $update = $this->connection->getQueryBuilder(); + $update->update($this->tableName); + + foreach ($this->reflection->getProperties() as $property) { + /** @var list<\ReflectionAttribute> $columns */ + $columns = $property->getAttributes(Column::class, \ReflectionAttribute::IS_INSTANCEOF); + if (empty($columns)) { + continue; // Not in the DB + } + + $column = $columns[0]->newInstance(); + + if (count($property->getAttributes(Id::class, \ReflectionAttribute::IS_INSTANCEOF)) !== 0) { + if ($property->getValue($entity) === null) { + throw new \LogicException('Trying to update an entity with no primary key set.'); + } + + $update->andWhere($update->expr()->eq($this->_mappingPropertyToColumn[$this->idProperty], $update->createNamedParameter($property->getValue($entity)))); + // don't update the id + continue; + }; + + $type = $this->getParameterType($column->type, false); + $update->set($column->name, $update->createNamedParameter($property->getValue($entity), $type)); + } + + $update->executeStatement(); + return $entity; + } + + public function delete(object $entity): void { + $delete = $this->connection->getQueryBuilder(); + $delete->delete($this->tableName); + + $foundId = false; + foreach ($this->reflection->getProperties() as $property) { + $columns = $property->getAttributes(Column::class, \ReflectionAttribute::IS_INSTANCEOF); + if (empty($columns)) { + continue; // Not in the DB + } + + $column = $columns[0]->newInstance(); + + if (count($property->getAttributes(Id::class, \ReflectionAttribute::IS_INSTANCEOF)) !== 0) { + $delete->andWhere($delete->expr()->eq($column->name, $property->getValue($entity))); + $foundId = true; + }; + } + + if (!$foundId) { + throw new \LogicException('The given entity is missing a required #[Id] attribute on one of its properties.'); + } + + $delete->executeStatement(); + } + + /** + * @param Types::* $type + * @return IQueryBuilder::PARAM_* + */ + private function getParameterType(string $type, bool $isArray): string|int { + if ($isArray) { + return match ($type) { + Types::INTEGER, Types::SMALLINT => IQueryBuilder::PARAM_INT_ARRAY, + Types::STRING => IQueryBuilder::PARAM_STR_ARRAY, + Types::JSON => IQueryBuilder::PARAM_JSON, + default => throw new \LogicException("Parameter type '$type' is not supported as an array."), + }; + } + + return match ($type) { + Types::INTEGER, Types::SMALLINT => IQueryBuilder::PARAM_INT, + Types::STRING => IQueryBuilder::PARAM_STR, + Types::BOOLEAN => IQueryBuilder::PARAM_BOOL, + Types::BLOB => IQueryBuilder::PARAM_LOB, + Types::DATE, Types::DATETIME => IQueryBuilder::PARAM_DATETIME_MUTABLE, + Types::DATETIME_TZ => IQueryBuilder::PARAM_DATETIME_TZ_MUTABLE, + Types::DATE_IMMUTABLE => IQueryBuilder::PARAM_DATE_IMMUTABLE, + Types::DATETIME_IMMUTABLE => IQueryBuilder::PARAM_DATETIME_IMMUTABLE, + Types::DATETIME_TZ_IMMUTABLE => IQueryBuilder::PARAM_DATETIME_TZ_IMMUTABLE, + Types::TIME => IQueryBuilder::PARAM_TIME_MUTABLE, + Types::TIME_IMMUTABLE => IQueryBuilder::PARAM_TIME_IMMUTABLE, + Types::JSON => IQueryBuilder::PARAM_JSON, + default => IQueryBuilder::PARAM_STR, + }; + } + + /** + * Finds entities by a set of criteria. + * + * Use the property names for the criteria and orderBy key. + * + * @param array> $criteria + * @param array|null $orderBy + * @return \Generator + * @since 33.0.0 + */ + public function findBy(array $criteria, array $orderBy = [], ?int $limit = null, ?int $offset = null): \Generator { + $qb = $this->getSelectQueryBuilder($criteria, $orderBy); + + if ($limit !== null) { + $qb->setMaxResults($limit); + } + + if ($offset !== null) { + $qb->setFirstResult($offset); + } + + return $this->yieldEntities($qb); + } + + /** + * @param array> $criteria + * @return int The number of rows deleted + * @throws Exception + */ + public function deleteBy(array $criteria, ?int $limit = null): int { + $qb = $this->connection->getQueryBuilder(); + $qb->delete($this->tableName); + + foreach ($criteria as $property => $value) { + $column = $this->_mappingPropertyToColumn[$property]; + $type = $this->getParameterType($this->_mappingColumnToTypes[$column], is_array($value)); + $type = $this->getParameterType($this->_mappingColumnToTypes[$column], is_array($value)); + if (is_array($value)) { + // IN expression + $qb->andWhere($qb->expr()->in($column, $qb->createNamedParameter($value, $type))); + } else { + // = expression + $qb->andWhere($qb->expr()->eq($column, $qb->createNamedParameter($value, $type))); + } + } + + if ($limit !== null) { + $qb->setMaxResults($limit); + } + + return $qb->executeStatement(); + } + + /** + * Finds a single entity by a set of criteria. + * + * @param array> $criteria + * @param array|null $orderBy + * @return T|null + */ + public function findOneBy(array $criteria, array $orderBy = []): ?object { + $qb = $this->getSelectQueryBuilder($criteria, $orderBy); + + $qb->setMaxResults(1); + + try { + return $this->findEntity($qb); + } catch (DoesNotExistException) { + return null; + } + } + + /** + * @param array> $criteria + * @param array|null $orderBy + */ + private function getSelectQueryBuilder(array $criteria, array $orderBy = []): IQueryBuilder { + $qb = $this->connection->getQueryBuilder(); + $qb->select('*') + ->from($this->tableName); + + foreach ($criteria as $property => $value) { + $column = $this->_mappingPropertyToColumn[$property]; + $type = $this->getParameterType($this->_mappingColumnToTypes[$column], is_array($value)); + if (is_array($value)) { + // IN expression + $qb->andWhere($qb->expr()->in($column, $qb->createNamedParameter($value, $type))); + } else { + // = expression + $qb->andWhere($qb->expr()->eq($column, $qb->createNamedParameter($value, $type))); + } + } + foreach ($orderBy as $field => $direction) { + $qb->addOrderBy($qb->createNamedParameter($field), $direction); + } + + return $qb; + } + + /** + * Returns a db result and throws exceptions when there are more or less + * results + * + * @param IQueryBuilder $query + * @psalm-return T the entity + * @throws Exception + * @throws MultipleObjectsReturnedException if more than one item exist + * @throws DoesNotExistException if the item does not exist + * @since 33.0.0 + */ + protected function findEntity(IQueryBuilder $query): object { + $result = $query->executeQuery(); + + $row = $result->fetch(); + if ($row === false) { + $result->closeCursor(); + $msg = $this->buildDebugMessage( + 'Did expect one result but found none when executing', $query + ); + throw new DoesNotExistException($msg); + } + + $row2 = $result->fetch(); + $result->closeCursor(); + if ($row2 !== false) { + $msg = $this->buildDebugMessage( + 'Did not expect more than one result when executing', $query + ); + throw new MultipleObjectsReturnedException($msg); + } + + return $this->mapRowToEntity($row); + } + + public function getTableName(): string { + return $this->tableName; + } +} diff --git a/lib/Controller/DevicesController.php b/lib/Controller/DevicesController.php index 7a3d233ca..9d6fdb408 100644 --- a/lib/Controller/DevicesController.php +++ b/lib/Controller/DevicesController.php @@ -12,7 +12,7 @@ namespace OCA\Maps\Controller; -use OCA\Maps\DB\DeviceShareMapper; +use OCA\Maps\DB\DeviceShareRepository; use OCA\Maps\Service\DevicesService; use OCP\AppFramework\Controller; use OCP\AppFramework\Db\DoesNotExistException; @@ -41,7 +41,7 @@ public function __construct( IAppConfig $appConfig, private readonly IL10N $l, private readonly DevicesService $devicesService, - private readonly DeviceShareMapper $deviceShareMapper, + private readonly DeviceShareRepository $deviceShareMapper, private readonly IDateTimeZone $dateTimeZone, private readonly IRootFolder $root, private readonly ?string $userId, @@ -67,14 +67,13 @@ public function getDevices(?array $tokens = null, ?int $myMapId = null): DataRes $deviceIds = array_column($devices, 'id'); $shares = $this->deviceShareMapper->findByDeviceIds($deviceIds); foreach ($shares as $s) { - $devices[$s->getDeviceId()]['shares'][] = $s; + $devices[$s->deviceId]['shares'][] = $s; } } else { $devices = []; $userFolder = $this->root->getUserFolder($this->userId); - $folders = $userFolder->getById($myMapId); - $folder = array_shift($folders); - if (is_null($folder)) { + $folder = $userFolder->getFirstNodeById($myMapId); + if (!$folder instanceof Folder) { return new DataResponse($this->l->t('Map not Found'), 404); } $shares = $this->devicesService->getSharedDevicesFromFolder($folder); @@ -242,8 +241,10 @@ public function getSharedDevices(?int $myMapId = null): DataResponse { if (is_null($myMapId) || $myMapId === 0) { $sharedDevices = []; } else { - $folders = $this->userFolder->getById($myMapId); - $folder = array_shift($folders); + $folder = $this->userFolder->getFirstNodeById($myMapId); + if (!$folder instanceof Folder) { + return new DataResponse($this->l->t('Map not Found'), 404); + } $sharedDevices = $this->devicesService->getSharedDevicesFromFolder($folder); } @@ -277,9 +278,9 @@ public function removeDeviceShare(string $token): DataResponse { } catch (DoesNotExistException) { throw new NotFoundException(); } - $device = $this->devicesService->getDeviceFromDB($share->getDeviceId(), $this->userId); + $device = $this->devicesService->getDeviceFromDB($share->deviceId, $this->userId); if ($device !== null) { - return new DataResponse($this->deviceShareMapper->removeById($share->getId())); + return new DataResponse($this->deviceShareMapper->removeById($share->id)); } else { throw new NotFoundException(); } @@ -296,8 +297,7 @@ public function addSharedDeviceToMap(string $token, $targetMapId): DataResponse } catch (DoesNotExistException) { return new DataResponse($this->l->t('Share not Found'), 404); } - $folders = $this->userFolder->getById($targetMapId); - $folder = array_shift($folders); + $folder = $this->userFolder->getFirstNodeById($targetMapId); if (!$folder instanceof Folder) { return new DataResponse($this->l->t('Map not Found'), 404); } @@ -308,7 +308,7 @@ public function addSharedDeviceToMap(string $token, $targetMapId): DataResponse } $data = json_decode((string)$file->getContent(), true); foreach ($data as $s) { - if ($s->token == $share->getToken()) { + if ($s->token == $share->token) { return new DataResponse($this->l->t('Share was already on map')); } } @@ -318,8 +318,7 @@ public function addSharedDeviceToMap(string $token, $targetMapId): DataResponse } public function removeSharedDeviceFromMap(string $token, int $myMapId): DataResponse { - $folders = $this->userFolder->getById($myMapId); - $folder = array_shift($folders); + $folder = $this->userFolder->getFirstNodeById($myMapId); if (!$folder instanceof Folder) { return new DataResponse($this->l->t('Map not Found'), 404); } diff --git a/lib/DB/DeviceShare.php b/lib/DB/DeviceShare.php index 207b370fe..aebbc43b4 100644 --- a/lib/DB/DeviceShare.php +++ b/lib/DB/DeviceShare.php @@ -24,29 +24,24 @@ namespace OCA\Maps\DB; -use OCP\AppFramework\Db\Entity; +use OCA\Maps\AppFramework\Db\Attribute\Column; +use OCA\Maps\AppFramework\Db\Attribute\Entity; use OCP\DB\Types; -/** - * @method string getToken() - * @method string getDeviceId() - * @method string getTimestampFrom() - * @method string getTimestampTo() - * @method string setToken(string $token) - * @method string setDeviceId(int $deviceId) - * @method string setTimestampFrom(int $timestampFrom) - * @method string setTimestampTo(int $timestampTo) - */ -class DeviceShare extends Entity { - public $token; - public $deviceId; - public $timestampFrom; - public $timestampTo; - - public function __construct() { - $this->addType('token', Types::STRING); - $this->addType('deviceId', Types::INTEGER); - $this->addType('timestampFrom', Types::INTEGER); - $this->addType('timestampTo', Types::INTEGER); - } +#[Entity(name: 'maps_device_shares')] +class DeviceShare { + #[Column(name: 'id', type: Types::STRING)] + public int $id; + + #[Column(name: 'token', type: Types::STRING)] + public string $token; + + #[Column(name: 'device_id', type: Types::INTEGER)] + public int $deviceId; + + #[Column(name: 'timestamp_from', type: Types::INTEGER)] + public int $timestampFrom; + + #[Column(name: 'timestamp_to', type: Types::INTEGER)] + public int $timestampTo; } diff --git a/lib/DB/DeviceShareMapper.php b/lib/DB/DeviceShareMapper.php deleted file mode 100644 index 70b352eff..000000000 --- a/lib/DB/DeviceShareMapper.php +++ /dev/null @@ -1,229 +0,0 @@ - - * - * @author Paul Schwörer - * - * @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 OCA\Maps\DB; - -use OC\Share\Constants; -use OCP\AppFramework\Db\DoesNotExistException; -use OCP\AppFramework\Db\Entity; -use OCP\AppFramework\Db\MultipleObjectsReturnedException; -use OCP\AppFramework\Db\QBMapper; -use OCP\DB\Exception; -use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\Files\NotFoundException; -use OCP\IDBConnection; -use OCP\Security\ISecureRandom; - -/** @template-extends QBMapper */ -class DeviceShareMapper extends QBMapper { - /* @var ISecureRandom */ - private $secureRandom; - - public function __construct(IDBConnection $db, ISecureRandom $secureRandom) { - parent::__construct($db, 'maps_device_shares'); - - $this->secureRandom = $secureRandom; - } - - /** - * @param string $token - * @return DeviceShare|null - * @throws DoesNotExistException - * @throws MultipleObjectsReturnedException - */ - public function findByToken($token) { - $qb = $this->db->getQueryBuilder(); - - $qb->select('*') - ->from($this->getTableName()) - ->where( - $qb->expr()->eq('token', $qb->createNamedParameter($token, IQueryBuilder::PARAM_STR)) - ); - - return $this->findEntity($qb); - } - - /** - * @param string[] $token - * @return DeviceShare[]|null - * @throws DoesNotExistException - */ - public function findByTokens($tokens) { - $qb = $this->db->getQueryBuilder(); - - $qb->select('*') - ->from($this->getTableName()) - ->where( - $qb->expr()->in('token', $qb->createNamedParameter($tokens, IQueryBuilder::PARAM_STR)) - ); - - return $this->findEntities($qb); - } - - /** - * @param $deviceId - * @param $timestampFrom - * @param $timestampTo - * @return DeviceShare - */ - public function create($deviceId, $timestampFrom, $timestampTo): Entity { - $token = $this->secureRandom->generate( - Constants::TOKEN_LENGTH, - ISecureRandom::CHAR_HUMAN_READABLE - ); - - $newShare = new DeviceShare(); - $newShare->setToken($token); - $newShare->setDeviceId($deviceId); - $newShare->setTimestampFrom($timestampFrom); - $newShare->setTimestampTo($timestampTo); - - return $this->insert($newShare); - } - - /** - * @param $deviceId - * @return DeviceShare[] - * @throws DoesNotExistException - */ - public function findByDeviceId($deviceId) { - $qb = $this->db->getQueryBuilder(); - - $qb->select('*') - ->from($this->getTableName()) - ->where( - $qb->expr()->eq('device_id', $qb->createNamedParameter($deviceId, IQueryBuilder::PARAM_INT)) - ); - - return $this->findEntities($qb); - } - - /** - * @param $deviceIds - * @return DeviceShare[] - */ - public function findByDeviceIds($deviceIds) { - $qb = $this->db->getQueryBuilder(); - - $qb->select('*') - ->from($this->getTableName()) - ->where( - $qb->expr()->in('device_id', $qb->createNamedParameter($deviceIds, IQueryBuilder::PARAM_INT_ARRAY)) - ); - - return $this->findEntities($qb); - } - - /* - public function findByMapIdAndDeviceId($userId, $mapId, $deviceId) { - $shares = $this->findAllByMapId($userId, $mapId); - foreach ($shares as $share) { - if ($share->deviceId === $deviceId) { - return $share; - } - } - return null; - } - - - public function removeByMapIdAndDeviceId($userId, $mapId, $deviceId) { - $userFolder = $this->root->getUserFolder($userId); - $folders = $userFolder->getById($mapId); - $shares = []; - $deleted = null; - if (empty($folders)) { - return $deleted; - } - $folder = array_shift($folders); - if ($folder === null) { - return $deleted; - } - try { - $file=$folder->get(".device_shares.json"); - } catch (NotFoundException $e) { - $file=$folder->newFile(".device_shares.json", $content = '[]'); - } - $data = json_decode($file->getContent(),true); - foreach ($data as $share) { - $c = $share["deviceId"]; - if($c === $deviceId) { - $deleted = $share; - } else { - $shares[] = $share; - } - } - $file->putContent(json_encode($shares, JSON_PRETTY_PRINT)); - return $deleted; - } - */ - - /** - * @param $id - * @return DeviceShare|null - * @throws DoesNotExistException - * @throws Exception - * @throws MultipleObjectsReturnedException - */ - public function findById($id) { - $qb = $this->db->getQueryBuilder(); - - $qb->select('*') - ->from($this->getTableName()) - ->where( - $qb->expr()->eq('id', $qb->createNamedParameter($id)) - ); - - return $this->findEntity($qb); - } - - /** - * @param $id - * @return bool - */ - public function removeById($id) { - try { - $entity = $this->findById($id); - $this->delete($entity); - } catch (DoesNotExistException) { - return false; - } - return true; - } - - /** - * @param $deviceId - * @return bool - */ - public function removeAllByDeviceId($deviceId) { - try { - $entities = $this->findByDeviceId($deviceId); - foreach ($entities as $entity) { - $this->delete($entity); - } - } catch (DoesNotExistException) { - return false; - } - return true; - } -} diff --git a/lib/DB/DeviceShareRepository.php b/lib/DB/DeviceShareRepository.php new file mode 100644 index 000000000..172ccb3ba --- /dev/null +++ b/lib/DB/DeviceShareRepository.php @@ -0,0 +1,91 @@ + + * + * @author Paul Schwörer + * + * @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 OCA\Maps\DB; + +use OC\Share\Constants; +use OCA\Maps\AppFramework\Db\Repository; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; +use OCP\IDBConnection; +use OCP\Security\ISecureRandom; + +/** + * @template-extends Repository + */ +class DeviceShareRepository extends Repository { + public function __construct( + IDBConnection $db, + private readonly ISecureRandom $secureRandom, + ) { + parent::__construct($db, DeviceShare::class); + } + + /** + * @throws DoesNotExistException + * @throws MultipleObjectsReturnedException + */ + public function findByToken(string $token): ?DeviceShare { + return $this->findOneBy(['token' => $token]); + } + + public function create(int $deviceId, int $timestampFrom, int $timestampTo): DeviceShare { + $token = $this->secureRandom->generate( + Constants::TOKEN_LENGTH, + ISecureRandom::CHAR_HUMAN_READABLE + ); + + $newShare = new DeviceShare(); + $newShare->token = $token; + $newShare->deviceId = $deviceId; + $newShare->timestampFrom = $timestampFrom; + $newShare->timestampTo = $timestampTo; + + return $this->insert($newShare); + } + + /** + * @param list $deviceIds + * @return \Generator + */ + public function findByDeviceIds(array $deviceIds): \Generator { + return $this->findBy(['deviceId' => $deviceIds]); + } + + /** + * @throws DoesNotExistException + * @throws MultipleObjectsReturnedException + */ + public function findById(int $id): ?DeviceShare { + return $this->findOneBy(['id' => $id]); + } + + public function removeById(int $id): bool { + return $this->deleteBy(['id' => $id]) === 1; + } + + public function removeAllByDeviceId(int $deviceId): bool { + return $this->deleteBy(['deviceId' => $deviceId]) > 0; + } +} diff --git a/lib/Listener/ShareListener.php b/lib/Listener/ShareListener.php index 3a3543bcf..d769288f3 100644 --- a/lib/Listener/ShareListener.php +++ b/lib/Listener/ShareListener.php @@ -14,6 +14,7 @@ use OCA\Maps\Service\TracksService; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; +use OCP\Files\File; use OCP\Files\Folder; use OCP\Share\Events\BeforeShareDeletedEvent; use OCP\Share\Events\ShareCreatedEvent; @@ -69,6 +70,7 @@ private function handleBeforeShareDeleted(BeforeShareDeletedEvent $event): void private function handleShareCreated(ShareCreatedEvent $event): void { $share = $event->getShare(); if ($share->getNodeType() === 'file') { + /** @var File $file */ $file = $share->getNode(); $this->photofilesService->addByFile($file); $this->tracksService->safeAddByFile($file); diff --git a/tests/Unit/Controller/DevicesControllerTest.php b/tests/Unit/Controller/DevicesControllerTest.php index 507e8387c..102c2f846 100644 --- a/tests/Unit/Controller/DevicesControllerTest.php +++ b/tests/Unit/Controller/DevicesControllerTest.php @@ -13,7 +13,7 @@ namespace OCA\Maps\Controller; use OCA\Maps\AppInfo\Application; -use OCA\Maps\DB\DeviceShareMapper; +use OCA\Maps\DB\DeviceShareRepository; use OCA\Maps\Service\DevicesService; use OCP\AppFramework\Services\IAppConfig; use OCP\Files\Folder; @@ -75,7 +75,7 @@ protected function setUp(): void { $c->get(IAppConfig::class), $c->get(IFactory::class)->get('maps'), $c->get(DevicesService::class), - $c->get(DeviceShareMapper::class), + $c->get(DeviceShareRepository::class), $c->get(\OCP\IDateTimeZone::class), $c->get(IRootFolder::class), 'test' diff --git a/vendor-bin/psalm/composer.json b/vendor-bin/psalm/composer.json index d156c448e..dd96fec1d 100644 --- a/vendor-bin/psalm/composer.json +++ b/vendor-bin/psalm/composer.json @@ -6,7 +6,7 @@ }, "require-dev": { "doctrine/dbal": "^3.10.0", - "nextcloud/ocp": "dev-stable31", + "nextcloud/ocp": "dev-stable32", "sabre/dav": "^4.7.0", "symfony/console": "^6.4", "vimeo/psalm": "^6.10" diff --git a/vendor-bin/psalm/composer.lock b/vendor-bin/psalm/composer.lock index c5ed58c6c..1680d9538 100644 --- a/vendor-bin/psalm/composer.lock +++ b/vendor-bin/psalm/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "8ae0df6d06afb578f94b41e46fb6b4e5", + "content-hash": "8dcc41156ad04752614fc55994e6cab8", "packages": [], "packages-dev": [ { @@ -1833,16 +1833,16 @@ }, { "name": "nextcloud/ocp", - "version": "dev-stable31", + "version": "dev-stable32", "source": { "type": "git", "url": "https://github.com/nextcloud-deps/ocp.git", - "reference": "b369e02e33a1b9327eb3c4c5f639135b66e7bc8f" + "reference": "ef43111aa5096fc2818ff79631cd0801ec4e3cc4" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/nextcloud-deps/ocp/zipball/b369e02e33a1b9327eb3c4c5f639135b66e7bc8f", - "reference": "b369e02e33a1b9327eb3c4c5f639135b66e7bc8f", + "url": "https://api.github.com/repos/nextcloud-deps/ocp/zipball/ef43111aa5096fc2818ff79631cd0801ec4e3cc4", + "reference": "ef43111aa5096fc2818ff79631cd0801ec4e3cc4", "shasum": "" }, "require": { @@ -1855,7 +1855,7 @@ "type": "library", "extra": { "branch-alias": { - "dev-stable31": "31.0.0-dev" + "dev-stable32": "32.0.0-dev" } }, "notification-url": "https://packagist.org/downloads/", @@ -1875,9 +1875,9 @@ "description": "Composer package containing Nextcloud's public OCP API and the unstable NCU API", "support": { "issues": "https://github.com/nextcloud-deps/ocp/issues", - "source": "https://github.com/nextcloud-deps/ocp/tree/stable31" + "source": "https://github.com/nextcloud-deps/ocp/tree/stable32" }, - "time": "2026-02-06T01:05:41+00:00" + "time": "2026-06-03T02:42:41+00:00" }, { "name": "nikic/php-parser", From ba0a727814bf0ef7ad19ca8ae4dd95b9a1976c7f Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 11 Jun 2026 19:08:41 +0200 Subject: [PATCH 2/5] refactor: port remaining entities to new API Signed-off-by: Carl Schwan --- lib/AppFramework/Db/Repository.php | 11 +- lib/Controller/FavoritesController.php | 6 +- .../PublicFavoritePageController.php | 8 +- .../PublicFavoritesApiController.php | 13 +- lib/Controller/PublicFavoritesController.php | 4 +- lib/DB/DeviceShare.php | 12 +- lib/DB/FavoriteShare.php | 39 ++-- ...Mapper.php => FavoriteShareRepository.php} | 149 ++++--------- lib/DB/Geophoto.php | 38 ++-- lib/DB/GeophotoMapper.php | 210 ------------------ lib/DB/GeophotoRepository.php | 152 +++++++++++++ lib/Service/GeophotoService.php | 60 ++--- lib/Service/PhotofilesService.php | 40 ++-- .../Db/FavoriteShareMapperTest.php | 6 +- .../Controller/FavoritesControllerTest.php | 6 +- .../Unit/Controller/PhotosControllerTest.php | 4 +- .../PublicFavoritePageControllerTest.php | 8 +- .../PublicFavoritesApiControllerTest.php | 15 +- 18 files changed, 325 insertions(+), 456 deletions(-) rename lib/DB/{FavoriteShareMapper.php => FavoriteShareRepository.php} (51%) delete mode 100644 lib/DB/GeophotoMapper.php create mode 100644 lib/DB/GeophotoRepository.php diff --git a/lib/AppFramework/Db/Repository.php b/lib/AppFramework/Db/Repository.php index 404012bb5..3ccfd4a87 100644 --- a/lib/AppFramework/Db/Repository.php +++ b/lib/AppFramework/Db/Repository.php @@ -377,18 +377,15 @@ public function deleteBy(array $criteria, ?int $limit = null): int { * * @param array> $criteria * @param array|null $orderBy - * @return T|null + * @return T + * @throws DoesNotExistException */ - public function findOneBy(array $criteria, array $orderBy = []): ?object { + public function findOneBy(array $criteria, array $orderBy = []): object { $qb = $this->getSelectQueryBuilder($criteria, $orderBy); $qb->setMaxResults(1); - try { - return $this->findEntity($qb); - } catch (DoesNotExistException) { - return null; - } + return $this->findEntity($qb); } /** diff --git a/lib/Controller/FavoritesController.php b/lib/Controller/FavoritesController.php index 90905bfbf..4a8474064 100644 --- a/lib/Controller/FavoritesController.php +++ b/lib/Controller/FavoritesController.php @@ -14,7 +14,7 @@ namespace OCA\Maps\Controller; -use OCA\Maps\DB\FavoriteShareMapper; +use OCA\Maps\DB\FavoriteShareRepository; use OCA\Maps\Service\FavoritesService; use OCP\AppFramework\Controller; use OCP\AppFramework\Db\DoesNotExistException; @@ -46,7 +46,7 @@ public function __construct( IRootFolder $rootFolder, private readonly FavoritesService $favoritesService, private readonly IDateTimeZone $dateTimeZone, - private readonly FavoriteShareMapper $favoriteShareMapper, + private readonly FavoriteShareRepository $favoriteShareMapper, private readonly ?string $userId, ) { parent::__construct($appName, $request); @@ -214,7 +214,7 @@ public function renameCategories(array $categories, string $newName, ?int $myMap // Rename share if one exists try { $share = $this->favoriteShareMapper->findByOwnerAndCategory($this->userId, $cat); - $share->setCategory($newName); + $share->category = $newName; $this->favoriteShareMapper->update($share); } catch (DoesNotExistException|MultipleObjectsReturnedException) { } diff --git a/lib/Controller/PublicFavoritePageController.php b/lib/Controller/PublicFavoritePageController.php index 46d023d01..bd4eee37b 100644 --- a/lib/Controller/PublicFavoritePageController.php +++ b/lib/Controller/PublicFavoritePageController.php @@ -24,7 +24,7 @@ namespace OCA\Maps\Controller; -use OCA\Maps\DB\FavoriteShareMapper; +use OCA\Maps\DB\FavoriteShareRepository; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Http; @@ -43,7 +43,7 @@ public function __construct( string $appName, IRequest $request, ISession $session, - private readonly FavoriteShareMapper $favoriteShareMapper, + private readonly FavoriteShareRepository $favoriteShareMapper, \OCP\IAppConfig $appConfig, ) { $this->appConfig = $appConfig; @@ -76,9 +76,9 @@ public function sharedFavoritesCategory($token) { $response = new PublicTemplateResponse('maps', 'public/favorites_index', []); - $ownerName = \OCP\Server::get(IUserManager::class)->get($share->getOwner())->getDisplayName(); + $ownerName = \OCP\Server::get(IUserManager::class)->get($share->owner)->getDisplayName(); - $response->setHeaderTitle($share->getCategory()); + $response->setHeaderTitle($share->category); $response->setHeaderDetails('shared by ' . $ownerName); $this->addCsp($response); diff --git a/lib/Controller/PublicFavoritesApiController.php b/lib/Controller/PublicFavoritesApiController.php index 7832fba69..993246c29 100644 --- a/lib/Controller/PublicFavoritesApiController.php +++ b/lib/Controller/PublicFavoritesApiController.php @@ -24,11 +24,12 @@ namespace OCA\Maps\Controller; -use OCA\Maps\DB\FavoriteShareMapper; +use OCA\Maps\DB\FavoriteShare; use OCA\Maps\Service\FavoritesService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\PublicShareController; use OCP\IRequest; @@ -40,7 +41,7 @@ public function __construct( IRequest $request, ISession $session, private readonly \OCA\Maps\Service\FavoritesService $favoritesService, - private readonly \OCA\Maps\DB\FavoriteShareMapper $favoriteShareMapper, + private readonly \OCA\Maps\DB\FavoriteShareRepository $favoriteShareMapper, ) { parent::__construct($appName, $request, $session); } @@ -63,6 +64,7 @@ public function isValidToken(): bool { return true; } + /** public function canEdit(): bool { try { $share = $this->favoriteShareMapper->findByToken($this->getToken()); @@ -71,11 +73,12 @@ public function canEdit(): bool { } return $share->getAllowEdits(); - } + }*/ /** - * @PublicPage + * @return DataResponse}, array{}>|DataResponse */ + #[PublicPage] public function getFavorites(): DataResponse { try { $share = $this->favoriteShareMapper->findByToken($this->getToken()); @@ -85,7 +88,7 @@ public function getFavorites(): DataResponse { return new DataResponse([], Http::STATUS_INTERNAL_SERVER_ERROR); } - $favorites = $this->favoritesService->getFavoritesFromDB($share->getOwner(), 0, $share->getCategory(), false, false, true); + $favorites = $this->favoritesService->getFavoritesFromDB($share->owner, 0, $share->category, false, false, true); return new DataResponse([ 'share' => $share, diff --git a/lib/Controller/PublicFavoritesController.php b/lib/Controller/PublicFavoritesController.php index d76f7c6f6..dd8cf5ade 100644 --- a/lib/Controller/PublicFavoritesController.php +++ b/lib/Controller/PublicFavoritesController.php @@ -14,7 +14,7 @@ namespace OCA\Maps\Controller; -use OCA\Maps\DB\FavoriteShareMapper; +use OCA\Maps\DB\FavoriteShareRepository; use OCA\Maps\Service\FavoritesService; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataResponse; @@ -51,7 +51,7 @@ public function __construct( protected \OCP\IGroupManager $groupManager, private readonly IL10N $l, private readonly FavoritesService $favoritesService, - private readonly FavoriteShareMapper $favoriteShareMapper, + private readonly FavoriteShareRepository $favoriteShareMapper, IEventDispatcher $eventDispatcher, ) { parent::__construct($appName, $request, $session, $urlGenerator, $eventDispatcher, $appConfig, $initialState, $shareManager, $userManager); diff --git a/lib/DB/DeviceShare.php b/lib/DB/DeviceShare.php index aebbc43b4..eb4498d64 100644 --- a/lib/DB/DeviceShare.php +++ b/lib/DB/DeviceShare.php @@ -26,22 +26,24 @@ use OCA\Maps\AppFramework\Db\Attribute\Column; use OCA\Maps\AppFramework\Db\Attribute\Entity; +use OCA\Maps\AppFramework\Db\Attribute\Id; use OCP\DB\Types; #[Entity(name: 'maps_device_shares')] class DeviceShare { - #[Column(name: 'id', type: Types::STRING)] + #[Id] + #[Column(name: 'id', type: Types::INTEGER)] public int $id; - #[Column(name: 'token', type: Types::STRING)] + #[Column(name: 'token', type: Types::STRING, nullable: false, length: 64)] public string $token; - #[Column(name: 'device_id', type: Types::INTEGER)] + #[Column(name: 'device_id', type: Types::INTEGER, nullable: false)] public int $deviceId; - #[Column(name: 'timestamp_from', type: Types::INTEGER)] + #[Column(name: 'timestamp_from', type: Types::INTEGER, nullable: false)] public int $timestampFrom; - #[Column(name: 'timestamp_to', type: Types::INTEGER)] + #[Column(name: 'timestamp_to', type: Types::INTEGER, nullable: false)] public int $timestampTo; } diff --git a/lib/DB/FavoriteShare.php b/lib/DB/FavoriteShare.php index a10bd179c..a1228dc83 100644 --- a/lib/DB/FavoriteShare.php +++ b/lib/DB/FavoriteShare.php @@ -24,28 +24,23 @@ namespace OCA\Maps\DB; -use OCP\AppFramework\Db\Entity; +use OCA\Maps\AppFramework\Db\Attribute\Column; +use OCA\Maps\AppFramework\Db\Attribute\Entity; +use OCA\Maps\AppFramework\Db\Attribute\Id; +use OCP\DB\Types; -/** - * @method string getToken() - * @method string getCategory() - * @method bool getAllowEdits() - * @method string getOwner() - * @method string setToken(string $token) - * @method string setCategory(string $category) - * @method string setAllowEdits(bool $allowEdits) - * @method string setOwner(string $owner) - */ -class FavoriteShare extends Entity { - public $owner; - public $token; - public $category; - public $allowEdits = false; +#[Entity(name: 'maps_favorite_shares')] +class FavoriteShare { + #[Id] + #[Column(name: 'id', type: Types::INTEGER)] + public int $id; + + #[Column(name: 'owner', type: Types::STRING, nullable: false, length: 64)] + public string $owner; + + #[Column(name: 'token', type: Types::STRING, nullable: false, length: 64)] + public string $token; - public function __construct() { - $this->addType('owner', 'string'); - $this->addType('token', 'string'); - $this->addType('category', 'string'); - $this->addType('allowEdits', 'boolean'); - } + #[Column(name: 'category', type: Types::STRING, nullable: false, length: 64)] + public string $category; } diff --git a/lib/DB/FavoriteShareMapper.php b/lib/DB/FavoriteShareRepository.php similarity index 51% rename from lib/DB/FavoriteShareMapper.php rename to lib/DB/FavoriteShareRepository.php index 017d128c1..644d73106 100644 --- a/lib/DB/FavoriteShareMapper.php +++ b/lib/DB/FavoriteShareRepository.php @@ -25,43 +25,34 @@ namespace OCA\Maps\DB; use OC\Share\Constants; +use OCA\Maps\AppFramework\Db\Repository; use OCP\AppFramework\Db\DoesNotExistException; -use OCP\AppFramework\Db\Entity; use OCP\AppFramework\Db\MultipleObjectsReturnedException; -use OCP\AppFramework\Db\QBMapper; -use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\Files\File; +use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\IDBConnection; use OCP\Security\ISecureRandom; -/** @template-extends QBMapper */ -class FavoriteShareMapper extends QBMapper { - /* @var ISecureRandom */ - private $secureRandom; - private $root; - - public function __construct(IDBConnection $db, ISecureRandom $secureRandom, IRootFolder $root) { - parent::__construct($db, 'maps_favorite_shares'); - - $this->secureRandom = $secureRandom; - $this->root = $root; +/** + * @template-extends Repository + */ +class FavoriteShareRepository extends Repository { + public function __construct( + IDBConnection $db, + private readonly ISecureRandom $secureRandom, + private readonly IRootFolder $root, + ) { + parent::__construct($db, FavoriteShare::class); } /** * @throws DoesNotExistException * @throws MultipleObjectsReturnedException */ - public function findByToken(string $token): ?FavoriteShare { - $qb = $this->db->getQueryBuilder(); - - $qb->select('*') - ->from($this->getTableName()) - ->where( - $qb->expr()->eq('token', $qb->createNamedParameter($token, IQueryBuilder::PARAM_STR)) - ); - - return $this->findEntity($qb); + public function findByToken(string $token): FavoriteShare { + return $this->findOneBy(['token' => $token]); } public function create(string $owner, string $category): FavoriteShare { @@ -71,62 +62,46 @@ public function create(string $owner, string $category): FavoriteShare { ); $newShare = new FavoriteShare(); - $newShare->setToken($token); - $newShare->setCategory($category); - $newShare->setOwner($owner); + $newShare->token = $token; + $newShare->category = $category; + $newShare->owner = $owner; return $this->insert($newShare); } /** - * @param $owner - * @return array|Entity[] + * @return \Generator */ - public function findAllByOwner($owner) { - $qb = $this->db->getQueryBuilder(); - - $qb->select('*') - ->from($this->getTableName()) - ->where( - $qb->expr()->eq('owner', $qb->createNamedParameter($owner, IQueryBuilder::PARAM_STR)) - ); - - return $this->findEntities($qb); + public function findAllByOwner(string $owner): \Generator { + return $this->findBy(['owner' => $owner]); } /** * @param $userId * @param $mapId - * @return array|mixed * @throws \OCP\Files\NotPermittedException * @throws \OC\User\NoUserException */ - public function findAllByMapId($userId, $mapId) { + public function findAllByMapId(string $userId, int $mapId): array { $userFolder = $this->root->getUserFolder($userId); - $folders = $userFolder->getById($mapId); - $shares = []; - if (empty($folders)) { - return $shares; - } - $folder = array_shift($folders); - if ($folder === null) { - return $shares; + $folder = $userFolder->getFirstNodeById($mapId); + if (!$folder instanceof Folder) { + return []; } return $this->findAllByFolder($folder); } /** - * @param $folder - * @param $isCreatable - * @return mixed * @throws NotFoundException */ - public function findAllByFolder($folder, $isCreatable = true) { + public function findAllByFolder(Folder $folder, bool $isCreatable = true): array { try { + /** @var File $file */ $file = $folder->get('.favorite_shares.json'); } catch (NotFoundException) { if ($isCreatable) { $file = $folder->newFile('.favorite_shares.json', $content = '[]'); + return []; } else { throw new NotFoundException(); } @@ -138,18 +113,11 @@ public function findAllByFolder($folder, $isCreatable = true) { * @throws DoesNotExistException * @throws MultipleObjectsReturnedException */ - public function findByOwnerAndCategory(string $owner, string $category): ?FavoriteShare { - $qb = $this->db->getQueryBuilder(); - - $qb->select('*') - ->from($this->getTableName()) - ->where( - $qb->expr()->eq('category', $qb->createNamedParameter($category, IQueryBuilder::PARAM_STR)) - )->andWhere( - $qb->expr()->eq('owner', $qb->createNamedParameter($owner, IQueryBuilder::PARAM_STR)) - ); - - return $this->findEntity($qb); + public function findByOwnerAndCategory(string $owner, string $category): FavoriteShare { + return $this->findOneBy([ + 'owner' => $owner, + 'category' => $category, + ]); } /** @@ -160,7 +128,7 @@ public function findByOwnerAndCategory(string $owner, string $category): ?Favori * @throws \OCP\Files\NotPermittedException * @throws \OC\User\NoUserException */ - public function findByMapIdAndCategory($userId, $mapId, $category) { + public function findByMapIdAndCategory(string $userId, int $mapId, string $category) { $shares = $this->findAllByMapId($userId, $mapId); foreach ($shares as $share) { if ($share->category === $category) { @@ -170,17 +138,13 @@ public function findByMapIdAndCategory($userId, $mapId, $category) { return null; } - public function removeByMapIdAndCategory($userId, $mapId, $category) { + public function removeByMapIdAndCategory(string $userId, int $mapId, string $category): ?array { $userFolder = $this->root->getUserFolder($userId); - $folders = $userFolder->getById($mapId); + $folder = $userFolder->getFirstNodeById($mapId); $shares = []; $deleted = null; - if (empty($folders)) { - return $deleted; - } - $folder = array_shift($folders); - if ($folder === null) { - return $deleted; + if (!$folder instanceof Folder) { + return null; } try { $file = $folder->get('.favorite_shares.json'); @@ -200,39 +164,18 @@ public function removeByMapIdAndCategory($userId, $mapId, $category) { return $deleted; } - /** - * @param $owner - * @param $category - * @return Entity|null - */ - public function findOrCreateByOwnerAndCategory($owner, $category) { - /* @var Entity */ - $entity = null; - + public function findOrCreateByOwnerAndCategory(string $owner, string $category): FavoriteShare { try { - $entity = $this->findByOwnerAndCategory($owner, $category); + return $this->findByOwnerAndCategory($owner, $category); } catch (DoesNotExistException) { - $entity = $this->create($owner, $category); - } catch (MultipleObjectsReturnedException) { + return $this->create($owner, $category); } - - return $entity; } - /** - * @param $owner - * @param $category - * @return bool - */ - public function removeByOwnerAndCategory($owner, $category) { - try { - $entity = $this->findByOwnerAndCategory($owner, $category); - } catch (DoesNotExistException|MultipleObjectsReturnedException) { - return false; - } - - $this->delete($entity); - - return true; + public function removeByOwnerAndCategory(string $owner, string $category): bool { + return $this->deleteBy([ + 'owner' => $owner, + 'category' => $category, + ]) > 0; } } diff --git a/lib/DB/Geophoto.php b/lib/DB/Geophoto.php index e2133738a..914b55511 100644 --- a/lib/DB/Geophoto.php +++ b/lib/DB/Geophoto.php @@ -12,20 +12,26 @@ namespace OCA\Maps\DB; -use OCP\AppFramework\Db\Entity; - -class Geophoto extends Entity { - - protected $fileId; - protected $lat; - protected $lng; - protected $dateTaken; - protected $userId; - - public function __construct() { - $this->addType('fileId', 'integer'); - $this->addType('lat', 'float'); - $this->addType('lng', 'float'); - $this->addType('dateTaken', 'integer'); - } +use OCA\Maps\AppFramework\Db\Attribute\Column; +use OCA\Maps\AppFramework\Db\Attribute\Entity; +use OCA\Maps\AppFramework\Db\Attribute\Id; +use OCP\DB\Types; + +#[Entity(name: 'maps_photo')] +class Geophoto { + #[Id] + #[Column(name: 'id', type: Types::INTEGER, nullable: false)] + public int $fileId; + + #[Column(name: 'lat', type: Types::FLOAT, nullable: true)] + public ?float $lat = null; + + #[Column(name: 'lng', type: Types::FLOAT, nullable: true)] + public ?float $lng = null; + + #[Column(name: 'date_taken', type: Types::DATETIME, nullable: true)] + public ?int $dateTaken = null; + + #[Column(name: 'user_id', type: Types::STRING, nullable: false)] + public string $userId; } diff --git a/lib/DB/GeophotoMapper.php b/lib/DB/GeophotoMapper.php deleted file mode 100644 index 372c4eef1..000000000 --- a/lib/DB/GeophotoMapper.php +++ /dev/null @@ -1,210 +0,0 @@ - - * @copyright Piotr Bator 2017 - */ - -namespace OCA\Maps\DB; - -use OCP\AppFramework\Db\QBMapper; -use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\IDBConnection; - -/** @template-extends QBMapper */ -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) { - $qb = $this->db->getQueryBuilder(); - - $qb->select('*') - ->from($this->getTableName()) - ->where( - $qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_STR)) - ); - - return $this->findEntity($qb); - } - - /** - * @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) { - $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) { - $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', 'DESC'); - if (!is_null($offset)) { - $qb->setFirstResult($offset); - } - if (!is_null($limit)) { - $qb->setMaxResults($limit); - } - return array_reverse($this->findEntities($qb)); - } - - /** - * @param $fileId - * @return int - * @throws \OCP\DB\Exception - */ - public function deleteByFileId($fileId) { - $qb = $this->db->getQueryBuilder(); - - $qb->delete($this->getTableName()) - ->where( - $qb->expr()->eq('file_id', $qb->createNamedParameter($fileId, IQueryBuilder::PARAM_STR)) - ); - - return $qb->executeStatement(); - } - - /** - * @param $fileId - * @param $userId - * @return int - * @throws \OCP\DB\Exception - */ - public function deleteByFileIdUserId($fileId, $userId) { - $qb = $this->db->getQueryBuilder(); - - $qb->delete($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(); - } - - /** - * @param $userId - * @return int - * @throws \OCP\DB\Exception - */ - public function deleteAll($userId) { - $qb = $this->db->getQueryBuilder(); - - $qb->delete($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) { - $qb = $this->db->getQueryBuilder(); - - $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/DB/GeophotoRepository.php b/lib/DB/GeophotoRepository.php new file mode 100644 index 000000000..fef11e75c --- /dev/null +++ b/lib/DB/GeophotoRepository.php @@ -0,0 +1,152 @@ + + * @copyright Piotr Bator 2017 + */ + +namespace OCA\Maps\DB; + +use OCA\Maps\AppFramework\Db\Repository; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; + +/** + * @template-extends Repository + */ +class GeophotoRepository extends Repository { + + public function __construct( + private readonly IDBConnection $db, + ) { + parent::__construct($db, Geophoto::class); + } + + /** + * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException + * @throws \OCP\DB\Exception + */ + public function find(int $id): ?Geophoto { + return $this->findOneBy(['id' => $id]); + } + + /** + * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException + * @throws \OCP\DB\Exception + */ + public function findByFileIdUserId(int $fileId, string $userId): Geophoto { + return $this->findOneBy([ + 'userId' => $userId, + 'fileId' => $fileId, + ]); + } + + /** + * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException + * @throws \OCP\DB\Exception + */ + public function findByFileId(int $fileId): ?Geophoto { + return $this->findOneBy([ + 'fileId' => $fileId, + ]); + } + + /** + * @return list + * @throws \OCP\DB\Exception + */ + public function findAll(string $userId, int $limit = null, int $offset = null): array { + $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); + } + + /** + * @return list + * @throws \OCP\DB\Exception + */ + public function findAllNonLocalized(string $userId, int $limit = null, int $offset = null): array { + $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', 'DESC'); + if (!is_null($offset)) { + $qb->setFirstResult($offset); + } + if (!is_null($limit)) { + $qb->setMaxResults($limit); + } + return array_reverse($this->findEntities($qb)); + } + + /** + * @throws \OCP\DB\Exception + */ + public function deleteByFileId(int $fileId): int { + return $this->deleteBy([ + 'fileId' => $fileId, + ]); + } + + /** + * @throws \OCP\DB\Exception + */ + public function deleteByFileIdUserId(int $fileId, string $userId): int { + return $this->deleteBy([ + 'userId' => $userId, + 'fileId' => $fileId, + ]); + } + + /** + * @throws \OCP\DB\Exception + */ + public function deleteAll(string $userId): int { + return $this->deleteBy([ + 'userId' => $userId, + ]); + } + + /** + * @throws \OCP\DB\Exception + */ + public function updateByFileId(int $fileId, ?float $lat, ?float $lng): int { + $qb = $this->db->getQueryBuilder(); + + $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/GeophotoService.php b/lib/Service/GeophotoService.php index 9b70663d6..bf32b854a 100644 --- a/lib/Service/GeophotoService.php +++ b/lib/Service/GeophotoService.php @@ -16,6 +16,7 @@ use OC\Files\Search\SearchComparison; use OC\Files\Search\SearchQuery; use OC\User\NoUserException; +use OCA\Maps\DB\GeophotoRepository; use OCP\DB\Exception; use OCP\Files\File; use OCP\Files\Folder; @@ -24,35 +25,27 @@ use OCP\Files\NotPermittedException; use OCP\Files\Search\ISearchBinaryOperator; use OCP\Files\Search\ISearchComparison; +use OCP\ICache; use OCP\ICacheFactory; -use OCP\IL10N; use OCP\IPreview; use RuntimeException; class GeophotoService { - private $root; - private $preview; private $timeorderedPointSets; - private $cacheFactory; - private readonly \OCP\ICache $photosCache; - private readonly \OCP\ICache $timeOrderedPointSetsCache; - private readonly \OCP\ICache $backgroundJobCache; + private readonly ICache $photosCache; + private readonly ICache $timeOrderedPointSetsCache; + private readonly ICache $backgroundJobCache; public function __construct( - IRootFolder $root, - IL10N $l10n, - private readonly \OCA\Maps\DB\GeophotoMapper $photoMapper, - IPreview $preview, - private readonly \OCA\Maps\Service\TracksService $tracksService, - private readonly \OCA\Maps\Service\DevicesService $devicesService, - ICacheFactory $cacheFactory, - $userId, + private readonly IRootFolder $root, + private readonly GeophotoRepository $photoMapper, + private readonly IPreview $preview, + private readonly TracksService $tracksService, + private readonly DevicesService $devicesService, + private readonly ICacheFactory $cacheFactory, ) { - $this->root = $root; - $this->preview = $preview; $this->timeorderedPointSets = null; - $this->cacheFactory = $cacheFactory; $this->photosCache = $this->cacheFactory->createDistributed('maps:photos'); $this->timeOrderedPointSetsCache = $this->cacheFactory->createDistributed('maps:time-ordered-point-sets'); $this->backgroundJobCache = $this->cacheFactory->createDistributed('maps:background-jobs'); @@ -99,7 +92,7 @@ public function getAll(string $userId, $folder = null, bool $respectNomediaAndNo // this path is relative to owner's storage //$path = $cacheEntry->getPath(); //but we want it relative to current user's storage - $files = $folder->getById($photoEntity->getFileId()); + $files = $folder->getById($photoEntity->fileId); if (empty($files)) { continue; } @@ -120,11 +113,11 @@ public function getAll(string $userId, $folder = null, bool $respectNomediaAndNo $isRoot = $file === $userFolder; $file_object = new \stdClass(); - $file_object->fileId = $photoEntity->getFileId(); + $file_object->fileId = $photoEntity->fileId; $file_object->fileid = $file_object->fileId; - $file_object->lat = $photoEntity->getLat(); - $file_object->lng = $photoEntity->getLng(); - $file_object->dateTaken = $photoEntity->getDateTaken() ?? \time(); + $file_object->lat = $photoEntity->lat; + $file_object->lng = $photoEntity->lng; + $file_object->dateTaken = $photoEntity->dateTaken ?? \time(); $file_object->basename = $isRoot ? '' : $file->getName(); $file_object->filename = $this->normalizePath($path); $file_object->etag = $file->getEtag(); @@ -181,7 +174,7 @@ public function getNonLocalized(string $userId, $folder = null, bool $respectNom // this path is relative to owner's storage //$path = $cacheEntry->getPath(); // but we want it relative to current user's storage - $files = $folder->getById($photoEntity->getFileId()); + $files = $folder->getById($photoEntity->fileId); if (empty($files)) { continue; } @@ -201,13 +194,13 @@ public function getNonLocalized(string $userId, $folder = null, bool $respectNom $isRoot = $file === $userFolder; //Unfortunately Exif stores the local and not the UTC time. There is no way to get the timezone, therefore it has to be given by the user. - $date = $photoEntity->getDateTaken() ?? \time(); + $date = $photoEntity->dateTaken ?? \time(); $dateWithTimezone = new \DateTime(gmdate('Y-m-d H:i:s', $date), $tz); $locations = $this->getLocationGuesses($dateWithTimezone->getTimestamp()); foreach ($locations as $key => $location) { $file_object = new \stdClass(); - $file_object->fileId = $photoEntity->getFileId(); + $file_object->fileId = $photoEntity->fileId; $file_object->fileid = $file_object->fileId; $file_object->path = $this->normalizePath($path); $file_object->mime = $file->getMimetype(); @@ -240,13 +233,11 @@ public function getNonLocalized(string $userId, $folder = null, bool $respectNom } /** - * @param $userId - * @param $folder * @throws \OCP\Files\NotFoundException * @throws \OCP\Files\NotPermittedException * @throws \OC\User\NoUserException */ - private function getIgnoredPaths(string $userId, $folder = null, bool $hideImagesOnCustomMaps = true): array { + private function getIgnoredPaths(string $userId, ?Folder $folder = null, bool $hideImagesOnCustomMaps = true): array { $ignoredPaths = []; $userFolder = $this->getFolderForUser($userId); if (is_null($folder)) { @@ -277,9 +268,7 @@ private function getIgnoredPaths(string $userId, $folder = null, bool $hideImage } /** - * returns a array of locations for a given date - * - * @param $dateTaken int + * Returns an array of locations for a given date */ private function getLocationGuesses(int $dateTaken): array { $locations = []; @@ -414,12 +403,7 @@ private function normalizePath($path): string|array { return str_replace('files', '', $path); } - /** - * @param string $userId the user id - * @return Folder - */ - private function getFolderForUser(string $userId) { + private function getFolderForUser(string $userId): Folder { return $this->root->getUserFolder($userId); } - } diff --git a/lib/Service/PhotofilesService.php b/lib/Service/PhotofilesService.php index 807ecf6b3..c70ad83c1 100644 --- a/lib/Service/PhotofilesService.php +++ b/lib/Service/PhotofilesService.php @@ -15,7 +15,7 @@ use OCA\Maps\BackgroundJob\AddPhotoJob; use OCA\Maps\BackgroundJob\UpdatePhotoByFileJob; use OCA\Maps\DB\Geophoto; -use OCA\Maps\DB\GeophotoMapper; +use OCA\Maps\DB\GeophotoRepository; use OCA\Maps\Helper\ExifDataInvalidException; use OCA\Maps\Helper\ExifDataNoLocationException; use OCA\Maps\Helper\ExifGeoData; @@ -47,12 +47,12 @@ class PhotofilesService { public const PHOTO_MIME_TYPES = ['image/jpeg', 'image/tiff', 'image/heic']; public function __construct( - private readonly LoggerInterface $logger, - private readonly ICacheFactory $cacheFactory, - private readonly IRootFolder $root, - private readonly GeophotoMapper $photoMapper, - private readonly IManager $shareManager, - private readonly IJobList $jobList, + private readonly LoggerInterface $logger, + private readonly ICacheFactory $cacheFactory, + private readonly IRootFolder $root, + private readonly GeophotoRepository $photoMapper, + private readonly IManager $shareManager, + private readonly IJobList $jobList, ) { $this->photosCache = $this->cacheFactory->createDistributed('maps:photos'); $this->backgroundJobCache = $this->cacheFactory->createDistributed('maps:background-jobs'); @@ -223,8 +223,8 @@ private function setDirectoriesCoords(string $userId, $paths, $lats, $lngs): arr 'path' => preg_replace('/^files/', '', (string)$node->getInternalPath()), 'lat' => $lat, 'lng' => $lng, - 'oldLat' => $photo ? $photo->getLat() : null, - 'oldLng' => $photo ? $photo->getLng() : null, + 'oldLat' => $photo ? $photo->lat : null, + 'oldLng' => $photo ? $photo->lng : null, ]; $this->setExifCoords($node, $lat, $lng); $this->updateByFileNow($node); @@ -259,8 +259,8 @@ private function setFilesCoords(string $userId, $paths, array $lats, array $lngs 'path' => preg_replace('/^files/', '', (string)$file->getInternalPath()), 'lat' => $lat, 'lng' => $lng, - 'oldLat' => $photo ? $photo->getLat() : null, - 'oldLng' => $photo ? $photo->getLng() : null, + 'oldLat' => $photo ? $photo->lat : null, + 'oldLng' => $photo ? $photo->lng : null, ]; $this->setExifCoords($file, $lat, $lng); $this->updateByFileNow($file); @@ -287,8 +287,8 @@ public function resetPhotosFilesCoords(string $userId, array $paths): array { 'path' => preg_replace('/^files/', '', (string)$file->getInternalPath()), 'lat' => null, 'lng' => null, - 'oldLat' => $photo ? $photo->getLat() : null, - 'oldLng' => $photo ? $photo->getLng() : null, + 'oldLat' => $photo ? $photo->lat : null, + 'oldLng' => $photo ? $photo->lng : null, ]; $this->resetExifCoords($file); $this->photoMapper->updateByFileId($file->getId(), null, null); @@ -323,16 +323,12 @@ public function addPhotoNow(File $photo, string $userId): void { private function insertPhoto(File $photo, string $userId, ExifGeoData $exif): void { $photoEntity = new Geophoto(); - $photoEntity->setFileId($photo->getId()); - $photoEntity->setLat( - is_numeric($exif->lat) && !is_nan($exif->lat) ? $exif->lat : null - ); - $photoEntity->setLng( - is_numeric($exif->lng) && !is_nan($exif->lng) ? $exif->lng : null - ); - $photoEntity->setUserId($userId); + $photoEntity->fileId = $photo->getId(); + $photoEntity->lat = is_numeric($exif->lat) && !is_nan($exif->lat) ? $exif->lat : null; + $photoEntity->lng = is_numeric($exif->lng) && !is_nan($exif->lng) ? $exif->lng : null; + $photoEntity->userId = $userId; // alternative should be file creation date - $photoEntity->setDateTaken($exif->dateTaken ?? $photo->getMTime()); + $photoEntity->dateTaken = $exif->dateTaken ?? $photo->getMTime(); $this->photoMapper->insert($photoEntity); $this->photosCache->clear($userId); diff --git a/tests/Integration/Db/FavoriteShareMapperTest.php b/tests/Integration/Db/FavoriteShareMapperTest.php index f339f1629..f364fa5b3 100644 --- a/tests/Integration/Db/FavoriteShareMapperTest.php +++ b/tests/Integration/Db/FavoriteShareMapperTest.php @@ -27,7 +27,7 @@ use ChristophWurst\Nextcloud\Testing\DatabaseTransaction; use ChristophWurst\Nextcloud\Testing\TestCase; use OCA\Maps\DB\FavoriteShare; -use OCA\Maps\DB\FavoriteShareMapper; +use OCA\Maps\DB\FavoriteShareRepository; use OCP\AppFramework\Db\DoesNotExistException; use OCP\Files\IRootFolder; use OCP\Security\ISecureRandom; @@ -36,12 +36,12 @@ class FavoriteShareMapperTest extends TestCase { use DatabaseTransaction; - private FavoriteShareMapper $mapper; + private FavoriteShareRepository $mapper; public function setUp(): void { parent::setUp(); - $this->mapper = new FavoriteShareMapper( + $this->mapper = new FavoriteShareRepository( Server::get(\OCP\IDBConnection::class), Server::get(ISecureRandom::class), Server::get(IRootFolder::class), diff --git a/tests/Unit/Controller/FavoritesControllerTest.php b/tests/Unit/Controller/FavoritesControllerTest.php index e65b697da..ab916ba3e 100644 --- a/tests/Unit/Controller/FavoritesControllerTest.php +++ b/tests/Unit/Controller/FavoritesControllerTest.php @@ -15,7 +15,7 @@ namespace OCA\Maps\Controller; use OCA\Maps\AppInfo\Application; -use OCA\Maps\DB\FavoriteShareMapper; +use OCA\Maps\DB\FavoriteShareRepository; use OCA\Maps\Service\FavoritesService; use OCP\AppFramework\Http; use OCP\AppFramework\Services\IAppConfig; @@ -87,7 +87,7 @@ protected function setUp(): void { $c->get(IRootFolder::class), $c->get(FavoritesService::class), $c->get(\OCP\IDateTimeZone::class), - $c->get(FavoriteShareMapper::class), + $c->get(FavoriteShareRepository::class), 'test' ); @@ -99,7 +99,7 @@ protected function setUp(): void { $c->get(IRootFolder::class), $c->get(FavoritesService::class), $c->get(\OCP\IDateTimeZone::class), - $c->get(FavoriteShareMapper::class), + $c->get(FavoriteShareRepository::class), 'test2' ); $this->mapFolder = $this->createMapFolder(); diff --git a/tests/Unit/Controller/PhotosControllerTest.php b/tests/Unit/Controller/PhotosControllerTest.php index 4ac002cf0..018537a7e 100644 --- a/tests/Unit/Controller/PhotosControllerTest.php +++ b/tests/Unit/Controller/PhotosControllerTest.php @@ -13,7 +13,7 @@ namespace OCA\Maps\Controller; use OCA\Maps\AppInfo\Application; -use OCA\Maps\DB\GeophotoMapper; +use OCA\Maps\DB\GeophotoRepository; use OCA\Maps\Service\GeophotoService; use OCA\Maps\Service\PhotofilesService; use OCP\DB\QueryBuilder\IQueryBuilder; @@ -84,7 +84,7 @@ protected function setUp(): void { $c->get(LoggerInterface::class), $c->get(ICacheFactory::class), $this->rootFolder, - $c->get(GeophotoMapper::class), + $c->get(GeophotoRepository::class), $c->get(IManager::class), $c->get(\OCP\BackgroundJob\IJobList::class) ); diff --git a/tests/Unit/Controller/PublicFavoritePageControllerTest.php b/tests/Unit/Controller/PublicFavoritePageControllerTest.php index 5b024996b..190856f3a 100644 --- a/tests/Unit/Controller/PublicFavoritePageControllerTest.php +++ b/tests/Unit/Controller/PublicFavoritePageControllerTest.php @@ -25,7 +25,7 @@ namespace OCA\Maps\Controller; use OCA\Maps\AppInfo\Application; -use OCA\Maps\DB\FavoriteShareMapper; +use OCA\Maps\DB\FavoriteShareRepository; use OCA\Maps\Service\FavoritesService; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; @@ -44,7 +44,7 @@ class PublicFavoritePageControllerTest extends TestCase { private Application $app; private ContainerInterface $container; private FavoritesService $favoritesService; - private FavoriteShareMapper $favoriteShareMapper; + private FavoriteShareRepository $favoriteShareMapper; protected function setUp(): void { // Begin transaction @@ -64,7 +64,7 @@ protected function setUp(): void { $container->get(\OCP\IDBConnection::class) ); - $this->favoriteShareMapper = new FavoriteShareMapper( + $this->favoriteShareMapper = new FavoriteShareRepository( $container->get(\OCP\IDBConnection::class), $container->get(ISecureRandom::class), $container->get(IRootFolder::class) @@ -98,7 +98,7 @@ public function testSharedFavoritesCategory(): void { ->addFavoriteToDB($testUserName, 'Test', 0, 0, $categoryName, '', null); $share = $this->favoriteShareMapper->create($testUserName, $categoryName); - $result = $this->publicPageController->sharedFavoritesCategory($share->getToken()); + $result = $this->publicPageController->sharedFavoritesCategory($share->token); // Assertions $this->assertTrue($result instanceof TemplateResponse); diff --git a/tests/Unit/Controller/PublicFavoritesApiControllerTest.php b/tests/Unit/Controller/PublicFavoritesApiControllerTest.php index 560e486f9..58afc55e0 100644 --- a/tests/Unit/Controller/PublicFavoritesApiControllerTest.php +++ b/tests/Unit/Controller/PublicFavoritesApiControllerTest.php @@ -26,7 +26,7 @@ use OCA\Maps\AppInfo\Application; use OCA\Maps\DB\FavoriteShare; -use OCA\Maps\DB\FavoriteShareMapper; +use OCA\Maps\DB\FavoriteShareRepository; use OCA\Maps\Service\FavoritesService; use OCP\AppFramework\Http; use OCP\Files\IRootFolder; @@ -38,7 +38,7 @@ class PublicFavoritesApiControllerTest extends TestCase { private PublicFavoritesApiController $publicFavoritesApiController; private FavoritesService $favoritesService; - private FavoriteShareMapper $favoriteShareMapper; + private FavoriteShareRepository $favoriteShareMapper; protected function setUp(): void { // Begin transaction @@ -58,7 +58,7 @@ protected function setUp(): void { $container->get(\OCP\IDBConnection::class) ); - $this->favoriteShareMapper = new FavoriteShareMapper( + $this->favoriteShareMapper = new FavoriteShareRepository( $container->get(\OCP\IDBConnection::class), $container->get(ISecureRandom::class), $container->get(IserverContainer::class)->get(IRootFolder::class) @@ -90,21 +90,22 @@ public function testGetFavorites(): void { $share = $this->favoriteShareMapper->create($testUser, $categoryName); // Mock token sent by request - $this->publicFavoritesApiController->setToken($share->getToken()); + $this->publicFavoritesApiController->setToken($share->token); $response = $this->publicFavoritesApiController->getFavorites(); $this->assertEquals(Http::STATUS_OK, $response->getStatus()); + /** @var array{share: FavoriteShare, favorites: list} $data */ $data = $response->getData(); $this->assertIsArray($data); $this->assertArrayHasKey('share', $data); $this->assertArrayHasKey('favorites', $data); - $this->assertEquals($testUser, $data['share']->getOwner()); - $this->assertEquals($categoryName, $data['share']->getCategory()); - $this->assertEquals($share->getToken(), $data['share']->getToken()); + $this->assertEquals($testUser, $data['share']->owner); + $this->assertEquals($categoryName, $data['share']->category); + $this->assertEquals($share->token, $data['share']->token); $this->assertEquals(1, count($data['favorites'])); From c1cd3cdec40773c3035544b28408e0a1b58fc38b Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 11 Jun 2026 20:02:24 +0200 Subject: [PATCH 3/5] fix(tests): Fix tests broken by new entity api Signed-off-by: Carl Schwan --- .gitignore | 2 + lib/AppFramework/Db/Attribute/Column.php | 2 +- lib/AppFramework/Db/Attribute/Entity.php | 2 +- lib/AppFramework/Db/Repository.php | 49 ++-- lib/Controller/FavoritesController.php | 9 +- .../PublicFavoritesApiController.php | 18 +- lib/DB/DeviceShare.php | 4 +- lib/DB/FavoriteShare.php | 4 +- lib/DB/FavoriteShareRepository.php | 3 - lib/DB/Geophoto.php | 9 +- lib/DB/GeophotoRepository.php | 4 +- lib/Service/GeophotoService.php | 14 +- lib/Service/PhotofilesService.php | 15 +- .../Controller/FavoritesControllerTest.php | 6 +- tests/Unit/DB/DeviceShareRepositoryTest.php | 154 ++++++++++++ tests/Unit/DB/FavoriteShareRepositoryTest.php | 137 +++++++++++ tests/Unit/DB/GeophotoRepositoryTest.php | 224 ++++++++++++++++++ 17 files changed, 589 insertions(+), 67 deletions(-) create mode 100644 tests/Unit/DB/DeviceShareRepositoryTest.php create mode 100644 tests/Unit/DB/FavoriteShareRepositoryTest.php create mode 100644 tests/Unit/DB/GeophotoRepositoryTest.php diff --git a/.gitignore b/.gitignore index 889bbbcfb..f7de733d2 100644 --- a/.gitignore +++ b/.gitignore @@ -80,3 +80,5 @@ typings/ # VScode settings .vscode/settings.json + +tests/.phpunit.cache/test-results diff --git a/lib/AppFramework/Db/Attribute/Column.php b/lib/AppFramework/Db/Attribute/Column.php index ee5909273..d7beefd2a 100644 --- a/lib/AppFramework/Db/Attribute/Column.php +++ b/lib/AppFramework/Db/Attribute/Column.php @@ -28,7 +28,7 @@ * @since 35.0.0 */ #[Attribute(Attribute::TARGET_PROPERTY)] -#[Consumable(since: '33.0.0')] +#[Consumable(since: '35.0.0')] final readonly class Column { public function __construct( public string $name, diff --git a/lib/AppFramework/Db/Attribute/Entity.php b/lib/AppFramework/Db/Attribute/Entity.php index e4dd44099..abc0fde6d 100644 --- a/lib/AppFramework/Db/Attribute/Entity.php +++ b/lib/AppFramework/Db/Attribute/Entity.php @@ -27,7 +27,7 @@ * @since 35.0.0 */ #[Attribute(Attribute::TARGET_CLASS)] -#[Consumable(since: '33.0.0')] +#[Consumable(since: '35.0.0')] final readonly class Entity { public function __construct( public string $name, diff --git a/lib/AppFramework/Db/Repository.php b/lib/AppFramework/Db/Repository.php index 3ccfd4a87..07a00fdb5 100644 --- a/lib/AppFramework/Db/Repository.php +++ b/lib/AppFramework/Db/Repository.php @@ -28,7 +28,7 @@ * @since 35.0.0 */ class Repository { - private string $tableName; + private readonly string $tableName; /** @var array */ private array $_mappingColumnToTypes = []; @@ -40,12 +40,11 @@ class Repository { private array $_mappingPropertyToColumn = []; /** @var \ReflectionClass */ - private \ReflectionClass $reflection; + private readonly \ReflectionClass $reflection; - private string $idProperty; + private \ReflectionProperty $idProperty; /** - * @param IDBConnection $connection * @param class-string $entityClass * @throws \ReflectionException */ @@ -76,8 +75,8 @@ public function __construct( /** @var list<\ReflectionAttribute> $ids */ $ids = $property->getAttributes(Id::class, \ReflectionAttribute::IS_INSTANCEOF); - if (!empty($ids)) { - $this->idProperty = $property->getName(); + if ($ids !== []) { + $this->idProperty = $property; } } } @@ -85,7 +84,6 @@ public function __construct( /** * Runs a sql query and yields each resulting entity to obtain database entries in a memory-efficient way * - * @param IQueryBuilder $query * @return Generator Generator of fetched entities * @psalm-return Generator Generator of fetched entities * @throws Exception @@ -104,7 +102,6 @@ public function yieldEntities(IQueryBuilder $query): Generator { /** * Runs a sql query and returns an array of entities * - * @param IQueryBuilder $query * @psalm-return list all fetched entities * @throws Exception */ @@ -133,9 +130,14 @@ private function mapRowToEntity(mixed $row): object { $type = Types::STRING; } - if ($column === $this->idProperty) { - $entity->$property = (string)$value; - continue; + if ($column === $this->idProperty->getName()) { + /** @var list<\ReflectionAttribute> $ids */ + $ids = $this->idProperty->getAttributes(Id::class, \ReflectionAttribute::IS_INSTANCEOF); + $id = array_shift($ids); + if ($id->newInstance()->generatorClass !== null) { + $entity->$property = (string)$value; + continue; + } } switch ($type) { @@ -166,7 +168,7 @@ private function mapRowToEntity(mixed $row): object { break; case Types::JSON: if (!is_array($value)) { - $value = json_decode($value, true); + $value = json_decode((string)$value, true); } break; } @@ -203,13 +205,15 @@ public function insert(object $entity): object { if (count($ids) > 0 && $property->getValue($entity) === null) { $primaryProperty = $property; $generatorClass = $ids[0]->newInstance()->generatorClass; - $generator = Server::get($generatorClass); - /** @psalm-suppress UndefinedClass NC 33 and above */ - if (class_exists(ISnowflakeGenerator::class) && $generator instanceof ISnowflakeGenerator) { - $isSnowflake = true; - /** @psalm-suppress UndefinedClass */ - $values[$column->name] = $generator->nextId(); - $property->setValue($entity, $insert->createNamedParameter($values[$column->name])); + if ($generatorClass) { + $generator = Server::get($generatorClass); + /** @psalm-suppress UndefinedClass NC 33 and above */ + if (class_exists(ISnowflakeGenerator::class) && $generator instanceof ISnowflakeGenerator) { + $isSnowflake = true; + /** @psalm-suppress UndefinedClass */ + $values[$column->name] = $generator->nextId(); + $property->setValue($entity, $insert->createNamedParameter($values[$column->name])); + } } } else { $type = $this->getParameterType($column->type, false); @@ -249,7 +253,7 @@ public function update(object $entity): object { throw new \LogicException('Trying to update an entity with no primary key set.'); } - $update->andWhere($update->expr()->eq($this->_mappingPropertyToColumn[$this->idProperty], $update->createNamedParameter($property->getValue($entity)))); + $update->andWhere($update->expr()->eq($this->_mappingPropertyToColumn[$this->idProperty->getName()], $update->createNamedParameter($property->getValue($entity)))); // don't update the id continue; }; @@ -276,7 +280,7 @@ public function delete(object $entity): void { $column = $columns[0]->newInstance(); if (count($property->getAttributes(Id::class, \ReflectionAttribute::IS_INSTANCEOF)) !== 0) { - $delete->andWhere($delete->expr()->eq($column->name, $property->getValue($entity))); + $delete->andWhere($delete->expr()->eq($column->name, $delete->createNamedParameter($property->getValue($entity)))); $foundId = true; }; } @@ -327,7 +331,6 @@ private function getParameterType(string $type, bool $isArray): string|int { * @param array> $criteria * @param array|null $orderBy * @return \Generator - * @since 33.0.0 */ public function findBy(array $criteria, array $orderBy = [], ?int $limit = null, ?int $offset = null): \Generator { $qb = $this->getSelectQueryBuilder($criteria, $orderBy); @@ -419,12 +422,10 @@ private function getSelectQueryBuilder(array $criteria, array $orderBy = []): IQ * Returns a db result and throws exceptions when there are more or less * results * - * @param IQueryBuilder $query * @psalm-return T the entity * @throws Exception * @throws MultipleObjectsReturnedException if more than one item exist * @throws DoesNotExistException if the item does not exist - * @since 33.0.0 */ protected function findEntity(IQueryBuilder $query): object { $result = $query->executeQuery(); diff --git a/lib/Controller/FavoritesController.php b/lib/Controller/FavoritesController.php index 4a8474064..50d4848d2 100644 --- a/lib/Controller/FavoritesController.php +++ b/lib/Controller/FavoritesController.php @@ -14,6 +14,7 @@ namespace OCA\Maps\Controller; +use OCA\Maps\DB\FavoriteShare; use OCA\Maps\DB\FavoriteShareRepository; use OCA\Maps\Service\FavoritesService; use OCP\AppFramework\Controller; @@ -169,8 +170,7 @@ public function editFavorite(int $id, ?string $name, float $lat, float $lng, ?st if (is_null($myMapId) || $myMapId === 0) { $favorite = $this->favoritesService->getFavoriteFromDB($id, $this->userId); if ($favorite !== null) { - if (is_numeric($lng) - ) { + if (is_numeric($lng)) { $this->favoritesService->editFavoriteInDB($id, $name, $lat, $lng, $category, $comment, $extensions); $editedFavorite = $this->favoritesService->getFavoriteFromDB($id); return new DataResponse($editedFavorite); @@ -279,9 +279,12 @@ public function getSharedCategories(?int $myMapId = null): DataResponse { $categories = $this->favoriteShareMapper->findAllByMapId($this->userId, $myMapId); } - return new DataResponse($categories); + return new DataResponse(iterator_to_array($categories)); } + /** + * @return DataResponse|DataResponse + */ #[NoAdminRequired] public function shareCategory(string $category): DataResponse { if ($this->favoritesService->countFavorites($this->userId, [$category], null, null) === 0) { diff --git a/lib/Controller/PublicFavoritesApiController.php b/lib/Controller/PublicFavoritesApiController.php index 993246c29..614166200 100644 --- a/lib/Controller/PublicFavoritesApiController.php +++ b/lib/Controller/PublicFavoritesApiController.php @@ -65,15 +65,15 @@ public function isValidToken(): bool { } /** - public function canEdit(): bool { - try { - $share = $this->favoriteShareMapper->findByToken($this->getToken()); - } catch (DoesNotExistException|MultipleObjectsReturnedException) { - return false; - } - - return $share->getAllowEdits(); - }*/ + * public function canEdit(): bool { + * try { + * $share = $this->favoriteShareMapper->findByToken($this->getToken()); + * } catch (DoesNotExistException|MultipleObjectsReturnedException) { + * return false; + * } + * + * return $share->getAllowEdits(); + * }*/ /** * @return DataResponse}, array{}>|DataResponse diff --git a/lib/DB/DeviceShare.php b/lib/DB/DeviceShare.php index eb4498d64..e8dde0865 100644 --- a/lib/DB/DeviceShare.php +++ b/lib/DB/DeviceShare.php @@ -32,8 +32,8 @@ #[Entity(name: 'maps_device_shares')] class DeviceShare { #[Id] - #[Column(name: 'id', type: Types::INTEGER)] - public int $id; + #[Column(name: 'id', type: Types::BIGINT)] + public ?int $id = null; #[Column(name: 'token', type: Types::STRING, nullable: false, length: 64)] public string $token; diff --git a/lib/DB/FavoriteShare.php b/lib/DB/FavoriteShare.php index a1228dc83..368b0f544 100644 --- a/lib/DB/FavoriteShare.php +++ b/lib/DB/FavoriteShare.php @@ -32,8 +32,8 @@ #[Entity(name: 'maps_favorite_shares')] class FavoriteShare { #[Id] - #[Column(name: 'id', type: Types::INTEGER)] - public int $id; + #[Column(name: 'id', type: Types::BIGINT)] + public ?int $id = null; #[Column(name: 'owner', type: Types::STRING, nullable: false, length: 64)] public string $owner; diff --git a/lib/DB/FavoriteShareRepository.php b/lib/DB/FavoriteShareRepository.php index 644d73106..c57719b77 100644 --- a/lib/DB/FavoriteShareRepository.php +++ b/lib/DB/FavoriteShareRepository.php @@ -121,9 +121,6 @@ public function findByOwnerAndCategory(string $owner, string $category): Favorit } /** - * @param $userId - * @param $mapId - * @param $category * @return mixed|null * @throws \OCP\Files\NotPermittedException * @throws \OC\User\NoUserException diff --git a/lib/DB/Geophoto.php b/lib/DB/Geophoto.php index 914b55511..ba816c0b8 100644 --- a/lib/DB/Geophoto.php +++ b/lib/DB/Geophoto.php @@ -17,10 +17,13 @@ use OCA\Maps\AppFramework\Db\Attribute\Id; use OCP\DB\Types; -#[Entity(name: 'maps_photo')] +#[Entity(name: 'maps_photos')] class Geophoto { #[Id] - #[Column(name: 'id', type: Types::INTEGER, nullable: false)] + #[Column(name: 'id', type: Types::BIGINT, nullable: false)] + public ?int $id = null; + + #[Column(name: 'file_id', type: Types::INTEGER, nullable: false)] public int $fileId; #[Column(name: 'lat', type: Types::FLOAT, nullable: true)] @@ -30,7 +33,7 @@ class Geophoto { public ?float $lng = null; #[Column(name: 'date_taken', type: Types::DATETIME, nullable: true)] - public ?int $dateTaken = null; + public ?\DateTime $dateTaken = null; #[Column(name: 'user_id', type: Types::STRING, nullable: false)] public string $userId; diff --git a/lib/DB/GeophotoRepository.php b/lib/DB/GeophotoRepository.php index fef11e75c..b77f08a67 100644 --- a/lib/DB/GeophotoRepository.php +++ b/lib/DB/GeophotoRepository.php @@ -60,7 +60,7 @@ public function findByFileId(int $fileId): ?Geophoto { * @return list * @throws \OCP\DB\Exception */ - public function findAll(string $userId, int $limit = null, int $offset = null): array { + public function findAll(string $userId, ?int $limit = null, ?int $offset = null): array { $qb = $this->db->getQueryBuilder(); $qb->select('*') @@ -85,7 +85,7 @@ public function findAll(string $userId, int $limit = null, int $offset = null): * @return list * @throws \OCP\DB\Exception */ - public function findAllNonLocalized(string $userId, int $limit = null, int $offset = null): array { + public function findAllNonLocalized(string $userId, ?int $limit = null, ?int $offset = null): array { $qb = $this->db->getQueryBuilder(); $qb->select('*') diff --git a/lib/Service/GeophotoService.php b/lib/Service/GeophotoService.php index bf32b854a..9f6799efb 100644 --- a/lib/Service/GeophotoService.php +++ b/lib/Service/GeophotoService.php @@ -38,12 +38,12 @@ class GeophotoService { private readonly ICache $backgroundJobCache; public function __construct( - private readonly IRootFolder $root, + private readonly IRootFolder $root, private readonly GeophotoRepository $photoMapper, - private readonly IPreview $preview, - private readonly TracksService $tracksService, - private readonly DevicesService $devicesService, - private readonly ICacheFactory $cacheFactory, + private readonly IPreview $preview, + private readonly TracksService $tracksService, + private readonly DevicesService $devicesService, + private readonly ICacheFactory $cacheFactory, ) { $this->timeorderedPointSets = null; $this->photosCache = $this->cacheFactory->createDistributed('maps:photos'); @@ -194,9 +194,9 @@ public function getNonLocalized(string $userId, $folder = null, bool $respectNom $isRoot = $file === $userFolder; //Unfortunately Exif stores the local and not the UTC time. There is no way to get the timezone, therefore it has to be given by the user. - $date = $photoEntity->dateTaken ?? \time(); + $date = $photoEntity->dateTaken ?? new \DateTime('now'); - $dateWithTimezone = new \DateTime(gmdate('Y-m-d H:i:s', $date), $tz); + $dateWithTimezone = new \DateTime($date->format('Y-m-d H:i:s'), $tz); $locations = $this->getLocationGuesses($dateWithTimezone->getTimestamp()); foreach ($locations as $key => $location) { $file_object = new \stdClass(); diff --git a/lib/Service/PhotofilesService.php b/lib/Service/PhotofilesService.php index c70ad83c1..62bac8601 100644 --- a/lib/Service/PhotofilesService.php +++ b/lib/Service/PhotofilesService.php @@ -47,12 +47,12 @@ class PhotofilesService { public const PHOTO_MIME_TYPES = ['image/jpeg', 'image/tiff', 'image/heic']; public function __construct( - private readonly LoggerInterface $logger, - private readonly ICacheFactory $cacheFactory, - private readonly IRootFolder $root, + private readonly LoggerInterface $logger, + private readonly ICacheFactory $cacheFactory, + private readonly IRootFolder $root, private readonly GeophotoRepository $photoMapper, - private readonly IManager $shareManager, - private readonly IJobList $jobList, + private readonly IManager $shareManager, + private readonly IJobList $jobList, ) { $this->photosCache = $this->cacheFactory->createDistributed('maps:photos'); $this->backgroundJobCache = $this->cacheFactory->createDistributed('maps:background-jobs'); @@ -134,7 +134,7 @@ public function deleteByFile(File $file): void { // delete photo only if it's not accessible to user anymore // it might have been shared multiple times by different users - public function deleteByFileIdUserId($fileId, $userId): void { + public function deleteByFileIdUserId(int $fileId, string $userId): void { $userFolder = $this->root->getUserFolder($userId); $files = $userFolder->getById($fileId); if (!is_array($files) or count($files) === 0) { @@ -328,7 +328,8 @@ private function insertPhoto(File $photo, string $userId, ExifGeoData $exif): vo $photoEntity->lng = is_numeric($exif->lng) && !is_nan($exif->lng) ? $exif->lng : null; $photoEntity->userId = $userId; // alternative should be file creation date - $photoEntity->dateTaken = $exif->dateTaken ?? $photo->getMTime(); + $photoEntity->dateTaken = new \DateTime(); + $photoEntity->dateTaken->setTimestamp($exif->dateTaken ?? $photo->getMTime()); $this->photoMapper->insert($photoEntity); $this->photosCache->clear($userId); diff --git a/tests/Unit/Controller/FavoritesControllerTest.php b/tests/Unit/Controller/FavoritesControllerTest.php index ab916ba3e..c69ee3450 100644 --- a/tests/Unit/Controller/FavoritesControllerTest.php +++ b/tests/Unit/Controller/FavoritesControllerTest.php @@ -482,7 +482,7 @@ public function testShareUnShareCategory(): void { $this->assertEquals(Http::STATUS_OK, $response1->getStatus()); $this->assertEquals(Http::STATUS_OK, $response2->getStatus()); - $this->assertIsString($response1->getData()->getToken()); + $this->assertIsString($response1->getData()->token); $this->assertTrue($response2->getData()['did_exist']); } @@ -532,7 +532,7 @@ public function testGetSharedCategories(): void { $this->assertIsArray($categories->getData()); - $mappedCategories = array_map(fn ($el) => $el->getCategory(), $categories->getData()); + $mappedCategories = array_map(fn ($el) => $el->category, $categories->getData()); foreach ($categoryNames as $categoryName) { $this->assertContains($categoryName, $mappedCategories); @@ -561,7 +561,7 @@ public function testFavoriteShareIsRenamedCorrectly(): void { $shares = $this->favoritesController->getSharedCategories()->getData(); - $shareNames = array_map(fn ($el) => $el->getCategory(), $shares); + $shareNames = array_map(fn ($el) => $el->category, $shares); $this->favoritesController->deleteFavorite($id); $this->favoritesController->unShareCategory($newCategoryName); diff --git a/tests/Unit/DB/DeviceShareRepositoryTest.php b/tests/Unit/DB/DeviceShareRepositoryTest.php new file mode 100644 index 000000000..3b3971e10 --- /dev/null +++ b/tests/Unit/DB/DeviceShareRepositoryTest.php @@ -0,0 +1,154 @@ + +// SPDX-License-Identifier: AGPL-3.0-or-later + +namespace Unit\DB; + +use OCA\Maps\DB\DeviceShare; +use OCA\Maps\DB\DeviceShareRepository; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\Server; +use PHPUnit\Framework\TestCase; + +class DeviceShareRepositoryTest extends TestCase { + private DeviceShareRepository $repository; + /** @var list */ + private array $created = []; + + public function setUp(): void { + parent::setUp(); + $this->repository = Server::get(DeviceShareRepository::class); + } + + public function tearDown(): void { + foreach ($this->created as $share) { + try { + $this->repository->delete($share); + } catch (\Throwable) { + // already removed during the test + } + } + $this->created = []; + parent::tearDown(); + } + + private function createShare(int $deviceId = 1, int $from = 100, int $to = 200): DeviceShare { + $share = $this->repository->create($deviceId, $from, $to); + $this->created[] = $share; + return $share; + } + + public function testCreate(): void { + $share = $this->createShare(42, 100, 200); + + $this->assertNotNull($share->id); + $this->assertNotEmpty($share->token); + $this->assertEquals(42, $share->deviceId); + $this->assertEquals(100, $share->timestampFrom); + $this->assertEquals(200, $share->timestampTo); + } + + public function testFindByToken(): void { + $deviceShare = $this->createShare(1, 1, 2); + + $deviceShareDatabase = $this->repository->findByToken($deviceShare->token); + $this->assertEquals($deviceShare->id, $deviceShareDatabase->id); + $this->assertEquals($deviceShare->token, $deviceShareDatabase->token); + $this->assertEquals($deviceShare->timestampFrom, $deviceShareDatabase->timestampFrom); + $this->assertEquals($deviceShare->timestampTo, $deviceShareDatabase->timestampTo); + } + + public function testFindByTokenNotFound(): void { + $this->expectException(DoesNotExistException::class); + $this->repository->findByToken('nonexistent-token-that-does-not-exist'); + } + + public function testFindById(): void { + $share = $this->createShare(1, 10, 20); + + $found = $this->repository->findById($share->id); + $this->assertEquals($share->id, $found->id); + $this->assertEquals($share->token, $found->token); + $this->assertEquals($share->deviceId, $found->deviceId); + $this->assertEquals($share->timestampFrom, $found->timestampFrom); + $this->assertEquals($share->timestampTo, $found->timestampTo); + } + + public function testFindByIdNotFound(): void { + $this->expectException(DoesNotExistException::class); + $this->repository->findById(-1); + } + + public function testFindByDeviceIds(): void { + $share1 = $this->createShare(10, 100, 200); + $share2 = $this->createShare(10, 300, 400); + $share3 = $this->createShare(20, 100, 200); + + $shares = iterator_to_array($this->repository->findByDeviceIds([10])); + $ids = array_map(fn (DeviceShare $s) => $s->id, $shares); + + $this->assertContains($share1->id, $ids); + $this->assertContains($share2->id, $ids); + $this->assertNotContains($share3->id, $ids); + } + + public function testFindByDeviceIdsMultipleDevices(): void { + $share1 = $this->createShare(11, 100, 200); + $share2 = $this->createShare(12, 100, 200); + $share3 = $this->createShare(13, 100, 200); + + $shares = iterator_to_array($this->repository->findByDeviceIds([11, 12])); + $ids = array_map(fn (DeviceShare $s) => $s->id, $shares); + + $this->assertContains($share1->id, $ids); + $this->assertContains($share2->id, $ids); + $this->assertNotContains($share3->id, $ids); + } + + public function testFindByDeviceIdsEmpty(): void { + $shares = iterator_to_array($this->repository->findByDeviceIds([])); + $this->assertEmpty($shares); + } + + public function testRemoveById(): void { + $share = $this->createShare(1, 1, 2); + $id = $share->id; + + $result = $this->repository->removeById($id); + $this->assertTrue($result); + + $this->expectException(DoesNotExistException::class); + $this->repository->findById($id); + } + + public function testRemoveByIdNotFound(): void { + $result = $this->repository->removeById(-1); + $this->assertFalse($result); + } + + public function testRemoveAllByDeviceId(): void { + $this->createShare(99, 1, 2); + $this->createShare(99, 3, 4); + + $result = $this->repository->removeAllByDeviceId(99); + $this->assertTrue($result); + + $remaining = iterator_to_array($this->repository->findByDeviceIds([99])); + $this->assertEmpty($remaining); + } + + public function testRemoveAllByDeviceIdNotFound(): void { + $result = $this->repository->removeAllByDeviceId(-1); + $this->assertFalse($result); + } + + public function testCreateTokensAreUnique(): void { + $share1 = $this->createShare(1, 1, 2); + $share2 = $this->createShare(1, 1, 2); + + $this->assertNotEquals($share1->token, $share2->token); + } +} diff --git a/tests/Unit/DB/FavoriteShareRepositoryTest.php b/tests/Unit/DB/FavoriteShareRepositoryTest.php new file mode 100644 index 000000000..1201a85d9 --- /dev/null +++ b/tests/Unit/DB/FavoriteShareRepositoryTest.php @@ -0,0 +1,137 @@ + +// SPDX-License-Identifier: AGPL-3.0-or-later + +namespace Unit\DB; + +use OCA\Maps\DB\FavoriteShare; +use OCA\Maps\DB\FavoriteShareRepository; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\Server; +use PHPUnit\Framework\TestCase; + +class FavoriteShareRepositoryTest extends TestCase { + private FavoriteShareRepository $repository; + /** @var list */ + private array $created = []; + + public function setUp(): void { + parent::setUp(); + $this->repository = Server::get(FavoriteShareRepository::class); + } + + public function tearDown(): void { + foreach ($this->created as $share) { + try { + $this->repository->delete($share); + } catch (\Throwable) { + // already removed during the test + } + } + $this->created = []; + parent::tearDown(); + } + + private function createShare(string $owner = 'alice', string $category = 'Test'): FavoriteShare { + $share = $this->repository->create($owner, $category); + $this->created[] = $share; + return $share; + } + + public function testCreate(): void { + $share = $this->createShare('alice', 'Holidays'); + + $this->assertNotNull($share->id); + $this->assertNotEmpty($share->token); + $this->assertEquals('alice', $share->owner); + $this->assertEquals('Holidays', $share->category); + } + + public function testFindByToken(): void { + $share = $this->createShare('alice', 'Work'); + + $found = $this->repository->findByToken($share->token); + $this->assertEquals($share->id, $found->id); + $this->assertEquals($share->token, $found->token); + $this->assertEquals($share->owner, $found->owner); + $this->assertEquals($share->category, $found->category); + } + + public function testFindByTokenNotFound(): void { + $this->expectException(DoesNotExistException::class); + $this->repository->findByToken('nonexistent-token-that-does-not-exist'); + } + + public function testFindByOwnerAndCategory(): void { + $share = $this->createShare('bob', 'Friends'); + + $found = $this->repository->findByOwnerAndCategory('bob', 'Friends'); + $this->assertEquals($share->id, $found->id); + $this->assertEquals($share->token, $found->token); + } + + public function testFindByOwnerAndCategoryNotFound(): void { + $this->expectException(DoesNotExistException::class); + $this->repository->findByOwnerAndCategory('nobody', 'NoSuchCategory'); + } + + public function testFindAllByOwner(): void { + $share1 = $this->createShare('charlie', 'Cat1'); + $share2 = $this->createShare('charlie', 'Cat2'); + $share3 = $this->createShare('dave', 'Cat1'); + + $shares = iterator_to_array($this->repository->findAllByOwner('charlie')); + $ids = array_map(fn (FavoriteShare $s) => $s->id, $shares); + + $this->assertContains($share1->id, $ids); + $this->assertContains($share2->id, $ids); + $this->assertNotContains($share3->id, $ids); + } + + public function testFindAllByOwnerEmpty(): void { + $shares = iterator_to_array($this->repository->findAllByOwner('nosuchuser_xyzxyz')); + $this->assertEmpty($shares); + } + + public function testFindOrCreateByOwnerAndCategoryCreates(): void { + $share = $this->repository->findOrCreateByOwnerAndCategory('eve', 'NewCat'); + $this->created[] = $share; + + $this->assertNotNull($share->id); + $this->assertEquals('eve', $share->owner); + $this->assertEquals('NewCat', $share->category); + } + + public function testFindOrCreateByOwnerAndCategoryFindsExisting(): void { + $existing = $this->createShare('frank', 'ExistingCat'); + + $found = $this->repository->findOrCreateByOwnerAndCategory('frank', 'ExistingCat'); + $this->assertEquals($existing->id, $found->id); + $this->assertEquals($existing->token, $found->token); + } + + public function testRemoveByOwnerAndCategory(): void { + $share = $this->createShare('grace', 'ToRemove'); + + $result = $this->repository->removeByOwnerAndCategory('grace', 'ToRemove'); + $this->assertTrue($result); + + $this->expectException(DoesNotExistException::class); + $this->repository->findByOwnerAndCategory('grace', 'ToRemove'); + } + + public function testRemoveByOwnerAndCategoryNotFound(): void { + $result = $this->repository->removeByOwnerAndCategory('nobody', 'NoSuchCategory'); + $this->assertFalse($result); + } + + public function testCreateTokensAreUnique(): void { + $share1 = $this->createShare('heidi', 'Cat1'); + $share2 = $this->createShare('heidi', 'Cat2'); + + $this->assertNotEquals($share1->token, $share2->token); + } +} diff --git a/tests/Unit/DB/GeophotoRepositoryTest.php b/tests/Unit/DB/GeophotoRepositoryTest.php new file mode 100644 index 000000000..4b5229d4b --- /dev/null +++ b/tests/Unit/DB/GeophotoRepositoryTest.php @@ -0,0 +1,224 @@ + +// SPDX-License-Identifier: AGPL-3.0-or-later + +namespace Unit\DB; + +use OCA\Maps\DB\Geophoto; +use OCA\Maps\DB\GeophotoRepository; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\Server; +use PHPUnit\Framework\TestCase; + +class GeophotoRepositoryTest extends TestCase { + private GeophotoRepository $repository; + /** @var list */ + private array $created = []; + + public function setUp(): void { + parent::setUp(); + $this->repository = Server::get(GeophotoRepository::class); + } + + public function tearDown(): void { + foreach ($this->created as $geophoto) { + try { + $this->repository->delete($geophoto); + } catch (\Throwable) { + // already removed during the test + } + } + $this->created = []; + parent::tearDown(); + } + + private function createGeophoto( + int $fileId, + string $userId = 'testuser', + ?float $lat = 48.8566, + ?float $lng = 2.3522, + ?string $dateTaken = '2023-06-01', + ): Geophoto { + $geophoto = new Geophoto(); + $geophoto->fileId = $fileId; + $geophoto->userId = $userId; + $geophoto->lat = $lat; + $geophoto->lng = $lng; + $geophoto->dateTaken = $dateTaken !== null ? new \DateTime($dateTaken) : null; + + $geophoto = $this->repository->insert($geophoto); + $this->created[] = $geophoto; + return $geophoto; + } + + public function testFind(): void { + $geophoto = $this->createGeophoto(1001); + + $found = $this->repository->find($geophoto->id); + $this->assertEquals($geophoto->id, $found->id); + $this->assertEquals($geophoto->fileId, $found->fileId); + $this->assertEquals($geophoto->userId, $found->userId); + $this->assertEqualsWithDelta($geophoto->lat, $found->lat, 0.0001); + $this->assertEqualsWithDelta($geophoto->lng, $found->lng, 0.0001); + } + + public function testFindNotFound(): void { + $this->expectException(DoesNotExistException::class); + $this->repository->find(-1); + } + + public function testFindByFileIdUserId(): void { + $geophoto = $this->createGeophoto(2001, 'alice'); + + $found = $this->repository->findByFileIdUserId(2001, 'alice'); + $this->assertEquals($geophoto->id, $found->id); + $this->assertEquals(2001, $found->fileId); + $this->assertEquals('alice', $found->userId); + } + + public function testFindByFileIdUserIdNotFound(): void { + $this->expectException(DoesNotExistException::class); + $this->repository->findByFileIdUserId(-1, 'nobody'); + } + + public function testFindByFileId(): void { + $geophoto = $this->createGeophoto(3001); + + $found = $this->repository->findByFileId(3001); + $this->assertEquals($geophoto->id, $found->id); + $this->assertEquals(3001, $found->fileId); + } + + public function testFindByFileIdNotFound(): void { + $this->expectException(DoesNotExistException::class); + $this->repository->findByFileId(-1); + } + + public function testFindAllReturnsOnlyLocalized(): void { + $localized = $this->createGeophoto(4001, 'bob', 51.5, -0.1); + $nonLocalized = $this->createGeophoto(4002, 'bob', null, null); + + $results = $this->repository->findAll('bob'); + $ids = array_map(fn (Geophoto $g) => $g->id, $results); + + $this->assertContains($localized->id, $ids); + $this->assertNotContains($nonLocalized->id, $ids); + } + + public function testFindAllOrderedByDateTaken(): void { + $older = $this->createGeophoto(5001, 'carol', 1.0, 1.0, '2020-01-01'); + $newer = $this->createGeophoto(5002, 'carol', 2.0, 2.0, '2022-01-01'); + + $results = $this->repository->findAll('carol'); + $ids = array_map(fn (Geophoto $g) => $g->id, $results); + + $this->assertContains($older->id, $ids); + $this->assertContains($newer->id, $ids); + $olderIndex = array_search($older->id, $ids); + $newerIndex = array_search($newer->id, $ids); + $this->assertLessThan($newerIndex, $olderIndex, 'Older photo should come before newer one'); + } + + public function testFindAllWithLimitAndOffset(): void { + $this->createGeophoto(6001, 'dan', 1.0, 1.0, '2021-01-01'); + $this->createGeophoto(6002, 'dan', 2.0, 2.0, '2022-01-01'); + $this->createGeophoto(6003, 'dan', 3.0, 3.0, '2023-01-01'); + + $limited = $this->repository->findAll('dan', limit: 2); + $this->assertCount(2, $limited); + + $offset = $this->repository->findAll('dan', offset: 1); + $this->assertCount(2, $offset); + } + + public function testFindAllNonLocalizedReturnsOnlyNonLocalized(): void { + $localized = $this->createGeophoto(7001, 'erin', 48.8, 2.3); + $nonLocalized = $this->createGeophoto(7002, 'erin', null, null); + + $results = $this->repository->findAllNonLocalized('erin'); + $ids = array_map(fn (Geophoto $g) => $g->id, $results); + + $this->assertContains($nonLocalized->id, $ids); + $this->assertNotContains($localized->id, $ids); + } + + public function testDeleteByFileId(): void { + $geophoto = $this->createGeophoto(8001); + + $count = $this->repository->deleteByFileId(8001); + $this->assertEquals(1, $count); + + $this->expectException(DoesNotExistException::class); + $this->repository->find($geophoto->id); + } + + public function testDeleteByFileIdNoneDeleted(): void { + $count = $this->repository->deleteByFileId(-1); + $this->assertEquals(0, $count); + } + + public function testDeleteByFileIdUserId(): void { + $geophoto = $this->createGeophoto(9001, 'frank'); + + $count = $this->repository->deleteByFileIdUserId(9001, 'frank'); + $this->assertEquals(1, $count); + + $this->expectException(DoesNotExistException::class); + $this->repository->find($geophoto->id); + } + + public function testDeleteByFileIdUserIdOnlyDeletesMatchingUser(): void { + $geophoto = $this->createGeophoto(9002, 'grace'); + + $count = $this->repository->deleteByFileIdUserId(9002, 'otheruser'); + $this->assertEquals(0, $count); + + $found = $this->repository->find($geophoto->id); + $this->assertEquals($geophoto->id, $found->id); + } + + public function testDeleteAll(): void { + $this->createGeophoto(10001, 'heidi'); + $this->createGeophoto(10002, 'heidi'); + $this->createGeophoto(10003, 'ivan'); + + $count = $this->repository->deleteAll('heidi'); + $this->assertEquals(2, $count); + + $remaining = $this->repository->findAll('heidi'); + $this->assertEmpty($remaining); + + $ivanPhotos = $this->repository->findAll('ivan'); + $this->assertCount(1, $ivanPhotos); + } + + public function testUpdateByFileId(): void { + $geophoto = $this->createGeophoto(11001, 'judy', null, null); + + $count = $this->repository->updateByFileId(11001, 52.37, 4.90); + $this->assertEquals(1, $count); + + $updated = $this->repository->find($geophoto->id); + $this->assertEqualsWithDelta(52.37, $updated->lat, 0.0001); + $this->assertEqualsWithDelta(4.90, $updated->lng, 0.0001); + } + + public function testUpdateByFileIdToNull(): void { + $geophoto = $this->createGeophoto(11002, 'judy', 48.8, 2.3); + + $count = $this->repository->updateByFileId(11002, null, null); + $this->assertEquals(1, $count); + + $updated = $this->repository->find($geophoto->id); + $this->assertNull($updated->lat); + $this->assertNull($updated->lng); + } + + public function testUpdateByFileIdNoneUpdated(): void { + $count = $this->repository->updateByFileId(-1, 1.0, 1.0); + $this->assertEquals(0, $count); + } +} From a070dadfdd1b97caef7266c2e61baf7b8aed89b6 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 11 Jun 2026 20:41:05 +0200 Subject: [PATCH 4/5] fix(Db): Improve doc Signed-off-by: Carl Schwan --- lib/AppFramework/Db/Attribute/Column.php | 9 +++++++-- lib/AppFramework/Db/Attribute/Entity.php | 1 + lib/AppFramework/Db/Attribute/Id.php | 3 +-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/AppFramework/Db/Attribute/Column.php b/lib/AppFramework/Db/Attribute/Column.php index d7beefd2a..13a883d71 100644 --- a/lib/AppFramework/Db/Attribute/Column.php +++ b/lib/AppFramework/Db/Attribute/Column.php @@ -12,13 +12,13 @@ use Attribute; use OCP\AppFramework\Attribute\Consumable; +use OCP\DB\Types; /** * Attribute for mapping a property in an entity to a database column. * * ```php - * #[Entity] - * #[Table(name: 'my_entity'] + * #[Entity(name: 'my_entity'] * final class MyEntity { * #[Column(name: 'my_column', type: Types::String, default: '')] * public string $myColumn = ''; @@ -31,10 +31,15 @@ #[Consumable(since: '35.0.0')] final readonly class Column { public function __construct( + /** @param non-empty-string $name The name of the column in the database. */ public string $name, + /** @param Types::* $type The type of the column in the database. */ public string $type, + /** @param ?int $length The length of the column (relevant for Types::STRING) */ public ?int $length = null, + /** @param bool $nullable Whether the column is nullable in the database */ public bool $nullable = false, + /** @param mixed $default The default value for the column in the database. */ public mixed $default = null, ) { } diff --git a/lib/AppFramework/Db/Attribute/Entity.php b/lib/AppFramework/Db/Attribute/Entity.php index abc0fde6d..22e813f41 100644 --- a/lib/AppFramework/Db/Attribute/Entity.php +++ b/lib/AppFramework/Db/Attribute/Entity.php @@ -30,6 +30,7 @@ #[Consumable(since: '35.0.0')] final readonly class Entity { public function __construct( + /** @param non-empty-string $name */ public string $name, ) { } diff --git a/lib/AppFramework/Db/Attribute/Id.php b/lib/AppFramework/Db/Attribute/Id.php index 30d57a067..9659e137d 100644 --- a/lib/AppFramework/Db/Attribute/Id.php +++ b/lib/AppFramework/Db/Attribute/Id.php @@ -18,8 +18,7 @@ * Attribute for marking a column as a primary id. * * ```php - * #[Entity] - * #[Table(name: 'my_entity'] + * #[Entity(name: 'my_entity'] * final class MyEntity { * #[Id(generatorClass: ISnowflakeGenerator::class)] * #[Column(name: 'id', type: Types::BIGINT)] From e587b9cf6761b0a1421e4c7e45bd410405628bff Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 11 Jun 2026 20:41:23 +0200 Subject: [PATCH 5/5] chore(tests): run rector Signed-off-by: Carl Schwan --- tests/Unit/DB/DeviceShareRepositoryTest.php | 4 ++-- tests/Unit/DB/FavoriteShareRepositoryTest.php | 4 ++-- tests/Unit/DB/GeophotoRepositoryTest.php | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/Unit/DB/DeviceShareRepositoryTest.php b/tests/Unit/DB/DeviceShareRepositoryTest.php index 3b3971e10..62ca56296 100644 --- a/tests/Unit/DB/DeviceShareRepositoryTest.php +++ b/tests/Unit/DB/DeviceShareRepositoryTest.php @@ -88,7 +88,7 @@ public function testFindByDeviceIds(): void { $share3 = $this->createShare(20, 100, 200); $shares = iterator_to_array($this->repository->findByDeviceIds([10])); - $ids = array_map(fn (DeviceShare $s) => $s->id, $shares); + $ids = array_map(fn (DeviceShare $s): ?int => $s->id, $shares); $this->assertContains($share1->id, $ids); $this->assertContains($share2->id, $ids); @@ -101,7 +101,7 @@ public function testFindByDeviceIdsMultipleDevices(): void { $share3 = $this->createShare(13, 100, 200); $shares = iterator_to_array($this->repository->findByDeviceIds([11, 12])); - $ids = array_map(fn (DeviceShare $s) => $s->id, $shares); + $ids = array_map(fn (DeviceShare $s): ?int => $s->id, $shares); $this->assertContains($share1->id, $ids); $this->assertContains($share2->id, $ids); diff --git a/tests/Unit/DB/FavoriteShareRepositoryTest.php b/tests/Unit/DB/FavoriteShareRepositoryTest.php index 1201a85d9..85933aff2 100644 --- a/tests/Unit/DB/FavoriteShareRepositoryTest.php +++ b/tests/Unit/DB/FavoriteShareRepositoryTest.php @@ -84,7 +84,7 @@ public function testFindAllByOwner(): void { $share3 = $this->createShare('dave', 'Cat1'); $shares = iterator_to_array($this->repository->findAllByOwner('charlie')); - $ids = array_map(fn (FavoriteShare $s) => $s->id, $shares); + $ids = array_map(fn (FavoriteShare $s): ?int => $s->id, $shares); $this->assertContains($share1->id, $ids); $this->assertContains($share2->id, $ids); @@ -114,7 +114,7 @@ public function testFindOrCreateByOwnerAndCategoryFindsExisting(): void { } public function testRemoveByOwnerAndCategory(): void { - $share = $this->createShare('grace', 'ToRemove'); + $this->createShare('grace', 'ToRemove'); $result = $this->repository->removeByOwnerAndCategory('grace', 'ToRemove'); $this->assertTrue($result); diff --git a/tests/Unit/DB/GeophotoRepositoryTest.php b/tests/Unit/DB/GeophotoRepositoryTest.php index 4b5229d4b..1d3f5b847 100644 --- a/tests/Unit/DB/GeophotoRepositoryTest.php +++ b/tests/Unit/DB/GeophotoRepositoryTest.php @@ -102,7 +102,7 @@ public function testFindAllReturnsOnlyLocalized(): void { $nonLocalized = $this->createGeophoto(4002, 'bob', null, null); $results = $this->repository->findAll('bob'); - $ids = array_map(fn (Geophoto $g) => $g->id, $results); + $ids = array_map(fn (Geophoto $g): ?int => $g->id, $results); $this->assertContains($localized->id, $ids); $this->assertNotContains($nonLocalized->id, $ids); @@ -113,7 +113,7 @@ public function testFindAllOrderedByDateTaken(): void { $newer = $this->createGeophoto(5002, 'carol', 2.0, 2.0, '2022-01-01'); $results = $this->repository->findAll('carol'); - $ids = array_map(fn (Geophoto $g) => $g->id, $results); + $ids = array_map(fn (Geophoto $g): ?int => $g->id, $results); $this->assertContains($older->id, $ids); $this->assertContains($newer->id, $ids); @@ -139,7 +139,7 @@ public function testFindAllNonLocalizedReturnsOnlyNonLocalized(): void { $nonLocalized = $this->createGeophoto(7002, 'erin', null, null); $results = $this->repository->findAllNonLocalized('erin'); - $ids = array_map(fn (Geophoto $g) => $g->id, $results); + $ids = array_map(fn (Geophoto $g): ?int => $g->id, $results); $this->assertContains($nonLocalized->id, $ids); $this->assertNotContains($localized->id, $ids);