fix: CyCraft#116

packages/plugin-firestore-admin/src/helpers/getFirestore.ts

@@ -1,4 +1,5 @@
 import type { DocMetadata, QueryClause } from '@magnetarjs/types'
+import { isWhereClause } from '@magnetarjs/types'
 import type { FirestoreModuleConfig } from '@magnetarjs/utils-firestore'
 import type {
   CollectionReference,
@@ -10,7 +11,7 @@ import type {
   WriteBatch,
 } from 'firebase-admin/firestore'
 import { FieldValue, Filter } from 'firebase-admin/firestore'
-import { isArray, isNumber } from 'is-what'
+import { isNumber } from 'is-what'
 export type {
   CollectionReference,
   DocumentReference,
@@ -36,16 +37,22 @@ function queryToFilter(
   queryClause: QueryClause
 ): ReturnType<(typeof Filter)['or']> | ReturnType<(typeof Filter)['and']> {
   if ('and' in queryClause) {
-    if (isArray(queryClause.and)) {
-      return Filter.and(...queryClause.and.map((whereClause) => Filter.where(...whereClause)))
-    }
-    return queryToFilter(queryClause.and)
+    return Filter.and(
+      ...queryClause.and.map((whereClauseOrQueryClause) =>
+        isWhereClause(whereClauseOrQueryClause)
+          ? Filter.where(...whereClauseOrQueryClause)
+          : queryToFilter(queryClause)
+      )
+    )
   }
   // if ('or' in queryClause)
-  if (isArray(queryClause.or)) {
-    return Filter.or(...queryClause.or.map((whereClause) => Filter.where(...whereClause)))
-  }
-  return queryToFilter(queryClause.or)
+  return Filter.or(
+    ...queryClause.or.map((whereClauseOrQueryClause) =>
+      isWhereClause(whereClauseOrQueryClause)
+        ? Filter.where(...whereClauseOrQueryClause)
+        : queryToFilter(queryClause)
+    )
+  )
 }
 
 /**

packages/plugin-firestore-admin/test/external/fetch.test.ts

@@ -1,6 +1,6 @@
+import { pokedex } from '@magnetarjs/test-utils'
 import test from 'ava'
 import { createMagnetarInstance } from '../helpers/createMagnetarInstance'
-import { pokedex } from '@magnetarjs/test-utils'
 
 {
   const testName = 'fetch (collection)'
@@ -276,6 +276,97 @@ import { pokedex } from '@magnetarjs/test-utils'
     }
   })
 }
+{
+  const testName = 'fetch (collection) query-filter: and'
+  test(testName, async (t) => {
+    const { pokedexModule } = await createMagnetarInstance('read')
+    try {
+      const queryModuleRef = pokedexModule
+        .query({
+          and: [
+            ['type', 'array-contains', 'Fire'],
+            ['base.Speed', '>=', 100],
+          ],
+        })
+        .orderBy('base.Speed', 'asc')
+      await queryModuleRef.fetch({ force: true }, { onError: 'stop' })
+      const actual = [...queryModuleRef.data.values()].map((p) => p.base.Speed)
+      const expected = [pokedex(6), pokedex(38), pokedex(78)].map((p) => p.base.Speed)
+      t.deepEqual(actual, expected as any)
+      // also check the collection without query
+      const actualDocCountWithoutQuery = pokedexModule.data.size
+      const expectedDocCountWithoutQuery = expected.length
+      t.deepEqual(actualDocCountWithoutQuery, expectedDocCountWithoutQuery)
+    } catch (error) {
+      t.fail(JSON.stringify(error))
+    }
+  })
+}
+{
+  const testName = 'fetch (collection) query-filter: or'
+  test(testName, async (t) => {
+    const { pokedexModule } = await createMagnetarInstance('read')
+    try {
+      const queryModuleRef = pokedexModule.query({
+        or: [
+          ['name', '==', 'Bulbasaur'],
+          ['name', '==', 'Ivysaur'],
+          ['name', '==', 'Venusaur'],
+        ],
+      })
+      await queryModuleRef.fetch({ force: true }, { onError: 'stop' })
+      const actual = [...queryModuleRef.data.values()].map((p) => p.base.Speed)
+      const expected = [pokedex(1), pokedex(2), pokedex(3)].map((p) => p.base.Speed)
+      t.deepEqual(actual, expected as any)
+      // also check the collection without query
+      const actualDocCountWithoutQuery = pokedexModule.data.size
+      const expectedDocCountWithoutQuery = expected.length
+      t.deepEqual(actualDocCountWithoutQuery, expectedDocCountWithoutQuery)
+    } catch (error) {
+      t.fail(JSON.stringify(error))
+    }
+  })
+}
+{
+  const testName = 'fetch (collection) query-filter: combine or, and'
+  test(testName, async (t) => {
+    const { pokedexModule } = await createMagnetarInstance('read')
+    try {
+      const queryModuleRef = pokedexModule.query({
+        or: [
+          {
+            and: [
+              ['name', '==', 'Bulbasaur'],
+              ['base.Speed', '==', 45],
+            ],
+          },
+          {
+            and: [
+              ['name', '==', 'Ivysaur'],
+              ['base.Speed', '==', 60],
+            ],
+          },
+          {
+            and: [
+              ['name', '==', 'Venusaur'],
+              ['base.Speed', '==', 80],
+            ],
+          },
+        ],
+      })
+      await queryModuleRef.fetch({ force: true }, { onError: 'stop' })
+      const actual = [...queryModuleRef.data.values()].map((p) => p.base.Speed)
+      const expected = [pokedex(1), pokedex(2), pokedex(3)].map((p) => p.base.Speed)
+      t.deepEqual(actual, expected as any)
+      // also check the collection without query
+      const actualDocCountWithoutQuery = pokedexModule.data.size
+      const expectedDocCountWithoutQuery = expected.length
+      t.deepEqual(actualDocCountWithoutQuery, expectedDocCountWithoutQuery)
+    } catch (error) {
+      t.fail(JSON.stringify(error))
+    }
+  })
+}
 {
   const testName = 'fetch (collection) compound queries'
   test(testName, async (t) => {

packages/plugin-firestore-admin/test/helpers/initFirebase.ts

@@ -1,4 +1,4 @@
-import { initializeApp, cert } from 'firebase-admin/app'
+import { cert, initializeApp } from 'firebase-admin/app'
 import { getFirestore } from 'firebase-admin/firestore'
 
 const config = {

packages/plugin-firestore/src/helpers/getFirestore.ts

@@ -1,10 +1,11 @@
-import { DocMetadata, QueryClause } from '@magnetarjs/types'
+import { DocMetadata, isWhereClause, QueryClause } from '@magnetarjs/types'
 import type { FirestoreModuleConfig } from '@magnetarjs/utils-firestore'
 import type {
   CollectionReference,
   DocumentSnapshot,
   Firestore,
   Query,
+  QueryCompositeFilterConstraint,
   QueryDocumentSnapshot,
 } from 'firebase/firestore'
 import {
@@ -18,22 +19,26 @@ import {
   startAfter,
   where,
 } from 'firebase/firestore'
-import { isArray, isNumber } from 'is-what'
+import { isNumber } from 'is-what'
 
-function applyQuery(q: CollectionReference | Query, queryClause: QueryClause): Query {
+function applyQuery(queryClause: QueryClause): QueryCompositeFilterConstraint {
   if ('and' in queryClause) {
-    if (isArray(queryClause.and)) {
-      return query(q, and(...queryClause.and.map((whereClause) => where(...whereClause))))
-    }
-    return applyQuery(q, queryClause.and)
-  }
-  if ('or' in queryClause) {
-    if (isArray(queryClause.or)) {
-      return query(q, or(...queryClause.or.map((whereClause) => where(...whereClause))))
-    }
-    return applyQuery(q, queryClause.or)
+    return and(
+      ...queryClause.and.map((whereClauseOrQueryClause) =>
+        isWhereClause(whereClauseOrQueryClause)
+          ? where(...whereClauseOrQueryClause)
+          : applyQuery(whereClauseOrQueryClause)
+      )
+    )
   }
-  return q
+  // if ('or' in queryClause)
+  return or(
+    ...queryClause.or.map((whereClauseOrQueryClause) =>
+      isWhereClause(whereClauseOrQueryClause)
+        ? where(...whereClauseOrQueryClause)
+        : applyQuery(whereClauseOrQueryClause)
+    )
+  )
 }
 
 /**
@@ -49,7 +54,7 @@ export function getQueryInstance(
     ? collectionGroup(db, collectionPath.split('*/')[1])
     : collection(db, collectionPath)
   for (const queryClause of config.query || []) {
-    q = applyQuery(q, queryClause)
+    q = query(q, applyQuery(queryClause))
   }
   for (const whereClause of config.where || []) {
     q = query(q, where(...whereClause))

packages/plugin-firestore/test/external/fetch.test.ts

@@ -1,6 +1,6 @@
+import { pokedex } from '@magnetarjs/test-utils'
 import test from 'ava'
 import { createMagnetarInstance } from '../helpers/createMagnetarInstance'
-import { pokedex } from '@magnetarjs/test-utils'
 
 {
   const testName = 'fetch (collection)'
@@ -276,6 +276,97 @@ import { pokedex } from '@magnetarjs/test-utils'
     }
   })
 }
