Transactions + Protocols Best Practices

[matt-dewitt]

I've been playing with GRDB for my latest iOS project's storage needs and love all that it provides. Looking for some guidance on the best practices for handling a certain scenario of abstracting GRDB and transactions behind a protocol.

Given this example of a counter object and it's storage protocol:

struct Counter: Codable, FetchableRecord, PersistableRecord {
    var id: Int64
    var value: Int64
}

protocol CounterStorageProtocol {
    func save(counter: Counter) throws
    func deleteCounter(id: Int64) throws -> Bool
    func doTransaction(_ transactionBlock: () throws -> Void) throws
}

When bulk processing counters from a server response we want to group all updates into a single transaction to keep the UI from updating until all updates are processed:

struct CounterServerResponse {
    var currentCounters: [Counter]
    var removedCounters: [Int64]
}

struct CounterServerProcessor {
    var counterStorage: CounterStorageProtocol

    func process(response: CounterServerResponse) throws {
        try counterStorage.doTransaction {
            try response.removedCounters.forEach {
                _ = try counterStorage.deleteCounter(id: $0)
            }
            try response.currentCounters.forEach {
               try counterStorage.save(counter: $0)
            }
        }
    }
}

How can we abstract this away using GRDB? A basic, standard implementation like:

struct CounterGRDBInMemoryStorage: CounterStorageProtocol {
    private let dbQueue = DatabaseQueue()

    func save(counter: Counter) throws {
        try dbQueue.write { db in
            try counter.insert(db)
        }
    }

    func deleteCounter(id: Int64) throws -> Bool {
        try dbQueue.write { db in
            try Counter.deleteOne(db, key: ["id" : id])
        }
    }

    func doTransaction(_ transactionBlock: () throws -> Void) throws {
        try dbQueue.inDatabase { db in
            try db.beginTransaction()
            try transactionBlock()
            try db.commit()
        }
    }
}

Throws fatal errors because Fatal error: Database methods are not reentrant. so I've managed to get something working by using unsafeReentrantWrite and unsafeReentrantRead in place of write and read but that doesn't seem like the best solution. Is there something I'm missing or a better way to model this type of abstraction using GRDB?

Thanks!

[groue]

Hello @matt-dewitt,

It looks like you could write working code with unsafeReentrant methods, and this looks fine to me. I expect something like:

struct CounterGRDBInMemoryStorage: CounterStorageProtocol {
    func save(counter: Counter) throws {
        try dbQueue.unsafeReentrantWrite { db in
            try counter.insert(db)
        }
    }

    func doTransaction(_ transactionBlock: () throws -> Void) throws {
        try dbQueue.unsafeReentrantWrite { db in
            try db.inTransaction {
                try transactionBlock()
                return .commit
            }
        }
    }
}

You must use "unsafe" methods, because CounterStorageProtocol does not force the user to think about transactions. Consider those two ways to save counters:

// Sure, why not?
for counter in counters {
    counterStorage.save(counter)
}

// Why bother?
counterStorage.doTransaction {
    for counter in counters {
        counterStorage.save(counter)
    }
}

The first version is easier to write than the second, more correct, version. Not only is the protocol easy to misuse, it actually fosters misuse. There lies the reason why you need to use "unsafe" methods.

Of course, in your specific application, the actual possibility for misuse can be very low. So you do not have to worry about the use of "unsafe" methods. Maybe write a documentation comment that explains when one should remember to use doTransaction (database integrity concerns, observation efficiency concerns, write performance concerns). That would be nice to your coworkers, or your future self. This comment would lift the "unsafety".

---

Beware how you handle your transactions. The code below is not correct:

// DON'T DO THAT
try dbQueue.inDatabase { db in
    try db.beginTransaction()
    try transactionBlock()
    try db.commit()
}

It is not correct, because errors thrown by transactionBlock() are not handled. If an error is thrown, then the transaction is not committed and not rollbacked. This is almost never what is intended by the programmer. Yet this is what the code does. So the code is unfaithful, and needs to be changed. And that's why you'll get a fatal error "A transaction has been left opened at the end of a database access". See Explicit Transactions for more details.

Instead, use inTransaction or inSavepoint. They handle errors by rollbacking the transaction, and never leave the database in an undefined state:

// CORRECT
try dbQueue.inDatabase { db in
    try db.inTransaction {
        try transactionBlock()
        return .commit
    }
}

Maybe a future GRDB version will rename beginTransaction and commit to beginUnsafeTransaction and unsafeCommit 😉 You get the point: some methods must be used with caution, when one cares about the integrity of the database and clear transaction boundaries.

[matt-dewitt]

Awesome, thanks for the quick reply! I'll make the recommended changes and add some comments and give it a try!