Create a generic CoreDataQuery type

[crazytonyli]

Follow up of #19353 (comment). This draft PR is for gathering feedback about a new way of querying Core Data models.

This PR proposes a way of building Core Data query predicates, using a strongly typed API, instead of writing predicate strings:

// old way
let request = NSFetchRequest<AbstractPost>(entityName: NSStringFromClass(AbstractPost.self))
request.predicate = NSPredicate(format: "blog = %@ AND original = NULL AND postID = %ld", self, postID)
return (try? context.fetch(request))?.first

// new way
try? AbstractPost.query()
    .equal(\.blog, self)
    .null(\.original)
    .equal(\.postID, postID)
    .first(in: context)

I want to check with you all to see if this is something that'd be useful to the current codebase, i.e. do we see benefits of using strongly typed API over predicate string? Will this become a overhead, that makes building custom query harder?

These are a few other things in my mind that haven't been implemented. Like OR operator, currently all queries are treated as AND.

Regression Notes

1. Potential unintended areas of impact

2. What I did to test those areas of impact (or what existing automated tests I relied on)

3. What automated tests I added (or what prevented me from doing so)

PR submission checklist:

• I have completed the Regression Notes.
• I have considered adding unit tests for my changes.
• I have considered adding accessibility improvements for my changes.
• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[wpmobilebot]

You can test the changes in Jetpack from this Pull Request by:

• Clicking here or scanning the QR code below to access App Center
• Then installing the build number pr19394-fb3970e on your iPhone

If you need access to App Center, please ask a maintainer to add you.

[wpmobilebot]

You can test the changes in WordPress from this Pull Request by:

• Clicking here or scanning the QR code below to access App Center
• Then installing the build number pr19394-fb3970e on your iPhone

If you need access to App Center, please ask a maintainer to add you.

[jkmassel]

This is a neat idea. I definitely see the value in strong typing here, but I have a few concerns about the abstraction layer:

1. This is sort of like a read-only ORM. That's ok, but to me it feels like we're somewhere between a query builder and a repository object given the current API, and I worry that's painting ourselves into a corner. I wonder if we could leverage some prior art from other ORMs with an API something like:

CoreDataQuery<Blog>
.lookup(where: .allOf(
    .hostname(contains: hostname),
    .username(equals: site.username),
    .anyOf(
     .hostedByWPCom(is: true),
     .isVisible
   )
))
.sortedBy(\.siteID, .ascending)

2. Related to the above, IMHO this isn't worth implementing unless we go all the way on it (lookup-wise, anyway) – if we end up only implementing part of it, guaranteed we'll end up in a spot where we need some additional feature tomorrow and it's not there 😅.

3. While this whole plan feels nicely "correct", I wonder if it's a situation where we'll see tangible benefit stability or velocity-wise? While this can prevent a certain class of bug (great!) the type of bug we're avoiding seems rather shallow – it should cause a (hopefully) obvious crash (ie – if a format string is incorrect). Not to say that this isn't worth doing, but if it'll be a significant time investment to implement a full set of querying tools, it might be better to push this off. If it's actually pretty quick and easy, then I think it's worth the investment.

Happy to discuss further! 😀

[jkmassel]

It'll be a bit more work, but let's not eat the error here – we should pass it up to the caller to deal with (for now we can have the same implementation at a higher level, but there's a difference between "there are no cells" and "there was an error trying to fetch the data", and the higher we can propagate that error, the more likely we can help the user understand what's going on)

[jkmassel]

Nit: Result here is confusing to me because of the Result<T, Error> type. WDYT?

[AliSoftware]

I wonder if there isn't already a SPM or CocoaPods library which supports type-safe CoreData queries already or there? Would be worth checking before reinventing the wheel?

For example a quick search on cocoapods.org led me to https://cocoapods.org/pods/CoreDataQueryInterface ; I have no idea if that one is a good one (stability, test coverage, good API?) but at least it shows some exists 🙃

And if we can't find one we're happy about, maybe it's worth moving your implementation into a Swift Package so it could be reusable? (At least by our other apps like WooCommerce for example)?

[crazytonyli]

@jkmassel Thanks for the detailed feedback. I agree there is a lot to explore regarding the API. This PR probably went with the most simple one. The NSComparisonPredicate API should be able to support the implementation, and we just need to figure out how to wrap it in a nice API. @mokagio has mentioned the Active Record style query a couple of times, which I have never used but seems pretty nice and clean. I imagine the end result of this PR would be like other syntax sugar, they don't necessarily reduce many bugs, but they are more likely to provide a clean syntax to write (and read) code and potentially save some time from minor bugs caused by typos. I'm not sure how the end result would look like, or useful at all, but would be a fun little thing to explore. 😄

@AliSoftware I didn't look for any open source projects. But from a quick search, I see there are a few similar projects, like PredicateKit. I got this idea when I was working on other changes (this BlogQuery type to be specific), and I basically extended that BlogQuery type a little bit to make it generic. For now, I'll leave this PR on the side and come back when I have some free time. I will do a more deeper search for existing open source project then.

[AliSoftware]

PredicateKit looks really nice!
And I think that's the one I saw around a while back (back in the day when I wrote CoreData 👴🏼😅) had this had in mind when I suggested libs above 🙂