Multi level undo in a single database write?

Related Projects GRDB
#1

Hi,

I've a function that deletes some objects, but as the SQL data is hierarchical I need a (recursive) helper function that modifies the hierarchy before the deletion.
Both functions register their changes with the UndoManager and undo/redo works fine.
The only catch is that the initial deletion/modification is done in a single database write (by passing the Database parameter to the helper function, but the undo/redo is done by several calls from the undo manager, resulting in all separate writes.

Do you have some suggestion on how I can make sure that a multilevel undo/redo is also performed in a single database write?

Maybe there is a way to 'lock' the database for other reads/writes for a longer duration than what barrierWriteWithoutTransaction gives? If so, I can lock the database on the NSUndoManagerWillUndoChange and unlock on NSUndoManagerDidUndoChange notification.

Thanks in advance.

Kind regards,

Remco Poelstra

#2

Hello Remco, I'm not sure I have the full picture of your setup, but it looks like this asymmetry is the source of the question.

If this is the case, then maybe the solution is simply to have two sets of writing methods. One set of internal methods which call write and open a transaction. And one set of private methods which perform the actual database job, accept a Database argument, and are called by the internal methods. The internal methods define the transaction boundaries. The private methods perform composable database accesses. With such a setup, it is usually easy to build things like:

// Internal

func foo() throws {
  try dbPool.write { db in
    try trucbidule(db)
    try frobnicate(db)
  }
}

func bar() throws {
    try dbPool.write(frobnicate)
}

// Private

private func frobnicate(_ db: Database) throws {
  // CRUD
}

private func trucbidule(_ db: Database) throws {
  // CRUD
}
#3

Hi,

Thanks for your answer. This is more or less the structure that I've now for performing the initial action. The problem is when I add undo support:

func foo() throws {
  try dbPool.write { db in
    try trucbidule(db)
    try frobnicate(db)
  }
}

func bar() throws {
    try dbPool.write(frobnicate)
}

// Private

private func frobnicate(_ db: Database) throws {
  let oldElements = fetch.......

  // CRUD

  undoManager.registerUndo(withTarget: self) {
    $0.defrobnicate(oldElements)
  }
}

private func trucbidule(_ db: Database) throws {
  let oldElements = fetch.......

  // CRUD

  undoManager.registerUndo(withTarget: self) {
    $0.detrucbidule(oldElements)
  }
}

private func defrobnicate() {
  //de-CRUD
}

private func detrucbidule() {
  //de-CRUD
}

Here the de-functions are called by the undoManager but they currently have no option to share a Database. This isn't an problem for bar, but it might be for foo.

Things get even more complicated because in my situation trucbidule calls itself (and hence registers multiple calls to the undoManager), as well as that the de-functions should also register with the undo manager for redo.

Kind regards,

Remco

#4

OK @remcopoelstra. What happens when the undo registrations happen in the internal foo and bar methods?

Something along:

func bar() throws {
    let oldFrobnicateElements = try dbPool.write(frobnicate)
    undoManager.registerUndo(withTarget: self) {
        $0.undoBar(oldFrobnicateElements)
    }
}

func foo() throws {
    let (oldTrucBiduleElements, oldFrobnicateElements) = try dbPool.write { db in
        try (trucbidule(db), frobnicate(db))
    }
    undoManager.registerUndo(withTarget: self) {
        $0.undoFoo(oldTrucBiduleElements, oldFrobnicateElements)
    }
}

I'm not saying it's pretty, and we could look at a way to improve it. But you see the pattern:

  1. The private methods return values that help undoing their job.
  2. The internal methods perform one transaction, and register another undo transaction that uses those values.

Most importantly, it enforces the "undoable action <=> transaction/anti-transaction" equivalence, which, IMHO, is a guideline that should never be broken. Maybe this fundamental invariant will help design a better looking api.

#5

I’m not sure if this will be a better pattern, but you can also get UndoManager to coalesce actions using beginUndoGrouping() (and endUndoGrouping()).

1 Like
#6

Hi,

Thanks for the suggestion.
By default an undo group is already created for every runloop run. This does not actually group the calls into a single function call, which would be required here.

#7

Ah, sorry, I misread the question as "I'm doing one operation but getting two Undos" rather than "I'm doing one operation but getting two transactions". I understand now.

#8

Hi @gwendal.roue,

I'll investigate a solution like this.
Currently I've only been using functions that were more general, not specifically written to undo the operation of another function. That might well be a (the?) solution to this problem, as they can be made to handle the very specific requirements of the undo operation.

#9

Do we need the ability to define transactions step by step? You could register several undo steps: open undo transaction, undo step 1, undo step 2, commit undo transaction ?

#10

Whether we 'need' it is a difficult question, but it was basically what I was thinking originally. Start a transaction when the undo manager notifies that it's starting an undo operation and end the transaction for the end notification.

But your other suggestion might work out just fine....

#11

The "step by step" transaction is indeed a challenge to implement today, due to GRDB handling of database access locking with DispatchQueue. But it may reveal quite suited for your use case and possibly others.

#12

Or is there any UndoManager callback that would let you setup an array of undo functions that could be initialized at the beginning of the undo action, and performed as a transaction as the termination of that undo action? I'm sorry I know so little about NSUndoManager.

#13

No, there is no such callback possible. It's a rather dumb system. It just calls the blocks you registered in reverse order. It does send out a notification before and after though.
What would be possible (maybe) would be to block all writers, except the one holding some kind of token. Like a snapshot of the database, but writable (and hence always the latest state).
This is deadly of course if for some reason the token is not released. All users of the database would remain blocked.

Except for the additional code, writing specific undo functions and collecting all modified objects from private functions might be the safer way.

#14

It works quite well!
Thanks for the support!

1 Like
Terms of Service

Privacy Policy

Cookie Policy