WIP: removed some complexity from data service

internal/common/utils.go

@@ -42,3 +42,15 @@ func HashFromData(data []byte) (string, error) {
 
 	return fmt.Sprintf("%x", hash.Sum(nil)), nil
 }
+
+func RemoveDuplicates[T comparable](sliceList []T) []T {
+	allKeys := make(map[T]bool)
+	list := []T{}
+	for _, item := range sliceList {
+		if _, value := allKeys[item]; !value {
+			allKeys[item] = true
+			list = append(list, item)
+		}
+	}
+	return list
+}

internal/database/db_data_service.go

@@ -86,7 +86,7 @@ func (d *dbService) initDbTuneForCreation(
 
 	err = copier.Copy(&dbTune, &ct)
 	if err != nil {
-		return nil, fmt.Errorf("could not create db tune object")
+		return nil, fmt.Errorf("could not create db tune object: %v", err)
 	}
 	if tuneType != nil {
 		dbTune.TuneTypeID = &tuneType.ID
@@ -274,16 +274,48 @@ func (d *dbService) CreateMusicSet(
 		return nil, fmt.Errorf("can't create music set without a title")
 	}
 
-	dbSet := model.MusicSet{}
+	dbSet, err := d.initDbSetForCreation(musicSet, importFile)
+	if err != nil {
+		return nil, err
+	}
+
+	apiSet, err := d.createMusicSetWithTuneIDs(
+		dbSet,
+		musicSet.Tunes,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	return apiSet, nil
+}
+
+func (d *dbService) initDbSetForCreation(
+	musicSet apimodel.CreateSet,
+	importFile *model.ImportFile,
+) (*model.MusicSet, error) {
+	dbSet := &model.MusicSet{}
 	if importFile != nil {
 		dbSet.ImportFileID = importFile.ID
 	}
-	if err := copier.Copy(&dbSet, &musicSet); err != nil {
-		return &apimodel.MusicSet{}, fmt.Errorf("could not create db object")
+
+	err := copier.Copy(dbSet, &musicSet)
+	if err != nil {
+		return nil, fmt.Errorf("could not create db music set object: %v", err)
 	}
+
+	// reset tunes to nil as copier creates an empty tune object for every tune id
+	// this leads to a foreign key constraint violation
 	dbSet.Tunes = nil
 
-	newTunes, err := d.dbTunesFromIDs(musicSet.Tunes)
+	return dbSet, nil
+}
+
+func (d *dbService) createMusicSetWithTuneIDs(
+	dbSet *model.MusicSet,
+	tuneIDs []uuid.UUID,
+) (*apimodel.MusicSet, error) {
+	mSetTunes, err := d.dbTunesFromIDs(tuneIDs)
 	if err != nil {
 		return nil, err
 	}
@@ -293,7 +325,7 @@ func (d *dbService) CreateMusicSet(
 			return err
 		}
 
-		if err := d.assignMusicSetTunes(dbSet.ID, musicSet.Tunes); err != nil {
+		if err := d.assignMusicSetTunes(dbSet.ID, tuneIDs); err != nil {
 			return err
 		}
 
@@ -303,10 +335,10 @@ func (d *dbService) CreateMusicSet(
 		return nil, err
 	}
 
-	dbSet.Tunes = newTunes
-	apiSet, err := apiSetFromDbSet(&dbSet)
+	dbSet.Tunes = mSetTunes
+	apiSet, err := apiSetFromDbSet(dbSet)
 	if err != nil {
-		return &apimodel.MusicSet{}, err
+		return nil, err
 	}
 
 	return apiSet, nil
@@ -344,6 +376,7 @@ func apiSetFromDbSet(dbSet *model.MusicSet) (*apimodel.MusicSet, error) {
 	return apiSet, nil
 }
 
+// returns that music set that contains all tunes in the given order
 func (d *dbService) getMusicSetByTuneIDs(tuneIDs []uuid.UUID) (*apimodel.MusicSet, error) {
 	allMusicSets, err := d.MusicSets()
 	if err != nil {
@@ -352,18 +385,7 @@ func (d *dbService) getMusicSetByTuneIDs(tuneIDs []uuid.UUID) (*apimodel.MusicSe
 
 	var matchingSet *apimodel.MusicSet
 	for _, set := range allMusicSets {
-		if len(set.Tunes) != len(tuneIDs) {
-			continue
-		}
-
-		allTunesMatch := true
-		for i, t := range set.Tunes {
-			if tuneIDs[i] != t.Id {
-				allTunesMatch = false
-				break
-			}
-		}
-		if allTunesMatch {
+		if musicSetHasTunesInOrder(set, tuneIDs) {
 			matchingSet = set
 			break
 		}
@@ -376,6 +398,23 @@ func (d *dbService) getMusicSetByTuneIDs(tuneIDs []uuid.UUID) (*apimodel.MusicSe
 	return matchingSet, nil
 }
 
+func musicSetHasTunesInOrder(
+	set *apimodel.MusicSet,
+	tuneIDs []uuid.UUID,
+) bool {
+	if len(set.Tunes) != len(tuneIDs) {
+		return false
+	}
+
+	for i, t := range set.Tunes {
+		if t.Id != tuneIDs[i] {
+			return false
+		}
+	}
+
+	return true
+}
+
 func (d *dbService) setTunesInAPISet(apiSet *apimodel.MusicSet) error {
 	var setTunes []model.Tune
 	err := d.db.Joins("JOIN music_set_tunes mst on tunes.id = mst.tune_id").
@@ -478,18 +517,7 @@ func (d *dbService) AssignTunesToMusicSet(
 		return nil, err
 	}
 
-	// delete old music set -> tune relations and create new ones
-	err = d.db.Transaction(func(_ *gorm.DB) error {
-		if err := d.deleteMusicSetTunes(set); err != nil {
-			return err
-		}
-
-		if err := d.assignMusicSetTunes(set.ID, tuneIDs); err != nil {
-			return err
-		}
-
-		return nil
-	})
+	err = d.replaceMusicSetTuneRelations(set, tuneIDs)
 	if err != nil {
 		return nil, err
 	}
@@ -505,6 +533,29 @@ func (d *dbService) AssignTunesToMusicSet(
 	return apiSet, nil
 }
 
+func (d *dbService) replaceMusicSetTuneRelations(
+	set *model.MusicSet,
+	tuneIDs []uuid.UUID,
+) error {
+	// delete old music set-tune relations and create new ones
+	err := d.db.Transaction(func(_ *gorm.DB) error {
+		if err := d.deleteMusicSetTunes(set); err != nil {
+			return err
+		}
+
+		if err := d.assignMusicSetTunes(set.ID, tuneIDs); err != nil {
+			return err
+		}
+
+		return nil
+	})
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // dbTunesFromIDs returns the database tune objects in the same order as the
 // given tuneIDs. If there is an id that belongs to a non existing tune,
 // an error will be returned.
@@ -513,18 +564,7 @@ func (d *dbService) dbTunesFromIDs(tuneIDs []uuid.UUID) ([]model.Tune, error) {
 		return nil, nil
 	}
 
-	var distinctTuneIDs []uuid.UUID
-	for _, id := range tuneIDs {
-		inDistinct := false
-		for _, distTuneID := range distinctTuneIDs {
-			if id == distTuneID {
-				inDistinct = true
-			}
-		}
-		if !inDistinct {
-			distinctTuneIDs = append(distinctTuneIDs, id)
-		}
-	}
+	distinctTuneIDs := common.RemoveDuplicates(tuneIDs)
 
 	var dbTunes []model.Tune
 	if err := d.db.Where("id IN (?)", distinctTuneIDs).Find(&dbTunes).Error; err != nil {
@@ -687,26 +727,28 @@ func (d *dbService) ImportTunes(
 func (d *dbService) getExistingImportedTune(
 	impTune *messages.ImportedTune,
 ) (*apimodel.ImportTune, error) {
-	hasData, err := d.hasSingleFileData(impTune.TuneFileData)
+	hasData, err := d.hasSingleTuneFileData(impTune.TuneFileData)
 	if err != nil {
 		return nil, err
 	}
 
-	if hasData {
-		alreadyImportedTune, err := d.getTuneWithSingleFileData(impTune.TuneFileData)
-		if err != nil {
-			return nil, err
-		}
+	if !hasData {
+		return nil, common.ErrNotFound
+	}
 
-		if alreadyImportedTune != nil {
-			impTune := &apimodel.ImportTune{}
-			err = copier.Copy(impTune, alreadyImportedTune)
-			if err != nil {
-				return nil, fmt.Errorf("failed creating import tune from already imported tune: %s", err.Error())
-			}
+	alreadyImportedTune, err := d.getTuneWithSingleFileData(impTune.TuneFileData)
+	if err != nil {
+		return nil, err
+	}
 
-			return impTune, nil
+	if alreadyImportedTune != nil {
+		impTune := &apimodel.ImportTune{}
+		err = copier.Copy(impTune, alreadyImportedTune)
+		if err != nil {
+			return nil, fmt.Errorf("failed creating import tune from already imported tune: %s", err.Error())
 		}
+
+		return impTune, nil
 	}
 
 	return nil, common.ErrNotFound
@@ -826,8 +868,8 @@ func isMsr(tunes []*apimodel.ImportTune) bool {
 	return false
 }
 
-// hasSingleFileData true if a tune with the same single tune file data exists in the database.
-func (d *dbService) hasSingleFileData(
+// hasSingleTuneFileData true if a tune with the same single tune file data exists in the database.
+func (d *dbService) hasSingleTuneFileData(
 	data []byte,
 ) (bool, error) {
 	if len(data) == 0 {

internal/database/db_data_service_crud_test.go

@@ -16,7 +16,7 @@ import (
 	"gorm.io/gorm"
 )
 
-var _ = Describe("DbDataService", func() {
+var _ = Describe("DbDataService CRUD", func() {
 	var err error
 	var cfg *config.Config
 	var service *dbService