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 Aug 31, 2021
1 parent 83ecbc3 commit dbed5c4
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 12 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Change Log

## v1.17.1 - 8/31/2021

[1.17.0 Milestone](https://github.com/eclipse-theia/theia/milestone/23)

<a name="breaking_changes_1.17.1">[Breaking Changes:](#breaking_changes_1.17.1)</a>

- [task] `TaskDefinition.properties.required` is now optional to align with the specification [#10015](https://github.com/eclipse-theia/theia/pull/10015)

## v1.17.0 - 8/26/2021

[1.17.0 Milestone](https://github.com/eclipse-theia/theia/milestone/23)
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
2 changes: 1 addition & 1 deletion packages/task/src/browser/task-configurations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ export class TaskConfigurations implements Disposable {
const taskDefinition = this.taskDefinitionRegistry.getDefinition(taskConfig);
if (taskDefinition) {
const cus = customizationByType.filter(customization =>
taskDefinition.properties.required.every(rp => customization[rp] === taskConfig[rp])
taskDefinition.properties.required?.every(rp => customization[rp] === taskConfig[rp])
)[0]; // Only support having one customization per task
return cus;
}
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 dbed5c4

Please sign in to comment.