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.

Sign up for GitHub

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

Implement fetching posts/pages list in PostRepository #21814

Merged
merged 6 commits into from
Oct 19, 2023
Merged

Implement fetching posts/pages list in PostRepository #21814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a0c6388
Implement fetching posts/pages list in PostRepository
crazytonyli Oct 18, 2023
4f8178a
Add API docs
crazytonyli Oct 18, 2023
fa641ec
Allow fetching post in empty range which means returning unlimited re…
crazytonyli Oct 18, 2023
ed47e2b
Update the doc to fix incorrect REST API behaviour
crazytonyli Oct 18, 2023
c980a30
Add unit tests for fetching posts list
crazytonyli Oct 18, 2023
4dac5b7
Use map instead of compactMap
crazytonyli Oct 19, 2023
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
202 changes: 202 additions & 0 deletions WordPress/Classes/Services/PostRepository.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,205 @@ final class PostRepository {
}

}

// MARK: - Posts/Pages List

private final class PostRepositoryPostsSerivceRemoteOptions: NSObject, PostServiceRemoteOptions {
struct Options {
var statuses: [String]?
var number: Int = 100
var offset: Int = 0
var order: PostServiceResultsOrder = .descending
var orderBy: PostServiceResultsOrdering = .byDate
var authorID: NSNumber?
var search: String?
var meta: String? = "autosave"
}

var options: Options

init(options: Options) {
self.options = options
}

func statuses() -> [String]? {
options.statuses
}
Comment on lines +252 to +254
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you considered making this and the other computed vars?

I don't know if there's a recommendation somewhere. It might just be my personal preference, but I find that this kind of getters read better as properties than functions. I guess methods give me the idea that something is happening under the hood, whereas properties feel more like reading information.

Update: Never mind... This conforms to the PostServiceRemoteOptions protocol (a protocol that should be a struct by now...) and therefore they need to be func:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, Objective-C...


func number() -> NSNumber {
NSNumber(value: options.number)
}

func offset() -> NSNumber {
NSNumber(value: options.offset)
}

func order() -> PostServiceResultsOrder {
options.order
}

func orderBy() -> PostServiceResultsOrdering {
options.orderBy
}

func authorID() -> NSNumber? {
options.authorID
}

func search() -> String? {
options.search
}

func meta() -> String? {
options.meta
}
}

private extension PostServiceRemote {

func getPosts(ofType type: String, options: PostRepositoryPostsSerivceRemoteOptions) async throws -> [RemotePost] {
try await withCheckedThrowingContinuation { continuation in
self.getPostsOfType(type, options: self.dictionary(with: options), success: {
continuation.resume(returning: $0 ?? [])
}, failure: {
continuation.resume(throwing: $0!)
})
}
}
}

extension PostRepository {

/// Fetch posts or pages from the given site page by page. All fetched posts are saved to the local database.
///
/// - Parameters:
/// - type: `Post.self` and `Page.self` are the only acceptable types.
/// - statuses: Filter posts or pages with given status.
/// - authorUserID: Filter posts or pages that are authored by given user.
/// - offset: The position of the paginated request. Pass 0 for the first page and count of already fetched results for following pages.
/// - number: Number of posts or pages should be fetched.
/// - blogID: The blog from which to fetch posts or pages
/// - Returns: Object identifiers of the fetched posts.
/// - SeeAlso: https://developer.wordpress.com/docs/api/1.1/get/sites/%24site/posts/
func paginate<P: AbstractPost>(
type: P.Type = P.self,
statuses: [BasePost.Status],
authorUserID: NSNumber? = nil,
offset: Int,
number: Int,
in blogID: TaggedManagedObjectID<Blog>
) async throws -> [TaggedManagedObjectID<P>] {
try await fetch(
type: type,
statuses: statuses,
authorUserID: authorUserID,
range: offset..<(offset + max(number, 0)),
orderBy: .byDate,
descending: true,
// Only delete other local posts if the current call is the first pagination request.
deleteOtherLocalPosts: offset == 0,
in: blogID
)
}

/// Search posts or pages in the given site. All fetched posts are saved to the local database.
///
/// - Parameters:
/// - type: `Post.self` and `Page.self` are the only acceptable types.
/// - input: The text input from user. Or `nil` for searching all posts or pages.
/// - statuses: Filter posts or pages with given status.
/// - authorUserID: Filter posts or pages that are authored by given user.
/// - limit: Number of posts or pages should be fetched.
/// - orderBy: The property by which to sort posts or pages.
/// - descending: Whether to sort the results in descending order.
/// - blogID: The blog from which to search posts or pages
/// - Returns: Object identifiers of the search result.
/// - SeeAlso: https://developer.wordpress.com/docs/api/1.1/get/sites/%24site/posts/
func search<P: AbstractPost>(
type: P.Type = P.self,
input: String?,
statuses: [BasePost.Status],
authorUserID: NSNumber? = nil,
limit: Int,
orderBy: PostServiceResultsOrdering,
descending: Bool,
in blogID: TaggedManagedObjectID<Blog>
) async throws -> [TaggedManagedObjectID<P>] {
try await fetch(
type: type,
searchInput: input,
statuses: statuses,
authorUserID: authorUserID,
range: 0..<max(limit, 0),
orderBy: orderBy,
descending: descending,
deleteOtherLocalPosts: false,
in: blogID
)
}

private func fetch<P: AbstractPost>(
type: P.Type,
searchInput: String? = nil,
statuses: [BasePost.Status]?,
authorUserID: NSNumber?,
range: Range<Int>,
orderBy: PostServiceResultsOrdering = .byDate,
descending: Bool = true,
deleteOtherLocalPosts: Bool,
in blogID: TaggedManagedObjectID<Blog>
) async throws -> [TaggedManagedObjectID<P>] {
assert(type == Post.self || type == Page.self, "Only support fetching Post or Page")
assert(range.lowerBound >= 0)

let postType: String
if type == Post.self {
postType = "post"
} else if type == Page.self {
postType = "page"
} else {
// There is an assertion above to ensure the app doesn't fall into this case.
return []
Comment on lines +388 to +389
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you considered fatalError, given this should be an unreachable code path?

Or, is return [] a safeguard for the production code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just don't want to duplicate the assertion code and its messaging. 😄

}

let remote = try await coreDataStack.performQuery { [remoteFactory] context in
let blog = try context.existingObject(with: blogID)
return remoteFactory.forBlog(blog)
}
guard let remote else {
throw PostRepository.Error.remoteAPIUnavailable
}

let options = PostRepositoryPostsSerivceRemoteOptions(options: .init(
statuses: statuses?.strings,
number: range.count,
offset: range.lowerBound,
order: descending ? .descending : .ascending,
orderBy: orderBy,
authorID: authorUserID,
search: searchInput
))
let remotePosts = try await remote.getPosts(ofType: postType, options: options)

let updatedPosts = try await coreDataStack.performAndSave { context in
let updatedPosts = PostHelper.merge(
remotePosts,
ofType: postType,
withStatuses: statuses?.strings,
byAuthor: authorUserID,
for: try context.existingObject(with: blogID),
purgeExisting: deleteOtherLocalPosts,
in: context
)
return updatedPosts.map {
guard let post = $0 as? P else {
fatalError("Expecting a \(postType) as \(type), but got \($0)")
}
return TaggedManagedObjectID(post)
}
}

return updatedPosts
}

}
4 changes: 4 additions & 0 deletions WordPress/WordPress.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1267,6 +1267,7 @@
4AA33F022999D11A005B6E23 /* ReaderSiteTopic+Lookup.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4AA33F002999D11A005B6E23 /* ReaderSiteTopic+Lookup.swift */; };
4AA33F04299A1F93005B6E23 /* ReaderTagTopic+Lookup.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4AA33F03299A1F93005B6E23 /* ReaderTagTopic+Lookup.swift */; };
4AA33F05299A1F93005B6E23 /* ReaderTagTopic+Lookup.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4AA33F03299A1F93005B6E23 /* ReaderTagTopic+Lookup.swift */; };
4AA7EE0F2ADF7367007D261D /* PostRepositoryPostsListTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4AA7EE0E2ADF7367007D261D /* PostRepositoryPostsListTests.swift */; };
4AAD69082A6F68A5007FE77E /* MediaRepositoryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4AAD69072A6F68A5007FE77E /* MediaRepositoryTests.swift */; };
4AD5656C28E3D0670054C676 /* ReaderPost+Helper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4AD5656B28E3D0670054C676 /* ReaderPost+Helper.swift */; };
4AD5656D28E3D0670054C676 /* ReaderPost+Helper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4AD5656B28E3D0670054C676 /* ReaderPost+Helper.swift */; };
Expand Down Expand Up @@ -6869,6 +6870,7 @@
4AA33EFA2999AE3B005B6E23 /* ReaderListTopic+Creation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ReaderListTopic+Creation.swift"; sourceTree = "<group>"; };
4AA33F002999D11A005B6E23 /* ReaderSiteTopic+Lookup.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ReaderSiteTopic+Lookup.swift"; sourceTree = "<group>"; };
4AA33F03299A1F93005B6E23 /* ReaderTagTopic+Lookup.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ReaderTagTopic+Lookup.swift"; sourceTree = "<group>"; };
4AA7EE0E2ADF7367007D261D /* PostRepositoryPostsListTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PostRepositoryPostsListTests.swift; sourceTree = "<group>"; };
4AAD69072A6F68A5007FE77E /* MediaRepositoryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MediaRepositoryTests.swift; sourceTree = "<group>"; };
4AD5656B28E3D0670054C676 /* ReaderPost+Helper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ReaderPost+Helper.swift"; sourceTree = "<group>"; };
4AD5656E28E413160054C676 /* Blog+History.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Blog+History.swift"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -15478,6 +15480,7 @@
570B037622F1FFF6009D8411 /* PostCoordinatorFailedPostsFetcherTests.swift */,
57569CF1230485680052EE14 /* PostAutoUploadInteractorTests.swift */,
4A2C73F62A9585B000ACE79E /* PostRepositoryTests.swift */,
4AA7EE0E2ADF7367007D261D /* PostRepositoryPostsListTests.swift */,
57D66B9C234BB78B005A2D74 /* PostServiceWPComTests.swift */,
57240223234E5BE200227067 /* PostServiceSelfHostedTests.swift */,
575802122357C41200E4C63C /* MediaCoordinatorTests.swift */,
Expand Down Expand Up @@ -23563,6 +23566,7 @@
3FFE3C0828FE00D10021BB96 /* StatsSegmentedControlDataTests.swift in Sources */,
015BA4EB29A788A300920F4B /* StatsTotalInsightsCellTests.swift in Sources */,
D81C2F5820F86CEA002AE1F1 /* NetworkStatus.swift in Sources */,
4AA7EE0F2ADF7367007D261D /* PostRepositoryPostsListTests.swift in Sources */,
E1C545801C6C79BB001CEB0E /* MediaSettingsTests.swift in Sources */,
806BA11C2A492B0F00052422 /* BlazeCampaignDetailsWebViewModelTests.swift in Sources */,
7E987F5A2108122A00CAFB88 /* NotificationUtility.swift in Sources */,
Expand Down
Loading