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

JSON.stringify() a model skips some properties #5464

Closed
pixelfreak opened this issue Mar 13, 2014 · 36 comments
Closed

JSON.stringify() a model skips some properties #5464

pixelfreak opened this issue Mar 13, 2014 · 36 comments

Comments

@pixelfreak
Copy link

I'm on Sails 0.10. When I do JSON.stringify(someModel), some properties, specifically properties associated with other model is not outputted. How do I get it enumerated?

Thanks!

@mikermcneil
Copy link
Member

Hmm wonder if toJSON() gets called automatically by JSON.stringify()? Doubt it.. What happens if you call toJSON() on the model instance first?

@pixelfreak
Copy link
Author

Same thing, the properties still won't show up.

@particlebanana
Copy link
Contributor

@pixelfreak can you post your models and the code you are using the JSON.stringify() with. I just spun up an example project to test this out and couldn't recreate it. I used the models and controller code below:

// User model
module.exports = {
  attributes: {
    name: 'string',
    pets: {
      collection: 'pet',
      via: 'owner'
    }
  }
};
// Pet model
module.exports = {
  attributes: {
    name: 'string',
    owner: {
      model: 'user'
    }
  }
};
// Controller method
find: function(req, res) {
  User.findOne(1).populate('pets').exec(function(err, user) {
    if(err) return res.serverError(err);
    var str = JSON.stringify(user);
    console.log(str);
    res.send(str);
  });
}

@pixelfreak
Copy link
Author

Here it is:

module.exports =
{
    tableName: 'screen_category',
    attributes:
    {
        id:
        {
            type: 'INTEGER',
            primaryKey: true
        },
        name: 'STRING',
        type: { type: 'STRING', in: ['bowl-builder', 'grid-item'] },
        sortOrder: 'INTEGER',
        items:
        {
            collection: 'Item',
            via: 'screenCategory'
        }
    }
};
module.exports =
{
    tableName: 'item',
    attributes:
    {
        id:
        {
            type: 'INTEGER',
            primaryKey: true
        },
        name: 'STRING',
        description: 'TEXT',
        price: 'DECIMAL',
        sku: 'STRING',
        active: 'BOOLEAN',
        sortOrder: 'INTEGER',
        screenCategory:
        {
            model: 'ScreenCategory'
        },
        choiceCategories:
        {
            collection: 'ChoiceCategory',
            via: 'items',
            dominant: true
        },
        storeItems:
        {
            collection: 'StoreItemMap',
            via: 'itemId'
        }
    }
};

items in screen_category is not showing up.

@pixelfreak
Copy link
Author

I found that this is not just when JSON.stringify() is called. It also happens when I do res.json(someModel);

@particlebanana
Copy link
Contributor

@pixelfreak there was an issue a few days ago we ran into with populate and custom defined primary keys that may have been the issue here. I just pushed 0.10.0-rc6 to npm if you want to give it a shot. I built your example above and got this back when populating items:

[
    {
        "items": [
            {
                "id": 1,
                "name": "movie-item",
                "createdAt": "2014-03-20T20:52:30.082Z",
                "updatedAt": "2014-03-20T20:52:30.082Z",
                "screenCategory": 1
            },
            {
                "id": 2,
                "name": "music-item",
                "createdAt": "2014-03-20T20:52:37.200Z",
                "updatedAt": "2014-03-20T20:52:37.200Z",
                "screenCategory": 1
            }
        ],
        "id": 1,
        "name": "movie",
        "createdAt": "2014-03-20T20:51:58.989Z",
        "updatedAt": "2014-03-20T20:51:58.989Z"
    }
]

@pixelfreak
Copy link
Author

Same thing. items is missing when the screenCategory is stringified. But if I stringify screenCategory.items it's there. :(

@pixelfreak
Copy link
Author

This is so weird...if I do this:

myScreenCategory.foo = myScreenCategory.items;
JSON.stringify(myScreenCategory);

...then foo will show up with all the data that items has but items itself is still missing...

@ArdentZeal
Copy link

I had the same Problem with populated items not showing up after using toJSON, so I omitted it. I suspect it to be the additional add and remove function in many associations, which will break the stringify.

