[Regression on closed #541] TeamsService.create and updateSplits are back to one SELECT per member to check existence

Aperta
  • Qualcosa non va?
#730 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
75/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
postgresql, typescript
Ambito
backend, databases, performance, testing

Direzione di ricerca

Start in src/teams/teams.service.ts at create and updateSplits, where each member is checked with a separate findOne. Read the related team service spec and inspect how repository queries and transactions are used in both methods. Replace the repeated checks with a shared batched existence check, then add a spec confirming one user query for a multi-member payload and run the team service tests.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug performance

Location — src/teams/teams.service.ts:28-35 (create) and src/teams/teams.service.ts:80-87 (updateSplits).

Problem — The per-member existence loop closed as #541 is present again, duplicated verbatim in both methods:

// Verify all member userIds exist
for (const member of dto.members) {
  const userExists = await this.userRepo.findOne({
    where: { id: member.userId },
  });
  if (!userExists) {
    throw new NotFoundException(`User ${member.userId} not found`);
  }
}

Each iteration is an awaited round trip, so the queries are strictly sequential: a 20-member team issues 20 serial SELECT * FROM users WHERE id = $1 LIMIT 1 statements before any write happens. findOne with no select also hydrates the full User row (including payout address and other PII columns) when only the row's existence is needed.

Why it matters — Request latency scales linearly with member count at full network round-trip cost, on a write path that then holds a transaction open. It is also a needless source of partial-failure windows: by the time member 20 is checked, member 1 may already have been deleted, so the FK can still reject the insert (see the non-transactional delete/insert problem in updateSplits). And the same five lines now exist in two places, so the next fix has to be applied twice — exactly how this regressed.

Reproduction / failure path

  1. Enable DATABASE_LOGGING=true.
  2. POST /teams with 20 distinct valid member userIds.
  3. The log shows 20 separate users SELECTs before the teams INSERT, instead of one.
  4. Same for PATCH /teams/:id/splits with 20 members.

Suggested fix

  • Replace both loops with one batched query and a set difference: 
    const ids = members.map((m) => m.userId);
const found = await this.userRepo.find({ where: { id: In(ids) }, select: { id: true } });
const missing = ids.filter((id) => !found.some((u) => u.id === id));
if (missing.length) throw new NotFoundException(`User(s) not found: ${missing.join(', ')}`);
    Note this also reports every missing id at once instead of only the first.
  • Extract it into one private helper (e.g. assertMembersExist(members, manager?)) used by both create and updateSplits, taking an optional EntityManager so it can run inside the same transaction as the write.
  • Add a spec asserting userRepo.find is called exactly once for a multi-member payload, so a future merge cannot quietly reintroduce the loop.
Lingua principale
TypeScript
Stelle
2
Fork
90
Merge medio
31m
PR unite (30g)
20

Preparare l'ambiente

  • Include un Dockerfile o un file Docker Compose
  • Nessun modello di pull request
  • Nessuna guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di MergeFi/backend

Tutte le issue di MergeFi/backend

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.