Skip to content

Commit

Permalink
task: make required optional
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
vince-fugnitto committed Sep 3, 2021
1 parent 068e78d commit dde9213
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<a name="breaking_changes_1.18.0">[Breaking Changes:](#breaking_changes_1.18.0)</a>

- [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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
8 changes: 4 additions & 4 deletions packages/task/src/browser/provided-task-configurations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
7 changes: 3 additions & 4 deletions packages/task/src/browser/task-configurations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions packages/task/src/browser/task-definition-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion packages/task/src/browser/task-schema-updater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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] };
Expand Down
6 changes: 5 additions & 1 deletion packages/task/src/common/task-protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit dde9213

Please sign in to comment.