Skip to content

Commit 8a0c466

Browse files
kevinwang5658kevinwang5658
authored
Kevin/bug fixes (#10)
* Fixed input transformations * Fixed bug is stateful-parameter isEqual types * Fixed bug with isElementEqual in stateless mode. Correct filtering of the current array should now be applied (it will only keep the first element that equals the desired even if there are multiple) * Bumped node types version * Passed user input to stateful parameters and added allowUnionTypes to ajv validation * Apply input transformations before passing desiredConfig to validate * Added input transformations for validate and stateConfigs
1 parent f8fab8d commit 8a0c466

13 files changed

+313
-112
lines changed

‎package-lock.json‎

Lines changed: 10 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "codify-plugin-lib",
3-
"version": "1.0.77",
3+
"version": "1.0.88",
44
"description": "Library plugin library",
55
"main": "dist/index.js",
66
"typings": "dist/index.d.ts",
@@ -22,7 +22,7 @@
2222
"@oclif/prettier-config": "^0.2.1",
2323
"@oclif/test": "^3",
2424
"@types/npmcli__promise-spawn": "^6.0.3",
25-
"@types/node": "^18",
25+
"@types/node": "^20",
2626
"@types/semver": "^7.5.4",
2727
"@types/sinon": "^17.0.3",
2828
"@types/uuid": "^10.0.0",

‎src/plan/change-set.test.ts‎

Lines changed: 83 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { ChangeSet } from './change-set.js';
22
import { ParameterOperation, ResourceOperation } from 'codify-schemas';
33
import { describe, expect, it } from 'vitest';
4+
import { ParsedResourceSettings } from '../resource/parsed-resource-settings.js';
45

56
describe('Change set tests', () => {
67
it ('Correctly diffs two resource configs (modify)', () => {
@@ -31,7 +32,7 @@ describe('Change set tests', () => {
3132
propA: 'after',
3233
}
3334

34-
const cs = ChangeSet.calculateModification(after, before,);
35+
const cs = ChangeSet.calculateModification(after, before);
3536
expect(cs.parameterChanges.length).to.eq(2);
3637
expect(cs.parameterChanges[0].operation).to.eq(ParameterOperation.MODIFY);
3738
expect(cs.parameterChanges[1].operation).to.eq(ParameterOperation.ADD);
@@ -104,7 +105,14 @@ describe('Change set tests', () => {
104105
propA: ['b', 'a', 'c'],
105106
}
106107

107-
const cs = ChangeSet.calculateModification(after, before, { propA: { type: 'array' } });
108+
const parameterSettings = new ParsedResourceSettings({
109+
id: 'type',
110+
parameterSettings: {
111+
propA: { type: 'array' }
112+
}
113+
}).parameterSettings
114+
115+
const cs = ChangeSet.calculateModification(after, before, parameterSettings);
108116
expect(cs.parameterChanges.length).to.eq(1);
109117
expect(cs.parameterChanges[0].operation).to.eq(ParameterOperation.NOOP);
110118
expect(cs.operation).to.eq(ResourceOperation.NOOP)
@@ -119,7 +127,14 @@ describe('Change set tests', () => {
119127
propA: ['b', 'a'],
120128
}
121129

122-
const cs = ChangeSet.calculateModification(after, before, { propA: { type: 'array' } });
130+
const parameterSettings = new ParsedResourceSettings({
131+
id: 'type',
132+
parameterSettings: {
133+
propA: { type: 'array' }
134+
}
135+
}).parameterSettings
136+
137+
const cs = ChangeSet.calculateModification(after, before, parameterSettings);
123138
expect(cs.parameterChanges.length).to.eq(1);
124139
expect(cs.parameterChanges[0].operation).to.eq(ParameterOperation.MODIFY);
125140
expect(cs.operation).to.eq(ResourceOperation.RECREATE)
@@ -135,7 +150,14 @@ describe('Change set tests', () => {
135150
propB: 'before'
136151
}
137152

138-
const cs = ChangeSet.calculateModification(after, before, { propA: { canModify: true } });
153+
const parameterSettings = new ParsedResourceSettings({
154+
id: 'type',
155+
parameterSettings: {
156+
propA: { canModify: true }
157+
}
158+
}).parameterSettings
159+
160+
const cs = ChangeSet.calculateModification(after, before, parameterSettings);
139161
expect(cs.parameterChanges.length).to.eq(2);
140162
expect(cs.parameterChanges[0].operation).to.eq(ParameterOperation.MODIFY);
141163
expect(cs.parameterChanges[1].operation).to.eq(ParameterOperation.REMOVE);
@@ -152,10 +174,15 @@ describe('Change set tests', () => {
152174
propB: 'before'
153175
}
154176

155-
const cs = ChangeSet.calculateModification<any>(after, before, {
156-
propA: { canModify: true },
157-
propB: { canModify: true }
158-
});
177+
const parameterSettings = new ParsedResourceSettings({
178+
id: 'type',
179+
parameterSettings: {
180+
propA: { canModify: true },
181+
propB: { canModify: true }
182+
},
183+
}).parameterSettings
184+
185+
const cs = ChangeSet.calculateModification<any>(after, before, parameterSettings);
159186
expect(cs.parameterChanges.length).to.eq(2);
160187
expect(cs.parameterChanges[0].operation).to.eq(ParameterOperation.MODIFY);
161188
expect(cs.parameterChanges[1].operation).to.eq(ParameterOperation.REMOVE);
@@ -167,7 +194,15 @@ describe('Change set tests', () => {
167194
const arrA = ['a', 'b', 'd'];
168195
const arrB = ['a', 'b', 'd'];
169196

170-
const result = ChangeSet.calculateModification({ propA: arrA }, { propA: arrB }, { propA: { type: 'array' } })
197+
const parameterSettings = new ParsedResourceSettings({
198+
id: 'type',
199+
parameterSettings: {
200+
propA: { type: 'array' }
201+
},
202+
}).parameterSettings
203+
204+
205+
const result = ChangeSet.calculateModification({ propA: arrA }, { propA: arrB }, parameterSettings)
171206

172207
expect(result.operation).to.eq(ResourceOperation.NOOP);
173208
})
@@ -176,7 +211,14 @@ describe('Change set tests', () => {
176211
const arrA = ['a', 'b'];
177212
const arrB = ['a', 'b', 'd'];
178213

179-
const result = ChangeSet.calculateModification({ propA: arrA }, { propA: arrB }, { propA: { type: 'array' } })
214+
const parameterSettings = new ParsedResourceSettings({
215+
id: 'type',
216+
parameterSettings: {
217+
propA: { type: 'array' }
218+
},
219+
}).parameterSettings
220+
221+
const result = ChangeSet.calculateModification({ propA: arrA }, { propA: arrB }, parameterSettings)
180222

181223
expect(result.parameterChanges[0].operation).to.eq(ParameterOperation.MODIFY);
182224
})
@@ -185,7 +227,14 @@ describe('Change set tests', () => {
185227
const arrA = ['b', 'a', 'd'];
186228
const arrB = ['a', 'b', 'd'];
187229

188-
const result = ChangeSet.calculateModification({ propA: arrA }, { propA: arrB }, { propA: { type: 'array' } })
230+
const parameterSettings = new ParsedResourceSettings({
231+
id: 'type',
232+
parameterSettings: {
233+
propA: { type: 'array' }
234+
},
235+
}).parameterSettings
236+
237+
const result = ChangeSet.calculateModification({ propA: arrA }, { propA: arrB }, parameterSettings)
189238

190239
expect(result.parameterChanges[0].operation).to.eq(ParameterOperation.NOOP);
191240
})
@@ -194,12 +243,18 @@ describe('Change set tests', () => {
194243
const arrA = [{ key1: 'a' }, { key1: 'a' }, { key1: 'a' }];
195244
const arrB = [{ key1: 'a' }, { key1: 'a' }, { key1: 'b' }];
196245

197-
const result = ChangeSet.calculateModification({ propA: arrA }, { propA: arrB }, {
198-
propA: {
199-
type: 'array',
200-
isElementEqual: (a, b) => a.key1 === b.key1
201-
}
202-
})
246+
const parameterSettings = new ParsedResourceSettings({
247+
id: 'type',
248+
parameterSettings: {
249+
propA: {
250+
type: 'array',
251+
isElementEqual: (a, b) => a.key1 === b.key1
252+
}
253+
},
254+
}).parameterSettings
255+
256+
257+
const result = ChangeSet.calculateModification({ propA: arrA }, { propA: arrB }, parameterSettings)
203258

204259
expect(result.parameterChanges[0].operation).to.eq(ParameterOperation.MODIFY);
205260
})
@@ -208,12 +263,17 @@ describe('Change set tests', () => {
208263
const arrA = [{ key1: 'b' }, { key1: 'a' }, { key1: 'a' }];
209264
const arrB = [{ key1: 'a' }, { key1: 'a' }, { key1: 'b' }];
210265

211-
const result = ChangeSet.calculateModification({ propA: arrA }, { propA: arrB }, {
212-
propA: {
213-
type: 'array',
214-
isElementEqual: (a, b) => a.key1 === b.key1
215-
}
216-
})
266+
const parameterSettings = new ParsedResourceSettings({
267+
id: 'type',
268+
parameterSettings: {
269+
propA: {
270+
type: 'array',
271+
isElementEqual: (a, b) => a.key1 === b.key1
272+
}
273+
},
274+
}).parameterSettings
275+
276+
const result = ChangeSet.calculateModification({ propA: arrA }, { propA: arrB }, parameterSettings)
217277

218278
expect(result.parameterChanges[0].operation).to.eq(ParameterOperation.NOOP);
219279
})

‎src/plan/change-set.ts‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { ParameterOperation, ResourceOperation, StringIndexedObject } from 'codify-schemas';
22

3-
import { ArrayParameterSetting, ParameterSetting, StatefulParameterSetting } from '../resource/resource-settings.js';
4-
import { areArraysEqual } from '../utils/utils.js';
3+
import { ParameterSetting } from '../resource/resource-settings.js';
54

65
/**
76
* A parameter change describes a parameter level change to a resource.
@@ -116,6 +115,16 @@ export class ChangeSet<T extends StringIndexedObject> {
116115
return new ChangeSet<T>(resourceOperation, pc);
117116
}
118117

118+
/**
119+
* Calculates the differences between the desired and current parameters,
120+
* and returns a list of parameter changes that describe what needs to be added,
121+
* removed, or modified to match the desired state.
122+
*
123+
* @param {Partial<T>} desiredParameters - The desired target state of the parameters.
124+
* @param {Partial<T>} currentParameters - The current state of the parameters.
125+
* @param {Partial<Record<keyof T, ParameterSetting>>} [parameterOptions] - Optional settings used when comparing parameters.
126+
* @return {ParameterChange<T>[]} A list of changes required to transition from the current state to the desired state.
127+
*/
119128
private static calculateParameterChanges<T extends StringIndexedObject>(
120129
desiredParameters: Partial<T>,
121130
currentParameters: Partial<T>,
@@ -205,21 +214,6 @@ export class ChangeSet<T extends StringIndexedObject> {
205214
current: unknown,
206215
setting?: ParameterSetting,
207216
): boolean {
208-
switch (setting?.type) {
209-
case 'stateful': {
210-
const statefulSetting = (setting as StatefulParameterSetting).definition.getSettings()
211-
212-
return ChangeSet.isSame(desired, current, statefulSetting as ParameterSetting);
213-
}
214-
215-
case 'array': {
216-
const arrayParameter = setting as ArrayParameterSetting;
217-
return areArraysEqual(arrayParameter, desired, current)
218-
}
219-
220-
default: {
221-
return (setting?.isEqual ?? ((a, b) => a === b))(desired, current)
222-
}
223-
}
217+
return (setting?.isEqual ?? ((a, b) => a === b))(desired, current)
224218
}
225219
}

‎src/plan/plan.ts‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,16 +253,31 @@ export class Plan<T extends StringIndexedObject> {
253253
&& Array.isArray(v)
254254
}
255255

256+
// For stateless mode, we must filter the current array so that the diff algorithm will not detect any deletes
256257
function filterArrayStatefulParameter(k: string, v: unknown[]): unknown[] {
257258
const desiredArray = desired![k] as unknown[];
258259
const matcher = ((settings.parameterSettings![k] as StatefulParameterSetting)
259260
.definition
260261
.getSettings() as ArrayParameterSetting)
261-
.isElementEqual;
262+
.isElementEqual ?? ((a, b) => a === b);
262263

263-
return v.filter((cv) =>
264-
desiredArray.find((dv) => (matcher ?? ((a: any, b: any) => a === b))(dv, cv))
265-
)
264+
const desiredCopy = [...desiredArray];
265+
const currentCopy = [...v];
266+
const result = [];
267+
268+
for (let counter = desiredCopy.length - 1; counter >= 0; counter--) {
269+
const idx = currentCopy.findIndex((e2) => matcher(desiredCopy[counter], e2))
270+
271+
if (idx === -1) {
272+
continue;
273+
}
274+
275+
desiredCopy.splice(counter, 1)
276+
const [element] = currentCopy.splice(idx, 1)
277+
result.push(element)
278+
}
279+
280+
return result;
266281
}
267282
}
268283

‎src/plugin/plugin.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
import { Plan } from '../plan/plan.js';
1212
import { Resource } from '../resource/resource.js';
1313
import { ResourceController } from '../resource/resource-controller.js';
14-
import { splitUserConfig } from '../utils/utils.js';
1514

1615
export class Plugin {
1716
planStorage: Map<string, Plan<any>>;
@@ -55,10 +54,9 @@ export class Plugin {
5554
throw new Error(`Resource type not found: ${config.type}`);
5655
}
5756

58-
const { parameters, coreParameters } = splitUserConfig(config);
5957
const validation = await this.resourceControllers
6058
.get(config.type)!
61-
.validate(parameters, coreParameters);
59+
.validate(config);
6260

6361
validationResults.push(validation);
6462
}

0 commit comments

Comments
 (0)