From 3c40c1de64b13b93de7a5da16def25ef8001c8f3 Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Tue, 17 Apr 2018 13:17:19 +0200 Subject: [PATCH] Allow optional params (#68) * Allow optional params * Don't double-up on method signature * Cleanup formatters/util --- .../api-format/src/{util.js => format.js} | 23 ++---- packages/api-format/src/format.spec.js | 78 ++++++++++++++++++ packages/api-format/src/input.js | 4 +- packages/api-format/src/output.js | 4 +- packages/api-format/src/util.spec.js | 82 ------------------- packages/api-jsonrpc/src/state/getStorage.js | 1 + packages/api-jsonrpc/src/types.js | 1 + packages/api/src/create/methodSend.js | 2 +- packages/api/src/create/methodSend.spec.js | 2 +- packages/api/src/create/methodSubscribe.js | 2 +- packages/api/src/create/params.js | 9 +- packages/api/src/create/params.spec.js | 38 +++++++++ 12 files changed, 140 insertions(+), 106 deletions(-) rename packages/api-format/src/{util.js => format.js} (72%) create mode 100644 packages/api-format/src/format.spec.js delete mode 100644 packages/api-format/src/util.spec.js create mode 100644 packages/api/src/create/params.spec.js diff --git a/packages/api-format/src/util.js b/packages/api-format/src/format.js similarity index 72% rename from packages/api-format/src/util.js rename to packages/api-format/src/format.js index 9ab8e64e5b..eccefd26bf 100644 --- a/packages/api-format/src/util.js +++ b/packages/api-format/src/format.js @@ -44,22 +44,15 @@ function formatArrayType (formatters: FormattersFunctionMap, type: FormattersFun }); } -function format (formatters: FormattersFunctionMap, type: FormattersFunctionType, value: mixed | Array): mixed { - if (arrayTypeRegex.test(type)) { - // $FlowFixMe array type - return formatArrayType(formatters, type, value); - } +module.exports = function format (formatters: FormattersFunctionMap, types: Array, values: Array): Array { + return values.map((value, index): mixed => { + const type = types[index]; - return formatSingleType(formatters, type, value); -} + if (arrayTypeRegex.test(type)) { + // $FlowFixMe array type + return formatArrayType(formatters, type, value); + } -function formatArray (formatters: FormattersFunctionMap, types: Array, values: Array): Array { - return types.map((type, index): mixed => { - return format(formatters, type, values[index]); + return formatSingleType(formatters, type, value); }); -} - -module.exports = { - format, - formatArray }; diff --git a/packages/api-format/src/format.spec.js b/packages/api-format/src/format.spec.js new file mode 100644 index 0000000000..c16317833e --- /dev/null +++ b/packages/api-format/src/format.spec.js @@ -0,0 +1,78 @@ +// Copyright 2017-2018 Jaco Greeff +// This software may be modified and distributed under the terms +// of the ISC license. See the LICENSE file for details. + +const format = require('./format'); +const echo = require('./echo'); + +describe('format', () => { + let formatters; + let echoSpy; + let warnSpy; + + beforeEach(() => { + echoSpy = jest.fn(echo); + warnSpy = jest.spyOn(console, 'warn'); + + formatters = { + 'Address': echoSpy, + 'Address[]': echoSpy, + 'Exception': () => { + throw new Error('something went wrong'); + } + }; + }); + + afterEach(() => { + echoSpy.mockRestore(); + warnSpy.mockRestore(); + }); + + it('formats unknown types with a fallback', () => { + expect( + format(formatters, ['Unknown'], ['test']) + ).toEqual(['test']); + }); + + it('logs a warning with unknown types', () => { + format(formatters, ['Unknown'], ['test']); + expect(warnSpy).toHaveBeenCalledWith( + expect.anything(), expect.anything(), "Unable to find default formatter for 'Unknown', falling back to echo" + ); + }); + + it('wraps exceptions with the type', () => { + expect( + () => format(formatters, ['Exception'], ['test']) + ).toThrow(/Error formatting 'test' as 'Exception'/); + }); + + describe('primitives', () => { + it('formats known types using the supplied formatter', () => { + format(formatters, ['Address'], ['0xaddress']); + + expect(echoSpy).toHaveBeenCalledWith('0xaddress'); + }); + }); + + describe('primitive[]', () => { + it('formats using the primitive type', () => { + format(formatters, ['Address[]'], [['0x123', '0x234']]); + + expect(echoSpy).toHaveBeenCalledWith('0x123'); + expect(echoSpy).toHaveBeenCalledWith('0x234'); + }); + + it('returns formatted values as arrays', () => { + expect( + format(formatters, ['Address[]'], [['0x123', '0x234']]) + ).toEqual([['0x123', '0x234']]); + }); + }); + + it('formats each value in an array', () => { + expect( + format(formatters, ['Unknown', 'Unknown'], ['test', 'blah']) + ).toEqual(['test', 'blah']); + }); +}); diff --git a/packages/api-format/src/input.js b/packages/api-format/src/input.js index 2b1561dee0..a1f4fdee7f 100644 --- a/packages/api-format/src/input.js +++ b/packages/api-format/src/input.js @@ -8,7 +8,7 @@ import type { InterfaceInputType } from '@polkadot/api-jsonrpc/types'; const bytesEncode = require('@polkadot/primitives-json/bytes/encode'); const hashEncode = require('@polkadot/primitives-json/hash/encode'); -const util = require('./util'); +const format = require('./format'); const formatters = { 'Bytes': bytesEncode, @@ -19,5 +19,5 @@ const formatters = { module.exports = function formatInputs (inputs: Array, values: Array): Array { const types = inputs.map(({ type }) => type); - return util.formatArray(formatters, types, values); + return format(formatters, types, values); }; diff --git a/packages/api-format/src/output.js b/packages/api-format/src/output.js index 7193a54527..35b443fa5e 100644 --- a/packages/api-format/src/output.js +++ b/packages/api-format/src/output.js @@ -11,7 +11,7 @@ const headerDecode = require('@polkadot/primitives-json/header/decode'); const isNull = require('@polkadot/util/is/null'); const isUndefined = require('@polkadot/util/is/undefined'); -const util = require('./util'); +const format = require('./format'); const formatters = { 'BlockNumber': bnDecode, @@ -25,5 +25,5 @@ module.exports = function formatOutput (output: InterfaceOutputType, value?: mix return value; } - return util.format(formatters, output.type, value); + return format(formatters, [output.type], [value])[0]; }; diff --git a/packages/api-format/src/util.spec.js b/packages/api-format/src/util.spec.js deleted file mode 100644 index 4abbfe8940..0000000000 --- a/packages/api-format/src/util.spec.js +++ /dev/null @@ -1,82 +0,0 @@ -// Copyright 2017-2018 Jaco Greeff -// This software may be modified and distributed under the terms -// of the ISC license. See the LICENSE file for details. - -const { format, formatArray } = require('./util'); -const echo = require('./echo'); - -describe('util', () => { - let formatters; - let echoSpy; - let warnSpy; - - beforeEach(() => { - echoSpy = jest.fn(echo); - warnSpy = jest.spyOn(console, 'warn'); - - formatters = { - 'Address': echoSpy, - 'Address[]': echoSpy, - 'Exception': () => { - throw new Error('something went wrong'); - } - }; - }); - - afterEach(() => { - echoSpy.mockRestore(); - warnSpy.mockRestore(); - }); - - describe('format', () => { - it('formats unknown types with a fallback', () => { - expect( - format(formatters, 'Unknown', 'test') - ).toEqual('test'); - }); - - it('logs a warning with unknown types', () => { - format(formatters, 'Unknown', 'test'); - expect(warnSpy).toHaveBeenCalledWith( - expect.anything(), expect.anything(), "Unable to find default formatter for 'Unknown', falling back to echo" - ); - }); - - it('wraps exceptions with the type', () => { - expect( - () => format(formatters, 'Exception', 'test') - ).toThrow(/Error formatting 'test' as 'Exception'/); - }); - - describe('primitives', () => { - it('formats known types using the supplied formatter', () => { - format(formatters, 'Address', '0xaddress'); - - expect(echoSpy).toHaveBeenCalledWith('0xaddress'); - }); - }); - - describe('primitive[]', () => { - it('formats using the primitive type', () => { - format(formatters, 'Address[]', ['0x123', '0x234']); - - expect(echoSpy).toHaveBeenCalledWith('0x123'); - expect(echoSpy).toHaveBeenCalledWith('0x234'); - }); - - it('returns formatted values as arrays', () => { - expect( - format(formatters, 'Address[]', ['0x123', '0x234']) - ).toEqual(['0x123', '0x234']); - }); - }); - }); - - describe('formatArray', () => { - it('formats each value in an array', () => { - expect( - formatArray(formatters, ['Unknown', 'Unknown'], ['test', 'blah']) - ).toEqual(['test', 'blah']); - }); - }); -}); diff --git a/packages/api-jsonrpc/src/state/getStorage.js b/packages/api-jsonrpc/src/state/getStorage.js index 3fcebe81ab..cb5f2bf47d 100644 --- a/packages/api-jsonrpc/src/state/getStorage.js +++ b/packages/api-jsonrpc/src/state/getStorage.js @@ -19,6 +19,7 @@ module.exports = ({ type: 'H256' }, { + isOptional: true, name: 'blockHash', type: 'HeaderHash' } diff --git a/packages/api-jsonrpc/src/types.js b/packages/api-jsonrpc/src/types.js index 6493a5f201..ae057f7903 100644 --- a/packages/api-jsonrpc/src/types.js +++ b/packages/api-jsonrpc/src/types.js @@ -10,6 +10,7 @@ export type FormatInputType = 'Bytes' | 'H256' | 'HeaderHash' | 'String'; export type FormatOutputType = 'BlockNumber' | 'Bytes' | 'Header' | 'HeaderHash' | 'U64'; export type InterfaceInputType = { + isOptional?: boolean, name: string, type: FormatInputType }; diff --git a/packages/api/src/create/methodSend.js b/packages/api/src/create/methodSend.js index 0da74ca994..6f3210b455 100644 --- a/packages/api/src/create/methodSend.js +++ b/packages/api/src/create/methodSend.js @@ -17,7 +17,7 @@ module.exports = function createMethodSend (provider: ProviderInterface, rpcName const call = async (..._params: Array): Promise => { // TODO: Deprecated warning try { - const params = createParams(rpcName, _params, inputs); + const params = createParams(_params, inputs); const result = await provider.send(rpcName, params); return formatOutput(output, result); diff --git a/packages/api/src/create/methodSend.spec.js b/packages/api/src/create/methodSend.spec.js index 426361a310..eb8e6e3934 100644 --- a/packages/api/src/create/methodSend.spec.js +++ b/packages/api/src/create/methodSend.spec.js @@ -41,7 +41,7 @@ describe('methodCall', () => { const method = createMethod(provider, 'test_bleh', 'bleh', methods.bleh); return method(1).catch((error) => { - expect(error.message).toMatch(/0 params expected, found 1 instead/); + expect(error.message).toMatch(/no params expected, found 1 instead/); }); }); diff --git a/packages/api/src/create/methodSubscribe.js b/packages/api/src/create/methodSubscribe.js index 91a1e53bbb..6835d9df98 100644 --- a/packages/api/src/create/methodSubscribe.js +++ b/packages/api/src/create/methodSubscribe.js @@ -25,7 +25,7 @@ module.exports = function methodSubscribe (provider: ProviderInterface, rpcName: assert(isFunction(cb), `Expected callback in last position of params`); - const params = createParams(rpcName, _params, inputs); + const params = createParams(_params, inputs); const update = (error: ?Error, result?: mixed) => { cb(error, formatOutput(output, result)); }; diff --git a/packages/api/src/create/params.js b/packages/api/src/create/params.js index 7ad1869e5d..4770c265a3 100644 --- a/packages/api/src/create/params.js +++ b/packages/api/src/create/params.js @@ -8,8 +8,13 @@ import type { InterfaceInputType } from '@polkadot/api-jsonrpc/types'; const formatInputs = require('@polkadot/api-format/input'); const assert = require('@polkadot/util/assert'); -module.exports = function createParams (rpcName: string, params: Array, inputs: Array): Array { - assert(inputs.length === params.length, `${rpcName}: ${inputs.length} params expected, found ${params.length} instead`); +module.exports = function createParams (params: Array, inputs: Array): Array { + const required = inputs.filter(({ isOptional }) => !isOptional); + const optionalText = inputs.length + ? ` (${(inputs.length - required.length) || 'none'} optional)` + : ''; + + assert(params.length >= required.length && params.length <= inputs.length, `${inputs.length || 'no'} params expected${optionalText}, found ${params.length} instead`); return formatInputs(inputs, params); }; diff --git a/packages/api/src/create/params.spec.js b/packages/api/src/create/params.spec.js new file mode 100644 index 0000000000..5a1500602d --- /dev/null +++ b/packages/api/src/create/params.spec.js @@ -0,0 +1,38 @@ +// Copyright 2017-2018 Jaco Greeff +// This software may be modified and distributed under the terms +// of the ISC license. See the LICENSE file for details. + +const params = require('./params'); + +describe('params', () => { + let methods; + + beforeEach(() => { + methods = { + blah: { + inputs: [ + { name: 'foo', type: 'Bytes' } + ], + output: { type: 'Bytes' } + }, + bleh: { + inputs: [ + { name: 'foo', type: 'Bytes', isOptional: true } + ], + output: { type: 'Bytes' } + } + }; + }); + + it('check against params', () => { + expect( + () => params([], methods.blah.inputs) + ).toThrow(/params expected/); + }); + + it('check against params (required)', () => { + expect( + params([], methods.bleh.inputs) + ).toBeDefined(); + }); +});