Modify AddContacts to use PUT requests rather than the alternative GET method.

On August 21st, a PR was merged into Go's master (https://go-review.googlesource.com/c/go/+/99135). That PR forces url.URL queries to actually be properly encoded, which breaks our previous implementation.

Author: epels

client.go

@@ -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,24 @@ 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("Content-Type", string(contentType))
 	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 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 +141,24 @@ 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) {
+	// Nil bodies are accepted by `net/http`, so this is not an error.
+	if data == nil {
+		return nil, contentTypeEmpty, nil
+	}
+
+	s, ok := data.(string)
+	if ok {
+		return []byte(s), contentTypeFormURLEncoded, nil
+	}
+
+	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

@@ -169,10 +169,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)
 	}
 }