diff --git a/github/actions_cache.go b/github/actions_cache.go index 852e9860f1c..c883effb900 100644 --- a/github/actions_cache.go +++ b/github/actions_cache.go @@ -114,7 +114,7 @@ func (s *ActionsService) ListCaches(ctx context.Context, owner, repo string, opt //meta:operation DELETE /repos/{owner}/{repo}/actions/caches func (s *ActionsService) DeleteCachesByKey(ctx context.Context, owner, repo, key string, ref *string) (*Response, error) { u := fmt.Sprintf("repos/%v/%v/actions/caches", owner, repo) - u, err := addOptions(u, ActionsCache{Key: &key, Ref: ref}) + u, err := addOptions(u, &ActionsCache{Key: &key, Ref: ref}) if err != nil { return nil, err } diff --git a/github/github.go b/github/github.go index 0ebeaee96be..4abaa7c01b1 100644 --- a/github/github.go +++ b/github/github.go @@ -19,7 +19,6 @@ import ( "io" "net/http" "net/url" - "reflect" "regexp" "strconv" "strings" @@ -312,11 +311,12 @@ type RawOptions struct { Type RawType } +type structPtr[T any] interface{ *T } + // addOptions adds the parameters in opts as URL query parameters to s. opts // must be a struct whose fields may contain "url" tags. -func addOptions(s string, opts any) (string, error) { - v := reflect.ValueOf(opts) - if v.Kind() == reflect.Pointer && v.IsNil() { +func addOptions[P structPtr[T], T any](s string, opts P) (string, error) { + if opts == nil { return s, nil } diff --git a/github/github_test.go b/github/github_test.go index e9409acdec3..a43eb7764a7 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -187,33 +187,6 @@ func testJSONMarshal(t *testing.T, v any, want string) { } } -// Test whether the v fields have the url tag and the parsing of v -// produces query parameters that corresponds to the want string. -func testAddURLOptions(t *testing.T, url string, v any, want string) { - t.Helper() - - vt := reflect.Indirect(reflect.ValueOf(v)).Type() - for i := range vt.NumField() { - field := vt.Field(i) - if alias, ok := field.Tag.Lookup("url"); ok { - if alias == "" { - t.Errorf("The field %+v has a blank url tag", field) - } - } else { - t.Errorf("The field %+v has no url tag specified", field) - } - } - - got, err := addOptions(url, v) - if err != nil { - t.Errorf("Unable to add %#v as query parameters", v) - } - - if got != want { - t.Errorf("addOptions(%q, %#v) returned %v, want %v", url, v, got, want) - } -} - // Test how bad options are handled. Method f under test should // return an error. func testBadOptions(t *testing.T, methodName string, f func() error) { @@ -3055,13 +3028,6 @@ func TestAbuseRateLimitError(t *testing.T) { } } -func TestAddOptions_QueryValues(t *testing.T) { - t.Parallel() - if _, err := addOptions("yo", ""); err == nil { - t.Error("addOptions err = nil, want error") - } -} - func TestBareDo_returnsOpenBody(t *testing.T) { t.Parallel() client, mux, _ := setup(t) diff --git a/github/scim_test.go b/github/scim_test.go index bde81493b27..3b092427ccd 100644 --- a/github/scim_test.go +++ b/github/scim_test.go @@ -20,6 +20,7 @@ func TestSCIMService_ListSCIMProvisionedIdentities(t *testing.T) { mux.HandleFunc("/scim/v2/organizations/o/Users", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") + testFormValues(t, r, values{"startIndex": "1", "count": "10", "filter": `userName="Octocat"`}) w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{ "schemas": [ @@ -61,7 +62,11 @@ func TestSCIMService_ListSCIMProvisionedIdentities(t *testing.T) { }) ctx := t.Context() - opts := &ListSCIMProvisionedIdentitiesOptions{} + opts := &ListSCIMProvisionedIdentitiesOptions{ + StartIndex: Ptr(1), + Count: Ptr(10), + Filter: Ptr(`userName="Octocat"`), + } identities, _, err := client.SCIM.ListSCIMProvisionedIdentities(ctx, "o", opts) if err != nil { t.Errorf("SCIM.ListSCIMProvisionedIdentities returned error: %v", err) @@ -115,7 +120,7 @@ func TestSCIMService_ListSCIMProvisionedIdentities(t *testing.T) { }) testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { - _, r, err := client.SCIM.ListSCIMProvisionedIdentities(ctx, "o", opts) + _, r, err := client.SCIM.ListSCIMProvisionedIdentities(ctx, "o", nil) return r, err }) } @@ -444,40 +449,6 @@ func TestUpdateAttributeForSCIMUserOptions_Marshal(t *testing.T) { testJSONMarshal(t, u, want) } -func TestListSCIMProvisionedIdentitiesOptions_addOptions(t *testing.T) { - t.Parallel() - testJSONMarshal(t, &ListSCIMProvisionedIdentitiesOptions{}, `{ - "StartIndex": null, - "Count": null, - "Filter": null - }`) - - url := "some/path" - - testAddURLOptions(t, url, &ListSCIMProvisionedIdentitiesOptions{}, url) - - testAddURLOptions( - t, - url, - &ListSCIMProvisionedIdentitiesOptions{ - StartIndex: Ptr(1), - Count: Ptr(10), - }, - fmt.Sprintf("%v?count=10&startIndex=1", url), - ) - - testAddURLOptions( - t, - url, - &ListSCIMProvisionedIdentitiesOptions{ - StartIndex: Ptr(1), - Count: Ptr(10), - Filter: Ptr("test"), - }, - fmt.Sprintf("%v?count=10&filter=test&startIndex=1", url), - ) -} - func TestSCIMUserName_Marshal(t *testing.T) { t.Parallel() testJSONMarshal(t, &SCIMUserName{}, `{