Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SCIM query params #2680

Merged
merged 7 commits into from
Mar 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,33 @@ func testJSONMarshal(t *testing.T, v interface{}, 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 interface{}, want string) {
t.Helper()

vt := reflect.Indirect(reflect.ValueOf(v)).Type()
for i := 0; i < vt.NumField(); i++ {
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) {
Expand Down
6 changes: 3 additions & 3 deletions github/scim.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,14 @@ type SCIMProvisionedIdentities struct {
//
// Github API docs: https://docs.github.com/en/rest/scim#list-scim-provisioned-identities--parameters
type ListSCIMProvisionedIdentitiesOptions struct {
StartIndex *int `json:"startIndex,omitempty"` // Used for pagination: the index of the first result to return. (Optional.)
Count *int `json:"count,omitempty"` // Used for pagination: the number of results to return. (Optional.)
StartIndex *int `url:"startIndex,omitempty"` // Used for pagination: the index of the first result to return. (Optional.)
Count *int `url:"count,omitempty"` // Used for pagination: the number of results to return. (Optional.)
// Filter results using the equals query parameter operator (eq).
// You can filter results that are equal to id, userName, emails, and external_id.
// For example, to search for an identity with the userName Octocat, you would use this query: ?filter=userName%20eq%20\"Octocat\".
// To filter results for the identity with the email [email protected], you would use this query: ?filter=emails%20eq%20\"[email protected]\".
// (Optional.)
Filter *string `json:"filter,omitempty"`
Filter *string `url:"filter,omitempty"`
}

// ListSCIMProvisionedIdentities lists SCIM provisioned identities.
Expand Down
42 changes: 29 additions & 13 deletions github/scim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package github

import (
"context"
"fmt"
"net/http"
"testing"
"time"
Expand Down Expand Up @@ -428,22 +429,37 @@ func TestUpdateAttributeForSCIMUserOptions_Marshal(t *testing.T) {
testJSONMarshal(t, u, want)
}

func TestListSCIMProvisionedIdentitiesOptions_Marshal(t *testing.T) {
testJSONMarshal(t, &ListSCIMProvisionedIdentitiesOptions{}, `{}`)
func TestListSCIMProvisionedIdentitiesOptions_addOptions(t *testing.T) {
testJSONMarshal(t, &ListSCIMProvisionedIdentitiesOptions{}, `{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, now I understand why you removed this before, as it really is no longer relevant.
Feel free to delete this if you wish, either in this PR or in your upcoming one.

"StartIndex": null,
"Count": null,
"Filter": null
}`)

u := &ListSCIMProvisionedIdentitiesOptions{
StartIndex: Int(1),
Count: Int(10),
Filter: String("test"),
}
url := "some/path"

want := `{
"startIndex": 1,
"count": 10,
"filter": "test"
}`
testAddURLOptions(t, url, &ListSCIMProvisionedIdentitiesOptions{}, url)

testJSONMarshal(t, u, want)
testAddURLOptions(
t,
url,
&ListSCIMProvisionedIdentitiesOptions{
StartIndex: Int(1),
Count: Int(10),
},
fmt.Sprintf("%s?count=10&startIndex=1", url),
)

testAddURLOptions(
t,
url,
&ListSCIMProvisionedIdentitiesOptions{
StartIndex: Int(1),
Count: Int(10),
Filter: String("test"),
},
fmt.Sprintf("%s?count=10&filter=test&startIndex=1", url),
)
}

func TestSCIMUserName_Marshal(t *testing.T) {
Expand Down