Kind Regards

Sent from my iPhone

On 22.03.2014, at 03:32, William Khoe notifications@github.com wrote:

This is so weird...if I do this:

myScreenCategory.foo = myScreenCategory.items;
JSON.stringify(myScreenCategory);
...then foo will show up with all the data that items have but items itself is still missing...


Reply to this email directly or view it on GitHub.

@TheFinestArtist
Copy link

My problem is when I populate an attribute, it shows all attribute including deleted one from to JSON


module.exports = {

  attributes: {

    type: {
      type: 'string',
      required: true
    },

    notification_key: {
      type: 'string'
    },

    // One to One
    owner: {
        model: 'Users'
    },

    toJSON: function() {
      var obj = this.toObject();
      delete obj.createdAt;
      delete obj.updatedAt;
      delete obj.notification_key;
      return obj;
    }
  }
}
{
  username: "appletest0",
  email: "apple0@apple.com",
  full_name: "Apple Test 0",
  profile_image: null,
  device: {
    type: "iphone",
    notification_key: "4710ebf36bee1a463b8b33f9128b6515505385800",
    id: 1,
    createdAt: "2014-02-22T04:21:53.000Z",
    updatedAt: "2014-03-22T16:59:39.000Z",
    owner: 1
  },
  id: 1
},

@particlebanana
Copy link
Contributor

@TheFinestArtist can you check which version of Waterline you are using? You should be on 0.10.0-rc6. Using that and your data models this is what I get back:

 {
        "device": {
            "type": "iphone",
            "id": 1,
            "owner": 1
        },
        "username": "appletest0",
        "email": "apple0@apple.com",
        "full_name": "Apple Test 0",
        "createdAt": "2014-03-24T17:08:08.633Z",
        "updatedAt": "2014-03-24T17:08:08.633Z",
        "id": 1
    }

@pixelfreak
Copy link
Author

