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

MLIBZ-2304: making the page size adjustable for auto pagination #259

Merged
merged 2 commits into from Jan 12, 2018

Conversation

Projects
None yet
2 participants
@heyzooi
Contributor

heyzooi commented Jan 11, 2018

Description

Be able to change the default value of 10k for the max page size

Changes

  • Added new property for Options
  • FindOperation now checks if a value was provided, otherwise uses the default 10k value

Tests

  • Unit test adjusted to cover the use case

@heyzooi heyzooi self-assigned this Jan 11, 2018

@heyzooi heyzooi requested a review from tejasranade Jan 11, 2018

@@ -102,17 +102,18 @@ internal class FindOperation<T: Persistable>: ReadOperation<T, AnyRandomAccessCo
private func fetchAutoPagination(multiRequest: MultiRequest, count: Int) -> Promise<AnyRandomAccessCollection<T>> {
return Promise<AnyRandomAccessCollection<T>> { fulfill, reject in
let nPages = Int64(ceil(Double(count) / Double(MaxSizePerResultSet)))
let maxSizePerResultSet = options?.maxSizePerResultSet ?? MaxSizePerResultSet

This comment has been minimized.

@tejasranade

tejasranade Jan 11, 2018

Member

Corner case, but does zero cause a problem?

@tejasranade

tejasranade Jan 11, 2018

Member

Corner case, but does zero cause a problem?

This comment has been minimized.

@heyzooi

heyzooi Jan 12, 2018

Contributor

good question, also negative values should not be allowed. I will add some protection to avoid this scenario

@heyzooi

heyzooi Jan 12, 2018

Contributor

good question, also negative values should not be allowed. I will add some protection to avoid this scenario

@@ -2512,10 +2520,10 @@ class NetworkStoreTests: StoreTestCase {
weak var expectationFind = expectation(description: "Find")
store.find(options: Options(readPolicy: .forceNetwork)) { (result: Result<AnyRandomAccessCollection<Products>, Swift.Error>) in
store.find(options: Options(readPolicy: .forceNetwork, maxSizePerResultSet: pageSizeLimit)) { (result: Result<AnyRandomAccessCollection<Products>, Swift.Error>) in

This comment has been minimized.

@tejasranade

tejasranade Jan 11, 2018

Member

I'm confused, where is the code that adds this to Options? What is the relation between Options and RequestFactory?

@tejasranade

tejasranade Jan 11, 2018

Member

I'm confused, where is the code that adds this to Options? What is the relation between Options and RequestFactory?

This comment has been minimized.

@heyzooi

heyzooi Jan 12, 2018

Contributor

the Options struct is in the same file of RequestFactory. We can create a separate file for Options as well. I will do that.

@heyzooi

heyzooi Jan 12, 2018

Contributor

the Options struct is in the same file of RequestFactory. We can create a separate file for Options as well. I will do that.

This comment has been minimized.

@tejasranade

tejasranade Jan 12, 2018

Member

Ah, I missed that. Better now!

@tejasranade

tejasranade Jan 12, 2018

Member

Ah, I missed that. Better now!

public var maxSizePerResultSet: Int? {
willSet {
if let newValue = newValue, newValue <= 0 {
fatalError("maxSizePerResultSet must be greater than 0 (zero)")

This comment has been minimized.

@heyzooi

heyzooi Jan 12, 2018

Contributor

@tejasranade validation to avoid negative values or zero values

@heyzooi

heyzooi Jan 12, 2018

Contributor

@tejasranade validation to avoid negative values or zero values

@@ -2512,10 +2520,10 @@ class NetworkStoreTests: StoreTestCase {
weak var expectationFind = expectation(description: "Find")
store.find(options: Options(readPolicy: .forceNetwork)) { (result: Result<AnyRandomAccessCollection<Products>, Swift.Error>) in
store.find(options: Options(readPolicy: .forceNetwork, maxSizePerResultSet: pageSizeLimit)) { (result: Result<AnyRandomAccessCollection<Products>, Swift.Error>) in

This comment has been minimized.

@tejasranade

tejasranade Jan 12, 2018

Member

Ah, I missed that. Better now!

@tejasranade

tejasranade Jan 12, 2018

Member

Ah, I missed that. Better now!

@heyzooi heyzooi merged commit 9bf4002 into develop Jan 12, 2018

0 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
codebeat 0 issues resolved and 1 introduced
Details

@heyzooi heyzooi deleted the feature/MLIBZ-2304-adjustable_page_size branch Jan 12, 2018

heyzooi added a commit that referenced this pull request Jan 18, 2018

MLIBZ-2304: making the page size adjustable for auto pagination (#259)
* MLIBZ-2304: making the page size adjustable for auto pagination

* creating a separate file for the Options struct
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment