feat(firestore): support for aggregate queries including sum() & average()

[russellwheatley]

Description

• Support for getAggregateFromServer(). Modular support only.

Related issues

closes #8027

Release Summary

Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • Yes
• My change supports the following platforms;
  • Android
  • iOS
• My change includes tests;
  • e2e tests added or updated in packages/\*\*/e2e
  • jest tests added or updated in packages/\*\*/__tests__
• I have updated TypeScript types that are affected by my change.
• This is a breaking change;
  • Yes
  • No

Test Plan

---

Think react-native-firebase is great? Please consider supporting the project with any of the below:

• 👉 Star this repo on GitHub ⭐️
• 👉 Follow React Native Firebase and Invertase on Twitter

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-native-firebase	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 13, 2024 0:37am

[mikehardy]

exciting! couldn't but look through it even though draft+android-only+no-e2e at the moment because I contemplated implementing this myself months ago :-)

Had a couple quick thoughts but mostly just wanted to say this is exciting. Cheers

[mikehardy]

awesome!

[mikehardy]

still LGTM - just looked at the last couple test additions (nice)

[alanschwarz]

Hi!

I upgraded to 21.5.0 today because I was super happy for the support for sum(). But I think there might be an issue with typescript... I made some screenshots. Maybe I am doing something wrong, but the docs aren't updated yet, so I'm not sure how to help...

Thank you! If I can help in any way please let me know :)

[joaqo]

Same here @mikehardy, can't for the life of me make sum() work. I read the PR's code and tried things like this:

import firestore, { count, sum, getAggregateFromServer} from "@react-native-firebase/firestore"

// Gets permission error
const result = await getAggregateFromServer(Reference.ordersForSeller(seller.id).where("status", "==", "arrived"), { sum: sum("totalCents") })

// Works fine
const loler = await Reference.ordersForSeller(seller.id).where("status", "==", "arrived").count().get()

But I get weird permission errors, and as @alanschwarz said, there is no sum method on query objects, so I really don't know what to do.

Could you provide a working example code snippet on how to make sum work? Thanks for the amazing work!

[mikehardy]

@joaqo there is a pile of tests right here that executes in CI all the time

react-native-firebase/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js

Line 103 in 9c6f607

describe('count(), average() & sum()', function () {

Now, sometimes our typings aren't quite right - I've seen that - but these tests definitely work and I remember working through them with Russell to probe the behavior of the numbers vs firebase-js-sdk given there are lots of little gotchas in how javascript handles numbers due to IEEE doubles as the numeric type

[joaqo]

Hi @mikehardy, I actually based my tests on that snippet, but I really can't get it to work. I get a permission-denied firestore error. This is the test I ran:

async function getOrderCount(seller: any) {
  const aggregateSpec = {
    countCollection: count(),
  }
  const ref = Reference.ordersForSeller(seller.id).where("status", "==", "arrived")
  const result = await getAggregateFromServer(ref, aggregateSpec)
  const data = result.data()
  console.log("Count", data.countCollection)  // Gets permission errors!
}

async function getOrderCountSlow(seller: any) {
  const ref = Reference.ordersForSeller(seller.id).where("status", "==", "arrived")
  const result = await ref.get()
  console.log("Count Slow", result.docs.length)  // Works just fine!
}

So I do have read permissions for this collection as evidenced on the second function, but for some reason the first function returns a permission-denied firestore error. Is it possible that these aggregators need a different permission than just a READ permission? According to the docs they shouldn't though.

Thanks!

Edit: Just realized that ordersForSeller is a collection group, could this have anything to do with it?

[mikehardy]

I don't know, sorry, haven't played with the intersection of cloud security rules and the aggregates. This isn't react-native specific, I'd suggest using stackoverlow and tagging your issue well, the firebase support team is frequently there in official capacity

[joaqo]

I think I might have found the issue @mikehardy. Decided to turn off all security rules for that collection group and the aggregation query worked, but with a very interesting caveat, it was completely ignoring my where clause.

On my slow query I get around 200 orders, and on my aggregated query I count around 1900 orders, which are all my orders regardless of seller! This means that for some reason the aggregated query is ignoring the where clause which filters by seller_id, and that was the reason for the original permission-denied error, it was trying to access orders from sellers other than themselves!

So I suspect this is an issue with your aggregated queries not respecting any where clauses attached to them? That or I'm doing something wrong with them I guess.

PD: If I do collection(...).where(...).count() the where clause gets considered correctly, but if I use getAggregateFromServer as I showed above, it doesn't!

[joaqo]

Here is a more concise example that shows the issue:

async function getOrderCount(seller: any) {
  const aggregateSpec = { countCollection: count() }
  const ref = Reference.ordersForSeller(seller.id).where("status", "==", "arrived")

  const correct = await ref.count().get()
  console.log("Count correct", correct.data().count)  // Correctly lists filtered orders

  const incorrect = await getAggregateFromServer(ref, aggregateSpec)
  const data = incorrect.data()
  console.log("Count incorrect", data.countCollection)  // Lists ALL orders, which is incorrect
}

[mikehardy]

You may be on to something here @joaqo ! Very interesting. And it looks like it may be react-native-firebase specific, contrary to my pointer towards stackoverflow for more general support

May I ask for you to open up a new issue with details on versions, platforms you reproduce with etc? (it might vary between ios and android even). This looks like something we need to investigate as an issue vs comments on a closed PR (however helpful the comments may be!)

Slight follow-on - not sure how practical this is for you, but are you able to test with the firebase-js-sdk perhaps? Just to confirm that this behavior isn't generic to the aggregate queries for whatever reason but really is react-native-firebase local

[joaqo]

No problem, I can certainly create a separate issue with more details! I don't think I can provide a firebase-js example though as I'm pretty tight on time, and our app is native only. Thanks again!

[joaqo]

Done @mikehardy: #8253