Skip to content

Commit 84807cf

Browse files
committed
Bug fixes:
1. Made dependencies specified in plugins as soft dependencies 2. Fixed stderr bug on plugin-process. Stderr does not mean error. 3. Fixed in-degree bug on topological sort
1 parent 8d1c195 commit 84807cf

5 files changed

Lines changed: 11 additions & 79 deletions

File tree

src/entities/project.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,14 @@ export class Project {
3232
const resourceMap = new Map(this.resourceConfigs.map((r) => [r.id, r] as const));
3333

3434
this.resourceConfigs.forEach((r) => {
35-
r.parseDependenciesFromParameters((id) => resourceMap.has(id));
36-
r.addDependencies(dependencyMap.get(r.id) ?? []);
35+
// User specified dependencies are hard dependencies. They must be present.
36+
r.addDependenciesBasedOnParameters((id) => resourceMap.has(id));
37+
38+
// Plugin dependencies are soft dependencies. They only activate if the dependent resource is present.
39+
r.addDependencies(dependencyMap.get(r.id)
40+
?.filter((id) => resourceMap.has(id))
41+
?? []
42+
);
3743
})
3844
}
3945

src/entities/resource-config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export class ResourceConfig implements ConfigBlock {
6060
return this.name === null || this.name === undefined ? this.type : `${this.type}.${this.name}`;
6161
}
6262

63-
parseDependenciesFromParameters(resourceExists: (id: string) => boolean) {
63+
addDependenciesBasedOnParameters(resourceExists: (id: string) => boolean) {
6464
// TODO: Only string dependencies are supported currently
6565
const parametersWithDependencies = Object.entries(this.parameters)
6666
.filter(([, v]) => typeof v === 'string')

src/plugins/plugin-collection.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ export class PluginCollection {
5454

5555
const planResult = await this.plugins.get(pluginName)!.plan(config);
5656

57-
5857
result.push(planResult);
5958
}
6059

src/plugins/plugin-process.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ export class PluginProcess {
4545
const handler = new SendMessageForResultHandler(message, this.process, resolve, reject);
4646

4747
this.process.on('message', handler.messageListener);
48-
this.process.stderr!.on('data', handler.reject);
4948
this.process.send(message);
5049
});
5150
}
@@ -94,7 +93,6 @@ class SendMessageForResultHandler {
9493
}
9594

9695
this.process.removeListener('message', this.messageListener);
97-
this.process.stderr!.removeListener('data', this.reject);
9896
this.promiseReject(err);
9997
}
10098

@@ -104,12 +102,11 @@ class SendMessageForResultHandler {
104102
}
105103

106104
this.process.removeListener('message', this.messageListener);
107-
this.process.stderr!.removeListener('data', this.reject);
108105
this.promiseResolve(value);
109106
}
110107

111108
private setResultTimeout = (timeout: number) => setTimeout(() => {
112-
this.reject(new Error(`Plugin did not respond in 10s to call: ${this.messageToSend.cmd}`))
109+
this.reject(new Error(`Plugin did not respond in 10 minutes to call: ${this.messageToSend.cmd}`))
113110
}, timeout);
114111

115112
private validateIpcMessage(response: unknown): response is IpcMessage {

src/utils/dependency-graph-resolver.ts

Lines changed: 1 addition & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -45,86 +45,16 @@ export class DependencyGraphResolver {
4545
}
4646

4747
node.dependencies.forEach((parentNode) => {
48-
if (--parentNode.indegree) {
48+
if (--parentNode.indegree === 0) {
4949
queue.push(parentNode);
5050
}
5151
});
5252
result.push(node);
5353
}
5454

55-
// const { resourceConfigs } = project;
56-
// const resourceMap = new Map<string, ResourceConfig>(
57-
// resourceConfigs.map((r) => [r.id, r] as const)
58-
// );
59-
//
60-
// const resourceReferenceRegex = /\${([\w.]+)}/g
61-
//
62-
// // TODO: Support named resources in the future
63-
//
64-
// for (const config of resourceConfigs) {
65-
// const referenceParameters = findParametersWithReferences(new Map(Object.entries(config.parameters)))
66-
//
67-
// for (const [name, match] of referenceParameters) {
68-
// const parts = match.split('.');
69-
// if (parts.length < 2) {
70-
// throw new Error(`Only resource parameter references are allowed. ${match}`);
71-
// }
72-
//
73-
// const referencedId = findId(parts);
74-
// if (!referencedId) {
75-
// throw new Error(`Unable to find resource being referenced. ${match}`);
76-
// }
77-
//
78-
// const referencedResource = resourceMap.get(referencedId)
79-
// if (!referencedResource) {
80-
// throw new Error(`Unable to find resource being referenced. ${match}`);
81-
// }
82-
//
83-
// const referencedParameterName = findParameterName(parts, referencedId);
84-
// const referencedParameter = referencedResource.parameters.get(referencedParameterName);
85-
// if (!referencedParameter) {
86-
// throw new Error(`Un-able to find parameter being referenced. ${match}`);
87-
// }
88-
//
89-
// // TODO: Add recursive check for parameters of type parameter
90-
//
91-
// config.dependencies.push(referencedResource);
92-
//
93-
// // Substitute with actual value
94-
// config.parameters.set(name,
95-
// String(config.parameters.get(name)).replace(`\${${match}}`, String(referencedParameter))
96-
// );
97-
// }
98-
9955
return result.map((n) => n.val);
10056
}
10157

102-
// function findParametersWithReferences(parameters: Record<string, unknown>) {
103-
// return [...parameters.entries()]
104-
// .map(([name, value]) => [name, String(value), String(value).matchAll(resourceReferenceRegex)] as const)
105-
// .filter(([, _, match]) => match)
106-
// .flatMap(([name, _, matches]) =>
107-
// [...matches].map(match => [name, match[1]] as const)
108-
// );
109-
// }
110-
//
111-
// function findId(parts: string[]): null | string {
112-
// if (applyableGraph.has(parts[0])) {
113-
// return parts[0];
114-
// }
115-
//
116-
// if (applyableGraph.has(parts[0] + '.' + parts[1])) {
117-
// return parts[0] + '.' + parts[1];
118-
// }
119-
//
120-
// return null;
121-
// }
122-
//
123-
// function findParameterName(parts: string[], id: string): string {
124-
// return id.split('.').length === 1 ? parts[1] : parts[2];
125-
// }
126-
// };
127-
12858
private static populateNodeDependencies<T>(nodeMap: Map<string, Node<T>>, getDependencyIds: (t: T) => string[]) {
12959
[...nodeMap.values()].forEach((n) => {
13060
const dependencies = getDependencyIds(n.val)

0 commit comments

Comments
 (0)