Rebuild association tables, ensure file-system-based galleries cannot be changed (#2955)

* Re-create tables to include primary keys
* Filesystem-based galleries cannot change images

internal/api/resolver_mutation_gallery.go

@@ -13,7 +13,6 @@ import (
 	"github.com/stashapp/stash/pkg/image"
 	"github.com/stashapp/stash/pkg/models"
 	"github.com/stashapp/stash/pkg/plugin"
-	"github.com/stashapp/stash/pkg/sliceutil/intslice"
 	"github.com/stashapp/stash/pkg/sliceutil/stringslice"
 	"github.com/stashapp/stash/pkg/utils"
 )
@@ -422,13 +421,7 @@ func (r *mutationResolver) AddGalleryImages(ctx context.Context, input GalleryAd
 			return errors.New("gallery not found")
 		}
 
-		newIDs, err := qb.GetImageIDs(ctx, galleryID)
+		return r.galleryService.AddImages(ctx, gallery, imageIDs...)
-		if err != nil {
-			return err
-		}
-
-		newIDs = intslice.IntAppendUniques(newIDs, imageIDs)
-		return qb.UpdateImages(ctx, galleryID, newIDs)
 	}); err != nil {
 		return false, err
 	}
@@ -458,13 +451,7 @@ func (r *mutationResolver) RemoveGalleryImages(ctx context.Context, input Galler
 			return errors.New("gallery not found")
 		}
 
-		newIDs, err := qb.GetImageIDs(ctx, galleryID)
+		return r.galleryService.RemoveImages(ctx, gallery, imageIDs...)
-		if err != nil {
-			return err
-		}
-
-		newIDs = intslice.IntExclude(newIDs, imageIDs)
-		return qb.UpdateImages(ctx, galleryID, newIDs)
 	}); err != nil {
 		return false, err
 	}

internal/api/resolver_mutation_image.go

@@ -92,6 +92,15 @@ func (r *mutationResolver) imageUpdate(ctx context.Context, input ImageUpdateInp
 		return nil, err
 	}
 
+	i, err := r.repository.Image.Find(ctx, imageID)
+	if err != nil {
+		return nil, err
+	}
+
+	if i == nil {
+		return nil, fmt.Errorf("image not found %d", imageID)
+	}
+
 	updatedImage := models.NewImagePartial()
 	updatedImage.Title = translator.optionalString(input.Title, "title")
 	updatedImage.Rating = translator.optionalInt(input.Rating, "rating")
