Add improvements

Signed-off-by: AkramBitar <akram@il.ibm.com>

Author: AKRAM@il.ibm.com

token/services/ttx/collectendorsements.go

@@ -86,14 +86,7 @@ func (c *CollectEndorsementsView) Call(context view.Context) (interface{}, error
 	externalWallets := make(map[string]ExternalWalletSigner)
 
 	// Ensure Done() is called on all external wallets regardless of errors
-	defer func() {
-		logger.DebugfContext(context.Context(), "Inform external wallets that endorsement is complete")
-		for id, signer := range externalWallets {
-			if err := signer.Done(); err != nil {
-				logger.ErrorfContext(context.Context(), "failed to signal done external wallet [%s]", id)
-			}
-		}
-	}()
+	defer c.CleanupExternalWallets(context, externalWallets)
 
 	// 1. First collect signatures on the token request
 	issueSigmas, err := c.requestSignaturesOnIssues(context, externalWallets)
@@ -667,3 +660,13 @@ func TransferDistributionList(r *token.Request) []view.Identity {
 	}
 	return distributionList
 }
+
+// CleanupExternalWallets calls Done() on all external wallets to signal completion
+func (c *CollectEndorsementsView) CleanupExternalWallets(context view.Context, externalWallets map[string]ExternalWalletSigner) {
+	logger.DebugfContext(context.Context(), "Inform external wallets that endorsement is complete")
+	for id, signer := range externalWallets {
+		if err := signer.Done(); err != nil {
+			logger.ErrorfContext(context.Context(), "failed to signal done external wallet [%s]", id)
+		}
+	}
+}

token/services/ttx/collectendorsements_test.go

