From 527524bc5baa8bef51ef7b6b4754ff22db37eec6 Mon Sep 17 00:00:00 2001 From: Mengwei Ding Date: Tue, 9 Jul 2019 10:16:36 -0700 Subject: [PATCH] [Code] always read file content from the original bare repo in indexer (#40176) (#40648) * [Code] reenable code mocha tests * skip multi node test * [Code] read file content from the original bare repo if fails to open workspace in indexr * More refactoring * Add more unit tests * Add unit tests * always read file content from the original bare repo * remove catching the workspace prepartion errors in indexer --- x-pack/legacy/plugins/code/model/search.ts | 1 - .../__tests__/lsp_incremental_indexer.ts | 32 +++---- .../code/server/__tests__/lsp_indexer.ts | 2 +- .../server/indexer/lsp_incremental_indexer.ts | 67 ++++----------- .../code/server/indexer/lsp_indexer.ts | 85 +++++++++++-------- 5 files changed, 84 insertions(+), 103 deletions(-) diff --git a/x-pack/legacy/plugins/code/model/search.ts b/x-pack/legacy/plugins/code/model/search.ts index 3c042915b8a56c..9c6d21eaad487f 100644 --- a/x-pack/legacy/plugins/code/model/search.ts +++ b/x-pack/legacy/plugins/code/model/search.ts @@ -27,7 +27,6 @@ export interface IndexRequest { // The request for LspIndexer export interface LspIndexRequest extends IndexRequest { - localRepoPath: string; // The repository local file path filePath: string; // The file path within the repository revision: string; // The revision of the current repository } diff --git a/x-pack/legacy/plugins/code/server/__tests__/lsp_incremental_indexer.ts b/x-pack/legacy/plugins/code/server/__tests__/lsp_incremental_indexer.ts index a39b32e5e581b8..fc8f6378289e49 100644 --- a/x-pack/legacy/plugins/code/server/__tests__/lsp_incremental_indexer.ts +++ b/x-pack/legacy/plugins/code/server/__tests__/lsp_incremental_indexer.ts @@ -39,6 +39,7 @@ const esClient = { function prepareProject(url: string, p: string) { const opts: CloneOptions = { + bare: 1, fetchOpts: { callbacks: { certificateCheck: () => 0, @@ -179,7 +180,7 @@ describe('lsp_incremental_indexer unit tests', () => { const indexer = new LspIncrementalIndexer( 'github.com/elastic/TypeScript-Node-Starter', 'HEAD', - '6206f643', + '67002808', lspservice, serverOptions, gitOps, @@ -193,19 +194,20 @@ describe('lsp_incremental_indexer unit tests', () => { assert.strictEqual(createSpy.callCount, 0); assert.strictEqual(putAliasSpy.callCount, 0); - // DeletebyQuery is called 8 times (1 file + 1 symbol reuqests per diff item) - // for 4 MODIFIED items - assert.strictEqual(deleteByQuerySpy.callCount, 8); + // DeletebyQuery is called 10 times (1 file + 1 symbol reuqests per diff item) + // for 5 MODIFIED items + assert.strictEqual(deleteByQuerySpy.callCount, 10); - // There are 4 MODIFIED items and 1 ADDED item. 1 file + 1 symbol + 1 reference - // = 3 objects to index for each item. Total doc indexed should be 5 * 3 = 15, + // There are 5 MODIFIED items and 1 ADDED item. Only 1 file is in supported + // language. Each file with supported language has 1 file + 1 symbol + 1 reference. + // Total doc indexed should be 8 * 3 = 15, // which can be fitted into a single batch index. assert.strictEqual(bulkSpy.callCount, 2); let total = 0; for (let i = 0; i < bulkSpy.callCount; i++) { total += bulkSpy.getCall(i).args[0].body.length; } - assert.strictEqual(total, 15 * 2); + assert.strictEqual(total, 8 * 2); // @ts-ignore }).timeout(20000); @@ -235,7 +237,7 @@ describe('lsp_incremental_indexer unit tests', () => { const indexer = new LspIncrementalIndexer( 'github.com/elastic/TypeScript-Node-Starter', 'HEAD', - '6206f643', + '67002808', lspservice, serverOptions, gitOps, @@ -282,7 +284,7 @@ describe('lsp_incremental_indexer unit tests', () => { const indexer = new LspIncrementalIndexer( 'github.com/elastic/TypeScript-Node-Starter', 'HEAD', - '6206f643', + '67002808', lspservice, serverOptions, gitOps, @@ -295,8 +297,7 @@ describe('lsp_incremental_indexer unit tests', () => { repoUri: '', filePath: 'package.json', revision: 'HEAD', - originRevision: '6206f643', - localRepoPath: '', + originRevision: '67002808', kind: DiffKind.MODIFIED, }); @@ -305,16 +306,15 @@ describe('lsp_incremental_indexer unit tests', () => { assert.strictEqual(createSpy.callCount, 0); assert.strictEqual(putAliasSpy.callCount, 0); - // There are 2 items after the checkpoint. 1 file - // + 1 symbol + 1 ref = 3 objects to be indexed for each item. Total doc - // indexed should be 3 * 2 = 6, which can be fitted into a single batch index. + // There are 2 items after the checkpoint. No items is in supported language. + // Total doc indexed should be 3 * 1 = 3, which can be fitted into a single batch index. assert.strictEqual(bulkSpy.callCount, 2); let total = 0; for (let i = 0; i < bulkSpy.callCount; i++) { total += bulkSpy.getCall(i).args[0].body.length; } - assert.strictEqual(total, 6 * 2); - assert.strictEqual(deleteByQuerySpy.callCount, 2); + assert.strictEqual(total, 5 * 2); + assert.strictEqual(deleteByQuerySpy.callCount, 4); // @ts-ignore }).timeout(20000); // @ts-ignore diff --git a/x-pack/legacy/plugins/code/server/__tests__/lsp_indexer.ts b/x-pack/legacy/plugins/code/server/__tests__/lsp_indexer.ts index eecbdd20af5e55..454ad79b7342a3 100644 --- a/x-pack/legacy/plugins/code/server/__tests__/lsp_indexer.ts +++ b/x-pack/legacy/plugins/code/server/__tests__/lsp_indexer.ts @@ -38,6 +38,7 @@ const esClient = { function prepareProject(url: string, p: string) { const opts: CloneOptions = { + bare: 1, fetchOpts: { callbacks: { certificateCheck: () => 0, @@ -314,7 +315,6 @@ describe('lsp_indexer unit tests', function(this: any) { repoUri: '', filePath: 'src/public/js/main.ts', revision: 'HEAD', - localRepoPath: '', }); // Expect EsClient deleteByQuery called 0 times for repository cleaning while diff --git a/x-pack/legacy/plugins/code/server/indexer/lsp_incremental_indexer.ts b/x-pack/legacy/plugins/code/server/indexer/lsp_incremental_indexer.ts index 7c72bb6f92b3ae..870101a1be35a1 100644 --- a/x-pack/legacy/plugins/code/server/indexer/lsp_incremental_indexer.ts +++ b/x-pack/legacy/plugins/code/server/indexer/lsp_incremental_indexer.ts @@ -4,10 +4,6 @@ * you may not use this file except in compliance with the Elastic License. */ -import fs from 'fs'; -import util from 'util'; -import path from 'path'; - import { ProgressReporter } from '.'; import { Diff, DiffKind } from '../../common/git_diff'; import { toCanonicalUrl } from '../../common/uri_util'; @@ -25,7 +21,7 @@ import { LspService } from '../lsp/lsp_service'; import { ServerOptions } from '../server_options'; import { detectLanguage } from '../utils/detect_language'; import { LspIndexer } from './lsp_indexer'; -import { DocumentIndexName, ReferenceIndexName, SymbolIndexName } from './schema'; +import { DocumentIndexName, SymbolIndexName } from './schema'; export class LspIncrementalIndexer extends LspIndexer { protected type: string = 'lsp_inc'; @@ -125,15 +121,10 @@ export class LspIncrementalIndexer extends LspIndexer { protected async *getIndexRequestIterator(): AsyncIterableIterator { try { - const { workspaceDir } = await this.lspService.workspaceHandler.openWorkspace( - this.repoUri, - HEAD - ); if (this.diff) { for (const f of this.diff.files) { yield { repoUri: this.repoUri, - localRepoPath: workspaceDir, filePath: f.path, originPath: f.originPath, // Always use HEAD for now until we have multi revision. @@ -175,48 +166,25 @@ export class LspIncrementalIndexer extends LspIndexer { } private async handleAddedRequest(request: LspIncIndexRequest, stats: IndexStats) { - const { repoUri, revision, filePath, localRepoPath } = request; - - const lspDocUri = toCanonicalUrl({ repoUri, revision, file: filePath, schema: 'git:' }); - const symbolNames = new Set(); + const { repoUri, filePath } = request; + let content = ''; try { - const response = await this.lspService.sendRequest('textDocument/full', { - textDocument: { - uri: lspDocUri, - }, - reference: this.options.enableGlobalReference, - }); - - if (response && response.result.length > 0) { - const { symbols, references } = response.result[0]; - for (const symbol of symbols) { - await this.lspBatchIndexHelper.index(SymbolIndexName(repoUri), symbol); - symbolNames.add(symbol.symbolInformation.name); - } - stats.set(IndexStatsKey.Symbol, symbols.length); - - for (const ref of references) { - await this.lspBatchIndexHelper.index(ReferenceIndexName(repoUri), ref); - } - stats.set(IndexStatsKey.Reference, references.length); + content = await this.getFileSource(request); + } catch (error) { + if ((error as Error).message === this.FILE_OVERSIZE_ERROR_MSG) { + // Skip this index request if the file is oversized + this.log.debug(this.FILE_OVERSIZE_ERROR_MSG); + return stats; } else { - this.log.debug(`Empty response from lsp server. Skip symbols and references indexing.`); + // Rethrow the issue if for other reasons + throw error; } - } catch (error) { - this.log.error(`Index symbols or references error. Skip to file indexing.`); - this.log.error(error); } - const localFilePath = path.join(localRepoPath, filePath); - const lstat = util.promisify(fs.lstat); - const stat = await lstat(localFilePath); - - const readLink = util.promisify(fs.readlink); - const readFile = util.promisify(fs.readFile); - const content = stat.isSymbolicLink() - ? await readLink(localFilePath, 'utf8') - : await readFile(localFilePath, 'utf8'); + const { symbolNames, symbolsLength, referencesLength } = await this.execLspIndexing(request); + stats.set(IndexStatsKey.Symbol, symbolsLength); + stats.set(IndexStatsKey.Reference, referencesLength); const language = await detectLanguage(filePath, Buffer.from(content)); const body: Document = { @@ -269,17 +237,14 @@ export class LspIncrementalIndexer extends LspIndexer { } private async handleModifiedRequest(request: LspIncIndexRequest, stats: IndexStats) { - const { kind, originRevision, originPath, repoUri, localRepoPath } = request; + const { originRevision, originPath } = request; // 1. first delete all related indexed data await this.handleDeletedRequest( { - repoUri, - localRepoPath, + ...request, revision: originRevision, filePath: originPath ? originPath : '', - kind, - originRevision, }, stats ); diff --git a/x-pack/legacy/plugins/code/server/indexer/lsp_indexer.ts b/x-pack/legacy/plugins/code/server/indexer/lsp_indexer.ts index 4c0b2f406ba0f5..19afb658299adf 100644 --- a/x-pack/legacy/plugins/code/server/indexer/lsp_indexer.ts +++ b/x-pack/legacy/plugins/code/server/indexer/lsp_indexer.ts @@ -4,10 +4,6 @@ * you may not use this file except in compliance with the Elastic License. */ -import fs from 'fs'; -import util from 'util'; -import path from 'path'; - import { ResponseError } from 'vscode-jsonrpc'; import { ProgressReporter } from '.'; @@ -103,16 +99,11 @@ export class LspIndexer extends AbstractIndexer { protected async *getIndexRequestIterator(): AsyncIterableIterator { try { - const { workspaceDir } = await this.lspService.workspaceHandler.openWorkspace( - this.repoUri, - HEAD - ); const fileIterator = await this.gitOps.iterateRepo(this.repoUri, HEAD); for await (const file of fileIterator) { const filePath = file.path!; const req: LspIndexRequest = { repoUri: this.repoUri, - localRepoPath: workspaceDir, filePath, // Always use HEAD for now until we have multi revision. // Also, since the workspace might get updated during the index, we always @@ -200,32 +191,29 @@ export class LspIndexer extends AbstractIndexer { } } - protected async processRequest(request: LspIndexRequest): Promise { - const stats: IndexStats = new Map() - .set(IndexStatsKey.Symbol, 0) - .set(IndexStatsKey.Reference, 0) - .set(IndexStatsKey.File, 0); - const { repoUri, revision, filePath, localRepoPath } = request; + protected FILE_OVERSIZE_ERROR_MSG = 'File size exceeds limit. Skip index.'; + protected async getFileSource(request: LspIndexRequest): Promise { + const { revision, filePath } = request; + // Always read file content from the original bare repo + const blob = await this.gitOps.fileContent(this.repoUri, filePath, revision); + if (blob.rawsize() > TEXT_FILE_LIMIT) { + throw new Error(this.FILE_OVERSIZE_ERROR_MSG); + } + return blob.content().toString(); + } - this.log.debug(`Indexing ${filePath} at revision ${revision} for ${repoUri}`); + protected async execLspIndexing( + request: LspIndexRequest + ): Promise<{ + symbolNames: Set; + symbolsLength: number; + referencesLength: number; + }> { + const { repoUri, revision, filePath } = request; const lspDocUri = toCanonicalUrl({ repoUri, revision, file: filePath, schema: 'git:' }); const symbolNames = new Set(); - - const localFilePath = path.join(localRepoPath, filePath); - const lstat = util.promisify(fs.lstat); - const stat = await lstat(localFilePath); - - if (stat.size > TEXT_FILE_LIMIT) { - this.log.debug(`File size exceeds limit. Skip index.`); - return stats; - } - - const readLink = util.promisify(fs.readlink); - const readFile = util.promisify(fs.readFile); - const content = stat.isSymbolicLink() - ? await readLink(localFilePath, 'utf8') - : await readFile(localFilePath, 'utf8'); - + let symbolsLength = 0; + let referencesLength = 0; try { const lang = detectLanguageByFilename(filePath); // filter file by language @@ -243,12 +231,12 @@ export class LspIndexer extends AbstractIndexer { await this.lspBatchIndexHelper.index(SymbolIndexName(repoUri), symbol); symbolNames.add(symbol.symbolInformation.name); } - stats.set(IndexStatsKey.Symbol, symbols.length); + symbolsLength = symbols.length; for (const ref of references) { await this.lspBatchIndexHelper.index(ReferenceIndexName(repoUri), ref); } - stats.set(IndexStatsKey.Reference, references.length); + referencesLength = references.length; } else { this.log.debug(`Empty response from lsp server. Skip symbols and references indexing.`); } @@ -269,6 +257,35 @@ export class LspIndexer extends AbstractIndexer { } } + return { symbolNames, symbolsLength, referencesLength }; + } + + protected async processRequest(request: LspIndexRequest): Promise { + const stats: IndexStats = new Map() + .set(IndexStatsKey.Symbol, 0) + .set(IndexStatsKey.Reference, 0) + .set(IndexStatsKey.File, 0); + const { repoUri, revision, filePath } = request; + this.log.debug(`Indexing ${filePath} at revision ${revision} for ${repoUri}`); + + let content = ''; + try { + content = await this.getFileSource(request); + } catch (error) { + if ((error as Error).message === this.FILE_OVERSIZE_ERROR_MSG) { + // Skip this index request if the file is oversized + this.log.debug(this.FILE_OVERSIZE_ERROR_MSG); + return stats; + } else { + // Rethrow the issue if for other reasons + throw error; + } + } + + const { symbolNames, symbolsLength, referencesLength } = await this.execLspIndexing(request); + stats.set(IndexStatsKey.Symbol, symbolsLength); + stats.set(IndexStatsKey.Reference, referencesLength); + const language = await detectLanguage(filePath, Buffer.from(content)); const body: Document = { repoUri,