Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Added suffix support to populate() #991

Closed
wants to merge 2 commits into from

3 participants

@STRML

ExtJS and other frameworks don't play well with the populate() function in Mongoose. ExtJS expects a certain field to be populated with an association, or null if the object is not denormalized. The problem is that if I choose not to denormalize an object and send it down to ExtJS, its Model structure will attempt to treat the object ID as an actual Model and fail.

Call populate(path, fields, conditions, options) with options.suffix defined, and the populated object will go into a new field, key+suffix, instead of overwriting the key field.

For example, for compatibility with ExtJS4, we might want to send down an object in this manner to be friendly to ExtJS' association support:

User.findById("id").populate('comment', null, null, {suffix: '_obj'}).run(function(err, user){...});

Which yields:

User{
...
comment: "4f5cd5f6e9786ec113000006"
comment_obj: {text: "foo"}
...
}

ExtJS is then instructed that the associationKey is 'comment_obj'. This means it will correctly use the contents of 'comment_obj' to construct a model when it is present, but will not if it is not present.

@STRML STRML Added suffix support to populate().
Call populate(path, fields, conditions, options) with options.suffix
defined, and the populated object will go into a new field, key+suffix,
instead of overwriting the id field.

For example, for compatibility with ExtJS4, we might want to send down
an object in this manner to be friendly to ExtJS' association support:

User{
   comment: "4f5cd5f6e9786ec113000006"
   comment_obj: {text: "foo"}
}

Calling User.findOne().populate('comment', null, null, {suffix:
'_obj'}) will add this suffix.
e0fe98a
@defunctzombie

-1 for adding extjs specific functionality. I would think that the extjs side should be responsible for being able to setup associations on their end? Or just denormalize it yourself before sending.

@STRML

This is not strictly ExtJS specific, there are many reasons to want to denormalize an object into a field that is not the foreign key field. It would be nice to at least have that option.

@aheckmann
Owner

so far the only reason stated is better extjs compatibility. thats a respectable goal, are there any other use cases where this would help?

@STRML

I don't have any concrete examples as I have only used Mongoose with ExtJS, but any strongly-typed language or rigid framework receiving information from Mongoose is going to balk at the ambiguity created by populate(). Sometimes a model association key is a String, sometimes it is an Object. With a suffix, you can be sure that the association key is always a String or null, and the association key + "_obj" is always an Object or null.

