fix(core): Ensure deterministic sorting in case of duplicates (#2632)

packages/core/e2e/default-search-plugin.e2e-spec.ts

@@ -20,6 +20,7 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest';
 import { initialData } from '../../../e2e-common/e2e-initial-data';
 import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-config';
 
+import { SEARCH_PRODUCTS_ADMIN } from './graphql/admin-definitions';
 import {
     ChannelFragment,
     CurrencyCode,
@@ -1920,24 +1921,6 @@ export const REINDEX = gql`
     }
 `;
 
-export const SEARCH_PRODUCTS_ADMIN = gql`
-    query SearchProductsAdmin($input: SearchInput!) {
-        search(input: $input) {
-            totalItems
-            items {
-                enabled
-                productId
-                productName
-                slug
-                description
-                productVariantId
-                productVariantName
-                sku
-            }
-        }
-    }
-`;
-
 export const SEARCH_GET_FACET_VALUES = gql`
     query SearchFacetValues($input: SearchInput!) {
         search(input: $input) {

packages/core/e2e/default-search-plugin/default-search-plugin-sort-by.e2e-spec.ts

@@ -0,0 +1,158 @@
+import { DefaultJobQueuePlugin, DefaultSearchPlugin, mergeConfig } from '@vendure/core';
+import { createTestEnvironment } from '@vendure/testing';
+import path from 'path';
+import { afterAll, beforeAll, describe, expect, it } from 'vitest';
+
+import { initialData } from '../../../../e2e-common/e2e-initial-data';
+import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../../e2e-common/test-config';
+import { SEARCH_PRODUCTS_ADMIN } from '../graphql/admin-definitions';
+import {
+    SearchResultSortParameter,
+    SortOrder,
+    SearchProductsAdminQuery,
+    SearchProductsAdminQueryVariables,
+} from '../graphql/generated-e2e-admin-types';
+import {
+    SearchProductsShopQuery,
+    SearchProductsShopQueryVariables,
+} from '../graphql/generated-e2e-shop-types';
+import { SEARCH_PRODUCTS_SHOP } from '../graphql/shop-definitions';
+import { awaitRunningJobs } from '../utils/await-running-jobs';
+
+describe('Default search plugin', () => {
+    const { server, adminClient, shopClient } = createTestEnvironment(
+        mergeConfig(testConfig(), {
+            plugins: [
+                DefaultSearchPlugin.init({
+                    indexStockStatus: true,
+                }),
+                DefaultJobQueuePlugin,
+            ],
+        }),
+    );
+
+    beforeAll(async () => {
+        await server.init({
+            initialData,
+            productsCsvPath: path.join(__dirname, 'fixtures', 'default-search-plugin-sort-by.csv'),
+            customerCount: 1,
+        });
+        await adminClient.asSuperAdmin();
+        await awaitRunningJobs(adminClient);
+    }, TEST_SETUP_TIMEOUT_MS);
+
+    afterAll(async () => {
+        await awaitRunningJobs(adminClient);
+        await server.destroy();
+    });
+
+    function searchProductsShop(queryVariables: SearchProductsShopQueryVariables) {
+        return shopClient.query<SearchProductsShopQuery, SearchProductsShopQueryVariables>(
+            SEARCH_PRODUCTS_SHOP,
+            queryVariables,
+        );
+    }
+
+    function searchProductsAdmin(queryVariables: SearchProductsAdminQueryVariables) {
+        return adminClient.query<SearchProductsAdminQuery, SearchProductsAdminQueryVariables>(
+            SEARCH_PRODUCTS_ADMIN,
+            queryVariables,
+        );
+    }
+
+    type SearchProducts = (
+        queryVariables: SearchProductsShopQueryVariables | SearchProductsAdminQueryVariables,
+    ) => Promise<SearchProductsShopQuery | SearchProductsAdminQuery>;
+
+    async function testSearchProducts(
+        searchProducts: SearchProducts,
+        groupByProduct: boolean,
+        sortBy: keyof SearchResultSortParameter,
+        sortOrder: (typeof SortOrder)[keyof typeof SortOrder],
+        skip: number,
+        take: number,
+    ) {
+        return searchProducts({
+            input: {
+                groupByProduct,
+                sort: {
+                    [sortBy]: sortOrder,
+                },
+                skip,
+                take,
+            },
+        });
+    }
+
+    async function testSortByPriceAsc(searchProducts: SearchProducts) {
+        const resultPage1 = await testSearchProducts(searchProducts, false, 'price', SortOrder.ASC, 0, 3);
+        const resultPage2 = await testSearchProducts(searchProducts, false, 'price', SortOrder.ASC, 3, 3);
+
+        const pvIds1 = resultPage1.search.items.map(i => i.productVariantId);
+        const pvIds2 = resultPage2.search.items.map(i => i.productVariantId);
+        const pvIds3 = pvIds1.concat(pvIds2);
+
+        expect(new Set(pvIds3).size).equals(6);
+        expect(resultPage1.search.items.map(i => i.productVariantId)).toEqual(['T_4', 'T_5', 'T_6']);
+        expect(resultPage2.search.items.map(i => i.productVariantId)).toEqual(['T_7', 'T_8', 'T_9']);
+    }
+
+    async function testSortByPriceDesc(searchProducts: SearchProducts) {
+        const resultPage1 = await testSearchProducts(searchProducts, false, 'price', SortOrder.DESC, 0, 3);
+        const resultPage2 = await testSearchProducts(searchProducts, false, 'price', SortOrder.DESC, 3, 3);
+
+        const pvIds1 = resultPage1.search.items.map(i => i.productVariantId);
+        const pvIds2 = resultPage2.search.items.map(i => i.productVariantId);
+        const pvIds3 = pvIds1.concat(pvIds2);
+
+        expect(new Set(pvIds3).size).equals(6);
+        expect(resultPage1.search.items.map(i => i.productVariantId)).toEqual(['T_1', 'T_2', 'T_3']);
+        expect(resultPage2.search.items.map(i => i.productVariantId)).toEqual(['T_4', 'T_5', 'T_6']);
+    }
+
+    async function testSortByPriceAscGroupByProduct(searchProducts: SearchProducts) {
+        const resultPage1 = await testSearchProducts(searchProducts, true, 'price', SortOrder.ASC, 0, 3);
+        const resultPage2 = await testSearchProducts(searchProducts, true, 'price', SortOrder.ASC, 3, 3);
+
+        const pvIds1 = resultPage1.search.items.map(i => i.productVariantId);
+        const pvIds2 = resultPage2.search.items.map(i => i.productVariantId);
+        const pvIds3 = pvIds1.concat(pvIds2);
+
+        expect(new Set(pvIds3).size).equals(6);
+        expect(resultPage1.search.items.map(i => i.productId)).toEqual(['T_4', 'T_5', 'T_6']);
+        expect(resultPage2.search.items.map(i => i.productId)).toEqual(['T_1', 'T_2', 'T_3']);
+    }
+
+    async function testSortByPriceDescGroupByProduct(searchProducts: SearchProducts) {
+        const resultPage1 = await testSearchProducts(searchProducts, true, 'price', SortOrder.DESC, 0, 3);
+        const resultPage2 = await testSearchProducts(searchProducts, true, 'price', SortOrder.DESC, 3, 3);
+
+        const pvIds1 = resultPage1.search.items.map(i => i.productVariantId);
+        const pvIds2 = resultPage2.search.items.map(i => i.productVariantId);
+        const pvIds3 = pvIds1.concat(pvIds2);
+
+        expect(new Set(pvIds3).size).equals(6);
+        expect(resultPage1.search.items.map(i => i.productId)).toEqual(['T_1', 'T_2', 'T_3']);
+        expect(resultPage2.search.items.map(i => i.productId)).toEqual(['T_4', 'T_5', 'T_6']);
+    }
+
+    describe('Search products shop', () => {
+        const searchProducts = searchProductsShop;
+
+        it('sort by price ASC', () => testSortByPriceAsc(searchProducts));
+        it('sort by price DESC', () => testSortByPriceDesc(searchProducts));
+
+        it('sort by price ASC group by product', () => testSortByPriceAscGroupByProduct(searchProducts));
+        it('sort by price DESC group by product', () => testSortByPriceDescGroupByProduct(searchProducts));
+    });
+
+    describe('Search products admin', () => {
+        const searchProducts = searchProductsAdmin;
+
+        it('sort by price ACS', () => testSortByPriceAsc(searchProducts));
+        it('sort by price DESC', () => testSortByPriceDesc(searchProducts));
+
+        it('sort by price ASC group by product', () => testSortByPriceAscGroupByProduct(searchProducts));
+        it('sort by price DESC group by product', () => testSortByPriceDescGroupByProduct(searchProducts));
+    });
+});

packages/core/e2e/default-search-plugin/fixtures/default-search-plugin-sort-by.csv

@@ -0,0 +1,16 @@
+name,slug,description,assets,facets,optionGroups,optionValues,sku,price,taxCategory,stockOnHand,trackInventory,variantAssets,variantFacets
+Boot A,boot-a,Boot A Size 40,,category:sports equipment,shoe size,Size 40,BA40,100,standard,100,true,,
+Boot B,boot-b,Boot B Size 40,,category:sports equipment,shoe size,Size 40,BB40,100,standard,100,true,,
+Boot C,boot-c,Boot C Size 40,,category:sports equipment,shoe size,Size 40,BC40,100,standard,100,true,,
+Sneaker A,sneaker-a,Sneaker A Size 40,,category:sports equipment,shoe size,Size 40,SA40,99.99,standard,100,true,,
+,,Sneaker A Size 41,,,,Size 41,SA41,99.99,standard,100,true,,
+,,Sneaker A Size 42,,,,Size 42,SA42,99.99,standard,100,true,,
+,,Sneaker A Size 43,,,,Size 43,SA43,99.99,standard,100,true,,
+Sneaker B,sneaker-b,Sneaker B Size 40,,category:sports equipment,shoe size,Size 40,SB40,99.99,standard,100,true,,
+,,Sneaker B Size 41,,,,Size 41,SB41,99.99,standard,100,true,,
+,,Sneaker B Size 42,,,,Size 42,SB42,99.99,standard,100,true,,
+,,Sneaker B Size 43,,,,Size 43,SB43,99.99,standard,100,true,,
+Sneaker C,sneaker-c,Sneaker C Size 40,,category:sports equipment,shoe size,Size 40,SC40,99.99,standard,100,true,,
+,,Sneaker C Size 41,,,,Size 41,SC41,99.99,standard,100,true,,
+,,Sneaker C Size 42,,,,Size 42,SC42,99.99,standard,100,true,,
+,,Sneaker C Size 43,,,,Size 43,SC43,99.99,standard,100,true,,

packages/core/e2e/graphql/admin-definitions.ts

@@ -0,0 +1,19 @@
+import gql from 'graphql-tag';
+
+export const SEARCH_PRODUCTS_ADMIN = gql`
+    query SearchProductsAdmin($input: SearchInput!) {
+        search(input: $input) {
+            totalItems
+            items {
+                enabled
+                productId
+                productName
+                slug
+                description
+                productVariantId
+                productVariantName
+                sku
+            }
+        }
+    }
+`;

packages/core/src/plugin/default-search-plugin/search-strategy/mysql-search-strategy.ts

@@ -95,19 +95,24 @@ export class MysqlSearchStrategy implements SearchStrategy {
                 .addSelect('MIN(si.priceWithTax)', 'minPriceWithTax')
                 .addSelect('MAX(si.priceWithTax)', 'maxPriceWithTax');
         }
+
         this.applyTermAndFilters(ctx, qb, input);
+
         if (sort) {
             if (sort.name) {
-                qb.addOrderBy(input.groupByProduct ? 'MIN(si.productName)' : 'si.productName', sort.name);
+                qb.addOrderBy('si_productName', sort.name);
             }
             if (sort.price) {
-                qb.addOrderBy(input.groupByProduct ? 'MIN(si.price)' : 'si.price', sort.price);
-            }
-        } else {
-            if (input.term && input.term.length > this.minTermLength) {
-                qb.orderBy('score', 'DESC');
+                qb.addOrderBy('si_price', sort.price);
             }
+        } else if (input.term && input.term.length > this.minTermLength) {
+            qb.addOrderBy('score', 'DESC');
         }
+
+        // Required to ensure deterministic sorting.
+        // E.g. in case of sorting products with duplicate name, price or score results.
+        qb.addOrderBy('si_productVariantId', 'ASC');
+
         if (enabledOnly) {
             qb.andWhere('si.enabled = :enabled', { enabled: true });
         }

packages/core/src/plugin/default-search-plugin/search-strategy/postgres-search-strategy.ts

@@ -95,6 +95,7 @@ export class PostgresSearchStrategy implements SearchStrategy {
                 .addSelect('MIN(si.priceWithTax)', 'minPriceWithTax')
                 .addSelect('MAX(si.priceWithTax)', 'maxPriceWithTax');
         }
+
         this.applyTermAndFilters(ctx, qb, input);
 
         if (sort) {
@@ -104,13 +105,14 @@ export class PostgresSearchStrategy implements SearchStrategy {
             if (sort.price) {
                 qb.addOrderBy('"si_price"', sort.price);
             }
-        } else {
-            if (input.term && input.term.length > this.minTermLength) {
-                qb.addOrderBy('score', 'DESC');
-            } else {
-                qb.addOrderBy('"si_productVariantId"', 'ASC');
-            }
+        } else if (input.term && input.term.length > this.minTermLength) {
+            qb.addOrderBy('score', 'DESC');
         }
+
+        // Required to ensure deterministic sorting.
+        // E.g. in case of sorting products with duplicate name, price or score results.
+        qb.addOrderBy('"si_productVariantId"', 'ASC');
+
         if (enabledOnly) {
             qb.andWhere('"si"."enabled" = :enabled', { enabled: true });
         }

packages/core/src/plugin/default-search-plugin/search-strategy/sqlite-search-strategy.ts

@@ -87,16 +87,14 @@ export class SqliteSearchStrategy implements SearchStrategy {
         const sort = input.sort;
         const qb = this.connection.getRepository(ctx, SearchIndexItem).createQueryBuilder('si');
         if (input.groupByProduct) {
-            qb.addSelect('MIN(si.price)', 'minPrice').addSelect('MAX(si.price)', 'maxPrice');
-            qb.addSelect('MIN(si.priceWithTax)', 'minPriceWithTax').addSelect(
-                'MAX(si.priceWithTax)',
-                'maxPriceWithTax',
-            );
+            qb.addSelect('MIN(si.price)', 'minPrice');
+            qb.addSelect('MAX(si.price)', 'maxPrice');
+            qb.addSelect('MIN(si.priceWithTax)', 'minPriceWithTax');
+            qb.addSelect('MAX(si.priceWithTax)', 'maxPriceWithTax');
         }
+
         this.applyTermAndFilters(ctx, qb, input);
-        if (input.term && input.term.length > this.minTermLength) {
-            qb.orderBy('score', 'DESC');
-        }
+
         if (sort) {
             if (sort.name) {
                 // TODO: v3 - set the collation on the SearchIndexItem entity
@@ -105,9 +103,14 @@ export class SqliteSearchStrategy implements SearchStrategy {
             if (sort.price) {
                 qb.addOrderBy('si.price', sort.price);
             }
-        } else {
-            qb.addOrderBy('si.productVariantId', 'ASC');
+        } else if (input.term && input.term.length > this.minTermLength) {
+            qb.addOrderBy('score', 'DESC');
         }
+
+        // Required to ensure deterministic sorting.
+        // E.g. in case of sorting products with duplicate name, price or score results.
+        qb.addOrderBy('si.productVariantId', 'ASC');
+
         if (enabledOnly) {
             qb.andWhere('si.enabled = :enabled', { enabled: true });
         }

packages/dev-server/.gitignore

@@ -11,3 +11,4 @@ dev-config-override.ts
 vendure.log
 scripts/dev-test
 custom-admin-ui
+docker-compose.*.yml