Browse Source

fix(server): Rework reindexing of search index

Instead of relying on the (unreliable) "is the index empty?" heuristic, we now explicitly call the reindex() method after populating the collection.
Michael Bromley 6 years ago
parent
commit
d7be976548

+ 5 - 1
server/src/data-import/data-import.module.ts

@@ -1,6 +1,7 @@
 import { Module } from '@nestjs/common';
 
 import { ConfigModule } from '../config/config.module';
+import { PluginModule } from '../plugin/plugin.module';
 import { ServiceModule } from '../service/service.module';
 
 import { ImportParser } from './providers/import-parser/import-parser';
@@ -8,7 +9,10 @@ import { Importer } from './providers/importer/importer';
 import { Populator } from './providers/populator/populator';
 
 @Module({
-    imports: [ServiceModule, ConfigModule],
+    // Important! PluginModule must be defined before ServiceModule
+    // in order that overrides of Services (e.g. SearchService) are correctly
+    // registered with the injector.
+    imports: [PluginModule, ServiceModule, ConfigModule],
     exports: [ImportParser, Importer, Populator],
     providers: [ImportParser, Importer, Populator],
 })

+ 6 - 0
server/src/data-import/providers/populator/populator.ts

@@ -11,6 +11,7 @@ import { Zone } from '../../../entity/zone/zone.entity';
 import { CollectionService, FacetValueService, ShippingMethodService } from '../../../service';
 import { ChannelService } from '../../../service/services/channel.service';
 import { CountryService } from '../../../service/services/country.service';
+import { SearchService } from '../../../service/services/search.service';
 import { TaxCategoryService } from '../../../service/services/tax-category.service';
 import { TaxRateService } from '../../../service/services/tax-rate.service';
 import { ZoneService } from '../../../service/services/zone.service';
@@ -46,6 +47,7 @@ export class Populator {
         private shippingMethodService: ShippingMethodService,
         private collectionService: CollectionService,
         private facetValueService: FacetValueService,
+        private searchService: SearchService,
     ) {}
 
     /**
@@ -101,6 +103,10 @@ export class Populator {
             });
             collectionMap.set(collectionDef.name, collection);
         }
+        // Wait for the created collection operations to complete before running
+        // the reindex of the search index.
+        await new Promise(resolve => setTimeout(resolve, 50));
+        await this.searchService.reindex(ctx.languageCode);
     }
 
     private async createRequestContext(data: InitialData) {

+ 22 - 8
server/src/plugin/default-search-plugin/default-search-plugin.ts

@@ -1,23 +1,37 @@
+import { Provider } from '@nestjs/common';
 import gql from 'graphql-tag';
 
 import { SearchReindexResponse } from '../../../../shared/generated-types';
 import { Type } from '../../../../shared/shared-types';
-import { DEFAULT_LANGUAGE_CODE } from '../../common/constants';
-import { APIExtensionDefinition, InjectorFn, VendurePlugin } from '../../config';
+import { APIExtensionDefinition, VendurePlugin } from '../../config';
+import { ProductVariant } from '../../entity/product-variant/product-variant.entity';
+import { Product } from '../../entity/product/product.entity';
+import { EventBus } from '../../event-bus/event-bus';
+import { CatalogModificationEvent } from '../../event-bus/events/catalog-modification-event';
+import { CollectionModificationEvent } from '../../event-bus/events/collection-modification-event';
+import { SearchService } from '../../service/services/search.service';
 
 import { AdminFulltextSearchResolver, ShopFulltextSearchResolver } from './fulltext-search.resolver';
 import { FulltextSearchService } from './fulltext-search.service';
 import { SearchIndexItem } from './search-index-item.entity';
 
-export interface DefaultSearceReindexResonse extends SearchReindexResponse {
+export interface DefaultSearchReindexResponse extends SearchReindexResponse {
     timeTaken: number;
     indexedItemCount: number;
 }
 
 export class DefaultSearchPlugin implements VendurePlugin {
-    async onBootstrap(inject: InjectorFn): Promise<void> {
-        const searchService = inject(FulltextSearchService);
-        await searchService.checkIndex(DEFAULT_LANGUAGE_CODE);
+    onBootstrap(inject: <T>(type: Type<T>) => T): void | Promise<void> {
+        const eventBus = inject(EventBus);
+        const fulltextSearchService = inject(FulltextSearchService);
+        eventBus.subscribe(CatalogModificationEvent, event => {
+            if (event.entity instanceof Product || event.entity instanceof ProductVariant) {
+                return fulltextSearchService.updateProductOrVariant(event.ctx, event.entity);
+            }
+        });
+        eventBus.subscribe(CollectionModificationEvent, event => {
+            return fulltextSearchService.updateVariantsById(event.ctx, event.productVariantIds);
+        });
     }
 
     extendAdminAPI(): APIExtensionDefinition {
@@ -48,7 +62,7 @@ export class DefaultSearchPlugin implements VendurePlugin {
         return [SearchIndexItem];
     }
 
-    defineProviders(): Array<Type<any>> {
-        return [FulltextSearchService];
+    defineProviders(): Provider[] {
+        return [FulltextSearchService, { provide: SearchService, useClass: FulltextSearchService }];
     }
 }

+ 2 - 2
server/src/plugin/default-search-plugin/fulltext-search.resolver.ts

@@ -10,7 +10,7 @@ import { SearchResolver as BaseSearchResolver } from '../../api/resolvers/admin/
 import { Translated } from '../../common/types/locale-types';
 import { FacetValue } from '../../entity';
 
-import { DefaultSearceReindexResonse } from './default-search-plugin';
+import { DefaultSearchReindexResponse } from './default-search-plugin';
 import { FulltextSearchService } from './fulltext-search.service';
 
 @Resolver('SearchResponse')
@@ -60,7 +60,7 @@ export class AdminFulltextSearchResolver implements BaseSearchResolver {
 
     @Mutation()
     @Allow(Permission.UpdateCatalog)
-    async reindex(@Ctx() ctx: RequestContext): Promise<DefaultSearceReindexResonse> {
+    async reindex(@Ctx() ctx: RequestContext): Promise<DefaultSearchReindexResponse> {
         return this.fulltextSearchService.reindex(ctx.languageCode);
     }
 }

+ 4 - 28
server/src/plugin/default-search-plugin/fulltext-search.service.ts

@@ -12,12 +12,11 @@ import { InternalServerError } from '../../common/error/errors';
 import { Translated } from '../../common/types/locale-types';
 import { FacetValue, Product, ProductVariant } from '../../entity';
 import { EventBus } from '../../event-bus/event-bus';
-import { CatalogModificationEvent } from '../../event-bus/events/catalog-modification-event';
-import { CollectionModificationEvent } from '../../event-bus/events/collection-modification-event';
 import { translateDeep } from '../../service/helpers/utils/translate-entity';
 import { FacetValueService } from '../../service/services/facet-value.service';
+import { SearchService } from '../../service/services/search.service';
 
-import { DefaultSearceReindexResonse } from './default-search-plugin';
+import { DefaultSearchReindexResponse } from './default-search-plugin';
 import { SearchIndexItem } from './search-index-item.entity';
 import { MysqlSearchStrategy } from './search-strategy/mysql-search-strategy';
 import { PostgresSearchStrategy } from './search-strategy/postgres-search-strategy';
@@ -29,7 +28,7 @@ import { SqliteSearchStrategy } from './search-strategy/sqlite-search-strategy';
  * SearchStrategy implementations for db-specific code.
  */
 @Injectable()
