From 9897c4b4d1d0afb065d90e3eea4573a70d919688 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 4 Nov 2025 21:55:40 +0100 Subject: [PATCH 1/7] 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 06e2bba45eb35..d35bf43298053 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -76,11 +76,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 04b041895cca8..90e9df2f9e1d9 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -117,11 +117,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 d3d36bde5c714a9e46770d7f5b5feae0b8cb8d14 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 15 Jun 2026 17:26:42 +0200 Subject: [PATCH 2/7] 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 d35bf43298053..efc2f064a6cfe 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -76,16 +76,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', @@ -151,6 +146,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', @@ -1205,6 +1206,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 90e9df2f9e1d9..c571d7e8425a2 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -117,16 +117,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', @@ -192,6 +187,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', @@ -1246,6 +1247,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 d8901c1fb1ad37d1419d40f405949a6d2b35315a Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 21 Jul 2026 01:58:00 +0200 Subject: [PATCH 3/7] 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 fa7763aee2b4ec6f099087cea24f832fe995072f Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 21 Jul 2026 12:21:47 +0200 Subject: [PATCH 4/7] 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 2f9a909af9f2732b113f4d54b11cbc6b6d9074a4 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 21 Jul 2026 12:33:21 +0200 Subject: [PATCH 5/7] 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 9955105ca1fbf9092f703a617566130df7c83ccf Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 21 Jul 2026 13:51:01 +0200 Subject: [PATCH 6/7] 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 dbd7cb5ac8be2..c29b3890653ee 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -4161,15 +4161,6 @@ - - - - - - - - - circleToTeam(...), $this->circlesManager->probeCircles())]]> diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index efc2f064a6cfe..7d75a0d6f6af1 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -150,6 +150,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 c571d7e8425a2..12c34bda94d97 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -191,6 +191,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 68c1790868d066f0f2ef6ac27c11a4235ca5de36 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 21 Jul 2026 15:28:59 +0200 Subject: [PATCH 7/7] 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 {