Refactor cursor pagination

- Expect explicit list of fields to sort on.
- If not provided, take the list of sort fields from the query itself.
- Fix crash on naming clash when e.g. paginating over `id` with subquery also having `id`.

Author: IlyaSemenov

.changeset/pretty-laws-explode.md

@@ -0,0 +1,10 @@
+---
+"objection-graphql-resolver": minor
+"orchid-graphql": minor
+---
+
+Refactor cursor pagination:
+
+- Expect explicit list of fields to sort on.
+- If not provided, take the list of sort fields from the query itself.
+- Fix crash on naming clash when e.g. paginating over `id` with subquery also having `id`.

packages/base/package.json

@@ -13,6 +13,7 @@
     "graphql": "^16"
   },
   "devDependencies": {
+    "@types/node": "^22.10.1",
     "tsconfig-vite-node": "^1.1.2"
   }
 }

packages/base/src/index.ts

@@ -1,4 +1,4 @@
-export { OrmAdapter, OrmModifier } from "./orm/orm"
+export { OrmAdapter, OrmModifier, SortOrder } from "./orm/orm"
 export { Paginator } from "./paginators/base"
 export { defineCursorPaginator } from "./paginators/cursor"
 export { defineFieldResolver } from "./resolvers/field"

packages/base/src/orm/orm.ts

@@ -5,6 +5,12 @@ export type OrmModifier<Orm extends OrmAdapter> = (
 	...args: any[]
 ) => Orm["Query"]
 