-export class FulltextSearchService {
+export class FulltextSearchService implements SearchService {
     private searchStrategy: SearchStrategy;
     private readonly minTermLength = 2;
     private readonly variantRelations = [
@@ -48,15 +47,6 @@ export class FulltextSearchService {
         private eventBus: EventBus,
         private facetValueService: FacetValueService,
     ) {
-        eventBus.subscribe(CatalogModificationEvent, event => {
-            if (event.entity instanceof Product || event.entity instanceof ProductVariant) {
-                return this.updateProductOrVariant(event.ctx, event.entity);
-            }
-        });
-        eventBus.subscribe(CollectionModificationEvent, event => {
-            return this.updateVariantsById(event.ctx, event.productVariantIds);
-        });
-
         this.setSearchStrategy();
     }
 
@@ -83,7 +73,7 @@ export class FulltextSearchService {
     /**
      * Rebuilds the full search index.
      */
-    async reindex(languageCode: LanguageCode): Promise<DefaultSearceReindexResonse> {
+    async reindex(languageCode: LanguageCode): Promise<DefaultSearchReindexResponse> {
         const timeStart = Date.now();
         const qb = await this.connection.getRepository(ProductVariant).createQueryBuilder('variants');
         FindOptionsUtils.applyFindManyOptionsOrConditionsToQueryBuilder(qb, {
@@ -146,20 +136,6 @@ export class FulltextSearchService {
         }
     }
 
-    /**
-     * Checks to see if the index is empty, and if so triggers a re-index operation.
-     */
-    async checkIndex(languageCode: LanguageCode) {
-        const indexSize = await this.connection.getRepository(SearchIndexItem).count({
-            where: {
-                languageCode,
-            },
-        });
-        if (indexSize === 0) {
-            await this.reindex(languageCode);
-        }
-    }
-
     /**
      * Sets the SearchStrategy appropriate to th configured database type.
      */

+ 2 - 0
server/src/service/service.module.ts

@@ -34,6 +34,7 @@ import { ProductVariantService } from './services/product-variant.service';
 import { ProductService } from './services/product.service';
 import { PromotionService } from './services/promotion.service';
 import { RoleService } from './services/role.service';
+import { SearchService } from './services/search.service';
 import { ShippingMethodService } from './services/shipping-method.service';
 import { TaxCategoryService } from './services/tax-category.service';
 import { TaxRateService } from './services/tax-rate.service';
@@ -61,6 +62,7 @@ const exportedProviders = [
     ProductVariantService,
     RoleService,
     ShippingMethodService,
+    SearchService,
     TaxCategoryService,
     TaxRateService,
     UserService,

+ 27 - 0
server/src/service/services/search.service.ts

@@ -0,0 +1,27 @@
+import { Injectable } from '@nestjs/common';
+
+import { LanguageCode, SearchReindexResponse } from '../../../../shared/generated-types';
+
+/**
+ * This service should be overridden by a VendurePlugin which implements search.
+ *
+ * ```
+ * defineProviders(): Provider[] {
+ *     return [
+ *         { provide: SearchService, useClass: MySearchService },
+ *     ];
+ * }
+ * ```
+ */
+@Injectable()
+export class SearchService {
+    async reindex(languageCode: LanguageCode): Promise<SearchReindexResponse> {
+        // tslint:disable-next-line:no-console
+        console.error(`The SearchService should be overridden by an appropriate search plugin.`);
+        return {
+            indexedItemCount: 0,
+            success: false,
+            timeTaken: 0,
+        };
+    }
+}