From f8fc98d684d431378b2d23f04442b41cf18fd0e5 Mon Sep 17 00:00:00 2001 From: Joshua Rich Date: Fri, 12 Jan 2024 11:51:50 +1000 Subject: [PATCH] refactor(all): :recycle: registration code clean-up remove unneeded interfaces; simplify registration logic --- internal/agent/register.go | 26 +++-- internal/agent/ui/fyneUI/fyneUI.go | 10 +- internal/hass/api/config.go | 11 --- internal/hass/api/mock_AgentConfig_test.go | 80 --------------- internal/hass/api/mock_Agent_test.go | 80 --------------- internal/hass/api/mock_DeviceInfo_test.go | 6 +- internal/hass/api/registration.go | 25 ++--- internal/hass/api/registration_test.go | 107 ++++++++------------- 8 files changed, 67 insertions(+), 278 deletions(-) delete mode 100644 internal/hass/api/config.go delete mode 100644 internal/hass/api/mock_AgentConfig_test.go delete mode 100644 internal/hass/api/mock_Agent_test.go diff --git a/internal/agent/register.go b/internal/agent/register.go index bafc1feee..2a351aaf5 100644 --- a/internal/agent/register.go +++ b/internal/agent/register.go @@ -55,19 +55,14 @@ func saveRegistration(cfg config.Config, r *api.RegistrationResponse, d api.Devi // registration details and save the results into the agent config. func (agent *Agent) performRegistration(ctx context.Context, cfg config.Config) { log.Info().Msg("Registration required. Starting registration process.") - if agent.options.Server != "" { - if !validateRegistrationSetting("server", agent.options.Server) { - log.Fatal().Msg("Server setting is not valid.") - } else if err := cfg.Set(config.PrefHost, agent.options.Server); err != nil { - log.Fatal().Err(err).Msg("Could not set host preference.") - } + if !validRegistrationSetting("server", agent.options.Server) || !validRegistrationSetting("token", agent.options.Token) { + log.Fatal().Msg("Cannot register, invalid host and/or token.") } - if agent.options.Token != "" { - if !validateRegistrationSetting("token", agent.options.Token) { - log.Fatal().Msg("Token setting is not valid.") - } else if err := cfg.Set(config.PrefToken, agent.options.Token); err != nil { - log.Fatal().Err(err).Msg("Could not set token preference.") - } + if err := cfg.Set(config.PrefHost, agent.options.Server); err != nil { + log.Fatal().Err(err).Msg("Could not set host preference.") + } + if err := cfg.Set(config.PrefToken, agent.options.Token); err != nil { + log.Fatal().Err(err).Msg("Could not set token preference.") } device := newDevice(ctx) @@ -76,7 +71,7 @@ func (agent *Agent) performRegistration(ctx context.Context, cfg config.Config) agent.ui.DisplayRegistrationWindow(ctx, agent, cfg, userInputDone) <-userInputDone } - resp, err := api.RegisterWithHass(ctx, cfg, device) + resp, err := api.RegisterWithHass(ctx, agent.options.Server, agent.options.Token, device) if err != nil { log.Fatal().Err(err).Msg("Could not register with Home Assistant.") } @@ -104,7 +99,10 @@ func (agent *Agent) checkRegistration(t *tracker.SensorTracker, c config.Config) } } -func validateRegistrationSetting(key, value string) bool { +func validRegistrationSetting(key, value string) bool { + if value == "" { + return false + } validate := validator.New() check := func(value string, validation string) bool { if err := validate.Var(value, validation); err != nil { diff --git a/internal/agent/ui/fyneUI/fyneUI.go b/internal/agent/ui/fyneUI/fyneUI.go index b54ab16c8..aceeb049e 100644 --- a/internal/agent/ui/fyneUI/fyneUI.go +++ b/internal/agent/ui/fyneUI/fyneUI.go @@ -142,7 +142,7 @@ func (i *fyneUI) DisplayRegistrationWindow(ctx context.Context, agent ui.Agent, var allFormItems []*widget.FormItem - allFormItems = append(allFormItems, i.serverConfigItems(ctx, agent, cfg, i.text)...) + allFormItems = append(allFormItems, serverConfigItems(ctx, agent, cfg, i.text)...) registrationForm := widget.NewForm(allFormItems...) registrationForm.OnSubmit = func() { w.Close() @@ -297,13 +297,13 @@ func (i *fyneUI) agentSettingsWindow(agent ui.Agent, t *translations.Translator) // serverConfigItems generates a list of form item widgets for selecting a // server to register the agent against. -func (i *fyneUI) serverConfigItems(ctx context.Context, agent ui.Agent, cfg config.Config, t *translations.Translator) []*widget.FormItem { +func serverConfigItems(ctx context.Context, agent ui.Agent, cfg config.Config, t *translations.Translator) []*widget.FormItem { allServers := hass.FindServers(ctx) - tokenEntry := configEntry(agent, cfg, config.PrefToken, "ASecretLongLivedToken", false) + tokenEntry := configEntry(cfg, config.PrefToken, "ASecretLongLivedToken", false) tokenEntry.Validator = validation.NewRegexp("[A-Za-z0-9_\\.]+", "Invalid token format") - serverEntry := configEntry(agent, cfg, config.PrefHost, allServers[0], false) + serverEntry := configEntry(cfg, config.PrefHost, allServers[0], false) serverEntry.Validator = httpValidator() serverEntry.Disable() @@ -378,7 +378,7 @@ func (i *fyneUI) serverConfigItems(ctx context.Context, agent ui.Agent, cfg conf // configEntry creates a form entry widget that is tied to the given config // value of the given agent. When the value of the entry widget changes, the // corresponding config value will be updated. -func configEntry(agent ui.Agent, cfg config.Config, name, placeholder string, secret bool) *widget.Entry { +func configEntry(cfg config.Config, name, placeholder string, secret bool) *widget.Entry { var entry *widget.Entry if secret { entry = widget.NewPasswordEntry() diff --git a/internal/hass/api/config.go b/internal/hass/api/config.go deleted file mode 100644 index dc535fc95..000000000 --- a/internal/hass/api/config.go +++ /dev/null @@ -1,11 +0,0 @@ -// Copyright (c) 2023 Joshua Rich -// -// This software is released under the MIT License. -// https://opensource.org/licenses/MIT - -package api - -//go:generate moq -out mock_Agent_test.go . Agent -type Agent interface { - GetConfig(string, any) error -} diff --git a/internal/hass/api/mock_AgentConfig_test.go b/internal/hass/api/mock_AgentConfig_test.go deleted file mode 100644 index b6a6647d4..000000000 --- a/internal/hass/api/mock_AgentConfig_test.go +++ /dev/null @@ -1,80 +0,0 @@ -// Code generated by moq; DO NOT EDIT. -// github.com/matryer/moq - -package api - -import ( - "sync" -) - -// Ensure, that AgentConfigMock does implement AgentConfig. -// If this is not the case, regenerate this file with moq. -var _ Agent = &AgentConfigMock{} - -// AgentConfigMock is a mock implementation of AgentConfig. -// -// func TestSomethingThatUsesAgentConfig(t *testing.T) { -// -// // make and configure a mocked AgentConfig -// mockedAgentConfig := &AgentConfigMock{ -// GetFunc: func(s string, ifaceVal interface{}) error { -// panic("mock out the Get method") -// }, -// } -// -// // use mockedAgentConfig in code that requires AgentConfig -// // and then make assertions. -// -// } -type AgentConfigMock struct { - // GetFunc mocks the Get method. - GetFunc func(s string, ifaceVal interface{}) error - - // calls tracks calls to the methods. - calls struct { - // Get holds details about calls to the Get method. - Get []struct { - // S is the s argument value. - S string - // IfaceVal is the ifaceVal argument value. - IfaceVal interface{} - } - } - lockGet sync.RWMutex -} - -// GetConfig calls GetFunc. -func (mock *AgentConfigMock) GetConfig(s string, ifaceVal interface{}) error { - if mock.GetFunc == nil { - panic("AgentConfigMock.GetFunc: method is nil but AgentConfig.Get was just called") - } - callInfo := struct { - S string - IfaceVal interface{} - }{ - S: s, - IfaceVal: ifaceVal, - } - mock.lockGet.Lock() - mock.calls.Get = append(mock.calls.Get, callInfo) - mock.lockGet.Unlock() - return mock.GetFunc(s, ifaceVal) -} - -// GetCalls gets all the calls that were made to Get. -// Check the length with: -// -// len(mockedAgentConfig.GetCalls()) -func (mock *AgentConfigMock) GetCalls() []struct { - S string - IfaceVal interface{} -} { - var calls []struct { - S string - IfaceVal interface{} - } - mock.lockGet.RLock() - calls = mock.calls.Get - mock.lockGet.RUnlock() - return calls -} diff --git a/internal/hass/api/mock_Agent_test.go b/internal/hass/api/mock_Agent_test.go deleted file mode 100644 index d9ec9f798..000000000 --- a/internal/hass/api/mock_Agent_test.go +++ /dev/null @@ -1,80 +0,0 @@ -// Code generated by moq; DO NOT EDIT. -// github.com/matryer/moq - -package api - -import ( - "sync" -) - -// Ensure, that AgentMock does implement Agent. -// If this is not the case, regenerate this file with moq. -var _ Agent = &AgentMock{} - -// AgentMock is a mock implementation of Agent. -// -// func TestSomethingThatUsesAgent(t *testing.T) { -// -// // make and configure a mocked Agent -// mockedAgent := &AgentMock{ -// GetConfigFunc: func(s string, ifaceVal interface{}) error { -// panic("mock out the GetConfig method") -// }, -// } -// -// // use mockedAgent in code that requires Agent -// // and then make assertions. -// -// } -type AgentMock struct { - // GetConfigFunc mocks the GetConfig method. - GetConfigFunc func(s string, ifaceVal interface{}) error - - // calls tracks calls to the methods. - calls struct { - // GetConfig holds details about calls to the GetConfig method. - GetConfig []struct { - // S is the s argument value. - S string - // IfaceVal is the ifaceVal argument value. - IfaceVal interface{} - } - } - lockGetConfig sync.RWMutex -} - -// GetConfig calls GetConfigFunc. -func (mock *AgentMock) GetConfig(s string, ifaceVal interface{}) error { - if mock.GetConfigFunc == nil { - panic("AgentMock.GetConfigFunc: method is nil but Agent.GetConfig was just called") - } - callInfo := struct { - S string - IfaceVal interface{} - }{ - S: s, - IfaceVal: ifaceVal, - } - mock.lockGetConfig.Lock() - mock.calls.GetConfig = append(mock.calls.GetConfig, callInfo) - mock.lockGetConfig.Unlock() - return mock.GetConfigFunc(s, ifaceVal) -} - -// GetConfigCalls gets all the calls that were made to GetConfig. -// Check the length with: -// -// len(mockedAgent.GetConfigCalls()) -func (mock *AgentMock) GetConfigCalls() []struct { - S string - IfaceVal interface{} -} { - var calls []struct { - S string - IfaceVal interface{} - } - mock.lockGetConfig.RLock() - calls = mock.calls.GetConfig - mock.lockGetConfig.RUnlock() - return calls -} diff --git a/internal/hass/api/mock_DeviceInfo_test.go b/internal/hass/api/mock_DeviceInfo_test.go index bb9124691..ad81dc017 100644 --- a/internal/hass/api/mock_DeviceInfo_test.go +++ b/internal/hass/api/mock_DeviceInfo_test.go @@ -17,7 +17,7 @@ var _ DeviceInfo = &DeviceInfoMock{} // // // make and configure a mocked DeviceInfo // mockedDeviceInfo := &DeviceInfoMock{ -// AppDataFunc: func() interface{} { +// AppDataFunc: func() any { // panic("mock out the AppData method") // }, // AppIDFunc: func() string { @@ -61,7 +61,7 @@ var _ DeviceInfo = &DeviceInfoMock{} // } type DeviceInfoMock struct { // AppDataFunc mocks the AppData method. - AppDataFunc func() interface{} + AppDataFunc func() any // AppIDFunc mocks the AppID method. AppIDFunc func() string @@ -150,7 +150,7 @@ type DeviceInfoMock struct { } // AppData calls AppDataFunc. -func (mock *DeviceInfoMock) AppData() interface{} { +func (mock *DeviceInfoMock) AppData() any { if mock.AppDataFunc == nil { panic("DeviceInfoMock.AppDataFunc: method is nil but DeviceInfo.AppData was just called") } diff --git a/internal/hass/api/registration.go b/internal/hass/api/registration.go index 421fc7a9c..fd17db288 100644 --- a/internal/hass/api/registration.go +++ b/internal/hass/api/registration.go @@ -7,18 +7,18 @@ package api import ( "context" - "errors" "net/url" "time" "github.com/carlmjohnson/requests" - "github.com/joshuar/go-hass-agent/internal/agent/config" "github.com/rs/zerolog/log" ) const ( - websocketPath = "/api/websocket" - webHookPath = "/api/webhook/" + websocketPath = "/api/websocket" + webHookPath = "/api/webhook/" + registrationPath = "/api/mobile_app/registrations" + authHeader = "Authorization" ) //go:generate moq -out mock_RegistrationInfo_test.go . RegistrationInfo @@ -86,33 +86,24 @@ type RegistrationRequest struct { SupportsEncryption bool `json:"supports_encryption"` } -func RegisterWithHass(ctx context.Context, cfg config.Config, device DeviceInfo) (*RegistrationResponse, error) { +func RegisterWithHass(ctx context.Context, server, token string, device DeviceInfo) (*RegistrationResponse, error) { request, err := device.MarshalJSON() if err != nil { return nil, err } - var u string - if err = cfg.Get(config.PrefHost, &u); err != nil { - return nil, errors.New("invalid host") - } - serverURL, err := url.Parse(u) + serverURL, err := url.Parse(server) if err != nil { return nil, err } - serverURL = serverURL.JoinPath("/api/mobile_app/registrations") - - var token string - if err = cfg.Get(config.PrefToken, &token); err != nil || token == "" { - return nil, errors.New("invalid token") - } + serverURL = serverURL.JoinPath(registrationPath) var response *RegistrationResponse ctx, cancel := context.WithTimeout(ctx, time.Minute) defer cancel() err = requests. URL(serverURL.String()). - Header("Authorization", "Bearer "+token). + Header(authHeader, "Bearer "+token). BodyBytes(request). ToJSON(&response). Fetch(ctx) diff --git a/internal/hass/api/registration_test.go b/internal/hass/api/registration_test.go index d77c6a85c..bb20dab33 100644 --- a/internal/hass/api/registration_test.go +++ b/internal/hass/api/registration_test.go @@ -14,7 +14,6 @@ import ( "reflect" "testing" - "github.com/joshuar/go-hass-agent/internal/agent/config" "github.com/stretchr/testify/assert" ) @@ -164,65 +163,32 @@ func TestRegisterWithHass(t *testing.T) { Secret: "", WebhookID: "someID", } - okJson, err := json.Marshal(okResponse) + okJSON, err := json.Marshal(okResponse) + assert.Nil(t, err) + + notokResponse := &RegistrationResponse{ + CloudhookURL: "", + RemoteUIURL: "", + Secret: "", + WebhookID: "", + } + notokJSON, err := json.Marshal(notokResponse) assert.Nil(t, err) newMockServer := func(t *testing.T) *httptest.Server { return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - w.Write(okJson) + token := r.Header.Get(authHeader) + if token != "Bearer" { + w.WriteHeader(http.StatusOK) + w.Write(okJSON) + } else { + w.WriteHeader(http.StatusBadRequest) + w.Write(notokJSON) + } })) } mockServer := newMockServer(t) - goodRegConfig := &AgentConfigMock{ - GetFunc: func(s string, ifaceVal any) error { - v := ifaceVal.(*string) - switch s { - case config.PrefHost: - *v = mockServer.URL - return nil - case config.PrefToken: - *v = "aToken" - return nil - default: - return errors.New("not found") - } - }, - } - - badRegServer := &AgentConfigMock{ - GetFunc: func(s string, ifaceVal any) error { - v := ifaceVal.(*string) - switch s { - case config.PrefHost: - *v = "notaurl" - return nil - case config.PrefToken: - *v = "aToken" - return nil - default: - return errors.New("not found") - } - }, - } - - badRegToken := &AgentConfigMock{ - GetFunc: func(s string, ifaceVal any) error { - v := ifaceVal.(*string) - switch s { - case config.PrefHost: - *v = mockServer.URL - return nil - case config.PrefToken: - *v = "" - return nil - default: - return errors.New("not found") - } - }, - } - mockDevInfo := &DeviceInfoMock{ MarshalJSONFunc: func() ([]byte, error) { return []byte(`{"AppName":"aDevice"}`), nil }, } @@ -231,9 +197,10 @@ func TestRegisterWithHass(t *testing.T) { } type args struct { - ctx context.Context - cfg config.Config - device DeviceInfo + ctx context.Context + server string + token string + device DeviceInfo } tests := []struct { name string @@ -244,18 +211,20 @@ func TestRegisterWithHass(t *testing.T) { { name: "successful test", args: args{ - ctx: context.Background(), - cfg: goodRegConfig, - device: mockDevInfo, + ctx: context.Background(), + server: mockServer.URL, + token: "aToken", + device: mockDevInfo, }, want: okResponse, }, { name: "bad device", args: args{ - ctx: context.Background(), - cfg: goodRegConfig, - device: mockBadDevInfo, + ctx: context.Background(), + server: mockServer.URL, + token: "aToken", + device: mockBadDevInfo, }, want: nil, wantErr: true, @@ -263,9 +232,10 @@ func TestRegisterWithHass(t *testing.T) { { name: "bad server url", args: args{ - ctx: context.Background(), - cfg: badRegServer, - device: mockDevInfo, + ctx: context.Background(), + server: "notAURL", + token: "aToken", + device: mockDevInfo, }, want: nil, wantErr: true, @@ -273,9 +243,10 @@ func TestRegisterWithHass(t *testing.T) { { name: "bad token", args: args{ - ctx: context.Background(), - cfg: badRegToken, - device: mockDevInfo, + ctx: context.Background(), + server: mockServer.URL, + token: "", + device: mockDevInfo, }, want: nil, wantErr: true, @@ -283,7 +254,7 @@ func TestRegisterWithHass(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := RegisterWithHass(tt.args.ctx, tt.args.cfg, tt.args.device) + got, err := RegisterWithHass(tt.args.ctx, tt.args.server, tt.args.token, tt.args.device) if (err != nil) != tt.wantErr { t.Errorf("RegisterWithHass() error = %v, wantErr %v", err, tt.wantErr) return