refactor(core) : RBAC Task 1 - polish apres revue qualite

- Permission : guards constructeur (code/label/module non vides, code avec point)
- Permission::revive() reutilise updateMetadata() pour eviter la duplication
- Suppression de SystemRolesTest (tautologique, ne capture aucun comportement)
- Role::permissions : commentaire explicite sur la raison du fetch EAGER
- Alignement des types de retour sur static (style User.php)
- Nouveau test Role::addPermission avec permissions distinctes

Ticket #343 - Task 1 polish (revue qualite).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

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;

src/Module/Core/Domain/Entity/Role.php

@@ -42,6 +42,15 @@ class Role
     private bool $isSystem = false;
 
     /** @var Collection<int, Permission> */
+    // 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);
 

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', '');
+    }
 }

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');

tests/Module/Core/Domain/Security/SystemRolesTest.php

@@ -1,24 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace App\Tests\Module\Core\Domain\Security;
-
-use App\Module\Core\Domain\Security\SystemRoles;
-use PHPUnit\Framework\TestCase;
-
-/**
- * @internal
- */
-final class SystemRolesTest extends TestCase
-{
-    public function testAdminCodeConstant(): void
-    {
-        self::assertSame('admin', SystemRoles::ADMIN_CODE);
-    }
-
-    public function testUserCodeConstant(): void
-    {
-        self::assertSame('user', SystemRoles::USER_CODE);
-    }
-}