Skip to content

Commit

Permalink
Merge pull request #245 from manisharma/panic-fix
Browse files Browse the repository at this point in the history
method.IsNil() panics and the actual error is never returned
  • Loading branch information
baywet authored Nov 24, 2023
2 parents c73cc93 + 471e50b commit 1eda545
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 1 deletion.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

## [1.0.1] - 2023-11-24

### Changed

- Fixed a bug where page iterator would panic if it couldn't find the GetValue method on the collection.

## [1.0.0] - 2023-05-04

### Changed
Expand Down
2 changes: 1 addition & 1 deletion page_iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func convertToPage[T interface{}](response interface{}) (PageResult[T], error) {
}

method := reflect.ValueOf(response).MethodByName("GetValue")
if method.IsNil() {
if method.Kind() == reflect.Invalid {
return page, errors.New("value property missing in response object")
}
value := method.Call(nil)[0]
Expand Down
129 changes: 129 additions & 0 deletions page_iterator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,3 +261,132 @@ func buildGraphResponse() *internal.UsersResponse {

return res
}

func Test_convertToPage(t *testing.T) {
type args struct {
response interface{}
}
tests := []struct {
name string
args args
want PageResult[validTestStruct]
wantErr bool
}{
{
name: "should pass",
args: args{
response: &validTestStruct{
obj: []validTestStruct{
{
obj: []validTestStruct{
{
obj: []validTestStruct{},
},
},
},
}},
},
want: PageResult[validTestStruct]{
oDataNextLink: nil,
oDataDeltaLink: nil,
value: []validTestStruct{
{
obj: []validTestStruct{
{
obj: []validTestStruct{},
},
},
},
},
},
},
{
name: "should return error 'saying response cannot be nil' for nil response",
args: args{
response: nil,
},
want: PageResult[validTestStruct]{
oDataNextLink: nil,
oDataDeltaLink: nil,
value: nil,
},
wantErr: true,
},
{
name: "should return error 'value property missing in response object' for missing 'GetValue' method",
args: args{
response: &invalidTestStruct{
obj: []invalidTestStruct{
{
obj: []invalidTestStruct{
{
obj: []invalidTestStruct{},
},
},
},
}},
},
want: PageResult[validTestStruct]{
oDataNextLink: nil,
oDataDeltaLink: nil,
},
wantErr: true,
},
{
name: "should return error 'response does not have next link accessor' for missing 'GetOdataNextLink() *string' method",
args: args{
response: &invalidTestStruct{
obj: []invalidTestStruct{
{
obj: []invalidTestStruct{
{
obj: []invalidTestStruct{},
},
},
},
}},
},
want: PageResult[validTestStruct]{
oDataNextLink: nil,
oDataDeltaLink: nil,
},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := convertToPage[validTestStruct](tt.args.response)
if (err != nil) != tt.wantErr {
t.Errorf("convertToPage() error = %v, wantErr %v", err, tt.wantErr)
return
}
assert.Equal(t, tt.want.oDataDeltaLink, got.oDataDeltaLink, "got %v, want %v", got.oDataNextLink, tt.want.oDataNextLink)
assert.Equal(t, tt.want.oDataNextLink, got.oDataNextLink, "got %v, want %v", got.oDataDeltaLink, tt.want.oDataDeltaLink)
assert.Equal(t, tt.want.value, got.value, "got %v, want %v", got.value, tt.want.value)
})
}
}

type validTestStruct struct {
obj []validTestStruct
}

func (t *validTestStruct) GetValue() []validTestStruct {
return t.obj
}

func (t *validTestStruct) GetOdataNextLink() *string {
return nil
}

func (t *validTestStruct) GetOdataDeltaLink() *string {
return nil
}

type invalidTestStruct struct {
obj []invalidTestStruct
}

func (t *invalidTestStruct) GetOdataDeltaLink() *string {
return nil
}

0 comments on commit 1eda545

Please sign in to comment.