Skip to content

Commit

Permalink
feat(pagination): smart pagination (#479)
Browse files Browse the repository at this point in the history
* fix(paginate): smart paginate our results

* fix(paginate): smart paginate our results

* fix(pagination): ensuring we pagintate smartly

* fix(tests): updating pagination tests

* fix(archive): load archive on login

* fix(pagination): fix some calls

* fix(pagination): fix tests
  • Loading branch information
bassrock committed Mar 10, 2023
1 parent 383dbb1 commit 14c0d02
Show file tree
Hide file tree
Showing 23 changed files with 4,225 additions and 302 deletions.
7 changes: 7 additions & 0 deletions Pocket.xcodeproj/xcshareddata/xcschemes/Pocket (iOS).xcscheme
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,13 @@
isEnabled = "NO">
</EnvironmentVariable>
</EnvironmentVariables>
<AdditionalOptions>
<AdditionalOption
key = "NSZombieEnabled"
value = "YES"
isEnabled = "YES">
</AdditionalOption>
</AdditionalOptions>
</LaunchAction>
<ProfileAction
buildConfiguration = "Release"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class RefreshCoordinator {
}

func initialize() {
_ = taskScheduler.registerHandler(forTaskWithIdentifier: Self.taskID, using: .main) { [weak self] task in
_ = taskScheduler.registerHandler(forTaskWithIdentifier: Self.taskID, using: .global(qos: .background)) { [weak self] task in
self?.refresh(task)
self?.submitRequest()
}
Expand Down
1 change: 1 addition & 0 deletions PocketKit/Sources/PocketKit/Root/RootViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ class RootViewModel {
tracker.addPersistentEntity(UserEntity(guid: session.guid, userID: session.userIdentifier))
Log.setUserID(session.userIdentifier)
source.refreshSaves()
source.refreshArchive()
}

private func tearDownSession() {
Expand Down
26 changes: 13 additions & 13 deletions PocketKit/Sources/Sync/Operations/FetchArchive.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,18 @@ class FetchArchive: SyncOperation {
private let space: Space
private let events: SyncEvents
private let initialDownloadState: CurrentValueSubject<InitialDownloadState, Never>
private let maxItems: Int
private let lastRefresh: LastRefresh

init(
apollo: ApolloClientProtocol,
space: Space,
events: SyncEvents,
initialDownloadState: CurrentValueSubject<InitialDownloadState, Never>,
maxItems: Int,
lastRefresh: LastRefresh
) {
self.apollo = apollo
self.space = space
self.events = events
self.maxItems = maxItems
self.lastRefresh = lastRefresh
self.initialDownloadState = initialDownloadState
}
Expand Down Expand Up @@ -64,19 +61,19 @@ class FetchArchive: SyncOperation {
}

private func fetchArchive() async throws {
var pagination = PaginationSpec(maxItems: maxItems)
var pagination = PaginationSpec(maxItems: SyncConstants.Archive.firstLoadMaxCount, pageSize: SyncConstants.Archive.initalPageSize)

repeat {
let result = try await fetchPage(pagination)

if case .started = initialDownloadState.value,
let totalCount = result.data?.user?.savedItems?.totalCount,
pagination.cursor == nil {
initialDownloadState.send(.paginating(totalCount: totalCount))
initialDownloadState.send(.paginating(totalCount: min(totalCount, pagination.maxItems)))
}

try await updateLocalStorage(result: result)
pagination = pagination.nextPage(result: result)
pagination = pagination.nextPage(result: result, pageSize: SyncConstants.Archive.pageSize)
} while pagination.shouldFetchNextPage

initialDownloadState.send(.completed)
Expand All @@ -86,7 +83,7 @@ class FetchArchive: SyncOperation {
let query = FetchArchiveQuery(
pagination: .some(PaginationInput(
after: pagination.cursor ?? .none,
first: .some(pagination.maxItems)
first: .some(pagination.pageSize)
)),
filter: .none,
sort: .some(SavedItemsSort(sortBy: .init(.archivedAt), sortOrder: .init(.desc)))
Expand Down Expand Up @@ -134,28 +131,31 @@ class FetchArchive: SyncOperation {
let cursor: String?
let shouldFetchNextPage: Bool
let maxItems: Int
let pageSize: Int

init(maxItems: Int) {
self.init(cursor: nil, shouldFetchNextPage: false, maxItems: maxItems)
init(maxItems: Int, pageSize: Int) {
self.init(cursor: nil, shouldFetchNextPage: false, maxItems: maxItems, pageSize: pageSize)
}

private init(cursor: String?, shouldFetchNextPage: Bool, maxItems: Int) {
private init(cursor: String?, shouldFetchNextPage: Bool, maxItems: Int, pageSize: Int) {
self.cursor = cursor
self.shouldFetchNextPage = shouldFetchNextPage
self.maxItems = maxItems
self.pageSize = pageSize
}

func nextPage(result: GraphQLResult<FetchArchiveQuery.Data>) -> PaginationSpec {
func nextPage(result: GraphQLResult<FetchArchiveQuery.Data>, pageSize: Int) -> PaginationSpec {
guard let savedItems = result.data?.user?.savedItems,
let itemCount = savedItems.edges?.count,
let endCursor = savedItems.pageInfo.endCursor else {
return PaginationSpec(cursor: nil, shouldFetchNextPage: false, maxItems: maxItems)
return PaginationSpec(cursor: nil, shouldFetchNextPage: false, maxItems: maxItems, pageSize: pageSize)
}

return PaginationSpec(
cursor: endCursor,
shouldFetchNextPage: savedItems.pageInfo.hasNextPage && itemCount < maxItems,
maxItems: maxItems - itemCount
maxItems: maxItems - itemCount,
pageSize: pageSize
)
}
}
Expand Down
30 changes: 15 additions & 15 deletions PocketKit/Sources/Sync/Operations/FetchSaves.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ class FetchSaves: SyncOperation {
private let space: Space
private let events: SyncEvents
private let initialDownloadState: CurrentValueSubject<InitialDownloadState, Never>
private let maxItems: Int
private let lastRefresh: LastRefresh

init(
Expand All @@ -19,21 +18,19 @@ class FetchSaves: SyncOperation {
space: Space,
events: SyncEvents,
initialDownloadState: CurrentValueSubject<InitialDownloadState, Never>,
maxItems: Int,
lastRefresh: LastRefresh
) {
self.user = user
self.apollo = apollo
self.space = space
self.events = events
self.maxItems = maxItems
self.lastRefresh = lastRefresh
self.initialDownloadState = initialDownloadState
}

func execute() async -> SyncOperationResult {
do {
try await fetchList()
try await fetchSaves()
try await fetchTags()

lastRefresh.refreshedSaves()
Expand Down Expand Up @@ -67,8 +64,8 @@ class FetchSaves: SyncOperation {
}
}

private func fetchList() async throws {
var pagination = PaginationSpec(maxItems: maxItems)
private func fetchSaves() async throws {
var pagination = PaginationSpec(maxItems: SyncConstants.Saves.firstLoadMaxCount, pageSize: SyncConstants.Saves.initalPageSize)

repeat {
let result = try await fetchPage(pagination)
Expand All @@ -79,11 +76,11 @@ class FetchSaves: SyncOperation {
if case .started = initialDownloadState.value,
let totalCount = result.data?.user?.savedItems?.totalCount,
pagination.cursor == nil {
initialDownloadState.send(.paginating(totalCount: totalCount))
initialDownloadState.send(.paginating(totalCount: min(totalCount, pagination.maxItems)))
}

try await updateLocalStorage(result: result)
pagination = pagination.nextPage(result: result)
pagination = pagination.nextPage(result: result, pageSize: SyncConstants.Saves.pageSize)
} while pagination.shouldFetchNextPage

initialDownloadState.send(.completed)
Expand Down Expand Up @@ -111,7 +108,7 @@ class FetchSaves: SyncOperation {
let query = FetchSavesQuery(
pagination: .some(PaginationInput(
after: pagination.cursor ?? .none,
first: .some(pagination.maxItems)
first: .some(pagination.pageSize)
)),
savedItemsFilter: .none
)
Expand Down Expand Up @@ -168,28 +165,31 @@ class FetchSaves: SyncOperation {
let cursor: String?
let shouldFetchNextPage: Bool
let maxItems: Int
let pageSize: Int

init(maxItems: Int) {
self.init(cursor: nil, shouldFetchNextPage: false, maxItems: maxItems)
init(maxItems: Int, pageSize: Int) {
self.init(cursor: nil, shouldFetchNextPage: false, maxItems: maxItems, pageSize: pageSize)
}

private init(cursor: String?, shouldFetchNextPage: Bool, maxItems: Int) {
private init(cursor: String?, shouldFetchNextPage: Bool, maxItems: Int, pageSize: Int) {
self.cursor = cursor
self.shouldFetchNextPage = shouldFetchNextPage
self.maxItems = maxItems
self.pageSize = pageSize
}

func nextPage(result: GraphQLResult<FetchSavesQuery.Data>) -> PaginationSpec {
func nextPage(result: GraphQLResult<FetchSavesQuery.Data>, pageSize: Int) -> PaginationSpec {
guard let savedItems = result.data?.user?.savedItems,
let itemCount = savedItems.edges?.count,
let endCursor = savedItems.pageInfo.endCursor else {
return PaginationSpec(cursor: nil, shouldFetchNextPage: false, maxItems: maxItems)
return PaginationSpec(cursor: nil, shouldFetchNextPage: false, maxItems: maxItems, pageSize: pageSize)
}

return PaginationSpec(
cursor: endCursor,
shouldFetchNextPage: savedItems.pageInfo.hasNextPage && itemCount < maxItems,
maxItems: maxItems - itemCount
maxItems: maxItems - itemCount,
pageSize: pageSize
)
}
}
Expand Down
6 changes: 0 additions & 6 deletions PocketKit/Sources/Sync/Operations/SyncOperationFactory.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ protocol SyncOperationFactory {
space: Space,
events: SyncEvents,
initialDownloadState: CurrentValueSubject<InitialDownloadState, Never>,
maxItems: Int,
lastRefresh: LastRefresh
) -> SyncOperation

Expand All @@ -21,7 +20,6 @@ protocol SyncOperationFactory {
space: Space,
events: SyncEvents,
initialDownloadState: CurrentValueSubject<InitialDownloadState, Never>,
maxItems: Int,
lastRefresh: LastRefresh
) -> SyncOperation

Expand Down Expand Up @@ -53,7 +51,6 @@ class OperationFactory: SyncOperationFactory {
space: Space,
events: SyncEvents,
initialDownloadState: CurrentValueSubject<InitialDownloadState, Never>,
maxItems: Int,
lastRefresh: LastRefresh
) -> SyncOperation {
return FetchSaves(
Expand All @@ -62,7 +59,6 @@ class OperationFactory: SyncOperationFactory {
space: space,
events: events,
initialDownloadState: initialDownloadState,
maxItems: maxItems,
lastRefresh: lastRefresh
)
}
Expand All @@ -72,15 +68,13 @@ class OperationFactory: SyncOperationFactory {
space: Space,
events: SyncEvents,
initialDownloadState: CurrentValueSubject<InitialDownloadState, Never>,
maxItems: Int,
lastRefresh: LastRefresh
) -> SyncOperation {
return FetchArchive(
apollo: apollo,
space: space,
events: events,
initialDownloadState: initialDownloadState,
maxItems: maxItems,
lastRefresh: lastRefresh
)
}
Expand Down
4 changes: 2 additions & 2 deletions PocketKit/Sources/Sync/Operations/SyncTask.swift
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import Foundation

enum SyncTask: Codable {
case fetchSaves(maxItems: Int)
case fetchArchive(maxItems: Int)
case fetchSaves
case fetchArchive
case favorite(remoteID: String)
case unfavorite(remoteID: String)
case delete(remoteID: String)
Expand Down
16 changes: 6 additions & 10 deletions PocketKit/Sources/Sync/PocketSource.swift
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ public class PocketSource: Source {

// MARK: - Saves/Archive items
extension PocketSource {
public func refreshSaves(maxItems: Int = 400, completion: (() -> Void)? = nil) {
public func refreshSaves(completion: (() -> Void)? = nil) {
if lastRefresh.lastRefreshSaves == nil {
initialSavesDownloadState.send(.started)
}
Expand All @@ -215,14 +215,13 @@ extension PocketSource {
space: space,
events: _events,
initialDownloadState: initialSavesDownloadState,
maxItems: maxItems,
lastRefresh: lastRefresh
)

enqueue(operation: operation, task: .fetchSaves(maxItems: maxItems), completion: completion)
enqueue(operation: operation, task: .fetchSaves, completion: completion)
}

public func refreshArchive(maxItems: Int = 400, completion: (() -> Void)? = nil) {
public func refreshArchive(completion: (() -> Void)? = nil) {
if lastRefresh.lastRefreshArchive == nil {
initialArchiveDownloadState.send(.started)
}
Expand All @@ -232,11 +231,10 @@ extension PocketSource {
space: space,
events: _events,
initialDownloadState: initialArchiveDownloadState,
maxItems: maxItems,
lastRefresh: lastRefresh
)

enqueue(operation: operation, task: .fetchSaves(maxItems: maxItems), completion: completion)
enqueue(operation: operation, task: .fetchSaves, completion: completion)
}

public func favorite(item: SavedItem) {
Expand Down Expand Up @@ -516,24 +514,22 @@ extension PocketSource {
mutation: FavoriteItemMutation(itemID: remoteID)
)
enqueue(operation: operation, persistentTask: persistentTask)
case .fetchSaves(let maxItems):
case .fetchSaves:
let operation = operations.fetchSaves(
user: user,
apollo: apollo,
space: space,
events: _events,
initialDownloadState: initialSavesDownloadState,
maxItems: maxItems,
lastRefresh: lastRefresh
)
enqueue(operation: operation, persistentTask: persistentTask)
case .fetchArchive(let maxItems):
case .fetchArchive:
let operation = operations.fetchArchive(
apollo: apollo,
space: space,
events: _events,
initialDownloadState: initialArchiveDownloadState,
maxItems: maxItems,
lastRefresh: lastRefresh
)
enqueue(operation: operation, persistentTask: persistentTask)
Expand Down
4 changes: 2 additions & 2 deletions PocketKit/Sources/Sync/Slates/SlateService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class APISlateService: SlateService {
}

func fetchSlateLineup(_ identifier: String) async throws {
let query = GetSlateLineupQuery(lineupID: identifier, maxRecommendations: 5)
let query = GetSlateLineupQuery(lineupID: identifier, maxRecommendations: SyncConstants.Home.recomendationsPerSlateFromSlateLineup)

guard let remote = try await apollo.fetch(query: query).data?.getSlateLineup else {
Log.capture(message: "Error loading slate lineup")
Expand All @@ -32,7 +32,7 @@ class APISlateService: SlateService {
}

func fetchSlate(_ slateID: String) async throws {
let query = GetSlateQuery(slateID: slateID, recommendationCount: 25)
let query = GetSlateQuery(slateID: slateID, recommendationCount: SyncConstants.Home.recomendationsPerSlateDetail)

guard let remote = try await apollo.fetch(query: query)
.data?.getSlate.fragments.slateParts else {
Expand Down
4 changes: 2 additions & 2 deletions PocketKit/Sources/Sync/Source+async.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
extension Source {
public func refresh(maxItems: Int = 400) async {
public func refresh() async {
await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) in
refreshSaves(maxItems: maxItems) {
refreshSaves {
continuation.resume(returning: ())
}
}
Expand Down
Loading

0 comments on commit 14c0d02

Please sign in to comment.