From d95eff9beb7c4c2d7ac509e4fd0665891669e89b Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Sun, 7 Jul 2024 23:33:28 +0200 Subject: [PATCH 01/13] Add ToolShedService to search tools --- .../server-common/src/inversify.config.ts | 4 +- .../server-common/src/languageTypes.ts | 14 +++++ .../server-common/src/services/toolShed.ts | 51 +++++++++++++++++++ .../server-common/tests/testHelpers.ts | 31 +++++++++++ 4 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 server/packages/server-common/src/services/toolShed.ts diff --git a/server/packages/server-common/src/inversify.config.ts b/server/packages/server-common/src/inversify.config.ts index fce9f36..ff2379b 100644 --- a/server/packages/server-common/src/inversify.config.ts +++ b/server/packages/server-common/src/inversify.config.ts @@ -1,12 +1,14 @@ import { Container } from "inversify"; import { ConfigService, ConfigServiceImpl } from "./configService"; -import { DocumentsCache, TYPES, WorkflowDataProvider } from "./languageTypes"; +import { DocumentsCache, TYPES, ToolshedService, WorkflowDataProvider } from "./languageTypes"; import { DocumentsCacheImpl } from "./models/documentsCache"; import { WorkflowDataProviderImpl } from "./providers/workflowDataProvider"; +import { ToolshedServiceImpl } from "./services/toolShed"; const container = new Container(); container.bind(TYPES.ConfigService).to(ConfigServiceImpl).inSingletonScope(); container.bind(TYPES.DocumentsCache).to(DocumentsCacheImpl).inSingletonScope(); container.bind(TYPES.WorkflowDataProvider).to(WorkflowDataProviderImpl).inSingletonScope(); +container.bind(TYPES.ToolshedService).to(ToolshedServiceImpl).inSingletonScope(); export { container }; diff --git a/server/packages/server-common/src/languageTypes.ts b/server/packages/server-common/src/languageTypes.ts index 3c1fd1c..6d2ad14 100644 --- a/server/packages/server-common/src/languageTypes.ts +++ b/server/packages/server-common/src/languageTypes.ts @@ -298,6 +298,19 @@ export interface WorkflowDataProvider { getWorkflowOutputs(workflowDocumentUri: string): Promise; } +export interface ToolInfo { + id: string; + name: string; + description: string; + owner: string; + repository: string; + url: string; +} + +export interface ToolshedService { + searchTools(query: string): Promise; +} + const TYPES = { DocumentsCache: Symbol.for("DocumentsCache"), ConfigService: Symbol.for("ConfigService"), @@ -307,6 +320,7 @@ const TYPES = { GalaxyWorkflowLanguageServer: Symbol.for("GalaxyWorkflowLanguageServer"), WorkflowDataProvider: Symbol.for("WorkflowDataProvider"), SymbolsProvider: Symbol.for("SymbolsProvider"), + ToolshedService: Symbol.for("ToolshedService"), }; export { TYPES }; diff --git a/server/packages/server-common/src/services/toolShed.ts b/server/packages/server-common/src/services/toolShed.ts new file mode 100644 index 0000000..f1edc48 --- /dev/null +++ b/server/packages/server-common/src/services/toolShed.ts @@ -0,0 +1,51 @@ +import { injectable } from "inversify"; +import { ToolInfo, ToolshedService } from "../languageTypes"; + +const TOOLSHED_URL = "https://toolshed.g2.bx.psu.edu"; + +interface ToolShedResponse { + total_results: string; + page: string; + page_size: string; + hits: { + tool: { + id: string; + repo_owner_username: string; + repo_name: string; + name: string; + description: string; + }; + matched_terms: { + name?: string; + description?: string; + help?: string; + }; + score: number; + }[]; + hostname: string; +} + +@injectable() +export class ToolshedServiceImpl implements ToolshedService { + public async searchTools(query: string): Promise { + try { + const response = await fetch(`${TOOLSHED_URL}/api/tools?q=${query}&page_size=5`); + const json = (await response.json()) as ToolShedResponse; + const hits = json.hits; + return hits.map((hit) => { + const tool = hit.tool; + return { + id: tool.id, + name: tool.name, + description: tool.description, + owner: tool.repo_owner_username, + repository: tool.repo_name, + url: `${TOOLSHED_URL}/repos/${tool.repo_owner_username}/${tool.repo_name}/${tool.id}`, + }; + }); + } catch (error) { + console.error("Error fetching tools from the toolshed", error); + return []; + } + } +} diff --git a/server/packages/server-common/tests/testHelpers.ts b/server/packages/server-common/tests/testHelpers.ts index 3ac5e32..9881b2b 100644 --- a/server/packages/server-common/tests/testHelpers.ts +++ b/server/packages/server-common/tests/testHelpers.ts @@ -2,6 +2,8 @@ import { ASTNode, PropertyASTNode } from "../src/ast/types"; import { CompletionItem, CompletionList, + ToolInfo, + ToolshedService, WorkflowDataProvider, WorkflowInput, WorkflowOutput, @@ -127,3 +129,32 @@ export const FAKE_WORKFLOW_DATA_PROVIDER: WorkflowDataProvider = { }; }, }; + +export function createFakeToolInfo( + id: string, + name?: string, + description?: string, + owner: string = "fakeowner", + repo: string = "fakerepo" +): ToolInfo { + return { + id, + name: name ?? `Tool ${id}`, + description: description ?? `This is a tool description for tool ${id}.`, + owner, + repository: repo, + url: `https://toolshed.testing.fake/repos/${owner}/${repo}/${id}`, + }; +} + +export const FAKE_TOOLS: ToolInfo[] = []; +for (let i = 1; i < 3; i++) { + const num = `${i}`.padStart(3, "0"); + FAKE_TOOLS.push(createFakeToolInfo(`tool${num}`)); +} + +export const FAKE_TOOLSHED_SERVICE: ToolshedService = { + async searchTools(_query: string) { + return FAKE_TOOLS; + }, +}; From 4b9caa82d200d4f1fa4a4f5ad6a854e29a958454 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Sun, 7 Jul 2024 23:45:38 +0200 Subject: [PATCH 02/13] Add tool search when autocompleting tool_id in format2 workflows --- .../src/languageService.ts | 6 ++- .../src/services/completionService.ts | 49 ++++++++++++++++--- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/server/gx-workflow-ls-format2/src/languageService.ts b/server/gx-workflow-ls-format2/src/languageService.ts index f923c97..3f71cd4 100644 --- a/server/gx-workflow-ls-format2/src/languageService.ts +++ b/server/gx-workflow-ls-format2/src/languageService.ts @@ -12,6 +12,7 @@ import { TYPES, TextDocument, TextEdit, + ToolshedService, } from "@gxwf/server-common/src/languageTypes"; import { TYPES as YAML_TYPES } from "@gxwf/yaml-language-service/src/inversify.config"; import { YAMLLanguageService } from "@gxwf/yaml-language-service/src/yamlLanguageService"; @@ -44,13 +45,14 @@ export class GxFormat2WorkflowLanguageServiceImpl constructor( @inject(YAML_TYPES.YAMLLanguageService) yamlLanguageService: YAMLLanguageService, - @inject(TYPES.SymbolsProvider) private symbolsProvider: SymbolsProvider + @inject(TYPES.SymbolsProvider) private symbolsProvider: SymbolsProvider, + @inject(TYPES.ToolshedService) private toolshedService: ToolshedService ) { super(LANGUAGE_ID); this._schemaLoader = new GalaxyWorkflowFormat2SchemaLoader(); this._yamlLanguageService = yamlLanguageService; this._hoverService = new GxFormat2HoverService(this._schemaLoader.nodeResolver); - this._completionService = new GxFormat2CompletionService(this._schemaLoader.nodeResolver); + this._completionService = new GxFormat2CompletionService(this._schemaLoader.nodeResolver, this.toolshedService); this._schemaValidationService = new GxFormat2SchemaValidationService(this._schemaLoader.nodeResolver); } diff --git a/server/gx-workflow-ls-format2/src/services/completionService.ts b/server/gx-workflow-ls-format2/src/services/completionService.ts index 1b53c7e..617eb86 100644 --- a/server/gx-workflow-ls-format2/src/services/completionService.ts +++ b/server/gx-workflow-ls-format2/src/services/completionService.ts @@ -1,5 +1,13 @@ import { ASTNode } from "@gxwf/server-common/src/ast/types"; -import { CompletionItem, CompletionItemKind, CompletionList, Position } from "@gxwf/server-common/src/languageTypes"; +import { + CompletionItem, + CompletionItemKind, + CompletionList, + Position, + Range, + ToolInfo, + ToolshedService, +} from "@gxwf/server-common/src/languageTypes"; import { TextBuffer } from "@gxwf/yaml-language-service/src/utils/textBuffer"; import { GxFormat2WorkflowDocument } from "../gxFormat2WorkflowDocument"; import { FieldSchemaNode, RecordSchemaNode, SchemaNode, SchemaNodeResolver } from "../schema"; @@ -12,9 +20,12 @@ export class GxFormat2CompletionService { */ private readonly ignoredSchemaRefs = new Set(["InputParameter", "OutputParameter", "WorkflowStep"]); - constructor(protected readonly schemaNodeResolver: SchemaNodeResolver) {} + constructor( + protected readonly schemaNodeResolver: SchemaNodeResolver, + protected readonly toolshedService: ToolshedService + ) {} - public doComplete(documentContext: GxFormat2WorkflowDocument, position: Position): Promise { + public async doComplete(documentContext: GxFormat2WorkflowDocument, position: Position): Promise { const textDocument = documentContext.textDocument; const nodeManager = documentContext.nodeManager; const result: CompletionList = { @@ -42,17 +53,17 @@ export class GxFormat2CompletionService { } if (schemaNode) { const existing = nodeManager.getDeclaredPropertyNames(node); - result.items = this.getProposedItems(schemaNode, textBuffer, existing, offset); + result.items = await this.getProposedItems(schemaNode, textBuffer, existing, offset); } return Promise.resolve(result); } - private getProposedItems( + private async getProposedItems( schemaNode: SchemaNode, textBuffer: TextBuffer, exclude: Set, offset: number - ): CompletionItem[] { + ): Promise { const result: CompletionItem[] = []; const currentWord = textBuffer.getCurrentWord(offset); const overwriteRange = textBuffer.getCurrentWordRange(offset); @@ -116,11 +127,20 @@ export class GxFormat2CompletionService { result.push(item); return result; } + if (schemaNode.name === "tool_id") { + if (currentWord) { + const tools = await this.toolshedService.searchTools(currentWord); + for (const tool of tools) { + const item: CompletionItem = this.buildCompletionItemFromTool(tool, overwriteRange); + result.push(item); + } + } + } } else if (schemaNode.isUnionType) { for (const typeRef of schemaNode.typeRefs) { const typeNode = this.schemaNodeResolver.getSchemaNodeByTypeRef(typeRef); if (typeNode === undefined) continue; - result.push(...this.getProposedItems(typeNode, textBuffer, exclude, offset)); + result.push(...(await this.getProposedItems(typeNode, textBuffer, exclude, offset))); } return result; } @@ -136,6 +156,21 @@ export class GxFormat2CompletionService { } return result; } + + private buildCompletionItemFromTool(tool: ToolInfo, overwriteRange: Range): CompletionItem { + const toolEntry = tool.url.replace("https://", ""); + const item: CompletionItem = { + label: tool.id, + kind: CompletionItemKind.Value, + documentation: tool.description, + insertText: toolEntry, + textEdit: { + range: overwriteRange, + newText: toolEntry, + }, + }; + return item; + } } function _DEBUG_printNodeName(node: ASTNode): void { From f62fd2095ebd465746b88f361e891be4bf386585 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Sun, 7 Jul 2024 23:46:48 +0200 Subject: [PATCH 03/13] Add unit tests for tool_id auto-completion --- .../tests/integration/completion.test.ts | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/server/gx-workflow-ls-format2/tests/integration/completion.test.ts b/server/gx-workflow-ls-format2/tests/integration/completion.test.ts index d5b7f16..1db1d6d 100644 --- a/server/gx-workflow-ls-format2/tests/integration/completion.test.ts +++ b/server/gx-workflow-ls-format2/tests/integration/completion.test.ts @@ -1,5 +1,10 @@ import { CompletionList } from "@gxwf/server-common/src/languageTypes"; -import { getCompletionItemsLabels, parseTemplate } from "@gxwf/server-common/tests/testHelpers"; +import { + FAKE_TOOLS, + FAKE_TOOLSHED_SERVICE, + getCompletionItemsLabels, + parseTemplate, +} from "@gxwf/server-common/tests/testHelpers"; import "reflect-metadata"; import { GalaxyWorkflowFormat2SchemaLoader } from "../../src/schema"; @@ -10,7 +15,7 @@ describe("Format2 Workflow Completion Service", () => { let service: GxFormat2CompletionService; beforeAll(() => { const schemaNodeResolver = new GalaxyWorkflowFormat2SchemaLoader().nodeResolver; - service = new GxFormat2CompletionService(schemaNodeResolver); + service = new GxFormat2CompletionService(schemaNodeResolver, FAKE_TOOLSHED_SERVICE); }); async function getCompletions( @@ -380,4 +385,32 @@ inputs: expect(completions?.items).toHaveLength(0); }); + + it("should suggest toolshed tools when the cursor is inside the `tool_id` property and there is at least one character", async () => { + const template = ` +class: GalaxyWorkflow +steps: + my_step: + tool_id: t$`; + const EXPECTED_COMPLETION_LABELS = FAKE_TOOLS.map((tool) => tool.id); + const { contents, position } = parseTemplate(template); + + const completions = await getCompletions(contents, position); + + const completionLabels = getCompletionItemsLabels(completions); + expect(completionLabels).toEqual(EXPECTED_COMPLETION_LABELS); + }); + + it("should not suggest toolshed tools when the cursor is inside the `tool_id` property and there is no character", async () => { + const template = ` +class: GalaxyWorkflow +steps: + my_step: + tool_id: $`; + const { contents, position } = parseTemplate(template); + + const completions = await getCompletions(contents, position); + + expect(completions?.items).toHaveLength(0); + }); }); From 29407d5747c9e59dd4ce7dd8f8411f3976a2a1a4 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Mon, 8 Jul 2024 00:34:58 +0200 Subject: [PATCH 04/13] Update ToolshedServiceImpl to use a limit for tool search --- server/packages/server-common/src/services/toolShed.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/packages/server-common/src/services/toolShed.ts b/server/packages/server-common/src/services/toolShed.ts index f1edc48..7bc8a51 100644 --- a/server/packages/server-common/src/services/toolShed.ts +++ b/server/packages/server-common/src/services/toolShed.ts @@ -27,9 +27,10 @@ interface ToolShedResponse { @injectable() export class ToolshedServiceImpl implements ToolshedService { + private readonly limit = 5; public async searchTools(query: string): Promise { try { - const response = await fetch(`${TOOLSHED_URL}/api/tools?q=${query}&page_size=5`); + const response = await fetch(`${toolshedUrl}/api/tools?q=${query}&page_size=${this.limit}`); const json = (await response.json()) as ToolShedResponse; const hits = json.hits; return hits.map((hit) => { From 2fd300259f467c3eba80c9d2858f261275a491b8 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Sat, 13 Jul 2024 11:18:55 +0200 Subject: [PATCH 05/13] Update ConfigService To get default configuration section when none is provided. --- server/packages/server-common/src/configService.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/server/packages/server-common/src/configService.ts b/server/packages/server-common/src/configService.ts index e22a096..a98c3bf 100644 --- a/server/packages/server-common/src/configService.ts +++ b/server/packages/server-common/src/configService.ts @@ -46,10 +46,12 @@ const documentSettingsCache: Map = new Map(); export interface ConfigService { readonly connection: Connection; initialize(capabilities: ClientCapabilities, onConfigurationChanged: () => void): void; - getDocumentSettings(uri: string): Promise; + getDocumentSettings(uri?: string): Promise; onDocumentClose(uri: string): void; } +const sectionName = "galaxyWorkflows"; + @injectable() export class ConfigServiceImpl implements ConfigService { protected hasConfigurationCapability = false; @@ -67,15 +69,20 @@ export class ConfigServiceImpl implements ConfigService { this.hasConfigurationCapability = !!(capabilities.workspace && !!capabilities.workspace.configuration); } - public async getDocumentSettings(uri: string): Promise { + public async getDocumentSettings(uri?: string): Promise { if (!this.hasConfigurationCapability) { return Promise.resolve(globalSettings); } + + if (!uri) { + return await this.connection.workspace.getConfiguration(sectionName); + } + let result = documentSettingsCache.get(uri); if (!result) { result = await this.connection.workspace.getConfiguration({ scopeUri: uri, - section: "galaxyWorkflows", + section: sectionName, }); result = result || globalSettings; this.addToDocumentConfigCache(uri, result); From 1cd3dc138331dad2f946d9bb75950351c7855bab Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Sat, 13 Jul 2024 11:19:47 +0200 Subject: [PATCH 06/13] Add Toolshed URL to settings --- package.json | 6 ++++++ server/packages/server-common/src/configService.ts | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/package.json b/package.json index 7a14de0..0b9b0fb 100644 --- a/package.json +++ b/package.json @@ -124,6 +124,12 @@ "Stricter validation to comply with the `Intergalactic Workflow Commission` best practices." ], "default": "basic" + }, + "galaxyWorkflows.toolshed.url": { + "markdownDescription": "The URL of the Galaxy Toolshed to use for tool resolution.", + "scope": "resource", + "type": "string", + "default": "https://toolshed.g2.bx.psu.edu" } } } diff --git a/server/packages/server-common/src/configService.ts b/server/packages/server-common/src/configService.ts index a98c3bf..6d1e5d2 100644 --- a/server/packages/server-common/src/configService.ts +++ b/server/packages/server-common/src/configService.ts @@ -11,6 +11,7 @@ import { TYPES } from "./languageTypes"; interface ExtensionSettings { cleaning: CleaningSettings; validation: ValidationSettings; + toolshed: Toolshed; } /** Contains settings for workflow cleaning. */ @@ -29,6 +30,12 @@ interface ValidationSettings { profile: "basic" | "iwc"; } +/** Contains settings for the Toolshed service. */ +interface Toolshed { + /** The URL of the Toolshed to fetch information about tools. */ + url: string; +} + const defaultSettings: ExtensionSettings = { cleaning: { cleanableProperties: ["position", "uuid", "errors", "version"], @@ -36,6 +43,9 @@ const defaultSettings: ExtensionSettings = { validation: { profile: "basic", }, + toolshed: { + url: "https://toolshed.g2.bx.psu.edu", + }, }; let globalSettings: ExtensionSettings = defaultSettings; From 340c0dae93f20cf7a2d69cb79163c6f246c544c8 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Sat, 13 Jul 2024 11:37:30 +0200 Subject: [PATCH 07/13] Update ToolshedService to use config and handle errors --- .../server-common/src/services/toolShed.ts | 31 +++++++++++++------ server/packages/server-common/src/utils.ts | 15 +++++++++ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/server/packages/server-common/src/services/toolShed.ts b/server/packages/server-common/src/services/toolShed.ts index 7bc8a51..47b3d98 100644 --- a/server/packages/server-common/src/services/toolShed.ts +++ b/server/packages/server-common/src/services/toolShed.ts @@ -1,7 +1,7 @@ -import { injectable } from "inversify"; -import { ToolInfo, ToolshedService } from "../languageTypes"; - -const TOOLSHED_URL = "https://toolshed.g2.bx.psu.edu"; +import { inject, injectable } from "inversify"; +import { ConfigService } from "../configService"; +import { TYPES, ToolInfo, ToolshedService } from "../languageTypes"; +import { getResponseErrorMessage } from "../utils"; interface ToolShedResponse { total_results: string; @@ -27,12 +27,25 @@ interface ToolShedResponse { @injectable() export class ToolshedServiceImpl implements ToolshedService { - private readonly limit = 5; - public async searchTools(query: string): Promise { + constructor(@inject(TYPES.ConfigService) public readonly configService: ConfigService) {} + + public async searchTools(query: string, limit = 5): Promise { + const settings = await this.configService.getDocumentSettings(); + const toolshedUrl = settings.toolshed.url; + try { - const response = await fetch(`${toolshedUrl}/api/tools?q=${query}&page_size=${this.limit}`); + const whooshQueryById = `id:${query}`; + const response = await fetch(`${toolshedUrl}/api/tools?q=${whooshQueryById}&page_size=${limit}`); + + if (!response.ok) { + const error = await getResponseErrorMessage(response); + console.error(`Error fetching tools from the toolshed at '${toolshedUrl}'`, error); + return []; + } + const json = (await response.json()) as ToolShedResponse; const hits = json.hits; + return hits.map((hit) => { const tool = hit.tool; return { @@ -41,11 +54,11 @@ export class ToolshedServiceImpl implements ToolshedService { description: tool.description, owner: tool.repo_owner_username, repository: tool.repo_name, - url: `${TOOLSHED_URL}/repos/${tool.repo_owner_username}/${tool.repo_name}/${tool.id}`, + url: `${toolshedUrl}/repos/${tool.repo_owner_username}/${tool.repo_name}/${tool.id}`, }; }); } catch (error) { - console.error("Error fetching tools from the toolshed", error); + console.error(`Error fetching tools from the toolshed at '${toolshedUrl}'`, error); return []; } } diff --git a/server/packages/server-common/src/utils.ts b/server/packages/server-common/src/utils.ts index 741d709..4d5143b 100644 --- a/server/packages/server-common/src/utils.ts +++ b/server/packages/server-common/src/utils.ts @@ -57,3 +57,18 @@ export function isSimpleType(type?: string): boolean { } return SIMPLE_TYPES.includes(type); } + +/** + * Extract the error message from a fetch response. + * @param response The fetch response. + * @returns The error message. + */ +export async function getResponseErrorMessage(response: Response): Promise { + const contentType = response.headers.get("content-type"); + if (contentType && contentType.includes("application/json")) { + const json = await response.json(); + return JSON.stringify(json) || response.statusText; + } + const text = await response.text(); + return text || response.statusText; +} From 6a6248f9dcc866b1abb8337e93633ebf9b822bae Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Sat, 13 Jul 2024 12:48:30 +0200 Subject: [PATCH 08/13] Refactor ToolshedService implementation --- .../src/services/completionService.ts | 2 +- .../server-common/src/languageTypes.ts | 11 ++++- .../server-common/src/services/toolShed.ts | 45 +++++++++++++++---- .../server-common/tests/testHelpers.ts | 2 +- 4 files changed, 49 insertions(+), 11 deletions(-) diff --git a/server/gx-workflow-ls-format2/src/services/completionService.ts b/server/gx-workflow-ls-format2/src/services/completionService.ts index 617eb86..bc2c53e 100644 --- a/server/gx-workflow-ls-format2/src/services/completionService.ts +++ b/server/gx-workflow-ls-format2/src/services/completionService.ts @@ -129,7 +129,7 @@ export class GxFormat2CompletionService { } if (schemaNode.name === "tool_id") { if (currentWord) { - const tools = await this.toolshedService.searchTools(currentWord); + const tools = await this.toolshedService.searchToolsById(currentWord); for (const tool of tools) { const item: CompletionItem = this.buildCompletionItemFromTool(tool, overwriteRange); result.push(item); diff --git a/server/packages/server-common/src/languageTypes.ts b/server/packages/server-common/src/languageTypes.ts index 6d2ad14..361c68c 100644 --- a/server/packages/server-common/src/languageTypes.ts +++ b/server/packages/server-common/src/languageTypes.ts @@ -307,8 +307,17 @@ export interface ToolInfo { url: string; } +/** + * Interface for a service that can provide information about tools from the Toolshed. + */ export interface ToolshedService { - searchTools(query: string): Promise; + /** + * Searches for tools by their approximate ID. + * @param toolId The ID of the tool to search for. Doesn't have to be an exact match. + * @param limit The maximum number of tools to return. + * @returns A list of tools that match the search criteria. + */ + searchToolsById(toolId: string): Promise; } const TYPES = { diff --git a/server/packages/server-common/src/services/toolShed.ts b/server/packages/server-common/src/services/toolShed.ts index 47b3d98..f027beb 100644 --- a/server/packages/server-common/src/services/toolShed.ts +++ b/server/packages/server-common/src/services/toolShed.ts @@ -25,17 +25,20 @@ interface ToolShedResponse { hostname: string; } +interface BuildRequestResult { + request: Request; + baseUrl: URL; +} + @injectable() export class ToolshedServiceImpl implements ToolshedService { constructor(@inject(TYPES.ConfigService) public readonly configService: ConfigService) {} - public async searchTools(query: string, limit = 5): Promise { - const settings = await this.configService.getDocumentSettings(); - const toolshedUrl = settings.toolshed.url; - + public async searchToolsById(toolId: string, limit = 5): Promise { + const { request, baseUrl } = await this.buildToolSearchRequest(toolId, limit); + const toolshedUrl = baseUrl.origin; try { - const whooshQueryById = `id:${query}`; - const response = await fetch(`${toolshedUrl}/api/tools?q=${whooshQueryById}&page_size=${limit}`); + const response = await fetch(request); if (!response.ok) { const error = await getResponseErrorMessage(response); @@ -43,8 +46,9 @@ export class ToolshedServiceImpl implements ToolshedService { return []; } - const json = (await response.json()) as ToolShedResponse; - const hits = json.hits; + const json = await response.json(); + const toolshedResponse = json as ToolShedResponse; + const hits = toolshedResponse.hits; return hits.map((hit) => { const tool = hit.tool; @@ -62,4 +66,29 @@ export class ToolshedServiceImpl implements ToolshedService { return []; } } + + private async buildToolSearchRequest(toolId: string, limit: number): Promise { + const toolshedUrl = await this.validateToolshedUrl(); + const toolsApiUrl = `${toolshedUrl}/api/tools`; + const queryParams = new URLSearchParams({ + q: `id:${toolId}`, + page_size: limit.toString(), + }); + + return { + request: new Request(`${toolsApiUrl}?${queryParams}`), + baseUrl: toolshedUrl, + }; + } + + private async validateToolshedUrl(): Promise { + const settings = await this.configService.getDocumentSettings(); + let validatedUrl: URL; + try { + validatedUrl = new URL(settings.toolshed.url); + } catch (error) { + throw new Error(`Invalid Toolshed URL in settings: '${settings.toolshed.url}' - ${error}`); + } + return validatedUrl; + } } diff --git a/server/packages/server-common/tests/testHelpers.ts b/server/packages/server-common/tests/testHelpers.ts index 9881b2b..5ad9e36 100644 --- a/server/packages/server-common/tests/testHelpers.ts +++ b/server/packages/server-common/tests/testHelpers.ts @@ -154,7 +154,7 @@ for (let i = 1; i < 3; i++) { } export const FAKE_TOOLSHED_SERVICE: ToolshedService = { - async searchTools(_query: string) { + async searchToolsById(_query: string) { return FAKE_TOOLS; }, }; From 148d98a7bdd81178ccc0f988c650e1490d3c354f Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Sat, 13 Jul 2024 13:08:58 +0200 Subject: [PATCH 09/13] Improve error message when invalid URL provided --- server/packages/server-common/src/services/toolShed.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/packages/server-common/src/services/toolShed.ts b/server/packages/server-common/src/services/toolShed.ts index f027beb..f6252dd 100644 --- a/server/packages/server-common/src/services/toolShed.ts +++ b/server/packages/server-common/src/services/toolShed.ts @@ -86,8 +86,10 @@ export class ToolshedServiceImpl implements ToolshedService { let validatedUrl: URL; try { validatedUrl = new URL(settings.toolshed.url); - } catch (error) { - throw new Error(`Invalid Toolshed URL in settings: '${settings.toolshed.url}' - ${error}`); + } catch { + throw new Error( + `Invalid Toolshed URL: '${settings.toolshed.url}'. Please provide a valid URL for the setting 'galaxyWorkflows.toolshed.url'.` + ); } return validatedUrl; } From d0c17a67f4cb3fcf04b181070cb9d7aa9e3372e6 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Sat, 13 Jul 2024 13:21:24 +0200 Subject: [PATCH 10/13] Avoid tool_id completion whith slashes For now just try to search for tool ids without parsing possible additional toolsheds or repositories. --- .../src/services/completionService.ts | 2 +- .../tests/integration/completion.test.ts | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/server/gx-workflow-ls-format2/src/services/completionService.ts b/server/gx-workflow-ls-format2/src/services/completionService.ts index bc2c53e..fdbbad2 100644 --- a/server/gx-workflow-ls-format2/src/services/completionService.ts +++ b/server/gx-workflow-ls-format2/src/services/completionService.ts @@ -128,7 +128,7 @@ export class GxFormat2CompletionService { return result; } if (schemaNode.name === "tool_id") { - if (currentWord) { + if (currentWord && !currentWord.includes("/")) { const tools = await this.toolshedService.searchToolsById(currentWord); for (const tool of tools) { const item: CompletionItem = this.buildCompletionItemFromTool(tool, overwriteRange); diff --git a/server/gx-workflow-ls-format2/tests/integration/completion.test.ts b/server/gx-workflow-ls-format2/tests/integration/completion.test.ts index 1db1d6d..2f6f1a6 100644 --- a/server/gx-workflow-ls-format2/tests/integration/completion.test.ts +++ b/server/gx-workflow-ls-format2/tests/integration/completion.test.ts @@ -413,4 +413,17 @@ steps: expect(completions?.items).toHaveLength(0); }); + + it("should not suggest toolshed tools when the cursor is inside the `tool_id` property and the current word contains slashes", async () => { + const template = ` +class: GalaxyWorkflow +steps: + my_step: + tool_id: toolshed/owner/repo/tool$`; + const { contents, position } = parseTemplate(template); + + const completions = await getCompletions(contents, position); + + expect(completions?.items).toHaveLength(0); + }); }); From c4901acfcb37ee9f8862ad4b68c5c89df6340876 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Sat, 20 Jul 2024 11:23:29 +0200 Subject: [PATCH 11/13] Refactor ToolshedService mock in completion tests --- .../tests/integration/completion.test.ts | 59 +++++++++++-------- .../server-common/tests/testHelpers.ts | 39 +++++------- 2 files changed, 50 insertions(+), 48 deletions(-) diff --git a/server/gx-workflow-ls-format2/tests/integration/completion.test.ts b/server/gx-workflow-ls-format2/tests/integration/completion.test.ts index 2f6f1a6..fb035d8 100644 --- a/server/gx-workflow-ls-format2/tests/integration/completion.test.ts +++ b/server/gx-workflow-ls-format2/tests/integration/completion.test.ts @@ -1,21 +1,22 @@ -import { CompletionList } from "@gxwf/server-common/src/languageTypes"; -import { - FAKE_TOOLS, - FAKE_TOOLSHED_SERVICE, - getCompletionItemsLabels, - parseTemplate, -} from "@gxwf/server-common/tests/testHelpers"; +import { CompletionList, ToolshedService } from "@gxwf/server-common/src/languageTypes"; +import { buildFakeToolInfoList, getCompletionItemsLabels, parseTemplate } from "@gxwf/server-common/tests/testHelpers"; import "reflect-metadata"; import { GalaxyWorkflowFormat2SchemaLoader } from "../../src/schema"; import { GxFormat2CompletionService } from "../../src/services/completionService"; import { createFormat2WorkflowDocument } from "../testHelpers"; +const searchToolsByIdMock = jest.fn(); + +const ToolshedServiceMock: ToolshedService = { + searchToolsById: searchToolsByIdMock, +}; + describe("Format2 Workflow Completion Service", () => { let service: GxFormat2CompletionService; beforeAll(() => { const schemaNodeResolver = new GalaxyWorkflowFormat2SchemaLoader().nodeResolver; - service = new GxFormat2CompletionService(schemaNodeResolver, FAKE_TOOLSHED_SERVICE); + service = new GxFormat2CompletionService(schemaNodeResolver, ToolshedServiceMock); }); async function getCompletions( @@ -386,44 +387,54 @@ inputs: expect(completions?.items).toHaveLength(0); }); - it("should suggest toolshed tools when the cursor is inside the `tool_id` property and there is at least one character", async () => { - const template = ` + describe("Toolshed tool suggestions", () => { + it("should suggest toolshed tools when the cursor is inside the `tool_id` property and there is at least one character", async () => { + const expectedTools = buildFakeToolInfoList([{ id: "tool1" }, { id: "tool2" }, { id: "tool3" }]); + searchToolsByIdMock.mockResolvedValue(expectedTools); + + const template = ` class: GalaxyWorkflow steps: my_step: tool_id: t$`; - const EXPECTED_COMPLETION_LABELS = FAKE_TOOLS.map((tool) => tool.id); - const { contents, position } = parseTemplate(template); + const expectedLabels = expectedTools.map((tool) => tool.id); + const { contents, position } = parseTemplate(template); - const completions = await getCompletions(contents, position); + const completions = await getCompletions(contents, position); + expect(searchToolsByIdMock).toHaveBeenCalledWith("t"); + + const completionLabels = getCompletionItemsLabels(completions); + expect(completionLabels).toEqual(expectedLabels); + }); const completionLabels = getCompletionItemsLabels(completions); expect(completionLabels).toEqual(EXPECTED_COMPLETION_LABELS); }); - it("should not suggest toolshed tools when the cursor is inside the `tool_id` property and there is no character", async () => { - const template = ` + it("should not suggest toolshed tools when the cursor is inside the `tool_id` property and there is no character", async () => { + const template = ` class: GalaxyWorkflow steps: my_step: tool_id: $`; - const { contents, position } = parseTemplate(template); + const { contents, position } = parseTemplate(template); - const completions = await getCompletions(contents, position); + const completions = await getCompletions(contents, position); - expect(completions?.items).toHaveLength(0); - }); + expect(completions?.items).toHaveLength(0); + }); - it("should not suggest toolshed tools when the cursor is inside the `tool_id` property and the current word contains slashes", async () => { - const template = ` + it("should not suggest toolshed tools when the cursor is inside the `tool_id` property and the current word contains slashes", async () => { + const template = ` class: GalaxyWorkflow steps: my_step: tool_id: toolshed/owner/repo/tool$`; - const { contents, position } = parseTemplate(template); + const { contents, position } = parseTemplate(template); - const completions = await getCompletions(contents, position); + const completions = await getCompletions(contents, position); - expect(completions?.items).toHaveLength(0); + expect(completions?.items).toHaveLength(0); + }); }); }); diff --git a/server/packages/server-common/tests/testHelpers.ts b/server/packages/server-common/tests/testHelpers.ts index 5ad9e36..78d3b78 100644 --- a/server/packages/server-common/tests/testHelpers.ts +++ b/server/packages/server-common/tests/testHelpers.ts @@ -3,7 +3,6 @@ import { CompletionItem, CompletionList, ToolInfo, - ToolshedService, WorkflowDataProvider, WorkflowInput, WorkflowOutput, @@ -130,31 +129,23 @@ export const FAKE_WORKFLOW_DATA_PROVIDER: WorkflowDataProvider = { }, }; -export function createFakeToolInfo( - id: string, - name?: string, - description?: string, - owner: string = "fakeowner", - repo: string = "fakerepo" -): ToolInfo { +interface PartialToolInfo extends Partial { + id: string; // Only the id is required +} + +export function buildFakeToolInfo(tool: PartialToolInfo): ToolInfo { + const owner = tool.owner ?? "fakeowner"; + const repository = tool.repository ?? "fakerepo"; return { - id, - name: name ?? `Tool ${id}`, - description: description ?? `This is a tool description for tool ${id}.`, - owner, - repository: repo, - url: `https://toolshed.testing.fake/repos/${owner}/${repo}/${id}`, + id: tool.id, + name: tool.name ?? `Tool ${tool.id}`, + description: tool.description ?? `This is a tool description for tool ${tool.id}.`, + owner: owner, + repository: repository, + url: `https://toolshed.testing.fake/repos/${owner}/${repository}/${tool.id}`, }; } -export const FAKE_TOOLS: ToolInfo[] = []; -for (let i = 1; i < 3; i++) { - const num = `${i}`.padStart(3, "0"); - FAKE_TOOLS.push(createFakeToolInfo(`tool${num}`)); +export function buildFakeToolInfoList(tools: PartialToolInfo[]): ToolInfo[] { + return tools.map((tool) => buildFakeToolInfo(tool)); } - -export const FAKE_TOOLSHED_SERVICE: ToolshedService = { - async searchToolsById(_query: string) { - return FAKE_TOOLS; - }, -}; From f7d3531c941dc4b10b435091434a0e171be6f0dd Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Sat, 20 Jul 2024 12:22:28 +0200 Subject: [PATCH 12/13] Support multi-word tool search in tool_id completion + tests --- .../src/services/completionService.ts | 27 ++++++++++----- .../tests/integration/completion.test.ts | 33 ++++++++++++++----- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/server/gx-workflow-ls-format2/src/services/completionService.ts b/server/gx-workflow-ls-format2/src/services/completionService.ts index fdbbad2..0f28bdb 100644 --- a/server/gx-workflow-ls-format2/src/services/completionService.ts +++ b/server/gx-workflow-ls-format2/src/services/completionService.ts @@ -1,3 +1,4 @@ +import { ASTNodeManager } from "@gxwf/server-common/src/ast/nodeManager"; import { ASTNode } from "@gxwf/server-common/src/ast/types"; import { CompletionItem, @@ -53,7 +54,7 @@ export class GxFormat2CompletionService { } if (schemaNode) { const existing = nodeManager.getDeclaredPropertyNames(node); - result.items = await this.getProposedItems(schemaNode, textBuffer, existing, offset); + result.items = await this.getProposedItems(schemaNode, textBuffer, existing, offset, nodeManager, node); } return Promise.resolve(result); } @@ -62,11 +63,13 @@ export class GxFormat2CompletionService { schemaNode: SchemaNode, textBuffer: TextBuffer, exclude: Set, - offset: number + offset: number, + nodeManager: ASTNodeManager, + node?: ASTNode ): Promise { const result: CompletionItem[] = []; const currentWord = textBuffer.getCurrentWord(offset); - const overwriteRange = textBuffer.getCurrentWordRange(offset); + let overwriteRange = textBuffer.getCurrentWordRange(offset); const position = textBuffer.getPosition(offset); const isPositionAfterColon = textBuffer.isPositionAfterToken(position, ":"); if (schemaNode instanceof EnumSchemaNode) { @@ -127,9 +130,17 @@ export class GxFormat2CompletionService { result.push(item); return result; } - if (schemaNode.name === "tool_id") { - if (currentWord && !currentWord.includes("/")) { - const tools = await this.toolshedService.searchToolsById(currentWord); + if ( + schemaNode.name === "tool_id" && + node && + node.type === "property" && + node.valueNode && + node.valueNode.type === "string" + ) { + const searchTerm = node.valueNode.value; + overwriteRange = nodeManager.getNodeRange(node.valueNode); + if (searchTerm && !searchTerm.includes("/")) { + const tools = await this.toolshedService.searchToolsById(searchTerm); for (const tool of tools) { const item: CompletionItem = this.buildCompletionItemFromTool(tool, overwriteRange); result.push(item); @@ -140,7 +151,7 @@ export class GxFormat2CompletionService { for (const typeRef of schemaNode.typeRefs) { const typeNode = this.schemaNodeResolver.getSchemaNodeByTypeRef(typeRef); if (typeNode === undefined) continue; - result.push(...(await this.getProposedItems(typeNode, textBuffer, exclude, offset))); + result.push(...(await this.getProposedItems(typeNode, textBuffer, exclude, offset, nodeManager, node))); } return result; } @@ -151,7 +162,7 @@ export class GxFormat2CompletionService { const schemaRecord = this.schemaNodeResolver.getSchemaNodeByTypeRef(schemaNode.typeRef); if (schemaRecord) { - return this.getProposedItems(schemaRecord, textBuffer, exclude, offset); + return this.getProposedItems(schemaRecord, textBuffer, exclude, offset, nodeManager, node); } } return result; diff --git a/server/gx-workflow-ls-format2/tests/integration/completion.test.ts b/server/gx-workflow-ls-format2/tests/integration/completion.test.ts index fb035d8..a14e7be 100644 --- a/server/gx-workflow-ls-format2/tests/integration/completion.test.ts +++ b/server/gx-workflow-ls-format2/tests/integration/completion.test.ts @@ -388,6 +388,11 @@ inputs: }); describe("Toolshed tool suggestions", () => { + beforeEach(() => { + searchToolsByIdMock.mockReset(); + searchToolsByIdMock.mockResolvedValue([]); + }); + it("should suggest toolshed tools when the cursor is inside the `tool_id` property and there is at least one character", async () => { const expectedTools = buildFakeToolInfoList([{ id: "tool1" }, { id: "tool2" }, { id: "tool3" }]); searchToolsByIdMock.mockResolvedValue(expectedTools); @@ -407,11 +412,21 @@ steps: const completionLabels = getCompletionItemsLabels(completions); expect(completionLabels).toEqual(expectedLabels); }); - const completionLabels = getCompletionItemsLabels(completions); - expect(completionLabels).toEqual(EXPECTED_COMPLETION_LABELS); - }); - it("should not suggest toolshed tools when the cursor is inside the `tool_id` property and there is no character", async () => { + it("should try to search for tools using the full value of `tool_id`", async () => { + const template = ` +class: GalaxyWorkflow +steps: + my_step: + tool_id: search for this$`; + const { contents, position } = parseTemplate(template); + + await getCompletions(contents, position); + + expect(searchToolsByIdMock).toHaveBeenCalledWith("search for this"); + }); + + it("should not try to search tools when the value in `tool_id` is empty", async () => { const template = ` class: GalaxyWorkflow steps: @@ -419,12 +434,12 @@ steps: tool_id: $`; const { contents, position } = parseTemplate(template); - const completions = await getCompletions(contents, position); + await getCompletions(contents, position); - expect(completions?.items).toHaveLength(0); + expect(searchToolsByIdMock).not.toHaveBeenCalled(); }); - it("should not suggest toolshed tools when the cursor is inside the `tool_id` property and the current word contains slashes", async () => { + it("should not try to search tools when the value in `tool_id` contains slashes", async () => { const template = ` class: GalaxyWorkflow steps: @@ -432,9 +447,9 @@ steps: tool_id: toolshed/owner/repo/tool$`; const { contents, position } = parseTemplate(template); - const completions = await getCompletions(contents, position); + await getCompletions(contents, position); - expect(completions?.items).toHaveLength(0); + expect(searchToolsByIdMock).not.toHaveBeenCalled(); }); }); }); From 969ffc4ff49030ee59002d16655611a11cfff8e4 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Sat, 20 Jul 2024 12:25:07 +0200 Subject: [PATCH 13/13] Do not restrict tool search to tool ID This will produce less accurate results but order of magnitude faster. There is probably something that needs to be tweaked in the whoosh search in the toolshed for this to work with IDs only. --- server/packages/server-common/src/services/toolShed.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/packages/server-common/src/services/toolShed.ts b/server/packages/server-common/src/services/toolShed.ts index f6252dd..354049e 100644 --- a/server/packages/server-common/src/services/toolShed.ts +++ b/server/packages/server-common/src/services/toolShed.ts @@ -71,7 +71,7 @@ export class ToolshedServiceImpl implements ToolshedService { const toolshedUrl = await this.validateToolshedUrl(); const toolsApiUrl = `${toolshedUrl}/api/tools`; const queryParams = new URLSearchParams({ - q: `id:${toolId}`, + q: toolId, page_size: limit.toString(), });