Merge pull request #50 from messagebird/fix-query

Fix query

Author: Artur Khabibullin

client.go

@@ -25,7 +25,7 @@ import (
 
 const (
 	// ClientVersion is used in User-Agent request header to provide server with API level.
-	ClientVersion = "5.1.0"
+	ClientVersion = "5.1.1"
 
 	// Endpoint points you to MessageBird REST API.
 	Endpoint = "https://rest.messagebird.com"
@@ -47,6 +47,14 @@ type Client struct {
 	DebugLog   *log.Logger  // Optional logger for debugging purposes
 }
 
+type contentType string
+
+const (
+	contentTypeEmpty          contentType = ""
+	contentTypeJSON           contentType = "application/json"
+	contentTypeFormURLEncoded contentType = "application/x-www-form-urlencoded"
+)
+
 // New creates a new MessageBird client object.
 func New(accessKey string) *Client {
 	return &Client{
@@ -67,27 +75,26 @@ func (c *Client) Request(v interface{}, method, path string, data interface{}) e
 		return err
 	}
 
-	var jsonEncoded []byte
-	if data != nil {
-		jsonEncoded, err = json.Marshal(data)
-		if err != nil {
-			return err
-		}
+	body, contentType, err := prepareRequestBody(data)
+	if err != nil {
+		return err
 	}
 
-	request, err := http.NewRequest(method, uri.String(), bytes.NewBuffer(jsonEncoded))
+	request, err := http.NewRequest(method, uri.String(), bytes.NewBuffer(body))
 	if err != nil {
 		return err
 	}
 
-	request.Header.Set("Content-Type", "application/json")
 	request.Header.Set("Accept", "application/json")
 	request.Header.Set("Authorization", "AccessKey "+c.AccessKey)
 	request.Header.Set("User-Agent", "MessageBird/ApiClient/"+ClientVersion+" Go/"+runtime.Version())
+	if contentType != contentTypeEmpty {
+		request.Header.Set("Content-Type", string(contentType))
+	}
 
 	if c.DebugLog != nil {
 		if data != nil {
-			c.DebugLog.Printf("HTTP REQUEST: %s %s %s", method, uri.String(), jsonEncoded)
+			c.DebugLog.Printf("HTTP REQUEST: %s %s %s", method, uri.String(), body)
 		} else {
 			c.DebugLog.Printf("HTTP REQUEST: %s %s", method, uri.String())
 		}
@@ -136,3 +143,22 @@ func (c *Client) Request(v interface{}, method, path string, data interface{}) e
 		return errorResponse
 	}
 }
+
+// prepareRequestBody takes untyped data and attempts constructing a meaningful
+// request body from it. It also returns the appropriate Content-Type.
+func prepareRequestBody(data interface{}) ([]byte, contentType, error) {
+	switch data := data.(type) {
+	case nil:
+		// Nil bodies are accepted by `net/http`, so this is not an error.
+		return nil, contentTypeEmpty, nil
+	case string:
+		return []byte(data), contentTypeFormURLEncoded, nil
+	default:
+		b, err := json.Marshal(data)
+		if err != nil {
+			return nil, contentType(""), err
+		}
+
+		return b, contentTypeJSON, nil
+	}
+}

group/group.go

@@ -161,10 +161,9 @@ func AddContacts(c *messagebird.Client, groupID string, contactIDs []string) err
 		return err
 	}
 
-	query := addContactsQuery(contactIDs)
-	formattedPath := fmt.Sprintf("%s/%s/%s?%s", path, groupID, contactPath, query)
+	data := addContactsData(contactIDs)
 
-	return c.Request(nil, http.MethodGet, formattedPath, nil)
+	return c.Request(nil, http.MethodPut, path+"/"+groupID+"/"+contactPath, data)
 }
 
 func validateAddContacts(contactIDs []string) error {
@@ -182,19 +181,12 @@ func validateAddContacts(contactIDs []string) error {
 	return nil
 }
 
-// addContactsQuery gets a query string to add contacts to a group. We're using
-// the alternative "/foo?_method=PUT&key=value" format to send the contact IDs
-// as GET params. Sending these in the request body would require a painful
-// workaround, as client.Request sends request bodies as JSON by default. See
-// also: https://developers.messagebird.com/docs/alternatives.
-//
-// It should also be noted that we're intentionally not using url.Values for
-// building the query string: the API expects `ids[]=foo&ids[]=bar` format,
-// while url.Values encodes to `ids=foo&ids=bar`.
-func addContactsQuery(contactIDs []string) string {
-	// Slice's length is one bigger than len(IDs) for the _method param.
-	params := make([]string, 0, len(contactIDs)+1)
-	params = append(params, "_method="+http.MethodPut)
+// addContactsData gets the data string for adding a contact to a group. We're
+// intentionally not using url.Values for building the string: the API expects
+// `ids[]=foo&ids[]=bar` format, while url.Values encodes to `ids=foo&ids=bar`.
+func addContactsData(contactIDs []string) string {
+	cap := len(contactIDs)
+	params := make([]string, 0, cap)
 
 	for _, contactID := range contactIDs {
 		params = append(params, "ids[]="+contactID)

group/group_test.go

@@ -159,6 +159,10 @@ func TestUpdate(t *testing.T) {
 
 	mbtest.AssertEndpointCalled(t, http.MethodPatch, "/groups/group-id")
 	mbtest.AssertTestdata(t, "groupRequestUpdateObject.json", mbtest.Request.Body)
+
+	if mbtest.Request.ContentType != "application/json" {
+		t.Fatalf("got %s, expected application/json", mbtest.Request.ContentType)
+	}
 }
 
 func TestAddContacts(t *testing.T) {
@@ -169,10 +173,11 @@ func TestAddContacts(t *testing.T) {
 		t.Fatalf("unexpected error removing Contacts from Group: %s", err)
 	}
 
-	mbtest.AssertEndpointCalled(t, http.MethodGet, "/groups/group-id/contacts")
+	mbtest.AssertEndpointCalled(t, http.MethodPut, "/groups/group-id/contacts")
+	mbtest.AssertTestdata(t, "groupRequestAddContactsObject.txt", mbtest.Request.Body)
 
-	if mbtest.Request.URL.RawQuery != "_method=PUT&ids[]=first-contact-id&ids[]=second-contact-id" {
-		t.Fatalf("got %s, expected _method=PUT&ids[]=first-contact-id&ids[]=second-contact-id", mbtest.Request.URL.RawQuery)
+	if mbtest.Request.ContentType != "application/x-www-form-urlencoded" {
+		t.Fatalf("got %s, expected application/x-www-form-urlencoded", mbtest.Request.ContentType)
 	}
 }
 

group/testdata/groupRequestAddContactsObject.txt

@@ -0,0 +1 @@
+ids[]=first-contact-id&ids[]=second-contact-id

internal/mbtest/test_server.go

@@ -10,9 +10,9 @@ import (
 )
 
 type request struct {
-	Body   []byte
-	Method string
-	URL    *url.URL
+	Body                []byte
+	ContentType, Method string
+	URL                 *url.URL
 }
 
 // Request contains the lastly received http.Request by the fake server.
@@ -35,8 +35,9 @@ func EnableServer(m *testing.M) {
 func initAndStartServer() {
 	server = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		Request = request{
-			Method: r.Method,
-			URL:    r.URL,
+			ContentType: r.Header.Get("Content-Type"),
+			Method:      r.Method,
+			URL:         r.URL,
 		}
 
 		var err error