tabshift/payloadcms
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

fix(db-*): ensure consistent sorting even when sorting on non-unique fields or no sort parameters at all (#12447)

Browse Source 
The databases do not keep track of document order internally so when
sorting by non-unique fields such as shared `order` number values, the
returned order will be random and not consistent.

While this issue is far more noticeable on mongo it could also occur in
postgres on certain environments.

This combined with pagination can lead to the perception of duplicated
or inconsistent data.

This PR adds a second sort parameter to queries so that we always have a
fallback, `-createdAt` will be used by default or `id` if timestamps are
disabled.
This commit is contained in:
Paul
2025-05-19 12:59:12 -07:00
committed by GitHub
parent 2a929cf385
commit 72ab319d37
6 changed files with 268 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
packages/db-mongodb/src/queries/buildSortParam.ts
View File

@@ -150,6 +150,18 @@ export const buildSortParam = ({
sort = [sort]
}
// In the case of Mongo, when sorting by a field that is not unique, the results are not guaranteed to be in the same order each time.
// So we add a fallback sort to ensure that the results are always in the same order.
let fallbackSort = '-id'
if (timestamps) {
fallbackSort = '-createdAt'
}
if (!(sort.includes(fallbackSort) || sort.includes(fallbackSort.replace('-', '')))) {
sort.push(fallbackSort)
}
const sorting = sort.reduce<Record<string, string>>((acc, item) => {
let sortProperty: string
let sortDirection: SortDirection

15
packages/drizzle/src/queries/buildOrderBy.ts
View File

@@ -39,8 +39,9 @@ export const buildOrderBy = ({
}: Args): BuildQueryResult['orderBy'] => {
const orderBy: BuildQueryResult['orderBy'] = []
const createdAt = adapter.tables[tableName]?.createdAt
if (!sort) {
const createdAt = adapter.tables[tableName]?.createdAt
if (createdAt) {
sort = '-createdAt'
} else {
@@ -52,6 +53,18 @@ export const buildOrderBy = ({
sort = [sort]
}
// In the case of Mongo, when sorting by a field that is not unique, the results are not guaranteed to be in the same order each time.
// So we add a fallback sort to ensure that the results are always in the same order.
let fallbackSort = '-id'
if (createdAt) {
fallbackSort = '-createdAt'
}
if (!(sort.includes(fallbackSort) || sort.includes(fallbackSort.replace('-', '')))) {
sort.push(fallbackSort)
}
for (const sortItem of sort) {
let sortProperty: string
let sortDirection: 'asc' | 'desc'

17
test/sort/collections/NonUniqueSort/index.ts Normal file
View File

@@ -0,0 +1,17 @@
import type { CollectionConfig } from 'payload'
export const nonUniqueSortSlug = 'non-unique-sort'
export const NonUniqueSortCollection: CollectionConfig = {
slug: nonUniqueSortSlug,
fields: [
{
name: 'title',
type: 'text',
},
{
name: 'order',
type: 'number',
},
],
}

25
test/sort/config.ts
View File

@@ -2,12 +2,14 @@ import type { CollectionSlug, Payload } from 'payload'
import { fileURLToPath } from 'node:url'
import path from 'path'
import { wait } from 'payload/shared'
import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
import { devUser } from '../credentials.js'
import { DefaultSortCollection } from './collections/DefaultSort/index.js'
import { DraftsCollection } from './collections/Drafts/index.js'
import { LocalizedCollection } from './collections/Localized/index.js'
import { NonUniqueSortCollection, nonUniqueSortSlug } from './collections/NonUniqueSort/index.js'
import { OrderableCollection } from './collections/Orderable/index.js'
import { OrderableJoinCollection } from './collections/OrderableJoin/index.js'
import { PostsCollection } from './collections/Posts/index.js'
@@ -19,6 +21,7 @@ export default buildConfigWithDefaults({
PostsCollection,
DraftsCollection,
DefaultSortCollection,
NonUniqueSortCollection,
LocalizedCollection,
OrderableCollection,
OrderableJoinCollection,
@@ -87,6 +90,28 @@ async function seedSortable(payload: Payload) {
await payload.create({ collection: 'orderable-join', data: { title: 'Join B' } })
// Create 10 items to be sorted by non-unique field
for (const i of Array.from({ length: 10 }, (_, index) => index)) {
let order = 1
if (i > 3) {
order = 2
} else if (i > 6) {
order = 3
}
await payload.create({
collection: nonUniqueSortSlug,
data: {
title: `Post ${i}`,
order,
},
})
// Wait 2 seconds to guarantee that the createdAt date is different
// await wait(2000)
}
return new Response(JSON.stringify({ success: true }), {
headers: { 'Content-Type': 'application/json' },
status: 200,

149
test/sort/int.spec.ts
View File

@@ -8,6 +8,7 @@ import type { Draft, Orderable, OrderableJoin } from './payload-types.js'
import { initPayloadInt } from '../helpers/initPayloadInt.js'
import { draftsSlug } from './collections/Drafts/index.js'
import { nonUniqueSortSlug } from './collections/NonUniqueSort/index.js'
import { orderableSlug } from './collections/Orderable/index.js'
import { orderableJoinSlug } from './collections/OrderableJoin/index.js'
@@ -133,6 +134,154 @@ describe('Sort', () => {
})
})
describe('Non-unique sorting', () => {
// There are situations where the sort order is not guaranteed to be consistent, such as when sorting by a non-unique field in MongoDB which does not keep an internal order of items
// As a result, every time you fetch, including fetching specific pages, the order of items may change and appear as duplicated to some users.
it('should always be consistent when sorting', async () => {
const posts = await payload.find({
collection: nonUniqueSortSlug,
sort: 'order',
})
const initialMap = posts.docs.map((post) => post.title)
const dataFetches = await Promise.all(
Array.from({ length: 3 }).map(() =>
payload.find({
collection: nonUniqueSortSlug,
sort: 'order',
}),
),
)
const [fetch1, fetch2, fetch3] = dataFetches.map((fetch) =>
fetch.docs.map((post) => post.title),
)
expect(fetch1).toEqual(initialMap)
expect(fetch2).toEqual(initialMap)
expect(fetch3).toEqual(initialMap)
})
it('should always be consistent when sorting - with limited pages', async () => {
const posts = await payload.find({
collection: nonUniqueSortSlug,
sort: 'order',
limit: 5,
page: 2,
})
const initialMap = posts.docs.map((post) => post.title)
const dataFetches = await Promise.all(
Array.from({ length: 3 }).map(() =>
payload.find({
collection: nonUniqueSortSlug,
sort: 'order',
limit: 5,
page: 2,
}),
),
)
const [fetch1, fetch2, fetch3] = dataFetches.map((fetch) =>
fetch.docs.map((post) => post.title),
)
expect(fetch1).toEqual(initialMap)
expect(fetch2).toEqual(initialMap)
expect(fetch3).toEqual(initialMap)
})
it('should sort by createdAt as fallback', async () => {
// This is the (reverse - newest first) order that the posts are created in so this should remain consistent as the sort should fallback to '-createdAt'
const postsInOrder = ['Post 9', 'Post 8', 'Post 7', 'Post 6']
const dataFetches = await Promise.all(
Array.from({ length: 3 }).map(() =>
payload.find({
collection: nonUniqueSortSlug,
sort: 'order',
page: 2,
limit: 4,
}),
),
)
const [fetch1, fetch2, fetch3] = dataFetches.map((fetch) =>
fetch.docs.map((post) => post.title),
)
console.log({ fetch1, fetch2, fetch3 })
expect(fetch1).toEqual(postsInOrder)
expect(fetch2).toEqual(postsInOrder)
expect(fetch3).toEqual(postsInOrder)
})
it('should always be consistent without sort params in the query', async () => {
const posts = await payload.find({
collection: nonUniqueSortSlug,
})
const initialMap = posts.docs.map((post) => post.title)
const dataFetches = await Promise.all(
Array.from({ length: 3 }).map(() =>
payload.find({
collection: nonUniqueSortSlug,
}),
),
)
const [fetch1, fetch2, fetch3] = dataFetches.map((fetch) =>
fetch.docs.map((post) => post.title),
)
expect(fetch1).toEqual(initialMap)
expect(fetch2).toEqual(initialMap)
expect(fetch3).toEqual(initialMap)
})
it('should always be consistent without sort params in the query - with limited pages', async () => {
const posts = await payload.find({
collection: nonUniqueSortSlug,
page: 2,
limit: 4,
})
const initialMap = posts.docs.map((post) => post.title)
const dataFetches = await Promise.all(
Array.from({ length: 3 }).map(() =>
payload.find({
collection: nonUniqueSortSlug,
page: 2,
limit: 4,
}),
),
)
const [fetch1, fetch2, fetch3] = dataFetches.map((fetch) =>
fetch.docs.map((post) => post.title),
)
expect(fetch1).toEqual(initialMap)
expect(fetch2).toEqual(initialMap)
expect(fetch3).toEqual(initialMap)
})
})
describe('Sort by multiple fields', () => {
it('should sort posts by multiple fields', async () => {
const posts = await payload.find({

71
test/sort/payload-types.ts
View File

@@ -70,6 +70,7 @@ export interface Config {
posts: Post;
drafts: Draft;
'default-sort': DefaultSort;
'non-unique-sort': NonUniqueSort;
localized: Localized;
orderable: Orderable;
'orderable-join': OrderableJoin;
@@ -89,6 +90,7 @@ export interface Config {
posts: PostsSelect<false> | PostsSelect<true>;
drafts: DraftsSelect<false> | DraftsSelect<true>;
'default-sort': DefaultSortSelect<false> | DefaultSortSelect<true>;
'non-unique-sort': NonUniqueSortSelect<false> | NonUniqueSortSelect<true>;
localized: LocalizedSelect<false> | LocalizedSelect<true>;
orderable: OrderableSelect<false> | OrderableSelect<true>;
'orderable-join': OrderableJoinSelect<false> | OrderableJoinSelect<true>;
@@ -98,7 +100,7 @@ export interface Config {
'payload-migrations': PayloadMigrationsSelect<false> | PayloadMigrationsSelect<true>;
};
db: {
defaultIDType: string;
defaultIDType: number;
};
globals: {};
globalsSelect: {};
@@ -134,7 +136,7 @@ export interface UserAuthOperations {
* via the `definition` "posts".
*/
export interface Post {
id: string;
id: number;
text?: string | null;
number?: number | null;
number2?: number | null;
@@ -164,18 +166,29 @@ export interface Draft {
* via the `definition` "default-sort".
*/
export interface DefaultSort {
id: string;
id: number;
text?: string | null;
number?: number | null;
updatedAt: string;
createdAt: string;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "non-unique-sort".
*/
export interface NonUniqueSort {
id: number;
title?: string | null;
order?: number | null;
updatedAt: string;
createdAt: string;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "localized".
*/
export interface Localized {
id: string;
id: number;
text?: string | null;
number?: number | null;
number2?: number | null;
@@ -196,7 +209,7 @@ export interface Orderable {
_orderable_orderableJoinField1_order?: string | null;
_order?: string | null;
title?: string | null;
orderableField?: (string | null) | OrderableJoin;
orderableField?: (number | null) | OrderableJoin;
updatedAt: string;
createdAt: string;
}
@@ -205,20 +218,20 @@ export interface Orderable {
* via the `definition` "orderable-join".
*/
export interface OrderableJoin {
id: string;
id: number;
title?: string | null;
orderableJoinField1?: {
docs?: (string | Orderable)[];
docs?: (number | Orderable)[];
hasNextPage?: boolean;
totalDocs?: number;
};
orderableJoinField2?: {
docs?: (string | Orderable)[];
docs?: (number | Orderable)[];
hasNextPage?: boolean;
totalDocs?: number;
};
nonOrderableJoinField?: {
docs?: (string | Orderable)[];
docs?: (number | Orderable)[];
hasNextPage?: boolean;
totalDocs?: number;
};
@@ -230,7 +243,7 @@ export interface OrderableJoin {
* via the `definition` "users".
*/
export interface User {
id: string;
id: number;
updatedAt: string;
createdAt: string;
email: string;
@@ -247,40 +260,44 @@ export interface User {
* via the `definition` "payload-locked-documents".
*/
export interface PayloadLockedDocument {
id: string;
id: number;
document?:
| ({
relationTo: 'posts';
value: string | Post;
value: number | Post;
} | null)
| ({
relationTo: 'drafts';
value: string | Draft;
value: number | Draft;
} | null)
| ({
relationTo: 'default-sort';
value: string | DefaultSort;
value: number | DefaultSort;
} | null)
| ({
relationTo: 'non-unique-sort';
value: number | NonUniqueSort;
} | null)
| ({
relationTo: 'localized';
value: string | Localized;
value: number | Localized;
} | null)
| ({
relationTo: 'orderable';
value: string | Orderable;
value: number | Orderable;
} | null)
| ({
relationTo: 'orderable-join';
value: string | OrderableJoin;
value: number | OrderableJoin;
} | null)
| ({
relationTo: 'users';
value: string | User;
value: number | User;
} | null);
globalSlug?: string | null;
user: {
relationTo: 'users';
value: string | User;
value: number | User;
};
updatedAt: string;
createdAt: string;
@@ -290,10 +307,10 @@ export interface PayloadLockedDocument {
* via the `definition` "payload-preferences".
*/
export interface PayloadPreference {
id: string;
id: number;
user: {
relationTo: 'users';
value: string | User;
value: number | User;
};
key?: string | null;
value?:
@@ -313,7 +330,7 @@ export interface PayloadPreference {
* via the `definition` "payload-migrations".
*/
export interface PayloadMigration {
id: string;
id: number;
name?: string | null;
batch?: number | null;
updatedAt: string;
@@ -359,6 +376,16 @@ export interface DefaultSortSelect<T extends boolean = true> {
updatedAt?: T;
createdAt?: T;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "non-unique-sort_select".
*/
export interface NonUniqueSortSelect<T extends boolean = true> {
title?: T;
order?: T;
updatedAt?: T;
createdAt?: T;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "localized_select".