From 7f241c01f86c347ce504fa10dbeca0afdbca7516 Mon Sep 17 00:00:00 2001 From: Damian Orzepowski Date: Fri, 13 Sep 2024 09:17:59 +0200 Subject: [PATCH] refactor(SPV-1020): require cache to be not nil, small adjustments in variables names. --- engine/action_contact.go | 28 +++++++++---------- engine/paymail/paymail_service_client.go | 16 +++-------- engine/paymail/paymail_test.go | 6 ++-- engine/tester/cache.go | 10 +++++++ .../paymailmock/paymail_service_fixture.go | 4 +-- 5 files changed, 33 insertions(+), 31 deletions(-) diff --git a/engine/action_contact.go b/engine/action_contact.go index 6368ee2b..2a9939c4 100644 --- a/engine/action_contact.go +++ b/engine/action_contact.go @@ -11,32 +11,32 @@ import ( ) // UpsertContact adds a new contact if not exists or updates the existing one. -func (c *Client) UpsertContact(ctx context.Context, ctcFName, ctcPaymail, requesterXPubID, requesterPaymail string, opts ...ModelOps) (*Contact, error) { - reqPm, err := c.getPaymail(ctx, requesterXPubID, requesterPaymail) +func (c *Client) UpsertContact(ctx context.Context, ctcFName, ctcPaymail, requesterXPubID, requesterPaymailAddress string, opts ...ModelOps) (*Contact, error) { + requesterPaymail, err := c.getPaymail(ctx, requesterXPubID, requesterPaymailAddress) if err != nil { return nil, err } paymailService := c.PaymailService() - contactPm, err := paymailService.GetSanitizedPaymail(ctcPaymail) + contactPaymail, err := paymailService.GetSanitizedPaymail(ctcPaymail) if err != nil { return nil, spverrors.Wrapf(err, "requested contact paymail is invalid") } - contact, err := c.upsertContact(ctx, paymailService, requesterXPubID, ctcFName, contactPm, opts...) + contact, err := c.upsertContact(ctx, paymailService, requesterXPubID, ctcFName, contactPaymail, opts...) if err != nil { return nil, err } // request new contact requesterContactRequest := paymail.PikeContactRequestPayload{ - FullName: reqPm.PublicName, - Paymail: reqPm.String(), + FullName: requesterPaymail.PublicName, + Paymail: requesterPaymail.String(), } - if _, err = paymailService.AddContactRequest(ctx, contactPm, &requesterContactRequest); err != nil { + if _, err = paymailService.AddContactRequest(ctx, contactPaymail, &requesterContactRequest); err != nil { c.Logger().Warn(). - Str("requesterPaymail", reqPm.String()). + Str("requesterPaymail", requesterPaymail.String()). Str("requestedContact", ctcPaymail). Msgf("adding contact request failed: %s", err.Error()) @@ -50,20 +50,20 @@ func (c *Client) UpsertContact(ctx context.Context, ctcFName, ctcPaymail, reques func (c *Client) AddContactRequest(ctx context.Context, fullName, paymailAdress, requesterXPubID string, opts ...ModelOps) (*Contact, error) { paymailService := c.PaymailService() - contactPm, err := paymailService.GetSanitizedPaymail(paymailAdress) + contactPaymail, err := paymailService.GetSanitizedPaymail(paymailAdress) if err != nil { c.Logger().Error().Msgf("requested contact paymail is invalid. Reason: %s", err.Error()) return nil, spverrors.ErrRequestedContactInvalid } - contactPki, err := paymailService.GetPkiForPaymail(ctx, contactPm) + contactPki, err := paymailService.GetPkiForPaymail(ctx, contactPaymail) if err != nil { c.Logger().Error().Msgf("getting PKI for %s failed. Reason: %v", paymailAdress, err) return nil, spverrors.ErrGettingPKIFailed } // check if exists already - contact, err := getContact(ctx, contactPm.Address, requesterXPubID, c.DefaultModelOptions()...) + contact, err := getContact(ctx, contactPaymail.Address, requesterXPubID, c.DefaultModelOptions()...) if err != nil { return nil, err } @@ -74,7 +74,7 @@ func (c *Client) AddContactRequest(ctx context.Context, fullName, paymailAdress, } else { contact = newContact( fullName, - contactPm.Address, + contactPaymail.Address, contactPki.PubKey, requesterXPubID, ContactAwaitAccept, @@ -368,8 +368,8 @@ func (c *Client) getPaymail(ctx context.Context, xpubID, paymailAddr string) (*P return paymails[0], nil } -func (c *Client) upsertContact(ctx context.Context, pmSrvnt paymailclient.ServiceClient, reqXPubID, ctcFName string, ctcPaymail *paymail.SanitisedPaymail, opts ...ModelOps) (*Contact, error) { - contactPki, err := pmSrvnt.GetPkiForPaymail(ctx, ctcPaymail) +func (c *Client) upsertContact(ctx context.Context, paymailService paymailclient.ServiceClient, reqXPubID, ctcFName string, ctcPaymail *paymail.SanitisedPaymail, opts ...ModelOps) (*Contact, error) { + contactPki, err := paymailService.GetPkiForPaymail(ctx, ctcPaymail) if err != nil { return nil, spverrors.ErrGettingPKIFailed } diff --git a/engine/paymail/paymail_service_client.go b/engine/paymail/paymail_service_client.go index 69221c5b..3924014e 100644 --- a/engine/paymail/paymail_service_client.go +++ b/engine/paymail/paymail_service_client.go @@ -27,7 +27,7 @@ func NewServiceClient(cache cachestore.ClientInterface, paymailClient paymail.Cl } if cache == nil { - log.Info().Msg("Doesn't receive cachestore, won't use cache for capabilities") + panic(spverrors.Newf("cache is required to create a new paymail service")) } return &service{ @@ -60,7 +60,7 @@ func (s *service) GetCapabilities(ctx context.Context, domain string) (*paymail. return nil, err } if capabilities != nil { - return capabilities, err + return capabilities, nil } response, err := s.loadCapabilities(domain) @@ -74,10 +74,6 @@ func (s *service) GetCapabilities(ctx context.Context, domain string) (*paymail. } func (s *service) loadCapabilitiesFromCache(ctx context.Context, key string) (*paymail.CapabilitiesPayload, error) { - if s.cache == nil { - return nil, nil - } - capabilities := new(paymail.CapabilitiesPayload) err := s.cache.GetModel(ctx, key, capabilities) if errors.Is(err, cachestore.ErrKeyNotFound) { @@ -95,23 +91,19 @@ func (s *service) loadCapabilitiesFromCache(ctx context.Context, key string) (*p } func (s *service) putCapabilitiesInCache(ctx context.Context, key string, capabilities paymail.CapabilitiesPayload) { - if s.cache == nil || s.cache.Engine().IsEmpty() { - return - } - err := s.cache.SetModel(ctx, key, capabilities, cacheTTLCapabilities) if err != nil { s.log.Warn().Err(err).Msgf("failed to store capabilities for key %s in cache", key) } } -func (s *service) loadCapabilities(domain string) (response *paymail.CapabilitiesResponse, err error) { +func (s *service) loadCapabilities(domain string) (*paymail.CapabilitiesResponse, error) { // Get SRV record (domain can be different!) srv, err := s.paymailClient.GetSRVRecord( paymail.DefaultServiceName, paymail.DefaultProtocol, domain, ) if err != nil { - return + return nil, err } return s.paymailClient.GetCapabilities(srv.Target, int(srv.Port)) //nolint:wrapcheck // we have handler for paymail errors } diff --git a/engine/paymail/paymail_test.go b/engine/paymail/paymail_test.go index 44b56867..8bdc8db8 100644 --- a/engine/paymail/paymail_test.go +++ b/engine/paymail/paymail_test.go @@ -33,7 +33,7 @@ func Test_GetP2P(t *testing.T) { client := paymailmock.MockClient(testDomain) client.WillRespondWithBasicCapabilities() - paymailClient := paymailclient.NewServiceClient(nil, client, xtester.Logger()) + paymailClient := paymailclient.NewServiceClient(xtester.CacheStore(), client, xtester.Logger()) hasP2P, p2pDestinationURL, p2pSubmitTxURL, _ := paymailClient.GetP2P(context.Background(), testDomain) assert.False(t, hasP2P) @@ -45,7 +45,7 @@ func Test_GetP2P(t *testing.T) { client := paymailmock.MockClient(testDomain) client.WillRespondWithP2PCapabilities() - paymailClient := paymailclient.NewServiceClient(nil, client, xtester.Logger()) + paymailClient := paymailclient.NewServiceClient(xtester.CacheStore(), client, xtester.Logger()) hasP2P, p2pDestinationURL, p2pSubmitTxURL, format := paymailClient.GetP2P(context.Background(), testDomain) assert.True(t, hasP2P) @@ -58,7 +58,7 @@ func Test_GetP2P(t *testing.T) { client := paymailmock.MockClient(testDomain) client.WillRespondWithP2PWithBEEFCapabilities() - paymailClient := paymailclient.NewServiceClient(nil, client, xtester.Logger()) + paymailClient := paymailclient.NewServiceClient(xtester.CacheStore(), client, xtester.Logger()) hasP2P, p2pDestinationURL, p2pSubmitTxURL, format := paymailClient.GetP2P(context.Background(), testDomain) assert.True(t, hasP2P) diff --git a/engine/tester/cache.go b/engine/tester/cache.go index b1faee40..bc07d625 100644 --- a/engine/tester/cache.go +++ b/engine/tester/cache.go @@ -4,11 +4,21 @@ import ( "context" "time" + "github.com/bitcoin-sv/spv-wallet/engine/spverrors" "github.com/gomodule/redigo/redis" "github.com/mrz1836/go-cache" + "github.com/mrz1836/go-cachestore" "github.com/rafaeljusto/redigomock" ) +func CacheStore() cachestore.ClientInterface { + cacheStore, err := cachestore.NewClient(context.Background(), cachestore.WithFreeCache()) + if err != nil { + panic(spverrors.Wrapf(err, "cannot create cache store for tests")) + } + return cacheStore +} + // LoadMockRedis will load a mocked redis connection func LoadMockRedis( idleTimeout time.Duration, diff --git a/engine/tester/paymailmock/paymail_service_fixture.go b/engine/tester/paymailmock/paymail_service_fixture.go index bdd56b88..e8c4c54d 100644 --- a/engine/tester/paymailmock/paymail_service_fixture.go +++ b/engine/tester/paymailmock/paymail_service_fixture.go @@ -2,7 +2,7 @@ package paymailmock import ( "github.com/bitcoin-sv/spv-wallet/engine/paymail" - xtester "github.com/bitcoin-sv/spv-wallet/engine/tester" + "github.com/bitcoin-sv/spv-wallet/engine/tester" ) // PaymailClientServiceMock is a paymail.ServiceClient with mocked paymail.Client @@ -14,7 +14,7 @@ type PaymailClientServiceMock struct { // CreatePaymailClientService creates a new paymail.ServiceClient with mocked paymail.Client func CreatePaymailClientService(domain string, otherDomains ...string) *PaymailClientServiceMock { pmClient := MockClient(domain, otherDomains...) - client := paymail.NewServiceClient(nil, pmClient, xtester.Logger()) + client := paymail.NewServiceClient(tester.CacheStore(), pmClient, tester.Logger()) return &PaymailClientServiceMock{ ServiceClient: client,