[GH-ISSUE #80] Refactor retrieval package to use dependency injection instead of package-level function vars #27

New issue

Open

opened 2026-02-28 01:16:53 +03:00 by kerem · 0 comments

kerem commented

2026-02-28 01:16:53 +03:00

Owner

Originally created by @therealpaulgg on GitHub (Feb 24, 2026).
Original GitHub issue: https://github.com/therealpaulgg/ssh-sync/issues/80

Background

The pkg/retrieval package currently uses package-level function variables to allow GetToken and RetrieveMasterKey to be swapped out in tests:

var getToken = utils.GetToken
var retrieveMasterKey = utils.RetrieveMasterKey

This was introduced to make the tests CI-compatible (previously they read directly from ~/.ssh-sync/ and required a configured environment). It works, but has drawbacks:

Global mutable state — not safe if tests ever run in parallel
Dependencies are invisible from function signatures
Requires t.Cleanup discipline to avoid test pollution

Proposed solution

Introduce a Client struct in pkg/retrieval that holds the dependencies as fields:

type Client struct {
    GetToken          func() (string, error)
    RetrieveMasterKey func() ([]byte, error)
}

func NewClient() Client {
    return Client{
        GetToken:          utils.GetToken,
        RetrieveMasterKey: utils.RetrieveMasterKey,
    }
}

func (c Client) GetUserData(profile *models.Profile) (dto.DataDto, error) { ... }
func (c Client) DeleteKey(profile *models.Profile, key dto.KeyDto) error   { ... }
func (c Client) GetMachines(profile *models.Profile) ([]dto.MachineDto, error) { ... }
func (c Client) DeleteMachine(profile *models.Profile, name string) error  { ... }

Tests construct a Client directly with stub functions — no globals, no cleanup:

c := Client{
    GetToken:          func() (string, error) { return "test-token", nil },
    RetrieveMasterKey: func() ([]byte, error) { return testKey, nil },
}

Scope

Refactor pkg/retrieval functions to methods on Client
Update all callers in pkg/actions to construct and use a Client
Remove deps.go and the package-level vars
Update tests to construct Client inline — no mocking helpers needed

Originally created by @therealpaulgg on GitHub (Feb 24, 2026). Original GitHub issue: https://github.com/therealpaulgg/ssh-sync/issues/80 ## Background The `pkg/retrieval` package currently uses package-level function variables to allow `GetToken` and `RetrieveMasterKey` to be swapped out in tests: ```go var getToken = utils.GetToken var retrieveMasterKey = utils.RetrieveMasterKey ``` This was introduced to make the tests CI-compatible (previously they read directly from `~/.ssh-sync/` and required a configured environment). It works, but has drawbacks: - Global mutable state — not safe if tests ever run in parallel - Dependencies are invisible from function signatures - Requires `t.Cleanup` discipline to avoid test pollution ## Proposed solution Introduce a `Client` struct in `pkg/retrieval` that holds the dependencies as fields: ```go type Client struct { GetToken func() (string, error) RetrieveMasterKey func() ([]byte, error) } func NewClient() Client { return Client{ GetToken: utils.GetToken, RetrieveMasterKey: utils.RetrieveMasterKey, } } func (c Client) GetUserData(profile *models.Profile) (dto.DataDto, error) { ... } func (c Client) DeleteKey(profile *models.Profile, key dto.KeyDto) error { ... } func (c Client) GetMachines(profile *models.Profile) ([]dto.MachineDto, error) { ... } func (c Client) DeleteMachine(profile *models.Profile, name string) error { ... } ``` Tests construct a `Client` directly with stub functions — no globals, no cleanup: ```go c := Client{ GetToken: func() (string, error) { return "test-token", nil }, RetrieveMasterKey: func() ([]byte, error) { return testKey, nil }, } ``` ## Scope - Refactor `pkg/retrieval` functions to methods on `Client` - Update all callers in `pkg/actions` to construct and use a `Client` - Remove `deps.go` and the package-level vars - Update tests to construct `Client` inline — no mocking helpers needed