+{
+  const testName = 'fetch (collection) query-filter: and'
+  test(testName, async (t) => {
+    const { pokedexModule } = await createMagnetarInstance('read')
+    try {
+      const queryModuleRef = pokedexModule
+        .query({
+          and: [
+            ['type', 'array-contains', 'Fire'],
+            ['base.Speed', '>=', 100],
+          ],
+        })
+        .orderBy('base.Speed', 'asc')
+      await queryModuleRef.fetch({ force: true }, { onError: 'stop' })
+      const actual = [...queryModuleRef.data.values()].map((p) => p.base.Speed)
+      const expected = [pokedex(6), pokedex(38), pokedex(78)].map((p) => p.base.Speed)
+      t.deepEqual(actual, expected as any)
+      // also check the collection without query
+      const actualDocCountWithoutQuery = pokedexModule.data.size
+      const expectedDocCountWithoutQuery = expected.length
+      t.deepEqual(actualDocCountWithoutQuery, expectedDocCountWithoutQuery)
+    } catch (error) {
+      t.fail(JSON.stringify(error))
+    }
+  })
+}
+{
+  const testName = 'fetch (collection) query-filter: or'
+  test(testName, async (t) => {
+    const { pokedexModule } = await createMagnetarInstance('read')
+    try {
+      const queryModuleRef = pokedexModule.query({
+        or: [
+          ['name', '==', 'Bulbasaur'],
+          ['name', '==', 'Ivysaur'],
+          ['name', '==', 'Venusaur'],
+        ],
+      })
+      await queryModuleRef.fetch({ force: true }, { onError: 'stop' })
+      const actual = [...queryModuleRef.data.values()].map((p) => p.base.Speed)
+      const expected = [pokedex(1), pokedex(2), pokedex(3)].map((p) => p.base.Speed)
+      t.deepEqual(actual, expected as any)
+      // also check the collection without query
+      const actualDocCountWithoutQuery = pokedexModule.data.size
+      const expectedDocCountWithoutQuery = expected.length
+      t.deepEqual(actualDocCountWithoutQuery, expectedDocCountWithoutQuery)
+    } catch (error) {
+      t.fail(JSON.stringify(error))
+    }
+  })
+}
+{
+  const testName = 'fetch (collection) query-filter: combine or, and'
+  test(testName, async (t) => {
+    const { pokedexModule } = await createMagnetarInstance('read')
+    try {
+      const queryModuleRef = pokedexModule.query({
+        or: [
+          {
+            and: [
+              ['name', '==', 'Bulbasaur'],
+              ['base.Speed', '==', 45],
+            ],
+          },
+          {
+            and: [
+              ['name', '==', 'Ivysaur'],
+              ['base.Speed', '==', 60],
+            ],
+          },
+          {
+            and: [
+              ['name', '==', 'Venusaur'],
+              ['base.Speed', '==', 80],
+            ],
+          },
+        ],
+      })
+      await queryModuleRef.fetch({ force: true }, { onError: 'stop' })
+      const actual = [...queryModuleRef.data.values()].map((p) => p.base.Speed)
+      const expected = [pokedex(1), pokedex(2), pokedex(3)].map((p) => p.base.Speed)
+      t.deepEqual(actual, expected as any)
+      // also check the collection without query
+      const actualDocCountWithoutQuery = pokedexModule.data.size
+      const expectedDocCountWithoutQuery = expected.length
+      t.deepEqual(actualDocCountWithoutQuery, expectedDocCountWithoutQuery)
+    } catch (error) {
+      t.fail(JSON.stringify(error))
+    }
+  })
+}
 {
   const testName = 'fetch (collection) compound queries'
   test(testName, async (t) => {

packages/types/src/types/clauses.ts

@@ -1,3 +1,4 @@
+import { isArray } from 'is-what'
 import { ArrayValues } from './utils/ArrayValues'
 import { DeepPropType } from './utils/DeepPropType'
 import { DefaultTo } from './utils/DefaultTo'
@@ -27,6 +28,12 @@ export type WhereFilterOp =
  */
 export type WhereClause = [string, WhereFilterOp, any]
 
+export function isWhereClause(
+  whereClauseOrQueryClause: WhereClause | QueryClause
+): whereClauseOrQueryClause is WhereClause {
+  return isArray(whereClauseOrQueryClause)
+}
+
 /**
  * Sort by the specified field, optionally in descending order instead of ascending.
  *
@@ -43,7 +50,9 @@ export type OrderByClause = [string, ('asc' | 'desc')?]
  * It has no knowledge on the actual types of the data.
  * The orderBy clause is defined in a more complex manner at `CollectionInstance["query"]`
  */
-export type QueryClause = { and: WhereClause[] | QueryClause } | { or: WhereClause[] | QueryClause }
+export type QueryClause =
+  | { and: (WhereClause | QueryClause)[] }
+  | { or: (WhereClause | QueryClause)[] }
 
 /**
  * The maximum number of items to return.
@@ -79,5 +88,5 @@ export type WhereClauseTuple<
 ]
 
 export type Query<T extends Record<string, any> = Record<string, any>> =
-  | { or: WhereClauseTuple<T>[] | Query<T> }
-  | { and: WhereClauseTuple<T>[] | Query<T> }
+  | { or: (WhereClauseTuple<T> | Query<T>)[] }
+  | { and: (WhereClauseTuple<T> | Query<T>)[] }

packages/utils/src/internal/dataHelpers.ts

@@ -1,4 +1,4 @@
-import { Clauses, QueryClause, WhereClause } from '@magnetarjs/types'
+import { Clauses, isWhereClause, QueryClause, WhereClause } from '@magnetarjs/types'
 import { ISortByObjectSorter, sort } from 'fast-sort'
 import { isArray, isNumber } from 'is-what'
 import { getProp } from 'path-to-prop'
@@ -49,14 +49,18 @@ function passesWhere(docData: Record<string, unknown>, whereQuery: WhereClause):
 
 function passesQuery(docData: Record<string, unknown>, queryClause: QueryClause): boolean {
   if ('and' in queryClause) {
-    return isArray(queryClause.and)
-      ? queryClause.and.every((whereClause) => passesWhere(docData, whereClause))
-      : passesQuery(docData, queryClause.and)
+    return queryClause.and.every((whereClauseOrQueryClause) =>
+      isWhereClause(whereClauseOrQueryClause)
+        ? passesWhere(docData, whereClauseOrQueryClause)
+        : passesQuery(docData, whereClauseOrQueryClause)
+    )
   }
   // if ('or' in queryClause)
-  return isArray(queryClause.or)
-    ? queryClause.or.some((whereClause) => passesWhere(docData, whereClause))
-    : passesQuery(docData, queryClause.or)
+  return queryClause.or.some((whereClauseOrQueryClause) =>
+    isWhereClause(whereClauseOrQueryClause)
+      ? passesWhere(docData, whereClauseOrQueryClause)
+      : passesQuery(docData, whereClauseOrQueryClause)
+  )
 }
 
 /**