Repository: hyperledger/firefly
Pull request #974 (2022-08-17 18:33:36)

Leverage transaction cache in a few more places


Patch:
diff --git a/internal/orchestrator/data_query.go b/internal/orchestrator/data_query.go
index e9eb76d2d..c67bf217e 100644
--- a/internal/orchestrator/data_query.go
+++ b/internal/orchestrator/data_query.go
@@ -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) {
@@ -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) {
diff --git a/internal/txcommon/txcommon.go b/internal/txcommon/txcommon.go
index eb3b2856c..2c7948586 100644
--- a/internal/txcommon/txcommon.go
+++ b/internal/txcommon/txcommon.go
@@ -112,8 +112,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
 	}


Test patch:
diff --git a/internal/orchestrator/data_query_test.go b/internal/orchestrator/data_query_test.go
index 6a68ac775..74cb7ef82 100644
--- a/internal/orchestrator/data_query_test.go
+++ b/internal/orchestrator/data_query_test.go
@@ -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)
 }
@@ -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())
diff --git a/internal/orchestrator/txn_status_test.go b/internal/orchestrator/txn_status_test.go
index 07fccaca8..b01e31fcd 100644
--- a/internal/orchestrator/txn_status_test.go
+++ b/internal/orchestrator/txn_status_test.go
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
 
@@ -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")
@@ -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)
@@ -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"))
@@ -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)
@@ -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"))
@@ -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"))
@@ -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"))
@@ -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"))
@@ -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)