fix(marketplace): wrap DELETE+loop-CREATE in transaction
Some checks failed
Frontend CI / test (push) Failing after 0s

Two seller-facing mutations followed the same buggy pattern:

  1. s.db.Delete(...all existing rows...)   ← committed immediately
  2. for range inputs { s.db.Create(new) }  ← if any fails mid-loop,
                                              deletes are already
                                              committed → product
                                              left in an inconsistent
                                              state (0 images or
                                              0 licenses) until the
                                              seller retries.

Affected:
  - Service.UpdateProductImages  — 0 images = product page broken
  - Service.SetProductLicenses   — 0 licenses = product unsellable

Fix: wrap each function body in s.db.WithContext(ctx).Transaction,
using tx.* instead of s.db.* throughout. Rollback on any error in
the loop restores the previous images/licenses.

Side benefit: ctx is now propagated into the reads (WithContext on
the transaction root), so timeout middleware applies to the whole
sequence — previously the reads bypassed request timeouts.

Tests: ./internal/core/marketplace/ green (0.478s). go build + vet
clean.

Scope:
  - Subscription service already uses Transaction() for multi-step
    mutations (service.go:287, :395); its single-row Saves
    (scheduleDowngrade, CancelSubscription) are atomic by nature.
  - Wishlist / cart / education / discover core services audited —
    no matching DELETE+LOOP-CREATE pattern found.
  - Single-row mutations (AddProductPreview, UpdateProduct) don't
    need wrapping — atomic in Postgres.

Refs: AUDIT_REPORT.md §4.4 "Transactions insuffisantes" + §9 #3
(critical: marketplace/service.go transactions manquantes).
Narrower than the original audit flagged — real bugs were these 2
functions, not the broader "1050+" region.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
senke 2026-04-21 09:57:50 +02:00
parent ebf3276daa
commit b5281bec98

View file

@ -1114,37 +1114,47 @@ func (s *Service) AddProductPreview(ctx context.Context, productID uuid.UUID, se
// UpdateProductImages replaces all images for a product (v0.401 M1)
func (s *Service) UpdateProductImages(ctx context.Context, productID uuid.UUID, sellerID uuid.UUID, images []ProductImageInput) ([]ProductImage, error) {
var product Product
if err := s.db.First(&product, "id = ?", productID).Error; err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
return nil, ErrProductNotFound
// Wrap DELETE + loop-CREATE in a transaction so a failure mid-loop
// doesn't leave the product with zero images (the delete would
// otherwise already be committed).
var result []ProductImage
err := s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
var product Product
if err := tx.First(&product, "id = ?", productID).Error; err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
return ErrProductNotFound
}
return err
}
if product.SellerID != sellerID {
return ErrInvalidSeller
}
if err := tx.Where("product_id = ?", productID).Delete(&ProductImage{}).Error; err != nil {
return err
}
result = make([]ProductImage, 0, len(images))
for i, img := range images {
if img.URL == "" {
continue
}
pi := &ProductImage{
ProductID: productID,
URL: img.URL,
SortOrder: img.SortOrder,
}
if pi.SortOrder == 0 && i > 0 {
pi.SortOrder = i
}
if err := tx.Create(pi).Error; err != nil {
return err
}
result = append(result, *pi)
}
return nil
})
if err != nil {
return nil, err
}
if product.SellerID != sellerID {
return nil, ErrInvalidSeller
}
if err := s.db.Where("product_id = ?", productID).Delete(&ProductImage{}).Error; err != nil {
return nil, err
}
result := make([]ProductImage, 0, len(images))
for i, img := range images {
if img.URL == "" {
continue
}
pi := &ProductImage{
ProductID: productID,
URL: img.URL,
SortOrder: img.SortOrder,
}
if pi.SortOrder == 0 && i > 0 {
pi.SortOrder = i
}
if err := s.db.Create(pi).Error; err != nil {
return nil, err
}
result = append(result, *pi)
}
return result, nil
}
@ -1159,35 +1169,45 @@ func (s *Service) GetProductLicenses(ctx context.Context, productID uuid.UUID) (
// SetProductLicenses replaces all licenses for a product (v0.401 M2)
func (s *Service) SetProductLicenses(ctx context.Context, productID uuid.UUID, sellerID uuid.UUID, licenses []ProductLicenseInput) ([]ProductLicense, error) {
var product Product
if err := s.db.First(&product, "id = ?", productID).Error; err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
return nil, ErrProductNotFound
// Same DELETE+LOOP-CREATE atomicity concern as UpdateProductImages:
// a failure mid-loop would leave the product with zero licenses,
// making it unsellable until a retry.
var result []ProductLicense
err := s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
var product Product
if err := tx.First(&product, "id = ?", productID).Error; err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
return ErrProductNotFound
}
return err
}
if product.SellerID != sellerID {
return ErrInvalidSeller
}
if err := tx.Where("product_id = ?", productID).Delete(&ProductLicense{}).Error; err != nil {
return err
}
result = make([]ProductLicense, 0, len(licenses))
for _, in := range licenses {
if in.LicenseType == "" || in.PriceCents < 0 {
continue
}
pl := &ProductLicense{
ProductID: productID,
LicenseType: in.LicenseType,
PriceCents: in.PriceCents,
TermsText: in.TermsText,
}
if err := tx.Create(pl).Error; err != nil {
return err
}
result = append(result, *pl)
}
return nil
})
if err != nil {
return nil, err
}
if product.SellerID != sellerID {
return nil, ErrInvalidSeller
}
if err := s.db.Where("product_id = ?", productID).Delete(&ProductLicense{}).Error; err != nil {
return nil, err
}
result := make([]ProductLicense, 0, len(licenses))
for _, in := range licenses {
if in.LicenseType == "" || in.PriceCents < 0 {
continue
}
pl := &ProductLicense{
ProductID: productID,
LicenseType: in.LicenseType,
PriceCents: in.PriceCents,
TermsText: in.TermsText,
}
if err := s.db.Create(pl).Error; err != nil {
return nil, err
}
result = append(result, *pl)
}
return result, nil
}