From 2031c888fdd8e1dcae7765dd832f38a0ddc30270 Mon Sep 17 00:00:00 2001 From: Joshua Rich Date: Sun, 7 Jul 2024 21:28:38 +1000 Subject: [PATCH] perf(hass): :recycle: rework request logic - response interface now expects to be able to represent an error - responses can unmarshal a the raw response into their own error type for later usage - update response structs to satisfy new response interface --- internal/hass/config.go | 17 +++++- internal/hass/mock_Response_test.go | 83 ++++++++++++++++++++++++++++- internal/hass/registration.go | 15 ++++++ internal/hass/registration_test.go | 10 ++++ internal/hass/request.go | 39 +++++++------- internal/hass/request_test.go | 18 +++---- internal/hass/sensor/sensor.go | 46 +++++++++++++++- internal/hass/states.go | 29 ++++++++++ 8 files changed, 223 insertions(+), 34 deletions(-) diff --git a/internal/hass/config.go b/internal/hass/config.go index 813e25eb9..92a8f79cc 100644 --- a/internal/hass/config.go +++ b/internal/hass/config.go @@ -3,6 +3,7 @@ // This software is released under the MIT License. // https://opensource.org/licenses/MIT +//nolint:errname // structs are dual-purpose response and error //revive:disable:unused-receiver package hass @@ -20,7 +21,8 @@ var ErrLoadPrefsFailed = errors.New("could not load preferences") type Config struct { Details *ConfigEntries - mu sync.Mutex + *APIError + mu sync.Mutex } type ConfigEntries struct { @@ -69,6 +71,19 @@ func (c *Config) UnmarshalJSON(b []byte) error { return nil } +func (c *Config) UnmarshalError(data []byte) error { + err := json.Unmarshal(data, c.APIError) + if err != nil { + return fmt.Errorf("could not unmarshal: %w", err) + } + + return nil +} + +func (c *Config) Error() string { + return c.APIError.Error() +} + type configRequest struct{} func (c *configRequest) RequestBody() json.RawMessage { diff --git a/internal/hass/mock_Response_test.go b/internal/hass/mock_Response_test.go index eb47f67a3..a62b33b81 100644 --- a/internal/hass/mock_Response_test.go +++ b/internal/hass/mock_Response_test.go @@ -17,6 +17,12 @@ var _ Response = &ResponseMock{} // // // make and configure a mocked Response // mockedResponse := &ResponseMock{ +// ErrorFunc: func() string { +// panic("mock out the Error method") +// }, +// UnmarshalErrorFunc: func(data []byte) error { +// panic("mock out the UnmarshalError method") +// }, // UnmarshalJSONFunc: func(bytes []byte) error { // panic("mock out the UnmarshalJSON method") // }, @@ -27,18 +33,93 @@ var _ Response = &ResponseMock{} // // } type ResponseMock struct { + // ErrorFunc mocks the Error method. + ErrorFunc func() string + + // UnmarshalErrorFunc mocks the UnmarshalError method. + UnmarshalErrorFunc func(data []byte) error + // UnmarshalJSONFunc mocks the UnmarshalJSON method. UnmarshalJSONFunc func(bytes []byte) error // calls tracks calls to the methods. calls struct { + // Error holds details about calls to the Error method. + Error []struct { + } + // UnmarshalError holds details about calls to the UnmarshalError method. + UnmarshalError []struct { + // Data is the data argument value. + Data []byte + } // UnmarshalJSON holds details about calls to the UnmarshalJSON method. UnmarshalJSON []struct { // Bytes is the bytes argument value. Bytes []byte } } - lockUnmarshalJSON sync.RWMutex + lockError sync.RWMutex + lockUnmarshalError sync.RWMutex + lockUnmarshalJSON sync.RWMutex +} + +// Error calls ErrorFunc. +func (mock *ResponseMock) Error() string { + if mock.ErrorFunc == nil { + panic("ResponseMock.ErrorFunc: method is nil but Response.Error was just called") + } + callInfo := struct { + }{} + mock.lockError.Lock() + mock.calls.Error = append(mock.calls.Error, callInfo) + mock.lockError.Unlock() + return mock.ErrorFunc() +} + +// ErrorCalls gets all the calls that were made to Error. +// Check the length with: +// +// len(mockedResponse.ErrorCalls()) +func (mock *ResponseMock) ErrorCalls() []struct { +} { + var calls []struct { + } + mock.lockError.RLock() + calls = mock.calls.Error + mock.lockError.RUnlock() + return calls +} + +// UnmarshalError calls UnmarshalErrorFunc. +func (mock *ResponseMock) UnmarshalError(data []byte) error { + if mock.UnmarshalErrorFunc == nil { + panic("ResponseMock.UnmarshalErrorFunc: method is nil but Response.UnmarshalError was just called") + } + callInfo := struct { + Data []byte + }{ + Data: data, + } + mock.lockUnmarshalError.Lock() + mock.calls.UnmarshalError = append(mock.calls.UnmarshalError, callInfo) + mock.lockUnmarshalError.Unlock() + return mock.UnmarshalErrorFunc(data) +} + +// UnmarshalErrorCalls gets all the calls that were made to UnmarshalError. +// Check the length with: +// +// len(mockedResponse.UnmarshalErrorCalls()) +func (mock *ResponseMock) UnmarshalErrorCalls() []struct { + Data []byte +} { + var calls []struct { + Data []byte + } + mock.lockUnmarshalError.RLock() + calls = mock.calls.UnmarshalError + mock.lockUnmarshalError.RUnlock() + return calls } // UnmarshalJSON calls UnmarshalJSONFunc. diff --git a/internal/hass/registration.go b/internal/hass/registration.go index 5f896c3e2..ee5569967 100644 --- a/internal/hass/registration.go +++ b/internal/hass/registration.go @@ -3,6 +3,7 @@ // This software is released under the MIT License. // https://opensource.org/licenses/MIT +//nolint:errname // structs are dual-purpose response and error package hass import ( @@ -93,6 +94,7 @@ func newRegistrationRequest(info *DeviceInfo, token string) *registrationRequest type registrationResponse struct { Details *RegistrationDetails + *APIError } func (r *registrationResponse) UnmarshalJSON(b []byte) error { @@ -104,6 +106,19 @@ func (r *registrationResponse) UnmarshalJSON(b []byte) error { return nil } +func (r *registrationResponse) UnmarshalError(data []byte) error { + err := json.Unmarshal(data, r.APIError) + if err != nil { + return fmt.Errorf("could not unmarshal: %w", err) + } + + return nil +} + +func (r *registrationResponse) Error() string { + return r.APIError.Error() +} + //nolint:exhaustruct func newRegistrationResponse() *registrationResponse { return ®istrationResponse{} diff --git a/internal/hass/registration_test.go b/internal/hass/registration_test.go index db322d518..a83df2f6d 100644 --- a/internal/hass/registration_test.go +++ b/internal/hass/registration_test.go @@ -29,14 +29,24 @@ var mockDevInfo = &DeviceInfo{ SupportsEncryption: false, } +//nolint:errname type failedResponse struct { Details *RegistrationDetails + *APIError } func (r *failedResponse) UnmarshalJSON(b []byte) error { return json.Unmarshal(b, &r.Details) } +func (r *failedResponse) UnmarshalError(b []byte) error { + return json.Unmarshal(b, r.APIError) +} + +func (r *failedResponse) Error() string { + return r.APIError.Error() +} + // setup creates a context using a test http client and server which will // return the given response when the ExecuteRequest function is called. var setupTestServer = func(t *testing.T, response Response) *httptest.Server { diff --git a/internal/hass/request.go b/internal/hass/request.go index b15e8a21d..248359a2f 100644 --- a/internal/hass/request.go +++ b/internal/hass/request.go @@ -23,9 +23,10 @@ var ( ErrInvalidURL = errors.New("invalid URL") ErrInvalidClient = errors.New("invalid client") ErrResponseMalformed = errors.New("malformed response") - ErrNoPrefs = errors.New("loading preferences failed") - defaultTimeout = 30 * time.Second - defaultRetry = func(r *resty.Response, _ error) bool { + ErrUnknown = errors.New("unknown error occurred") + + defaultTimeout = 30 * time.Second + defaultRetry = func(r *resty.Response, _ error) bool { return r.StatusCode() == http.StatusTooManyRequests } ) @@ -76,6 +77,8 @@ type Encrypted interface { //go:generate moq -out mock_Response_test.go . Response type Response interface { json.Unmarshaler + UnmarshalError(data []byte) error + error } // ExecuteRequest sends an API request to Home Assistant. It supports either the @@ -84,23 +87,17 @@ type Response interface { // satisfy the PostRequest interface. To add authentication where required, // satisfy the Auth interface. To send an encrypted request, satisfy the Secret // interface. -// -//nolint:exhaustruct func ExecuteRequest(ctx context.Context, client *resty.Client, url string, request any, response Response) error { if client == nil { return ErrInvalidClient } - // ?: handle nil response here - var responseErr *APIError - var resp *resty.Response var err error webClient := client.R(). - SetContext(ctx). - SetError(&responseErr) + SetContext(ctx) if a, ok := request.(Authenticated); ok { webClient = webClient.SetAuthToken(a.Auth()) } @@ -123,6 +120,7 @@ func ExecuteRequest(ctx context.Context, client *resty.Client, url string, reque resp, err = webClient.Get(url) } + // If the client fails to send the request, return a wrapped error. if err != nil { return fmt.Errorf("could not send request: %w", err) } @@ -136,21 +134,20 @@ func ExecuteRequest(ctx context.Context, client *resty.Client, url string, reque RawJSON("body", resp.Body()). Msg("Response received.") + // If the response is an error code, unmarshal it with the error method. if resp.IsError() { - if responseErr != nil { - responseErr.StatusCode = resp.StatusCode() - - return responseErr + if err := response.UnmarshalError(resp.Body()); err != nil { + return ErrUnknown } - return &APIError{ - StatusCode: resp.StatusCode(), - Message: resp.Status(), - } + return response } - - if err := response.UnmarshalJSON(resp.Body()); err != nil { - return errors.Join(ErrResponseMalformed, err) + // Otherwise for a successful response, if the response body is not an empty + // string, unmarshal it. + if string(resp.Body()) != "" { + if err := response.UnmarshalJSON(resp.Body()); err != nil { + return errors.Join(ErrResponseMalformed, err) + } } return nil diff --git a/internal/hass/request_test.go b/internal/hass/request_test.go index 1a9ee1428..c6b1ceea9 100644 --- a/internal/hass/request_test.go +++ b/internal/hass/request_test.go @@ -94,6 +94,10 @@ func TestExecuteRequest(t *testing.T) { badPostReq := PostRequestMock{ RequestBodyFunc: func() json.RawMessage { return json.RawMessage(`{"field":"value"}`) }, } + badPostResp := &ResponseMock{ + UnmarshalErrorFunc: func(_ []byte) error { return nil }, + ErrorFunc: func() string { return "400 Bad Request" }, + } // badPostResp := &APIError{ // StatusCode: 400, @@ -133,19 +137,13 @@ func TestExecuteRequest(t *testing.T) { }, { name: "invalid post request", - args: args{ctx: context.TODO(), client: NewDefaultHTTPClient(mockServer.URL), url: "/badPost", request: badPostReq, response: &ResponseMock{}}, - want: &APIError{ - StatusCode: 400, - Message: "400 Bad Request", - }, + args: args{ctx: context.TODO(), client: NewDefaultHTTPClient(mockServer.URL), url: "/badPost", request: badPostReq, response: badPostResp}, + want: badPostResp, }, { name: "invalid get request", - args: args{ctx: context.TODO(), client: NewDefaultHTTPClient(mockServer.URL), url: "/badGet", request: "anything", response: &ResponseMock{}}, - want: &APIError{ - StatusCode: 400, - Message: "400 Bad Request", - }, + args: args{ctx: context.TODO(), client: NewDefaultHTTPClient(mockServer.URL), url: "/badGet", request: "anything", response: badPostResp}, + want: badPostResp, }, // { // name: "badData", diff --git a/internal/hass/sensor/sensor.go b/internal/hass/sensor/sensor.go index f95238c06..5a0a3fbc4 100644 --- a/internal/hass/sensor/sensor.go +++ b/internal/hass/sensor/sensor.go @@ -3,6 +3,7 @@ // This software is released under the MIT License. // https://opensource.org/licenses/MIT +//nolint:errname // structs are dual-purpose response and error package sensor import ( @@ -191,6 +192,7 @@ type haError struct { type updateResponse struct { Body map[string]*response + *hass.APIError } func (u *updateResponse) UnmarshalJSON(b []byte) error { @@ -202,7 +204,21 @@ func (u *updateResponse) UnmarshalJSON(b []byte) error { return nil } +func (u *updateResponse) UnmarshalError(data []byte) error { + err := json.Unmarshal(data, u.APIError) + if err != nil { + return fmt.Errorf("could not unmarshal: %w", err) + } + + return nil +} + +func (u *updateResponse) Error() string { + return u.APIError.Error() +} + type registrationResponse struct { + *hass.APIError Body response } @@ -215,9 +231,37 @@ func (r *registrationResponse) UnmarshalJSON(b []byte) error { return nil } -type locationResponse struct{} +func (r *registrationResponse) UnmarshalError(data []byte) error { + err := json.Unmarshal(data, r.APIError) + if err != nil { + return fmt.Errorf("could not unmarshal: %w", err) + } + + return nil +} + +func (r *registrationResponse) Error() string { + return r.APIError.Error() +} + +type locationResponse struct { + *hass.APIError +} //revive:disable:unused-receiver func (l *locationResponse) UnmarshalJSON(_ []byte) error { return nil } + +func (l *locationResponse) UnmarshalError(data []byte) error { + err := json.Unmarshal(data, l.APIError) + if err != nil { + return fmt.Errorf("could not unmarshal: %w", err) + } + + return nil +} + +func (l *locationResponse) Error() string { + return l.APIError.Error() +} diff --git a/internal/hass/states.go b/internal/hass/states.go index a77170cc0..b4d564974 100644 --- a/internal/hass/states.go +++ b/internal/hass/states.go @@ -3,6 +3,7 @@ // This software is released under the MIT License. // https://opensource.org/licenses/MIT +//nolint:errname // structs are dual-purpose response and error //revive:disable:unused-receiver package hass @@ -37,6 +38,7 @@ func (e *EntityStateRequest) Auth() string { type EntityStateResponse struct { State *EntityState + *APIError } func (e *EntityStateResponse) UnmarshalJSON(b []byte) error { @@ -48,6 +50,19 @@ func (e *EntityStateResponse) UnmarshalJSON(b []byte) error { return nil } +func (e *EntityStateResponse) UnmarshalError(data []byte) error { + err := json.Unmarshal(data, e.APIError) + if err != nil { + return fmt.Errorf("could not unmarshal: %w", err) + } + + return nil +} + +func (e *EntityStateResponse) Error() string { + return e.APIError.Error() +} + //nolint:exhaustruct func GetEntityState(ctx context.Context, sensorType, entityID string) (*EntityStateResponse, error) { prefs, err := preferences.ContextGetPrefs(ctx) @@ -77,6 +92,7 @@ func (e *EntityStatesRequest) Auth() string { } type EntityStatesResponse struct { + *APIError States []EntityStateResponse } @@ -89,6 +105,19 @@ func (e *EntityStatesResponse) UnmarshalJSON(b []byte) error { return nil } +func (e *EntityStatesResponse) UnmarshalError(data []byte) error { + err := json.Unmarshal(data, e.APIError) + if err != nil { + return fmt.Errorf("could not unmarshal: %w", err) + } + + return nil +} + +func (e *EntityStatesResponse) Error() string { + return e.APIError.Error() +} + //nolint:exhaustruct func GetAllEntityStates(ctx context.Context) (*EntityStatesResponse, error) { prefs, err := preferences.ContextGetPrefs(ctx)