From b98d032f72d20dbd6fe0a7044a693b516eef9344 Mon Sep 17 00:00:00 2001 From: LukaMrt Date: Fri, 22 May 2026 14:05:34 +0200 Subject: [PATCH 01/10] fix: resolve 500 on /api/persons, session recursion and image OOM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace full-list /api/persons with search endpoint (?q=, limit=20) to eliminate N+1 lazy loading OOM on the sponsor picker - Refactor SuggestInput to support async mode with debounce (200ms) and replace PersonAutocomplete with SuggestInput - Add __serialize/__unserialize on User to prevent infinite recursion when Symfony serializes the security token (Person→Sponsor→Person) - Switch Liip Imagine driver from gd to imagick, output webp thumbnails, bust old cache before upload and warm new cache after - Reduce maxWidth/maxHeight from 4096 to 2000 on avatar uploads - Add AvatarCacheService (warmUp + bust) and DeployResetCommand (invalidate sessions + regenerate all avatar caches) - Remove MigratePicturesCommand (migration complete) - Update e2e sponsor-add tests to cover async autocomplete behaviour Co-Authored-By: Claude Sonnet 4.6 --- backend/config/packages/liip_imagine.yaml | 4 +- backend/config/services.yaml | 2 +- backend/src/Command/DeployResetCommand.php | 85 ++++++++ .../src/Command/MigratePicturesCommand.php | 98 --------- .../Controller/Api/PersonApiController.php | 28 ++- backend/src/Entity/Person/Person.php | 4 +- backend/src/Entity/Person/User.php | 26 +++ backend/src/Repository/PersonRepository.php | 19 +- backend/src/Service/AvatarCacheService.php | 37 ++++ backend/src/Service/PersonService.php | 4 +- docker/Dockerfile | 6 +- e2e/tests/profile/sponsor-add.spec.ts | 77 ++++++- frontend/src/components/ui/SuggestInput.tsx | 194 ++++++++++++++++-- frontend/src/lib/api/persons.ts | 15 +- frontend/src/lib/queries.ts | 11 +- frontend/src/pages/person/EditPersonPage.tsx | 149 +++----------- 16 files changed, 504 insertions(+), 255 deletions(-) create mode 100644 backend/src/Command/DeployResetCommand.php delete mode 100644 backend/src/Command/MigratePicturesCommand.php create mode 100644 backend/src/Service/AvatarCacheService.php diff --git a/backend/config/packages/liip_imagine.yaml b/backend/config/packages/liip_imagine.yaml index b58115b..df93969 100644 --- a/backend/config/packages/liip_imagine.yaml +++ b/backend/config/packages/liip_imagine.yaml @@ -1,7 +1,7 @@ liip_imagine: twig: mode: lazy - driver: gd + driver: imagick resolvers: default: web_path: @@ -14,6 +14,7 @@ liip_imagine: filter_sets: avatar_thumb: quality: 85 + format: webp filters: thumbnail: size: [200, 200] @@ -22,6 +23,7 @@ liip_imagine: ~ avatar_full: quality: 90 + format: webp filters: thumbnail: size: [400, 400] diff --git a/backend/config/services.yaml b/backend/config/services.yaml index ecb0a04..4f3d986 100644 --- a/backend/config/services.yaml +++ b/backend/config/services.yaml @@ -23,7 +23,7 @@ services: arguments: $resolvers: !tagged_iterator app.contact_resolver - App\Command\MigratePicturesCommand: + App\Command\DeployResetCommand: arguments: $projectDir: '%kernel.project_dir%' diff --git a/backend/src/Command/DeployResetCommand.php b/backend/src/Command/DeployResetCommand.php new file mode 100644 index 0000000..58cd84c --- /dev/null +++ b/backend/src/Command/DeployResetCommand.php @@ -0,0 +1,85 @@ +section('Invalidation des sessions'); + + $sessionDir = $this->projectDir . '/var/sessions'; + $cleared = 0; + + if (is_dir($sessionDir)) { + $finder = new Finder(); + $finder->files()->in($sessionDir); + + foreach ($finder as $file) { + unlink($file->getRealPath()); + ++$cleared; + } + } + + $io->writeln(sprintf(' ✓ %d session(s) supprimée(s)', $cleared)); + + // ── 2. Régénérer le cache des avatars ──────────────────────────────── + $io->section('Régénération du cache avatars (WebP)'); + + $persons = $this->personRepository->findAll(); + $warmed = 0; + $skipped = 0; + + foreach ($persons as $person) { + $picture = $person->getPicture(); + + if ($picture === null) { + ++$skipped; + continue; + } + + try { + $this->avatarCacheService->warmUp($picture); + $io->writeln(sprintf(' ✓ %s', $person->getFullName())); + ++$warmed; + } catch (\Throwable $e) { + $io->warning(sprintf('%s — %s', $person->getFullName(), $e->getMessage())); + } + } + + $io->success(sprintf( + '%d avatar(s) régénéré(s), %d sans image, %d session(s) invalidée(s).', + $warmed, + $skipped, + $cleared, + )); + + return Command::SUCCESS; + } +} diff --git a/backend/src/Command/MigratePicturesCommand.php b/backend/src/Command/MigratePicturesCommand.php deleted file mode 100644 index 6947396..0000000 --- a/backend/src/Command/MigratePicturesCommand.php +++ /dev/null @@ -1,98 +0,0 @@ -projectDir . '/public/uploads/pictures/'; - $newDir = $this->projectDir . '/public/uploads/avatars/'; - - if (!is_dir($newDir) && !mkdir($newDir, 0755, true) && !is_dir($newDir)) { - $io->error('Impossible de créer le dossier ' . $newDir); - return Command::FAILURE; - } - - $persons = $this->personRepository->findAll(); - $migrated = 0; - $skipped = 0; - $missing = 0; - - foreach ($persons as $person) { - $oldName = $person->getPicture(); - - if ($oldName === null) { - continue; - } - - // Ancien placeholder sans image → NULL - if ($oldName === 'no-picture.svg') { - $person->setPicture(null); - $this->personRepository->update($person); - $io->writeln(sprintf(' ∅ %s → NULL (no-picture.svg)', $person->getFullName())); - ++$migrated; - continue; - } - - // Déjà migré (hash SHA-256 de 40 chars) - if (preg_match('/^[0-9a-f]{40}\.[a-z]+$/', $oldName)) { - ++$skipped; - continue; - } - - $oldPath = $oldDir . $oldName; - - if (!file_exists($oldPath)) { - $io->warning('Fichier introuvable, ignoré : ' . $oldName); - ++$missing; - continue; - } - - $hash = hash_file('sha256', $oldPath); - $ext = pathinfo($oldName, PATHINFO_EXTENSION); - $newName = substr((string) $hash, 0, 40) . '.' . $ext; - $newPath = $newDir . $newName; - - if (file_exists($newPath)) { - $io->writeln(sprintf(' ⟳ %s → %s (déjà présent, BDD mise à jour)', $oldName, $newName)); - } else { - if (!copy($oldPath, $newPath)) { - $io->error(sprintf('Échec de la copie : %s → %s', $oldName, $newName)); - return Command::FAILURE; - } - - $io->writeln(sprintf(' ✓ %s → %s', $oldName, $newName)); - } - - $person->setPicture($newName); - $this->personRepository->update($person); - ++$migrated; - } - - $io->success(sprintf('Migration terminée : %d migrée(s), %d déjà à jour, %d introuvable(s).', $migrated, $skipped, $missing)); - - return Command::SUCCESS; - } -} diff --git a/backend/src/Controller/Api/PersonApiController.php b/backend/src/Controller/Api/PersonApiController.php index 4ba78d1..c8df7ee 100644 --- a/backend/src/Controller/Api/PersonApiController.php +++ b/backend/src/Controller/Api/PersonApiController.php @@ -15,6 +15,7 @@ use App\Entity\Person\Role; use App\Entity\Person\User; use App\Security\Voter\PersonVoter; +use App\Service\AvatarCacheService; use App\Service\PersonService; use App\Service\UserService; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; @@ -33,6 +34,7 @@ public function __construct( private readonly PersonService $personService, private readonly UserService $userService, private readonly ValidatorInterface $validator, + private readonly AvatarCacheService $avatarCacheService, ) { } @@ -48,7 +50,15 @@ public function list(Request $request): JsonResponse ]; $orderByParam = $request->query->getString('orderBy', 'id'); $orderBy = in_array($orderByParam, $allowedOrderBy, true) ? $orderByParam : 'id'; - $people = $this->personService->getAll($orderBy); + + $q = trim($request->query->getString('q', '')); + $limit = $request->query->getInt('limit', 20); + + if ($q === '') { + return ApiResponse::success([]); + } + + $people = $this->personService->getAll($orderBy, $q, $limit); /** @var PersonResponseDto[] $dtos */ $dtos = array_map( @@ -124,8 +134,8 @@ public function uploadPicture(Person $person, Request $request): JsonResponse 'image/webp', 'image/gif', ], - maxWidth: 4096, - maxHeight: 4096, + maxWidth: 2000, + maxHeight: 2000, detectCorrupted: true, ), ]); @@ -137,10 +147,20 @@ public function uploadPicture(Person $person, Request $request): JsonResponse ); } + $oldPicture = $person->getPicture(); + if ($oldPicture !== null) { + $this->avatarCacheService->bust($oldPicture); + } + $this->personService->update($person); $person->setPictureFile(null); - return ApiResponse::success(['picture' => $person->getPicture()]); + $newPicture = $person->getPicture(); + if ($newPicture !== null) { + $this->avatarCacheService->warmUp($newPicture); + } + + return ApiResponse::success(['picture' => $newPicture]); } #[Route('/api/persons/{id}/account', name: 'api_persons_get_account', methods: ['GET'])] diff --git a/backend/src/Entity/Person/Person.php b/backend/src/Entity/Person/Person.php index 2ec7a18..10b5286 100644 --- a/backend/src/Entity/Person/Person.php +++ b/backend/src/Entity/Person/Person.php @@ -52,8 +52,8 @@ class Person implements \Stringable 'image/webp', 'image/gif', ], - maxWidth: 4096, - maxHeight: 4096, + maxWidth: 2000, + maxHeight: 2000, detectCorrupted: true, )] private ?File $pictureFile = null; diff --git a/backend/src/Entity/Person/User.php b/backend/src/Entity/Person/User.php index 87f34f8..fb3b02c 100644 --- a/backend/src/Entity/Person/User.php +++ b/backend/src/Entity/Person/User.php @@ -220,4 +220,30 @@ public function getCreatedAt(): ?\DateTimeInterface { return $this->createdAt; } + + /** + * @return array + */ + public function __serialize(): array + { + return [ + 'id' => $this->id, + 'email' => $this->email, + 'password' => $this->password, + 'roles' => $this->roles, + 'isValidated' => $this->isValidated, + ]; + } + + /** + * @param array{id: int|null, email: string|null, password: string|null, roles: string[], isValidated: bool} $data + */ + public function __unserialize(array $data): void + { + $this->id = $data['id']; + $this->email = $data['email']; + $this->password = $data['password']; + $this->roles = $data['roles']; + $this->isValidated = $data['isValidated']; + } } diff --git a/backend/src/Repository/PersonRepository.php b/backend/src/Repository/PersonRepository.php index 5a61096..72f7b70 100644 --- a/backend/src/Repository/PersonRepository.php +++ b/backend/src/Repository/PersonRepository.php @@ -23,7 +23,7 @@ public function __construct(ManagerRegistry $managerRegistry) /** * @return Person[] */ - public function getAll(string $orderBy = 'id'): array + public function getAll(string $orderBy = 'id', ?string $q = null, int $limit = 20): array { $allowedColumns = [ 'id', @@ -39,16 +39,23 @@ public function getAll(string $orderBy = 'id'): array ); } - /** @var Person[] $result */ - $result = $this->createQueryBuilder('p') + $qb = $this->createQueryBuilder('p') ->leftJoin('p.filieres', 'pf')->addSelect('pf') ->leftJoin('pf.filiere', 'f')->addSelect('f') ->leftJoin('pf.school', 's')->addSelect('s') ->leftJoin('p.associations', 'pa')->addSelect('pa') ->leftJoin('pa.association', 'assoc')->addSelect('assoc') - ->orderBy('p.' . $orderBy, 'ASC') - ->getQuery() - ->getResult(); + ->orderBy('p.' . $orderBy, 'ASC'); + + if ($q !== null && trim($q) !== '') { + $escaped = addcslashes(trim($q), '%_\\'); + $qb->andWhere('LOWER(p.firstName) LIKE LOWER(:q) OR LOWER(p.lastName) LIKE LOWER(:q)') + ->setParameter('q', '%' . $escaped . '%') + ->setMaxResults(max(1, min($limit, 100))); + } + + /** @var Person[] $result */ + $result = $qb->getQuery()->getResult(); return $result; } diff --git a/backend/src/Service/AvatarCacheService.php b/backend/src/Service/AvatarCacheService.php new file mode 100644 index 0000000..05f798b --- /dev/null +++ b/backend/src/Service/AvatarCacheService.php @@ -0,0 +1,37 @@ +filterService->warmUpCache($path, $filter, null, true); + } + } + + public function bust(string $filename): void + { + $path = '/uploads/avatars/' . $filename; + + foreach (self::FILTERS as $filter) { + $this->filterService->bustCache($path, $filter); + } + } +} diff --git a/backend/src/Service/PersonService.php b/backend/src/Service/PersonService.php index a6add16..33be7b5 100644 --- a/backend/src/Service/PersonService.php +++ b/backend/src/Service/PersonService.php @@ -51,9 +51,9 @@ public function getWithRelations(int $id): ?Person * @param 'id'|'firstName'|'lastName'|'startYear'|'createdAt' $orderBy * @return Person[] */ - public function getAll(string $orderBy = 'id'): array + public function getAll(string $orderBy = 'id', ?string $q = null, int $limit = 20): array { - return $this->personRepository->getAll($orderBy); + return $this->personRepository->getAll($orderBy, $q, $limit); } public function findByIdentity(string $firstName, string $lastName): ?Person diff --git a/docker/Dockerfile b/docker/Dockerfile index 631106b..fed62c0 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -18,11 +18,13 @@ ENV SERVER_NAME=":80" RUN apk add --no-cache netcat-openbsd -RUN apk add --no-cache --virtual .build-deps libpng-dev libjpeg-turbo-dev libwebp-dev icu-dev \ +RUN apk add --no-cache --virtual .build-deps libpng-dev libjpeg-turbo-dev libwebp-dev icu-dev imagemagick-dev \ && docker-php-ext-configure gd --with-jpeg --with-webp \ && docker-php-ext-install pdo_mysql gd intl \ + && pecl install imagick \ + && docker-php-ext-enable imagick \ && apk del .build-deps \ - && apk add --no-cache libpng libjpeg-turbo libwebp icu-libs + && apk add --no-cache libpng libjpeg-turbo libwebp icu-libs imagemagick WORKDIR /app diff --git a/e2e/tests/profile/sponsor-add.spec.ts b/e2e/tests/profile/sponsor-add.spec.ts index 26219fd..e41bcc1 100644 --- a/e2e/tests/profile/sponsor-add.spec.ts +++ b/e2e/tests/profile/sponsor-add.spec.ts @@ -7,6 +7,80 @@ test.beforeEach(() => { clearMailpit(); }); +test.describe('person autocomplete (async SuggestInput)', () => { + test('no dropdown with fewer than 2 characters', async ({ page }) => { + await loginAs(page, LUKA_EMAIL, DEFAULT_PASSWORD); + const me = await getMe(page); + await page.goto(`/person/${me.person.id.toString()}/edit`); + + const autocomplete = page.getByPlaceholder('Rechercher une personne…'); + await autocomplete.fill('H'); + + // minChars = 2 : aucune liste ne doit apparaître + await expect(page.getByRole('list')).not.toBeVisible(); + }); + + test('shows loading state then results after debounce', async ({ page }) => { + await loginAs(page, LUKA_EMAIL, DEFAULT_PASSWORD); + const me = await getMe(page); + await page.goto(`/person/${me.person.id.toString()}/edit`); + + const autocomplete = page.getByPlaceholder('Rechercher une personne…'); + await autocomplete.fill('Hen'); + + // État intermédiaire "Recherche…" pendant le debounce + await expect(page.getByText('Recherche…')).toBeVisible({ timeout: 1_000 }); + + // Résultats après la requête réseau + const henriOption = page.getByRole('listitem').filter({ hasText: 'Henri Durand' }).first(); + await expect(henriOption).toBeVisible({ timeout: 5_000 }); + }); + + test('current person is excluded from results', async ({ page }) => { + await loginAs(page, LUKA_EMAIL, DEFAULT_PASSWORD); + const me = await getMe(page); + await page.goto(`/person/${me.person.id.toString()}/edit`); + + // Récupère le nom complet de l'utilisateur connecté + const personResp = await page.request.get(`/api/persons/${me.person.id.toString()}`); + const personBody = (await personResp.json()) as { data: { fullName: string } }; + const myFullName = personBody.data.fullName; + + // Cherche son propre prénom dans l'autocomplete + const firstName = myFullName.split(' ')[0] ?? 'Luka'; + const autocomplete = page.getByPlaceholder('Rechercher une personne…'); + await autocomplete.fill(firstName); + + await page.waitForTimeout(400); // debounce + réseau + + // Son propre nom ne doit pas apparaître dans la liste + await expect( + page.getByRole('listitem').filter({ hasText: myFullName }), + ).not.toBeVisible(); + }); + + test('selecting a result resets if the user types again', async ({ page }) => { + await loginAs(page, LUKA_EMAIL, DEFAULT_PASSWORD); + const me = await getMe(page); + await page.goto(`/person/${me.person.id.toString()}/edit`); + + const autocomplete = page.getByPlaceholder('Rechercher une personne…'); + await autocomplete.fill('Hen'); + + const henriOption = page.getByRole('listitem').filter({ hasText: 'Henri Durand' }).first(); + await expect(henriOption).toBeVisible({ timeout: 5_000 }); + await henriOption.click(); + + // L'input affiche le nom complet sélectionné + await expect(autocomplete).toHaveValue('Henri Durand'); + + // Si on retape, la sélection est effacée (bouton Ajouter reste disabled) + await autocomplete.fill('Hen'); + const addButton = page.getByRole('button', { name: 'Ajouter', exact: true }); + await expect(addButton).toBeDisabled(); + }); +}); + test('add a HEART sponsor link from Luka to Henri', async ({ page }) => { await loginAs(page, LUKA_EMAIL, DEFAULT_PASSWORD); const me = await getMe(page); @@ -18,7 +92,7 @@ test('add a HEART sponsor link from Luka to Henri', async ({ page }) => { // Rôle Parrain (Luka devient parrain de quelqu'un) await page.getByRole('button', { name: 'Parrain', exact: true }).click(); - // Autocomplete : taper "Henri" → cliquer sur "Henri Durand" + // Autocomplete : taper "Henri" → attendre les résultats → cliquer sur "Henri Durand" const autocomplete = page.getByPlaceholder('Rechercher une personne…'); await autocomplete.fill('Henri'); const henriOption = page.getByRole('listitem').filter({ hasText: 'Henri Durand' }).first(); @@ -38,7 +112,6 @@ test('add a HEART sponsor link from Luka to Henri', async ({ page }) => { await expect(page.getByText('Parrainage ajouté')).toBeVisible({ timeout: 5_000 }); // Une SponsorRow avec Henri Durand apparaît dans la section "Fillots" - // (le nom est rendu en MAJUSCULES dans la SponsorRow) await expect(page.getByText('Henri DURAND').first()).toBeVisible(); // Vérification API : Luka a maintenant Henri parmi ses godChildren diff --git a/frontend/src/components/ui/SuggestInput.tsx b/frontend/src/components/ui/SuggestInput.tsx index 88ca957..3050500 100644 --- a/frontend/src/components/ui/SuggestInput.tsx +++ b/frontend/src/components/ui/SuggestInput.tsx @@ -1,18 +1,47 @@ -import { useRef, useState } from 'react'; -import type { InputHTMLAttributes, KeyboardEvent } from 'react'; +import { useEffect, useRef, useState } from 'react'; +import type { InputHTMLAttributes, KeyboardEvent, ReactNode } from 'react'; import { cn } from '../../lib/cn'; -interface SuggestInputProps extends Omit< - InputHTMLAttributes, - 'value' | 'onChange' -> { +type BaseProps = Omit, 'value' | 'onChange'> & { value: string; onChange: (value: string) => void; - suggestions: string[]; wrapperClassName?: string; +}; + +type SyncProps = BaseProps & { + suggestions: string[]; + search?: never; + getLabel?: never; + getKey?: never; + renderItem?: never; + onPick?: (item: string) => void; + debounceMs?: never; + minChars?: never; +}; + +type AsyncProps = BaseProps & { + suggestions?: never; + search: (query: string) => Promise; + getLabel: (item: T) => string; + getKey: (item: T) => string | number; + renderItem?: (item: T, active: boolean) => ReactNode; + onPick: (item: T) => void; + debounceMs?: number; + minChars?: number; +}; + +export type SuggestInputProps = SyncProps | AsyncProps; + +export function SuggestInput(props: SuggestInputProps) { + if ('search' in props && typeof props.search === 'function') { + return ; + } + return ; } -export function SuggestInput({ +// ── Mode synchrone (rétrocompat : filières, écoles…) ───────────────────────── + +function SyncSuggestInput({ value, onChange, suggestions, @@ -20,8 +49,9 @@ export function SuggestInput({ className, onFocus, onBlur, - ...props -}: SuggestInputProps) { + onPick, + ...rest +}: SyncProps) { const [open, setOpen] = useState(false); const [cursor, setCursor] = useState(0); const wrapperRef = useRef(null); @@ -34,6 +64,7 @@ export function SuggestInput({ function pick(s: string) { onChange(s); + onPick?.(s); setOpen(false); setCursor(0); } @@ -78,15 +109,13 @@ export function SuggestInput({ 'h-9 w-full rounded-[9px] border border-line bg-surface px-3.5 text-sm text-ink outline-none transition-colors placeholder:text-ink-4 focus:border-ink-2', className, )} - {...props} + {...rest} /> {open && filtered.length > 0 && (