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

Ft page iterator #13 #27

Merged
merged 31 commits into from
Jan 27, 2022
Merged

Ft page iterator #13 #27

merged 31 commits into from
Jan 27, 2022

Conversation

jobala
Copy link
Contributor

@jobala jobala commented Jan 17, 2022

Overview

Adds PageIterator

Demo

Create a page iterator task

pageIterator, err := msgraphgocore.NewPageIterator(graphResponse, *reqAdapter, ParsableCons)
pageIterator.SetHeaders(map[string]string{"Content-Type": "application/json"})

Iterate

err := pageIterator.Iterate(func(pageItem interface{}) bool {
      item := pageItem.(graph.User)
      res = append(res, *item.GetId())
      return true
})

Pause & Resume

pageIterator.Iterate(func(pageItem interface{}) bool {
      item := pageItem.(graph.User)
      res = append(res, *item.GetId())
      
      return *item.GetId() != "2" // pauses when id == 2
})

// resumes iteration from user with id 3
pageIterator.Iterate(func(pageItem interface{}) bool {
      item := pageItem.(graph.User)
      res2 = append(res2, *item.GetId())
      
      return true
})

Notes

  • I made the Next & HasNext methods private because we have not reached a consensus on making them part of the Iterator's API

Testing

  • Run go test

Closes #13

Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

good first draft, added a few comments.

page_iterator.go Outdated Show resolved Hide resolved
page_iterator.go Outdated Show resolved Hide resolved
page_iterator.go Outdated Show resolved Hide resolved
page_iterator.go Outdated Show resolved Hide resolved
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

almost there! some minor changes. Don't hesitate to switch the PR to ready whenever it's ready.

page_iterator.go Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
@baywet
Copy link
Member

baywet commented Jan 21, 2022

also CC @jasonjoh for the public docs update of this page

@jobala can you update the snippets in your initial comment with the latest changes so Jason has visibility please?

@jobala jobala marked this pull request as ready for review January 22, 2022 13:24
internal/user.go Show resolved Hide resolved
msgraphgocore_test/graph_odata_query_handler_test.go Outdated Show resolved Hide resolved
page_iterator.go Show resolved Hide resolved
page_iterator.go Outdated Show resolved Hide resolved
page_iterator.go Outdated Show resolved Hide resolved
page_iterator.go Outdated Show resolved Hide resolved
page_iterator.go Show resolved Hide resolved
page_iterator.go Show resolved Hide resolved
page_iterator.go Show resolved Hide resolved
page_iterator.go Show resolved Hide resolved
page_iterator.go Show resolved Hide resolved
page_iterator.go Outdated Show resolved Hide resolved
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

thanks for making the changes! only two docs comments missing and we should be good to go.
I also configured sonarcloud to exclude internal package from the metrics so it reflects the actual coverage.

page_iterator.go Outdated Show resolved Hide resolved
page_iterator.go Show resolved Hide resolved
@baywet baywet added this to the Core specification alignment milestone Jan 27, 2022
@baywet
Copy link
Member

baywet commented Jan 27, 2022

@sonarcloud
Copy link

sonarcloud bot commented Jan 27, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

82.6% 82.6% Coverage
0.0% 0.0% Duplication

Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

thanks for the excellent work!

@baywet baywet merged commit ec4b64e into main Jan 27, 2022
@baywet baywet deleted the ft-page-iterator-#13 branch January 27, 2022 14:17
@jobala jobala self-assigned this Feb 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add page iterator
2 participants