Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feat deser class by schema collection #370

Merged
merged 29 commits into from
Jan 14, 2024
Merged

Feat deser class by schema collection #370

merged 29 commits into from
Jan 14, 2024

Conversation

fospring
Copy link
Contributor

@fospring fospring commented Nov 27, 2023 •
edited
Loading

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • Commit messages follow the conventional commits spec
  • If this is a code change: I have written unit tests.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

Improve user experience in writing smart contract:

  • decode json object to Origin Class, so users can call Class's methods after decode
  • auto check nested data structure inner the collections types so user no need to set options.reconstructor in get() method

Proposal and more details can be found in the file of AUTO_RECONSCTRUCT_BY_JSON_SCHEME.md

Test Plan

current example class to be decoded by json schema：

@NearBindgen({})
export class StatusDeserializeClass {
    static schema = {
        is_inited: "boolean",
        records: {map: { key: 'string', value: 'string' }},
        truck: Truck,
        messages: {array: {value: 'string'}},
        efficient_recordes: {unordered_map: {value: 'string'}},
        nested_efficient_recordes: {unordered_map: {value: { unordered_map: {value: 'string'}}}},
        nested_lookup_recordes: {unordered_map: {value: { lookup_map: {value: 'string'}}}},
        vector_nested_group: {vector: {value: { lookup_map: {value: 'string'}}}},
        lookup_nest_vec: {lookup_map: {value: { vector: { value: 'string' }}}},
        unordered_set: {unordered_set: {value: 'string'}},
        user_car_map: {unordered_map: {value: Car }},
        big_num: 'bigint',
        date: 'date'
    };
    constructor() {
        this.is_inited = false;
        this.records = {};
        this.truck = new Truck();
        this.messages = [];
        // account_id -> message
        this.efficient_recordes = new UnorderedMap("a");
        // id -> account_id -> message
        this.nested_efficient_recordes = new UnorderedMap("b");
        // id -> account_id -> message
        this.nested_lookup_recordes = new UnorderedMap("c");
        // index -> account_id -> message
        this.vector_nested_group = new Vector("d");
        // account_id -> index -> message
        this.lookup_nest_vec = new LookupMap("e");
        this.unordered_set = new UnorderedSet("f");
        this.user_car_map = new UnorderedMap("g");
        this.big_num = 1n;
        this.date = new Date();
        this.message_without_schema_defined = "";
        this.number_without_schema_defined = 0;
        this.records_without_schema_defined = {};
    }
}
cd ./examples
# build
pnpm run build:status-deserialize-class
# run
pnpm run test:status-deserialize-class

Related issues/PRs

#239

