Skip to content

Commit bf435c6

Browse files
martacarbonemartacarbone
committed
Address review comments.
1 parent 466245c commit bf435c6

File tree

2 files changed

+128
-78
lines changed

2 files changed

+128
-78
lines changed

‎cmd/arduino-app-cli/version/version.go‎

Lines changed: 62 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,23 @@ package version
1818
import (
1919
"encoding/json"
2020
"fmt"
21-
"io"
2221
"net"
2322
"net/http"
2423
"net/url"
24+
"strings"
2525
"time"
2626

2727
"github.com/spf13/cobra"
2828

2929
"github.com/arduino/arduino-app-cli/cmd/feedback"
30-
"github.com/arduino/arduino-app-cli/cmd/i18n"
3130
)
3231

3332
// The actual listening address for the daemon
3433
// is defined in the installation package
3534
const (
3635
DefaultHostname = "localhost"
3736
DefaultPort = "8800"
37+
ProgramName = "Arduino App CLI"
3838
)
3939

4040
func NewVersionCmd(clientVersion string) *cobra.Command {
@@ -44,104 +44,103 @@ func NewVersionCmd(clientVersion string) *cobra.Command {
4444
Run: func(cmd *cobra.Command, args []string) {
4545
host, _ := cmd.Flags().GetString("host")
4646

47-
versionHandler(clientVersion, host)
47+
validatedHostAndPort, err := validateHost(host)
48+
if err != nil {
49+
feedback.Fatal("Error: invalid host:port format", feedback.ErrBadArgument)
50+
}
51+
52+
httpClient := http.Client{
53+
Timeout: time.Second,
54+
}
55+
56+
result, err := versionHandler(httpClient, clientVersion, validatedHostAndPort)
57+
if err != nil {
58+
feedback.Warnf("Waning: " + err.Error() + "\n")
59+
}
60+
feedback.PrintResult(result)
4861
},
4962
}
5063
cmd.Flags().String("host", fmt.Sprintf("%s:%s", DefaultHostname, DefaultPort),
5164
"The daemon network address [host]:[port]")
5265
return cmd
5366
}
5467