@@ -7,285 +7,123 @@ SPDX-License-Identifier: Apache-2.0
 package ttx_test
 
 import (
+	"context"
 	"testing"
 
 	"github.com/hyperledger-labs/fabric-smart-client/pkg/utils/errors"
-	"github.com/hyperledger-labs/fabric-smart-client/platform/view/services/endpoint"
 	"github.com/hyperledger-labs/fabric-smart-client/platform/view/view"
-	"github.com/hyperledger-labs/fabric-token-sdk/token"
-	"github.com/hyperledger-labs/fabric-token-sdk/token/driver"
-	"github.com/hyperledger-labs/fabric-token-sdk/token/driver/mock"
 	"github.com/hyperledger-labs/fabric-token-sdk/token/services/ttx"
 	mock2 "github.com/hyperledger-labs/fabric-token-sdk/token/services/ttx/dep/mock"
-	"github.com/hyperledger-labs/fabric-token-sdk/token/services/ttx/dep/tokenapi"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
-// mockExternalWalletSigner is a mock implementation of ExternalWalletSigner
+// mockExternalWalletSigner is a mock implementation of ExternalWalletSigner for testing
 type mockExternalWalletSigner struct {
-	signCalled int
-	doneCalled int
-	signError  error
-	doneError  error
+	signFunc   func(party view.Identity, message []byte) ([]byte, error)
+	doneFunc   func() error
+	doneCalled bool
+	signCalled bool
 }
 
 func (m *mockExternalWalletSigner) Sign(party view.Identity, message []byte) ([]byte, error) {
-	m.signCalled++
-	if m.signError != nil {
-		return nil, m.signError
+	m.signCalled = true
+	if m.signFunc != nil {
+		return m.signFunc(party, message)
 	}
-	return []byte("external_signature"), nil
+	return []byte("mock_signature"), nil
 }
 
 func (m *mockExternalWalletSigner) Done() error {
-	m.doneCalled++
-	return m.doneError
-}
-
-func TestCollectEndorsementsView_ExternalWalletCleanup(t *testing.T) {
-	testCases := []struct {
-		name                    string
-		setupError              error
-		expectDoneCalled        bool
-		expectDoneCallCount     int
-		errorInSignature        bool
-		errorInTransferSignature bool
-	}{
-		{
-			name:                "success - Done called once",
-			setupError:          nil,
-			expectDoneCalled:    true,
-			expectDoneCallCount: 1,
-		},
-		{
-			name:                "error in issue signatures - Done still called",
-			setupError:          nil,
-			expectDoneCalled:    true,
-			expectDoneCallCount: 1,
-			errorInSignature:    true,
-		},
-		{
-			name:                     "error in transfer signatures - Done still called",
-			setupError:               nil,
-			expectDoneCalled:         true,
-			expectDoneCallCount:      1,
-			errorInTransferSignature: true,
-		},
+	m.doneCalled = true
+	if m.doneFunc != nil {
+		return m.doneFunc()
 	}
+	return nil
+}
 
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			// Setup mock context
-			ctx := &mock2.Context{}
-			ctx.ContextReturns(t.Context())
-
-			// Setup token management service
-			tms := &mock2.TokenManagementServiceWithExtensions{}
-			tms.NetworkReturns("test_network")
-			tms.ChannelReturns("test_channel")
-			tmsID := token.TMSID{
-				Network:   "test_network",
-				Channel:   "test_channel",
-				Namespace: "test_namespace",
-			}
-			tms.IDReturns(tmsID)
-
-			// Setup token deserializer and identity provider
-			tokenDes := &mock.Deserializer{}
-			tokenIP := &mock.IdentityProvider{}
-			tokenIP.IsMeReturns(false) // Not me, so it will try external wallet
-			tms.SigServiceReturns(token.NewSignatureService(tokenDes, tokenIP))
-
-			// Setup wallet manager with external wallet
-			walletManager := &mock2.WalletManager{}
-			wallet := &mock2.Wallet{}
-			wallet.IDReturns("external_wallet_1")
-			walletManager.OwnerWalletReturns(wallet)
-			tms.WalletManagerReturns(walletManager)
-
-			tokenAPITMS := tokenapi.NewMockedManagementService(t, tmsID)
-			tms.SetTokenManagementServiceStub = func(arg1 *token.Request) error {
-				arg1.SetTokenService(tokenAPITMS)
-				return nil
-			}
-
-			tmsp := &mock2.TokenManagementServiceProvider{}
-			tmsp.TokenManagementServiceReturns(tms, nil)
-
-			network := &mock2.Network{}
-			network.ComputeTxIDReturns("test_anchor")
-			np := &mock2.NetworkProvider{}
-			np.GetNetworkReturns(network, nil)
-
-			// Create request with issue metadata
-			req := token.NewRequest(nil, "test_anchor")
-			req.Metadata.Issues = []*driver.IssueMetadata{
-				{
-					Issuer: driver.AuditableIdentity{
-						Identity: []byte("external_issuer"),
-					},
-				},
-			}
-			tms.NewRequestReturns(req, nil)
-
-			// Setup context service calls
-			ctx.GetServiceReturnsOnCall(0, tmsp, nil)
-			ctx.GetServiceReturnsOnCall(1, np, nil)
-			ctx.GetServiceReturnsOnCall(2, &endpoint.Service{}, nil)
-			ctx.GetServiceReturnsOnCall(3, np, nil)
-			ctx.GetServiceReturnsOnCall(4, tmsp, nil)
-
-			// Create transaction
-			tx, err := ttx.NewTransaction(ctx, []byte("test_signer"))
-			require.NoError(t, err)
-
-			// Create mock external wallet signer
-			mockEWS := &mockExternalWalletSigner{}
-			if tc.errorInSignature {
-				mockEWS.signError = errors.New("signature error")
-			}
-
-			// Create CollectEndorsementsView with external wallet signer option
-			opts := []ttx.EndorsementsOpt{
-				ttx.WithExternalWalletSignerProvider(func(walletID string) ttx.ExternalWalletSigner {
-					return mockEWS
-				}),
-				ttx.WithSkipApproval(),
-				ttx.WithSkipAuditing(),
-			}
-
-			view := ttx.NewCollectEndorsementsView(tx, opts...)
-
-			// Call the view
-			_, err = view.Call(ctx)
-
-			// Verify Done was called regardless of error
-			if tc.expectDoneCalled {
-				assert.Equal(t, tc.expectDoneCallCount, mockEWS.doneCalled,
-					"Done() should be called %d time(s)", tc.expectDoneCallCount)
-			} else {
-				assert.Equal(t, 0, mockEWS.doneCalled, "Done() should not be called")
-			}
+// TestCleanupExternalWallets_Success tests that CleanupExternalWallets calls Done() on all wallets
+func TestCleanupExternalWallets_Success(t *testing.T) {
+	wallet := &mockExternalWalletSigner{}
 
-			// Verify error behavior
-			if tc.errorInSignature || tc.errorInTransferSignature {
-				require.Error(t, err, "Expected error when signature fails")
-			}
-		})
+	externalWallets := map[string]ttx.ExternalWalletSigner{
+		"wallet1": wallet,
 	}
-}
 
-func TestCollectEndorsementsView_MultipleExternalWallets(t *testing.T) {
-	// Setup mock context
+	// Create a minimal view and context
+	view := &ttx.CollectEndorsementsView{}
 	ctx := &mock2.Context{}
-	ctx.ContextReturns(t.Context())
+	ctx.ContextReturns(context.Background())
 
-	// Setup token management service
-	tms := &mock2.TokenManagementServiceWithExtensions{}
-	tms.NetworkReturns("test_network")
-	tms.ChannelReturns("test_channel")
-	tmsID := token.TMSID{
-		Network:   "test_network",
-		Channel:   "test_channel",
-		Namespace: "test_namespace",
-	}
-	tms.IDReturns(tmsID)
+	// Call the cleanup method
+	view.CleanupExternalWallets(ctx, externalWallets)
 
-	// Setup token deserializer and identity provider
-	tokenDes := &mock.Deserializer{}
-	tokenIP := &mock.IdentityProvider{}
-	tokenIP.IsMeReturns(false)
-	tms.SigServiceReturns(token.NewSignatureService(tokenDes, tokenIP))
+	assert.True(t, wallet.doneCalled, "Done() should have been called on the wallet")
+}
 
-	// Setup wallet manager with multiple external wallets
-	walletManager := &mock2.WalletManager{}
-	wallet1 := &mock2.Wallet{}
-	wallet1.IDReturns("external_wallet_1")
-	wallet2 := &mock2.Wallet{}
-	wallet2.IDReturns("external_wallet_2")
+// TestCleanupExternalWallets_MultipleWallets tests that Done() is called on all wallets
+func TestCleanupExternalWallets_MultipleWallets(t *testing.T) {
+	wallet1 := &mockExternalWalletSigner{}
+	wallet2 := &mockExternalWalletSigner{}
+	wallet3 := &mockExternalWalletSigner{}
 
-	callCount := 0
-	walletManager.OwnerWalletStub = func(ctx2 interface{}, identity view.Identity) interface{} {
-		callCount++
-		if callCount%2 == 1 {
-			return wallet1
-		}
-		return wallet2
+	externalWallets := map[string]ttx.ExternalWalletSigner{
+		"wallet1": wallet1,
+		"wallet2": wallet2,
+		"wallet3": wallet3,
 	}
-	tms.WalletManagerReturns(walletManager)
 
-	tokenAPITMS := tokenapi.NewMockedManagementService(t, tmsID)
-	tms.SetTokenManagementServiceStub = func(arg1 *token.Request) error {
-		arg1.SetTokenService(tokenAPITMS)
-		return nil
-	}
+	view := &ttx.CollectEndorsementsView{}
+	ctx := &mock2.Context{}
+	ctx.ContextReturns(context.Background())
 
-	tmsp := &mock2.TokenManagementServiceProvider{}
-	tmsp.TokenManagementServiceReturns(tms, nil)
+	view.CleanupExternalWallets(ctx, externalWallets)
 
-	network := &mock2.Network{}
-	network.ComputeTxIDReturns("test_anchor")
-	np := &mock2.NetworkProvider{}
-	np.GetNetworkReturns(network, nil)
+	assert.True(t, wallet1.doneCalled, "Done() should have been called on wallet1")
+	assert.True(t, wallet2.doneCalled, "Done() should have been called on wallet2")
+	assert.True(t, wallet3.doneCalled, "Done() should have been called on wallet3")
+}
 
-	// Create request with multiple issues
-	req := token.NewRequest(nil, "test_anchor")
-	req.Metadata.Issues = []*driver.IssueMetadata{
-		{
-			Issuer: driver.AuditableIdentity{
-				Identity: []byte("external_issuer_1"),
-			},
+// TestCleanupExternalWallets_DoneError tests that errors from Done() don't stop cleanup
+func TestCleanupExternalWallets_DoneError(t *testing.T) {
+	wallet1 := &mockExternalWalletSigner{
+		doneFunc: func() error {
+			return errors.New("wallet1 done failed")
 		},
-		{
-			Issuer: driver.AuditableIdentity{
-				Identity: []byte("external_issuer_2"),
-			},
+	}
+	wallet2 := &mockExternalWalletSigner{}
+	wallet3 := &mockExternalWalletSigner{
+		doneFunc: func() error {
+			return errors.New("wallet3 done failed")
 		},
 	}
-	tms.NewRequestReturns(req, nil)
 
-	// Setup context service calls
-	ctx.GetServiceReturnsOnCall(0, tmsp, nil)
-	ctx.GetServiceReturnsOnCall(1, np, nil)
-	ctx.GetServiceReturnsOnCall(2, &endpoint.Service{}, nil)
-	ctx.GetServiceReturnsOnCall(3, np, nil)
-	ctx.GetServiceReturnsOnCall(4, tmsp, nil)
-
-	// Create transaction
-	tx, err := ttx.NewTransaction(ctx, []byte("test_signer"))
-	require.NoError(t, err)
-
-	// Create mock external wallet signers
-	mockEWS1 := &mockExternalWalletSigner{}
-	mockEWS2 := &mockExternalWalletSigner{}
+	externalWallets := map[string]ttx.ExternalWalletSigner{
+		"wallet1": wallet1,
+		"wallet2": wallet2,
+		"wallet3": wallet3,
+	}
 
-	// Simulate error in second wallet's signature
-	mockEWS2.signError = errors.New("wallet 2 signature error")
+	view := &ttx.CollectEndorsementsView{}
+	ctx := &mock2.Context{}
+	ctx.ContextReturns(context.Background())
 
-	walletSigners := map[string]*mockExternalWalletSigner{
-		"external_wallet_1": mockEWS1,
-		"external_wallet_2": mockEWS2,
-	}
+	// Should not panic even if Done() returns errors
+	view.CleanupExternalWallets(ctx, externalWallets)
 
-	// Create CollectEndorsementsView with external wallet signer option
-	opts := []ttx.EndorsementsOpt{
-		ttx.WithExternalWalletSignerProvider(func(walletID string) ttx.ExternalWalletSigner {
-			return walletSigners[walletID]
-		}),
-		ttx.WithSkipApproval(),
-		ttx.WithSkipAuditing(),
-	}
+	assert.True(t, wallet1.doneCalled, "Done() should have been called on wallet1 despite error")
+	assert.True(t, wallet2.doneCalled, "Done() should have been called on wallet2")
+	assert.True(t, wallet3.doneCalled, "Done() should have been called on wallet3 despite error")
+}
 
-	view := ttx.NewCollectEndorsementsView(tx, opts...)
+// TestCleanupExternalWallets_EmptyMap tests cleanup with no wallets
+func TestCleanupExternalWallets_EmptyMap(t *testing.T) {
+	externalWallets := map[string]ttx.ExternalWalletSigner{}
 
-	// Call the view - should fail due to wallet 2 error
-	_, err = view.Call(ctx)
-	require.Error(t, err, "Expected error from wallet 2")
+	view := &ttx.CollectEndorsementsView{}
+	ctx := &mock2.Context{}
+	ctx.ContextReturns(context.Background())
 
-	// Verify Done was called on both wallets despite the error
-	assert.Equal(t, 1, mockEWS1.doneCalled, "Done() should be called on wallet 1")
-	assert.Equal(t, 1, mockEWS2.doneCalled, "Done() should be called on wallet 2 even after error")
+	// Should not panic with empty map
+	view.CleanupExternalWallets(ctx, externalWallets)
 }
 
-// Made with Bob