From 4e4c7367a05011753853bb465616539a4467a7db Mon Sep 17 00:00:00 2001 From: Kelvin Oghenerhoro Omereshone Date: Fri, 11 Apr 2025 15:00:19 +0100 Subject: [PATCH 1/5] feat: improve forgeStageTwoQuery to allow select/omit as valid subcriteria but not both --- .../utils/query/forge-stage-two-query.js | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/lib/waterline/utils/query/forge-stage-two-query.js b/lib/waterline/utils/query/forge-stage-two-query.js index 6fd6ea1f6..4a59b41a3 100644 --- a/lib/waterline/utils/query/forge-stage-two-query.js +++ b/lib/waterline/utils/query/forge-stage-two-query.js @@ -873,10 +873,14 @@ module.exports = function forgeStageTwoQuery(query, orm) { if (_.isEqual(query.populates[populateAttrName], {})) { query.populates[populateAttrName] = true; } - // Otherwise, this simply must be `true`. Otherwise it's invalid. + // Otherwise, this must either be `true` or a valid subcriteria containing only `omit` or `select`. else { - - if (query.populates[populateAttrName] !== true) { + // Only allow a plain dictionary with just `select` and/or `omit`. + // Anything else isn't valid for a singular ("model") association. + if (query.populates[populateAttrName] !== true && !( + _.isPlainObject(query.populates[populateAttrName]) && + _.difference(Object.keys(query.populates[populateAttrName]), ['select', 'omit']).length === 0 + )) { throw buildUsageError( 'E_INVALID_POPULATES', 'Could not populate `'+populateAttrName+'`. '+ @@ -886,14 +890,30 @@ module.exports = function forgeStageTwoQuery(query, orm) { 'since it generally wouldn\'t make any sense. But that\'s the trouble-- it '+ 'looks like some sort of a subcriteria (or something) _was_ provided!\n'+ '(Note that subcriterias consisting ONLY of `omit` or `select` are a special '+ - 'case that _does_ make sense. This usage will be supported in a future version '+ - 'of Waterline.)\n'+ + 'case that _does_ make sense.\n'+ + '\n'+ + 'Here\'s what was passed in:\n'+ + util.inspect(query.populates[populateAttrName], {depth: 5}), + query.using + ); + }//-• + + // Ensure that only `select` or `omit` is provided, but not both. + if (_.isPlainObject(query.populates[populateAttrName]) && + query.populates[populateAttrName].select && + query.populates[populateAttrName].omit) { + throw buildUsageError( + 'E_INVALID_POPULATES', + 'Could not populate `'+populateAttrName+'`. '+ + 'This is a singular ("model") association, and while subcriterias consisting '+ + 'ONLY of `omit` or `select` are supported, providing both `select` AND `omit` '+ + 'at the same time is not allowed.\n'+ '\n'+ 'Here\'s what was passed in:\n'+ util.inspect(query.populates[populateAttrName], {depth: 5}), query.using ); - }//-• + }//-• }//>-• From 31f15b573d4afdc037e601534c93c5ad1022d95f Mon Sep 17 00:00:00 2001 From: Kelvin Oghenerhoro Omereshone Date: Fri, 11 Apr 2025 15:08:34 +0100 Subject: [PATCH 2/5] chore: fix code formatting --- .../utils/query/forge-stage-two-query.js | 57 +++++++++---------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/lib/waterline/utils/query/forge-stage-two-query.js b/lib/waterline/utils/query/forge-stage-two-query.js index 4a59b41a3..e48ecfde6 100644 --- a/lib/waterline/utils/query/forge-stage-two-query.js +++ b/lib/waterline/utils/query/forge-stage-two-query.js @@ -877,44 +877,43 @@ module.exports = function forgeStageTwoQuery(query, orm) { else { // Only allow a plain dictionary with just `select` and/or `omit`. // Anything else isn't valid for a singular ("model") association. - if (query.populates[populateAttrName] !== true && !( + if (query.populates[populateAttrName] !== true && !( _.isPlainObject(query.populates[populateAttrName]) && _.difference(Object.keys(query.populates[populateAttrName]), ['select', 'omit']).length === 0 - )) { + )) { throw buildUsageError( - 'E_INVALID_POPULATES', - 'Could not populate `'+populateAttrName+'`. '+ - 'This is a singular ("model") association, which means it never refers to '+ - 'more than _one_ associated record. So passing in subcriteria (i.e. as '+ - 'the second argument to `.populate()`) is not supported for this association, '+ - 'since it generally wouldn\'t make any sense. But that\'s the trouble-- it '+ - 'looks like some sort of a subcriteria (or something) _was_ provided!\n'+ - '(Note that subcriterias consisting ONLY of `omit` or `select` are a special '+ - 'case that _does_ make sense.\n'+ - '\n'+ - 'Here\'s what was passed in:\n'+ - util.inspect(query.populates[populateAttrName], {depth: 5}), - query.using + 'E_INVALID_POPULATES', + 'Could not populate `'+populateAttrName+'`. '+ + 'This is a singular ("model") association, which means it never refers to '+ + 'more than _one_ associated record. So passing in subcriteria (i.e. as '+ + 'the second argument to `.populate()`) is not supported for this association, '+ + 'since it generally wouldn\'t make any sense. But that\'s the trouble-- it '+ + 'looks like some sort of a subcriteria (or something) _was_ provided!\n'+ + '(Note that subcriterias consisting ONLY of `omit` or `select` are a special '+ + 'case that _does_ make sense.\n'+ + '\n'+ + 'Here\'s what was passed in:\n'+ + util.inspect(query.populates[populateAttrName], {depth: 5}), + query.using ); - }//-• + }//-• - // Ensure that only `select` or `omit` is provided, but not both. - if (_.isPlainObject(query.populates[populateAttrName]) && + // Ensure that only `select` or `omit` is provided, but not both. + if (_.isPlainObject(query.populates[populateAttrName]) && query.populates[populateAttrName].select && query.populates[populateAttrName].omit) { throw buildUsageError( - 'E_INVALID_POPULATES', - 'Could not populate `'+populateAttrName+'`. '+ - 'This is a singular ("model") association, and while subcriterias consisting '+ - 'ONLY of `omit` or `select` are supported, providing both `select` AND `omit` '+ - 'at the same time is not allowed.\n'+ - '\n'+ - 'Here\'s what was passed in:\n'+ - util.inspect(query.populates[populateAttrName], {depth: 5}), - query.using + 'E_INVALID_POPULATES', + 'Could not populate `'+populateAttrName+'`. '+ + 'This is a singular ("model") association, and while subcriterias consisting '+ + 'ONLY of `omit` or `select` are supported, providing both `select` AND `omit` '+ + 'at the same time is not allowed.\n'+ + '\n'+ + 'Here\'s what was passed in:\n'+ + util.inspect(query.populates[populateAttrName], {depth: 5}), + query.using ); - }//-• - + }//-• }//>-• } From e51a2363ad793512596e37d852a7247f1b28d6dc Mon Sep 17 00:00:00 2001 From: Kelvin Oghenerhoro Omereshone Date: Thu, 18 Sep 2025 00:00:54 +0100 Subject: [PATCH 3/5] feat: add tests for populate with select and omit subcriteria for singular associations --- test/unit/query/associations/populateArray.js | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/unit/query/associations/populateArray.js b/test/unit/query/associations/populateArray.js index 28a079993..1461408dc 100644 --- a/test/unit/query/associations/populateArray.js +++ b/test/unit/query/associations/populateArray.js @@ -123,5 +123,49 @@ describe('Collection Query ::', function() { return done(); }); }); + + it('should allow populate with select subcriteria for singular associations', function(done) { + Car.find() + .populate('driver', { + select: ['name'] + }) + .exec(function(err, car) { + if (err) { + return done(err); + } + + assert(car[0].driver); + return done(); + }); + }); + + it('should allow populate with omit subcriteria for singular associations', function(done) { + Car.find() + .populate('driver', { + omit: ['id'] + }) + .exec(function(err, car) { + if (err) { + return done(err); + } + + assert(car[0].driver); + return done(); + }); + }); + + it('should throw error when both select and omit are provided together', function(done) { + Car.find() + .populate('driver', { + select: ['name'], + omit: ['id'] + }) + .exec(function(err, car) { + assert(err); + assert.equal(err.code, 'E_INVALID_POPULATES'); + assert(err.message.indexOf('providing both `select` AND `omit`') > -1); + return done(); + }); + }); }); }); From 372db72d50e3edc7ff2a330e5fe213b3ad3092a8 Mon Sep 17 00:00:00 2001 From: Kelvin Oghenerhoro Omereshone Date: Thu, 18 Sep 2025 22:05:01 +0100 Subject: [PATCH 4/5] test: add tests for populate select/omit functionality - Add test for populate with select subcriteria for singular associations - Add test for populate with omit subcriteria for singular associations - Add test for populate with empty object (converts to true) - Add test for error when both select and omit are provided together These tests validate the new functionality that allows select/omit subcriteria for singular (model) associations while ensuring proper validation that prevents using both select and omit simultaneously. --- test/unit/query/associations/populateArray.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/unit/query/associations/populateArray.js b/test/unit/query/associations/populateArray.js index 1461408dc..736aa9496 100644 --- a/test/unit/query/associations/populateArray.js +++ b/test/unit/query/associations/populateArray.js @@ -135,6 +135,7 @@ describe('Collection Query ::', function() { } assert(car[0].driver); + assert(car[0].driver.name); return done(); }); }); @@ -150,6 +151,21 @@ describe('Collection Query ::', function() { } assert(car[0].driver); + assert(car[0].driver.name); + return done(); + }); + }); + + it('should allow populate with empty object (converts to true)', function(done) { + Car.find() + .populate('driver', {}) + .exec(function(err, car) { + if (err) { + return done(err); + } + + assert(car[0].driver); + assert(car[0].driver.name); return done(); }); }); From 414c927590893a5eefa632f92cbed1f0b3d75d42 Mon Sep 17 00:00:00 2001 From: Kelvin Oghenerhoro Omereshone Date: Thu, 18 Sep 2025 22:09:57 +0100 Subject: [PATCH 5/5] refactor: improve populate select/omit test assertions - Change omit test to use 'car' field instead of 'id' (more realistic) - Focus tests on validation logic rather than actual field filtering - Ensure tests validate that select/omit subcriteria are accepted without throwing validation errors The tests verify the core functionality works while being more realistic about what fields can typically be omitted in an ORM. --- test/unit/query/associations/populateArray.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/query/associations/populateArray.js b/test/unit/query/associations/populateArray.js index 736aa9496..b8c77239a 100644 --- a/test/unit/query/associations/populateArray.js +++ b/test/unit/query/associations/populateArray.js @@ -143,7 +143,7 @@ describe('Collection Query ::', function() { it('should allow populate with omit subcriteria for singular associations', function(done) { Car.find() .populate('driver', { - omit: ['id'] + omit: ['car'] }) .exec(function(err, car) { if (err) {