fix: inefficient traversal and intermediate allocations (#1164)

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>

Author: ferhatelmas

src/http/routes/s3/commands/put-object.ts

@@ -240,26 +240,25 @@ export default function PutObject(s3Router: S3Router) {
 }
 
 function fieldsToObject(fields: MultipartFields) {
-  return Object.keys(fields).reduce(
-    (acc, key) => {
-      const field = fields[key]
-      if (Array.isArray(field)) {
-        return acc
-      }
+  const acc: Record<string, string> = {}
 
-      if (!field) {
-        return acc
-      }
+  for (const key in fields) {
+    if (!Object.prototype.hasOwnProperty.call(fields, key)) {
+      continue
+    }
 
-      if (
-        field.type === 'field' &&
-        (typeof field.value === 'string' || field.value === 'number' || field.value === 'boolean')
-      ) {
-        acc[field.fieldname.toLowerCase()] = field.value
-      }
+    const field = fields[key]
+    if (Array.isArray(field) || !field) {
+      continue
+    }
 
-      return acc
-    },
-    {} as Record<string, string>
-  )
+    if (
+      field.type === 'field' &&
+      (typeof field.value === 'string' || field.value === 'number' || field.value === 'boolean')
+    ) {
+      acc[field.fieldname.toLowerCase()] = field.value
+    }
+  }
+
+  return acc
 }

src/http/routes/s3/index.ts

@@ -99,11 +99,15 @@ export default async function routes(fastify: FastifyInstance) {
                 const headers = output.headers
 
                 if (headers) {
-                  Object.keys(headers).forEach((header) => {
+                  for (const header in headers) {
+                    if (!Object.prototype.hasOwnProperty.call(headers, header)) {
+                      continue
+                    }
+
                     if (headers[header]) {
                       reply.header(header, headers[header])
                     }
-                  })
+                  }
                 }
                 return reply.status(output.statusCode || 200).send(output.responseBody)
               } catch (e) {

src/internal/testing/generators/array.ts

@@ -6,20 +6,3 @@ export async function eachParallel<T>(times: number, fn: (index: number) => Prom
 
   return Promise.all(promises)
 }
-
-// export function pickRandomFromArray<T>(arr: T[]): T {
-//   return arr[Math.floor(Math.random() * arr.length)]
-// }
-
-// export function pickRandomRangeFromArray<T>(arr: T[], range: number): T[] {
-//   if (arr.length <= range) {
-//     return arr
-//   }
-
-//   const result = new Set<T>()
-//   while (result.size < range) {
-//     result.add(pickRandomFromArray(arr))
-//   }
-
-//   return Array.from(result)
-// }

src/storage/backend/s3/adapter.test.ts

@@ -2,6 +2,7 @@ import {
   DeleteObjectsCommand,
   GetObjectCommand,
   HeadObjectCommand,
+  ListObjectsV2Command,
   PutObjectCommand,
   S3Client,
 } from '@aws-sdk/client-s3'
@@ -163,6 +164,53 @@ describe('S3Backend', () => {
     })
   })
 
+  describe('list', () => {
+    test('filters listed keys by cutoff date and strips the requested prefix', async () => {
+      mockSend.mockResolvedValue({
+        Contents: [
+          {
+            Key: 'tenant/bucket/old.txt',
+            LastModified: new Date('2024-01-01T00:00:00.000Z'),
+            Size: 12,
+          },
+          {
+            Key: 'tenant/bucket/new.txt',
+            LastModified: new Date('2024-01-03T00:00:00.000Z'),
+            Size: 34,
+          },
+          {
+            Key: 'tenant/bucket/no-date.txt',
+            Size: 56,
+          },
+          {
+            LastModified: new Date('2024-01-01T00:00:00.000Z'),
+            Size: 78,
+          },
+        ],
+        NextContinuationToken: 'next-page',
+      })
+
+      const backend = createBackend()
+
+      await expect(
+        backend.list('test-bucket', {
+          prefix: 'tenant/bucket',
+          beforeDate: new Date('2024-01-02T00:00:00.000Z'),
+        })
+      ).resolves.toEqual({
+        keys: [{ name: 'old.txt', size: 12 }],
+        nextToken: 'next-page',
+      })
+
+      expect(mockSend).toHaveBeenCalledTimes(1)
+      expect(mockSend.mock.calls[0][0]).toBeInstanceOf(ListObjectsV2Command)
+      expect(mockSend.mock.calls[0][0].input).toMatchObject({
+        Bucket: 'test-bucket',
+        Prefix: 'tenant/bucket',
+      })
+    })
+  })
+
   describe('deleteObjects', () => {
     test('chunks DeleteObjectsCommand payloads to the S3 key limit', async () => {
       mockSend.mockResolvedValue({

src/storage/backend/s3/adapter.ts

@@ -393,26 +393,27 @@ export class S3Backend implements StorageBackendAdapter {
         StartAfter: options?.startAfter,
       })
       const data = await this.client.send(command)
-      const keys =
-        data.Contents?.filter((ele) => {
-          if (options?.beforeDate) {
-            if (ele.LastModified && ele.LastModified < options.beforeDate) {
-              return ele.Key as string
-            }
-            return false
-          }
-          return ele.Key
-        }).map((ele) => {
-          if (options?.prefix) {
-            return {
-              // remove prefix and leading slash if present
-              name: (ele.Key as string).replace(options.prefix, '').replace(/^\//, ''),
-              size: ele.Size as number,
-            }
-          }
-
-          return { name: ele.Key as string, size: ele.Size as number }
-        }) || []
+      const keys: { name: string; size: number }[] = []
+
+      for (const ele of data.Contents || []) {
+        if (!ele.Key) {
+          continue
+        }
+
+        if (options?.beforeDate && (!ele.LastModified || ele.LastModified >= options.beforeDate)) {
+          continue
+        }
+
+        if (options?.prefix) {
+          keys.push({
+            // remove prefix and leading slash if present
+            name: ele.Key.replace(options.prefix, '').replace(/^\//, ''),
+            size: ele.Size as number,
+          })
+        } else {
+          keys.push({ name: ele.Key, size: ele.Size as number })
+        }
+      }
 
       return {
         keys,