From 4715c5ebb2f2e97ad78900e7157b5a987b990b44 Mon Sep 17 00:00:00 2001 From: DingDongSoLong4 <99329275+DingDongSoLong4@users.noreply.github.com> Date: Thu, 2 Nov 2023 07:23:54 +0200 Subject: [PATCH] Fix performer validation (#4248) * Fix performer validation * Add tests * Rename QueryCount argument * Minor refactoring * Add duplicate alias validation * Make UI alias validation also case-insensitive --- internal/api/resolver_mutation_performer.go | 47 +-- pkg/models/date.go | 4 + pkg/models/mocks/PerformerReaderWriter.go | 10 +- pkg/models/repository_performer.go | 2 +- pkg/models/update.go | 20 +- pkg/models/update_test.go | 29 +- pkg/performer/update.go | 76 ---- pkg/performer/validate.go | 255 ++++++++++++- pkg/performer/validate_test.go | 352 +++++++++++++++--- pkg/studio/update.go | 2 +- .../PerformerDetails/PerformerEditPanel.tsx | 20 +- 11 files changed, 588 insertions(+), 229 deletions(-) delete mode 100644 pkg/performer/update.go diff --git a/internal/api/resolver_mutation_performer.go b/internal/api/resolver_mutation_performer.go index 007a3e56eb1..17c87dd154b 100644 --- a/internal/api/resolver_mutation_performer.go +++ b/internal/api/resolver_mutation_performer.go @@ -34,6 +34,7 @@ func (r *mutationResolver) PerformerCreate(ctx context.Context, input models.Per newPerformer.Name = input.Name newPerformer.Disambiguation = translator.string(input.Disambiguation) + newPerformer.Aliases = models.NewRelatedStrings(input.AliasList) newPerformer.URL = translator.string(input.URL) newPerformer.Gender = input.Gender newPerformer.Ethnicity = translator.string(input.Ethnicity) @@ -68,22 +69,11 @@ func (r *mutationResolver) PerformerCreate(ctx context.Context, input models.Per return nil, fmt.Errorf("converting death date: %w", err) } - // prefer alias_list over aliases - if input.AliasList != nil { - newPerformer.Aliases = models.NewRelatedStrings(input.AliasList) - } - newPerformer.TagIDs, err = translator.relatedIds(input.TagIds) if err != nil { return nil, fmt.Errorf("converting tag ids: %w", err) } - if err := performer.ValidateDeathDate(nil, input.Birthdate, input.DeathDate); err != nil { - if err != nil { - return nil, err - } - } - // Process the base 64 encoded image string var imageData []byte if input.Image != nil { @@ -97,7 +87,7 @@ func (r *mutationResolver) PerformerCreate(ctx context.Context, input models.Per if err := r.withTxn(ctx, func(ctx context.Context) error { qb := r.repository.Performer - if err := performer.EnsureNameUnique(ctx, newPerformer.Name, newPerformer.Disambiguation, qb); err != nil { + if err := performer.ValidateCreate(ctx, newPerformer, qb); err != nil { return err } @@ -196,21 +186,7 @@ func (r *mutationResolver) PerformerUpdate(ctx context.Context, input models.Per if err := r.withTxn(ctx, func(ctx context.Context) error { qb := r.repository.Performer - // need to get existing performer - existing, err := qb.Find(ctx, performerID) - if err != nil { - return err - } - - if existing == nil { - return fmt.Errorf("performer with id %d not found", performerID) - } - - if err := performer.EnsureUpdateNameUnique(ctx, existing, updatedPerformer.Name, updatedPerformer.Disambiguation, qb); err != nil { - return err - } - - if err := performer.ValidateDeathDate(existing, input.Birthdate, input.DeathDate); err != nil { + if err := performer.ValidateUpdate(ctx, performerID, updatedPerformer, qb); err != nil { return err } @@ -301,22 +277,7 @@ func (r *mutationResolver) BulkPerformerUpdate(ctx context.Context, input BulkPe qb := r.repository.Performer for _, performerID := range performerIDs { - // need to get existing performer - existing, err := qb.Find(ctx, performerID) - if err != nil { - return err - } - - if existing == nil { - return fmt.Errorf("performer with id %d not found", performerID) - } - - if err := performer.EnsureUpdateNameUnique(ctx, existing, updatedPerformer.Name, updatedPerformer.Disambiguation, qb); err != nil { - return err - } - - err = performer.ValidateDeathDate(existing, input.Birthdate, input.DeathDate) - if err != nil { + if err := performer.ValidateUpdate(ctx, performerID, updatedPerformer, qb); err != nil { return err } diff --git a/pkg/models/date.go b/pkg/models/date.go index c88aeee359a..151e32c1db2 100644 --- a/pkg/models/date.go +++ b/pkg/models/date.go @@ -17,6 +17,10 @@ func (d Date) String() string { return d.Format(dateFormat) } +func (d Date) After(o Date) bool { + return d.Time.After(o.Time) +} + // ParseDate uses utils.ParseDateStringAsTime to parse a string into a date. func ParseDate(s string) (Date, error) { ret, err := utils.ParseDateStringAsTime(s) diff --git a/pkg/models/mocks/PerformerReaderWriter.go b/pkg/models/mocks/PerformerReaderWriter.go index 265a467590f..7bbc6ef794e 100644 --- a/pkg/models/mocks/PerformerReaderWriter.go +++ b/pkg/models/mocks/PerformerReaderWriter.go @@ -434,20 +434,20 @@ func (_m *PerformerReaderWriter) Query(ctx context.Context, performerFilter *mod return r0, r1, r2 } -// QueryCount provides a mock function with given fields: ctx, galleryFilter, findFilter -func (_m *PerformerReaderWriter) QueryCount(ctx context.Context, galleryFilter *models.PerformerFilterType, findFilter *models.FindFilterType) (int, error) { - ret := _m.Called(ctx, galleryFilter, findFilter) +// QueryCount provides a mock function with given fields: ctx, performerFilter, findFilter +func (_m *PerformerReaderWriter) QueryCount(ctx context.Context, performerFilter *models.PerformerFilterType, findFilter *models.FindFilterType) (int, error) { + ret := _m.Called(ctx, performerFilter, findFilter) var r0 int if rf, ok := ret.Get(0).(func(context.Context, *models.PerformerFilterType, *models.FindFilterType) int); ok { - r0 = rf(ctx, galleryFilter, findFilter) + r0 = rf(ctx, performerFilter, findFilter) } else { r0 = ret.Get(0).(int) } var r1 error if rf, ok := ret.Get(1).(func(context.Context, *models.PerformerFilterType, *models.FindFilterType) error); ok { - r1 = rf(ctx, galleryFilter, findFilter) + r1 = rf(ctx, performerFilter, findFilter) } else { r1 = ret.Error(1) } diff --git a/pkg/models/repository_performer.go b/pkg/models/repository_performer.go index aac7e0488e4..22ade1d1d7d 100644 --- a/pkg/models/repository_performer.go +++ b/pkg/models/repository_performer.go @@ -23,7 +23,7 @@ type PerformerFinder interface { // PerformerQueryer provides methods to query performers. type PerformerQueryer interface { Query(ctx context.Context, performerFilter *PerformerFilterType, findFilter *FindFilterType) ([]*Performer, int, error) - QueryCount(ctx context.Context, galleryFilter *PerformerFilterType, findFilter *FindFilterType) (int, error) + QueryCount(ctx context.Context, performerFilter *PerformerFilterType, findFilter *FindFilterType) (int, error) } type PerformerAutoTagQueryer interface { diff --git a/pkg/models/update.go b/pkg/models/update.go index c841cbc8aeb..2302a2e699a 100644 --- a/pkg/models/update.go +++ b/pkg/models/update.go @@ -89,13 +89,13 @@ func (u *UpdateIDs) ImpactedIDs(existing []int) []int { return nil } -// GetEffectiveIDs returns the new IDs that will be effective after the update. -func (u *UpdateIDs) EffectiveIDs(existing []int) []int { +// Apply applies the update to a list of existing ids, returning the result. +func (u *UpdateIDs) Apply(existing []int) []int { if u == nil { - return nil + return existing } - return effectiveValues(u.IDs, u.Mode, existing) + return applyUpdate(u.IDs, u.Mode, existing) } type UpdateStrings struct { @@ -111,17 +111,17 @@ func (u *UpdateStrings) Strings() []string { return u.Values } -// GetEffectiveIDs returns the new IDs that will be effective after the update. -func (u *UpdateStrings) EffectiveValues(existing []string) []string { +// Apply applies the update to a list of existing strings, returning the result. +func (u *UpdateStrings) Apply(existing []string) []string { if u == nil { - return nil + return existing } - return effectiveValues(u.Values, u.Mode, existing) + return applyUpdate(u.Values, u.Mode, existing) } -// effectiveValues returns the new values that will be effective after the update. -func effectiveValues[T comparable](values []T, mode RelationshipUpdateMode, existing []T) []T { +// applyUpdate applies values to existing, using the update mode specified. +func applyUpdate[T comparable](values []T, mode RelationshipUpdateMode, existing []T) []T { switch mode { case RelationshipUpdateModeAdd: return sliceutil.AppendUniques(existing, values) diff --git a/pkg/models/update_test.go b/pkg/models/update_test.go index 0baf7926f7a..5411d184a30 100644 --- a/pkg/models/update_test.go +++ b/pkg/models/update_test.go @@ -3,6 +3,8 @@ package models import ( "reflect" "testing" + + "github.com/stretchr/testify/assert" ) func TestUpdateIDs_ImpactedIDs(t *testing.T) { @@ -48,45 +50,40 @@ func TestUpdateIDs_ImpactedIDs(t *testing.T) { } } -func TestUpdateIDs_EffectiveIDs(t *testing.T) { +func TestApplyUpdate(t *testing.T) { tests := []struct { name string - IDs []int - Mode RelationshipUpdateMode + values []int + mode RelationshipUpdateMode existing []int want []int }{ { name: "add", - IDs: []int{2, 3}, - Mode: RelationshipUpdateModeAdd, + values: []int{2, 3}, + mode: RelationshipUpdateModeAdd, existing: []int{1, 2}, want: []int{1, 2, 3}, }, { name: "remove", - IDs: []int{2, 3}, - Mode: RelationshipUpdateModeRemove, + values: []int{2, 3}, + mode: RelationshipUpdateModeRemove, existing: []int{1, 2}, want: []int{1}, }, { name: "set", - IDs: []int{1, 2, 3}, - Mode: RelationshipUpdateModeSet, + values: []int{1, 2, 3}, + mode: RelationshipUpdateModeSet, existing: []int{1, 2}, want: []int{1, 2, 3}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - u := &UpdateIDs{ - IDs: tt.IDs, - Mode: tt.Mode, - } - if got := u.EffectiveIDs(tt.existing); !reflect.DeepEqual(got, tt.want) { - t.Errorf("UpdateIDs.EffectiveIDs() = %v, want %v", got, tt.want) - } + got := applyUpdate(tt.values, tt.mode, tt.existing) + assert.Equal(t, tt.want, got) }) } } diff --git a/pkg/performer/update.go b/pkg/performer/update.go deleted file mode 100644 index 9fb5e83c859..00000000000 --- a/pkg/performer/update.go +++ /dev/null @@ -1,76 +0,0 @@ -package performer - -import ( - "context" - "fmt" - - "github.com/stashapp/stash/pkg/models" -) - -type NameExistsError struct { - Name string - Disambiguation string -} - -func (e *NameExistsError) Error() string { - if e.Disambiguation != "" { - return fmt.Sprintf("performer with name '%s' and disambiguation '%s' already exists", e.Name, e.Disambiguation) - } - return fmt.Sprintf("performer with name '%s' already exists", e.Name) -} - -// EnsureNameUnique returns an error if the performer name and disambiguation provided -// is used by another performer -func EnsureNameUnique(ctx context.Context, name string, disambig string, qb models.PerformerReaderWriter) error { - performerFilter := models.PerformerFilterType{ - Name: &models.StringCriterionInput{ - Value: name, - Modifier: models.CriterionModifierEquals, - }, - } - - if disambig != "" { - performerFilter.Disambiguation = &models.StringCriterionInput{ - Value: disambig, - Modifier: models.CriterionModifierEquals, - } - } - - pp := 1 - findFilter := models.FindFilterType{ - PerPage: &pp, - } - - existing, _, err := qb.Query(ctx, &performerFilter, &findFilter) - if err != nil { - return err - } - - if len(existing) > 0 { - return &NameExistsError{ - Name: name, - Disambiguation: disambig, - } - } - - return nil -} - -// EnsureUpdateNameUnique performs the same check as EnsureNameUnique, but is used when modifying an existing performer. -func EnsureUpdateNameUnique(ctx context.Context, existing *models.Performer, name models.OptionalString, disambig models.OptionalString, qb models.PerformerReaderWriter) error { - newName := existing.Name - newDisambig := existing.Disambiguation - - if name.Set { - newName = name.Value - } - if disambig.Set { - newDisambig = disambig.Value - } - - if newName == existing.Name && newDisambig == existing.Disambiguation { - return nil - } - - return EnsureNameUnique(ctx, newName, newDisambig, qb) -} diff --git a/pkg/performer/validate.go b/pkg/performer/validate.go index 4c0b5a91979..0106490cf62 100644 --- a/pkg/performer/validate.go +++ b/pkg/performer/validate.go @@ -1,39 +1,260 @@ package performer import ( + "context" "errors" + "fmt" + "strings" "github.com/stashapp/stash/pkg/models" - "github.com/stashapp/stash/pkg/utils" ) -func ValidateDeathDate(performer *models.Performer, birthdate *string, deathDate *string) error { - // don't validate existing values - if birthdate == nil && deathDate == nil { - return nil +var ( + ErrNameMissing = errors.New("performer name must not be blank") +) + +type NotFoundError struct { + id int +} + +func (e *NotFoundError) Error() string { + return fmt.Sprintf("performer with id %d not found", e.id) +} + +type NameExistsError struct { + Name string + Disambiguation string +} + +func (e *NameExistsError) Error() string { + if e.Disambiguation != "" { + return fmt.Sprintf("performer with name '%s' and disambiguation '%s' already exists", e.Name, e.Disambiguation) + } + return fmt.Sprintf("performer with name '%s' already exists", e.Name) +} + +type DuplicateAliasError struct { + Alias string +} + +func (e *DuplicateAliasError) Error() string { + return fmt.Sprintf("performer contains duplicate alias '%s'", e.Alias) +} + +type DeathDateError struct { + Birthdate models.Date + DeathDate models.Date +} + +func (e *DeathDateError) Error() string { + return fmt.Sprintf("death date %s should be after birthdate %s", e.DeathDate, e.Birthdate) +} + +func ValidateCreate(ctx context.Context, performer models.Performer, qb models.PerformerReader) error { + if err := ValidateName(ctx, performer.Name, performer.Disambiguation, qb); err != nil { + return err } - if performer != nil { - if birthdate == nil && performer.Birthdate != nil { - s := performer.Birthdate.String() - birthdate = &s + if err := ValidateAliases(performer.Name, performer.Aliases); err != nil { + return err + } + + if err := ValidateDeathDate(performer.Birthdate, performer.DeathDate); err != nil { + return err + } + + return nil +} + +func ValidateUpdate(ctx context.Context, id int, partial models.PerformerPartial, qb models.PerformerReader) error { + existing, err := qb.Find(ctx, id) + if err != nil { + return err + } + + if existing == nil { + return &NotFoundError{id} + } + + if err := ValidateUpdateName(ctx, *existing, partial.Name, partial.Disambiguation, qb); err != nil { + return err + } + + if err := existing.LoadAliases(ctx, qb); err != nil { + return err + } + if err := ValidateUpdateAliases(*existing, partial.Name, partial.Aliases); err != nil { + return err + } + + if err := ValidateUpdateDeathDate(*existing, partial.Birthdate, partial.DeathDate); err != nil { + return err + } + + return nil +} + +func validateName(ctx context.Context, name string, disambig string, existingID *int, qb models.PerformerQueryer) error { + performerFilter := models.PerformerFilterType{ + Name: &models.StringCriterionInput{ + Value: name, + Modifier: models.CriterionModifierEquals, + }, + } + + if disambig != "" { + performerFilter.Disambiguation = &models.StringCriterionInput{ + Value: disambig, + Modifier: models.CriterionModifierEquals, + } + } + + if existingID == nil { + // creating: error if any existing performer matches + + pp := 1 + findFilter := models.FindFilterType{ + PerPage: &pp, + } + + count, err := qb.QueryCount(ctx, &performerFilter, &findFilter) + if err != nil { + return err + } + + if count > 0 { + return &NameExistsError{ + Name: name, + Disambiguation: disambig, + } } - if deathDate == nil && performer.DeathDate != nil { - s := performer.DeathDate.String() - deathDate = &s + + return nil + } else { + // updating: check for matches, but ignore self + + pp := 2 + findFilter := models.FindFilterType{ + PerPage: &pp, + } + + conflicts, _, err := qb.Query(ctx, &performerFilter, &findFilter) + if err != nil { + return err + } + + if len(conflicts) > 0 { + // valid if the only conflict is the existing performer + if len(conflicts) > 1 || conflicts[0].ID != *existingID { + return &NameExistsError{ + Name: name, + Disambiguation: disambig, + } + } } + + return nil + } +} + +// ValidateName returns an error if the performer name and disambiguation provided is used by another performer. +func ValidateName(ctx context.Context, name string, disambig string, qb models.PerformerQueryer) error { + if name == "" { + return ErrNameMissing + } + + return validateName(ctx, name, disambig, nil, qb) +} + +// ValidateUpdateName performs the same check as ValidateName, but is used when modifying an existing performer. +func ValidateUpdateName(ctx context.Context, existing models.Performer, name models.OptionalString, disambig models.OptionalString, qb models.PerformerQueryer) error { + // if neither name nor disambig is set, don't check anything + if !name.Set && !disambig.Set { + return nil } - if birthdate == nil || deathDate == nil || *birthdate == "" || *deathDate == "" { + newName := existing.Name + if name.Set { + newName = name.Value + } + + if newName == "" { + return ErrNameMissing + } + + newDisambig := existing.Disambiguation + if disambig.Set { + newDisambig = disambig.Value + } + + return validateName(ctx, newName, newDisambig, &existing.ID, qb) +} + +func ValidateAliases(name string, aliases models.RelatedStrings) error { + if !aliases.Loaded() { return nil } - f, _ := utils.ParseDateStringAsTime(*birthdate) - t, _ := utils.ParseDateStringAsTime(*deathDate) + m := make(map[string]bool) + nameL := strings.ToLower(name) + m[nameL] = true - if f.After(t) { - return errors.New("the date of death should be higher than the date of birth") + for _, alias := range aliases.List() { + aliasL := strings.ToLower(alias) + if m[aliasL] { + return &DuplicateAliasError{alias} + } + m[aliasL] = true } return nil } + +func ValidateUpdateAliases(existing models.Performer, name models.OptionalString, aliases *models.UpdateStrings) error { + // if neither name nor aliases is set, don't check anything + if !name.Set && aliases == nil { + return nil + } + + newName := existing.Name + if name.Set { + newName = name.Value + } + + newAliases := aliases.Apply(existing.Aliases.List()) + + return ValidateAliases(newName, models.NewRelatedStrings(newAliases)) +} + +// ValidateDeathDate returns an error if the birthdate is after the death date. +func ValidateDeathDate(birthdate *models.Date, deathDate *models.Date) error { + if birthdate == nil || deathDate == nil { + return nil + } + + if birthdate.After(*deathDate) { + return &DeathDateError{Birthdate: *birthdate, DeathDate: *deathDate} + } + + return nil +} + +// ValidateUpdateDeathDate performs the same check as ValidateDeathDate, but is used when modifying an existing performer. +func ValidateUpdateDeathDate(existing models.Performer, birthdate models.OptionalDate, deathDate models.OptionalDate) error { + // if neither birthdate nor deathDate is set, don't check anything + if !birthdate.Set && !deathDate.Set { + return nil + } + + newBirthdate := existing.Birthdate + if birthdate.Set { + newBirthdate = birthdate.Ptr() + } + + newDeathDate := existing.DeathDate + if deathDate.Set { + newDeathDate = deathDate.Ptr() + } + + return ValidateDeathDate(newBirthdate, newDeathDate) +} diff --git a/pkg/performer/validate_test.go b/pkg/performer/validate_test.go index 86e69bc1d5d..778459f1751 100644 --- a/pkg/performer/validate_test.go +++ b/pkg/performer/validate_test.go @@ -4,58 +4,310 @@ import ( "testing" "github.com/stashapp/stash/pkg/models" + "github.com/stashapp/stash/pkg/models/mocks" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) +func nameFilter(n string) *models.PerformerFilterType { + return &models.PerformerFilterType{ + Name: &models.StringCriterionInput{ + Value: n, + Modifier: models.CriterionModifierEquals, + }, + } +} + +func disambigFilter(n string, d string) *models.PerformerFilterType { + return &models.PerformerFilterType{ + Name: &models.StringCriterionInput{ + Value: n, + Modifier: models.CriterionModifierEquals, + }, + Disambiguation: &models.StringCriterionInput{ + Value: d, + Modifier: models.CriterionModifierEquals, + }, + } +} + +func TestValidateName(t *testing.T) { + db := mocks.NewDatabase() + + const ( + name1 = "name 1" + name2 = "name 2" + disambig = "disambiguation" + newName = "new name" + newDisambig = "new disambiguation" + ) + // existing1 := models.Performer{ + // Name: name1, + // } + // existing2 := models.Performer{ + // Name: name2, + // Disambiguation: disambig, + // } + + pp := 1 + findFilter := &models.FindFilterType{ + PerPage: &pp, + } + + db.Performer.On("QueryCount", testCtx, nameFilter(name1), findFilter).Return(1, nil) + db.Performer.On("QueryCount", testCtx, nameFilter(name2), findFilter).Return(1, nil) + db.Performer.On("QueryCount", testCtx, disambigFilter(name2, disambig), findFilter).Return(1, nil) + db.Performer.On("QueryCount", testCtx, mock.Anything, findFilter).Return(0, nil) + + tests := []struct { + tName string + name string + disambig string + want error + }{ + {"missing name", "", newDisambig, ErrNameMissing}, + {"new name", newName, "", nil}, + {"new name new disambig", newName, newDisambig, nil}, + {"new name existing disambig", newName, disambig, nil}, + {"existing name", name1, "", &NameExistsError{name1, ""}}, + {"existing name new disambig", name1, newDisambig, nil}, + {"existing name existing disambig", name1, disambig, nil}, + {"existing name and disambig", name2, disambig, &NameExistsError{name2, disambig}}, + } + + for _, tt := range tests { + t.Run(tt.tName, func(t *testing.T) { + got := ValidateName(testCtx, tt.name, tt.disambig, db.Performer) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestValidateUpdateName(t *testing.T) { + db := mocks.NewDatabase() + + const ( + name1 = "name 1" + name2 = "name 2" + disambig1 = "disambiguation 1" + disambig2 = "disambiguation 2" + newName = "new name" + newDisambig = "new disambiguation" + ) + + osUnset := models.OptionalString{} + osNull := models.OptionalString{Set: true, Null: true} + osName1 := models.NewOptionalString(name1) + osName2 := models.NewOptionalString(name2) + osDisambig1 := models.NewOptionalString(disambig1) + osDisambig2 := models.NewOptionalString(disambig2) + osNewName := models.NewOptionalString(newName) + osNewDisambig := models.NewOptionalString(newDisambig) + + existing1 := models.Performer{ + ID: 1, + Name: name1, + } + existing2 := models.Performer{ + ID: 2, + Name: name2, + Disambiguation: disambig1, + } + existing3 := models.Performer{ + ID: 3, + Name: name2, + Disambiguation: disambig2, + } + + pp := 2 + findFilter := &models.FindFilterType{ + PerPage: &pp, + } + + db.Performer.On("Query", testCtx, nameFilter(name1), findFilter).Return([]*models.Performer{&existing1}, 1, nil) + db.Performer.On("Query", testCtx, nameFilter(name2), findFilter).Return([]*models.Performer{&existing2, &existing3}, 2, nil) + db.Performer.On("Query", testCtx, disambigFilter(name2, disambig1), findFilter).Return([]*models.Performer{&existing2}, 1, nil) + db.Performer.On("Query", testCtx, disambigFilter(name2, disambig2), findFilter).Return([]*models.Performer{&existing3}, 1, nil) + db.Performer.On("Query", testCtx, mock.Anything, findFilter).Return(nil, 0, nil) + + tests := []struct { + tName string + performer models.Performer + name models.OptionalString + disambig models.OptionalString + want error + }{ + {"missing name", existing1, osNull, osUnset, ErrNameMissing}, + {"same name", existing3, osName2, osUnset, nil}, + {"same disambig", existing2, osUnset, osDisambig1, nil}, + {"same name same disambig", existing2, osName2, osDisambig1, nil}, + {"new name", existing1, osNewName, osUnset, nil}, + {"new disambig", existing1, osUnset, osNewDisambig, nil}, + {"new name new disambig", existing1, osNewName, osNewDisambig, nil}, + {"remove disambig", existing3, osUnset, osNull, &NameExistsError{name2, ""}}, + {"existing name keep disambig", existing3, osName1, osUnset, nil}, + {"existing name remove disambig", existing3, osName1, osNull, &NameExistsError{name1, ""}}, + {"existing disambig", existing2, osUnset, osDisambig2, &NameExistsError{name2, disambig2}}, + {"existing name and disambig", existing1, osName2, osDisambig1, &NameExistsError{name2, disambig1}}, + } + + for _, tt := range tests { + t.Run(tt.tName, func(t *testing.T) { + got := ValidateUpdateName(testCtx, tt.performer, tt.name, tt.disambig, db.Performer) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestValidateAliases(t *testing.T) { + const ( + name1 = "name 1" + name1U = "NAME 1" + name2 = "name 2" + name3 = "name 3" + name4 = "name 4" + ) + + tests := []struct { + tName string + name string + aliases []string + want error + }{ + {"no aliases", name1, nil, nil}, + {"valid aliases", name2, []string{name3, name4}, nil}, + {"duplicate alias", name1, []string{name2, name3, name2}, &DuplicateAliasError{name2}}, + {"duplicate name", name4, []string{name4, name3}, &DuplicateAliasError{name4}}, + {"duplicate alias caps", name2, []string{name1, name1U}, &DuplicateAliasError{name1U}}, + {"duplicate name caps", name1U, []string{name1}, &DuplicateAliasError{name1}}, + } + + for _, tt := range tests { + t.Run(tt.tName, func(t *testing.T) { + got := ValidateAliases(tt.name, models.NewRelatedStrings(tt.aliases)) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestValidateUpdateAliases(t *testing.T) { + const ( + name1 = "name 1" + name1U = "NAME 1" + name2 = "name 2" + name3 = "name 3" + name4 = "name 4" + ) + + existing := models.Performer{ + Name: name1, + Aliases: models.NewRelatedStrings([]string{name2}), + } + + osUnset := models.OptionalString{} + os1 := models.NewOptionalString(name1) + os2 := models.NewOptionalString(name2) + os3 := models.NewOptionalString(name3) + os4 := models.NewOptionalString(name4) + + tests := []struct { + tName string + name models.OptionalString + aliases []string + want error + }{ + {"both unset", osUnset, nil, nil}, + {"invalid name set", os2, nil, &DuplicateAliasError{name2}}, + {"valid name set", os3, nil, nil}, + {"valid aliases empty", os1, []string{}, nil}, + {"invalid aliases set", osUnset, []string{name1U}, &DuplicateAliasError{name1U}}, + {"valid aliases set", osUnset, []string{name3, name2}, nil}, + {"invalid both set", os4, []string{name4}, &DuplicateAliasError{name4}}, + {"valid both set", os2, []string{name1}, nil}, + } + + for _, tt := range tests { + t.Run(tt.tName, func(t *testing.T) { + var aliases *models.UpdateStrings + if tt.aliases != nil { + aliases = &models.UpdateStrings{ + Values: tt.aliases, + Mode: models.RelationshipUpdateModeSet, + } + } + got := ValidateUpdateAliases(existing, tt.name, aliases) + assert.Equal(t, tt.want, got) + }) + } +} + func TestValidateDeathDate(t *testing.T) { - assert := assert.New(t) - - date1 := "2001-01-01" - date2 := "2002-01-01" - date3 := "2003-01-01" - date4 := "2004-01-01" - empty := "" - - md2, _ := models.ParseDate(date2) - md3, _ := models.ParseDate(date3) - - emptyPerformer := models.Performer{} - invalidPerformer := models.Performer{ - Birthdate: &md3, - DeathDate: &md2, - } - validPerformer := models.Performer{ - Birthdate: &md2, - DeathDate: &md3, - } - - // nil values should always return nil - assert.Nil(ValidateDeathDate(nil, nil, &date1)) - assert.Nil(ValidateDeathDate(nil, &date2, nil)) - assert.Nil(ValidateDeathDate(&emptyPerformer, nil, &date1)) - assert.Nil(ValidateDeathDate(&emptyPerformer, &date2, nil)) - - // empty strings should always return nil - assert.Nil(ValidateDeathDate(nil, &empty, &date1)) - assert.Nil(ValidateDeathDate(nil, &date2, &empty)) - assert.Nil(ValidateDeathDate(&emptyPerformer, &empty, &date1)) - assert.Nil(ValidateDeathDate(&emptyPerformer, &date2, &empty)) - assert.Nil(ValidateDeathDate(&validPerformer, &empty, &date1)) - assert.Nil(ValidateDeathDate(&validPerformer, &date2, &empty)) - - // nil inputs should return nil even if performer is invalid - assert.Nil(ValidateDeathDate(&invalidPerformer, nil, nil)) - - // invalid input values should return error - assert.NotNil(ValidateDeathDate(nil, &date2, &date1)) - assert.NotNil(ValidateDeathDate(&validPerformer, &date2, &date1)) - - // valid input values should return nil - assert.Nil(ValidateDeathDate(nil, &date1, &date2)) - - // use performer values if performer set and values available - assert.NotNil(ValidateDeathDate(&validPerformer, nil, &date1)) - assert.NotNil(ValidateDeathDate(&validPerformer, &date4, nil)) - assert.Nil(ValidateDeathDate(&validPerformer, nil, &date4)) - assert.Nil(ValidateDeathDate(&validPerformer, &date1, nil)) + date1, _ := models.ParseDate("2001-01-01") + date2, _ := models.ParseDate("2002-01-01") + date3, _ := models.ParseDate("2003-01-01") + date4, _ := models.ParseDate("2004-01-01") + + tests := []struct { + name string + birthdate *models.Date + deathdate *models.Date + want error + }{ + {"both nil", nil, nil, nil}, + {"birthdate nil", nil, &date1, nil}, + {"deathdate nil", nil, &date2, nil}, + {"valid", &date3, &date4, nil}, + {"invalid", &date3, &date2, &DeathDateError{date3, date2}}, + {"same date", &date1, &date1, nil}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ValidateDeathDate(tt.birthdate, tt.deathdate) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestValidateUpdateDeathDate(t *testing.T) { + date1, _ := models.ParseDate("2001-01-01") + date2, _ := models.ParseDate("2002-01-01") + date3, _ := models.ParseDate("2003-01-01") + date4, _ := models.ParseDate("2004-01-01") + + existing := models.Performer{ + Birthdate: &date2, + DeathDate: &date3, + } + + odUnset := models.OptionalDate{} + odNull := models.OptionalDate{Set: true, Null: true} + od1 := models.NewOptionalDate(date1) + od2 := models.NewOptionalDate(date2) + od3 := models.NewOptionalDate(date3) + od4 := models.NewOptionalDate(date4) + + tests := []struct { + name string + birthdate models.OptionalDate + deathdate models.OptionalDate + want error + }{ + {"both unset", odUnset, odUnset, nil}, + {"invalid birthdate set", od4, odUnset, &DeathDateError{date4, date3}}, + {"valid birthdate set", od1, odUnset, nil}, + {"valid birthdate set null", odNull, odUnset, nil}, + {"invalid deathdate set", odUnset, od1, &DeathDateError{date2, date1}}, + {"valid deathdate set", odUnset, od4, nil}, + {"valid deathdate set null", odUnset, odNull, nil}, + {"invalid both set", od3, od2, &DeathDateError{date3, date2}}, + {"valid both set", od2, od3, nil}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ValidateUpdateDeathDate(existing, tt.birthdate, tt.deathdate) + assert.Equal(t, tt.want, got) + }) + } } diff --git a/pkg/studio/update.go b/pkg/studio/update.go index 6f81b4464b8..3125e674ea9 100644 --- a/pkg/studio/update.go +++ b/pkg/studio/update.go @@ -103,7 +103,7 @@ func ValidateModify(ctx context.Context, s models.StudioPartial, qb ValidateModi return err } - effectiveAliases := s.Aliases.EffectiveValues(existing.Aliases.List()) + effectiveAliases := s.Aliases.Apply(existing.Aliases.List()) if err := EnsureAliasesUnique(ctx, s.ID, effectiveAliases, qb); err != nil { return err } diff --git a/ui/v2.5/src/components/Performers/PerformerDetails/PerformerEditPanel.tsx b/ui/v2.5/src/components/Performers/PerformerDetails/PerformerEditPanel.tsx index 77c57e20b95..13e7d819794 100644 --- a/ui/v2.5/src/components/Performers/PerformerDetails/PerformerEditPanel.tsx +++ b/ui/v2.5/src/components/Performers/PerformerDetails/PerformerEditPanel.tsx @@ -106,16 +106,16 @@ export const PerformerEditPanel: React.FC = ({ .test({ name: "unique", test: (value, context) => { - const aliases = [context.parent.name, ...value]; - const dupes = aliases - .map((e, i, a) => { - if (a.indexOf(e) !== i) { - return String(i - 1); - } else { - return null; - } - }) - .filter((e) => e !== null) as string[]; + const aliases = [context.parent.name.toLowerCase()]; + const dupes: number[] = []; + for (let i = 0; i < value.length; i++) { + const a = value[i].toLowerCase(); + if (aliases.includes(a)) { + dupes.push(i); + } else { + aliases.push(a); + } + } if (dupes.length === 0) return true; return new yup.ValidationError(dupes.join(" "), value, "alias_list"); },