From 36522908fc142818e9f0cdf77228b11a801ba263 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 7 Nov 2023 17:33:51 +0000 Subject: [PATCH 01/14] common: add operation factory --- packages/common/src/Adaptor.js | 15 ++++++++++++++ packages/common/test/index.test.js | 33 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/packages/common/src/Adaptor.js b/packages/common/src/Adaptor.js index 7e3105f64..fdce71665 100644 --- a/packages/common/src/Adaptor.js +++ b/packages/common/src/Adaptor.js @@ -15,6 +15,21 @@ export * as dateFns from './dateFns'; const schemaCache = {}; +// operation creates a new operation factory +// It accepts a function which takes user arguments AND state +// It returns a function which acccepts user args +// Which in turn returns a function which accepts state +// and then triggers the original function +// It's complex BUT the end user code should be way way simpler +export const operation = (fn) => { + return (...args) => { + return (state) => { + return fn(state, ...args) + } + } +} + + /** * Execute a sequence of operations. * Main outer API for executing expressions. diff --git a/packages/common/test/index.test.js b/packages/common/test/index.test.js index 6938e75d6..37928c07e 100644 --- a/packages/common/test/index.test.js +++ b/packages/common/test/index.test.js @@ -28,12 +28,45 @@ import { splitKeys, toArray, validate, + + operation, } from '../src/Adaptor'; const mockAgent = new MockAgent(); setGlobalDispatcher(mockAgent); const mockPool = mockAgent.get('https://localhost:1'); +// TODO move to the bottom or another file +describe.only('operation', () => { + it('should return an operation factory that works', async () => { + // declare an operation called fetch + const fetch = operation(async (state, url) => { + // do a simple request with undici and write it to state + const response = await request(url) + state.data = await response.body.text() + return state; + }) + + mockPool + .intercept({ + method: 'GET', + path: '/a', + }) + .reply(200, 'ok'); + + const state = {}; + + // run it + // Note that there's no way to mock the operation yet + // Answers are a) provide a dymanic mock callback + // b) provide an inner function which accepts a client + await fetch('https://localhost:1/a')(state) + + // check the state + expect(state.data).to.eql('ok') + }) +}) + describe('execute', () => { it('executes each operation in sequence', done => { let state = {}; From be05ec015f4a059fb781229b7d5f461b84413ab6 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 7 Nov 2023 17:50:13 +0000 Subject: [PATCH 02/14] common: start refactoring to use new operation helper --- packages/common/src/Adaptor.js | 100 +---------------------------- packages/common/src/operations.js | 100 +++++++++++++++++++++++++++++ packages/common/test/each.test.js | 3 +- packages/common/test/index.test.js | 10 +-- 4 files changed, 110 insertions(+), 103 deletions(-) create mode 100644 packages/common/src/operations.js diff --git a/packages/common/src/Adaptor.js b/packages/common/src/Adaptor.js index fdce71665..17f419a81 100644 --- a/packages/common/src/Adaptor.js +++ b/packages/common/src/Adaptor.js @@ -53,34 +53,6 @@ export function execute(...operations) { }; } -/** - * alias for "fn()" - * @function - * @param {Function} func is the function - * @returns {Operation} - */ -export function alterState(func) { - return fn(func); -} - -/** - * Creates a custom step (or operation) for more flexible job writing. - * @public - * @example - * fn(state => { - * // do some things to state - * return state; - * }); - * @function - * @param {Function} func is the function - * @returns {Operation} - */ -export function fn(func) { - return state => { - return func(state); - }; -} - /** * Picks out a single value from a JSON object. * If a JSONPath returns more than one value for the reference, the first @@ -97,39 +69,8 @@ export function jsonValue(obj, path) { return JSONPath({ path, json: obj })[0]; } -/** - * Picks out a single value from source data. - * If a JSONPath returns more than one value for the reference, the first - * item will be returned. - * @public - * @example - * sourceValue('$.key') - * @function - * @param {String} path - JSONPath referencing a point in `state`. - * @returns {Operation} - */ -export function sourceValue(path) { - return state => { - return JSONPath({ path, json: state })[0]; - }; -} -/** - * Picks out a value from source data. - * Will return whatever JSONPath returns, which will always be an array. - * If you need a single value use `sourceValue` instead. - * @public - * @example - * source('$.key') - * @function - * @param {String} path - JSONPath referencing a point in `state`. - * @returns {Array.} - */ -export function source(path) { - return state => { - return JSONPath({ path, json: state }); - }; -} + /** * Ensures a path points at the data. @@ -245,7 +186,7 @@ export const map = curry(function (path, operation, state) { export function asData(data, state) { switch (typeof data) { case 'string': - return source(data)(state); + return JSONPath({ path: data, json: state }); case 'object': return data; case 'function': @@ -253,44 +194,7 @@ export function asData(data, state) { } } -/** - * Scopes an array of data based on a JSONPath. - * Useful when the source data has `n` items you would like to map to - * an operation. - * The operation will receive a slice of the data based of each item - * of the JSONPath provided. - * - * It also ensures the results of an operation make their way back into - * the state's references. - * @public - * @example - * each("$.[*]", - * create("SObject", - * field("FirstName", sourceValue("$.firstName")) - * ) - * ) - * @function - * @param {DataSource} dataSource - JSONPath referencing a point in `state`. - * @param {Operation} operation - The operation needed to be repeated. - * @returns {Operation} - */ -export function each(dataSource, operation) { - if (!dataSource) { - throw new TypeError('dataSource argument for each operation is invalid.'); - } - return state => { - return asData(dataSource, state).reduce((state, data, index) => { - if (state.then) { - return state.then(state => { - return operation({ ...state, data, index }); - }); - } else { - return operation({ ...state, data, index }); - } - }, state); - }; -} /** * Combines two operations into one diff --git a/packages/common/src/operations.js b/packages/common/src/operations.js new file mode 100644 index 000000000..4b9922d43 --- /dev/null +++ b/packages/common/src/operations.js @@ -0,0 +1,100 @@ +// This file contains all the operations exported by the common adaptor +// ie, everything you can call from job code +import { JSONPath } from 'jsonpath-plus'; + +import { operation, asData } from './Adaptor'; + +/** + * Creates a custom step (or operation) for more flexible job writing. + * @public + * @example + * fn(state => { + * // do some things to state + * return state; + * }); + * @function + * @param {Function} func is the function + * @returns {Operation} + */ + +export const fn = operation((state, func) => { + return func(state); +}) + +/** + * alias for "fn()" + * @function + * @param {Function} func is the function + * @returns {Operation} + */ + +export const alterState = fn; + +// NOTE: docs shouldn't change, but I'd like to check that typings still work +// May need a little finessing +/** + * Picks out a single value from source data. + * If a JSONPath returns more than one value for the reference, the first + * item will be returned. + * @public + * @example + * sourceValue('$.key') + * @function + * @param {String} path - JSONPath referencing a point in `state`. + * @returns {Operation} + */ +export const sourceValue = operation((state, path) => { + return JSONPath({ path, json: state })[0]; +}) + +/** + * Picks out a value from source data. + * Will return whatever JSONPath returns, which will always be an array. + * If you need a single value use `sourceValue` instead. + * @public + * @example + * source('$.key') + * @function + * @param {String} path - JSONPath referencing a point in `state`. + * @returns {Array.} + */ +export const source = operation((state, path) => { + return JSONPath({ path, json: state }); +}) + +/** + * Scopes an array of data based on a JSONPath. + * Useful when the source data has `n` items you would like to map to + * an operation. + * The operation will receive a slice of the data based of each item + * of the JSONPath provided. + * + * It also ensures the results of an operation make their way back into + * the state's references. + * @public + * @example + * each("$.[*]", + * create("SObject", + * field("FirstName", sourceValue("$.firstName")) + * ) + * ) + * @function + * @param {DataSource} dataSource - JSONPath referencing a point in `state`. + * @param {Operation} operation - The operation needed to be repeated. + * @returns {Operation} + */ +export const each = operation((state, dataSource, fn) => { + if (!dataSource) { + throw new TypeError('dataSource argument for each fn is invalid.'); + } + + return asData(dataSource, state).reduce((state, data, index) => { + if (state.then) { + return state.then(state => { + return fn({ ...state, data, index }); + }); + } else { + return fn({ ...state, data, index }); + } + }, state); +}) \ No newline at end of file diff --git a/packages/common/test/each.test.js b/packages/common/test/each.test.js index 37e448080..f2f009087 100644 --- a/packages/common/test/each.test.js +++ b/packages/common/test/each.test.js @@ -1,6 +1,7 @@ import { expect } from 'chai'; import testData from './fixtures/data.json' assert { type: 'json' }; -import { each, beta } from '../src/Adaptor'; +import { beta } from '../src/Adaptor'; +import { each } from '../src/operations'; function shouldBehaveLikeEach(each) { let state, operation; diff --git a/packages/common/test/index.test.js b/packages/common/test/index.test.js index 37928c07e..a7f8d20be 100644 --- a/packages/common/test/index.test.js +++ b/packages/common/test/index.test.js @@ -9,7 +9,6 @@ import { combine, dataPath, dataValue, - each, execute, expandReferences, field, @@ -23,21 +22,24 @@ import { parseCsv, referencePath, scrubEmojis, - source, - sourceValue, splitKeys, toArray, validate, operation, } from '../src/Adaptor'; +import { + sourceValue, + each, + source, +} from '../src/operations'; const mockAgent = new MockAgent(); setGlobalDispatcher(mockAgent); const mockPool = mockAgent.get('https://localhost:1'); // TODO move to the bottom or another file -describe.only('operation', () => { +describe('operation', () => { it('should return an operation factory that works', async () => { // declare an operation called fetch const fetch = operation(async (state, url) => { From e7ea571af33a24dcad15bed9ac3b6a47ba9756c6 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 8 Nov 2023 07:53:33 +0000 Subject: [PATCH 03/14] msgraph: new operations for getDrive --- packages/common/ast.json | 200 ----------------------------- packages/msgraph/src/Adaptor.js | 103 --------------- packages/msgraph/src/Utils.js | 21 ++- packages/msgraph/src/impl.js | 62 +++++++++ packages/msgraph/src/operations.js | 58 +++++++++ packages/msgraph/test/impl.test.js | 24 ++++ 6 files changed, 153 insertions(+), 315 deletions(-) create mode 100644 packages/msgraph/src/impl.js create mode 100644 packages/msgraph/src/operations.js create mode 100644 packages/msgraph/test/impl.test.js diff --git a/packages/common/ast.json b/packages/common/ast.json index 50aaf973b..534e56958 100644 --- a/packages/common/ast.json +++ b/packages/common/ast.json @@ -43,49 +43,6 @@ }, "valid": true }, - { - "name": "fn", - "params": [ - "func" - ], - "docs": { - "description": "Creates a custom step (or operation) for more flexible job writing.", - "tags": [ - { - "title": "public", - "description": null, - "type": null - }, - { - "title": "example", - "description": "fn(state => {\n // do some things to state\n return state;\n});" - }, - { - "title": "function", - "description": null, - "name": null - }, - { - "title": "param", - "description": "is the function", - "type": { - "type": "NameExpression", - "name": "Function" - }, - "name": "func" - }, - { - "title": "returns", - "description": null, - "type": { - "type": "NameExpression", - "name": "Operation" - } - } - ] - }, - "valid": true - }, { "name": "jsonValue", "params": [ @@ -139,110 +96,6 @@ }, "valid": true }, - { - "name": "sourceValue", - "params": [ - "path" - ], - "docs": { - "description": "Picks out a single value from source data.\nIf a JSONPath returns more than one value for the reference, the first\nitem will be returned.", - "tags": [ - { - "title": "public", - "description": null, - "type": null - }, - { - "title": "example", - "description": "sourceValue('$.key')" - }, - { - "title": "function", - "description": null, - "name": null - }, - { - "title": "param", - "description": "JSONPath referencing a point in `state`.", - "type": { - "type": "NameExpression", - "name": "String" - }, - "name": "path" - }, - { - "title": "returns", - "description": null, - "type": { - "type": "NameExpression", - "name": "Operation" - } - } - ] - }, - "valid": true - }, - { - "name": "source", - "params": [ - "path" - ], - "docs": { - "description": "Picks out a value from source data.\nWill return whatever JSONPath returns, which will always be an array.\nIf you need a single value use `sourceValue` instead.", - "tags": [ - { - "title": "public", - "description": null, - "type": null - }, - { - "title": "example", - "description": "source('$.key')" - }, - { - "title": "function", - "description": null, - "name": null - }, - { - "title": "param", - "description": "JSONPath referencing a point in `state`.", - "type": { - "type": "NameExpression", - "name": "String" - }, - "name": "path" - }, - { - "title": "returns", - "description": null, - "type": { - "type": "TypeApplication", - "expression": { - "type": "NameExpression", - "name": "Array" - }, - "applications": [ - { - "type": "UnionType", - "elements": [ - { - "type": "NameExpression", - "name": "String" - }, - { - "type": "NameExpression", - "name": "Object" - } - ] - } - ] - } - } - ] - }, - "valid": true - }, { "name": "dataPath", "params": [ @@ -481,59 +334,6 @@ }, "valid": true }, - { - "name": "each", - "params": [ - "dataSource", - "operation" - ], - "docs": { - "description": "Scopes an array of data based on a JSONPath.\nUseful when the source data has `n` items you would like to map to\nan operation.\nThe operation will receive a slice of the data based of each item\nof the JSONPath provided.\n\nIt also ensures the results of an operation make their way back into\nthe state's references.", - "tags": [ - { - "title": "public", - "description": null, - "type": null - }, - { - "title": "example", - "description": "each(\"$.[*]\",\n create(\"SObject\",\n field(\"FirstName\", sourceValue(\"$.firstName\"))\n )\n)" - }, - { - "title": "function", - "description": null, - "name": null - }, - { - "title": "param", - "description": "JSONPath referencing a point in `state`.", - "type": { - "type": "NameExpression", - "name": "DataSource" - }, - "name": "dataSource" - }, - { - "title": "param", - "description": "The operation needed to be repeated.", - "type": { - "type": "NameExpression", - "name": "Operation" - }, - "name": "operation" - }, - { - "title": "returns", - "description": null, - "type": { - "type": "NameExpression", - "name": "Operation" - } - } - ] - }, - "valid": true - }, { "name": "combine", "params": [ diff --git a/packages/msgraph/src/Adaptor.js b/packages/msgraph/src/Adaptor.js index f346b27dd..9a8ba497b 100644 --- a/packages/msgraph/src/Adaptor.js +++ b/packages/msgraph/src/Adaptor.js @@ -52,109 +52,6 @@ export function execute(...operations) { }; } -/** - * Create some resource in msgraph - * @public - * @example - * create("applications", {"displayName": "My App"}) - * @function - * @param {string} resource - The type of entity that will be created - * @param {object} data - The data to create the new resource - * @param {function} callback - An optional callback function - * @returns {Operation} - */ -export function create(resource, data, callback) { - return state => { - const [resolvedResource, resolvedData] = expandReferences( - state, - resource, - data - ); - - const { accessToken, apiVersion } = state.configuration; - - const url = setUrl({ apiVersion, resolvedResource }); - - const options = { - accessToken, - body: JSON.stringify(resolvedData), - method: 'POST', - }; - - return request(url, options).then(response => - handleResponse(response, state, callback) - ); - }; -} - -/** - * Make a GET request to msgraph resource - * @public - * @example - * get('sites/root/lists') - * @function - * @param {string} path - Path to resource - * @param {object} query - Query, Headers and Authentication parameters - * @param {function} callback - (Optional) Callback function - * @returns {Operation} - */ -export function get(path, query, callback = false) { - return state => { - const { accessToken, apiVersion } = state.configuration; - const [resolvedPath, resolvedQuery] = expandReferences(state, path, query); - - const url = setUrl(resolvedPath, apiVersion); - - return request(url, { query: resolvedQuery, accessToken }).then(response => - handleResponse(response, state, callback) - ); - }; -} - -/** - * Get a Drive or SharePoint document library. The drive metadata will be written - * to state.drives, where it can be used by other adaptor functions. - * Pass { id } to get a drive by id or { id, owner } to get default drive for - * some parent resource, like a group - * @public - * @example Get a drive by ID - * getDrive({ id: "YXzpkoLwR06bxC8tNdg71m" }) - * @example Get the default drive for a site - * getDrive({ id: "openfn.sharepoint.com", owner: "sites" }) - * @param specifier {Object} - A definition of the drive to retrieve - * - id {string} - The ID of the resource or owner. - * - owner {string} - The type of drive owner (e.g. sites, groups). - * @param {string} name - The local name of the drive used to write to state.drives, ie, state.drives[name] - * @param {function} [callback = s => s] (Optional) Callback function - * @return {Operation} - */ -export function getDrive(specifier, name = 'default', callback = s => s) { - return state => { - const { accessToken, apiVersion } = state.configuration; - const [resolvedSpecifier, resolvedName] = expandReferences( - state, - specifier, - name - ); - - const { id, owner = 'drive' } = resolvedSpecifier; - - let resource; - if (owner === 'drive') { - resource = `drives/${id}`; - } else { - resource = `${owner}/${id}/drive`; - } - - const url = setUrl(resource, apiVersion); - - return request(url, { accessToken }).then(response => { - state.drives[resolvedName] = response; - return callback(state); - }); - }; -} - /** * Get the contents or metadata of a folder. * @public diff --git a/packages/msgraph/src/Utils.js b/packages/msgraph/src/Utils.js index f741b27b7..c0d16d998 100644 --- a/packages/msgraph/src/Utils.js +++ b/packages/msgraph/src/Utils.js @@ -1,4 +1,4 @@ -import xlsx from 'xlsx'; +// import xlsx from 'xlsx'; import { fetch } from 'undici'; import { Readable, Writable } from 'node:stream'; import { composeNextState, asData } from '@openfn/language-common'; @@ -50,22 +50,19 @@ const isStream = value => { return false; }; -export function handleResponse(response, state, callback) { - let nextState; +export function handleResponse(response, state, callback = (s) => s) { // Don't compose state if response is a stream if (isStream(response)) { - nextState = { + return callback({ ...state, data: response, - }; - } else { - nextState = { - ...composeNextState(state, response), - response, - }; + }); } - if (callback) return callback(nextState); - return nextState; + + callback({ + ...composeNextState(state, response), + response, + }); } export function handleResponseError(response, data, method) { diff --git a/packages/msgraph/src/impl.js b/packages/msgraph/src/impl.js new file mode 100644 index 000000000..ad422a630 --- /dev/null +++ b/packages/msgraph/src/impl.js @@ -0,0 +1,62 @@ +import { handleResponse, setUrl } from './Utils'; +import { expandReferences } from '@openfn/language-common/util'; + +// This is a totally normal js function +// It can be unit tested by passing a request function in +export const create = (state, request, resource, data, callback) => { + const [resolvedResource, resolvedData] = expandReferences( + state, + resource, + data + ); + + const { accessToken, apiVersion } = state.configuration; + + const url = setUrl({ apiVersion, resolvedResource }); + + const options = { + accessToken, + body: JSON.stringify(resolvedData), + method: 'POST', + }; + + return request(url, options).then(response => + handleResponse(response, state, callback) + ); +} + +export const get = (state, request, path, query) => { + const { accessToken, apiVersion } = state.configuration; + const [resolvedPath, resolvedQuery] = expandReferences(state, path, query); + + const url = setUrl(resolvedPath, apiVersion); + + return request(url, { query: resolvedQuery, accessToken }).then(response => + handleResponse(response, state, callback) + ); +} + +export const getDrive = (state, request, specifier, name = 'default', callback = s => s) => { + const { accessToken, apiVersion } = state.configuration; + const [resolvedSpecifier, resolvedName] = expandReferences( + state, + specifier, + name + ); + + const { id, owner = 'drive' } = resolvedSpecifier; + + let resource; + if (owner === 'drive') { + resource = `drives/${id}`; + } else { + resource = `${owner}/${id}/drive`; + } + + const url = setUrl(resource, apiVersion); + + return request(url, { accessToken }).then(response => { + state.drives[resolvedName] = response; + return callback(state); + }); +} \ No newline at end of file diff --git a/packages/msgraph/src/operations.js b/packages/msgraph/src/operations.js new file mode 100644 index 000000000..4a4872694 --- /dev/null +++ b/packages/msgraph/src/operations.js @@ -0,0 +1,58 @@ + +// TODO this import maybe needs to be common/util +// if the main export only exports operations +import { operation } from '@openfn/language-common' +import * as impl from './impl'; +import { request } from './Utils'; + +/** + * Create some resource in msgraph + * @public + * @example + * create("applications", {"displayName": "My App"}) + * @function + * @param {string} resource - The type of entity that will be created + * @param {object} data - The data to create the new resource + * @param {function} callback - An optional callback function + * @returns {Operation} + */ +export const create = operation((state, resource, data, callback) => { + return impl.create(state, request, resource, data, callback) +}) + +/** + * Make a GET request to msgraph resource + * @public + * @example + * get('sites/root/lists') + * @function + * @param {string} path - Path to resource + * @param {object} query - Query, Headers and Authentication parameters + * @param {function} callback - (Optional) Callback function + * @returns {Operation} + */ +export function get(state, path, query, callback) { + return impl.get(state, request, path, query, callback) +} + + +/** + * Get a Drive or SharePoint document library. The drive metadata will be written + * to state.drives, where it can be used by other adaptor functions. + * Pass { id } to get a drive by id or { id, owner } to get default drive for + * some parent resource, like a group + * @public + * @example Get a drive by ID + * getDrive({ id: "YXzpkoLwR06bxC8tNdg71m" }) + * @example Get the default drive for a site + * getDrive({ id: "openfn.sharepoint.com", owner: "sites" }) + * @param specifier {Object} - A definition of the drive to retrieve + * - id {string} - The ID of the resource or owner. + * - owner {string} - The type of drive owner (e.g. sites, groups). + * @param {string} name - The local name of the drive used to write to state.drives, ie, state.drives[name] + * @param {function} [callback = s => s] (Optional) Callback function + * @return {Operation} + */ +export function getDrive(specifier, name, callback) { + return impl.getDrive(state, request, specifier, name, callback) +} diff --git a/packages/msgraph/test/impl.test.js b/packages/msgraph/test/impl.test.js new file mode 100644 index 000000000..7f303a58a --- /dev/null +++ b/packages/msgraph/test/impl.test.js @@ -0,0 +1,24 @@ +import { expect } from 'chai'; +import * as impl from '../src/impl'; +import { fixtures } from './fixtures'; + +describe('getDrive', () => { + // look how simple this unit test is now + it('should get a drive by id and set it to state', async () => { + // no config needed, we take that for granted here + const state = { + configuration: {}, + drives: {} // TODO I shouldn't need to set this though + }; + + // Mock out request to just return the drive response data + // TODO if we want, we cah test crentials here + const request = async () => fixtures.driveResponse; + + // We can literally just test the result of get drive + const result = await impl.getDrive(state, request, { id: 'b!YXzpkoLwR06bxC8tNdg71m_' }) + + // TODO not sure why this is wrapped in default... + expect(result.drives).to.eql({ default: fixtures.driveResponse }); + }); +}); \ No newline at end of file From 0dd859e303917d116385ac9b09cb624e124d83f4 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 8 Nov 2023 09:10:41 +0000 Subject: [PATCH 04/14] added some docs --- .gitignore | 2 +- docs/best-practice.md | 33 +++++++++++++ docs/creating-adaptors.md | 101 ++++++++++++++++++++++++++++++++++++++ docs/overview.md | 40 +++++++++++++++ docs/testing.md | 15 ++++++ 5 files changed, 190 insertions(+), 1 deletion(-) create mode 100644 docs/best-practice.md create mode 100644 docs/creating-adaptors.md create mode 100644 docs/overview.md create mode 100644 docs/testing.md diff --git a/.gitignore b/.gitignore index bb528281e..1b14da5ff 100644 --- a/.gitignore +++ b/.gitignore @@ -7,7 +7,7 @@ yarn-error.log* lerna-debug.log* # Generated jsdocs -docs/ +packages/**/docs/ # Working folders tmp/ diff --git a/docs/best-practice.md b/docs/best-practice.md new file mode 100644 index 000000000..87bd39334 --- /dev/null +++ b/docs/best-practice.md @@ -0,0 +1,33 @@ +style guide and best practice notes + +## Code Style + +* Use `camelCase` in variable and parameter names. This helps keep adaptors consistent. +* Try to avoid exposing the underlying implementation. +* * For example, if wrapping a function and the function exposes an options object, it's usually better to define your own options object and map it. +* * This lets us provide our own documention, enforce camelCase conventions (particularly when wrapping web services), and insulates us from changes in the underlying implementation. +* * It can also simplify an API to just expose options an end-user needs. For example we'd expect than an adaptor should deal with pagination automagically, so exporting page options is probably unhelpful +* * This isn't _always_ possible or sensible, particularly for large APIs, but should be considered. + +## Documentation + +All operations should be well documented in JSdoc annotations. + +It is usually best not to link to the underlying implementation's documentation - their context, requirements and expectations are very different to ours, and it may confuse our users. Better to keep users on our own docsite, with consistent style and navigation. + +## API Design + +A successful adaptor should have a clean, re-usable and easy to understand API design. + +There are two schools of thought for adaptor API design [and frankly we haven't quite settled on an answer yet]. + +1) An adaptor is a one-for-one wrapper around some backing web service or library. For each endpoint/function in the service, there should be an operation, with the same name, in the adaptor. +2) An adaptor is a high-level, user friendly, opinionated wrapper around a web service or library. It is not a mapping, but a high level abstraction. + +The approach you take my depend on your experience, and that of your users. If you know your users are experts in a particular rest interface, it probably makes sense just to expose that interface in an openfn-y way (so, a 1:1 mapping). + +On the other hand, if an API is particularly complex or heavy in boilerplate, it probably makes sense to provide some user-friendly, high level abstractions. + +A good approach is probably to do a little of both. Provide low-level helper functions to give access to the underlying implementaton (wrapping the `request` function is often the first step to a successful adaptor!), but then creating opinionated helper functions with specific tasks in mind. + +Also, when creating/editing adaptors, it's often best to have a goal in mind. Don't try and wrap an entire implementation - just do the bits you need for the job you (or your users) have in mind. \ No newline at end of file diff --git a/docs/creating-adaptors.md b/docs/creating-adaptors.md new file mode 100644 index 000000000..12011c2d7 --- /dev/null +++ b/docs/creating-adaptors.md @@ -0,0 +1,101 @@ +explain the basic structure of how to create an adaptor + +explain the adaptor/impl pattern + +## Folder structure + +In order to make it easier for developers to create and maintain adaptors, we adopt strong conventions around style and structure. + +An adaptor should have the following file structure: +``` +src + +- operations.js + +- impl.js + +- util.js + +- index.js + +- mock.js +``` + +`operations.js` is the most important file and every adaptor should have one. It's where your operations will live, along with your documentation. Anything you expect users to call from job code should live here. Keeping your operations in one place makes it easy for other developers and maintainers to see where the user-facing code is. + +`impl.js` is not required but encouraged. A useful pattern is to declare the interface to your operations in `operations.js`, but to add the actual implementation, the actual logic, in `impl.js`. Splitting each operation into two parts makes it easier to define unit tests (see the testing doc) + +`util.js` is another optional file. If you have helper functions to support your implementation, you can add them here. If you think they might be of use to job writers, you can export them in index (see below) + +`mock.js` contains mock code and data for your unit and integration tests. This is excluded from the release build and not published to npm. + +`index.js` declares the exports used by the final npm module. + +## Defining operations + +The common adaptor exposes a helper function to make it easier to define adaptors. + +Call the `operation` function with a single function argument. The function takes state as its first argument and whatever else you need. Assign the function to an exported const. + +``` +export const getPatient = operation((state, name, country, options) => { + // Implement the operation here +}) +``` + +If you prefer, you can stll define operations the "old" way, by creating your own factory wrapper. +``` +export function getPatient(name, country, options) { + return (state) => { + // Implement the operation here + } +} +``` +You'll still see this pattern used across the repo. + +## Implementing Operations + +Testing OpenFn adaptors has traditionally been a very hard problem. The factory pattern and the nature of the backend service makes it very hard to test code in isolation. + +The impl pattern helps this. + +You can cut the body of each operation out into an impl. The impl has almost the same structure, but it also accepts a library, client or exectutor function as an argument. This makes the implementation mockable. + +For example: + +``` +// operations.js +import client from 'my-library'; +import impl from './impl'; + +export const getPatient = operation((state, name, country, options) => { + impl.getPatient(state, client, name, country, options) +}) +``` + +This operation is really just a signature and declaration which calls out to an implementation function. Look closely at the impl signature though and you'll notice an extra argument: `client`. + +Our implementation might look like ths: +``` +// impl.js +export const getPatient = (state, client, name, country, options) => { + const result = await client.findPatient(name, { country, ...options }) + state.data = result.patients[0]; + return state; +} +``` + +Default values should be set in the outer operation wrapper. + +The implementation is easy to unit test because we can pass in a mock client object or function. Different adaptors have different needs but they ultimately will either call out to a webservice or another npm module. + +## Exports + +In order for anyone to use your adaptor, you need to export the correct functions in the index. + +Usually this is as simple as: +``` +export * from './operations` +``` +Which will make all operations available to user jobs. + +You may also want to export util functions to job code. It's good practice to namespace these so that it's clear to users what is an operation and what is a function. +``` +export * as util from './utils' +``` +Users can then do `util.convertPatient(patient)` in their job code. \ No newline at end of file diff --git a/docs/overview.md b/docs/overview.md new file mode 100644 index 000000000..0b80e1e8e --- /dev/null +++ b/docs/overview.md @@ -0,0 +1,40 @@ +Overview of adaptors and how they work + +key concepts - explain the words job, expression, workflow etc + +## Jobs + +A job is a unit of work, expressed via Javascript-like + +We sometimes use the word "expression" when referring to a job. Technically, the code written by end users (and executed by the OpenFn runtime) is the _expression_. A job is an expression plus a bunch of metadata about it, like which adaptor to use, what initial state to pass in, what options to use during execution. + +But mostly the terms are interchangable. + +An expression can be compiled or uncompiled. + +## DSL + +OpenFn code does not use Javascript. + +It actually uses a Javascript-like DSL (Domain Specific Language). This needs to be compiled into executable Javascript. + +The major differences between openfn code and Javascript are: +* The top level functions in the code are executed synchronously (in sequence), even if they contain asynchronous code +* OpenFn code does not contain import statements (although technically it can). These are compiled in. + +## Operations + +Users write openfn code by composing operations into a sequence of commands. + +Adaptors export operations - basically helper functions - to users. + +### Operations vs functions + +While an operation is of course a Javascript function, there are several differences between operationns and regular javascript functions. + +* An operation is a function that returns a function which in turn takes and returns a state object. +* The operations declared in an adaptor are _factories_ which return state-handling functions. +* Operations can be invoked at the top level of job code +* To call an operation within another function in job code, it has to be wrapped with state, ie `myOp(options)(state)`. We consider this to be an antipattern in job code - but it is occasionally neccessary + +In short, an operation is a function designed to run in an openfn job and handle state. \ No newline at end of file diff --git a/docs/testing.md b/docs/testing.md new file mode 100644 index 000000000..a074ef30d --- /dev/null +++ b/docs/testing.md @@ -0,0 +1,15 @@ +explain how unit tests and integration tests work + + +There are two types of test which serve different purposes; + +* Unit Tests are tightly scoped tests which execute specific functions in isolation. The smaller a unit of code is tested, the better the test. We would expect one unit test per operation (usually against the implementation, rather than the operation) +* Integration tests are broadly scoped tests against multiple operations. These look like actual job code and are executed against the actual Openfn Runtime + +## How to Unit Test + +TODO + +## How to Integration Test + +TODO \ No newline at end of file From 0467ee64239390bda40baf4ccc1329f098f6183f Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Sun, 19 Nov 2023 16:58:28 +0000 Subject: [PATCH 05/14] experimenting --- docs/changes.md | 9 ++++++++ packages/msgraph/src/Utils.js | 5 +++++ packages/msgraph/src/operations.js | 8 +++++++ packages/msgraph/test/integations.js | 33 ++++++++++++++++++++++++++++ packages/msgraph/test/mock.js | 4 ++++ 5 files changed, 59 insertions(+) create mode 100644 docs/changes.md create mode 100644 packages/msgraph/test/integations.js create mode 100644 packages/msgraph/test/mock.js diff --git a/docs/changes.md b/docs/changes.md new file mode 100644 index 000000000..a3dae5c2b --- /dev/null +++ b/docs/changes.md @@ -0,0 +1,9 @@ +## Internal / Temporary docs + +Tracking changes in this repo at a high level + +This will feed into the PR. + +* Introduce the `operation` factory helper +* Introduce impl pattern +* Introduce integration testing \ No newline at end of file diff --git a/packages/msgraph/src/Utils.js b/packages/msgraph/src/Utils.js index c0d16d998..344e24501 100644 --- a/packages/msgraph/src/Utils.js +++ b/packages/msgraph/src/Utils.js @@ -4,6 +4,11 @@ import { Readable, Writable } from 'node:stream'; import { composeNextState, asData } from '@openfn/language-common'; import { expandReferences } from '@openfn/language-common/util'; +// should be built out of this file +export const setRequestFunction = (mockFetch) => { + fetch = mockFetch +} + export function assertDrive(state, driveName) { if (!state.drives[driveName]) { const errorString = [ diff --git a/packages/msgraph/src/operations.js b/packages/msgraph/src/operations.js index 4a4872694..6ac9e75c6 100644 --- a/packages/msgraph/src/operations.js +++ b/packages/msgraph/src/operations.js @@ -5,6 +5,14 @@ import { operation } from '@openfn/language-common' import * as impl from './impl'; import { request } from './Utils'; +// overide the request handler +// Difficult: request itself has quite a bit of logic that's worth testing +// including side effects +// Really I only want to mock out hte actual fetch +export const _setMocks = () => { + +} + /** * Create some resource in msgraph * @public diff --git a/packages/msgraph/test/integations.js b/packages/msgraph/test/integations.js new file mode 100644 index 000000000..af291f3fb --- /dev/null +++ b/packages/msgraph/test/integations.js @@ -0,0 +1,33 @@ +// Integation testing + +// Load a test source script from examples + +// Inject the mock + +// run it + +// Check the result + +import * as Operations from './operations' + +// load a mock request handler here +// this should match urls +const mocks = { + 'www': someFilePayload +} + +// Hang on, what if I setup the mock fro here, before execute? +// I can prime the adaptor scope +Operations._setMocks(mocks) + +const job = ` +getDrive('some-id') +getFile('some-file') +` + +// execute with custom globals +// https://github.com/OpenFn/adaptors/pull/296/files#diff-9a88bc922d602321be848d9b9251be018a4b801387034c2fc1cfda54ef554ff5 +const finalState = await execute(job, { + Adaptor + mock: mocks // this sets the mock object inside the job +}) \ No newline at end of file diff --git a/packages/msgraph/test/mock.js b/packages/msgraph/test/mock.js new file mode 100644 index 000000000..1f9d78a37 --- /dev/null +++ b/packages/msgraph/test/mock.js @@ -0,0 +1,4 @@ +const initMock = (endpoints) => { + // here we basically need to override the request function + // Ho do we do this? +} \ No newline at end of file From b87d8bd25d847b58bcaac8fc9db57c97134195cc Mon Sep 17 00:00:00 2001 From: jclark Date: Tue, 5 Mar 2024 11:21:10 +0000 Subject: [PATCH 06/14] Add a high level tutorial for changes on this branch --- tutorial.md | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 tutorial.md diff --git a/tutorial.md b/tutorial.md new file mode 100644 index 000000000..8b5bdbb72 --- /dev/null +++ b/tutorial.md @@ -0,0 +1,140 @@ +# New Stuff Tutorial + +A nice clean presentation about how I'm thinking about next gen adaptors. + +## TODO + +Immediate short-term todos, ideally before nairobi + +- examples as tests + +## Features + +Here's what I've introduced + +* Operation factories +* operation/implementation split +* real runtime/job tests +* A pattern for mocking (probably better than demanding docker containers?) + +## Motivations + +Here are the problems I'm trying to solve + +* Some sort of capacity for useful unit tests + * They're good for development, maintenance and preventing regressions +* Clarity over what an operation is, and why it matters +* Encouraging good documetnation _in the right place_ + * I see lots of problems of devs documenting the wrong functions, wasting time and energy + +## Examples + +I've started implementing this stuff in `msgraph` (and common) to see how it all comes together + +I should really push this further and will try and spend some time on it before I fly. + +Maybe `salesforce`, `dhis2` or `http` would be better examples? + +## Issues + +Here's what's not right yet: + +* Basically I think this adds formality and complication. Does any of this acutally make adaptor writing easier? +* Expanding references. I'd really like to standardise and simplify this further + - jsdoc path vs dataValue() vs open function + - I am sure that you can just make expand references read '$a.b.c' as a jsonpath + +## Operation Factories + +Here's an operation factory +``` +export const get = operation((state, url, auth, callback) => { + // code goes here + return state +}) +``` + +Why is this good? +- There's less complicated nesting of arrow functions +- It's clear and explicit that this is an Operation + +## Seperate the implementation + +This makes it really easy to write unit tests against an implementation. You just pass a mock client in your tests. + +```js +// operation.js +import { operation } from '@openfn/language-common' +import * as impl from './impl'; + +export const create = operation((state, resource, data, callback) => { + return impl.create(state, request, resource, data, callback) +}) +``` + +Note the extra `request` argument! + +I can now unit test the implementation: + +```js +describe('getDrive', () => { + + it('should get a drive by id and set it to state', async () => { + const state = { + configuration: {}, + drives: {} + }; + + const mockRequest = async () => fixtures.driveResponse; + + // Call the implementation with the mock + const result = await impl.getDrive(state, mockRequest, { id: 'b!YXzpkoLwR06bxC8tNdg71m_' }) + + expect(result.drives).to.eql({ default: fixtures.driveResponse }); + }); +}); +``` + +## Real runtime tests + +## Future Work & Gripes + +Here's some other stuff we need to look at + +* A really good way to "record" a "curl" to the datasource + - I almost see a CLI command `openfn record` which will run a job (probably written against common.http) and save the final `state.data` (or something) to a json file in the right format to be used by the mock. maybe there's a map of `'path.json': data` So you write a job to populate your mock data, then just run it to save the data into the monorepo, then run the unit tests which should run against the mock data. + +* Can unit tests also be examples? + - My feeling is that if you save the job code to a file like "get-transactions.js", and that file is a fully runnable job (run it from the cli with state!), you can also load that into the website (somehow) as an example + - I am not sure how you setup the mock - I guess that happens in the test file. We can append a magic string or something, do something to prep the client for mocking + - I am also not sure where documentation goes - I guess in a comment at the top + +* Logging + - Well I've already hooked up and adaptor logger. So console.log should be fine right? Adaptor logging should already be better + - console.success should also be available + +* Replace the export { fn } from 'common' pattern + - Maybe automate it in the build? + +* Reusable functions + - A much broader concern. How do I define a function in my workflow which can be re-used in multiple jobs? + +* Seperate config out + - God I'd love to do this + - `await execute(state, config)` + - `init(config)` + +* What is a breaking change for us? + - Or, a clear policy for what constitutes a major version bump + - Configuration schema is a big sticking point here + +## Documentation + +This is probably a different consideration. But basically I want to rip the doc site up and start again. + +I want a really good, clear, concise, easy to find reference + +I want a few really good, clear, concise, easy to find tutorials + +I want some clear best practice to be outlined. We have this, but it quickly runs stale and isn't very good: https://github.com/OpenFn/adaptors/wiki/Adaptor-Wrting-Best-Practice. It needs to be immediate and obvious. + From 63f884da83abc596af6ee93eb774980cb80125aa Mon Sep 17 00:00:00 2001 From: jclark Date: Tue, 5 Mar 2024 13:02:59 +0000 Subject: [PATCH 07/14] set up an example expression which also is a mocked unit test --- packages/msgraph/examples/get-drive.js | 8 + packages/msgraph/package.json | 5 +- packages/msgraph/src/Adaptor.js | 86 +++---- packages/msgraph/src/index.js | 4 +- packages/msgraph/src/operations.js | 71 +++++- packages/msgraph/test/examples.test.js | 47 ++++ pnpm-lock.yaml | 301 ++++++++++++++++++++++++- tools/execute/execute.js | 22 ++ tools/execute/index.js | 3 + tools/execute/package.json | 15 ++ tutorial.md | 1 + 11 files changed, 502 insertions(+), 61 deletions(-) create mode 100644 packages/msgraph/examples/get-drive.js create mode 100644 packages/msgraph/test/examples.test.js create mode 100644 tools/execute/execute.js create mode 100644 tools/execute/index.js create mode 100644 tools/execute/package.json diff --git a/packages/msgraph/examples/get-drive.js b/packages/msgraph/examples/get-drive.js new file mode 100644 index 000000000..719f6ceb8 --- /dev/null +++ b/packages/msgraph/examples/get-drive.js @@ -0,0 +1,8 @@ +/** + * A simple test job which gets a drive + */ +getDrive(( + state) => ({ id: state.id }), // get the drife id from state + "default", // drive name + (state) => ({ ...state, savedDrives: state.drives }) // alias savedDrives onto state (it gets removed by the adaptor) +) diff --git a/packages/msgraph/package.json b/packages/msgraph/package.json index b2bda9c3c..838cc3579 100644 --- a/packages/msgraph/package.json +++ b/packages/msgraph/package.json @@ -12,8 +12,8 @@ }, "scripts": { "build": "pnpm clean && build-adaptor msgraph", - "test": "mocha --experimental-specifier-resolution=node --no-warnings", - "test:watch": "mocha -w --experimental-specifier-resolution=node --no-warnings", + "test": "mocha --experimental-specifier-resolution=node --experimental-vm-modules --no-warnings", + "test:watch": "mocha -w --experimental-specifier-resolution=node --experimental-vm-modules --no-warnings", "clean": "rimraf dist types docs", "pack": "pnpm pack --pack-destination ../../dist", "lint": "eslint src" @@ -34,6 +34,7 @@ "devDependencies": { "@openfn/buildtools": "workspace:^1.0.2", "@openfn/simple-ast": "0.4.1", + "@openfn/test-execute": "workspace:^", "assertion-error": "2.0.0", "chai": "4.3.6", "deep-eql": "4.1.1", diff --git a/packages/msgraph/src/Adaptor.js b/packages/msgraph/src/Adaptor.js index 9a8ba497b..28762a5e7 100644 --- a/packages/msgraph/src/Adaptor.js +++ b/packages/msgraph/src/Adaptor.js @@ -1,4 +1,4 @@ -import { execute as commonExecute } from '@openfn/language-common'; +// TODO all this is deprecated now.. import { expandReferences } from '@openfn/language-common/util'; import { @@ -9,48 +9,48 @@ import { assertResources, } from './Utils'; -/** - * Execute a sequence of operations. - * Wraps `language-common/execute` to make working with this API easier. - * @example - * execute( - * create('foo'), - * delete('bar') - * )(state) - * @private - * @param {Operations} operations - Operations to be performed. - * @returns {Operation} - */ -export function execute(...operations) { - const initialState = { - references: [], - data: null, - drives: {}, - }; - - const cleanup = finalState => { - if (finalState?.buffer) { - delete finalState.buffer; - } - if (finalState?.drives) { - delete finalState.drives; - } - - return finalState; - }; - - return state => { - return commonExecute(...operations)({ - ...initialState, - ...state, - }) - .then(cleanup) - .catch(error => { - cleanup(state); - throw error; - }); - }; -} +// /** +// * Execute a sequence of operations. +// * Wraps `language-common/execute` to make working with this API easier. +// * @example +// * execute( +// * create('foo'), +// * delete('bar') +// * )(state) +// * @private +// * @param {Operations} operations - Operations to be performed. +// * @returns {Operation} +// */ +// export function execute(...operations) { +// const initialState = { +// references: [], +// data: null, +// drives: {}, +// }; + +// const cleanup = finalState => { +// if (finalState?.buffer) { +// delete finalState.buffer; +// } +// if (finalState?.drives) { +// delete finalState.drives; +// } + +// return finalState; +// }; + +// return state => { +// return commonExecute(...operations)({ +// ...initialState, +// ...state, +// }) +// .then(cleanup) +// .catch(error => { +// cleanup(state); +// throw error; +// }); +// }; +// } /** * Get the contents or metadata of a folder. diff --git a/packages/msgraph/src/index.js b/packages/msgraph/src/index.js index a013b3648..4d2c22fd4 100644 --- a/packages/msgraph/src/index.js +++ b/packages/msgraph/src/index.js @@ -1,4 +1,2 @@ -import * as Adaptor from './Adaptor'; +import * as Adaptor from './operations'; export default Adaptor; - -export * from './Adaptor'; \ No newline at end of file diff --git a/packages/msgraph/src/operations.js b/packages/msgraph/src/operations.js index 6ac9e75c6..e902a1b12 100644 --- a/packages/msgraph/src/operations.js +++ b/packages/msgraph/src/operations.js @@ -1,16 +1,63 @@ // TODO this import maybe needs to be common/util // if the main export only exports operations -import { operation } from '@openfn/language-common' +import { operation, execute as commonExecute } from '@openfn/language-common' import * as impl from './impl'; import { request } from './Utils'; -// overide the request handler -// Difficult: request itself has quite a bit of logic that's worth testing -// including side effects -// Really I only want to mock out hte actual fetch -export const _setMocks = () => { +let customHandler; +// TODO I'd quite like to exclude this from the build? +export const setRequestHandler = (fn) => { + customHandler = fn +} + +// TODO need to work out the best pattern for this +const getRequestHandler = () => { + return customHandler || request; +} + +/** + * Execute a sequence of operations. + * Wraps `language-common/execute` to make working with this API easier. + * @example + * execute( + * create('foo'), + * delete('bar') + * )(state) + * @private + * @param {Operations} operations - Operations to be performed. + * @returns {Operation} + */ +export function execute(...operations) { + const initialState = { + references: [], + data: null, + drives: {}, + }; + + const cleanup = finalState => { + if (finalState?.buffer) { + delete finalState.buffer; + } + if (finalState?.drives) { + delete finalState.drives; + } + + return finalState; + }; + + return state => { + return commonExecute(...operations)({ + ...initialState, + ...state, + }) + .then(cleanup) + .catch(error => { + cleanup(state); + throw error; + }); + }; } /** @@ -24,8 +71,8 @@ export const _setMocks = () => { * @param {function} callback - An optional callback function * @returns {Operation} */ -export const create = operation((state, resource, data, callback) => { - return impl.create(state, request, resource, data, callback) +export const create = operation((state, resource, data, callback) => { + return impl.create(state, getRequestHandler(), resource, data, callback) }) /** @@ -40,7 +87,7 @@ export const create = operation((state, resource, data, callback) => { * @returns {Operation} */ export function get(state, path, query, callback) { - return impl.get(state, request, path, query, callback) + return impl.get(state, getRequestHandler(), path, query, callback) } @@ -61,6 +108,6 @@ export function get(state, path, query, callback) { * @param {function} [callback = s => s] (Optional) Callback function * @return {Operation} */ -export function getDrive(specifier, name, callback) { - return impl.getDrive(state, request, specifier, name, callback) -} +export const getDrive = operation((state, specifier, name, callback) => { + return impl.getDrive(state, getRequestHandler(), specifier, name, callback) +}) diff --git a/packages/msgraph/test/examples.test.js b/packages/msgraph/test/examples.test.js new file mode 100644 index 000000000..55bfc2fb6 --- /dev/null +++ b/packages/msgraph/test/examples.test.js @@ -0,0 +1,47 @@ +import { expect } from 'chai'; +import execute from '@openfn/test-execute'; + +import Adaptor from '../src/index'; +import { fixtures } from './fixtures'; + +// TODO move to tools +const loadExample = (name) => { + // TODO we cheat for now, no-one is watching + return 'getDrive((state) => ({ id: state.id }), "default", (state) => ({ ...state, savedDrives: state.drives }))' +} + +describe('examples', () => { + it('get-drive', async () => { + // Load our example code + const source = loadExample('get-drive') + + // Set up some input state + const state = { + id: 'xxx', + configuration: { + // The mock could test for these things and throw appropriately if I wanted + accessToken: 'a', + apiVersion: '1', + }, + }; + + // Set up the mock + Adaptor.setRequestHandler(async (url) => { + // assert that the URL has the correct id in it + expect(url).to.match(/xxx/) + + return fixtures.driveResponse; + }) + + // Compile and run the job against this adaptor + const finalState = await execute(source, Adaptor, state) + + // Make some final assertion against state + + // Remember that state.drives is actually removed... + expect(finalState.drives).to.be.undefined + + // So our job re-writes it + expect(finalState.savedDrives).to.eql({ default: fixtures.driveResponse }); + }) +}); \ No newline at end of file diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1f4b1886e..6f4ac8df7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1120,6 +1120,9 @@ importers: '@openfn/simple-ast': specifier: 0.4.1 version: 0.4.1 + '@openfn/test-execute': + specifier: workspace:^ + version: link:../../tools/execute assertion-error: specifier: 2.0.0 version: 2.0.0 @@ -2082,6 +2085,15 @@ importers: specifier: 17.6.0 version: 17.6.0 + tools/execute: + dependencies: + '@openfn/compiler': + specifier: ^0.0.32 + version: 0.0.32 + '@openfn/runtime': + specifier: ^1.0.0 + version: 1.1.1 + tools/import-tests: dependencies: '@openfn/language-common': @@ -3168,6 +3180,74 @@ packages: resolution: {integrity: sha512-ZnQMnLV4e7hDlUvw8H+U8ASL02SS2Gn6+9Ac3wGGLIe7+je2AeAOxPY+izIPJDfFDb7eDjev0Us8MO1iFRN8hA==} dev: true + /@inquirer/confirm@0.0.28-alpha.0: + resolution: {integrity: sha512-ZpQQMRt0yign/M2F/PRv+RwnjRhLZz2OIU3ohhDS0H6LoY2/W6xiPJpRJYxb15KN8n/QRql0yXzPg0ugeHCwKg==} + dependencies: + '@inquirer/core': 0.0.30-alpha.0 + '@inquirer/input': 0.0.28-alpha.0 + chalk: 5.2.0 + dev: false + + /@inquirer/confirm@2.0.6: + resolution: {integrity: sha512-1lPtPRq/1so8wmND43QTIn+hg5WIPpy2u3b8G2MveQ6B1Y2pm6/2Q5DEEt2ndi0kfidjPwQEjfGMlUNcXzQQVw==} + engines: {node: '>=14.18.0'} + dependencies: + '@inquirer/core': 3.1.2 + '@inquirer/type': 1.2.0 + chalk: 4.1.2 + dev: false + + /@inquirer/core@0.0.30-alpha.0: + resolution: {integrity: sha512-bLDyC8LA+Aqy0/uAo9pm53qRFBvFexdo5Z1MTXbMAniNB8SeC6HtQnd4TRCJQVYbpR4C//xVtqB+jOD3oLSaCw==} + dependencies: + '@inquirer/type': 0.0.4-alpha.0 + ansi-escapes: 6.2.0 + chalk: 5.2.0 + cli-spinners: 2.9.2 + cli-width: 4.1.0 + lodash: 4.17.21 + mute-stream: 0.0.8 + run-async: 2.4.1 + string-width: 5.1.2 + strip-ansi: 7.1.0 + wrap-ansi: 8.1.0 + dev: false + + /@inquirer/core@3.1.2: + resolution: {integrity: sha512-lR2GaqBkp42Ew9BOAOqf2pSp+ymVES1qN8OC90WWh45yeoYLl0Ty1GyCxmkKqBJtq/+Ea1MF12AdFcZcpRNFsw==} + engines: {node: '>=14.18.0'} + dependencies: + '@inquirer/type': 1.2.0 + '@types/mute-stream': 0.0.1 + '@types/node': 20.5.0 + '@types/wrap-ansi': 3.0.0 + ansi-escapes: 4.3.2 + chalk: 4.1.2 + cli-spinners: 2.9.2 + cli-width: 4.1.0 + figures: 3.2.0 + mute-stream: 1.0.0 + run-async: 3.0.0 + strip-ansi: 6.0.1 + wrap-ansi: 6.2.0 + dev: false + + /@inquirer/input@0.0.28-alpha.0: + resolution: {integrity: sha512-jbMvmAY7irZFQco9i1+H+S/yMozOEvBp+gYgk1zhP+1J/4dBywCi3E/s0U6eq/U3CUm5HaxkRphl9d9OP13Lpg==} + dependencies: + '@inquirer/core': 0.0.30-alpha.0 + chalk: 5.2.0 + dev: false + + /@inquirer/type@0.0.4-alpha.0: + resolution: {integrity: sha512-D+6Z4o89zClJkfM6tMaASjiS29YzAMi18/ZgG1nxUhMLjldSnnRUw6EceIqv4fZp5PL2O6MyZkcV9c4GgREdKg==} + dev: false + + /@inquirer/type@1.2.0: + resolution: {integrity: sha512-/vvkUkYhrjbm+RolU7V1aUFDydZVKNKqKHR5TsE+j5DXgXFwrsOPcoGUJ02K0O7q7O53CU2DOTMYCHeGZ25WHA==} + engines: {node: '>=18'} + dev: false + /@jridgewell/gen-mapping@0.1.1: resolution: {integrity: sha512-sQXCasFk+U8lWYEe66WxRDOE9PjVz4vSM51fTu3Hw+ClTpUSQb718772vH3pyS5pShp6lvQM7SxgIDXXXmOX7w==} engines: {node: '>=6.0.0'} @@ -3280,6 +3360,63 @@ packages: '@nodelib/fs.scandir': 2.1.5 fastq: 1.13.0 + /@openfn/compiler@0.0.32: + resolution: {integrity: sha512-wrMORSFLSHNzu+Eh7Yj/Or17rMaBDovW5k/ilRmtUtc0Xw8ducR1vqXN1eL4OGXBmenjCLVtXaaGA0M/MwkXVw==} + engines: {node: '>=16', pnpm: '>=7'} + dependencies: + '@openfn/describe-package': 0.0.16 + '@openfn/logger': 0.0.13 + acorn: 8.8.1 + ast-types: 0.14.2 + recast: 0.21.5 + yargs: 17.7.2 + transitivePeerDependencies: + - encoding + - supports-color + dev: false + + /@openfn/describe-package@0.0.16: + resolution: {integrity: sha512-q7MILQ/f0V3tRBMC2DxIrYp1WXvMy0SUHs3ujPv1+zNDAPCPiatnsc3UO/q8S3BA6xt6HCss0yhFdv8p7YBl+Q==} + engines: {node: '>=16', pnpm: '>=7'} + dependencies: + '@typescript/vfs': 1.5.0 + cross-fetch: 3.1.8 + node-localstorage: 2.2.1 + typescript: 4.8.4 + url-join: 5.0.0 + transitivePeerDependencies: + - encoding + - supports-color + dev: false + + /@openfn/logger@0.0.13: + resolution: {integrity: sha512-5xx2D6CJBKohhP8AENItOT8nDBUvlkeSeMm6xfxcaXcjdhn9Ff2kN75dTKbs5KdgB4GNgMOl0zb3Ju4l2HXaIA==} + engines: {node: '>=16'} + dependencies: + '@inquirer/confirm': 0.0.28-alpha.0 + chalk: 5.2.0 + fast-safe-stringify: 2.1.1 + figures: 5.0.0 + dev: false + + /@openfn/logger@1.0.1: + resolution: {integrity: sha512-WWWs5pkdQDFwGQJInMwL0D2cN17j8vkuYsVkLnsoNzlgfK5GdsDfgmtA8i7ayLgHbupQ9dcvfMiZ/lXpOAiaSQ==} + engines: {node: '>=16'} + dependencies: + '@inquirer/confirm': 2.0.6 + chalk: 5.2.0 + fast-safe-stringify: 2.1.1 + figures: 5.0.0 + dev: false + + /@openfn/runtime@1.1.1: + resolution: {integrity: sha512-K8IMd0xsjC4LKXOtTEstDDt5YSzgeQHpCRodwpiMsXE4j/yvRG0iqFUt2tdLY8IRPkUFA1C6+TXGufdosR/O2A==} + dependencies: + '@openfn/logger': 1.0.1 + fast-safe-stringify: 2.1.1 + semver: 7.6.0 + dev: false + /@openfn/simple-ast@0.3.2: resolution: {integrity: sha512-NIvZsKSBQmGjQwqv8uDFpsTQquHkpoBH09pg+SJsInoa4L8CEW1g+ZU2O9D+i4xYeNciYb1nsfJ9n9TjxYAvzg==} hasBin: true @@ -3481,6 +3618,12 @@ packages: resolution: {integrity: sha512-jhuKLIRrhvCPLqwPcx6INqmKeiA5EWrsCOPhrlFSrbrmU4ZMPjj5Ul/oLCMDO98XRUIwVm78xICz4EPCektzeQ==} dev: true + /@types/mute-stream@0.0.1: + resolution: {integrity: sha512-0yQLzYhCqGz7CQPE3iDmYjhb7KMBFOP+tBkyw+/Y2YyDI5wpS7itXXxneN1zSsUwWx3Ji6YiVYrhAnpQGS/vkw==} + dependencies: + '@types/node': 18.17.5 + dev: false + /@types/node-fetch@2.6.10: resolution: {integrity: sha512-PPpPK6F9ALFTn59Ka3BaL+qGuipRfxNE8qVgkp0bVixeiR2c2/L+IVOiBdu9JhhT22sWnQEp6YyHGI2b2+CMcA==} dependencies: @@ -3536,6 +3679,18 @@ packages: '@types/node': 18.17.5 dev: false + /@types/wrap-ansi@3.0.0: + resolution: {integrity: sha512-ltIpx+kM7g/MLRZfkbL7EsCEjfzCcScLpkg37eXEtx5kmrAKBkTJwd1GIAjDSL8wTpM6Hzn5YO4pSb91BEwu1g==} + dev: false + + /@typescript/vfs@1.5.0: + resolution: {integrity: sha512-AJS307bPgbsZZ9ggCT3wwpg3VbTKMFNHfaY/uF0ahSkYYrPF2dSSKDNIDIQAHm9qJqbLvCsSJH7yN4Vs/CsMMg==} + dependencies: + debug: 4.3.4(supports-color@8.1.1) + transitivePeerDependencies: + - supports-color + dev: false + /@ungap/promise-all-settled@1.1.2: resolution: {integrity: sha512-sL/cEvJWAnClXw0wHk85/2L0G6Sj8UB0Ctc1TEMbKSsmpRosqhwj9gWgFRZSrBr2f9tiXISwNhCPmlfqUqyb9Q==} dev: true @@ -3632,6 +3787,20 @@ packages: array-back: 3.1.0 dev: false + /ansi-escapes@4.3.2: + resolution: {integrity: sha512-gKXj5ALrKWQLsYG9jlTRmR/xKluxHV+Z9QEwNIgCfM1/uwPMCuzVVnh5mwTd+OuBZcwSIMbqssNWRm1lE51QaQ==} + engines: {node: '>=8'} + dependencies: + type-fest: 0.21.3 + dev: false + + /ansi-escapes@6.2.0: + resolution: {integrity: sha512-kzRaCqXnpzWs+3z5ABPQiVke+iq0KXkHo8xiWV4RPTi5Yli0l97BEQuhXV1s7+aSU/fu1kUuxgS4MsQ0fRuygw==} + engines: {node: '>=14.16'} + dependencies: + type-fest: 3.13.1 + dev: false + /ansi-regex@2.1.1: resolution: {integrity: sha512-TIGnTpdo+E3+pCyAluZvtED5p5wCqLdezCyhPZzKPcxvFplEt4i+W7OONCKgeZFT3+y5NZZfOOS/Bdcanm1MYA==} engines: {node: '>=0.10.0'} @@ -3897,6 +4066,20 @@ packages: dev: true optional: true + /ast-types@0.14.2: + resolution: {integrity: sha512-O0yuUDnZeQDL+ncNGlJ78BiO4jnYI3bvMsD5prT0/nsgijG/LpNBIr63gTjVTNsiGkgQhiyCShTgxt8oXOrklA==} + engines: {node: '>=4'} + dependencies: + tslib: 2.5.0 + dev: false + + /ast-types@0.15.2: + resolution: {integrity: sha512-c27loCv9QkZinsa5ProX751khO9DJl/AcB5c2KNtA6NRvHKS0PgLfcftz72KVq504vB0Gku5s2kUZzDBvQWvHg==} + engines: {node: '>=4'} + dependencies: + tslib: 2.5.0 + dev: false + /async-each@1.0.3: resolution: {integrity: sha512-z/WhQ5FPySLdvREByI2vZiTWwCnF0moMJ1hK9YQwDTHKh6I7/uSckMetoRGb5UBZPC1z0jlw+n/XCgjeH7y1AQ==} requiresBuild: true @@ -5339,6 +5522,11 @@ packages: engines: {node: '>=0.10.0'} dev: false + /cli-spinners@2.9.2: + resolution: {integrity: sha512-ywqV+5MmyL4E7ybXgKys4DugZbX0FC6LnwrhjuykIjnK9k8OQacQ7axGKnjDXWNhns0xot3bZI5h55H8yo9cJg==} + engines: {node: '>=6'} + dev: false + /cli-truncate@3.1.0: resolution: {integrity: sha512-wfOBkjXteqSnI59oPcJkcPl/ZmwvMMOj340qUIY1SKZCv0B9Cf4D4fAucRkIKQmsIuYK3x1rrgU7MeGRruiuiA==} engines: {node: ^12.20.0 || ^14.13.1 || >=16.0.0} @@ -5347,6 +5535,11 @@ packages: string-width: 5.1.2 dev: false + /cli-width@4.1.0: + resolution: {integrity: sha512-ouuZd4/dm2Sw5Gmqy6bGyNNNe1qt9RpmxveLSO7KcgsTnU7RXfsw+/bukWGo1abgBiMAic068rclZsO4IWmmxQ==} + engines: {node: '>= 12'} + dev: false + /cliui@3.2.0: resolution: {integrity: sha512-0yayqDxWQbqk3ojkYqUKqaAQ6AfNKeKWRNA8kR0WXzAsdHpP4BIaOmMAG87JGuO6qcobyW4GjxHd9PmhEd+T9w==} dependencies: @@ -5628,6 +5821,14 @@ packages: resolution: {integrity: sha512-dcKFX3jn0MpIaXjisoRvexIJVEKzaq7z2rZKxf+MSr9TkdmHmsU4m2lcLojrj/FHl8mk5VxMmYA+ftRkP/3oKQ==} dev: false + /cross-fetch@3.1.8: + resolution: {integrity: sha512-cvA+JwZoU0Xq+h6WkMvAUqPEYy92Obet6UdKLfW60qn99ftItKjB5T+BkyWOFWe2pUyfQ+IJHmpOTznqk1M6Kg==} + dependencies: + node-fetch: 2.7.0 + transitivePeerDependencies: + - encoding + dev: false + /cross-spawn@5.1.0: resolution: {integrity: sha512-pTgQJ5KC0d2hcY8eyL1IzlBPYjTkyH72XRZPnLyKus2mBfNjQs3klqbJU2VILqZryAZUt9JOb3h/mWMy23/f5A==} dependencies: @@ -7005,6 +7206,13 @@ packages: tunnel-agent: 0.6.0 dev: false + /figures@3.2.0: + resolution: {integrity: sha512-yaduQFRKLXYOGgEn6AZau90j3ggSOyiqXU0F9JZfeXYhNa+Jk4X+s45A2zg5jns87GAFa34BBm2kXw4XpNcbdg==} + engines: {node: '>=8'} + dependencies: + escape-string-regexp: 1.0.5 + dev: false + /figures@5.0.0: resolution: {integrity: sha512-ej8ksPF4x6e5wvK9yevct0UCXh8TTFlWGVLlgjZuoBH1HwjIfKE/IdL5mq89sFA7zELi1VhKpmtDnrs7zWyeyg==} engines: {node: '>=14'} @@ -9621,6 +9829,15 @@ packages: hasBin: true dev: false + /mute-stream@0.0.8: + resolution: {integrity: sha512-nnbWWOkoWyUsTjKrhgD0dcz22mdkSnpYqbEjIm2nhwhuxlSkpywJmBo8h0ZqJdkp73mb90SssHkN4rsRaBAfAA==} + dev: false + + /mute-stream@1.0.0: + resolution: {integrity: sha512-avsJQhyd+680gKXyG/sQc0nXaC6rBkPOfyHYcFb9+hdkqQkR9bdnkJ0AMZhke0oesPqIO+mFFJ+IdBc7mst4IA==} + engines: {node: ^14.17.0 || ^16.13.0 || >=18.0.0} + dev: false + /mysql@2.18.1: resolution: {integrity: sha512-Bca+gk2YWmqp2Uf6k5NFEurwY/0td0cpebAucFpY/3jhrwrVGuxU2uQFCHjU19SJfje0yQvi+rVWdq78hR5lig==} engines: {node: '>= 0.6'} @@ -9791,6 +10008,18 @@ packages: whatwg-url: 5.0.0 dev: false + /node-fetch@2.7.0: + resolution: {integrity: sha512-c4FRfUm/dbcWZ7U+1Wq0AwCyFL+3nt2bEw05wfxSz+DWpWsitgmSgYmy2dQdWyKC1694ELPqMs/YzUSNozLt8A==} + engines: {node: 4.x || >=6.0.0} + peerDependencies: + encoding: ^0.1.0 + peerDependenciesMeta: + encoding: + optional: true + dependencies: + whatwg-url: 5.0.0 + dev: false + /node-forge@1.3.1: resolution: {integrity: sha512-dPEtOeMvF9VMcYV/1Wb8CPoVAXtp6MKMlcbAt4ddqmGqUJ6fQZFXkNZNkNlfevtNkGtaSoXf/vNNNSvgrdXwtA==} engines: {node: '>= 6.13.0'} @@ -9800,6 +10029,13 @@ packages: resolution: {integrity: sha512-O5lz91xSOeoXP6DulyHfllpq+Eg00MWitZIbtPfoSEvqIHdl5gfcY6hYzDWnj0qD5tz52PI08u9qUvSVeUBeHw==} dev: false + /node-localstorage@2.2.1: + resolution: {integrity: sha512-vv8fJuOUCCvSPjDjBLlMqYMHob4aGjkmrkaE42/mZr0VT+ZAU10jRF8oTnX9+pgU9/vYJ8P7YT3Vd6ajkmzSCw==} + engines: {node: '>=0.12'} + dependencies: + write-file-atomic: 1.3.4 + dev: false + /node-releases@2.0.13: resolution: {integrity: sha512-uYr7J37ae/ORWdZeQ1xxMJe3NtdmqMC/JZK+geofDrkLUApKRHPd18/TxtBOJ4A0/+uUIliorNrfYV6s1b02eQ==} @@ -10827,6 +11063,16 @@ packages: dependencies: picomatch: 2.3.1 + /recast@0.21.5: + resolution: {integrity: sha512-hjMmLaUXAm1hIuTqOdeYObMslq/q+Xff6QE3Y2P+uoHAg2nmVlLBps2hzh1UJDdMtDTMXOFewK6ky51JQIeECg==} + engines: {node: '>= 4'} + dependencies: + ast-types: 0.15.2 + esprima: 4.0.1 + source-map: 0.6.1 + tslib: 2.5.0 + dev: false + /redent@3.0.0: resolution: {integrity: sha512-6tDA8g98We0zd0GvVeMT9arEOnTw9qM03L9cJXaCjrip1OO764RDBLBfrB4cwzNGDj5OA5ioymC9GkizgWJDUg==} engines: {node: '>=8'} @@ -11139,6 +11385,16 @@ packages: resolution: {integrity: sha512-R3wLbuAYejpxQjL/SjXo1Cjv4wcJECnMRT/FlcCfTwCBhaji9rWaRCoVEQ1SPiTJ4kKK+yh+bZLAV7SCafoDDw==} dev: false + /run-async@2.4.1: + resolution: {integrity: sha512-tvVnVv01b8c1RrA6Ep7JkStj85Guv/YrMcwqYQnwjsAS2cTmmPGBBjAjpCW7RrSodNSoE2/qg9O4bceNvUuDgQ==} + engines: {node: '>=0.12.0'} + dev: false + + /run-async@3.0.0: + resolution: {integrity: sha512-540WwVDOMxA6dN6We19EcT9sc3hkXPw5mzRNGM3FkdN/vtE9NFvj5lFAPNwUDmJjXidm3v7TC1cTE7t17Ulm1Q==} + engines: {node: '>=0.12.0'} + dev: false + /run-parallel@1.2.0: resolution: {integrity: sha512-5l4VyZR86LZ/lDxZTR6jqL8AFE2S0IFLMP26AbjsLVADxHdhB/c0GUsH+y39UfCi3dzz8OlQuPmnaJOMoDHQBA==} dependencies: @@ -11228,6 +11484,14 @@ packages: dependencies: lru-cache: 6.0.0 + /semver@7.6.0: + resolution: {integrity: sha512-EnwXhrlwXMk9gKu5/flx5sv/an57AkRplG3hTK68W7FRDN+k+OWBj65M7719OkA82XLBxrcX0KSHj+X5COhOVg==} + engines: {node: '>=10'} + hasBin: true + dependencies: + lru-cache: 6.0.0 + dev: false + /sequin@0.1.1: resolution: {integrity: sha512-hJWMZRwP75ocoBM+1/YaCsvS0j5MTPeBHJkS2/wruehl9xwtX30HlDF1Gt6UZ8HHHY8SJa2/IL+jo+JJCd59rA==} engines: {node: '>=0.4.0'} @@ -11360,6 +11624,10 @@ packages: is-fullwidth-code-point: 4.0.0 dev: false + /slide@1.1.6: + resolution: {integrity: sha512-NwrtjCg+lZoqhFU8fOwl4ay2ei8PaqCBOUV3/ektPY9trO1yQ1oXEfmHAhKArUVUr/hOHvy5f6AdP17dCM0zMw==} + dev: false + /smartwrap@2.0.2: resolution: {integrity: sha512-vCsKNQxb7PnCNd2wY1WClWifAc2lwqsG8OaswpJkVJsvMGcnEntdTCDajZCkk93Ay1U3t/9puJmb525Rg5MZBA==} engines: {node: '>=6'} @@ -12430,6 +12698,11 @@ packages: engines: {node: '>=10'} dev: true + /type-fest@0.21.3: + resolution: {integrity: sha512-t0rzBq87m3fVcduHDUFhKmyyX+9eo6WQjZvf51Ea/M0Q7+T374Jp1aUiyUl0GKxp8M/OETVHSDvmkyPgvX+X2w==} + engines: {node: '>=10'} + dev: false + /type-fest@0.6.0: resolution: {integrity: sha512-q+MB8nYR1KDLrgr4G5yemftpMC7/QLqVndBmEEdqzmNj5dcFOO4Oo8qlwZE3ULT3+Zim1F8Kq4cBnikNhlCMlg==} engines: {node: '>=8'} @@ -12440,6 +12713,11 @@ packages: engines: {node: '>=8'} dev: true + /type-fest@3.13.1: + resolution: {integrity: sha512-tLq3bSNx+xSpwvAJnzrK0Ep5CLNWjvFTOp71URMaAEWBfRb9nnJiBoUe0tF8bI4ZFO3omgBR6NvnbzVUT3Ly4g==} + engines: {node: '>=14.16'} + dev: false + /typed-array-length@1.0.4: resolution: {integrity: sha512-KjZypGq+I/H7HI5HlOoGHkWUUGq+Q0TPhQurLbyrVrvnKTBgzLhIJ7j6J/XTQOi0d1RjyZ0wdas8bKs2p0x3Ng==} dependencies: @@ -12602,6 +12880,11 @@ packages: resolution: {integrity: sha512-jk1+QP6ZJqyOiuEI9AEWQfju/nB2Pw466kbA0LEZljHwKeMgd9WrAEgEGxjPDD2+TNbbb37rTyhEfrCXfuKXnA==} dev: false + /url-join@5.0.0: + resolution: {integrity: sha512-n2huDr9h9yzd6exQVnH/jU5mr+Pfx08LRXXZhkLLetAMESRj+anQsTAh940iMrIetKAmry9coFuZQ2jY8/p3WA==} + engines: {node: ^12.20.0 || ^14.13.1 || >=16.0.0} + dev: false + /url-parse@1.5.10: resolution: {integrity: sha512-WypcfiRhfeUP9vvF0j6rw0J3hrWrw6iZv3+22h6iRMJ/8z1Tj6XfLP4DsUix5MhMPnXpiHDoKyoZ/bdCkwBCiQ==} dependencies: @@ -12870,7 +13153,6 @@ packages: ansi-styles: 4.3.0 string-width: 4.2.3 strip-ansi: 6.0.1 - dev: true /wrap-ansi@7.0.0: resolution: {integrity: sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==} @@ -12880,9 +13162,26 @@ packages: string-width: 4.2.3 strip-ansi: 6.0.1 + /wrap-ansi@8.1.0: + resolution: {integrity: sha512-si7QWI6zUMq56bESFvagtmzMdGOtoxfR+Sez11Mobfc7tm+VkUckk9bW2UeffTGVUbOksxmSw0AA2gs8g71NCQ==} + engines: {node: '>=12'} + dependencies: + ansi-styles: 6.2.1 + string-width: 5.1.2 + strip-ansi: 7.1.0 + dev: false + /wrappy@1.0.2: resolution: {integrity: sha512-l4Sp/DRseor9wL6EvV2+TuQn63dMkPjZ/sp9XkghTEbV9KlPS1xUsZ3u7/IQO4wxtcFB4bgpQPRcR3QCvezPcQ==} + /write-file-atomic@1.3.4: + resolution: {integrity: sha512-SdrHoC/yVBPpV0Xq/mUZQIpW2sWXAShb/V4pomcJXh92RuaO+f3UTWItiR3Px+pLnV2PvC2/bfn5cwr5X6Vfxw==} + dependencies: + graceful-fs: 4.2.11 + imurmurhash: 0.1.4 + slide: 1.1.6 + dev: false + /write-file-atomic@5.0.0: resolution: {integrity: sha512-R7NYMnHSlV42K54lwY9lvW6MnSm1HSJqZL3xiSgi9E7//FYaI74r2G0rd+/X6VAMkHEdzxQaU5HUOXWUz5kA/w==} engines: {node: ^14.17.0 || ^16.13.0 || >=18.0.0} diff --git a/tools/execute/execute.js b/tools/execute/execute.js new file mode 100644 index 000000000..3d6357275 --- /dev/null +++ b/tools/execute/execute.js @@ -0,0 +1,22 @@ +/** + * Helper function to execute job source in unit tests + */ +import compile from '@openfn/compiler'; +import execute from '@openfn/runtime'; + +export default (job, adaptor, state = {}) => { + // Compile without an adaptor, so there's no import statements + let compiledJob = compile(job) + + // BUT we do need to export the execute function, if present + if (adaptor.execute) { + compiledJob = `export const execute = globalThis.execute; +${compiledJob}` + } + + return execute(compiledJob, state, { + globals: { + ...adaptor, + } + }) +} \ No newline at end of file diff --git a/tools/execute/index.js b/tools/execute/index.js new file mode 100644 index 000000000..1db9a8b3e --- /dev/null +++ b/tools/execute/index.js @@ -0,0 +1,3 @@ +import execute from './execute'; + +export default execute; \ No newline at end of file diff --git a/tools/execute/package.json b/tools/execute/package.json new file mode 100644 index 000000000..60b491166 --- /dev/null +++ b/tools/execute/package.json @@ -0,0 +1,15 @@ +{ + "name": "@openfn/test-execute", + "version": "1.0.0", + "description": "Unit test helper to run jobs from source", + "main": "index.js", + "type": "module", + "private": true, + "keywords": [], + "author": "", + "license": "ISC", + "dependencies": { + "@openfn/compiler": "^0.0.32", + "@openfn/runtime": "^1.0.0" + } +} \ No newline at end of file diff --git a/tutorial.md b/tutorial.md index 8b5bdbb72..9da7add70 100644 --- a/tutorial.md +++ b/tutorial.md @@ -85,6 +85,7 @@ describe('getDrive', () => { drives: {} }; + // We could add test assertions in the mock if we wanted const mockRequest = async () => fixtures.driveResponse; // Call the implementation with the mock From ccef8a07f8668cbfc44937030be9729aae95d24c Mon Sep 17 00:00:00 2001 From: jclark Date: Tue, 5 Mar 2024 14:40:51 +0000 Subject: [PATCH 08/14] update tutorial --- packages/msgraph/examples/get-drive.js | 2 +- tutorial.md | 93 +++++++++++++++++++++----- 2 files changed, 78 insertions(+), 17 deletions(-) diff --git a/packages/msgraph/examples/get-drive.js b/packages/msgraph/examples/get-drive.js index 719f6ceb8..6e95683c5 100644 --- a/packages/msgraph/examples/get-drive.js +++ b/packages/msgraph/examples/get-drive.js @@ -2,7 +2,7 @@ * A simple test job which gets a drive */ getDrive(( - state) => ({ id: state.id }), // get the drife id from state + state) => ({ id: state.id }), // get the drive id from state "default", // drive name (state) => ({ ...state, savedDrives: state.drives }) // alias savedDrives onto state (it gets removed by the adaptor) ) diff --git a/tutorial.md b/tutorial.md index 9da7add70..6d7cf689c 100644 --- a/tutorial.md +++ b/tutorial.md @@ -2,19 +2,13 @@ A nice clean presentation about how I'm thinking about next gen adaptors. -## TODO - -Immediate short-term todos, ideally before nairobi - -- examples as tests - ## Features Here's what I've introduced * Operation factories -* operation/implementation split -* real runtime/job tests +* Operation/implementation split +* Real runtime/job tests * A pattern for mocking (probably better than demanding docker containers?) ## Motivations @@ -24,7 +18,7 @@ Here are the problems I'm trying to solve * Some sort of capacity for useful unit tests * They're good for development, maintenance and preventing regressions * Clarity over what an operation is, and why it matters -* Encouraging good documetnation _in the right place_ +* Encouraging good documentation _in the right place_ * I see lots of problems of devs documenting the wrong functions, wasting time and energy ## Examples @@ -39,7 +33,7 @@ Maybe `salesforce`, `dhis2` or `http` would be better examples? Here's what's not right yet: -* Basically I think this adds formality and complication. Does any of this acutally make adaptor writing easier? +* I worry that this adds formality and complication. Does any of this acutally make adaptor writing easier? * Expanding references. I'd really like to standardise and simplify this further - jsdoc path vs dataValue() vs open function - I am sure that you can just make expand references read '$a.b.c' as a jsonpath @@ -47,7 +41,7 @@ Here's what's not right yet: ## Operation Factories Here's an operation factory -``` +```js export const get = operation((state, url, auth, callback) => { // code goes here return state @@ -96,8 +90,80 @@ describe('getDrive', () => { }); ``` +The ability to mock the implementation like this also enables real runtime testing + ## Real runtime tests +I really hate the existing test syntax we use right now. What I actually want to do is write a job and pass it to the actual run time so execution. + +So I've implemented this! + + +First, we create an example job in a source file + +```js +// examples.get-drive.js +/** + * A simple test job which gets a drive + */ +getDrive(( + state) => ({ id: state.id }), // get the drive id from state + "default", // drive name + (state) => ({ ...state, savedDrives: state.drives }) // alias savedDrives onto state (it gets removed by the adaptor) +) + +``` + +(we can also do this inline in a unit test in a string) + +In a unit test, we can: +* Load this example source +* Load the adaptor module +* Set a mock client/request object in the adaptor +* Pass the source, input state and adaptor into the actual runtime and compiler + +That gives us a test that looks like this: +```js +describe('examples', () => { + it('get-drive', async () => { + // Load our example code + const source = loadExample('get-drive') + + // Set up some input state + const state = { + id: 'xxx', + }; + + // Set up the mock + Adaptor.setRequestHandler(async (url) => { + // Return some mock data (perhaps as a pre-saved fixture, or we define it in-line) + return { ... } + }) + + // Compile and run the job against this adaptor + const finalState = await execute(source, Adaptor, state) + + // Assert on the result state + expect(finalState).to.eql({ ...}); + }) +}); +``` +What's nice about this pattern is that we can run test assertions on the mock handler function as well on the result state + +Note: there is an alternative to all this for real runtime tests. It should be possible to mock out the http requests with undici. I think? Surely at some level of abstraction we can do it. + +I think it's a lot harder to map and mock the different request URLs, but it may be a viable option. + +## Examples + +There's one other benefit of this approach. + +The files in examples.js are just jobs. Regular jobs which you can run through the CLI (given the correct input state and credentials). + +But we should also be able to load these in docusaurus and present them in docs. They're real, runnable, testable execution examples. + +Surley we can take advantage of this? + ## Future Work & Gripes Here's some other stuff we need to look at @@ -105,11 +171,6 @@ Here's some other stuff we need to look at * A really good way to "record" a "curl" to the datasource - I almost see a CLI command `openfn record` which will run a job (probably written against common.http) and save the final `state.data` (or something) to a json file in the right format to be used by the mock. maybe there's a map of `'path.json': data` So you write a job to populate your mock data, then just run it to save the data into the monorepo, then run the unit tests which should run against the mock data. -* Can unit tests also be examples? - - My feeling is that if you save the job code to a file like "get-transactions.js", and that file is a fully runnable job (run it from the cli with state!), you can also load that into the website (somehow) as an example - - I am not sure how you setup the mock - I guess that happens in the test file. We can append a magic string or something, do something to prep the client for mocking - - I am also not sure where documentation goes - I guess in a comment at the top - * Logging - Well I've already hooked up and adaptor logger. So console.log should be fine right? Adaptor logging should already be better - console.success should also be available From 57e6f860a44cc14973fc7f7170675f1d61ad6820 Mon Sep 17 00:00:00 2001 From: jclark Date: Tue, 5 Mar 2024 15:06:17 +0000 Subject: [PATCH 09/14] msgraph: fully migrate to new structure --- packages/common/src/Adaptor.js | 16 +- packages/common/src/operations.js | 3 +- packages/common/src/util/index.js | 14 + packages/msgraph/src/Adaptor.js | 281 ------------------ packages/msgraph/src/impl.js | 160 +++++++++- packages/msgraph/src/operations.js | 113 +++++-- packages/msgraph/src/{Utils.js => utils.js} | 6 - .../{Adaptor.test.js => Adaptor.deprecated} | 0 packages/msgraph/test/integations.js | 33 -- packages/msgraph/test/mock.js | 4 - .../{mockAgent.js => mockAgent.deprecated} | 0 tutorial.md | 4 + 12 files changed, 275 insertions(+), 359 deletions(-) delete mode 100644 packages/msgraph/src/Adaptor.js rename packages/msgraph/src/{Utils.js => utils.js} (97%) rename packages/msgraph/test/{Adaptor.test.js => Adaptor.deprecated} (100%) delete mode 100644 packages/msgraph/test/integations.js delete mode 100644 packages/msgraph/test/mock.js rename packages/msgraph/test/{mockAgent.js => mockAgent.deprecated} (100%) diff --git a/packages/common/src/Adaptor.js b/packages/common/src/Adaptor.js index 17f419a81..7b08c84e3 100644 --- a/packages/common/src/Adaptor.js +++ b/packages/common/src/Adaptor.js @@ -13,21 +13,9 @@ export * as beta from './beta'; export * as http from './http.deprecated'; export * as dateFns from './dateFns'; -const schemaCache = {}; +export { each, fn, sourceValue } from './operations' -// operation creates a new operation factory -// It accepts a function which takes user arguments AND state -// It returns a function which acccepts user args -// Which in turn returns a function which accepts state -// and then triggers the original function -// It's complex BUT the end user code should be way way simpler -export const operation = (fn) => { - return (...args) => { - return (state) => { - return fn(state, ...args) - } - } -} +const schemaCache = {}; /** diff --git a/packages/common/src/operations.js b/packages/common/src/operations.js index 4b9922d43..bee3dcb48 100644 --- a/packages/common/src/operations.js +++ b/packages/common/src/operations.js @@ -2,7 +2,8 @@ // ie, everything you can call from job code import { JSONPath } from 'jsonpath-plus'; -import { operation, asData } from './Adaptor'; +import { asData } from './Adaptor'; +import { operation } from './util'; /** * Creates a custom step (or operation) for more flexible job writing. diff --git a/packages/common/src/util/index.js b/packages/common/src/util/index.js index a97522682..8a299fd21 100644 --- a/packages/common/src/util/index.js +++ b/packages/common/src/util/index.js @@ -1,2 +1,16 @@ export * from './http'; export * from './references'; + +// operation creates a new operation factory +// It accepts a function which takes user arguments AND state +// It returns a function which acccepts user args +// Which in turn returns a function which accepts state +// and then triggers the original function +// It's complex BUT the end user code should be way way simpler +export const operation = (fn) => { + return (...args) => { + return (state) => { + return fn(state, ...args) + } + } +} \ No newline at end of file diff --git a/packages/msgraph/src/Adaptor.js b/packages/msgraph/src/Adaptor.js deleted file mode 100644 index 28762a5e7..000000000 --- a/packages/msgraph/src/Adaptor.js +++ /dev/null @@ -1,281 +0,0 @@ -// TODO all this is deprecated now.. -import { expandReferences } from '@openfn/language-common/util'; - -import { - request, - setUrl, - handleResponse, - assertDrive, - assertResources, -} from './Utils'; - -// /** -// * Execute a sequence of operations. -// * Wraps `language-common/execute` to make working with this API easier. -// * @example -// * execute( -// * create('foo'), -// * delete('bar') -// * )(state) -// * @private -// * @param {Operations} operations - Operations to be performed. -// * @returns {Operation} -// */ -// export function execute(...operations) { -// const initialState = { -// references: [], -// data: null, -// drives: {}, -// }; - -// const cleanup = finalState => { -// if (finalState?.buffer) { -// delete finalState.buffer; -// } -// if (finalState?.drives) { -// delete finalState.drives; -// } - -// return finalState; -// }; - -// return state => { -// return commonExecute(...operations)({ -// ...initialState, -// ...state, -// }) -// .then(cleanup) -// .catch(error => { -// cleanup(state); -// throw error; -// }); -// }; -// } - -/** - * Get the contents or metadata of a folder. - * @public - * @example Get a folder by ID - * getFolder('01LUM6XOCKDTZKQC7AVZF2VMHE2I3O6OY3') - * @example Get a folder for a named drive by id - * getFolder("01LUM6XOCKDTZKQC7AVZF2VMHE2I3O6OY3",{ driveName: "mydrive"}) - * @param {string} pathOrId - A path to a folder or folder id - * @param {object} options - (Optional) Query parameters - * @param {function} [callback = s => s] (Optional) Callback function - * @return {Operation} - */ -export function getFolder(pathOrId, options, callback = s => s) { - return async state => { - const defaultOptions = { - driveName: 'default', // Named drive in state.drives - metadata: false, // If false return folder files if true return folder metadata - // $filter: '', // Eg: "file/mimeType eq \'application/vnd.ms-excel\'" - }; - const { accessToken, apiVersion } = state.configuration; - const [resolvedPathOrId, resolvedOptions] = expandReferences( - state, - pathOrId, - options - ); - - const { driveName, metadata } = { ...defaultOptions, ...resolvedOptions }; - - assertDrive(state, driveName); - - const { id: driveId } = state.drives[driveName]; - - let resource; - - if (resolvedPathOrId.startsWith('/')) { - resource = `drives/${driveId}/root:/${encodeURIComponent( - resolvedPathOrId - )}`; - } else { - resource = `drives/${driveId}/items/${resolvedPathOrId}`; - } - - if (!metadata) { - resource += resolvedPathOrId.startsWith('/') ? ':/children' : '/children'; - } - - const url = setUrl(resource, apiVersion); - - return request(url, { accessToken }).then(response => - handleResponse(response, state, callback) - ); - }; -} - -/** - * Get file metadata or file content. - * @public - * @example Get a file by ID - * getFile('01LUM6XOGRONYNTZ26DBBJPTN5IFTQPBIW') - * @example Get a file for a named drive by id - * getFile("01LUM6XOGRONYNTZ26DBBJPTN5IFTQPBIW",{ driveName: "mydrive"}) - * @param {string} pathOrId - A path to a file or file id - * @param {object} options - (Optional) Query parameters - * @param {function} [callback = s => s] (Optional) Callback function - * @return {Operation} - */ -export function getFile(pathOrId, options, callback = s => s) { - const defaultOptions = { - driveName: 'default', // named drive in state.drives - metadata: false, // Returns file msgraph metadata - // $filter: '', // Eg: "file/mimeType eq \'application/vnd.ms-excel\'" - // select: '', // Eg: id,@microsoft.graph.downloadUrl - }; - return async state => { - const { accessToken, apiVersion } = state.configuration; - const [resolvedPathOrId, resolvedOptions] = expandReferences( - state, - pathOrId, - options - ); - - const { driveName, metadata } = { - ...defaultOptions, - ...resolvedOptions, - }; - - assertDrive(state, driveName); - - const { id: driveId } = state.drives[driveName]; - - let resource; - - if (resolvedPathOrId.startsWith('/')) { - resource = `drives/${driveId}/root:/${encodeURIComponent( - resolvedPathOrId - )}`; - } else { - resource = `drives/${driveId}/items/${resolvedPathOrId}`; - } - - if (!metadata) { - resource += resolvedPathOrId.startsWith('/') ? ':/content' : '/content'; - } - - const url = setUrl(resource, apiVersion); - - const response = await request(url, { - accessToken, - parseAs: metadata ? 'json' : 'text', - }); - - return handleResponse(response, state, callback); - }; -} - -const defaultResource = { - contentType: 'application/octet-stream', - driveId: '', - folderId: '', - fileName: 'sheet.xls', - onConflict: 'replace', -}; - -/** - * Upload a file to a drive - * @public - * @example - * Upload Excel file to a drive using `driveId` and `parantItemId` - * uploadFile( - * state => ({ - * driveId: state.driveId, - * folderId: state.folderId, - * fileName: `Tracker.xlsx`, - * }), - * state => state.buffer - * ); - * @example - * Upload Excel file to a SharePoint drive using `siteId` and `parantItemId` - * uploadFile( - * state => ({ - * siteId: state.siteId, - * folderId: state.folderId, - * fileName: `Report.xlsx`, - * }), - * state => state.buffer - * ); - * @function - * @param {Object} resource - Resource Object - * @param {String} [resource.driveId] - Drive Id - * @param {String} [resource.driveId] - Site Id - * @param {String} [resource.folderId] - Parent folder id - * @param {String} [resource.contentType] - Resource content-type - * @param {String} [resource.onConflict] - Specify conflict behavior if file with the same name exists. Can be "rename | fail | replace" - * @param {Object} data - A buffer containing the file. - * @param {Function} callback - Optional callback function - * @returns {Operation} - */ -export function uploadFile(resource, data, callback) { - return async state => { - const { accessToken, apiVersion } = state.configuration; - - const [resolvedResource, resolvedData] = expandReferences( - state, - resource, - data - ); - - const { contentType, driveId, siteId, folderId, onConflict, fileName } = { - ...defaultResource, - ...resolvedResource, - }; - - assertResources({ driveId, siteId, folderId }); - - const path = - (driveId && - `drives/${driveId}/items/${folderId}:/${fileName}:/createUploadSession`) || - (siteId && - `sites/${siteId}/drive/items/${folderId}:/${fileName}:/createUploadSession`); - - const uploadSession = await request(setUrl(path, apiVersion), { - method: 'POST', - accessToken, - headers: { - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ - '@microsoft.graph.conflictBehavior': onConflict, - name: fileName, - }), - }); - - const uploadUrl = uploadSession.uploadUrl; - - console.log(`Uploading file...`); - - return request(uploadUrl, { - method: 'PUT', - accessToken, - headers: { - 'Content-Type': contentType, - 'Content-Length': `${resolvedData.length}`, - 'Content-Range': `bytes 0-${resolvedData.length - 1}/${ - resolvedData.length - }`, - }, - - body: resolvedData, - }).then(response => handleResponse(response, state, callback)); - }; -} - -export { request, sheetToBuffer } from './Utils'; - -export { - dataPath, - dataValue, - dateFns, - each, - field, - fields, - fn, - lastReferenceValue, - merge, - sourceValue, - parseCsv, -} from '@openfn/language-common'; diff --git a/packages/msgraph/src/impl.js b/packages/msgraph/src/impl.js index ad422a630..9cbc6a94d 100644 --- a/packages/msgraph/src/impl.js +++ b/packages/msgraph/src/impl.js @@ -1,9 +1,13 @@ -import { handleResponse, setUrl } from './Utils'; +import { handleResponse, setUrl } from './utils'; import { expandReferences } from '@openfn/language-common/util'; +// Default the callback here in the impl, so that unit tests +// can optionall exclude it +const noop = s => s; + // This is a totally normal js function // It can be unit tested by passing a request function in -export const create = (state, request, resource, data, callback) => { +export const create = (state, request, resource, data, callback = noop) => { const [resolvedResource, resolvedData] = expandReferences( state, resource, @@ -36,7 +40,7 @@ export const get = (state, request, path, query) => { ); } -export const getDrive = (state, request, specifier, name = 'default', callback = s => s) => { +export const getDrive = (state, request, specifier, name = 'default', callback = noop) => { const { accessToken, apiVersion } = state.configuration; const [resolvedSpecifier, resolvedName] = expandReferences( state, @@ -59,4 +63,154 @@ export const getDrive = (state, request, specifier, name = 'default', callback = state.drives[resolvedName] = response; return callback(state); }); +} + +export const getFolder = (state, request, pathOrId, options, callback = noop) => { + const defaultOptions = { + driveName: 'default', // Named drive in state.drives + metadata: false, // If false return folder files if true return folder metadata + // $filter: '', // Eg: "file/mimeType eq \'application/vnd.ms-excel\'" + }; + const { accessToken, apiVersion } = state.configuration; + const [resolvedPathOrId, resolvedOptions] = expandReferences( + state, + pathOrId, + options + ); + + const { driveName, metadata } = { ...defaultOptions, ...resolvedOptions }; + + assertDrive(state, driveName); + + const { id: driveId } = state.drives[driveName]; + + let resource; + + if (resolvedPathOrId.startsWith('/')) { + resource = `drives/${driveId}/root:/${encodeURIComponent( + resolvedPathOrId + )}`; + } else { + resource = `drives/${driveId}/items/${resolvedPathOrId}`; + } + + if (!metadata) { + resource += resolvedPathOrId.startsWith('/') ? ':/children' : '/children'; + } + + const url = setUrl(resource, apiVersion); + + return request(url, { accessToken }).then(response => + handleResponse(response, state, callback) + ); +} + +export const getFile = async (state, request, pathOrId, options, callback) => { + const defaultOptions = { + driveName: 'default', // named drive in state.drives + metadata: false, // Returns file msgraph metadata + // $filter: '', // Eg: "file/mimeType eq \'application/vnd.ms-excel\'" + // select: '', // Eg: id,@microsoft.graph.downloadUrl + }; + + const { accessToken, apiVersion } = state.configuration; + const [resolvedPathOrId, resolvedOptions] = expandReferences( + state, + pathOrId, + options + ); + + const { driveName, metadata } = { + ...defaultOptions, + ...resolvedOptions, + }; + + assertDrive(state, driveName); + + const { id: driveId } = state.drives[driveName]; + + let resource; + + if (resolvedPathOrId.startsWith('/')) { + resource = `drives/${driveId}/root:/${encodeURIComponent( + resolvedPathOrId + )}`; + } else { + resource = `drives/${driveId}/items/${resolvedPathOrId}`; + } + + if (!metadata) { + resource += resolvedPathOrId.startsWith('/') ? ':/content' : '/content'; + } + + const url = setUrl(resource, apiVersion); + + const response = await request(url, { + accessToken, + parseAs: metadata ? 'json' : 'text', + }); + + return handleResponse(response, state, callback); +} + +export const uploadFile = async (state, request, resource, data, callback) => { + const defaultResource = { + contentType: 'application/octet-stream', + driveId: '', + folderId: '', + fileName: 'sheet.xls', + onConflict: 'replace', + }; + + const { accessToken, apiVersion } = state.configuration; + + const [resolvedResource, resolvedData] = expandReferences( + state, + resource, + data + ); + + const { contentType, driveId, siteId, folderId, onConflict, fileName } = { + ...defaultResource, + ...resolvedResource, + }; + + assertResources({ driveId, siteId, folderId }); + + const path = + (driveId && + `drives/${driveId}/items/${folderId}:/${fileName}:/createUploadSession`) || + (siteId && + `sites/${siteId}/drive/items/${folderId}:/${fileName}:/createUploadSession`); + + const uploadSession = await request(setUrl(path, apiVersion), { + method: 'POST', + accessToken, + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ + '@microsoft.graph.conflictBehavior': onConflict, + name: fileName, + }), + }); + + const uploadUrl = uploadSession.uploadUrl; + + console.log(`Uploading file...`); + + const response = await request(uploadUrl, { + method: 'PUT', + accessToken, + headers: { + 'Content-Type': contentType, + 'Content-Length': `${resolvedData.length}`, + 'Content-Range': `bytes 0-${resolvedData.length - 1}/${ + resolvedData.length + }`, + }, + body: resolvedData, + }) + + return handleResponse(response, state, callback); } \ No newline at end of file diff --git a/packages/msgraph/src/operations.js b/packages/msgraph/src/operations.js index e902a1b12..ca21db58c 100644 --- a/packages/msgraph/src/operations.js +++ b/packages/msgraph/src/operations.js @@ -1,13 +1,12 @@ +import { execute as commonExecute } from '@openfn/language-common' +import { operation } from '@openfn/language-common/util' -// TODO this import maybe needs to be common/util -// if the main export only exports operations -import { operation, execute as commonExecute } from '@openfn/language-common' import * as impl from './impl'; -import { request } from './Utils'; +import { request } from './utils'; let customHandler; -// TODO I'd quite like to exclude this from the build? +// TODO I'd quite like to exclude this from the build? export const setRequestHandler = (fn) => { customHandler = fn } @@ -20,14 +19,6 @@ const getRequestHandler = () => { /** * Execute a sequence of operations. * Wraps `language-common/execute` to make working with this API easier. - * @example - * execute( - * create('foo'), - * delete('bar') - * )(state) - * @private - * @param {Operations} operations - Operations to be performed. - * @returns {Operation} */ export function execute(...operations) { const initialState = { @@ -61,7 +52,7 @@ export function execute(...operations) { } /** - * Create some resource in msgraph + * Create some resource in msgraph. * @public * @example * create("applications", {"displayName": "My App"}) @@ -86,16 +77,16 @@ export const create = operation((state, resource, data, callback) => { * @param {function} callback - (Optional) Callback function * @returns {Operation} */ -export function get(state, path, query, callback) { +export const get = operation((state, path, query, callback) => { return impl.get(state, getRequestHandler(), path, query, callback) -} +}) /** * Get a Drive or SharePoint document library. The drive metadata will be written * to state.drives, where it can be used by other adaptor functions. * Pass { id } to get a drive by id or { id, owner } to get default drive for - * some parent resource, like a group + * some parent resource, like a group. * @public * @example Get a drive by ID * getDrive({ id: "YXzpkoLwR06bxC8tNdg71m" }) @@ -111,3 +102,91 @@ export function get(state, path, query, callback) { export const getDrive = operation((state, specifier, name, callback) => { return impl.getDrive(state, getRequestHandler(), specifier, name, callback) }) + +/** + * Get the contents or metadata of a folder. + * @public + * @example Get a folder by ID + * getFolder('01LUM6XOCKDTZKQC7AVZF2VMHE2I3O6OY3') + * @example Get a folder for a named drive by id + * getFolder("01LUM6XOCKDTZKQC7AVZF2VMHE2I3O6OY3",{ driveName: "mydrive"}) + * @param {string} pathOrId - A path to a folder or folder id + * @param {object} options - (Optional) Query parameters + * @param {function} [callback = s => s] (Optional) Callback function + * @return {Operation} + */ +export const getFolder = operation((state, pathOrId, options, callback) => { + return impl.getFolder(state, getRequestHandler(), pathOrId, options, callback) +}); + + +/** + * Get file metadata or file content. + * @public + * @example Get a file by ID + * getFile('01LUM6XOGRONYNTZ26DBBJPTN5IFTQPBIW') + * @example Get a file for a named drive by id + * getFile("01LUM6XOGRONYNTZ26DBBJPTN5IFTQPBIW",{ driveName: "mydrive"}) + * @param {string} pathOrId - A path to a file or file id + * @param {object} options - (Optional) Query parameters + * @param {function} [callback = s => s] (Optional) Callback function + * @return {Operation} + */ +export const getFile = operation((state, pathOrId, options, callback) => { + return impl.getFile(state, getRequestHandler(), pathOrId, options, callback) +}); + +/** + * Upload a file to a drive. + * @public + * @example + * Upload Excel file to a drive using `driveId` and `parantItemId` + * uploadFile( + * state => ({ + * driveId: state.driveId, + * folderId: state.folderId, + * fileName: `Tracker.xlsx`, + * }), + * state => state.buffer + * ); + * @example + * Upload Excel file to a SharePoint drive using `siteId` and `parantItemId` + * uploadFile( + * state => ({ + * siteId: state.siteId, + * folderId: state.folderId, + * fileName: `Report.xlsx`, + * }), + * state => state.buffer + * ); + * @function + * @param {Object} resource - Resource Object + * @param {String} [resource.driveId] - Drive Id + * @param {String} [resource.driveId] - Site Id + * @param {String} [resource.folderId] - Parent folder id + * @param {String} [resource.contentType] - Resource content-type + * @param {String} [resource.onConflict] - Specify conflict behavior if file with the same name exists. Can be "rename | fail | replace" + * @param {Object} data - A buffer containing the file. + * @param {Function} callback - Optional callback function + * @returns {Operation} + */ +export const uploadFile = operation((state, resource, data, callback) => { + return impl.uploadFile(state, getRequestHandler(), pathOrId, options, callback) +}); + +export { request, sheetToBuffer } from './utils'; + +export { + dataPath, + dataValue, + dateFns, + each, + field, + fields, + fn, + lastReferenceValue, + merge, + sourceValue, + parseCsv, +} from '@openfn/language-common'; + diff --git a/packages/msgraph/src/Utils.js b/packages/msgraph/src/utils.js similarity index 97% rename from packages/msgraph/src/Utils.js rename to packages/msgraph/src/utils.js index 344e24501..0efe53173 100644 --- a/packages/msgraph/src/Utils.js +++ b/packages/msgraph/src/utils.js @@ -1,14 +1,8 @@ -// import xlsx from 'xlsx'; import { fetch } from 'undici'; import { Readable, Writable } from 'node:stream'; import { composeNextState, asData } from '@openfn/language-common'; import { expandReferences } from '@openfn/language-common/util'; -// should be built out of this file -export const setRequestFunction = (mockFetch) => { - fetch = mockFetch -} - export function assertDrive(state, driveName) { if (!state.drives[driveName]) { const errorString = [ diff --git a/packages/msgraph/test/Adaptor.test.js b/packages/msgraph/test/Adaptor.deprecated similarity index 100% rename from packages/msgraph/test/Adaptor.test.js rename to packages/msgraph/test/Adaptor.deprecated diff --git a/packages/msgraph/test/integations.js b/packages/msgraph/test/integations.js deleted file mode 100644 index af291f3fb..000000000 --- a/packages/msgraph/test/integations.js +++ /dev/null @@ -1,33 +0,0 @@ -// Integation testing - -// Load a test source script from examples - -// Inject the mock - -// run it - -// Check the result - -import * as Operations from './operations' - -// load a mock request handler here -// this should match urls -const mocks = { - 'www': someFilePayload -} - -// Hang on, what if I setup the mock fro here, before execute? -// I can prime the adaptor scope -Operations._setMocks(mocks) - -const job = ` -getDrive('some-id') -getFile('some-file') -` - -// execute with custom globals -// https://github.com/OpenFn/adaptors/pull/296/files#diff-9a88bc922d602321be848d9b9251be018a4b801387034c2fc1cfda54ef554ff5 -const finalState = await execute(job, { - Adaptor - mock: mocks // this sets the mock object inside the job -}) \ No newline at end of file diff --git a/packages/msgraph/test/mock.js b/packages/msgraph/test/mock.js deleted file mode 100644 index 1f9d78a37..000000000 --- a/packages/msgraph/test/mock.js +++ /dev/null @@ -1,4 +0,0 @@ -const initMock = (endpoints) => { - // here we basically need to override the request function - // Ho do we do this? -} \ No newline at end of file diff --git a/packages/msgraph/test/mockAgent.js b/packages/msgraph/test/mockAgent.deprecated similarity index 100% rename from packages/msgraph/test/mockAgent.js rename to packages/msgraph/test/mockAgent.deprecated diff --git a/tutorial.md b/tutorial.md index 6d7cf689c..00c1260ef 100644 --- a/tutorial.md +++ b/tutorial.md @@ -37,6 +37,10 @@ Here's what's not right yet: * Expanding references. I'd really like to standardise and simplify this further - jsdoc path vs dataValue() vs open function - I am sure that you can just make expand references read '$a.b.c' as a jsonpath +* I'm not totally sold on some parts of the mocking pattern + - Getting mock data feels a bit hard (although there may be an answer to that here somewhere) + - The setclient function on the adaptor is a bit awkward + - Can we exclude the setClient function from the final build? ## Operation Factories From fb18a8a9a4f46255431fccaca52cc7180b83a245 Mon Sep 17 00:00:00 2001 From: jclark Date: Tue, 5 Mar 2024 16:54:39 +0000 Subject: [PATCH 10/14] uodate notes --- tutorial.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tutorial.md b/tutorial.md index 00c1260ef..f4fc8be4e 100644 --- a/tutorial.md +++ b/tutorial.md @@ -2,7 +2,7 @@ A nice clean presentation about how I'm thinking about next gen adaptors. -## Features +### Features Here's what I've introduced @@ -11,7 +11,7 @@ Here's what I've introduced * Real runtime/job tests * A pattern for mocking (probably better than demanding docker containers?) -## Motivations +### Motivations Here are the problems I'm trying to solve @@ -21,7 +21,7 @@ Here are the problems I'm trying to solve * Encouraging good documentation _in the right place_ * I see lots of problems of devs documenting the wrong functions, wasting time and energy -## Examples +### Examples I've started implementing this stuff in `msgraph` (and common) to see how it all comes together @@ -29,7 +29,7 @@ I should really push this further and will try and spend some time on it before Maybe `salesforce`, `dhis2` or `http` would be better examples? -## Issues +### Issues Here's what's not right yet: @@ -42,7 +42,7 @@ Here's what's not right yet: - The setclient function on the adaptor is a bit awkward - Can we exclude the setClient function from the final build? -## Operation Factories +### Operation Factories Here's an operation factory ```js @@ -56,7 +56,7 @@ Why is this good? - There's less complicated nesting of arrow functions - It's clear and explicit that this is an Operation -## Seperate the implementation +### Seperate the implementation This makes it really easy to write unit tests against an implementation. You just pass a mock client in your tests. @@ -96,7 +96,7 @@ describe('getDrive', () => { The ability to mock the implementation like this also enables real runtime testing -## Real runtime tests +### Real runtime tests I really hate the existing test syntax we use right now. What I actually want to do is write a job and pass it to the actual run time so execution. @@ -108,7 +108,7 @@ First, we create an example job in a source file ```js // examples.get-drive.js /** - * A simple test job which gets a drive + * A simple test job which gets a drive by id */ getDrive(( state) => ({ id: state.id }), // get the drive id from state @@ -158,7 +158,7 @@ Note: there is an alternative to all this for real runtime tests. It should be p I think it's a lot harder to map and mock the different request URLs, but it may be a viable option. -## Examples +### Examples There's one other benefit of this approach. @@ -168,7 +168,7 @@ But we should also be able to load these in docusaurus and present them in docs. Surley we can take advantage of this? -## Future Work & Gripes +### Future Work & Gripes Here's some other stuff we need to look at @@ -194,7 +194,7 @@ Here's some other stuff we need to look at - Or, a clear policy for what constitutes a major version bump - Configuration schema is a big sticking point here -## Documentation +### Documentation This is probably a different consideration. But basically I want to rip the doc site up and start again. From 9b25167abb33d37cd781acbff57ec659db1fcbe5 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Sun, 10 Mar 2024 18:24:36 +0300 Subject: [PATCH 11/14] play with mocks as first-class citizens --- packages/msgraph/examples/get-drive.js | 4 +- .../msgraph/{test => src/mock}/fixtures.js | 0 packages/msgraph/src/mock/mock.js | 93 +++++++++++++++++++ packages/msgraph/src/operations.js | 30 ++++-- packages/msgraph/test/examples.test.js | 25 +++-- packages/msgraph/test/mock.test.js | 52 +++++++++++ tutorial.md | 1 + 7 files changed, 181 insertions(+), 24 deletions(-) rename packages/msgraph/{test => src/mock}/fixtures.js (100%) create mode 100644 packages/msgraph/src/mock/mock.js create mode 100644 packages/msgraph/test/mock.test.js diff --git a/packages/msgraph/examples/get-drive.js b/packages/msgraph/examples/get-drive.js index 6e95683c5..8a64f0c83 100644 --- a/packages/msgraph/examples/get-drive.js +++ b/packages/msgraph/examples/get-drive.js @@ -1,8 +1,8 @@ /** * A simple test job which gets a drive */ -getDrive(( - state) => ({ id: state.id }), // get the drive id from state +getDrive( + (state) => ({ id: state.id }), // get the drive id from state "default", // drive name (state) => ({ ...state, savedDrives: state.drives }) // alias savedDrives onto state (it gets removed by the adaptor) ) diff --git a/packages/msgraph/test/fixtures.js b/packages/msgraph/src/mock/fixtures.js similarity index 100% rename from packages/msgraph/test/fixtures.js rename to packages/msgraph/src/mock/fixtures.js diff --git a/packages/msgraph/src/mock/mock.js b/packages/msgraph/src/mock/mock.js new file mode 100644 index 000000000..1675995c4 --- /dev/null +++ b/packages/msgraph/src/mock/mock.js @@ -0,0 +1,93 @@ +import { setGlobalDispatcher, MockAgent } from 'undici'; +import { fixtures } from './fixtures'; + +/** + * This provides a mock interface to the msgraph client + * + * each function should be overridable through a common setMock API + * + * We need to document this in a way that the playground can use + */ + +// first step: mock with default data each msgraph function + +// ok that's fun, it's a http client + +// So this DOES use undici. So I think I can just write interceptors, rght? + +let mockAgent; +let mockPool; + +// TODO this was copied out of util but it should be a common helper +// actually we don't need this +// export function getBaseUrl(state) { +// const{ resource, apiVersion} = state.configuration +// // if (isValidHttpUrl(resource)) return resource; + +// const pathSuffix = apiVersion +// ? `${apiVersion}/${resource}` +// : `v1.0/${resource}`; +// return `https://graph.microsoft.com/${pathSuffix}`; +// } + +// To make custom overrides easier, export some stanadard patterns +export const patterns = { + // TODO the regex might be a bit hard here, as we + // need to distinguish the get drive patterns + drives: /\/drives\// +} + +export const enable = (state) => { + // set the global dispacher on undici + mockAgent = new MockAgent(); + + // Don't let the agentcall out to the real internet + mockAgent.disableNetConnect(); + + setGlobalDispatcher(mockAgent); + + mockPool = mockAgent.get('https://graph.microsoft.com'); + + enableDefaultRoutes(); +} + +// export const reset = (state) => { +// return enable(state) +// } + +export const disable = () => { + throw "unimplemented" +} + +/** + * The problems with this are: + * 1) When I register a persistent route, there's no way + * to disable it + * 2) The first route to bind will always win, so custom + * overrides basically don't work. + * Bugger. + */ +export const enableDefaultRoutes = () => { + mockRoute(patterns.drives, fixtures.driveResponse) +} + +// API to enable a particular path to be mocked +export const mockRoute = (path, data, options = {}) => { + +const jsonResponse = { + headers: { + 'Content-Type': 'application/json', + }, +}; + + const { method = 'GET', headers, once = false } = options + const scope = mockPool.intercept({ + path, + method, + headers, + }).reply(200, JSON.stringify(data),jsonResponse) + + if (!once) { + scope.persist() + } +} \ No newline at end of file diff --git a/packages/msgraph/src/operations.js b/packages/msgraph/src/operations.js index ca21db58c..9d85ef856 100644 --- a/packages/msgraph/src/operations.js +++ b/packages/msgraph/src/operations.js @@ -3,17 +3,28 @@ import { operation } from '@openfn/language-common/util' import * as impl from './impl'; import { request } from './utils'; +import { enable, mockRoute } from './mock/mock'; +// TODO this may be deprecated now ? +// I don't think we need it for this adaptor, which is nice! let customHandler; +const getRequestHandler = () => { + return customHandler || request; +} -// TODO I'd quite like to exclude this from the build? -export const setRequestHandler = (fn) => { - customHandler = fn +/** Alternative, standardised, mock handler */ + +// The runtime itself will call this to flick the whole thing into mock mode +export const enableMock = (state) => { + enable(state) } -// TODO need to work out the best pattern for this -const getRequestHandler = () => { - return customHandler || request; +// Every adator uses this API to override mock mode +// The pattern is adaptor-specific +// data is whatever you want the client to return +// TODO: alias to addMockRule ? +export const mock = (pattern, data, options = {}) => { + return mockRoute(pattern, data, options) } /** @@ -45,7 +56,7 @@ export function execute(...operations) { }) .then(cleanup) .catch(error => { - cleanup(state); + cleanup(state); throw error; }); }; @@ -63,6 +74,9 @@ export function execute(...operations) { * @returns {Operation} */ export const create = operation((state, resource, data, callback) => { + // this pattern isn't so helpful for a http adaptor, the mock client + // isn't needed + // going to save it for now because I think it's still good for eg salesforce, postgres return impl.create(state, getRequestHandler(), resource, data, callback) }) @@ -93,7 +107,7 @@ export const get = operation((state, path, query, callback) => { * @example Get the default drive for a site * getDrive({ id: "openfn.sharepoint.com", owner: "sites" }) * @param specifier {Object} - A definition of the drive to retrieve - * - id {string} - The ID of the resource or owner. + * - id {string} - The ID{ defa} of the resource or owner. * - owner {string} - The type of drive owner (e.g. sites, groups). * @param {string} name - The local name of the drive used to write to state.drives, ie, state.drives[name] * @param {function} [callback = s => s] (Optional) Callback function diff --git a/packages/msgraph/test/examples.test.js b/packages/msgraph/test/examples.test.js index 55bfc2fb6..3c7300486 100644 --- a/packages/msgraph/test/examples.test.js +++ b/packages/msgraph/test/examples.test.js @@ -2,7 +2,13 @@ import { expect } from 'chai'; import execute from '@openfn/test-execute'; import Adaptor from '../src/index'; -import { fixtures } from './fixtures'; +import { fixtures } from '../src/mock/fixtures'; + +const configuration = { + // The mock could test for these things and throw appropriately if I wanted + accessToken: 'a', + apiVersion: '1', +}; // TODO move to tools const loadExample = (name) => { @@ -10,6 +16,9 @@ const loadExample = (name) => { return 'getDrive((state) => ({ id: state.id }), "default", (state) => ({ ...state, savedDrives: state.drives }))' } +// Configure the adaptor to use default mocks for these tests +Adaptor.enableMock({ configuration }); + describe('examples', () => { it('get-drive', async () => { // Load our example code @@ -18,21 +27,9 @@ describe('examples', () => { // Set up some input state const state = { id: 'xxx', - configuration: { - // The mock could test for these things and throw appropriately if I wanted - accessToken: 'a', - apiVersion: '1', - }, + configuration }; - // Set up the mock - Adaptor.setRequestHandler(async (url) => { - // assert that the URL has the correct id in it - expect(url).to.match(/xxx/) - - return fixtures.driveResponse; - }) - // Compile and run the job against this adaptor const finalState = await execute(source, Adaptor, state) diff --git a/packages/msgraph/test/mock.test.js b/packages/msgraph/test/mock.test.js new file mode 100644 index 000000000..fd1d12b63 --- /dev/null +++ b/packages/msgraph/test/mock.test.js @@ -0,0 +1,52 @@ +/** + * these tests check default and custom mock values + * they do not exercise execute or anything + */ +import { expect } from 'chai'; + +import { fixtures } from '../src/mock/fixtures'; +import { patterns } from '../src/mock/mock'; + +// not sure if this is the right import +// I guess it's fine +import Adaptor from '../src/index.js' + +const defaultState = { configuration: {resource: 'x'}, drives: {}}; + +Adaptor.enableMock(defaultState); + +// Test all the default mock behaviours +describe('default values', () => { + it('getDrive', async () => { + const state = { ...defaultState }; + + const result = await Adaptor.getDrive({ id: 'b!YXzpkoLwR06bxC8tNdg71m_' })(state) + expect(result.drives).to.eql({ default: fixtures.driveResponse }) + }) + + it('getDrive basically ignores the drive id argument', async () => { + const state = { ...defaultState }; + + const result = await Adaptor.getDrive({ id: 'abcdefg' })(state) + expect(result.drives).to.eql({ default: fixtures.driveResponse }) + }) +}) + +// TODO this doesn't work with Undici :( +// Which means we don't have sufficient control of the mock +describe.skip('custom values', () => { + it('getDrive', async () => { + const state = { ...defaultState }; + + // This should override the default mock... (forever) + // This is annoying, I think the first mock to bind is the winner in case of a conflict + // That doesn't suit me! + // A shame because this is actually a really nice pattern + Adaptor.mock(patterns.drives, { + x: 22 + }) + + const result = await Adaptor.getDrive({ id: 'b!YXzpkoLwR06bxC8tNdg71m_' })(state) + expect(result.drives).to.eql({ default: { x: 22 } }) + }) +}) \ No newline at end of file diff --git a/tutorial.md b/tutorial.md index f4fc8be4e..5150b4ce1 100644 --- a/tutorial.md +++ b/tutorial.md @@ -41,6 +41,7 @@ Here's what's not right yet: - Getting mock data feels a bit hard (although there may be an answer to that here somewhere) - The setclient function on the adaptor is a bit awkward - Can we exclude the setClient function from the final build? +* When the client is abstracted out (like in msgraph), it can beh ar to know what it is. You look at `impl.js` and you see request, you don't know what it is. So it's actually kinda hard to use. hmm. ### Operation Factories From f48a9b4086ccb599d77a9112021ce0e516fd713c Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Sun, 10 Mar 2024 18:40:02 +0300 Subject: [PATCH 12/14] figured out a more robust mocking pattern --- packages/msgraph/src/mock/mock.js | 67 ++++++++++-------------------- packages/msgraph/src/operations.js | 4 +- packages/msgraph/test/impl.test.js | 2 +- packages/msgraph/test/mock.test.js | 26 ++++++------ 4 files changed, 39 insertions(+), 60 deletions(-) diff --git a/packages/msgraph/src/mock/mock.js b/packages/msgraph/src/mock/mock.js index 1675995c4..d5b6cb4d1 100644 --- a/packages/msgraph/src/mock/mock.js +++ b/packages/msgraph/src/mock/mock.js @@ -9,27 +9,9 @@ import { fixtures } from './fixtures'; * We need to document this in a way that the playground can use */ -// first step: mock with default data each msgraph function - -// ok that's fun, it's a http client - -// So this DOES use undici. So I think I can just write interceptors, rght? - let mockAgent; let mockPool; -// TODO this was copied out of util but it should be a common helper -// actually we don't need this -// export function getBaseUrl(state) { -// const{ resource, apiVersion} = state.configuration -// // if (isValidHttpUrl(resource)) return resource; - -// const pathSuffix = apiVersion -// ? `${apiVersion}/${resource}` -// : `v1.0/${resource}`; -// return `https://graph.microsoft.com/${pathSuffix}`; -// } - // To make custom overrides easier, export some stanadard patterns export const patterns = { // TODO the regex might be a bit hard here, as we @@ -37,7 +19,14 @@ export const patterns = { drives: /\/drives\// } -export const enable = (state) => { +// name: { pattern, data, options } +const defaultRoutes = { + 'drives': { pattern: patterns.drives, data: fixtures.driveResponse } +} + +export const enable = (state, routes = {}) => { + // TODO if an agent already exists, should we destroy it? + // set the global dispacher on undici mockAgent = new MockAgent(); @@ -48,37 +37,25 @@ export const enable = (state) => { mockPool = mockAgent.get('https://graph.microsoft.com'); - enableDefaultRoutes(); -} - -// export const reset = (state) => { -// return enable(state) -// } - -export const disable = () => { - throw "unimplemented" -} + const mockRoutes = { + ...defaultRoutes, + ...routes + } -/** - * The problems with this are: - * 1) When I register a persistent route, there's no way - * to disable it - * 2) The first route to bind will always win, so custom - * overrides basically don't work. - * Bugger. - */ -export const enableDefaultRoutes = () => { - mockRoute(patterns.drives, fixtures.driveResponse) + // Set up all the mock routes + for (const name in mockRoutes) { + const { pattern, data, options } = mockRoutes[name]; + mockRoute(pattern, data, options) + } } // API to enable a particular path to be mocked export const mockRoute = (path, data, options = {}) => { - -const jsonResponse = { - headers: { - 'Content-Type': 'application/json', - }, -}; + const jsonResponse = { + headers: { + 'Content-Type': 'application/json', + }, + }; const { method = 'GET', headers, once = false } = options const scope = mockPool.intercept({ diff --git a/packages/msgraph/src/operations.js b/packages/msgraph/src/operations.js index 9d85ef856..9685fcf5a 100644 --- a/packages/msgraph/src/operations.js +++ b/packages/msgraph/src/operations.js @@ -15,8 +15,8 @@ const getRequestHandler = () => { /** Alternative, standardised, mock handler */ // The runtime itself will call this to flick the whole thing into mock mode -export const enableMock = (state) => { - enable(state) +export const enableMock = (state, routes) => { + return enable(state, routes); } // Every adator uses this API to override mock mode diff --git a/packages/msgraph/test/impl.test.js b/packages/msgraph/test/impl.test.js index 7f303a58a..66bfac699 100644 --- a/packages/msgraph/test/impl.test.js +++ b/packages/msgraph/test/impl.test.js @@ -1,6 +1,6 @@ import { expect } from 'chai'; import * as impl from '../src/impl'; -import { fixtures } from './fixtures'; +import { fixtures } from '../src/mock/fixtures'; describe('getDrive', () => { // look how simple this unit test is now diff --git a/packages/msgraph/test/mock.test.js b/packages/msgraph/test/mock.test.js index fd1d12b63..b485f4dfe 100644 --- a/packages/msgraph/test/mock.test.js +++ b/packages/msgraph/test/mock.test.js @@ -13,10 +13,14 @@ import Adaptor from '../src/index.js' const defaultState = { configuration: {resource: 'x'}, drives: {}}; -Adaptor.enableMock(defaultState); // Test all the default mock behaviours describe('default values', () => { + before(() => { + Adaptor.enableMock(defaultState); + }); + + // These can all use the default mock it('getDrive', async () => { const state = { ...defaultState }; @@ -32,19 +36,17 @@ describe('default values', () => { }) }) -// TODO this doesn't work with Undici :( -// Which means we don't have sufficient control of the mock -describe.skip('custom values', () => { +describe('custom values', () => { it('getDrive', async () => { - const state = { ...defaultState }; + Adaptor.enableMock(defaultState, { + 'drives': { + pattern: patterns.drives, + data: { x: 22 }, + options: { once: true } + } + }); - // This should override the default mock... (forever) - // This is annoying, I think the first mock to bind is the winner in case of a conflict - // That doesn't suit me! - // A shame because this is actually a really nice pattern - Adaptor.mock(patterns.drives, { - x: 22 - }) + const state = { ...defaultState }; const result = await Adaptor.getDrive({ id: 'b!YXzpkoLwR06bxC8tNdg71m_' })(state) expect(result.drives).to.eql({ default: { x: 22 } }) From c17ec33b27f504d0d135802a81d5d86885ef56ec Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Sun, 10 Mar 2024 22:52:36 +0300 Subject: [PATCH 13/14] update notes --- packages/msgraph/src/operations.js | 9 --- tutorial.md | 95 ++++++++++++++++++++++++------ 2 files changed, 76 insertions(+), 28 deletions(-) diff --git a/packages/msgraph/src/operations.js b/packages/msgraph/src/operations.js index 9685fcf5a..daf4b546c 100644 --- a/packages/msgraph/src/operations.js +++ b/packages/msgraph/src/operations.js @@ -18,15 +18,6 @@ const getRequestHandler = () => { export const enableMock = (state, routes) => { return enable(state, routes); } - -// Every adator uses this API to override mock mode -// The pattern is adaptor-specific -// data is whatever you want the client to return -// TODO: alias to addMockRule ? -export const mock = (pattern, data, options = {}) => { - return mockRoute(pattern, data, options) -} - /** * Execute a sequence of operations. * Wraps `language-common/execute` to make working with this API easier. diff --git a/tutorial.md b/tutorial.md index 5150b4ce1..b96b40ea4 100644 --- a/tutorial.md +++ b/tutorial.md @@ -9,7 +9,7 @@ Here's what I've introduced * Operation factories * Operation/implementation split * Real runtime/job tests -* A pattern for mocking (probably better than demanding docker containers?) +* A pattern for mocking which SHOULD enable a live playground (!!) and maybe easier adaptor creation ### Motivations @@ -20,14 +20,13 @@ Here are the problems I'm trying to solve * Clarity over what an operation is, and why it matters * Encouraging good documentation _in the right place_ * I see lots of problems of devs documenting the wrong functions, wasting time and energy +* (new) If we had a live playground on docs.openfn.org, how would we handle adaptors? ### Examples I've started implementing this stuff in `msgraph` (and common) to see how it all comes together -I should really push this further and will try and spend some time on it before I fly. - -Maybe `salesforce`, `dhis2` or `http` would be better examples? +I urgently need to start on `salesforce` to explore client-based mocking ### Issues @@ -37,15 +36,12 @@ Here's what's not right yet: * Expanding references. I'd really like to standardise and simplify this further - jsdoc path vs dataValue() vs open function - I am sure that you can just make expand references read '$a.b.c' as a jsonpath -* I'm not totally sold on some parts of the mocking pattern - - Getting mock data feels a bit hard (although there may be an answer to that here somewhere) - - The setclient function on the adaptor is a bit awkward - - Can we exclude the setClient function from the final build? -* When the client is abstracted out (like in msgraph), it can beh ar to know what it is. You look at `impl.js` and you see request, you don't know what it is. So it's actually kinda hard to use. hmm. +* When the client is abstracted out (like in msgraph), it can be hard to know what it is. You look at `impl.js` and you see request, you don't know what it is. So it's actually kinda hard to use. hmm. +* Maybe the impl pattern makes less sense with the new mocking pattern? Depends how salesforce looks ### Operation Factories -Here's an operation factory +Here's an operation factorybeh ar ```js export const get = operation((state, url, auth, callback) => { // code goes here @@ -73,7 +69,7 @@ export const create = operation((state, resource, data, callback) => { Note the extra `request` argument! -I can now unit test the implementation: +I can now unit test the implementation really cleanly: ```js describe('getDrive', () => { @@ -97,13 +93,77 @@ describe('getDrive', () => { The ability to mock the implementation like this also enables real runtime testing +### Mocking as first-class adaptor code + +What if each adaptor was able to run in a mock mode. In this mode it will mock out the client entirely and return mock data. A default data suite is included, but can be overridden. + +This works pretty great with undici, I think it should work nicely with client based adaptors too. + +First, the adaptor to expose an `enableMock` function, which is called with state (for config) and optionally some mock data. If mockdata is not provided, defaults will be used. + +Later, the runtime could call `enableMock` in certain circumstances (like a live playground). + +```js +// mock.js (exported by index.js) +export const enableMock = (state, routes = {}) => { + // setup undici to mock at the http level + mockAgent = new MockAgent(); + mockAgent.disableNetConnect(); + + mockPool = mockAgent.get('https://graph.microsoft.com'); + + const mockRoutes = { + ...defaultRoutes, + ...routes + } + + setupMockRoutes() +} + +// name: { pattern, data, options } +const defaultRoutes = { + 'drives': { pattern: /\/drives\//, data: fixtures.driveResponse } +} +``` + +Mocks work based on routing. Obvious with HTTP but I think we can do it with clients too (patterns confirm to` client.`) + +Each adaptor is responsible for implementing its own mock in whatever way makes sense. We will provide strong patterns and utilitie. + +Now, if you want to write unit tests against this mock, you can do so +```js +it('with default mock', async () => { + const state = { ... }; + Adaptor.enableMock(state); + + const result = await Adaptor.getDrive({ id: 'abc' })(state) + expect(result.drives).to.eql({ default: fixtures.driveResponse }) +}) + +it('with custom mock', async () => { + const state = { ... }; + + Adaptor.enableMock(defaultState, { + 'drives': { + pattern: patterns.drives, + data: { x: 22 }, + options: { once: true } + } + }); + + const result = await Adaptor.getDrive({ id: 'abc' })(state) + expect(result.drives).to.eql({ default: fixtures.driveResponse }) +}) +``` + +Important detail: the mock should be considered frozen when activated. If you want to change mock data, call `enable` again + ### Real runtime tests I really hate the existing test syntax we use right now. What I actually want to do is write a job and pass it to the actual run time so execution. So I've implemented this! - First, we create an example job in a source file ```js @@ -124,12 +184,15 @@ getDrive(( In a unit test, we can: * Load this example source * Load the adaptor module -* Set a mock client/request object in the adaptor +* Use mock data from above (works great) * Pass the source, input state and adaptor into the actual runtime and compiler That gives us a test that looks like this: ```js describe('examples', () => { + // setup our mock with whatever data we want + Adaptor.enableMock({ configuration }); + it('get-drive', async () => { // Load our example code const source = loadExample('get-drive') @@ -139,12 +202,6 @@ describe('examples', () => { id: 'xxx', }; - // Set up the mock - Adaptor.setRequestHandler(async (url) => { - // Return some mock data (perhaps as a pre-saved fixture, or we define it in-line) - return { ... } - }) - // Compile and run the job against this adaptor const finalState = await execute(source, Adaptor, state) From b510eba62f7f8e5d65f99ebd7b569fd70327e681 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 13 Mar 2024 08:56:16 +0300 Subject: [PATCH 14/14] a wee tidy up --- packages/msgraph/src/operations.js | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/packages/msgraph/src/operations.js b/packages/msgraph/src/operations.js index daf4b546c..8794e3812 100644 --- a/packages/msgraph/src/operations.js +++ b/packages/msgraph/src/operations.js @@ -3,16 +3,7 @@ import { operation } from '@openfn/language-common/util' import * as impl from './impl'; import { request } from './utils'; -import { enable, mockRoute } from './mock/mock'; - -// TODO this may be deprecated now ? -// I don't think we need it for this adaptor, which is nice! -let customHandler; -const getRequestHandler = () => { - return customHandler || request; -} - -/** Alternative, standardised, mock handler */ +import { enable } from './mock/mock'; // The runtime itself will call this to flick the whole thing into mock mode export const enableMock = (state, routes) => { @@ -65,10 +56,7 @@ export function execute(...operations) { * @returns {Operation} */ export const create = operation((state, resource, data, callback) => { - // this pattern isn't so helpful for a http adaptor, the mock client - // isn't needed - // going to save it for now because I think it's still good for eg salesforce, postgres - return impl.create(state, getRequestHandler(), resource, data, callback) + return impl.create(state, request, resource, data, callback) }) /** @@ -83,7 +71,7 @@ export const create = operation((state, resource, data, callback) => { * @returns {Operation} */ export const get = operation((state, path, query, callback) => { - return impl.get(state, getRequestHandler(), path, query, callback) + return impl.get(state, request, path, query, callback) }) @@ -105,7 +93,7 @@ export const get = operation((state, path, query, callback) => { * @return {Operation} */ export const getDrive = operation((state, specifier, name, callback) => { - return impl.getDrive(state, getRequestHandler(), specifier, name, callback) + return impl.getDrive(state, request, specifier, name, callback) }) /** @@ -121,7 +109,7 @@ export const getDrive = operation((state, specifier, name, callback) => { * @return {Operation} */ export const getFolder = operation((state, pathOrId, options, callback) => { - return impl.getFolder(state, getRequestHandler(), pathOrId, options, callback) + return impl.getFolder(state, request, pathOrId, options, callback) }); @@ -138,7 +126,7 @@ export const getFolder = operation((state, pathOrId, options, callback) => { * @return {Operation} */ export const getFile = operation((state, pathOrId, options, callback) => { - return impl.getFile(state, getRequestHandler(), pathOrId, options, callback) + return impl.getFile(state, request, pathOrId, options, callback) }); /** @@ -176,7 +164,7 @@ export const getFile = operation((state, pathOrId, options, callback) => { * @returns {Operation} */ export const uploadFile = operation((state, resource, data, callback) => { - return impl.uploadFile(state, getRequestHandler(), pathOrId, options, callback) + return impl.uploadFile(state, request, pathOrId, options, callback) }); export { request, sheetToBuffer } from './utils';