From 18646ccb1ad767d8c0225dbe50ddf5b510020373 Mon Sep 17 00:00:00 2001
From: Eric Peterson <ericpeterson@spotify.com>
Date: Tue, 29 Nov 2022 15:12:23 +0100
Subject: [PATCH] Update lunr engine to preserve pre-existing indices if
 indexer receives 0 documents

Signed-off-by: Eric Peterson <ericpeterson@spotify.com>
---
 .../src/engines/LunrSearchEngine.test.ts      | 56 ++++++++++++++++++-
 .../src/engines/LunrSearchEngine.ts           | 29 +++++++++-
 2 files changed, 82 insertions(+), 3 deletions(-)

diff --git a/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts b/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts
index 4ef380943f..27cd65d63b 100644
--- a/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts
+++ b/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts
@@ -1026,7 +1026,7 @@ describe('LunrSearchEngine', () => {
 
       // Get the indexer and invoke its close handler.
       await inspectableSearchEngine.getIndexer('test-index');
-      const onClose = indexerMock.on.mock.calls[0][1] as Function;
+      const onClose = indexerMock.on.mock.calls[1][1] as Function;
       onClose();
 
       // Ensure mocked methods were called.
@@ -1044,6 +1044,60 @@ describe('LunrSearchEngine', () => {
         'new-location': doc,
       });
     });
+
+    it('should not replace index or docs if no docs were indexed', async () => {
+      // Set up an inspectable search engine to pre-set some data.
+      const doc = { title: 'A doc', text: 'test', location: 'some-location' };
+      const inspectableSearchEngine = new LunrSearchEngineForTests({
+        logger: getVoidLogger(),
+      });
+      inspectableSearchEngine.setDocStore({ 'existing-location': doc });
+
+      // Mock methods called by close handler (resolving no documents)
+      indexerMock.buildIndex.mockReturnValueOnce('expected-index');
+      indexerMock.getDocumentStore.mockReturnValueOnce({});
+
+      // Get the indexer and invoke its close handler.
+      await inspectableSearchEngine.getIndexer('test-index');
+      const onClose = indexerMock.on.mock.calls[1][1] as Function;
+      onClose();
+
+      // Ensure buildIndex method was not called.
+      expect(indexerMock.buildIndex).not.toHaveBeenCalled();
+
+      // Ensure pre-existing documents still exist in the store
+      expect(inspectableSearchEngine.getDocStore()).toStrictEqual({
+        'existing-location': doc,
+      });
+    });
+
+    it('should not replace index or docs if an error was thrown', async () => {
+      // Set up an inspectable search engine to pre-set some data.
+      const doc = { title: 'A doc', text: 'test', location: 'some-location' };
+      const inspectableSearchEngine = new LunrSearchEngineForTests({
+        logger: getVoidLogger(),
+      });
+      inspectableSearchEngine.setDocStore({ 'existing-location': doc });
+
+      // Mock methods called by close handler (resolving no documents)
+      indexerMock.buildIndex.mockReturnValueOnce('expected-index');
+      indexerMock.getDocumentStore.mockReturnValueOnce({});
+
+      // Get the indexer and invoke its close handler after firing an error.
+      await inspectableSearchEngine.getIndexer('test-index');
+      const onError = indexerMock.on.mock.calls[0][1] as Function;
+      const onClose = indexerMock.on.mock.calls[1][1] as Function;
+      onError(new Error('Some collator error'));
+      onClose();
+
+      // Ensure buildIndex method was not called.
+      expect(indexerMock.buildIndex).not.toHaveBeenCalled();
+
+      // Ensure pre-existing documents still exist in the store
+      expect(inspectableSearchEngine.getDocStore()).toStrictEqual({
+        'existing-location': doc,
+      });
+    });
   });
 });
 
diff --git a/plugins/search-backend-node/src/engines/LunrSearchEngine.ts b/plugins/search-backend-node/src/engines/LunrSearchEngine.ts
index ff2c828402..b69b5ea863 100644
--- a/plugins/search-backend-node/src/engines/LunrSearchEngine.ts
+++ b/plugins/search-backend-node/src/engines/LunrSearchEngine.ts
@@ -153,12 +153,37 @@ export class LunrSearchEngine implements SearchEngine {
 
   async getIndexer(type: string) {
     const indexer = new LunrSearchEngineIndexer();
+    const indexerLogger = this.logger.child({ documentType: type });
+    let errorThrown: Error | undefined;
+
+    indexer.on('error', err => {
+      errorThrown = err;
+    });
 
     indexer.on('close', () => {
       // Once the stream is closed, build the index and store the documents in
       // memory for later retrieval.
-      this.lunrIndices[type] = indexer.buildIndex();
-      this.docStore = { ...this.docStore, ...indexer.getDocumentStore() };
+      const newDocuments = indexer.getDocumentStore();
+      const docStoreExists = this.lunrIndices[type] !== undefined;
+      const documentsIndexed = Object.keys(newDocuments).length;
+
+      // Do not set the index if there was an error or if no documents were
+      // indexed. This ensures search continues to work for an index, even in
+      // case of transient issues in underlying collators.
+      if (!errorThrown && documentsIndexed > 0) {
+        this.lunrIndices[type] = indexer.buildIndex();
+        this.docStore = { ...this.docStore, ...newDocuments };
+      } else {
+        indexerLogger.warn(
+          `Index for ${type} was not ${
+            docStoreExists ? 'replaced' : 'created'
+          }: ${
+            errorThrown
+              ? 'an error was encountered'
+              : 'indexer received 0 documents'
+          }`,
+        );
+      }
     });
 
     return indexer;