From fb7848d1c5dae1aab88fd70148ae8cb783dda11f Mon Sep 17 00:00:00 2001
From: Christoph Glaubitz <christoph.glaubitz@innovo-cloud.de>
Date: Mon, 8 May 2017 16:40:49 +0200
Subject: [PATCH] Prepared client for POST/PUT/PATCH/DELETE
1. Fixed issue with concating urls, removed use of path.Join
The problem here: With path.Join, the trailing slash has been removed on
each call. Even when using `NewRequest(..., "/api/XXX/", nil)`. As a result,
Do always queries `http://nebox.example.com/api/XXX`, getting back a 301
and retries the new Location. While this works well with GET, it breaks
all the pushing methods. To not override too much, or too less, of
`Client.u`, I append/prepend slashes if necessary.
2. Added two new functions to construct requests
* c.NewDataRequest to create a request with body
* c.NewJSONRequest to make posts more convenient
* Changed c.NewRequest to be a wrap for NewJSONRequest
As a result, using NewRequest stays the same, but NewJSONRequest can be
used to create "writing" functions, without the need of repeatedly
checking for errors. e.g. for `tenant-groups`:
```
func (s *TenantGroupsService) Update(group *TenantGroup) (*TenantGroup, error) {
req, err := s.c.NewJSONRequest(
http.MethodPatch,
fmt.Sprintf("api/tenancy/tenant-groups/%d/", group.ID),
nil,
group)
if err != nil {
return nil, err
}
g := new(TenantGroup)
err = s.c.Do(req, g)
return g, err
}
func (s *TenantGroupsService) Delete(group *TenantGroup) error {
req, err := s.c.NewRequest(
http.MethodDelete,
fmt.Sprintf("api/tenancy/tenant-groups/%d/", group.ID),
nil)
if err != nil {
return err
}
return s.c.Do(req, nil)
}
```
---
netbox/client.go | 74 +++++++++++++++++++++++++++++++++++++------
netbox/client_test.go | 41 ++++++++++++++++++++++++
2 files changed, 106 insertions(+), 9 deletions(-)
diff --git a/netbox/client.go b/netbox/client.go
index 333fea6..1cf4c12 100644
--- a/netbox/client.go
+++ b/netbox/client.go
@@ -15,12 +15,15 @@
package netbox
import (
+ "bytes"
"encoding/json"
+ "errors"
"fmt"
+ "io"
"io/ioutil"
"net/http"
"net/url"
- "path"
+ "strings"
)
// A Client is a NetBox client. It can be used to retrieve network and
@@ -46,6 +49,13 @@ func NewClient(addr string, client *http.Client) (*Client, error) {
client = &http.Client{}
}
+ // Append trailing slash there is none. This is necessary
+ // to be able to concat url parts in a correct manner.
+ // See NewRequest
+ if !strings.HasSuffix(addr, "/") {
+ addr = addr + "/"
+ }
+
u, err := url.Parse(addr)
if err != nil {
return nil, err
@@ -68,25 +78,40 @@ func NewClient(addr string, client *http.Client) (*Client, error) {
// If a nil Valuer is specified, no query parameters will be sent with the
// request.
func (c *Client) NewRequest(method string, endpoint string, options Valuer) (*http.Request, error) {
- rel, err := url.Parse(endpoint)
- if err != nil {
- return nil, err
- }
+ return c.NewDataRequest(method, endpoint, options, nil)
+}
+// NewDataRequest creates a HTTP request using the input HTTP method, URL
+// endpoint, a Valuer which creates URL parameters for the request, and
+// a io.Reader as the body of the request.
+//
+// If a nil Valuer is specified, no query parameters will be sent with the
+// request.
+// If a nil io.Reader, no body will be sent with the request.
+func (c *Client) NewDataRequest(method string, endpoint string, options Valuer, body io.Reader) (*http.Request, error) {
// Allow specifying a base path for API requests, so if a NetBox server
// resides at a path like http://example.com/netbox/, API requests will
// be sent to http://example.com/netbox/api/...
//
// Enables support of: https://github.com/digitalocean/netbox/issues/212.
- if c.u.Path != "" {
- rel.Path = path.Join(c.u.Path, rel.Path)
+ //
+ // Remove leading slash if there is one. This is necessary to be able to
+ // concat url parts in a correct manner. We can not use path.Join here,
+ // because this always trims the trailing slash, which causes the
+ // Do function to always run into 301 and then retrying the correct
+ // Location. With GET, it does work with one useless request, but it breaks
+ // each other http method.
+ // Doing this, because out-of-tree extensions are more robust.
+ rel, err := url.Parse(strings.TrimLeft(endpoint, "/"))
+ if err != nil {
+ return nil, err
}
u := c.u.ResolveReference(rel)
// If no valuer specified, create a request with no query parameters
if options == nil {
- return http.NewRequest(method, u.String(), nil)
+ return http.NewRequest(method, u.String(), body)
}
values, err := options.Values()
@@ -95,7 +120,38 @@ func (c *Client) NewRequest(method string, endpoint string, options Valuer) (*ht
}
u.RawQuery = values.Encode()
- return http.NewRequest(method, u.String(), nil)
+ return http.NewRequest(method, u.String(), body)
+}
+
+// NewJSONRequest creates a HTTP request using the input HTTP method, URL
+// endpoint, a Valuer which creates URL parameters for the request, and
+// a io.Reader as the body of the request. For body, expecting some
+// json.Marshal-able struct. nil body is not allowed.
+// NewJSONRequest also sets HTTP Header
+// "Content-Type: application/json; utf-8"
+//
+// If a nil Valuer is specified, no query parameters will be sent with the
+// request.
+func (c *Client) NewJSONRequest(method string, endpoint string, options Valuer, body interface{}) (*http.Request, error) {
+ if body == nil {
+ return nil, errors.New("expected body to be not nil")
+ }
+ b := new(bytes.Buffer)
+ err := json.NewEncoder(b).Encode(body)
+ if err != nil {
+ return nil, err
+ }
+ req, err := c.NewDataRequest(
+ method,
+ endpoint,
+ options,
+ b,
+ )
+ if err != nil {
+ return nil, err
+ }
+ req.Header.Set("Content-Type", "application/json; charset=utf-8")
+ return req, nil
}
// Do executes an HTTP request and if v is not nil, Do unmarshals result
diff --git a/netbox/client_test.go b/netbox/client_test.go
index 75f8e0b..02d65f2 100644
--- a/netbox/client_test.go
+++ b/netbox/client_test.go
@@ -18,6 +18,7 @@ import (
"encoding/json"
"errors"
"fmt"
+ "io/ioutil"
"net/http"
"net/http/httptest"
"net/url"
@@ -95,6 +96,46 @@ func TestClientBadStatusCode(t *testing.T) {
}
}
+func TestNewJSONRequest(t *testing.T) {
+ c, done := testClient(t, func(w http.ResponseWriter, r *http.Request) {
+ w.Write([]byte("foo"))
+ })
+ defer done()
+ wantBody := "{\"id\":1,\"name\":\"Test 1\"}\n"
+ wantHeader := "application/json; charset=utf-8"
+
+ req, err := c.NewJSONRequest(http.MethodPost, "/", nil, &struct {
+ ID int `json:"id"`
+ Name string `json:"name"`
+ }{
+ ID: 1,
+ Name: "Test 1",
+ })
+ if err != nil {
+ t.Fatal("expected no error, but an error returned")
+ }
+
+ res, err := ioutil.ReadAll(req.Body)
+ if err != nil {
+ t.Fatal("expected no error, but an error returned")
+ }
+ if want, got := wantBody, string(res); got != want {
+ t.Fatalf("unexpected body:\n- want: %v\n- got: %v", want, got)
+ }
+
+ if want, got := wantHeader, req.Header.Get("Content-Type"); got != want {
+ t.Fatalf("unexpected body:\n- want: %v\n- got: %v", want, got)
+ }
+
+ req, err = c.NewJSONRequest(http.MethodPost, "/", nil, nil)
+ if err == nil {
+ t.Fatal("expected an error, but there was none")
+ }
+ if req != nil {
+ t.Fatalf("expected a nil request, but got %v", req)
+ }
+}
+
func TestClientQueryParameters(t *testing.T) {
c := &Client{
u: &url.URL{},
--
GitLab