-
Notifications
You must be signed in to change notification settings - Fork 0
Review 2 #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Review 2 #2
Changes from all commits
787d36b
984c1ae
84a1840
544598f
7eeb249
2e1866b
af6213f
a4eff38
d85e17d
fea6934
0db88d0
93dc4b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| .PHONY: run | ||
| run: ## Run the app from source code | ||
| go run main.go | ||
|
|
||
| .PHONY: docker | ||
| docker: ## Run the app in a container built from docker file | ||
| docker compose up app_docker | ||
|
|
||
| .PHONY: local | ||
| local: ## Run the app in a container using source code | ||
| docker compose up app_local | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,64 +4,67 @@ import ( | |
| "bytes" | ||
| "context" | ||
| "encoding/json" | ||
| "errors" | ||
| "fmt" | ||
| "net/http" | ||
| "net/url" | ||
| "time" | ||
|
|
||
| "github.com/NickGowdy/deveui-cli/device" | ||
| ) | ||
|
|
||
| // Client used to communicate to external services | ||
| type Client interface { | ||
| Do(*http.Request) (resp *http.Response, err error) | ||
| } | ||
|
|
||
| // LoraWAN used to communicate to LoRaWAN external system | ||
| type LoraWAN struct { | ||
| baseURL string | ||
| client Client | ||
| } | ||
|
|
||
| func NewLoraWAN(baseURL string, client Client) *LoraWAN { | ||
| return &LoraWAN{ | ||
| baseURL: baseURL, | ||
| client: client, | ||
| } | ||
| fullURL *url.URL | ||
| client *http.Client | ||
| } | ||
|
|
||
| const endpoint = "/sensor-onboarding-sample" // endpoint for saving DevEUI via LoRaWAN | ||
|
|
||
| // RegisterDevice registers new device using LoraWAN external service | ||
| func (l *LoraWAN) RegisterDevice(ctx context.Context) (*device.Device, error) { | ||
| device := device.NewDevice() | ||
| identifier := device.GetIdentifier() | ||
| b := new(bytes.Buffer) | ||
| reqBody := map[string]string{"Deveui": identifier} | ||
|
|
||
| err := json.NewEncoder(b).Encode(&reqBody) | ||
| func NewLoraWAN(baseURL string, timeout time.Duration) (*LoraWAN, error) { | ||
| path, err := url.JoinPath(baseURL, endpoint) | ||
| if err != nil { | ||
| return nil, err | ||
| return nil, fmt.Errorf("failed to join url: %w", err) | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrapping errors can help to identify where an error came from, especially when multiple functions can return the same error message. The https://github.com/pkg/errors package can give you stack trace along with the error, and their .Wrap method is convenient to use, but I try not to use this package for new projects anymore as per the description in the repo roadmap. |
||
| } | ||
|
|
||
| fullUrl := l.baseURL + endpoint | ||
| req, err := http.NewRequestWithContext(ctx, "POST", fullUrl, b) | ||
| fullURL, err := url.Parse(path) | ||
| if err != nil { | ||
| return nil, err | ||
| return nil, fmt.Errorf("failed to parse url: %w", err) | ||
| } | ||
|
|
||
| resp, err := l.client.Do(req) | ||
| return &LoraWAN{ | ||
| fullURL: fullURL, | ||
| client: &http.Client{ | ||
| Timeout: timeout, | ||
| }, | ||
| }, nil | ||
| } | ||
|
|
||
| // RegisterDevice registers a new device using LoraWAN external service | ||
| func (l *LoraWAN) RegisterDevice(ctx context.Context, newDevice *device.Device) error { | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having the device as a parameter makes this better, because
|
||
| reqBody := map[string]string{"deveui": newDevice.GetIdentifier()} | ||
|
|
||
| b := new(bytes.Buffer) | ||
| if err := json.NewEncoder(b).Encode(&reqBody); err != nil { | ||
| return fmt.Errorf("failed to encode request body: %w", err) | ||
| } | ||
|
|
||
| req, err := http.NewRequestWithContext(ctx, "POST", l.fullURL.String(), b) | ||
| if err != nil { | ||
| return nil, err | ||
| return fmt.Errorf("failed to create request: %w", err) | ||
| } | ||
|
|
||
| req.Header.Set("Content-Type", "application/json") | ||
|
|
||
| resp, err := l.client.Do(req) | ||
| if err != nil { | ||
| return nil, err | ||
| return fmt.Errorf("failed to send request: %w", err) | ||
| } | ||
|
|
||
| defer resp.Body.Close() | ||
|
|
||
| if resp.StatusCode == http.StatusOK { | ||
| return device, nil | ||
| } else { | ||
| return nil, errors.New(resp.Status) | ||
| if resp.StatusCode < 200 || resp.StatusCode > 299 { | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to make it a bit more robust. |
||
| return fmt.Errorf("request failed: %s", resp.Status) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,95 +1,177 @@ | ||
| package client | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "fmt" | ||
| "io" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "net/url" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/go-chi/chi/v5" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
|
|
||
| "github.com/NickGowdy/deveui-cli/device" | ||
| ) | ||
|
|
||
| type MockClient struct { | ||
| DoFunc func(*http.Request) (resp *http.Response, err error) | ||
| func TestNewLoraWAN(t *testing.T) { | ||
| t.Parallel() | ||
| type args struct { | ||
| baseURL string | ||
| timeout time.Duration | ||
| } | ||
| tests := []struct { | ||
| name string | ||
| args args | ||
| want *LoraWAN | ||
| wantErr assert.ErrorAssertionFunc | ||
| }{ | ||
| { | ||
| name: "invalid-base-url", | ||
| args: args{ | ||
| baseURL: ":invalid", | ||
| }, | ||
| want: nil, | ||
| wantErr: func(t assert.TestingT, err error, i ...interface{}) bool { | ||
| return assert.ErrorContains(t, err, "failed to join url") | ||
| }, | ||
| }, | ||
| { | ||
| name: "ok", | ||
| args: args{ | ||
| baseURL: "base", | ||
| timeout: 1 * time.Second, | ||
| }, | ||
| want: &LoraWAN{ | ||
| fullURL: func() *url.URL { | ||
| u, err := url.Parse(fmt.Sprintf("%s%s", "base", endpoint)) | ||
| require.NoError(t, err) | ||
|
|
||
| return u | ||
| }(), | ||
| client: &http.Client{ | ||
| Timeout: 1 * time.Second, | ||
| }, | ||
| }, | ||
| wantErr: func(t assert.TestingT, err error, i ...interface{}) bool { | ||
| return assert.NoError(t, err) | ||
| }, | ||
| }, | ||
| } | ||
| for _, tt := range tests { | ||
| tt := tt | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
| got, err := NewLoraWAN(tt.args.baseURL, tt.args.timeout) | ||
| if !tt.wantErr(t, err, fmt.Sprintf("NewLoraWAN(%v, %v)", tt.args.baseURL, tt.args.timeout)) { | ||
| return | ||
| } | ||
| assert.EqualValues(t, tt.want, got) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestLoraWAN_RegisterDevice_Request(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| const timeout = 10 * time.Second | ||
|
|
||
| t.Run("server-side-checks", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| newDevice, err := device.NewDevice() | ||
| require.NoError(t, err) | ||
|
|
||
| ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| assert.Equal(t, http.MethodPost, r.Method) | ||
| assert.Equal(t, endpoint, r.URL.Path) | ||
| assert.Equal(t, "application/json", r.Header.Get("Content-Type")) | ||
|
|
||
| data, err := io.ReadAll(r.Body) | ||
| defer r.Body.Close() | ||
| require.NoError(t, err) | ||
| var body struct { | ||
| Deveui string `json:"deveui"` | ||
| } | ||
| err = json.Unmarshal(data, &body) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, newDevice.GetIdentifier(), body.Deveui) | ||
|
|
||
| w.WriteHeader(http.StatusCreated) | ||
| })) | ||
| defer ts.Close() | ||
|
|
||
| client, err := NewLoraWAN(ts.URL, timeout) | ||
| require.NoError(t, err) | ||
|
|
||
| err = client.RegisterDevice(context.Background(), newDevice) | ||
| assert.NoError(t, err) | ||
| }) | ||
| } | ||
|
|
||
| // func TestLorawanClientHappyPath(t *testing.T) { | ||
| // mockClient := &MockClient{ | ||
| // DoFunc: func(*http.Request) (resp *http.Response, err error) { | ||
| // return &http.Response{}, nil | ||
| // }, | ||
| // } | ||
|
|
||
| // loraWAN := NewLoraWAN("www.example.com", mockClient) | ||
|
|
||
| // b := new(bytes.Buffer) | ||
| // reqBody := map[string]string{"Deveui": "Abcde"} | ||
|
|
||
| // _ = json.NewEncoder(b).Encode(&reqBody) | ||
|
|
||
| // ctx, cancel := context.WithCancel(context.Background()) | ||
|
|
||
| // if cancel == nil { | ||
| // t.Errorf("cancel should not be nil but is: %v", cancel) | ||
| // } | ||
|
|
||
| // resp, err := loraWAN.DoPost(b, ctx) | ||
|
|
||
| // if err != nil { | ||
| // t.Errorf("err should be nil but is: %s", err.Error()) | ||
| // } | ||
| // defer resp.Body.Close() | ||
|
|
||
| // if resp.StatusCode != 200 { | ||
| // t.Errorf("resp should be nil but is: %d", resp.StatusCode) | ||
| // } | ||
|
|
||
| // body, _ := io.ReadAll(resp.Body) | ||
| // val := string(body) | ||
|
|
||
| // if strings.TrimSpace(val) != "true" { | ||
| // t.Errorf("body should equal true but is: %d", body) | ||
| // } | ||
| // } | ||
|
|
||
| // func TestNewLoraWanClient(t *testing.T) { | ||
| // client := &http.Client{ | ||
| // Timeout: 30 * time.Second, | ||
| // } | ||
| // t.Parallel() | ||
| // type args struct { | ||
| // timeout time.Duration | ||
| // } | ||
| // tests := []struct { | ||
| // name string | ||
| // args args | ||
| // want *LoraWAN | ||
| // }{ | ||
| // { | ||
| // name: "create-new-lorawan-client", | ||
| // args: args{ | ||
| // timeout: 30, | ||
| // }, | ||
| // want: &LoraWAN{ | ||
| // baseURL: "https://www.example.com", | ||
| // client: client, | ||
| // }, | ||
| // }, | ||
| // } | ||
| // for _, tt := range tests { | ||
| // tt := tt // it is important to capture range variable | ||
| // t.Run(tt.name, func(t *testing.T) { | ||
| // t.Parallel() // this makes sure that all cases from the table here are executed in parallel | ||
| // if got := NewLoraWAN("https://www.example.com", client); !reflect.DeepEqual(got, tt.want) { | ||
| // t.Errorf("NewLoraWanClient() = %v, want %v", got, tt.want) | ||
| // } | ||
| // }) | ||
| // } | ||
| // } | ||
|
|
||
| // func (m *MockClient) Do(*http.Request) (resp *http.Response, err error) { | ||
| // b := new(bytes.Buffer) | ||
| // reqBody := true | ||
|
|
||
| // _ = json.NewEncoder(b).Encode(&reqBody) | ||
| // return &http.Response{ | ||
| // StatusCode: http.StatusOK, | ||
| // Body: io.NopCloser(b), | ||
| // Status: "200 OK"}, | ||
| // nil | ||
| // } | ||
| func TestLoraWAN_RegisterDevice(t *testing.T) { | ||
| t.Parallel() | ||
| type fields struct { | ||
| handler func(w http.ResponseWriter, r *http.Request) | ||
| } | ||
| tests := []struct { | ||
| name string | ||
| fields fields | ||
| want *device.Device | ||
| wantErr assert.ErrorAssertionFunc | ||
| }{ | ||
| { | ||
| name: "error-status-code", | ||
| fields: fields{ | ||
| handler: func(w http.ResponseWriter, r *http.Request) { | ||
| w.WriteHeader(http.StatusUnprocessableEntity) | ||
| }, | ||
| }, | ||
| want: nil, | ||
| wantErr: func(t assert.TestingT, err error, i ...interface{}) bool { | ||
| return assert.ErrorContains(t, err, "request failed") | ||
| }, | ||
| }, | ||
| { | ||
| name: "ok-status-code", | ||
| fields: fields{ | ||
| handler: func(w http.ResponseWriter, r *http.Request) { | ||
| w.WriteHeader(http.StatusOK) | ||
| }, | ||
| }, | ||
| want: &device.Device{}, | ||
| wantErr: func(t assert.TestingT, err error, i ...interface{}) bool { | ||
| return assert.NoError(t, err) | ||
| }, | ||
| }, | ||
| } | ||
| for _, tt := range tests { | ||
| tt := tt | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| r := chi.NewRouter() | ||
| r.Post(endpoint, tt.fields.handler) | ||
| ts := httptest.NewServer(r) | ||
| defer ts.Close() | ||
|
|
||
| tsPath, err := url.JoinPath(ts.URL, endpoint) | ||
| require.NoError(t, err) | ||
|
|
||
| tsURL, err := url.Parse(tsPath) | ||
| require.NoError(t, err) | ||
|
|
||
| l := &LoraWAN{ | ||
| fullURL: tsURL, | ||
| client: ts.Client(), | ||
| } | ||
|
|
||
| tt.wantErr(t, l.RegisterDevice(context.Background(), &device.Device{}), "RegisterDevice()") | ||
| }) | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A makefile can help up local development, it's like adding command aliases.