fix(backup): return error if there is no backup path set

node/backup/controller.go

@@ -14,11 +14,17 @@ import (
 	"github.com/status-im/status-go/signal"
 )
 
+//go:generate go tool mockgen -package=mock_backup_controller -source controller.go -destination=mock/mock_backup_controller.go
+
 type BackupConfig struct {
-	PrivateKey     []byte
-	FileNameGetter func() (string, error)
-	BackupEnabled  bool
-	Interval       time.Duration
+	PrivateKey       []byte
+	FileNameProvider FilenameProvider
+	BackupEnabled    bool
+	Interval         time.Duration
+}
+
+type FilenameProvider interface {
+	GetBackupFilename() (string, error)
 }
 
 type BackupProvider interface {
@@ -52,8 +58,8 @@ func NewController(config BackupConfig, logger *zap.Logger) (*Controller, error)
 	if len(config.PrivateKey) == 0 {
 		return nil, errors.New("private key must be provided")
 	}
-	if config.FileNameGetter == nil {
-		return nil, errors.New("filename getter must be provided")
+	if common.IsNil(config.FileNameProvider) {
+		return nil, errors.New("filename provider must be provided")
 	}
 
 	return &Controller{
@@ -111,7 +117,7 @@ func (c *Controller) PerformBackup() (string, error) {
 		return "", err
 	}
 
-	fileName, err := c.config.FileNameGetter()
+	fileName, err := c.config.FileNameProvider.GetBackupFilename()
 	if err != nil {
 		return "", err
 	}

node/backup/controller_test.go

@@ -7,10 +7,13 @@ import (
 	"testing"
 
 	"github.com/brianvoe/gofakeit/v6"
+	"go.uber.org/mock/gomock"
 
 	"go.uber.org/zap"
 
 	"github.com/stretchr/testify/require"
+
+	mock_backup_controller "github.com/status-im/status-go/node/backup/mock"
 )
 
 type Foo struct {
@@ -55,9 +58,14 @@ func TestController(t *testing.T) {
 	logger, err := zap.NewDevelopment()
 	require.NoError(t, err)
 	filename := t.TempDir() + "/test_backup.bak"
+
+	ctrl := gomock.NewController(t)
+	filenameProvider := mock_backup_controller.NewMockFilenameProvider(ctrl)
+	filenameProvider.EXPECT().GetBackupFilename().Return(filename, nil).AnyTimes()
+
 	controller, err := NewController(BackupConfig{
-		FileNameGetter: func() (string, error) { return filename, nil },
-		PrivateKey:     []byte("0123456789abcdef0123456789abcdef"),
+		FileNameProvider: filenameProvider,
+		PrivateKey:       []byte("0123456789abcdef0123456789abcdef"),
 	}, logger)
 	require.NoError(t, err)
 

node/get_status_node.go

@@ -248,34 +248,11 @@ func (n *StatusNode) StartLocalBackup() error {
 
 	privateKey := chatAccount.PrivateKey()
 
-	filenameGetter := func() (string, error) {
-		backupPath, err := n.accountsSrvc.GetBackupPath()
-		if err != nil {
-			return "", err
-		}
-
-		compressedPubKey, err := utils.SerializePublicKey(crypto.CompressPubkey(&privateKey.PublicKey))
-		if err != nil {
-			return "", err
-		}
-
-		var backupDir string
-		if backupPath != "" {
-			backupDir = backupPath
-		} else {
-			backupDir = filepath.Join(n.config.RootDataDir, "backups")
-		}
-
-		fullPath := filepath.Join(backupDir, fmt.Sprintf("%s_user_data.bkp", compressedPubKey[len(compressedPubKey)-6:]))
-
-		return fullPath, nil
-	}
-
 	n.localBackup, err = backup.NewController(backup.BackupConfig{
-		PrivateKey:     crypto.Keccak256(crypto.FromECDSA(privateKey)),
-		FileNameGetter: filenameGetter,
-		BackupEnabled:  true,
-		Interval:       time.Minute * 30,
+		PrivateKey:       crypto.Keccak256(crypto.FromECDSA(privateKey)),
+		FileNameProvider: n,
+		BackupEnabled:    true,
+		Interval:         time.Minute * 30,
 	}, n.logger.Named("LocalBackup"))
 	if err != nil {
 		return err
@@ -298,6 +275,33 @@ func (n *StatusNode) StartLocalBackup() error {
 	return nil
 }
 
+func (n *StatusNode) GetBackupFilename() (string, error) {
+	chatAccount, err := n.gethAccountsManager.SelectedChatAccount()
+	if err != nil {
+		return "", err
+	}
+
+	privateKey := chatAccount.PrivateKey()
+
+	backupPath, err := n.accountsSrvc.GetBackupPath()
+	if err != nil {
+		return "", err
+	}
+
+	compressedPubKey, err := utils.SerializePublicKey(crypto.CompressPubkey(&privateKey.PublicKey))
+	if err != nil {
+		return "", err
+	}
+
+	if backupPath == "" {
+		return "", errors.New("backup path is not set")
+	}
+
+	fullPath := filepath.Join(backupPath, fmt.Sprintf("%s_user_data.bkp", compressedPubKey[len(compressedPubKey)-6:]))
+
+	return fullPath, nil
+}
+
 func (n *StatusNode) PerformLocalBackup() (string, error) {
 	return n.localBackup.PerformBackup()
 }

tests-functional/tests/test_local_backup.py

@@ -1,11 +1,13 @@
 import os
+import re
 
 import pytest
 
 from clients.status_backend import StatusBackend
 from resources.constants import Account, user_1
 from resources.test_data import profile_showcase_utils
 from utils import fake
+from clients.api import ApiResponseError
 
 test_one_to_one_chat_id = (
     "0x043329fc08727f15c4ec9a17bf7d6a3dc44e9d0d8f782a55804f3660b28827194a365dc1765cf96b6fa5cd666b9ee5298e8ac82f51f7952c4110cbf321d4f63864"
@@ -194,6 +196,11 @@ def test_local_backup(self, tmp_path):
         assert group_chat_recovered, "Group chat was not restored correctly"
         assert one_on_one_chat_recovered, "One-to-one chat was not restored correctly"
 
+        # Test that it fails if no backupPath is set
+        backend_client.settings_service.save_setting("backup-path", "")
+        with pytest.raises(ApiResponseError, match=re.escape("backup path is not set")):
+            backend_client.api_request_json("PerformLocalBackup", "")
+
         # Change the backup path (Docker restricts access to a lot of folders)
         backend_client.settings_service.save_setting("backup-path", "/usr/status-user/backups")