From ad2939cc4f73030c5f125c24780b36cd54fcba8e Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Tue, 18 Feb 2020 13:35:42 +0100 Subject: [PATCH] Rework Unconstructable type (#1888) * Rework Unconstructable type * hasType updates * Unknown -> Unconstructable (as it was) * Remove commented test * class name * getOrThrow tests * typo. * getOrUnknown --- copy-to-apps.sh | 18 ++++++------- packages/types/src/create/createClass.spec.ts | 13 +++++---- packages/types/src/create/createClass.ts | 4 +-- packages/types/src/create/registry.spec.ts | 16 ++++++++++- packages/types/src/create/registry.ts | 27 ++++++++++++++----- packages/types/src/index.spec.ts | 1 + packages/types/src/interfaceRegistry.ts | 5 +++- .../types/src/primitive/Unconstructable.ts | 12 +++++---- packages/types/src/primitive/index.ts | 1 + packages/types/src/types/registry.ts | 3 ++- 10 files changed, 70 insertions(+), 30 deletions(-) diff --git a/copy-to-apps.sh b/copy-to-apps.sh index 40f7826a62..dbf2cce6d1 100755 --- a/copy-to-apps.sh +++ b/copy-to-apps.sh @@ -2,7 +2,7 @@ function copy_folder () { SRC="packages/$1/build" - DST="../apps/node_modules/@polkadot/$2" + DST="../apps/node_modules/@polkadot/$1" echo "** Copying $SRC to $DST" @@ -12,11 +12,11 @@ function copy_folder () { yarn polkadot-dev-build-ts -copy_folder "api" "api" -copy_folder "api-contract" "api-contract" -copy_folder "api-derive" "api-derive" -copy_folder "metadata" "metadata" -copy_folder "rpc-core" "rpc-core" -copy_folder "rpc-provider" "rpc-provider" -copy_folder "type-jsonrpc" "jsonrpc" -copy_folder "types" "types" +copy_folder "api" +copy_folder "api-contract" +copy_folder "api-derive" +copy_folder "metadata" +copy_folder "rpc-core" +copy_folder "rpc-provider" +copy_folder "jsonrpc" +copy_folder "types" diff --git a/packages/types/src/create/createClass.spec.ts b/packages/types/src/create/createClass.spec.ts index 5783953e97..c8f090eabe 100644 --- a/packages/types/src/create/createClass.spec.ts +++ b/packages/types/src/create/createClass.spec.ts @@ -2,7 +2,7 @@ // This software may be modified and distributed under the terms // of the Apache-2.0 license. See the LICENSE file for details. -import { Codec, Constructor } from '../types'; +import { TypeDefInfo } from '../types'; import { createClass, getTypeClass, TypeRegistry } from '.'; @@ -27,9 +27,12 @@ describe('createClass', (): void => { describe('getTypeClass', (): void => { const registry = new TypeRegistry(); - it('does not allow invalid types', (): void => { - expect( - (): Constructor => getTypeClass(registry, 'SomethingInvalid' as any) - ).toThrow(/determine type/); + it('warns on invalid types', (): void => { + const spy = jest.spyOn(console, 'warn'); + const typeDef = { info: TypeDefInfo.Plain, type: 'ABC' }; + + getTypeClass(registry, typeDef); + + expect(spy).toHaveBeenCalledWith('Unable to resolve type ABC, it will fail on constrution'); }); }); diff --git a/packages/types/src/create/createClass.ts b/packages/types/src/create/createClass.ts index 494c15df35..39d39c3e77 100644 --- a/packages/types/src/create/createClass.ts +++ b/packages/types/src/create/createClass.ts @@ -107,7 +107,7 @@ const infoMapping: Record C [TypeDefInfo.Option]: (registry: Registry, value: TypeDef): Constructor => Option.with(getSubType(value)), [TypeDefInfo.Plain]: (registry: Registry, value: TypeDef): Constructor => - registry.getOrThrow(value.type, `Unable to find plain type for ${JSON.stringify(value)}`), + registry.getOrUnknown(value.type), [TypeDefInfo.Result]: (registry: Registry, value: TypeDef): Constructor => { const [Ok, Error] = getTypeClassArray(value); @@ -166,7 +166,7 @@ export function getTypeClass (registry: Registry, value const getFn = infoMapping[value.info]; if (!getFn) { - throw new Error(`Unable to determine type from ${JSON.stringify(value)}`); + throw new Error(`Unable to construct class from ${JSON.stringify(value)}`); } return getFn(registry, value) as Constructor; diff --git a/packages/types/src/create/registry.spec.ts b/packages/types/src/create/registry.spec.ts index 046d68467e..4fad9dedf5 100644 --- a/packages/types/src/create/registry.spec.ts +++ b/packages/types/src/create/registry.spec.ts @@ -2,11 +2,12 @@ // This software may be modified and distributed under the terms // of the Apache-2.0 license. See the LICENSE file for details. -import { Constructor } from '../types'; +import { Codec, Constructor } from '../types'; import Struct from '../codec/Struct'; import Text from '../primitive/Text'; import U32 from '../primitive/U32'; +import Unconstructable from '../primitive/Unconstructable'; import { TypeRegistry } from './registry'; // Copied from interfacesTs @@ -24,6 +25,19 @@ describe('TypeRegistry', (): void => { expect(registry.get('non-exist')).toBeUndefined(); }); + it('throws on non-existent via getOrThrow', (): void => { + expect((): Constructor => registry.getOrThrow('non-exist')).toThrow('type non-exist not found'); + expect((): Constructor => registry.getOrThrow('non-exist', 'foo bar blah')).toThrow('foo bar blah'); + }); + + it('handles non exist type as Unknown (via getOrUnknown)', (): void => { + const Type = registry.getOrUnknown('non-exist'); + + expect(Type).toBeDefined(); + // eslint-disable-next-line no-prototype-builtins + expect(isChildClass(Unconstructable, Type)); + }); + it('can register single type', (): void => { registry.register(Text); expect(registry.get('Text')).toBe(Text); diff --git a/packages/types/src/create/registry.ts b/packages/types/src/create/registry.ts index e1ddf4dc58..ded301a0d1 100644 --- a/packages/types/src/create/registry.ts +++ b/packages/types/src/create/registry.ts @@ -12,6 +12,7 @@ import { assert, formatBalance, isFunction, isString, isU8a, isUndefined, string import Raw from '../codec/Raw'; import { EventData } from '../primitive/Generic/Event'; import { defaultExtensions, expandExtensionTypes, findUnknownExtensions } from '../primitive/Extrinsic/signedExtensions'; +import Unconstructable from '../primitive/Unconstructable'; import { createClass, getTypeClass } from './createClass'; import { getTypeDef } from './getTypeDef'; @@ -90,6 +91,8 @@ export class TypeRegistry implements Registry { private _metadataExtensions: string[] = defaultExtensions; + private _unknownTypes: Map = new Map(); + constructor () { // we only want to import these on creation, i.e. we want to avoid types // weird side-effects from circular references. (Since registry is injected @@ -157,17 +160,25 @@ export class TypeRegistry implements Registry { return Event; } - public get (name: string): Constructor | undefined { + public get (name: string, withUnknown?: boolean): Constructor | undefined { let Type = this._classes.get(name); // we have not already created the type, attempt it if (!Type) { const definition = this._definitions.get(name); + let BaseType: Constructor | undefined; // we have a definition, so create the class now (lazily) if (definition) { - const BaseType = createClass(this, definition); + BaseType = createClass(this, definition); + } else if (withUnknown) { + console.warn(`Unable to resolve type ${name}, it will fail on constrution`); + this._unknownTypes.set(name, true); + BaseType = Unconstructable.with(name); + } + + if (BaseType) { // NOTE If we didn't extend here, we would have strange artifacts. An example is // Balance, with this, new Balance() instanceof u128 is true, but Balance !== u128 // Additionally, we now pass through the registry, which is a link to ourselves @@ -189,13 +200,17 @@ export class TypeRegistry implements Registry { } public getOrThrow (name: string, msg?: string): Constructor { - const type = this.get(name); + const Type = this.get(name); - if (isUndefined(type)) { + if (isUndefined(Type)) { throw new Error(msg || `type ${name} not found`); } - return type; + return Type; + } + + public getOrUnknown (name: string): Constructor { + return this.get(name, true) as Constructor; } public getSignedExtensionExtra (): Record { @@ -215,7 +230,7 @@ export class TypeRegistry implements Registry { } public hasType (name: string): boolean { - return this.hasClass(name) || this.hasDef(name); + return !this._unknownTypes.get(name) && (this.hasClass(name) || this.hasDef(name)); } public register (type: Constructor | RegistryTypes): void; diff --git a/packages/types/src/index.spec.ts b/packages/types/src/index.spec.ts index 889c996484..70eb53c722 100644 --- a/packages/types/src/index.spec.ts +++ b/packages/types/src/index.spec.ts @@ -18,6 +18,7 @@ const UNCONSTRUCTABLE = [ 'ExtrinsicPayloadUnknown', 'GenericExtrinsicPayloadUnknown', 'ExtrinsicUnknown', 'GenericExtrinsicUnknown', 'GenericOrigin', 'Origin', + 'Unconstructable', 'usize' ].map((v): string => v.toLowerCase()); diff --git a/packages/types/src/interfaceRegistry.ts b/packages/types/src/interfaceRegistry.ts index 34d6527692..789a16479d 100644 --- a/packages/types/src/interfaceRegistry.ts +++ b/packages/types/src/interfaceRegistry.ts @@ -2,7 +2,7 @@ /* eslint-disable @typescript-eslint/no-empty-interface */ import { Compact, Option, Raw, Vec } from '@polkadot/types/codec'; -import { BitVec, Bytes, Data, Fixed64, H160, H256, H512, IdentityFields, Null, StorageData, StorageKey, Text, Type, U256, bool, i128, i16, i256, i32, i64, i8, u128, u16, u256, u32, u64, u8, usize } from '@polkadot/types/primitive'; +import { BitVec, Bytes, Data, Fixed64, H160, H256, H512, IdentityFields, Null, StorageData, StorageKey, Text, Type, U256, Unconstructable, bool, i128, i16, i256, i32, i64, i8, u128, u16, u256, u32, u64, u8, usize } from '@polkadot/types/primitive'; import { AccountId, AccountIdOf, AccountIndex, Address, AssetId, Balance, BalanceOf, Block, BlockNumber, Call, ChangesTrieConfiguration, Consensus, ConsensusEngineId, Digest, DigestItem, DispatchClass, DispatchInfo, DispatchInfoTo190, EcdsaSignature, Ed25519Signature, Extrinsic, ExtrinsicEra, ExtrinsicPayload, ExtrinsicPayloadUnknown, ExtrinsicPayloadV1, ExtrinsicPayloadV2, ExtrinsicPayloadV3, ExtrinsicPayloadV4, ExtrinsicSignatureV1, ExtrinsicSignatureV2, ExtrinsicSignatureV3, ExtrinsicSignatureV4, ExtrinsicUnknown, ExtrinsicV1, ExtrinsicV2, ExtrinsicV3, ExtrinsicV4, Hash, Header, ImmortalEra, Index, Justification, KeyTypeId, KeyValue, LockIdentifier, LookupSource, LookupTarget, Moment, MortalEra, MultiSignature, Origin, Perbill, Percent, Permill, Perquintill, Phantom, PhantomData, PreRuntime, Seal, SealV0, Signature, SignedBlock, SignerPayload, Sr25519Signature, ValidatorId, Weight, WeightMultiplier } from '@polkadot/types/interfaces/runtime'; import { InclusionHeight, Uncle, UncleEntryItem } from '@polkadot/types/interfaces/authorship'; import { RawAuraPreDigest } from '@polkadot/types/interfaces/aura'; @@ -124,6 +124,9 @@ export interface InterfaceRegistry { 'Compact': Compact; 'Option': Option; 'Vec': Vec; + Unconstructable: Unconstructable; + 'Option': Option; + 'Vec': Vec; usize: usize; 'Compact': Compact; 'Option': Option; diff --git a/packages/types/src/primitive/Unconstructable.ts b/packages/types/src/primitive/Unconstructable.ts index ee6d841156..f61f02af17 100644 --- a/packages/types/src/primitive/Unconstructable.ts +++ b/packages/types/src/primitive/Unconstructable.ts @@ -9,18 +9,20 @@ import Null from './Null'; /** * @name Unconstructable * @description - * A type that should not be constructed + * An unknown type theat fails on constrction with the type info */ export default class Unconstructable extends Null { - constructor (registry: Registry) { + constructor (registry: Registry, typeName: string) { super(registry); - throw new Error('Unconstructable should not be constructed, it is only a placeholder for compatibility'); + throw new Error(`Cannot construct unknown type ${typeName}`); } - // eslint-disable-next-line @typescript-eslint/no-unused-vars - public static with (typeDef: any): Constructor { + public static with (typeName: string): Constructor { return class extends Unconstructable { + constructor (registry: Registry) { + super(registry, typeName); + } }; } } diff --git a/packages/types/src/primitive/index.ts b/packages/types/src/primitive/index.ts index 2a4eeeb2e6..4e897e3c7e 100644 --- a/packages/types/src/primitive/index.ts +++ b/packages/types/src/primitive/index.ts @@ -34,6 +34,7 @@ export { default as u32 } from './U32'; export { default as u64 } from './U64'; export { default as u128 } from './U128'; export { default as u256, default as U256 } from './U256'; +export { default as Unconstructable } from './Unconstructable'; export { default as usize } from './USize'; export * from './Generic'; diff --git a/packages/types/src/types/registry.ts b/packages/types/src/types/registry.ts index 87e4e8648c..95538bb7d7 100644 --- a/packages/types/src/types/registry.ts +++ b/packages/types/src/types/registry.ts @@ -92,10 +92,11 @@ export interface Registry { // keep this as a generic Codec, however the actual impl. returns the correct findMetaEvent (eventIndex: Uint8Array): Constructor; - get (name: string): Constructor | undefined; + get (name: string, withUnknown?: boolean): Constructor | undefined; getChainProperties (): ChainProperties | undefined; getDefinition (name: string): string | undefined; getOrThrow (name: string, msg?: string): Constructor; + getOrUnknown (name: string): Constructor; getSignedExtensionExtra (): Record; getSignedExtensionTypes (): Record; hasClass (name: string): boolean;