From 7be93b1fbb55c098a0afb904a868005bc9afb475 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Tue, 17 Nov 2020 09:53:50 +0000 Subject: [PATCH] API: Make sure pagination sort options are respected for search and other requests --- joplin.code-workspace | 1 + packages/app-cli/package.json | 1 + packages/app-cli/tests/services_rest_Api.ts | 61 ++++++++++++++++++- .../utils/collectionToPaginatedResults.ts | 30 ++++++++- 4 files changed, 89 insertions(+), 4 deletions(-) diff --git a/joplin.code-workspace b/joplin.code-workspace index a1724691d..350011c68 100644 --- a/joplin.code-workspace +++ b/joplin.code-workspace @@ -336,5 +336,6 @@ "**/*.js": {"when": "$(basename).ts"}, "**/*?.js": { "when": "$(basename).tsx"}, }, + "cSpell.enabled": true, } } \ No newline at end of file diff --git a/packages/app-cli/package.json b/packages/app-cli/package.json index 18287f80c..fc03a9fc0 100644 --- a/packages/app-cli/package.json +++ b/packages/app-cli/package.json @@ -6,6 +6,7 @@ "private": true, "scripts": { "test": "jest --config=jest.config.js --runInBand --bail --forceExit", + "test-one": "jest --verbose=false --config=jest.config.js --runInBand --bail --forceExit", "test-ci": "jest --config=jest.config.js --runInBand --forceExit", "build": "gulp build", "start": "gulp build -L && node \"build/main.js\" --stack-trace-enabled --log-level debug --env dev", diff --git a/packages/app-cli/tests/services_rest_Api.ts b/packages/app-cli/tests/services_rest_Api.ts index e8c413a0f..ffae40656 100644 --- a/packages/app-cli/tests/services_rest_Api.ts +++ b/packages/app-cli/tests/services_rest_Api.ts @@ -27,10 +27,13 @@ const createFolderForPagination = async (num: number, time: number) => { }, { autoTimestamp: false }); }; -const createNoteForPagination = async (num: number, time: number) => { +const createNoteForPagination = async (numOrTitle: number | string, time: number) => { + const title = typeof numOrTitle === 'string' ? numOrTitle : `note${numOrTitle}`; + const body = typeof numOrTitle === 'string' ? `Note body ${numOrTitle}` : `noteBody${numOrTitle}`; + await Note.save({ - title: `note${num}`, - body: `noteBody${num}`, + title: title, + body: body, updated_time: time, created_time: time, }, { autoTimestamp: false }); @@ -643,6 +646,58 @@ describe('services_rest_Api', function() { expect(r2.items[0].id).toBe(note3.id); })); + it('should sort search paginated results', asyncTest(async () => { + SearchEngine.instance().setDb(db()); + + await createNoteForPagination('note c', 1000); + await createNoteForPagination('note e', 1001); + await createNoteForPagination('note b', 1002); + await createNoteForPagination('note a', 1003); + await createNoteForPagination('note d', 1004); + + await SearchEngine.instance().syncTables(); + + { + const baseQuery = { + query: 'note', + fields: ['id', 'title', 'updated_time'], + limit: 3, + order_dir: PaginationOrderDir.ASC, + order_by: 'updated_time', + }; + + const r1 = await api.route(RequestMethod.GET, 'search', baseQuery); + expect(r1.items[0].updated_time).toBe(1000); + expect(r1.items[1].updated_time).toBe(1001); + expect(r1.items[2].updated_time).toBe(1002); + + const r2 = await api.route(RequestMethod.GET, 'search', { ...baseQuery, page: 2 }); + expect(r2.items[0].updated_time).toBe(1003); + expect(r2.items[1].updated_time).toBe(1004); + } + + { + const baseQuery = { + query: 'note', + fields: ['id', 'title', 'updated_time'], + limit: 2, + order_dir: PaginationOrderDir.DESC, + order_by: 'title', + }; + + const r1 = await api.route(RequestMethod.GET, 'search', baseQuery); + expect(r1.items[0].title).toBe('note e'); + expect(r1.items[1].title).toBe('note d'); + + const r2 = await api.route(RequestMethod.GET, 'search', { ...baseQuery, page: 2 }); + expect(r2.items[0].title).toBe('note c'); + expect(r2.items[1].title).toBe('note b'); + + const r3 = await api.route(RequestMethod.GET, 'search', { ...baseQuery, page: 3 }); + expect(r3.items[0].title).toBe('note a'); + } + })); + it('should return default fields', asyncTest(async () => { const folder = await Folder.save({ title: 'folder' }); const note1 = await Note.save({ title: 'note1', parent_id: folder.id }); diff --git a/packages/lib/services/rest/utils/collectionToPaginatedResults.ts b/packages/lib/services/rest/utils/collectionToPaginatedResults.ts index 0979159a5..ab0024e7c 100644 --- a/packages/lib/services/rest/utils/collectionToPaginatedResults.ts +++ b/packages/lib/services/rest/utils/collectionToPaginatedResults.ts @@ -1,15 +1,43 @@ import { ModelFeedPage } from '../../../models/utils/paginatedFeed'; +import { PaginationOrderDir } from '../../../models/utils/types'; import { Request } from '../Api'; import requestPaginationOptions from '../utils/requestPaginationOptions'; +// Note that this is inefficient pagination as it requires all the items to +// have been loaded first, even if not all of them are returned. +// +// It's however convenient for smaller lists as it reduces the need for +// building complex SQL queries. export default function(items: any[], request: Request): ModelFeedPage { const pagination = requestPaginationOptions(request); const startIndex = (pagination.page - 1) * pagination.limit; const itemCount = Math.min(items.length - startIndex, pagination.limit); const hasMore = itemCount >= pagination.limit; + const sortBy = pagination.order[0].by; + const sortDir = pagination.order[0].dir; + const caseInsensitive = pagination.order[0].caseInsensitive; + const sortedItems = items.slice(); + + sortedItems.sort((a: any, b: any) => { + let v1 = a && (sortBy in a) ? a[sortBy] : ''; + let v2 = b && (sortBy in b) ? b[sortBy] : ''; + if (caseInsensitive && typeof v1 === 'string') v1 = v1.toLowerCase(); + if (caseInsensitive && typeof v2 === 'string') v2 = v2.toLowerCase(); + + if (sortDir === PaginationOrderDir.ASC) { + if (v1 < v2) return -1; + if (v2 > v1) return +1; + } else { + if (v1 > v2) return -1; + if (v2 < v1) return +1; + } + + return 0; + }); + return { - items: items.slice(startIndex, startIndex + itemCount), + items: sortedItems.slice(startIndex, startIndex + itemCount), has_more: hasMore, }; }