Skip to content

Commit

Permalink
Merge pull request #974 from kaleido-io/tx
Browse files Browse the repository at this point in the history 
Leverage transaction cache in a few more places
  • Loading branch information
@peterbroadhurst
peterbroadhurst authored Aug 18, 2022
2 parents d54355d + e48c679 commit b8556a4
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 27 deletions.
4 changes: 2 additions & 2 deletions internal/orchestrator/data_query.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (or *orchestrator) GetTransactionByID(ctx context.Context, id string) (*cor
if err != nil {
return nil, err
}
return or.database().GetTransactionByID(ctx, or.namespace.LocalName, u)
return or.txHelper.GetTransactionByIDCached(ctx, u)
}

func (or *orchestrator) GetTransactionOperations(ctx context.Context, id string) ([]*core.Operation, *database.FilterResult, error) {
Expand Down Expand Up @@ -198,7 +198,7 @@ func (or *orchestrator) GetMessageTransaction(ctx context.Context, id string) (*
if err != nil {
return nil, err
}
return or.database().GetTransactionByID(ctx, or.namespace.LocalName, txID)
return or.txHelper.GetTransactionByIDCached(ctx, txID)
}

func (or *orchestrator) GetMessageEvents(ctx context.Context, id string, filter database.AndFilter) ([]*core.Event, *database.FilterResult, error) {
Expand Down
4 changes: 2 additions & 2 deletions internal/orchestrator/data_query_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestGetTransactionByID(t *testing.T) {
or := newTestOrchestrator()
defer or.cleanup(t)
u := fftypes.NewUUID()
or.mdi.On("GetTransactionByID", mock.Anything, "ns", u).Return(nil, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, u).Return(nil, nil)
_, err := or.GetTransactionByID(context.Background(), u.String())
assert.NoError(t, err)
}
Expand Down Expand Up @@ -266,7 +266,7 @@ func TestGetMessageTransactionOk(t *testing.T) {
ID: txID,
},
}, nil)
or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(&core.Transaction{
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(&core.Transaction{
ID: txID,
}, nil)
tx, err := or.GetMessageTransaction(context.Background(), msgID.String())
Expand Down
42 changes: 21 additions & 21 deletions internal/orchestrator/txn_status_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func TestGetTransactionStatusBatchPinSuccess(t *testing.T) {
},
}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(ops, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(events, nil, nil)
or.mdi.On("GetBatches", mock.Anything, "ns", mock.Anything).Return(batches, nil, nil)
Expand Down Expand Up @@ -137,7 +137,7 @@ func TestGetTransactionStatusBatchPinFail(t *testing.T) {
events := []*core.BlockchainEvent{}
batches := []*core.BatchPersisted{}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(ops, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(events, nil, nil)
or.mdi.On("GetBatches", mock.Anything, "ns", mock.Anything).Return(batches, nil, nil)
Expand Down Expand Up @@ -191,7 +191,7 @@ func TestGetTransactionStatusBatchPinPending(t *testing.T) {
events := []*core.BlockchainEvent{}
batches := []*core.BatchPersisted{}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(ops, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(events, nil, nil)
or.mdi.On("GetBatches", mock.Anything, "ns", mock.Anything).Return(batches, nil, nil)
Expand Down Expand Up @@ -262,7 +262,7 @@ func TestGetTransactionStatusTokenPoolSuccess(t *testing.T) {
},
}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(ops, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(events, nil, nil)
or.mdi.On("GetTokenPools", mock.Anything, "ns", mock.Anything).Return(pools, nil, nil)
Expand Down Expand Up @@ -324,7 +324,7 @@ func TestGetTransactionStatusTokenPoolPending(t *testing.T) {
events := []*core.BlockchainEvent{}
pools := []*core.TokenPool{}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(ops, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(events, nil, nil)
or.mdi.On("GetTokenPools", mock.Anything, "ns", mock.Anything).Return(pools, nil, nil)
Expand Down Expand Up @@ -382,7 +382,7 @@ func TestGetTransactionStatusTokenPoolUnconfirmed(t *testing.T) {
},
}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(ops, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(events, nil, nil)
or.mdi.On("GetTokenPools", mock.Anything, "ns", mock.Anything).Return(pools, nil, nil)
Expand Down Expand Up @@ -450,7 +450,7 @@ func TestGetTransactionStatusTokenTransferSuccess(t *testing.T) {
},
}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(ops, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(events, nil, nil)
or.mdi.On("GetTokenTransfers", mock.Anything, "ns", mock.Anything).Return(transfers, nil, nil)
Expand Down Expand Up @@ -526,7 +526,7 @@ func TestGetTransactionStatusTokenApprovalSuccess(t *testing.T) {
},
}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(ops, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(events, nil, nil)
or.mdi.On("GetTokenApprovals", mock.Anything, "ns", mock.Anything).Return(approvals, nil, nil)
Expand Down Expand Up @@ -586,7 +586,7 @@ func TestGetTransactionStatusTokenTransferPending(t *testing.T) {
events := []*core.BlockchainEvent{}
transfers := []*core.TokenTransfer{}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(ops, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(events, nil, nil)
or.mdi.On("GetTokenTransfers", mock.Anything, "ns", mock.Anything).Return(transfers, nil, nil)
Expand Down Expand Up @@ -648,7 +648,7 @@ func TestGetTransactionStatusTokenTransferRetry(t *testing.T) {
events := []*core.BlockchainEvent{}
transfers := []*core.TokenTransfer{}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(ops, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(events, nil, nil)
or.mdi.On("GetTokenTransfers", mock.Anything, "ns", mock.Anything).Return(transfers, nil, nil)
Expand Down Expand Up @@ -707,7 +707,7 @@ func TestGetTransactionStatusTokenApprovalPending(t *testing.T) {
events := []*core.BlockchainEvent{}
approvals := []*core.TokenApproval{}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(ops, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(events, nil, nil)
or.mdi.On("GetTokenApprovals", mock.Anything, "ns", mock.Anything).Return(approvals, nil, nil)
Expand Down Expand Up @@ -761,7 +761,7 @@ func TestGetTransactionStatusContractInvokeSuccess(t *testing.T) {
}
events := []*core.BlockchainEvent{}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(ops, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(events, nil, nil)

Expand Down Expand Up @@ -791,7 +791,7 @@ func TestGetTransactionStatusTXError(t *testing.T) {
or := newTestOrchestrator()

txID := fftypes.NewUUID()
or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(nil, fmt.Errorf("pop"))
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(nil, fmt.Errorf("pop"))

_, err := or.GetTransactionStatus(context.Background(), txID.String())
assert.EqualError(t, err, "pop")
Expand All @@ -803,7 +803,7 @@ func TestGetTransactionStatusNotFound(t *testing.T) {
or := newTestOrchestrator()

txID := fftypes.NewUUID()
or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(nil, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(nil, nil)

_, err := or.GetTransactionStatus(context.Background(), txID.String())
assert.Regexp(t, "FF10109", err)
Expand All @@ -815,7 +815,7 @@ func TestGetTransactionStatusOpError(t *testing.T) {
or := newTestOrchestrator()

txID := fftypes.NewUUID()
or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(&core.Transaction{
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(&core.Transaction{
Namespace: "ns1",
}, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(nil, nil, fmt.Errorf("pop"))
Expand All @@ -830,7 +830,7 @@ func TestGetTransactionStatusBlockchainEventError(t *testing.T) {
or := newTestOrchestrator()

txID := fftypes.NewUUID()
or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(&core.Transaction{
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(&core.Transaction{
Namespace: "ns1",
}, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(nil, nil, nil)
Expand All @@ -851,7 +851,7 @@ func TestGetTransactionStatusBatchError(t *testing.T) {
Type: core.TransactionTypeBatchPin,
}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(nil, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(nil, nil, nil)
or.mdi.On("GetBatches", mock.Anything, "ns", mock.Anything).Return(nil, nil, fmt.Errorf("pop"))
Expand All @@ -871,7 +871,7 @@ func TestGetTransactionStatusPoolError(t *testing.T) {
Type: core.TransactionTypeTokenPool,
}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(nil, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(nil, nil, nil)
or.mdi.On("GetTokenPools", mock.Anything, "ns", mock.Anything).Return(nil, nil, fmt.Errorf("pop"))
Expand All @@ -891,7 +891,7 @@ func TestGetTransactionStatusTransferError(t *testing.T) {
Type: core.TransactionTypeTokenTransfer,
}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(nil, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(nil, nil, nil)
or.mdi.On("GetTokenTransfers", mock.Anything, "ns", mock.Anything).Return(nil, nil, fmt.Errorf("pop"))
Expand All @@ -911,7 +911,7 @@ func TestGetTransactionStatusApprovalError(t *testing.T) {
Type: core.TransactionTypeTokenApproval,
}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(nil, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(nil, nil, nil)
or.mdi.On("GetTokenApprovals", mock.Anything, "ns", mock.Anything).Return(nil, nil, fmt.Errorf("pop"))
Expand All @@ -931,7 +931,7 @@ func TestGetTransactionStatusUnknownType(t *testing.T) {
Type: "bad",
}

or.mdi.On("GetTransactionByID", mock.Anything, "ns", txID).Return(tx, nil)
or.mth.On("GetTransactionByIDCached", mock.Anything, txID).Return(tx, nil)
or.mdi.On("GetOperations", mock.Anything, "ns", mock.Anything).Return(nil, nil, nil)
or.mdi.On("GetBlockchainEvents", mock.Anything, "ns", mock.Anything).Return(nil, nil, nil)

Expand Down
3 changes: 1 addition & 2 deletions internal/txcommon/txcommon.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ func (t *transactionHelper) SubmitNewTransaction(ctx context.Context, txType cor
// PersistTransaction is called when we need to ensure a transaction exists in the DB, and optionally associate a new BlockchainTXID to it
func (t *transactionHelper) PersistTransaction(ctx context.Context, id *fftypes.UUID, txType core.TransactionType, blockchainTXID string) (valid bool, err error) {

// TODO: Consider if this can exploit caching
tx, err := t.database.GetTransactionByID(ctx, t.namespace, id)
tx, err := t.GetTransactionByIDCached(ctx, id)
if err != nil {
return false, err
}
Expand Down

0 comments on commit b8556a4

Please sign in to comment.