From 6413e15833b123b872da3b9746588ec0ba7a3740 Mon Sep 17 00:00:00 2001 From: Elias Van Ootegem Date: Fri, 14 Jun 2024 10:29:53 +0100 Subject: [PATCH] fix: handle commit-rollback in unit tests Signed-off-by: Elias Van Ootegem --- datanode/sqlstore/assets_test.go | 9 +-------- datanode/sqlstore/connection_tx.go | 8 +++++++- datanode/sqlstore/sqlstore_test.go | 2 ++ 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/datanode/sqlstore/assets_test.go b/datanode/sqlstore/assets_test.go index 72afef77975..47ee0abde8e 100644 --- a/datanode/sqlstore/assets_test.go +++ b/datanode/sqlstore/assets_test.go @@ -120,18 +120,11 @@ func TestAssetCache(t *testing.T) { require.NoError(t, err) assert.Equal(t, asset2, fetched) - // Commit the sub-transaction and fetch the asset, we should not yet get the asset with the new symbol + // after commit, the new asset should be there already err = connectionSource.Commit(txCtx) require.NoError(t, err) fetched, err = as.GetByID(ctx, string(asset.ID)) require.NoError(t, err) - assert.Equal(t, asset2, fetched) - - // now commit the main transaction, then we should get the new symbol - err = connectionSource.Commit(ctx) - require.NoError(t, err) - fetched, err = as.GetByID(context.Background(), string(asset.ID)) - require.NoError(t, err) assert.Equal(t, asset3, fetched) } diff --git a/datanode/sqlstore/connection_tx.go b/datanode/sqlstore/connection_tx.go index 203ed3610c7..e997c4163f3 100644 --- a/datanode/sqlstore/connection_tx.go +++ b/datanode/sqlstore/connection_tx.go @@ -142,7 +142,8 @@ func (c *ConnectionSource) Commit(ctx context.Context) error { return fmt.Errorf("failed to commit transaction for context: %s, error: %w", ctx, err) } // invoke all post-commit hooks once the transaction (and its sub transactions) have been committed - if tx.parent != nil { + // make an exception for unit tests, so we don't need to commit DB transactions for hooks on the nested transaction. + if !c.isTest && tx.parent != nil { // this is a nested transaction, don't invoke hooks until the parent is committed // instead prepend the hooks and return. tx.parent.mu.Lock() @@ -156,6 +157,11 @@ func (c *ConnectionSource) Commit(ctx context.Context) error { for _, f := range post { f() } + if tx.parent != nil { + tx.parent.mu.Lock() + delete(tx.parent.subTx, tx.id) + tx.parent.mu.Unlock() + } return nil } diff --git a/datanode/sqlstore/sqlstore_test.go b/datanode/sqlstore/sqlstore_test.go index ddc4cc44beb..3e107f11d44 100644 --- a/datanode/sqlstore/sqlstore_test.go +++ b/datanode/sqlstore/sqlstore_test.go @@ -53,6 +53,8 @@ func TestMain(m *testing.M) { databasetest.TestMain(m, ctx, func(cfg sqlstore.Config, source *sqlstore.ConnectionSource, postgresLog *bytes.Buffer, ) { + // ensures nested transactions execute the post-commit hooks while the parent transaction still rolls back the overall changes. + source.ToggleTest() testDBPort = cfg.ConnectionConfig.Port connectionSource = source testConfig = cfg