+export interface SortOrder {
+	field: string
+	dir: "ASC" | "DESC"
+	// TODO: add nulls first/last
+}
+
 export interface OrmAdapter<Table = any, Query = any, QueryTransform = any> {
 	// Types
 
@@ -57,7 +63,9 @@ export interface OrmAdapter<Table = any, Query = any, QueryTransform = any> {
 
 	reset_query_order(query: Query): Query
 
-	add_query_order(query: Query, field: string, desc: boolean): Query
+	add_query_order(query: Query, order: SortOrder): Query
+
+	get_query_order(query: Query): SortOrder[]
 
 	set_query_limit(query: Query, limit: number): Query
 

packages/base/src/paginators/cursor.ts

@@ -1,4 +1,6 @@
-import { OrmAdapter } from "../orm/orm"
+import { Buffer } from "node:buffer"
+
+import { OrmAdapter, SortOrder } from "../orm/orm"
 import { PaginateContext, Paginator } from "./base"
 
 export function defineCursorPaginator(
@@ -8,8 +10,8 @@ export function defineCursorPaginator(
 }
 
 export interface CursorPaginatorOptions {
-	fields: string[]
-	take: number
+	fields?: string[]
+	take?: number
 }
 
 export interface CursorPaginatorArgs {
@@ -26,20 +28,17 @@ class CursorPaginator<Orm extends OrmAdapter, Context>
 	implements Paginator<Orm, Context>
 {
 	readonly path = ["nodes"]
-	readonly options: CursorPaginatorOptions
 
-	readonly fields: Array<{
+	readonly pageSize: number
+
+	readonly fields?: Array<{
 		name: string
 		desc: boolean
 	}>
 
-	constructor(options: Partial<CursorPaginatorOptions> = {}) {
-		this.options = {
-			fields: ["id"],
-			take: 10,
-			...options,
-		}
-		this.fields = this.options.fields.map((field) => {
+	constructor(options: CursorPaginatorOptions = {}) {
+		this.pageSize = options.take ?? 10
+		this.fields = options.fields?.map((field) => {
 			if (field.startsWith("-")) {
 				return { name: field.slice(1), desc: true }
 			} else {
@@ -52,63 +51,92 @@ class CursorPaginator<Orm extends OrmAdapter, Context>
 		const { orm } = context.graph
 		const { args } = context.tree
 
-		const take = (args.take as number | undefined) ?? this.options.take
+		const pageSize = (args.take as number | undefined) ?? this.pageSize
 		const cursor = args.cursor as string | undefined
 
-		// Set query order
-		query = orm.reset_query_order(query)
-		for (const field of this.fields) {
-			// TODO: prevent potential name clash with aliases like .as(`_${table_ref}_order_key_0`)
-			query = orm.select_field(query, { field: field.name, as: field.name })
-			query = orm.add_query_order(query, field.name, field.desc)
+		const table = orm.get_query_table(query)
+
+		const orderFields = (
+			this.fields
+				? this.fields.map<SortOrder>((f) => ({
+						field: f.name,
+						dir: f.desc ? "DESC" : "ASC",
+					}))
+				: orm.get_query_order(query)
+		).map((o) => ({ ...o, alias: "_order_" + o.field }))
+
+		if (this.fields) {
+			query = orm.reset_query_order(query)
+			for (const { alias, dir } of orderFields) {
+				query = orm.add_query_order(query, { field: alias, dir })
+			}
+		}
+
+		if (!orderFields.length) {
+			throw new Error("Query must be ordered.")
+		}
+
+		for (const { field, alias } of orderFields) {
+			query = orm.select_field(query, { field, as: alias })
 		}
 
 		if (cursor) {
-			const { expression, bindings } = this._parse_cursor(cursor)
-			query = orm.where_raw(query, expression, bindings)
+			const parsedCursor = parseCursor(cursor)
+			// Prepare raw SQL.
+			// For example, for (amount asc, id asc) order, that would be:
+			// (amount, $id) >= ($amount, id)
+			const left: string[] = []
+			const right: string[] = []
+			for (let i = 0; i < orderFields.length; ++i) {
+				const { field, alias, dir } = orderFields[i]
+				const [expressions, placeholders] =
+					dir === "ASC" ? [left, right] : [right, left]
+				expressions.push(`"${table}"."${field}"`)
+				placeholders.push("$" + alias)
+			}
+			const sqlExpr = `(${left.join(",")}) > (${right.join(",")})`
+			const bindings = Object.fromEntries(
+				orderFields.map(({ alias }, i) => [alias, parsedCursor[i]]),
+			)
+			query = orm.where_raw(query, sqlExpr, bindings)
+		}
+		query = orm.set_query_limit(query, pageSize + 1)
+
+		// TODO add support for reverse cursor, borrow implementation from orchid-pagination.
+		function createNodeCursor(node: any) {
+			return createCursor(
+				orderFields.map(({ field, alias }) => {
+					const value = node[alias]
+					// TODO add support for custom serializer(s).
+					if (value === undefined) {
+						throw new Error(
+							`Unable to create cursor: undefined field ${field} (${alias})`,
+						)
+					}
+					return String(value)
+				}),
+			)
 		}
-		query = orm.set_query_limit(query, take + 1)
 
 		return orm.set_query_page_result(query, (nodes) => {
 			let cursor: string | undefined
-			if (nodes.length > take) {
-				cursor = this._create_cursor(nodes[take - 1])
-				nodes = nodes.slice(0, take)
+			if (nodes.length > pageSize) {
+				cursor = createNodeCursor(nodes[pageSize - 1])
+				nodes = nodes.slice(0, pageSize)
 			}
 			return { nodes, cursor }
 		})
 	}
+}
 
-	_create_cursor(instance: any) {
-		return JSON.stringify(
-			this.fields.map((field) => {
-				const value = instance[field.name]
-				if (value === undefined) {
-					throw new Error(
-						`Unable to create cursor: undefined field ${field.name}`,
-					)
-				}
-				return String(value)
-			}),
-		)
-	}
+function createCursor(parts: string[]) {
+	return Buffer.from(parts.map(String).join(String.fromCharCode(0))).toString(
+		"base64url",
+	)
+}
 
-	_parse_cursor(cursor: string) {
-		const values = JSON.parse(cursor)
-		const left: string[] = []
-		const right: string[] = []
-		const bindings: Record<string, any> = {}
-		for (let i = 0; i < this.fields.length; ++i) {
-			const field = this.fields[i]
-			const expressions = field.desc ? right : left
-			const placeholders = field.desc ? left : right
-			expressions.push(`"${field.name}"`)
-			placeholders.push("$" + field.name)
-			bindings[field.name] = values[i]
-		}
-		return {
-			expression: `(${left.join(",")}) > (${right.join(",")})`,
-			bindings,
-		}
-	}
+function parseCursor(cursor: string): string[] {
+	return Buffer.from(cursor, "base64url")
+		.toString()
+		.split(String.fromCharCode(0))
 }

packages/objection/docs/pagination.md

@@ -14,7 +14,7 @@ The library includes simple `CursorPaginator` implementation which traverses ord
     { "id": 1, "foo": "bar" },
     { "id": 2, "foo": "baz" }
   ],
-  "cursor": "xyzzy"
+  "cursor": "encoded-string-cursor-to-fetch-next-page"
 }
 ```
 
@@ -81,11 +81,10 @@ const graph = r.graph({
     fields: {
       id: true,
       name: true,
-      // If it were posts: true, all posts will be returned.
+      // If it were posts: true, all posts would be returned.
       // Instead, return a page of posts sorted by newest first.
       posts: r.page(r.cursor({ fields: ["-id"], take: 10 })),
-      // Should you want this, it's still possible to pull all posts (non-paginated)
-      // under a different GraphQL field
+      // Pull all posts (non-paginated) under a different GraphQL field.
       all_posts: r.relation({ modelField: "posts" }),
     },
   }),
@@ -99,8 +98,10 @@ const resolvers = {
     },
     posts: (parent, args, context, info) => {
       return graph.resolvePage(
-        Post.query(),
-        r.cursor({ fields: ["-id"], take: 10 }),
+        // Pagination fields can be taken from cursor({ fields })
+        // or from the query itself, like this:
+        Post.query().orderBy("id", "desc"),
+        r.cursor({ take: 10 }),
         { context, info },
       )
     },

packages/objection/src/orm/orm.ts

@@ -1,4 +1,9 @@
-import { OrmAdapter, OrmModifier, run_after_query } from "graphql-orm"
+import {
+	OrmAdapter,
+	OrmModifier,
+	run_after_query,
+	SortOrder,
+} from "graphql-orm"
 import { QueryBuilder, raw, ref, RelationMappings } from "objection"
 import { Model } from "objection"
 import { AnyModelConstructor, AnyQueryBuilder, ModelClass } from "objection"
@@ -100,8 +105,19 @@ export const orm: ObjectionOrm = {
 		return query.clearOrder()
 	},
 
-	add_query_order(query, field, desc) {
-		return query.orderBy(field, desc ? "desc" : "asc")
+	add_query_order(query, { field, dir }) {
+		return query.orderBy(field, dir)
+	},
+
+	get_query_order(query) {
+		const sortOrders: SortOrder[] = []
+		;(query as any).forEachOperation(/orderBy/, (op: any) => {
+			if (op.name === "orderBy") {
+				const [field, dir] = op.args
+				sortOrders.push({ field, dir })
+			}
+		})
+		return sortOrders
 	},
 
 	set_query_limit(query, limit) {

packages/objection/tests/main/nested-pagination.test.ts

@@ -111,12 +111,12 @@ const graph = r.graph({
 		fields: {
 			id: true,
 			name: true,
-			posts: r.page(r.cursor({ take: 2 })),
+			posts: r.page(r.cursor({ fields: ["id"], take: 2 })),
 			// TODO: test the fields below
-			posts_page: r.page(r.cursor({ take: 2 }), {
+			posts_page: r.page(r.cursor({ fields: ["id"], take: 2 }), {
 				modelField: "posts",
 			}),
-			posts_by_one: r.page(r.cursor({ take: 1 }), {
+			posts_by_one: r.page(r.cursor({ fields: ["id"], take: 1 }), {
 				modelField: "posts",
 			}),
 			all_posts: "posts",
@@ -127,7 +127,7 @@ const graph = r.graph({
 		fields: {
 			id: true,
 			name: true,
-			posts: r.page(r.cursor({ take: 2 }), {
+			posts: r.page(r.cursor({ fields: ["id"], take: 2 }), {
 				filters: true,
 			}),
 		},
@@ -208,7 +208,7 @@ test("nested pagination", async () => {
 		  "user": {
 		    "name": "Alice",
 		    "posts": {
-		      "cursor": "["2"]",
+		      "cursor": "Mg",
 		      "nodes": [
 		        {
 		          "id": 1,
@@ -270,7 +270,7 @@ test("nested pagination", async () => {
 		    "id": 1,
 		    "name": "Alice",
 		    "posts": {
-		      "cursor": "["2"]",
+		      "cursor": "Mg",
 		      "nodes": [
 		        {
 		          "author": {
@@ -280,7 +280,7 @@ test("nested pagination", async () => {
 		          "section": {
 		            "name": "News",
 		            "posts": {
-		              "cursor": "["2"]",
+		              "cursor": "Mg",
 		              "nodes": [
 		                {
 		                  "author": {
@@ -313,7 +313,7 @@ test("nested pagination", async () => {
 		          "section": {
 		            "name": "Editorial",
 		            "posts": {
-		              "cursor": "["2"]",
+		              "cursor": "Mg",
 		              "nodes": [
 		                {
 		                  "author": {
@@ -374,7 +374,7 @@ test("nested pagination", async () => {
 		  "user": {
 		    "name": "Bob",
 		    "posts": {
-		      "cursor": "["4"]",
+		      "cursor": "NA",
 		      "nodes": [
 		        {
 		          "author": {