Experiment: Change transaction management

[rh-amarin]

This PR is to share an idea to change the current transaction management in TRex
Right now transactions are managed by a Transaction middleware that is configured in the API server

• All HTTP requests use transactions, regardless of them being GET/OPTIONS requests
• Rollbacks are performed by explicitly setting the transaction for rollback, in DAO or other layers
• Controllers/jobs should also manage the transactions, similar to transaction middleware, but there is none

This PR changes transaction management to happen at the service layer

• This is more explicit, the application can do finer grained transaction management
• Transactions are rolled back if an error is returned to the service layer
  • This may need better handling, what if we want to return and error AND do a transaction?
• In order to keep the business loginc methods in the service as free as possible from transaction management, that logic is delegated to an internal function. E.g. when creating a dinosaur, the Create function manages the tx, but the code to create the dinosaur and send an event is in the create function (note the case)

This PR is far from definitive, the transaction manager could be:

• Abstracted better
  • (e.g. https://blog.thibaut-rousseau.com/blog/sql-transactions-in-go-the-good-way/)
  • Use generics
• Allow services to return errors but commit transaction

One downside of doing tx in the service layer is the reference to db concerns, not abstracted in DAOs.
It is similar in this case to advisory lock management

Assisted-by: Claude

--- from here on, generated by claude ---

Transaction Management in TRex

Why We Don't Use db.MarkForRollback Anymore

This document explains the evolution of transaction management in the TRex application and why we moved away from manual rollback tracking.

Old Approach (Middleware-based)

How it worked

// Middleware creates transaction
TransactionMiddleware(next http.Handler) {
    ctx, err := NewContext(ctx, connection)  // Start transaction
    defer Resolve(ctx)  // Commit or rollback based on flag

    next.ServeHTTP(w, r)
}

// DAO explicitly marks for rollback
func (d *dao) Create(ctx, item) {
    err := db.Create(item)
    if err != nil {
        db.MarkForRollback(ctx, err)  // Set flag
        return err
    }
}

Problems

• Transaction started at HTTP layer (too early in the request lifecycle)
• Needed to track rollback state in context (added complexity)
• DAOs had to explicitly mark for rollback (error-prone, boilerplate)
• Transaction lived for entire HTTP request (including read operations)

New Approach (Service-layer)

How it works

// Service wraps logic in transaction
func (s *service) Create(ctx, item) {
    return db.WithTransaction(ctx, s.sessionFactory, func(txCtx) error {
        // Call internal business logic
        result, err := s.create(txCtx, item)
        if err != nil {
            return err  // GORM auto-rollbacks on error
        }
        return nil  // GORM auto-commits on nil
    })
}

// DAO just returns errors - no manual rollback marking
func (d *dao) Create(ctx, item) {
    err := db.Create(item)
    if err != nil {
        return err  // Just return the error
    }
    return nil
}

Key difference

// WithTransaction uses GORM's built-in transaction support
return db.Transaction(func(tx *gorm.DB) error {
    err := fn(txCtx)
    if err != nil {
        // GORM automatically rolls back
        return err
    }
    // GORM automatically commits
    return nil
})

Why This is Better

1. Automatic Management

GORM's .Transaction() automatically:

• Rolls back if function returns an error
• Commits if function returns nil
• No need to track rollback flags

2. Simpler Code

DAOs are cleaner - just return errors, don't need to call MarkForRollback:

// Old way - manual rollback tracking
func (d *dao) Create(ctx, item) {
    err := db.Create(item)
    if err != nil {
        db.MarkForRollback(ctx, err)  // Extra call needed
        return err
    }
}

// New way - automatic rollback
func (d *dao) Create(ctx, item) {
    err := db.Create(item)
    if err != nil {
        return err  // That's it!
    }
}

3. Natural Error Propagation

Errors flow up naturally:

DAO error → Service error → GORM rollback

4. Explicit Boundaries

Transaction boundaries are clear in the code:

db.WithTransaction(...)  // Transaction starts and ends here

5. Better Separation of Concerns

• HTTP Layer: Handles requests/responses
• Service Layer: Manages business logic and transactions
• DAO Layer: Performs database operations

Architecture Comparison

Old Architecture

HTTP Request
    ↓
Middleware (creates transaction)
    ↓
Handler
    ↓
Service
    ↓
DAO (marks for rollback)
    ↓
Middleware (resolves transaction)

New Architecture

HTTP Request
    ↓
Handler
    ↓
Service (creates transaction)
    ↓
Internal Service Method (business logic)
    ↓
DAO (returns errors)
    ↓
Service (GORM auto-commits/rollbacks)

Example: Create Dinosaur

Old Implementation

// Middleware
func TransactionMiddleware(next http.Handler, connection SessionFactory) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        ctx, err := NewContext(r.Context(), connection)
        if err != nil {
            // handle error
            return
        }
        *r = *r.WithContext(ctx)
        defer func() { Resolve(r.Context()) }()
        next.ServeHTTP(w, r)
    })
}

// Service
func (s *service) Create(ctx, dinosaur) {
    dino, err := s.dao.Create(ctx, dinosaur)
    if err != nil {
        return nil, handleCreateError(err)
    }

    _, err = s.events.Create(ctx, event)
    if err != nil {
        return nil, handleCreateError(err)
    }

    return dino, nil
}

// DAO
func (d *dao) Create(ctx, dinosaur) {
    err := g2.Create(dinosaur).Error
    if err != nil {
        db.MarkForRollback(ctx, err)  // Manual rollback marking
        return nil, err
    }
    return dinosaur, nil
}

