Skip to content

fix: resolve 500 on /api/persons, session recursion and image OOM#30

Merged
LukaMrt merged 10 commits into
masterfrom
develop
May 22, 2026
Merged

fix: resolve 500 on /api/persons, session recursion and image OOM#30
LukaMrt merged 10 commits into
masterfrom
develop

Conversation

@LukaMrt

@LukaMrt LukaMrt commented May 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • 500 sur /api/persons : l'endpoint chargeait toute la table avec N+1 lazy loading. Remplacé par un endpoint de recherche ?q=&limit=20 ; retourne [] si q absent — impossible d'OOM le worker
  • Récursion infinie PersistentCollection : User sérialisé en session descendait dans le graphe Person → Sponsor → Person. Ajout de __serialize/__unserialize sur User pour ne stocker que les scalaires en session
  • OOM Imagick/GD : switch du driver Liip Imagine de gd vers imagick, output WebP, maxWidth/maxHeight réduit de 4096 à 2000, bust du cache avant upload et warm-up après
  • Autocomplete parrain/fillot : PersonAutocomplete (synchrone, précharge toute la liste) remplacé par SuggestInput<Person> asynchrone avec debounce 200ms
  • DeployResetCommand (app:deploy-reset) : invalide les sessions existantes + régénère tous les caches avatars en WebP — à lancer au déploiement
  • MigratePicturesCommand supprimée (migration déjà effectuée en prod)

Test plan

  • php bin/console app:deploy-reset s'exécute sans erreur
  • Upload d'avatar : bust de l'ancien cache + warm-up du nouveau en WebP
  • Sélecteur parrain/fillot : pas de dropdown sous 2 chars, état "Recherche…" visible, résultats après debounce
  • GET /api/persons sans ?q= retourne [] (plus de 500)
  • Sessions invalidées → utilisateurs déconnectés proprement au redéploiement
  • Tests e2e sponsor-add.spec.ts passent

🤖 Generated with Claude Code

LukaMrt and others added 2 commits May 22, 2026 14:05
- 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<Person>
- 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 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 22, 2026 12:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses multiple production-impacting issues around person listing/search, session serialization recursion, and avatar image processing/caching, while updating the sponsor “autocomplete” UX to use an async server-backed suggest input.

Changes:

  • Replace “list all persons” behavior with a bounded search endpoint (/api/persons?q=&limit=) and update the sponsor picker to use async suggestions.
  • Prevent session serialization recursion by custom-scalar serializing User; add a deploy-time reset command to clear sessions and regenerate avatar caches.
  • Switch LiipImagine to Imagick + WebP output and reduce accepted avatar dimensions; add cache bust/warm-up on avatar upload.

Reviewed changes

Copilot reviewed 16 out of 20 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
frontend/src/pages/person/EditPersonPage.tsx Replaces in-memory person autocomplete with async SuggestInput + server search; removes full-table prefetch.
frontend/src/lib/queries.ts Adds personQueries.search(q, limit) query config for the new search endpoint.
frontend/src/lib/api/persons.ts Replaces getPersons with searchPersons({ q, limit, orderBy }).
frontend/src/components/ui/SuggestInput.tsx Introduces async mode with debounce/loading/empty states while keeping sync mode for existing uses.
e2e/tests/profile/sponsor-add.spec.ts Adds e2e coverage for async autocomplete behavior and updates sponsor-add flow.
docker/Dockerfile Installs Imagick extension and runtime packages for LiipImagine driver switch.
backend/src/Service/PersonService.php Extends getAll() signature to accept search query + limit and forwards to repository.
backend/src/Repository/PersonRepository.php Implements query filtering + max results for person search.
backend/src/Controller/Api/PersonApiController.php Returns [] when q is missing; applies q/limit search; busts + warms avatar caches on upload.
backend/src/Service/AvatarCacheService.php Adds centralized LiipImagine avatar cache bust/warm-up helpers.
backend/src/Entity/Person/User.php Adds __serialize/__unserialize to avoid persisting the full entity graph in session.
backend/src/Entity/Person/Person.php Reduces max allowed uploaded image dimensions (4096 → 2000).
backend/src/Command/MigratePicturesCommand.php Removes the already-completed migration command.
backend/src/Command/DeployResetCommand.php Adds app:deploy-reset to clear sessions and warm avatar caches post-deploy.
backend/config/services.yaml Registers the new deploy reset command with projectDir argument.
backend/config/packages/liip_imagine.yaml Switches driver to imagick and forces avatar filters to output WebP.
Comments suppressed due to low confidence (1)

e2e/tests/profile/sponsor-add.spec.ts:55

  • Avoid fixed waitForTimeout in e2e tests; it is inherently flaky across CI load/network variance. Prefer waiting on a deterministic UI condition (e.g., expect a specific option to appear / expect "Aucun résultat" / wait for the /api/persons?q= response).
    await autocomplete.fill(firstName);

    await page.waitForTimeout(400); // debounce + réseau


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread frontend/src/pages/person/EditPersonPage.tsx Outdated
Comment thread frontend/src/components/ui/SuggestInput.tsx
Comment thread e2e/tests/profile/sponsor-add.spec.ts
Comment thread backend/src/Service/AvatarCacheService.php
Comment thread backend/src/Command/DeployResetCommand.php
Comment thread backend/src/Entity/Person/User.php
Comment thread docker/Dockerfile Outdated
Comment thread frontend/src/pages/person/EditPersonPage.tsx
LukaMrt and others added 7 commits May 22, 2026 14:14
autoconf is required by phpize/pecl to compile the imagick extension
on Alpine Linux.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pecl requires gcc, make, and musl-dev to compile imagick from source.
build-base is the Alpine equivalent of Debian's build-essential.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- AvatarCacheService: remove leading slash from Liip Imagine path
  (cache keys must match resolver URLs: uploads/avatars/<file>)
- User::__unserialize: use null-coalescing defaults to handle sessions
  created before this change (prevents 500s on deploy)
- DeployResetCommand: guard getRealPath() false return before unlink
- AddSponsorForm: wrap searchOtherPersons in useCallback to stabilise
  the search reference passed to SuggestInput's useEffect dependency
- AddSponsorForm: clear otherQuery on successful sponsor submission
- AsyncSuggestInput: add role=listbox/option ARIA attributes
- e2e: update selectors from listitem/list to option/listbox

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The 'Recherche…' intermediate state is too transient to catch reliably
in CI (debounce + fast network can complete before Playwright checks).
Replace with a direct assertion on results visibility and verify the
listbox role is present. Also revert setLoading placement to satisfy
react-hooks/set-state-in-effect lint rule.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The API returns last names in uppercase. The test was comparing against
mixed-case 'Henri Durand' instead of 'Henri DURAND'.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GET /api/persons without ?q= now returns [] to prevent N+1 OOM.
Use loginAs + getMe to retrieve Luka's person ID instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@LukaMrt
LukaMrt merged commit b9492e2 into master May 22, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants