Support for GRDB 3.0 #27

hartbit · 2018-03-12T21:30:32Z

The tests all pass. Does this look good to you? I had to set the submodule to the latest commit of GRDB3-Associations, but I'm not sure is that how you keep repos in sync.

rxswiftcommunity · 2018-03-12T21:30:46Z

	Warnings
⚠️	It looks like code was changed without adding anything to the Changelog. If this is a trivial PR that doesn't need a changelog, add #trivial to the PR title or body.

Generated by 🚫 dangerJS

hartbit · 2018-03-12T21:31:41Z

RxGRDB/DatabaseWriter+Rx.swift

@@ -49,7 +49,7 @@ extension Reactive where Base: DatabaseWriter {
    /// - parameter requests: The observed requests.
    /// - parameter startImmediately: When true (the default), the first
    ///   element is emitted synchronously, on subscription.
-    public func changes(
+    public func changes<Request: FetchRequest>(


Because FetchRequest is a PAT now, I've had to make it into a generic argument here.

groue

That's great, David, thank you. This will ship with GRDB3!

groue · 2018-03-13T06:46:21Z

RxGRDB/DatabaseWriter+Rx.swift

@@ -90,7 +90,7 @@ extension Reactive where Base: DatabaseWriter {
    ///   its first right upon subscription.
    /// - parameter scheduler: The scheduler on which mapFetch emits its
    ///   elements (default is MainScheduler.instance).
-    public func fetchTokens(
+    public func fetchTokens<Request: FetchRequest>(


Don't we face the PAT kiss here?

let players = Player.all() let teams = Team.all() dbQueue.fetchTokens(in: [players, teams])... // I expect a compiler error

There's room for improvement here, even if this array of observed requests will sometimes be replaced with a single joined request.

Oh yes, of course. The API requires the type-erased wrapper:

public func fetchTokens( in requests: [AnyFetchRequest], startImmediately: Bool = true, scheduler: ImmediateSchedulerType = MainScheduler.instance) -> Observable<FetchToken>

which unfortunately makes the use-site uglier:

let players = Player.all() let teams = Team.all() dbQueue.fetchTokens(in: [AnyFetchRequest(players), AnyFetchRequest(teams)])

Does it work? (I still lack a few PAT reflex) AnyFetchRequest is also tagged with its fetched type, so we'd get an heterogeneous array of AnyFetchRequest<Player> and AnyFetchRequest<Team>...

David: you don't have to bother now about this. Your PR is already extra good, and should already allow you to play with the new APIs - we'll face issues as they bubble up. The final package will address all those little glitches with enhanced APIs, or enhanced documentation, but we don't have to rush.

Does it work? (I still lack a few PAT reflex) AnyFetchRequest is also tagged with its fetched type, so we'd get an heterogeneous array of AnyFetchRequest and AnyFetchRequest...

You're right, sorry 😛 I thought AnyFetchRequest also type-erased the fetched type. This probably needs a different API then.

David: you don't have to bother now about this. Your PR is already extra good, and should already allow you to play with the new APIs - we'll face issues as they bubble up. The final package will address all those little glitches with enhanced APIs, or enhanced documentation, but we don't have to rush.

No worries! I don't mind giving a hand if necessary. Just let me know if you want me to improve any other aspects of this PR.

A totally-erased FetchRequest type associated with Row (or Void) could be an interesting solution.

I did not fully erase AnyFetchRequest because AnySequence et al. are not. And GRDB currently returns AnyFetchRequest from request.asRequest(of: Type.self), which is a necessary tool:

let maxScoreRequest = Player.select(max(Column("score"))).asRequest(of: Int.self) maxScoreRequest.rx.fetchOne(in:dbQueue).subscribe { maxScore in ... } let bookInfoRequest = Book.including(...).asRequest(of: BookInfo.self) bookInfoRequest.rx.fetchOne(in:dbQueue).subscribe { bookInfo in ... }

So... we can reshape all things, that's what major versions are for, but it requires a little more thinking.

Oh yeah, I wasn't questioning your design of AnyFetchRequest. It does indeed feel necessary. Perhaps a Row based typed-erased FetchRequest would be interesting as you mention, but I'm not a fan of forcing the user to do all this explicit wrapping. If only we had generalised existentials!!!

GRDB2 had Request (not a PAT), and TypedRequest (a PAT), and the GRDB3 FetchRequest is the same as GRDB2's TypedRequest. I made this change because I thought that those two protocols were too complex, and that a single one would make it.

But it does not come without problems, as we can see.

And @freak4pc wondered about "fetching all authors with all their books". Since such request can't be expressed with a single SQL query, it falls out of the FetchRequest protocol. And it thus can't be observed with RxGRDB. Big problem, too.

I wouldn't like to have to bump to GRDB4 when we add support for "fetching all authors with all their books" eventually.

Maybe a new request protocol hierarchy is necessary:

// The non-PAT protocol for all requests that can be observed with RxGRDB et al. protocol DatabaseRegionRequest { func databaseRegion(_ db: Database) throws -> DatabaseRegion } /// The non-PAT protocol for all requests that can turn into a single SQL query. protocol SelectStatementRequest: DatabaseRegionRequest { func prepare(_ db: Database) throws -> (SelectStatement, RowAdapter?) } /// Free adoption of DatabaseRegionRequest for SelectStatementRequest /// (But can still be overridden by QueryInterfaceRequest so that it can build a /// more precise region thanks to its knowledge of the database schema /// and the query) extension SelectStatementRequest { func databaseRegion(_ db: Database) throws -> DatabaseRegion { let (statement, _) = try prepare(db) return statement.fetchedDatabaseRegion } } /// The PAT for typed requests, the one that users are supposed /// to adopt for their custom requests, if the built-in concrete types /// (SQLRequest and friends) do not fit the bill. /// (The same protocol as with already have in GRDB3) protocol FetchRequest: SelectStatementRequest { associatedtype RowDecoder } /// Our good old SQLRequest, so useful for feeding RxGRDB /// with raw SQL. /// (The same as with already have in GRDB3) struct SQLRequest<T>: FetchRequest { typealias RowDecoder = T } /// Our old pal QueryInterfaceRequest: Player.filter(...).order(...) /// (The same as with already have in GRDB3) struct QueryInterfaceRequest<T>: FetchRequest { typealias RowDecoder = T } /// The classic type-eraser /// (The same as with already have in GRDB3) struct AnyFetchRequest<T>: FetchRequest { typealias RowDecoder = T }

With these changes, we solve two problems:

dbQueue.rx.changes(in: [r1, r2, r3]) would just need DatabaseRegionRequest. No more PAT problem.

We can later introduce a new request type for "fetching all authors with all their books". It would not adopt SelectStatementRequest, because it can't run with a single SQL query. But it would adopt DatabaseRegionRequest so that it can be observed by RxGRDB. And it would expose an ad-hoc fetchAll() method, just like FetchRequest does today.

Sounds good. I'm happy that SQLRequest is now generic and that you kept it that way in this new hierarchy 👍

Done in groue/GRDB.swift@GRDB3-Associations: groue/GRDB.swift@2750230

Done in hartbit/RxGRDB@GRDB3: 1e1531f

I agree David: SQLRequest is what is was meant to be, now :-)

groue · 2018-03-13T06:49:08Z

RxGRDB/DatabaseWriter+Rx.swift

@@ -106,7 +106,7 @@ extension Reactive where Base: DatabaseWriter {

    /// Fixit for legacy API
    @available(*, unavailable, renamed:"fetchTokens(in:startImmediately:scheduler:)")
-    public func changeTokens(
+    public func changeTokens<Request: FetchRequest>(


I don't know if this fixit that helps migrating from older versions of RxGRDB survive the PAT kiss. If not, we'd have better remove it.

groue · 2018-03-13T06:49:59Z

Tests/DatabaseWriterTests.swift

@@ -29,7 +29,7 @@ extension DatabaseWriterTests {
            }
        }

-        let requests = [
+        let requests: [SQLRequest<Row>] = [


groue · 2018-03-13T07:00:49Z

The tests all pass. Does this look good to you?

Yes :-)

I had to set the submodule to the latest commit of GRDB3-Associations, but I'm not sure is that how you keep repos in sync.

This is what I currently do, yes.

Do you use CocoaPods to embed those libraries in your apps? If so, I guess you have updated your Podfile so that it points to groue/GRDB.swift@GRDB3-Associations, and hartbit/RxGRDB@GRDB3. I hope everything works as expected, and that you'll have one or two use cases for associations ;-)

DatabaseRequest -> SelectStatementRequest -> FetchRequest This commit has two goals: - introduce a protocol without any associated type which can feed RxGRDB multiple-requests observation methods without user pain (RxSwiftCommunity/RxGRDB#27 (comment)) - introduce a protocol that can feed RxGRDB observers and is able to execute multiple SQL queries (the eventual future "fetch all authors with all their books" request).

groue · 2018-03-26T18:06:55Z

@hartbit, this branch has been merged with RxGRDB v0.10.0. It is a recommended update, which fixes a surprising scheduling behavior of v0.9.0.

groue · 2018-05-05T10:51:21Z

Obsoleted by #32. Thanks @hartbit!

Support for GRDB 3.0

02fb998

hartbit commented Mar 12, 2018

View reviewed changes

groue approved these changes Mar 13, 2018

View reviewed changes

groue mentioned this pull request Mar 13, 2018

Associations & Joins groue/GRDB.swift#319

Merged

groue added 3 commits March 13, 2018 13:32

Solve the PAT problem at GRDB level

1e1531f

Merge branch 'master' into GRDB3

97103b1

Update RxGRDBDemo for GRDB 3.0

4ae6f26

groue added 3 commits April 29, 2018 19:33

Support for latest GRDB3-Associations

049911b

Fix tests that were failing because of groue/GRDB.swift#332

98c3666

Xcode 9.3+, Swift 4.1+ required

edf8b4f

groue closed this May 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for GRDB 3.0 #27

Support for GRDB 3.0 #27

hartbit commented Mar 12, 2018 •

edited by groue

Loading

rxswiftcommunity bot commented Mar 12, 2018 •

edited

Loading

hartbit Mar 12, 2018

groue Mar 13, 2018

groue left a comment

groue Mar 13, 2018

hartbit Mar 13, 2018 •

edited

Loading

groue Mar 13, 2018

hartbit Mar 13, 2018

groue Mar 13, 2018 •

edited

Loading

hartbit Mar 13, 2018

groue Mar 13, 2018 •

edited

Loading

hartbit Mar 13, 2018 •

edited

Loading

groue Mar 13, 2018 •

edited

Loading

groue Mar 13, 2018

groue Mar 13, 2018

groue Mar 13, 2018

groue commented Mar 13, 2018

groue commented Mar 26, 2018

groue commented May 5, 2018

Support for GRDB 3.0 #27

Support for GRDB 3.0 #27

Conversation

hartbit commented Mar 12, 2018 • edited by groue Loading

rxswiftcommunity bot commented Mar 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

groue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hartbit Mar 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

groue Mar 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

groue Mar 13, 2018 • edited Loading

Choose a reason for hiding this comment

hartbit Mar 13, 2018 • edited Loading

Choose a reason for hiding this comment

groue Mar 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

groue commented Mar 13, 2018

groue commented Mar 26, 2018

groue commented May 5, 2018

hartbit commented Mar 12, 2018 •

edited by groue

Loading

rxswiftcommunity bot commented Mar 12, 2018 •

edited

Loading

hartbit Mar 13, 2018 •

edited

Loading

groue Mar 13, 2018 •

edited

Loading

groue Mar 13, 2018 •

edited

Loading

hartbit Mar 13, 2018 •

edited

Loading

groue Mar 13, 2018 •

edited

Loading