fix(photos): копия фото не наследует отредактированную версию

Clone переносил все привязки photo_objects, включая роли edit*, хотя правка принадлежит фото, чьи правки её породили. Теперь копия делит только обычные варианты и показывается в исходном виде. - EditRoles() и IsEditRole() в edit_render.go — единый список ролей; - фильтр edit-ролей в Store.Clone и SQLiteStore.Clone; - миграции 000016 (Postgres) и 000008 (SQLite) убирают привязки у копий, созданных до фикса; - контрактная проверка копирования, сервисные и e2e-тесты.
2026-10-21 21:01:33 +03:00
parent 759e78ff79
commit 8dd8cb6f0c
12 changed files with 344 additions and 6 deletions
@@ -0,0 +1,164 @@
+package e2e
+
+import (
+	"context"
+	"io/fs"
+	"path/filepath"
+	"testing"
+	"time"
+
+	assets "galo"
+	"galo/internal/albums"
+	"galo/internal/photos"
+)
+
+// A copy of an edited photo must not inherit the edited version: the edit
+// belongs to the photo whose edits produced it.
+func TestCopyDoesNotShareEditedVersion(t *testing.T) {
+	ctx := context.Background()
+	st, closeDB := openStack(t, filepath.Join(t.TempDir(), "galo.db"), t.TempDir())
+	defer closeDB()
+
+	user, _, err := st.auth.Register(ctx, "copy@test.local", "", "password123", "C")
+	if err != nil {
+		t.Fatalf("register: %v", err)
+	}
+	album, err := st.albums.Create(ctx, user.ID, albums.CreateInput{Title: "Source"})
+	if err != nil {
+		t.Fatalf("create album: %v", err)
+	}
+	target, err := st.albums.Create(ctx, user.ID, albums.CreateInput{Title: "Target"})
+	if err != nil {
+		t.Fatalf("create target album: %v", err)
+	}
+
+	p, err := st.photos.Upload(ctx, photos.UploadInput{AlbumID: album.ID, UserID: user.ID, Filename: "a.png", Data: makePNG(t)})
+	if err != nil {
+		t.Fatalf("upload: %v", err)
+	}
+
+	w := photos.NewWorker(st.photos, time.Second, 10, time.Minute, 2)
+	w.ProcessOnce(ctx)
+
+	if _, err := st.photos.EditPhoto(ctx, p.ID, user.ID, photos.EditParams{Light: photos.LightParams{Exposure: 0.5}}); err != nil {
+		t.Fatalf("edit photo: %v", err)
+	}
+	w.ProcessOnce(ctx)
+
+	source, err := st.photos.Get(ctx, p.ID, user.ID)
+	if err != nil {
+		t.Fatalf("get source: %v", err)
+	}
+	if source.EditThumbURL == "" || source.EditPreviewURL == "" {
+		t.Fatalf("source has no edited version: %+v", source)
+	}
+
+	clones, err := st.photos.CopyBatch(ctx, []string{p.ID}, user.ID, target.ID)
+	if err != nil || len(clones) != 1 {
+		t.Fatalf("copy: %v len=%d", err, len(clones))
+	}
+	clone := clones[0]
+
+	// The copy shares the base variants with the source.
+	if clone.ThumbURL != source.ThumbURL || clone.PreviewURL != source.PreviewURL {
+		t.Fatalf("clone does not share base variants: src=%+v clone=%+v", source, clone)
+	}
+	// But it shows the original picture and carries no edit parameters.
+	if clone.EditURL != "" || clone.EditThumbURL != "" || clone.EditMediumURL != "" || clone.EditPreviewURL != "" {
+		t.Fatalf("clone shares the edited version: %+v", clone)
+	}
+	if len(clone.EditParams) != 0 {
+		t.Fatalf("clone inherited edit params: %+v", clone.EditParams)
+	}
+}
+
+// The migration drops the edit bindings that copies carried before the fix,
+// while the edited version of the photo that owns them stays untouched.
+func TestMigrationDropsEditBindingsOfCopies(t *testing.T) {
+	ctx := context.Background()
+	st, closeDB := openStack(t, filepath.Join(t.TempDir(), "galo.db"), t.TempDir())
+	defer closeDB()
+
+	user, _, err := st.auth.Register(ctx, "mig@test.local", "", "password123", "M")
+	if err != nil {
+		t.Fatalf("register: %v", err)
+	}
+	album, err := st.albums.Create(ctx, user.ID, albums.CreateInput{Title: "Source"})
+	if err != nil {
+		t.Fatalf("create album: %v", err)
+	}
+	target, err := st.albums.Create(ctx, user.ID, albums.CreateInput{Title: "Target"})
+	if err != nil {
+		t.Fatalf("create target album: %v", err)
+	}
+
+	p, err := st.photos.Upload(ctx, photos.UploadInput{AlbumID: album.ID, UserID: user.ID, Filename: "a.png", Data: makePNG(t)})
+	if err != nil {
+		t.Fatalf("upload: %v", err)
+	}
+	w := photos.NewWorker(st.photos, time.Second, 10, time.Minute, 2)
+	w.ProcessOnce(ctx)
+	if _, err := st.photos.EditPhoto(ctx, p.ID, user.ID, photos.EditParams{Light: photos.LightParams{Exposure: 0.5}}); err != nil {
+		t.Fatalf("edit photo: %v", err)
+	}
+	w.ProcessOnce(ctx)
+
+	var editObjectID string
+	if err := st.db.QueryRowContext(ctx,
+		`SELECT object_id FROM photo_objects WHERE photo_id = $1 AND role = 'edit_thumb'`, p.ID).Scan(&editObjectID); err != nil {
+		t.Fatalf("edited thumb object: %v", err)
+	}
+
+	clones, err := st.photos.CopyBatch(ctx, []string{p.ID}, user.ID, target.ID)
+	if err != nil || len(clones) != 1 {
+		t.Fatalf("copy: %v len=%d", err, len(clones))
+	}
+	clone := clones[0]
+
+	// Reproduce the state left by the old code: the copy carries the edited
+	// version of the source photo.
+	if _, err := st.db.ExecContext(ctx,
+		`INSERT INTO photo_objects (photo_id, object_id, role) VALUES ($1, $2, 'edit_thumb')`,
+		clone.ID, editObjectID); err != nil {
+		t.Fatalf("bind edited version to the copy: %v", err)
+	}
+
+	migFS, err := fs.Sub(assets.Migrations, "migrations/sqlite")
+	if err != nil {
+		t.Fatalf("migrations fs: %v", err)
+	}
+	up, err := fs.ReadFile(migFS, "000008_photo_edit_copy_bindings.up.sql")
+	if err != nil {
+		t.Fatalf("read migration: %v", err)
+	}
+	if _, err := st.db.ExecContext(ctx, string(up)); err != nil {
+		t.Fatalf("apply migration: %v", err)
+	}
+
+	var n int
+	if err := st.db.QueryRowContext(ctx,
+		`SELECT count(*) FROM photo_objects WHERE photo_id = $1 AND role LIKE 'edit%'`, clone.ID).Scan(&n); err != nil {
+		t.Fatalf("count copy edit bindings: %v", err)
+	}
+	if n != 0 {
+		t.Fatalf("copy still carries %d edit bindings", n)
+	}
+	// The photo that owns the edited version keeps it.
+	if err := st.db.QueryRowContext(ctx,
+		`SELECT count(*) FROM photo_objects WHERE photo_id = $1 AND role = 'edit_thumb'`, p.ID).Scan(&n); err != nil {
+		t.Fatalf("count source edit bindings: %v", err)
+	}
+	if n != 1 {
+		t.Fatalf("source edit bindings = %d, want 1", n)
+	}
+	// A bound object is never garbage, so the edited version stays alive.
+	claimed, err := st.repos.Photos.ClaimGarbage(ctx, 10)
+	if err != nil {
+		t.Fatalf("claim garbage: %v", err)
+	}
+	for _, o := range claimed {
+		if o.ID == editObjectID {
+			t.Fatalf("edited version of the owner was claimed as garbage")
+		}
+	}
+}
@@ -74,6 +74,60 @@ func TestCopySharesStorageObjects(t *testing.T) {
 	}
 }

+func TestCopyDoesNotShareEditedVersion(t *testing.T) {
+	pool := testPool(t)
+	ctx := context.Background()
+	svc, cl, _ := newRecordingServiceFS(t, pool)
+	user := createUser(t, ctx, pool)
+	albumA := createAlbum(t, ctx, pool, user)
+	albumB := createAlbum(t, ctx, pool, user)
+
+	p, err := svc.Upload(ctx, UploadInput{
+		AlbumID: albumA, UserID: user, Filename: "p.png", MimeType: "image/png", Data: makePNG(t),
+	})
+	if err != nil {
+		t.Fatalf("upload: %v", err)
+	}
+	t.Cleanup(func() { cleanupPhoto(t, ctx, pool, cl, p.ID) })
+
+	w := NewWorker(svc, time.Second, 10, time.Minute, 2)
+	w.processVariantJobs(ctx)
+
+	// The edit belongs to the photo: the parameters live in its metadata and
+	// the background render binds the edited version to it.
+	if _, err := svc.EditPhoto(ctx, p.ID, user, EditParams{Light: LightParams{Exposure: 0.5}}); err != nil {
+		t.Fatalf("edit photo: %v", err)
+	}
+	w.processEditJobs(ctx)
+
+	src, err := svc.store.GetByID(ctx, p.ID, "")
+	if err != nil {
+		t.Fatalf("get source: %v", err)
+	}
+	if src.EditThumbKey == "" || src.EditPreviewKey == "" {
+		t.Fatalf("source has no edited version: %+v", src)
+	}
+
+	clones, err := svc.CopyBatch(ctx, []string{p.ID}, user, albumB)
+	if err != nil || len(clones) != 1 {
+		t.Fatalf("copy: %v len=%d", err, len(clones))
+	}
+	clone := clones[0]
+	t.Cleanup(func() { cleanupPhoto(t, ctx, pool, cl, clone.ID) })
+
+	// The copy shares the base variants with the source.
+	if clone.ThumbKey != src.ThumbKey || clone.MediumKey != src.MediumKey || clone.PreviewKey != src.PreviewKey {
+		t.Fatalf("clone does not share base variants: src=%+v clone=%+v", src, clone)
+	}
+	// But it never inherits the edited version or its parameters.
+	if clone.EditKey != "" || clone.EditThumbKey != "" || clone.EditMediumKey != "" || clone.EditPreviewKey != "" {
+		t.Fatalf("clone shares the edited version: %+v", clone)
+	}
+	if len(clone.EditParams) != 0 {
+		t.Fatalf("clone inherited edit params: %+v", clone.EditParams)
+	}
+}
+
 func sameSet(a, b []string) bool {
 	if len(a) != len(b) {
 		return false
@@ -19,6 +19,22 @@ const (
 	RoleEditPreview = "edit_preview"
 )

+// EditRoles lists the roles of the edited version. The edited version belongs
+// to the photo whose edits produced it, so copies of that photo never share
+// these roles.
+func EditRoles() []string {
+	return []string{RoleEdit, RoleEditThumb, RoleEditMedium, RoleEditPreview}
+}
+
+// IsEditRole reports whether the role belongs to the edited version.
+func IsEditRole(role string) bool {
+	switch role {
+	case RoleEdit, RoleEditThumb, RoleEditMedium, RoleEditPreview:
+		return true
+	}
+	return false
+}
+
 // applyLight renders the light adjustments into a copy of the image. The
 // original pixels are never modified in place.
 func applyLight(src image.Image, p LightParams) *image.NRGBA {
@@ -74,8 +74,7 @@ func (s *Service) ResetEdit(ctx context.Context, photoID, userID string) (*model
 	if err := s.store.DeleteEditJob(ctx, photoID); err != nil {
 		return nil, err
 	}
-	if err := s.store.DeleteObjectBindings(ctx, photoID,
-		RoleEdit, RoleEditThumb, RoleEditMedium, RoleEditPreview); err != nil {
+	if err := s.store.DeleteObjectBindings(ctx, photoID, EditRoles()...); err != nil {
 		return nil, err
 	}
 	return s.publishEdit(ctx, photoID, userID)
@@ -34,6 +34,23 @@ func TestParseEditParamsClampsValues(t *testing.T) {
 	}
 }

+func TestEditRolesCoverTheEditedVersion(t *testing.T) {
+	roles := EditRoles()
+	if len(roles) != 4 {
+		t.Fatalf("edit roles = %v, want four", roles)
+	}
+	for _, role := range roles {
+		if !IsEditRole(role) {
+			t.Fatalf("role %q must be an edit role", role)
+		}
+	}
+	for _, role := range []string{RoleOriginal, RoleThumb, RoleMedium, RolePreview, RolePlayback} {
+		if IsEditRole(role) {
+			t.Fatalf("role %q must not be an edit role", role)
+		}
+	}
+}
+
 func TestEditParamsIsEmpty(t *testing.T) {
 	var zero EditParams
 	if !zero.IsEmpty() {
@@ -250,10 +250,18 @@ func (s *SQLiteStore) Clone(ctx context.Context, src *models.Photo, albumID, upl
 		mediaKind(src.Kind), src.DurationMS); err != nil {
 		return err
 	}
+	// The edited version belongs to the photo whose edits produced it, so the
+	// copy shares the base variants only.
+	roles := EditRoles()
+	args := []any{id, src.ID}
+	for _, role := range roles {
+		args = append(args, role)
+	}
 	if _, err := tx.ExecContext(ctx,
 		`INSERT INTO photo_objects (photo_id, object_id, role)
-		 SELECT $1, object_id, role FROM photo_objects WHERE photo_id = $2`,
-		id, src.ID); err != nil {
+		 SELECT $1, object_id, role FROM photo_objects
+		 WHERE photo_id = $2 AND role NOT IN (`+sqliteutil.Placeholders(3, len(roles))+`)`,
+		args...); err != nil {
 		return err
 	}
 	return tx.Commit()
@@ -290,10 +290,13 @@ func (s *Store) Clone(ctx context.Context, src *models.Photo, albumID, uploaderI
 		mediaKind(src.Kind), src.DurationMS); err != nil {
 		return err
 	}
+	// The edited version belongs to the photo whose edits produced it, so the
+	// copy shares the base variants only.
 	if _, err := tx.Exec(ctx,
 		`INSERT INTO photo_objects (photo_id, object_id, role)
-		 SELECT $1, object_id, role FROM photo_objects WHERE photo_id = $2`,
-		id, src.ID); err != nil {
+		 SELECT $1, object_id, role FROM photo_objects
+		 WHERE photo_id = $2 AND role <> ALL($3::text[])`,
+		id, src.ID, EditRoles()); err != nil {
 		return err
 	}
 	return tx.Commit(ctx)
@@ -49,6 +49,7 @@ func Run(t *testing.T, r Repos) {
 		{"Social", testSocial},
 		{"Photos", testPhotos},
 		{"Objects", testObjects},
+		{"Clone", testClone},
 		{"UserAvatar", testUserAvatar},
 		{"VariantJobKinds", testVariantJobKinds},
 		{"Settings", testSettings},
@@ -709,6 +710,56 @@ func testObjects(t *testing.T, ctx context.Context, r Repos) {
 	}
 }

+func testClone(t *testing.T, ctx context.Context, r Repos) {
+	owner := seedUser(t, ctx, r, "Owner")
+	a := seedAlbum(t, ctx, r, owner.ID)
+	target := seedAlbum(t, ctx, r, owner.ID)
+	p := seedPhoto(t, ctx, r, owner.ID, a.ID)
+
+	// The photo carries the base variants and an edited version.
+	thumb := &photos.Object{ID: newID(), StorageKey: "objects/" + newID(), MimeType: "image/webp", State: photos.ObjectPending}
+	if err := r.Photos.CreateObject(ctx, thumb); err != nil {
+		t.Fatalf("create thumb: %v", err)
+	}
+	if err := r.Photos.ActivateObject(ctx, thumb.ID); err != nil {
+		t.Fatalf("activate thumb: %v", err)
+	}
+	if err := r.Photos.BindObject(ctx, p.ID, thumb.ID, photos.RoleThumb); err != nil {
+		t.Fatalf("bind thumb: %v", err)
+	}
+	edited := &photos.Object{ID: newID(), StorageKey: "objects/" + newID(), MimeType: "image/webp", State: photos.ObjectPending}
+	if err := r.Photos.CreateObject(ctx, edited); err != nil {
+		t.Fatalf("create edited object: %v", err)
+	}
+	if err := r.Photos.ActivateObject(ctx, edited.ID); err != nil {
+		t.Fatalf("activate edited object: %v", err)
+	}
+	if err := r.Photos.BindObject(ctx, p.ID, edited.ID, photos.RoleEdit); err != nil {
+		t.Fatalf("bind edited version: %v", err)
+	}
+
+	cloneID := newID()
+	if err := r.Photos.Clone(ctx, p, target.ID, owner.ID, cloneID); err != nil {
+		t.Fatalf("clone: %v", err)
+	}
+
+	cloneObjects, err := r.Photos.PhotoObjectIDs(ctx, cloneID)
+	if err != nil {
+		t.Fatalf("clone objects: %v", err)
+	}
+	// The copy shares the base variants.
+	if !contains(cloneObjects, thumb.ID) {
+		t.Fatalf("clone objects = %+v, want thumb %s", cloneObjects, thumb.ID)
+	}
+	// The edited version belongs to the photo whose edits produced it.
+	if contains(cloneObjects, edited.ID) {
+		t.Fatalf("clone shares the edited version: %+v", cloneObjects)
+	}
+	if refs, _ := r.Photos.ObjectRefCount(ctx, edited.ID); refs != 1 {
+		t.Fatalf("edited object refs = %d, want 1", refs)
+	}
+}
+
 func testUserAvatar(t *testing.T, ctx context.Context, r Repos) {
 	owner := seedUser(t, ctx, r, "Owner")