Skip to content

Commit

Permalink
fix: replace adaptors in plan with resolved adaptors at install (#857)
Browse files Browse the repository at this point in the history
* fix: replace adaptors in plan to what they resolved to

This mostly effects adaptors specified with aliases like common@latest which will be replaced to the what they resolved to like [email protected]

* test: override plan test

* test: check for resolved with number to be sure linking stage passed

* test: a workflow using different versions of the same adaptor

* version: [email protected]

---------

Co-authored-by: Joe Clark <[email protected]>
  • Loading branch information
doc-han and josephjclark authored Jan 17, 2025
1 parent a1a071b commit 0337621
Show file tree
Hide file tree
Showing 8 changed files with 224 additions and 2 deletions.
15 changes: 14 additions & 1 deletion integration-tests/cli/test/autoinstall.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ test.serial(
t.regex(stdout, /Installing packages.../);
t.regex(stdout, /Will install @openfn\/language-common version/);
t.regex(stdout, /Installed @openfn\/language-common@/);
t.regex(
stdout,
/Resolved adaptor @openfn\/language-common to version \d+(\.\d+)*/
);
}
);

Expand All @@ -68,6 +72,10 @@ test.serial(
stdout,
/Skipping @openfn\/language-common@(.+) as already installed/
);
t.regex(
stdout,
/Resolved adaptor @openfn\/language-common to version \d+(\.\d+)*/
);
}
);

Expand All @@ -78,7 +86,6 @@ test.serial(
const { stdout, err } = await run(t.title);

// t.falsy(err); // TODO I think this is a broken adaptor build?

t.regex(stdout, /Auto-installing language adaptors/);
t.regex(stdout, /Looked up latest version of @openfn\/language-testing/);
t.regex(stdout, /Installing packages.../);
Expand All @@ -92,6 +99,12 @@ test.serial(
stdout,
new RegExp(`Installed @openfn\/language-testing@${TEST_LATEST}`)
);
t.regex(
stdout,
new RegExp(
`Resolved adaptor @openfn/language-testing to version ${TEST_LATEST}`
)
);
}
);

Expand Down
22 changes: 22 additions & 0 deletions integration-tests/cli/test/execute-workflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,25 @@ test.serial(
});
}
);

test.serial(
`openfn ${jobsPath}/different-adaptor-versions.json -l debug`,
async (t) => {
const { err, stdout } = await run(t.title);
t.falsy(err);

t.regex(
stdout,
/Resolved adaptor @openfn\/language-common to version 2.1.0/
);
t.regex(
stdout,
/Resolved adaptor @openfn\/language-common to version 2.0.3/
);

const out = getJSON();
t.deepEqual(out, {
z: 1,
});
}
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"options": {},
"workflow": {
"name": "Steps with different adaptor versions",
"steps": [
{
"id": "lesser",
"adaptor": "[email protected]",
"state": {
"x": 1
},
"expression": "fn(state=> ({y: state.x}))",
"next": {
"latest-again": true
}
},
{
"id": "latest-again",
"adaptor": "[email protected]",
"expression": "fn(state=> ({z: state.y}))"
}
]
}
}
6 changes: 6 additions & 0 deletions packages/cli/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# @openfn/cli

## 1.10.2

### Patch Changes

- Properly resolve adaptor versions when they are explicitly set to @latest

## 1.10.1

### Patch Changes
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@openfn/cli",
"version": "1.10.1",
"version": "1.10.2",
"description": "CLI devtools for the openfn toolchain.",
"engines": {
"node": ">=18",
Expand Down
13 changes: 13 additions & 0 deletions packages/cli/src/execute/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { clearCache } from '../util/cache';
import fuzzyMatchStep from '../util/fuzzy-match-step';
import abort from '../util/abort';
import validatePlan from '../util/validate-plan';
import overridePlanAdaptors from '../util/override-plan-adaptors';

const matchStep = (
plan: ExecutionPlan,
Expand Down Expand Up @@ -55,6 +56,7 @@ const executeHandler = async (options: ExecuteOptions, logger: Logger) => {
await clearCache(plan, options, logger);
}

const moduleResolutions: Record<string, string> = {};
const { repoDir, monorepoPath, autoinstall } = options;
if (autoinstall) {
if (monorepoPath) {
Expand All @@ -67,6 +69,14 @@ const executeHandler = async (options: ExecuteOptions, logger: Logger) => {
{ packages: autoInstallTargets, repoDir },
logger
);

// create a module resolution dictionary.
// this is to map aliases like @latest & @next to what they resolved into
if (autoInstallTargets.length === options.adaptors.length) {
for (let i = 0; i < autoInstallTargets.length; i++) {
moduleResolutions[autoInstallTargets[i]] = options.adaptors[i];
}
}
}
}
}
Expand Down Expand Up @@ -104,6 +114,9 @@ const executeHandler = async (options: ExecuteOptions, logger: Logger) => {
}
const state = await loadState(plan, options, logger, customStart);

// replacing adaptors in the original plan to what they resolved to.
plan = overridePlanAdaptors(plan, moduleResolutions);

if (options.compile) {
plan = await compile(plan, options, logger);
} else {
Expand Down
34 changes: 34 additions & 0 deletions packages/cli/src/util/override-plan-adaptors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { ExecutionPlan, Job, Step } from '@openfn/lexicon';

function overridePlanAdaptors(
plan: ExecutionPlan,
resolutions: Record<string, string>
): ExecutionPlan {
const hasRes = Object.entries(resolutions).some(
([key, value]) => key !== value
);

// there's nothing to override when resolutions have the same values
if (!hasRes) return plan;
return {
...plan,
workflow: {
...plan.workflow,
steps: plan.workflow.steps.map((step) => {
if (isJob(step))
return {
...step,
adaptors: step.adaptors?.map((a) => resolutions[a] || a),
};
else return step;
}),
},
};
}

export function isJob(step: Step): step is Job {
// @ts-ignore
return step && typeof step.expression === 'string';
}

export default overridePlanAdaptors;
110 changes: 110 additions & 0 deletions packages/cli/test/util/override-plan-adaptor.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import { ExecutionPlan } from '@openfn/lexicon';
import test from 'ava';
import overridePlanAdaptors, {
isJob,
} from '../../src/util/override-plan-adaptors';

// validates adaptors in a plan against an array.
function getAdaptors(plan: ExecutionPlan) {
// check for a given id whether the
const steps = plan.workflow.steps;

const adaptors = steps.map((step) => {
if (isJob(step)) return step.adaptors;
return undefined;
});
return adaptors;
}

test('replace adaptors in plan using provided resolutions', (t) => {
const plan: ExecutionPlan = {
workflow: {
steps: [
{
id: 'one',
expression: 'fn(state=> state)',
adaptors: ['@openfn/language-http@latest'],
},
{
id: 'two',
expression: 'fn(state=> state)',
adaptors: ['@openfn/language-common@next'],
},
{
expression: 'fn(state=> state)',
adaptors: ['@openfn/[email protected]'],
},
],
},
};

const resolutions = {
'@openfn/language-http@latest': '@openfn/[email protected]',
'@openfn/language-common@next': '@openfn/[email protected]',
};
const finalPlan = overridePlanAdaptors(plan, resolutions);

t.deepEqual(getAdaptors(finalPlan), [
['@openfn/[email protected]'],
['@openfn/[email protected]'],
['@openfn/[email protected]'],
]);
});

test("ignore override when there's nothing to override", (t) => {
const plan: ExecutionPlan = {
workflow: {
steps: [
{
id: 'one',
expression: 'fn(state=> state)',
adaptors: ['@openfn/language-http@latest'],
},
{
id: 'two',
expression: 'fn(state=> state)',
adaptors: ['@openfn/language-common@next'],
},
{
expression: 'fn(state=> state)',
adaptors: ['@openfn/[email protected]'],
},
],
},
};

const resolutions = {
'@openfn/[email protected]': '@openfn/[email protected]',
'@openfn/[email protected]': '@openfn/[email protected]',
};

const finalPlan = overridePlanAdaptors(plan, resolutions);
t.is(plan, finalPlan);
});

test('replace adaptors on only job steps', (t) => {
const plan: ExecutionPlan = {
workflow: {
steps: [
{
id: 'x',
next: 'two',
},
{
id: 'two',
expression: 'fn(state=> state)',
adaptors: ['@openfn/language-common@next'],
},
],
},
};
const resolutions = {
'@openfn/language-common@next': '@openfn/[email protected]',
};

const finalPlan = overridePlanAdaptors(plan, resolutions);
t.deepEqual(getAdaptors(finalPlan), [
undefined,
['@openfn/[email protected]'],
]);
});

0 comments on commit 0337621

Please sign in to comment.