add and use new util functions to simplify alias logic

internal/processing/account/alias.go

@@ -22,125 +22,128 @@ import (
 	"errors"
 	"fmt"
 	"net/url"
+	"slices"
 
 	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
+	"github.com/superseriousbusiness/gotosocial/internal/util"
 )
 
 func (p *Processor) Alias(
 	ctx context.Context,
 	account *gtsmodel.Account,
-	newAKAURIs []string,
+	newAKAURIStrs []string,
 ) (*apimodel.Account, gtserror.WithCode) {
-	var (
-		currentLen = len(account.AlsoKnownAsURIs)
-		newLen     = len(newAKAURIs)
-		// Update by making a
-		// call to the database.
-		update bool
-	)
-
-	switch {
-	case newLen == 0 && currentLen == 0:
-		// Empty also_known_as_uris,
-		// and no aliases set anyway,
-		// don't bother with a db call.
+	if slices.Equal(
+		newAKAURIStrs,
+		account.AlsoKnownAsURIs,
+	) {
+		// No changes to do
+		// here. Return early.
+		return p.c.GetAPIAccountSensitive(ctx, account)
+	}
 
-	case newLen == 0:
-		// Unset existing aliases.
+	newLen := len(newAKAURIStrs)
+	if newLen == 0 {
+		// Simply unset existing
+		// aliases and return early.
 		account.AlsoKnownAsURIs = nil
 		account.AlsoKnownAs = nil
-		update = true
-
-	default:
-		// Prepare to update AlsoKnownAs entries on the account.
-		account.AlsoKnownAs = make([]*gtsmodel.Account, 0, newLen)
-		account.AlsoKnownAsURIs = make([]string, 0, newLen)
-		update = true
-
-		// Use a map to deduplicate desired URIs.
-		newAKAsMap := make(map[string]struct{}, len(newAKAURIs))
-
-		// For each entry, ensure it's a valid URI,
-		// get and check the target account, and set.
-		for _, rawURI := range newAKAURIs {
-			alsoKnownAsURI, err := url.Parse(rawURI)
-			if err != nil {
-				err := fmt.Errorf(
-					"invalid also_known_as_uri (%s) provided in account alias request: %w",
-					rawURI, err,
-				)
-				return nil, gtserror.NewErrorBadRequest(err, err.Error())
-			}
-
-			// We only deref http or https, so check this.
-			if alsoKnownAsURI.Scheme != "https" && alsoKnownAsURI.Scheme != "http" {
-				err := fmt.Errorf(
-					"invalid also_known_as_uri (%s) provided in account alias request: %w",
-					rawURI, errors.New("uri must not be empty and scheme must be http or https"),
-				)
-				return nil, gtserror.NewErrorBadRequest(err, err.Error())
-			}
-
-			// Use parsed version of this URI from
-			// now on to normalize casing etc.
-			alsoKnownAsURIStr := alsoKnownAsURI.String()
-
-			// Avoid processing duplicate entries.
-			if _, ok := newAKAsMap[alsoKnownAsURIStr]; !ok {
-				newAKAsMap[alsoKnownAsURIStr] = struct{}{}
-			} else {
-				// Already done
-				// this one.
-				continue
-			}
-
-			// Don't let account do anything
-			// daft by aliasing to itself.
-			if alsoKnownAsURIStr == account.URI {
-				continue
-			}
-
-			// Ensure we have a valid, up-to-date
-			// representation of the target account.
-			targetAccount, _, err := p.federator.GetAccountByURI(ctx, account.Username, alsoKnownAsURI)
-			if err != nil {
-				err := fmt.Errorf(
-					"error dereferencing also_known_as_uri (%s) account: %w",
-					rawURI, err,
-				)
-				return nil, gtserror.NewErrorUnprocessableEntity(err, err.Error())
-			}
-
-			// Alias target must not be suspended.
-			if !targetAccount.SuspendedAt.IsZero() {
-				err := fmt.Errorf(
-					"target account %s is suspended from this instance; "+
-						"you will not be able to set alsoKnownAs to that account",
-					alsoKnownAsURIStr,
-				)
-				return nil, gtserror.NewErrorUnprocessableEntity(err, err.Error())
-			}
-
-			// Alrighty-roo, looks good, add this one.
-			account.AlsoKnownAsURIs = append(account.AlsoKnownAsURIs, alsoKnownAsURIStr)
-			account.AlsoKnownAs = append(account.AlsoKnownAs, targetAccount)
-		}
-	}
 
-	if update {
 		err := p.state.DB.UpdateAccount(ctx, account, "also_known_as_uris")
 		if err != nil {
 			err := gtserror.Newf("db error updating also_known_as_uri: %w", err)
 			return nil, gtserror.NewErrorInternalError(err)
 		}
+
+		return p.c.GetAPIAccountSensitive(ctx, account)
+	}
+
+	// We need to set new AKA URIs!
+	//
+	// First parse them to URI ptrs and
+	// normalized string representations.
+	//
+	// Use this cheeky type to avoid
+	// repeatedly calling uri.String().
+	type uri struct {
+		uri *url.URL // Parsed URI.
+		str string   // uri.String().
 	}
 
-	acctSensitive, err := p.converter.AccountToAPIAccountSensitive(ctx, account)
+	newAKAs := make([]uri, newLen)
+	for i, newAKAURIStr := range newAKAURIStrs {
+		newAKAURI, err := url.Parse(newAKAURIStr)
+		if err != nil {
+			err := fmt.Errorf(
+				"invalid also_known_as_uri (%s) provided in account alias request: %w",
+				newAKAURIStr, err,
+			)
+			return nil, gtserror.NewErrorBadRequest(err, err.Error())
+		}
+
+		// We only deref http or https, so check this.
+		if newAKAURI.Scheme != "https" && newAKAURI.Scheme != "http" {
+			err := fmt.Errorf(
+				"invalid also_known_as_uri (%s) provided in account alias request: %w",
+				newAKAURIStr, errors.New("uri must not be empty and scheme must be http or https"),
+			)
+			return nil, gtserror.NewErrorBadRequest(err, err.Error())
+		}
+
+		newAKAs[i].uri = newAKAURI
+		newAKAs[i].str = newAKAURI.String()
+	}
+
+	// Dedupe the URI/string pairs.
+	newAKAs = util.DeduplicateFunc(
+		newAKAs,
+		func(v uri) string {
+			return v.str
+		},
+	)
+
+	// For each deduped entry, get and
+	// check the target account, and set.
+	for _, newAKA := range newAKAs {
+		// Don't let account do anything
+		// daft by aliasing to itself.
+		if newAKA.str == account.URI {
+			continue
+		}
+
+		// Ensure we have a valid, up-to-date
+		// representation of the target account.
+		targetAccount, _, err := p.federator.GetAccountByURI(ctx, account.Username, newAKA.uri)
+		if err != nil {
+			err := fmt.Errorf(
+				"error dereferencing also_known_as_uri (%s) account: %w",
+				newAKA.str, err,
+			)
+			return nil, gtserror.NewErrorUnprocessableEntity(err, err.Error())
+		}
+
+		// Alias target must not be suspended.
+		if !targetAccount.SuspendedAt.IsZero() {
+			err := fmt.Errorf(
+				"target account %s is suspended from this instance; "+
+					"you will not be able to set alsoKnownAs to that account",
+				newAKA.str,
+			)
+			return nil, gtserror.NewErrorUnprocessableEntity(err, err.Error())
+		}
+
+		// Alrighty-roo, looks good, add this one.
+		account.AlsoKnownAsURIs = append(account.AlsoKnownAsURIs, newAKA.str)
+		account.AlsoKnownAs = append(account.AlsoKnownAs, targetAccount)
+	}
+
+	err := p.state.DB.UpdateAccount(ctx, account, "also_known_as_uris")
 	if err != nil {
-		err := gtserror.Newf("error converting account: %w", err)
+		err := gtserror.Newf("db error updating also_known_as_uri: %w", err)
 		return nil, gtserror.NewErrorInternalError(err)
 	}
-	return acctSensitive, nil
+
+	return p.c.GetAPIAccountSensitive(ctx, account)
 }

internal/processing/account/alias_test.go

@@ -113,6 +113,25 @@ func (suite *AliasTestSuite) TestAliasAccount() {
 				"http://localhost:8080/users/admin",
 			},
 		},
