Repository: nicklaw5/helix
Pull request #176 (2023-02-05 11:34:32)

Fix query builder adding all struct fields as parameters
To test, print the request being made in something like the "BanUser" request, it will show this:
`https://api.twitch.tv/helix/moderation/bans?=%7B60++22484632%7D&=117166826&=117166826`

This is because the `buildQueryString` builder doesn't care if the `query` tag exists, it'll add values with an empty key.

The test run before the fix commit:
```
$ go test -v -p 1 -run 'Query'
=== RUN   TestQueryStringBuilderAllQuery
=== PAUSE TestQueryStringBuilderAllQuery
=== RUN   TestQueryStringBuilderPartialQuery
=== PAUSE TestQueryStringBuilderPartialQuery
=== RUN   TestQueryStringBuilderNoQuery
=== PAUSE TestQueryStringBuilderNoQuery
=== CONT  TestQueryStringBuilderAllQuery
=== CONT  TestQueryStringBuilderNoQuery
--- PASS: TestQueryStringBuilderAllQuery (0.00s)
=== NAME  TestQueryStringBuilderNoQuery
    helix_test.go:732: expected q to be "", got "=value&=value"
--- FAIL: TestQueryStringBuilderNoQuery (0.00s)
=== CONT  TestQueryStringBuilderPartialQuery
    helix_test.go:707: expected q to be "foo=value", got "=value&foo=value"
--- FAIL: TestQueryStringBuilderPartialQuery (0.00s)
FAIL
exit status 1
FAIL    github.com/nicklaw5/helix/v2    0.003s
```

The lack of coverage has to do with previously-parsed-as-query-params-code that is now correctly only serialized into the json body during tests.



Patch:
diff --git a/helix.go b/helix.go
index e0ed1f4..a374eff 100644
--- a/helix.go
+++ b/helix.go
@@ -202,6 +202,10 @@ func buildQueryString(req *http.Request, v interface{}) (string, error) {
 		field := vType.Field(i)
 		tag := field.Tag.Get("query")
 
+		if tag == "" {
+			continue
+		}
+
 		// Get the default value from the struct tag
 		if strings.Contains(tag, ",") {
 			tagSlice := strings.Split(tag, ",")


Test patch:
diff --git a/helix_test.go b/helix_test.go
index f09730f..41c7b1b 100644
--- a/helix_test.go
+++ b/helix_test.go
@@ -657,3 +657,78 @@ func TestHydrateRequestCommon(t *testing.T) {
 		t.Errorf("expected ErrorMessage to be \"%s\", got \"%s\"", sampleErrorMessage, targetResponse.ResponseCommon.ErrorMessage)
 	}
 }
+
+func TestQueryStringBuilderAllQuery(t *testing.T) {
+	t.Parallel()
+
+	// For structs where every member has a query tag
+	type Struct struct {
+		Foo string `query:"foo"`
+		Bar string `query:"bar"`
+	}
+	v := &Struct{Foo: "value", Bar: "value"}
+	expectedQueryString := `bar=value&foo=value`
+
+	req, err := http.NewRequest("GET", "https://example.com", nil)
+	if err != nil {
+		t.Error("request creation failed")
+	}
+
+	q, err := buildQueryString(req, v)
+	if err != nil {
+		t.Errorf("expected buildQueryString to not error, got \"%s\"", err)
+	}
+	if q != expectedQueryString {
+		t.Errorf(`expected q to be "%s", got "%s"`, expectedQueryString, q)
+	}
+}
+
+func TestQueryStringBuilderPartialQuery(t *testing.T) {
+	t.Parallel()
+
+	// For structs where only some members have a query tag
+	type Struct struct {
+		Foo string `query:"foo"`
+		Bar string `json:"bar"`
+	}
+	v := &Struct{Foo: "value", Bar: "value"}
+	expectedQueryString := `foo=value`
+
+	req, err := http.NewRequest("GET", "https://example.com", nil)
+	if err != nil {
+		t.Error("request creation failed")
+	}
+
+	q, err := buildQueryString(req, v)
+	if err != nil {
+		t.Errorf("expected buildQueryString to not error, got \"%s\"", err)
+	}
+	if q != expectedQueryString {
+		t.Errorf(`expected q to be "%s", got "%s"`, expectedQueryString, q)
+	}
+}
+
+func TestQueryStringBuilderNoQuery(t *testing.T) {
+	t.Parallel()
+
+	// For structs where no members have query tags
+	type Struct struct {
+		Foo string `json:"foo"`
+		Bar string `json:"bar"`
+	}
+	v := &Struct{Foo: "value", Bar: "value"}
+	expectedQueryString := ``
+
+	req, err := http.NewRequest("GET", "https://example.com", nil)
+	if err != nil {
+		t.Error("request creation failed")
+	}
+
+	q, err := buildQueryString(req, v)
+	if err != nil {
+		t.Errorf("expected buildQueryString to not error, got \"%s\"", err)
+	}
+	if q != expectedQueryString {
+		t.Errorf(`expected q to be "%s", got "%s"`, expectedQueryString, q)
+	}
+}