asfernandes / node-firebird-drivers

Node.js Firebird Drivers
MIT License
53 stars 17 forks source link

How to determine either transaction active or not? #112

Closed gsbelarus closed 2 years ago

gsbelarus commented 2 years ago

In Delphi we have a lot of code which executes some SQL statement and then commits or rolls back a transaction in dependence of were SQL statement executed successfully or not.

Conceptually the code looks like:

  Transaction.StartTransaction;
  try
    Query.Transaction := Transaction;
    Query.Exec;
    Transaction.Commit;
  finally
    if Transaction.InTransaction then
      Transaction.Rollback;
  end;

But in node-firebird-driver there is no way to determine state of transaction. So, either additional manually set flag should be used or we need in wrapping Transaction object within another object containing state flag, possibly using HOF.

Is it possible to add property InTransaction: () => boolean to the Transaction interface, which will return true if the transaction is active or false otherwise.

gsbelarus commented 2 years ago

https://github.com/asfernandes/node-firebird-drivers/pull/118

gsbelarus commented 2 years ago

Please, consider merging PR above as it would save me from writing ugly code like this:

export const closeConnection = async (client: Client, attachment: Attachment, transaction: Transaction) => {
  if ((transaction as any).attachment) {
    await transaction.commit();
  }
  await attachment.disconnect();
  await client.dispose();
};
asfernandes commented 2 years ago

I thought I had commented before on this, but seems not.

My problem with this is that all classes are expected to not be used after there are disposed.

In the case of transactions, commit/rollback disposes them.

So every valid transaction object is active.

If this rule is broken, then all classes should have isValid method, but I'm not sure it would be good.

gsbelarus commented 2 years ago

But what if the code has complex branching and I want to clean up DB connection at the end as:

  finally {
    await closeConnection(client, attachment, transaction);
  }
};

as transaction.commit() will not destroy the object I have to maintain separate flag for every transaction to know either it is safe to commit in the end or not.

gsbelarus commented 2 years ago

Consider following example:

  const { client, attachment, transaction} = await setConnection();

  try {
    // here goes lengthy code which could end with exception
    await transaction.commit();
    // another code
  }
  finally {
    // here we want to ensure that transaction and connection
    // are closed properly in spite of exception was raised or not
    await transaction.commit();
    await attachment.disconnect();
    await client.dispose();
  }

in Delphi (and we have tons of such code) the task has a simple solution:

  try
    //...
    tr.Commit;
    //...
  finally
     if tr.InTransaction then
       tr.Commit;
     tr.Free;
  end

Would be great to have something like this in JS too.

asfernandes commented 2 years ago

You could also assign them to null after dispose and test them for null later.

As I said, I would accept PR with isValid for all classes.

Blob could delegate the call to its attachment.

gsbelarus commented 2 years ago

implemented as an isValid() method.

https://github.com/asfernandes/node-firebird-drivers/pull/119