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.

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

Deserializing to custom targets (classes) #253

Closed
wants to merge 2 commits into from
Closed

Deserializing to custom targets (classes) #253

wants to merge 2 commits into from

Conversation

rahbari
Copy link

@rahbari rahbari commented Jun 7, 2018

This pull request tries to cover this feature request https://github.com/mongodb/js-bson/issues/211

I thought of two options for nested targeting:

1- one (which current code is based on) is using static props of the top target, for example consider following document in mongodb:

{
    _id: 1,
    comments: [
          {_id: 1, content: ""},
          {_id: 2, content: ""}
    ]
}

Assigning the top document to an Article object:

class Article {}
articles.find({}, {target: Article});

Assigning articles to an extended Array:

class Article {
     static $comments = class Comments extends Array {}
} 
articles.find({}, {target: Article});

assigning articles items to a Comment class:

class Article {
     static $comments = class Comments extends Array {
            static $ = class Comment {}
     }
} 
articles.find({}, {target: Article});

I used $ at the begging of field names to prevent confusion with Class self properties, and used $ for array items.

2- Using a custom data structure like this:
articles.find({}, {targets: {root: Article, Comments: {root: Comments, item: Comment}}});

I believe first one is nicer but it's more confusing and the whole hierarchy of data must be defines.
If the maintainers accepts the general idea then it can be discussed.

For now I used firstBatch and nextBatch to detect documents parsing, until someone suggest a better approach.

if you want to test this code please use my modified mongodb core driver (I've added necessary code to commands.js).

daprahamian added a commit to mongodb/node-mongodb-native that referenced this pull request Jun 25, 2018
Reverts #1698. With mongodb/js-bson#253 and other approaches to
document-class mapping on the table, we'd like to have further
discussion before committing to any one API approach.

Reopens NODE-1450
daprahamian added a commit to mongodb/node-mongodb-native that referenced this pull request Jun 25, 2018
Reverts #1698. With mongodb/js-bson#253 and other approaches to
document-class mapping on the table, we'd like to have further
discussion before committing to any one API approach.

Reopens NODE-1450
daprahamian added a commit to mongodb/node-mongodb-native that referenced this pull request Jun 25, 2018
Reverts #1698. With mongodb/js-bson#253 and other approaches to
document-class mapping on the table, we'd like to have further
discussion before committing to any one API approach.

Reopens NODE-1450
@j
Copy link

j commented Jul 6, 2018

Hey @rahbari,

I too feel like it'd be nice to have this feature where (de)serialization happens.

This is cool but I still would hope for some sort of way of controlling the object instantiation.

Some potential ideas (typescript):

class Author {
  public name: string;
}

class Comment {
  public static $bson = {
    targets: {
      author: Author
    }
  };

  public author: Author;
  public body: string;
}

class Article {
  public static $bson = {
    targets: {
      author: Author,
      comments: [Comment]
    }
  };

  public comments: Comment[] = [];
  public author: Author;
  public body: string;
  public createdAt: Date;
}

articles.find<Article>({}, { target: Article })

The only thing I see that may be potentially missing is if people wanted to do field name mapping so that "createdAt" is the class property but is stored as "ca" in the database.

None-the-less, this would be a joy to have. :)

I slapped this together and have to go, so hopefully it all makes sense.

@j
Copy link

j commented Jul 9, 2018

Playing with this code isn't intuitive right now. It's not passing the target option to the deserializer.

@j
Copy link

j commented Jul 9, 2018

Also, the document detection question you have is the problem I had when trying to implement mapping into BSON. :(.. So I'm not sure the best way. I wish ONLY documents went through (de)serialize functions.

kiku-jw pushed a commit to kiku-jw/node-mongodb-native that referenced this pull request Feb 11, 2019
Reverts mongodb#1698. With mongodb/js-bson#253 and other approaches to
document-class mapping on the table, we'd like to have further
discussion before committing to any one API approach.

Reopens NODE-1450
@agolin95 agolin95 added the tracked-in-jira There is a ticket in Mongo's Jira instance tracking this issue/PR label Apr 8, 2021
@agolin95
Copy link
Contributor

agolin95 commented Apr 8, 2021

@bajanam
Copy link

bajanam commented Mar 7, 2022

Thank you for submitting this PR! While our current development plan is at capacity, we will track this in Jira for future prioritization.

@bajanam bajanam closed this Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tracked-in-jira There is a ticket in Mongo's Jira instance tracking this issue/PR
Projects
None yet
4 participants