From df95f1e91473b95c017145c71eb67fd033e3ae5f Mon Sep 17 00:00:00 2001 From: Slava Kim Date: Tue, 3 Dec 2013 13:39:50 -0800 Subject: [PATCH 1/2] Properly handle projections where '_id' is the only rule. + Tests. Fixes #1651 --- packages/minimongo/minimongo.js | 9 ++++++++- packages/minimongo/minimongo_tests.js | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index 2ddb61b84d..46ce091b15 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -1099,7 +1099,14 @@ LocalCollection._compileProjection = function (fields) { // Find the non-_id keys (_id is handled specially because it is included unless // explicitly excluded). Sort the keys, so that our code to detect overlaps // like 'foo' and 'foo.bar' can assume that 'foo' comes first. - var fieldsKeys = _.reject(_.keys(fields).sort(), function (key) { return key === '_id'; }); + var fieldsKeys = _.keys(fields).sort(); + + // If there are other rules other than '_id', treat '_id' differently in a + // separate case. If '_id' is the only rule, use it to understand if it is + // including/excluding projection. + if (fieldsKeys.length > 0 && !(fieldsKeys.length === 1 && fieldsKeys[0] === '_id')) + fieldsKeys = _.reject(fieldsKeys, function (key) { return key === '_id'; }); + var including = null; // Unknown var projectionRulesTree = {}; // Tree represented as nested objects diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index 3a2d2260eb..a8948e0983 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -992,6 +992,30 @@ Tinytest.add("minimongo - projection_compiler", function (test) { "blacklist nested - path not found in doc"] ]); + testProjection({ _id: 1 }, [ + [{ _id: 42, x: 1, y: { z: "2" } }, + { _id: 42 }, + "_id whitelisted"], + [{ _id: 33 }, + { _id: 33 }, + "_id whitelisted, _id only"], + [{ x: 1 }, + {}, + "_id whitelisted, no _id"] + ]); + + testProjection({ _id: 0 }, [ + [{ _id: 42, x: 1, y: { z: "2" } }, + { x: 1, y: { z: "2" } }, + "_id blacklisted"], + [{ _id: 33 }, + {}, + "_id blacklisted, _id only"], + [{ x: 1 }, + { x: 1 }, + "_id blacklisted, no _id"] + ]); + test.throws(function () { testProjection({ 'inc': 1, 'excl': 0 }, [ [ { inc: 42, excl: 42 }, { inc: 42 }, "Can't combine incl/excl rules" ] From 6582a8860815a10af79cb888be96b58792f4c5e4 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 3 Dec 2013 11:52:16 -0800 Subject: [PATCH 2/2] Fix 0.6.6 regression in setting MAIL_URL Also: - ensure that AppConfig callbacks are always called at least once - don't start a new AppConfig Fiber every time you call Email.send Fixes #1649. --- packages/application-configuration/config.js | 14 +++---- packages/email/email.js | 42 +++++++++++--------- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/packages/application-configuration/config.js b/packages/application-configuration/config.js index f7054ab3ac..75ee285d8b 100644 --- a/packages/application-configuration/config.js +++ b/packages/application-configuration/config.js @@ -58,9 +58,6 @@ try { packages: { 'mongo-livedata': { url: process.env.MONGO_URL - }, - 'email': { - url: process.env.MAIL_URL } } }; @@ -83,10 +80,13 @@ AppConfig.getAppConfig = function () { AppConfig.configurePackage = function (packageName, configure) { var appConfig = AppConfig.getAppConfig(); // Will either be based in the env var, // or wait for galaxy to connect. - var lastConfig = appConfig && appConfig.packages && appConfig.packages[packageName]; - if (lastConfig) { - configure(lastConfig); - } + var lastConfig = + (appConfig && appConfig.packages && appConfig.packages[packageName]) || {}; + // Always call the configure callback "soon" even if the initial configuration + // is empty (synchronously, though deferred would be OK). + // XXX make sure that all callers of configurePackage deal well with multiple + // callback invocations! eg, email does not + configure(lastConfig); var configureIfDifferent = function (app) { if (!EJSON.equals(app.config && app.config.packages && app.config.packages[packageName], lastConfig)) { diff --git a/packages/email/email.js b/packages/email/email.js index 6490f4ea65..8fce809930 100644 --- a/packages/email/email.js +++ b/packages/email/email.js @@ -33,19 +33,26 @@ var makePool = function (mailUrlString) { // We construct smtpPool at the first call to Email.send, so that // Meteor.startup code can set $MAIL_URL. -var smtpPool = null; -var maybeMakePool = function () { - // We check MAIL_URL in case someone else set it in Meteor.startup code. - var poolFuture = new Future(); - AppConfig.configurePackage('email', function (config) { - // TODO: allow reconfiguration. - if (!smtpPool && (config.url || process.env.MAIL_URL)) { - smtpPool = makePool(config.url || process.env.MAIL_URL); - } - poolFuture.return(); - }); +var smtpPoolFuture = new Future;; +var configured = false; - poolFuture.wait(); +var getPool = function () { + // We check MAIL_URL in case someone else set it in Meteor.startup code. + if (!configured) { + configured = true; + AppConfig.configurePackage('email', function (config) { + // XXX allow reconfiguration when the app config changes + if (smtpPoolFuture.isResolved()) + return; + var url = config.url || process.env.MAIL_URL; + var pool = null; + if (url) + pool = makePool(url); + smtpPoolFuture.return(pool); + }); + } + + return smtpPoolFuture.wait(); }; var next_devmode_mail_id = 0; @@ -81,8 +88,8 @@ var devModeSend = function (mc) { future.wait(); }; -var smtpSend = function (mc) { - smtpPool._future_wrapped_sendMail(mc).wait(); +var smtpSend = function (pool, mc) { + pool._future_wrapped_sendMail(mc).wait(); }; /** @@ -141,10 +148,9 @@ Email.send = function (options) { mc.addHeader(name, value); }); - maybeMakePool(); - - if (smtpPool) { - smtpSend(mc); + var pool = getPool(); + if (pool) { + smtpSend(pool, mc); } else { devModeSend(mc); }