From ffcd06c8c5f0e8762fa9c2c2370c38a1daac907e Mon Sep 17 00:00:00 2001 From: Jacob Wallraff Date: Fri, 3 Mar 2023 12:28:29 -0800 Subject: [PATCH] Move caching to language service --- languageservice/src/complete.ts | 17 +++-- languageservice/src/document-links.ts | 9 ++- languageservice/src/hover.ts | 19 +++--- languageservice/src/utils/workflow-cache.ts | 63 +++++++++++++++++++ languageservice/src/validate.ts | 20 +++--- workflow-parser/src/model/convert.ts | 16 ----- .../src/workflows/workflow-parser.ts | 17 ----- 7 files changed, 95 insertions(+), 66 deletions(-) create mode 100644 languageservice/src/utils/workflow-cache.ts diff --git a/languageservice/src/complete.ts b/languageservice/src/complete.ts index 23226b7..0a8301f 100644 --- a/languageservice/src/complete.ts +++ b/languageservice/src/complete.ts @@ -1,11 +1,9 @@ import {complete as completeExpression, DescriptionDictionary} from "@github/actions-expressions"; import {CompletionItem as ExpressionCompletionItem} from "@github/actions-expressions/completion"; import { - convertWorkflowTemplate, isBasicExpression, isSequence, - isString, - parseWorkflow + isString } from "@github/actions-workflow-parser"; import {ErrorPolicy} from "@github/actions-workflow-parser/model/convert"; import {OPEN_EXPRESSION} from "@github/actions-workflow-parser/templates/template-constants"; @@ -21,7 +19,6 @@ import {getContext, Mode} from "./context-providers/default"; import {getWorkflowContext, WorkflowContext} from "./context/workflow-context"; import {validatorFunctions} from "./expression-validation/functions"; import {error} from "./log"; -import {nullTrace} from "./nulltrace"; import {isPotentiallyExpression} from "./utils/expression-detection"; import {findToken} from "./utils/find-token"; import {guessIndentation} from "./utils/indentation-guesser"; @@ -31,6 +28,7 @@ import {transform} from "./utils/transform"; import {Value, ValueProviderConfig} from "./value-providers/config"; import {defaultValueProviders} from "./value-providers/default"; import {definitionValues} from "./value-providers/definition"; +import {getParsedWorkflow, getWorkflowTemplate} from "./utils/workflow-cache"; export function getExpressionInput(input: string, pos: number): string { // Find start marker around the cursor position @@ -66,21 +64,22 @@ export async function complete( // Fix the input to work around YAML parsing issues const [newDoc, newPos] = transform(textDocument, position); - const file: File = { name: textDocument.uri, content: newDoc.getText() }; - const result = parseWorkflow(file, nullTrace); - if (!result.value) { + + const parsedWorkflow = getParsedWorkflow(file, textDocument.uri); + if (!parsedWorkflow) { return []; } - const {token, keyToken, parent, path} = findToken(newPos, result.value); - const template = await convertWorkflowTemplate(file.name, result.context, result.value, config?.fileProvider, { + const template = await getWorkflowTemplate(file, parsedWorkflow, textDocument.uri, config, { fetchReusableWorkflowDepth: config?.fileProvider ? 1 : 0, errorPolicy: ErrorPolicy.TryConversion }); + + const {token, keyToken, parent, path} = findToken(newPos, parsedWorkflow.value); const workflowContext = getWorkflowContext(textDocument.uri, template, path); // If we are inside an expression, take a different code-path. The workflow parser does not correctly create diff --git a/languageservice/src/document-links.ts b/languageservice/src/document-links.ts index 44ab65b..8cbe2a2 100644 --- a/languageservice/src/document-links.ts +++ b/languageservice/src/document-links.ts @@ -1,12 +1,11 @@ -import {convertWorkflowTemplate, parseWorkflow} from "@github/actions-workflow-parser"; import {ErrorPolicy} from "@github/actions-workflow-parser/model/convert"; import {isJob} from "@github/actions-workflow-parser/model/type-guards"; import {File} from "@github/actions-workflow-parser/workflows/file"; import {TextDocument} from "vscode-languageserver-textdocument"; import {DocumentLink} from "vscode-languageserver-types"; import {parseActionReference} from "./action"; -import {nullTrace} from "./nulltrace"; import {mapRange} from "./utils/range"; +import {getParsedWorkflow, getWorkflowTemplate} from "./utils/workflow-cache"; export async function documentLinks(document: TextDocument): Promise { const file: File = { @@ -14,12 +13,12 @@ export async function documentLinks(document: TextDocument): Promise(); +const workflowTemplateCache = new Map(); + +export function cacheParsedWorkflow(uri: string, parsedWorkflowResult: ParseWorkflowResult) { + parsedWorkflowCache.set(uri, parsedWorkflowResult); +} + +export function getParsedWorkflowCacheEntry(uri: string): ParseWorkflowResult | undefined { + return parsedWorkflowCache.get(uri); +} + +export function clearParsedCacheEntry(uri: string) { + parsedWorkflowCache.delete(uri); +} + +export function clearParsedCache() { + parsedWorkflowCache.clear(); +} + +export function cacheWorkflowTemplate(uri: string, workflowTemplate: WorkflowTemplate) { + workflowTemplateCache.set(uri, workflowTemplate); +} + +export function getWorkflowTemplateCacheEntry(uri: string): WorkflowTemplate | undefined { + return workflowTemplateCache.get(uri); +} + +export function clearWorkflowTemplateCacheEntry(uri: string) { + workflowTemplateCache.delete(uri); +} + +export function clearWorkflowTemplateCache() { + workflowTemplateCache.clear(); +} + +export function getParsedWorkflow(file: File, uri: string): ParseWorkflowResult | undefined { + let result = getParsedWorkflowCacheEntry(uri); + if (!result || !result.value) { + result = parseWorkflow(file, nullTrace); + if (!result.value) { + return undefined; + } + cacheParsedWorkflow(uri, result) + } + return result; +} + +export async function getWorkflowTemplate(file: File, parsedWorkflow: ParseWorkflowResult, uri: string, config?: CompletionConfig, options?: WorkflowTemplateConverterOptions): Promise { + let template = getWorkflowTemplateCacheEntry(uri); + if (!template) { + template = await convertWorkflowTemplate(file.name, parsedWorkflow.context, parsedWorkflow.value!, config?.fileProvider, options); + cacheWorkflowTemplate(uri, template); + } + return template; +} \ No newline at end of file diff --git a/languageservice/src/validate.ts b/languageservice/src/validate.ts index 9b4c55c..e785a4d 100644 --- a/languageservice/src/validate.ts +++ b/languageservice/src/validate.ts @@ -1,11 +1,8 @@ import {Evaluator, ExpressionEvaluationError, Lexer, Parser} from "@github/actions-expressions"; import {Expr} from "@github/actions-expressions/ast"; import { - convertWorkflowTemplate, isBasicExpression, isString, - parseWorkflow, - ParseWorkflowResult, WorkflowTemplate } from "@github/actions-workflow-parser"; import {ErrorPolicy} from "@github/actions-workflow-parser/model/convert"; @@ -18,19 +15,18 @@ import {FileProvider} from "@github/actions-workflow-parser/workflows/file-provi import {TextDocument} from "vscode-languageserver-textdocument"; import {Diagnostic, DiagnosticSeverity, URI} from "vscode-languageserver-types"; import {ActionMetadata, ActionReference} from "./action"; - import {ContextProviderConfig} from "./context-providers/config"; import {getContext, Mode} from "./context-providers/default"; import {getWorkflowContext, WorkflowContext} from "./context/workflow-context"; import {AccessError, wrapDictionary} from "./expression-validation/error-dictionary"; import {validatorFunctions} from "./expression-validation/functions"; import {error} from "./log"; -import {nullTrace} from "./nulltrace"; import {findToken} from "./utils/find-token"; import {mapRange} from "./utils/range"; import {validateAction} from "./validate-action"; import {ValueProviderConfig, ValueProviderKind} from "./value-providers/config"; import {defaultValueProviders} from "./value-providers/default"; +import { getParsedWorkflow, getWorkflowTemplate } from "./utils/workflow-cache"; export type ValidationConfig = { valueProviderConfig?: ValueProviderConfig; @@ -54,20 +50,24 @@ export async function validate(textDocument: TextDocument, config?: ValidationCo const diagnostics: Diagnostic[] = []; try { - const result: ParseWorkflowResult = parseWorkflow(file, nullTrace); - if (result.value) { + const parsedWorkflow = getParsedWorkflow(file, textDocument.uri); + if (!parsedWorkflow) { + return []; + } + + if (parsedWorkflow.value) { // Errors will be updated in the context. Attempt to do the conversion anyway in order to give the user more information - const template = await convertWorkflowTemplate(file.name, result.context, result.value, config?.fileProvider, { + const template = await getWorkflowTemplate(file, parsedWorkflow, textDocument.uri, config, { fetchReusableWorkflowDepth: config?.fileProvider ? 1 : 0, errorPolicy: ErrorPolicy.TryConversion }); // Validate expressions and value providers - await additionalValidations(diagnostics, textDocument.uri, template, result.value, config); + await additionalValidations(diagnostics, textDocument.uri, template, parsedWorkflow.value, config); } // For now map parser errors directly to diagnostics - for (const error of result.context.errors.getErrors()) { + for (const error of parsedWorkflow.context.errors.getErrors()) { let range = mapRange(error.range); diagnostics.push({ diff --git a/workflow-parser/src/model/convert.ts b/workflow-parser/src/model/convert.ts index 109d5d2..4e11dfc 100644 --- a/workflow-parser/src/model/convert.ts +++ b/workflow-parser/src/model/convert.ts @@ -48,17 +48,11 @@ const defaultOptions: Required = { }; export async function convertWorkflowTemplate( - uri: string, context: TemplateContext, root: TemplateToken, fileProvider?: FileProvider, options: WorkflowTemplateConverterOptions = defaultOptions ): Promise { - const cachedResult = workflowTemplateCache.get(uri) - if (cachedResult) { - return cachedResult; - } - const result = {} as WorkflowTemplate; const opts = getOptionsWithDefaults(options); @@ -66,7 +60,6 @@ export async function convertWorkflowTemplate( result.errors = context.errors.getErrors().map(x => ({ Message: x.message })); - workflowTemplateCache.set(uri, result); return result; } @@ -138,7 +131,6 @@ export async function convertWorkflowTemplate( } } - workflowTemplateCache.set(uri, result); return result; } @@ -155,11 +147,3 @@ function getOptionsWithDefaults(options: WorkflowTemplateConverterOptions): Requ errorPolicy: options.errorPolicy !== undefined ? options.errorPolicy : defaultOptions.errorPolicy }; } - -export function clearWorkflowTemplateCacheEntry(uri: string) { - workflowTemplateCache.delete(uri); -} - -export function clearWorkflowTemplateCache() { - workflowTemplateCache.clear(); -} \ No newline at end of file diff --git a/workflow-parser/src/workflows/workflow-parser.ts b/workflow-parser/src/workflows/workflow-parser.ts index 978f12d..b70d0f7 100644 --- a/workflow-parser/src/workflows/workflow-parser.ts +++ b/workflow-parser/src/workflows/workflow-parser.ts @@ -7,8 +7,6 @@ import {WORKFLOW_ROOT} from "./workflow-constants"; import {getWorkflowSchema} from "./workflow-schema"; import {YamlObjectReader} from "./yaml-object-reader"; -const parsedWorkflowCache = new Map(); - export interface ParseWorkflowResult { context: TemplateContext; value: TemplateToken | undefined; @@ -17,11 +15,6 @@ export interface ParseWorkflowResult { export function parseWorkflow(entryFile: File, trace: TraceWriter): ParseWorkflowResult; export function parseWorkflow(entryFile: File, context: TemplateContext): ParseWorkflowResult; export function parseWorkflow(entryFile: File, contextOrTrace: TraceWriter | TemplateContext): ParseWorkflowResult { - const cachedResult = parsedWorkflowCache.get(entryFile.name) - if (cachedResult) { - return cachedResult; - } - const context = contextOrTrace instanceof TemplateContext ? contextOrTrace @@ -38,7 +31,6 @@ export function parseWorkflow(entryFile: File, contextOrTrace: TraceWriter | Tem context, value: undefined }; - parsedWorkflowCache.set(entryFile.name, result); return result } const templateToken = templateReader.readTemplate(context, WORKFLOW_ROOT, reader, fileId); @@ -47,14 +39,5 @@ export function parseWorkflow(entryFile: File, contextOrTrace: TraceWriter | Tem context, value: templateToken } satisfies ParseWorkflowResult; - parsedWorkflowCache.set(entryFile.name, result); return result; } - -export function clearParsedCacheEntry(path: string) { - parsedWorkflowCache.delete(path); -} - -export function clearParsedCache() { - parsedWorkflowCache.clear(); -}