From dde92135be460e60fc21297c74b2b45b391da10a Mon Sep 17 00:00:00 2001 From: vince-fugnitto Date: Tue, 31 Aug 2021 13:50:55 -0400 Subject: [PATCH] task: make `required` optional The commit makes `TaskDefinition.properties.required` optional based on the json schema validation definition. Previously, if an extension defines a schema that omits the `required` property the application will throw errors and fail to display the menu. The change treats the missing `required` property as an empty array as per the spec. Signed-off-by: vince-fugnitto --- CHANGELOG.md | 1 + .../plugin-ext/src/hosted/node/scanners/scanner-theia.ts | 2 +- packages/task/src/browser/provided-task-configurations.ts | 8 ++++---- packages/task/src/browser/task-configurations.ts | 7 +++---- packages/task/src/browser/task-definition-registry.ts | 8 ++++---- packages/task/src/browser/task-schema-updater.ts | 3 ++- packages/task/src/common/task-protocol.ts | 6 +++++- 7 files changed, 20 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18a5cb905b259..c2f73de501c45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ [Breaking Changes:](#breaking_changes_1.18.0) - [core] added `BreadcrumbsRendererFactory` to constructor arguments of `DockPanelRenderer` and `ToolbarAwareTabBar`. [#9920](https://github.com/eclipse-theia/theia/pull/9920) +- [task] `TaskDefinition.properties.required` is now optional to align with the specification [#10015](https://github.com/eclipse-theia/theia/pull/10015) ## v1.17.2 - 9/1/2021 diff --git a/packages/plugin-ext/src/hosted/node/scanners/scanner-theia.ts b/packages/plugin-ext/src/hosted/node/scanners/scanner-theia.ts index 276ae053314e1..4a5b047947b63 100644 --- a/packages/plugin-ext/src/hosted/node/scanners/scanner-theia.ts +++ b/packages/plugin-ext/src/hosted/node/scanners/scanner-theia.ts @@ -698,7 +698,7 @@ export class TheiaPluginScanner implements PluginScanner { taskType: definitionContribution.type, source: pluginName, properties: { - required: definitionContribution.required, + required: definitionContribution.required || [], all: propertyKeys, schema: definitionContribution } diff --git a/packages/task/src/browser/provided-task-configurations.ts b/packages/task/src/browser/provided-task-configurations.ts index 5253e63310d1f..87db0bc09faf3 100644 --- a/packages/task/src/browser/provided-task-configurations.ts +++ b/packages/task/src/browser/provided-task-configurations.ts @@ -101,12 +101,12 @@ export class ProvidedTaskConfigurations { let highest = -1; const tasks = await this.getTasks(token); for (const task of tasks) { // find detected tasks that match the `definition` - let score = 0; - if (!definition.properties.required.every(requiredProp => customization[requiredProp] !== undefined)) { + const required = definition.properties.required || []; + if (!required.every(requiredProp => customization[requiredProp] !== undefined)) { continue; } - score += definition.properties.required.length; // number of required properties - const requiredProps = new Set(definition.properties.required); + let score = required.length; // number of required properties + const requiredProps = new Set(required); // number of optional properties score += definition.properties.all.filter(p => !requiredProps.has(p) && customization[p] !== undefined).length; if (score >= highest) { diff --git a/packages/task/src/browser/task-configurations.ts b/packages/task/src/browser/task-configurations.ts index 26fe7af24d574..5642b87ec550d 100644 --- a/packages/task/src/browser/task-configurations.ts +++ b/packages/task/src/browser/task-configurations.ts @@ -249,10 +249,9 @@ export class TaskConfigurations implements Disposable { if (hasCustomization) { const taskDefinition = this.taskDefinitionRegistry.getDefinition(taskConfig); if (taskDefinition) { - const cus = customizationByType.filter(customization => - taskDefinition.properties.required.every(rp => customization[rp] === taskConfig[rp]) - )[0]; // Only support having one customization per task - return cus; + const required = taskDefinition.properties.required || []; + // Only support having one customization per task. + return customizationByType.find(customization => required.every(property => customization[property] === taskConfig[property])); } } return undefined; diff --git a/packages/task/src/browser/task-definition-registry.ts b/packages/task/src/browser/task-definition-registry.ts index ae8154de23d2a..a35b4693af305 100644 --- a/packages/task/src/browser/task-definition-registry.ts +++ b/packages/task/src/browser/task-definition-registry.ts @@ -71,12 +71,12 @@ export class TaskDefinitionRegistry { let matchedDefinition: TaskDefinition | undefined; let highest = -1; for (const def of definitions) { - let score = 0; - if (!def.properties.required.every(requiredProp => taskConfiguration[requiredProp] !== undefined)) { + const required = def.properties.required || []; + if (!required.every(requiredProp => taskConfiguration[requiredProp] !== undefined)) { continue; } - score += def.properties.required.length; // number of required properties - const requiredProps = new Set(def.properties.required); + let score = required.length; // number of required properties + const requiredProps = new Set(required); // number of optional properties score += def.properties.all.filter(p => !requiredProps.has(p) && taskConfiguration[p] !== undefined).length; if (score > highest) { diff --git a/packages/task/src/browser/task-schema-updater.ts b/packages/task/src/browser/task-schema-updater.ts index 0c698af967730..bd07660b74672 100644 --- a/packages/task/src/browser/task-schema-updater.ts +++ b/packages/task/src/browser/task-schema-updater.ts @@ -168,8 +168,9 @@ export class TaskSchemaUpdater implements JsonSchemaContribution { description: 'The task type to customize' }; customizedDetectedTask.properties!.type = taskType; + const required = def.properties.required || []; def.properties.all.forEach(taskProp => { - if (!!def.properties.required.find(requiredProp => requiredProp === taskProp)) { // property is mandatory + if (required.find(requiredProp => requiredProp === taskProp)) { // property is mandatory customizedDetectedTask.required!.push(taskProp); } customizedDetectedTask.properties![taskProp] = { ...def.properties.schema.properties![taskProp] }; diff --git a/packages/task/src/common/task-protocol.ts b/packages/task/src/common/task-protocol.ts index 548851ec312ec..6ec6b40756b78 100644 --- a/packages/task/src/common/task-protocol.ts +++ b/packages/task/src/common/task-protocol.ts @@ -274,7 +274,11 @@ export interface TaskDefinition { taskType: string; source: string; properties: { - required: string[]; + /** + * Should be treated as an empty array if omitted. + * https://json-schema.org/draft/2020-12/json-schema-validation.html#rfc.section.6.5.3 + */ + required?: string[]; all: string[]; schema: IJSONSchema; }