scottwrobinson / camo

A class-based ES6 ODM for Mongo-like databases.
556 stars 80 forks source link

Support to exclude methods when extracting toJSON() #79

Closed BWorld closed 8 years ago

BWorld commented 8 years ago

Hi,

I think this project is awesome but I bumped into a small issue which I am creating this PR for. When extracting a model into JSON it is currently not (easily) possible to exclude methods.

I added an option which can disable the extraction of methods and only extract properties defined in the schema.

Happy to receive feedback.

scottwrobinson commented 8 years ago

Thanks for the PR! I just ran in to this problem recently as well. Guess I didn't think through the original implementation very well.

I'm thinking methods should always be excluded when serializing with toJSON(), or at least excluded by default, since this will be the vast majority of use-cases. I can see some cases where people will need to serialize functions and regexps, but that will require some extra work (see serialize-javascript). For now, I think we just exclude all methods.

BWorld commented 8 years ago

Hi Scott,

Thanks for your response. I implemented this as an option for keeping BC. Should it stay there until a major release is done?

Another thought I was having was that maybe introducing some extraction / hydration interface to the document instances will make it more flexible and documents more re-usable. i.e toJSON() proxies to a new method extract('json')

When you have input data you can easily hydrate it into your document instance using hydrate(data, 'ApiInput') for example because data hydrated from the mongo storage accepts more data than from some api endpoint for example. Same go's for outgoing data (extraction).

A code example:

const person = new Person();
person.hydrate({
    a : 123,
    b : 'abz'
}, 'foo');

class Person extends Document {
    hydrate(data, context) {

    }

    extract(context) {
        switch(context) {
            case 'api':
                return {...} // will format things specificly for api responses
            case 'statistics':
                return {....} // will format things specificly for statistic overviews maybe
            case ...
        }
    }
}

Default behavior would be hydrating and extracting based on the document definition. (as how it is done at the moment).

This will be just a start, you could also register hydrators by id having a class instance as value so you can separate the logic for hydration/extraction from the document instance itself. When you do this you can easily build some default hydrator which can take strategies into account on a per field basis.

Hydrator interface would be something like this (pseudo code)

interface Hydrator {
   Object extract(Document doc);
   void hydrate(Object data, Document doc);
}

class DefaultHydrator implements Hydrator {
        // extraction and hydration is based on doc schema
    extract(Document doc) {

    }
    hydrate(Object data, Document doc) {

    }
}

These are just thoughts and ideas to make it more flexible. Tell me what you think!

scottwrobinson commented 8 years ago

Thanks for the notes, Chris.

I was treating this as a bug, although you might be right that it should have been preserved for backwards compatibility.

As for the Hydrator, I like the idea, but I think it might be a bit out of scope for Camo. The same thing could be implemented in a base class and doesn't really need to be in Camo's Document.

However, I've been wanting to implement a plugin system in Camo, and I think the Hydrator would be perfect for that.

Let me know what you think. Thanks for the ideas!

BWorld commented 8 years ago

Hi Scott,

I will let that up to you how you like to release the change.

I think you are right that it belongs inside a plugin that will provide hydration and extraction per context.

For now, personally, I can do all the things with overriding the toJSON method combined / setting schema properties to private.

But ofcourse there are many other things you can do with plugins and it will be easier for others to contribute so +1 for plugins.

Are you still looking for contributors?

Chris. Op 29 jun. 2016 04:42 schreef "Scott Robinson" notifications@github.com:

Thanks for the notes, Chris.

I was treating this as a bug, although you might be right that it should have been preserved for backwards compatibility.

As for the Hydrator, I like the idea, but I think it might be a bit out of scope for Camo. The same thing could be implemented in a base class and doesn't really need to be in Camo's Document.

However, I've been wanting to implement a plugin system in Camo, and I think the Hydrator would be perfect for that.

Let me know what you think. Thanks for the ideas!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/scottwrobinson/camo/pull/79#issuecomment-229241709, or mute the thread https://github.com/notifications/unsubscribe/AAwmH9cQT4bnmQSUNH0zmQn2LB1EUbJLks5qQduagaJpZM4I9s_0 .

scottwrobinson commented 8 years ago

Yes! I'm still looking for contributors. Are you interested? I need to get a bit more organized, but I think there is some interesting stuff to work on.

Let me know what you think.

Scott

BWorld commented 8 years ago

Sounds good and yes I am interested.

Send me an e-mail with your ideas and then we discuss this further in private. My e-mail is: chris at dutchfrontiers dot com

Chris. Op 30 jun. 2016 03:54 schreef "Scott Robinson" notifications@github.com:

Yes! I'm still looking for contributors. Are you interested? I need to get a bit more organized, but I think there is some interesting stuff to work on.

Let me know what you think.

Scott

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/scottwrobinson/camo/pull/79#issuecomment-229539316, or mute the thread https://github.com/notifications/unsubscribe/AAwmH9tllXZ3RAK-mfzfTXNUzbtaxDdaks5qQyHrgaJpZM4I9s_0 .

kurdin commented 7 years ago

This update kills virtual properties getter to functional.

Now is this code, user object does not contain fullName key with value Billy Bob

class User extends Document {
                constructor() {
                    super();
                    this.firstName = String;
                    this.lastName = String;
                }

                set fullName(name) {
                    var split = name.split(' ');
                    this.firstName = split[0];
                    this.lastName = split[1];
                }

                get fullName() {
                    return this.firstName + ' ' + this.lastName;
                }
            }

            var user = User.create({
                fullName: 'Billy Bob'
            });

            console.log('user', user);