fospring added 15 commits December 4, 2023 00:06
@fospring
feat: deserialized js class with schema info
78567dc
@fospring
feat: WIP add test of get_record
2fa650c
@fospring
test: test decode class get record
2d65f4d
@fospring
test: test call inner class's method
4b9201c
@fospring
test: test decode class with inner array
a7230bc
@fospring
test: test decode UnorderMap and nested collections
f3e01c9
@fospring
test: get subtype of UnorderedMap and nested UnorderedMap
8cb2bd0
@fospring
feat: UnorderedMap remove reconstructor by add subtype and dynamic ch…
7c2cfe6 
…eck subtype
@fospring
feat: auto reconstruct for LookupMap
707054e
@fospring
feat: refact set_reconstructor and modulize
95e8e0f
@fospring
feat: vector nest other collections and test, decode and test Unorder…
1083681 
…edSet
@fospring
feat: add lookupset and remove useless log
ac40cd9
@fospring
feat: mv decodeObj2class to sdk
482ce68
@fospring
feat: decode subtype recursively
564440c
@fospring
test: add test of nested class of UnorderedMap
203e139
@fospring fospring force-pushed the feat-deser-class-by-schema-collection branch from 0451a4e to 203e139 Compare December 3, 2023 16:15
@fospring fospring force-pushed the feat-deser-class-by-schema-collection branch 2 times, most recently from 4d7bd09 to 5ab1e68 Compare December 9, 2023 14:37
@fospring fospring force-pushed the feat-deser-class-by-schema-collection branch from 5ab1e68 to 1eb4e97 Compare December 9, 2023 14:40
@fospring fospring changed the title (WIP)Feat deser class by schema collection Feat deser class by schema collection Dec 10, 2023
@fospring fospring force-pushed the feat-deser-class-by-schema-collection branch 3 times, most recently from a04ce8c to aed493d Compare December 10, 2023 17:16
@fospring fospring force-pushed the feat-deser-class-by-schema-collection branch 2 times, most recently from 63f3ece to 4c2e622 Compare December 17, 2023 15:56
@fospring fospring force-pushed the feat-deser-class-by-schema-collection branch from 4c2e622 to c43ab16 Compare December 17, 2023 16:53
ailisp
ailisp reviewed Dec 20, 2023
View reviewed changes
packages/near-sdk-js/src/utils.ts Outdated
}
}
// eslint-disable-next-line no-prototype-builtins
} else if (ty !== undefined && ty.hasOwnProperty(UNORDERED_MAP_SCHE)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we check if ty has reconstructor method, and call that instead of check UNORDERED_MAP_SCHE, etc.? If this is possible I feel it is more general and extensible, because assume user implement a new collection class, if he has reconstruct method implemented, check reconstructor approach will automatically work, but this way will need modify decodeObj2class.

On highlevel this is like: if schema exist, use schema. Otherwise try reconstructor. Else, simply class_instance[key] = obj[key]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I will try it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolve! I use this format in schema, a general collection type with reconstructor method:

@NearBindgen({})
export class StatusDeserializeClass {
    static schema = {
        is_inited: "boolean",
        records: {map: { key: 'string', value: 'string' }},
        truck: Truck,
        messages: {array: {value: 'string'}},
        efficient_recordes: {collection: {reconstructor: UnorderedMap.reconstruct, value: 'string'}},
        nested_efficient_recordes: {collection: {reconstructor: UnorderedMap.reconstruct, value: { collection: {reconstructor: UnorderedMap.reconstruct, value: 'string'}}}},
        nested_lookup_recordes: {collection: {reconstructor: UnorderedMap.reconstruct, value: { collection: {reconstructor: LookupMap.reconstruct, value: 'string'}}}},
        vector_nested_group: {collection: {reconstructor: Vector.reconstruct, value: { collection: {reconstructor: LookupMap.reconstruct, value: 'string'}}}},
        lookup_nest_vec: {collection: {reconstructor: LookupMap.reconstruct, value: { collection: { reconstructor: Vector.reconstruct, value: 'string' }}}},
        unordered_set: {collection: {reconstructor: UnorderedSet.reconstruct, value: 'string'}},
        user_car_map: {collection: {reconstructor: UnorderedMap.reconstruct, value: Car }},
        big_num: 'bigint',
        date: 'date'
    };
    constructor() {
        this.is_inited = false;
        this.records = {};
        this.truck = new Truck();
        this.messages = [];
        // account_id -> message
        this.efficient_recordes = new UnorderedMap("a");
        // id -> account_id -> message
        this.nested_efficient_recordes = new UnorderedMap("b");
        // id -> account_id -> message
        this.nested_lookup_recordes = new UnorderedMap("c");
        // index -> account_id -> message
        this.vector_nested_group = new Vector("d");
        // account_id -> index -> message
        this.lookup_nest_vec = new LookupMap("e");
        this.unordered_set = new UnorderedSet("f");
        this.user_car_map = new UnorderedMap("g");
        this.big_num = 1n;
        this.date = new Date();
        this.message_without_schema_defined = "";
        this.number_without_schema_defined = 0;
        this.records_without_schema_defined = {};
    }
// ..
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is challenging and your implementation is great! just one little catch,

        nested_efficient_recordes: {collection: {reconstructor: UnorderedMap.reconstruct, value: { collection: {reconstructor: UnorderedMap.reconstruct, value: 'string'}}}},

API looks a little bit unfriendly, can we slightly generalize and improve the API, to be:

nested_efficient_recordes: {class: UnorderedMap, value: UnorderedMap} // value: UnorderedMap is a shortcut for value: {class: UnorderedMap, value:'string'}

sdk should first try if UnorderedMap has a static schema and use it to recursively decode. In this case, UnorderedMap doesn't. So sdk should next try call UnorderedMap.reconstruct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I will try it

ailisp
ailisp approved these changes Dec 20, 2023
View reviewed changes
Copy link
Member

@ailisp ailisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is an excellent work that solves our long existing problem! There can be small improvements and additional features on top of this, for example if it's in typescript, make nearbindgen auto generate schema when build. But this PR is already a great enhancement, a well tested and a lot of work, let's only work on the PR reviews that can be quickly addressed, and create issue for bigger ones such as typescript auto generate.

Again this is a great job and I think this plus the borsh support worth a 2.0 release. Let's release after merge this PR!

@fospring fospring force-pushed the feat-deser-class-by-schema-collection branch from d2c5b40 to 24f4e5e Compare December 23, 2023 09:55
@fospring fospring force-pushed the feat-deser-class-by-schema-collection branch from a219875 to 1b4ec32 Compare December 23, 2023 13:19
@fospring fospring force-pushed the feat-deser-class-by-schema-collection branch from e43fe94 to 4ec38f8 Compare December 27, 2023 16:57
@fospring fospring force-pushed the feat-deser-class-by-schema-collection branch from 4ec38f8 to c8baf76 Compare December 27, 2023 17:00
@fospring fospring merged commit 0827713 into near:develop Jan 14, 2024
6 checks passed
@fospring fospring deleted the feat-deser-class-by-schema-collection branch January 25, 2024 17:24
@fospring fospring mentioned this pull request Jan 25, 2024
Merged
4 tasks
@fospring fospring mentioned this pull request Feb 25, 2024
Closed
4 tasks
@ailisp ailisp mentioned this pull request Apr 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ailisp ailisp ailisp approved these changes

@volovyks volovyks Awaiting requested review from volovyks volovyks is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fospring @ailisp