Skip to content

Commit c9ea23a

Browse files
committed
Remove useless iot client interface (#101)
the large iot.Client interface was not a good idea: it added a level of indirection without bringing benefits this pr removes a large interface from the producer side and implement few little interfaces on the consumers when needed, following the google guidelines https://github.com/golang/go/wiki/CodeReviewComments#interfaces
1 parent e35bcd3 commit c9ea23a

File tree

11 files changed

+111
-545
lines changed

11 files changed

+111
-545
lines changed

Diff for: command/device/create.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func Create(params *CreateParams) (*DeviceInfo, error) {
8080

8181
prov := &provision{
8282
Commander: comm,
83-
Client: iotClient,
83+
cert: iotClient,
8484
board: board,
8585
id: dev.Id,
8686
}

Diff for: command/device/createlora.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ func extractEUI(port string) (string, error) {
171171
return eui, nil
172172
}
173173

174-
func getDeviceLoraInfo(iotClient iot.Client, loraDev *iotclient.ArduinoLoradevicev1) (*DeviceLoraInfo, error) {
174+
func getDeviceLoraInfo(iotClient *iot.Client, loraDev *iotclient.ArduinoLoradevicev1) (*DeviceLoraInfo, error) {
175175
dev, err := iotClient.DeviceShow(loraDev.DeviceId)
176176
if err != nil {
177177
return nil, fmt.Errorf("cannot retrieve device from the cloud: %w", err)

Diff for: command/device/provision.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ import (
2626

2727
"github.com/arduino/arduino-cloud-cli/arduino"
2828
"github.com/arduino/arduino-cloud-cli/internal/binary"
29-
"github.com/arduino/arduino-cloud-cli/internal/iot"
3029
"github.com/arduino/arduino-cloud-cli/internal/serial"
3130
"github.com/arduino/go-paths-helper"
31+
iotclient "github.com/arduino/iot-client-go"
3232
"github.com/sirupsen/logrus"
3333
)
3434

@@ -63,11 +63,15 @@ func downloadProvisioningFile(fqbn string) (string, error) {
6363
return p.String(), nil
6464
}
6565

66+
type certificateCreator interface {
67+
CertificateCreate(id, csr string) (*iotclient.ArduinoCompressedv2, error)
68+
}
69+
6670
// provision is responsible for running the provisioning
6771
// procedures for boards with crypto-chip.
6872
type provision struct {
6973
arduino.Commander
70-
iot.Client
74+
cert certificateCreator
7175
ser *serial.Serial
7276
board *board
7377
id string
@@ -125,7 +129,7 @@ func (p provision) configBoard() error {
125129
if err != nil {
126130
return err
127131
}
128-
cert, err := p.CertificateCreate(p.id, string(csr))
132+
cert, err := p.cert.CertificateCreate(p.id, string(csr))
129133
if err != nil {
130134
return err
131135
}

Diff for: command/ota/massupload.go

+14-6
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,15 @@ func MassUpload(params *MassUploadParams) ([]Result, error) {
104104
return res, nil
105105
}
106106

107-
func idsGivenTags(iotClient iot.Client, tags map[string]string) ([]string, error) {
107+
type deviceLister interface {
108+
DeviceList(tags map[string]string) ([]iotclient.ArduinoDevicev2, error)
109+
}
110+
111+
func idsGivenTags(lister deviceLister, tags map[string]string) ([]string, error) {
108112
if tags == nil {
109113
return nil, nil
110114
}
111-
devs, err := iotClient.DeviceList(tags)
115+
devs, err := lister.DeviceList(tags)
112116
if err != nil {
113117
return nil, fmt.Errorf("%s: %w", "cannot retrieve devices from cloud", err)
114118
}
@@ -119,8 +123,8 @@ func idsGivenTags(iotClient iot.Client, tags map[string]string) ([]string, error
119123
return devices, nil
120124
}
121125

122-
func validateDevices(iotClient iot.Client, ids []string, fqbn string) (valid []string, invalid []Result, err error) {
123-
devs, err := iotClient.DeviceList(nil)
126+
func validateDevices(lister deviceLister, ids []string, fqbn string) (valid []string, invalid []Result, err error) {
127+
devs, err := lister.DeviceList(nil)
124128
if err != nil {
125129
return nil, nil, fmt.Errorf("%s: %w", "cannot retrieve devices from cloud", err)
126130
}
@@ -150,7 +154,11 @@ func validateDevices(iotClient iot.Client, ids []string, fqbn string) (valid []s
150154
return valid, invalid, nil
151155
}
152156

153-
func run(iotClient iot.Client, ids []string, otaFile string, expiration int) []Result {
157+
type otaUploader interface {
158+
DeviceOTA(id string, file *os.File, expireMins int) error
159+
}
160+
161+
func run(uploader otaUploader, ids []string, otaFile string, expiration int) []Result {
154162
type job struct {
155163
id string
156164
file *os.File
@@ -174,7 +182,7 @@ func run(iotClient iot.Client, ids []string, otaFile string, expiration int) []R
174182
for i := 0; i < numConcurrentUploads; i++ {
175183
go func() {
176184
for job := range jobs {
177-
err := iotClient.DeviceOTA(job.id, job.file, expiration)
185+
err := uploader.DeviceOTA(job.id, job.file, expiration)
178186
resCh <- Result{ID: job.id, Err: err}
179187
}
180188
}()

Diff for: command/ota/massupload_test.go

+27-15
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,19 @@ import (
66
"strings"
77
"testing"
88

9-
"github.com/arduino/arduino-cloud-cli/internal/iot/mocks"
109
iotclient "github.com/arduino/iot-client-go"
11-
"github.com/stretchr/testify/mock"
1210
)
1311

1412
const testFilename = "testdata/empty.bin"
1513

14+
type deviceUploaderTest struct {
15+
deviceOTA func(id string, file *os.File, expireMins int) error
16+
}
17+
18+
func (d *deviceUploaderTest) DeviceOTA(id string, file *os.File, expireMins int) error {
19+
return d.deviceOTA(id, file, expireMins)
20+
}
21+
1622
func TestRun(t *testing.T) {
1723
var (
1824
failPrefix = "00000000"
@@ -23,14 +29,14 @@ func TestRun(t *testing.T) {
2329
okID2 = okPrefix + "-003f-42f9-a80c-85a1de36753b"
2430
okID3 = okPrefix + "-dac4-4a6a-80a4-698062fe2af5"
2531
)
26-
mockClient := &mocks.Client{}
27-
mockDeviceOTA := func(id string, file *os.File, expireMins int) error {
28-
if strings.Split(id, "-")[0] == failPrefix {
29-
return errors.New("err")
30-
}
31-
return nil
32+
mockClient := &deviceUploaderTest{
33+
deviceOTA: func(id string, file *os.File, expireMins int) error {
34+
if strings.Split(id, "-")[0] == failPrefix {
35+
return errors.New("err")
36+
}
37+
return nil
38+
},
3239
}
33-
mockClient.On("DeviceOTA", mock.Anything, mock.Anything, mock.Anything).Return(mockDeviceOTA, nil)
3440

3541
devs := []string{okID1, failID1, okID2, failID2, okID3}
3642
res := run(mockClient, devs, testFilename, 0)
@@ -49,6 +55,14 @@ func TestRun(t *testing.T) {
4955
}
5056
}
5157

58+
type deviceListerTest struct {
59+
list []iotclient.ArduinoDevicev2
60+
}
61+
62+
func (d *deviceListerTest) DeviceList(tags map[string]string) ([]iotclient.ArduinoDevicev2, error) {
63+
return d.list, nil
64+
}
65+
5266
func TestValidateDevices(t *testing.T) {
5367
var (
5468
correctFQBN = "arduino:samd:nano_33_iot"
@@ -60,23 +74,21 @@ func TestValidateDevices(t *testing.T) {
6074
idNotFound = "deb17b7f-b39d-47a2-adf3-d26cdf474707"
6175
)
6276

63-
mockClient := &mocks.Client{}
64-
mockDeviceList := func(tags map[string]string) []iotclient.ArduinoDevicev2 {
65-
return []iotclient.ArduinoDevicev2{
77+
mockDeviceList := deviceListerTest{
78+
list: []iotclient.ArduinoDevicev2{
6679
{Id: idCorrect1, Fqbn: correctFQBN},
6780
{Id: idCorrect2, Fqbn: correctFQBN},
6881
{Id: idNotValid, Fqbn: wrongFQBN},
69-
}
82+
},
7083
}
71-
mockClient.On("DeviceList", mock.Anything).Return(mockDeviceList, nil)
7284

7385
ids := []string{
7486
idCorrect1,
7587
idNotFound,
7688
idCorrect2,
7789
idNotValid,
7890
}
79-
v, i, err := validateDevices(mockClient, ids, correctFQBN)
91+
v, i, err := validateDevices(&mockDeviceList, ids, correctFQBN)
8092
if err != nil {
8193
t.Errorf("unexpected error: %s", err.Error())
8294
}

Diff for: command/thing/clone.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,12 @@ func Clone(params *CloneParams) (*ThingInfo, error) {
6161
return t, nil
6262
}
6363

64-
func retrieve(client iot.Client, thingID string) (*iotclient.ThingCreate, error) {
65-
clone, err := client.ThingShow(thingID)
64+
type thingFetcher interface {
65+
ThingShow(id string) (*iotclient.ArduinoThing, error)
66+
}
67+
68+
func retrieve(fetcher thingFetcher, thingID string) (*iotclient.ThingCreate, error) {
69+
clone, err := fetcher.ThingShow(thingID)
6670
if err != nil {
6771
return nil, fmt.Errorf("%s: %w", "retrieving the thing to be cloned", err)
6872
}

0 commit comments

Comments
 (0)