+		// Alias zork to turtle AND admin,
+		// duplicates should be removed.
+		{
+			newAliases: []string{
+				"http://localhost:8080/users/1happyturtle",
+				"http://localhost:8080/users/admin",
+				"http://localhost:8080/users/1happyturtle",
+				"http://localhost:8080/users/admin",
+				"http://localhost:8080/users/1happyturtle",
+				"http://localhost:8080/users/1happyturtle",
+				"http://localhost:8080/users/1happyturtle",
+				"http://localhost:8080/users/admin",
+				"http://localhost:8080/users/admin",
+			},
+			expectedAliases: []string{
+				"http://localhost:8080/users/1happyturtle",
+				"http://localhost:8080/users/admin",
+			},
+		},
 	} {
 		var (
 			ctx      = context.Background()

internal/processing/common/account.go.go

@@ -162,6 +162,23 @@ func (p *Processor) GetAPIAccountBlocked(
 	return apiAccount, nil
 }
 
+// GetAPIAccountSensitive fetches the "sensitive" account model for the given target.
+// *BE CAREFUL!* Only return a sensitive account if targetAcc == account making the request.
+func (p *Processor) GetAPIAccountSensitive(
+	ctx context.Context,
+	targetAcc *gtsmodel.Account,
+) (
+	apiAcc *apimodel.Account,
+	errWithCode gtserror.WithCode,
+) {
+	apiAccount, err := p.converter.AccountToAPIAccountSensitive(ctx, targetAcc)
+	if err != nil {
+		err = gtserror.Newf("error converting account: %w", err)
+		return nil, gtserror.NewErrorInternalError(err)
+	}
+	return apiAccount, nil
+}
+
 // GetVisibleAPIAccounts converts an array of gtsmodel.Accounts (inputted by next function) into
 // public API model accounts, checking first for visibility. Please note that all errors will be
 // logged at ERROR level, but will not be returned. Callers are likely to run into show-stopping

internal/util/deduplicate.go

@@ -0,0 +1,63 @@
+// GoToSocial
+// Copyright (C) GoToSocial Authors admin@gotosocial.org
+// SPDX-License-Identifier: AGPL-3.0-or-later
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package util
+
+// Deduplicate deduplicates entries in the given slice.
+func Deduplicate[T comparable](in []T) []T {
+	var (
+		inL     = len(in)
+		unique  = make(map[T]struct{}, inL)
+		deduped = make([]T, 0, inL)
+	)
+
+	for _, v := range in {
+		if _, ok := unique[v]; ok {
+			// Already have this.
+			continue
+		}
+
+		unique[v] = struct{}{}
+		deduped = append(deduped, v)
+	}
+
+	return deduped
+}
+
+// DeduplicateFunc deduplicates entries in the given
+// slice, using the result of key() to gauge uniqueness.
+func DeduplicateFunc[T any, C comparable](in []T, key func(v T) C) []T {
+	var (
+		inL     = len(in)
+		unique  = make(map[C]struct{}, inL)
+		deduped = make([]T, 0, inL)
+	)
+
+	for _, v := range in {
+		k := key(v)
+
+		if _, ok := unique[k]; ok {
+			// Already have this.
+			continue
+		}
+
+		unique[k] = struct{}{}
+		deduped = append(deduped, v)
+	}
+
+	return deduped
+}