#4911: removed NewSystem unit test and related factory creation metho…

…ds for DataStore, BufferedStorageBackend, review feedback
stellar · Jul 17, 2024 · 08f3ab2 · 08f3ab2
1 parent e8261f8
commit 08f3ab2
Show file tree

Hide file tree

Showing 5 changed files with 4 additions and 41 deletions.
diff --git a/ingest/ledgerbackend/buffered_storage_backend.go b/ingest/ledgerbackend/buffered_storage_backend.go
@@ -17,8 +17,6 @@ import (
 // Ensure BufferedStorageBackend implements LedgerBackend
 var _ LedgerBackend = (*BufferedStorageBackend)(nil)
 
-type BufferedStorageBackendFactory func(config BufferedStorageBackendConfig, dataStore datastore.DataStore) (*BufferedStorageBackend, error)
-
 type BufferedStorageBackendConfig struct {
 	BufferSize uint32        `toml:"buffer_size"`
 	NumWorkers uint32        `toml:"num_workers"`

diff --git a/services/horizon/cmd/db.go b/services/horizon/cmd/db.go
@@ -15,14 +15,12 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 
-	"github.com/stellar/go/ingest/ledgerbackend"
 	horizon "github.com/stellar/go/services/horizon/internal"
 	"github.com/stellar/go/services/horizon/internal/db2/history"
 	"github.com/stellar/go/services/horizon/internal/db2/schema"
 	"github.com/stellar/go/services/horizon/internal/ingest"
 	"github.com/stellar/go/support/config"
 	support "github.com/stellar/go/support/config"
-	"github.com/stellar/go/support/datastore"
 	"github.com/stellar/go/support/db"
 	"github.com/stellar/go/support/errors"
 	hlog "github.com/stellar/go/support/log"
@@ -645,8 +643,6 @@ func loadStorageBackendConfig(storageBackendConfigPath string) (ingest.StorageBa
 		return ingest.StorageBackendConfig{}, fmt.Errorf("error unmarshalling datastore ledgerbackend TOML config: %w", err)
 	}
 
-	storageBackendConfig.BufferedStorageBackendFactory = ledgerbackend.NewBufferedStorageBackend
-	storageBackendConfig.DataStoreFactory = datastore.NewDataStore
 	return storageBackendConfig, nil
 }
 

diff --git a/services/horizon/internal/ingest/main.go b/services/horizon/internal/ingest/main.go
@@ -101,10 +101,8 @@ func (s LedgerBackendType) String() string {
 }
 
 type StorageBackendConfig struct {
-	DataStoreConfig               datastore.DataStoreConfig `toml:"datastore_config"`
-	DataStoreFactory              datastore.DataStoreFactory
-	BufferedStorageBackendConfig  ledgerbackend.BufferedStorageBackendConfig `toml:"buffered_storage_backend_config"`
-	BufferedStorageBackendFactory ledgerbackend.BufferedStorageBackendFactory
+	DataStoreConfig              datastore.DataStoreConfig                  `toml:"datastore_config"`
+	BufferedStorageBackendConfig ledgerbackend.BufferedStorageBackendConfig `toml:"buffered_storage_backend_config"`
 }
 
 type Config struct {
@@ -295,12 +293,12 @@ func NewSystem(config Config) (System, error) {
 	if config.LedgerBackendType == BufferedStorageBackend {
 		// Ingest from datastore
 		var dataStore datastore.DataStore
-		dataStore, err = config.StorageBackendConfig.DataStoreFactory(context.Background(), config.StorageBackendConfig.DataStoreConfig)
+		dataStore, err = datastore.NewDataStore(context.Background(), config.StorageBackendConfig.DataStoreConfig)
 		if err != nil {
 			cancel()
 			return nil, fmt.Errorf("failed to create datastore: %w", err)
 		}
-		ledgerBackend, err = config.StorageBackendConfig.BufferedStorageBackendFactory(config.StorageBackendConfig.BufferedStorageBackendConfig, dataStore)
+		ledgerBackend, err = ledgerbackend.NewBufferedStorageBackend(config.StorageBackendConfig.BufferedStorageBackendConfig, dataStore)
 		if err != nil {
 			cancel()
 			return nil, fmt.Errorf("failed to create buffered storage backend: %w", err)

diff --git a/services/horizon/internal/ingest/main_test.go b/services/horizon/internal/ingest/main_test.go
@@ -17,7 +17,6 @@ import (
 	"github.com/stellar/go/ingest"
 	"github.com/stellar/go/ingest/ledgerbackend"
 	"github.com/stellar/go/services/horizon/internal/db2/history"
-	"github.com/stellar/go/support/datastore"
 	"github.com/stellar/go/support/db"
 	"github.com/stellar/go/support/errors"
 	logpkg "github.com/stellar/go/support/log"
@@ -111,32 +110,6 @@ func TestNewSystem(t *testing.T) {
 	assert.Equal(t, system.maxLedgerPerFlush, MaxLedgersPerFlush)
 }
 
-func TestNewSystemBuffered(t *testing.T) {
-	mockDataStore := &datastore.MockDataStore{}
-	bufferedStorageBackend := &ledgerbackend.BufferedStorageBackend{}
-	config := Config{
-		HistorySession:      &db.Session{DB: &sqlx.DB{}},
-		HistoryArchiveURLs:  []string{"https://history.stellar.org/prd/core-live/core_live_001"},
-		CheckpointFrequency: 64,
-		LedgerBackendType:   BufferedStorageBackend,
-		StorageBackendConfig: StorageBackendConfig{
-			DataStoreConfig:              datastore.DataStoreConfig{Type: "GCS", Params: map[string]string{"destination_bucket_path": "Test"}},
-			BufferedStorageBackendConfig: ledgerbackend.BufferedStorageBackendConfig{NumWorkers: 1, BufferSize: 2},
-			DataStoreFactory: func(ctx context.Context, datastoreConfig datastore.DataStoreConfig) (datastore.DataStore, error) {
-				return mockDataStore, nil
-			},
-			BufferedStorageBackendFactory: func(config ledgerbackend.BufferedStorageBackendConfig, dataStore datastore.DataStore) (*ledgerbackend.BufferedStorageBackend, error) {
-				return bufferedStorageBackend, nil
-			},
-		},
-	}
-
-	sIface, err := NewSystem(config)
-	assert.NoError(t, err)
-	system := sIface.(*system)
-	assert.Same(t, system.ledgerBackend, bufferedStorageBackend)
-}
-
 // Custom comparator function.This function is needed because structs in Go that contain function fields
 // cannot be directly compared using assert.Equal, so here we compare each individual field, skipping the function fields.
 func CompareConfigs(t *testing.T, expected, actual Config) bool {

diff --git a/support/datastore/datastore.go b/support/datastore/datastore.go
@@ -13,8 +13,6 @@ type DataStoreConfig struct {
 	Schema DataStoreSchema   `toml:"schema"`
 }
 
-type DataStoreFactory func(ctx context.Context, datastoreConfig DataStoreConfig) (DataStore, error)
-
 // DataStore defines an interface for interacting with data storage
 type DataStore interface {
 	GetFileMetadata(ctx context.Context, path string) (map[string]string, error)