diff --git a/TODO_pregled_koda.md b/TODO_pregled_koda.md index d4231ac..1293ad5 100644 --- a/TODO_pregled_koda.md +++ b/TODO_pregled_koda.md @@ -39,8 +39,8 @@ Legenda ozbiljnosti: 🔴 visok · 🟡 srednji · 🟢 nizak - [ ] Svih **20** `BeginTx(...)` poziva u `internal/db/sqlite/*.go` ima `defer tx.Rollback()` odmah posle provere greške (provereno u: `artikal.go`, `rezervni_kodovi.go`, `servisni_potrazivani_delovi.go`, `nabavka.go`, `prodaja.go`, `nivelacija.go`, `servisni_radovi.go`, `servisni_delovi.go`, `servis.go`) — **nema propusta**, bez akcije. -- [ ] 🟡 **`ServisRepo.Kreiraj` (`internal/db/sqlite/servis.go:142`) nema idempotency zaštitu** kakva je upravo dodata za `ProdajaRepo.Kreiraj` (migracija 106, `idempotency_key`). Broj naloga (`sledeciBrojServisa`) se generiše unutar iste tx (ispravno, sprečava koliziju brojeva), ali DVA nezavisna HTTP POST zahteva (mrežni retry, "Nazad" pa resubmit, dva otvorena taba) i dalje mogu napraviti DVA validna servisna naloga sa istim podacima ali različitim brojevima — isti arhitekturni gep koji je upravo zatvoren za prodaju. Handler: `internal/handler/servis.go` `SacuvajNalog` (red ~218). - **Preporuka**: primeniti isti obrazac (idempotency_key kolona + parcijalni UNIQUE indeks + skriveno polje u `servis_forma.html`) ako se želi potpuna zaštita, konzistentno sa prodajom. +- [x] 🟡 **`ServisRepo.Kreiraj` nema idempotency zaštitu — ISPRAVLJENO.** + Primenjen isti obrazac kao za prodaju (migracija 106): nova migracija `migrations/107_servis_idempotency_key.sql` (kolona `idempotency_key` + parcijalni `UNIQUE` indeks na `servisni_nalozi`), `model.ServisniNalog.IdempotencyKey` polje, `ServisRepo.Kreiraj` (`internal/db/sqlite/servis.go`) proverava postojeći ključ pre insert-a i vraća postojeći ID ako je nalog već kreiran istim ključem, `parseFormuNaloga` (`internal/handler/servis.go`) čita `idempotency_key` iz forme, `servis_forma.html` dobija skriveno polje + JS UUID generator (samo za novi nalog, ne za izmenu). Dodat `internal/db/sqlite/servis_idempotency_test.go` (2 testa: sa ključem vraća isti ID, bez ključa prave se dva odvojena naloga kao ranije) — oba prolaze. - [ ] 🟢 `NabavkaRepo.Kreiraj` (`internal/db/sqlite/nabavka.go:178`) nema interni sekvencijalni broj (koristi `broj_racuna` dobavljača, korisnički unet) — nema race-a oko generisanja broja, ali isti opšti rizik "dva odvojena POST-a = dve nabavke" postoji arhitekturno kao i svuda gde nema idempotency ključa. Niži prioritet jer nema poznatu žalbu/simptom. diff --git a/internal/db/sqlite/servis.go b/internal/db/sqlite/servis.go index 93d211f..7530cbd 100644 --- a/internal/db/sqlite/servis.go +++ b/internal/db/sqlite/servis.go @@ -5,6 +5,7 @@ import ( "crypto/rand" "database/sql" "encoding/hex" + "errors" "fmt" "time" @@ -145,6 +146,21 @@ func (r *ServisRepo) Kreiraj(ctx context.Context, n *model.ServisniNalog) (int64 } defer tx.Rollback() + // idempotency zaštita: isti obrazac kao ProdajaRepo.Kreiraj — ako pozivalac pošalje + // ključ i nalog sa tim ključem već postoji, to je dupliran POST — vraćamo postojeći ID. + if n.IdempotencyKey != "" { + var postojeciID int64 + err := tx.QueryRowContext(ctx, + "SELECT id FROM servisni_nalozi WHERE idempotency_key = ?", n.IdempotencyKey, + ).Scan(&postojeciID) + if err == nil { + return postojeciID, nil + } + if !errors.Is(err, sql.ErrNoRows) { + return 0, fmt.Errorf("ntech: ServisRepo.Kreiraj: provera idempotency key: %w", err) + } + } + brojNaloga, err := sledeciBrojServisa(ctx, tx) if err != nil { return 0, fmt.Errorf("ntech: ServisRepo.Kreiraj: broj naloga: %w", err) @@ -155,15 +171,15 @@ func (r *ServisRepo) Kreiraj(ctx context.Context, n *model.ServisniNalog) (int64 INSERT INTO servisni_nalozi (klijent_id, tehnicar_id, broj_naloga, uredjaj, serijski_broj, opis_kvara, trazene_nadogradnje, status, cena_od, cena_do, cena_konacna, avans, napomena, garancija_do, garancija_dana, datum_zavrsetka, predvidjen_datum, - ostecenja, pin_uredjaja, pribor, datum_prijema, javni_token) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + ostecenja, pin_uredjaja, pribor, datum_prijema, javni_token, idempotency_key) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, nullInt64(n.KlijentID), nullInt64(n.TehnicarID), n.BrojNaloga, n.Uredjaj, nullString(n.SerijskiBroj), n.OpisKvara, n.TrazeneNadogradnje, n.Status, nullFloat64(n.CenaOd), nullFloat64(n.CenaDo), nullFloat64(n.CenaKonacna), nullFloat64(n.Avans), nullString(n.Napomena), nullTime(n.GarancijaDo), nullInt(n.GarancijaDana), nullTime(n.DatumZavrsetka), nullTime(n.PredvidjenDatum), nullString(n.Ostecenja), nullString(n.PinUredjaja), nullString(n.Pribor), - n.DatumPrijema, token, + n.DatumPrijema, token, nullString(n.IdempotencyKey), ) if err != nil { return 0, fmt.Errorf("ntech: ServisRepo.Kreiraj: %w", err) diff --git a/internal/db/sqlite/servis_idempotency_test.go b/internal/db/sqlite/servis_idempotency_test.go new file mode 100644 index 0000000..73b9b6a --- /dev/null +++ b/internal/db/sqlite/servis_idempotency_test.go @@ -0,0 +1,65 @@ +package sqlite + +import ( + "context" + "testing" + + "ntech/internal/model" +) + +// TestServisKreiraj_IdempotencyKey: dva poziva Kreiraj sa istim IdempotencyKey +// (simulira dupliran POST — dupli klik, "Nazad" pa ponovni submit, mrežni retry) +// vraćaju ISTI nalogID i ne prave drugi nalog — isti obrazac kao za prodaju +// (v. TestProdajaKreiraj_IdempotencyKey u prodaja_kreiraj_test.go). +func TestServisKreiraj_IdempotencyKey(t *testing.T) { + ctx := context.Background() + baza := testDB(t) + repo := NoviServisRepo(baza) + + id1, err := repo.Kreiraj(ctx, &model.ServisniNalog{ + Uredjaj: "Laptop", OpisKvara: "ne pali", Status: "Primljeno", + IdempotencyKey: "test-servis-kljuc-123", + }) + if err != nil { + t.Fatalf("prvi Kreiraj: %v", err) + } + + // drugi poziv sa ISTIM ključem (nov nalog, kao pri ponovljenom POST-u) + id2, err := repo.Kreiraj(ctx, &model.ServisniNalog{ + Uredjaj: "Laptop", OpisKvara: "ne pali", Status: "Primljeno", + IdempotencyKey: "test-servis-kljuc-123", + }) + if err != nil { + t.Fatalf("drugi Kreiraj (dupliran POST): %v", err) + } + + if id1 != id2 { + t.Errorf("drugi poziv sa istim idempotency ključem vratio drugačiji ID: %d != %d — napravljen dupli nalog", id1, id2) + } + + var brNaloga int + baza.QueryRowContext(ctx, "SELECT COUNT(*) FROM servisni_nalozi WHERE idempotency_key = ?", "test-servis-kljuc-123").Scan(&brNaloga) + if brNaloga != 1 { + t.Errorf("broj naloga sa ovim idempotency ključem = %d, očekivano 1", brNaloga) + } +} + +// TestServisKreiraj_BezIdempotencyKljuca: prazan ključ (pozivalac ga ne koristi) — +// dva odvojena poziva prave DVA odvojena naloga, kao i pre uvođenja zaštite. +func TestServisKreiraj_BezIdempotencyKljuca(t *testing.T) { + ctx := context.Background() + baza := testDB(t) + repo := NoviServisRepo(baza) + + id1, err := repo.Kreiraj(ctx, &model.ServisniNalog{Uredjaj: "PC", OpisKvara: "kvar", Status: "Primljeno"}) + if err != nil { + t.Fatalf("prvi Kreiraj: %v", err) + } + id2, err := repo.Kreiraj(ctx, &model.ServisniNalog{Uredjaj: "PC", OpisKvara: "kvar", Status: "Primljeno"}) + if err != nil { + t.Fatalf("drugi Kreiraj: %v", err) + } + if id1 == id2 { + t.Errorf("bez idempotency ključa očekivana dva različita naloga, dobijen isti ID %d", id1) + } +} diff --git a/internal/handler/servis.go b/internal/handler/servis.go index 200aa06..6c1a990 100644 --- a/internal/handler/servis.go +++ b/internal/handler/servis.go @@ -1324,6 +1324,9 @@ func parseFormuNaloga(r *http.Request) (model.ServisniNalog, string) { PinUredjaja: strings.TrimSpace(r.FormValue("pin_uredjaja")), Pribor: strings.TrimSpace(r.FormValue("pribor")), DatumPrijema: time.Now(), + // idempotency_key: UUID koji frontend generiše po otvaranju forme (skriveno polje); + // koristi ga samo ServisRepo.Kreiraj (zaštita od duplog POST-a), Izmeni ga ignoriše + IdempotencyKey: strings.TrimSpace(r.FormValue("idempotency_key")), } // datum prijema — korisnik može da unese drugi datum (npr. retroaktivno) diff --git a/internal/model/servis.go b/internal/model/servis.go index c043eaa..2d84ae6 100644 --- a/internal/model/servis.go +++ b/internal/model/servis.go @@ -62,6 +62,11 @@ type ServisniNalog struct { Naplaceno float64 // iznos koji je naplaćen pri preuzimanju Stornirano bool RazlogStorniranja string + // IdempotencyKey je UUID koji frontend generiše po otvaranju forme (skriveno polje). + // Ako isti ključ već postoji u bazi, Kreiraj ne pravi novi nalog nego vraća postojeći — + // štiti od duplog POST-a (dupli klik, "Nazad" pa ponovni submit, mrežni retry, dva taba). + // Prazan string znači da pozivalac ne koristi zaštitu (npr. testovi, budući pozivaoci). + IdempotencyKey string } // ServisniLog je jedan zapis u istoriji događaja servisnog naloga. diff --git a/migrations/107_servis_idempotency_key.sql b/migrations/107_servis_idempotency_key.sql new file mode 100644 index 0000000..5374191 --- /dev/null +++ b/migrations/107_servis_idempotency_key.sql @@ -0,0 +1,10 @@ +-- Idempotency ključ za servisni nalog — isti obrazac kao za prodaju +-- (migracija 106_prodaja_idempotency_key.sql): frontend generiše UUID po otvaranju +-- forme i šalje ga kao skriveno polje. Ako isti POST stigne na server dva puta +-- (dupli klik, "Nazad" pa ponovni submit, mrežni retry, dva otvorena taba), +-- drugi zahtev se prepoznaje po već postojećem ključu i vraća VEĆ kreirani nalog +-- umesto da napravi drugi. NULL dozvoljen i ne ulazi u UNIQUE proveru (stari +-- zapisi, ili budući pozivaoci koji ne šalju ključ). +ALTER TABLE servisni_nalozi ADD COLUMN idempotency_key TEXT; +CREATE UNIQUE INDEX IF NOT EXISTS idx_servisni_nalozi_idempotency_key + ON servisni_nalozi(idempotency_key) WHERE idempotency_key IS NOT NULL; diff --git a/web/templates/stranice/servis_forma.html b/web/templates/stranice/servis_forma.html index ef65f0c..279d384 100644 --- a/web/templates/stranice/servis_forma.html +++ b/web/templates/stranice/servis_forma.html @@ -35,6 +35,17 @@ + {{if not .Izmena}} + + + + {{end}}