@aheckmann aheckmann commented on the diff
lib/model.js
@@ -246,14 +249,15 @@ Model.prototype.init = function init (doc, query, fn) {
} else {
self._populate(schema, obj[i], poppath, function (err, doc) {
if (err) return error(err);
- obj[i] = doc;
+ if(suffix && doc !== null) obj[i + suffix] = doc;
+ else obj[i] = doc;
@aheckmann Owner

please remove the null check. should be consistent regardless of prefix

@STRML
STRML added a note

The reason the null check is here is so that it will make obj[i]=null if the doc is null. Not sure what the desired behavior would be in this situation. If I remove the null check obj[i+suffix]=null but obj[i] still contains the id.

In a sense the null check makes it consistent because the id is eliminated if it isn't valid, regardless of whether or not you've set a suffix.

@aheckmann Owner

the id isn't necessarily invalid. the user may have passed conditions to filter out data.

if there's a suffix just use it and leave the original alone. please add tests for both singe property population and population of arrays.

@STRML
STRML added a note

Good point. Okay, I will add those.

@aheckmann Owner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@aheckmann aheckmann commented on the diff
lib/model.js
@@ -234,7 +236,8 @@ Model.prototype.init = function init (doc, query, fn) {
self._populate(schema.schema.path(key), subobj[key], poppath.sub[key], done);
function done (err, doc) {
if (err) return error(err);
- subobj[key] = doc;
+ if(suffix && doc !== null) subobj[key + suffix] = doc;
@aheckmann Owner

please remove the null check. should be consistent regardless of prefix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@aheckmann
Owner

ok I'm down. I added some comments

@aheckmann aheckmann closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 30, 2012
  1. @STRML

    Added suffix support to populate().

    STRML authored
    Call populate(path, fields, conditions, options) with options.suffix
    defined, and the populated object will go into a new field, key+suffix,
    instead of overwriting the id field.
    
    For example, for compatibility with ExtJS4, we might want to send down
    an object in this manner to be friendly to ExtJS' association support:
    
    User{
       comment: "4f5cd5f6e9786ec113000006"
       comment_obj: {text: "foo"}
    }
    
    Calling User.findOne().populate('comment', null, null, {suffix:
    '_obj'}) will add this suffix.
Commits on Jul 9, 2012
  1. @STRML
This page is out of date. Refresh to see the latest.
View
16 lib/model.js
@@ -171,7 +171,7 @@ Model.prototype._populate = function populate (schema, oid, query, fn) {
*/
Model.prototype.init = function init (doc, query, fn) {
- if ('function' == typeof query) {
+ if ('function' === typeof query) {
fn = query;
query = null;
}
@@ -206,6 +206,7 @@ Model.prototype.init = function init (doc, query, fn) {
, schema = self.schema.path(path)
, total = 0
, poppath
+ , suffix;
if (!schema && obj[i] && 'Object' === obj[i].constructor.name) {
// assume nested object
@@ -218,12 +219,13 @@ Model.prototype.init = function init (doc, query, fn) {
// it to prevent query condition contamination between
// one populate call to the next.
poppath = utils.clone(populate[path]);
+ suffix = poppath.options.suffix || ""; // for populating objects into a separate field, like user_obj
if (poppath.sub) {
obj[i].forEach(function (subobj) {
var pkeys = Object.keys(poppath.sub)
, pi = pkeys.length
- , key
+ , key;
while (pi--) {
key = pkeys[pi];
@@ -234,7 +236,8 @@ Model.prototype.init = function init (doc, query, fn) {
self._populate(schema.schema.path(key), subobj[key], poppath.sub[key], done);
function done (err, doc) {
if (err) return error(err);
- subobj[key] = doc;
+ if(suffix && doc !== null) subobj[key + suffix] = doc;
@aheckmann Owner

please remove the null check. should be consistent regardless of prefix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ else subobj[key] = doc;
--total || next();
}
})(key);
@@ -246,14 +249,15 @@ Model.prototype.init = function init (doc, query, fn) {
} else {
self._populate(schema, obj[i], poppath, function (err, doc) {
if (err) return error(err);
- obj[i] = doc;
+ if(suffix && doc !== null) obj[i + suffix] = doc;
+ else obj[i] = doc;
@aheckmann Owner

please remove the null check. should be consistent regardless of prefix

@STRML
STRML added a note

The reason the null check is here is so that it will make obj[i]=null if the doc is null. Not sure what the desired behavior would be in this situation. If I remove the null check obj[i+suffix]=null but obj[i] still contains the id.

In a sense the null check makes it consistent because the id is eliminated if it isn't valid, regardless of whether or not you've set a suffix.

@aheckmann Owner

the id isn't necessarily invalid. the user may have passed conditions to filter out data.

if there's a suffix just use it and leave the original alone. please add tests for both singe property population and population of arrays.

@STRML
STRML added a note

Good point. Okay, I will add those.

@aheckmann Owner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
next();
});
}
- };
+ }
next();
- };
+ }
function error (err) {
if (error.err) return;
View
18 support/expresso/bin/expresso
@@ -5,7 +5,7 @@
* Copyright(c) TJ Holowaychuk <tj@vision-media.ca>
* (MIT Licensed)
*/
-
+
/**
* Module dependencies.
*/
@@ -14,7 +14,7 @@ var assert = require('assert'),
childProcess = require('child_process'),
http = require('http'),
path = require('path'),
- sys = require('sys'),
+ sys = require('util'),
cwd = process.cwd(),
fs = require('fs'),
defer;
@@ -41,13 +41,13 @@ var testcount = 0;
/**
* Whitelist of tests to run.
*/
-
+
var only = [];
/**
* Boring output.
*/
-
+
var boring = false;
/**
@@ -536,7 +536,7 @@ function reportCoverage(cov) {
if (name.match(/\.js$/)) {
var file = cov[name];
if ((file.coverage < 100) || !quiet) {
- print('\n [bold]{' + name + '}:');
+ print('\n [bold]{' + name + '}:');
print(file.source);
sys.print('\n');
}
@@ -551,11 +551,11 @@ function reportCoverage(cov) {
*/
function populateCoverage(cov) {
- cov.LOC =
+ cov.LOC =
cov.SLOC =
cov.totalFiles =
cov.totalHits =
- cov.totalMisses =
+ cov.totalMisses =
cov.coverage = 0;
for (var name in cov) {
var file = cov[name];
@@ -601,7 +601,7 @@ function coverage(data, val) {
for (var i = 0, len = data.length; i < len; ++i) {
if (data[i] !== undefined && data[i] == val) ++n;
}
- return n;
+ return n;
}
/**
@@ -764,7 +764,7 @@ function runSuite(title, tests, fn) {
clearTimeout(id);
next();
});
- }
+ }
});
} else {
test(function(fn){
View
34 test/model.ref.test.js
@@ -1441,7 +1441,7 @@ module.exports = {
});
});
},
-
+
// gh-675
'toJSON should also be called for refs': function () {
var db = start()
@@ -1524,5 +1524,37 @@ module.exports = {
});
});
})
+ },
+
+ 'test populating with suffixes': function () {
+ var db = start()
+ , BlogPost = db.model('RefBlogPost', posts)
+ , User = db.model('RefUser', users)
+
+ User.create({
+ name : 'Guillermo'
+ , email : 'rauchg@gmail.com'
+ }, function (err, creator) {
+ should.strictEqual(err, null);
+
+ BlogPost.create({
+ title : 'woot'
+ , _creator : creator
+ }, function (err, post) {
+ should.strictEqual(err, null);
+
+ BlogPost
+ .findById(post._id)
+ .populate('_creator', null, null, {suffix: '_obj'})
+ .run(function (err, post) {
+ db.close();
+ should.strictEqual(err, null);
+ post._creator.should.be.an.instanceof(DocObjectId);
+ post._creator_obj.should.be.an.instanceof(User);
+ post._creator_obj.name.should.equal('Guillermo');
+ post._creator_obj.email.should.equal('rauchg@gmail.com');
+ });
+ });
+ });
}
};
Something went wrong with that request. Please try again.