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