Skip to content

Commit

Permalink
Fall back to match registered type by name
Browse files Browse the repository at this point in the history
  • Loading branch information
spalladino committed Aug 25, 2023
1 parent 898fbe0 commit ff1cd9f
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 7 deletions.
22 changes: 18 additions & 4 deletions yarn-project/foundation/src/json-rpc/class_converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ export class ClassConverter {
* @returns If it is a registered class.
*/
isRegisteredClass(obj: any) {
return this.toName.has(obj);
const name = obj.prototype.constructor.name;
return this.toName.has(obj) || this.isRegisteredClassName(name);
}
/**
* Convert a JSON-like object to a class object.
Expand All @@ -182,10 +183,23 @@ export class ClassConverter {
* @returns The class object.
*/
toJsonObj(classObj: any): JsonEncodedClass | StringEncodedClass {
const result = this.toName.get(classObj.constructor);
assert(result, `Could not find class in lookup.`);
const [type, encoding] = result;
const { type, encoding } = this.lookupObject(classObj);
const data = encoding === 'string' ? classObj.toString() : classObj.toJSON();
return { type: type!, data };
}

/**
* Loads the corresponding type for this class based on constructor first and constructor name if not found.
* Constructor match works in the event of a minifier changing function names, and constructor name match
* works in the event of duplicated instances of node modules being loaded (see #1826).
* @param classObj - Object to lookup in the registered types.
* @returns Registered type name and encoding.
*/
private lookupObject(classObj: any) {
const nameResult = this.toName.get(classObj.constructor);
if (nameResult) return { type: nameResult[0], encoding: nameResult[1] };
const classResult = this.toClass.get(classObj.constructor.name);
if (classResult) return { type: classObj.constructor.name, encoding: classResult[1] };
throw new Error(`Could not find class ${classObj.constructor.name} in lookup.`);
}
}
32 changes: 32 additions & 0 deletions yarn-project/foundation/src/json-rpc/convert.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { Buffer } from 'buffer';

import { ClassConverter } from './class_converter.js';
import { convertBigintsInObj, convertFromJsonObj, convertToJsonObj } from './convert.js';
import { ToStringClass as ToStringClassA } from './fixtures/class_a.js';
import { ToStringClass as ToStringClassB } from './fixtures/class_b.js';
import { TestNote } from './fixtures/test_state.js';

const TEST_BASE64 = 'YmFzZTY0IGRlY29kZXI=';
Expand All @@ -24,3 +26,33 @@ test('does not convert a string', () => {
expect(convertBigintsInObj('hello')).toEqual('hello');
expect(convertBigintsInObj({ msg: 'hello' })).toEqual({ msg: 'hello' });
});

test('converts a registered class', () => {
const cc = new ClassConverter({ ToStringClass: ToStringClassA });
const obj = { content: new ToStringClassA('a', 'b') };
const serialised = convertToJsonObj(cc, obj);
const deserialised = convertFromJsonObj(cc, serialised) as { content: ToStringClassA };
expect(deserialised.content).toBeInstanceOf(ToStringClassA);
expect(deserialised.content.x).toEqual('a');
expect(deserialised.content.y).toEqual('b');
});

test('converts a class by name in the event of duplicate modules being loaded', () => {
const cc = new ClassConverter({ ToStringClass: ToStringClassA });
const obj = { content: new ToStringClassB('a', 'b') };
const serialised = convertToJsonObj(cc, obj);
const deserialised = convertFromJsonObj(cc, serialised) as { content: ToStringClassA };
expect(deserialised.content).toBeInstanceOf(ToStringClassA);
expect(deserialised.content.x).toEqual('a');
expect(deserialised.content.y).toEqual('b');
});

test('converts a class by constructor instead of name in the event of minified bundle', () => {
const cc = new ClassConverter({ NotMinifiedToStringClassName: ToStringClassA });
const obj = { content: new ToStringClassA('a', 'b') };
const serialised = convertToJsonObj(cc, obj);
const deserialised = convertFromJsonObj(cc, serialised) as { content: ToStringClassA };
expect(deserialised.content).toBeInstanceOf(ToStringClassA);
expect(deserialised.content.x).toEqual('a');
expect(deserialised.content.y).toEqual('b');
});
6 changes: 3 additions & 3 deletions yarn-project/foundation/src/json-rpc/convert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ export function convertFromJsonObj(cc: ClassConverter, obj: any): any {
return newObj;
}
// Throw if this is a non-primitive class that was not registered
if (typeof obj === 'object' && obj.constructor !== Object) {
throw new Error(`Object ${obj.constructor.name} not registered for deserialisation`);
if (typeof obj === 'object' && Object.getPrototypeOf(obj) === Object.getPrototypeOf({})) {
throw new Error(`Class ${obj.constructor.name} not registered for deserialisation`);
}
// Leave alone, assume JSON primitive
return obj;
Expand Down Expand Up @@ -122,7 +122,7 @@ export function convertToJsonObj(cc: ClassConverter, obj: any): any {
return newObj;
}
// Throw if this is a non-primitive class that was not registered
if (typeof obj === 'object' && obj.constructor !== Object) {
if (typeof obj === 'object' && Object.getPrototypeOf(obj) === Object.getPrototypeOf({})) {
throw new Error(`Object ${obj.constructor.name} not registered for serialisation`);
}
// Leave alone, assume JSON primitive
Expand Down
15 changes: 15 additions & 0 deletions yarn-project/foundation/src/json-rpc/fixtures/class_a.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* Test class for testing string converter.
*/
export class ToStringClass {
constructor(/** A value */ public readonly x: string, /** Another value */ public readonly y: string) {}

toString(): string {
return [this.x, this.y].join('-');
}

static fromString(value: string) {
const [x, y] = value.split('-');
return new ToStringClass(x, y);
}
}
15 changes: 15 additions & 0 deletions yarn-project/foundation/src/json-rpc/fixtures/class_b.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* Test class for testing string converter.
*/
export class ToStringClass {
constructor(/** A value */ public readonly x: string, /** Another value */ public readonly y: string) {}

toString(): string {
return [this.x, this.y].join('-');
}

static fromString(value: string) {
const [x, y] = value.split('-');
return new ToStringClass(x, y);
}
}

0 comments on commit ff1cd9f

Please sign in to comment.