fix(core): Fix DefaultSearchPlugin for non-default languages (#2515)

Fixes #2197
vendure-ecommerce · Nov 22, 2023 · fb0ea13 · fb0ea13
1 parent 5775447
commit fb0ea13
Show file tree

Hide file tree

Showing 14 changed files with 634 additions and 377 deletions.
diff --git a/e2e-common/vitest.config.ts b/e2e-common/vitest.config.ts
@@ -4,7 +4,7 @@ import { defineConfig } from 'vitest/config';
 
 export default defineConfig({
     test: {
-        include: '**/*.e2e-spec.ts',
+        include: ['**/*.e2e-spec.ts'],
         /**
          * For local debugging of the e2e tests, we set a very long timeout value otherwise tests will
          * automatically fail for going over the 5 second default timeout.

diff --git a/packages/core/e2e/default-search-plugin.e2e-spec.ts b/packages/core/e2e/default-search-plugin.e2e-spec.ts
diff --git a/packages/core/e2e/utils/await-running-jobs.ts b/packages/core/e2e/utils/await-running-jobs.ts
@@ -1,7 +1,6 @@
 import { SimpleGraphQLClient } from '@vendure/testing';
-import { afterAll, beforeAll, describe, expect, it } from 'vitest';
 
-import { GetRunningJobs, JobState } from '../graphql/generated-e2e-admin-types';
+import { GetRunningJobsQuery, GetRunningJobsQueryVariables } from '../graphql/generated-e2e-admin-types';
 import { GET_RUNNING_JOBS } from '../graphql/shared-definitions';
 
 /**
@@ -20,18 +19,18 @@ export async function awaitRunningJobs(
     // e.g. event debouncing is used before triggering the job.
     await new Promise(resolve => setTimeout(resolve, delay));
     do {
-        const { jobs } = await adminClient.query<
-            Codegen.GetRunningJobsQuery,
-            Codegen.GetRunningJobsQueryVariables
-        >(GET_RUNNING_JOBS, {
-            options: {
-                filter: {
-                    isSettled: {
-                        eq: false,
+        const { jobs } = await adminClient.query<GetRunningJobsQuery, GetRunningJobsQueryVariables>(
+            GET_RUNNING_JOBS,
+            {
+                options: {
+                    filter: {
+                        isSettled: {
+                            eq: false,
+                        },
                     },
                 },
             },
-        });
+        );
         runningJobs = jobs.totalItems;
         timedOut = timeout < +new Date() - startTime;
     } while (runningJobs > 0 && !timedOut);

diff --git a/packages/core/src/plugin/default-search-plugin/search-strategy/mysql-search-strategy.ts b/packages/core/src/plugin/default-search-plugin/search-strategy/mysql-search-strategy.ts
@@ -249,8 +249,9 @@ export class MysqlSearchStrategy implements SearchStrategy {
             qb.andWhere('FIND_IN_SET (:collectionSlug, si.collectionSlugs)', { collectionSlug });
         }
 
-        applyLanguageConstraints(qb, ctx.languageCode, ctx.channel.defaultLanguageCode);
         qb.andWhere('si.channelId = :channelId', { channelId: ctx.channelId });
+        applyLanguageConstraints(qb, ctx.languageCode, ctx.channel.defaultLanguageCode);
+
         if (input.groupByProduct === true) {
             qb.groupBy('si.productId');
             qb.addSelect('BIT_OR(si.enabled)', 'productEnabled');

diff --git a/packages/core/src/plugin/default-search-plugin/search-strategy/postgres-search-strategy.ts b/packages/core/src/plugin/default-search-plugin/search-strategy/postgres-search-strategy.ts
@@ -247,11 +247,13 @@ export class PostgresSearchStrategy implements SearchStrategy {
             });
         }
 
-        applyLanguageConstraints(qb, ctx.languageCode, ctx.channel.defaultLanguageCode);
         qb.andWhere('si.channelId = :channelId', { channelId: ctx.channelId });
+        applyLanguageConstraints(qb, ctx.languageCode, ctx.channel.defaultLanguageCode);
+
         if (input.groupByProduct === true) {
             qb.groupBy('si.productId');
         }
+
         return qb;
     }
 

diff --git a/packages/core/src/plugin/default-search-plugin/search-strategy/search-strategy-common.ts b/packages/core/src/plugin/default-search-plugin/search-strategy/search-strategy-common.ts
@@ -22,14 +22,6 @@ export const fieldsToSelect = [
     'productVariantPreviewFocalPoint',
 ];
 
-export const identifierFields = [
-    'channelId',
-    'productVariantId',
-    'productId',
-    'productAssetId',
-    'productVariantAssetId',
-];
-
 export function getFieldsToSelect(includeStockStatus: boolean = false) {
     return includeStockStatus ? [...fieldsToSelect, 'inStock', 'productInStock'] : fieldsToSelect;
 }
diff --git a/packages/core/src/plugin/default-search-plugin/search-strategy/search-strategy-utils.ts b/packages/core/src/plugin/default-search-plugin/search-strategy/search-strategy-utils.ts
@@ -9,12 +9,10 @@ import {
 } from '@vendure/common/lib/generated-types';
 import { ID } from '@vendure/common/lib/shared-types';
 import { unique } from '@vendure/common/lib/unique';
-import { QueryBuilder, SelectQueryBuilder } from 'typeorm';
+import { Brackets, QueryBuilder, SelectQueryBuilder } from 'typeorm';
 
 import { SearchIndexItem } from '../entities/search-index-item.entity';
 
-import { identifierFields } from './search-strategy-common';
-
 /**
  * Maps a raw database result to a SearchResult.
  */
@@ -131,31 +129,34 @@ export function applyLanguageConstraints(
     defaultLanguageCode: LanguageCode,
 ) {
     const lcEscaped = qb.escape('languageCode');
+    const ciEscaped = qb.escape('channelId');
+    const pviEscaped = qb.escape('productVariantId');
+
     if (languageCode === defaultLanguageCode) {
-        qb.andWhere(`si.${lcEscaped} = :languageCode`, { languageCode });
+        qb.andWhere(`si.${lcEscaped} = :languageCode`, {
+            languageCode,
+        });
     } else {
         qb.andWhere(`si.${lcEscaped} IN (:...languageCodes)`, {
             languageCodes: [languageCode, defaultLanguageCode],
         });
 
-        const joinFieldConditions = identifierFields
-            .map(field => `si.${qb.escape(field)} = sil.${qb.escape(field)}`)
-            .join(' AND ');
-
         qb.leftJoin(
             SearchIndexItem,
             'sil',
-            `
-            ${joinFieldConditions}
-            AND si.${lcEscaped} != sil.${lcEscaped}
-            AND sil.${lcEscaped} = :languageCode
-        `,
+            `sil.${lcEscaped} = :languageCode AND sil.${ciEscaped} = si.${ciEscaped} AND sil.${pviEscaped} = si.${pviEscaped}`,
             {
                 languageCode,
             },
         );
 
-        qb.andWhere(`sil.${lcEscaped} IS NULL`);
+        qb.andWhere(
+            new Brackets(qb1 => {
+                qb1.where(`si.${lcEscaped} = :languageCode1`, {
+                    languageCode1: languageCode,
+                }).orWhere(`sil.${lcEscaped} IS NULL`);
+            }),
+        );
     }
 
     return qb;

diff --git a/packages/core/src/plugin/default-search-plugin/search-strategy/sqlite-search-strategy.ts b/packages/core/src/plugin/default-search-plugin/search-strategy/sqlite-search-strategy.ts
@@ -99,7 +99,8 @@ export class SqliteSearchStrategy implements SearchStrategy {
         }
         if (sort) {
             if (sort.name) {
-                qb.addOrderBy('si.productName', sort.name);
+                // TODO: v3 - set the collation on the SearchIndexItem entity
+                qb.addOrderBy('si.productName COLLATE NOCASE', sort.name);
             }
             if (sort.price) {
                 qb.addOrderBy('si.price', sort.price);
@@ -230,8 +231,8 @@ export class SqliteSearchStrategy implements SearchStrategy {
             });
         }
 
-        applyLanguageConstraints(qb, ctx.languageCode, ctx.channel.defaultLanguageCode);
         qb.andWhere('si.channelId = :channelId', { channelId: ctx.channelId });
+        applyLanguageConstraints(qb, ctx.languageCode, ctx.channel.defaultLanguageCode);
 
         if (input.groupByProduct === true) {
             qb.groupBy('si.productId');

diff --git a/packages/dev-server/load-testing/graphql/shop/complete-order.graphql b/packages/dev-server/load-testing/graphql/shop/complete-order.graphql
@@ -1,4 +1,4 @@
-mutation SetShippingMethod($id: ID!) {
+mutation SetShippingMethod($id: [ID!]!) {
     setOrderShippingMethod(shippingMethodId: $id) {
         ...on Order {
             code

diff --git a/packages/dev-server/load-testing/init-load-test.ts b/packages/dev-server/load-testing/init-load-test.ts
@@ -2,7 +2,7 @@
 /// <reference path="../../core/typings.d.ts" />
 import { bootstrap, JobQueueService, Logger } from '@vendure/core';
 import { populate } from '@vendure/core/cli/populate';
-import { clearAllTables, populateCustomers } from '@vendure/testing';
+import { clearAllTables, populateCustomers, SimpleGraphQLClient } from '@vendure/testing';
 import stringify from 'csv-stringify';
 import fs from 'fs';
 import path from 'path';
@@ -17,6 +17,8 @@ import {
     getProductCsvFilePath,
 } from './load-test-config';
 
+import { awaitRunningJobs } from '../../core/e2e/utils/await-running-jobs';
+
 /* eslint-disable no-console */
 
 /**
@@ -49,6 +51,18 @@ if (require.main === module) {
                             csvFile,
                         ),
                     )
+                    .then(async app => {
+                        console.log('synchronize on search index updated...');
+                        const { port, adminApiPath, shopApiPath } = config.apiOptions;
+                        const adminClient = new SimpleGraphQLClient(
+                            config,
+                            `http://localhost:${port}/${adminApiPath!}`,
+                        );
+                        await adminClient.asSuperAdmin();
+                        await new Promise(resolve => setTimeout(resolve, 5000));
+                        await awaitRunningJobs(adminClient, 5000000);
+                        return app;
+                    })
                     .then(async app => {
                         console.log('populating customers...');
                         await populateCustomers(app, 10, message => Logger.error(message));

diff --git a/packages/dev-server/load-testing/load-test-config.ts b/packages/dev-server/load-testing/load-test-config.ts
@@ -72,7 +72,7 @@ export function getLoadTestConfig(
                 assetUploadDir: path.join(__dirname, 'static/assets'),
                 route: 'assets',
             }),
-            DefaultSearchPlugin,
+            DefaultSearchPlugin.init({ bufferUpdates: false, indexStockStatus: false }),
             DefaultJobQueuePlugin.init({
                 pollInterval: 1000,
             }),

diff --git a/packages/dev-server/load-testing/run-load-test.ts b/packages/dev-server/load-testing/run-load-test.ts
@@ -26,7 +26,7 @@ if (require.main === module) {
         stdio: 'inherit',
     });
 
-    init.on('exit', code => {
+    init.on('exit', async code => {
         if (code === 0) {
             const databaseName = `vendure-load-testing-${count}`;
             return bootstrap(getLoadTestConfig('cookie', databaseName))
@@ -49,7 +49,7 @@ if (require.main === module) {
     });
 }
 
-function runLoadTestScript(script: string): Promise<LoadTestSummary> {
+async function runLoadTestScript(script: string): Promise<LoadTestSummary> {
     const rawResultsFile = `${script}.${count}.json`;
 
     return new Promise((resolve, reject) => {

diff --git a/packages/dev-server/load-testing/scripts/search-and-checkout.js b/packages/dev-server/load-testing/scripts/search-and-checkout.js
@@ -26,10 +26,12 @@ export default function () {
         addToCart(randomItem(product.variants).id);
     }
     setShippingAddressAndCustomer();
-    const data = getShippingMethodsQuery.post().data;
-    const result = completeOrderMutation.post({ id: data.eligibleShippingMethods[0].id }).data;
-    check(result, {
-        'Order completed': r => r.addPaymentToOrder.state === 'PaymentAuthorized',
+    const { data: shippingMethods } = getShippingMethodsQuery.post();
+    const { data: order } = completeOrderMutation.post({
+        id: [shippingMethods.eligibleShippingMethods.at(0).id],
+    });
+    check(order, {
+        'Order completed': o => o.addPaymentToOrder.state === 'PaymentAuthorized',
     });
 }
 

diff --git a/packages/dev-server/load-testing/utils/api-request.js b/packages/dev-server/load-testing/utils/api-request.js
@@ -17,13 +17,16 @@ export class ApiRequest {
     post(variables = {}, authToken) {
         const res = http.post(
             this.apiUrl,
-            {
+            JSON.stringify({
                 query: this.document,
-                variables: JSON.stringify(variables),
-            },
+                variables,
+            }),
             {
                 timeout: 120 * 1000,
-                headers: { Authorization: authToken ? `Bearer ${authToken}` : undefined },
+                headers: {
+                    Authorization: authToken ? `Bearer ${authToken}` : '',
+                    'Content-Type': 'application/json',
+                },
             },
         );
         check(res, {