From 04e3c1d1d23608efe373d50712d820687dee9201 Mon Sep 17 00:00:00 2001 From: truongnguyen Date: Wed, 25 Nov 2020 14:54:21 +0700 Subject: [PATCH 1/4] fix(api-gateway): :bug: graphql relay connection findAll do not work (#120) --- .../api-gateway/src/graphql-common-utils.ts | 84 +++++++++++++++++++ packages/api-gateway/src/graphql-relay.ts | 13 +-- packages/example-api-gateway/src/models.ts | 29 +++++-- 3 files changed, 109 insertions(+), 17 deletions(-) create mode 100644 packages/api-gateway/src/graphql-common-utils.ts diff --git a/packages/api-gateway/src/graphql-common-utils.ts b/packages/api-gateway/src/graphql-common-utils.ts new file mode 100644 index 0000000..ad687c1 --- /dev/null +++ b/packages/api-gateway/src/graphql-common-utils.ts @@ -0,0 +1,84 @@ +import { Connection, cursorToOffset, offsetToCursor } from 'graphql-relay'; +import type { QueryInput } from '@aloxide/demux'; + +const DEFAULT_ITEM_PER_PAGE = 20; + +export function updateCursorToOffet(args: any): QueryInput { + const updatedArgs: QueryInput = Object.assign({}, args); + const { after, before } = updatedArgs; + + if (typeof before === 'string' && !isNaN(cursorToOffset(before))) { + updatedArgs.before = cursorToOffset(before).toString(); + } else delete updatedArgs.before; + + if (typeof after === 'string' && !isNaN(cursorToOffset(after))) { + updatedArgs.after = cursorToOffset(after).toString(); + } else delete updatedArgs.after; + + if (updatedArgs.first) updatedArgs.first += 1; + if (updatedArgs.last) updatedArgs.last += 1; + + return updatedArgs; +} + +export function paginationInfo(items: any[], args: QueryInput): Connection { + if (args.first) { + return forwardPaginationInfo(items, args.first, args.after); + } + + if (args.last) { + return backwardPaginationInfo(items, args.last, args.before); + } + + return forwardPaginationInfo(items, DEFAULT_ITEM_PER_PAGE); +} + +export function forwardPaginationInfo( + edges: any[], + first: number, + after: string = null, +): Connection { + let hasNextPage = false; + if (edges.length > first) { + edges = edges.slice(0, first); + hasNextPage = true; + } + edges = edges.map(value => ({ + cursor: offsetToCursor(value.id), + node: value, + })); + return { + edges, + pageInfo: { + startCursor: edges[0]?.cursor, + endCursor: edges[edges.length - 1]?.cursor, + hasNextPage, + hasPreviousPage: after ? true : false, + }, + }; +} + +export function backwardPaginationInfo( + edges: any[], + last: number, + before: string = null, +): Connection { + let hasPreviousPage = false; + if (edges.length > last) { + edges = edges.slice(0, last); + hasPreviousPage = true; + } + edges = edges?.reverse()?.map(value => ({ + cursor: offsetToCursor(value.id), + node: value, + })); + return { + edges, + pageInfo: { + startCursor: edges[0]?.cursor, + endCursor: edges[edges.length - 1]?.cursor, + hasNextPage: before ? true : false, + hasPreviousPage, + }, + }; +} diff --git a/packages/api-gateway/src/graphql-relay.ts b/packages/api-gateway/src/graphql-relay.ts index a0b9dc7..79b9cb3 100644 --- a/packages/api-gateway/src/graphql-relay.ts +++ b/packages/api-gateway/src/graphql-relay.ts @@ -1,14 +1,10 @@ import { FieldTypeEnum, Interpreter } from '@aloxide/bridge'; import { AloxideDataManager } from '@aloxide/demux'; import { GraphQLObjectType } from 'graphql'; -import { - connectionArgs, - ConnectionConfigNodeType, - connectionDefinitions, - connectionFromArray, -} from 'graphql-relay'; +import { connectionArgs, ConnectionConfigNodeType, connectionDefinitions } from 'graphql-relay'; import { GraphqlTypeInterpreter } from './GraphqlTypeInterpreter'; +import { updateCursorToOffet, paginationInfo } from './graphql-common-utils'; import type { GraphQLFieldConfig, GraphQLScalarType, GraphQLNamedType } from 'graphql'; import type { GraphQLConnectionDefinitions } from 'graphql-relay'; @@ -79,11 +75,10 @@ export function createGraphQl(config: CreateGraphQlConfig): CreateGraphQlOutput[ type: connectionType, args: connectionArgs, resolve: (_, args) => { - const queryInput: QueryInput = args; - // TODO fix find all + const queryInput: QueryInput = updateCursorToOffet(args); return dataAdapter .findAll(name, queryInput, metaData) - .then(items => connectionFromArray(items, args)); + .then(items => paginationInfo(items, args)); }, }; diff --git a/packages/example-api-gateway/src/models.ts b/packages/example-api-gateway/src/models.ts index d8f4200..9bbcc62 100644 --- a/packages/example-api-gateway/src/models.ts +++ b/packages/example-api-gateway/src/models.ts @@ -57,15 +57,28 @@ export function createDataProvider( return m.count(); }, - findAll({ limit, after }, { entity: { key } }): Promise { - return m.findAll({ - limit, - where: after && { - [key]: { - [Op.gt]: after, + findAll({ first, after, last, before }, { entity: { key } }): Promise { + if (first) + return m.findAll({ + limit: first, + where: after && { + [key]: { + [Op.gt]: after, + }, }, - }, - }); + order: [[key, 'asc']], + }); + + if (last) + return m.findAll({ + limit: last, + where: before && { + [key]: { + [Op.lt]: before, + }, + }, + order: [[key, 'desc']], + }); }, find(id: any, meta?: any): Promise { From d93c71b8c56635972c082522651d5e7461ac5800 Mon Sep 17 00:00:00 2001 From: truongnguyen Date: Thu, 26 Nov 2020 10:39:59 +0700 Subject: [PATCH 2/4] fix(api-gateway): :bug: use connection name as prefix when create cursor (#120) --- .../api-gateway/src/graphql-common-utils.ts | 107 ++++++++++++------ packages/api-gateway/src/graphql-relay.ts | 6 +- 2 files changed, 75 insertions(+), 38 deletions(-) diff --git a/packages/api-gateway/src/graphql-common-utils.ts b/packages/api-gateway/src/graphql-common-utils.ts index ad687c1..0869c28 100644 --- a/packages/api-gateway/src/graphql-common-utils.ts +++ b/packages/api-gateway/src/graphql-common-utils.ts @@ -1,19 +1,50 @@ -import { Connection, cursorToOffset, offsetToCursor } from 'graphql-relay'; +import { Connection, ConnectionCursor } from 'graphql-relay'; import type { QueryInput } from '@aloxide/demux'; -const DEFAULT_ITEM_PER_PAGE = 20; +export interface PageInfoInput { + entityName: string; + edges: any[]; + limit: number; + offset?: string; +} + +const NUMER_OF_ITEMS_PER_PAGE = 20; + +export type Base64String = string; + +export function base64(i: string): Base64String { + return Buffer.from(i, 'utf8').toString('base64'); +} + +export function unbase64(i: Base64String): string { + return Buffer.from(i, 'base64').toString('utf8'); +} -export function updateCursorToOffet(args: any): QueryInput { - const updatedArgs: QueryInput = Object.assign({}, args); - const { after, before } = updatedArgs; +/** + * Creates the cursor string from an offset. + */ +export function offsetToCursor(prefix: string, offset: number): ConnectionCursor { + return base64(prefix + offset); +} +/** + * Rederives the offset from the cursor string. + */ +export function cursorToOffset(prefix: string, cursor: ConnectionCursor): number { + if (cursor) return parseInt(unbase64(cursor).substring(prefix.length), 10); + return NaN; +} + +export function convertCursorToOffet(entityName: string, args: any): QueryInput { + const updatedArgs: QueryInput = { ...args }; + + const beforOffset = cursorToOffset(entityName, updatedArgs.before); + const afterOffset = cursorToOffset(entityName, updatedArgs.after); - if (typeof before === 'string' && !isNaN(cursorToOffset(before))) { - updatedArgs.before = cursorToOffset(before).toString(); - } else delete updatedArgs.before; + if (isNaN(beforOffset)) delete updatedArgs.before; + else updatedArgs.before = beforOffset.toString(); - if (typeof after === 'string' && !isNaN(cursorToOffset(after))) { - updatedArgs.after = cursorToOffset(after).toString(); - } else delete updatedArgs.after; + if (isNaN(afterOffset)) delete updatedArgs.after; + else updatedArgs.after = afterOffset.toString(); if (updatedArgs.first) updatedArgs.first += 1; if (updatedArgs.last) updatedArgs.last += 1; @@ -21,30 +52,40 @@ export function updateCursorToOffet(args: any): QueryInput { return updatedArgs; } -export function paginationInfo(items: any[], args: QueryInput): Connection { +export function paginationInfo( + entityName: string, + items: any[], + args: QueryInput, +): Connection { if (args.first) { - return forwardPaginationInfo(items, args.first, args.after); + return forwardPaginationInfo({ + entityName, + edges: items, + limit: args.first, + offset: args.after, + }); } if (args.last) { - return backwardPaginationInfo(items, args.last, args.before); + return backwardPaginationInfo({ + entityName, + edges: items, + limit: args.last, + offset: args.before, + }); } - return forwardPaginationInfo(items, DEFAULT_ITEM_PER_PAGE); + return forwardPaginationInfo({ entityName, edges: items, limit: NUMER_OF_ITEMS_PER_PAGE }); } -export function forwardPaginationInfo( - edges: any[], - first: number, - after: string = null, -): Connection { +export function forwardPaginationInfo(pageInput: PageInfoInput): Connection { let hasNextPage = false; - if (edges.length > first) { - edges = edges.slice(0, first); + if (pageInput.edges.length > pageInput.limit) { + pageInput.edges = pageInput.edges.slice(0, pageInput.limit); hasNextPage = true; } - edges = edges.map(value => ({ - cursor: offsetToCursor(value.id), + const edges = pageInput.edges.map(value => ({ + cursor: offsetToCursor(pageInput.entityName, value.id), node: value, })); return { @@ -53,23 +94,19 @@ export function forwardPaginationInfo( startCursor: edges[0]?.cursor, endCursor: edges[edges.length - 1]?.cursor, hasNextPage, - hasPreviousPage: after ? true : false, + hasPreviousPage: pageInput.offset ? true : false, }, }; } -export function backwardPaginationInfo( - edges: any[], - last: number, - before: string = null, -): Connection { +export function backwardPaginationInfo(pageInput: PageInfoInput): Connection { let hasPreviousPage = false; - if (edges.length > last) { - edges = edges.slice(0, last); + if (pageInput.edges.length > pageInput.limit) { + pageInput.edges = pageInput.edges.slice(0, pageInput.limit); hasPreviousPage = true; } - edges = edges?.reverse()?.map(value => ({ - cursor: offsetToCursor(value.id), + const edges = pageInput.edges?.reverse()?.map(value => ({ + cursor: offsetToCursor(pageInput.entityName, value.id), node: value, })); return { @@ -77,7 +114,7 @@ export function backwardPaginationInfo( pageInfo: { startCursor: edges[0]?.cursor, endCursor: edges[edges.length - 1]?.cursor, - hasNextPage: before ? true : false, + hasNextPage: pageInput.offset ? true : false, hasPreviousPage, }, }; diff --git a/packages/api-gateway/src/graphql-relay.ts b/packages/api-gateway/src/graphql-relay.ts index 79b9cb3..8931a2f 100644 --- a/packages/api-gateway/src/graphql-relay.ts +++ b/packages/api-gateway/src/graphql-relay.ts @@ -4,7 +4,7 @@ import { GraphQLObjectType } from 'graphql'; import { connectionArgs, ConnectionConfigNodeType, connectionDefinitions } from 'graphql-relay'; import { GraphqlTypeInterpreter } from './GraphqlTypeInterpreter'; -import { updateCursorToOffet, paginationInfo } from './graphql-common-utils'; +import { convertCursorToOffet, paginationInfo } from './graphql-common-utils'; import type { GraphQLFieldConfig, GraphQLScalarType, GraphQLNamedType } from 'graphql'; import type { GraphQLConnectionDefinitions } from 'graphql-relay'; @@ -75,10 +75,10 @@ export function createGraphQl(config: CreateGraphQlConfig): CreateGraphQlOutput[ type: connectionType, args: connectionArgs, resolve: (_, args) => { - const queryInput: QueryInput = updateCursorToOffet(args); + const queryInput: QueryInput = convertCursorToOffet(connectionType.name, args); return dataAdapter .findAll(name, queryInput, metaData) - .then(items => paginationInfo(items, args)); + .then(items => paginationInfo(connectionType.name, items, args)); }, }; From 20636e8801c9c2361c58ac81e5682bb14338236a Mon Sep 17 00:00:00 2001 From: truongnguyen Date: Thu, 26 Nov 2020 14:46:00 +0700 Subject: [PATCH 3/4] fix(api-gateway): :white_check_mark: add unit test (#120) --- .../test/graphql-common-ultils.test.ts | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 packages/api-gateway/test/graphql-common-ultils.test.ts diff --git a/packages/api-gateway/test/graphql-common-ultils.test.ts b/packages/api-gateway/test/graphql-common-ultils.test.ts new file mode 100644 index 0000000..661570d --- /dev/null +++ b/packages/api-gateway/test/graphql-common-ultils.test.ts @@ -0,0 +1,111 @@ +import { QueryInput } from '@aloxide/demux/src'; +import { + base64, + unbase64, + offsetToCursor, + cursorToOffset, + convertCursorToOffet, + forwardPaginationInfo, + backwardPaginationInfo, + paginationInfo, +} from '../src/graphql-common-utils'; + +describe('Graphql Common Ultils', () => { + const entityName = 'test'; + + it('Should convert to base64', () => { + expect(base64('hello')).toBe('aGVsbG8='); + }); + + it('Should revert to string', () => { + expect(unbase64('aGVsbG8=')).toBe('hello'); + }); + + it('Should parse offset to cursor', () => { + expect(offsetToCursor(entityName, 1)).toBe('dGVzdDE='); + }); + + it('Should revert cursor to offset', () => { + expect(cursorToOffset(entityName, 'dGVzdDE=')).toBe(1); + }); + + it('Revert offset Should return NaN', () => { + expect(cursorToOffset(entityName, 'hello')).toBeNaN(); + }); + + it('Should convert cursor to offset', () => { + const input: QueryInput = { + first: 2, + after: 'test', + last: 2, + before: 'test', + }; + + const resp = convertCursorToOffet(entityName, input); + expect(resp.first).toBe(3); + expect(resp.last).toBe(3); + expect(resp.after).toBeUndefined(); + expect(resp.before).toBeUndefined(); + }); + + it('Should convert cursor to offset', () => { + const input: QueryInput = { + first: 2, + after: 'dGVzdDE=', + last: 2, + before: 'dGVzdDE=', + }; + + const resp = convertCursorToOffet(entityName, input); + expect(resp.first).toBe(3); + expect(resp.last).toBe(3); + expect(resp.after).toBe('1'); + expect(resp.before).toBe('1'); + }); + + it('Should forward pagination', () => { + const items = [ + { + id: 1, + }, + { + id: 2, + }, + { + id: 3, + }, + ]; + + const resp = paginationInfo(entityName, items, { first: 2 }); + + expect(resp.edges).toBeDefined(); + expect(resp.edges[0].cursor).toBe('dGVzdDE='); + expect(resp.pageInfo.startCursor).toBe('dGVzdDE='); + expect(resp.pageInfo.endCursor).toBe('dGVzdDI='); + expect(resp.pageInfo.hasNextPage).toBe(true); + expect(resp.pageInfo.hasPreviousPage).toBe(false); + }); + + it('Should backward pagination 2', () => { + const items = [ + { + id: 37, + }, + { + id: 36, + }, + { + id: 35, + }, + ]; + + const resp = paginationInfo(entityName, items, { last: 2 }); + + expect(resp.edges).toBeDefined(); + expect(resp.edges[0].cursor).toBe('dGVzdDM2'); + expect(resp.pageInfo.startCursor).toBe('dGVzdDM2'); + expect(resp.pageInfo.endCursor).toBe('dGVzdDM3'); + expect(resp.pageInfo.hasNextPage).toBe(false); + expect(resp.pageInfo.hasPreviousPage).toBe(true); + }); +}); From e09a50c05e4d0a30953cd2f1d8c6d97cbfde3cac Mon Sep 17 00:00:00 2001 From: truongnguyen Date: Thu, 26 Nov 2020 16:20:26 +0700 Subject: [PATCH 4/4] fix(api-gateway): :white_check_mark: enhance unit test of graphql-common-utils to 100% --- .../api-gateway/src/graphql-common-utils.ts | 2 +- .../test/graphql-common-ultils.test.ts | 58 ++++++++++++++++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/packages/api-gateway/src/graphql-common-utils.ts b/packages/api-gateway/src/graphql-common-utils.ts index 0869c28..f934b41 100644 --- a/packages/api-gateway/src/graphql-common-utils.ts +++ b/packages/api-gateway/src/graphql-common-utils.ts @@ -105,7 +105,7 @@ export function backwardPaginationInfo(pageInput: PageInfoInput): Connection< pageInput.edges = pageInput.edges.slice(0, pageInput.limit); hasPreviousPage = true; } - const edges = pageInput.edges?.reverse()?.map(value => ({ + const edges = pageInput.edges.reverse().map(value => ({ cursor: offsetToCursor(pageInput.entityName, value.id), node: value, })); diff --git a/packages/api-gateway/test/graphql-common-ultils.test.ts b/packages/api-gateway/test/graphql-common-ultils.test.ts index 661570d..67f0aa1 100644 --- a/packages/api-gateway/test/graphql-common-ultils.test.ts +++ b/packages/api-gateway/test/graphql-common-ultils.test.ts @@ -86,7 +86,30 @@ describe('Graphql Common Ultils', () => { expect(resp.pageInfo.hasPreviousPage).toBe(false); }); - it('Should backward pagination 2', () => { + it('Should forward pagination with after', () => { + const items = [ + { + id: 1, + }, + { + id: 2, + }, + { + id: 3, + }, + ]; + + const resp = paginationInfo(entityName, items, { first: 2, after: 'test' }); + + expect(resp.edges).toBeDefined(); + expect(resp.edges[0].cursor).toBe('dGVzdDE='); + expect(resp.pageInfo.startCursor).toBe('dGVzdDE='); + expect(resp.pageInfo.endCursor).toBe('dGVzdDI='); + expect(resp.pageInfo.hasNextPage).toBe(true); + expect(resp.pageInfo.hasPreviousPage).toBe(true); + }); + + it('Should backward pagination', () => { const items = [ { id: 37, @@ -108,4 +131,37 @@ describe('Graphql Common Ultils', () => { expect(resp.pageInfo.hasNextPage).toBe(false); expect(resp.pageInfo.hasPreviousPage).toBe(true); }); + + it('Should backward pagination with before', () => { + const items = [ + { + id: 37, + }, + { + id: 36, + }, + { + id: 35, + }, + ]; + + const resp = paginationInfo(entityName, items, { last: 2, before: 'test' }); + + expect(resp.edges).toBeDefined(); + expect(resp.edges[0].cursor).toBe('dGVzdDM2'); + expect(resp.pageInfo.startCursor).toBe('dGVzdDM2'); + expect(resp.pageInfo.endCursor).toBe('dGVzdDM3'); + expect(resp.pageInfo.hasNextPage).toBe(true); + expect(resp.pageInfo.hasPreviousPage).toBe(true); + }); + + it('Should backward pagination by emtpy items', () => { + const resp = paginationInfo(entityName, [], { last: 2, before: 'test' }); + + expect(resp.edges).toBeDefined(); + expect(resp.pageInfo.startCursor).toBeUndefined(); + expect(resp.pageInfo.endCursor).toBeUndefined(); + expect(resp.pageInfo.hasNextPage).toBe(true); + expect(resp.pageInfo.hasPreviousPage).toBe(false); + }); });