diff --git a/packages/core/integration-test/integration-test.js b/packages/core/integration-test/integration-test.js index f2e908b95..7c73a277e 100644 --- a/packages/core/integration-test/integration-test.js +++ b/packages/core/integration-test/integration-test.js @@ -521,7 +521,7 @@ const doTest = (runner) => { it('should log requests', () => { const event = { command: 'execute', - method: 'resources.requestfunc.list.operation.perform', + method: 'resources.requestsugar.list.operation.perform', logExtra: { app_cli_id: 666, }, diff --git a/packages/core/src/app-middlewares/after/checks.js b/packages/core/src/app-middlewares/after/checks.js index eb1236500..33b3a2fd3 100644 --- a/packages/core/src/app-middlewares/after/checks.js +++ b/packages/core/src/app-middlewares/after/checks.js @@ -32,7 +32,7 @@ const checkOutput = (output) => { }) .map((check) => { return check - .run(event.method, output.results, compiledApp) + .run(event.method, output.results, compiledApp, event.bundle) .map((err) => ({ name: check.name, error: err })); }); const checkResults = _.flatten(rawResults); diff --git a/packages/core/src/checks/index.js b/packages/core/src/checks/index.js index 981090bac..e477efc01 100644 --- a/packages/core/src/checks/index.js +++ b/packages/core/src/checks/index.js @@ -9,4 +9,5 @@ module.exports = { triggerHasId: require('./trigger-has-id'), firehoseSubscriptionIsArray: require('./firehose_is_array'), firehoseSubscriptionKeyIsString: require('./firehose_is_string'), + performBulkReturnType: require('./perform-bulk-return-type'), }; diff --git a/packages/core/src/checks/is-create.js b/packages/core/src/checks/is-create.js index c14f7596e..dde2a49d9 100644 --- a/packages/core/src/checks/is-create.js +++ b/packages/core/src/checks/is-create.js @@ -1,6 +1,10 @@ module.exports = (method) => { + // `method` will never start with "resources." in production. + // Seems only for testing. return ( - (method.startsWith('creates.') && method.endsWith('.operation.perform')) || + (method.startsWith('creates.') && + (method.endsWith('.operation.perform') || + method.endsWith('.operation.performBulk'))) || (method.startsWith('resources.') && method.endsWith('.create.operation.perform')) ); diff --git a/packages/core/src/checks/is-trigger.js b/packages/core/src/checks/is-trigger.js index 955a99e68..8f78dbcff 100644 --- a/packages/core/src/checks/is-trigger.js +++ b/packages/core/src/checks/is-trigger.js @@ -1,6 +1,7 @@ module.exports = (method) => { return ( - // `method` will never start with "resources.". Seems like legacy code. + // `method` will never start with "resources." in production. + // Seems only for testing. (method.startsWith('triggers.') && method.endsWith('.operation.perform')) || (method.startsWith('resources.') && method.endsWith('.list.operation.perform')) diff --git a/packages/core/src/checks/perform-bulk-return-type.js b/packages/core/src/checks/perform-bulk-return-type.js new file mode 100644 index 000000000..0f15adc0c --- /dev/null +++ b/packages/core/src/checks/perform-bulk-return-type.js @@ -0,0 +1,65 @@ +const _ = require('lodash'); + +const performBulkEchoesIds = { + name: 'performBulkReturnType', + + shouldRun: (method, bundle) => { + return ( + Array.isArray(bundle.bulk) && + method.endsWith('.operation.performBulk') && + method.startsWith('creates.') + ); + }, + + run: (method, results, compiledApp, bundle) => { + if (!_.isPlainObject(results)) { + // create-is-object should have caught this + return []; + } + + const inputIds = bundle.bulk + .map((b) => { + return b && b.meta ? b.meta.id : null; + }) + .filter((id) => id); + + const outputIds = Object.keys(results); + const missingIds = inputIds.filter((id) => !outputIds.includes(id)); + + if (missingIds.length > 0) { + const LIMIT = 3; + let missingIdsStr = missingIds.slice(0, LIMIT).join(', '); + const remainingCount = missingIds.length - LIMIT; + if (remainingCount > 0) { + // Don't want to flood the user with too many IDs + missingIdsStr += `, and ${remainingCount} more`; + } + return [`Result object is missing these IDs as keys: ${missingIdsStr}`]; + } + + const errors = []; + for (const id of inputIds) { + const item = results[id]; + + if (!_.isPlainObject(item)) { + errors.push(`Result object member with ID '${id}' must be an object`); + } else if ( + !_.isPlainObject(item.outputData) && + typeof item.error !== 'string' + ) { + errors.push( + `Result object member with ID '${id}' must have 'outputData' object or 'error' string` + ); + } + + if (errors.length >= 4) { + // No need to flood the user with too many errors + break; + } + } + + return errors; + }, +}; + +module.exports = performBulkEchoesIds; diff --git a/packages/core/test/checks.js b/packages/core/test/checks.js index ca0ee5e1a..d2ddceb86 100644 --- a/packages/core/test/checks.js +++ b/packages/core/test/checks.js @@ -247,6 +247,78 @@ describe('checks', () => { isFirehoseWebhook(firehoseMethod).should.be.true(); isFirehoseWebhook('triggers.blah.operation.perform').should.be.false(); // the firehose webhook check is at app level, not trigger }); + + it('should check performBulk missing IDs', () => { + const results = { one: {}, two: {} }; + const bundle = { + bulk: [ + { meta: { id: 'one' } }, + { meta: { id: 'two' } }, + { meta: { id: 'three' } }, + { meta: { id: 'four' } }, + { meta: { id: 'five' } }, + { meta: { id: 'six' } }, + { meta: { id: 'seven' } }, + { meta: { id: 'eight' } }, + { meta: { id: 'nine' } }, + ], + }; + const errors = checks.performBulkReturnType.run( + 'creates.blah.operation.performBulk', + results, + {}, + bundle + ); + errors.length.should.eql(1); + errors[0].should.match( + /missing these IDs as keys: three, four, five, and 4 more/ + ); + }); + + it('should check performBulk object shape', () => { + const results = { + one: 'not an object', + two: { error: 123 }, + three: { outputData: {} }, + four: { error: 'test' }, + five: { + outputData: 'this one should pass because it is not in the input', + }, + }; + const bundle = { + bulk: [ + { meta: { id: 'one' } }, + { meta: { id: 'two' } }, + { meta: { id: 'three' } }, + { meta: { id: 'four' } }, + ], + }; + const errors = checks.performBulkReturnType.run( + 'creates.blah.operation.performBulk', + results, + {}, + bundle + ); + errors.length.should.eql(2); + errors[0].should.match(/member with ID 'one' must be an object/); + errors[1].should.match( + /member with ID 'two' must have 'outputData' object or 'error' string/ + ); + }); + + it('should pass performBulk check', () => { + const results = { one: { outputData: {} }, two: { outputData: {} } }; + const bundle = { + bulk: [{ meta: { id: 'one' } }, { meta: { id: 'two' } }], + }; + const errors = checks.performBulkReturnType.run( + 'creates.blah.operation.performBulk', + results, + {}, + bundle + ); + errors.should.be.empty(); + }); }); describe('checkOutput', () => { diff --git a/packages/core/test/create-app.js b/packages/core/test/create-app.js index c1db93d70..8631e0aa8 100644 --- a/packages/core/test/create-app.js +++ b/packages/core/test/create-app.js @@ -17,9 +17,10 @@ describe('create-app', () => { const app = createApp(appDefinition); - const createTestInput = (method) => { + const createTestInput = (method, bundle) => { const event = { - bundle: {}, + command: 'execute', + bundle: { ...bundle }, method, callback_url: 'calback_url', }; @@ -108,13 +109,14 @@ describe('create-app', () => { const input = createTestInput('resources.contact.list.operation.perform'); const output = await app(input); - should.exist(output.results.headers); + const result = output.results[0]; + should.exist(result.headers); // verify that custom http before middleware was applied - output.results.headers['X-Hashy'].should.deepEqual([ + result.headers['X-Hashy'].should.deepEqual([ '1a3ba5251cb33ee7ade01af6a7b960b8', ]); - output.results.headers['X-Author'].should.deepEqual(['One Cool Dev']); + result.headers['X-Author'].should.deepEqual(['One Cool Dev']); }); it('should fail on a live request call', async () => { @@ -155,28 +157,18 @@ describe('create-app', () => { throw new Error('expected an error'); }); - it('should make call via z.request', async () => { - const input = createTestInput('triggers.requestfuncList.operation.perform'); - - const output = await app(input); - - output.results[0].headers['X-Hashy'].should.deepEqual([ - '1a3ba5251cb33ee7ade01af6a7b960b8', - ]); - output.results[0].headers['X-Author'].should.deepEqual(['One Cool Dev']); - }); - it('should make call via z.request with sugar url param', async () => { const input = createTestInput( 'triggers.requestsugarList.operation.perform' ); const output = await app(input); + const result = output.results[0]; - output.results.headers['X-Hashy'].should.deepEqual([ + result.headers['X-Hashy'].should.deepEqual([ '1a3ba5251cb33ee7ade01af6a7b960b8', ]); - output.results.headers['X-Author'].should.deepEqual(['One Cool Dev']); + result.headers['X-Author'].should.deepEqual(['One Cool Dev']); }); it('should fail on a sync function', (done) => { @@ -256,11 +248,13 @@ describe('create-app', () => { const input = createTestInput('triggers.contactList.operation.perform'); const output = await app(input); - output.results.url.should.eql(`${HTTPBIN_URL}/get`); - output.results.headers['X-Hashy'].should.deepEqual([ + const result = output.results[0]; + + result.url.should.eql(`${HTTPBIN_URL}/get`); + result.headers['X-Hashy'].should.deepEqual([ '1a3ba5251cb33ee7ade01af6a7b960b8', ]); - output.results.headers['X-Author'].should.deepEqual(['One Cool Dev']); + result.headers['X-Author'].should.deepEqual(['One Cool Dev']); }); it('should return a rendered URL for OAuth2 authorizeURL', (done) => { @@ -319,6 +313,46 @@ describe('create-app', () => { .catch(done); }); + describe('output checks', () => { + it('should check performBulk output', async () => { + const definition = dataTools.jsonCopy(appDefinition); + definition.resources.row = { + key: 'row', + noun: 'Row', + create: { + display: { + label: 'Insert Row', + }, + operation: { + performBulk: (z, bundle) => { + const firstId = bundle.bulk[0].meta.id; + return { [firstId]: {} }; + }, + }, + }, + }; + const app = createApp(definition); + const bundle = { + bulk: [ + { meta: { id: 'ffee-0000' } }, + { meta: { id: 'ffee-0001' } }, + { meta: { id: 'ffee-0002' } }, + { meta: { id: 'ffee-0003' } }, + { meta: { id: 'ffee-0004' } }, + { meta: { id: 'ffee-0005' } }, + ], + }; + const input = createTestInput( + 'creates.rowCreate.operation.performBulk', + bundle + ); + const err = await app(input).should.be.rejected(); + err.name.should.eql('CheckError'); + err.message.should.match(/missing these IDs as keys/); + err.message.should.match(/ffee-0001, ffee-0002, ffee-0003, and 2 more/); + }); + }); + describe('HTTP after middleware for auth refresh', () => { // general purpose response tester const testResponse = async (appDef, useShorthand, errorVerifier) => { @@ -499,29 +533,37 @@ describe('create-app', () => { }); describe('hydration', () => { - it('should hydrate method', (done) => { + it('should hydrate method', async () => { const input = createTestInput( 'resources.honkerdonker.list.operation.perform' ); - - app(input) - .then((output) => { - output.results.should.eql([ + const output = await app(input); + output.results.should.eql([ + { + id: 1, + $HOIST$: 'hydrate|||{"type":"method","method":"resources.honkerdonker.get.operation.perform","bundle":{"honkerId":1}}|||hydrate', + }, + { + id: 2, + $HOIST$: 'hydrate|||{"type":"method","method":"resources.honkerdonker.get.operation.perform","bundle":{"honkerId":2}}|||hydrate', + }, + { + id: 3, + $HOIST$: 'hydrate|||{"type":"method","method":"resources.honkerdonker.get.operation.perform","bundle":{"honkerId":3}}|||hydrate', - ]); - done(); - }) - .catch(done); + }, + ]); }); }); + describe('calling a callback method', () => { let results; before(() => app( createTestInput( - 'resources.executeCallbackRequest.list.operation.perform' + 'resources.executeCallbackRequest.create.operation.perform' ) ).then((output) => { results = output; @@ -530,8 +572,12 @@ describe('create-app', () => { it('returns a CALLBACK envelope', () => results.status.should.eql('CALLBACK')); + it('returns the methods values', () => - results.results.should.eql({ callbackUrl: 'calback_url' })); + results.results.should.eql({ + id: 'dontcare', + callbackUrl: 'calback_url', + })); }); describe('using require', () => { @@ -579,13 +625,13 @@ describe('create-app', () => { it('should import and use the crypto module from node', async () => { const definition = createDefinition(` const crypto = z.require('crypto'); - return crypto.createHash('md5').update('abc').digest('hex'); + return [{id: crypto.createHash('md5').update('abc').digest('hex')}]; `); const input = createTestInput('triggers.testRequire.operation.perform'); const appPass = createApp(definition); const { results } = await appPass(input); - results.should.eql('900150983cd24fb0d6963f7d28e17f72'); + results[0].id.should.eql('900150983cd24fb0d6963f7d28e17f72'); }); it('should handle require errors', async () => { diff --git a/packages/core/test/tools/resolve-method-path.js b/packages/core/test/tools/resolve-method-path.js index 4bd5a4e47..8ee200291 100644 --- a/packages/core/test/tools/resolve-method-path.js +++ b/packages/core/test/tools/resolve-method-path.js @@ -25,8 +25,8 @@ describe('resolve-method-path', () => { it('should resolve a request method object with a url', () => { resolveMethodPath( app, - app.resources.contact.list.operation.perform - ).should.eql('resources.contact.list.operation.perform'); + app.resources.contacterror.list.operation.perform + ).should.eql('resources.contacterror.list.operation.perform'); }); it('should resolve a method in a module', () => { diff --git a/packages/core/test/userapp/index.js b/packages/core/test/userapp/index.js index 7f4dff217..20acf67d3 100644 --- a/packages/core/test/userapp/index.js +++ b/packages/core/test/userapp/index.js @@ -35,8 +35,11 @@ const Contact = { description: 'Trigger on new contacts.', }, operation: { - perform: { - url: '{{process.env.BASE_URL}}/get', + perform: async (z, bundle) => { + const response = await z.request({ + url: `${process.env.BASE_URL}/get`, + }); + return [{ id: 'dontcare', ...response.data }]; }, inputFields: [ { @@ -121,28 +124,6 @@ const LoggingFunc = { }, }; -const RequestFunc = { - key: 'requestfunc', - noun: 'requestfunc', - list: { - display: { - label: 'New Request Func', - description: 'Makes an http request via z.request.', - }, - operation: { - perform: (z /* , bundle */) => { - return z - .request({ url: '{{process.env.BASE_URL}}/get' }) - .then((resp) => { - const result = resp.data; - result.id = 123; - return [result]; - }); - }, - }, - }, -}; - const RequestSugar = { key: 'requestsugar', noun: 'requestsugar', @@ -154,7 +135,7 @@ const RequestSugar = { operation: { perform: (z /* , bundle */) => { return z.request(`${HTTPBIN_URL}/get`).then((resp) => { - return resp.data; + return [{ id: 'dontcare', ...resp.data }]; }); }, }, @@ -386,9 +367,7 @@ const CachedCustomInputFields = { description: 'Get/Set custom input fields in zcache', }, operation: { - inputFields: [ - helpers.getCustomFields, - ], + inputFields: [helpers.getCustomFields], perform: () => {}, }, }, @@ -403,7 +382,11 @@ const HonkerDonker = { description: 'This will be dehydrated by list', }, operation: { - perform: (z, bundle) => `honker donker number ${bundle.honkerId}`, + perform: (z, bundle) => { + return { + message: `honker donker number ${bundle.honkerId}`, + }; + }, }, }, list: { @@ -415,9 +398,12 @@ const HonkerDonker = { perform: (z, bundle) => { const honkerIds = [1, 2, 3]; return honkerIds.map((id) => { - return z.dehydrate(HonkerDonker.get.operation.perform, { - honkerId: id, - }); + return { + id, + $HOIST$: z.dehydrate(HonkerDonker.get.operation.perform, { + honkerId: id, + }), + }; }); }, }, @@ -509,7 +495,7 @@ const EnvironmentVariable = { const ExecuteCallbackRequest = { key: 'executeCallbackRequest', noun: 'Callback', - list: { + create: { display: { label: 'Callback Usage in a perform', description: 'Used for one-offs in the tests.', @@ -518,7 +504,7 @@ const ExecuteCallbackRequest = { perform: (z) => { // we need to access the callback url const callbackUrl = z.generateCallbackUrl(); - return { callbackUrl }; + return { id: 'dontcare', callbackUrl }; }, performResume: (z, bundle) => { return Object.assign({}, bundle.outputData, bundle.cleanedRequest); @@ -586,7 +572,6 @@ const App = { [ContactError.key]: ContactError, [ContactSource.key]: ContactSource, [LoggingFunc.key]: LoggingFunc, - [RequestFunc.key]: RequestFunc, [RequestSugar.key]: RequestSugar, [WorkingFunc.key]: WorkingFunc, [WorkingFuncAsync.key]: WorkingFuncAsync,