Batch and transaction writes across multiple collections about fireorm HOT 9 CLOSED

Now that I ended up writing all of this I'm pumped to implement it. If you want to implement it go ahead or let me know so I can take it.

from fireorm.

@wovalle Happy for you to take it! I think you have a better idea of how it would work.

The only thing I'd mention with regards to your previous comment is I think it might be confusing having multiple ways of running transactions. IMO it should be just a single API to handle this, is there much value in having runTransaction in bandRepository too?

from fireorm.

Yeah, you're right. I cannot remove it now because that's a breaking change but I'll deprecate it and remove it in the upcoming v1.

from fireorm.

Makes sense 😄

from fireorm.

Yes, you're right. Today with fireorm you would need to open multiple transactions which is not only not ideal, it defeats the purpose of a transaction itself. A global runTransaction makes a lot of sense.

How would the API look like? I want to enforce type checking in the transactions methods.

A runTransaction where you pass the type to all the methods in the TransactionRepository?

import { runTransaction } from 'fireorm';

runTransaction(async () => {
  await transactionRepository.set<Entity>(entity);
})

cc @mamodom, I know you expressed interest in this feature before

from fireorm.

@wovalle That would work, although I believe it would lose type safety if the user forgot to add <Entity>, which would allow bugs to occur.

Maybe something like this to enforce it better?

await transactionRepository.set(Entity, data);

Then the type could be something along the lines of this:

set(entity: T, data: keyof T)

from fireorm.

I took a look into this, I managed to get most of it done but I'm facing major hurdles when trying to separate the TransactionRepository from AbstractFirestoreRepository.

I switched over to the following syntax:

// Modified InstantiableIEntity
type Instantiable<T> = { new(): T }

findById(entity: Instantiable<T>, id: string): Promise<T> {
    const { firestoreColRef } = getRepository(entity);
    const query = firestoreColRef.doc(id);

    return this.transaction.get(query).then(this.extractTFromDocSnap);
}

However, in the constructor for TransactionRepository, there isn't a collection to pass to AbstractFirestoreRepository. What would you suggest here? Transactions rely on some of the methods from the AbstractFirestoreRepository, but overall it doesn't look like the two should be linked anymore.

from fireorm.

Originally I made TransactionRepository inherit from AbstractFirestoreRepository to avoid implementing the where* methods again. It worked on that end but it ended up with the following downsides:

• Had to overwrite limit and orderBy* methods and return errors
• As you mentioned in another issue, update/create/delete methods are declared as async when they're just adding the operations to the transaction object, they should be sync

So I wouldn't be too against removing that relation if it inherits from BaseRepository and implement the same interfaces IQueryBuilder and IQueryExecutor.

About your proposal in this comment I don't think that'll work the way I'm envisioning it. I have a requirement that you shouldn't need to pass T when you invoke a transaction from the repository

The way I see it is this something like this:

Having a generic Transaction class with the normal transaction class that contains two methods: getRepository and commit. This Transaction class will be the one that holds the private Firestore Transaction.

When invoked in a repository, the object passed to runTransaction will just be the result of getRepository(T>).

See the examples:

With Global runTransaction:

import { runTransaction } from 'fireorm';

runTransaction(transaction => {
  const bandTransactionRepository = transaction.getRepository(Band);
  const membersTransactionRepository = transaction.getRepository(Members);


  const band = await bandTransactionRepository.findById('opeth');
  const members = await membersTransactionRepository.whereEqualTo(m => b.bandId, 'opeth');

  band.deleted = true;

  members.forEach(m => {
    m.bandDisabled = true;
    membersTransactionRepository.update(m);
  });

  bandTransactionRepository.update(band);

  await transaction.commit();
});

With runTransaction inside a repository:

import { getRepository } from 'fireorm';

const bandRepository = getRepository(Band);

bandRepository.runTransaction(bandTransactionRepository => {
  // Where bandTransactionRepository is the same object returned by 
  // const bandTransactionRepository = transaction.getRepository(Band);
  // in the example below

  const band = await bandTransactionRepository.findById('opeth');

  band.deleted = true;

  bandTransactionRepository.update(band);

  await bandTransactionRepository.commit();
})

from fireorm.

🎉 This issue has been resolved in version 0.13.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

from fireorm.