diff --git a/src/Module/Core/Domain/Entity/Permission.php b/src/Module/Core/Domain/Entity/Permission.php index f23759a..19e5ff5 100644 --- a/src/Module/Core/Domain/Entity/Permission.php +++ b/src/Module/Core/Domain/Entity/Permission.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace App\Module\Core\Domain\Entity; use Doctrine\ORM\Mapping as ORM; +use InvalidArgumentException; // TODO: brancher repositoryClass au ticket 343 partie 2 (Task 2). #[ORM\Entity] @@ -31,8 +32,29 @@ class Permission #[ORM\Column(options: ['default' => false])] private bool $orphan = false; + /** + * Invariants : une permission doit avoir un code non vide respectant la + * convention "module.resource[.sub].action" (donc contenir au moins un + * point), un libelle non vide et un module proprietaire non vide. Ces + * garde-fous evitent la persistence de lignes incoherentes si un appelant + * (fixture, commande de synchro, import) oublie un champ ou passe une + * chaine vide. + */ public function __construct(string $code, string $label, string $module) { + if ('' === $code) { + throw new InvalidArgumentException('Le code de permission ne peut pas etre vide.'); + } + if (!str_contains($code, '.')) { + throw new InvalidArgumentException(sprintf('Le code de permission "%s" ne respecte pas la convention "module.resource[.sub].action".', $code)); + } + if ('' === $label) { + throw new InvalidArgumentException('Le libelle de permission ne peut pas etre vide.'); + } + if ('' === $module) { + throw new InvalidArgumentException('Le module proprietaire de la permission ne peut pas etre vide.'); + } + $this->code = $code; $this->label = $label; $this->module = $module; @@ -69,7 +91,7 @@ class Permission * permettre une reactivation ulterieure, mais doit etre ignoree par les * verifications d'autorisation. */ - public function markOrphan(): self + public function markOrphan(): static { $this->orphan = true; @@ -78,14 +100,14 @@ class Permission /** * Reactive une permission precedemment orpheline : son code reapparait - * dans le code source d'un module. On en profite pour rafraichir les - * metadonnees (libelle et module d'appartenance). + * dans le code source d'un module. Equivaut a updateMetadata() suivi d'un + * clearing du flag orphan ; on delegue a updateMetadata() pour ne pas + * dupliquer la logique d'affectation des metadonnees. */ - public function revive(string $label, string $module): self + public function revive(string $label, string $module): static { + $this->updateMetadata($label, $module); $this->orphan = false; - $this->label = $label; - $this->module = $module; return $this; } @@ -95,7 +117,7 @@ class Permission * statut d'orphelin. Utilise par la commande de synchronisation lorsque * seul le libelle ou le module proprietaire a change cote code. */ - public function updateMetadata(string $label, string $module): self + public function updateMetadata(string $label, string $module): static { $this->label = $label; $this->module = $module; diff --git a/src/Module/Core/Domain/Entity/Role.php b/src/Module/Core/Domain/Entity/Role.php index 8db87df..3782a58 100644 --- a/src/Module/Core/Domain/Entity/Role.php +++ b/src/Module/Core/Domain/Entity/Role.php @@ -42,6 +42,15 @@ class Role private bool $isSystem = false; /** @var Collection */ + // Choix deliberé de fetch: 'EAGER' (durcissement, pas oubli de perf) : + // - Evite un lazy-load silencieux pendant un refresh de token JWT ou une + // serialisation hors contexte EntityManager (voir ticket #343, section + // 11 risque #1) ou la collection serait inaccessible et provoquerait + // une erreur opaque. + // - Compromis accepte : surcout SQL volontaire, acceptable a l'echelle + // d'un CRM/ERP PME ou un role porte quelques dizaines de permissions. + // - Si la volumetrie augmente significativement : revoir vers une + // projection cachee (ticket a ouvrir a ce moment-la). #[ORM\ManyToMany(targetEntity: Permission::class, fetch: 'EAGER')] #[ORM\JoinTable(name: 'role_permission')] private Collection $permissions; @@ -90,7 +99,7 @@ class Role * Met a jour le libelle affichable du role. Le code reste immuable pour * garantir la stabilite des references cote fixtures et migrations. */ - public function setLabel(string $label): self + public function setLabel(string $label): static { $this->label = $label; @@ -100,7 +109,7 @@ class Role /** * Met a jour la description libre du role (champ documentaire). */ - public function setDescription(?string $description): self + public function setDescription(?string $description): static { $this->description = $description; @@ -111,7 +120,7 @@ class Role * Ajoute une permission au role. Idempotent : ajouter deux fois la meme * permission n'entraine pas de doublon dans la collection. */ - public function addPermission(Permission $permission): self + public function addPermission(Permission $permission): static { if (!$this->permissions->contains($permission)) { $this->permissions->add($permission); @@ -124,7 +133,7 @@ class Role * Retire une permission du role. Idempotent : retirer une permission * absente est un no-op silencieux. */ - public function removePermission(Permission $permission): self + public function removePermission(Permission $permission): static { $this->permissions->removeElement($permission); diff --git a/tests/Module/Core/Domain/Entity/PermissionTest.php b/tests/Module/Core/Domain/Entity/PermissionTest.php index 0b039c8..af4695c 100644 --- a/tests/Module/Core/Domain/Entity/PermissionTest.php +++ b/tests/Module/Core/Domain/Entity/PermissionTest.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace App\Tests\Module\Core\Domain\Entity; use App\Module\Core\Domain\Entity\Permission; +use InvalidArgumentException; use PHPUnit\Framework\TestCase; /** @@ -54,4 +55,33 @@ final class PermissionTest extends TestCase self::assertTrue($permission->isOrphan()); self::assertSame('Lbl', $permission->getLabel()); } + + public function testConstructorRejectsEmptyCode(): void + { + $this->expectException(InvalidArgumentException::class); + + new Permission('', 'Libelle', 'core'); + } + + public function testConstructorRejectsCodeWithoutDot(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('invalid_format'); + + new Permission('invalid_format', 'Libelle', 'core'); + } + + public function testConstructorRejectsEmptyLabel(): void + { + $this->expectException(InvalidArgumentException::class); + + new Permission('core.users.view', '', 'core'); + } + + public function testConstructorRejectsEmptyModule(): void + { + $this->expectException(InvalidArgumentException::class); + + new Permission('core.users.view', 'Libelle', ''); + } } diff --git a/tests/Module/Core/Domain/Entity/RoleTest.php b/tests/Module/Core/Domain/Entity/RoleTest.php index 3910550..23e8dd2 100644 --- a/tests/Module/Core/Domain/Entity/RoleTest.php +++ b/tests/Module/Core/Domain/Entity/RoleTest.php @@ -37,6 +37,18 @@ final class RoleTest extends TestCase self::assertSame(1, $role->getPermissions()->count()); } + public function testAddPermissionAddsMultipleDistinct(): void + { + $role = new Role('custom', 'Custom'); + $permissionView = new Permission('core.users.view', 'Voir', 'core'); + $permissionEdit = new Permission('core.users.edit', 'Editer', 'core'); + + $role->addPermission($permissionView); + $role->addPermission($permissionEdit); + + self::assertSame(2, $role->getPermissions()->count()); + } + public function testRemovePermissionRemovesWhenPresent(): void { $role = new Role('custom', 'Custom'); diff --git a/tests/Module/Core/Domain/Security/SystemRolesTest.php b/tests/Module/Core/Domain/Security/SystemRolesTest.php deleted file mode 100644 index 0a1934e..0000000 --- a/tests/Module/Core/Domain/Security/SystemRolesTest.php +++ /dev/null @@ -1,24 +0,0 @@ -