From b912d92c804ba411c197028776695803349aeb19 Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> Date: Wed, 19 Feb 2025 21:08:05 -0500 Subject: [PATCH 01/12] Feat: Add users collection pagination Signed-off-by: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> --- src/principal/service.ts | 23 +- src/user/controller/collection.ts | 9 +- src/user/formats/hal.ts | 28 ++- test/pagination/collection.ts | 339 ++++++++++++++++++++++++++++++ 4 files changed, 389 insertions(+), 10 deletions(-) create mode 100644 test/pagination/collection.ts diff --git a/src/principal/service.ts b/src/principal/service.ts index f93968b8..94be4f6d 100644 --- a/src/principal/service.ts +++ b/src/principal/service.ts @@ -46,11 +46,11 @@ export class PrincipalService { } - async findAll(type: 'user'): Promise; + async findAll(type: 'user', page: number): Promise; async findAll(type: 'group'): Promise; async findAll(type: 'app'): Promise; async findAll(): Promise; - async findAll(type?: PrincipalType): Promise { + async findAll(type?: PrincipalType, page?: number): Promise { this.privileges.require('a12n:principals:list'); const filters: Record = {}; @@ -58,8 +58,23 @@ export class PrincipalService { filters.type = userTypeToInt(type); } - const result = await db('principals') - .where(filters); + let result: PrincipalsRecord[] = []; + + if(type && page !== undefined){ + + page = page < 1 ? 1 : page; + const pageSize = 100; + const offset = (page - 1) * pageSize; + + result = await db('principals') + .where(filters) + .limit(pageSize) + .offset(offset); + + } else { + result = await db('principals') + .where(filters); + } const principals: Principal[] = []; for (const principal of result) { diff --git a/src/user/controller/collection.ts b/src/user/controller/collection.ts index 9165485a..8a7a1cc1 100644 --- a/src/user/controller/collection.ts +++ b/src/user/controller/collection.ts @@ -11,7 +11,11 @@ class UserCollectionController extends Controller { async get(ctx: Context) { const principalService = new services.principal.PrincipalService(ctx.privileges); - const users = await principalService.findAll('user'); + + const pageInt = parseInt(ctx.request.query.page); + const page = isNaN(pageInt) ? 1 : pageInt; + + const users = await principalService.findAll('user', page); const embed = ctx.request.prefer('transclude').toString().includes('item') || ctx.query.embed?.includes('item'); const embeddedUsers: HalResource[] = []; @@ -36,7 +40,8 @@ class UserCollectionController extends Controller { } } - ctx.response.body = hal.collection(users, embeddedUsers); + const pageSize = 100; + ctx.response.body = hal.collection(users, embeddedUsers, page, pageSize); } diff --git a/src/user/formats/hal.ts b/src/user/formats/hal.ts index 811b7656..e0fc0d45 100644 --- a/src/user/formats/hal.ts +++ b/src/user/formats/hal.ts @@ -4,12 +4,22 @@ import { HalResource } from 'hal-types'; import { LazyPrivilegeBox } from '../../privilege/service.ts'; import { UserNewResult } from '../../api-types.ts'; -export function collection(users: User[], embeddedUsers: HalResource[]): HalResource { +export function collection(users: User[], embeddedUsers: HalResource[], currentPage: number, pageSize: number): HalResource { + + const totalUsers = users.length; + const totalPages = Math.ceil(totalUsers / pageSize); + const nextPage = currentPage < totalPages ? currentPage + 1 : null; + const prevPage = currentPage > 1 ? currentPage - 1 : null; + + const startIdx = (currentPage - 1) * pageSize; + const endIdx = Math.min(startIdx + pageSize, totalUsers); + + const paginatedUsers = users.slice(startIdx, endIdx); const hal: HalResource = { _links: { - 'self': { href: '/user' }, - 'item': users.map( user => ({ + 'self': { href: `/user?page=${currentPage}` }, + 'item': paginatedUsers.map( user => ({ href: user.href, title: user.nickname, })), @@ -20,9 +30,19 @@ export function collection(users: User[], embeddedUsers: HalResource[]): HalReso templated: true, }, }, - total: users.length, + total: totalUsers, + currentPage, + totalPages, }; + if(nextPage){ + hal._links['next'] = { href: `/user?page=${nextPage}` }; + } + + if(prevPage){ + hal._links['previous'] = { href: `/user?page=${prevPage}` }; + } + if (embeddedUsers.length) { hal._embedded = { item: embeddedUsers diff --git a/test/pagination/collection.ts b/test/pagination/collection.ts new file mode 100644 index 00000000..a92bb7b2 --- /dev/null +++ b/test/pagination/collection.ts @@ -0,0 +1,339 @@ +import { strict as assert } from 'node:assert'; +import { describe, it } from 'node:test'; + +import * as hal from '../../src/user/formats/hal.ts'; +import { User } from '../../src/types.ts'; +import { HalResource } from 'hal-types'; + +describe('hal.collection', () => { + const users: User[] = [ + { + id: 1,href: '/user/1', + externalId: 'user-1', + type: 'user', + nickname: 'User One', + createdAt: new Date(Date.now()), + modifiedAt: new Date(Date.now()), + active: true, + system: false, + }, + { + id: 2, + href: '/user/2', + externalId: 'user-2', + type: 'user', + nickname: 'User Two', + createdAt: new Date(Date.now()), + modifiedAt: new Date(Date.now()), + active: true, + system: false, + }, + { + id: 3, + href: '/user/3', + externalId: 'user-3', + type: 'user', + nickname: 'User Three', + createdAt: new Date(Date.now()), + modifiedAt: new Date(Date.now()), + active: true, + system: false, + }, + { + id: 4, + href: '/user/4', + externalId: 'user-4', + type: 'user', + nickname: 'User Four', + createdAt: new Date(Date.now()), + modifiedAt: new Date(Date.now()), + active: true, + system: false, + }, + { + id: 5, + href: '/user/5', + externalId: 'user-5', + type: 'user', + nickname: 'User Five', + createdAt: new Date(Date.now()), + modifiedAt: new Date(Date.now()), + active: true, + system: false, + }, + { + id: 6, + href: '/user/6', + externalId: 'user-6', + type: 'user', + nickname: 'User Six', + createdAt: new Date(Date.now()), + modifiedAt: new Date(Date.now()), + active: true, + system: false, + }, + { + id: 7, + href: '/user/7', + externalId: 'user-7', + type: 'user', + nickname: 'User Seven', + createdAt: new Date(Date.now()), + modifiedAt: new Date(Date.now()), + active: true, + system: false, + }, + { + id: 8, + href: '/user/8', + externalId: 'user-8', + type: 'user', + nickname: 'User Eight', + createdAt: new Date(Date.now()), + modifiedAt: new Date(Date.now()), + active: true, + system: false, + }, + { + id: 9, + href: '/user/9', + externalId: 'user-9', + type: 'user', + nickname: 'User Nine', + createdAt: new Date(Date.now()), + modifiedAt: new Date(Date.now()), + active: true, + system: false, + }, + { + id: 10, + href: '/user/10', + externalId: 'user-10', + type: 'user', + nickname: 'User Ten', + createdAt: new Date(Date.now()), + modifiedAt: new Date(Date.now()), + active: true, + system: false, + }, + ]; + + it('should display all users', () => { + const embeddedUsers: HalResource[] = []; + const currentPage = 1; + const pageSize = 100; + + const halRes = hal.collection(users, embeddedUsers, currentPage, pageSize); + + const expected = { + _links: { + 'create-form': { + href: '/user/new', + title: 'Create New User', + type: 'text/html' + }, + 'find-by-href': { + href: '/user/byhref/{href}', + templated: true, + title: 'Find a user through a identity/href (exact match)' + }, + item: [ + { + href: '/user/1', + title: 'User One' + }, + { + href: '/user/2', + title: 'User Two' + }, + { + href: '/user/3', + title: 'User Three' + }, + { + href: '/user/4', + title: 'User Four' + }, + { + href: '/user/5', + title: 'User Five' + }, + { + href: '/user/6', + title: 'User Six' + }, + { + href: '/user/7', + title: 'User Seven' + }, + { + href: '/user/8', + title: 'User Eight' + }, + { + href: '/user/9', + title: 'User Nine' + }, + { + href: '/user/10', + title: 'User Ten' + } + ], + self: { + href: '/user?page=1' + } + }, + currentPage: 1, + total: 10, + totalPages: 1 + }; + + assert.deepEqual(halRes, expected); + + }); + + it('should display the first 3 users', () => { + const embeddedUsers: HalResource[] = []; + const currentPage = 1; + const pageSize = 3; + + const halRes = hal.collection(users, embeddedUsers, currentPage, pageSize); + + const expected = { + _links: { + 'create-form': { + href: '/user/new', + title: 'Create New User', + type: 'text/html' + }, + 'find-by-href': { + href: '/user/byhref/{href}', + templated: true, + title: 'Find a user through a identity/href (exact match)' + }, + item: [ + { + href: '/user/1', + title: 'User One' + }, + { + href: '/user/2', + title: 'User Two' + }, + { + href: '/user/3', + title: 'User Three' + }, + ], + next: { + href: '/user?page=2' + }, + self: { + href: '/user?page=1' + } + }, + currentPage: 1, + total: 10, + totalPages: 4 + }; + + assert.deepEqual(halRes, expected); + + }); + + it('should display the second 3 users', () => { + const embeddedUsers: HalResource[] = []; + const currentPage = 2; + const pageSize = 3; + + const halRes = hal.collection(users, embeddedUsers, currentPage, pageSize); + + const expected = { + _links: { + 'create-form': { + href: '/user/new', + title: 'Create New User', + type: 'text/html' + }, + 'find-by-href': { + href: '/user/byhref/{href}', + templated: true, + title: 'Find a user through a identity/href (exact match)' + }, + item: [ + { + href: '/user/4', + title: 'User Four' + }, + { + href: '/user/5', + title: 'User Five' + }, + { + href: '/user/6', + title: 'User Six' + }, + ], + next: { + href: '/user?page=3' + }, + previous: { + href: '/user?page=1' + }, + self: { + href: '/user?page=2' + } + }, + currentPage: 2, + total: 10, + totalPages: 4 + }; + + assert.deepEqual(halRes, expected); + + }); + it('should display the last 2 users', () => { + const embeddedUsers: HalResource[] = []; + const currentPage = 5; + const pageSize = 2; + + const halRes = hal.collection(users, embeddedUsers, currentPage, pageSize); + + const expected = { + _links: { + 'create-form': { + href: '/user/new', + title: 'Create New User', + type: 'text/html' + }, + 'find-by-href': { + href: '/user/byhref/{href}', + templated: true, + title: 'Find a user through a identity/href (exact match)' + }, + item: [ + { + href: '/user/9', + title: 'User Nine' + }, + { + href: '/user/10', + title: 'User Ten' + } + ], + previous: { + href: '/user?page=4' + }, + self: { + href: '/user?page=5' + } + }, + currentPage: 5, + total: 10, + totalPages: 5 + }; + + assert.deepEqual(halRes, expected); + + }); +}); From a43eedf980b5a48f20ad32dff0ffc08a8a6273fc Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> Date: Thu, 20 Feb 2025 12:16:23 -0500 Subject: [PATCH 02/12] Update: Paginate using the db query Signed-off-by: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> --- src/principal/service.ts | 26 ++++++++++++++++++++++---- src/types.ts | 11 +++++++++++ src/user/controller/collection.ts | 7 ++++--- src/user/formats/hal.ts | 25 +++++++++++-------------- test/pagination/collection.ts | 11 ++++++----- 5 files changed, 54 insertions(+), 26 deletions(-) diff --git a/src/principal/service.ts b/src/principal/service.ts index 94be4f6d..9a44ade4 100644 --- a/src/principal/service.ts +++ b/src/principal/service.ts @@ -4,6 +4,7 @@ import { BasePrincipal, Group, NewPrincipal, + PaginatedResult, Principal, PrincipalIdentity, PrincipalStats, @@ -46,11 +47,11 @@ export class PrincipalService { } - async findAll(type: 'user', page: number): Promise; + async findAll(type: 'user', page: number): Promise>; async findAll(type: 'group'): Promise; async findAll(type: 'app'): Promise; async findAll(): Promise; - async findAll(type?: PrincipalType, page?: number): Promise { + async findAll(type?: PrincipalType, page?: number): Promise> { this.privileges.require('a12n:principals:list'); const filters: Record = {}; @@ -59,13 +60,19 @@ export class PrincipalService { } let result: PrincipalsRecord[] = []; + let total = 0; + let hasNextPage = false; + const pageSize = 100; if(type && page !== undefined){ + total = (await getPrincipalStats()).user; + page = page < 1 ? 1 : page; - const pageSize = 100; const offset = (page - 1) * pageSize; + hasNextPage = (offset + pageSize) < total; + result = await db('principals') .where(filters) .limit(pageSize) @@ -80,7 +87,18 @@ export class PrincipalService { for (const principal of result) { principals.push(recordToModel(principal)); } - return principals; + + if(page !== undefined){ // type? + return { + principals, + total, + page, + pageSize, + hasNextPage, + }; + } else { + return principals; + } } diff --git a/src/types.ts b/src/types.ts index 00ff2a0d..615d25bd 100644 --- a/src/types.ts +++ b/src/types.ts @@ -155,6 +155,17 @@ export type PrincipalStats = { group: number; }; +/** + * Paginated Result + */ +export type PaginatedResult = { + principals: T[]; + total: number; + page: number; + pageSize: number; + hasNextPage: boolean; +} + /** * The App Client refers to a single set of credentials for an app. * diff --git a/src/user/controller/collection.ts b/src/user/controller/collection.ts index 8a7a1cc1..75a14c0f 100644 --- a/src/user/controller/collection.ts +++ b/src/user/controller/collection.ts @@ -15,7 +15,9 @@ class UserCollectionController extends Controller { const pageInt = parseInt(ctx.request.query.page); const page = isNaN(pageInt) ? 1 : pageInt; - const users = await principalService.findAll('user', page); + const paginatedResult = await principalService.findAll('user', page); + const users = paginatedResult.principals; + const embed = ctx.request.prefer('transclude').toString().includes('item') || ctx.query.embed?.includes('item'); const embeddedUsers: HalResource[] = []; @@ -40,8 +42,7 @@ class UserCollectionController extends Controller { } } - const pageSize = 100; - ctx.response.body = hal.collection(users, embeddedUsers, page, pageSize); + ctx.response.body = hal.collection(embeddedUsers, paginatedResult); } diff --git a/src/user/formats/hal.ts b/src/user/formats/hal.ts index e0fc0d45..22e4fee9 100644 --- a/src/user/formats/hal.ts +++ b/src/user/formats/hal.ts @@ -1,25 +1,19 @@ import { PrivilegeMap } from '../../privilege/types.ts'; -import { Principal, Group, User, PrincipalIdentity } from '../../types.ts'; +import { Principal, Group, User, PrincipalIdentity, PaginatedResult } from '../../types.ts'; import { HalResource } from 'hal-types'; import { LazyPrivilegeBox } from '../../privilege/service.ts'; import { UserNewResult } from '../../api-types.ts'; -export function collection(users: User[], embeddedUsers: HalResource[], currentPage: number, pageSize: number): HalResource { +export function collection(embeddedUsers: HalResource[], paginatedResult: PaginatedResult): HalResource { - const totalUsers = users.length; - const totalPages = Math.ceil(totalUsers / pageSize); - const nextPage = currentPage < totalPages ? currentPage + 1 : null; - const prevPage = currentPage > 1 ? currentPage - 1 : null; + const { principals: users, page: currentPage, total, pageSize, hasNextPage } = paginatedResult; - const startIdx = (currentPage - 1) * pageSize; - const endIdx = Math.min(startIdx + pageSize, totalUsers); - - const paginatedUsers = users.slice(startIdx, endIdx); + const totalPages = Math.ceil(total / pageSize); const hal: HalResource = { _links: { 'self': { href: `/user?page=${currentPage}` }, - 'item': paginatedUsers.map( user => ({ + 'item': users.map( user => ({ href: user.href, title: user.nickname, })), @@ -30,16 +24,19 @@ export function collection(users: User[], embeddedUsers: HalResource[], currentP templated: true, }, }, - total: totalUsers, + total, currentPage, totalPages, }; - if(nextPage){ + if(hasNextPage){ + const nextPage = currentPage + 1; hal._links['next'] = { href: `/user?page=${nextPage}` }; } - if(prevPage){ + const hasPrevPage = currentPage > 1; + if(hasPrevPage){ + const prevPage = currentPage - 1; hal._links['previous'] = { href: `/user?page=${prevPage}` }; } diff --git a/test/pagination/collection.ts b/test/pagination/collection.ts index a92bb7b2..11f6b673 100644 --- a/test/pagination/collection.ts +++ b/test/pagination/collection.ts @@ -5,7 +5,8 @@ import * as hal from '../../src/user/formats/hal.ts'; import { User } from '../../src/types.ts'; import { HalResource } from 'hal-types'; -describe('hal.collection', () => { +// TODO: test the collection controller directly instead +describe.skip('hal.collection', () => { const users: User[] = [ { id: 1,href: '/user/1', @@ -123,7 +124,7 @@ describe('hal.collection', () => { const currentPage = 1; const pageSize = 100; - const halRes = hal.collection(users, embeddedUsers, currentPage, pageSize); + const halRes = hal.collection(embeddedUsers, { principals: users, total: users.length, page: currentPage, pageSize, hasNextPage: false }); const expected = { _links: { @@ -197,7 +198,7 @@ describe('hal.collection', () => { const currentPage = 1; const pageSize = 3; - const halRes = hal.collection(users, embeddedUsers, currentPage, pageSize); + const halRes = hal.collection(embeddedUsers, { principals: users, total: users.length, page: currentPage, pageSize, hasNextPage: false }); const expected = { _links: { @@ -246,7 +247,7 @@ describe('hal.collection', () => { const currentPage = 2; const pageSize = 3; - const halRes = hal.collection(users, embeddedUsers, currentPage, pageSize); + const halRes = hal.collection(embeddedUsers, { principals: users, total: users.length, page: currentPage, pageSize, hasNextPage: false }); const expected = { _links: { @@ -297,7 +298,7 @@ describe('hal.collection', () => { const currentPage = 5; const pageSize = 2; - const halRes = hal.collection(users, embeddedUsers, currentPage, pageSize); + const halRes = hal.collection(embeddedUsers, { principals: users, total: users.length, page: currentPage, pageSize, hasNextPage: false }); const expected = { _links: { From b114e11f71cafa4d5644d4cc5ae0b03fec0b7f48 Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> Date: Thu, 20 Feb 2025 12:24:18 -0500 Subject: [PATCH 03/12] Refactor: cast page to Number Signed-off-by: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> --- src/user/controller/collection.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/user/controller/collection.ts b/src/user/controller/collection.ts index 75a14c0f..daa78d34 100644 --- a/src/user/controller/collection.ts +++ b/src/user/controller/collection.ts @@ -12,8 +12,7 @@ class UserCollectionController extends Controller { const principalService = new services.principal.PrincipalService(ctx.privileges); - const pageInt = parseInt(ctx.request.query.page); - const page = isNaN(pageInt) ? 1 : pageInt; + const page = +ctx.request.query.page || 1; const paginatedResult = await principalService.findAll('user', page); const users = paginatedResult.principals; From ce8adcfbc5163a39845222464110a8d3f78d6e0a Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> Date: Thu, 20 Feb 2025 19:11:32 -0500 Subject: [PATCH 04/12] Test: findAll query Signed-off-by: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> --- .env.test | 2 + src/database.ts | 2 + src/principal/service.ts | 12 +++-- test/pagination/collection.ts | 2 +- test/pagination/services.ts | 85 +++++++++++++++++++++++++++++++++++ 5 files changed, 95 insertions(+), 8 deletions(-) create mode 100644 .env.test create mode 100644 test/pagination/services.ts diff --git a/.env.test b/.env.test new file mode 100644 index 00000000..454b35fa --- /dev/null +++ b/.env.test @@ -0,0 +1,2 @@ +DB_DRIVER=sqlite +DB_FILENAME=:memory: diff --git a/src/database.ts b/src/database.ts index c2e8980c..f23cd98f 100644 --- a/src/database.ts +++ b/src/database.ts @@ -3,6 +3,8 @@ import { Knex, default as knex } from 'knex'; import * as path from 'node:path'; import { fileURLToPath } from 'node:url'; import './env.ts'; +import dotenv from 'dotenv'; +dotenv.config({ path: './.env.test' }); let settings: Knex.Config | null = null; const db: Knex = knex(getSettings()); diff --git a/src/principal/service.ts b/src/principal/service.ts index 9a44ade4..00e8d131 100644 --- a/src/principal/service.ts +++ b/src/principal/service.ts @@ -59,15 +59,14 @@ export class PrincipalService { filters.type = userTypeToInt(type); } + const pageSize = 100; + const total = (await getPrincipalStats()).user; + let result: PrincipalsRecord[] = []; - let total = 0; let hasNextPage = false; - const pageSize = 100; if(type && page !== undefined){ - total = (await getPrincipalStats()).user; - page = page < 1 ? 1 : page; const offset = (page - 1) * pageSize; @@ -88,7 +87,7 @@ export class PrincipalService { principals.push(recordToModel(principal)); } - if(page !== undefined){ // type? + if(type && page !== undefined){ return { principals, total, @@ -436,8 +435,7 @@ export async function getPrincipalStats(): Promise { } - -function recordToModel(user: PrincipalsRecord): Principal { +export function recordToModel(user: PrincipalsRecord): Principal { return { id: user.id, diff --git a/test/pagination/collection.ts b/test/pagination/collection.ts index 11f6b673..68bc345a 100644 --- a/test/pagination/collection.ts +++ b/test/pagination/collection.ts @@ -5,7 +5,7 @@ import * as hal from '../../src/user/formats/hal.ts'; import { User } from '../../src/types.ts'; import { HalResource } from 'hal-types'; -// TODO: test the collection controller directly instead +// TODO: delete this & test the findAll db query directly instead describe.skip('hal.collection', () => { const users: User[] = [ { diff --git a/test/pagination/services.ts b/test/pagination/services.ts new file mode 100644 index 00000000..d7da38bd --- /dev/null +++ b/test/pagination/services.ts @@ -0,0 +1,85 @@ +import { strict as assert } from 'node:assert'; +import { after, before, describe, it } from 'node:test'; + +import * as dotenv from 'dotenv'; +dotenv.config({ path: './.env.test'}); + +import { PrincipalService, recordToModel } from '../../src/principal/service.ts'; +import db, { insertAndGetId } from '../../src/database.ts'; + +describe('findAll users pagination', () => { + + const userRecordsDb: unknown[] = []; + before(async () => { + + await db.schema.createTable('principals', (table) => { + table.increments('id').primary(); + table.string('external_id').notNullable(); + table.string('nickname').notNullable(); + table.integer('type').notNullable(); + table.boolean('active').notNullable(); + table.timestamp('created_at').defaultTo(db.fn.now()); + table.timestamp('modified_at').defaultTo(db.fn.now()); + table.boolean('system').notNullable(); + }); + + // 3 pages worth of users + for(let i = 1; i < 251; i++){ + const data = { + external_id: i.toString(), + nickname: `User ${i}`, + type: 1, + active: 1, + created_at: Date.now(), + modified_at: Date.now(), + system: 0 + }; + + const id = await insertAndGetId('principals', data); + userRecordsDb.push({...data, id}); + } + }); + + after(async () => { + await db.destroy(); + }); + + it('should display first page', async () => { + const principalService = new PrincipalService('insecure'); + + const currentPage = 1; + const { principals, pageSize, page, hasNextPage, total } = await principalService.findAll('user', currentPage); + const expectedUsers = userRecordsDb.slice(0, pageSize).map((data) => recordToModel(data)); + + assert.equal(page, currentPage); + assert.equal(hasNextPage, true); + assert.equal(total, userRecordsDb.length); + assert.deepEqual(principals, expectedUsers); + }); + + it('should display second page', async () => { + const principalService = new PrincipalService('insecure'); + + const currentPage = 2; + const { principals, pageSize, page, hasNextPage, total } = await principalService.findAll('user', currentPage); + const expectedUsers = userRecordsDb.slice(pageSize, 200).map((data) => recordToModel(data)); + + assert.equal(page, currentPage); + assert.equal(hasNextPage, true); + assert.equal(total, userRecordsDb.length); + assert.deepEqual(principals, expectedUsers); + }); + + it('should display last (third) page', async () => { + const principalService = new PrincipalService('insecure'); + + const currentPage = 3; + const { principals, page, hasNextPage, total } = await principalService.findAll('user', currentPage); + const expectedUsers = userRecordsDb.slice(200, userRecordsDb.length).map((data) => recordToModel(data)); + + assert.equal(page, currentPage); + assert.equal(hasNextPage, false); + assert.equal(total, userRecordsDb.length); + assert.deepEqual(principals, expectedUsers); + }); +}); From b95fa07db3e39b44efc8a561941d3db33efc3e2a Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> Date: Thu, 20 Feb 2025 19:36:58 -0500 Subject: [PATCH 05/12] Fix: Add conditional use of `.env.test` file Signed-off-by: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> --- Makefile | 2 +- src/database.ts | 2 -- src/env.ts | 8 +++++++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index d48bf0db..b9a77d56 100644 --- a/Makefile +++ b/Makefile @@ -17,7 +17,7 @@ docker-run: docker run -it --rm --name $(DOCKER_IMAGE_NAME)-01 $(DOCKER_IMAGE_NAME) test: - npx tsx --test ${TEST_FILES} + NODE_ENV=test npx tsx --test ${TEST_FILES} lint: npx tsc --noemit diff --git a/src/database.ts b/src/database.ts index f23cd98f..c2e8980c 100644 --- a/src/database.ts +++ b/src/database.ts @@ -3,8 +3,6 @@ import { Knex, default as knex } from 'knex'; import * as path from 'node:path'; import { fileURLToPath } from 'node:url'; import './env.ts'; -import dotenv from 'dotenv'; -dotenv.config({ path: './.env.test' }); let settings: Knex.Config | null = null; const db: Knex = knex(getSettings()); diff --git a/src/env.ts b/src/env.ts index 72b4976e..5f7f3201 100644 --- a/src/env.ts +++ b/src/env.ts @@ -12,7 +12,13 @@ if (process.env.PUBLIC_URI === undefined) { // that we may be missing a .env file. // // This is the only required environment variable. - dotenv.config({path: dirname(fileURLToPath(import.meta.url)) + '/../.env'}); + if(process.env.NODE_ENV === 'test'){ + dotenv.config({ path: './.env.test' }); + } + else{ + dotenv.config({path: dirname(fileURLToPath(import.meta.url)) + '/../.env'}); + } + } else { console.warn('/env.js was loaded twice?'); } From fcd1cd4b0f7d3fd52962e3a167f5f9f092ea7750 Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> Date: Thu, 20 Feb 2025 19:45:05 -0500 Subject: [PATCH 06/12] Delete: hal.collection test Signed-off-by: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> --- test/pagination/collection.ts | 340 ---------------------------------- 1 file changed, 340 deletions(-) delete mode 100644 test/pagination/collection.ts diff --git a/test/pagination/collection.ts b/test/pagination/collection.ts deleted file mode 100644 index 68bc345a..00000000 --- a/test/pagination/collection.ts +++ /dev/null @@ -1,340 +0,0 @@ -import { strict as assert } from 'node:assert'; -import { describe, it } from 'node:test'; - -import * as hal from '../../src/user/formats/hal.ts'; -import { User } from '../../src/types.ts'; -import { HalResource } from 'hal-types'; - -// TODO: delete this & test the findAll db query directly instead -describe.skip('hal.collection', () => { - const users: User[] = [ - { - id: 1,href: '/user/1', - externalId: 'user-1', - type: 'user', - nickname: 'User One', - createdAt: new Date(Date.now()), - modifiedAt: new Date(Date.now()), - active: true, - system: false, - }, - { - id: 2, - href: '/user/2', - externalId: 'user-2', - type: 'user', - nickname: 'User Two', - createdAt: new Date(Date.now()), - modifiedAt: new Date(Date.now()), - active: true, - system: false, - }, - { - id: 3, - href: '/user/3', - externalId: 'user-3', - type: 'user', - nickname: 'User Three', - createdAt: new Date(Date.now()), - modifiedAt: new Date(Date.now()), - active: true, - system: false, - }, - { - id: 4, - href: '/user/4', - externalId: 'user-4', - type: 'user', - nickname: 'User Four', - createdAt: new Date(Date.now()), - modifiedAt: new Date(Date.now()), - active: true, - system: false, - }, - { - id: 5, - href: '/user/5', - externalId: 'user-5', - type: 'user', - nickname: 'User Five', - createdAt: new Date(Date.now()), - modifiedAt: new Date(Date.now()), - active: true, - system: false, - }, - { - id: 6, - href: '/user/6', - externalId: 'user-6', - type: 'user', - nickname: 'User Six', - createdAt: new Date(Date.now()), - modifiedAt: new Date(Date.now()), - active: true, - system: false, - }, - { - id: 7, - href: '/user/7', - externalId: 'user-7', - type: 'user', - nickname: 'User Seven', - createdAt: new Date(Date.now()), - modifiedAt: new Date(Date.now()), - active: true, - system: false, - }, - { - id: 8, - href: '/user/8', - externalId: 'user-8', - type: 'user', - nickname: 'User Eight', - createdAt: new Date(Date.now()), - modifiedAt: new Date(Date.now()), - active: true, - system: false, - }, - { - id: 9, - href: '/user/9', - externalId: 'user-9', - type: 'user', - nickname: 'User Nine', - createdAt: new Date(Date.now()), - modifiedAt: new Date(Date.now()), - active: true, - system: false, - }, - { - id: 10, - href: '/user/10', - externalId: 'user-10', - type: 'user', - nickname: 'User Ten', - createdAt: new Date(Date.now()), - modifiedAt: new Date(Date.now()), - active: true, - system: false, - }, - ]; - - it('should display all users', () => { - const embeddedUsers: HalResource[] = []; - const currentPage = 1; - const pageSize = 100; - - const halRes = hal.collection(embeddedUsers, { principals: users, total: users.length, page: currentPage, pageSize, hasNextPage: false }); - - const expected = { - _links: { - 'create-form': { - href: '/user/new', - title: 'Create New User', - type: 'text/html' - }, - 'find-by-href': { - href: '/user/byhref/{href}', - templated: true, - title: 'Find a user through a identity/href (exact match)' - }, - item: [ - { - href: '/user/1', - title: 'User One' - }, - { - href: '/user/2', - title: 'User Two' - }, - { - href: '/user/3', - title: 'User Three' - }, - { - href: '/user/4', - title: 'User Four' - }, - { - href: '/user/5', - title: 'User Five' - }, - { - href: '/user/6', - title: 'User Six' - }, - { - href: '/user/7', - title: 'User Seven' - }, - { - href: '/user/8', - title: 'User Eight' - }, - { - href: '/user/9', - title: 'User Nine' - }, - { - href: '/user/10', - title: 'User Ten' - } - ], - self: { - href: '/user?page=1' - } - }, - currentPage: 1, - total: 10, - totalPages: 1 - }; - - assert.deepEqual(halRes, expected); - - }); - - it('should display the first 3 users', () => { - const embeddedUsers: HalResource[] = []; - const currentPage = 1; - const pageSize = 3; - - const halRes = hal.collection(embeddedUsers, { principals: users, total: users.length, page: currentPage, pageSize, hasNextPage: false }); - - const expected = { - _links: { - 'create-form': { - href: '/user/new', - title: 'Create New User', - type: 'text/html' - }, - 'find-by-href': { - href: '/user/byhref/{href}', - templated: true, - title: 'Find a user through a identity/href (exact match)' - }, - item: [ - { - href: '/user/1', - title: 'User One' - }, - { - href: '/user/2', - title: 'User Two' - }, - { - href: '/user/3', - title: 'User Three' - }, - ], - next: { - href: '/user?page=2' - }, - self: { - href: '/user?page=1' - } - }, - currentPage: 1, - total: 10, - totalPages: 4 - }; - - assert.deepEqual(halRes, expected); - - }); - - it('should display the second 3 users', () => { - const embeddedUsers: HalResource[] = []; - const currentPage = 2; - const pageSize = 3; - - const halRes = hal.collection(embeddedUsers, { principals: users, total: users.length, page: currentPage, pageSize, hasNextPage: false }); - - const expected = { - _links: { - 'create-form': { - href: '/user/new', - title: 'Create New User', - type: 'text/html' - }, - 'find-by-href': { - href: '/user/byhref/{href}', - templated: true, - title: 'Find a user through a identity/href (exact match)' - }, - item: [ - { - href: '/user/4', - title: 'User Four' - }, - { - href: '/user/5', - title: 'User Five' - }, - { - href: '/user/6', - title: 'User Six' - }, - ], - next: { - href: '/user?page=3' - }, - previous: { - href: '/user?page=1' - }, - self: { - href: '/user?page=2' - } - }, - currentPage: 2, - total: 10, - totalPages: 4 - }; - - assert.deepEqual(halRes, expected); - - }); - it('should display the last 2 users', () => { - const embeddedUsers: HalResource[] = []; - const currentPage = 5; - const pageSize = 2; - - const halRes = hal.collection(embeddedUsers, { principals: users, total: users.length, page: currentPage, pageSize, hasNextPage: false }); - - const expected = { - _links: { - 'create-form': { - href: '/user/new', - title: 'Create New User', - type: 'text/html' - }, - 'find-by-href': { - href: '/user/byhref/{href}', - templated: true, - title: 'Find a user through a identity/href (exact match)' - }, - item: [ - { - href: '/user/9', - title: 'User Nine' - }, - { - href: '/user/10', - title: 'User Ten' - } - ], - previous: { - href: '/user?page=4' - }, - self: { - href: '/user?page=5' - } - }, - currentPage: 5, - total: 10, - totalPages: 5 - }; - - assert.deepEqual(halRes, expected); - - }); -}); From df4707094adbd1c1f130d33cc515d45644ea8711 Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> Date: Thu, 20 Feb 2025 20:00:27 -0500 Subject: [PATCH 07/12] Feat: first and last page links Signed-off-by: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> --- src/user/formats/hal.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/user/formats/hal.ts b/src/user/formats/hal.ts index 22e4fee9..6e80f90f 100644 --- a/src/user/formats/hal.ts +++ b/src/user/formats/hal.ts @@ -40,6 +40,11 @@ export function collection(embeddedUsers: HalResource[], paginatedResult: Pagina hal._links['previous'] = { href: `/user?page=${prevPage}` }; } + if(hasNextPage || hasPrevPage){ + hal._links['first'] = { href: '/user?page=1' }; + hal._links['last'] = { href: `/user?page=${totalPages}` }; + } + if (embeddedUsers.length) { hal._embedded = { item: embeddedUsers From 75a696526934fda67beb7ef5a57b2a4261221a48 Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> Date: Fri, 21 Feb 2025 08:55:56 -0500 Subject: [PATCH 08/12] Refactor: Pass db related env var to `make test` target Signed-off-by: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> --- .env.test | 2 -- Makefile | 2 +- src/env.ts | 8 +------- test/pagination/services.ts | 3 --- 4 files changed, 2 insertions(+), 13 deletions(-) delete mode 100644 .env.test diff --git a/.env.test b/.env.test deleted file mode 100644 index 454b35fa..00000000 --- a/.env.test +++ /dev/null @@ -1,2 +0,0 @@ -DB_DRIVER=sqlite -DB_FILENAME=:memory: diff --git a/Makefile b/Makefile index b9a77d56..cbf3eded 100644 --- a/Makefile +++ b/Makefile @@ -17,7 +17,7 @@ docker-run: docker run -it --rm --name $(DOCKER_IMAGE_NAME)-01 $(DOCKER_IMAGE_NAME) test: - NODE_ENV=test npx tsx --test ${TEST_FILES} + DB_DRIVER=sqlite DB_FILENAME=":memory:" npx tsx --test ${TEST_FILES} lint: npx tsc --noemit diff --git a/src/env.ts b/src/env.ts index 5f7f3201..72b4976e 100644 --- a/src/env.ts +++ b/src/env.ts @@ -12,13 +12,7 @@ if (process.env.PUBLIC_URI === undefined) { // that we may be missing a .env file. // // This is the only required environment variable. - if(process.env.NODE_ENV === 'test'){ - dotenv.config({ path: './.env.test' }); - } - else{ - dotenv.config({path: dirname(fileURLToPath(import.meta.url)) + '/../.env'}); - } - + dotenv.config({path: dirname(fileURLToPath(import.meta.url)) + '/../.env'}); } else { console.warn('/env.js was loaded twice?'); } diff --git a/test/pagination/services.ts b/test/pagination/services.ts index d7da38bd..c3435ea0 100644 --- a/test/pagination/services.ts +++ b/test/pagination/services.ts @@ -1,9 +1,6 @@ import { strict as assert } from 'node:assert'; import { after, before, describe, it } from 'node:test'; -import * as dotenv from 'dotenv'; -dotenv.config({ path: './.env.test'}); - import { PrincipalService, recordToModel } from '../../src/principal/service.ts'; import db, { insertAndGetId } from '../../src/database.ts'; From be810be7dcca0fced2efa295002d091f197f4daf Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> Date: Fri, 21 Feb 2025 09:49:20 -0500 Subject: [PATCH 09/12] Feat: Add generic search function Signed-off-by: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> --- src/principal/service.ts | 64 ++++++++++----------- src/types.ts | 2 +- src/user/controller/collection.ts | 5 +- src/user/formats/hal.ts | 4 +- test/pagination/{services.ts => service.ts} | 23 +++++--- tsconfig.json | 5 +- 6 files changed, 55 insertions(+), 48 deletions(-) rename test/pagination/{services.ts => service.ts} (76%) diff --git a/src/principal/service.ts b/src/principal/service.ts index 00e8d131..546c0b0c 100644 --- a/src/principal/service.ts +++ b/src/principal/service.ts @@ -47,11 +47,11 @@ export class PrincipalService { } - async findAll(type: 'user', page: number): Promise>; + async findAll(type: 'user'): Promise; async findAll(type: 'group'): Promise; async findAll(type: 'app'): Promise; async findAll(): Promise; - async findAll(type?: PrincipalType, page?: number): Promise> { + async findAll(type?: PrincipalType): Promise { this.privileges.require('a12n:principals:list'); const filters: Record = {}; @@ -59,46 +59,46 @@ export class PrincipalService { filters.type = userTypeToInt(type); } - const pageSize = 100; - const total = (await getPrincipalStats()).user; + const result = await db('principals') + .where(filters); - let result: PrincipalsRecord[] = []; - let hasNextPage = false; + const principals: Principal[] = []; + for (const principal of result) { + principals.push(recordToModel(principal)); + } + return principals; - if(type && page !== undefined){ + } - page = page < 1 ? 1 : page; - const offset = (page - 1) * pageSize; + async search(type: PrincipalType, page: number = 1): Promise> { - hasNextPage = (offset + pageSize) < total; + this.privileges.require('a12n:principals:list'); + const filters: Record = {}; + filters.type = userTypeToInt(type); - result = await db('principals') - .where(filters) - .limit(pageSize) - .offset(offset); + const pageSize = 100; + const offset = (page - 1) * pageSize; - } else { - result = await db('principals') - .where(filters); - } + const total = (await getPrincipalStats()).user; + const hasNextPage = (offset + pageSize) < total; - const principals: Principal[] = []; - for (const principal of result) { - principals.push(recordToModel(principal)); - } + const result = await db('principals') + .where(filters) + .limit(pageSize) + .offset(offset); - if(type && page !== undefined){ - return { - principals, - total, - page, - pageSize, - hasNextPage, - }; - } else { - return principals; + const items: T[] = []; + for (const principal of result) { + items.push(recordToModel(principal) as T); } + return { + items, + total, + page, + pageSize, + hasNextPage, + }; } async findByIdentity(identity: PrincipalIdentity|string): Promise { diff --git a/src/types.ts b/src/types.ts index 615d25bd..7674ad3d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -159,7 +159,7 @@ export type PrincipalStats = { * Paginated Result */ export type PaginatedResult = { - principals: T[]; + items: T[]; total: number; page: number; pageSize: number; diff --git a/src/user/controller/collection.ts b/src/user/controller/collection.ts index daa78d34..1fb307a2 100644 --- a/src/user/controller/collection.ts +++ b/src/user/controller/collection.ts @@ -5,6 +5,7 @@ import * as hal from '../formats/hal.ts'; import * as services from '../../services.ts'; import { PrincipalNew } from '../../api-types.ts'; import { HalResource } from 'hal-types'; +import { User } from '../../../src/types.ts'; class UserCollectionController extends Controller { @@ -14,8 +15,8 @@ class UserCollectionController extends Controller { const page = +ctx.request.query.page || 1; - const paginatedResult = await principalService.findAll('user', page); - const users = paginatedResult.principals; + const paginatedResult = await principalService.search('user', page); + const users = paginatedResult.items; const embed = ctx.request.prefer('transclude').toString().includes('item') || ctx.query.embed?.includes('item'); diff --git a/src/user/formats/hal.ts b/src/user/formats/hal.ts index 6e80f90f..c5b84f86 100644 --- a/src/user/formats/hal.ts +++ b/src/user/formats/hal.ts @@ -4,9 +4,9 @@ import { HalResource } from 'hal-types'; import { LazyPrivilegeBox } from '../../privilege/service.ts'; import { UserNewResult } from '../../api-types.ts'; -export function collection(embeddedUsers: HalResource[], paginatedResult: PaginatedResult): HalResource { +export function collection(embeddedUsers: HalResource[], paginatedResult: PaginatedResult): HalResource { - const { principals: users, page: currentPage, total, pageSize, hasNextPage } = paginatedResult; + const { items: users, page: currentPage, total, pageSize, hasNextPage } = paginatedResult; const totalPages = Math.ceil(total / pageSize); diff --git a/test/pagination/services.ts b/test/pagination/service.ts similarity index 76% rename from test/pagination/services.ts rename to test/pagination/service.ts index c3435ea0..578aed31 100644 --- a/test/pagination/services.ts +++ b/test/pagination/service.ts @@ -1,16 +1,20 @@ import { strict as assert } from 'node:assert'; import { after, before, describe, it } from 'node:test'; +import { PrincipalsRecord } from 'knex/types/tables.ts'; + import { PrincipalService, recordToModel } from '../../src/principal/service.ts'; import db, { insertAndGetId } from '../../src/database.ts'; -describe('findAll users pagination', () => { +describe('users pagination', () => { + + const userRecordsDb: PrincipalsRecord[] = []; - const userRecordsDb: unknown[] = []; before(async () => { await db.schema.createTable('principals', (table) => { table.increments('id').primary(); + table.string('identity').nullable(); table.string('external_id').notNullable(); table.string('nickname').notNullable(); table.integer('type').notNullable(); @@ -22,7 +26,8 @@ describe('findAll users pagination', () => { // 3 pages worth of users for(let i = 1; i < 251; i++){ - const data = { + const data: Omit = { + identity: null, external_id: i.toString(), nickname: `User ${i}`, type: 1, @@ -45,38 +50,38 @@ describe('findAll users pagination', () => { const principalService = new PrincipalService('insecure'); const currentPage = 1; - const { principals, pageSize, page, hasNextPage, total } = await principalService.findAll('user', currentPage); + const { items, pageSize, page, hasNextPage, total } = await principalService.search('user', currentPage); const expectedUsers = userRecordsDb.slice(0, pageSize).map((data) => recordToModel(data)); assert.equal(page, currentPage); assert.equal(hasNextPage, true); assert.equal(total, userRecordsDb.length); - assert.deepEqual(principals, expectedUsers); + assert.deepEqual(items, expectedUsers); }); it('should display second page', async () => { const principalService = new PrincipalService('insecure'); const currentPage = 2; - const { principals, pageSize, page, hasNextPage, total } = await principalService.findAll('user', currentPage); + const { items, pageSize, page, hasNextPage, total } = await principalService.search('user', currentPage); const expectedUsers = userRecordsDb.slice(pageSize, 200).map((data) => recordToModel(data)); assert.equal(page, currentPage); assert.equal(hasNextPage, true); assert.equal(total, userRecordsDb.length); - assert.deepEqual(principals, expectedUsers); + assert.deepEqual(items, expectedUsers); }); it('should display last (third) page', async () => { const principalService = new PrincipalService('insecure'); const currentPage = 3; - const { principals, page, hasNextPage, total } = await principalService.findAll('user', currentPage); + const { items, page, hasNextPage, total } = await principalService.search('user', currentPage); const expectedUsers = userRecordsDb.slice(200, userRecordsDb.length).map((data) => recordToModel(data)); assert.equal(page, currentPage); assert.equal(hasNextPage, false); assert.equal(total, userRecordsDb.length); - assert.deepEqual(principals, expectedUsers); + assert.deepEqual(items, expectedUsers); }); }); diff --git a/tsconfig.json b/tsconfig.json index 83794659..8a4e69c2 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -23,9 +23,10 @@ "DOM", "ES2022" ], - "declaration": true + "declaration": true, }, "include": [ - "src/**/*" + "src/**/*", + "test/**/*" ] } From 5b8f18f2ac3cdf3fba163f1e00e508d229191e42 Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> Date: Fri, 21 Feb 2025 10:12:27 -0500 Subject: [PATCH 10/12] Refactor: Use `principalService.save()` for integration test Signed-off-by: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> --- src/principal/service.ts | 2 +- test/pagination/service.ts | 48 ++++++++++++++++---------------------- tsconfig.json | 3 +-- 3 files changed, 22 insertions(+), 31 deletions(-) diff --git a/src/principal/service.ts b/src/principal/service.ts index 546c0b0c..60fd6a9b 100644 --- a/src/principal/service.ts +++ b/src/principal/service.ts @@ -435,7 +435,7 @@ export async function getPrincipalStats(): Promise { } -export function recordToModel(user: PrincipalsRecord): Principal { +function recordToModel(user: PrincipalsRecord): Principal { return { id: user.id, diff --git a/test/pagination/service.ts b/test/pagination/service.ts index 578aed31..beab1aad 100644 --- a/test/pagination/service.ts +++ b/test/pagination/service.ts @@ -1,14 +1,15 @@ import { strict as assert } from 'node:assert'; import { after, before, describe, it } from 'node:test'; -import { PrincipalsRecord } from 'knex/types/tables.ts'; - -import { PrincipalService, recordToModel } from '../../src/principal/service.ts'; -import db, { insertAndGetId } from '../../src/database.ts'; +import { PrincipalService } from '../../src/principal/service.ts'; +import { PrincipalNew } from '../../src/api-types.ts'; +import { User } from '../../src/types.ts'; +import db from '../../src/database.ts'; describe('users pagination', () => { - const userRecordsDb: PrincipalsRecord[] = []; + let users: User[] = []; + const principalService = new PrincipalService('insecure'); before(async () => { @@ -26,20 +27,17 @@ describe('users pagination', () => { // 3 pages worth of users for(let i = 1; i < 251; i++){ - const data: Omit = { - identity: null, - external_id: i.toString(), + const data = { + type: 'user' as PrincipalNew['type'], nickname: `User ${i}`, - type: 1, - active: 1, - created_at: Date.now(), - modified_at: Date.now(), - system: 0 + createdAt: new Date(Date.now()), + modifiedAt: new Date(Date.now()), + active: true, }; - - const id = await insertAndGetId('principals', data); - userRecordsDb.push({...data, id}); + await principalService.save(data); } + + users = await principalService.findAll('user'); }); after(async () => { @@ -47,41 +45,35 @@ describe('users pagination', () => { }); it('should display first page', async () => { - const principalService = new PrincipalService('insecure'); - const currentPage = 1; const { items, pageSize, page, hasNextPage, total } = await principalService.search('user', currentPage); - const expectedUsers = userRecordsDb.slice(0, pageSize).map((data) => recordToModel(data)); + const expectedUsers = users.slice(0, pageSize); assert.equal(page, currentPage); assert.equal(hasNextPage, true); - assert.equal(total, userRecordsDb.length); + assert.equal(total, users.length); assert.deepEqual(items, expectedUsers); }); it('should display second page', async () => { - const principalService = new PrincipalService('insecure'); - const currentPage = 2; const { items, pageSize, page, hasNextPage, total } = await principalService.search('user', currentPage); - const expectedUsers = userRecordsDb.slice(pageSize, 200).map((data) => recordToModel(data)); + const expectedUsers = users.slice(pageSize, 200); assert.equal(page, currentPage); assert.equal(hasNextPage, true); - assert.equal(total, userRecordsDb.length); + assert.equal(total, users.length); assert.deepEqual(items, expectedUsers); }); it('should display last (third) page', async () => { - const principalService = new PrincipalService('insecure'); - const currentPage = 3; const { items, page, hasNextPage, total } = await principalService.search('user', currentPage); - const expectedUsers = userRecordsDb.slice(200, userRecordsDb.length).map((data) => recordToModel(data)); + const expectedUsers = users.slice(200, users.length); assert.equal(page, currentPage); assert.equal(hasNextPage, false); - assert.equal(total, userRecordsDb.length); + assert.equal(total, users.length); assert.deepEqual(items, expectedUsers); }); }); diff --git a/tsconfig.json b/tsconfig.json index 8a4e69c2..0423bb41 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -26,7 +26,6 @@ "declaration": true, }, "include": [ - "src/**/*", - "test/**/*" + "src/**/*" ] } From c4ef6dbb01b3458d6fa1078fab5e78e00c03c94e Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> Date: Fri, 21 Feb 2025 10:39:42 -0500 Subject: [PATCH 11/12] Test: Run migrations on test database Signed-off-by: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> --- test/pagination/service.ts | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/test/pagination/service.ts b/test/pagination/service.ts index beab1aad..dad21687 100644 --- a/test/pagination/service.ts +++ b/test/pagination/service.ts @@ -4,7 +4,7 @@ import { after, before, describe, it } from 'node:test'; import { PrincipalService } from '../../src/principal/service.ts'; import { PrincipalNew } from '../../src/api-types.ts'; import { User } from '../../src/types.ts'; -import db from '../../src/database.ts'; +import db, { init } from '../../src/database.ts'; describe('users pagination', () => { @@ -12,18 +12,22 @@ describe('users pagination', () => { const principalService = new PrincipalService('insecure'); before(async () => { + await init(); - await db.schema.createTable('principals', (table) => { - table.increments('id').primary(); - table.string('identity').nullable(); - table.string('external_id').notNullable(); - table.string('nickname').notNullable(); - table.integer('type').notNullable(); - table.boolean('active').notNullable(); - table.timestamp('created_at').defaultTo(db.fn.now()); - table.timestamp('modified_at').defaultTo(db.fn.now()); - table.boolean('system').notNullable(); - }); + const hasTable = await db.schema.hasTable('principals'); + if (!hasTable) { + await db.schema.createTable('principals', (table) => { + table.increments('id').primary(); + table.string('identity').nullable(); + table.string('external_id').notNullable(); + table.string('nickname').notNullable(); + table.integer('type').notNullable(); + table.bigInteger('created_at').defaultTo(db.fn.now()); + table.bigInteger('modified_at').defaultTo(db.fn.now()); + table.boolean('active').notNullable().defaultTo(false); + table.tinyint('system').notNullable().defaultTo(0); + }); + } // 3 pages worth of users for(let i = 1; i < 251; i++){ From 768cf1b7bd1f41993bae67f9134b90f202b05df5 Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> Date: Fri, 21 Feb 2025 11:40:17 -0500 Subject: [PATCH 12/12] Feat: Add `getUserPageHref` util and tests Signed-off-by: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> --- src/user/controller/collection.ts | 2 +- src/user/formats/hal.ts | 23 ++++-- test/pagination/service.ts | 128 +++++++++++++++++++++++------- 3 files changed, 116 insertions(+), 37 deletions(-) diff --git a/src/user/controller/collection.ts b/src/user/controller/collection.ts index 1fb307a2..2cfba1e6 100644 --- a/src/user/controller/collection.ts +++ b/src/user/controller/collection.ts @@ -42,7 +42,7 @@ class UserCollectionController extends Controller { } } - ctx.response.body = hal.collection(embeddedUsers, paginatedResult); + ctx.response.body = hal.collection(paginatedResult, embeddedUsers); } diff --git a/src/user/formats/hal.ts b/src/user/formats/hal.ts index c5b84f86..915e8d2f 100644 --- a/src/user/formats/hal.ts +++ b/src/user/formats/hal.ts @@ -1,10 +1,10 @@ import { PrivilegeMap } from '../../privilege/types.ts'; import { Principal, Group, User, PrincipalIdentity, PaginatedResult } from '../../types.ts'; -import { HalResource } from 'hal-types'; +import { HalLink, HalResource } from 'hal-types'; import { LazyPrivilegeBox } from '../../privilege/service.ts'; import { UserNewResult } from '../../api-types.ts'; -export function collection(embeddedUsers: HalResource[], paginatedResult: PaginatedResult): HalResource { +export function collection(paginatedResult: PaginatedResult, embeddedUsers: HalResource[]): HalResource { const { items: users, page: currentPage, total, pageSize, hasNextPage } = paginatedResult; @@ -12,7 +12,7 @@ export function collection(embeddedUsers: HalResource[], paginatedResult: Pagina const hal: HalResource = { _links: { - 'self': { href: `/user?page=${currentPage}` }, + 'self': getUserPageHref(currentPage), 'item': users.map( user => ({ href: user.href, title: user.nickname, @@ -31,18 +31,17 @@ export function collection(embeddedUsers: HalResource[], paginatedResult: Pagina if(hasNextPage){ const nextPage = currentPage + 1; - hal._links['next'] = { href: `/user?page=${nextPage}` }; + hal._links['next'] = getUserPageHref(nextPage); } const hasPrevPage = currentPage > 1; if(hasPrevPage){ const prevPage = currentPage - 1; - hal._links['previous'] = { href: `/user?page=${prevPage}` }; + hal._links['previous'] = getUserPageHref(prevPage); } - if(hasNextPage || hasPrevPage){ - hal._links['first'] = { href: '/user?page=1' }; - hal._links['last'] = { href: `/user?page=${totalPages}` }; + hal._links['first'] = getUserPageHref(1); + hal._links['last'] = getUserPageHref(totalPages); } if (embeddedUsers.length) { @@ -55,6 +54,14 @@ export function collection(embeddedUsers: HalResource[], paginatedResult: Pagina } +function getUserPageHref(page: number): HalLink { + if(page === 1){ + return { href: '/user' }; + } + + return { href: `/user?page=${page}` }; +} + /** * Generate a HAL response for a specific user * diff --git a/test/pagination/service.ts b/test/pagination/service.ts index dad21687..458e6aaf 100644 --- a/test/pagination/service.ts +++ b/test/pagination/service.ts @@ -3,13 +3,14 @@ import { after, before, describe, it } from 'node:test'; import { PrincipalService } from '../../src/principal/service.ts'; import { PrincipalNew } from '../../src/api-types.ts'; -import { User } from '../../src/types.ts'; +import * as hal from '../../src/user/formats/hal.ts'; import db, { init } from '../../src/database.ts'; +import { User } from '../../src/types.ts'; +import { HalResource } from 'hal-types'; describe('users pagination', () => { - - let users: User[] = []; const principalService = new PrincipalService('insecure'); + let users: User[] = []; before(async () => { await init(); @@ -48,36 +49,107 @@ describe('users pagination', () => { await db.destroy(); }); - it('should display first page', async () => { - const currentPage = 1; - const { items, pageSize, page, hasNextPage, total } = await principalService.search('user', currentPage); - const expectedUsers = users.slice(0, pageSize); + describe('search service', () => { + it('should display first page', async () => { + const currentPage = 1; + const { items, pageSize, page, hasNextPage, total } = await principalService.search('user', currentPage); + const expectedUsers = users.slice(0, pageSize); - assert.equal(page, currentPage); - assert.equal(hasNextPage, true); - assert.equal(total, users.length); - assert.deepEqual(items, expectedUsers); - }); + assert.equal(page, currentPage); + assert.equal(hasNextPage, true); + assert.equal(total, users.length); + assert.deepEqual(items, expectedUsers); + }); + + it('should display second page', async () => { + const currentPage = 2; + const { items, pageSize, page, hasNextPage, total } = await principalService.search('user', currentPage); + const expectedUsers = users.slice(pageSize, 200); + + assert.equal(page, currentPage); + assert.equal(hasNextPage, true); + assert.equal(total, users.length); + assert.deepEqual(items, expectedUsers); + }); - it('should display second page', async () => { - const currentPage = 2; - const { items, pageSize, page, hasNextPage, total } = await principalService.search('user', currentPage); - const expectedUsers = users.slice(pageSize, 200); + it('should display last (third) page', async () => { + const currentPage = 3; + const { items, page, hasNextPage, total } = await principalService.search('user', currentPage); + const expectedUsers = users.slice(200, users.length); - assert.equal(page, currentPage); - assert.equal(hasNextPage, true); - assert.equal(total, users.length); - assert.deepEqual(items, expectedUsers); + assert.equal(page, currentPage); + assert.equal(hasNextPage, false); + assert.equal(total, users.length); + assert.deepEqual(items, expectedUsers); + }); }); - it('should display last (third) page', async () => { - const currentPage = 3; - const { items, page, hasNextPage, total } = await principalService.search('user', currentPage); - const expectedUsers = users.slice(200, users.length); + describe('hal.collection links', () => { + const embeddedUsers: HalResource[] = []; + + it('should not display `previous` link on first page', async () => { + const currentPage = 1; + + const paginatedResult = await principalService.search('user', currentPage); + const halRes = hal.collection(paginatedResult, embeddedUsers); + + assert.equal(halRes._links.previous, undefined); + assert.deepEqual(halRes._links.self, { + href: '/user', + }); + assert.deepEqual(halRes._links.first, { + href: '/user', + }); + assert.deepEqual(halRes._links.last, { + href: '/user?page=3', + }); + assert.deepEqual(halRes._links.next, { + href: '/user?page=2', + }); + }); - assert.equal(page, currentPage); - assert.equal(hasNextPage, false); - assert.equal(total, users.length); - assert.deepEqual(items, expectedUsers); + it('should not display `next` link on last page', async () => { + const currentPage = 3; + + const paginatedResult = await principalService.search('user', currentPage); + const halRes = hal.collection(paginatedResult, embeddedUsers); + + assert.equal(halRes._links.next, undefined); + assert.deepEqual(halRes._links.self, { + href: '/user?page=3', + }); + assert.deepEqual(halRes._links.first, { + href: '/user', + }); + assert.deepEqual(halRes._links.last, { + href: '/user?page=3', + }); + assert.deepEqual(halRes._links.previous, { + href: '/user?page=2', + }); + }); + + it('should display both `previous` & `next` links on middle page', async () => { + const currentPage = 2; + + const paginatedResult = await principalService.search('user', currentPage); + const halRes = hal.collection(paginatedResult, embeddedUsers); + + assert.deepEqual(halRes._links.self, { + href: '/user?page=2', + }); + assert.deepEqual(halRes._links.first, { + href: '/user', + }); + assert.deepEqual(halRes._links.last, { + href: '/user?page=3', + }); + assert.deepEqual(halRes._links.previous, { + href: '/user', + }); + assert.deepEqual(halRes._links.next, { + href: '/user?page=3', + }); + }); }); });