From 22e1be8f35cd13eb0e27312c5a8d801196158f2a Mon Sep 17 00:00:00 2001 From: Marwen Abid Date: Wed, 1 Nov 2023 19:16:06 -0700 Subject: [PATCH] SDP-761 - refactor, remove SqlxDB() from interface --- db/db.go | 1 - db/db_connection_pool_with_metrics.go | 5 --- db/db_connection_pool_with_metrics_test.go | 18 ----------- internal/data/assets_test.go | 28 ++++++++-------- internal/data/countries_test.go | 6 ++-- internal/data/wallets_test.go | 32 +++++++++---------- .../serve/httphandler/payments_handler.go | 2 +- 7 files changed, 34 insertions(+), 58 deletions(-) diff --git a/db/db.go b/db/db.go index 476ff658a..b5fd1a990 100644 --- a/db/db.go +++ b/db/db.go @@ -31,7 +31,6 @@ type DBConnectionPool interface { Close() error Ping() error SqlDB() *sql.DB - SqlxDB() *sqlx.DB DSN(ctx context.Context) (string, error) } diff --git a/db/db_connection_pool_with_metrics.go b/db/db_connection_pool_with_metrics.go index a94eeb9d9..e8fdd5cd2 100644 --- a/db/db_connection_pool_with_metrics.go +++ b/db/db_connection_pool_with_metrics.go @@ -5,7 +5,6 @@ import ( "database/sql" "fmt" - "github.com/jmoiron/sqlx" "github.com/stellar/stellar-disbursement-platform-backend/internal/monitor" ) @@ -51,10 +50,6 @@ func (dbc *DBConnectionPoolWithMetrics) SqlDB() *sql.DB { return dbc.dbConnectionPool.SqlDB() } -func (dbc *DBConnectionPoolWithMetrics) SqlxDB() *sqlx.DB { - return dbc.dbConnectionPool.SqlxDB() -} - func (dbc *DBConnectionPoolWithMetrics) DSN(ctx context.Context) (string, error) { return dbc.dbConnectionPool.DSN(ctx) } diff --git a/db/db_connection_pool_with_metrics_test.go b/db/db_connection_pool_with_metrics_test.go index 9214f2d14..25cef1e63 100644 --- a/db/db_connection_pool_with_metrics_test.go +++ b/db/db_connection_pool_with_metrics_test.go @@ -5,30 +5,12 @@ import ( "database/sql" "testing" - "github.com/jmoiron/sqlx" "github.com/stellar/stellar-disbursement-platform-backend/db/dbtest" "github.com/stellar/stellar-disbursement-platform-backend/internal/monitor" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestDBConnectionPoolWithMetrics_SqlxDB(t *testing.T) { - dbt := dbtest.Open(t) - defer dbt.Close() - dbConnectionPool, err := OpenDBConnectionPool(dbt.DSN) - require.NoError(t, err) - defer dbConnectionPool.Close() - - mMonitorService := &monitor.MockMonitorService{} - - dbConnectionPoolWithMetrics, err := NewDBConnectionPoolWithMetrics(dbConnectionPool, mMonitorService) - require.NoError(t, err) - - sqlxDB := dbConnectionPoolWithMetrics.SqlxDB() - - assert.IsType(t, &sqlx.DB{}, sqlxDB) -} - func TestDBConnectionPoolWithMetrics_SqlDB(t *testing.T) { dbt := dbtest.Open(t) defer dbt.Close() diff --git a/internal/data/assets_test.go b/internal/data/assets_test.go index 650b7d551..a8d666a6d 100644 --- a/internal/data/assets_test.go +++ b/internal/data/assets_test.go @@ -31,7 +31,7 @@ func Test_AssetModelGet(t *testing.T) { }) t.Run("returns asset successfully", func(t *testing.T) { - expected := CreateAssetFixture(t, ctx, dbConnectionPool.SqlxDB(), "USDC", "GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVV") + expected := CreateAssetFixture(t, ctx, dbConnectionPool, "USDC", "GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVV") actual, err := assetModel.Get(ctx, expected.ID) require.NoError(t, err) assert.Equal(t, expected, actual) @@ -57,7 +57,7 @@ func Test_AssetModelGetByCodeAndIssuer(t *testing.T) { }) t.Run("returns asset successfully", func(t *testing.T) { - expected := CreateAssetFixture(t, ctx, dbConnectionPool.SqlxDB(), "USDC", "GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVV") + expected := CreateAssetFixture(t, ctx, dbConnectionPool, "USDC", "GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVV") actual, err := assetModel.GetByCodeAndIssuer(ctx, expected.Code, expected.Issuer) require.NoError(t, err) assert.Equal(t, expected, actual) @@ -77,7 +77,7 @@ func Test_AssetModelGetAll(t *testing.T) { assetModel := &AssetModel{dbConnectionPool: dbConnectionPool} t.Run("returns all assets successfully", func(t *testing.T) { - expected := ClearAndCreateAssetFixtures(t, ctx, dbConnectionPool.SqlxDB()) + expected := ClearAndCreateAssetFixtures(t, ctx, dbConnectionPool) actual, err := assetModel.GetAll(ctx) require.NoError(t, err) @@ -85,7 +85,7 @@ func Test_AssetModelGetAll(t *testing.T) { }) t.Run("returns empty array when no assets", func(t *testing.T) { - DeleteAllAssetFixtures(t, ctx, dbConnectionPool.SqlxDB()) + DeleteAllAssetFixtures(t, ctx, dbConnectionPool) actual, err := assetModel.GetAll(ctx) require.NoError(t, err) @@ -145,7 +145,7 @@ func Test_AssetModelInsert(t *testing.T) { assetModel := &AssetModel{dbConnectionPool: dbConnectionPool} t.Run("inserts asset successfully", func(t *testing.T) { - DeleteAllAssetFixtures(t, ctx, dbConnectionPool.SqlxDB()) + DeleteAllAssetFixtures(t, ctx, dbConnectionPool) code := "USDT" issuer := "GBVHJTRLQRMIHRYTXZQOPVYCVVH7IRJN3DOFT7VC6U75CBWWBVDTWURG" @@ -159,7 +159,7 @@ func Test_AssetModelInsert(t *testing.T) { }) t.Run("re-create a deleted asset", func(t *testing.T) { - DeleteAllAssetFixtures(t, ctx, dbConnectionPool.SqlxDB()) + DeleteAllAssetFixtures(t, ctx, dbConnectionPool) code := "USDT" issuer := "GBVHJTRLQRMIHRYTXZQOPVYCVVH7IRJN3DOFT7VC6U75CBWWBVDTWURG" @@ -204,7 +204,7 @@ func Test_AssetModelInsert(t *testing.T) { }) t.Run("does not insert the same asset again", func(t *testing.T) { - DeleteAllAssetFixtures(t, ctx, dbConnectionPool.SqlxDB()) + DeleteAllAssetFixtures(t, ctx, dbConnectionPool) code := "USDT" issuer := "GBVHJTRLQRMIHRYTXZQOPVYCVVH7IRJN3DOFT7VC6U75CBWWBVDTWURG" @@ -218,7 +218,7 @@ func Test_AssetModelInsert(t *testing.T) { }) t.Run("creates the stellar native asset successfully", func(t *testing.T) { - DeleteAllAssetFixtures(t, ctx, dbConnectionPool.SqlxDB()) + DeleteAllAssetFixtures(t, ctx, dbConnectionPool) asset, err := assetModel.Insert(ctx, dbConnectionPool, "XLM", "") require.NoError(t, err) @@ -229,7 +229,7 @@ func Test_AssetModelInsert(t *testing.T) { }) t.Run("does not create an asset with empty issuer", func(t *testing.T) { - DeleteAllAssetFixtures(t, ctx, dbConnectionPool.SqlxDB()) + DeleteAllAssetFixtures(t, ctx, dbConnectionPool) asset, err := assetModel.Insert(ctx, dbConnectionPool, "USDC", "") assert.EqualError(t, err, `error inserting asset: pq: new row for relation "assets" violates check constraint "asset_issuer_length_check"`) @@ -237,7 +237,7 @@ func Test_AssetModelInsert(t *testing.T) { }) t.Run("does not create an asset with a invalid issuer", func(t *testing.T) { - DeleteAllAssetFixtures(t, ctx, dbConnectionPool.SqlxDB()) + DeleteAllAssetFixtures(t, ctx, dbConnectionPool) asset, err := assetModel.Insert(ctx, dbConnectionPool, "USDC", "INVALID") assert.EqualError(t, err, `error inserting asset: pq: new row for relation "assets" violates check constraint "asset_issuer_length_check"`) @@ -275,7 +275,7 @@ func Test_AssetModelGetOrCreate(t *testing.T) { }) t.Run("returns asset successfully", func(t *testing.T) { - expected := CreateAssetFixture(t, ctx, dbConnectionPool.SqlxDB(), "USDC", "GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVV") + expected := CreateAssetFixture(t, ctx, dbConnectionPool, "USDC", "GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVV") asset, err := assetModel.GetOrCreate(ctx, expected.Code, expected.Issuer) require.NoError(t, err) assert.Equal(t, expected.ID, asset.ID) @@ -295,8 +295,8 @@ func Test_AssetModelSoftDelete(t *testing.T) { assetModel := &AssetModel{dbConnectionPool: dbConnectionPool} t.Run("delete successful", func(t *testing.T) { - DeleteAllAssetFixtures(t, ctx, dbConnectionPool.SqlxDB()) - expected := CreateAssetFixture(t, ctx, dbConnectionPool.SqlxDB(), "USDC", "GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVV") + DeleteAllAssetFixtures(t, ctx, dbConnectionPool) + expected := CreateAssetFixture(t, ctx, dbConnectionPool, "USDC", "GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVV") asset, err := assetModel.SoftDelete(ctx, dbConnectionPool, expected.ID) require.NoError(t, err) @@ -311,7 +311,7 @@ func Test_AssetModelSoftDelete(t *testing.T) { }) t.Run("delete unsuccessful, cannot find asset", func(t *testing.T) { - DeleteAllAssetFixtures(t, ctx, dbConnectionPool.SqlxDB()) + DeleteAllAssetFixtures(t, ctx, dbConnectionPool) _, err := assetModel.SoftDelete(ctx, dbConnectionPool, "non-existant") require.Error(t, err) diff --git a/internal/data/countries_test.go b/internal/data/countries_test.go index 21f79e848..d9c3208c5 100644 --- a/internal/data/countries_test.go +++ b/internal/data/countries_test.go @@ -28,7 +28,7 @@ func Test_CountryModelGet(t *testing.T) { }) t.Run("returns asset successfully", func(t *testing.T) { - expected := CreateCountryFixture(t, ctx, dbConnectionPool.SqlxDB(), "FRA", "France") + expected := CreateCountryFixture(t, ctx, dbConnectionPool, "FRA", "France") actual, err := countryModel.Get(ctx, "FRA") require.NoError(t, err) @@ -48,7 +48,7 @@ func Test_CountryModelGetAll(t *testing.T) { countryModel := &CountryModel{dbConnectionPool: dbConnectionPool} t.Run("returns all countries successfully", func(t *testing.T) { - expected := ClearAndCreateCountryFixtures(t, ctx, dbConnectionPool.SqlxDB()) + expected := ClearAndCreateCountryFixtures(t, ctx, dbConnectionPool) actual, err := countryModel.GetAll(ctx) require.NoError(t, err) @@ -56,7 +56,7 @@ func Test_CountryModelGetAll(t *testing.T) { }) t.Run("returns empty array when no countries", func(t *testing.T) { - DeleteAllCountryFixtures(t, ctx, dbConnectionPool.SqlxDB()) + DeleteAllCountryFixtures(t, ctx, dbConnectionPool) actual, err := countryModel.GetAll(ctx) require.NoError(t, err) diff --git a/internal/data/wallets_test.go b/internal/data/wallets_test.go index 6121dd5ec..b41bd2528 100644 --- a/internal/data/wallets_test.go +++ b/internal/data/wallets_test.go @@ -28,7 +28,7 @@ func Test_WalletModelGet(t *testing.T) { }) t.Run("returns wallet successfully", func(t *testing.T) { - expected := CreateWalletFixture(t, ctx, dbConnectionPool.SqlxDB(), + expected := CreateWalletFixture(t, ctx, dbConnectionPool, "NewWallet", "https://newwallet.com", "newwallet.com", @@ -63,7 +63,7 @@ func Test_WalletModelGetByWalletName(t *testing.T) { }) t.Run("returns wallet successfully", func(t *testing.T) { - expected := CreateWalletFixture(t, ctx, dbConnectionPool.SqlxDB(), + expected := CreateWalletFixture(t, ctx, dbConnectionPool, "NewWallet", "https://newwallet.com", "newwallet.com", @@ -95,7 +95,7 @@ func Test_WalletModelGetAll(t *testing.T) { xlm := CreateAssetFixture(t, ctx, dbConnectionPool, "XLM", "") t.Run("returns all wallets successfully", func(t *testing.T) { - wallets := ClearAndCreateWalletFixtures(t, ctx, dbConnectionPool.SqlxDB()) + wallets := ClearAndCreateWalletFixtures(t, ctx, dbConnectionPool) wallet1 := wallets[0] wallet2 := wallets[1] @@ -129,7 +129,7 @@ func Test_WalletModelGetAll(t *testing.T) { }) t.Run("returns empty array when no wallets", func(t *testing.T) { - DeleteAllWalletFixtures(t, ctx, dbConnectionPool.SqlxDB()) + DeleteAllWalletFixtures(t, ctx, dbConnectionPool) actual, err := walletModel.GetAll(ctx) require.NoError(t, err) @@ -148,10 +148,10 @@ func Test_WalletModelFindWallets(t *testing.T) { walletModel := &WalletModel{dbConnectionPool: dbConnectionPool} t.Run("returns only enabled wallets", func(t *testing.T) { - wallets := ClearAndCreateWalletFixtures(t, ctx, dbConnectionPool.SqlxDB()) + wallets := ClearAndCreateWalletFixtures(t, ctx, dbConnectionPool) - EnableOrDisableWalletFixtures(t, ctx, dbConnectionPool.SqlxDB(), false, wallets[0].ID) - EnableOrDisableWalletFixtures(t, ctx, dbConnectionPool.SqlxDB(), true, wallets[1].ID) + EnableOrDisableWalletFixtures(t, ctx, dbConnectionPool, false, wallets[0].ID) + EnableOrDisableWalletFixtures(t, ctx, dbConnectionPool, true, wallets[1].ID) findEnabled := true actual, err := walletModel.FindWallets(ctx, &findEnabled) @@ -162,10 +162,10 @@ func Test_WalletModelFindWallets(t *testing.T) { }) t.Run("returns only disabled wallets", func(t *testing.T) { - wallets := ClearAndCreateWalletFixtures(t, ctx, dbConnectionPool.SqlxDB()) + wallets := ClearAndCreateWalletFixtures(t, ctx, dbConnectionPool) - EnableOrDisableWalletFixtures(t, ctx, dbConnectionPool.SqlxDB(), false, wallets[0].ID) - EnableOrDisableWalletFixtures(t, ctx, dbConnectionPool.SqlxDB(), true, wallets[1].ID) + EnableOrDisableWalletFixtures(t, ctx, dbConnectionPool, false, wallets[0].ID) + EnableOrDisableWalletFixtures(t, ctx, dbConnectionPool, true, wallets[1].ID) findDisabled := false actual, err := walletModel.FindWallets(ctx, &findDisabled) @@ -176,7 +176,7 @@ func Test_WalletModelFindWallets(t *testing.T) { }) t.Run("returns empty array when no wallets", func(t *testing.T) { - DeleteAllWalletFixtures(t, ctx, dbConnectionPool.SqlxDB()) + DeleteAllWalletFixtures(t, ctx, dbConnectionPool) actual, err := walletModel.FindWallets(ctx, nil) require.NoError(t, err) @@ -396,7 +396,7 @@ func Test_WalletModelGetOrCreate(t *testing.T) { t.Run("returns error wallet name already been used", func(t *testing.T) { DeleteAllWalletFixtures(t, ctx, dbConnectionPool) - CreateWalletFixture(t, ctx, dbConnectionPool.SqlxDB(), + CreateWalletFixture(t, ctx, dbConnectionPool, "test_wallet", "https://www.new_wallet.com", "www.new_wallet.com", @@ -429,7 +429,7 @@ func Test_WalletModelGetOrCreate(t *testing.T) { t.Run("returns wallet successfully", func(t *testing.T) { DeleteAllWalletFixtures(t, ctx, dbConnectionPool) - expected := CreateWalletFixture(t, ctx, dbConnectionPool.SqlxDB(), + expected := CreateWalletFixture(t, ctx, dbConnectionPool, "test_wallet", "https://www.test_wallet.com", "www.test_wallet.com", @@ -463,7 +463,7 @@ func Test_WalletModelGetAssets(t *testing.T) { t.Run("return empty when wallet doesn't have assets", func(t *testing.T) { DeleteAllWalletFixtures(t, ctx, dbConnectionPool) - wallet := CreateWalletFixture(t, ctx, dbConnectionPool.SqlxDB(), + wallet := CreateWalletFixture(t, ctx, dbConnectionPool, "NewWallet", "https://newwallet.com", "newwallet.com", @@ -476,7 +476,7 @@ func Test_WalletModelGetAssets(t *testing.T) { t.Run("return wallet's assets", func(t *testing.T) { DeleteAllWalletFixtures(t, ctx, dbConnectionPool) - wallet := CreateWalletFixture(t, ctx, dbConnectionPool.SqlxDB(), + wallet := CreateWalletFixture(t, ctx, dbConnectionPool, "NewWallet", "https://newwallet.com", "newwallet.com", @@ -572,7 +572,7 @@ func Test_WalletModelUpdate(t *testing.T) { _, err = walletModel.Update(ctx, "unknown", true) assert.Equal(t, ErrRecordNotFound, err) - wallet := CreateWalletFixture(t, ctx, dbConnectionPool.SqlxDB(), + wallet := CreateWalletFixture(t, ctx, dbConnectionPool, "NewWallet", "https://newwallet.com", "newwallet.com", diff --git a/internal/serve/httphandler/payments_handler.go b/internal/serve/httphandler/payments_handler.go index 0d26652b4..a955fcdae 100644 --- a/internal/serve/httphandler/payments_handler.go +++ b/internal/serve/httphandler/payments_handler.go @@ -43,7 +43,7 @@ func (r *RetryPaymentsRequest) validate() *httperror.HTTPError { func (p PaymentsHandler) GetPayment(w http.ResponseWriter, r *http.Request) { payment_id := chi.URLParam(r, "id") - payment, err := p.Models.Payment.Get(r.Context(), payment_id, p.DBConnectionPool.SqlxDB()) + payment, err := p.Models.Payment.Get(r.Context(), payment_id, p.DBConnectionPool) if err != nil { if errors.Is(data.ErrRecordNotFound, err) { errorResponse := fmt.Sprintf("Cannot retrieve payment with ID: %s", payment_id)