Repository: hyperledger/firefly
Pull request #1216 (2023-03-10 05:11:14)

Ensure cached transaction gets updated with new blockchain IDs
There are multiple paths that record the blockchainID for a transaction. Along this path, we were updating the column in the database but not the cache - resulting in this field being intermittently out of sync.

codecov-commenter: ## [Codecov](https://codecov.io/gh/hyperledger/firefly/pull/1216?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hyperledger) Report
> Merging [#1216](https://codecov.io/gh/hyperledger/firefly/pull/1216?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hyperledger) (888a615) into [main](https://codecov.io/gh/hyperledger/firefly/commit/54cc1123a19aa5b8317551d5da4ac4d12cba8180?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hyperledger) (54cc112) will **not change** coverage.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff            @@
##              main     #1216   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          305       305           
  Lines        20074     20075    +1     
=========================================
+ Hits         20074     20075    +1     
```


| [Impacted Files](https://codecov.io/gh/hyperledger/firefly/pull/1216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hyperledger) | Coverage Δ | |
|---|---|---|
| [internal/txcommon/txcommon.go](https://codecov.io/gh/hyperledger/firefly/pull/1216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hyperledger#diff-aW50ZXJuYWwvdHhjb21tb24vdHhjb21tb24uZ28=) | `100.00% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hyperledger). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hyperledger)


Patch:
diff --git a/internal/txcommon/txcommon.go b/internal/txcommon/txcommon.go
index a17e730e7..85fe7de95 100644
--- a/internal/txcommon/txcommon.go
+++ b/internal/txcommon/txcommon.go
@@ -1,4 +1,4 @@
-// Copyright © 2022 Kaleido, Inc.
+// Copyright © 2023 Kaleido, Inc.
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -138,12 +138,13 @@ func (t *transactionHelper) PersistTransaction(ctx context.Context, id *fftypes.
 			return false, nil
 		}
 
-		newBlockchainIDs, changed := tx.BlockchainIDs.AddToSortedSet(blockchainTXID)
+		var changed bool
+		tx.BlockchainIDs, changed = tx.BlockchainIDs.AddToSortedSet(blockchainTXID)
 		if !changed {
 			return true, nil
 		}
 
-		if err = t.database.UpdateTransaction(ctx, t.namespace, tx.ID, database.TransactionQueryFactory.NewUpdate(ctx).Set("blockchainids", newBlockchainIDs)); err != nil {
+		if err = t.database.UpdateTransaction(ctx, t.namespace, tx.ID, database.TransactionQueryFactory.NewUpdate(ctx).Set("blockchainids", tx.BlockchainIDs)); err != nil {
 			return false, err
 		}
 	} else {


Test patch:
diff --git a/internal/txcommon/txcommon_test.go b/internal/txcommon/txcommon_test.go
index dda4a479f..6c21d1ff2 100644
--- a/internal/txcommon/txcommon_test.go
+++ b/internal/txcommon/txcommon_test.go
@@ -178,7 +178,7 @@ func TestPersistTransactionNew(t *testing.T) {
 
 }
 
-func TestPersistTransactionNewInserTFail(t *testing.T) {
+func TestPersistTransactionNewInsertFail(t *testing.T) {
 
 	mdi := &databasemocks.Plugin{}
 	mdm := &datamocks.Manager{}
@@ -205,7 +205,8 @@ func TestPersistTransactionExistingAddBlockchainID(t *testing.T) {
 	mdm := &datamocks.Manager{}
 	ctx := context.Background()
 	cmi := &cachemocks.Manager{}
-	cmi.On("GetCache", mock.Anything).Return(cache.NewUmanagedCache(ctx, 100, 5*time.Minute), nil)
+	cache := cache.NewUmanagedCache(ctx, 100, 5*time.Minute)
+	cmi.On("GetCache", mock.Anything).Return(cache, nil)
 	txHelper, _ := NewTransactionHelper(ctx, "ns1", mdi, mdm, cmi)
 
 	txid := fftypes.NewUUID()
@@ -222,6 +223,9 @@ func TestPersistTransactionExistingAddBlockchainID(t *testing.T) {
 	assert.NoError(t, err)
 	assert.True(t, valid)
 
+	cachedTX := cache.Get(txid.String()).(*core.Transaction)
+	assert.Equal(t, fftypes.FFStringArray{"0x111111", "0x222222"}, cachedTX.BlockchainIDs)
+
 	mdi.AssertExpectations(t)
 
 }