From 42eb264d134f9cdc1deff17d97b2f868597e275d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 21 Feb 2026 09:23:10 +0000 Subject: [PATCH 1/3] Initial plan From eb995e409a2b1cb82f875a3908a292e4ca1af193 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 21 Feb 2026 09:28:31 +0000 Subject: [PATCH 2/3] Add BeginTransactionWithContext and WithExternalTransaction for GORM integration Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com> --- enforcer_transactional.go | 55 ++++++++++++ transaction.go | 1 + transaction_commit.go | 30 ++++--- transaction_test.go | 170 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 245 insertions(+), 11 deletions(-) diff --git a/enforcer_transactional.go b/enforcer_transactional.go index c0abfb7dd..11ff32477 100644 --- a/enforcer_transactional.go +++ b/enforcer_transactional.go @@ -119,3 +119,58 @@ func (te *TransactionalEnforcer) WithTransaction(ctx context.Context, fn func(*T return tx.Commit() } + +// BeginTransactionWithContext starts a new Casbin transaction using an existing TransactionContext. +// This enables Casbin operations to participate in an externally-managed database transaction +// (e.g. a GORM transaction). Casbin will apply buffered policy operations through +// txContext.GetAdapter() but will NOT call txContext.Commit() or txContext.Rollback() — +// the caller is solely responsible for committing or rolling back the external transaction. +func (te *TransactionalEnforcer) BeginTransactionWithContext(ctx context.Context, txContext persist.TransactionContext) (*Transaction, error) { + buffer := NewTransactionBuffer(te.model) + + tx := &Transaction{ + id: uuid.New().String(), + enforcer: te, + buffer: buffer, + txContext: txContext, + ctx: ctx, + baseVersion: atomic.LoadInt64(&te.modelVersion), + startTime: time.Now(), + isExternal: true, + } + + te.activeTransactions.Store(tx.id, tx) + return tx, nil +} + +// WithExternalTransaction executes fn within the scope of an existing, externally-managed +// database transaction. txContext must wrap the external transaction and provide a +// Casbin adapter (via GetAdapter) that writes through it. +// +// On success, Casbin applies the buffered policy operations to the database using the +// external transaction and updates the in-memory model. The database transaction itself +// is NOT committed by Casbin — the caller must commit (or roll back) it. +// +// On failure (fn returns an error or a panic occurs), Casbin does NOT roll back the +// external transaction; the caller is responsible for that as well. +func (te *TransactionalEnforcer) WithExternalTransaction(ctx context.Context, txContext persist.TransactionContext, fn func(*Transaction) error) error { + tx, err := te.BeginTransactionWithContext(ctx, txContext) + if err != nil { + return err + } + + defer func() { + if r := recover(); r != nil { + _ = tx.Rollback() + panic(r) + } + }() + + err = fn(tx) + if err != nil { + _ = tx.Rollback() + return err + } + + return tx.Commit() +} diff --git a/transaction.go b/transaction.go index 3aac9e305..0aa1b40b9 100644 --- a/transaction.go +++ b/transaction.go @@ -43,6 +43,7 @@ type Transaction struct { rolledBack bool // Whether the transaction has been rolled back. startTime time.Time // Transaction start timestamp. mutex sync.RWMutex // Protects transaction state. + isExternal bool // Whether the DB transaction is managed externally. } // AddPolicy adds a policy within the transaction. diff --git a/transaction_commit.go b/transaction_commit.go index bdb6bd646..7196caa48 100644 --- a/transaction_commit.go +++ b/transaction_commit.go @@ -61,8 +61,10 @@ func (tx *Transaction) Commit() error { // If no operations, just commit the database transaction and clear state. if !tx.buffer.HasOperations() { - if err := tx.txContext.Commit(); err != nil { - return err + if !tx.isExternal { + if err := tx.txContext.Commit(); err != nil { + return err + } } tx.committed = true tx.enforcer.activeTransactions.Delete(tx.id) @@ -71,16 +73,20 @@ func (tx *Transaction) Commit() error { // Phase 1: Apply all buffered operations to the database if err := tx.applyOperationsToDatabase(); err != nil { - // Rollback database transaction on failure. - _ = tx.txContext.Rollback() + // Rollback database transaction on failure (only when managing our own transaction). + if !tx.isExternal { + _ = tx.txContext.Rollback() + } tx.enforcer.activeTransactions.Delete(tx.id) return err } - // Commit database transaction. - if err := tx.txContext.Commit(); err != nil { - tx.enforcer.activeTransactions.Delete(tx.id) - return err + // Commit database transaction (only when managing our own transaction). + if !tx.isExternal { + if err := tx.txContext.Commit(); err != nil { + tx.enforcer.activeTransactions.Delete(tx.id) + return err + } } // Phase 2: Apply changes to the in-memory model @@ -120,9 +126,11 @@ func (tx *Transaction) Rollback() error { return errors.New("transaction already rolled back") } - // Rollback database transaction. - if err := tx.txContext.Rollback(); err != nil { - return err + // Rollback database transaction (only when managing our own transaction). + if !tx.isExternal { + if err := tx.txContext.Rollback(); err != nil { + return err + } } tx.rolledBack = true diff --git a/transaction_test.go b/transaction_test.go index 5dcc1c0ae..2acf8768d 100644 --- a/transaction_test.go +++ b/transaction_test.go @@ -335,3 +335,173 @@ func TestTransactionBuffer(t *testing.T) { tx.Rollback() } + +// externalTxContext wraps a MockTransactionalAdapter to simulate an externally-managed +// DB transaction (e.g. GORM). Commit and Rollback are intentional no-ops because the +// external system owns the transaction lifecycle. +type externalTxContext struct { + adapter *MockTransactionalAdapter + committed bool + rolledBack bool +} + +func (e *externalTxContext) Commit() error { + e.committed = true + return nil +} + +func (e *externalTxContext) Rollback() error { + e.rolledBack = true + return nil +} + +func (e *externalTxContext) GetAdapter() persist.Adapter { + return e.adapter +} + +// TestBeginTransactionWithContext verifies that Casbin operations are applied to the +// database adapter but the external transaction's Commit/Rollback are never called. +func TestBeginTransactionWithContext(t *testing.T) { + adapter := NewMockTransactionalAdapter() + e, err := NewTransactionalEnforcer("examples/rbac_model.conf", adapter) + if err != nil { + t.Fatalf("Failed to create transactional enforcer: %v", err) + } + adapter.Enforcer = e.Enforcer + + ctx := context.Background() + extCtx := &externalTxContext{adapter: adapter} + + tx, err := e.BeginTransactionWithContext(ctx, extCtx) + if err != nil { + t.Fatalf("Failed to begin transaction with context: %v", err) + } + + ok, err := tx.AddPolicy("alice", "data1", "read") + if !ok || err != nil { + t.Fatalf("Failed to add policy in external transaction: %v", err) + } + + if err := tx.Commit(); err != nil { + t.Fatalf("Failed to commit external transaction: %v", err) + } + + // Casbin should NOT have called Commit on the external context. + if extCtx.committed { + t.Error("Casbin must not commit the external transaction context") + } + if extCtx.rolledBack { + t.Error("Casbin must not rollback the external transaction context") + } + + // The in-memory model should reflect the added policy. + bufferedModel := e.GetModel() + hasPolicy, _ := bufferedModel.HasPolicy("p", "p", []string{"alice", "data1", "read"}) + if !hasPolicy { + t.Fatal("In-memory model should contain the added policy after commit") + } +} + +// TestBeginTransactionWithContextRollback verifies that rolling back an external +// transaction does not touch the external DB transaction. +func TestBeginTransactionWithContextRollback(t *testing.T) { + adapter := NewMockTransactionalAdapter() + e, err := NewTransactionalEnforcer("examples/rbac_model.conf", adapter) + if err != nil { + t.Fatalf("Failed to create transactional enforcer: %v", err) + } + adapter.Enforcer = e.Enforcer + + ctx := context.Background() + extCtx := &externalTxContext{adapter: adapter} + + tx, err := e.BeginTransactionWithContext(ctx, extCtx) + if err != nil { + t.Fatalf("Failed to begin transaction with context: %v", err) + } + + if _, err := tx.AddPolicy("alice", "data1", "read"); err != nil { + t.Fatalf("Failed to add policy in external transaction: %v", err) + } + + if err := tx.Rollback(); err != nil { + t.Fatalf("Failed to rollback external transaction: %v", err) + } + + // Casbin should NOT have called Rollback on the external context. + if extCtx.rolledBack { + t.Error("Casbin must not rollback the external transaction context") + } + if extCtx.committed { + t.Error("Casbin must not commit the external transaction context") + } +} + +// TestWithExternalTransaction verifies the convenience wrapper applies operations +// and does not commit/rollback the external context. +func TestWithExternalTransaction(t *testing.T) { + adapter := NewMockTransactionalAdapter() + e, err := NewTransactionalEnforcer("examples/rbac_model.conf", adapter) + if err != nil { + t.Fatalf("Failed to create transactional enforcer: %v", err) + } + adapter.Enforcer = e.Enforcer + + ctx := context.Background() + extCtx := &externalTxContext{adapter: adapter} + + err = e.WithExternalTransaction(ctx, extCtx, func(tx *Transaction) error { + _, err := tx.AddPolicy("bob", "data2", "write") + return err + }) + if err != nil { + t.Fatalf("WithExternalTransaction failed: %v", err) + } + + // External context must not be committed/rolled back by Casbin. + if extCtx.committed { + t.Error("Casbin must not commit the external transaction context") + } + if extCtx.rolledBack { + t.Error("Casbin must not rollback the external transaction context") + } + + // In-memory model should reflect the change. + hasPolicy, _ := e.GetModel().HasPolicy("p", "p", []string{"bob", "data2", "write"}) + if !hasPolicy { + t.Fatal("In-memory model should contain the added policy after WithExternalTransaction") + } +} + +// TestWithExternalTransactionRollbackOnError verifies that when fn returns an error, +// the external context is not rolled back by Casbin. +func TestWithExternalTransactionRollbackOnError(t *testing.T) { + adapter := NewMockTransactionalAdapter() + e, err := NewTransactionalEnforcer("examples/rbac_model.conf", adapter) + if err != nil { + t.Fatalf("Failed to create transactional enforcer: %v", err) + } + adapter.Enforcer = e.Enforcer + + ctx := context.Background() + extCtx := &externalTxContext{adapter: adapter} + + fnErr := errors.New("business logic failure") + err = e.WithExternalTransaction(ctx, extCtx, func(tx *Transaction) error { + if _, addErr := tx.AddPolicy("charlie", "data3", "read"); addErr != nil { + return addErr + } + return fnErr + }) + if !errors.Is(err, fnErr) { + t.Fatalf("Expected fnErr, got %v", err) + } + + // Casbin must not touch the external transaction. + if extCtx.rolledBack { + t.Error("Casbin must not rollback the external transaction context on error") + } + if extCtx.committed { + t.Error("Casbin must not commit the external transaction context on error") + } +} From f0f0a126b3bfbd00fe9b9683bd3aa23ef9872ac7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 21 Feb 2026 09:41:47 +0000 Subject: [PATCH 3/3] Fix govet shadow: rename err to addErr in test closure Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com> --- transaction_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/transaction_test.go b/transaction_test.go index 2acf8768d..ad69e82d9 100644 --- a/transaction_test.go +++ b/transaction_test.go @@ -451,8 +451,8 @@ func TestWithExternalTransaction(t *testing.T) { extCtx := &externalTxContext{adapter: adapter} err = e.WithExternalTransaction(ctx, extCtx, func(tx *Transaction) error { - _, err := tx.AddPolicy("bob", "data2", "write") - return err + _, addErr := tx.AddPolicy("bob", "data2", "write") + return addErr }) if err != nil { t.Fatalf("WithExternalTransaction failed: %v", err)