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

@@ -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
 			}
 

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)

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)
 	}

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 {

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)

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)
 		})
 	}
 }

pkg/performer/update.go

@@ -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)
-}

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 deathDate == nil && performer.DeathDate != nil {
-			s := performer.DeathDate.String()
-			deathDate = &s
-		}
+	if err := ValidateAliases(performer.Name, performer.Aliases); err != nil {
+		return err
 	}
 
-	if birthdate == nil || deathDate == nil || *birthdate == "" || *deathDate == "" {
-		return nil
-	}
-
-	f, _ := utils.ParseDateStringAsTime(*birthdate)
-	t, _ := utils.ParseDateStringAsTime(*deathDate)
-
-	if f.After(t) {
-		return errors.New("the date of death should be higher than the date of birth")
+	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,
+			}
+		}
+
+		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
+	}
+
+	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
+	}
+
+	m := make(map[string]bool)
+	nameL := strings.ToLower(name)
+	m[nameL] = true
+
+	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)
+}

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 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,
+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) {
+	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)
+		})
 	}
-	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))
 }

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
 		}

ui/v2.5/src/components/Performers/PerformerDetails/PerformerEditPanel.tsx

@@ -106,16 +106,16 @@ export const PerformerEditPanel: React.FC<IPerformerDetails> = ({
       .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");
         },