refactor: remove duplicated delete code

Give the tado client a central delete method that all setter functions can rely on.
gonzolino · Nov 11, 2021 · 05942f0 · 05942f0
1 parent 33aabe1
commit 05942f0
Show file tree

Hide file tree

Showing 3 changed files with 96 additions and 36 deletions.
diff --git a/client.go b/client.go
@@ -142,3 +142,20 @@ func (c *Client) put(url string, v interface{}) error {
 	}
 	return nil
 }
+
+// delete deletes an object from the tado° API.
+func (c *Client) delete(url string) error {
+	resp, err := c.Request(http.MethodDelete, url, nil)
+	if err != nil {
+		return err
+	}
+
+	if err := isError(resp); err != nil {
+		return fmt.Errorf("tado° API error: %w", err)
+	}
+
+	if resp.StatusCode != http.StatusNoContent {
+		return fmt.Errorf("unexpected tado° API response status: %s", resp.Status)
+	}
+	return nil
+}
diff --git a/client_test.go b/client_test.go
@@ -62,6 +62,7 @@ func TestWithCredentials(t *testing.T) {
 func makeResponse(code int, body string) *http.Response {
 	return &http.Response{
 		StatusCode: code,
+		Status:     http.StatusText(code),
 		Body:       io.NopCloser(strings.NewReader(body)),
 	}
 }
@@ -285,3 +286,77 @@ func TestPut(t *testing.T) {
 		})
 	}
 }
+
+func TestDelete(t *testing.T) {
+	tests := map[string]struct {
+		url      string
+		mockResp *http.Response
+		mockErr  error
+		wantErr  error
+	}{
+		"Simple": {
+			url:      "http://example.org",
+			mockResp: makeResponse(http.StatusNoContent, ""),
+			mockErr:  nil,
+			wantErr:  nil,
+		},
+		"UnexepctedResponseCode": {
+			url:      "http://example.org",
+			mockResp: makeResponse(http.StatusOK, `{"foo": "foo","bar": "bar"}`),
+			mockErr:  nil,
+			wantErr:  fmt.Errorf("unexpected tado° API response status: OK"),
+		},
+		"InvalidURL": {
+			url:      "invalid://url%%",
+			mockResp: nil,
+			mockErr:  nil,
+			wantErr:  fmt.Errorf("unable to create http request: parse \"invalid://url%%%%\": invalid URL escape \"%%%%\""),
+		},
+		"HTTPClientError": {
+			url:      "http://example.org",
+			mockResp: nil,
+			mockErr:  fmt.Errorf("http client error"),
+			wantErr:  fmt.Errorf("unable to talk to tado° API: http client error"),
+		},
+		"EmptyErrorList": {
+			url:      "http://example.org",
+			mockResp: makeResponse(http.StatusInternalServerError, `{"errors":[]}`),
+			mockErr:  nil,
+			wantErr:  fmt.Errorf("tado° API error: API returned empty error"),
+		},
+		"SingleError": {
+			url:      "http://example.org",
+			mockResp: makeResponse(http.StatusInternalServerError, `{"errors":[{"code":"1","title":"One"}]}`),
+			mockErr:  nil,
+			wantErr:  fmt.Errorf("tado° API error: 1: One"),
+		},
+		"MultiError": {
+			url:      "http://example.org",
+			mockResp: makeResponse(http.StatusInternalServerError, `{"errors":[{"code":"1","title":"One"},{"code":"2","title":"Two"}]}`),
+			mockErr:  nil,
+			wantErr:  fmt.Errorf("tado° API error: 1: One, 2: Two"),
+		},
+		"UnparseableError": {
+			url:      "http://example.org",
+			mockResp: makeResponse(http.StatusInternalServerError, `{errorjson}`),
+			mockErr:  nil,
+			wantErr:  fmt.Errorf("tado° API error: unable to decode API error: invalid character 'e' looking for beginning of object key string"),
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			client := NewClient("test", "test")
+			client.http = mockHTTPClient{Response: tc.mockResp, Error: tc.mockErr}
+
+			err := client.delete(tc.url)
+
+			if tc.wantErr != nil {
+				assert.EqualError(t, err, tc.wantErr.Error())
+			} else {
+				assert.NoError(t, err)
+			}
+
+		})
+	}
+}
diff --git a/tado.go b/tado.go
@@ -545,15 +545,7 @@ func SetZoneOverlayHeatingOn(client *Client, userHome *UserHome, zone *Zone, tem
 
 // DeleteZoneOverlay removes an overlay from a zone, thereby returning a zone to smart schedule
 func DeleteZoneOverlay(client *Client, userHome *UserHome, zone *Zone) error {
-	resp, err := client.Request(http.MethodDelete, apiURL("homes/%d/zones/%d/overlay", userHome.ID, zone.ID), nil)
-	if err != nil {
-		return err
-	}
-
-	if resp.StatusCode != http.StatusNoContent {
-		return fmt.Errorf("unexpected tado° API response status: %s", resp.Status)
-	}
-	return nil
+	return client.delete(apiURL("homes/%d/zones/%d/overlay", userHome.ID, zone.ID))
 }
 
 // SetWindowOpen marks the window in a zone as open (open window must have been detected before)
@@ -571,15 +563,7 @@ func SetWindowOpen(client *Client, userHome *UserHome, zone *Zone) error {
 
 // SetWindowClosed marks the window in a zone as closed
 func SetWindowClosed(client *Client, userHome *UserHome, zone *Zone) error {
-	resp, err := client.Request(http.MethodDelete, apiURL("homes/%d/zones/%d/state/openWindow", userHome.ID, zone.ID), nil)
-	if err != nil {
-		return err
-	}
-
-	if resp.StatusCode != http.StatusNoContent {
-		return fmt.Errorf("unexpected tado° API response status: %s", resp.Status)
-	}
-	return nil
+	return client.delete(apiURL("homes/%d/zones/%d/state/openWindow", userHome.ID, zone.ID))
 }
 
 // GetTimetables lists available schedule timetables for the given zone
@@ -718,15 +702,7 @@ func SetPresenceAway(client *Client, userHome *UserHome) error {
 
 // SetPresenceAuto removes a locked geofencing presence and returns to auto mode
 func SetPresenceAuto(client *Client, userHome *UserHome) error {
-	resp, err := client.Request(http.MethodDelete, apiURL("homes/%d/presenceLock", userHome.ID), nil)
-	if err != nil {
-		return err
-	}
-
-	if resp.StatusCode != http.StatusNoContent {
-		return fmt.Errorf("unexpected tado° API response status: %s", resp.Status)
-	}
-	return nil
+	return client.delete(apiURL("homes/%d/presenceLock", userHome.ID))
 }
 
 // IsEarlyStartEnabled returns if the given zone has turned on early start
@@ -791,15 +767,7 @@ func GetMobileDevices(client *Client, userHome *UserHome) ([]*MobileDevice, erro
 
 // DeleteMobileDevice deletes the given mobile device
 func DeleteMobileDevice(client *Client, userHome *UserHome, mobileDevice *MobileDevice) error {
-	resp, err := client.Request(http.MethodDelete, apiURL("homes/%d/mobileDevices/%d", userHome.ID, mobileDevice.ID), nil)
-	if err != nil {
-		return err
-	}
-
-	if resp.StatusCode != http.StatusNoContent {
-		return fmt.Errorf("unexpected tado° API response status: %s", resp.Status)
-	}
-	return nil
+	return client.delete(apiURL("homes/%d/mobileDevices/%d", userHome.ID, mobileDevice.ID))
 }
 
 // SetMobileDeviceSettings updates the given mobile device with the given settings