Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement pipeMode Parameter to EvalCommand #346

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 17 additions & 2 deletions ts/packages/ts/src/commands/EvalCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ export enum EvaluatedConfigOrigin {
EmptyValue = 'EMPTY_VALUE',
}

export type EvalCommandPipeMode = 'include' | 'forward';

export type EvalCommandReturn = {
[key: string]: {
context: { store: string; set: string; schema: string; key: string; cfgu: Cfgu };
context: { store: string; set: string; schema: string; key: string; cfgu: Cfgu; pipeMode: EvalCommandPipeMode };
result: { origin: EvaluatedConfigOrigin; source: string; value: string };
};
};
Expand All @@ -28,6 +30,7 @@ export type EvalCommandParameters = {
configs?: { [key: string]: string };
pipe?: EvalCommandReturn;
validate?: boolean;
pipeMode?: EvalCommandPipeMode;
};

export class EvalCommand extends Command<EvalCommandReturn> {
Expand Down Expand Up @@ -224,6 +227,15 @@ export class EvalCommand extends Command<EvalCommandReturn> {
return resultWithTemplates;
}

private removeForwards(result: EvalCommandReturn): EvalCommandReturn {
const { pipe } = this.parameters;
if (!pipe) {
return result;
}
console.log(result);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

return _.omitBy(result, (current) => pipe[current.context.key] && current.context.pipeMode === 'forward');
}

private validateResult(result: EvalCommandReturn): void {
const { validate = true } = this.parameters;

Expand Down Expand Up @@ -265,7 +277,7 @@ export class EvalCommand extends Command<EvalCommandReturn> {
}

async run() {
const { store, set, schema } = this.parameters;
const { store, set, schema, pipeMode = 'include' } = this.parameters;

await store.init();

Expand All @@ -277,6 +289,7 @@ export class EvalCommand extends Command<EvalCommandReturn> {
schema: schema.name,
key,
cfgu,
pipeMode,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a mistake that leads to a bug. Consider a scenario where the 2nd eval also forwards the same keys as the previous, this case will lead to all the keys being lost due to the logic inside of the removal method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good one 💯
Thanks. I'l revise and test

},
result: {
origin: EvaluatedConfigOrigin.EmptyValue,
Expand Down Expand Up @@ -308,6 +321,8 @@ export class EvalCommand extends Command<EvalCommandReturn> {
...this.evalTemplates(result),
};

result = this.removeForwards(result);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thats should be after validation as all Configs must be validated


this.validateResult(result);

return result;
Expand Down
145 changes: 145 additions & 0 deletions ts/packages/ts/src/commands/commands.test.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like in the other PR, let's recycle the EvalCommand usages into a function to keep this file leaner

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what other PR?
please elaborate and be specific here. PR's are not related.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please adjust the schema examples to be more generic, perhaps we should always do tests on start.cfgu.json.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
type EvalCommandParameters,
DeleteCommand,
type EvalCommandReturn,
ExportCommand,
} from '..';

describe(`commands`, () => {
Expand Down Expand Up @@ -221,5 +222,149 @@ describe(`commands`, () => {
await Promise.all(deletePromises);
}
});
describe('PipeMode', () => {
test('run EvalCommand and pipe to another, all keys should exist in export', async () => {
const evalMyName = await new EvalCommand({
store: store1,
set: set1,
schema: new ConfigSchema('myName', {
MY_NAME: {
type: 'String',
},
}),
configs: {
MY_NAME: 'What?',
},
}).run();
const evalQuote = await new EvalCommand({
store: store1,
set: set1,
schema: new ConfigSchema('quote', {
QUOTE: {
type: 'String',
template: 'Hi! My name is {{MY_NAME}}, my name is Who?',
},
}),
pipe: evalMyName,
}).run();
const evaluatedConfigs = await new ExportCommand({ pipe: evalQuote }).run();
expect(evaluatedConfigs).toStrictEqual({ MY_NAME: 'What?', QUOTE: 'Hi! My name is What?, my name is Who?' });
});
test("run EvalCommand(pipeMode='forward') and pipe to another, only keys from the second should exist in export", async () => {
const evalMyName = await new EvalCommand({
store: store1,
set: set1,
schema: new ConfigSchema('myName', {
MY_NAME: {
type: 'String',
},
}),
configs: {
MY_NAME: 'What?',
},
pipeMode: 'forward',
}).run();
const evalQuote = await new EvalCommand({
store: store1,
set: set1,
schema: new ConfigSchema('quote', {
QUOTE: {
type: 'String',
template: 'Hi! My name is {{MY_NAME}}, my name is Who?',
},
}),
pipe: evalMyName,
}).run();
const evaluatedConfigs = await new ExportCommand({ pipe: evalQuote }).run();
expect(evaluatedConfigs).toStrictEqual({ QUOTE: 'Hi! My name is What?, my name is Who?' });
});
test("run EvalCommand(pipeMode='forward') and pipe to two other evals, only keys from the second should exist in export", async () => {
const evalMyName = await new EvalCommand({
store: store1,
set: set1,
schema: new ConfigSchema('myName', {
MY_NAME: {
type: 'String',
},
}),
configs: {
MY_NAME: 'What?',
},
pipeMode: 'forward',
}).run();
const evalQuote = await new EvalCommand({
store: store1,
set: set1,
schema: new ConfigSchema('quote', {
QUOTE: {
type: 'String',
template: 'Hi! My name is {{MY_NAME}}, my name is Who?',
},
}),
pipeMode: 'forward',
pipe: evalMyName,
}).run();
const lastEval = await new EvalCommand({
store: store1,
set: set1,
schema: new ConfigSchema('last', {
MY_QUOTE: {
type: 'String',
template: '{{QUOTE}} My Name is Chiky Chicky Slim Shady',
},
}),
pipeMode: 'forward',
pipe: evalQuote,
}).run();
const evaluatedConfigs = await new ExportCommand({ pipe: lastEval }).run();
expect(evaluatedConfigs).toStrictEqual({
MY_QUOTE: 'Hi! My name is , my name is Who? My Name is Chiky Chicky Slim Shady',
});
});
test('check for the scenario where a 2nd eval overrides the same key from a forwarded eval and ensure that the key is still there', async () => {
const evalMyName = await new EvalCommand({
store: store1,
set: set1,
schema: new ConfigSchema('myName', {
MY_NAME: {
type: 'String',
},
}),
configs: {
MY_NAME: 'Who?',
},
pipeMode: 'forward',
}).run();
const evalMyNameAgain = await new EvalCommand({
store: store1,
set: set1,
schema: new ConfigSchema('myName', {
MY_NAME: {
type: 'String',
},
}),
configs: {
MY_NAME: 'What?',
},
pipeMode: 'forward',
pipe: evalMyName,
}).run();
const lastEval = await new EvalCommand({
store: store1,
set: set1,
schema: new ConfigSchema('last', {
MY_QUOTE: {
type: 'String',
template: 'Hi! My name is {{MY_NAME}}, my name is Who?',
},
}),
pipe: evalMyNameAgain,
}).run();
const evaluatedConfigs = await new ExportCommand({ pipe: lastEval }).run();
expect(evaluatedConfigs).toStrictEqual({
MY_QUOTE: 'Hi! My name is What?, my name is Who?',
});
});
});
});
});