I am on 0.10.0-rc7 now and it is still happening :(

It definitely looks like the javascript object properties defined is causing some of the properties to not be enumerable. Why is it done this way, I don't know...

sails-mysql@0.10.0-rc3
sails@0.10.0-rc5

@particlebanana
Copy link
Contributor

@pixelfreak so here's a run down on how the associations and the .toJSON() method work together
so maybe you can see where the issue is. I can't recreate your issue with any custom controller code
so if you could post that maybe it would help out.

The .toJSON() method you define in your model is nothing special. It's simply overriding the normal
object .toJSON() method which is similar to a .toString() method and will get run anytime you do
a JSON.stringify() or res.json() call. The special method from Waterline that does the work is
.toObject() which uses the params passed into the queries .populate() method to determine which
virtual collection attributes should be attached to the response and which ones should be left out.

So given the model:

attributes: {
  cars: {
    collection: 'car',
    via: 'driver'
  },

  boats: {
    collection: 'boat',
    via: 'sailors'
  }
}

There are two virtual attributes: cars and boats that live on the model but not at the database
level. So if I do a query like so:

Model.find().exec(function(/* ... */))

I would get the following response when passed through .toObject():

[
  {
    id: 1,
    createdAt: 'some timestamp',
    updatedAt: 'some timestamp'
  }
]

But if I did the following query I would get a different response:

Model.find()
.populate('cars')
.populate('boats')
.exec(function(/* ... */))

Would give me:

[
  {
    id: 1,
    cars: [],
    boats: [],
    createdAt: 'some timestamp',
    updatedAt: 'some timestamp'
  }
]

This is because I told the model class that I care about the cars and boats virtual attributes.

What I suspect is happening in your case, is that you are attaching associated collection properties
onto a model, probably for deeper populations, and then the .toObject() method is run and is
stripping them. This is because in the context of the parent it doesn't know you are interested in the
collection attributes that you have tacked on and will strip them out.

If these virtual collection attributes are not stripped out when you don't query them, you would
always get back empty arrays for the collection attributes if they were not populated. Which is the
wrong result and why it works that way.

If this is the case, a fix in your code is to call .toObject() before you attach the additional
properties. This will return a new plain javascript object (POJO) that you can attach anything to.
Another option would be to clone things before mutating them which should give you a similar effect.

cc @mikermcneil

@skrichten
Copy link

@particlebanana I've been trying to do "deeper populations" as you put it, and I'm not having much luck finding a nice clean way to do it. I would love to see a good example of how to do this. Why not have .toObject() check if any associated data is there rather than relying on attributes of the parent query?

@dantswain
Copy link

I may be having a similar issue, though there's a weird twist with express. I have a pair of HABTM model - DiaryItem and Tag. I have some logic in my controller to handle creating a DiaryItem with Tags attached to it, and then a helper method to reload the final result and send it back to the user. The reload-and-send method looks like this:

function reloadAndSendJSON(id, res) {
  DiaryItem.findOne({id: id}).populate('tags').exec(function(err, found) {
    if(err) { res.send(err) };

    console.log(found.toJSON());
    res.send(found);
  });
};

If I post something like this:

{
  "name": "OHAI",
  "tags": [ { "name": "fun" }, {"name": "awesome"} ]
}

I see the right things in the console and response:

{
  "tags": [
    {
      "name": "fun",
      "id": 48,
      "createdAt": "2014-04-21T03:54:15.000Z",
      "updatedAt": "2014-04-21T03:54:15.000Z"
    },
    {
      "name": "awesome",
      "id": 49,
      "createdAt": "2014-04-21T03:54:15.000Z",
      "updatedAt": "2014-04-21T03:54:15.000Z"
    }
  ],
  "name": "OHAI",
  "time": "2014-04-21T04:12:26.000Z",
  "id": 178,
  "createdAt": "2014-04-21T04:12:26.000Z",
  "updatedAt": "2014-04-21T04:12:26.000Z"
}

However, if I do res.send(found.toJSON()) instead of res.send(found), the response omits the tags. They will still show up in the call to console.log, but will be missing from the response. I can even do something like this:

var s = found.toJSON();
console.log(s);
res.send(s);

and I still get the weird behavior (tags present in log and missing in response).

@particlebanana
Copy link
Contributor

Hey guys as explained above the toJSON is not the issue the toObject method is. @dantswain when you are changing this object you are not changing a plain javascript object, you are changing an instance of a model.

The solution is to either call toJSON or toObject before you change it or to clone it _.cloneDeep(found) which would give you a plain object.

We are working on deeper populations which once available will allow you do this stuff without worrying about building the response manually.

@skrichten
Copy link

Here is an example of how I managed to get a deep population to work...
https://github.com/balderdashy/waterline/issues/256#issuecomment-42742446

@andrewjmead
Copy link

+1 this is still an issue for me. I'm using 0.10.16.

I tried calling toJSON and tried toObject, neither worked.

@wallaceicy06
Copy link

+1 this is still an issue for me as well

@anasyb
Copy link

anasyb commented Mar 4, 2015

This is still an issue. A simple (but very bad) workaround is to set the .toJSON property of the returned record to anything other than a function, for example:

User.find().exec(function(err, users){
  console.log(JSON.stringify(users[0]))
})
//{ id:1, name:'Susan'} //no post IDs!

User.find().exec(function(err, users){
  console.log(users[0].posts);
})
//['1', '2', '3', add:[Function add], remove:[Function remove]]  //but actually here they are!!

User.find().exec(function(err, users){
  users[0].toJSON="I won't allow you to use sails toJSON, just use the native stringification";
  console.log(JSON.stringify(users[0]))
})
//{ id:1, name:'Susan', posts:[1,2,3]} //post IDs are back!

As indicated by @particlebanana this is a problem with toObject, in particular it has to do with this line, blame it for all this mess!. Fixing the if condition or commenting it in your local waterline solves the problem, which is obviously a much better workaround, that is until it get official fixed.

This is very related to the user-case behind https://github.com/balderdashy/waterline/issues/532, although it is a completely separate issue. cc @mphasize

@tjwebb
Copy link
Contributor

tjwebb commented Mar 4, 2015

@particlebanana this was marked In Progress forever ago. is there any update or did it call through the cracks

@devinivy
Copy link

Any word here? @anasyb do you think your fix is solid enough to submit a PR?

@anasyb
Copy link

anasyb commented Mar 26, 2015

@devinivy I had it running for a while with that line commented out... no notice of anything going wrong. Before a PR I suppose we need an understanding of what that's line purpose in life is/was.

Can someone run the tests with that line commented and post here which ones fail (if any)?

@devinivy
Copy link

I wonder if showJoins is possibly not being accounted for on that line.

If someone could even write a failing test here, that would be awesome.

@anasyb
Copy link

anasyb commented Mar 27, 2015

Test case should be fairly simple, test if an association that has some items in it (therefore .length > 0) has same .length before and after calling .toObject(). Here is an example:

User.find().exec(function(err, rec){
  //15 (or whatever length truly is!)
  console.log(rec[0].posts.length);

  //0 (even though length isn't 0 as shown above)
  console.log(rec[0].toObject().posts.length);

  //false (so this should be the test case)
  console.log(rec[0].posts.length===rec[0].toObject().posts.length 
    && rec[0].posts.length !== 0);
})

Sorry didn't have time as yet to put that into a test file... let me know if you can do that :P

@anasyb
Copy link

anasyb commented Mar 28, 2015

@devinivy interesting.. seems like there is a test case to make sure the bug is there :D maybe we're missing something?!

If we're not missing something: assuming new model({ name: 'foobar' }); is creating the association arrays (otherwise the existing test cases doesn't make sense to me?!) I would change that case to:

      it('should NOT strip out the association key when no options are passed', function() {
        var person = new model({ name: 'foobar' });
        var obj = person.toObject();

        assert(obj === Object(obj));
        assert(obj.name === 'foobar');
        assert(obj.bars);
        assert(obj.foobars);
        assert(person.bars);
        assert(person.foobars);

        assert(person.bars.length > 0);
        assert(person.foobars.length > 0);
        assert(person.bars.length === obj.bars.length);
        assert(person.foobars.length === person.foobars.length);
      });