@@ -106,6 +115,15 @@ func (r *mutationResolver) imageUpdate(ctx context.Context, input ImageUpdateInp
 		if err != nil {
 			return nil, fmt.Errorf("converting gallery ids: %w", err)
 		}
+
+		// ensure gallery IDs are loaded
+		if err := i.LoadGalleryIDs(ctx, r.repository.Image); err != nil {
+			return nil, err
+		}
+
+		if err := r.galleryService.ValidateImageGalleryChange(ctx, i, *updatedImage.GalleryIDs); err != nil {
+			return nil, err
+		}
 	}
 
 	if translator.hasField("performer_ids") {
@@ -178,6 +196,26 @@ func (r *mutationResolver) BulkImageUpdate(ctx context.Context, input BulkImageU
 		qb := r.repository.Image
 
 		for _, imageID := range imageIDs {
+			i, err := r.repository.Image.Find(ctx, imageID)
+			if err != nil {
+				return err
+			}
+
+			if i == nil {
+				return fmt.Errorf("image not found %d", imageID)
+			}
+
+			if updatedImage.GalleryIDs != nil {
+				// ensure gallery IDs are loaded
+				if err := i.LoadGalleryIDs(ctx, r.repository.Image); err != nil {
+					return err
+				}
+
+				if err := r.galleryService.ValidateImageGalleryChange(ctx, i, *updatedImage.GalleryIDs); err != nil {
+					return err
+				}
+			}
+
 			image, err := qb.UpdatePartial(ctx, imageID, updatedImage)
 			if err != nil {
 				return err

internal/manager/repository.go

@@ -100,5 +100,10 @@ type ImageService interface {
 }
 
 type GalleryService interface {
+	AddImages(ctx context.Context, g *models.Gallery, toAdd ...int) error
+	RemoveImages(ctx context.Context, g *models.Gallery, toRemove ...int) error
+
 	Destroy(ctx context.Context, i *models.Gallery, fileDeleter *image.FileDeleter, deleteGenerated, deleteFile bool) ([]*models.Image, error)
+
+	ValidateImageGalleryChange(ctx context.Context, i *models.Image, updateIDs models.UpdateIDs) error
 }

pkg/gallery/service.go

@@ -13,9 +13,11 @@ type FinderByFile interface {
 }
 
 type Repository interface {
+	models.GalleryFinder
 	FinderByFile
 	Destroy(ctx context.Context, id int) error
 	models.FileLoader
+	ImageUpdater
 }
 
 type ImageFinder interface {

pkg/gallery/update.go

@@ -4,7 +4,6 @@ import (
 	"context"
 
 	"github.com/stashapp/stash/pkg/models"
-	"github.com/stashapp/stash/pkg/sliceutil/intslice"
 )
 
 type PartialUpdater interface {
@@ -13,17 +12,31 @@ type PartialUpdater interface {
 
 type ImageUpdater interface {
 	GetImageIDs(ctx context.Context, galleryID int) ([]int, error)
-	UpdateImages(ctx context.Context, galleryID int, imageIDs []int) error
+	AddImages(ctx context.Context, galleryID int, imageIDs ...int) error
+	RemoveImages(ctx context.Context, galleryID int, imageIDs ...int) error
 }
 
-func AddImage(ctx context.Context, qb ImageUpdater, galleryID int, imageID int) error {
+// AddImages adds images to the provided gallery.
-	imageIDs, err := qb.GetImageIDs(ctx, galleryID)
+// It returns an error if the gallery does not support adding images, or if
-	if err != nil {
+// the operation fails.
+func (s *Service) AddImages(ctx context.Context, g *models.Gallery, toAdd ...int) error {
+	if err := validateContentChange(g); err != nil {
 		return err
 	}
 
-	imageIDs = intslice.IntAppendUnique(imageIDs, imageID)
+	return s.Repository.AddImages(ctx, g.ID, toAdd...)
-	return qb.UpdateImages(ctx, galleryID, imageIDs)
+}
+
+// RemoveImages removes images from the provided gallery.
+// It does not validate if the images are part of the gallery.
+// It returns an error if the gallery does not support removing images, or if
+// the operation fails.
+func (s *Service) RemoveImages(ctx context.Context, g *models.Gallery, toRemove ...int) error {
+	if err := validateContentChange(g); err != nil {
+		return err
+	}
+
+	return s.Repository.RemoveImages(ctx, g.ID, toRemove...)
 }
 
 func AddPerformer(ctx context.Context, qb PartialUpdater, o *models.Gallery, performerID int) error {

pkg/gallery/validation.go

@@ -0,0 +1,60 @@
+package gallery
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/stashapp/stash/pkg/models"
+	"github.com/stashapp/stash/pkg/sliceutil/intslice"
+)
+
+type ContentsChangedError struct {
+	Gallery *models.Gallery
+}
+
+func (e *ContentsChangedError) Error() string {
+	typ := "zip-based"
+	if e.Gallery.FolderID != nil {
+		typ = "folder-based"
+	}
+
+	return fmt.Sprintf("cannot change contents of %s gallery %q", typ, e.Gallery.GetTitle())
+}
+
+// validateContentChange returns an error if a gallery cannot have its contents changed.
+// Only manually created galleries can have images changed.
+func validateContentChange(g *models.Gallery) error {
+	if g.FolderID != nil || g.PrimaryFileID != nil {
+		return &ContentsChangedError{
+			Gallery: g,
+		}
+	}
+
+	return nil
+}
+
+func (s *Service) ValidateImageGalleryChange(ctx context.Context, i *models.Image, updateIDs models.UpdateIDs) error {
+	// determine what is changing
+	var changedIDs []int
+
+	switch updateIDs.Mode {
+	case models.RelationshipUpdateModeAdd, models.RelationshipUpdateModeRemove:
+		changedIDs = updateIDs.IDs
+	case models.RelationshipUpdateModeSet:
+		// get the difference between the two lists
+		changedIDs = intslice.IntNotIntersect(i.GalleryIDs.List(), updateIDs.IDs)
+	}
+
+	galleries, err := s.Repository.FindMany(ctx, changedIDs)
+	if err != nil {
+		return err
+	}
+
+	for _, g := range galleries {
+		if err := validateContentChange(g); err != nil {
+			return fmt.Errorf("changing galleries of image %q: %w", i.GetTitle(), err)
+		}
+	}
+
+	return nil
+}

pkg/sliceutil/intslice/int_collections.go

@@ -65,6 +65,24 @@ func IntIntercect(v1, v2 []int) []int {
 	return ret
 }
 
+// IntNotIntersect returns a slice of ints containing values that do not exist in both provided slices.
+func IntNotIntersect(v1, v2 []int) []int {
+	var ret []int
+	for _, v := range v1 {
+		if !IntInclude(v2, v) {
+			ret = append(ret, v)
+		}
+	}
+
+	for _, v := range v2 {
+		if !IntInclude(v1, v) {
+			ret = append(ret, v)
+		}
+	}
+
+	return ret
+}
+
 // IntSliceToStringSlice converts a slice of ints to a slice of strings.
 func IntSliceToStringSlice(ss []int) []string {
 	ret := make([]string, len(ss))

pkg/sqlite/database.go

@@ -21,7 +21,7 @@ import (
 	"github.com/stashapp/stash/pkg/logger"
 )
 
-var appSchemaVersion uint = 34
+var appSchemaVersion uint = 35
 
 //go:embed migrations/*.sql
 var migrationsBox embed.FS

pkg/sqlite/gallery.go

@@ -1168,6 +1168,14 @@ func (qb *GalleryStore) GetImageIDs(ctx context.Context, galleryID int) ([]int,
 	return qb.imagesRepository().getIDs(ctx, galleryID)
 }
 
+func (qb *GalleryStore) AddImages(ctx context.Context, galleryID int, imageIDs ...int) error {
+	return qb.imagesRepository().insertOrIgnore(ctx, galleryID, imageIDs...)
+}
+
+func (qb *GalleryStore) RemoveImages(ctx context.Context, galleryID int, imageIDs ...int) error {
+	return qb.imagesRepository().destroyJoins(ctx, galleryID, imageIDs...)
+}
+
 func (qb *GalleryStore) UpdateImages(ctx context.Context, galleryID int, imageIDs []int) error {
 	// Delete the existing joins and then create new ones
 	return qb.imagesRepository().replace(ctx, galleryID, imageIDs)

pkg/sqlite/gallery_test.go

@@ -785,16 +785,16 @@ func Test_galleryQueryBuilder_UpdatePartialRelationships(t *testing.T) {
 
 			// only compare fields that were in the partial
 			if tt.partial.PerformerIDs != nil {
-				assert.Equal(tt.want.PerformerIDs, got.PerformerIDs)
+				assert.ElementsMatch(tt.want.PerformerIDs.List(), got.PerformerIDs.List())
-				assert.Equal(tt.want.PerformerIDs, s.PerformerIDs)
+				assert.ElementsMatch(tt.want.PerformerIDs.List(), s.PerformerIDs.List())
 			}
 			if tt.partial.TagIDs != nil {
-				assert.Equal(tt.want.TagIDs, got.TagIDs)
+				assert.ElementsMatch(tt.want.TagIDs.List(), got.TagIDs.List())
-				assert.Equal(tt.want.TagIDs, s.TagIDs)
+				assert.ElementsMatch(tt.want.TagIDs.List(), s.TagIDs.List())
 			}
 			if tt.partial.SceneIDs != nil {
-				assert.Equal(tt.want.SceneIDs, got.SceneIDs)
+				assert.ElementsMatch(tt.want.SceneIDs.List(), got.SceneIDs.List())
-				assert.Equal(tt.want.SceneIDs, s.SceneIDs)
+				assert.ElementsMatch(tt.want.SceneIDs.List(), s.SceneIDs.List())
 			}
 		})
 	}
@@ -2403,6 +2403,164 @@ func TestGalleryQuerySorting(t *testing.T) {
 	}
 }
 
+func TestGalleryStore_AddImages(t *testing.T) {
+	tests := []struct {
+		name      string
+		galleryID int
+		imageIDs  []int
+		wantErr   bool
+	}{
+		{
+			"single",
+			galleryIDs[galleryIdx1WithImage],
+			[]int{imageIDs[imageIdx1WithPerformer]},
+			false,
+		},
+		{
+			"multiple",
+			galleryIDs[galleryIdx1WithImage],
+			[]int{imageIDs[imageIdx1WithPerformer], imageIDs[imageIdx1WithStudio]},
+			false,
+		},
+		{
+			"invalid gallery id",
+			invalidID,
+			[]int{imageIDs[imageIdx1WithPerformer]},
+			true,
+		},
+		{
+			"single invalid",
+			galleryIDs[galleryIdx1WithImage],
+			[]int{invalidID},
+			true,
+		},
+		{
+			"one invalid",
+			galleryIDs[galleryIdx1WithImage],
+			[]int{imageIDs[imageIdx1WithPerformer], invalidID},
+			true,
+		},
+		{
+			"existing",
+			galleryIDs[galleryIdx1WithImage],
+			[]int{imageIDs[imageIdxWithGallery]},
+			false,
+		},
+		{
+			"one new",
+			galleryIDs[galleryIdx1WithImage],
+			[]int{imageIDs[imageIdx1WithPerformer], imageIDs[imageIdxWithGallery]},
+			false,
+		},
+	}
+
+	qb := db.Gallery
+
+	for _, tt := range tests {
+		runWithRollbackTxn(t, tt.name, func(t *testing.T, ctx context.Context) {
+			if err := qb.AddImages(ctx, tt.galleryID, tt.imageIDs...); (err != nil) != tt.wantErr {
+				t.Errorf("GalleryStore.AddImages() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			if tt.wantErr {
+				return
+			}
+
+			// ensure image was added
+			imageIDs, err := qb.GetImageIDs(ctx, tt.galleryID)
+			if err != nil {
+				t.Errorf("GalleryStore.GetImageIDs() error = %v", err)
+				return
+			}
+
+			assert := assert.New(t)
+			for _, wantedID := range tt.imageIDs {
+				assert.Contains(imageIDs, wantedID)
+			}
+		})
+	}
+}
+
+func TestGalleryStore_RemoveImages(t *testing.T) {
+	tests := []struct {
+		name      string
+		galleryID int
+		imageIDs  []int
+		wantErr   bool
+	}{
+		{
+			"single",
+			galleryIDs[galleryIdxWithTwoImages],
+			[]int{imageIDs[imageIdx1WithGallery]},
+			false,
+		},
+		{
+			"multiple",
+			galleryIDs[galleryIdxWithTwoImages],
+			[]int{imageIDs[imageIdx1WithGallery], imageIDs[imageIdx2WithGallery]},
+			false,
+		},
+		{
+			"invalid gallery id",
+			invalidID,
+			[]int{imageIDs[imageIdx1WithGallery]},
+			false,
+		},
+		{
+			"single invalid",
+			galleryIDs[galleryIdxWithTwoImages],
+			[]int{invalidID},
+			false,
+		},
+		{
+			"one invalid",
+			galleryIDs[galleryIdxWithTwoImages],
+			[]int{imageIDs[imageIdx1WithGallery], invalidID},
+			false,
+		},
+		{
+			"not existing",
+			galleryIDs[galleryIdxWithTwoImages],
+			[]int{imageIDs[imageIdxWithPerformer]},
+			false,
+		},
+		{
+			"one existing",
+			galleryIDs[galleryIdxWithTwoImages],
+			[]int{imageIDs[imageIdx1WithPerformer], imageIDs[imageIdx1WithGallery]},
+			false,
+		},
+	}
+
+	qb := db.Gallery
+
+	for _, tt := range tests {
+		runWithRollbackTxn(t, tt.name, func(t *testing.T, ctx context.Context) {
+			if err := qb.RemoveImages(ctx, tt.galleryID, tt.imageIDs...); (err != nil) != tt.wantErr {
+				t.Errorf("GalleryStore.RemoveImages() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			if tt.wantErr {
+				return
+			}
+
+			// ensure image was removed
+			imageIDs, err := qb.GetImageIDs(ctx, tt.galleryID)
+			if err != nil {
+				t.Errorf("GalleryStore.GetImageIDs() error = %v", err)
+				return
+			}
+
+			assert := assert.New(t)
+			for _, excludedID := range tt.imageIDs {
+				assert.NotContains(imageIDs, excludedID)
+			}
+		})
+	}
+}
+
 // TODO Count
 // TODO All
 // TODO Query

pkg/sqlite/image_test.go

@@ -768,16 +768,16 @@ func Test_imageQueryBuilder_UpdatePartialRelationships(t *testing.T) {
 
 			// only compare fields that were in the partial
 			if tt.partial.PerformerIDs != nil {
-				assert.Equal(tt.want.PerformerIDs, got.PerformerIDs)
+				assert.ElementsMatch(tt.want.PerformerIDs.List(), got.PerformerIDs.List())
-				assert.Equal(tt.want.PerformerIDs, s.PerformerIDs)
+				assert.ElementsMatch(tt.want.PerformerIDs.List(), s.PerformerIDs.List())
 			}
 			if tt.partial.TagIDs != nil {
-				assert.Equal(tt.want.TagIDs, got.TagIDs)
+				assert.ElementsMatch(tt.want.TagIDs.List(), got.TagIDs.List())
-				assert.Equal(tt.want.TagIDs, s.TagIDs)
+				assert.ElementsMatch(tt.want.TagIDs.List(), s.TagIDs.List())
 			}
 			if tt.partial.GalleryIDs != nil {
-				assert.Equal(tt.want.GalleryIDs, got.GalleryIDs)
+				assert.ElementsMatch(tt.want.GalleryIDs.List(), got.GalleryIDs.List())
-				assert.Equal(tt.want.GalleryIDs, s.GalleryIDs)
+				assert.ElementsMatch(tt.want.GalleryIDs.List(), s.GalleryIDs.List())
 			}
 		})
 	}

pkg/sqlite/migrations/35_assoc_tables.up.sql

@@ -0,0 +1,553 @@
+-- add primary keys to association tables that are missing them
+PRAGMA foreign_keys=OFF;
+
+CREATE TABLE `performers_image_new` (
+  `performer_id` integer primary key,
+  `image` blob not null,
+  foreign key(`performer_id`) references `performers`(`id`) on delete CASCADE
+);
+
+INSERT INTO `performers_image_new`
+  (
+    `performer_id`,
+    `image`
+  )
+  SELECT 
+    `performer_id`,
+    `image`
+  FROM `performers_image`;
+
+DROP TABLE `performers_image`;
+ALTER TABLE `performers_image_new` rename to `performers_image`;
+
+-- the following index is removed in favour of primary key
+-- CREATE UNIQUE INDEX `index_performer_image_on_performer_id` on `performers_image` (`performer_id`);
+
+
+CREATE TABLE `studios_image_new` (
+  `studio_id` integer primary key,
+  `image` blob not null,
+  foreign key(`studio_id`) references `studios`(`id`) on delete CASCADE
+);
+
+INSERT INTO `studios_image_new`
+  (
+    `studio_id`,
+    `image`
+  )
+  SELECT 
+    `studio_id`,
+    `image`
+  FROM `studios_image`;
+
+DROP TABLE `studios_image`;
+ALTER TABLE `studios_image_new` rename to `studios_image`;
+
+-- the following index is removed in favour of primary key
+-- CREATE UNIQUE INDEX `index_studio_image_on_studio_id` on `studios_image` (`studio_id`);
+
+
+CREATE TABLE `movies_images_new` (
+  `movie_id` integer primary key,
+  `front_image` blob not null,
+  `back_image` blob,
+  foreign key(`movie_id`) references `movies`(`id`) on delete CASCADE
+);
+
+INSERT INTO `movies_images_new`
+  (
+    `movie_id`,
+    `front_image`,
+    `back_image`
+  )
+  SELECT 
+    `movie_id`,
+    `front_image`,
+    `back_image`
+  FROM `movies_images`;
+
+DROP TABLE `movies_images`;
+ALTER TABLE `movies_images_new` rename to `movies_images`;
+
+-- the following index is removed in favour of primary key
+-- CREATE UNIQUE INDEX `index_movie_images_on_movie_id` on `movies_images` (`movie_id`);
+
+
+CREATE TABLE `tags_image_new` (
+  `tag_id` integer primary key,
+  `image` blob not null,
+  foreign key(`tag_id`) references `tags`(`id`) on delete CASCADE
+);
+
+INSERT INTO `tags_image_new`
+  (
+    `tag_id`,
+    `image`
+  )
+  SELECT 
+    `tag_id`,
+    `image`
+  FROM `tags_image`;
+
+DROP TABLE `tags_image`;
+ALTER TABLE `tags_image_new` rename to `tags_image`;
+
+-- the following index is removed in favour of primary key
+-- CREATE UNIQUE INDEX `index_tag_image_on_tag_id` on `tags_image` (`tag_id`);
+
+-- add on delete cascade to foreign keys
+CREATE TABLE `performers_scenes_new` (
+  `performer_id` integer,
+  `scene_id` integer,
+  foreign key(`performer_id`) references `performers`(`id`) on delete CASCADE,
+  foreign key(`scene_id`) references `scenes`(`id`) on delete CASCADE,
+  PRIMARY KEY (`scene_id`, `performer_id`)
+);
+
+INSERT INTO `performers_scenes_new`
+  (
+    `performer_id`,
+    `scene_id`
+  )
+  SELECT 
+    `performer_id`,
+    `scene_id`
+  FROM `performers_scenes`;
+
+DROP TABLE `performers_scenes`;
+ALTER TABLE `performers_scenes_new` rename to `performers_scenes`;
+
+CREATE INDEX `index_performers_scenes_on_performer_id` on `performers_scenes` (`performer_id`);
+
+-- the following index is removed in favour of primary key
+-- CREATE INDEX `index_performers_scenes_on_scene_id` on `performers_scenes` (`scene_id`);
+
+
+CREATE TABLE `scene_markers_tags_new` (
+  `scene_marker_id` integer,
+  `tag_id` integer,
+  foreign key(`scene_marker_id`) references `scene_markers`(`id`) on delete CASCADE,
+  foreign key(`tag_id`) references `tags`(`id`) on delete CASCADE,
+  PRIMARY KEY(`scene_marker_id`, `tag_id`)
+);
+
+INSERT INTO `scene_markers_tags_new`
+  (
+    `scene_marker_id`,
+    `tag_id`
+  )
+  SELECT 
+    `scene_marker_id`,
+    `tag_id`
+  FROM `scene_markers_tags` WHERE true
+  ON CONFLICT (`scene_marker_id`, `tag_id`) DO NOTHING;
+
+DROP TABLE `scene_markers_tags`;
+ALTER TABLE `scene_markers_tags_new` rename to `scene_markers_tags`;
+
+CREATE INDEX `index_scene_markers_tags_on_tag_id` on `scene_markers_tags` (`tag_id`);
+
+-- the following index is removed in favour of primary key
+-- CREATE INDEX `index_scene_markers_tags_on_scene_marker_id` on `scene_markers_tags` (`scene_marker_id`);
+
+-- add delete cascade to tag_id
+CREATE TABLE `scenes_tags_new` (
+  `scene_id` integer,
+  `tag_id` integer,
+  foreign key(`scene_id`) references `scenes`(`id`) on delete CASCADE,
+  foreign key(`tag_id`) references `tags`(`id`) on delete CASCADE,
+  PRIMARY KEY(`scene_id`, `tag_id`)
+);
+
+INSERT INTO `scenes_tags_new`
+  (
+    `scene_id`,
+    `tag_id`
+  )
+  SELECT 
+    `scene_id`,
+    `tag_id`
+  FROM `scenes_tags` WHERE true
+  ON CONFLICT (`scene_id`, `tag_id`) DO NOTHING;
+
+DROP TABLE `scenes_tags`;
+ALTER TABLE `scenes_tags_new` rename to `scenes_tags`;
+
+CREATE INDEX `index_scenes_tags_on_tag_id` on `scenes_tags` (`tag_id`);
+
+-- the following index is removed in favour of primary key
+-- CREATE INDEX `index_scenes_tags_on_scene_id` on `scenes_tags` (`scene_id`);
+
+
+CREATE TABLE `movies_scenes_new` (
+  `movie_id` integer,
+  `scene_id` integer,
+  `scene_index` tinyint,
+  foreign key(`movie_id`) references `movies`(`id`) on delete cascade,
+  foreign key(`scene_id`) references `scenes`(`id`) on delete cascade,
+  PRIMARY KEY(`movie_id`, `scene_id`)
+);
+
+INSERT INTO `movies_scenes_new`
+  (
+    `movie_id`,
+    `scene_id`,
+    `scene_index`
+  )
+  SELECT 
+    `movie_id`,
+    `scene_id`,
+    `scene_index`
+  FROM `movies_scenes`;
+
+DROP TABLE `movies_scenes`;
+ALTER TABLE `movies_scenes_new` rename to `movies_scenes`;
+
+CREATE INDEX `index_movies_scenes_on_movie_id` on `movies_scenes` (`movie_id`);
+
+-- the following index is removed in favour of primary key
+-- CREATE INDEX `index_movies_scenes_on_scene_id` on `movies_scenes` (`scene_id`);
+
+
+CREATE TABLE `scenes_cover_new` (
+  `scene_id` integer primary key,
+  `cover` blob not null,
+  foreign key(`scene_id`) references `scenes`(`id`) on delete CASCADE
+);
+
+INSERT INTO `scenes_cover_new`
+  (
+    `scene_id`,
+    `cover`
+  )
+  SELECT 
+    `scene_id`,
+    `cover`
+  FROM `scenes_cover`;
+
+DROP TABLE `scenes_cover`;
+ALTER TABLE `scenes_cover_new` rename to `scenes_cover`;
+
+-- the following index is removed in favour of primary key
+-- CREATE UNIQUE INDEX `index_scene_covers_on_scene_id` on `scenes_cover` (`scene_id`);
+
+
+CREATE TABLE `performers_images_new` (
+  `performer_id` integer,
+  `image_id` integer,
+  foreign key(`performer_id`) references `performers`(`id`) on delete CASCADE,
+  foreign key(`image_id`) references `images`(`id`) on delete CASCADE,
+  PRIMARY KEY(`image_id`, `performer_id`)
+);
+
+INSERT INTO `performers_images_new`
+  (
+    `performer_id`,
+    `image_id`
+  )
+  SELECT 
+    `performer_id`,
+    `image_id`
+  FROM `performers_images`;
+
+DROP TABLE `performers_images`;
+ALTER TABLE `performers_images_new` rename to `performers_images`;
+
+CREATE INDEX `index_performers_images_on_performer_id` on `performers_images` (`performer_id`);
+
+-- the following index is removed in favour of primary key
+-- CREATE INDEX `index_performers_images_on_image_id` on `performers_images` (`image_id`);
+
+
+CREATE TABLE `images_tags_new` (
+  `image_id` integer,
+  `tag_id` integer,
+  foreign key(`image_id`) references `images`(`id`) on delete CASCADE,
+  foreign key(`tag_id`) references `tags`(`id`) on delete CASCADE,
+  PRIMARY KEY(`image_id`, `tag_id`)
+);
+
+INSERT INTO `images_tags_new`
+  (
+    `image_id`,
+    `tag_id`
+  )
+  SELECT 
+    `image_id`,
+    `tag_id`
+  FROM `images_tags` WHERE true
+  ON CONFLICT (`image_id`, `tag_id`) DO NOTHING;
+
+DROP TABLE `images_tags`;
+ALTER TABLE `images_tags_new` rename to `images_tags`;
+
+CREATE INDEX `index_images_tags_on_tag_id` on `images_tags` (`tag_id`);
+
+-- the following index is removed in favour of primary key
+-- CREATE INDEX `index_images_tags_on_image_id` on `images_tags` (`image_id`);
+
+
+CREATE TABLE `scene_stash_ids_new` (
+  `scene_id` integer NOT NULL,
+  `endpoint` varchar(255) NOT NULL,
+  `stash_id` varchar(36) NOT NULL,
+  foreign key(`scene_id`) references `scenes`(`id`) on delete CASCADE,
+  PRIMARY KEY(`scene_id`, `endpoint`)
+);
+
+INSERT INTO `scene_stash_ids_new`
+  (
+    `scene_id`,
+    `endpoint`,
+    `stash_id`
+  )
+  SELECT 
+    `scene_id`,
+    `endpoint`,
+    `stash_id`
+  FROM `scene_stash_ids`;
+
+DROP TABLE `scene_stash_ids`;
+ALTER TABLE `scene_stash_ids_new` rename to `scene_stash_ids`;
+
+-- the following index is removed in favour of primary key
+-- CREATE INDEX `index_scene_stash_ids_on_scene_id` ON `scene_stash_ids` (`scene_id`);
+
+
+CREATE TABLE `performer_stash_ids_new` (
+  `performer_id` integer NOT NULL,
+  `endpoint` varchar(255) NOT NULL,
+  `stash_id` varchar(36) NOT NULL,
+  foreign key(`performer_id`) references `performers`(`id`) on delete CASCADE,
+  PRIMARY KEY(`performer_id`, `endpoint`)
+);
+
+INSERT INTO `performer_stash_ids_new`
+  (
+    `performer_id`,
+    `endpoint`,
+    `stash_id`
+  )
+  SELECT 
+    `performer_id`,
+    `endpoint`,
+    `stash_id`
+  FROM `performer_stash_ids`;
+
+DROP TABLE `performer_stash_ids`;
+ALTER TABLE `performer_stash_ids_new` rename to `performer_stash_ids`;
+
+-- the following index is removed in favour of primary key
+-- CREATE INDEX `index_performer_stash_ids_on_performer_id` ON `performer_stash_ids` (`performer_id`);
+
+
+CREATE TABLE `studio_stash_ids_new` (
+  `studio_id` integer NOT NULL,
+  `endpoint` varchar(255) NOT NULL,
+  `stash_id` varchar(36) NOT NULL,
+  foreign key(`studio_id`) references `studios`(`id`) on delete CASCADE,
+  PRIMARY KEY(`studio_id`, `endpoint`)
+);
+
+INSERT INTO `studio_stash_ids_new`
+  (
+    `studio_id`,
+    `endpoint`,
+    `stash_id`
+  )
+  SELECT 
+    `studio_id`,
+    `endpoint`,
+    `stash_id`
+  FROM `studio_stash_ids`;
+
+DROP TABLE `studio_stash_ids`;
+ALTER TABLE `studio_stash_ids_new` rename to `studio_stash_ids`;
+
+-- the following index is removed in favour of primary key
+-- CREATE INDEX `index_studio_stash_ids_on_studio_id` ON `studio_stash_ids` (`studio_id`);
+
+
+CREATE TABLE `scenes_galleries_new` (
+  `scene_id` integer NOT NULL,
+  `gallery_id` integer NOT NULL,
+  foreign key(`scene_id`) references `scenes`(`id`) on delete CASCADE,
+  foreign key(`gallery_id`) references `galleries`(`id`) on delete CASCADE,
+  PRIMARY KEY(`scene_id`, `gallery_id`)
+);
+
+INSERT INTO `scenes_galleries_new`
+  (
+    `scene_id`,
+    `gallery_id`
+  )
+  SELECT 
+    `scene_id`,
+    `gallery_id`
+  FROM `scenes_galleries`;
+
+DROP TABLE `scenes_galleries`;
+ALTER TABLE `scenes_galleries_new` rename to `scenes_galleries`;
+
+CREATE INDEX `index_scenes_galleries_on_gallery_id` on `scenes_galleries` (`gallery_id`);
+
+-- the following index is removed in favour of primary key
+-- CREATE INDEX `index_scenes_galleries_on_scene_id` on `scenes_galleries` (`scene_id`);
+
+
+CREATE TABLE `galleries_images_new` (
+  `gallery_id` integer NOT NULL,
+  `image_id` integer NOT NULL,
+  foreign key(`gallery_id`) references `galleries`(`id`) on delete CASCADE,
+  foreign key(`image_id`) references `images`(`id`) on delete CASCADE,
+  PRIMARY KEY(`gallery_id`, `image_id`)
+);
+
+INSERT INTO `galleries_images_new`
+  (
+    `gallery_id`,
+    `image_id`
+  )
+  SELECT 
+    `gallery_id`,
+    `image_id`
+  FROM `galleries_images`;
+
+DROP TABLE `galleries_images`;
+ALTER TABLE `galleries_images_new` rename to `galleries_images`;
+
+CREATE INDEX `index_galleries_images_on_image_id` on `galleries_images` (`image_id`);
+
+-- the following index is removed in favour of primary key
+-- CREATE INDEX `index_galleries_images_on_gallery_id` on `galleries_images` (`gallery_id`);
+
+
+CREATE TABLE `performers_galleries_new` (
+  `performer_id` integer NOT NULL,
+  `gallery_id` integer NOT NULL,
+  foreign key(`performer_id`) references `performers`(`id`) on delete CASCADE,
+  foreign key(`gallery_id`) references `galleries`(`id`) on delete CASCADE,
+  PRIMARY KEY(`gallery_id`, `performer_id`)
+);
+
+INSERT INTO `performers_galleries_new`
+  (
+    `performer_id`,
+    `gallery_id`
+  )
+  SELECT 
+    `performer_id`,
+    `gallery_id`
+  FROM `performers_galleries`;
+
+DROP TABLE `performers_galleries`;
+ALTER TABLE `performers_galleries_new` rename to `performers_galleries`;
+
+CREATE INDEX `index_performers_galleries_on_performer_id` on `performers_galleries` (`performer_id`);
+
+-- the following index is removed in favour of primary key
+-- CREATE INDEX `index_performers_galleries_on_gallery_id` on `performers_galleries` (`gallery_id`);
+
+
+CREATE TABLE `galleries_tags_new` (
+  `gallery_id` integer NOT NULL,
+  `tag_id` integer NOT NULL,
+  foreign key(`gallery_id`) references `galleries`(`id`) on delete CASCADE,
+  foreign key(`tag_id`) references `tags`(`id`) on delete CASCADE,
+  PRIMARY KEY(`gallery_id`, `tag_id`)
+);
+
+INSERT INTO `galleries_tags_new`
+  (
+    `gallery_id`,
+    `tag_id`
+  )
+  SELECT 
+    `gallery_id`,
+    `tag_id`
+  FROM `galleries_tags` WHERE true
+  ON CONFLICT (`gallery_id`, `tag_id`) DO NOTHING;
+
+DROP TABLE `galleries_tags`;
+ALTER TABLE `galleries_tags_new` rename to `galleries_tags`;
+
+CREATE INDEX `index_galleries_tags_on_tag_id` on `galleries_tags` (`tag_id`);
+
+-- the following index is removed in favour of primary key
+-- CREATE INDEX `index_galleries_tags_on_gallery_id` on `galleries_tags` (`gallery_id`);
+
+
+CREATE TABLE `performers_tags_new` (
+  `performer_id` integer NOT NULL,
+  `tag_id` integer NOT NULL,
+  foreign key(`performer_id`) references `performers`(`id`) on delete CASCADE,
+  foreign key(`tag_id`) references `tags`(`id`) on delete CASCADE,
+  PRIMARY KEY(`performer_id`, `tag_id`)
+);
+
+INSERT INTO `performers_tags_new`
+  (
+    `performer_id`,
+    `tag_id`
+  )
+  SELECT 
+    `performer_id`,
+    `tag_id`
+  FROM `performers_tags` WHERE true
+  ON CONFLICT (`performer_id`, `tag_id`) DO NOTHING;
+
+DROP TABLE `performers_tags`;
+ALTER TABLE `performers_tags_new` rename to `performers_tags`;
+
+CREATE INDEX `index_performers_tags_on_tag_id` on `performers_tags` (`tag_id`);
+
+-- the following index is removed in favour of primary key
+-- CREATE INDEX `index_performers_tags_on_performer_id` on `performers_tags` (`performer_id`);
+
+
+CREATE TABLE `tag_aliases_new` (
+  `tag_id` integer NOT NULL,
+  `alias` varchar(255) NOT NULL,
+  foreign key(`tag_id`) references `tags`(`id`) on delete CASCADE,
+  PRIMARY KEY(`tag_id`, `alias`)
+);
+
+INSERT INTO `tag_aliases_new`
+  (
+    `tag_id`,
+    `alias`
+  )
+  SELECT 
+    `tag_id`,
+    `alias`
+  FROM `tag_aliases`;
+
+DROP TABLE `tag_aliases`;
+ALTER TABLE `tag_aliases_new` rename to `tag_aliases`;
+
+CREATE UNIQUE INDEX `tag_aliases_alias_unique` on `tag_aliases` (`alias`);
+
+
+CREATE TABLE `studio_aliases_new` (
+  `studio_id` integer NOT NULL,
+  `alias` varchar(255) NOT NULL,
+  foreign key(`studio_id`) references `studios`(`id`) on delete CASCADE,
+  PRIMARY KEY(`studio_id`, `alias`)
+);
+
+INSERT INTO `studio_aliases_new`
+  (
+    `studio_id`,
+    `alias`
+  )
+  SELECT 
+    `studio_id`,
+    `alias`
+  FROM `studio_aliases`;
+
+DROP TABLE `studio_aliases`;
+ALTER TABLE `studio_aliases_new` rename to `studio_aliases`;
+
+CREATE UNIQUE INDEX `studio_aliases_alias_unique` on `studio_aliases` (`alias`);
+
+PRAGMA foreign_keys=ON;

pkg/sqlite/repository.go

@@ -324,9 +324,53 @@ func (r *joinRepository) getIDs(ctx context.Context, id int) ([]int, error) {
 	return r.runIdsQuery(ctx, query, []interface{}{id})
 }
 
-func (r *joinRepository) insert(ctx context.Context, id, foreignID int) (sql.Result, error) {
+func (r *joinRepository) insert(ctx context.Context, id int, foreignIDs ...int) error {
-	stmt := fmt.Sprintf("INSERT INTO %s (%s, %s) VALUES (?, ?)", r.tableName, r.idColumn, r.fkColumn)
+	stmt, err := r.tx.Prepare(ctx, fmt.Sprintf("INSERT INTO %s (%s, %s) VALUES (?, ?)", r.tableName, r.idColumn, r.fkColumn))
-	return r.tx.Exec(ctx, stmt, id, foreignID)
+	if err != nil {
+		return err
+	}
+
+	defer stmt.Close()
+
+	for _, fk := range foreignIDs {
+		if _, err := r.tx.ExecStmt(ctx, stmt, id, fk); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// insertOrIgnore inserts a join into the table, silently failing in the event that a conflict occurs (ie when the join already exists)
+func (r *joinRepository) insertOrIgnore(ctx context.Context, id int, foreignIDs ...int) error {
+	stmt, err := r.tx.Prepare(ctx, fmt.Sprintf("INSERT INTO %s (%s, %s) VALUES (?, ?) ON CONFLICT (%[2]s, %s) DO NOTHING", r.tableName, r.idColumn, r.fkColumn))
+	if err != nil {
+		return err
+	}
+
+	defer stmt.Close()
+
+	for _, fk := range foreignIDs {
+		if _, err := r.tx.ExecStmt(ctx, stmt, id, fk); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (r *joinRepository) destroyJoins(ctx context.Context, id int, foreignIDs ...int) error {
+	stmt := fmt.Sprintf("DELETE FROM %s WHERE %s = ? AND %s IN %s", r.tableName, r.idColumn, r.fkColumn, getInBinding(len(foreignIDs)))
+
+	args := make([]interface{}, len(foreignIDs)+1)
+	args[0] = id
+	for i, v := range foreignIDs {
+		args[i+1] = v
+	}
+
+	if _, err := r.tx.Exec(ctx, stmt, args...); err != nil {
+		return err
+	}
+
+	return nil
 }
 
 func (r *joinRepository) replace(ctx context.Context, id int, foreignIDs []int) error {
@@ -335,7 +379,7 @@ func (r *joinRepository) replace(ctx context.Context, id int, foreignIDs []int)
 	}
 
 	for _, fk := range foreignIDs {
-		if _, err := r.insert(ctx, id, fk); err != nil {
+		if err := r.insert(ctx, id, fk); err != nil {
 			return err
 		}
 	}

pkg/sqlite/scene_test.go

@@ -1160,24 +1160,24 @@ func Test_sceneQueryBuilder_UpdatePartialRelationships(t *testing.T) {
 
 			// only compare fields that were in the partial
 			if tt.partial.PerformerIDs != nil {
-				assert.Equal(tt.want.PerformerIDs, got.PerformerIDs)
+				assert.ElementsMatch(tt.want.PerformerIDs.List(), got.PerformerIDs.List())
-				assert.Equal(tt.want.PerformerIDs, s.PerformerIDs)
+				assert.ElementsMatch(tt.want.PerformerIDs.List(), s.PerformerIDs.List())
 			}
 			if tt.partial.TagIDs != nil {
-				assert.Equal(tt.want.TagIDs, got.TagIDs)
+				assert.ElementsMatch(tt.want.TagIDs.List(), got.TagIDs.List())
-				assert.Equal(tt.want.TagIDs, s.TagIDs)
+				assert.ElementsMatch(tt.want.TagIDs.List(), s.TagIDs.List())
 			}
 			if tt.partial.GalleryIDs != nil {
-				assert.Equal(tt.want.GalleryIDs, got.GalleryIDs)
+				assert.ElementsMatch(tt.want.GalleryIDs.List(), got.GalleryIDs.List())
-				assert.Equal(tt.want.GalleryIDs, s.GalleryIDs)
+				assert.ElementsMatch(tt.want.GalleryIDs.List(), s.GalleryIDs.List())
 			}
 			if tt.partial.MovieIDs != nil {
-				assert.Equal(tt.want.Movies, got.Movies)
+				assert.ElementsMatch(tt.want.Movies.List(), got.Movies.List())
-				assert.Equal(tt.want.Movies, s.Movies)
+				assert.ElementsMatch(tt.want.Movies.List(), s.Movies.List())
 			}
 			if tt.partial.StashIDs != nil {
-				assert.Equal(tt.want.StashIDs, got.StashIDs)
+				assert.ElementsMatch(tt.want.StashIDs.List(), got.StashIDs.List())
-				assert.Equal(tt.want.StashIDs, s.StashIDs)
+				assert.ElementsMatch(tt.want.StashIDs.List(), s.StashIDs.List())
 			}
 		})
 	}

pkg/sqlite/tag.go

@@ -684,13 +684,16 @@ func (qb *tagQueryBuilder) Merge(ctx context.Context, source []int, destination
 	inBinding := getInBinding(len(source))
 
 	args := []interface{}{destination}
-	for _, id := range source {
+	srcArgs := make([]interface{}, len(source))
+	for i, id := range source {
 		if id == destination {
 			return errors.New("cannot merge where source == destination")
 		}
-		args = append(args, id)
+		srcArgs[i] = id
 	}
 
+	args = append(args, srcArgs...)
+
 	tagTables := map[string]string{
 		scenesTagsTable:      sceneIDColumn,
 		"scene_markers_tags": "scene_marker_id",
@@ -701,7 +704,7 @@ func (qb *tagQueryBuilder) Merge(ctx context.Context, source []int, destination
 
 	args = append(args, destination)
 	for table, idColumn := range tagTables {
-		_, err := qb.tx.Exec(ctx, `UPDATE `+table+`
+		_, err := qb.tx.Exec(ctx, `UPDATE OR IGNORE `+table+`
 SET tag_id = ?
 WHERE tag_id IN `+inBinding+`
 AND NOT EXISTS(SELECT 1 FROM `+table+` o WHERE o.`+idColumn+` = `+table+`.`+idColumn+` AND o.tag_id = ?)`,
@@ -710,6 +713,11 @@ AND NOT EXISTS(SELECT 1 FROM `+table+` o WHERE o.`+idColumn+` = `+table+`.`+idCo
 		if err != nil {
 			return err
 		}
+
+		// delete source tag ids from the table where they couldn't be set
+		if _, err := qb.tx.Exec(ctx, `DELETE FROM `+table+` WHERE tag_id IN `+inBinding, srcArgs...); err != nil {
+			return err
+		}
 	}
 
 	_, err := qb.tx.Exec(ctx, "UPDATE "+sceneMarkerTable+" SET primary_tag_id = ? WHERE primary_tag_id IN "+inBinding, args...)

pkg/sqlite/tx.go

@@ -21,6 +21,11 @@ type dbReader interface {
 	QueryxContext(ctx context.Context, query string, args ...interface{}) (*sqlx.Rows, error)
 }
 
+type stmt struct {
+	*sql.Stmt
+	query string
+}
+
 func logSQL(start time.Time, query string, args ...interface{}) {
 	since := time.Since(start)
 	if since >= slowLogTime {
@@ -117,3 +122,35 @@ func (*dbWrapper) Exec(ctx context.Context, query string, args ...interface{}) (
 
 	return ret, sqlError(err, query, args...)
 }
+
+// Prepare creates a prepared statement.
+func (*dbWrapper) Prepare(ctx context.Context, query string, args ...interface{}) (*stmt, error) {
+	tx, err := getTx(ctx)
+	if err != nil {
+		return nil, sqlError(err, query, args...)
+	}
+
+	// nolint:sqlclosecheck
+	ret, err := tx.PrepareContext(ctx, query)
+	if err != nil {
+		return nil, sqlError(err, query, args...)
+	}
+
+	return &stmt{
+		query: query,
+		Stmt:  ret,
+	}, nil
+}
+
+func (*dbWrapper) ExecStmt(ctx context.Context, stmt *stmt, args ...interface{}) (sql.Result, error) {
+	_, err := getTx(ctx)
+	if err != nil {
+		return nil, sqlError(err, stmt.query, args...)
+	}
+
+	start := time.Now()
+	ret, err := stmt.ExecContext(ctx, args...)
+	logSQL(start, stmt.query, args...)
+
+	return ret, sqlError(err, stmt.query, args...)
+}