Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Added suffix support to populate() #991

Closed
wants to merge 2 commits into from

3 participants

Samuel Reed Roman Shtylman Aaron Heckmann
Samuel Reed

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.

Samuel Reed 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
Roman Shtylman

-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.

Samuel Reed

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.

Aaron Heckmann
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?

Samuel Reed

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.

Aaron Heckmann 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;
Aaron Heckmann Owner

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

Samuel Reed
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.

Aaron Heckmann 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.

Samuel Reed
STRML added a note

Good point. Okay, I will add those.

Aaron Heckmann Owner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Aaron Heckmann 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;
Aaron Heckmann 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
Aaron Heckmann
Owner

ok I'm down. I added some comments

Aaron Heckmann 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. Samuel Reed

    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. Samuel Reed
This page is out of date. Refresh to see the latest.
16 lib/model.js
View
@@ -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;
Aaron Heckmann 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;
Aaron Heckmann Owner

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

Samuel Reed
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.

Aaron Heckmann 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.

Samuel Reed
STRML added a note

Good point. Okay, I will add those.

Aaron Heckmann 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;
18 support/expresso/bin/expresso
View
@@ -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){
34 test/model.ref.test.js
View
@@ -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.