From a426118dbe746157891b622d73491d7707133dac Mon Sep 17 00:00:00 2001 From: Arek W Date: Tue, 27 Jun 2023 11:30:51 +0200 Subject: [PATCH] Add rows to hasMany join table in order provided Previously, they were added in reverse order --- Changelog.md | 3 ++ lib/Associations/Many.js | 40 ++++++++++--------------- lib/Associations/One.js | 34 +++++++-------------- lib/Hook.js | 40 ++++++++++++------------- package-lock.json | 2 +- package.json | 2 +- test/integration/association-hasmany.js | 18 +++++++++++ 7 files changed, 70 insertions(+), 69 deletions(-) diff --git a/Changelog.md b/Changelog.md index bb906174..d0bab257 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,6 @@ +### v8.0.0 +- [POSSIBLY BREAKING] Add rows to hasMany join table in order provided (previously, they were added in reverse order) ([862](../../pull/862)) + ### v7.1.1 - Resolve most development-only security vulnerabilities (no production ones present) ([861](../../pull/861)) - Don't run test on node 10 & 12 - dev deps don't work diff --git a/lib/Associations/Many.js b/lib/Associations/Many.js index 684dbae2..265f4ef5 100644 --- a/lib/Associations/Many.js +++ b/lib/Associations/Many.js @@ -1,4 +1,5 @@ var _ = require("lodash"); +var async = require("async"); var Hook = require("../Hook"); var Settings = require("../Settings"); var Property = require("../Property"); @@ -369,17 +370,13 @@ function extendInstance(Model, Instance, Driver, association, opts, createInstan value: function () { var Associations = []; var opts = {}; - var cb = noOperation; + var next = noOperation; var run = function () { - var savedAssociations = []; - var saveNextAssociation = function () { - if (Associations.length === 0) { - return cb(null, savedAssociations); - } + var saveAssociation = function (Association, cb) { + const hookOpts = Object.keys(association.props).length > 0 ? opts : undefined; - var Association = Associations.pop(); - var saveAssociation = function (err) { + Hook.wait(Association, association.hooks.beforeSave, function (err) { if (err) { return cb(err); } @@ -405,9 +402,7 @@ function extendInstance(Model, Instance, Driver, association, opts, createInstan return cb(err); } - savedAssociations.push(Association); - - return saveNextAssociation(); + return cb(); }); } @@ -419,27 +414,24 @@ function extendInstance(Model, Instance, Driver, association, opts, createInstan return cb(err); } - savedAssociations.push(Association); - - return saveNextAssociation(); + return cb(); }); }); - }; - - if (Object.keys(association.props).length) { - Hook.wait(Association, association.hooks.beforeSave, saveAssociation, opts); - } else { - Hook.wait(Association, association.hooks.beforeSave, saveAssociation); - } + }, hookOpts); }; - return saveNextAssociation(); + async.eachSeries(Associations, saveAssociation, function (err) { + if (err) { + return next(err); + } + next(null, Associations); + }); }; for (var i = 0; i < arguments.length; i++) { switch (typeof arguments[i]) { case "function": - cb = arguments[i]; + next = arguments[i]; break; case "object": if (Array.isArray(arguments[i])) { @@ -462,7 +454,7 @@ function extendInstance(Model, Instance, Driver, association, opts, createInstan } else { this.save(function (err) { if (err) { - return cb(err); + return next(err); } return run(); diff --git a/lib/Associations/One.js b/lib/Associations/One.js index e9a3d3f8..5f08eacc 100644 --- a/lib/Associations/One.js +++ b/lib/Associations/One.js @@ -1,4 +1,5 @@ var _ = require("lodash"); +var async = require("async"); var util = require("../Utilities"); var ORMError = require("../Error"); var Accessors = { "get": "get", "set": "set", "has": "has", "del": "remove" }; @@ -214,52 +215,39 @@ function extendInstance(Model, Instance, Driver, association) { writable: true }); Object.defineProperty(Instance, association.setAccessor, { - value: function (OtherInstance, cb) { + value: function (OtherInstance, next) { if (association.reversed) { Instance.save(function (err) { if (err) { - return cb(err); + return next(err); } if (!Array.isArray(OtherInstance)) { util.populateConditions(Model, Object.keys(association.field), Instance, OtherInstance, true); - return OtherInstance.save({}, { saveAssociations: false }, cb); + return OtherInstance.save({}, { saveAssociations: false }, next); } - var associations = _.clone(OtherInstance); - - var saveNext = function () { - if (!associations.length) { - return cb(); - } - - var other = associations.pop(); + var saveAssociation = function (otherInstance, cb) { + util.populateConditions(Model, Object.keys(association.field), Instance, otherInstance, true); - util.populateConditions(Model, Object.keys(association.field), Instance, other, true); - - other.save({}, { saveAssociations: false }, function (err) { - if (err) { - return cb(err); - } - - saveNext(); - }); + otherInstance.save({}, { saveAssociations: false }, cb); }; - return saveNext(); + var associations = _.clone(OtherInstance); + async.eachSeries(associations, saveAssociation, next); }); } else { OtherInstance.save({}, { saveAssociations: false }, function (err) { if (err) { - return cb(err); + return next(err); } Instance[association.name] = OtherInstance; util.populateConditions(association.model, Object.keys(association.field), OtherInstance, Instance); - return Instance.save({}, { saveAssociations: false }, cb); + return Instance.save({}, { saveAssociations: false }, next); }); } diff --git a/lib/Hook.js b/lib/Hook.js index 22285460..3466305b 100644 --- a/lib/Hook.js +++ b/lib/Hook.js @@ -8,30 +8,30 @@ exports.trigger = function () { } }; -exports.wait = function () { - var args = Array.prototype.slice.apply(arguments); - var self = args.shift(); - var hook = args.shift(); - var next = args.shift(); +exports.wait = function (self, hook, next, opts) { + if (typeof hook !== "function") { + return next(); + } - args.push(next); - if (typeof hook === "function") { - var hookValue = hook.apply(self, args); + var hookDoesntExpectCallback = hook.length < 1; + var hookValue; + if (opts) { + hookValue = hook.call(self, opts, next); + hookDoesntExpectCallback = hook.length < 2; + } else { + hookValue = hook.call(self, next); + } - var hookDoesntExpectCallback = hook.length < args.length; - var isPromise = hookValue && typeof(hookValue.then) === "function"; + var isPromise = hookValue && typeof(hookValue.then) === "function"; - if (hookDoesntExpectCallback) { - if (isPromise) { - return hookValue - .then(function () { - next(); - }) - .catch(next); - } - return next(); + if (hookDoesntExpectCallback) { + if (isPromise) { + return hookValue + .then(function () { + next(); + }) + .catch(next); } - } else { return next(); } }; diff --git a/package-lock.json b/package-lock.json index b3d22717..987051f2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "orm", - "version": "7.1.1", + "version": "8.0.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 83525649..6ef4c8bd 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "sqlite", "mongodb" ], - "version": "7.1.1", + "version": "8.0.0", "license": "MIT", "homepage": "http://dresende.github.io/node-orm2", "repository": "http://github.com/dresende/node-orm2.git", diff --git a/test/integration/association-hasmany.js b/test/integration/association-hasmany.js index 3170b07e..6d870e20 100644 --- a/test/integration/association-hasmany.js +++ b/test/integration/association-hasmany.js @@ -513,6 +513,24 @@ describe("hasMany", function () { }); }); + it("should add rows to join tables in order provided", async function () { + const pets = await Pet.createAsync([{ name: 'Fluffy' }, { name: 'Quacky' }, { name: 'Horsey' }]); + const Justin = await Person.oneAsync({ name: "Justin" }); + const justinsPetsBefore = await Justin.getPetsAsync(); + await Justin.addPetsAsync(pets); + const justinsPetsAfter = await Justin.getPetsAsync(); + + should.equal(justinsPetsBefore.length, justinsPetsAfter.length - 3); + + const joinRows = await db.driver.execQueryAsync("SELECT * FROM person_pets"); + + should.deepEqual( + // Mysql returns array of RowDataPacket. Concert to plain object. + joinRows.slice(-pets.length).map((item) => Object.assign({}, item)), + pets.map(function (pet) { return { person_id: Justin.id, pets_id: pet.id }; }), + ); + }); + it("should throw if no items passed", function (done) { Person.one(function (err, person) { should.equal(err, null);