New Implementation

// No middleware needed!

// Public service method - manages transaction
func (s *service) Create(ctx, dinosaur) (*api.Dinosaur, *errors.ServiceError) {
    var result *api.Dinosaur
    var serviceErr *errors.ServiceError

    txErr := db.WithTransaction(ctx, s.sessionFactory, func(txCtx context.Context) error {
        result, serviceErr = s.create(txCtx, dinosaur)
        if serviceErr != nil {
            return serviceErr
        }
        return nil
    })

    if txErr != nil {
        return nil, serviceErr
    }
    return result, nil
}

// Internal service method - business logic
func (s *service) create(ctx, dinosaur) (*api.Dinosaur, *errors.ServiceError) {
    dino, err := s.dao.Create(ctx, dinosaur)
    if err != nil {
        return nil, handleCreateError(err)
    }

    _, eventErr := s.events.Create(ctx, event)
    if eventErr != nil {
        return nil, eventErr  // Error propagates, GORM rolls back
    }

    return dino, nil
}

// DAO - clean, no rollback marking
func (d *dao) Create(ctx, dinosaur) {
    err := g2.Create(dinosaur).Error
    if err != nil {
        return nil, err  // Just return the error
    }
    return dinosaur, nil
}

Transaction Behavior

When Transaction Commits

db.WithTransaction(ctx, factory, func(txCtx) error {
    s.dao.Create(txCtx, item)      // Success
    s.events.Create(txCtx, event)  // Success
    return nil                      // ← Commits here
})

When Transaction Rolls Back

db.WithTransaction(ctx, factory, func(txCtx) error {
    s.dao.Create(txCtx, item)      // Success
    s.events.Create(txCtx, event)  // Error!
    return err                      // ← Rolls back here
})

What Gets Rolled Back

When an error occurs:

1. Dinosaur creation is rolled back
2. Event creation is rolled back
3. Both operations are atomic - all or nothing

Summary

Old Way

• Manual rollback tracking with flags
• db.MarkForRollback(ctx, err) everywhere
• Transaction managed by middleware
• Longer transaction lifetime
• More boilerplate code

New Way

• Automatic rollback based on error returns
• GORM handles it: error = rollback, nil = commit
• Transaction managed by service layer
• Transaction only when needed
• Cleaner, simpler, more idiomatic Go code

The MarkForRollback approach was necessary with middleware-based transactions, but with service-layer transactions using GORM's built-in support, we get automatic transaction management for free!

Additional Benefits

1. Performance: Transactions only for write operations (POST, PATCH, DELETE)
2. Testability: Easier to test - can mock session factory
3. Maintainability: Less code, fewer bugs
4. Clarity: Transaction scope is explicit in the code
5. Composability: Services can call other services within the same transaction

Transaction Management Refactoring

Summary

Moved database transaction management from HTTP middleware layer to service layer for better separation of concerns and more precise transaction control.

Changes Made

1. New Transaction Helper

• Added: db.WithTransaction() in pkg/db/transactions.go
• Wraps GORM's built-in transaction support
• Automatically commits on success, rolls back on error
• Supports test mode with nil database

2. Service Layer Updates

• Modified: pkg/services/dinosaurs.go
• Public methods (Create, Replace, Delete) manage transactions
• Internal methods (create, replace, delete) contain business logic
• Added sessionFactory parameter to service constructor

3. Session Factory Enhancement

• Modified: pkg/db/db_session/default.go and test.go
• Checks context for existing transaction before creating new session
• Enables nested transaction support

4. Advisory Lock Compatibility

• Modified: pkg/db/advisory_locks.go
• Detects and reuses parent transaction when available
• Only commits if it started the transaction (not nested)

5. Middleware Removal

• Modified: cmd/trex/server/routes.go
• Removed TransactionMiddleware registration
• HTTP layer no longer manages transactions

6. DAO Cleanup

• Modified: pkg/dao/dinosaur.go and pkg/dao/event.go
• Removed all db.MarkForRollback() calls
• DAOs now simply return errors

7. Test Infrastructure

• Added: pkg/db/mocks/session_factory.go
• Modified: pkg/services/dinosaurs_test.go
• Added: Integration test for transaction commit/rollback in test/integration/dinosaurs_test.go

Benefits

1. Precise Control: Transactions only wrap operations that modify data
2. Automatic Management: GORM handles commit/rollback based on error returns
3. Simpler Code: No manual rollback tracking needed
4. Better Architecture: Clear separation between HTTP, service, and data layers
5. Improved Testability: Services can be tested without database

Migration Notes

For new services or entities:

• Use db.WithTransaction() in service methods that modify data
• No need to call db.MarkForRollback()
• Just return errors normally - GORM handles the rest
• Pass sessionFactory to service constructors

Example:

func (s *service) Create(ctx context.Context, item *Item) (*Item, error) {
    var result *Item
    var serviceErr error

    txErr := db.WithTransaction(ctx, s.sessionFactory, func(txCtx context.Context) error {
        result, serviceErr = s.create(txCtx, item)
        if serviceErr != nil {
            return serviceErr
        }
        return nil
    })

    if txErr != nil {
        return nil, serviceErr
    }
    return result, nil
}

[markturansky]

I agree 100% with removing transactions from the HTTP handler. we discussed on a call AMS's implementation using owner_ids for resolution and so on. I'd like to talk through this impl again because i think it's correct: https://gitlab.cee.redhat.com/service/uhc-account-manager/-/blob/master/pkg/db/transactions.go?ref_type=heads#L16