From e543adb1c72e03a066a9448ba80125c1bf85aed7 Mon Sep 17 00:00:00 2001 From: Blaster4385 Date: Sat, 1 Oct 2022 13:29:20 +0530 Subject: [PATCH 1/4] Add missing return statement to CollectionPrototype._validatedInsert - This fixes the issue where CollectionPrototype._validatedInsert doesn't return the insertedId for newly inserted documents. Changed Files: - packages/allow-deny/allow-deny.js --- packages/allow-deny/allow-deny.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/allow-deny/allow-deny.js b/packages/allow-deny/allow-deny.js index e1bff9f9ab..617c4a44f3 100644 --- a/packages/allow-deny/allow-deny.js +++ b/packages/allow-deny/allow-deny.js @@ -260,7 +260,7 @@ CollectionPrototype._validatedInsert = function (userId, doc, if (generatedId !== null) doc._id = generatedId; - self._collection.insert.call(self._collection, doc); + return self._collection.insert.call(self._collection, doc); }; // Simulate a mongo `update` operation while validating that the access From 5bba84423525315e34b6147674727dd5f77c4829 Mon Sep 17 00:00:00 2001 From: sanki92 Date: Fri, 3 Oct 2025 20:25:36 +0530 Subject: [PATCH 2/4] Add tests for _validatedInsert return value Tests verify that CollectionPrototype._validatedInsert correctly returns the inserted document ID, making it consistent with _validatedUpdate and _validatedRemove. Addresses feedback on PR #12200 regarding missing tests. --- packages/mongo/mongo_livedata_tests.js | 56 ++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/packages/mongo/mongo_livedata_tests.js b/packages/mongo/mongo_livedata_tests.js index 3224b6d3d6..8a1e3e4c4f 100644 --- a/packages/mongo/mongo_livedata_tests.js +++ b/packages/mongo/mongo_livedata_tests.js @@ -3555,3 +3555,59 @@ if (Meteor.isServer) { }); }); } + +// Test for CollectionPrototype._validatedInsert returning the inserted ID +if (Meteor.isServer) { + Tinytest.add("mongo-livedata - _validatedInsert returns insertedId", function (test) { + const run = test.runId(); + const coll = new Mongo.Collection("validatedInsert_test_" + run); + + // Set up allow rule to enable validation + coll.allow({ + insert: function () { return true; } + }); + + // Test document + const testDoc = { name: "test document", value: 42 }; + + // Call _validatedInsert directly and check return value + const result = coll._validatedInsert(null, testDoc, null); + + // Should return the inserted ID + test.isTrue(result, "_validatedInsert should return a truthy value (the inserted ID)"); + test.equal(typeof result, "string", "_validatedInsert should return a string ID"); + + // Verify the document was actually inserted with the returned ID + const insertedDoc = coll.findOne(result); + test.isTrue(insertedDoc, "Document should be found with the returned ID"); + test.equal(insertedDoc.name, testDoc.name, "Inserted document should have correct name"); + test.equal(insertedDoc.value, testDoc.value, "Inserted document should have correct value"); + test.equal(insertedDoc._id, result, "Document _id should match returned ID"); + }); + + Tinytest.add("mongo-livedata - _validatedInsert with generated ID", function (test) { + const run = test.runId(); + const coll = new Mongo.Collection("validatedInsertGenId_test_" + run); + + // Set up allow rule to enable validation + coll.allow({ + insert: function () { return true; } + }); + + // Test document without ID (will generate one) + const testDoc = { name: "test document with generated ID", value: 123 }; + const generatedId = coll._makeNewID(); + + // Call _validatedInsert with a generated ID + const result = coll._validatedInsert(null, testDoc, generatedId); + + // Should return the generated ID + test.equal(result, generatedId, "_validatedInsert should return the generated ID"); + + // Verify the document was inserted with the generated ID + const insertedDoc = coll.findOne(generatedId); + test.isTrue(insertedDoc, "Document should be found with the generated ID"); + test.equal(insertedDoc._id, generatedId, "Document _id should be the generated ID"); + test.equal(insertedDoc.name, testDoc.name, "Inserted document should have correct name"); + }); +} From 9eac9f33c3a57175e11070530efc9bf93ce2e3e4 Mon Sep 17 00:00:00 2001 From: shanky Date: Sat, 4 Oct 2025 23:02:43 +0530 Subject: [PATCH 3/4] test: fix tests for _validatedInsertAsync return value - Update tests to use named collections instead of anonymous collections - Add allow rules to trigger validated insert code path - Use insertAsync (public API) instead of calling internal _validatedInsertAsync - Add server-only guard since allow/deny only applies on server - Add proper cleanup with dropCollectionAsync() - Tests verify both auto-generated IDs and explicit IDs - Tests run for both STRING and MONGO ID generation modes This addresses radekmie's comment about missing code changes. The actual fix is already present in the codebase at line 315 of packages/allow-deny/allow-deny.js where _validatedInsertAsync returns the result of insertAsync. Fixes: #12159 --- packages/mongo/tests/mongo_livedata_tests.js | 83 ++++++++++++-------- 1 file changed, 52 insertions(+), 31 deletions(-) diff --git a/packages/mongo/tests/mongo_livedata_tests.js b/packages/mongo/tests/mongo_livedata_tests.js index 7b9b5af149..136d84f22a 100644 --- a/packages/mongo/tests/mongo_livedata_tests.js +++ b/packages/mongo/tests/mongo_livedata_tests.js @@ -2776,38 +2776,59 @@ const setsEqual = function (a, b) { }); }); - // Test for CollectionPrototype._validatedInsert return value (issue #12159) - testAsyncMulti('mongo-livedata - _validatedInsert returns inserted id, ' + idGeneration, [ - async function(test, expect) { - var coll = new Mongo.Collection(null, { idGeneration: idGeneration }); - - // Test that _validatedInsert returns the inserted ID - var result = await coll._validatedInsert({name: "test"}); - test.isNotNull(result); - test.isNotUndefined(result); - - // Verify the document was actually inserted and has the returned ID - var doc = await coll.findOneAsync({name: "test"}); - test.isNotNull(doc); - test.equal(doc._id, result); - } - ]); + // Test for CollectionPrototype._validatedInsertAsync return value (issue #12159) + // This test verifies that insertAsync returns the inserted ID when using allow/deny rules + if (Meteor.isServer) { + testAsyncMulti('mongo-livedata - insertAsync with allow/deny returns inserted id, ' + idGeneration, [ + async function(test, expect) { + var collectionName = 'test_validated_insert_' + Random.id(); + var coll = new Mongo.Collection(collectionName, { idGeneration: idGeneration }); + + // Set up allow rule to test _validatedInsertAsync return value + coll.allow({ + insert: function() { return true; } + }); + + // Test that insertAsync (which uses _validatedInsertAsync internally) returns the inserted ID + var result = await coll.insertAsync({name: "test"}); + test.isNotNull(result); + test.isNotUndefined(result); + + // Verify the document was actually inserted and has the returned ID + var doc = await coll.findOneAsync({name: "test"}); + test.isNotNull(doc); + test.equal(doc._id, result); + + // Clean up + await coll.dropCollectionAsync(); + } + ]); - testAsyncMulti('mongo-livedata - _validatedInsert returns generated id when explicit id provided, ' + idGeneration, [ - async function(test, expect) { - var coll = new Mongo.Collection(null, { idGeneration: idGeneration }); - - // Test with explicit ID - var explicitId = idGeneration === 'MONGO' ? new Mongo.ObjectID() : 'explicit-test-id'; - var result = await coll._validatedInsert({_id: explicitId, name: "explicit-test"}); - test.equal(result, explicitId); - - // Verify the document was inserted with the explicit ID - var doc = await coll.findOneAsync({name: "explicit-test"}); - test.isNotNull(doc); - test.equal(doc._id, explicitId); - } - ]); + testAsyncMulti('mongo-livedata - insertAsync with allow/deny returns explicit id, ' + idGeneration, [ + async function(test, expect) { + var collectionName = 'test_validated_insert_explicit_' + Random.id(); + var coll = new Mongo.Collection(collectionName, { idGeneration: idGeneration }); + + // Set up allow rule + coll.allow({ + insert: function() { return true; } + }); + + // Test with explicit ID + var explicitId = idGeneration === 'MONGO' ? new Mongo.ObjectID() : 'explicit-test-id'; + var result = await coll.insertAsync({_id: explicitId, name: "explicit-test"}); + test.equal(result, explicitId); + + // Verify the document was inserted with the explicit ID + var doc = await coll.findOneAsync({name: "explicit-test"}); + test.isNotNull(doc); + test.equal(doc._id, explicitId); + + // Clean up + await coll.dropCollectionAsync(); + } + ]); + } }); // end idGeneration parametrization From fcd3de3d8c35702bbcdf386e3078af51db42f342 Mon Sep 17 00:00:00 2001 From: shanky Date: Wed, 8 Oct 2025 21:38:35 +0530 Subject: [PATCH 4/4] test: expand coverage to all operations with allow/deny Follow pattern from #13891 for comprehensive operation testing --- packages/mongo/tests/mongo_livedata_tests.js | 63 +++++++++----------- 1 file changed, 28 insertions(+), 35 deletions(-) diff --git a/packages/mongo/tests/mongo_livedata_tests.js b/packages/mongo/tests/mongo_livedata_tests.js index 136d84f22a..f3e32106a5 100644 --- a/packages/mongo/tests/mongo_livedata_tests.js +++ b/packages/mongo/tests/mongo_livedata_tests.js @@ -2776,53 +2776,46 @@ const setsEqual = function (a, b) { }); }); - // Test for CollectionPrototype._validatedInsertAsync return value (issue #12159) - // This test verifies that insertAsync returns the inserted ID when using allow/deny rules + // Test operation result fields with allow/deny rules (similar to issue #12159) if (Meteor.isServer) { - testAsyncMulti('mongo-livedata - insertAsync with allow/deny returns inserted id, ' + idGeneration, [ + testAsyncMulti('mongo-livedata - operation result fields with allow/deny, ' + idGeneration, [ async function(test, expect) { - var collectionName = 'test_validated_insert_' + Random.id(); + var collectionName = 'test_operation_results_' + Random.id(); var coll = new Mongo.Collection(collectionName, { idGeneration: idGeneration }); - // Set up allow rule to test _validatedInsertAsync return value + // Set up allow rules for all operations coll.allow({ - insert: function() { return true; } + insert: function() { return true; }, + update: function() { return true; }, + remove: function() { return true; } }); - // Test that insertAsync (which uses _validatedInsertAsync internally) returns the inserted ID - var result = await coll.insertAsync({name: "test"}); - test.isNotNull(result); - test.isNotUndefined(result); + // Test insert + var insertedId = await coll.insertAsync({name: 'doc1'}); + test.isTrue(insertedId !== undefined, 'insert should return an ID'); - // Verify the document was actually inserted and has the returned ID - var doc = await coll.findOneAsync({name: "test"}); - test.isNotNull(doc); - test.equal(doc._id, result); + // Test update + var updateResult = await coll.updateAsync({name: 'doc1'}, {$set: {value: 1}}); + test.equal(updateResult, 1, 'update should return affected count'); - // Clean up - await coll.dropCollectionAsync(); - } - ]); - - testAsyncMulti('mongo-livedata - insertAsync with allow/deny returns explicit id, ' + idGeneration, [ - async function(test, expect) { - var collectionName = 'test_validated_insert_explicit_' + Random.id(); - var coll = new Mongo.Collection(collectionName, { idGeneration: idGeneration }); + // Test upsert (update case) + var upsertUpdateResult = await coll.upsertAsync({name: 'doc1'}, {$set: {value: 2}}); + test.equal(upsertUpdateResult.numberAffected, 1); + test.isFalse(upsertUpdateResult.hasOwnProperty('insertedId')); - // Set up allow rule - coll.allow({ - insert: function() { return true; } - }); + // Test upsert (insert case) + var upsertInsertResult = await coll.upsertAsync({name: 'doc2'}, {$set: {value: 3}}); + test.equal(upsertInsertResult.numberAffected, 1); + test.isTrue(upsertInsertResult.hasOwnProperty('insertedId')); - // Test with explicit ID + // Test remove + var removeResult = await coll.removeAsync({name: 'doc1'}); + test.equal(removeResult, 1, 'remove should return removed count'); + + // Test insert with explicit ID var explicitId = idGeneration === 'MONGO' ? new Mongo.ObjectID() : 'explicit-test-id'; - var result = await coll.insertAsync({_id: explicitId, name: "explicit-test"}); - test.equal(result, explicitId); - - // Verify the document was inserted with the explicit ID - var doc = await coll.findOneAsync({name: "explicit-test"}); - test.isNotNull(doc); - test.equal(doc._id, explicitId); + var insertExplicitResult = await coll.insertAsync({_id: explicitId, name: 'explicit-doc'}); + test.equal(insertExplicitResult, explicitId, 'insert with explicit ID should return that ID'); // Clean up await coll.dropCollectionAsync();