fix(memory): Ensure correct pagination totals (#3307)

packages/memory/src/index.ts

@@ -76,7 +76,6 @@ export class MemoryAdapter<
     const { query, filters } = this.getQuery(params)
 
     let values = _.values(this.store)
-    const total = values.length
     const hasSkip = filters.$skip !== undefined
     const hasSort = filters.$sort !== undefined
     const hasLimit = filters.$limit !== undefined
@@ -86,11 +85,41 @@ export class MemoryAdapter<
       values.sort(this.options.sorter(filters.$sort))
     }
 
+    if (paginate) {
+      if (hasQuery) {
+        values = values.filter(this.options.matcher(query))
+      }
+
+      const total = values.length
+
+      if (hasSkip) {
+        values = values.slice(filters.$skip)
+      }
+
+      if (hasLimit) {
+        values = values.slice(0, filters.$limit)
+      }
+
+      const result: Paginated<Result> = {
+        total,
+        limit: filters.$limit,
+        skip: filters.$skip || 0,
+        data: values.map((value) => _select(value, params, this.id))
+      }
+
+      return result
+    }
+
+    /*  Without pagination, we don't have to match every result and gain considerable performance improvements with a breaking for loop. */
     if (hasQuery || hasLimit || hasSkip) {
       let skipped = 0
       const matcher = this.options.matcher(query)
       const matched = []
 
+      if (hasLimit && filters.$limit === 0) {
+        return []
+      }
+
       for (let index = 0, length = values.length; index < length; index++) {
         const value = values[index]
 
@@ -110,23 +139,10 @@ export class MemoryAdapter<
         }
       }
 
-      values = matched
-    } else {
-      values = values.map((value) => _select(value, params, this.id))
-    }
-
-    const result: Paginated<Result> = {
-      total: hasQuery ? values.length : total,
-      limit: filters.$limit,
-      skip: filters.$skip || 0,
-      data: filters.$limit === 0 ? [] : values
-    }
-
-    if (!paginate) {
-      return result.data
+      return matched
     }
 
-    return result
+    return values.map((value) => _select(value, params, this.id))
   }
 
   async _get(id: Id, params: ServiceParams = {} as ServiceParams): Promise<Result> {

packages/memory/test/index.test.ts

@@ -93,12 +93,31 @@ describe('Feathers Memory Service', () => {
   const events = ['testing']
   const app = feathers<{
     people: MemoryService<Person>
+    'people-paginate': MemoryService<Person>
     'people-customid': MemoryService<Person>
     animals: MemoryService<Animal>
     matcher: MemoryService
   }>()
 
-  app.use('people', new MemoryService<Person>({ events }))
+  app.use(
+    'people',
+    new MemoryService<Person>({
+      events
+    })
+  )
+
+  app.use(
+    'people-paginate',
+    new MemoryService<Person>({
+      events,
+      multi: true,
+      paginate: {
+        default: 10,
+        max: 100
+      }
+    })
+  )
+
   app.use(
     'people-customid',
     new MemoryService<Person>({
@@ -218,6 +237,44 @@ describe('Feathers Memory Service', () => {
     await people.remove(person.id)
   })
 
+  it('using $limit still returns correct total', async () => {
+    const service = app.service('people-paginate')
+
+    for (let i = 0; i < 10; i++) {
+      await service.create({
+        name: `Tester ${i}`,
+        age: 19
+      })
+
+      await service.create({
+        name: `Tester ${i}`,
+        age: 20
+      })
+    }
+
+    try {
+      const results = await service.find({
+        query: {
+          $skip: 3,
+          $limit: 5,
+          age: 19
+        }
+      })
+
+      assert.strictEqual(results.total, 10)
+      assert.strictEqual(results.skip, 3)
+      assert.strictEqual(results.limit, 5)
+    } finally {
+      await service.remove(null, {
+        query: {
+          age: {
+            $in: [19, 20]
+          }
+        }
+      })
+    }
+  })
+
   testSuite(app, errors, 'people')
   testSuite(app, errors, 'people-customid', 'customid')
 })