fix: add pagination to user/uploads endpoint (#1408)

* fix: add initial `offset` param for `GET user/uploads` to support pagination. Add removed `after` filtering parameter to the endpoint. Document `sortBy`, `sortOrder`, `after` & `offset` in the swagger docs. Set `Size` param default to 10 from 25.

* fix: Revert default size back to 25, update tests.

* fix: Add tests for pagination to ensure correct link with offset and size is returned. Add tests for date filtering (before & after) and sorting by date.

* fix: Add test for sorting by `sortOrder=Asc` in the `GET user/uploads` endpoint in the API.

* fix: Fix linting

* fix: Adds `count`, `size`, `offset`, and `Prev_link` to header response for get user/uploads to allow for pagination. Renames `Link` to `Next_link` in header to be more descriptive.

* fix: Address PR feedback, fix linting.

* fix: remove .only in test, change `.not` to `.notStrictEqual`

* fix: Fix the db tests to handle a returned count as well as upload.

* fix: Address PR feedback on tests, use `.ok` over `.notStrictEqual` to prevent comparing to message string. Remove .only.

* fix: Add correct cors headers so that upload metadata can be accessed.

* fix: Address prfeedback, use `page` over `offset` param.

* chore: update documentation and headers to use page over offset.

* fix: Address varous PR feedback.

* fix: Add indexes on the upload table for `name` and `inserted_at` to optimise `sortBy` queries on the `GET user/uploads` endpoint.

* fix: Fix broken tests for uploaads Link header

* chore: RM generate_uploads.js file

* fix: RM index creation for upload sorting.

* fix: Add validation for `sortOrder` in the uploads API. Remove url encoding of variables for header links.

packages/api/src/cors.js

@@ -22,7 +22,7 @@ export function corsOptions (request) {
       // such as Authorization (Bearer) or X-Client-Name-Version
       'Access-Control-Allow-Headers':
         headers.get('Access-Control-Request-Headers') || '',
-      'Access-Control-Expose-Headers': 'Link'
+      'Access-Control-Expose-Headers': 'Link, Count, Page, Size'
     }
 
     return new Response(null, {
@@ -67,6 +67,6 @@ export function addCorsHeaders (request, response) {
   } else {
     response.headers.set('Access-Control-Allow-Origin', '*')
   }
-  response.headers.set('Access-Control-Expose-Headers', 'Link')
+  response.headers.set('Access-Control-Expose-Headers', 'Link, Count, Page, Size')
   return response
 }

packages/api/src/user.js

@@ -221,6 +221,9 @@ export async function userTokensDelete (request, env) {
   return new JSONResponse(res)
 }
 
+const sortableValues = ['Name', 'Date']
+const sortableOrders = ['Asc', 'Desc']
+
 /**
  * Retrieve a page of user uploads.
  *
@@ -240,37 +243,74 @@ export async function userUploadsGet (request, env) {
     size = parsedSize
   }
 
-  let before = new Date()
+  let offset = 0
+  let page = 1
+  if (searchParams.has('page')) {
+    const parsedPage = parseInt(searchParams.get('page'))
+    if (isNaN(parsedPage) || parsedPage <= 0) {
+      throw Object.assign(new Error('invalid page number'), { status: 400 })
+    }
+    offset = (parsedPage - 1) * size
+    page = parsedPage
+  }
+
+  let before
   if (searchParams.has('before')) {
     const parsedBefore = new Date(searchParams.get('before'))
     if (isNaN(parsedBefore.getTime())) {
       throw Object.assign(new Error('invalid before date'), { status: 400 })
     }
-    before = parsedBefore
+    before = parsedBefore.toISOString()
+  }
+
+  let after
+  if (searchParams.has('after')) {
+    const parsedAfter = new Date(searchParams.get('after'))
+    if (isNaN(parsedAfter.getTime())) {
+      throw Object.assign(new Error('invalid after date'), { status: 400 })
+    }
+    after = parsedAfter.toISOString()
   }
 
   const sortBy = searchParams.get('sortBy') || 'Date'
   const sortOrder = searchParams.get('sortOrder') || 'Desc'
 
-  const uploads = await env.db.listUploads(request.auth.user._id, {
+  if (!sortableOrders.includes(sortOrder)) {
+    throw Object.assign(new Error(`Sort ordering by '${sortOrder}' is not supported. Supported sort orders are: [${sortableOrders.toString()}]`), { status: 400 })
+  }
+
+  if (!sortableValues.includes(sortBy)) {
+    throw Object.assign(new Error(`Sorting by '${sortBy}' is not supported. Supported sort orders are: [${sortableValues.toString()}]`), { status: 400 })
+  }
+
+  const data = await env.db.listUploads(request.auth.user._id, {
     size,
-    before: before.toISOString(),
+    offset,
+    before,
+    after,
     sortBy,
     sortOrder
   })
 
-  const oldest = uploads[uploads.length - 1]
-  const headers =
-    uploads.length === size
-      ? {
-          Link: `<${
-            requestUrl.pathname
-          }?size=${size}&before=${encodeURIComponent(
-            oldest.created
-          )}>; rel="next"`
-        }
-      : undefined
-  return new JSONResponse(uploads, { headers })
+  let link = ''
+  // If there's more results to show...
+  if (page > 1) {
+    link += `<${requestUrl.pathname}?size=${size}&page=${page - 1}>; rel="previous"`
+  }
+
+  if (data.uploads.length + offset < data.count) {
+    if (link !== '') link += ', '
+    link += `<${requestUrl.pathname}?size=${size}&page=${page + 1}>; rel="next"`
+  }
+
+  const headers = {
+    Count: data.count,
+    Size: size,
+    Page: page,
+    Link: link
+  }
+
+  return new JSONResponse(data.uploads, { headers })
 }
 
 /**

packages/api/test/user.spec.js

@@ -209,6 +209,80 @@ describe('GET /user/uploads', () => {
     assert.deepStrictEqual(uploads, [...userUploads].sort((a, b) => b.name.localeCompare(a.name)))
   })
 
+  it('lists uploads sorted by date', async () => {
+    const token = await getTestJWT()
+    const res = await fetch(new URL('/user/uploads?sortBy=Date', endpoint).toString(), {
+      method: 'GET',
+      headers: { Authorization: `Bearer ${token}` }
+    })
+    assert(res.ok)
+    const uploads = await res.json()
+    assert.deepStrictEqual(uploads, [...userUploads].sort((a, b) => b.created.localeCompare(a.created)))
+  })
+
+  it('lists uploads in reverse order when sorting by Asc', async () => {
+    const token = await getTestJWT()
+    const res = await fetch(new URL('/user/uploads?sortBy=Name&sortOrder=Asc', endpoint).toString(), {
+      method: 'GET',
+      headers: { Authorization: `Bearer ${token}` }
+    })
+
+    assert(res.ok)
+
+    const uploads = await res.json()
+    const sortedUploads = [...userUploads].sort((a, b) => a.name.localeCompare(b.name))
+
+    assert.deepStrictEqual(uploads, sortedUploads)
+  })
+
+  it('filters results by before date', async () => {
+    const token = await getTestJWT()
+
+    const beforeFilterDate = new Date('2021-07-10T00:00:00.000000+00:00').toISOString()
+    const res = await fetch(new URL(`/user/uploads?before=${beforeFilterDate}`, endpoint).toString(), {
+      method: 'GET',
+      headers: { Authorization: `Bearer ${token}` }
+    })
+
+    assert(res.ok)
+
+    const uploads = await res.json()
+
+    assert(uploads.length < userUploads.length, 'Ensure some results are filtered out.')
+    assert(uploads.length > 0, 'Ensure some results are returned.')
+
+    // Filter uploads fixture by the filter date.
+    const uploadsBeforeFilterDate = userUploads.filter((upload) => {
+      return upload.created <= beforeFilterDate
+    })
+
+    assert.deepStrictEqual(uploads, [...uploadsBeforeFilterDate])
+  })
+
+  it('filters results by after date', async () => {
+    const token = await getTestJWT()
+
+    const afterFilterDate = new Date('2021-07-10T00:00:00.000000+00:00').toISOString()
+    const res = await fetch(new URL(`/user/uploads?after=${afterFilterDate}`, endpoint).toString(), {
+      method: 'GET',
+      headers: { Authorization: `Bearer ${token}` }
+    })
+
+    assert(res.ok)
+
+    const uploads = await res.json()
+
+    assert(uploads.length < userUploads.length, 'Ensure some results are filtered out.')
+    assert(uploads.length > 0, 'Ensure some results are returned.')
+
+    // Filter uploads fixture by the filter date.
+    const uploadsAfterFilterDate = userUploads.filter((upload) => {
+      return upload.created >= afterFilterDate
+    })
+
+    assert.deepStrictEqual(uploads, [...uploadsAfterFilterDate])
+  })
+
   it('lists uploads via magic auth', async () => {
     const token = 'test-magic'
     const res = await fetch(new URL('/user/uploads', endpoint).toString(), {
@@ -220,20 +294,47 @@ describe('GET /user/uploads', () => {
     assert.deepStrictEqual(uploads, userUploads)
   })
 
-  it('paginates', async () => {
+  it('paginates by page', async () => {
     const token = await getTestJWT()
     const size = 1
+    const page = 2
+    const res = await fetch(new URL(`/user/uploads?size=${size}&page=${page}`, endpoint).toString(), {
+      method: 'GET',
+      headers: { Authorization: `Bearer ${token}` }
+    })
+    assert(res.ok)
+
+    // Ensure we have all pagination metadata in the headers.
+    const link = res.headers.get('link')
+    assert(link, 'has a link header for the next page')
+    assert.strictEqual(link, `</user/uploads?size=${size}&page=${page - 1}>; rel="previous", </user/uploads?size=${size}&page=${page + 1}>; rel="next"`)
+
+    const resCount = res.headers.get('Count')
+    assert.strictEqual(parseInt(resCount), userUploads.length, 'has a count for calculating page numbers')
+
+    const resSize = res.headers.get('Size')
+    assert.strictEqual(parseInt(resSize), size, 'has a size for calculating page numbers')
+
+    const resPage = res.headers.get('Page')
+    assert.strictEqual(parseInt(resPage), page, 'has a page number for calculating page numbers')
+
+    // Should get second result (page 2).
+    const uploads = await res.json()
+    const expected = [userUploads[1]]
+    assert.deepStrictEqual(uploads, expected)
+  })
+
+  it('does not paginate when all results are returned', async () => {
+    const token = await getTestJWT()
+    const size = 1000
     const res = await fetch(new URL(`/user/uploads?size=${size}`, endpoint).toString(), {
       method: 'GET',
       headers: { Authorization: `Bearer ${token}` }
     })
     assert(res.ok)
 
-    const expected = [userUploads[0]]
-    const link = res.headers.get('Link')
-    assert(link, 'has a Link header for the next page')
-    assert.strictEqual(link, `</user/uploads?size=${size}&before=${encodeURIComponent(expected[0].created)}>; rel="next"`)
     const uploads = await res.json()
+    const expected = userUploads
     assert.deepStrictEqual(uploads, expected)
   })
 })

packages/db/db-client-types.ts

@@ -254,6 +254,10 @@ export type ListUploadsOptions = {
    * Max records (default: 10).
    */
   size?: number
+  /**
+   * Offset records (default: 0).
+   */
+  offset?: number
   /**
    * Sort by given property.
    */
@@ -264,8 +268,12 @@ export type ListUploadsOptions = {
   sortOrder?: 'Asc' | 'Desc'
 }
 
+export type ListUploadReturn = {
+  count: number,
+  uploads: Promise<UploadItemOutput[]>,
+}
 
-// Pinninng
+// Pinning
 
 // PinRequest
 export type PsaPinRequestUpsertInput = {

packages/db/index.js

@@ -76,6 +76,12 @@ const listPinsQuery = `
     )
   )`
 
+/**  Mapping of Upload table sortable columns to ListUploads sortBy arguments. */
+const sortableColumnToUploadArgMap = {
+  inserted_at: 'Date',
+  name: 'Name'
+}
+
 /**
  * @typedef {import('./postgres/pg-rest-api-types').definitions} definitions
  * @typedef {import('@supabase/postgrest-js').PostgrestError} PostgrestError
@@ -511,19 +517,27 @@ export class DBClient {
    *
    * @param {number} userId
    * @param {import('./db-client-types').ListUploadsOptions} [opts]
-   * @returns {Promise<Array<import('./db-client-types').UploadItemOutput>>}
+   * @returns {import('./db-client-types').ListUploadReturn}
    */
   async listUploads (userId, opts = {}) {
+    const rangeFrom = opts.offset || 0
+    const rangeTo = rangeFrom + (opts.size || 25)
+    const isAscendingSortOrder = opts.sortOrder === 'Asc'
+    const defaultSortByColumn = Object.keys(sortableColumnToUploadArgMap)[0]
+    const sortByColumn = Object.keys(sortableColumnToUploadArgMap).find(key => sortableColumnToUploadArgMap[key] === opts.sortBy)
+    const sortBy = sortByColumn || defaultSortByColumn
+
     let query = this._client
       .from('upload')
-      .select(uploadQuery)
+      .select(uploadQuery, { count: 'exact' })
       .eq('user_id', userId)
       .is('deleted_at', null)
-      .limit(opts.size || 10)
-      .order(opts.sortBy === 'Name' ? 'name' : 'inserted_at', {
-        ascending: opts.sortOrder === 'Asc'
-      })
+      .order(
+        sortBy,
+        { ascending: isAscendingSortOrder }
+      )
 
+    // Apply filtering
     if (opts.before) {
       query = query.lt('inserted_at', opts.before)
     }
@@ -532,8 +546,11 @@ export class DBClient {
       query = query.gte('inserted_at', opts.after)
     }
 
-    /** @type {{ data: Array<import('./db-client-types').UploadItem>, error: Error }} */
-    const { data: uploads, error } = await query
+    // Apply pagination or limiting.
+    query = query.range(rangeFrom, rangeTo - 1)
+
+    /** @type {{ data: Array<import('./db-client-types').UploadItem>, error: Error, count: Number }} */
+    const { data: uploads, error, count } = await query
 
     if (error) {
       throw new DBError(error)
@@ -543,10 +560,13 @@ export class DBClient {
     const cids = uploads?.map((u) => u.content.cid)
     const deals = await this.getDealsForCids(cids)
 
-    return uploads?.map((u) => ({
-      ...normalizeUpload(u),
-      deals: deals[u.content.cid] || []
-    }))
+    return {
+      count,
+      uploads: uploads?.map((u) => ({
+        ...normalizeUpload(u),
+        deals: deals[u.content.cid] || []
+      }))
+    }
   }
 
   /**