Currencies: Move ContainsAll duplicate check out

Not the responsibility of ContainsAll to check for duplicates.
Can't actually work out why we'd need to check for duplicates in the
incoming list anyway.
If we're meant to be protecting against storage of duplicates, that
should be in StorePairs.

Adds DuplicatePairs

currency/manager.go

@@ -210,6 +210,10 @@ func (p *PairsManager) StorePairs(a asset.Item, pairs Pairs, enabled bool) error
 		p.Pairs[a] = pairStore
 	}
 
+	if d := pairs.DuplicatePairs(); len(d) != 0 {
+		return fmt.Errorf("%w: %s", ErrDuplicatePairs, d.Join())
+	}
+
 	if enabled {
 		// All new enabled pairs must be in Available already
 		if err := pairStore.Available.ContainsAll(pairs, true); err != nil {

currency/manager_test.go

@@ -289,86 +289,31 @@ func TestStorePairs(t *testing.T) {
 	p := initTest(t)
 
 	err := p.StorePairs(0, nil, false)
-	if !errors.Is(err, asset.ErrNotSupported) {
-		t.Fatalf("received: %v but expected: %v", err, asset.ErrNotSupported)
-	}
+	assert.ErrorIs(t, err, asset.ErrNotSupported)
 
 	p.Pairs = nil
 
-	ethusdPairs, err := NewPairsFromStrings([]string{"ETH-USD"})
-	if err != nil {
-		t.Fatal(err)
-	}
+	ethusd := NewPairWithDelimiter("ETH", "USD", "-")
+	ethkrw := NewPairWithDelimiter("ETH", "KRW", "-")
 
-	err = p.StorePairs(asset.Spot, ethusdPairs, false)
-	if !errors.Is(err, nil) {
-		t.Fatalf("received: %v but expected: %v", err, nil)
-	}
+	err = p.StorePairs(asset.Spot, Pairs{ethusd}, false)
+	require.NoError(t, err)
 
 	pairs, err := p.GetPairs(asset.Spot, false)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	ethusd, err := NewPairFromString("ETH-USD")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if !pairs.Contains(ethusd, true) {
-		t.Errorf("TestStorePairs failed, unexpected result")
-	}
-
-	p = initTest(t)
-	err = p.StorePairs(asset.Spot, ethusdPairs, false)
-	if !errors.Is(err, nil) {
-		t.Fatalf("received: %v but expected: %v", err, nil)
-	}
-	pairs, err = p.GetPairs(asset.Spot, false)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if pairs == nil {
-		t.Errorf("pairs should be populated")
-	}
-
-	if !pairs.Contains(ethusd, true) {
-		t.Errorf("TestStorePairs failed, unexpected result")
-	}
-
-	ethkrwPairs, err := NewPairsFromStrings([]string{"ETH-KRW"})
-	if err != nil {
-		t.Error(err)
-	}
+	require.NoError(t, err)
+	require.NotEmpty(t, pairs)
+	assert.True(t, pairs.Contains(ethusd, true), "StorePairs should store the pairs")
 
-	err = p.StorePairs(asset.Futures, ethkrwPairs, true)
-	if !errors.Is(err, nil) {
-		t.Fatalf("received: %v but expected: %v", err, nil)
-	}
+	err = p.StorePairs(asset.Futures, Pairs{ethkrw}, false)
+	require.NoError(t, err)
 
-	err = p.StorePairs(asset.Futures, ethkrwPairs, false)
-	if !errors.Is(err, nil) {
-		t.Fatalf("received: %v but expected: %v", err, nil)
-	}
+	err = p.StorePairs(asset.Futures, Pairs{ethkrw}, true)
+	require.NoError(t, err)
 
 	pairs, err = p.GetPairs(asset.Futures, true)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if pairs == nil {
-		t.Errorf("pairs futures should be populated")
-	}
-
-	ethkrw, err := NewPairFromString("ETH-KRW")
-	if err != nil {
-		t.Error(err)
-	}
-
-	if !pairs.Contains(ethkrw, true) {
-		t.Errorf("TestStorePairs failed, unexpected result")
-	}
+	require.NoError(t, err)
+	require.NotEmpty(t, pairs)
+	assert.True(t, pairs.Contains(ethkrw, true))
 }
 
 func TestDisablePair(t *testing.T) {

currency/pair_test.go

@@ -627,8 +627,8 @@ func TestFindPairDifferences(t *testing.T) {
 	}
 
 	_, err = pairList.FindDifferences(duplication, EMPTYFORMAT)
-	if !errors.Is(err, ErrPairDuplication) {
-		t.Fatalf("received: '%v' but expected: '%v'", err, ErrPairDuplication)
+	if !errors.Is(err, ErrDuplicatePairs) {
+		t.Fatalf("received: '%v' but expected: '%v'", err, ErrDuplicatePairs)
 	}
 
 	// This will allow for the removal of the duplicated item to be returned if

currency/pairs.go

@@ -7,16 +7,19 @@ import (
 	"math/rand"
 	"slices"
 	"strings"
+
+	"github.com/thrasher-corp/gocryptotrader/common"
+)
+
+// Public errors
+var (
+	ErrDuplicatePairs = errors.New("duplicate currency pair")
 )
 
 var (
 	errSymbolEmpty                = errors.New("symbol is empty")
 	errNoDelimiter                = errors.New("no delimiter was supplied")
 	errPairFormattingInconsistent = errors.New("pair formatting is inconsistent")
-
-	// ErrPairDuplication defines an error when there is multiple of the same
-	// currency pairs found.
-	ErrPairDuplication = errors.New("currency pair duplication")
 )
 
 // NewPairsFromStrings takes in currency pair strings and returns a currency
@@ -122,25 +125,18 @@ func (p Pairs) Contains(check Pair, exact bool) bool {
 
 // ContainsAll checks to see if all pairs supplied are contained within the original pairs list
 func (p Pairs) ContainsAll(check Pairs, exact bool) error {
-	comparative := slices.Clone(p)
+	cmp := slices.Clone(p)
 list:
-	for x := range check {
-		for y := range comparative {
-			if (exact && check[x].Equal(comparative[y])) ||
-				(!exact && check[x].EqualIncludeReciprocal(comparative[y])) {
-				// Reduce list size to decrease array traversal speed on iteration.
-				comparative[y] = comparative[len(comparative)-1]
-				comparative = comparative[:len(comparative)-1]
+	for _, a := range check {
+		for i, b := range cmp {
+			if (exact && a.Equal(b)) || (!exact && a.EqualIncludeReciprocal(b)) {
+				// Move last element to matched to reduce iterations
+				cmp[i], cmp = cmp[len(cmp)-1], cmp[:len(cmp)-1]
 				continue list
 			}
 		}
 
-		// Opted for in error original check for duplication.
-		if p.Contains(check[x], exact) {
-			return fmt.Errorf("%s %w", check[x], ErrPairDuplication)
-		}
-
-		return fmt.Errorf("%s %w", check[x], ErrPairNotFound)
+		return fmt.Errorf("%s %w", a, ErrPairNotFound)
 	}
 	return nil
 }
@@ -156,6 +152,19 @@ func (p Pairs) ContainsCurrency(check Code) bool {
 	return false
 }
 
+// ContainsDuplicate checks to see if the pairs contains duplicate pairs
+func (p Pairs) DuplicatePairs() (dups Pairs) {
+	p = common.SortStrings(p)
+
+	for i := 0; i < len(p)-1; i++ {
+		if p[i].Equal(p[i+1]) {
+			dups = append(dups, p[i])
+			i++
+		}
+	}
+	return
+}
+
 // RemovePairsByFilter checks to see if a pair contains a specific currency
 // and removes it from the list of pairs
 func (p Pairs) RemovePairsByFilter(filter Code) Pairs {
@@ -239,7 +248,7 @@ func (p Pairs) FindDifferences(incoming Pairs, pairFmt PairFormat) (PairDifferen
 		}
 		format := EMPTYFORMAT.Format(incoming[x])
 		if check[format] {
-			return PairDifference{}, fmt.Errorf("contained in the incoming pairs %w", ErrPairDuplication)
+			return PairDifference{}, fmt.Errorf("contained in the incoming pairs %w", ErrDuplicatePairs)
 		}
 		check[format] = true
 		if !p.Contains(incoming[x], true) {
@@ -413,7 +422,7 @@ func (p Pairs) ValidateAndConform(pFmt PairFormat, bypassFormatting bool) (Pairs
 		}
 		strippedPair := EMPTYFORMAT.Format(p[x])
 		if processedPairs[strippedPair] {
-			return nil, fmt.Errorf("cannot update pairs %w with [%s]", ErrPairDuplication, p[x])
+			return nil, fmt.Errorf("cannot update pairs %w with [%s]", ErrDuplicatePairs, p[x])
 		}
 		// Force application of supplied formatting
 		processedPairs[strippedPair] = true

currency/pairs_test.go

@@ -270,24 +270,33 @@ func TestContainsAll(t *testing.T) {
 		NewPair(BTC, USD),
 		NewPair(LTC, USD),
 		NewPair(USD, ZRX),
+		NewPair(ETH, USD),
+		NewPair(ETH, BTC),
 	}
 
-	assert.ErrorIs(t, pairs.ContainsAll(nil, true), ErrCurrencyPairsEmpty)
 	assert.NoError(t, pairs.ContainsAll(Pairs{NewPair(BTC, USD)}, true))
 	assert.NoError(t, pairs.ContainsAll(Pairs{NewPair(USD, BTC)}, false))
 	assert.ErrorIs(t, pairs.ContainsAll(Pairs{NewPair(XRP, BTC)}, false), ErrPairNotFound)
 	assert.ErrorIs(t, pairs.ContainsAll(Pairs{NewPair(XRP, BTC)}, true), ErrPairNotFound)
 	assert.NoError(t, pairs.ContainsAll(pairs, true))
 	assert.NoError(t, pairs.ContainsAll(pairs, false))
+}
 
-	var duplication = Pairs{
+func TestDuplicatePairs(t *testing.T) {
+	t.Parallel()
+	var pairs = Pairs{
 		NewPair(BTC, USD),
 		NewPair(LTC, USD),
 		NewPair(USD, ZRX),
-		NewPair(USD, ZRX),
 	}
 
-	assert.ErrorIs(t, pairs.ContainsAll(duplication, false), ErrPairDuplication)
+	assert.Empty(t, pairs.DuplicatePairs())
+	pairs = append(pairs, pairs[0])
+	require.Len(t, pairs.DuplicatePairs(), 1)
+	assert.Equal(t, pairs.DuplicatePairs()[0], pairs[0])
+	pairs[3] = pairs[2]
+	require.Len(t, pairs.DuplicatePairs(), 1)
+	assert.Equal(t, pairs.DuplicatePairs()[0], pairs[2])
 }
 
 func TestDeriveFrom(t *testing.T) {
@@ -632,8 +641,8 @@ func TestValidateAndConform(t *testing.T) {
 	}
 
 	_, err = conformMe.ValidateAndConform(EMPTYFORMAT, false)
-	if !errors.Is(err, ErrPairDuplication) {
-		t.Fatalf("received: '%v' but expected '%v'", err, ErrPairDuplication)
+	if !errors.Is(err, ErrDuplicatePairs) {
+		t.Fatalf("received: '%v' but expected '%v'", err, ErrDuplicatePairs)
 	}
 
 	conformMe = Pairs{

exchanges/exchange_test.go

@@ -1059,7 +1059,7 @@ func TestUpdatePairs(t *testing.T) {
 	assert.ErrorIs(t, err, currency.ErrCurrencyPairEmpty, "UpdatePairs should error on empty pairs")
 
 	err = UAC.UpdatePairs(currency.Pairs{btcusdPair, btcusdPair}, asset.Spot, false, true)
-	assert.ErrorIs(t, err, currency.ErrPairDuplication, "UpdatePairs should error on Duplicates")
+	assert.ErrorIs(t, err, currency.ErrDuplicatePairs, "UpdatePairs should error on Duplicates")
 
 	err = UAC.UpdatePairs(currency.Pairs{btcusdPair}, asset.Spot, false, true)
 	assert.NoError(t, err, "UpdatePairs should not error")