@dmarcelino
Copy link
Member

One possible reason for removing the associations (I'm speculating) is to avoid circular references before using .toJSON(). If by any chance that is the reason we need to decycle the object. It would be good to hear from the guys who actually wrote that code about this.

@anasyb
Copy link

anasyb commented Mar 28, 2015

Interesting... though on a second thought I can't see how cycles can emerge given that the if statement is doing the opposite: it's returning when the join wasn't used (which means the IDs will stay mere strings)

@m0g
Copy link

m0g commented Mar 31, 2015

Hi, I'm using Waterline v0.10.19 and I'm having the same issue.

Question.findOne({ id: req.params.id })
      .populate('responses')
      .populate('options')
      .exec(function(err, question) {
        res.json(question.toObject());
      });

And in the browser responses & options are now gone.

"survey": 120,
"title": "...",
"number": null,
"maxNumericDigits": null,
"responseType": null,
"isConditional": null,
"numChoice": null,
"synced": null,
"type": "binary",
"id": 455,
"createdAt": "2015-03-31T11:08:41.000Z",
"updatedAt": "2015-03-31T11:08:41.000Z"

@anasyb
Copy link

anasyb commented Mar 31, 2015

Hi @m0g !
Sorry the workarounds I posted above might not work as they did for me... it turned out I was so lucky that a bug was allowing it to work for me!
The strange thing in your code is that you're already populating... so stripping out the associated record should be de-activated on your toObject by default!

Here is a quick and dirty code snippet that keeps only the IDs if that's what you're after, can you try that?

var Model = Question;

//populates those of N cardinality on any/both side(s)
var populateManyAssociations = function ( query, associations) {
  var query = query;
  for (var i = associations.length - 1; i >= 0; i--) {
    var assoc = associations[i];
    //if cardinality isn't many skip
    if (assoc.type !== "collection") continue;
    query = query.populate(assoc.alias);
  };
  return query;
}

var query = Model.findOne({ id: req.params.id });
query = populateManyAssociations(query, Model.associations);
query.exec(function(err, r) {
  var robj = r.toObject();
  function getId (obj){
    if (obj)  return obj.id;
  }

  //now manually keep IDs
  for (var i = Model.associations.length - 1; i >= 0; i--) {
    var assoc = Model.associations[i];

    //if cardinality isn't many skip
    if (assoc.type !== "collection") continue;

    robj[assoc.alias] = robj[assoc.alias].map(getId);
  };

  res.json(robj);
});

@m0g
Copy link

m0g commented Apr 1, 2015

Hey @anasyb, thanks for you help.

Let me elaborate a bit more.

What I want to do is to convert question into an object. Because I want to be able to override the content of question.options if ever I feel like doing so.

If I don't convert question to object then I can't override the options, unless I add a custom property like question.customOptions = customOptions.

And if I do convert question to an object then all the associations are disappearing when outputting to JSON (but remains on the console.log).

Here is an example:

var populateManyAssociations = function ( query, associations) {
  for (var i = associations.length - 1; i >= 0; i--) {
    var assoc = associations[i];
    //if cardinality isn't many skip
    if (assoc.type !== "collection") continue;
    query = query.populate(assoc.alias);
  }

  return query;
};

 var query = Question.findOne({ id: req.params.id });
    query = populateManyAssociations(query, Question.associations);

    query.exec(function(err, question) {
      var questionObj = question.toObject();

      var getId = function(obj) {
        return obj;
      };

      for (var i = Question.associations.length - 1; i >= 0; i--) {
        var assoc = Question.associations[i];

        if (assoc.type !== "collection") continue;

        questionObj[assoc.alias] = questionObj[assoc.alias].map(getId);
      }

      console.log('responses', questionObj.responses.length);
      res.json(questionObj);
  });

Server side output

responses 1196

Browser output

{"survey": 120,
"title": "...",
"number": null,
"maxNumericDigits": null,
"responseType": null,
"isConditional": null,
"numChoice": null,
"synced": null,
"type": "binary",
"id": 455,
"createdAt": "2015-03-31T11:08:41.000Z",
"updatedAt": "2015-03-31T11:08:41.000Z"}

Thanks

@dmarcelino
Copy link
Member

I'm wondering if this can be caused by #891 (Model#toJSON returns an object with a toJSON method)?

@m0g
Copy link

m0g commented Apr 1, 2015

So I figured out an ugly workaround to my issue. If you guys are interested.

Here it is

somethingReturningCustomOptions(function(err, customOptions) {
  Question.findOne({ id: req.params.id })
    .populate('responses')
    .populate('options')
    .exec(function(err, question) {
      var assocs = []
        , questionObj = {};

      for (var i = Question.associations.length - 1; i >= 0; i--) {
        var assoc = Question.associations[i];
        if (assoc.type !== "collection") continue;
        assocs.push(assoc.alias);
      }

      var questionKeys = Object.keys(question).filter(function(key) {
        return (assocs.indexOf(key) == -1);
      });

      // We create a new object that has all 
      // the properties minus the associations
      questionKeys.forEach(function(key) {
        questionObj[key] = question[key];
      });

      // Then we can manually add the associations
      questionObj.responses = question.responses;

      // If we have some customOptions then we override
      // the question.options property
      questionObj.options = (customOptions) ? customOptions : question.options;

      res.json(questionObj);
    });
});

@anasyb
Copy link

anasyb commented Apr 1, 2015

@m0g sorry you had to go as far as that... this is indeed caused by #891... in particular please read this comment https://github.com/balderdashy/waterline/issues/891#issuecomment-88361748

All you need is to stop waterline from being involved in your object. You can do so simply by:

var questionObj = question.toObject();
questionObj.toJSON = undefined;//this will stop waterline's `toJSON` from being called

//...now do whatever you want with your obj
//...
res.json(questionObj);

@dmarcelino
Copy link
Member

@m0g, @dantswain, @pixelfreak, #969 has been merged to master, can you check if it fixes your issues?

@dmarcelino
Copy link
Member

As @devinivy said back in #891:

Closing since this is allegedly fixed in master and the topic has become quiet! Anyone feel free to reopen if deemed necessary.

@raqem raqem transferred this issue from balderdashy/waterline May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests