diff --git a/packages/cli/src/merge/command.ts b/packages/cli/src/merge/command.ts index f61db6750..a07447e1d 100644 --- a/packages/cli/src/merge/command.ts +++ b/packages/cli/src/merge/command.ts @@ -1,6 +1,6 @@ import yargs from 'yargs'; import { Opts } from '../options'; -import { ensure, build } from '../util/command-builders'; +import { ensure, build, override } from '../util/command-builders'; import * as o from '../options'; export type MergeOptions = Required< @@ -12,6 +12,7 @@ export type MergeOptions = Required< | 'removeUnmapped' | 'workflowMappings' | 'log' + | 'force' > >; @@ -21,6 +22,9 @@ const options = [ o.removeUnmapped, o.workflowMappings, o.log, + override(o.force, { + description: 'Force a merge even when workflows are incompatible', + }), ]; const mergeCommand: yargs.CommandModule = { diff --git a/packages/cli/src/merge/handler.ts b/packages/cli/src/merge/handler.ts index eed5f527e..fe3ed2283 100644 --- a/packages/cli/src/merge/handler.ts +++ b/packages/cli/src/merge/handler.ts @@ -55,6 +55,7 @@ const mergeHandler = async (options: MergeOptions, logger: Logger) => { const final = Project.merge(sourceProject, targetProject, { removeUnmapped: options.removeUnmapped, workflowMappings: options.workflowMappings, + force: options.force, }); const yaml = final.serialize('state', { format: 'yaml' }); await fs.writeFile(finalPath, yaml); @@ -65,6 +66,7 @@ const mergeHandler = async (options: MergeOptions, logger: Logger) => { command: 'checkout', projectPath: commandPath, projectName: final.name || '', + log: options.log, }, logger ); diff --git a/packages/lexicon/core.d.ts b/packages/lexicon/core.d.ts index 8d0ae075f..267acf6bb 100644 --- a/packages/lexicon/core.d.ts +++ b/packages/lexicon/core.d.ts @@ -83,6 +83,9 @@ export type Workflow = { globals?: string; openfn?: OpenFnMetadata; + + // holds history information of a workflow + history?: string[] }; export type StepId = string; diff --git a/packages/project/src/Workflow.ts b/packages/project/src/Workflow.ts index 19a9b7574..cf787519a 100644 --- a/packages/project/src/Workflow.ts +++ b/packages/project/src/Workflow.ts @@ -30,6 +30,9 @@ class Workflow { this.workflow = clone(workflow); + // history needs to be on workflow object. + this.workflow.history = workflow.history?.length ? workflow.history : []; + const { id, name, openfn, steps, ...options } = workflow; if (!(id || name)) { throw new Error('A Workflow MUST have a name or id'); @@ -167,6 +170,19 @@ class Workflow { getVersionHash() { return generateHash(this); } + + pushHistory(versionHash: string) { + this.workflow.history?.push(versionHash); + } + + // return true if the current workflow can be merged into the target workflow without losing any changes + canMergeInto(target: Workflow) { + if (!target.workflow.history.length) return true; + const targetHead = + target.workflow.history[target.workflow.history.length - 1]; + if (this.workflow.history.indexOf(targetHead) > -1) return true; + return false; + } } export default Workflow; diff --git a/packages/project/src/merge/merge-project.ts b/packages/project/src/merge/merge-project.ts index 09becaae5..9d3b7b1a6 100644 --- a/packages/project/src/merge/merge-project.ts +++ b/packages/project/src/merge/merge-project.ts @@ -1,4 +1,3 @@ -import { Workflow } from '@openfn/lexicon'; import { defaultsDeep, isEmpty } from 'lodash-es'; import { Project } from '../Project'; @@ -6,12 +5,12 @@ import { mergeWorkflows } from './merge-node'; import mapUuids from './map-uuids'; import baseMerge from '../util/base-merge'; import getDuplicates from '../util/get-duplicates'; +import Workflow from '../Workflow'; export type MergeProjectOptions = Partial<{ workflowMappings: Record; // removeUnmapped: boolean; - - force: boolean; // TODO not implemented yet + force: boolean; }>; /** @@ -56,6 +55,27 @@ export function merge( return !!options?.workflowMappings[w.id]; }); + // mergeability + const potentialConflicts: Record = {}; + for (const sourceWorkflow of sourceWorkflows) { + const targetId = + options.workflowMappings?.[sourceWorkflow.id] ?? sourceWorkflow.id; + const targetWorkflow = target.getWorkflow(targetId); + if (targetWorkflow && !sourceWorkflow.canMergeInto(targetWorkflow)) { + potentialConflicts[sourceWorkflow.name] = targetWorkflow?.name; + } + } + + if (Object.keys(potentialConflicts).length && !options?.force) { + throw new Error( + `The below workflows can't be merged directly without losing data\n${Object.entries( + potentialConflicts + ) + .map(([from, to]) => `${from} → ${to}`) + .join('\n')}\nPass --force to force the merge anyway` + ); + } + for (const sourceWorkflow of sourceWorkflows) { const targetId = options.workflowMappings?.[sourceWorkflow.id] ?? sourceWorkflow.id; diff --git a/packages/project/test/gen/fixtures.ts b/packages/project/test/gen/fixtures.ts index e75597307..a33dd9725 100644 --- a/packages/project/test/gen/fixtures.ts +++ b/packages/project/test/gen/fixtures.ts @@ -4,6 +4,7 @@ export const ab = { id: 'workflow', name: 'Workflow', + history: [], steps: [ { id: 'a', diff --git a/packages/project/test/gen/generator.test.ts b/packages/project/test/gen/generator.test.ts index 794a0c59b..0c2ab6863 100644 --- a/packages/project/test/gen/generator.test.ts +++ b/packages/project/test/gen/generator.test.ts @@ -120,6 +120,7 @@ test('it should generate a simple workflow with any letter', (t) => { const expected = { id: 'workflow', name: 'Workflow', + history: [], steps: [ { id: 'x', @@ -152,6 +153,7 @@ test('it should generate a simple workflow with words, numbers and underscores', const expected = { id: 'workflow', name: 'Workflow', + history: [], steps: [ { id: 'node_1', @@ -184,6 +186,7 @@ test('it should generate two node pairs', (t) => { const expected = { id: 'workflow', name: 'Workflow', + history: [], steps: [ { id: 'a', @@ -231,6 +234,7 @@ test('it should generate two node pairs from one parent', (t) => { const expected = { id: 'workflow', name: 'Workflow', + history: [], steps: [ { id: 'a', @@ -276,6 +280,7 @@ test('it should generate several node pairs', (t) => { const expected = { id: 'workflow', name: 'Workflow', + history: [], steps: [ { id: 'a', @@ -439,6 +444,7 @@ test('it should generate a simple workflow with mapped uuids', (t) => { const expected = { id: 'workflow', name: 'Workflow', + history: [], steps: [ { id: 'a', @@ -481,6 +487,7 @@ test('it should generate a project with uuids', (t) => { const expected = { id: 'workflow', name: 'Workflow', + history: [], steps: [ { id: 'a', diff --git a/packages/project/test/parse/from-app-state.test.ts b/packages/project/test/parse/from-app-state.test.ts index 16fda2237..86c3ef6ed 100644 --- a/packages/project/test/parse/from-app-state.test.ts +++ b/packages/project/test/parse/from-app-state.test.ts @@ -114,6 +114,7 @@ test('should create a Project from prov state with a workflow', (t) => { t.deepEqual(project.workflows[0].toJSON(), { id: 'my-workflow', name: 'My Workflow', + history: [], steps: [ { id: 'trigger', diff --git a/packages/project/test/project.test.ts b/packages/project/test/project.test.ts index ac5475322..8a0ac6bf1 100644 --- a/packages/project/test/project.test.ts +++ b/packages/project/test/project.test.ts @@ -2,7 +2,7 @@ import test from 'ava'; import type { Provisioner } from '@openfn/lexicon/lightning'; import { Project } from '../src/Project'; -import generateWorkflow from '../src/gen/generator'; +import generateWorkflow, { generateProject } from '../src/gen/generator'; // TODO move to fixtures and re-use? // Or use util function instead? @@ -154,3 +154,34 @@ test('should return UUIDs for everything', (t) => { }, }); }); + +test('incompatible-merge: should throw error when merge is incompatible', (t) => { + const source = generateWorkflow('trigger-x'); + source.pushHistory(source.getVersionHash()); + const target = generateWorkflow('trigger-y'); + target.pushHistory(target.getVersionHash()); + + t.false(source.canMergeInto(target)); + + const sourceProject = new Project({ workflows: [source] }); + const targetProject = new Project({ workflows: [target] }); + t.throws(() => Project.merge(sourceProject, targetProject), { + message: `The below workflows can't be merged directly without losing data\nWorkflow → Workflow\nPass --force to force the merge anyway`, + }); +}); + +test('incompatible-merge-force: should ignore incompatiblity and merge when forced', (t) => { + // same as the above test with force + const source = generateWorkflow('trigger-x'); + source.pushHistory(source.getVersionHash()); + const target = generateWorkflow('trigger-y'); + target.pushHistory(target.getVersionHash()); + + t.false(source.canMergeInto(target)); + + const sourceProject = new Project({ workflows: [source] }); + const targetProject = new Project({ workflows: [target] }); + t.notThrows(() => + Project.merge(sourceProject, targetProject, { force: true }) + ); +}); diff --git a/packages/project/test/workflow.test.ts b/packages/project/test/workflow.test.ts index 82e7b914a..1c5cedfee 100644 --- a/packages/project/test/workflow.test.ts +++ b/packages/project/test/workflow.test.ts @@ -1,9 +1,11 @@ import { randomUUID } from 'node:crypto'; import test from 'ava'; import Workflow from '../src/Workflow'; +import { generateWorkflow } from '../src'; const simpleWorkflow = { id: 'my-workflow', + history: [], name: 'My Workflow', steps: [ { @@ -205,3 +207,76 @@ test('map uuids to ids', (t) => { t.deepEqual(w.index.id[uuid_ac], 'a-c'); t.deepEqual(w.index.id[uuid_bc], 'b-c'); }); + +test('canMergeInto: can merge empty histories', (t) => { + const sourceWf = generateWorkflow('trigger-x'); + const targetWf = generateWorkflow('trigger-y'); + + t.deepEqual(sourceWf.workflow.history, []); + t.deepEqual(targetWf.workflow.history, []); + + t.true(sourceWf.canMergeInto(targetWf)); // allowed +}); + +test('canMergeInto: can merge empty target history', (t) => { + const sourceWf = generateWorkflow('trigger-x'); + const targetWf = generateWorkflow('trigger-y'); + sourceWf.workflow.history = ['history-1']; + + t.is(sourceWf.workflow.history.length, 1); + t.deepEqual(targetWf.workflow.history, []); + + t.true(sourceWf.canMergeInto(targetWf)); // allowed +}); + +test('canMergeInto: cannot merge empty source history', (t) => { + const sourceWf = generateWorkflow('trigger-x'); + const targetWf = generateWorkflow('trigger-y'); + targetWf.workflow.history = ['history-1']; + + t.is(targetWf.workflow.history.length, 1); + t.deepEqual(sourceWf.workflow.history, []); + + t.false(sourceWf.canMergeInto(targetWf)); // not allowed +}); + +test('canMergeInto: can merge target head in source', (t) => { + const sourceWf = generateWorkflow('trigger-x'); + const targetWf = generateWorkflow('trigger-y'); + targetWf.workflow.history = ['history-1', 'history-3', 'history-5']; + sourceWf.workflow.history = ['history-2', 'history-5', 'history-6']; + + t.true(sourceWf.canMergeInto(targetWf)); // allowed +}); + +test('canMergeInto: cannot merge target head not in source', (t) => { + const sourceWf = generateWorkflow('trigger-x'); + const targetWf = generateWorkflow('trigger-y'); + targetWf.workflow.history = ['history-1', 'history-3', 'history-5']; + sourceWf.workflow.history = ['history-2', 'history-4', 'history-6']; + + t.false(sourceWf.canMergeInto(targetWf)); // not allowed +}); + +test('canMergeInto: cannot merge source head in target', (t) => { + const sourceWf = generateWorkflow('trigger-x'); + const targetWf = generateWorkflow('trigger-y'); + targetWf.workflow.history = [ + 'history-1', + 'history-3', + 'history-6', + 'history-5', + ]; + sourceWf.workflow.history = ['history-2', 'history-4', 'history-6']; + + t.false(sourceWf.canMergeInto(targetWf)); // not allowed +}); + +test('canMergeInto: can merge same heads', (t) => { + const sourceWf = generateWorkflow('trigger-x'); + const targetWf = generateWorkflow('trigger-y'); + targetWf.workflow.history = ['history-1', 'history-3', 'history-6']; + sourceWf.workflow.history = ['history-2', 'history-4', 'history-6']; + + t.true(sourceWf.canMergeInto(targetWf)); // allowed +});