diff --git a/integration-tests/cli/CHANGELOG.md b/integration-tests/cli/CHANGELOG.md index fee170e24..9daeba9c9 100644 --- a/integration-tests/cli/CHANGELOG.md +++ b/integration-tests/cli/CHANGELOG.md @@ -1,5 +1,14 @@ # @openfn/integration-tests-cli +## 1.0.37 + +### Patch Changes + +- Updated dependencies [760a984] +- Updated dependencies [760a984] + - @openfn/project@0.23.0 + - @openfn/lightning-mock@2.5.7 + ## 1.0.36 ### Patch Changes diff --git a/integration-tests/cli/package.json b/integration-tests/cli/package.json index 587103abb..74bf51651 100644 --- a/integration-tests/cli/package.json +++ b/integration-tests/cli/package.json @@ -1,7 +1,7 @@ { "name": "@openfn/integration-tests-cli", "private": true, - "version": "1.0.36", + "version": "1.0.37", "description": "CLI integration tests", "author": "Open Function Group ", "license": "ISC", diff --git a/integration-tests/cli/src/util.ts b/integration-tests/cli/src/util.ts index 3c0fa1a83..a7e8794b2 100644 --- a/integration-tests/cli/src/util.ts +++ b/integration-tests/cli/src/util.ts @@ -3,7 +3,7 @@ import path from 'node:path'; export const getJSON = (pathToJson?: string) => { if (!pathToJson) { - pathToJson = path.resolve('test/fixtures', 'output.json'); + pathToJson = path.resolve('test/fixtures', 'tmp', 'output.json'); } const data = readFileSync(pathToJson, 'utf8'); return JSON.parse(data); diff --git a/integration-tests/cli/test/execute-workflow.test.ts b/integration-tests/cli/test/execute-workflow.test.ts index 9e69d8465..32aa89bec 100644 --- a/integration-tests/cli/test/execute-workflow.test.ts +++ b/integration-tests/cli/test/execute-workflow.test.ts @@ -323,7 +323,7 @@ test.serial( async (t) => { const res = await run(t.title); t.falsy(res.err); - const out = getJSON(); + const out = getJSON('tmp/output.json'); t.deepEqual(out, { data: { result: 'love-some-suffix', @@ -332,12 +332,12 @@ test.serial( } ); -test.serial( +test.serial.only( `openfn ${jobsPath}/globals-job.js --globals ${jobsPath}/globals-path-file.js -a common`, async (t) => { const res = await run(t.title); t.falsy(res.err); - const out = getJSON(); + const out = getJSON('tmp/output.json'); t.deepEqual(out, { data: { result: 'love-humble-suffix', diff --git a/integration-tests/cli/test/fixtures/.gitignore b/integration-tests/cli/test/fixtures/.gitignore index 325a56443..c0fdb0ef0 100644 --- a/integration-tests/cli/test/fixtures/.gitignore +++ b/integration-tests/cli/test/fixtures/.gitignore @@ -1 +1,2 @@ -output.json \ No newline at end of file +output.json +tmp diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index 01568f364..3dd520ebe 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -1,5 +1,20 @@ # @openfn/cli +## 1.44.0 + +### Minor Changes + +- d91fa7c: Update the default output path used by `execute`. Output is now written to `tmp/output.json` relative to the thing that was executed (the workflow yaml or expression js)" +- d91fa7c: Default `execute` output to `tmp/output.json` relative to the workflow being run. +- 760a984: Support workflow aliases: rename a workflow's folder to use it as a shorter name + +### Patch Changes + +- b29758d: Fix an issue where deploying a project and passing a UUID directly, an error would be thrown if a local project file does not exist +- Updated dependencies [760a984] +- Updated dependencies [760a984] + - @openfn/project@0.23.0 + ## 1.43.1 ### Patch Changes diff --git a/packages/cli/README.md b/packages/cli/README.md index 744b4ca38..cc71f6a91 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -268,6 +268,21 @@ openfn deploy - `-c, --config, --config-path` - path to the config file (defaults to `.config.json`) - `--no-confirm` - skip the confirmation prompt +## Workflow Aliases + +Workflow ids can get long. To give a workflow a shorter name, rename its folder in the workspace: + +```bash +mv workflows/my-really-long-workflow-name workflows/wf +openfn wf +``` + +The folder name is the alias. It is local only: it's never deployed, and `pull` and `checkout` keep the folder in place. + +Aliases are matched to workflows by id, so if a workflow is renamed in the app (which changes its id), the next `pull` writes it back to a folder named after the new id. Just rename the folder again. + +An alias can't be the same as another workflow's id or alias. Clashing aliases are dropped with a warning, and the workflow goes back to a folder named after its id. + ## Checking a Workflow's Version Hash `openfn project version` prints the version hash used to detect drift between a local and remote workflow (see `@openfn/project`'s `src/util/version.ts`). It accepts three kinds of input: diff --git a/packages/cli/package.json b/packages/cli/package.json index ad719c01e..5c2b1987d 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/cli", - "version": "1.43.1", + "version": "1.44.0", "description": "CLI devtools for the OpenFn toolchain", "repository": { "url": "https://github.com/OpenFn/kit" diff --git a/packages/cli/src/execute/handler.ts b/packages/cli/src/execute/handler.ts index 607e73b33..72c20bcd2 100644 --- a/packages/cli/src/execute/handler.ts +++ b/packages/cli/src/execute/handler.ts @@ -13,7 +13,7 @@ import compile from '../compile/compile'; import { Logger, printDuration } from '../util/logger'; import loadState from '../util/load-state'; import validateAdaptors from '../util/validate-adaptors'; -import loadPlan from '../util/load-plan'; +import loadPlan, { DEFAULT_OUTPUT } from '../util/load-plan'; import assertPath from '../util/assert-path'; import { clearCache } from '../util/cache'; import fuzzyMatchStep from '../util/fuzzy-match-step'; @@ -175,6 +175,10 @@ const executeHandler = async (options: ExecuteOptions, logger: Logger) => { try { const result = await execute(finalPlan, state, options, logger); + // Last resort default if loadPlan didn't resolve one + if (!options.outputStdout) { + options.outputPath ??= DEFAULT_OUTPUT; + } await serializeOutput(options, result, logger); const duration = printDuration(new Date().getTime() - start); if (result?.errors) { diff --git a/packages/cli/src/options.ts b/packages/cli/src/options.ts index 557560574..a22f1d4c6 100644 --- a/packages/cli/src/options.ts +++ b/packages/cli/src/options.ts @@ -463,10 +463,12 @@ export const outputPath: CLIOption = { } else { if (opts.outputStdout) { delete opts.outputPath; - } else { + } else if (opts.command !== 'execute') { const base = getBaseDir(opts); setDefaultValue(opts, 'outputPath', nodePath.join(base, 'output.json')); } + // For execute, the default is resolved late: relative to the workflow + // in loadPlan, or tmp/output.json in the cwd in the execute handler } // remove the alias delete (opts as { o?: string }).o; diff --git a/packages/cli/src/projects/checkout.ts b/packages/cli/src/projects/checkout.ts index 9a9dd4973..710ee90a6 100644 --- a/packages/cli/src/projects/checkout.ts +++ b/packages/cli/src/projects/checkout.ts @@ -174,6 +174,20 @@ export const handler = async (options: CheckoutOptions, logger?: Logger) => { } } + // Workflow aliases are just folder names, so carry them over from the + // checked out project to keep renamed folders in place + const incomingIds = new Set(switchProject.workflows.map((wf) => wf.id)); + for (const wf of switchProject.workflows) { + const alias = localProject?.getWorkflow(wf.id)?.alias; + if (alias && incomingIds.has(alias)) { + logger?.warn( + `Dropping alias "${alias}" for workflow "${wf.id}": it clashes with another workflow` + ); + } else { + wf.alias ??= alias; + } + } + // delete workflow dir before expanding project if (options.clean) { await rimraf(workspace.workflowsPath); diff --git a/packages/cli/src/projects/deploy.ts b/packages/cli/src/projects/deploy.ts index 91f3234a3..80e3d3dcd 100644 --- a/packages/cli/src/projects/deploy.ts +++ b/packages/cli/src/projects/deploy.ts @@ -25,6 +25,7 @@ import { import { build, ensure } from '../util/command-builders'; import { printRichDiff } from './diff'; import ensureCheckout from './ensure-checkout'; +import isUuid from '../util/is-uuid'; import { getCredentialsVisitor, remapCredentials } from './credentials-helpers'; import type { Provisioner } from '@openfn/lexicon/lightning'; @@ -192,6 +193,13 @@ export const channelsChanged = (local: Project, remote: Project) => toResourceChannels(remote.channels, remote.credentials) ); +// The remote project to deploy to. This is all we need to fetch it, +// so it does not have to exist in the local workspace +type DeployTarget = { + uuid: string; + endpoint: string; +}; + export type SyncResult = { merged: Project; remoteProject: Project; @@ -209,30 +217,31 @@ const syncProjects = async ( config: Required, ws: Workspace, localProject: Project, - trackedProject: Project, // the project we want to update + target: DeployTarget, // the remote project we want to update logger: Logger, skipLocallyChangedCheck = false ): Promise => { - // First step, fetch the latest version and write + // First step, fetch the latest version (held in memory, not written to disk) + // The target may not be tracked locally at all: all we need is a uuid and endpoint // this may throw! let remoteProject: Project; try { - logger.info('Fetching remote target ', printProjectName(trackedProject)); - // TODO should we prefer endpoint over alias? - // maybe if it's explicitly passed? - const endpoint = trackedProject.openfn?.endpoint ?? config.endpoint; + logger.info( + 'Fetching remote target ', + `${target.uuid} at ${target.endpoint}` + ); const { data } = await fetchProject( - endpoint, + target.endpoint, config.apiKey, - trackedProject.uuid!, + target.uuid, logger ); remoteProject = await Project.from('state', data!, { - endpoint: endpoint, + endpoint: target.endpoint, }); - logger.info('Downloaded latest version of project at ', endpoint); + logger.info('Downloaded latest version of project at ', target.endpoint); } catch (e) { logger.error(e); throw e; @@ -426,7 +435,8 @@ export async function handler(options: DeployOptions, logger: Logger) { // Track the remote we want to target // If the user passed an explicit target, we need to use that // Otherwise just sync with the local project's own tracked remote - let tracker; + let tracker: Project | null | undefined; + let targetUuid: string | undefined; if (options.new) { // reset all metadata localProject.openfn = { @@ -451,45 +461,44 @@ export async function handler(options: DeployOptions, logger: Logger) { tracker = ws.getTrackedProject(); } - // A project loaded from a file already knows which remote it belongs - // to, so it can serve as its own deploy destination if there's no - // locally tracked copy of it - if (!tracker && filePath) { - logger.debug( - 'No locally tracked project found: deploying to the remote named in the file' - ); - tracker = localProject; + // If the target isn't tracked locally we can still deploy to it, as long + // as we know its uuid: we'll fetch it from the endpoint + // (an alias can't be resolved without a local copy) + if (tracker) { + targetUuid = tracker.uuid ?? undefined; + // the local copy belongs to the project we deployed at, not the one + // we deployed from + alias ??= tracker.alias ?? undefined; + } else if (targetIdentifier && isUuid(targetIdentifier)) { + targetUuid = targetIdentifier; + } else if (filePath && !targetIdentifier) { + // A project loaded from a file already knows which remote it belongs to + targetUuid = localProject.uuid ?? undefined; } - if (!tracker) { - // Is this really an error? Unlikely to happen I think - console.log( - `ERROR: Failed to find tracked remote project ${ - targetIdentifier ?? localProject.uuid! - } locally` - ); - console.log('To deploy a new project, add --new to the command'); - // TODO can we automate the fetch bit? - // If it's a UUID it should be ok? - console.log( - 'You may need to fetch the project before you can safely deploy' + if (!targetUuid) { + throw new Error( + `Failed to find project ${ + targetIdentifier ?? '' + } in the workspace. Pass a project UUID to deploy to a project that is not tracked locally, or add --new to create a new project` ); - - throw new Error('Failed to find remote project locally'); } - - // the local copy belongs to the project we deployed at, not the one - // we deployed from - alias ??= tracker.alias ?? undefined; + logger.debug( + tracker + ? 'Deploying to locally tracked project' + : 'No locally tracked project found: fetching target from the remote' + ); } // Choose the target endpoint we want to deploy to // If the user explicity passed --endpoint, use that // If the local project is tracking an endpoint, use that (90% of cases) // Otherwise fallback to to the auto-loaded config (probably coming from env) - let endpoint: string = + // (an untracked file deploy falls back to the endpoint in the file) + const endpoint: string = options.endpoint ?? tracker?.openfn?.endpoint ?? + (targetUuid && !tracker ? localProject.openfn?.endpoint : undefined) ?? config.endpoint ?? DEFAULT_ENDPOINT; @@ -527,7 +536,7 @@ export async function handler(options: DeployOptions, logger: Logger) { config, ws!, localProject, - tracker!, + { uuid: targetUuid!, endpoint }, logger, // a file's own snapshot tells us nothing about the target !!filePath diff --git a/packages/cli/src/projects/fetch.ts b/packages/cli/src/projects/fetch.ts index 64ed56bad..76878bab2 100644 --- a/packages/cli/src/projects/fetch.ts +++ b/packages/cli/src/projects/fetch.ts @@ -15,6 +15,7 @@ import { getSerializePath, } from './util'; import { writeFile } from 'node:fs/promises'; +import isUuid from '../util/is-uuid'; export type FetchOptions = Pick< Opts, @@ -259,7 +260,7 @@ export async function fetchRemoteProject( // A full UUID, endpoint and output path leave nothing to resolve locally, // so don't trip over duplicate local copies const localProject = - options.outputPath && options.endpoint && UUID_RE.test(options.project!) + options.outputPath && options.endpoint && isUuid(options.project) ? undefined : workspace.get(options.project!); if ( @@ -310,9 +311,6 @@ export async function fetchRemoteProject( return project; } -const UUID_RE = - /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; - function ensureTargetCompatible( options: FetchOptions, remoteProject: Project, diff --git a/packages/cli/src/projects/list.ts b/packages/cli/src/projects/list.ts index 2b0576f88..0f4c85d8a 100644 --- a/packages/cli/src/projects/list.ts +++ b/packages/cli/src/projects/list.ts @@ -47,19 +47,28 @@ export const handler = async (options: ProjectListOptions, logger: Logger) => { // project, so there's no need to abort if we can't ask which it is workspace = await ensureCheckout(workspace, logger, { required: false }); + // Workflow aliases are folder names, so only the checked out project has them + const local = await workspace.getCheckedOutProject(); + logger.always(`Available openfn projects\n\n${workspace .list() - .map((p) => describeProject(p, p === workspace.getTrackedProject())) + .map((p) => { + const active = p === workspace.getTrackedProject(); + return describeProject(p, active, active ? local : undefined); + }) .join('\n\n')} `); }; -function describeProject(project: Project, active = false) { +function describeProject(project: Project, active = false, local?: Project) { // @ts-ignore const uuid = project.openfn?.uuid; return `${project.alias || '(no alias)'} | ${project.id} ${ active ? '(active)' : '' }\n ${uuid || ''}\n workflows:\n${project.workflows - .map((w) => ' - ' + w.id) + .map((w) => { + const alias = local?.workflows.find((l) => l.id === w.id)?.alias; + return ' - ' + w.id + (alias ? ` (${alias})` : ''); + }) .join('\n')}`; } diff --git a/packages/cli/src/projects/version.ts b/packages/cli/src/projects/version.ts index 625c86ed5..2d4ef1a4f 100644 --- a/packages/cli/src/projects/version.ts +++ b/packages/cli/src/projects/version.ts @@ -119,7 +119,7 @@ export const handler = async (options: VersionOptions, logger: Logger) => { const activeProject = await workspace.getCheckedOutProject(); if (options.workflow) { - const workflow = activeProject?.getWorkflow(options.workflow); + const workflow = activeProject?.findWorkflow(options.workflow); if (!workflow) { logger.error(`No workflow found with id ${options.workflow}`); return; diff --git a/packages/cli/src/util/is-uuid.ts b/packages/cli/src/util/is-uuid.ts new file mode 100644 index 000000000..258b3b24c --- /dev/null +++ b/packages/cli/src/util/is-uuid.ts @@ -0,0 +1,6 @@ +const UUID_RE = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +const isUuid = (str?: string | null): boolean => !!str && UUID_RE.test(str); + +export default isUuid; diff --git a/packages/cli/src/util/load-plan.ts b/packages/cli/src/util/load-plan.ts index a4f4e3d84..037b332c1 100644 --- a/packages/cli/src/util/load-plan.ts +++ b/packages/cli/src/util/load-plan.ts @@ -15,6 +15,8 @@ import resolvePath from './resolve-path'; import { CREDENTIALS_KEY } from '../execute/apply-credential-map'; import { CACHE_DIR } from './cache'; +export const DEFAULT_OUTPUT = 'tmp/output.json'; + const loadPlan = async ( options: Pick< Opts, @@ -30,6 +32,7 @@ const loadPlan = async ( | 'credentials' | 'collectionsEndpoint' | 'cachePath' + | 'outputPath' > & { workflow?: Opts['workflow']; workspace?: string; // from project opts @@ -51,7 +54,7 @@ const loadPlan = async ( const proj = await workspace.getCheckedOutProject(); const name = workflowName || options.workflow!; - const workflow = proj?.getWorkflow(name); + const workflow = proj?.findWorkflow(name); if (!workflow) { const e = new Error(`Could not find Workflow "${name}"`); delete e.stack; @@ -65,12 +68,17 @@ const loadPlan = async ( options.credentials ??= workspace.getConfig().credentials; options.collectionsEndpoint ??= proj?.openfn?.endpoint; // Set the cache path to be relative to the workflow - options.cachePath ??= workspace.workflowsPath + `/${name}/${CACHE_DIR}`; + const workflowDir = `${workspace.workflowsPath}/${ + workflow.alias ?? workflow.id + }`; + options.cachePath ??= `${workflowDir}/${CACHE_DIR}`; + options.outputPath ??= `${workflowDir}/${DEFAULT_OUTPUT}`; } if (options.path && /ya?ml$/.test(options.path)) { const content = await fs.readFile(path.resolve(options.path), 'utf-8'); options.baseDir = dirname(options.path); + options.outputPath ??= path.join(dirname(options.path), DEFAULT_OUTPUT); workflowObj = yamlToJson(content); const { options: o, ...rest } = workflowObj; // restructure the workflow so that options are not on the workflow object, @@ -89,6 +97,9 @@ const loadPlan = async ( if (jsonPath && !options.baseDir) { options.baseDir = path.dirname(jsonPath); } + if (jsonPath) { + options.outputPath ??= path.join(path.dirname(jsonPath), DEFAULT_OUTPUT); + } workflowObj = workflowObj ?? (await loadJson(jsonPath!, logger)); const defaultName = workflowObj.name || path.parse(jsonPath ?? '').name; diff --git a/packages/cli/test/commands.test.ts b/packages/cli/test/commands.test.ts index 0e3f7c798..823a6b9cc 100644 --- a/packages/cli/test/commands.test.ts +++ b/packages/cli/test/commands.test.ts @@ -54,7 +54,7 @@ async function run(command: string, job: string, options: RunOptions = {}) { const expressionPath = options.expressionPath || 'job.js'; const statePath = options.statePath || 'state.json'; - const outputPath = options.outputPath || 'output.json'; + const outputPath = options.outputPath || 'tmp/output.json'; const state = JSON.stringify(options.state) || '{ "data": {}, "configuration": {} }'; @@ -86,6 +86,9 @@ async function run(command: string, job: string, options: RunOptions = {}) { // Override some options after the command has been parsed opts.path = expressionPath; opts.repoDir = options.repoDir; + if (options.outputPath) { + opts.outputPath ??= options.outputPath; + } opts.log = { default: 'none' }; opts.skipAdaptorValidation = true; @@ -164,7 +167,6 @@ test.serial('run an execution plan', async (t) => { }; const options = { - outputPath: 'output.json', expressionPath: 'wf.json', // just to fool the test }; @@ -192,7 +194,6 @@ test.serial('run an execution plan with start', async (t) => { }; const options = { - outputPath: 'output.json', expressionPath: 'wf.json', // just to fool the test }; @@ -264,7 +265,6 @@ test.serial('run a workflow', async (t) => { }; const options = { - outputPath: 'output.json', expressionPath: 'wf.json', // just to fool the test }; @@ -286,7 +286,6 @@ test.serial('run a workflow with config as an object', async (t) => { }; const options = { - outputPath: 'output.json', expressionPath: 'wf.json', // just to fool the test }; const result = await run('openfn wf.json', JSON.stringify(workflow), options); @@ -309,7 +308,6 @@ test.serial('run a workflow with config as a path', async (t) => { }; const options = { - outputPath: 'output.json', expressionPath: 'wf.json', // just to fool the test mockfs: { '/config.json': JSON.stringify({ y: 0 }), diff --git a/packages/cli/test/execute/options.test.ts b/packages/cli/test/execute/options.test.ts index 96a87016a..413300a1b 100644 --- a/packages/cli/test/execute/options.test.ts +++ b/packages/cli/test/execute/options.test.ts @@ -18,7 +18,7 @@ test('correct default options', (t) => { t.is(options.immutable, false); t.is(options.expressionPath, 'job.js'); t.falsy(options.logJson); // TODO this is undefined right now - t.is(options.outputPath, 'output.json'); + t.is(options.outputPath, undefined); t.is(options.outputStdout, false); t.is(options.path, 'job.js'); t.is(options.skipAdaptorValidation, false); diff --git a/packages/cli/test/options/execute.test.ts b/packages/cli/test/options/execute.test.ts index e9950e22f..82e8e60f9 100644 --- a/packages/cli/test/options/execute.test.ts +++ b/packages/cli/test/options/execute.test.ts @@ -17,9 +17,9 @@ test("execute: expressionPath'.'", (t) => { t.assert(options.expressionPath === 'job.js'); }); -test('execute: default outputPath to ./output.json', (t) => { +test('execute: outputPath has no parse-time default', (t) => { const options = parse('execute tmp/job.js'); - t.assert(options.outputPath === 'tmp/output.json'); + t.is(options.outputPath, undefined); }); test('execute: set outputPath to ./output.json', (t) => { diff --git a/packages/cli/test/projects/checkout.test.ts b/packages/cli/test/projects/checkout.test.ts index d69e35442..9bd5eb277 100644 --- a/packages/cli/test/projects/checkout.test.ts +++ b/packages/cli/test/projects/checkout.test.ts @@ -234,6 +234,64 @@ test.serial( } ); +test.serial( + 'checkout: preserves a renamed (aliased) workflow folder', + async (t) => { + await checkoutHandler( + { command: 'project-checkout', project: 'my-project', workspace: '/ws' }, + logger + ); + fs.renameSync('/ws/workflows/simple-workflow-main', '/ws/workflows/wf'); + + await checkoutHandler( + { command: 'project-checkout', project: 'my-project', workspace: '/ws' }, + logger + ); + + t.deepEqual( + fs.readdirSync('/ws/workflows').sort(), + ['another-workflow-main', 'wf'].sort() + ); + t.true(fs.existsSync('/ws/workflows/wf/simple-workflow-main.yaml')); + } +); + +test.serial( + 'checkout: drops an alias that clashes with an incoming workflow id', + async (t) => { + await checkoutHandler( + { command: 'project-checkout', project: 'my-project', workspace: '/ws' }, + logger + ); + fs.renameSync('/ws/workflows/simple-workflow-main', '/ws/workflows/wf'); + + // a workflow called "wf" gets added remotely + const statePath = '/ws/.projects/project@app.openfn.org.yaml'; + const state = yamlToJson(fs.readFileSync(statePath, 'utf8')); + state.workflows.push({ ...state.workflows[1], name: 'wf', id: 'wf-id' }); + fs.writeFileSync(statePath, jsonToYaml(state)); + + await checkoutHandler( + { + command: 'project-checkout', + project: 'my-project', + workspace: '/ws', + force: true, + }, + logger + ); + + t.deepEqual( + fs.readdirSync('/ws/workflows').sort(), + ['another-workflow-main', 'simple-workflow-main', 'wf'].sort() + ); + t.deepEqual(fs.readdirSync('/ws/workflows/wf').sort(), [ + 'transform-data-to-fhir-standard.js', + 'wf.yaml', + ]); + } +); + test.serial('checkout: switching to and back between projects', async (t) => { // before checkout. my-project is active and expanded const bcheckout = new Workspace('/ws'); diff --git a/packages/cli/test/projects/deploy.test.ts b/packages/cli/test/projects/deploy.test.ts index f0f545e10..6abddc2a2 100644 --- a/packages/cli/test/projects/deploy.test.ts +++ b/packages/cli/test/projects/deploy.test.ts @@ -397,7 +397,7 @@ test.serial('deploy an updated project direct to an endpoint', async (t) => { apiKey: 'test-api-key', workspace: '/ws', project: '/ws/dev@localhost.yaml', - target: 'dev', + target: UUID, confirm: false, // log: 'debug', } as any, @@ -1499,3 +1499,113 @@ test.serial( t.is(Object.keys(server.state.projects).length, 2); } ); + +test.serial( + 'deploy: deploys the checked-out project to an untracked remote project by uuid', + async (t) => { + const OTHER_UUID = 'a1b2c3d4-0000-4000-8000-000000000001'; + server.addProject({ ...myProject_v1, id: OTHER_UUID }); + + await setup(projectYaml); + await writeFile('/ws/workflows/my-workflow/transform-data.js', 'log()'); + + await deploy( + { + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + project: OTHER_UUID, + checkout: false, + confirm: false, + } as any, + logger + ); + + t.truthy(logger._find('success', /Updated project at/)); + const body = (id: string) => + server.state.projects[id].workflows['my-workflow'].jobs['transform-data'] + .body; + t.is(body(OTHER_UUID), 'log()'); + // the project we deployed from is untouched + t.is(body(UUID), 'fn()'); + } +); + +test.serial( + 'deploy: an explicit endpoint overrides the one in the project file', + async (t) => { + mockFs({ + '/ws/project.yaml': projectYaml + .replace(ENDPOINT, 'http://localhost:1') + .replace('fn()', 'jam()'), + '/ws/openfn.yaml': '', + }); + + await deploy( + { + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + project: '/ws/project.yaml', + confirm: false, + } as any, + logger + ); + + t.is( + server.state.projects[UUID].workflows['my-workflow'].jobs[ + 'transform-data' + ].body, + 'jam()' + ); + } +); + +test.serial( + 'deploy: throws if the target is an alias that is not tracked locally', + async (t) => { + await setup(projectYaml); + + await t.throwsAsync( + () => + deploy( + { + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + project: 'nope', + checkout: false, + confirm: false, + } as any, + logger + ), + { message: /Failed to find project nope/ } + ); + } +); + +test.serial( + 'deploy: throws if a file is deployed to an alias that is not tracked locally', + async (t) => { + mockFs({ + '/ws/project.yaml': projectYaml, + '/ws/openfn.yaml': '', + }); + + await t.throwsAsync( + () => + deploy( + { + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + project: '/ws/project.yaml', + target: 'typo', + confirm: false, + } as any, + logger + ), + { message: /Failed to find project typo/ } + ); + } +); diff --git a/packages/cli/test/projects/list.test.ts b/packages/cli/test/projects/list.test.ts index 1faf89453..2a23e4a90 100644 --- a/packages/cli/test/projects/list.test.ts +++ b/packages/cli/test/projects/list.test.ts @@ -7,7 +7,7 @@ import { jsonToYaml } from '@openfn/project'; // a workspace with several projects but no checkout metadata const noCheckoutProject = jsonToYaml({ id: 'my-project', workflows: [] }); -mock({ +const files: Record = { 'no-ws/': { 'some.yaml': 'name: smth' }, '/no-checkout/openfn.yaml': '', '/no-checkout/.projects/main@app.openfn.org.yaml': noCheckoutProject, @@ -125,8 +125,19 @@ mock({ }, ], }), +}; + +// the same workspace, with a workflow folder renamed to an alias +files['/aliased/openfn.yaml'] = files['/ws/openfn.yaml']; +files['/aliased/.projects/main@app.openfn.org.yaml'] = + files['/ws/.projects/main@app.openfn.org.yaml']; +files['/aliased/workflows/sw/simple-workflow.yaml'] = jsonToYaml({ + id: 'simple-workflow', + steps: [], }); +mock(files); + const logger = createMockLogger('', { level: 'debug' }); test('throw for invalid workspace directory', async (t) => { @@ -171,6 +182,17 @@ staging | my-project ); }); +test('shows workflow aliases on the active project', async (t) => { + const privateLogger = createMockLogger('', { level: 'debug' }); + await list({ command: 'projects', workspace: '/aliased' }, privateLogger); + + const { message } = privateLogger._find( + 'always', + /available openfn projects/i + ); + t.regex(message as string, /^ - simple-workflow \(sw\)$/m); +}); + test.serial( 'lists projects without a checkout, without prompting or aborting', async (t) => { diff --git a/packages/cli/test/util/is-uuid.test.ts b/packages/cli/test/util/is-uuid.test.ts new file mode 100644 index 000000000..e3d763591 --- /dev/null +++ b/packages/cli/test/util/is-uuid.test.ts @@ -0,0 +1,16 @@ +import test from 'ava'; +import isUuid from '../../src/util/is-uuid'; + +test('accepts a uuid, in either case', (t) => { + t.true(isUuid('a1b2c3d4-0000-4000-8000-000000000001')); + t.true(isUuid('A1B2C3D4-0000-4000-8000-000000000001')); +}); + +test('rejects aliases, partial uuids and empty values', (t) => { + t.false(isUuid('staging')); + t.false(isUuid('a1b2c3d4-0000-4000-8000')); + t.false(isUuid(' a1b2c3d4-0000-4000-8000-000000000001')); + t.false(isUuid('')); + t.false(isUuid(undefined)); + t.false(isUuid(null)); +}); diff --git a/packages/cli/test/util/load-plan.test.ts b/packages/cli/test/util/load-plan.test.ts index 6597e07a2..bc02812cc 100644 --- a/packages/cli/test/util/load-plan.test.ts +++ b/packages/cli/test/util/load-plan.test.ts @@ -691,3 +691,31 @@ dirs: t.is(opts.credentials, '/creds.yaml'); } ); + +test.serial( + 'outputPath: default to tmp/output.json beside a workflow', + async (t) => { + const opts: any = { workflowPath: 'test/wf.json', expandAdaptors: true }; + + await loadPlan(opts, logger); + t.is(opts.outputPath, 'test/tmp/output.json'); + } +); + +test.serial('outputPath: do not override an explicit path', async (t) => { + const opts: any = { + workflowPath: 'test/wf.json', + expandAdaptors: true, + outputPath: 'out.json', + }; + + await loadPlan(opts, logger); + t.is(opts.outputPath, 'out.json'); +}); + +test.serial('outputPath: do not default for an expression', async (t) => { + const opts: any = { expressionPath: 'test/job.js', expandAdaptors: true }; + + await loadPlan(opts, logger); + t.is(opts.outputPath, undefined); +}); diff --git a/packages/lightning-mock/CHANGELOG.md b/packages/lightning-mock/CHANGELOG.md index a71f49953..a654734a8 100644 --- a/packages/lightning-mock/CHANGELOG.md +++ b/packages/lightning-mock/CHANGELOG.md @@ -1,5 +1,13 @@ # @openfn/lightning-mock +## 2.5.7 + +### Patch Changes + +- Updated dependencies [760a984] +- Updated dependencies [760a984] + - @openfn/project@0.23.0 + ## 2.5.6 ### Patch Changes diff --git a/packages/lightning-mock/package.json b/packages/lightning-mock/package.json index 4f9b524c0..54bbb4667 100644 --- a/packages/lightning-mock/package.json +++ b/packages/lightning-mock/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/lightning-mock", - "version": "2.5.6", + "version": "2.5.7", "private": true, "description": "A mock Lightning server", "main": "dist/index.js", diff --git a/packages/project/CHANGELOG.md b/packages/project/CHANGELOG.md index a05d4720c..f771e68a3 100644 --- a/packages/project/CHANGELOG.md +++ b/packages/project/CHANGELOG.md @@ -1,5 +1,15 @@ # @openfn/project +## 0.23.0 + +### Minor Changes + +- 760a984: Add `findWorkflow()` for fuzzy workflow lookups and make `getWorkflow()` match by id only. + + To migrate, replace `getWorkflow(nameOrUuid)` with `findWorkflow(nameOrUuid)`, which throws if more than one workflow matches. + +- 760a984: Support workflow aliases: rename a workflow's folder to use it as a shorter name + ## 0.22.1 ### Patch Changes diff --git a/packages/project/package.json b/packages/project/package.json index 2cbbafdf5..f0ddc3158 100644 --- a/packages/project/package.json +++ b/packages/project/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/project", - "version": "0.22.1", + "version": "0.23.0", "description": "Read, serialize, replicate and sync OpenFn projects", "author": "Open Function Group ", "license": "ISC", diff --git a/packages/project/src/Project.ts b/packages/project/src/Project.ts index b81136b31..a4eb59943 100644 --- a/packages/project/src/Project.ts +++ b/packages/project/src/Project.ts @@ -24,6 +24,8 @@ import { } from '@openfn/lexicon'; import { parse } from './util/get-credential-name'; +export class MultipleMatchingWorkflowsError extends Error {} + const maybeCreateWorkflow = (wf: any) => wf instanceof Workflow ? wf : new Workflow(wf); @@ -219,13 +221,28 @@ export class Project { throw new Error(`Cannot serialize ${type}`); } - // get workflow by name, id or uuid - getWorkflow(idOrName: string) { - return ( - this.workflows.find((wf) => wf.id == idOrName) || - this.workflows.find((wf) => wf.name === idOrName) || - this.workflows.find((wf) => wf.openfn?.uuid === idOrName) + // get workflow by id + getWorkflow(id: string) { + return this.workflows.find((wf) => wf.id === id); + } + + // find a workflow by id, alias, name or uuid. Throws if more than one matches + findWorkflow(term: string) { + const matches = this.workflows.filter( + (wf) => + wf.id === term || + wf.alias === term || + wf.name === term || + wf.openfn?.uuid === term ); + if (matches.length > 1) { + throw new MultipleMatchingWorkflowsError( + `Failed to resolve unique workflow for "${term}", clashes with: ${matches + .map((wf) => wf.id) + .join(', ')}` + ); + } + return matches[0]; } /** Returns a fully qualified name for the project, id, alias@domain */ diff --git a/packages/project/src/Workflow.ts b/packages/project/src/Workflow.ts index c38c74931..99238a979 100644 --- a/packages/project/src/Workflow.ts +++ b/packages/project/src/Workflow.ts @@ -15,6 +15,9 @@ class Workflow { openfn?: l.WorkflowMeta; options: any; // TODO + // Local-only alias, taken from the workflow's folder name on disk + alias?: string; + constructor(workflow: l.WorkflowState) { this.index = { steps: {}, // steps by id diff --git a/packages/project/src/Workspace.ts b/packages/project/src/Workspace.ts index d86878fc5..e45e6eb6d 100644 --- a/packages/project/src/Workspace.ts +++ b/packages/project/src/Workspace.ts @@ -175,6 +175,7 @@ export class Workspace { root: this.root, config: this.config, branch: this.branch, + logger: this.logger, // The checked out project can't meaningfully be said to have an alias // But we can force one if it makes sense from context alias: alias ?? null, diff --git a/packages/project/src/index.ts b/packages/project/src/index.ts index 47da4594c..924474356 100644 --- a/packages/project/src/index.ts +++ b/packages/project/src/index.ts @@ -1,10 +1,10 @@ -import { Project } from './Project'; +import { Project, MultipleMatchingWorkflowsError } from './Project'; import { Workspace } from './Workspace'; import { yamlToJson, jsonToYaml } from './util/yaml'; export default Project; -export { Workspace, yamlToJson, jsonToYaml }; +export { Workspace, yamlToJson, jsonToYaml, MultipleMatchingWorkflowsError }; export { generateWorkflow, generateProject } from './gen/generator'; diff --git a/packages/project/src/parse/from-fs.ts b/packages/project/src/parse/from-fs.ts index 6235a3ab4..4772f0577 100644 --- a/packages/project/src/parse/from-fs.ts +++ b/packages/project/src/parse/from-fs.ts @@ -4,6 +4,7 @@ import { glob } from 'glob'; import * as l from '@openfn/lexicon'; import { Project } from '../Project'; +import Workflow from '../Workflow'; import { yamlToJson } from '../util/yaml'; import { buildConfig, @@ -11,7 +12,7 @@ import { findWorkspaceFile, loadCheckoutFile, } from '../util/config'; -import { omit } from 'lodash-es'; +import { countBy, omit } from 'lodash-es'; import { Logger } from '@openfn/logger'; import omitNil from '../util/omit-nil'; import slugify from '../util/slugify'; @@ -137,7 +138,14 @@ export const parseProject = async (options: FromFsConfig) => { } } - proj.workflows.push(wf); + const workflow = new Workflow(wf); + // A workflow folder renamed away from the id is treated as an alias + const dir = path.dirname(filePath); + const folder = path.basename(dir); + if (dir !== path.resolve(root, workflowDir) && folder !== workflow.id) { + workflow.alias = folder; + } + proj.workflows.push(workflow); } } catch (e) { logger?.log(e); @@ -147,6 +155,18 @@ export const parseProject = async (options: FromFsConfig) => { } } + // Aliased workflows would share a folder with a clashing id or alias + const ids = new Set(proj.workflows.map((wf: Workflow) => wf.id)); + const aliases = countBy(proj.workflows, 'alias'); + for (const wf of proj.workflows as Workflow[]) { + if (wf.alias && (ids.has(wf.alias) || aliases[wf.alias] > 1)) { + logger?.warn( + `Ignoring alias "${wf.alias}" for workflow "${wf.id}": it clashes with another workflow` + ); + delete wf.alias; + } + } + return new Project(proj as l.ProjectState, { alias, ...context.workspace, diff --git a/packages/project/src/serialize/to-fs.ts b/packages/project/src/serialize/to-fs.ts index 6293ce131..536472ef5 100644 --- a/packages/project/src/serialize/to-fs.ts +++ b/packages/project/src/serialize/to-fs.ts @@ -65,7 +65,7 @@ export const extractWorkflow = ( (project.config as any).workflowRoot ?? 'workflows/'; - const path = nodepath.join(root, workflow.id, workflow.id); + const path = nodepath.join(root, workflow.alias ?? workflow.id, workflow.id); const wf = Object.assign( { @@ -115,7 +115,10 @@ export const extractStep = ( project.config?.dirs.workflows ?? (project.config as any)?.workflowRoot ?? 'workflows/'; - const path = nodepath.join(root, `${workflow.id}/${step.id}.js`); + const path = nodepath.join( + root, + `${workflow.alias ?? workflow.id}/${step.id}.js` + ); const content = step.expression; return { path, content }; diff --git a/packages/project/test/parse/from-fs.test.ts b/packages/project/test/parse/from-fs.test.ts index 8cd1b3c8a..22dc5d4a2 100644 --- a/packages/project/test/parse/from-fs.test.ts +++ b/packages/project/test/parse/from-fs.test.ts @@ -67,6 +67,55 @@ test.serial('should include multiple workflows (legacy format)', async (t) => { t.is(wf2.name, 'Workflow 2'); }); +test.serial('should treat a renamed workflow folder as an alias', async (t) => { + mockFile('/ws/openfn.yaml', buildConfig()); + + mockFile('/ws/workflows/wf/my-long-workflow-name.yaml', { + id: 'my-long-workflow-name', + steps: [{ id: 'a', expression: 'a.js' }], + }); + mockFile('/ws/workflows/wf/a.js', `fn(s => s)`); + + const project = await parseProject({ root: '/ws' }); + + const wf = project.findWorkflow('wf')!; + t.is(wf.id, 'my-long-workflow-name'); + t.is(wf.alias, 'wf'); + t.is(wf.steps[0].expression, 'fn(s => s)'); + t.falsy((wf.toJSON() as any).alias); +}); + +test.serial( + 'should not set an alias when the folder matches the id', + async (t) => { + mockFile('/ws/openfn.yaml', buildConfig()); + + mockFile('/ws/workflows/my-workflow/my-workflow.yaml', { + id: 'my-workflow', + steps: [{ id: 'a' }], + }); + + const project = await parseProject({ root: '/ws' }); + + t.is(project.getWorkflow('my-workflow')!.alias, undefined); + } +); + +test.serial( + 'should ignore an alias that clashes with another id', + async (t) => { + mockFile('/ws/openfn.yaml', buildConfig()); + + mockFile('/ws/workflows/wf-a/wf-a.yaml', { id: 'wf-a', steps: [] }); + mockFile('/ws/workflows/wf-a/wf-b.yaml', { id: 'wf-b', steps: [] }); + + const project = await parseProject({ root: '/ws' }); + + t.is(project.getWorkflow('wf-b')!.alias, undefined); + t.is(project.getWorkflow('wf-a')!.id, 'wf-a'); + } +); + test.serial('should load a workflow expression (legacy format)', async (t) => { mockFile('/ws/openfn.yaml', buildConfig()); diff --git a/packages/project/test/project.test.ts b/packages/project/test/project.test.ts index 1b07ee93d..a5150f3b6 100644 --- a/packages/project/test/project.test.ts +++ b/packages/project/test/project.test.ts @@ -1,7 +1,7 @@ // should parse a project from app state and back again import test from 'ava'; import type { Provisioner } from '@openfn/lexicon/lightning'; -import { Project } from '../src/Project'; +import { Project, MultipleMatchingWorkflowsError } from '../src/Project'; import generateWorkflow from '../src/gen/generator'; import { UnsafeMergeError } from '../src/merge/merge-project'; @@ -112,6 +112,51 @@ test('should default alias to "main"', (t) => { t.is(project.alias, 'main'); }); +const createAliasedProject = () => { + const project = new Project({ + workflows: [ + { + id: 'my-workflow', + name: 'My Workflow', + openfn: { uuid: '' }, + steps: [], + }, + { id: 'other-workflow', steps: [] }, + ], + }); + project.workflows[0].alias = 'wf'; + return project; +}; + +test('getWorkflow only matches by id', (t) => { + const project = createAliasedProject(); + + t.is(project.getWorkflow('my-workflow')!.id, 'my-workflow'); + t.falsy(project.getWorkflow('wf')); + t.falsy(project.getWorkflow('My Workflow')); + t.falsy(project.getWorkflow('')); +}); + +test('findWorkflow matches by id, alias, name or uuid', (t) => { + const project = createAliasedProject(); + + for (const term of ['my-workflow', 'wf', 'My Workflow', '']) { + t.is(project.findWorkflow(term)!.id, 'my-workflow'); + } + t.falsy(project.findWorkflow('nope')); +}); + +test('findWorkflow throws if more than one workflow matches', (t) => { + const project = createAliasedProject(); + project.workflows[1].name = 'my-workflow'; + + t.throws(() => project.findWorkflow('my-workflow'), { + instanceOf: MultipleMatchingWorkflowsError, + message: + 'Failed to resolve unique workflow for "my-workflow", clashes with: my-workflow, other-workflow', + }); +}); + test('should support credentials', (t) => { const project = new Project({ credentials: [ diff --git a/packages/project/test/serialize/to-fs.test.ts b/packages/project/test/serialize/to-fs.test.ts index 5f6dd437c..5e26fe956 100644 --- a/packages/project/test/serialize/to-fs.test.ts +++ b/packages/project/test/serialize/to-fs.test.ts @@ -334,6 +334,20 @@ test('toFs: extract a project with 1 workflow and 1 step', (t) => { t.is(files['workflows/my-workflow/step.js'], 'fn(s => s)'); }); +test('toFs: writes an aliased workflow into its alias folder', (t) => { + const project = new Project({ + name: 'My Project', + workflows: [{ id: 'my-long-workflow-name', steps: [step] }], + }); + project.workflows[0].alias = 'wf'; + + const files = toFs(project); + + t.truthy(files['workflows/wf/my-long-workflow-name.yaml']); + t.is(files['workflows/wf/step.js'], 'fn(s => s)'); + t.falsy(files['workflows/my-long-workflow-name/my-long-workflow-name.yaml']); +}); + test('toFs: extract a project with forked_from meta', (t) => { const project = new Project( {