From 054a14630925fd1248f08e6fa26756221417a49b Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 4 Nov 2025 21:55:40 +0100 Subject: [PATCH 01/10] feat(entity): Provide a new API for entities Entity are now simple data object with attributes for the database mapping. They are manipulated via a Repository which reads the attributes to insert, update, delete and queries the PDO from the database. Using attributes makes it easier to extends in the future with relations (ManyToMany, OneToMany, OneToOne, ...). The design of the Repository is based on a mix between the Doctrine ORM repository while keeping some methods from the QBMapper for easier porting. Signed-off-by: Carl Schwan --- .../lib/Db/BackupCode.php | 42 +- .../lib/Db/BackupCodeMapper.php | 48 +- .../lib/Service/BackupCodeStorage.php | 31 +- .../tests/Db/BackupCodeMapperTest.php | 34 +- .../Unit/Service/BackupCodeStorageTest.php | 83 ++-- lib/composer/composer/autoload_classmap.php | 5 + lib/composer/composer/autoload_static.php | 5 + lib/private/TagManager.php | 2 +- lib/private/Tagging/Tag.php | 80 +--- lib/private/Tagging/TagMapper.php | 45 +- lib/private/Tags.php | 14 +- .../AppFramework/Db/Attribute/Column.php | 41 ++ .../AppFramework/Db/Attribute/Entity.php | 33 ++ lib/public/AppFramework/Db/Attribute/Id.php | 39 ++ .../AppFramework/Db/Attribute/Table.php | 37 ++ lib/public/AppFramework/Db/QBMapper.php | 2 +- lib/public/AppFramework/Db/Repository.php | 434 ++++++++++++++++++ 17 files changed, 741 insertions(+), 234 deletions(-) create mode 100644 lib/public/AppFramework/Db/Attribute/Column.php create mode 100644 lib/public/AppFramework/Db/Attribute/Entity.php create mode 100644 lib/public/AppFramework/Db/Attribute/Id.php create mode 100644 lib/public/AppFramework/Db/Attribute/Table.php create mode 100644 lib/public/AppFramework/Db/Repository.php diff --git a/apps/twofactor_backupcodes/lib/Db/BackupCode.php b/apps/twofactor_backupcodes/lib/Db/BackupCode.php index 07ce5cdca1162..c20d851406e3d 100644 --- a/apps/twofactor_backupcodes/lib/Db/BackupCode.php +++ b/apps/twofactor_backupcodes/lib/Db/BackupCode.php @@ -9,24 +9,26 @@ namespace OCA\TwoFactorBackupCodes\Db; -use OCP\AppFramework\Db\Entity; - -/** - * @method string getUserId() - * @method void setUserId(string $userId) - * @method string getCode() - * @method void setCode(string $code) - * @method int getUsed() - * @method void setUsed(int $code) - */ -class BackupCode extends Entity { - - /** @var string */ - protected $userId; - - /** @var string */ - protected $code; - - /** @var int */ - protected $used; +use OCP\AppFramework\Db\Attribute\Column; +use OCP\AppFramework\Db\Attribute\Entity; +use OCP\AppFramework\Db\Attribute\Id; +use OCP\AppFramework\Db\Attribute\Table; +use OCP\DB\Types; +use OCP\Snowflake\IGenerator; + +#[Entity] +#[Table(name: 'twofactor_backupcodes')] +final class BackupCode { + #[Id(generatorClass: IGenerator::class)] + #[Column(name: 'id', type: Types::STRING, length: 64, nullable: false)] + public ?string $id = null; + + #[Column(name: 'user_id', type: Types::STRING, length: 64, nullable: false)] + public string $userId; + + #[Column(name: 'code', type: Types::STRING, length: 128, nullable: false)] + public string $code; + + #[Column(name: 'used', type: Types::SMALLINT, nullable: false, default: 0)] + public int $used = 0; } diff --git a/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php b/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php index 3b7829ff1281b..c2453e4ec4db3 100644 --- a/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php +++ b/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php @@ -9,51 +9,37 @@ namespace OCA\TwoFactorBackupCodes\Db; -use OCP\AppFramework\Db\QBMapper; -use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\AppFramework\Db\Repository; use OCP\IDBConnection; use OCP\IUser; /** - * @template-extends QBMapper + * @template-extends Repository */ -class BackupCodeMapper extends QBMapper { +class BackupCodeMapper extends Repository { public function __construct(IDBConnection $db) { - parent::__construct($db, 'twofactor_backupcodes'); + parent::__construct($db, BackupCode::class); } /** - * @param IUser $user - * @return BackupCode[] + * @return \Generator */ - public function getBackupCodes(IUser $user): array { - /* @var IQueryBuilder $qb */ - $qb = $this->db->getQueryBuilder(); - - $qb->select('id', 'user_id', 'code', 'used') - ->from('twofactor_backupcodes') - ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($user->getUID()))); - - return self::findEntities($qb); + public function findByUser(IUser $user): \Generator { + return $this->findBy([ + 'userId' => $user->getUID(), + ]); } - /** - * @param IUser $user - */ - public function deleteCodes(IUser $user): void { - $this->deleteCodesByUserId($user->getUID()); + public function deleteByUser(IUser $user): void { + $this->deleteBy([ + 'userId' => $user->getUID(), + ]); } - /** - * @param string $uid - */ - public function deleteCodesByUserId(string $uid): void { - /* @var IQueryBuilder $qb */ - $qb = $this->db->getQueryBuilder(); - - $qb->delete('twofactor_backupcodes') - ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($uid))); - $qb->executeStatement(); + public function findOneByUser(IUser $user): ?BackupCode { + return $this->findOneBy([ + 'userId' => $user->getUID(), + ]); } /** diff --git a/apps/twofactor_backupcodes/lib/Service/BackupCodeStorage.php b/apps/twofactor_backupcodes/lib/Service/BackupCodeStorage.php index 9251858b1d203..918c76463441b 100644 --- a/apps/twofactor_backupcodes/lib/Service/BackupCodeStorage.php +++ b/apps/twofactor_backupcodes/lib/Service/BackupCodeStorage.php @@ -37,16 +37,16 @@ public function createCodes(IUser $user, int $number = 10): array { $result = []; // Delete existing ones - $this->mapper->deleteCodes($user); + $this->mapper->deleteByUser($user); $uid = $user->getUID(); foreach (range(1, min([$number, 20])) as $i) { $code = $this->random->generate(self::CODE_LENGTH, ISecureRandom::CHAR_HUMAN_READABLE); $dbCode = new BackupCode(); - $dbCode->setUserId($uid); - $dbCode->setCode($this->hasher->hash($code)); - $dbCode->setUsed(0); + $dbCode->userId = $uid; + $dbCode->code = $this->hasher->hash($code); + $dbCode->used = 0; $this->mapper->insert($dbCode); $result[] = $code; @@ -62,8 +62,7 @@ public function createCodes(IUser $user, int $number = 10): array { * @return bool */ public function hasBackupCodes(IUser $user): bool { - $codes = $this->mapper->getBackupCodes($user); - return count($codes) > 0; + return $this->mapper->findOneByUser($user) !== null; } /** @@ -71,14 +70,16 @@ public function hasBackupCodes(IUser $user): bool { * @return array */ public function getBackupCodesState(IUser $user): array { - $codes = $this->mapper->getBackupCodes($user); - $total = count($codes); + $codes = $this->mapper->findByUser($user); + $total = 0; $used = 0; - array_walk($codes, function (BackupCode $code) use (&$used): void { - if ((int)$code->getUsed() === 1) { + + foreach ($codes as $code) { + $total++; + if ($code->used === 1) { $used++; } - }); + } return [ 'enabled' => $total > 0, 'total' => $total, @@ -87,17 +88,17 @@ public function getBackupCodesState(IUser $user): array { } public function validateCode(IUser $user, string $code): bool { - $dbCodes = $this->mapper->getBackupCodes($user); + $dbCodes = $this->mapper->findByUser($user); foreach ($dbCodes as $dbCode) { - if ((int)$dbCode->getUsed() === 0 && $this->hasher->verify($code, $dbCode->getCode())) { - return ($this->mapper->markUsedIfUnused($dbCode) === 1); + if ($dbCode->used === 0 && $this->hasher->verify($code, $dbCode->code)) { + return $this->mapper->markUsedIfUnused($dbCode) === 1; } } return false; } public function deleteCodes(IUser $user): void { - $this->mapper->deleteCodes($user); + $this->mapper->deleteByUser($user); } } diff --git a/apps/twofactor_backupcodes/tests/Db/BackupCodeMapperTest.php b/apps/twofactor_backupcodes/tests/Db/BackupCodeMapperTest.php index 5df3af9ba993a..15b2b3ce46030 100644 --- a/apps/twofactor_backupcodes/tests/Db/BackupCodeMapperTest.php +++ b/apps/twofactor_backupcodes/tests/Db/BackupCodeMapperTest.php @@ -46,14 +46,14 @@ protected function tearDown(): void { public function testGetBackupCodes(): void { $code1 = new BackupCode(); - $code1->setUserId($this->testUID); - $code1->setCode('1|$2y$10$Fyo.DkMtkaHapVvRVbQBeeIdi5x/6nmPnxiBzD0GDKa08NMus5xze'); - $code1->setUsed(1); + $code1->userId = $this->testUID; + $code1->code = '1|$2y$10$Fyo.DkMtkaHapVvRVbQBeeIdi5x/6nmPnxiBzD0GDKa08NMus5xze'; + $code1->used = 1; $code2 = new BackupCode(); - $code2->setUserId($this->testUID); - $code2->setCode('1|$2y$10$nj3sZaCqGN8t6.SsnNADt.eX34UCkdX6FPx.r.rIwE6Jj3vi5wyt2'); - $code2->setUsed(0); + $code2->userId = $this->testUID; + $code2->code = '1|$2y$10$nj3sZaCqGN8t6.SsnNADt.eX34UCkdX6FPx.r.rIwE6Jj3vi5wyt2'; + $code2->used = 0; $this->mapper->insert($code1); $this->mapper->insert($code2); @@ -63,7 +63,7 @@ public function testGetBackupCodes(): void { ->method('getUID') ->willReturn($this->testUID); - $dbCodes = $this->mapper->getBackupCodes($user); + $dbCodes = iterator_to_array($this->mapper->findByUser($user)); $this->assertCount(2, $dbCodes); $this->assertInstanceOf(BackupCode::class, $dbCodes[0]); @@ -72,9 +72,9 @@ public function testGetBackupCodes(): void { public function testDeleteCodes(): void { $code = new BackupCode(); - $code->setUserId($this->testUID); - $code->setCode('1|$2y$10$CagG8pEhZL.xDirtCCP/KuuWtnsAasgq60zY9rU46dBK4w8yW0Z/y'); - $code->setUsed(1); + $code->userId = $this->testUID; + $code->code = '1|$2y$10$CagG8pEhZL.xDirtCCP/KuuWtnsAasgq60zY9rU46dBK4w8yW0Z/y'; + $code->used = 1; $user = $this->getMockBuilder(IUser::class)->getMock(); $user->expects($this->any()) ->method('getUID') @@ -82,24 +82,24 @@ public function testDeleteCodes(): void { $this->mapper->insert($code); - $this->assertCount(1, $this->mapper->getBackupCodes($user)); + $this->assertCount(1, iterator_to_array($this->mapper->findByUser($user))); - $this->mapper->deleteCodes($user); + $this->mapper->deleteByUser($user); - $this->assertCount(0, $this->mapper->getBackupCodes($user)); + $this->assertCount(0, iterator_to_array($this->mapper->findByUser($user))); } public function testInsertArgonEncryptedCodes(): void { $code = new BackupCode(); - $code->setUserId($this->testUID); - $code->setCode('2|$argon2i$v=19$m=1024,t=2,p=2$MjJWUjRFWndtMm5BWGxOag$BusVxLeFyiLLWtaVvX/JRFBiPdZcjRrzpQ/rAhn6vqY'); - $code->setUsed(1); + $code->userId = $this->testUID; + $code->code = '2|$argon2i$v=19$m=1024,t=2,p=2$MjJWUjRFWndtMm5BWGxOag$BusVxLeFyiLLWtaVvX/JRFBiPdZcjRrzpQ/rAhn6vqY'; + $code->used = 1; $user = $this->getMockBuilder(IUser::class)->getMock(); $user->expects($this->any()) ->method('getUID') ->willReturn($this->testUID); $this->mapper->insert($code); - $this->assertCount(1, $this->mapper->getBackupCodes($user)); + $this->assertCount(1, iterator_to_array($this->mapper->findByUser($user))); } } diff --git a/apps/twofactor_backupcodes/tests/Unit/Service/BackupCodeStorageTest.php b/apps/twofactor_backupcodes/tests/Unit/Service/BackupCodeStorageTest.php index 5c4d820f4bd53..2b61bcacf5c6f 100644 --- a/apps/twofactor_backupcodes/tests/Unit/Service/BackupCodeStorageTest.php +++ b/apps/twofactor_backupcodes/tests/Unit/Service/BackupCodeStorageTest.php @@ -51,9 +51,9 @@ public function testCreateCodes(): void { ->with('CODEABCDEF') ->willReturn('HASHEDCODE'); $row = new BackupCode(); - $row->setUserId('fritz'); - $row->setCode('HASHEDCODE'); - $row->setUsed(0); + $row->userId = 'fritz'; + $row->code = 'HASHEDCODE'; + $row->used = 0; $this->mapper->expects($this->exactly($number)) ->method('insert') ->with($this->equalTo($row)); @@ -72,27 +72,24 @@ public function testCreateCodes(): void { public function testHasBackupCodes(): void { $user = $this->createMock(IUser::class); - $codes = [ - new BackupCode(), - new BackupCode(), - ]; $this->mapper->expects($this->once()) - ->method('getBackupCodes') + ->method('findOneByUser') ->with($user) - ->willReturn($codes); + ->willReturnCallback(function () { + return new BackupCode(); + }); $this->assertTrue($this->storage->hasBackupCodes($user)); } public function testHasBackupCodesNoCodes(): void { $user = $this->createMock(IUser::class); - $codes = []; $this->mapper->expects($this->once()) - ->method('getBackupCodes') + ->method('findOneByUser') ->with($user) - ->willReturn($codes); + ->willReturn(null); $this->assertFalse($this->storage->hasBackupCodes($user)); } @@ -101,18 +98,17 @@ public function testGetBackupCodeState(): void { $user = $this->createMock(IUser::class); $code1 = new BackupCode(); - $code1->setUsed(1); + $code1->used = 1; $code2 = new BackupCode(); - $code2->setUsed(0); - $codes = [ - $code1, - $code2, - ]; + $code2->used = 0; $this->mapper->expects($this->once()) - ->method('getBackupCodes') + ->method('findByUser') ->with($user) - ->willReturn($codes); + ->willReturnCallback(function () use ($code1, $code2) { + yield $code1; + yield $code2; + }); $expected = [ 'enabled' => true, @@ -125,12 +121,10 @@ public function testGetBackupCodeState(): void { public function testGetBackupCodeDisabled(): void { $user = $this->createMock(IUser::class); - $codes = []; - $this->mapper->expects($this->once()) - ->method('getBackupCodes') + ->method('findByUser') ->with($user) - ->willReturn($codes); + ->willReturnCallback(function () { if (false) { yield true; }}); $expected = [ 'enabled' => false, @@ -143,16 +137,15 @@ public function testGetBackupCodeDisabled(): void { public function testValidateCode(): void { $user = $this->createMock(IUser::class); $code = new BackupCode(); - $code->setUsed(0); - $code->setCode('HASHEDVALUE'); - $codes = [ - $code, - ]; + $code->used = 0; + $code->code = 'HASHEDVALUE'; $this->mapper->expects($this->once()) - ->method('getBackupCodes') + ->method('findByUser') ->with($user) - ->willReturn($codes); + ->willReturnCallback(function () use ($code) { + yield $code; + }); $this->hasher->expects($this->once()) ->method('verify') ->with('CHALLENGE', 'HASHEDVALUE', $this->anything()) @@ -168,16 +161,15 @@ public function testValidateCode(): void { public function testValidateUsedCode(): void { $user = $this->createMock(IUser::class); $code = new BackupCode(); - $code->setUsed(1); - $code->setCode('HASHEDVALUE'); - $codes = [ - $code, - ]; + $code->used = 1; + $code->code = 'HASHEDVALUE'; $this->mapper->expects($this->once()) - ->method('getBackupCodes') + ->method('findByUser') ->with($user) - ->willReturn($codes); + ->willReturnCallback(function () use ($code) { + yield $code; + }); $this->hasher->expects($this->never()) ->method('verify'); $this->mapper->expects($this->never()) @@ -189,16 +181,15 @@ public function testValidateUsedCode(): void { public function testValidateCodeWithWrongHash(): void { $user = $this->createMock(IUser::class); $code = new BackupCode(); - $code->setUsed(0); - $code->setCode('HASHEDVALUE'); - $codes = [ - $code, - ]; + $code->used = 0; + $code->code = 'HASHEDVALUE'; $this->mapper->expects($this->once()) - ->method('getBackupCodes') + ->method('findByUser') ->with($user) - ->willReturn($codes); + ->willReturnCallback(function () use ($code) { + yield $code; + }); $this->hasher->expects($this->once()) ->method('verify') ->with('CHALLENGE', 'HASHEDVALUE') @@ -212,7 +203,7 @@ public function testValidateCodeWithWrongHash(): void { public function testDeleteCodes(): void { $user = $this->createMock(IUser::class); $this->mapper->expects($this->once()) - ->method('deleteCodes') + ->method('deleteByUser') ->with($user); $this->storage->deleteCodes($user); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index b23819d093969..54f02f0ca198d 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -108,11 +108,16 @@ 'OCP\\AppFramework\\Bootstrap\\IBootstrap' => $baseDir . '/lib/public/AppFramework/Bootstrap/IBootstrap.php', 'OCP\\AppFramework\\Bootstrap\\IRegistrationContext' => $baseDir . '/lib/public/AppFramework/Bootstrap/IRegistrationContext.php', 'OCP\\AppFramework\\Controller' => $baseDir . '/lib/public/AppFramework/Controller.php', + 'OCP\\AppFramework\\Db\\Attribute\\Column' => $baseDir . '/lib/public/AppFramework/Db/Attribute/Column.php', + 'OCP\\AppFramework\\Db\\Attribute\\Entity' => $baseDir . '/lib/public/AppFramework/Db/Attribute/Entity.php', + 'OCP\\AppFramework\\Db\\Attribute\\Id' => $baseDir . '/lib/public/AppFramework/Db/Attribute/Id.php', + 'OCP\\AppFramework\\Db\\Attribute\\Table' => $baseDir . '/lib/public/AppFramework/Db/Attribute/Table.php', 'OCP\\AppFramework\\Db\\DoesNotExistException' => $baseDir . '/lib/public/AppFramework/Db/DoesNotExistException.php', 'OCP\\AppFramework\\Db\\Entity' => $baseDir . '/lib/public/AppFramework/Db/Entity.php', 'OCP\\AppFramework\\Db\\IMapperException' => $baseDir . '/lib/public/AppFramework/Db/IMapperException.php', 'OCP\\AppFramework\\Db\\MultipleObjectsReturnedException' => $baseDir . '/lib/public/AppFramework/Db/MultipleObjectsReturnedException.php', 'OCP\\AppFramework\\Db\\QBMapper' => $baseDir . '/lib/public/AppFramework/Db/QBMapper.php', + 'OCP\\AppFramework\\Db\\Repository' => $baseDir . '/lib/public/AppFramework/Db/Repository.php', 'OCP\\AppFramework\\Db\\SnowflakeAwareEntity' => $baseDir . '/lib/public/AppFramework/Db/SnowflakeAwareEntity.php', 'OCP\\AppFramework\\Db\\TTransactional' => $baseDir . '/lib/public/AppFramework/Db/TTransactional.php', 'OCP\\AppFramework\\Http' => $baseDir . '/lib/public/AppFramework/Http.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 1c629facd2da4..22b3c893f0e43 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -149,11 +149,16 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\AppFramework\\Bootstrap\\IBootstrap' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Bootstrap/IBootstrap.php', 'OCP\\AppFramework\\Bootstrap\\IRegistrationContext' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Bootstrap/IRegistrationContext.php', 'OCP\\AppFramework\\Controller' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Controller.php', + 'OCP\\AppFramework\\Db\\Attribute\\Column' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/Attribute/Column.php', + 'OCP\\AppFramework\\Db\\Attribute\\Entity' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/Attribute/Entity.php', + 'OCP\\AppFramework\\Db\\Attribute\\Id' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/Attribute/Id.php', + 'OCP\\AppFramework\\Db\\Attribute\\Table' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/Attribute/Table.php', 'OCP\\AppFramework\\Db\\DoesNotExistException' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/DoesNotExistException.php', 'OCP\\AppFramework\\Db\\Entity' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/Entity.php', 'OCP\\AppFramework\\Db\\IMapperException' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/IMapperException.php', 'OCP\\AppFramework\\Db\\MultipleObjectsReturnedException' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/MultipleObjectsReturnedException.php', 'OCP\\AppFramework\\Db\\QBMapper' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/QBMapper.php', + 'OCP\\AppFramework\\Db\\Repository' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/Repository.php', 'OCP\\AppFramework\\Db\\SnowflakeAwareEntity' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/SnowflakeAwareEntity.php', 'OCP\\AppFramework\\Db\\TTransactional' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/TTransactional.php', 'OCP\\AppFramework\\Http' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http.php', diff --git a/lib/private/TagManager.php b/lib/private/TagManager.php index 2a598d47d71b0..857fccb0159c9 100644 --- a/lib/private/TagManager.php +++ b/lib/private/TagManager.php @@ -67,7 +67,7 @@ public function load($type, $defaultTags = [], $includeShared = false, $userId = } /** - * Get all users who favorited an object + * Get all users who marked an object as favorite. * * @param string $objectType * @param int $objectId diff --git a/lib/private/Tagging/Tag.php b/lib/private/Tagging/Tag.php index 11af0539cfacf..97117c53d309b 100644 --- a/lib/private/Tagging/Tag.php +++ b/lib/private/Tagging/Tag.php @@ -8,73 +8,29 @@ namespace OC\Tagging; -use OCP\AppFramework\Db\Entity; +use OCP\AppFramework\Db\Attribute\Column; +use OCP\AppFramework\Db\Attribute\Entity; +use OCP\AppFramework\Db\Attribute\Id; +use OCP\AppFramework\Db\Attribute\Table; +use OCP\DB\Types; +use OCP\Snowflake\IGenerator; /** * Class to represent a tag. - * - * @method string getOwner() - * @method void setOwner(string $owner) - * @method string getType() - * @method void setType(string $type) - * @method string getName() - * @method void setName(string $name) */ -class Tag extends Entity { - protected $owner; - protected $type; - protected $name; +#[Entity] +#[Table(name: 'vcategory')] +final class Tag { + #[Id(generatorClass: IGenerator::class)] + #[Column(name: 'id', type: Types::BIGINT, nullable: false)] + public ?string $id = null; - /** - * Constructor. - * - * @param string $owner The tag's owner - * @param string $type The type of item this tag is used for - * @param string $name The tag's name - */ - public function __construct($owner = null, $type = null, $name = null) { - $this->setOwner($owner); - $this->setType($type); - $this->setName($name); - } + #[Column(name: 'uid', type: Types::STRING, length: 64, nullable: false)] + public string $owner; - /** - * Transform a database columnname to a property - * - * @param string $columnName the name of the column - * @return string the property name - * @todo migrate existing database columns to the correct names - * to be able to drop this direct mapping - */ - #[\Override] - public function columnToProperty(string $columnName): string { - if ($columnName === 'category') { - return 'name'; - } + #[Column(name: 'type', type: Types::STRING, length: 64, nullable: false)] + public string $type; - if ($columnName === 'uid') { - return 'owner'; - } - - return parent::columnToProperty($columnName); - } - - /** - * Transform a property to a database column name - * - * @param string $property the name of the property - * @return string the column name - */ - #[\Override] - public function propertyToColumn(string $property): string { - if ($property === 'name') { - return 'category'; - } - - if ($property === 'owner') { - return 'uid'; - } - - return parent::propertyToColumn($property); - } + #[Column(name: 'category', type: Types::STRING, length: 255, nullable: false)] + public string $name; } diff --git a/lib/private/Tagging/TagMapper.php b/lib/private/Tagging/TagMapper.php index f829df9a4b624..9163d026251ed 100644 --- a/lib/private/Tagging/TagMapper.php +++ b/lib/private/Tagging/TagMapper.php @@ -8,24 +8,22 @@ namespace OC\Tagging; -use OCP\AppFramework\Db\DoesNotExistException; -use OCP\AppFramework\Db\QBMapper; -use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\AppFramework\Db\Repository; use OCP\IDBConnection; /** * Mapper for Tag entity * - * @template-extends QBMapper + * @template-extends Repository */ -class TagMapper extends QBMapper { +class TagMapper extends Repository { /** * Constructor. * * @param IDBConnection $db Instance of the Db abstraction layer. */ public function __construct(IDBConnection $db) { - parent::__construct($db, 'vcategory', Tag::class); + parent::__construct($db, Tag::class); } /** @@ -33,35 +31,14 @@ public function __construct(IDBConnection $db) { * * @param array $owners The user(s) whose tags we are going to load. * @param string $type The type of item for which we are loading tags. - * @return array An array of Tag objects. + * @return list An array of Tag objects. */ public function loadTags(array $owners, string $type): array { - $qb = $this->db->getQueryBuilder(); - $qb->select(['id', 'uid', 'type', 'category']) - ->from($this->getTableName()) - ->where($qb->expr()->in('uid', $qb->createNamedParameter($owners, IQueryBuilder::PARAM_STR_ARRAY))) - ->andWhere($qb->expr()->eq('type', $qb->createNamedParameter($type, IQueryBuilder::PARAM_STR))) - ->orderBy('category'); - return $this->findEntities($qb); - } - - /** - * Check if a given Tag object already exists in the database. - * - * @param Tag $tag The tag to look for in the database. - */ - public function tagExists(Tag $tag): bool { - $qb = $this->db->getQueryBuilder(); - $qb->select(['id', 'uid', 'type', 'category']) - ->from($this->getTableName()) - ->where($qb->expr()->eq('uid', $qb->createNamedParameter($tag->getOwner(), IQueryBuilder::PARAM_STR))) - ->andWhere($qb->expr()->eq('type', $qb->createNamedParameter($tag->getType(), IQueryBuilder::PARAM_STR))) - ->andWhere($qb->expr()->eq('category', $qb->createNamedParameter($tag->getName(), IQueryBuilder::PARAM_STR))); - try { - $this->findEntity($qb); - } catch (DoesNotExistException $e) { - return false; - } - return true; + return iterator_to_array($this->findBy([ + 'owner' => $owners, + 'type' => $type, + ], [ + 'name' => 'ASC', + ])); } } diff --git a/lib/private/Tags.php b/lib/private/Tags.php index 917ccb03f33a8..2029d558dcb81 100644 --- a/lib/private/Tags.php +++ b/lib/private/Tags.php @@ -379,14 +379,14 @@ public function addMultiple($names, bool $sync = false, ?int $id = null): bool { protected function save(): void { foreach ($this->tags as $tag) { try { - if (!$this->mapper->tagExists($tag)) { - $this->mapper->insert($tag); + $this->mapper->insert($tag); + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + $this->logger->error($e->getMessage(), [ + 'exception' => $e, + 'app' => 'core', + ]); } - } catch (\Exception $e) { - $this->logger->error($e->getMessage(), [ - 'exception' => $e, - 'app' => 'core', - ]); } } diff --git a/lib/public/AppFramework/Db/Attribute/Column.php b/lib/public/AppFramework/Db/Attribute/Column.php new file mode 100644 index 0000000000000..752ad690363da --- /dev/null +++ b/lib/public/AppFramework/Db/Attribute/Column.php @@ -0,0 +1,41 @@ + */ + private array $_mappingColumnToTypes = []; + + /** @var array */ + private array $_mappingColumnToProperty = []; + + /** @var array */ + private array $_mappingPropertyToColumn = []; + + /** @var \ReflectionClass */ + private \ReflectionClass $reflection; + + private string $idProperty; + + 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 (empty($entities)) { + throw new \InvalidArgumentException('The given entity is missing a required #[Entity] attribute'); + } + + $tables = $this->reflection->getAttributes(Table::class, \ReflectionAttribute::IS_INSTANCEOF); + if (empty($tables)) { + throw new \InvalidArgumentException('The given entityClass is missing a required #[Table] attribute'); + } + $this->tableName = $tables[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 + * @since 30.0.0 + */ + 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 + * @since 33.0.0 + */ + public function findEntities(IQueryBuilder $query): array { + $result = $query->executeQuery(); + try { + $entities = []; + while ($row = $result->fetch()) { + $entities[] = $this->mapRowToEntity($row); + } + return $entities; + } finally { + $result->closeCursor(); + } + } + + 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; + } + + /** + * @psalm-param T $entity + * @return T + */ + public function insert(object $entity): object { + $insert = $this->connection->getQueryBuilder(); + + $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) { + $generatorClass = $ids[0]->newInstance()->generatorClass; + $generator = Server::get($generatorClass); + if ($generator instanceof IGenerator) { + $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(); + 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); + } + + public function deleteBy(array $criteria, ?int $limit = null): void { + $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)); + $qb->andWhere($qb->expr()->eq($column, $qb->createNamedParameter($value, $type))); + } + + if ($limit !== null) { + $qb->setMaxResults($limit); + } + + $qb->executeStatement(); + } + + /** + * Finds a single entity by a set of criteria. + * + * @param array $criteria + * @param array|null $orderBy + * @return T|null + * @since 33.0.0 + */ + 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; + } + } + + 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)); + $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; + } +} From 1051cfd72899beaa463f391c5133fed9cde1f94b Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 15 Jun 2026 17:26:42 +0200 Subject: [PATCH 02/10] feat: Implement OneToOne entity And do a lot of refactoring Signed-off-by: Carl Schwan --- apps/comments/composer/composer/installed.php | 4 +- .../lib/Db/BackupCode.php | 10 +- .../lib/Db/BackupCodeMapper.php | 2 +- lib/composer/composer/autoload_classmap.php | 14 +- lib/composer/composer/autoload_static.php | 14 +- lib/private/AppFramework/ORM/EntityInfo.php | 86 ++++ .../AppFramework/ORM/EntityManager.php | 292 ++++++++++++ .../AppFramework/ORM/PropertyAttributes.php | 25 + lib/private/Tagging/Tag.php | 10 +- lib/private/Tagging/TagMapper.php | 7 +- .../AppFramework/Db/Attribute/Table.php | 37 -- lib/public/AppFramework/Db/Repository.php | 434 ------------------ .../{Db => ORM}/Attribute/Column.php | 17 +- .../{Db => ORM}/Attribute/Entity.php | 14 +- .../AppFramework/{Db => ORM}/Attribute/Id.php | 18 +- .../AppFramework/ORM/Attribute/JoinColumn.php | 44 ++ .../AppFramework/ORM/Attribute/OneToOne.php | 81 ++++ lib/public/AppFramework/ORM/Repository.php | 319 +++++++++++++ 18 files changed, 906 insertions(+), 522 deletions(-) create mode 100644 lib/private/AppFramework/ORM/EntityInfo.php create mode 100644 lib/private/AppFramework/ORM/EntityManager.php create mode 100644 lib/private/AppFramework/ORM/PropertyAttributes.php delete mode 100644 lib/public/AppFramework/Db/Attribute/Table.php delete mode 100644 lib/public/AppFramework/Db/Repository.php rename lib/public/AppFramework/{Db => ORM}/Attribute/Column.php (55%) rename lib/public/AppFramework/{Db => ORM}/Attribute/Entity.php (71%) rename lib/public/AppFramework/{Db => ORM}/Attribute/Id.php (58%) create mode 100644 lib/public/AppFramework/ORM/Attribute/JoinColumn.php create mode 100644 lib/public/AppFramework/ORM/Attribute/OneToOne.php create mode 100644 lib/public/AppFramework/ORM/Repository.php diff --git a/apps/comments/composer/composer/installed.php b/apps/comments/composer/composer/installed.php index 1a66c7f2416b6..cc1bc16b122c2 100644 --- a/apps/comments/composer/composer/installed.php +++ b/apps/comments/composer/composer/installed.php @@ -3,7 +3,7 @@ 'name' => '__root__', 'pretty_version' => 'dev-master', 'version' => 'dev-master', - 'reference' => 'b1797842784b250fb01ed5e3bf130705eb94751b', + 'reference' => '85618b795280107ce542bff646bc0957bc09a452', 'type' => 'library', 'install_path' => __DIR__ . '/../', 'aliases' => array(), @@ -13,7 +13,7 @@ '__root__' => array( 'pretty_version' => 'dev-master', 'version' => 'dev-master', - 'reference' => 'b1797842784b250fb01ed5e3bf130705eb94751b', + 'reference' => '85618b795280107ce542bff646bc0957bc09a452', 'type' => 'library', 'install_path' => __DIR__ . '/../', 'aliases' => array(), diff --git a/apps/twofactor_backupcodes/lib/Db/BackupCode.php b/apps/twofactor_backupcodes/lib/Db/BackupCode.php index c20d851406e3d..bad6abf431e02 100644 --- a/apps/twofactor_backupcodes/lib/Db/BackupCode.php +++ b/apps/twofactor_backupcodes/lib/Db/BackupCode.php @@ -9,15 +9,13 @@ namespace OCA\TwoFactorBackupCodes\Db; -use OCP\AppFramework\Db\Attribute\Column; -use OCP\AppFramework\Db\Attribute\Entity; -use OCP\AppFramework\Db\Attribute\Id; -use OCP\AppFramework\Db\Attribute\Table; +use OCP\AppFramework\ORM\Attribute\Column; +use OCP\AppFramework\ORM\Attribute\Entity; +use OCP\AppFramework\ORM\Attribute\Id; use OCP\DB\Types; use OCP\Snowflake\IGenerator; -#[Entity] -#[Table(name: 'twofactor_backupcodes')] +#[Entity(name: 'twofactor_backupcodes')] final class BackupCode { #[Id(generatorClass: IGenerator::class)] #[Column(name: 'id', type: Types::STRING, length: 64, nullable: false)] diff --git a/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php b/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php index c2453e4ec4db3..e8259eeaf4e5b 100644 --- a/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php +++ b/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php @@ -9,7 +9,7 @@ namespace OCA\TwoFactorBackupCodes\Db; -use OCP\AppFramework\Db\Repository; +use OCP\AppFramework\ORM\Repository; use OCP\IDBConnection; use OCP\IUser; diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 54f02f0ca198d..48100df35d1a4 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -108,16 +108,11 @@ 'OCP\\AppFramework\\Bootstrap\\IBootstrap' => $baseDir . '/lib/public/AppFramework/Bootstrap/IBootstrap.php', 'OCP\\AppFramework\\Bootstrap\\IRegistrationContext' => $baseDir . '/lib/public/AppFramework/Bootstrap/IRegistrationContext.php', 'OCP\\AppFramework\\Controller' => $baseDir . '/lib/public/AppFramework/Controller.php', - 'OCP\\AppFramework\\Db\\Attribute\\Column' => $baseDir . '/lib/public/AppFramework/Db/Attribute/Column.php', - 'OCP\\AppFramework\\Db\\Attribute\\Entity' => $baseDir . '/lib/public/AppFramework/Db/Attribute/Entity.php', - 'OCP\\AppFramework\\Db\\Attribute\\Id' => $baseDir . '/lib/public/AppFramework/Db/Attribute/Id.php', - 'OCP\\AppFramework\\Db\\Attribute\\Table' => $baseDir . '/lib/public/AppFramework/Db/Attribute/Table.php', 'OCP\\AppFramework\\Db\\DoesNotExistException' => $baseDir . '/lib/public/AppFramework/Db/DoesNotExistException.php', 'OCP\\AppFramework\\Db\\Entity' => $baseDir . '/lib/public/AppFramework/Db/Entity.php', 'OCP\\AppFramework\\Db\\IMapperException' => $baseDir . '/lib/public/AppFramework/Db/IMapperException.php', 'OCP\\AppFramework\\Db\\MultipleObjectsReturnedException' => $baseDir . '/lib/public/AppFramework/Db/MultipleObjectsReturnedException.php', 'OCP\\AppFramework\\Db\\QBMapper' => $baseDir . '/lib/public/AppFramework/Db/QBMapper.php', - 'OCP\\AppFramework\\Db\\Repository' => $baseDir . '/lib/public/AppFramework/Db/Repository.php', 'OCP\\AppFramework\\Db\\SnowflakeAwareEntity' => $baseDir . '/lib/public/AppFramework/Db/SnowflakeAwareEntity.php', 'OCP\\AppFramework\\Db\\TTransactional' => $baseDir . '/lib/public/AppFramework/Db/TTransactional.php', 'OCP\\AppFramework\\Http' => $baseDir . '/lib/public/AppFramework/Http.php', @@ -184,6 +179,12 @@ 'OCP\\AppFramework\\OCS\\OCSForbiddenException' => $baseDir . '/lib/public/AppFramework/OCS/OCSForbiddenException.php', 'OCP\\AppFramework\\OCS\\OCSNotFoundException' => $baseDir . '/lib/public/AppFramework/OCS/OCSNotFoundException.php', 'OCP\\AppFramework\\OCS\\OCSPreconditionFailedException' => $baseDir . '/lib/public/AppFramework/OCS/OCSPreconditionFailedException.php', + 'OCP\\AppFramework\\ORM\\Attribute\\Column' => $baseDir . '/lib/public/AppFramework/ORM/Attribute/Column.php', + 'OCP\\AppFramework\\ORM\\Attribute\\Entity' => $baseDir . '/lib/public/AppFramework/ORM/Attribute/Entity.php', + 'OCP\\AppFramework\\ORM\\Attribute\\Id' => $baseDir . '/lib/public/AppFramework/ORM/Attribute/Id.php', + 'OCP\\AppFramework\\ORM\\Attribute\\JoinColumn' => $baseDir . '/lib/public/AppFramework/ORM/Attribute/JoinColumn.php', + 'OCP\\AppFramework\\ORM\\Attribute\\OneToOne' => $baseDir . '/lib/public/AppFramework/ORM/Attribute/OneToOne.php', + 'OCP\\AppFramework\\ORM\\Repository' => $baseDir . '/lib/public/AppFramework/ORM/Repository.php', 'OCP\\AppFramework\\PublicShareController' => $baseDir . '/lib/public/AppFramework/PublicShareController.php', 'OCP\\AppFramework\\QueryException' => $baseDir . '/lib/public/AppFramework/QueryException.php', 'OCP\\AppFramework\\Services\\IAppConfig' => $baseDir . '/lib/public/AppFramework/Services/IAppConfig.php', @@ -1209,6 +1210,9 @@ 'OC\\AppFramework\\OCS\\BaseResponse' => $baseDir . '/lib/private/AppFramework/OCS/BaseResponse.php', 'OC\\AppFramework\\OCS\\V1Response' => $baseDir . '/lib/private/AppFramework/OCS/V1Response.php', 'OC\\AppFramework\\OCS\\V2Response' => $baseDir . '/lib/private/AppFramework/OCS/V2Response.php', + 'OC\\AppFramework\\ORM\\EntityInfo' => $baseDir . '/lib/private/AppFramework/ORM/EntityInfo.php', + 'OC\\AppFramework\\ORM\\EntityManager' => $baseDir . '/lib/private/AppFramework/ORM/EntityManager.php', + 'OC\\AppFramework\\ORM\\PropertyAttributes' => $baseDir . '/lib/private/AppFramework/ORM/PropertyAttributes.php', 'OC\\AppFramework\\Routing\\RouteActionHandler' => $baseDir . '/lib/private/AppFramework/Routing/RouteActionHandler.php', 'OC\\AppFramework\\Routing\\RouteParser' => $baseDir . '/lib/private/AppFramework/Routing/RouteParser.php', 'OC\\AppFramework\\ScopedPsrLogger' => $baseDir . '/lib/private/AppFramework/ScopedPsrLogger.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 22b3c893f0e43..5e1904c5f6707 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -149,16 +149,11 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\AppFramework\\Bootstrap\\IBootstrap' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Bootstrap/IBootstrap.php', 'OCP\\AppFramework\\Bootstrap\\IRegistrationContext' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Bootstrap/IRegistrationContext.php', 'OCP\\AppFramework\\Controller' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Controller.php', - 'OCP\\AppFramework\\Db\\Attribute\\Column' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/Attribute/Column.php', - 'OCP\\AppFramework\\Db\\Attribute\\Entity' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/Attribute/Entity.php', - 'OCP\\AppFramework\\Db\\Attribute\\Id' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/Attribute/Id.php', - 'OCP\\AppFramework\\Db\\Attribute\\Table' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/Attribute/Table.php', 'OCP\\AppFramework\\Db\\DoesNotExistException' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/DoesNotExistException.php', 'OCP\\AppFramework\\Db\\Entity' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/Entity.php', 'OCP\\AppFramework\\Db\\IMapperException' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/IMapperException.php', 'OCP\\AppFramework\\Db\\MultipleObjectsReturnedException' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/MultipleObjectsReturnedException.php', 'OCP\\AppFramework\\Db\\QBMapper' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/QBMapper.php', - 'OCP\\AppFramework\\Db\\Repository' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/Repository.php', 'OCP\\AppFramework\\Db\\SnowflakeAwareEntity' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/SnowflakeAwareEntity.php', 'OCP\\AppFramework\\Db\\TTransactional' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Db/TTransactional.php', 'OCP\\AppFramework\\Http' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http.php', @@ -225,6 +220,12 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\AppFramework\\OCS\\OCSForbiddenException' => __DIR__ . '/../../..' . '/lib/public/AppFramework/OCS/OCSForbiddenException.php', 'OCP\\AppFramework\\OCS\\OCSNotFoundException' => __DIR__ . '/../../..' . '/lib/public/AppFramework/OCS/OCSNotFoundException.php', 'OCP\\AppFramework\\OCS\\OCSPreconditionFailedException' => __DIR__ . '/../../..' . '/lib/public/AppFramework/OCS/OCSPreconditionFailedException.php', + 'OCP\\AppFramework\\ORM\\Attribute\\Column' => __DIR__ . '/../../..' . '/lib/public/AppFramework/ORM/Attribute/Column.php', + 'OCP\\AppFramework\\ORM\\Attribute\\Entity' => __DIR__ . '/../../..' . '/lib/public/AppFramework/ORM/Attribute/Entity.php', + 'OCP\\AppFramework\\ORM\\Attribute\\Id' => __DIR__ . '/../../..' . '/lib/public/AppFramework/ORM/Attribute/Id.php', + 'OCP\\AppFramework\\ORM\\Attribute\\JoinColumn' => __DIR__ . '/../../..' . '/lib/public/AppFramework/ORM/Attribute/JoinColumn.php', + 'OCP\\AppFramework\\ORM\\Attribute\\OneToOne' => __DIR__ . '/../../..' . '/lib/public/AppFramework/ORM/Attribute/OneToOne.php', + 'OCP\\AppFramework\\ORM\\Repository' => __DIR__ . '/../../..' . '/lib/public/AppFramework/ORM/Repository.php', 'OCP\\AppFramework\\PublicShareController' => __DIR__ . '/../../..' . '/lib/public/AppFramework/PublicShareController.php', 'OCP\\AppFramework\\QueryException' => __DIR__ . '/../../..' . '/lib/public/AppFramework/QueryException.php', 'OCP\\AppFramework\\Services\\IAppConfig' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Services/IAppConfig.php', @@ -1250,6 +1251,9 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\AppFramework\\OCS\\BaseResponse' => __DIR__ . '/../../..' . '/lib/private/AppFramework/OCS/BaseResponse.php', 'OC\\AppFramework\\OCS\\V1Response' => __DIR__ . '/../../..' . '/lib/private/AppFramework/OCS/V1Response.php', 'OC\\AppFramework\\OCS\\V2Response' => __DIR__ . '/../../..' . '/lib/private/AppFramework/OCS/V2Response.php', + 'OC\\AppFramework\\ORM\\EntityInfo' => __DIR__ . '/../../..' . '/lib/private/AppFramework/ORM/EntityInfo.php', + 'OC\\AppFramework\\ORM\\EntityManager' => __DIR__ . '/../../..' . '/lib/private/AppFramework/ORM/EntityManager.php', + 'OC\\AppFramework\\ORM\\PropertyAttributes' => __DIR__ . '/../../..' . '/lib/private/AppFramework/ORM/PropertyAttributes.php', 'OC\\AppFramework\\Routing\\RouteActionHandler' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Routing/RouteActionHandler.php', 'OC\\AppFramework\\Routing\\RouteParser' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Routing/RouteParser.php', 'OC\\AppFramework\\ScopedPsrLogger' => __DIR__ . '/../../..' . '/lib/private/AppFramework/ScopedPsrLogger.php', diff --git a/lib/private/AppFramework/ORM/EntityInfo.php b/lib/private/AppFramework/ORM/EntityInfo.php new file mode 100644 index 0000000000000..d3f66966025c9 --- /dev/null +++ b/lib/private/AppFramework/ORM/EntityInfo.php @@ -0,0 +1,86 @@ + */ + public array $mappingColumnToTypes = []; + + /** @var array */ + public array $mappingColumnToProperty = []; + + /** @var array */ + public array $mappingPropertyToColumn = []; + + /** @var \ReflectionClass */ + public readonly \ReflectionClass $reflection; + + public ?\ReflectionProperty $idProperty = null; + + /** + * @var list $propertiesAttributes + */ + public array $propertiesAttributes = []; + + public function __construct( + public readonly string $entityClass, + ) { + $this->reflection = new \ReflectionClass($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) { + $attributes = $property->getAttributes(); + $propertyAttributes = new PropertyAttributes($property); + + foreach ($attributes as $attribute) { + $instance = $attribute->newInstance(); + if ($instance instanceof Column) { + $propertyAttributes->column = $instance; + $this->mappingColumnToTypes[$instance->name] = $instance->type; + $this->mappingColumnToProperty[$instance->name] = $property->getName(); + $this->mappingPropertyToColumn[$property->getName()] = $instance->name; + } elseif ($instance instanceof Id) { + $propertyAttributes->id = $instance; + $this->idProperty = $property; + } elseif ($instance instanceof OneToOne) { + $propertyAttributes->oneToOne = $instance; + } elseif ($instance instanceof JoinColumn) { + $propertyAttributes->joinColumn = $instance; + } + } + + if ($propertyAttributes->id !== null && $propertyAttributes->column === null) { + throw new \RuntimeException($this->entityClass . ' has a Id attribute on ' . $property->getName() . ' but not the corresponding required Column attribute.'); + } + + $this->propertiesAttributes[] = $propertyAttributes; + } + + if ($this->idProperty === null) { + throw new \RuntimeException($this->entityClass . ' does not have a primary key. This is not supported for repositories backed tables.'); + } + } +} diff --git a/lib/private/AppFramework/ORM/EntityManager.php b/lib/private/AppFramework/ORM/EntityManager.php new file mode 100644 index 0000000000000..620833fc59be7 --- /dev/null +++ b/lib/private/AppFramework/ORM/EntityManager.php @@ -0,0 +1,292 @@ +, EntityInfo> $entitiesInfo */ + private array $entitiesInfo = []; + + /** + * @template T + * @param class-string $entityClass + * @return EntityInfo + */ + public function getEntityInfo(string $entityClass): EntityInfo { + if (!isset($this->entitiesInfo[$entityClass])) { + $this->entitiesInfo[$entityClass] = new EntityInfo($entityClass); + } + /** @var EntityInfo $entityInfo */ + $entityInfo = $this->entitiesInfo[$entityClass]; + return $entityInfo; + } + + /** + * @template T + * @psalm-param T $entity + * @return T + */ + public function insert(object $entity): object { + $entityClass = get_class($entity); + + $entityInfo = $this->getEntityInfo($entityClass); + $insert = $this->connection->getQueryBuilder(); + + $isSnowflake = false; + $primaryProperty = null; + $values = []; + + foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { + $property = $propertyAttributes->property; + if ($propertyAttributes->id !== null && $propertyAttributes->column !== null) { + $primaryProperty = $property; + $generatorClass = $propertyAttributes->id->generatorClass; + 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[$propertyAttributes->column->name] = $generator->nextId(); + $property->setValue($entity, $insert->createNamedParameter($values[$propertyAttributes['column']->name->name])); + } + } + continue; + } + + if ($propertyAttributes->oneToOne !== null && $propertyAttributes->joinColumn !== null) { + $oneToOne = $propertyAttributes->oneToOne; + if ($oneToOne->invertedBy === null) { + continue; + } + + $joinColumn = $propertyAttributes->joinColumn; + /** @var object $object */ + $targetEntity = $property->getValue($entity); + $targetEntityInfo = $this->getEntityInfo($oneToOne->targetEntity); + if ($targetEntity === null) { + $values[$joinColumn->name] = $insert->createNamedParameter(null); + } else { + $values[$joinColumn->name] = $insert->createNamedParameter($targetEntityInfo->idProperty->getValue($targetEntity)); + } + + continue; + } + + if ($propertyAttributes->column !== null) { + $type = $this->getParameterType($propertyAttributes->column->type, false); + $values[$propertyAttributes->column->name] = $insert->createNamedParameter($property->getValue($entity), $type); + } + } + + $insert->insert($entityInfo->tableName) + ->values($values) + ->executeStatement(); + + if (!$isSnowflake) { + $primaryProperty->setValue($entity, $insert->getLastInsertId()); + } + return $entity; + } + + /** + * @template T + * @psalm-param T $entity + * @return T + */ + public function update(object $entity): object { + $entityClass = get_class($entity); + $entityInfo = $this->getEntityInfo($entityClass); + + $update = $this->connection->getQueryBuilder(); + $update->update($entityInfo->tableName); + + foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { + $property = $propertyAttributes->property; + $value = $property->getValue($entity); + + if ($propertyAttributes->id !== null && $propertyAttributes->column !== null) { + if ($value === null) { + throw new \LogicException('Trying to update an entity with no primary key set.'); + } + + $update->andWhere($update->expr()->eq($entityInfo->mappingPropertyToColumn[$entityInfo->idProperty->getName()], $update->createNamedParameter($property->getValue($entity)))); + // don't update the id + continue; + } + + if ($propertyAttributes->oneToOne !== null && $propertyAttributes->joinColumn !== null) { + $oneToOne = $propertyAttributes->oneToOne; + if ($oneToOne->invertedBy === null) { + continue; + } + + /** @var JoinColumn $joinColumn */ + $joinColumn = $propertyAttributes->joinColumn; + /** @var object $object */ + $targetEntity = $value; + $targetEntityInfo = $this->getEntityInfo($oneToOne->targetEntity); + if ($targetEntity === null) { + $update->set($joinColumn->name, $update->createNamedParameter(null)); + } else { + $update->set($joinColumn->name, $update->createNamedParameter($targetEntityInfo->idProperty->getValue($targetEntity))); + } + + continue; + } + + if ($propertyAttributes->column !== null) { + $type = $this->getParameterType($propertyAttributes->column->type, false); + $update->set($propertyAttributes->column->name, $update->createNamedParameter($value, $type)); + } + } + + $update->executeStatement(); + return $entity; + } + + /** + * @template T + * @psalm-param T $entity + */ + public function delete(object $entity): void { + $entityClass = get_class($entity); + $entityInfo = $this->getEntityInfo($entityClass); + + $delete = $this->connection->getQueryBuilder(); + $delete->delete($entityInfo->tableName); + + $foundId = false; + foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { + if ($propertyAttributes->id !== null && $propertyAttributes->column !== null) { + $property = $propertyAttributes->property; + $value = $property->getValue($entity); + + $delete->andWhere($delete->expr()->eq($propertyAttributes->column->name, $delete->createNamedParameter($value))); + $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_* + */ + public 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, + }; + } + + /** + * @internal Only for unit tests. + * + * @param class-string $entityClass + */ + public function createTable(string $entityClass, Schema $schema, string $prefix): void { + $entityInfo = $this->getEntityInfo($entityClass); + + $table = $schema->createTable($prefix . $entityInfo->tableName); + + foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { + $this->createProperty($propertyAttributes, $table); + + $this->createOneToOne($propertyAttributes, $table); + } + } + + private function createProperty(PropertyAttributes $attributes, Table $table): void { + if ($attributes->column === null) { + return; + } + /** @var Column $columnAttribute */ + $columnAttribute = $attributes->column; + $options = [ + 'notnull' => !$columnAttribute->nullable, + ]; + if ($columnAttribute->length !== null) { + $options['length'] = $columnAttribute->length; + } + if ($columnAttribute->default !== null) { + $options['default'] = $columnAttribute->default; + } + + if ($attributes->id !== null && $attributes->id->generatorClass === null) { + $options['autoincrement'] = true; + } + + $table->addColumn($columnAttribute->name, $columnAttribute->type, $options); + + if ($attributes->id !== null) { + $table->setPrimaryKey([$columnAttribute->name]); + } + } + + private function createOneToOne(PropertyAttributes $attributes, Table $table): void { + if ($attributes->joinColumn === null || $attributes->oneToOne === null) { + return; + } + + if ($attributes->oneToOne->invertedBy !== null) { + $table->addColumn($attributes->joinColumn->name, Types::BIGINT, [ + 'notnull' => !$attributes->joinColumn->nullable, + ]); + + $foreignEntityInfo = $this->getEntityInfo($attributes->oneToOne->targetEntity); + + $options = []; + if ($attributes->joinColumn->onDelete === 'CASCADE') { + $options['onDelete'] = 'CASCADE'; + } + + $table->addForeignKeyConstraint($foreignEntityInfo->tableName, [$attributes->joinColumn->name], [$attributes->joinColumn->referencedColumnName], $options); + } + } +} diff --git a/lib/private/AppFramework/ORM/PropertyAttributes.php b/lib/private/AppFramework/ORM/PropertyAttributes.php new file mode 100644 index 0000000000000..159f1ea6c9b4f --- /dev/null +++ b/lib/private/AppFramework/ORM/PropertyAttributes.php @@ -0,0 +1,25 @@ + */ class TagMapper extends Repository { - /** - * Constructor. - * - * @param IDBConnection $db Instance of the Db abstraction layer. - */ public function __construct(IDBConnection $db) { parent::__construct($db, Tag::class); } diff --git a/lib/public/AppFramework/Db/Attribute/Table.php b/lib/public/AppFramework/Db/Attribute/Table.php deleted file mode 100644 index 72ac0c30f322d..0000000000000 --- a/lib/public/AppFramework/Db/Attribute/Table.php +++ /dev/null @@ -1,37 +0,0 @@ - */ - private array $_mappingColumnToTypes = []; - - /** @var array */ - private array $_mappingColumnToProperty = []; - - /** @var array */ - private array $_mappingPropertyToColumn = []; - - /** @var \ReflectionClass */ - private \ReflectionClass $reflection; - - private string $idProperty; - - 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 (empty($entities)) { - throw new \InvalidArgumentException('The given entity is missing a required #[Entity] attribute'); - } - - $tables = $this->reflection->getAttributes(Table::class, \ReflectionAttribute::IS_INSTANCEOF); - if (empty($tables)) { - throw new \InvalidArgumentException('The given entityClass is missing a required #[Table] attribute'); - } - $this->tableName = $tables[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 - * @since 30.0.0 - */ - 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 - * @since 33.0.0 - */ - public function findEntities(IQueryBuilder $query): array { - $result = $query->executeQuery(); - try { - $entities = []; - while ($row = $result->fetch()) { - $entities[] = $this->mapRowToEntity($row); - } - return $entities; - } finally { - $result->closeCursor(); - } - } - - 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; - } - - /** - * @psalm-param T $entity - * @return T - */ - public function insert(object $entity): object { - $insert = $this->connection->getQueryBuilder(); - - $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) { - $generatorClass = $ids[0]->newInstance()->generatorClass; - $generator = Server::get($generatorClass); - if ($generator instanceof IGenerator) { - $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(); - 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); - } - - public function deleteBy(array $criteria, ?int $limit = null): void { - $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)); - $qb->andWhere($qb->expr()->eq($column, $qb->createNamedParameter($value, $type))); - } - - if ($limit !== null) { - $qb->setMaxResults($limit); - } - - $qb->executeStatement(); - } - - /** - * Finds a single entity by a set of criteria. - * - * @param array $criteria - * @param array|null $orderBy - * @return T|null - * @since 33.0.0 - */ - 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; - } - } - - 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)); - $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/public/AppFramework/Db/Attribute/Column.php b/lib/public/AppFramework/ORM/Attribute/Column.php similarity index 55% rename from lib/public/AppFramework/Db/Attribute/Column.php rename to lib/public/AppFramework/ORM/Attribute/Column.php index 752ad690363da..26a13a17ef2bb 100644 --- a/lib/public/AppFramework/Db/Attribute/Column.php +++ b/lib/public/AppFramework/ORM/Attribute/Column.php @@ -8,33 +8,38 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -namespace OCP\AppFramework\Db\Attribute; +namespace OCP\AppFramework\ORM\Attribute; 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 = ''; * } * ``` * - * @since 33.0.0 + * @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( + /** @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, - public int|null $length = null, + /** @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/public/AppFramework/Db/Attribute/Entity.php b/lib/public/AppFramework/ORM/Attribute/Entity.php similarity index 71% rename from lib/public/AppFramework/Db/Attribute/Entity.php rename to lib/public/AppFramework/ORM/Attribute/Entity.php index c7385a34a5909..04229ec82e822 100644 --- a/lib/public/AppFramework/Db/Attribute/Entity.php +++ b/lib/public/AppFramework/ORM/Attribute/Entity.php @@ -8,7 +8,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -namespace OCP\AppFramework\Db\Attribute; +namespace OCP\AppFramework\ORM\Attribute; use Attribute; use OCP\AppFramework\Attribute\Consumable; @@ -17,17 +17,21 @@ * Attribute for marking a class as an entity mapped to some database table. * * ```php - * #[Entity] - * #[Table(name: 'my_entity'] + * #[Entity(name: 'my_entity')] * final class MyEntity { * #[Column(name: 'my_column', type: Types::String, default: '')] * public string $myColumn = ''; * } * ``` * - * @since 33.0.0 + * @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( + /** @param non-empty-string $name */ + public string $name, + ) { + } } diff --git a/lib/public/AppFramework/Db/Attribute/Id.php b/lib/public/AppFramework/ORM/Attribute/Id.php similarity index 58% rename from lib/public/AppFramework/Db/Attribute/Id.php rename to lib/public/AppFramework/ORM/Attribute/Id.php index 15c2070ff1a43..3bfb63efa2d47 100644 --- a/lib/public/AppFramework/Db/Attribute/Id.php +++ b/lib/public/AppFramework/ORM/Attribute/Id.php @@ -8,32 +8,32 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -namespace OCP\AppFramework\Db\Attribute; +namespace OCP\AppFramework\ORM\Attribute; use Attribute; use OCP\AppFramework\Attribute\Consumable; +use OCP\Snowflake\ISnowflakeGenerator; /** * Attribute for marking a column as a primary id. * * ```php - * #[Entity] - * #[Table(name: 'my_entity'] + * #[Entity(name: 'my_entity'] * final class MyEntity { - * #[Id(generatorClass: IGenerator::class)] + * #[Id(generatorClass: ISnowflakeGenerator::class)] * #[Column(name: 'id', type: Types::BIGINT)] - * public string $id = ''; + * public ?string $id = null; * } * ``` * - * @since 33.0.0 + * @since 35.0.0 */ #[Attribute(Attribute::TARGET_PROPERTY)] -#[Consumable(since: '33.0.0')] +#[Consumable(since: '35.0.0')] final readonly class Id { public function __construct( - /** @params string-class $generatorClass */ - public string $generatorClass, + /** @param class-string $generatorClass */ + public ?string $generatorClass = null, ) { } } diff --git a/lib/public/AppFramework/ORM/Attribute/JoinColumn.php b/lib/public/AppFramework/ORM/Attribute/JoinColumn.php new file mode 100644 index 0000000000000..f50995836cd50 --- /dev/null +++ b/lib/public/AppFramework/ORM/Attribute/JoinColumn.php @@ -0,0 +1,44 @@ + $targetEntity */ + public string $targetEntity, + /** @param ?non-empty-string $mappedBy */ + public ?string $mappedBy = null, + /** @param ?non-empty-string $invertedBy */ + public ?string $invertedBy = null, + ) { + } +} diff --git a/lib/public/AppFramework/ORM/Repository.php b/lib/public/AppFramework/ORM/Repository.php new file mode 100644 index 0000000000000..4dd55a787b4cb --- /dev/null +++ b/lib/public/AppFramework/ORM/Repository.php @@ -0,0 +1,319 @@ + $entityClass + * @throws \ReflectionException + */ + public function __construct( + protected readonly IDBConnection $connection, + protected readonly string $entityClass, + ) { + $this->entityManager = Server::get(EntityManager::class); + } + + /** + * Runs a sql query and yields each resulting entity to obtain database entries in a memory-efficient way + * + * @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 + * + * @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 { + $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); + + $entity = new $this->entityClass(); + foreach ($row as $column => $value) { + $property = $entityInfo->mappingColumnToProperty[$column]; + $type = $entityInfo->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 === $entityInfo->idProperty->getName()) { + /** @var list<\ReflectionAttribute> $ids */ + $ids = $entityInfo->idProperty->getAttributes(Id::class, \ReflectionAttribute::IS_INSTANCEOF); + $id = array_shift($ids); + if ($id->newInstance()->generatorClass !== null) { + $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((string)$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 { + return $this->entityManager->insert($entity); + } + + /** + * @psalm-param T $entity + * @return T + */ + public function update(object $entity): object { + return $this->entityManager->update($entity); + } + + /** + * @psalm-param T $entity + */ + public function delete(object $entity): void { + $this->entityManager->delete($entity); + } + + /** + * 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 + */ + 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 { + $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); + + $qb = $this->connection->getQueryBuilder(); + $qb->delete($entityInfo->tableName); + + foreach ($criteria as $property => $value) { + $column = $entityInfo->mappingPropertyToColumn[$property]; + $type = $this->entityManager->getParameterType($entityInfo->mappingColumnToTypes[$column], is_array($value)); + $type = $this->entityManager->getParameterType($entityInfo->mappingColumnToTypes[$column], is_array($value)); + if ($value === null) { + $qb->andWhere($qb->expr()->isNull($column)); + } elseif (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 + * @throws DoesNotExistException + */ + public function findOneBy(array $criteria, array $orderBy = []): object { + $qb = $this->getSelectQueryBuilder($criteria, $orderBy); + + $qb->setMaxResults(1); + + return $this->findEntity($qb); + } + + /** + * @param array> $criteria + * @param array|null $orderBy + */ + private function getSelectQueryBuilder(array $criteria, array $orderBy = []): IQueryBuilder { + $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); + + $qb = $this->connection->getQueryBuilder(); + $qb->select('*') + ->from($entityInfo->tableName); + + foreach ($criteria as $property => $value) { + $column = $entityInfo->mappingPropertyToColumn[$property]; + $type = $this->entityManager->getParameterType($entityInfo->mappingColumnToTypes[$column], is_array($value)); + if ($value === null) { + $qb->andWhere($qb->expr()->isNull($column)); + } elseif (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 + * + * @psalm-return T the entity + * @throws Exception + * @throws MultipleObjectsReturnedException if more than one item exist + * @throws DoesNotExistException if the item does not exist + */ + 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); + } + + /** + * @return Generator + * @throws Exception + */ + public function yieldAll(): \Generator { + $qb = $this->connection->getQueryBuilder(); + $qb->select('*') + ->from($this->getTableName()); + + return $this->yieldEntities($qb); + } + + public function getTableName(): string { + $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); + return $entityInfo->tableName; + } +} From e68a1a4595f8481e36d1708b8c907125198d4263 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 21 Jul 2026 01:58:00 +0200 Subject: [PATCH 03/10] fix(ORM): Add missing tests Signed-off-by: Carl Schwan --- .../AppFramework/ORM/EntityManager.php | 24 +- tests/lib/AppFramework/ORM/RepositoryTest.php | 264 ++++++++++++++++++ 2 files changed, 279 insertions(+), 9 deletions(-) create mode 100644 tests/lib/AppFramework/ORM/RepositoryTest.php diff --git a/lib/private/AppFramework/ORM/EntityManager.php b/lib/private/AppFramework/ORM/EntityManager.php index 620833fc59be7..bd2e684f35eb1 100644 --- a/lib/private/AppFramework/ORM/EntityManager.php +++ b/lib/private/AppFramework/ORM/EntityManager.php @@ -7,8 +7,8 @@ namespace OC\AppFramework\ORM; -use Doctrine\DBAL\Schema\Schema; use Doctrine\DBAL\Schema\Table; +use OC\DB\SchemaWrapper; use OCP\AppFramework\ORM\Attribute\Column; use OCP\AppFramework\ORM\Attribute\JoinColumn; use OCP\AppFramework\ORM\Attribute\OneToOne; @@ -47,9 +47,7 @@ public function getEntityInfo(string $entityClass): EntityInfo { * @return T */ public function insert(object $entity): object { - $entityClass = get_class($entity); - - $entityInfo = $this->getEntityInfo($entityClass); + $entityInfo = $this->getEntityInfo($entity::class); $insert = $this->connection->getQueryBuilder(); $isSnowflake = false; @@ -230,16 +228,23 @@ public function getParameterType(string $type, bool $isArray): string|int { * * @param class-string $entityClass */ - public function createTable(string $entityClass, Schema $schema, string $prefix): void { + public function createTable(string $entityClass, SchemaWrapper $schema): void { $entityInfo = $this->getEntityInfo($entityClass); - $table = $schema->createTable($prefix . $entityInfo->tableName); + $table = $schema->createTable($entityInfo->tableName); foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { $this->createProperty($propertyAttributes, $table); - $this->createOneToOne($propertyAttributes, $table); + $this->createOneToOne($propertyAttributes, $table, $schema); } + + $schema->getWrappedSchema()->toSql($this->connection->getDatabasePlatform()); + } + + public function dropTable(string $entityClass, string $prefix): void { + $entityInfo = $this->getEntityInfo($entityClass); + $this->connection->dropTable($prefix . $entityInfo->tableName); } private function createProperty(PropertyAttributes $attributes, Table $table): void { @@ -269,7 +274,7 @@ private function createProperty(PropertyAttributes $attributes, Table $table): v } } - private function createOneToOne(PropertyAttributes $attributes, Table $table): void { + private function createOneToOne(PropertyAttributes $attributes, Table $table, SchemaWrapper $schema): void { if ($attributes->joinColumn === null || $attributes->oneToOne === null) { return; } @@ -286,7 +291,8 @@ private function createOneToOne(PropertyAttributes $attributes, Table $table): v $options['onDelete'] = 'CASCADE'; } - $table->addForeignKeyConstraint($foreignEntityInfo->tableName, [$attributes->joinColumn->name], [$attributes->joinColumn->referencedColumnName], $options); + $foreignTableName = $schema->getTable($foreignEntityInfo->tableName)->getName(); + $table->addForeignKeyConstraint($foreignTableName, [$attributes->joinColumn->name], [$attributes->joinColumn->referencedColumnName], $options); } } } diff --git a/tests/lib/AppFramework/ORM/RepositoryTest.php b/tests/lib/AppFramework/ORM/RepositoryTest.php new file mode 100644 index 0000000000000..533ac637a9f48 --- /dev/null +++ b/tests/lib/AppFramework/ORM/RepositoryTest.php @@ -0,0 +1,264 @@ + */ +class NoPrimaryKeyRepository extends Repository { + public function __construct( + IDBConnection $connection, + ) { + parent::__construct($connection, NoPrimaryKey::class); + } +} + +#[Entity(name: 'repository_test_test2')] +class PrimaryKey { + #[Id] + #[Column(name: 'id', type: Types::INTEGER, nullable: false)] + public ?int $id = null; + + #[Column(name: 'name', type: Types::STRING, nullable: true)] + public ?string $name = null; + + #[Column(name: 'notNullable', type: Types::STRING, nullable: false)] + public string $notNullable; + + #[Column(name: 'integer_val', type: Types::INTEGER, nullable: false)] + public int $integer; + + #[Column(name: 'bigInt_val', type: Types::BIGINT, nullable: false)] + public int $bigInt; + + #[Column(name: 'float_val', type: Types::FLOAT, nullable: false)] + public float $float; + + #[Column(name: 'date_val', type: Types::DATETIME, nullable: false)] + public \DateTime $date; +} + +/** @template-extends Repository */ +class PrimaryKeyRepository extends Repository { + public function __construct( + IDBConnection $connection, + ) { + parent::__construct($connection, PrimaryKey::class); + } +} + +#[Entity(name: 'repository_customer')] + final class Customer { + #[Id] + #[Column(name: 'id', type: Types::BIGINT)] + public ?int $id = null; + + #[OneToOne(targetEntity: Cart::class, mappedBy: 'customer')] + #[JoinColumn(name: 'cart_id', referencedColumnName: 'id', onDelete: 'CASCADE')] + public Cart|null $cart = null; + + #[Column(name: 'name', type: Types::STRING, nullable: false)] + public string $name; +} + +/** @template-extends Repository */ +class CustomerRepository extends Repository { + public function __construct( + IDBConnection $connection, + ) { + parent::__construct($connection, Customer::class); + } +} + + #[Entity(name: 'repository_cart')] + final class Cart { + #[Id] + #[Column(name: 'id', type: Types::BIGINT)] + public ?int $id = null; + + #[OneToOne(targetEntity: Customer::class, invertedBy: 'cart')] + #[JoinColumn(name: 'customer_id', referencedColumnName: 'id')] + public Customer|null $customer; +} + +/** @template-extends Repository */ +class CartRepository extends Repository { + public function __construct( + IDBConnection $connection, + ) { + parent::__construct($connection, Cart::class); + } +} + +class RepositoryTest extends TestCase { + /** @var array> */ + public static array $entitiesClasses = [ + NoPrimaryKey::class => NoPrimaryKeyRepository::class, + PrimaryKey::class => PrimaryKeyRepository::class, + Customer::class => CustomerRepository::class, + Cart::class => CartRepository::class, + ]; + + public static function setUpBeforeClass(): void { + $schema = new SchemaWrapper(Server::get(Connection::class)); + $entityManager = Server::get(EntityManager::class); + foreach (static::$entitiesClasses as $entityClass => $repositoryClass) { + try { + $entityManager->createTable($entityClass, $schema); + } catch (\RuntimeException) { + self::assertEquals(NoPrimaryKey::class, $entityClass); + } + } + Server::get(Connection::class)->migrateToSchema($schema->getWrappedSchema()); + } + + public static function tearDownAfterClass(): void { + $entityManager = Server::get(EntityManager::class); + $prefix = Server::get(IConfig::class)->getSystemValueString('dbtableprefix', 'oc_'); + foreach (static::$entitiesClasses as $entityClass => $repositoryClass) { + try { + $entityManager->dropTable($entityClass, $prefix); + } catch (\RuntimeException) { + self::assertEquals(NoPrimaryKey::class, $entityClass); + } + } + } + + public function testMissingPrimaryKey(): void { + $this->expectException(\RuntimeException::class); + + $repo = Server::get(NoPrimaryKeyRepository::class); + $_ = $repo->getTableName(); + } + + public function testPrimaryKey(): void { + $repo = Server::get(PrimaryKeyRepository::class); + $this->assertEquals('repository_test_test2', $repo->getTableName()); + $entity = new PrimaryKey(); + $entity->name = null; + $entity->notNullable = 'ab'; + $entity->integer = 5; + $entity->bigInt = 5; + $entity->float = 3.0; + $entity->date = new \DateTime('now'); + + $repo->insert($entity); + $this->assertNotNull($entity->id); + + // Confirm only one entry in the DB + $entities = iterator_to_array($repo->yieldAll()); + $this->assertCount(1, $entities); + $savedEntity = $entities[0]; + $this->assertEquals($entity->name, $savedEntity->name); + $this->assertEquals($entity->notNullable, $savedEntity->notNullable); + $this->assertEquals($entity->integer, $savedEntity->integer); + $this->assertEquals($entity->bigInt, $savedEntity->bigInt); + $this->assertEquals($entity->float, $savedEntity->float); + $this->assertEquals($entity->date->getTimestamp(), $savedEntity->date->getTimestamp()); + + // Search by primary key + $savedEntity = $repo->findOneBy(['id' => $entity->id]); + $this->assertEquals($entity->name, $savedEntity->name); + $this->assertEquals($entity->notNullable, $savedEntity->notNullable); + $this->assertEquals($entity->integer, $savedEntity->integer); + $this->assertEquals($entity->bigInt, $savedEntity->bigInt); + $this->assertEquals($entity->float, $savedEntity->float); + $this->assertEquals($entity->date->getTimestamp(), $savedEntity->date->getTimestamp()); + + // Search by null + $savedEntity = $repo->findOneBy(['name' => $entity->name]); + $this->assertEquals($entity->name, $savedEntity->name); + $this->assertEquals($entity->notNullable, $savedEntity->notNullable); + $this->assertEquals($entity->integer, $savedEntity->integer); + $this->assertEquals($entity->bigInt, $savedEntity->bigInt); + $this->assertEquals($entity->float, $savedEntity->float); + $this->assertEquals($entity->date->getTimestamp(), $savedEntity->date->getTimestamp()); + + // Search by other fields + $savedEntity = $repo->findOneBy([ + 'notNullable' => $entity->notNullable, + 'integer' => $entity->integer, + 'bigInt' => $entity->bigInt, + ]); + /** @psalm-assert PrimaryKey $savedEntity */ + $this->assertEquals($entity->name, $savedEntity->name); + $this->assertEquals($entity->notNullable, $savedEntity->notNullable); + $this->assertEquals($entity->integer, $savedEntity->integer); + $this->assertEquals($entity->bigInt, $savedEntity->bigInt); + $this->assertEquals($entity->float, $savedEntity->float); + $this->assertEquals($entity->date->getTimestamp(), $savedEntity->date->getTimestamp()); + + // update + $entity->name = 'not null anymore'; + $repo->update($entity); + + $savedEntity = $repo->findOneBy(['id' => $entity->id]); + $this->assertEquals($entity->name, $savedEntity->name); + + // delete + $repo->delete($savedEntity); + } + + public function testDeleteBy(): void { + $repo = Server::get(PrimaryKeyRepository::class); + + $entity = new PrimaryKey(); + $entity->name = null; + $entity->notNullable = 'ab'; + $entity->integer = 5; + $entity->bigInt = 5; + $entity->float = 3.0; + $entity->date = new \DateTime('now'); + + $repo->insert($entity); + $this->assertNotNull($entity->id); + + // Confirm only one entry in the DB + $entities = iterator_to_array($repo->yieldAll()); + $this->assertCount(1, $entities); + + $repo->deleteBy(['id' => $entity->id]); + + $entities = iterator_to_array($repo->yieldAll()); + $this->assertCount(0, $entities); + } + + public function testOneToOne(): void { + $cartRepo = Server::get(CartRepository::class); + $customerRepo = Server::get(CustomerRepository::class); + + $customer = new Customer(); + $customer->name = 'foo'; + $customerRepo->insert($customer); + $this->assertNotNull($customer->id); + + $cart = new Cart(); + $cart->customer = $customer; + $cartRepo->insert($cart); + $this->assertNotNull($cart->id); + } +} From d5abf1d7366af1bb3dd962e49c0b5f69eb4d94bb Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 21 Jul 2026 12:21:47 +0200 Subject: [PATCH 04/10] fix(orm): Add error handling Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: Carl Schwan --- lib/private/AppFramework/ORM/EntityInfo.php | 41 ++- .../AppFramework/ORM/EntityManager.php | 15 +- lib/public/AppFramework/ORM/Repository.php | 321 ++++++++++++------ tests/lib/AppFramework/ORM/RepositoryTest.php | 264 +++++++++++++- 4 files changed, 541 insertions(+), 100 deletions(-) diff --git a/lib/private/AppFramework/ORM/EntityInfo.php b/lib/private/AppFramework/ORM/EntityInfo.php index d3f66966025c9..25e7ab6ae3217 100644 --- a/lib/private/AppFramework/ORM/EntityInfo.php +++ b/lib/private/AppFramework/ORM/EntityInfo.php @@ -15,7 +15,6 @@ /** * @template T - * @internal */ class EntityInfo { public readonly string $tableName; @@ -76,6 +75,17 @@ public function __construct( throw new \RuntimeException($this->entityClass . ' has a Id attribute on ' . $property->getName() . ' but not the corresponding required Column attribute.'); } + if ($propertyAttributes->oneToOne !== null + && $propertyAttributes->oneToOne->mappedBy !== null + && $propertyAttributes->joinColumn !== null + && $propertyAttributes->joinColumn->onDelete !== null) { + throw new \RuntimeException($this->entityClass . '::' . $property->getName() . ' sets JoinColumn::$onDelete on the mappedBy (inverse) side of a OneToOne relation, where it has no effect. Set it on the owning (invertedBy) side instead.'); + } + + if ($propertyAttributes->oneToOne !== null && $propertyAttributes->oneToOne->mappedBy !== null) { + $this->validateMappedBy($property, $propertyAttributes->oneToOne); + } + $this->propertiesAttributes[] = $propertyAttributes; } @@ -83,4 +93,33 @@ public function __construct( throw new \RuntimeException($this->entityClass . ' does not have a primary key. This is not supported for repositories backed tables.'); } } + + /** + * Checks that mappedBy actually points at the owning side of the relation, so a typo + * fails loudly at construction time instead of silently resolving to null forever. + */ + private function validateMappedBy(\ReflectionProperty $property, OneToOne $oneToOne): void { + $prefix = $this->entityClass . '::' . $property->getName() . ' has mappedBy: \'' . $oneToOne->mappedBy . '\', but '; + $targetReflection = new \ReflectionClass($oneToOne->targetEntity); + + if (!$targetReflection->hasProperty($oneToOne->mappedBy)) { + throw new \RuntimeException($prefix . $oneToOne->targetEntity . ' has no property named \'' . $oneToOne->mappedBy . '\'.'); + } + + $targetProperty = $targetReflection->getProperty($oneToOne->mappedBy); + $targetOneToOneAttributes = $targetProperty->getAttributes(OneToOne::class, \ReflectionAttribute::IS_INSTANCEOF); + $targetOneToOne = $targetOneToOneAttributes === [] ? null : $targetOneToOneAttributes[0]->newInstance(); + + if ($targetOneToOne === null || $targetOneToOne->invertedBy === null) { + throw new \RuntimeException($prefix . $oneToOne->targetEntity . '::' . $oneToOne->mappedBy . ' is not the owning (invertedBy) side of a OneToOne relation.'); + } + + if ($targetOneToOne->invertedBy !== $property->getName()) { + throw new \RuntimeException($prefix . $oneToOne->targetEntity . '::' . $oneToOne->mappedBy . '\'s invertedBy points at \'' . $targetOneToOne->invertedBy . '\' instead.'); + } + + if ($targetProperty->getAttributes(JoinColumn::class, \ReflectionAttribute::IS_INSTANCEOF) === []) { + throw new \RuntimeException($prefix . $oneToOne->targetEntity . '::' . $oneToOne->mappedBy . ' has no JoinColumn attribute.'); + } + } } diff --git a/lib/private/AppFramework/ORM/EntityManager.php b/lib/private/AppFramework/ORM/EntityManager.php index bd2e684f35eb1..08d6ac5493410 100644 --- a/lib/private/AppFramework/ORM/EntityManager.php +++ b/lib/private/AppFramework/ORM/EntityManager.php @@ -12,6 +12,7 @@ use OCP\AppFramework\ORM\Attribute\Column; use OCP\AppFramework\ORM\Attribute\JoinColumn; use OCP\AppFramework\ORM\Attribute\OneToOne; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\DB\Types; use OCP\IDBConnection; @@ -75,6 +76,9 @@ public function insert(object $entity): object { if ($propertyAttributes->oneToOne !== null && $propertyAttributes->joinColumn !== null) { $oneToOne = $propertyAttributes->oneToOne; if ($oneToOne->invertedBy === null) { + if ($property->getValue($entity) !== null) { + throw new \LogicException($entity::class . '::' . $property->getName() . ' is the mappedBy (inverse) side of a OneToOne relation and cannot be persisted directly; set it from the owning (invertedBy) side instead.'); + } continue; } @@ -189,7 +193,14 @@ public function delete(object $entity): void { throw new \LogicException('The given entity is missing a required #[Id] attribute on one of its properties.'); } - $delete->executeStatement(); + try { + $delete->executeStatement(); + } catch (Exception $e) { + if ($e->getReason() === Exception::REASON_FOREIGN_KEY_VIOLATION) { + throw new \LogicException($entityClass . ' cannot be deleted: another entity still references it via a OneToOne relation. Delete the related entity first, or set onDelete: \'CASCADE\' on the owning JoinColumn.', 0, $e); + } + throw $e; + } } /** @@ -284,6 +295,8 @@ private function createOneToOne(PropertyAttributes $attributes, Table $table, Sc 'notnull' => !$attributes->joinColumn->nullable, ]); + $table->addUniqueIndex([$attributes->joinColumn->name]); + $foreignEntityInfo = $this->getEntityInfo($attributes->oneToOne->targetEntity); $options = []; diff --git a/lib/public/AppFramework/ORM/Repository.php b/lib/public/AppFramework/ORM/Repository.php index 4dd55a787b4cb..f84299d43aa2c 100644 --- a/lib/public/AppFramework/ORM/Repository.php +++ b/lib/public/AppFramework/ORM/Repository.php @@ -2,17 +2,16 @@ declare(strict_types=1); -/** - * SPDX-FileCopyrightText: 2025 Nextcloud GmbH - * SPDX-FileContributor: Carl Schwan - * SPDX-License-Identifier: AGPL-3.0-or-later - */ +// SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors +// SPDX-License-Identifier: AGPL-3.0-or-later namespace OCP\AppFramework\ORM; -use Generator; +use OC\AppFramework\ORM\EntityInfo; use OC\AppFramework\ORM\EntityManager; +use OC\AppFramework\ORM\PropertyAttributes; use OCP\AppFramework\ORM\Attribute\Id; +use OCP\AppFramework\ORM\Attribute\OneToOne; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\DB\Exception; @@ -39,46 +38,21 @@ public function __construct( $this->entityManager = Server::get(EntityManager::class); } - /** - * Runs a sql query and yields each resulting entity to obtain database entries in a memory-efficient way - * - * @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 - * - * @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() . '"; '; } /** + * Builds an entity from a flat row of its own scalar columns. OneToOne relations are + * always left null here; resolving them is mapJoinedRowToEntity()'s job. + * + * @param class-string $entityClass * @param array $row - * @return T */ - private function mapRowToEntity(mixed $row): object { - $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); + private function hydrateRow(string $entityClass, mixed $row): object { + $entityInfo = $this->entityManager->getEntityInfo($entityClass); - $entity = new $this->entityClass(); + $entity = new $entityClass(); foreach ($row as $column => $value) { $property = $entityInfo->mappingColumnToProperty[$column]; $type = $entityInfo->mappingColumnToTypes[$column]; @@ -134,13 +108,207 @@ private function mapRowToEntity(mixed $row): object { } $entity->$property = $value; } + + foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { + if ($propertyAttributes->oneToOne !== null) { + $entity->{$propertyAttributes->property->getName()} = null; + } + } + return $entity; } /** - * Insert the entity in the database. + * Builds a select query resolving both sides of any OneToOne relation via a LEFT JOIN. + * Columns are aliased `e_` (main entity) and `r_` (each relation) + * to stay unique even when tables share column names. * - * This will additionally generate a value for the primary key. + * @return array{0: IQueryBuilder, 1: array} + */ + private function buildJoinedSelectQuery(EntityInfo $entityInfo): array { + $qb = $this->connection->getQueryBuilder(); + $qb->from($entityInfo->tableName, 'e'); + + foreach (array_keys($entityInfo->mappingColumnToProperty) as $column) { + $qb->selectAlias('e.' . $column, 'e_' . $column); + } + + /** @var array $relations */ + $relations = []; + $index = 0; + foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { + if ($propertyAttributes->oneToOne === null) { + continue; + } + + if ($propertyAttributes->joinColumn !== null && $propertyAttributes->oneToOne->invertedBy !== null) { + // Owning side: the join column lives on our own table. + $targetEntityInfo = $this->entityManager->getEntityInfo($propertyAttributes->oneToOne->targetEntity); + $alias = 'r' . $index++; + + $this->joinRelation( + $qb, + $alias, + $targetEntityInfo, + 'e.' . $propertyAttributes->joinColumn->name, + $alias . '.' . $propertyAttributes->joinColumn->referencedColumnName, + ); + + $relations[$alias] = ['attributes' => $propertyAttributes, 'entityInfo' => $targetEntityInfo]; + continue; + } + + if ($propertyAttributes->oneToOne->mappedBy !== null) { + // Inverse side: the join column lives on the target's table, pointing back at us. + $targetEntityInfo = $this->entityManager->getEntityInfo($propertyAttributes->oneToOne->targetEntity); + + $owningPropertyAttributes = null; + foreach ($targetEntityInfo->propertiesAttributes as $candidate) { + if ($candidate->property->getName() === $propertyAttributes->oneToOne->mappedBy) { + $owningPropertyAttributes = $candidate; + break; + } + } + + if ($owningPropertyAttributes === null || $owningPropertyAttributes->joinColumn === null) { + continue; + } + + $alias = 'r' . $index++; + $this->joinRelation( + $qb, + $alias, + $targetEntityInfo, + $alias . '.' . $owningPropertyAttributes->joinColumn->name, + 'e.' . $owningPropertyAttributes->joinColumn->referencedColumnName, + ); + + $relations[$alias] = ['attributes' => $propertyAttributes, 'entityInfo' => $targetEntityInfo]; + } + } + + return [$qb, $relations]; + } + + private function joinRelation(IQueryBuilder $qb, string $alias, EntityInfo $targetEntityInfo, string $leftExpr, string $rightExpr): void { + $qb->leftJoin('e', $targetEntityInfo->tableName, $alias, $qb->expr()->eq($leftExpr, $rightExpr)); + + foreach (array_keys($targetEntityInfo->mappingColumnToProperty) as $column) { + $qb->selectAlias($alias . '.' . $column, $alias . '_' . $column); + } + } + + /** + * @param array $relations + * @param array $row + * @return T + */ + private function mapJoinedRowToEntity(array $relations, mixed $row): object { + $mainRow = []; + /** @var array> $relationRows */ + $relationRows = []; + foreach ($row as $key => $value) { + if (str_starts_with($key, 'e_')) { + $mainRow[substr($key, 2)] = $value; + continue; + } + + foreach ($relations as $alias => $relation) { + $prefix = $alias . '_'; + if (str_starts_with($key, $prefix)) { + $relationRows[$alias][substr($key, strlen($prefix))] = $value; + continue 2; + } + } + } + + $entity = $this->hydrateRow($this->entityClass, $mainRow); + + foreach ($relations as $alias => $relation) { + $propertyName = $relation['attributes']->property->getName(); + $targetEntityInfo = $relation['entityInfo']; + $idColumn = $targetEntityInfo->mappingPropertyToColumn[$targetEntityInfo->idProperty->getName()]; + $relationRow = $relationRows[$alias] ?? []; + + if (($relationRow[$idColumn] ?? null) === null) { + $entity->$propertyName = null; + continue; + } + + /** @var OneToOne $oneToOne */ + $oneToOne = $relation['attributes']->oneToOne; + $entity->$propertyName = $this->hydrateRow($oneToOne->targetEntity, $relationRow); + } + + // Safety net for a malformed mapping that never made it into $relations. + $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); + foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { + if ($propertyAttributes->oneToOne === null) { + continue; + } + + $alreadyResolved = false; + foreach ($relations as $relation) { + if ($relation['attributes'] === $propertyAttributes) { + $alreadyResolved = true; + break; + } + } + + if (!$alreadyResolved) { + $entity->{$propertyAttributes->property->getName()} = null; + } + } + + /** @var T */ + return $entity; + } + + /** + * @param array $relations + * @return \Generator + */ + private function yieldJoinedEntities(IQueryBuilder $query, array $relations): \Generator { + $result = $query->executeQuery(); + try { + while ($row = $result->fetch()) { + yield $this->mapJoinedRowToEntity($relations, $row); + } + } finally { + $result->closeCursor(); + } + } + + /** + * @param array $relations + * @return T + * @throws DoesNotExistException + * @throws MultipleObjectsReturnedException + */ + private function findJoinedEntity(IQueryBuilder $query, array $relations): object { + $result = $query->executeQuery(); + + $row = $result->fetch(); + if ($row === false) { + $result->closeCursor(); + throw new DoesNotExistException($this->buildDebugMessage( + 'Did expect one result but found none when executing', $query + )); + } + + $row2 = $result->fetch(); + $result->closeCursor(); + if ($row2 !== false) { + throw new MultipleObjectsReturnedException($this->buildDebugMessage( + 'Did not expect more than one result when executing', $query + )); + } + + return $this->mapJoinedRowToEntity($relations, $row); + } + + /** + * Inserts the entity and populates its generated primary key. * * @psalm-param T $entity * @return T @@ -165,16 +333,14 @@ public function delete(object $entity): void { } /** - * Finds entities by a set of criteria. - * - * Use the property names for the criteria and orderBy key. + * Finds entities by a set of criteria, keyed by property name. * * @param array> $criteria * @param array|null $orderBy * @return \Generator */ public function findBy(array $criteria, array $orderBy = [], ?int $limit = null, ?int $offset = null): \Generator { - $qb = $this->getSelectQueryBuilder($criteria, $orderBy); + [$qb, $relations] = $this->getJoinedSelectQueryBuilder($criteria, $orderBy); if ($limit !== null) { $qb->setMaxResults($limit); @@ -184,7 +350,7 @@ public function findBy(array $criteria, array $orderBy = [], ?int $limit = null, $qb->setFirstResult($offset); } - return $this->yieldEntities($qb); + return $this->yieldJoinedEntities($qb, $relations); } /** @@ -201,7 +367,6 @@ public function deleteBy(array $criteria, ?int $limit = null): int { foreach ($criteria as $property => $value) { $column = $entityInfo->mappingPropertyToColumn[$property]; $type = $this->entityManager->getParameterType($entityInfo->mappingColumnToTypes[$column], is_array($value)); - $type = $this->entityManager->getParameterType($entityInfo->mappingColumnToTypes[$column], is_array($value)); if ($value === null) { $qb->andWhere($qb->expr()->isNull($column)); } elseif (is_array($value)) { @@ -221,7 +386,7 @@ public function deleteBy(array $criteria, ?int $limit = null): int { } /** - * Finds a single entity by a set of criteria. + * Finds a single entity by a set of criteria, keyed by property name. * * @param array> $criteria * @param array|null $orderBy @@ -229,87 +394,51 @@ public function deleteBy(array $criteria, ?int $limit = null): int { * @throws DoesNotExistException */ public function findOneBy(array $criteria, array $orderBy = []): object { - $qb = $this->getSelectQueryBuilder($criteria, $orderBy); + [$qb, $relations] = $this->getJoinedSelectQueryBuilder($criteria, $orderBy); $qb->setMaxResults(1); - return $this->findEntity($qb); + return $this->findJoinedEntity($qb, $relations); } /** * @param array> $criteria * @param array|null $orderBy + * @return array{0: IQueryBuilder, 1: array} */ - private function getSelectQueryBuilder(array $criteria, array $orderBy = []): IQueryBuilder { + private function getJoinedSelectQueryBuilder(array $criteria, array $orderBy = []): array { $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); - - $qb = $this->connection->getQueryBuilder(); - $qb->select('*') - ->from($entityInfo->tableName); + [$qb, $relations] = $this->buildJoinedSelectQuery($entityInfo); foreach ($criteria as $property => $value) { $column = $entityInfo->mappingPropertyToColumn[$property]; $type = $this->entityManager->getParameterType($entityInfo->mappingColumnToTypes[$column], is_array($value)); if ($value === null) { - $qb->andWhere($qb->expr()->isNull($column)); + $qb->andWhere($qb->expr()->isNull('e.' . $column)); } elseif (is_array($value)) { // IN expression - $qb->andWhere($qb->expr()->in($column, $qb->createNamedParameter($value, $type))); + $qb->andWhere($qb->expr()->in('e.' . $column, $qb->createNamedParameter($value, $type))); } else { // = expression - $qb->andWhere($qb->expr()->eq($column, $qb->createNamedParameter($value, $type))); + $qb->andWhere($qb->expr()->eq('e.' . $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 - * - * @psalm-return T the entity - * @throws Exception - * @throws MultipleObjectsReturnedException if more than one item exist - * @throws DoesNotExistException if the item does not exist - */ - 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); + return [$qb, $relations]; } /** - * @return Generator + * @return \Generator * @throws Exception */ public function yieldAll(): \Generator { - $qb = $this->connection->getQueryBuilder(); - $qb->select('*') - ->from($this->getTableName()); + $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); + [$qb, $relations] = $this->buildJoinedSelectQuery($entityInfo); - return $this->yieldEntities($qb); + return $this->yieldJoinedEntities($qb, $relations); } public function getTableName(): string { diff --git a/tests/lib/AppFramework/ORM/RepositoryTest.php b/tests/lib/AppFramework/ORM/RepositoryTest.php index 533ac637a9f48..96d8de38a081d 100644 --- a/tests/lib/AppFramework/ORM/RepositoryTest.php +++ b/tests/lib/AppFramework/ORM/RepositoryTest.php @@ -16,11 +16,12 @@ use OCP\AppFramework\ORM\Attribute\JoinColumn; use OCP\AppFramework\ORM\Attribute\OneToOne; use OCP\AppFramework\ORM\Repository; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\DB\Types; use OCP\IConfig; use OCP\IDBConnection; use OCP\Server; -use PHPUnit\Framework\TestCase; +use Test\TestCase; #[Entity(name: 'repository_test_test1')] class NoPrimaryKey { @@ -78,7 +79,7 @@ final class Customer { public ?int $id = null; #[OneToOne(targetEntity: Cart::class, mappedBy: 'customer')] - #[JoinColumn(name: 'cart_id', referencedColumnName: 'id', onDelete: 'CASCADE')] + #[JoinColumn(name: 'cart_id', referencedColumnName: 'id')] public Cart|null $cart = null; #[Column(name: 'name', type: Types::STRING, nullable: false)] @@ -114,6 +115,94 @@ public function __construct( } } +#[Entity(name: 'repository_invalid_owner')] +final class InvalidOwningOnDelete { + #[Id] + #[Column(name: 'id', type: Types::BIGINT)] + public ?int $id = null; + + #[OneToOne(targetEntity: InvalidMappedByOnDelete::class, invertedBy: 'owner')] + #[JoinColumn(name: 'invalid_id', referencedColumnName: 'id')] + public InvalidMappedByOnDelete|null $invalid = null; +} + +#[Entity(name: 'repository_invalid_mapped')] +final class InvalidMappedByOnDelete { + #[Id] + #[Column(name: 'id', type: Types::BIGINT)] + public ?int $id = null; + + #[OneToOne(targetEntity: InvalidOwningOnDelete::class, mappedBy: 'invalid')] + #[JoinColumn(name: 'owner_id', referencedColumnName: 'id', onDelete: 'CASCADE')] + public InvalidOwningOnDelete|null $owner = null; +} + +#[Entity(name: 'repository_typo_owner')] +final class TypoOwning { + #[Id] + #[Column(name: 'id', type: Types::BIGINT)] + public ?int $id = null; + + #[OneToOne(targetEntity: TypoTarget::class, invertedBy: 'owner')] + #[JoinColumn(name: 'target_id', referencedColumnName: 'id')] + public TypoTarget|null $target = null; +} + +#[Entity(name: 'repository_typo_target')] +final class TypoTarget { + #[Id] + #[Column(name: 'id', type: Types::BIGINT)] + public ?int $id = null; + + #[OneToOne(targetEntity: TypoOwning::class, mappedBy: 'ownerTypo')] + #[JoinColumn(name: 'owner_id', referencedColumnName: 'id')] + public TypoOwning|null $owner = null; +} + +#[Entity(name: 'repository_cascade_parent')] +final class CascadeParent { + #[Id] + #[Column(name: 'id', type: Types::BIGINT)] + public ?int $id = null; + + #[Column(name: 'name', type: Types::STRING, nullable: true)] + public ?string $name = null; + + #[OneToOne(targetEntity: CascadeChild::class, mappedBy: 'parent')] + #[JoinColumn(name: 'child_id', referencedColumnName: 'id')] + public CascadeChild|null $child = null; +} + +/** @template-extends Repository */ +class CascadeParentRepository extends Repository { + public function __construct( + IDBConnection $connection, + ) { + parent::__construct($connection, CascadeParent::class); + } +} + +#[Entity(name: 'repository_cascade_child')] +final class CascadeChild { + #[Id] + #[Column(name: 'id', type: Types::BIGINT)] + public ?int $id = null; + + #[OneToOne(targetEntity: CascadeParent::class, invertedBy: 'child')] + #[JoinColumn(name: 'parent_id', referencedColumnName: 'id', onDelete: 'CASCADE')] + public CascadeParent|null $parent = null; +} + +/** @template-extends Repository */ +class CascadeChildRepository extends Repository { + public function __construct( + IDBConnection $connection, + ) { + parent::__construct($connection, CascadeChild::class); + } +} + +#[\PHPUnit\Framework\Attributes\Group('DB')] class RepositoryTest extends TestCase { /** @var array> */ public static array $entitiesClasses = [ @@ -121,6 +210,8 @@ class RepositoryTest extends TestCase { PrimaryKey::class => PrimaryKeyRepository::class, Customer::class => CustomerRepository::class, Cart::class => CartRepository::class, + CascadeParent::class => CascadeParentRepository::class, + CascadeChild::class => CascadeChildRepository::class, ]; public static function setUpBeforeClass(): void { @@ -256,9 +347,178 @@ public function testOneToOne(): void { $customerRepo->insert($customer); $this->assertNotNull($customer->id); + $savedCustomer = $customerRepo->findOneBy(['id' => $customer->id]); + $this->assertNull($savedCustomer->cart); + $cart = new Cart(); $cart->customer = $customer; $cartRepo->insert($cart); $this->assertNotNull($cart->id); + + // Owning side: reading the cart back resolves the customer relation + $savedCart = $cartRepo->findOneBy(['id' => $cart->id]); + $this->assertNotNull($savedCart->customer); + $this->assertEquals($customer->id, $savedCart->customer->id); + $this->assertEquals($customer->name, $savedCart->customer->name); + + // Inverse side: reading the customer back resolves the cart relation + $savedCustomer = $customerRepo->findOneBy(['id' => $customer->id]); + $this->assertNotNull($savedCustomer->cart); + $this->assertEquals($cart->id, $savedCustomer->cart->id); + } + + public function testInsertOnMappedBySideThrows(): void { + $customerRepo = Server::get(CustomerRepository::class); + + $cart = new Cart(); + $cart->customer = null; + + $customer = new Customer(); + $customer->name = 'foo'; + $customer->cart = $cart; + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('is the mappedBy (inverse) side of a OneToOne relation'); + + $customerRepo->insert($customer); + } + + public function testOneToOneUniqueConstraint(): void { + $cartRepo = Server::get(CartRepository::class); + $customerRepo = Server::get(CustomerRepository::class); + + $customer = new Customer(); + $customer->name = 'shared'; + $customerRepo->insert($customer); + + $cart1 = new Cart(); + $cart1->customer = $customer; + $cartRepo->insert($cart1); + + $cart2 = new Cart(); + $cart2->customer = $customer; + + try { + $cartRepo->insert($cart2); + $this->fail('Expected inserting a second Cart for the same Customer to violate the unique constraint.'); + } catch (\OCP\DB\Exception $e) { + $this->assertEquals(\OCP\DB\Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION, $e->getReason()); + } + } + + public function testOneToOneNullRelation(): void { + $customerRepo = Server::get(CustomerRepository::class); + + $customer = new Customer(); + $customer->name = 'no cart yet'; + $customerRepo->insert($customer); + + $savedCustomer = $customerRepo->findOneBy(['id' => $customer->id]); + $this->assertNull($savedCustomer->cart); + } + + public function testMappedByOnDeleteThrows(): void { + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('sets JoinColumn::$onDelete on the mappedBy (inverse) side'); + + Server::get(EntityManager::class)->getEntityInfo(InvalidMappedByOnDelete::class); + } + + public function testTypoInMappedByThrows(): void { + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('has no property named \'ownerTypo\''); + + Server::get(EntityManager::class)->getEntityInfo(TypoTarget::class); + } + + public function testDeleteWithDanglingMappedByThrows(): void { + $cartRepo = Server::get(CartRepository::class); + $customerRepo = Server::get(CustomerRepository::class); + + $customer = new Customer(); + $customer->name = 'has a cart'; + $customerRepo->insert($customer); + + $cart = new Cart(); + $cart->customer = $customer; + $cartRepo->insert($cart); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('still references it'); + + $customerRepo->delete($customer); + } + + public function testDeleteCascadesWhenConfigured(): void { + $parentRepo = Server::get(CascadeParentRepository::class); + $childRepo = Server::get(CascadeChildRepository::class); + + $parent = new CascadeParent(); + $parentRepo->insert($parent); + + $child = new CascadeChild(); + $child->parent = $parent; + $childRepo->insert($child); + + $parentRepo->delete($parent); + + $entities = iterator_to_array($childRepo->yieldAll()); + $this->assertCount(0, $entities); + } + + private function normalizeSql(string $sql): string { + return str_replace(['`', '"'], '', $sql); + } + + public function testOwningSideGeneratesLeftJoin(): void { + $cartRepo = Server::get(CartRepository::class); + + /** @var array{0: IQueryBuilder, 1: array} $result */ + $result = self::invokePrivate($cartRepo, 'getJoinedSelectQueryBuilder', [['id' => 1], []]); + [$qb, $relations] = $result; + + $this->assertCount(1, $relations); + $this->assertEquals( + 'SELECT e.id AS e_id, r0.id AS r0_id, r0.name AS r0_name ' + . 'FROM *PREFIX*repository_cart e ' + . 'LEFT JOIN *PREFIX*repository_customer r0 ON e.customer_id = r0.id ' + . 'WHERE e.id = :dcValue1', + $this->normalizeSql($qb->getSQL()), + ); + } + + public function testInverseSideGeneratesLeftJoin(): void { + $customerRepo = Server::get(CustomerRepository::class); + + /** @var array{0: IQueryBuilder, 1: array} $result */ + $result = self::invokePrivate($customerRepo, 'getJoinedSelectQueryBuilder', [['id' => 1], []]); + [$qb, $relations] = $result; + + $this->assertCount(1, $relations); + $this->assertEquals( + 'SELECT e.id AS e_id, e.name AS e_name, r0.id AS r0_id ' + . 'FROM *PREFIX*repository_customer e ' + . 'LEFT JOIN *PREFIX*repository_cart r0 ON r0.customer_id = e.id ' + . 'WHERE e.id = :dcValue1', + $this->normalizeSql($qb->getSQL()), + ); + } + + public function testEntityWithoutRelationsGeneratesNoJoin(): void { + $repo = Server::get(PrimaryKeyRepository::class); + + /** @var array{0: IQueryBuilder, 1: array} $result */ + $result = self::invokePrivate($repo, 'getJoinedSelectQueryBuilder', [['id' => 1], []]); + [$qb, $relations] = $result; + + $this->assertCount(0, $relations); + $this->assertEquals( + 'SELECT e.id AS e_id, e.name AS e_name, e.notNullable AS e_notNullable, ' + . 'e.integer_val AS e_integer_val, e.bigInt_val AS e_bigInt_val, ' + . 'e.float_val AS e_float_val, e.date_val AS e_date_val ' + . 'FROM *PREFIX*repository_test_test2 e ' + . 'WHERE e.id = :dcValue1', + $this->normalizeSql($qb->getSQL()), + ); } } From 074392b58f35fbc60ed43b10976edd6a00e890b2 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 21 Jul 2026 12:33:21 +0200 Subject: [PATCH 05/10] refactor(orm): Remove boilerplate in tests Signed-off-by: Carl Schwan --- .../AppFramework/ORM/EntityManager.php | 10 ++ tests/lib/AppFramework/ORM/RepositoryTest.php | 114 ++++++------------ 2 files changed, 44 insertions(+), 80 deletions(-) diff --git a/lib/private/AppFramework/ORM/EntityManager.php b/lib/private/AppFramework/ORM/EntityManager.php index 08d6ac5493410..448936f2696a9 100644 --- a/lib/private/AppFramework/ORM/EntityManager.php +++ b/lib/private/AppFramework/ORM/EntityManager.php @@ -12,6 +12,7 @@ use OCP\AppFramework\ORM\Attribute\Column; use OCP\AppFramework\ORM\Attribute\JoinColumn; use OCP\AppFramework\ORM\Attribute\OneToOne; +use OCP\AppFramework\ORM\Repository; use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\DB\Types; @@ -42,6 +43,15 @@ public function getEntityInfo(string $entityClass): EntityInfo { return $entityInfo; } + /** + * @template T of object + * @param class-string $entityClass + * @return Repository + */ + public function getRepository(string $entityClass): Repository { + return new Repository($this->connection, $entityClass); + } + /** * @template T * @psalm-param T $entity diff --git a/tests/lib/AppFramework/ORM/RepositoryTest.php b/tests/lib/AppFramework/ORM/RepositoryTest.php index 96d8de38a081d..2507413d1f21b 100644 --- a/tests/lib/AppFramework/ORM/RepositoryTest.php +++ b/tests/lib/AppFramework/ORM/RepositoryTest.php @@ -19,7 +19,6 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\DB\Types; use OCP\IConfig; -use OCP\IDBConnection; use OCP\Server; use Test\TestCase; @@ -29,15 +28,6 @@ class NoPrimaryKey { public ?int $id = null; } -/** @template-extends Repository */ -class NoPrimaryKeyRepository extends Repository { - public function __construct( - IDBConnection $connection, - ) { - parent::__construct($connection, NoPrimaryKey::class); - } -} - #[Entity(name: 'repository_test_test2')] class PrimaryKey { #[Id] @@ -63,15 +53,6 @@ class PrimaryKey { public \DateTime $date; } -/** @template-extends Repository */ -class PrimaryKeyRepository extends Repository { - public function __construct( - IDBConnection $connection, - ) { - parent::__construct($connection, PrimaryKey::class); - } -} - #[Entity(name: 'repository_customer')] final class Customer { #[Id] @@ -86,15 +67,6 @@ final class Customer { public string $name; } -/** @template-extends Repository */ -class CustomerRepository extends Repository { - public function __construct( - IDBConnection $connection, - ) { - parent::__construct($connection, Customer::class); - } -} - #[Entity(name: 'repository_cart')] final class Cart { #[Id] @@ -106,15 +78,6 @@ final class Cart { public Customer|null $customer; } -/** @template-extends Repository */ -class CartRepository extends Repository { - public function __construct( - IDBConnection $connection, - ) { - parent::__construct($connection, Cart::class); - } -} - #[Entity(name: 'repository_invalid_owner')] final class InvalidOwningOnDelete { #[Id] @@ -173,15 +136,6 @@ final class CascadeParent { public CascadeChild|null $child = null; } -/** @template-extends Repository */ -class CascadeParentRepository extends Repository { - public function __construct( - IDBConnection $connection, - ) { - parent::__construct($connection, CascadeParent::class); - } -} - #[Entity(name: 'repository_cascade_child')] final class CascadeChild { #[Id] @@ -193,31 +147,22 @@ final class CascadeChild { public CascadeParent|null $parent = null; } -/** @template-extends Repository */ -class CascadeChildRepository extends Repository { - public function __construct( - IDBConnection $connection, - ) { - parent::__construct($connection, CascadeChild::class); - } -} - #[\PHPUnit\Framework\Attributes\Group('DB')] class RepositoryTest extends TestCase { - /** @var array> */ + /** @var list */ public static array $entitiesClasses = [ - NoPrimaryKey::class => NoPrimaryKeyRepository::class, - PrimaryKey::class => PrimaryKeyRepository::class, - Customer::class => CustomerRepository::class, - Cart::class => CartRepository::class, - CascadeParent::class => CascadeParentRepository::class, - CascadeChild::class => CascadeChildRepository::class, + NoPrimaryKey::class, + PrimaryKey::class, + Customer::class, + Cart::class, + CascadeParent::class, + CascadeChild::class, ]; public static function setUpBeforeClass(): void { $schema = new SchemaWrapper(Server::get(Connection::class)); $entityManager = Server::get(EntityManager::class); - foreach (static::$entitiesClasses as $entityClass => $repositoryClass) { + foreach (static::$entitiesClasses as $entityClass) { try { $entityManager->createTable($entityClass, $schema); } catch (\RuntimeException) { @@ -230,7 +175,7 @@ public static function setUpBeforeClass(): void { public static function tearDownAfterClass(): void { $entityManager = Server::get(EntityManager::class); $prefix = Server::get(IConfig::class)->getSystemValueString('dbtableprefix', 'oc_'); - foreach (static::$entitiesClasses as $entityClass => $repositoryClass) { + foreach (static::$entitiesClasses as $entityClass) { try { $entityManager->dropTable($entityClass, $prefix); } catch (\RuntimeException) { @@ -239,15 +184,24 @@ public static function tearDownAfterClass(): void { } } + /** + * @template T of object + * @param class-string $entityClass + * @return Repository + */ + private function getRepository(string $entityClass): Repository { + return Server::get(EntityManager::class)->getRepository($entityClass); + } + public function testMissingPrimaryKey(): void { $this->expectException(\RuntimeException::class); - $repo = Server::get(NoPrimaryKeyRepository::class); + $repo = $this->getRepository(NoPrimaryKey::class); $_ = $repo->getTableName(); } public function testPrimaryKey(): void { - $repo = Server::get(PrimaryKeyRepository::class); + $repo = $this->getRepository(PrimaryKey::class); $this->assertEquals('repository_test_test2', $repo->getTableName()); $entity = new PrimaryKey(); $entity->name = null; @@ -315,7 +269,7 @@ public function testPrimaryKey(): void { } public function testDeleteBy(): void { - $repo = Server::get(PrimaryKeyRepository::class); + $repo = $this->getRepository(PrimaryKey::class); $entity = new PrimaryKey(); $entity->name = null; @@ -339,8 +293,8 @@ public function testDeleteBy(): void { } public function testOneToOne(): void { - $cartRepo = Server::get(CartRepository::class); - $customerRepo = Server::get(CustomerRepository::class); + $cartRepo = $this->getRepository(Cart::class); + $customerRepo = $this->getRepository(Customer::class); $customer = new Customer(); $customer->name = 'foo'; @@ -368,7 +322,7 @@ public function testOneToOne(): void { } public function testInsertOnMappedBySideThrows(): void { - $customerRepo = Server::get(CustomerRepository::class); + $customerRepo = $this->getRepository(Customer::class); $cart = new Cart(); $cart->customer = null; @@ -384,8 +338,8 @@ public function testInsertOnMappedBySideThrows(): void { } public function testOneToOneUniqueConstraint(): void { - $cartRepo = Server::get(CartRepository::class); - $customerRepo = Server::get(CustomerRepository::class); + $cartRepo = $this->getRepository(Cart::class); + $customerRepo = $this->getRepository(Customer::class); $customer = new Customer(); $customer->name = 'shared'; @@ -407,7 +361,7 @@ public function testOneToOneUniqueConstraint(): void { } public function testOneToOneNullRelation(): void { - $customerRepo = Server::get(CustomerRepository::class); + $customerRepo = $this->getRepository(Customer::class); $customer = new Customer(); $customer->name = 'no cart yet'; @@ -432,8 +386,8 @@ public function testTypoInMappedByThrows(): void { } public function testDeleteWithDanglingMappedByThrows(): void { - $cartRepo = Server::get(CartRepository::class); - $customerRepo = Server::get(CustomerRepository::class); + $cartRepo = $this->getRepository(Cart::class); + $customerRepo = $this->getRepository(Customer::class); $customer = new Customer(); $customer->name = 'has a cart'; @@ -450,8 +404,8 @@ public function testDeleteWithDanglingMappedByThrows(): void { } public function testDeleteCascadesWhenConfigured(): void { - $parentRepo = Server::get(CascadeParentRepository::class); - $childRepo = Server::get(CascadeChildRepository::class); + $parentRepo = $this->getRepository(CascadeParent::class); + $childRepo = $this->getRepository(CascadeChild::class); $parent = new CascadeParent(); $parentRepo->insert($parent); @@ -471,7 +425,7 @@ private function normalizeSql(string $sql): string { } public function testOwningSideGeneratesLeftJoin(): void { - $cartRepo = Server::get(CartRepository::class); + $cartRepo = $this->getRepository(Cart::class); /** @var array{0: IQueryBuilder, 1: array} $result */ $result = self::invokePrivate($cartRepo, 'getJoinedSelectQueryBuilder', [['id' => 1], []]); @@ -488,7 +442,7 @@ public function testOwningSideGeneratesLeftJoin(): void { } public function testInverseSideGeneratesLeftJoin(): void { - $customerRepo = Server::get(CustomerRepository::class); + $customerRepo = $this->getRepository(Customer::class); /** @var array{0: IQueryBuilder, 1: array} $result */ $result = self::invokePrivate($customerRepo, 'getJoinedSelectQueryBuilder', [['id' => 1], []]); @@ -505,7 +459,7 @@ public function testInverseSideGeneratesLeftJoin(): void { } public function testEntityWithoutRelationsGeneratesNoJoin(): void { - $repo = Server::get(PrimaryKeyRepository::class); + $repo = $this->getRepository(PrimaryKey::class); /** @var array{0: IQueryBuilder, 1: array} $result */ $result = self::invokePrivate($repo, 'getJoinedSelectQueryBuilder', [['id' => 1], []]); From 1a404bb45b986daa5f83eb15115d11025eba1b73 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 21 Jul 2026 13:51:01 +0200 Subject: [PATCH 06/10] refactor: Cleanup and fix tests Signed-off-by: Carl Schwan --- .../lib/Db/BackupCode.php | 4 +- .../lib/Db/BackupCodeMapper.php | 16 +- .../lib/Listener/UserDeleted.php | 2 +- build/psalm-baseline.xml | 9 - lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/AppFramework/ORM/EntityInfo.php | 8 +- .../AppFramework/ORM/EntityManager.php | 57 ++--- .../AppFramework/ORM/PropertyAttributes.php | 19 ++ lib/private/Tagging/Tag.php | 6 +- lib/private/Tagging/TagMapper.php | 2 +- lib/private/Tags.php | 208 +++++------------- .../AppFramework/ORM/Attribute/Column.php | 7 +- .../AppFramework/ORM/Attribute/Entity.php | 7 +- lib/public/AppFramework/ORM/Attribute/Id.php | 7 +- .../AppFramework/ORM/Attribute/JoinColumn.php | 7 +- .../AppFramework/ORM/Attribute/ManyToOne.php | 50 +++++ .../AppFramework/ORM/Attribute/OneToOne.php | 7 +- lib/public/AppFramework/ORM/Repository.php | 30 ++- lib/public/ITags.php | 24 +- tests/lib/AppFramework/ORM/RepositoryTest.php | 77 +++++++ 21 files changed, 297 insertions(+), 252 deletions(-) create mode 100644 lib/public/AppFramework/ORM/Attribute/ManyToOne.php diff --git a/apps/twofactor_backupcodes/lib/Db/BackupCode.php b/apps/twofactor_backupcodes/lib/Db/BackupCode.php index bad6abf431e02..5a3850ba015d2 100644 --- a/apps/twofactor_backupcodes/lib/Db/BackupCode.php +++ b/apps/twofactor_backupcodes/lib/Db/BackupCode.php @@ -13,11 +13,11 @@ use OCP\AppFramework\ORM\Attribute\Entity; use OCP\AppFramework\ORM\Attribute\Id; use OCP\DB\Types; -use OCP\Snowflake\IGenerator; +use OCP\Snowflake\ISnowflakeGenerator; #[Entity(name: 'twofactor_backupcodes')] final class BackupCode { - #[Id(generatorClass: IGenerator::class)] + #[Id(generatorClass: ISnowflakeGenerator::class)] #[Column(name: 'id', type: Types::STRING, length: 64, nullable: false)] public ?string $id = null; diff --git a/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php b/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php index e8259eeaf4e5b..0bbc5b520ecbd 100644 --- a/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php +++ b/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php @@ -9,7 +9,9 @@ namespace OCA\TwoFactorBackupCodes\Db; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\ORM\Repository; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\IUser; @@ -37,9 +39,13 @@ public function deleteByUser(IUser $user): void { } public function findOneByUser(IUser $user): ?BackupCode { - return $this->findOneBy([ - 'userId' => $user->getUID(), - ]); + try { + return $this->findOneBy([ + 'userId' => $user->getUID(), + ]); + } catch (DoesNotExistException) { + return null; + } } /** @@ -47,10 +53,10 @@ public function findOneByUser(IUser $user): ?BackupCode { * @return int number of affected rows */ public function markUsedIfUnused(BackupCode $code): int { - $qb = $this->db->getQueryBuilder(); + $qb = $this->connection->getQueryBuilder(); $qb->update($this->getTableName()) ->set('used', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT)) - ->where($qb->expr()->eq('id', $qb->createNamedParameter($code->getId(), IQueryBuilder::PARAM_INT))) + ->where($qb->expr()->eq('id', $qb->createNamedParameter($code->id, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('used', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); return $qb->executeStatement(); } diff --git a/apps/twofactor_backupcodes/lib/Listener/UserDeleted.php b/apps/twofactor_backupcodes/lib/Listener/UserDeleted.php index 020718fb75654..22369c1ec4b78 100644 --- a/apps/twofactor_backupcodes/lib/Listener/UserDeleted.php +++ b/apps/twofactor_backupcodes/lib/Listener/UserDeleted.php @@ -28,6 +28,6 @@ public function handle(Event $event): void { return; } - $this->backupCodeMapper->deleteCodes($event->getUser()); + $this->backupCodeMapper->deleteByUser($event->getUser()); } } diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 8fbfd584765b4..d3d98faafb5e8 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -4151,15 +4151,6 @@ - - - - - - - - - circleToTeam(...), $this->circlesManager->probeCircles())]]> diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 48100df35d1a4..bf6ecd83ffd8a 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -183,6 +183,7 @@ 'OCP\\AppFramework\\ORM\\Attribute\\Entity' => $baseDir . '/lib/public/AppFramework/ORM/Attribute/Entity.php', 'OCP\\AppFramework\\ORM\\Attribute\\Id' => $baseDir . '/lib/public/AppFramework/ORM/Attribute/Id.php', 'OCP\\AppFramework\\ORM\\Attribute\\JoinColumn' => $baseDir . '/lib/public/AppFramework/ORM/Attribute/JoinColumn.php', + 'OCP\\AppFramework\\ORM\\Attribute\\ManyToOne' => $baseDir . '/lib/public/AppFramework/ORM/Attribute/ManyToOne.php', 'OCP\\AppFramework\\ORM\\Attribute\\OneToOne' => $baseDir . '/lib/public/AppFramework/ORM/Attribute/OneToOne.php', 'OCP\\AppFramework\\ORM\\Repository' => $baseDir . '/lib/public/AppFramework/ORM/Repository.php', 'OCP\\AppFramework\\PublicShareController' => $baseDir . '/lib/public/AppFramework/PublicShareController.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 5e1904c5f6707..ab6e49f8cbb7a 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -224,6 +224,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\AppFramework\\ORM\\Attribute\\Entity' => __DIR__ . '/../../..' . '/lib/public/AppFramework/ORM/Attribute/Entity.php', 'OCP\\AppFramework\\ORM\\Attribute\\Id' => __DIR__ . '/../../..' . '/lib/public/AppFramework/ORM/Attribute/Id.php', 'OCP\\AppFramework\\ORM\\Attribute\\JoinColumn' => __DIR__ . '/../../..' . '/lib/public/AppFramework/ORM/Attribute/JoinColumn.php', + 'OCP\\AppFramework\\ORM\\Attribute\\ManyToOne' => __DIR__ . '/../../..' . '/lib/public/AppFramework/ORM/Attribute/ManyToOne.php', 'OCP\\AppFramework\\ORM\\Attribute\\OneToOne' => __DIR__ . '/../../..' . '/lib/public/AppFramework/ORM/Attribute/OneToOne.php', 'OCP\\AppFramework\\ORM\\Repository' => __DIR__ . '/../../..' . '/lib/public/AppFramework/ORM/Repository.php', 'OCP\\AppFramework\\PublicShareController' => __DIR__ . '/../../..' . '/lib/public/AppFramework/PublicShareController.php', diff --git a/lib/private/AppFramework/ORM/EntityInfo.php b/lib/private/AppFramework/ORM/EntityInfo.php index 25e7ab6ae3217..9123651551c85 100644 --- a/lib/private/AppFramework/ORM/EntityInfo.php +++ b/lib/private/AppFramework/ORM/EntityInfo.php @@ -11,10 +11,11 @@ use OCP\AppFramework\ORM\Attribute\Entity; use OCP\AppFramework\ORM\Attribute\Id; use OCP\AppFramework\ORM\Attribute\JoinColumn; +use OCP\AppFramework\ORM\Attribute\ManyToOne; use OCP\AppFramework\ORM\Attribute\OneToOne; /** - * @template T + * @template T as object */ class EntityInfo { public readonly string $tableName; @@ -38,6 +39,9 @@ class EntityInfo { */ public array $propertiesAttributes = []; + /** + * @param class-string $entityClass + */ public function __construct( public readonly string $entityClass, ) { @@ -66,6 +70,8 @@ public function __construct( $this->idProperty = $property; } elseif ($instance instanceof OneToOne) { $propertyAttributes->oneToOne = $instance; + } elseif ($instance instanceof ManyToOne) { + $propertyAttributes->manyToOne = $instance; } elseif ($instance instanceof JoinColumn) { $propertyAttributes->joinColumn = $instance; } diff --git a/lib/private/AppFramework/ORM/EntityManager.php b/lib/private/AppFramework/ORM/EntityManager.php index 448936f2696a9..4ab208f9b2551 100644 --- a/lib/private/AppFramework/ORM/EntityManager.php +++ b/lib/private/AppFramework/ORM/EntityManager.php @@ -11,7 +11,6 @@ use OC\DB\SchemaWrapper; use OCP\AppFramework\ORM\Attribute\Column; use OCP\AppFramework\ORM\Attribute\JoinColumn; -use OCP\AppFramework\ORM\Attribute\OneToOne; use OCP\AppFramework\ORM\Repository; use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; @@ -26,7 +25,7 @@ public function __construct( ) { } - /** @var array, EntityInfo> $entitiesInfo */ + /** @var array> $entitiesInfo */ private array $entitiesInfo = []; /** @@ -77,15 +76,15 @@ public function insert(object $entity): object { $isSnowflake = true; /** @psalm-suppress UndefinedClass */ $values[$propertyAttributes->column->name] = $generator->nextId(); - $property->setValue($entity, $insert->createNamedParameter($values[$propertyAttributes['column']->name->name])); + $property->setValue($entity, $insert->createNamedParameter($values[$propertyAttributes->column->name])); } } continue; } - if ($propertyAttributes->oneToOne !== null && $propertyAttributes->joinColumn !== null) { - $oneToOne = $propertyAttributes->oneToOne; - if ($oneToOne->invertedBy === null) { + if ($propertyAttributes->isRelation() && $propertyAttributes->joinColumn !== null) { + $targetEntityClass = $propertyAttributes->getOwningRelationTarget(); + if ($targetEntityClass === null) { if ($property->getValue($entity) !== null) { throw new \LogicException($entity::class . '::' . $property->getName() . ' is the mappedBy (inverse) side of a OneToOne relation and cannot be persisted directly; set it from the owning (invertedBy) side instead.'); } @@ -95,7 +94,7 @@ public function insert(object $entity): object { $joinColumn = $propertyAttributes->joinColumn; /** @var object $object */ $targetEntity = $property->getValue($entity); - $targetEntityInfo = $this->getEntityInfo($oneToOne->targetEntity); + $targetEntityInfo = $this->getEntityInfo($targetEntityClass); if ($targetEntity === null) { $values[$joinColumn->name] = $insert->createNamedParameter(null); } else { @@ -147,9 +146,9 @@ public function update(object $entity): object { continue; } - if ($propertyAttributes->oneToOne !== null && $propertyAttributes->joinColumn !== null) { - $oneToOne = $propertyAttributes->oneToOne; - if ($oneToOne->invertedBy === null) { + if ($propertyAttributes->isRelation() && $propertyAttributes->joinColumn !== null) { + $targetEntityClass = $propertyAttributes->getOwningRelationTarget(); + if ($targetEntityClass === null) { continue; } @@ -157,7 +156,7 @@ public function update(object $entity): object { $joinColumn = $propertyAttributes->joinColumn; /** @var object $object */ $targetEntity = $value; - $targetEntityInfo = $this->getEntityInfo($oneToOne->targetEntity); + $targetEntityInfo = $this->getEntityInfo($targetEntityClass); if ($targetEntity === null) { $update->set($joinColumn->name, $update->createNamedParameter(null)); } else { @@ -207,7 +206,7 @@ public function delete(object $entity): void { $delete->executeStatement(); } catch (Exception $e) { if ($e->getReason() === Exception::REASON_FOREIGN_KEY_VIOLATION) { - throw new \LogicException($entityClass . ' cannot be deleted: another entity still references it via a OneToOne relation. Delete the related entity first, or set onDelete: \'CASCADE\' on the owning JoinColumn.', 0, $e); + throw new \LogicException($entityClass . ' cannot be deleted: another entity still references it. Delete the related entity first, or set onDelete: \'CASCADE\' on the owning JoinColumn.', 0, $e); } throw $e; } @@ -247,7 +246,7 @@ public function getParameterType(string $type, bool $isArray): string|int { /** * @internal Only for unit tests. * - * @param class-string $entityClass + * @param class-string $entityClass */ public function createTable(string $entityClass, SchemaWrapper $schema): void { $entityInfo = $this->getEntityInfo($entityClass); @@ -257,7 +256,7 @@ public function createTable(string $entityClass, SchemaWrapper $schema): void { foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { $this->createProperty($propertyAttributes, $table); - $this->createOneToOne($propertyAttributes, $table, $schema); + $this->createRelationColumn($propertyAttributes, $table, $schema); } $schema->getWrappedSchema()->toSql($this->connection->getDatabasePlatform()); @@ -295,27 +294,29 @@ private function createProperty(PropertyAttributes $attributes, Table $table): v } } - private function createOneToOne(PropertyAttributes $attributes, Table $table, SchemaWrapper $schema): void { - if ($attributes->joinColumn === null || $attributes->oneToOne === null) { + private function createRelationColumn(PropertyAttributes $attributes, Table $table, SchemaWrapper $schema): void { + $targetEntityClass = $attributes->getOwningRelationTarget(); + if ($attributes->joinColumn === null || $targetEntityClass === null) { return; } - if ($attributes->oneToOne->invertedBy !== null) { - $table->addColumn($attributes->joinColumn->name, Types::BIGINT, [ - 'notnull' => !$attributes->joinColumn->nullable, - ]); + $table->addColumn($attributes->joinColumn->name, Types::BIGINT, [ + 'notnull' => !$attributes->joinColumn->nullable, + ]); + if ($attributes->oneToOne !== null) { + // Enforces the "one" in OneToOne; ManyToOne intentionally allows duplicates. $table->addUniqueIndex([$attributes->joinColumn->name]); + } - $foreignEntityInfo = $this->getEntityInfo($attributes->oneToOne->targetEntity); - - $options = []; - if ($attributes->joinColumn->onDelete === 'CASCADE') { - $options['onDelete'] = 'CASCADE'; - } + $foreignEntityInfo = $this->getEntityInfo($targetEntityClass); - $foreignTableName = $schema->getTable($foreignEntityInfo->tableName)->getName(); - $table->addForeignKeyConstraint($foreignTableName, [$attributes->joinColumn->name], [$attributes->joinColumn->referencedColumnName], $options); + $options = []; + if ($attributes->joinColumn->onDelete === 'CASCADE') { + $options['onDelete'] = 'CASCADE'; } + + $foreignTableName = $schema->getTable($foreignEntityInfo->tableName)->getName(); + $table->addForeignKeyConstraint($foreignTableName, [$attributes->joinColumn->name], [$attributes->joinColumn->referencedColumnName], $options); } } diff --git a/lib/private/AppFramework/ORM/PropertyAttributes.php b/lib/private/AppFramework/ORM/PropertyAttributes.php index 159f1ea6c9b4f..e2b9261e9d75b 100644 --- a/lib/private/AppFramework/ORM/PropertyAttributes.php +++ b/lib/private/AppFramework/ORM/PropertyAttributes.php @@ -10,16 +10,35 @@ use OCP\AppFramework\ORM\Attribute\Column; use OCP\AppFramework\ORM\Attribute\Id; use OCP\AppFramework\ORM\Attribute\JoinColumn; +use OCP\AppFramework\ORM\Attribute\ManyToOne; use OCP\AppFramework\ORM\Attribute\OneToOne; class PropertyAttributes { public ?Id $id = null; public ?Column $column = null; public ?OneToOne $oneToOne = null; + public ?ManyToOne $manyToOne = null; public ?JoinColumn $joinColumn = null; public function __construct( public readonly \ReflectionProperty $property, ) { } + + public function getOwningRelationTarget(): ?string { + if ($this->joinColumn === null) { + return null; + } + if ($this->manyToOne !== null) { + return $this->manyToOne->targetEntity; + } + if ($this->oneToOne !== null && $this->oneToOne->invertedBy !== null) { + return $this->oneToOne->targetEntity; + } + return null; + } + + public function isRelation(): bool { + return $this->oneToOne !== null || $this->manyToOne !== null; + } } diff --git a/lib/private/Tagging/Tag.php b/lib/private/Tagging/Tag.php index 336ef4a38c282..66be51bbf392a 100644 --- a/lib/private/Tagging/Tag.php +++ b/lib/private/Tagging/Tag.php @@ -12,16 +12,16 @@ use OCP\AppFramework\ORM\Attribute\Entity; use OCP\AppFramework\ORM\Attribute\Id; use OCP\DB\Types; -use OCP\Snowflake\IGenerator; +use OCP\Snowflake\ISnowflakeGenerator; /** * Class to represent a tag. */ #[Entity(name: 'vcategory')] final class Tag { - #[Id(generatorClass: IGenerator::class)] + #[Id] #[Column(name: 'id', type: Types::BIGINT, nullable: false)] - public ?string $id = null; + public ?int $id = null; #[Column(name: 'uid', type: Types::STRING, length: 64, nullable: false)] public string $owner; diff --git a/lib/private/Tagging/TagMapper.php b/lib/private/Tagging/TagMapper.php index 2dd2b4946b5db..7f33d3e170b68 100644 --- a/lib/private/Tagging/TagMapper.php +++ b/lib/private/Tagging/TagMapper.php @@ -26,7 +26,7 @@ public function __construct(IDBConnection $db) { * * @param array $owners The user(s) whose tags we are going to load. * @param string $type The type of item for which we are loading tags. - * @return list An array of Tag objects. + * @return array An array of Tag objects. */ public function loadTags(array $owners, string $type): array { return iterator_to_array($this->findBy([ diff --git a/lib/private/Tags.php b/lib/private/Tags.php index 2029d558dcb81..54a456a84b204 100644 --- a/lib/private/Tags.php +++ b/lib/private/Tags.php @@ -28,16 +28,18 @@ class Tags implements ITags { * Used for storing objectid/categoryname pairs while rescanning. */ private array $relations = []; + /** @var list $tags */ private array $tags = []; /** * The current user, plus any owners of the items shared with the current * user, if $this->includeShared === true. + * @var list $owners */ private array $owners = []; - public const TAG_TABLE = 'vcategory'; - public const RELATION_TABLE = 'vcategory_to_object'; + public const string TAG_TABLE = 'vcategory'; + public const string RELATION_TABLE = 'vcategory_to_object'; /** * Constructor. @@ -68,25 +70,13 @@ public function __construct( } } - /** - * Check if any tags are saved for this type and user. - * - * @return boolean - */ #[\Override] public function isEmpty(): bool { return count($this->tags) === 0; } - /** - * Returns an array mapping a given tag's properties to its values: - * ['id' => 0, 'name' = 'Tag', 'owner' = 'User', 'type' => 'tagtype'] - * - * @param string $id The ID of the tag that is going to be mapped - * @return array|false - */ #[\Override] - public function getTag(string $id) { + public function getTag(string $id): array|false { $key = $this->getTagById($id); if ($key !== false) { return $this->tagMap($this->tags[$key]); @@ -94,30 +84,19 @@ public function getTag(string $id) { return false; } - /** - * Get the tags for a specific user. - * - * This returns an array with maps containing each tag's properties: - * [ - * ['id' => 0, 'name' = 'First tag', 'owner' = 'User', 'type' => 'tagtype'], - * ['id' => 1, 'name' = 'Shared tag', 'owner' = 'Other user', 'type' => 'tagtype'], - * ] - * - * @return array - */ #[\Override] public function getTags(): array { if (!count($this->tags)) { return []; } - usort($this->tags, function ($a, $b) { - return strnatcasecmp($a->getName(), $b->getName()); + usort($this->tags, function (Tag $a, Tag $b): int { + return strnatcasecmp($a->name, $b->name); }); $tagMap = []; foreach ($this->tags as $tag) { - if ($tag->getName() !== ITags::TAG_FAVORITE) { + if ($tag->name !== ITags::TAG_FAVORITE) { $tagMap[] = $this->tagMap($tag); } } @@ -133,8 +112,8 @@ public function getTags(): array { */ public function getTagsForUser(string $user): array { return array_filter($this->tags, - function ($tag) use ($user) { - return $tag->getOwner() === $user; + function (Tag $tag) use ($user) { + return $tag->owner === $user; } ); } @@ -147,7 +126,7 @@ function ($tag) use ($user) { * or false if an error occurred */ #[\Override] - public function getTagsForObjects(array $objIds) { + public function getTagsForObjects(array $objIds): array|false { $entries = []; try { @@ -169,7 +148,7 @@ public function getTagsForObjects(array $objIds) { if (!isset($entries[$objId])) { $entries[$objId] = []; } - $entries[$objId][] = $row['category']; + $entries[$objId][] = (string)$row['category']; } $result->closeCursor(); } @@ -184,21 +163,12 @@ public function getTagsForObjects(array $objIds) { return $entries; } - /** - * Get the a list if items tagged with $tag. - * - * Throws an exception if the tag could not be found. - * - * @param string $tag Tag id or name. - * @return int[]|false An array of object ids or false on error. - * @throws \Exception - */ #[\Override] - public function getIdsForTag($tag) { + public function getIdsForTag(int|string $tag): array|false { $tagId = false; if (is_numeric($tag)) { $tagId = $tag; - } elseif (is_string($tag)) { + } else { $tag = trim($tag); if ($tag === '') { $this->logger->debug(__METHOD__ . ' Cannot use empty tag names', ['app' => 'core']); @@ -237,36 +207,18 @@ public function getIdsForTag($tag) { return $ids; } - /** - * Checks whether a tag is saved for the given user, - * disregarding the ones shared with them. - * - * @param string $name The tag name to check for. - * @param string $user The user whose tags are to be checked. - */ #[\Override] public function userHasTag(string $name, string $user): bool { return $this->array_searchi($name, $this->getTagsForUser($user)) !== false; } - /** - * Checks whether a tag is saved for or shared with the current user. - * - * @param string $name The tag name to check for. - */ #[\Override] public function hasTag(string $name): bool { return $this->getTagId($name) !== false; } - /** - * Add a new tag. - * - * @param string $name A string with a name of the tag - * @return false|int the id of the added tag or false on error. - */ #[\Override] - public function add(string $name) { + public function add(string $name): false|int { $name = trim($name); if ($name === '') { @@ -278,7 +230,10 @@ public function add(string $name) { return false; } try { - $tag = new Tag($this->user, $this->type, $name); + $tag = new Tag(); + $tag->owner = $this->user; + $tag->type = $this->type; + $tag->name = $name; $tag = $this->mapper->insert($tag); $this->tags[] = $tag; } catch (\Exception $e) { @@ -288,20 +243,13 @@ public function add(string $name) { ]); return false; } - $this->logger->debug(__METHOD__ . ' Added an tag with ' . $tag->getId(), ['app' => 'core']); - return $tag->getId() ?? false; + $this->logger->debug(__METHOD__ . ' Added an tag with ' . $tag->id, ['app' => 'core']); + return $tag->id ?? false; } - /** - * Rename tag. - * - * @param string|integer $from The name or ID of the existing tag - * @param string $to The new name of the tag. - * @return bool - */ #[\Override] - public function rename($from, string $to): bool { - $from = trim($from); + public function rename(string|int $from, string $to): bool { + $from = trim((string)$from); $to = trim($to); if ($to === '' || $from === '') { @@ -320,13 +268,13 @@ public function rename($from, string $to): bool { } $tag = $this->tags[$key]; - if ($this->userHasTag($to, $tag->getOwner())) { - $this->logger->debug(__METHOD__ . 'A tag named' . $to . 'already exists for user' . $tag->getOwner(), ['app' => 'core']); + if ($this->userHasTag($to, $tag->owner)) { + $this->logger->debug(__METHOD__ . 'A tag named' . $to . 'already exists for user' . $tag->owner, ['app' => 'core']); return false; } try { - $tag->setName($to); + $tag->name = $to; $this->tags[$key] = $this->mapper->update($tag); } catch (\Exception $e) { $this->logger->error($e->getMessage(), [ @@ -338,17 +286,8 @@ public function rename($from, string $to): bool { return true; } - /** - * Add a list of new tags. - * - * @param string|string[] $names A string with a name or an array of strings containing - * the name(s) of the tag(s) to add. - * @param bool $sync When true, save the tags - * @param int|null $id int Optional object id to add to this|these tag(s) - * @return bool Returns false on error. - */ #[\Override] - public function addMultiple($names, bool $sync = false, ?int $id = null): bool { + public function addMultiple(string|array $names, bool $sync = false, ?int $id = null): bool { if (!is_array($names)) { $names = [$names]; } @@ -358,7 +297,11 @@ public function addMultiple($names, bool $sync = false, ?int $id = null): bool { $newones = []; foreach ($names as $name) { if (!$this->hasTag($name) && $name !== '') { - $newones[] = new Tag($this->user, $this->type, $name); + $tag = new Tag(); + $tag->owner = $this->user; + $tag->type = $this->type; + $tag->name = $name; + $newones[] = $tag; } if (!is_null($id)) { // Insert $objectid, $categoryid pairs if not exist. @@ -447,13 +390,8 @@ public function purgeObjects(array $ids): bool { return true; } - /** - * Get favorites for an object type - * - * @return array|false An array of object ids. - */ #[\Override] - public function getFavorites() { + public function getFavorites(): array|false { if (!$this->userHasTag(ITags::TAG_FAVORITE, $this->user)) { return []; } @@ -472,36 +410,21 @@ public function getFavorites() { } } - /** - * Add an object to favorites - * - * @param int $objid The id of the object - * @return boolean - */ #[\Override] - public function addToFavorites($objid) { + public function addToFavorites($objid): bool { if (!$this->userHasTag(ITags::TAG_FAVORITE, $this->user)) { $this->add(ITags::TAG_FAVORITE); } return $this->tagAs($objid, ITags::TAG_FAVORITE); } - /** - * Remove an object from favorites - * - * @param int $objid The id of the object - * @return boolean - */ #[\Override] - public function removeFromFavorites($objid) { + public function removeFromFavorites($objid): bool { return $this->unTag($objid, ITags::TAG_FAVORITE); } - /** - * Creates a tag/object relation. - */ #[\Override] - public function tagAs($objid, $tag, ?string $path = null) { + public function tagAs($objid, $tag, ?string $path = null): bool { if (is_string($tag) && !is_numeric($tag)) { $tag = trim($tag); if ($tag === '') { @@ -546,11 +469,8 @@ public function tagAs($objid, $tag, ?string $path = null) { return true; } - /** - * Delete single tag/object relation from the db - */ #[\Override] - public function unTag($objid, $tag, ?string $path = null) { + public function unTag($objid, $tag, ?string $path = null): bool { if (is_string($tag) && !is_numeric($tag)) { $tag = trim($tag); if ($tag === '') { @@ -592,19 +512,13 @@ public function unTag($objid, $tag, ?string $path = null) { return true; } - /** - * Delete tags from the database. - * - * @param string[]|integer[] $names An array of tags (names or IDs) to delete - * @return bool Returns false on error - */ #[\Override] - public function delete($names) { + public function delete(array|string|int $names): bool { if (!is_array($names)) { - $names = [$names]; + $names = [(string)$names]; } - $names = array_map('trim', $names); + $names = array_map('trim', array_map('strval', $names)); array_filter($names); $this->logger->debug(__METHOD__ . ', before: ' . print_r($this->tags, true)); @@ -618,13 +532,13 @@ public function delete($names) { } if ($key !== false) { $tag = $this->tags[$key]; - $id = $tag->getId(); + $id = $tag->id; unset($this->tags[$key]); $this->mapper->delete($tag); } else { $this->logger->error(__METHOD__ . 'Cannot delete tag ' . $name . ': not found.'); } - if (!is_null($id) && $id !== false) { + if ($id !== null) { try { $qb = $this->db->getQueryBuilder(); $qb->delete(self::RELATION_TABLE) @@ -643,13 +557,10 @@ public function delete($names) { } // case-insensitive array_search - protected function array_searchi($needle, $haystack, $mem = 'getName') { - if (!is_array($haystack)) { - return false; - } + protected function array_searchi(string $needle, array $haystack, $mem = 'name'): int|false { return array_search(strtolower($needle), array_map( function ($tag) use ($mem) { - return strtolower(call_user_func([$tag, $mem])); + return strtolower($tag->{$mem}); }, $haystack), true ); @@ -659,12 +570,12 @@ function ($tag) use ($mem) { * Get a tag's ID. * * @param string $name The tag name to look for. - * @return string|bool The tag's id or false if no matching tag is found. + * @return int|false The tag's id or false if no matching tag is found. */ - private function getTagId($name) { + private function getTagId(string $name): int|false { $key = $this->array_searchi($name, $this->tags); if ($key !== false) { - return $this->tags[$key]->getId(); + return $this->tags[$key]->id ?? -1; } return false; } @@ -673,22 +584,21 @@ private function getTagId($name) { * Get a tag by its name. * * @param string $name The tag name. - * @return integer|bool The tag object's offset within the $this->tags + * @return integer|false The tag object's offset within the $this->tags * array or false if it doesn't exist. */ - private function getTagByName($name) { - return $this->array_searchi($name, $this->tags, 'getName'); + private function getTagByName(string $name): int|false { + return $this->array_searchi($name, $this->tags); } /** * Get a tag by its ID. * * @param string $id The tag ID to look for. - * @return integer|bool The tag object's offset within the $this->tags - * array or false if it doesn't exist. + * @return integer|false The tag object's offset within the $this->tags array or false if it doesn't exist. */ - private function getTagById($id) { - return $this->array_searchi($id, $this->tags, 'getId'); + private function getTagById(string $id): int|false { + return $this->array_searchi($id, $this->tags, 'id'); } /** @@ -696,14 +606,14 @@ private function getTagById($id) { * ['id' => 0, 'name' = 'Tag', 'owner' = 'User', 'type' => 'tagtype'] * * @param Tag $tag The tag that is going to be mapped - * @return array + * @return array{id: ?int, name: string, owner: string, type: string} */ - private function tagMap(Tag $tag) { + private function tagMap(Tag $tag): array { return [ - 'id' => $tag->getId(), - 'name' => $tag->getName(), - 'owner' => $tag->getOwner(), - 'type' => $tag->getType() + 'id' => $tag->id, + 'name' => $tag->name, + 'owner' => $tag->owner, + 'type' => $tag->type ]; } } diff --git a/lib/public/AppFramework/ORM/Attribute/Column.php b/lib/public/AppFramework/ORM/Attribute/Column.php index 26a13a17ef2bb..31388da2b7fa0 100644 --- a/lib/public/AppFramework/ORM/Attribute/Column.php +++ b/lib/public/AppFramework/ORM/Attribute/Column.php @@ -2,11 +2,8 @@ declare(strict_types=1); -/** - * SPDX-FileCopyrightText: 2025 Nextcloud GmbH - * SPDX-FileContributor: Carl Schwan - * SPDX-License-Identifier: AGPL-3.0-or-later - */ +// SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors +// SPDX-License-Identifier: AGPL-3.0-or-later namespace OCP\AppFramework\ORM\Attribute; diff --git a/lib/public/AppFramework/ORM/Attribute/Entity.php b/lib/public/AppFramework/ORM/Attribute/Entity.php index 04229ec82e822..729f470d92358 100644 --- a/lib/public/AppFramework/ORM/Attribute/Entity.php +++ b/lib/public/AppFramework/ORM/Attribute/Entity.php @@ -2,11 +2,8 @@ declare(strict_types=1); -/** - * SPDX-FileCopyrightText: 2025 Nextcloud GmbH - * SPDX-FileContributor: Carl Schwan - * SPDX-License-Identifier: AGPL-3.0-or-later - */ +// SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors +// SPDX-License-Identifier: AGPL-3.0-or-later namespace OCP\AppFramework\ORM\Attribute; diff --git a/lib/public/AppFramework/ORM/Attribute/Id.php b/lib/public/AppFramework/ORM/Attribute/Id.php index 3bfb63efa2d47..0d13134e5b68b 100644 --- a/lib/public/AppFramework/ORM/Attribute/Id.php +++ b/lib/public/AppFramework/ORM/Attribute/Id.php @@ -2,11 +2,8 @@ declare(strict_types=1); -/** - * SPDX-FileCopyrightText: 2025 Nextcloud GmbH - * SPDX-FileContributor: Carl Schwan - * SPDX-License-Identifier: AGPL-3.0-or-later - */ +// SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors +// SPDX-License-Identifier: AGPL-3.0-or-later namespace OCP\AppFramework\ORM\Attribute; diff --git a/lib/public/AppFramework/ORM/Attribute/JoinColumn.php b/lib/public/AppFramework/ORM/Attribute/JoinColumn.php index f50995836cd50..c1bb5927faa8f 100644 --- a/lib/public/AppFramework/ORM/Attribute/JoinColumn.php +++ b/lib/public/AppFramework/ORM/Attribute/JoinColumn.php @@ -2,11 +2,8 @@ declare(strict_types=1); -/** - * SPDX-FileCopyrightText: 2025 Nextcloud GmbH - * SPDX-FileContributor: Carl Schwan - * SPDX-License-Identifier: AGPL-3.0-or-later - */ +// SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors +// SPDX-License-Identifier: AGPL-3.0-or-later namespace OCP\AppFramework\ORM\Attribute; diff --git a/lib/public/AppFramework/ORM/Attribute/ManyToOne.php b/lib/public/AppFramework/ORM/Attribute/ManyToOne.php new file mode 100644 index 0000000000000..0b0dfd064705d --- /dev/null +++ b/lib/public/AppFramework/ORM/Attribute/ManyToOne.php @@ -0,0 +1,50 @@ + $targetEntity */ + public string $targetEntity, + ) { + } +} diff --git a/lib/public/AppFramework/ORM/Attribute/OneToOne.php b/lib/public/AppFramework/ORM/Attribute/OneToOne.php index 3be76c218536a..4a4f0033f34ec 100644 --- a/lib/public/AppFramework/ORM/Attribute/OneToOne.php +++ b/lib/public/AppFramework/ORM/Attribute/OneToOne.php @@ -2,11 +2,8 @@ declare(strict_types=1); -/** - * SPDX-FileCopyrightText: 2026 Nextcloud GmbH - * SPDX-FileContributor: Carl Schwan - * SPDX-License-Identifier: AGPL-3.0-or-later - */ +// SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors +// SPDX-License-Identifier: AGPL-3.0-or-later namespace OCP\AppFramework\ORM\Attribute; diff --git a/lib/public/AppFramework/ORM/Repository.php b/lib/public/AppFramework/ORM/Repository.php index f84299d43aa2c..4735d68b34acc 100644 --- a/lib/public/AppFramework/ORM/Repository.php +++ b/lib/public/AppFramework/ORM/Repository.php @@ -11,7 +11,6 @@ use OC\AppFramework\ORM\EntityManager; use OC\AppFramework\ORM\PropertyAttributes; use OCP\AppFramework\ORM\Attribute\Id; -use OCP\AppFramework\ORM\Attribute\OneToOne; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\DB\Exception; @@ -110,7 +109,7 @@ private function hydrateRow(string $entityClass, mixed $row): object { } foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { - if ($propertyAttributes->oneToOne !== null) { + if ($propertyAttributes->isRelation()) { $entity->{$propertyAttributes->property->getName()} = null; } } @@ -119,7 +118,7 @@ private function hydrateRow(string $entityClass, mixed $row): object { } /** - * Builds a select query resolving both sides of any OneToOne relation via a LEFT JOIN. + * Builds a select query resolving OneToOne and ManyToOne relations via a LEFT JOIN. * Columns are aliased `e_` (main entity) and `r_` (each relation) * to stay unique even when tables share column names. * @@ -137,13 +136,14 @@ private function buildJoinedSelectQuery(EntityInfo $entityInfo): array { $relations = []; $index = 0; foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { - if ($propertyAttributes->oneToOne === null) { + if (!$propertyAttributes->isRelation()) { continue; } - if ($propertyAttributes->joinColumn !== null && $propertyAttributes->oneToOne->invertedBy !== null) { - // Owning side: the join column lives on our own table. - $targetEntityInfo = $this->entityManager->getEntityInfo($propertyAttributes->oneToOne->targetEntity); + $owningTargetClass = $propertyAttributes->getOwningRelationTarget(); + if ($owningTargetClass !== null) { + // Owning side (OneToOne's invertedBy, or ManyToOne): the join column lives on our own table. + $targetEntityInfo = $this->entityManager->getEntityInfo($owningTargetClass); $alias = 'r' . $index++; $this->joinRelation( @@ -158,7 +158,7 @@ private function buildJoinedSelectQuery(EntityInfo $entityInfo): array { continue; } - if ($propertyAttributes->oneToOne->mappedBy !== null) { + if ($propertyAttributes->oneToOne !== null && $propertyAttributes->oneToOne->mappedBy !== null) { // Inverse side: the join column lives on the target's table, pointing back at us. $targetEntityInfo = $this->entityManager->getEntityInfo($propertyAttributes->oneToOne->targetEntity); @@ -235,15 +235,13 @@ private function mapJoinedRowToEntity(array $relations, mixed $row): object { continue; } - /** @var OneToOne $oneToOne */ - $oneToOne = $relation['attributes']->oneToOne; - $entity->$propertyName = $this->hydrateRow($oneToOne->targetEntity, $relationRow); + $entity->$propertyName = $this->hydrateRow($targetEntityInfo->entityClass, $relationRow); } // Safety net for a malformed mapping that never made it into $relations. $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { - if ($propertyAttributes->oneToOne === null) { + if (!$propertyAttributes->isRelation()) { continue; } @@ -336,7 +334,7 @@ public function delete(object $entity): void { * Finds entities by a set of criteria, keyed by property name. * * @param array> $criteria - * @param array|null $orderBy + * @param array $orderBy * @return \Generator */ public function findBy(array $criteria, array $orderBy = [], ?int $limit = null, ?int $offset = null): \Generator { @@ -389,7 +387,7 @@ public function deleteBy(array $criteria, ?int $limit = null): int { * Finds a single entity by a set of criteria, keyed by property name. * * @param array> $criteria - * @param array|null $orderBy + * @param array $orderBy * @return T * @throws DoesNotExistException */ @@ -403,10 +401,10 @@ public function findOneBy(array $criteria, array $orderBy = []): object { /** * @param array> $criteria - * @param array|null $orderBy + * @param array $orderBy * @return array{0: IQueryBuilder, 1: array} */ - private function getJoinedSelectQueryBuilder(array $criteria, array $orderBy = []): array { + private function getJoinedSelectQueryBuilder(array $criteria, ?array $orderBy = []): array { $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); [$qb, $relations] = $this->buildJoinedSelectQuery($entityInfo); diff --git a/lib/public/ITags.php b/lib/public/ITags.php index 7c32224405bd7..88275a33cb741 100644 --- a/lib/public/ITags.php +++ b/lib/public/ITags.php @@ -28,7 +28,7 @@ interface ITags { /** * @since 19.0.0 */ - public const TAG_FAVORITE = '_$!!$_'; + public const string TAG_FAVORITE = '_$!!$_'; /** * Check if any tags are saved for this type and user. @@ -41,7 +41,7 @@ public function isEmpty(): bool; * ['id' => 0, 'name' = 'Tag', 'owner' = 'User', 'type' => 'tagtype'] * * @param string $id The ID of the tag that is going to be mapped - * @return array|false + * @return array{id: ?int, name: string, owner: string, type: string}|false * @since 8.0.0 */ public function getTag(string $id); @@ -53,12 +53,12 @@ public function getTag(string $id); * * ```php * [ - * ['id' => 0, 'name' = 'First tag'], - * ['id' => 1, 'name' = 'Second tag'], + * ['id' => 0, 'name' = 'First tag', 'owner' = 'User A', 'type' => 'tagtype'], + * ['id' => 1, 'name' = 'Second tag', 'owner' = 'User B', 'type' => 'tagtype'], * ] * ``` * - * @return array + * @return list * @since 6.0.0 */ public function getTags(): array; @@ -81,7 +81,7 @@ public function getTags(): array; * of tag names as value or false if an error occurred * @since 8.0.0 */ - public function getTagsForObjects(array $objIds); + public function getTagsForObjects(array $objIds): array|false; /** * Get a list of items tagged with $tag. @@ -92,7 +92,7 @@ public function getTagsForObjects(array $objIds); * @return array|false An array of object ids or false on error. * @since 6.0.0 */ - public function getIdsForTag($tag); + public function getIdsForTag(string|int $tag): array|false; /** * Checks whether a tag is already saved. @@ -130,19 +130,19 @@ public function add(string $name); * @return bool * @since 6.0.0 */ - public function rename($from, string $to): bool; + public function rename(string|int $from, string $to): bool; /** * Add a list of new tags. * - * @param string|string[] $names A string with a name or an array of strings containing + * @param string|list $names A string with a name or an array of strings containing * the name(s) of the to add. * @param bool $sync When true, save the tags * @param int|null $id int Optional object id to add to this|these tag(s) * @return bool Returns false on error. * @since 6.0.0 */ - public function addMultiple($names, bool $sync = false, ?int $id = null): bool; + public function addMultiple(string|array $names, bool $sync = false, ?int $id = null): bool; /** * Delete tag/object relations from the db @@ -208,9 +208,9 @@ public function unTag($objid, $tag, ?string $path = null); /** * Delete tags from the database * - * @param string[]|integer[] $names An array of tags (names or IDs) to delete + * @param string[]|integer[]|string|int $names An array of tags (names or IDs) to delete * @return bool Returns false on error * @since 6.0.0 */ - public function delete($names); + public function delete(array|string|int $names): bool; } diff --git a/tests/lib/AppFramework/ORM/RepositoryTest.php b/tests/lib/AppFramework/ORM/RepositoryTest.php index 2507413d1f21b..06f366454da6a 100644 --- a/tests/lib/AppFramework/ORM/RepositoryTest.php +++ b/tests/lib/AppFramework/ORM/RepositoryTest.php @@ -14,6 +14,7 @@ use OCP\AppFramework\ORM\Attribute\Entity; use OCP\AppFramework\ORM\Attribute\Id; use OCP\AppFramework\ORM\Attribute\JoinColumn; +use OCP\AppFramework\ORM\Attribute\ManyToOne; use OCP\AppFramework\ORM\Attribute\OneToOne; use OCP\AppFramework\ORM\Repository; use OCP\DB\QueryBuilder\IQueryBuilder; @@ -147,6 +148,27 @@ final class CascadeChild { public CascadeParent|null $parent = null; } +#[Entity(name: 'repository_merchant')] +final class Merchant { + #[Id] + #[Column(name: 'id', type: Types::BIGINT)] + public ?int $id = null; + + #[Column(name: 'name', type: Types::STRING, nullable: true)] + public ?string $name = null; +} + +#[Entity(name: 'repository_order')] +final class Order { + #[Id] + #[Column(name: 'id', type: Types::BIGINT)] + public ?int $id = null; + + #[ManyToOne(targetEntity: Merchant::class)] + #[JoinColumn(name: 'merchant_id', referencedColumnName: 'id', nullable: true)] + public Merchant|null $merchant = null; +} + #[\PHPUnit\Framework\Attributes\Group('DB')] class RepositoryTest extends TestCase { /** @var list */ @@ -157,6 +179,8 @@ class RepositoryTest extends TestCase { Cart::class, CascadeParent::class, CascadeChild::class, + Merchant::class, + Order::class, ]; public static function setUpBeforeClass(): void { @@ -420,6 +444,42 @@ public function testDeleteCascadesWhenConfigured(): void { $this->assertCount(0, $entities); } + public function testManyToOne(): void { + $merchantRepo = $this->getRepository(Merchant::class); + $orderRepo = $this->getRepository(Order::class); + + $merchant = new Merchant(); + $merchant->name = 'Acme'; + $merchantRepo->insert($merchant); + + $order1 = new Order(); + $order1->merchant = $merchant; + $orderRepo->insert($order1); + + // Unlike OneToOne, a second Order for the same Merchant must be allowed. + $order2 = new Order(); + $order2->merchant = $merchant; + $orderRepo->insert($order2); + + $savedOrder1 = $orderRepo->findOneBy(['id' => $order1->id]); + $this->assertNotNull($savedOrder1->merchant); + $this->assertEquals($merchant->id, $savedOrder1->merchant->id); + $this->assertEquals($merchant->name, $savedOrder1->merchant->name); + + $savedOrder2 = $orderRepo->findOneBy(['id' => $order2->id]); + $this->assertEquals($merchant->id, $savedOrder2->merchant->id); + } + + public function testManyToOneNullRelation(): void { + $orderRepo = $this->getRepository(Order::class); + + $order = new Order(); + $orderRepo->insert($order); + + $savedOrder = $orderRepo->findOneBy(['id' => $order->id]); + $this->assertNull($savedOrder->merchant); + } + private function normalizeSql(string $sql): string { return str_replace(['`', '"'], '', $sql); } @@ -441,6 +501,23 @@ public function testOwningSideGeneratesLeftJoin(): void { ); } + public function testManyToOneGeneratesLeftJoin(): void { + $orderRepo = $this->getRepository(Order::class); + + /** @var array{0: IQueryBuilder, 1: array} $result */ + $result = self::invokePrivate($orderRepo, 'getJoinedSelectQueryBuilder', [['id' => 1], []]); + [$qb, $relations] = $result; + + $this->assertCount(1, $relations); + $this->assertEquals( + 'SELECT e.id AS e_id, r0.id AS r0_id, r0.name AS r0_name ' + . 'FROM *PREFIX*repository_order e ' + . 'LEFT JOIN *PREFIX*repository_merchant r0 ON e.merchant_id = r0.id ' + . 'WHERE e.id = :dcValue1', + $this->normalizeSql($qb->getSQL()), + ); + } + public function testInverseSideGeneratesLeftJoin(): void { $customerRepo = $this->getRepository(Customer::class); From 4fe37800c9b8317915b63d1dfcd23d54e00fd4e7 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 21 Jul 2026 15:28:59 +0200 Subject: [PATCH 07/10] refactor: Don't check if ISnowflakeGenerator exists This is a left over from the maps prototype Signed-off-by: Carl Schwan --- lib/private/AppFramework/ORM/EntityManager.php | 5 ++--- lib/private/Tags.php | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/private/AppFramework/ORM/EntityManager.php b/lib/private/AppFramework/ORM/EntityManager.php index 4ab208f9b2551..44059ffb6d8fc 100644 --- a/lib/private/AppFramework/ORM/EntityManager.php +++ b/lib/private/AppFramework/ORM/EntityManager.php @@ -70,9 +70,8 @@ public function insert(object $entity): object { $primaryProperty = $property; $generatorClass = $propertyAttributes->id->generatorClass; if ($generatorClass) { - $generator = Server::get($generatorClass); - /** @psalm-suppress UndefinedClass NC 33 and above */ - if (class_exists(ISnowflakeGenerator::class) && $generator instanceof ISnowflakeGenerator) { + if ($generatorClass === ISnowflakeGenerator::class) { + $generator = Server::get($generatorClass); $isSnowflake = true; /** @psalm-suppress UndefinedClass */ $values[$propertyAttributes->column->name] = $generator->nextId(); diff --git a/lib/private/Tags.php b/lib/private/Tags.php index 54a456a84b204..fdcb20488501a 100644 --- a/lib/private/Tags.php +++ b/lib/private/Tags.php @@ -165,7 +165,6 @@ public function getTagsForObjects(array $objIds): array|false { #[\Override] public function getIdsForTag(int|string $tag): array|false { - $tagId = false; if (is_numeric($tag)) { $tagId = $tag; } else { From beae168d18cee9fd36cc9d2dd6eae3b85c48a1f1 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 6 Aug 2026 10:46:54 +0200 Subject: [PATCH 08/10] fix(ORM): Address feedback from review Signed-off-by: Carl Schwan --- .../lib/Db/BackupCodeMapper.php | 5 +- .../Unit/Service/BackupCodeStorageTest.php | 6 ++- .../AppFramework/ORM/EntityManager.php | 9 +++- lib/private/Tagging/Tag.php | 1 - lib/private/Tagging/TagMapper.php | 5 +- lib/private/Tags.php | 2 +- .../AppFramework/ORM/Attribute/Column.php | 1 + .../AppFramework/ORM/Attribute/Entity.php | 3 +- lib/public/AppFramework/ORM/Attribute/Id.php | 1 + .../AppFramework/ORM/Attribute/JoinColumn.php | 1 + .../AppFramework/ORM/Attribute/ManyToOne.php | 1 + .../AppFramework/ORM/Attribute/OneToOne.php | 3 ++ lib/public/AppFramework/ORM/Repository.php | 54 ++++++++++++++----- lib/public/ITags.php | 2 +- tests/lib/AppFramework/ORM/RepositoryTest.php | 44 +++++++-------- 15 files changed, 89 insertions(+), 49 deletions(-) diff --git a/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php b/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php index 0bbc5b520ecbd..68bbe03500d7f 100644 --- a/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php +++ b/apps/twofactor_backupcodes/lib/Db/BackupCodeMapper.php @@ -12,16 +12,13 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\ORM\Repository; use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\IDBConnection; use OCP\IUser; /** * @template-extends Repository */ class BackupCodeMapper extends Repository { - public function __construct(IDBConnection $db) { - parent::__construct($db, BackupCode::class); - } + public const string entityClass = BackupCode::class; /** * @return \Generator diff --git a/apps/twofactor_backupcodes/tests/Unit/Service/BackupCodeStorageTest.php b/apps/twofactor_backupcodes/tests/Unit/Service/BackupCodeStorageTest.php index 2b61bcacf5c6f..f4ce957f1efc7 100644 --- a/apps/twofactor_backupcodes/tests/Unit/Service/BackupCodeStorageTest.php +++ b/apps/twofactor_backupcodes/tests/Unit/Service/BackupCodeStorageTest.php @@ -124,7 +124,11 @@ public function testGetBackupCodeDisabled(): void { $this->mapper->expects($this->once()) ->method('findByUser') ->with($user) - ->willReturnCallback(function () { if (false) { yield true; }}); + ->willReturnCallback(function () { + if (false) { + yield true; + } + }); $expected = [ 'enabled' => false, diff --git a/lib/private/AppFramework/ORM/EntityManager.php b/lib/private/AppFramework/ORM/EntityManager.php index 44059ffb6d8fc..3ba8aa4f11080 100644 --- a/lib/private/AppFramework/ORM/EntityManager.php +++ b/lib/private/AppFramework/ORM/EntityManager.php @@ -21,7 +21,7 @@ class EntityManager { public function __construct( - readonly private IDBConnection $connection, + private readonly IDBConnection $connection, ) { } @@ -43,12 +43,17 @@ public function getEntityInfo(string $entityClass): EntityInfo { } /** + * Generic, runtime-typed repository factory for callers that only have the entity class as + * a value (e.g. tests). Hand-written repositories (e.g. BackupCodeMapper) should instead + * extend Repository and override its `entityClass` constant, which also gets them proper + * static analysis of their entity type. + * * @template T of object * @param class-string $entityClass * @return Repository */ public function getRepository(string $entityClass): Repository { - return new Repository($this->connection, $entityClass); + return new Repository($this->connection, $this, $entityClass); } /** diff --git a/lib/private/Tagging/Tag.php b/lib/private/Tagging/Tag.php index 66be51bbf392a..06c06edc818d9 100644 --- a/lib/private/Tagging/Tag.php +++ b/lib/private/Tagging/Tag.php @@ -12,7 +12,6 @@ use OCP\AppFramework\ORM\Attribute\Entity; use OCP\AppFramework\ORM\Attribute\Id; use OCP\DB\Types; -use OCP\Snowflake\ISnowflakeGenerator; /** * Class to represent a tag. diff --git a/lib/private/Tagging/TagMapper.php b/lib/private/Tagging/TagMapper.php index 7f33d3e170b68..746ffc426da0f 100644 --- a/lib/private/Tagging/TagMapper.php +++ b/lib/private/Tagging/TagMapper.php @@ -9,7 +9,6 @@ namespace OC\Tagging; use OCP\AppFramework\ORM\Repository; -use OCP\IDBConnection; /** * Mapper for Tag entity @@ -17,9 +16,7 @@ * @template-extends Repository */ class TagMapper extends Repository { - public function __construct(IDBConnection $db) { - parent::__construct($db, Tag::class); - } + public const string entityClass = Tag::class; /** * Load tags from the database. diff --git a/lib/private/Tags.php b/lib/private/Tags.php index fdcb20488501a..f0120ac1d1049 100644 --- a/lib/private/Tags.php +++ b/lib/private/Tags.php @@ -584,7 +584,7 @@ private function getTagId(string $name): int|false { * * @param string $name The tag name. * @return integer|false The tag object's offset within the $this->tags - * array or false if it doesn't exist. + * array or false if it doesn't exist. */ private function getTagByName(string $name): int|false { return $this->array_searchi($name, $this->tags); diff --git a/lib/public/AppFramework/ORM/Attribute/Column.php b/lib/public/AppFramework/ORM/Attribute/Column.php index 31388da2b7fa0..0eaeb5f34a60e 100644 --- a/lib/public/AppFramework/ORM/Attribute/Column.php +++ b/lib/public/AppFramework/ORM/Attribute/Column.php @@ -27,6 +27,7 @@ #[Attribute(Attribute::TARGET_PROPERTY)] #[Consumable(since: '35.0.0')] final readonly class Column { + /** @since 35.0.0 */ public function __construct( /** @param non-empty-string $name The name of the column in the database. */ public string $name, diff --git a/lib/public/AppFramework/ORM/Attribute/Entity.php b/lib/public/AppFramework/ORM/Attribute/Entity.php index 729f470d92358..de794d0a04d17 100644 --- a/lib/public/AppFramework/ORM/Attribute/Entity.php +++ b/lib/public/AppFramework/ORM/Attribute/Entity.php @@ -26,8 +26,9 @@ #[Attribute(Attribute::TARGET_CLASS)] #[Consumable(since: '35.0.0')] final readonly class Entity { + /** @since 35.0.0 */ public function __construct( - /** @param non-empty-string $name */ + /** @param non-empty-string $name The name of the table in the database. */ public string $name, ) { } diff --git a/lib/public/AppFramework/ORM/Attribute/Id.php b/lib/public/AppFramework/ORM/Attribute/Id.php index 0d13134e5b68b..125ccfbdf31a0 100644 --- a/lib/public/AppFramework/ORM/Attribute/Id.php +++ b/lib/public/AppFramework/ORM/Attribute/Id.php @@ -28,6 +28,7 @@ #[Attribute(Attribute::TARGET_PROPERTY)] #[Consumable(since: '35.0.0')] final readonly class Id { + /** @since 35.0.0 */ public function __construct( /** @param class-string $generatorClass */ public ?string $generatorClass = null, diff --git a/lib/public/AppFramework/ORM/Attribute/JoinColumn.php b/lib/public/AppFramework/ORM/Attribute/JoinColumn.php index c1bb5927faa8f..e01f19614de44 100644 --- a/lib/public/AppFramework/ORM/Attribute/JoinColumn.php +++ b/lib/public/AppFramework/ORM/Attribute/JoinColumn.php @@ -27,6 +27,7 @@ #[Attribute(Attribute::TARGET_PROPERTY)] #[Consumable(since: '35.0.0')] final readonly class JoinColumn { + /** @since 35.0.0 */ public function __construct( /** @param non-empty-string $name The name of the column in the database. */ public string $name, diff --git a/lib/public/AppFramework/ORM/Attribute/ManyToOne.php b/lib/public/AppFramework/ORM/Attribute/ManyToOne.php index 0b0dfd064705d..32ead782c2c20 100644 --- a/lib/public/AppFramework/ORM/Attribute/ManyToOne.php +++ b/lib/public/AppFramework/ORM/Attribute/ManyToOne.php @@ -42,6 +42,7 @@ #[Attribute(Attribute::TARGET_PROPERTY)] #[Consumable(since: '35.0.0')] final readonly class ManyToOne { + /** @since 35.0.0 */ public function __construct( /** @param class-string $targetEntity */ public string $targetEntity, diff --git a/lib/public/AppFramework/ORM/Attribute/OneToOne.php b/lib/public/AppFramework/ORM/Attribute/OneToOne.php index 4a4f0033f34ec..f38e8edaf5c53 100644 --- a/lib/public/AppFramework/ORM/Attribute/OneToOne.php +++ b/lib/public/AppFramework/ORM/Attribute/OneToOne.php @@ -66,6 +66,9 @@ #[Attribute(Attribute::TARGET_PROPERTY)] #[Consumable(since: '35.0.0')] final readonly class OneToOne { + /** + * @since 35.0.0 + */ public function __construct( /** @param class-string $targetEntity */ public string $targetEntity, diff --git a/lib/public/AppFramework/ORM/Repository.php b/lib/public/AppFramework/ORM/Repository.php index 4735d68b34acc..c9304dea1b457 100644 --- a/lib/public/AppFramework/ORM/Repository.php +++ b/lib/public/AppFramework/ORM/Repository.php @@ -10,31 +10,50 @@ use OC\AppFramework\ORM\EntityInfo; use OC\AppFramework\ORM\EntityManager; use OC\AppFramework\ORM\PropertyAttributes; -use OCP\AppFramework\ORM\Attribute\Id; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; +use OCP\AppFramework\ORM\Attribute\Id; use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\DB\Types; use OCP\IDBConnection; -use OCP\Server; /** * @template T as object * @since 35.0.0 */ class Repository { - private readonly EntityManager $entityManager; + /** + * The class this repository holds. + * @var class-string $entityClass + * @since 35.0.0 + * @psalm-suppress InvalidConstantAssignmentValue + */ + public const string entityClass = ''; /** - * @param class-string $entityClass + * @param class-string|null $entityClassOverride Only meant for generic, runtime-typed + * repositories (e.g. EntityManager::getRepository()). + * Hand-written subclasses should override the + * `entityClass` constant instead and leave this null. * @throws \ReflectionException + * @internal + * @since 35.0.0 */ public function __construct( protected readonly IDBConnection $connection, - protected readonly string $entityClass, + protected readonly EntityManager $entityManager, + private readonly ?string $entityClassOverride = null, ) { - $this->entityManager = Server::get(EntityManager::class); + } + + /** + * @return class-string + */ + private function getEntityClass(): string { + // static:: so a subclass's overridden constant is picked up rather than this base + // class's empty default. + return $this->entityClassOverride ?? static::entityClass; } private function buildDebugMessage(string $msg, IQueryBuilder $sql): string { @@ -222,7 +241,7 @@ private function mapJoinedRowToEntity(array $relations, mixed $row): object { } } - $entity = $this->hydrateRow($this->entityClass, $mainRow); + $entity = $this->hydrateRow($this->getEntityClass(), $mainRow); foreach ($relations as $alias => $relation) { $propertyName = $relation['attributes']->property->getName(); @@ -239,7 +258,7 @@ private function mapJoinedRowToEntity(array $relations, mixed $row): object { } // Safety net for a malformed mapping that never made it into $relations. - $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); + $entityInfo = $this->entityManager->getEntityInfo($this->getEntityClass()); foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { if (!$propertyAttributes->isRelation()) { continue; @@ -310,6 +329,7 @@ private function findJoinedEntity(IQueryBuilder $query, array $relations): objec * * @psalm-param T $entity * @return T + * @since 35.0.0 */ public function insert(object $entity): object { return $this->entityManager->insert($entity); @@ -318,6 +338,7 @@ public function insert(object $entity): object { /** * @psalm-param T $entity * @return T + * @since 35.0.0 */ public function update(object $entity): object { return $this->entityManager->update($entity); @@ -325,6 +346,7 @@ public function update(object $entity): object { /** * @psalm-param T $entity + * @since 35.0.0 */ public function delete(object $entity): void { $this->entityManager->delete($entity); @@ -336,6 +358,7 @@ public function delete(object $entity): void { * @param array> $criteria * @param array $orderBy * @return \Generator + * @since 35.0.0 */ public function findBy(array $criteria, array $orderBy = [], ?int $limit = null, ?int $offset = null): \Generator { [$qb, $relations] = $this->getJoinedSelectQueryBuilder($criteria, $orderBy); @@ -355,9 +378,10 @@ public function findBy(array $criteria, array $orderBy = [], ?int $limit = null, * @param array> $criteria * @return int The number of rows deleted * @throws Exception + * @since 35.0.0 */ public function deleteBy(array $criteria, ?int $limit = null): int { - $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); + $entityInfo = $this->entityManager->getEntityInfo($this->getEntityClass()); $qb = $this->connection->getQueryBuilder(); $qb->delete($entityInfo->tableName); @@ -390,6 +414,7 @@ public function deleteBy(array $criteria, ?int $limit = null): int { * @param array $orderBy * @return T * @throws DoesNotExistException + * @since 35.0.0 */ public function findOneBy(array $criteria, array $orderBy = []): object { [$qb, $relations] = $this->getJoinedSelectQueryBuilder($criteria, $orderBy); @@ -405,7 +430,7 @@ public function findOneBy(array $criteria, array $orderBy = []): object { * @return array{0: IQueryBuilder, 1: array} */ private function getJoinedSelectQueryBuilder(array $criteria, ?array $orderBy = []): array { - $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); + $entityInfo = $this->entityManager->getEntityInfo($this->getEntityClass()); [$qb, $relations] = $this->buildJoinedSelectQuery($entityInfo); foreach ($criteria as $property => $value) { @@ -431,16 +456,21 @@ private function getJoinedSelectQueryBuilder(array $criteria, ?array $orderBy = /** * @return \Generator * @throws Exception + * @since 35.0.0 */ public function yieldAll(): \Generator { - $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); + $entityInfo = $this->entityManager->getEntityInfo($this->getEntityClass()); [$qb, $relations] = $this->buildJoinedSelectQuery($entityInfo); return $this->yieldJoinedEntities($qb, $relations); } + /** + * @internal + * @since 35.0.0 + */ public function getTableName(): string { - $entityInfo = $this->entityManager->getEntityInfo($this->entityClass); + $entityInfo = $this->entityManager->getEntityInfo($this->getEntityClass()); return $entityInfo->tableName; } } diff --git a/lib/public/ITags.php b/lib/public/ITags.php index 88275a33cb741..4f7aff94c0f0c 100644 --- a/lib/public/ITags.php +++ b/lib/public/ITags.php @@ -136,7 +136,7 @@ public function rename(string|int $from, string $to): bool; * Add a list of new tags. * * @param string|list $names A string with a name or an array of strings containing - * the name(s) of the to add. + * the name(s) of the to add. * @param bool $sync When true, save the tags * @param int|null $id int Optional object id to add to this|these tag(s) * @return bool Returns false on error. diff --git a/tests/lib/AppFramework/ORM/RepositoryTest.php b/tests/lib/AppFramework/ORM/RepositoryTest.php index 06f366454da6a..26e9cd9731df5 100644 --- a/tests/lib/AppFramework/ORM/RepositoryTest.php +++ b/tests/lib/AppFramework/ORM/RepositoryTest.php @@ -55,28 +55,28 @@ class PrimaryKey { } #[Entity(name: 'repository_customer')] - final class Customer { - #[Id] - #[Column(name: 'id', type: Types::BIGINT)] - public ?int $id = null; +final class Customer { + #[Id] + #[Column(name: 'id', type: Types::BIGINT)] + public ?int $id = null; - #[OneToOne(targetEntity: Cart::class, mappedBy: 'customer')] - #[JoinColumn(name: 'cart_id', referencedColumnName: 'id')] - public Cart|null $cart = null; + #[OneToOne(targetEntity: Cart::class, mappedBy: 'customer')] + #[JoinColumn(name: 'cart_id', referencedColumnName: 'id')] + public ?Cart $cart = null; #[Column(name: 'name', type: Types::STRING, nullable: false)] public string $name; } - #[Entity(name: 'repository_cart')] - final class Cart { - #[Id] - #[Column(name: 'id', type: Types::BIGINT)] - public ?int $id = null; +#[Entity(name: 'repository_cart')] +final class Cart { + #[Id] + #[Column(name: 'id', type: Types::BIGINT)] + public ?int $id = null; - #[OneToOne(targetEntity: Customer::class, invertedBy: 'cart')] - #[JoinColumn(name: 'customer_id', referencedColumnName: 'id')] - public Customer|null $customer; + #[OneToOne(targetEntity: Customer::class, invertedBy: 'cart')] + #[JoinColumn(name: 'customer_id', referencedColumnName: 'id')] + public ?Customer $customer; } #[Entity(name: 'repository_invalid_owner')] @@ -87,7 +87,7 @@ final class InvalidOwningOnDelete { #[OneToOne(targetEntity: InvalidMappedByOnDelete::class, invertedBy: 'owner')] #[JoinColumn(name: 'invalid_id', referencedColumnName: 'id')] - public InvalidMappedByOnDelete|null $invalid = null; + public ?InvalidMappedByOnDelete $invalid = null; } #[Entity(name: 'repository_invalid_mapped')] @@ -98,7 +98,7 @@ final class InvalidMappedByOnDelete { #[OneToOne(targetEntity: InvalidOwningOnDelete::class, mappedBy: 'invalid')] #[JoinColumn(name: 'owner_id', referencedColumnName: 'id', onDelete: 'CASCADE')] - public InvalidOwningOnDelete|null $owner = null; + public ?InvalidOwningOnDelete $owner = null; } #[Entity(name: 'repository_typo_owner')] @@ -109,7 +109,7 @@ final class TypoOwning { #[OneToOne(targetEntity: TypoTarget::class, invertedBy: 'owner')] #[JoinColumn(name: 'target_id', referencedColumnName: 'id')] - public TypoTarget|null $target = null; + public ?TypoTarget $target = null; } #[Entity(name: 'repository_typo_target')] @@ -120,7 +120,7 @@ final class TypoTarget { #[OneToOne(targetEntity: TypoOwning::class, mappedBy: 'ownerTypo')] #[JoinColumn(name: 'owner_id', referencedColumnName: 'id')] - public TypoOwning|null $owner = null; + public ?TypoOwning $owner = null; } #[Entity(name: 'repository_cascade_parent')] @@ -134,7 +134,7 @@ final class CascadeParent { #[OneToOne(targetEntity: CascadeChild::class, mappedBy: 'parent')] #[JoinColumn(name: 'child_id', referencedColumnName: 'id')] - public CascadeChild|null $child = null; + public ?CascadeChild $child = null; } #[Entity(name: 'repository_cascade_child')] @@ -145,7 +145,7 @@ final class CascadeChild { #[OneToOne(targetEntity: CascadeParent::class, invertedBy: 'child')] #[JoinColumn(name: 'parent_id', referencedColumnName: 'id', onDelete: 'CASCADE')] - public CascadeParent|null $parent = null; + public ?CascadeParent $parent = null; } #[Entity(name: 'repository_merchant')] @@ -166,7 +166,7 @@ final class Order { #[ManyToOne(targetEntity: Merchant::class)] #[JoinColumn(name: 'merchant_id', referencedColumnName: 'id', nullable: true)] - public Merchant|null $merchant = null; + public ?Merchant $merchant = null; } #[\PHPUnit\Framework\Attributes\Group('DB')] From 358471277cfbea2bf5e6463adada885d5a9f1bd5 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 6 Aug 2026 11:53:23 +0200 Subject: [PATCH 09/10] refactor: Use psalm:strict for AppFramework/ORM Signed-off-by: Carl Schwan --- build/rector-strict.php | 2 + lib/private/AppFramework/ORM/EntityInfo.php | 43 ++++++----- .../AppFramework/ORM/EntityManager.php | 69 ++++++++++-------- .../AppFramework/ORM/PropertyAttributes.php | 20 ++++-- lib/private/Sharing/SharingManager.php | 2 +- .../AppFramework/ORM/Attribute/Column.php | 10 +-- .../AppFramework/ORM/Attribute/Entity.php | 2 +- lib/public/AppFramework/ORM/Attribute/Id.php | 2 +- .../AppFramework/ORM/Attribute/JoinColumn.php | 9 ++- .../AppFramework/ORM/Attribute/ManyToOne.php | 2 +- .../AppFramework/ORM/Attribute/OneToOne.php | 6 +- lib/public/AppFramework/ORM/Repository.php | 71 +++++++++++++------ psalm-strict.xml | 5 +- tests/lib/AppFramework/ORM/RepositoryTest.php | 8 +-- tests/lib/TagsTest.php | 2 +- 15 files changed, 158 insertions(+), 95 deletions(-) diff --git a/build/rector-strict.php b/build/rector-strict.php index db161bfd07bb0..a0c89b6b04ddb 100644 --- a/build/rector-strict.php +++ b/build/rector-strict.php @@ -47,6 +47,8 @@ $nextcloudDir . '/tests/Core/Sharing', $nextcloudDir . '/apps/files/lib/Sharing', $nextcloudDir . '/apps/files/tests/Sharing', + $nextcloudDir . '/lib/public/AppFramework/ORM', + $nextcloudDir . '/lib/private/AppFramework/ORM', ]) ->withAutoloadPaths([ // ensure rector properly autoload the public interfaces diff --git a/lib/private/AppFramework/ORM/EntityInfo.php b/lib/private/AppFramework/ORM/EntityInfo.php index 9123651551c85..76d74f001dfe8 100644 --- a/lib/private/AppFramework/ORM/EntityInfo.php +++ b/lib/private/AppFramework/ORM/EntityInfo.php @@ -13,14 +13,15 @@ use OCP\AppFramework\ORM\Attribute\JoinColumn; use OCP\AppFramework\ORM\Attribute\ManyToOne; use OCP\AppFramework\ORM\Attribute\OneToOne; +use OCP\DB\Types; /** * @template T as object */ -class EntityInfo { +final class EntityInfo { public readonly string $tableName; - /** @var array */ + /** @var array */ public array $mappingColumnToTypes = []; /** @var array */ @@ -77,55 +78,65 @@ public function __construct( } } - if ($propertyAttributes->id !== null && $propertyAttributes->column === null) { + if ($propertyAttributes->id instanceof Id && !$propertyAttributes->column instanceof Column) { throw new \RuntimeException($this->entityClass . ' has a Id attribute on ' . $property->getName() . ' but not the corresponding required Column attribute.'); } - if ($propertyAttributes->oneToOne !== null + if ($propertyAttributes->oneToOne instanceof OneToOne && $propertyAttributes->oneToOne->mappedBy !== null - && $propertyAttributes->joinColumn !== null + && $propertyAttributes->joinColumn instanceof JoinColumn && $propertyAttributes->joinColumn->onDelete !== null) { throw new \RuntimeException($this->entityClass . '::' . $property->getName() . ' sets JoinColumn::$onDelete on the mappedBy (inverse) side of a OneToOne relation, where it has no effect. Set it on the owning (invertedBy) side instead.'); } - if ($propertyAttributes->oneToOne !== null && $propertyAttributes->oneToOne->mappedBy !== null) { - $this->validateMappedBy($property, $propertyAttributes->oneToOne); + if ($propertyAttributes->oneToOne instanceof OneToOne && $propertyAttributes->oneToOne->mappedBy !== null) { + $this->validateMappedBy($property, $propertyAttributes->oneToOne, $propertyAttributes->oneToOne->mappedBy); } $this->propertiesAttributes[] = $propertyAttributes; } - if ($this->idProperty === null) { + if (!$this->idProperty instanceof \ReflectionProperty) { throw new \RuntimeException($this->entityClass . ' does not have a primary key. This is not supported for repositories backed tables.'); } } + public function getIdProperty(): \ReflectionProperty { + if (!$this->idProperty instanceof \ReflectionProperty) { + throw new \LogicException('Unreachable: the constructor already guarantees idProperty is set.'); + } + + return $this->idProperty; + } + /** * Checks that mappedBy actually points at the owning side of the relation, so a typo * fails loudly at construction time instead of silently resolving to null forever. + * + * @param non-empty-string $mappedBy Same value as $oneToOne->mappedBy, narrowed non-null by the caller. */ - private function validateMappedBy(\ReflectionProperty $property, OneToOne $oneToOne): void { - $prefix = $this->entityClass . '::' . $property->getName() . ' has mappedBy: \'' . $oneToOne->mappedBy . '\', but '; + private function validateMappedBy(\ReflectionProperty $property, OneToOne $oneToOne, string $mappedBy): void { + $prefix = $this->entityClass . '::' . $property->getName() . " has mappedBy: '" . $mappedBy . "', but "; $targetReflection = new \ReflectionClass($oneToOne->targetEntity); - if (!$targetReflection->hasProperty($oneToOne->mappedBy)) { - throw new \RuntimeException($prefix . $oneToOne->targetEntity . ' has no property named \'' . $oneToOne->mappedBy . '\'.'); + if (!$targetReflection->hasProperty($mappedBy)) { + throw new \RuntimeException($prefix . $oneToOne->targetEntity . " has no property named '" . $mappedBy . "'."); } - $targetProperty = $targetReflection->getProperty($oneToOne->mappedBy); + $targetProperty = $targetReflection->getProperty($mappedBy); $targetOneToOneAttributes = $targetProperty->getAttributes(OneToOne::class, \ReflectionAttribute::IS_INSTANCEOF); $targetOneToOne = $targetOneToOneAttributes === [] ? null : $targetOneToOneAttributes[0]->newInstance(); if ($targetOneToOne === null || $targetOneToOne->invertedBy === null) { - throw new \RuntimeException($prefix . $oneToOne->targetEntity . '::' . $oneToOne->mappedBy . ' is not the owning (invertedBy) side of a OneToOne relation.'); + throw new \RuntimeException($prefix . $oneToOne->targetEntity . '::' . $mappedBy . ' is not the owning (invertedBy) side of a OneToOne relation.'); } if ($targetOneToOne->invertedBy !== $property->getName()) { - throw new \RuntimeException($prefix . $oneToOne->targetEntity . '::' . $oneToOne->mappedBy . '\'s invertedBy points at \'' . $targetOneToOne->invertedBy . '\' instead.'); + throw new \RuntimeException($prefix . $oneToOne->targetEntity . '::' . $mappedBy . "'s invertedBy points at '" . $targetOneToOne->invertedBy . "' instead."); } if ($targetProperty->getAttributes(JoinColumn::class, \ReflectionAttribute::IS_INSTANCEOF) === []) { - throw new \RuntimeException($prefix . $oneToOne->targetEntity . '::' . $oneToOne->mappedBy . ' has no JoinColumn attribute.'); + throw new \RuntimeException($prefix . $oneToOne->targetEntity . '::' . $mappedBy . ' has no JoinColumn attribute.'); } } } diff --git a/lib/private/AppFramework/ORM/EntityManager.php b/lib/private/AppFramework/ORM/EntityManager.php index 3ba8aa4f11080..ac9b072d551dd 100644 --- a/lib/private/AppFramework/ORM/EntityManager.php +++ b/lib/private/AppFramework/ORM/EntityManager.php @@ -10,7 +10,9 @@ use Doctrine\DBAL\Schema\Table; use OC\DB\SchemaWrapper; use OCP\AppFramework\ORM\Attribute\Column; +use OCP\AppFramework\ORM\Attribute\Id; use OCP\AppFramework\ORM\Attribute\JoinColumn; +use OCP\AppFramework\ORM\Attribute\OneToOne; use OCP\AppFramework\ORM\Repository; use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; @@ -19,7 +21,7 @@ use OCP\Server; use OCP\Snowflake\ISnowflakeGenerator; -class EntityManager { +final class EntityManager { public function __construct( private readonly IDBConnection $connection, ) { @@ -37,6 +39,7 @@ public function getEntityInfo(string $entityClass): EntityInfo { if (!isset($this->entitiesInfo[$entityClass])) { $this->entitiesInfo[$entityClass] = new EntityInfo($entityClass); } + /** @var EntityInfo $entityInfo */ $entityInfo = $this->entitiesInfo[$entityClass]; return $entityInfo; @@ -53,11 +56,12 @@ public function getEntityInfo(string $entityClass): EntityInfo { * @return Repository */ public function getRepository(string $entityClass): Repository { + /** @psalm-suppress InternalMethod both are private */ return new Repository($this->connection, $this, $entityClass); } /** - * @template T + * @template T of object * @psalm-param T $entity * @return T */ @@ -66,23 +70,21 @@ public function insert(object $entity): object { $insert = $this->connection->getQueryBuilder(); $isSnowflake = false; - $primaryProperty = null; $values = []; foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { $property = $propertyAttributes->property; if ($propertyAttributes->id !== null && $propertyAttributes->column !== null) { - $primaryProperty = $property; $generatorClass = $propertyAttributes->id->generatorClass; if ($generatorClass) { if ($generatorClass === ISnowflakeGenerator::class) { $generator = Server::get($generatorClass); $isSnowflake = true; - /** @psalm-suppress UndefinedClass */ $values[$propertyAttributes->column->name] = $generator->nextId(); $property->setValue($entity, $insert->createNamedParameter($values[$propertyAttributes->column->name])); } } + continue; } @@ -92,17 +94,18 @@ public function insert(object $entity): object { if ($property->getValue($entity) !== null) { throw new \LogicException($entity::class . '::' . $property->getName() . ' is the mappedBy (inverse) side of a OneToOne relation and cannot be persisted directly; set it from the owning (invertedBy) side instead.'); } + continue; } $joinColumn = $propertyAttributes->joinColumn; - /** @var object $object */ + /** @var object|null $targetEntity */ $targetEntity = $property->getValue($entity); $targetEntityInfo = $this->getEntityInfo($targetEntityClass); if ($targetEntity === null) { $values[$joinColumn->name] = $insert->createNamedParameter(null); } else { - $values[$joinColumn->name] = $insert->createNamedParameter($targetEntityInfo->idProperty->getValue($targetEntity)); + $values[$joinColumn->name] = $insert->createNamedParameter($targetEntityInfo->getIdProperty()->getValue($targetEntity)); } continue; @@ -119,18 +122,19 @@ public function insert(object $entity): object { ->executeStatement(); if (!$isSnowflake) { - $primaryProperty->setValue($entity, $insert->getLastInsertId()); + $entityInfo->getIdProperty()->setValue($entity, $insert->getLastInsertId()); } + return $entity; } /** - * @template T + * @template T of object * @psalm-param T $entity * @return T */ public function update(object $entity): object { - $entityClass = get_class($entity); + $entityClass = $entity::class; $entityInfo = $this->getEntityInfo($entityClass); $update = $this->connection->getQueryBuilder(); @@ -138,6 +142,7 @@ public function update(object $entity): object { foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { $property = $propertyAttributes->property; + /** @psalm-suppress MixedAssignment */ $value = $property->getValue($entity); if ($propertyAttributes->id !== null && $propertyAttributes->column !== null) { @@ -145,7 +150,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($entityInfo->mappingPropertyToColumn[$entityInfo->idProperty->getName()], $update->createNamedParameter($property->getValue($entity)))); + $update->andWhere($update->expr()->eq($entityInfo->mappingPropertyToColumn[$entityInfo->getIdProperty()->getName()], $update->createNamedParameter($property->getValue($entity)))); // don't update the id continue; } @@ -156,15 +161,14 @@ public function update(object $entity): object { continue; } - /** @var JoinColumn $joinColumn */ $joinColumn = $propertyAttributes->joinColumn; - /** @var object $object */ + /** @var object|null $targetEntity */ $targetEntity = $value; $targetEntityInfo = $this->getEntityInfo($targetEntityClass); if ($targetEntity === null) { $update->set($joinColumn->name, $update->createNamedParameter(null)); } else { - $update->set($joinColumn->name, $update->createNamedParameter($targetEntityInfo->idProperty->getValue($targetEntity))); + $update->set($joinColumn->name, $update->createNamedParameter($targetEntityInfo->getIdProperty()->getValue($targetEntity))); } continue; @@ -181,11 +185,11 @@ public function update(object $entity): object { } /** - * @template T + * @template T of object * @psalm-param T $entity */ public function delete(object $entity): void { - $entityClass = get_class($entity); + $entityClass = $entity::class; $entityInfo = $this->getEntityInfo($entityClass); $delete = $this->connection->getQueryBuilder(); @@ -195,6 +199,7 @@ public function delete(object $entity): void { foreach ($entityInfo->propertiesAttributes as $propertyAttributes) { if ($propertyAttributes->id !== null && $propertyAttributes->column !== null) { $property = $propertyAttributes->property; + /** @var int|string $value */ $value = $property->getValue($entity); $delete->andWhere($delete->expr()->eq($propertyAttributes->column->name, $delete->createNamedParameter($value))); @@ -208,11 +213,12 @@ public function delete(object $entity): void { try { $delete->executeStatement(); - } catch (Exception $e) { - if ($e->getReason() === Exception::REASON_FOREIGN_KEY_VIOLATION) { - throw new \LogicException($entityClass . ' cannot be deleted: another entity still references it. Delete the related entity first, or set onDelete: \'CASCADE\' on the owning JoinColumn.', 0, $e); + } catch (Exception $exception) { + if ($exception->getReason() === Exception::REASON_FOREIGN_KEY_VIOLATION) { + throw new \LogicException($entityClass . " cannot be deleted: another entity still references it. Delete the related entity first, or set onDelete: 'CASCADE' on the owning JoinColumn.", 0, $exception); } - throw $e; + + throw $exception; } } @@ -222,14 +228,16 @@ public function delete(object $entity): void { */ public function getParameterType(string $type, bool $isArray): string|int { if ($isArray) { + /** @psalm-suppress DeprecatedConstant Types::JSON is only discouraged in WHERE clauses; mapping it is still supported. */ 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."), + default => throw new \LogicException(sprintf("Parameter type '%s' is not supported as an array.", $type)), }; } + /** @psalm-suppress DeprecatedConstant Types::JSON is only discouraged in WHERE clauses; mapping it is still supported. */ return match ($type) { Types::INTEGER, Types::SMALLINT => IQueryBuilder::PARAM_INT, Types::STRING => IQueryBuilder::PARAM_STR, @@ -262,20 +270,21 @@ public function createTable(string $entityClass, SchemaWrapper $schema): void { $this->createRelationColumn($propertyAttributes, $table, $schema); } - - $schema->getWrappedSchema()->toSql($this->connection->getDatabasePlatform()); } + /** + * @param class-string $entityClass + */ public function dropTable(string $entityClass, string $prefix): void { $entityInfo = $this->getEntityInfo($entityClass); $this->connection->dropTable($prefix . $entityInfo->tableName); } private function createProperty(PropertyAttributes $attributes, Table $table): void { - if ($attributes->column === null) { + if (!$attributes->column instanceof Column) { return; } - /** @var Column $columnAttribute */ + $columnAttribute = $attributes->column; $options = [ 'notnull' => !$columnAttribute->nullable, @@ -283,24 +292,26 @@ private function createProperty(PropertyAttributes $attributes, Table $table): v if ($columnAttribute->length !== null) { $options['length'] = $columnAttribute->length; } + if ($columnAttribute->default !== null) { + /** @psalm-suppress MixedAssignment default can be anything */ $options['default'] = $columnAttribute->default; } - if ($attributes->id !== null && $attributes->id->generatorClass === null) { + if ($attributes->id instanceof Id && $attributes->id->generatorClass === null) { $options['autoincrement'] = true; } $table->addColumn($columnAttribute->name, $columnAttribute->type, $options); - if ($attributes->id !== null) { + if ($attributes->id instanceof Id) { $table->setPrimaryKey([$columnAttribute->name]); } } private function createRelationColumn(PropertyAttributes $attributes, Table $table, SchemaWrapper $schema): void { $targetEntityClass = $attributes->getOwningRelationTarget(); - if ($attributes->joinColumn === null || $targetEntityClass === null) { + if (!$attributes->joinColumn instanceof JoinColumn || $targetEntityClass === null) { return; } @@ -308,7 +319,7 @@ private function createRelationColumn(PropertyAttributes $attributes, Table $tab 'notnull' => !$attributes->joinColumn->nullable, ]); - if ($attributes->oneToOne !== null) { + if ($attributes->oneToOne instanceof OneToOne) { // Enforces the "one" in OneToOne; ManyToOne intentionally allows duplicates. $table->addUniqueIndex([$attributes->joinColumn->name]); } diff --git a/lib/private/AppFramework/ORM/PropertyAttributes.php b/lib/private/AppFramework/ORM/PropertyAttributes.php index e2b9261e9d75b..2d78d3f7fef07 100644 --- a/lib/private/AppFramework/ORM/PropertyAttributes.php +++ b/lib/private/AppFramework/ORM/PropertyAttributes.php @@ -13,11 +13,15 @@ use OCP\AppFramework\ORM\Attribute\ManyToOne; use OCP\AppFramework\ORM\Attribute\OneToOne; -class PropertyAttributes { +final class PropertyAttributes { public ?Id $id = null; + public ?Column $column = null; + public ?OneToOne $oneToOne = null; + public ?ManyToOne $manyToOne = null; + public ?JoinColumn $joinColumn = null; public function __construct( @@ -25,20 +29,26 @@ public function __construct( ) { } + /** + * @return class-string|null + */ public function getOwningRelationTarget(): ?string { - if ($this->joinColumn === null) { + if (!$this->joinColumn instanceof JoinColumn) { return null; } - if ($this->manyToOne !== null) { + + if ($this->manyToOne instanceof ManyToOne) { return $this->manyToOne->targetEntity; } - if ($this->oneToOne !== null && $this->oneToOne->invertedBy !== null) { + + if ($this->oneToOne instanceof OneToOne && $this->oneToOne->invertedBy !== null) { return $this->oneToOne->targetEntity; } + return null; } public function isRelation(): bool { - return $this->oneToOne !== null || $this->manyToOne !== null; + return $this->oneToOne instanceof OneToOne || $this->manyToOne instanceof ManyToOne; } } diff --git a/lib/private/Sharing/SharingManager.php b/lib/private/Sharing/SharingManager.php index 76593a8f4bc0b..dc96ad31a85ff 100644 --- a/lib/private/Sharing/SharingManager.php +++ b/lib/private/Sharing/SharingManager.php @@ -698,7 +698,7 @@ private function validateInteraction(ShareAccessContext $accessContext, Share $s $action = new ShareAction(null, array_values(array_map(static fn (SharePermission $permission): string => $permission->class, $share->getEnabledPermissions()))); $usersToCheck = []; - if ($share->owner->instance === null && ($ownerUser = $this->userManager->get($share->owner->userId)) !== null) { + if ($share->owner->instance === null && ($ownerUser = $this->userManager->get($share->owner->userId)) instanceof IUser) { $usersToCheck[] = $ownerUser; } diff --git a/lib/public/AppFramework/ORM/Attribute/Column.php b/lib/public/AppFramework/ORM/Attribute/Column.php index 0eaeb5f34a60e..3c7f63df64299 100644 --- a/lib/public/AppFramework/ORM/Attribute/Column.php +++ b/lib/public/AppFramework/ORM/Attribute/Column.php @@ -29,15 +29,15 @@ final readonly class Column { /** @since 35.0.0 */ public function __construct( - /** @param non-empty-string $name The name of the column in the database. */ + /** @var non-empty-string The name of the column in the database. */ public string $name, - /** @param Types::* $type The type of the column in the database. */ + /** @var Types::* The type of the column in the database. */ public string $type, - /** @param ?int $length The length of the column (relevant for Types::STRING) */ + /** @var ?int The length of the column (relevant for Types::STRING) */ public ?int $length = null, - /** @param bool $nullable Whether the column is nullable in the database */ + /** @var bool Whether the column is nullable in the database */ public bool $nullable = false, - /** @param mixed $default The default value for the column in the database. */ + /** @var mixed The default value for the column in the database. */ public mixed $default = null, ) { } diff --git a/lib/public/AppFramework/ORM/Attribute/Entity.php b/lib/public/AppFramework/ORM/Attribute/Entity.php index de794d0a04d17..60ddba3c7164d 100644 --- a/lib/public/AppFramework/ORM/Attribute/Entity.php +++ b/lib/public/AppFramework/ORM/Attribute/Entity.php @@ -28,7 +28,7 @@ final readonly class Entity { /** @since 35.0.0 */ public function __construct( - /** @param non-empty-string $name The name of the table in the database. */ + /** @var non-empty-string The name of the table in the database. */ public string $name, ) { } diff --git a/lib/public/AppFramework/ORM/Attribute/Id.php b/lib/public/AppFramework/ORM/Attribute/Id.php index 125ccfbdf31a0..1deada1383e62 100644 --- a/lib/public/AppFramework/ORM/Attribute/Id.php +++ b/lib/public/AppFramework/ORM/Attribute/Id.php @@ -30,7 +30,7 @@ final readonly class Id { /** @since 35.0.0 */ public function __construct( - /** @param class-string $generatorClass */ + /** @var class-string|null */ public ?string $generatorClass = null, ) { } diff --git a/lib/public/AppFramework/ORM/Attribute/JoinColumn.php b/lib/public/AppFramework/ORM/Attribute/JoinColumn.php index e01f19614de44..f82ee70625b5f 100644 --- a/lib/public/AppFramework/ORM/Attribute/JoinColumn.php +++ b/lib/public/AppFramework/ORM/Attribute/JoinColumn.php @@ -9,7 +9,6 @@ use Attribute; use OCP\AppFramework\Attribute\Consumable; -use OCP\DB\Types; /** * Attribute for mapping a property in an entity to a database column. @@ -29,13 +28,13 @@ final readonly class JoinColumn { /** @since 35.0.0 */ public function __construct( - /** @param non-empty-string $name The name of the column in the database. */ + /** @var non-empty-string The name of the column in the database. */ public string $name, - /** @param non-empty-string $referencedColumnName The name of the column in the other table */ + /** @var non-empty-string The name of the column in the other table */ public string $referencedColumnName, - /** @param bool $nullable Whether the column is nullable in the database */ + /** @var bool Whether the column is nullable in the database */ public bool $nullable = false, - /** @param 'CASCADE'|null $onDelete The action what happen when deleting the foreign entity */ + /** @var 'CASCADE'|null The action what happen when deleting the foreign entity */ public ?string $onDelete = null, ) { } diff --git a/lib/public/AppFramework/ORM/Attribute/ManyToOne.php b/lib/public/AppFramework/ORM/Attribute/ManyToOne.php index 32ead782c2c20..61f2e16918044 100644 --- a/lib/public/AppFramework/ORM/Attribute/ManyToOne.php +++ b/lib/public/AppFramework/ORM/Attribute/ManyToOne.php @@ -44,7 +44,7 @@ final readonly class ManyToOne { /** @since 35.0.0 */ public function __construct( - /** @param class-string $targetEntity */ + /** @var class-string */ public string $targetEntity, ) { } diff --git a/lib/public/AppFramework/ORM/Attribute/OneToOne.php b/lib/public/AppFramework/ORM/Attribute/OneToOne.php index f38e8edaf5c53..9822141519459 100644 --- a/lib/public/AppFramework/ORM/Attribute/OneToOne.php +++ b/lib/public/AppFramework/ORM/Attribute/OneToOne.php @@ -70,11 +70,11 @@ * @since 35.0.0 */ public function __construct( - /** @param class-string $targetEntity */ + /** @var class-string */ public string $targetEntity, - /** @param ?non-empty-string $mappedBy */ + /** @var ?non-empty-string */ public ?string $mappedBy = null, - /** @param ?non-empty-string $invertedBy */ + /** @var ?non-empty-string */ public ?string $invertedBy = null, ) { } diff --git a/lib/public/AppFramework/ORM/Repository.php b/lib/public/AppFramework/ORM/Repository.php index c9304dea1b457..5633fd3ed98d7 100644 --- a/lib/public/AppFramework/ORM/Repository.php +++ b/lib/public/AppFramework/ORM/Repository.php @@ -25,7 +25,8 @@ class Repository { /** * The class this repository holds. - * @var class-string $entityClass + * + * @var class-string * @since 35.0.0 * @psalm-suppress InvalidConstantAssignmentValue */ @@ -34,8 +35,6 @@ class Repository { /** * @param class-string|null $entityClassOverride Only meant for generic, runtime-typed * repositories (e.g. EntityManager::getRepository()). - * Hand-written subclasses should override the - * `entityClass` constant instead and leave this null. * @throws \ReflectionException * @internal * @since 35.0.0 @@ -48,11 +47,9 @@ public function __construct( } /** - * @return class-string + * @return class-string */ private function getEntityClass(): string { - // static:: so a subclass's overridden constant is picked up rather than this base - // class's empty default. return $this->entityClassOverride ?? static::entityClass; } @@ -64,13 +61,17 @@ private function buildDebugMessage(string $msg, IQueryBuilder $sql): string { * Builds an entity from a flat row of its own scalar columns. OneToOne relations are * always left null here; resolving them is mapJoinedRowToEntity()'s job. * - * @param class-string $entityClass + * @template S of object + * @param class-string $entityClass * @param array $row + * @return S */ private function hydrateRow(string $entityClass, mixed $row): object { $entityInfo = $this->entityManager->getEntityInfo($entityClass); + /** @psalm-suppress MixedMethodCall Entities are a contract of this ORM: every mapped entity class has a public no-argument constructor. */ $entity = new $entityClass(); + /** @psalm-suppress MixedAssignment $value is a raw, untyped DB driver value. */ foreach ($row as $column => $value) { $property = $entityInfo->mappingColumnToProperty[$column]; $type = $entityInfo->mappingColumnToTypes[$column]; @@ -79,51 +80,67 @@ private function hydrateRow(string $entityClass, mixed $row): object { if (is_resource($value)) { $value = stream_get_contents($value); } + $type = Types::STRING; } - if ($column === $entityInfo->idProperty->getName()) { - /** @var list<\ReflectionAttribute> $ids */ - $ids = $entityInfo->idProperty->getAttributes(Id::class, \ReflectionAttribute::IS_INSTANCEOF); + if ($column === $entityInfo->getIdProperty()->getName()) { + $ids = $entityInfo->getIdProperty()->getAttributes(Id::class, \ReflectionAttribute::IS_INSTANCEOF); $id = array_shift($ids); + if ($id === null) { + throw new \LogicException('Unreachable: the id property is missing its #[Id] attribute.'); + } + if ($id->newInstance()->generatorClass !== null) { $entity->$property = (string)$value; continue; } } + /** @psalm-suppress DeprecatedConstant Types::JSON is only discouraged in WHERE clauses; mapping it is still supported. */ + /** @psalm-suppress MixedAssignment $value is a raw DB driver value; each branch below settype()s or reconstructs it. */ switch ($type) { case Types::BIGINT: case Types::SMALLINT: - settype($value, Types::INTEGER); + $value = (int)$value; + break; + case Types::FLOAT: + $value = (float)$value; + break; + case Types::BOOLEAN: + $value = (bool)$value; break; case Types::BINARY: case Types::DECIMAL: case Types::TEXT: - settype($value, Types::STRING); + $value = (string)$value; break; case Types::TIME: case Types::DATE: case Types::DATETIME: case Types::DATETIME_TZ: if (!$value instanceof \DateTime) { - $value = new \DateTime($value); + $value = new \DateTime((string)$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); + $value = new \DateTimeImmutable((string)$value); } + break; case Types::JSON: if (!is_array($value)) { $value = json_decode((string)$value, true); } + break; } + $entity->$property = $value; } @@ -161,6 +178,11 @@ private function buildJoinedSelectQuery(EntityInfo $entityInfo): array { $owningTargetClass = $propertyAttributes->getOwningRelationTarget(); if ($owningTargetClass !== null) { + $joinColumn = $propertyAttributes->joinColumn; + if ($joinColumn === null) { + throw new \LogicException('Unreachable: owning relation without a JoinColumn'); + } + // Owning side (OneToOne's invertedBy, or ManyToOne): the join column lives on our own table. $targetEntityInfo = $this->entityManager->getEntityInfo($owningTargetClass); $alias = 'r' . $index++; @@ -169,8 +191,8 @@ private function buildJoinedSelectQuery(EntityInfo $entityInfo): array { $qb, $alias, $targetEntityInfo, - 'e.' . $propertyAttributes->joinColumn->name, - $alias . '.' . $propertyAttributes->joinColumn->referencedColumnName, + 'e.' . $joinColumn->name, + $alias . '.' . $joinColumn->referencedColumnName, ); $relations[$alias] = ['attributes' => $propertyAttributes, 'entityInfo' => $targetEntityInfo]; @@ -189,7 +211,11 @@ private function buildJoinedSelectQuery(EntityInfo $entityInfo): array { } } - if ($owningPropertyAttributes === null || $owningPropertyAttributes->joinColumn === null) { + if ($owningPropertyAttributes === null) { + continue; + } + + if ($owningPropertyAttributes->joinColumn === null) { continue; } @@ -226,13 +252,14 @@ private function mapJoinedRowToEntity(array $relations, mixed $row): object { $mainRow = []; /** @var array> $relationRows */ $relationRows = []; + /** @psalm-suppress MixedAssignment $value is a raw, untyped DB driver value. */ foreach ($row as $key => $value) { if (str_starts_with($key, 'e_')) { $mainRow[substr($key, 2)] = $value; continue; } - foreach ($relations as $alias => $relation) { + foreach (array_keys($relations) as $alias) { $prefix = $alias . '_'; if (str_starts_with($key, $prefix)) { $relationRows[$alias][substr($key, strlen($prefix))] = $value; @@ -241,12 +268,13 @@ private function mapJoinedRowToEntity(array $relations, mixed $row): object { } } + /** @var T $entity */ $entity = $this->hydrateRow($this->getEntityClass(), $mainRow); foreach ($relations as $alias => $relation) { $propertyName = $relation['attributes']->property->getName(); $targetEntityInfo = $relation['entityInfo']; - $idColumn = $targetEntityInfo->mappingPropertyToColumn[$targetEntityInfo->idProperty->getName()]; + $idColumn = $targetEntityInfo->mappingPropertyToColumn[$targetEntityInfo->getIdProperty()->getName()]; $relationRow = $relationRows[$alias] ?? []; if (($relationRow[$idColumn] ?? null) === null) { @@ -277,7 +305,6 @@ private function mapJoinedRowToEntity(array $relations, mixed $row): object { } } - /** @var T */ return $entity; } @@ -429,7 +456,7 @@ public function findOneBy(array $criteria, array $orderBy = []): object { * @param array $orderBy * @return array{0: IQueryBuilder, 1: array} */ - private function getJoinedSelectQueryBuilder(array $criteria, ?array $orderBy = []): array { + private function getJoinedSelectQueryBuilder(array $criteria, array $orderBy = []): array { $entityInfo = $this->entityManager->getEntityInfo($this->getEntityClass()); [$qb, $relations] = $this->buildJoinedSelectQuery($entityInfo); @@ -446,6 +473,7 @@ private function getJoinedSelectQueryBuilder(array $criteria, ?array $orderBy = $qb->andWhere($qb->expr()->eq('e.' . $column, $qb->createNamedParameter($value, $type))); } } + foreach ($orderBy as $field => $direction) { $qb->addOrderBy($qb->createNamedParameter($field), $direction); } @@ -466,7 +494,6 @@ public function yieldAll(): \Generator { } /** - * @internal * @since 35.0.0 */ public function getTableName(): string { diff --git a/psalm-strict.xml b/psalm-strict.xml index c1b5dc95b0a22..0f217d5b9bada 100644 --- a/psalm-strict.xml +++ b/psalm-strict.xml @@ -53,6 +53,8 @@ + + @@ -88,7 +90,8 @@ - + + diff --git a/tests/lib/AppFramework/ORM/RepositoryTest.php b/tests/lib/AppFramework/ORM/RepositoryTest.php index 26e9cd9731df5..ec5907600148c 100644 --- a/tests/lib/AppFramework/ORM/RepositoryTest.php +++ b/tests/lib/AppFramework/ORM/RepositoryTest.php @@ -38,13 +38,13 @@ class PrimaryKey { #[Column(name: 'name', type: Types::STRING, nullable: true)] public ?string $name = null; - #[Column(name: 'notNullable', type: Types::STRING, nullable: false)] + #[Column(name: 'not_nullable', type: Types::STRING, nullable: false)] public string $notNullable; #[Column(name: 'integer_val', type: Types::INTEGER, nullable: false)] public int $integer; - #[Column(name: 'bigInt_val', type: Types::BIGINT, nullable: false)] + #[Column(name: 'bigint_val', type: Types::BIGINT, nullable: false)] public int $bigInt; #[Column(name: 'float_val', type: Types::FLOAT, nullable: false)] @@ -544,8 +544,8 @@ public function testEntityWithoutRelationsGeneratesNoJoin(): void { $this->assertCount(0, $relations); $this->assertEquals( - 'SELECT e.id AS e_id, e.name AS e_name, e.notNullable AS e_notNullable, ' - . 'e.integer_val AS e_integer_val, e.bigInt_val AS e_bigInt_val, ' + 'SELECT e.id AS e_id, e.name AS e_name, e.not_nullable AS e_not_nullable, ' + . 'e.integer_val AS e_integer_val, e.bigint_val AS e_bigint_val, ' . 'e.float_val AS e_float_val, e.date_val AS e_date_val ' . 'FROM *PREFIX*repository_test_test2 e ' . 'WHERE e.id = :dcValue1', diff --git a/tests/lib/TagsTest.php b/tests/lib/TagsTest.php index a3aa2e68f2de7..f03bdc5d38561 100644 --- a/tests/lib/TagsTest.php +++ b/tests/lib/TagsTest.php @@ -73,7 +73,7 @@ protected function setUp(): void { ->willReturnCallback(fn () => 'file.txt'); $this->objectType = $this->getUniqueID('type_'); - $this->tagMapper = new TagMapper(Server::get(IDBConnection::class)); + $this->tagMapper = Server::get(TagMapper::class); $this->tagMgr = new TagManager( $this->tagMapper, $this->userSession, From 476c1966f971231599f1af90da4a0952e87a8aa6 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 6 Aug 2026 13:26:56 +0200 Subject: [PATCH 10/10] fix: Revert change to Tags Let's keep this for a later refactor Signed-off-by: Carl Schwan --- .../AppFramework/ORM/EntityManager.php | 1 + lib/private/Tagging/TagMapper.php | 14 +++++++++++ lib/private/Tags.php | 9 +------- lib/public/AppFramework/ORM/Repository.php | 23 +++++++++++++++++++ 4 files changed, 39 insertions(+), 8 deletions(-) diff --git a/lib/private/AppFramework/ORM/EntityManager.php b/lib/private/AppFramework/ORM/EntityManager.php index ac9b072d551dd..713105cc35208 100644 --- a/lib/private/AppFramework/ORM/EntityManager.php +++ b/lib/private/AppFramework/ORM/EntityManager.php @@ -64,6 +64,7 @@ public function getRepository(string $entityClass): Repository { * @template T of object * @psalm-param T $entity * @return T + * @throws Exception */ public function insert(object $entity): object { $entityInfo = $this->getEntityInfo($entity::class); diff --git a/lib/private/Tagging/TagMapper.php b/lib/private/Tagging/TagMapper.php index 746ffc426da0f..0f2877803ec46 100644 --- a/lib/private/Tagging/TagMapper.php +++ b/lib/private/Tagging/TagMapper.php @@ -8,6 +8,7 @@ namespace OC\Tagging; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\ORM\Repository; /** @@ -33,4 +34,17 @@ public function loadTags(array $owners, string $type): array { 'name' => 'ASC', ])); } + + public function tagExists(Tag $tag): bool { + try { + $this->findOneBy([ + 'owner' => $tag->owner, + 'type' => $tag->type, + 'name' => $tag->name, + ]); + return true; + } catch (DoesNotExistException) { + return false; + } + } } diff --git a/lib/private/Tags.php b/lib/private/Tags.php index f0120ac1d1049..5c196d6ff606b 100644 --- a/lib/private/Tags.php +++ b/lib/private/Tags.php @@ -320,15 +320,8 @@ public function addMultiple(string|array $names, bool $sync = false, ?int $id = */ protected function save(): void { foreach ($this->tags as $tag) { - try { + if (!$this->mapper->tagExists($tag)) { $this->mapper->insert($tag); - } catch (Exception $e) { - if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { - $this->logger->error($e->getMessage(), [ - 'exception' => $e, - 'app' => 'core', - ]); - } } } diff --git a/lib/public/AppFramework/ORM/Repository.php b/lib/public/AppFramework/ORM/Repository.php index 5633fd3ed98d7..32c2013b8a787 100644 --- a/lib/public/AppFramework/ORM/Repository.php +++ b/lib/public/AppFramework/ORM/Repository.php @@ -356,6 +356,7 @@ private function findJoinedEntity(IQueryBuilder $query, array $relations): objec * * @psalm-param T $entity * @return T + * @throws Exception * @since 35.0.0 */ public function insert(object $entity): object { @@ -379,6 +380,28 @@ public function delete(object $entity): void { $this->entityManager->delete($entity); } + /** + * Tries to create a new entry in the db from an entity and + * updates an existing entry if duplicate keys are detected + * by the database + * + * @param T $entity the entity that should be created/updated + * @return T the saved entity with the (new) id + * @throws Exception + * @throws \InvalidArgumentException if entity has no id + * @since 15.0.0 + */ + public function insertOrUpdate(object $entity): object { + try { + return $this->insert($entity); + } catch (Exception $ex) { + if ($ex->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + return $this->update($entity); + } + throw $ex; + } + } + /** * Finds entities by a set of criteria, keyed by property name. *