From c5f782fe627de3e19de1b3c03e5e4a1fb45d9a16 Mon Sep 17 00:00:00 2001
From: Charles Billette <charles@cbillette.com>
Date: Tue, 26 Nov 2024 09:25:18 -0500
Subject: [PATCH] Refactor sort function to use external sort value fetcher

Updated the Sort function to accept a SortValueFetcher interface and changed its implementations accordingly. This change improves flexibility and decouples sort value fetching logic from the client type. Also added zap logger dependency to NewClients initialization in tests.
---
 rpc/clients.go               |  4 ++--
 rpc/rolling_strategy_test.go |  4 +++-
 rpc/sort.go                  | 14 ++++++--------
 rpc/sort_test.go             | 17 ++++++++++++++---
 4 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/rpc/clients.go b/rpc/clients.go
index b326335..36a0066 100644
--- a/rpc/clients.go
+++ b/rpc/clients.go
@@ -28,11 +28,11 @@ func NewClients[C any](maxBlockFetchDuration time.Duration, rollingStrategy Roll
 	}
 }
 
-func (c *Clients[C]) StartSorting(ctx context.Context, direction SortDirection, every time.Duration) {
+func (c *Clients[C]) StartSorting(ctx context.Context, direction SortDirection, sortValueFetcher SortValueFetcher[C], every time.Duration) {
 	go func() {
 		for {
 			c.logger.Info("sorting clients")
-			err := Sort(ctx, c, direction)
+			err := Sort(ctx, c, sortValueFetcher, direction)
 			if err != nil {
 				c.logger.Warn("sorting", zap.Error(err))
 			}
diff --git a/rpc/rolling_strategy_test.go b/rpc/rolling_strategy_test.go
index d63c344..f1f8f55 100644
--- a/rpc/rolling_strategy_test.go
+++ b/rpc/rolling_strategy_test.go
@@ -6,6 +6,8 @@ import (
 	"testing"
 	"time"
 
+	"go.uber.org/zap"
+
 	"github.com/stretchr/testify/require"
 )
 
@@ -24,7 +26,7 @@ func TestStickyRollingStrategy(t *testing.T) {
 	rollingStrategy := NewStickyRollingStrategy[*rollClient]()
 	rollingStrategy.reset()
 
-	clients := NewClients(2*time.Second, rollingStrategy)
+	clients := NewClients(2*time.Second, rollingStrategy, zap.NewNop())
 	clients.Add(&rollClient{name: "c.1"})
 	clients.Add(&rollClient{name: "c.2"})
 	clients.Add(&rollClient{name: "c.3"})
diff --git a/rpc/sort.go b/rpc/sort.go
index 9f7902c..c7aafda 100644
--- a/rpc/sort.go
+++ b/rpc/sort.go
@@ -5,8 +5,8 @@ import (
 	"sort"
 )
 
-type SortValueFetcher interface {
-	fetchSortValue(ctx context.Context) (sortValue uint64, err error)
+type SortValueFetcher[C any] interface {
+	fetchSortValue(ctx context.Context, client C) (sortValue uint64, err error)
 }
 
 type SortDirection int
@@ -16,7 +16,7 @@ const (
 	SortDirectionDescending
 )
 
-func Sort[C any](ctx context.Context, clients *Clients[C], direction SortDirection) error {
+func Sort[C any](ctx context.Context, clients *Clients[C], sortValueFetch SortValueFetcher[C], direction SortDirection) error {
 	type sortable struct {
 		clientIndex int
 		sortValue   uint64
@@ -25,11 +25,9 @@ func Sort[C any](ctx context.Context, clients *Clients[C], direction SortDirecti
 	for i, client := range clients.clients {
 		var v uint64
 		var err error
-		if s, ok := any(client).(SortValueFetcher); ok {
-			v, err = s.fetchSortValue(ctx)
-			if err != nil {
-				//do nothing
-			}
+		v, err = sortValueFetch.fetchSortValue(ctx, client)
+		if err != nil {
+			//do nothing
 		}
 		sortableValues = append(sortableValues, sortable{i, v})
 	}
diff --git a/rpc/sort_test.go b/rpc/sort_test.go
index eb032e8..50c200f 100644
--- a/rpc/sort_test.go
+++ b/rpc/sort_test.go
@@ -5,21 +5,32 @@ import (
 	"testing"
 	"time"
 
+	"go.uber.org/zap"
+
 	"github.com/stretchr/testify/require"
 )
 
+type testSortFetcher struct {
+}
+
+func (t testSortFetcher) fetchSortValue(ctx context.Context, client *rollClient) (sortValue uint64, err error) {
+	return client.sortValue, nil
+}
+
 func TestClientsSort(t *testing.T) {
 	rollingStrategy := NewStickyRollingStrategy[*rollClient]()
 	rollingStrategy.reset()
 
-	clients := NewClients(2*time.Second, rollingStrategy)
+	clients := NewClients(2*time.Second, rollingStrategy, zap.NewNop())
 	clients.Add(&rollClient{name: "c.1", sortValue: 100})
 	clients.Add(&rollClient{name: "c.2", sortValue: 101})
 	clients.Add(&rollClient{name: "c.3", sortValue: 102})
 	clients.Add(&rollClient{name: "c.a", sortValue: 103})
 	clients.Add(&rollClient{name: "c.b", sortValue: 104})
 
-	err := Sort(context.Background(), clients, SortDirectionDescending)
+	fetcher := testSortFetcher{}
+
+	err := Sort(context.Background(), clients, fetcher, SortDirectionDescending)
 	require.NoError(t, err)
 
 	var names []string
@@ -29,7 +40,7 @@ func TestClientsSort(t *testing.T) {
 
 	require.Equal(t, []string{"c.b", "c.a", "c.3", "c.2", "c.1"}, names)
 
-	err = Sort(context.Background(), clients, SortDirectionAscending)
+	err = Sort(context.Background(), clients, fetcher, SortDirectionAscending)
 	require.NoError(t, err)
 
 	names = []string{}