SQLite dependency inversion

[designatednerd]

In this PR, in preparation for ripping out SQLite.swift (see #1658), I've pulled stuff out into a protocol to try to figure out exactly which APIs we're using in SQLite.swift so we can make a better determination of how to replace it.

Doing this as a separate PR since a) it gives me more options when trying to replace it and b) helps me know what I done broke when I do try to replace it 😛

[AnthonyMDev]

This is so awesome! Would just like to see documentation of the protocol. Otherwise LGTM!

[AnthonyMDev]

Would like to see the functions on this protocol very well documented. If someone wants to inject a DIFFERENT SQLLite dependency in the future, they should be able to easily understand what their adapter needs to do in each of these functions.

[designatednerd]

oh for sure - i was thinking i'd do that when i hit the final version but it's probably just as well to do it now given my goldfish brain

[AnthonyMDev]

As long as we do it at some point. If you want to do it later, go ahead and merge this. Up to you!

[groue]

Hello, since I'm a recent Apollo user, I dare to chime in!

This mergeRecords method performs several database operations.

If it were wrapped in a transaction, SQLite would be faster. I do not know if it is a concern.

If it were wrapped in a transaction, you'd get never get a half-modified database on disk, should an error happen at some point during the method execution. I also do not know if it is a concern.

If it were wrapped in a transaction, eventual concurrent reads (during the execution of mergeRecords) would not see a half-modified database. Maybe a concern, or not.

Since you're defining what's a database adapter, maybe it is time to ask those questions.

[AnthonyMDev]

This is the pinnacle of open source collaboration. <3
A maintainer of a popular open source repo being a user of another repo using their repo. And making suggestions to improve the usage of their repo using their knowledge of both of them. :) Haha thanks @groue this is a great comment.

[AnthonyMDev]

I guess technically we aren't using GRDB yet .... but we are getting there. >_<

[groue]

That's why this pull request is a very good idea :-) I tried to ask questions in a way that is independent from the preferred adapter. I surely don't want to imply anything else, influence your decision, or lock Apollo in a corner. It's just that defining an adapter protocol is a good opportunity to wonder about how Apollo interacts with its cache, and document the expected behavior, as you said above.

[AnthonyMDev]

Do you think that the SQLiteDatabase protocol should have some concept of a transaction built-in? I imagine that most any SQLLite implementation would likely have transactions and being able to utilize them via the inverted dependency protocol would be valuable.

[groue]

I don't know for sure. I mentioned mergeRecords because it looks like a method that could enjoy a transaction (for speed, or in order to enforce integrity guarantees). Transactions, when needed, are usually defined by the database client (here, Apollo), because only the database client knows how individual database operations should be grouped. Maybe Apollo does not need any transaction! But if it does, then I would indeed ask the adapter to provide transaction boundaries.

Yes, all "serious" SQLite wrappers make it able to perform transactions. APIs fall generally into three groups:

1. Block-based transactions

   // Form 1
   try wrapper.transaction {
        try wrapper.doStuff()
   }
   
   // Form 2
   try wrapper.transaction { dbAccess in
        try dbAccess.doStuff()
   }
   
   // Form 3
   try wrapper.perform {
        try wrapper.doStuff()
        try wrapper.save()
   }

   SQLite.swift and Realm are form 1. FMDB, GRDB and YapDatabase are form 2. Core Data is form 3.

2. "Free-form" transactions

   try wrapper.beginTransaction()
   try wrapper.doStuff()
   try wrapper.commit()

   Most SQLite implementations that allow raw SQL are here. Some kind of concurrency control must be added, in order to prevent interleaving transactions.

3. Transaction objects

   let transaction = try wrapper.makeTransaction()
   try transaction.doStuff()
   try transaction.commit()

   Core Data can be used like this, by detaching an NSManagedObjectContext. Other SQLite wrappers as well, by creating a new connection. OK, this is really not frequent.

Some wrappers distinguish read from read/write transactions, since this is a useful SQLite feature (YapDatabase, GRDB, CoreData, at least, support the "WAL mode" out of the box). This is an opportunity to improve multi-threaded performance by allowing multiple readers with a single writer.

I think you can accept all forms above with an adapter that looks like:

try databaseAdapter.write { writeTransactionAdapter in
    try writeTransactionAdapter.doStuff()
}

Something should serialize writes (either the adapter contract, or Apollo), or SQLite won't be happy.

If Apollo needs to read data, with a concern about cache integrity, and a need for multiple readers and single writer, then a read transaction is useful as well (adapters that do not support single writer/multiple readers can still implement it on top of their single writer):

let stuff = try databaseAdapter.read { readTransactionAdapter in
    return try readTransactionAdapter.loadStuff()
}

---

This only applies if you think Apollo needs transactions to access its cache.

[designatednerd]

@groue @AnthonyMDev 🎶 It's the ciiiiiiiircle of liiiiiiiife... 🎶

I think i probably want to leave transaction support out for this PR since i'm just trying to do a 1:1 "what are we already doing" conversion before adding any further complexity.

However, you're absolutely correct that this would be way better, particularly from a standpoint of write-ahead log support (which is better for things with extensions etc). I'll take a look at that in my next PR.

[designatednerd]

Merging this bit.