diff --git a/internal/common/utils.go b/internal/common/utils.go index e8158e7..f3f24e1 100644 --- a/internal/common/utils.go +++ b/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 +} diff --git a/internal/database/db_data_service.go b/internal/database/db_data_service.go index 88e11d0..6a6d631 100644 --- a/internal/database/db_data_service.go +++ b/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 { diff --git a/internal/database/db_data_service_crud_test.go b/internal/database/db_data_service_crud_test.go index ea8a25b..8fa7ad3 100644 --- a/internal/database/db_data_service_crud_test.go +++ b/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