From 75c2a5073592140a81c236ce7ecbd327a48fef84 Mon Sep 17 00:00:00 2001 From: JimmyDaddy Date: Mon, 21 Feb 2022 11:04:11 +0800 Subject: [PATCH] fix: upsert should set createdAt by default while it not set (#277) Co-authored-by: JimmyDaddy --- src/drivers/abstract/spellbook.js | 6 +-- src/drivers/mysql/spellbook.js | 5 +- test/integration/postgres.test.js | 23 +++++++++ test/integration/sqlite.test.js | 23 +++++++++ test/integration/suite/basics.test.js | 71 +++++++++++++++++++++++++++ test/unit/spell.test.js | 30 ++++++++++- 6 files changed, 151 insertions(+), 7 deletions(-) diff --git a/src/drivers/abstract/spellbook.js b/src/drivers/abstract/spellbook.js index 74d606e1..37d55a33 100644 --- a/src/drivers/abstract/spellbook.js +++ b/src/drivers/abstract/spellbook.js @@ -356,15 +356,14 @@ function formatInsert(spell) { } } else { for (const name in involved) { - // upsert should not update createdAt - if (updateOnDuplicate && createdAt && name === createdAt) continue; attributes.push(Model.attributes[name]); } } for (const entry of attributes) { columns.push(entry.columnName); - if (updateOnDuplicate && createdAt && entry.name === createdAt) continue; + if (updateOnDuplicate && createdAt && entry.name === createdAt + && !(Array.isArray(updateOnDuplicate) && updateOnDuplicate.includes(createdAt))) continue; updateOnDuplicateColumns.push(entry.columnName); } @@ -385,7 +384,6 @@ function formatInsert(spell) { } for (const name in sets) { const value = sets[name]; - // upsert should not update createdAt columns.push(Model.unalias(name)); if (value instanceof Raw) { values.push(SqlString.raw(value.value)); diff --git a/src/drivers/mysql/spellbook.js b/src/drivers/mysql/spellbook.js index 0569f200..796cf402 100644 --- a/src/drivers/mysql/spellbook.js +++ b/src/drivers/mysql/spellbook.js @@ -49,7 +49,10 @@ module.exports = { const sets = []; // Make sure the correct LAST_INSERT_ID is returned. // - https://stackoverflow.com/questions/778534/mysql-on-duplicate-key-last-insert-id - sets.push(`${escapeId(primaryColumn)} = LAST_INSERT_ID(${escapeId(primaryColumn)})`); + // if insert attributes include primary column, `primaryKey = LAST_INSERT_ID(primaryKey)` is not need any more + if (!columns.includes(primaryColumn)) { + sets.push(`${escapeId(primaryColumn)} = LAST_INSERT_ID(${escapeId(primaryColumn)})`); + } sets.push(...columns.map(column => `${escapeId(column)}=VALUES(${escapeId(column)})`)); return `ON DUPLICATE KEY UPDATE ${sets.join(', ')}`; diff --git a/test/integration/postgres.test.js b/test/integration/postgres.test.js index 9fcc66fb..5e1a1051 100644 --- a/test/integration/postgres.test.js +++ b/test/integration/postgres.test.js @@ -2,6 +2,7 @@ const assert = require('assert').strict; const path = require('path'); +const sinon = require('sinon'); const { connect, raw } = require('../..'); @@ -57,6 +58,28 @@ describe('=> upsert', function () { new Post({ id: 1, title: 'New Post', createdAt: raw('CURRENT_TIMESTAMP()'), updatedAt: raw('CURRENT_TIMESTAMP()') }).upsert().toString(), `INSERT INTO "articles" ("id", "title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES (1, 'New Post', false, 0, CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP()) ON CONFLICT ("id") DO UPDATE SET "id"=EXCLUDED."id", "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified" RETURNING "id"` ); + const date = new Date(2017, 11, 12); + const fakeDate = date.getTime(); + sinon.useFakeTimers(fakeDate); + assert.equal( + new Post({ id: 1, title: 'New Post', createdAt: date, updatedAt: date }).upsert().toString(), + `INSERT INTO "articles" ("id", "title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES (1, 'New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "id"=EXCLUDED."id", "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified" RETURNING "id"` + ); + assert.equal( + new Post({ title: 'New Post', createdAt: date, updatedAt: date }).upsert().toString(), + `INSERT INTO "articles" ("title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES ('New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified" RETURNING "id"` + ); + // default set createdAt + assert.equal( + new Post({ id: 1, title: 'New Post' }).upsert().toString(), + `INSERT INTO "articles" ("id", "title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES (1, 'New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "id"=EXCLUDED."id", "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified" RETURNING "id"` + ); + + assert.equal( + Post.upsert({ title: 'New Post' }).toSqlString(), + `INSERT INTO "articles" ("title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES ('New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified" RETURNING "id"` + ); + }); it('upsert returning multiple columns', function() { diff --git a/test/integration/sqlite.test.js b/test/integration/sqlite.test.js index 48341d23..cc99821a 100644 --- a/test/integration/sqlite.test.js +++ b/test/integration/sqlite.test.js @@ -2,6 +2,8 @@ const assert = require('assert').strict; const path = require('path'); +const sinon = require('sinon'); + const { connect, raw, Bone } = require('../..'); const { checkDefinitions } = require('./helpers'); @@ -63,5 +65,26 @@ describe('=> upsert (sqlite)', function () { new Post({ id: 1, title: 'New Post', createdAt: raw('CURRENT_TIMESTAMP()'), updatedAt: raw('CURRENT_TIMESTAMP()') }).upsert().toString(), `INSERT INTO "articles" ("id", "title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES (1, 'New Post', false, 0, CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP()) ON CONFLICT ("id") DO UPDATE SET "id"=EXCLUDED."id", "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified"` ); + const date = new Date(2017, 11, 12); + const fakeDate = date.getTime(); + sinon.useFakeTimers(fakeDate); + assert.equal( + new Post({ id: 1, title: 'New Post', createdAt: date, updatedAt: date }).upsert().toString(), + `INSERT INTO "articles" ("id", "title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES (1, 'New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "id"=EXCLUDED."id", "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified"` + ); + assert.equal( + new Post({ title: 'New Post', createdAt: date, updatedAt: date }).upsert().toString(), + `INSERT INTO "articles" ("title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES ('New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified"` + ); + // default set createdAt + assert.equal( + new Post({ id: 1, title: 'New Post' }).upsert().toString(), + `INSERT INTO "articles" ("id", "title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES (1, 'New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "id"=EXCLUDED."id", "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified"` + ); + + assert.equal( + Post.upsert({ title: 'New Post' }).toSqlString(), + `INSERT INTO "articles" ("title", "is_private", "word_count", "gmt_create", "gmt_modified") VALUES ('New Post', false, 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON CONFLICT ("id") DO UPDATE SET "title"=EXCLUDED."title", "is_private"=EXCLUDED."is_private", "word_count"=EXCLUDED."word_count", "gmt_modified"=EXCLUDED."gmt_modified"` + ); }); }); diff --git a/test/integration/suite/basics.test.js b/test/integration/suite/basics.test.js index b31a3fef..9a5e234f 100644 --- a/test/integration/suite/basics.test.js +++ b/test/integration/suite/basics.test.js @@ -853,6 +853,16 @@ describe('=> Basic', () => { assert.equal(await post.upsert(), 0); }); + it('bone.upsert should set createdAt default', async () => { + await (new Post({ title: 'New Post', isPrivate: 0 }).upsert()); + const post = await Post.findOne('title = ?', 'New Post'); + assert(post.createdAt); + const date = new Date(2017, 11, 12); + await (new Post({ title: 'post 1', isPrivate: 0, createdAt: date }).upsert()); + const post1 = await Post.findOne('title = ?', 'post 1'); + assert.deepEqual(post1.createdAt, date); + }); + it('bone.upsert() should return affectedRows', async function() { const post = new Post({ title: 'New Post', isPrivate: 0 }); // INSERT ... UPDATE returns 1 if the INSERT branch were chosen @@ -892,6 +902,16 @@ describe('=> Basic', () => { assert.equal(count, 1); }); + it('Bone.upsert should set createdAt default', async () => { + await Post.upsert({ title: 'New Post', isPrivate: 0 }); + const post = await Post.findOne('title = ?', 'New Post'); + assert(post.createdAt); + const date = new Date(2017, 11, 12); + await Post.upsert({ title: 'post 1', isPrivate: 0, createdAt: date }); + const post1 = await Post.findOne('title = ?', 'post 1'); + assert.deepEqual(post1.createdAt, date); + }); + it('Bone.upsert() should return affectedRows', async function() { const tag = await Tag.create({ name: 'Sekiro', type: 1 }); // INSERT ... UPDATE returns 1 if the INSERT branch were chosen @@ -1156,6 +1176,57 @@ describe('=> Basic', () => { assert.equal(p2.title, 'Leah1'); }); + it('Bone.bulkCreate() updateOnDuplicate with createdAt default or set', async () => { + await User.bulkCreate([ + { nickname: 'TYRAEL', email: 'hello@h1.com', status: 1 }, + { nickname: 'LEAH', email: 'hello1@h1.com', status: 1 }, + ], { + updateOnDuplicate: true, + }); + + assert.equal(await User.count(), 2); + let p1 = await User.findOne({ email: 'hello@h1.com' }); + assert.equal(p1.nickname, 'TYRAEL'); + const p1CreatedAt = p1.createdAt; + assert(p1CreatedAt); + let p2 = await User.findOne({ email: 'hello1@h1.com' }); + assert.equal(p2.nickname, 'LEAH'); + const p2CreatedAt = p2.createdAt; + assert(p2CreatedAt); + + await User.bulkCreate([ + { nickname: 'TYRAEL1', email: 'hello@h1.com', status: 1 }, + { nickname: 'LEAH1', email: 'hello1@h1.com', status: 1 }, + ], { + updateOnDuplicate: [ 'nickname', 'status' ] + }); + + let p1Updated1 = await User.findOne({ email: 'hello@h1.com' }); + assert.equal(p1Updated1.nickname, 'TYRAEL1'); + const p1Updated1CreatedAt = p1Updated1.createdAt; + assert.deepEqual(p1Updated1CreatedAt, p1CreatedAt); + let p2Updated1 = await User.findOne({ email: 'hello1@h1.com' }); + assert.equal(p2Updated1.nickname, 'LEAH1'); + const p2Updated1CreatedAt = p2Updated1.createdAt; + assert.deepEqual(p2Updated1CreatedAt, p2CreatedAt); + + await User.bulkCreate([ + { nickname: 'TYRAEL2', email: 'hello@h1.com', status: 1 }, + { nickname: 'LEAH2', email: 'hello1@h1.com', status: 1 }, + ], { + updateOnDuplicate: [ 'nickname', 'status', 'createdAt' ] + }); + + let p1Updated2 = await User.findOne({ email: 'hello@h1.com' }); + assert.equal(p1Updated2.nickname, 'TYRAEL2'); + const p1Updated2CreatedAt = p1Updated2.createdAt; + assert.notDeepEqual(p1Updated2CreatedAt, p1CreatedAt); + let p2Updated2 = await User.findOne({ email: 'hello1@h1.com' }); + assert.equal(p2Updated2.nickname, 'LEAH2'); + const p2Updated2CreatedAt = p2Updated2.createdAt; + assert.notDeepEqual(p2Updated2CreatedAt, p2CreatedAt); + }); + it('Bone.bulkCreate() should work with updateOnDuplicate keys', async () => { await User.bulkCreate([ { nickname: 'Tyrael', email: 'hello@h1.com', status: 1 }, diff --git a/test/unit/spell.test.js b/test/unit/spell.test.js index 734a73b0..db2db13a 100644 --- a/test/unit/spell.test.js +++ b/test/unit/spell.test.js @@ -55,9 +55,35 @@ describe('=> Spell', function() { it('insert ... on duplicate key update', function() { const date = new Date(2017, 11, 12); + const fakeDate = date.getTime(); + sinon.useFakeTimers(fakeDate); assert.equal( new Post({ id: 1, title: 'New Post', createdAt: date, updatedAt: date }).upsert().toString(), - "INSERT INTO `articles` (`id`, `is_private`, `title`, `word_count`, `gmt_create`, `gmt_modified`) VALUES (1, 0, 'New Post', 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON DUPLICATE KEY UPDATE `id` = LAST_INSERT_ID(`id`), `id`=VALUES(`id`), `is_private`=VALUES(`is_private`), `title`=VALUES(`title`), `word_count`=VALUES(`word_count`), `gmt_modified`=VALUES(`gmt_modified`)" + "INSERT INTO `articles` (`id`, `is_private`, `title`, `word_count`, `gmt_create`, `gmt_modified`) VALUES (1, 0, 'New Post', 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON DUPLICATE KEY UPDATE `id`=VALUES(`id`), `is_private`=VALUES(`is_private`), `title`=VALUES(`title`), `word_count`=VALUES(`word_count`), `gmt_modified`=VALUES(`gmt_modified`)" + ); + assert.equal( + new Post({ title: 'New Post', createdAt: date, updatedAt: date }).upsert().toString(), + "INSERT INTO `articles` (`is_private`, `title`, `word_count`, `gmt_create`, `gmt_modified`) VALUES (0, 'New Post', 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON DUPLICATE KEY UPDATE `id` = LAST_INSERT_ID(`id`), `is_private`=VALUES(`is_private`), `title`=VALUES(`title`), `word_count`=VALUES(`word_count`), `gmt_modified`=VALUES(`gmt_modified`)" + ); + // default set createdAt + assert.equal( + new Post({ id: 1, title: 'New Post' }).upsert().toString(), + "INSERT INTO `articles` (`id`, `is_private`, `title`, `word_count`, `gmt_create`, `gmt_modified`) VALUES (1, 0, 'New Post', 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON DUPLICATE KEY UPDATE `id`=VALUES(`id`), `is_private`=VALUES(`is_private`), `title`=VALUES(`title`), `word_count`=VALUES(`word_count`), `gmt_modified`=VALUES(`gmt_modified`)" + ); + + assert.equal( + Post.upsert({ title: 'New Post' }).toSqlString(), + "INSERT INTO `articles` (`is_private`, `title`, `word_count`, `gmt_create`, `gmt_modified`) VALUES (0, 'New Post', 0, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON DUPLICATE KEY UPDATE `id` = LAST_INSERT_ID(`id`), `is_private`=VALUES(`is_private`), `title`=VALUES(`title`), `word_count`=VALUES(`word_count`), `gmt_modified`=VALUES(`gmt_modified`)" + ); + + assert.equal( + new Book({ name: 'Dark', price: 100 }).upsert().toSqlString(), + "INSERT INTO `books` (`name`, `price`, `gmt_create`, `gmt_modified`) VALUES ('Dark', 100, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON DUPLICATE KEY UPDATE `isbn` = LAST_INSERT_ID(`isbn`), `name`=VALUES(`name`), `price`=VALUES(`price`), `gmt_modified`=VALUES(`gmt_modified`)" + ); + + assert.equal( + new Book({ isbn: 10000, name: 'Dark', price: 100 }).upsert().toSqlString(), + "INSERT INTO `books` (`isbn`, `name`, `price`, `gmt_create`, `gmt_modified`) VALUES (10000, 'Dark', 100, '2017-12-12 00:00:00.000', '2017-12-12 00:00:00.000') ON DUPLICATE KEY UPDATE `isbn`=VALUES(`isbn`), `name`=VALUES(`name`), `price`=VALUES(`price`), `gmt_modified`=VALUES(`gmt_modified`)" ); }); @@ -729,7 +755,7 @@ describe('=> Spell', function() { it('upsert with raw sql', function () { assert.equal( new Post({ id: 1, title: 'New Post', createdAt: raw('CURRENT_TIMESTAMP()'), updatedAt: raw('CURRENT_TIMESTAMP()') }).upsert().toString(), - "INSERT INTO `articles` (`id`, `is_private`, `title`, `word_count`, `gmt_create`, `gmt_modified`) VALUES (1, 0, 'New Post', 0, CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP()) ON DUPLICATE KEY UPDATE `id` = LAST_INSERT_ID(`id`), `id`=VALUES(`id`), `is_private`=VALUES(`is_private`), `title`=VALUES(`title`), `word_count`=VALUES(`word_count`), `gmt_modified`=VALUES(`gmt_modified`)" + "INSERT INTO `articles` (`id`, `is_private`, `title`, `word_count`, `gmt_create`, `gmt_modified`) VALUES (1, 0, 'New Post', 0, CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP()) ON DUPLICATE KEY UPDATE `id`=VALUES(`id`), `is_private`=VALUES(`is_private`), `title`=VALUES(`title`), `word_count`=VALUES(`word_count`), `gmt_modified`=VALUES(`gmt_modified`)" ); });