55-
func versionHandler(clientVersion string, host string) {
56-
httpClient := http.Client{
57-
Timeout: time.Second,
68+
func versionHandler(httpClient http.Client, clientVersion string, hostAndPort string) (versionResult, error) {
69+
url := url.URL{
70+
Scheme: "http",
71+
Host: hostAndPort,
72+
Path: "/v1/version",
5873
}
59-
result := doVersionHandler(httpClient, clientVersion, host)
60-
feedback.PrintResult(result)
61-
}
6274

63-
func doVersionHandler(httpClient http.Client, clientVersion string, host string) versionResult {
64-
url, err := getValidOrDefaultUrl(host)
65-
if err != nil {
66-
feedback.Fatal(i18n.Tr("Error: invalid host:port format"), feedback.ErrBadArgument)
67-
}
75+
daemonVersion := getServerVersion(httpClient, url.String())
6876

69-
serverVersion, err := getServerVersion(httpClient, url)
70-
if err != nil {
71-
serverVersion = fmt.Sprintf("n/a (cannot connect to the server %s://%s)", url.Scheme, url.Host)
77+
result := versionResult{
78+
Name: ProgramName,
79+
ClientVersion: clientVersion,
80+
DaemonVersion: daemonVersion,
7281
}
7382

74-
return versionResult{
75-
ClientVersion: clientVersion,
76-
ServerVersion: serverVersion,
83+
if daemonVersion == "" {
84+
return result, fmt.Errorf("cannot connect to %s", hostAndPort)
7785
}
86+
return result, nil
7887
}
7988

80-
func getValidOrDefaultUrl(hostPort string) (url.URL, error) {
81-
host := DefaultHostname
82-
port := DefaultPort
83-
84-
if hostPort != "" {
85-
h, p, err := net.SplitHostPort(hostPort)
86-
if err != nil {
87-
return url.URL{}, err
88-
}
89-
if h != "" {
90-
host = h
91-
}
92-
if p != "" {
93-
port = p
94-
}
95-
89+
func validateHost(hostPort string) (string, error) {
90+
if !strings.Contains(hostPort, ":") {
91+
hostPort = hostPort + ":"
9692
}
9793

98-
hostAndPort := net.JoinHostPort(host, port)
99-
100-
u := url.URL{
101-
Scheme: "http",
102-
Host: hostAndPort,
103-
Path: "/v1/version",
94+
h, p, err := net.SplitHostPort(hostPort)
95+
if err != nil {
96+
return "", err
97+
}
98+
if h == "" {
99+
h = DefaultHostname
100+
}
101+
if p == "" {
102+
p = DefaultPort
104103
}
105104

106-
return u, nil
105+
return net.JoinHostPort(h, p), nil
107106
}
108107

109-
func getServerVersion(httpClient http.Client, url url.URL) (string, error) {
110-
resp, err := httpClient.Get(url.String())
108+
func getServerVersion(httpClient http.Client, url string) string {
109+
resp, err := httpClient.Get(url)
111110
if err != nil {
112-
return "", err
111+
return ""
113112
}
114113
defer resp.Body.Close()
115114

116115
if resp.StatusCode != http.StatusOK {
117-
return "", fmt.Errorf("request failed with status %d", resp.StatusCode)
116+
return ""
118117
}
119118

120-
body, err := io.ReadAll(resp.Body)
121-
if err != nil {
122-
return "", err
119+
var serverResponse struct {
120+
Version string `json:"version"`
123121
}
124-
125-
var serverResponse serverVersionResponse
126-
if err := json.Unmarshal(body, &serverResponse); err != nil {
127-
return "", err
122+
if err := json.NewDecoder(resp.Body).Decode(&serverResponse); err != nil {
123+
return ""
128124
}
129125

130-
return serverResponse.Version, nil
131-
}
132-
133-
type serverVersionResponse struct {
134-
Version string `json:"version"`
126+
return serverResponse.Version
135127
}
136128

137129
type versionResult struct {
138-
ClientVersion string `json:"version"`
139-
ServerVersion string `json:"serverVersion"`
130+
Name string `json:"name"`
131+
ClientVersion string `json:"client_version"`
132+
DaemonVersion string `json:"daemon_version,omitempty"`
140133
}
141134

142135
func (r versionResult) String() string {
143-
return fmt.Sprintf("client: %s\nserver: %s",
144-
r.ClientVersion, r.ServerVersion)
136+
serverMessage := fmt.Sprintf("%s client version %s",
137+
ProgramName, r.ClientVersion)
138+
139+
if r.DaemonVersion != "" {
140+
serverMessage = fmt.Sprintf("%s\ndaemon version: %s",
141+
serverMessage, r.DaemonVersion)
142+
}
143+
return serverMessage
145144
}
146145

147146
func (r versionResult) Data() interface{} {

‎cmd/arduino-app-cli/version/version_test.go‎

Lines changed: 66 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ package version
1717

1818
import (
1919
"errors"
20-
"fmt"
2120
"io"
2221
"net/http"
2322
"strings"
@@ -26,50 +25,102 @@ import (
2625
"github.com/stretchr/testify/require"
2726
)
2827

28+
func TestGetValidUrl(t *testing.T) {
29+
testCases := []struct {
30+
name string
31+
hostPort string
32+
expectedResult string
33+
}{
34+
{
35+
name: "Valid host and port should return default.",
36+
hostPort: "localhost:8800",
37+
expectedResult: "localhost:8800",
38+
},
39+
{
40+
name: "Missing host should return default host.",
41+
hostPort: ":8800",
42+
expectedResult: "localhost:8800",
43+
},
44+
{
45+
name: "Missing port should return default port.",
46+
hostPort: "localhost:",
47+
expectedResult: "localhost:8800",
48+
},
49+
{
50+
name: "Custom host and port should return the default.",
51+
hostPort: "192.168.100.1:1234",
52+
expectedResult: "192.168.100.1:1234",
53+
},
54+
{
55+
name: "Host only should return provided input and default port.",
56+
hostPort: "192.168.1.1",
57+
expectedResult: "192.168.1.1:8800",
58+
},
59+
{
60+
name: "Missing host and port should return default.",
61+
hostPort: "",
62+
expectedResult: "localhost:8800",
63+
},
64+
}
65+
66+
for _, tc := range testCases {
67+
t.Run(tc.name, func(t *testing.T) {
68+
url, _ := validateHost(tc.hostPort)
69+
require.Equal(t, tc.expectedResult, url)
70+
})
71+
}
72+
}
73+
2974
func TestServerVersion(t *testing.T) {
3075
clientVersion := "5.1-dev"
76+
unreacheableUrl := "unreacheable:123"
77+
daemonVersion := ""
3178

3279
testCases := []struct {
3380
name string
3481
serverStub Tripper
3582
expectedResult versionResult
36-
host string
83+
hostAndPort string
3784
}{
3885
{
3986
name: "return the server version when the server is up",
4087
serverStub: successServer,
4188
expectedResult: versionResult{
42-
ClientVersion: "5.1-dev",
43-
ServerVersion: "3.0",
89+
Name: ProgramName,
90+
ClientVersion: clientVersion,
91+
DaemonVersion: "3.0",
4492
},
45-
host: "",
93+
hostAndPort: "localhost:8800",
4694
},
4795
{
4896
name: "return error if default server is not listening",
4997
serverStub: failureServer,
5098
expectedResult: versionResult{
51-
ClientVersion: "5.1-dev",
52-
ServerVersion: fmt.Sprintf("n/a (cannot connect to the server http://%s:%s)", DefaultHostname, DefaultPort),
99+
Name: ProgramName,
100+
ClientVersion: clientVersion,
101+
DaemonVersion: daemonVersion,
53102
},
54-
host: "",
103+
hostAndPort: unreacheableUrl,
55104
},
56105
{
57106
name: "return error if provided server is not listening",
58107
serverStub: failureServer,
59108
expectedResult: versionResult{
60-
ClientVersion: "5.1-dev",
61-
ServerVersion: "n/a (cannot connect to the server http://unreacheable:123)",
109+
Name: ProgramName,
110+
ClientVersion: clientVersion,
111+
DaemonVersion: daemonVersion,
62112
},
63-
host: "unreacheable:123",
113+
hostAndPort: unreacheableUrl,
64114
},
65115
{
66116
name: "return error for server resopnse 500 Internal Server Error",
67117
serverStub: failureInternalServerError,
68118
expectedResult: versionResult{
69-
ClientVersion: "5.1-dev",
70-
ServerVersion: "n/a (cannot connect to the server http://unreacheable:123)",
119+
Name: ProgramName,
120+
ClientVersion: clientVersion,
121+
DaemonVersion: daemonVersion,
71122
},
72-
host: "unreacheable:123",
123+
hostAndPort: unreacheableUrl,
73124
},
74125
}
75126
for _, tc := range testCases {
@@ -79,7 +130,7 @@ func TestServerVersion(t *testing.T) {
79130
httpClient.Transport = tc.serverStub
80131

81132
// act
82-
result := doVersionHandler(httpClient, clientVersion, tc.host)
133+
result, _ := versionHandler(httpClient, clientVersion, tc.hostAndPort)
83134

84135
// assert
85136
require.Equal(t, tc.expectedResult, result)

0 commit comments

Comments
 (0)